From 411da6239a21cf569b3ae40cec8059cefa164105 Mon Sep 17 00:00:00 2001 From: Lucas Faria Date: Tue, 18 Feb 2025 21:41:59 -0300 Subject: [PATCH] feat: allow widget surveys to be repeated indefinitely --- frontend/src/scenes/surveys/SurveyEdit.tsx | 19 ++++--- .../scenes/surveys/SurveyRepeatSchedule.tsx | 18 ++++--- frontend/src/scenes/surveys/SurveyView.tsx | 53 ++++++++++++------- .../src/scenes/surveys/Surveys.stories.tsx | 3 ++ frontend/src/scenes/surveys/constants.tsx | 2 + .../src/scenes/surveys/surveyLogic.test.ts | 5 ++ frontend/src/scenes/surveys/surveyLogic.tsx | 10 +--- frontend/src/types.ts | 3 ++ posthog/api/survey.py | 10 ++++ posthog/api/test/test_survey.py | 38 +++++++++++++ posthog/migrations/0671_survey_schedule.py | 21 ++++++++ posthog/migrations/max_migration.txt | 2 +- posthog/models/feedback/survey.py | 10 ++++ 13 files changed, 152 insertions(+), 42 deletions(-) create mode 100644 posthog/migrations/0671_survey_schedule.py diff --git a/frontend/src/scenes/surveys/SurveyEdit.tsx b/frontend/src/scenes/surveys/SurveyEdit.tsx index 1ce5c7754318b..f4e66c8ddc08e 100644 --- a/frontend/src/scenes/surveys/SurveyEdit.tsx +++ b/frontend/src/scenes/surveys/SurveyEdit.tsx @@ -242,7 +242,6 @@ export default function SurveyEdit(): JSX.Element { setSelectedPageIndex, setSelectedSection, setFlagPropertyErrors, - setSchedule, deleteBranchingLogic, } = useActions(surveyLogic) const { surveysMultipleQuestionsAvailable, surveysEventsAvailable, surveysActionsAvailable } = @@ -251,10 +250,6 @@ export default function SurveyEdit(): JSX.Element { const sortedItemIds = survey.questions.map((_, idx) => idx.toString()) const { thankYouMessageDescriptionContentType = null } = survey.appearance ?? {} - if (survey.iteration_count && survey.iteration_count > 0) { - setSchedule('recurring') - } - function onSortEnd({ oldIndex, newIndex }: { oldIndex: number; newIndex: number }): void { function move(arr: SurveyQuestion[], from: number, to: number): SurveyQuestion[] { const clone = [...arr] @@ -301,7 +296,12 @@ export default function SurveyEdit(): JSX.Element {
onChange(SurveyType.Popover)} + onClick={() => { + onChange(SurveyType.Popover) + if (survey.schedule === 'always') { + setSurveyValue('schedule', 'once') + } + }} title="Popover" description="Automatically appears when PostHog JS is installed" value={SurveyType.Popover} @@ -312,7 +312,12 @@ export default function SurveyEdit(): JSX.Element { onChange(SurveyType.API)} + onClick={() => { + onChange(SurveyType.API) + if (survey.schedule === 'always') { + setSurveyValue('schedule', 'once') + } + }} title="API" description="Use the PostHog API to show/hide your survey programmatically" value={SurveyType.API} diff --git a/frontend/src/scenes/surveys/SurveyRepeatSchedule.tsx b/frontend/src/scenes/surveys/SurveyRepeatSchedule.tsx index 3beda89e6511f..951aa93e7b0a9 100644 --- a/frontend/src/scenes/surveys/SurveyRepeatSchedule.tsx +++ b/frontend/src/scenes/surveys/SurveyRepeatSchedule.tsx @@ -6,12 +6,12 @@ import { useActions, useValues } from 'kea' import { LemonField } from 'lib/lemon-ui/LemonField' import { LemonRadio } from 'lib/lemon-ui/LemonRadio' -import { ScheduleType, SurveyEditSection, surveyLogic } from './surveyLogic' +import { SurveyEditSection, surveyLogic } from './surveyLogic' import { surveysLogic } from './surveysLogic' function SurveyIterationOptions(): JSX.Element { - const { showSurveyRepeatSchedule, schedule } = useValues(surveyLogic) - const { setSurveyValue, setSchedule } = useActions(surveyLogic) + const { showSurveyRepeatSchedule, survey } = useValues(surveyLogic) + const { setSurveyValue } = useActions(surveyLogic) const { surveysRecurringScheduleAvailable } = useValues(surveysLogic) const surveysRecurringScheduleDisabledReason = surveysRecurringScheduleAvailable @@ -22,10 +22,10 @@ function SurveyIterationOptions(): JSX.Element { <> { - setSchedule(newValue as ScheduleType) - if (newValue === 'once') { + setSurveyValue('schedule', newValue) + if (newValue === 'once' || newValue === 'always') { setSurveyValue('iteration_count', 0) setSurveyValue('iteration_frequency_days', 0) } else if (newValue === 'recurring') { @@ -45,6 +45,12 @@ function SurveyIterationOptions(): JSX.Element { 'data-attr': 'survey-iteration-frequency-days', disabledReason: surveysRecurringScheduleDisabledReason, }, + { + value: 'always', + label: 'All the time', + 'data-attr': 'survey-iteration-frequency-days', + disabledReason: survey.type !== 'widget' ? 'Only available for widget surveys' : undefined, + }, ]} /> diff --git a/frontend/src/scenes/surveys/SurveyView.tsx b/frontend/src/scenes/surveys/SurveyView.tsx index aadba754451e8..b9002cc0985e8 100644 --- a/frontend/src/scenes/surveys/SurveyView.tsx +++ b/frontend/src/scenes/surveys/SurveyView.tsx @@ -61,6 +61,37 @@ function SurveyResultsFilters(): JSX.Element { ) } +function SurveySchedule(): JSX.Element { + const { survey } = useValues(surveyLogic) + if (survey.schedule === 'recurring' && survey.iteration_count && survey.iteration_frequency_days) { + return ( + <> + Schedule + + Repeats every {survey.iteration_frequency_days}{' '} + {pluralize(survey.iteration_frequency_days, 'day', 'days', false)}, {survey.iteration_count}{' '} + {pluralize(survey.iteration_count, 'time', 'times', false)} + + + ) + } + + if (survey.schedule === 'once') { + return ( + <> + Schedule + Once + + ) + } + + return ( + <> + Schedule + Always + + ) +} export function SurveyView({ id }: { id: string }): JSX.Element { const { survey, surveyLoading, selectedPageIndex, targetingFlagFilters } = useValues(surveyLogic) const { @@ -349,25 +380,9 @@ export function SurveyView({ id }: { id: string }): JSX.Element { )}
- {survey.iteration_count && - survey.iteration_frequency_days && - survey.iteration_count > 0 && - survey.iteration_frequency_days > 0 ? ( -
- Schedule - - Repeats every {survey.iteration_frequency_days}{' '} - {pluralize( - survey.iteration_frequency_days, - 'day', - 'days', - false - )} - , {survey.iteration_count}{' '} - {pluralize(survey.iteration_count, 'time', 'times', false)} - -
- ) : null} +
+ +
{surveyUsesLimit && ( <> diff --git a/frontend/src/scenes/surveys/Surveys.stories.tsx b/frontend/src/scenes/surveys/Surveys.stories.tsx index 624c993b3e559..196c36be1a154 100644 --- a/frontend/src/scenes/surveys/Surveys.stories.tsx +++ b/frontend/src/scenes/surveys/Surveys.stories.tsx @@ -46,6 +46,7 @@ const MOCK_BASIC_SURVEY: Survey = { responses_limit: null, iteration_count: null, iteration_frequency_days: null, + schedule: 'once', } const MOCK_SURVEY_WITH_MULTIPLE_OPTIONS: Survey = { @@ -86,6 +87,7 @@ const MOCK_SURVEY_WITH_MULTIPLE_OPTIONS: Survey = { responses_limit: null, iteration_count: null, iteration_frequency_days: null, + schedule: 'once', } const MOCK_SURVEY_WITH_RELEASE_CONS: Survey = { @@ -164,6 +166,7 @@ const MOCK_SURVEY_WITH_RELEASE_CONS: Survey = { responses_limit: null, iteration_count: null, iteration_frequency_days: null, + schedule: 'once', } const MOCK_SURVEY_SHOWN = { diff --git a/frontend/src/scenes/surveys/constants.tsx b/frontend/src/scenes/surveys/constants.tsx index 36fa1c7434254..f7a0bc9e5ccfd 100644 --- a/frontend/src/scenes/surveys/constants.tsx +++ b/frontend/src/scenes/surveys/constants.tsx @@ -151,6 +151,7 @@ export interface NewSurvey | 'response_sampling_interval_type' | 'response_sampling_interval' | 'response_sampling_limit' + | 'schedule' > { id: 'new' linked_flag_id: number | null @@ -160,6 +161,7 @@ export const NEW_SURVEY: NewSurvey = { id: 'new', name: '', description: '', + schedule: 'once', questions: [ { type: SurveyQuestionType.Open, diff --git a/frontend/src/scenes/surveys/surveyLogic.test.ts b/frontend/src/scenes/surveys/surveyLogic.test.ts index 20e1e3b8a25f9..70df9f4d51d35 100644 --- a/frontend/src/scenes/surveys/surveyLogic.test.ts +++ b/frontend/src/scenes/surveys/surveyLogic.test.ts @@ -61,6 +61,7 @@ const MULTIPLE_CHOICE_SURVEY: Survey = { responses_limit: null, iteration_count: null, iteration_frequency_days: null, + schedule: 'once', } const SINGLE_CHOICE_SURVEY: Survey = { @@ -109,6 +110,7 @@ const SINGLE_CHOICE_SURVEY: Survey = { responses_limit: null, iteration_count: null, iteration_frequency_days: null, + schedule: 'once', } const MULTIPLE_CHOICE_SURVEY_WITH_OPEN_CHOICE: Survey = { @@ -158,6 +160,7 @@ const MULTIPLE_CHOICE_SURVEY_WITH_OPEN_CHOICE: Survey = { responses_limit: null, iteration_count: null, iteration_frequency_days: null, + schedule: 'once', } const SINGLE_CHOICE_SURVEY_WITH_OPEN_CHOICE: Survey = { @@ -207,6 +210,7 @@ const SINGLE_CHOICE_SURVEY_WITH_OPEN_CHOICE: Survey = { responses_limit: null, iteration_count: null, iteration_frequency_days: null, + schedule: 'once', } describe('multiple choice survey logic', () => { @@ -455,6 +459,7 @@ describe('set response-based survey branching', () => { archived: false, targeting_flag_filters: undefined, responses_limit: null, + schedule: 'once', } describe('main', () => { diff --git a/frontend/src/scenes/surveys/surveyLogic.tsx b/frontend/src/scenes/surveys/surveyLogic.tsx index 5f63829f80374..c67651d5f6f07 100644 --- a/frontend/src/scenes/surveys/surveyLogic.tsx +++ b/frontend/src/scenes/surveys/surveyLogic.tsx @@ -133,7 +133,6 @@ export interface QuestionResultsReady { } export type DataCollectionType = 'until_stopped' | 'until_limit' | 'until_adaptive_limit' -export type ScheduleType = 'once' | 'recurring' export interface SurveyDateRange { date_from: string | null @@ -219,7 +218,6 @@ export const surveyLogic = kea([ setSurveyTemplateValues: (template: any) => ({ template }), setSelectedPageIndex: (idx: number | null) => ({ idx }), setSelectedSection: (section: SurveyEditSection | null) => ({ section }), - setSchedule: (schedule: ScheduleType) => ({ schedule }), resetTargeting: true, resetSurveyAdaptiveSampling: true, resetSurveyResponseLimits: true, @@ -1012,12 +1010,6 @@ export const surveyLogic = kea([ setWritingHTMLDescription: (_, { writingHTML }) => writingHTML, }, ], - schedule: [ - 'once', - { - setSchedule: (_, { schedule }) => schedule, - }, - ], flagPropertyErrors: [ null as any, { @@ -1107,7 +1099,7 @@ export const surveyLogic = kea([ return survey.questions.length > 1 }, ], - showSurveyRepeatSchedule: [(s) => [s.schedule], (schedule: ScheduleType) => schedule == 'recurring'], + showSurveyRepeatSchedule: [(s) => [s.survey], (survey: Survey) => survey.schedule === 'recurring'], descriptionContentType: [ (s) => [s.survey], (survey: Survey) => (questionIndex: number) => { diff --git a/frontend/src/types.ts b/frontend/src/types.ts index 115a39b6e7583..c71bcf479c737 100644 --- a/frontend/src/types.ts +++ b/frontend/src/types.ts @@ -2773,12 +2773,15 @@ export interface SetInsightOptions { fromPersistentApi?: boolean } +export type SurveySchedule = 'once' | 'recurring' | 'always' + export interface Survey { /** UUID */ id: string name: string type: SurveyType description: string + schedule: SurveySchedule linked_flag_id: number | null linked_flag: FeatureFlagBasicType | null targeting_flag: FeatureFlagBasicType | null diff --git a/posthog/api/survey.py b/posthog/api/survey.py index 6ce7a004ef3aa..12ac03b2410ea 100644 --- a/posthog/api/survey.py +++ b/posthog/api/survey.py @@ -64,6 +64,7 @@ class SurveySerializer(serializers.ModelSerializer): iteration_count = serializers.IntegerField( required=False, allow_null=True, max_value=MAX_ITERATION_COUNT, min_value=0 ) + schedule = serializers.CharField(required=False, allow_null=True) def get_feature_flag_keys(self, survey: Survey) -> list: return [ @@ -86,6 +87,7 @@ class Meta: "name", "description", "type", + "schedule", "linked_flag", "linked_flag_id", "targeting_flag", @@ -136,6 +138,7 @@ class SurveySerializerCreateUpdateOnly(serializers.ModelSerializer): iteration_count = serializers.IntegerField( required=False, allow_null=True, max_value=MAX_ITERATION_COUNT, min_value=0 ) + schedule = serializers.CharField(required=False, allow_null=True) class Meta: model = Survey @@ -144,6 +147,7 @@ class Meta: "name", "description", "type", + "schedule", "linked_flag", "linked_flag_id", "targeting_flag_id", @@ -297,6 +301,11 @@ def validate_questions(self, value): return cleaned_questions + def validate_schedule(self, value): + if value is not None and value not in ["once", "recurring", "always"]: + raise serializers.ValidationError("Schedule must be one of: once, recurring, always") + return value + def validate(self, data): linked_flag_id = data.get("linked_flag_id") if linked_flag_id: @@ -851,6 +860,7 @@ class Meta: # since they were using it as a way to store sensitive information. Given that we don't ever use # that field to render the survey, we can safely remove it from the API response. "type", + "schedule", "linked_flag_key", "targeting_flag_key", "internal_targeting_flag_key", diff --git a/posthog/api/test/test_survey.py b/posthog/api/test/test_survey.py index 4bf1904a8ae6d..a60413669ca25 100644 --- a/posthog/api/test/test_survey.py +++ b/posthog/api/test/test_survey.py @@ -1551,6 +1551,44 @@ def _assert_survey_activity(self, expected): activity = self.client.get(f"/api/projects/{self.team.id}/surveys/activity").json() self.assertEqual(activity["results"], expected) + def test_validate_schedule_on_create(self): + response = self.client.post( + f"/api/projects/{self.team.id}/surveys/", + data={ + "name": "survey with invalid schedule", + "type": "popover", + "schedule": "invalid_value", + "questions": [ + { + "type": "open", + "question": "What's a survey?", + } + ], + }, + format="json", + ) + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert response.json()["detail"] == "Schedule must be one of: once, recurring, always" + + def test_validate_schedule_on_update(self): + survey = Survey.objects.create( + team=self.team, + created_by=self.user, + name="survey to update", + type="popover", + questions=[{"type": "open", "question": "Why's a hedgehog?"}], + ) + + response = self.client.patch( + f"/api/projects/{self.team.id}/surveys/{survey.id}/", + data={ + "schedule": "invalid_value", + }, + format="json", + ) + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert response.json()["detail"] == "Schedule must be one of: once, recurring, always" + class TestMultipleChoiceQuestions(APIBaseTest): def test_create_survey_has_open_choice(self): diff --git a/posthog/migrations/0671_survey_schedule.py b/posthog/migrations/0671_survey_schedule.py new file mode 100644 index 0000000000000..48e4139ec9cee --- /dev/null +++ b/posthog/migrations/0671_survey_schedule.py @@ -0,0 +1,21 @@ +# Generated by Django 4.2.18 on 2025-02-19 00:40 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("posthog", "0670_querytabstate_querytabstate_unique_team_created_by"), + ] + + operations = [ + migrations.AddField( + model_name="survey", + name="schedule", + field=models.CharField( + choices=[("once", "once"), ("recurring", "recurring"), ("always", "always")], + default="once", + max_length=40, + ), + ), + ] diff --git a/posthog/migrations/max_migration.txt b/posthog/migrations/max_migration.txt index 63f7835f981c9..580b0f75c625f 100644 --- a/posthog/migrations/max_migration.txt +++ b/posthog/migrations/max_migration.txt @@ -1 +1 @@ -0670_querytabstate_querytabstate_unique_team_created_by +0671_survey_schedule diff --git a/posthog/models/feedback/survey.py b/posthog/models/feedback/survey.py index 6fda292c89217..cf9c9dbbefa51 100644 --- a/posthog/models/feedback/survey.py +++ b/posthog/models/feedback/survey.py @@ -32,6 +32,11 @@ class SurveySamplingIntervalType(models.TextChoices): WEEK = "week", "week" MONTH = "month", "month" + class Schedule(models.TextChoices): + ONCE = "once", "once" + RECURRING = "recurring", "recurring" + ALWAYS = "always", "always" + class Meta: constraints = [models.UniqueConstraint(fields=["team", "name"], name="unique survey name for team")] @@ -43,6 +48,11 @@ class Meta: ) name = models.CharField(max_length=400) description = models.TextField(blank=True) + schedule = models.CharField( + max_length=40, + choices=Schedule.choices, + default=Schedule.ONCE, + ) linked_flag = models.ForeignKey( "posthog.FeatureFlag", null=True,