11740: Sending emails from Admin in Multi-Store Environment defaults to Primary Store

[RomaKis]

Description

Sending emails from Admin in Multi-Store Environment defaults to Primary Store

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

Manual testing scenarios

1. Create two websites: ABC.com (primary) and XYZ.com each with its store/storeview accordingly.
2. Make sure You have a different Sender Name and Sender Email for XYZ.com from ABC.com (Configuration -> General -> Store Email Address -> Sales Representative).
3. Have customer place order on XYZ.com.
4. Go into admin, click on order from XYZ.com, and resend order 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)

[ishakhsuvarov]

It is not encouraged to add new public methods in classes marked as api

[RomaKis]

@ishakhsuvarov, fixed. I created separate class. Notify me if you accept these changes, i'll squash commits.

[gwharton]

This is a hack. Surely the correct decision should have been to mark the old setFrom function as deprecated and introduce a new working function, setFromByStore in the API.

The resulting fix that was implemented was not good, involving making objects static that shouldnt have been static, to allow other classes access its protected members. This is far worse in my opinion and has already lead to two further issues being raised caused by this commit. Can this not be looked at again and fixed properly instead of the hack that is currently in place to avoid changing the transportBuilder class.

[ishakhsuvarov]

Looks like solution may be simplified

[ishakhsuvarov]

Is there any reason to introduce another class and method instead of just adding an optional parameter scopeId to \Magento\Framework\Mail\Template\TransportBuilder::setFrom?

[RomaKis]

@ishakhsuvarov, is it allowed? According to the backward compatibility(http://devdocs.magento.com/guides/v2.2/contributor-guide/backward-compatible-development/index.html) seems it is prohibited.
May I add optional parameter scopeId?

[ishakhsuvarov]

Correct. I overlooked that for this case

[gwharton]

@RomaKis This commit breaks situations where Magento sends multiple emails at the same time.

Just before senderBuilder sends the email it calls transportBuilder->getTransport() which resets the email ready for the next one. This includes instantiating a new $message object. From this point onwards the $message object is no longer statically shared with transportBuilderByStore.

On sending the second, and subsequent emails, the function fails becasuse although transportBuilder has a nice new fresh $message object to work on, transportBuilderByStore does not.