Skip to content

Order Email Sender #13769

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
pawcioma opened this issue Feb 21, 2018 · 8 comments
Closed

Order Email Sender #13769

pawcioma opened this issue Feb 21, 2018 · 8 comments
Assignees
Labels
Fixed in 2.1.x The issue has been fixed in 2.1 release line Fixed in 2.2.x The issue has been fixed in 2.2 release line Fixed in 2.3.x The issue has been fixed in 2.3 release line Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Issue: Format is not valid Gate 1 Failed. Automatic verification of issue format is failed Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release

Comments

@pawcioma
Copy link
Contributor

pawcioma commented Feb 21, 2018

Probably I found wrong logical in "\Magento\Sales\Model\Order\Email\Sender::checkAndSend" because you catch Exception and then you return true. In database will be information about send_email and email_sent as true, but email can not be sent.

Original code:
`protected function checkAndSend(Order $order)
{
$this->identityContainer->setStore($order->getStore());
if (!$this->identityContainer->isEnabled()) {
return false;
}
$this->prepareTemplate($order);

    /** @var SenderBuilder $sender */
    $sender = $this->getSender();

    try {
        $sender->send();
        $sender->sendCopyTo();
    } catch (\Exception $e) {
        $this->logger->error($e->getMessage());
    }

    return true;

}`

For my opinion should be:
`protected function checkAndSend(Order $order)
{
$this->identityContainer->setStore($order->getStore());

    if (!$this->identityContainer->isEnabled()) {
        return false;
    }

    $this->prepareTemplate($order);

    /** @var SenderBuilder $sender */
    $sender = $this->getSender();

    try {
        $sender->send();
        $sender->sendCopyTo();
    } catch (\Exception $e) {
        $this->logger->error($e->getMessage());

        return false;
    }

    return true;
}`
@magento-engcom-team magento-engcom-team added Issue: Format is not valid Gate 1 Failed. Automatic verification of issue format is failed Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release labels Feb 21, 2018
@magento-engcom-team
Copy link
Contributor

@pawcioma, thank you for your report.
We've acknowledged the issue and added to our backlog.

@pawcioma
Copy link
Contributor Author

If you want I can take care of it.

@ArjenMiedema
Copy link

@pawcioma you can assign yourself to this issue and create a pull request so it can be reviewed and merged.

@pawcioma
Copy link
Contributor Author

Ok . I will prepare "pull request", but how can I assign to the issue?

@ArjenMiedema
Copy link

Just did it for you. Good luck fixing the issue!

@magento-engcom-team magento-engcom-team added the Fixed in 2.2.x The issue has been fixed in 2.2 release line label Mar 8, 2018
@magento-engcom-team
Copy link
Contributor

Hi @pawcioma. Thank you for your report.
The issue has been fixed in #13878 by @pawcioma in 2.2-develop branch
Related commit(s):

The fix will be available with the upcoming 2.2.5 release.

@magento-engcom-team
Copy link
Contributor

Hi @pawcioma. Thank you for your report.
The issue has been fixed in #14051 by @pawcioma in 2.3-develop branch
Related commit(s):

The fix will be available with the upcoming 2.3.0 release.

@sidolov
Copy link
Contributor

sidolov commented Jul 26, 2018

Hi @pawcioma. Thank you for your report.
The issue has been fixed in #17087 by @mageprince in 2.1-develop branch
Related commit(s):

The fix will be available with the upcoming 2.1.15 release.

@sidolov sidolov added the Fixed in 2.1.x The issue has been fixed in 2.1 release line label Jul 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Fixed in 2.1.x The issue has been fixed in 2.1 release line Fixed in 2.2.x The issue has been fixed in 2.2 release line Fixed in 2.3.x The issue has been fixed in 2.3 release line Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Issue: Format is not valid Gate 1 Failed. Automatic verification of issue format is failed Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release
Projects
None yet
Development

No branches or pull requests

4 participants