Skip to content

Commit

Permalink
feat: make User.full_name field non-nullable
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
MichaelRoytman committed Oct 2, 2023
1 parent 6321690 commit ea96800
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 2 deletions.
18 changes: 18 additions & 0 deletions edx_exams/apps/core/migrations/0019_alter_user_full_name.py
Original file line number Diff line number Diff line change
@@ -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'),
),
]
3 changes: 2 additions & 1 deletion edx_exams/apps/core/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
2 changes: 1 addition & 1 deletion edx_exams/apps/core/tests/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down

0 comments on commit ea96800

Please sign in to comment.