Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(web-analytics): Choose sensible interval for graphs #18860

Merged
merged 11 commits into from
Nov 30, 2023
40 changes: 32 additions & 8 deletions frontend/src/lib/components/IntervalFilter/IntervalFilter.tsx
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import { LemonSelect } from '@posthog/lemon-ui'
import { LemonSelect, LemonSelectOption } from '@posthog/lemon-ui'
import { useActions, useValues } from 'kea'
import { insightLogic } from 'scenes/insights/insightLogic'
import { insightVizDataLogic } from 'scenes/insights/insightVizDataLogic'

import { InsightQueryNode } from '~/queries/schema'
import { IntervalType } from '~/types'

interface IntervalFilterProps {
disabled?: boolean
Expand All @@ -19,21 +20,44 @@ export function IntervalFilter({ disabled }: IntervalFilterProps): JSX.Element {
<span>
<span className="hide-lte-md">grouped </span>by
</span>
<LemonSelect
size={'small'}
<IntervalFilterStandalone
disabled={disabled}
value={interval || 'day'}
dropdownMatchSelectWidth={false}
onChange={(value) => {
interval={interval || 'day'}
onIntervalChange={(value) => {
updateQuerySource({ interval: value } as Partial<InsightQueryNode>)
}}
data-attr="interval-filter"
options={Object.entries(enabledIntervals).map(([value, { label, disabledReason }]) => ({
value,
value: value as IntervalType,
label,
disabledReason,
}))}
/>
</>
)
}

interface IntervalFilterStandaloneProps {
disabled?: boolean
interval: IntervalType | undefined
onIntervalChange: (interval: IntervalType) => void
options: LemonSelectOption<IntervalType>[]
}

export function IntervalFilterStandalone({
disabled,
interval,
onIntervalChange,
options,
}: IntervalFilterStandaloneProps): JSX.Element {
return (
<LemonSelect
size={'small'}
disabled={disabled}
value={interval || 'day'}
dropdownMatchSelectWidth={false}
onChange={onIntervalChange}
data-attr="interval-filter"
options={options}
/>
)
}
68 changes: 68 additions & 0 deletions frontend/src/lib/utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import tk from 'timekeeper'
import { ElementType, EventType, PropertyType, TimeUnitType } from '~/types'

import {
areDatesValidForInterval,
areObjectValuesEmpty,
average,
booleanOperatorMap,
Expand All @@ -23,6 +24,7 @@ import {
eventToDescription,
floorMsToClosestSecond,
genericOperatorMap,
getDefaultInterval,
getFormattedLastWeekDate,
hexToRGBA,
humanFriendlyDuration,
Expand Down Expand Up @@ -382,6 +384,72 @@ describe('dateStringToDayJs', () => {
})
})

describe('getDefaultInterval', () => {
it('should return days for last 7 days', () => {
expect(getDefaultInterval('-7d', null)).toEqual('day')
})

it('should return hours for last 24 hours', () => {
expect(getDefaultInterval('-24h', null)).toEqual('hour')
})

it('should return days for month to date', () => {
expect(getDefaultInterval('mStart', null)).toEqual('day')
})

it('should return month for year to date', () => {
expect(getDefaultInterval('yStart', null)).toEqual('month')
})

it('should return month for all time', () => {
expect(getDefaultInterval('all', null)).toEqual('month')
})

it('should handle explicit dates 6 months apart', () => {
expect(getDefaultInterval('2023-10-01', '2023-04-01')).toEqual('month')
})
it('should handle explicit dates a month apart', () => {
expect(getDefaultInterval('2023-10-01', '2023-09-01')).toEqual('week')
})
it('should handle explicit dates a week apart', () => {
expect(getDefaultInterval('2023-10-01', '2023-09-25')).toEqual('day')
})
it('should handle explicit dates a day apart', () => {
expect(getDefaultInterval('2023-10-02', '2023-10-01')).toEqual('hour')
})
it('should handle explicit dates 12 hours apart', () => {
expect(getDefaultInterval('2023-10-01T18:00:00', '2023-10-01T6:00:00')).toEqual('hour')
})
})

describe('areDatesValidForInterval', () => {
it('should require interval to be month for all time', () => {
expect(areDatesValidForInterval('month', 'all', null)).toEqual(true)
expect(areDatesValidForInterval('week', 'all', null)).toEqual(false)
expect(areDatesValidForInterval('day', 'all', null)).toEqual(false)
expect(areDatesValidForInterval('hour', 'all', null)).toEqual(false)
})
it('should return false if the dates are one interval apart', () => {
expect(areDatesValidForInterval('day', '-24h', null)).toEqual(false)
expect(areDatesValidForInterval('week', '-7d', null)).toEqual(false)
expect(areDatesValidForInterval('day', '-1d', null)).toEqual(false)
})
it('should return true if the dates are two intervals apart', () => {
expect(areDatesValidForInterval('day', '-48h', null)).toEqual(true)
expect(areDatesValidForInterval('week', '-14d', null)).toEqual(true)
expect(areDatesValidForInterval('day', '-2d', null)).toEqual(true)
})
it('should return false for hourly if over 2 weeks', () => {
expect(areDatesValidForInterval('hour', '-15d', null)).toEqual(false)
})
it('should support explicit dates', () => {
expect(areDatesValidForInterval('month', '2023-08-01', '2023-11-01')).toEqual(true)
expect(areDatesValidForInterval('week', '2023-10-01', '2023-11-01')).toEqual(true)
expect(areDatesValidForInterval('day', '2023-10-16', '2023-11-01')).toEqual(true)
expect(areDatesValidForInterval('hour', '2023-11-01T12', '2023-11-01T18')).toEqual(true)
})
})

describe('hexToRGBA()', () => {
it('converts hex to RGBA correctly', () => {
expect(hexToRGBA('#ff0000', 0.3)).toEqual('rgba(255,0,0,0.3)')
Expand Down
190 changes: 159 additions & 31 deletions frontend/src/lib/utils.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
DateMappingOption,
EventType,
GroupActorType,
IntervalType,
PropertyOperator,
PropertyType,
TimeUnitType,
Expand Down Expand Up @@ -863,7 +864,8 @@ const dateOptionsMap = {
m: 'month',
w: 'week',
d: 'day',
}
h: 'hour',
} as const

export function dateFilterToText(
dateFrom: string | dayjs.Dayjs | null | undefined,
Expand Down Expand Up @@ -935,45 +937,171 @@ export function dateFilterToText(
return defaultValue
}

export function dateStringToComponents(date: string | null): {
amount: number
unit: (typeof dateOptionsMap)[keyof typeof dateOptionsMap]
clip: 'Start' | 'End'
} | null {
if (!date) {
return null
}
const parseDate = /^([-+]?)([0-9]*)([hdwmqy])(|Start|End)$/
const matches = date.match(parseDate)
if (!matches) {
return null
}
const [, sign, rawAmount, rawUnit, clip] = matches
const amount = rawAmount ? parseInt(sign + rawAmount) : 0
const unit = dateOptionsMap[rawUnit] || 'day'
return { amount, unit, clip: clip as 'Start' | 'End' }
}

/** Convert a string like "-30d" or "2022-02-02" or "-1mEnd" to `Dayjs().startOf('day')` */
export function dateStringToDayJs(date: string | null): dayjs.Dayjs | null {
if (isDate.test(date || '')) {
return dayjs(date)
}
const parseDate = /^([-+]?)([0-9]*)([dmwqy])(|Start|End)$/
const matches = (date || '').match(parseDate)
let response: null | dayjs.Dayjs = null
if (matches) {
const [, sign, rawAmount, rawUnit, clip] = matches
const amount = rawAmount ? parseInt(sign + rawAmount) : 0
const unit = dateOptionsMap[rawUnit] || 'day'

switch (unit) {
case 'year':
response = dayjs().add(amount, 'year')
break
case 'quarter':
response = dayjs().add(amount * 3, 'month')
break
case 'month':
response = dayjs().add(amount, 'month')
break
case 'week':
response = dayjs().add(amount * 7, 'day')
break
default:
response = dayjs().add(amount, 'day')
break
const dateComponents = dateStringToComponents(date)
if (!dateComponents) {
return null
}

const { unit, amount, clip } = dateComponents
let response: dayjs.Dayjs

switch (unit) {
case 'year':
response = dayjs().add(amount, 'year')
break
case 'quarter':
response = dayjs().add(amount * 3, 'month')
break
case 'month':
response = dayjs().add(amount, 'month')
break
case 'week':
response = dayjs().add(amount * 7, 'day')
break
case 'day':
response = dayjs().add(amount, 'day')
break
case 'hour':
response = dayjs().add(amount, 'hour')
break
default:
throw new UnexpectedNeverError(unit)
}

if (clip === 'Start') {
return response.startOf(unit)
} else if (clip === 'End') {
return response.endOf(unit)
}
return response.startOf('day')
}

export const getDefaultInterval = (dateFrom: string | null, dateTo: string | null): IntervalType => {
// use the default mapping if we can
for (const mapping of dateMapping) {
const mappingFrom = mapping.values[0] ?? null
const mappingTo = mapping.values[1] ?? null
if (mappingFrom === dateFrom && mappingTo === dateTo && mapping.defaultInterval) {
return mapping.defaultInterval
}
}

const parsedDateFrom = dateStringToComponents(dateFrom)
const parsedDateTo = dateStringToComponents(dateTo)

if (clip === 'Start') {
return response.startOf(unit)
} else if (clip === 'End') {
return response.endOf(unit)
if (parsedDateFrom?.unit === 'hour' || parsedDateTo?.unit === 'hour') {
return 'hour'
}

if (parsedDateFrom?.unit === 'day' || parsedDateTo?.unit === 'day' || dateFrom === 'mStart') {
return 'day'
}

if (
parsedDateFrom?.unit === 'month' ||
parsedDateTo?.unit === 'month' ||
parsedDateFrom?.unit === 'quarter' ||
parsedDateTo?.unit === 'quarter' ||
parsedDateFrom?.unit === 'year' ||
parsedDateTo?.unit === 'year' ||
dateFrom === 'all'
) {
return 'month'
}

const dateFromDayJs = dateStringToDayJs(dateFrom)
const dateToDayJs = dateStringToDayJs(dateTo)

const intervalMonths = dateFromDayJs?.diff(dateToDayJs, 'month')
if (intervalMonths != null && Math.abs(intervalMonths) >= 2) {
return 'month'
}
const intervalDays = dateFromDayJs?.diff(dateToDayJs, 'day')
if (intervalDays != null && Math.abs(intervalDays) >= 14) {
return 'week'
}
if (intervalDays != null && Math.abs(intervalDays) >= 2) {
return 'day'
}
const intervalHours = dateFromDayJs?.diff(dateToDayJs, 'hour')
if (intervalHours != null && Math.abs(intervalHours) >= 1) {
return 'hour'
}

return 'day'
}

/* If the interval changes, check if it's compatible with the selected dates, and return new dates
* from a map of sensible defaults if not */
export const areDatesValidForInterval = (
interval: IntervalType,
oldDateFrom: string | null,
oldDateTo: string | null
): boolean => {
const parsedOldDateFrom = dateStringToDayJs(oldDateFrom)
const parsedOldDateTo = dateStringToDayJs(oldDateTo) || dayjs()

if (oldDateFrom === 'all' || !parsedOldDateFrom) {
return interval === 'month'
} else if (interval === 'month') {
return parsedOldDateTo.diff(parsedOldDateFrom, 'month') >= 2
} else if (interval === 'week') {
return parsedOldDateTo.diff(parsedOldDateFrom, 'week') >= 2
} else if (interval === 'day') {
const diff = parsedOldDateTo.diff(parsedOldDateFrom, 'day')
return diff >= 2
} else if (interval === 'hour') {
return (
parsedOldDateTo.diff(parsedOldDateFrom, 'hour') >= 2 &&
parsedOldDateTo.diff(parsedOldDateFrom, 'hour') < 24 * 7 * 2 // 2 weeks
)
}
throw new UnexpectedNeverError(interval)
}

const defaultDatesForInterval = {
hour: { dateFrom: '-24h', dateTo: null },
day: { dateFrom: '-7d', dateTo: null },
week: { dateFrom: '-28d', dateTo: null },
month: { dateFrom: '-6m', dateTo: null },
}

export const updateDatesWithInterval = (
interval: IntervalType,
oldDateFrom: string | null,
oldDateTo: string | null
): { dateFrom: string | null; dateTo: string | null } => {
if (areDatesValidForInterval(interval, oldDateFrom, oldDateTo)) {
return {
dateFrom: oldDateFrom,
dateTo: oldDateTo,
}
return response.startOf('day')
}
return response
return defaultDatesForInterval[interval]
}

export function clamp(value: number, min: number, max: number): number {
Expand Down
9 changes: 8 additions & 1 deletion frontend/src/scenes/trends/viz/ActionsLineGraph.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,13 @@ export function ActionsLineGraph({
isStickiness,
} = useValues(trendsDataLogic(insightProps))

const labels =
(indexedResults.length === 2 &&
indexedResults.every((x) => x.compare) &&
indexedResults.find((x) => x.compare_label === 'current')?.days) ||
(indexedResults[0] && indexedResults[0].labels) ||
[]

return indexedResults &&
indexedResults[0]?.data &&
indexedResults.filter((result) => result.count !== 0).length > 0 ? (
Expand All @@ -51,7 +58,7 @@ export function ActionsLineGraph({
type={display === ChartDisplayType.ActionsBar || isLifecycle ? GraphType.Bar : GraphType.Line}
hiddenLegendKeys={hiddenLegendKeys}
datasets={indexedResults}
labels={(indexedResults[0] && indexedResults[0].labels) || []}
labels={labels}
inSharedMode={inSharedMode}
labelGroupType={labelGroupType}
showPersonsModal={showPersonsModal}
Expand Down
Loading
Loading