diff --git a/AGENTS.md b/AGENTS.md index de1254c..4baa587 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -109,15 +109,17 @@ them. Revisit only if the stated reason stops holding. — keys, string-vs-plural kind, and the plural categories `Intl.PluralRules` says that locale needs — and a mismatch stops startup, same fail-loud contract as a bad manifest. A plugin may ship fewer locales than the host (its strings fall back to `en-US` per key), never one the host lacks. -- **The core building blocks carry the locale; a plugin doesn't have to.** `pagination`, - `filter-bar`, `data-table`, `auth-card`, `flow-body`, `menu` and the nav wrap every href they - render in `localeHref`, and the two GET forms carry it as a hidden `locale` input (a GET submit - replaces the whole query string, so no href wrapper can reach it). Putting the obligation on each - call site was tried first and missed five of eight sites inside one commit — including the admin - screens. `ctx.localeHref` remains for hrefs a plugin's own markup emits. Decided 2026-08-03 after - an architecture review. -- **`locale` is a host-owned query param.** It is in `parseListQuery`'s reserved set, so it never - shows up as a plugin filter; the i18n view locals (`t`, `locale`, `locales`, `localeHref`, +- **The core building blocks carry the locale; a plugin doesn't have to.** The shell (breadcrumbs), + `pagination`, `filter-bar`, `data-table`, `auth-card`, `flow-body`, `field` and `menu` wrap every + href they render in `localeHref`; the nav is wrapped upstream in `chrome.ts`; and the two GET forms + (filter bar, rows-per-page) carry it as a hidden `locale` input, since a GET submit replaces the + whole query string and no href wrapper can reach it. Putting the obligation on each call site was + tried first and missed five of eight sites inside one commit — including the admin screens. + `ctx.localeHref` remains for hrefs a plugin's own markup emits (the admin example's delete links). + Decided 2026-08-03 after an architecture review; a second pass then found breadcrumbs still raw, + so: when a link renders from the core chrome, it is the chrome's job to carry the locale. +- **`locale` is a host-owned query param.** It is in `parseListQuery`'s reserved set (`list-query.ts`), + so a localized list page doesn't hand a plugin a phantom `locale` filter; the i18n view locals (`t`, `locale`, `locales`, `localeHref`, `localeParam`, `localeSwitch`, `dir`) are likewise reserved names, merged after a handler's `data` so a collision loses the key instead of breaking the shell. - **`locales/` at the repo root is a drop-in mount, like `plugins/` and `config/`.** A catalog there @@ -223,6 +225,11 @@ Same test before adding a row to a table or the file map — a clause, not a par that re-parses `ctx.url.pathname`: it duplicates the URL shape, ignores the router's params, and has to re-handle HEAD. Factor shared per-request setup (auth gate, `ctx.system` capability resolution, target fetch) into a small `withX` wrapper — see `examples/plugins/admin/`. +- **`handleRequest` (`src/http/app.ts`) is a known complexity hotspot** — ~160 lines tracking + canonical host, static, locale, session + re-mint, CSRF, chrome, hooks, plugin routing, builtin + routing, 405/404 and error mapping. The pure parts are already extracted and separately tested; what + remains is orchestration. Planned split along those seams; don't grow it further without taking one + out. Raised by the architecture review 2026-08-03, deliberately not done inside the i18n change. - Reviews are maintainer-triggered (e.g. via the larv-review skill) — never auto-run reviewer agents. Decided 2026-08-02, replacing the earlier run-after-every-implementation rule. - **A user-visible string belongs in a catalog, not in the code or a view.** Core strings go in diff --git a/README.md b/README.md index 7e85206..1679284 100644 --- a/README.md +++ b/README.md @@ -998,10 +998,12 @@ Three rules worth knowing: that deliberately carries markup is rendered with `<%- %>` — and must never interpolate untrusted data, since nothing escapes it there. - **Dates and numbers are `Intl`'s job**, not the catalog's: `new Intl.DateTimeFormat(ctx.locale)`. -- **The core building blocks carry the locale for you.** `pagination`, `filter-bar`, `data-table`, - `auth-card` and the nav wrap every href they render, and the two GET forms carry it as a hidden - `locale` input (a GET submit replaces the whole query string). `ctx.localeHref` is only for hrefs - your own markup emits — and `localeParam` (a view local: the tag, or null) for your own GET forms. +- **The core building blocks carry the locale for you.** The shell's breadcrumbs, `pagination`, + `filter-bar`, `data-table`, `auth-card`, `field` and the nav wrap every href they render, and the + two GET forms (filter bar, rows-per-page) carry it as a hidden `locale` input — a GET submit + replaces the whole query string, so no href wrapper can reach it. `ctx.localeHref` is only for + hrefs your own markup emits, and `localeParam` (a view local: the tag, or null) for your own GET + forms. `locale` is reserved: `parseListQuery` never returns it as a filter. - **Reuse the core words.** Generic UI verbs live in the core catalog — `common.add/cancel/delete/ edit/new/remove/save`, `filter.*`, `pagination.*`, `table.*` — and a plugin's lookup falls through to them. Keep your catalog for your domain words, so N plugins don't re-translate "Cancel" N times. diff --git a/e2e-tests/full-flow.spec.ts b/e2e-tests/full-flow.spec.ts index ebb3edb..bab89a0 100644 --- a/e2e-tests/full-flow.spec.ts +++ b/e2e-tests/full-flow.spec.ts @@ -67,6 +67,11 @@ test.describe.serial("authenticated admin journey", () => { await page.getByRole("button", { name: "Visa" }).click(); // the rows-per-page GET form await expect(page).toHaveURL(/locale=sv-SE/); await expect(page.locator("html")).toHaveAttribute("lang", "sv-SE"); + + // The breadcrumb is the chrome's way back up — it is rendered by the shell, not by the screen. + await page.getByRole("navigation", { name: "Sidsökväg" }).getByRole("link").first().click(); + await expect(page).toHaveURL(/locale=sv-SE/); + await expect(page.locator("html")).toHaveAttribute("lang", "sv-SE"); }); test("menu filters by permission: an admin sees the gated Admin section + the plugin", async () => { diff --git a/examples/plugins/admin/i18n/en-US.ts b/examples/plugins/admin/i18n/en-US.ts index 800e371..ede0f3c 100644 --- a/examples/plugins/admin/i18n/en-US.ts +++ b/examples/plugins/admin/i18n/en-US.ts @@ -81,6 +81,10 @@ const messages = { "admin.notFound.message": "That item doesn't exist.", "admin.notFound.title": "Not found", + "admin.permissions.actions": "Permission actions", + "admin.permissions.allAssigned": "All users and groups already have this permission.", + "admin.permissions.assign": "Assign the permission", + "admin.permissions.assignAction": "Assign", "admin.permissions.assignTo": "Assign to", "admin.permissions.assignedTo": "Assigned to", "admin.permissions.column.members": "Members", @@ -90,10 +94,13 @@ const messages = { "admin.permissions.deleteMessage": "Delete permission {{name}}? This revokes it from everyone it's assigned to.", "admin.permissions.error.adminUndeletable": "The admin permission can't be deleted — it would remove all admin access.", "admin.permissions.error.selfRevoke": "You can't revoke your own admin access.", + "admin.permissions.effective": "Effective access", + "admin.permissions.effectiveHint": "Everyone who holds this permission — directly or through a group (resolved by Keto).", "admin.permissions.field.name": "Permission name", "admin.permissions.field.nameHint": "Lowercase letters, digits, dashes and underscores.", "admin.permissions.filter": "Filter permissions", "admin.permissions.new": "New permission", + "admin.permissions.noEffective": "No users hold this permission yet.", "admin.permissions.noMembers": "Not assigned to anyone yet.", "admin.permissions.pagination": "Permissions pagination", "admin.permissions.revoke": "Revoke", diff --git a/examples/plugins/admin/i18n/sv-SE.ts b/examples/plugins/admin/i18n/sv-SE.ts index 819036f..349e819 100644 --- a/examples/plugins/admin/i18n/sv-SE.ts +++ b/examples/plugins/admin/i18n/sv-SE.ts @@ -81,6 +81,10 @@ const messages: AdminMessages = { "admin.notFound.message": "Objektet finns inte.", "admin.notFound.title": "Hittades inte", + "admin.permissions.actions": "Behörighetsåtgärder", + "admin.permissions.allAssigned": "Alla användare och grupper har redan den här behörigheten.", + "admin.permissions.assign": "Tilldela behörigheten", + "admin.permissions.assignAction": "Tilldela", "admin.permissions.assignTo": "Tilldela till", "admin.permissions.assignedTo": "Tilldelad till", "admin.permissions.column.members": "Medlemmar", @@ -90,10 +94,13 @@ const messages: AdminMessages = { "admin.permissions.deleteMessage": "Ta bort behörigheten {{name}}? Den återkallas från alla den är tilldelad till.", "admin.permissions.error.adminUndeletable": "Behörigheten admin kan inte tas bort — det skulle ta bort all administratörsåtkomst.", "admin.permissions.error.selfRevoke": "Du kan inte återkalla din egen administratörsåtkomst.", + "admin.permissions.effective": "Faktisk åtkomst", + "admin.permissions.effectiveHint": "Alla som har behörigheten — direkt eller via en grupp (uppslaget av Keto).", "admin.permissions.field.name": "Behörighetens namn", "admin.permissions.field.nameHint": "Små bokstäver, siffror, bindestreck och understreck.", "admin.permissions.filter": "Filtrera behörigheter", "admin.permissions.new": "Ny behörighet", + "admin.permissions.noEffective": "Ingen användare har den här behörigheten ännu.", "admin.permissions.noMembers": "Inte tilldelad till någon ännu.", "admin.permissions.pagination": "Sidnavigering för behörigheter", "admin.permissions.revoke": "Återkalla", diff --git a/examples/plugins/admin/views/partials/client-detail-body.ejs b/examples/plugins/admin/views/partials/client-detail-body.ejs index 5addf44..1fee872 100644 --- a/examples/plugins/admin/views/partials/client-detail-body.ejs +++ b/examples/plugins/admin/views/partials/client-detail-body.ejs @@ -33,6 +33,6 @@
">

<%= t("admin.clients.rereg") %>

- <%= t("admin.clients.delete") %> + <%= t("admin.clients.delete") %>
diff --git a/examples/plugins/admin/views/partials/group-detail-body.ejs b/examples/plugins/admin/views/partials/group-detail-body.ejs index 73a37e0..2171a61 100644 --- a/examples/plugins/admin/views/partials/group-detail-body.ejs +++ b/examples/plugins/admin/views/partials/group-detail-body.ejs @@ -37,6 +37,6 @@ <% } -%>
"> - <%= t("admin.groups.delete") %> + <%= t("admin.groups.delete") %>
diff --git a/examples/plugins/admin/views/partials/permission-detail-body.ejs b/examples/plugins/admin/views/partials/permission-detail-body.ejs index 1061264..1cc1be3 100644 --- a/examples/plugins/admin/views/partials/permission-detail-body.ejs +++ b/examples/plugins/admin/views/partials/permission-detail-body.ejs @@ -31,8 +31,8 @@ <% } -%>
-

Effective access

-

Everyone who holds this permission — directly or through a group (resolved by Keto).

+

<%= t("admin.permissions.effective") %>

+

<%= t("admin.permissions.effectiveHint") %>

<% if (effective.length) { -%> <% } else { -%> -

No users hold this permission yet.

+

<%= t("admin.permissions.noEffective") %>

<% } -%>
-

Assign the permission

+

<%= t("admin.permissions.assign") %>

<% if (add.options.length) { -%> -
+
<% } else { -%> -

All users and groups already have this permission.

+

<%= t("admin.permissions.allAssigned") %>

<% } -%>
-
- Delete permission +
"> + <%= t("admin.permissions.delete") %>
diff --git a/examples/plugins/admin/views/partials/user-form-body.ejs b/examples/plugins/admin/views/partials/user-form-body.ejs index f4de774..920e065 100644 --- a/examples/plugins/admin/views/partials/user-form-body.ejs +++ b/examples/plugins/admin/views/partials/user-form-body.ejs @@ -30,7 +30,7 @@
">
- <%= t("admin.users.delete") %> + <%= t("admin.users.delete") %>
<% } -%> diff --git a/src/ui/list-query.test.ts b/src/ui/list-query.test.ts index 6cd9258..875b1f9 100644 --- a/src/ui/list-query.test.ts +++ b/src/ui/list-query.test.ts @@ -51,3 +51,8 @@ test("parseListQuery honours custom reserved names and page-size bounds", () => assert.equal(parseListQuery("?n=999", { maxPageSize: 50, pageSizeParam: "n" }).pageSize, 50); assert.deepEqual(parseListQuery("?q=hi", { qParam: "search" }).filters, { q: ["hi"] }); }); + +test("`locale` is the host's, not a filter — every localized list link carries it", () => { + const query = parseListQuery("/admin/users?locale=sv-SE&status=active"); + assert.deepEqual(Object.keys(query.filters), ["status"]); +}); diff --git a/src/ui/list-query.ts b/src/ui/list-query.ts index bae8aed..9c90415 100644 --- a/src/ui/list-query.ts +++ b/src/ui/list-query.ts @@ -31,7 +31,9 @@ export function parseListQuery(url: URL | URLSearchParams | string, options: Lis const sortParam = options.sortParam ?? "sort"; const pageParam = options.pageParam ?? "page"; const pageSizeParam = options.pageSizeParam ?? "pageSize"; - const reserved = new Set([pageParam, pageSizeParam, qParam, sortParam]); + // `locale` is host-owned (README → Languages): every list link and both GET forms carry it, so + // without this it would arrive as a phantom filter on every localized list page. + const reserved = new Set([pageParam, pageSizeParam, qParam, sortParam, "locale"]); const filters: Record = {}; for (const key of new Set(params.keys())) { diff --git a/views/partials/field.ejs b/views/partials/field.ejs index fb62ca8..a8df996 100644 --- a/views/partials/field.ejs +++ b/views/partials/field.ejs @@ -21,7 +21,7 @@ -%>
<% if (link || optional) { -%> -
<% if (link) { %><%= link.label %><% } else { %><%= t("field.optional") %><% } %>
+
<% if (link) { %><%= link.label %><% } else { %><%= t("field.optional") %><% } %>
<% } else { -%> <% } -%> diff --git a/views/partials/shell.ejs b/views/partials/shell.ejs index 59b3afe..cac2628 100644 --- a/views/partials/shell.ejs +++ b/views/partials/shell.ejs @@ -104,7 +104,7 @@ <% if (title) { %>

<%= title %>

<% } %> <% if (breadcrumbs.length) { %> <% } %>