diff --git a/frontend/src/scenes/dashboard/dashboardLogic.test.ts b/frontend/src/scenes/dashboard/dashboardLogic.test.ts index ce18870f1bc7c..f4650e1473804 100644 --- a/frontend/src/scenes/dashboard/dashboardLogic.test.ts +++ b/frontend/src/scenes/dashboard/dashboardLogic.test.ts @@ -16,7 +16,6 @@ import { InsightShortId, InsightType, TextModel, - TileLayout, } from '~/types' import { resumeKeaLoadersErrors, silenceKeaLoadersErrors } from '~/initKea' import { useMocks } from '~/mocks/jest' @@ -24,6 +23,7 @@ import { dayjs, now } from 'lib/dayjs' import { teamLogic } from 'scenes/teamLogic' import { MOCK_TEAM_ID } from 'lib/api.mock' import api from 'lib/api' +import { sceneLogic } from 'scenes/sceneLogic' const dashboardJson = _dashboardJson as any as DashboardType @@ -274,6 +274,7 @@ describe('dashboardLogic', () => { initKeaTests() dashboardsModel.mount() insightsModel.mount() + sceneLogic.mount() }) describe('tile layouts', () => { @@ -299,77 +300,6 @@ describe('dashboardLogic', () => { ], }, }) - .toDispatchActions([ - // frustratingly the same properties are sent in a different order - // and the matcher cares about the order - logic.actionCreators.saveLayouts([ - { - id: 0, - layouts: { - sm: { i: '0', x: 0, y: 0, w: 8, h: 8, minW: 3, minH: 5 }, - xs: { i: '0', x: 0, y: 0, w: 1, h: 8, minW: 1, minH: 5 }, - }, - }, - { - id: 1, - layouts: { - sm: { i: '1', x: 0, y: 8, w: 6, h: 5, minW: 3, minH: 5 }, - xs: { i: '1', x: 0, y: 8, w: 1, h: 5, minW: 1, minH: 5 }, - }, - }, - { - id: 4, - layouts: { - sm: { i: '4', x: 6, y: 8, w: 6, h: 6, minW: 3, minH: 5 }, - xs: { i: '4', x: 0, y: 13, w: 1, h: 6, minW: 1, minH: 5 }, - }, - }, - ]), - ]) - }) - - it('saving layouts with no provided tiles updates all tiles', async () => { - jest.spyOn(api, 'update') - - await expectLogic(logic, () => { - logic.actions.saveLayouts() - }).toFinishAllListeners() - - expect(api.update).toHaveBeenCalledWith(`api/projects/${MOCK_TEAM_ID}/dashboards/5`, { - no_items_field: true, - tiles: [ - { - id: 9, - layouts: {}, - }, - { - id: 10, - layouts: {}, - }, - { - id: 4, - layouts: {}, - }, - ], - }) - }) - - it('saving layouts with provided tiles updates only those tiles', async () => { - jest.spyOn(api, 'update') - - await expectLogic(logic, () => { - logic.actions.saveLayouts([{ id: 1, layouts: { sm: {} as TileLayout, xs: {} as TileLayout } }]) - }).toFinishAllListeners() - - expect(api.update).toHaveBeenCalledWith(`api/projects/${MOCK_TEAM_ID}/dashboards/5`, { - no_items_field: true, - tiles: [ - { - id: 1, - layouts: { sm: {} as TileLayout, xs: {} as TileLayout }, - }, - ], - }) }) }) @@ -804,6 +734,7 @@ describe('dashboardLogic', () => { }) }) }) + describe('lastRefreshed', () => { it('should be the earliest refreshed dashboard', async () => { logic = dashboardLogic({ id: 5 }) @@ -891,18 +822,15 @@ describe('dashboardLogic', () => { beforeEach(async () => { logic = dashboardLogic({ id: 5 }) logic.mount() - await expectLogic(logic).toFinishAllListeners() }) it('can remove text tiles', async () => { await expectLogic(logic, () => { logic.actions.removeTile(TEXT_TILE) - }) - .toFinishAllListeners() - .toDispatchActions([ - dashboardsModel.actionTypes.tileRemovedFromDashboard, - logic.actionTypes.removeTileSuccess, - ]) + }).toDispatchActions([ + dashboardsModel.actionTypes.tileRemovedFromDashboard, + logic.actionTypes.removeTileSuccess, + ]) expect(logic.values.textTiles).toEqual([]) }) diff --git a/frontend/src/scenes/dashboard/dashboardLogic.tsx b/frontend/src/scenes/dashboard/dashboardLogic.tsx index 000ed665a92b8..c557a79a5d93f 100644 --- a/frontend/src/scenes/dashboard/dashboardLogic.tsx +++ b/frontend/src/scenes/dashboard/dashboardLogic.tsx @@ -1,12 +1,26 @@ -import { isBreakpoint, kea } from 'kea' -import api, { getJSONOrThrow } from 'lib/api' +import { + actions, + connect, + events, + isBreakpoint, + kea, + key, + listeners, + path, + props, + reducers, + selectors, + sharedListeners, +} from 'kea' +import api, { ApiMethodOptions, getJSONOrThrow } from 'lib/api' import { dashboardsModel } from '~/models/dashboardsModel' -import { router } from 'kea-router' -import { areObjectValuesEmpty, clearDOMTextSelection, isUserLoggedIn, toParams, uuid } from 'lib/utils' +import { router, urlToAction } from 'kea-router' +import { clearDOMTextSelection, toParams, uuid } from 'lib/utils' 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' @@ -38,6 +52,11 @@ import { Link } from 'lib/components/Link' import { isPathsFilter, isRetentionFilter, isTrendsFilter } from 'scenes/insights/sharedUtils' import { captureTimeToSeeData, TimeToSeeDataPayload } from 'lib/internalMetrics' import { getResponseBytes, sortDates } from '../insights/utils' +import { featureFlagLogic } from 'lib/logic/featureFlagLogic' +import { Scene } from 'scenes/sceneTypes' +import { subscriptions } from 'kea-subscriptions' +import { loaders } from 'kea-loaders' +import { sceneLogic } from 'scenes/sceneLogic' export const BREAKPOINTS: Record = { sm: 1024, @@ -106,29 +125,26 @@ function updateExistingInsightState({ cachedInsight, dashboardId, refreshedInsig } } -export const dashboardLogic = kea({ - path: ['scenes', 'dashboard', 'dashboardLogic'], - connect: () => ({ - values: [teamLogic, ['currentTeamId']], +export const dashboardLogic = kea([ + path(['scenes', 'dashboard', 'dashboardLogic']), + connect(() => ({ + values: [teamLogic, ['currentTeamId'], featureFlagLogic, ['featureFlags'], sceneLogic, ['scene']], logic: [dashboardsModel, insightsModel, eventUsageLogic], - }), - - props: {} as DashboardLogicProps, - - key: (props) => { + })), + props({} as DashboardLogicProps), + key((props: DashboardLogicProps) => { if (typeof props.id === 'string') { throw Error('Must init dashboardLogic with a numeric key') } - return props.id ?? 'new' - }, - - actions: { + return props.id?.toString() || 'new' + }), + actions({ loadExportedDashboard: (dashboard: DashboardType | null) => ({ dashboard }), - loadDashboardItems: (payload: { refresh?: boolean; action: string }) => payload, + loadDashboardItems: (payload: LoadDashboardItemsProps) => payload, triggerDashboardUpdate: (payload) => ({ payload }), /** The current state in which the dashboard is being viewed, see DashboardMode. */ setDashboardMode: (mode: DashboardMode | null, source: DashboardEventSource | null) => ({ mode, source }), - saveLayouts: (tilesToSave: DashboardTileLayoutUpdatePayload[] = []) => ({ tilesToSave }), + saveLayouts: true, updateLayouts: (layouts: Layouts) => ({ layouts }), updateContainerWidth: (containerWidth: number, columns: number) => ({ containerWidth, columns }), updateTileColor: (tileId: number, color: string | null) => ({ tileId, color }), @@ -172,9 +188,9 @@ export const dashboardLogic = kea({ duplicateTile: (tile: DashboardTile) => ({ tile }), loadingDashboardItemsStarted: (action: string, dashboardQueryId: string) => ({ action, dashboardQueryId }), setInitialLoadResponseBytes: (responseBytes: number) => ({ responseBytes }), - }, - - loaders: ({ actions, props, values }) => ({ + abortQuery: (payload: { queryId: string; exception?: Record }) => payload, + }), + loaders(({ actions, props, values }) => ({ // TODO this is a terrible name... it is "dashboard" but there's a "dashboard" reducer ¯\_(ツ)_/¯ allItems: [ null as DashboardType | null, @@ -276,8 +292,8 @@ export const dashboardLogic = kea({ }, }, ], - }), - reducers: ({ props }) => ({ + })), + reducers(({ props }) => ({ receivedErrorsFromAPI: [ false, { @@ -488,6 +504,7 @@ export const dashboardLogic = kea({ [shortId]: { error: true, timer: state[shortId]?.timer || null }, }), refreshAllDashboardItems: () => ({}), + abortQuery: () => ({}), }, ], columns: [ @@ -563,8 +580,8 @@ export const dashboardLogic = kea({ setTextTileId: (_, { textTileId }) => textTileId, }, ], - }), - selectors: ({ actions }) => ({ + })), + selectors(() => ({ asDashboardTemplate: [ (s) => [s.allItems], (dashboard: DashboardType): string => { @@ -674,8 +691,6 @@ export const dashboardLogic = kea({ layouts: [ (s) => [s.tiles], (tiles) => { - const tilesWithNoLayout = tiles.filter((t) => !t.layouts || areObjectValuesEmpty(t.layouts)) - const allLayouts: Partial> = {} for (const col of Object.keys(BREAKPOINT_COLUMN_COUNTS) as (keyof typeof BREAKPOINT_COLUMN_COUNTS)[]) { @@ -757,15 +772,6 @@ export const dashboardLogic = kea({ allLayouts[col] = cleanLayouts } - if (tilesWithNoLayout.length > 0) { - const layoutsByTileId = layoutsByTile(allLayouts) - actions.saveLayouts( - tilesWithNoLayout.map((t) => ({ - id: t.id, - layouts: layoutsByTileId[t.id], - })) - ) - } return allLayouts }, ], @@ -804,8 +810,8 @@ export const dashboardLogic = kea({ }, ], ], - }), - events: ({ actions, cache, props }) => ({ + })), + events(({ actions, cache, props }) => ({ afterMount: () => { if (props.id) { if (props.dashboard) { @@ -819,13 +825,14 @@ export const dashboardLogic = kea({ } }, beforeUnmount: () => { + cache.abortController?.abort() if (cache.autoRefreshInterval) { window.clearInterval(cache.autoRefreshInterval) cache.autoRefreshInterval = null } }, - }), - sharedListeners: ({ values, props }) => ({ + })), + sharedListeners(({ values, props }) => ({ reportRefreshTiming: ({ shortId }) => { const refreshStatus = values.refreshStatus[shortId] @@ -844,8 +851,8 @@ export const dashboardLogic = kea({ eventUsageLogic.actions.reportDashboardLoadingTime(loadingMilliseconds, props.id) } }, - }), - listeners: ({ actions, values, cache, props, sharedListeners }) => ({ + })), + listeners(({ actions, values, cache, props, sharedListeners }) => ({ setRefreshError: sharedListeners.reportRefreshTiming, setRefreshStatuses: sharedListeners.reportRefreshTiming, setRefreshStatus: sharedListeners.reportRefreshTiming, @@ -883,27 +890,33 @@ export const dashboardLogic = kea({ updateLayouts: () => { actions.saveLayouts() }, - saveLayouts: async ({ tilesToSave }, breakpoint) => { - await breakpoint(300) - if (!isUserLoggedIn()) { - // If user is anonymous (i.e. viewing a shared dashboard logged out), we don't save any layout changes. - return - } - if (!props.id) { - // what are we saving layouts against?! - return - } + saveLayouts: async (_, breakpoint) => { + try { + await breakpoint(300) - const layoutsToUpdate = tilesToSave.length - ? tilesToSave - : (values.allItems?.tiles || []).map((tile) => ({ id: tile.id, layouts: tile.layouts })) + if (!props.id) { + // what are we saving layouts against?! + return + } - breakpoint() + breakpoint() - return await api.update(`api/projects/${values.currentTeamId}/dashboards/${props.id}`, { - tiles: layoutsToUpdate, - no_items_field: true, - }) + const layoutsToUpdate = (values.allItems?.tiles || []).map((tile) => ({ + id: tile.id, + layouts: tile.layouts, + })) + + await api.update(`api/projects/${values.currentTeamId}/dashboards/${props.id}`, { + tiles: layoutsToUpdate, + no_items_field: true, + }) + } catch (e: any) { + if (isBreakpoint(e)) { + console.log('out of order layout save was ignored') + } else { + throw e + } + } }, moveToDashboardSuccess: ({ payload }) => { if (payload?.toDashboard === undefined || payload?.tile === undefined) { @@ -940,7 +953,7 @@ export const dashboardLogic = kea({ actions.resetInterval() actions.refreshAllDashboardItems({ action: 'refresh_manual' }) }, - refreshAllDashboardItems: async ({ tiles, action, initialLoad, dashboardQueryId }, breakpoint) => { + refreshAllDashboardItems: async ({ tiles, action, initialLoad, dashboardQueryId = uuid() }, breakpoint) => { if (!props.id) { // what are we loading the insight card on?! return @@ -956,31 +969,41 @@ export const dashboardLogic = kea({ return } - let breakpointTriggered = false + let cancelled = false actions.setRefreshStatuses( insights.map((item) => item.short_id), true ) + // If a query is in progress, kill that query + // we will use one abort controller for all insight queries for this dashboard + if (cache.abortController) { + cache.abortController.abort() + } + cache.abortController = new AbortController() + const methodOptions: ApiMethodOptions = { + signal: cache.abortController.signal, + } + const refreshStartTime = performance.now() - dashboardQueryId = dashboardQueryId ?? uuid() + let refreshesFinished = 0 let totalResponseBytes = 0 // array of functions that reload each item const fetchItemFunctions = insights.map((insight) => async () => { - // :TODO: Support query cancellation and use this queryId in the actual query. const queryId = `${dashboardQueryId}::${uuid()}` const queryStartTime = performance.now() const apiUrl = `api/projects/${values.currentTeamId}/insights/${insight.id}/?${toParams({ refresh: true, from_dashboard: dashboardId, // needed to load insight in correct context + client_query_id: queryId, })}` try { breakpoint() - const refreshedInsightResponse: Response = await api.getResponse(apiUrl) + const refreshedInsightResponse: Response = await api.getResponse(apiUrl, methodOptions) const refreshedInsight: InsightModel = await getJSONOrThrow(refreshedInsightResponse) breakpoint() updateExistingInsightState({ cachedInsight: insight, dashboardId, refreshedInsight }) @@ -1007,7 +1030,13 @@ export const dashboardLogic = kea({ } catch (e: any) { console.error(e) if (isBreakpoint(e)) { - breakpointTriggered = true + cancelled = true + } else if (e.name === 'AbortError' || e.message?.name === 'AbortError') { + if (!cancelled) { + // cancel all insight requests for this query in one go + actions.abortQuery({ queryId: dashboardQueryId }) + } + cancelled = true } else { actions.setRefreshError(insight.short_id) } @@ -1015,6 +1044,9 @@ export const dashboardLogic = kea({ refreshesFinished += 1 if (refreshesFinished === insights.length) { + breakpoint() + cache.abortController = null + const payload: TimeToSeeDataPayload = { type: 'dashboard_load', context: 'dashboard', @@ -1041,7 +1073,7 @@ export const dashboardLogic = kea({ // run 4 item reloaders in parallel function loadNextPromise(): void { - if (!breakpointTriggered && fetchItemFunctions.length > 0) { + if (!cancelled && fetchItemFunctions.length > 0) { fetchItemFunctions.shift()?.().then(loadNextPromise) } } @@ -1053,6 +1085,11 @@ export const dashboardLogic = kea({ eventUsageLogic.actions.reportDashboardRefreshed(dashboardId, values.lastRefreshed) }, updateAndRefreshDashboard: async (_, breakpoint) => { + if (cache.abortController) { + cache.abortController.abort() + } + cache.abortController = null + await breakpoint(200) await api.update(`api/projects/${values.currentTeamId}/dashboards/${props.id}`, { filters: values.filters, @@ -1188,9 +1225,14 @@ export const dashboardLogic = kea({ actions.setShouldReportOnAPILoad(true) } }, - }), - - urlToAction: ({ actions }) => ({ + abortQuery: ({ queryId }) => { + const { currentTeamId } = values + if (values.featureFlags[FEATURE_FLAGS.CANCEL_RUNNING_QUERIES]) { + api.create(`api/projects/${currentTeamId}/insights/cancel`, { client_query_id: queryId }) + } + }, + })), + urlToAction(({ actions }) => ({ '/dashboard/:id/subscriptions(/:subscriptionId)': ({ subscriptionId }) => { const id = subscriptionId ? subscriptionId == 'new' @@ -1217,5 +1259,18 @@ export const dashboardLogic = kea({ actions.setDashboardMode(null, null) actions.setTextTileId(textTileId === undefined ? 'new' : textTileId !== 'new' ? Number(textTileId) : 'new') }, - }), -}) + })), + subscriptions(({ cache }) => ({ + scene: (value: Scene | null | undefined, lastValue: Scene | null | undefined) => { + const scenesThatMightAbortClickHouseQueries = [Scene.Insight, Scene.Dashboard, Scene.ProjectHomepage] + if ( + value && + lastValue && + scenesThatMightAbortClickHouseQueries.includes(lastValue) && + !scenesThatMightAbortClickHouseQueries.includes(value) + ) { + cache.abortController?.abort() + } + }, + })), +]) diff --git a/frontend/src/scenes/insights/insightLogic.test.ts b/frontend/src/scenes/insights/insightLogic.test.ts index 5bd6cba7a4b62..d89d8a1b25aca 100644 --- a/frontend/src/scenes/insights/insightLogic.test.ts +++ b/frontend/src/scenes/insights/insightLogic.test.ts @@ -13,12 +13,10 @@ import { InsightModel, InsightShortId, InsightType, - ItemMode, PropertyFilterType, PropertyGroupFilter, PropertyOperator, } from '~/types' -import { eventUsageLogic } from 'lib/utils/eventUsageLogic' import { combineUrl, router } from 'kea-router' import { dashboardLogic } from 'scenes/dashboard/dashboardLogic' import { savedInsightsLogic } from 'scenes/saved-insights/savedInsightsLogic' @@ -32,6 +30,7 @@ import { MOCK_DEFAULT_TEAM } from 'lib/api.mock' import { dashboardsModel } from '~/models/dashboardsModel' import { insightsModel } from '~/models/insightsModel' import { DashboardPrivilegeLevel, DashboardRestrictionLevel } from 'lib/constants' +import { sceneLogic } from 'scenes/sceneLogic' const API_FILTERS: Partial = { insight: InsightType.TRENDS as InsightType, @@ -110,6 +109,7 @@ describe('insightLogic', () => { useAvailableFeatures([AvailableFeature.DASHBOARD_COLLABORATION]) useMocks({ get: { + '/api/projects/:team/tags/': { results: ['good', 'bad'] }, '/api/projects/:team/insights/trend/': (req) => { if (JSON.parse(req.url.searchParams.get('events') || '[]')?.[0]?.throw) { return [500, { status: 0, detail: 'error from the API' }] @@ -210,6 +210,7 @@ describe('insightLogic', () => { .toFinishAllListeners() .toMatchValues({ currentTeam: partial({ test_account_filters_default_checked: true }) }) insightsModel.mount() + sceneLogic.mount() }) it('requires props', () => { @@ -295,20 +296,7 @@ describe('insightLogic', () => { await expectLogic(logic, () => { logic.actions.setFilters({ insight: InsightType.FUNNELS }) - }).toDispatchActions([ - eventUsageLogic.actionCreators.reportInsightViewed( - insight, - { insight: InsightType.FUNNELS }, - ItemMode.View, - true, - false, - 0, - { - changed_insight: InsightType.TRENDS, - }, - false - ), - ]) + }).toDispatchActions(['reportInsightViewed']) }) }) diff --git a/frontend/src/scenes/insights/insightLogic.ts b/frontend/src/scenes/insights/insightLogic.ts index 3490a472284c5..9be0d753f66b0 100644 --- a/frontend/src/scenes/insights/insightLogic.ts +++ b/frontend/src/scenes/insights/insightLogic.ts @@ -59,6 +59,7 @@ import { toLocalFilters } from './filters/ActionFilter/entityFilterLogic' import { loaders } from 'kea-loaders' import { legacyInsightQuery } from '~/queries/query' import { tagsModel } from '~/models/tagsModel' +import { subscriptions } from 'kea-subscriptions' const IS_TEST_MODE = process.env.NODE_ENV === 'test' const SHOW_TIMEOUT_MESSAGE_AFTER = 15000 @@ -104,6 +105,8 @@ export const insightLogic = kea([ ['cohortsById'], mathsLogic, ['mathDefinitions'], + sceneLogic, + ['scene'], ], actions: [tagsModel, ['loadTags']], logic: [eventUsageLogic, dashboardsModel, prompt({ key: `save-as-insight` })], @@ -346,6 +349,7 @@ export const insightLogic = kea([ } response = await getJSONOrThrow(fetchResponse) } catch (e: any) { + let cancelled = false if (e.name === 'AbortError' || e.message?.name === 'AbortError') { actions.abortQuery({ queryId, @@ -353,6 +357,7 @@ export const insightLogic = kea([ scene: scene, exception: e, }) + cancelled = true } breakpoint() cache.abortController = null @@ -376,7 +381,10 @@ export const insightLogic = kea([ e.message ) } - throw e + + if (!cancelled) { + throw e + } } breakpoint() @@ -1042,6 +1050,19 @@ export const insightLogic = kea([ } }, })), + subscriptions(({ cache }) => ({ + scene: (value: Scene | null | undefined, lastValue: Scene | null | undefined) => { + const scenesThatMightAbortClickHouseQueries = [Scene.Insight, Scene.Dashboard, Scene.ProjectHomepage] + if ( + value && + lastValue && + scenesThatMightAbortClickHouseQueries.includes(lastValue) && + !scenesThatMightAbortClickHouseQueries.includes(value) + ) { + cache.abortController?.abort() + } + }, + })), events(({ props, cache, values, actions }) => ({ afterMount: () => { if (!props.cachedInsight || !props.cachedInsight?.result || !!props.cachedInsight?.filters) { diff --git a/posthog/api/insight.py b/posthog/api/insight.py index 2c54ed2756e81..704373f5dd23e 100644 --- a/posthog/api/insight.py +++ b/posthog/api/insight.py @@ -787,10 +787,12 @@ def activity(self, request: request.Request, **kwargs): def cancel(self, request: request.Request, **kwargs): if "client_query_id" not in request.data: raise serializers.ValidationError({"client_query_id": "Field is required."}) + sync_execute( f"KILL QUERY ON CLUSTER '{CLICKHOUSE_CLUSTER}' WHERE query_id LIKE %(client_query_id)s", {"client_query_id": f"{self.team.pk}_{request.data['client_query_id']}%"}, ) + statsd.incr("clickhouse.insight.query.cancellation_requested", tags={"team_id": self.team.pk}) return Response(status=status.HTTP_201_CREATED) @action(methods=["POST"], detail=False)