diff --git a/CHANGELOG.md b/CHANGELOG.md index a67ac18..37658c0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 flight into another timezone. Paging the month widget forward and back also stops quietly pinning it to that month, so it follows the date again instead of being stranded on the month you happened to be looking at ([#228]). +- **"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 @@ -1482,3 +1492,4 @@ automatically, with zero telemetry and no internet permission. [#196]: https://codeberg.org/jlmakiola/calendula/issues/196 [#214]: https://codeberg.org/jlmakiola/calendula/issues/214 [#228]: https://codeberg.org/jlmakiola/calendula/issues/228 +[#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..503bc6e 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,13 @@ class AndroidCalendarDataSource @Inject constructor( form: EventForm, allDayReminderTimeMinutes: Int, ): Long { + val row = querySeriesRow(eventId) + // Stricter than deleteOccurrence's bare _sync_id check: EXDATE only means + // something on a row that recurs, so a non-recurring one keeps the + // exception path rather than getting a recurrence set written onto it. + 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 +1224,89 @@ 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 and the edit is lost (Codeberg #234). + * EXDATE plus a standalone row needs no link — what a detached instance + * degrades to without a `RECURRENCE-ID` to carry it. + * + * The detached row keeps no stored link back to its series, so: it no longer + * travels with it ([moveEvent] copies the master and its `ORIGINAL_ID` + * children, and this is neither); its EXDATE hole is an absolute instant, so + * re-timing the whole series brings the occurrence back beside the copy (a + * #47 delete resurrects the same way); and it is built from the form, not + * cloned, so `ORGANIZER`, `STATUS` and the attendee rows [reconcileAttendees] + * preserves are dropped — the same limitation as [moveEvent]. + * + * Insert first, so a failure leaves the series untouched + * ([updateEventFromOccurrence]'s discipline); roll the new row back if the + * EXDATE update then fails, since it would be a visible duplicate. The + * reverse order risks the worse outcome — an excluded occurrence 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) from a stale screen still pointing at the + // parent: the EXDATE merge would fold the repeat away and still report a + // changed row, quietly leaving a *second* standalone copy. + if (exdateContains(row.exdate, beginMillis, isAllDay = row.allDay != 0)) { + throw NoSuchEventException(eventId) + } + // Reminders, guests and colour come along like any new event, and so does + // a fresh UID — the detached row is a separate event now, and 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 — 1 whenever the series row still + // exists. It catches the row disappearing under us, not an EXDATE the + // provider's expansion fails to match. + 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 the worst + * case is the duplicate we were avoiding — 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..8380fba 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,21 @@ 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. + * + * The "edit only this event" shape for a series with **no `_sync_id`**, where an + * exception row can't attach to its parent at all (Codeberg #234). + * + * The exception path gets the rule dropped for free — the provider clears the + * RRULE it cloned when an exception carries DTSTART + DURATION + * ([buildOccurrenceExceptionValues]). Here nothing is cloned, so it is stripped + * by hand; leaving it on would insert a second *series* overlapping the first. + */ +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` @@ -435,6 +450,23 @@ internal fun buildOccurrenceExdateValues( ) } +/** + * Whether [existingExdate] already excludes the occurrence at [occurrenceMillis] + * — i.e. it has already been dropped from the series, deleted or detached. + * + * Guards the detach path against running twice from a stale screen: the EXDATE + * merge folds the repeat away silently and the update still reports one row + * changed, so a second save would leave a second standalone copy. + */ +internal fun exdateContains( + existingExdate: String?, + occurrenceMillis: Long, + isAllDay: Boolean, +): Boolean { + val stamp = formatExdateStamp(occurrenceMillis, isAllDay) + return existingExdate?.split(',')?.any { it.trim() == stamp } == true +} + /** * 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 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..034935c 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,16 @@ fun EventEditScreen( viewModel.reset() onSaved() } + // A failed save leaves the form looking unchanged, so the snackbar is + // the only sign anything happened — long rather than the default + // flash (Codeberg #234: it 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..d00d80b 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 @@ -751,7 +754,15 @@ class EventEditViewModel @Inject constructor( throw e } catch (e: SecurityException) { SaveUiState.NeedsPermission + } catch (e: NoSuchEventException) { + // The event or occurrence is already gone: the same answer the + // pre-check gives, and better than a bare "couldn't save". + SaveUiState.Gone } catch (e: Exception) { + // The user only gets a generic snackbar, so without this a failed + // write leaves nothing 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..1722a51 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,174 @@ 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") + // A surviving rule 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 rather than a duration. + 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 carries every edited field onto the new row`() { + // Built from the form, not cloned from the parent: 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 — never 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 aren't columns; the insert path seeds them from the form. + 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 must agree on the instant, or the user sees the + // occurrence twice or not at all. + 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) + } + + // --- exdateContains (guards a second detach of the same occurrence) --- + + @Test + fun `an occurrence already excluded is recognised, timed and all-day`() { + // Detaching twice would leave a second standalone copy. + assertThat( + 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. + assertThat( + exdateContains( + "20260604T080000Z, 20260611T080000Z,20260618T080000Z", + 1_781_164_800_000L, + isAllDay = false, + ), + ).isTrue() + } + // --- per-event colour --- @Test