Calendar: edit and delete a single occurrence
Closes #132. Stalwart 0.16.20 accepts a synthetic id on `CalendarEvent/set`, writing a `recurrenceOverrides` entry rather than touching the series, so editing one date of a recurring event is now something the server does and this does too. Editing asks the scope *before* the form opens, because it decides which event the form is even about: a form populated from the master shows the series' start date, so editing Wednesday's standup would have offered to move Monday's. Deleting asks in place of the old confirm. The patch is narrowed rather than posted hopefully. 0.16.20 sorts per-occurrence properties into three groups and only one is honest: ten are refused with `invalidProperties`, twelve more are dropped from the patch while the response still reports success, and the rest are applied. That silent 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, reports the second to the caller, and the editor leaves out the five it always sends. A patch that would be entirely dropped is not sent at all. The refusal for an occurrence of a this-and-future change offers the series instead of a bare error toast. Nothing here writes one of those, but an event synced from another client can carry one. Two things the scope prompt cost, both worth knowing. A dialog is queued in a store the moment it is asked for, so it outlives the effect that asked: without a ref guard a remount queues a second prompt the first answer cannot retract. And 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. The mock expands recurrences for the first time, which is what makes any of this developable. It hands out synthetic ids for everything including one-offs, gives occurrences a `recurrenceId` and no rule, and reproduces the refusals - including the silent drops, since a mock that applied them would let a client that sends them look correct everywhere but a real server.
This commit is contained in:
@@ -1,6 +1,6 @@
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { CAP, client } from "@/jmap/client";
|
||||
import { eventIdForScope, isOccurrence, useCalendar } from "@/store/calendar";
|
||||
import { CalendarSetError, eventIdForScope, isOccurrence, isThisAndFutureRefusal, occurrencePatch, OccurrenceScopeError, useCalendar } from "@/store/calendar";
|
||||
import type { CalendarEvent, JmapSession } from "@/jmap/types";
|
||||
|
||||
/**
|
||||
@@ -161,3 +161,71 @@ describe("rsvp", () => {
|
||||
await expect(useCalendar.getState().rsvp(OCCURRENCE, "accepted")).rejects.toThrow(/not a participant/i);
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
describe("occurrencePatch", () => {
|
||||
it("lets through what one date will actually take", () => {
|
||||
const { patch, dropped } = occurrencePatch({ title: "Just today", color: "#f00" });
|
||||
expect(patch).toEqual({ title: "Just today", color: "#f00" });
|
||||
expect(dropped).toEqual([]);
|
||||
});
|
||||
|
||||
it("throws on a property the server refuses outright", () => {
|
||||
// Loud is correct here: moving one occurrence to another calendar is not
|
||||
// something the user can be quietly given a different answer to.
|
||||
expect(() => occurrencePatch({ calendarIds: { c2: true } })).toThrow(OccurrenceScopeError);
|
||||
expect(() => occurrencePatch({ useDefaultAlerts: false })).toThrow(/whole series/i);
|
||||
});
|
||||
|
||||
it("removes an inherited property and reports it, rather than letting it vanish", () => {
|
||||
// The server would take this patch, drop `privacy`, and answer "updated".
|
||||
// Anything that believes the response believes the change landed.
|
||||
const { patch, dropped } = occurrencePatch({ title: "x", privacy: "private", recurrenceRule: null });
|
||||
expect(patch).toEqual({ title: "x" });
|
||||
expect(dropped).toEqual(["privacy", "recurrenceRule"]);
|
||||
});
|
||||
|
||||
it("judges a pointer patch on its first token, as the server does", () => {
|
||||
expect(occurrencePatch({ "participants/me/participationStatus": "accepted" }).patch)
|
||||
.toEqual({ "participants/me/participationStatus": "accepted" });
|
||||
expect(occurrencePatch({ "participants/me/calendarAddress": "mailto:x@y" }).dropped)
|
||||
.toEqual(["participants/me/calendarAddress"]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("updateEvent, per occurrence", () => {
|
||||
it("narrows the patch before sending it and reports what it kept back", async () => {
|
||||
const calls = server();
|
||||
const dropped = await useCalendar.getState().updateEvent(OCCURRENCE, { title: "Just today", privacy: "private" }, false, "occurrence");
|
||||
expect(calls[0]!.update).toEqual({ iaaaaas: { title: "Just today" } });
|
||||
expect(dropped).toEqual(["privacy"]);
|
||||
});
|
||||
|
||||
it("sends nothing at all when a patch is entirely inherited", async () => {
|
||||
// A request that could only be a no-op is worse than no request: the
|
||||
// response would say "updated" and mean nothing by it.
|
||||
const calls = server();
|
||||
const dropped = await useCalendar.getState().updateEvent(OCCURRENCE, { privacy: "private" }, false, "occurrence");
|
||||
expect(calls).toEqual([]);
|
||||
expect(dropped).toEqual(["privacy"]);
|
||||
});
|
||||
|
||||
it("leaves a series patch exactly as the caller wrote it", async () => {
|
||||
const calls = server();
|
||||
await useCalendar.getState().updateEvent(OCCURRENCE, { privacy: "private", useDefaultAlerts: false }, false, "series");
|
||||
expect(calls[0]!.update!.i).toEqual({ privacy: "private", useDefaultAlerts: false });
|
||||
});
|
||||
});
|
||||
|
||||
describe("isThisAndFutureRefusal", () => {
|
||||
it("recognises the refusal worth offering the series for", () => {
|
||||
expect(isThisAndFutureRefusal(new CalendarSetError({
|
||||
type: "invalidProperties",
|
||||
description: "Occurrences of a this-and-future change cannot be modified individually.",
|
||||
}))).toBe(true);
|
||||
});
|
||||
it("does not claim an unrelated refusal", () => {
|
||||
expect(isThisAndFutureRefusal(new CalendarSetError({ type: "forbidden", description: "Nope." }))).toBe(false);
|
||||
expect(isThisAndFutureRefusal(new Error("Occurrences of a this-and-future change"))).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -83,6 +83,95 @@ export function isOccurrence(event: CalendarEvent): boolean {
|
||||
return event.baseEventId != null && event.baseEventId !== event.id;
|
||||
}
|
||||
|
||||
/**
|
||||
* What `CalendarEvent/set` will not take on a single occurrence, and why the
|
||||
* client has to know rather than letting the server sort it out.
|
||||
*
|
||||
* 0.16.20's per-occurrence validator sorts properties into three groups, and
|
||||
* only one of them is honest about itself:
|
||||
*
|
||||
* - **Rejected** — `invalidProperties`, *"This property cannot be modified on a
|
||||
* single occurrence."* Loud, and fine.
|
||||
* - **Inherited** — dropped from the patch, and the response still says the
|
||||
* update succeeded. Nothing anywhere reports it.
|
||||
* - Everything else, which is applied to the override.
|
||||
*
|
||||
* The middle group is the whole problem. It is the same failure as [#26], where
|
||||
* a participant map addressed the RFC 8984 way was discarded without an error
|
||||
* and the client showed the guests as saved: a successful response is not
|
||||
* evidence that anything was written. So a per-occurrence patch is checked here
|
||||
* before it is sent — rejected properties throw, inherited ones are reported to
|
||||
* the caller — rather than being posted hopefully and believed.
|
||||
*
|
||||
* [#26]: https://github.com/Coffey-Labs/ihasmail/issues/26
|
||||
*/
|
||||
const OCCURRENCE_REJECTED = new Set([
|
||||
"baseEventId", "calendarIds", "isDraft", "isOrigin", "utcStart", "utcEnd",
|
||||
"useDefaultAlerts", "mayInviteSelf", "mayInviteOthers", "hideAttendees",
|
||||
]);
|
||||
|
||||
/** Applied to the series and never to one date; dropped in silence if sent. */
|
||||
const OCCURRENCE_INHERITED = new Set([
|
||||
"@type", "method", "organizerCalendarAddress", "privacy", "prodId",
|
||||
"recurrenceId", "recurrenceIdTimeZone", "sentBy", "uid",
|
||||
"recurrenceOverrides", "recurrenceRule", "relatedTo",
|
||||
]);
|
||||
|
||||
/**
|
||||
* A `notUpdated`/`notDestroyed` entry, kept whole rather than flattened.
|
||||
*
|
||||
* Some refusals are worth acting on rather than only showing: 0.16.20 will not
|
||||
* edit an occurrence that belongs to a this-and-future change, and the useful
|
||||
* response to that is to offer the series, which needs the reason and not just
|
||||
* its text.
|
||||
*/
|
||||
export class CalendarSetError extends Error {
|
||||
constructor(readonly setError: { type: string; description?: string; properties?: string[] }) {
|
||||
super(setErrorMessage(setError));
|
||||
this.name = "CalendarSetError";
|
||||
}
|
||||
}
|
||||
|
||||
/** Whether a refusal was "this occurrence belongs to a this-and-future change". */
|
||||
export function isThisAndFutureRefusal(err: unknown): boolean {
|
||||
return err instanceof CalendarSetError && /this-and-future/i.test(err.setError.description ?? "");
|
||||
}
|
||||
|
||||
export class OccurrenceScopeError extends Error {
|
||||
constructor(readonly property: string) {
|
||||
super(`"${property}" applies to the whole series and cannot be changed for one occurrence.`);
|
||||
this.name = "OccurrenceScopeError";
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* A patch narrowed to what one occurrence will actually accept.
|
||||
*
|
||||
* Throws `OccurrenceScopeError` on a property the server would refuse, and
|
||||
* returns the inherited ones it removed so a caller can say what it could not
|
||||
* do for this date alone instead of claiming it did.
|
||||
*
|
||||
* Patch *pointers* are judged on their first token, the way the server does:
|
||||
* `participants/{key}/participationStatus` is allowed, and
|
||||
* `participants/{key}/calendarAddress` is one of the silent drops.
|
||||
*/
|
||||
export function occurrencePatch(patch: Record<string, unknown>): { patch: Record<string, unknown>; dropped: string[] } {
|
||||
const out: Record<string, unknown> = {};
|
||||
const dropped: string[] = [];
|
||||
for (const [key, value] of Object.entries(patch)) {
|
||||
const [head, , third] = key.split("/");
|
||||
const root = head ?? key;
|
||||
if (OCCURRENCE_REJECTED.has(root)) throw new OccurrenceScopeError(root);
|
||||
if (OCCURRENCE_INHERITED.has(root)) { dropped.push(root); continue; }
|
||||
if (root === "participants" && third === "calendarAddress") { dropped.push(key); continue; }
|
||||
// `id` is immutable; the server errors on a value that is not the event's
|
||||
// own, and ignores one that is. Neither is worth sending.
|
||||
if (root === "id") { dropped.push(root); continue; }
|
||||
out[key] = value;
|
||||
}
|
||||
return { patch: out, dropped };
|
||||
}
|
||||
|
||||
/** A calendar somebody else shared, and the account it lives in. */
|
||||
export interface SharedCalendar {
|
||||
accountId: Id;
|
||||
@@ -122,7 +211,8 @@ interface CalendarState {
|
||||
instancesIn(start: Date, end: Date): EventInstance[];
|
||||
getEvent(id: Id): Promise<CalendarEvent | null>;
|
||||
createEvent(event: Partial<CalendarEvent>, calendarId: Id, sendInvites: boolean): Promise<Id>;
|
||||
updateEvent(event: CalendarEvent, patch: Record<string, unknown>, sendInvites: boolean, scope: EventScope): Promise<void>;
|
||||
/** Returns the properties that had to be left to the series, if any. */
|
||||
updateEvent(event: CalendarEvent, patch: Record<string, unknown>, sendInvites: boolean, scope: EventScope): Promise<string[]>;
|
||||
destroyEvent(event: CalendarEvent, sendInvites: boolean, scope: EventScope): Promise<void>;
|
||||
rsvp(event: CalendarEvent, status: "accepted" | "tentative" | "declined", comment?: string): Promise<void>;
|
||||
createCalendar(data: Partial<Calendar>): Promise<Id>;
|
||||
@@ -399,10 +489,15 @@ export const useCalendar = create<CalendarState>((set, get) => ({
|
||||
async updateEvent(event, patch, sendInvites, scope) {
|
||||
const accountId = get().accountId!;
|
||||
const id = eventIdForScope(event, scope);
|
||||
const res = await client.call<SetResponse>("CalendarEvent/set", { accountId, update: { [id]: patch }, sendSchedulingMessages: sendInvites });
|
||||
// An occurrence takes less than the series does, and says so about only
|
||||
// half of it. Narrow the patch here rather than posting it hopefully.
|
||||
const { patch: body, dropped } = scope === "occurrence" ? occurrencePatch(patch) : { patch, dropped: [] as string[] };
|
||||
if (!Object.keys(body).length) return dropped;
|
||||
const res = await client.call<SetResponse>("CalendarEvent/set", { accountId, update: { [id]: body }, sendSchedulingMessages: sendInvites });
|
||||
const err = res.notUpdated?.[id];
|
||||
if (err) throw new Error(setErrorMessage(err));
|
||||
if (err) throw new CalendarSetError(err);
|
||||
get().invalidate();
|
||||
return dropped;
|
||||
},
|
||||
|
||||
async destroyEvent(event, sendInvites, scope) {
|
||||
@@ -410,7 +505,7 @@ export const useCalendar = create<CalendarState>((set, get) => ({
|
||||
const id = eventIdForScope(event, scope);
|
||||
const res = await client.call<SetResponse>("CalendarEvent/set", { accountId, destroy: [id], sendSchedulingMessages: sendInvites });
|
||||
const err = res.notDestroyed?.[id];
|
||||
if (err) throw new Error(setErrorMessage(err));
|
||||
if (err) throw new CalendarSetError(err);
|
||||
set((s) => {
|
||||
const events = { ...s.events };
|
||||
// Drop both ids: the one that was sent, and the object as the caller
|
||||
|
||||
Reference in New Issue
Block a user