feat(surveys): Add group property filtering to survey responses#34946
feat(surveys): Add group property filtering to survey responses#34946lucasheriques merged 7 commits intoPostHog:masterfrom
Conversation
There was a problem hiding this comment.
Greptile Summary
This PR adds group property filtering capabilities to survey responses, allowing users to filter survey answers based on organizational attributes like company name and industry. The implementation extends the existing PropertyFilters component to support group taxonomic types and integrates seamlessly with the current survey filter infrastructure. The changes follow established patterns from feature flags and insights, ensuring a consistent user experience.
The key technical changes include:
- Integration of groupsModel to access groupsTaxonomicTypes
- Extension of PropertyFilters to include group-related filter types
- Addition of test coverage for various group property filtering scenarios
Confidence score: 4 /5
- This PR is safe to merge as it's an additive feature with comprehensive test coverage
- The score is 4 because while the implementation is solid and well-tested, any filtering changes require careful monitoring in production to ensure query performance
- Files needing attention:
- surveyLogic.test.ts: Verify test coverage for edge cases
- SurveyResponseFilters.tsx: Check if all group property types are correctly handled
2 files reviewed, 1 comment
Edit PR Review Bot Settings | Greptile
| taxonomicGroupTypes={[ | ||
| TaxonomicFilterGroupType.EventProperties, | ||
| TaxonomicFilterGroupType.PersonProperties, | ||
| TaxonomicFilterGroupType.EventFeatureFlags, | ||
| TaxonomicFilterGroupType.Cohorts, | ||
| TaxonomicFilterGroupType.Elements, | ||
| TaxonomicFilterGroupType.HogQLExpression, | ||
| ...groupsTaxonomicTypes, | ||
| ]} |
There was a problem hiding this comment.
style: Consider moving taxonomic group types to a constant to maintain consistency across components
|
@lucasheriques can you test this, please, since you have more context? thanks |
|
Thanks for the PR @naaa760 |
lucasheriques
left a comment
There was a problem hiding this comment.
hey @naaa760 again appreciate the PR ❤️
code-wise it looks good, however, have one comment about the new filters.
This is now the full list of categories we have in the filters for Surveys.
However, a few of them are not relevant. Survey events are always captured via posthog.capture, and the event is always one of ['survey show', 'survey sent', 'survey dismissed']. So a couple of those do not make sense here like:
- autocapture elements (those are for $autocapture events)
- instance properties (since we only care about the answers / event properties)
- account properties
can you remove them? 🙏
|
@lucasheriques ! fixed! Removed autocapture elements, instance properties, and account properties from survey filters. |
lucasheriques
left a comment
There was a problem hiding this comment.
looks great! thanks for the PR. send me an email at lucas at posthog.com so I can also give you some merch as a reward 🙏
Thank you so much, I sent it! :) |
|
@naaa760 just sent it! no need to re-merge master again. I enabled automerge, so once the CI finishes we'll get this in! |
Sure! |
Hi, I just made a Pull Request!
Allow filtering survey answers with group properties, similar to how it works in feature flags and insights. This enhances survey analysis by letting users segment responses based on organizational/company attributes.
#34937
Added unit tests in surveyLogic.test.ts covering:
Single group property filters
Multiple group property filters
Mixed person and group property filters
Preserving existing survey ID filters