From a20b2b6d7f11138b5742b903fe42b58cdac7de32 Mon Sep 17 00:00:00 2001 From: Li Yi Yu Date: Wed, 15 Nov 2023 15:25:39 -0500 Subject: [PATCH] inactive surveys do not have active targeting flags --- posthog/api/survey.py | 14 +++++++-- posthog/api/test/test_survey.py | 56 +++++++++++++++++++++++++++++++-- 2 files changed, 66 insertions(+), 4 deletions(-) diff --git a/posthog/api/survey.py b/posthog/api/survey.py index a2b3e8c3fcdd3..3c6f22aac4d09 100644 --- 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 index 92008ce32657d..75cd3d1c91e5b 100644 --- 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/",