diff --git a/frontend/src/queries/nodes/InsightViz/InsightVizDisplay.tsx b/frontend/src/queries/nodes/InsightViz/InsightVizDisplay.tsx index ad2645c51a8c..ca512a714456 100644 --- a/frontend/src/queries/nodes/InsightViz/InsightVizDisplay.tsx +++ b/frontend/src/queries/nodes/InsightViz/InsightVizDisplay.tsx @@ -96,7 +96,24 @@ function DashboardInsightRefreshHintOrLoading({ /> ) } - return + return +} + +function refreshHintContext(insightProps: InsightLogicProps): { + dashboardId: number | null + insightShortId: string | null + hasOverrides: boolean +} { + const { dashboardItemId } = insightProps + return { + dashboardId: insightProps.dashboardId ?? null, + insightShortId: typeof dashboardItemId === 'string' ? dashboardItemId : null, + hasOverrides: !!( + insightProps.filtersOverride || + insightProps.variablesOverride || + insightProps.tileFiltersOverride + ), + } } /** Dashboard tile: show refresh when merged `result` is still nullish (empty success is `[]`, not `null`). */ @@ -304,7 +321,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..b83f8b723e9a 100644 --- a/frontend/src/scenes/insights/EmptyStates/EmptyStates.tsx +++ b/frontend/src/scenes/insights/EmptyStates/EmptyStates.tsx @@ -118,7 +118,26 @@ 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, + dashboardId, + insightShortId, + hasOverrides, +}: { + onRetry: () => void + dashboardId?: number | null + insightShortId?: string | null + hasOverrides?: boolean +}): JSX.Element { + // The hint used to be silent, so a blank tile was only ever visible in session replay. + useOnMountEffect(() => { + posthog.capture('insight refresh hint shown', { + dashboard_id: dashboardId ?? null, + insight_short_id: insightShortId ?? null, + has_overrides: !!hasOverrides, + }) + }) + 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 you can't edit it. + Discard them to go back to the saved insight. diff --git a/posthog/hogql_queries/refresh_policy.py b/posthog/hogql_queries/refresh_policy.py index f9100223bb18..7df80d513b14 100644 --- a/posthog/hogql_queries/refresh_policy.py +++ b/posthog/hogql_queries/refresh_policy.py @@ -59,6 +59,21 @@ class ComputeSurface(StrEnum): } +# Client-supplied overrides (dashboard filters, variables, tile filters) are merged into the query +# before it is fingerprinted, so the overridden variant has a cache key of its own that no scheduled +# refresh ever warms. Reading it cache-only returns `result: null`, which the UI renders as a +# dead-end "Chart data didn't load" tile. +OVERRIDE_QUERY_PARAMS = ("filters_override", "variables_override", "tile_filters_override") + +# What an override-carrying request falls back to instead of cache-only: serve the cache when it is +# warm, refresh in the background when stale, and compute synchronously only on a genuine miss. +OVERRIDE_MISS_EXECUTION_MODE = ExecutionMode.RECENT_CACHE_CALCULATE_ASYNC_IF_STALE_AND_BLOCKING_ON_MISS + + +def _client_overrides_present(request: "Request") -> bool: + return any(request.query_params.get(param) for param in OVERRIDE_QUERY_PARAMS) + + def _refresh_param_present(request: "Request") -> bool: """Whether the client sent a `refresh` param at all (query string or body), regardless of its value — so an explicit `refresh=false` is distinguished from an absent param.""" @@ -87,6 +102,14 @@ def resolve_execution_mode( execution_mode = execution_mode_from_refresh(refresh_requested_by_client(request)) else: execution_mode = SURFACE_DEFAULT_EXECUTION_MODE[surface] + # Overrides are ignored on shared/embedded resources, so their cache key is unaffected there + # and anonymous demand must never force a recompute. + if ( + not is_shared + and execution_mode == ExecutionMode.CACHE_ONLY_NEVER_CALCULATE + and _client_overrides_present(request) + ): + execution_mode = OVERRIDE_MISS_EXECUTION_MODE if is_shared: return shared_insights_execution_mode(execution_mode) diff --git a/posthog/hogql_queries/test/test_refresh_policy.py b/posthog/hogql_queries/test/test_refresh_policy.py index 1fc8e463e417..3a4ad66a114f 100644 --- a/posthog/hogql_queries/test/test_refresh_policy.py +++ b/posthog/hogql_queries/test/test_refresh_policy.py @@ -12,8 +12,11 @@ from posthog.hogql_queries.refresh_policy import ComputeSurface, resolve_execution_mode -def _request(refresh: str | None = None) -> Request: - drf_request = cast(Request, Request(HttpRequest())) +def _request(refresh: str | None = None, **query_params: str) -> Request: + http_request = HttpRequest() + for key, value in query_params.items(): + http_request.GET[key] = value + drf_request = cast(Request, Request(http_request)) drf_request._full_data = {"refresh": refresh} if refresh is not None else {} # type: ignore[attr-defined] return drf_request @@ -59,6 +62,29 @@ def test_explicit_refresh_false_stays_cache_only_when_surface_default_is_flipped false_mode, _ = resolve_execution_mode(_request("false"), surface=ComputeSurface.DASHBOARD_TILE) assert false_mode == ExecutionMode.CACHE_ONLY_NEVER_CALCULATE + @parameterized.expand([("filters_override",), ("variables_override",), ("tile_filters_override",)]) + def test_client_overrides_escalate_past_cache_only(self, param: str) -> None: + # Overrides change the cache key, so nothing warms it — cache-only would hand the UI + # result: null and a "Chart data didn't load" tile. + for surface in ComputeSurface: + mode, _ = resolve_execution_mode(_request(**{param: '{"date_from":"-14d"}'}), surface=surface) + assert mode == ExecutionMode.RECENT_CACHE_CALCULATE_ASYNC_IF_STALE_AND_BLOCKING_ON_MISS, surface + + def test_overrides_do_not_escalate_an_explicit_opt_out(self) -> None: + mode, _ = resolve_execution_mode( + _request("force_cache", filters_override='{"date_from":"-14d"}'), surface=ComputeSurface.DASHBOARD_TILE + ) + assert mode == ExecutionMode.CACHE_ONLY_NEVER_CALCULATE + + def test_overrides_do_not_change_the_shared_path(self) -> None: + # Shared/embedded rendering ignores client overrides entirely, so its cache key — and its + # execution mode — must be identical with and without them. + without = resolve_execution_mode(_request(), surface=ComputeSurface.SHARED, is_shared=True) + with_override = resolve_execution_mode( + _request(filters_override='{"date_from":"-14d"}'), surface=ComputeSurface.SHARED, is_shared=True + ) + assert with_override == without + def test_shared_flag_applies_the_clamp(self) -> None: # resolve_execution_mode's own responsibility is to route through the shared clamp iff # is_shared (the exhaustive clamp mapping is covered in test_query_runner). Contrast the