From 207475ef3783ee18cdc3e6e3337dc3d314634943 Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Wed, 21 Oct 2026 19:00:56 +0200 Subject: [PATCH] fix(alarms): keep the alarm-set notice until a screen has shown it The usual way to set an alarm is pick, OK, back, and back disposed the editor's snackbar a moment after it appeared. Day taps left within the one-second settle were never announced at all, and a rotation threw a showing snackbar away for good. The announcer is now an app-wide singleton holding the notice as state until a snackbar has run to the end, so the list picks up what the editor could not finish showing. An editor closed mid-settle announces on the way out. Announcing is serialised, so a quick on-then-off cannot land the "on" read last, and a notice nobody showed within ten seconds is dropped rather than shown stale. The engine's one-alarm read now shares upcoming()'s preview helper. Refs #19 --- .../clockula/alarm/AlarmEngine.kt | 23 ++-- .../clockula/ui/alarms/AlarmEditorScreen.kt | 2 +- .../ui/alarms/AlarmEditorViewModel.kt | 23 ++-- .../clockula/ui/alarms/AlarmSetSnackbar.kt | 129 +++++++++++------- .../clockula/ui/alarms/AlarmsScreen.kt | 2 +- .../clockula/ui/alarms/AlarmsViewModel.kt | 10 +- .../ui/alarms/AlarmEditorViewModelTest.kt | 119 +++++++++++----- .../clockula/ui/alarms/AlarmsViewModelTest.kt | 46 +++---- docs/ARCHITECTURE.md | 6 +- 9 files changed, 225 insertions(+), 135 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 d567025..320512d 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/alarm/AlarmEngine.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/AlarmEngine.kt @@ -29,6 +29,7 @@ import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.flow.map import kotlinx.coroutines.sync.Mutex import kotlinx.coroutines.sync.withLock +import java.time.ZoneId import kotlin.time.Duration.Companion.minutes import kotlin.time.Instant import javax.inject.Inject @@ -187,17 +188,7 @@ class AlarmEngine @Inject constructor( val zone = zones.current() val byId = states.associateBy { it.alarmId } alarms - .map { alarm -> - // The resolver's state is deliberately discarded: a read must - // not write, however much it resolved on the way past. - val resolution = AlarmResolver.resolve( - alarm, - byId[alarm.id] ?: AlarmRingState.initial(alarm.id), - now, - zone, - ) - UpcomingAlarm(alarm, resolution.nextFire, resolution.source) - } + .map { alarm -> preview(alarm, byId[alarm.id] ?: AlarmRingState.initial(alarm.id), now, zone) } .sortedWith( compareBy(nullsLast()) { it.nextFire } .thenBy { it.alarm.id }, @@ -211,7 +202,15 @@ class AlarmEngine @Inject constructor( */ suspend fun upcomingFor(alarmId: Long): UpcomingAlarm? { val alarm = alarms.find(alarmId) ?: return null - val resolution = AlarmResolver.resolve(alarm, states.state(alarmId), wallClock.now(), zones.current()) + return preview(alarm, states.state(alarmId), wallClock.now(), zones.current()) + } + + /** + * The resolver in preview mode. Its state is deliberately discarded: a read + * must not write, however much it resolved on the way past. + */ + private fun preview(alarm: Alarm, state: AlarmRingState, now: Instant, zone: ZoneId): UpcomingAlarm { + val resolution = AlarmResolver.resolve(alarm, state, now, zone) return UpcomingAlarm(alarm, resolution.nextFire, resolution.source) } 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 44787a6..088a42a 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 @@ -63,7 +63,7 @@ fun AlarmEditorScreen( var confirmingDelete by rememberSaveable { mutableStateOf(false) } val formatTime = rememberAlarmTimeFormatter() val snackbarHost = remember { SnackbarHostState() } - AlarmSetSnackbarEffect(viewModel.snackbars, snackbarHost) + AlarmSetSnackbarEffect(viewModel.alarmSetNotice, snackbarHost, viewModel::onAlarmSetNoticeShown) // Registered unconditionally, and deliberately **not** inside the // `picker?.let` below: the picker's open flag is ViewModel state that does 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 cffc860..f8c2ba1 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 @@ -14,7 +14,6 @@ import de.jeanlucmakiola.clockula.domain.ClockDefaults import de.jeanlucmakiola.clockula.domain.DismissChallenge import de.jeanlucmakiola.clockula.domain.Ringtones import de.jeanlucmakiola.clockula.domain.TimeOfDay -import de.jeanlucmakiola.clockula.domain.time.WallClock import de.jeanlucmakiola.clockula.ui.common.RingtonePicker import de.jeanlucmakiola.clockula.ui.common.RingtonePickerUiState import de.jeanlucmakiola.floret.di.ApplicationScope @@ -47,7 +46,7 @@ class AlarmEditorViewModel @Inject internal constructor( private val engine: AlarmEngine, private val ringtones: RingtoneCatalog, private val preview: RingtonePreviewer, - wall: WallClock, + private val announcer: AlarmSetAnnouncer, // `@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, @@ -81,17 +80,20 @@ class AlarmEditorViewModel @Inject internal constructor( /** What the previewer is sounding, so the next preview knows to stop it. */ private var previewing: String? = null - private val announcer = AlarmSetAnnouncer(engine, wall) - /** 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 + /** - * One-shot: "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. + * "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. */ - val snackbars: Flow = announcer.snackbars + val alarmSetNotice: Flow = announcer.notice + + fun onAlarmSetNoticeShown(notice: AlarmSetNotice) = announcer.consumed(notice) /** Memoised catalog answers, keyed on URI — see [metaOf]. */ private val ringtoneMeta = mutableMapOf() @@ -254,6 +256,9 @@ 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() } @@ -287,8 +292,10 @@ class AlarmEditorViewModel @Inject internal constructor( */ private fun announce(settle: Duration = Duration.ZERO) { announcing?.cancel() + announcementPending = true announcing = viewModelScope.launch { delay(settle) + announcementPending = false announcer.announce(alarmId) } } 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 d13123a..cc26029 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 @@ -4,84 +4,119 @@ import android.content.res.Resources import androidx.compose.material3.SnackbarHostState import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect +import androidx.compose.runtime.getValue +import androidx.compose.runtime.rememberUpdatedState import androidx.compose.ui.platform.LocalResources 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.WallClock -import kotlinx.coroutines.channels.BufferOverflow import kotlinx.coroutines.flow.Flow -import kotlinx.coroutines.flow.MutableSharedFlow -import kotlinx.coroutines.flow.asSharedFlow +import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.collectLatest +import kotlinx.coroutines.flow.map +import kotlinx.coroutines.sync.Mutex +import kotlinx.coroutines.sync.withLock +import javax.inject.Inject +import javax.inject.Singleton +import kotlin.time.Duration.Companion.seconds +import kotlin.time.Instant -/** A one-shot for the alarm screens' "Alarm set for … from now" snackbar (#19). */ -sealed interface AlarmSetSnackbar { - data class Show(val countdown: NextFireLabel) : AlarmSetSnackbar - - /** Takes back a "set for" that stopped being true: the alarm was switched off. */ - data object Hide : AlarmSetSnackbar -} +/** "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, +) /** * Says how long until an alarm rings, right after a user action in the app set * it — the moment an AM/PM or wrong-day slip is cheapest to notice (#19). * - * Only the list and the editor own one, so an alarm set through the + * Only the list and the editor call it, so an alarm set through the * `AlarmClock` intents never announces itself: those already get the - * status-bar icon. Not thread-safe; both owners call it from `viewModelScope`. + * 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 + * 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. */ -internal class AlarmSetAnnouncer( +@Singleton +internal class AlarmSetAnnouncer @Inject constructor( private val engine: AlarmEngine, private val wall: WallClock, ) { - // Dropped, not buffered, with nobody collecting: a countdown that surfaced - // minutes later, when the screen came back, would be stale — and about an - // action the user has long since moved on from. - private val pending = MutableSharedFlow( - extraBufferCapacity = 1, - onBufferOverflow = BufferOverflow.DROP_OLDEST, - ) + private val current = MutableStateFlow(null) - val snackbars: Flow = pending.asSharedFlow() + /** + * 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. + */ + private val lock = Mutex() - /** The alarm the last [AlarmSetSnackbar.Show] was about, so only it can be taken back. */ - private var announced: 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. + */ + val notice: Flow = current.map { notice -> + notice?.takeIf { wall.now() - it.announcedAt < FRESH_FOR } + } - /** Call once the write is stored. Says nothing for an alarm that will not ring. */ - suspend fun announce(alarmId: Long) { - val countdown = AlarmSetCountdown.of(engine.upcomingFor(alarmId), wall.now()) - when { - countdown != null -> { - announced = alarmId - pending.tryEmit(AlarmSetSnackbar.Show(countdown)) - } - announced == alarmId -> { - announced = null - pending.tryEmit(AlarmSetSnackbar.Hide) - } + /** + * 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. + */ + 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 } } + + /** [notice] was shown until it timed out or was swiped away. */ + fun consumed(notice: AlarmSetNotice) { + current.compareAndSet(notice, null) + } + + private companion object { + /** Comfortably longer than a short snackbar, so a hand-over between screens fits. */ + val FRESH_FOR = 10.seconds + } } /** - * Shows [snackbars] on [host]. Latest wins: a new countdown replaces the one on + * 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. + * that ends up saying the right thing, 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. */ @Composable -internal fun AlarmSetSnackbarEffect(snackbars: Flow, host: SnackbarHostState) { +internal fun AlarmSetSnackbarEffect( + notice: Flow, + host: SnackbarHostState, + onShown: (AlarmSetNotice) -> Unit, +) { val resources = LocalResources.current - LaunchedEffect(snackbars, host, resources) { - snackbars.collectLatest { snackbar -> - when (snackbar) { - is AlarmSetSnackbar.Show -> - alarmSetMessage(resources, snackbar.countdown)?.let { host.showSnackbar(it) } - // The previous Show was cancelled by collectLatest, which already - // dismissed it; this covers one shown before a recomposition. - AlarmSetSnackbar.Hide -> host.currentSnackbarData?.dismiss() - } + 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) } } } diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmsScreen.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmsScreen.kt index d3ca118..519bb2f 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmsScreen.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmsScreen.kt @@ -69,7 +69,7 @@ fun AlarmsScreen( // the id to navigate to arrives from the ViewModel (M5 D2, D22). viewModel.openEditor.collectLatest(onCreateAlarm) } - AlarmSetSnackbarEffect(viewModel.snackbars, snackbarHost) + AlarmSetSnackbarEffect(viewModel.alarmSetNotice, snackbarHost, viewModel::onAlarmSetNoticeShown) CollapsingScaffold( title = stringResource(R.string.tab_alarms), 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 3152279..cd16481 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 @@ -28,6 +28,7 @@ class AlarmsViewModel @Inject internal constructor( private val wall: WallClock, private val zones: ZoneProvider, private val source: AlarmListSource, + private val announcer: AlarmSetAnnouncer, ) : ViewModel() { val state: StateFlow = source.rows() @@ -44,10 +45,13 @@ class AlarmsViewModel @Inject internal constructor( // into two navigations (M5 D22). private val newAlarms = Channel(Channel.CONFLATED) - private val announcer = AlarmSetAnnouncer(engine, wall) + /** + * "Alarm set for … from now", after the row's switch turns an alarm on — or + * handed over by the editor the user has just left (#19). + */ + val alarmSetNotice: Flow = announcer.notice - /** One-shot: "Alarm set for … from now" after the row's switch turns an alarm on (#19). */ - val snackbars: Flow = announcer.snackbars + fun onAlarmSetNoticeShown(notice: AlarmSetNotice) = announcer.consumed(notice) /** One-shot: the id of a freshly created alarm, for the caller to navigate to. */ val openEditor: Flow = newAlarms.receiveAsFlow().onEach { creating = false } 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 4759e80..02ad01b 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 @@ -23,6 +23,7 @@ import de.jeanlucmakiola.clockula.testing.instantAt import de.jeanlucmakiola.clockula.testing.offsetInstant import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.flow.first import kotlinx.coroutines.test.StandardTestDispatcher import kotlinx.coroutines.test.TestScope import kotlinx.coroutines.test.advanceTimeBy @@ -34,6 +35,7 @@ import org.junit.jupiter.api.extension.RegisterExtension import org.junit.jupiter.api.io.TempDir import java.nio.file.Path import java.time.DayOfWeek +import kotlin.time.Duration.Companion.hours /** * The editor over the **real** alarm engine: every control writes at once @@ -54,7 +56,11 @@ class AlarmEditorViewModelTest { private val sevenToday = offsetInstant("2026-06-10T07:00+02:00") private val sevenTomorrow = offsetInstant("2026-06-11T07:00+02:00") - private fun TestScope.editor(harness: AlarmEngineHarness, alarmId: Long = 1L) = + private fun TestScope.editor( + harness: AlarmEngineHarness, + alarmId: Long = 1L, + announcer: AlarmSetAnnouncer = AlarmSetAnnouncer(harness.engine, harness.wallClock), + ) = AlarmEditorViewModel( savedStateHandle = SavedStateHandle(mapOf(AlarmRoutes.ARG_ALARM_ID to alarmId)), alarms = harness.alarms, @@ -62,7 +68,7 @@ class AlarmEditorViewModelTest { engine = harness.engine, ringtones = catalog, preview = previewer, - wall = harness.wallClock, + announcer = announcer, appScope = this, ) @@ -258,6 +264,8 @@ class AlarmEditorViewModelTest { // --- "Alarm set for … from now" (#19) --- + private suspend fun AlarmEditorViewModel.countdown() = alarmSetNotice.first()?.countdown + @Test fun `picking a time says how long until it rings`(@TempDir tempDir: Path) = runTest(main.dispatcher) { @@ -266,12 +274,10 @@ class AlarmEditorViewModelTest { val viewModel = editor(harness) advanceUntilIdle() - viewModel.snackbars.test { - viewModel.onTimePicked(6, 45) - advanceUntilIdle() + viewModel.onTimePicked(6, 45) + advanceUntilIdle() - assertThat(awaitItem()).isEqualTo(AlarmSetSnackbar.Show(NextFireLabel.InHoursMinutes(1, 45))) - } + assertThat(viewModel.countdown()).isEqualTo(NextFireLabel.InHoursMinutes(1, 45)) } @Test @@ -282,14 +288,12 @@ class AlarmEditorViewModelTest { val viewModel = editor(harness) advanceUntilIdle() - viewModel.snackbars.test { - // The AM/PM slip the snackbar is there to catch: 4:30 is behind - // us, so it rings tomorrow, 23½ hours away. - viewModel.onTimePicked(4, 30) - advanceUntilIdle() + // The AM/PM slip the snackbar is there to catch: 4:30 is behind + // us, so it rings tomorrow, 23½ hours away. + viewModel.onTimePicked(4, 30) + advanceUntilIdle() - assertThat(awaitItem()).isEqualTo(AlarmSetSnackbar.Show(NextFireLabel.InHoursMinutes(23, 30))) - } + assertThat(viewModel.countdown()).isEqualTo(NextFireLabel.InHoursMinutes(23, 30)) } @Test @@ -300,12 +304,10 @@ class AlarmEditorViewModelTest { val viewModel = editor(harness) advanceUntilIdle() - viewModel.snackbars.test { - viewModel.onNewAlarmTimeKept() - advanceUntilIdle() + viewModel.onNewAlarmTimeKept() + advanceUntilIdle() - assertThat(awaitItem()).isEqualTo(AlarmSetSnackbar.Show(NextFireLabel.InHoursMinutes(1, 0))) - } + assertThat(viewModel.countdown()).isEqualTo(NextFireLabel.InHoursMinutes(1, 0)) } @Test @@ -316,7 +318,8 @@ class AlarmEditorViewModelTest { val viewModel = editor(harness) advanceUntilIdle() - viewModel.snackbars.test { + viewModel.alarmSetNotice.test { + assertThat(awaitItem()).isNull() viewModel.onDayToggled(DayOfWeek.MONDAY) advanceTimeBy(500) viewModel.onDayToggled(DayOfWeek.TUESDAY) @@ -327,11 +330,29 @@ class AlarmEditorViewModelTest { advanceUntilIdle() // 2026-06-10 is a Wednesday: 7:00 today, two hours out. - assertThat(awaitItem()).isEqualTo(AlarmSetSnackbar.Show(NextFireLabel.InHoursMinutes(2, 0))) + assertThat(awaitItem()?.countdown).isEqualTo(NextFireLabel.InHoursMinutes(2, 0)) expectNoEvents() } } + @Test + fun `leaving while the days settle still announces them, for the list`(@TempDir tempDir: Path) = + runTest(main.dispatcher) { + val harness = alarmEngineHarness(tempDir, now = fiveToday) + harness.givenAlarmWith(hour = 7) + val announcer = AlarmSetAnnouncer(harness.engine, harness.wallClock) + val viewModel = editor(harness, announcer = announcer) + advanceUntilIdle() + + viewModel.onDayToggled(DayOfWeek.MONDAY) + advanceTimeBy(200) + viewModel.onCleared() + advanceUntilIdle() + + // Wednesday 05:00 to Monday 07:00. + assertThat(announcer.notice.first()?.countdown).isEqualTo(NextFireLabel.InDaysHours(5, 2)) + } + @Test fun `arming a skip says it rings at the occurrence after it`(@TempDir tempDir: Path) = runTest(main.dispatcher) { @@ -341,12 +362,10 @@ class AlarmEditorViewModelTest { val viewModel = editor(harness) advanceUntilIdle() - viewModel.snackbars.test { - viewModel.onSkipNextChanged(true) - advanceUntilIdle() + viewModel.onSkipNextChanged(true) + advanceUntilIdle() - assertThat(awaitItem()).isEqualTo(AlarmSetSnackbar.Show(NextFireLabel.InDaysHours(1, 2))) - } + assertThat(viewModel.countdown()).isEqualTo(NextFireLabel.InDaysHours(1, 2)) } @Test @@ -357,12 +376,10 @@ class AlarmEditorViewModelTest { val viewModel = editor(harness) advanceUntilIdle() - viewModel.snackbars.test { - viewModel.onTimePicked(6, 45) - advanceUntilIdle() + viewModel.onTimePicked(6, 45) + advanceUntilIdle() - expectNoEvents() - } + assertThat(viewModel.alarmSetNotice.first()).isNull() } @Test @@ -373,13 +390,41 @@ class AlarmEditorViewModelTest { val viewModel = editor(harness) advanceUntilIdle() - viewModel.snackbars.test { - viewModel.onLabelTyped("Gym") - viewModel.onSnoozeMinutesChanged(5) - advanceUntilIdle() + viewModel.onLabelTyped("Gym") + viewModel.onSnoozeMinutesChanged(5) + advanceUntilIdle() - expectNoEvents() - } + assertThat(viewModel.alarmSetNotice.first()).isNull() + } + + @Test + fun `a notice shown to the end is gone`(@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() + + viewModel.onAlarmSetNoticeShown(checkNotNull(viewModel.alarmSetNotice.first())) + + assertThat(viewModel.alarmSetNotice.first()).isNull() + } + + @Test + fun `a notice nobody showed in time is dropped, not shown stale`(@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() + + harness.wallClock.instant = fiveToday + 1.hours + + assertThat(viewModel.alarmSetNotice.first()).isNull() } // --- label --- 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 4fc6f00..64b0276 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 @@ -13,6 +13,7 @@ import de.jeanlucmakiola.clockula.testing.alarmEngineHarness import de.jeanlucmakiola.clockula.testing.instantAt import de.jeanlucmakiola.clockula.testing.offsetInstant import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.flow.first import kotlinx.coroutines.launch import kotlinx.coroutines.test.StandardTestDispatcher import kotlinx.coroutines.test.TestScope @@ -52,6 +53,7 @@ class AlarmsViewModelTest { wall = harness.wallClock, zones = harness.zones, source = AlarmListSource(harness.engine, harness.wallClock, harness.ticker), + announcer = AlarmSetAnnouncer(harness.engine, harness.wallClock), ).also { viewModel -> backgroundScope.launch { viewModel.state.collect {} } } private fun AlarmsUiState.rows(): List = (this as AlarmsUiState.Loaded).rows @@ -198,12 +200,10 @@ class AlarmsViewModelTest { val viewModel = viewModel(harness) advanceUntilIdle() - viewModel.snackbars.test { - viewModel.onToggleEnabled(1L, enabled = true) - advanceUntilIdle() + viewModel.onToggleEnabled(1L, enabled = true) + advanceUntilIdle() - assertThat(awaitItem()).isEqualTo(AlarmSetSnackbar.Show(NextFireLabel.InHoursMinutes(2, 12))) - } + assertThat(viewModel.alarmSetNotice.first()?.countdown).isEqualTo(NextFireLabel.InHoursMinutes(2, 12)) } @Test @@ -214,31 +214,29 @@ class AlarmsViewModelTest { val viewModel = viewModel(harness) advanceUntilIdle() - viewModel.snackbars.test { - viewModel.onToggleEnabled(1L, enabled = true) - advanceUntilIdle() - skipItems(1) - viewModel.onToggleEnabled(1L, enabled = false) - advanceUntilIdle() + // Both taps before either has run: the "on" read must not land last. + viewModel.onToggleEnabled(1L, enabled = true) + viewModel.onToggleEnabled(1L, enabled = false) + advanceUntilIdle() - assertThat(awaitItem()).isEqualTo(AlarmSetSnackbar.Hide) - } + assertThat(viewModel.alarmSetNotice.first()).isNull() } @Test - fun `switching off an alarm nobody was told about says nothing`(@TempDir tempDir: Path) = + fun `switching off another alarm leaves the announcement standing`(@TempDir tempDir: Path) = runTest(main.dispatcher) { val harness = alarmEngineHarness(tempDir) - harness.givenAlarm(id = 1L, hour = 7) + harness.givenAlarm(id = 1L, hour = 7, enabled = false) + harness.givenAlarm(id = 2L, hour = 9) val viewModel = viewModel(harness) advanceUntilIdle() - viewModel.snackbars.test { - viewModel.onToggleEnabled(1L, enabled = false) - advanceUntilIdle() + viewModel.onToggleEnabled(1L, enabled = true) + advanceUntilIdle() + viewModel.onToggleEnabled(2L, enabled = false) + advanceUntilIdle() - expectNoEvents() - } + assertThat(viewModel.alarmSetNotice.first()?.alarmId).isEqualTo(1L) } @Test @@ -248,12 +246,10 @@ class AlarmsViewModelTest { val viewModel = viewModel(harness) advanceUntilIdle() - viewModel.snackbars.test { - viewModel.onAddAlarm() - advanceUntilIdle() + viewModel.onAddAlarm() + advanceUntilIdle() - expectNoEvents() - } + assertThat(viewModel.alarmSetNotice.first()).isNull() } @Test diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 261deb5..2051aee 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -1123,7 +1123,11 @@ only a natural occurrence of an enabled alarm, rounded as the row's countdown is, never a snooze or a ring. The `AlarmClock` intents never reach it, since they already get the status-bar icon (#19). The editor announces a time pick at once, but waits for repeat-day taps to settle for a second, so Mon–Fri is -one snackbar rather than five. +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. `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