Skip to content

Commit

Permalink
fix(insights): handle large numbers correctly (#27316)
Browse files Browse the repository at this point in the history
  • Loading branch information
thmsobrmlr authored Jan 8, 2025
1 parent fcb7a91 commit 75ff89d
Show file tree
Hide file tree
Showing 7 changed files with 38 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export interface OperatorValueSelectProps {
type?: PropertyFilterType
propertyKey?: string
operator?: PropertyOperator | null
value?: string | number | Array<string | number> | null
value?: string | number | bigint | Array<string | number | bigint> | null
placeholder?: string
endpoint?: string
onChange: (operator: PropertyOperator, value: PropertyFilterValue) => void
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,19 @@ import { useEffect, useState } from 'react'
import { PropertyOperator } from '~/types'

const dayJSMightParse = (
candidateDateTimeValue: string | number | (string | number)[] | null | undefined
candidateDateTimeValue: string | number | bigint | (string | number | bigint)[] | null | undefined
): candidateDateTimeValue is string | number | undefined => ['string', 'number'].includes(typeof candidateDateTimeValue)

const narrowToString = (
candidateDateTimeValue: string | number | (string | number)[] | null | undefined
candidateDateTimeValue: string | number | bigint | (string | number | bigint)[] | null | undefined
): candidateDateTimeValue is string | null | undefined =>
candidateDateTimeValue == undefined || typeof candidateDateTimeValue === 'string'

interface PropertyFilterDatePickerProps {
autoFocus: boolean
operator: PropertyOperator
setValue: (newValue: PropertyValueProps['value']) => void
value: string | number | (string | number)[] | null | undefined
value: string | number | bigint | (string | number | bigint)[] | null | undefined
}

const dateAndTimeFormat = 'YYYY-MM-DD HH:mm:ss'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ export interface PropertyValueProps {
endpoint?: string // Endpoint to fetch options from
placeholder?: string
onSet: CallableFunction
value?: string | number | Array<string | number> | null
value?: string | number | bigint | Array<string | number | bigint> | null
operator: PropertyOperator
autoFocus?: boolean
eventNames?: string[]
Expand Down
22 changes: 22 additions & 0 deletions frontend/src/scenes/insights/utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -400,6 +400,28 @@ describe('formatBreakdownLabel()', () => {
expect(formatBreakdownLabel('661', breakdownFilter2, undefined, formatter, 0)).toEqual('661s')
})

it('handles large stringified numbers', () => {
const formatter = (_breakdown: any, v: any): any => `${v}s`

const breakdownFilter1: BreakdownFilter = {
breakdown: '$session_duration',
breakdown_type: 'session',
}
expect(formatBreakdownLabel('661', breakdownFilter1, undefined, formatter)).toEqual('661s')

const breakdownFilter2: BreakdownFilter = {
breakdowns: [
{
property: '$session_duration',
type: 'session',
},
],
}
expect(formatBreakdownLabel('987654321012345678', breakdownFilter2, undefined, formatter, 0)).toEqual(
'987654321012345678s'
)
})

it('handles array first', () => {
const formatter = (_: any, value: any, type: any): any => (type === 'session' ? `${value}s` : value)

Expand Down
14 changes: 9 additions & 5 deletions frontend/src/scenes/insights/utils.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ export function isOtherBreakdown(breakdown_value: string | number | null | undef
)
}

export function isNullBreakdown(breakdown_value: string | number | null | undefined): boolean {
export function isNullBreakdown(breakdown_value: string | number | bigint | null | undefined): boolean {
return (
breakdown_value === BREAKDOWN_NULL_STRING_LABEL ||
breakdown_value === BREAKDOWN_NULL_NUMERIC_LABEL ||
Expand All @@ -241,7 +241,7 @@ function isValidJsonArray(maybeJson: string): boolean {
}

function formatNumericBreakdownLabel(
breakdown_value: number,
breakdown_value: number | bigint,
breakdownFilter: BreakdownFilter | null | undefined,
formatPropertyValueForDisplay: FormatPropertyValueForDisplayFunction | undefined,
multipleBreakdownIndex: number | undefined
Expand Down Expand Up @@ -332,10 +332,14 @@ export function formatBreakdownLabel(
)
}

const maybeNumericValue = Number(breakdown_value)
if (!Number.isNaN(maybeNumericValue)) {
// stringified numbers
if (!Number.isNaN(Number(breakdown_value))) {
const numericValue =
Number.isInteger(Number(breakdown_value)) && !Number.isSafeInteger(Number(breakdown_value))
? BigInt(breakdown_value!)
: Number(breakdown_value)
return formatNumericBreakdownLabel(
maybeNumericValue,
numericValue,
breakdownFilter,
formatPropertyValueForDisplay,
multipleBreakdownIndex
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/scenes/web-analytics/webAnalyticsLogic.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,7 @@ export const webAnalyticsLogic = kea<webAnalyticsLogicType>([
return f
}
const oldValue = (Array.isArray(f.value) ? f.value : [f.value]).filter(isNotNil)
let newValue: (string | number)[]
let newValue: (string | number | bigint)[]
if (oldValue.includes(value)) {
// If there are multiple values for this filter, reduce that to just the one being clicked
if (oldValue.length > 1) {
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -663,7 +663,7 @@ export interface ToolbarProps extends ToolbarParams {

export type PathCleaningFilter = { alias?: string; regex?: string }

export type PropertyFilterValue = string | number | (string | number)[] | null
export type PropertyFilterValue = string | number | bigint | (string | number | bigint)[] | null

/** Sync with plugin-server/src/types.ts */
export enum PropertyOperator {
Expand Down

0 comments on commit 75ff89d

Please sign in to comment.