Merge pull request 'fix(notifications): reload the inbox and its settings when the account changes' (#45) from fix/inbox-session-scope into edge

Reviewed-on: #45

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> = _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()
+                    }
+                }
+        }
     }
 
     /**

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() {

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<Long>(), 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() {