fix: separate filters for surveys logic #28570
Conversation
|
Size Change: -47 B (0%) Total Size: 1.18 MB ℹ️ View Unchanged
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
This PR addresses a bug where survey result filters were only being applied to the data table and not to the visualization charts. Here's a concise summary of the key changes:
- Added property filter state management in
surveyLogic.tsxto centrally control filtering across all survey visualizations - Integrated
SurveyResultsFilterscomponent inSurveyView.tsxto provide a unified filtering interface - Added comprehensive test coverage in
surveyLogic.test.tsto validate filter behavior across different survey visualization types - Modified HogQL queries to properly incorporate property filters in all survey metric calculations
- Implemented filter state persistence to maintain consistency when reloading survey data
Key points:
- Added
propertyFiltersstate and actions insurveyLogic.tsxto manage filters centrally - Created new
SurveyResultsFilterscomponent that integrates with existingPropertyFiltersUI - Added test cases to verify filter application across multiple visualization types
- Ensured survey ID property is preserved when applying new filters
3 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile
| // Reload all survey data when filters change | ||
| if (values.survey.questions) { | ||
| values.survey.questions.forEach((question, index) => { | ||
| if (question.type === SurveyQuestionType.Rating) { |
There was a problem hiding this comment.
nit: I think a switch here would make the code more readable.
There was a problem hiding this comment.
i agree, will fix that on #28466, which is the PR on top of this, as this logic already changed a little bit
📸 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. |
Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
Problem
Closes #27729
Filters were only being applied to the DataTableQuery on the SurveyResults page.
Now they're applied to all survey visualizations.
Changes
Loom demo
Store the PropertyFilter values inside
surveyLogic.Pass it down to the other queries as well.
👉 Stay up-to-date with PostHog coding conventions for a smoother review.
Does this work well for both Cloud and self-hosted?
Yes
How did you test this code?
Unit tests with the logic filters
Manual tests to see if filters are working for all charts