From eb0ff35d4759173f3b32669e145dba09518adf6c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Wed, 30 Jul 2025 17:47:38 +0100 Subject: [PATCH 1/8] feat(web): add infrastructure for displaying out-of-sync alert Introduce a reusable component that listens for change events on a given scope and displays a toast alert when the change comes from a different client. This helps notify users when the interface may be out of sync due to external changes, without forcing a reload or disrupting their workflow. Its usage is straightforward and consists of mounting one instance per watched scope in the desired pages. In future iterations, the component may support accepting multiple scopes if a real need for that arises. --- web/src/assets/styles/index.scss | 6 +- .../components/core/AlertOutOfSync.test.tsx | 136 ++++++++++++++++++ web/src/components/core/AlertOutOfSync.tsx | 115 +++++++++++++++ 3 files changed, 256 insertions(+), 1 deletion(-) create mode 100644 web/src/components/core/AlertOutOfSync.test.tsx create mode 100644 web/src/components/core/AlertOutOfSync.tsx diff --git a/web/src/assets/styles/index.scss b/web/src/assets/styles/index.scss index 8c1dc3f55e..29723f3686 100644 --- a/web/src/assets/styles/index.scss +++ b/web/src/assets/styles/index.scss @@ -339,7 +339,7 @@ strong { // --pf-v6-c-nav--PaddingBlockStart: 0; // } -.pf-v6-c-alert { +:not(.pf-v6-c-alert-group__item) > .pf-v6-c-alert { --pf-v6-c-alert--m-info__title--Color: var(--agm-t--color--waterhole); --pf-v6-c-alert__icon--FontSize: var(--pf-t--global--font--size--md); --pf-v6-c-content--MarginBlockEnd: var(--pf-t--global--spacer--xs); @@ -377,6 +377,10 @@ strong { } } +.pf-v6-c-alert-group.pf-m-toast { + padding: var(--pf-t--global--spacer--xl); +} + .pf-v6-c-alert__title { font-size: var(--pf-t--global--font--size--md); } diff --git a/web/src/components/core/AlertOutOfSync.test.tsx b/web/src/components/core/AlertOutOfSync.test.tsx new file mode 100644 index 0000000000..47687c4607 --- /dev/null +++ b/web/src/components/core/AlertOutOfSync.test.tsx @@ -0,0 +1,136 @@ +/* + * Copyright (c) [2025] SUSE LLC + * + * All Rights Reserved. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the Free + * Software Foundation; either version 2 of the License, or (at your option) + * any later version. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, contact SUSE LLC. + * + * To contact SUSE LLC about this file by physical or electronic mail, you may + * find current contact information at www.suse.com. + */ + +import React, { act } from "react"; +import { screen } from "@testing-library/dom"; +import { installerRender, plainRender } from "~/test-utils"; +import AlertOutOfSync from "./AlertOutOfSync"; + +const mockOnEvent = jest.fn(); + +const mockClient = { + id: "current-client", + isConnected: jest.fn().mockResolvedValue(true), + isRecoverable: jest.fn(), + onConnect: jest.fn(), + onClose: jest.fn(), + onError: jest.fn(), + onEvent: mockOnEvent, +}; + +let consoleErrorSpy: jest.SpyInstance; + +jest.mock("~/context/installer", () => ({ + ...jest.requireActual("~/context/installer"), + useInstallerClient: () => mockClient, +})); + +describe("AlertOutOfSync", () => { + beforeAll(() => { + consoleErrorSpy = jest.spyOn(console, "error"); + consoleErrorSpy.mockImplementation(); + }); + + it("renders nothing if scope is missing", () => { + // @ts-expect-error: scope is required prop + const { container } = plainRender(); + expect(container).toBeEmptyDOMElement(); + expect(console.error).toHaveBeenCalledWith( + expect.stringContaining("must receive a value for `scope`"), + ); + }); + + it("renders nothing if scope empty", () => { + const { container } = plainRender(); + expect(container).toBeEmptyDOMElement(); + expect(console.error).toHaveBeenCalledWith( + expect.stringContaining("must receive a value for `scope`"), + ); + }); + + it("shows alert on matching changes event from a different client for subscribed scope", () => { + let eventCallback; + mockClient.onEvent.mockImplementation((cb) => { + eventCallback = cb; + return () => {}; + }); + installerRender(); + + // Should not render the alert initially + expect(screen.queryByText("Info alert:")).toBeNull(); + + // Simulate a change event for a different scope + act(() => { + eventCallback({ type: "NotWatchedChanged", clientId: "other-client" }); + }); + + expect(screen.queryByText("Info alert:")).toBeNull(); + + // Simulate a change event for the subscribed scope, from current client + act(() => { + eventCallback({ type: "WatchedChanged", clientId: "current-client" }); + }); + + expect(screen.queryByText("Info alert:")).toBeNull(); + + // Simulate a change event for the subscribed scope, from different client + act(() => { + eventCallback({ type: "WatchedChanged", clientId: "other-client" }); + }); + + screen.getByText("Info alert:"); + screen.getByText("Configuration out of sync"); + screen.getByText(/issues or data loss/); + screen.getByRole("link", { name: "Reload now" }); + }); + + it("dismisses automatically the alert on matching changes event from current client for subscribed scope", () => { + let eventCallback; + mockClient.onEvent.mockImplementation((cb) => { + eventCallback = cb; + return () => {}; + }); + installerRender(); + + // Should not render the alert initially + expect(screen.queryByText("Info alert:")).toBeNull(); + + // Simulate a change event for the subscribed scope, from different client + act(() => { + eventCallback({ type: "WatchedChanged", clientId: "other-client" }); + }); + + screen.getByText("Info alert:"); + screen.getByText("Configuration out of sync"); + screen.getByText(/issues or data loss/); + screen.getByRole("link", { name: "Reload now" }); + + // Simulate a change event for the subscribed scope, from current client + act(() => { + eventCallback({ type: "WatchedChanged", clientId: "current-client" }); + }); + + expect(screen.queryByText("Info alert:")).toBeNull(); + }); + + it.todo("refresh page when clicking on `Reload now`"); +}); diff --git a/web/src/components/core/AlertOutOfSync.tsx b/web/src/components/core/AlertOutOfSync.tsx new file mode 100644 index 0000000000..392954e3d0 --- /dev/null +++ b/web/src/components/core/AlertOutOfSync.tsx @@ -0,0 +1,115 @@ +/* + * Copyright (c) [2025] SUSE LLC + * + * All Rights Reserved. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the Free + * Software Foundation; either version 2 of the License, or (at your option) + * any later version. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, contact SUSE LLC. + * + * To contact SUSE LLC about this file by physical or electronic mail, you may + * find current contact information at www.suse.com. + */ + +import React, { useEffect, useState } from "react"; +import { + Alert, + AlertActionCloseButton, + AlertGroup, + AlertProps, + Content, +} from "@patternfly/react-core"; +import { Link } from "react-router-dom"; +import Text from "./Text"; +import { useInstallerClient } from "~/context/installer"; +import { isEmpty } from "radashi"; +import { _ } from "~/i18n"; + +type AlertOutOfSyncProps = Partial & { + /** + * The scope to listen for change events on (e.g., `SoftwareProposal`, + * `L10nConfig`). + */ + scope: string; +}; + +/** + * Reactive alert shown when the configuration for a given scope has been + * changed externally. + * + * It warns that the interface may be out of sync and recommends reloading + * before continuing to avoid issues and data loss. Reloading is intentionally + * left up to the user rather than forced automatically, to prevent confusion + * caused by unexpected refreshes. + * + * It works by listening for "Changed" events on the specified scope: + * + * - Displays a toast alert if the event originates from a different client + * (based on client ID). + * - Automatically dismisses the alert if a subsequent event originates from + * the current client. + * + * @example + * ```tsx + * + * ``` + */ +export default function AlertOutOfSync({ scope, ...alertProps }: AlertOutOfSyncProps) { + const client = useInstallerClient(); + const [active, setActive] = useState(false); + const missingScope = isEmpty(scope); + + useEffect(() => { + if (missingScope) return; + + return client.onEvent((event) => { + event.type === `${scope}Changed` && setActive(event.clientId !== client.id); + }); + }); + + if (missingScope) { + console.error("AlertOutOfSync must receive a value for `scope` prop"); + return; + } + + const title = _("Configuration out of sync"); + + return ( + + {active && ( + setActive(false)} + /> + } + {...alertProps} + key={`${scope}-out-of-sync`} + > + + {_( + "The configuration has been updated externally. To avoid issues or data loss, \ +please reload the page to ensure you're working with the latest data.", + )} + + + {_("Reload now")} + + + )} + + ); +} From bf2faa2aaa8fdea0f31b4af4e64a85b7eeae1624 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Thu, 31 Jul 2025 08:38:25 +0100 Subject: [PATCH 2/8] fix(web): simplify out of sync alert text By making sentence shorter and not using "Please". --- web/src/components/core/AlertOutOfSync.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/web/src/components/core/AlertOutOfSync.tsx b/web/src/components/core/AlertOutOfSync.tsx index 392954e3d0..31962ff87f 100644 --- a/web/src/components/core/AlertOutOfSync.tsx +++ b/web/src/components/core/AlertOutOfSync.tsx @@ -101,8 +101,8 @@ export default function AlertOutOfSync({ scope, ...alertProps }: AlertOutOfSyncP > {_( - "The configuration has been updated externally. To avoid issues or data loss, \ -please reload the page to ensure you're working with the latest data.", + "The configuration has been updated externally. \ +Reload the page to get the latest data and avoid issues or data loss.", )} From 847ad24ec044f16ef02de85e9f7355f7a452f05e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Thu, 31 Jul 2025 10:04:29 +0100 Subject: [PATCH 3/8] feat(web): make client aware of its ID Listen for the event that provides the ID assigned by the server and store it internally. This allows the client to use its ID when needed, for example, to determine when to show an out-of-sync alert triggered by events from other clients. --- web/src/client/index.ts | 2 ++ web/src/context/installer.tsx | 7 +++++++ 2 files changed, 9 insertions(+) diff --git a/web/src/client/index.ts b/web/src/client/index.ts index 81c33e12db..c881f3d887 100644 --- a/web/src/client/index.ts +++ b/web/src/client/index.ts @@ -26,6 +26,8 @@ type VoidFn = () => void; type BooleanFn = () => boolean; export type InstallerClient = { + /** Unique client identifier. */ + id?: string; /** Whether the client is connected. */ isConnected: BooleanFn; /** Whether the client is recoverable after disconnecting. */ diff --git a/web/src/context/installer.tsx b/web/src/context/installer.tsx index 4a2f8c650f..799419ef79 100644 --- a/web/src/context/installer.tsx +++ b/web/src/context/installer.tsx @@ -53,6 +53,13 @@ function InstallerClientProvider({ children, client = null }: InstallerClientPro useEffect(() => { const connectClient = async () => { const client = await createDefaultClient(); + + client.onEvent((event) => { + if (event.type === "ClientConnected") { + client.id = event.clientId; + } + }); + setValue(client); }; From eec9d8ad2ff85de8e2cef64dd0335eb7115a23b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Thu, 31 Jul 2025 10:09:22 +0100 Subject: [PATCH 4/8] feat(web): enable use of `replace` prop in core/Link Allow using the `replace` prop in core/Link to support replacing the current history entry during navigation. This enables using core/Link instead of react-router-dom/Link to maintain consistent look and feel across the app, as core/Link is built on top of PF/Button. --- web/src/components/core/AlertOutOfSync.tsx | 4 ++-- web/src/components/core/Link.tsx | 26 +++++++++++++++++++--- 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/web/src/components/core/AlertOutOfSync.tsx b/web/src/components/core/AlertOutOfSync.tsx index 31962ff87f..ea9c62b321 100644 --- a/web/src/components/core/AlertOutOfSync.tsx +++ b/web/src/components/core/AlertOutOfSync.tsx @@ -28,7 +28,7 @@ import { AlertProps, Content, } from "@patternfly/react-core"; -import { Link } from "react-router-dom"; +import Link from "./Link"; import Text from "./Text"; import { useInstallerClient } from "~/context/installer"; import { isEmpty } from "radashi"; @@ -105,7 +105,7 @@ export default function AlertOutOfSync({ scope, ...alertProps }: AlertOutOfSyncP Reload the page to get the latest data and avoid issues or data loss.", )} - + {_("Reload now")} diff --git a/web/src/components/core/Link.tsx b/web/src/components/core/Link.tsx index c5816fcfa1..d05401db7b 100644 --- a/web/src/components/core/Link.tsx +++ b/web/src/components/core/Link.tsx @@ -22,11 +22,13 @@ import React from "react"; import { Button, ButtonProps } from "@patternfly/react-core"; -import { To, useHref } from "react-router-dom"; +import { To, useHref, useLinkClickHandler } from "react-router-dom"; export type LinkProps = Omit & { /** The target route */ to: string | To; + /** Whether the link should replace the current entry in the browser history */ + replace?: boolean; /** Whether use PF/Button primary variant */ isPrimary?: boolean; }; @@ -37,11 +39,29 @@ export type LinkProps = Omit & { * @note when isPrimary not given or false and props does not contain a variant prop, * it will default to "secondary" variant */ -export default function Link({ to, isPrimary, variant, children, ...props }: LinkProps) { +export default function Link({ + to, + replace = false, + isPrimary, + variant, + children, + onClick, + ...props +}: LinkProps) { const href = useHref(to); const linkVariant = isPrimary ? "primary" : variant || "secondary"; + const handleClick = useLinkClickHandler(to, { replace }); return ( - ); From 062686956985365a77ed3dce7a624a6350fb8b4e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Thu, 31 Jul 2025 10:16:57 +0100 Subject: [PATCH 5/8] doc(web): add entry to changes file --- web/package/agama-web-ui.changes | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/web/package/agama-web-ui.changes b/web/package/agama-web-ui.changes index 5418693509..98eaa8b0f8 100644 --- a/web/package/agama-web-ui.changes +++ b/web/package/agama-web-ui.changes @@ -1,3 +1,9 @@ +------------------------------------------------------------------- +Thu Jul 31 09:13:09 UTC 2025 - David Diaz + +- Allow displaying out-of-sync alerts by listening to changes + events (gh#agama-project/agama#2630). + ------------------------------------------------------------------- Mon Jul 21 15:07:40 UTC 2025 - Imobach Gonzalez Sosa From 88839db660345923d43ddc118db88aa13300d1bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Thu, 31 Jul 2025 10:27:57 +0100 Subject: [PATCH 6/8] fix(web): add missing mock To avoid real execution of react-router-dom/useLinkClickHandler, now used at core/Link --- web/src/test-utils.tsx | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/web/src/test-utils.tsx b/web/src/test-utils.tsx index c74603f33f..bd4fb144e3 100644 --- a/web/src/test-utils.tsx +++ b/web/src/test-utils.tsx @@ -96,6 +96,11 @@ jest.mock("react-router-dom", () => ({ Navigate: ({ to: route }) => <>Navigating to {route}, Outlet: () => <>Outlet Content, useRevalidator: () => mockUseRevalidator, + useLinkClickHandler: + ({ to }) => + () => { + to; + }, })); const Providers = ({ children, withL10n }) => { From 84743d83024b55aea9cb908c831c4aa6fd5aace0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Thu, 31 Jul 2025 11:06:27 +0100 Subject: [PATCH 7/8] fix(web): force a reload via JavaScript By using the locationReload util instead of just "navigating to the same place", since this does not work for storage. --- .../components/core/AlertOutOfSync.test.tsx | 30 +++++++++++++++++-- web/src/components/core/AlertOutOfSync.tsx | 10 +++---- 2 files changed, 32 insertions(+), 8 deletions(-) diff --git a/web/src/components/core/AlertOutOfSync.test.tsx b/web/src/components/core/AlertOutOfSync.test.tsx index 47687c4607..1eaf6166db 100644 --- a/web/src/components/core/AlertOutOfSync.test.tsx +++ b/web/src/components/core/AlertOutOfSync.test.tsx @@ -26,6 +26,7 @@ import { installerRender, plainRender } from "~/test-utils"; import AlertOutOfSync from "./AlertOutOfSync"; const mockOnEvent = jest.fn(); +const mockReload = jest.fn(); const mockClient = { id: "current-client", @@ -43,6 +44,10 @@ jest.mock("~/context/installer", () => ({ ...jest.requireActual("~/context/installer"), useInstallerClient: () => mockClient, })); +jest.mock("~/utils", () => ({ + ...jest.requireActual("~/utils"), + locationReload: () => mockReload(), +})); describe("AlertOutOfSync", () => { beforeAll(() => { @@ -100,7 +105,7 @@ describe("AlertOutOfSync", () => { screen.getByText("Info alert:"); screen.getByText("Configuration out of sync"); screen.getByText(/issues or data loss/); - screen.getByRole("link", { name: "Reload now" }); + screen.getByRole("button", { name: "Reload now" }); }); it("dismisses automatically the alert on matching changes event from current client for subscribed scope", () => { @@ -122,7 +127,7 @@ describe("AlertOutOfSync", () => { screen.getByText("Info alert:"); screen.getByText("Configuration out of sync"); screen.getByText(/issues or data loss/); - screen.getByRole("link", { name: "Reload now" }); + screen.getByRole("button", { name: "Reload now" }); // Simulate a change event for the subscribed scope, from current client act(() => { @@ -132,5 +137,24 @@ describe("AlertOutOfSync", () => { expect(screen.queryByText("Info alert:")).toBeNull(); }); - it.todo("refresh page when clicking on `Reload now`"); + it("trigger a location relaod when clicking on `Reload now`", async () => { + let eventCallback; + mockClient.onEvent.mockImplementation((cb) => { + eventCallback = cb; + return () => {}; + }); + const { user } = installerRender(); + + // Should not render the alert initially + expect(screen.queryByText("Info alert:")).toBeNull(); + + // Simulate a change event for the subscribed scope, from different client + act(() => { + eventCallback({ type: "WatchedChanged", clientId: "other-client" }); + }); + + const reloadButton = screen.getByRole("button", { name: "Reload now" }); + await user.click(reloadButton); + expect(mockReload).toHaveBeenCalled(); + }); }); diff --git a/web/src/components/core/AlertOutOfSync.tsx b/web/src/components/core/AlertOutOfSync.tsx index ea9c62b321..773bed87c2 100644 --- a/web/src/components/core/AlertOutOfSync.tsx +++ b/web/src/components/core/AlertOutOfSync.tsx @@ -26,13 +26,13 @@ import { AlertActionCloseButton, AlertGroup, AlertProps, + Button, Content, } from "@patternfly/react-core"; -import Link from "./Link"; -import Text from "./Text"; import { useInstallerClient } from "~/context/installer"; import { isEmpty } from "radashi"; import { _ } from "~/i18n"; +import { locationReload } from "~/utils"; type AlertOutOfSyncProps = Partial & { /** @@ -105,9 +105,9 @@ export default function AlertOutOfSync({ scope, ...alertProps }: AlertOutOfSyncP Reload the page to get the latest data and avoid issues or data loss.", )} - - {_("Reload now")} - + )} From 9e72bb54388226004fe30741028b151b3f20baa0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Thu, 31 Jul 2025 11:42:36 +0100 Subject: [PATCH 8/8] fix(web): minor adjustments in test file --- web/src/components/core/AlertOutOfSync.test.tsx | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/web/src/components/core/AlertOutOfSync.test.tsx b/web/src/components/core/AlertOutOfSync.test.tsx index 1eaf6166db..9e2f76b1aa 100644 --- a/web/src/components/core/AlertOutOfSync.test.tsx +++ b/web/src/components/core/AlertOutOfSync.test.tsx @@ -78,6 +78,7 @@ describe("AlertOutOfSync", () => { eventCallback = cb; return () => {}; }); + installerRender(); // Should not render the alert initially @@ -114,10 +115,8 @@ describe("AlertOutOfSync", () => { eventCallback = cb; return () => {}; }); - installerRender(); - // Should not render the alert initially - expect(screen.queryByText("Info alert:")).toBeNull(); + installerRender(); // Simulate a change event for the subscribed scope, from different client act(() => { @@ -137,16 +136,14 @@ describe("AlertOutOfSync", () => { expect(screen.queryByText("Info alert:")).toBeNull(); }); - it("trigger a location relaod when clicking on `Reload now`", async () => { + it("triggers a location relaod when clicking on `Reload now`", async () => { let eventCallback; mockClient.onEvent.mockImplementation((cb) => { eventCallback = cb; return () => {}; }); - const { user } = installerRender(); - // Should not render the alert initially - expect(screen.queryByText("Info alert:")).toBeNull(); + const { user } = installerRender(); // Simulate a change event for the subscribed scope, from different client act(() => {