Reregistration Flows #215

oliver-zhou · 2017-07-12T10:38:38Z

Changes to address #213

REREGISTRATION_SHOW_RESPONSE

If False (default), the /register/ endpoint will always return
a HTTP_201_CREATED response, as well as the UserSerializer serializer.data response
of one of those users, so that it makes it difficult to distinguish if a User has this email

Default: True

SEND_REREGISTRATION_EMAIL

If True, register endpoint will send warning and reminder email to user.
Only active users are able to reset their passwords (User.is_active is True).
The email will ask the User to reset their password since someone is trying to
re-register their account.
If User.is_active is False, will send a warning

Default: False

Reregistration Email is also different - if the User is_active or not, they will either have the option to reset password or will be reminded to complete activation.

…esponse, UserReregistrationEmailFactory, override RegistrationView.create(), add RegistrationView.send_reregistration_email()

… either is_active or not, also allow for all combinations of settings.REREGISTRATION_SHOW_RESPONSE and settings.SEND_REREGISTRATION_EMAIL

coveralls · 2017-07-12T10:43:36Z

Coverage decreased (-3.7%) to 93.069% when pulling b3c694c on oliver-zhou:reregistration into 1ffeb41 on sunscrapers:master.

coveralls · 2017-07-12T21:08:35Z

Coverage decreased (-3.7%) to 93.069% when pulling 12ef654 on oliver-zhou:reregistration into 1ffeb41 on sunscrapers:master.

pszpetkowski

I'd like to introduce few changes to this PR. Also it would be nice to have some tests, since you are bringing new features.

pszpetkowski · 2017-07-14T08:30:02Z

djoser/conf.py

@@ -15,6 +15,7 @@
    'USE_HTML_EMAIL_TEMPLATES': False,
    'SEND_ACTIVATION_EMAIL': False,
    'SEND_CONFIRMATION_EMAIL': False,
+    'SEND_REREGISTRATION_EMAIL': False,


What do you think about calling it RESEND_REGISTRATION_EMAIL?

Sounds fine to me - will try to keep nouns consistent and modify the verbs if it pops up elsewhere as well.

pszpetkowski · 2017-07-14T08:34:06Z

djoser/conf.py

@@ -38,6 +39,7 @@
    },
    'LOGOUT_ON_PASSWORD_CHANGE': False,
    'USER_EMAIL_FIELD_NAME': 'email',
+    'REREGISTRATION_SHOW_RESPONSE': True,


Documentation states that the default is False however you have provided True as a default.

I had it mixed up in what I wrote for the docs : the Default was set to True, but in-line the description was False. Fixing the docs to match.

pszpetkowski · 2017-07-14T08:36:34Z

djoser/views.py

+        except User.DoesNotExist:
+            pass
+        response = super(RegistrationView, self).create(request, *args, **kwargs)
+        return response


This could be simplified into one line:

return super(RegistrationView, self).create(request, *args, **kwargs)

pszpetkowski · 2017-07-14T08:43:10Z

djoser/views.py

+
+    def create(self, request, *args, **kwargs):
+        try:
+            email_users = self.get_email_users(request.data.get('email'))


The field does not necessarily needs to be called email.

pszpetkowski · 2017-07-14T08:45:36Z

djoser/views.py

@@ -79,6 +95,28 @@ def send_confirmation_email(self, user):
        email = email_factory.create()
        email.send()

+    def send_reregistration_email(self, user):


Again, how about calling it resend_registration_email?

pszpetkowski · 2017-07-14T08:51:48Z

djoser/templates/reregistration_inactive_email_body.html

+
+<p>{% blocktrans %}The {{ site_name }} team{% endblocktrans %}</p>
+</body></html>
+{% endautoescape %}


This template as well as djoser/templates/reregistration_inactive_email_body.txt could be merged with djoser/templates/reregistration_email_body.[html|txt] and you could simply use the if statement controlled by context variable.

pszpetkowski · 2017-07-14T08:54:24Z

djoser/conf.py

@@ -38,6 +39,7 @@
    },
    'LOGOUT_ON_PASSWORD_CHANGE': False,
    'USER_EMAIL_FIELD_NAME': 'email',
+    'REREGISTRATION_SHOW_RESPONSE': True,


Also, to keep it consistent I would rather call it REGISTRATION_SHOW_EMAIL_FOUND.

pszpetkowski · 2017-07-14T09:31:41Z

docs/source/settings.rst

+Only active users are able to reset their passwords (User.is_active is True).
+The email will ask the User to reset their password since someone is trying to
+re-register their account.
+If User.is_active is False, will send a warning


Some of code values are decorated with higlight like ``True``, but it seems that you have forgot about the others.

pszpetkowski · 2017-07-14T09:35:03Z

djoser/views.py

+            if not settings.REREGISTRATION_SHOW_RESPONSE and email_users:
+                return Response(serializer.data, status=status.HTTP_201_CREATED, headers=headers)
+        except User.DoesNotExist:
+            pass


I'm wondering if this try ... except ... block could not be replaced with some smart if statement, since you are not doing anything with the exception.

pszpetkowski · 2017-07-14T09:42:33Z

djoser/templates/reregistration_inactive_email_body.html

+
+<p>{% blocktrans %}The {{ site_name }} team{% endblocktrans %}</p>
+</body></html>
+{% endautoescape %}


Also, wouldn't we prefer to resend activation email in case of such API call? Emails tend to (rarely) get lost e.g. temporary mail server downtime.

It might be a good idea to have a discussion on that @haxoza @KaczuH

… REGISTRATION_SHOW_EMAIL_FOUND, use "get_email_field_name" to properly find the User model's email field, rename SEND_REREGISTRATION_EMAIL to RESEND_REGISTRATION_EMAIL

coveralls · 2017-07-21T21:10:48Z

Coverage decreased (-3.6%) to 93.083% when pulling 9a1e5d7 on oliver-zhou:reregistration into 1ffeb41 on sunscrapers:master.

oliver-zhou · 2018-05-15T06:01:42Z

I have some time to revisit this pull request, and bring this up to speed with Djoser 1.15 - before I spend the investment - what are the plans for the branches in development right now? If I'm working off 1.15, will it be future resistant - or should I wait for 2.0? @piotr-szpetkowski

zefciu · 2019-01-24T14:37:10Z

@oliver-zhou It would be great to pull this into 1.0 branch. The 2.0 needs to be refactored so there is no point in waiting with it.

dekoza · 2019-07-23T18:33:42Z

@oliver-zhou Sorry for such long delay with this PR. Do you feel it is suitable for version 2.0 that was released recently? I'd like to revisit this feature request.

oliver-zhou added 4 commits July 12, 2017 01:19

Add reregistration option : conf.py configurations to control email/r…

7ad0e32

…esponse, UserReregistrationEmailFactory, override RegistrationView.create(), add RegistrationView.send_reregistration_email()

Docs and additional email templates

b058137

Fix typo of extra <p> tag

75f9e00

Create the ability to send different kinds of emails to Users who are…

b3c694c

… either is_active or not, also allow for all combinations of settings.REREGISTRATION_SHOW_RESPONSE and settings.SEND_REREGISTRATION_EMAIL

Use request.data instead of request.POST

12ef654

pszpetkowski suggested changes Jul 14, 2017

View reviewed changes

Address a few requests in PR : Rename REREGISTRATION_SHOW_RESPONSE to…

9a1e5d7

… REGISTRATION_SHOW_EMAIL_FOUND, use "get_email_field_name" to properly find the User model's email field, rename SEND_REREGISTRATION_EMAIL to RESEND_REGISTRATION_EMAIL

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reregistration Flows #215

Reregistration Flows #215

oliver-zhou commented Jul 12, 2017

coveralls commented Jul 12, 2017 •

edited

Loading

coveralls commented Jul 12, 2017 •

edited

Loading

pszpetkowski left a comment

pszpetkowski Jul 14, 2017

oliver-zhou Jul 17, 2017

pszpetkowski Jul 14, 2017

oliver-zhou Jul 19, 2017

pszpetkowski Jul 14, 2017

pszpetkowski Jul 14, 2017

pszpetkowski Jul 14, 2017

pszpetkowski Jul 14, 2017

pszpetkowski Jul 14, 2017

pszpetkowski Jul 14, 2017

pszpetkowski Jul 14, 2017

pszpetkowski Jul 14, 2017

coveralls commented Jul 21, 2017 •

edited

Loading

oliver-zhou commented May 15, 2018 •

edited

Loading

zefciu commented Jan 24, 2019

dekoza commented Jul 23, 2019

Reregistration Flows #215

Are you sure you want to change the base?

Reregistration Flows #215

Conversation

oliver-zhou commented Jul 12, 2017

REREGISTRATION_SHOW_RESPONSE

SEND_REREGISTRATION_EMAIL

coveralls commented Jul 12, 2017 • edited Loading

coveralls commented Jul 12, 2017 • edited Loading

pszpetkowski left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Jul 21, 2017 • edited Loading

oliver-zhou commented May 15, 2018 • edited Loading

zefciu commented Jan 24, 2019

dekoza commented Jul 23, 2019

coveralls commented Jul 12, 2017 •

edited

Loading

coveralls commented Jul 12, 2017 •

edited

Loading

coveralls commented Jul 21, 2017 •

edited

Loading

oliver-zhou commented May 15, 2018 •

edited

Loading