Allow user registration without short name (#23)

Following discussions at DjangoCon US 2017, this commit will make it so
that both full_name and short_name have blank=True, making it so that
Users can (by default) register to sites using only emailand password.

The rationale is that making the field required in a custom form is
easier than removing the requirement (blank=False) from the model.

This commit also modifies the test runner to check that the migration
file(s) is(are) in sync with the user model.

runtests.py

@@ -4,6 +4,7 @@
 from os.path import dirname, join
 
 from django import VERSION as DjangoVersion, setup
+from django.apps import apps
 from django.conf import settings
 from django.core.management import execute_from_command_line
 
@@ -18,10 +19,8 @@
     exit(-1)
 
 
-def run_test_suite(*args):
-    """Heart of script: setup Django, run tests based on args"""
-    test_args = list(args) or []
-
+def configure_django():
+    """Configure Django before tests"""
     middleware = [
         'django.contrib.sessions.middleware.SessionMiddleware',
         'django.contrib.auth.middleware.AuthenticationMiddleware',
@@ -70,8 +69,54 @@ def run_test_suite(*args):
         **middleware_kwargs  # noqa: C815
     )
     setup()
+
+
+def run_test_suite(*args):
+    """Run the test suite"""
+    test_args = list(args) or []
     execute_from_command_line(['manage.py', 'test'] + test_args)
 
 
+def check_missing_migrations():
+    """Check that user model and migration files are in sync"""
+    from django.db.migrations.autodetector import MigrationAutodetector
+    from django.db.migrations.loader import MigrationLoader
+    from django.db.migrations.state import ProjectState
+    try:
+        from django.db.migrations.questioner import (
+            NonInteractiveMigrationQuestioner as Questioner,
+        )
+    except ImportError:
+        # TODO: remove this once Dj1.8 dropped
+        from django.db.migrations.questioner import (
+            InteractiveMigrationQuestioner as Questioner,
+        )
+
+    loader = MigrationLoader(None, ignore_no_migrations=True)
+    conflicts = loader.detect_conflicts()
+    if conflicts:
+        raise Exception(
+            'Migration conflicts detected. Please fix your migrations.')
+    questioner = Questioner(dry_run=True, specified_apps=None)
+    autodetector = MigrationAutodetector(
+        loader.project_state(),
+        ProjectState.from_apps(apps),
+        questioner,
+    )
+    changes = autodetector.changes(
+        graph=loader.graph,
+        trim_to_apps=None,
+        convert_apps=None,
+        migration_name=None,
+    )
+    if changes:
+        raise Exception(
+            'Migration changes detected. '
+            'Please update or add to the migration file as appropriate')
+    print('Migration-checker detected no problems.')
+
+
 if __name__ == '__main__':
+    configure_django()
+    check_missing_migrations()
     run_test_suite(*sys.argv[1:])

setup.py

@@ -125,7 +125,11 @@ def run(self):
                 self.run_tests()
 
     def run_tests(self):
-        from runtests import run_test_suite
+        from runtests import (
+            check_missing_migrations, configure_django, run_test_suite,
+        )
+        configure_django()
+        check_missing_migrations()
         run_test_suite(*self.args)
 
 

src/improved_user/migrations/0001_initial.py

@@ -23,7 +23,7 @@ class Migration(migrations.Migration):
                 ('last_login', models.DateTimeField(blank=True, null=True, verbose_name='last login')),
                 ('is_superuser', models.BooleanField(default=False, help_text='Designates that this user has all permissions without explicitly assigning them.', verbose_name='superuser status')),
                 ('full_name', models.CharField(blank=True, max_length=200, verbose_name='full name')),
-                ('short_name', models.CharField(max_length=50, verbose_name='short name')),
+                ('short_name', models.CharField(blank=True, max_length=50, verbose_name='short name')),
                 ('is_staff', models.BooleanField(default=False, help_text='Designates whether the user can log into the admin site.', verbose_name='staff status')),
                 ('is_active', models.BooleanField(default=True, help_text='Designates whether this user should be treated as active. Unselect this instead of deleting accounts.', verbose_name='active')),
                 ('date_joined', models.DateTimeField(default=django.utils.timezone.now, verbose_name='date joined')),

src/improved_user/models.py

@@ -78,8 +78,8 @@ def get_full_name(self):
 
 
 class ShortNameMixin(models.Model):
-    """A mixin to provide a non-blank short name field"""
-    short_name = models.CharField(_('short name'), max_length=50)
+    """A mixin to provide an optional short name field"""
+    short_name = models.CharField(_('short name'), max_length=50, blank=True)
 
     class Meta:
         abstract = True

tests/test_admin.py

@@ -33,12 +33,10 @@ def setUpTestData(cls):
         cls.user1 = User.objects.create_user(
             email='testclient@example.com',
             password='password',
-            short_name='client',
         )
         cls.user2 = User.objects.create_user(
             email='staffmember@example.com',
             password='password',
-            short_name='staff',
         )
 
     def setUp(self):
@@ -136,7 +134,6 @@ def test_user_add(self):
             reverse('auth_test_admin:improved_user_user_add'),
             {
                 'email': 'newuser@example.com',
-                'short_name': 'New User',
                 'password1': 'passw0rd1!',
                 'password2': 'passw0rd1!',
             },
@@ -147,7 +144,6 @@ def test_user_add(self):
             User.objects.filter(email='newuser@example.com').exists())
         new_user = User.objects.get(email='newuser@example.com')
         self.assertTrue(new_user.check_password('passw0rd1!'))
-        self.assertEqual(new_user.short_name, 'New User')
 
     def test_user_change_email(self):
         """Test that user can change email in Admin"""

tests/test_forms.py

@@ -48,7 +48,6 @@ def test_user_already_exists(self):
             'email': 'testclient@example.com',
             'password1': 'test123',
             'password2': 'test123',
-            'short_name': 'John',
         }
         form = UserCreationForm(data)
         self.assertFalse(form.is_valid())
@@ -61,7 +60,6 @@ def test_invalid_data(self):
             'email': '%%%',
             'password1': 'test123',
             'password2': 'test123',
-            'short_name': 'John',
         }
         form = UserCreationForm(data)
         self.assertFalse(form.is_valid())
@@ -76,7 +74,6 @@ def test_password_verification(self):
             'email': 'jsmith@example.com',
             'password1': 'test123',
             'password2': 'test',
-            'short_name': 'John',
         }
         form = UserCreationForm(data)
         self.assertFalse(form.is_valid())
@@ -107,7 +104,7 @@ def test_success(self, password_changed):
         data = {
             'email': 'jsmith@example.com',
             'full_name': 'John Smith',  # optional field
-            'short_name': 'John',
+            'short_name': 'John',  # optional field
             'password1': 'test123',
             'password2': 'test123',
         }
@@ -118,6 +115,9 @@ def test_success(self, password_changed):
         user = form.save()
         self.assertEqual(password_changed.call_count, 1)
         self.assertEqual(repr(user), '<User: jsmith@example.com>')
+        self.assertEqual(user.get_short_name(), 'John')
+        self.assertEqual(user.get_full_name(), 'John Smith')
+        self.assertTrue(user.check_password('test123'))
 
     # TODO: Remove this test in favor of above after Dj1.8 dropped
     @skipUnless(
@@ -128,14 +128,17 @@ def test_success_pre_19(self):
         data = {
             'email': 'jsmith@example.com',
             'full_name': 'John Smith',  # optional field
-            'short_name': 'John',
+            'short_name': 'John',  # optional field
             'password1': 'test123',
             'password2': 'test123',
         }
         form = UserCreationForm(data)
         self.assertTrue(form.is_valid())
         user = form.save()
         self.assertEqual(repr(user), '<User: jsmith@example.com>')
+        self.assertEqual(user.get_short_name(), 'John')
+        self.assertEqual(user.get_full_name(), 'John Smith')
+        self.assertTrue(user.check_password('test123'))
 
     @skipUnless(
         DjangoVersion >= (1, 9),
@@ -153,7 +156,6 @@ def test_common_password(self):
             'email': 'jsmith@example.com',
             'password1': 'password',
             'password2': 'password',
-            'short_name': 'John',
         }
         form = UserCreationForm(data)
         self.assertFalse(form.is_valid())
@@ -179,7 +181,6 @@ def test_validates_password_similarity_length(self):
             'email': 'jsmith@example.com',
             'password1': 'jsmith',
             'password2': 'jsmith',
-            'short_name': 'John',
         }
         form = UserCreationForm(data)
         self.assertFalse(form.is_valid())
@@ -241,7 +242,6 @@ def test_password_whitespace_not_stripped(self):
             'email': 'jsmith@example.com',
             'password1': '   test password   ',
             'password2': '   test password   ',
-            'short_name': 'John',
         }
         form = UserCreationForm(data)
         self.assertTrue(form.is_valid())