diff --git a/FEATURES.md b/FEATURES.md index eaed6a5..6e19c06 100644 --- a/FEATURES.md +++ b/FEATURES.md @@ -546,6 +546,24 @@ nothing for anybody else. - **iCal import** through `CalendarEvent/parse` (a file of any number of events), from the calendar's own menu, into that calendar. The events are filed rather than scheduled: no invitations go out to anyone named in them. +- **Re-importing updates rather than duplicates**, as a contacts import does. + An event is recognised by its UID, per calendar, and what the file carries + wins -- so a corrected export corrects what the first attempt got wrong. + + Two things are deliberately left alone: **who accepted**, and **edits to a + single occurrence**. Both are answers and decisions taken here after the file + was written, and a file that mentions them at all describes them as they were + at export, so writing either one over would throw away work silently and + return no error anywhere. A corrected export therefore fixes the time, the + title and the location, and leaves the RSVPs and the "just this Wednesday" + changes where they are. + + The cost runs both ways and is worth knowing. An attendee added at the source + since the last import does not arrive, because nothing here can tell that + apart from an answer given in ihasmail. And an import still sends no + scheduling messages, so an event a re-import moves is moved *here* -- + everybody else's copy still says the old time until whoever is organising + sends the update from the event itself. - **Subscribed calendars** by URL — a timetable, a rota, a public holiday list. Added in Settings › Calendar & contacts, read-only, and shown beside your own with their own colour. diff --git a/web/src/locales/de.ts b/web/src/locales/de.ts index 1ab3d95..0fd8fa6 100644 --- a/web/src/locales/de.ts +++ b/web/src/locales/de.ts @@ -1407,8 +1407,7 @@ export const catalog: Catalog = { "Your administrator changed {n} settings": { one: "Ihre Administration hat {n} Einstellung geändert", other: "Ihre Administration hat {n} Einstellungen geändert" }, "Exported {n} events": { one: "{n} Termin exportiert", other: "{n} Termine exportiert" }, "Imported {n} events": { one: "{n} Termin importiert", other: "{n} Termine importiert" }, - "Already here: {n} events, nothing imported": { one: "Bereits vorhanden: {n} Termin, nichts importiert", other: "Bereits vorhanden: {n} Termine, nichts importiert" }, - "{n} were already here": { one: "{n} war bereits vorhanden", other: "{n} waren bereits vorhanden" }, + "Updated {n} events, nothing new": { one: "{n} Termin aktualisiert, nichts Neues", other: "{n} Termine aktualisiert, nichts Neues" }, "{n} messages": { one: "{n} Nachricht", other: "{n} Nachrichten" }, "{n} selected": { one: "{n} ausgewählt", other: "{n} ausgewählt" }, "{n} conversations": { one: "{n} Konversation", other: "{n} Konversationen" }, diff --git a/web/src/locales/es.ts b/web/src/locales/es.ts index 7fb1ce0..6f07709 100644 --- a/web/src/locales/es.ts +++ b/web/src/locales/es.ts @@ -1380,8 +1380,7 @@ export const catalog: Catalog = { "Your administrator changed {n} settings": { one: "Tu administración cambió {n} ajuste", other: "Tu administración cambió {n} ajustes" }, "Exported {n} events": { one: "{n} evento exportado", other: "{n} eventos exportados" }, "Imported {n} events": { one: "{n} evento importado", other: "{n} eventos importados" }, - "Already here: {n} events, nothing imported": { one: "Ya estaba aquí: {n} evento, no se importó nada", other: "Ya estaban aquí: {n} eventos, no se importó nada" }, - "{n} were already here": { one: "{n} ya estaba aquí", other: "{n} ya estaban aquí" }, + "Updated {n} events, nothing new": { one: "{n} evento actualizado, nada nuevo", other: "{n} eventos actualizados, nada nuevo" }, "{n} messages": { one: "{n} mensaje", other: "{n} mensajes" }, "{n} selected": { one: "{n} seleccionado", other: "{n} seleccionados" }, "{n} conversations": { one: "{n} conversación", other: "{n} conversaciones" }, diff --git a/web/src/locales/fr.ts b/web/src/locales/fr.ts index de8f10a..820e1db 100644 --- a/web/src/locales/fr.ts +++ b/web/src/locales/fr.ts @@ -1385,8 +1385,7 @@ export const catalog: Catalog = { "Your administrator changed {n} settings": { one: "Votre administration a modifié {n} paramètre", other: "Votre administration a modifié {n} paramètres" }, "Exported {n} events": { one: "{n} événement exporté", other: "{n} événements exportés" }, "Imported {n} events": { one: "{n} événement importé", other: "{n} événements importés" }, - "Already here: {n} events, nothing imported": { one: "Déjà présent : {n} événement, rien d’importé", other: "Déjà présents : {n} événements, rien d’importé" }, - "{n} were already here": { one: "{n} était déjà présent", other: "{n} étaient déjà présents" }, + "Updated {n} events, nothing new": { one: "{n} événement mis à jour, rien de nouveau", other: "{n} événements mis à jour, rien de nouveau" }, "{n} messages": { one: "{n} message", other: "{n} messages" }, "{n} selected": { one: "{n} sélectionné", other: "{n} sélectionnés" }, "{n} conversations": { one: "{n} conversation", other: "{n} conversations" }, diff --git a/web/src/locales/ja.ts b/web/src/locales/ja.ts index 0754f40..3b1f49e 100644 --- a/web/src/locales/ja.ts +++ b/web/src/locales/ja.ts @@ -1388,8 +1388,7 @@ export const catalog: Catalog = { "Your administrator changed {n} settings": { other: "管理者が {n} 件の設定を変更しました" }, "Exported {n} events": { other: "{n} 件の予定をエクスポートしました" }, "Imported {n} events": { other: "{n} 件の予定をインポートしました" }, - "Already here: {n} events, nothing imported": { other: "すでに存在: {n} 件、インポートなし" }, - "{n} were already here": { other: "{n} 件はすでに存在していました" }, + "Updated {n} events, nothing new": { other: "{n} 件の予定を更新しました。新規はありません" }, /* * One form each, because Japanese has one. Intl.PluralRules returns * `other` for every number, so `one`, `few` and `many` would never be diff --git a/web/src/locales/nl.ts b/web/src/locales/nl.ts index 4f31106..e62f744 100644 --- a/web/src/locales/nl.ts +++ b/web/src/locales/nl.ts @@ -1376,8 +1376,7 @@ export const catalog: Catalog = { "Your administrator changed {n} settings": { one: "Uw beheerder heeft {n} instelling gewijzigd", other: "Uw beheerder heeft {n} instellingen gewijzigd" }, "Exported {n} events": { one: "{n} afspraak geëxporteerd", other: "{n} afspraken geëxporteerd" }, "Imported {n} events": { one: "{n} afspraak geïmporteerd", other: "{n} afspraken geïmporteerd" }, - "Already here: {n} events, nothing imported": { one: "Al aanwezig: {n} afspraak, niets geïmporteerd", other: "Al aanwezig: {n} afspraken, niets geïmporteerd" }, - "{n} were already here": { one: "{n} was er al", other: "{n} waren er al" }, + "Updated {n} events, nothing new": { one: "{n} afspraak bijgewerkt, niets nieuws", other: "{n} afspraken bijgewerkt, niets nieuws" }, "{n} messages": { one: "{n} bericht", other: "{n} berichten" }, "{n} selected": { one: "{n} geselecteerd", other: "{n} geselecteerd" }, "{n} conversations": { one: "{n} gesprek", other: "{n} gesprekken" }, diff --git a/web/src/locales/pt-BR.ts b/web/src/locales/pt-BR.ts index cd24d68..64991de 100644 --- a/web/src/locales/pt-BR.ts +++ b/web/src/locales/pt-BR.ts @@ -1383,8 +1383,7 @@ export const catalog: Catalog = { "Your administrator changed {n} settings": { one: "Sua administração alterou {n} configuração", other: "Sua administração alterou {n} configurações" }, "Exported {n} events": { one: "{n} evento exportado", other: "{n} eventos exportados" }, "Imported {n} events": { one: "{n} evento importado", other: "{n} eventos importados" }, - "Already here: {n} events, nothing imported": { one: "Já estava aqui: {n} evento, nada importado", other: "Já estavam aqui: {n} eventos, nada importado" }, - "{n} were already here": { one: "{n} já estava aqui", other: "{n} já estavam aqui" }, + "Updated {n} events, nothing new": { one: "{n} evento atualizado, nada novo", other: "{n} eventos atualizados, nada novo" }, "{n} messages": { one: "{n} mensagem", other: "{n} mensagens" }, "{n} selected": { one: "{n} selecionada", other: "{n} selecionadas" }, "{n} conversations": { one: "{n} conversa", other: "{n} conversas" }, diff --git a/web/src/locales/ru.ts b/web/src/locales/ru.ts index 41baa9d..0e04aed 100644 --- a/web/src/locales/ru.ts +++ b/web/src/locales/ru.ts @@ -1382,8 +1382,7 @@ export const catalog: Catalog = { "Your administrator changed {n} settings": { one: "Администратор изменил {n} настройку", few: "Администратор изменил {n} настройки", many: "Администратор изменил {n} настроек", other: "Администратор изменил {n} настройки" }, "Exported {n} events": { one: "Экспортировано {n} событие", few: "Экспортировано {n} события", many: "Экспортировано {n} событий", other: "Экспортировано {n} события" }, "Imported {n} events": { one: "Импортировано {n} событие", few: "Импортировано {n} события", many: "Импортировано {n} событий", other: "Импортировано {n} события" }, - "Already here: {n} events, nothing imported": { one: "Уже есть: {n} событие, ничего не импортировано", few: "Уже есть: {n} события, ничего не импортировано", many: "Уже есть: {n} событий, ничего не импортировано", other: "Уже есть: {n} события, ничего не импортировано" }, - "{n} were already here": { one: "{n} уже было здесь", few: "{n} уже были здесь", many: "{n} уже были здесь", other: "{n} уже были здесь" }, + "Updated {n} events, nothing new": { one: "Обновлено {n} событие, новых нет", few: "Обновлено {n} события, новых нет", many: "Обновлено {n} событий, новых нет", other: "Обновлено {n} события, новых нет" }, /* * Three forms, which is the whole reason plural() takes a map rather than * (one, other). Intl.PluralRules picks: 1 is `one`, 2-4 are `few`, 5-20 diff --git a/web/src/locales/uk.ts b/web/src/locales/uk.ts index f0bd293..bf7fd3a 100644 --- a/web/src/locales/uk.ts +++ b/web/src/locales/uk.ts @@ -1376,8 +1376,7 @@ export const catalog: Catalog = { "Your administrator changed {n} settings": { one: "Адміністратор змінив {n} налаштування", few: "Адміністратор змінив {n} налаштування", many: "Адміністратор змінив {n} налаштувань", other: "Адміністратор змінив {n} налаштування" }, "Exported {n} events": { one: "Експортовано {n} подію", few: "Експортовано {n} події", many: "Експортовано {n} подій", other: "Експортовано {n} події" }, "Imported {n} events": { one: "Імпортовано {n} подію", few: "Імпортовано {n} події", many: "Імпортовано {n} подій", other: "Імпортовано {n} події" }, - "Already here: {n} events, nothing imported": { one: "Уже є: {n} подія, нічого не імпортовано", few: "Уже є: {n} події, нічого не імпортовано", many: "Уже є: {n} подій, нічого не імпортовано", other: "Уже є: {n} події, нічого не імпортовано" }, - "{n} were already here": { one: "{n} уже була тут", few: "{n} уже були тут", many: "{n} уже були тут", other: "{n} уже були тут" }, + "Updated {n} events, nothing new": { one: "Оновлено {n} подію, нових немає", few: "Оновлено {n} події, нових немає", many: "Оновлено {n} подій, нових немає", other: "Оновлено {n} події, нових немає" }, /* * Ukrainian takes the same three forms as Russian and the same rule, but * not the same words. Sharing a plural structure is not sharing a diff --git a/web/src/locales/zh-Hans.ts b/web/src/locales/zh-Hans.ts index 7032067..c969111 100644 --- a/web/src/locales/zh-Hans.ts +++ b/web/src/locales/zh-Hans.ts @@ -1387,8 +1387,7 @@ export const catalog: Catalog = { "Your administrator changed {n} settings": { other: "管理员更改了 {n} 项设置" }, "Exported {n} events": { other: "已导出 {n} 个日程" }, "Imported {n} events": { other: "已导入 {n} 个日程" }, - "Already here: {n} events, nothing imported": { other: "已存在 {n} 个，未导入" }, - "{n} were already here": { other: "{n} 个已存在" }, + "Updated {n} events, nothing new": { other: "已更新 {n} 个日程，无新增" }, /* * One form each, because Chinese has one. Intl.PluralRules returns `other` * for every number, so `one`, `few` and `many` would never be selected — diff --git a/web/src/store/__tests__/ics-import.test.ts b/web/src/store/__tests__/ics-import.test.ts index f300f0b..5e1e051 100644 --- a/web/src/store/__tests__/ics-import.test.ts +++ b/web/src/store/__tests__/ics-import.test.ts @@ -28,17 +28,18 @@ const PARSED = [ }, ]; -interface SetArgs { create?: Record>; sendSchedulingMessages?: boolean } +interface SetArgs { create?: Record>; update?: Record>; sendSchedulingMessages?: boolean } /** * @param parsed what `CalendarEvent/parse` answers with; a bare object rather * than an array is the single-event shape, which Stalwart also returns. * @param notCreated refusals to hand back instead of creations. + * @param notUpdated refusals to hand back instead of updates. * @param max the ceiling on objects in one call, refused the way Stalwart * refuses it: the whole call, creating nothing. * @param failOn which `/set` call (0-based) answers with an error instead. */ -function server(parsed: unknown, opts: { notCreated?: Record; max?: number; failOn?: number; existing?: Array<{ id: string; uid: string; calendarIds: Record }> } = {}) { +function server(parsed: unknown, opts: { notCreated?: Record; notUpdated?: Record; max?: number; failOn?: number; existing?: Array<{ id: string; uid: string; calendarIds: Record }> } = {}) { const sets: SetArgs[] = []; const existing = opts.existing ?? []; const fetchMock = vi.fn(async (_url: string, init: RequestInit) => { @@ -50,18 +51,28 @@ function server(parsed: unknown, opts: { notCreated?: Record; m } if (name === "CalendarEvent/set") { const nth = sets.length; - sets.push({ create: args.create as Record>, sendSchedulingMessages: args.sendSchedulingMessages as boolean }); + sets.push({ + create: args.create as Record>, + update: args.update as Record>, + sendSchedulingMessages: args.sendSchedulingMessages as boolean, + }); const keys = Object.keys((args.create ?? {}) as object); - // Whole-call refusals, both of them: nothing in this call is created. - if (opts.max != null && keys.length > opts.max) { + const patched = Object.keys((args.update ?? {}) as object); + /* Whole-call refusals, both of them: nothing in this call gets written. + Creates and updates count against the ceiling together, which is why + the store batches them together. */ + if (opts.max != null && keys.length + patched.length > opts.max) { return ["error", { type: "requestTooLarge", description: "The number of ids requested by the client exceeds the maximum number the server is willing to process in a single method call." }, id]; } if (opts.failOn === nth) return ["error", { type: "serverFail", description: "the roof fell in" }, id]; const notCreated = opts.notCreated ?? {}; + const notUpdated = opts.notUpdated ?? {}; return [name, { accountId: "a1", oldState: "1", newState: "2", created: Object.fromEntries(keys.filter((k) => !(k in notCreated)).map((k) => [k, { id: `new-${k}` }])), notCreated, + updated: Object.fromEntries(patched.filter((k) => !(k in notUpdated)).map((k) => [k, null])), + notUpdated, }, id]; } // The scan for UIDs already in the calendar: a query for the account's @@ -122,7 +133,7 @@ describe("importing an .ics file", () => { it("creates every event in one call when the file fits in one, not one call each", async () => { const sets = server(PARSED); const n = await useCalendar.getState().importIcs("x", "cal1"); - expect(n).toEqual({ created: 2, skipped: 0 }); + expect(n).toEqual({ created: 2, updated: 0 }); expect(sets).toHaveLength(1); expect(Object.keys(sets[0]!.create!)).toEqual(["e0", "e1"]); }); @@ -163,7 +174,7 @@ describe("importing an .ics file", () => { it("takes a single event, which is what a one-event file parses to", async () => { const sets = server(PARSED[0]); const n = await useCalendar.getState().importIcs("x", "cal1"); - expect(n).toEqual({ created: 1, skipped: 0 }); + expect(n).toEqual({ created: 1, updated: 0 }); expect(Object.keys(sets[0]!.create!)).toEqual(["e0"]); }); @@ -179,7 +190,7 @@ describe("importing an .ics file", () => { it("counts what got in when only some of it did", async () => { server(PARSED, { notCreated: { e1: { type: "invalidProperties" } } }); - await expect(useCalendar.getState().importIcs("x", "cal1")).resolves.toEqual({ created: 1, skipped: 0 }); + await expect(useCalendar.getState().importIcs("x", "cal1")).resolves.toEqual({ created: 1, updated: 0 }); }); }); @@ -202,14 +213,14 @@ describe("importing a file bigger than the server will take at once", () => { it("splits it into calls the server will accept, and files all of it", async () => { const sets = server(many(1200), { max: MAX }); - await expect(useCalendar.getState().importIcs("x", "cal1")).resolves.toEqual({ created: 1200, skipped: 0 }); + await expect(useCalendar.getState().importIcs("x", "cal1")).resolves.toEqual({ created: 1200, updated: 0 }); expect(sets.map((s) => Object.keys(s.create!).length)).toEqual([500, 500, 200]); }); it("splits by what the session advertises, not by a number of its own", async () => { client.session!.capabilities[CAP.core] = { maxObjectsInGet: 40, maxObjectsInSet: 40 }; const sets = server(many(100), { max: 40 }); - await expect(useCalendar.getState().importIcs("x", "cal1")).resolves.toEqual({ created: 100, skipped: 0 }); + await expect(useCalendar.getState().importIcs("x", "cal1")).resolves.toEqual({ created: 100, updated: 0 }); expect(sets.map((s) => Object.keys(s.create!).length)).toEqual([40, 40, 20]); }); @@ -256,23 +267,59 @@ describe("importing a file bigger than the server will take at once", () => { * whole of what is needed to recognise an event that is already here -- and * nothing looked. Importing an export twice left second copies of everything, * which the reporter's colleague hit during testing (#173, decided there: - * "duplicate checks on UIDs if UID present in event"). Issue #222. + * "duplicate checks on UIDs if UID present in event"). Issue #222 made that a + * skip; #279 made it an update, because the reason to import a file a second + * time is usually that the first one was not right. */ describe("re-importing events the calendar already has", () => { const here = (uid: string, calendarId = "cal1") => ({ id: `srv-${uid}`, uid, calendarIds: { [calendarId]: true } }); - it("skips an event whose uid is already in this calendar", async () => { + it("updates an event whose uid is already in this calendar", async () => { const sets = server(PARSED, { existing: [here("uid-one@example.org")] }); - await expect(useCalendar.getState().importIcs("x", "cal1")).resolves.toEqual({ created: 1, skipped: 1 }); - // Only the second event, which has no uid of its own, was sent. + await expect(useCalendar.getState().importIcs("x", "cal1")).resolves.toEqual({ created: 1, updated: 1 }); + // Only the second event, which has no uid of its own, is new. expect(Object.values(sets[0]!.create!).map((e) => e.title)).toEqual(["Retro (no uid)"]); + // The update is addressed to the event that is here, not to the file's id. + expect(Object.keys(sets[0]!.update!)).toEqual(["srv-uid-one@example.org"]); + expect(sets[0]!.update!["srv-uid-one@example.org"]!.title).toBe("Kickoff"); + }); + + it("holds back the answers and the per-occurrence edits, which live on the event", async () => { + /* + * The one thing #279 turned on. `participants` carries who accepted and + * `recurrenceOverrides` carries every "just this Wednesday" change made + * here; a file describes both as they were at export, so writing either one + * over throws away work with no error anywhere. Everything else in the file + * wins, which is the point of importing it again. + */ + const withPeople = [{ + ...PARSED[0], + title: "Kickoff (moved)", + participants: { "someone@example.org": { "@type": "Participant", participationStatus: "needs-action" } }, + recurrenceOverrides: { "2026-09-09T09:00:00": { title: "Skip" } }, + }]; + const sets = server(withPeople, { existing: [here("uid-one@example.org")] }); + await expect(useCalendar.getState().importIcs("x", "cal1")).resolves.toEqual({ created: 0, updated: 1 }); + const patch = sets[0]!.update!["srv-uid-one@example.org"]!; + expect(patch.title).toBe("Kickoff (moved)"); + expect(patch).not.toHaveProperty("participants"); + expect(patch).not.toHaveProperty("recurrenceOverrides"); + // The identity the two were matched on is not re-asserted as a field. + expect(patch).not.toHaveProperty("uid"); + }); + + it("does not mail anyone about an event it updated", async () => { + // Filing a file is not scheduling, on an update as much as on a create. + const sets = server(PARSED, { existing: [here("uid-one@example.org")] }); + await useCalendar.getState().importIcs("x", "cal1"); + expect(sets[0]!.sendSchedulingMessages).toBe(false); }); it("imports an event whose uid is in a different calendar", async () => { // A UID is what makes an event the same event *across* calendars, so the // same event legitimately being in two of them is not a duplicate. const sets = server(PARSED, { existing: [here("uid-one@example.org", "cal2")] }); - await expect(useCalendar.getState().importIcs("x", "cal1")).resolves.toEqual({ created: 2, skipped: 0 }); + await expect(useCalendar.getState().importIcs("x", "cal1")).resolves.toEqual({ created: 2, updated: 0 }); expect(Object.keys(sets[0]!.create!)).toHaveLength(2); }); @@ -282,12 +329,44 @@ describe("re-importing events the calendar already has", () => { expect(Object.values(sets[0]!.create!)[0]!.uid).toEqual(expect.any(String)); }); - it("sends nothing at all when the whole file is already here", async () => { - // A file whose every event carries a uid the calendar holds: there is - // nothing to create, and nothing wrong either. + it("updates the lot when the whole file is already here, creating nothing", async () => { const both = [PARSED[0], { ...PARSED[1], uid: "uid-two@example.org" }]; const sets = server(both, { existing: [here("uid-one@example.org"), here("uid-two@example.org")] }); - await expect(useCalendar.getState().importIcs("x", "cal1")).resolves.toEqual({ created: 0, skipped: 2 }); - expect(sets).toHaveLength(0); + await expect(useCalendar.getState().importIcs("x", "cal1")).resolves.toEqual({ created: 0, updated: 2 }); + expect(sets).toHaveLength(1); + expect(Object.keys(sets[0]!.create!)).toHaveLength(0); + expect(Object.keys(sets[0]!.update!)).toHaveLength(2); + }); + + it("reports a refusal to update, rather than reporting nothing imported", async () => { + // Everything in the file is already here, so an update is the whole of the + // import -- and a refusal of it is the only thing there is to say. + const both = [PARSED[0], { ...PARSED[1], uid: "uid-two@example.org" }]; + server(both, { + existing: [here("uid-one@example.org"), here("uid-two@example.org")], + notUpdated: { + "srv-uid-one@example.org": { type: "forbidden", description: "the calendar is read-only" }, + "srv-uid-two@example.org": { type: "forbidden" }, + }, + }); + await expect(useCalendar.getState().importIcs("x", "cal1")).rejects.toThrow(/read-only/); + }); + + it("splits creates and updates against one ceiling, not one each", async () => { + /* + * Stalwart counts every object in a `/set` against `maxObjectsInSet` + * together and refuses the whole call over it. 300 new and 300 changed + * batched separately would be two calls of 300 -- neither over 500, both + * refused. + */ + const MAX = 500; + const file = Array.from({ length: 600 }, (_, i) => ({ + "@type": "Event", uid: `uid-${i}@example.org`, title: `Event ${i}`, + start: "2026-09-02T09:00:00", duration: "PT1H", timeZone: "Etc/UTC", + })); + const existing = Array.from({ length: 300 }, (_, i) => here(`uid-${i}@example.org`)); + const sets = server(file, { max: MAX, existing }); + await expect(useCalendar.getState().importIcs("x", "cal1")).resolves.toEqual({ created: 300, updated: 300 }); + expect(sets.map((s) => Object.keys(s.create ?? {}).length + Object.keys(s.update ?? {}).length)).toEqual([500, 100]); }); }); diff --git a/web/src/store/calendar.ts b/web/src/store/calendar.ts index 45ea5f0..a0e56a3 100644 --- a/web/src/store/calendar.ts +++ b/web/src/store/calendar.ts @@ -296,8 +296,8 @@ interface CalendarState { findByUid(uid: string): Promise; parseIcs(blobId: Id): Promise; importEvent(event: Partial, calendarId: Id): Promise; - /** Import a whole .ics file. Says how many it created, and how many were already here. */ - importIcs(text: string, calendarId: Id): Promise<{ created: number; skipped: number }>; + /** Import a whole .ics file. Says how many it created and how many it updated. */ + importIcs(text: string, calendarId: Id): Promise<{ created: number; updated: number }>; /** The whole calendar as one .ics document, and how many events went into it. */ exportIcs(calendarId: Id): Promise<{ text: string; count: number }>; applyChanges(types: Set): void; @@ -333,7 +333,7 @@ function forImport(event: Partial): Partial { } /** - * The UIDs a calendar already holds. + * The events a calendar already holds, for recognising a re-import. * * A UID is what makes an event the same event across calendars, and the import * already keeps the file's own wherever there is one -- so the thing needed to @@ -367,10 +367,21 @@ async function eventsInCalendar(accountId: Id, calendarId: Id, properties: strin return found; } -/** Just the UIDs, for deciding what a re-import would duplicate. */ -async function uidsInCalendar(accountId: Id, calendarId: Id): Promise> { +/** + * uid -> the id of the event carrying it, for deciding what a re-import updates. + * + * The id and not just the UID, because an event already here is now updated + * rather than skipped and updating needs something to address -- the same + * arrangement, and for the same reason, as contacts' `scanBook`. A UID the + * calendar somehow holds twice keeps the first: two events with one UID is + * already a state nothing here can make sense of, and addressing one of them + * is better than writing the file over both. + */ +async function eventIdsByUid(accountId: Id, calendarId: Id): Promise> { const events = await eventsInCalendar(accountId, calendarId, ["uid", "calendarIds"]); - return new Set(events.map((e) => e.uid).filter(Boolean)); + const byUid = new Map(); + for (const e of events) if (e.uid && !byUid.has(e.uid)) byUid.set(e.uid, e.id); + return byUid; } export const useCalendar = create((set, get) => ({ @@ -862,17 +873,24 @@ export const useCalendar = create((set, get) => ({ * year of events one at a time would refetch the calendar a few hundred * times. One invalidate here, after the last batch. * - * No scheduling messages. Importing a file is filing something you already - * have, and mailing its participants would be a surprise to everyone. + * No scheduling messages, on a create or an update. Importing a file is + * filing something you already have, and mailing its participants would be a + * surprise to everyone. That is plainly right for a create and it is a real + * cost on an update -- moving an event without telling anyone leaves every + * attendee's own copy saying the old time, with nothing anywhere reporting + * the disagreement. Weighed on #279 and kept: an import is not the place to + * start sending mail on somebody's behalf, and the alternative is a file + * dropped into a calendar mailing a room full of people who never asked for + * it. Whoever is organising can send the update from the event itself. */ async importIcs(text, calendarId) { const accountId = get().accountId!; const up = await client.upload(accountId, new Blob([text], { type: "text/calendar" }), { type: "text/calendar" }); const events = await get().parseIcs(up.blobId); if (!events.length) throw new Error("it has no events in it"); - const already = await uidsInCalendar(accountId, calendarId); + const already = await eventIdsByUid(accountId, calendarId); const create: Record = {}; - let skipped = 0; + const update: Record = {}; events.forEach((e, i) => { const rest = forImport(e); /* @@ -883,39 +901,78 @@ export const useCalendar = create((set, get) => ({ * about. Re-importing an export used to leave second copies of * everything; asked for on #173, decided there. */ - if (rest.uid && already.has(rest.uid)) { - skipped++; + const existing = rest.uid ? already.get(rest.uid) : undefined; + if (existing) { + /* + * An event this calendar already holds is updated from the file, the + * way a re-imported contact is (#242, #274): the reason to import a + * file a second time is usually that the first one was not right, and + * skipping meant a corrected export corrected nothing. + * + * Two properties are held back, decided on #279. `participants` carries + * every attendee's accepted/declined and `recurrenceOverrides` holds + * every "just this Wednesday" edit made here -- both are answers and + * decisions that happened after the file was written, and a file that + * mentions them at all describes them as they were at export. Writing + * either one over would destroy work nobody asked to lose, silently, + * with no error returned anywhere. So a corrected export fixes the + * time, the title and the location and leaves who said yes alone. + * + * The cost runs the other way: an attendee added at the source since + * the last import does not arrive, and nothing here can tell that apart + * from an RSVP given in ihasmail. Losing an answer somebody gave is + * worse than not gaining an attendee somebody can still be told about. + * + * `uid` is held back too -- it is what the two were matched on, so it + * is already equal, and it is the event's identity rather than a field + * of it worth re-asserting. + */ + const { uid: _u, participants: _p, recurrenceOverrides: _r, ...patch } = rest; + update[existing] = patch; return; } create[`e${i}`] = { "@type": "Event", ...rest, uid: rest.uid || crypto.randomUUID(), calendarIds: { [calendarId]: true } }; }); - // Everything in the file was already here. Nothing to send, and nothing - // wrong either -- say so rather than reporting an import of no events. - if (!Object.keys(create).length) return { created: 0, skipped }; - const keys = Object.keys(create); + /* + * Creates and updates share one budget. Stalwart counts every object in a + * `/set` against `maxObjectsInSet` together, so batching the two separately + * would send a file of 300 new events and 300 changed ones as two calls of + * 300 and be refused for a ceiling of 500 that neither half crosses. + * Contacts' `writeCards` splits the same way for the same reason. + */ + const keys = [ + ...Object.keys(create).map((k) => ["create", k] as const), + ...Object.keys(update).map((k) => ["update", k] as const), + ]; let created = 0; + let updated = 0; let refused: SetError | undefined; try { for (const part of chunk(keys, client.maxObjectsInSet)) { - const sub: Record = {}; - for (const k of part) sub[k] = create[k]; - const res = await client.call>("CalendarEvent/set", { accountId, create: sub, sendSchedulingMessages: false }); + const subCreate: Record = {}; + const subUpdate: Record = {}; + for (const [kind, k] of part) { + if (kind === "create") subCreate[k] = create[k]; + else subUpdate[k] = update[k]; + } + const res = await client.call>("CalendarEvent/set", { accountId, create: subCreate, update: subUpdate, sendSchedulingMessages: false }); created += Object.keys(res.created ?? {}).length; - refused ??= Object.values(res.notCreated ?? {})[0]; + updated += Object.keys(res.updated ?? {}).length; + refused ??= Object.values(res.notCreated ?? {})[0] ?? Object.values(res.notUpdated ?? {})[0]; } } catch (err) { - // A batch that failed with earlier ones already filed: those events are + // A batch that failed with earlier ones already written: those events are // in the calendar, and an error saying only that the import failed sends // someone looking for events that are already there. - if (!created) throw err; - throw new Error(`${created} of ${keys.length} events were imported before this happened: ${(err as Error).message}`); + if (!created && !updated) throw err; + throw new Error(`${created + updated} of ${keys.length} events were imported before this happened: ${(err as Error).message}`); } finally { - if (created) get().invalidate(); + if (created || updated) get().invalidate(); } // Nothing at all got in: say why rather than report importing zero events // as though the file had been empty. - if (!created) throw new Error(refused ? setErrorMessage(refused) : "the server did not accept any of its events"); - return { created, skipped }; + if (!created && !updated) throw new Error(refused ? setErrorMessage(refused) : "the server did not accept any of its events"); + return { created, updated }; }, /* diff --git a/web/src/views/calendar/CalendarSidebar.tsx b/web/src/views/calendar/CalendarSidebar.tsx index 546d26a..7916a33 100644 --- a/web/src/views/calendar/CalendarSidebar.tsx +++ b/web/src/views/calendar/CalendarSidebar.tsx @@ -65,16 +65,19 @@ export function CalendarSidebar() { const calendarId = importInto.current; if (!calendarId) return; try { - const { created, skipped } = await cal.importIcs(await file.text(), calendarId); + const { created, updated } = await cal.importIcs(await file.text(), calendarId); /* - * The two counts are kept apart on purpose. "Imported 40 events" over a - * file of 240 reads as a failure when 200 of them were simply already - * here, and a re-import where everything is already here would otherwise - * report importing nothing at all. + * The two counts are kept apart on purpose, the way the contacts import + * keeps them. "Imported 40 events" over a file of 240 reads as a failure + * when the other 200 were updated, and a re-import of a corrected export + * -- the reason for doing this at all -- creates nothing and would + * otherwise report importing nothing at all. */ - if (!created) toast.success(plural(skipped, { one: "Already here: {n} event, nothing imported", other: "Already here: {n} events, nothing imported" })); - else if (skipped) toast.success(`${plural(created, { one: "Imported {n} event", other: "Imported {n} events" })} · ${plural(skipped, { one: "{n} was already here", other: "{n} were already here" })}`); - else toast.success(plural(created, { one: "Imported {n} event", other: "Imported {n} events" })); + const imported = plural(created, { one: "Imported {n} event", other: "Imported {n} events" }); + const refreshed = plural(updated, { one: "{n} updated", other: "{n} updated" }); + if (!created) toast.success(plural(updated, { one: "Updated {n} event, nothing new", other: "Updated {n} events, nothing new" })); + else if (updated) toast.success(`${imported} · ${refreshed}`); + else toast.success(imported); } catch (err) { toast.error(t("Could not import this file: {error}", { error: (err as Error).message })); }