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(data-exploration): improve data table crashes #13806

Merged
merged 27 commits into from
Jan 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
8148b29
rename to "hogql" and "HogQLContext"
mariusandra Jan 18, 2023
9986712
make context mandatory
mariusandra Jan 18, 2023
bd69a86
rename TaxonomicBreakdownFilter
mariusandra Jan 18, 2023
c83b9fd
hogql property types
mariusandra Jan 18, 2023
a2dacaf
support hogql property filters in the frontend
mariusandra Jan 18, 2023
bb19848
make standard hogql filters it work on events page
mariusandra Jan 18, 2023
a5a2b13
this is count()
mariusandra Jan 18, 2023
ca3b98e
filter based on flag
mariusandra Jan 18, 2023
5bb99fc
add test for hogql filters in events query
mariusandra Jan 18, 2023
28a1332
Merge branch 'master' into hogql-small-refactor
mariusandra Jan 18, 2023
5142bd9
Update snapshots
github-actions[bot] Jan 18, 2023
ed8b560
fix merged path
mariusandra Jan 18, 2023
0cf96e6
Merge branch 'hogql-small-refactor' of github.com:PostHog/posthog int…
mariusandra Jan 18, 2023
4b13036
revert default factories (fix mypy)
mariusandra Jan 18, 2023
6105cfe
mypy
mariusandra Jan 19, 2023
24f3cce
Merge branch 'master' into hogql-small-refactor
mariusandra Jan 19, 2023
7f423d6
feat(data-exploration): improve data table crashes
mariusandra Jan 19, 2023
797fe33
fix row highlighting
mariusandra Jan 19, 2023
6a072ea
be explicit about columns
mariusandra Jan 19, 2023
22eb2d4
Merge branch 'master' into data-table-crash-detection
mariusandra Jan 19, 2023
38aed5c
simplify
mariusandra Jan 19, 2023
753949f
use a dataclass
mariusandra Jan 19, 2023
e0877b6
Merge branch 'master' into data-table-crash-detection
mariusandra Jan 20, 2023
4f1843c
just clear results on error
mariusandra Jan 20, 2023
669e4ae
Merge branch 'master' into data-table-crash-detection
mariusandra Jan 20, 2023
c0c3040
Merge branch 'master' into data-table-crash-detection
mariusandra Jan 20, 2023
d51a32c
Merge branch 'master' into data-table-crash-detection
mariusandra Jan 20, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions frontend/src/initKea.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ const ERROR_FILTER_WHITELIST = [
'signup', // Special error handling on login
'loadLatestVersion',
'loadBilling', // Gracefully handled if it fails
'loadData', // Gracefully handled in the data table
]

interface InitKeaProps {
Expand Down
4 changes: 2 additions & 2 deletions frontend/src/queries/nodes/DataNode/ElapsedTime.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { dataNodeLogic } from '~/queries/nodes/DataNode/dataNodeLogic'
import { useState } from 'react'

export function ElapsedTime(): JSX.Element | null {
const { elapsedTime, loadingStart } = useValues(dataNodeLogic)
const { elapsedTime, loadingStart, responseError } = useValues(dataNodeLogic)
const [, setTick] = useState(0)

let time = elapsedTime
Expand All @@ -19,5 +19,5 @@ export function ElapsedTime(): JSX.Element | null {
return null
}

return <div>{`${(time / 1000).toFixed(time < 1000 ? 2 : 1)}s`}</div>
return <div className={responseError ? 'text-danger' : ''}>{`${(time / 1000).toFixed(time < 1000 ? 2 : 1)}s`}</div>
}
23 changes: 18 additions & 5 deletions frontend/src/queries/nodes/DataNode/dataNodeLogic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,13 +62,14 @@ export const dataNodeLogic = kea<dataNodeLogicType>([
const methodOptions: ApiMethodOptions = {
signal: cache.abortController.signal,
}
const now = performance.now()
try {
const now = performance.now()
const data = (await query<DataNode>(props.query, methodOptions, refresh)) ?? null
breakpoint()
actions.setElapsedTime(performance.now() - now)
return data
} catch (e: any) {
actions.setElapsedTime(performance.now() - now)
if (e.name === 'AbortError' || e.message?.name === 'AbortError') {
return values.response
} else {
Expand Down Expand Up @@ -161,6 +162,18 @@ export const dataNodeLogic = kea<dataNodeLogicType>([
loadNextData: () => performance.now(),
},
],
response: {
// Clear the response if a failure to avoid showing inconsistencies in the UI
loadDataFailure: () => null,
},
responseError: [
null as string | null,
{
loadData: () => null,
loadDataFailure: () => 'Error loading data',
loadDataSuccess: () => null,
},
],
elapsedTime: [
null as number | null,
{
Expand Down Expand Up @@ -199,9 +212,9 @@ export const dataNodeLogic = kea<dataNodeLogicType>([
],
canLoadNewData: [(s) => [s.newQuery], (newQuery) => !!newQuery],
nextQuery: [
(s, p) => [p.query, s.response],
(query, response): DataNode | null => {
if (isEventsQuery(query)) {
(s, p) => [p.query, s.response, s.responseError],
(query, response, responseError): DataNode | null => {
if (isEventsQuery(query) && !responseError) {
if ((response as EventsQuery['response'])?.hasMore) {
const sortKey = query.orderBy?.[0] ?? '-timestamp'
const typedResults = (response as EventsQuery['response'])?.results
Expand All @@ -225,7 +238,7 @@ export const dataNodeLogic = kea<dataNodeLogicType>([
}
}
}
if (isPersonsNode(query) && response) {
if (isPersonsNode(query) && response && !responseError) {
const personsResults = (response as PersonsNode['response'])?.results
const nextQuery: PersonsNode = {
...query,
Expand Down
434 changes: 218 additions & 216 deletions frontend/src/queries/nodes/DataTable/DataTable.tsx

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion frontend/src/queries/nodes/DataTable/EventRowActions.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ export function EventRowActions({ event }: EventActionProps): JSX.Element {
Create action from event
</LemonButton>
)}
{!!event.properties.$session_id && (
{!!event.properties?.$session_id && (
<LemonButton
status="stealth"
to={urls.sessionRecording(event.properties.$session_id)}
Expand Down
22 changes: 9 additions & 13 deletions frontend/src/queries/nodes/DataTable/dataTableLogic.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { initKeaTests } from '~/test/init'
import { featureFlagLogic } from 'lib/logic/featureFlagLogic'
import { expectLogic, partial } from 'kea-test-utils'
import { categoryRowKey, dataTableLogic } from '~/queries/nodes/DataTable/dataTableLogic'
import { dataTableLogic } from '~/queries/nodes/DataTable/dataTableLogic'
import { dataNodeLogic } from '~/queries/nodes/DataNode/dataNodeLogic'
import { DataTableNode, NodeKind } from '~/queries/schema'
import { query } from '~/queries/query'
Expand Down Expand Up @@ -209,18 +209,14 @@ describe('dataTableLogic', () => {
.delay(0)
.toMatchValues({ responseLoading: false, response: partial({ results }) })
await expectLogic(logic).toMatchValues({
resultsWithLabelRows: [
results[0],
{
[categoryRowKey]: 'December 23, 2022',
},
results[1],
results[2],
{
[categoryRowKey]: 'December 22, 2022',
},
results[3],
results[4],
dataTableRows: [
{ result: results[0] },
{ label: 'December 23, 2022' },
{ result: results[1] },
{ result: results[2] },
{ label: 'December 22, 2022' },
{ result: results[3] },
{ result: results[4] },
],
})
})
Expand Down
49 changes: 36 additions & 13 deletions frontend/src/queries/nodes/DataTable/dataTableLogic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,22 @@ import { featureFlagLogic } from 'lib/logic/featureFlagLogic'
import { FEATURE_FLAGS } from 'lib/constants'
import { dataNodeLogic } from '~/queries/nodes/DataNode/dataNodeLogic'
import { dayjs } from 'lib/dayjs'
import equal from 'fast-deep-equal'

export interface DataTableLogicProps {
key: string
query: DataTableNode
}

export const categoryRowKey = Symbol('__categoryRow__')
export interface DataTableRow {
/** Display a row with a label. */
label?: JSX.Element | string | null
/** Display a row with results */
result?: Record<string, any> | any[]
}

export const loadingColumn = Symbol('...')
export const errorColumn = Symbol('Error!')

export const dataTableLogic = kea<dataTableLogicType>([
props({} as DataTableLogicProps),
Expand All @@ -37,7 +46,7 @@ export const dataTableLogic = kea<dataTableLogicType>([
featureFlagLogic,
['featureFlags'],
dataNodeLogic({ key: props.key, query: props.query.source }),
['response', 'responseLoading'],
['response', 'responseLoading', 'responseError'],
],
})),
selectors({
Expand All @@ -57,41 +66,55 @@ export const dataTableLogic = kea<dataTableLogicType>([
? (response?.columns as string[])
: null,
],
resultsWithLabelRows: [
(s) => [s.sourceKind, s.orderBy, s.response, s.columnsInResponse, s.columnsInQuery],
(sourceKind, orderBy, response, columnsInResponse, columnsInQuery): any[] | null => {
if (sourceKind === NodeKind.EventsQuery) {
const results = (response as EventsQuery['response'] | null)?.results
if (results) {
dataTableRows: [
(s) => [s.sourceKind, s.orderBy, s.response, s.columnsInQuery],
(sourceKind, orderBy, response: AnyDataNode['response'], columnsInQuery): DataTableRow[] | null => {
if (response && sourceKind === NodeKind.EventsQuery) {
const eventsQueryResponse = response as EventsQuery['response'] | null
if (eventsQueryResponse) {
const { results, columns: columnsInResponse } = eventsQueryResponse
const orderKey = orderBy?.[0]?.startsWith('-') ? orderBy[0].slice(1) : orderBy?.[0]
const orderKeyIndex = (columnsInResponse ?? columnsInQuery).findIndex(
const orderKeyIndex = columnsInResponse.findIndex(
(column) =>
removeExpressionComment(column) === orderKey ||
removeExpressionComment(column) === `-${orderKey}`
)

const columnMap = Object.fromEntries(columnsInResponse.map((c, i) => [c, i]))
const resultToDataTableRow = equal(columnsInQuery, columnsInResponse)
? (result: any[]): DataTableRow => ({ result })
: (result: any[]): DataTableRow => ({
result: columnsInQuery.map((c) =>
c in columnMap ? result[columnMap[c]] : loadingColumn
),
})

// Add a label between results if the day changed
if (orderKey === 'timestamp' && orderKeyIndex !== -1) {
let lastResult: any | null = null
const newResults: any[] = []
const newResults: DataTableRow[] = []
for (const result of results) {
if (
result &&
lastResult &&
!dayjs(result[orderKeyIndex]).isSame(lastResult[orderKeyIndex], 'day')
) {
newResults.push({
[categoryRowKey]: dayjs(result[orderKeyIndex]).format('LL'),
label: dayjs(result[orderKeyIndex]).format('LL'),
})
}
newResults.push(result)
newResults.push(resultToDataTableRow(result))
lastResult = result
}
return newResults
} else {
return results.map((result) => resultToDataTableRow(result))
}
}
}
return response && 'results' in response ? (response as any).results ?? null : null
return response && 'results' in response && Array.isArray(response.results)
? response.results.map((result: any) => ({ result })) ?? null
: null
},
],
queryWithDefaults: [
Expand Down
9 changes: 8 additions & 1 deletion frontend/src/queries/nodes/DataTable/renderColumn.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ import { combineUrl, router } from 'kea-router'
import { CopyToClipboardInline } from 'lib/components/CopyToClipboard'
import { DeletePersonButton } from '~/queries/nodes/PersonsNode/DeletePersonButton'
import ReactJson from 'react-json-view'
import { errorColumn, loadingColumn } from '~/queries/nodes/DataTable/dataTableLogic'
import { Spinner } from 'lib/components/Spinner/Spinner'
import { LemonTag } from 'lib/components/LemonTag/LemonTag'

export function renderColumn(
key: string,
Expand All @@ -21,7 +24,11 @@ export function renderColumn(
setQuery?: (query: DataTableNode) => void,
context?: QueryContext
): JSX.Element | string {
if (key === 'event' && isEventsQuery(query.source)) {
if (value === loadingColumn) {
return <Spinner />
} else if (value === errorColumn) {
return <LemonTag color="red">Error</LemonTag>
} else if (key === 'event' && isEventsQuery(query.source)) {
const resultRow = record as any[]
const eventRecord = query.source.select.includes('*') ? resultRow[query.source.select.indexOf('*')] : null

Expand Down