fix(03): WR-01 preserve existing RRULE on edit instead of resetting to none
This commit is contained in:
@@ -31,7 +31,7 @@ import { createFastmailClient } from './client.js'
|
|||||||
import { decryptPassword } from './crypto.js'
|
import { decryptPassword } from './crypto.js'
|
||||||
import { syncCalendar } from './sync.js'
|
import { syncCalendar } from './sync.js'
|
||||||
import { createCalendarEvent, updateCalendarEvent, deleteCalendarEvent } from './write.js'
|
import { createCalendarEvent, updateCalendarEvent, deleteCalendarEvent } from './write.js'
|
||||||
import { buildVeventString, RRULE_PRESETS } from './vevent.js'
|
import { buildVeventString, extractRruleString, RRULE_PRESETS } from './vevent.js'
|
||||||
import type { FastmailClient } from './client.js'
|
import type { FastmailClient } from './client.js'
|
||||||
|
|
||||||
// ── Constants (D-07) ────────────────────────────────────────────────────────
|
// ── Constants (D-07) ────────────────────────────────────────────────────────
|
||||||
@@ -183,16 +183,19 @@ async function dispatchRow(row: OutboxRow): Promise<DispatchResult> {
|
|||||||
} catch {
|
} catch {
|
||||||
return { success: false, conflict: false, hardFail: true, transient: false, error: 'payload parse failed' }
|
return { success: false, conflict: false, hardFail: true, transient: false, error: 'payload parse failed' }
|
||||||
}
|
}
|
||||||
const { icsString } = buildVeventString({
|
// WR-01: recurrence preservation. The PWA omits `recurrence` from an edit payload
|
||||||
uid: row.uid,
|
// (it cannot read the existing RRULE — not in the occurrence contract, D-03), so on
|
||||||
summary: fields.title as string,
|
// update we must NOT rebuild the VEVENT with no RRULE — that would silently convert a
|
||||||
allDay: fields.allDay as boolean,
|
// recurring series into a single event. When the payload carries no explicit
|
||||||
dtstart: fields.allDay ? (fields.start as string) : new Date(fields.start as string),
|
// recurrence, fall back to the RRULE already stored in calendarEvents.rawVevent.
|
||||||
dtend: fields.allDay ? (fields.end as string) : new Date(fields.end as string),
|
// An explicit recurrence value (including 'none') still overrides — that is a
|
||||||
location: fields.location as string | undefined,
|
// deliberate user change. Read rawVevent in the same scoped query as the fresh etag.
|
||||||
description: fields.description as string | undefined,
|
let preservedRrule: string | undefined
|
||||||
rruleString: fields.recurrence && fields.recurrence !== 'none' ? RRULE_PRESETS[fields.recurrence as string] : undefined,
|
const hasExplicitRecurrence = Object.prototype.hasOwnProperty.call(fields, 'recurrence')
|
||||||
})
|
const rruleFromPayload =
|
||||||
|
fields.recurrence && fields.recurrence !== 'none'
|
||||||
|
? RRULE_PRESETS[fields.recurrence as string]
|
||||||
|
: undefined
|
||||||
// WR-02: re-read the freshest etag from calendarEvents just before PUT.
|
// WR-02: re-read the freshest etag from calendarEvents just before PUT.
|
||||||
// Rapid successive edits to the same uid enqueue multiple update rows, each
|
// Rapid successive edits to the same uid enqueue multiple update rows, each
|
||||||
// carrying the etag at enqueue time. If a prior edit succeeded and triggered
|
// carrying the etag at enqueue time. If a prior edit succeeded and triggered
|
||||||
@@ -210,7 +213,7 @@ async function dispatchRow(row: OutboxRow): Promise<DispatchResult> {
|
|||||||
// calendarUrl so the freshest etag belongs to the writing member.
|
// calendarUrl so the freshest etag belongs to the writing member.
|
||||||
let etagForPut: string | null = row.etag ?? null
|
let etagForPut: string | null = row.etag ?? null
|
||||||
const freshEtagRows = (await db
|
const freshEtagRows = (await db
|
||||||
.select({ etag: calendarEvents.etag })
|
.select({ etag: calendarEvents.etag, rawVevent: calendarEvents.rawVevent })
|
||||||
.from(calendarEvents)
|
.from(calendarEvents)
|
||||||
.innerJoin(calendars, eq(calendarEvents.calendarId, calendars.id))
|
.innerJoin(calendars, eq(calendarEvents.calendarId, calendars.id))
|
||||||
.where(
|
.where(
|
||||||
@@ -220,11 +223,28 @@ async function dispatchRow(row: OutboxRow): Promise<DispatchResult> {
|
|||||||
eq(calendars.url, row.calendarUrl),
|
eq(calendars.url, row.calendarUrl),
|
||||||
),
|
),
|
||||||
)
|
)
|
||||||
.limit(1)) as Array<{ etag: string | null }>
|
.limit(1)) as Array<{ etag: string | null; rawVevent: string | null }>
|
||||||
if (freshEtagRows.length > 0 && freshEtagRows[0].etag != null) {
|
if (freshEtagRows.length > 0 && freshEtagRows[0].etag != null) {
|
||||||
etagForPut = freshEtagRows[0].etag
|
etagForPut = freshEtagRows[0].etag
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// WR-01: when the edit payload carries no explicit recurrence, preserve the RRULE
|
||||||
|
// already on the stored event so an edit does not strip a recurring series.
|
||||||
|
if (!hasExplicitRecurrence && freshEtagRows.length > 0 && freshEtagRows[0].rawVevent) {
|
||||||
|
preservedRrule = extractRruleString(freshEtagRows[0].rawVevent)
|
||||||
|
}
|
||||||
|
|
||||||
|
const { icsString } = buildVeventString({
|
||||||
|
uid: row.uid,
|
||||||
|
summary: fields.title as string,
|
||||||
|
allDay: fields.allDay as boolean,
|
||||||
|
dtstart: fields.allDay ? (fields.start as string) : new Date(fields.start as string),
|
||||||
|
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: hasExplicitRecurrence ? rruleFromPayload : (preservedRrule ?? rruleFromPayload),
|
||||||
|
})
|
||||||
|
|
||||||
response = await updateCalendarEvent(
|
response = await updateCalendarEvent(
|
||||||
client,
|
client,
|
||||||
row.calendarObjectUrl,
|
row.calendarObjectUrl,
|
||||||
|
|||||||
@@ -43,6 +43,29 @@ export const RRULE_PRESETS: Record<string, string> = {
|
|||||||
yearly: 'FREQ=YEARLY',
|
yearly: 'FREQ=YEARLY',
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* WR-01: Extract the existing RRULE string from a stored VCALENDAR/VEVENT, so the
|
||||||
|
* outbox worker can preserve recurrence on an edit whose payload omits it (the PWA
|
||||||
|
* occurrence contract does not expose recurrence, D-03). Returns the RRULE value as a
|
||||||
|
* 'FREQ=…' string (e.g. 'FREQ=WEEKLY'), or undefined when the event has no RRULE or
|
||||||
|
* the input cannot be parsed.
|
||||||
|
*/
|
||||||
|
export function extractRruleString(rawVevent: string): string | undefined {
|
||||||
|
let parsed: ReturnType<typeof ICAL.parse>
|
||||||
|
try {
|
||||||
|
parsed = ICAL.parse(rawVevent)
|
||||||
|
} catch {
|
||||||
|
return undefined
|
||||||
|
}
|
||||||
|
const comp = new ICAL.Component(parsed)
|
||||||
|
const vevent = comp.getFirstSubcomponent('vevent')
|
||||||
|
if (!vevent) return undefined
|
||||||
|
const rrule = vevent.getFirstPropertyValue('rrule')
|
||||||
|
if (!rrule) return undefined
|
||||||
|
// ICAL.Recur#toString() yields the RECUR value, e.g. 'FREQ=WEEKLY'.
|
||||||
|
return typeof rrule === 'string' ? rrule : (rrule as ICAL.Recur).toString()
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Builds a VCALENDAR/VEVENT iCalendar string from form parameters.
|
* Builds a VCALENDAR/VEVENT iCalendar string from form parameters.
|
||||||
*
|
*
|
||||||
|
|||||||
@@ -138,7 +138,10 @@ export interface CreateEventPayload {
|
|||||||
allDay: boolean
|
allDay: boolean
|
||||||
start: string // 'YYYY-MM-DD' for allDay; ISO 8601 for timed
|
start: string // 'YYYY-MM-DD' for allDay; ISO 8601 for timed
|
||||||
end: string // same format as start
|
end: string // same format as start
|
||||||
recurrence: RecurrencePreset
|
// WR-01: optional. CREATE always sends it; EDIT omits it so the API/worker preserve
|
||||||
|
// the event's existing RRULE (the occurrence contract does not expose recurrence, so
|
||||||
|
// the form cannot echo it back without silently resetting it to 'none').
|
||||||
|
recurrence?: RecurrencePreset
|
||||||
location?: string
|
location?: string
|
||||||
description?: string
|
description?: string
|
||||||
calendarUrl?: string // omit to use the member's default writable calendar (D-01)
|
calendarUrl?: string // omit to use the member's default writable calendar (D-01)
|
||||||
|
|||||||
@@ -305,12 +305,19 @@ export function EventForm() {
|
|||||||
endTime,
|
endTime,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
// WR-01: on EDIT, omit `recurrence` from the payload. The occurrence/expand contract
|
||||||
|
// does not expose the event's existing recurrence (D-03), so the form cannot know it
|
||||||
|
// and would otherwise send 'none' — silently stripping the RRULE and converting a
|
||||||
|
// recurring series into a single event. Omitting the field signals "unchanged"; the
|
||||||
|
// outbox worker then preserves the stored RRULE (see outboxWorker.ts WR-01). On
|
||||||
|
// CREATE the user explicitly chose a recurrence, so it is always sent.
|
||||||
|
const isEdit = eventFormMode === 'edit' && !!eventFormUid
|
||||||
const payload: CreateEventPayload = {
|
const payload: CreateEventPayload = {
|
||||||
title: title.trim(),
|
title: title.trim(),
|
||||||
allDay,
|
allDay,
|
||||||
start: serializedStart,
|
start: serializedStart,
|
||||||
end: serializedEnd,
|
end: serializedEnd,
|
||||||
recurrence,
|
...(isEdit ? {} : { recurrence }),
|
||||||
...(location.trim() ? { location: location.trim() } : {}),
|
...(location.trim() ? { location: location.trim() } : {}),
|
||||||
...(description.trim() ? { description: description.trim() } : {}),
|
...(description.trim() ? { description: description.trim() } : {}),
|
||||||
...(writableCalendars.length > 1 && calendarUrl ? { calendarUrl } : {}),
|
...(writableCalendars.length > 1 && calendarUrl ? { calendarUrl } : {}),
|
||||||
@@ -702,7 +709,11 @@ export function EventForm() {
|
|||||||
</div>
|
</div>
|
||||||
)}
|
)}
|
||||||
|
|
||||||
{/* Recurrence picker (D-11: whole-series only) */}
|
{/* Recurrence picker (D-11: whole-series only).
|
||||||
|
WR-01: disabled in edit mode — the occurrence contract does not expose the
|
||||||
|
event's recurrence, so the form cannot show/change it without risking a
|
||||||
|
silent reset. Editing recurrence is deferred until the contract exposes it;
|
||||||
|
the existing RRULE is preserved server-side on edit. */}
|
||||||
<div style={fieldStyle}>
|
<div style={fieldStyle}>
|
||||||
<label htmlFor="event-recurrence" style={labelStyle}>
|
<label htmlFor="event-recurrence" style={labelStyle}>
|
||||||
Repeat
|
Repeat
|
||||||
@@ -710,8 +721,14 @@ export function EventForm() {
|
|||||||
<select
|
<select
|
||||||
id="event-recurrence"
|
id="event-recurrence"
|
||||||
value={recurrence}
|
value={recurrence}
|
||||||
|
disabled={eventFormMode === 'edit'}
|
||||||
onChange={(e) => setRecurrence(e.target.value as RecurrencePreset)}
|
onChange={(e) => setRecurrence(e.target.value as RecurrencePreset)}
|
||||||
style={{ ...inputStyle, padding: '0 var(--space-3)', cursor: 'pointer' }}
|
style={{
|
||||||
|
...inputStyle,
|
||||||
|
padding: '0 var(--space-3)',
|
||||||
|
cursor: eventFormMode === 'edit' ? 'not-allowed' : 'pointer',
|
||||||
|
opacity: eventFormMode === 'edit' ? 0.6 : 1,
|
||||||
|
}}
|
||||||
>
|
>
|
||||||
<option value="none">None</option>
|
<option value="none">None</option>
|
||||||
<option value="daily">Daily</option>
|
<option value="daily">Daily</option>
|
||||||
|
|||||||
Reference in New Issue
Block a user