dev/core#4273 - CRM/Mailing - Allow user to configure donotreply email address

[olayiwola-compucorp]

Overview

See - https://lab.civicrm.org/dev/core/-/issues/4273

Before

The user cannot configure the do not reply email address and the default do-not-reply@$emailDomain is used.

After

Users can now configure which of their created From Email Addresses to use as do not reply email address.

[civibot]

No issue was found matching the number given in the pull request title. Please check the issue number.

[civibot]

(Standard links)

• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[colemanw]

I would have expected a setting like this to be a textfield, but you've configured it as a dropdown-select using getAvailableFromEmailAddresses - what's the purpose of constraining the input like that? Seems like it would make configuring this setting more difficult because first you have to add the do-not-reply address on a different screen then come back to this screen and select it.

[olayiwola-compucorp]

Firstly, we aimed to reuse existing from email address. Additionally, since the /admin/options/from_email_address screen already performs email validation, there is no need to revalidate user input on the settings screen.

@colemanw

[jamienovick]

@colemanw thanks for taking a look! Do you have any further thoughts on this or is it good to merge? 🤞

[olayiwola-compucorp]

@colemanw Thanks for the review, the PR has been updated with your suggestions..

kindly check now.

[colemanw]

Hmm, it seems to me that having 'default' => NULL ought to tell the form that the setting is optional and doesn't need to be validated if left blank. Does that make sense to you @eileenmcnaughton ?

[eileenmcnaughton]

@colemanw I would say specifically that the email rule should early return on empty values - the job of vailidating required is different to validating an email

[colemanw]

@eileenmcnaughton ok I've updated it, but I feel like we're going to have to update a lot of rules if we want them all to allow empty strings to pass through unvalidated.