inactive surveys do not have active targeting flags

PostHog · Nov 15, 2023 · a20b2b6 · a20b2b6
1 parent 422e8f8
commit a20b2b6
Show file tree

Hide file tree

Showing 2 changed files with 66 additions and 4 deletions.
diff --git a/posthog/api/survey.py b/posthog/api/survey.py
@@ -221,19 +221,29 @@ def update(self, instance: Survey, validated_data):
                 existing_flag_serializer.is_valid(raise_exception=True)
                 existing_flag_serializer.save()
             else:
-                new_flag = self._create_new_targeting_flag(instance.name, new_filters)
+                new_flag = self._create_new_targeting_flag(instance.name, new_filters, instance.start_date)
                 validated_data["targeting_flag_id"] = new_flag.id
             validated_data.pop("targeting_flag_filters")
 
+        end_date = validated_data.get("end_date")
+        if instance.targeting_flag:
+            # turn off feature flag if survey is ended
+            if end_date is None:
+                instance.targeting_flag.active = True
+            elif end_date:
+                instance.targeting_flag.active = False
+            instance.targeting_flag.save()
+
         return super().update(instance, validated_data)
 
-    def _create_new_targeting_flag(self, name, filters):
+    def _create_new_targeting_flag(self, name, filters, start_date=None):
         feature_flag_key = slugify(f"{SURVEY_TARGETING_FLAG_PREFIX}{name}")
         feature_flag_serializer = FeatureFlagSerializer(
             data={
                 "key": feature_flag_key,
                 "name": f"Targeting flag for survey {name}",
                 "filters": filters,
+                "active": True if start_date else False,
             },
             context=self.context,
         )

diff --git a/posthog/api/test/test_survey.py b/posthog/api/test/test_survey.py
@@ -365,7 +365,7 @@ def test_updating_survey_with_targeting_creates_or_updates_targeting_flag(self):
             "groups": [{"variant": None, "properties": [], "rollout_percentage": 20}]
         }
 
-    def test_updating_survey_to_remove_targeting_doesnt_delete_targeting_flag(self):
+    def test_updating_survey_to_send_none_targeting_doesnt_delete_targeting_flag(self):
         survey_with_targeting = self.client.post(
             f"/api/projects/{self.team.id}/surveys/",
             data={
@@ -409,7 +409,7 @@ def test_updating_survey_to_remove_targeting_doesnt_delete_targeting_flag(self):
 
         assert FeatureFlag.objects.filter(id=flagId).exists()
 
-    def test_updating_survey_to_send_none_targeting_deletes_targeting_flag(self):
+    def test_updating_survey_to_remove_targeting_deletes_targeting_flag(self):
         survey_with_targeting = self.client.post(
             f"/api/projects/{self.team.id}/surveys/",
             data={
@@ -697,6 +697,58 @@ def test_deleting_survey_deletes_targeting_flag(self):
         assert deleted_survey.status_code == status.HTTP_204_NO_CONTENT
         assert not FeatureFlag.objects.filter(id=response.json()["targeting_flag"]["id"]).exists()
 
+    def test_inactive_surveys_disables_targeting_flag(self):
+        survey_with_targeting = self.client.post(
+            f"/api/projects/{self.team.id}/surveys/",
+            data={
+                "name": "survey with targeting",
+                "type": "popover",
+                "targeting_flag_filters": {
+                    "groups": [
+                        {
+                            "variant": None,
+                            "rollout_percentage": None,
+                            "properties": [
+                                {
+                                    "key": "billing_plan",
+                                    "value": ["cloud"],
+                                    "operator": "exact",
+                                    "type": "person",
+                                }
+                            ],
+                        }
+                    ]
+                },
+                "conditions": {"url": "https://app.posthog.com/notebooks"},
+            },
+            format="json",
+        ).json()
+        assert FeatureFlag.objects.filter(id=survey_with_targeting["targeting_flag"]["id"]).get().active is False
+        # launch survey
+        self.client.patch(
+            f"/api/projects/{self.team.id}/surveys/{survey_with_targeting['id']}/",
+            data={
+                "start_date": datetime.now() - timedelta(days=1),
+            },
+        )
+        assert FeatureFlag.objects.filter(id=survey_with_targeting["targeting_flag"]["id"]).get().active is True
+        # stop the survey
+        self.client.patch(
+            f"/api/projects/{self.team.id}/surveys/{survey_with_targeting['id']}/",
+            data={
+                "end_date": datetime.now() + timedelta(days=1),
+            },
+        )
+        assert FeatureFlag.objects.filter(id=survey_with_targeting["targeting_flag"]["id"]).get().active is False
+        # resume survey again
+        self.client.patch(
+            f"/api/projects/{self.team.id}/surveys/{survey_with_targeting['id']}/",
+            data={
+                "end_date": None,
+            },
+        )
+        assert FeatureFlag.objects.filter(id=survey_with_targeting["targeting_flag"]["id"]).get().active is True
+
     def test_can_list_surveys(self):
         self.client.post(
             f"/api/projects/{self.team.id}/surveys/",