From b504e41bf30370a401d25c93c608b6faa5050b79 Mon Sep 17 00:00:00 2001 From: Myasnikov Daniil Date: Mon, 31 Aug 2026 18:54:05 +0500 Subject: [PATCH] fix(dashboard): focus SourceField's own controls, and keep the picker on the tenant list The focus helper named SourceField as the widget it existed for and its test pinned `.source`, but vm-disk marks source.http.url, source.disk.name and source.image.name required and source itself not, so `.source` is a property RJSF cannot report. What it does report is one level deeper, where SourceField's controls carried neither id nor name, so choosing the http source, leaving the URL empty and pressing Deploy blocked the submit and then did nothing at all. Give those controls the generated id, label them with it, and test the shape the shipped schema actually produces. /admin/tenants is not cluster-scoped: its rows come from the picker-filtered context list, unlike Modules and External IPs which list tenant namespaces themselves. Hiding the picker there stranded the tree rooted at whichever child the user had drilled into, with no control to widen it, and the unanchored prefix took the row detail and edit routes with it. Drop it from the list and anchor the match on a segment boundary. The picker now carries a test marker on every branch, because asserting on its label could not distinguish "not rendered" from "still loading" and collided with the tenant page's own Tenant button -- which is why that page went untested while both defects sat in it. Assisted-by: LLM Signed-off-by: Myasnikov Daniil --- .../apps/console/src/App.routing.test.tsx | 42 ++++++++++------ .../images/console/apps/console/src/App.tsx | 11 +++-- .../console/src/components/Breadcrumb.tsx | 14 ++++-- .../src/components/SourceField.test.tsx | 49 ++++++++++++++++++- .../console/src/components/SourceField.tsx | 10 +++- .../console/src/lib/focus-first-error.test.ts | 28 ++++++++--- .../apps/console/src/lib/focus-first-error.ts | 12 +++-- 7 files changed, 129 insertions(+), 37 deletions(-) diff --git a/packages/system/dashboard/images/console/apps/console/src/App.routing.test.tsx b/packages/system/dashboard/images/console/apps/console/src/App.routing.test.tsx index 326b61b9d..4081ecf5b 100644 --- a/packages/system/dashboard/images/console/apps/console/src/App.routing.test.tsx +++ b/packages/system/dashboard/images/console/apps/console/src/App.routing.test.tsx @@ -124,31 +124,41 @@ describe("shell subtitle", () => { metadata: { name, labels: {} }, })) - it("hides the tenant picker in the admin portal", async () => { - const client = makeClient(TENANTS) - renderWithK8sProvider(, { client, initialRoute: "/admin/modules" }) + // Every cluster-scoped prefix, and a deeper path under each, so dropping one + // from the list fails here instead of passing quietly. + const CLUSTER_SCOPED = [ + "/admin/capacity/cluster", + "/admin/backups/backupclasses", + "/admin/external-ips", + "/admin/modules", + ] - // The picker's first render says "Loading tenants…"; wait that out, or the - // absence below passes only because the list had not arrived yet. + it.each(CLUSTER_SCOPED)("hides the tenant picker on %s", async (route) => { + const client = makeClient(TENANTS) + renderWithK8sProvider(, { client, initialRoute: route }) + + // The marker is on every Breadcrumb branch, so absence means the subtitle + // was not rendered at all rather than that the list had not arrived. await waitFor(() => expect(screen.queryByText("Loading tenants…")).toBeNull()) - expect(screen.queryByText("Tenant", { exact: true })).toBeNull() + expect(screen.queryByTestId("tenant-picker")).toBeNull() }) - // The rest of /admin mounts the same tenant-scoped resource pages as - // /console, and they resolve their namespace from the tenant context, so the - // picker has to stay: hiding it there leaves the active tenant invisible and - // unchangeable on the page it actually governs. - it("keeps the tenant picker on tenant-scoped admin pages", async () => { - const client = makeClient(TENANTS) - renderWithK8sProvider(, { client, initialRoute: "/admin/virtualmachines" }) + // The tenant list is scoped by the picker's own selection, so it needs the + // picker even though it lives under /admin. + it.each(["/admin/tenants", "/admin/vminstances", "/admin/vminstances/db1"])( + "keeps the tenant picker on %s", + async (route) => { + const client = makeClient(TENANTS) + renderWithK8sProvider(, { client, initialRoute: route }) - expect(await screen.findByText("Tenant", { exact: true })).toBeTruthy() - }) + expect(await screen.findByTestId("tenant-picker")).toBeTruthy() + }, + ) it("still shows the tenant picker outside the admin portal", async () => { const client = makeClient(TENANTS) renderWithK8sProvider(, { client, initialRoute: "/console" }) - expect(await screen.findByText("Tenant", { exact: true })).toBeTruthy() + expect(await screen.findByTestId("tenant-picker")).toBeTruthy() }) }) diff --git a/packages/system/dashboard/images/console/apps/console/src/App.tsx b/packages/system/dashboard/images/console/apps/console/src/App.tsx index a88a07257..c800cb287 100644 --- a/packages/system/dashboard/images/console/apps/console/src/App.tsx +++ b/packages/system/dashboard/images/console/apps/console/src/App.tsx @@ -18,11 +18,14 @@ import { CommandPalette } from "./components/command-palette/command-palette.tsx import { AppConfigContext, useAppConfig, type AppConfig } from "./lib/config.ts" import { DEFAULT_LANDING_PATH } from "./lib/portal.ts" +// Admin pages whose content does not depend on the selected tenant, so the +// picker has nothing to say on them. /admin/tenants is deliberately absent: +// its rows come from the picker-filtered context list, unlike Modules and +// External IPs which list tenant namespaces themselves. const CLUSTER_SCOPED_ADMIN = [ "/admin/capacity", "/admin/backups/backupclasses", "/admin/external-ips", - "/admin/tenants", "/admin/modules", ] @@ -36,8 +39,10 @@ function Shell({ username }: { username?: string }) { // (see lib/portal.ts), and those resolve their namespace from the tenant // context, so hiding the picker there would leave the active tenant both // invisible and unchangeable. - const inClusterScope = CLUSTER_SCOPED_ADMIN.some((prefix) => - pathname.startsWith(prefix), + // Anchored on a segment boundary: an unanchored prefix would also swallow a + // sibling route that merely starts with the same characters. + const inClusterScope = CLUSTER_SCOPED_ADMIN.some( + (prefix) => pathname === prefix || pathname.startsWith(`${prefix}/`), ) const marketplaceSections = useMarketplaceSidebarSections() const consoleSections = useConsoleSidebarSections() diff --git a/packages/system/dashboard/images/console/apps/console/src/components/Breadcrumb.tsx b/packages/system/dashboard/images/console/apps/console/src/components/Breadcrumb.tsx index 8f6a8641f..15ad1f4fb 100644 --- a/packages/system/dashboard/images/console/apps/console/src/components/Breadcrumb.tsx +++ b/packages/system/dashboard/images/console/apps/console/src/components/Breadcrumb.tsx @@ -10,14 +10,22 @@ export function Breadcrumb() { const { tenants, selectedTenant, selectTenant, isLoading } = useTenantContext() if (isLoading && !tenants.length) { - return Loading tenants… + return ( + + Loading tenants… + + ) } if (!tenants.length) { - return No tenants found + return ( + + No tenants found + + ) } return ( - + Tenant / {tenants.length > 1 ? ( diff --git a/packages/system/dashboard/images/console/apps/console/src/components/SourceField.test.tsx b/packages/system/dashboard/images/console/apps/console/src/components/SourceField.test.tsx index 54d30361a..101060357 100644 --- a/packages/system/dashboard/images/console/apps/console/src/components/SourceField.test.tsx +++ b/packages/system/dashboard/images/console/apps/console/src/components/SourceField.test.tsx @@ -1,5 +1,5 @@ import { describe, it, expect, vi } from "vitest" -import { screen } from "@testing-library/react" +import { screen, fireEvent } from "@testing-library/react" import type { FieldProps } from "@rjsf/utils" import { SourceField } from "./SourceField.tsx" import { createMockK8sClient } from "../test-utils/mock-k8s-client.ts" @@ -81,3 +81,50 @@ describe("SourceField image picker", () => { expect(screen.queryByRole("option", { name: "ubuntu" })).not.toBeInTheDocument() }) }) + +// The shipped vm-disk shape: each branch marks its own leaf required, and +// `source` itself is not, so validation reports `.source.http.url`. +const sourceSchema = { + type: "object", + description: "The source image location used to create a disk.", + properties: { + http: { + type: "object", + required: ["url"], + properties: { url: { type: "string", title: "URL" } }, + }, + disk: { + type: "object", + required: ["name"], + properties: { name: { type: "string", title: "Name" } }, + }, + }, +} + +function props(overrides: Partial = {}): FieldProps { + return { + schema: sourceSchema, + formData: {}, + onChange: vi.fn(), + name: "source", + required: false, + idSchema: { $id: "root_source" }, + ...overrides, + } as unknown as FieldProps +} + +describe("SourceField", () => { + // Validation reports the branch leaf, and focus-first-error resolves the + // field by RJSF's generated id, so the control has to carry it or a blocked + // submit scrolls nowhere. Nothing else in the widget renders that id. + it("puts the generated id on the selected branch's control", () => { + renderWithK8sProvider(, { + client: createMockK8sClient({ lists: [] }), + }) + + fireEvent.click(screen.getByRole("radio", { name: /http/i })) + + expect(document.getElementById("root_source_http_url")).not.toBeNull() + }) + +}) diff --git a/packages/system/dashboard/images/console/apps/console/src/components/SourceField.tsx b/packages/system/dashboard/images/console/apps/console/src/components/SourceField.tsx index 3fe49c429..ed5a6dcef 100644 --- a/packages/system/dashboard/images/console/apps/console/src/components/SourceField.tsx +++ b/packages/system/dashboard/images/console/apps/console/src/components/SourceField.tsx @@ -1,6 +1,7 @@ import { useState } from "react" import type { FieldProps } from "@rjsf/utils" import { useK8sList } from "@cozystack/k8s-client" +import { RJSF_ID_SEPARATOR } from "../lib/rjsf-ids.ts" import { APPS_GROUP, APPS_VERSION } from "@cozystack/types" import { useTenantContext } from "../lib/tenant-context.tsx" import type { OptionObject } from "./DynamicOptionsWidget.tsx" @@ -121,10 +122,14 @@ export function SourceField(props: FieldProps) { const isDiskName = option === "disk" && key === "name" const isImageName = option === "image" && key === "name" + // RJSF's id for this control. Its required sub-fields are what + // validation reports (source.http.url, not source), so the focus + // helper resolves the field by this id and finds nothing without it. + const fieldId = [idSchema.$id, option, key].join(RJSF_ID_SEPARATOR) return (
-