Repository: PostHog/posthog
Pull request: feat(surveys): allow to stop surveys once reached enough responses
## Problem

Fixes https://github.com/PostHog/posthog/issues/21179

## Changes

1. It allows a user to set responses limits for surveys on frontend
2. 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:
1. The survey wait period field [improvement](https://github.com/PostHog/posthog/assets/6707908/08e4c42a-f863-4630-82ea-7e6545f3818b)
   1. The number field is cleared on unchecking the checkbox
   2. One can clear the field manually now
2. The responses limit field can be set and is saved ([screencast](https://github.com/PostHog/posthog/assets/6707908/293e56bc-26c3-483f-bd56-68b736e48bbe))
3. The responses limit is used ([screencast](https://github.com/PostHog/posthog/assets/6707908/7b76d8bc-17aa-46b5-8e4e-ea1cd6c0772f), I'm waiting for the job invocation on it for 1 minute)


Problem statement:
Stop a survey once enough responses received
## Is your feature request related to a problem?

*Please describe.*

## Describe the solution you'd like



## Describe alternatives you've considered



## Additional context



#### *Thank you* for your feature request – we love each and every one!

Hints from issue discussion:
Hi @jurajmajerik, can I work on this issue?

So this requires setting a goal for a survey. I came up with [this](https://github.com/PostHog/posthog/assets/6707908/193cf0f8-7afa-4f8c-9a82-2c265c2ecb6c) design.

This also requires viewing the goal for the survey, something like [this](https://github.com/PostHog/posthog/assets/6707908/6ba5f1eb-e129-4713-a9b1-94ff4a15d10a).

From the backend pov I don't expect any challenges, but I didn't look into it. I think on every survey response I can check if the response count is above the goal and if I need to stop the survey I stop it.

There's one tricky part I see - if somebody want's to resume the survey once the goal is reached I assume it'd be better to start ignoring the goal, meaning to override the value that represents the "Stop survey on reaching the goal" checkbox from my design.

Also I wonder if you want to show the user that their survey was stopped because it reached the goal? If so I could try to come up with something, but I don't have anything good of the top of my head.

Oh, one more thing - what release cycle do you have? Do I support this first on backend, wait for its release and then support it on frontend?

Please let me know what you think of these suggestions
Hey @nikitaevg! Survey responses are not submitted via the Surveys API - they're simply events sent from end users' frontends via the /capture endpoint. Because of this disconnect, I expect solving this will be more involved. I'm not sure if we support additional logic when submitting events. @neilkakkar?

As for the release cycle, we're pretty flexible. If it's a small change, ideally you would put both frontend and backend changes in the same PR. If you want to split it, implementing the backend first makes more sense.
We wouldn't want to slow down the ingestion pipeline, so indeed this will need to happen elsewhere: Using our celery task poll for number of responses and end the survey when we hit the threshold. This means we can't exactly stop at X, but roughly X (assuming there's no huge spike of responses between two pollings, but this should be ok given its controlled by user input).

So, we'd want the UI to be mindful of this constraint^. On the UI side, I'm not sure the goal makes sense, given they can overshoot it, and then re-enable a survey when the goal is reached, thus effectively ignoring it. A config option for "Stop surveys when X [Input] responses are received" is enough (like the survey wait period setting in targeting) -and agree on this going in another section, but I'm not sure of the name yet, additional settings? Response settings? unclear what else will go in here.

Thank you both for explaining!

> We wouldn't want to slow down the ingestion pipeline, so indeed this will need to happen elsewhere: Using our celery task poll for number of responses and end the survey when we hit the threshold. This means we can't exactly stop at X, but roughly X (assuming there's no huge spike of responses between two pollings, but this should be ok given its controlled by user input).

Yeah, that sounds reasonable. I don't see any existing task suitable for this functionality, so I would introduce a new task, sounds good?

> On the UI side, I'm not sure the goal makes sense, given they can overshoot it, and then re-enable a survey when the goal is reached, thus effectively ignoring it.

I'm a bit concerned that users won't understand why their surveys suddenly stopped, but I will trust your judgement on this.

> A config option for "Stop surveys when X [Input] responses are received" is enough (like the survey wait period setting in targeting) -and agree on this going in another section, but I'm not sure of the name yet, additional settings? Response settings? unclear what else will go in here.

Huh, I haven't seen this wait period. Yeah, it makes sense to design it the same way, thanks! Regarding the naming, maybe "Stop conditions"? @neilkakkar wdyt?

Commit message: fix targeting

Patch to review:
diff --git a/frontend/src/scenes/surveys/SurveyEdit.tsx b/frontend/src/scenes/surveys/SurveyEdit.tsx
index 1121d79be7f3a..85fc6e3e40270 100644
--- a/frontend/src/scenes/surveys/SurveyEdit.tsx
+++ b/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}
                                                                     onChange={(val) => {
                                                                         if (val !== undefined && val > 0) {
                                                                             onChange({
                                                                                 ...value,
                                                                                 seenSurveyWaitPeriodInDays: val,
                                                                             })
+                                                                        } else {
+                                                                            onChange({
+                                                                                ...value,
+                                                                                seenSurveyWaitPeriodInDays: null,
+                                                                            })
                                                                         }
                                                                     }}
                                                                     className="w-16"
@@ -597,6 +602,37 @@ export default function SurveyEdit(): JSX.Element {
                                 </LemonField.Pure>
                             ),
                         },
+                        {
+                            key: SurveyEditSection.CompletionConditions,
+                            header: 'Completion conditions',
+                            content: (
+                                <div className="flex flex-row gap-2 items-center">
+                                    <LemonCheckbox
+                                        checked={!!survey.responses_limit}
+                                        onChange={(checked) => {
+                                            const newValue = checked ? 100 : null
+                                            setSurveyValue('responses_limit', newValue)
+                                        }}
+                                    />
+                                    Stop the survey once received
+                                    <LemonInput
+                                        type="number"
+                                        size="small"
+                                        min={1}
+                                        value={survey.responses_limit || NaN}
+                                        onChange={(value) => {
+                                            if (value && value > 0) {
+                                                setSurveyValue('responses_limit', value)
+                                            } else {
+                                                setSurveyValue('responses_limit', null)
+                                            }
+                                        }}
+                                        className="w-16"
+                                    />{' '}
+                                    responses.
+                                </div>
+                            ),
+                        },
                     ]}
                 />
             </div>
diff --git a/frontend/src/scenes/surveys/constants.tsx b/frontend/src/scenes/surveys/constants.tsx
index c123fed68eb1b..d748a13c4ef26 100644
--- a/frontend/src/scenes/surveys/constants.tsx
+++ b/frontend/src/scenes/surveys/constants.tsx
@@ -118,6 +118,7 @@ export interface NewSurvey
         | 'archived'
         | 'appearance'
         | 'targeting_flag_filters'
+        | 'responses_limit'
     > {
     id: 'new'
     linked_flag_id: number | null
@@ -145,6 +146,7 @@ export const NEW_SURVEY: NewSurvey = {
     conditions: null,
     archived: false,
     appearance: defaultSurveyAppearance,
+    responses_limit: null,
 }
 
 export enum SurveyTemplateType {
diff --git a/frontend/src/scenes/surveys/surveyLogic.tsx b/frontend/src/scenes/surveys/surveyLogic.tsx
index 557f19289d976..78af26bc65688 100644
--- a/frontend/src/scenes/surveys/surveyLogic.tsx
+++ b/frontend/src/scenes/surveys/surveyLogic.tsx
@@ -36,6 +36,7 @@ export enum SurveyEditSection {
     Appearance = 'appearance',
     Customization = 'customization',
     Targeting = 'targeting',
+    CompletionConditions = 'CompletionConditions',
 }
 export interface SurveyLogicProps {
     /** Either a UUID or 'new'. */
@@ -439,6 +440,7 @@ export const surveyLogic = kea<surveyLogicType>([
             actions.setSurveyValue('targeting_flag', NEW_SURVEY.targeting_flag)
             actions.setSurveyValue('conditions', NEW_SURVEY.conditions)
             actions.setSurveyValue('remove_targeting_flag', true)
+            actions.setSurveyValue('responses_limit', NEW_SURVEY.responses_limit)
         },
         submitSurveyFailure: async () => {
             // When errors occur, scroll to the error, but wait for errors to be set in the DOM first
diff --git a/frontend/src/types.ts b/frontend/src/types.ts
index 32bd062be1587..ec69610d653a7 100644
--- a/frontend/src/types.ts
+++ b/frontend/src/types.ts
@@ -2424,6 +2424,7 @@ export interface Survey {
     end_date: string | null
     archived: boolean
     remove_targeting_flag?: boolean
+    responses_limit: number | null
 }
 
 export enum SurveyUrlMatchType {
diff --git a/latest_migrations.manifest b/latest_migrations.manifest
index 644041805cebf..24aa5dcc51821 100644
--- a/latest_migrations.manifest
+++ b/latest_migrations.manifest
@@ -5,7 +5,7 @@ contenttypes: 0002_remove_content_type_name
 ee: 0016_rolemembership_organization_member
 otp_static: 0002_throttling
 otp_totp: 0002_auto_20190420_0723
-posthog: 0400_datawarehousetable_row_count
+posthog: 0401_survey_responses_limit
 sessions: 0001_initial
 social_django: 0010_uid_db_index
 two_factor: 0007_auto_20201201_1019
diff --git a/posthog/api/survey.py b/posthog/api/survey.py
index cb991a5f95abe..8b15e0b6eb89f 100644
--- a/posthog/api/survey.py
+++ b/posthog/api/survey.py
@@ -58,6 +58,7 @@ class Meta:
             "start_date",
             "end_date",
             "archived",
+            "responses_limit",
         ]
         read_only_fields = ["id", "created_at", "created_by"]
 
@@ -89,6 +90,7 @@ class Meta:
             "start_date",
             "end_date",
             "archived",
+            "responses_limit",
         ]
         read_only_fields = ["id", "linked_flag", "targeting_flag", "created_at"]
 
diff --git a/posthog/migrations/0401_survey_responses_limit.py b/posthog/migrations/0401_survey_responses_limit.py
new file mode 100644
index 0000000000000..9ed5768fd31c6
--- /dev/null
+++ b/posthog/migrations/0401_survey_responses_limit.py
@@ -0,0 +1,17 @@
+# Generated by Django 4.1.13 on 2024-04-14 13:56
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("posthog", "0400_datawarehousetable_row_count"),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name="survey",
+            name="responses_limit",
+            field=models.PositiveIntegerField(null=True),
+        ),
+    ]
diff --git a/posthog/models/feedback/survey.py b/posthog/models/feedback/survey.py
index d1914d5d2b00b..33507ba50bbcd 100644
--- a/posthog/models/feedback/survey.py
+++ b/posthog/models/feedback/survey.py
@@ -56,6 +56,8 @@ class Meta:
     end_date: models.DateTimeField = models.DateTimeField(null=True)
     updated_at: models.DateTimeField = models.DateTimeField(auto_now=True)
     archived: models.BooleanField = models.BooleanField(default=False)
+    # It's not a strict limit as it's enforced in a periodic task
+    responses_limit = models.PositiveIntegerField(null=True)
 
 
 @mutable_receiver([post_save, post_delete], sender=Survey)
diff --git a/posthog/tasks/scheduled.py b/posthog/tasks/scheduled.py
index 537a1a49bbe20..d4893f72d0724 100644
--- a/posthog/tasks/scheduled.py
+++ b/posthog/tasks/scheduled.py
@@ -47,6 +47,7 @@
     update_quota_limiting,
     verify_persons_data_in_sync,
 )
+from posthog.tasks.stop_surveys_reached_target import stop_surveys_reached_target
 from posthog.utils import get_crontab
 
 
@@ -238,6 +239,12 @@ def setup_periodic_tasks(sender: Celery, **kwargs: Any) -> None:
             name="clickhouse clear deleted person data",
         )
 
+    sender.add_periodic_task(
+        crontab(minute="0"),
+        stop_surveys_reached_target.s(),
+        name="stop surveys that reached responses limits",
+    )
+
     if settings.EE_AVAILABLE:
         # every interval seconds, we calculate N replay embeddings
         # the goal is to process _enough_ every 24 hours that
diff --git a/posthog/tasks/stop_surveys_reached_target.py b/posthog/tasks/stop_surveys_reached_target.py
new file mode 100644
index 0000000000000..0477e2605ff5c
--- /dev/null
+++ b/posthog/tasks/stop_surveys_reached_target.py
@@ -0,0 +1,47 @@
+from typing import List, Dict
+from posthog.models import Survey
+from posthog.client import sync_execute
+from posthog.clickhouse.client.connection import Workload
+from django.db.models import UUIDField
+from django.utils import timezone
+from celery import shared_task
+
+
+def _get_surveys_response_counts(surveys_ids: List[UUIDField]) -> Dict[str, int]:
+    data = sync_execute(
+        """
+        SELECT JSONExtractString(properties, '$survey_id') as survey_id, count()
+        FROM events
+        WHERE event = 'survey sent' AND survey_id in %(surveys_ids)s
+        GROUP BY survey_id
+    """,
+        {"surveys_ids": surveys_ids},
+        workload=Workload.OFFLINE,
+    )
+
+    counts = {}
+    for survey_id, count in data:
+        counts[survey_id] = count
+    return counts
+
+
+@shared_task(ignore_result=True)
+def stop_surveys_reached_target() -> None:
+    surveys = Survey.objects.exclude(responses_limit__isnull=True).only("id", "responses_limit")
+    if not surveys:
+        return
+
+    surveys_ids = [survey.id for survey in surveys]
+    response_counts = _get_surveys_response_counts(surveys_ids)
+    for survey in surveys:
+        survey_id = str(survey.id)
+        if survey_id not in response_counts:
+            continue
+
+        response_count = response_counts[survey_id]
+        if response_count < survey.responses_limit:
+            continue
+
+        survey.end_date = timezone.now()
+        survey.responses_limit = None
+        survey.save(update_fields=["end_date", "responses_limit"])


Code review comments:
diff_hunk: @@ -0,0 +1,47 @@
+from typing import List, Dict
path: posthog/tasks/stop_surveys_reached_target.py
text: @author:
Seems like the formatter wasn't applied here, also I doubt the the types are correct in this file. I'll fix it soon

diff_hunk: @@ -47,6 +47,7 @@
     update_quota_limiting,
     verify_persons_data_in_sync,
 )
+from posthog.tasks.stop_surveys_reached_target import stop_surveys_reached_target
path: posthog/tasks/scheduled.py
text: @author:
I wonder why all other tasks are imported from tasks.py. Maybe I need to do so too?

@user1:
Yeah please do! Like this one -> https://github.com/PostHog/posthog/blob/master/posthog/tasks/tasks.py#L533 (keep the `stop_surveys_reached_target` file)