Skip to content

Commit

Permalink
Merge branch 'master' into validate-app-fetch
Browse files Browse the repository at this point in the history
  • Loading branch information
Twixes committed Sep 18, 2023
2 parents 02c6e0e + c9c4d9a commit 04eca49
Show file tree
Hide file tree
Showing 10 changed files with 34 additions and 292 deletions.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
3 changes: 2 additions & 1 deletion frontend/src/lib/components/Support/supportLogic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ export const TARGET_AREA_TO_NAME = {
feature_flags: 'Feature Flags',
analytics: 'Product Analytics (Insights, Dashboards, Annotations)',
session_replay: 'Session Replay (Recordings)',
toolbar: 'Toolbar & heatmaps',
surveys: 'Surveys',
}

Expand All @@ -81,7 +82,7 @@ export const URL_PATH_TO_TARGET_AREA: Record<string, SupportTicketTargetArea> =
persons: 'data_integrity',
groups: 'data_integrity',
app: 'apps',
toolbar: 'analytics',
toolbar: 'session_replay',
warehouse: 'data_warehouse',
surveys: 'surveys',
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,12 +81,10 @@ export function NotebookSelectList(props: NotebookSelectProps): JSX.Element {
const openNewNotebook = (): void => {
const title = newNotebookTitle ?? `Notes ${dayjs().format('DD/MM')}`

if (resource) {
createNotebook(title, NotebookTarget.Popover, [resource], (theNotebookLogic) => {
props.onNotebookOpened?.(theNotebookLogic)
loadNotebooksContainingResource()
})
}
createNotebook(title, NotebookTarget.Popover, resource ? [resource] : undefined, (theNotebookLogic) => {
props.onNotebookOpened?.(theNotebookLogic)
loadNotebooksContainingResource()
})

setShowPopover(false)
}
Expand Down
24 changes: 13 additions & 11 deletions frontend/src/scenes/surveys/Survey.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -386,17 +386,19 @@ export function SurveyForm({ id }: { id: string }): JSX.Element {
<div className="mt-2">
<FeatureFlagReleaseConditions excludeTitle={true} />
</div>
<LemonButton
type="secondary"
status="danger"
className="w-max"
onClick={() => {
setSurveyValue('targeting_flag_filters', undefined)
setSurveyValue('targeting_flag', null)
}}
>
Remove all user properties
</LemonButton>
{id === 'new' && (
<LemonButton
type="secondary"
status="danger"
className="w-max"
onClick={() => {
setSurveyValue('targeting_flag_filters', undefined)
setSurveyValue('targeting_flag', null)
}}
>
Remove all user properties
</LemonButton>
)}
</>
)}
</BindLogic>
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/scenes/surveys/surveyLogic.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,7 @@ export const surveyLogic = kea<surveyLogicType>([
hasTargetingFlag: [
(s) => [s.survey],
(survey): boolean => {
return !!survey.targeting_flag || !!survey.targeting_flag_filters
return !!survey.targeting_flag || !!(survey.id === 'new' && survey.targeting_flag_filters)
},
],
}),
Expand Down
5 changes: 3 additions & 2 deletions frontend/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2074,7 +2074,7 @@ export interface Survey {
linked_flag: FeatureFlagBasicType | null
targeting_flag: FeatureFlagBasicType | null
targeting_flag_filters: Pick<FeatureFlagFilters, 'groups'> | undefined
conditions: { url: string; selector: string; is_headless?: boolean; seenSurveyWaitPeriodInDays?: number } | null
conditions: { url: string; selector: string; is_headless?: boolean } | null
appearance: SurveyAppearance
questions: (BasicSurveyQuestion | LinkSurveyQuestion | RatingSurveyQuestion | MultipleSurveyQuestion)[]
created_at: string
Expand Down Expand Up @@ -2113,7 +2113,7 @@ interface SurveyQuestionBase {
}

export interface BasicSurveyQuestion extends SurveyQuestionBase {
type: SurveyQuestionType.Open
type: SurveyQuestionType.Open | SurveyQuestionType.NPS
}

export interface LinkSurveyQuestion extends SurveyQuestionBase {
Expand All @@ -2140,6 +2140,7 @@ export enum SurveyQuestionType {
Open = 'open',
MultipleChoice = 'multiple_choice',
SingleChoice = 'single_choice',
NPS = 'nps',
Rating = 'rating',
Link = 'link',
}
Expand Down
37 changes: 5 additions & 32 deletions posthog/api/survey.py
Original file line number Diff line number Diff line change
Expand Up @@ -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, allow_null=True)
targeting_flag_filters = serializers.JSONField(required=False, write_only=True)

class Meta:
model = Survey
Expand All @@ -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")
linked_flag_id = data.get("linked_flag_id", None)
if linked_flag_id:
try:
FeatureFlag.objects.get(pk=linked_flag_id)
Expand All @@ -91,38 +91,15 @@ def validate(self, data):

if (
self.context["request"].method == "POST"
and Survey.objects.filter(name=data.get("name"), team_id=self.context["team_id"]).exists()
and Survey.objects.filter(name=data.get("name", None), 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"):
if validated_data.get("targeting_flag_filters", None):
targeting_feature_flag = self._create_new_targeting_flag(
validated_data["name"], validated_data["targeting_flag_filters"]
)
Expand All @@ -134,7 +111,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"):
if validated_data.get("targeting_flag_filters", None):
if instance.targeting_flag:
existing_targeting_flag = instance.targeting_flag
serialized_data_filters = {
Expand All @@ -153,10 +130,6 @@ 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):
Expand Down
4 changes: 2 additions & 2 deletions posthog/api/test/test_feature_flag.py
Original file line number Diff line number Diff line change
Expand Up @@ -961,7 +961,7 @@ def test_getting_flags_is_not_nplus1(self) -> None:
format="json",
).json()

with self.assertNumQueries(FuzzyInt(11, 12)):
with self.assertNumQueries(FuzzyInt(10, 11)):
response = self.client.get(f"/api/projects/{self.team.id}/feature_flags")
self.assertEqual(response.status_code, status.HTTP_200_OK)

Expand All @@ -972,7 +972,7 @@ def test_getting_flags_is_not_nplus1(self) -> None:
format="json",
).json()

with self.assertNumQueries(FuzzyInt(11, 12)):
with self.assertNumQueries(FuzzyInt(10, 11)):
response = self.client.get(f"/api/projects/{self.team.id}/feature_flags")
self.assertEqual(response.status_code, status.HTTP_200_OK)

Expand Down
Loading

0 comments on commit 04eca49

Please sign in to comment.