fix: address five defects found reviewing the upstream feature stack (dddperth #208)

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<string, { title: string; rowSpan: number; colSpan: number; isService: boolean; endLabel: string }>
+    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<string, string>()
-    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) {

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()

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<SponsorProfileRow>()
+            return results.map(toProfile)
+        },
 
-            const profileResult = await db.prepare(`SELECT * FROM sponsor_profiles`).all<SponsorProfileRow>()
-            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(

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([

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<AssignmentRow>()
+            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

website/app/lib/services/cloudflare/jira-sponsor-sync.server.ts

@@ -190,14 +190,15 @@ 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(),
                     sponsors.getAllContacts(),
                 ])
 
                 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.

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<Array<{ issueKey: string; active: boolean }>>
     getAllContacts(): Promise<Array<{ email: string; issueKey: string }>>
+    /** Capture before the Jira read so reconciliation can reject concurrent portal saves. */
+    getProfilesForSync(): Promise<SponsorProfile[]>
 
     saveDetails(
         issueKey: string,
@@ -153,7 +155,11 @@ export interface SponsorsStore {
      * did the stamping (i.e. the profile just became complete). */
     markProfileCompleted(issueKey: string): Promise<boolean>
 
-    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