Skip to content

Commit

Permalink
feat: allow widget surveys to be repeated indefinitely
Browse files Browse the repository at this point in the history
  • Loading branch information
lucasheriques committed Feb 19, 2025
1 parent 925b70c commit 411da62
Show file tree
Hide file tree
Showing 13 changed files with 152 additions and 42 deletions.
19 changes: 12 additions & 7 deletions frontend/src/scenes/surveys/SurveyEdit.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,6 @@ export default function SurveyEdit(): JSX.Element {
setSelectedPageIndex,
setSelectedSection,
setFlagPropertyErrors,
setSchedule,
deleteBranchingLogic,
} = useActions(surveyLogic)
const { surveysMultipleQuestionsAvailable, surveysEventsAvailable, surveysActionsAvailable } =
Expand All @@ -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]
Expand Down Expand Up @@ -301,7 +296,12 @@ export default function SurveyEdit(): JSX.Element {
<div className="flex gap-4">
<PresentationTypeCard
active={value === SurveyType.Popover}
onClick={() => 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}
Expand All @@ -312,7 +312,12 @@ export default function SurveyEdit(): JSX.Element {
</PresentationTypeCard>
<PresentationTypeCard
active={value === SurveyType.API}
onClick={() => 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}
Expand Down
18 changes: 12 additions & 6 deletions frontend/src/scenes/surveys/SurveyRepeatSchedule.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -22,10 +22,10 @@ function SurveyIterationOptions(): JSX.Element {
<>
<LemonField.Pure>
<LemonRadio
value={schedule}
value={survey.schedule}
onChange={(newValue) => {
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') {
Expand All @@ -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,
},
]}
/>
</LemonField.Pure>
Expand Down
53 changes: 34 additions & 19 deletions frontend/src/scenes/surveys/SurveyView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
<>
<span className="card-secondary">Schedule</span>
<span>
Repeats every {survey.iteration_frequency_days}{' '}
{pluralize(survey.iteration_frequency_days, 'day', 'days', false)}, {survey.iteration_count}{' '}
{pluralize(survey.iteration_count, 'time', 'times', false)}
</span>
</>
)
}

if (survey.schedule === 'once') {
return (
<>
<span className="card-secondary">Schedule</span>
<span>Once</span>
</>
)
}

return (
<>
<span className="card-secondary">Schedule</span>
<span>Always</span>
</>
)
}
export function SurveyView({ id }: { id: string }): JSX.Element {
const { survey, surveyLoading, selectedPageIndex, targetingFlagFilters } = useValues(surveyLogic)
const {
Expand Down Expand Up @@ -349,25 +380,9 @@ export function SurveyView({ id }: { id: string }): JSX.Element {
)}
</div>
<div className="flex flex-row gap-8">
{survey.iteration_count &&
survey.iteration_frequency_days &&
survey.iteration_count > 0 &&
survey.iteration_frequency_days > 0 ? (
<div className="flex flex-col">
<span className="mt-4 card-secondary">Schedule</span>
<span>
Repeats every {survey.iteration_frequency_days}{' '}
{pluralize(
survey.iteration_frequency_days,
'day',
'days',
false
)}
, {survey.iteration_count}{' '}
{pluralize(survey.iteration_count, 'time', 'times', false)}
</span>
</div>
) : null}
<div className="flex flex-col mt-4">
<SurveySchedule />
</div>
</div>
{surveyUsesLimit && (
<>
Expand Down
3 changes: 3 additions & 0 deletions frontend/src/scenes/surveys/Surveys.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down Expand Up @@ -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 = {
Expand Down Expand Up @@ -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 = {
Expand Down
2 changes: 2 additions & 0 deletions frontend/src/scenes/surveys/constants.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -160,6 +161,7 @@ export const NEW_SURVEY: NewSurvey = {
id: 'new',
name: '',
description: '',
schedule: 'once',
questions: [
{
type: SurveyQuestionType.Open,
Expand Down
5 changes: 5 additions & 0 deletions frontend/src/scenes/surveys/surveyLogic.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down Expand Up @@ -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 = {
Expand Down Expand Up @@ -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 = {
Expand Down Expand Up @@ -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', () => {
Expand Down Expand Up @@ -455,6 +459,7 @@ describe('set response-based survey branching', () => {
archived: false,
targeting_flag_filters: undefined,
responses_limit: null,
schedule: 'once',
}

describe('main', () => {
Expand Down
10 changes: 1 addition & 9 deletions frontend/src/scenes/surveys/surveyLogic.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -219,7 +218,6 @@ export const surveyLogic = kea<surveyLogicType>([
setSurveyTemplateValues: (template: any) => ({ template }),
setSelectedPageIndex: (idx: number | null) => ({ idx }),
setSelectedSection: (section: SurveyEditSection | null) => ({ section }),
setSchedule: (schedule: ScheduleType) => ({ schedule }),
resetTargeting: true,
resetSurveyAdaptiveSampling: true,
resetSurveyResponseLimits: true,
Expand Down Expand Up @@ -1012,12 +1010,6 @@ export const surveyLogic = kea<surveyLogicType>([
setWritingHTMLDescription: (_, { writingHTML }) => writingHTML,
},
],
schedule: [
'once',
{
setSchedule: (_, { schedule }) => schedule,
},
],
flagPropertyErrors: [
null as any,
{
Expand Down Expand Up @@ -1107,7 +1099,7 @@ export const surveyLogic = kea<surveyLogicType>([
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) => {
Expand Down
3 changes: 3 additions & 0 deletions frontend/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 10 additions & 0 deletions posthog/api/survey.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 [
Expand All @@ -86,6 +87,7 @@ class Meta:
"name",
"description",
"type",
"schedule",
"linked_flag",
"linked_flag_id",
"targeting_flag",
Expand Down Expand Up @@ -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
Expand All @@ -144,6 +147,7 @@ class Meta:
"name",
"description",
"type",
"schedule",
"linked_flag",
"linked_flag_id",
"targeting_flag_id",
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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",
Expand Down
38 changes: 38 additions & 0 deletions posthog/api/test/test_survey.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
Loading

0 comments on commit 411da62

Please sign in to comment.