search

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

Send email on commit #355

Closed adibo closed 5 years ago

adibo commented 5 years ago

I propose to use on_commit for sending out activation emails.

This could be useful when e.g. sending out activation emails are delegated to a celery task. Potential race condition exists, when a user object has yet not been committed to a db while a task broker already attempts to execute email sending. A similar problem is described here: https://dev.to/k4ml/django-fixing-race-condition-when-queuing-with-oncommit-hook-7ae

Since django-registration relies on Django 1.11+, we could safely use on_commit to avoid such situations, as sugested by the post.

The only caveat is that (some) tests would need to use TransactionTestCase. All the necessary changes are proposed in this PR.

coveralls commented 5 years ago

Coverage Status

Coverage remained the same at 97.302% when pulling 73dbc052a045012dedb9667592ba76b64a5bc9a5 on adibo:send-email-on-commit into 3ae06c7f24a3f166c59ddbcd20d952227a57e031 on macropin:master.

joshblum commented 5 years ago

thanks for the PR! this change looks good, will accept once the testing issue is resolved