From f4aeeb4fcc33ab6910dae4753e05acc899dd7555 Mon Sep 17 00:00:00 2001 From: philluiz2323 Date: Fri, 17 Jul 2026 16:10:02 -0300 Subject: [PATCH] fix(ui): use LoadingState/ErrorState in NotificationReadinessCard The card collapsed loading and error into one plain-text branch ("Loading notification model..." vs "Notification model unavailable."), discarding the errorKind/error useApiResource already exposes and giving no retry action -- unlike every other useApiResource consumer in this app, which already uses the shared LoadingState/ErrorState from state-views.tsx. Replaced the collapsed branch with those two components, wiring through the real error message, errorKind, and a retry action. The success-path rendering is unchanged. --- .../site/notification-readiness-card.test.tsx | 97 +++++++++++++++++++ .../site/notification-readiness-card.tsx | 14 ++- 2 files changed, 106 insertions(+), 5 deletions(-) create mode 100644 apps/loopover-ui/src/components/site/notification-readiness-card.test.tsx diff --git a/apps/loopover-ui/src/components/site/notification-readiness-card.test.tsx b/apps/loopover-ui/src/components/site/notification-readiness-card.test.tsx new file mode 100644 index 0000000000..c5a42c53a1 --- /dev/null +++ b/apps/loopover-ui/src/components/site/notification-readiness-card.test.tsx @@ -0,0 +1,97 @@ +import { fireEvent, render, screen } from "@testing-library/react"; +import { describe, expect, it, vi } from "vitest"; + +// #6985: a real fetch failure used to render the same generic text as "still loading" — these tests +// pin the three render paths (loading / error / success) now that LoadingState/ErrorState replace it. +const { useApiResource } = vi.hoisted(() => ({ useApiResource: vi.fn() })); +vi.mock("@/lib/api/use-api-resource", () => ({ + useApiResource: (...args: unknown[]) => useApiResource(...args), +})); + +import { NotificationReadinessCard } from "./notification-readiness-card"; + +const notificationModelFixture = { + notificationModel: { + mode: "opt_in", + defaultState: "disabled", + channels: [ + { + id: "browser_push", + transport: "web_push", + defaultEnabled: false, + purpose: "PR review updates", + }, + ], + privacyGuards: ["No content leaves the browser without consent."], + fallbackWhenUnavailable: "email digest", + }, + pwa: { nativeDependency: false, manifestPath: "/manifest.json", serviceWorkerPath: "/sw.js" }, + mobileReadyRoutes: [], + nativeMobileFuture: [], +}; + +describe("NotificationReadinessCard loading/error states (#6985)", () => { + it("shows a LoadingState (not the generic spinner-free text) while the model loads", () => { + useApiResource.mockReturnValue({ + status: "loading", + data: null, + error: null, + loadedAt: null, + reload: () => {}, + }); + + render(); + + expect(screen.getByText("Loading notification model…")).toBeTruthy(); + expect(screen.queryByText("Notification model unavailable.")).toBeNull(); + }); + + it("shows an ErrorState with the real error message and a working retry, distinguishing it from loading", () => { + const reload = vi.fn(); + useApiResource.mockReturnValue({ + status: "error", + data: null, + error: "The server returned a 500.", + errorKind: "http", + loadedAt: null, + reload, + }); + + render(); + + expect(screen.getByText("The server returned a 500.")).toBeTruthy(); + expect(screen.queryByText("Loading notification model…")).toBeNull(); + fireEvent.click(screen.getByRole("button", { name: "Try again" })); + expect(reload).toHaveBeenCalledTimes(1); + }); + + it("renders the network-specific ErrorState copy when errorKind indicates an unreachable server", () => { + useApiResource.mockReturnValue({ + status: "error", + data: null, + error: "Failed to fetch", + errorKind: "network", + loadedAt: null, + reload: () => {}, + }); + + render(); + + expect(screen.getByText("Can't reach the server")).toBeTruthy(); + }); + + it("still renders the privacy guards list unchanged on success", () => { + useApiResource.mockReturnValue({ + status: "ready", + data: notificationModelFixture, + error: null, + loadedAt: Date.now(), + reload: () => {}, + }); + + const { container } = render(); + + expect(container.textContent).toContain("No content leaves the browser without consent."); + expect(screen.queryByText("Loading notification model…")).toBeNull(); + }); +}); diff --git a/apps/loopover-ui/src/components/site/notification-readiness-card.tsx b/apps/loopover-ui/src/components/site/notification-readiness-card.tsx index 853477affd..36459045c2 100644 --- a/apps/loopover-ui/src/components/site/notification-readiness-card.tsx +++ b/apps/loopover-ui/src/components/site/notification-readiness-card.tsx @@ -2,6 +2,7 @@ import { Bell, BellOff } from "lucide-react"; import { useMemo, useState } from "react"; import { StatusPill } from "./control-primitives"; +import { ErrorState, LoadingState } from "./state-views"; import { useApiResource } from "@/lib/api/use-api-resource"; import { useLocalStorage } from "@/lib/use-local-storage"; @@ -112,12 +113,15 @@ export function NotificationReadinessCard() { ))} + ) : model.status === "loading" ? ( + ) : ( -

- {model.status === "loading" - ? "Loading notification model…" - : "Notification model unavailable."} -

+ )} );