Skip to content
17 changes: 14 additions & 3 deletions frontend/src/queries/nodes/DataVisualization/Components/Table.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
import { LemonBanner, LemonTable, LemonTableColumn, Tooltip } from '@posthog/lemon-ui'

import { dayjs } from 'lib/dayjs'
import { execHog } from 'lib/hog'
import { lightenDarkenColor } from 'lib/utils/colors'
import { InsightEmptyState, InsightErrorState } from 'scenes/insights/EmptyStates'

Expand Down Expand Up @@ -67,7 +66,7 @@

// Numbers sort numerically, dates chronologically by epoch, everything else by a
// numeric-aware string compare, and empty cells sort to the bottom of an ascending sort.
export function compareTableCells(a: TableDataCell<any> | undefined, b: TableDataCell<any> | undefined): number {

Check warning on line 69 in frontend/src/queries/nodes/DataVisualization/Components/Table.tsx

View workflow job for this annotation

GitHub Actions / Frontend formatting

lint:complexity

`compareTableCells` has cyclomatic complexity 11 (warn >10)
const aValue = a?.value
const bValue = b?.value
if (aValue == null && bValue == null) {
Expand Down Expand Up @@ -166,6 +165,8 @@
isTransposed,
hasSortedTable,
hasMoreData,
hogVm,
hogVmLoadFailed,
} = useValues(dataVisualizationLogic)
const { toggleColumnPin, setTableSorted } = useActions(dataVisualizationLogic)

Expand All @@ -178,10 +179,10 @@
const formattedTitle = typeof columnTitle === 'string' ? formatColumnTitle(columnTitle) : columnTitle
const fullColumnTitle = typeof columnTitle === 'string' ? columnTitle : undefined

const computeConditionalFormattingBackground = (data: TableDataCell<any>[]): string | undefined => {

Check warning on line 182 in frontend/src/queries/nodes/DataVisualization/Components/Table.tsx

View workflow job for this annotation

GitHub Actions / Frontend formatting

lint:complexity

`computeConditionalFormattingBackground` has cyclomatic complexity 13 (warn >10)
const cell = data[index]

if (cell.isTransposedHeader) {
if (cell.isTransposedHeader || !hogVm) {
return undefined
}

Expand All @@ -201,7 +202,7 @@
})
.map((n) => ({
rule: n,
result: execHog(n.bytecode, {
result: hogVm.execHog(n.bytecode, {
globals: {
value: cell.value,
input: convertTableValue(n.input, sourceColumnType),
Expand Down Expand Up @@ -336,6 +337,16 @@

return (
<>
{hogVmLoadFailed ? (
<LemonBanner
type="warning"
className="mb-2"
action={{ children: 'Reload page', onClick: () => window.location.reload() }}
>
Couldn't load conditional formatting, so cells show without their colors. Reload the page to try
again.
</LemonBanner>
) : null}
{hasSortedTable && hasMoreData && (
<LemonBanner type="info" className="mb-2" dismissKey="data-visual">
Sorting only reorders the rows already loaded, not the full dataset.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { expectLogic } from 'kea-test-utils'

import { DataVisualizationNode, NodeKind } from '~/queries/schema/schema-general'
import { ConditionalFormattingRule, DataVisualizationNode, NodeKind } from '~/queries/schema/schema-general'
import { initKeaTests } from '~/test/init'
import { ChartDisplayType } from '~/types'

Expand All @@ -24,6 +24,16 @@ const defaultQuery: DataVisualizationNode = {
display: ChartDisplayType.Auto,
}

const equalsRule: ConditionalFormattingRule = {
id: 'equals',
templateId: 'equals',
columnName: 'value',
input: '1',
color: '#FFADAD',
colorMode: 'light',
bytecode: ['_H', 1, 32, 'input', 1, 1, 32, 'value', 1, 1, 11, 38],
}

describe('dataVisualizationLogic', () => {
let logic: ReturnType<typeof dataVisualizationLogic.build>

Expand Down Expand Up @@ -659,6 +669,65 @@ describe('dataVisualizationLogic', () => {
await expectLogic(logic).toMatchValues({ hasSortedTable: true })
})

test.each([
{
name: 'loads the Hog VM for a table with formatting rules',
display: ChartDisplayType.ActionsTable,
rules: [equalsRule],
hogVm: expect.anything(),
},
{
name: 'does not load the Hog VM for a table without formatting rules',
display: ChartDisplayType.ActionsTable,
rules: [],
hogVm: null,
},
{
name: 'does not load the Hog VM for an auto visualization before its data arrives',
display: ChartDisplayType.Auto,
rules: [equalsRule],
hogVm: null,
},
{
name: 'does not load the Hog VM for a chart that kept table formatting rules',
display: ChartDisplayType.ActionsLineGraph,
rules: [equalsRule],
hogVm: null,
},
])('$name', async ({ display, rules, hogVm }) => {
const tableLogic = dataVisualizationLogic({
key: 'hog-vm-loading',
query: { ...defaultQuery, display, tableSettings: { conditionalFormatting: rules } },
dataNodeCollectionId,
} as DataVisualizationLogicProps)
tableLogic.mount()

await expectLogic(tableLogic).toFinishAllListeners().toMatchValues({ hogVm })
tableLogic.unmount()
})

test.each([
{ name: 'shows a Hog VM load failure while the table needs the VM', rules: [equalsRule], failed: true },
{ name: 'hides a Hog VM load failure once the table has no rules', rules: [], failed: false },
])('$name', async ({ rules, failed }) => {
const tableLogic = dataVisualizationLogic({
key: 'hog-vm-load-failure',
query: {
...defaultQuery,
display: ChartDisplayType.ActionsTable,
tableSettings: { conditionalFormatting: rules },
},
dataNodeCollectionId,
} as DataVisualizationLogicProps)
tableLogic.mount()
await expectLogic(tableLogic).toFinishAllListeners()

tableLogic.actions.setHogVmLoadError(new Error('chunk failed'))

await expectLogic(tableLogic).toMatchValues({ hogVmLoadFailed: failed })
tableLogic.unmount()
})

it('does not mutate the original query when updating y-axis formatting', async () => {
const queryWithAxisSettings: DataVisualizationNode = {
...defaultQuery,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,17 @@
import type { BreakPointFunction } from 'kea'
import { subscriptions } from 'kea-subscriptions'
import mergeObject from 'lodash.merge'
import posthog from 'posthog-js'

import { PIE_DISPLAY_TYPES } from 'lib/constants'
import { dayjs } from 'lib/dayjs'
import type { execHog } from 'lib/hog'
import { RGBToHex, lightenDarkenColor } from 'lib/utils/colors'
import { uuid } from 'lib/utils/dom'
import { isChunkLoadError } from 'lib/utils/isChunkLoadError'
import { compactNumber } from 'lib/utils/numbers'
import { objectsEqual } from 'lib/utils/objects'
import { retryImport } from 'lib/utils/retryImport'
import { sceneLogic } from 'scenes/sceneLogic'
import { Scene } from 'scenes/sceneTypes'
import { teamLogic } from 'scenes/teamLogic'
Expand Down Expand Up @@ -153,7 +157,7 @@
const TRANSPOSED_FIELD_COLUMN_NAME = '__transpose_field__'
const TRANSPOSED_ROW_COLUMN_PREFIX = '__transpose_row__'

export const formatDataWithSettings = (

Check warning on line 160 in frontend/src/queries/nodes/DataVisualization/dataVisualizationLogic.ts

View workflow job for this annotation

GitHub Actions / Frontend formatting

lint:complexity

`formatDataWithSettings` has cyclomatic complexity 11 (warn >10)
data: number | string | null | object,
settings?: AxisSeriesSettings
): string | object | null => {
Expand Down Expand Up @@ -196,7 +200,7 @@
return dataAsString
}

export const convertTableValue = (

Check warning on line 203 in frontend/src/queries/nodes/DataVisualization/dataVisualizationLogic.ts

View workflow job for this annotation

GitHub Actions / Frontend formatting

lint:complexity

`convertTableValue` has cyclomatic complexity 11 (warn >10)
value: string | number | null,
type: ColumnScalar
): string | number | boolean | null => {
Expand Down Expand Up @@ -235,7 +239,7 @@
return value
}

const toFriendlyClickhouseTypeName = (type: string | undefined): ColumnScalar => {

Check warning on line 242 in frontend/src/queries/nodes/DataVisualization/dataVisualizationLogic.ts

View workflow job for this annotation

GitHub Actions / Frontend formatting

lint:complexity

`toFriendlyClickhouseTypeName` has cyclomatic complexity 11 (warn >10)
if (!type) {
return 'UNKNOWN'
}
Expand Down Expand Up @@ -400,7 +404,7 @@
})
}

const mergeChartSettings = (state: ChartSettings, settings: ChartSettings): ChartSettings => {

Check warning on line 407 in frontend/src/queries/nodes/DataVisualization/dataVisualizationLogic.ts

View workflow job for this annotation

GitHub Actions / Frontend formatting

lint:complexity

`mergeChartSettings` has cyclomatic complexity 17 (warn >10)
return {
...state,
...settings,
Expand Down Expand Up @@ -505,7 +509,7 @@
return numericalColumns.find((column) => !selectedYAxisNames.has(column.name)) ?? numericalColumns[0]
}

export function applyVisualizationType(

Check warning on line 512 in frontend/src/queries/nodes/DataVisualization/dataVisualizationLogic.ts

View workflow job for this annotation

GitHub Actions / Frontend formatting

lint:complexity

`applyVisualizationType` has cyclomatic complexity 28 (warn >10)
query: DataVisualizationNode,
visualizationType: ChartDisplayType,
columns: Column[],
Expand Down Expand Up @@ -625,6 +629,11 @@
}
}

// A named interface, because kea-typegen writes `typeof import('lib/hog')` as an absolute path.
interface HogVm {
execHog: typeof execHog
}

// Generated by kea-typegen. Update if you're an agent, ignore if you're human.
export interface dataVisualizationLogicValues {
hasMoreData: boolean // dataNodeLogic
Expand Down Expand Up @@ -661,12 +670,16 @@
effectiveVisualizationType: ChartDisplayType
hasDateTimeColumns: boolean
hasSortedTable: boolean
hogVm: HogVm | null
hogVmLoadError: unknown
hogVmLoadFailed: boolean
isChartSettingsPanelOpen: boolean
isColumnPinned: (columnName: string) => boolean
isPinningEnabled: boolean
isShowingCachedResults: boolean
isTableVisualization: boolean
isTransposed: boolean
needsHogVm: boolean
numericalColumns: Column[]
pinnedColumns: string[]
presetChartHeight: boolean
Expand Down Expand Up @@ -732,9 +745,18 @@
deleteYSeries: (seriesIndex: number) => {
seriesIndex: number
}
loadHogVm: () => {
value: true
}
setConditionalFormattingRulesPanelActiveKeys: (keys: string[]) => {
keys: string[]
}
setHogVm: (hogVm: HogVm) => {
hogVm: HogVm
}
setHogVmLoadError: (error: unknown) => {
error: unknown
}
setQuery: (setter: (node: DataVisualizationNode) => DataVisualizationNode) => {
setter: (node: DataVisualizationNode) => DataVisualizationNode
}
Expand Down Expand Up @@ -965,6 +987,13 @@
| TraceSpansTreeQueryResponse
| null
) => ChartDisplayType
needsHogVm: (
visualizationType: ChartDisplayType,
effectiveVisualizationType: ChartDisplayType,
columns: Column[],
conditionalFormattingRules: ConditionalFormattingRule[]
) => boolean
hogVmLoadFailed: (needsHogVm: boolean, hogVmLoadError: unknown) => boolean
isTableVisualization: (effectiveVisualizationType: ChartDisplayType) => boolean
showTableSettings: (effectiveVisualizationType: ChartDisplayType) => boolean
isColumnPinned: (pinnedColumns: string[]) => (columnName: string) => boolean
Expand Down Expand Up @@ -1027,6 +1056,9 @@
}),
props({ query: { source: {} } } as DataVisualizationLogicProps),
actions(({ values }) => ({
loadHogVm: true,
setHogVm: (hogVm: HogVm) => ({ hogVm }),
setHogVmLoadError: (error: unknown) => ({ error }),
setVisualizationType: (visualizationType: ChartDisplayType) => ({
visualizationType,
node: applyVisualizationType(
Expand Down Expand Up @@ -1077,6 +1109,8 @@
_setQuery: (node: DataVisualizationNode) => ({ node }),
})),
reducers(({ props }) => ({
hogVm: [null as HogVm | null, { setHogVm: (_, { hogVm }) => hogVm }],
hogVmLoadError: [null as unknown, { setHogVmLoadError: (_, { error }) => error, setHogVm: () => null }],
query: [
props.query,
{
Expand Down Expand Up @@ -1524,7 +1558,7 @@

return {
column,
data: data.map((n) => {

Check warning on line 1561 in frontend/src/queries/nodes/DataVisualization/dataVisualizationLogic.ts

View workflow job for this annotation

GitHub Actions / Frontend formatting

lint:complexity

`<anonymous>` has cyclomatic complexity 11 (warn >10)
try {
const multiplier = series.settings.formatting?.style === 'percent' ? 100 : 1

Expand Down Expand Up @@ -1881,6 +1915,24 @@
| import('~/queries/schema/schema-general').TraceSpansQueryResponse
): ChartDisplayType => getAutoVisualizationType(columns, rowCountFromResponse(response)),
],
// The Hog VM and its crypto polyfills are large, so only a table that shows formatting rules loads them.
// Auto resolves to a table until columns arrive, so it waits for them before it decides.
needsHogVm: [
(s) => [s.visualizationType, s.effectiveVisualizationType, s.columns, s.conditionalFormattingRules],
(
visualizationType: ChartDisplayType,
effectiveVisualizationType: ChartDisplayType,
columns: Column[],
rules: ConditionalFormattingRule[]
): boolean =>
(visualizationType !== ChartDisplayType.Auto || columns.length > 0) &&
effectiveVisualizationType === ChartDisplayType.ActionsTable &&
rules.length > 0,
],
hogVmLoadFailed: [
(s) => [s.needsHogVm, s.hogVmLoadError],
(needsHogVm: boolean, hogVmLoadError: unknown): boolean => needsHogVm && !!hogVmLoadError,
],
isTableVisualization: [
(s) => [s.effectiveVisualizationType],
(visualizationType: ChartDisplayType): boolean =>
Expand Down Expand Up @@ -1954,6 +2006,17 @@
},
})),
listeners(({ props, values, actions, sharedListeners }) => ({
loadHogVm: async () => {
try {
actions.setHogVm(await retryImport(() => import('lib/hog')))
} catch (error) {
// A chunk that fails to load is a network or stale-deploy problem, not a bug to report.
if (!isChunkLoadError(error)) {
posthog.captureException(error)
}
actions.setHogVmLoadError(error)
}
Comment thread
pauldambra marked this conversation as resolved.
},
updateChartSettings: ({ settings }) => {
actions.setQuery((query) => ({
...query,
Expand Down Expand Up @@ -2000,7 +2063,12 @@
toggleColumnPin: [sharedListeners.pinnedColumnsChanged],
})),
subscriptions(({ actions, values }) => ({
needsHogVm: (needsHogVm: boolean) => {
if (needsHogVm && !values.hogVm) {
actions.loadHogVm()
}
},
columns: (value: Column[], oldValue: Column[]) => {

Check warning on line 2071 in frontend/src/queries/nodes/DataVisualization/dataVisualizationLogic.ts

View workflow job for this annotation

GitHub Actions / Frontend formatting

lint:complexity

`columns` has cyclomatic complexity 21 (warn >10)
// If the response is cleared, then don't update any internal values
if (!values.response || (!(values.response as any).results && !(values.response as any).result)) {
return
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,8 @@ const meta: Meta<typeof DataTableVisualization> = {
parameters: {
testOptions: {
snapshotBrowsers: ['chromium'],
waitForSelector: '.DataVisualizationTable',
// The table loads the Hog VM after it renders, so wait for a cell that a rule colored.
waitForSelector: '.DataVisualizationTable td[style*="background-color"]',
},
},
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import api from 'lib/api'
import { convertHogToJS, execHog } from 'lib/hog'
import { retryImport } from 'lib/utils/retryImport'

import type { WidgetFrameApi, WidgetStatusApiInputBindings } from 'products/notebooks/frontend/generated/api.schemas'

Expand Down Expand Up @@ -49,6 +49,7 @@ export async function applyReusableWidgetBinding(
throw new Error(`The input mapping for "${logicalName}" has invalid compiled Hog code.`)
}
const sourceRows = frameRowsAsObjects(frame)
const { convertHogToJS, execHog } = await retryImport(() => import('lib/hog'))
const execution = execHog(bytecode, {
globals: {
columns: frame.columns,
Expand Down
Loading