Skip to content

Commit 7f55ff5

Browse files
myasnikovdaniilmyasnikovdaniil
committed
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 <[email protected]>
1 parent e44e41d commit 7f55ff5

4 files changed

Lines changed: 158 additions & 12 deletions

File tree

‎packages/system/dashboard/images/console/apps/console/src/lib/links.test.ts‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
import { describe, it, expect } from "vitest"
2+
import type { MouseEvent } from "react"
3+
import { navigatesThisTab } from "./links.ts"
4+
5+
function click(overrides: Partial<MouseEvent> = {}): MouseEvent {
6+
return {
7+
button: 0,
8+
metaKey: false,
9+
altKey: false,
10+
ctrlKey: false,
11+
shiftKey: false,
12+
...overrides,
13+
} as MouseEvent
14+
}
15+
16+
describe("navigatesThisTab", () => {
17+
it("accepts a plain primary click", () => {
18+
expect(navigatesThisTab(click())).toBe(true)
19+
})
20+
21+
// Each of these hands the navigation to a new tab or window, so the tab the
22+
// click happened in keeps whatever tenant it was showing.
23+
it.each(["metaKey", "ctrlKey", "shiftKey", "altKey"] as const)(
24+
"refuses a click modified with %s",
25+
(modifier) => {
26+
expect(navigatesThisTab(click({ [modifier]: true }))).toBe(false)
27+
},
28+
)
29+
30+
it("refuses a non-primary button", () => {
31+
expect(navigatesThisTab(click({ button: 1 }))).toBe(false)
32+
})
33+
})

‎packages/system/dashboard/images/console/apps/console/src/lib/links.ts‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
import type { MouseEvent } from "react"
2+
3+
/**
4+
* Whether a click on an anchor will navigate the tab it happened in, rather
5+
* than open a new tab or window. React runs `onClick` either way, and a
6+
* cross-tenant link's handler aligns the tenant context of the tab it is
7+
* leaving — which is wrong when that tab is staying where it is. Same button
8+
* and modifier test react-router's own `Link` applies before it intercepts a
9+
* click, minus the `target` check these links have no attribute for.
10+
*/
11+
export function navigatesThisTab(e: MouseEvent): boolean {
12+
return e.button === 0 && !e.metaKey && !e.altKey && !e.ctrlKey && !e.shiftKey
13+
}

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

Lines changed: 74 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { describe, it, expect, vi, beforeAll } from "vitest"
2-
import { screen } from "@testing-library/react"
2+
import { fireEvent, screen } from "@testing-library/react"
33
import userEvent from "@testing-library/user-event"
44
import { useLocation } from "react-router"
55
import {
@@ -9,6 +9,7 @@ import {
99
} from "@cozystack/k8s-client"
1010
import { TenantsPage } from "./TenantsPage.tsx"
1111
import { TenantProvider } from "../lib/tenant-provider.tsx"
12+
import { useTenantContext } from "../lib/tenant-context.tsx"
1213
import { SELECTED_TENANT_KEY } from "../lib/constants.ts"
1314
import { renderWithK8sProvider } from "../test-utils/render.tsx"
1415

@@ -36,6 +37,10 @@ function LocationProbe() {
3637
return <div data-testid="location">{pathname}</div>
3738
}
3839

40+
function SelectedTenant() {
41+
return <output data-testid="selected-tenant">{useTenantContext().selectedTenant}</output>
42+
}
43+
3944
function makeClient(tenants: unknown[] = VISIBLE): K8sClient {
4045
const client = new K8sClient()
4146
vi.spyOn(client, "list").mockImplementation(async (_g, _v, plural) => {
@@ -95,6 +100,12 @@ describe("TenantsPage tree", () => {
95100
expect(editButtons).toHaveLength(1)
96101
expect(screen.getByTitle("Edit tenant whmcs-x")).toBeInTheDocument()
97102
expect(screen.queryByTitle("Edit tenant whmcs-a-b")).not.toBeInTheDocument()
103+
104+
// The name links to the detail page, where Delete is, exactly where Edit is
105+
// offered: a row whose CR namespace cannot be read stays plain text.
106+
expect(screen.getByRole("link", { name: "x" })).toHaveAttribute("href", "/admin/tenants/x")
107+
expect(screen.queryByRole("link", { name: "a-b" })).toBeNull()
108+
expect(screen.queryByRole("link", { name: "whmcs" })).toBeNull()
98109
})
99110

100111
it("collapses a subtree via the row toggle", async () => {
@@ -135,6 +146,7 @@ function renderTenants(tenants: unknown[], selected: string) {
135146
<TenantProvider>
136147
<TenantsPage />
137148
<LocationProbe />
149+
<SelectedTenant />
138150
</TenantProvider>,
139151
{ client: makeClient(tenants), initialRoute: "/admin/tenants" },
140152
)
@@ -181,3 +193,64 @@ describe("TenantsPage self-referential root", () => {
181193
expect(screen.getByTestId("location")).toHaveTextContent("/admin/tenants/whmcs/edit")
182194
})
183195
})
196+
197+
describe("TenantsPage row link", () => {
198+
it("links every row whose Tenant CR it can name", async () => {
199+
renderTenants(ROOT_VISIBLE, "root")
200+
201+
// The root's CR is self-referential: `root` inside its own namespace.
202+
expect(await screen.findByRole("link", { name: "root" })).toHaveAttribute(
203+
"href",
204+
"/admin/tenants/root",
205+
)
206+
expect(screen.getByRole("link", { name: "acme" })).toHaveAttribute("href", "/admin/tenants/acme")
207+
expect(screen.getByRole("link", { name: "eu" })).toHaveAttribute("href", "/admin/tenants/eu")
208+
expect(screen.getByRole("link", { name: "root-x" })).toHaveAttribute(
209+
"href",
210+
"/admin/tenants/root-x",
211+
)
212+
})
213+
214+
it("links a self-referential root to its own CR", async () => {
215+
renderTenants([tn("tenant-whmcs", [])], "whmcs")
216+
217+
expect(await screen.findByRole("link", { name: "whmcs" })).toHaveAttribute(
218+
"href",
219+
"/admin/tenants/whmcs",
220+
)
221+
})
222+
223+
// The detail page resolves `eu` in the selected tenant's namespace, and eu's
224+
// CR lives in acme's, so the click has to land the tab on acme.
225+
it("switches to the row's parent and opens the detail page", async () => {
226+
renderTenants(ROOT_VISIBLE, "root")
227+
228+
fireEvent.click(await screen.findByRole("link", { name: "eu" }))
229+
230+
expect(screen.getByTestId("selected-tenant").textContent).toBe("acme")
231+
expect(screen.getByTestId("location")).toHaveTextContent("/admin/tenants/eu")
232+
})
233+
234+
// Edit shares the route with the link, and its form resolves the CR the same
235+
// way, so it has to land on the parent too.
236+
it("switches to the row's parent before opening the edit form", async () => {
237+
renderTenants(ROOT_VISIBLE, "root")
238+
239+
await userEvent.click(await screen.findByTitle("Edit tenant acme-eu"))
240+
241+
expect(screen.getByTestId("selected-tenant").textContent).toBe("acme")
242+
})
243+
244+
// A ctrl-click opens a new tab; the tab it happened in stays where it is.
245+
it("leaves the current tab alone on a ctrl-click", async () => {
246+
renderTenants(ROOT_VISIBLE, "root")
247+
const link = await screen.findByRole("link", { name: "eu" })
248+
// jsdom cannot open the new tab; the handler under test has run by then.
249+
link.addEventListener("click", (e) => e.preventDefault())
250+
251+
fireEvent.click(link, { ctrlKey: true })
252+
253+
expect(screen.getByTestId("selected-tenant").textContent).toBe("root")
254+
expect(screen.getByTestId("location")).toHaveTextContent(/^\/admin\/tenants$/)
255+
})
256+
})

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

Lines changed: 38 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { useMemo, useState } from "react"
2-
import { useNavigate } from "react-router"
2+
import { Link, useNavigate } from "react-router"
33
import { Plus, Edit, Info, ChevronDown, ChevronRight, CornerDownRight } from "lucide-react"
44
import { Spinner, Section, Button } from "@cozystack/ui"
55
import { useK8sList } from "@cozystack/k8s-client"
@@ -14,6 +14,7 @@ import {
1414
type TenantTreeNode,
1515
} from "../lib/tenant-tree.ts"
1616
import { ROOT_TENANT_NAMESPACE, TENANT_NAMESPACE_PREFIX } from "../lib/constants.ts"
17+
import { navigatesThisTab } from "../lib/links.ts"
1718
import { formatAge } from "../lib/status.ts"
1819
import { TenantQuotaCompact } from "../components/QuotaDisplay.tsx"
1920
import type { ResourceQuota } from "../components/QuotaDisplay.tsx"
@@ -131,8 +132,8 @@ export function TenantsPage() {
131132
// A node's Tenant CR lives in its REAL parent's namespace, which is not
132133
// necessarily the parent it hangs under: a node whose real parent is
133134
// inaccessible is bridged onto the nearest visible ancestor. Deriving the CR
134-
// from that bridged ancestor names a CR that does not exist, so the edit
135-
// target comes from realParentNamespace instead — and is offered only when
135+
// from that bridged ancestor names a CR that does not exist, so Tenant CR
136+
// routes come from realParentNamespace instead — and are offered only when
136137
// that namespace is actually readable, since otherwise there is no CR the
137138
// user could open. The hierarchy root (no ancestor labels at all) is
138139
// self-referential: its CR (`root`) lives in its own namespace.
@@ -143,12 +144,20 @@ export function TenantsPage() {
143144
const real = realParentNamespace(node.tn)
144145
return real && visibleNs.has(real) ? real : undefined
145146
}
146-
const canEdit = (node: TreeNode) => !!editParentNs(node)
147-
const editNode = (node: TreeNode) => {
147+
const tenantRoute = (node: TreeNode) => {
148148
const parentNs = editParentNs(node)
149-
if (!parentNs) return
150-
selectTenant(parentNs.slice(TENANT_NAMESPACE_PREFIX.length))
151-
navigate(`${basePath}/tenants/${tenantCrName(node.tn.metadata.name, parentNs)}/edit`)
149+
if (!parentNs) return undefined
150+
return {
151+
parentTenant: parentNs.slice(TENANT_NAMESPACE_PREFIX.length),
152+
path: `${basePath}/tenants/${tenantCrName(node.tn.metadata.name, parentNs)}`,
153+
}
154+
}
155+
const canEdit = (node: TreeNode) => !!tenantRoute(node)
156+
const editNode = (node: TreeNode) => {
157+
const route = tenantRoute(node)
158+
if (!route) return
159+
selectTenant(route.parentTenant)
160+
navigate(`${route.path}/edit`)
152161
}
153162

154163
return (
@@ -189,6 +198,7 @@ export function TenantsPage() {
189198
const modules = modulesByNamespace.get(ns) ?? []
190199
const tenantQuotas = quotasByNamespace.get(ns) ?? []
191200
const host = node.tn.metadata.labels?.[HOST_LABEL]
201+
const route = tenantRoute(node)
192202
return (
193203
<tr key={ns} className="hover:bg-slate-50">
194204
<td className="px-4 py-3">
@@ -213,9 +223,26 @@ export function TenantsPage() {
213223
<CornerDownRight className="size-3.5 shrink-0 text-slate-300" />
214224
) : null}
215225
<div className="min-w-0">
216-
<p className="truncate text-sm font-medium text-slate-900">
217-
{relativeTenantName(node)}
218-
</p>
226+
{route ? (
227+
<Link
228+
to={route.path}
229+
// The detail page reads the namespace from the
230+
// tenant context, so the tab that navigates has
231+
// to be on the row's parent first; a ctrl-click
232+
// opens another tab and leaves this one alone.
233+
onClick={(e) => {
234+
if (!navigatesThisTab(e)) return
235+
selectTenant(route.parentTenant)
236+
}}
237+
className="block truncate text-sm font-medium text-slate-900 hover:underline"
238+
>
239+
{relativeTenantName(node)}
240+
</Link>
241+
) : (
242+
<p className="truncate text-sm font-medium text-slate-900">
243+
{relativeTenantName(node)}
244+
</p>
245+
)}
219246
<p className="truncate font-mono text-[11px] text-slate-400">
220247
{ns}
221248
</p>

0 commit comments

Comments
 (0)