fix: replace alter field with adding a new field (#4817)

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"
+    )

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"
+                    ),
+                ),
+            ]
+        )
     ]

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=""),
+    ]

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"