Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Let's fix the SignupForm form #43

Closed
wants to merge 13 commits into from

2 participants

@nigma

The current SignupForm.save contains old django-friends invitation handling code and is hard to understand and customize.

I suggest to extract any email confirmation related code to separate method, keep the base form simple and provide legacy DjangoFriendsSignupForm or just get rid of that code (currently it is not triggered because no confirmation_key is passed to the form from the signup view anyway).

I also added KaleoSignupForm to the patch as example how easy it is to provide custom email verification method using the new approach.

I include full commit history in case you want to follow my way of thinking and code refactoring steps.

nigma added some commits
@nigma nigma Pass confirmation key/code param to account.signup form as initial va…
…lue.
35fb300
@nigma nigma Moved common user creation code out of if statements. 926bb41
@nigma nigma Moved old django-friends confirmation code to separate method of Sign…
…upForm.
a13eac9
@nigma nigma Added missing call param. 1d4260e
@nigma nigma Simplifying signup form code. 4f39ed8
@nigma nigma Added a bit of doc. e4a3537
@nigma nigma Returning verification status from SignupForm.handle_confirmation.
Don't set user as inactive when his email address was already verified. Otherwise he won't have a chance to verify email address because he won't receive any confirmation message.
a905734
@nigma nigma Don't tie confirmation key to SignupCode model in case some other ver…
…ification system is used in forms.
1204cfd
@nigma nigma Wrapped user creation in db transaction block.
Otherwise User object without attached EmailAddress may be created if something goes wrong during the process.
25f5182
@nigma nigma Reorganized code and updated docstring. 759b9fa
@nigma nigma Abstracted email confirmation code during signup process.
Added default email confirmation handler to SignupForm and moved legacy django-friends join invitation code to DjangoFriendsSignupForm.
ae30944
@nigma nigma Added KaleoSignupForm as example of handling user to user invitations…
… and marking email address as verified.
3c16051
@nigma nigma Removed unused import. 1dfb035
@brosner
Owner

I need to do a bit more thinking with how I want this to work. I have been thinking a lot lately with how to fix pinax.apps.account. I don't think this is quite what I want. However, I am glad that you have made a start on this and getting me to think about pinax.apps.account differently.

@brosner
Owner

We are changing the direction of account related bits in Pinax. We've held some discussions during PyCon 2012 and have decided it is best to move pinax.apps.account outside of Pinax. We've created django-user-accounts where all future efforts will take place. I do believe it solves the issues you've tried to solve here.

I am very appreciative of the work you've put in here and I hope I can get you directed towards the new effort. I look forward to any new ideas you can bring to the table. Thank you!

@brosner brosner closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Feb 21, 2012
  1. @nigma
  2. @nigma
  3. @nigma
  4. @nigma

    Added missing call param.

    nigma authored
  5. @nigma

    Simplifying signup form code.

    nigma authored
  6. @nigma

    Added a bit of doc.

    nigma authored
  7. @nigma

    Returning verification status from SignupForm.handle_confirmation.

    nigma authored
    Don't set user as inactive when his email address was already verified. Otherwise he won't have a chance to verify email address because he won't receive any confirmation message.
  8. @nigma

    Don't tie confirmation key to SignupCode model in case some other ver…

    nigma authored
    …ification system is used in forms.
  9. @nigma

    Wrapped user creation in db transaction block.

    nigma authored
    Otherwise User object without attached EmailAddress may be created if something goes wrong during the process.
  10. @nigma
  11. @nigma

    Abstracted email confirmation code during signup process.

    nigma authored
    Added default email confirmation handler to SignupForm and moved legacy django-friends join invitation code to DjangoFriendsSignupForm.
  12. @nigma

    Added KaleoSignupForm as example of handling user to user invitations…

    nigma authored
    … and marking email address as verified.
  13. @nigma

    Removed unused import.

    nigma authored
This page is out of date. Refresh to see the latest.
Showing with 131 additions and 51 deletions.
  1. +125 −49 pinax/apps/account/forms.py
  2. +6 −2 pinax/apps/account/views.py
View
174 pinax/apps/account/forms.py
@@ -3,6 +3,7 @@
from django import forms
from django.conf import settings
from django.core.mail import send_mail
+from django.db import transaction
from django.template.loader import render_to_string
from django.utils.translation import ugettext_lazy as _, ugettext
from django.utils.http import int_to_base36
@@ -197,60 +198,41 @@ def save(self, request=None):
# don't assume a username is available. it is a common removal if
# site developer wants to use email authentication.
username = self.cleaned_data.get("username")
- email = self.cleaned_data["email"]
-
- if self.cleaned_data["confirmation_key"]:
- from friends.models import JoinInvitation # @@@ temporary fix for issue 93
- try:
- join_invitation = JoinInvitation.objects.get(confirmation_key=self.cleaned_data["confirmation_key"])
- confirmed = True
- except JoinInvitation.DoesNotExist:
- confirmed = False
- else:
- confirmed = False
-
- # @@@ clean up some of the repetition below -- DRY!
-
- if confirmed:
- if email == join_invitation.contact.email:
- new_user = self.create_user(username)
- join_invitation.accept(new_user) # should go before creation of EmailAddress below
- if request:
- messages.add_message(request, messages.INFO,
- ugettext(u"Your email address has already been verified")
- )
- # already verified so can just create
- EmailAddress(user=new_user, email=email, verified=True, primary=True).save()
- else:
- new_user = self.create_user(username)
- join_invitation.accept(new_user) # should go before creation of EmailAddress below
- if email:
- if request:
- messages.add_message(request, messages.INFO,
- ugettext(u"Confirmation email sent to %(email)s") % {
- "email": email,
- }
- )
- EmailAddress.objects.add_email(new_user, email)
- else:
+
+ with transaction.commit_on_success():
new_user = self.create_user(username)
- if email:
- if request and not EMAIL_VERIFICATION:
- messages.add_message(request, messages.INFO,
- ugettext(u"Confirmation email sent to %(email)s") % {
- "email": email,
- }
- )
- EmailAddress.objects.add_email(new_user, email)
-
- if EMAIL_VERIFICATION:
- new_user.is_active = False
- new_user.save()
+ verified = self.handle_confirmation(new_user, request=request)
+ if EMAIL_VERIFICATION and not verified:
+ new_user.is_active = False
+ new_user.save()
self.after_signup(new_user)
return new_user
-
+
+ def handle_confirmation(self, new_user, request=None):
+ """
+ Creates EmailAddress object for new user and triggers email
+ verification process.
+
+ Override this method to performs any signup/invite code verification
+ (i.e. unique code sent in friend invite email).
+
+ It must return verification status indicating if user account
+ email is verified at this point or not.
+ """
+ email = self.cleaned_data["email"]
+
+ if email:
+ EmailAddress.objects.add_email(new_user, email)
+ messages.info(request,
+ ugettext(u"Confirmation email sent to %(email)s") % {
+ "email": email,
+ }
+ )
+
+ return False # No email verified yet
+
def is_valid(self, *args, **kwargs):
result = super(SignupForm, self).is_valid(*args, **kwargs)
user_sign_up_attempt.send(
@@ -268,6 +250,100 @@ def after_signup(self, user, **kwargs):
user_signed_up.send(sender=SignupForm, user=user)
+class DjangoFriendsSignupForm(SignupForm):
+
+ def handle_confirmation(self, new_user, request=None):
+ """
+ This is the legacy django-friends join invitation handler used to mark
+ email address as verified when:
+ - provided confirmation key matches join invitation key and
+ - provided email address matches join invitation email address
+ """
+ from friends.models import JoinInvitation
+
+ email = self.cleaned_data["email"]
+
+ join_invitation = None
+ if self.cleaned_data["confirmation_key"]:
+ try:
+ join_invitation = JoinInvitation.objects.get(
+ confirmation_key=self.cleaned_data["confirmation_key"])
+ except JoinInvitation.DoesNotExist:
+ pass
+
+ verified = False
+ if join_invitation:
+ join_invitation.accept(new_user) # should go before creation of EmailAddress below
+ if email and email == join_invitation.contact.email:
+ verified = True
+
+ if verified:
+ EmailAddress(user=new_user, email=email, verified=True, primary=True).save()
+ if request:
+ messages.info(request,
+ ugettext(u"Your email address has already been verified"))
+ else:
+ if email:
+ EmailAddress.objects.add_email(new_user, email)
+ # TODO: according to original logic email is sent always, but
+ # the message is displayed conditionally. Why?
+ if request and (join_invitation or not EMAIL_VERIFICATION):
+ messages.info(request,
+ ugettext(u"Confirmation email sent to %(email)s") % {
+ "email": email,
+ }
+ )
+
+ return verified
+
+
+class KaleoSignupForm(SignupForm):
+
+ def handle_confirmation(self, new_user, request=None):
+ """
+ Checks for matching confirmation code generated by Kaleo
+ (https://github.com/eldarion/kaleo) user to user invitation app
+ and marks EmailAddress as verified if possible.
+ """
+ from kaleo.models import JoinInvitation
+
+ email = self.cleaned_data["email"]
+ verified = False
+ join_invitation = None
+
+ if self.cleaned_data["confirmation_key"]:
+ try:
+ join_invitation = JoinInvitation.objects.get(
+ signup_code__code=self.cleaned_data["confirmation_key"])
+ except JoinInvitation.DoesNotExist:
+ pass
+
+ if email:
+ if join_invitation and email == join_invitation.to_user_email():
+ # Signup code and provided email address match. Mark email
+ # address as verified and don't send confirmation email.
+
+ # Emits signup_codes.signals.signup_code_used signal handled
+ # by Kaleo to update invitation status.
+ # Other processing like marking users as friends can be done
+ # by handling kaleo.signals.invite_accepted signal.
+ join_invitation.signup_code.use(new_user)
+
+ EmailAddress(user=new_user, email=email, verified=True,
+ primary=True).save()
+ verified = True
+ else:
+ # Send confirmation email to verify provided email address
+ if request:
+ messages.info(request,
+ ugettext(u"Confirmation email sent to %(email)s") % {
+ "email": email,
+ })
+ EmailAddress.objects.add_email(new_user, email)
+
+ return verified
+
+
class OpenIDSignupForm(SignupForm):
def __init__(self, *args, **kwargs):
View
8 pinax/apps/account/views.py
@@ -156,8 +156,12 @@ def signup(request, **kwargs):
)
return HttpResponseRedirect(success_url)
else:
- form = form_class(group=group)
-
+ confirmation_key = request.GET.get("code")
+ initial = {}
+ if confirmation_key:
+ initial["confirmation_key"] = confirmation_key
+ form = form_class(group=group, initial=initial)
+
ctx.update({
"form": form,
"redirect_field_name": redirect_field_name,
Something went wrong with that request. Please try again.