Keep the chrome lazy for error pages, guard the guard-error render, split the recovery link
CI / full-gate (push) Successful in 2m38s
CI / full-gate (push) Successful in 2m38s
This commit is contained in:
@@ -127,6 +127,11 @@ them. Revisit only if the stated reason stops holding.
|
|||||||
so a localized list page doesn't hand a plugin a phantom `locale` filter; the i18n view locals (`t`, `locale`, `locales`, `localeHref`,
|
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`
|
`localeParam`, `localeSwitch`, `dir`) are likewise reserved names, merged after a handler's `data`
|
||||||
so a collision loses the key instead of breaking the shell.
|
so a collision loses the key instead of breaking the shell.
|
||||||
|
- **The language picker never appears on a POST-rendered page.** That URL frequently answers no GET
|
||||||
|
(`POST /admin/users/:id/recovery` renders a page and has no GET sibling), so a link there dead-ends
|
||||||
|
on a 405; on a re-rendered form it would also discard the visitor's input. `i18nLocals` returns an
|
||||||
|
empty `localeSwitch` for any non-GET/HEAD method, and the picker renders nothing below two choices.
|
||||||
|
Decided 2026-08-03 after a review reproduced the 405.
|
||||||
- **A plugin-owned render always runs on that plugin's context.** The landing slots (`home`,
|
- **A plugin-owned render always runs on that plugin's context.** The landing slots (`home`,
|
||||||
`dashboard`) and an `onRequest` short-circuit dispatch a plugin's handler, so they build the
|
`dashboard`) and an `onRequest` short-circuit dispatch a plugin's handler, so they build the
|
||||||
context with `contextFor(pluginId)` exactly as a plugin route does — otherwise `ctx.t` is the core
|
context with `contextFor(pluginId)` exactly as a plugin route does — otherwise `ctx.t` is the core
|
||||||
|
|||||||
@@ -940,7 +940,10 @@ is stored, so a link is shareable and a page is what its address says it is. Whe
|
|||||||
for a language, the host carries `?locale=` onto every link *it* renders (menu, sign-in, its own
|
for a language, the host carries `?locale=` onto every link *it* renders (menu, sign-in, its own
|
||||||
redirects) and `ctx.localeHref(href)` does the same for a plugin's links. The picker in the
|
redirects) and `ctx.localeHref(href)` does the same for a plugin's links. The picker in the
|
||||||
sidebar footer (and on the auth pages) lists every installed locale, each a plain link to the
|
sidebar footer (and on the auth pages) lists every installed locale, each a plain link to the
|
||||||
same page in that language — it renders only when more than one is installed.
|
same page in that language. It renders only when more than one is installed, and **not at all on a
|
||||||
|
page rendered in response to a POST** — that URL often answers no GET (following the link would
|
||||||
|
dead-end), and on a re-rendered form it would throw away what the visitor typed. A plugin building
|
||||||
|
its own picker from `ctx.locales` should do the same (`ctx.req.method`).
|
||||||
|
|
||||||
**Writing a catalog.** `en-US.ts` exports the object and its type; every other locale is written
|
**Writing a catalog.** `en-US.ts` exports the object and its type; every other locale is written
|
||||||
against that type, so a missing or misspelled key is a type error before the app ever boots:
|
against that type, so a missing or misspelled key is a type error before the app ever boots:
|
||||||
|
|||||||
@@ -88,6 +88,7 @@ test.describe.serial("authenticated admin journey", () => {
|
|||||||
const row = page.locator("tr", { hasText: `lang-${suffix}@plainpages.local` });
|
const row = page.locator("tr", { hasText: `lang-${suffix}@plainpages.local` });
|
||||||
const editHref = await row.locator('a[href^="/admin/users/"]').first().getAttribute("href");
|
const editHref = await row.locator('a[href^="/admin/users/"]').first().getAttribute("href");
|
||||||
await page.goto(`${editHref}`);
|
await page.goto(`${editHref}`);
|
||||||
|
await expect(page.locator('summary[aria-label="Språk"]')).toHaveCount(1); // control: it IS there on the GET
|
||||||
await page.getByRole("button", { name: "Skapa återställningskod" }).click(); // POST-only route
|
await page.getByRole("button", { name: "Skapa återställningskod" }).click(); // POST-only route
|
||||||
await expect(page.getByText("Återställningskod skapad")).toBeVisible();
|
await expect(page.getByText("Återställningskod skapad")).toBeVisible();
|
||||||
await expect(page.locator('summary[aria-label="Språk"]')).toHaveCount(0); // no dead-end link offered
|
await expect(page.locator('summary[aria-label="Språk"]')).toHaveCount(0); // no dead-end link offered
|
||||||
|
|||||||
@@ -138,8 +138,8 @@ const messages = {
|
|||||||
"admin.users.new": "New user",
|
"admin.users.new": "New user",
|
||||||
"admin.users.pagination": "Users pagination",
|
"admin.users.pagination": "Users pagination",
|
||||||
"admin.users.reactivate": "Reactivate",
|
"admin.users.reactivate": "Reactivate",
|
||||||
"admin.users.recovery.body":
|
"admin.users.recovery.body": "Give it to the user — they enter it to set a new password (generate a fresh one if it has expired):",
|
||||||
"Give it to the user — they enter it on the <a href=\"/recovery\">password-reset screen</a> to set a new password (generate a fresh one if it has expired).",
|
"admin.users.recovery.link": "the password-reset screen",
|
||||||
"admin.users.recovery.generate": "Generate recovery code",
|
"admin.users.recovery.generate": "Generate recovery code",
|
||||||
"admin.users.recovery.title": "Recovery code generated",
|
"admin.users.recovery.title": "Recovery code generated",
|
||||||
"admin.users.save": "Save changes",
|
"admin.users.save": "Save changes",
|
||||||
|
|||||||
@@ -138,8 +138,8 @@ const messages: AdminMessages = {
|
|||||||
"admin.users.new": "Ny användare",
|
"admin.users.new": "Ny användare",
|
||||||
"admin.users.pagination": "Sidnavigering för användare",
|
"admin.users.pagination": "Sidnavigering för användare",
|
||||||
"admin.users.reactivate": "Aktivera igen",
|
"admin.users.reactivate": "Aktivera igen",
|
||||||
"admin.users.recovery.body":
|
"admin.users.recovery.body": "Ge den till användaren — koden anges för att sätta ett nytt lösenord (skapa en ny om den hunnit gå ut):",
|
||||||
"Ge den till användaren — koden anges på <a href=\"/recovery\">sidan för lösenordsåterställning</a> för att sätta ett nytt lösenord (skapa en ny om den hunnit gå ut).",
|
"admin.users.recovery.link": "sidan för lösenordsåterställning",
|
||||||
"admin.users.recovery.generate": "Skapa återställningskod",
|
"admin.users.recovery.generate": "Skapa återställningskod",
|
||||||
"admin.users.recovery.title": "Återställningskod skapad",
|
"admin.users.recovery.title": "Återställningskod skapad",
|
||||||
"admin.users.save": "Spara ändringar",
|
"admin.users.save": "Spara ändringar",
|
||||||
|
|||||||
@@ -14,7 +14,7 @@
|
|||||||
<%- include("partials/alert", { text: locals.error, tone: "neg" }) %>
|
<%- include("partials/alert", { text: locals.error, tone: "neg" }) %>
|
||||||
<% } -%>
|
<% } -%>
|
||||||
<% if (recovery) { -%>
|
<% if (recovery) { -%>
|
||||||
<div class="alert alert-pos" role="status"><svg class="ico ico-sm" aria-hidden="true"><use href="#i-check-circle"/></svg><div class="alert-body"><strong><%= t("admin.users.recovery.title") %></strong><span><%- t("admin.users.recovery.body") %></span><% if (recovery.code) { %><span class="recovery-code"><code><%= recovery.code %></code></span><% } %></div></div>
|
<div class="alert alert-pos" role="status"><svg class="ico ico-sm" aria-hidden="true"><use href="#i-check-circle"/></svg><div class="alert-body"><strong><%= t("admin.users.recovery.title") %></strong><span><%= t("admin.users.recovery.body") %> <a href="<%= localeHref("/recovery") %>"><%= t("admin.users.recovery.link") %></a></span><% if (recovery.code) { %><span class="recovery-code"><code><%= recovery.code %></code></span><% } %></div></div>
|
||||||
<% } -%>
|
<% } -%>
|
||||||
<form class="form-card" method="post" action="<%= localeHref(form.action) %>">
|
<form class="form-card" method="post" action="<%= localeHref(form.action) %>">
|
||||||
<input type="hidden" name="_csrf" value="<%= form.csrfToken %>">
|
<input type="hidden" name="_csrf" value="<%= form.csrfToken %>">
|
||||||
|
|||||||
@@ -23,6 +23,7 @@ import type { Plugin } from "../plugin-host/plugin.ts";
|
|||||||
import { contentTypeFor, resolveStaticPath, routePublic } from "./static.ts";
|
import { contentTypeFor, resolveStaticPath, routePublic } from "./static.ts";
|
||||||
import adminManifest from "../../examples/plugins/admin/plugin.ts";
|
import adminManifest from "../../examples/plugins/admin/plugin.ts";
|
||||||
import { createI18n } from "../i18n/runtime.ts";
|
import { createI18n } from "../i18n/runtime.ts";
|
||||||
|
import type { MenuConfig } from "../ui/menu-config.ts";
|
||||||
import { loadI18n } from "../i18n/load.ts";
|
import { loadI18n } from "../i18n/load.ts";
|
||||||
|
|
||||||
const viewsDir = join(dirname(fileURLToPath(import.meta.url)), "..", "..", "views");
|
const viewsDir = join(dirname(fileURLToPath(import.meta.url)), "..", "..", "views");
|
||||||
@@ -1486,3 +1487,19 @@ test("a plugin that owns a landing page, or short-circuits a hook, translates fr
|
|||||||
assert.equal(await (await fetch(`${url}/hooked`)).text(), "Hello from the plugin");
|
assert.equal(await (await fetch(`${url}/hooked`)).text(), "Hello from the plugin");
|
||||||
assert.equal(await (await fetch(`${url}/dashboard`, { headers: { cookie } })).text(), "Hello from the plugin");
|
assert.equal(await (await fetch(`${url}/dashboard`, { headers: { cookie } })).text(), "Hello from the plugin");
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test("an error page renders without composing the menu — it exists for when the shell's data is what failed", async (t) => {
|
||||||
|
// The chrome getter is lazy on purpose; a render that reads no chrome must not trigger it, or a
|
||||||
|
// broken menu takes the error pages down with it.
|
||||||
|
let built = 0;
|
||||||
|
const menu: MenuConfig = { branding: { get name() { built++; return "Plainpages"; } }, override: {} };
|
||||||
|
const app = createApp({ jwks: staticJwks([ecJwk]), menu });
|
||||||
|
await new Promise<void>((r) => app.listen(0, r));
|
||||||
|
t.after(() => app.close());
|
||||||
|
const url = `http://localhost:${(app.address() as AddressInfo).port}`;
|
||||||
|
|
||||||
|
const res = await fetch(`${url}/no-such-page`);
|
||||||
|
assert.equal(res.status, 404);
|
||||||
|
assert.match(await res.text(), /Page not found/);
|
||||||
|
assert.equal(built, 0);
|
||||||
|
});
|
||||||
|
|||||||
+21
-2
@@ -120,7 +120,17 @@ export function createApp(options: AppOptions = {}): Server {
|
|||||||
// it. A plugin's context carries that plugin's translator, so its own catalog wins in its own views.
|
// it. A plugin's context carries that plugin's translator, so its own catalog wins in its own views.
|
||||||
// They are merged LAST: these names are reserved (README → Building plugins), and a handler that
|
// They are merged LAST: these names are reserved (README → Building plugins), and a handler that
|
||||||
// happens to use one loses that key rather than breaking the shell that renders around it.
|
// happens to use one loses that key rather than breaking the shell that renders around it.
|
||||||
const localsOf = (ctx: RequestContext): I18nRequest => ({ ...ctx, method: ctx.req.method ?? "GET" });
|
// Named field by field on purpose: spreading the context would trigger its lazy `chrome` getter,
|
||||||
|
// composing the menu for every render — including the standalone error pages, which exist to
|
||||||
|
// render when the shell's own data is what failed.
|
||||||
|
const localsOf = (ctx: RequestContext): I18nRequest => ({
|
||||||
|
locale: ctx.locale,
|
||||||
|
localeHref: ctx.localeHref,
|
||||||
|
locales: ctx.locales,
|
||||||
|
method: ctx.req.method ?? "GET",
|
||||||
|
t: ctx.t,
|
||||||
|
url: ctx.url,
|
||||||
|
});
|
||||||
const viewsFor = (ctx: RequestContext): ViewRenderer => (view, data) => render(view, { ...data, ...i18nLocals(localsOf(ctx)) });
|
const viewsFor = (ctx: RequestContext): ViewRenderer => (view, data) => render(view, { ...data, ...i18nLocals(localsOf(ctx)) });
|
||||||
const pluginViewsFor = (ctx: RequestContext, id: string): ViewRenderer => (view, data) => renderView(id, view, { ...data, ...i18nLocals(localsOf(ctx)) });
|
const pluginViewsFor = (ctx: RequestContext, id: string): ViewRenderer => (view, data) => renderView(id, view, { ...data, ...i18nLocals(localsOf(ctx)) });
|
||||||
|
|
||||||
@@ -307,7 +317,9 @@ export function createApp(options: AppOptions = {}): Server {
|
|||||||
}
|
}
|
||||||
csrfMint.setCookie();
|
csrfMint.setCookie();
|
||||||
const result = (await match.route.handler(routeCtx)) ?? null;
|
const result = (await match.route.handler(routeCtx)) ?? null;
|
||||||
if (anyResponseHooks) await runResponseHooks(plugins, contextFor, result); // observers; a throw → 500
|
// The responding plugin observes its own route, params and all; the others get a plain
|
||||||
|
// context for their own id (never another plugin's params).
|
||||||
|
if (anyResponseHooks) await runResponseHooks(plugins, (id) => (id === match.plugin.id ? routeCtx : contextFor(id)), result);
|
||||||
await sendResult(res, result, pluginViewsFor(routeCtx, match.plugin.id), carryLocale);
|
await sendResult(res, result, pluginViewsFor(routeCtx, match.plugin.id), carryLocale);
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
@@ -334,7 +346,14 @@ export function createApp(options: AppOptions = {}): Server {
|
|||||||
if (err instanceof GuardError) {
|
if (err instanceof GuardError) {
|
||||||
if (res.headersSent) return void res.end();
|
if (res.headersSent) return void res.end();
|
||||||
if (err.location) return void res.writeHead(303, { location: err.location }).end();
|
if (err.location) return void res.writeHead(303, { location: err.location }).end();
|
||||||
|
try {
|
||||||
return void sendHtml(res, err.status, await renderPage("403", {}));
|
return void sendHtml(res, err.status, await renderPage("403", {}));
|
||||||
|
} catch (renderErr) {
|
||||||
|
// Same last resort as the 500 branch below: a throw here would leave the socket open
|
||||||
|
// (this catch is the one that would have handled it), so end the response ourselves.
|
||||||
|
reqLog.error("error page render failed", { error: renderErr instanceof Error ? (renderErr.stack ?? renderErr.message) : String(renderErr) });
|
||||||
|
return void res.writeHead(err.status, { "content-type": "text/plain; charset=utf-8" }).end("Forbidden");
|
||||||
|
}
|
||||||
}
|
}
|
||||||
reqLog.error("unhandled request error", { error: err instanceof Error ? (err.stack ?? err.message) : String(err) });
|
reqLog.error("unhandled request error", { error: err instanceof Error ? (err.stack ?? err.message) : String(err) });
|
||||||
if (res.headersSent) return void res.end(); // a partial body is already on the wire
|
if (res.headersSent) return void res.end(); // a partial body is already on the wire
|
||||||
|
|||||||
@@ -141,7 +141,7 @@ const messages = {
|
|||||||
"pagination.go": "Go",
|
"pagination.go": "Go",
|
||||||
"pagination.label": "Pagination",
|
"pagination.label": "Pagination",
|
||||||
"pagination.next": "Next page",
|
"pagination.next": "Next page",
|
||||||
"pagination.of": "of",
|
"pagination.summary": "{{from}}–{{to}} of <b>{{total}}</b>",
|
||||||
"pagination.previous": "Previous page",
|
"pagination.previous": "Previous page",
|
||||||
"pagination.rows": "Rows",
|
"pagination.rows": "Rows",
|
||||||
|
|
||||||
|
|||||||
@@ -127,7 +127,7 @@ const messages: CoreMessages = {
|
|||||||
"pagination.go": "Visa",
|
"pagination.go": "Visa",
|
||||||
"pagination.label": "Sidnavigering",
|
"pagination.label": "Sidnavigering",
|
||||||
"pagination.next": "Nästa sida",
|
"pagination.next": "Nästa sida",
|
||||||
"pagination.of": "av",
|
"pagination.summary": "{{from}}–{{to}} av <b>{{total}}</b>",
|
||||||
"pagination.previous": "Föregående sida",
|
"pagination.previous": "Föregående sida",
|
||||||
"pagination.rows": "Rader",
|
"pagination.rows": "Rader",
|
||||||
|
|
||||||
|
|||||||
@@ -3,7 +3,7 @@
|
|||||||
Page items are <a>, inert ones (current/ellipsis/disabled) aren't.
|
Page items are <a>, inert ones (current/ellipsis/disabled) aren't.
|
||||||
Config (all optional; never throws):
|
Config (all optional; never throws):
|
||||||
label? nav aria-label (default "Pagination")
|
label? nav aria-label (default "Pagination")
|
||||||
summary? { from, to, total } → "from–to of <b>total</b>"
|
summary? { from, to, total } → the pagination.summary message
|
||||||
rows? { name, value?, options, label?, submitLabel?, action?, hidden? } rows-per-page form
|
rows? { name, value?, options, label?, submitLabel?, action?, hidden? } rows-per-page form
|
||||||
options: (number | { value, label })[]; hidden: { name, value }[] carries list state
|
options: (number | { value, label })[]; hidden: { name, value }[] carries list state
|
||||||
prev?, next? { href? } page step; omit href ⇒ disabled
|
prev?, next? { href? } page step; omit href ⇒ disabled
|
||||||
@@ -21,7 +21,7 @@
|
|||||||
-%>
|
-%>
|
||||||
<footer class="pager">
|
<footer class="pager">
|
||||||
<% if (summary) { -%>
|
<% if (summary) { -%>
|
||||||
<span><%= summary.from %>–<%= summary.to %> <%= t("pagination.of") %> <b><%= summary.total %></b></span>
|
<span><%- t("pagination.summary", { from: summary.from, to: summary.to, total: summary.total }) %></span>
|
||||||
<% } -%>
|
<% } -%>
|
||||||
<% if (rows) { -%>
|
<% if (rows) { -%>
|
||||||
<form class="pager-rows" method="get"<% if (rows.action) { %> action="<%= localeHref(rows.action) %>"<% } %>>
|
<form class="pager-rows" method="get"<% if (rows.action) { %> action="<%= localeHref(rows.action) %>"<% } %>>
|
||||||
|
|||||||
Reference in New Issue
Block a user