diff --git a/app/src/main/java/com/runicgateway/app/ui/notifications/InboxViewModel.kt b/app/src/main/java/com/runicgateway/app/ui/notifications/InboxViewModel.kt index a8e7d49..6881f1f 100644 --- a/app/src/main/java/com/runicgateway/app/ui/notifications/InboxViewModel.kt +++ b/app/src/main/java/com/runicgateway/app/ui/notifications/InboxViewModel.kt @@ -20,6 +20,8 @@ import dagger.hilt.android.lifecycle.HiltViewModel import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow +import kotlinx.coroutines.flow.distinctUntilChanged +import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.update import kotlinx.coroutines.launch import javax.inject.Inject @@ -39,6 +41,19 @@ import javax.inject.Inject * **Paging is keyset, not offset.** The next page is `before = the last id on * screen`, because the list gains rows at the top while it is being read and an * offset would show the same item twice or skip one. + * + * **It reloads when the ACCOUNT changes, not merely when it is created.** A + * drawer route's view model outlives a sign-out: `navigateTopLevel` saves and + * restores back-stack state, so the `NavBackStackEntry` keeps its + * `ViewModelStore` and a view model that loaded only in `init` never runs again. + * Signing out and back in as somebody else showed the second account the FIRST + * account's inbox — titles and body text written for another person — with no + * request made at all, while the badge beside it showed the new account's real + * count, because the shell refreshes that one on every session change. + * + * [InboxCache] was never the hole: it is keyed by `(base URL, user id)` and a + * snapshot has never crossed an account. The hole was the in-memory state, which + * nothing invalidated. */ @HiltViewModel class InboxViewModel @Inject constructor( @@ -68,7 +83,25 @@ class InboxViewModel @Inject constructor( val state: StateFlow = _state.asStateFlow() init { - load() + viewModelScope.launch { + sessionManager.state + .map { (it as? Session.SignedIn)?.user?.id } + .distinctUntilChanged() + .collect { userId -> + if (userId == null) { + // Signed out. The shell is already navigating away; drop the + // rows rather than leave them addressable behind it. + _state.value = State(items = UiState.Success(emptyList())) + } else { + // Reset BEFORE loading, not after: `load()` paints the cache + // only when there is no `Success` on screen, so the previous + // account's rows would otherwise stay up — and stay up for + // the whole round trip. + _state.value = State() + load() + } + } + } } /** diff --git a/app/src/main/java/com/runicgateway/app/ui/notifications/NotificationSettingsViewModel.kt b/app/src/main/java/com/runicgateway/app/ui/notifications/NotificationSettingsViewModel.kt index c522809..b4dc15a 100644 --- a/app/src/main/java/com/runicgateway/app/ui/notifications/NotificationSettingsViewModel.kt +++ b/app/src/main/java/com/runicgateway/app/ui/notifications/NotificationSettingsViewModel.kt @@ -7,6 +7,8 @@ import androidx.annotation.StringRes import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope import com.runicgateway.app.R +import com.runicgateway.app.core.auth.Session +import com.runicgateway.app.core.auth.SessionManager import com.runicgateway.app.core.push.PushManager import com.runicgateway.app.core.result.ApiResult import com.runicgateway.app.data.api.dto.NotificationChannelItemDto @@ -19,6 +21,8 @@ import dagger.hilt.android.lifecycle.HiltViewModel import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow +import kotlinx.coroutines.flow.distinctUntilChanged +import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.update import kotlinx.coroutines.launch import javax.inject.Inject @@ -51,6 +55,7 @@ class NotificationSettingsViewModel @Inject constructor( private val notifications: NotificationsRepository, private val playerShard: PlayerShardRepository, private val pushManager: PushManager, + sessionManager: SessionManager, ) : ViewModel() { data class Feedback(val ok: Boolean, @param:StringRes val messageRes: Int) @@ -72,7 +77,17 @@ class NotificationSettingsViewModel @Inject constructor( viewModelScope.launch { pushManager.supported.collect { supported -> _state.update { it.copy(supported = supported) } } } - load() + // Reloaded on an account change for the reason the inbox is, and one + // reason more: these controls are WRITTEN from. A screen still rendering + // the previous account's preferences would send this account's PUT built + // out of them, so a stale render here corrupts rather than merely + // discloses. + viewModelScope.launch { + sessionManager.state + .map { (it as? Session.SignedIn)?.user?.id } + .distinctUntilChanged() + .collect { userId -> if (userId != null) load() } + } } fun load() { diff --git a/app/src/test/java/com/runicgateway/app/ui/notifications/InboxViewModelTest.kt b/app/src/test/java/com/runicgateway/app/ui/notifications/InboxViewModelTest.kt index 2e6c461..2ec5b24 100644 --- a/app/src/test/java/com/runicgateway/app/ui/notifications/InboxViewModelTest.kt +++ b/app/src/test/java/com/runicgateway/app/ui/notifications/InboxViewModelTest.kt @@ -11,6 +11,7 @@ import com.runicgateway.app.core.net.BaseUrlHolder import com.runicgateway.app.data.api.dto.NotificationInboxDto import com.runicgateway.app.data.api.dto.NotificationItemDto import com.runicgateway.app.data.api.dto.NotificationReadResultDto +import com.runicgateway.app.data.api.dto.SafeUserDto import com.runicgateway.app.data.api.fake.FakeNotificationsApi import com.runicgateway.app.data.repository.NotificationsRepository import com.runicgateway.app.ui.UiState @@ -194,6 +195,45 @@ class InboxViewModelTest { assertNull(vm.linkFor(item(1).copy(url = "intent://evil#Intent;end"))) } + // ── The inbox belongs to ONE account ───────────────────────────── + + @Test fun switchingAccountDoesNotShowThePreviousOnesInbox() { + // **Found on the emulator, not by a test.** A drawer route's view model + // outlives a sign-out: `navigateTopLevel` saves and restores back-stack + // state, so the entry keeps its ViewModelStore and a view model that + // loaded only in `init` never runs again. Signing out and back in as + // somebody else showed the second account the FIRST account's inbox — + // titles and body text written for another person — with no request made + // at all, while the badge beside it showed the new account's real count. + val sessions = session(userId = 7) + api.pages = mapOf(null to NotificationInboxDto(items = listOf(item(1), item(2)), unread = 2)) + val vm = InboxViewModel(NotificationsRepository(api), cache, sessions, baseUrl) + assertEquals(listOf(1L, 2L), shown(vm)!!.map { it.id }) + + api.pages = mapOf(null to NotificationInboxDto(items = listOf(item(9)), unread = 1)) + sessions.onSignedOut() + // Signed out, the previous account's rows are gone rather than left + // addressable behind a shell that is navigating away. + assertEquals(emptyList(), shown(vm)!!.map { it.id }) + + sessions.onSignedIn("a", "r", SafeUserDto(id = 8, username = "bob", role = "player")) + assertEquals(listOf(9L), shown(vm)!!.map { it.id }) + } + + @Test fun aResumeRevalidationReturningTheSameUserDoesNotRefetch() { + // The gate is the account, not every session emission — the app + // re-validates its role on every resume. + val sessions = session(userId = 7) + api.pages = mapOf(null to NotificationInboxDto(items = listOf(item(1)), unread = 1)) + val vm = InboxViewModel(NotificationsRepository(api), cache, sessions, baseUrl) + val callsAfterFirstLoad = api.inboxCalls.size + + sessions.onUserRefreshed(SafeUserDto(id = 7, username = "alice", role = "admin")) + + assertEquals(callsAfterFirstLoad, api.inboxCalls.size) + assertEquals(listOf(1L), shown(vm)!!.map { it.id }) + } + // ── Opening an item in the app rather than a browser (M13) ───────── @Test fun anEventAnnouncementOpensNativelyAndKeepsItsRun() {