From 7f55ff5d85fdffd56fdf3f896b092b041a9ae900 Mon Sep 17 00:00:00 2001 From: Myasnikov Daniil Date: Fri, 30 Oct 2026 16:24:54 +0500 Subject: [PATCH] fix(dashboard): link a tenant row to its detail page The tenant list rendered the name as plain text and put the row's only link on an Edit button, so nothing in the list reached the detail page that every other kind gets, tabs and a Delete action included. The only way there was to open the edit form and cancel out of it. The name cell becomes a link wherever Edit is offered, to the same CR. The detail page reads the namespace from the tenant context, so a plain click aligns the context to the row's parent first, the way Edit already does. React runs that handler for a ctrl-, cmd- or shift-click too, which the browser turns into a new tab or window, so it runs only for the click that navigates its own tab; otherwise opening a tenant beside the current one would move the tab left behind as well. Assisted-by: LLM Signed-off-by: Myasnikov Daniil --- .../apps/console/src/lib/links.test.ts | 33 ++++++++ .../console/apps/console/src/lib/links.ts | 13 ++++ .../console/src/routes/TenantsPage.test.tsx | 75 ++++++++++++++++++- .../apps/console/src/routes/TenantsPage.tsx | 49 +++++++++--- 4 files changed, 158 insertions(+), 12 deletions(-) create mode 100644 packages/system/dashboard/images/console/apps/console/src/lib/links.test.ts create mode 100644 packages/system/dashboard/images/console/apps/console/src/lib/links.ts diff --git a/packages/system/dashboard/images/console/apps/console/src/lib/links.test.ts b/packages/system/dashboard/images/console/apps/console/src/lib/links.test.ts new file mode 100644 index 000000000..84ae4ed2a --- /dev/null +++ b/packages/system/dashboard/images/console/apps/console/src/lib/links.test.ts @@ -0,0 +1,33 @@ +import { describe, it, expect } from "vitest" +import type { MouseEvent } from "react" +import { navigatesThisTab } from "./links.ts" + +function click(overrides: Partial = {}): MouseEvent { + return { + button: 0, + metaKey: false, + altKey: false, + ctrlKey: false, + shiftKey: false, + ...overrides, + } as MouseEvent +} + +describe("navigatesThisTab", () => { + it("accepts a plain primary click", () => { + expect(navigatesThisTab(click())).toBe(true) + }) + + // Each of these hands the navigation to a new tab or window, so the tab the + // click happened in keeps whatever tenant it was showing. + it.each(["metaKey", "ctrlKey", "shiftKey", "altKey"] as const)( + "refuses a click modified with %s", + (modifier) => { + expect(navigatesThisTab(click({ [modifier]: true }))).toBe(false) + }, + ) + + it("refuses a non-primary button", () => { + expect(navigatesThisTab(click({ button: 1 }))).toBe(false) + }) +}) diff --git a/packages/system/dashboard/images/console/apps/console/src/lib/links.ts b/packages/system/dashboard/images/console/apps/console/src/lib/links.ts new file mode 100644 index 000000000..c6dc28599 --- /dev/null +++ b/packages/system/dashboard/images/console/apps/console/src/lib/links.ts @@ -0,0 +1,13 @@ +import type { MouseEvent } from "react" + +/** + * Whether a click on an anchor will navigate the tab it happened in, rather + * than open a new tab or window. React runs `onClick` either way, and a + * cross-tenant link's handler aligns the tenant context of the tab it is + * leaving — which is wrong when that tab is staying where it is. Same button + * and modifier test react-router's own `Link` applies before it intercepts a + * click, minus the `target` check these links have no attribute for. + */ +export function navigatesThisTab(e: MouseEvent): boolean { + return e.button === 0 && !e.metaKey && !e.altKey && !e.ctrlKey && !e.shiftKey +} diff --git a/packages/system/dashboard/images/console/apps/console/src/routes/TenantsPage.test.tsx b/packages/system/dashboard/images/console/apps/console/src/routes/TenantsPage.test.tsx index 0f7d35c14..fe2864829 100644 --- a/packages/system/dashboard/images/console/apps/console/src/routes/TenantsPage.test.tsx +++ b/packages/system/dashboard/images/console/apps/console/src/routes/TenantsPage.test.tsx @@ -1,5 +1,5 @@ import { describe, it, expect, vi, beforeAll } from "vitest" -import { screen } from "@testing-library/react" +import { fireEvent, screen } from "@testing-library/react" import userEvent from "@testing-library/user-event" import { useLocation } from "react-router" import { @@ -9,6 +9,7 @@ import { } from "@cozystack/k8s-client" import { TenantsPage } from "./TenantsPage.tsx" import { TenantProvider } from "../lib/tenant-provider.tsx" +import { useTenantContext } from "../lib/tenant-context.tsx" import { SELECTED_TENANT_KEY } from "../lib/constants.ts" import { renderWithK8sProvider } from "../test-utils/render.tsx" @@ -36,6 +37,10 @@ function LocationProbe() { return
{pathname}
} +function SelectedTenant() { + return {useTenantContext().selectedTenant} +} + function makeClient(tenants: unknown[] = VISIBLE): K8sClient { const client = new K8sClient() vi.spyOn(client, "list").mockImplementation(async (_g, _v, plural) => { @@ -95,6 +100,12 @@ describe("TenantsPage tree", () => { expect(editButtons).toHaveLength(1) expect(screen.getByTitle("Edit tenant whmcs-x")).toBeInTheDocument() expect(screen.queryByTitle("Edit tenant whmcs-a-b")).not.toBeInTheDocument() + + // The name links to the detail page, where Delete is, exactly where Edit is + // offered: a row whose CR namespace cannot be read stays plain text. + expect(screen.getByRole("link", { name: "x" })).toHaveAttribute("href", "/admin/tenants/x") + expect(screen.queryByRole("link", { name: "a-b" })).toBeNull() + expect(screen.queryByRole("link", { name: "whmcs" })).toBeNull() }) it("collapses a subtree via the row toggle", async () => { @@ -135,6 +146,7 @@ function renderTenants(tenants: unknown[], selected: string) { + , { client: makeClient(tenants), initialRoute: "/admin/tenants" }, ) @@ -181,3 +193,64 @@ describe("TenantsPage self-referential root", () => { expect(screen.getByTestId("location")).toHaveTextContent("/admin/tenants/whmcs/edit") }) }) + +describe("TenantsPage row link", () => { + it("links every row whose Tenant CR it can name", async () => { + renderTenants(ROOT_VISIBLE, "root") + + // The root's CR is self-referential: `root` inside its own namespace. + expect(await screen.findByRole("link", { name: "root" })).toHaveAttribute( + "href", + "/admin/tenants/root", + ) + expect(screen.getByRole("link", { name: "acme" })).toHaveAttribute("href", "/admin/tenants/acme") + expect(screen.getByRole("link", { name: "eu" })).toHaveAttribute("href", "/admin/tenants/eu") + expect(screen.getByRole("link", { name: "root-x" })).toHaveAttribute( + "href", + "/admin/tenants/root-x", + ) + }) + + it("links a self-referential root to its own CR", async () => { + renderTenants([tn("tenant-whmcs", [])], "whmcs") + + expect(await screen.findByRole("link", { name: "whmcs" })).toHaveAttribute( + "href", + "/admin/tenants/whmcs", + ) + }) + + // The detail page resolves `eu` in the selected tenant's namespace, and eu's + // CR lives in acme's, so the click has to land the tab on acme. + it("switches to the row's parent and opens the detail page", async () => { + renderTenants(ROOT_VISIBLE, "root") + + fireEvent.click(await screen.findByRole("link", { name: "eu" })) + + expect(screen.getByTestId("selected-tenant").textContent).toBe("acme") + expect(screen.getByTestId("location")).toHaveTextContent("/admin/tenants/eu") + }) + + // Edit shares the route with the link, and its form resolves the CR the same + // way, so it has to land on the parent too. + it("switches to the row's parent before opening the edit form", async () => { + renderTenants(ROOT_VISIBLE, "root") + + await userEvent.click(await screen.findByTitle("Edit tenant acme-eu")) + + expect(screen.getByTestId("selected-tenant").textContent).toBe("acme") + }) + + // A ctrl-click opens a new tab; the tab it happened in stays where it is. + it("leaves the current tab alone on a ctrl-click", async () => { + renderTenants(ROOT_VISIBLE, "root") + const link = await screen.findByRole("link", { name: "eu" }) + // jsdom cannot open the new tab; the handler under test has run by then. + link.addEventListener("click", (e) => e.preventDefault()) + + fireEvent.click(link, { ctrlKey: true }) + + expect(screen.getByTestId("selected-tenant").textContent).toBe("root") + expect(screen.getByTestId("location")).toHaveTextContent(/^\/admin\/tenants$/) + }) +}) diff --git a/packages/system/dashboard/images/console/apps/console/src/routes/TenantsPage.tsx b/packages/system/dashboard/images/console/apps/console/src/routes/TenantsPage.tsx index 7eae5e45c..3944467f4 100644 --- a/packages/system/dashboard/images/console/apps/console/src/routes/TenantsPage.tsx +++ b/packages/system/dashboard/images/console/apps/console/src/routes/TenantsPage.tsx @@ -1,5 +1,5 @@ import { useMemo, useState } from "react" -import { useNavigate } from "react-router" +import { Link, useNavigate } from "react-router" import { Plus, Edit, Info, ChevronDown, ChevronRight, CornerDownRight } from "lucide-react" import { Spinner, Section, Button } from "@cozystack/ui" import { useK8sList } from "@cozystack/k8s-client" @@ -14,6 +14,7 @@ import { type TenantTreeNode, } from "../lib/tenant-tree.ts" import { ROOT_TENANT_NAMESPACE, TENANT_NAMESPACE_PREFIX } from "../lib/constants.ts" +import { navigatesThisTab } from "../lib/links.ts" import { formatAge } from "../lib/status.ts" import { TenantQuotaCompact } from "../components/QuotaDisplay.tsx" import type { ResourceQuota } from "../components/QuotaDisplay.tsx" @@ -131,8 +132,8 @@ export function TenantsPage() { // A node's Tenant CR lives in its REAL parent's namespace, which is not // necessarily the parent it hangs under: a node whose real parent is // inaccessible is bridged onto the nearest visible ancestor. Deriving the CR - // from that bridged ancestor names a CR that does not exist, so the edit - // target comes from realParentNamespace instead — and is offered only when + // from that bridged ancestor names a CR that does not exist, so Tenant CR + // routes come from realParentNamespace instead — and are offered only when // that namespace is actually readable, since otherwise there is no CR the // user could open. The hierarchy root (no ancestor labels at all) is // self-referential: its CR (`root`) lives in its own namespace. @@ -143,12 +144,20 @@ export function TenantsPage() { const real = realParentNamespace(node.tn) return real && visibleNs.has(real) ? real : undefined } - const canEdit = (node: TreeNode) => !!editParentNs(node) - const editNode = (node: TreeNode) => { + const tenantRoute = (node: TreeNode) => { const parentNs = editParentNs(node) - if (!parentNs) return - selectTenant(parentNs.slice(TENANT_NAMESPACE_PREFIX.length)) - navigate(`${basePath}/tenants/${tenantCrName(node.tn.metadata.name, parentNs)}/edit`) + if (!parentNs) return undefined + return { + parentTenant: parentNs.slice(TENANT_NAMESPACE_PREFIX.length), + path: `${basePath}/tenants/${tenantCrName(node.tn.metadata.name, parentNs)}`, + } + } + const canEdit = (node: TreeNode) => !!tenantRoute(node) + const editNode = (node: TreeNode) => { + const route = tenantRoute(node) + if (!route) return + selectTenant(route.parentTenant) + navigate(`${route.path}/edit`) } return ( @@ -189,6 +198,7 @@ export function TenantsPage() { const modules = modulesByNamespace.get(ns) ?? [] const tenantQuotas = quotasByNamespace.get(ns) ?? [] const host = node.tn.metadata.labels?.[HOST_LABEL] + const route = tenantRoute(node) return ( @@ -213,9 +223,26 @@ export function TenantsPage() { ) : null}
-

- {relativeTenantName(node)} -

+ {route ? ( + { + if (!navigatesThisTab(e)) return + selectTenant(route.parentTenant) + }} + className="block truncate text-sm font-medium text-slate-900 hover:underline" + > + {relativeTenantName(node)} + + ) : ( +

+ {relativeTenantName(node)} +

+ )}

{ns}