Skip to content

Commit

Permalink
fix(insights): polishing of multiple breakdowns in Trends (#23649)
Browse files Browse the repository at this point in the history
Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
  • Loading branch information
2 people authored and thmsobrmlr committed Jul 24, 2024
1 parent ffb7446 commit bc12d36
Show file tree
Hide file tree
Showing 14 changed files with 307 additions and 77 deletions.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { render } from '@testing-library/react'
import { MOCK_TEAM_ID } from 'lib/api.mock'
import { makeTestSetup } from 'lib/components/ActivityLog/activityLogLogic.test.setup'

import { BreakdownFilter } from '~/queries/schema'
import { ActivityScope } from '~/types'

jest.mock('lib/colors')
Expand Down Expand Up @@ -84,81 +85,104 @@ describe('the activity log logic', () => {
})

it('can handle change of insight query', async () => {
const logic = await insightTestSetup('test insight', 'updated', [
{
type: ActivityScope.INSIGHT,
action: 'changed',
field: 'query',
after: {
kind: 'TrendsQuery',
properties: {
type: 'AND',
values: [
{
type: 'OR',
values: [
{
type: 'event',
key: '$current_url',
operator: 'exact',
value: ['https://hedgebox.net/files/'],
},
{
type: 'event',
key: '$geoip_country_code',
operator: 'exact',
value: ['US', 'AU'],
},
],
},
],
},
filterTestAccounts: false,
interval: 'day',
dateRange: {
date_from: '-7d',
},
series: [
const insightMock = {
type: ActivityScope.INSIGHT,
action: 'changed',
field: 'query',
after: {
kind: 'TrendsQuery',
properties: {
type: 'AND',
values: [
{
kind: 'EventsNode',
name: '$pageview',
custom_name: 'Views',
event: '$pageview',
properties: [
type: 'OR',
values: [
{
type: 'event',
key: '$browser',
key: '$current_url',
operator: 'exact',
value: 'Chrome',
value: ['https://hedgebox.net/files/'],
},
{
type: 'cohort',
key: 'id',
value: 2,
type: 'event',
key: '$geoip_country_code',
operator: 'exact',
value: ['US', 'AU'],
},
],
limit: 100,
},
],
trendsFilter: {
display: 'ActionsAreaGraph',
},
breakdownFilter: {
breakdown: '$geoip_country_code',
breakdown_type: 'event',
},
filterTestAccounts: false,
interval: 'day',
dateRange: {
date_from: '-7d',
},
series: [
{
kind: 'EventsNode',
name: '$pageview',
custom_name: 'Views',
event: '$pageview',
properties: [
{
type: 'event',
key: '$browser',
operator: 'exact',
value: 'Chrome',
},
{
type: 'cohort',
key: 'id',
value: 2,
},
],
limit: 100,
},
],
trendsFilter: {
display: 'ActionsAreaGraph',
},
breakdownFilter: {
breakdown: '$geoip_country_code',
breakdown_type: 'event',
},
},
])
const actual = logic.values.humanizedActivity
}

const renderedDescription = render(<>{actual[0].description}</>).container
let logic = await insightTestSetup('test insight', 'updated', [insightMock as any])
let actual = logic.values.humanizedActivity

let renderedDescription = render(<>{actual[0].description}</>).container
expect(renderedDescription).toHaveTextContent('peter changed query definition on test insight')

const renderedExtendedDescription = render(<>{actual[0].extendedDescription}</>).container
let renderedExtendedDescription = render(<>{actual[0].extendedDescription}</>).container
expect(renderedExtendedDescription).toHaveTextContent(
"Query summaryAShowing \"Views\"Pageviewcounted by total countwhere event'sBrowser= equals Chromeand person belongs to cohortID 2FiltersEvent'sCurrent URL= equals https://hedgebox.net/files/or event'sCountry Code= equals US or AUBreakdown byCountry Code"
)
;(insightMock.after.breakdownFilter as BreakdownFilter) = {
breakdowns: [
{
property: '$geoip_country_code',
type: 'event',
},
{
property: '$session_duration',
type: 'session',
},
],
}

logic = await insightTestSetup('test insight', 'updated', [insightMock as any])
actual = logic.values.humanizedActivity

renderedDescription = render(<>{actual[0].description}</>).container
expect(renderedDescription).toHaveTextContent('peter changed query definition on test insight')

renderedExtendedDescription = render(<>{actual[0].extendedDescription}</>).container
expect(renderedExtendedDescription).toHaveTextContent(
"Query summaryAShowing \"Views\"Pageviewcounted by total countwhere event'sBrowser= equals Chromeand person belongs to cohortID 2FiltersEvent'sCurrent URL= equals https://hedgebox.net/files/or event'sCountry Code= equals US or AUBreakdown byCountry CodeSession duration"
)
})

it('can handle change of filters on a retention graph', async () => {
Expand Down
27 changes: 27 additions & 0 deletions frontend/src/lib/components/ActivityLog/complex.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
SELECT
count() AS total,
toStartOfDay(min_timestamp) AS day_start,
breakdown_value AS breakdown_value
FROM
(SELECT
min(timestamp) AS min_timestamp,
argMin(breakdown_value, timestamp) AS breakdown_value
FROM
(SELECT
person_id,
timestamp,
ifNull(nullIf(toString(properties.$browser), ''), '$$_posthog_breakdown_null_$$') AS breakdown_value
FROM
events AS e SAMPLE 1
WHERE
and(equals(event, '$pageview'), lessOrEquals(timestamp, assumeNotNull(toDateTime('2025-01-20 23:59:59'))))
)
GROUP BY
person_id
)
WHERE
greaterOrEquals(min_timestamp, toStartOfDay(assumeNotNull(toDateTime('2020-01-09 00:00:00'))))
GROUP BY
day_start,
breakdown_value
LIMIT 50000
41 changes: 41 additions & 0 deletions frontend/src/lib/components/ActivityLog/full_query.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
SELECT
arrayMap(number -> plus(toStartOfDay(assumeNotNull(toDateTime('2024-07-16 00:00:00'))), toIntervalDay(number)), range(0, plus(coalesce(dateDiff('day', toStartOfDay(assumeNotNull(toDateTime('2024-07-16 00:00:00'))), toStartOfDay(assumeNotNull(toDateTime('2024-07-23 23:59:59'))))), 1))) AS date,
arrayMap(_match_date -> arraySum(arraySlice(groupArray(count), indexOf(groupArray(day_start) AS _days_for_count, _match_date) AS _index, plus(minus(arrayLastIndex(x -> equals(x, _match_date), _days_for_count), _index), 1))), date) AS total
FROM
(SELECT
sum(total) AS count,
day_start
FROM (SELECT
count() AS total,
day_start,
breakdown_value
FROM (
SELECT
min(timestamp) as day_start,
argMin(breakdown_value, timestamp) AS breakdown_value,
FROM
(
SELECT
person_id,
toStartOfDay(timestamp) AS timestamp,
ifNull(nullIf(toString(person.properties.email), ''), '$$_posthog_breakdown_null_$$') AS breakdown_value
FROM
events AS e SAMPLE 1
WHERE
and(lessOrEquals(timestamp, assumeNotNull(toDateTime('2024-07-23 23:59:59'))), equals(properties.$browser, 'Safari'))
)
WHERE
greaterOrEquals(timestamp, toStartOfDay(assumeNotNull(toDateTime('2024-07-16 00:00:00'))))
GROUP BY
person_id
)
GROUP BY
day_start,
breakdown_value)
GROUP BY
day_start
ORDER BY
day_start ASC)
ORDER BY
arraySum(total) DESC
LIMIT 50000
45 changes: 45 additions & 0 deletions frontend/src/lib/components/ActivityLog/weekly.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
SELECT
arrayMap(number -> plus(toStartOfDay(assumeNotNull(toDateTime('2024-07-16 00:00:00'))), toIntervalDay(number)), range(0, plus(coalesce(dateDiff('day', toStartOfDay(assumeNotNull(toDateTime('2024-07-16 00:00:00'))), toStartOfDay(assumeNotNull(toDateTime('2024-07-23 23:59:59'))))), 1))) AS date,
arrayMap(_match_date -> arraySum(arraySlice(groupArray(count), indexOf(groupArray(day_start) AS _days_for_count, _match_date) AS _index, plus(minus(arrayLastIndex(x -> equals(x, _match_date), _days_for_count), _index), 1))), date) AS total
FROM
(SELECT
sum(total) AS count,
day_start
FROM
(SELECT
counts AS total,
toStartOfDay(timestamp) AS day_start
FROM
(SELECT
d.timestamp,
count(DISTINCT actor_id) AS counts
FROM
(SELECT
minus(toStartOfDay(assumeNotNull(toDateTime('2024-07-23 23:59:59'))), toIntervalDay(number)) AS timestamp
FROM
numbers(dateDiff('day', minus(toStartOfDay(assumeNotNull(toDateTime('2024-07-16 00:00:00'))), toIntervalDay(7)), assumeNotNull(toDateTime('2024-07-23 23:59:59')))) AS numbers) AS d
CROSS JOIN (SELECT
timestamp AS timestamp,
e.person_id AS actor_id
FROM
events AS e SAMPLE 1
WHERE
and(equals(event, '$pageview'), greaterOrEquals(timestamp, minus(assumeNotNull(toDateTime('2024-07-16 00:00:00')), toIntervalDay(7))), lessOrEquals(timestamp, assumeNotNull(toDateTime('2024-07-23 23:59:59'))))
GROUP BY
timestamp,
actor_id) AS e
WHERE
and(lessOrEquals(e.timestamp, plus(d.timestamp, toIntervalDay(1))), greater(e.timestamp, minus(d.timestamp, toIntervalDay(6))))
GROUP BY
d.timestamp
ORDER BY
d.timestamp ASC)
WHERE
and(greaterOrEquals(timestamp, toStartOfDay(assumeNotNull(toDateTime('2024-07-16 00:00:00')))), lessOrEquals(timestamp, assumeNotNull(toDateTime('2024-07-23 23:59:59')))))
GROUP BY
day_start
ORDER BY
day_start ASC)
ORDER BY
arraySum(total) DESC
LIMIT 50000
30 changes: 14 additions & 16 deletions frontend/src/lib/components/Cards/InsightCard/InsightDetails.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import {
isLifecycleQuery,
isPathsQuery,
isTrendsQuery,
isValidBreakdown,
} from '~/queries/utils'
import {
AnyPropertyFilter,
Expand Down Expand Up @@ -155,11 +156,7 @@ function SeriesDisplay({
const { mathDefinitions } = useValues(mathsLogic)
const filter = query.series[seriesIndex]

const hasBreakdown =
isInsightQueryWithBreakdown(query) &&
query.breakdownFilter != null &&
query.breakdownFilter.breakdown_type != null &&
query.breakdownFilter.breakdown != null
const hasBreakdown = isInsightQueryWithBreakdown(query) && isValidBreakdown(query.breakdownFilter)

const mathDefinition = mathDefinitions[
isLifecycleQuery(query)
Expand Down Expand Up @@ -338,25 +335,26 @@ export function LEGACY_FilterBasedBreakdownSummary({ filters }: { filters: Parti
}

export function BreakdownSummary({ query }: { query: InsightQueryNode }): JSX.Element | null {
if (
!isInsightQueryWithBreakdown(query) ||
!query.breakdownFilter ||
query.breakdownFilter.breakdown_type == null ||
query.breakdownFilter.breakdown == null
) {
if (!isInsightQueryWithBreakdown(query) || !isValidBreakdown(query.breakdownFilter)) {
return null
}

const { breakdown_type, breakdown } = query.breakdownFilter
const breakdownArray = Array.isArray(breakdown) ? breakdown : [breakdown]
const { breakdown_type, breakdown, breakdowns } = query.breakdownFilter

return (
<>
<h5>Breakdown by</h5>
<section className="InsightDetails__breakdown">
{breakdownArray.map((b) => (
<BreakdownTag key={b} breakdown={b} breakdownType={breakdown_type} />
))}
{Array.isArray(breakdowns)
? breakdowns.map((b) => (
<BreakdownTag key={`${b.type}-${b.property}`} breakdown={b.property} breakdownType={b.type} />
))
: breakdown &&
(Array.isArray(breakdown)
? breakdown
: [breakdown].map((b) => (
<BreakdownTag key={b} breakdown={b} breakdownType={breakdown_type} />
)))}
</section>
</>
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import { PathStepPicker } from 'scenes/insights/views/Paths/PathStepPicker'
import { trendsDataLogic } from 'scenes/trends/trendsDataLogic'
import { useDebouncedCallback } from 'use-debounce'

import { isValidBreakdown } from '~/queries/utils'
import { ChartDisplayType } from '~/types'

export function InsightDisplayConfig(): JSX.Element {
Expand Down Expand Up @@ -62,7 +63,7 @@ export function InsightDisplayConfig(): JSX.Element {
isLifecycle ||
((isTrends || isStickiness) && !(display && NON_TIME_SERIES_DISPLAY_TYPES.includes(display)))
const showSmoothing =
isTrends && !breakdownFilter?.breakdown_type && (!display || display === ChartDisplayType.ActionsLineGraph)
isTrends && !isValidBreakdown(breakdownFilter) && (!display || display === ChartDisplayType.ActionsLineGraph)

const { showValuesOnSeries, mightContainFractionalNumbers } = useValues(trendsDataLogic(insightProps))

Expand Down
11 changes: 11 additions & 0 deletions frontend/src/queries/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -447,3 +447,14 @@ export function hogql(strings: TemplateStringsArray, ...values: any[]): string {
return strings.reduce((acc, str, i) => acc + str + (i < strings.length - 1 ? formatHogQlValue(values[i]) : ''), '')
}
hogql.identifier = hogQlIdentifier

/**
* Wether we have a valid `breakdownFilter` or not.
*/
export function isValidBreakdown(breakdownFilter?: BreakdownFilter | null): breakdownFilter is BreakdownFilter {
return !!(
breakdownFilter &&
((breakdownFilter.breakdown && breakdownFilter.breakdown_type) ||
(breakdownFilter.breakdowns && breakdownFilter.breakdowns.length > 0))
)
}
Loading

0 comments on commit bc12d36

Please sign in to comment.