From 94a5c26d6c570ff9b7d5df797027e412e8674ec4 Mon Sep 17 00:00:00 2001 From: wtclaude Date: Sat, 8 Aug 2026 07:07:10 -0500 Subject: [PATCH] feat(nav): honor the shard's public nav order, labels and hiding (M12 phase 5) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The drawer has been the app's coded `APP_MENU` in coded order since M1. Phase 5 lets an admin's `nav_public` row relabel, reorder and hide its public rows, which is the first time anything in the app's navigation comes from the shard. The public nav is keyed by **website** paths, so this needs a translation table, and it is the one new piece of cross-repo coupling the milestone introduces. It lives in a single file with the website's own `NAV` array quoted beside it — `NavPaths.kt` — so the coupling is visible and reviewable in one place instead of spread across the drawer's call sites. The `feature` values are deliberately not mirrored: `APP_MENU` stays the app's own source of truth for gating, and a second copy of a security-relevant value that drifts silently is worth more than it costs. Nine of the sixteen website rows have a drawer row. The other seven map to a screen the app reaches another way — three news categories are tabs on one News screen, and champs / guilds / governors / houses sit behind the Shard hub because that is the better shape on a phone — and an override for one of them is **ignored**, which is §6.1's rule that a nav override may never introduce navigation. The hub is a design decision, not an accident to correct. The mapping still exists for all sixteen because phase 6's added links resolve an admin-authored path against the same table, and there a category tab or a hub board is a perfectly good destination: the admin asked for it by path. The merge is a port of the website's `applyNavOverrides`, narrowed to what a drawer can express — `label`, `order`, `hidden`, and nothing else. It runs **before** `visibleEntries`, so the two gates from M10/M11 still decide what this caller may see and remain the actual boundary: an override that relabels the Market row, moves it to the front and says `hidden: false` still shows nothing to a caller whose shard does not publish the market. Hiding is subtractive, never additive. One thing the design did not settle and the sort turns on: an untouched row's implicit key has to be its index in the **website's** nav, not the app's. A stored `order` is a position in that list, so a key taken from the app's shorter list would put explicit and implicit keys on two incomparable number lines and scramble a partially-overridden nav. Both tie-breaks are the web's — an explicit order beats a coincidental index, and two explicit orders keep code order. `Routes.news(category)` and an optional NavHost argument ship here as the table's route builder; phase 6 is their first caller. Navigating to plain `Routes.NEWS` matches the new pattern with no argument and opens the default tab, so the drawer and the push deep-link are unaffected — but `destination.route` is now a pattern with a query, so the top-level and selected-row checks compare on the part before it. Two questions went to the org lead before any code. The three news-category paths get a mapped route but no drawer row of their own, on the same rule as the hub four. And an admin **may** hide Home, mirroring the website, where `/` is hideable too: Home stays the NavHost's start destination and stays reachable by back-press, and the app does not invent a policy the site doesn't have. 442 unit tests green (410 + 32), `lintDebug` and `assembleDebug` clean. The strongest of them is AC-1's: with no stored row the merge returns `APP_MENU` itself — identity, not equality — so an instance whose admin never touched the nav provably gets the drawer the app shipped with. Co-Authored-By: Claude --- .../java/com/runicgateway/app/MainActivity.kt | 2 +- .../java/com/runicgateway/app/ui/RunicApp.kt | 37 ++- .../runicgateway/app/ui/navigation/Menu.kt | 9 + .../app/ui/navigation/NavOverrides.kt | 136 +++++++++ .../app/ui/navigation/NavPaths.kt | 121 ++++++++ .../runicgateway/app/ui/navigation/Routes.kt | 22 ++ .../runicgateway/app/ui/news/NewsViewModel.kt | 10 +- .../app/ui/ContentViewModelTest.kt | 26 +- .../app/ui/navigation/NavOverridesTest.kt | 271 ++++++++++++++++++ .../app/ui/navigation/NavPathsTest.kt | 119 ++++++++ 10 files changed, 743 insertions(+), 10 deletions(-) create mode 100644 app/src/main/java/com/runicgateway/app/ui/navigation/NavOverrides.kt create mode 100644 app/src/main/java/com/runicgateway/app/ui/navigation/NavPaths.kt create mode 100644 app/src/test/java/com/runicgateway/app/ui/navigation/NavOverridesTest.kt create mode 100644 app/src/test/java/com/runicgateway/app/ui/navigation/NavPathsTest.kt diff --git a/app/src/main/java/com/runicgateway/app/MainActivity.kt b/app/src/main/java/com/runicgateway/app/MainActivity.kt index 24ecace..2a22b4b 100644 --- a/app/src/main/java/com/runicgateway/app/MainActivity.kt +++ b/app/src/main/java/com/runicgateway/app/MainActivity.kt @@ -96,7 +96,7 @@ class MainActivity : ComponentActivity() { ConnectScreen(onConnected = appViewModel::onConnected) is AppState.Ready -> RunicApp( - brand = s.appearance.brand, + appearance = s.appearance, onChangeServer = appViewModel::changeServer, deepLinkStream = pendingStream, onDeepLinkConsumed = { pendingStream = null }, 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 3be465b..f0caed5 100644 --- a/app/src/main/java/com/runicgateway/app/ui/RunicApp.kt +++ b/app/src/main/java/com/runicgateway/app/ui/RunicApp.kt @@ -49,6 +49,7 @@ import androidx.navigation.navArgument import com.runicgateway.app.R import com.runicgateway.app.core.auth.Session import com.runicgateway.app.data.api.dto.BrandDto +import com.runicgateway.app.data.appearance.SiteAppearance import com.runicgateway.app.ui.auth.AccountScreen import com.runicgateway.app.ui.auth.LoginScreen import com.runicgateway.app.ui.auth.RecoveryCodesScreen @@ -59,6 +60,7 @@ import com.runicgateway.app.ui.contact.ContactScreen import com.runicgateway.app.ui.home.HomeScreen import com.runicgateway.app.ui.navigation.APP_MENU import com.runicgateway.app.ui.navigation.Routes +import com.runicgateway.app.ui.navigation.applyNavOverrides import com.runicgateway.app.ui.navigation.visibleEntries import com.runicgateway.app.ui.news.NewsScreen import com.runicgateway.app.ui.news.PostScreen @@ -110,13 +112,14 @@ private val TOP_LEVEL_ROUTES = setOf( @OptIn(ExperimentalMaterial3Api::class) @Composable fun RunicApp( - brand: BrandDto?, + appearance: SiteAppearance, onChangeServer: () -> Unit, modifier: Modifier = Modifier, deepLinkStream: String? = null, onDeepLinkConsumed: () -> Unit = {}, sessionViewModel: SessionViewModel = hiltViewModel(), ) { + val brand = appearance.brand val navController = rememberNavController() val drawerState = rememberDrawerState(DrawerValue.Closed) val scope = rememberCoroutineScope() @@ -142,9 +145,19 @@ fun RunicApp( } val backStackEntry by navController.currentBackStackEntryAsState() - val currentRoute = backStackEntry?.destination?.route + // A destination's route is its NavHost *pattern*, so News reports + // "news?category={category}" (§6.2). Compare on the part before the query. + val currentRoute = backStackEntry?.destination?.route?.substringBefore('?') val isTopLevel = currentRoute in TOP_LEVEL_ROUTES - val entries = visibleEntries(APP_MENU, session, shardFeatures) + // The admin's nav overrides, then the gates — never the other way round. An + // override is presentation only: it may relabel, reorder and hide, so + // `visibleEntries` still decides what this caller may see and remains the + // boundary (§6.1, AC-3). With no stored row the merge returns APP_MENU itself. + val entries = visibleEntries( + applyNavOverrides(APP_MENU, appearance.navPublic), + session, + shardFeatures, + ) ModalNavigationDrawer( drawerState = drawerState, @@ -180,7 +193,9 @@ fun RunicApp( Spacer(Modifier.height(8.dp)) entries.forEach { entry -> NavigationDrawerItem( - label = { Text(stringResource(entry.labelRes)) }, + // An admin's label wins over the bundled one, and is the + // same string in every locale — see MenuEntry.label. + label = { Text(entry.label ?: stringResource(entry.labelRes)) }, selected = currentRoute == entry.route, onClick = { scope.launch { drawerState.close() } @@ -304,7 +319,19 @@ private fun RunicNavHost( composable(Routes.HOME) { HomeScreen(brand = brand) } - composable(Routes.NEWS) { + // The category is optional: navigating to plain Routes.NEWS matches this + // pattern with no argument and opens the default tab, which is every route + // into the screen except an admin's nav override or added link (§6.2). + composable( + route = Routes.NEWS_ROUTE, + arguments = listOf( + navArgument(Routes.Args.CATEGORY) { + type = NavType.StringType + nullable = true + defaultValue = null + }, + ), + ) { NewsScreen(onOpenPost = { category, idOrSlug -> navController.navigate(Routes.post(category, idOrSlug)) }) diff --git a/app/src/main/java/com/runicgateway/app/ui/navigation/Menu.kt b/app/src/main/java/com/runicgateway/app/ui/navigation/Menu.kt index 994a9fb..da255b6 100644 --- a/app/src/main/java/com/runicgateway/app/ui/navigation/Menu.kt +++ b/app/src/main/java/com/runicgateway/app/ui/navigation/Menu.kt @@ -52,6 +52,15 @@ data class MenuEntry( * isn't shard-derived and only [access] applies. */ val feature: String? = null, + /** + * An admin's own label for this row, from the shard's `nav_public` override + * (THEMING_AND_NAV.md §6). Null — always, as coded — means [labelRes] stands. + * + * A label set this way is **not localized**: it is one string for every locale, + * which is what an admin typing a label means, and it matches the website. It + * only ever arrives via [applyNavOverrides]; nothing in [APP_MENU] sets it. + */ + val label: String? = null, ) /** diff --git a/app/src/main/java/com/runicgateway/app/ui/navigation/NavOverrides.kt b/app/src/main/java/com/runicgateway/app/ui/navigation/NavOverrides.kt new file mode 100644 index 0000000..bae3d69 --- /dev/null +++ b/app/src/main/java/com/runicgateway/app/ui/navigation/NavOverrides.kt @@ -0,0 +1,136 @@ +/* + * SPDX-License-Identifier: GPL-3.0-or-later + */ +package com.runicgateway.app.ui.navigation + +import kotlinx.serialization.json.JsonObject +import kotlinx.serialization.json.JsonPrimitive +import kotlinx.serialization.json.booleanOrNull +import kotlinx.serialization.json.doubleOrNull + +/** + * Apply the admin's stored public-nav overrides to the app's coded menu + * (THEMING_AND_NAV.md §6). The Kotlin counterpart of the website's + * `client/src/lib/navOverrides.js`, narrowed to what a drawer can express. + * + * **This is presentation, never authorization.** An override carries `label`, + * `order` and `hidden` and nothing else: it cannot introduce a route, cannot + * touch [MenuEntry.access] or [MenuEntry.feature], and cannot un-hide anything — + * `hidden: false` is simply the absence of hiding. [visibleEntries] therefore runs + * **after** this merge, unchanged, and remains the actual boundary (§6.1, AC-3). + * + * Fail-safe throughout, matching the web: anything unrecognized — an unknown path, + * a non-string label, a path the app doesn't surface in its menu — is ignored + * rather than rejected, so a stale or hand-edited settings row degrades to the + * coded menu instead of rendering a broken drawer. + */ + +/** A usable override for one menu row. Absent fields mean "as coded". */ +private data class NavOverride( + val label: String? = null, + val order: Double? = null, + val hidden: Boolean = false, +) { + val isEmpty: Boolean get() = label == null && order == null && !hidden +} + +/** + * The `items` map out of a stored `nav_public` value. + * + * Two shapes exist, because website phase 10 added sections and links without + * migrating what phases 6-8 had already stored: `{items, sections, links}` and a + * bare map of path → override. A bare map is unambiguous — every key is a path, + * so a key can never be the string `items`. + * + * `sections` and `links` are deliberately not read here; they are phase 6's. + */ +private fun itemsOf(navPublic: JsonObject?): Map { + if (navPublic == null) return emptyMap() + val wrapped = navPublic["items"] as? JsonObject + val items = wrapped ?: navPublic + return items.entries + .mapNotNull { (key, value) -> (value as? JsonObject)?.let { key to it } } + .toMap() +} + +// Field by field, like every other read in M12: a bad `label` must not discard a +// good `order` beside it. +// +// `group` and `section` are ignored. The app renders no sections in this phase +// (phase 6) and never renders the admin sidebar's groups at all, and a value it +// cannot honor is better dropped than half-applied. +private fun cleanOverride(raw: JsonObject): NavOverride { + val label = (raw["label"] as? JsonPrimitive) + ?.takeIf { it.isString } + ?.content + ?.trim() + ?.takeIf { it.isNotEmpty() } + val order = (raw["order"] as? JsonPrimitive) + ?.takeIf { !it.isString } + ?.doubleOrNull + ?.takeIf { it.isFinite() } + val hidden = (raw["hidden"] as? JsonPrimitive) + ?.takeIf { !it.isString } + ?.booleanOrNull == true + return NavOverride(label = label, order = order, hidden = hidden) +} + +/** + * [base] with the admin's overrides applied: rows relabeled, reordered and + * dropped as the stored row asks. + * + * @param base the coded menu — the only source of `route`, `access` and `feature` + * @param navPublic the parsed `nav_public` row, or null when the admin never + * edited the nav. Null, malformed, and "nothing usable in it" all return [base] + * itself, which is what makes an untouched instance's drawer provably today's + * (§2, AC-1). + */ +fun applyNavOverrides(base: List, navPublic: JsonObject?): List { + val items = itemsOf(navPublic) + if (items.isEmpty()) return base + + val coded = base.map { it.route }.toSet() + // Keyed by app route, and only for a route the coded menu actually declares. + // This is where an override for a path the app doesn't surface in its drawer — + // a news category tab, a Shard hub board — is dropped (§6.2). The web does the + // same with an unknown `to`. + val overrides = buildMap { + for ((path, raw) in items) { + val route = appRouteForWebPath(path) ?: continue + if (route !in coded) continue + val override = cleanOverride(raw) + if (!override.isEmpty) put(route, override) + } + } + if (overrides.isEmpty()) return base + + // Rows the website's nav knows about are the ones an override can move; the + // app's own surfaces (Contact, Account, the player groups, the staff rows) + // have no counterpart to be reordered against and keep their coded order, + // appended after the public block — which is exactly where they sit today, so + // this partition is the current layout rather than a new one (§6.2). + val (mapped, appOnly) = base.partition { it.route in WEB_ROUTE_ORDER } + + val sorted = mapped + // An untouched row's sort key is its index in the WEBSITE's nav, not the + // app's: a stored `order` is a position in that list, so both keys have to + // sit on one number line to be comparable at all. + // + // Two tie-breaks, the web's: an explicit order beats a coincidental index + // (the admin said "first", so first), and two explicit orders keep code + // order, because the sort is stable. + .sortedWith( + compareBy { entry -> + overrides[entry.route]?.order ?: WEB_ROUTE_ORDER.getValue(entry.route).toDouble() + }.thenByDescending { overrides[it.route]?.order != null }, + ) + + return (sorted + appOnly).mapNotNull { entry -> + val override = overrides[entry.route] ?: return@mapNotNull entry + when { + override.hidden -> null + override.label != null -> entry.copy(label = override.label) + else -> entry + } + } +} 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 new file mode 100644 index 0000000..bba1a95 --- /dev/null +++ b/app/src/main/java/com/runicgateway/app/ui/navigation/NavPaths.kt @@ -0,0 +1,121 @@ +/* + * SPDX-License-Identifier: GPL-3.0-or-later + */ +package com.runicgateway.app.ui.navigation + +import com.runicgateway.app.data.repository.ContentRepository.PostCategory + +/** + * The website path → app route table (THEMING_AND_NAV.md §6.2). + * + * The public nav an admin edits is keyed by **website** paths, so honoring it in + * the app needs a translation. This is the one new piece of cross-repo coupling + * the milestone introduces, which is why it lives in a single file with the + * website's own array quoted right beside it — the coupling is visible and + * reviewable in one place rather than spread across the drawer's call sites. + * + * Verbatim from `website/client/src/components/SiteHeader.jsx`, which is the + * exported owner of the list (`export const NAV`, and Admin → Navigation edits + * exactly it): + * + * ```js + * export const NAV = [ + * { label: 'Home', to: '/', end: true }, + * { label: 'News', to: '/site/news' }, + * { label: 'Screenshots', to: '/site/screenshots' }, + * { label: 'Five on Friday', to: '/site/five-on-friday' }, + * { label: 'Newsletter', to: '/site/newsletter' }, + * { label: 'Wiki', to: '/wiki' }, + * { label: 'Shard', to: '/site/shard', feature: 'status' }, + * { label: 'Champions', to: '/site/champs', feature: 'champs' }, + * { label: 'Guilds', to: '/site/guilds', feature: 'guilds' }, + * { label: 'Governors', to: '/site/governors', feature: 'governors' }, + * { label: 'Houses', to: '/site/houses', feature: 'houses' }, + * { label: 'Rules', to: '/site/rules', feature: 'ruleset' }, + * { label: 'Atlas', to: '/site/atlas', feature: 'atlas' }, + * { label: 'Leaderboards', to: '/site/leaderboards', feature: 'leaderboards' }, + * { label: 'Market', to: '/site/market', feature: 'market' }, + * { label: 'About', to: '/site/about' }, + * ] + * ``` + * + * The `feature` values are **not** mirrored here on purpose. [APP_MENU] is the + * app's own source of truth for gating, and a second copy of a security-relevant + * value that drifts silently is worth more than it costs. This table carries the + * mapping and nothing else. + * + * Not every row maps to something the app shows in its drawer, and that is the + * design rather than an omission — see [WEB_PATH_TO_ROUTE]. + */ + +/** One row of the website's public nav: its path, and the app route it opens. */ +data class WebNavPath(val path: String, val route: String) + +/** + * The website's public nav in **its** order, mapped to app routes. + * + * The order is load-bearing, not decorative: a stored `order` is an index into + * *this* list (the admin's editor writes the position a row holds on the web), so + * a row the admin never moved has to take its key from the same number line or + * explicit and implicit keys would be incomparable. See `NavOverrides.kt`. + */ +val WEBSITE_PUBLIC_NAV: List = listOf( + WebNavPath("/", Routes.HOME), + WebNavPath("/site/news", Routes.NEWS), + // The app's News screen carries all four categories as tabs, so these three + // have a route but no drawer row of their own — see the note below. + WebNavPath("/site/screenshots", Routes.news(PostCategory.SCREENSHOTS)), + WebNavPath("/site/five-on-friday", Routes.news(PostCategory.FIVE_ON_FRIDAY)), + WebNavPath("/site/newsletter", Routes.news(PostCategory.NEWSLETTER)), + WebNavPath("/wiki", Routes.WIKI), + WebNavPath("/site/shard", Routes.SHARD), + // Behind the Shard hub in the app, deliberately — no drawer row either. + WebNavPath("/site/champs", Routes.SHARD_CHAMPS), + WebNavPath("/site/guilds", Routes.SHARD_GUILDS), + WebNavPath("/site/governors", Routes.SHARD_GOVERNORS), + WebNavPath("/site/houses", Routes.SHARD_HOUSES), + WebNavPath("/site/rules", Routes.SHARD_RULES), + WebNavPath("/site/atlas", Routes.ATLAS), + WebNavPath("/site/leaderboards", Routes.SHARD_LEADERBOARDS), + WebNavPath("/site/market", Routes.SHARD_MARKET), + WebNavPath("/site/about", Routes.page("about")), +) + +/** + * The same table as a lookup. + * + * **A mapped route is not the same thing as a drawer row.** Seven of these paths + * resolve to a screen the app reaches some other way: the three news categories + * are tabs on one News screen, and champs / guilds / governors / houses sit behind + * the Shard hub because that is the better shape on a phone. An override for one + * of them is **ignored** — §6.1's rule is that a nav override may never introduce + * navigation, and the hub is a design decision, not an accident to correct. The + * merge enforces that by intersecting with [APP_MENU]; nothing here needs to know + * which rows those are. + * + * The mapping still exists for all sixteen because phase 6's added links resolve + * an admin-authored path against the same table, and *there* a category tab or a + * hub board is a perfectly good destination — the admin asked for it by path. + */ +val WEB_PATH_TO_ROUTE: Map = + WEBSITE_PUBLIC_NAV.associate { it.path to it.route } + +/** + * Each app route's index in the website's own nav order — the sort key a row the + * admin never moved takes, so it lands on the same number line as a stored + * `order`. All sixteen routes are distinct, so this loses nothing. + */ +internal val WEB_ROUTE_ORDER: Map = + WEBSITE_PUBLIC_NAV.withIndex().associate { (index, row) -> row.route to index } + +/** + * The app route a website nav path opens, or null when the app has no screen for + * it. A trailing slash is tolerated (`/wiki/` is `/wiki`) since a hand-edited + * settings row may carry one; the root path is left alone. + */ +fun appRouteForWebPath(path: String?): String? { + val trimmed = path?.trim().orEmpty() + if (trimmed.isEmpty()) return null + val normalized = if (trimmed.length > 1) trimmed.trimEnd('/') else trimmed + return WEB_PATH_TO_ROUTE[normalized.ifEmpty { "/" }] +} 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 449f980..ac0dc4d 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 @@ -3,6 +3,8 @@ */ package com.runicgateway.app.ui.navigation +import com.runicgateway.app.data.repository.ContentRepository + /** * Navigation destinations for the M1 public surface (PLAN.md §5). Routes are * plain strings for Navigation-Compose; argument-bearing routes expose a @@ -14,6 +16,19 @@ object Routes { const val WIKI = "wiki" const val CONTACT = "contact" + /** + * The News hub's NavHost pattern: [NEWS] plus an optional category, so a link + * to one of the website's three category pages can land on the matching tab + * (THEMING_AND_NAV.md §6.2). Navigating to plain [NEWS] matches this pattern + * with no argument and opens the default tab, so every existing call site — + * the drawer, [forStream] — is unaffected. + * + * Declared beside [NEWS] rather than replacing it because the two are used for + * different things: this is what `composable()` and `destination.route` speak, + * [NEWS] is what callers navigate to. + */ + const val NEWS_ROUTE = "news?category={category}" + /** Native login (§4.1) and the signed-in account surface (§5). */ const val LOGIN = "login" const val ACCOUNT = "account" @@ -79,6 +94,13 @@ object Routes { fun page(slug: String) = "page/$slug" fun post(categoryUrlSlug: String, idOrSlug: String) = "news/$categoryUrlSlug/$idOrSlug" + + /** + * The News hub with [category] preselected. Takes the enum rather than a slug + * so an unmapped category cannot reach the NavHost — the screen's tabs are the + * enum's entries, and a slug it doesn't know would select nothing. + */ + fun news(category: ContentRepository.PostCategory) = "news?category=${category.urlSlug}" fun wikiPage(slug: String) = "wiki/$slug" /** The character-sheet route for an in-game serial (e.g. "0x24C"). */ diff --git a/app/src/main/java/com/runicgateway/app/ui/news/NewsViewModel.kt b/app/src/main/java/com/runicgateway/app/ui/news/NewsViewModel.kt index c4107b3..1deca01 100644 --- a/app/src/main/java/com/runicgateway/app/ui/news/NewsViewModel.kt +++ b/app/src/main/java/com/runicgateway/app/ui/news/NewsViewModel.kt @@ -3,12 +3,14 @@ */ package com.runicgateway.app.ui.news +import androidx.lifecycle.SavedStateHandle import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope import com.runicgateway.app.data.api.dto.PostDto import com.runicgateway.app.data.repository.ContentRepository import com.runicgateway.app.data.repository.ContentRepository.PostCategory import com.runicgateway.app.ui.UiState +import com.runicgateway.app.ui.navigation.Routes import com.runicgateway.app.ui.toUiState import dagger.hilt.android.lifecycle.HiltViewModel import kotlinx.coroutines.flow.MutableStateFlow @@ -21,9 +23,15 @@ import javax.inject.Inject @HiltViewModel class NewsViewModel @Inject constructor( private val contentRepository: ContentRepository, + savedStateHandle: SavedStateHandle, ) : ViewModel() { - private val _category = MutableStateFlow(PostCategory.NEWS) + // Which tab to open on. Absent — every route into this screen except an + // admin's nav override or added link (THEMING_AND_NAV.md §6.2) — is the + // default feed, and so is a slug the app doesn't know. + private val _category = MutableStateFlow( + PostCategory.fromUrlSlug(savedStateHandle[Routes.Args.CATEGORY]) ?: PostCategory.NEWS, + ) val category: StateFlow = _category.asStateFlow() private val _state = MutableStateFlow>>(UiState.Loading) diff --git a/app/src/test/java/com/runicgateway/app/ui/ContentViewModelTest.kt b/app/src/test/java/com/runicgateway/app/ui/ContentViewModelTest.kt index fedb2da..470f222 100644 --- a/app/src/test/java/com/runicgateway/app/ui/ContentViewModelTest.kt +++ b/app/src/test/java/com/runicgateway/app/ui/ContentViewModelTest.kt @@ -38,15 +38,22 @@ class ContentViewModelTest { private val settings = SettingsRepository(api) // ── News hub ────────────────────────────────────────────────────────── + /** No category argument: how every route into the hub but §6.2's arrives. */ + private fun newsViewModel(category: String? = null) = + NewsViewModel( + content, + SavedStateHandle(category?.let { mapOf(Routes.Args.CATEGORY to it) } ?: emptyMap()), + ) + @Test fun newsLoadsSelectedCategory() { api.posts = listOf(PostDto(id = 1, category = "news", title = "Hi")) - val vm = NewsViewModel(content) + val vm = newsViewModel() assertTrue(vm.state.value is UiState.Success) assertEquals(1, (vm.state.value as UiState.Success).data.size) } @Test fun newsSelectCategoryReloads() { - val vm = NewsViewModel(content) + val vm = newsViewModel() api.posts = listOf(PostDto(id = 2, category = "newsletter", title = "N")) vm.selectCategory(ContentRepository.PostCategory.NEWSLETTER) assertEquals(ContentRepository.PostCategory.NEWSLETTER, vm.category.value) @@ -55,7 +62,20 @@ class ContentViewModelTest { @Test fun newsServerErrorIsUiError() { api.error = httpError(500) - assertTrue(NewsViewModel(content).state.value is UiState.Error) + assertTrue(newsViewModel().state.value is UiState.Error) + } + + @Test fun newsOpensOnTheCategoryTheRouteAsksFor() { + // The app's half of an admin's nav override or added link pointing at one of + // the website's three category pages (THEMING_AND_NAV.md §6.2). + val vm = newsViewModel("five-on-friday") + assertEquals(ContentRepository.PostCategory.FIVE_ON_FRIDAY, vm.category.value) + } + + @Test fun newsFallsBackToTheDefaultFeedForAnUnknownCategory() { + // A hand-edited settings row, or a category the site has and the app doesn't. + assertEquals(ContentRepository.PostCategory.NEWS, newsViewModel("bogus").category.value) + assertEquals(ContentRepository.PostCategory.NEWS, newsViewModel().category.value) } // ── Post detail (SavedStateHandle args) ───────────────────────────────── diff --git a/app/src/test/java/com/runicgateway/app/ui/navigation/NavOverridesTest.kt b/app/src/test/java/com/runicgateway/app/ui/navigation/NavOverridesTest.kt new file mode 100644 index 0000000..1fcd70e --- /dev/null +++ b/app/src/test/java/com/runicgateway/app/ui/navigation/NavOverridesTest.kt @@ -0,0 +1,271 @@ +/* + * SPDX-License-Identifier: GPL-3.0-or-later + */ +package com.runicgateway.app.ui.navigation + +import com.runicgateway.app.core.auth.Role +import com.runicgateway.app.core.auth.Session +import com.runicgateway.app.core.auth.SessionUser +import com.runicgateway.app.data.repository.ShardFeature +import com.runicgateway.app.data.repository.ShardFeatures +import kotlinx.serialization.json.JsonObject +import kotlinx.serialization.json.buildJsonObject +import kotlinx.serialization.json.put +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Assert.assertSame +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * The public-nav override merge (THEMING_AND_NAV.md §6): label, order and hidden, + * applied to the coded [APP_MENU] and nothing else. + * + * Two things these tests are really about. **AC-1** — an instance whose admin never + * touched the nav must get the drawer the app shipped with, which here is the + * strongest possible assertion: the same list instance back. And **AC-3** — the + * merge runs before [visibleEntries] and cannot reach past it, so a `hidden: false` + * on a gated row still shows nothing. + */ +class NavOverridesTest { + + private fun nav(vararg items: Pair): JsonObject = + buildJsonObject { for ((path, entry) in items) put(path, entry) } + + private fun entry( + label: String? = null, + order: Int? = null, + hidden: Boolean? = null, + ): JsonObject = buildJsonObject { + label?.let { put("label", it) } + order?.let { put("order", it) } + hidden?.let { put("hidden", it) } + } + + private fun routes(nav: JsonObject?) = applyNavOverrides(APP_MENU, nav).map { it.route } + + /** The public block's routes, in coded order — the first nine of APP_MENU. */ + private val codedPublic = listOf( + Routes.HOME, Routes.NEWS, Routes.WIKI, Routes.SHARD, Routes.SHARD_RULES, + Routes.ATLAS, Routes.SHARD_LEADERBOARDS, Routes.SHARD_MARKET, Routes.page("about"), + ) + + // ── AC-1: the untouched instance ───────────────────────────────────── + + @Test fun noStoredRowReturnsTheCodedMenuItself() { + // Identity, not equality: the drawer of an instance that never edited its + // nav is the shipped one, and nothing was rebuilt to arrive at it. + assertSame(APP_MENU, applyNavOverrides(APP_MENU, null)) + } + + @Test fun anEmptyRowReturnsTheCodedMenuItself() { + assertSame(APP_MENU, applyNavOverrides(APP_MENU, buildJsonObject { })) + } + + @Test fun aRowWithNothingUsableInItReturnsTheCodedMenuItself() { + // A blank label, a non-finite order, `hidden: false`, a path the app has no + // screen for, and a path it maps but doesn't put in the drawer. None of it + // says anything, so none of it may cost the coded menu. + val stored = nav( + "/" to entry(label = " "), + "/site/news" to entry(hidden = false), + "/admin/appearance" to entry(label = "Nope"), + "/site/screenshots" to entry(label = "Shots", order = 0), + "/site/champs" to entry(hidden = true), + ) + + assertSame(APP_MENU, applyNavOverrides(APP_MENU, stored)) + } + + @Test fun aMalformedEntryIsDroppedAndItsNeighbourKept() { + val stored = buildJsonObject { + put("/site/news", "not an object") + put("/wiki", entry(label = "Codex")) + } + + val merged = applyNavOverrides(APP_MENU, stored) + + assertEquals(codedPublic, merged.take(9).map { it.route }) + assertEquals("Codex", merged.first { it.route == Routes.WIKI }.label) + assertNull(merged.first { it.route == Routes.NEWS }.label) + } + + // ── Labels ─────────────────────────────────────────────────────────── + + @Test fun aLabelOverridesTheBundledString() { + val merged = applyNavOverrides(APP_MENU, nav("/site/shard" to entry(label = " The Realm "))) + + val shard = merged.first { it.route == Routes.SHARD } + assertEquals("The Realm", shard.label) + // The override lands on `label` and nothing else — the gates are untouched. + assertEquals(ShardFeature.STATUS, shard.feature) + assertEquals(MenuAccess.PUBLIC, shard.access) + assertEquals(codedPublic, merged.take(9).map { it.route }) + } + + @Test fun aNonStringLabelIsIgnored() { + val stored = buildJsonObject { put("/wiki", buildJsonObject { put("label", 7) }) } + + assertSame(APP_MENU, applyNavOverrides(APP_MENU, stored)) + } + + // ── Hidden ─────────────────────────────────────────────────────────── + + @Test fun hiddenDropsTheRow() { + val routes = routes(nav("/site/market" to entry(hidden = true))) + + assertTrue(Routes.SHARD_MARKET !in routes) + assertEquals(APP_MENU.size - 1, routes.size) + } + + @Test fun homeCanBeHidden() { + // Mirrors the website, where `/` is hideable too. Home stays the NavHost's + // start destination and stays reachable by back-press; the app does not + // invent a policy the site doesn't have. + val routes = routes(nav("/" to entry(hidden = true))) + + assertTrue(Routes.HOME !in routes) + } + + @Test fun hiddenFalseHidesNothing() { + assertSame(APP_MENU, applyNavOverrides(APP_MENU, nav("/site/market" to entry(hidden = false)))) + } + + @Test fun hiddenWinsOverALabelOnTheSameRow() { + val routes = routes(nav("/wiki" to entry(label = "Codex", hidden = true))) + + assertTrue(Routes.WIKI !in routes) + } + + // ── Order ──────────────────────────────────────────────────────────── + + @Test fun anExplicitOrderMovesTheRowWithinThePublicBlock() { + // The website's own indices: About is 15 and Home is 0, so swapping them + // is what an admin dragging About to the top writes. + val routes = routes( + nav( + "/site/about" to entry(order = 0), + "/" to entry(order = 15), + ), + ) + + assertEquals( + listOf( + Routes.page("about"), Routes.NEWS, Routes.WIKI, Routes.SHARD, Routes.SHARD_RULES, + Routes.ATLAS, Routes.SHARD_LEADERBOARDS, Routes.SHARD_MARKET, Routes.HOME, + ), + routes.take(9), + ) + } + + @Test fun anUntouchedRowKeepsItsPlaceOnTheWebsitesNumberLine() { + // The tie-break that needs the website's order rather than the app's: an + // explicit 5 meets Wiki's implicit 5 (its index in the site's nav, where + // the three news categories sit between News and Wiki). Explicit wins. + val routes = routes(nav("/site/about" to entry(order = 5))) + + assertEquals( + listOf(Routes.HOME, Routes.NEWS, Routes.page("about"), Routes.WIKI), + routes.take(4), + ) + } + + @Test fun theAppsOwnRowsKeepTheirCodedOrderAfterThePublicBlock() { + // Contact, Account, Notifications, the three player groups and the four + // staff rows have no website counterpart to be reordered against (§6.2). + val tail = APP_MENU.drop(9).map { it.route } + + val merged = routes(nav("/site/about" to entry(order = 0))) + + assertEquals(tail, merged.drop(9)) + } + + @Test fun reorderingAndHidingCompose() { + val routes = routes( + nav( + "/site/about" to entry(order = 0), + "/" to entry(hidden = true), + ), + ) + + assertEquals(Routes.page("about"), routes.first()) + assertTrue(Routes.HOME !in routes) + } + + // ── The two stored shapes ──────────────────────────────────────────── + + @Test fun theWrappedShapeIsRead() { + // Website phase 10 wraps the map as {items, sections, links} without + // migrating what phases 6-8 stored bare, so both shapes are live. + val stored = buildJsonObject { + put("items", nav("/wiki" to entry(label = "Codex"))) + put("sections", buildJsonObject { }) + put("links", buildJsonObject { }) + } + + val merged = applyNavOverrides(APP_MENU, stored) + + assertEquals("Codex", merged.first { it.route == Routes.WIKI }.label) + } + + @Test fun sectionsAndLinksAreIgnoredInThisPhase() { + // Phase 6 renders them. Until then their presence must not disturb the + // items merge — and an `items` map that says nothing still returns the + // coded menu even when sections exist beside it. + val stored = buildJsonObject { + put("items", buildJsonObject { }) + put("sections", buildJsonObject { put("id", "lore") }) + } + + assertSame(APP_MENU, applyNavOverrides(APP_MENU, stored)) + } + + // ── AC-3: the merge cannot reach past the gates ────────────────────── + + @Test fun anOverrideCannotUnhideAFeatureGatedRow() { + val stored = nav( + "/site/market" to entry(label = "Bazaar", hidden = false, order = 0), + ) + + val visible = visibleEntries( + applyNavOverrides(APP_MENU, stored), + Session.SignedIn(SessionUser(id = 1, username = "u", role = Role.ADMIN)), + ShardFeatures(level = "admin", visible = setOf(ShardFeature.STATUS)), + ).map { it.route } + + // Relabeled and moved to the front, and still not shown: the shard does not + // publish the market, and an admin does not outrank that. + assertTrue(Routes.SHARD_MARKET !in visible) + assertTrue(Routes.SHARD in visible) + } + + @Test fun anOverrideCannotUnhideARoleGatedRow() { + val stored = nav("/" to entry(order = 99)) + + val visible = visibleEntries( + applyNavOverrides(APP_MENU, stored), + Session.SignedOut, + features = null, + ).map { it.route } + + assertTrue(Routes.ACCOUNT !in visible) + assertTrue(Routes.ADMIN_DASHBOARD !in visible) + assertTrue(Routes.PLAYER_CHARACTERS !in visible) + } + + @Test fun theGatesRunOnTheMergedListNotTheCodedOne() { + // Hiding is subtractive on top of the gates, so the two compose: the row an + // admin hid is gone, and so is the row this caller may not see. + val stored = nav("/wiki" to entry(hidden = true)) + + val visible = visibleEntries( + applyNavOverrides(APP_MENU, stored), + Session.SignedOut, + ShardFeatures(level = "anonymous", visible = setOf(ShardFeature.STATUS)), + ).map { it.route } + + assertTrue(Routes.WIKI !in visible) + assertTrue(Routes.SHARD_MARKET !in visible) + assertTrue(Routes.HOME in visible) + } +} diff --git a/app/src/test/java/com/runicgateway/app/ui/navigation/NavPathsTest.kt b/app/src/test/java/com/runicgateway/app/ui/navigation/NavPathsTest.kt new file mode 100644 index 0000000..31a91d3 --- /dev/null +++ b/app/src/test/java/com/runicgateway/app/ui/navigation/NavPathsTest.kt @@ -0,0 +1,119 @@ +/* + * SPDX-License-Identifier: GPL-3.0-or-later + */ +package com.runicgateway.app.ui.navigation + +import com.runicgateway.app.data.repository.ContentRepository.PostCategory +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * The website path → app route table (THEMING_AND_NAV.md §6.2). + * + * This is the milestone's one piece of cross-repo coupling, so the tests are + * mostly about the table's *shape* — that it stays complete, unambiguous, and + * honest about which rows the app actually surfaces in its drawer. + */ +class NavPathsTest { + + @Test fun everyWebsiteNavPathIsMapped() { + // The sixteen rows of SiteHeader.jsx's NAV, quoted in NavPaths.kt. If the + // site adds one, this is the test that says so — a path with no mapping is + // silently unresolvable in phase 6's link handling. + assertEquals(16, WEBSITE_PUBLIC_NAV.size) + assertEquals(WEBSITE_PUBLIC_NAV.size, WEB_PATH_TO_ROUTE.size) + } + + @Test fun everyMappedRouteIsDistinct() { + // WEB_ROUTE_ORDER is keyed by route, so a duplicate would silently drop a + // row's position from the sort. + assertEquals(WEBSITE_PUBLIC_NAV.size, WEBSITE_PUBLIC_NAV.map { it.route }.toSet().size) + assertEquals(WEBSITE_PUBLIC_NAV.size, WEB_ROUTE_ORDER.size) + } + + @Test fun theWebsitesOrderIsPreserved() { + // Load-bearing: a stored `order` is an index into this list. + assertEquals(0, WEB_ROUTE_ORDER[Routes.HOME]) + assertEquals(1, WEB_ROUTE_ORDER[Routes.NEWS]) + assertEquals(5, WEB_ROUTE_ORDER[Routes.WIKI]) + assertEquals(15, WEB_ROUTE_ORDER[Routes.page("about")]) + } + + @Test fun theNineDrawerRowsAreTheIntersectionWithAppMenu() { + // Nine of the sixteen have a drawer row. The other seven are mapped but not + // surfaced — three news category tabs and the four Shard hub boards — and + // an override for one of them is ignored rather than obeyed (§6.2). + val coded = APP_MENU.map { it.route }.toSet() + val surfaced = WEBSITE_PUBLIC_NAV.filter { it.route in coded }.map { it.path } + + assertEquals( + listOf( + "/", "/site/news", "/wiki", "/site/shard", "/site/rules", + "/site/atlas", "/site/leaderboards", "/site/market", "/site/about", + ), + surfaced, + ) + } + + @Test fun theSevenUnsurfacedPathsStillResolveToAScreen() { + // Phase 6's added links resolve against the same table, and there a category + // tab or a hub board is a perfectly good destination. + val unsurfaced = listOf( + "/site/screenshots", "/site/five-on-friday", "/site/newsletter", + "/site/champs", "/site/guilds", "/site/governors", "/site/houses", + ) + + assertTrue(unsurfaced.all { appRouteForWebPath(it) != null }) + assertTrue(unsurfaced.none { appRouteForWebPath(it) in APP_MENU.map { e -> e.route } }) + } + + @Test fun theNewsCategoriesMapToTheirTab() { + assertEquals("news?category=screenshots", appRouteForWebPath("/site/screenshots")) + assertEquals("news?category=five-on-friday", appRouteForWebPath("/site/five-on-friday")) + assertEquals("news?category=newsletter", appRouteForWebPath("/site/newsletter")) + // The plain news path is the un-argumented route, so it matches the drawer's + // coded row and opens the default tab. + assertEquals(Routes.NEWS, appRouteForWebPath("/site/news")) + } + + @Test fun theCategoryRouteMatchesTheNavHostPattern() { + // The pattern the NavHost declares and the value callers navigate to have to + // agree on the query key, or the argument arrives as null and the screen + // silently opens the default tab. + assertEquals("news?category={category}", Routes.NEWS_ROUTE) + assertTrue(Routes.NEWS_ROUTE.startsWith("${Routes.NEWS}?")) + for (category in PostCategory.entries) { + assertEquals("${Routes.NEWS}?category=${category.urlSlug}", Routes.news(category)) + } + } + + @Test fun theRoutePatternStripsToTheTopLevelRoute() { + // How RunicApp recognizes the News destination: `destination.route` is the + // pattern, and the drawer's row is the bare route. + assertEquals(Routes.NEWS, Routes.NEWS_ROUTE.substringBefore('?')) + assertEquals(Routes.NEWS, Routes.news(PostCategory.NEWSLETTER).substringBefore('?')) + } + + // ── Lookup hygiene ─────────────────────────────────────────────────── + + @Test fun anUnknownPathResolvesToNothing() { + assertNull(appRouteForWebPath("/admin/appearance")) + assertNull(appRouteForWebPath("/site/news/some-post")) + assertNull(appRouteForWebPath("https://elsewhere.example/")) + } + + @Test fun blankAndNullResolveToNothing() { + assertNull(appRouteForWebPath(null)) + assertNull(appRouteForWebPath("")) + assertNull(appRouteForWebPath(" ")) + } + + @Test fun aTrailingSlashIsTolerated() { + // A hand-edited settings row may carry one; the root is left alone. + assertEquals(Routes.WIKI, appRouteForWebPath("/wiki/")) + assertEquals(Routes.SHARD, appRouteForWebPath(" /site/shard/ ")) + assertEquals(Routes.HOME, appRouteForWebPath("/")) + } +} -- 2.49.1