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 <myasnikovdaniil2001@gmail.com>

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(<App />, { 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(<App />, { 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(<App />, { 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(<App />, { 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(<App />, { client, initialRoute: "/console" })
 
-    expect(await screen.findByText("Tenant", { exact: true })).toBeTruthy()
+    expect(await screen.findByTestId("tenant-picker")).toBeTruthy()
   })
 })

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()

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 <span className="text-slate-400">Loading tenants…</span>
+    return (
+      <span data-testid="tenant-picker" className="text-slate-400">
+        Loading tenants…
+      </span>
+    )
   }
   if (!tenants.length) {
-    return <span className="text-amber-600">No tenants found</span>
+    return (
+      <span data-testid="tenant-picker" className="text-amber-600">
+        No tenants found
+      </span>
+    )
   }
 
   return (
-    <span className="inline-flex items-center gap-1.5">
+    <span data-testid="tenant-picker" className="inline-flex items-center gap-1.5">
       <span className="text-slate-400">Tenant</span>
       <span className="text-slate-400">/</span>
       {tenants.length > 1 ? (

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> = {}): 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(<SourceField {...props()} />, {
+      client: createMockK8sClient({ lists: [] }),
+    })
+
+    fireEvent.click(screen.getByRole("radio", { name: /http/i }))
+
+    expect(document.getElementById("root_source_http_url")).not.toBeNull()
+  })
+
+})

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 (
             <div key={key} className="flex flex-col gap-1">
-              <label className="text-sm font-medium text-slate-700">
+              <label htmlFor={fieldId} className="text-sm font-medium text-slate-700">
                 {subProp.title || key}
                 {prop.required?.includes(key) && <span className="text-red-500 ml-1">*</span>}
               </label>
@@ -133,6 +138,7 @@ export function SourceField(props: FieldProps) {
               )}
               {isDiskName ? (
                 <select
+                  id={fieldId}
                   value={currentValue}
                   onChange={(e) => handleChange(e.target.value)}
                   disabled={disksLoading}
@@ -153,6 +159,7 @@ export function SourceField(props: FieldProps) {
                 </select>
               ) : isImageName ? (
                 <select
+                  id={fieldId}
                   value={currentValue}
                   onChange={(e) => handleChange(e.target.value)}
                   disabled={imagesLoading}
@@ -171,6 +178,7 @@ export function SourceField(props: FieldProps) {
                 </select>
               ) : (
                 <input
+                  id={fieldId}
                   type="text"
                   value={currentValue}
                   onChange={(e) => handleChange(e.target.value)}

packages/system/dashboard/images/console/apps/console/src/lib/focus-first-error.test.ts

@@ -66,15 +66,27 @@ describe("focusFirstError", () => {
     expect(document.activeElement).toBe(field)
   })
 
-  // SourceField and SourceWidget render no element carrying the generated id,
-  // only radios named `<id>-source`, so the exact lookup misses and a blocked
-  // submit used to scroll nowhere.
-  it("falls back to the name attribute when no element carries the id", () => {
+  // vm-disk marks source.http.url required, not source, so this is the shape
+  // validation actually reports for a SourceField. The widget sets the id on
+  // its own control; the exact lookup is what has to find it.
+  it("focuses a SourceField sub-control by its generated id", () => {
     document.body.innerHTML = `
-      <div id="root_source-help" class="field">
-        <input type="radio" name="root_source-source" value="http" />
-        <input type="radio" name="root_source-source" value="pvc" />
-      </div>
+      <input type="radio" name="root_source-source" value="http" />
+      <input id="root_source_http_url" />
+    `
+    const field = document.getElementById("root_source_http_url")
+
+    focusFirstError({ property: ".source.http.url" })
+
+    expect(document.activeElement).toBe(field)
+  })
+
+  // A radio group carries only `name="<id>-source"`, so the name half of the
+  // fallback is the only thing that can reach it.
+  it("falls back to the name attribute for a radio group with no id", () => {
+    document.body.innerHTML = `
+      <input type="radio" name="root_source-source" value="http" />
+      <input type="radio" name="root_source-source" value="pvc" />
     `
     const first = document.querySelector<HTMLInputElement>('input[value="http"]')
 

packages/system/dashboard/images/console/apps/console/src/lib/focus-first-error.ts

@@ -20,11 +20,13 @@ function propertySegments(property: string): string[] {
  * focus resolves the field through `form.elements`, which the `tagName="div"`
  * form does not have, so resolve it by generated id instead.
  *
- * Not every widget puts that id on an element. `SourceField` and
- * `SourceWidget` render radios carrying only `name="<id>-source"`, so an exact
- * lookup finds nothing and the submit scrolls nowhere. Fall back to a
- * descendant input, matched on a segment boundary: a bare prefix would let an
- * error on `data` grab the sibling `database`, scrolling to the wrong field.
+ * Not every widget puts that id on a control, so fall back to a descendant,
+ * matched on a segment boundary: a bare prefix would let an error on `data`
+ * grab the sibling `database`, scrolling to the wrong field. The `name` half
+ * catches a radio group, which carries `name="<id>-source"` and no id.
+ *
+ * A widget that renders neither is unreachable, which is why the widgets this
+ * console binds set the generated id themselves rather than relying on this.
  */
 export function focusFirstError(
   error: Pick<RJSFValidationError, "property">,