From 334e4f8353560eb2392620096f50059a5336c5c8 Mon Sep 17 00:00:00 2001 From: Coracle-Bot Date: Wed, 30 Sep 2026 16:27:17 +0000 Subject: [PATCH] File a comment under the content item it answers rather than the room it carries --- e2e/USER_STORIES.md | 12 +++++++ e2e/specs/notifications.spec.ts | 60 +++++++++++++++++++++++++++++++++ src/app/content.ts | 15 +++++++++ src/app/notifications.ts | 30 +++++++---------- src/app/routes.ts | 30 ++++------------- 5 files changed, 105 insertions(+), 42 deletions(-) diff --git a/e2e/USER_STORIES.md b/e2e/USER_STORIES.md index d5924a76..ef86b058 100644 --- a/e2e/USER_STORIES.md +++ b/e2e/USER_STORIES.md @@ -1729,6 +1729,18 @@ Acceptance: - A thread bob starts in the same room raises none, however much of it reaches her client. +### US-137 — Reach a thread from the inbox + +As alice, I want an unread reply to a thread to lead me to the thread, so that +the inbox never drops me in a room chat to go looking for it. + +Acceptance: + +- A comment bob writes on a thread in a room alice belongs to leaves the room's + inbox row showing the room's own last message. +- The same comment counts toward the space's unread thread activity, and the + thread it answers carries the unread dot on the threads board. + ### US-105 — Land on the home page As a new user, I want the home page to route me somewhere useful, so that I'm diff --git a/e2e/specs/notifications.spec.ts b/e2e/specs/notifications.spec.ts index 80d01324..55e88cbb 100644 --- a/e2e/specs/notifications.spec.ts +++ b/e2e/specs/notifications.spec.ts @@ -702,6 +702,66 @@ test("US-112 see which threads are unread", async ({seed, as}) => { await expect(unreadDot(threadsNav)).toHaveCount(0) }) +test("US-137 reach a thread from the inbox activity its comment raised", async ({seed, as}) => { + const scenario = await seed(({relay, user, at}) => { + const space = relay("space") + + space.room("general", {name: "General"}) + space.join(user.alice, "general") + space.join(user.bob, "general") + space.message(user.bob, "general", "the server is on fire", at(3, HOUR)) + + const topic = seedThread(space, user.bob, "where is the sextant", at(2, HOUR)) + + space.event( + user.bob, + () => + space + .kind(Comment) + .writer() + // publishComment carries the root's room forward, so a comment on a thread is tagged h too. + .setRoom(space.url, "general") + .setRootFromEvent(topic.event) + .setParentFromEvent(topic.event) + .setContent("starboard locker, under the charts") + .renderTemplate(), + at(1, HOUR), + ) + }) + + const space = scenario.space("space") + const alice = await as(users.alice, "/home") + + // The room's row is its own chat, whatever the newest thing tagged for the room happens to be + const conversation = alice.getByRole("link").filter({hasText: "the server is on fire"}) + + await expect(conversation).toBeVisible() + await expect(conversation).toContainText("General") + await expect(alice.getByRole("link").filter({hasText: "starboard locker"})).toHaveCount(0) + + // The comment is activity on the thread, so the space's thread count is what it raises + await expect(alice.getByRole("link").filter({hasText: "1 thread"})).toBeVisible() + + await conversation.click() + + await expect(alice).toHaveURL(pathPattern(roomPath(space.url, "general"))) + + await contentNavItem(alice, "Threads").click() + + const topic = alice.getByRole("row").filter({hasText: "where is the sextant"}) + + await expect(topic).toBeVisible() + + // syncChecked marks the page read 300ms later and latestActivityByPath is throttled to a second. + await alice.waitForTimeout(1500) + + await expect(unreadDot(topic)).toBeVisible() + + await topic.click() + + await expect(alice.getByText("starboard locker, under the charts")).toBeVisible() +}) + const seedPoll = (space: SeededSpace, user: TestUser, title: string, createdAt: number) => space.event( user, diff --git a/src/app/content.ts b/src/app/content.ts index 08472288..77fbbca7 100644 --- a/src/app/content.ts +++ b/src/app/content.ts @@ -97,6 +97,21 @@ export const makeDeleteFilter = (kinds: number[], extra: Filter = {}) => ({ ...extra, }) +// What an event is activity on: the root a comment sits under, or the event itself. +export const getActivityTarget = (event: TrustedEvent) => { + if (event.kind === COMMENT) { + const {kind, id, address} = reader(Comment)(event).root() + const rootKind = parseInt(kind || "") + const idOrAddress = address || id + + if (rootKind && idOrAddress) { + return {kind: rootKind, idOrAddress} + } + } + + return {kind: event.kind, idOrAddress: getIdOrAddress(event)} +} + // An event is active as of its own creation or the newest comment on it, keyed by address when addressable. export const partitionByActivity = (kind: number, events: TrustedEvent[]) => { const [items, comments] = partition(spec({kind}), events) diff --git a/src/app/notifications.ts b/src/app/notifications.ts index 2b7a17b2..67fca107 100644 --- a/src/app/notifications.ts +++ b/src/app/notifications.ts @@ -11,18 +11,18 @@ import { sortBy, throttle, parseJson, + spec, gt, int, HOUR, } from "@welshman/lib" import type {TrustedEvent} from "@welshman/util" -import {getIdOrAddress, sortEventsDesc, tagSpec, tagValue, COMMENT, MESSAGE} from "@welshman/util" -import {Comment} from "@welshman/domain" +import {sortEventsDesc, tagSpec, tagValue, MESSAGE} from "@welshman/util" import {synced, throttled, withGetter} from "@welshman/store" import {Events, Relays} from "@welshman/app" -import {app, fromApp, reader} from "@app/core" +import {app, fromApp} from "@app/core" import {makeRoomPath, makeSpaceChatPath, makeChatPath, makeContentPath} from "@app/routes" -import {CONTENT_KINDS, makeCommentFilter} from "@app/content" +import {CONTENT_KINDS, getActivityTarget, makeCommentFilter} from "@app/content" import {getIsMuted, userSettingsValues} from "@app/settings" import {Chats} from "@app/chats" import {dufflepud, DUFFLEPUD_URL} from "@app/env" @@ -159,20 +159,12 @@ export const syncCheckedRemote = () => { // Derived notifications state -// The content item an event belongs to - either the content event itself, or the root of a comment +// The content item an event belongs to, when it belongs to one at all. const getContentTarget = (event: TrustedEvent) => { - if (CONTENT_KINDS.includes(event.kind)) { - return {kind: event.kind, idOrAddress: getIdOrAddress(event)} - } + const target = getActivityTarget(event) - if (event.kind === COMMENT) { - const root = reader(Comment)(event).root() - const kind = parseInt(root.kind || "") - const idOrAddress = root.address || root.id - - if (CONTENT_KINDS.includes(kind) && idOrAddress) { - return {kind, idOrAddress} - } + if (CONTENT_KINDS.includes(target.kind)) { + return target } } @@ -239,9 +231,11 @@ export const latestActivityByPath = derived( for (const url of $activeSpaceUrls) { const events = sortEventsDesc((eventsByIdByUrl.get(url) || new Map()).values()) + // A chat is its own messages; content and its comments carry the room h but belong to the item. + const messages = events.filter(spec({kind: MESSAGE})) if (spaceSupportsRooms(url)) { - for (const [h, [event]] of groupBy(e => tagValue(tagSpec("h"), e.tags), events)) { + for (const [h, [event]] of groupBy(e => tagValue(tagSpec("h"), e.tags), messages)) { // A muted room is left out entirely, so it can't light up its own badge or the space's if (h && !getIsMuted($settings, url, h)) { const path = makeRoomPath(url, h) @@ -250,7 +244,7 @@ export const latestActivityByPath = derived( } } } else { - const event = first(events) + const event = first(messages) if (event) { const path = makeSpaceChatPath(url) diff --git a/src/app/routes.ts b/src/app/routes.ts index a1d1eaf3..5f3614ee 100644 --- a/src/app/routes.ts +++ b/src/app/routes.ts @@ -6,7 +6,6 @@ import type {Maybe} from "@welshman/lib" import type {TrustedEvent} from "@welshman/util" import { CLASSIFIED, - COMMENT, EVENT_TIME, LONG_FORM, MESSAGE, @@ -15,18 +14,16 @@ import { POLL, THREAD, ZAP_GOAL, - getIdOrAddress, hexTags, tagSpec, tagValue, tagValues, } from "@welshman/util" -import {Comment} from "@welshman/domain" -import {app, messagingRelayLists, reader, user} from "@app/core" +import {app, messagingRelayLists, user} from "@app/core" import {makeChatId} from "@app/chats" import {entityLink, PLATFORM_URL, PLATFORM_RELAYS} from "@app/env" import {decodeRelay, encodeRelay} from "@app/relays" -import {DM_KINDS} from "@app/content" +import {DM_KINDS, getActivityTarget} from "@app/content" import {navigate, pushModal} from "@app/modal" import type {NavigateOptions} from "@app/modal" import type {NotePointer} from "@app/social" @@ -212,33 +209,18 @@ export const makeEventPath = (event: TrustedEvent, urls: string[]): Maybe 0) { const url = urls[0] + const {kind, idOrAddress} = getActivityTarget(event) - if (event.kind === MESSAGE) { + // A comment inherits the h of the message it answers, so both anchor the same transcript. + if (kind === MESSAGE) { return makeMessagePath(url, event) } - const path = makeContentPath(url, event.kind, getIdOrAddress(event)) + const path = makeContentPath(url, kind, idOrAddress) if (path) { return path } - - if (event.kind === COMMENT) { - const root = reader(Comment)(event).root() - const rootIdOrAddress = root.address ?? root.id - - if (root.kind && rootIdOrAddress) { - if (parseInt(root.kind) === MESSAGE) { - return makeMessagePath(url, event) - } - - const rootPath = makeContentPath(url, parseInt(root.kind), rootIdOrAddress) - - if (rootPath) { - return rootPath - } - } - } } // A note belongs to no space, so its path carries the relays it was found on.