From d8776382b653ee890d30160fd0bd1fa9fd27ff02 Mon Sep 17 00:00:00 2001 From: Phani Raj Date: Mon, 18 Nov 2024 14:53:59 -0600 Subject: [PATCH] feat(surveys): Allow user to edit survey response adaptive limits (#26223) feat(surveys): Allow user to edit survey response adaptive limits Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Daniel Bachhuber --- cypress/e2e/surveys.cy.ts | 3 +- frontend/src/lib/constants.tsx | 1 + frontend/src/scenes/surveys/SurveyEdit.tsx | 212 +++++++++++++++--- frontend/src/scenes/surveys/SurveyView.tsx | 14 +- frontend/src/scenes/surveys/constants.tsx | 4 + frontend/src/scenes/surveys/surveyLogic.tsx | 44 ++++ frontend/src/types.ts | 5 + .../test_update_survey_adaptive_sampling.py | 13 ++ .../tasks/update_survey_adaptive_sampling.py | 8 +- 9 files changed, 264 insertions(+), 40 deletions(-) diff --git a/cypress/e2e/surveys.cy.ts b/cypress/e2e/surveys.cy.ts index 28082edb3de..1cccfb545fc 100644 --- a/cypress/e2e/surveys.cy.ts +++ b/cypress/e2e/surveys.cy.ts @@ -269,6 +269,7 @@ describe('Surveys', () => { // Set responses limit cy.get('.LemonCollapsePanel').contains('Completion conditions').click() + cy.get('[data-attr=survey-collection-until-limit]').first().click() cy.get('[data-attr=survey-responses-limit-input]').focus().type('228').click() // Save the survey @@ -276,7 +277,7 @@ describe('Surveys', () => { cy.get('button[data-attr="launch-survey"]').should('have.text', 'Launch') cy.reload() - cy.contains('The survey will be stopped once 228 responses are received.').should('be.visible') + cy.contains('The survey will be stopped once 100228 responses are received.').should('be.visible') }) it('creates a new survey with branching logic', () => { diff --git a/frontend/src/lib/constants.tsx b/frontend/src/lib/constants.tsx index 881e9691b4f..b3edb1d6971 100644 --- a/frontend/src/lib/constants.tsx +++ b/frontend/src/lib/constants.tsx @@ -169,6 +169,7 @@ export const FEATURE_FLAGS = { SURVEYS_EVENTS: 'surveys-events', // owner: #team-feature-success SURVEYS_ACTIONS: 'surveys-actions', // owner: #team-feature-success SURVEYS_RECURRING: 'surveys-recurring', // owner: #team-feature-success + SURVEYS_ADAPTIVE_COLLECTION: 'surveys-recurring', // owner: #team-feature-success YEAR_IN_HOG: 'year-in-hog', // owner: #team-replay SESSION_REPLAY_EXPORT_MOBILE_DATA: 'session-replay-export-mobile-data', // owner: #team-replay DISCUSSIONS: 'discussions', // owner: #team-replay diff --git a/frontend/src/scenes/surveys/SurveyEdit.tsx b/frontend/src/scenes/surveys/SurveyEdit.tsx index 89f6eef2c22..28a2b8d9205 100644 --- a/frontend/src/scenes/surveys/SurveyEdit.tsx +++ b/frontend/src/scenes/surveys/SurveyEdit.tsx @@ -6,6 +6,7 @@ import { IconInfo } from '@posthog/icons' import { IconLock, IconPlus, IconTrash } from '@posthog/icons' import { LemonButton, + LemonCalendarSelect, LemonCheckbox, LemonCollapse, LemonDialog, @@ -15,17 +16,21 @@ import { LemonTag, LemonTextArea, Link, + Popover, } from '@posthog/lemon-ui' import { BindLogic, useActions, useValues } from 'kea' import { EventSelect } from 'lib/components/EventSelect/EventSelect' import { FlagSelector } from 'lib/components/FlagSelector' import { TaxonomicFilterGroupType } from 'lib/components/TaxonomicFilter/types' import { FEATURE_FLAGS } from 'lib/constants' +import { dayjs } from 'lib/dayjs' import { IconCancel } from 'lib/lemon-ui/icons' import { LemonField } from 'lib/lemon-ui/LemonField' import { LemonRadio } from 'lib/lemon-ui/LemonRadio' import { Tooltip } from 'lib/lemon-ui/Tooltip' import { featureFlagLogic as enabledFeaturesLogic } from 'lib/logic/featureFlagLogic' +import { formatDate } from 'lib/utils' +import { useMemo, useState } from 'react' import { featureFlagLogic } from 'scenes/feature-flags/featureFlagLogic' import { FeatureFlagReleaseConditions } from 'scenes/feature-flags/FeatureFlagReleaseConditions' @@ -62,15 +67,21 @@ export default function SurveyEdit(): JSX.Element { schedule, hasBranchingLogic, surveyRepeatedActivationAvailable, + dataCollectionType, + surveyUsesLimit, + surveyUsesAdaptiveLimit, } = useValues(surveyLogic) const { setSurveyValue, resetTargeting, + resetSurveyResponseLimits, + resetSurveyAdaptiveSampling, setSelectedPageIndex, setSelectedSection, setFlagPropertyErrors, setSchedule, deleteBranchingLogic, + setDataCollectionType, } = useActions(surveyLogic) const { surveysMultipleQuestionsAvailable, @@ -79,11 +90,25 @@ export default function SurveyEdit(): JSX.Element { surveysActionsAvailable, } = useValues(surveysLogic) const { featureFlags } = useValues(enabledFeaturesLogic) + const [visible, setVisible] = useState(false) const sortedItemIds = survey.questions.map((_, idx) => idx.toString()) const { thankYouMessageDescriptionContentType = null } = survey.appearance ?? {} const surveysRecurringScheduleDisabledReason = surveysRecurringScheduleAvailable ? undefined : 'Upgrade your plan to use repeating surveys' + const surveysAdaptiveLimitsDisabledReason = surveysRecurringScheduleAvailable + ? undefined + : 'Upgrade your plan to use an adaptive limit on survey responses' + + useMemo(() => { + if (surveyUsesLimit) { + setDataCollectionType('until_limit') + } else if (surveyUsesAdaptiveLimit) { + setDataCollectionType('until_adaptive_limit') + } else { + setDataCollectionType('until_stopped') + } + }, [surveyUsesLimit, surveyUsesAdaptiveLimit, setDataCollectionType]) if (survey.iteration_count && survey.iteration_count > 0) { setSchedule('recurring') @@ -852,44 +877,157 @@ export default function SurveyEdit(): JSX.Element { header: 'Completion conditions', content: ( <> - - {({ onChange, value }) => { - return ( -
- { - const newResponsesLimit = checked ? 100 : null - onChange(newResponsesLimit) - }} - /> - Stop the survey once - { - if (newValue && newValue > 0) { - onChange(newValue) - } else { - onChange(null) - } - }} - className="w-16" - />{' '} - responses are received. - - - -
- ) - }} - +
+

How long would you like to collect survey responses?

+ + { + if (newValue === 'until_limit') { + resetSurveyAdaptiveSampling() + setSurveyValue('responses_limit', survey.responses_limit || 100) + } else if (newValue === 'until_adaptive_limit') { + resetSurveyResponseLimits() + setSurveyValue( + 'response_sampling_interval', + survey.response_sampling_interval || 1 + ) + setSurveyValue( + 'response_sampling_interval_type', + survey.response_sampling_interval_type || 'month' + ) + setSurveyValue( + 'response_sampling_limit', + survey.response_sampling_limit || 100 + ) + setSurveyValue( + 'response_sampling_start_date', + survey.response_sampling_start_date || dayjs() + ) + } else { + resetSurveyResponseLimits() + resetSurveyAdaptiveSampling() + } + setDataCollectionType(newValue) + }} + options={[ + { + value: 'until_stopped', + label: 'Keep collecting responses until the survey is stopped', + 'data-attr': 'survey-collection-until-stopped', + }, + { + value: 'until_limit', + label: 'Stop displaying the survey after reaching a certain number of completed surveys', + 'data-attr': 'survey-collection-until-limit', + }, + { + value: 'until_adaptive_limit', + label: 'Collect a certain number of surveys per day, week or month', + 'data-attr': 'survey-collection-until-adaptive-limit', + disabledReason: surveysAdaptiveLimitsDisabledReason, + }, + ]} + /> + +
+ {dataCollectionType == 'until_adaptive_limit' && ( + +
+ Starting on{' '} + { + setSurveyValue('response_sampling_start_date', value) + setVisible(false) + }} + showTimeToggle={false} + onClose={() => setVisible(false)} + /> + } + visible={visible} + onClickOutside={() => setVisible(false)} + > + setVisible(!visible)}> + {formatDate(dayjs(survey.response_sampling_start_date || ''))} + + + , capture up to + { + setSurveyValue('response_sampling_limit', newValue) + }} + value={survey.response_sampling_limit || 0} + /> + responses, every + { + setSurveyValue('response_sampling_interval', newValue) + }} + value={survey.response_sampling_interval || 0} + /> + { + setSurveyValue('response_sampling_interval_type', newValue) + }} + options={[ + { value: 'day', label: 'Day(s)' }, + { value: 'week', label: 'Week(s)' }, + { value: 'month', label: 'Month(s)' }, + ]} + /> + + + +
+ + )} + {dataCollectionType == 'until_limit' && ( + + {({ onChange, value }) => { + return ( +
+ Stop the survey once + { + if (newValue && newValue > 0) { + onChange(newValue) + } else { + onChange(null) + } + }} + className="w-16" + />{' '} + responses are received. + + + +
+ ) + }} + + )} {featureFlags[FEATURE_FLAGS.SURVEYS_RECURRING] && ( -
-

How often should we show this survey?

+
+

How often should we show this survey?

) : null}
- {survey.responses_limit && ( + {surveyUsesLimit && ( <> Completion conditions @@ -351,6 +352,17 @@ export function SurveyView({ id }: { id: string }): JSX.Element { )} + {surveyUsesAdaptiveLimit && ( + <> + Completion conditions + + Survey response collection is limited to receive{' '} + {survey.response_sampling_limit} responses every{' '} + {survey.response_sampling_interval}{' '} + {survey.response_sampling_interval_type}(s). + + + )} { id: 'new' linked_flag_id: number | null diff --git a/frontend/src/scenes/surveys/surveyLogic.tsx b/frontend/src/scenes/surveys/surveyLogic.tsx index 528aac6db6e..65345656590 100644 --- a/frontend/src/scenes/surveys/surveyLogic.tsx +++ b/frontend/src/scenes/surveys/surveyLogic.tsx @@ -105,6 +105,7 @@ export interface QuestionResultsReady { [key: string]: boolean } +export type DataCollectionType = 'until_stopped' | 'until_limit' | 'until_adaptive_limit' export type ScheduleType = 'once' | 'recurring' const getResponseField = (i: number): string => (i === 0 ? '$survey_response' : `$survey_response_${i}`) @@ -168,6 +169,9 @@ export const surveyLogic = kea([ nextStep, specificQuestionIndex, }), + setDataCollectionType: (dataCollectionType: DataCollectionType) => ({ + dataCollectionType, + }), resetBranchingForQuestion: (questionIndex) => ({ questionIndex }), deleteBranchingLogic: true, archiveSurvey: true, @@ -178,6 +182,8 @@ export const surveyLogic = kea([ setSchedule: (schedule: ScheduleType) => ({ schedule }), resetTargeting: true, + resetSurveyAdaptiveSampling: true, + resetSurveyResponseLimits: true, setFlagPropertyErrors: (errors: any) => ({ errors }), }), loaders(({ props, actions, values }) => ({ @@ -608,6 +614,19 @@ export const surveyLogic = kea([ loadSurveySuccess: () => { actions.loadSurveyUserStats() }, + resetSurveyResponseLimits: () => { + actions.setSurveyValue('responses_limit', null) + }, + + resetSurveyAdaptiveSampling: () => { + actions.setSurveyValues({ + response_sampling_interval: null, + response_sampling_interval_type: null, + response_sampling_limit: null, + response_sampling_start_date: null, + response_sampling_daily_limits: null, + }) + }, resetTargeting: () => { actions.setSurveyValue('linked_flag_id', NEW_SURVEY.linked_flag_id) actions.setSurveyValue('targeting_flag_filters', NEW_SURVEY.targeting_flag_filters) @@ -647,6 +666,12 @@ export const surveyLogic = kea([ setSurveyMissing: () => true, }, ], + dataCollectionType: [ + 'until_stopped' as DataCollectionType, + { + setDataCollectionType: (_, { dataCollectionType }) => dataCollectionType, + }, + ], survey: [ { ...NEW_SURVEY } as NewSurvey | Survey, @@ -877,6 +902,24 @@ export const surveyLogic = kea([ return !!(survey.start_date && !survey.end_date) }, ], + surveyUsesLimit: [ + (s) => [s.survey], + (survey: Survey): boolean => { + return !!(survey.responses_limit && survey.responses_limit > 0) + }, + ], + surveyUsesAdaptiveLimit: [ + (s) => [s.survey], + (survey: Survey): boolean => { + return !!( + survey.response_sampling_interval && + survey.response_sampling_interval > 0 && + survey.response_sampling_interval_type !== '' && + survey.response_sampling_limit && + survey.response_sampling_limit > 0 + ) + }, + ], surveyShufflingQuestionsAvailable: [ (s) => [s.survey], (survey: Survey): boolean => { @@ -1022,6 +1065,7 @@ export const surveyLogic = kea([ } }, ], + getBranchingDropdownValue: [ (s) => [s.survey], (survey) => (questionIndex: number, question: RatingSurveyQuestion | MultipleSurveyQuestion) => { diff --git a/frontend/src/types.ts b/frontend/src/types.ts index 94f5aae7176..24c25f4df48 100644 --- a/frontend/src/types.ts +++ b/frontend/src/types.ts @@ -2767,6 +2767,11 @@ export interface Survey { iteration_start_dates?: string[] current_iteration?: number | null current_iteration_start_date?: string + response_sampling_start_date?: string | null + response_sampling_interval_type?: string | null + response_sampling_interval?: number | null + response_sampling_limit?: number | null + response_sampling_daily_limits?: string[] | null } export enum SurveyUrlMatchType { diff --git a/posthog/tasks/test/test_update_survey_adaptive_sampling.py b/posthog/tasks/test/test_update_survey_adaptive_sampling.py index cd7964ffe15..91696cf1ce8 100644 --- a/posthog/tasks/test/test_update_survey_adaptive_sampling.py +++ b/posthog/tasks/test/test_update_survey_adaptive_sampling.py @@ -1,3 +1,4 @@ +import json from unittest.mock import patch, MagicMock from datetime import datetime from django.utils import timezone @@ -39,6 +40,18 @@ class TestUpdateSurveyAdaptiveSampling(BaseTest): self.assertEqual(internal_response_sampling_flag.rollout_percentage, 20) mock_get_count.assert_called_once_with(self.survey.id) + @freeze_time("2024-12-21T12:00:00Z") + @patch("posthog.tasks.update_survey_adaptive_sampling._get_survey_responses_count") + def test_updates_rollout_after_interval_is_over(self, mock_get_count: MagicMock) -> None: + mock_get_count.return_value = 50 + update_survey_adaptive_sampling() + internal_response_sampling_flag = FeatureFlag.objects.get(id=self.internal_response_sampling_flag.id) + self.assertEqual(internal_response_sampling_flag.rollout_percentage, 100) + mock_get_count.assert_called_once_with(self.survey.id) + survey = Survey.objects.get(id=self.survey.id) + response_sampling_daily_limits = json.loads(survey.response_sampling_daily_limits) + self.assertEqual(response_sampling_daily_limits[0].get("date"), "2024-12-22") + @freeze_time("2024-12-13T12:00:00Z") @patch("posthog.tasks.update_survey_adaptive_sampling._get_survey_responses_count") def test_no_update_when_limit_reached(self, mock_get_count: MagicMock) -> None: diff --git a/posthog/tasks/update_survey_adaptive_sampling.py b/posthog/tasks/update_survey_adaptive_sampling.py index bdd7d4ed048..bedf79f7f3c 100644 --- a/posthog/tasks/update_survey_adaptive_sampling.py +++ b/posthog/tasks/update_survey_adaptive_sampling.py @@ -1,5 +1,5 @@ import json -from datetime import datetime +from datetime import datetime, timedelta from django.utils.timezone import now from posthog.clickhouse.client import sync_execute @@ -29,6 +29,12 @@ def _update_survey_adaptive_sampling(survey: Survey) -> None: internal_response_sampling_flag.rollout_percentage = today_entry["rollout_percentage"] internal_response_sampling_flag.save() + # this also doubles as a way to check that we're processing the final entry in the current sequence. + if today_entry["rollout_percentage"] == 100: + tomorrow = today_date + timedelta(days=1) + survey.response_sampling_start_date = tomorrow + survey.save(update_fields=["response_sampling_start_date", "response_sampling_daily_limits"]) + def _get_survey_responses_count(survey_id: int) -> int: data = sync_execute(