macropin / django-registration

Django-registration (redux) provides user registration functionality for Django websites.
http://django-registration-redux.readthedocs.org
Other
975 stars 350 forks source link

not being able to define per site email address #26

Closed sircco closed 6 years ago

sircco commented 9 years ago

Since it is already possible to use site for email notifications it would be great if it could be possible to override settings.DEFAULT_FROM_ADDRESS from function

macropin commented 9 years ago

Somewhat related to #54

joshblum commented 8 years ago

@sircco does the fix in https://github.com/macropin/django-registration/pull/54 solve your problem?

sircco commented 8 years ago

looking at code, i don't think that this does it unless i can override this line on per case basis. from_email = getattr(settings, 'REGISTRATION_DEFAULT_FROM_EMAIL', settings.DEFAULT_FROM_EMAIL)

Our app is whitelabel, meaning that more than one domain can use it, register its own customers (and send registration emails)

dicato commented 8 years ago

@sircco the use case makes sense 👍

If you'd like to implement we are happy to review.

joshblum commented 8 years ago

strawman could be to allow REGISTRATION_DEFAULT_FROM_EMAIL to be a python dotted path.to.function which would point a function that take site as an argument and does custom logic to return the email? Thoughts?

# settings.py
REGISTRATION_DEFAULT_FROM_EMAIL = 'path.to.function'

# path/to.py
def function(site):
   return 'example@example.com'
joshblum commented 8 years ago

(probably cleaner to have a separate settings variable REGISTRATION_FROM_EMAIL_FUNCTION or similar but same idea)

sircco commented 8 years ago

How about something like this, overriding default if variable not provided during instantiation. I didnt look too much at the code recently but here is some dummy code to explain my idea.

class Registration(Object): def init_(self, mail_from=None, .... ): self.mail_from, = mail_from or settings.REGISTRATION_DEFAULT_FROM_EMAIL

On Tue, Jun 7, 2016 at 6:14 AM, Joshua Blum notifications@github.com wrote:

(probably cleaner to have a separate settings variable REGISTRATION_FROM_EMAIL_FUNCTION but same idea)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/macropin/django-registration/issues/26#issuecomment-224170248, or mute the thread https://github.com/notifications/unsubscribe/AJ9osiYzs0R4cFXyRZeY07tAUzef4qE-ks5qJPASgaJpZM4Da72R .

dicato commented 8 years ago

I think it's acceptable to use static configuration for this as well. You can use a dictionary to map emails to sites, just as an example.

joshblum commented 8 years ago

@sircco would a static configuration meet your use case?

sircco commented 8 years ago

It will work for now. If we get so much traffic on our whitelabel site that we can't manage static setting we will think of something and submit code for review.

joshblum commented 8 years ago

Great. Do you want to take a first stab at implementing this? El El mié, jun 15, 2016 a las 01:14, Damir notifications@github.com escribió:

It will work for now. If we get so much traffic on our whitelabel site that we can't manage static setting we will think of something and submit code for review.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/macropin/django-registration/issues/26#issuecomment-226089853, or mute the thread https://github.com/notifications/unsubscribe/ABF01NyzOHc4NZQXqii166U7mMXFzlHSks5qL4pFgaJpZM4Da72R .

joshblum commented 6 years ago

@sircco take a look at #294 and let me know if this address your use case!

sircco commented 6 years ago

should work fine!

joshblum commented 6 years ago

Awesome! I'll leave this open until #294 is merged in

joshblum commented 6 years ago

closed by #294