Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 25 commits into from
Apr 26, 2024
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
0bc6e84
feat(surveys): allow to stop surveys once reached enough responses
nikitaevg Apr 7, 2024
eb7367c
fix targeting
nikitaevg Apr 14, 2024
6d46d59
polish
nikitaevg Apr 14, 2024
1ec1b50
fix type errors
nikitaevg Apr 14, 2024
4bd29e7
make the periodic job better, add tests for it
nikitaevg Apr 21, 2024
709ce86
Merge remote-tracking branch 'upstream/master' into 21179-stop-survey
nikitaevg Apr 21, 2024
ef47dd9
resolve the conflict, revert some files
nikitaevg Apr 21, 2024
a2fcfc9
revert files
nikitaevg Apr 21, 2024
afd068c
fix types problems, add info about response limit
nikitaevg Apr 21, 2024
6d25e56
fix tests
nikitaevg Apr 21, 2024
ca7a81c
make the responses limit bold
nikitaevg Apr 21, 2024
dc4eec9
Merge branch 'master' into 21179-stop-survey
neilkakkar Apr 22, 2024
70b9fef
Update frontend/src/scenes/surveys/SurveyEdit.tsx
nikitaevg Apr 23, 2024
e6abdee
address the PR comments
nikitaevg Apr 23, 2024
d707e02
fix typing error
nikitaevg Apr 23, 2024
34902b1
add a frontend test and snapshots
nikitaevg Apr 24, 2024
6f4ac1a
Merge remote-tracking branch 'upstream/master' into 21179-stop-survey
nikitaevg Apr 24, 2024
e3ed0ee
add a forgotten migration
nikitaevg Apr 24, 2024
a6a7435
revert formatted files
nikitaevg Apr 24, 2024
23043e6
add a forgotten file
nikitaevg Apr 24, 2024
68b6a71
Merge remote-tracking branch 'upstream/master' into 21179-stop-survey
nikitaevg Apr 25, 2024
ff42287
why I always forget to save this file
nikitaevg Apr 25, 2024
6c1cacd
replace typing.List and typing.Dict
nikitaevg Apr 25, 2024
9aedb05
fix types, add snapshot, clean test
neilkakkar Apr 26, 2024
78f5e02
stabilise uuids
neilkakkar Apr 26, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 48 additions & 1 deletion frontend/src/scenes/surveys/SurveyEdit.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import './EditSurvey.scss'

import { DndContext } from '@dnd-kit/core'
import { SortableContext, verticalListSortingStrategy } from '@dnd-kit/sortable'
import { IconInfo } from '@posthog/icons'
import { IconLock, IconPlus, IconTrash } from '@posthog/icons'
import {
LemonButton,
Expand All @@ -19,6 +20,7 @@ import { FlagSelector } from 'lib/components/FlagSelector'
import { FEATURE_FLAGS } from 'lib/constants'
import { IconCancel } from 'lib/lemon-ui/icons'
import { LemonField } from 'lib/lemon-ui/LemonField'
import { Tooltip } from 'lib/lemon-ui/Tooltip'
import { featureFlagLogic as enabledFeaturesLogic } from 'lib/logic/featureFlagLogic'
import { featureFlagLogic } from 'scenes/feature-flags/featureFlagLogic'
import { FeatureFlagReleaseConditions } from 'scenes/feature-flags/FeatureFlagReleaseConditions'
Expand Down Expand Up @@ -516,13 +518,18 @@ export default function SurveyEdit(): JSX.Element {
type="number"
size="small"
min={0}
value={value?.seenSurveyWaitPeriodInDays}
value={value?.seenSurveyWaitPeriodInDays || NaN}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This cleans the input field when the value is null

onChange={(val) => {
if (val !== undefined && val > 0) {
onChange({
...value,
seenSurveyWaitPeriodInDays: val,
})
} else {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This allows one to actually clean the input field

onChange({
...value,
seenSurveyWaitPeriodInDays: null,
})
}
}}
className="w-16"
Expand Down Expand Up @@ -597,6 +604,46 @@ export default function SurveyEdit(): JSX.Element {
</LemonField.Pure>
),
},
{
key: SurveyEditSection.CompletionConditions,
header: 'Completion conditions',
content: (
<LemonField name="responses_limit">
{({ onChange, value }) => {
return (
<div className="flex flex-row gap-2 items-center">
<LemonCheckbox
checked={!!value}
onChange={(checked) => {
const newResponsesLimit = checked ? 100 : null
onChange(newResponsesLimit)
}}
/>
Stop the survey once
<LemonInput
type="number"
size="small"
min={1}
value={value || NaN}
onChange={(newValue) => {
if (newValue && newValue > 0) {
onChange(newValue)
} else {
onChange(null)
}
}}
className="w-16"
/>{' '}
responses are received.
<Tooltip title="The survey might receive slightly more responses than the limit specifies.">
<IconInfo />
</Tooltip>
</div>
)
}}
</LemonField>
),
},
]}
/>
</div>
Expand Down
9 changes: 9 additions & 0 deletions frontend/src/scenes/surveys/SurveyView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,15 @@ export function SurveyView({ id }: { id: string }): JSX.Element {
</div>
)}
</div>
{survey.responses_limit && (
<>
<span className="card-secondary mt-4">Completion conditions</span>
<span>
The survey will be stopped once <b>{survey.responses_limit}</b>{' '}
responses are received.
</span>
</>
)}
<LemonDivider />
<SurveyReleaseSummary
id={id}
Expand Down
2 changes: 2 additions & 0 deletions frontend/src/scenes/surveys/Surveys.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ const MOCK_BASIC_SURVEY: Survey = {
start_date: null,
end_date: null,
archived: false,
responses_limit: null,
}

const MOCK_SURVEY_WITH_RELEASE_CONS: Survey = {
Expand Down Expand Up @@ -110,6 +111,7 @@ const MOCK_SURVEY_WITH_RELEASE_CONS: Survey = {
start_date: '2023-04-29T10:04:37.977401Z',
end_date: null,
archived: false,
responses_limit: null,
}

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 @@ -118,6 +118,7 @@ export interface NewSurvey
| 'archived'
| 'appearance'
| 'targeting_flag_filters'
| 'responses_limit'
> {
id: 'new'
linked_flag_id: number | null
Expand Down Expand Up @@ -145,6 +146,7 @@ export const NEW_SURVEY: NewSurvey = {
conditions: null,
archived: false,
appearance: defaultSurveyAppearance,
responses_limit: null,
}

export enum SurveyTemplateType {
Expand Down
4 changes: 4 additions & 0 deletions frontend/src/scenes/surveys/surveyLogic.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ const MULTIPLE_CHOICE_SURVEY: Survey = {
end_date: null,
archived: false,
targeting_flag_filters: undefined,
responses_limit: null,
}

const SINGLE_CHOICE_SURVEY: Survey = {
Expand Down Expand Up @@ -93,6 +94,7 @@ const SINGLE_CHOICE_SURVEY: Survey = {
end_date: null,
archived: false,
targeting_flag_filters: undefined,
responses_limit: null,
}

const MULTIPLE_CHOICE_SURVEY_WITH_OPEN_CHOICE: Survey = {
Expand Down Expand Up @@ -139,6 +141,7 @@ const MULTIPLE_CHOICE_SURVEY_WITH_OPEN_CHOICE: Survey = {
end_date: null,
archived: false,
targeting_flag_filters: undefined,
responses_limit: null,
}

const SINGLE_CHOICE_SURVEY_WITH_OPEN_CHOICE: Survey = {
Expand Down Expand Up @@ -185,6 +188,7 @@ const SINGLE_CHOICE_SURVEY_WITH_OPEN_CHOICE: Survey = {
end_date: null,
archived: false,
targeting_flag_filters: undefined,
responses_limit: null,
}

describe('multiple choice survey logic', () => {
Expand Down
2 changes: 2 additions & 0 deletions frontend/src/scenes/surveys/surveyLogic.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ export enum SurveyEditSection {
Appearance = 'appearance',
Customization = 'customization',
Targeting = 'targeting',
CompletionConditions = 'CompletionConditions',
}
export interface SurveyLogicProps {
/** Either a UUID or 'new'. */
Expand Down Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions frontend/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2432,6 +2432,7 @@ export interface Survey {
end_date: string | null
archived: boolean
remove_targeting_flag?: boolean
responses_limit: number | null
}

export enum SurveyUrlMatchType {
Expand Down
2 changes: 1 addition & 1 deletion latest_migrations.manifest
Original file line number Diff line number Diff line change
Expand Up @@ -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: 0403_plugin_has_private_access
posthog: 0404_survey_responses_limit
sessions: 0001_initial
social_django: 0010_uid_db_index
two_factor: 0007_auto_20201201_1019
2 changes: 2 additions & 0 deletions posthog/api/survey.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ class Meta:
"start_date",
"end_date",
"archived",
"responses_limit",
]
read_only_fields = ["id", "created_at", "created_by"]

Expand Down Expand Up @@ -89,6 +90,7 @@ class Meta:
"start_date",
"end_date",
"archived",
"responses_limit",
]
read_only_fields = ["id", "linked_flag", "targeting_flag", "created_at"]

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1525,7 +1525,8 @@
"posthog_survey"."start_date",
"posthog_survey"."end_date",
"posthog_survey"."updated_at",
"posthog_survey"."archived"
"posthog_survey"."archived",
"posthog_survey"."responses_limit"
FROM "posthog_survey"
WHERE "posthog_survey"."linked_flag_id" = 2
'''
Expand Down
1 change: 1 addition & 0 deletions posthog/api/test/__snapshots__/test_survey.ambr
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,7 @@
"posthog_survey"."end_date",
"posthog_survey"."updated_at",
"posthog_survey"."archived",
"posthog_survey"."responses_limit",
"posthog_featureflag"."id",
"posthog_featureflag"."key",
"posthog_featureflag"."name",
Expand Down
1 change: 1 addition & 0 deletions posthog/api/test/test_survey.py
Original file line number Diff line number Diff line change
Expand Up @@ -853,6 +853,7 @@ def test_can_list_surveys(self):
"archived": False,
"start_date": None,
"end_date": None,
"responses_limit": None,
}
],
}
Expand Down
17 changes: 17 additions & 0 deletions posthog/migrations/0404_survey_responses_limit.py
Original file line number Diff line number Diff line change
@@ -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", "0403_plugin_has_private_access"),
]

operations = [
migrations.AddField(
model_name="survey",
name="responses_limit",
field=models.PositiveIntegerField(null=True),
),
]
2 changes: 2 additions & 0 deletions posthog/models/feedback/survey.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
7 changes: 7 additions & 0 deletions posthog/tasks/scheduled.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
update_event_partitions,
update_quota_limiting,
verify_persons_data_in_sync,
stop_surveys_reached_target,
)
from posthog.utils import get_crontab

Expand Down Expand Up @@ -238,6 +239,12 @@ def setup_periodic_tasks(sender: Celery, **kwargs: Any) -> None:
name="clickhouse clear deleted person data",
)

sender.add_periodic_task(
crontab(hour="*/12"),
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
Expand Down
62 changes: 62 additions & 0 deletions posthog/tasks/stop_surveys_reached_target.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
from typing import List, Dict

from itertools import groupby
from django.db.models import UUIDField, DateTimeField
from django.utils import timezone

from posthog.clickhouse.client.connection import Workload
from posthog.client import sync_execute
from posthog.models import Survey


def _get_surveys_response_counts(
surveys_ids: List[UUIDField], team_id: UUIDField, earliest_survey_start_date: DateTimeField
) -> Dict[str, int]:
data = sync_execute(
"""
SELECT JSONExtractString(properties, '$survey_id') as survey_id, count()
FROM events
WHERE event = 'survey sent'
AND team_id = %(team_id)s
AND timestamp >= %(earliest_survey_start_date)s
AND survey_id in %(surveys_ids)s
GROUP BY survey_id
""",
{"surveys_ids": surveys_ids, "team_id": team_id, "earliest_survey_start_date": earliest_survey_start_date},
workload=Workload.OFFLINE,
)

counts = {}
for survey_id, count in data:
counts[survey_id] = count
return counts


def _stop_survey_if_reached_limit(survey: Survey, responses_count: int) -> None:
assert survey.responses_limit is not None
if responses_count < survey.responses_limit:
return

survey.end_date = timezone.now()
survey.responses_limit = None
survey.save(update_fields=["end_date", "responses_limit"])


def stop_surveys_reached_target() -> None:
all_surveys = Survey.objects.exclude(responses_limit__isnull=True).only(
"id", "responses_limit", "team_id", "created_at"
)

all_surveys_sorted = sorted(all_surveys, key=lambda survey: survey.team_id)
for team_id, team_surveys in groupby(all_surveys_sorted, lambda survey: survey.team_id):
team_surveys_list = list(team_surveys)
surveys_ids = [survey.id for survey in team_surveys_list]
earliest_survey_start_date = min([survey.created_at for survey in team_surveys_list])

response_counts = _get_surveys_response_counts(surveys_ids, team_id, earliest_survey_start_date)
for survey in team_surveys_list:
survey_id = str(survey.id)
if survey_id not in response_counts:
continue

_stop_survey_if_reached_limit(survey, response_counts[survey_id])
7 changes: 7 additions & 0 deletions posthog/tasks/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -631,6 +631,13 @@ def verify_persons_data_in_sync() -> None:
verify()


@shared_task(ignrore_result=True)
def stop_surveys_reached_target() -> None:
from posthog.tasks.stop_surveys_reached_target import stop_surveys_reached_target

stop_surveys_reached_target()


def recompute_materialized_columns_enabled() -> bool:
from posthog.models.instance_setting import get_instance_setting

Expand Down
Loading
Loading