feat(accounts): Added prefilling of forms with GET query strings #3410

varunsaral · 2023-09-03T06:27:14Z

This pr adds email prefilling suport in signup form, closes #2861

allauth/account/views.py

pennersr · 2023-09-03T06:48:13Z

allauth/account/views.py

@@ -290,6 +290,13 @@ def get_context_data(self, **kwargs):
        )
        return ret

+    def get_initial(self):
+        if self.request.GET.get("email", None):


Suggested change

if self.request.GET.get("email", None):

if "email" in self.request.GET:

allauth/account/views.py

pennersr · 2023-09-03T08:18:30Z

allauth/account/views.py

+            initial = super().get_initial()
+            initial["email"] = self.request.GET.get("email")
+            return initial
+        return super().get_initial()


Would also be nice to make this a bit more DRY. You're invoking super().get_initial twice. How about:

initial = super().get_initial(): if ...: prefill return initial

noted, will do

pennersr · 2023-09-03T10:03:49Z

allauth/account/views.py

+        twice_email = app_settings.SIGNUP_EMAIL_ENTER_TWICE
+        request_get = self.request.GET
+
+        def validate_and_set_email(email_key):


This looks a little bit complex to me, e.g. there is no need to validate the email twice. How about simplifying into:

from django.core.validators import validate_email def get_initial(self): intial = super().get_initial() email = self.request.GET("email") if email: try: email = validate_email(email) except ValidationError: pass else: intial["email"] = email if app_settings.SIGNUP_EMAIL_ENTER_TWICE: intial["email2"] = email return intial

varunsaral · 2023-09-03T10:59:56Z

I think this should be fine @pennersr, don't know why black,isort are failing i applied them before pushing

fdobrovolny

Not sure why your black and isort, are not working. But the following changes seem like the reason the tests are failing.

fdobrovolny · 2023-09-06T20:41:22Z

allauth/account/views.py

+from django.core.validators import EmailValidator
+from django.forms import ValidationError


The isort is failing on these two lines if you take a look at the logs:

--- /home/runner/work/django-allauth/django-allauth/allauth/account/views.py:before 2023-09-03 10:56:32.162313 +++ /home/runner/work/django-allauth/django-allauth/allauth/account/views.py:after 2023-09-03 10:56:51.994464 @@ -2,6 +2,8 @@ from django.contrib.auth import REDIRECT_FIELD_NAME from django.contrib.auth.decorators import login_required from django.contrib.sites.shortcuts import get_current_site +from django.core.validators import EmailValidator +from django.forms import ValidationError from django.http import ( Http404, HttpResponsePermanentRedirect, @@ -46,8 +48,7 @@ sync_user_email_addresses, url_str_to_user_pk, ) -from django.core.validators import EmailValidator -from django.forms import ValidationError +

There should be two lines between the first code line and imports. You removed one.

The imports should be moved to be sorted alphabetically, as can be seen in the patch by isort.

Thanks @fdobrovolny for the suggestion, turns out the isort and black installation were messed up.

fdobrovolny · 2023-09-06T20:44:26Z

allauth/account/views.py

@@ -290,6 +291,19 @@ def get_context_data(self, **kwargs):
        )
        return ret

+    def get_initial(self):
+        intial = super(SignupView,self).get_initial()


Suggested change

intial = super(SignupView,self).get_initial()

intial = super(SignupView, self).get_initial()

To make black happy you need a space here.

coveralls · 2023-09-12T12:50:16Z

coverage: 91.483% (-0.007%) from 91.49% when pulling 8fac170 on varunsaral:prefilled_forms into ba92ef3 on pennersr:main.

varunsaral · 2023-09-15T19:58:03Z

@pennersr this can be closed. Everything changed according to the suggestions

varunsaral · 2023-09-15T19:59:02Z

Along with #3003

pennersr · 2023-09-17T12:00:09Z

allauth/account/tests/test_signup.py

@@ -315,3 +315,17 @@ def test_django_password_validation(self):
                "password1",
                ["This password is too short. It must contain at least 9 characters."],
            )
+
+    def test_get_initial_with_valid_email(self):


The codebase is transitioning towards using pytest based tests. Can you turn this into a regular (pytest) function? As in, not make it a mapper of a class, let it accept rf (request factory) as a parameter, and so on...

pennersr · 2023-09-17T12:01:21Z

allauth/account/views.py

@@ -290,6 +292,19 @@ def get_context_data(self, **kwargs):
        )
        return ret

+    def get_initial(self):
+        intial = super(SignupView, self).get_initial()


Typ-o: in-i-tial

I think the spelling is correct, it's in-i-tial

But it says intial = ...

pennersr · 2023-09-17T12:01:56Z

allauth/account/views.py

+        email = self.request.GET.get("email")
+        if email:
+            try:
+                EmailValidator()(email)


There is no need to spawn a validator yourself, just call this method:

https://docs.djangoproject.com/en/4.2/ref/validators/#validate-email

pennersr · 2023-09-17T12:03:42Z

allauth/account/views.py

@@ -290,6 +292,19 @@ def get_context_data(self, **kwargs):
        )
        return ret

+    def get_initial(self):
+        intial = super(SignupView, self).get_initial()


Just use super() -- no need to be Python 2 compliant anymore.

varunsaral · 2023-09-23T08:07:32Z

@pennersr Mistakenly rebased wrong and now whole branch got deleted 😭 , sorry about the trouble. I have redone all the changes and raised the pr #3447. I have checked it throughly and changed everything with your review suggestions.

pennersr requested changes Sep 3, 2023

View reviewed changes

pennersr reviewed Sep 3, 2023

View reviewed changes

fdobrovolny reviewed Sep 6, 2023

View reviewed changes

varunsaral force-pushed the prefilled_forms branch from 8efe858 to cfcf9f1 Compare September 12, 2023 11:53

pennersr requested changes Sep 17, 2023

View reviewed changes

pennersr reviewed Sep 17, 2023

View reviewed changes

pennersr mentioned this pull request Sep 17, 2023

feat(accounts): Added prefilling of forms with GET query strings #3003

Closed

4 tasks

varunsaral closed this Sep 23, 2023

varunsaral force-pushed the prefilled_forms branch from 6d6f773 to aaa0b67 Compare September 23, 2023 07:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(accounts): Added prefilling of forms with GET query strings #3410

feat(accounts): Added prefilling of forms with GET query strings #3410

varunsaral commented Sep 3, 2023

pennersr Sep 3, 2023

pennersr Sep 3, 2023

varunsaral Sep 3, 2023 •

edited

Loading

pennersr Sep 3, 2023

varunsaral commented Sep 3, 2023

fdobrovolny left a comment

fdobrovolny Sep 6, 2023

varunsaral Sep 15, 2023

fdobrovolny Sep 6, 2023

coveralls commented Sep 12, 2023

varunsaral commented Sep 15, 2023

varunsaral commented Sep 15, 2023

pennersr Sep 17, 2023

pennersr Sep 17, 2023

varunsaral Sep 23, 2023

pennersr Sep 23, 2023

pennersr Sep 17, 2023

pennersr Sep 17, 2023

varunsaral commented Sep 23, 2023

	if self.request.GET.get("email", None):
	if "email" in self.request.GET:

		from django.core.validators import EmailValidator
		from django.forms import ValidationError

	intial = super(SignupView,self).get_initial()
	intial = super(SignupView, self).get_initial()

feat(accounts): Added prefilling of forms with GET query strings #3410

feat(accounts): Added prefilling of forms with GET query strings #3410

Conversation

varunsaral commented Sep 3, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

varunsaral Sep 3, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

varunsaral commented Sep 3, 2023

fdobrovolny 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

coveralls commented Sep 12, 2023

varunsaral commented Sep 15, 2023

varunsaral commented Sep 15, 2023

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

varunsaral commented Sep 23, 2023

varunsaral Sep 3, 2023 •

edited

Loading