fix(alarms): keep the alarm-set notice live and editor writes in order

The notice now follows the engine's upcoming() list, so a snackbar
already on screen comes down when its alarm is switched off by another
path, and one whose ring moved, such as a skip armed from the
notification, shows the new countdown. Consuming matches the
announcement, not the words.

Editor writes that move a fire time, skip-next and delete run on the
application scope through one fair queue, entered undispatched so they
commit in tap order. Without it, "skip next" followed at once by
untapping the only repeat day could land out of order and leave a
one-shot with a skip armed, which disables it.

Refs #19

app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorViewModel.kt

@@ -18,6 +18,7 @@ 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.CoroutineStart
 import kotlinx.coroutines.flow.Flow
 import kotlinx.coroutines.flow.MutableStateFlow
 import kotlinx.coroutines.flow.StateFlow
@@ -26,6 +27,8 @@ import kotlinx.coroutines.flow.collectLatest
 import kotlinx.coroutines.flow.combine
 import kotlinx.coroutines.flow.map
 import kotlinx.coroutines.launch
+import kotlinx.coroutines.sync.Mutex
+import kotlinx.coroutines.sync.withLock
 import java.time.DayOfWeek
 import javax.inject.Inject
 import kotlin.time.Duration
@@ -91,6 +94,9 @@ class AlarmEditorViewModel @Inject internal constructor(
 
     fun onAlarmSetNoticeShown(notice: AlarmSetNotice) = announcer.consumed(notice)
 
+    /** Orders [scheduleWrite]s. */
+    private val scheduleWrites = Mutex()
+
     /** Memoised catalog answers, keyed on URI — see [metaOf]. */
     private val ringtoneMeta = mutableMapOf<String, RingtoneMeta>()
 
@@ -146,9 +152,9 @@ class AlarmEditorViewModel @Inject internal constructor(
     }
 
     fun onSkipNextChanged(skip: Boolean) {
-        // On the application scope, like every write that moves a fire time:
-        // see [edit].
-        appScope.launch {
+        // Absolute, so it must land in tap order against a day toggle that
+        // disarms the skip: see [scheduleWrite].
+        scheduleWrite {
             alarms.setSkipNextOccurrence(alarmId, skip)
             engine.reschedule()
             // Skipping moves the next ring a day or more out, which is exactly
@@ -238,7 +244,7 @@ class AlarmEditorViewModel @Inject internal constructor(
 
     /** Dismisses a ring first (M5 D11), deletes, and the state becomes [AlarmEditorUiState.Gone]. */
     fun onDelete() {
-        viewModelScope.launch {
+        scheduleWrite {
             if (engine.ringSession()?.alarm?.id == alarmId) engine.dismiss(alarmId)
             alarms.delete(alarmId)
             engine.reschedule()
@@ -281,13 +287,30 @@ class AlarmEditorViewModel @Inject internal constructor(
         settle: Duration = Duration.ZERO,
         transform: (Alarm) -> Alarm,
     ) {
-        (if (reschedule) appScope else viewModelScope).launch {
+        if (!reschedule) {
+            viewModelScope.launch { alarms.edit(alarmId, transform) }
+            return
+        }
+        scheduleWrite {
             alarms.edit(alarmId, transform)
-            if (reschedule) engine.reschedule()
+            engine.reschedule()
             if (announces) announcer.announce(alarmId, settle)
         }
     }
 
+    /**
+     * A write that moves a fire time, its reschedule and whatever follows, on
+     * the application scope (see [edit]) and **in tap order**: started
+     * undispatched, each takes its place in [scheduleWrites]' fair queue on the
+     * caller's thread, before the first suspension. The application scope is
+     * multi-threaded, so without it "skip next" followed at once by untapping
+     * the only repeat day could commit out of order, leaving a one-shot with a
+     * skip armed — which the resolver answers by disabling it (D14).
+     */
+    private fun scheduleWrite(block: suspend () -> Unit) {
+        appScope.launch(start = CoroutineStart.UNDISPATCHED) { scheduleWrites.withLock { block() } }
+    }
+
     private suspend fun render(inputs: Inputs): AlarmEditorUiState {
         val alarm = inputs.alarm ?: return AlarmEditorUiState.Gone
         return AlarmEditorUiState.Editing(

app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmSetSnackbar.kt

@@ -23,7 +23,8 @@ 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.combine
+import kotlinx.coroutines.flow.distinctUntilChanged
 import kotlinx.coroutines.flow.update
 import kotlinx.coroutines.launch
 import kotlinx.coroutines.sync.Mutex
@@ -84,14 +85,19 @@ internal class AlarmSetAnnouncer @Inject constructor(
     /**
      * 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 a while later
-     * would be wrong, and about something they did long ago. It is checked
-     * against the engine again on the way out, so an alarm switched off or
-     * deleted by any path — the ring screen, a notification, an intent, a
-     * restore — is never announced as set.
+     * would be wrong, and about something they did long ago.
+     *
+     * Live against the engine: every alarm write re-reads it, so an alarm
+     * switched off or deleted by any path — the ring screen, a notification,
+     * an intent, a restore — takes a snackbar down even while it is showing,
+     * and one whose ring moved (a skip armed from the notification) says the
+     * new countdown rather than the one it was announced with.
      */
-    val notice: Flow<AlarmSetNotice?> = current.map { notice ->
-        notice?.takeIf { isFresh(it) && stillRings(it) }
-    }
+    val notice: Flow<AlarmSetNotice?> = combine(current, engine.upcoming()) { notice, upcoming ->
+        if (notice == null || !isFresh(notice)) return@combine null
+        val entry = upcoming.firstOrNull { it.alarm.id == notice.alarmId }
+        AlarmSetCountdown.of(entry, wall.now())?.let { notice.copy(countdown = it) }
+    }.distinctUntilChanged()
 
     /**
      * Call once the write is stored. Announces [alarmId] after [settle], unless
@@ -124,9 +130,14 @@ internal class AlarmSetAnnouncer @Inject constructor(
         }
     }
 
-    /** [notice] was shown until it timed out or was swiped away. */
+    /**
+     * [notice] was shown until it timed out or was swiped away. Matched on the
+     * announcement, not the words: the countdown shown may be a re-read one.
+     */
     fun consumed(notice: AlarmSetNotice) {
-        current.compareAndSet(notice, null)
+        current.update { standing ->
+            standing?.takeUnless { it.alarmId == notice.alarmId && it.announcedAt == notice.announcedAt }
+        }
     }
 
     private suspend fun publish(alarmId: Long) = lock.withLock {
@@ -143,8 +154,6 @@ internal class AlarmSetAnnouncer @Inject constructor(
     private fun isFresh(notice: AlarmSetNotice): Boolean =
         elapsed.elapsedRealtime() - notice.announcedAt < FRESH_FOR
 
-    private suspend fun stillRings(notice: AlarmSetNotice): Boolean = countdownOf(notice.alarmId) != null
-
     private suspend fun countdownOf(alarmId: Long): NextFireLabel? {
         // 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.

app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorViewModelTest.kt

@@ -473,6 +473,38 @@ class AlarmEditorViewModelTest {
             assertThat(announcer.notice.first()).isNull()
         }
 
+    @Test
+    fun `a ring moved by another path is announced with its new countdown`(@TempDir tempDir: Path) =
+        runTest(main.dispatcher) {
+            val harness = alarmEngineHarness(tempDir, now = fiveToday)
+            harness.givenAlarmWith(hour = 7, repeatDays = RepeatDays.EVERY_DAY)
+            val viewModel = editor(harness)
+            advanceUntilIdle()
+            viewModel.onTimePicked(7, 0)
+            advanceUntilIdle()
+
+            // A skip armed from the upcoming-alarm notification: no retract.
+            harness.alarms.setSkipNextOccurrence(1L, true)
+
+            assertThat(viewModel.countdown()).isEqualTo(NextFireLabel.InDaysHours(1, 2))
+        }
+
+    @Test
+    fun `skip then untapping the only day lands in tap order`(@TempDir tempDir: Path) =
+        runTest(main.dispatcher) {
+            val harness = alarmEngineHarness(tempDir, now = fiveToday)
+            harness.givenAlarmWith(hour = 7, repeatDays = RepeatDays.of(DayOfWeek.MONDAY))
+            val viewModel = editor(harness)
+            advanceUntilIdle()
+
+            viewModel.onSkipNextChanged(true)
+            viewModel.onDayToggled(DayOfWeek.MONDAY)
+            advanceUntilIdle()
+
+            val stored = harness.alarmDao.stored.single()
+            assertThat(stored.enabled to stored.skipNextOccurrence).isEqualTo(true to false)
+        }
+
     @Test
     fun `an alarm switched off by another path is not announced as set`(@TempDir tempDir: Path) =
         runTest(main.dispatcher) {