fix(03): CR-01 preserve RRULE on edit-as-move (forward source rule to create row)
This commit is contained in:
@@ -282,6 +282,23 @@ async function dispatchRow(row: OutboxRow): Promise<DispatchResult> {
|
||||
} catch {
|
||||
return { success: false, conflict: false, hardFail: true, transient: false, error: 'payload parse failed' }
|
||||
}
|
||||
// CR-01: edit-as-move RRULE preservation. The same-calendar `update` branch
|
||||
// preserves a recurring series' RRULE by reading rawVevent; the `create` branch
|
||||
// (used for the create half of an edit-as-move, D-04) has no source for the
|
||||
// original RRULE because it writes under a brand-new uid. The edit route extracts
|
||||
// the source event's RRULE and stashes it on the payload as `_preservedRrule` so
|
||||
// the worker can re-apply it here. An explicit `recurrence` on the payload still
|
||||
// wins (deliberate user change); the preserved RRULE only fills the gap when the
|
||||
// edit omitted recurrence — matching the update-branch semantics and the WR-01 fix.
|
||||
const hasExplicitRecurrence = Object.prototype.hasOwnProperty.call(fields, 'recurrence')
|
||||
const rruleFromPayload =
|
||||
fields.recurrence && fields.recurrence !== 'none'
|
||||
? RRULE_PRESETS[fields.recurrence as string]
|
||||
: undefined
|
||||
const preservedRrule =
|
||||
typeof fields._preservedRrule === 'string' && fields._preservedRrule.length > 0
|
||||
? fields._preservedRrule
|
||||
: undefined
|
||||
const { icsString } = buildVeventString({
|
||||
uid: row.uid,
|
||||
summary: fields.title as string,
|
||||
@@ -290,7 +307,7 @@ async function dispatchRow(row: OutboxRow): Promise<DispatchResult> {
|
||||
dtend: fields.allDay ? (fields.end as string) : new Date(fields.end as string),
|
||||
location: fields.location as string | undefined,
|
||||
description: fields.description as string | undefined,
|
||||
rruleString: fields.recurrence && fields.recurrence !== 'none' ? RRULE_PRESETS[fields.recurrence as string] : undefined,
|
||||
rruleString: hasExplicitRecurrence ? rruleFromPayload : (preservedRrule ?? rruleFromPayload),
|
||||
})
|
||||
// Build a minimal DAVCalendar for the write wrapper (only url is needed)
|
||||
const davCalendar = { url: row.calendarUrl } as Parameters<typeof createCalendarEvent>[1]
|
||||
|
||||
@@ -31,6 +31,7 @@ import { sql } from 'drizzle-orm'
|
||||
import { db } from '../db/client.js'
|
||||
import { calendarEvents, calendars, users, calendarOutbox } from '../db/schema.js'
|
||||
import { expandOccurrences } from '../broker/expand.js'
|
||||
import { extractRruleString } from '../broker/vevent.js'
|
||||
import { getAuth } from '../auth/middleware.js'
|
||||
import { upsertUser, deriveDisplayName } from '../auth/user.js'
|
||||
// Side-effect import: brings in the ContextVariableMap augmentation for c.get('user')
|
||||
@@ -334,6 +335,7 @@ eventsRouter.patch('/:uid/edit', zValidator('json', eventFieldsSchema), async (c
|
||||
calendarId: calendarEvents.calendarId,
|
||||
calendarUrl: calendars.url,
|
||||
userId: calendars.userId,
|
||||
rawVevent: calendarEvents.rawVevent,
|
||||
})
|
||||
.from(calendarEvents)
|
||||
.innerJoin(calendars, eq(calendarEvents.calendarId, calendars.id))
|
||||
@@ -373,6 +375,25 @@ eventsRouter.patch('/:uid/edit', zValidator('json', eventFieldsSchema), async (c
|
||||
const newUid = `${randomUUID()}@familysync`
|
||||
const groupId = randomUUID()
|
||||
|
||||
// CR-01: carry the existing RRULE through the move. The edit payload omits
|
||||
// `recurrence` (the occurrence contract does not expose it, D-03), and the
|
||||
// create lands under a brand-new uid that the worker can never look up the
|
||||
// original RRULE from. Unlike the same-calendar `update` branch — which reads
|
||||
// rawVevent and re-applies the stored RRULE — the create branch has no source
|
||||
// for it. Extract the RRULE from the source event here and stash it on the
|
||||
// create payload so the worker re-applies it, preventing a recurring series
|
||||
// from silently collapsing into a single occurrence on a calendar move.
|
||||
// Only stash when the edit did NOT carry an explicit recurrence: an explicit
|
||||
// value (including 'none') is a deliberate user change and must win.
|
||||
const preservedRrule =
|
||||
payload.recurrence === undefined
|
||||
? extractRruleString(eventRow.rawVevent ?? '')
|
||||
: undefined
|
||||
const createPayload =
|
||||
preservedRrule !== undefined
|
||||
? { ...payload, _preservedRrule: preservedRrule }
|
||||
: payload
|
||||
|
||||
await db.transaction(async (tx) => {
|
||||
// Delete from old calendar
|
||||
await tx.insert(calendarOutbox).values({
|
||||
@@ -392,7 +413,7 @@ eventsRouter.patch('/:uid/edit', zValidator('json', eventFieldsSchema), async (c
|
||||
status: 'pending',
|
||||
uid: newUid,
|
||||
calendarUrl: newCalendarUrl,
|
||||
payload: JSON.stringify(payload),
|
||||
payload: JSON.stringify(createPayload),
|
||||
groupId,
|
||||
})
|
||||
})
|
||||
|
||||
@@ -281,6 +281,58 @@ describe('runOutboxDrain — ICS building from form JSON (CR-02)', () => {
|
||||
const setArg = mockUpdateSet.mock.calls[0]?.[0] as { status?: string }
|
||||
expect(setArg?.status).toBe('failed')
|
||||
})
|
||||
|
||||
// CR-01 (iteration 2): the edit-as-move create branch must re-apply the RRULE the
|
||||
// route stashed on the payload as `_preservedRrule`, so a moved recurring series keeps
|
||||
// its RRULE instead of collapsing into a single occurrence.
|
||||
it('CR-01: create row re-applies _preservedRrule → emitted ICS contains RRULE:', async () => {
|
||||
const { createCalendarEvent } = await import('../../src/broker/write.js')
|
||||
let capturedIcsString: unknown = null
|
||||
vi.mocked(createCalendarEvent).mockImplementation(async (_client, _cal, _uid, icsString) => {
|
||||
capturedIcsString = icsString
|
||||
return makeResponse(201)
|
||||
})
|
||||
// Move payload: no explicit `recurrence`, but the route stashed the source RRULE.
|
||||
const movePayload = JSON.stringify({
|
||||
title: 'Moved weekly standup',
|
||||
allDay: false,
|
||||
start: '2026-06-10T12:00:00',
|
||||
end: '2026-06-10T13:00:00',
|
||||
_preservedRrule: 'FREQ=WEEKLY;BYDAY=MO',
|
||||
})
|
||||
mockPendingRows = [makeRow({ operation: 'create', payload: movePayload, groupId: 'move-grp-1' })]
|
||||
|
||||
await runOutboxDrain()
|
||||
|
||||
expect(typeof capturedIcsString).toBe('string')
|
||||
expect(capturedIcsString as string).toContain('RRULE:')
|
||||
expect(capturedIcsString as string).toContain('FREQ=WEEKLY')
|
||||
})
|
||||
|
||||
// CR-01 corollary: an explicit `recurrence` on a create still wins over any preserved
|
||||
// RRULE (deliberate user choice); recurrence:'none' must emit no RRULE.
|
||||
it("CR-01: explicit recurrence:'none' wins → emitted ICS has no RRULE even if _preservedRrule present", async () => {
|
||||
const { createCalendarEvent } = await import('../../src/broker/write.js')
|
||||
let capturedIcsString: unknown = null
|
||||
vi.mocked(createCalendarEvent).mockImplementation(async (_client, _cal, _uid, icsString) => {
|
||||
capturedIcsString = icsString
|
||||
return makeResponse(201)
|
||||
})
|
||||
const payload = JSON.stringify({
|
||||
title: 'One-off',
|
||||
allDay: false,
|
||||
start: '2026-06-10T12:00:00',
|
||||
end: '2026-06-10T13:00:00',
|
||||
recurrence: 'none',
|
||||
_preservedRrule: 'FREQ=WEEKLY;BYDAY=MO',
|
||||
})
|
||||
mockPendingRows = [makeRow({ operation: 'create', payload })]
|
||||
|
||||
await runOutboxDrain()
|
||||
|
||||
expect(typeof capturedIcsString).toBe('string')
|
||||
expect(capturedIcsString as string).not.toContain('RRULE:')
|
||||
})
|
||||
})
|
||||
|
||||
describe('runOutboxDrain — edit-as-move ordering (D-04)', () => {
|
||||
|
||||
@@ -413,6 +413,60 @@ describe('PATCH /api/events/:uid/edit', () => {
|
||||
})
|
||||
expect(res.status).toBe(202)
|
||||
})
|
||||
|
||||
// CR-01 (iteration 2): edit-as-move must carry the source event's RRULE through
|
||||
// to the create row so a moved recurring series does not silently collapse into a
|
||||
// single occurrence. The edit payload omits `recurrence`; the route must extract the
|
||||
// RRULE from the source event's rawVevent and stash it on the create payload as
|
||||
// `_preservedRrule` so the worker re-applies it.
|
||||
it('CR-01: move of a recurring event stashes the source RRULE on the create outbox row', async () => {
|
||||
// Seed a recurring source event (RRULE:FREQ=WEEKLY;BYDAY=MO) on the user's calendar.
|
||||
mockDbRows = [
|
||||
{
|
||||
uid: 'uid-001@familysync',
|
||||
etag: '"etag-abc"',
|
||||
objectUrl: 'https://caldav.fastmail.com/dav/calendars/user/test@fm.com/Default/uid-001.ics',
|
||||
calendarId: 1,
|
||||
calendarUrl: 'https://caldav.fastmail.com/dav/calendars/user/test@fm.com/Default/',
|
||||
userId: 1,
|
||||
rawVevent: SAMPLE_VEVENT_RECURRING_TIMED,
|
||||
},
|
||||
]
|
||||
const mockInnerJoinWhere = vi.fn().mockReturnValue({
|
||||
orderBy: vi.fn().mockReturnValue({
|
||||
limit: vi.fn().mockImplementation(() => Promise.resolve(mockDbRows)),
|
||||
}),
|
||||
})
|
||||
const mockInnerJoin = vi.fn().mockReturnValue({ where: mockInnerJoinWhere })
|
||||
mockFromFn.mockReturnValue({ innerJoin: mockInnerJoin })
|
||||
mockSelectFn.mockReturnValue({ from: mockFromFn })
|
||||
|
||||
const { app } = await import('../../src/index.js')
|
||||
const res = await app.request('/api/events/uid-001%40familysync/edit', {
|
||||
method: 'PATCH',
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify({
|
||||
title: 'Moved event',
|
||||
allDay: false,
|
||||
start: '2026-06-15T10:00:00Z',
|
||||
end: '2026-06-15T11:00:00Z',
|
||||
// Different calendarUrl than the source → triggers the edit-as-move path (D-04)
|
||||
calendarUrl: 'https://caldav.fastmail.com/dav/calendars/user/test@fm.com/Family/',
|
||||
}),
|
||||
})
|
||||
expect(res.status).toBe(202)
|
||||
|
||||
// The move inserts a delete+create pair inside a transaction. Find the create row's
|
||||
// payload and assert the preserved RRULE survived.
|
||||
const createInsertCall = mockInsertValuesFn.mock.calls.find((call) => {
|
||||
const arg = call[0] as { operation?: string; payload?: string }
|
||||
return arg?.operation === 'create'
|
||||
})
|
||||
expect(createInsertCall).toBeDefined()
|
||||
const createArg = createInsertCall![0] as { payload: string }
|
||||
const parsed = JSON.parse(createArg.payload) as { _preservedRrule?: string }
|
||||
expect(parsed._preservedRrule).toBe('FREQ=WEEKLY;BYDAY=MO')
|
||||
})
|
||||
})
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
Reference in New Issue
Block a user