From d66c6c04da6ac1d82e85d6625759d1d2559dd303 Mon Sep 17 00:00:00 2001 From: Jake Ginnivan Date: Mon, 19 Oct 2026 10:32:01 +0800 Subject: [PATCH] fix: address five defects found reviewing the upstream feature stack Upstreamed from dddwa/dddperth#208. - fix(admin): revalidate planner data only after queued edits. Replaces the edge-triggered fix carried into `useQueuedSave` when #24 was rebased over #21; the fork fixed the same loop with a `needsRevalidation` ref, and keeping one version on both sides stops the next `/core-pull` conflicting on this file. - fix(agenda): serialize shortlist reports to preserve identity and order - fix(volunteers): reject assignments across overlapping sessions - fix(sponsors): preserve Jira paragraph and line breaks - fix(sponsors): preserve portal saves made during Jira synchronization Every file is byte-identical to the fork's post-#208 copy except `jira-client.test.ts`, which keeps upstream's `describe.runIf(conferenceManifest.sponsorPortal)` guards. The new rich-text test builds its own Jira config, so it runs in the stub. --- website/app/components/volunteer-planner.tsx | 35 ++++++---- .../d1-sponsors-store.server.test.ts | 34 ++++++++++ .../cloudflare/d1-sponsors-store.server.ts | 29 +++++++-- .../d1-volunteers-store.server.test.ts | 58 ++++++++++++----- .../cloudflare/d1-volunteers-store.server.ts | 24 ++++--- .../cloudflare/jira-sponsor-sync.server.ts | 3 +- website/app/lib/services/sponsors-store.ts | 8 ++- website/app/lib/services/volunteers-store.ts | 29 ++++++++- .../app/lib/sponsors/jira-client.server.ts | 21 +++--- website/app/lib/sponsors/jira-client.test.ts | 64 +++++++++++++++++++ website/app/lib/use-my-agenda.test.tsx | 58 +++++++++++++++++ website/app/lib/use-my-agenda.ts | 20 ++++-- website/app/lib/use-queued-save.test.tsx | 49 ++++++++++++++ website/app/lib/use-queued-save.ts | 14 ++-- website/app/routes/admin.volunteers.tsx | 55 +++++++++++++--- 15 files changed, 423 insertions(+), 78 deletions(-) create mode 100644 website/app/lib/use-my-agenda.test.tsx create mode 100644 website/app/lib/use-queued-save.test.tsx diff --git a/website/app/components/volunteer-planner.tsx b/website/app/components/volunteer-planner.tsx index 3585c89..37c7143 100644 --- a/website/app/components/volunteer-planner.tsx +++ b/website/app/components/volunteer-planner.tsx @@ -1,5 +1,10 @@ import { Fragment, useEffect, useRef, useState, type CSSProperties } from 'react' -import { VOLUNTEER_ROLES, type VolunteerAssignment, type VolunteerRole } from '~/lib/services/volunteers-store' +import { + volunteerSeatsOverlap, + VOLUNTEER_ROLES, + type VolunteerAssignment, + type VolunteerRole, +} from '~/lib/services/volunteers-store' import { css, cx } from '~/styled-system/css' import { Box, styled } from '~/styled-system/jsx' @@ -53,7 +58,10 @@ export interface VolunteerPlannerSlot { * how many rooms from this one it fills — more than one for a plenum * talk, whose shifts still belong to this room. */ - sessions: Record + sessions: Record< + string, + { title: string; rowSpan: number; colSpan: number; isService: boolean; endLabel: string; endsAt: string } + > } export interface VolunteerPlannerVolunteer { @@ -268,12 +276,13 @@ export function VolunteerPlanner({ seats.set(key, a.intent === 'assign' ? [...others, a.volunteerId] : others) } - // Where each volunteer is during each slot. - const seatOf = new Map() - for (const [key, ids] of seats) { - const [slotId] = key.split('|') - for (const id of ids) seatOf.set(`${slotId}|${id}`, key) - } + const sessionTimes = slots.flatMap((slot) => + Object.entries(slot.sessions).map(([roomId, session]) => ({ + slotId: slot.id, + roomId, + endsAt: session.endsAt, + })), + ) const volunteerById = new Map(volunteers.map((v) => [v.id, v])) const keyOf = (seat: Seat) => seatKey(slots[seat.slotIndex].id, seat.roomId, seat.role) @@ -340,10 +349,14 @@ export function VolunteerPlanner({ } } - /** Somewhere else this slot. `leaving` is a seat they're being moved off. */ + /** Somewhere else during any part of this session. */ function busyElsewhere(volunteerId: string, seat: Seat, leaving?: Seat) { - const at = seatOf.get(`${slots[seat.slotIndex].id}|${volunteerId}`) - return at !== undefined && at !== keyOf(seat) && at !== (leaving && keyOf(leaving)) + const target = { slotId: slots[seat.slotIndex].id, roomId: seat.roomId } + return [...seats].some(([key, ids]) => { + if (!ids.includes(volunteerId) || key === keyOf(seat) || (leaving && key === keyOf(leaving))) return false + const [slotId, roomId] = key.split('|') + return volunteerSeatsOverlap(target, { slotId, roomId }, sessionTimes) + }) } function canAdd(volunteerId: string, seat: Seat, leaving?: Seat) { diff --git a/website/app/lib/services/cloudflare/d1-sponsors-store.server.test.ts b/website/app/lib/services/cloudflare/d1-sponsors-store.server.test.ts index acc23f4..01bc5b1 100644 --- a/website/app/lib/services/cloudflare/d1-sponsors-store.server.test.ts +++ b/website/app/lib/services/cloudflare/d1-sponsors-store.server.test.ts @@ -43,6 +43,40 @@ describe('sponsor two-way sync (real SQL)', () => { await store.applySyncPlan(plan) } + it('does not replace a save made during the Jira read, even within the same second', async () => { + await sync() + await store.saveDetails('SPN-1', { blurb: 'Original', websiteUrl: 'https://old.test', socials: {} }, 'sponsor') + await store.saveLogistics('SPN-1', { rafflePrize: 'Original prize' }, 'sponsor') + sqlite.exec('UPDATE sponsor_profiles SET details_updated_at = 100, logistics_updated_at = 100') + const beforeRead = await store.getProfilesForSync() + // Jira returns the old values, while the sponsor successfully saves new ones. + await store.saveDetails('SPN-1', { blurb: 'New', websiteUrl: 'https://new.test', socials: {} }, 'sponsor') + await store.saveLogistics('SPN-1', { rafflePrize: 'New prize' }, 'sponsor') + sqlite.exec('UPDATE sponsor_profiles SET details_updated_at = 100, logistics_updated_at = 100') + const plan = computeSyncPlan({ + year: '2026', + source: [ + { + issueKey: 'SPN-1', + companyName: 'Acme', + tier: 'Gold', + contactEmails: [], + quote: 'Old', + website: 'https://old.test', + logistics: { rafflePrize: 'Old prize' }, + logisticsKeys: ['rafflePrize'], + }, + ], + currentSponsors: await store.getAllSponsorsForSync(), + currentContacts: await store.getAllContacts(), + }) + await store.applySyncPlan(plan, beforeRead) + expect(await store.getProfile('SPN-1')).toMatchObject({ blurb: 'New', logistics: { rafflePrize: 'New prize' } }) + // A later read can still import deliberate committee changes immediately. + await store.applySyncPlan(plan, await store.getProfilesForSync()) + expect(await store.getProfile('SPN-1')).toMatchObject({ blurb: 'Old', logistics: { rafflePrize: 'Old prize' } }) + }) + it('stores Jira prefills without inventing a portal submission', async () => { await sync({ quote: 'From Jira', website: 'https://jira.test', socials: { linkedin: 'https://in.test' } }) expect(await store.getProfile('SPN-1')).toBeNull() diff --git a/website/app/lib/services/cloudflare/d1-sponsors-store.server.ts b/website/app/lib/services/cloudflare/d1-sponsors-store.server.ts index fd79bc9..ac0b663 100644 --- a/website/app/lib/services/cloudflare/d1-sponsors-store.server.ts +++ b/website/app/lib/services/cloudflare/d1-sponsors-store.server.ts @@ -281,11 +281,15 @@ export function createD1SponsorsStore(db: D1Database): SponsorsStore { return (result.meta.changes ?? 0) > 0 }, - async applySyncPlan(plan) { - const statements: D1PreparedStatement[] = [] + async getProfilesForSync() { + const { results } = await db.prepare(`SELECT * FROM sponsor_profiles`).all() + return results.map(toProfile) + }, - const profileResult = await db.prepare(`SELECT * FROM sponsor_profiles`).all() - const profilesByIssue = new Map((profileResult.results ?? []).map((row) => [row.issue_key, toProfile(row)])) + async applySyncPlan(plan, profilesBeforeRead) { + const statements: D1PreparedStatement[] = [] + const profiles = profilesBeforeRead ?? (await this.getProfilesForSync()) + const profilesByIssue = new Map(profiles.map((profile) => [profile.issueKey, profile])) for (const s of plan.upserts) { const profile = profilesByIssue.get(s.issueKey) @@ -297,13 +301,18 @@ export function createD1SponsorsStore(db: D1Database): SponsorsStore { `UPDATE sponsor_profiles SET blurb = ?, website_url = ?, socials_json = ?, updated_at = unixepoch(), updated_by = 'jira-sync' - WHERE issue_key = ?`, + WHERE issue_key = ? AND details_updated_at IS ? + AND blurb IS ? AND website_url IS ? AND COALESCE(socials_json, '{}') = ?`, ) .bind( reconciliation.details.blurb ?? null, reconciliation.details.websiteUrl ?? null, JSON.stringify(reconciliation.details.socials), s.issueKey, + profile?.detailsUpdatedAt ?? null, + profile?.blurb ?? null, + profile?.websiteUrl ?? null, + JSON.stringify(profile?.socials ?? {}), ), ) } @@ -313,9 +322,15 @@ export function createD1SponsorsStore(db: D1Database): SponsorsStore { .prepare( `UPDATE sponsor_profiles SET logistics_json = ?, updated_at = unixepoch(), updated_by = 'jira-sync' - WHERE issue_key = ?`, + WHERE issue_key = ? AND logistics_updated_at IS ? + AND COALESCE(logistics_json, '{}') = ?`, ) - .bind(JSON.stringify(reconciliation.logistics), s.issueKey), + .bind( + JSON.stringify(reconciliation.logistics), + s.issueKey, + profile?.logisticsUpdatedAt ?? null, + JSON.stringify(profile?.logistics ?? {}), + ), ) } statements.push( diff --git a/website/app/lib/services/cloudflare/d1-volunteers-store.server.test.ts b/website/app/lib/services/cloudflare/d1-volunteers-store.server.test.ts index 158bd9b..d2a539c 100644 --- a/website/app/lib/services/cloudflare/d1-volunteers-store.server.test.ts +++ b/website/app/lib/services/cloudflare/d1-volunteers-store.server.test.ts @@ -5,6 +5,11 @@ import type { VolunteersStore } from '../volunteers-store' import { createD1VolunteersStore } from './d1-volunteers-store.server' const seat = { slotId: '09:30:00', roomId: '101', role: 'room-coordinators' } as const +const sessions = [ + { slotId: '09:30:00', roomId: '101', endsAt: '10:30:00' }, + { slotId: '09:30:00', roomId: '102', endsAt: '10:30:00' }, + { slotId: '10:30:00', roomId: '101', endsAt: '11:30:00' }, +] const BOTH = ['room-coordinators', 'photographers'] as const describe('D1 volunteers store (real SQL)', () => { @@ -19,6 +24,26 @@ describe('D1 volunteers store (real SQL)', () => { afterEach(() => sqlite.close()) + it('rejects overlaps across different slot starts but permits adjacent sessions', async () => { + const volunteer = await store.addVolunteer('A', null, [...BOTH]) + const sessions = [ + { slotId: '09:00:00', roomId: '101', endsAt: '10:00:00' }, + { slotId: '09:30:00', roomId: '102', endsAt: '10:00:00' }, + { slotId: '10:00:00', roomId: '102', endsAt: '10:30:00' }, + ] + await store.assign('2026', { ...seat, slotId: '09:00:00', volunteerId: volunteer.id }, 'admin', sessions) + await expect( + store.assign('2026', { ...seat, roomId: '102', volunteerId: volunteer.id }, 'admin', sessions), + ).rejects.toThrow(/somewhere else/) + await store.assign( + '2026', + { ...seat, slotId: '10:00:00', roomId: '102', volunteerId: volunteer.id }, + 'admin', + sessions, + ) + expect(await store.listAssignments('2026')).toHaveLength(2) + }) + it('lists volunteers alphabetically, ignoring case, with their roles', async () => { await store.addVolunteer('zoe', null, ['photographers']) await store.addVolunteer('Alex', 'alex@example.com', [...BOTH]) @@ -32,9 +57,9 @@ describe('D1 volunteers store (real SQL)', () => { const a = await store.addVolunteer('A', null, [...BOTH]) const b = await store.addVolunteer('B', null, [...BOTH]) - await store.assign('2026', { ...seat, volunteerId: a.id }, 'admin@example.com') - await store.assign('2026', { ...seat, volunteerId: b.id }, 'admin@example.com') - await store.assign('2027', { ...seat, volunteerId: a.id }, 'admin@example.com') + await store.assign('2026', { ...seat, volunteerId: a.id }, 'admin@example.com', sessions) + await store.assign('2026', { ...seat, volunteerId: b.id }, 'admin@example.com', sessions) + await store.assign('2027', { ...seat, volunteerId: a.id }, 'admin@example.com', sessions) expect(await store.listAssignments('2026')).toEqual([ { ...seat, volunteerId: a.id }, @@ -45,36 +70,37 @@ describe('D1 volunteers store (real SQL)', () => { it("refuses a role the volunteer doesn't hold", async () => { const a = await store.addVolunteer('A', null, ['photographers']) - await expect(store.assign('2026', { ...seat, volunteerId: a.id }, 'admin@example.com')).rejects.toThrow( - /isn't down as a room coordinator/, - ) + await expect( + store.assign('2026', { ...seat, volunteerId: a.id }, 'admin@example.com', sessions), + ).rejects.toThrow(/isn't down as a room coordinator/) }) it('refuses to put someone in two places in the same slot, but allows re-saving the same seat', async () => { const a = await store.addVolunteer('A', null, [...BOTH]) - await store.assign('2026', { ...seat, volunteerId: a.id }, 'admin@example.com') - await store.assign('2026', { ...seat, volunteerId: a.id }, 'admin@example.com') + await store.assign('2026', { ...seat, volunteerId: a.id }, 'admin@example.com', sessions) + await store.assign('2026', { ...seat, volunteerId: a.id }, 'admin@example.com', sessions) await expect( - store.assign('2026', { ...seat, role: 'photographers', volunteerId: a.id }, 'admin@example.com'), + store.assign('2026', { ...seat, role: 'photographers', volunteerId: a.id }, 'admin@example.com', sessions), ).rejects.toThrow(/somewhere else/) await expect( - store.assign('2026', { ...seat, roomId: '102', volunteerId: a.id }, 'admin@example.com'), + store.assign('2026', { ...seat, roomId: '102', volunteerId: a.id }, 'admin@example.com', sessions), ).rejects.toThrow(/somewhere else/) // A different slot is fine. - await store.assign('2026', { ...seat, slotId: '10:30:00', volunteerId: a.id }, 'admin@example.com') + await store.assign('2026', { ...seat, slotId: '10:30:00', volunteerId: a.id }, 'admin@example.com', sessions) expect(await store.listAssignments('2026')).toHaveLength(2) }) it("dropping a role clears that year's shifts in it, and leaves other years alone", async () => { const a = await store.addVolunteer('A', null, [...BOTH]) - await store.assign('2025', { ...seat, volunteerId: a.id }, 'admin@example.com') - await store.assign('2026', { ...seat, volunteerId: a.id }, 'admin@example.com') + await store.assign('2025', { ...seat, volunteerId: a.id }, 'admin@example.com', sessions) + await store.assign('2026', { ...seat, volunteerId: a.id }, 'admin@example.com', sessions) await store.assign( '2026', { ...seat, slotId: '10:30:00', role: 'photographers', volunteerId: a.id }, 'admin@example.com', + sessions, ) await store.setVolunteerRoles(a.id, ['photographers'], '2026') @@ -89,9 +115,9 @@ describe('D1 volunteers store (real SQL)', () => { it('unassigns one person from a seat, and removing a volunteer clears their shifts', async () => { const a = await store.addVolunteer('A', null, [...BOTH]) const b = await store.addVolunteer('B', null, [...BOTH]) - await store.assign('2026', { ...seat, volunteerId: a.id }, 'admin@example.com') - await store.assign('2026', { ...seat, volunteerId: b.id }, 'admin@example.com') - await store.assign('2026', { ...seat, slotId: '10:30:00', volunteerId: a.id }, 'admin@example.com') + await store.assign('2026', { ...seat, volunteerId: a.id }, 'admin@example.com', sessions) + await store.assign('2026', { ...seat, volunteerId: b.id }, 'admin@example.com', sessions) + await store.assign('2026', { ...seat, slotId: '10:30:00', volunteerId: a.id }, 'admin@example.com', sessions) await store.unassign('2026', { ...seat, volunteerId: a.id }) expect(await store.listAssignments('2026')).toEqual([ diff --git a/website/app/lib/services/cloudflare/d1-volunteers-store.server.ts b/website/app/lib/services/cloudflare/d1-volunteers-store.server.ts index 51964cf..f961664 100644 --- a/website/app/lib/services/cloudflare/d1-volunteers-store.server.ts +++ b/website/app/lib/services/cloudflare/d1-volunteers-store.server.ts @@ -1,5 +1,6 @@ import { isVolunteerRole, + volunteerSeatsOverlap, VOLUNTEER_ROLES, type Volunteer, type VolunteerAssignment, @@ -95,7 +96,7 @@ export function createD1VolunteersStore(db: D1Database): VolunteersStore { return results.map(toAssignment) }, - async assign(year, { slotId, roomId, role, volunteerId }, assignedBy) { + async assign(year, { slotId, roomId, role, volunteerId }, assignedBy, sessions) { const volunteer = await db .prepare(`SELECT roles_json FROM volunteers WHERE id = ?`) .bind(volunteerId) @@ -106,13 +107,20 @@ export function createD1VolunteersStore(db: D1Database): VolunteersStore { throw new Error(`That volunteer isn't down as a ${label.toLowerCase()}.`) } - const existing = await db - .prepare(`SELECT room_id, role FROM volunteer_assignments WHERE year = ? AND slot_id = ? AND volunteer_id = ?`) - .bind(year, slotId, volunteerId) - .first<{ room_id: string; role: string }>() - if (existing) { - if (existing.room_id === roomId && existing.role === role) return - throw new Error('That volunteer is already rostered somewhere else during this slot.') + if (!sessions.some((s) => s.slotId === slotId && s.roomId === roomId)) { + throw new Error('That session is no longer on the agenda.') + } + const { results: existing } = await db + .prepare( + `SELECT slot_id, room_id, role, volunteer_id FROM volunteer_assignments WHERE year = ? AND volunteer_id = ?`, + ) + .bind(year, volunteerId) + .all() + for (const row of existing) { + if (row.slot_id === slotId && row.room_id === roomId && row.role === role) return + if (volunteerSeatsOverlap({ slotId, roomId }, toAssignment(row), sessions)) { + throw new Error('That volunteer is already rostered somewhere else during this session.') + } } await db diff --git a/website/app/lib/services/cloudflare/jira-sponsor-sync.server.ts b/website/app/lib/services/cloudflare/jira-sponsor-sync.server.ts index 4640f22..90cb5bd 100644 --- a/website/app/lib/services/cloudflare/jira-sponsor-sync.server.ts +++ b/website/app/lib/services/cloudflare/jira-sponsor-sync.server.ts @@ -190,6 +190,7 @@ export function createJiraSponsorSyncService(args: { const runId = await sponsors.startSyncRun(trigger) try { + const profilesBeforeRead = await sponsors.getProfilesForSync() const [source, currentSponsors, currentContacts] = await Promise.all([ client.searchSponsorIssues(), sponsors.getAllSponsorsForSync(), @@ -197,7 +198,7 @@ export function createJiraSponsorSyncService(args: { ]) const plan = computeSyncPlan({ year: portalConfig.year, source, currentSponsors, currentContacts }) - const counts = await sponsors.applySyncPlan(plan) + const counts = await sponsors.applySyncPlan(plan, profilesBeforeRead) // After the plan is applied — the sponsor data landing in D1 // matters more than the label, and this must not fail the run. diff --git a/website/app/lib/services/sponsors-store.ts b/website/app/lib/services/sponsors-store.ts index 1be4595..c518a7c 100644 --- a/website/app/lib/services/sponsors-store.ts +++ b/website/app/lib/services/sponsors-store.ts @@ -134,6 +134,8 @@ export interface SponsorsStore { * planner can deactivate anything that left the sync query. */ getAllSponsorsForSync(): Promise> getAllContacts(): Promise> + /** Capture before the Jira read so reconciliation can reject concurrent portal saves. */ + getProfilesForSync(): Promise saveDetails( issueKey: string, @@ -153,7 +155,11 @@ export interface SponsorsStore { * did the stamping (i.e. the profile just became complete). */ markProfileCompleted(issueKey: string): Promise - applySyncPlan(plan: SponsorSyncPlan): Promise<{ + /** With a pre-read snapshot, only unchanged profile fields are reconciled. */ + applySyncPlan( + plan: SponsorSyncPlan, + profilesBeforeRead?: SponsorProfile[], + ): Promise<{ sponsorsUpserted: number sponsorsDeactivated: number contactsAdded: number diff --git a/website/app/lib/services/volunteers-store.ts b/website/app/lib/services/volunteers-store.ts index 1771334..4bae002 100644 --- a/website/app/lib/services/volunteers-store.ts +++ b/website/app/lib/services/volunteers-store.ts @@ -40,6 +40,26 @@ export interface VolunteerAssignment { volunteerId: string } +/** Conference-local times from the agenda, keyed like roster seats. */ +export interface VolunteerSession { + slotId: string + roomId: string + endsAt: string +} + +export function volunteerSeatsOverlap( + a: Pick, + b: Pick, + sessions: readonly VolunteerSession[], +): boolean { + if (a.slotId === b.slotId) return true + const end = (seat: typeof a) => sessions.find((s) => s.slotId === seat.slotId && s.roomId === seat.roomId)?.endsAt + const aEnd = end(a) + const bEnd = end(b) + // Orphaned assignments are reported separately by the planner. + return !!aEnd && !!bEnd && a.slotId < bEnd && b.slotId < aEnd +} + export interface VolunteersStore { listVolunteers(): Promise addVolunteer(name: string, email: string | null, roles: VolunteerRole[]): Promise @@ -56,10 +76,15 @@ export interface VolunteersStore { /** * Adds a volunteer to a seat alongside anyone already there. No-op if * they're already on it. Throws if they don't hold `role`, or if they're - * somewhere *else* during `slotId` — a person can't be in two rooms, or + * somewhere *else* during the session's interval — a person can't be in two rooms, or * two roles, at once. */ - assign(year: string, assignment: VolunteerAssignment, assignedBy: string): Promise + assign( + year: string, + assignment: VolunteerAssignment, + assignedBy: string, + sessions: readonly VolunteerSession[], + ): Promise /** No-op if they aren't on that seat. */ unassign(year: string, assignment: VolunteerAssignment): Promise } diff --git a/website/app/lib/sponsors/jira-client.server.ts b/website/app/lib/sponsors/jira-client.server.ts index bc43b7b..459b0e1 100644 --- a/website/app/lib/sponsors/jira-client.server.ts +++ b/website/app/lib/sponsors/jira-client.server.ts @@ -183,18 +183,19 @@ function fieldAsText(fields: Record, fieldId: string | undefine if ('value' in value && typeof value.value === 'string') { return value.value.trim() || undefined } - // Rich text (ADF): flatten every text node. + // ADF inline nodes concatenate directly; blocks and explicit breaks + // separate lines. Flattening with spaces merges attendee names and lists. if ('type' in value && value.type === 'doc') { - const texts: string[] = [] - const walk = (node: unknown): void => { - if (!node || typeof node !== 'object') return - const n = node as { text?: unknown; content?: unknown } - if (typeof n.text === 'string') texts.push(n.text) - if (Array.isArray(n.content)) n.content.forEach(walk) + const blocks = new Set(['paragraph', 'heading', 'listItem', 'blockquote', 'tableRow']) + const text = (node: unknown): string => { + if (!node || typeof node !== 'object') return '' + const n = node as { type?: string; text?: unknown; content?: unknown } + if (n.type === 'hardBreak') return '\n' + if (typeof n.text === 'string') return n.text + const content = Array.isArray(n.content) ? n.content.map(text).join('') : '' + return blocks.has(n.type ?? '') && !content.endsWith('\n') ? content + '\n' : content } - walk(value) - const joined = texts.join(' ').trim() - return joined || undefined + return text(value).trim() || undefined } } diff --git a/website/app/lib/sponsors/jira-client.test.ts b/website/app/lib/sponsors/jira-client.test.ts index bc6f06f..3d67dd3 100644 --- a/website/app/lib/sponsors/jira-client.test.ts +++ b/website/app/lib/sponsors/jira-client.test.ts @@ -323,3 +323,67 @@ describe.runIf(conferenceManifest.sponsorPortal)('searchSponsorIssues', () => { expect(JSON.parse(requestBody).nextPageToken).toBe('page-2') }) }) + +describe('Jira rich-text round trips', () => { + afterEach(() => vi.unstubAllGlobals()) + + it('keeps attendees on separate lines and inline formatting within a name', async () => { + const portalConfig = { + year: '2026', + jira: { + baseUrl: 'https://jira.example.test', + projectKey: 'SPN', + jql: 'project = SPN', + fields: { + companyName: 'company', + website: 'website', + contactEmail: 'contacts', + tier: 'tier', + assetsStatus: 'assets', + logistics: { loadingDockAttendees: 'attendees' }, + }, + assetsCompleteOptionId: 'done', + assetsPendingOptionIds: ['pending'], + tierMap: {}, + }, + } + vi.stubGlobal( + 'fetch', + vi.fn(async () => + Response.json({ + isLast: true, + issues: [ + { + key: 'SPN-1', + fields: { + attendees: { + type: 'doc', + version: 1, + content: [ + { + type: 'paragraph', + content: [ + { type: 'text', text: 'Alice ' }, + { type: 'text', text: 'Smith', marks: [{ type: 'strong' }] }, + { type: 'hardBreak' }, + { type: 'text', text: 'Bob Jones' }, + ], + }, + { type: 'paragraph', content: [{ type: 'text', text: 'Carol Adams' }] }, + ], + }, + }, + }, + ], + }), + ), + ) + const client = createJiraClient({ portalConfig, apiEmail: 'test@example.com', apiToken: 'test' }) + expect((await client.searchSponsorIssues())[0].logistics?.loadingDockAttendees).toBe( + 'Alice Smith\nBob Jones\nCarol Adams', + ) + expect((await client.getExhibitorLogistics()).get('SPN-1')?.loadingDockAttendees).toBe( + 'Alice Smith\nBob Jones\nCarol Adams', + ) + }) +}) diff --git a/website/app/lib/use-my-agenda.test.tsx b/website/app/lib/use-my-agenda.test.tsx new file mode 100644 index 0000000..4f29f28 --- /dev/null +++ b/website/app/lib/use-my-agenda.test.tsx @@ -0,0 +1,58 @@ +// @vitest-environment jsdom +import { act, cleanup, renderHook, waitFor } from '@testing-library/react' +import { afterEach, expect, it, vi } from 'vitest' +import { useMyAgenda } from './use-my-agenda' + +afterEach(() => { + cleanup() + vi.unstubAllGlobals() +}) + +it('waits for the first shortlist identity and preserves add/remove ordering', async () => { + const storage = new Map() + vi.stubGlobal('localStorage', { + getItem: (key: string) => storage.get(key) ?? null, + setItem: (key: string, value: string) => storage.set(key, value), + }) + let finish: (response: Response) => void = () => {} + const fetchMock = vi.fn( + () => + new Promise((resolve) => { + finish = resolve + }), + ) + vi.stubGlobal('fetch', fetchMock) + const a = { id: 'A', startsAt: '2026-11-14T09:00:00', endsAt: '2026-11-14T10:00:00' } + const b = { ...a, id: 'B' } + const { result } = renderHook(() => + useMyAgenda( + '2026', + new Map([ + ['A', a], + ['B', b], + ]), + ), + ) + act(() => { + result.current.toggle(a) + result.current.toggle(b) + }) + expect(result.current.picked).toEqual(['B']) + await waitFor(() => expect(fetchMock).toHaveBeenCalledTimes(1)) + await act(async () => finish(Response.json({ ok: true }))) + await waitFor(() => expect(fetchMock).toHaveBeenCalledTimes(2)) + await act(async () => finish(Response.json({ ok: true }))) + await waitFor(() => expect(fetchMock).toHaveBeenCalledTimes(3)) + await act(async () => finish(Response.json({ ok: true }))) + expect( + fetchMock.mock.calls.map(([, init]) => { + const body = init?.body + if (!(body instanceof URLSearchParams)) throw new Error('Expected shortlist form body') + return [body.get('talkId'), body.get('action')] + }), + ).toEqual([ + ['A', 'add'], + ['B', 'add'], + ['A', 'remove'], + ]) +}) diff --git a/website/app/lib/use-my-agenda.ts b/website/app/lib/use-my-agenda.ts index ed6d535..15ed818 100644 --- a/website/app/lib/use-my-agenda.ts +++ b/website/app/lib/use-my-agenda.ts @@ -53,13 +53,21 @@ function subscribe(listener: () => void) { } } +// All hook instances share the queue: the first response establishes the +// HttpOnly identity, and later add/remove requests must preserve edit order. +let shortlistQueue: Promise = Promise.resolve() + function reportShortlist(year: string, talkId: string, action: 'add' | 'remove') { - void fetch('/api/agenda/shortlist', { - method: 'POST', - body: new URLSearchParams({ year, talkId, action }), - // Survives the person navigating away straight after picking. - keepalive: true, - }).catch(() => {}) + shortlistQueue = shortlistQueue + .then(() => + fetch('/api/agenda/shortlist', { + method: 'POST', + body: new URLSearchParams({ year, talkId, action }), + // Survives the person navigating away straight after picking. + keepalive: true, + }), + ) + .catch(() => {}) } export interface ToggleOutcome { diff --git a/website/app/lib/use-queued-save.test.tsx b/website/app/lib/use-queued-save.test.tsx new file mode 100644 index 0000000..3e12af0 --- /dev/null +++ b/website/app/lib/use-queued-save.test.tsx @@ -0,0 +1,49 @@ +// @vitest-environment jsdom +import { act, cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react' +import { createMemoryRouter, RouterProvider } from 'react-router' +import { afterEach, expect, it, vi } from 'vitest' +import { useQueuedSave } from './use-queued-save' + +afterEach(() => { + cleanup() + vi.unstubAllGlobals() +}) + +it('revalidates once after a queue drains and never starts an idle reload loop', async () => { + let finish: (response: Response) => void = () => {} + const fetchMock = vi.fn( + () => + new Promise((resolve) => { + finish = resolve + }), + ) + vi.stubGlobal('fetch', fetchMock) + const loader = vi.fn(async () => { + // Bound a regression so the old idle loop cannot run forever. + if (loader.mock.calls.length > 4) throw new Error('Repeated idle reload') + return null + }) + function Planner() { + const { save, isSaving } = useQueuedSave('test', '/save') + return + } + const router = createMemoryRouter([{ path: '/', loader, Component: Planner }]) + render() + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 20)) + }) + expect(loader).toHaveBeenCalledTimes(1) + fireEvent.click(screen.getByText('Edit')) + fireEvent.click(screen.getByText('Saving')) + await waitFor(() => expect(fetchMock).toHaveBeenCalledTimes(1)) + await act(async () => finish(Response.json({ success: true }))) + await waitFor(() => expect(fetchMock).toHaveBeenCalledTimes(2)) + expect(loader).toHaveBeenCalledTimes(1) + await act(async () => finish(Response.json({ success: true }))) + await waitFor(() => expect(screen.getByText('Edit')).toBeTruthy()) + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 20)) + }) + expect(loader).toHaveBeenCalledTimes(2) + router.dispose() +}) diff --git a/website/app/lib/use-queued-save.ts b/website/app/lib/use-queued-save.ts index f4aca02..b7eb150 100644 --- a/website/app/lib/use-queued-save.ts +++ b/website/app/lib/use-queued-save.ts @@ -17,12 +17,14 @@ import { useRevalidator } from 'react-router' */ export function useQueuedSave>(logLabel: string, actionPath: string) { const revalidator = useRevalidator() + const needsRevalidation = useRef(false) const queue = useRef>(Promise.resolve()) const [inFlight, setInFlight] = useState(0) const [saveError, setSaveError] = useState(null) const save = useCallback( (change: Change) => { + needsRevalidation.current = true setInFlight((n) => n + 1) queue.current = queue.current .then(async () => { @@ -36,7 +38,9 @@ export function useQueuedSave>(logLabel: s // An expired admin session redirects to the (HTML) login // page, which fetch follows and reports as a 200. if (!response.headers.get('Content-Type')?.includes('application/json')) { - throw new Error('Save failed — your session may have expired. Reload the page to sign in again.') + throw new Error( + 'Save failed — your session may have expired. Reload the page to sign in again.', + ) } const result: { success: boolean; error?: string } = await response.json() if (!result.success) { @@ -55,15 +59,11 @@ export function useQueuedSave>(logLabel: s const isSaving = inFlight > 0 - // Only fire on the saving -> idle edge: `revalidator` is a new object on - // every state change, so keying on idle alone re-runs after each - // revalidation and loops forever. - const wasSaving = useRef(false) useEffect(() => { - if (wasSaving.current && !isSaving) { + if (!isSaving && needsRevalidation.current && revalidator.state === 'idle') { + needsRevalidation.current = false void revalidator.revalidate() } - wasSaving.current = isSaving }, [isSaving, revalidator]) return { save, isSaving, saveError } diff --git a/website/app/routes/admin.volunteers.tsx b/website/app/routes/admin.volunteers.tsx index 95103ee..b90c5ce 100644 --- a/website/app/routes/admin.volunteers.tsx +++ b/website/app/routes/admin.volunteers.tsx @@ -79,6 +79,7 @@ export async function loader({ request, context }: Route.LoaderArgs) { colSpan, isService: room.session.isServiceSession, endLabel: endsAt ? formatTime(endsAt) : '', + endsAt: endsAt ?? timeSlots[slotIndex + 1]?.slotStart ?? '24:00:00', }, ] }), @@ -150,9 +151,25 @@ export async function action({ request, context }: Route.ActionArgs) { case 'unassign': { const role = str('role') if (!isVolunteerRole(role)) return { success: false as const, error: `Unknown role: ${role}` } - const assignment = { slotId: str('slotId'), roomId: str('roomId'), role, volunteerId: str('volunteerId') } + const assignment = { + slotId: str('slotId'), + roomId: str('roomId'), + role, + volunteerId: str('volunteerId'), + } if (intent === 'assign') { - await services.volunteers.assign(year, assignment, email) + const schedule = await getScheduleForOrganisers(context, year) + const slots = schedule?.timeSlots ?? [] + const sessions = slots.flatMap((slot, index) => + slot.rooms + .filter((room) => !room.session.isServiceSession) + .map((room) => ({ + slotId: slot.slotStart, + roomId: String(room.id), + endsAt: room.session.endsAt?.slice(11, 19) ?? slots[index + 1]?.slotStart ?? '24:00:00', + })), + ) + await services.volunteers.assign(year, assignment, email, sessions) } else { await services.volunteers.unassign(year, assignment) } @@ -200,7 +217,12 @@ export default function AdminVolunteers() { if (!checked) { const shifts = assignments.filter((a) => a.volunteerId === volunteer.id && a.role === role).length const label = VOLUNTEER_ROLES.find((r) => r.id === role)?.label.toLowerCase() - if (shifts && !confirm(`Take ${volunteer.name} off ${label}? They'll be taken off the ${shifts} ${shifts === 1 ? 'talk' : 'talks'} they're rostered for as a ${label}.`)) { + if ( + shifts && + !confirm( + `Take ${volunteer.name} off ${label}? They'll be taken off the ${shifts} ${shifts === 1 ? 'talk' : 'talks'} they're rostered for as a ${label}.`, + ) + ) { return false } } @@ -213,13 +235,22 @@ export default function AdminVolunteers() { const warning = volunteer.shifts ? ` They'll be taken off the ${volunteer.shifts} ${volunteer.shifts === 1 ? 'talk' : 'talks'} they're rostered for.` : '' - if (confirm(`Remove ${volunteer.name}?${warning}`)) save({ intent: 'remove_volunteer', volunteerId: volunteer.id }) + if (confirm(`Remove ${volunteer.name}?${warning}`)) + save({ intent: 'remove_volunteer', volunteerId: volunteer.id }) } return ( {error && ( - + {error} )} @@ -348,12 +379,18 @@ export default function AdminVolunteers() { Time slots and rooms come from the {year} agenda in Sessionize, including before it's - published. Changes save as you make them. Several people can share a role in a room; volunteers - only drop into roles they're down for, and not into a time slot they're already rostered - in. + published. Changes save as you make them. Several people can share a role in a room; volunteers only + drop into roles they're down for, and not into a time slot they're already rostered in. {orphanedCount > 0 && ( - + {orphanedCount} rostered {orphanedCount === 1 ? 'shift is' : 'shifts are'} for a time slot or room that's no longer on the agenda, so {orphanedCount === 1 ? "it isn't" : "they aren't"}{' '} shown below.