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 {