diff --git a/frontend/src/scenes/surveys/Survey.tsx b/frontend/src/scenes/surveys/Survey.tsx
index d59ed4b674e69..ec13e3cc733b2 100644
--- a/frontend/src/scenes/surveys/Survey.tsx
+++ b/frontend/src/scenes/surveys/Survey.tsx
@@ -386,19 +386,17 @@ 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', undefined)
+ setSurveyValue('targeting_flag', null)
+ }}
+ >
+ 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..21b8924699319 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
@@ -2113,7 +2113,7 @@ interface SurveyQuestionBase {
}
export interface BasicSurveyQuestion extends SurveyQuestionBase {
- type: SurveyQuestionType.Open | SurveyQuestionType.NPS
+ type: SurveyQuestionType.Open
}
export interface LinkSurveyQuestion extends SurveyQuestionBase {
@@ -2140,7 +2140,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..53f7fe2b491d7 100644
--- a/posthog/api/survey.py
+++ b/posthog/api/survey.py
@@ -56,7 +56,7 @@ 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)
class Meta:
model = Survey
@@ -82,7 +82,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 +91,38 @@ 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):
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"]
)
@@ -111,7 +134,7 @@ def create(self, validated_data):
def update(self, instance: Survey, validated_data):
# 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"):
if instance.targeting_flag:
existing_targeting_flag = instance.targeting_flag
serialized_data_filters = {
@@ -130,6 +153,10 @@ def update(self, instance: Survey, validated_data):
new_flag = self._create_new_targeting_flag(instance.name, validated_data["targeting_flag_filters"])
validated_data["targeting_flag_id"] = new_flag.id
validated_data.pop("targeting_flag_filters")
+ else:
+ if instance.targeting_flag:
+ instance.targeting_flag.delete()
+ validated_data["targeting_flag_id"] = None
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..4f3ac5d9b27b9 100644
--- a/posthog/api/test/test_survey.py
+++ b/posthog/api/test/test_survey.py
@@ -219,14 +219,206 @@ 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_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={
+ "name": "other",
+ # "targeting_flag_filters": None, # 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 None
+
+ with self.assertRaises(FeatureFlag.DoesNotExist):
+ FeatureFlag.objects.get(id=flagId)
+
+ 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={
+ "targeting_flag_filters": None, # delete these
+ },
+ )
+
+ 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_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 +508,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):