feat(events): events UI v3 (W-01…W-11)

[Saul-Gomez-J]

Size of the change

61 files, +5 613 / −153. Of the added lines:

	lines	%
Tests (src/tests/, 12 files)	1 513	27 %
Documentation	0	—
Production code (.astro, .ts, .tsx, CSS)	4 100	73 %

The v3 documentation lives entirely in the backend repository
(docs/SPEC-v3.md and docs/API.md); none is added here. The tests mostly
cover the admin panel helpers, which is where the logic is: filters, forms,
team grouping and vocabulary labelling.

Fixes on the events UI v3 (W-01…W-11) after a review with subagents.

• Type as nullable what the backend serializes as null. Four lists
  (registration.specialties, registration.levels, submission.checks,
  submission.prize_requires) arrive as null when the slice is nil, and
  were typed string[]. Now astro check is what enforces the guard.
  Conversely, submission.fields can never be null (a struct by value in
  Go): the fallbacks treating it as optional were dead code.
• A type guard instead of a text sweep. The regex over ev.<field> did
  not see a destructuring and complained about correct code. The _me.astro
  guard is rewritten because it did not catch what it claimed either: finding
  a call was enough, and .join( already counted.
• preact-render-to-string becomes a declared devDependency. It came in
  by accident through npm hoisting, which is not a contract.
• The panel labels specialties and levels like the public site. The
  organiser read devops where the participant read Devops. Nine spots in
  the panel now go through two shared helpers. The vocabulary editor stays raw
  on purpose: that is where the tokens that go into event.json are written.

Verification

tsc --noEmit clean, astro check 153 files / 0 errors and 857 tests, and
each commit on its own as well. The new guards were checked with mutations:
removing a ?? [] makes astro check fail on the destructured const, and the
three forms of the _me.astro antipattern fail the new guard (two of them
passed the old one).

Left out on purpose

The panel editor does not expose location, url or demo_day_end; the
Gauntlet shows up twice in /events; the repo has no LICENSE.

🤖 Generated with Claude Code

https://claude.ai/code/session_014ToscGMqPkLEBZ6x8MhcP7

[crstian19]

Reviewed in depth. npm test (857 tests), astro check (153 files, 0 errors)
and npm run build all pass clean, and I verified the contract against
nan-events-backend#1: the ~30 routes the panel calls all exist and match
one-to-one, dry_run works both as a query param and in the body
(response.go:35), and the PUT is a merge — so location, url,
demo_day_end and the voting switches are not wiped on save. I confirmed
that last one by feeding UpdateEvent the exact body formToEventBody
produces: all five survive (clone() + Unmarshal merges per field, and
protectEventFields pins Voting.Open and LeaderboardPublic on top).

The overall shape is good and well argued in the comments: guards resolved in
the route file, POST→redirect→GET with no client JS, ?ok= filtered against
known keys, ids validated before they reach a URL, admin key never forwarded.
One correctness finding, then smaller things.

1. The nullable-type safety net only covers the public half

eed8127 types specialties, levels, checks and prize_requires as
string[] | null in events.ts so astro check enforces the ?? []. But
AdminEventView in src/lib/eventsAdmin.ts:157-163 declares the same data
from the same backend as non-nullable string[].

The backend does send null: with a hand-written event.json missing those
keys, GET {slug}/admin serialises "specialties":null, "levels":null,
"checks":null and "prize_requires":null — AdminView() does
v.Event = d.Event with no sanitising and the struct tags have no omitempty.
Backend commit 55ff2a5 fixed the clone path for exactly this reason and its
message names this failure: "el panel web hacía .length sobre ese null y la
pantalla de participantes se quedaba [rota]". The load-from-disk path is still
open.

And the gap is measurable. Dropping the ?? []:

File	astro check
_participantes.astro:46-47 (panel)	0 errors
_submission.astro:44-46 (public)	5 errors

So in the panel the ?? [] is discipline, not a compiler guarantee. A new
screen that writes specialties.map(...) passes the check and breaks at
runtime — the same bug 55ff2a5 just fixed.

Worth deciding with nan-events-backend#1 where this gets closed: sanitising on
load in the backend fixes it for every client at once; aligning the four fields
in AdminEventView with events.ts fixes it here. Doing both is cheap.

2. Security

Minor, but the stealth 404 isn't quite stealthy: hasSessionCookie does
cookie.includes('nan_session'), so a decoy cookie stops the proxy returning
404 and makes it call the backend. Verified with curl:

no cookie                               → 404 {"ok":false,"error":"not_found"}
Cookie: basura=xx-nan_session-xx        → reaches the backend
Cookie: no_es_nan_session_de_verdad=1   → reaches the backend

It grants no access (the backend still authorises), but the goal stated in the
comment — "the operation endpoints don't exist as far as the outside is
concerned" — isn't met. Parse the cookie name instead of substring-matching.

Two defence-in-depth notes, both optional:

• sameOrigin() returns true when neither Origin nor Sec-Fetch-Site is
  present. For POST the safe default is the opposite.
• The proxy doesn't check the origin itself; today Astro's checkOrigin covers
  it (verified: cross-origin POST with text/plain and with form-urlencod → 403 in both cases), but that's a default astro.config.mjsdoesn't pin, and it now guards admin operations. Anif (!sameOrigin(request))` in the han
  removes the dependency.

3. Reuse

• eventsAdmin.ts:41-47 redefines apiBase() and ssrHeaders(), which
  events.ts already exports — and this PR exported them for that reason
  (eventsCalendar.ts does reuse them). The admin copy only adds an extra
  parameter nothing uses.
• bad() is duplicated verbatim in eventsAdminTeams.ts:56 and
  eventsAdminSubmissions.ts:69, plus three inline copies of the same literal
  in Participants, Votes and Audit. Five copies of one error object: it belongs
  in eventsAdminForms.ts, next to readForm/doneHref.
• SAFE_ID appears three times; SAFE_SEGMENT twice.
• Duplicated types: LeaderboardRow (events.ts / eventsAdminVotes.ts, differing
  only in owner.type), Check vs SubmissionCheck, inline Owner.
• Check labels live in three places: CHECKS (forms), CHECK_LABELS
  (submissions) and i18n events.options.

Same drift 558b9f7 fixes for specialties and levels.

4. Cleanup

• _index.astro:62 — class:list={['adm-pill', { 'adm-pill-warn': false }]}:
  constant condition, the pill never gets marked. Should this be !w.open?
• Proxy: text || '{}' returns {} with content-type: text/csv or
  text/calendar when the upstream body is empty.

5. Tests

Solid and well thought through — swapping the text scan for a type guard in
9e02b01 is the right call. Two gaps:

• resolveAdminEventRoute has no test. eventsAdmin.test.ts covers
  fetchStaffSession, resolveAdminRoute and adminFetch, but
  resolveAdminEventRoute is the guard 6 of the 7 routes use, and the one that
  decides the 404 when the event doesn't exist. reloadAdminEventView isn't
  covered either.
• The eventToForm → formToEventBody round-trip uses an event with no
  location, url or demo_day_end. The backend merge saves them, but the
  test doesn't document that dependency; adding them would make it explicit.

6. Language

Going forward, let's keep code comments and commit messages in English
across the repo (~1250 comment lines in src/ are currently in Spanish).
Identifiers are already English, so this is about prose.

To be explicit about what does not change: the panel's user-facing strings
stay in Spanish — STATUS_LABELS, FLASH_LABELS, PARTICIPANT_STATUS_LABELS,
button copy. That's product content, not code. No need to retrofit this PR
wholesale; new and touched lines is enough.

[sre-helmcode]

Re-reviewed at 87d2edb. vitest run (862 tests, up from 857), astro check
(154 files, 0 errors) and astro build all pass clean.

Verified rather than taken on trust, and confirmed fixed:

• §1 nullable lists. 5165a93 types the four fields as string[] | null in
  AdminEventView and EventLike. I re-ran the measurement from the review:
  dropping the ?? [] from _participantes.astro:41-42 now gives 6 errors
  where it gave 0 before. In the panel it's a compiler guarantee now, not
  discipline. nan-events-backend#1 closed the load path on its side too, so
  both halves are covered, which was the ask.
• §2 cookie parsing. f759bea replaces the substring match with
  cookieHeaderHasSession (events.ts:359), which splits on ; and compares
  the name. Ran it against the three cases from the review:
  basura=xx-nan_session-xx and no_es_nan_session_de_verdad=1 are both
  false, a=1; nan_session=abc is true.
• §3 reuse. apiBase/ssrHeaders now come from events.ts, the five
  copies of bad() are one badForm (eventsAdminForms.ts:448), SAFE_ID and
  SAFE_SEGMENT live in one place each, and the duplicated types are aliases or
  re-exports. Check labels all derive from a single CHECK_DEFS. Leaving the
  i18n options separate for the public form is the right call: that's
  bilingual product copy, not configuration.
• §4 the pill. 95407f6, !w.open. Correct.
• §5 tests. 66adf3c covers resolveAdminEventRoute including the 404 for
  an unknown slug, and reloadAdminEventView including the fallback when the
  reload fails. The round-trip now puts demo_day_end in the event and asserts
  it does not travel in the body (eventsAdminForms.test.ts:96-112), which
  documents the backend-merge dependency. That was the point of the request.

Three things left.

1. §4: text || '{}' is still there, and the passthrough makes it worse

src/pages/api/events/[...path].ts:67 still returns {} for an empty upstream
body. With the content-type passthrough added on lines 55-57, an empty
participants/export.csv or calendar.ics is now served as {} with
content-type: text/csv, so the panel's own download (export?download=1)
hands the operator a file with {} inside it. Returning the body unchanged when
passthrough is true is enough.

2. §2 is fixed but nothing pins it

The fix is right, but neither cookieHeaderHasSession nor hasSessionCookie
appears in any file under src/tests/. For a security finding that's worth four
assertions. Using the repo's own mock pattern:

vi.mock('cloudflare:workers', () => ({ env: { CLOUD_API_URL: 'https://api.test' } }));
// ...
expect(cookieHeaderHasSession('basura=xx-nan_session-xx')).toBe(false);
expect(cookieHeaderHasSession('no_es_nan_session_de_verdad=1')).toBe(false);
expect(cookieHeaderHasSession('a=1; nan_session=abc')).toBe(true);
expect(cookieHeaderHasSession('')).toBe(false);

I ran exactly that against the current branch and it passes. It just needs to
live in the repo, so the next refactor of that helper can't quietly undo it.

3. §6 went the other way, and this one is not a preference

87d2edb is titled "los comentarios de los ficheros anteriores al PR pasan al
español" and translates 50 files from English to Spanish, on the stated grounds
that "el criterio del repo es español". That is not the criterion, and §6 said
so before the commit was written. Stating it as plainly as I can, because I'd
rather over-communicate this than have it drift again:

Everything written for other developers goes in English. No exceptions to
negotiate. That covers, concretely:

• code comments of every kind (//, /* */, JSDoc, <!-- -->, {/* */}, CSS
  comments)
• commit messages and PR titles and descriptions
• test names and describe / it strings
• variable, function, type and file names, which are already English
• READMEs, ADRs and anything else in the repo aimed at whoever reads the code next

The single exception is user-facing product copy, which stays in Spanish
exactly as it is today: STATUS_LABELS, FLASH_LABELS,
PARTICIPANT_STATUS_LABELS, button text, and the i18n catalogues. That is
product content, not code, and none of it changes.

This is the house style across our repos, not a taste call on this PR, and it's
the reason it's worth being firm about: the codebase has to read the same way to
everyone who opens it, including people who join later and don't speak Spanish.

Two separate problems with that commit, then:

• It applies the criterion backwards, so it needs reverting.
• It also pulls files into this PR that have nothing to do with events:
  astro.config.mjs, src/components/docs/ApiReference.astro,
  DocsTopBar.astro, RateLimits.astro, Pricing.astro, Models.astro,
  content.config.ts, env.d.ts, layouts/Docs.astro. Even had the criterion
  been the right way round, a repo-wide comment sweep doesn't belong in the
  events PR.

Reverting 87d2edb settles both at once. For this PR's own comment lines I'm
still not asking for a wholesale retrofit in this branch: new and touched lines,
plus every commit message from here on. But the direction is English, and any
future commit that moves prose the other way should be treated as a mistake to
revert rather than a style choice to discuss.