Merge pull request 'fix(notifications): always serialize streams so clearing the last subscription saves' (#25) from fix/notifications-empty-subscriptions into main

Reviewed-on: #25
Reviewed-by: Colby Whitlock <whitlocktech@gmail.com>

app/src/main/java/com/runicgateway/app/data/api/dto/NotificationsDto.kt

@@ -60,8 +60,15 @@ data class NotificationStreamsDto(
  * `GET/PUT /auth/me/notifications/subscriptions` — the user's opted-in stream ids.
  * PUT replaces the full set; unknown ids are dropped server-side and the stored set
  * echoed back.
+ *
+ * [streams] intentionally has NO default: this DTO doubles as the PUT body, and the
+ * backend validator requires the `streams` field (`body('streams').isArray()`).
+ * kotlinx omits a property equal to its default (encodeDefaults=false), so a default
+ * of `emptyList()` would drop the field when the user clears their LAST subscription,
+ * sending `{}` → 400 "Validation failed" (the "can't turn off the last one" bug). With
+ * no default the empty list always serializes as `{"streams":[]}`. Do not re-add a default.
  */
 @Serializable
 data class NotificationSubscriptionsDto(
-    val streams: List<String> = emptyList(),
+    val streams: List<String>,
 )

app/src/main/java/com/runicgateway/app/ui/navigation/Menu.kt

@@ -21,7 +21,12 @@ enum class MenuAccess {
     /** Visible to any signed-in account (§5, "My Account"). */
     SIGNED_IN,
 
-    /** Visible only to a player — the linked game-data groups (§6.3). */
+    /**
+     * The linked game-data groups (§6.3). Visible to any player **or** staff:
+     * staff are a superset of players (all player abilities plus their staff
+     * tools), and the backend's player self-service surface is role-agnostic, so
+     * a signed-in admin/editor/moderator sees + uses their own characters too.
+     */
     PLAYER,
 
     /** Visible to any staff role (admin/editor/moderator) — the M10 staff surface (§1). */
@@ -70,7 +75,7 @@ fun visibleEntries(entries: List<MenuEntry>, session: Session): List<MenuEntry>
         when (entry.access) {
             MenuAccess.PUBLIC -> true
             MenuAccess.SIGNED_IN -> session is Session.SignedIn
-            MenuAccess.PLAYER -> session is Session.SignedIn && session.user.isPlayer
+            MenuAccess.PLAYER -> session is Session.SignedIn && (session.user.isPlayer || session.user.isStaff)
             MenuAccess.STAFF -> session is Session.SignedIn && session.user.isStaff
             MenuAccess.MODERATOR -> session is Session.SignedIn && session.user.isModerator
         }

app/src/test/java/com/runicgateway/app/data/api/dto/NotificationsDtoTest.kt

@@ -3,6 +3,7 @@
  */
 package com.runicgateway.app.data.api.dto
 
+import kotlinx.serialization.encodeToString
 import kotlinx.serialization.json.Json
 import org.junit.Assert.assertEquals
 import org.junit.Assert.assertFalse
@@ -58,6 +59,15 @@ class NotificationsDtoTest {
         assertEquals(listOf("news.post", "champ.start"), dto.streams)
     }
 
+    @Test fun emptySubscriptionsStillSerializeStreamsField() {
+        // Regression: clearing the LAST subscription sends an empty set. The backend
+        // validator requires `streams`, so it must be present as `[]`, not omitted.
+        // Uses the production Json config (no encodeDefaults) to prove the field is
+        // always emitted because the DTO field has no default.
+        val body = json.encodeToString(NotificationSubscriptionsDto(emptyList()))
+        assertEquals("""{"streams":[]}""", body)
+    }
+
     @Test fun settingsPushBlockDecodes() {
         val dto = json.decodeFromString<SettingsDto>(
             """{"site_title":"Shard","brand":{"name":"Shard"},"push":{"ntfyUrl":"https://ntfy.shard.tld"}}""",

app/src/test/java/com/runicgateway/app/ui/navigation/MenuAccessTest.kt

@@ -37,12 +37,16 @@ class MenuAccessTest {
         assertTrue(visible.contains(Routes.HOME))
     }
 
-    @Test fun staffSeeAccountButNoPlayerOnlyGroups() {
-        val visible = routes(signedIn(Role.EDITOR))
-        assertTrue(visible.contains(Routes.ACCOUNT))
-        // No PLAYER-access entry (the M4 game-data groups) leaks to staff.
-        val playerOnly = APP_MENU.filter { it.access == MenuAccess.PLAYER }.map { it.route }
-        assertTrue(playerOnly.none { visible.contains(it) })
+    @Test fun staffSeeThePlayerGameDataGroups() {
+        // Staff are a superset of players: every staff role sees the PLAYER-access
+        // game-data groups too (their own linked characters, via the role-agnostic
+        // /player self-service surface), on top of their staff entries.
+        val playerGroups = APP_MENU.filter { it.access == MenuAccess.PLAYER }.map { it.route }
+        for (role in listOf(Role.ADMIN, Role.EDITOR, Role.MODERATOR)) {
+            val visible = routes(signedIn(role))
+            assertTrue("$role should see Account", visible.contains(Routes.ACCOUNT))
+            assertTrue("$role should see the player game-data groups", playerGroups.all { visible.contains(it) })
+        }
     }
 
     @Test fun publicEntryCountIsStableAcrossSessions() {
@@ -58,10 +62,13 @@ class MenuAccessTest {
     }
 
     @Test fun playerAccessGatedFunction() {
-        // A synthetic PLAYER-gated entry is visible to a player, hidden from staff/anon.
+        // A PLAYER-gated entry is visible to a player AND to every staff role
+        // (staff superset), hidden only from an unrecognized role and anon.
         val entries = listOf(MenuEntry("game", 0, MenuAccess.PLAYER))
-        assertTrue(visibleEntries(entries, signedIn(Role.PLAYER)).isNotEmpty())
-        assertTrue(visibleEntries(entries, signedIn(Role.ADMIN)).isEmpty())
+        for (role in listOf(Role.PLAYER, Role.ADMIN, Role.EDITOR, Role.MODERATOR)) {
+            assertTrue("$role should see a PLAYER entry", visibleEntries(entries, signedIn(role)).isNotEmpty())
+        }
+        assertTrue(visibleEntries(entries, signedIn(Role.UNKNOWN)).isEmpty())
         assertTrue(visibleEntries(entries, Session.SignedOut).isEmpty())
     }