diff --git a/web/src/components/core/Page.test.tsx b/web/src/components/core/Page.test.tsx index ba197984b3..7cf43b43be 100644 --- a/web/src/components/core/Page.test.tsx +++ b/web/src/components/core/Page.test.tsx @@ -21,27 +21,33 @@ */ import React from "react"; -import { screen, within } from "@testing-library/react"; -import { installerRender, mockNavigateFn, mockRoutes, plainRender } from "~/test-utils"; +import { screen, waitFor, within } from "@testing-library/react"; +import { + installerRender, + mockNavigateFn, + mockProgresses, + mockRoutes, + plainRender, +} from "~/test-utils"; +import useTrackQueriesRefetch from "~/hooks/use-track-queries-refetch"; +import { COMMON_PROPOSAL_KEYS } from "~/hooks/model/proposal"; import { PRODUCT, ROOT } from "~/routes/paths"; import { _ } from "~/i18n"; import Page from "./Page"; let consoleErrorSpy: jest.SpyInstance; +let mockStartTracking: jest.Mock = jest.fn(); + +jest.mock("~/hooks/use-track-queries-refetch", () => ({ + __esModule: true, + default: jest.fn(), +})); jest.mock("~/components/product/ProductRegistrationAlert", () => () => (
ProductRegistrationAlertMock
)); -const mockUseStatus = jest.fn(); -jest.mock("~/hooks/model/status", () => ({ - useStatus: () => mockUseStatus(), -})); - -const mockOnProposalUpdated = jest.fn(); -jest.mock("~/hooks/model/proposal", () => ({ - onProposalUpdated: (callback: (detail: any) => void) => mockOnProposalUpdated(callback), -})); +const mockUseTrackQueriesRefetch = jest.mocked(useTrackQueriesRefetch); describe("Page", () => { beforeAll(() => { @@ -54,12 +60,11 @@ describe("Page", () => { }); beforeEach(() => { - mockUseStatus.mockReturnValue({ - progresses: [], + // Set up default mock for useTrackQueriesRefetch + mockUseTrackQueriesRefetch.mockReturnValue({ + startTracking: mockStartTracking, }); - - mockOnProposalUpdated.mockReturnValue(() => {}); - + mockNavigateFn.mockClear(); }); @@ -191,6 +196,7 @@ describe("Page", () => { expect(onClick).toHaveBeenCalled(); }); }); + describe("Page.Header", () => { it("renders a node that sticks to top", () => { const { container } = plainRender(The Header); @@ -266,10 +272,6 @@ describe("Page", () => { describe("when progress scope is provided but no matching progress exists", () => { it("does not render the backdrop", () => { - mockUseStatus.mockReturnValue({ - progresses: [], - }); - installerRender(Content); expect(screen.queryByRole("alert")).toBeNull(); }); @@ -277,19 +279,16 @@ describe("Page", () => { describe("when progress scope matches an active progress", () => { it("renders the backdrop with progress information", () => { - mockUseStatus.mockReturnValue({ - progresses: [ - { - scope: "software", - step: "Installing packages", - index: 2, - size: 5, - }, - ], - }); - + mockProgresses([ + { + scope: "software", + step: "Installing packages", + steps: [], + index: 2, + size: 5, + }, + ]); installerRender(Content); - const backdrop = screen.getByRole("alert", { name: /Installing packages/ }); expect(backdrop.classList).toContain("agm-main-content-overlay"); within(backdrop).getByText(/step 2 of 5/); @@ -297,62 +296,241 @@ describe("Page", () => { }); describe("when progress finishes", () => { - it.todo("shows 'Refreshing data...' message temporarily"); - it.todo("hides backdrop after proposal update event"); + let mockStartTracking: jest.Mock; + let mockCallback: (startedAt: number, completedAt: number) => void; + + beforeEach(() => { + mockStartTracking = jest.fn(); + mockUseTrackQueriesRefetch.mockImplementation((keys, callback) => { + mockCallback = callback; + return { startTracking: mockStartTracking }; + }); + }); + + // Test skipped because rerender fails when using installerRender, + // caused by how InstallerProvider manages context. + it.skip("shows 'Refreshing data...' message temporarily", async () => { + // Start with active progress + mockProgresses([ + { + scope: "storage", + step: "Calculating proposal", + steps: ["Calculating proposal"], + index: 1, + size: 1, + }, + ]); + + const { rerender } = installerRender(Content); + + const backdrop = screen.getByRole("alert", { name: /Calculating proposal/ }); + + // Progress finishes + mockProgresses([]); + + rerender(Content); + + // Should show "Refreshing data..." message + await waitFor(() => { + within(backdrop).getByText(/Refreshing data/); + }); + + // Should start tracking queries + expect(mockStartTracking).toHaveBeenCalled(); + }); + + // Test skipped because rerender fails when using installerRender, + // caused by how InstallerProvider manages context. + it.skip("hides backdrop after queries are refetched", async () => { + // Start with active progress + mockProgresses([ + { + scope: "storage", + step: "Calculating proposal", + steps: ["Calculating proposal"], + index: 1, + size: 1, + }, + ]); + + const { rerender } = installerRender(Content); + + // Progress finishes + mockProgresses([]); + + const backdrop = screen.getByRole("alert", { name: /Calculating proposal/ }); + + rerender(Content); + + // Should show refreshing message + await waitFor(() => { + within(backdrop).getByText(/Refreshing data/); + }); + + // Simulate queries completing by calling the callback + const startedAt = Date.now(); + mockCallback(startedAt, startedAt + 100); + + // Backdrop should be hidden + await waitFor(() => { + expect(screen.queryByRole("alert")).toBeNull(); + }); + }); }); describe("when progress scope does not match", () => { it("does not show backdrop for different scope", () => { - mockUseStatus.mockReturnValue({ - progresses: [ - { - scope: "software", - step: "Installing packages", - index: 2, - size: 5, - }, - ], - }); - + mockProgresses([ + { + scope: "software", + step: "Installing packages", + steps: [], + index: 2, + size: 5, + }, + ]); installerRender(Content); - - expect(screen.queryByRole("alert", { name: /Installing pckages/ })).toBeNull(); + expect(screen.queryByRole("alert", { name: /Installing packages/ })).toBeNull(); }); }); describe("multiple progress updates", () => { it("updates the backdrop message when progress changes", () => { - mockUseStatus.mockReturnValue({ - progresses: [ - { - scope: "software", - step: "Downloading packages", - index: 1, - size: 5, - }, - ], - }); - + mockProgresses([ + { + scope: "software", + step: "Downloading packages", + steps: [], + index: 1, + size: 5, + }, + ]); const { rerender } = installerRender(Content); + const backdrop = screen.getByRole("alert", { name: /Downloading packages/ }); + within(backdrop).getByText(/step 1 of 5/); + + mockProgresses([ + { + scope: "software", + step: "Installing packages", + steps: [], + index: 3, + size: 5, + }, + ]); + rerender(Content); + within(backdrop).getByText(/Installing packages/); + within(backdrop).getByText(/step 3 of 5/); + }); + }); - expect(screen.getByText(/Downloading packages/)).toBeInTheDocument(); - expect(screen.getByText(/step 1 of 5/)).toBeInTheDocument(); - - mockUseStatus.mockReturnValue({ - progresses: [ - { - scope: "software", - step: "Installing packages", - index: 3, - size: 5, - }, - ], - }); + describe("additionalProgressKeys prop", () => { + it("tracks common proposal keys by default", () => { + mockProgresses([ + { + scope: "software", + step: "Installing packages", + steps: [], + index: 1, + size: 3, + }, + ]); - rerender(Content); + installerRender(Content); - expect(screen.getByText(/Installing packages/)).toBeInTheDocument(); - expect(screen.getByText(/step 3 of 5/)).toBeInTheDocument(); + // Should be called with COMMON_PROPOSAL_KEYS and undefined additionalKeys + expect(mockUseTrackQueriesRefetch).toHaveBeenCalledWith( + expect.arrayContaining(COMMON_PROPOSAL_KEYS), + expect.any(Function), + ); + }); + + it("tracks additional query key along with common ones", () => { + mockProgresses([ + { + scope: "storage", + step: "Calculating proposal", + steps: [], + index: 1, + size: 1, + }, + ]); + + installerRender( + + Content + , + ); + + // Should be called with COMMON_PROPOSAL_KEYS + storageModel + expect(mockUseTrackQueriesRefetch).toHaveBeenCalledWith( + expect.arrayContaining([...COMMON_PROPOSAL_KEYS, "storageModel"]), + expect.any(Function), + ); + }); + + it("tracks multiple additional query keys along with common ones", () => { + mockProgresses([ + { + scope: "network", + step: "Configuring network", + steps: [], + index: 1, + size: 2, + }, + ]); + + installerRender( + + Content + , + ); + + // Should be called with COMMON_PROPOSAL_KEYS + networkConfig + connections + expect(mockUseTrackQueriesRefetch).toHaveBeenCalledWith( + expect.arrayContaining([...COMMON_PROPOSAL_KEYS, "networkConfig", "connections"]), + expect.any(Function), + ); + }); + + // Test skipped because rerender fails when using installerRender, + // caused by how InstallerProvider manages context. + it.skip("starts tracking when progress finishes", async () => { + // Start with active progress + mockProgresses([ + { + scope: "storage", + step: "Calculating proposal", + steps: ["Calculating proposal"], + index: 1, + size: 1, + }, + ]); + + const { rerender } = installerRender( + + Content + , + ); + + // Progress finishes + mockProgresses([]); + + rerender( + + Content + , + ); + rerender( + + Content + , + ); + + // Should have called startTracking + await waitFor(() => { + expect(mockStartTracking).toHaveBeenCalled(); + }); }); }); }); diff --git a/web/src/components/core/Page.tsx b/web/src/components/core/Page.tsx index f08ada87f8..6b853c51ea 100644 --- a/web/src/components/core/Page.tsx +++ b/web/src/components/core/Page.tsx @@ -51,13 +51,14 @@ import Link, { LinkProps } from "~/components/core/Link"; import textStyles from "@patternfly/react-styles/css/utilities/Text/text"; import flexStyles from "@patternfly/react-styles/css/utilities/Flex/flex"; import { useLocation, useNavigate } from "react-router"; -import { isEmpty, isObject } from "radashi"; +import { concat, isEmpty, isObject } from "radashi"; import { SIDE_PATHS } from "~/routes/paths"; import { _ } from "~/i18n"; import { sprintf } from "sprintf-js"; -import { onProposalUpdated } from "~/hooks/model/proposal"; +import { COMMON_PROPOSAL_KEYS } from "~/hooks/model/proposal"; import { useStatus } from "~/hooks/model/status"; import type { Scope } from "~/model/status"; +import useTrackQueriesRefetch from "~/hooks/use-track-queries-refetch"; /** * Props accepted by Page.Section @@ -316,6 +317,29 @@ type ProgressBackdropProps = { * displayed. */ progressScope?: Scope; + /** + * Additional query keys to track during progress operations. + * + * The progress backdrop automatically tracks common proposal-related queries. + * Use this prop to specify additional query keys that must complete + * refetching before the backdrop unblocks the UI. + * + * This is useful when a page needs to wait for additional queries beyond the + * common proposal-related ones, either because the page depends on them or + * because operations on the page invalidate them. + * + * @example + * // Track storage model updates in addition to common proposal queries + * + * + * @example + * // Track multiple additional queries + * + */ + additionalProgressKeys?: string | string[]; }; /** @@ -332,26 +356,30 @@ type ProgressBackdropProps = { * newer than when the progress finished arrives, ensuring the UI doesn't * unblock prematurely. */ -const ProgressBackdrop = ({ progressScope }: ProgressBackdropProps): React.ReactNode => { +const ProgressBackdrop = ({ + progressScope, + additionalProgressKeys, +}: ProgressBackdropProps): React.ReactNode => { const { progresses: tasks } = useStatus(); const [isBlocked, setIsBlocked] = useState(false); const [progressFinishedAt, setProgressFinishedAt] = useState(null); const progress = !isEmpty(progressScope) && tasks.find((t) => t.scope === progressScope); + const { startTracking } = useTrackQueriesRefetch( + concat(COMMON_PROPOSAL_KEYS, additionalProgressKeys), + (_, completedAt) => { + if (completedAt > progressFinishedAt) { + setIsBlocked(false); + setProgressFinishedAt(null); + } + }, + ); useEffect(() => { if (!progress && isBlocked && !progressFinishedAt) { setProgressFinishedAt(Date.now()); + startTracking(); } - }, [progress, isBlocked, progressFinishedAt]); - - useEffect(() => { - return onProposalUpdated((detail) => { - if (detail.completedAt > progressFinishedAt) { - setIsBlocked(false); - setProgressFinishedAt(null); - } - }); - }, [progressFinishedAt]); + }, [progress, isBlocked, progressFinishedAt, startTracking]); if (progress && !isBlocked) { setIsBlocked(true); @@ -427,12 +455,16 @@ const ProgressBackdrop = ({ progressScope }: ProgressBackdropProps): React.React const Page = ({ children, progressScope, + additionalProgressKeys, ...pageGroupProps }: PageGroupProps & ProgressBackdropProps): React.ReactNode => { return ( {children} - + ); }; diff --git a/web/src/components/storage/ProposalPage.tsx b/web/src/components/storage/ProposalPage.tsx index 8f33f919ba..59ba11f97d 100644 --- a/web/src/components/storage/ProposalPage.tsx +++ b/web/src/components/storage/ProposalPage.tsx @@ -55,7 +55,7 @@ import { useAvailableDevices } from "~/hooks/model/system/storage"; import { useIssues } from "~/hooks/model/issue"; import { useReset } from "~/hooks/model/config/storage"; import { useProposal } from "~/hooks/model/proposal/storage"; -import { useConfigModel } from "~/hooks/model/storage/config-model"; +import { STORAGE_MODEL_KEY, useConfigModel } from "~/hooks/model/storage/config-model"; import { useZFCPSupported } from "~/queries/storage/zfcp"; import { useDASDSupported } from "~/queries/storage/dasd"; import { STORAGE as PATHS } from "~/routes/paths"; @@ -328,7 +328,7 @@ export default function ProposalPage(): React.ReactNode { if (resetNeeded) return; return ( - + diff --git a/web/src/hooks/model/config.ts b/web/src/hooks/model/config.ts index 17fe1cc89c..3b237d17d6 100644 --- a/web/src/hooks/model/config.ts +++ b/web/src/hooks/model/config.ts @@ -27,8 +27,11 @@ import { useSystem } from "~/hooks/model/system"; import type { system } from "~/api"; import type { Config } from "~/model/config"; +const CONFIG_KEY = "config"; +const EXTENDED_CONFIG_KEY = "extendedConfig"; + const configQuery = { - queryKey: ["config"], + queryKey: [CONFIG_KEY], queryFn: getConfig, }; @@ -37,7 +40,7 @@ function useConfig(): Config | null { } const extendedConfigQuery = { - queryKey: ["extendedConfig"], + queryKey: [EXTENDED_CONFIG_KEY], queryFn: getExtendedConfig, }; @@ -61,5 +64,13 @@ function useProduct(): system.Product | null { return data; } -export { configQuery, extendedConfigQuery, useConfig, useExtendedConfig, useProduct }; +export { + CONFIG_KEY, + EXTENDED_CONFIG_KEY, + configQuery, + extendedConfigQuery, + useConfig, + useExtendedConfig, + useProduct, +}; export * as storage from "~/hooks/model/config/storage"; diff --git a/web/src/hooks/model/proposal.ts b/web/src/hooks/model/proposal.ts index aefa24bb83..b124071ccc 100644 --- a/web/src/hooks/model/proposal.ts +++ b/web/src/hooks/model/proposal.ts @@ -20,116 +20,19 @@ * find current contact information at www.suse.com. */ -import React, { useMemo } from "react"; +import React from "react"; import { useSuspenseQuery, useQueryClient } from "@tanstack/react-query"; import { getProposal } from "~/api"; import { useInstallerClient } from "~/context/installer"; import type { Proposal } from "~/model/proposal"; +import { EXTENDED_CONFIG_KEY } from "~/hooks/model/config"; +import { STORAGE_MODEL_KEY } from "~/hooks/model/storage/config-model"; -import useTrackQueriesRefetch from "~/hooks/use-track-queries-refetch"; - -/** - * Payload carried by the `proposal:updated` event. - */ -type ProposalUpdatedDetail = { - /** - * Timestamp (ms since epoch) indicating when the proposal update tracking - * process started. - */ - startedAt: number; - - /** - * Timestamp (ms since epoch) indicating when the proposal update process - * is considered finished. - */ - completedAt: number; -}; - -/** - * Event name dispatched when a proposal has finished updating. - * - * This event is used as a lightweight communication mechanism to notify - * interested parts of the application when a proposal is considered fresh - * after being invalidated and refetched at the TanStack Query level. - * - * The event is dispatched on `document` and carries timing information - * about the proposal update lifecycle. - */ -const PROPOSAL_UPDATED_EVENT = "proposal:updated" as const; - -/** - * Subscribes to the `proposal:updated` event. - * - * The provided handler will be called every time the event is dispatched. - * - * @param handler Callback invoked with the event detail payload. - * - * @returns A cleanup function that MUST be called to unsubscribe the - * listener and prevent memory leaks. - * - * @remarks - * This event-based API is intentionally DOM-based (`CustomEvent`) instead of - * using a global state or pub/sub library, in order to: - * - avoid extra dependencies - * - keep cross-module communication explicit - * - allow easy testing and inspection - * - * @example - * Plain usage - * ```ts - * const unsubscribe = onProposalUpdated(({ startedAt, completedAt }) => { - * console.log(`Proposal update tracking started at ${new Date(startedAt)}`); - * console.log(`Proposal updated at ${new Date(completedAt)}`); - * }); - * - * // Later, when no longer needed - * unsubscribe(); - * ``` - * - * @example - * Usage with React `useEffect` - * ```ts - * useEffect(() => { - * return onProposalUpdated(({ startedAt, completedAt }) => { - * console.log(`Proposal update tracking started at ${new Date(startedAt)}`); - * console.log(`Proposal updated at ${new Date(completedAt)}`); - * doSomethingElse(); - * }); - * }, []); - * ``` - */ -function onProposalUpdated(handler: (detail: ProposalUpdatedDetail) => void) { - const listener = (e: Event) => { - const customEvent = e as CustomEvent; - handler(customEvent.detail); - }; - - document.addEventListener(PROPOSAL_UPDATED_EVENT, listener); - - return () => document.removeEventListener(PROPOSAL_UPDATED_EVENT, listener); -} - -/** - * Dispatches the `proposal:updated` event. - * - * This should be called once a proposal update cycle has fully completed. - * All active subscribers registered via `onProposalUpdated` will be - * notified. - * - * @param detail Timing information about the update lifecycle. - * - * @example - * ```ts - * dispatchProposalUpdated({ startedAt, completedAt}); - * ``` - */ -function dispatchProposalUpdated(detail: ProposalUpdatedDetail) { - const event = new CustomEvent(PROPOSAL_UPDATED_EVENT, { detail }); - document.dispatchEvent(event); -} +const PROPOSAL_KEY = "proposal" as const; +const COMMON_PROPOSAL_KEYS = [PROPOSAL_KEY, EXTENDED_CONFIG_KEY] as const; const proposalQuery = { - queryKey: ["proposal"], + queryKey: [PROPOSAL_KEY], queryFn: getProposal, }; @@ -140,13 +43,6 @@ function useProposal(): Proposal | null { function useProposalChanges() { const queryClient = useQueryClient(); const client = useInstallerClient(); - const queriesToInvalidate = useMemo(() => ["proposal", "extendedConfig", "storageModel"], []); - const { startTracking } = useTrackQueriesRefetch( - queriesToInvalidate, - (startedAt, completedAt) => { - dispatchProposalUpdated({ startedAt, completedAt }); - }, - ); React.useEffect(() => { if (!client) return; @@ -154,16 +50,15 @@ function useProposalChanges() { // TODO: replace the scope instead of invalidating the query. return client.onEvent((event) => { if (event.type === "ProposalChanged") { - queriesToInvalidate.forEach((queryKey) => { + [...COMMON_PROPOSAL_KEYS, STORAGE_MODEL_KEY].forEach((queryKey) => { queryClient.invalidateQueries({ queryKey: [queryKey] }); }); - startTracking(); } }); - }, [client, queryClient, queriesToInvalidate, startTracking]); + }, [client, queryClient]); } -export { proposalQuery, useProposal, useProposalChanges, onProposalUpdated }; +export { COMMON_PROPOSAL_KEYS, proposalQuery, useProposal, useProposalChanges }; export * as l10n from "~/hooks/model/proposal/l10n"; export * as network from "~/hooks/model/proposal/network"; export * as storage from "~/hooks/model/proposal/storage"; diff --git a/web/src/hooks/model/storage/config-model.ts b/web/src/hooks/model/storage/config-model.ts index 6b3e07b4bd..70e26ff921 100644 --- a/web/src/hooks/model/storage/config-model.ts +++ b/web/src/hooks/model/storage/config-model.ts @@ -27,8 +27,10 @@ import { solveStorageModel, getStorageModel, putStorageModel } from "~/api"; import configModel from "~/model/storage/config-model"; import type { ConfigModel, Data, Partitionable } from "~/model/storage/config-model"; +const STORAGE_MODEL_KEY = "storageModel" as const; + const configModelQuery = { - queryKey: ["storageModel"], + queryKey: [STORAGE_MODEL_KEY], queryFn: getStorageModel, }; @@ -326,6 +328,7 @@ function useSetSpacePolicy(): setSpacePolicyFn { } export { + STORAGE_MODEL_KEY, useConfigModel, useSolvedConfigModel, useMissingMountPaths, diff --git a/web/src/test-utils.tsx b/web/src/test-utils.tsx index 0c0cd8b78d..8605ea914a 100644 --- a/web/src/test-utils.tsx +++ b/web/src/test-utils.tsx @@ -39,6 +39,7 @@ import { InstallerL10nProvider } from "~/context/installerL10n"; import { StorageUiStateProvider } from "~/context/storage-ui-state"; import { isObject, noop } from "radashi"; import { DummyWSClient } from "./client/ws"; +import { Status } from "./model/status"; /** * Internal mock for manipulating routes, using ["/"] by default @@ -104,6 +105,54 @@ jest.mock("react-router", () => ({ }, })); +/** + * Internal mock for manipulating progresses + */ +let progressesMock = jest.fn().mockReturnValue([]); + +/** + * Internal mock for manipulating stage + */ +let stageMock = jest.fn().mockReturnValue("configuring"); + +/** + * Allows mocking useStatus#progresses for testing purpose + * + * @example + * mockProgresses( + * [ + * { + * "scope": "software", + * "size": 3, + * "steps": [ + * "Updating the list of repositories", + * "Refreshing metadata from the repositories", + * "Calculating the software proposal" + * ], + * "step": "Refreshing metadata from the repositories", + * "index": 2 + * } + * ] + * ) + */ +const mockProgresses = (progresses: Status["progresses"]) => + progressesMock.mockReturnValue(progresses); + +/** + * Allows mocking useStatus#stage for testing purpose + * + * @example + * mockStage("configuring"); + */ +const mockStage = (stage: Status["stage"]) => stageMock.mockReturnValue(stage); + +jest.mock("~/hooks/model/status", () => ({ + useStatus: () => ({ + progresses: progressesMock(), + stage: stageMock(), + }), +})); + const Providers = ({ children, withL10n }) => { const ws = new DummyWSClient(); const client = createClient(new URL("https://localhost"), ws); @@ -136,11 +185,7 @@ const Providers = ({ children, withL10n }) => { ); } - return ( - - {children} - - ); + return {children}; }; /** @@ -259,4 +304,6 @@ export { mockUseRevalidator, resetLocalStorage, getColumnValues, + mockProgresses, + mockStage, };