[snowy: 22/26] Create user account after user has given details instead of before

[Sanford Armstrong <sharm src gnome org>]

commit 5e69e963fd7511df1faeb3e0427246262bb34bd5
Author: Leon Handreke <leon handreke gmail com>
Date:   Sat May 29 11:55:35 2010 +0200

    Create user account after user has given details instead of before
    
    Redesign registration page
    Share the same form for OpenID and normal registration
    [django_openid_auth] login_complete does not create user account anymore
    [django_openid_auth] Add argument to authenticate method to explicitly create user account

 accounts/forms.py                                  |   52 ++++++---------
 .../templates/accounts/initial_preferences.html    |   35 ----------
 .../templates/registration/registration_form.html  |   69 +++++++++++++++-----
 accounts/urls.py                                   |    7 +-
 accounts/views.py                                  |   61 +++++++++++------
 lib/django_openid_auth/auth.py                     |   38 +++++------
 lib/django_openid_auth/views.py                    |   11 ++--
 lib/recaptcha_django/__init__.py                   |    5 +-
 settings.py                                        |    3 +-
 site_media/css/accounts.css                        |   12 ++--
 10 files changed, 151 insertions(+), 142 deletions(-)
---
diff --git a/accounts/forms.py b/accounts/forms.py
index 27cf82e..1f448a7 100644
--- a/accounts/forms.py
+++ b/accounts/forms.py
@@ -22,46 +22,42 @@ from recaptcha_django import ReCaptchaField
 from django.conf import settings
 from django import forms
 
-def validate_username_blacklist(username):
+class RegistrationFormUniqueUser(RegistrationFormUniqueEmail):
     """
-    Verifies that the username is not on the blacklist of reserved usernames
+    Subclass of ``RegistrationFormUniqueEmail`` which verifies usernames
+    against a blacklist.
     """
-    print "Validate"
+    captcha = ReCaptchaField(label=_(u'Verify words'))
+
     username_blacklist = ['about', 'accounts', 'admin', 'api', 'blog',
                           'contact', 'css', 'friends', 'images', 'index.html',
                           'news', 'notes', 'oauth', 'pony', 'register',
                           'registration', 'site_media', 'snowy', 'tomboy']
-    if username in username_blacklist:
-        raise forms.ValidationError(_(u'This username has been reserved.  Please choose another.'))
-
-class RegistrationFormUniqueUser(RegistrationFormUniqueEmail):
-    """
-    Subclass of ``RegistrationFormUniqueEmail`` which verifies usernames
-    against a blacklist and adds a captcha.
-    """
-    captcha = ReCaptchaField(label=_(u'Verify words:'))
 
     def __init__(self, *args, **kwargs):
         super(RegistrationFormUniqueUser, self).__init__(*args, **kwargs)
 
+        #print self.fields['captcha'].widget.html
         if not settings.RECAPTCHA_ENABLED:
             del self.fields['captcha']
         
-        self.fields['username'].label = _(u'Username:')
+        self.fields['username'].label = _(u'Username')
         self.fields['username'].help_text = _(u'Maximum of 30 characters in length.')
-        self.fields['username'].validators = [validate_username_blacklist,
-                                              validate_username_available]
 
-        self.fields['email'].label = _(u'Email address:')
+        self.fields['email'].label = _(u'Email address')
 
-        self.fields['password1'].label = _(u'Choose a password:')
+        self.fields['password1'].label = _(u'Choose a password')
         self.fields['password1'].help_text = _(u'Minimum of 6 characters in length.')
 
-        self.fields['password2'].label = _(u'Re-enter password:')
+        self.fields['password2'].label = _(u'Re-enter password')
 
     def clean_username(self):
+        """
+        Validate that the user doesn't exist in our blacklist.
+        """
         username = self.cleaned_data['username']
-        validate_username_blacklist(username)
+        if username in self.username_blacklist:
+            raise forms.ValidationError(_(u'This username has been reserved.  Please choose another.'))
         return username
 
     def clean_password1(self):
@@ -74,6 +70,12 @@ class RegistrationFormUniqueUser(RegistrationFormUniqueEmail):
 
         return password
 
+class OpenIDRegistrationFormUniqueUser(RegistrationFormUniqueUser):
+    def __init__(self, *args, **kwargs):
+        super(OpenIDRegistrationFormUniqueUser, self).__init__(*args, **kwargs)
+        del self.fields['password1']
+        del self.fields['password2']
+
 from snowy.accounts.models import UserProfile
 
 class InternationalizationForm(forms.ModelForm):
@@ -94,15 +96,3 @@ class EmailChangeForm(forms.ModelForm):
     def __init__(self, *args, **kwargs):
         super(EmailChangeForm, self).__init__(*args, **kwargs)
         self.fields['email'].required = True
-
-class UsernameChangeForm(forms.ModelForm):
-    class Meta:
-        model = User
-        fields = ('username', )
-
-    def clean_username(self):
-        username = self.cleaned_data['username']
-        validate_username_blacklist(username)
-        return username
-
-
diff --git a/accounts/templates/registration/registration_form.html b/accounts/templates/registration/registration_form.html
index c38912d..8642f67 100644
--- a/accounts/templates/registration/registration_form.html
+++ b/accounts/templates/registration/registration_form.html
@@ -1,24 +1,61 @@
-{% extends 'site_base.html' %}
+{% extends "site_base.html" %}
 
 {% load i18n %}
 
 {% block title %}{% trans "Create an account" %} | {{ block.super }}{% endblock %}
+{% block extra_head %}
+{{ block.super }}
+<link rel="stylesheet" type="text/css" href="{{ MEDIA_URL }}css/accounts.css">
+{% endblock %}
 
 {% block content %}
-<h1>{% trans "Create an account" %}</h1>
-<form method='POST'>
-    <table class="input-form">
-        <tbody>
-            {{ form.as_table }}
-        </tbody>
-        <tfoot>
-            <tr>
-                <th></th>
-                <td>
-                    <input method="submit" type="submit" value="{% trans "Create Account" %}"/>
-                </td>
-            </tr>
-        </tfoot>
-    </table>
+<h1>Tell {{ site.name }} about yourself!</h1>
+
+<form method="POST">
+    <p>
+      <label for="username" class="registration-form-label">{{ form.username.label }}:</label>
+      {{ form.username }}
+      <span class="registration-form-errors">{{ form.username.errors.0 }}</span>
+      <br /><span class="registration-form-help-text">{{ form.username.help_text }}<span>
+    </p>
+
+    <p>
+      <label for="email" class="registration-form-label">{{ form.email.label }}:</label>
+      {{ form.email }}
+      <span class="registration-form-errors">{{ form.email.errors.0 }}</span>
+      <br /><span class="registration-form-help-text">{{ form.email.help_text }}<span>
+    </p>
+
+    {% if form.password1 and form.password2 %}
+    <p>
+      <label for="password1" class="registration-form-label">{{ form.password1.label }}:</label>
+      {{ form.password1 }}
+      <span class="registration-form-errors">{{ form.password1.errors.0 }}</span>
+      <br /><span class="registration-form-help-text">{{ form.password1.help_text }}<span>
+    </p>
+    <p>
+      <label for="password2" class="registration-form-label">{{ form.password2.label }}:</label>
+      {{ form.password2 }}
+      <span class="registration-form-errors">{{ form.password2.errors.0 }}</span>
+      <br /><span class="registration-form-help-text">{{ form.password2.help_text }}<span>
+    </p>
+    {% endif %}
+
+    {% if form.display_name %}
+    <p>
+      <label for="display_name" class="registration-form-label">{{ form.display_name.label }}:</label>
+      {{ form.display_name }}
+      <span class="registration-form-errors">{{ form.display_name.errors.0 }}</span>
+      <br /><span class="registration-form-help-text">{{ form.display_name.help_text }}<span>
+    </p>
+    {% endif %}
+
+    {% if form.captcha %}
+    <p>
+      {{ form.captcha }}
+    </p>
+    {% endif %}
+
+    <input type="submit" value="{% trans "Create Account" %}"/>
 </form>
 {% endblock %}
diff --git a/accounts/urls.py b/accounts/urls.py
index f2f1c87..300f899 100644
--- a/accounts/urls.py
+++ b/accounts/urls.py
@@ -24,15 +24,14 @@ from django.conf.urls.defaults import *
 from registration.views import activate
 from registration.views import register
 
+from snowy.accounts.views import openid_registration
+
 import django_openid_auth.views
 
 urlpatterns = patterns('',
     url(r'^preferences/$', 'snowy.accounts.views.accounts_preferences',
         name='preferences'),
 
-    url(r'^initial_preferences/$', 'snowy.accounts.views.initial_preferences',
-        name='initial_preferences'),
-
     url(r'^logout/$', auth_views.logout, {'template_name': 'registration/logout.html'},
         name='auth_logout'),
 
@@ -40,6 +39,8 @@ urlpatterns = patterns('',
     url(r'^openid/login/$', django_openid_auth.views.login_begin,
         {'template_name': 'openid/login.html'}, name='openid_login'),
     url(r'^openid/complete/$', django_openid_auth.views.login_complete),
+    url(r'^openid/registration/$', openid_registration,
+        name='openid_registration'),
 
     # Registration URLs
     url(r'^activate/(?P<activation_key>\w+)/$', activate, name='registration_activate'),
diff --git a/accounts/views.py b/accounts/views.py
index f1a66d0..bd88e89 100644
--- a/accounts/views.py
+++ b/accounts/views.py
@@ -15,39 +15,58 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #
 
+from django.utils.translation import ugettext_lazy as _
+
 from django.contrib.auth.forms import UserChangeForm, PasswordChangeForm
+from django.contrib.auth.models import User
+from django.contrib.auth import authenticate, login
 from django.contrib.auth.decorators import login_required
+
 from django.shortcuts import render_to_response
-from django.http import HttpResponseRedirect
+from django.http import HttpResponseRedirect, HttpResponseNotAllowed
 from django.template import RequestContext
 from django.conf import settings
 
-from snowy.accounts.forms import InternationalizationForm, EmailChangeForm, \
-    DisplayNameChangeForm, UsernameChangeForm
+from snowy.accounts.models import UserProfile
+from snowy.accounts.forms import InternationalizationForm, OpenIDRegistrationFormUniqueUser
 
- login_required
-def initial_preferences(request, template_name='accounts/initial_preferences.html'):
-    user = request.user
-    profile = user.get_profile()
+def openid_registration(request, template_name='registration/registration_form.html'):
+    try:
+        openid_response = request.session['openid_response']
+        # do sreg magic here
+    except KeyError:
+        return HttpResponseNotAllowed(_(u'No openid_response object for this session!'))
 
-    username_form = UsernameChangeForm(request.POST or None, instance=user)
-    email_form = EmailChangeForm(request.POST or None, instance=user)
-    display_name_form = DisplayNameChangeForm(request.POST or None, instance=profile)
+    registration_form = OpenIDRegistrationFormUniqueUser(request.POST or None)
 
-    forms = [username_form, email_form, display_name_form]
-    for form in forms:
-        if form.is_valid():
-            form.save()
+    if registration_form.is_valid():
+        user = authenticate(openid_response=openid_response,
+                            username=registration_form.cleaned_data.get('username', ''),
+                            create_user=True)
+        # Clear the openid_response from the session so it can't be used to create another account
+        del request.session['openid_response']
 
-    # redirect if all forms are valid
-    if all(form.is_valid() for form in forms):
-        return HttpResponseRedirect(settings.LOGIN_REDIRECT_URL)
+        if user is not None:
+            email = registration_form.cleaned_data.get('email', '')
+            if email:
+                user.email = email
+
+            display_name = registration_form.cleaned_data.get('display_name', '')
+            if display_name:
+                user.get_profile().display_name = display_name
+
+            user.save()
+            user.get_profile().save()
+            if user.is_active:
+                login(request, user)
+                return HttpResponseRedirect(settings.LOGIN_REDIRECT_URL)
+            else:
+                return HttpResponseNotAllowed(_(u'Disabled account'))
+        else:
+            return HttpResponseNotAllowed(_(u'Unknown user'))
 
     return render_to_response(template_name,
-                              {'user': user,
-                               'username_form' : username_form,
-                               'email_form' : email_form,
-                               'display_name_form' : display_name_form},
+                              {'form' : registration_form},
                               context_instance=RequestContext(request))
 
 @login_required
diff --git a/lib/django_openid_auth/auth.py b/lib/django_openid_auth/auth.py
index f277d8e..1670e02 100644
--- a/lib/django_openid_auth/auth.py
+++ b/lib/django_openid_auth/auth.py
@@ -71,8 +71,17 @@ class OpenIDBackend:
             user_openid = UserOpenID.objects.get(
                 claimed_id__exact=openid_response.identity_url)
         except UserOpenID.DoesNotExist:
-            if getattr(settings, 'OPENID_CREATE_USERS', False):
-                user = self.create_user_from_openid(openid_response)
+            if kwargs.get('create_user'):
+                if kwargs.get('username'):
+                    user = self.create_user_from_openid(openid_response,
+                                                        username=kwargs.get('username'))
+                else:
+                    return None
+            else:
+                if getattr(settings, 'OPENID_CREATE_USERS', False):
+                    user = self.create_user_from_openid(openid_response)
+                else:
+                    return None
         else:
             user = user_openid.user
 
@@ -92,38 +101,25 @@ class OpenIDBackend:
 
         return user
 
-    def create_user_from_openid(self, openid_response):
+    def create_user_from_openid(self, openid_response, username='snowyuser', email=''):
         sreg_response = sreg.SRegResponse.fromSuccessResponse(openid_response)
-        # Don't set username from sreg - django doesn't like some characters
-        # Related bug: https://bugs.launchpad.net/django-openid-auth/+bug/388890
-        """if sreg_response:
-            nickname = sreg_response.get('nickname', 'openiduser')
-            email = sreg_response.get('email', '')
-        else:
-            nickname = 'openiduser'
-            email = ''
-            """
-        nickname = 'openiduser'
-        email = ''
-
         # Pick a username for the user based on their nickname,
         # checking for conflicts.
         i = 1
         while True:
-            username = nickname
+            # use the name nickname here so we don't interfere with the username argument
+            nickname = username
             if i > 1:
-                username += str(i)
+                nickname += str(i)
             try:
-                User.objects.get(username__exact=username)
+                User.objects.get(username__exact=nickname)
             except User.DoesNotExist:
                 break
             i += 1
 
-        user = User.objects.create_user(username, email, password=None)
-        print user
+        user = User.objects.create_user(nickname, email, password=None)
         user.get_profile().openid_user = True
         user.get_profile().save()
-        print str(user.get_profile().openid_user)
 
         if sreg_response:
             self.update_user_details_from_sreg(user, sreg_response)
diff --git a/lib/django_openid_auth/views.py b/lib/django_openid_auth/views.py
index 1144adc..cc22aed 100644
--- a/lib/django_openid_auth/views.py
+++ b/lib/django_openid_auth/views.py
@@ -207,15 +207,14 @@ def login_complete(request, redirect_field_name=REDIRECT_FIELD_NAME):
         if user is not None:
             if user.is_active:
                 auth_login(request, user)
-                # Check if the user has filled in relevant credentials
-                if (user.get_profile().registration_complete()):
-                    return HttpResponseRedirect(sanitise_redirect_url(redirect_to))
-                else:
-                    return HttpResponseRedirect(reverse('initial_preferences'))
+                return HttpResponseRedirect(sanitise_redirect_url(redirect_to))
             else:
                 return render_failure(request, 'Disabled account')
         else:
-            return render_failure(request, 'Unknown user')
+            # save openid reponse in the session to create the user later
+            request.session['openid_response'] = openid_response
+            return HttpResponseRedirect(reverse('openid_registration'))
+            #return render_failure(request, 'Unknown user')
     elif openid_response.status == FAILURE:
         return render_failure(
             request, 'OpenID authentication failed: %s' %
diff --git a/lib/recaptcha_django/__init__.py b/lib/recaptcha_django/__init__.py
index 57591e6..c909037 100644
--- a/lib/recaptcha_django/__init__.py
+++ b/lib/recaptcha_django/__init__.py
@@ -11,6 +11,7 @@ from django.conf import settings
 from django.utils.translation import get_language
 from django.utils.html import conditional_escape
 from recaptcha.client import captcha
+from django.utils.safestring import mark_safe
 
 class ReCaptchaWidget(Widget):
     """
@@ -24,13 +25,13 @@ class ReCaptchaWidget(Widget):
         html = captcha.displayhtml(settings.RECAPTCHA_PUBLIC_KEY, error=error)
         options = u',\n'.join([u'%s: "%s"' % (k, conditional_escape(v)) \
                    for k, v in final_attrs.items() if k in self.options])
-        return """<script type="text/javascript">
+        return mark_safe("""<script type="text/javascript">
         var RecaptchaOptions = {
             %s
         };
         </script>
         %s
-        """ % (options, html)
+        """ % (options, html))
 
 
     def value_from_datadict(self, data, files, name):
diff --git a/settings.py b/settings.py
index 3c6801e..2c407a3 100644
--- a/settings.py
+++ b/settings.py
@@ -144,7 +144,8 @@ ACCOUNT_ACTIVATION_DAYS = 15
 
 AUTH_PROFILE_MODULE = 'accounts.UserProfile'
 
-OPENID_CREATE_USERS = True
+# we create users ourselves after they have given more details
+OPENID_CREATE_USERS = False
 
 LOGIN_REDIRECT_URL = '/'
 LOGIN_URL = '/accounts/openid/login/'
diff --git a/site_media/css/accounts.css b/site_media/css/accounts.css
index dc8786f..3d41d10 100644
--- a/site_media/css/accounts.css
+++ b/site_media/css/accounts.css
@@ -20,22 +20,22 @@
     margin-left: 10px;
 }
 
-.initial-preferences-label {
-    width: 10em;
+.registration-form-label {
+    width: 15em;
     font-weight: bold;
     text-align: right;
     display: inline-block;
     margin-right: 0.5em;
 }
 
-.initial-preferences-errors {
+.registration-form-errors {
     color: red;
     margin-left: 2em;
 }
 
-.initial-preferences-help-text {
+.registration-form-help-text {
     /* set the help text off from the input field above it */
     line-height: 2;
     /* make the help text align with the input field above it */
-    margin-left: 11em;
-}
\ No newline at end of file
+    margin-left: 16em;
+}