launchpad-reviewers team mailing list archive

[Raphaël Badin <raphael.badin@xxxxxxxxxxxxx>]

Raphaël Badin has proposed merging lp:~rvb/maas/maas-bug-944563 into lp:maas.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #944563 in MaaS: "First and last name are separate fields"
  https://bugs.launchpad.net/maas/+bug/944563
  Bug #946875 in MaaS: "No ability to set user's name when adding a new user"
  https://bugs.launchpad.net/maas/+bug/946875

For more details, see:
https://code.launchpad.net/~rvb/maas/maas-bug-944563/+merge/95876

This branch:
- gets rid of Django's field first_name
- overrides the label of the field last_name so that it now says "Full name"
- fixes the "user add" form to include the last_name field.
-- 
https://code.launchpad.net/~rvb/maas/maas-bug-944563/+merge/95876
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/maas/maas-bug-944563 into lp:maas.

=== modified file 'src/maasserver/forms.py'
--- src/maasserver/forms.py	2012-02-29 19:00:41 +0000
+++ src/maasserver/forms.py	2012-03-05 11:36:18 +0000
@@ -111,9 +111,14 @@
 
 
 class ProfileForm(ModelForm):
+    # We use the field 'last_name' to store the user's full name (and
+    # don't display Django's 'first_name' field).
+    last_name = forms.CharField(
+        label="Full name", max_length=30, required=False)
+
     class Meta:
         model = User
-        fields = ('first_name', 'last_name', 'email')
+        fields = ('last_name', 'email')
 
 
 class NewUserCreationForm(UserCreationForm):
@@ -121,10 +126,21 @@
     is_superuser = forms.BooleanField(
         label="Administrator status", required=False)
 
+    def __init__(self, *args, **kwargs):
+        super(NewUserCreationForm, self).__init__(*args, **kwargs)
+        # Insert 'last_name' field at the right place (right after
+        # the 'username' field).
+        self.fields.insert(
+            1, 'last_name',
+            forms.CharField(label="Full name", max_length=30, required=False))
+
     def save(self, commit=True):
         user = super(NewUserCreationForm, self).save(commit=False)
         if self.cleaned_data.get('is_superuser', False):
             user.is_superuser = True
+        new_last_name = self.cleaned_data.get('last_name', None)
+        if new_last_name is not None:
+            user.last_name = new_last_name
         user.save()
         return user
 
@@ -133,11 +149,13 @@
     # Override the default label.
     is_superuser = forms.BooleanField(
         label="Administrator status", required=False)
+    last_name = forms.CharField(
+        label="Full name", max_length=30, required=False)
 
     class Meta:
         model = User
         fields = (
-            'username', 'first_name', 'last_name', 'email', 'is_superuser')
+            'username', 'last_name', 'email', 'is_superuser')
 
 
 class ConfigForm(Form):

=== modified file 'src/maasserver/templates/maasserver/user_view.html'
--- src/maasserver/templates/maasserver/user_view.html	2012-03-02 03:33:00 +0000
+++ src/maasserver/templates/maasserver/user_view.html	2012-03-05 11:36:18 +0000
@@ -28,7 +28,7 @@
     </li>
     <li class="block size3">
       <h4>Full name</h4>
-      <span>{{ view_user.first_name }} {{ view_user.last_name }}</span>
+      <span>{{ view_user.last_name }}</span>
     </li>
     <li class="block size3">
       <h4>Email address</h4>

=== modified file 'src/maasserver/tests/test_views.py'
--- src/maasserver/tests/test_views.py	2012-03-05 03:30:37 +0000
+++ src/maasserver/tests/test_views.py	2012-03-05 11:36:18 +0000
@@ -72,19 +72,14 @@
         # The preferences page displays a form with the user's personal
         # information.
         user = self.logged_in_user
-        user.first_name = 'Steve'
-        user.last_name = 'Bam'
+        user.last_name = 'Steve Bam'
         user.save()
         response = self.client.get('/account/prefs/')
         doc = fromstring(response.content)
         self.assertSequenceEqual(
-            ['Bam'],
+            ['Steve Bam'],
             [elem.value for elem in
                 doc.cssselect('input#id_profile-last_name')])
-        self.assertSequenceEqual(
-            ['Steve'],
-            [elem.value for elem in
-                doc.cssselect('input#id_profile-first_name')])
 
     def test_prefs_GET_api(self):
         # The preferences page displays the API access tokens.
@@ -112,15 +107,13 @@
             get_prefixed_form_data(
                 'profile',
                 {
-                    'first_name': 'John',
-                    'last_name': 'Doe',
+                    'last_name': 'John Doe',
                     'email': 'jon@xxxxxxxxxxx',
                 }))
 
         self.assertEqual(httplib.FOUND, response.status_code)
         user = User.objects.get(id=self.logged_in_user.id)
-        self.assertEqual('John', user.first_name)
-        self.assertEqual('Doe', user.last_name)
+        self.assertEqual('John Doe', user.last_name)
         self.assertEqual('jon@xxxxxxxxxxx', user.email)
 
     def test_prefs_POST_password(self):
@@ -278,17 +271,44 @@
 class UserManagementTest(AdminLoggedInTestCase):
 
     def test_add_user_POST(self):
+        new_last_name = factory.getRandomString(30)
+        new_password = factory.getRandomString()
+        new_admin_status = factory.getRandomBoolean()
         response = self.client.post(
             reverse('accounts-add'),
             {
                 'username': 'my_user',
-                'password1': 'pw',
-                'password2': 'pw',
+                'last_name': new_last_name,
+                'password1': new_password,
+                'password2': new_password,
+                'is_superuser': new_admin_status,
             })
         self.assertEqual(httplib.FOUND, response.status_code)
         users = list(User.objects.filter(username='my_user'))
         self.assertEqual(1, len(users))
-        self.assertTrue(users[0].check_password('pw'))
+        self.assertEqual(new_last_name, users[0].last_name)
+        self.assertEqual(new_admin_status, users[0].is_superuser)
+        self.assertTrue(users[0].check_password(new_password))
+
+    def test_edit_user_POST(self):
+        user = factory.make_user(username='user')
+        user_id = user.id
+        new_last_name = factory.getRandomString(30)
+        new_admin_status = factory.getRandomBoolean()
+        response = self.client.post(
+            reverse('accounts-edit', args=['user']),
+            {
+                'username': 'new_user',
+                'last_name': new_last_name,
+                'email': 'new_test@xxxxxxxxxxx',
+                'is_superuser': new_admin_status,
+            })
+        self.assertEqual(httplib.FOUND, response.status_code)
+        users = list(User.objects.filter(username='new_user'))
+        self.assertEqual(1, len(users))
+        self.assertEqual(user_id, users[0].id)
+        self.assertEqual(new_last_name, users[0].last_name)
+        self.assertEqual(new_admin_status, users[0].is_superuser)
 
     def test_delete_user_GET(self):
         # The user delete page displays a confirmation page with a form.