feat: Allow creating a basic survey from a flag #17358
Conversation
📸 UI snapshots have been updated1 snapshot changes in total. 0 added, 1 modified, 0 deleted:
Triggered by this commit. |
| @@ -162,8 +162,7 @@ export const dataTableLogic = kea<dataTableLogicType>([ | |||
| showReload: query.showReload ?? showIfFull, | |||
| showTimings: query.showTimings ?? flagQueryTimingsEnabled, | |||
| showElapsedTime: | |||
| query.showTimings || | |||
| flagQueryTimingsEnabled || | |||
| (query.showTimings ?? flagQueryTimingsEnabled) || | |||
There was a problem hiding this comment.
@mariusandra I'm assuming this was your intention here, as otherwise I can't override things
| source: { | ||
| kind: NodeKind.HogQLQuery, | ||
| query: surveysShownHogqlQuery, | ||
| filters: { dateRange: { date_from: dayjs(createdAt).format('YYYY-MM-DD') } }, |
There was a problem hiding this comment.
@liyiy I've added date ranges to all queries which should make them faster + more accurate
📸 UI snapshots have been updated1 snapshot changes in total. 0 added, 1 modified, 0 deleted:
Triggered by this commit. |
…nto notebook-survey-action
| hasSurveys: [ | ||
| (s) => [s.featureFlag], | ||
| (featureFlag) => { | ||
| return (featureFlag?.surveys?.length || 0) > 0 |
There was a problem hiding this comment.
This selector will not update unless the feature flag changes. So if surveys are added / removed this selector will not update. Is that intended?
There was a problem hiding this comment.
oh good point, the flag is loaded only on mount right now, or when you revisit the feature flag, I'll add the appropriate listeners to update surveys on flag on survey creation, unless you think there's a better way to go about this?
Just noticed that if I delete something, the notebook logic stays mounted so it still shows the survey, need a refresh for it to say not found. Don't know if we want to do anything here as well
There was a problem hiding this comment.
hmm, this gets much messier, too many places for updates, need a better way around it 🤔
There was a problem hiding this comment.
(to be clear, this is a problem when the survey is created/deleted outside the notebook flow, as within the notebook this is handled appropriately) - need to refresh in this case for now
| return ( | ||
| (nextNode?.type.name === NotebookNodeType.Survey && | ||
| hasSurveys && | ||
| featureFlag.surveys && |
There was a problem hiding this comment.
Do we need this check given we're already guaranteeing surveys exist as part of the hasSurveys selector
There was a problem hiding this comment.
just satisfying typing 🙈 , because it can't parse that hasSurveys implies surveys exist
📸 UI snapshots have been updated1 snapshot changes in total. 0 added, 1 modified, 0 deleted:
Triggered by this commit. |
📸 UI snapshots have been updated1 snapshot changes in total. 0 added, 1 modified, 0 deleted:
Triggered by this commit. |
📸 UI snapshots have been updated1 snapshot changes in total. 0 added, 1 modified, 0 deleted:
Triggered by this commit. |
Problem
Clean up the survey model & interface a bit - will create the migration separately to update the fields.
And allows creating a default survey from a flag, automatically linking it together.
I need to think about how to order these options - but will do that at a later stage, since not everything can allow all these options, and some are more important than others.
Changes
👉 Stay up-to-date with PostHog coding conventions for a smoother review.
How did you test this code?