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

from_email should accept None and set DEFAULT_FROM_EMAIL in that case #97

Open
fjsj opened this issue Dec 1, 2016 · 3 comments · May be fixed by #131
Open

from_email should accept None and set DEFAULT_FROM_EMAIL in that case #97

fjsj opened this issue Dec 1, 2016 · 3 comments · May be fixed by #131

Comments

@fjsj
Copy link
Member

fjsj commented Dec 1, 2016

What do you think @aericson ?

@aericson
Copy link
Contributor

aericson commented Dec 3, 2016

I believe the idea here was to mimic as close as possible the arguments of django.core.mail.send_mail

If we decide to change, what would be the correct approach to do this while keeping the backward compatibility and args order?
Leave all other required arguments with None default and manually raise ValueError if they are None?

On a side note: Because of Django's EmailMessage, even though it's not ideal, if you pass None as a from_email it will fall back to use the DEFAULT_FROM_EMAIL. [1]

@fjsj
Copy link
Member Author

fjsj commented Dec 3, 2016

Yes, the idea is to mimic Django behavior. But if we're going to lose backward compatibility for arg order, then don't worry.

We should only have a test to ensure from_email=None falls back to DEFAULT_FROM_EMAIL

@QasimK
Copy link

QasimK commented Mar 28, 2018

Am I understanding this correctly: if settings.TEMPLATED_EMAIL_FROM_EMAIL is not set, then, because of Django, it will fall back to DEFAULT_FROM_EMAIL? From the documentation, I was under the impression that TEMPLATED_EMAIL_FROM_EMAIL was a required setting if you did not specify from_email... It may just be a silly assumption on my part, do you think it's worth clarifying in the docs?

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 a pull request may close this issue.

3 participants