diff --git a/api/tests/unit/users/test_unit_users_migrations.py b/api/tests/unit/users/test_unit_users_migrations.py new file mode 100644 index 000000000000..4f55dbc567c8 --- /dev/null +++ b/api/tests/unit/users/test_unit_users_migrations.py @@ -0,0 +1,53 @@ +import pytest +from django.conf import settings +from django_test_migrations.migrator import Migrator + +pytestmark = pytest.mark.skipif( + settings.SKIP_MIGRATION_TESTS is True, + reason="Skip migration tests to speed up tests where necessary", +) + + +def test_0039_ffadminuser_first_name_v2__values_expected(migrator: Migrator) -> None: + # Given + old_state = migrator.apply_initial_migration( + ("users", "0038_create_hubspot_tracker") + ) + OldFFAdminUser = old_state.apps.get_model("users", "FFAdminUser") + + user = OldFFAdminUser.objects.create(first_name="Testfirstname") + + # When + new_state = migrator.apply_tested_migration( + ("users", "0039_alter_ffadminuser_first_name") + ) + NewFFAdminUser = new_state.apps.get_model("users", "FFAdminUser") + + # Then + assert NewFFAdminUser.objects.get(id=user.id).first_name == user.first_name + + +def test_0039_ffadminuser_first_name_v2__reverse__values_expected( + migrator: Migrator, +) -> None: + # Given + old_state = migrator.apply_initial_migration( + ("users", "0039_alter_ffadminuser_first_name") + ) + NewFFAdminUser = old_state.apps.get_model("users", "FFAdminUser") + + user = NewFFAdminUser.objects.create( + first_name="TestfirstnameTestfirstnameTestfirstnameTestfirstname" + ) + + # When + new_state = migrator.apply_tested_migration( + ("users", "0038_create_hubspot_tracker") + ) + OldFFAdminUser = new_state.apps.get_model("users", "FFAdminUser") + + # Then + assert ( + OldFFAdminUser.objects.get(id=user.id).first_name + == "TestfirstnameTestfirstnameTest" + ) diff --git a/api/users/migrations/0039_alter_ffadminuser_first_name.py b/api/users/migrations/0039_alter_ffadminuser_first_name.py index 11406f6da6e9..0731faf64c2e 100644 --- a/api/users/migrations/0039_alter_ffadminuser_first_name.py +++ b/api/users/migrations/0039_alter_ffadminuser_first_name.py @@ -1,6 +1,21 @@ # Generated by Django 4.2.16 on 2024-11-04 17:09 - +from django.apps.registry import Apps from django.db import migrations, models +from django.db.backends.base.schema import BaseDatabaseSchemaEditor +from django.db.models import F +from django.db.models.functions import Left + + +def populate_new_first_name_field(apps: Apps, schema_editor: BaseDatabaseSchemaEditor) -> None: + FFAdminUser = apps.get_model("users", "FFAdminUser") + + FFAdminUser.objects.update(first_name_v2=F("first_name")) + + +def populate_old_first_name_field(apps: Apps, schema_editor: BaseDatabaseSchemaEditor) -> None: + FFAdminUser = apps.get_model("users", "FFAdminUser") + + FFAdminUser.objects.update(first_name=Left("first_name_v2", 30)) class Migration(migrations.Migration): @@ -10,9 +25,38 @@ class Migration(migrations.Migration): ] operations = [ - migrations.AlterField( + migrations.AddField( model_name="ffadminuser", - name="first_name", - field=models.CharField(max_length=150, verbose_name="first name"), + name="first_name_v2", + field=models.CharField(max_length=150, default=""), ), + migrations.RunPython(populate_new_first_name_field, reverse_code=populate_old_first_name_field), + migrations.SeparateDatabaseAndState( + state_operations=[ + migrations.RenameField( + model_name="ffadminuser", + old_name="first_name", + new_name="_first_name_old", + ), + migrations.RenameField( + model_name="ffadminuser", + old_name="first_name_v2", + new_name="first_name", + ), + migrations.AlterField( + model_name="ffadminuser", + name="_first_name_old", + field=models.CharField( + db_column="first_name", max_length=30, verbose_name="first name" + ), + ), + migrations.AlterField( + model_name="ffadminuser", + name="first_name", + field=models.CharField( + db_column="first_name_v2", max_length=150, verbose_name="first name" + ), + ), + ] + ) ] diff --git a/api/users/migrations/0040_alter_ffadminuser_first_name_fix.py b/api/users/migrations/0040_alter_ffadminuser_first_name_fix.py new file mode 100644 index 000000000000..2f186185185e --- /dev/null +++ b/api/users/migrations/0040_alter_ffadminuser_first_name_fix.py @@ -0,0 +1,42 @@ +""" +This migration exists because 0039 was retrospectively fixed to prevent locking issues +on the users table. This migration ensures that: + + 1. in environments where 0039 was run before the fix was added, the new column is + correctly added to the database to match what the ORM expects and the data + is correctly migrted to the new column. + 2. in environments where 0039 was run after the fix was added, nothing happens. + +Note that we only need to care about Postgres databases here because we did not release +the bad migration for 0039 in a format that can be consumed by oracle / mysql. +""" + +# Generated by Django 4.2.16 on 2024-11-12 18:05 +import logging + +from django.db import migrations + +from core.migration_helpers import PostgresOnlyRunSQL + +forwards_sql = """ +DO +$do$ +BEGIN + IF NOT EXISTS (SELECT 1 FROM information_schema.columns WHERE table_name='users_ffadminuser' and column_name='first_name_v2') THEN + ALTER TABLE "users_ffadminuser" ADD COLUMN "first_name_v2" VARCHAR(150) NOT NULL DEFAULT ''; + UPDATE "users_ffadminuser" SET "first_name_v2" = "first_name"; + END IF; +END +$do$ +""" + + +class Migration(migrations.Migration): + + dependencies = [ + ("users", "0039_alter_ffadminuser_first_name"), + ] + + operations = [ + PostgresOnlyRunSQL(forwards_sql, reverse_sql=""), + ] diff --git a/api/users/models.py b/api/users/models.py index 2030ad05c3f7..32d7a567aa18 100644 --- a/api/users/models.py +++ b/api/users/models.py @@ -99,7 +99,9 @@ class FFAdminUser(LifecycleModel, AbstractUser): email = models.EmailField(unique=True, null=False) objects = UserManager() username = models.CharField(unique=True, max_length=150, null=True, blank=True) - first_name = models.CharField("first name", max_length=150) + first_name = models.CharField( + "first name", max_length=150, db_column="first_name_v2" + ) last_name = models.CharField("last name", max_length=150) google_user_id = models.CharField(max_length=50, null=True, blank=True) github_user_id = models.CharField(max_length=50, null=True, blank=True) @@ -111,6 +113,10 @@ class FFAdminUser(LifecycleModel, AbstractUser): choices=SignUpType.choices, max_length=100, blank=True, null=True ) + _first_name_old = models.CharField( + "first name", max_length=30, db_column="first_name" + ) + uuid = models.UUIDField(default=uuid.uuid4, editable=False, unique=True) USERNAME_FIELD = "email"