Compare commits

..

2 Commits

Author SHA1 Message Date
bc49730ea5 fix: guard the detach path against a second copy, and say what it costs (#234)
Review pass on the detach path. Three changes, none to the write shape itself.

A second detach of the same occurrence is now refused. Reached from a stale
screen still pointing at the parent, it used to succeed twice over: the EXDATE
merge folds the repeated stamp away and the parent update still reports one row
changed, so the save looked fine and left a *second* standalone copy of one
occurrence, this time with neither copy in the series to make it obvious. The
occurrence is already gone from the series at that point, so the write reports
it as gone — and EventEditViewModel now maps NoSuchEventException from the write
to the same "this event no longer exists" answer its pre-check already gives,
instead of a bare "couldn't save" for something that isn't there.

The rollback comment claimed more than the code delivers. ContentResolver.update
returns rows touched, not occurrences excluded, so the zero check catches the
series row vanishing mid-write and nothing else — an EXDATE the provider's
expansion fails to match reports success and leaves the duplicate standing.
Renamed the variable to match, and the catch now mirrors moveEvent's Throwable
idiom rather than inventing a narrower one for the same insert-then-roll-back
shape.

The rest is honesty about what a detached row can't do, since none of it is
recoverable later and the KDoc previously mentioned only the calendar-move case:
a whole-series delete leaves it standing where an exception row would have gone
with the parent; a series-wide *time* edit moves the generated instances but not
the absolute-instant EXDATE hole, so the occurrence returns alongside its copy;
and building the row from the form rather than cloning the parent drops
ORGANIZER, STATUS and the organizer/resource attendee rows, exactly as moveEvent
does. The EXDATE staleness is not new — a #47 delete resurrects the same way —
but a duplicate is a louder symptom than a resurrection, and it wants fixing at
the series-update end, not here.

Also notes why the branch predicate is stricter than deleteOccurrence's: EXDATE
only means something on a row that recurs, so a row without an RRULE keeps the
old path rather than having a recurrence set written onto a one-off event.

Tests: the tautological "detached == copy(rrule = null)" assertion is replaced by
one that pins every edited field through to the inserted columns, and
exdateContains gets its own cases (timed, all-day, absent, a neighbouring
occurrence, and a multi-entry list with whitespace).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-27 20:10:22 +02:00
a1d1894f84 fix: detach the occurrence when editing one instance of an unsynced series (#234)
"Edit only this event" wrote a modified-occurrence exception unconditionally.
That is the right shape for a synced series and the wrong one for everything
else: an exception attaches to its parent through ORIGINAL_SYNC_ID, so on a row
with no _sync_id the link never forms — the insert fails or lands an orphan, the
generic catch in EventEditViewModel turns it into a snackbar, and the scope
dialog just closes again. To the reporter that read as "nothing happens, ever",
with a stray copy of the event left behind the one time the insert did land.

This is the same constraint deleteOccurrence has documented since #47, and the
same calendars: a local calendar, Calendula's own contact special-date
calendars, and — the reporter's case — a Google calendar whose rows the sync
adapter has not stamped yet, which is exactly why it "shows as on-device".

updateOccurrence now branches on _sync_id the way deleteOccurrence does. A
synced series keeps the exception path untouched; its write shape is load-
bearing and verified. A series without one gets the occurrence excluded from the
parent via EXDATE and the edited values inserted as a standalone event on the
same calendar — a detached instance, minus the RECURRENCE-ID the provider has no
way to store here. The cost is honest and worth naming: the edited occurrence
stops travelling with its series. The alternative is an edit that silently does
nothing.

Two things shape the write. The parent update reuses buildOccurrenceExdateValues
unchanged, so it keeps carrying the whole time/recurrence set — an EXDATE-only
update is not a recurrence change to the provider and leaves the expanded
instances standing (#47's first quirk). And the form's RRULE is stripped before
the insert: the exception path gets an inherited rule cleared for free by
DTSTART + DURATION, but nothing clears one here, so leaving it would insert a
second *series* overlapping the first.

Ordering is chosen for the failure cases, not the happy path. The insert runs
first, so a failure there leaves the series completely untouched — the discipline
updateEventFromOccurrence already follows. If the EXDATE update then fails, the
new row is a visible duplicate of an occurrence still in the series, so it is
rolled back (best effort) before the failure surfaces. The reverse order could
strand an occurrence excluded from its series with nothing standing in for it,
turning an edit into a silent delete.

Also makes a failed save legible, since this bug was invisible precisely because
it wasn't: the failure snackbar gets the long duration instead of a flash, and
the catch logs the scope and event id — never the form's content — so a failure
leaves something to report.

Adds JVM tests for the detached shape: the rule is dropped and the row becomes a
one-off with DTEND, all-day stays on UTC midnights, and the detached row's
DTSTART agrees with the EXDATE stamp that removes it from the parent, timed and
all-day. The provider behaviour itself still needs a device.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-27 19:57:35 +02:00
6 changed files with 364 additions and 411 deletions

View File

@@ -8,16 +8,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
## [Unreleased]
### Fixed
- **A deleted occurrence no longer comes back when the series is re-timed.**
Delete a single occurrence of a repeating event, then change the time of the
whole series, and the occurrence you had removed reappeared. Doing it the
other way round — "This and all following events" — lost every removal in the
part of the series being changed. Removals now travel with the event: they
keep their place in the series across a time change, a move to a different
day, a timezone change and a switch to or from an all-day event, and they
carry over when a series is split. Repeating events on your device's own
calendars are affected, including the birthday and anniversary calendars
Calendula creates from your contacts ([#248]).
- **"Only this event" now actually saves your edit.** On some calendars —
including Google ones that still show as on-device, and any local calendar —
editing a single occurrence of a repeating event did nothing at all: the scope
dialog closed, the edit screen stayed put, and saving again just repeated it.
Android can only attach a single-occurrence change to its series once the
calendar has been synced at least once, so on those calendars the change had
nowhere to go. Calendula now removes that one occurrence from the series and
saves the edit as its own event instead, which is what you see either way. A
save that does fail also says so for longer, rather than flashing past
([#234]).
## [2.19.3] — 2026-08-22
@@ -1483,4 +1483,4 @@ automatically, with zero telemetry and no internet permission.
[#192]: https://codeberg.org/jlmakiola/calendula/issues/192
[#196]: https://codeberg.org/jlmakiola/calendula/issues/196
[#214]: https://codeberg.org/jlmakiola/calendula/issues/214
[#248]: https://codeberg.org/jlmakiola/calendula/issues/248
[#234]: https://codeberg.org/jlmakiola/calendula/issues/234

View File

@@ -232,10 +232,15 @@ interface CalendarDataSource {
): Long
/**
* Change a single occurrence of a recurring event by inserting a
* modified-occurrence exception at [beginMillis] (the occurrence's
* `Instances.BEGIN`) carrying [form]'s values; returns the exception
* row's `Events._ID`. [allDayReminderTimeMinutes]: see [insertEvent].
* Change a single occurrence of a recurring event at [beginMillis] (the
* occurrence's `Instances.BEGIN`) to [form]'s values; returns the
* `Events._ID` of the row now holding them.
*
* A series with a `_sync_id` gets a modified-occurrence exception. One
* without gets the occurrence excluded from the parent via EXDATE plus a
* standalone event carrying the edits — an exception cannot link to its
* parent there (Codeberg #234, the same constraint as [deleteOccurrence]).
* [allDayReminderTimeMinutes]: see [insertEvent].
*/
fun updateOccurrence(
eventId: Long,
@@ -976,12 +981,10 @@ class AndroidCalendarDataSource @Inject constructor(
updated: EventForm,
allDayReminderTimeMinutes: Int,
) {
val row = querySeriesRow(eventId)
val values = buildEventUpdateValues(
original = original,
updated = updated,
seriesDtStartMillis = row.dtStartMillis,
seriesExdate = row.exdate,
seriesDtStartMillis = querySeriesRow(eventId).dtStartMillis,
zone = ZoneId.systemDefault(),
)
if (values.isNotEmpty()) {
@@ -1196,6 +1199,16 @@ class AndroidCalendarDataSource @Inject constructor(
form: EventForm,
allDayReminderTimeMinutes: Int,
): Long {
val row = querySeriesRow(eventId)
// Deliberately stricter than deleteOccurrence's bare _sync_id check: the
// detach path drops the occurrence with EXDATE, which only means anything
// on a row that actually recurs. Without an RRULE there is nothing to
// exclude from, so such a row keeps the existing path rather than getting
// a recurrence set written onto a one-off event. The UI only offers the
// scope choice for a recurring event, so neither case is reachable today.
if (row.syncId == null && !row.rrule.isNullOrBlank()) {
return detachOccurrence(eventId, beginMillis, row, form, allDayReminderTimeMinutes)
}
// The provider clones the series row and applies these values on top.
val values = buildOccurrenceExceptionValues(
form = form,
@@ -1214,6 +1227,116 @@ class AndroidCalendarDataSource @Inject constructor(
return exceptionId
}
/**
* "Edit only this event" on a series with **no `_sync_id`**: drop the
* occurrence from the parent with EXDATE and insert the edited values as a
* standalone event on the same calendar.
*
* A modified exception attaches to its parent only through `ORIGINAL_SYNC_ID`,
* exactly like the cancelled one [deleteOccurrence] documents. With no
* `_sync_id` the link never forms: the insert either fails outright or lands
* an orphan row, and the parent's expansion collapses — which is why editing
* one occurrence of such a series did nothing at all, and occasionally left a
* stray copy behind (Codeberg #234). The reporter's calendar was a Google one
* that "shows as on-device", i.e. rows the sync adapter had not stamped yet;
* Calendula's own local and contact special-date calendars are permanently in
* this shape.
*
* EXDATE + a standalone row needs no parent link, and is what a detached
* instance degrades to when there is no `RECURRENCE-ID` to carry it: the user
* sees one edited event where the occurrence was, and the rest of the series
* untouched.
*
* What that costs, plainly, because none of it is recoverable later — the
* detached row has *no* stored link back to its series, which is the whole
* reason this path exists:
* - It stops travelling with the series. Moving the series to another
* calendar leaves it behind ([moveEvent] copies the master and its
* `ORIGINAL_ID` children; this is neither), and deleting the whole series
* leaves it standing where an exception row would have gone with the parent.
* - Its EXDATE hole is an absolute instant. Editing the series' *time* for all
* events moves the generated instances but not the hole, so the occurrence
* comes back alongside the detached copy. That staleness predates this path
* — a #47 delete resurrects the same way — but a duplicate is a louder
* symptom than a resurrection, and it wants fixing at the series-update end.
* - It is built from the form, not cloned from the parent, so columns the form
* doesn't model are dropped rather than inherited: `ORGANIZER`, `STATUS`,
* and the organizer/resource attendee rows [reconcileAttendees] otherwise
* preserves. Same limitation as [moveEvent]. Rare on the calendars that
* reach this path, which are locally authored, but not impossible.
*
* Order is deliberate. The insert goes first, so a failure there leaves the
* series completely untouched (the same discipline as
* [updateEventFromOccurrence]). If the EXDATE update then fails, the new row
* is a visible duplicate of an occurrence that is still in the series, so it
* is rolled back before the failure surfaces — better a save the user can
* retry than a silent duplicate. The reverse order would risk the opposite:
* an occurrence excluded from the series with no replacement, i.e. an edit
* that quietly deletes.
*/
private fun detachOccurrence(
eventId: Long,
beginMillis: Long,
row: SeriesRow,
form: EventForm,
allDayReminderTimeMinutes: Int,
): Long {
// Already detached (or deleted): the series no longer contains this
// occurrence, so there is nothing here to edit. Reached from a stale
// screen still pointing at the parent — without this the EXDATE merge
// would fold the repeat away, the update would still report a changed
// row, and the save would quietly leave a *second* standalone copy.
if (exdateContains(row.exdate, beginMillis, isAllDay = row.allDay != 0)) {
throw NoSuchEventException(eventId)
}
// Carries the form's reminders, guests and colour like any new event —
// and a fresh UID, because the detached row really is a separate event
// now: sharing the parent's would collide with it in .ics restore dedup.
val detachedId = insertEvent(form.toDetachedOccurrence(), allDayReminderTimeMinutes)
val values = buildOccurrenceExdateValues(
existingExdate = row.exdate,
occurrenceMillis = beginMillis,
dtStartMillis = row.dtStartMillis,
rrule = row.rrule,
duration = row.duration,
timezone = row.timezone,
allDay = row.allDay,
)
// Rows touched, not occurrences excluded: this is 1 whenever the series
// row still exists. It catches the row disappearing under us, not an
// EXDATE the provider's expansion fails to match — that would report
// success and leave the duplicate. Same rollback idiom as moveEvent.
val updatedRows = try {
resolver.update(
ContentUris.withAppendedId(CalendarContract.Events.CONTENT_URI, eventId),
values.toContentValues(), null, null,
)
} catch (t: Throwable) {
rollBackDetached(detachedId)
throw t
}
if (updatedRows == 0) {
rollBackDetached(detachedId)
throw WriteFailedException(
"exdate occurrence for edit, event id=$eventId begin=$beginMillis",
)
}
return detachedId
}
/**
* Undo the standalone row [detachOccurrence] inserted before its EXDATE
* update failed. Best effort: the caller is already throwing, and a rollback
* that itself fails must not replace the real failure with a confusing one.
* The worst case is the duplicate we were trying to avoid, which the user can
* see and delete — never a lost occurrence.
*/
private fun rollBackDetached(detachedId: Long) {
runCatching { deleteEvent(detachedId) }.onFailure {
Log.w(TAG, "Failed to roll back detached occurrence $detachedId", it)
}
}
override fun updateEventFromOccurrence(
eventId: Long,
beginMillis: Long,
@@ -1230,66 +1353,10 @@ class AndroidCalendarDataSource @Inject constructor(
}
// Insert the new series first: if it fails, the original is untouched.
val newEventId = insertEvent(updated, allDayReminderTimeMinutes)
carrySplitExdate(newEventId, row, beginMillis, original, updated)
truncateSeries(eventId, row, beginMillis)
return newEventId
}
/**
* Carry the [parent]'s exclusions for occurrences past [beginMillis] onto the
* series [newEventId] that now owns them, re-timed by the shift the split
* applied ([shiftedExdate]/[exdateAfter]). [insertEvent] builds the new row
* from the form, which knows nothing about them, so without this every
* occurrence the user had deleted from the tail of the series comes back.
*
* The whole time/recurrence set rides along with EXDATE for the reason
* [buildOccurrenceExdateValues] documents: on its own the provider does not
* read an EXDATE write as a recurrence change, and leaves the instances it
* expanded on insert standing.
*
* A failure rolls the new series back and throws, so the split fails whole
* rather than landing with the exclusions quietly dropped — the parent is
* still untruncated at this point, so the event is left exactly as it was.
*/
private fun carrySplitExdate(
newEventId: Long,
parent: SeriesRow,
beginMillis: Long,
original: EventForm,
updated: EventForm,
) {
// Dropping the recurrence in the same save leaves a one-off tail, and an
// exclusion means nothing on a row that doesn't recur.
if (updated.rrule.isNullOrBlank()) return
val carried = shiftedExdate(
existingExdate = exdateAfter(parent.exdate, beginMillis, original.isAllDay),
original = original,
updated = updated,
zone = ZoneId.systemDefault(),
) ?: return
val row = querySeriesRow(newEventId)
val values = ContentValues().apply {
put(CalendarContract.Events.EXDATE, carried)
put(CalendarContract.Events.DTSTART, row.dtStartMillis)
put(CalendarContract.Events.RRULE, row.rrule)
put(CalendarContract.Events.DURATION, row.duration)
put(CalendarContract.Events.EVENT_TIMEZONE, row.timezone)
put(CalendarContract.Events.ALL_DAY, row.allDay)
}
try {
val rows = resolver.update(
ContentUris.withAppendedId(CalendarContract.Events.CONTENT_URI, newEventId),
values, null, null,
)
if (rows == 0) {
throw WriteFailedException("carry exdate onto split series id=$newEventId")
}
} catch (t: Throwable) {
runCatching { deleteEvent(newEventId) }
throw t
}
}
override fun deleteEventFromOccurrence(eventId: Long, beginMillis: Long) {
val row = querySeriesRow(eventId)
// From the first occurrence on = the whole series; also the fallback

View File

@@ -10,9 +10,6 @@ import java.time.Duration
import java.time.Instant
import java.time.ZoneId
import java.time.ZoneOffset
import java.time.format.DateTimeFormatter
import java.time.format.ResolverStyle
import java.time.LocalDate as JavaLocalDate
import java.time.LocalDateTime as JavaLocalDateTime
/** Provider-ready DTSTART / DTEND / EVENT_TIMEZONE for an event write. */
@@ -137,21 +134,18 @@ internal fun buildEventInsertValues(
*
* Time fields travel together (the provider validates them as a unit):
* - unchanged times, all-day flag and rrule → no time columns at all;
* - non-recurring result → DTSTART/DTEND, DURATION, RRULE and EXDATE cleared;
* - non-recurring result → DTSTART/DTEND, DURATION and RRULE cleared;
* - recurring result → the *series* DTSTART moves by the same **wall-clock**
* shift the user applied to the displayed occurrence and is re-resolved in the
* event's zone ([seriesDtStartMillis] is the row's current DTSTART), DURATION
* replaces DTEND, RRULE is written, and the row's exclusions
* ([seriesExdate], the current EXDATE) travel with the anchor. This keeps past
* occurrences intact when someone edits a later occurrence's time, and keeps
* the anchor's time-of-day stable across a DST boundary or a zone change
* between the two.
* replaces DTEND, RRULE is written. This keeps past occurrences intact when
* someone edits a later occurrence's time, and keeps the anchor's time-of-day
* stable across a DST boundary or a zone change between the two.
*/
internal fun buildEventUpdateValues(
original: EventForm,
updated: EventForm,
seriesDtStartMillis: Long,
seriesExdate: String?,
zone: ZoneId,
): Map<String, Any?> = buildMap {
if (updated.title.trim() != original.title.trim()) {
@@ -191,10 +185,6 @@ internal fun buildEventUpdateValues(
put(CalendarContract.Events.DTEND, newTimes.dtEndMillis)
put(CalendarContract.Events.RRULE, null)
put(CalendarContract.Events.DURATION, null)
// The exclusions named occurrences of a series that no longer exists.
// Left behind they are dormant rather than harmless: adding a recurrence
// back later would punch the old holes into the new one.
put(CalendarContract.Events.EXDATE, null)
} else {
// Move the series anchor by the *wall-clock* shift the user applied to the
// displayed occurrence, then re-resolve it in the event's (possibly new)
@@ -216,14 +206,6 @@ internal fun buildEventUpdateValues(
put(CalendarContract.Events.DTEND, null)
put(CalendarContract.Events.RRULE, updated.rrule)
put(CalendarContract.Events.DURATION, newTimes.toRfc2445Duration(updated.isAllDay))
// An EXDATE stamp is an absolute instant, so it excludes an occurrence
// only while the series keeps generating one at exactly that instant. The
// anchor has just moved, so every occurrence regenerates elsewhere and a
// stamp left behind matches none of them — the occurrence the user deleted
// comes back. Move the stamps the same way the anchor moved.
shiftedExdate(seriesExdate, original, updated, zone)
?.takeIf { it != seriesExdate }
?.let { put(CalendarContract.Events.EXDATE, it) }
}
}
@@ -261,6 +243,26 @@ internal fun buildOccurrenceExceptionValues(
putAll(eventColorColumns(form.colorKey, form.color))
}
/**
* The form as a **detached occurrence**: the same edited values, with the
* series rule dropped so [buildEventInsertValues] writes a standalone one-off
* row (DTSTART + DTEND, no RRULE/DURATION) at the occurrence's own times.
*
* This is the "edit only this event" shape for a series with **no `_sync_id`**,
* where an exception row can't be used at all — see [buildOccurrenceExdateValues]
* for why the parent link never forms. The occurrence is dropped from the parent
* with EXDATE and re-created as its own event, which is what a detached instance
* degrades to without a `RECURRENCE-ID` to carry it.
*
* Dropping the rule mirrors what the exception path gets for free: the provider
* clears the RRULE it cloned from the parent when an exception carries
* DTSTART + DURATION ([buildOccurrenceExceptionValues]). Here nothing is cloned,
* so the rule has to be stripped by hand — leaving it on would insert a second
* *series* overlapping the first, which is the duplication this path exists to
* avoid.
*/
internal fun EventForm.toDetachedOccurrence(): EventForm = copy(rrule = null)
/**
* Raw provider snapshot of a master/one-off Events row, enough to re-insert it
* verbatim on another calendar (a calendar move is copy+delete — `CALENDAR_ID`
@@ -438,7 +440,11 @@ internal fun buildOccurrenceExdateValues(
allDay: Int,
): Map<String, Any?> {
val stamp = formatExdateStamp(occurrenceMillis, isAllDay = allDay != 0)
val merged = (exdateStamps(existingExdate) + stamp).distinct().joinToString(",")
val existing = existingExdate?.split(',')
?.map { it.trim() }
?.filter { it.isNotEmpty() }
.orEmpty()
val merged = (existing + stamp).distinct().joinToString(",")
return mapOf(
CalendarContract.Events.EXDATE to merged,
CalendarContract.Events.DTSTART to dtStartMillis,
@@ -450,121 +456,41 @@ internal fun buildOccurrenceExdateValues(
}
/**
* [existingExdate] with every stamp re-timed by the same **wall-clock** shift the
* series anchor takes from [original] to [updated] — the exclusions' half of the
* anchor move [buildEventUpdateValues] performs, and what carries them onto the
* new series when "this and following" splits one.
* Whether [existingExdate] already excludes the occurrence at [occurrenceMillis]
* — i.e. this occurrence has already been dropped from the series (deleted, or
* detached into its own event).
*
* A stamp is an absolute instant, so it keeps naming its occurrence only if it
* moves exactly as the occurrence does: shifted in wall clock and re-resolved in
* the event's (possibly new) zone. A millisecond delta would instead bake in the
* offset that happened to apply at the edited occurrence — an hour off for any
* exclusion on the far side of a DST boundary. All-day-ness is read from
* [original] and written from [updated], so the `VALUE=DATE` and date-time forms
* convert into each other when the event switches.
*
* Null when there is nothing to carry: no stamps, or a stamp in a form Calendula
* never writes (a `TZID=`-parameterised or floating one from a sync adapter). Those
* are left exactly as they are rather than guessed at — a stale stamp excludes
* nothing, but a mangled one could exclude the wrong occurrence.
* Guards the detach path against running twice for the same occurrence, which a
* stale detail screen still pointing at the parent can otherwise reach. The
* EXDATE merge folds the repeat away silently and the parent update still
* reports one row changed, so without this check the second save would leave a
* *second* standalone copy and call it a success.
*/
internal fun shiftedExdate(
internal fun exdateContains(
existingExdate: String?,
original: EventForm,
updated: EventForm,
zone: ZoneId,
): String? {
val stamps = exdateStamps(existingExdate)
if (stamps.isEmpty()) return null
val fromZone = original.writeZone(zone)
val toZone = updated.writeZone(zone)
val wallClockShift = Duration.between(original.anchorLocal(), updated.anchorLocal())
return stamps
.map { stamp ->
val local = parseExdateStamp(stamp, original.isAllDay, fromZone) ?: return null
formatExdateStamp(local.plus(wallClockShift), updated.isAllDay, toZone)
}
.distinct()
.joinToString(",")
occurrenceMillis: Long,
isAllDay: Boolean,
): Boolean {
val stamp = formatExdateStamp(occurrenceMillis, isAllDay)
return existingExdate?.split(',')?.any { it.trim() == stamp } == true
}
/**
* The stamps of [existingExdate] naming occurrences after [beginMillis] — the ones
* that belong to the *new* series once "this and following" splits a recurring
* event there. The parent keeps the full list; its stamps past the split point are
* simply inert once it stops generating those occurrences.
*
* The occurrence at [beginMillis] itself is never carried. It is the one the user
* is editing, so it exists by definition, and honouring a stale exclusion for it
* would swallow the edit whole.
*
* Null when nothing qualifies, or when a stamp can't be read (see [shiftedExdate]).
*/
internal fun exdateAfter(existingExdate: String?, beginMillis: Long, isAllDay: Boolean): String? {
val stamps = exdateStamps(existingExdate)
if (stamps.isEmpty()) return null
return stamps
.filter { stamp ->
val utc = parseExdateStamp(stamp, isAllDay, ZoneOffset.UTC) ?: return null
utc.toInstant(ZoneOffset.UTC).toEpochMilli() > beginMillis
}
.takeIf { it.isNotEmpty() }
?.joinToString(",")
}
/** The individual stamps of an EXDATE column value; it is a comma-separated list. */
private fun exdateStamps(exdate: String?): List<String> = exdate
?.split(',')
?.map { it.trim() }
?.filter { it.isNotEmpty() }
.orEmpty()
/**
* One EXDATE entry for the occurrence starting at [occurrenceMillis]. Both forms
* are UTC: the provider stores an all-day DTSTART at UTC midnight, so its date
* reads off the UTC calendar day.
*/
private fun formatExdateStamp(occurrenceMillis: Long, isAllDay: Boolean): String =
formatExdateStamp(
local = Instant.ofEpochMilli(occurrenceMillis).atZone(ZoneOffset.UTC).toLocalDateTime(),
isAllDay = isAllDay,
zone = ZoneOffset.UTC,
)
/**
* One EXDATE entry for the occurrence whose wall clock in [zone] is [local]. An
* all-day entry keeps only the date (its time-of-day is the anchor's, not the
* occurrence's); a timed one is resolved in [zone] and written as a UTC instant.
*/
private fun formatExdateStamp(local: JavaLocalDateTime, isAllDay: Boolean, zone: ZoneId): String =
if (isAllDay) {
local.toLocalDate().format(ALL_DAY_EXDATE)
private fun formatExdateStamp(occurrenceMillis: Long, isAllDay: Boolean): String {
val utc = Instant.ofEpochMilli(occurrenceMillis).atZone(ZoneOffset.UTC)
return if (isAllDay) {
"%04d%02d%02d".format(utc.year, utc.monthValue, utc.dayOfMonth)
} else {
local.atZone(zone).withZoneSameInstant(ZoneOffset.UTC).format(TIMED_EXDATE)
"%04d%02d%02dT%02d%02d%02dZ".format(
utc.year, utc.monthValue, utc.dayOfMonth,
utc.hour, utc.minute, utc.second,
)
}
/**
* [stamp] as the wall clock it names in [zone] — the inverse of
* [formatExdateStamp]. Null for anything but the two forms Calendula writes, so a
* caller can tell "not ours, leave it alone" from a value it may safely re-time.
*/
private fun parseExdateStamp(stamp: String, isAllDay: Boolean, zone: ZoneId): JavaLocalDateTime? =
runCatching {
if (isAllDay) {
JavaLocalDate.parse(stamp, ALL_DAY_EXDATE).atStartOfDay()
} else {
JavaLocalDateTime.parse(stamp, TIMED_EXDATE)
.atZone(ZoneOffset.UTC).withZoneSameInstant(zone).toLocalDateTime()
}
}.getOrNull()
/** `VALUE=DATE` EXDATE form, for an all-day series. */
private val ALL_DAY_EXDATE: DateTimeFormatter =
DateTimeFormatter.ofPattern("uuuuMMdd").withResolverStyle(ResolverStyle.STRICT)
/** UTC date-time EXDATE form, for a timed series. */
private val TIMED_EXDATE: DateTimeFormatter =
DateTimeFormatter.ofPattern("uuuuMMdd'T'HHmmss'Z'").withResolverStyle(ResolverStyle.STRICT)
}
/**
* The `EVENT_COLOR` / `EVENT_COLOR_KEY` columns for a colour selection. A

View File

@@ -64,6 +64,7 @@ import androidx.compose.material3.Scaffold
import androidx.compose.material3.SegmentedButton
import androidx.compose.material3.SegmentedButtonDefaults
import androidx.compose.material3.SingleChoiceSegmentedButtonRow
import androidx.compose.material3.SnackbarDuration
import androidx.compose.material3.SnackbarHost
import androidx.compose.material3.SnackbarHostState
import androidx.compose.material3.Surface
@@ -264,13 +265,17 @@ fun EventEditScreen(
viewModel.reset()
onSaved()
}
// A failed save leaves the user on a form that looks exactly as it
// did, so the snackbar is the only sign anything happened — it gets
// the long duration rather than the default flash (Codeberg #234:
// the failure read as "nothing happens at all").
SaveUiState.Failed -> {
viewModel.consumeSaveResult()
snackbarHostState.showSnackbar(saveFailedMessage)
snackbarHostState.showSnackbar(saveFailedMessage, duration = SnackbarDuration.Long)
}
SaveUiState.NeedsPermission -> {
viewModel.consumeSaveResult()
snackbarHostState.showSnackbar(writeDeniedMessage)
snackbarHostState.showSnackbar(writeDeniedMessage, duration = SnackbarDuration.Long)
}
// AwaitingScope/AwaitingConflict/Gone render as dialogs below.
else -> Unit

View File

@@ -1,5 +1,6 @@
package de.jeanlucmakiola.calendula.ui.edit
import android.util.Log
import androidx.lifecycle.ViewModel
import androidx.lifecycle.viewModelScope
import dagger.hilt.android.lifecycle.HiltViewModel
@@ -56,6 +57,8 @@ import kotlin.time.Duration.Companion.minutes
import kotlin.time.Instant
import javax.inject.Inject
private const val TAG = "EventEdit"
/**
* Where a prefilled [EventEditViewModel.openImported] form came from. The sources
* want different reminder handling (#49), and differ in whether they own the
@@ -751,7 +754,16 @@ class EventEditViewModel @Inject constructor(
throw e
} catch (e: SecurityException) {
SaveUiState.NeedsPermission
} catch (e: NoSuchEventException) {
// The write found the event (or the occurrence) already gone —
// the same answer the pre-check gives, and a far better one than
// a bare "couldn't save" for something that no longer exists.
SaveUiState.Gone
} catch (e: Exception) {
// The user only gets a generic snackbar, so without this a
// failed write leaves no trace at all to report (Codeberg #234).
// Scope and event id only — never the form's content.
Log.w(TAG, "Save failed (scope=$scope, eventId=${target?.eventId})", e)
SaveUiState.Failed
}
}

View File

@@ -128,8 +128,7 @@ class EventWriteMapperTest {
original: EventForm,
updated: EventForm,
series: Long = seriesStart,
exdate: String? = null,
): Map<String, Any?> = buildEventUpdateValues(original, updated, series, exdate, berlin)
): Map<String, Any?> = buildEventUpdateValues(original, updated, series, berlin)
/** The instant [local] names in [zoneId], as the provider would store it. */
private fun instantAt(local: String, zoneId: String): Long =
@@ -559,237 +558,181 @@ class EventWriteMapperTest {
assertThat(values[CalendarContract.Events.ALL_DAY]).isEqualTo(1)
}
// --- EXDATE follows the series when its times change (Codeberg #248) ---
/** A weekly series whose displayed occurrence runs 15 July 2026, 09:0010:00. */
private fun julySeries(): EventForm = form(
start = LocalDateTime(LocalDate(2026, 7, 15), LocalTime(9, 0)),
end = LocalDateTime(LocalDate(2026, 7, 15), LocalTime(10, 0)),
).copy(rrule = "FREQ=WEEKLY")
/** [julySeries] pushed to [hour]:00, the shift an "all events" time edit makes. */
private fun EventForm.atHour(hour: Int): EventForm = copy(
start = LocalDateTime(LocalDate(2026, 7, 15), LocalTime(hour, 0)),
end = LocalDateTime(LocalDate(2026, 7, 15), LocalTime(hour + 1, 0)),
)
// --- toDetachedOccurrence ("edit only this event", no _sync_id) ---
@Test
fun `a series time edit moves its exclusions with the anchor`() {
// The bug: the stamp stayed at the old instant, which the moved series no
// longer generates, so the occurrence the user deleted came back.
val series = instantAt("2026-07-01T09:00", "Europe/Berlin")
val original = julySeries()
// 8 July 09:00 Berlin (CEST, +2) == 07:00Z; at 10:00 it must read 08:00Z.
val values = update(original, original.atHour(10), series, "20260708T070000Z")
assertThat(values[CalendarContract.Events.EXDATE]).isEqualTo("20260708T080000Z")
}
fun `a detached occurrence drops the series rule and becomes a one-off row`() {
val edited = form().copy(title = "Moved", rrule = "FREQ=WEEKLY;BYDAY=TH")
val detached = edited.toDetachedOccurrence()
@Test
fun `an exclusion keeps its wall clock across a DST boundary`() {
// Anchor and edited occurrence are in July (CEST, +2); the excluded
// occurrence sits in January (CET, +1). Shifting by the millisecond delta
// measured at the edited occurrence would leave it an hour off.
val series = instantAt("2026-01-07T09:00", "Europe/Berlin")
val original = julySeries()
// 14 January 09:00 Berlin == 08:00Z; at 10:00 it must read 09:00Z.
val values = update(original, original.atHour(10), series, "20260114T080000Z")
assertThat(values[CalendarContract.Events.EXDATE]).isEqualTo("20260114T090000Z")
}
@Test
fun `pinning a series to another zone re-resolves its exclusions there`() {
val series = instantAt("2026-07-01T09:00", "Europe/Berlin")
val original = julySeries()
val values = update(
original,
original.copy(timezone = "Asia/Tokyo"),
series,
"20260716T070000Z",
val values = buildEventInsertValues(
form = detached,
uid = "uid@calendula",
times = detached.toWriteTimes(berlin),
)
// The exclusion still reads 09:00 — now 09:00 in Tokyo (+9) == 00:00Z.
assertThat(values[CalendarContract.Events.EXDATE]).isEqualTo("20260716T000000Z")
assertThat(values[CalendarContract.Events.TITLE]).isEqualTo("Moved")
// The rule must not survive: without a _sync_id nothing clears an
// inherited RRULE the way the exception path's DTSTART + DURATION does,
// so keeping it would insert a second *series* overlapping the first
// (Codeberg #234's stray duplicate).
assertThat(values).doesNotContainKey(CalendarContract.Events.RRULE)
assertThat(values).doesNotContainKey(CalendarContract.Events.DURATION)
// A one-off row carries DTEND — the invariant buildEventInsertValues
// holds for every non-recurring event.
assertThat(values[CalendarContract.Events.DTSTART]).isEqualTo(1_781_164_800_000L)
assertThat(values[CalendarContract.Events.DTEND]).isEqualTo(1_781_170_200_000L)
assertThat(values[CalendarContract.Events.EVENT_TIMEZONE]).isEqualTo("Europe/Berlin")
}
@Test
fun `an all-day series move shifts its exclusions by whole days`() {
val series = instantAt("2026-07-01T00:00", "UTC")
val original = form(
fun `a detached occurrence carries every edited field onto the new row`() {
// The detached row is built from the form rather than cloned from the
// parent, so anything the user edited has to survive the trip — a field
// dropped here is an edit silently lost.
val edited = form(timezone = "America/New_York").copy(
title = " Standup ",
location = "Room 2",
description = "notes",
reminders = listOf(10),
availability = Availability.Free,
accessLevel = AccessLevel.Private,
rrule = "FREQ=DAILY",
)
val detached = edited.toDetachedOccurrence()
val values = buildEventInsertValues(
form = detached,
uid = "uid@calendula",
times = detached.toWriteTimes(berlin),
)
assertThat(values[CalendarContract.Events.TITLE]).isEqualTo("Standup")
assertThat(values[CalendarContract.Events.EVENT_LOCATION]).isEqualTo("Room 2")
assertThat(values[CalendarContract.Events.DESCRIPTION]).isEqualTo("notes")
assertThat(values[CalendarContract.Events.AVAILABILITY])
.isEqualTo(CalendarContract.Events.AVAILABILITY_FREE)
assertThat(values[CalendarContract.Events.ACCESS_LEVEL])
.isEqualTo(CalendarContract.Events.ACCESS_PRIVATE)
// The pinned zone survives too — a detached occurrence must not be
// silently re-anchored to the device.
assertThat(values[CalendarContract.Events.EVENT_TIMEZONE]).isEqualTo("America/New_York")
assertThat(values[CalendarContract.Events.UID_2445]).isEqualTo("uid@calendula")
// Reminders and guests aren't columns — the insert path seeds them from
// the form, so they only have to survive on the form itself.
assertThat(detached.reminders).containsExactly(10)
}
@Test
fun `a detached all-day occurrence stays on UTC midnights`() {
val edited = form(
isAllDay = true,
start = LocalDateTime(LocalDate(2026, 7, 15), LocalTime(0, 0)),
end = LocalDateTime(LocalDate(2026, 7, 15), LocalTime(0, 0)),
).copy(rrule = "FREQ=WEEKLY")
val moved = original.copy(
start = LocalDateTime(LocalDate(2026, 7, 17), LocalTime(0, 0)),
end = LocalDateTime(LocalDate(2026, 7, 17), LocalTime(0, 0)),
start = LocalDateTime(LocalDate(2026, 6, 11), LocalTime(0, 0)),
end = LocalDateTime(LocalDate(2026, 6, 11), LocalTime(0, 0)),
).copy(title = "Birthday", rrule = "FREQ=YEARLY")
val detached = edited.toDetachedOccurrence()
val values = buildEventInsertValues(
form = detached,
uid = "uid@calendula",
times = detached.toWriteTimes(berlin),
)
assertThat(update(original, moved, series, "20260722")[CalendarContract.Events.EXDATE])
.isEqualTo("20260724")
assertThat(values[CalendarContract.Events.ALL_DAY]).isEqualTo(1)
assertThat(values[CalendarContract.Events.EVENT_TIMEZONE]).isEqualTo("UTC")
assertThat(values[CalendarContract.Events.DTSTART]).isEqualTo(1_781_136_000_000L)
// Exclusive DTEND — the next UTC midnight.
assertThat(values[CalendarContract.Events.DTEND]).isEqualTo(1_781_222_400_000L)
assertThat(values).doesNotContainKey(CalendarContract.Events.RRULE)
}
@Test
fun `switching a series to all-day rewrites its exclusions as dates`() {
// The two forms aren't interchangeable: a date-time stamp on an all-day
// series matches no occurrence, so the exclusion would be lost.
val series = instantAt("2026-07-01T09:00", "Europe/Berlin")
val original = julySeries()
val values = update(original, original.copy(isAllDay = true), series, "20260722T070000Z")
assertThat(values[CalendarContract.Events.EXDATE]).isEqualTo("20260722")
fun `the detached row lands exactly where the parent's exdate removes it`() {
// The two halves of the no-_sync_id edit have to agree, or the user sees
// the occurrence twice or not at all. With the time left untouched, the
// EXDATE stamp and the detached row's DTSTART describe the same instant.
val edited = form().copy(title = "Renamed", rrule = "FREQ=WEEKLY")
val occurrenceMillis = 1_781_164_800_000L
val parent = buildOccurrenceExdateValues(
existingExdate = null,
occurrenceMillis = occurrenceMillis,
dtStartMillis = 1_780_560_000_000L,
rrule = "FREQ=WEEKLY",
duration = "P5400S",
timezone = "Europe/Berlin",
allDay = 0,
)
val detached = edited.toDetachedOccurrence()
val inserted = buildEventInsertValues(
form = detached,
uid = "uid@calendula",
times = detached.toWriteTimes(berlin),
)
assertThat(parent[CalendarContract.Events.EXDATE]).isEqualTo("20260611T080000Z")
assertThat(inserted[CalendarContract.Events.DTSTART]).isEqualTo(occurrenceMillis)
// The parent keeps its own anchor and rule — only this occurrence leaves.
assertThat(parent[CalendarContract.Events.DTSTART]).isEqualTo(1_780_560_000_000L)
assertThat(parent[CalendarContract.Events.RRULE]).isEqualTo("FREQ=WEEKLY")
}
@Test
fun `switching a series back to timed rewrites its exclusions as instants`() {
val series = instantAt("2026-07-01T00:00", "UTC")
val original = form(
fun `a detached all-day occurrence matches its date-only exdate stamp`() {
val parent = buildOccurrenceExdateValues(
existingExdate = null,
occurrenceMillis = 1_781_136_000_000L, // 2026-06-11T00:00:00Z
dtStartMillis = 1_749_600_000_000L,
rrule = "FREQ=YEARLY",
duration = "P1D",
timezone = "UTC",
allDay = 1,
)
val detached = form(
isAllDay = true,
start = LocalDateTime(LocalDate(2026, 7, 15), LocalTime(0, 0)),
end = LocalDateTime(LocalDate(2026, 7, 15), LocalTime(0, 0)),
).copy(rrule = "FREQ=WEEKLY")
val timed = original.copy(
isAllDay = false,
start = LocalDateTime(LocalDate(2026, 7, 15), LocalTime(9, 0)),
end = LocalDateTime(LocalDate(2026, 7, 15), LocalTime(10, 0)),
start = LocalDateTime(LocalDate(2026, 6, 11), LocalTime(0, 0)),
end = LocalDateTime(LocalDate(2026, 6, 11), LocalTime(0, 0)),
).copy(rrule = "FREQ=YEARLY").toDetachedOccurrence()
val inserted = buildEventInsertValues(
form = detached,
uid = "uid@calendula",
times = detached.toWriteTimes(berlin),
)
// The excluded day gains the new 09:00 Berlin time-of-day == 07:00Z.
assertThat(update(original, timed, series, "20260722")[CalendarContract.Events.EXDATE])
.isEqualTo("20260722T070000Z")
assertThat(parent[CalendarContract.Events.EXDATE]).isEqualTo("20260611")
assertThat(inserted[CalendarContract.Events.DTSTART]).isEqualTo(1_781_136_000_000L)
}
@Test
fun `a text-only edit leaves the exclusions alone`() {
val original = julySeries()
val values = update(original, original.copy(title = "Renamed"), exdate = "20260722T070000Z")
assertThat(values).doesNotContainKey(CalendarContract.Events.EXDATE)
}
// --- exdateContains (guards a second detach of the same occurrence) ---
@Test
fun `changing only the rule rewrites no exclusions`() {
// The times are untouched, so every surviving occurrence keeps its instant
// and the stamps still name the right ones.
val original = julySeries()
val values = update(original, original.copy(rrule = "FREQ=DAILY"), exdate = "20260722T070000Z")
assertThat(values[CalendarContract.Events.RRULE]).isEqualTo("FREQ=DAILY")
assertThat(values).doesNotContainKey(CalendarContract.Events.EXDATE)
}
@Test
fun `an exdate form we do not write is left untouched`() {
// A sync adapter may store a TZID-parameterised or floating stamp. A stale
// stamp excludes nothing; a mangled one could exclude the wrong occurrence.
val original = julySeries()
val values = update(
original,
original.atHour(10),
instantAt("2026-07-01T09:00", "Europe/Berlin"),
"TZID=Europe/Berlin;20260722T090000",
)
assertThat(values).doesNotContainKey(CalendarContract.Events.EXDATE)
}
@Test
fun `removing the recurrence clears the exclusions with it`() {
// Dormant, not harmless: adding a recurrence back later would punch the old
// holes into the new one.
val original = julySeries()
val values = update(original, original.copy(rrule = null), exdate = "20260722T070000Z")
assertThat(values).containsEntry(CalendarContract.Events.EXDATE, null)
}
@Test
fun `undoing a series move lands the exclusions back where they started`() {
val series = instantAt("2026-01-07T09:00", "Europe/Berlin")
val original = julySeries()
val moved = original.copy(
start = LocalDateTime(LocalDate(2026, 7, 17), LocalTime(14, 30)),
end = LocalDateTime(LocalDate(2026, 7, 17), LocalTime(15, 30)),
)
val exdate = "20260722T070000Z"
val forward = update(original, moved, series, exdate)
val movedExdate = forward[CalendarContract.Events.EXDATE] as String
assertThat(movedExdate).isNotEqualTo(exdate)
val back = update(
moved,
original,
forward[CalendarContract.Events.DTSTART] as Long,
movedExdate,
)
assertThat(back[CalendarContract.Events.EXDATE]).isEqualTo(exdate)
}
@Test
fun `a series with no exclusions writes no exdate column`() {
val series = instantAt("2026-07-01T09:00", "Europe/Berlin")
val original = julySeries()
assertThat(update(original, original.atHour(10), series, exdate = null))
.doesNotContainKey(CalendarContract.Events.EXDATE)
}
// --- exdateAfter (the exclusions a "this and following" split inherits) ---
@Test
fun `a split carries only the exclusions past the split point`() {
// The occurrence at the split point is the one being edited, so it exists
// by definition — a stale exclusion for it would swallow the edit whole.
fun `an occurrence already excluded is recognised, timed and all-day`() {
// Detaching twice would fold the repeated EXDATE away and still report a
// changed row, leaving a second standalone copy of one occurrence.
assertThat(
exdateAfter(
existingExdate = "20260708T080000Z,20260715T080000Z,20260722T080000Z",
beginMillis = instantAt("2026-07-15T08:00", "UTC"),
exdateContains("20260611T080000Z", 1_781_164_800_000L, isAllDay = false),
).isTrue()
assertThat(
exdateContains("20260611", 1_781_136_000_000L, isAllDay = true),
).isTrue()
}
@Test
fun `an occurrence not in the exdate list is not mistaken for an excluded one`() {
assertThat(exdateContains(null, 1_781_164_800_000L, isAllDay = false)).isFalse()
assertThat(exdateContains("", 1_781_164_800_000L, isAllDay = false)).isFalse()
// A neighbouring occurrence must not match — the guard is per-instant.
assertThat(
exdateContains("20260610T080000Z", 1_781_164_800_000L, isAllDay = false),
).isFalse()
}
@Test
fun `an exclusion is found anywhere in a multi-entry exdate list`() {
// Whitespace after a comma is legal in the stored column and must not
// hide an exclusion — that would let a duplicate through.
assertThat(
exdateContains(
"20260604T080000Z, 20260611T080000Z,20260618T080000Z",
1_781_164_800_000L,
isAllDay = false,
),
).isEqualTo("20260722T080000Z")
}
@Test
fun `a split carries nothing when every exclusion is behind it`() {
assertThat(
exdateAfter("20260708T080000Z", instantAt("2026-07-15T08:00", "UTC"), isAllDay = false),
).isNull()
assertThat(exdateAfter(null, 0L, isAllDay = false)).isNull()
}
@Test
fun `all-day exclusions split on their UTC date`() {
assertThat(
exdateAfter("20260708,20260722", instantAt("2026-07-15T00:00", "UTC"), isAllDay = true),
).isEqualTo("20260722")
}
@Test
fun `an unreadable exclusion carries nothing across a split`() {
assertThat(
exdateAfter(
"20260722T080000Z,TZID=Europe/Berlin;20260729T100000",
instantAt("2026-07-15T08:00", "UTC"),
isAllDay = false,
),
).isNull()
}
@Test
fun `split exclusions move by the same shift as the new series start`() {
// What the split path composes: filter to the tail, then re-time it by the
// shift the user applied to the occurrence they split at.
val original = julySeries()
val carried = shiftedExdate(
existingExdate = exdateAfter(
"20260716T070000Z",
instantAt("2026-07-15T07:00", "UTC"),
isAllDay = false,
),
original = original,
updated = original.atHour(11),
zone = berlin,
)
// 16 July 09:00 Berlin, pushed two hours, is 11:00 Berlin == 09:00Z.
assertThat(carried).isEqualTo("20260716T090000Z")
}
@Test
fun `nothing to shift yields no exdate`() {
assertThat(shiftedExdate(null, julySeries(), julySeries().atHour(10), berlin)).isNull()
assertThat(shiftedExdate(" ", julySeries(), julySeries().atHour(10), berlin)).isNull()
).isTrue()
}
// --- per-event colour ---