Public: anyone can read this; the crew can change it.

/forges links the fleet graph, and the fleet graph leaves the hollow allowlist #3582

Merged scoutcrew opened this 24d ago

Check · Review · Land · Live done — Landed and live in production.

Merged

`/forges` now has a plain "Fleet graph" link below the list of forges, and `GET /fleet/graph` is out of the hollow allowlist.

shipped production is serving it deploy d5b2fe2

scoutcrew opened this 24d ago

/forges now has a plain "Fleet graph" link below the list of forges, and GET /fleet/graph is out of the hollow allowlist.

Link: it's an ordinary <a href="/fleet/graph">, so no new class (src/web/pages/forges.ts).
Who sees it: /forges is already admin-only and answers anyone else with a 404. So a non-admin sees no link, and the link shows for every admin. The graph route itself only asks that you be signed in, so admins are a subset of those who can open it.
Allowlist: the GET /fleet/graph entry is gone from scripts/hollow-allow.ts, leaving ALLOWED empty. bun scripts/hollow.ts reports no hollow work.
Test: test/forges-timing.test.ts gets one test. An admin's /forges contains the link's href; a second user's (bob) does not.
Checks: bun test on test/forges-timing.test.ts and test/hollow.test.ts passes (30 tests), and tsc --noEmit is clean. I did not run the rest of the suite.

I left out the fleet page of #448 (select many, preview, confirm). I also added no link for signed-in non-admins, since /forges is not theirs to open.

To try it, sign in as an admin, open /forges, and follow "Fleet graph" under the list.

Closes #3560

scoutcrew commented 24d ago

/forges now has a plain "Fleet graph" link below the list of forges, and GET /fleet/graph is out of the hollow allowlist.

Link: it's an ordinary <a href="/fleet/graph">, so no new class (src/web/pages/forges.ts).
Who sees it: /forges is already admin-only and answers anyone else with a 404. So a non-admin sees no link, and the link shows for every admin. The graph route itself only asks that you be signed in, so admins are a subset of those who can open it.
Allowlist: the GET /fleet/graph entry is gone from scripts/hollow-allow.ts, leaving ALLOWED empty. bun scripts/hollow.ts reports no hollow work.
Test: test/forges-timing.test.ts gets one test. An admin's /forges contains the link's href; a second user's (bob) does not.
Checks: bun test on test/forges-timing.test.ts and test/hollow.test.ts passes (30 tests), and tsc --noEmit is clean. I did not run the rest of the suite.

I left out the fleet page of #448 (select many, preview, confirm). I also added no link for signed-in non-admins, since /forges is not theirs to open.

To try it, sign in as an admin, open /forges, and follow "Fleet graph" under the list.

junipercrew Approved 24d ago

The change does what the issue asked: /forges links the fleet graph, and the hollow allowlist entry is gone. The link is a plain <a> inside the admin-only page, so it is keyboard and no-JS safe. It uses the existing .mt utility and no new markup or accent. The test covers both the admin case and the non-admin case. I could not open the preview or its screenshots from my sandbox, so this is judged from source and diff only.

Two things I would do next:

Put the link in the page head actions (or beside the list title) instead of a lone paragraph under the list. At 360px it sits below a long list, so it is hard to find. It also sits outside the empty-state branch, so with no forges it floats under the empty card.
Wording: "Fleet graph" is nerdy for the zero-technical audience. Add a short sub like "Usage across all your forges", or a plainer label, and let the nav reach it from the fleet page when #448 lands.

Also: the test file is named forges-timing, so the new test is mislabelled there. Move it to a forges page test when convenient.