diff --git a/frontend/src/scenes/surveys/Survey.tsx b/frontend/src/scenes/surveys/Survey.tsx
index d59ed4b674e69..73879b41e70ac 100644
--- a/frontend/src/scenes/surveys/Survey.tsx
+++ b/frontend/src/scenes/surveys/Survey.tsx
@@ -376,6 +376,7 @@ export function SurveyForm({ id }: { id: string }): JSX.Element {
className="w-max"
onClick={() => {
setSurveyValue('targeting_flag_filters', { groups: [] })
+ setSurveyValue('remove_targeting_flag', false)
}}
>
Add user targeting
@@ -386,19 +387,18 @@ export function SurveyForm({ id }: { id: string }): JSX.Element {
- {id === 'new' && (
- {
- setSurveyValue('targeting_flag_filters', undefined)
- setSurveyValue('targeting_flag', null)
- }}
- >
- Remove all user properties
-
- )}
+ {
+ setSurveyValue('targeting_flag_filters', null)
+ setSurveyValue('targeting_flag', null)
+ setSurveyValue('remove_targeting_flag', true)
+ }}
+ >
+ Remove all user properties
+
)}
diff --git a/frontend/src/scenes/surveys/surveyLogic.tsx b/frontend/src/scenes/surveys/surveyLogic.tsx
index 9c4fe305a8b79..a09148203b5ef 100644
--- a/frontend/src/scenes/surveys/surveyLogic.tsx
+++ b/frontend/src/scenes/surveys/surveyLogic.tsx
@@ -346,7 +346,7 @@ export const surveyLogic = kea([
hasTargetingFlag: [
(s) => [s.survey],
(survey): boolean => {
- return !!survey.targeting_flag || !!(survey.id === 'new' && survey.targeting_flag_filters)
+ return !!survey.targeting_flag || !!survey.targeting_flag_filters
},
],
}),
diff --git a/frontend/src/types.ts b/frontend/src/types.ts
index de8b30192d4a6..32e0e2862ca3f 100644
--- a/frontend/src/types.ts
+++ b/frontend/src/types.ts
@@ -2074,7 +2074,7 @@ export interface Survey {
linked_flag: FeatureFlagBasicType | null
targeting_flag: FeatureFlagBasicType | null
targeting_flag_filters: Pick | undefined
- conditions: { url: string; selector: string; is_headless?: boolean } | null
+ conditions: { url: string; selector: string; is_headless?: boolean; seenSurveyWaitPeriodInDays?: number } | null
appearance: SurveyAppearance
questions: (BasicSurveyQuestion | LinkSurveyQuestion | RatingSurveyQuestion | MultipleSurveyQuestion)[]
created_at: string
@@ -2082,6 +2082,7 @@ export interface Survey {
start_date: string | null
end_date: string | null
archived: boolean
+ remove_targeting_flag?: boolean
}
export enum SurveyType {
@@ -2113,7 +2114,7 @@ interface SurveyQuestionBase {
}
export interface BasicSurveyQuestion extends SurveyQuestionBase {
- type: SurveyQuestionType.Open | SurveyQuestionType.NPS
+ type: SurveyQuestionType.Open
}
export interface LinkSurveyQuestion extends SurveyQuestionBase {
@@ -2140,7 +2141,6 @@ export enum SurveyQuestionType {
Open = 'open',
MultipleChoice = 'multiple_choice',
SingleChoice = 'single_choice',
- NPS = 'nps',
Rating = 'rating',
Link = 'link',
}
diff --git a/posthog/api/survey.py b/posthog/api/survey.py
index 0e6135508edbe..b06e63487cd67 100644
--- a/posthog/api/survey.py
+++ b/posthog/api/survey.py
@@ -56,7 +56,8 @@ class Meta:
class SurveySerializerCreateUpdateOnly(SurveySerializer):
linked_flag_id = serializers.IntegerField(required=False, write_only=True, allow_null=True)
targeting_flag_id = serializers.IntegerField(required=False, write_only=True)
- targeting_flag_filters = serializers.JSONField(required=False, write_only=True)
+ targeting_flag_filters = serializers.JSONField(required=False, write_only=True, allow_null=True)
+ remove_targeting_flag = serializers.BooleanField(required=False, write_only=True, allow_null=True)
class Meta:
model = Survey
@@ -70,6 +71,7 @@ class Meta:
"targeting_flag_id",
"targeting_flag",
"targeting_flag_filters",
+ "remove_targeting_flag",
"questions",
"conditions",
"appearance",
@@ -82,7 +84,7 @@ class Meta:
read_only_fields = ["id", "linked_flag", "targeting_flag", "created_at"]
def validate(self, data):
- linked_flag_id = data.get("linked_flag_id", None)
+ linked_flag_id = data.get("linked_flag_id")
if linked_flag_id:
try:
FeatureFlag.objects.get(pk=linked_flag_id)
@@ -91,15 +93,41 @@ def validate(self, data):
if (
self.context["request"].method == "POST"
- and Survey.objects.filter(name=data.get("name", None), team_id=self.context["team_id"]).exists()
+ and Survey.objects.filter(name=data.get("name"), team_id=self.context["team_id"]).exists()
):
raise serializers.ValidationError("There is already a survey with this name.", code="unique")
+ existing_survey: Survey | None = self.instance
+
+ if (
+ existing_survey
+ and existing_survey.name != data.get("name")
+ and Survey.objects.filter(name=data.get("name"), team_id=self.context["team_id"])
+ .exclude(id=existing_survey.id)
+ .exists()
+ ):
+ raise serializers.ValidationError("There is already another survey with this name.", code="unique")
+
+ if data.get("targeting_flag_filters"):
+ groups = (data.get("targeting_flag_filters") or {}).get("groups") or []
+ full_rollout = any(
+ group.get("rollout_percentage") in [100, None] and len(group.get("properties", [])) == 0
+ for group in groups
+ )
+
+ if full_rollout:
+ raise serializers.ValidationError(
+ "Invalid operation: User targeting rolls out to everyone. If you want to roll out to everyone, delete this targeting",
+ code="invalid",
+ )
return data
def create(self, validated_data):
+ if "remove_targeting_flag" in validated_data:
+ validated_data.pop("remove_targeting_flag")
+
validated_data["team_id"] = self.context["team_id"]
- if validated_data.get("targeting_flag_filters", None):
+ if validated_data.get("targeting_flag_filters"):
targeting_feature_flag = self._create_new_targeting_flag(
validated_data["name"], validated_data["targeting_flag_filters"]
)
@@ -110,13 +138,21 @@ def create(self, validated_data):
return super().create(validated_data)
def update(self, instance: Survey, validated_data):
+
+ if validated_data.get("remove_targeting_flag"):
+ if instance.targeting_flag:
+ instance.targeting_flag.delete()
+ validated_data["targeting_flag_id"] = None
+ validated_data.pop("remove_targeting_flag")
+
# if the target flag filters come back with data, update the targeting feature flag if there is one, otherwise create a new one
- if validated_data.get("targeting_flag_filters", None):
+ if validated_data.get("targeting_flag_filters"):
+ new_filters = validated_data["targeting_flag_filters"]
if instance.targeting_flag:
existing_targeting_flag = instance.targeting_flag
serialized_data_filters = {
**existing_targeting_flag.filters,
- **validated_data["targeting_flag_filters"],
+ **new_filters,
}
existing_flag_serializer = FeatureFlagSerializer(
existing_targeting_flag,
@@ -127,9 +163,10 @@ 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, validated_data["targeting_flag_filters"])
+ new_flag = self._create_new_targeting_flag(instance.name, new_filters)
validated_data["targeting_flag_id"] = new_flag.id
validated_data.pop("targeting_flag_filters")
+
return super().update(instance, validated_data)
def _create_new_targeting_flag(self, name, filters):
diff --git a/posthog/api/test/test_feature_flag.py b/posthog/api/test/test_feature_flag.py
index b0d6f73c87ebb..b243b46200764 100644
--- a/posthog/api/test/test_feature_flag.py
+++ b/posthog/api/test/test_feature_flag.py
@@ -961,7 +961,7 @@ def test_getting_flags_is_not_nplus1(self) -> None:
format="json",
).json()
- with self.assertNumQueries(FuzzyInt(10, 11)):
+ with self.assertNumQueries(FuzzyInt(11, 12)):
response = self.client.get(f"/api/projects/{self.team.id}/feature_flags")
self.assertEqual(response.status_code, status.HTTP_200_OK)
@@ -972,7 +972,7 @@ def test_getting_flags_is_not_nplus1(self) -> None:
format="json",
).json()
- with self.assertNumQueries(FuzzyInt(10, 11)):
+ with self.assertNumQueries(FuzzyInt(11, 12)):
response = self.client.get(f"/api/projects/{self.team.id}/feature_flags")
self.assertEqual(response.status_code, status.HTTP_200_OK)
diff --git a/posthog/api/test/test_survey.py b/posthog/api/test/test_survey.py
index f393e5cec4379..820e4127edff4 100644
--- a/posthog/api/test/test_survey.py
+++ b/posthog/api/test/test_survey.py
@@ -77,6 +77,48 @@ def test_can_create_survey_with_linked_flag_and_targeting(self):
{"type": "open", "question": "What would you want to improve from notebooks?"}
]
+ def test_can_create_survey_with_targeting_with_remove_parameter(self):
+
+ response = self.client.post(
+ f"/api/projects/{self.team.id}/surveys/",
+ data={
+ "name": "Notebooks power users survey",
+ "type": "popover",
+ "questions": [{"type": "open", "question": "What would you want to improve from notebooks?"}],
+ "targeting_flag_filters": {
+ "groups": [
+ {
+ "variant": None,
+ "rollout_percentage": None,
+ "properties": [
+ {"key": "billing_plan", "value": ["cloud"], "operator": "exact", "type": "person"}
+ ],
+ }
+ ]
+ },
+ "remove_targeting_flag": False,
+ "conditions": {"url": "https://app.posthog.com/notebooks"},
+ },
+ format="json",
+ )
+
+ response_data = response.json()
+ assert response.status_code == status.HTTP_201_CREATED, response_data
+ assert FeatureFlag.objects.filter(id=response_data["targeting_flag"]["id"]).exists()
+ assert response_data["targeting_flag"]["filters"] == {
+ "groups": [
+ {
+ "variant": None,
+ "properties": [{"key": "billing_plan", "value": ["cloud"], "operator": "exact", "type": "person"}],
+ "rollout_percentage": None,
+ }
+ ]
+ }
+ assert response_data["conditions"] == {"url": "https://app.posthog.com/notebooks"}
+ assert response_data["questions"] == [
+ {"type": "open", "question": "What would you want to improve from notebooks?"}
+ ]
+
def test_used_in_survey_is_populated_correctly_for_feature_flag_list(self) -> None:
self.maxDiff = None
@@ -219,14 +261,241 @@ def test_updating_survey_with_targeting_creates_or_updates_targeting_flag(self):
updated_survey_updates_targeting_flag = self.client.patch(
f"/api/projects/{self.team.id}/surveys/{survey_with_targeting['id']}/",
data={
- "targeting_flag_filters": {"groups": [{"variant": None, "rollout_percentage": None, "properties": []}]},
+ "targeting_flag_filters": {"groups": [{"variant": None, "rollout_percentage": 20, "properties": []}]},
},
)
assert updated_survey_updates_targeting_flag.status_code == status.HTTP_200_OK
assert FeatureFlag.objects.filter(id=survey_with_targeting["targeting_flag"]["id"]).get().filters == {
- "groups": [{"variant": None, "properties": [], "rollout_percentage": None}]
+ "groups": [{"variant": None, "properties": [], "rollout_percentage": 20}]
}
+ def test_updating_survey_to_remove_targeting_doesnt_delete_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()
+
+ flagId = survey_with_targeting["targeting_flag"]["id"]
+ assert FeatureFlag.objects.filter(id=flagId).exists()
+
+ updated_survey_deletes_targeting_flag = self.client.patch(
+ f"/api/projects/{self.team.id}/surveys/{survey_with_targeting['id']}/",
+ data={
+ "name": "other",
+ # "targeting_flag_filters": None, # don't delete these
+ },
+ )
+
+ assert updated_survey_deletes_targeting_flag.status_code == status.HTTP_200_OK
+ assert updated_survey_deletes_targeting_flag.json()["name"] == "other"
+ assert updated_survey_deletes_targeting_flag.json()["targeting_flag"] is not None
+
+ assert FeatureFlag.objects.filter(id=flagId).exists()
+
+ def test_updating_survey_to_send_none_targeting_deletes_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()
+
+ flagId = survey_with_targeting["targeting_flag"]["id"]
+ assert FeatureFlag.objects.filter(id=flagId).exists()
+
+ updated_survey_deletes_targeting_flag = self.client.patch(
+ f"/api/projects/{self.team.id}/surveys/{survey_with_targeting['id']}/",
+ data={
+ "remove_targeting_flag": True, # delete targeting flag
+ },
+ )
+
+ assert updated_survey_deletes_targeting_flag.status_code == status.HTTP_200_OK
+ assert updated_survey_deletes_targeting_flag.json()["name"] == "survey with targeting"
+ assert updated_survey_deletes_targeting_flag.json()["targeting_flag"] is None
+
+ with self.assertRaises(FeatureFlag.DoesNotExist):
+ FeatureFlag.objects.get(id=flagId)
+
+ def test_updating_survey_other_props_doesnt_delete_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()
+
+ flagId = survey_with_targeting["targeting_flag"]["id"]
+ assert FeatureFlag.objects.filter(id=flagId).exists()
+
+ updated_survey_deletes_targeting_flag = self.client.patch(
+ f"/api/projects/{self.team.id}/surveys/{survey_with_targeting['id']}/",
+ data={"start_date": "2023-04-01T12:00:10"},
+ )
+
+ assert updated_survey_deletes_targeting_flag.status_code == status.HTTP_200_OK
+ assert updated_survey_deletes_targeting_flag.json()["name"] == "survey with targeting"
+ assert updated_survey_deletes_targeting_flag.json()["targeting_flag"] is not None
+
+ assert FeatureFlag.objects.filter(id=flagId).exists()
+
+ def test_survey_targeting_flag_validation(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()
+
+ flagId = survey_with_targeting["targeting_flag"]["id"]
+ assert FeatureFlag.objects.filter(id=flagId).exists()
+
+ updated_survey_deletes_targeting_flag = self.client.patch(
+ f"/api/projects/{self.team.id}/surveys/{survey_with_targeting['id']}/",
+ data={
+ "targeting_flag_filters": {
+ "groups": [
+ {
+ "variant": None,
+ "rollout_percentage": None,
+ "properties": [],
+ }
+ ]
+ },
+ },
+ )
+
+ invalid_detail = "Invalid operation: User targeting rolls out to everyone. If you want to roll out to everyone, delete this targeting"
+
+ assert updated_survey_deletes_targeting_flag.status_code == status.HTTP_400_BAD_REQUEST
+ assert updated_survey_deletes_targeting_flag.json()["detail"] == invalid_detail
+
+ updated_survey_deletes_targeting_flag = self.client.patch(
+ f"/api/projects/{self.team.id}/surveys/{survey_with_targeting['id']}/",
+ data={
+ "targeting_flag_filters": {
+ "groups": [
+ {
+ "variant": None,
+ "rollout_percentage": 100,
+ "properties": [{"key": "value"}],
+ },
+ {
+ "variant": None,
+ "rollout_percentage": None,
+ "properties": [],
+ },
+ ]
+ },
+ },
+ )
+
+ assert updated_survey_deletes_targeting_flag.status_code == status.HTTP_400_BAD_REQUEST
+ assert updated_survey_deletes_targeting_flag.json()["detail"] == invalid_detail
+
+ updated_survey_deletes_targeting_flag = self.client.patch(
+ f"/api/projects/{self.team.id}/surveys/{survey_with_targeting['id']}/",
+ data={
+ "targeting_flag_filters": {
+ "groups": [
+ {
+ "variant": None,
+ "rollout_percentage": 100,
+ "properties": [{"key": "value"}],
+ },
+ {
+ "variant": None,
+ "rollout_percentage": 100,
+ "properties": [],
+ },
+ ]
+ },
+ },
+ )
+
+ assert updated_survey_deletes_targeting_flag.status_code == status.HTTP_400_BAD_REQUEST
+ assert updated_survey_deletes_targeting_flag.json()["detail"] == invalid_detail
+
+ updated_survey_deletes_targeting_flag = self.client.patch(
+ f"/api/projects/{self.team.id}/surveys/{survey_with_targeting['id']}/",
+ data={
+ "targeting_flag_filters": {
+ "groups": [
+ {
+ "variant": None,
+ "rollout_percentage": 100,
+ "properties": [{"key": "value", "type": "person", "value": "bleh"}],
+ },
+ {
+ "variant": None,
+ "rollout_percentage": 30,
+ "properties": [],
+ },
+ ]
+ },
+ },
+ )
+
+ assert updated_survey_deletes_targeting_flag.status_code == status.HTTP_200_OK
+
def test_deleting_survey_does_not_delete_linked_flag(self):
linked_flag = FeatureFlag.objects.create(team=self.team, key="early-access", created_by=self.user)
@@ -316,6 +585,49 @@ def test_can_list_surveys(self):
],
}
+ def test_updating_survey_name_validates(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()
+
+ self.client.post(
+ f"/api/projects/{self.team.id}/surveys/",
+ data={
+ "name": "survey without targeting",
+ "type": "popover",
+ },
+ format="json",
+ ).json()
+
+ updated_survey_deletes_targeting_flag = self.client.patch(
+ f"/api/projects/{self.team.id}/surveys/{survey_with_targeting['id']}/",
+ data={
+ "name": "survey without targeting",
+ },
+ )
+
+ assert updated_survey_deletes_targeting_flag.status_code == status.HTTP_400_BAD_REQUEST
+ assert (
+ updated_survey_deletes_targeting_flag.json()["detail"] == "There is already another survey with this name."
+ )
+
class TestSurveysAPIList(BaseTest, QueryMatchingTest):
def setUp(self):