Skip to content

Commit

Permalink
fix(taxonomic-filter): Make selecting between property types more sea… (
Browse files Browse the repository at this point in the history
  • Loading branch information
neilkakkar authored Feb 5, 2024
1 parent 56a4ca6 commit ca28a69
Show file tree
Hide file tree
Showing 10 changed files with 150 additions and 12 deletions.
63 changes: 63 additions & 0 deletions cypress/e2e/featureFlags.cy.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,24 @@
import { decideResponse } from '../fixtures/api/decide'

describe('Feature Flags', () => {
let name

beforeEach(() => {

cy.intercept('https://app.posthog.com/decide/*', (req) =>
req.reply(
decideResponse({
'new-feature-flag-operators': true,
})
)
)

cy.intercept('/api/projects/1/property_definitions?type=person&search*', {
fixture: 'api/feature-flags/property_definition',
})
cy.intercept('/api/person/values/*', {
fixture: 'api/feature-flags/property_values',
})
name = 'feature-flag-' + Math.floor(Math.random() * 10000000)
cy.visit('/feature_flags')
})
Expand Down Expand Up @@ -99,4 +116,50 @@ describe('Feature Flags', () => {
cy.get('[data-attr=delete-feature-flag]').click()
cy.get('.Toastify').contains('Undo').should('be.visible')
})

it.only('Move between property types smoothly, and support relative dates', () => {

This comment has been minimized.

Copy link
@zlwaterfield

zlwaterfield Mar 21, 2024

Contributor

@neilkakkar I noticed .only was set here so the other tests in the file aren't going to be running. Is that expected?

This comment has been minimized.

Copy link
@neilkakkar

neilkakkar Mar 21, 2024

Author Contributor

dang it, no, thanks for catching!

// ensure unique names to avoid clashes
cy.get('[data-attr=top-bar-name]').should('contain', 'Feature flags')
cy.get('[data-attr=new-feature-flag]').click()
cy.get('[data-attr=feature-flag-key]').click().type(`{moveToEnd}${name}`).should('have.value', name)

// select "add filter" and "property"
cy.get('[data-attr=property-select-toggle-0').click()

// select the first property
cy.get('[data-attr=taxonomic-filter-searchfield]').click()
cy.get('[data-attr=taxonomic-filter-searchfield]').type('is_demo')
cy.get('[data-attr=taxonomic-tab-person_properties]').click()
// select numeric $browser_version
cy.get('[data-attr=prop-filter-person_properties-2]').click({ force: true })
// select operator "is greater than" which isn't present for non-numeric properties
cy.get('[data-attr=taxonomic-operator]').contains('= equals').click({ force: true })
cy.get('.operator-value-option').contains('> greater than').click({ force: true })

// selects the first value
cy.get('[data-attr=prop-val]').click()
cy.get('[data-attr=prop-val-0]').click({ force: true })

// now change property type
cy.get('[data-attr=property-select-toggle-0').click()
cy.get('[data-attr=taxonomic-tab-person_properties]').click()

// select dateTime date_prop
cy.get('[data-attr=prop-filter-person_properties-3]').click({ force: true })
cy.get('[data-attr=taxonomic-operator]').contains('= equals').click({ force: true })
cy.get('.operator-value-option').contains('> after').click({ force: true })

// By default says "Select a value"
cy.get('[data-attr=taxonomic-value-select]').contains('Select a value').click()
cy.get('.Popover__content').contains('Last 7 days').click({ force: true})
cy.get('[data-attr=taxonomic-value-select]').contains('Last 7 days')

// now change property type
cy.get('[data-attr=property-select-toggle-0').click()
cy.get('[data-attr=taxonomic-tab-person_properties]').click()
// select regular prop
cy.get('[data-attr=prop-filter-person_properties-1]').click({ force: true })
cy.get('[data-attr=taxonomic-operator]').contains('= equals').click({ force: true })
cy.get('.operator-value-option').contains('> after').should('not.exist')
})
})
39 changes: 39 additions & 0 deletions cypress/fixtures/api/feature-flags/property_definition.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
{
"count": 4,
"next": null,
"previous": null,
"results": [
{
"id": "017dde0e-1cb5-0000-68b4-44835b7c894k",
"name": "is_demo",
"is_numerical": false,
"query_usage_30_day": null,
"property_type": null,
"is_seen_on_filtered_events": null
},
{
"id": "017dde0e-1cb5-0000-68b4-44835b7c894f",
"name": "$feature/awesome-new-hidden-thing",
"is_numerical": false,
"query_usage_30_day": null,
"property_type": "String",
"is_seen_on_filtered_events": null
},
{
"id": "017dde0e-1cb5-0000-68b4-44835b7c894g",
"name": "$browser_version",
"is_numerical": true,
"query_usage_30_day": null,
"property_type": null,
"is_seen_on_filtered_events": null
},
{
"id": "017dde0e-1cb5-0000-68b4-44835b7c894h",
"name": "date_prop",
"is_numerical": false,
"query_usage_30_day": null,
"property_type": "DateTime",
"is_seen_on_filtered_events": null
}
]
}
13 changes: 13 additions & 0 deletions cypress/fixtures/api/feature-flags/property_values.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
[
{ "name": "3", "count": 160 },
{ "name": "is_demo", "count": 160 },
{ "name": "email", "count": 36 },
{ "name": "address", "count": 35 },
{ "name": "name", "count": 35 },
{ "name": "phone", "count": 35 },
{ "name": "$browser", "count": 1 },
{ "name": "$browser_version", "count": 1 },
{ "name": "$initial_referrer", "count": 1 },
{ "name": "$initial_referring_domain", "count": 1 },
{ "name": "$os", "count": 1 }
]
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
4 changes: 2 additions & 2 deletions frontend/src/lib/components/DateFilter/DateFilter.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ import { useActions, useValues } from 'kea'
import {
CUSTOM_OPTION_DESCRIPTION,
CUSTOM_OPTION_KEY,
CUSTOM_OPTION_VALUE,
DateFilterLogicProps,
DateFilterView,
NO_OVERRIDE_RANGE_PLACEHOLDER,
} from 'lib/components/DateFilter/types'
import { dayjs } from 'lib/dayjs'
import { IconCalendar } from 'lib/lemon-ui/icons'
Expand Down Expand Up @@ -163,7 +163,7 @@ export function DateFilter({
active={isActive}
fullWidth
>
{key === CUSTOM_OPTION_KEY ? CUSTOM_OPTION_VALUE : key}
{key === CUSTOM_OPTION_KEY ? NO_OVERRIDE_RANGE_PLACEHOLDER : key}
</LemonButton>
</Tooltip>
)
Expand Down
27 changes: 23 additions & 4 deletions frontend/src/lib/components/DateFilter/dateFilterLogic.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
import { actions, kea, key, listeners, path, props, reducers, selectors } from 'kea'
import { CUSTOM_OPTION_VALUE, DateFilterLogicProps, DateFilterView } from 'lib/components/DateFilter/types'
import {
DateFilterLogicProps,
DateFilterView,
NO_OVERRIDE_RANGE_PLACEHOLDER,
SELECT_FIXED_VALUE_PLACEHOLDER,
} from 'lib/components/DateFilter/types'
import { Dayjs, dayjs } from 'lib/dayjs'
import { dateFilterToText, dateStringToDayJs, formatDate, formatDateRange, isDate } from 'lib/utils'

Expand Down Expand Up @@ -95,15 +100,29 @@ export const dateFilterLogic = kea<dateFilterLogicType>([
),
],
label: [
(s) => [s.dateFrom, s.dateTo, s.isFixedRange, s.isDateToNow, s.isFixedDate, s.dateOptions],
(dateFrom, dateTo, isFixedRange, isDateToNow, isFixedDate, dateOptions) =>
(s) => [
s.dateFrom,
s.dateTo,
s.isFixedRange,
s.isDateToNow,
s.isFixedDate,
s.dateOptions,
(_, p) => p.isFixedDateMode,
],
(dateFrom, dateTo, isFixedRange, isDateToNow, isFixedDate, dateOptions, isFixedDateMode) =>
isFixedRange
? formatDateRange(dayjs(dateFrom), dayjs(dateTo))
: isDateToNow
? `${formatDate(dayjs(dateFrom))} to now`
: isFixedDate
? formatDate(dateStringToDayJs(dateFrom) ?? dayjs(dateFrom))
: dateFilterToText(dateFrom, dateTo, CUSTOM_OPTION_VALUE, dateOptions, false),
: dateFilterToText(
dateFrom,
dateTo,
isFixedDateMode ? SELECT_FIXED_VALUE_PLACEHOLDER : NO_OVERRIDE_RANGE_PLACEHOLDER,
dateOptions,
false
),
],
}),
listeners(({ actions, values, props }) => ({
Expand Down
3 changes: 2 additions & 1 deletion frontend/src/lib/components/DateFilter/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,6 @@ export type DateFilterLogicProps = {
}

export const CUSTOM_OPTION_KEY = 'Custom'
export const CUSTOM_OPTION_VALUE = 'No date range override'
export const SELECT_FIXED_VALUE_PLACEHOLDER = 'Select a value'
export const NO_OVERRIDE_RANGE_PLACEHOLDER = 'No date range override'
export const CUSTOM_OPTION_DESCRIPTION = 'Use the original date ranges of insights'
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,9 @@ export function OperatorValueSelect({
const propertyDefinition = propertyDefinitions.find((pd) => pd.name === propkey)

// DateTime properties should not default to Exact
const isDateTimeProperty = propertyDefinition?.property_type == PropertyType.DateTime
const startingOperator =
propertyDefinition?.property_type == PropertyType.DateTime && (!operator || operator == PropertyOperator.Exact)
isDateTimeProperty && (!operator || operator == PropertyOperator.Exact)
? PropertyOperator.IsDateExact
: operator || PropertyOperator.Exact
const [currentOperator, setCurrentOperator] = useState(startingOperator)
Expand All @@ -88,10 +89,12 @@ export function OperatorValueSelect({
)
const operators = Object.keys(operatorMapping) as Array<PropertyOperator>
setOperators(operators)
if (currentOperator !== operator) {
if ((currentOperator !== operator && operators.includes(startingOperator)) || !propertyDefinition) {
setCurrentOperator(startingOperator)
} else if (limitedElementProperty && !operators.includes(currentOperator)) {
setCurrentOperator(PropertyOperator.Exact)
} else if (!operators.includes(currentOperator) && propertyDefinition) {
// Whenever the property type changes such that the operator is not compatible, we need to reset the operator
// But, only if the propertyDefinition is available
setCurrentOperator(isDateTimeProperty ? PropertyOperator.IsDateExact : PropertyOperator.Exact)
}
}, [propertyDefinition, propkey, operator])

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ export function PropertyValue({
makeLabel={(_, startOfRange) => (
<span className="hide-when-small">
Matches all values {operator === PropertyOperator.IsDateBefore ? 'before' : 'after'}{' '}
{startOfRange}
{startOfRange} if evaluated today.
</span>
)}
/>
Expand Down

0 comments on commit ca28a69

Please sign in to comment.