[2.3] Fix Store Emails - Missing From Name #18473

gwharton · 2018-10-08T19:02:17Z

The refactoring of the Framework/Mail/Message.php for Magento 2.3 does not implement the second parameter in the setFrom() function.

1st Parameter is email
2nd Parameter (missing) is from address

Description

This PR adds the missing from name to the setFrom function

Fixed Issues (if relevant)

Store emails have correct from address but do not have a from name #18470: Store emails have correct from address but do not have a from name

Manual testing scenarios

Apply [2.3] Alternative fix for Multi Store Emails issue, Fix Async Emails issues, Fix Multiple Email issues #18471 so emails work correctly in the first place
Set a valid from name and from email address for sales contact
Add test product
Purchase test product
Verify that confirmation email has the correct from address and name

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)

rogyar · 2018-10-09T09:43:55Z

Hi @gwharton. Thank you for your contribution. Adding new arguments to public methods is forbidden because of possible backward compatibility issues (https://devdocs.magento.com/guides/v2.2/contributor-guide/backward-compatible-development/#php). Instead of introducing new arguments to public methods you may mark the old method as deprecated and create a new one with all necessary parameters. Thank you

gwharton · 2018-10-09T10:22:51Z

Hi @rogyar

I understand the need for that rule, however implementing a new function and deprecating the old one is problematic. I'll explain.

Magento 2.2 code base uses a mixture of setFrom($email) and setFrom($email, $name) calls
Magento 2.2 Framework implements setFrom($email, $name=null)

Magento 2.3 code base uses a mixture of setFrom($email) and setFrom($email, $name) calls
Magento 2.3 Framework implements setFrom($email)

This just looks like a plain mistake in the implementation of the setFrom routine in 2.3.

If I was to add a new function setFromWithName($email, $name) then I would have to go through the entire codebase and find every calling of setFrom($email, $name) and changing it to the new function, further diverging the 2.2 and 2.3 codebases.

I would have thought that common sense in this situation would allow the adding of a "default" parameter to the existing function, as this would mean that nothing would change anywhere else. Existing calls to setFrom($email) would continue unaffected and calls to setFrom($email, $name) would now work as designed.

rogyar · 2018-10-09T13:49:57Z

Hi @gwharton. I completely agree with your point. There might be a mistake during the initial implementation.
However, there's a big chance that some 3rd party implementation extends the setFrom method from the original class. In that case, by changing the original public class we can kill the existing stores. That's why we have such strict backward compatibility rules.

rogyar · 2018-10-09T14:05:58Z

@magento-engcom-team give me 2.3-develop instance

magento-engcom-team · 2018-10-09T14:06:02Z

Hi @rogyar. Thank you for your request. I'm working on Magento 2.3-develop instance for you

magento-engcom-team · 2018-10-09T14:06:38Z

Hi @rogyar, here is your Magento instance.
Admin access: https://i-18473-2-3-develop.engcom.dev.magento.com/admin
Login: admin Password: 123123q
Instance will be terminated in up to 3 hours.

brucevalle · 2019-01-23T14:02:22Z

@gwharton could please let me know how to implement this fix on 2.3? We have set our email address mail@myemail.com but the clients got the email from storedep@server.webworksla.com that its a different server email account. Not sure why happen this.

gwharton · 2019-01-23T14:50:04Z

The Pull Request detailing the fix for 2.3.0 can be found here.

#18471

This is scheduled to be included in the 2.3.1 release.

brucevalle · 2019-01-23T14:58:24Z

Thanks sorry but could explain how to implement it?

…

On Jan 23, 2019, at 9:50 AM, gwharton ***@***.***> wrote: The Pull Request detailing the fix for 2.3.0 can be found here. #18471 <#18471> This is scheduled to be included in the 2.3.1 release. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#18473 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AD_TF-GjlsBryqRmpJJPcRnyirQ43x4Mks5vGHbNgaJpZM4XNf-d>.

ladle3000 · 2019-03-05T21:59:40Z

@gwharton I patched all my 2.3 files with the PR 18471 and now still have this issue of no FROM NAME.

Can you tell me how I can get that working for my customers as well?

gwharton · 2019-03-06T01:06:51Z

Hi @snoroozi You're almost there. This PR was combined into PR #18471 so the underlying fix should already be there. Note though, that any third party module that uses the $message->setFrom($emailaddress) function, will need to be updated to use the new $message->setFromAddress($emailaddress, $fromname) function introduced in #18471. Note #18471 updated all Magento functions but third party modules may not have implemented the fix yet, so you will have to do this manually with whichever modules you are using. Search in your modules for "setFrom" and adjust accordingly.

ladle3000 · 2019-03-06T01:32:06Z

OK. I'm trying this now. Thanks for getting back to me @gwharton

…

On Tue, Mar 5, 2019 at 8:07 PM gwharton ***@***.***> wrote: Hi @snoroozi <https://github.com/snoroozi> You're almost there. This PR was combined into PR #18471 <#18471> so the underlying fix should already be there. Note though, that any third party module that uses the $message->setFrom($emailaddress) function, will need to be updated to use the new $message->setFromAddress($emailaddress, $fromname) function introduced in #18471 <#18471>. Note #18471 <#18471> updated all Magento functions but third party modules may not have implemented the fix yet, so you will have to do this manually with whichever modules you are using. Search in your modules for "setFrom" and adjust accordingly. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#18473 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACQbJX1xpJsfxHbnl09bgmZVxTFkFJ2Vks5vTxTWgaJpZM4XNf-d> .

ladle3000 · 2019-03-06T01:56:33Z

@gwharton Like this?

public_html$ grep -rin 'setFrom' app/
app/code/Mageplaza/GiftCard/Helper/Email.php.orig:233: ->setFrom($sender)
app/code/Mageplaza/GiftCard/Helper/Email.php:234: ->setFrom($sender)
app/code/Mageplaza/GiftCard/Helper/Email.php.modif:234: ->setFrom($sender)
app/code/Infortis/Infortis/view/adminhtml/web/js/jquery/spectrum/spectrum.js:327: textInput.change(setFromTextInput);
app/code/Infortis/Infortis/view/adminhtml/web/js/jquery/spectrum/spectrum.js:329: setTimeout(setFromTextInput, 1);
app/code/Infortis/Infortis/view/adminhtml/web/js/jquery/spectrum/spectrum.js:331: textInput.keydown(function (e) { if (e.keyCode == 13) { setFromTextInput(); } });
app/code/Infortis/Infortis/view/adminhtml/web/js/jquery/spectrum/spectrum.js:582: function setFromTextInput() {
app/code/Magebird/Popup/Model/Subscriber.php:56: ->setFrom($senderInfo)
app/code/Firebear/ImportExport/view/base/web/js/lib/ace/mode-sqlserver.js:69: "@@DATEFIRST|@@language|CURRENT_TIMESTAMP|DATEADD|DATEDIFF|DATEFROMPARTS|DATENAME|DATEPART|DATETIME2FROMPARTS|DATETIMEFROMPARTS|DATETIMEOFFSETFROMPARTS|DAY|EOMONTH|GETDATE|GETUTCDATE|ISDATE|MONTH|SET DATEFIRST|SET DATEFORMAT|SET LANGUAGE|SMALLDATETIMEFROMPARTS|SP_HELPLANGUAGE|SWITCHOFFSET|SYSDATETIME|SYSDATETIMEOFFSET|SYSUTCDATETIME|TIMEFROMPARTS|TODATETIMEOFFSET|YEAR|" +
app/code/Firebear/ImportExport/Model/Import/Product/Price/Rule.php:17: * @method \Magento\CatalogRule\Model\Rule setFromDate(string $value)

gwharton · 2019-03-06T02:00:27Z

Are all of your emails missing a from name, or only ones related to Mageplaza gift card/Magebird Popup.

ladle3000 · 2019-03-06T02:01:14Z

All @gwharton

ladle3000 · 2019-03-06T02:05:19Z

And here's the vendor directory. But yes, even default magento emails are still missing
vendor.txt

ladle3000 · 2019-03-06T16:52:07Z

Any thoughts @gwharton

Mageplaza has offered up a fix in their smtp module that addresses most of these issues, but also some custom extensions not working. Maybe for same reason you say here.

ladle3000 · 2019-03-07T13:00:56Z

It seems mageplaza's solution is to reset the setFrom name function as a workaround in place of patching 2.3.

Still it seems strange that none of my emails were working correctly after I did have this patch applied previously, don't you think? Or is it possible 3rd party modules could be interfering outside of their own module in the same way that amazon modules were affecting things for some merchants?

Added from name parameter to Framework/Mail/Message.php

81706f5

magento-engcom-team added Component: Framework/Mail Release Line: 2.3 labels Oct 8, 2018

rogyar self-assigned this Oct 9, 2018

rogyar added the Progress: needs update label Oct 9, 2018

gwharton closed this Oct 9, 2018

gwharton mentioned this pull request Oct 15, 2018

Confirmation emails have no FROM or FROM email address 2.2.4 #14952

Closed

gwharton reopened this Oct 16, 2018

gwharton closed this Oct 31, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[2.3] Fix Store Emails - Missing From Name #18473

[2.3] Fix Store Emails - Missing From Name #18473

gwharton commented Oct 8, 2018 •

edited

Loading

rogyar commented Oct 9, 2018

gwharton commented Oct 9, 2018

rogyar commented Oct 9, 2018

rogyar commented Oct 9, 2018

magento-engcom-team commented Oct 9, 2018

magento-engcom-team commented Oct 9, 2018

brucevalle commented Jan 23, 2019 •

edited

Loading

gwharton commented Jan 23, 2019

brucevalle commented Jan 23, 2019 via email

ladle3000 commented Mar 5, 2019

gwharton commented Mar 6, 2019

ladle3000 commented Mar 6, 2019 via email

ladle3000 commented Mar 6, 2019

gwharton commented Mar 6, 2019

ladle3000 commented Mar 6, 2019

ladle3000 commented Mar 6, 2019

ladle3000 commented Mar 6, 2019

ladle3000 commented Mar 7, 2019

[2.3] Fix Store Emails - Missing From Name #18473

[2.3] Fix Store Emails - Missing From Name #18473

Conversation

gwharton commented Oct 8, 2018 • edited Loading

Description

Fixed Issues (if relevant)

Manual testing scenarios

Contribution checklist

rogyar commented Oct 9, 2018

gwharton commented Oct 9, 2018

rogyar commented Oct 9, 2018

rogyar commented Oct 9, 2018

magento-engcom-team commented Oct 9, 2018

magento-engcom-team commented Oct 9, 2018

brucevalle commented Jan 23, 2019 • edited Loading

gwharton commented Jan 23, 2019

brucevalle commented Jan 23, 2019 via email

ladle3000 commented Mar 5, 2019

gwharton commented Mar 6, 2019

ladle3000 commented Mar 6, 2019 via email

ladle3000 commented Mar 6, 2019

gwharton commented Mar 6, 2019

ladle3000 commented Mar 6, 2019

ladle3000 commented Mar 6, 2019

ladle3000 commented Mar 6, 2019

ladle3000 commented Mar 7, 2019

gwharton commented Oct 8, 2018 •

edited

Loading

brucevalle commented Jan 23, 2019 •

edited

Loading