fix: keep occurrence alert edits on 1.0 and count the organizer

Fixes from a second review of the Stalwart 1.0 calendar work. - Changing the alerts of one occurrence of a series that uses the calendar's default alerts said "Event updated" and kept the default on 1.0: while a series uses them, 1.0 shows the defaults in place of an occurrence's own alerts and refuses `useDefaultAlerts` on a single occurrence. The series now stops using the defaults first, keeping the alerts it shows under ids of their own, and then the occurrence's alerts go out. 0.16.23 showed the edit already and still does. - The participant limit counts the organizer entry a save adds (live on 1.0: 19 attendees and the organizer are accepted, 20 are refused), so the editor warned one attendee late. It warns at the limit now, and the warning and the refusal say the organizer counts, with plural forms in every locale. - Adding or answering an invitation from the mail view, and applying a proposal, said "could not complete" when the server refused the event: the import swallowed its SetError. The banner now asks the import to reject with the refusal and names the participant limit like the calendar does. - A range load that finished after a later one could put back its own events, range or failure. Only the latest load writes the store, and a load for an account the user left is dropped. - A search filter with an OR, a NOT or a condition of several keys next to its date bounds goes out as it is instead of being expanded, which 1.0 refuses. - A time zone setting typed in another letter case ("europe/berlin") is used as Intl spells it. 1.0 refused it as a query argument, and 0.16 and 1.0 both dropped it from new events, which were stored floating. - A failed save no longer shows the store's English toast next to the translated message. - Tests for a limited read that asks later chunks of a request again after a refused chunk or a full page.
2026-11-02 16:15:39 +00:00 · 2026-09-25 21:21:28 +02:00
parent fabecb2128
commit a8cccf1ad1
38 changed files with 597 additions and 88 deletions
@@ -410,6 +410,112 @@ describe('expanded ranges longer than maxExpandedQueryDuration', () => {
    await expect(client.queryCalendarEvents(WINDOW)).rejects.toThrow(/single FilterCondition/);
    expect(server.queries()).toHaveLength(3);
  });
+
+  // 1.0 expands recurrences only for one AND of single conditions; an OR,
+  // a NOT or a condition of several keys next to the bounds is refused.
+  it.each<[string, Record<string, unknown>]>([
+    ['an OR', { operator: 'OR', conditions: [{ text: 'budget' }, { text: 'review' }] }],
+    ['a condition of two keys', { title: 'budget', description: 'review' }],
+  ])('sends an AND holding %s as it is, not expanded, on Stalwart 1.0', async (_label, other) => {
+    await connect('1.0');
+    const filter = { operator: 'AND' as const, conditions: [{ after: WINDOW.after }, { before: WINDOW.before }, other] };
+
+    await client.queryCalendarEvents(filter);
+
+    expect(expandedQueries()).toHaveLength(0);
+    expect(server.queries().map((q) => q.args.filter)).toEqual([filter]);
+  });
+});
+
+// With a limit, the chunks of a long range go out a few per request in the
+// sort order. When one of them needs another query - it was refused and is
+// halved, or it filled its page with an occurrence found before - the chunks
+// after it in that request are asked again after it, and what they returned
+// is dropped: the result keeps the sort order.
+describe('limited reads of several chunks per request', () => {
+  // Chunks of 99 days from 2025-01-01, four per request.
+  const WINDOW = { after: '2025-01-01T00:00:00', before: '2027-02-16T23:59:59' };
+  let fetchSpy: ReturnType<typeof vi.spyOn>;
+  let client: JMAPClient;
+  let server: StalwartCalendarMock;
+
+  async function connect(version: StalwartVersion, occurrences: RangeOccurrence[], maxInstances?: number) {
+    fetchSpy = vi.spyOn(globalThis, 'fetch');
+    client = await connectCalendarClient(fetchSpy, stalwartCalendarSession(version, { maxExpandedQueryDuration: 'P100D' }));
+    server = installStalwartCalendarMock(fetchSpy, {
+      version,
+      objects: {
+        series: {
+          id: 'series', '@type': 'Event', uid: 'series@example.com', title: 'Standup', baseEventId: 'series',
+          start: '2025-01-02T09:00:00', duration: 'PT1H', recurrenceRule: { frequency: 'daily' },
+          recurrenceOverrides: null, calendarIds: { cal: true },
+        },
+      },
+      occurrences: occurrenceObjects('series', occurrences),
+      answer: expandingQueryAnswer(version, { seriesId: 'series', occurrences, maxInstances }),
+    });
+  }
+
+  afterEach(() => {
+    client.disconnect();
+    fetchSpy.mockRestore();
+  });
+
+  const byRequest = () => {
+    const requests = new Map<number, Array<{ after: string; before: string; position: unknown }>>();
+    for (const q of server.queries().filter((query) => query.args.expandRecurrences)) {
+      const filter = q.args.filter as { after: string; before: string };
+      requests.set(q.request, [...(requests.get(q.request) ?? []), { after: filter.after, before: filter.before, position: q.args.position }]);
+    }
+    return [...requests.values()];
+  };
+
+  it('asks again after a chunk that filled its page with an occurrence found before on Stalwart 1.0', async () => {
+    await connect('1.0', [
+      { id: 'o-a', start: '2025-01-02T09:00:00', seconds: 3600 },
+      // Across the border of the fourth and fifth chunk: both find it.
+      { id: 'o-b', start: '2026-01-31T23:30:00', seconds: 3600 },
+      { id: 'o-c', start: '2026-03-01T09:00:00', seconds: 3600 },
+      { id: 'o-d', start: '2026-04-01T09:00:00', seconds: 3600 },
+    ]);
+
+    const events = await client.queryCalendarEvents(WINDOW, [{ property: 'start', isAscending: true }], 3);
+
+    const [first, second, third] = byRequest();
+    expect(first).toHaveLength(4);
+    // The fifth chunk's page of one is o-b again: it is asked on from
+    // there, with the three chunks after it.
+    expect(second).toHaveLength(4);
+    expect(third).toHaveLength(4);
+    expect(third[0]).toEqual({ ...second[0], position: 1 });
+    expect(third.slice(1)).toEqual(second.slice(1));
+    expect(events.map((e) => e.id)).toEqual(['o-a', 'o-b', 'o-c']);
+  });
+
+  it.each<[StalwartVersion]>([['0.16'], ['1.0']])(
+    'asks the rest of a request again after the halves of a refused chunk on Stalwart %s',
+    async (version) => {
+      // The first chunk expands to three occurrences, one more than allowed.
+      await connect(version, [
+        { id: 'o-jan', start: '2025-01-02T09:00:00', seconds: 3600 },
+        { id: 'o-feb', start: '2025-02-02T09:00:00', seconds: 3600 },
+        { id: 'o-mar', start: '2025-03-02T09:00:00', seconds: 3600 },
+        { id: 'o-may', start: '2025-05-01T09:00:00', seconds: 3600 },
+      ], 2);
+
+      const events = await client.queryCalendarEvents(WINDOW, [{ property: 'start', isAscending: true }], 4);
+
+      const [first, second] = byRequest();
+      expect(first).toHaveLength(4);
+      // The halves of the first chunk, then the second and third chunks again.
+      expect(second).toHaveLength(4);
+      expect(second[0].after).toBe(first[0].after);
+      expect(second[1].before).toBe(first[0].before);
+      expect(second.slice(2)).toEqual(first.slice(1, 3));
+      expect(byRequest()).toHaveLength(2);
+      expect(events.map((e) => e.id)).toEqual(['o-jan', 'o-feb', 'o-mar', 'o-may']);
+    },
+  );
 });

 // Stalwart 1.0 fails every CalendarEvent get and query whose `timeZone` its
@@ -523,6 +629,17 @@ describe('a time zone the server does not know', () => {
    expect(warn).not.toHaveBeenCalled();
  });

+  it('sends a zone typed in another letter case as Intl spells it on Stalwart 1.0', async () => {
+    await connect('1.0');
+    useSettingsStore.setState({ timeZone: 'europe/berlin' });
+
+    await client.getAllCalendars();
+    await client.queryCalendarEvents(GRID);
+
+    expect(zones().every((zone) => zone === 'Europe/Berlin')).toBe(true);
+    expect(warn).not.toHaveBeenCalled();
+  });
+
  it('sends a zone Stalwart 1.0 knows as it is', async () => {
    await connect('1.0');
    useSettingsStore.setState({ timeZone: 'Europe/Berlin' });
@@ -9,6 +9,7 @@ import {
  getWallClock,
  isDisplayToday,
  isValidTimeZone,
+  canonicalTimeZone,
  listTimeZones,
  moveLocalDateTime,
  resolveTimeZone,
@@ -48,6 +49,14 @@ describe('resolveTimeZone', () => {
    expect(resolveTimeZone('Asia/Tokyo', 'Europe/Berlin')).toBe('Asia/Tokyo');
  });

+  // Stalwart 1.0 refuses "europe/berlin" as a query's timeZone, and both
+  // 0.16 and 1.0 drop it from an event, storing the event as floating.
+  it('spells a zone typed in another case the way Intl does', () => {
+    expect(resolveTimeZone('europe/berlin', 'UTC')).toBe('Europe/Berlin');
+    expect(canonicalTimeZone('AMERICA/NEW_YORK')).toBe('America/New_York');
+    expect(canonicalTimeZone('Mars/Olympus_Mons')).toBeNull();
+  });
+
  it('validates zone ids against Intl', () => {
    expect(isValidTimeZone('America/New_York')).toBe(true);
    expect(isValidTimeZone('Not/A_Zone')).toBe(false);
@@ -25,7 +25,7 @@ export { SchedulingDeniedError };
 import { acceptedFileName, fileNameRulesFrom, type FileNameRules } from "@/lib/file-name-rules";
 import { folderDepthError } from "@/lib/file-errors";
 import { mediaTypeEssence, OCTET_STREAM } from "@/lib/media-type";
-import { getEffectiveTimeZone, moveLocalDateTime, toLocalDateTime } from "@/lib/timezone";
+import { canonicalTimeZone, getEffectiveTimeZone, moveLocalDateTime, toLocalDateTime } from "@/lib/timezone";
 import { buildEmailSort, compareEmails, hasKeywordLevels, hasTextLevels, type KeywordSortPolarity, type SortLevel } from "@/lib/message-list-order";
 import type { AccountInfo } from "./types";
 import { stalwartChangeId } from "./stalwart-state";
@@ -612,11 +612,14 @@ function normalizeCalendarTask(task: CalendarTask): CalendarTask {
 * shifts range boundaries and floating-event times for any user not in UTC.
 * A zone the server's time zone database lacks is read as UTC by 0.16 and
 * refused by 1.0, which fails the whole call; JMAPClient.calendarTimeZone
- * decides what is actually sent.
+ * decides what is actually sent. The name goes out as Intl spells it: 1.0
+ * matches it exactly, so a setting typed "europe/berlin" would read as
+ * unknown.
 */
 function getUserTimeZone(): string | undefined {
  try {
-    return getEffectiveTimeZone() || undefined;
+    const zone = getEffectiveTimeZone();
+    return (zone && canonicalTimeZone(zone)) || undefined;
  } catch {
    return undefined;
  }
@@ -858,10 +861,14 @@ interface QueryRange {
 * conditions of a top-level AND - how global search sends a range with
 * several text parts. Rebuilt, such an AND stays flat: Stalwart 1.0 expands
 * recurrences only for a single FilterCondition, and reads a condition of
- * two keys as an AND of its own.
+ * two keys as an AND of its own. An AND that holds anything else - an OR, a
+ * NOT, a condition of several keys - is no range: 1.0 would refuse to expand
+ * it, so it goes out as it is.
 */
 function rangeOf(filter: CalendarEventFilter): QueryRange {
  if (filter.operator === 'AND' && Array.isArray(filter.conditions)) {
+    const plain = (condition: CalendarEventFilter) => !condition.operator && Object.keys(condition).length === 1;
+    if (!filter.conditions.every(plain)) return { withBounds: () => ({ ...filter }) };
    let after: string | undefined;
    let before: string | undefined;
    const rest = filter.conditions.filter((condition) => {
@@ -63,15 +63,37 @@ export function isValidTimeZone(tz: unknown): tz is string {
  return valid;
 }

+const canonicalCache = new Map<string, string | null>();
+
 /**
- * Resolve a stored `timeZone` setting to a concrete IANA zone. `auto`, empty
- * or unknown values (a zone synced from a browser with a newer tz database,
- * a hand-edited import) fall back to the browser zone instead of throwing
- * later inside Intl.
+ * `tz` as `Intl` spells it ("europe/berlin" -> "Europe/Berlin"), or null when
+ * `Intl` rejects it. Intl takes a zone in any letter case; Stalwart 1.0
+ * matches the name exactly and refuses one it does not know.
+ */
+export function canonicalTimeZone(tz: string): string | null {
+  let canonical = canonicalCache.get(tz);
+  if (canonical === undefined) {
+    try {
+      canonical = new Intl.DateTimeFormat(undefined, { timeZone: tz }).resolvedOptions().timeZone || null;
+    } catch {
+      canonical = null;
+    }
+    canonicalCache.set(tz, canonical);
+  }
+  return canonical;
+}
+
+/**
+ * Resolve a stored `timeZone` setting to a concrete IANA zone, spelled the
+ * way Intl spells it. `auto`, empty or unknown values (a zone synced from a
+ * browser with a newer tz database, a hand-edited import) fall back to the
+ * browser zone instead of throwing later inside Intl. A zone in another
+ * letter case would be refused by Stalwart 1.0 as an argument, and dropped
+ * from an event by both 0.16 and 1.0, which stored it as floating.
 */
 export function resolveTimeZone(setting: string | null | undefined, browserTimeZone = getBrowserTimeZone()): string {
  if (!setting || setting === AUTO_TIME_ZONE) return browserTimeZone;
-  return isValidTimeZone(setting) ? setting : browserTimeZone;
+  return canonicalTimeZone(setting) ?? browserTimeZone;
 }

 /** The zone all instants should be displayed in: the user's override, else the browser's. */