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) } }