fix(events): staff could not reach their own participation history
Some checks failed
PR Checks / server-tests (pull_request) Failing after 5m57s
PR Checks / client-build (pull_request) Failing after 10m14s
PR Checks / bot-tests (pull_request) Successful in 8m36s

Found by the live walk, signed in as an admin: /account/events redirected to
the dashboard. `GET /player/events/history` is behind requireAuth alone and
self-scoped on req.user.id -- staff are a superset of players -- but the WEB
has two logged-in shells, and RequirePlayer sends anyone who is not a
`player` out of /account. A single mount there is a screen the reviewing
admin can never open.

Engagement Phase 7 hit this exact wall with the inbox and answered it with
two routes, one pair of components and one mapping. `eventHistoryPath` joins
`inboxPath` and `notificationSettingsPath` in notificationPaths.js rather
than starting a second file with the same comment at the top of it. The
staff path is /admin/events/mine, in the Events section of the sidebar, and
it is the one row in that group with no `roles`.

Also: the eventAnnounce fixture carried no slug, state or `listed`, so
`eventUrl` answered undefined in every test in that file and the new code
was exercised by none of them. The fixture now looks like a definition row,
and three tests cover the link, the unlisted case and the draft case.

The run.failed assertion that came with them was reading the wrong layer:
`baseFor` assembles eventUrl for every trigger and the SEAM drops the keys a
trigger does not declare, so the declaration test is what proves it. Removed,
with a note saying where the rule actually lives.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016wDDVXWMDz82WqE1i969r4
This commit is contained in:
2026-09-08 11:59:03 -05:00
parent 1667e636bd
commit eb167558e3
4 changed files with 66 additions and 0 deletions

View File

@@ -262,6 +262,15 @@ export default function App() {
two paths; `lib/notificationPaths.js` is the one mapping. */}
<Route path="notifications" element={<PlayerInbox />} />
<Route path="notifications/settings" element={<PlayerNotifications />} />
{/* And participation history, for the same reason and by the same
arrangement (Phase 14a): `/player/events/history` is behind
requireAuth alone, so a staff member has one — but
`RequirePlayer` sends them out of `/account`. Declared BEFORE
`events/:id`, though it need not be: a static segment outranks
a dynamic one whatever the order, which is the rule that made
`events/new` unreachable for seven phases. Written in the order
it resolves. */}
<Route path="events/mine" element={<PlayerEvents />} />
{/* Installed modules' admin pages, at /admin/<id>/…, already inside
RequireAuth + AdminLayout. A module cannot supply its own auth
wrapper — only an optional { roles }, which core applies as the

View File

@@ -19,3 +19,17 @@ export const inboxPath = (user) => (isStaff(user) ? '/admin/notifications' : '/a
/** The per-channel preferences screen. */
export const notificationSettingsPath = (user) =>
isStaff(user) ? '/admin/notifications/settings' : '/account/notifications/settings'
/**
* This account's own event participation (events Phase 14a).
*
* The third screen to need this mapping, and it needed it for exactly the reason
* the two above did: `GET /player/events/history` is behind `requireAuth` alone,
* self-scoped on `req.user.id` — a staff member has a participation history like
* anyone else, and the group's own header says staff are a superset of players.
* The WEB is what disagrees, because `RequirePlayer` sends them to the login
* page. Found the same way the notifications pair was: signed in as an admin,
* the screen simply redirected.
*/
export const eventHistoryPath = (user) =>
isStaff(user) ? '/admin/events/mine' : '/account/events'

View File

@@ -142,6 +142,12 @@ export const NAV = [
// governs: what a deployment permits at all is configuration, not a read,
// and the server gates both the GET and the PUT on `admin`.
{ to: '/admin/events/actions', label: 'Actions', icon: IconGear, roles: ['admin'] },
// Phase 14a, and the one row here that is not about running the
// deployment: it is this staff member's OWN attendance, the same screen
// and the same route a player reads at /account/events. It has no `roles`
// because it needs none — every account has a participation history, and
// the server scopes it to the caller.
{ to: '/admin/events/mine', label: 'My participation', icon: IconCalendar },
],
},
{
@@ -229,6 +235,7 @@ const TITLES = {
'/admin/events': 'Events',
'/admin/events/calendar': 'Event calendar',
'/admin/events/actions': 'Event actions',
'/admin/events/mine': 'My participation',
'/admin/events/new': 'New event',
}

View File

@@ -45,9 +45,16 @@ after(() => db.close())
const DEFINITION = {
id: 3,
title: 'The Yew Invasion',
slug: 'the-yew-invasion',
summary: 'Orcish warbands are massing north of Yew.',
series_name: 'The Yew Campaign',
timezone: 'America/New_York',
// Both are load-bearing for `eventUrl` (Phase 14a): an event with no public
// page gets no link. They were absent from this fixture, which meant the url
// was undefined in every test here and the new code was exercised by none of
// them.
state: 'ready',
listed: true,
}
const RUN = {
@@ -201,6 +208,12 @@ test('run.cancelled carries the operator\'s reason, and omits it when none was g
assert.equal(only().envelope.data.reason, undefined)
})
// `run.failed` alone gets no public page, and the DECLARATION is what enforces
// that rather than anything here: `baseFor` assembles `eventUrl` for every
// trigger and the seam drops the keys a trigger does not declare. The test above
// that asserts run.failed's url variables are exactly `['runUrl']` is therefore
// the one that proves it — an assertion on this envelope would be reading the
// wrong layer, because the filtering has not happened yet at this point.
test('run.failed links the run console — the one destination that exists today', async () => {
await announce.runFailed(RUN, 'sidecar responded 503')
const { data } = only().envelope
@@ -209,6 +222,29 @@ test('run.failed links the run console — the one destination that exists today
assert.equal(data.runUrl, '/admin/events/runs/3692')
})
test('every public emit carries the page for THIS occurrence', async () => {
await announce.runStarted(RUN)
// The slug is the definition's and the run is in the query string. Without
// `?run=` a mail about last Friday's occurrence would open next Friday's.
assert.equal(only().envelope.data.eventUrl, '/site/events/the-yew-invasion?run=3692')
})
test('an UNLISTED event announces with no link rather than a link that 404s', async () => {
// `eventUrl` is declared optional exactly so `email.button` can drop itself.
// A path here would render as a dead button in every mail — worse than none,
// because it advertises a link the reader cannot follow. `news.post` paid for
// that once already.
definitionsDb.getById = async () => ({ ...DEFINITION, listed: false })
await announce.runStarted(RUN)
assert.equal(only().envelope.data.eventUrl, undefined)
})
test('a definition that is not yet `ready` has no page either', async () => {
definitionsDb.getById = async () => ({ ...DEFINITION, state: 'draft' })
await announce.runStarted(RUN)
assert.equal(only().envelope.data.eventUrl, undefined)
})
test('run.failed falls back to the run\'s own last error', async () => {
await announce.runFailed({ ...RUN, last_error: 'the pinned version has no phases' }, null)
assert.equal(only().envelope.data.error, 'the pinned version has no phases')