feat(surveys): allow to stop surveys once reached enough responses #21528

nikitaevg · 2024-04-14T16:47:50Z

Problem

Fixes #21179

Changes

It allows a user to set responses limits for surveys on frontend
It creates a periodic task, which stops surveys which reached their limits

Does this work well for both Cloud and self-hosted?

Yes

How did you test this code?

Manually:

The survey wait period field improvement
1. The number field is cleared on unchecking the checkbox
2. One can clear the field manually now
The responses limit field can be set and is saved (screencast)
The responses limit is used (screencast, I'm waiting for the job invocation on it for 1 minute)

posthog/tasks/scheduled.py

nikitaevg · 2024-04-14T16:53:10Z

frontend/src/scenes/surveys/SurveyEdit.tsx

@@ -516,13 +516,18 @@ export default function SurveyEdit(): JSX.Element {
                                                                    type="number"
                                                                    size="small"
                                                                    min={0}
-                                                                    value={value?.seenSurveyWaitPeriodInDays}
+                                                                    value={value?.seenSurveyWaitPeriodInDays || NaN}


This cleans the input field when the value is null

nikitaevg · 2024-04-14T16:53:23Z

frontend/src/scenes/surveys/SurveyEdit.tsx

                                                                    onChange={(val) => {
                                                                        if (val !== undefined && val > 0) {
                                                                            onChange({
                                                                                ...value,
                                                                                seenSurveyWaitPeriodInDays: val,
                                                                            })
+                                                                        } else {


This allows one to actually clean the input field

posthog/tasks/stop_surveys_reached_target.py

neilkakkar

Hey @nikitaevg - excellent work so far! Great job improving the survey wait period component. Some comments on things to fix.

Also, we want some indication on the UI when the survey is running: A dismissible info banner saying the survey will be stopped when X responses are reached.

posthog/tasks/scheduled.py

posthog/tasks/stop_surveys_reached_target.py

frontend/src/scenes/surveys/SurveyEdit.tsx

nikitaevg · 2024-04-21T17:43:00Z

Thanks for the comments @neilkakkar

Also, we want some indication on the UI when the survey is running: A dismissible info banner saying the survey will be stopped when X responses are reached.

Yeah, it makes sense. I'm not sure if it should be dismissible though. WDYT about this place at the bottom, in the Overview? IMO it's pretty suitable there.

I see the storybook tests fail here. I tried updating the screenshots, but storybook fails for chromium on my laptop with an error page.waitForLoadState: Timeout 10000ms exceeded. for all tests, you can see the command with the full error here. If you know why this might happen (assuming Posthog is up and running locally), please let me know. But I looked into it for only 10m or so, I'll give it another try soon. Upd: hmm, it fails on waiting for the "networkidle" state.

frontend/src/scenes/surveys/SurveyEdit.tsx

neilkakkar · 2024-04-22T09:10:03Z

nice, agree not dismissible like you have is much better 👍 . I think I was thinking of dismissible banner after the celery task runs and ends the survey. But we don't have a good way to detect this currently. Hmm.

neilkakkar · 2024-04-22T09:16:49Z

Haven't seen that storybook error before, so unsure, but for the CI failure, it seems like updating snapshots from a fork PR isn't possible, hmm, let me try pushing to see if that makes a difference.

posthog/tasks/test/test_stop_surveys_reached_target.py

neilkakkar · 2024-04-22T11:23:39Z

You can copy the snapshot updates from my PR above if you prefer, or I can merge your changes into that PR and merge that. Upto you :)

Co-authored-by: Neil Kakkar <neilkakkar@gmail.com>

nikitaevg · 2024-04-23T19:40:34Z

nice, agree not dismissible like you have is much better 👍 . I think I was thinking of dismissible banner after the celery task runs and ends the survey. But we don't have a good way to detect this currently. Hmm.

I agree it would be nice. Let's see if users get confused about this and if so I could add a banner. My current idea is that we would need to persist the responses_limit until the survey is relaunched, so that when a user sees a finished survey we could determine if it was finished due to the responses_limit and show such a banner. That would complicate the solution in a sense that responses_limit should now be updated in the survey launch.

You can copy the snapshot updates from my PR above if you prefer, or I can merge your changes into that PR and merge that. Upto you :)

Thank you! Will transfer the screenshots to my PR once it's ready to be submitted

neilkakkar · 2024-04-23T19:44:18Z

Seriously great work here, I'm impressed how you managed to sort this really big chunk of work mostly autonomously, and had great suggestions on how to do things 🚀 🙌

nikitaevg · 2024-04-23T19:45:35Z

Seriously great work here, I'm impressed how you managed to sort this really big chunk of work mostly autonomously, and had great suggestions on how to do things 🚀 🙌

Thank you:) I'm pleased to hear that!

nikitaevg · 2024-04-24T19:24:49Z

@neilkakkar I added a cypress test and transferred the screenshots, could you please have another look?

neilkakkar

looks good thanks!

* master: (28 commits) fix: refactor update animation to do less (#21901) fix(plugin-server): add time component to person.force_upgrade (#21899) fix(dashboards): Indicate 404/400/500 properly (#21853) fix: recording exports (#21900) feat: Update SLAs, remove GitHub buttons (#21818) chore: Removed feature flag for heatmaps (#21886) fix(data-warehouse): Added an error message for empty data source files (#21843) feat(surveys): allow to stop surveys once reached enough responses (#21528) fix: patch missing pause method in rrweb (#21898) fix(hogql): make funnels fast again (#21890) fix: Explicitly call out readonly user fields (#21889) chore(dev): Allow mypy to run in VS Code extension (#21891) feat: Small heatmap UI fixes (#21849) feat: Improve Heatmaps UI and notices (#21887) fix: Redirection to keep query params (#21881) fix: hot path is cooler now (#21888) feat(data-warehouse): Moved the table model creation to the data pipeline activity (#21817) fix: snapshot response timings (#21885) chore(deps): Update posthog-js to 1.130.0 (#21884) feat(debug): tabs + better view for non-hogql-query nodes (#21867) ...

nikitaevg added 2 commits April 14, 2024 17:17

feat(surveys): allow to stop surveys once reached enough responses

0bc6e84

fix targeting

eb7367c

nikitaevg commented Apr 14, 2024

View reviewed changes

posthog/tasks/stop_surveys_reached_target.py Outdated Show resolved Hide resolved

nikitaevg added 2 commits April 14, 2024 18:04

polish

6d46d59

fix type errors

1ec1b50

nikitaevg mentioned this pull request Apr 14, 2024

Stop a survey once enough responses received #21179

Closed

neilkakkar reviewed Apr 15, 2024

View reviewed changes

nikitaevg added 7 commits April 21, 2024 16:14

make the periodic job better, add tests for it

4bd29e7

Merge remote-tracking branch 'upstream/master' into 21179-stop-survey

709ce86

resolve the conflict, revert some files

ef47dd9

revert files

a2fcfc9

fix types problems, add info about response limit

afd068c

fix tests

6d25e56

make the responses limit bold

ca7a81c

neilkakkar reviewed Apr 22, 2024

View reviewed changes

frontend/src/scenes/surveys/SurveyEdit.tsx Outdated Show resolved Hide resolved

Merge branch 'master' into 21179-stop-survey

dc4eec9

neilkakkar reviewed Apr 22, 2024

View reviewed changes

posthog/tasks/test/test_stop_surveys_reached_target.py Outdated Show resolved Hide resolved

neilkakkar mentioned this pull request Apr 22, 2024

feat(surveys): allow to stop surveys once reached enough responses #21716

Closed

nikitaevg and others added 2 commits April 23, 2024 20:38

Update frontend/src/scenes/surveys/SurveyEdit.tsx

70b9fef

Co-authored-by: Neil Kakkar <neilkakkar@gmail.com>

address the PR comments

e6abdee

neilkakkar mentioned this pull request Apr 23, 2024

Survey responses count is inefficient #21775

Closed

fix typing error

d707e02

nikitaevg added 5 commits April 24, 2024 19:01

add a frontend test and snapshots

34902b1

Merge remote-tracking branch 'upstream/master' into 21179-stop-survey

6f4ac1a

add a forgotten migration

e3ed0ee

revert formatted files

a6a7435

add a forgotten file

23043e6

nikitaevg added 2 commits April 25, 2024 19:17

Merge remote-tracking branch 'upstream/master' into 21179-stop-survey

68b6a71

why I always forget to save this file

ff42287

nikitaevg requested a review from neilkakkar April 25, 2024 18:37

nikitaevg and others added 3 commits April 25, 2024 19:39

replace typing.List and typing.Dict

6c1cacd

fix types, add snapshot, clean test

9aedb05

stabilise uuids

78f5e02

neilkakkar approved these changes Apr 26, 2024

View reviewed changes

neilkakkar enabled auto-merge (squash) April 26, 2024 13:24

neilkakkar merged commit 50360e7 into PostHog:master Apr 26, 2024
100 of 101 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(surveys): allow to stop surveys once reached enough responses #21528

feat(surveys): allow to stop surveys once reached enough responses #21528

nikitaevg commented Apr 14, 2024 •

edited

Loading

nikitaevg Apr 14, 2024

nikitaevg Apr 14, 2024

neilkakkar left a comment •

edited

Loading

nikitaevg commented Apr 21, 2024 •

edited

Loading

neilkakkar commented Apr 22, 2024

neilkakkar commented Apr 22, 2024

neilkakkar commented Apr 22, 2024

nikitaevg commented Apr 23, 2024

neilkakkar commented Apr 23, 2024

nikitaevg commented Apr 23, 2024

nikitaevg commented Apr 24, 2024

neilkakkar left a comment

feat(surveys): allow to stop surveys once reached enough responses #21528

feat(surveys): allow to stop surveys once reached enough responses #21528

Conversation

nikitaevg commented Apr 14, 2024 • edited Loading

Problem

Changes

Does this work well for both Cloud and self-hosted?

How did you test this code?

nikitaevg Apr 14, 2024

Choose a reason for hiding this comment

nikitaevg Apr 14, 2024

Choose a reason for hiding this comment

neilkakkar left a comment • edited Loading

Choose a reason for hiding this comment

nikitaevg commented Apr 21, 2024 • edited Loading

neilkakkar commented Apr 22, 2024

neilkakkar commented Apr 22, 2024

neilkakkar commented Apr 22, 2024

nikitaevg commented Apr 23, 2024

neilkakkar commented Apr 23, 2024

nikitaevg commented Apr 23, 2024

nikitaevg commented Apr 24, 2024

neilkakkar left a comment

Choose a reason for hiding this comment

nikitaevg commented Apr 14, 2024 •

edited

Loading

neilkakkar left a comment •

edited

Loading

nikitaevg commented Apr 21, 2024 •

edited

Loading