diff --git a/frontend/src/queries/nodes/InsightViz/InsightVizDisplay.tsx b/frontend/src/queries/nodes/InsightViz/InsightVizDisplay.tsx index ad2645c51a8c..0b079f2472f5 100644 --- a/frontend/src/queries/nodes/InsightViz/InsightVizDisplay.tsx +++ b/frontend/src/queries/nodes/InsightViz/InsightVizDisplay.tsx @@ -96,7 +96,7 @@ function DashboardInsightRefreshHintOrLoading({ /> ) } - return + return } /** Dashboard tile: show refresh when merged `result` is still nullish (empty success is `[]`, not `null`). */ @@ -304,7 +304,7 @@ export function InsightVizDisplay({ /> ) } - return + return } if (activeView === InsightType.FUNNELS && !isFlowViz) { diff --git a/frontend/src/scenes/insights/EmptyStates/EmptyStates.tsx b/frontend/src/scenes/insights/EmptyStates/EmptyStates.tsx index 24d90b3d0698..80e14064588d 100644 --- a/frontend/src/scenes/insights/EmptyStates/EmptyStates.tsx +++ b/frontend/src/scenes/insights/EmptyStates/EmptyStates.tsx @@ -27,7 +27,7 @@ import { humanFriendlyNumber, humanizeBytes } from 'lib/utils/numbers' import { isTrustedPostHogUrl } from 'lib/utils/trustedUrl' import { funnelDataLogic } from 'scenes/funnels/funnelDataLogic' import { entityFilterLogic } from 'scenes/insights/filters/ActionFilter/entityFilterLogic' -import { insightLogic } from 'scenes/insights/insightLogic' +import { insightLogic, insightOverridesPresent } from 'scenes/insights/insightLogic' import { autoRunMaxPrompt } from 'scenes/max/maxPrompt' import { preflightLogic } from 'scenes/PreflightCheck/preflightLogic' import { SavedInsightFilters } from 'scenes/saved-insights/savedInsightsLogic' @@ -118,7 +118,27 @@ export function InsightEmptyState({ } /** Shown when the chart area would otherwise be blank (e.g. cache miss + aborted refresh). */ -export function InsightRefreshDataHint({ onRetry }: { onRetry: () => void }): JSX.Element { +export function InsightRefreshDataHint({ + onRetry, + insightProps, +}: { + onRetry: () => void + insightProps?: InsightLogicProps +}): JSX.Element { + // This dead-end state used to be invisible outside session replay — capture it so blank + // tiles are measurable and can be sliced by dashboard context and override presence. + useOnMountEffect(() => { + posthog.capture('insight refresh hint shown', { + dashboard_id: insightProps?.dashboardId ?? null, + insight_short_id: typeof insightProps?.dashboardItemId === 'string' ? insightProps.dashboardItemId : null, + has_overrides: insightOverridesPresent( + insightProps?.filtersOverride, + insightProps?.variablesOverride, + insightProps?.tileFiltersOverride + ), + }) + }) + return (
- You are viewing this insight with filter/variable overrides. Discard them to edit the - insight. + You're viewing this insight with a dashboard's filters applied, so it can't be edited. + Discard the filters to edit the saved insight. diff --git a/frontend/src/scenes/insights/insightLogic.test.ts b/frontend/src/scenes/insights/insightLogic.test.ts index e44690b3fb74..c3398adda270 100644 --- a/frontend/src/scenes/insights/insightLogic.test.ts +++ b/frontend/src/scenes/insights/insightLogic.test.ts @@ -1069,6 +1069,70 @@ describe('insightLogic', () => { }) }) + describe('loadInsight refresh mode', () => { + // Overridden queries get their own cache key, which nothing warms — a cache-only-ish + // `async` read yields `result: null` and the scene shows "Chart data didn't load". + const seenRefreshParams: (string | null)[] = [] + + beforeEach(() => { + seenRefreshParams.length = 0 + useMocks({ + get: { + '/api/environments/:team_id/insights/': ({ request }) => { + const url = new URL(request.url) + seenRefreshParams.push(url.searchParams.get('refresh')) + return [ + 200, + { + results: [ + { + id: 42, + short_id: Insight42, + result: ['result from api'], + filters: API_FILTERS, + name: 'original name', + }, + ], + }, + ] + }, + }, + }) + }) + + it('uses async without overrides', async () => { + logic = insightLogic({ dashboardItemId: Insight42 }) + logic.mount() + await expectLogic(logic).toDispatchActions(['loadInsightSuccess']) + + expect(seenRefreshParams).toEqual(['async']) + }) + + it('blocks on a cache miss when overrides are present', async () => { + logic = insightLogic({ + dashboardItemId: Insight42, + dashboardId: 33, + filtersOverride: { date_from: '-14d' }, + }) + logic.mount() + await expectLogic(logic).toDispatchActions(['loadInsightSuccess']) + + expect(seenRefreshParams).toEqual(['async_except_on_cache_miss']) + }) + + it('treats empty overrides as no overrides', async () => { + logic = insightLogic({ + dashboardItemId: Insight42, + dashboardId: 33, + filtersOverride: {}, + }) + logic.mount() + await expectLogic(logic).toDispatchActions(['loadInsightSuccess']) + + expect(seenRefreshParams).toEqual(['async']) + }) + }) + describe('editingDisabledReason', () => { it.each([ ['overrides present', { filtersOverride: { date_from: '-7d' } }, 'Discard overrides to edit the insight.'], diff --git a/frontend/src/scenes/insights/insightLogic.tsx b/frontend/src/scenes/insights/insightLogic.tsx index 3abb13bc9f5c..adc6b6f6c572 100644 --- a/frontend/src/scenes/insights/insightLogic.tsx +++ b/frontend/src/scenes/insights/insightLogic.tsx @@ -517,6 +517,18 @@ export type insightLogicType = MakeLogicType< insightLogicMeta > +export function insightOverridesPresent( + filtersOverride?: DashboardFilter | null, + variablesOverride?: Record | null, + tileFiltersOverride?: TileFilters | null +): boolean { + return ( + !isDashboardFilterEmpty(filtersOverride) || + (isObject(variablesOverride) && !isEmptyObject(variablesOverride)) || + !isDashboardFilterEmpty(tileFiltersOverride) + ) +} + export const insightLogic: LogicWrapper = kea([ props({ filtersOverride: null, variablesOverride: null, tileFiltersOverride: null } as InsightLogicProps), key((props) => keyForInsightLogicProps('new')(props)), @@ -615,10 +627,21 @@ export const insightLogic: LogicWrapper = kea { await breakpoint(100) try { + // Overrides are merged into the query before caching, giving the overridden + // variant a cache key of its own that no scheduled refresh warms. A plain + // `async` read returns `result: null` on that cold key, and in dashboard + // context the data node won't load on its own — the scene dead-ends on + // "Chart data didn't load". Block on a genuine miss instead; warm and stale + // keys behave exactly as before. + const hasOverrides = insightOverridesPresent( + filtersOverride, + variablesOverride, + tileFiltersOverride + ) const insight = await insightsApi.getByShortId( shortId, undefined, - 'async', + hasOverrides ? 'async_except_on_cache_miss' : 'async', filtersOverride, variablesOverride, tileFiltersOverride @@ -1003,11 +1026,7 @@ export const insightLogic: LogicWrapper = kea | null, tileFiltersOverride: TileFilters | null ) => { - return ( - !isDashboardFilterEmpty(filtersOverride) || - (isObject(variablesOverride) && !isEmptyObject(variablesOverride)) || - !isDashboardFilterEmpty(tileFiltersOverride) - ) + return insightOverridesPresent(filtersOverride, variablesOverride, tileFiltersOverride) }, ], editingDisabledReason: [ diff --git a/playwright/e2e/product-analytics/insights/funnels.spec.ts b/playwright/e2e/product-analytics/insights/funnels.spec.ts index 62b3654bed18..0524449e73d8 100644 --- a/playwright/e2e/product-analytics/insights/funnels.spec.ts +++ b/playwright/e2e/product-analytics/insights/funnels.spec.ts @@ -434,7 +434,7 @@ test.describe('Funnel insights', () => { await insight.goToInsight(seededInsightId(), { queryParams: { filters_override: { date_from: '-14d' }, dashboard: dashboardId }, }) - await expect(page.getByText('filter/variable overrides')).toBeVisible({ timeout: 20000 }) + await expect(page.getByText("a dashboard's filters applied")).toBeVisible({ timeout: 20000 }) await expect( page .getByRole('button', { name: 'Discard overrides' }) @@ -447,7 +447,7 @@ test.describe('Funnel insights', () => { .getByRole('button', { name: 'Discard overrides' }) .or(page.getByRole('link', { name: 'Discard overrides' })) .click() - await expect(page.getByText('filter/variable overrides')).not.toBeVisible() + await expect(page.getByText("a dashboard's filters applied")).not.toBeVisible() await expect(insight.editButton).toBeVisible() })