Email headers from form not respected

[mzagmajster]

Q	A
Bug fix? (use the a.b branch)	✔️
New feature/enhancement? (use the a.x branch)	❌
Deprecations?	❌
BC breaks? (use the c.x branch)	❌
Automated tests included?	❌
Related user documentation PR URL	mautic/user-documentation#...
Related developer documentation PR URL	mautic/developer-documentation-new#...
Issue(s) addressed	Fixes #...

Description

This PR makes sure headers specified in email forms to send direct email or segment email (see images below) are respected. Until this bugfix values were properly stored in the database, but they were not used when messages were sent.

I believe this addresses two issues: #14080 and #14483

---

📋 Steps to test this PR:

1. Open this PR on Gitpod or pull down for testing locally (see docs on testing PRs here)

Prepare Mautic instance:

1. Create a contact
2. Create a segment
3. Add contact to a segment
4. Specify form, from name, reply to values in global email configuration with some values A, B, C

Test sending with global config:

6. Try to send a direct email to a contact without specifying any values for the from, from name and reply to values
7. When you receive a message from, from name and reply to must be set the same as in your global configuration

Test sending direct message with form config:

8. Try to send another message but this time specify from, from name, reply to values on the form
9. When you receive a message message should have headers set the way you set them in form

Test sending segment message:

10. Create a segment email
11. Repeat the steps 6,7
12. When you receive a message headers should have default (global) config. values
13. Repeat the steps 8,9
14. When you receive a message headers should have values you have set on the form under advanced tab.

[mzagmajster]

@escopecz or any other Mautic member, I would appreciate some guidance on fixing this failing checks, I do not think I am approaching this the right way. It broke a lot of tests for simple change. Traceback is not accurate when I go look into source code when trying to debug and fix the checks.

Thanks in advance.

[escopecz]

I think that what you are trying to do here is already handled in https://github.com/mautic/mautic/blob/6.x/app/bundles/EmailBundle/Helper/FromEmailHelper.php, isn't it? If not, it should be implemented there.

[escopecz]

Why the email address is allowed to be null? I don't understand this.

[mzagmajster]

This was just a desperate attempt to make the test pass. I guess I will have to run tests on the machine and not commit until they actually pass, otherwise we will have way to many commits here. The message from the test I think suggested that email was null, which was also puzzling since I imagined there is no way it could be null the way I structured the solution.

[escopecz]

I guess you meant

Suggested change

	$this->setSubject($subject) ?? '';
	$this->setSubject($subject ?? '');

But even that wouldn't be OK to do just to make the unit tests pass. I'd make sure that the subject is set in the Email entity in the test and if the Email entity is mocked then the getSubject method must be configured on the mock to return a string. I'm not sure why the tests fail this way though. I don't see any change in subject here.

[mzagmajster]

@escopecz

thanks for the pointers. I can see this file: FromEmailHelper.php, is trying to do some things, that are handled by the PR, but I am not sure its all of them.

The second thing is that when you go and you test this in GUI it does not work, thats why I introduced the changes. But you make a good point, that putting the logic for defaults in the helper would make more sense. Will try to move this logic to helper class and make the test pass.

[escopecz]

Yep, the FROM setting is a mine field to get it to work for all use cases. I recently fixed one issue with the API email transports that was failing a test in our Sparkpost plugin: #14661. We need really robust functional tests to cover them all. Because one bug fix may break other workflows and create other bugs.