From 4cd9e397cd968ca4e194af7d06a2b5a24ccb5133 Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Mon, 2 Nov 2026 20:58:47 +0100 Subject: [PATCH] Moving a past start or due later brings its default reminders (#121) (#133) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A one-off task saved with a start of "now" (the "Default start to today" setting, or the picker's default time) leaves out the "at start" default, since it would ring at once. Moving that start later afterwards never brought it back: the task already had a start, so the edit didn't count as gaining one. `StoredReminders.withDefaults` now also brings a date's defaults when that date moves and all of its defaults had already rung at the old date (checked with `ReminderPlanner.triggerAt`, so all-day dates go by their calendar day). Only when: - the task is one-off and has reminders of its own, none from that date — a task with no reminders keeps following the planner's defaults, and one with reminders turned off stays off; - the date really moved — flipping all-day on the same day doesn't count. Applies to start and due alike. The editor passes the loaded all-day flag so a kind switch is told apart from a move. Deviation from the issue: creating a task with a future start already took the start default (checked on the emulator); what the reporter hit is editing a task saved with a start of "now". Known trade-off: a reminder from a date removed by hand comes back if that date is moved after its default would have rung — stored data can't tell the two apart. Closes #121 Co-authored-by: Jean-Luc Makiola Reviewed-on: https://codeberg.org/jlmakiola/agendula/pulls/133 --- CHANGELOG.md | 3 + .../agendula/data/tasks/StoredReminders.kt | 48 +++++++++++-- .../agendula/ui/edit/TaskEditViewModel.kt | 3 + .../data/tasks/StoredRemindersTest.kt | 71 +++++++++++++++++++ 4 files changed, 121 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3e76d5c..e6dd604 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,9 @@ All notable changes to this project are documented here. The format follows upcoming reminder and clears one already showing (#127, #129). - The splash screen follows the app's theme, including Black and dynamic colour, instead of flashing white in dark mode (#119). +- A task saved without its default start or due reminder because it had + already rung (a start of "now", say) gets it once that date is moved later, + if the task has no reminder from that date (#121). ## [1.2.0] - 2026-10-26 diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/StoredReminders.kt b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/StoredReminders.kt index f67ffcc..f16cffc 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/StoredReminders.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/StoredReminders.kt @@ -5,6 +5,7 @@ import de.jeanlucmakiola.agendula.data.reminders.ReminderPlanner import de.jeanlucmakiola.agendula.domain.Task import de.jeanlucmakiola.agendula.domain.TaskForm import de.jeanlucmakiola.agendula.domain.TaskReminder +import de.jeanlucmakiola.agendula.domain.calendarDate import java.time.ZoneId import kotlin.time.Instant @@ -22,7 +23,8 @@ internal object StoredReminders { * * A task getting its first date takes the defaults only when the form * names no reminder; one that already had a date takes those of the new - * date that no reminder of the form counts from. Otherwise the form is + * date that no reminder of the form counts from, and those of a date + * whose defaults had all rung when it moved — see [revived]. Otherwise the form is * kept, so an edit that clears every reminder means "use the default * here" as before. * @@ -33,7 +35,7 @@ internal object StoredReminders { fun of(form: TaskForm, current: Task?, settings: Settings, now: Long, zone: ZoneId): List { val wanted = form.reminders.map { it.lead }.distinct() if (form.remindersChosen) return wanted - return withDefaults(form, current?.start, current?.due, settings, now, zone) + return withDefaults(form, current?.start, current?.due, current?.isAllDay ?: form.isAllDay, settings, now, zone) } /** [of] for a task whose dates were [start] and [due], whatever the form chose. */ @@ -41,6 +43,7 @@ internal object StoredReminders { form: TaskForm, start: Instant?, due: Instant?, + wasAllDay: Boolean, settings: Settings, now: Long, zone: ZoneId, @@ -48,10 +51,12 @@ internal object StoredReminders { val wanted = form.reminders.map { it.lead }.distinct() val had = anchorsOf(start, due) val gained = anchorsOf(form.start, form.due) - had - if (gained.isEmpty()) return wanted + val revived = revived(form, start, due, wasAllDay, wanted, settings, now, zone) + if (gained.isEmpty() && revived.isEmpty()) return wanted if (had.isEmpty() || wanted.isEmpty()) return wanted.ifEmpty { defaultsFor(form, settings, now, zone) } val named = wanted.map { it.fromStart }.toSet() - return wanted + defaultsFor(form, settings, now, zone).filter { it.fromStart in gained && it.fromStart !in named } + val fresh = gained + revived + return wanted + defaultsFor(form, settings, now, zone).filter { it.fromStart in fresh && it.fromStart !in named } } /** @@ -79,6 +84,41 @@ internal object StoredReminders { return kept.distinct().ifEmpty { defaultsFor(form, settings, now, zone) } } + /** + * The dates of a one-off task that moved after every default of theirs + * had rung: a task saved then was left without them as past (#121), so + * the new date brings them like a date the task gains. Only on a task + * with reminders of its own and none from that date — no reminders at + * all is the planner's default or turned off, and either holds. + */ + private fun revived( + form: TaskForm, + start: Instant?, + due: Instant?, + wasAllDay: Boolean, + wanted: List, + settings: Settings, + now: Long, + zone: ZoneId, + ): Set { + if (form.rrule != null || wanted.isEmpty()) return emptySet() + val named = wanted.map { it.fromStart }.toSet() + val defaults = settings.defaultRemindersFor(form.listId, wasAllDay) + fun revived(fromStart: Boolean, was: Instant?, isNow: Instant?): Boolean { + if (was == null || isNow == null || fromStart in named) return false + val moved = if (wasAllDay == form.isAllDay) { + isNow != was + } else { + isNow.calendarDate(form.isAllDay, zone) != was.calendarDate(wasAllDay, zone) + } + val leads = defaults.filter { it.fromStart == fromStart } + return moved && leads.isNotEmpty() && leads.all { + ReminderPlanner.triggerAt(was, wasAllDay, it.minutesBefore, settings.allDayReminderMinuteOfDay, zone) < now + } + } + return setOfNotNull(true.takeIf { revived(true, start, form.start) }, false.takeIf { revived(false, due, form.due) }) + } + /** The dates a reminder can count from, as [TaskReminder.fromStart]. */ private fun anchorsOf(start: Any?, due: Any?): Set = setOfNotNull(true.takeIf { start != null }, false.takeIf { due != null }) diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/ui/edit/TaskEditViewModel.kt b/app/src/main/java/de/jeanlucmakiola/agendula/ui/edit/TaskEditViewModel.kt index b55b28e..1261482 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/ui/edit/TaskEditViewModel.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/ui/edit/TaskEditViewModel.kt @@ -147,6 +147,7 @@ class TaskEditViewModel @Inject constructor( private var loadedReminders: List = emptyList() private var loadedStart: Instant? = null private var loadedDue: Instant? = null + private var loadedAllDay = false /** Start a fresh task, optionally pre-selecting a list / parent. */ fun bindNew( @@ -228,6 +229,7 @@ class TaskEditViewModel @Inject constructor( storedReminders = loadedReminders.toSet() loadedStart = task.start loadedDue = task.due + loadedAllDay = task.isAllDay _state.value = withFields( TaskEditUiState( loading = false, @@ -362,6 +364,7 @@ class TaskEditViewModel @Inject constructor( toForm().copy(reminders = loadedReminders), loadedStart, loadedDue, + loadedAllDay, settings, nowMillis(), ZoneId.systemDefault(), diff --git a/app/src/test/java/de/jeanlucmakiola/agendula/data/tasks/StoredRemindersTest.kt b/app/src/test/java/de/jeanlucmakiola/agendula/data/tasks/StoredRemindersTest.kt index ed6392c..78ea5ce 100644 --- a/app/src/test/java/de/jeanlucmakiola/agendula/data/tasks/StoredRemindersTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/agendula/data/tasks/StoredRemindersTest.kt @@ -61,6 +61,77 @@ class StoredRemindersTest { assertThat(stored(form.copy(reminders = listOf(hourBeforeDue)), due)).containsExactly(hourBeforeDue, dayBeforeStart) } + private fun moved( + form: TaskForm, + start: Instant?, + due: Instant?, + wasAllDay: Boolean = form.isAllDay, + at: ZoneId = zone, + ) = StoredReminders.withDefaults(form, start, due, wasAllDay, settings, now.toEpochMilliseconds(), at) + + @Test + fun `a date moved after its defaults rang brings them`() { + val form = TaskForm(title = "t", listId = 1, start = now + 2.days, due = now + 5.days, reminders = listOf(hourBeforeDue)) + assertThat(moved(form, start = now, due = now + 5.days)).containsExactly(hourBeforeDue, dayBeforeStart) + // Its date still ahead, but the lead already past. + assertThat(moved(form, start = now + 2.hours, due = now + 5.days)).containsExactly(hourBeforeDue, dayBeforeStart) + } + + @Test + fun `a moved due brings its defaults the same way`() { + val ownStart = TaskReminder(0, fromStart = true) + val form = TaskForm(title = "t", listId = 1, start = now - 3.days, due = now + 5.days, reminders = listOf(ownStart)) + assertThat(moved(form, start = now - 3.days, due = now)).containsExactly(ownStart, dayBeforeDue, hourBeforeDue) + } + + @Test + fun `a moved date keeps the form when its defaults had not rung or are named`() { + val form = TaskForm(title = "t", listId = 1, start = now + 3.days, due = now + 5.days, reminders = listOf(hourBeforeDue)) + assertThat(moved(form, start = now + 2.days, due = now + 5.days)).containsExactly(hourBeforeDue) + val ownStart = TaskReminder(30, fromStart = true) + assertThat(moved(form.copy(reminders = listOf(ownStart)), start = now, due = now + 5.days)).containsExactly(ownStart) + } + + @Test + fun `a task without reminders of its own keeps none when a past date moves`() { + val form = TaskForm(title = "t", listId = 1, start = now + 2.days, due = now + 5.days) + assertThat(moved(form, start = now, due = now + 5.days)).isEmpty() + assertThat(moved(form.copy(rrule = "FREQ=DAILY", reminders = listOf(hourBeforeDue)), start = now, due = now + 5.days)) + .containsExactly(hourBeforeDue) + } + + @Test + fun `turning all-day without moving the date is not a move`() { + val form = TaskForm( + title = "t", + listId = 1, + start = allDayInstantOf(LocalDate.of(2026, 10, 1)), + due = allDayInstantOf(LocalDate.of(2026, 10, 6)), + isAllDay = true, + reminders = listOf(TaskReminder(1_440)), + ) + assertThat(moved(form, start = now, due = now + 5.days, wasAllDay = false)).containsExactly(TaskReminder(1_440)) + } + + @Test + fun `an all-day date is past by its own calendar, not by UTC midnight`() { + val ownStart = TaskReminder(0, fromStart = true) + val newYork = ZoneId.of("America/New_York") + // 20:00 on Sep 30 in New York: Oct 1 has not begun there. + val form = TaskForm( + title = "t", + listId = 1, + start = allDayInstantOf(LocalDate.of(2026, 9, 28)), + due = allDayInstantOf(LocalDate.of(2026, 10, 2)), + isAllDay = true, + reminders = listOf(ownStart), + ) + val due = allDayInstantOf(LocalDate.of(2026, 10, 1)) + assertThat( + StoredReminders.withDefaults(form, form.start, due, true, settings, Instant.parse("2026-10-01T00:00:00Z").toEpochMilliseconds(), newYork), + ).containsExactly(ownStart) + } + @Test fun `a gained date the form already counts from keeps the form`() { val current = testTask(start = now + 2.days)