Calendar store: resolve the base event id at the boundary, not at four call sites #133

New Issue

Closed

opened 2026-08-31 03:50:18 +00:00 by jcoffey-dev · 0 comments

jcoffey-dev commented

2026-08-31 03:50:18 +00:00

Owner

web/src/store/calendar.ts sends whatever id it is handed:

async updateEvent(id, patch, sendInvites) { ... update: { [id]: patch } ... }
async destroyEvent(id, sendInvites)      { ... destroy: [id] ... }

The baseEventId ?? id resolution that makes those calls hit the series lives at the call sites instead — four of them:

web/src/views/calendar/EventPopover.tsx — const baseId = ev.baseEventId ?? ev.id
web/src/views/calendar/CalendarContextMenu.tsx — same line, feeding both patch() and del()
web/src/views/calendar/EventEditor.tsx — loads the base with cal.getEvent(init.event.baseEventId) and edits that
web/src/views/mail/InviteCard.tsx — passes existing.id, which is safe only because findByUid queries without expandRecurrences and therefore returns a base id

All four are correct today. The problem is what happens when one of them stops being correct.

Stalwart 0.16.20 removed the guard that caught a slip

Through 0.16.19, a synthetic id reaching destroy came back as a hard error — "Deleting synthetic ids is not yet supported" — and the toast said so. A mistake at a call site was loud.

As of 0.16.20 (dfb157d4) the same call silently deletes one occurrence and reports success, while the confirm dialog that preceded it said "Delete all occurrences?". The update path changed the same way: a synthetic id that used to be refused now quietly writes a recurrenceOverrides entry instead of touching the series.

Nothing is broken right now — this is not a bug report against main. It is that an invariant which was previously backstopped by the server is now enforced only by four lines in four files, at exactly the moment #132 wants to start sending synthetic ids on purpose.

What to do

Make the scope explicit at the boundary rather than incidental at the caller:

updateEvent and destroyEvent take the target scope as an argument — series or occurrence — and resolve the id themselves from the event, rather than trusting a pre-resolved id.
A caller that wants the series cannot get an occurrence by forgetting a ??, and a caller that wants an occurrence has to say so.
findByUid's unexpanded query stops being load-bearing by accident; if it is relied on, it says so in a comment.
Tests covering both scopes, including that a series call never sends a synthetic id.

Best done before #132 rather than during it: that issue adds the first callers that legitimately pass synthetic ids, and doing both at once means the change that introduces the ambiguity is also the one that has to resolve it.

_{Rebuilt from: session transcript.}

`web/src/store/calendar.ts` sends whatever id it is handed: ```ts async updateEvent(id, patch, sendInvites) { ... update: { [id]: patch } ... } async destroyEvent(id, sendInvites) { ... destroy: [id] ... } ``` The `baseEventId ?? id` resolution that makes those calls hit the series lives at the call sites instead — four of them: - `web/src/views/calendar/EventPopover.tsx` — `const baseId = ev.baseEventId ?? ev.id` - `web/src/views/calendar/CalendarContextMenu.tsx` — same line, feeding both `patch()` and `del()` - `web/src/views/calendar/EventEditor.tsx` — loads the base with `cal.getEvent(init.event.baseEventId)` and edits that - `web/src/views/mail/InviteCard.tsx` — passes `existing.id`, which is safe only because `findByUid` queries without `expandRecurrences` and therefore returns a base id All four are correct today. The problem is what happens when one of them stops being correct. ## Stalwart 0.16.20 removed the guard that caught a slip Through 0.16.19, a synthetic id reaching `destroy` came back as a hard error — *"Deleting synthetic ids is not yet supported"* — and the toast said so. A mistake at a call site was loud. As of 0.16.20 ([`dfb157d4`](https://github.com/stalwartlabs/stalwart/commit/dfb157d4)) the same call **silently deletes one occurrence and reports success**, while the confirm dialog that preceded it said *"Delete all occurrences?"*. The update path changed the same way: a synthetic id that used to be refused now quietly writes a `recurrenceOverrides` entry instead of touching the series. Nothing is broken right now — this is not a bug report against `main`. It is that an invariant which was previously backstopped by the server is now enforced only by four lines in four files, at exactly the moment #132 wants to start sending synthetic ids on purpose. ## What to do Make the scope explicit at the boundary rather than incidental at the caller: - [ ] `updateEvent` and `destroyEvent` take the target scope as an argument — series or occurrence — and resolve the id themselves from the event, rather than trusting a pre-resolved id. - [ ] A caller that wants the series cannot get an occurrence by forgetting a `??`, and a caller that wants an occurrence has to say so. - [ ] `findByUid`'s unexpanded query stops being load-bearing by accident; if it is relied on, it says so in a comment. - [ ] Tests covering both scopes, including that a series call never sends a synthetic id. Best done before #132 rather than during it: that issue adds the first callers that legitimately pass synthetic ids, and doing both at once means the change that introduces the ambiguity is also the one that has to resolve it. <sub>Rebuilt from: session transcript.</sub>

This repo is archived. You cannot comment on issues.

1 Participants

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: coffey-labs/ihasmail-github-archive#133