diff --git a/cypress/e2e/insights-date-picker.ts b/cypress/e2e/insights-date-picker.ts index 77e586b439680..d05e083a23efd 100644 --- a/cypress/e2e/insights-date-picker.ts +++ b/cypress/e2e/insights-date-picker.ts @@ -13,10 +13,10 @@ describe('insights date picker', () => { it('Can set a custom rolling date range', () => { cy.get('[data-attr=date-filter]').click() - cy.get('[data-attr=rolling-date-range-input]').type('{selectall}5{enter}') - cy.get('[data-attr=rolling-date-range-date-options-selector]').click() + cy.get('.Popover [data-attr=rolling-date-range-input]').type('{selectall}5{enter}') + cy.get('.Popover [data-attr=rolling-date-range-date-options-selector]').click() cy.get('.RollingDateRangeFilter__popover > div').contains('days').should('exist').click() - cy.get('.RollingDateRangeFilter__label').should('contain', 'In the last').click() + cy.get('.Popover .RollingDateRangeFilter__label').should('contain', 'In the last').click() // Test that the button shows the correct formatted range cy.get('[data-attr=date-filter]').get('.LemonButton__content').contains('Last 5 days').should('exist') diff --git a/frontend/__snapshots__/scenes-app-insights--funnel-left-to-right-breakdown-edit--light.png b/frontend/__snapshots__/scenes-app-insights--funnel-left-to-right-breakdown-edit--light.png index 010304dab3a6a..184d362c02243 100644 Binary files a/frontend/__snapshots__/scenes-app-insights--funnel-left-to-right-breakdown-edit--light.png and b/frontend/__snapshots__/scenes-app-insights--funnel-left-to-right-breakdown-edit--light.png differ diff --git a/frontend/__snapshots__/scenes-app-insights--stickiness--dark.png b/frontend/__snapshots__/scenes-app-insights--stickiness--dark.png index 6ba86e71c9e92..7e91e07979dc2 100644 Binary files a/frontend/__snapshots__/scenes-app-insights--stickiness--dark.png and b/frontend/__snapshots__/scenes-app-insights--stickiness--dark.png differ diff --git a/frontend/__snapshots__/scenes-app-insights--stickiness--light--webkit.png b/frontend/__snapshots__/scenes-app-insights--stickiness--light--webkit.png index 4cd62a602993f..686a437f38b66 100644 Binary files a/frontend/__snapshots__/scenes-app-insights--stickiness--light--webkit.png and b/frontend/__snapshots__/scenes-app-insights--stickiness--light--webkit.png differ diff --git a/frontend/__snapshots__/scenes-app-insights--stickiness--light.png b/frontend/__snapshots__/scenes-app-insights--stickiness--light.png index 3a54d418417f7..fab7851e7b7e6 100644 Binary files a/frontend/__snapshots__/scenes-app-insights--stickiness--light.png and b/frontend/__snapshots__/scenes-app-insights--stickiness--light.png differ diff --git a/frontend/__snapshots__/scenes-app-insights--trends-line-edit--light.png b/frontend/__snapshots__/scenes-app-insights--trends-line-edit--light.png index c91ec8b5f49c0..39f23e4aab8d1 100644 Binary files a/frontend/__snapshots__/scenes-app-insights--trends-line-edit--light.png and b/frontend/__snapshots__/scenes-app-insights--trends-line-edit--light.png differ diff --git a/frontend/__snapshots__/scenes-app-insights--trends-line-multi--dark--webkit.png b/frontend/__snapshots__/scenes-app-insights--trends-line-multi--dark--webkit.png index 0937b969d9941..a45cfb32ef45e 100644 Binary files a/frontend/__snapshots__/scenes-app-insights--trends-line-multi--dark--webkit.png and b/frontend/__snapshots__/scenes-app-insights--trends-line-multi--dark--webkit.png differ diff --git a/frontend/__snapshots__/scenes-app-insights--trends-line-multi--light--webkit.png b/frontend/__snapshots__/scenes-app-insights--trends-line-multi--light--webkit.png index 0dfdcc2bc744e..9e4b287fe0756 100644 Binary files a/frontend/__snapshots__/scenes-app-insights--trends-line-multi--light--webkit.png and b/frontend/__snapshots__/scenes-app-insights--trends-line-multi--light--webkit.png differ diff --git a/frontend/__snapshots__/scenes-app-insights--trends-line-multi-edit--dark--webkit.png b/frontend/__snapshots__/scenes-app-insights--trends-line-multi-edit--dark--webkit.png index a85918cb1399d..af6218b14837b 100644 Binary files a/frontend/__snapshots__/scenes-app-insights--trends-line-multi-edit--dark--webkit.png and b/frontend/__snapshots__/scenes-app-insights--trends-line-multi-edit--dark--webkit.png differ diff --git a/frontend/__snapshots__/scenes-app-insights--trends-line-multi-edit--light--webkit.png b/frontend/__snapshots__/scenes-app-insights--trends-line-multi-edit--light--webkit.png index 3910a2bda319d..f994c388f54a1 100644 Binary files a/frontend/__snapshots__/scenes-app-insights--trends-line-multi-edit--light--webkit.png and b/frontend/__snapshots__/scenes-app-insights--trends-line-multi-edit--light--webkit.png differ diff --git a/frontend/__snapshots__/scenes-app-insights--trends-number--dark--webkit.png b/frontend/__snapshots__/scenes-app-insights--trends-number--dark--webkit.png index c77fb98eda2fe..5b2da9f35d612 100644 Binary files a/frontend/__snapshots__/scenes-app-insights--trends-number--dark--webkit.png and b/frontend/__snapshots__/scenes-app-insights--trends-number--dark--webkit.png differ diff --git a/frontend/__snapshots__/scenes-app-insights--trends-number--dark.png b/frontend/__snapshots__/scenes-app-insights--trends-number--dark.png index a4161cfad8604..88f5c66d35cbf 100644 Binary files a/frontend/__snapshots__/scenes-app-insights--trends-number--dark.png and b/frontend/__snapshots__/scenes-app-insights--trends-number--dark.png differ diff --git a/frontend/__snapshots__/scenes-app-insights--trends-number--light--webkit.png b/frontend/__snapshots__/scenes-app-insights--trends-number--light--webkit.png index 2077f40f7e192..1b663d5f43d2a 100644 Binary files a/frontend/__snapshots__/scenes-app-insights--trends-number--light--webkit.png and b/frontend/__snapshots__/scenes-app-insights--trends-number--light--webkit.png differ diff --git a/frontend/__snapshots__/scenes-app-insights--trends-number--light.png b/frontend/__snapshots__/scenes-app-insights--trends-number--light.png index 45063465a53fb..b46d7c46b805a 100644 Binary files a/frontend/__snapshots__/scenes-app-insights--trends-number--light.png and b/frontend/__snapshots__/scenes-app-insights--trends-number--light.png differ diff --git a/frontend/__snapshots__/scenes-app-insights--trends-table--dark--webkit.png b/frontend/__snapshots__/scenes-app-insights--trends-table--dark--webkit.png index f432358013e1b..fa7e0b7095b43 100644 Binary files a/frontend/__snapshots__/scenes-app-insights--trends-table--dark--webkit.png and b/frontend/__snapshots__/scenes-app-insights--trends-table--dark--webkit.png differ diff --git a/frontend/__snapshots__/scenes-app-insights--trends-table--dark.png b/frontend/__snapshots__/scenes-app-insights--trends-table--dark.png index 73961bce59898..c58b8c0e9c075 100644 Binary files a/frontend/__snapshots__/scenes-app-insights--trends-table--dark.png and b/frontend/__snapshots__/scenes-app-insights--trends-table--dark.png differ diff --git a/frontend/__snapshots__/scenes-app-insights--trends-table--light--webkit.png b/frontend/__snapshots__/scenes-app-insights--trends-table--light--webkit.png index 80682a9cf5579..f117bb4f0e65f 100644 Binary files a/frontend/__snapshots__/scenes-app-insights--trends-table--light--webkit.png and b/frontend/__snapshots__/scenes-app-insights--trends-table--light--webkit.png differ diff --git a/frontend/__snapshots__/scenes-app-insights--trends-table--light.png b/frontend/__snapshots__/scenes-app-insights--trends-table--light.png index c51f4052ca4a8..6742f5014900f 100644 Binary files a/frontend/__snapshots__/scenes-app-insights--trends-table--light.png and b/frontend/__snapshots__/scenes-app-insights--trends-table--light.png differ diff --git a/frontend/__snapshots__/scenes-app-insights--trends-table-breakdown--dark--webkit.png b/frontend/__snapshots__/scenes-app-insights--trends-table-breakdown--dark--webkit.png index 943a8fe2e7a02..fa599935a9bb4 100644 Binary files a/frontend/__snapshots__/scenes-app-insights--trends-table-breakdown--dark--webkit.png and b/frontend/__snapshots__/scenes-app-insights--trends-table-breakdown--dark--webkit.png differ diff --git a/frontend/__snapshots__/scenes-app-insights--trends-table-breakdown--dark.png b/frontend/__snapshots__/scenes-app-insights--trends-table-breakdown--dark.png index a2eaf137c795a..396a99b4c2a24 100644 Binary files a/frontend/__snapshots__/scenes-app-insights--trends-table-breakdown--dark.png and b/frontend/__snapshots__/scenes-app-insights--trends-table-breakdown--dark.png differ diff --git a/frontend/__snapshots__/scenes-app-insights--trends-table-breakdown--light--webkit.png b/frontend/__snapshots__/scenes-app-insights--trends-table-breakdown--light--webkit.png index 270d6fb375dda..32cb7aef89cf4 100644 Binary files a/frontend/__snapshots__/scenes-app-insights--trends-table-breakdown--light--webkit.png and b/frontend/__snapshots__/scenes-app-insights--trends-table-breakdown--light--webkit.png differ diff --git a/frontend/__snapshots__/scenes-app-insights--trends-table-breakdown--light.png b/frontend/__snapshots__/scenes-app-insights--trends-table-breakdown--light.png index fa0ceacfe612f..447a5b947815d 100644 Binary files a/frontend/__snapshots__/scenes-app-insights--trends-table-breakdown--light.png and b/frontend/__snapshots__/scenes-app-insights--trends-table-breakdown--light.png differ diff --git a/frontend/__snapshots__/scenes-app-insights--trends-table-edit--light.png b/frontend/__snapshots__/scenes-app-insights--trends-table-edit--light.png index f5be9d1dff79c..89fcc727ac439 100644 Binary files a/frontend/__snapshots__/scenes-app-insights--trends-table-edit--light.png and b/frontend/__snapshots__/scenes-app-insights--trends-table-edit--light.png differ diff --git a/frontend/__snapshots__/scenes-app-insights--trends-value--dark--webkit.png b/frontend/__snapshots__/scenes-app-insights--trends-value--dark--webkit.png index bf8f17ca227ab..b3a80510da7e3 100644 Binary files a/frontend/__snapshots__/scenes-app-insights--trends-value--dark--webkit.png and b/frontend/__snapshots__/scenes-app-insights--trends-value--dark--webkit.png differ diff --git a/frontend/__snapshots__/scenes-app-insights--trends-value--dark.png b/frontend/__snapshots__/scenes-app-insights--trends-value--dark.png index 14c6d10d30787..27724a4d7ab4f 100644 Binary files a/frontend/__snapshots__/scenes-app-insights--trends-value--dark.png and b/frontend/__snapshots__/scenes-app-insights--trends-value--dark.png differ diff --git a/frontend/__snapshots__/scenes-app-insights--trends-value--light--webkit.png b/frontend/__snapshots__/scenes-app-insights--trends-value--light--webkit.png index f866ce663e0d9..8341bc13dfa82 100644 Binary files a/frontend/__snapshots__/scenes-app-insights--trends-value--light--webkit.png and b/frontend/__snapshots__/scenes-app-insights--trends-value--light--webkit.png differ diff --git a/frontend/__snapshots__/scenes-app-insights--trends-value--light.png b/frontend/__snapshots__/scenes-app-insights--trends-value--light.png index 9688c0603e79a..9e752dd01b4c7 100644 Binary files a/frontend/__snapshots__/scenes-app-insights--trends-value--light.png and b/frontend/__snapshots__/scenes-app-insights--trends-value--light.png differ diff --git a/frontend/__snapshots__/scenes-app-insights--trends-value-breakdown--dark--webkit.png b/frontend/__snapshots__/scenes-app-insights--trends-value-breakdown--dark--webkit.png index 7acd797b06b40..fff949caec2be 100644 Binary files a/frontend/__snapshots__/scenes-app-insights--trends-value-breakdown--dark--webkit.png and b/frontend/__snapshots__/scenes-app-insights--trends-value-breakdown--dark--webkit.png differ diff --git a/frontend/__snapshots__/scenes-app-insights--trends-value-breakdown--dark.png b/frontend/__snapshots__/scenes-app-insights--trends-value-breakdown--dark.png index 5b74a754e0341..7143e2ea87f7e 100644 Binary files a/frontend/__snapshots__/scenes-app-insights--trends-value-breakdown--dark.png and b/frontend/__snapshots__/scenes-app-insights--trends-value-breakdown--dark.png differ diff --git a/frontend/__snapshots__/scenes-app-insights--trends-value-breakdown--light--webkit.png b/frontend/__snapshots__/scenes-app-insights--trends-value-breakdown--light--webkit.png index 0791ad8c99974..d0f5034fefe22 100644 Binary files a/frontend/__snapshots__/scenes-app-insights--trends-value-breakdown--light--webkit.png and b/frontend/__snapshots__/scenes-app-insights--trends-value-breakdown--light--webkit.png differ diff --git a/frontend/__snapshots__/scenes-app-insights--trends-value-breakdown--light.png b/frontend/__snapshots__/scenes-app-insights--trends-value-breakdown--light.png index 94161a694333c..6139e4bf792f9 100644 Binary files a/frontend/__snapshots__/scenes-app-insights--trends-value-breakdown--light.png and b/frontend/__snapshots__/scenes-app-insights--trends-value-breakdown--light.png differ diff --git a/frontend/__snapshots__/scenes-app-insights-error-empty-states--estimated-query-execution-time-too-long--dark.png b/frontend/__snapshots__/scenes-app-insights-error-empty-states--estimated-query-execution-time-too-long--dark.png index bbae4cd9e2d1f..e0a52006ef20a 100644 Binary files a/frontend/__snapshots__/scenes-app-insights-error-empty-states--estimated-query-execution-time-too-long--dark.png and b/frontend/__snapshots__/scenes-app-insights-error-empty-states--estimated-query-execution-time-too-long--dark.png differ diff --git a/frontend/__snapshots__/scenes-app-insights-error-empty-states--estimated-query-execution-time-too-long--light.png b/frontend/__snapshots__/scenes-app-insights-error-empty-states--estimated-query-execution-time-too-long--light.png index c6bdc807090a6..dfc1c37967f46 100644 Binary files a/frontend/__snapshots__/scenes-app-insights-error-empty-states--estimated-query-execution-time-too-long--light.png and b/frontend/__snapshots__/scenes-app-insights-error-empty-states--estimated-query-execution-time-too-long--light.png differ diff --git a/frontend/src/lib/api.ts b/frontend/src/lib/api.ts index 515da6a68d6c2..bded3aa4f319d 100644 --- a/frontend/src/lib/api.ts +++ b/frontend/src/lib/api.ts @@ -123,6 +123,7 @@ export interface ActivityLogPaginatedResponse extends PaginatedResponse { export interface ApiMethodOptions { signal?: AbortSignal headers?: Record + async?: boolean } export class ApiError extends Error { diff --git a/frontend/src/lib/components/CompareFilter/CompareFilter.tsx b/frontend/src/lib/components/CompareFilter/CompareFilter.tsx index a70f38cc31c17..23999b9dce4db 100644 --- a/frontend/src/lib/components/CompareFilter/CompareFilter.tsx +++ b/frontend/src/lib/components/CompareFilter/CompareFilter.tsx @@ -1,29 +1,94 @@ -import { LemonCheckbox } from '@posthog/lemon-ui' +import { LemonSelect } from '@posthog/lemon-ui' import { useActions, useValues } from 'kea' +import { RollingDateRangeFilter } from 'lib/components/DateFilter/RollingDateRangeFilter' +import { dateFromToText } from 'lib/utils' +import { useEffect, useState } from 'react' import { insightLogic } from 'scenes/insights/insightLogic' import { insightVizDataLogic } from 'scenes/insights/insightVizDataLogic' export function CompareFilter(): JSX.Element | null { const { insightProps, canEditInsight } = useValues(insightLogic) - const { compare, supportsCompare } = useValues(insightVizDataLogic(insightProps)) - const { updateInsightFilter } = useActions(insightVizDataLogic(insightProps)) + const { compareFilter, supportsCompare } = useValues(insightVizDataLogic(insightProps)) + const { updateCompareFilter } = useActions(insightVizDataLogic(insightProps)) + + // This keeps the state of the rolling date range filter, even when different drop down options are selected + // The default value for this is one month + const [tentativeCompareTo, setTentativeCompareTo] = useState(compareFilter?.compare_to || '-1m') const disabled: boolean = !canEditInsight || !supportsCompare + useEffect(() => { + const newCompareTo = compareFilter?.compare_to + if (!!newCompareTo && tentativeCompareTo != newCompareTo) { + setTentativeCompareTo(newCompareTo) + } + }, [compareFilter?.compare_to]) + // Hide compare filter control when disabled to avoid states where control is "disabled but checked" if (disabled) { return null } + const options = [ + { + value: 'none', + label: 'No comparison between periods', + }, + { + value: 'previous', + label: 'Compare to previous period', + }, + { + value: 'compareTo', + label: ( + { + updateCompareFilter({ compare: true, compare_to }) + }} + /> + ), + }, + ] + + let value = 'none' + if (compareFilter?.compare) { + if (compareFilter?.compare_to) { + value = 'compareTo' + } else { + value = 'previous' + } + } + return ( - { - updateInsightFilter({ compare }) + { + if (newValue == 'compareTo') { + updateCompareFilter({ compare: true, compare_to: tentativeCompareTo }) + } + }} + renderButtonContent={(leaf) => + (leaf?.value == 'compareTo' + ? `Compare to ${dateFromToText(tentativeCompareTo)} earlier` + : leaf?.label) || 'Compare to' + } + value={value} + dropdownMatchSelectWidth={false} + onChange={(value) => { + if (value == 'none') { + updateCompareFilter({ compare: false, compare_to: undefined }) + } else if (value == 'previous') { + updateCompareFilter({ compare: true, compare_to: undefined }) + } }} - checked={!!compare} - label={Compare to previous period} - bordered + data-attr="compare-filter" + options={options} size="small" /> ) diff --git a/frontend/src/lib/components/DateFilter/RollingDateRangeFilter.tsx b/frontend/src/lib/components/DateFilter/RollingDateRangeFilter.tsx index 483d865c23f59..e637e1716c7ea 100644 --- a/frontend/src/lib/components/DateFilter/RollingDateRangeFilter.tsx +++ b/frontend/src/lib/components/DateFilter/RollingDateRangeFilter.tsx @@ -17,8 +17,12 @@ const dateOptions: LemonSelectOptionLeaf[] = [ ] type RollingDateRangeFilterProps = { + isButton?: boolean pageKey?: string + /** specifies if the filter is selected in the dropdown (to darken) */ selected?: boolean + /** specifies if the filter is in use (causes it to read props) */ + inUse?: boolean dateFrom?: string | null | dayjs.Dayjs max?: number | null onChange?: (fromDate: string) => void @@ -27,29 +31,89 @@ type RollingDateRangeFilterProps = { ref?: React.MutableRefObject } dateRangeFilterLabel?: string + dateRangeFilterSuffixLabel?: string allowedDateOptions?: DateOption[] fullWidth?: LemonButtonProps['fullWidth'] } export function RollingDateRangeFilter({ + isButton = true, onChange, makeLabel, popover, dateFrom, selected, + inUse, max, dateRangeFilterLabel = 'In the last', + dateRangeFilterSuffixLabel, pageKey, allowedDateOptions = ['days', 'weeks', 'months', 'years'], fullWidth, }: RollingDateRangeFilterProps): JSX.Element { - const logicProps = { onChange, dateFrom, selected, max, pageKey } + const logicProps = { onChange, dateFrom, inUse: selected || inUse, max, pageKey } const { increaseCounter, decreaseCounter, setCounter, setDateOption, toggleDateOptionsSelector, select } = useActions(rollingDateRangeFilterLogic(logicProps)) const { counter, dateOption, formattedDate, startOfDateRange } = useValues(rollingDateRangeFilterLogic(logicProps)) - return ( - + let contents = ( +
+

{dateRangeFilterLabel}

+
e.stopPropagation()}> + + - + + setCounter(value)} + /> + + + + +
+ setDateOption(newValue)} + onClick={(e): void => { + e.stopPropagation() + toggleDateOptionsSelector() + }} + dropdownMatchSelectWidth={false} + options={dateOptions.filter((option) => allowedDateOptions.includes(option.value))} + menu={{ + ...popover, + className: 'RollingDateRangeFilter__popover', + }} + size="xsmall" + /> + {dateRangeFilterSuffixLabel ? ( +

{dateRangeFilterSuffixLabel}

+ ) : null} +
+ ) + + if (isButton) { + contents = ( -

{dateRangeFilterLabel}

-
e.stopPropagation()}> - - - - - setCounter(value)} - /> - - + - -
- setDateOption(newValue)} - onClick={(e): void => { - e.stopPropagation() - toggleDateOptionsSelector() - }} - dropdownMatchSelectWidth={false} - options={dateOptions.filter((option) => allowedDateOptions.includes(option.value))} - menu={{ - ...popover, - className: 'RollingDateRangeFilter__popover', - }} - size="xsmall" - /> + {contents} - - ) + ) + } + + if (makeLabel) { + contents = {contents} + } + + return contents } diff --git a/frontend/src/lib/components/DateFilter/rollingDateRangeFilterLogic.ts b/frontend/src/lib/components/DateFilter/rollingDateRangeFilterLogic.ts index a89be66dfc975..65a44b4537560 100644 --- a/frontend/src/lib/components/DateFilter/rollingDateRangeFilterLogic.ts +++ b/frontend/src/lib/components/DateFilter/rollingDateRangeFilterLogic.ts @@ -18,15 +18,15 @@ const dateOptionsMap = { export type DateOption = (typeof dateOptionsMap)[keyof typeof dateOptionsMap] export type RollingDateFilterLogicPropsType = { - selected?: boolean + inUse?: boolean onChange?: (fromDate: string) => void dateFrom?: Dayjs | string | null max?: number | null pageKey?: string } -const counterDefault = (selected: boolean | undefined, dateFrom: Dayjs | string | null | undefined): number => { - if (selected && dateFrom && typeof dateFrom === 'string') { +const counterDefault = (inUse: boolean | undefined, dateFrom: Dayjs | string | null | undefined): number => { + if (inUse && dateFrom && typeof dateFrom === 'string') { const counter = parseInt(dateFrom.slice(1, -1)) if (counter) { return counter @@ -35,8 +35,8 @@ const counterDefault = (selected: boolean | undefined, dateFrom: Dayjs | string return 3 } -const dateOptionDefault = (selected: boolean | undefined, dateFrom: Dayjs | string | null | undefined): DateOption => { - if (selected && dateFrom && typeof dateFrom === 'string') { +const dateOptionDefault = (inUse: boolean | undefined, dateFrom: Dayjs | string | null | undefined): DateOption => { + if (inUse && dateFrom && typeof dateFrom === 'string') { const dateOption = dateOptionsMap[dateFrom.slice(-1)] if (dateOption) { return dateOption @@ -59,7 +59,7 @@ export const rollingDateRangeFilterLogic = kea( }), reducers(({ props }) => ({ counter: [ - counterDefault(props.selected, props.dateFrom) as number | null, + counterDefault(props.inUse, props.dateFrom) as number | null, { increaseCounter: (state) => (state ? (!props.max || state < props.max ? state + 1 : state) : 1), decreaseCounter: (state) => { @@ -73,7 +73,7 @@ export const rollingDateRangeFilterLogic = kea( }, ], dateOption: [ - dateOptionDefault(props.selected, props.dateFrom), + dateOptionDefault(props.inUse, props.dateFrom), { setDateOption: (_, { option }) => option, }, diff --git a/frontend/src/lib/components/InsightLegend/InsightLegendRow.tsx b/frontend/src/lib/components/InsightLegend/InsightLegendRow.tsx index e5546c8839374..f72a41f82aca8 100644 --- a/frontend/src/lib/components/InsightLegend/InsightLegendRow.tsx +++ b/frontend/src/lib/components/InsightLegend/InsightLegendRow.tsx @@ -27,7 +27,10 @@ export function InsightLegendRow({ rowIndex, item }: InsightLegendRowProps): JSX const { insightProps, hiddenLegendKeys, highlightedSeries } = useValues(insightLogic) const { toggleVisibility } = useActions(insightLogic) - const { compare, display, trendsFilter, breakdownFilter, isSingleSeries } = useValues(trendsDataLogic(insightProps)) + const { display, trendsFilter, compareFilter, breakdownFilter, isSingleSeries } = useValues( + trendsDataLogic(insightProps) + ) + const compare = compareFilter && !!compareFilter.compare const highlighted = shouldHighlightThisRow(hiddenLegendKeys, rowIndex, highlightedSeries) const highlightStyle: Record = highlighted diff --git a/frontend/src/lib/utils.tsx b/frontend/src/lib/utils.tsx index dd026cbd2f122..923ad064a07da 100644 --- a/frontend/src/lib/utils.tsx +++ b/frontend/src/lib/utils.tsx @@ -913,6 +913,16 @@ export function dateFilterToText( return defaultValue } +// Converts a dateFrom string ("-2w") into english: "2 weeks" +export function dateFromToText(dateFrom: string): string | undefined { + const dateOption: (typeof dateOptionsMap)[keyof typeof dateOptionsMap] = dateOptionsMap[dateFrom.slice(-1)] + const counter = parseInt(dateFrom.slice(1, -1)) + if (dateOption && counter) { + return `${counter} ${dateOption}${counter > 1 ? 's' : ''}` + } + return undefined +} + export function dateStringToComponents(date: string | null): { amount: number unit: (typeof dateOptionsMap)[keyof typeof dateOptionsMap] diff --git a/frontend/src/queries/nodes/InsightQuery/utils/filtersToQueryNode.test.ts b/frontend/src/queries/nodes/InsightQuery/utils/filtersToQueryNode.test.ts index 984dc0c2ca9b7..a3c0a3b835ff3 100644 --- a/frontend/src/queries/nodes/InsightQuery/utils/filtersToQueryNode.test.ts +++ b/frontend/src/queries/nodes/InsightQuery/utils/filtersToQueryNode.test.ts @@ -370,6 +370,7 @@ describe('filtersToQueryNode', () => { show_legend: true, hidden_legend_keys: { 0: true, 10: true }, compare: true, + compare_to: '-4d', aggregation_axis_format: 'numeric', aggregation_axis_prefix: '£', aggregation_axis_postfix: '%', @@ -389,7 +390,6 @@ describe('filtersToQueryNode', () => { smoothingIntervals: 1, showLegend: true, hidden_legend_indexes: [0, 10], - compare: true, aggregationAxisFormat: 'numeric', aggregationAxisPrefix: '£', aggregationAxisPostfix: '%', @@ -401,6 +401,10 @@ describe('filtersToQueryNode', () => { breakdownFilter: { breakdown_histogram_bin_count: 1, }, + compareFilter: { + compare: true, + compare_to: '-4d', + }, series: [], } expect(result).toEqual(query) @@ -609,6 +613,7 @@ describe('filtersToQueryNode', () => { const filters: Partial = { insight: InsightType.STICKINESS, compare: true, + compare_to: '-4d', show_legend: true, hidden_legend_keys: { 0: true, 10: true }, shown_as: ShownAsValue.STICKINESS, @@ -620,11 +625,14 @@ describe('filtersToQueryNode', () => { const query: StickinessQuery = { kind: NodeKind.StickinessQuery, stickinessFilter: { - compare: true, showLegend: true, hidden_legend_indexes: [0, 10], display: ChartDisplayType.ActionsLineGraph, }, + compareFilter: { + compare: true, + compare_to: '-4d', + }, series: [], } expect(result).toEqual(query) @@ -967,9 +975,11 @@ describe('filtersToQueryNode', () => { }, filterTestAccounts: true, trendsFilter: { - compare: true, display: ChartDisplayType.BoldNumber, }, + compareFilter: { + compare: true, + }, } expect(result).toEqual(query) }) diff --git a/frontend/src/queries/nodes/InsightQuery/utils/filtersToQueryNode.ts b/frontend/src/queries/nodes/InsightQuery/utils/filtersToQueryNode.ts index 3a5ddbb071cce..965c6205b9152 100644 --- a/frontend/src/queries/nodes/InsightQuery/utils/filtersToQueryNode.ts +++ b/frontend/src/queries/nodes/InsightQuery/utils/filtersToQueryNode.ts @@ -15,6 +15,7 @@ import { ActionsNode, AnalyticsQueryResponseBase, BreakdownFilter, + CompareFilter, DataWarehouseNode, EventsNode, FunnelExclusionActionsNode, @@ -35,6 +36,7 @@ import { import { isFunnelsQuery, isInsightQueryWithBreakdown, + isInsightQueryWithCompare, isInsightQueryWithSeries, isLifecycleQuery, isPathsQuery, @@ -300,6 +302,11 @@ export const filtersToQueryNode = (filters: Partial): InsightQueryNo query.breakdownFilter = breakdownFilterToQuery(filters, isTrendsFilter(filters)) } + // compare filter + if (isInsightQueryWithCompare(query)) { + query.compareFilter = compareFilterToQuery(filters) + } + // group aggregation if (filters.aggregation_group_type_index !== undefined) { query.aggregation_group_type_index = filters.aggregation_group_type_index @@ -345,7 +352,6 @@ export const trendsFilterToQuery = (filters: Partial): TrendsF smoothingIntervals: filters.smoothing_intervals, showLegend: filters.show_legend, hidden_legend_indexes: cleanHiddenLegendIndexes(filters.hidden_legend_keys), - compare: filters.compare, aggregationAxisFormat: filters.aggregation_axis_format, aggregationAxisPrefix: filters.aggregation_axis_prefix, aggregationAxisPostfix: filters.aggregation_axis_postfix, @@ -425,7 +431,6 @@ export const filtersToFunnelPathsQuery = (filters: Partial): Fu export const stickinessFilterToQuery = (filters: Record): StickinessFilter => { return objectCleanWithEmpty({ display: filters.display, - compare: filters.compare, showLegend: filters.show_legend, hidden_legend_indexes: cleanHiddenLegendIndexes(filters.hidden_legend_keys), showValuesOnSeries: filters.show_values_on_series, @@ -456,3 +461,10 @@ export const breakdownFilterToQuery = (filters: Record, isTrends: b : {}), }) } + +export const compareFilterToQuery = (filters: Record): CompareFilter => { + return objectCleanWithEmpty({ + compare: filters.compare, + compare_to: filters.compare_to, + }) +} diff --git a/frontend/src/queries/nodes/InsightQuery/utils/legacy.ts b/frontend/src/queries/nodes/InsightQuery/utils/legacy.ts index b36c90bf36672..abe49230016e7 100644 --- a/frontend/src/queries/nodes/InsightQuery/utils/legacy.ts +++ b/frontend/src/queries/nodes/InsightQuery/utils/legacy.ts @@ -14,6 +14,8 @@ export const isLegacyTrendsFilter = (filters: Record | undefined): 'show_values_on_series', 'show_percent_stack_view', 'show_labels_on_series', + 'compare', + 'compare_to', ] return legacyKeys.some((key) => key in filters) } @@ -87,7 +89,7 @@ export const isLegacyStickinessFilter = (filters: Record | undefine return false } - const legacyKeys = ['show_legend', 'hidden_legend_keys', 'show_values_on_series'] + const legacyKeys = ['show_legend', 'hidden_legend_keys', 'show_values_on_series', 'compare', 'compare_to'] return legacyKeys.some((key) => key in filters) } diff --git a/frontend/src/queries/nodes/InsightQuery/utils/queryNodeToFilter.test.ts b/frontend/src/queries/nodes/InsightQuery/utils/queryNodeToFilter.test.ts index 593d6b466c55a..0247396dd4aa1 100644 --- a/frontend/src/queries/nodes/InsightQuery/utils/queryNodeToFilter.test.ts +++ b/frontend/src/queries/nodes/InsightQuery/utils/queryNodeToFilter.test.ts @@ -73,7 +73,6 @@ describe('queryNodeToFilter', () => { }, trendsFilter: { smoothingIntervals: 3, - compare: true, formula: 'A + B', display: ChartDisplayType.ActionsBar, // breakdown_histogram_bin_count?: TrendsFilterLegacy['breakdown_histogram_bin_count'] @@ -87,6 +86,10 @@ describe('queryNodeToFilter', () => { showPercentStackView: true, // hidden_legend_indexes?: TrendsFilterLegacy['hidden_legend_indexes'] }, + compareFilter: { + compare: true, + compare_to: '-4d', + }, } const result = queryNodeToFilter(query) @@ -100,6 +103,7 @@ describe('queryNodeToFilter', () => { display: ChartDisplayType.ActionsBar, formula: 'A + B', compare: true, + compare_to: '-4d', decimal_places: 5, aggregation_axis_format: 'numeric', aggregation_axis_prefix: 'M', diff --git a/frontend/src/queries/nodes/InsightQuery/utils/queryNodeToFilter.ts b/frontend/src/queries/nodes/InsightQuery/utils/queryNodeToFilter.ts index edeceeb51359d..ad72d4037a3c1 100644 --- a/frontend/src/queries/nodes/InsightQuery/utils/queryNodeToFilter.ts +++ b/frontend/src/queries/nodes/InsightQuery/utils/queryNodeToFilter.ts @@ -4,6 +4,7 @@ import { isFunnelsFilter, isLifecycleFilter, isStickinessFilter, isTrendsFilter import { ActionsNode, BreakdownFilter, + CompareFilter, DataWarehouseNode, EventsNode, FunnelsFilterLegacy, @@ -149,6 +150,10 @@ export const queryNodeToFilter = (query: InsightQueryNode): Partial Object.assign(filters, objectClean>>(query.breakdownFilter)) } + if ((isTrendsQuery(query) || isStickinessQuery(query)) && query.compareFilter) { + Object.assign(filters, objectClean>>(query.compareFilter)) + } + if (!isStickinessQuery(query)) { Object.assign( filters, diff --git a/frontend/src/queries/nodes/InsightViz/utils.ts b/frontend/src/queries/nodes/InsightViz/utils.ts index 169105a837de1..9e2becf3e9ddf 100644 --- a/frontend/src/queries/nodes/InsightViz/utils.ts +++ b/frontend/src/queries/nodes/InsightViz/utils.ts @@ -5,6 +5,7 @@ import { getEventNamesForAction, isEmptyObject } from 'lib/utils' import { ActionsNode, BreakdownFilter, + CompareFilter, DataWarehouseNode, EventsNode, InsightQueryNode, @@ -15,6 +16,7 @@ import { } from '~/queries/schema' import { isInsightQueryWithBreakdown, + isInsightQueryWithCompare, isInsightQueryWithSeries, isLifecycleQuery, isStickinessQuery, @@ -51,15 +53,6 @@ export const getDisplay = (query: InsightQueryNode): ChartDisplayType | undefine return undefined } -export const getCompare = (query: InsightQueryNode): boolean | undefined => { - if (isStickinessQuery(query)) { - return query.stickinessFilter?.compare - } else if (isTrendsQuery(query)) { - return query.trendsFilter?.compare - } - return undefined -} - export const getFormula = (query: InsightQueryNode): string | undefined => { if (isTrendsQuery(query)) { return query.trendsFilter?.formula @@ -88,6 +81,13 @@ export const getBreakdown = (query: InsightQueryNode): BreakdownFilter | undefin return undefined } +export const getCompareFilter = (query: InsightQueryNode): CompareFilter | undefined => { + if (isInsightQueryWithCompare(query)) { + return query.compareFilter + } + return undefined +} + export const getShowLegend = (query: InsightQueryNode): boolean | undefined => { if (isStickinessQuery(query)) { return query.stickinessFilter?.showLegend diff --git a/frontend/src/queries/query.ts b/frontend/src/queries/query.ts index b5127fe672d8a..ff1478b9fb4a8 100644 --- a/frontend/src/queries/query.ts +++ b/frontend/src/queries/query.ts @@ -112,6 +112,7 @@ async function executeQuery( setPollResponse?: (response: QueryStatus) => void ): Promise> { const isAsyncQuery = + methodOptions?.async !== false && !SYNC_ONLY_QUERY_KINDS.includes(queryNode.kind) && !!featureFlagLogic.findMounted()?.values.featureFlags?.[FEATURE_FLAGS.QUERY_ASYNC] diff --git a/frontend/src/queries/schema.json b/frontend/src/queries/schema.json index bb951afb167e1..f84829a8acfdc 100644 --- a/frontend/src/queries/schema.json +++ b/frontend/src/queries/schema.json @@ -1717,6 +1717,18 @@ "required": ["key", "type", "value"], "type": "object" }, + "CompareFilter": { + "additionalProperties": false, + "properties": { + "compare": { + "type": "boolean" + }, + "compare_to": { + "type": "string" + } + }, + "type": "object" + }, "CountPerActorMathType": { "enum": [ "avg_count_per_actor", @@ -7575,6 +7587,9 @@ "compare": { "type": "boolean" }, + "compare_to": { + "type": "string" + }, "display": { "$ref": "#/definitions/ChartDisplayType" }, @@ -7596,6 +7611,10 @@ "StickinessQuery": { "additionalProperties": false, "properties": { + "compareFilter": { + "$ref": "#/definitions/CompareFilter", + "description": "Compare to date range" + }, "dateRange": { "$ref": "#/definitions/InsightDateRange", "description": "Date range for the query" @@ -7924,10 +7943,6 @@ "breakdown_histogram_bin_count": { "type": "number" }, - "compare": { - "default": false, - "type": "boolean" - }, "decimalPlaces": { "type": "number" }, @@ -7985,6 +8000,9 @@ "compare": { "type": "boolean" }, + "compare_to": { + "type": "string" + }, "decimal_places": { "type": "number" }, @@ -8029,6 +8047,10 @@ "$ref": "#/definitions/BreakdownFilter", "description": "Breakdown of the events and actions" }, + "compareFilter": { + "$ref": "#/definitions/CompareFilter", + "description": "Compare to date range" + }, "dateRange": { "$ref": "#/definitions/InsightDateRange", "description": "Date range for the query" diff --git a/frontend/src/queries/schema.ts b/frontend/src/queries/schema.ts index 9b28e12ebfe3c..c677137646ee5 100644 --- a/frontend/src/queries/schema.ts +++ b/frontend/src/queries/schema.ts @@ -679,8 +679,6 @@ export type TrendsFilterLegacy = Omit< export type TrendsFilter = { /** @default 1 */ smoothingIntervals?: integer - /** @default false */ - compare?: TrendsFilterLegacy['compare'] formula?: TrendsFilterLegacy['formula'] /** @default ActionsLineGraph */ display?: TrendsFilterLegacy['display'] @@ -700,6 +698,22 @@ export type TrendsFilter = { hidden_legend_indexes?: TrendsFilterLegacy['hidden_legend_indexes'] } +export const TRENDS_FILTER_PROPERTIES = new Set([ + 'smoothingIntervals', + 'formula', + 'display', + 'showLegend', + 'breakdown_histogram_bin_count', + 'aggregationAxisFormat', + 'aggregationAxisPrefix', + 'aggregationAxisPostfix', + 'decimalPlaces', + 'showValuesOnSeries', + 'showLabelsOnSeries', + 'showPercentStackView', + 'hidden_legend_indexes', +]) + export interface TrendsQueryResponse extends AnalyticsQueryResponseBase[]> {} export type CachedTrendsQueryResponse = CachedQueryResponse @@ -718,6 +732,8 @@ export interface TrendsQuery extends InsightsQueryBase { trendsFilter?: TrendsFilter /** Breakdown of the events and actions */ breakdownFilter?: BreakdownFilter + /** Compare to date range */ + compareFilter?: CompareFilter } /** `FunnelsFilterType` minus everything inherited from `FilterType` and persons modal related params @@ -897,6 +913,14 @@ export type StickinessFilter = { hidden_legend_indexes?: StickinessFilterLegacy['hidden_legend_indexes'] } +export const STICKINESS_FILTER_PROPERTIES = new Set([ + 'compare', + 'display', + 'showLegend', + 'showValuesOnSeries', + 'hidden_legend_indexes', +]) + export interface StickinessQueryResponse extends AnalyticsQueryResponseBase[]> {} export type CachedStickinessQueryResponse = CachedQueryResponse @@ -913,6 +937,8 @@ export interface StickinessQuery series: AnyEntityNode[] /** Properties specific to the stickiness insight */ stickinessFilter?: StickinessFilter + /** Compare to date range */ + compareFilter?: CompareFilter } /** `LifecycleFilterType` minus everything inherited from `FilterType` */ @@ -1537,6 +1563,11 @@ export interface BreakdownFilter { breakdown_hide_other_aggregation?: boolean | null // hides the "other" field for trends } +export interface CompareFilter { + compare?: boolean + compare_to?: string +} + // TODO: Rename to `DashboardFilters` for consistency with `HogQLFilters` export interface DashboardFilter { date_from?: string | null diff --git a/frontend/src/queries/utils.ts b/frontend/src/queries/utils.ts index c153870934f1b..b56b76afbd740 100644 --- a/frontend/src/queries/utils.ts +++ b/frontend/src/queries/utils.ts @@ -191,6 +191,10 @@ export function isInsightQueryWithBreakdown(node?: Record | null): return isTrendsQuery(node) || isFunnelsQuery(node) } +export function isInsightQueryWithCompare(node?: Record | null): node is TrendsQuery | StickinessQuery { + return isTrendsQuery(node) || isStickinessQuery(node) +} + export function isDatabaseSchemaQuery(node?: Node): node is DatabaseSchemaQuery { return node?.kind === NodeKind.DatabaseSchemaQuery } diff --git a/frontend/src/scenes/insights/insightCommandLogic.ts b/frontend/src/scenes/insights/insightCommandLogic.ts index d6512d8eaa2fa..e08d168468831 100644 --- a/frontend/src/scenes/insights/insightCommandLogic.ts +++ b/frontend/src/scenes/insights/insightCommandLogic.ts @@ -27,8 +27,11 @@ export const insightCommandLogic = kea([ icon: IconTrending, display: 'Toggle "Compare Previous" on Graph', executor: () => { - const compare = insightVizDataLogic(props).values.compare - insightVizDataLogic(props).actions.updateInsightFilter({ compare: !compare }) + const compareFilter = insightVizDataLogic(props).values.compareFilter + insightVizDataLogic(props).actions.updateCompareFilter({ + compare: !compareFilter?.compare, + compare_to: compareFilter?.compare_to, + }) }, }, ...dateMapping.map(({ key, values }) => ({ diff --git a/frontend/src/scenes/insights/insightVizDataLogic.ts b/frontend/src/scenes/insights/insightVizDataLogic.ts index 59815f4f664d4..02b289407ff62 100644 --- a/frontend/src/scenes/insights/insightVizDataLogic.ts +++ b/frontend/src/scenes/insights/insightVizDataLogic.ts @@ -17,7 +17,7 @@ import { BASE_MATH_DEFINITIONS } from 'scenes/trends/mathsLogic' import { queryNodeToFilter, seriesNodeToFilter } from '~/queries/nodes/InsightQuery/utils/queryNodeToFilter' import { getBreakdown, - getCompare, + getCompareFilter, getDisplay, getFormula, getInterval, @@ -30,6 +30,7 @@ import { } from '~/queries/nodes/InsightViz/utils' import { BreakdownFilter, + CompareFilter, DatabaseSchemaField, DataWarehouseNode, DateRange, @@ -96,6 +97,7 @@ export const insightVizDataLogic = kea([ updateInsightFilter: (insightFilter: InsightFilter) => ({ insightFilter }), updateDateRange: (dateRange: DateRange) => ({ dateRange }), updateBreakdownFilter: (breakdownFilter: BreakdownFilter) => ({ breakdownFilter }), + updateCompareFilter: (compareFilter: CompareFilter) => ({ compareFilter }), updateDisplay: (display: ChartDisplayType | undefined) => ({ display }), setTimedOutQueryId: (id: string | null) => ({ id }), }), @@ -150,8 +152,8 @@ export const insightVizDataLogic = kea([ dateRange: [(s) => [s.querySource], (q) => (q ? q.dateRange : null)], breakdownFilter: [(s) => [s.querySource], (q) => (q ? getBreakdown(q) : null)], + compareFilter: [(s) => [s.querySource], (q) => (q ? getCompareFilter(q) : null)], display: [(s) => [s.querySource], (q) => (q ? getDisplay(q) : null)], - compare: [(s) => [s.querySource], (q) => (q ? getCompare(q) : null)], formula: [(s) => [s.querySource], (q) => (q ? getFormula(q) : null)], series: [(s) => [s.querySource], (q) => (q ? getSeries(q) : null)], interval: [(s) => [s.querySource], (q) => (q ? getInterval(q) : null)], @@ -362,7 +364,13 @@ export const insightVizDataLogic = kea([ listeners(({ actions, values, props }) => ({ updateDateRange: async ({ dateRange }, breakpoint) => { await breakpoint(300) - actions.updateQuerySource({ dateRange: { ...values.dateRange, ...dateRange } }) + actions.updateQuerySource({ + dateRange: { + ...values.dateRange, + ...dateRange, + }, + ...(dateRange.date_from == 'all' ? ({ compareFilter: undefined } as Partial) : {}), + }) }, updateBreakdownFilter: async ({ breakdownFilter }, breakpoint) => { await breakpoint(500) // extra debounce time because of number input @@ -370,6 +378,12 @@ export const insightVizDataLogic = kea([ breakdownFilter: { ...values.breakdownFilter, ...breakdownFilter }, } as Partial) }, + updateCompareFilter: async ({ compareFilter }, breakpoint) => { + await breakpoint(500) // extra debounce time because of number input + actions.updateQuerySource({ + compareFilter: { ...values.compareFilter, ...compareFilter }, + } as Partial) + }, updateInsightFilter: async ({ insightFilter }, breakpoint) => { await breakpoint(300) const filterProperty = filterKeyForQuery(values.localQuerySource) diff --git a/frontend/src/scenes/insights/utils/cleanFilters.ts b/frontend/src/scenes/insights/utils/cleanFilters.ts index 661191c94451d..764518498a1db 100644 --- a/frontend/src/scenes/insights/utils/cleanFilters.ts +++ b/frontend/src/scenes/insights/utils/cleanFilters.ts @@ -465,6 +465,7 @@ export function cleanFilters( if (filters.date_from === 'all' || isLifecycleFilter(filters)) { trendLikeFilter['compare'] = false + trendLikeFilter['compare_to'] = undefined } if (trendLikeFilter.interval && trendLikeFilter.smoothing_intervals) { diff --git a/frontend/src/scenes/insights/views/BoldNumber/BoldNumber.tsx b/frontend/src/scenes/insights/views/BoldNumber/BoldNumber.tsx index bce29e83078a1..e2c6dc0070037 100644 --- a/frontend/src/scenes/insights/views/BoldNumber/BoldNumber.tsx +++ b/frontend/src/scenes/insights/views/BoldNumber/BoldNumber.tsx @@ -85,14 +85,14 @@ function useBoldNumberTooltip({ export function BoldNumber({ showPersonsModal = true }: ChartParams): JSX.Element { const { insightProps } = useValues(insightLogic) - const { insightData, trendsFilter, querySource, isDataWarehouseSeries } = useValues( + const { insightData, trendsFilter, compareFilter, querySource, isDataWarehouseSeries } = useValues( insightVizDataLogic(insightProps) ) const [isTooltipShown, setIsTooltipShown] = useState(false) const valueRef = useBoldNumberTooltip({ showPersonsModal, isTooltipShown }) - const showComparison = !!trendsFilter?.compare && insightData?.result?.length > 1 + const showComparison = !!compareFilter?.compare && insightData?.result?.length > 1 const resultSeries = insightData?.result?.[0] as TrendResult | undefined return resultSeries ? ( diff --git a/frontend/src/scenes/insights/views/InsightsTable/InsightsTable.tsx b/frontend/src/scenes/insights/views/InsightsTable/InsightsTable.tsx index 5197cc749fd43..5c89e6e8d3c9c 100644 --- a/frontend/src/scenes/insights/views/InsightsTable/InsightsTable.tsx +++ b/frontend/src/scenes/insights/views/InsightsTable/InsightsTable.tsx @@ -54,7 +54,7 @@ export function InsightsTable({ insightDataLoading, indexedResults, isNonTimeSeriesDisplay, - compare, + compareFilter, isTrends, display, interval, @@ -103,7 +103,7 @@ export function InsightsTable({ item={item} indexedResults={indexedResults} canEditSeriesNameInline={canEditSeriesNameInline} - compare={compare} + compare={!!compareFilter?.compare} handleEditClick={handleSeriesEditClick} hasMultipleSeries={!isSingleSeries} /> @@ -198,7 +198,7 @@ export function InsightsTable({ ), @@ -227,7 +227,7 @@ export function InsightsTable({ emptyState="No insight results" data-attr="insights-table-graph" useURLForSorting={insightMode !== ItemMode.Edit} - rowRibbonColor={isLegend ? (item) => getSeriesColor(item.seriesIndex, compare || false) : undefined} + rowRibbonColor={isLegend ? (item) => getSeriesColor(item.seriesIndex, !!compareFilter?.compare) : undefined} firstColumnSticky maxHeaderWidth="20rem" /> diff --git a/frontend/src/scenes/notebooks/Notebook/migrations/migrate.test.ts b/frontend/src/scenes/notebooks/Notebook/migrations/migrate.test.ts index 1ec2fe486cb47..d1409e1716f6d 100644 --- a/frontend/src/scenes/notebooks/Notebook/migrations/migrate.test.ts +++ b/frontend/src/scenes/notebooks/Notebook/migrations/migrate.test.ts @@ -13,7 +13,7 @@ describe('migrate()', () => { { type: 'ph-query', attrs: { - query: '{"kind":"InsightVizNode","source":{"kind":"TrendsQuery","properties":{"type":"AND","values":[{"type":"AND","values":[]}]},"filterTestAccounts":true,"dateRange":{"date_to":null,"date_from":"-90d"},"series":[{"kind":"EventsNode","event":"$pageview","name":"$pageview","properties":[{"key":"$referring_domain","type":"event","value":"google|duckduckgo|brave|bing","operator":"regex"},{"key":"utm_source","type":"event","value":"is_not_set","operator":"is_not_set"},{"key":"$host","type":"event","value":["posthog.com"],"operator":"exact"}],"math":"dau"}],"interval":"week","breakdown":{"breakdown_type":"event","breakdown":"$referring_domain"},"trendsFilter":{"compare":false,"display":"ActionsBar"}}}', + query: '{"kind":"InsightVizNode","source":{"kind":"TrendsQuery","properties":{"type":"AND","values":[{"type":"AND","values":[]}]},"filterTestAccounts":true,"dateRange":{"date_to":null,"date_from":"-90d"},"series":[{"kind":"EventsNode","event":"$pageview","name":"$pageview","properties":[{"key":"$referring_domain","type":"event","value":"google|duckduckgo|brave|bing","operator":"regex"},{"key":"utm_source","type":"event","value":"is_not_set","operator":"is_not_set"},{"key":"$host","type":"event","value":["posthog.com"],"operator":"exact"}],"math":"dau"}],"interval":"week","breakdown":{"breakdown_type":"event","breakdown":"$referring_domain"},"trendsFilter":{"compare":true,"display":"ActionsBar"}}}', title: 'SEO trend last 90 days', __init: null, height: null, @@ -58,7 +58,8 @@ describe('migrate()', () => { ], interval: 'week', breakdownFilter: { breakdown_type: 'event', breakdown: '$referring_domain' }, - trendsFilter: { compare: false, display: 'ActionsBar' }, + trendsFilter: { display: 'ActionsBar' }, + compareFilter: { compare: true }, }, }, title: 'SEO trend last 90 days', @@ -411,7 +412,8 @@ describe('migrate()', () => { type: 'AND', values: [{ type: 'AND', values: [] }], }, - trendsFilter: { compare: false, display: 'ActionsBar' }, + trendsFilter: { display: 'ActionsBar' }, + compareFilter: { compare: true, compare_to: '-4w' }, filterTestAccounts: true, }, }, @@ -469,7 +471,8 @@ describe('migrate()', () => { type: 'AND', values: [{ type: 'AND', values: [] }], }, - trendsFilter: { compare: false, display: 'ActionsBar' }, + trendsFilter: { display: 'ActionsBar' }, + compareFilter: { compare: true, compare_to: '-4w' }, filterTestAccounts: true, }, }, @@ -654,11 +657,11 @@ describe('migrate()', () => { ], }, trendsFilter: { - compare: false, display: 'ActionsLineGraph', show_legend: false, smoothing_intervals: 1, show_values_on_series: true, + compare: true, }, filterTestAccounts: false, }, @@ -784,12 +787,12 @@ describe('migrate()', () => { ], }, trendsFilter: { - compare: false, display: 'ActionsLineGraph', showLegend: false, smoothingIntervals: 1, showValuesOnSeries: true, }, + compareFilter: { compare: true }, filterTestAccounts: false, }, }, @@ -802,6 +805,129 @@ describe('migrate()', () => { }, ], ], + [ + 'migrates compare from previously migrated trends query', + [ + { + type: 'ph-query', + attrs: { + query: { + kind: 'InsightVizNode', + source: { + kind: 'TrendsQuery', + series: [ + { + kind: 'EventsNode', + event: '$pageview', + }, + ], + trendsFilter: { + compare: true, + aggregationAxisFormat: 'percentage', + }, + }, + }, + title: 'Some insight', + __init: null, + height: null, + nodeId: '4c2a07ee-fc9f-45c5-b36c-5e14a10f8e22', + children: null, + }, + }, + ], + [ + { + type: 'ph-query', + attrs: { + query: { + kind: 'InsightVizNode', + source: { + kind: 'TrendsQuery', + series: [ + { + kind: 'EventsNode', + event: '$pageview', + }, + ], + trendsFilter: { + aggregationAxisFormat: 'percentage', + }, + compareFilter: { + compare: true, + }, + }, + }, + title: 'Some insight', + __init: null, + height: null, + nodeId: '4c2a07ee-fc9f-45c5-b36c-5e14a10f8e22', + children: null, + }, + }, + ], + ], + + [ + 'migrates compare from previously migrated stickiness query', + [ + { + type: 'ph-query', + attrs: { + query: { + kind: 'InsightVizNode', + source: { + kind: 'StickinessQuery', + series: [ + { + kind: 'EventsNode', + event: '$pageview', + }, + ], + stickinessFilter: { + compare: true, + showValuesOnSeries: true, + }, + }, + }, + title: 'Some insight', + __init: null, + height: null, + nodeId: '4c2a07ee-fc9f-45c5-b36c-5e14a10f8e22', + children: null, + }, + }, + ], + [ + { + type: 'ph-query', + attrs: { + query: { + kind: 'InsightVizNode', + source: { + kind: 'StickinessQuery', + series: [ + { + kind: 'EventsNode', + event: '$pageview', + }, + ], + stickinessFilter: { + showValuesOnSeries: true, + }, + compareFilter: { + compare: true, + }, + }, + }, + title: 'Some insight', + __init: null, + height: null, + nodeId: '4c2a07ee-fc9f-45c5-b36c-5e14a10f8e22', + children: null, + }, + }, + ], + ], ] it.each(contentToExpected)('migrates %s', (_name, prevContent, nextContent) => { diff --git a/frontend/src/scenes/notebooks/Notebook/migrations/migrate.ts b/frontend/src/scenes/notebooks/Notebook/migrations/migrate.ts index 041db92232c18..df9a4dd7b30b3 100644 --- a/frontend/src/scenes/notebooks/Notebook/migrations/migrate.ts +++ b/frontend/src/scenes/notebooks/Notebook/migrations/migrate.ts @@ -1,7 +1,9 @@ import { JSONContent } from '@tiptap/core' +import { isEmptyObject } from 'lib/utils' import { breakdownFilterToQuery, + compareFilterToQuery, exlusionEntityToNode, funnelsFilterToQuery, lifecycleFilterToQuery, @@ -19,7 +21,16 @@ import { isLegacyStickinessFilter, isLegacyTrendsFilter, } from '~/queries/nodes/InsightQuery/utils/legacy' -import { InsightVizNode, NodeKind } from '~/queries/schema' +import { + InsightVizNode, + NodeKind, + STICKINESS_FILTER_PROPERTIES, + StickinessFilter, + StickinessFilterLegacy, + TRENDS_FILTER_PROPERTIES, + TrendsFilter, + TrendsFilterLegacy, +} from '~/queries/schema' import { FunnelExclusionLegacy, NotebookNodeType, NotebookType } from '~/types' // NOTE: Increment this number when you add a new content migration @@ -102,7 +113,19 @@ function convertInsightQueriesToNewSchema(content: JSONContent[]): JSONContent[] * Insight filters */ if (query.kind === NodeKind.TrendsQuery && isLegacyTrendsFilter(query.trendsFilter as any)) { - query.trendsFilter = trendsFilterToQuery(query.trendsFilter as any) + const compareFilter = compareFilterToQuery(query.trendsFilter as any) + if (!isEmptyObject(compareFilter)) { + query.compareFilter = compareFilter + } + + delete (query.trendsFilter as TrendsFilterLegacy).compare + delete (query.trendsFilter as TrendsFilterLegacy).compare_to + + query.trendsFilter = Object.fromEntries( + Object.entries(query.trendsFilter as TrendsFilter) + .filter(([k, _]) => TRENDS_FILTER_PROPERTIES.has(k)) + .concat(Object.entries(trendsFilterToQuery(query.trendsFilter as any))) + ) } if (query.kind === NodeKind.FunnelsQuery) { @@ -127,7 +150,19 @@ function convertInsightQueriesToNewSchema(content: JSONContent[]): JSONContent[] } if (query.kind === NodeKind.StickinessQuery && isLegacyStickinessFilter(query.stickinessFilter as any)) { - query.stickinessFilter = stickinessFilterToQuery(query.stickinessFilter as any) + const compareFilter = compareFilterToQuery(query.stickinessFilter as any) + if (!isEmptyObject(compareFilter)) { + query.compareFilter = compareFilter + } + delete (query.stickinessFilter as StickinessFilterLegacy).compare + delete (query.stickinessFilter as StickinessFilterLegacy).compare_to + + // This has to come after compare, because it removes compare + query.stickinessFilter = Object.fromEntries( + Object.entries(query.stickinessFilter as StickinessFilter) + .filter(([k, _]) => STICKINESS_FILTER_PROPERTIES.has(k)) + .concat(Object.entries(stickinessFilterToQuery(query.stickinessFilter as any))) + ) } if (query.kind === NodeKind.LifecycleQuery && isLegacyLifecycleFilter(query.lifecycleFilter as any)) { diff --git a/frontend/src/scenes/surveys/surveyViewViz.tsx b/frontend/src/scenes/surveys/surveyViewViz.tsx index ba302e631e89f..603b651d3a90a 100644 --- a/frontend/src/scenes/surveys/surveyViewViz.tsx +++ b/frontend/src/scenes/surveys/surveyViewViz.tsx @@ -308,7 +308,6 @@ export function NPSSurveyResultsBarChart({ hideColorCol: true, }} trendsFilter={{ - compare: true, showLegend: true, }} datasets={[ diff --git a/frontend/src/scenes/trends/persons-modal/personsModalLogic.ts b/frontend/src/scenes/trends/persons-modal/personsModalLogic.ts index a7594451ad0fc..cf6e8bdac8c57 100644 --- a/frontend/src/scenes/trends/persons-modal/personsModalLogic.ts +++ b/frontend/src/scenes/trends/persons-modal/personsModalLogic.ts @@ -175,7 +175,7 @@ export const personsModalLogic = kea([ kind: NodeKind.InsightActorsQueryOptions, source: query, } - const response = await performQuery(optionsQuery) + const response = await performQuery(optionsQuery, { async: false }) return Object.fromEntries( Object.entries(response).filter(([key, _]) => diff --git a/frontend/src/scenes/trends/trendsDataLogic.ts b/frontend/src/scenes/trends/trendsDataLogic.ts index 51f5c9fd93123..f0bbff852f0c1 100644 --- a/frontend/src/scenes/trends/trendsDataLogic.ts +++ b/frontend/src/scenes/trends/trendsDataLogic.ts @@ -48,7 +48,7 @@ export const trendsDataLogic = kea([ 'series', 'formula', 'display', - 'compare', + 'compareFilter', 'interval', 'breakdownFilter', 'showValuesOnSeries', diff --git a/frontend/src/scenes/trends/viz/ActionsLineGraph.tsx b/frontend/src/scenes/trends/viz/ActionsLineGraph.tsx index 3da2d5728f10c..4ef86f2b01f24 100644 --- a/frontend/src/scenes/trends/viz/ActionsLineGraph.tsx +++ b/frontend/src/scenes/trends/viz/ActionsLineGraph.tsx @@ -26,7 +26,7 @@ export function ActionsLineGraph({ labelGroupType, incompletenessOffsetFromEnd, formula, - compare, + compareFilter, display, interval, showValuesOnSeries, @@ -100,7 +100,7 @@ export function ActionsLineGraph({ } : undefined } - compare={compare} + compare={!!compareFilter?.compare} isInProgress={!isStickiness && incompletenessOffsetFromEnd < 0} isArea={display === ChartDisplayType.ActionsAreaGraph} incompletenessOffsetFromEnd={incompletenessOffsetFromEnd} diff --git a/frontend/src/scenes/web-analytics/webAnalyticsLogic.ts b/frontend/src/scenes/web-analytics/webAnalyticsLogic.ts index d5962ba312861..a1d4f0fb49ee5 100644 --- a/frontend/src/scenes/web-analytics/webAnalyticsLogic.ts +++ b/frontend/src/scenes/web-analytics/webAnalyticsLogic.ts @@ -480,9 +480,11 @@ export const webAnalyticsLogic = kea([ }, ], trendsFilter: { - compare, display: ChartDisplayType.ActionsLineGraph, }, + compareFilter: { + compare: compare, + }, filterTestAccounts: true, properties: webAnalyticsFilters, }, @@ -513,9 +515,11 @@ export const webAnalyticsLogic = kea([ }, ], trendsFilter: { - compare, display: ChartDisplayType.ActionsLineGraph, }, + compareFilter: { + compare: compare, + }, filterTestAccounts: true, properties: webAnalyticsFilters, }, @@ -546,9 +550,11 @@ export const webAnalyticsLogic = kea([ }, ], trendsFilter: { - compare, display: ChartDisplayType.ActionsLineGraph, }, + compareFilter: { + compare: compare, + }, filterTestAccounts: true, properties: webAnalyticsFilters, }, diff --git a/frontend/src/types.ts b/frontend/src/types.ts index bcda304202dc2..b3cca7bd90f98 100644 --- a/frontend/src/types.ts +++ b/frontend/src/types.ts @@ -2141,8 +2141,9 @@ export interface TrendsFilterType extends FilterType { // number of intervals, e.g. for a day interval, we may want to smooth over // 7 days to remove weekly variation. Smoothing is performed as a moving average. smoothing_intervals?: number - compare?: boolean formula?: string + compare_to?: string + compare?: boolean /** @deprecated */ shown_as?: ShownAsValue display?: ChartDisplayType @@ -2161,6 +2162,7 @@ export interface TrendsFilterType extends FilterType { } export interface StickinessFilterType extends FilterType { + compare_to?: string compare?: boolean /** @deprecated */ shown_as?: ShownAsValue diff --git a/posthog/api/insight_serializers.py b/posthog/api/insight_serializers.py index d99642d4ea1fa..1406ccf8e0752 100644 --- a/posthog/api/insight_serializers.py +++ b/posthog/api/insight_serializers.py @@ -118,6 +118,14 @@ def validate(self, data): return super().validate(data) +class CompareMixin(serializers.Serializer): + compare = serializers.BooleanField(required=False, help_text="To compare or not") + compare_to = serializers.CharField( + required=False, + help_text="What to compare to", + ) + + class ResultsMixin(serializers.Serializer): is_cached = serializers.BooleanField( help_text="Whether the result is cached. To force a refresh, pass ?refresh=true" @@ -146,7 +154,7 @@ class TrendResultsSerializer(ResultsMixin): result = TrendResultSerializer(many=True) -class TrendSerializer(GenericInsightsSerializer, BreakdownMixin): +class TrendSerializer(GenericInsightsSerializer, BreakdownMixin, CompareMixin): display = serializers.ChoiceField( choices=get_args(DISPLAY_TYPES), required=False, @@ -159,10 +167,6 @@ class TrendSerializer(GenericInsightsSerializer, BreakdownMixin): help_text="Combine the result of events or actions into a single number. For example `A + B` or `(A-B)/B`. The letters correspond to the order of the `events` or `actions` lists.", allow_blank=True, ) - compare = serializers.BooleanField( - help_text="For each returned result show the current period and the previous period. The result will contain `compare:true` and a `compare_label` with either `current` or `previous`.", - required=False, - ) class FunnelExclusionSerializer(serializers.Serializer): diff --git a/posthog/constants.py b/posthog/constants.py index ff5b0d9732920..fc8f7a9142195 100644 --- a/posthog/constants.py +++ b/posthog/constants.py @@ -133,6 +133,7 @@ class AvailableFeature(str, Enum): BREAKDOWN_VALUE = "breakdown_value" BREAKDOWN_GROUP_TYPE_INDEX = "breakdown_group_type_index" COMPARE = "compare" +COMPARE_TO = "compare_to" INSIGHT = "insight" SESSION = "session" BREAKDOWN = "breakdown" diff --git a/posthog/hogql_queries/insights/stickiness_query_runner.py b/posthog/hogql_queries/insights/stickiness_query_runner.py index 45c8a26268f11..3087e4d99ee5d 100644 --- a/posthog/hogql_queries/insights/stickiness_query_runner.py +++ b/posthog/hogql_queries/insights/stickiness_query_runner.py @@ -16,6 +16,7 @@ from posthog.hogql.query import execute_hogql_query from posthog.hogql.timings import HogQLTimings from posthog.hogql_queries.query_runner import QueryRunner +from posthog.hogql_queries.utils.query_compare_to_date_range import QueryCompareToDateRange from posthog.hogql_queries.utils.query_date_range import QueryDateRange from posthog.hogql_queries.utils.query_previous_period_date_range import QueryPreviousPeriodDateRange from posthog.models import Team @@ -242,7 +243,7 @@ def calculate(self): } # Modifications for when comparing to previous period - if self.query.stickinessFilter is not None and self.query.stickinessFilter.compare: + if self.query.compareFilter is not None and self.query.compareFilter.compare: series_object["compare"] = True series_object["compare_label"] = ( "previous" if series_with_extra.is_previous_period_series else "current" @@ -355,7 +356,7 @@ def setup_series(self) -> list[SeriesWithExtras]: for series in self.query.series ] - if self.query.stickinessFilter is not None and self.query.stickinessFilter.compare: + if self.query.compareFilter is not None and self.query.compareFilter.compare: updated_series = [] for series in series_with_extras: updated_series.append( @@ -377,7 +378,6 @@ def setup_series(self) -> list[SeriesWithExtras]: def date_range(self, series: SeriesWithExtras): if series.is_previous_period_series: return self.query_previous_date_range - return self.query_date_range @cached_property @@ -391,6 +391,14 @@ def query_date_range(self): @cached_property def query_previous_date_range(self): + if self.query.compareFilter is not None and isinstance(self.query.compareFilter.compare_to, str): + return QueryCompareToDateRange( + date_range=self.query.dateRange, + team=self.team, + interval=self.query.interval, + now=datetime.now(), + compare_to=self.query.compareFilter.compare_to, + ) return QueryPreviousPeriodDateRange( date_range=self.query.dateRange, team=self.team, diff --git a/posthog/hogql_queries/insights/test/test_stickiness_query_runner.py b/posthog/hogql_queries/insights/test/test_stickiness_query_runner.py index f9a2cbc17a038..9a93bf00c9630 100644 --- a/posthog/hogql_queries/insights/test/test_stickiness_query_runner.py +++ b/posthog/hogql_queries/insights/test/test_stickiness_query_runner.py @@ -32,6 +32,7 @@ StickinessFilter, StickinessQuery, StickinessQueryResponse, + CompareFilter, ) from posthog.settings import HOGQL_INCREASED_MAX_EXECUTION_TIME from posthog.test.base import APIBaseTest, _create_event, _create_person @@ -201,6 +202,7 @@ def _run_query( filters: Optional[StickinessFilter] = None, filter_test_accounts: Optional[bool] = False, limit_context: Optional[LimitContext] = None, + compare_filters: Optional[CompareFilter] = None, ): query_series: list[EventsNode | ActionsNode] = [EventsNode(event="$pageview")] if series is None else series query_date_from = date_from or self.default_date_from @@ -213,6 +215,7 @@ def _run_query( interval=query_interval, properties=properties, stickinessFilter=filters, + compareFilter=compare_filters, filterTestAccounts=filter_test_accounts, ) return StickinessQueryRunner(team=self.team, query=query, limit_context=limit_context).calculate() @@ -509,7 +512,7 @@ def test_actions(self): def test_compare(self): self._create_test_events() - response = self._run_query(filters=StickinessFilter(compare=True)) + response = self._run_query(filters=StickinessFilter(), compare_filters=CompareFilter(compare=True)) assert response.results[0]["count"] == 2 assert response.results[0]["compare_label"] == "current" @@ -517,6 +520,24 @@ def test_compare(self): assert response.results[1]["count"] == 0 assert response.results[1]["compare_label"] == "previous" + def test_compare_to(self): + self._create_test_events() + + response = self._run_query( + date_from="2020-01-12", + date_to="2020-01-20", + filters=StickinessFilter(), + compare_filters=CompareFilter(compare=True, compare_to="-1d"), + ) + + assert response.results[0]["count"] == 2 + assert response.results[0]["compare_label"] == "current" + assert response.results[0]["data"] == [0, 0, 0, 1, 0, 0, 0, 1, 0] + + assert response.results[1]["count"] == 2 + assert response.results[1]["compare_label"] == "previous" + assert response.results[1]["data"] == [0, 0, 0, 0, 1, 0, 0, 0, 1] + def test_filter_test_accounts(self): self._create_test_events() diff --git a/posthog/hogql_queries/insights/trends/test/test_trends.py b/posthog/hogql_queries/insights/trends/test/test_trends.py index 085155158221e..26a1b7f1a6d4f 100644 --- a/posthog/hogql_queries/insights/trends/test/test_trends.py +++ b/posthog/hogql_queries/insights/trends/test/test_trends.py @@ -52,6 +52,7 @@ PropertyGroupFilter, TrendsFilter, TrendsQuery, + CompareFilter, ) from posthog.test.base import ( APIBaseTest, @@ -193,10 +194,10 @@ def convert_filter_to_trends_query(filter: Filter) -> TrendsQuery: trendsFilter=TrendsFilter( display=filter.display, breakdown_histogram_bin_count=filter.breakdown_histogram_bin_count, - compare=filter.compare, formula=filter.formula, smoothingIntervals=filter.smoothing_intervals, ), + compareFilter=CompareFilter(compare=filter.compare, compare_to=filter.compare_to), ) return tq diff --git a/posthog/hogql_queries/insights/trends/test/test_trends_actors_query_builder.py b/posthog/hogql_queries/insights/trends/test/test_trends_actors_query_builder.py index ab9761f445441..fd95febddf61a 100644 --- a/posthog/hogql_queries/insights/trends/test/test_trends_actors_query_builder.py +++ b/posthog/hogql_queries/insights/trends/test/test_trends_actors_query_builder.py @@ -19,6 +19,7 @@ IntervalType, TrendsFilter, TrendsQuery, + CompareFilter, ) from posthog.test.base import BaseTest @@ -111,7 +112,7 @@ def test_date_range_hourly(self): def test_date_range_compare_previous(self): self.team.timezone = "Europe/Berlin" - trends_query = default_query.model_copy(update={"trendsFilter": TrendsFilter(compare=True)}, deep=True) + trends_query = default_query.model_copy(update={"compareFilter": CompareFilter(compare=True)}, deep=True) self.assertEqual( self._get_date_where_sql(trends_query=trends_query, time_frame="2023-05-10", compare_value=Compare.CURRENT), @@ -127,7 +128,7 @@ def test_date_range_compare_previous(self): def test_date_range_compare_previous_hourly(self): self.team.timezone = "Europe/Berlin" trends_query = default_query.model_copy( - update={"trendsFilter": TrendsFilter(compare=True), "interval": IntervalType.HOUR}, deep=True + update={"compareFilter": CompareFilter(compare=True), "interval": IntervalType.HOUR}, deep=True ) self.assertEqual( self._get_date_where_sql( @@ -142,6 +143,40 @@ def test_date_range_compare_previous_hourly(self): "greaterOrEquals(timestamp, toDateTime('2023-05-03 13:00:00.000000')), less(timestamp, toDateTime('2023-05-03 14:00:00.000000'))", ) + def test_date_range_compare_to(self): + self.team.timezone = "Europe/Berlin" + trends_query = default_query.model_copy( + update={"compareFilter": CompareFilter(compare=True, compare_to="-3d")}, deep=True + ) + + self.assertEqual( + self._get_date_where_sql(trends_query=trends_query, time_frame="2023-05-10", compare_value=Compare.CURRENT), + "greaterOrEquals(timestamp, toDateTime('2023-05-09 22:00:00.000000')), less(timestamp, toDateTime('2023-05-10 22:00:00.000000'))", + ) + self.assertEqual( + self._get_date_where_sql( + trends_query=trends_query, time_frame="2023-05-10", compare_value=Compare.PREVIOUS + ), + "greaterOrEquals(timestamp, toDateTime('2023-05-06 22:00:00.000000')), less(timestamp, toDateTime('2023-05-07 22:00:00.000000'))", + ) + + def test_date_range_compare_to_hours(self): + self.team.timezone = "Europe/Berlin" + trends_query = default_query.model_copy( + update={"compareFilter": CompareFilter(compare=True, compare_to="-3h")}, deep=True + ) + + self.assertEqual( + self._get_date_where_sql(trends_query=trends_query, time_frame="2023-05-10", compare_value=Compare.CURRENT), + "greaterOrEquals(timestamp, toDateTime('2023-05-09 22:00:00.000000')), less(timestamp, toDateTime('2023-05-10 22:00:00.000000'))", + ) + self.assertEqual( + self._get_date_where_sql( + trends_query=trends_query, time_frame="2023-05-10", compare_value=Compare.PREVIOUS + ), + "greaterOrEquals(timestamp, toDateTime('2023-05-09 19:00:00.000000')), less(timestamp, toDateTime('2023-05-10 19:00:00.000000'))", + ) + def test_date_range_total_value(self): self.team.timezone = "Europe/Berlin" trends_query = default_query.model_copy( @@ -157,7 +192,11 @@ def test_date_range_total_value(self): def test_date_range_total_value_compare_previous(self): self.team.timezone = "Europe/Berlin" trends_query = default_query.model_copy( - update={"trendsFilter": TrendsFilter(display=ChartDisplayType.BOLD_NUMBER, compare=True)}, deep=True + update={ + "trendsFilter": TrendsFilter(display=ChartDisplayType.BOLD_NUMBER), + "compareFilter": CompareFilter(compare=True), + }, + deep=True, ) with freeze_time("2022-06-15T12:00:00.000Z"): @@ -170,6 +209,26 @@ def test_date_range_total_value_compare_previous(self): "greaterOrEquals(timestamp, toDateTime('2022-05-31 22:00:00.000000')), lessOrEquals(timestamp, toDateTime('2022-06-08 21:59:59.999999'))", ) + def test_date_range_total_value_compare_to(self): + self.team.timezone = "Europe/Berlin" + trends_query = default_query.model_copy( + update={ + "trendsFilter": TrendsFilter(display=ChartDisplayType.BOLD_NUMBER), + "compareFilter": CompareFilter(compare=True, compare_to="-3d"), + }, + deep=True, + ) + + with freeze_time("2022-06-15T12:00:00.000Z"): + self.assertEqual( + self._get_date_where_sql(trends_query=trends_query, compare_value=Compare.CURRENT), + "greaterOrEquals(timestamp, toDateTime('2022-06-07 22:00:00.000000')), lessOrEquals(timestamp, toDateTime('2022-06-15 21:59:59.999999'))", + ) + self.assertEqual( + self._get_date_where_sql(trends_query=trends_query, compare_value=Compare.PREVIOUS), + "greaterOrEquals(timestamp, toDateTime('2022-06-04 22:00:00.000000')), lessOrEquals(timestamp, toDateTime('2022-06-12 21:59:59.999999'))", + ) + def test_date_range_weekly_active_users_math(self): self.team.timezone = "Europe/Berlin" trends_query = default_query.model_copy( @@ -187,7 +246,7 @@ def test_date_range_weekly_active_users_math_compare_previous(self): trends_query = default_query.model_copy( update={ "series": [EventsNode(event="$pageview", math=BaseMathType.WEEKLY_ACTIVE)], - "trendsFilter": TrendsFilter(compare=True), + "compareFilter": CompareFilter(compare=True), }, deep=True, ) @@ -206,6 +265,30 @@ def test_date_range_weekly_active_users_math_compare_previous(self): "greaterOrEquals(timestamp, minus(toDateTime('2024-05-19 22:00:00.000000'), toIntervalDay(6))), less(timestamp, toDateTime('2024-05-20 22:00:00.000000'))", ) + def test_date_range_weekly_active_users_math_compare_to(self): + self.team.timezone = "Europe/Berlin" + trends_query = default_query.model_copy( + update={ + "series": [EventsNode(event="$pageview", math=BaseMathType.WEEKLY_ACTIVE)], + "compareFilter": CompareFilter(compare=True, compare_to="-3d"), + }, + deep=True, + ) + + with freeze_time("2024-05-30T12:00:00.000Z"): + self.assertEqual( + self._get_date_where_sql( + trends_query=trends_query, time_frame="2024-05-27", compare_value=Compare.CURRENT + ), + "greaterOrEquals(timestamp, minus(toDateTime('2024-05-26 22:00:00.000000'), toIntervalDay(6))), less(timestamp, toDateTime('2024-05-27 22:00:00.000000'))", + ) + self.assertEqual( + self._get_date_where_sql( + trends_query=trends_query, time_frame="2024-05-27", compare_value=Compare.PREVIOUS + ), + "greaterOrEquals(timestamp, minus(toDateTime('2024-05-23 22:00:00.000000'), toIntervalDay(6))), less(timestamp, toDateTime('2024-05-24 22:00:00.000000'))", + ) + def test_date_range_weekly_active_users_math_total_value(self): self.team.timezone = "Europe/Berlin" trends_query = default_query.model_copy( @@ -227,7 +310,8 @@ def test_date_range_weekly_active_users_math_total_value_compare_previous(self): trends_query = default_query.model_copy( update={ "series": [EventsNode(event="$pageview", math=BaseMathType.WEEKLY_ACTIVE)], - "trendsFilter": TrendsFilter(compare=True, display=ChartDisplayType.BOLD_NUMBER), + "trendsFilter": TrendsFilter(display=ChartDisplayType.BOLD_NUMBER), + "compareFilter": CompareFilter(compare=True), }, deep=True, ) diff --git a/posthog/hogql_queries/insights/trends/test/test_trends_dashboard_filters.py b/posthog/hogql_queries/insights/trends/test/test_trends_dashboard_filters.py index db0879c188fac..2443e0b4975ad 100644 --- a/posthog/hogql_queries/insights/trends/test/test_trends_dashboard_filters.py +++ b/posthog/hogql_queries/insights/trends/test/test_trends_dashboard_filters.py @@ -16,6 +16,7 @@ PropertyGroupFilterValue, TrendsFilter, TrendsQuery, + CompareFilter, ) from posthog.test.base import BaseTest @@ -35,6 +36,7 @@ def _create_query_runner( hogql_modifiers: Optional[HogQLQueryModifiers] = None, limit_context: Optional[LimitContext] = None, explicit_date: Optional[bool] = None, + compare_filters: Optional[CompareFilter] = None, ) -> TrendsQueryRunner: query_series: list[EventsNode | ActionsNode] = [EventsNode(event="$pageview")] if series is None else series query = TrendsQuery( @@ -45,6 +47,7 @@ def _create_query_runner( breakdownFilter=breakdown, filterTestAccounts=filter_test_accounts, properties=properties, + compareFilter=compare_filters, ) return TrendsQueryRunner(team=self.team, query=query, modifiers=hogql_modifiers, limit_context=limit_context) @@ -298,7 +301,8 @@ def test_compare_is_removed_for_all_time_range(self): "2024-07-14", IntervalType.DAY, None, - trends_filters=TrendsFilter(compare=True), + trends_filters=TrendsFilter(), + compare_filters=CompareFilter(compare=True), ) assert query_runner.query.dateRange is not None @@ -306,7 +310,8 @@ def test_compare_is_removed_for_all_time_range(self): assert query_runner.query.dateRange.date_to == "2024-07-14" assert query_runner.query.properties is None assert query_runner.query.breakdownFilter is None - assert query_runner.query.trendsFilter == TrendsFilter(compare=True) + assert query_runner.query.trendsFilter == TrendsFilter() + assert query_runner.query.compareFilter == CompareFilter(compare=True) query_runner.apply_dashboard_filters(DashboardFilter(date_from="all")) @@ -314,6 +319,7 @@ def test_compare_is_removed_for_all_time_range(self): assert query_runner.query.dateRange.date_to is None assert query_runner.query.properties is None # type: ignore [unreachable] assert query_runner.query.breakdownFilter is None - assert query_runner.query.trendsFilter == TrendsFilter( - compare=False # There's no previous period for the "all time" date range - ) + assert query_runner.query.trendsFilter == TrendsFilter() + assert query_runner.query.compareFilter == CompareFilter( + compare=False + ) # There's no previous period for the "all time" date range diff --git a/posthog/hogql_queries/insights/trends/test/test_trends_persons.py b/posthog/hogql_queries/insights/trends/test/test_trends_persons.py index 34cde171dfa4f..1751c7482dd40 100644 --- a/posthog/hogql_queries/insights/trends/test/test_trends_persons.py +++ b/posthog/hogql_queries/insights/trends/test/test_trends_persons.py @@ -24,6 +24,7 @@ PropertyMathType, TrendsFilter, TrendsQuery, + CompareFilter, HogQLQueryModifiers, ) from posthog.test.base import APIBaseTest, ClickhouseTestMixin, _create_event, _create_person @@ -604,7 +605,8 @@ def test_trends_compare_persons(self): source_query = TrendsQuery( series=[EventsNode(event="$pageview")], dateRange=InsightDateRange(date_from="-7d"), - trendsFilter=TrendsFilter(compare=True), + trendsFilter=TrendsFilter(), + compareFilter=CompareFilter(compare=True), ) result = self._get_actors(trends_query=source_query, day="2023-05-06", compare=Compare.CURRENT) diff --git a/posthog/hogql_queries/insights/trends/test/test_trends_query_runner.py b/posthog/hogql_queries/insights/trends/test/test_trends_query_runner.py index 98a75f49d4a6a..c87d55643d771 100644 --- a/posthog/hogql_queries/insights/trends/test/test_trends_query_runner.py +++ b/posthog/hogql_queries/insights/trends/test/test_trends_query_runner.py @@ -32,6 +32,7 @@ PropertyMathType, TrendsFilter, TrendsQuery, + CompareFilter, ) from posthog.schema import Series as InsightActorsQuerySeries @@ -177,6 +178,7 @@ def _create_query_runner( series: Optional[list[EventsNode | ActionsNode]], trends_filters: Optional[TrendsFilter] = None, breakdown: Optional[BreakdownFilter] = None, + compare_filters: Optional[CompareFilter] = None, filter_test_accounts: Optional[bool] = None, hogql_modifiers: Optional[HogQLQueryModifiers] = None, limit_context: Optional[LimitContext] = None, @@ -189,6 +191,7 @@ def _create_query_runner( series=query_series, trendsFilter=trends_filters, breakdownFilter=breakdown, + compareFilter=compare_filters, filterTestAccounts=filter_test_accounts, ) return TrendsQueryRunner(team=self.team, query=query, modifiers=hogql_modifiers, limit_context=limit_context) @@ -201,6 +204,7 @@ def _run_trends_query( series: Optional[list[EventsNode | ActionsNode]], trends_filters: Optional[TrendsFilter] = None, breakdown: Optional[BreakdownFilter] = None, + compare_filters: Optional[CompareFilter] = None, *, filter_test_accounts: Optional[bool] = None, hogql_modifiers: Optional[HogQLQueryModifiers] = None, @@ -213,6 +217,7 @@ def _run_trends_query( series=series, trends_filters=trends_filters, breakdown=breakdown, + compare_filters=compare_filters, filter_test_accounts=filter_test_accounts, hogql_modifiers=hogql_modifiers, limit_context=limit_context, @@ -400,7 +405,8 @@ def test_formula_with_compare(self): "2020-01-19", IntervalType.DAY, [EventsNode(event="$pageview"), EventsNode(event="$pageleave")], - TrendsFilter(formula="A+2*B", compare=True), + TrendsFilter(formula="A+2*B"), + compare_filters=CompareFilter(compare=True), ) # one for current, one for previous @@ -420,6 +426,64 @@ def test_formula_with_compare(self): self.assertEqual("Formula (A+2*B)", response.results[0]["label"]) self.assertEqual(True, response.results[0]["compare"]) + def test_formula_with_compare_to_day(self): + self._create_test_events() + + response = self._run_trends_query( + "2020-01-15", + "2020-01-19", + IntervalType.DAY, + [EventsNode(event="$pageview"), EventsNode(event="$pageleave")], + TrendsFilter(formula="A+2*B"), + compare_filters=CompareFilter(compare=True, compare_to="-2d"), + ) + + # one for current, one for previous + self.assertEqual(2, len(response.results)) + + # current + self.assertEqual("current", response.results[0]["compare_label"]) + self.assertEqual(6, response.results[0]["count"]) + self.assertEqual([2, 2, 1, 0, 1], response.results[0]["data"]) + + # previous + self.assertEqual("previous", response.results[1]["compare_label"]) + self.assertEqual(12, response.results[1]["count"]) + self.assertEqual([7, 0, 2, 2, 1], response.results[1]["data"]) + + # response shape + self.assertEqual("Formula (A+2*B)", response.results[0]["label"]) + self.assertEqual(True, response.results[0]["compare"]) + + def test_formula_with_compare_to_week(self): + self._create_test_events() + + response = self._run_trends_query( + "2020-01-15", + "2020-01-19", + IntervalType.DAY, + [EventsNode(event="$pageview"), EventsNode(event="$pageleave")], + TrendsFilter(formula="A+2*B"), + compare_filters=CompareFilter(compare=True, compare_to="-1w"), + ) + + # one for current, one for previous + self.assertEqual(2, len(response.results)) + + # current + self.assertEqual("current", response.results[0]["compare_label"]) + self.assertEqual(6, response.results[0]["count"]) + self.assertEqual([2, 2, 1, 0, 1], response.results[0]["data"]) + + # previous + self.assertEqual("previous", response.results[1]["compare_label"]) + self.assertEqual(9, response.results[1]["count"]) + self.assertEqual([0, 1, 0, 3, 5], response.results[1]["data"]) + + # response shape + self.assertEqual("Formula (A+2*B)", response.results[0]["label"]) + self.assertEqual(True, response.results[0]["compare"]) + def test_formula_with_compare_total_value(self): self._create_test_events() @@ -431,8 +495,8 @@ def test_formula_with_compare_total_value(self): TrendsFilter( formula="A+2*B", display=ChartDisplayType.BOLD_NUMBER, # total value - compare=True, ), + compare_filters=CompareFilter(compare=True), ) # one for current, one for previous @@ -451,6 +515,37 @@ def test_formula_with_compare_total_value(self): self.assertEqual(0, response.results[0]["count"]) # it has always been so :shrug: self.assertEqual(None, response.results[0].get("data")) + def test_formula_with_compare_to_total_value(self): + self._create_test_events() + + response = self._run_trends_query( + "2020-01-15", + "2020-01-19", + IntervalType.DAY, + [EventsNode(event="$pageview"), EventsNode(event="$pageleave")], + TrendsFilter( + formula="A+2*B", + display=ChartDisplayType.BOLD_NUMBER, # total value + ), + compare_filters=CompareFilter(compare=True, compare_to="-1w"), + ) + + # one for current, one for previous + self.assertEqual(2, len(response.results)) + + # current + self.assertEqual("current", response.results[0]["compare_label"]) + self.assertEqual(6, response.results[0]["aggregated_value"]) + + # previous + self.assertEqual("previous", response.results[1]["compare_label"]) + self.assertEqual(9, response.results[1]["aggregated_value"]) + + # response shape + self.assertEqual("Formula (A+2*B)", response.results[0]["label"]) + self.assertEqual(0, response.results[0]["count"]) # it has always been so :shrug: + self.assertEqual(None, response.results[0].get("data")) + def test_formula_with_breakdown(self): self._create_test_events() @@ -487,8 +582,9 @@ def test_formula_with_breakdown_and_compare(self): "2020-01-19", IntervalType.DAY, [EventsNode(event="$pageview"), EventsNode(event="$pageleave")], - TrendsFilter(formula="A+2*B", compare=True), + TrendsFilter(formula="A+2*B"), BreakdownFilter(breakdown_type=BreakdownType.EVENT, breakdown="$browser"), + CompareFilter(compare=True), ) # chrome, ff and edge for previous, and chrome and safari for current @@ -509,6 +605,38 @@ def test_formula_with_breakdown_and_compare(self): assert response.results[2]["breakdown_value"] == "Chrome" assert response.results[2]["count"] == 9 + def test_formula_with_breakdown_and_compare_to(self): + self._create_test_events() + + response = self._run_trends_query( + "2020-01-15", + "2020-01-19", + IntervalType.DAY, + [EventsNode(event="$pageview"), EventsNode(event="$pageleave")], + TrendsFilter(formula="A+2*B"), + BreakdownFilter(breakdown_type=BreakdownType.EVENT, breakdown="$browser"), + CompareFilter(compare=True, compare_to="-3d"), + ) + + # chrome, ff, edge and safari for previous, and chrome and safari for current + assert len(response.results) == 6 + + assert response.results[0]["compare_label"] == "current" + assert response.results[0]["breakdown_value"] == "Chrome" + assert response.results[0]["label"] == "Formula (A+2*B)" + assert response.results[0]["count"] == 3 + assert response.results[0]["data"] == [1, 0, 1, 0, 1] + + assert response.results[1]["compare_label"] == "current" + assert response.results[1]["breakdown_value"] == "Safari" + assert response.results[1]["count"] == 3 + + assert response.results[2]["compare_label"] == "previous" + assert response.results[2]["label"] == "Formula (A+2*B)" + assert response.results[2]["breakdown_value"] == "Chrome" + assert response.results[2]["count"] == 7 + assert response.results[2]["data"] == [3, 3, 0, 1, 0] + def test_formula_with_breakdown_and_compare_total_value(self): self._create_test_events() @@ -520,9 +648,9 @@ def test_formula_with_breakdown_and_compare_total_value(self): TrendsFilter( formula="A+2*B", display=ChartDisplayType.BOLD_NUMBER, # total value - compare=True, ), BreakdownFilter(breakdown_type=BreakdownType.EVENT, breakdown="$browser"), + CompareFilter(compare=True), ) # chrome, ff and edge for previous, and chrome and safari for current @@ -695,7 +823,8 @@ def test_trends_compare(self): "2020-01-19", IntervalType.DAY, [EventsNode(event="$pageview")], - TrendsFilter(compare=True), + TrendsFilter(), + compare_filters=CompareFilter(compare=True), ) self.assertEqual(2, len(response.results)) @@ -739,7 +868,8 @@ def test_trends_compare_weeks(self): None, IntervalType.DAY, [EventsNode(event="$pageview")], - TrendsFilter(compare=True), + TrendsFilter(), + compare_filters=CompareFilter(compare=True), ) self.assertEqual(2, len(response.results)) @@ -976,8 +1106,9 @@ def test_trends_breakdowns_and_compare(self): "2020-01-20", IntervalType.DAY, [EventsNode(event="$pageview")], - TrendsFilter(compare=True), + TrendsFilter(), BreakdownFilter(breakdown_type=BreakdownType.EVENT, breakdown="$browser"), + CompareFilter(compare=True), ) breakdown_labels = [result["breakdown_value"] for result in response.results] @@ -1438,8 +1569,9 @@ def test_previous_period_with_number_display(self): "2020-01-20", IntervalType.DAY, [EventsNode(event="$pageview")], - TrendsFilter(display=ChartDisplayType.BOLD_NUMBER, compare=True), + TrendsFilter(display=ChartDisplayType.BOLD_NUMBER), None, + compare_filters=CompareFilter(compare=True), ) assert len(response.results) == 2 @@ -1754,8 +1886,48 @@ def test_to_actors_query_options_compare(self): "2020-01-20", IntervalType.DAY, [EventsNode(event="$pageview")], - TrendsFilter(compare=True), + TrendsFilter(), + None, + CompareFilter(compare=True), + ) + response = runner.to_actors_query_options() + + assert response.day == [ + DayItem(label="9-Jan-2020", value=datetime(2020, 1, 9, 0, 0, tzinfo=zoneinfo.ZoneInfo(key="UTC"))), + DayItem(label="10-Jan-2020", value=datetime(2020, 1, 10, 0, 0, tzinfo=zoneinfo.ZoneInfo(key="UTC"))), + DayItem(label="11-Jan-2020", value=datetime(2020, 1, 11, 0, 0, tzinfo=zoneinfo.ZoneInfo(key="UTC"))), + DayItem(label="12-Jan-2020", value=datetime(2020, 1, 12, 0, 0, tzinfo=zoneinfo.ZoneInfo(key="UTC"))), + DayItem(label="13-Jan-2020", value=datetime(2020, 1, 13, 0, 0, tzinfo=zoneinfo.ZoneInfo(key="UTC"))), + DayItem(label="14-Jan-2020", value=datetime(2020, 1, 14, 0, 0, tzinfo=zoneinfo.ZoneInfo(key="UTC"))), + DayItem(label="15-Jan-2020", value=datetime(2020, 1, 15, 0, 0, tzinfo=zoneinfo.ZoneInfo(key="UTC"))), + DayItem(label="16-Jan-2020", value=datetime(2020, 1, 16, 0, 0, tzinfo=zoneinfo.ZoneInfo(key="UTC"))), + DayItem(label="17-Jan-2020", value=datetime(2020, 1, 17, 0, 0, tzinfo=zoneinfo.ZoneInfo(key="UTC"))), + DayItem(label="18-Jan-2020", value=datetime(2020, 1, 18, 0, 0, tzinfo=zoneinfo.ZoneInfo(key="UTC"))), + DayItem(label="19-Jan-2020", value=datetime(2020, 1, 19, 0, 0, tzinfo=zoneinfo.ZoneInfo(key="UTC"))), + DayItem(label="20-Jan-2020", value=datetime(2020, 1, 20, 0, 0, tzinfo=zoneinfo.ZoneInfo(key="UTC"))), + ] + + assert response.breakdown is None + + assert response.series == [InsightActorsQuerySeries(label="$pageview", value=0)] + + assert response.compare == [ + CompareItem(label="Current", value="current"), + CompareItem(label="Previous", value="previous"), + ] + + def test_to_actors_query_options_compare_to(self): + self._create_test_events() + flush_persons_and_events() + + runner = self._create_query_runner( + "2020-01-09", + "2020-01-20", + IntervalType.DAY, + [EventsNode(event="$pageview")], + TrendsFilter(), None, + compare_filters=CompareFilter(compare=True, compare_to="-1w"), ) response = runner.to_actors_query_options() diff --git a/posthog/hogql_queries/insights/trends/trends_actors_query_builder.py b/posthog/hogql_queries/insights/trends/trends_actors_query_builder.py index c29be46cc791c..2d404f0a8c092 100644 --- a/posthog/hogql_queries/insights/trends/trends_actors_query_builder.py +++ b/posthog/hogql_queries/insights/trends/trends_actors_query_builder.py @@ -1,3 +1,4 @@ +import typing from datetime import datetime from functools import cached_property from typing import Optional @@ -13,6 +14,7 @@ from posthog.hogql_queries.insights.trends.aggregation_operations import AggregationOperations from posthog.hogql_queries.insights.trends.breakdown import Breakdown from posthog.hogql_queries.insights.trends.display import TrendsDisplay +from posthog.hogql_queries.utils.query_compare_to_date_range import QueryCompareToDateRange from posthog.hogql_queries.utils.query_date_range import QueryDateRange from posthog.hogql_queries.utils.query_previous_period_date_range import QueryPreviousPeriodDateRange from posthog.models import Action, Team @@ -25,6 +27,7 @@ HogQLQueryModifiers, TrendsFilter, TrendsQuery, + CompareFilter, ) @@ -92,7 +95,15 @@ def trends_date_range(self) -> QueryDateRange: ) @cached_property - def trends_previous_date_range(self) -> QueryPreviousPeriodDateRange: + def trends_previous_date_range(self) -> QueryPreviousPeriodDateRange | QueryCompareToDateRange: + if self.is_compare_to: + return QueryCompareToDateRange( + date_range=self.trends_query.dateRange, + team=self.team, + interval=self.trends_query.interval, + now=datetime.now(), + compare_to=typing.cast(str, typing.cast(CompareFilter, self.trends_query.compareFilter).compare_to), + ) return QueryPreviousPeriodDateRange( date_range=self.trends_query.dateRange, team=self.team, @@ -118,7 +129,14 @@ def trends_aggregation_operations(self) -> AggregationOperations: @cached_property def is_compare_previous(self) -> bool: return ( - bool(self.trends_query.trendsFilter and self.trends_query.trendsFilter.compare) + bool(self.trends_query.compareFilter and self.trends_query.compareFilter.compare) + and self.compare_value == Compare.PREVIOUS + ) + + @cached_property + def is_compare_to(self) -> bool: + return ( + bool(self.trends_query.compareFilter and isinstance(self.trends_query.compareFilter.compare_to, str)) and self.compare_value == Compare.PREVIOUS ) @@ -253,9 +271,12 @@ def _prop_where_expr(self) -> list[ast.Expr]: def _date_where_expr(self) -> list[ast.Expr]: # types - date_range: QueryDateRange = ( - self.trends_previous_date_range if self.is_compare_previous else self.trends_date_range - ) + date_range: QueryDateRange | QueryCompareToDateRange | QueryPreviousPeriodDateRange + if self.is_compare_previous: + date_range = self.trends_previous_date_range + else: + date_range = self.trends_date_range + query_from, query_to = date_range.date_from(), date_range.date_to() actors_from: datetime actors_from_expr: ast.Expr @@ -280,12 +301,15 @@ def _date_where_expr(self) -> list[ast.Expr]: # use previous day/week/... for time_frame if self.is_compare_previous: - relative_delta = relativedelta(**date_range.date_from_delta_mappings()) # type: ignore - previous_time_frame = self.time_frame - relative_delta - if self.is_hourly: - self.time_frame = previous_time_frame + if self.is_compare_to: + self.time_frame = query_from + (self.time_frame - self.trends_date_range.date_from()) else: - self.time_frame = previous_time_frame.replace(hour=0, minute=0, second=0, microsecond=0) + relative_delta = relativedelta(**date_range.date_from_delta_mappings()) # type: ignore + previous_time_frame = self.time_frame - relative_delta + if self.is_hourly: + self.time_frame = previous_time_frame + else: + self.time_frame = previous_time_frame.replace(hour=0, minute=0, second=0, microsecond=0) actors_from = self.time_frame actors_to = actors_from + date_range.interval_relativedelta() diff --git a/posthog/hogql_queries/insights/trends/trends_query_runner.py b/posthog/hogql_queries/insights/trends/trends_query_runner.py index d64e80a38330a..f44268d296e03 100644 --- a/posthog/hogql_queries/insights/trends/trends_query_runner.py +++ b/posthog/hogql_queries/insights/trends/trends_query_runner.py @@ -33,6 +33,7 @@ from posthog.hogql_queries.insights.trends.series_with_extras import SeriesWithExtras from posthog.hogql_queries.query_runner import QueryRunner from posthog.hogql_queries.utils.formula_ast import FormulaAST +from posthog.hogql_queries.utils.query_compare_to_date_range import QueryCompareToDateRange from posthog.hogql_queries.utils.query_date_range import QueryDateRange from posthog.hogql_queries.utils.query_previous_period_date_range import ( QueryPreviousPeriodDateRange, @@ -206,7 +207,7 @@ def to_actors_query_options(self) -> InsightActorsQueryOptionsResponse: res_series.append(Series(label="All events" if series_label is None else series_label, value=index)) # Compare - if self.query.trendsFilter is not None and self.query.trendsFilter.compare: + if self.query.compareFilter is not None and self.query.compareFilter.compare: res_compare = [ CompareItem(label="Current", value="current"), CompareItem(label="Previous", value="previous"), @@ -360,7 +361,7 @@ def run(index: int, query: ast.SelectQuery | ast.SelectUnionQuery, is_parallel: and self.query.trendsFilter.formula != "" ): with self.timings.measure("apply_formula"): - has_compare = bool(self.query.trendsFilter and self.query.trendsFilter.compare) + has_compare = bool(self.query.compareFilter and self.query.compareFilter.compare) if has_compare: current_results = returned_results[: len(returned_results) // 2] previous_results = returned_results[len(returned_results) // 2 :] @@ -398,7 +399,7 @@ def get_value(name: str, val: Any): return val[index] real_series_count = series_count - if self.query.trendsFilter is not None and self.query.trendsFilter.compare: + if self.query.compareFilter is not None and self.query.compareFilter.compare: real_series_count = ceil(series_count / 2) res = [] @@ -480,7 +481,7 @@ def get_value(name: str, val: Any): } # Modifications for when comparing to previous period - if self.query.trendsFilter is not None and self.query.trendsFilter.compare: + if self.query.compareFilter is not None and self.query.compareFilter.compare: labels = [ "{} {}".format( self.query.interval if self.query.interval is not None else "day", @@ -565,6 +566,14 @@ def query_date_range(self): @cached_property def query_previous_date_range(self): + if self.query.compareFilter is not None and isinstance(self.query.compareFilter.compare_to, str): + return QueryCompareToDateRange( + date_range=self.query.dateRange, + team=self.team, + interval=self.query.interval, + now=datetime.now(), + compare_to=self.query.compareFilter.compare_to, + ) return QueryPreviousPeriodDateRange( date_range=self.query.dateRange, team=self.team, @@ -652,7 +661,7 @@ def setup_series(self) -> list[SeriesWithExtras]: ) series_with_extras = updated_series - if self.query.trendsFilter is not None and self.query.trendsFilter.compare: + if self.query.compareFilter is not None and self.query.compareFilter.compare: updated_series = [] for series in series_with_extras: updated_series.append( @@ -674,6 +683,7 @@ def setup_series(self) -> list[SeriesWithExtras]: aggregate_values=self._trends_display.is_total_value(), ) ) + series_with_extras = updated_series return series_with_extras @@ -681,7 +691,7 @@ def setup_series(self) -> list[SeriesWithExtras]: def apply_formula( self, formula: str, results: list[list[dict[str, Any]]], in_breakdown_clause=False ) -> list[dict[str, Any]]: - has_compare = bool(self.query.trendsFilter and self.query.trendsFilter.compare) + has_compare = bool(self.query.compareFilter and self.query.compareFilter.compare) has_breakdown = bool(self.query.breakdownFilter and self.query.breakdownFilter.breakdown) is_total_value = self._trends_display.is_total_value() @@ -905,10 +915,10 @@ def apply_dashboard_filters(self, dashboard_filter: DashboardFilter): self.query.breakdownFilter.breakdown_limit = None if ( - self.query.trendsFilter is not None - and self.query.trendsFilter.compare + self.query.compareFilter is not None + and self.query.compareFilter.compare and dashboard_filter.date_from == "all" ): # TODO: Move this "All time" range handling out of `apply_dashboard_filters` – if the date range is "all", # we should disable `compare` _no matter how_ we arrived at the final executed query - self.query.trendsFilter.compare = False + self.query.compareFilter.compare = False diff --git a/posthog/hogql_queries/legacy_compatibility/filter_to_query.py b/posthog/hogql_queries/legacy_compatibility/filter_to_query.py index cad685ec9675e..996e10173814e 100644 --- a/posthog/hogql_queries/legacy_compatibility/filter_to_query.py +++ b/posthog/hogql_queries/legacy_compatibility/filter_to_query.py @@ -28,6 +28,7 @@ TrendsFilter, TrendsQuery, FunnelVizType, + CompareFilter, ) from posthog.types import InsightQueryNode from posthog.utils import str_to_bool @@ -291,6 +292,21 @@ def _breakdown_filter(_filter: dict): return {"breakdownFilter": BreakdownFilter(**breakdownFilter)} +def _compare_filter(_filter: dict): + if _insight_type(_filter) != "TRENDS" and _insight_type(_filter) != "STICKINESS": + return {} + + compareFilter = { + "compare": _filter.get("compare"), + "compare_to": _filter.get("compare_to"), + } + + if len(CompareFilter(**compareFilter).model_dump(exclude_defaults=True)) == 0: + return {} + + return {"compareFilter": CompareFilter(**compareFilter)} + + def _group_aggregation_filter(filter: dict): if _insight_type(filter) == "STICKINESS" or _insight_type(filter) == "LIFECYCLE": return {} @@ -304,7 +320,6 @@ def _insight_filter(filter: dict): smoothingIntervals=filter.get("smoothing_intervals"), showLegend=filter.get("show_legend"), # hidden_legend_indexes=cleanHiddenLegendIndexes(filter.get('hidden_legend_keys')), - compare=filter.get("compare"), aggregationAxisFormat=filter.get("aggregation_axis_format"), aggregationAxisPrefix=filter.get("aggregation_axis_prefix"), aggregationAxisPostfix=filter.get("aggregation_axis_postfix"), @@ -390,7 +405,6 @@ def _insight_filter(filter: dict): elif _insight_type(filter) == "STICKINESS": insight_filter = { "stickinessFilter": StickinessFilter( - compare=filter.get("compare"), showLegend=filter.get("show_legend"), # hidden_legend_indexes: cleanHiddenLegendIndexes(filter.get('hidden_legend_keys')), showValuesOnSeries=filter.get("show_values_on_series"), @@ -441,6 +455,7 @@ def filter_to_query(filter: dict) -> InsightQueryNode: **_filter_test_accounts(filter), **_properties(filter), **_breakdown_filter(filter), + **_compare_filter(filter), **_group_aggregation_filter(filter), **_insight_filter(filter), } diff --git a/posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py b/posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py index 87c007c110833..e38cf0b62b4f5 100644 --- a/posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py +++ b/posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py @@ -47,6 +47,7 @@ StickinessFilter, LifecycleFilter, TrendsQuery, + CompareFilter, ) from posthog.test.base import BaseTest @@ -1318,6 +1319,17 @@ def test_breakdown(self): BreakdownFilter(breakdown_type=BreakdownType.EVENT, breakdown="$browser"), ) + def test_compare(self): + filter = {"compare": True, "compare_to": "-5w"} + + query = filter_to_query(filter) + + assert isinstance(query, TrendsQuery) + self.assertEqual( + query.compareFilter, + CompareFilter(**filter), + ) + def test_breakdown_converts_multi(self): filter = {"breakdowns": [{"type": "event", "property": "$browser"}]} @@ -1343,7 +1355,6 @@ def test_breakdown_type_default(self): def test_trends_filter(self): filter = { "smoothing_intervals": 2, - "compare": True, "aggregation_axis_format": "duration_ms", "aggregation_axis_prefix": "pre", "aggregation_axis_postfix": "post", @@ -1362,7 +1373,6 @@ def test_trends_filter(self): query.trendsFilter, TrendsFilter( smoothingIntervals=2, - compare=True, aggregationAxisFormat=AggregationAxisFormat.DURATION_MS, aggregationAxisPrefix="pre", aggregationAxisPostfix="post", @@ -1576,7 +1586,6 @@ def test_paths_filter(self): def test_stickiness_filter(self): filter = { "insight": "STICKINESS", - "compare": True, "show_legend": True, "show_values_on_series": True, "shown_as": "Stickiness", @@ -1587,7 +1596,7 @@ def test_stickiness_filter(self): assert isinstance(query, StickinessQuery) self.assertEqual( query.stickinessFilter, - StickinessFilter(compare=True, showLegend=True, showValuesOnSeries=True), + StickinessFilter(showLegend=True, showValuesOnSeries=True), ) def test_lifecycle_filter(self): diff --git a/posthog/hogql_queries/utils/query_compare_to_date_range.py b/posthog/hogql_queries/utils/query_compare_to_date_range.py new file mode 100644 index 0000000000000..b05d977640e69 --- /dev/null +++ b/posthog/hogql_queries/utils/query_compare_to_date_range.py @@ -0,0 +1,49 @@ +from datetime import datetime +from typing import Optional + +from posthog.hogql_queries.utils.query_date_range import QueryDateRange +from posthog.models.team import Team +from posthog.schema import DateRange, IntervalType, InsightDateRange +from posthog.utils import ( + relative_date_parse, +) + + +class QueryCompareToDateRange(QueryDateRange): + """Moves the start of a date range back by the compare_to interval and sets the end to have the same delta as the original""" + + _team: Team + _date_range: Optional[InsightDateRange | DateRange] + _interval: Optional[IntervalType] + _now_without_timezone: datetime + _compare_to: str + + def __init__( + self, + date_range: Optional[InsightDateRange | DateRange], + team: Team, + interval: Optional[IntervalType], + now: datetime, + compare_to: str, + ) -> None: + super().__init__(date_range, team, interval, now) + self.compare_to = compare_to + + def dates(self) -> tuple[datetime, datetime]: + current_period_date_from = super().date_from() + current_period_date_to = super().date_to() + + start_date = relative_date_parse(self.compare_to, self._team.timezone_info, now=current_period_date_from) + + return ( + start_date, + start_date + (current_period_date_to - current_period_date_from), + ) + + def date_to(self) -> datetime: + previous_period_date_to = self.dates()[1] + return previous_period_date_to + + def date_from(self) -> datetime: + previous_period_date_from = self.dates()[0] + return previous_period_date_from diff --git a/posthog/hogql_queries/utils/test/test_query_compare_to_date_range.py b/posthog/hogql_queries/utils/test/test_query_compare_to_date_range.py new file mode 100644 index 0000000000000..30181887fde8c --- /dev/null +++ b/posthog/hogql_queries/utils/test/test_query_compare_to_date_range.py @@ -0,0 +1,43 @@ +from dateutil import parser + +from posthog.hogql_queries.utils.query_compare_to_date_range import QueryCompareToDateRange +from posthog.schema import DateRange, IntervalType +from posthog.test.base import APIBaseTest + + +class TestQueryCompareToDateRange(APIBaseTest): + def test_zero(self): + now = parser.isoparse("2021-08-25T00:00:00.000Z") + date_range = DateRange(date_from="-48h") + query_date_range = QueryCompareToDateRange( + team=self.team, date_range=date_range, interval=IntervalType.DAY, now=now, compare_to="-0d" + ) + self.assertEqual(query_date_range.date_from(), parser.isoparse("2021-08-23T00:00:00Z")) + self.assertEqual(query_date_range.date_to(), parser.isoparse("2021-08-25T23:59:59.999999Z")) + + def test_minus_one_month(self): + now = parser.isoparse("2021-08-25T00:00:00.000Z") + date_range = DateRange(date_from="-48h") + query_date_range = QueryCompareToDateRange( + team=self.team, date_range=date_range, interval=IntervalType.DAY, now=now, compare_to="-1m" + ) + self.assertEqual(query_date_range.date_from(), parser.isoparse("2021-07-23T00:00:00Z")) + self.assertEqual(query_date_range.date_to(), parser.isoparse("2021-07-25T23:59:59.999999Z")) + + def test_minus_one_year(self): + now = parser.isoparse("2021-08-25T00:00:00.000Z") + date_range = DateRange(date_from="-48h") + query_date_range = QueryCompareToDateRange( + team=self.team, date_range=date_range, interval=IntervalType.DAY, now=now, compare_to="-1y" + ) + self.assertEqual(query_date_range.date_from(), parser.isoparse("2020-08-23T00:00:00Z")) + self.assertEqual(query_date_range.date_to(), parser.isoparse("2020-08-25T23:59:59.999999Z")) + + def test_feb(self): + now = parser.isoparse("2021-03-31T00:00:00.000Z") + date_range = DateRange(date_from="-48h") + query_date_range = QueryCompareToDateRange( + team=self.team, date_range=date_range, interval=IntervalType.DAY, now=now, compare_to="-1m" + ) + self.assertEqual(query_date_range.date_from(), parser.isoparse("2021-02-28T00:00:00Z")) + self.assertEqual(query_date_range.date_to(), parser.isoparse("2021-03-02T23:59:59.999999Z")) diff --git a/posthog/models/filters/filter.py b/posthog/models/filters/filter.py index e0549650981e6..339721642c1ae 100644 --- a/posthog/models/filters/filter.py +++ b/posthog/models/filters/filter.py @@ -59,9 +59,9 @@ class Filter( SelectorMixin, ShownAsMixin, BreakdownMixin, + CompareMixin, BreakdownValueMixin, FilterTestAccountsMixin, - CompareMixin, InsightMixin, OffsetMixin, LimitMixin, diff --git a/posthog/models/filters/mixins/common.py b/posthog/models/filters/mixins/common.py index 45f5cdc8737e9..57c2eccb0e4cb 100644 --- a/posthog/models/filters/mixins/common.py +++ b/posthog/models/filters/mixins/common.py @@ -48,6 +48,7 @@ TRENDS_WORLD_MAP, BreakdownAttributionType, BREAKDOWN_HIDE_OTHER_AGGREGATION, + COMPARE_TO, ) from posthog.hogql.constants import BREAKDOWN_VALUES_LIMIT, BREAKDOWN_VALUES_LIMIT_FOR_COUNTRIES from posthog.models.entity import Entity, ExclusionEntity, MathType @@ -344,6 +345,15 @@ def compare(self) -> bool: def compare_to_dict(self): return {"compare": self.compare} if self.compare else {} + @cached_property + def compare_to(self) -> bool: + _compare_to = self._data.get(COMPARE_TO, None) + return _compare_to + + @include_dict + def compare_to_to_dict(self): + return {"compare_to": self.compare_to} if self.compare_to else {} + class DateMixin(BaseParamMixin): date_from_delta_mapping: Optional[dict[str, int]] diff --git a/posthog/schema.py b/posthog/schema.py index 79d2c589e73ff..7da0fcffa77e2 100644 --- a/posthog/schema.py +++ b/posthog/schema.py @@ -197,6 +197,14 @@ class CohortPropertyFilter(BaseModel): value: int +class CompareFilter(BaseModel): + model_config = ConfigDict( + extra="forbid", + ) + compare: Optional[bool] = None + compare_to: Optional[str] = None + + class CountPerActorMathType(str, Enum): AVG_COUNT_PER_ACTOR = "avg_count_per_actor" MIN_COUNT_PER_ACTOR = "min_count_per_actor" @@ -965,6 +973,7 @@ class StickinessFilterLegacy(BaseModel): extra="forbid", ) compare: Optional[bool] = None + compare_to: Optional[str] = None display: Optional[ChartDisplayType] = None hidden_legend_indexes: Optional[list[float]] = None show_legend: Optional[bool] = None @@ -1082,7 +1091,6 @@ class TrendsFilter(BaseModel): aggregationAxisPostfix: Optional[str] = None aggregationAxisPrefix: Optional[str] = None breakdown_histogram_bin_count: Optional[float] = None - compare: Optional[bool] = False decimalPlaces: Optional[float] = None display: Optional[ChartDisplayType] = ChartDisplayType.ACTIONS_LINE_GRAPH formula: Optional[str] = None @@ -1103,6 +1111,7 @@ class TrendsFilterLegacy(BaseModel): aggregation_axis_prefix: Optional[str] = None breakdown_histogram_bin_count: Optional[float] = None compare: Optional[bool] = None + compare_to: Optional[str] = None decimal_places: Optional[float] = None display: Optional[ChartDisplayType] = None formula: Optional[str] = None @@ -3576,6 +3585,7 @@ class StickinessQuery(BaseModel): model_config = ConfigDict( extra="forbid", ) + compareFilter: Optional[CompareFilter] = Field(default=None, description="Compare to date range") dateRange: Optional[InsightDateRange] = Field(default=None, description="Date range for the query") filterTestAccounts: Optional[bool] = Field( default=False, description="Exclude internal and test users by applying the respective filters" @@ -3625,6 +3635,7 @@ class TrendsQuery(BaseModel): ) aggregation_group_type_index: Optional[int] = Field(default=None, description="Groups aggregation") breakdownFilter: Optional[BreakdownFilter] = Field(default=None, description="Breakdown of the events and actions") + compareFilter: Optional[CompareFilter] = Field(default=None, description="Compare to date range") dateRange: Optional[InsightDateRange] = Field(default=None, description="Date range for the query") filterTestAccounts: Optional[bool] = Field( default=False, description="Exclude internal and test users by applying the respective filters"