fix(cohorts): show between dual input and date picker in cohort filters#53807
Draft
fix(cohorts): show between dual input and date picker in cohort filters#53807
Conversation
Three root causes prevented the between operator from showing two inputs and date operators from showing a date picker: 1. Missing cohortFilterLogicKey on renderField calls caused all criteria rows to share the same kea logic instance, so operator changes in one row wouldn't properly propagate to another. 2. CohortPersonPropertiesValuesField lacked a key prop tied to the operator, so React wouldn't re-mount PropertyValue when the operator changed from e.g. "exact" to "between". 3. value_property was typed as string|null but the between operator stores [min, max] arrays, and resolveCohortFieldValue's return type didn't include PropertyFilterValue. Fixes #20821
Contributor
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
|
Size Change: +48 B (0%) Total Size: 128 MB ℹ️ View Unchanged
|
Address review feedback: use the explicit union type instead of Record<string, any> when casting criteria for field access.
Contributor
|
🎭 Playwright report · View test results →
These issues are not necessarily caused by your changes. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Cohort filters with the "between" operator only show a single input field instead of two (min/max), and date properties don't show a date picker when date-specific operators are selected.
Closes #20821
Changes
Three root causes prevented
PropertyValuefrom rendering the correct input variant (PropertyFilterBetweenfor between,PropertyFilterDatePickerfor date operators):Missing
cohortFilterLogicKey—renderFieldComponentinCohortCriteriaRowBuilderwasn't passing a unique kea logic key, so all criteria rows across the cohort shared the same logic instance. Added a per-row key:cohort_${groupIndex}_${index}.No React
keyonPropertyValue—CohortPersonPropertiesValuesFielddidn't force a re-mount when the operator changed (e.g. from "exact" to "between"), soPropertyValuekept rendering the old input type. Added akeyprop tied to${propertyKey}_${operator}.Wrong types for between values —
value_propertywas typed asstring | nullbut the between operator stores[min, max]arrays. Updated toPropertyFilterValue, and fixedresolveCohortFieldValue's return type and theonSetcallback type annotation.How did you test this code?
I'm an agent and haven't tested this manually in a browser. The fix is based on code analysis of the rendering pipeline from
CohortCriteriaRowBuilder→CohortField→PropertyValue. Existing tests pass. ThePropertyValuecomponent already has working between/date picker rendering — the issue was that the cohort-specific wrapper wasn't providing the right props for those code paths to activate.Publish to changelog?
No
Docs update
N/A
🤖 LLM context
Agent-authored PR fixing a long-standing bug where cohort filter inputs didn't match the selected operator. The fix is surgical — 4 files changed, all in the cohort filter frontend code path.