From eb43f5d259f693efb6649bfde09631d7fac6472b Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Wed, 9 Sep 2026 12:21:29 +0200 Subject: [PATCH] sync: a reminder set on one occurrence stays on that occurrence MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Forking an occurrence copies the master's properties onto the new override row, so the reminder was deliberately written to the master first to carry it across — and then left there. Changing one occurrence's reminder therefore changed the whole series', silently, and the next occurrence inherited it too. It is put back after the fork now. --- .../tasks/room/RoomTasksDataSourceTest.kt | 17 ++++++++++++++ .../data/tasks/TasksRepositoryImpl.kt | 22 +++++++++++++------ 2 files changed, 32 insertions(+), 7 deletions(-) diff --git a/app/src/androidTest/java/de/jeanlucmakiola/agendula/data/tasks/room/RoomTasksDataSourceTest.kt b/app/src/androidTest/java/de/jeanlucmakiola/agendula/data/tasks/room/RoomTasksDataSourceTest.kt index 4c9970a..eb7b748 100644 --- a/app/src/androidTest/java/de/jeanlucmakiola/agendula/data/tasks/room/RoomTasksDataSourceTest.kt +++ b/app/src/androidTest/java/de/jeanlucmakiola/agendula/data/tasks/room/RoomTasksDataSourceTest.kt @@ -369,6 +369,23 @@ class RoomTasksDataSourceTest { assertThat(db.tasks().allOverrides(listId)).isEmpty() } + @Test + fun aForkedOccurrenceCarriesTheSeriesReminder() { + val id = source.insertTask(form()) + makeRecurring(id, now) + source.setAlarm(id, minutesBeforeDue = 30) + val target = source.tasks(TaskQuery(listId = listId)) + .filter { it.taskId == id } + .first { it.distanceFromCurrent == 1 } + + source.updateInstance(id, target.occurrenceStart!!, form(title = "moved")) + + // The fork copies the master's properties, which is what carries the + // reminder across — the repository is what puts the series' own back. + val override = db.tasks().override(id, target.occurrenceStart)!! + assertThat(db.alarms().forTask(override.id).single().minutesBefore).isEqualTo(30) + } + @Test fun anOccurrenceCannotBeMovedOutOfItsSeriesList() { val other = source.createLocalList("Work", 0) diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/TasksRepositoryImpl.kt b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/TasksRepositoryImpl.kt index ab2ef75..7075d84 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/TasksRepositoryImpl.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/TasksRepositoryImpl.kt @@ -102,18 +102,26 @@ class TasksRepositoryImpl @Inject constructor( val seen = current?.lastModified if (seen != null && seen != expectedLastModified) throw TaskConflictException(taskId) } - // Write the reminder first: forking a recurring occurrence copies the - // task's properties onto the new override row, so setting the alarm - // beforehand is what carries it across. - dataSource.setAlarm(taskId, form.reminderMinutesBeforeDue) // A recurring task's start/due are one occurrence's resolved times, so // writing them back to the task row would re-anchor the whole series. // Going through the occurrence forks an override instead. val occurrence = current?.takeIf { it.isRecurring }?.occurrenceStart - if (occurrence != null) { - dataSource.updateInstance(taskId, occurrence, form) - } else { + if (occurrence == null) { + dataSource.setAlarm(taskId, form.reminderMinutesBeforeDue) dataSource.updateTask(taskId, form) + return@withContext + } + // ⚠️ Written to the series, and then put back. Forking an occurrence + // copies the task's *current* properties onto the new override row, + // so setting the alarm beforehand is the only way to carry it + // across — and leaving it there would silently give every other + // occurrence in the series the reminder the user set on one of them. + val seriesReminder = dataSource.alarms()[taskId]?.minutesBefore + dataSource.setAlarm(taskId, form.reminderMinutesBeforeDue) + try { + dataSource.updateInstance(taskId, occurrence, form) + } finally { + dataSource.setAlarm(taskId, seriesReminder) } }