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

Fixed #8177 & related bugs to show the correct "From" email information #8264

Open
wants to merge 1 commit into
base: hotfix-7.10.x
Choose a base branch
from

Conversation

thingumajig42
Copy link

Also fixes:
-#7407-"Users may send as themselves" broken - Invalid address: (punyEncode)
-#7692-From adress incorrect when replying to email

Reintroduced the code that was removed as a fix for #7407 and fixed it
by checking for the case within the condition.

Resolves: #8177, #7692, #7407

Description

Reintroduced the code that was removed which was causing issues #8177 & #7692 as a fix for #7407 and fixed the #7407 issue by checking for the case within the condition.

Motivation and Context

Fixed bugs.

How To Test This

Please look at the resolved issue for more information on testing this fix.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Final checklist

  • My code follows the code style of this project found here.
  • My change requires a change to the documentation.
  • I have read the How to Contribute guidelines.

…il information

Also fixes:
  -salesagility#8177-Email sent from case does not have the correct "From" information
  -salesagility#7407-"Users may send as themselves" broken - Invalid address: (punyEncode)
  -salesagility#7692-From adress incorrect when replying to email

Reintroduced the code that was removed as a fix for salesagility#7407 and fixed it
by checking for the case within the condition.

Resolves: salesagility#8177, salesagility#7692, salesagility#7407
@codecov-io
Copy link

Codecov Report

Merging #8264 into hotfix-7.10.x will decrease coverage by <.01%.
The diff coverage is 0%.

@@                Coverage Diff                @@
##           hotfix-7.10.x    #8264      +/-   ##
=================================================
- Coverage          10.59%   10.58%   -0.01%     
=================================================
  Files               3226     3226              
  Lines             240418   240419       +1     
=================================================
- Hits               25461    25460       -1     
- Misses            214957   214959       +2

@thingumajig42
Copy link
Author

mentioning @Dillon-Brown as he had fixed the bug #7407 earlier & @samus-aran, just to let the core team know about the PR.

@thingumajig42
Copy link
Author

@timo12357 has verified that the PR fixes the issue.
@timo12357 comment

@fcorluka
Copy link

This does not fix the custom OUTBOUND/INBOUND email accounts; it sets FROM user name not email name.

@labcerouno
Copy link

Tested. It's correct what @fcorluka says. It's sending with Suite user name as from name. I think the most appropriated would be to show "From name" field from Outgoing SMTP account. Thank you. BR

@fcorluka
Copy link

On GitHub issues #7692, #8177 and #8264 talk about the same thing.
Fix (#8264) that is proposed is only partial. In my case, I have 5 email addresses, and I need all of them to be active (inbound and outbound ).
But when a user tries to reply from one of the email account happens next:

  • from address is correct and says email account address
    
  • from name is wrong and says the name of the person you are trying to reply to (at first it looks like he is replying to himself).
    

I worked around this code, and it is very very very very complicated code.
To get this fully to work, you should change function (#8264) fix chnages first part but other is very important:
protected function fillDataAddressFromPersonal($dataAddresses)
from : modules/Emails/EmailsDataAddressCollector.php

Original code:


    $dataAddresses[$address]['attributes'] = [
                    'from' =>$fromString,
                    'name' => $userAddress['attributes']['name'],
                    'oe' => $userAddress['attributes']['oe'],
                    'reply_to' => $replyString
                ];

My change:

$dataAddresses[$address]['attributes'] = [
                    'from' => "{$userAddress['attributes']['name']} &lt;{$userAddress['attributes']['from']}&gt;",
                    'name' => $userAddress['attributes']['name'],
                    'oe' => $userAddress['attributes']['oe'],
                    'reply_to' => "{$userAddress['attributes']['name']} &lt;{$userAddress['attributes']['reply_to']}&gt;",
                ];

@labcerouno
Copy link

That's awesome, @fcorluka !!! In my first tests, it's working as it should. Thank you very much for your contribution. It's been so many years we use Email module and we never get it to work properly. I hope it will make it to the master branch.

I agree the code is very very complicated. Besides, the lack of a clear roadmap to Suite v8 and the feeling that its release is imminent, makes us think this kind of work over v7 is useless.

We hope v8 code is out soon, so we can test and contribute!
Thank you again. BR

@felixminom
Copy link

Thanks @fcorluka this also worked for me. I think this should go in a separate PR. Hope this is merged to master in the next release.

@SuiteBot
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants