From cbae82b2f045970254f27b89c65ddcabe66bf6b7 Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Sun, 1 Nov 2026 06:42:37 +0100 Subject: [PATCH] fix: keep the out-of-office event in step when the reply changes on another device The event counted as Bulwark's only while its times matched the record in this browser's settings, so a reply switched off on a phone, by a delegate, in Lite or with settings sync off left colleagues seeing a busy event for a cancelled absence. It now also counts as Bulwark's when it sits on the reply's dates from before the change, and it carries the keyword bulwark-out-of-office so a device without a record takes it over when the reply is loaded. Each account gets its own pending sync. --- lib/__tests__/vacation-calendar.test.ts | 79 ++++++++++++++- lib/vacation-calendar.ts | 122 ++++++++++++++++-------- 2 files changed, 159 insertions(+), 42 deletions(-) diff --git a/lib/__tests__/vacation-calendar.test.ts b/lib/__tests__/vacation-calendar.test.ts index 6b7f4a01c..4a995252c 100644 --- a/lib/__tests__/vacation-calendar.test.ts +++ b/lib/__tests__/vacation-calendar.test.ts @@ -1,6 +1,6 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { - blockTimes, decideBlock, ownFilterScriptStops, effectiveBlockSetting, eventSpan, isoDuration, oooUid, resetVacationCalendarSync, + blockTimes, decideBlock, ownFilterScriptStops, resolveBlockSetting, effectiveBlockSetting, eventSpan, isoDuration, oooUid, resetVacationCalendarSync, syncVacationBlock, vacationReplyLoaded, vacationReplySaved, type DecisionInput, } from '../vacation-calendar'; import { useSettingsStore } from '@/stores/settings-store'; @@ -231,3 +231,80 @@ describe('ownFilterScriptStops', () => { expect(await ownFilterScriptStops(sieve([{ name: 'vacation', isActive: true, content: '' }]))).toBe(false); }); }); + +describe('whose event it is without this device\'s record', () => { + it('follows the reply\'s previous dates: switched off or moved on another device or by a delegate', () => { + const event = span(last.start, last.end); + const previous = { fromDate: '2026-12-01T08:00:00Z', toDate: '2026-12-05T17:00:00Z' }; + expect(decideBlock(input({ reply: reply({ isEnabled: false }), event, previous }))).toEqual({ action: 'destroy' }); + expect(decideBlock(input({ reply: reply({ toDate: '2026-12-08T17:00:00Z' }), event, previous })).action).toBe('update'); + // Someone moved the event before: neither the record nor the old dates match. + expect(decideBlock(input({ reply: reply({ isEnabled: false }), event: span('2026-12-02T08:00:00Z', '2026-12-03T08:00:00Z'), previous }))) + .toEqual({ action: 'none' }); + }); + + it('takes a marked event as its own when the reply is loaded on a device with no record', () => { + const event = span(last.start, last.end); + expect(decideBlock(input({ reply: reply({ toDate: '2026-12-08T17:00:00Z' }), event, marked: true, trigger: 'load' })).action).toBe('update'); + expect(decideBlock(input({ reply: reply({ toDate: '2026-12-08T17:00:00Z' }), event, marked: false, trigger: 'load' }))).toEqual({ action: 'none' }); + expect(decideBlock(input({ reply: reply({ toDate: '2026-12-08T17:00:00Z' }), event, marked: true, trigger: 'save' }))).toEqual({ action: 'none' }); + }); + + it('marks the event it creates and keeps the mark when it moves it', async () => { + useSettingsStore.setState({ vacationBlockCalendar: true, vacationCalendarBlocks: {}, timeZone: 'Europe/Berlin' }); + const client = fakeClient(null); + await syncVacationBlock(client, undefined, reply(), 'save', NOW); + expect(client.createCalendarEvent.mock.calls[0][0]).toMatchObject({ keywords: { 'bulwark-out-of-office': true } }); + // Another device, no record: the reply\'s old dates make the event ours. + useSettingsStore.setState({ vacationCalendarBlocks: {} }); + const ours = { id: 'e1', ...blockTimes(new Date(last.start), new Date(last.end), 'Europe/Berlin') }; + const other = fakeClient(ours); + expect(await syncVacationBlock(other, undefined, reply({ isEnabled: false }), 'save', NOW, { fromDate: last.start, toDate: last.end })).toBe('destroy'); + useSettingsStore.setState({ timeZone: 'auto' }); + }); +}); + +describe('resolveBlockSetting', () => { + it('pins the organisation\'s value when locked and hides the choice when hidden', () => { + expect(resolveBlockSetting({ locked: true, value: false }, undefined, true)).toEqual({ value: false, locked: true, hidden: false }); + // A lock without a value takes the policy default, not the user's choice. + expect(resolveBlockSetting({ locked: true }, false, true)).toEqual({ value: false, locked: true, hidden: false }); + expect(resolveBlockSetting({ locked: true }, undefined, false)).toEqual({ value: true, locked: true, hidden: false }); + expect(resolveBlockSetting({ hidden: true }, undefined, false)).toEqual({ value: true, locked: false, hidden: true }); + expect(resolveBlockSetting(undefined, undefined, false)).toEqual({ value: false, locked: false, hidden: false }); + }); +}); + +describe('the sync scheduler with several accounts', () => { + beforeEach(() => { + vi.useFakeTimers(); + resetVacationCalendarSync(); + useSettingsStore.setState({ vacationBlockCalendar: true, vacationCalendarBlocks: {} }); + }); + afterEach(() => { + vi.useRealTimers(); + resetVacationCalendarSync(); + }); + + it('keeps one job per account, and the dates from before the first of several saves', async () => { + const client = fakeClient(null); + vacationReplySaved(client, undefined, reply(), reply({ subject: 'A' })); + vacationReplySaved(client, 'boss', reply(), reply({ subject: 'B' })); + await vi.advanceTimersByTimeAsync(5000); + const accounts = client.queryCalendarEvents.mock.calls.map((c) => (c as unknown[])[3]); + expect(accounts.sort()).toEqual(['boss', undefined].sort()); + }); + + it('syncs several quick saves once, against the dates from before the first', async () => { + useSettingsStore.setState({ timeZone: 'Europe/Berlin' }); + const ours = { id: 'e1', ...blockTimes(new Date(last.start), new Date(last.end), 'Europe/Berlin') }; + const client = fakeClient(ours); + const moved = reply({ toDate: '2026-12-09T17:00:00Z' }); + vacationReplySaved(client, undefined, reply(), moved); + vacationReplySaved(client, undefined, moved, { ...moved, isEnabled: false }); + await vi.advanceTimersByTimeAsync(5000); + expect(client.queryCalendarEvents).toHaveBeenCalledTimes(1); + expect(client.deleteCalendarEvent).toHaveBeenCalledWith('e1', false, undefined); + useSettingsStore.setState({ timeZone: 'auto' }); + }); +}); diff --git a/lib/vacation-calendar.ts b/lib/vacation-calendar.ts index 7be23521c..cbbd44e19 100644 --- a/lib/vacation-calendar.ts +++ b/lib/vacation-calendar.ts @@ -8,9 +8,14 @@ * timed, or all-day when both ends are midnight in the user's time zone; * busy, public, without participants, and never sends invitations. * - * Bulwark keeps the times it last wrote (synced settings, per account). - * When the event's times no longer match them, someone edited it, and - * Bulwark leaves it alone. Switching the reply or the block off removes an + * The event is Bulwark's while its times are the ones Bulwark last wrote + * (kept in the settings, per account) or the reply's dates before the change + * at hand. That second rule works on any device and for anyone who changes + * the reply (a delegate, the owner), also where the settings don't sync + * (Lite, sync off). The event also carries the keyword OOO_KEYWORD (a + * CATEGORIES value, which 0.16 and 1.0 both keep): on a device with no + * record, a marked event found when the reply is loaded counts as Bulwark's. + * When none of these hold, someone edited it, and Bulwark leaves it alone. Switching the reply or the block off removes an * event that hasn't started and ends one that has at once; a reply that * simply ran out leaves its past event in place. An event deleted elsewhere * is not put back until the reply's dates change. @@ -37,6 +42,9 @@ import { debug } from '@/lib/debug'; export const OOO_UID_PREFIX = 'bulwark-ooo-'; export const SYNC_DEBOUNCE_MS = 1500; +/** The keyword (iCalendar CATEGORIES) on the busy event. */ +export const OOO_KEYWORD = 'bulwark-out-of-office'; + export function oooUid(accountId: string): string { return `${OOO_UID_PREFIX}${accountId}`; } @@ -69,6 +77,10 @@ export interface DecisionInput { /** The event with our UID, as instants; null when there is none. */ event: { start: Date; end: Date } | null; last: WrittenBlock | null; + /** The reply's dates before the change being synced; null when unknown (a load). */ + previous?: Pick | null; + /** The event carries OOO_KEYWORD. */ + marked?: boolean; now: Date; /** `save`: the reply or the block just changed here. `load`: the reply was read. */ trigger: 'save' | 'load'; @@ -86,10 +98,12 @@ export function decideBlock(input: DecisionInput): BlockDecision { const ranOut = reply.isEnabled && !!to && to <= now; if (event) { - const ours = last ? same(event.start, last.start) && same(event.end, last.end) : false; - if (!ours) { - // Without a record, an event that already sits on the reply's dates - // needs nothing; any other was set up or changed by someone else. + const matchesLast = !!last && same(event.start, last.start) && same(event.end, last.end); + const matchesPrevious = !!input.previous?.fromDate && !!input.previous.toDate + && same(event.start, input.previous.fromDate) && same(event.end, input.previous.toDate); + const markedOnLoad = !last && !!input.marked && input.trigger === 'load'; + if (!matchesLast && !matchesPrevious && !markedOnLoad) { + // Set up or changed by someone else: left alone. return { action: 'none' }; } if (wanted) { @@ -171,15 +185,30 @@ export function blockCalendar(calendars: Calendar[], accountId: string, primaryA return !rights || rights.mayWriteAll || rights.mayWriteOwn ? calendar : null; } -/** The block setting as it applies: the organisation's lock, else the user's choice. */ +/** + * The block setting as it applies: locked, the organisation's value (the + * lock's value, else the policy default, else on); hidden, the policy default + * (else on), the user can't see the choice; otherwise the user's choice. + * The settings card and the sync both read it here. + */ +export function resolveBlockSetting( + restriction: { locked?: boolean; hidden?: boolean; value?: unknown } | undefined, + policyDefault: unknown, + userSetting: unknown, +): { value: boolean; locked: boolean; hidden: boolean } { + const orgValue = (v: unknown) => (typeof v === 'boolean' ? v : typeof policyDefault === 'boolean' ? policyDefault : true); + if (restriction?.locked === true) return { value: orgValue('value' in restriction ? restriction.value : undefined), locked: true, hidden: false }; + if (restriction?.hidden === true) return { value: orgValue(undefined), locked: false, hidden: true }; + return { value: userSetting !== false, locked: false, hidden: false }; +} + export function effectiveBlockSetting(): boolean { const policy = usePolicyStore.getState(); - const restriction = policy.getRestriction('vacationBlockCalendar'); - if (restriction?.locked === true) { - const value = 'value' in restriction ? restriction.value : policy.getEffectiveDefault('vacationBlockCalendar'); - return typeof value === 'boolean' ? value : true; - } - return useSettingsStore.getState().vacationBlockCalendar !== false; + return resolveBlockSetting( + policy.getRestriction('vacationBlockCalendar'), + policy.getEffectiveDefault('vacationBlockCalendar'), + useSettingsStore.getState().vacationBlockCalendar, + ).value; } function writtenBlocks(): Record { @@ -207,6 +236,7 @@ export async function syncVacationBlock( reply: ReplyState, trigger: 'save' | 'load', now = new Date(), + previous: Pick | null = null, ): Promise { if (typeof client.supportsCalendars !== 'function' || !client.supportsCalendars()) return 'none'; const primary = client.getAccountId(); @@ -217,7 +247,8 @@ export async function syncVacationBlock( const [found] = await client.queryCalendarEvents({ uid: oooUid(account) }, undefined, 1, target); const span = found ? eventSpan(found, zone) : null; const last = writtenBlocks()[account] ?? null; - const decision = decideBlock({ block: effectiveBlockSetting(), reply, event: span, last, now, trigger }); + const marked = !!found?.keywords?.[OOO_KEYWORD]; + const decision = decideBlock({ block: effectiveBlockSetting(), reply, event: span, last, previous, marked, now, trigger }); switch (decision.action) { case 'none': // A record of an event that is gone and not wanted any more is dropped. @@ -235,6 +266,7 @@ export async function syncVacationBlock( freeBusyStatus: 'busy', privacy: 'public', status: 'confirmed', + keywords: { [OOO_KEYWORD]: true }, } as Partial, false, target); rememberBlock(account, { start: decision.start.toISOString(), end: decision.end.toISOString() }); return 'create'; @@ -243,7 +275,8 @@ export async function syncVacationBlock( await client.updateCalendarEvent(found!.originalId ?? found!.id, { ...blockTimes(decision.start, decision.end, zone), title: reply.subject.trim() || OUT_OF_OFFICE_TITLE, - }, false, target); + [`keywords/${OOO_KEYWORD}`]: true, + } as Partial, false, target); rememberBlock(account, { start: decision.start.toISOString(), end: decision.end.toISOString() }); return 'update'; case 'end-now': { @@ -285,54 +318,61 @@ interface Pending { client: SyncClient; accountId: string | undefined; reply: ReplyState; + /** The dates before the first change still waiting to be synced. */ + previous: Pick | null; trigger: 'save' | 'load'; } // Kept on globalThis: the store and the settings page may be in different // bundle copies, and two schedulers would sync the same event twice. interface SchedulerState { - timer: ReturnType | null; - pending: Pending | null; - running: Promise | null; + /** Per account: one job waiting, one timer, one sync running. */ + timers: Map>; + pending: Map; + running: Map>; loaded: Set; } const SCHEDULER_KEY = '__bulwarkVacationCalendarSync'; function scheduler(): SchedulerState { const g = globalThis as unknown as Record; - return (g[SCHEDULER_KEY] ??= { timer: null, pending: null, running: null, loaded: new Set() }); + return (g[SCHEDULER_KEY] ??= { timers: new Map(), pending: new Map(), running: new Map(), loaded: new Set() }); } -function run(): void { +function run(key: string): void { const state = scheduler(); - state.timer = null; - if (state.running || !state.pending) return; - const job = state.pending; - state.pending = null; - state.running = syncVacationBlock(job.client, job.accountId, job.reply, job.trigger) + state.timers.delete(key); + const job = state.pending.get(key); + if (state.running.has(key) || !job) return; + state.pending.delete(key); + state.running.set(key, syncVacationBlock(job.client, job.accountId, job.reply, job.trigger, new Date(), job.previous) .finally(() => { - state.running = null; - if (state.pending && !state.timer) run(); - }); + state.running.delete(key); + if (state.pending.has(key) && !state.timers.has(key)) run(key); + })); } function schedule(job: Pending): void { const state = scheduler(); - // A save outranks a load that waits; the latest reply wins. - if (state.pending?.trigger === 'save' && job.trigger === 'load') return; - state.pending = job; - if (state.timer) clearTimeout(state.timer); - state.timer = setTimeout(run, SYNC_DEBOUNCE_MS); + const key = job.accountId ?? job.client.getAccountId(); + const waiting = state.pending.get(key); + // A save outranks a load that waits; the latest reply wins, with the + // dates from before the first change it replaces. + if (waiting?.trigger === 'save' && job.trigger === 'load') return; + state.pending.set(key, waiting?.trigger === 'save' ? { ...job, previous: waiting.previous } : job); + const timer = state.timers.get(key); + if (timer) clearTimeout(timer); + state.timers.set(key, setTimeout(() => run(key), SYNC_DEBOUNCE_MS)); } /** After a saved reply: sync when the switch, the dates or the subject changed. */ export function vacationReplySaved(client: SyncClient, accountId: string | undefined, before: ReplyState, after: ReplyState): void { if (relevant(before) === relevant(after)) return; - schedule({ client, accountId, reply: after, trigger: 'save' }); + schedule({ client, accountId, reply: after, previous: { fromDate: before.fromDate, toDate: before.toDate }, trigger: 'save' }); } /** The user changed the block setting: sync with the reply as it is. */ export function vacationBlockChanged(client: SyncClient, accountId: string | undefined, reply: ReplyState): void { - schedule({ client, accountId, reply, trigger: 'save' }); + schedule({ client, accountId, reply, previous: { fromDate: reply.fromDate, toDate: reply.toDate }, trigger: 'save' }); } /** After the reply was loaded: once per page load and account. */ @@ -341,16 +381,16 @@ export function vacationReplyLoaded(client: SyncClient, accountId: string | unde const state = scheduler(); if (state.loaded.has(key)) return; state.loaded.add(key); - schedule({ client, accountId, reply, trigger: 'load' }); + schedule({ client, accountId, reply, previous: null, trigger: 'load' }); } /** For tests: drop what the scheduler holds. */ export function resetVacationCalendarSync(): void { const state = scheduler(); - if (state.timer) clearTimeout(state.timer); - state.timer = null; - state.pending = null; - state.running = null; + for (const timer of state.timers.values()) clearTimeout(timer); + state.timers.clear(); + state.pending.clear(); + state.running.clear(); state.loaded.clear(); }