feat: test query cancellation

frontend/src/scenes/dashboard/dashboardLogic.queryCancellation.test.ts

@@ -0,0 +1,178 @@
+import { expectLogic, partial } from 'kea-test-utils'
+import { initKeaTests } from '~/test/init'
+import { dashboardLogic } from 'scenes/dashboard/dashboardLogic'
+import { dashboardsModel } from '~/models/dashboardsModel'
+import { insightsModel } from '~/models/insightsModel'
+import { eventUsageLogic } from 'lib/utils/eventUsageLogic'
+import { DashboardType, InsightModel, InsightShortId } from '~/types'
+import { useMocks } from '~/mocks/jest'
+import { now } from 'lib/dayjs'
+import { MOCK_TEAM_ID } from 'lib/api.mock'
+import api from 'lib/api'
+import { boxToString, dashboardResult, insightOnDashboard, tileFromInsight } from 'scenes/dashboard/dashboardLogic.test'
+
+const seenQueryIDs: string[] = []
+
+describe('dashboardLogic query cancellation', () => {
+    let logic: ReturnType<typeof dashboardLogic.build>
+
+    let dashboards: Record<number, DashboardType> = {}
+
+    let dashboardTwelveInsightLoadedCount = 0
+
+    beforeEach(() => {
+        jest.spyOn(api, 'update')
+
+        const insights: Record<number, InsightModel> = {
+            2040: {
+                ...insightOnDashboard(2040, [12]),
+                id: 2040,
+                short_id: '2040' as InsightShortId,
+                last_refresh: now().toISOString(),
+            },
+        }
+        dashboards = {
+            12: {
+                ...dashboardResult(12, [tileFromInsight(insights['2040'])]),
+            },
+        }
+        useMocks({
+            get: {
+                '/api/projects/:team/dashboards/12/': { ...dashboards['12'] },
+                '/api/projects/:team/dashboards/': {
+                    count: 1,
+                    next: null,
+                    previous: null,
+                    results: [{ ...dashboards['12'] }],
+                },
+                '/api/projects/:team/insights/:id/': (req) => {
+                    const clientQueryId = req.url.searchParams.get('client_query_id')
+                    if (clientQueryId !== null) {
+                        seenQueryIDs.push(clientQueryId)
+                    }
+
+                    const dashboard = req.url.searchParams.get('from_dashboard')
+                    if (!dashboard) {
+                        throw new Error('the logic must always add this param')
+                    }
+                    const matched = insights[boxToString(req.params['id'])]
+                    if (!matched) {
+                        return [404, null]
+                    }
+                    if (dashboard === '12') {
+                        dashboardTwelveInsightLoadedCount++
+                        // delay for 2 seconds before response without pausing
+                        // but only the first time that dashboard 12 refreshes its results
+                        if (dashboardTwelveInsightLoadedCount === 1) {
+                            return new Promise((resolve) =>
+                                setTimeout(() => {
+                                    resolve([200, matched])
+                                }, 2000)
+                            )
+                        }
+                    }
+                    return [200, matched]
+                },
+            },
+            post: {
+                '/api/projects/:team/insights/cancel/': [201],
+            },
+            patch: {
+                '/api/projects/:team/dashboards/:id/': async (req) => {
+                    const dashboardId = typeof req.params['id'] === 'string' ? req.params['id'] : req.params['id'][0]
+                    const payload = await req.json()
+                    return [200, { ...dashboards[dashboardId], ...payload }]
+                },
+                '/api/projects/:team/insights/:id/': async (req) => {
+                    try {
+                        const updates = await req.json()
+                        if (typeof updates !== 'object') {
+                            return [500, `this update should receive an object body not ${JSON.stringify(updates)}`]
+                        }
+                        const insightId = boxToString(req.params.id)
+
+                        const starting: InsightModel = insights[insightId]
+                        insights[insightId] = {
+                            ...starting,
+                            ...updates,
+                        }
+
+                        starting.dashboards?.forEach((dashboardId) => {
+                            // remove this insight from any dashboard it is already on
+                            dashboards[dashboardId].tiles = dashboards[dashboardId].tiles.filter(
+                                (t) => !!t.insight && t.insight.id !== starting.id
+                            )
+                        })
+
+                        insights[insightId].dashboards?.forEach((dashboardId: number) => {
+                            // then add it to any it new references
+                            dashboards[dashboardId].tiles.push(tileFromInsight(insights[insightId]))
+                        })
+
+                        return [200, insights[insightId]]
+                    } catch (e) {
+                        return [500, e]
+                    }
+                },
+            },
+        })
+        initKeaTests()
+        dashboardsModel.mount()
+        insightsModel.mount()
+    })
+
+    describe('cancelling queries', () => {
+        beforeEach(async () => {
+            logic = dashboardLogic({ id: 12 })
+            logic.mount()
+            await expectLogic(logic).toFinishAllListeners()
+        })
+
+        it('cancels a running query', async () => {
+            jest.spyOn(api, 'create')
+
+            setTimeout(() => {
+                // this change of filters will dispatch cancellation on the first query
+                // will run while the -180d query is still running
+                logic.actions.setDates('-90d', null)
+            }, 200)
+            // dispatches an artificially slow data request
+            // takes 3000 milliseconds to return
+            logic.actions.setDates('-180d', null)
+
+            await expectLogic(logic)
+                .toDispatchActions([
+                    'setDates',
+                    'updateFilters',
+                    'abortAnyRunningQuery',
+                    'refreshAllDashboardItems',
+                    'abortAnyRunningQuery',
+                    'setDates',
+                    'updateFilters',
+                    'abortAnyRunningQuery',
+                    'abortQuery',
+                    'refreshAllDashboardItems',
+                    eventUsageLogic.actionTypes.reportDashboardRefreshed,
+                ])
+                .toMatchValues({
+                    filters: partial({ date_from: '-90d' }),
+                })
+
+            const mockCreateCalls = (api.create as jest.Mock).mock.calls
+            // there will be at least two used client query ids
+            // the most recent has not been cancelled
+            // the one before that has been
+            // the query IDs are made of `${dashboard query ID}::{insight query ID}`
+            // only the dashboard query ID is sent to the API
+            const cancelledQueryID = seenQueryIDs[seenQueryIDs.length - 2].split('::')[0]
+            expect(mockCreateCalls).toEqual([
+                [
+                    `api/projects/${MOCK_TEAM_ID}/insights/cancel`,
+                    {
+                        client_query_id: cancelledQueryID,
+                    },
+                ],
+            ])
+        })
+    })
+})

frontend/src/scenes/dashboard/dashboardLogic.test.ts

@@ -27,7 +27,7 @@ import api from 'lib/api'
 
 const dashboardJson = _dashboardJson as any as DashboardType
 
-function insightOnDashboard(
+export function insightOnDashboard(
     insightId: number,
     dashboardsRelation: number[],
     insight: Partial<InsightModel> = {}
@@ -54,7 +54,7 @@ const TEXT_TILE: DashboardTile = {
 }
 
 let tileId = 0
-const tileFromInsight = (insight: InsightModel, id: number = tileId++): DashboardTile => ({
+export const tileFromInsight = (insight: InsightModel, id: number = tileId++): DashboardTile => ({
     id: id,
     layouts: {},
     color: null,
@@ -64,7 +64,7 @@ const tileFromInsight = (insight: InsightModel, id: number = tileId++): Dashboar
     refreshing: false,
 })
 
-const dashboardResult = (
+export const dashboardResult = (
     dashboardId: number,
     tiles: DashboardTile[],
     filters: Partial<Pick<FilterType, 'date_from' | 'date_to' | 'properties'>> = {}
@@ -79,12 +79,12 @@ const dashboardResult = (
 
 const uncached = (insight: InsightModel): InsightModel => ({ ...insight, result: null, last_refresh: null })
 
-const boxToString = (param: string | readonly string[]): string => {
+export const boxToString = (param: string | readonly string[]): string => {
     //path params from msw can be a string or an array
     if (typeof param === 'string') {
         return param
     } else {
-        throw new Error("this shouldn't be an arry")
+        throw new Error("this shouldn't be an array")
     }
 }
 
@@ -198,16 +198,18 @@ describe('dashboardLogic', () => {
                         throw new Error('the logic must always add this param')
                     }
                     const matched = insights[boxToString(req.params['id'])]
-                    if (matched) {
-                        return [200, matched]
-                    } else {
+                    if (!matched) {
                         return [404, null]
                     }
+                    return [200, matched]
                 },
             },
+            post: {
+                '/api/projects/:team/insights/cancel/': [201],
+            },
             patch: {
                 '/api/projects/:team/dashboards/:id/': async (req) => {
-                    const dashboardId = req.params['id'][0]
+                    const dashboardId = typeof req.params['id'] === 'string' ? req.params['id'] : req.params['id'][0]
                     const payload = await req.json()
                     return [200, { ...dashboards[dashboardId], ...payload }]
                 },

frontend/src/scenes/dashboard/dashboardLogic.tsx

@@ -20,7 +20,6 @@ import { insightsModel } from '~/models/insightsModel'
 import {
     AUTO_REFRESH_DASHBOARD_THRESHOLD_HOURS,
     DashboardPrivilegeLevel,
-    FEATURE_FLAGS,
     OrganizationMembershipLevel,
 } from 'lib/constants'
 import { DashboardEventSource, eventUsageLogic } from 'lib/utils/eventUsageLogic'
@@ -974,7 +973,6 @@ export const dashboardLogic = kea<dashboardLogicType>([
                     })
                     totalResponseBytes += getResponseBytes(refreshedInsightResponse)
                 } catch (e: any) {
-                    console.error(e)
                     if (isBreakpoint(e)) {
                         cancelled = true
                     } else if (e.name === 'AbortError' || e.message?.name === 'AbortError') {
@@ -990,8 +988,6 @@ export const dashboardLogic = kea<dashboardLogicType>([
 
                 refreshesFinished += 1
                 if (refreshesFinished === insights.length) {
-                    breakpoint()
-
                     const payload: TimeToSeeDataPayload = {
                         type: 'dashboard_load',
                         context: 'dashboard',
@@ -1163,23 +1159,22 @@ export const dashboardLogic = kea<dashboardLogicType>([
         },
         abortQuery: async ({ dashboardQueryId, queryId, queryStartTime }) => {
             const { currentTeamId } = values
-            if (values.featureFlags[FEATURE_FLAGS.CANCEL_RUNNING_QUERIES]) {
-                await api.create(`api/projects/${currentTeamId}/insights/cancel`, { client_query_id: dashboardQueryId })
-
-                // TRICKY: we cancel just once using the dashboard query id.
-                // we can record the queryId that happened to capture the AbortError exception
-                // and request the cancellation, but it is probably not particularly relevant
-                await captureTimeToSeeData(values.currentTeamId, {
-                    type: 'insight_load',
-                    context: 'dashboard',
-                    dashboard_query_id: dashboardQueryId,
-                    query_id: queryId,
-                    status: 'cancelled',
-                    time_to_see_data_ms: Math.floor(performance.now() - queryStartTime),
-                    insights_fetched: 0,
-                    insights_fetched_cached: 0,
-                })
-            }
+
+            await api.create(`api/projects/${currentTeamId}/insights/cancel`, { client_query_id: dashboardQueryId })
+
+            // TRICKY: we cancel just once using the dashboard query id.
+            // we can record the queryId that happened to capture the AbortError exception
+            // and request the cancellation, but it is probably not particularly relevant
+            await captureTimeToSeeData(values.currentTeamId, {
+                type: 'insight_load',
+                context: 'dashboard',
+                dashboard_query_id: dashboardQueryId,
+                query_id: queryId,
+                status: 'cancelled',
+                time_to_see_data_ms: Math.floor(performance.now() - queryStartTime),
+                insights_fetched: 0,
+                insights_fetched_cached: 0,
+            })
         },
     })),