fix(experiments): Restrict deleting linked flag (#18543)

latest_migrations.manifest

@@ -5,7 +5,7 @@ contenttypes: 0002_remove_content_type_name
 ee: 0015_add_verified_properties
 otp_static: 0002_throttling
 otp_totp: 0002_auto_20190420_0723
-posthog: 0361_add_plugin_config_ui_fields
+posthog: 0362_alter_experiment_feature_flag
 sessions: 0001_initial
 social_django: 0010_uid_db_index
 two_factor: 0007_auto_20201201_1019

posthog/api/feature_flag.py

@@ -1,7 +1,7 @@
 import json
 from typing import Any, Dict, List, Optional, cast
 
-from django.db.models import QuerySet, Q
+from django.db.models import QuerySet, Q, deletion
 from django.conf import settings
 from rest_framework import (
     authentication,
@@ -256,7 +256,14 @@ def create(self, validated_data: Dict, *args: Any, **kwargs: Any) -> FeatureFlag
                 "Invalid variant definitions: Variant rollout percentages must sum to 100."
             )
 
-        FeatureFlag.objects.filter(key=validated_data["key"], team_id=self.context["team_id"], deleted=True).delete()
+        try:
+            FeatureFlag.objects.filter(
+                key=validated_data["key"], team_id=self.context["team_id"], deleted=True
+            ).delete()
+        except deletion.RestrictedError:
+            raise exceptions.ValidationError(
+                "Feature flag with this key already exists and is used in an experiment. Please delete the experiment before deleting the flag."
+            )
         instance: FeatureFlag = super().create(validated_data)
 
         self._attempt_set_tags(tags, instance)

posthog/api/test/test_organization_feature_flag.py

@@ -226,6 +226,7 @@ def test_copy_feature_flag_update_existing(self):
 
     def test_copy_feature_flag_update_override_deleted(self):
         target_project = self.team_2
+        target_project_2 = Team.objects.create(organization=self.organization)
         rollout_percentage_existing = 99
 
         existing_deleted_flag = FeatureFlag.objects.create(
@@ -238,9 +239,18 @@ def test_copy_feature_flag_update_override_deleted(self):
             ensure_experience_continuity=False,
             deleted=True,
         )
+        existing_deleted_flag2 = FeatureFlag.objects.create(
+            team=target_project_2,
+            created_by=self.user,
+            key=self.feature_flag_key,
+            name="Existing flag",
+            filters={"groups": [{"rollout_percentage": rollout_percentage_existing}]},
+            rollout_percentage=rollout_percentage_existing,
+            ensure_experience_continuity=False,
+            deleted=True,
+        )
 
         # The following instances must be overriden for a soft-deleted flag
-        Experiment.objects.create(team=self.team_2, created_by=self.user, feature_flag_id=existing_deleted_flag.id)
         Survey.objects.create(team=self.team, created_by=self.user, linked_flag=existing_deleted_flag)
 
         analytics_dashboard = Dashboard.objects.create(
@@ -256,12 +266,17 @@ def test_copy_feature_flag_update_override_deleted(self):
         existing_deleted_flag.usage_dashboard = usage_dashboard
         existing_deleted_flag.save()
 
+        # Experiments restrict deleting soft-deleted flags
+        Experiment.objects.create(
+            team=target_project_2, created_by=self.user, feature_flag_id=existing_deleted_flag2.id
+        )
+
         url = f"/api/organizations/{self.organization.id}/feature_flags/copy_flags"
 
         data = {
             "feature_flag_key": self.feature_flag_to_copy.key,
             "from_project": self.feature_flag_to_copy.team_id,
-            "target_project_ids": [target_project.id],
+            "target_project_ids": [target_project.id, target_project_2.id],
         }
         response = self.client.post(url, data)
 
@@ -314,6 +329,14 @@ def test_copy_feature_flag_update_override_deleted(self):
             set(flag_response.keys()),
         )
 
+        # target_project_2 should have failed
+        self.assertEqual(len(response.json()["failed"]), 1)
+        self.assertEqual(response.json()["failed"][0]["project_id"], target_project_2.id)
+        self.assertEqual(
+            response.json()["failed"][0]["errors"],
+            "[ErrorDetail(string='Feature flag with this key already exists and is used in an experiment. Please delete the experiment before deleting the flag.', code='invalid')]",
+        )
+
     def test_copy_feature_flag_missing_fields(self):
         url = f"/api/organizations/{self.organization.id}/feature_flags/copy_flags"
         data: Dict[str, Any] = {}

posthog/migrations/0362_alter_experiment_feature_flag.py

@@ -0,0 +1,18 @@
+# Generated by Django 3.2.19 on 2023-11-10 11:44
+
+from django.db import migrations, models
+import django.db.models.deletion
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("posthog", "0361_add_plugin_config_ui_fields"),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name="experiment",
+            name="feature_flag",
+            field=models.ForeignKey(on_delete=django.db.models.deletion.RESTRICT, to="posthog.featureflag"),
+        ),
+    ]

posthog/models/experiment.py

@@ -23,7 +23,7 @@ class Experiment(models.Model):
     # A list of filters for secondary metrics
     secondary_metrics: models.JSONField = models.JSONField(default=list, null=True)
 
-    feature_flag: models.ForeignKey = models.ForeignKey("FeatureFlag", blank=False, on_delete=models.CASCADE)
+    feature_flag: models.ForeignKey = models.ForeignKey("FeatureFlag", blank=False, on_delete=models.RESTRICT)
     created_by: models.ForeignKey = models.ForeignKey("User", on_delete=models.CASCADE)
     start_date: models.DateTimeField = models.DateTimeField(null=True)
     end_date: models.DateTimeField = models.DateTimeField(null=True)