diff --git a/android/app/src/main/java/com/fabledsword/minstrel/notifications/ui/NotificationsScreen.kt b/android/app/src/main/java/com/fabledsword/minstrel/notifications/ui/NotificationsScreen.kt index dd933cd6..c3add71a 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/notifications/ui/NotificationsScreen.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/notifications/ui/NotificationsScreen.kt @@ -16,8 +16,9 @@ import androidx.compose.material3.HorizontalDivider import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Scaffold import androidx.compose.material3.Text -import androidx.compose.material3.TextButton import androidx.compose.runtime.Composable +import androidx.compose.runtime.DisposableEffect +import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.getValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier @@ -45,8 +46,9 @@ import kotlin.time.Duration.Companion.hours import kotlin.time.Duration.Companion.minutes /** - * The notifications inbox (M489): one row per notice, its title on one line - * and how long ago. Tapping a row marks it read and opens what it is about. + * The notifications inbox: one row per notice, its title on one line and how + * long ago. Showing the screen marks what it shows read; those rows still look + * new until the screen goes. Tapping a row opens what it is about. */ @Composable fun NotificationsScreen( @@ -54,7 +56,9 @@ fun NotificationsScreen( viewModel: NotificationsViewModel = hiltViewModel(), ) { val items by viewModel.items.collectAsStateWithLifecycle() - val anyUnread = items?.any { it.readAt == null } == true + val fresh by viewModel.fresh.collectAsStateWithLifecycle() + LaunchedEffect(items) { items?.let(viewModel::seen) } + DisposableEffect(Unit) { onDispose(viewModel::left) } Scaffold( contentWindowInsets = ShellContentWindowInsets, modifier = Modifier.fillMaxSize(), @@ -64,11 +68,6 @@ fun NotificationsScreen( navController = navController, currentRouteName = Notifications::class.qualifiedName, onBack = { navController.popBackStack() }, - actions = { - if (anyUnread) { - TextButton(onClick = viewModel::markAllRead) { Text("Mark all read") } - } - }, ) }, ) { inner -> @@ -88,6 +87,7 @@ fun NotificationsScreen( items(list, key = { it.id }) { item -> NotificationRow( item = item, + unread = item.readAt == null || item.id in fresh, onClick = { viewModel.open(item) routeForLink(item.link)?.let { navController.navigate(it) } @@ -102,15 +102,14 @@ fun NotificationsScreen( } @Composable -private fun NotificationRow(item: CachedNotificationEntity, onClick: () -> Unit) { - val unread = item.readAt == null +private fun NotificationRow(item: CachedNotificationEntity, unread: Boolean, onClick: () -> Unit) { Row( modifier = Modifier .fillMaxWidth() .clickable(onClick = onClick) .padding(horizontal = 16.dp, vertical = 14.dp) .semantics(mergeDescendants = true) { - if (unread) contentDescription = "Unread: ${item.title}" + if (unread) contentDescription = "New: ${item.title}" }, verticalAlignment = Alignment.CenterVertically, horizontalArrangement = Arrangement.spacedBy(12.dp), diff --git a/android/app/src/main/java/com/fabledsword/minstrel/notifications/ui/NotificationsViewModel.kt b/android/app/src/main/java/com/fabledsword/minstrel/notifications/ui/NotificationsViewModel.kt index d65e9c8f..9640bb07 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/notifications/ui/NotificationsViewModel.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/notifications/ui/NotificationsViewModel.kt @@ -8,16 +8,23 @@ import com.fabledsword.minstrel.connectivity.recoveries import com.fabledsword.minstrel.notifications.data.NotificationsRepository import dagger.hilt.android.lifecycle.HiltViewModel import kotlinx.coroutines.Job +import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.stateIn +import kotlinx.coroutines.flow.update import kotlinx.coroutines.launch import javax.inject.Inject private const val SHARE_STOP_TIMEOUT_MS = 5_000L -/** `items` is null until the cache has been read once, so the screen can tell "loading" from "empty". */ +/** + * `items` is null until the cache has been read once, so the screen can tell "loading" from "empty". + * Showing the inbox is reading it: what the screen shows is marked read at once, and stays in + * [fresh], so it still looks new while the screen is up. + */ @HiltViewModel class NotificationsViewModel @Inject constructor( private val repository: NotificationsRepository, @@ -27,6 +34,11 @@ class NotificationsViewModel @Inject constructor( .map, List?> { it } .stateIn(viewModelScope, SharingStarted.WhileSubscribed(SHARE_STOP_TIMEOUT_MS), null) + private val freshIds = MutableStateFlow>(emptySet()) + + /** Notices that were unread when the screen showed them. */ + val fresh: StateFlow> = freshIds.asStateFlow() + init { refresh() // Back online: the cached page may be stale (#1245's recovery idiom). @@ -43,7 +55,16 @@ class NotificationsViewModel @Inject constructor( viewModelScope.launch { repository.markRead(item.id) } } - fun markAllRead() { + /** The screen is showing [shown]: any unread among them are now read. */ + fun seen(shown: List) { + val unread = shown.filter { it.readAt == null && it.id !in freshIds.value } + if (unread.isEmpty()) return + freshIds.update { it + unread.map(CachedNotificationEntity::id) } viewModelScope.launch { repository.markAllRead() } } + + /** The screen has gone: next time, only what arrived since looks new. */ + fun left() { + freshIds.value = emptySet() + } } diff --git a/android/app/src/test/java/com/fabledsword/minstrel/notifications/ui/NotificationsViewModelTest.kt b/android/app/src/test/java/com/fabledsword/minstrel/notifications/ui/NotificationsViewModelTest.kt new file mode 100644 index 00000000..36be8d97 --- /dev/null +++ b/android/app/src/test/java/com/fabledsword/minstrel/notifications/ui/NotificationsViewModelTest.kt @@ -0,0 +1,80 @@ +package com.fabledsword.minstrel.notifications.ui + +import com.fabledsword.minstrel.cache.db.entities.CachedNotificationEntity +import com.fabledsword.minstrel.connectivity.NetworkStatusController +import com.fabledsword.minstrel.connectivity.ServerHealth +import com.fabledsword.minstrel.notifications.data.NotificationsRepository +import com.fabledsword.minstrel.testutil.MainDispatcherExtension +import io.mockk.coVerify +import io.mockk.every +import io.mockk.mockk +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.emptyFlow +import kotlinx.datetime.Instant +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.extension.ExtendWith +import kotlin.test.assertEquals + +/** Showing the inbox reads it: unread notices are marked read once, and keep looking new until the screen goes. */ +@ExtendWith(MainDispatcherExtension::class) +class NotificationsViewModelTest { + private val repository: NotificationsRepository = mockk(relaxed = true) { + every { notifications } returns emptyFlow() + } + private val network: NetworkStatusController = mockk { + every { state } returns MutableStateFlow(ServerHealth.Healthy) + } + private val viewModel = NotificationsViewModel(repository, network) + + private fun row(id: String, read: Boolean) = CachedNotificationEntity( + id = id, + kind = "request_completed", + title = "t", + body = "b", + link = "/requests", + createdAt = Instant.parse("2026-10-23T12:00:00Z"), + readAt = if (read) Instant.parse("2026-10-23T12:05:00Z") else null, + ) + + @Test + fun `showing unread notices marks them read and keeps them looking new`() { + viewModel.seen(listOf(row("a", read = false), row("b", read = true))) + + coVerify(exactly = 1) { repository.markAllRead() } + assertEquals(setOf("a"), viewModel.fresh.value) + } + + @Test + fun `the same notices shown again are not marked twice`() { + viewModel.seen(listOf(row("a", read = false))) + // The local mark lands and the cache re-emits; a re-emit before it lands repeats the row unread. + viewModel.seen(listOf(row("a", read = false))) + viewModel.seen(listOf(row("a", read = true))) + + coVerify(exactly = 1) { repository.markAllRead() } + } + + @Test + fun `a notice arriving while the screen is up is marked too`() { + viewModel.seen(listOf(row("a", read = false))) + viewModel.seen(listOf(row("new", read = false), row("a", read = true))) + + coVerify(exactly = 2) { repository.markAllRead() } + assertEquals(setOf("a", "new"), viewModel.fresh.value) + } + + @Test + fun `nothing unread marks nothing`() { + viewModel.seen(listOf(row("a", read = true))) + + coVerify(exactly = 0) { repository.markAllRead() } + } + + @Test + fun `leaving the screen stops them looking new`() { + viewModel.seen(listOf(row("a", read = false))) + viewModel.left() + + assertEquals(emptySet(), viewModel.fresh.value) + } +} diff --git a/web/src/lib/api/notifications.ts b/web/src/lib/api/notifications.ts index 75eefd18..1b283086 100644 --- a/web/src/lib/api/notifications.ts +++ b/web/src/lib/api/notifications.ts @@ -44,8 +44,19 @@ export async function markNotificationRead(id: string): Promise { await api.post(`/api/me/notifications/${encodeURIComponent(id)}/read`, {}); } -export async function markAllNotificationsRead(): Promise { - await api.post('/api/me/notifications/read-all', {}); +/** upTo limits it to notices created at or before then, so one the inbox never showed stays unread. */ +export async function markAllNotificationsRead(upTo?: string): Promise { + await api.post('/api/me/notifications/read-all', upTo ? { up_to: upTo } : {}); +} + +/** + * The read-all cutoff for what the inbox shows: the newest notice, plus a + * millisecond. The server keeps microseconds and Date only milliseconds, so + * without the rounding up the newest notice would stay unread. + */ +export function readAllCutoff(items: NotificationItem[]): string | undefined { + const newest = Math.max(...items.map((i) => Date.parse(i.created_at)).filter((t) => !Number.isNaN(t))); + return Number.isFinite(newest) ? new Date(newest + 1).toISOString() : undefined; } export function createNotificationsQuery() { diff --git a/web/src/lib/components/NotificationBell.svelte b/web/src/lib/components/NotificationBell.svelte index f7899fa0..3a0be71b 100644 --- a/web/src/lib/components/NotificationBell.svelte +++ b/web/src/lib/components/NotificationBell.svelte @@ -1,4 +1,6 @@ e.key === 'Escape' && (open = false)} /> @@ -96,17 +118,8 @@ role="dialog" aria-label="Notifications" > -
+
Notifications - {#if unread > 0} - - {/if}
{#if items.length === 0} @@ -118,20 +131,20 @@