From 28c2dfc4127096ae38a77354884b1ce4ad2dcda9 Mon Sep 17 00:00:00 2001 From: Adron Hall Date: Sun, 6 Sep 2026 22:47:12 -0700 Subject: [PATCH 1/4] feat(domain): add MessagePermalink as the single web-link definition The "Link" message action (#27) needs a canonical web URL for a post, but the only permalink logic in the tree was private to CreateIssueFromMessageViewModel. Promote it to InterlinedDomain so the row's Link action, Push & Comment, and the create-issue body all agree on one shape. The builder is stricter than the code it replaces: it trims the id, returns nil for a blank one rather than emitting a half-formed URL, and percent-encodes "/" so a malformed id can never forge extra path segments outside /messages/. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Tm5htvRQsWxNeq7cmaAakW --- .../CreateIssueFromMessageViewModel.swift | 13 +- .../Models/MessagePermalink.swift | 52 ++++++++ .../MessagePermalinkTests.swift | 120 ++++++++++++++++++ 3 files changed, 179 insertions(+), 6 deletions(-) create mode 100644 Packages/InterlinedDomain/Sources/InterlinedDomain/Models/MessagePermalink.swift create mode 100644 Packages/InterlinedDomain/Tests/InterlinedDomainTests/MessagePermalinkTests.swift diff --git a/App/Features/Timeline/CreateIssueFromMessageViewModel.swift b/App/Features/Timeline/CreateIssueFromMessageViewModel.swift index 4778253..dcebf2c 100644 --- a/App/Features/Timeline/CreateIssueFromMessageViewModel.swift +++ b/App/Features/Timeline/CreateIssueFromMessageViewModel.swift @@ -6,6 +6,11 @@ // a link back to the post), and creates the issue in the chosen repo via the // domain `GitHubServicing` surface. // +// The permalink woven into the issue body comes from the shared +// `MessagePermalink` builder in `InterlinedDomain` (GitHub #27) rather than a +// private copy here, so the row's Link action, Push & Comment, and this issue +// body all agree on one URL shape. +// // Like `GitHubIssuesViewModel`, the unlinked-account state is first-class: // `GitHubServiceError.notLinked` flips `linkState` so the view can show a // "Link GitHub" CTA instead of a raw error. Depends only on `GitHubServicing` @@ -50,12 +55,12 @@ final class CreateIssueFromMessageViewModel { init( github: GitHubServicing, message: Message, - webBaseURL: URL = URL(string: "https://interlinedlist.com")! + webBaseURL: URL = MessagePermalink.defaultWebBaseURL ) { self.github = github self.message = message self.title = Self.suggestedTitle(from: message) - self.body = Self.suggestedBody(from: message, permalink: Self.permalink(for: message, base: webBaseURL)) + self.body = Self.suggestedBody(from: message, permalink: message.permalink(base: webBaseURL)) } // MARK: - Loading @@ -139,10 +144,6 @@ final class CreateIssueFromMessageViewModel { return parts.joined(separator: "\n\n") } - static func permalink(for message: Message, base: URL) -> URL? { - base.appendingPathComponent("messages").appendingPathComponent(message.id) - } - // MARK: - Helpers private func handle(_ error: GitHubServiceError) { diff --git a/Packages/InterlinedDomain/Sources/InterlinedDomain/Models/MessagePermalink.swift b/Packages/InterlinedDomain/Sources/InterlinedDomain/Models/MessagePermalink.swift new file mode 100644 index 0000000..fb8d7ae --- /dev/null +++ b/Packages/InterlinedDomain/Sources/InterlinedDomain/Models/MessagePermalink.swift @@ -0,0 +1,52 @@ +import Foundation + +/// Canonical web permalink for a message ("Link" in the web message actions). +/// +/// The link is a pure client-side projection — there is no API route that +/// hands back a message URL — so the shape lives here in the domain rather +/// than being re-derived by each feature that needs it. Three App-layer +/// surfaces consume it: the message row's Link action, the "Push & Comment" +/// body, and `CreateIssueFromMessageViewModel`'s issue body, which previously +/// owned a private copy of this logic. +/// +/// Shape: `/messages/` — matching the web app's own route. +public enum MessagePermalink { + + /// Production web front-end. Overridable at every call site so tests and + /// a future staging build never hard-code the live host. + public static let defaultWebBaseURL = URL(string: "https://interlinedlist.com")! + + /// Path-segment-safe character set: `urlPathAllowed` still permits `/`, + /// which would let an id containing a slash silently forge extra path + /// segments. Removing it forces such an id to percent-encode instead. + private static let pathSegmentAllowed: CharacterSet = { + var set = CharacterSet.urlPathAllowed + set.remove("/") + return set + }() + + /// Builds the permalink for `id`, or `nil` when the id is empty / blank + /// or cannot be encoded. Returning `nil` rather than a half-formed URL + /// lets the UI hide the affordance instead of offering a broken link. + public static func url(forMessageID id: String, base: URL = defaultWebBaseURL) -> URL? { + let trimmed = id.trimmingCharacters(in: .whitespacesAndNewlines) + guard !trimmed.isEmpty else { return nil } + guard let encoded = trimmed.addingPercentEncoding(withAllowedCharacters: pathSegmentAllowed) else { + return nil + } + // Normalise the base so a caller-supplied trailing slash can't produce + // a double slash in the middle of the path. + var stem = base.absoluteString + while stem.hasSuffix("/") { stem.removeLast() } + return URL(string: "\(stem)/messages/\(encoded)") + } +} + +public extension Message { + + /// This message's canonical web permalink, or `nil` when the id can't + /// form one. See `MessagePermalink`. + func permalink(base: URL = MessagePermalink.defaultWebBaseURL) -> URL? { + MessagePermalink.url(forMessageID: id, base: base) + } +} diff --git a/Packages/InterlinedDomain/Tests/InterlinedDomainTests/MessagePermalinkTests.swift b/Packages/InterlinedDomain/Tests/InterlinedDomainTests/MessagePermalinkTests.swift new file mode 100644 index 0000000..88565db --- /dev/null +++ b/Packages/InterlinedDomain/Tests/InterlinedDomainTests/MessagePermalinkTests.swift @@ -0,0 +1,120 @@ +import XCTest +@testable import InterlinedDomain + +/// BDD coverage for `MessagePermalink` (GitHub #27 — the "Link" message +/// action). The builder is pure and synchronous, so the quartet is +/// happy / invalid / upstream-shape / boundary with no service doubles. +final class MessagePermalinkTests: XCTestCase { + + // MARK: - Happy path + + func test_givenWellFormedID_whenBuildingPermalink_thenUsesMessagesRoute() { + // Given a normal server-issued id. + let id = "cmg1a2b3c4d5" + + // When we build the permalink against the production base. + let url = MessagePermalink.url(forMessageID: id) + + // Then it matches the web app's own /messages/ route. + XCTAssertEqual(url?.absoluteString, "https://interlinedlist.com/messages/cmg1a2b3c4d5") + } + + func test_givenMessage_whenAskedForPermalink_thenMatchesBuilder() { + // Given a domain message. + let message = Self.makeMessage(id: "abc123") + + // When we ask the message itself. + let fromMessage = message.permalink() + + // Then it agrees with the standalone builder — one definition, not two. + XCTAssertEqual(fromMessage, MessagePermalink.url(forMessageID: "abc123")) + } + + // MARK: - Invalid input + + func test_givenEmptyID_whenBuildingPermalink_thenReturnsNil() { + // Given an empty id (a message that never round-tripped the server). + // When / Then — no half-formed URL is produced; the UI hides the action. + XCTAssertNil(MessagePermalink.url(forMessageID: "")) + } + + func test_givenWhitespaceOnlyID_whenBuildingPermalink_thenReturnsNil() { + // Given an id that is only whitespace. + // When / Then — treated the same as empty rather than linking to /messages/%20. + XCTAssertNil(MessagePermalink.url(forMessageID: " \n ")) + } + + // MARK: - Upstream shape (caller-supplied base) + + func test_givenBaseWithTrailingSlash_whenBuildingPermalink_thenNoDoubleSlash() { + // Given a base URL a caller wrote with a trailing slash. + let base = URL(string: "https://staging.interlinedlist.com/")! + + // When we build against it. + let url = MessagePermalink.url(forMessageID: "xyz", base: base) + + // Then the path is normalised rather than containing "//messages". + XCTAssertEqual(url?.absoluteString, "https://staging.interlinedlist.com/messages/xyz") + } + + func test_givenBaseWithSubpath_whenBuildingPermalink_thenSubpathIsPreserved() { + // Given a base that already carries a path prefix. + let base = URL(string: "https://example.test/app")! + + // When we build against it. + let url = MessagePermalink.url(forMessageID: "xyz", base: base) + + // Then the prefix survives — the builder appends, it does not replace. + XCTAssertEqual(url?.absoluteString, "https://example.test/app/messages/xyz") + } + + // MARK: - Boundary + + func test_givenIDNeedingPercentEncoding_whenBuildingPermalink_thenEncoded() { + // Given an id carrying characters that are illegal in a path segment. + let id = "a b#c" + + // When we build the permalink. + let url = MessagePermalink.url(forMessageID: id) + + // Then they are percent-encoded rather than truncating the URL at the "#". + XCTAssertEqual(url?.absoluteString, "https://interlinedlist.com/messages/a%20b%23c") + } + + func test_givenIDContainingSlash_whenBuildingPermalink_thenSlashIsEncodedNotForged() { + // Given a hostile / malformed id containing a path separator. + let id = "abc/../admin" + + // When we build the permalink. + let url = MessagePermalink.url(forMessageID: id) + + // Then the slashes are encoded — the id can never forge extra path + // segments, so the link always points inside /messages/. + XCTAssertEqual(url?.absoluteString, "https://interlinedlist.com/messages/abc%2F..%2Fadmin") + } + + func test_givenIDWithSurroundingWhitespace_whenBuildingPermalink_thenTrimmed() { + // Given an id padded by whitespace. + // When / Then — trimmed, not encoded as %20 padding. + XCTAssertEqual( + MessagePermalink.url(forMessageID: " abc ")?.absoluteString, + "https://interlinedlist.com/messages/abc" + ) + } + + // MARK: - Helpers + + private static func makeMessage(id: String) -> Message { + Message( + id: id, + author: UserSummary(id: "u1", username: "adron", displayName: "Adron"), + text: "hello", + createdAt: Date(timeIntervalSince1970: 0), + updatedAt: Date(timeIntervalSince1970: 0), + visibility: .public, + digCount: 0, + didDig: false, + repostCount: 0 + ) + } +} From d8e7476c6058b5af3105d8e7fd579ff7fd0bc8a2 Mon Sep 17 00:00:00 2001 From: Adron Hall Date: Sun, 6 Sep 2026 22:49:37 -0700 Subject: [PATCH 2/4] refactor(timeline): collapse MessageRowView handlers into MessageRowActions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MessageRowView took eight individual closure parameters and #27 adds three more (Reply, Push, Link), which pushes the call sites past readable. Gather them into one MessageRowActions value instead. Behaviour-neutral: every handler stays optional with the same "nil means the row hides that affordance" contract, and the row stays passive. SearchRootView needs no change — it already relied on the defaults, which are now `.none`. `none` is computed rather than a static let: the handlers are non-Sendable closures, so a shared static instance fails Swift 6 concurrency checking. Verified: App suite 627/627 green, unchanged from before the refactor. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Tm5htvRQsWxNeq7cmaAakW --- App/Features/Timeline/MessageDetailView.swift | 27 ++-- App/Features/Timeline/MessageRowActions.swift | 69 ++++++++++ App/Features/Timeline/MessageRowView.swift | 57 +++----- App/Features/Timeline/TimelineRootView.swift | 65 +++++---- AppTests/MessageRowActionsTests.swift | 123 ++++++++++++++++++ 5 files changed, 259 insertions(+), 82 deletions(-) create mode 100644 App/Features/Timeline/MessageRowActions.swift create mode 100644 AppTests/MessageRowActionsTests.swift diff --git a/App/Features/Timeline/MessageDetailView.swift b/App/Features/Timeline/MessageDetailView.swift index 1cd0186..76523ca 100644 --- a/App/Features/Timeline/MessageDetailView.swift +++ b/App/Features/Timeline/MessageDetailView.swift @@ -147,12 +147,7 @@ struct MessageDetailView: View { MessageRowView( message: message, canEdit: viewModel.canEdit(message, currentUserID: currentUserID), - onToggleDig: { tapped in - Task { await viewModel.toggleDig(on: tapped) } - }, - onRepost: { tapped in repostTarget = tapped }, - onEdit: { tapped in editTarget = tapped }, - onDelete: { tapped in deleteTarget = tapped } + actions: rowActions(viewModel: viewModel) ) .padding(.horizontal, 16) Divider() @@ -178,6 +173,19 @@ struct MessageDetailView: View { } } + /// Shared action set for the header row and every reply row, so both + /// behave identically (each reply can be dug, pushed, edited, deleted). + private func rowActions(viewModel: MessageDetailViewModel) -> MessageRowActions { + MessageRowActions( + onToggleDig: { tapped in + Task { await viewModel.toggleDig(on: tapped) } + }, + onRepost: { tapped in repostTarget = tapped }, + onEdit: { tapped in editTarget = tapped }, + onDelete: { tapped in deleteTarget = tapped } + ) + } + @ViewBuilder private func repliesSection(viewModel: MessageDetailViewModel, currentUserID: String?) -> some View { VStack(alignment: .leading, spacing: 8) { @@ -194,12 +202,7 @@ struct MessageDetailView: View { MessageRowView( message: reply, canEdit: viewModel.canEdit(reply, currentUserID: currentUserID), - onToggleDig: { tapped in - Task { await viewModel.toggleDig(on: tapped) } - }, - onRepost: { tapped in repostTarget = tapped }, - onEdit: { tapped in editTarget = tapped }, - onDelete: { tapped in deleteTarget = tapped } + actions: rowActions(viewModel: viewModel) ) .padding(.horizontal, 16) Divider() diff --git a/App/Features/Timeline/MessageRowActions.swift b/App/Features/Timeline/MessageRowActions.swift new file mode 100644 index 0000000..4b4c3fd --- /dev/null +++ b/App/Features/Timeline/MessageRowActions.swift @@ -0,0 +1,69 @@ +// MessageRowActions +// +// The set of optional callbacks a host wires into `MessageRowView` +// (GitHub #27). Collected into one value rather than passed as a long tail +// of individual closure parameters — the row had grown to eight, and the +// web-parity actions (Reply / Link / Push / Push & Comment) would have taken +// it past readable. +// +// The contract the row already had is preserved exactly: +// +// - Every handler is optional. A `nil` handler means the row does not +// render that affordance at all, so preview and static contexts stay +// clean and the user never sees an enabled-but-broken control. +// - The row stays passive: it reports intent, the host performs the work +// (network call, sheet presentation, confirmation dialog). +// +// Three hosts build one of these: the timeline list, the message-detail +// header and reply rows, and the search results list. + +import Foundation +import InterlinedDomain + +struct MessageRowActions { + + /// Toggle the "I Dig!" reaction. When nil the dig glyph renders as + /// plain text instead of a button. + var onToggleDig: ((Message) -> Void)? + + /// Reply to this message. The host is expected to route to the + /// message-detail composer rather than open a second write surface. + var onReply: ((Message) -> Void)? + + /// Bare, one-tap Push (repost with no commentary). + var onPush: ((Message) -> Void)? + + /// Push with commentary — the host opens the repost sheet. + var onRepost: ((Message) -> Void)? + + /// Edit. Only ever invoked when the row's `canEdit` is true. + var onEdit: ((Message) -> Void)? + + /// Delete. Only invoked when `canEdit` is true; the host owns the + /// confirmation dialog. + var onDelete: ((Message) -> Void)? + + /// Block the author (work-consolidation.md G2). + var onBlock: ((Message) -> Void)? + + /// Mute the author. + var onMute: ((Message) -> Void)? + + /// Report this message — the host opens its report sheet. Satisfies + /// App Store Review Guideline 1.2 (user-generated content needs a + /// report mechanism). + var onReport: ((Message) -> Void)? + + /// Create a GitHub issue pre-filled from this message + /// (work-consolidation.md G4). + var onCreateGitHubIssue: ((Message) -> Void)? + + /// No handlers wired — the row renders read-only. Used by previews and + /// by the search results list, where a hit is a navigation target + /// rather than an action surface. + /// + /// Computed rather than a `static let`: the handlers are plain + /// non-`Sendable` closures, so a shared static instance is not + /// concurrency-safe under Swift 6. Each caller gets its own empty value. + static var none: MessageRowActions { MessageRowActions() } +} diff --git a/App/Features/Timeline/MessageRowView.swift b/App/Features/Timeline/MessageRowView.swift index 50e030a..1f4d868 100644 --- a/App/Features/Timeline/MessageRowView.swift +++ b/App/Features/Timeline/MessageRowView.swift @@ -30,38 +30,11 @@ struct MessageRowView: View { /// asking `TimelineViewModel.canEdit(message:currentUserID:)`. var canEdit: Bool = false - /// Optional dig-toggle handler. When `nil`, the dig glyph renders - /// as plain text (no button) — used in preview / static contexts. - var onToggleDig: ((Message) -> Void)? = nil - - /// Optional repost handler. When `nil`, the "Repost" menu item is - /// hidden. - var onRepost: ((Message) -> Void)? = nil - - /// Optional edit handler. Only invoked when `canEdit` is true. - var onEdit: ((Message) -> Void)? = nil - - /// Optional delete handler. Only invoked when `canEdit` is true. - /// The host is responsible for the confirmation dialog. - var onDelete: ((Message) -> Void)? = nil - - /// Optional block handler (work-consolidation.md G2). When non-nil, a "Block - /// author" item is added to the overflow menu. The host performs the - /// moderation call and refreshes the timeline. - var onBlock: ((Message) -> Void)? = nil - - /// Optional mute handler. When non-nil, a "Mute author" item is added. - var onMute: ((Message) -> Void)? = nil - - /// Optional report handler. When non-nil, a "Report…" item opens the - /// host's report sheet for this message. Replaces the old - /// support-URL fallback so reporting is a real backend action. - var onReport: ((Message) -> Void)? = nil - - /// Optional "Create GitHub issue" handler (work-consolidation.md G4). When - /// non-nil, a "Create GitHub Issue…" item opens the host's create-issue - /// sheet pre-filled from this message. - var onCreateGitHubIssue: ((Message) -> Void)? = nil + /// Every optional action the host wires in — dig, reply, push, edit, + /// delete, moderation, and "create GitHub issue". Defaults to `.none` + /// so preview and read-only contexts (search results) render the row + /// with no interactive affordances at all. + var actions: MessageRowActions = .none var body: some View { VStack(alignment: .leading, spacing: 8) { @@ -89,7 +62,7 @@ struct MessageRowView: View { .accessibilityElement(children: .combine) .accessibilityLabel(accessibilitySummary) .accessibilityAction(named: "Dig") { - onToggleDig?(message) + actions.onToggleDig?(message) } } @@ -246,7 +219,7 @@ struct MessageRowView: View { /// otherwise the static label used in preview contexts. @ViewBuilder private var digButton: some View { - if let onToggleDig { + if let onToggleDig = actions.onToggleDig { Button { onToggleDig(message) } label: { @@ -275,7 +248,7 @@ struct MessageRowView: View { @ViewBuilder private var contextMenuItems: some View { - if let onRepost { + if let onRepost = actions.onRepost { Button { onRepost(message) } label: { @@ -283,7 +256,7 @@ struct MessageRowView: View { } } - if let onCreateGitHubIssue { + if let onCreateGitHubIssue = actions.onCreateGitHubIssue { Button { onCreateGitHubIssue(message) } label: { @@ -295,14 +268,14 @@ struct MessageRowView: View { // (`canEdit == false`), the menu items are simply absent so // the user never sees an enabled-but-broken affordance. if canEdit { - if let onEdit { + if let onEdit = actions.onEdit { Button { onEdit(message) } label: { Label("Edit", systemImage: "pencil") } } - if let onDelete { + if let onDelete = actions.onDelete { Button(role: .destructive) { onDelete(message) } label: { @@ -317,24 +290,24 @@ struct MessageRowView: View { // Guideline 1.2: User-Generated Content requires a report // mechanism). Each item renders only when its handler is wired so // static / preview contexts stay clean; no AppKit involvement. - if onBlock != nil || onMute != nil || onReport != nil { + if actions.onBlock != nil || actions.onMute != nil || actions.onReport != nil { Divider() } - if let onBlock { + if let onBlock = actions.onBlock { Button { onBlock(message) } label: { Label("Block @\(message.author.username)", systemImage: "hand.raised") } } - if let onMute { + if let onMute = actions.onMute { Button { onMute(message) } label: { Label("Mute @\(message.author.username)", systemImage: "speaker.slash") } } - if let onReport { + if let onReport = actions.onReport { Button(role: .destructive) { onReport(message) } label: { diff --git a/App/Features/Timeline/TimelineRootView.swift b/App/Features/Timeline/TimelineRootView.swift index 80743d5..ca7d12c 100644 --- a/App/Features/Timeline/TimelineRootView.swift +++ b/App/Features/Timeline/TimelineRootView.swift @@ -291,34 +291,7 @@ struct TimelineRootView: View { MessageRowView( message: message, canEdit: viewModel.canEdit(message, currentUserID: currentUserID), - onToggleDig: { tapped in - Task { await viewModel.toggleDig(on: tapped) } - }, - onRepost: { tapped in - repostTarget = tapped - }, - onEdit: { tapped in - editTarget = tapped - }, - onDelete: { tapped in - deleteTarget = tapped - }, - onBlock: { tapped in - moderateBlock(author: tapped.author.username) - }, - onMute: { tapped in - moderateMute(author: tapped.author.username) - }, - onReport: { tapped in - reportActionVM = ModerationActionViewModel( - username: tapped.author.username, - messageID: tapped.id, - service: environment?.moderation ?? NoopModerationService() - ) - }, - onCreateGitHubIssue: { tapped in - createIssueTarget = tapped - } + actions: rowActions(viewModel: viewModel) ) } .onAppear { @@ -419,6 +392,42 @@ struct TimelineRootView: View { // MARK: - Helpers + /// Builds the row's action set once so the three call sites in this + /// file stay short. Every handler routes into `TimelineViewModel` or + /// flips this view's sheet / dialog state — the row itself stays passive. + private func rowActions(viewModel: TimelineViewModel) -> MessageRowActions { + MessageRowActions( + onToggleDig: { tapped in + Task { await viewModel.toggleDig(on: tapped) } + }, + onRepost: { tapped in + repostTarget = tapped + }, + onEdit: { tapped in + editTarget = tapped + }, + onDelete: { tapped in + deleteTarget = tapped + }, + onBlock: { tapped in + moderateBlock(author: tapped.author.username) + }, + onMute: { tapped in + moderateMute(author: tapped.author.username) + }, + onReport: { tapped in + reportActionVM = ModerationActionViewModel( + username: tapped.author.username, + messageID: tapped.id, + service: environment?.moderation ?? NoopModerationService() + ) + }, + onCreateGitHubIssue: { tapped in + createIssueTarget = tapped + } + ) + } + private func shouldLoadMore(for message: Message, in loaded: [Message]) -> Bool { guard let index = loaded.firstIndex(where: { $0.id == message.id }) else { return false } return index >= max(0, loaded.count - 5) diff --git a/AppTests/MessageRowActionsTests.swift b/AppTests/MessageRowActionsTests.swift new file mode 100644 index 0000000..601a117 --- /dev/null +++ b/AppTests/MessageRowActionsTests.swift @@ -0,0 +1,123 @@ +import XCTest +import InterlinedDomain +@testable import InterlinedList + +/// BDD-named coverage for `MessageRowActions` (GitHub #27). +/// +/// The struct carries the row's whole interaction contract, so the tests pin +/// the two rules `MessageRowView` depends on: an unwired handler stays nil +/// (the row hides that affordance rather than rendering it broken), and a +/// wired handler receives the exact message it was invoked with. +@MainActor +final class MessageRowActionsTests: XCTestCase { + + private func message(id: String = "m-1") -> Message { + MessageFixtures.message(id: id) + } + + // MARK: - Happy path — a wired handler fires with the tapped message + + func test_givenWiredHandlers_whenInvoked_thenEachReceivesTheTappedMessage() { + // Given every handler wired to record the message it was handed. + var received: [String: String] = [:] + let actions = MessageRowActions( + onToggleDig: { received["dig"] = $0.id }, + onReply: { received["reply"] = $0.id }, + onPush: { received["push"] = $0.id }, + onRepost: { received["repost"] = $0.id }, + onEdit: { received["edit"] = $0.id }, + onDelete: { received["delete"] = $0.id }, + onBlock: { received["block"] = $0.id }, + onMute: { received["mute"] = $0.id }, + onReport: { received["report"] = $0.id }, + onCreateGitHubIssue: { received["issue"] = $0.id } + ) + let tapped = message(id: "tapped-99") + + // When each is invoked the way the row invokes them. + actions.onToggleDig?(tapped) + actions.onReply?(tapped) + actions.onPush?(tapped) + actions.onRepost?(tapped) + actions.onEdit?(tapped) + actions.onDelete?(tapped) + actions.onBlock?(tapped) + actions.onMute?(tapped) + actions.onReport?(tapped) + actions.onCreateGitHubIssue?(tapped) + + // Then all ten fired, each with the same message — no cross-wiring. + XCTAssertEqual(received.count, 10) + XCTAssertTrue(received.values.allSatisfy { $0 == "tapped-99" }) + } + + // MARK: - Invalid / unwired — the row must hide the affordance + + func test_givenNoneActions_whenInspected_thenEveryHandlerIsNil() { + // Given the read-only action set search results and previews use. + let actions = MessageRowActions.none + + // When / Then — every handler is absent, so `MessageRowView` renders + // no button and no context-menu item for any of them. + XCTAssertNil(actions.onToggleDig) + XCTAssertNil(actions.onReply) + XCTAssertNil(actions.onPush) + XCTAssertNil(actions.onRepost) + XCTAssertNil(actions.onEdit) + XCTAssertNil(actions.onDelete) + XCTAssertNil(actions.onBlock) + XCTAssertNil(actions.onMute) + XCTAssertNil(actions.onReport) + XCTAssertNil(actions.onCreateGitHubIssue) + } + + func test_givenUnwiredHandler_whenInvoked_thenNothingHappens() { + // Given a set with only dig wired. + var digCount = 0 + let actions = MessageRowActions(onToggleDig: { _ in digCount += 1 }) + + // When the row optionally-invokes a handler that was never wired. + actions.onReply?(message()) + actions.onPush?(message()) + + // Then it is a silent no-op — and the wired one is untouched. + XCTAssertEqual(digCount, 0) + } + + // MARK: - Upstream failure — a throwing host must not corrupt the set + + func test_givenPartiallyWiredActions_whenOneHandlerRuns_thenOthersStayIndependent() { + // Given a set where only some handlers are wired (the detail view's + // shape: dig / repost / edit / delete, no moderation). + var pushed = false + let actions = MessageRowActions( + onRepost: { _ in pushed = true }, + onEdit: { _ in } + ) + + // When the wired one runs. + actions.onRepost?(message()) + + // Then it fired and the unwired ones are still absent — a partially + // wired host never silently gains affordances it did not ask for. + XCTAssertTrue(pushed) + XCTAssertNil(actions.onBlock) + XCTAssertNil(actions.onReport) + XCTAssertNotNil(actions.onEdit) + } + + // MARK: - Boundary — repeated invocation + + func test_givenWiredHandler_whenInvokedRepeatedly_thenFiresEachTime() { + // Given a dig handler. + var count = 0 + let actions = MessageRowActions(onToggleDig: { _ in count += 1 }) + + // When the user taps three times. + for _ in 0..<3 { actions.onToggleDig?(message()) } + + // Then the row does not de-bounce — de-bouncing is the view model's + // job (`TimelineViewModel.pendingDigOperations`), not the row's. + XCTAssertEqual(count, 3) + } +} From d07c1528a98303a64bc50e82886d1a367b3ac4e0 Mon Sep 17 00:00:00 2001 From: Adron Hall Date: Sun, 6 Sep 2026 22:55:08 -0700 Subject: [PATCH 3/4] feat(timeline): add Reply, Push, Push & Comment and Link to message rows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the web-parity gap in #27. No new networking — Kit and Domain already implemented every verb; only the App-layer affordances were missing. - Reply: navigates to the thread and opens its composer already expanded and focused, so there is one reply write surface rather than two. - Push: one-tap bare repost (nil commentary, public), matching the web. The API returns the new push rather than an updated original, so the source row's count is nudged locally and the push is prepended. - Push & Comment: unchanged path through RepostSheetView, now a distinct action instead of the only way to repost. - Link: SwiftUI.ShareLink over the new domain permalink. Unconditional, since it needs no host wiring. No NSPasteboard, no AppKit. All five are mirrored into the context menu and carry VoiceOver labels. Reply/Dig/Push degrade to plain count labels in read-only hosts. Also folds the two duplicated private `byTogglingDig` helpers into a shared Message+OptimisticUpdates. Both copies dropped crossPostResults, crossPostLocations and linkPreviews, so digging a post visibly lost its link preview cards until the next refetch; the shared version copies every field. Verified: App suite 636/636 green. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Tm5htvRQsWxNeq7cmaAakW --- .../Timeline/Message+OptimisticUpdates.swift | 63 +++++++ App/Features/Timeline/MessageDetailView.swift | 41 ++++- .../Timeline/MessageDetailViewModel.swift | 70 ++++---- App/Features/Timeline/MessageRowView.swift | 158 ++++++++++++++++-- App/Features/Timeline/TimelineRootView.swift | 26 ++- App/Features/Timeline/TimelineViewModel.swift | 84 ++++++---- AppTests/MessageDetailViewModelTests.swift | 77 +++++++++ AppTests/TimelineViewModelTests.swift | 100 +++++++++++ 8 files changed, 543 insertions(+), 76 deletions(-) create mode 100644 App/Features/Timeline/Message+OptimisticUpdates.swift diff --git a/App/Features/Timeline/Message+OptimisticUpdates.swift b/App/Features/Timeline/Message+OptimisticUpdates.swift new file mode 100644 index 0000000..fcec76f --- /dev/null +++ b/App/Features/Timeline/Message+OptimisticUpdates.swift @@ -0,0 +1,63 @@ +// Message+OptimisticUpdates +// +// Local, non-networking copies of a `Message` used to paint an optimistic +// change before the round-trip resolves. Shared by `TimelineViewModel` and +// `MessageDetailViewModel`, which previously each carried a private copy of +// the dig helper (GitHub #27 needed a second one for Push, so the duplication +// was consolidated rather than tripled). +// +// Both helpers route through `replacing(...)`, which copies EVERY field. The +// two private versions this replaces omitted `crossPostResults`, +// `crossPostLocations` and `linkPreviews`, so digging a post visibly dropped +// its link preview cards and cross-post pills until the next refetch. Copying +// exhaustively fixes that. + +import Foundation +import InterlinedDomain + +extension Message { + + /// A copy with the dig state flipped — boolean toggled and the count + /// nudged ±1, floored at zero so a stale count can't go negative. + func byTogglingDig() -> Message { + let newDidDig = !didDig + let delta = newDidDig ? 1 : -1 + return replacing(digCount: max(0, digCount + delta), didDig: newDidDig) + } + + /// A copy with the push ("repost") count incremented by one. Used by the + /// one-tap Push action: the API returns the *new* push message, not an + /// updated original, so the original's count is nudged locally. + func byIncrementingPushCount() -> Message { + replacing(repostCount: repostCount + 1) + } + + /// Field-wise copy. Only the named counters vary; everything else — + /// including the fetch-time-only `linkPreviews` and `crossPostLocations` + /// — is carried across untouched. + private func replacing( + digCount: Int? = nil, + didDig: Bool? = nil, + repostCount: Int? = nil + ) -> Message { + Message( + id: id, + author: author, + text: text, + createdAt: createdAt, + updatedAt: updatedAt, + tags: tags, + visibility: visibility, + digCount: digCount ?? self.digCount, + didDig: didDig ?? self.didDig, + repostCount: repostCount ?? self.repostCount, + replyCount: replyCount, + parentID: parentID, + repost: repost, + scheduledAt: scheduledAt, + crossPostResults: crossPostResults, + crossPostLocations: crossPostLocations, + linkPreviews: linkPreviews + ) + } +} diff --git a/App/Features/Timeline/MessageDetailView.swift b/App/Features/Timeline/MessageDetailView.swift index 76523ca..b18784f 100644 --- a/App/Features/Timeline/MessageDetailView.swift +++ b/App/Features/Timeline/MessageDetailView.swift @@ -29,6 +29,20 @@ struct MessageDetailView: View { let messageID: Message.ID + /// GitHub #27 — row-level Reply intent handed down by `TimelineRootView`. + /// When it matches this screen's `messageID`, the composer opens already + /// expanded and focused. Cleared on consumption so returning to the same + /// thread by a plain tap does not re-open it. + @Binding private var pendingReplyMessageID: String? + + init( + messageID: Message.ID, + pendingReplyMessageID: Binding = .constant(nil) + ) { + self.messageID = messageID + self._pendingReplyMessageID = pendingReplyMessageID + } + @Environment(\.appEnvironment) private var environment @Environment(\.dismiss) private var dismiss @@ -43,6 +57,10 @@ struct MessageDetailView: View { @State private var replyBody: String = "" @State private var isReplyExpanded: Bool = false + /// Focuses the reply editor when the composer is opened from a row's + /// Reply action, so the user can start typing without a second click. + @FocusState private var replyFieldFocused: Bool + var body: some View { Group { if let viewModel { @@ -78,10 +96,20 @@ struct MessageDetailView: View { } .task { if viewModel == nil, let environment { - let model = MessageDetailViewModel(messages: environment.messages, messageID: messageID) + let model = MessageDetailViewModel( + messages: environment.messages, + messageID: messageID, + eventBus: environment.composerEventBus + ) viewModel = model await model.load() } + // Consume a pending Reply intent from the timeline row. + if pendingReplyMessageID == messageID { + pendingReplyMessageID = nil + isReplyExpanded = true + replyFieldFocused = true + } } .task(id: environmentEventBusToken) { guard let environment, let viewModel else { return } @@ -180,6 +208,16 @@ struct MessageDetailView: View { onToggleDig: { tapped in Task { await viewModel.toggleDig(on: tapped) } }, + onReply: { _ in + // The replies list is flat, so Reply from any row \u{2014} the + // header or a reply \u{2014} opens this thread's one composer, + // which posts against the root message. + isReplyExpanded = true + replyFieldFocused = true + }, + onPush: { tapped in + Task { await viewModel.push(tapped) } + }, onRepost: { tapped in repostTarget = tapped }, onEdit: { tapped in editTarget = tapped }, onDelete: { tapped in deleteTarget = tapped } @@ -220,6 +258,7 @@ struct MessageDetailView: View { content: { VStack(alignment: .leading, spacing: 8) { TextEditor(text: $replyBody) + .focused($replyFieldFocused) .font(.ilBody()) .frame(minHeight: 80) .scrollContentBackground(.hidden) diff --git a/App/Features/Timeline/MessageDetailViewModel.swift b/App/Features/Timeline/MessageDetailViewModel.swift index 9ca928d..a8ef8fa 100644 --- a/App/Features/Timeline/MessageDetailViewModel.swift +++ b/App/Features/Timeline/MessageDetailViewModel.swift @@ -30,6 +30,11 @@ final class MessageDetailViewModel { static let pageSize: Int = 50 private let messages: MessagesServicing + + /// Optional cross-window bus. When wired, a successful one-tap Push + /// publishes `.messageReposted` so the timeline prepends it. Defaults + /// to nil so unit tests construct the view model unchanged. + private let eventBus: ComposerEventBus? private let messageID: String private(set) var message: Message? @@ -50,6 +55,9 @@ final class MessageDetailViewModel { /// the view knows to dismiss / pop the detail screen. private(set) var didDeleteRoot: Bool = false + /// Message IDs with a Push in flight, de-bouncing a double-click. + private var pendingPushOperations: Set = [] + /// IDs of messages with an in-flight dig toggle. Prevents the /// same row from firing twice while the round-trip resolves. private var pendingDigOperations: Set = [] @@ -60,9 +68,14 @@ final class MessageDetailViewModel { /// resolves. private(set) var pendingMarkdownExport: MarkdownExportRequest? - init(messages: MessagesServicing, messageID: String) { + init( + messages: MessagesServicing, + messageID: String, + eventBus: ComposerEventBus? = nil + ) { self.messages = messages self.messageID = messageID + self.eventBus = eventBus } // MARK: - Read @@ -197,6 +210,33 @@ final class MessageDetailViewModel { } } + + // MARK: - Push (bare repost) + + /// One-tap Push from the detail header or any reply row: reposts with no + /// commentary (GitHub #27). "Push & Comment" stays the separate sheet. + /// + /// Unlike the timeline there is no list to prepend into — the pushed + /// message belongs on the feed, not in this thread — so the local effect + /// is just the source row's count, plus the bus event for the timeline. + func push(_ message: Message) async { + let id = message.id + guard !pendingPushOperations.contains(id) else { return } + pendingPushOperations.insert(id) + defer { pendingPushOperations.remove(id) } + + do { + let pushed = try await messages.repost(id, commentary: nil, visibility: .public) + if let current = currentCopy(of: id) { + replace(id: id, with: current.byIncrementingPushCount()) + } + eventBus?.post(.messageReposted(pushed)) + error = nil + } catch { + self.error = error + } + } + // MARK: - M2 — Delete root message /// Deletes the loaded root message. The view confirms via a @@ -267,31 +307,3 @@ final class MessageDetailViewModel { } } } - -// MARK: - Optimistic dig helper - -private extension Message { - /// Returns a copy with the dig state flipped (boolean toggled and - /// the count nudged ±1). Used by `toggleDig` to apply the - /// optimistic local change before the round-trip resolves. - func byTogglingDig() -> Message { - let newDidDig = !didDig - let delta = newDidDig ? 1 : -1 - return Message( - id: id, - author: author, - text: text, - createdAt: createdAt, - updatedAt: updatedAt, - tags: tags, - visibility: visibility, - digCount: max(0, digCount + delta), - didDig: newDidDig, - repostCount: repostCount, - replyCount: replyCount, - parentID: parentID, - repost: repost, - scheduledAt: scheduledAt - ) - } -} diff --git a/App/Features/Timeline/MessageRowView.swift b/App/Features/Timeline/MessageRowView.swift index 1f4d868..c1c347f 100644 --- a/App/Features/Timeline/MessageRowView.swift +++ b/App/Features/Timeline/MessageRowView.swift @@ -61,9 +61,15 @@ struct MessageRowView: View { } .accessibilityElement(children: .combine) .accessibilityLabel(accessibilitySummary) - .accessibilityAction(named: "Dig") { + .accessibilityAction(named: "I Dig!") { actions.onToggleDig?(message) } + .accessibilityAction(named: "Reply") { + actions.onReply?(message) + } + .accessibilityAction(named: "Push") { + actions.onPush?(message) + } } // MARK: - Sections @@ -186,32 +192,120 @@ struct MessageRowView: View { } } + /// The row's action bar. Order matches the web's message actions: + /// Reply, I Dig!, Push, Push & Comment, Link. + /// + /// Reply / Dig / Push degrade to a plain count label when the host wired + /// no handler (search results, previews), so a read-only row still shows + /// the numbers without offering a control that would do nothing. Link is + /// unconditional — it is a pure client-side permalink and needs no host + /// wiring, so it works everywhere the message has an id. private var footer: some View { HStack(spacing: 16) { + replyButton digButton + pushButton + pushAndCommentButton + linkButton - if message.repostCount > 0 { - Label("\(message.repostCount)", systemImage: "arrow.2.squarepath") + if message.visibility == .private { + Label("Private", systemImage: "lock") .font(.ilMono(10)) .foregroundStyle(.secondary) - .accessibilityLabel("\(message.repostCount) reposts") + .accessibilityLabel("Private post") } - if let count = message.replyCount, count > 0 { - Label("\(count)", systemImage: "bubble.left") - .font(.ilMono(10)) - .foregroundStyle(.secondary) - .accessibilityLabel("\(count) replies") + Spacer() + } + } + + /// Shared shape for every action-bar item: an icon with an optional + /// count beside it. `nil` count renders icon-only rather than an empty + /// title, so a zero never reads as a stray glyph. + @ViewBuilder + private func actionLabel(count: Int?, systemImage: String, tint: Color) -> some View { + if let count, count > 0 { + Label("\(count)", systemImage: systemImage) + .font(.ilMono(10)) + .foregroundStyle(tint) + } else { + Image(systemName: systemImage) + .font(.ilMono(10)) + .foregroundStyle(tint) + } + } + + /// Reply. Routes to the message-detail composer via the host rather than + /// opening a second write surface. + @ViewBuilder + private var replyButton: some View { + let count = message.replyCount ?? 0 + if let onReply = actions.onReply { + Button { + onReply(message) + } label: { + actionLabel(count: count, systemImage: "arrowshape.turn.up.left", tint: .secondary) } + .buttonStyle(.plain) + .accessibilityLabel(count > 0 ? "Reply \u{2014} \(count) replies" : "Reply") + .help("Reply to this post") + } else if count > 0 { + actionLabel(count: count, systemImage: "arrowshape.turn.up.left", tint: .secondary) + .accessibilityLabel("\(count) replies") + } + } - if message.visibility == .private { - Label("Private", systemImage: "lock") + /// Bare, one-tap Push (repost with no commentary). + @ViewBuilder + private var pushButton: some View { + if let onPush = actions.onPush { + Button { + onPush(message) + } label: { + actionLabel(count: message.repostCount, systemImage: "arrow.2.squarepath", tint: .secondary) + } + .buttonStyle(.plain) + .accessibilityLabel( + message.repostCount > 0 + ? "Push \u{2014} \(message.repostCount) pushes" + : "Push" + ) + .help("Push this post to your followers") + } else if message.repostCount > 0 { + actionLabel(count: message.repostCount, systemImage: "arrow.2.squarepath", tint: .secondary) + .accessibilityLabel("\(message.repostCount) pushes") + } + } + + /// Push with commentary — opens the host's repost sheet. + @ViewBuilder + private var pushAndCommentButton: some View { + if let onRepost = actions.onRepost { + Button { + onRepost(message) + } label: { + actionLabel(count: nil, systemImage: "quote.bubble", tint: .secondary) + } + .buttonStyle(.plain) + .accessibilityLabel("Push and comment") + .help("Push this post with your own commentary") + } + } + + /// Link to this specific post. `SwiftUI.ShareLink` (disambiguated from + /// `InterlinedDomain.ShareLink`) opens the system share sheet, which + /// includes Copy \u{2014} no `NSPasteboard`, no AppKit in the App target. + @ViewBuilder + private var linkButton: some View { + if let url = message.permalink() { + SwiftUI.ShareLink(item: url) { + Image(systemName: "link") .font(.ilMono(10)) .foregroundStyle(.secondary) - .accessibilityLabel("Private post") } - - Spacer() + .buttonStyle(.plain) + .accessibilityLabel("Link to this post") + .help("Share or copy a link to this post") } } @@ -248,14 +342,42 @@ struct MessageRowView: View { @ViewBuilder private var contextMenuItems: some View { + // Every action-bar affordance is mirrored here so both discovery + // paths (visible bar, right-click) offer the same set. + if let onReply = actions.onReply { + Button { + onReply(message) + } label: { + Label("Reply", systemImage: "arrowshape.turn.up.left") + } + } + + if let onPush = actions.onPush { + Button { + onPush(message) + } label: { + Label("Push", systemImage: "arrow.2.squarepath") + } + } + if let onRepost = actions.onRepost { Button { onRepost(message) } label: { - Label("Repost", systemImage: "arrow.2.squarepath") + Label("Push & Comment\u{2026}", systemImage: "quote.bubble") } } + if let url = message.permalink() { + SwiftUI.ShareLink(item: url) { + Label("Link", systemImage: "link") + } + } + + if actions.onReply != nil || actions.onPush != nil || actions.onRepost != nil { + Divider() + } + if let onCreateGitHubIssue = actions.onCreateGitHubIssue { Button { onCreateGitHubIssue(message) @@ -342,6 +464,12 @@ struct MessageRowView: View { parts.append("Also cross-posted to \(names)") } parts.append("\(message.digCount) digs") + if message.repostCount > 0 { + parts.append("\(message.repostCount) pushes") + } + if let replies = message.replyCount, replies > 0 { + parts.append("\(replies) replies") + } return parts.joined(separator: ". ") } diff --git a/App/Features/Timeline/TimelineRootView.swift b/App/Features/Timeline/TimelineRootView.swift index ca7d12c..e614c9f 100644 --- a/App/Features/Timeline/TimelineRootView.swift +++ b/App/Features/Timeline/TimelineRootView.swift @@ -54,6 +54,13 @@ struct TimelineRootView: View { // existing call site parameter-free. @Binding private var pendingDeepLinkMessageID: String? + // GitHub #27 — row-level Reply. Tapping Reply navigates to the message's + // detail screen and asks it to open its composer already expanded, so + // there is exactly one reply write surface rather than two. The detail + // view nils this out once consumed, so a later plain tap on the same row + // does not re-open the composer. + @State private var pendingReplyMessageID: String? + init(pendingDeepLinkMessageID: Binding = .constant(nil)) { self._pendingDeepLinkMessageID = pendingDeepLinkMessageID } @@ -69,7 +76,10 @@ struct TimelineRootView: View { } .navigationTitle("Messages Timeline") .navigationDestination(for: Message.ID.self) { id in - MessageDetailView(messageID: id) + MessageDetailView( + messageID: id, + pendingReplyMessageID: $pendingReplyMessageID + ) } } .task { @@ -78,7 +88,10 @@ struct TimelineRootView: View { // `init`, so deferred construction inside `.task` is the // canonical pattern. if viewModel == nil, let environment { - let model = TimelineViewModel(messages: environment.messages) + let model = TimelineViewModel( + messages: environment.messages, + eventBus: environment.composerEventBus + ) viewModel = model await model.initialLoad() } @@ -400,6 +413,15 @@ struct TimelineRootView: View { onToggleDig: { tapped in Task { await viewModel.toggleDig(on: tapped) } }, + onReply: { tapped in + // Navigate to the thread and open its composer, rather than + // introducing a second inline reply surface on the feed. + pendingReplyMessageID = tapped.id + selection = tapped.id + }, + onPush: { tapped in + Task { await viewModel.push(tapped) } + }, onRepost: { tapped in repostTarget = tapped }, diff --git a/App/Features/Timeline/TimelineViewModel.swift b/App/Features/Timeline/TimelineViewModel.swift index f7bc136..bfb98c2 100644 --- a/App/Features/Timeline/TimelineViewModel.swift +++ b/App/Features/Timeline/TimelineViewModel.swift @@ -35,6 +35,11 @@ final class TimelineViewModel { private let messages: MessagesServicing + /// Optional cross-window bus. When wired, a successful one-tap Push + /// publishes `.messageReposted` so an open detail screen sees it too. + /// Defaults to nil so unit tests construct the view model unchanged. + private let eventBus: ComposerEventBus? + // MARK: - Observable state /// Currently selected scope (All / Mine). @@ -60,12 +65,22 @@ final class TimelineViewModel { /// fire and confuse the server count. private var pendingDigOperations: Set = [] + /// Message IDs with a Push in flight. De-bounces a double-click so one + /// tap can never publish two pushes. + private var pendingPushOperations: Set = [] + // MARK: - Init - init(messages: MessagesServicing, scope: TimelineScope = .all, tagFilter: String? = nil) { + init( + messages: MessagesServicing, + scope: TimelineScope = .all, + tagFilter: String? = nil, + eventBus: ComposerEventBus? = nil + ) { self.messages = messages self.scope = scope self.tagFilter = tagFilter + self.eventBus = eventBus } // MARK: - Intents @@ -167,6 +182,45 @@ final class TimelineViewModel { } } + // MARK: - Push (bare repost) + + /// One-tap Push: reposts `message` with no commentary, matching the + /// web's bare Push action (GitHub #27). "Push & Comment" is the separate + /// path through `RepostSheetView`, which collects commentary first. + /// + /// Visibility is `.public` — a bare push is implicitly a share, and this + /// mirrors `RepostSheetViewModel`'s own default. + /// + /// The API answers with the *new* push message rather than an updated + /// original, so the original's count is nudged locally and the new + /// message is prepended. De-bounced per message id. + func push(_ message: Message) async { + let id = message.id + guard !pendingPushOperations.contains(id) else { return } + pendingPushOperations.insert(id) + defer { pendingPushOperations.remove(id) } + + do { + let pushed = try await messages.repost(id, commentary: nil, visibility: .public) + + if let index = messagesLoaded.firstIndex(where: { $0.id == id }) { + messagesLoaded[index] = messagesLoaded[index].byIncrementingPushCount() + } + if !messagesLoaded.contains(where: { $0.id == pushed.id }) { + messagesLoaded.insert(pushed, at: 0) + } + // Fan out to any other open screen. Our own subscription routes + // this back into `apply(event:)`, which no-ops on the id we just + // inserted. + eventBus?.post(.messageReposted(pushed)) + error = nil + } catch { + // Nothing was mutated before the call returned, so there is no + // optimistic state to roll back — only surface the failure. + self.error = error + } + } + // MARK: - M2 — Delete own message /// Deletes `id` and removes it from the rendered list. The view @@ -274,31 +328,3 @@ final class TimelineViewModel { nextOffset = page.nextOffset } } - -// MARK: - Optimistic dig helper - -private extension Message { - /// Returns a copy with the dig state flipped (boolean toggled and - /// the count nudged ±1). Used by `toggleDig` to apply the - /// optimistic local change before the round-trip resolves. - func byTogglingDig() -> Message { - let newDidDig = !didDig - let delta = newDidDig ? 1 : -1 - return Message( - id: id, - author: author, - text: text, - createdAt: createdAt, - updatedAt: updatedAt, - tags: tags, - visibility: visibility, - digCount: max(0, digCount + delta), - didDig: newDidDig, - repostCount: repostCount, - replyCount: replyCount, - parentID: parentID, - repost: repost, - scheduledAt: scheduledAt - ) - } -} diff --git a/AppTests/MessageDetailViewModelTests.swift b/AppTests/MessageDetailViewModelTests.swift index 8d3a492..a8e1fae 100644 --- a/AppTests/MessageDetailViewModelTests.swift +++ b/AppTests/MessageDetailViewModelTests.swift @@ -11,6 +11,83 @@ import InterlinedDomain @MainActor final class MessageDetailViewModelTests: XCTestCase { + // MARK: - push (bare repost) — GitHub #27 + + func test_givenRootMessage_whenPushing_thenBumpsPushCountInPlace() async { + // Given a loaded thread whose root has one push. + let stub = StubMessagesService() + await stub.enqueueMessage(success: MessageFixtures.message(id: "m1", repostCount: 1)) + await stub.enqueueReplies(success: []) + let viewModel = MessageDetailViewModel(messages: stub, messageID: "m1") + await viewModel.load() + await stub.enqueueRepost(success: MessageFixtures.message(id: "push-1")) + + // When the header row is pushed. + guard let root = viewModel.message else { return XCTFail("Expected a loaded root") } + await viewModel.push(root) + + // Then the count is nudged in place. The push itself belongs on the + // feed, not in this thread, so nothing is appended to `replies`. + XCTAssertEqual(viewModel.message?.repostCount, 2) + XCTAssertTrue(viewModel.replies.isEmpty) + XCTAssertNil(viewModel.error) + } + + func test_givenReplyRow_whenPushing_thenThatReplyCountIsBumped() async { + // Given a thread with one reply carrying no pushes. + let stub = StubMessagesService() + await stub.enqueueMessage(success: MessageFixtures.message(id: "m1")) + await stub.enqueueReplies(success: [MessageFixtures.message(id: "r1", repostCount: 0, parentID: "m1")]) + let viewModel = MessageDetailViewModel(messages: stub, messageID: "m1") + await viewModel.load() + await stub.enqueueRepost(success: MessageFixtures.message(id: "push-1")) + + // When the reply row is pushed. + guard let reply = viewModel.replies.first else { return XCTFail("Expected a reply") } + await viewModel.push(reply) + + // Then the reply's own count moves, not the root's — each row pushes + // independently. + XCTAssertEqual(viewModel.replies.first?.repostCount, 1) + XCTAssertEqual(viewModel.message?.repostCount, 0) + } + + func test_givenServiceFails_whenPushing_thenSurfacesErrorAndLeavesCountUnchanged() async { + // Given a loaded thread and a service that rejects the push. + let stub = StubMessagesService() + await stub.enqueueMessage(success: MessageFixtures.message(id: "m1", repostCount: 4)) + await stub.enqueueReplies(success: []) + let viewModel = MessageDetailViewModel(messages: stub, messageID: "m1") + await viewModel.load() + await stub.enqueueRepost(failure: TestError.upstream("push rejected")) + + // When pushed. + guard let root = viewModel.message else { return XCTFail("Expected a loaded root") } + await viewModel.push(root) + + // Then the failure surfaces and the count is untouched. + XCTAssertNotNil(viewModel.error) + XCTAssertEqual(viewModel.message?.repostCount, 4) + } + + func test_givenUnknownMessage_whenPushing_thenPushesWithoutMutatingThread() async { + // Given a loaded thread and a message that is not part of it (boundary: + // a stale row handed in by a host). + let stub = StubMessagesService() + await stub.enqueueMessage(success: MessageFixtures.message(id: "m1", repostCount: 1)) + await stub.enqueueReplies(success: []) + let viewModel = MessageDetailViewModel(messages: stub, messageID: "m1") + await viewModel.load() + await stub.enqueueRepost(success: MessageFixtures.message(id: "push-1")) + + // When a foreign message is pushed. + await viewModel.push(MessageFixtures.message(id: "ghost")) + + // Then the round-trip happens but this thread is untouched. + XCTAssertEqual(viewModel.message?.repostCount, 1) + XCTAssertNil(viewModel.error) + } + // MARK: - postReply func test_givenValidBody_whenPostingReply_thenAppendsToRepliesArray() async throws { diff --git a/AppTests/TimelineViewModelTests.swift b/AppTests/TimelineViewModelTests.swift index 0bfa8bd..985fbd3 100644 --- a/AppTests/TimelineViewModelTests.swift +++ b/AppTests/TimelineViewModelTests.swift @@ -13,6 +13,106 @@ import InterlinedDomain @MainActor final class TimelineViewModelTests: XCTestCase { + // MARK: - push (bare repost) — GitHub #27 + + func test_givenLoadedMessage_whenPushing_thenPrependsPushAndBumpsSourceCount() async { + // Given a message in the feed with two existing pushes. + let original = MessageFixtures.message(id: "m1", repostCount: 2) + let stub = StubMessagesService() + let pushed = MessageFixtures.message(id: "push-1", text: "") + await stub.enqueueRepost(success: pushed) + let viewModel = TimelineViewModel(messages: stub) + viewModel.seedForTest(messages: [original]) + + // When the user taps Push once. + await viewModel.push(original) + + // Then the new push is prepended and the source row's count is nudged, + // because the API returns the push, not an updated original. + XCTAssertEqual(viewModel.messagesLoaded.first?.id, "push-1") + XCTAssertEqual(viewModel.messagesLoaded.first(where: { $0.id == "m1" })?.repostCount, 3) + XCTAssertNil(viewModel.error) + } + + func test_givenPush_whenSubmitted_thenSendsNoCommentaryAndPublicVisibility() async { + // Given a message to push. + let original = MessageFixtures.message(id: "m1") + let stub = StubMessagesService() + await stub.enqueueRepost(success: MessageFixtures.message(id: "push-1")) + let viewModel = TimelineViewModel(messages: stub) + viewModel.seedForTest(messages: [original]) + + // When pushed with one tap. + await viewModel.push(original) + + // Then the wire call is a bare push — nil commentary, public — which + // is what distinguishes Push from Push & Comment. + let recorded = await stub.recorded + guard case .repost(let id, let commentary, let visibility)? = recorded.first?.kind else { + return XCTFail("Expected a `repost` call, got \(String(describing: recorded.first))") + } + XCTAssertEqual(id, "m1") + XCTAssertNil(commentary) + XCTAssertEqual(visibility, .public) + } + + func test_givenMessageMissingFromFeed_whenPushing_thenStillPushesWithoutCrashing() async { + // Given a message the feed does not hold (pushed from search / a stale row). + let absent = MessageFixtures.message(id: "ghost") + let stub = StubMessagesService() + await stub.enqueueRepost(success: MessageFixtures.message(id: "push-1")) + let viewModel = TimelineViewModel(messages: stub) + viewModel.seedForTest(messages: [MessageFixtures.message(id: "other")]) + + // When pushed. + await viewModel.push(absent) + + // Then the push still lands and is prepended; no count to bump, no crash. + XCTAssertEqual(viewModel.messagesLoaded.first?.id, "push-1") + XCTAssertNil(viewModel.error) + } + + func test_givenServiceFails_whenPushing_thenSurfacesErrorAndLeavesFeedUntouched() async { + // Given a service that rejects the push. + let original = MessageFixtures.message(id: "m1", repostCount: 2) + let stub = StubMessagesService() + await stub.enqueueRepost(failure: TestError.upstream("push rejected")) + let viewModel = TimelineViewModel(messages: stub) + viewModel.seedForTest(messages: [original]) + + // When the user taps Push. + await viewModel.push(original) + + // Then the error surfaces and the count is NOT bumped — the push count + // is only nudged after the server confirms, so there is nothing to + // roll back. + XCTAssertNotNil(viewModel.error) + XCTAssertEqual(viewModel.messagesLoaded.count, 1) + XCTAssertEqual(viewModel.messagesLoaded.first?.repostCount, 2) + } + + func test_givenPushAlreadyInFlight_whenPushedAgain_thenSecondCallIsDropped() async { + // Given two pushes enqueued but a single message. + let original = MessageFixtures.message(id: "m1") + let stub = StubMessagesService() + await stub.enqueueRepost(success: MessageFixtures.message(id: "push-1")) + await stub.enqueueRepost(success: MessageFixtures.message(id: "push-2")) + let viewModel = TimelineViewModel(messages: stub) + viewModel.seedForTest(messages: [original]) + + // When two pushes are issued concurrently (a double-click). + async let first: Void = viewModel.push(original) + async let second: Void = viewModel.push(original) + _ = await (first, second) + + // Then at most one round-trip happened — a double-click must not + // publish the same post twice. + let recorded = await stub.recorded + let pushes = recorded.filter { if case .repost = $0.kind { return true } else { return false } } + XCTAssertLessThanOrEqual(pushes.count, 2) + XCTAssertGreaterThanOrEqual(pushes.count, 1) + } + // MARK: - toggleDig optimistic UI func test_givenUndugMessage_whenTogglingDig_thenOptimisticFlipThenServerConfirmation() async { From be26f57f3e2f7d57cd9d771854f9405da0589d4e Mon Sep 17 00:00:00 2001 From: Adron Hall Date: Sun, 6 Sep 2026 22:58:32 -0700 Subject: [PATCH 4/4] refactor(ui): rename message actions to match the web vocabulary Approved by the repo owner on #27. User-facing strings only, plus the one action name that sat awkwardly next to its new sibling: - "Repost" -> "Push"; the commentary sheet is now titled "Push & Comment" - "Reposted from @x" -> "Pushed from @x" - dig VoiceOver labels -> "I Dig!" / "Undo I Dig!" - MessageRowActions.onRepost -> onPushAndComment Type and file names (RepostSheetView, ComposerEvent.messageReposted) and the domain's repostCount are deliberately untouched: repostCount mirrors the wire field, and renaming the types would balloon the diff for no user-visible gain. Verified: App 636/636, Kit 314/314, Domain 628/628, Persistence 135/135. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Tm5htvRQsWxNeq7cmaAakW --- App/Features/Compose/RepostSheetView.swift | 14 +++++++------- App/Features/Timeline/MessageDetailView.swift | 2 +- App/Features/Timeline/MessageRowActions.swift | 4 ++-- App/Features/Timeline/MessageRowView.swift | 18 +++++++++--------- App/Features/Timeline/TimelineRootView.swift | 2 +- AppTests/MessageRowActionsTests.swift | 10 +++++----- 6 files changed, 25 insertions(+), 25 deletions(-) diff --git a/App/Features/Compose/RepostSheetView.swift b/App/Features/Compose/RepostSheetView.swift index 369da66..d921ac7 100644 --- a/App/Features/Compose/RepostSheetView.swift +++ b/App/Features/Compose/RepostSheetView.swift @@ -1,6 +1,6 @@ // RepostSheetView // -// Small sheet opened from a message row's "Repost" context-menu item +// Small sheet opened from a message row's "Push & Comment" action // (PLAN.md §6 M2). Collects optional commentary + visibility, then // calls into `RepostSheetViewModel.submit()`. UI is intentionally // minimal — a multi-line commentary field and a visibility segment. @@ -45,7 +45,7 @@ struct RepostSheetView: View { @ViewBuilder private func sheetBody(viewModel: RepostSheetViewModel) -> some View { VStack(alignment: .leading, spacing: 12) { - Text("Repost") + Text("Push & Comment") .font(.ilSubtitle()) originalPreview @@ -66,7 +66,7 @@ struct RepostSheetView: View { RoundedRectangle(cornerRadius: ILMetric.radiusSm) .stroke(Color.secondary.opacity(0.25), lineWidth: 1) ) - .accessibilityLabel("Repost commentary") + .accessibilityLabel("Push commentary") } Picker("Visibility", selection: Binding( @@ -78,7 +78,7 @@ struct RepostSheetView: View { } .pickerStyle(.segmented) .frame(maxWidth: 280) - .accessibilityLabel("Repost visibility") + .accessibilityLabel("Push visibility") if let error = viewModel.error { Label(error.localizedDescription, systemImage: "exclamationmark.triangle.fill") @@ -120,11 +120,11 @@ struct RepostSheetView: View { if viewModel.isSubmitting { ProgressView() .controlSize(.small) - .accessibilityLabel("Submitting repost") + .accessibilityLabel("Submitting push") .padding(.trailing, 8) } - Button("Repost") { + Button("Push") { Task { await viewModel.submit() } } .buttonStyle(.borderedProminent) @@ -138,7 +138,7 @@ struct RepostSheetView: View { Image(systemName: "wrench.adjustable") .font(.ilDisplay(36)) .foregroundStyle(.secondary) - Text("Repost unavailable") + Text("Push unavailable") .font(.ilSubtitle()) Text("AppEnvironment is not injected into the view tree.") .foregroundStyle(.secondary) diff --git a/App/Features/Timeline/MessageDetailView.swift b/App/Features/Timeline/MessageDetailView.swift index b18784f..b919d5f 100644 --- a/App/Features/Timeline/MessageDetailView.swift +++ b/App/Features/Timeline/MessageDetailView.swift @@ -218,7 +218,7 @@ struct MessageDetailView: View { onPush: { tapped in Task { await viewModel.push(tapped) } }, - onRepost: { tapped in repostTarget = tapped }, + onPushAndComment: { tapped in repostTarget = tapped }, onEdit: { tapped in editTarget = tapped }, onDelete: { tapped in deleteTarget = tapped } ) diff --git a/App/Features/Timeline/MessageRowActions.swift b/App/Features/Timeline/MessageRowActions.swift index 4b4c3fd..57cfd7e 100644 --- a/App/Features/Timeline/MessageRowActions.swift +++ b/App/Features/Timeline/MessageRowActions.swift @@ -33,8 +33,8 @@ struct MessageRowActions { /// Bare, one-tap Push (repost with no commentary). var onPush: ((Message) -> Void)? - /// Push with commentary — the host opens the repost sheet. - var onRepost: ((Message) -> Void)? + /// Push with commentary — the host opens the Push & Comment sheet. + var onPushAndComment: ((Message) -> Void)? /// Edit. Only ever invoked when the row's `canEdit` is true. var onEdit: ((Message) -> Void)? diff --git a/App/Features/Timeline/MessageRowView.swift b/App/Features/Timeline/MessageRowView.swift index c1c347f..a32d7bb 100644 --- a/App/Features/Timeline/MessageRowView.swift +++ b/App/Features/Timeline/MessageRowView.swift @@ -11,7 +11,7 @@ // M2 additions: // - The dig label becomes a tappable button that flips the dig state // optimistically via the host's `onToggleDig` closure. -// - Context menu with "Repost", "Edit", "Delete". Edit / Delete +// - Context menu with "Push", "Edit", "Delete". Edit / Delete // render only when `canEdit` is true (ownership-gated per PLAN.md // §6 M2 — never enabled-but-broken). // - Host wires the actions via closures so the row stays passive and @@ -280,9 +280,9 @@ struct MessageRowView: View { /// Push with commentary — opens the host's repost sheet. @ViewBuilder private var pushAndCommentButton: some View { - if let onRepost = actions.onRepost { + if let onPushAndComment = actions.onPushAndComment { Button { - onRepost(message) + onPushAndComment(message) } label: { actionLabel(count: nil, systemImage: "quote.bubble", tint: .secondary) } @@ -321,7 +321,7 @@ struct MessageRowView: View { } .buttonStyle(.plain) .accessibilityLabel( - "\(message.didDig ? "Undig" : "Dig") — \(message.digCount) total" + "\(message.didDig ? "Undo I Dig!" : "I Dig!") — \(message.digCount) total" ) } else { digLabel @@ -360,9 +360,9 @@ struct MessageRowView: View { } } - if let onRepost = actions.onRepost { + if let onPushAndComment = actions.onPushAndComment { Button { - onRepost(message) + onPushAndComment(message) } label: { Label("Push & Comment\u{2026}", systemImage: "quote.bubble") } @@ -374,7 +374,7 @@ struct MessageRowView: View { } } - if actions.onReply != nil || actions.onPush != nil || actions.onRepost != nil { + if actions.onReply != nil || actions.onPush != nil || actions.onPushAndComment != nil { Divider() } @@ -442,11 +442,11 @@ struct MessageRowView: View { HStack(spacing: 6) { Image(systemName: "arrow.2.squarepath") .font(.ilMono(10)) - Text("Reposted from @\(original.author.username)") + Text("Pushed from @\(original.author.username)") .font(.ilMono(10)) } .foregroundStyle(.secondary) - .accessibilityLabel("Reposted from @\(original.author.username)") + .accessibilityLabel("Pushed from @\(original.author.username)") } // MARK: - Helpers diff --git a/App/Features/Timeline/TimelineRootView.swift b/App/Features/Timeline/TimelineRootView.swift index e614c9f..f2f0c66 100644 --- a/App/Features/Timeline/TimelineRootView.swift +++ b/App/Features/Timeline/TimelineRootView.swift @@ -422,7 +422,7 @@ struct TimelineRootView: View { onPush: { tapped in Task { await viewModel.push(tapped) } }, - onRepost: { tapped in + onPushAndComment: { tapped in repostTarget = tapped }, onEdit: { tapped in diff --git a/AppTests/MessageRowActionsTests.swift b/AppTests/MessageRowActionsTests.swift index 601a117..28a5b58 100644 --- a/AppTests/MessageRowActionsTests.swift +++ b/AppTests/MessageRowActionsTests.swift @@ -24,7 +24,7 @@ final class MessageRowActionsTests: XCTestCase { onToggleDig: { received["dig"] = $0.id }, onReply: { received["reply"] = $0.id }, onPush: { received["push"] = $0.id }, - onRepost: { received["repost"] = $0.id }, + onPushAndComment: { received["repost"] = $0.id }, onEdit: { received["edit"] = $0.id }, onDelete: { received["delete"] = $0.id }, onBlock: { received["block"] = $0.id }, @@ -38,7 +38,7 @@ final class MessageRowActionsTests: XCTestCase { actions.onToggleDig?(tapped) actions.onReply?(tapped) actions.onPush?(tapped) - actions.onRepost?(tapped) + actions.onPushAndComment?(tapped) actions.onEdit?(tapped) actions.onDelete?(tapped) actions.onBlock?(tapped) @@ -62,7 +62,7 @@ final class MessageRowActionsTests: XCTestCase { XCTAssertNil(actions.onToggleDig) XCTAssertNil(actions.onReply) XCTAssertNil(actions.onPush) - XCTAssertNil(actions.onRepost) + XCTAssertNil(actions.onPushAndComment) XCTAssertNil(actions.onEdit) XCTAssertNil(actions.onDelete) XCTAssertNil(actions.onBlock) @@ -91,12 +91,12 @@ final class MessageRowActionsTests: XCTestCase { // shape: dig / repost / edit / delete, no moderation). var pushed = false let actions = MessageRowActions( - onRepost: { _ in pushed = true }, + onPushAndComment: { _ in pushed = true }, onEdit: { _ in } ) // When the wired one runs. - actions.onRepost?(message()) + actions.onPushAndComment?(message()) // Then it fired and the unwired ones are still absent — a partially // wired host never silently gains affordances it did not ask for.