fix(notifications): resolve an item's relative url, and document the CI trigger
Some checks failed
PR Checks / android-build (pull_request) Failing after 42m5s
Some checks failed
PR Checks / android-build (pull_request) Failing after 42m5s
Two things the live rig found, and the README half of the trigger change. Phase 7 specifies an inbox item's `url` is RELATIVE-ONLY and validates it as such — right for a browser already on the site, a dead link on a phone. The first cut here only opened `http(s)`-prefixed strings, so on the rig every link in the inbox did nothing at all. `InboxViewModel.linkFor` now resolves against the configured base with OkHttp's `HttpUrl.resolve`, which absolutises the path and returns null for anything that would not end up http(s) — so a `javascript:` or `intent:` url in a notification body opens nothing. Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -118,11 +118,11 @@ fun InboxScreen(
|
||||
onEndReached = viewModel::loadMore,
|
||||
onOpen = { item ->
|
||||
viewModel.markRead(item.id)
|
||||
// The url is the site's own page for the item, and most items
|
||||
// have none — an inbox row is complete on its own. Anything
|
||||
// that is not http(s) is not opened at all.
|
||||
item.url?.takeIf { it.startsWith("http://") || it.startsWith("https://") }
|
||||
?.let { WebHandoff.open(context, it) }
|
||||
// Most items have no url at all — an inbox row is complete on
|
||||
// its own — and the ones that do carry a SITE-RELATIVE path,
|
||||
// so the view model resolves it against the configured shard
|
||||
// before anything is opened.
|
||||
viewModel.linkFor(item)?.let { WebHandoff.open(context, it) }
|
||||
},
|
||||
)
|
||||
}
|
||||
|
||||
@@ -201,6 +201,25 @@ class InboxViewModel @Inject constructor(
|
||||
cacheCurrent()
|
||||
}
|
||||
|
||||
/**
|
||||
* The absolute link for an item, or null when it has none this app can open.
|
||||
*
|
||||
* **An item's `url` is SITE-RELATIVE** — `/guilds/the-silver-anvil/forum/403`
|
||||
* is what the server writes, because it is rendered from the template's button
|
||||
* block for a browser that is already on the site. A phone is not, so it has to
|
||||
* be resolved against the configured base or every link in the inbox is dead;
|
||||
* the live rig is what caught that.
|
||||
*
|
||||
* `HttpUrl.resolve` does both jobs: it absolutises a relative path and it
|
||||
* returns null for anything that would not end up as http(s) — a `javascript:`
|
||||
* or `intent:` url in a notification body opens nothing at all.
|
||||
*/
|
||||
fun linkFor(item: NotificationItemDto): String? {
|
||||
val raw = item.url?.trim().orEmpty()
|
||||
if (raw.isEmpty()) return null
|
||||
return baseUrlHolder.current?.resolve(raw)?.toString()
|
||||
}
|
||||
|
||||
/**
|
||||
* Keep the snapshot in step with a local read.
|
||||
*
|
||||
|
||||
@@ -166,4 +166,31 @@ class InboxViewModelTest {
|
||||
// bring the badge back on the next cold open.
|
||||
assertEquals(writesAfterLoad + 1, cache.writes)
|
||||
}
|
||||
|
||||
// ── The item link (found by the live rig, not by a test) ──────────────
|
||||
|
||||
@Test fun aSiteRelativeUrlIsResolvedAgainstTheShard() {
|
||||
// What the server actually writes: the template's button block renders a
|
||||
// path, because on the web the reader is already on the site.
|
||||
val vm = viewModel()
|
||||
val item = item(1).copy(url = "/guilds/the-silver-anvil/forum/403")
|
||||
assertEquals("https://shard.example/guilds/the-silver-anvil/forum/403", vm.linkFor(item))
|
||||
}
|
||||
|
||||
@Test fun anAbsoluteUrlIsLeftAlone() {
|
||||
val vm = viewModel()
|
||||
assertEquals("https://elsewhere.example/x", vm.linkFor(item(1).copy(url = "https://elsewhere.example/x")))
|
||||
}
|
||||
|
||||
@Test fun anItemWithNoUrlHasNoLink() {
|
||||
val vm = viewModel()
|
||||
assertNull(vm.linkFor(item(1)))
|
||||
assertNull(vm.linkFor(item(1).copy(url = " ")))
|
||||
}
|
||||
|
||||
@Test fun aUrlThatCouldNotBeOpenedSafelyResolvesToNothing() {
|
||||
val vm = viewModel()
|
||||
assertNull(vm.linkFor(item(1).copy(url = "javascript:alert(1)")))
|
||||
assertNull(vm.linkFor(item(1).copy(url = "intent://evil#Intent;end")))
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user