Discharge7443/fleet
1
0
Fork 0
Code Issues Pull Requests Actions 21 Packages Projects Releases Wiki Activity

Fix policy and report pages showing previously viewed content (#47767)

Browse Source 
**Related issue:** Resolves #43310

Read-only policy and report detail pages rendered their displayed fields
(name, description, resolution, platforms, query) from the editing
context (`PolicyContext`/`QueryContext`), which only updates in
react-query's `onSuccess`. On a cached revisit, `isLoading` is `false`
(no spinner) and the freshly-loaded entity is available immediately, but
the context still held the previously-viewed entity's values for a frame
— briefly showing the wrong policy/report.

This change makes both detail pages render directly from the fresh
`useQuery` result (`storedPolicy`/`storedQuery`) and drops their
coupling to the editing context, matching the existing
`HostDetailsPage`/`SoftwareTitleDetailsPage` pattern. `QueryDetailsPage`
(Reports) had the identical latent bug and is fixed here too.

  # Checklist for submitter

  - [x] Changes file added for user-visible changes in `changes/`.

  ## Testing

  - [x] Added/updated automated tests
  - [x] QA'd all new/changed functionality manually

New regression tests (`PolicyDetailsPage.tests.tsx`,
`QueryDetailsPage.tests.tsx`) seed the context with stale values and
assert the page renders the freshly-loaded entity instead. Verified live
against a running Fleet instance with Playwright: navigating between
cached detail pages no longer flashes the previous entity's content.


[6f7d6d22134dc1fd968f8473c552ee49.webm](https://github.com/user-attachments/assets/c8ef5f3f-278c-4e13-adc1-689b3eff4e59)


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

## Summary by CodeRabbit

## Release Notes

* **Bug Fixes**
* Resolved an issue where policy and query details pages could briefly
display previously viewed policy/report information when switching
between items.

* **Tests**
* Added regression coverage to ensure the UI renders freshly loaded
policy/query name, description, and query/report details, and does not
show stale values.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
Carlo
2026-06-19 14:37:06 -04:00
committed by GitHub
parent bbdd784b73
commit a024d8e26a
6 changed files with 298 additions and 102 deletions
Download Patch File Download Diff File Expand all files Collapse all files
changes/43310-policy-report-details-flash
+1
View File
@@ -0,0 +1 @@
- Fixed the policy and report details pages briefly showing the previously-viewed policy/report's content when navigating between them.
frontend/pages/policies/details/PolicyDetailsPage/PolicyDetailsPage.tests.tsx
+137 -1
View File
@@ -1,13 +1,51 @@
import React from "react";
import { screen } from "@testing-library/react";
import { http, HttpResponse } from "msw";
import { IPolicy } from "interfaces/policy";
import { ILabelPolicy } from "interfaces/label";
import {
createCustomRenderer,
baseUrl,
createMockRouter,
} from "test/test-utils";
import mockServer from "test/mock-server";
import createMockUser from "__mocks__/userMock";
import createMockConfig from "__mocks__/configMock";
import { getLabelModalData } from "./PolicyDetailsPage";
import PolicyDetailsPage, { getLabelModalData } from "./PolicyDetailsPage";
// Stub SoftwareIcon to avoid asset resolution when importing the page module.
jest.mock("pages/SoftwarePage/components/icons/SoftwareIcon", () => {
return () => null;
});
// Avoid depending on react-router's browserHistory inside BackButton.
jest.mock("components/BackButton", () => ({
__esModule: true,
default: ({ text }: { text: string }) => (
<button type="button" data-testid="back-button">
{text}
</button>
),
}));
// Surface the modal's `query` prop as plain text (the real modal renders it in
// an Ace editor that isn't reliably assertable in jsdom).
jest.mock("components/modals/ShowQueryModal", () => ({
__esModule: true,
default: ({ query }: { query?: string }) => (
<div data-testid="show-query-modal">{query}</div>
),
}));
// Activities table fetches on mount; stub it out so the render test stays
// focused on the policy's own fields.
jest.mock("../components/PolicyAutomationsActivitiesTable", () => ({
__esModule: true,
default: () => null,
}));
const labels = (...names: string[]): ILabelPolicy[] =>
names.map((name, i) => ({ id: i + 1, name }));
@@ -169,3 +207,101 @@ describe("getLabelModalData", () => {
});
});
});
const POLICY_ID = 8;
const createProps = () => ({
router: createMockRouter(),
params: { id: String(POLICY_ID) },
location: {
pathname: `/policies/${POLICY_ID}`,
search: "",
query: {},
},
});
const baseAppContext = {
isGlobalAdmin: true,
isOnGlobalTeam: true,
// Free tier short-circuits useTeamIdParam's redirect logic when no fleet_id is
// set, keeping the test focused on which data source the page renders from.
isFreeTier: true,
isPremiumTier: false,
currentUser: createMockUser({ global_role: "admin" }),
config: createMockConfig(),
availableTeams: [],
};
describe("PolicyDetailsPage - renders fresh policy data (regression #43310)", () => {
it("renders the loaded policy's fields, not stale PolicyContext values", async () => {
mockServer.use(
// team_id: null keeps the team query disabled, so no second endpoint to mock.
http.get(baseUrl(`/policies/${POLICY_ID}`), () =>
HttpResponse.json({
policy: createMockPolicy({
id: POLICY_ID,
team_id: null,
name: "Fresh policy name",
description: "Fresh policy description",
resolution: "Fresh resolution steps",
platform: "darwin",
query: "SELECT 'fresh';",
critical: true,
}),
})
)
);
const render = createCustomRenderer({
withBackendMock: true,
context: {
app: baseAppContext,
// Stale values left over from a previously-viewed policy. The page must
// ignore all of these and render the freshly-loaded policy instead.
policy: {
lastEditedQueryName: "Stale policy name",
lastEditedQueryDescription: "Stale policy description",
lastEditedQueryResolution: "Stale resolution steps",
lastEditedQueryPlatform: "windows",
lastEditedQueryBody: "SELECT 'stale';",
lastEditedQueryCritical: false,
},
},
});
const { user, container } = render(
<PolicyDetailsPage {...(createProps() as any)} />
);
// name + description
expect(await screen.findByText("Fresh policy name")).toBeInTheDocument();
expect(screen.getByText("Fresh policy description")).toBeInTheDocument();
expect(screen.queryByText("Stale policy name")).not.toBeInTheDocument();
expect(
screen.queryByText("Stale policy description")
).not.toBeInTheDocument();
// resolution
expect(screen.getByText("Fresh resolution steps")).toBeInTheDocument();
expect(
screen.queryByText("Stale resolution steps")
).not.toBeInTheDocument();
// platform ("darwin" displays as "macOS"; stale "windows" must not appear)
expect(screen.getByText("macOS")).toBeInTheDocument();
expect(screen.queryByText("Windows")).not.toBeInTheDocument();
// critical (drives the critical-policy icon)
expect(
container.querySelector(".critical-policy-icon")
).toBeInTheDocument();
// query (shown via the "Show query" modal)
await user.click(screen.getByRole("button", { name: "Show query" }));
expect(screen.getByTestId("show-query-modal")).toHaveTextContent(
"SELECT 'fresh';"
);
expect(screen.getByTestId("show-query-modal")).not.toHaveTextContent(
"SELECT 'stale';"
);
});
});
frontend/pages/policies/details/PolicyDetailsPage/PolicyDetailsPage.tsx
+7 -53
View File
@@ -4,7 +4,6 @@ import { InjectedRouter, Params } from "react-router/lib/Router";
import { useErrorHandler } from "react-error-boundary";
import PATHS from "router/paths";
import { AppContext } from "context/app";
import { PolicyContext } from "context/policy";
import {
IPolicy,
IStoredPolicyResponse,
@@ -12,9 +11,7 @@ import {
} from "interfaces/policy";
import { ILabelPolicy } from "interfaces/label";
import {
API_ALL_TEAMS_ID,
API_NO_TEAM_ID,
APP_CONTEXT_ALL_TEAMS_ID,
APP_CONTEXT_ALL_TEAMS_SUMMARY,
APP_CONTEXT_NO_TEAM_SUMMARY,
} from "interfaces/team";
@@ -113,25 +110,6 @@ const PolicyDetailsPage = ({
config,
} = useContext(AppContext);
const {
lastEditedQueryName,
lastEditedQueryDescription,
lastEditedQueryResolution,
lastEditedQueryBody,
lastEditedQueryPlatform,
setLastEditedQueryId,
setLastEditedQueryName,
setLastEditedQueryDescription,
setLastEditedQueryBody,
setLastEditedQueryResolution,
setLastEditedQueryCritical,
setLastEditedQueryPlatform,
setLastEditedQueryLabelsIncludeAny,
setLastEditedQueryLabelsIncludeAll,
setLastEditedQueryLabelsExcludeAny,
setPolicyTeamId,
} = useContext(PolicyContext);
const {
isRouteOk,
teamIdForApi,
@@ -169,30 +147,6 @@ const PolicyDetailsPage = ({
refetchOnWindowFocus: false,
retry: false,
select: (data: IStoredPolicyResponse) => data.policy,
onSuccess: (returnedPolicy) => {
setLastEditedQueryId(returnedPolicy.id);
setLastEditedQueryName(returnedPolicy.name);
setLastEditedQueryDescription(returnedPolicy.description);
setLastEditedQueryBody(returnedPolicy.query);
setLastEditedQueryResolution(returnedPolicy.resolution);
setLastEditedQueryCritical(returnedPolicy.critical);
setLastEditedQueryPlatform(returnedPolicy.platform);
setLastEditedQueryLabelsIncludeAny(
returnedPolicy.labels_include_any || []
);
setLastEditedQueryLabelsIncludeAll(
returnedPolicy.labels_include_all || []
);
setLastEditedQueryLabelsExcludeAny(
returnedPolicy.labels_exclude_any || []
);
const deNulledTeamId = returnedPolicy.team_id ?? undefined;
setPolicyTeamId(
deNulledTeamId === API_ALL_TEAMS_ID
? APP_CONTEXT_ALL_TEAMS_ID
: deNulledTeamId
);
},
onError: (error) => handlePageError(error),
});
@@ -286,8 +240,8 @@ const PolicyDetailsPage = ({
};
const renderPlatforms = (): JSX.Element | null => {
if (!lastEditedQueryPlatform) return null;
const platforms = lastEditedQueryPlatform
if (!storedPolicy?.platform) return null;
const platforms = storedPolicy.platform
.split(",")
.map((p) => p.trim())
.filter((p): p is Platform => p in PLATFORM_DISPLAY_NAMES);
@@ -348,12 +302,12 @@ const PolicyDetailsPage = ({
};
const renderResolution = () => {
if (!lastEditedQueryResolution) return null;
if (!storedPolicy?.resolution) return null;
return (
<DataSet
className={`${baseClass}__resolve`}
title="Resolve"
value={lastEditedQueryResolution}
value={storedPolicy.resolution}
multiline
/>
);
@@ -413,7 +367,7 @@ const PolicyDetailsPage = ({
<div className={`${baseClass}__title-bar`}>
<div className={`${baseClass}__name-description`}>
<h1 className={`${baseClass}__policy-name`}>
{lastEditedQueryName}
{storedPolicy?.name}
{storedPolicy?.critical && (
<TooltipWrapper
tipContent="This policy has been marked as critical."
@@ -430,7 +384,7 @@ const PolicyDetailsPage = ({
</h1>
<PageDescription
className={`${baseClass}__policy-description`}
content={lastEditedQueryDescription}
content={storedPolicy?.description}
/>
</div>
<div className={`${baseClass}__action-button-container`}>
@@ -508,7 +462,7 @@ const PolicyDetailsPage = ({
)}
{showQueryModal && (
<ShowQueryModal
query={lastEditedQueryBody}
query={storedPolicy?.query}
onCancel={() => setShowQueryModal(false)}
/>
)}
frontend/pages/queries/details/QueryDetailsPage/QueryDetailsPage.tests.tsx
+136
View File
@@ -26,6 +26,34 @@ jest.mock("components/BackButton", () => ({
),
}));
// Surface the modal's `query` prop as plain text (the real modal renders it in
// an Ace editor that isn't reliably assertable in jsdom).
jest.mock("components/modals/ShowQueryModal", () => ({
__esModule: true,
default: ({ query }: { query?: string }) => (
<div data-testid="show-query-modal">{query}</div>
),
}));
// Surface the report-setting props derived from the loaded query so we can
// assert they reflect storedQuery rather than stale context.
jest.mock("../components/NoResults/NoResults", () => ({
__esModule: true,
default: ({
discardDataEnabled,
loggingSnapshot,
}: {
discardDataEnabled: boolean;
loggingSnapshot: boolean;
}) => (
<div
data-testid="no-results"
data-discard={String(discardDataEnabled)}
data-snapshot={String(loggingSnapshot)}
/>
),
}));
const QUERY_ID = 1;
const HOST_ID = 42;
const FILTERED_QUERIES_PATH = "/queries/manage?fleet_id=1";
@@ -79,6 +107,114 @@ const renderPage = (
return screen.findByTestId("back-button");
};
describe("QueryDetailsPage - renders fresh query data (regression #43310)", () => {
it("renders the loaded query's fields, not stale QueryContext values", async () => {
mockServer.use(
http.get(baseUrl(`/reports/${QUERY_ID}`), () =>
HttpResponse.json({
query: createMockSchedulableQuery({
id: QUERY_ID,
team_id: null,
name: "Fresh report name",
description: "Fresh report description",
query: "SELECT 'fresh';",
logging: "differential", // not "snapshot"
discard_data: true,
}),
})
),
http.get(baseUrl(`/reports/${QUERY_ID}/report`), () =>
HttpResponse.json(
createMockQueryReport({ query_id: QUERY_ID, results: [] })
)
)
);
const render = createCustomRenderer({
withBackendMock: true,
context: {
app: baseAppContext,
// Stale values left over from a previously-viewed report. The page must
// ignore all of these and render the freshly-loaded query instead.
query: {
lastEditedQueryName: "Stale report name",
lastEditedQueryDescription: "Stale report description",
lastEditedQueryBody: "SELECT 'stale';",
lastEditedQueryLoggingType: "snapshot",
lastEditedQueryDiscardData: false,
},
},
});
const { user } = render(<QueryDetailsPage {...(createProps() as any)} />);
// name + description
expect(await screen.findByText("Fresh report name")).toBeInTheDocument();
expect(screen.getByText("Fresh report description")).toBeInTheDocument();
expect(screen.queryByText("Stale report name")).not.toBeInTheDocument();
expect(
screen.queryByText("Stale report description")
).not.toBeInTheDocument();
// logging + discard_data (drive the report's caching state)
const noResults = screen.getByTestId("no-results");
expect(noResults).toHaveAttribute("data-discard", "true");
expect(noResults).toHaveAttribute("data-snapshot", "false");
// query (shown via the "Show query" modal)
await user.click(screen.getByRole("button", { name: "Show query" }));
expect(screen.getByTestId("show-query-modal")).toHaveTextContent(
"SELECT 'fresh';"
);
expect(screen.getByTestId("show-query-modal")).not.toHaveTextContent(
"SELECT 'stale';"
);
});
it("derives Live report visibility from the loaded query's observer_can_run, not stale context", async () => {
mockServer.use(
http.get(baseUrl(`/reports/${QUERY_ID}`), () =>
HttpResponse.json({
query: createMockSchedulableQuery({
id: QUERY_ID,
team_id: null,
observer_can_run: true,
}),
})
),
http.get(baseUrl(`/reports/${QUERY_ID}/report`), () =>
HttpResponse.json(
createMockQueryReport({ query_id: QUERY_ID, results: [] })
)
)
);
const render = createCustomRenderer({
withBackendMock: true,
context: {
// A plain observer: the only thing that can grant live-query access here
// is the query's own observer_can_run, so the button proves the source.
app: {
...baseAppContext,
isGlobalAdmin: false,
isGlobalMaintainer: false,
isObserverPlus: false,
isGlobalTechnician: false,
isTeamMaintainerOrTeamAdmin: false,
isTeamTechnician: false,
isOnGlobalTeam: false,
currentUser: createMockUser({ global_role: "observer" }),
},
query: { lastEditedQueryObserverCanRun: false },
},
});
render(<QueryDetailsPage {...(createProps() as any)} />);
expect(
await screen.findByRole("button", { name: /Live report/i })
).toBeInTheDocument();
});
});
describe("QueryDetailsPage - back navigation", () => {
beforeEach(() => setupQueryHandlers());
frontend/pages/queries/details/QueryDetailsPage/QueryDetailsPage.tsx
+13 -43
View File
@@ -5,7 +5,6 @@ import { useErrorHandler } from "react-error-boundary";
import PATHS from "router/paths";
import { AppContext } from "context/app";
import { QueryContext } from "context/query";
import {
IGetQueryResponse,
@@ -110,25 +109,6 @@ const QueryDetailsPage = ({
isGlobalTechnician,
isTeamTechnician,
} = useContext(AppContext);
const {
lastEditedQueryName,
lastEditedQueryDescription,
lastEditedQueryBody,
lastEditedQueryObserverCanRun,
lastEditedQueryDiscardData,
lastEditedQueryLoggingType,
setLastEditedQueryId,
setLastEditedQueryName,
setLastEditedQueryDescription,
setLastEditedQueryBody,
setLastEditedQueryObserverCanRun,
setLastEditedQueryFrequency,
setLastEditedQueryLoggingType,
setLastEditedQueryMinOsqueryVersion,
setLastEditedQueryPlatforms,
setLastEditedQueryDiscardData,
} = useContext(QueryContext);
const [showQueryModal, setShowQueryModal] = useState(false);
const [disabledCachingGlobally, setDisabledCachingGlobally] = useState(true);
@@ -138,8 +118,6 @@ const QueryDetailsPage = ({
}
}, [config]);
// disabled on page load so we can control the number of renders
// else it will re-populate the context on occasion
const {
isLoading: isStoredQueryLoading,
data: storedQuery,
@@ -151,18 +129,6 @@ const QueryDetailsPage = ({
enabled: !!queryId,
refetchOnWindowFocus: false,
select: (data) => data.query,
onSuccess: (returnedQuery) => {
setLastEditedQueryId(returnedQuery.id);
setLastEditedQueryName(returnedQuery.name);
setLastEditedQueryDescription(returnedQuery.description);
setLastEditedQueryBody(returnedQuery.query);
setLastEditedQueryObserverCanRun(returnedQuery.observer_can_run);
setLastEditedQueryFrequency(returnedQuery.interval);
setLastEditedQueryPlatforms(returnedQuery.platform);
setLastEditedQueryLoggingType(returnedQuery.logging);
setLastEditedQueryMinOsqueryVersion(returnedQuery.min_osquery_version);
setLastEditedQueryDiscardData(returnedQuery.discard_data);
},
onError: (error) => handlePageError(error),
}
);
@@ -192,7 +158,9 @@ const QueryDetailsPage = ({
data: queryReport,
error: queryReportError,
} = useQuery<IQueryReport, Error, IQueryReport>(
[],
// Key must include every queryFn parameter; an empty key bled one report's
// cached rows into another on revisit (and suppressed refetch on sort).
["queryReport", queryId, currentTeamId, serverSortBy],
() =>
queryReportAPI.load({
teamId: currentTeamId,
@@ -236,7 +204,7 @@ const QueryDetailsPage = ({
const isLiveQueryDisabled = config?.server_settings.live_query_disabled;
const canLiveQuery =
lastEditedQueryObserverCanRun ||
storedQuery?.observer_can_run ||
isObserverPlus ||
isGlobalAdmin ||
isGlobalMaintainer ||
@@ -280,7 +248,7 @@ const QueryDetailsPage = ({
<div className={`${baseClass}__title-bar`}>
<div className="name-description">
<h1 className={`${baseClass}__query-name`}>
{lastEditedQueryName}
{storedQuery?.name}
</h1>
</div>
<div className={`${baseClass}__action-button-container`}>
@@ -346,7 +314,7 @@ const QueryDetailsPage = ({
</div>
<PageDescription
className={`${baseClass}__query-description`}
content={lastEditedQueryDescription}
content={storedQuery?.description}
/>
<div className={`${baseClass}__settings`}>
<div className={`${baseClass}__automations`}>
@@ -406,9 +374,10 @@ const QueryDetailsPage = ({
);
const renderReport = () => {
const loggingSnapshot = lastEditedQueryLoggingType === "snapshot";
const discardData = !!storedQuery?.discard_data;
const loggingSnapshot = storedQuery?.logging === "snapshot";
const disabledCaching =
disabledCachingGlobally || lastEditedQueryDiscardData || !loggingSnapshot;
disabledCachingGlobally || discardData || !loggingSnapshot;
const emptyCache = (queryReport?.results?.length ?? 0) === 0;
if (isLoading) {
@@ -420,7 +389,7 @@ const QueryDetailsPage = ({
}
// Empty state with varying messages explaining why there's no results
if (emptyCache || lastEditedQueryDiscardData) {
if (emptyCache || discardData) {
return (
<NoResults
queryId={queryId}
@@ -428,7 +397,7 @@ const QueryDetailsPage = ({
queryUpdatedAt={storedQuery?.updated_at}
disabledCaching={disabledCaching}
disabledCachingGlobally={disabledCachingGlobally}
discardDataEnabled={lastEditedQueryDiscardData}
discardDataEnabled={discardData}
loggingSnapshot={loggingSnapshot}
canLiveQuery={canRunLiveReport}
canEditQuery={!!canEditQuery}
@@ -439,6 +408,7 @@ const QueryDetailsPage = ({
<QueryReport
queryReport={queryReport}
queryId={queryId}
queryName={storedQuery?.name}
isClipped={isClipped}
canLiveQuery={canRunLiveReport}
/>
@@ -453,7 +423,7 @@ const QueryDetailsPage = ({
{renderReport()}
{showQueryModal && (
<ShowQueryModal
query={lastEditedQueryBody}
query={storedQuery?.query}
onCancel={onShowQueryModal}
/>
)}
frontend/pages/queries/details/components/QueryReport/QueryReport.tsx
+4 -5
View File
@@ -1,8 +1,7 @@
import React, { useState, useContext, useMemo, useCallback } from "react";
import React, { useState, useMemo, useCallback } from "react";
import { Row, Column } from "react-table";
import FileSaver from "file-saver";
import { QueryContext } from "context/query";
import {
generateCSVFilename,
@@ -25,6 +24,7 @@ import generateReportColumnConfigsFromResults from "./QueryReportTableConfig";
interface IQueryReportProps {
queryReport?: IQueryReport;
queryId: number;
queryName?: string;
isClipped?: boolean;
canLiveQuery?: boolean;
}
@@ -49,11 +49,10 @@ const flattenResults = (results: IQueryReportResultRow[]) => {
const QueryReport = ({
queryReport,
queryId,
queryName,
isClipped,
canLiveQuery,
}: IQueryReportProps): JSX.Element => {
const { lastEditedQueryName } = useContext(QueryContext);
const [filteredResults, setFilteredResults] = useState<Row[]>(
flattenResults(queryReport?.results || [])
);
@@ -72,7 +71,7 @@ const QueryReport = ({
FileSaver.saveAs(
generateCSVQueryResults(
filteredResults,
generateCSVFilename(`${lastEditedQueryName || CSV_TITLE} - Report`),
generateCSVFilename(`${queryName || CSV_TITLE} - Report`),
columnConfigs
)
);