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.

Sign up for GitHub

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): Make selector targeting work, add user props #858

Merged
merged 4 commits into from
Oct 26, 2023
Merged

feat(surveys): Make selector targeting work, add user props #858

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ca6d5c1
feat(surveys): Make selector targeting work, add user props
neilkakkar Oct 25, 2023
8746ea7
merge
neilkakkar Oct 25, 2023
0eb83a3
fix
neilkakkar Oct 25, 2023
b423345
add test
neilkakkar Oct 25, 2023
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
11 changes: 9 additions & 2 deletions src/__tests__/extensions/surveys.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,9 @@ describe('survey display logic', () => {
$survey_id: 'testSurvey1',
$survey_name: 'Test survey 1',
sessionRecordingUrl: undefined,
$set: {
'$survey_dismiss/testSurvey1': true,
},
})
expect(localStorage.getItem(`seenSurvey_${mockSurveys[0].id}`)).toBe('true')

Expand Down Expand Up @@ -145,12 +148,16 @@ describe('survey display logic', () => {
expect(mockPostHog.capture).toBeCalledTimes(1)
})

test('when url changes, callSurveys runs again', () => {
test('callSurveys runs on interval irrespective of url change', () => {
jest.useFakeTimers()
jest.spyOn(global, 'setInterval')
generateSurveys(mockPostHog)
expect(mockPostHog.getActiveMatchingSurveys).toBeCalledTimes(1)
expect(setInterval).toHaveBeenLastCalledWith(expect.any(Function), 1500)
expect(setInterval).toHaveBeenLastCalledWith(expect.any(Function), 3000)

jest.advanceTimersByTime(3000)
expect(mockPostHog.getActiveMatchingSurveys).toBeCalledTimes(2)
expect(setInterval).toHaveBeenLastCalledWith(expect.any(Function), 3000)
})

test('multiple choice type question elements are unique', () => {
Expand Down
19 changes: 10 additions & 9 deletions src/extensions/surveys.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -383,6 +383,9 @@ export const createOpenTextOrLinkPopup = (
$survey_question: survey.questions[0].question,
$survey_response: surveyQuestionType === 'open' ? e.target.survey.value : 'link clicked',
sessionRecordingUrl: posthog.get_session_replay_url?.(),
$set: {
[`$survey_response/${survey.id}`]: true,
},
})
if (surveyQuestionType === 'link') {
window.open(question.link || undefined)
Expand Down Expand Up @@ -457,6 +460,9 @@ export const addCancelListeners = (
$survey_name: surveyEventName,
$survey_id: surveyId,
sessionRecordingUrl: posthog.get_session_replay_url?.(),
$set: {
[`$survey_dismiss/${surveyId}`]: true,
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 this was still in draft mode when I looked at it yesterday :( but this is probably better as survey_dismissed

Copy link
Contributor

Choose a reason for hiding this comment

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

Like one can probably gather what it's meant to do even with just "survey dismiss" but we do feature_flag_called and it makes a lot more sense than if we did feature_flag_call

},
})
})
}
Expand Down Expand Up @@ -888,13 +894,8 @@ function nextQuestion(currentQuestionIdx: number, surveyId: string) {
export function generateSurveys(posthog: PostHog) {
callSurveys(posthog, true)

let currentUrl = location.href
if (location.href) {
setInterval(() => {
if (location.href !== currentUrl) {
currentUrl = location.href
callSurveys(posthog, false)
}
}, 1500)
}
// recalculate surveys every 3 seconds to check if URL or selectors have changed
setInterval(() => {
callSurveys(posthog, false)
}, 3000)
}
Loading