[Backport][2.2] Alternative fix for Multi Store Emails issue, Fix Async Emails issues, Fix Multiple Email issues

[gwharton]

This PR replaces PR #16461 as for some reason, that PR has become detached from my repo and I don't think it will merge properly.

Original Pull Request

#18471

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.

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. 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
3. Store Email Addresses are not used anymore #14945:Store Email Addresses are not used anymore
4. Magento 2.2.4 not sending from sales sender #16355:Magento 2.2.4 not sending from sales sender

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

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)

[ihor-sviziev]

Hi @sidolov,
Any updates from discussing this issue with architects?

[ihor-sviziev]

@gwharton is it really backport of some fix from 2.3-develop branch?

[gwharton]

The same fault is present in 2.3. Magento is prioritising 2.3 now as i guess 2.3.0 is iminent. There is a new pr for 2.3 and i labelled this one backport. Hopefully they will make the decision on the 2.3 pr and this one will follow without discussion.

[sidolov]

Hi @ihor-sviziev , @gwharton , we discussed this case with @buskamuza recently. We still not allowed to add new parameters to public methods, the best way is to deprecate the old method and create the new one, like setFromByStore and change all usages to the new method.

[gwharton]

ok, someone else will have to pick this one up.

[hostep]

Just informational, but there is official Magento documentation around how to use the cweagans/composer-patches module: https://support.magento.com/hc/en-us/articles/360005484154-Create-a-patch-for-a-Magento-2-Composer-installation-from-a-GitHub-commit

Although it doesn't go over complex PR's like this one which span multiple modules, so you need to split out the diff in multiple diff files, one per module (magento/module-sales & magento/framework).

[ladle3000]

Ok thanks @hostep and @ihor-sviziev I tried for hours yesterday and did manage to figure out splitting to two modules.

Still getting failure on the framework one due to some unexpected ending. I'll have to look more today.

I still don't even know if this PR will fix my issue for sure yet.

[ladle3000]

I could not get the framework diff to apply, so I've just copied and pasted all the 6 files from raw to my magento and now get this error on testing an email send

Zend\Mail\Transport\Smtp transport expects either a Sender or at least one From address in the Message; none provided

Really need a fix for this, I'm a merchant running a store and sending out generic looking emails now.

[gwharton]

You should only copy the changes, not the entire raw file. By copying the entire raw file, you have effectively overwritten your 2.2.7 files with the 2.2-develop files which may contain other changes not compatible with the rest of 2.2.7.

Just go by the diff, if the line is red (prefixed with a "-"), it is removed, if the line is green (prefixed with a "+"), it is added. The white lines are not changed, and are there for you to locate the lines that have changed.

If you just want to get it to work, you could ignore the files with Test in the filename as they are not needed.

Be aware if this PR doesnt make it into 2.2.8 then when you upgrade to 2.2.8 you will need to make these changes again.

[ladle3000]

I'm on 2.3 now. Are these changes still compatible if I go through the trouble.

…

On Tue, Mar 5, 2019 at 12:31 PM gwharton ***@***.***> wrote: You should only copy the changes, not the entire raw file. By copying the entire raw file, you have effectively overwritten your 2.2.7 files with the 2.2-develop files which may contain other changes not compatible with the rest of 2.2.7. Just go by the diff, if the line is red (prefixed with a "-"), it is removed, if the line is green (prefixed with a "+"), it is added. The white lines are not changed, and are there for you to locate the lines that have changed. If you just want to get it to work, you could ignore the files with Test in the filename as they are not needed. Be aware if this PR doesnt make it into 2.2.8 then when you upgrade to 2.2.8 you will need to make these changes again. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#18472 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACQbJXGSSjnz-rkto5P2pFfeAXsCH0cIks5vTqhIgaJpZM4XNfnI> .

[gwharton]

PR #18471 is the equivalent fix for 2.3

This has already been merged into 2.3-develop and will be included in release 2.3.1

You need to revert what you have done, and make the changes according to PR #18471

[ladle3000]

So shouldn't copy of raw 2.3 develop files do the trick?

…

On Tue, Mar 5, 2019, 12:40 PM gwharton ***@***.***> wrote: PR #18471 <#18471> is the equivalent fix for 2.3 This has already been merged into 2.3-develop and will be included in release 2.3.1 You need to revert what you have done, and make the changes according to PR #18471 <#18471> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#18472 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACQbJRJWnn9h3wCwYZzMoXpzF2zdUzTwks5vTqwFgaJpZM4XNfnI> .

[gwharton]

Don't copy raw files from develop into a release version........ full stop. You're asking for trouble as the files may not be compatible.

If you are running 2.3.0 then revert your changes by copying over the files from YOUR release to get you back to the originals. Then if you must apply the PR manually, do so by only making the changes identified in the diff.

[ladle3000]

Ok. Good to know. I'm only trying on dev first, but glad to know about not copying the raw files.

…

On Tue, Mar 5, 2019, 12:57 PM gwharton ***@***.***> wrote: Don't copy raw files from develop into a release version........ full stop. You're asking for trouble as the files may not be compatible. If you are running 2.3.0 then revert your changes by copying over the files from YOUR release to get you back to the originals. Then if you must apply the PR manually, do so by only making the changes identified in the diff. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#18472 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACQbJTsAJpy78tJtWJ1dzC2fuPnMNMbKks5vTrAEgaJpZM4XNfnI> .

[ghost]

Hi @gwharton, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

[qrider]

I am seeing this for a 2.3.0 site still. From what I can tell 2.3 contains the patch. So not sure why it is not working.

[gwharton]

2.3.1 i believe got this. Not 2.3.0.

[qrider]

thanks

[LandonL82]

Has anyone figured out how to apply this patch to a composer 2.2.7 install?

[crankycyclops]

Has anyone figured out how to apply this patch to a composer 2.2.7 install?

@LandonL82, just make the following string replacements in your text editor before applying the patch:

app/code/Magento/Sales -> vendor/magento/module-sales
lib/internal/Magento/Framework -> vendor/magento/framework

[sudma]

@LandonL82
Here is the official Magento blog post about applying the patches via composer.

https://support.magento.com/hc/en-us/articles/360005484154-Create-a-patch-for-a-Magento-2-Composer-installation-from-a-GitHub-commit

[tuyennn]

@gwharton Good to have the backward compatible with TransportBuilder.php on <=2.2.6 on your PR

[ladle3000]

Yeah. I never got this patch working for me unfortunately either. I'm using an smtp module that the developer did a second check on the send from to temporarily fix it. It's the free mage plaza one if you need it. Just open a ticket with them and they'll add the same for you I'm sure

…

On Thu, Jun 13, 2019, 5:42 AM farhadfrz ***@***.***> wrote: I applied the changes to Magento 2.2.8, but still the admin emails are sent from default email! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#18472?email_source=notifications&email_token=AASBWJO5AWCE3GQCWSYKHTTP2II7FA5CNFSM4FZV7HEKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXTEITQ#issuecomment-501630030>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AASBWJNBC7ADW3X6DVNM2MDP2II7FANCNFSM4FZV7HEA> .

[tuyennn]

On 2.2.6 you should exclude Mail.php from patch because it was not implemented some missing functions. Actually the Mail.php was changed since 2.2.7

[MiomenteMUC]

Still got the problem in 2.3.1 for every payment method except Paypal and Amazon Pay.
Expectation:

• Send BCC Copy to admin when order is placed.
  Reality:
• Customer gets Order Mail, admin does not get Copy.
  It´s just working for Paypal and Amazon. Not even the integrated standard Magento prepayment or pick-up at store.
  Any solutions? No Error in Logs.
  Changes from https://github.com/magento/magento2/pull/18472/files/16afc0962463261e4464456b9bdaad4e8f3be924 are implemented, cause it´s 2.3.1 and was merged in 2.3.0 like mentioned before.

Regards, Loqic

[ihor-sviziev]

Hi @Loqic2633,
If you're still having this issue on latest Magento version - please create separate issue for that

Thank you!