fix(dashboards): Make sure loading dashboard items does not `POST /qu…

…ery/` (#24808)

cypress/e2e/dashboard-shared.cy.ts

@@ -62,7 +62,6 @@ describe('Shared dashboard', () => {
 
         cy.get('.InsightCard').should('have.length', 6)
         // Make sure no element with text "There are no matching events for this query" exists
-        // TODO this was failing, it shouldn't be but YOLO
-        // cy.get('.insight-empty-state').should('not.exist')
+        cy.get('.insight-empty-state').should('not.exist')
     })
 })

frontend/src/queries/nodes/DataNode/dataNodeLogic.ts

@@ -122,7 +122,11 @@ export const dataNodeLogic = kea<dataNodeLogicType>([
         }
     }),
     actions({
-        loadData: (refresh = false, queryId?: string) => ({ refresh, queryId: queryId || uuid() }),
+        loadData: (refresh = false, alreadyRunningQueryId?: string) => ({
+            refresh,
+            queryId: alreadyRunningQueryId || uuid(),
+            pollOnly: !!alreadyRunningQueryId,
+        }),
         abortAnyRunningQuery: true,
         abortQuery: (payload: { queryId: string }) => payload,
         cancelQuery: true,
@@ -142,7 +146,7 @@ export const dataNodeLogic = kea<dataNodeLogicType>([
             {
                 setResponse: (response) => response,
                 clearResponse: () => null,
-                loadData: async ({ refresh: refreshArg, queryId }, breakpoint) => {
+                loadData: async ({ refresh: refreshArg, queryId, pollOnly }, breakpoint) => {
                     const refresh = props.alwaysRefresh || refreshArg
                     if (props.doNotLoad) {
                         return props.cachedResults
@@ -200,7 +204,8 @@ export const dataNodeLogic = kea<dataNodeLogicType>([
                                             methodOptions,
                                             refresh,
                                             queryId,
-                                            actions.setPollResponse
+                                            actions.setPollResponse,
+                                            pollOnly
                                         )) ?? null
                                     const duration = performance.now() - now
                                     return { data, duration }

frontend/src/queries/nodes/DataTable/dataTableLogic.test.ts

@@ -65,7 +65,8 @@ describe('dataTableLogic', () => {
             expect.anything(),
             false,
             expect.any(String),
-            expect.any(Function)
+            expect.any(Function),
+            false
         )
         expect(performQuery).toHaveBeenCalledTimes(1)
     })

frontend/src/queries/query.ts

@@ -7,7 +7,14 @@ import posthog from 'posthog-js'
 import { OnlineExportContext, QueryExportContext } from '~/types'
 
 import { DataNode, HogQLQuery, HogQLQueryResponse, NodeKind, PersonsNode, QueryStatus } from './schema'
-import { isDataTableNode, isDataVisualizationNode, isHogQLQuery, isInsightVizNode, isPersonsNode } from './utils'
+import {
+    isAsyncResponse,
+    isDataTableNode,
+    isDataVisualizationNode,
+    isHogQLQuery,
+    isInsightVizNode,
+    isPersonsNode,
+} from './utils'
 
 const QUERY_ASYNC_MAX_INTERVAL_SECONDS = 3
 const QUERY_ASYNC_TOTAL_POLL_SECONDS = 10 * 60 + 6 // keep in sync with backend-side timeout (currently 10min) + a small buffer
@@ -29,7 +36,6 @@ export function queryExportContext<N extends DataNode>(
 const SYNC_ONLY_QUERY_KINDS = [
     'HogQuery',
     'HogQLMetadata',
-    'EventsQuery',
     'HogQLAutocomplete',
     'DatabaseSchemaQuery',
     'ErrorTrackingQuery',
@@ -39,7 +45,7 @@ export async function pollForResults(
     queryId: string,
     showProgress: boolean,
     methodOptions?: ApiMethodOptions,
-    callback?: (response: QueryStatus) => void
+    onPoll?: (response: QueryStatus) => void
 ): Promise<QueryStatus> {
     const pollStart = performance.now()
     let currentDelay = 300 // start low, because all queries will take at minimum this
@@ -50,12 +56,11 @@ export async function pollForResults(
 
         try {
             const statusResponse = (await api.queryStatus.get(queryId, showProgress)).query_status
-
             if (statusResponse.complete) {
                 return statusResponse
             }
-            if (callback) {
-                callback(statusResponse)
+            if (onPoll) {
+                onPoll(statusResponse)
             }
         } catch (e: any) {
             e.detail = e.data?.query_status?.error_message
@@ -73,27 +78,42 @@ async function executeQuery<N extends DataNode>(
     methodOptions?: ApiMethodOptions,
     refresh?: boolean,
     queryId?: string,
-    setPollResponse?: (response: QueryStatus) => void
+    setPollResponse?: (response: QueryStatus) => void,
+    /**
+     * Whether to limit the function to just polling the provided query ID.
+     * This is important in shared contexts, where we cannot create arbitrary queries via POST – we can only GET.
+     */
+    pollOnly = false
 ): Promise<NonNullable<N['response']>> {
     const isAsyncQuery =
         methodOptions?.async !== false &&
         !SYNC_ONLY_QUERY_KINDS.includes(queryNode.kind) &&
         !!featureFlagLogic.findMounted()?.values.featureFlags?.[FEATURE_FLAGS.QUERY_ASYNC]
-
     const showProgress = !!featureFlagLogic.findMounted()?.values.featureFlags?.[FEATURE_FLAGS.INSIGHT_LOADING_BAR]
 
-    const response = await api.query(queryNode, methodOptions, queryId, refresh, isAsyncQuery)
+    if (!pollOnly) {
+        const response = await api.query(queryNode, methodOptions, queryId, refresh, isAsyncQuery)
 
-    if (!response.query_status?.query_async) {
-        // Executed query synchronously or from cache
-        return response
-    }
-    if (response.query_status?.complete) {
-        // Async query returned immediately
-        return response.results
-    }
+        if (!isAsyncResponse(response)) {
+            // Executed query synchronously or from cache
+            return response
+        }
 
-    const statusResponse = await pollForResults(response.query_status.id, showProgress, methodOptions, setPollResponse)
+        if (response.query_status.complete) {
+            // Async query returned immediately
+            return response.results
+        }
+
+        queryId = response.query_status.id
+    } else {
+        if (!isAsyncQuery) {
+            throw new Error('pollOnly is only supported for async queries')
+        }
+        if (!queryId) {
+            throw new Error('pollOnly requires a queryId')
+        }
+    }
+    const statusResponse = await pollForResults(queryId, showProgress, methodOptions, setPollResponse)
     return statusResponse.results
 }
 
@@ -103,7 +123,8 @@ export async function performQuery<N extends DataNode>(
     methodOptions?: ApiMethodOptions,
     refresh?: boolean,
     queryId?: string,
-    setPollResponse?: (status: QueryStatus) => void
+    setPollResponse?: (status: QueryStatus) => void,
+    pollOnly = false
 ): Promise<NonNullable<N['response']>> {
     let response: NonNullable<N['response']>
     const logParams: Record<string, any> = {}
@@ -113,7 +134,7 @@ export async function performQuery<N extends DataNode>(
         if (isPersonsNode(queryNode)) {
             response = await api.get(getPersonsEndpoint(queryNode), methodOptions)
         } else {
-            response = await executeQuery(queryNode, methodOptions, refresh, queryId, setPollResponse)
+            response = await executeQuery(queryNode, methodOptions, refresh, queryId, setPollResponse, pollOnly)
             if (isHogQLQuery(queryNode) && response && typeof response === 'object') {
                 logParams.clickhouse_sql = (response as HogQLQueryResponse)?.clickhouse
             }

frontend/src/queries/utils.ts

@@ -30,6 +30,8 @@ import {
     NodeKind,
     PathsQuery,
     PersonsNode,
+    QuerySchema,
+    QueryStatusResponse,
     RetentionQuery,
     SavedInsightNode,
     SessionAttributionExplorerQuery,
@@ -78,6 +80,7 @@ export function isDataWarehouseNode(node?: Record<string, any> | null): node is
     return node?.kind === NodeKind.DataWarehouseNode
 }
 
+/** @deprecated `ActorsQuery` is now used instead of `PersonsNode`. */
 export function isPersonsNode(node?: Record<string, any> | null): node is PersonsNode {
     return node?.kind === NodeKind.PersonsNode
 }
@@ -204,6 +207,10 @@ export function isQueryForGroup(query: PersonsNode | ActorsQuery): boolean {
     )
 }
 
+export function isAsyncResponse(response: NonNullable<QuerySchema['response']>): response is QueryStatusResponse {
+    return 'query_status' in response && response.query_status
+}
+
 export function isInsightQueryWithSeries(
     node?: Node
 ): node is TrendsQuery | FunnelsQuery | StickinessQuery | LifecycleQuery {

frontend/src/scenes/funnels/funnelUtils.ts

@@ -389,7 +389,7 @@ export function flattenedStepsByBreakdown(
                     breakdown_value: 'Baseline',
                 })),
                 conversionRates: {
-                    total: (lastStep?.count ?? 0) / (baseStep?.count ?? 1),
+                    total: (lastStep?.count || 0) / (baseStep?.count || 1),
                 },
             })
         }

posthog/models/person_overrides/sql.py

@@ -69,7 +69,7 @@
     ENGINE = ReplicatedReplacingMergeTree(
         -- NOTE: for testing we use a uuid to ensure that we don't get conflicts
         -- when the tests tear down and recreate the table.
-        '/clickhouse/tables/{'{uuid}' if settings.TEST else ''}noshard/{CLICKHOUSE_DATABASE}.person_overrides',
+        '/clickhouse/tables/{'{uuid}' if settings.TEST or settings.E2E_TESTING else ''}noshard/{CLICKHOUSE_DATABASE}.person_overrides',
         '{{replica}}-{{shard}}',
         version
     )