Skip to content

Commit 80e3fa9

Browse files
myasnikovdaniilmyasnikovdaniil
authored
fix(dashboard): render error and unknown-type states instead of an infinite spinner (#3098)
## What this PR does Two console pages ordered their loading guard ahead of the terminal state, making the latter unreachable. Both are pre-existing upstream behaviors carried in by the cozystack-ui vendoring (#2963) and flagged as non-blocking findings in that PR's review. - **`ApplicationDetailPage`** checked `isLoading || !instance || !ad` before `if (error)`. On a failed GET (403/404/5xx) `isLoading` is false and `instance` stays undefined, so the page rendered the loading spinner forever and never reached the "not found" branch. The error branch now runs first. - **`ApplicationListPage`**'s `defsLoading || (!ad && plural)` guard stayed true for any unrecognized `plural` once definitions had loaded, so the "Unknown application type." branch was unreachable. The guard now only spins while definitions are loading. ## Verification - Added co-located regression tests for the failed-GET (detail) and unknown-plural (list) paths. - `pnpm typecheck` clean; full console suite green (`pnpm test`: 317 passed, +4 new). ```release-note fix(dashboard): the console now shows a not-found message on a failed application GET and an unknown-type message for an unrecognized resource, instead of spinning forever ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved loading and error handling on application pages so users see the correct message at the right time. * “Unknown application type” now appears only after application definitions finish loading, instead of being masked by a loading state. * Application detail pages now show a “not found” message when the requested app can’t be loaded, rather than continuing to display a spinner. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
2 parents 61a5743 + 4838baf commit 80e3fa9

4 files changed

Lines changed: 124 additions & 7 deletions

File tree

‎packages/system/dashboard/images/console/apps/console/src/routes/ApplicationListPage.test.tsx‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
import { describe, it, expect, vi } from "vitest"
2+
import { render, screen } from "@testing-library/react"
3+
import { MemoryRouter, Routes, Route } from "react-router"
4+
5+
// Drive the two upstream hooks per test; the page's guards are the unit under test.
6+
const h = vi.hoisted(() => ({
7+
defs: { data: undefined as unknown, isLoading: true },
8+
instances: { data: undefined as unknown, isLoading: false },
9+
}))
10+
11+
vi.mock("../lib/app-definitions.ts", () => ({
12+
useApplicationDefinitions: () => h.defs,
13+
useApplicationInstances: () => h.instances,
14+
iconDataUrl: () => null,
15+
appDisplayName: () => "App",
16+
}))
17+
vi.mock("../lib/tenant-context.tsx", () => ({
18+
useTenantContext: () => ({ tenantNamespace: "tenant-test", selectedTenant: "test" }),
19+
}))
20+
21+
const { ApplicationListPage } = await import("./ApplicationListPage.tsx")
22+
23+
function renderAt(plural: string) {
24+
return render(
25+
<MemoryRouter initialEntries={[`/console/${plural}`]}>
26+
<Routes>
27+
<Route path="/console/:plural" element={<ApplicationListPage />} />
28+
</Routes>
29+
</MemoryRouter>,
30+
)
31+
}
32+
33+
describe("ApplicationListPage guards", () => {
34+
it("shows the unknown-type message once defs have loaded and the plural is unrecognized", () => {
35+
// Regression: previously `defsLoading || (!ad && plural)` kept the spinner
36+
// up forever for an unknown plural, so this branch was unreachable.
37+
h.defs = { data: { items: [] }, isLoading: false }
38+
renderAt("nonexistent")
39+
expect(screen.getByText("Unknown application type.")).toBeInTheDocument()
40+
expect(screen.queryByText("Loading…")).not.toBeInTheDocument()
41+
})
42+
43+
it("still shows the spinner while definitions are loading", () => {
44+
h.defs = { data: undefined, isLoading: true }
45+
renderAt("postgreses")
46+
expect(screen.getByText("Loading…")).toBeInTheDocument()
47+
expect(screen.queryByText("Unknown application type.")).not.toBeInTheDocument()
48+
})
49+
})

‎packages/system/dashboard/images/console/apps/console/src/routes/ApplicationListPage.tsx‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,13 +26,14 @@ export function ApplicationListPage() {
2626
const { data, isLoading } = useApplicationInstances(ad, tenantNamespace ?? undefined)
2727
const items = data?.items ?? []
2828

29-
if (defsLoading || (!ad && plural)) {
29+
if (defsLoading) {
3030
return (
3131
<div className="flex items-center gap-2 p-6 text-sm text-slate-500">
3232
<Spinner /> Loading…
3333
</div>
3434
)
3535
}
36+
// defs have loaded; an unresolved plural is genuinely unknown, not pending.
3637
if (!ad) {
3738
return <div className="p-6 text-sm text-red-600">Unknown application type.</div>
3839
}

‎packages/system/dashboard/images/console/apps/console/src/routes/detail/ApplicationDetailPage.test.tsx‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
import { describe, it, expect, vi } from "vitest"
2+
import { render, screen } from "@testing-library/react"
3+
import { MemoryRouter, Routes, Route } from "react-router"
4+
5+
// Drive useK8sGet's result per test; the page's loading/error guards are the unit under test.
6+
const h = vi.hoisted(() => ({
7+
get: { data: undefined as unknown, isLoading: true, error: undefined as unknown },
8+
}))
9+
10+
vi.mock("@cozystack/k8s-client", () => ({
11+
useK8sGet: () => h.get,
12+
useK8sDelete: () => ({ mutateAsync: vi.fn() }),
13+
}))
14+
vi.mock("../../lib/app-definitions.ts", () => ({
15+
useApplicationDefinitions: () => ({
16+
data: { items: [{ metadata: { name: "postgres" }, spec: { application: { plural: "postgreses", kind: "Postgres" } } }] },
17+
}),
18+
appDisplayName: () => "Postgres",
19+
iconDataUrl: () => null,
20+
}))
21+
vi.mock("../../lib/tenant-context.tsx", () => ({
22+
useTenantContext: () => ({ tenantNamespace: "tenant-test" }),
23+
}))
24+
// Stub the tab tree — the loading/error branches return before any tab renders,
25+
// and these modules pull heavy deps (noVNC, Monaco) we don't want in jsdom.
26+
vi.mock("./tabs.tsx", () => ({ TabBar: () => null }))
27+
vi.mock("./OverviewTab.tsx", () => ({ OverviewTab: () => null }))
28+
vi.mock("./WorkloadsTab.tsx", () => ({ WorkloadsTab: () => null }))
29+
vi.mock("./ServicesTab.tsx", () => ({ ServicesTab: () => null }))
30+
vi.mock("./IngressesTab.tsx", () => ({ IngressesTab: () => null }))
31+
vi.mock("./SecretsTab.tsx", () => ({ SecretsTab: () => null }))
32+
vi.mock("./EventsTab.tsx", () => ({ EventsTab: () => null }))
33+
vi.mock("./VncTab.tsx", () => ({ VncTab: () => null }))
34+
vi.mock("./VMPowerControls.tsx", () => ({ VMPowerControls: () => null }))
35+
36+
const { ApplicationDetailPage } = await import("./ApplicationDetailPage.tsx")
37+
38+
function renderPage() {
39+
return render(
40+
<MemoryRouter initialEntries={["/console/postgreses/demo"]}>
41+
<Routes>
42+
<Route path="/console/:plural/:name" element={<ApplicationDetailPage />} />
43+
</Routes>
44+
</MemoryRouter>,
45+
)
46+
}
47+
48+
describe("ApplicationDetailPage guards", () => {
49+
it("renders the not-found message on a failed GET instead of an infinite spinner", () => {
50+
// Regression: previously `isLoading || !instance || !ad` ran before the
51+
// error branch, so a failed GET (isLoading=false, instance=undefined) spun
52+
// forever and the error branch was unreachable.
53+
h.get = { data: undefined, isLoading: false, error: new Error("403 Forbidden") }
54+
renderPage()
55+
expect(screen.getByText("not found.", { exact: false })).toBeInTheDocument()
56+
expect(screen.queryByText("Loading…")).not.toBeInTheDocument()
57+
})
58+
59+
it("still shows the spinner while the instance is loading", () => {
60+
h.get = { data: undefined, isLoading: true, error: undefined }
61+
renderPage()
62+
expect(screen.getByText("Loading…")).toBeInTheDocument()
63+
})
64+
})

‎packages/system/dashboard/images/console/apps/console/src/routes/detail/ApplicationDetailPage.tsx‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -69,17 +69,20 @@ export function ApplicationDetailPage() {
6969
})
7070

7171
if (!plural || !name) return <Navigate to="/console" replace />
72-
if (isLoading || !instance || !ad) {
72+
// Check the fetch error before the loading guard: on a failed GET, isLoading
73+
// is false and instance stays undefined, so a !instance-first guard would
74+
// spin forever and never reach this branch.
75+
if (error) {
7376
return (
74-
<div className="flex items-center gap-2 p-8 text-slate-500">
75-
<Spinner /> Loading…
77+
<div className="p-8 text-red-600">
78+
Application <code>{name}</code> not found.
7679
</div>
7780
)
7881
}
79-
if (error) {
82+
if (isLoading || !instance || !ad) {
8083
return (
81-
<div className="p-8 text-red-600">
82-
Application <code>{name}</code> not found.
84+
<div className="flex items-center gap-2 p-8 text-slate-500">
85+
<Spinner /> Loading…
8386
</div>
8487
)
8588
}

0 commit comments

Comments
 (0)