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): add configurable delay to popup surveys #22780

Merged
merged 87 commits into from
Jun 24, 2024
Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
87 commits
Select commit Hold shift + click to select a range
bf16859
initial commit
dmarticus Jun 6, 2024
5b2c1d5
Update UI snapshots for `chromium` (2)
github-actions[bot] Jun 6, 2024
59d5f71
Update UI snapshots for `chromium` (2)
github-actions[bot] Jun 7, 2024
d19810a
Update UI snapshots for `chromium` (1)
github-actions[bot] Jun 7, 2024
1e3c800
Update UI snapshots for `chromium` (1)
github-actions[bot] Jun 7, 2024
c48c4f6
Update UI snapshots for `chromium` (2)
github-actions[bot] Jun 7, 2024
728eef7
Update UI snapshots for `chromium` (2)
github-actions[bot] Jun 7, 2024
456c864
Update UI snapshots for `chromium` (2)
github-actions[bot] Jun 7, 2024
d2a526d
Update UI snapshots for `chromium` (2)
github-actions[bot] Jun 7, 2024
1531f6f
Update UI snapshots for `chromium` (2)
github-actions[bot] Jun 7, 2024
9afbdc5
Update UI snapshots for `chromium` (2)
github-actions[bot] Jun 7, 2024
64e68b8
fixing the ui a bit
dmarticus Jun 7, 2024
46a4b47
Merge branch 'feat/delay-survey-popup' of github.com:PostHog/posthog …
dmarticus Jun 7, 2024
d3ca624
write tests for the survey_popup_delay api
dmarticus Jun 7, 2024
1557789
remove todos that were todone
dmarticus Jun 7, 2024
f77b0b7
Update UI snapshots for `chromium` (2)
github-actions[bot] Jun 7, 2024
e6e8bad
Update UI snapshots for `chromium` (2)
github-actions[bot] Jun 7, 2024
c0cc810
Update UI snapshots for `chromium` (2)
github-actions[bot] Jun 7, 2024
c2eef3a
Update UI snapshots for `chromium` (2)
github-actions[bot] Jun 8, 2024
69e3ff3
Update UI snapshots for `chromium` (2)
github-actions[bot] Jun 8, 2024
02e6ace
Update UI snapshots for `chromium` (2)
github-actions[bot] Jun 8, 2024
1a096af
merge main
dmarticus Jun 18, 2024
daab9c7
Update UI snapshots for `chromium` (2)
github-actions[bot] Jun 18, 2024
cb1d5db
Update UI snapshots for `chromium` (2)
github-actions[bot] Jun 18, 2024
4ea8f8c
Update UI snapshots for `chromium` (2)
github-actions[bot] Jun 18, 2024
b31f67c
Update frontend/src/scenes/surveys/SurveyCustomization.tsx
dmarticus Jun 18, 2024
207ebc7
Update frontend/src/types.ts
dmarticus Jun 18, 2024
45aeb93
Update UI snapshots for `chromium` (1)
github-actions[bot] Jun 18, 2024
902352a
Update UI snapshots for `chromium` (2)
github-actions[bot] Jun 18, 2024
38273c1
Update UI snapshots for `chromium` (1)
github-actions[bot] Jun 18, 2024
7f82937
Update UI snapshots for `chromium` (2)
github-actions[bot] Jun 18, 2024
3414f79
code review feedback
dmarticus Jun 18, 2024
e973579
add hour max to survey delay
dmarticus Jun 18, 2024
dd015db
Merge branch 'feat/delay-survey-popup' of github.com:PostHog/posthog …
dmarticus Jun 18, 2024
a73184c
Merge branch 'master' into feat/delay-survey-popup
dmarticus Jun 18, 2024
c127648
Update UI snapshots for `chromium` (1)
github-actions[bot] Jun 18, 2024
cd7cc7f
Merge branch 'master' into feat/delay-survey-popup
dmarticus Jun 18, 2024
1928ead
Update UI snapshots for `chromium` (1)
github-actions[bot] Jun 18, 2024
926be64
Update UI snapshots for `chromium` (1)
github-actions[bot] Jun 18, 2024
f472115
Update UI snapshots for `chromium` (2)
github-actions[bot] Jun 18, 2024
b9d82f0
Update UI snapshots for `chromium` (2)
github-actions[bot] Jun 18, 2024
9bddb36
Update UI snapshots for `chromium` (1)
github-actions[bot] Jun 18, 2024
62e8e7d
Update UI snapshots for `chromium` (1)
github-actions[bot] Jun 18, 2024
1206c9e
Update UI snapshots for `chromium` (2)
github-actions[bot] Jun 18, 2024
d2a4104
Update UI snapshots for `chromium` (2)
github-actions[bot] Jun 19, 2024
bf7e465
Update UI snapshots for `chromium` (2)
github-actions[bot] Jun 19, 2024
57a6804
Update UI snapshots for `chromium` (1)
github-actions[bot] Jun 19, 2024
3533c40
Update UI snapshots for `chromium` (2)
github-actions[bot] Jun 19, 2024
6c06b35
Update UI snapshots for `chromium` (1)
github-actions[bot] Jun 19, 2024
8c95e8e
Update UI snapshots for `chromium` (1)
github-actions[bot] Jun 19, 2024
25403ff
Update UI snapshots for `chromium` (2)
github-actions[bot] Jun 19, 2024
392c01e
not sure how this was reverted, but it's back now
dmarticus Jun 19, 2024
28461a1
Merge branch 'feat/delay-survey-popup' of github.com:PostHog/posthog …
dmarticus Jun 19, 2024
df9c769
was this it?
dmarticus Jun 19, 2024
45815fa
was this it?
dmarticus Jun 19, 2024
e7d250e
Update UI snapshots for `chromium` (1)
github-actions[bot] Jun 19, 2024
c4a77b5
Update UI snapshots for `chromium` (1)
github-actions[bot] Jun 19, 2024
aa64630
Update UI snapshots for `chromium` (1)
github-actions[bot] Jun 19, 2024
c46cc62
Update UI snapshots for `chromium` (2)
github-actions[bot] Jun 19, 2024
69c45ce
Update UI snapshots for `chromium` (1)
github-actions[bot] Jun 20, 2024
25e342c
Update UI snapshots for `chromium` (2)
github-actions[bot] Jun 20, 2024
b5d277d
Update UI snapshots for `chromium` (2)
github-actions[bot] Jun 20, 2024
e48389f
Update UI snapshots for `chromium` (2)
github-actions[bot] Jun 20, 2024
60c9ba3
Merge branch 'master' into feat/delay-survey-popup
dmarticus Jun 24, 2024
837e446
Update query snapshots
github-actions[bot] Jun 24, 2024
7d89379
Update query snapshots
github-actions[bot] Jun 24, 2024
fdb4f6a
Update query snapshots
github-actions[bot] Jun 24, 2024
7aa6782
Update query snapshots
github-actions[bot] Jun 24, 2024
033b6bd
use latest version of posthog-js to activate the survey popup delay f…
dmarticus Jun 24, 2024
220a291
Merge branch 'feat/delay-survey-popup' of github.com:PostHog/posthog …
dmarticus Jun 24, 2024
45f9546
forgot to commit the lockfile
dmarticus Jun 24, 2024
0ba5e0f
Merge branch 'master' into feat/delay-survey-popup
dmarticus Jun 24, 2024
de4fac0
Update query snapshots
github-actions[bot] Jun 24, 2024
a8c7f6a
Update UI snapshots for `chromium` (2)
github-actions[bot] Jun 24, 2024
2354c1f
Update query snapshots
github-actions[bot] Jun 24, 2024
fa28581
Update UI snapshots for `chromium` (2)
github-actions[bot] Jun 24, 2024
f537add
Update query snapshots
github-actions[bot] Jun 24, 2024
bf0db84
Update query snapshots
github-actions[bot] Jun 24, 2024
ca1d4a9
Update query snapshots
github-actions[bot] Jun 24, 2024
260e930
Update query snapshots
github-actions[bot] Jun 24, 2024
f162eef
Update query snapshots
github-actions[bot] Jun 24, 2024
2ee6ce7
Update query snapshots
github-actions[bot] Jun 24, 2024
7f9b27f
Update UI snapshots for `chromium` (2)
github-actions[bot] Jun 24, 2024
dfcb609
Update UI snapshots for `chromium` (2)
github-actions[bot] Jun 24, 2024
ec3673b
Update query snapshots
github-actions[bot] Jun 24, 2024
262f597
Update query snapshots
github-actions[bot] Jun 24, 2024
4669828
Update query snapshots
github-actions[bot] Jun 24, 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
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
38 changes: 38 additions & 0 deletions frontend/src/scenes/surveys/SurveyCustomization.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { LemonButton, LemonCheckbox, LemonInput, LemonSelect } from '@posthog/le
import { useValues } from 'kea'
import { PayGateMini } from 'lib/components/PayGateMini/PayGateMini'
import { upgradeModalLogic } from 'lib/components/UpgradeModal/upgradeModalLogic'
import { LemonField } from 'lib/lemon-ui/LemonField'
import { surveyLogic } from 'scenes/surveys/surveyLogic'

import {
Expand Down Expand Up @@ -134,6 +135,43 @@ export function Customization({ appearance, surveyQuestionItem, onAppearanceChan
checked={appearance?.shuffleQuestions}
/>
</div>
<div className="mt-1">
Copy link
Contributor

Choose a reason for hiding this comment

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

Up to you, but we might want to gate by a feature flag here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

FYI, I'm electing to not feature flag this, because we

  1. don't have that many users on surveys, so a staged rollout here seems like overkill
  2. this is really easy to revert and doesn't leave many lasting (and scary) changes to our schema, etc, if we do.

<LemonField name="survey_popup_delay" className="font-medium">
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: Don't use a lemonField here since the name in LemonField refers to an actual parameter on the survey which in this case doesn't exist. If you want a LemonField like handling, use LemonField.Pure instead

{({ onChange }) => {
return (
<div className="flex flex-row gap-2 items-center">
<LemonCheckbox
checked={!!appearance?.surveyPopupDelay}
onChange={(checked) => {
const surveyPopupDelay = checked ? 5 : undefined
onChange(surveyPopupDelay)
Copy link
Collaborator

Choose a reason for hiding this comment

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

and then you don't need to call this onChange, since it refers to the survey_popup_delay field value. Right now I'd expect that is being added as well to the survey which isn't necessary

onAppearanceChange({ ...appearance, surveyPopupDelay })
}}
/>
Delay survey popup after page load by{' '}
dmarticus marked this conversation as resolved.
Show resolved Hide resolved
<LemonInput
type="number"
data-attr="survey-popup-delay-input"
size="small"
min={1}
Copy link
Collaborator

Choose a reason for hiding this comment

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

add a sensible max as well? If this is seconds, I think anything above 1 hour is crazy :P

value={appearance?.surveyPopupDelay || NaN}
onChange={(newValue) => {
if (newValue && newValue > 0) {
onChange(newValue)
onAppearanceChange({ ...appearance, surveyPopupDelay: newValue })
} else {
onChange(null)
onAppearanceChange({ ...appearance, surveyPopupDelay: undefined })
}
}}
className="w-12"
/>{' '}
seconds.
</div>
)
}}
</LemonField>
</div>
</div>
</>
)
Expand Down
3 changes: 2 additions & 1 deletion frontend/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2642,12 +2642,13 @@ export interface SurveyAppearance {
thankYouMessageDescriptionContentType?: SurveyQuestionDescriptionContentType
autoDisappear?: boolean
position?: string
shuffleQuestions?: boolean
surveyPopupDelay?: number
dmarticus marked this conversation as resolved.
Show resolved Hide resolved
// widget only
widgetType?: 'button' | 'tab' | 'selector'
widgetSelector?: string
widgetLabel?: string
widgetColor?: string
shuffleQuestions?: boolean
}

export interface SurveyQuestionBase {
Expand Down
114 changes: 57 additions & 57 deletions livestream/docker-compose.yml
Original file line number Diff line number Diff line change
@@ -1,62 +1,62 @@
version: "3.8"
version: '3.8'

Copy link
Collaborator

Choose a reason for hiding this comment

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

undo changes here please :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh lmao no idea how this got here. Undoing!

Copy link
Collaborator

Choose a reason for hiding this comment

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

pro-tip if you're unaware, when you don't have a specific commit to revert, you can bring a file back to its state on master via:

git checkout master livestream/docker-compose.yml

as long as master is upto date, this will effectively revert all changes in the file!

services:
postgres:
image: postgres:16-alpine
restart: always
ports:
- "5432:5432"
environment:
POSTGRES_USER: postgres
POSTGRES_PASSWORD: postgres
POSTGRES_DB: liveevents
healthcheck:
test: ["CMD-SHELL", "pg_isready -U postgres"]
interval: 5s
timeout: 5s
postgres:
image: postgres:16-alpine
restart: always
ports:
- '5432:5432'
environment:
POSTGRES_USER: postgres
POSTGRES_PASSWORD: postgres
POSTGRES_DB: liveevents
healthcheck:
test: ['CMD-SHELL', 'pg_isready -U postgres']
interval: 5s
timeout: 5s

redis:
image: redis:alpine
restart: always
ports:
- "6379:6379"
redis:
image: redis:alpine
restart: always
ports:
- '6379:6379'

redpanda:
image: vectorized/redpanda:v23.2.17
command:
- redpanda start
- --smp 1
- --overprovisioned
- --node-id 0
- --kafka-addr PLAINTEXT://0.0.0.0:29092,OUTSIDE://0.0.0.0:9092
- --advertise-kafka-addr PLAINTEXT://redpanda:29092,OUTSIDE://localhost:9092
- --pandaproxy-addr 0.0.0.0:8082
- --advertise-pandaproxy-addr localhost:8082
ports:
- 8081:8081
- 8082:8082
- 9092:9092
- 29092:29092
redpanda:
image: vectorized/redpanda:v23.2.17
command:
- redpanda start
- --smp 1
- --overprovisioned
- --node-id 0
- --kafka-addr PLAINTEXT://0.0.0.0:29092,OUTSIDE://0.0.0.0:9092
- --advertise-kafka-addr PLAINTEXT://redpanda:29092,OUTSIDE://localhost:9092
- --pandaproxy-addr 0.0.0.0:8082
- --advertise-pandaproxy-addr localhost:8082
ports:
- 8081:8081
- 8082:8082
- 9092:9092
- 29092:29092

console:
image: docker.redpanda.com/redpandadata/console:v2.3.8
restart: on-failure
entrypoint: /bin/sh
command: -c "echo \"$$CONSOLE_CONFIG_FILE\" > /tmp/config.yml; /app/console"
environment:
CONFIG_FILEPATH: /tmp/config.yml
CONSOLE_CONFIG_FILE: |
kafka:
brokers: ["redpanda:29092"]
schemaRegistry:
enabled: true
urls: ["http://redpanda:8081"]
connect:
enabled: true
clusters:
- name: datagen
url: http://connect:8083
ports:
- "8088:8088"
depends_on:
- redpanda
console:
image: docker.redpanda.com/redpandadata/console:v2.3.8
restart: on-failure
entrypoint: /bin/sh
command: -c "echo \"$$CONSOLE_CONFIG_FILE\" > /tmp/config.yml; /app/console"
environment:
CONFIG_FILEPATH: /tmp/config.yml
CONSOLE_CONFIG_FILE: |
kafka:
brokers: ["redpanda:29092"]
schemaRegistry:
enabled: true
urls: ["http://redpanda:8081"]
connect:
enabled: true
clusters:
- name: datagen
url: http://connect:8083
ports:
- '8088:8088'
depends_on:
- redpanda
4 changes: 4 additions & 0 deletions posthog/api/survey.py
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,10 @@ def validate_appearance(self, value):
"You need to upgrade to PostHog Enterprise to use HTML in survey thank you message"
)

survey_popup_delay_milliseconds = value.get("surveyPopupDelay")
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we accept seconds in the UI, so this should probably match. might be helpful to add the units to the surveyPopupDelay property to reduce confusion here.

if survey_popup_delay_milliseconds and survey_popup_delay_milliseconds < 0:
raise serializers.ValidationError("Survey popup delay must be a positive integer")

return value

def validate_questions(self, value):
Expand Down
34 changes: 34 additions & 0 deletions posthog/api/test/test_survey.py
Original file line number Diff line number Diff line change
Expand Up @@ -1249,6 +1249,7 @@ def test_create_basic_survey_question_validation(self):
"thankYouMessageHeader": "Thanks for your feedback!",
"thankYouMessageDescription": "<b>We'll use it to make notebooks better.<script>alert(0)</script>",
"shuffleQuestions": True,
"surveyPopupDelay": 60,
},
},
format="json",
Expand All @@ -1271,6 +1272,7 @@ def test_create_basic_survey_question_validation(self):
"thankYouMessageHeader": "Thanks for your feedback!",
"thankYouMessageDescription": "<b>We'll use it to make notebooks better.</b>",
"shuffleQuestions": True,
"surveyPopupDelay": 60,
}
assert response_data["created_by"]["id"] == self.user.id

Expand Down Expand Up @@ -1676,6 +1678,38 @@ def test_validate_thank_you_description_content_type(self):
assert response.status_code == status.HTTP_400_BAD_REQUEST, response_data
assert response_data["detail"] == "thankYouMessageDescriptionContentType must be one of ['text', 'html']"

def test_create_survey_with_survey_popup_delay(self):
response = self.client.post(
f"/api/projects/{self.team.id}/surveys/",
data={
"name": "Notebooks beta release survey",
"type": "popover",
"appearance": {
"surveyPopupDelay": 6000,
},
},
format="json",
)
response_data = response.json()
assert response.status_code == status.HTTP_201_CREATED, response_data
assert response_data["appearance"]["surveyPopupDelay"] == 6000

def test_validate_survey_popup_delay(self):
response = self.client.post(
f"/api/projects/{self.team.id}/surveys/",
data={
"name": "Notebooks beta release survey",
"type": "popover",
"appearance": {
"surveyPopupDelay": -100,
},
},
format="json",
)
response_data = response.json()
assert response.status_code == status.HTTP_400_BAD_REQUEST, response_data
assert response_data["detail"] == "Survey popup delay must be a positive integer"

def test_create_survey_with_valid_question_description_content_type_html(self):
response = self.client.post(
f"/api/projects/{self.team.id}/surveys/",
Expand Down
Loading