diff --git a/packages/web/src/events/grid-event-draft.adapter.test.ts b/packages/web/src/events/grid-event-draft.adapter.test.ts index e48a2511d..5c78f523d 100644 --- a/packages/web/src/events/grid-event-draft.adapter.test.ts +++ b/packages/web/src/events/grid-event-draft.adapter.test.ts @@ -306,6 +306,20 @@ test("a patch that echoes the hydrated rule unchanged keeps the draft's recurren }); }); +test("a patch with semantically equal INTERVAL=1 drift keeps preserve", () => { + const draft = editGridEventDraft(occurrenceEvent); + if (!draft) throw new Error("Expected scheduled event draft"); + + // SERIES_RULES omit INTERVAL; the form rebuild re-emits INTERVAL=1. + const updated = patchGridDraftRecurrence( + draft, + ["RRULE:FREQ=WEEKLY;INTERVAL=1;BYDAY=MO,TU,WE"], + SERIES_RULES, + ); + + expect(updated.values.recurrence).toEqual({ kind: "preserve" }); +}); + test("a patch with a genuinely different rule converts the draft to an explicit series edit", () => { const draft = editGridEventDraft(occurrenceEvent); if (!draft) throw new Error("Expected scheduled event draft"); diff --git a/packages/web/src/events/grid-event-draft.adapter.ts b/packages/web/src/events/grid-event-draft.adapter.ts index abd81373d..d5fe503ee 100644 --- a/packages/web/src/events/grid-event-draft.adapter.ts +++ b/packages/web/src/events/grid-event-draft.adapter.ts @@ -1,4 +1,5 @@ -import fastDeepEqual from "fast-deep-equal/react"; +import { ObjectId } from "bson"; +import { type Weekday } from "rrule"; import { Origin } from "@core/constants/core.constants"; import { type Calendar } from "@core/types/calendar.contracts"; import { type CompassEvent } from "@core/types/compass-event.contracts"; @@ -10,6 +11,7 @@ import { } from "@core/types/event-color.contracts"; import { type RecurrenceScope } from "@core/types/event-command.contracts"; import dayjs from "@core/util/date/dayjs"; +import { CompassEventRRule } from "@core/util/event/compass.event.rrule"; import { type GridEvent } from "@web/common/types/web.event.types"; import { getBrowserTimeZone } from "@web/common/utils/datetime/web.date.util"; import { gridEventDefaultPosition } from "@web/common/utils/event/event.util"; @@ -341,6 +343,89 @@ export function resolveDraftRecurrenceRules( return Array.isArray(rule) ? [...rule] : []; } +export function scheduleDatesFromDraft(draft: GridEventDraft) { + const { schedule } = draft.values; + + if (schedule.kind === "allDay") { + return { + startDate: dayjs(schedule.start).toYearMonthDayString(), + endDate: dayjs(schedule.end).toYearMonthDayString(), + }; + } + + return { + startDate: + dayjs(schedule.start).format() || dayjs().toRFC3339OffsetString(), + endDate: + dayjs(schedule.end).format() || + dayjs().add(1, "hour").toRFC3339OffsetString(), + }; +} + +const weekdayNumber = (day: number | Weekday): number => + typeof day === "number" ? day : day.weekday; + +export const sortedByweekday = ( + byweekday: Array | null | undefined, +): number[] => (byweekday ?? []).map(weekdayNumber).sort((a, b) => a - b); + +const normalizedCount = (count: number | null | undefined): number | null => + count == null || count === 0 ? null : count; + +const untilEqual = (a: Date | null | undefined, b: Date | null | undefined) => { + if (a == null && b == null) return true; + if (a == null || b == null) return false; + return dayjs(a).isSame(b); +}; + +// Pattern-only RRULE equality for open-for-edit / patch: ignore serialization +// drift (INTERVAL=1, WKST defaults, param/BYDAY order) and never treat the +// occurrence's clicked dtstart as a recurrence edit. +// Whitelist matches RecurrenceSection's editable surface (freq/interval/ +// weekdays/until/count). Fields the form cannot edit today (bymonthday, +// bysetpos, nth-weekday) are intentionally ignored — extend this list if +// the UI gains those controls, or edits to them will not flip preserve→series. +export function recurrenceRulesSemanticallyEqual( + a: readonly string[], + b: readonly string[], + dates: { startDate: string; endDate: string }, +): boolean { + if (a.length === 0 && b.length === 0) return true; + if (a.length === 0 || b.length === 0) return false; + + const shell = { + _id: new ObjectId(), + startDate: dates.startDate, + endDate: dates.endDate, + }; + const optsA = new CompassEventRRule({ + ...shell, + recurrence: { rule: [...a] }, + }).options; + const optsB = new CompassEventRRule({ + ...shell, + recurrence: { rule: [...b] }, + }).options; + + return ( + optsA.freq === optsB.freq && + (optsA.interval ?? 1) === (optsB.interval ?? 1) && + byweekdayEqual(optsA.byweekday, optsB.byweekday) && + normalizedCount(optsA.count) === normalizedCount(optsB.count) && + untilEqual(optsA.until, optsB.until) + ); +} + +function byweekdayEqual( + a: Array | null | undefined, + b: Array | null | undefined, +): boolean { + const left = sortedByweekday(a); + const right = sortedByweekday(b); + if (left.length !== right.length) return false; + return left.every((value, index) => value === right[index]); +} + // The draft renders its own recurrence preview separately (Draft.tsx's // getRecurringDraftPreviews), but the *saved* sibling occurrences of the // series being edited still come through the normal week query and render @@ -371,7 +456,11 @@ export function patchGridDraftRecurrence( seriesRules?: readonly string[], ): GridEventDraft { const currentRules = resolveDraftRecurrenceRules(draft, seriesRules); - const ruleUnchanged = fastDeepEqual(currentRules, [...nextRules]); + const ruleUnchanged = recurrenceRulesSemanticallyEqual( + currentRules, + nextRules, + scheduleDatesFromDraft(draft), + ); // Only useRecurrence calls this, and only with an explicit user edit (a // weekday/frequency/until change, or the Repeat toggle turned off) - a // draft that hasn't touched recurrence never reaches here, so it keeps @@ -379,24 +468,22 @@ export function patchGridDraftRecurrence( // always an explicit clear, on both create and edit drafts: "single", // never "preserve" (which for an edit draft would just resolve back to // the source event's original rules, making the Repeat toggle a no-op). - const recurrence = ruleUnchanged - ? draft.values.recurrence - : nextRules.length > 0 + if (ruleUnchanged) return draft; + + const recurrence = + nextRules.length > 0 ? { kind: "series" as const, rules: [...nextRules] } : ({ kind: "single" } as const); // The two branches look identical, but each is required to keep // GridEventDraft's discriminated union narrowed (see - // replaceGridDraftSchedule above) - `recurrence` can structurally carry - // "preserve" here (from the ruleUnchanged passthrough on an edit draft), - // which isn't assignable to a create draft's NewEventRecurrenceDraft. + // replaceGridDraftSchedule above). if (draft.kind === "create") { return { ...draft, values: { ...draft.values, - recurrence: - recurrence.kind === "preserve" ? { kind: "single" } : recurrence, + recurrence, }, }; } diff --git a/packages/web/src/views/Forms/EventForm/DateControlsSection/RecurrenceSection/useRecurrence/useRecurrence.test.ts b/packages/web/src/views/Forms/EventForm/DateControlsSection/RecurrenceSection/useRecurrence/useRecurrence.test.ts index ec11404a8..5906872be 100644 --- a/packages/web/src/views/Forms/EventForm/DateControlsSection/RecurrenceSection/useRecurrence/useRecurrence.test.ts +++ b/packages/web/src/views/Forms/EventForm/DateControlsSection/RecurrenceSection/useRecurrence/useRecurrence.test.ts @@ -30,6 +30,50 @@ const baseDraft = () => timeZone: "UTC", }); +const mountPreserveOccurrence = ( + seriesRules: string[], + seriesId = EventIdSchema.parse("0123456789abcdefaaaaaaaa"), +) => { + const source = createMockEvent({ + schedule: SCHEDULE, + recurrence: { + kind: "occurrence", + seriesId, + }, + }); + const editedDraft = editGridEventDraft(source); + if (!editedDraft) throw new Error("expected edit draft"); + + expect(editedDraft.values.recurrence).toEqual({ kind: "preserve" }); + expect(suppressedSeriesIdForDraft(editedDraft)).toBeNull(); + + let draft: GridEventDraft = editedDraft; + let setDraftCalls = 0; + const setDraft: Dispatch> = ( + updater, + ) => { + setDraftCalls++; + const next = typeof updater === "function" ? updater(draft) : updater; + if (next) draft = next; + }; + + const hook = renderHook(() => + useRecurrence(draft, { setDraft }, seriesRules), + ); + + return { + draft: () => draft, + setDraftCalls: () => setDraftCalls, + result: hook.result, + rerender: hook.rerender, + assertUntouched: () => { + expect(setDraftCalls).toBe(0); + expect(draft.values.recurrence).toEqual({ kind: "preserve" }); + expect(suppressedSeriesIdForDraft(draft)).toBeNull(); + }, + }; +}; + describe("useRecurrence hook", () => { it("initializes with no recurrence", () => { const draft = baseDraft(); @@ -220,33 +264,9 @@ describe("useRecurrence hook", () => { // preserve→series, suppress earlier siblings, and leave only the clicked // day + forward previews visible. it("does not rewrite a preserve occurrence draft whose series omits INTERVAL=1", () => { - const seriesRules = ["RRULE:FREQ=WEEKLY;BYDAY=MO,TU,WE,TH,FR"]; - const source = createMockEvent({ - schedule: SCHEDULE, - recurrence: { - kind: "occurrence", - seriesId: EventIdSchema.parse("0123456789abcdefaaaaaaaa"), - }, - }); - const editedDraft = editGridEventDraft(source); - if (!editedDraft) throw new Error("expected edit draft"); - - expect(editedDraft.values.recurrence).toEqual({ kind: "preserve" }); - expect(suppressedSeriesIdForDraft(editedDraft)).toBeNull(); - - let draft: GridEventDraft = editedDraft; - let setDraftCalls = 0; - const setDraft: Dispatch> = ( - updater, - ) => { - setDraftCalls++; - const next = typeof updater === "function" ? updater(draft) : updater; - if (next) draft = next; - }; - - const { result, rerender } = renderHook(() => - useRecurrence(draft, { setDraft }, seriesRules), - ); + const { result, rerender, assertUntouched } = mountPreserveOccurrence([ + "RRULE:FREQ=WEEKLY;BYDAY=MO,TU,WE,TH,FR", + ]); expect(result.current.hasRecurrence).toBe(true); expect(result.current.freq).toBe(Frequency.WEEKLY); @@ -257,15 +277,73 @@ describe("useRecurrence hook", () => { "thursday", "friday", ]); - expect(setDraftCalls).toBe(0); - expect(draft.values.recurrence).toEqual({ kind: "preserve" }); - expect(suppressedSeriesIdForDraft(draft)).toBeNull(); + assertUntouched(); + + rerender(); + assertUntouched(); + }); + + it("does not rewrite when series RRULE params are reordered", () => { + const { assertUntouched, rerender } = mountPreserveOccurrence([ + "RRULE:INTERVAL=1;FREQ=WEEKLY;BYDAY=MO,TU,WE,TH,FR", + ]); + + assertUntouched(); + rerender(); + assertUntouched(); + }); + + it("does not rewrite when series BYDAY order differs from the rebuilt rule", () => { + const { assertUntouched, rerender, result } = mountPreserveOccurrence([ + "RRULE:FREQ=WEEKLY;BYDAY=FR,MO,TU,WE,TH", + ]); + expect(result.current.weekDays).toEqual([ + "friday", + "monday", + "tuesday", + "wednesday", + "thursday", + ]); + assertUntouched(); + rerender(); + assertUntouched(); + }); + + it("does not rewrite when series includes a default WKST", () => { + const { assertUntouched, rerender } = mountPreserveOccurrence([ + "RRULE:FREQ=WEEKLY;BYDAY=MO,TU,WE,TH,FR;WKST=MO", + ]); + + assertUntouched(); + rerender(); + assertUntouched(); + }); + + it("writes a real weekday edit and flips preserve to series", () => { + const seriesId = EventIdSchema.parse("0123456789abcdefaaaaaaaa"); + const { draft, setDraftCalls, result, rerender } = mountPreserveOccurrence( + ["RRULE:FREQ=WEEKLY;BYDAY=MO,TU,WE,TH,FR"], + seriesId, + ); + + expect(setDraftCalls()).toBe(0); + expect(draft().values.recurrence).toEqual({ kind: "preserve" }); + expect(suppressedSeriesIdForDraft(draft())).toBeNull(); + + act(() => { + result.current.setWeekDays([ + "monday", + "tuesday", + "wednesday", + "thursday", + ]); + }); rerender(); - expect(setDraftCalls).toBe(0); - expect(draft.values.recurrence).toEqual({ kind: "preserve" }); - expect(suppressedSeriesIdForDraft(draft)).toBeNull(); + expect(setDraftCalls()).toBeGreaterThan(0); + expect(draft().values.recurrence).toMatchObject({ kind: "series" }); + expect(suppressedSeriesIdForDraft(draft())).toBe(seriesId); }); // Regression for React error #185 (max update depth exceeded): a timed diff --git a/packages/web/src/views/Forms/EventForm/DateControlsSection/RecurrenceSection/useRecurrence/useRecurrence.ts b/packages/web/src/views/Forms/EventForm/DateControlsSection/RecurrenceSection/useRecurrence/useRecurrence.ts index b8f138271..ea177dac5 100644 --- a/packages/web/src/views/Forms/EventForm/DateControlsSection/RecurrenceSection/useRecurrence/useRecurrence.ts +++ b/packages/web/src/views/Forms/EventForm/DateControlsSection/RecurrenceSection/useRecurrence/useRecurrence.ts @@ -1,5 +1,4 @@ import { ObjectId } from "bson"; -import fastDeepEqual from "fast-deep-equal/react"; import { type Dispatch, type SetStateAction, @@ -14,7 +13,10 @@ import { CompassEventRRule } from "@core/util/event/compass.event.rrule"; import { type GridEventDraft } from "@web/events/event-draft.types"; import { patchGridDraftRecurrence, + recurrenceRulesSemanticallyEqual, resolveDraftRecurrenceRules, + scheduleDatesFromDraft, + sortedByweekday, } from "@web/events/grid-event-draft.adapter"; import { type FrequencyValues, @@ -56,18 +58,6 @@ const WEEKDAY_MAP: Record< {} as Record, ); -// Strip RRULE defaults the rrule library re-emits on rebuild (INTERVAL=1, -// WKST) so open-for-edit doesn't treat serialization drift as a real edit. -// Without this, Google-style rules like FREQ=WEEKLY;BYDAY=MO..FR flip the -// draft from preserve→series and hide earlier sibling instances. -const normalizeRecurrenceRule = (rule: string[] | null | undefined): string[] => - (rule ?? []).map((entry) => - entry - .replace(/;INTERVAL=1(?=;|$)/g, "") - .replace(/:INTERVAL=1;/g, ":") - .replace(/;WKST=[A-Z]{2}/g, ""), - ); - const weekdayKeyFromByweekday = ( day: number | Weekday, ): keyof typeof WEEKDAY_RRULE_MAP => { @@ -75,23 +65,19 @@ const weekdayKeyFromByweekday = ( return REVERSE_WEEKDAY_LABELS_MAP[WEEKDAY_MAP[weekday].toString()]; }; -const scheduleDatesFromDraft = (draft: GridEventDraft) => { - const { schedule } = draft.values; - - if (schedule.kind === "allDay") { - return { - startDate: dayjs(schedule.start).toYearMonthDayString(), - endDate: dayjs(schedule.end).toYearMonthDayString(), - }; - } - - return { - startDate: - dayjs(schedule.start).format() || dayjs().toRFC3339OffsetString(), - endDate: - dayjs(schedule.end).format() || - dayjs().add(1, "hour").toRFC3339OffsetString(), - }; +const recurrencePatternSeedKey = ( + hasRecurrence: boolean, + options: { + freq: Frequency; + interval: number; + byweekday: Array | null | undefined; + count: number | null; + }, + until: Date | null, +) => { + const byweekday = sortedByweekday(options.byweekday).join(","); + const untilKey = until ? dayjs(until).valueOf() : ""; + return `${hasRecurrence}:${options.freq}:${options.interval ?? 1}:${byweekday}:${options.count ?? ""}:${untilKey}`; }; export const useRecurrence = ( @@ -151,8 +137,8 @@ export const useRecurrence = ( // back into a new CompassEventRRule's `options.until` below without // drifting it on every render (the bug this guards against: seeding from // the still-floating `options.until` would double-float on round-trip and - // never converge, since the deep-equal guard in the effect below would - // never pass). + // never converge, since the semantic-equality guard in the effect below + // would never pass). const [freq, setFreq] = useState(options.freq); const [interval, setInterval] = useState(options.interval); const [until, setUntilState] = useState(() => parsed.until); @@ -160,7 +146,11 @@ export const useRecurrence = ( const [wkst, setWkst] = useState(defaultWkst); const [weekDays, setWeekDays] = useState(defaultWeekDay); - const ruleSeedKey = `${hasRecurrence}:${JSON.stringify(normalizeRecurrenceRule(currentRules))}`; + const ruleSeedKey = recurrencePatternSeedKey( + hasRecurrence, + options, + parsed.until, + ); const [syncedRuleSeedKey, setSyncedRuleSeedKey] = useState(ruleSeedKey); if (ruleSeedKey !== syncedRuleSeedKey) { @@ -241,13 +231,9 @@ export const useRecurrence = ( useEffect(() => { if (!hasRecurrence) return; - const nextRule = JSON.parse(rule); - if ( - fastDeepEqual( - normalizeRecurrenceRule(currentRules), - normalizeRecurrenceRule(nextRule), - ) - ) { + const nextRule = JSON.parse(rule) as string[]; + const dates = { startDate, endDate }; + if (recurrenceRulesSemanticallyEqual(currentRules, nextRule, dates)) { return; } @@ -258,13 +244,21 @@ export const useRecurrence = ( currentDraft, seriesRules, ); - if (fastDeepEqual(projectedRules, nextRule)) { + if (recurrenceRulesSemanticallyEqual(projectedRules, nextRule, dates)) { return currentDraft; } return patchGridDraftRecurrence(currentDraft, nextRule, seriesRules); }); - }, [currentRules, hasRecurrence, rule, seriesRules, setDraft]); + }, [ + currentRules, + endDate, + hasRecurrence, + rule, + seriesRules, + setDraft, + startDate, + ]); return { hasRecurrence,