Public: anyone can read this; the crew can change it.
Errors on the origins and migrate pages are §5's field error, not .error-note #5090
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/5072-errors-on-the-origins-and-migrate-pages → main 2 commits fork 2 threads · 2 unresolved
4 files
changed +39
−12
src/web/pages/migrate.ts +5 −3
| @@ -791,9 +791,11 @@ export function migrationJobPage(ctx: Ctx): Response { | ||
| 791 | 791 | <h2>${job.fullName}</h2>`, |
| 792 | 792 | })} |
| 793 | 793 | ${failed |
| 794 | ? html`<p class="error-note"> | |
| 795 | ${job.error ?? "The migration failed."} | |
| 796 | </p> | |
| 794 | ? html`<div class="field" data-invalid> | |
| 795 | <span class="error" role="alert" id="migrate-job-err" | |
| 796 | >${job.error ?? "The migration failed."}</span | |
| 797 | > | |
| 798 | </div> | |
| 797 | 799 | <div class="bar mt"> |
| 798 | 800 | ${btn("Try again", { |
| 799 | 801 | href: `/migrate?${new URLSearchParams( |
src/web/pages/origins.ts +12 −8
| @@ -336,7 +336,11 @@ function originCard(origin: Origin, mirrors: number): Html { | ||
| 336 | 336 | </ul> |
| 337 | 337 | ${when( |
| 338 | 338 | origin.lastError !== "", |
| 339 | () => html`<p class="error-note mt">${origin.lastError}</p>`, | |
| 339 | () => html`<div class="field mt" data-invalid> | |
| 340 | <span class="error" role="alert" id="origin-${origin.id}-err" | |
| 341 | >${origin.lastError}</span | |
| 342 | > | |
| 343 | </div>`, | |
| 340 | 344 | )} |
| 341 | 345 | ${when(origin.needsReauth, () => reauthNote(origin, mirrors))} |
| 342 | 346 | <div class="bar mt"> |
| @@ -447,13 +451,13 @@ function forgejoSection( | ||
| 447 | 451 | ): Html { |
| 448 | 452 | const back = callbackUri(ctx); |
| 449 | 453 | if (back === "") { |
| 450 | return html`<p class="error-note"> | |
| 451 | This forge does not know its own public URL, so it cannot register a | |
| 452 | callback for the code flow. Set | |
| 453 | <span class="mono">FORGE_URL</span> to the base URL people reach it at | |
| 454 | (or <span class="mono">FORGE_HOST</span> to the hostname it is served | |
| 455 | on) and this path lights up. A pasted token works either way. | |
| 456 | </p>`; | |
| 454 | return html`<p class="muted small" id="origin-callback-note"> | |
outdated
Static config guidance, not an error that appeared. role=alert re-announces it on every visit and the .error style shrinks the paragraph to 12.5px. Drop role=alert, or use a muted note. Also untested. |
||
| 455 | This forge does not know its own public URL, so it cannot register a | |
| 456 | callback for the code flow. Set <span class="mono">FORGE_URL</span> to the | |
| 457 | base URL people reach it at (or <span class="mono">FORGE_HOST</span> to | |
| 458 | the hostname it is served on) and this path lights up. A pasted token | |
| 459 | works either way. | |
| 460 | </p>`; | |
| 457 | 461 | } |
| 458 | 462 | // Two verbs, two endpoints, and HTML forbids nesting one form in another — |
| 459 | 463 | // so the "forget" form sits beside the authorize form and its button joins |
test/migrate.test.ts +4
| @@ -1246,6 +1246,10 @@ describe("migrate", () => { | ||
| 1246 | 1246 | await browser(aliceCookie, `/migrate/jobs/${bad.job.id}`) |
| 1247 | 1247 | ).text(); |
| 1248 | 1248 | expect(stands).toContain("clone went wrong"); |
| 1249 | expect(stands).not.toContain("error-note"); | |
| 1250 | expect(stands).toMatch( | |
| 1251 | /<div class="field" data-invalid>\s*<span class="error" role="alert"[^>]*>clone went wrong/, | |
| 1252 | ); | |
| 1249 | 1253 | expect(stands).not.toContain("EventSource"); |
| 1250 | 1254 | expect(stands).not.toContain('http-equiv="refresh"'); |
| 1251 | 1255 | }); |
test/web-origins.test.ts +18 −1
| @@ -19,7 +19,7 @@ import { serve, type Servers } from "../src/http/server.ts"; | ||
| 19 | 19 | import { createRepo } from "../src/actions/repos.ts"; |
| 20 | 20 | import { enableApp } from "../src/actions/apps.ts"; |
| 21 | 21 | import { createToken } from "../src/actions/users.ts"; |
| 22 | import { listOrigins } from "../src/core/origins.ts"; | |
| 22 | import { listOrigins, recordSync } from "../src/core/origins.ts"; | |
| 23 | 23 | import { listPins } from "../src/core/pins.ts"; |
| 24 | 24 | import { getRepo } from "../src/core/repos.ts"; |
| 25 | 25 | import { getUserByName } from "../src/core/users.ts"; |
| @@ -274,6 +274,17 @@ describe("origins on the web", () => { | ||
| 274 | 274 | expect(stored[0]!.login).toBe(ORIGIN_LOGIN); |
| 275 | 275 | }); |
| 276 | 276 | |
| 277 | test("an origin whose last sync failed shows the error as a field error on its card", async () => { | |
| 278 | const id = listOrigins(app.db, getUserByName(app.db, "alice")!.id)[0]!.id; | |
| 279 | recordSync(app.db, id, { at: new Date().toISOString(), error: "upstream said 502" }); | |
| 280 | const html = await (await asUser(alice, "/settings/origins")).text(); | |
| 281 | expect(html).toMatch( | |
| 282 | /<div class="field mt" data-invalid>\s*<span class="error" role="alert" id="origin-[^"]+-err"\s*>upstream said 502/, | |
| 283 | ); | |
| 284 | expect(html).not.toContain("error-note"); | |
| 285 | recordSync(app.db, id, { at: new Date().toISOString() }); | |
| 286 | }); | |
| 287 | ||
| 277 | 288 | test("a bad token comes back inline with the host preserved and no secret", async () => { |
| 278 | 289 | const response = await asUser(alice, "/settings/origins/token", { |
| 279 | 290 | form: { host: forgejo.url, token: "not-the-right-token" }, |
| @@ -281,6 +292,8 @@ describe("origins on the web", () => { | ||
| 281 | 292 | expect(response.status).toBe(422); |
| 282 | 293 | const html = await response.text(); |
| 283 | 294 | expect(html).toContain("refused by the origin"); |
| 295 | expect(html).toMatch(/class="error"[^>]*>[^<]*refused by the origin/); | |
| 296 | expect(html).not.toContain("error-note"); | |
| 284 | 297 | expect(html).toContain(forgejo.url); |
| 285 | 298 | expect(html).not.toContain("not-the-right-token"); |
| 286 | 299 | }); |
| @@ -822,6 +835,10 @@ describe("origins on the web", () => { | ||
| 822 | 835 | const html = await page.text(); |
| 823 | 836 | expect(html).toContain("does not know its own public URL"); |
| 824 | 837 | expect(html).toContain("FORGE_URL"); |
| 838 | // Configuration guidance, not a failed input: a muted note, never an alert. | |
| 839 | expect(html).toMatch(/<p class="muted small" id="origin-callback-note">/); | |
| 840 | expect(html).not.toContain("error-note"); | |
| 841 | expect(html).not.toContain("origin-callback-err"); | |
| 825 | 842 | // The token path is unaffected, and still offered. |
| 826 | 843 | expect(html).toContain("Personal access token"); |
| 827 | 844 |
new above
offline
lastError path has no assertion. Add one in test/web-origins.test.ts that sets lastError and expects this markup and no error-note.