From 1dcd743e5b79ee0faed60b9eeb0aa536b516e324 Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Thu, 22 Oct 2026 07:43:30 +0200 Subject: [PATCH] fix(alarms): keep a skip on the next ring after any edit, and say when a pause ends Moving the time or changing the days left a pending skip pointing at the old sequence, so the skipped day could ring at its new time. Every editor change that can move a fire time now re-arms the skip. A pause under way now reads "Paused, back on Thu, Oct 29" instead of naming its last day, which read as the day it ends before. Reopening a begun pause keeps its start, the date pickers take their bounds from the clock when they open, and the dismiss chooser shows the scheduled date and pause like the list does. --- .../clockula/alarm/AlarmEngine.kt | 12 +++---- .../clockula/domain/alarm/AlarmSchedule.kt | 3 +- .../clockula/ui/alarms/AlarmDatePickers.kt | 15 ++++++--- .../clockula/ui/alarms/AlarmEditorScreen.kt | 26 +++++++++------ .../clockula/ui/alarms/AlarmEditorUiState.kt | 3 ++ .../ui/alarms/AlarmEditorViewModel.kt | 29 +++++++++++----- .../clockula/ui/alarms/AlarmRow.kt | 33 +++++++++++-------- .../clockula/ui/alarms/DismissAlarmDialog.kt | 2 +- app/src/main/res/values/strings.xml | 4 +-- .../ui/alarms/AlarmEditorScheduleTest.kt | 16 +++++++++ docs/ARCHITECTURE.md | 11 ++++--- 11 files changed, 101 insertions(+), 53 deletions(-) diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/alarm/AlarmEngine.kt b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/AlarmEngine.kt index 34e5be8..a0fa5a8 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/alarm/AlarmEngine.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/AlarmEngine.kt @@ -128,13 +128,13 @@ class AlarmEngine @Inject constructor( } /** - * The alarm's pause just changed: a skip armed on the old shape of its - * sequence is re-armed on the new one, so "skip next" still means the - * next ring — not one the old pause jumped to, nor one the new pause - * already silences. A watermark already due is left for the resolver to - * consume as usual. + * The alarm's time, days or pause were just edited: a skip armed on the + * old shape of its sequence is re-armed on the new one, so "skip next" + * still means the next ring — not one the old pause jumped to, an + * occurrence of the old time, nor one a new pause already silences. A + * watermark already due is left for the resolver to consume as usual. */ - suspend fun onPauseChanged(alarmId: Long): ScheduledFire? = lock.withLock { + suspend fun onScheduleEdited(alarmId: Long): ScheduledFire? = lock.withLock { val state = states.state(alarmId) val armed = state.skippedOccurrence if (armed != null && armed > wallClock.now()) states.save(state.copy(skippedOccurrence = null)) diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/domain/alarm/AlarmSchedule.kt b/app/src/main/java/de/jeanlucmakiola/clockula/domain/alarm/AlarmSchedule.kt index 495d1cb..d940490 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/domain/alarm/AlarmSchedule.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/domain/alarm/AlarmSchedule.kt @@ -53,7 +53,6 @@ object AlarmSchedule { */ fun earliestDate(time: TimeOfDay, now: Instant, zone: ZoneId): LocalDate { // Every day matches RepeatDays.NONE, so the sequence is never empty. - val next = AlarmOccurrences.next(time, RepeatDays.NONE, now, zone) ?: now - return today(next, zone) + return today(AlarmOccurrences.sequence(time, RepeatDays.NONE, now, zone).first(), zone) } } diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmDatePickers.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmDatePickers.kt index 5785fda..0210ce9 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmDatePickers.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmDatePickers.kt @@ -99,7 +99,11 @@ internal fun ScheduleDatePickerDialog( } } -/** Picks the days a repeating alarm stays quiet on. A start with no end is a one-day pause. */ +/** + * Picks the days a repeating alarm stays quiet on. A start with no end is a + * one-day pause. A pause already under way keeps its start, so reopening it + * to check or extend it does not rewrite the days already behind it. + */ @OptIn(ExperimentalMaterial3Api::class) @Composable internal fun PauseRangePickerDialog( @@ -109,14 +113,15 @@ internal fun PauseRangePickerDialog( onRemove: (() -> Unit)?, onDismiss: () -> Unit, ) { - val selectable = remember(today) { PickerDates.from(today) } - val start = initial?.start?.let { maxOf(it, today) } + val start = initial?.start val end = initial?.end?.takeUnless { it.isBefore(today) } + val earliest = start?.let { minOf(it, today) } ?: today + val selectable = remember(earliest) { PickerDates.from(earliest) } val state = rememberDateRangePickerState( initialSelectedStartDateMillis = start?.let(PickerDates::toMillis), initialSelectedEndDateMillis = end?.let(PickerDates::toMillis), - initialDisplayedMonthMillis = PickerDates.toMillis(start ?: today), - yearRange = PickerDates.yearRange(today), + initialDisplayedMonthMillis = PickerDates.toMillis(maxOf(start ?: today, today)), + yearRange = PickerDates.yearRange(earliest), selectableDates = selectable, ) val pickedStart = state.selectedStartDateMillis diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorScreen.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorScreen.kt index 5719181..207ad17 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorScreen.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorScreen.kt @@ -39,6 +39,7 @@ import de.jeanlucmakiola.floret.components.GroupedSurface import de.jeanlucmakiola.floret.components.InlineTextField import de.jeanlucmakiola.floret.components.Position import de.jeanlucmakiola.floret.components.positionOf +import java.time.LocalDate /** Which override picker is open. Null is "none"; the ringtone picker is the ViewModel's. */ private enum class OpenPicker { SCHEDULE_DATE, PAUSE, VIBRATE, VOLUME_RAMP, SNOOZE_LENGTH, SNOOZE_LIMIT, CHALLENGE } @@ -144,6 +145,17 @@ private fun AlarmEditorContent( // about losing a half-entered time). var openPicker by rememberSaveable { mutableStateOf(null) } var pickingTime by rememberSaveable { mutableStateOf(openTimePicker) } + // Epoch days, so they survive a rotation; read from the clock when a date + // picker opens, never from a render that may be minutes old. + var earliestDay by rememberSaveable { mutableStateOf(editing.earliestScheduleDate.toEpochDay()) } + var todayDay by rememberSaveable { mutableStateOf(editing.today.toEpochDay()) } + val openDatePicker = { picker: OpenPicker -> + val bounds = viewModel.datePickerBounds() + earliestDay = bounds.earliestScheduleDate.toEpochDay() + todayDay = bounds.today.toEpochDay() + viewModel.onOpenDatePicker() + openPicker = picker + } TimeCard(time = editing.time, onClick = { pickingTime = true }) Spacer(Modifier.height(16.dp)) @@ -165,10 +177,7 @@ private fun AlarmEditorContent( summary = editing.pause?.let { pauseRangeText(it) } ?: stringResource(R.string.editor_pause_none), position = positionOf(1, repeatRows), - onClick = { - viewModel.onOpenDatePicker() - openPicker = OpenPicker.PAUSE - }, + onClick = { openDatePicker(OpenPicker.PAUSE) }, ) GroupedRow( title = stringResource(R.string.editor_skip_next), @@ -188,10 +197,7 @@ private fun AlarmEditorContent( summary = editing.scheduledDate?.let { scheduledDateText(it) } ?: stringResource(R.string.editor_schedule_none), position = positionOf(1, repeatRows), - onClick = { - viewModel.onOpenDatePicker() - openPicker = OpenPicker.SCHEDULE_DATE - }, + onClick = { openDatePicker(OpenPicker.SCHEDULE_DATE) }, ) } @@ -258,7 +264,7 @@ private fun AlarmEditorContent( null -> Unit OpenPicker.SCHEDULE_DATE -> ScheduleDatePickerDialog( initial = editing.scheduledDate, - earliest = editing.earliestScheduleDate, + earliest = LocalDate.ofEpochDay(earliestDay), onConfirm = { viewModel.onScheduledDateChosen(it) openPicker = null @@ -273,7 +279,7 @@ private fun AlarmEditorContent( ) OpenPicker.PAUSE -> PauseRangePickerDialog( initial = editing.pause, - today = editing.today, + today = LocalDate.ofEpochDay(todayDay), onConfirm = { viewModel.onPauseChosen(it) openPicker = null diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorUiState.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorUiState.kt index 715a69e..f7b47be 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorUiState.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorUiState.kt @@ -62,3 +62,6 @@ sealed interface AlarmEditorUiState { val defaults: ClockDefaults, ) : AlarmEditorUiState } + +/** What a date picker may offer, as of the moment it opens. */ +data class DatePickerBounds(val earliestScheduleDate: LocalDate, val today: LocalDate) diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorViewModel.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorViewModel.kt index 6b5f732..87d45f8 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorViewModel.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorViewModel.kt @@ -163,18 +163,27 @@ class AlarmEditorViewModel @Inject internal constructor( /** null ends the pause. Only a repeating alarm can pause; a pause wholly in the past is refused. */ fun onPauseChosen(pause: PauseRange?) { val today = AlarmSchedule.today(wallClock.now(), zones.current()) - viewModelScope.launch { - alarms.edit(alarmId) { alarm -> - when { - !alarm.repeatDays.isRepeating -> alarm - pause != null && pause.end.isBefore(today) -> alarm - else -> alarm.copy(pause = pause) - } + edit(reschedule = true) { alarm -> + when { + !alarm.repeatDays.isRepeating -> alarm + pause != null && pause.end.isBefore(today) -> alarm + else -> alarm.copy(pause = pause) } - engine.onPauseChanged(alarmId) } } + /** The two pickers' lower bounds, read from the clock now rather than from the last render. */ + fun datePickerBounds(): DatePickerBounds { + val now = wallClock.now() + val zone = zones.current() + val time = (_state.value as? AlarmEditorUiState.Editing)?.time + val today = AlarmSchedule.today(now, zone) + return DatePickerBounds( + earliestScheduleDate = time?.let { AlarmSchedule.earliestDate(it, now, zone) } ?: today, + today = today, + ) + } + /** * Re-reads the clock before a date picker opens. The state is otherwise * rendered only when the alarm changes, so an editor left open past the @@ -311,7 +320,9 @@ class AlarmEditorViewModel @Inject internal constructor( private fun edit(reschedule: Boolean = false, transform: (Alarm) -> Alarm) { viewModelScope.launch { alarms.edit(alarmId, transform) - if (reschedule) engine.reschedule() + // A change that can move a fire time also re-arms a pending skip + // on the sequence as it now is (#21). + if (reschedule) engine.onScheduleEdited(alarmId) } } diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmRow.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmRow.kt index 6c58f8a..0f254be 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmRow.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmRow.kt @@ -121,20 +121,12 @@ internal fun AlarmRow( @Composable private fun rowSummary(row: AlarmRowState): AnnotatedString { val primary = MaterialTheme.colorScheme.primary - // A scheduled date says more than "Once", so it takes that word's place. - val repeat = row.scheduledDate - ?.let { stringResource(R.string.alarm_scheduled_for, scheduledDateText(it)) } - ?: repeatSummaryText(row.repeatDays) - val paused = row.pause?.let { pausedText(it, row.pauseBegun) } + val schedule = scheduleSummaryText(row) val next = nextFireClause(row) val skipping = stringResource(R.string.alarm_skipping_next) val separator = stringResource(R.string.alarm_summary_separator) return buildAnnotatedString { - append(repeat) - if (paused != null) { - append(separator) - append(paused) - } + append(schedule) if (next != null) { append(separator) if (row.enabled && row.isNext) { @@ -151,15 +143,30 @@ private fun rowSummary(row: AlarmRowState): AnnotatedString { } /** - * "Paused Oct 26 – 28" while the pause is still ahead, "Paused until Oct 28" - * once it has begun: by then its start is history and only the end matters. + * When the alarm rings, in words: the repeat days — or the date that takes + * "Once"'s place — and the pause. Shared by the list and the dismiss chooser, + * so two one-shots at the same time can still be told apart. + */ +@Composable +internal fun scheduleSummaryText(row: AlarmRowState): String { + val repeat = row.scheduledDate + ?.let { stringResource(R.string.alarm_scheduled_for, scheduledDateText(it)) } + ?: repeatSummaryText(row.repeatDays) + val paused = row.pause?.let { pausedText(it, row.pauseBegun) } ?: return repeat + return repeat + stringResource(R.string.alarm_summary_separator) + paused +} + +/** + * "Paused Oct 26 – 28" while the pause is still ahead, "Paused, back on Thu, + * Oct 29" once it has begun: by then its start is history, and naming the + * pause's last day would read as the day it ends *before*. */ @Composable private fun pausedText(pause: PauseRange, begun: Boolean): String = if (!begun) { stringResource(R.string.alarm_paused_dates, pauseRangeText(pause)) } else { - stringResource(R.string.alarm_paused_until, scheduledDateText(pause.end)) + stringResource(R.string.alarm_paused_back_on, scheduledDateText(pause.end.plusDays(1))) } /** Ringing and snoozed say so; anything else is the countdown, or nothing. */ diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/DismissAlarmDialog.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/DismissAlarmDialog.kt index 4c08163..e659d5d 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/DismissAlarmDialog.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/DismissAlarmDialog.kt @@ -68,7 +68,7 @@ private fun DismissAlarmRow( time: String, onClick: () -> Unit, ) { - val repeat = repeatSummaryText(row.repeatDays) + val repeat = scheduleSummaryText(row) val spoken = if (row.label.isBlank()) time else "$time, ${row.label}" val description = stringResource(R.string.interop_dismiss_row_description, spoken) OptionCard( diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index a30e877..9aef65a 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -122,8 +122,8 @@ Scheduled for %1$s Paused %1$s - - Paused until %1$s + + Paused, back on %1$s " \u00b7 " diff --git a/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorScheduleTest.kt b/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorScheduleTest.kt index 4d49e60..ba02a3d 100644 --- a/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorScheduleTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorScheduleTest.kt @@ -263,4 +263,20 @@ class AlarmEditorScheduleTest { assertThat(harness.scheduler.next?.fireAt).isEqualTo(offsetInstant("2026-06-14T07:00+02:00")) } + + @Test + fun `moving the time re-arms a pending skip, so the skipped day stays quiet at the new time`( + @TempDir tempDir: Path, + ) = runTest(main.dispatcher) { + val harness = alarmEngineHarness(tempDir, now = fiveToday) + harness.given(alarmAt(hour = 7, repeatDays = RepeatDays.EVERY_DAY, skipNextOccurrence = true)) + harness.engine.reschedule() + val viewModel = editor(harness) + advanceUntilIdle() + + viewModel.onTimePicked(8, 0) + advanceUntilIdle() + + assertThat(harness.scheduler.next?.fireAt).isEqualTo(offsetInstant("2026-06-11T08:00+02:00")) + } } diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index a4b2948..fe334f7 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -1329,11 +1329,12 @@ occurrence ring late through the grace window. The resolver reports and the engine writes them as column-targeted `UPDATE`s conditional on the stored value still being the one it resolved, so a pause the editor has just picked is never cleared by a resolution of the old one, and no other column is -rewritten. Changing a pause calls `AlarmEngine.onPauseChanged`, which drops a -skip watermark still in the future so the resolver re-arms it on the new -sequence: removing a pause must not leave the skip on the first ring after the -old one, and adding a pause over the skipped day must not spend the skip on a -day already silent. +rewritten. Every editor write that can move a fire time (time, days, date, +pause) goes through `AlarmEngine.onScheduleEdited`, which drops a skip +watermark still in the future so the resolver re-arms it on the new sequence: +removing a pause must not leave the skip on the first ring after the old one, +adding a pause over the skipped day must not spend the skip on a day already +silent, and moving the time must not let the skipped day ring at the new time. The surfaces never show a stale one in the meantime: `AlarmSchedule` shows a date only while its occurrence is still ahead and a pause only while its last