fix: add reverse sql to versioning migration (#4491)

api/segments/migrations/0023_add_versioning_to_segments.py

@@ -1,9 +1,17 @@
 # Generated by Django 3.2.25 on 2024-06-10 15:31
-import os
+from pathlib import Path
 
 import django.db.models.deletion
 from django.db import migrations, models
 
+parent_dir = Path(__file__).parent.resolve()
+
+with open(parent_dir / "sql/0023_add_versioning_to_segments.sql") as f:
+    segment_versioning_sql_forwards = f.read()
+
+with open(parent_dir / "sql/0023_add_versioning_to_segments_reverse.sql") as f:
+    segment_versioning_sql_reverse = f.read()
+
 
 class Migration(migrations.Migration):
 
@@ -46,13 +54,7 @@ class Migration(migrations.Migration):
             ),
         ),
         migrations.RunSQL(
-            sql=open(
-                os.path.join(
-                    os.path.dirname(__file__),
-                    "sql",
-                    "0023_add_versioning_to_segments.sql",
-                )
-            ).read(),
-            reverse_sql=migrations.RunSQL.noop,
+            sql=segment_versioning_sql_forwards,
+            reverse_sql=segment_versioning_sql_reverse,
         ),
     ]

api/segments/migrations/sql/0023_add_versioning_to_segments_reverse.sql

@@ -0,0 +1,3 @@
+UPDATE segments_segment
+SET deleted_at = now()
+WHERE version_of_id <> id;

api/tests/unit/segments/test_unit_segments_migrations.py

@@ -0,0 +1,239 @@
+import uuid
+
+import pytest
+from django.conf import settings as test_settings
+from django_test_migrations.migrator import Migrator
+from flag_engine.segments import constants
+from pytest_django.fixtures import SettingsWrapper
+
+
+@pytest.mark.skipif(
+    test_settings.SKIP_MIGRATION_TESTS is True,
+    reason="Skip migration tests to speed up tests where necessary",
+)
+def test_create_whitelisted_segments_migration(
+    migrator: Migrator,
+    settings: SettingsWrapper,
+) -> None:
+    # Given - The migration state is at 0020 (before the migration we want to test).
+    old_state = migrator.apply_initial_migration(
+        ("segments", "0020_detach_segment_from_project_cascade_delete")
+    )
+
+    Organisation = old_state.apps.get_model("organisations", "Organisation")
+    Project = old_state.apps.get_model("projects", "Project")
+    SegmentRule = old_state.apps.get_model("segments", "SegmentRule")
+    Segment = old_state.apps.get_model("segments", "Segment")
+    Condition = old_state.apps.get_model("segments", "Condition")
+
+    # Set the limit lower to allow for a faster test.
+    settings.SEGMENT_RULES_CONDITIONS_LIMIT = 3
+
+    # Next, create the setup data.
+    organisation = Organisation.objects.create(name="Big Corp Incorporated")
+    project = Project.objects.create(name="Huge Project", organisation=organisation)
+
+    segment_1 = Segment.objects.create(name="Segment1", project=project)
+    segment_2 = Segment.objects.create(name="Segment1", project=project)
+    segment_rule_1 = SegmentRule.objects.create(
+        segment=segment_1,
+        type="ALL",
+    )
+
+    # Subnested segment rules.
+    segment_rule_2 = SegmentRule.objects.create(
+        rule=segment_rule_1,
+        type="ALL",
+    )
+    segment_rule_3 = SegmentRule.objects.create(
+        rule=segment_rule_1,
+        type="ALL",
+    )
+
+    # Lonely segment rules for pass criteria for segment_2.
+    segment_rule_4 = SegmentRule.objects.create(
+        segment=segment_2,
+        type="ALL",
+    )
+    segment_rule_5 = SegmentRule.objects.create(
+        rule=segment_rule_4,
+        type="ALL",
+    )
+
+    Condition.objects.create(
+        operator=constants.EQUAL,
+        property="age",
+        value="21",
+        rule=segment_rule_2,
+    )
+    Condition.objects.create(
+        operator=constants.GREATER_THAN,
+        property="height",
+        value="210",
+        rule=segment_rule_2,
+    )
+    Condition.objects.create(
+        operator=constants.GREATER_THAN,
+        property="waist",
+        value="36",
+        rule=segment_rule_3,
+    )
+    Condition.objects.create(
+        operator=constants.LESS_THAN,
+        property="shoes",
+        value="12",
+        rule=segment_rule_3,
+    )
+
+    # Sole criteria for segment_2 conditions.
+    Condition.objects.create(
+        operator=constants.LESS_THAN,
+        property="toy_count",
+        value="7",
+        rule=segment_rule_5,
+    )
+
+    # When we run the migration.
+    new_state = migrator.apply_tested_migration(
+        ("segments", "0021_create_whitelisted_segments")
+    )
+
+    # Then the first segment is in the whitelist while the second is not.
+    NewSegment = new_state.apps.get_model("segments", "Segment")
+    new_segment_1 = NewSegment.objects.get(id=segment_1.id)
+    new_segment_2 = NewSegment.objects.get(id=segment_2.id)
+    assert new_segment_1.whitelisted_segment
+    assert getattr(new_segment_2, "whitelisted_segment", None) is None
+
+
+@pytest.mark.skipif(
+    test_settings.SKIP_MIGRATION_TESTS is True,
+    reason="Skip migration tests to speed up tests where necessary",
+)
+def test_add_versioning_to_segments_forwards(migrator: Migrator) -> None:
+    # Given - The migration state is at 0021 (before the migration we want to test).
+    old_state = migrator.apply_initial_migration(
+        ("segments", "0022_add_soft_delete_to_segment_rules_and_conditions")
+    )
+
+    Organisation = old_state.apps.get_model("organisations", "Organisation")
+    Project = old_state.apps.get_model("projects", "Project")
+    SegmentRule = old_state.apps.get_model("segments", "SegmentRule")
+    Segment = old_state.apps.get_model("segments", "Segment")
+    Condition = old_state.apps.get_model("segments", "Condition")
+
+    # Next, create the setup data.
+    organisation = Organisation.objects.create(name="Test Org")
+    project = Project.objects.create(name="Test Project", organisation_id=organisation.id)
+
+    segment = Segment.objects.create(name="Segment1", project_id=project.id)
+    segment_rule_1 = SegmentRule.objects.create(
+        segment_id=segment.id,
+        type="ALL",
+    )
+
+    # Subnested segment rules.
+    segment_rule_2 = SegmentRule.objects.create(
+        rule_id=segment_rule_1.id,
+        type="ALL",
+    )
+
+    Condition.objects.create(
+        operator=constants.EQUAL,
+        property="age",
+        value="21",
+        rule_id=segment_rule_2.id,
+    )
+
+    # When we run the migration.
+    new_state = migrator.apply_tested_migration(
+        ("segments", "0023_add_versioning_to_segments")
+    )
+
+    # Then the version_of attribute is correctly set.
+    NewSegment = new_state.apps.get_model("segments", "Segment")
+    new_segment = NewSegment.objects.get(id=segment.id)
+    assert new_segment.version_of == new_segment
+
+
+@pytest.mark.skipif(
+    test_settings.SKIP_MIGRATION_TESTS is True,
+    reason="Skip migration tests to speed up tests where necessary",
+)
+def test_add_versioning_to_segments_reverse(migrator: Migrator) -> None:
+    # Given - The migration state is at 0023 (after the migration we want to test).
+    old_state = migrator.apply_initial_migration(
+        ("segments", "0023_add_versioning_to_segments")
+    )
+
+    Organisation = old_state.apps.get_model("organisations", "Organisation")
+    Project = old_state.apps.get_model("projects", "Project")
+    SegmentRule = old_state.apps.get_model("segments", "SegmentRule")
+    Segment = old_state.apps.get_model("segments", "Segment")
+    Condition = old_state.apps.get_model("segments", "Condition")
+
+    # Next, create the setup data.
+    organisation = Organisation.objects.create(name="Test Org")
+    project = Project.objects.create(name="Test Project", organisation=organisation)
+
+    # Set the version manually since this is normally done via a lifecycle hook
+    # that doesn't run for models created in a migration state.
+    segment = Segment.objects.create(name="Segment1", project=project, version=1)
+    segment_rule_1 = SegmentRule.objects.create(
+        segment=segment,
+        type="ALL",
+    )
+
+    # We ideally want to call Segment.deep_clone but that's not
+    # possible when working in a migration state. As such, we
+    # do the basic amount necessary from that method to allow
+    # us to test the migration behaviour.
+    def _deep_clone(segment: Segment) -> Segment:
+        cloned_segment = Segment.objects.create(
+            name=segment.name,
+            project_id=segment.project_id,
+            description=segment.description,
+            feature=segment.feature,
+            uuid=uuid.uuid4(),
+            version_of_id=segment.id,
+        )
+
+        segment.version += 1
+        segment.save()
+
+        return cloned_segment
+
+    version_1 = _deep_clone(segment)
+    version_2 = _deep_clone(segment)
+
+    version_3 = segment
+
+    # Subnested segment rules.
+    segment_rule_2 = SegmentRule.objects.create(
+        rule=segment_rule_1,
+        type="ALL",
+    )
+
+    Condition.objects.create(
+        operator=constants.EQUAL,
+        property="age",
+        value="21",
+        rule=segment_rule_2,
+    )
+
+    # When we run the migration in reverse.
+    new_state = migrator.apply_tested_migration(
+        ("segments", "0022_add_soft_delete_to_segment_rules_and_conditions")
+    )
+
+    # Then any historical versions of the segment are deleted.
+    NewSegment = new_state.apps.get_model("segments", "Segment")
+
+    new_segment_v1 = NewSegment.objects.get(id=version_1.id)
+    assert new_segment_v1.deleted_at is not None
+
+    new_segment_v2 = NewSegment.objects.get(id=version_2.id)
+    assert new_segment_v2.deleted_at is not None
+
+    new_segment_v3 = NewSegment.objects.get(id=version_3.id)
+    assert new_segment_v3.deleted_at is None