coffey-labs/ihasmail-github-archive

Archived

Fork 0

Calendar: edit and delete a single occurrence #137

New Issue

Closed

opened 2026-08-31 04:25:50 +00:00 by jcoffey-dev · 1 comment

jcoffey-dev commented

2026-08-31 04:25:50 +00:00

Owner

Closes #132. Stacked on #135 — based on calendar-event-scope, so review that first and this diff stays small. Retarget to main once #135 lands.

Stalwart 0.16.20 accepts a synthetic id on CalendarEvent/set, writing a recurrenceOverrides entry instead of touching the series. Editing one date of a recurring event is now something the server does, so it is something ihasmail does.

Asking the scope, and when

Deleting asks in place of the old confirm. Editing asks before the form opens, because the answer decides which event the form is about — a form populated from the master shows the series' start date, so editing Wednesday's standup would have offered to move Monday's. Answer "this occurrence" and the form loads the occurrence, shows its own date, and hides the controls that belong to the series (repeat, calendar, visibility).

The patch is narrowed, not posted hopefully

0.16.20 sorts per-occurrence properties into three groups and only one is honest about itself:

	behaviour
`baseEventId`, `calendarIds`, `isDraft`, `isOrigin`, `utcStart`, `utcEnd`, `useDefaultAlerts`, `mayInviteSelf`, `mayInviteOthers`, `hideAttendees`	refused, `invalidProperties`
`@type`, `method`, `organizerCalendarAddress`, `privacy`, `prodId`, `recurrenceId`, `recurrenceIdTimeZone`, `sentBy`, `uid`, `recurrenceOverrides`, `recurrenceRule`, `relatedTo`	dropped, response still says "updated"
everything else	applied to the override

That middle group is how #26 reached a live server: a successful response is not evidence anything was written. So occurrencePatch throws on the first group, returns the second to the caller so the toast can say what was left to the series, and the editor omits the five it always sends (useDefaultAlerts, calendarIds, recurrenceRule, privacy, organizerCalendarAddress). A patch that would be entirely dropped is not sent at all — a request whose response can only be a meaningless "updated" is worse than no request.

An occurrence of a this-and-future change is refused; that one offers the series rather than a bare error toast. Nothing here writes such an override, but an event synced from another client can carry one.

Two things the scope prompt cost

Both found by driving it, not by reading it:

A dialog is queued in a store the moment it is requested, so it outlives the effect that asked for it. Without a ref guard, a remount queues a second prompt that the first answer cannot retract — StrictMode's double-invoke is just the first way to notice.
Gating the answer on the effect's cleanup flag is worse. StrictMode runs mount → cleanup → mount, so the flag is already set by the time anyone clicks and the editor never opens at all. The ref makes it once; the answer applies whenever it arrives.

The mock expands recurrences now

It never did, so none of this was developable against it. server/src/mock/recurrence.ts gives synthetic ids to everything including one-offs (which is why baseEventId proves nothing about a series), gives occurrences a recurrenceId and no rule, and reproduces the refusals — the silent drops included, since a mock that applied them would let a client that sends them look correct everywhere except a real server.

Also here: the colour picker goes

Second commit, unrelated to #132 and easy to split out if you would rather. A category already carries a colour, so a separate picker beside it was two ways to say one thing — and they could disagree, since an explicit colour wins over the category's in eventColor. The swatch grid goes; Clear custom colour stays and appears only when there is one to clear, so an event that already has one (set before this, or by CalDAV) is not stuck ignoring its category with nothing on the menu to explain it.

Verified

387 web tests, 104 server (up from 378/90), build clean. Driven end to end against the mock in a real browser: the weekday standup expands to five dates; deleting Wednesday alone leaves four; colouring Thursday alone leaves the rest; renaming Friday alone sticks; then renaming the series propagates to Monday, Tuesday and Thursday while Thursday keeps its colour, Friday keeps its title and Wednesday stays deleted. Overrides survived a full reload.

Not yet verified live against 0.16.20 — that needs an account on the instance, and per the issue nothing here counts as confirmed on the mock alone.

Merged 2026-08-30 as coffey-labs/ihasmail@362bd8b282

_{Rebuilt from: git history, session transcript.}

Closes #132. **Stacked on #135** — based on `calendar-event-scope`, so review that first and this diff stays small. Retarget to `main` once #135 lands. Stalwart 0.16.20 accepts a synthetic id on `CalendarEvent/set`, writing a `recurrenceOverrides` entry instead of touching the series. Editing one date of a recurring event is now something the server does, so it is something ihasmail does. ## Asking the scope, and when Deleting asks in place of the old confirm. Editing asks **before the form opens**, because the answer decides which event the form is about — a form populated from the master shows the *series'* start date, so editing Wednesday's standup would have offered to move Monday's. Answer "this occurrence" and the form loads the occurrence, shows its own date, and hides the controls that belong to the series (repeat, calendar, visibility). ## The patch is narrowed, not posted hopefully 0.16.20 sorts per-occurrence properties into three groups and only one is honest about itself: | | behaviour | |---|---| | `baseEventId`, `calendarIds`, `isDraft`, `isOrigin`, `utcStart`, `utcEnd`, `useDefaultAlerts`, `mayInviteSelf`, `mayInviteOthers`, `hideAttendees` | refused, `invalidProperties` | | `@type`, `method`, `organizerCalendarAddress`, `privacy`, `prodId`, `recurrenceId`, `recurrenceIdTimeZone`, `sentBy`, `uid`, `recurrenceOverrides`, `recurrenceRule`, `relatedTo` | **dropped, response still says "updated"** | | everything else | applied to the override | That middle group is how #26 reached a live server: a successful response is not evidence anything was written. So `occurrencePatch` throws on the first group, returns the second to the caller so the toast can say what was left to the series, and the editor omits the five it always sends (`useDefaultAlerts`, `calendarIds`, `recurrenceRule`, `privacy`, `organizerCalendarAddress`). A patch that would be entirely dropped is not sent at all — a request whose response can only be a meaningless "updated" is worse than no request. An occurrence of a this-and-future change is refused; that one offers the series rather than a bare error toast. Nothing here writes such an override, but an event synced from another client can carry one. ## Two things the scope prompt cost Both found by driving it, not by reading it: 1. A dialog is queued in a store the moment it is requested, so it **outlives the effect that asked for it**. Without a ref guard, a remount queues a second prompt that the first answer cannot retract — StrictMode's double-invoke is just the first way to notice. 2. Gating the *answer* on the effect's cleanup flag is worse. StrictMode runs mount → cleanup → mount, so the flag is already set by the time anyone clicks and **the editor never opens at all**. The ref makes it once; the answer applies whenever it arrives. ## The mock expands recurrences now It never did, so none of this was developable against it. `server/src/mock/recurrence.ts` gives synthetic ids to everything including one-offs (which is why `baseEventId` proves nothing about a series), gives occurrences a `recurrenceId` and no rule, and reproduces the refusals — the silent drops included, since a mock that applied them would let a client that sends them look correct everywhere except a real server. ## Also here: the colour picker goes Second commit, unrelated to #132 and easy to split out if you would rather. A category already carries a colour, so a separate picker beside it was two ways to say one thing — and they could disagree, since an explicit colour wins over the category's in `eventColor`. The swatch grid goes; **Clear custom colour** stays and appears only when there is one to clear, so an event that already has one (set before this, or by CalDAV) is not stuck ignoring its category with nothing on the menu to explain it. ## Verified 387 web tests, 104 server (up from 378/90), build clean. Driven end to end against the mock in a real browser: the weekday standup expands to five dates; deleting Wednesday alone leaves four; colouring Thursday alone leaves the rest; renaming Friday alone sticks; then renaming **the series** propagates to Monday, Tuesday and Thursday while Thursday keeps its colour, Friday keeps its title and Wednesday stays deleted. Overrides survived a full reload. **Not yet verified live against 0.16.20** — that needs an account on the instance, and per the issue nothing here counts as confirmed on the mock alone. **Merged** 2026-08-30 as coffey-labs/ihasmail@362bd8b282ab <sub>Rebuilt from: git history, session transcript.</sub>

jcoffey-dev commented

2026-08-31 04:39:47 +00:00

Author

Owner

Now verified live against 0.16.20, and it found two things the mock had wrong

Probed the running instance directly through the authenticated JMAP proxy, on one throwaway five-week series created and deleted for the purpose (nothing pre-existing was touched).

Confirmed as modelled: synthetic ids over a base with recurrenceId and no rule; a legal patch landing on the override with start/duration filled in by the server; useDefaultAlerts refused with "This property cannot be modified on a single occurrence."; privacy dropped while the response still said updated; a destroy removing one date and leaving the series; and a base + instance in one request refused together with "A base event and its instances cannot be modified in the same request."

Two things it got wrong:

1. Synthetic ids are renumbered by a write — and a stale one is wrong, not invalid

A five-week series held e i m q u over 03-01 … 03-29. After one override was written to 03-08, the same five ids addressed:

id	before	after
`eaaaaa2`	03-01	03-01
`iaaaaa2`	03-08	03-15
`maaaaa2`	03-15	03-29
`qaaaaa2`	03-22	03-08
`uaaaaa2`	03-29	03-22

Nothing was rejected. An id cached across a write silently addresses a different date, so a delete meant for one occurrence removes another — a confident answer about the wrong day, not a notFound anyone would notice.

recurrenceId is stable, because it is the date. updateEvent/destroyEvent now resolve the current id by it immediately before acting, and refuse outright if the date has left the series rather than falling back to the id in hand.

The mock had this backwards — it kept ids stable on purpose, which is to say it agreed with exactly the belief that is wrong. It now renumbers too (a different permutation, same hazard), and a test holds an id across a write and watches it change meaning.

2. A patch of only-inherited properties writes an override that loses the title

The inherited properties are dropped after the server has decided to write an override, so a patch made only of them still writes one — carrying the server-filled start/duration and nothing else:

update { <occurrence>: { "privacy": "private" } }   →  "updated"

…left privacy untouched on the series and left that date with no title at all.

Sending nothing when narrowing empties a patch was written into this PR as a principle — a request whose response could only be a meaningless "updated" is worse than no request — before I knew it prevented real data loss. It does.

Both recorded in KNOWN-ISSUES with their confirmation dates. 388 web + 106 server tests, build clean. Rebased onto main and retargeted now that #135 has landed.

## Now verified live against 0.16.20, and it found two things the mock had wrong Probed the running instance directly through the authenticated JMAP proxy, on one throwaway five-week series created and deleted for the purpose (nothing pre-existing was touched). **Confirmed as modelled:** synthetic ids over a base with `recurrenceId` and no rule; a legal patch landing on the override with `start`/`duration` filled in by the server; `useDefaultAlerts` refused with *"This property cannot be modified on a single occurrence."*; `privacy` dropped while the response still said `updated`; a destroy removing one date and leaving the series; and a base + instance in one request refused together with *"A base event and its instances cannot be modified in the same request."* **Two things it got wrong:** ### 1. Synthetic ids are renumbered by a write — and a stale one is *wrong*, not invalid A five-week series held `e i m q u` over 03-01 … 03-29. After **one** override was written to 03-08, the same five ids addressed: | id | before | after | |---|---|---| | `eaaaaa2` | 03-01 | 03-01 | | `iaaaaa2` | 03-08 | **03-15** | | `maaaaa2` | 03-15 | **03-29** | | `qaaaaa2` | 03-22 | **03-08** | | `uaaaaa2` | 03-29 | **03-22** | Nothing was rejected. An id cached across a write silently addresses a different date, so a delete meant for one occurrence removes another — a confident answer about the wrong day, not a `notFound` anyone would notice. `recurrenceId` is stable, because it *is* the date. `updateEvent`/`destroyEvent` now resolve the current id by it immediately before acting, and refuse outright if the date has left the series rather than falling back to the id in hand. The mock had this backwards — it kept ids stable **on purpose**, which is to say it agreed with exactly the belief that is wrong. It now renumbers too (a different permutation, same hazard), and a test holds an id across a write and watches it change meaning. ### 2. A patch of only-inherited properties writes an override that loses the title The inherited properties are dropped *after* the server has decided to write an override, so a patch made only of them still writes one — carrying the server-filled `start`/`duration` and nothing else: ``` update { <occurrence>: { "privacy": "private" } } → "updated" ``` …left `privacy` untouched on the series and left that date **with no title at all**. Sending nothing when narrowing empties a patch was written into this PR as a principle — *a request whose response could only be a meaningless "updated" is worse than no request* — before I knew it prevented real data loss. It does. --- Both recorded in KNOWN-ISSUES with their confirmation dates. 388 web + 106 server tests, build clean. Rebased onto `main` and retargeted now that #135 has landed.

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#137