Send emails only from allowed senders and replace From with Sender in Forms #6231
Conversation
SegiNyn
changed the title
|
Is it really necessary to change the APIs internally? At least what I had in mind was simply changing the logic in the emails module to determine how to use From and Reply-to and only changing the form label in the various other places...
|
I saw that you already had a function _rewrite_sender that makes changes to the From address that's why I removed that and added a new function. And I thought it was better to make the changes before creating the EmailMessage. Also, to be sure where in the emails module did you have in mind? |
|
The changes to the names to sender_address instead of from_address isn't actually necessary but it makes it obvious to developers as well not just the users as is the case if only the label is changed. |
| from_address = _get_actual_sender_address(sender_address) | ||
| if sender_address and from_address != sender_address: | ||
| reply_address |= {sender_address} |
There was a problem hiding this comment.
I'm not sure if (even though the RFC allows it) using multiple reply-to addresses is a good idea.
I wonder whether we shouldn't clean up the API of make_email to be less ambiguous. One possibility would be to have only the sender_address arg, and completely get rid of the reply_address options - and then rely on the existing logic to determine whether the address goes in From or Reply-to.
Or alternatively, to make the PR less invasive, keep the API mostly as it is, but simply make an explicitly specified reply_address override the one that may otherwise be taken from the from_address.
I'll think about it and try around a bit on my dev instance...
There was a problem hiding this comment.
I have no strong opinion because in our case we use the reply_address option in make_email since we were converting all from_address to reply_address. But there are some notification emails functions which are explicitly passing the reply_address address to the make_email function. So maybe your second option would be better
No description provided.