From fa5ce55503e7b9e79b377334fd670166278d255e Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Wed, 21 Oct 2026 19:10:42 +0200 Subject: [PATCH] fix(alarms): settle and show the alarm-set notice where it can't be lost The repeat-day settle ran on the editor's scope, so leaving mid-settle needed a flush that a cleared scope could still race. The announcer now owns the settle on the application scope. Screens show the notice only while resumed, so a cross-fade never shows it twice and a backgrounded app does not use it up unseen. The editor shows only notices about its own alarm, since the sentence names none. Deleting an alarm, or the AlarmClock chooser dismissing it, takes its notice back. Freshness is measured on the monotonic clock, one instant is used for both ends of the countdown, and an identical countdown keeps the standing notice instead of flickering it. Refs #19 --- .../clockula/alarm/AlarmEngine.kt | 11 +- .../domain/alarm/AlarmSetCountdown.kt | 5 +- .../ui/alarms/AlarmEditorViewModel.kt | 52 +++----- .../clockula/ui/alarms/AlarmSetSnackbar.kt | 125 +++++++++++++----- .../clockula/ui/alarms/AlarmsViewModel.kt | 9 +- .../ui/alarms/AlarmEditorViewModelTest.kt | 72 +++++++++- .../clockula/ui/alarms/AlarmsViewModelTest.kt | 21 ++- docs/ARCHITECTURE.md | 9 +- 8 files changed, 220 insertions(+), 84 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 320512d..070b660 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/alarm/AlarmEngine.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/AlarmEngine.kt @@ -196,13 +196,14 @@ class AlarmEngine @Inject constructor( } /** - * One alarm's [upcoming] entry, resolved against the wall clock now. Read-only - * like [upcoming], and for the same reason it takes no lock: it writes - * nothing. Null for an id no alarm has. + * One alarm's [upcoming] entry, resolved against [now]. Read-only like + * [upcoming], and for the same reason it takes no lock: it writes nothing. + * Null for an id no alarm has. A caller that measures a countdown passes + * the `now` it measures from, so both ends of it are the same instant. */ - suspend fun upcomingFor(alarmId: Long): UpcomingAlarm? { + suspend fun upcomingFor(alarmId: Long, now: Instant = wallClock.now()): UpcomingAlarm? { val alarm = alarms.find(alarmId) ?: return null - return preview(alarm, states.state(alarmId), wallClock.now(), zones.current()) + return preview(alarm, states.state(alarmId), now, zones.current()) } /** diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/domain/alarm/AlarmSetCountdown.kt b/app/src/main/java/de/jeanlucmakiola/clockula/domain/alarm/AlarmSetCountdown.kt index 3efd675..52e70af 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/domain/alarm/AlarmSetCountdown.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/domain/alarm/AlarmSetCountdown.kt @@ -15,8 +15,9 @@ import kotlin.time.Instant object AlarmSetCountdown { /** Pure. The rounding is [NextFireFormat]'s, so the snackbar and the row agree. */ fun of(upcoming: UpcomingAlarm?, now: Instant): NextFireLabel? { - if (upcoming == null || !upcoming.alarm.enabled) return null - if (upcoming.source != FireSource.OCCURRENCE) return null + // A switched-off alarm resolves to FireSource.NONE, so this one check + // also covers it. + if (upcoming?.source != FireSource.OCCURRENCE) return null val nextFire = upcoming.nextFire ?: return null return NextFireFormat.labelFor(nextFire, now) } 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 f8c2ba1..3f495eb 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 @@ -18,14 +18,13 @@ import de.jeanlucmakiola.clockula.ui.common.RingtonePicker import de.jeanlucmakiola.clockula.ui.common.RingtonePickerUiState import de.jeanlucmakiola.floret.di.ApplicationScope import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.Job -import kotlinx.coroutines.delay import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.flow.collectLatest import kotlinx.coroutines.flow.combine +import kotlinx.coroutines.flow.map import kotlinx.coroutines.launch import java.time.DayOfWeek import javax.inject.Inject @@ -80,18 +79,15 @@ class AlarmEditorViewModel @Inject internal constructor( /** What the previewer is sounding, so the next preview knows to stop it. */ private var previewing: String? = null - /** The announcement waiting out [ANNOUNCE_SETTLE], so the next change can replace it. */ - private var announcing: Job? = null - - /** True while [announcing] has not yet announced — what [onCleared] flushes. */ - private var announcementPending = false - /** * "Alarm set for … from now" after a change that moves when the alarm rings * (#19). Nothing for a switched-off alarm, and nothing for the label or the - * sound: those move no fire time. + * sound: those move no fire time. Only ever about *this* alarm — the + * countdown names no alarm, so one about the row the user just switched + * on in the list would read as this one's. */ - val alarmSetNotice: Flow = announcer.notice + val alarmSetNotice: Flow = + announcer.notice.map { notice -> notice?.takeIf { it.alarmId == alarmId } } fun onAlarmSetNoticeShown(notice: AlarmSetNotice) = announcer.consumed(notice) @@ -120,17 +116,17 @@ class AlarmEditorViewModel @Inject internal constructor( /** Clamped, never throws. */ fun onTimePicked(hour: Int, minute: Int) = - edit(reschedule = true, announce = true) { it.copy(time = TimeOfDay.clamped(hour, minute)) } + edit(reschedule = true, announces = true) { it.copy(time = TimeOfDay.clamped(hour, minute)) } /** * The picker a new alarm opens on was closed without a time. The alarm * exists and is enabled at its default hour all the same — this is the * moment it was set, so it is announced (#19). */ - fun onNewAlarmTimeKept() = announce() + fun onNewAlarmTimeKept() = announcer.announce(alarmId) /** Announced once the taps settle: setting Mon–Fri is one snackbar, not five (#19). */ - fun onDayToggled(day: DayOfWeek) = edit(reschedule = true, announce = true, settle = ANNOUNCE_SETTLE) { alarm -> + fun onDayToggled(day: DayOfWeek) = edit(reschedule = true, announces = true, settle = ANNOUNCE_SETTLE) { alarm -> val days = if (day in alarm.repeatDays) alarm.repeatDays - day else alarm.repeatDays + day alarm.copy( repeatDays = days, @@ -155,7 +151,7 @@ class AlarmEditorViewModel @Inject internal constructor( engine.reschedule() // Skipping moves the next ring a day or more out, which is exactly // the kind of jump worth saying out loud. - announce() + announcer.announce(alarmId) } } @@ -244,6 +240,8 @@ class AlarmEditorViewModel @Inject internal constructor( if (engine.ringSession()?.alarm?.id == alarmId) engine.dismiss(alarmId) alarms.delete(alarmId) engine.reschedule() + // "Set for 2 hours from now" must not outlive the alarm on the list. + announcer.retract(alarmId) _state.value = AlarmEditorUiState.Gone } } @@ -256,9 +254,6 @@ class AlarmEditorViewModel @Inject internal constructor( labelWrites.value?.let { text -> appScope.launch { alarms.edit(alarmId) { it.copy(label = text) } } } - // Back within a second of the last day tap: the announcement still - // settling is made now, for the list to show (#19). - if (announcementPending) appScope.launch { announcer.announce(alarmId) } super.onCleared() } @@ -270,33 +265,20 @@ class AlarmEditorViewModel @Inject internal constructor( /** * Every editor write. [reschedule] is on only for a change that can move a * fire time — so fiddling with sound settings costs no AlarmManager traffic - * (M5 D12). [announce] is on for the same changes, after [settle]. + * (M5 D12). [announces] is on for the same changes, after [settle] — on + * the announcer's application scope, so leaving the editor while the + * repeat days settle still has them said, for the list to show (#19). */ private fun edit( reschedule: Boolean = false, - announce: Boolean = false, + announces: Boolean = false, settle: Duration = Duration.ZERO, transform: (Alarm) -> Alarm, ) { viewModelScope.launch { alarms.edit(alarmId, transform) if (reschedule) engine.reschedule() - if (announce) announce(settle) - } - } - - /** - * Replaces any announcement still settling, so a burst of changes ends in - * one snackbar about where they landed. Read from storage when it fires, - * not from the change that asked, so it is never about a superseded value. - */ - private fun announce(settle: Duration = Duration.ZERO) { - announcing?.cancel() - announcementPending = true - announcing = viewModelScope.launch { - delay(settle) - announcementPending = false - announcer.announce(alarmId) + if (announces) announcer.announce(alarmId, settle) } } diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmSetSnackbar.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmSetSnackbar.kt index cc26029..f9a8a55 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmSetSnackbar.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmSetSnackbar.kt @@ -7,27 +7,38 @@ import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.getValue import androidx.compose.runtime.rememberUpdatedState import androidx.compose.ui.platform.LocalResources +import androidx.lifecycle.Lifecycle +import androidx.lifecycle.compose.LocalLifecycleOwner +import androidx.lifecycle.repeatOnLifecycle import de.jeanlucmakiola.clockula.R import de.jeanlucmakiola.clockula.alarm.AlarmEngine import de.jeanlucmakiola.clockula.domain.alarm.AlarmSetCountdown import de.jeanlucmakiola.clockula.domain.format.NextFireLabel +import de.jeanlucmakiola.clockula.domain.time.ElapsedRealtimeClock import de.jeanlucmakiola.clockula.domain.time.WallClock +import de.jeanlucmakiola.floret.di.ApplicationScope +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Job +import kotlinx.coroutines.delay import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.collectLatest import kotlinx.coroutines.flow.map +import kotlinx.coroutines.flow.update +import kotlinx.coroutines.launch import kotlinx.coroutines.sync.Mutex import kotlinx.coroutines.sync.withLock import javax.inject.Inject import javax.inject.Singleton +import kotlin.time.Duration import kotlin.time.Duration.Companion.seconds -import kotlin.time.Instant /** "Alarm set for … from now", about one alarm, until a screen has shown it (#19). */ data class AlarmSetNotice( val alarmId: Long, val countdown: NextFireLabel, - val announcedAt: Instant, + /** On the monotonic clock: a wall clock set back must not make an old notice fresh. */ + val announcedAt: Duration, ) /** @@ -38,56 +49,96 @@ data class AlarmSetNotice( * `AlarmClock` intents never announces itself: those already get the * status-bar icon. * - * A **state**, held app-wide, not an event: the notice stays until a screen - * has shown it to the end ([consumed]). The usual way to set an alarm is + * App-wide, and a **state** rather than an event: the notice stays until a + * screen has shown it to the end ([consumed]). The usual way to set an alarm is * pick → OK → back, and back disposes the editor's snackbar a moment after it - * appeared; a one-shot event would be gone with it, while this one is picked - * up again by the list underneath. A rotation mid-snackbar re-shows it the - * same way. + * appeared; the list underneath picks the same notice up. Announcing runs on + * the application scope for the same reason — leaving the editor while its + * repeat days settle must not lose them. */ @Singleton internal class AlarmSetAnnouncer @Inject constructor( private val engine: AlarmEngine, private val wall: WallClock, + private val elapsed: ElapsedRealtimeClock, + // `@param:` — the qualifier has no explicit @Target, and K2 resolves a bare + // annotation on a constructor `val` to the property instead. + @param:ApplicationScope private val appScope: CoroutineScope, ) { private val current = MutableStateFlow(null) /** - * Serialises [announce]: it reads storage and suspends before it writes - * [current], and two switch taps in quick succession must not let the - * earlier read land last — "set for 9 hours" about an alarm that is off. + * Serialises the read-then-publish: two switch taps in quick succession + * must not let the earlier read land last — "set for 9 hours" about an + * alarm that is off. */ private val lock = Mutex() + /** The request still settling, and the alarm it is about. Guarded by `this`. */ + private var pending: Job? = null + private var pendingFor: Long? = null + /** * The notice to show, or null. Past [FRESH_FOR] it is dropped unshown: a - * countdown that surfaced when the user came back to the app an hour - * later would be wrong, and about something they did long ago. + * countdown that surfaced when the user came back to the app a while later + * would be wrong, and about something they did long ago. */ val notice: Flow = current.map { notice -> - notice?.takeIf { wall.now() - it.announcedAt < FRESH_FOR } + notice?.takeIf { elapsed.elapsedRealtime() - it.announcedAt < FRESH_FOR } } /** - * Call once the write is stored. Reads the alarm as it is now, so it is never - * about a superseded value. An alarm that will not ring takes back a notice - * still pending about it, and otherwise says nothing. + * Call once the write is stored. Announces [alarmId] after [settle], unless + * a newer request replaces this one first — latest wins, so a burst of + * changes is one snackbar. It reads the alarm as it is then, so it is never + * about a superseded value; an alarm that will not ring takes back a notice + * still standing about it, and otherwise says nothing. */ - suspend fun announce(alarmId: Long): Unit = lock.withLock { - val now = wall.now() - val countdown = AlarmSetCountdown.of(engine.upcomingFor(alarmId), now) - if (countdown != null) { - current.value = AlarmSetNotice(alarmId, countdown, announcedAt = now) - } else if (current.value?.alarmId == alarmId) { - current.value = null + fun announce(alarmId: Long, settle: Duration = Duration.ZERO) { + synchronized(this) { + pending?.cancel() + pendingFor = alarmId + pending = appScope.launch { + delay(settle) + publish(alarmId) + } } } + /** Takes back anything said or about to be said about [alarmId]: it was deleted or dismissed. */ + fun retract(alarmId: Long) { + synchronized(this) { + if (pendingFor == alarmId) { + pending?.cancel() + pending = null + pendingFor = null + } + } + current.update { notice -> notice?.takeUnless { it.alarmId == alarmId } } + } + /** [notice] was shown until it timed out or was swiped away. */ fun consumed(notice: AlarmSetNotice) { current.compareAndSet(notice, null) } + private suspend fun publish(alarmId: Long) = lock.withLock { + // One `now` for both ends of the countdown, or a resolve that lands a + // hair past the occurrence would read as a day and an hour. + val now = wall.now() + val countdown = AlarmSetCountdown.of(engine.upcomingFor(alarmId, now), now) + current.update { standing -> + when { + // The same words about the same alarm stay as they are, rather + // than taking the snackbar down and putting it straight back. + countdown != null && standing?.alarmId == alarmId && standing.countdown == countdown -> standing + countdown != null -> AlarmSetNotice(alarmId, countdown, elapsed.elapsedRealtime()) + standing?.alarmId == alarmId -> null + else -> standing + } + } + } + private companion object { /** Comfortably longer than a short snackbar, so a hand-over between screens fits. */ val FRESH_FOR = 10.seconds @@ -95,13 +146,15 @@ internal class AlarmSetAnnouncer @Inject constructor( } /** - * Shows [notice] on [host]. Latest wins: a new countdown replaces the one on - * screen rather than queueing behind it, so a burst of changes is one snackbar - * that ends up saying the right thing, and a null takes it down. + * Shows [notice] on [host] while the screen is resumed. Latest wins: a new + * countdown replaces the one on screen rather than queueing behind it, and a + * null takes it down. * - * [onShown] runs only when the snackbar ended on its own or was swiped away. - * One cut short — by leaving the screen, or a rotation — stays pending for the - * next screen to show. + * Resumed, not merely composed: during a navigation cross-fade only the screen + * arriving is resumed, so the two never show it at once, and in the background + * nobody would see it. [onShown] runs only when the snackbar ended on its own or + * was swiped away; one cut short — by leaving, a rotation, the app going to the + * background — stays pending for whichever screen resumes next. */ @Composable internal fun AlarmSetSnackbarEffect( @@ -110,13 +163,15 @@ internal fun AlarmSetSnackbarEffect( onShown: (AlarmSetNotice) -> Unit, ) { val resources = LocalResources.current + val lifecycle = LocalLifecycleOwner.current.lifecycle val shown by rememberUpdatedState(onShown) - LaunchedEffect(notice, host, resources) { - notice.collectLatest { current -> - if (current == null) return@collectLatest - val message = alarmSetMessage(resources, current.countdown) - if (message != null) host.showSnackbar(message) - shown(current) + LaunchedEffect(notice, host, resources, lifecycle) { + lifecycle.repeatOnLifecycle(Lifecycle.State.RESUMED) { + notice.collectLatest { current -> + if (current == null) return@collectLatest + host.showSnackbar(alarmSetMessage(resources, current.countdown) ?: return@collectLatest) + shown(current) + } } } } diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmsViewModel.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmsViewModel.kt index cd16481..2b38175 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmsViewModel.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmsViewModel.kt @@ -83,7 +83,14 @@ class AlarmsViewModel @Inject internal constructor( // One engine verb, so the contract's dismissal and the user's are the // same code: it closes a ring, arms a repeating alarm's skip, or // disables a one-shot — each of which moves state only the engine owns. - viewModelScope.launch { engine.dismissUpcoming(alarmId) } + viewModelScope.launch { + engine.dismissUpcoming(alarmId) + // Whatever the dismissal did — disabled it, or skipped the very + // occurrence a notice counts down to — "set for" no longer holds. + // Not re-announced: this is the `AlarmClock` contract's request + // (M9 D13), which the snackbar stays out of (#19). + announcer.retract(alarmId) + } } /** Dismisses a ring first when switching off (M5 D11), then re-resolves. */ diff --git a/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorViewModelTest.kt b/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorViewModelTest.kt index 02ad01b..48d5f32 100644 --- a/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorViewModelTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorViewModelTest.kt @@ -1,6 +1,7 @@ package de.jeanlucmakiola.clockula.ui.alarms import androidx.lifecycle.SavedStateHandle +import androidx.lifecycle.viewModelScope import app.cash.turbine.test import com.google.common.truth.Truth.assertThat import de.jeanlucmakiola.clockula.data.alarms.AlarmMapper @@ -11,6 +12,7 @@ import de.jeanlucmakiola.clockula.domain.TimeOfDay import de.jeanlucmakiola.clockula.domain.format.NextFireLabel import de.jeanlucmakiola.clockula.testing.AlarmEngineHarness import de.jeanlucmakiola.clockula.testing.BERLIN +import de.jeanlucmakiola.clockula.testing.FakeElapsedRealtimeClock import de.jeanlucmakiola.clockula.testing.FakeRingtoneCatalog import de.jeanlucmakiola.clockula.testing.FakeRingtonePreviewer import de.jeanlucmakiola.clockula.testing.MainDispatcherExtension @@ -23,6 +25,7 @@ import de.jeanlucmakiola.clockula.testing.instantAt import de.jeanlucmakiola.clockula.testing.offsetInstant import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.cancel import kotlinx.coroutines.flow.first import kotlinx.coroutines.test.StandardTestDispatcher import kotlinx.coroutines.test.TestScope @@ -36,6 +39,7 @@ import org.junit.jupiter.api.io.TempDir import java.nio.file.Path import java.time.DayOfWeek import kotlin.time.Duration.Companion.hours +import kotlin.time.Duration.Companion.seconds /** * The editor over the **real** alarm engine: every control writes at once @@ -56,10 +60,15 @@ class AlarmEditorViewModelTest { private val sevenToday = offsetInstant("2026-06-10T07:00+02:00") private val sevenTomorrow = offsetInstant("2026-06-11T07:00+02:00") + private val elapsed = FakeElapsedRealtimeClock() + + private fun TestScope.announcer(harness: AlarmEngineHarness) = + AlarmSetAnnouncer(harness.engine, harness.wallClock, elapsed, appScope = this) + private fun TestScope.editor( harness: AlarmEngineHarness, alarmId: Long = 1L, - announcer: AlarmSetAnnouncer = AlarmSetAnnouncer(harness.engine, harness.wallClock), + announcer: AlarmSetAnnouncer = announcer(harness), ) = AlarmEditorViewModel( savedStateHandle = SavedStateHandle(mapOf(AlarmRoutes.ARG_ALARM_ID to alarmId)), @@ -340,12 +349,14 @@ class AlarmEditorViewModelTest { runTest(main.dispatcher) { val harness = alarmEngineHarness(tempDir, now = fiveToday) harness.givenAlarmWith(hour = 7) - val announcer = AlarmSetAnnouncer(harness.engine, harness.wallClock) + val announcer = announcer(harness) val viewModel = editor(harness, announcer = announcer) advanceUntilIdle() viewModel.onDayToggled(DayOfWeek.MONDAY) advanceTimeBy(200) + // What `ViewModel.clear()` does first: the settle must not die with it. + viewModel.viewModelScope.cancel() viewModel.onCleared() advanceUntilIdle() @@ -422,11 +433,66 @@ class AlarmEditorViewModelTest { viewModel.onTimePicked(6, 45) advanceUntilIdle() - harness.wallClock.instant = fiveToday + 1.hours + // Monotonic: a wall clock set back an hour must not make it fresh again. + elapsed.advance(1.hours) + harness.wallClock.instant = fiveToday - 1.hours assertThat(viewModel.alarmSetNotice.first()).isNull() } + @Test + fun `a notice about another alarm is not shown in this editor`(@TempDir tempDir: Path) = + runTest(main.dispatcher) { + val harness = alarmEngineHarness(tempDir, now = fiveToday) + harness.givenAlarmWith(id = 1L, hour = 7) + harness.givenAlarmWith(id = 2L, hour = 22) + val announcer = announcer(harness) + val viewModel = editor(harness, alarmId = 2L, announcer = announcer) + advanceUntilIdle() + + announcer.announce(1L) + advanceUntilIdle() + + assertThat(announcer.notice.first()?.alarmId to viewModel.alarmSetNotice.first()) + .isEqualTo(1L to null) + } + + @Test + fun `deleting the alarm takes its notice back`(@TempDir tempDir: Path) = + runTest(main.dispatcher) { + val harness = alarmEngineHarness(tempDir, now = fiveToday) + harness.givenAlarmWith(hour = 7) + val announcer = announcer(harness) + val viewModel = editor(harness, announcer = announcer) + advanceUntilIdle() + viewModel.onTimePicked(6, 45) + advanceUntilIdle() + + viewModel.onDelete() + advanceUntilIdle() + + assertThat(announcer.notice.first()).isNull() + } + + @Test + fun `the same countdown again leaves the standing notice alone`(@TempDir tempDir: Path) = + runTest(main.dispatcher) { + val harness = alarmEngineHarness(tempDir, now = fiveToday) + harness.givenAlarmWith(hour = 7) + val viewModel = editor(harness) + advanceUntilIdle() + viewModel.onTimePicked(6, 45) + advanceUntilIdle() + val first = viewModel.alarmSetNotice.first() + + // A snackbar taken down and put straight back would flicker. + elapsed.advance(1.seconds) + viewModel.onTimePicked(6, 45) + advanceUntilIdle() + + assertThat(viewModel.alarmSetNotice.first()).isSameInstanceAs(first) + } + // --- label --- @Test diff --git a/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmsViewModelTest.kt b/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmsViewModelTest.kt index 64b0276..64530dd 100644 --- a/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmsViewModelTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmsViewModelTest.kt @@ -7,6 +7,7 @@ import de.jeanlucmakiola.clockula.domain.TimeOfDay import de.jeanlucmakiola.clockula.domain.format.NextFireLabel import de.jeanlucmakiola.clockula.testing.AlarmEngineHarness import de.jeanlucmakiola.clockula.testing.BERLIN +import de.jeanlucmakiola.clockula.testing.FakeElapsedRealtimeClock import de.jeanlucmakiola.clockula.testing.MainDispatcherExtension import de.jeanlucmakiola.clockula.testing.RingEvent import de.jeanlucmakiola.clockula.testing.alarmEngineHarness @@ -53,7 +54,7 @@ class AlarmsViewModelTest { wall = harness.wallClock, zones = harness.zones, source = AlarmListSource(harness.engine, harness.wallClock, harness.ticker), - announcer = AlarmSetAnnouncer(harness.engine, harness.wallClock), + announcer = AlarmSetAnnouncer(harness.engine, harness.wallClock, FakeElapsedRealtimeClock(), this), ).also { viewModel -> backgroundScope.launch { viewModel.state.collect {} } } private fun AlarmsUiState.rows(): List = (this as AlarmsUiState.Loaded).rows @@ -239,6 +240,24 @@ class AlarmsViewModelTest { assertThat(viewModel.alarmSetNotice.first()?.alarmId).isEqualTo(1L) } + @Test + fun `dismissing the announced alarm takes the announcement back`(@TempDir tempDir: Path) = + runTest(main.dispatcher) { + val harness = alarmEngineHarness(tempDir) + harness.givenAlarm(id = 1L, hour = 7, repeatDays = RepeatDays.EVERY_DAY, enabled = false) + val viewModel = viewModel(harness) + advanceUntilIdle() + viewModel.onToggleEnabled(1L, enabled = true) + advanceUntilIdle() + + viewModel.onDismissUpcoming(1L) + advanceUntilIdle() + + // Skipped, still on — and still not re-announced: the dismissal is + // the AlarmClock contract's, which the snackbar stays out of. + assertThat(viewModel.alarmSetNotice.first()).isNull() + } + @Test fun `adding an alarm leaves the announcement to the editor`(@TempDir tempDir: Path) = runTest(main.dispatcher) { diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 2051aee..f63c3bc 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -1126,8 +1126,13 @@ at once, but waits for repeat-day taps to settle for a second, so Mon–Fri is one snackbar rather than five. `AlarmSetAnnouncer` is an app-wide singleton holding the notice as *state* until a screen has shown it to the end: pick → OK → back disposes the editor's snackbar a moment after it appears, and the -list underneath picks the same notice up. A notice older than ten seconds is -dropped unshown rather than shown with a stale countdown. +list underneath picks the same notice up. The settle runs on the application +scope, so leaving the editor mid-settle still announces; a screen shows the +notice only while resumed, so a cross-fade never shows it twice; the editor +shows only notices about its own alarm; deleting an alarm, or the `AlarmClock` +chooser dismissing it, takes its notice back. A notice older than ten seconds +on the monotonic clock is dropped unshown rather than shown with a stale +countdown. `upcoming(refresh: Flow)` takes its **re-read cadence as a parameter**. The resolver needs a `now`, and `upcoming` reads one inside a `combine` of the