Skip to content
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

Alternative fix for Multi Store Emails issue, Fix Async Emails issues, Fix Multiple Email issues. #16461

Closed
wants to merge 2 commits into from

Conversation

gwharton
Copy link
Contributor

@gwharton gwharton commented Jun 29, 2018

Description

This PR removes the previously introduced transportBuilderByStore class that was introduced in 2.2.4 to fix issue #11740 as this implementation caused unwanted regression/bug when sending multiple emails/async emails.
It adds new default parameter to setFrom method of transportBuilder class.

Possible sticking point, is this adds a new default parameter to public method of an API class.

Fixed Issues (if relevant)

  1. Sending emails from Admin in Multi-Store Environment defaults to Primary Store #11740:Sending emails from Admin in Multi-Store Environment defaults to Primary Store
  2. Sales Emails Async Sending issue #16165:Sales Emails Async Sending issue
  3. New order notification is not sent to the admin #16203:New order notification is not sent to the admin
  4. Confirmation emails have no FROM or FROM email address 2.2.4 #14952:Confirmation emails have no FROM or FROM email address 2.2.4
  5. Store Email Addresses are not used anymore #14945:Store Email Addresses are not used anymore
  6. Magento 2.2.4 not sending from sales sender #16355:Magento 2.2.4 not sending from sales sender
  7. Magento 2.2.4 Multishipping Order Confirmation Emails sent only for the first order #16509:Magento 2.2.4 Multishipping Order Confirmation Emails sent only for the first order
  8. Sending multiple emails using the TransportBuilder class causes an Zend_Mail exception #14236:Sending multiple emails using the TransportBuilder class causes an Zend_Mail exception
  9. "From Header set twice" when sending an email #7739:"From Header set twice" when sending an email
  10. Magento 2.2.5 Order Email Confirmation is not working #17781:Magento 2.2.5 Order Email Confirmation is not working

Manual testing scenarios

Scenareo 1

  • Single store
  • Setup store email addresses
  • Complete checkout
  • Verify from address in confirmation email

Scenareo 2

  • Multiple store
  • Setup store email addresses for Store 1
  • Setup store email addresses for Store 2
  • Complete checkout on Store 1- Verify from address in confirmation email
  • Complete checkout on Store 2- Verify from address in confirmation email

Scenareo 3

  • Single Store
  • Setup store email addresses
  • Enable Async Email sending
  • Disable cron job
  • Complete checkout
  • Complete checkout again
  • Run cronjob once from command line
  • Ensure both confirmation emails are received

Scenareo 4

  • Single Store
  • Setup store email addresses
  • Add admin confirmation email address, using "separate email" as type.
  • Complete checkout
  • Ensure both confirmation and copy confirmation email are received.

Contribution checklist

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with unit/integration tests (if applicable)
  • All automated tests passed successfully (all builds on Travis CI are green)

@magento-cicd2
Copy link
Contributor

magento-cicd2 commented Jun 29, 2018

CLA assistant check
All committers have signed the CLA.

@magento-engcom-team
Copy link
Contributor

Hi @gwharton. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

  • @magento-engcom-team give me test instance - deploy test instance based on PR changes
  • @magento-engcom-team give me {$VERSION} instance - deploy vanilla Magento instance

For more details, please, review the Magento Contributor Assistant documentation

@gwharton gwharton changed the title Removed TransportBuilderByStore class, introduced setFromByStore method to TransportBuilder. Alternative fix for Multi Store Emails issue, Fix Async Emails issues, Fix Multiple Email issues. Jun 29, 2018
@gwharton gwharton mentioned this pull request Jun 29, 2018
@gwharton
Copy link
Contributor Author

OK, to avoid over complicating things and to clean up, I will close down all other pull requests that are attempting to fix these issues. I have taken into account all other PR's and comments in this PR. Hopefully this will be the sum of all ideas. This is the PR that will go forward.

@gwharton
Copy link
Contributor Author

Once approved and merged, I will forwardport the necessary changes to 2.3-develop to ensure both branches are consistent.

@RedAtRareCandy
Copy link

Experiencing this issue also. Originally had 2.2.3 then upgraded by command line to 2.2.4 and noticed that emails would come through to email as the hostname instead of the From set in Magento admin. I've upgraded to 2.2.5 by command line, tested this again, and this is still an issue.

The only way to get emails to send correctly is through the 3rd party MageFix_Misc module that was linked in one of the other threads.

@gwharton
Copy link
Contributor Author

gwharton commented Aug 2, 2018

@RedAtRareCandy. A workaround for the issue you are having is to disable the amazon payments module. You don't need the magefix module.

@RedAtRareCandy
Copy link

@gwharton Appreciate the suggestion but that broke the one page checkout so I'm afraid that's not a workaround for me. MageFix_Misc works great though :)

@hostep
Copy link
Contributor

hostep commented Aug 3, 2018

@RedAtRareCandy : just FYI since you already have a solution (or for other people finding this):

The Amazon_Login module depends on the Amazon_Payment module and disabling only the latter will indeed cause the checkout to no longer work, so you have to disable both these modules by running:
bin/magento module:disable Amazon_Login Amazon_Payment

@RedAtRareCandy
Copy link

Thanks @hostep , that worked. Do you know why though? I don't want to have those modules disabled if they added functionality to the checkout - did they?

@hostep
Copy link
Contributor

hostep commented Aug 7, 2018

@RedAtRareCandy: just read the description on their github page to see what these modules are for: https://github.com/amzn/amazon-payments-magento-2-plugin/
If you don't need those functionalities, then just leave those modules disabled.

The issue is going to get fixed in an update of the Amazon module presumably, and also might get fixed in Magento itself by this PR (since that's probably the root cause). But I don't have any knowledge about time frames for those fixes.

@gwharton
Copy link
Contributor Author

Whats the general view on this one inside Magento? is it a goer. Anything else I can do to help push forward?

@ihor-sviziev
Copy link
Contributor

Hi,
Right now we have interal discussion about this issue. I'll send an update once we'll have some results

@matthewvolk
Copy link

Hi,
I'm assuming this PR is not included in 2.2.6 then. Following up on the last discussion about an "internal discussion", do we know when we might be able to see this patch applied?

@gwharton
Copy link
Contributor Author

gwharton commented Oct 8, 2018

Replaced with PR #18472. This PR has become detached from source repo somehow, so closing and opening new PR.

@gwharton gwharton closed this Oct 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants