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.
This commit is contained in:
@@ -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))
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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<OpenPicker?>(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
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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. */
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -122,8 +122,8 @@
|
||||
<string name="alarm_scheduled_for">Scheduled for %1$s</string>
|
||||
<!-- %1$s is a date range such as "Oct 26 – 28" -->
|
||||
<string name="alarm_paused_dates">Paused %1$s</string>
|
||||
<!-- %1$s is a date such as "Oct 28": the pause has already begun -->
|
||||
<string name="alarm_paused_until">Paused until %1$s</string>
|
||||
<!-- %1$s is the first date the alarm rings again, such as "Thu, Oct 29": the pause has already begun -->
|
||||
<string name="alarm_paused_back_on">Paused, back on %1$s</string>
|
||||
<string name="alarm_summary_separator" translatable="false">" \u00b7 "</string>
|
||||
|
||||
<!-- M5: the alarm editor -->
|
||||
|
||||
@@ -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"))
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user