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}