From ea96800cb306d5c4ed77176030d51468c9df38f6 Mon Sep 17 00:00:00 2001 From: michaelroytman Date: Mon, 2 Oct 2023 16:31:03 -0400 Subject: [PATCH] feat: make User.full_name field non-nullable This commit makes the User.full_name field non-nullable to ensure that there is only one representation of the empty value - the empty string. Currently, empty values for this field can be either None or the empty string. Because we're transitioning from a nullable to non-nullable state, a default is required to handle existing rows with a null value. This is best practice; Django discourages setting both null=True and blank=True on CharField model fields. Furthermore, this was required by our event bus work. If an empty value is represented by None, this causes issues with the event bus, because None is not JSON serializable. Instead of converting a None value to the empty string in the event producer, correcting the the model definition is a better approach. --- .../migrations/0019_alter_user_full_name.py | 18 ++++++++++++++++++ edx_exams/apps/core/models.py | 3 ++- edx_exams/apps/core/tests/test_models.py | 2 +- 3 files changed, 21 insertions(+), 2 deletions(-) create mode 100644 edx_exams/apps/core/migrations/0019_alter_user_full_name.py diff --git a/edx_exams/apps/core/migrations/0019_alter_user_full_name.py b/edx_exams/apps/core/migrations/0019_alter_user_full_name.py new file mode 100644 index 00000000..44194acf --- /dev/null +++ b/edx_exams/apps/core/migrations/0019_alter_user_full_name.py @@ -0,0 +1,18 @@ +# Generated by Django 3.2.21 on 2023-10-02 20:21 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('core', '0018_staff_roles'), + ] + + operations = [ + migrations.AlterField( + model_name='user', + name='full_name', + field=models.CharField(blank=True, default='', max_length=255, verbose_name='Full Name'), + ), + ] diff --git a/edx_exams/apps/core/models.py b/edx_exams/apps/core/models.py index d3e1df5a..7478f7a5 100644 --- a/edx_exams/apps/core/models.py +++ b/edx_exams/apps/core/models.py @@ -24,7 +24,8 @@ class User(AbstractUser): .. pii_retirement: local_api """ - full_name = models.CharField(_('Full Name'), max_length=255, blank=True, null=True) + # The default empty string was added to change full_name from nullable to non-nullable. + full_name = models.CharField(_('Full Name'), max_length=255, blank=True, default='') lms_user_id = models.IntegerField(null=True, db_index=True) diff --git a/edx_exams/apps/core/tests/test_models.py b/edx_exams/apps/core/tests/test_models.py index b3563f4f..d509d996 100644 --- a/edx_exams/apps/core/tests/test_models.py +++ b/edx_exams/apps/core/tests/test_models.py @@ -31,7 +31,7 @@ def test_get_full_name(self): first_name = 'Jerry' last_name = 'Seinfeld' - user = G(User, full_name=None, first_name=first_name, last_name=last_name) + user = G(User, first_name=first_name, last_name=last_name) expected = '{first_name} {last_name}'.format(first_name=first_name, last_name=last_name) self.assertEqual(user.get_full_name(), expected)