From 640b423dfaabd0fe79ea637b32c36226e757bc8b Mon Sep 17 00:00:00 2001 From: wtclaude Date: Wed, 23 Sep 2026 15:26:32 -0500 Subject: [PATCH] feat(notifications): a Rust notification on a phone (module-rust phase 11) Three places the app still assumed its site was a UO shard (D69-D71): - The linked-account check behind personal streams asked module-uo's /player/shard/accounts. On a Rust site that failed, and the whole raid alert row was disabled in every channel. It now asks the site's own module by capability, and the link only holds back switching push ON; switching anything off is never refused. - A tickle whose ref names an inbox row is titled from that row, pulled over the authenticated inbox API (wake-and-pull). The lock screen shows only the per-stream title. Each tickle runs in its own job, because collectLatest would cancel a pull on the next tickle. - /player/rust and /rust/servers/?tab= open natively; a tab the app does not have (clans) still hands off to the browser. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01E14m6SuuY6i1vASFeGDBeY --- .../runicgateway/app/core/push/PushContent.kt | 77 +++++++++++++ .../app/core/push/PushNotifier.kt | 59 ++++++++-- .../runicgateway/app/core/push/PushService.kt | 13 ++- .../repository/LinkedAccountRepository.kt | 56 ++++++++++ .../java/com/runicgateway/app/ui/RunicApp.kt | 9 +- .../app/ui/navigation/NavPaths.kt | 29 ++++- .../runicgateway/app/ui/navigation/Routes.kt | 12 ++- .../NotificationSettingsScreen.kt | 35 +++--- .../NotificationSettingsViewModel.kt | 43 ++++++-- .../app/ui/rust/RustServerViewModel.kt | 19 +++- app/src/main/res/values/strings.xml | 2 +- .../app/core/push/PushContentTest.kt | 101 ++++++++++++++++++ .../repository/LinkedAccountRepositoryTest.kt | 91 ++++++++++++++++ .../app/ui/navigation/RustNavigationTest.kt | 35 ++++++ .../notifications/NotificationRoutingTest.kt | 25 +++-- .../NotificationSettingsViewModelTest.kt | 32 +++++- 16 files changed, 579 insertions(+), 59 deletions(-) create mode 100644 app/src/main/java/com/runicgateway/app/core/push/PushContent.kt create mode 100644 app/src/main/java/com/runicgateway/app/data/repository/LinkedAccountRepository.kt create mode 100644 app/src/test/java/com/runicgateway/app/core/push/PushContentTest.kt create mode 100644 app/src/test/java/com/runicgateway/app/data/repository/LinkedAccountRepositoryTest.kt diff --git a/app/src/main/java/com/runicgateway/app/core/push/PushContent.kt b/app/src/main/java/com/runicgateway/app/core/push/PushContent.kt new file mode 100644 index 0000000..c03ccc4 --- /dev/null +++ b/app/src/main/java/com/runicgateway/app/core/push/PushContent.kt @@ -0,0 +1,77 @@ +/* + * SPDX-License-Identifier: GPL-3.0-or-later + */ +package com.runicgateway.app.core.push + +import com.runicgateway.app.core.result.ApiResult +import com.runicgateway.app.data.api.dto.NotificationItemDto +import com.runicgateway.app.data.repository.NotificationsRepository +import com.runicgateway.app.ui.navigation.Routes +import kotlinx.coroutines.withTimeoutOrNull +import javax.inject.Inject +import javax.inject.Singleton + +/** + * The inbox id a tickle's `notification:` ref names, or null. + * + * Prefix-exact, the same rule [Routes.forTickle] reads the ref by, and digits + * only. The relay is untrusted, so a ref is a hint, and anything that is not + * exactly a positive id is treated as no hint at all. + */ +internal fun inboxIdFromRef(ref: String?): Long? { + if (ref == null || !ref.startsWith(Routes.INBOX_REF_PREFIX)) return null + val digits = ref.substring(Routes.INBOX_REF_PREFIX.length) + if (digits.isEmpty() || digits.length > MAX_ID_DIGITS || !digits.all { it in '0'..'9' }) return null + return digits.toLongOrNull()?.takeIf { it > 0 } +} + +/** + * The item from [page] that [tickle] is about, or null. + * + * The item must be the one the ref names **and** come from the trigger the tickle + * names. Only the user's own inbox can be read, so a forged pair could only put + * one of their own items on their own phone. A tickle that disagrees with the row + * it points at is still not one to title from. + */ +internal fun itemForTickle(tickle: PushTickle, page: List): NotificationItemDto? { + val id = inboxIdFromRef(tickle.ref) ?: return null + return page.firstOrNull { it.id == id && it.triggerId == tickle.stream } +} + +/** + * Pulls the inbox row a tickle points at, so the notification can say what + * happened (`docs/modules/rust/PLAN.md` D70). + * + * This is the wake-and-pull contract core's `pushChannel.js` describes: the relay + * carries `{ stream, ref }` and nothing else, and the content comes over the + * authenticated, ownership-checked inbox API. Core has no single-item read, so + * this reads the first page. The row was written moments ago, so it is on that + * page, and if it is not the caller falls back to the per-stream title. + * + * **Every failure is null**: no ref, a signed-out app, a dead network, a timeout, + * an item not on the page. A GET of the inbox marks nothing read, so the badge + * is left alone. + */ +@Singleton +class PushContentResolver @Inject constructor( + private val notifications: NotificationsRepository, +) { + suspend fun itemFor(tickle: PushTickle): NotificationItemDto? { + if (inboxIdFromRef(tickle.ref) == null) return null + val page = withTimeoutOrNull(PULL_TIMEOUT_MS) { + (notifications.inbox() as? ApiResult.Ok)?.data?.items + } ?: return null + return itemForTickle(tickle, page) + } + + private companion object { + /** + * Long enough for a phone waking on a slow network, and short enough that a + * notification is never held back noticeably for a title. + */ + const val PULL_TIMEOUT_MS = 5_000L + } +} + +/** A `BIGINT UNSIGNED` never runs past 20 digits. */ +private const val MAX_ID_DIGITS = 20 diff --git a/app/src/main/java/com/runicgateway/app/core/push/PushNotifier.kt b/app/src/main/java/com/runicgateway/app/core/push/PushNotifier.kt index 6e1acf3..c5760ff 100644 --- a/app/src/main/java/com/runicgateway/app/core/push/PushNotifier.kt +++ b/app/src/main/java/com/runicgateway/app/core/push/PushNotifier.kt @@ -13,6 +13,7 @@ import androidx.core.app.NotificationCompat import androidx.core.app.NotificationManagerCompat import com.runicgateway.app.MainActivity import com.runicgateway.app.R +import com.runicgateway.app.data.api.dto.NotificationItemDto import dagger.hilt.android.qualifiers.ApplicationContext import java.util.concurrent.atomic.AtomicInteger import javax.inject.Inject @@ -20,10 +21,18 @@ import javax.inject.Singleton /** * Builds the notification channels and posts a notification for a received tickle - * (PLAN.md §11, M7 Part 2 work items 2/3/7). v1 shows a **generic per-stream** - * notification titled from the fixed [PushStreams] catalog — the content-free tickle - * carries nothing to render, so nothing is fetched to display the notification; tapping - * deep-links into [MainActivity] (which fetches fresh over the authenticated API). + * (PLAN.md §11, M7 Part 2 work items 2/3/7). Tapping deep-links into [MainActivity], + * which fetches fresh over the authenticated API. + * + * **The title comes from the inbox row when there is one** (`docs/modules/rust/PLAN.md` + * D70). The tickle carries nothing to render. When its ref names an inbox row, + * [PushContentResolver] has already pulled that row over the authenticated API, and + * the notification says what the row says. Every other tickle keeps the M7 behaviour: + * a **generic per-stream** title from the fixed [PushStreams] catalog. Until D70 that + * was every tickle, and "New notification" is all a Rust raid alert ever said. + * + * **The lock screen shows only the generic title.** The row's text is content, and + * a locked phone on a table is not the place for "a door was destroyed in S16". */ @Singleton class PushNotifier @Inject constructor( @@ -65,17 +74,32 @@ class PushNotifier @Inject constructor( .setContentIntent(deepLinkIntent(stream = null, ref = null)) .build() - /** Post a notification for a tickle, deep-linking to the stream's screen on tap. */ - fun notify(tickle: PushTickle) { + /** + * Post a notification for a tickle, deep-linking to the stream's screen on tap. + * + * [item] is the inbox row the tickle points at, when it could be pulled; null + * falls back to the per-stream title. + */ + fun notify(tickle: PushTickle, item: NotificationItemDto? = null) { if (!manager.areNotificationsEnabled()) return // POST_NOTIFICATIONS not granted - val title = context.getString(PushStreams.titleRes(tickle.stream)) - val notification = NotificationCompat.Builder(context, CHANNEL_MESSAGES) - .setContentTitle(title) + val generic = context.getString(PushStreams.titleRes(tickle.stream)) + val content = notificationText(item, generic) + val redacted = NotificationCompat.Builder(context, CHANNEL_MESSAGES) + .setContentTitle(generic) + .setSmallIcon(R.drawable.ic_stat_name) + .build() + val builder = NotificationCompat.Builder(context, CHANNEL_MESSAGES) + .setContentTitle(content.title) .setSmallIcon(R.drawable.ic_stat_name) .setAutoCancel(true) .setPriority(NotificationCompat.PRIORITY_DEFAULT) + .setVisibility(NotificationCompat.VISIBILITY_PRIVATE) + .setPublicVersion(redacted) .setContentIntent(deepLinkIntent(tickle.stream, tickle.ref)) - .build() + content.body?.let { body -> + builder.setContentText(body).setStyle(NotificationCompat.BigTextStyle().bigText(body)) + } + val notification = builder.build() try { manager.notify(nextId.getAndIncrement(), notification) } catch (_: SecurityException) { @@ -108,3 +132,18 @@ class PushNotifier @Inject constructor( const val EXTRA_REF = "com.runicgateway.app.push.REF" } } + +/** What a posted notification says: a title always, and a body when the row had one. */ +internal data class NotificationText(val title: String, val body: String?) + +/** + * The text for a notification about [item], falling back to [generic] (D70). + * + * A row with a blank title is treated as no row: a notification with an empty + * headline is worse than one that says only that something happened. + */ +internal fun notificationText(item: NotificationItemDto?, generic: String): NotificationText { + val title = item?.title?.trim().orEmpty() + if (title.isEmpty()) return NotificationText(generic, null) + return NotificationText(title, item?.body?.trim()?.takeIf { it.isNotEmpty() }) +} diff --git a/app/src/main/java/com/runicgateway/app/core/push/PushService.kt b/app/src/main/java/com/runicgateway/app/core/push/PushService.kt index 71e7607..dc8aa7e 100644 --- a/app/src/main/java/com/runicgateway/app/core/push/PushService.kt +++ b/app/src/main/java/com/runicgateway/app/core/push/PushService.kt @@ -16,7 +16,6 @@ import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.Job import kotlinx.coroutines.SupervisorJob import kotlinx.coroutines.cancel -import kotlinx.coroutines.flow.collectLatest import kotlinx.coroutines.launch import javax.inject.Inject @@ -33,6 +32,7 @@ class PushService : Service() { @Inject lateinit var streamClient: NtfyStreamClient @Inject lateinit var notifier: PushNotifier + @Inject lateinit var content: PushContentResolver @Inject lateinit var prefs: PushPreferences private val scope = CoroutineScope(SupervisorJob() + Dispatchers.IO) @@ -67,8 +67,15 @@ class PushService : Service() { stopSelf() return } - streamClient.events(snapshot.ntfyUrl, topic).collectLatest { event -> - if (event is NtfyStreamClient.Event.Message) notifier.notify(event.tickle) + // Each tickle in its own job (D70). Titling one from its inbox row + // suspends on a network read, and inside `collectLatest` the next tickle + // would cancel it. A burst (two owners' raid alerts, then a restart) would + // then post only the last. + streamClient.events(snapshot.ntfyUrl, topic).collect { event -> + if (event is NtfyStreamClient.Event.Message) { + val tickle = event.tickle + scope.launch { notifier.notify(tickle, content.itemFor(tickle)) } + } } } diff --git a/app/src/main/java/com/runicgateway/app/data/repository/LinkedAccountRepository.kt b/app/src/main/java/com/runicgateway/app/data/repository/LinkedAccountRepository.kt new file mode 100644 index 0000000..0d202eb --- /dev/null +++ b/app/src/main/java/com/runicgateway/app/data/repository/LinkedAccountRepository.kt @@ -0,0 +1,56 @@ +/* + * SPDX-License-Identifier: GPL-3.0-or-later + */ +package com.runicgateway.app.data.repository + +import com.runicgateway.app.core.result.ApiResult +import javax.inject.Inject +import javax.inject.Singleton + +/** A game module whose player surface can say whether this user has linked an account. */ +enum class LinkSource { RUST, SHARD } + +/** + * Which module's link read answers for this site (`docs/modules/rust/PLAN.md` D69). + * + * **Read off the capabilities, never assumed.** Before phase 11 the answer was + * always `module-uo`'s `/player/shard/accounts`, which does not exist on a Rust + * site. That call failed, the failure read as "not linked", and the raid alert + * was locked in every channel. + * + * A host that has **never** answered asks both, and a failed read is simply no + * link. A site runs one module (§24.5), so at most one of the two answers. That + * is the same fail-open direction [canUse] takes. A host that answered and named + * neither module has no link to ask about. + */ +fun linkSourcesFor(capabilities: SiteCapabilities?): List { + if (capabilities == null) return LinkSource.entries + return buildList { + if (Capability.RUST in capabilities) add(LinkSource.RUST) + if (Capability.SHARD in capabilities) add(LinkSource.SHARD) + } +} + +/** + * Whether the signed-in user holds at least one linked game account on this site. + * + * It exists for the settings screen's one courtesy gate: a `requiresLinkedAccount` + * stream cannot have push switched **on** without a link. It is a courtesy and not + * a boundary. The server enforces no such flag, and who is actually alerted is the + * module's recipient computation (Rust D59), so this never needs to be more than + * a best guess. + */ +@Singleton +class LinkedAccountRepository @Inject constructor( + private val capabilities: SiteCapabilitiesRepository, + private val rust: PlayerRustRepository, + private val shard: PlayerShardRepository, +) { + suspend fun hasLinkedAccount(): Boolean = + linkSourcesFor(capabilities.capabilities.value).any { source -> + when (source) { + LinkSource.RUST -> (rust.links() as? ApiResult.Ok)?.data?.isNotEmpty() == true + LinkSource.SHARD -> (shard.accounts() as? ApiResult.Ok)?.data?.isNotEmpty() == true + } + } +} diff --git a/app/src/main/java/com/runicgateway/app/ui/RunicApp.kt b/app/src/main/java/com/runicgateway/app/ui/RunicApp.kt index 4ffa73e..78d39cd 100644 --- a/app/src/main/java/com/runicgateway/app/ui/RunicApp.kt +++ b/app/src/main/java/com/runicgateway/app/ui/RunicApp.kt @@ -635,7 +635,14 @@ private fun RunicNavHost( } composable( route = Routes.RUST_SERVER, - arguments = listOf(navArgument(Routes.Args.SERVER_ID) { type = NavType.StringType }), + arguments = listOf( + navArgument(Routes.Args.SERVER_ID) { type = NavType.StringType }, + navArgument(Routes.Args.TAB) { + type = NavType.StringType + nullable = true + defaultValue = null + }, + ), ) { RustServerScreen(onBack = { navController.navigateTopLevel(Routes.RUST) }) } diff --git a/app/src/main/java/com/runicgateway/app/ui/navigation/NavPaths.kt b/app/src/main/java/com/runicgateway/app/ui/navigation/NavPaths.kt index b9da697..d993163 100644 --- a/app/src/main/java/com/runicgateway/app/ui/navigation/NavPaths.kt +++ b/app/src/main/java/com/runicgateway/app/ui/navigation/NavPaths.kt @@ -4,6 +4,7 @@ package com.runicgateway.app.ui.navigation import com.runicgateway.app.data.repository.ContentRepository.PostCategory +import com.runicgateway.app.ui.rust.RustTab /** * The website path → app route table (THEMING_AND_NAV.md §6.2). @@ -257,7 +258,8 @@ private val RESERVED_TOP_LEVEL = setOf( * /uo/atlas/ → ATLAS_CREATURE * /uo/market/vendors/ → SHARD_MARKET_VENDOR * /rust → RUST (module-rust's server list) - * /rust/servers/ → RUST_SERVER + * /rust/servers/[?tab=] → RUST_SERVER (a tab the app has; Rust D71) + * /player/rust → PLAYER_RUST (Rust D71) * /site/about → PAGE("about") * / → PAGE(slug), unless is reserved * anything else → null, i.e. the Custom Tab @@ -272,6 +274,12 @@ private val RESERVED_TOP_LEVEL = setOf( * string, any second parameter, and any fragment still hand off** — the carve-out * is one key on one path, not a general "parse the query". * + * **Rust D71 adds the second, on the same terms:** `tab` on a Rust server's path, + * which is what the *new leader* notice links to. It resolves only when the value + * is a tab the app has. `?tab=clans` still hands off, because the website has a + * Clans tab and the app does not, and opening the feed instead would be the quiet + * drop this rule exists to prevent. + * * That narrowness is the point: an admin who writes `/site/events/x?utm=mail` gets * the browser, which honors `utm`, rather than an app screen that silently ignored * it. @@ -309,6 +317,10 @@ fun resolveWebPath(path: String?): String? { val run = runParam(query) ?: return null return Routes.event(segments[2], run) } + if (segments.size == 3 && segments[0] == MODULE_RUST && segments[1] == "servers" && query.isNotEmpty()) { + val tab = rustTabParam(query) ?: return null + return Routes.rustServer(segments[2], tab) + } if (query.isNotEmpty()) return null return when { @@ -326,6 +338,10 @@ fun resolveWebPath(path: String?): String? { // answered by the nav table above, before this fallback is reached. segments[0] == MODULE_RUST && segments.size == 3 && segments[1] == "servers" -> Routes.rustServer(segments[2]) + // The player's own Rust account, which the *account linked* notice links + // to (D71). Not in the nav table above, which is built from the PUBLIC + // nav; a signed-out tap is caught by the player gate like any other. + segments.size == 2 && segments[0] == "player" && segments[1] == MODULE_RUST -> Routes.PLAYER_RUST else -> null } } @@ -346,6 +362,17 @@ private fun runParam(query: String): String? { return value.takeIf { '&' !in it && '=' !in it } } +/** + * The tab a query of **exactly** `tab=` names, when the app has that + * tab (Rust D71), or null, which hands the link to the browser. Same shape as + * [runParam], and for the same reasons. + */ +private fun rustTabParam(query: String): RustTab? { + val value = query.removePrefix("tab=") + if (value.length == query.length || value.isEmpty() || '&' in value || '=' in value) return null + return RustTab.fromWire(value) +} + /** * The module id whose public pages this table maps. * diff --git a/app/src/main/java/com/runicgateway/app/ui/navigation/Routes.kt b/app/src/main/java/com/runicgateway/app/ui/navigation/Routes.kt index 2654f47..68b3f9c 100644 --- a/app/src/main/java/com/runicgateway/app/ui/navigation/Routes.kt +++ b/app/src/main/java/com/runicgateway/app/ui/navigation/Routes.kt @@ -4,6 +4,7 @@ package com.runicgateway.app.ui.navigation import com.runicgateway.app.data.repository.ContentRepository +import com.runicgateway.app.ui.rust.RustTab /** * Navigation destinations for the M1 public surface (PLAN.md §5). Routes are @@ -84,7 +85,7 @@ object Routes { * two can be installed on the same backend, and then both trees exist at once. */ const val RUST = "rust" - const val RUST_SERVER = "rust/servers/{serverId}" + const val RUST_SERVER = "rust/servers/{serverId}?tab={tab}" /** * The player's own Rust account (§9 M15) — the Steam accounts they hold and @@ -149,6 +150,7 @@ object Routes { const val SERIAL = "serial" const val RUN = "run" const val SERVER_ID = "serverId" + const val TAB = "tab" } fun page(slug: String) = "page/$slug" @@ -179,8 +181,14 @@ object Routes { * JVM unit test and throws "not mocked" — this object is pure and every test * that builds a route would have to become an instrumented one to keep it * that way. + * + * [tab] is what the *new leader* notice's link carries (Rust D71), and it is + * dropped when absent, as `run` is on [event]. */ - fun rustServer(id: String) = "rust/servers/${encodePathSegment(id)}" + fun rustServer(id: String, tab: RustTab? = null): String { + val base = "rust/servers/${encodePathSegment(id)}" + return if (tab == null) base else "$base?tab=${tab.wire}" + } /** * Percent-encode one path segment, allowing only the unreserved set. diff --git a/app/src/main/java/com/runicgateway/app/ui/notifications/NotificationSettingsScreen.kt b/app/src/main/java/com/runicgateway/app/ui/notifications/NotificationSettingsScreen.kt index 0a1faf8..5d01498 100644 --- a/app/src/main/java/com/runicgateway/app/ui/notifications/NotificationSettingsScreen.kt +++ b/app/src/main/java/com/runicgateway/app/ui/notifications/NotificationSettingsScreen.kt @@ -153,7 +153,7 @@ private fun ChannelPrefsList( SectionLabel(stringResource(R.string.notifications_section_general)) Spacer(Modifier.height(8.dp)) general.forEach { item -> - ItemRow(item, channelsById, hint = null, enabled = !busy, pushSupported = pushSupported, onSetMode = onSetMode) + ItemRow(item, channelsById, hasLinkedAccount, busy, pushSupported, onSetMode) HorizontalDivider() } Spacer(Modifier.height(20.dp)) @@ -163,15 +163,7 @@ private fun ChannelPrefsList( SectionLabel(stringResource(R.string.notifications_section_personal)) Spacer(Modifier.height(8.dp)) personal.forEach { item -> - val selectable = itemSelectable(item, hasLinkedAccount) - ItemRow( - item = item, - channelsById = channelsById, - hint = if (!selectable) stringResource(R.string.notifications_requires_link) else null, - enabled = !busy && selectable, - pushSupported = pushSupported, - onSetMode = onSetMode, - ) + ItemRow(item, channelsById, hasLinkedAccount, busy, pushSupported, onSetMode) HorizontalDivider() } } @@ -181,33 +173,40 @@ private fun ChannelPrefsList( private fun ItemRow( item: NotificationChannelItemDto, channelsById: Map, - hint: String?, - enabled: Boolean, + hasLinkedAccount: Boolean, + busy: Boolean, pushSupported: Boolean, onSetMode: (NotificationChannelItemDto, String, String) -> Unit, ) { + // The link holds back one control, push switching on, so the row stays live + // and the hint names that one control (Rust D69). Where this device has no + // push at all, there is no control to hold back and nothing to explain. + val pushHeld = pushSupported && pushNeedsLink(item, hasLinkedAccount) Column(Modifier.fillMaxWidth().padding(vertical = 12.dp)) { Text( text = item.label, style = MaterialTheme.typography.bodyLarge, - color = if (enabled) MaterialTheme.colorScheme.onSurface else MaterialTheme.colorScheme.onSurfaceVariant, + color = MaterialTheme.colorScheme.onSurface, ) Text( - text = hint ?: item.description, + text = if (pushHeld) stringResource(R.string.notifications_requires_link) else item.description, style = MaterialTheme.typography.bodySmall, color = MaterialTheme.colorScheme.onSurfaceVariant, - fontStyle = if (hint != null) FontStyle.Italic else FontStyle.Normal, + fontStyle = if (pushHeld) FontStyle.Italic else FontStyle.Normal, ) // The item's OWN channel list, in the registry's order. An id nothing can // push carries no push control at all, rather than a dead switch. item.channels.forEach { channelId -> val channel = channelsById[channelId] ?: return@forEach if (channelId == CHANNEL_PUSH && !pushSupported) return@forEach + val mode = item.modes[channelId] ?: channel.defaultMode ChannelControl( channel = channel, - mode = item.modes[channelId] ?: channel.defaultMode, - enabled = enabled, - onSetMode = { mode -> onSetMode(item, channelId, mode) }, + mode = mode, + // A held push switch that is already ON stays enabled, because + // switching it off is never refused ([canSetMode]). + enabled = !busy && !(channelId == CHANNEL_PUSH && pushHeld && mode == MODE_OFF), + onSetMode = { next -> onSetMode(item, channelId, next) }, ) } } 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 b4dc15a..01ae3fb 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 @@ -14,7 +14,7 @@ import com.runicgateway.app.core.result.ApiResult import com.runicgateway.app.data.api.dto.NotificationChannelItemDto import com.runicgateway.app.data.api.dto.NotificationChannelPrefsDto import com.runicgateway.app.data.repository.NotificationsRepository -import com.runicgateway.app.data.repository.PlayerShardRepository +import com.runicgateway.app.data.repository.LinkedAccountRepository import com.runicgateway.app.ui.UiState import com.runicgateway.app.ui.toUiState import dagger.hilt.android.lifecycle.HiltViewModel @@ -53,7 +53,7 @@ const val MODE_OFF = "off" @HiltViewModel class NotificationSettingsViewModel @Inject constructor( private val notifications: NotificationsRepository, - private val playerShard: PlayerShardRepository, + private val linkedAccounts: LinkedAccountRepository, private val pushManager: PushManager, sessionManager: SessionManager, ) : ViewModel() { @@ -62,7 +62,10 @@ class NotificationSettingsViewModel @Inject constructor( data class State( val prefs: UiState = UiState.Loading, - /** Whether the user has ≥1 linked game account — personal streams need it. */ + /** + * Whether the user has ≥1 linked game account on this site's module, asked + * of that module (Rust D69). It only gates switching push ON (see [canSetMode]). + */ val hasLinkedAccount: Boolean = false, /** Whether this shard advertises a push relay at all (else the screen says so). */ val supported: Boolean = true, @@ -94,9 +97,7 @@ class NotificationSettingsViewModel @Inject constructor( _state.update { it.copy(prefs = UiState.Loading) } viewModelScope.launch { _state.update { it.copy(prefs = notifications.channelPrefs().toUiState()) } - // A linked game account gates the personal streams; failure → treat as none. - val linked = (playerShard.accounts() as? ApiResult.Ok)?.data?.isNotEmpty() == true - _state.update { it.copy(hasLinkedAccount = linked) } + _state.update { it.copy(hasLinkedAccount = linkedAccounts.hasLinkedAccount()) } } } @@ -114,7 +115,7 @@ class NotificationSettingsViewModel @Inject constructor( fun setMode(item: NotificationChannelItemDto, channel: String, mode: String) { val current = _state.value if (current.busy) return - if (channel == CHANNEL_PUSH && !itemSelectable(item, current.hasLinkedAccount)) return + if (!canSetMode(item, channel, mode, current.hasLinkedAccount)) return _state.update { it.copy(busy = true, feedback = null) } viewModelScope.launch { @@ -160,8 +161,28 @@ class NotificationSettingsViewModel @Inject constructor( } /** - * Whether an item's controls are selectable for a user: a personal stream needs a - * linked game account (PLAN.md §11). Pure so the gating is unit-tested without Compose. + * Whether this item's push is held back for want of a linked game account + * (PLAN.md §11, `docs/modules/rust/PLAN.md` D69). + * + * **Push only.** Until phase 11 the whole row went dead, every channel, which + * made a personal stream impossible to switch *off* on any site where the link + * check failed. That was every Rust site, because the check asked `module-uo`. + * In-app and email have nothing to do with the link, and neither does turning + * anything off. Pure so the gating is unit-tested without Compose. */ -fun itemSelectable(item: NotificationChannelItemDto, hasLinkedAccount: Boolean): Boolean = - !item.requiresLinkedAccount || hasLinkedAccount +fun pushNeedsLink(item: NotificationChannelItemDto, hasLinkedAccount: Boolean): Boolean = + item.requiresLinkedAccount && !hasLinkedAccount && CHANNEL_PUSH in item.channels + +/** + * Whether [mode] may be set on [channel] for [item]. + * + * The one thing refused is switching push **on** for an item [pushNeedsLink] holds + * back. Switching anything off is never refused: a gate that can trap a switch in + * the on position is worse than no gate. + */ +fun canSetMode( + item: NotificationChannelItemDto, + channel: String, + mode: String, + hasLinkedAccount: Boolean, +): Boolean = !(channel == CHANNEL_PUSH && mode != MODE_OFF && pushNeedsLink(item, hasLinkedAccount)) diff --git a/app/src/main/java/com/runicgateway/app/ui/rust/RustServerViewModel.kt b/app/src/main/java/com/runicgateway/app/ui/rust/RustServerViewModel.kt index 48ae83a..a1c9677 100644 --- a/app/src/main/java/com/runicgateway/app/ui/rust/RustServerViewModel.kt +++ b/app/src/main/java/com/runicgateway/app/ui/rust/RustServerViewModel.kt @@ -26,7 +26,20 @@ import kotlinx.coroutines.launch import javax.inject.Inject /** The four sections of a server's page (D13). */ -enum class RustTab { FEED, LEADERBOARD, ONLINE, WIPES } +enum class RustTab { + FEED, LEADERBOARD, ONLINE, WIPES; + + /** The website's name for this tab, as `?tab=` carries it (`ServerDetail.jsx`). */ + val wire: String get() = name.lowercase() + + companion object { + /** + * The tab the website calls [wire], or null for one the app does not have. + * The website also has `clans`, and a link to it is not a link to the feed. + */ + fun fromWire(wire: String?): RustTab? = entries.firstOrNull { it.wire == wire } + } +} /** What a leaderboard column sorts by — the API's own vocabulary, not the app's. */ object RustSort { @@ -91,11 +104,15 @@ class RustServerViewModel @Inject constructor( private val serverId: String = savedStateHandle[Routes.Args.SERVER_ID] ?: "" + /** The tab a link asked for (D71), or null for the feed every page opens on. */ + private val initialTab: RustTab? = RustTab.fromWire(savedStateHandle[Routes.Args.TAB]) + private val _state = MutableStateFlow(RustServerUi(serverId = serverId)) val state: StateFlow = _state.asStateFlow() init { load() + initialTab?.takeIf { it != RustTab.FEED }?.let(::selectTab) } /** A first load or a retry of the whole page. */ diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 793228c..f8f846f 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -490,7 +490,7 @@ Choose what this shard notifies you about, and how it reaches you. Nothing is sent unless you turn it on. General Your game account - Link a game account to enable this. + Link a game account to get this as a push notification. This shard offers no notification streams yet. This shard hasn\'t set up push notifications yet. Notification settings saved. diff --git a/app/src/test/java/com/runicgateway/app/core/push/PushContentTest.kt b/app/src/test/java/com/runicgateway/app/core/push/PushContentTest.kt new file mode 100644 index 0000000..97ff770 --- /dev/null +++ b/app/src/test/java/com/runicgateway/app/core/push/PushContentTest.kt @@ -0,0 +1,101 @@ +/* + * SPDX-License-Identifier: GPL-3.0-or-later + */ +package com.runicgateway.app.core.push + +import com.runicgateway.app.data.api.dto.NotificationInboxDto +import com.runicgateway.app.data.api.dto.NotificationItemDto +import com.runicgateway.app.data.api.fake.FakeNotificationsApi +import com.runicgateway.app.data.repository.NotificationsRepository +import kotlinx.coroutines.test.runTest +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Test +import java.io.IOException + +/** + * A tickle titled from the inbox row it points at (`docs/modules/rust/PLAN.md` + * D70), and every way that falls back to the per-stream title. + */ +class PushContentTest { + + private val raid = NotificationItemDto( + id = 42, + triggerId = "rust.base.destroyed", + title = "Your base is being raided", + body = "A door was destroyed in S16 on Oxide rig.", + ) + private val raidTickle = PushTickle(stream = "rust.base.destroyed", ref = "notification:42") + + // ── The ref ──────────────────────────────────────────────────────────── + + @Test fun aRefNamesAnInboxIdOnlyWhenItIsExactlyOne() { + assertEquals(42L, inboxIdFromRef("notification:42")) + assertNull(inboxIdFromRef(null)) + assertNull(inboxIdFromRef("notification:")) + assertNull(inboxIdFromRef("notification:0")) + assertNull(inboxIdFromRef("notification:-4")) + assertNull(inboxIdFromRef("notification:4a")) + assertNull(inboxIdFromRef("notification: 4")) + assertNull(inboxIdFromRef("xnotification:4")) + assertNull(inboxIdFromRef("0x24C")) + assertNull(inboxIdFromRef("notification:" + "9".repeat(40))) + } + + // ── Which row ────────────────────────────────────────────────────────── + + @Test fun theRowMustBeTheNamedOneFromTheNamedTrigger() { + val other = raid.copy(id = 41, title = "An older raid") + assertEquals(raid, itemForTickle(raidTickle, listOf(other, raid))) + // Same id, different trigger: the tickle disagrees with the row. + assertNull(itemForTickle(raidTickle.copy(stream = "rust.server.online"), listOf(raid))) + assertNull(itemForTickle(raidTickle.copy(ref = "notification:7"), listOf(raid))) + assertNull(itemForTickle(raidTickle.copy(ref = null), listOf(raid))) + } + + // ── What the notification says ───────────────────────────────────────── + + @Test fun theRowsTitleAndBodyAreUsed() { + val text = notificationText(raid, generic = "New notification") + assertEquals("Your base is being raided", text.title) + assertEquals("A door was destroyed in S16 on Oxide rig.", text.body) + } + + @Test fun noRowOrABlankTitleFallsBackToTheStreamTitle() { + assertEquals(NotificationText("New notification", null), notificationText(null, "New notification")) + assertEquals( + NotificationText("New notification", null), + notificationText(raid.copy(title = " "), "New notification"), + ) + assertEquals(null, notificationText(raid.copy(body = " "), "x").body) + } + + // ── The pull ─────────────────────────────────────────────────────────── + + private val api = FakeNotificationsApi() + private val resolver = PushContentResolver(NotificationsRepository(api)) + + @Test fun theResolverPullsTheFirstPage() = runTest { + api.pages = mapOf(null to NotificationInboxDto(items = listOf(raid), unread = 1)) + + assertEquals(raid, resolver.itemFor(raidTickle)) + assertEquals(listOf(null), api.inboxCalls) + } + + @Test fun aTickleWithNoInboxRefIsNeverPulled() = runTest { + // A classic M7 stream, or a push-only rule with no inbox row. + assertNull(resolver.itemFor(PushTickle(stream = "news.post", ref = "0x24C"))) + assertTrue(api.inboxCalls.isEmpty()) + } + + @Test fun aFailedPullIsNullNotAnError() = runTest { + api.error = IOException("offline") + assertNull(resolver.itemFor(raidTickle)) + } + + @Test fun aRowNotOnThePageIsNull() = runTest { + api.pages = mapOf(null to NotificationInboxDto(items = listOf(raid.copy(id = 99)))) + assertNull(resolver.itemFor(raidTickle)) + } +} diff --git a/app/src/test/java/com/runicgateway/app/data/repository/LinkedAccountRepositoryTest.kt b/app/src/test/java/com/runicgateway/app/data/repository/LinkedAccountRepositoryTest.kt new file mode 100644 index 0000000..9262d37 --- /dev/null +++ b/app/src/test/java/com/runicgateway/app/data/repository/LinkedAccountRepositoryTest.kt @@ -0,0 +1,91 @@ +/* + * SPDX-License-Identifier: GPL-3.0-or-later + */ +package com.runicgateway.app.data.repository + +import com.runicgateway.app.data.api.dto.InstalledModuleDto +import com.runicgateway.app.data.api.dto.ModulesDto +import com.runicgateway.app.data.api.dto.RustLinkDto +import com.runicgateway.app.data.api.dto.RustLinkListDto +import com.runicgateway.app.data.api.dto.ShardLinkDto +import com.runicgateway.app.data.api.fake.FakePlayerRustApi +import com.runicgateway.app.data.api.fake.FakePlayerShardApi +import com.runicgateway.app.data.api.fake.FakePublicApi +import com.runicgateway.app.util.httpError +import kotlinx.coroutines.test.runTest +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * "Has this user linked a game account" is asked of the site's own module + * (`docs/modules/rust/PLAN.md` D69). + * + * Before phase 11 it was always `module-uo`'s `/player/shard/accounts`. A Rust + * site has no such route, so the question always answered no, and the raid + * alert's row was locked in every channel. + */ +class LinkedAccountRepositoryTest { + + private val publicApi = FakePublicApi() + private val capabilities = SiteCapabilitiesRepository(publicApi) + private val rustApi = FakePlayerRustApi() + private val shardApi = FakePlayerShardApi() + private val repository = LinkedAccountRepository( + capabilities, + PlayerRustRepository(rustApi), + PlayerShardRepository(shardApi), + ) + + private suspend fun running(module: String, vararg caps: String) { + publicApi.modules = ModulesDto(modules = listOf(InstalledModuleDto(id = module, capabilities = caps.toList()))) + capabilities.refresh() + } + + @Test fun aRustSiteAsksTheRustModule() = runTest { + running("rust", "rust", "servers", "identity") + rustApi.links = RustLinkListDto(links = listOf(RustLinkDto(steamId = "76561198000000001"))) + // The UO read would say no, and it must not be the one that is asked. + shardApi.error = httpError(404) + + assertTrue(repository.hasLinkedAccount()) + assertEquals(1, rustApi.linksCalls) + } + + @Test fun aRustSiteWithNoLinkSaysNo() = runTest { + running("rust", "rust") + assertFalse(repository.hasLinkedAccount()) + } + + @Test fun aUoSiteStillAsksTheShardModuleAndNeverTheRustOne() = runTest { + running("uo", "shard", "atlas") + shardApi.accounts = listOf(ShardLinkDto(account = "walker")) + + assertTrue(repository.hasLinkedAccount()) + assertEquals(0, rustApi.linksCalls) + } + + @Test fun aSiteWithNeitherModuleHasNothingToAsk() = runTest { + running("other", "something") + rustApi.links = RustLinkListDto(links = listOf(RustLinkDto(steamId = "1"))) + + assertFalse(repository.hasLinkedAccount()) + assertEquals(0, rustApi.linksCalls) + } + + @Test fun aHostThatHasNeverAnsweredAsksBothAndAFailureIsNoLink() = runTest { + // One site runs one module, so one of the two 404s; that is not an error. + shardApi.error = httpError(404) + rustApi.links = RustLinkListDto(links = listOf(RustLinkDto(steamId = "1"))) + + assertTrue(repository.hasLinkedAccount()) + } + + @Test fun theSourcesAreReadOffTheCapabilities() { + assertEquals(LinkSource.entries, linkSourcesFor(null)) + assertEquals(listOf(LinkSource.RUST), linkSourcesFor(SiteCapabilities(emptySet(), setOf("rust")))) + assertEquals(listOf(LinkSource.SHARD), linkSourcesFor(SiteCapabilities(emptySet(), setOf("shard")))) + assertEquals(emptyList(), linkSourcesFor(SiteCapabilities(setOf("events"), emptySet()))) + } +} diff --git a/app/src/test/java/com/runicgateway/app/ui/navigation/RustNavigationTest.kt b/app/src/test/java/com/runicgateway/app/ui/navigation/RustNavigationTest.kt index 30e7a37..2dba399 100644 --- a/app/src/test/java/com/runicgateway/app/ui/navigation/RustNavigationTest.kt +++ b/app/src/test/java/com/runicgateway/app/ui/navigation/RustNavigationTest.kt @@ -8,6 +8,7 @@ import com.runicgateway.app.core.auth.Session import com.runicgateway.app.core.auth.SessionUser import com.runicgateway.app.data.repository.Capability import com.runicgateway.app.data.repository.SiteCapabilities +import com.runicgateway.app.ui.rust.RustTab import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse import org.junit.Assert.assertNull @@ -148,4 +149,38 @@ class RustNavigationTest { assertNull(resolveWebPath("/rust?tab=wipes")) assertNull(resolveWebPath("/rust/servers/main?wipe=w1")) } + + // ── The notification links (phase 11, D71) ──────────────────────────── + + @Test fun theNewLeaderNoticeOpensTheLeaderboardTab() { + // `leaderboardPath` in module-rust's triggers.js. + assertEquals("rust/servers/main?tab=leaderboard", resolveWebPath("/rust/servers/main?tab=leaderboard")) + assertEquals(Routes.rustServer("main", RustTab.LEADERBOARD), resolveWebPath("/rust/servers/main?tab=leaderboard")) + assertEquals(Routes.rustServer("main", RustTab.WIPES), resolveWebPath("/rust/servers/main?tab=wipes")) + } + + @Test fun onlyATabTheAppHasResolves() { + // The website has a Clans tab and the app does not: the browser, not the feed. + assertNull(resolveWebPath("/rust/servers/main?tab=clans")) + assertNull(resolveWebPath("/rust/servers/main?tab=")) + assertNull(resolveWebPath("/rust/servers/main?tab=wipes&wipe=w1")) + assertNull(resolveWebPath("/rust/servers/main?sort=kills")) + assertNull(resolveWebPath("/rust/servers/main?tab=LEADERBOARD")) + } + + @Test fun aTablessRouteStaysTheOneEveryOtherCallerBuilds() { + assertEquals("rust/servers/main", Routes.rustServer("main")) + assertEquals(RustTab.ONLINE, RustTab.fromWire("online")) + assertNull(RustTab.fromWire("clans")) + assertNull(RustTab.fromWire(null)) + } + + @Test fun theAccountLinkedNoticeOpensThePlayersOwnScreen() { + // `PATHS.account` in module-rust's triggers.js. M15 deferred this to the + // phase that would need it. + assertEquals(Routes.PLAYER_RUST, resolveWebPath("/player/rust")) + assertEquals(Routes.PLAYER_RUST, resolveWebPath("/player/rust/")) + assertNull(resolveWebPath("/player/rust?x=1")) + assertNull(resolveWebPath("/player/shard")) + } } diff --git a/app/src/test/java/com/runicgateway/app/ui/notifications/NotificationRoutingTest.kt b/app/src/test/java/com/runicgateway/app/ui/notifications/NotificationRoutingTest.kt index 269425a..8a0368e 100644 --- a/app/src/test/java/com/runicgateway/app/ui/notifications/NotificationRoutingTest.kt +++ b/app/src/test/java/com/runicgateway/app/ui/notifications/NotificationRoutingTest.kt @@ -14,7 +14,8 @@ import org.junit.Test /** * Tests the pure notification helpers: the stream → deep-link route map (PLAN.md * §11 work item 7), the tickle routing that ENGAGEMENT.md phase 8 layered over it, - * and the personal-item gating (a personal id needs a linked game account). + * and the personal-item gating (a personal id needs a linked game account to + * switch push on, and nothing else). */ class NotificationRoutingTest { @@ -33,15 +34,23 @@ class NotificationRoutingTest { assertEquals(Routes.HOME, Routes.forStream("something.new")) } - @Test fun personalItemNeedsLinkedAccount() { - val personal = NotificationChannelItemDto(id = "vendor.sale", personal = true, requiresLinkedAccount = true) - assertFalse(itemSelectable(personal, hasLinkedAccount = false)) - assertTrue(itemSelectable(personal, hasLinkedAccount = true)) + @Test fun personalItemNeedsALinkOnlyToSwitchPushOn() { + val personal = NotificationChannelItemDto( + id = "vendor.sale", personal = true, requiresLinkedAccount = true, + channels = listOf("push", "inapp", "email"), + ) + assertTrue(pushNeedsLink(personal, hasLinkedAccount = false)) + assertFalse(pushNeedsLink(personal, hasLinkedAccount = true)) + assertFalse(canSetMode(personal, CHANNEL_PUSH, "instant", hasLinkedAccount = false)) + assertTrue(canSetMode(personal, CHANNEL_PUSH, "instant", hasLinkedAccount = true)) } - @Test fun generalItemIsAlwaysSelectable() { - val general = NotificationChannelItemDto(id = "news.post", personal = false, requiresLinkedAccount = false) - assertTrue(itemSelectable(general, hasLinkedAccount = false)) + @Test fun generalItemIsNeverHeldBack() { + val general = NotificationChannelItemDto( + id = "news.post", personal = false, requiresLinkedAccount = false, channels = listOf("push"), + ) + assertFalse(pushNeedsLink(general, hasLinkedAccount = false)) + assertTrue(canSetMode(general, CHANNEL_PUSH, "instant", hasLinkedAccount = false)) } // ── The tickle → destination map (ENGAGEMENT.md phase 8) ─────────────── diff --git a/app/src/test/java/com/runicgateway/app/ui/notifications/NotificationSettingsViewModelTest.kt b/app/src/test/java/com/runicgateway/app/ui/notifications/NotificationSettingsViewModelTest.kt index b0dcd20..b212983 100644 --- a/app/src/test/java/com/runicgateway/app/ui/notifications/NotificationSettingsViewModelTest.kt +++ b/app/src/test/java/com/runicgateway/app/ui/notifications/NotificationSettingsViewModelTest.kt @@ -64,10 +64,36 @@ class NotificationSettingsViewModelTest { assertEquals(listOf("off", "instant"), push.modes) } - @Test fun personalItemsStillNeedALinkedAccount() { + @Test fun aTriggerOnlyPersonalItemIsNotHeldBackAtAll() { + // Nothing can push it, so there is no push control for the link to hold, + // and the hint would describe a control that is not on the screen. val personal = prefs().items.first { it.personal } - assertEquals(false, itemSelectable(personal, hasLinkedAccount = false)) - assertEquals(true, itemSelectable(personal, hasLinkedAccount = true)) + assertEquals(false, pushNeedsLink(personal, hasLinkedAccount = false)) + assertEquals(true, canSetMode(personal, "email", "instant", hasLinkedAccount = false)) + } + + // ── Rust D69: a Rust site's raid alert ───────────────────────────────── + + private val raid = NotificationChannelItemDto( + id = "rust.base.destroyed", label = "Your base was raided", + personal = true, requiresLinkedAccount = true, + channels = listOf("push", "inapp", "email"), + modes = mapOf("push" to "instant", "inapp" to "instant", "email" to "off"), + ) + + @Test fun withALinkPushCanBeSwitchedOn() { + assertEquals(true, canSetMode(raid, CHANNEL_PUSH, "instant", hasLinkedAccount = true)) + } + + @Test fun withoutALinkEveryChannelCanStillBeSwitchedOff() { + // Phase 10's criterion ends "and can be switched off there". Before phase + // 11 the whole row was disabled whenever the link check failed, and it + // always failed on a Rust site. + listOf(CHANNEL_PUSH, "inapp", "email").forEach { channel -> + assertEquals(channel, true, canSetMode(raid, channel, MODE_OFF, hasLinkedAccount = false)) + } + assertEquals(true, canSetMode(raid, "inapp", "instant", hasLinkedAccount = false)) + assertEquals(false, canSetMode(raid, CHANNEL_PUSH, "instant", hasLinkedAccount = false)) } @Test fun oneToggleSendsExactlyOnePair() = kotlinx.coroutines.runBlocking {