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

Add RFC for repeatable surveys #203

Closed
wants to merge 3 commits into from
Closed

Conversation

Phanatic
Copy link

@Phanatic Phanatic commented May 7, 2024

My first RFC at Posthog, would love any feedback you have on content/style/approach.

Copy link
Contributor

@neilkakkar neilkakkar left a comment

Choose a reason for hiding this comment

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

Nice!

Some questions on the implementation, but overall seems reasonable! (btw the template is just a rough guideline, you don't have to always follow it / feel free to delete sections that don't make too much sense)

optional, would love to see a bit more about exactly what do competitors do with the results display

#### UI changes.
1. Show the user an interface to pick the repeat interval of the survey, this can be relative (every 4 months) or absolute (pick specific dates).
4. Show the user an interface to pick the iteration count, how many times should this survey repeat?
5. If a survey has an iteration count of more than 1, change the existing UI to run custom clickhouse queries to account for a survey with multiple response sets, one for each iteration.
Copy link
Contributor

Choose a reason for hiding this comment

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

would love to hear more about what this means, and how we want to do this?

For example, what if during the 2nd iteration, the same person never responds? Does it still make sense to breakdown results into iteration sets?

It's not clear to me how we should special case this, if at all.

I think what might be useful here is a 2nd view only for users who've responded multiple times, and aggregating only for these users to show changes over time: so we have an overall view, that treats multiple responses as distinct, and this iteration-aggregation view, that only looks at people who've responded multiple times.

To make the above magic work, we absolutely need to make sure we get rid of this bug of users accidentally responding multiple times, which I think you're considering already.

Copy link
Author

@Phanatic Phanatic May 7, 2024

Choose a reason for hiding this comment

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

For example, what if during the 2nd iteration, the same person never responds?

Great callout, I hadn't considered variable response rate from users over a time period. I'll do some more thinking here and update the doc.

I think what might be useful here is a 2nd view only for users who've responded multiple times

Yeah, this makes sense. You can't track NPS if its a new set of people every time who responded in a given time period.

Copy link
Author

Choose a reason for hiding this comment

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

I added some more color about how the results will be displayed and additional chart types we can show

5. If a survey has an iteration count of more than 1, change the existing UI to run custom clickhouse queries to account for a survey with multiple response sets, one for each iteration.

#### posthog-js changes.
1. To decide when to show a survey, see if a user has a survey response submitted for the current survey start date, and show the survey if they haven't.
Copy link
Contributor

Choose a reason for hiding this comment

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

see if a user has a survey response submitted for the current survey start date

How are you planning to query this?

Also worth noting that you'll need to put the local storage cache on a ttl / find another way around not showing the same survey repeatedly. There are some complex patterns here to handle:

  1. When a user first dismisses/responds to a survey, we add something to local storage to not show this survey again.
  2. Now, I'm assuming there's some mechanism like flags(?) controlling when to next show it. This will almost certainly have some delay (since we need to send properties back via ingestion, which can take some time).
  3. Because of the async nature of (2), the client side needs some way to remember not to show this survey again, hence (1). But now, if we do want to re-show it, we also want to delete (1), hence some ttl there.

Copy link
Author

Choose a reason for hiding this comment

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

This is, as you noted, going to be complicated.
For a given survey iteration, we can't depend on the local storage having the necessary information to show/hide a survey given how long the iterations last.
To get over this, we could associate each iteration of the survey with its own feature flag, that has a time based activation.

e.g. survey-targeting-flag-iter-1 is only true for 90 days, until Aug 11 2024. and
survey-targeting-flag-iter-2 is from Aug 11 2024 to November 9th 2024. And we create these flags on a schedule based on the survey frequency. posthog-js just uses this feature flag information to show/hide the dialog. And when it sends the event, it sends the iteration_id as a person property, from the survey object properties.


#### posthog-js changes.
1. To decide when to show a survey, see if a user has a survey response submitted for the current survey start date, and show the survey if they haven't.
3. When a survey is submitted, add properties for survey iteration count and survey start date in the current iteration to the person properties. So this person is accounted as having submitted the survey for this iteration/start date.
Copy link
Contributor

Choose a reason for hiding this comment

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

unclear to me how this is used in point (1) above, or how we use this info?

Are you creating a targeting flag behind the scenes with info like:

$survey_response/${survey.id}/iteration_count is less than 4

AND

$survey_response/${survey.id}/${current_iteration} is not set ?

Copy link
Contributor

Choose a reason for hiding this comment

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

this looks somewhat different than what you're proposing

Copy link
Author

Choose a reason for hiding this comment

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

For a given survey iteration, e.g. 2 of 4, if a user submits a response for iteration 2 on May 13th 2024,
we will add the following person properties.

$survey_response/${survey.id}/2
$survey_response/${survey.id}/05-13-2024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants