From cacefb1f30f2af232f4b27c36a1dcfeef970897d Mon Sep 17 00:00:00 2001 From: Jared Vititoe Date: Thu, 9 Jul 2026 22:54:57 -0400 Subject: [PATCH] fix(scheduling): harden edit/reschedule after review Address findings from 3 review agents on the edit/reschedule feature: - Race (correctness): handleEdit now marks the old message as cancelling while its cancel is in flight, so its Edit/Cancel buttons are disabled. Previously the old row stayed live during the fire-and-forget cancel, so a fast second edit could orphan a still-scheduled event and send twice. - Durability (correctness): on a failed cancel-old, re-insert the old message if auto-prune removed its row while the modal was open, so the still-live delayed event stays visible and retriable instead of failing silently. Also clear any stale cancel error when starting an edit. - a11y: per-row Edit/Cancel buttons now carry distinct aria-labels that include the message preview and send-time, so screen-reader users can tell which of several scheduled messages each button targets. - UX: modal gains a submitLabel prop; the edit flow shows "Reschedule" instead of "Schedule". Modal now focuses the message body on open. Co-Authored-By: Claude Opus 4.8 --- .../features/room/ScheduleMessageModal.tsx | 7 ++- .../features/room/ScheduledMessagesTray.tsx | 55 ++++++++++++++++--- 2 files changed, 51 insertions(+), 11 deletions(-) diff --git a/src/app/features/room/ScheduleMessageModal.tsx b/src/app/features/room/ScheduleMessageModal.tsx index a922a02da..78669a035 100644 --- a/src/app/features/room/ScheduleMessageModal.tsx +++ b/src/app/features/room/ScheduleMessageModal.tsx @@ -30,6 +30,8 @@ interface ScheduleMessageModalProps { initialSendAt?: number; /** Header title; defaults to "Schedule Message". */ title?: string; + /** Primary-button label; defaults to "Schedule" (e.g. "Reschedule" when editing). */ + submitLabel?: string; onScheduled: (delayId: string, sendAt: number, content: IContent) => void; onClose: () => void; } @@ -94,6 +96,7 @@ export function ScheduleMessageModal({ initialBody, initialSendAt, title = 'Schedule Message', + submitLabel = 'Schedule', onScheduled, onClose, }: ScheduleMessageModalProps) { @@ -179,7 +182,7 @@ export function ScheduleMessageModal({ : undefined} > - Schedule + {submitLabel} diff --git a/src/app/features/room/ScheduledMessagesTray.tsx b/src/app/features/room/ScheduledMessagesTray.tsx index 43d072ee0..38b115d95 100644 --- a/src/app/features/room/ScheduledMessagesTray.tsx +++ b/src/app/features/room/ScheduledMessagesTray.tsx @@ -115,12 +115,25 @@ export function ScheduledMessagesTray({ roomId }: ScheduledMessagesTrayProps) { // cancel leaves it visible (and retriable) instead of letting it silently fire. const handleEdit = useCallback( (oldMsg: ScheduledMessage, newDelayId: string, sendAt: number, content: IContent) => { + // Add the newly-scheduled message up front (nothing lost yet). setScheduledMessages((prev) => { const next = new Map(prev); const current = (next.get(roomId) ?? []).filter((m) => m.delayId !== newDelayId); next.set(roomId, [{ delayId: newDelayId, roomId, content, sendAt }, ...current]); return next; }); + // Mark the old message as cancelling so its row's Edit/Cancel buttons are + // disabled while we tear it down. Without this the old row stays live during + // the in-flight cancel and a second edit could orphan a still-scheduled event + // (both would fire). Also clear any stale error from a prior failed cancel. + setCancelling((prev) => new Set(prev).add(oldMsg.delayId)); + setCancelErrors((prev) => { + if (!prev.has(oldMsg.delayId)) return prev; + const next = new Set(prev); + next.delete(oldMsg.delayId); + return next; + }); + setEditing(null); cancelScheduledMessage(mx, oldMsg.delayId) .then(() => { setScheduledMessages((prev) => { @@ -131,8 +144,28 @@ export function ScheduledMessagesTray({ roomId }: ScheduledMessagesTrayProps) { return next; }); }) - .catch(() => setCancelErrors((prev) => new Set(prev).add(oldMsg.delayId))); - setEditing(null); + .catch(() => { + // Cancel failed — the old delayed event is still live server-side, so it + // must stay visible and retriable. Re-insert it if auto-prune removed the + // row while the modal was open, otherwise the failure (and the duplicate + // it will send) would be invisible. + setScheduledMessages((prev) => { + const next = new Map(prev); + const current = next.get(roomId) ?? []; + if (!current.some((m) => m.delayId === oldMsg.delayId)) { + next.set(roomId, [...current, oldMsg]); + } + return next; + }); + setCancelErrors((prev) => new Set(prev).add(oldMsg.delayId)); + }) + .finally(() => { + setCancelling((prev) => { + const next = new Set(prev); + next.delete(oldMsg.delayId); + return next; + }); + }); }, [mx, roomId, setScheduledMessages], ); @@ -147,6 +180,7 @@ export function ScheduledMessagesTray({ roomId }: ScheduledMessagesTrayProps) { initialBody={typeof editing.content.body === 'string' ? editing.content.body : ''} initialSendAt={editing.sendAt} title="Edit scheduled message" + submitLabel="Reschedule" onScheduled={(newDelayId, sendAt, content) => handleEdit(editing, newDelayId, sendAt, content) } @@ -183,7 +217,11 @@ export function ScheduledMessagesTray({ roomId }: ScheduledMessagesTrayProps) { {/* Tray items */} {expanded && ( - {messages.map((msg) => ( + {messages.map((msg) => { + const bodyPreview = + typeof msg.content.body === 'string' ? (msg.content.body as string) : '(message)'; + const rowDesc = `${bodyPreview} at ${formatSendAt(msg.sendAt)}`; + return ( - {typeof msg.content.body === 'string' - ? (msg.content.body as string) - : '(message)'} + {bodyPreview} {formatSendAt(msg.sendAt)} @@ -214,7 +250,7 @@ export function ScheduledMessagesTray({ roomId }: ScheduledMessagesTrayProps) { size="300" radii="300" variant="SurfaceVariant" - aria-label="Edit scheduled message" + aria-label={`Edit scheduled message: ${rowDesc}`} disabled={cancelling.has(msg.delayId)} onClick={(e) => { e.stopPropagation(); @@ -227,7 +263,7 @@ export function ScheduledMessagesTray({ roomId }: ScheduledMessagesTrayProps) { size="300" radii="300" variant="SurfaceVariant" - aria-label="Cancel scheduled message" + aria-label={`Cancel scheduled message: ${rowDesc}`} disabled={cancelling.has(msg.delayId)} onClick={(e) => { e.stopPropagation(); @@ -246,7 +282,8 @@ export function ScheduledMessagesTray({ roomId }: ScheduledMessagesTrayProps) { )} - ))} + ); + })} )}