Skip to content

Commit 6586c64

Browse files
fix(ui): use LoadingState/ErrorState in NotificationReadinessCard (#7047)
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. Co-authored-by: philluiz2323 <philluiz2323@gmail.com>
1 parent 8ab3a4d commit 6586c64

2 files changed

Lines changed: 106 additions & 5 deletions

File tree

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
import { fireEvent, render, screen } from "@testing-library/react";
2+
import { describe, expect, it, vi } from "vitest";
3+
4+
// #6985: a real fetch failure used to render the same generic text as "still loading" — these tests
5+
// pin the three render paths (loading / error / success) now that LoadingState/ErrorState replace it.
6+
const { useApiResource } = vi.hoisted(() => ({ useApiResource: vi.fn() }));
7+
vi.mock("@/lib/api/use-api-resource", () => ({
8+
useApiResource: (...args: unknown[]) => useApiResource(...args),
9+
}));
10+
11+
import { NotificationReadinessCard } from "./notification-readiness-card";
12+
13+
const notificationModelFixture = {
14+
notificationModel: {
15+
mode: "opt_in",
16+
defaultState: "disabled",
17+
channels: [
18+
{
19+
id: "browser_push",
20+
transport: "web_push",
21+
defaultEnabled: false,
22+
purpose: "PR review updates",
23+
},
24+
],
25+
privacyGuards: ["No content leaves the browser without consent."],
26+
fallbackWhenUnavailable: "email digest",
27+
},
28+
pwa: { nativeDependency: false, manifestPath: "/manifest.json", serviceWorkerPath: "/sw.js" },
29+
mobileReadyRoutes: [],
30+
nativeMobileFuture: [],
31+
};
32+
33+
describe("NotificationReadinessCard loading/error states (#6985)", () => {
34+
it("shows a LoadingState (not the generic spinner-free text) while the model loads", () => {
35+
useApiResource.mockReturnValue({
36+
status: "loading",
37+
data: null,
38+
error: null,
39+
loadedAt: null,
40+
reload: () => {},
41+
});
42+
43+
render(<NotificationReadinessCard />);
44+
45+
expect(screen.getByText("Loading notification model…")).toBeTruthy();
46+
expect(screen.queryByText("Notification model unavailable.")).toBeNull();
47+
});
48+
49+
it("shows an ErrorState with the real error message and a working retry, distinguishing it from loading", () => {
50+
const reload = vi.fn();
51+
useApiResource.mockReturnValue({
52+
status: "error",
53+
data: null,
54+
error: "The server returned a 500.",
55+
errorKind: "http",
56+
loadedAt: null,
57+
reload,
58+
});
59+
60+
render(<NotificationReadinessCard />);
61+
62+
expect(screen.getByText("The server returned a 500.")).toBeTruthy();
63+
expect(screen.queryByText("Loading notification model…")).toBeNull();
64+
fireEvent.click(screen.getByRole("button", { name: "Try again" }));
65+
expect(reload).toHaveBeenCalledTimes(1);
66+
});
67+
68+
it("renders the network-specific ErrorState copy when errorKind indicates an unreachable server", () => {
69+
useApiResource.mockReturnValue({
70+
status: "error",
71+
data: null,
72+
error: "Failed to fetch",
73+
errorKind: "network",
74+
loadedAt: null,
75+
reload: () => {},
76+
});
77+
78+
render(<NotificationReadinessCard />);
79+
80+
expect(screen.getByText("Can't reach the server")).toBeTruthy();
81+
});
82+
83+
it("still renders the privacy guards list unchanged on success", () => {
84+
useApiResource.mockReturnValue({
85+
status: "ready",
86+
data: notificationModelFixture,
87+
error: null,
88+
loadedAt: Date.now(),
89+
reload: () => {},
90+
});
91+
92+
const { container } = render(<NotificationReadinessCard />);
93+
94+
expect(container.textContent).toContain("No content leaves the browser without consent.");
95+
expect(screen.queryByText("Loading notification model…")).toBeNull();
96+
});
97+
});

apps/loopover-ui/src/components/site/notification-readiness-card.tsx

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { Bell, BellOff } from "lucide-react";
22
import { useMemo, useState } from "react";
33

44
import { StatusPill } from "./control-primitives";
5+
import { ErrorState, LoadingState } from "./state-views";
56
import { useApiResource } from "@/lib/api/use-api-resource";
67
import { useLocalStorage } from "@/lib/use-local-storage";
78

@@ -112,12 +113,15 @@ export function NotificationReadinessCard() {
112113
))}
113114
</ul>
114115
</div>
116+
) : model.status === "loading" ? (
117+
<LoadingState className="mt-3" title="Loading notification model…" />
115118
) : (
116-
<p className="mt-3 text-token-sm text-muted-foreground">
117-
{model.status === "loading"
118-
? "Loading notification model…"
119-
: "Notification model unavailable."}
120-
</p>
119+
<ErrorState
120+
className="mt-3"
121+
description={model.error}
122+
errorKind={model.errorKind}
123+
onRetry={model.reload}
124+
/>
121125
)}
122126
</section>
123127
);

0 commit comments

Comments
 (0)