Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions frontend/src/queries/nodes/InsightViz/InsightVizDisplay.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ function DashboardInsightRefreshHintOrLoading({
/>
)
}
return <InsightRefreshDataHint onRetry={onRetry} />
return <InsightRefreshDataHint onRetry={onRetry} insightProps={insightProps} />
}

/** Dashboard tile: show refresh when merged `result` is still nullish (empty success is `[]`, not `null`). */
Expand Down Expand Up @@ -304,7 +304,7 @@ export function InsightVizDisplay({
/>
)
}
return <InsightRefreshDataHint onRetry={onRetry} />
return <InsightRefreshDataHint onRetry={onRetry} insightProps={insightProps} />
}

if (activeView === InsightType.FUNNELS && !isFlowViz) {
Expand Down
24 changes: 22 additions & 2 deletions frontend/src/scenes/insights/EmptyStates/EmptyStates.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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 (
<div
data-attr="insight-refresh-data-hint"
Expand Down
4 changes: 2 additions & 2 deletions frontend/src/scenes/insights/InsightSceneHeader.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ export function InsightSceneHeader({ insightLogicProps }: InsightSceneHeaderProp
<LemonBanner type="warning" className="mb-4">
<div className="flex flex-row items-center justify-between gap-2">
<span>
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.
</span>

<LemonButton type="secondary" to={urls.insightView(insightId as InsightShortId)}>
Expand Down
64 changes: 64 additions & 0 deletions frontend/src/scenes/insights/insightLogic.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.'],
Expand Down
31 changes: 25 additions & 6 deletions frontend/src/scenes/insights/insightLogic.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -517,6 +517,18 @@ export type insightLogicType = MakeLogicType<
insightLogicMeta
>

export function insightOverridesPresent(
filtersOverride?: DashboardFilter | null,
variablesOverride?: Record<string, HogQLVariable> | null,
tileFiltersOverride?: TileFilters | null
): boolean {
return (
!isDashboardFilterEmpty(filtersOverride) ||
(isObject(variablesOverride) && !isEmptyObject(variablesOverride)) ||
!isDashboardFilterEmpty(tileFiltersOverride)
)
}

export const insightLogic: LogicWrapper<insightLogicType> = kea<insightLogicType>([
props({ filtersOverride: null, variablesOverride: null, tileFiltersOverride: null } as InsightLogicProps),
key((props) => keyForInsightLogicProps('new')(props)),
Expand Down Expand Up @@ -615,10 +627,21 @@ export const insightLogic: LogicWrapper<insightLogicType> = kea<insightLogicType
) => {
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
Expand Down Expand Up @@ -1003,11 +1026,7 @@ export const insightLogic: LogicWrapper<insightLogicType> = kea<insightLogicType
variablesOverride: Record<string, HogQLVariable> | null,
tileFiltersOverride: TileFilters | null
) => {
return (
!isDashboardFilterEmpty(filtersOverride) ||
(isObject(variablesOverride) && !isEmptyObject(variablesOverride)) ||
!isDashboardFilterEmpty(tileFiltersOverride)
)
return insightOverridesPresent(filtersOverride, variablesOverride, tileFiltersOverride)
},
],
editingDisabledReason: [
Expand Down
4 changes: 2 additions & 2 deletions playwright/e2e/product-analytics/insights/funnels.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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' })
Expand All @@ -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()
})

Expand Down
Loading