diff --git a/packages/backend/src/core/workflow/step.ts b/packages/backend/src/core/workflow/step.ts index 3160480..5b104d2 100644 --- a/packages/backend/src/core/workflow/step.ts +++ b/packages/backend/src/core/workflow/step.ts @@ -1,3 +1,4 @@ +import { prisma } from "../../app/prisma.ts"; import { deleteStepData, findLastCompletedStep, @@ -79,6 +80,25 @@ export async function goBack( throw new Error("No session ID found in context"); } + // A finished session is a durable record, not something to unwind: rollback + // hooks undo real side effects (subject links, check-in timestamps), so + // walking backwards through one would quietly damage a completed check-in. + // The kiosk hides its back button once `session:completed` arrives, but + // `step:goBack` can still be sent directly (see the admin debug panel), so + // the guard belongs here rather than in the UI. Use undoCheckin to reverse a + // finished check-in. + const session = await prisma.checkinSession.findUniqueOrThrow({ + where: { id: sessionId }, + select: { completedAt: true, abortedAt: true }, + }); + if (session.completedAt || session.abortedAt) { + getLogger(c).warn( + { completedAt: session.completedAt, abortedAt: session.abortedAt }, + "Ignoring goBack for a session that is already completed or aborted", + ); + return; + } + // Walk backwards, skipping steps marked skipOnGoBack. const maxSteps = 100; for (let i = 0; i < maxSteps; i++) { diff --git a/packages/backend/src/core/workflow/stepContext.ts b/packages/backend/src/core/workflow/stepContext.ts index c46dde9..cbdbbf0 100644 --- a/packages/backend/src/core/workflow/stepContext.ts +++ b/packages/backend/src/core/workflow/stepContext.ts @@ -224,25 +224,24 @@ export function createStepContext( throw new Error("No session ID found in context"); } - const existingSubjects = await prisma.checkinSubject.findMany({ - where: { checkinSessionId: sessionId }, - }); - - if (existingSubjects.length > 0) { - // TODO: Can we handle this gracefully? We could delete old subjects and - // set new ones, but that could have unintended consequences. Maybe we - // should just let the user start a new session? - throw new Error( - "Subjects have already been set for this session, cannot set again", - ); - } - - await prisma.checkinSubject.createMany({ - data: participantIds.map((participantId) => ({ - checkinSessionId: sessionId, - participantId, - })), - }); + // Idempotent by design: the same set can legitimately be submitted twice. + // A rollback to the selecting step leaves the old rows behind, and a + // double-tapped submit sends two calls before the step advances (the + // stale-method guard in session.socket.ts only catches calls arriving + // after advancement). Replacing the set is safe here because no step + // after subject selection has run yet — anything that acts on subjects + // runs downstream and will see the new set. + await prisma.$transaction([ + prisma.checkinSubject.deleteMany({ + where: { checkinSessionId: sessionId }, + }), + prisma.checkinSubject.createMany({ + data: participantIds.map((participantId) => ({ + checkinSessionId: sessionId, + participantId, + })), + }), + ]); }, async clearSubjects() { const sessionId = c.get("wsSessionId"); diff --git a/plugins/base/src/selectSubjects/backend/selectSubjects.ts b/plugins/base/src/selectSubjects/backend/selectSubjects.ts index 53554a7..20807be 100644 --- a/plugins/base/src/selectSubjects/backend/selectSubjects.ts +++ b/plugins/base/src/selectSubjects/backend/selectSubjects.ts @@ -66,8 +66,11 @@ export const selectSubjects: StepImplementation = { subGroups, }); }, - async onStepRollback(_ctx) { - // await ctx.clearActor(); + async onStepRollback(ctx) { + // Rolling back deletes this step's data but not the subject rows it + // wrote, so they must be cleared here — otherwise the re-run selection + // is layered on top of the previous one. + await ctx.clearSubjects(); }, }, publicMethods: { diff --git a/plugins/base/src/selectSubjects/frontend/screens/SelectSubjectScreen.tsx b/plugins/base/src/selectSubjects/frontend/screens/SelectSubjectScreen.tsx index 63efa52..766f50a 100644 --- a/plugins/base/src/selectSubjects/frontend/screens/SelectSubjectScreen.tsx +++ b/plugins/base/src/selectSubjects/frontend/screens/SelectSubjectScreen.tsx @@ -63,6 +63,7 @@ export function SelectSubjectScreen({ payload }: { payload: object }) { const validPayload = Payload(payload); + const [submitted, setSubmitted] = useState(false); const [selectedParticipantIds, setSelectedParticipantIds] = useState< string[] >(() => { @@ -118,6 +119,14 @@ export function SelectSubjectScreen({ payload }: { payload: object }) { }; const submitSelected = () => { + // Guard against double-tapping: two confirmSubjects calls can otherwise be + // in flight at once, before the backend has advanced the step. `loading` + // already blocks the click (it sets the underlying disabled attribute), so + // this is a backstop for clicks that reach the host element directly. The + // screen unmounts on step advancement, so neither needs resetting. + if (submitted) return; + setSubmitted(true); + socket?.send({ name: "step:callMethod", data: { @@ -219,6 +228,7 @@ export function SelectSubjectScreen({ payload }: { payload: object }) { icon={ArrowRightIcon} iconPosition="after" disabled={selectedParticipantIds.length === 0} + loading={submitted} onClick={submitSelected} > {t("submit", {