diff --git a/CHANGELOG.md b/CHANGELOG.md index 5435a0a..7735c23 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed +- **"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 ### Added @@ -1471,3 +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 +[#234]: https://codeberg.org/jlmakiola/calendula/issues/234 diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/CalendarDataSource.kt b/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/CalendarDataSource.kt index 1b8507f..3792608 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/CalendarDataSource.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/CalendarDataSource.kt @@ -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, @@ -1194,6 +1199,10 @@ class AndroidCalendarDataSource @Inject constructor( form: EventForm, allDayReminderTimeMinutes: Int, ): Long { + val row = querySeriesRow(eventId) + 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, @@ -1212,6 +1221,88 @@ 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. It does mean the edited occurrence stops travelling with the + * series — moving the series later won't move it — which is the honest cost of + * a link the provider cannot store here. + * + * 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 { + // 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, + ) + val excluded = try { + resolver.update( + ContentUris.withAppendedId(CalendarContract.Events.CONTENT_URI, eventId), + values.toContentValues(), null, null, + ) + } catch (e: RuntimeException) { + rollBackDetached(detachedId) + throw e + } + if (excluded == 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, diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/EventWriteMapper.kt b/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/EventWriteMapper.kt index 8e64db5..483c08e 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/EventWriteMapper.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/EventWriteMapper.kt @@ -243,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` diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/edit/EventEditScreen.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/edit/EventEditScreen.kt index 4f36992..0956a03 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/edit/EventEditScreen.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/edit/EventEditScreen.kt @@ -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 diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/edit/EventEditViewModel.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/edit/EventEditViewModel.kt index 2c3dc61..f184150 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/edit/EventEditViewModel.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/edit/EventEditViewModel.kt @@ -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 @@ -752,6 +755,10 @@ class EventEditViewModel @Inject constructor( } catch (e: SecurityException) { SaveUiState.NeedsPermission } 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 } } diff --git a/app/src/test/java/de/jeanlucmakiola/calendula/data/calendar/EventWriteMapperTest.kt b/app/src/test/java/de/jeanlucmakiola/calendula/data/calendar/EventWriteMapperTest.kt index e8f4a75..5f8865f 100644 --- a/app/src/test/java/de/jeanlucmakiola/calendula/data/calendar/EventWriteMapperTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/calendula/data/calendar/EventWriteMapperTest.kt @@ -558,6 +558,130 @@ class EventWriteMapperTest { assertThat(values[CalendarContract.Events.ALL_DAY]).isEqualTo(1) } + // --- toDetachedOccurrence ("edit only this event", no _sync_id) --- + + @Test + 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() + + val values = buildEventInsertValues( + form = detached, + uid = "uid@calendula", + times = detached.toWriteTimes(berlin), + ) + 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 `a detached occurrence keeps everything but the rule`() { + val edited = form(timezone = "America/New_York").copy( + title = "Standup", + location = "Room 2", + description = "notes", + reminders = listOf(10), + availability = Availability.Free, + accessLevel = AccessLevel.Private, + colorKey = "7", + rrule = "FREQ=DAILY", + ) + val detached = edited.toDetachedOccurrence() + + assertThat(detached.rrule).isNull() + assertThat(detached).isEqualTo(edited.copy(rrule = null)) + // Reminders and guests ride along on the form: the insert path seeds them + // from it, so the detached row keeps the reminders the user just edited. + 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, 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(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 `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 `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, 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), + ) + assertThat(parent[CalendarContract.Events.EXDATE]).isEqualTo("20260611") + assertThat(inserted[CalendarContract.Events.DTSTART]).isEqualTo(1_781_136_000_000L) + } + // --- per-event colour --- @Test