Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(surveys): inactive surveys do not have active targeting flags #18664

Merged
merged 2 commits into from
Nov 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions posthog/api/survey.py
Original file line number Diff line number Diff line change
Expand Up @@ -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, bool(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
else:
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, active=False):
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": active,
},
context=self.context,
)
Expand Down
56 changes: 54 additions & 2 deletions posthog/api/test/test_survey.py
Original file line number Diff line number Diff line change
Expand Up @@ -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={
Expand Down Expand Up @@ -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={
Expand Down Expand Up @@ -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/",
Expand Down
Loading