From 0d2b03af1cc086a4914af9bb5d2b78871bcbb104 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Thu, 27 Aug 2026 14:08:53 +1200 Subject: [PATCH 01/10] Add the comment reply service method with typed composer errors --- .../Services/CommentsService.swift | 36 +++++++++++++++++++ .../Views/PreviewSupport.swift | 1 + .../CommentsServiceTests.swift | 23 ++++++++++++ .../Support/BlockingCommentsService.swift | 12 +++++++ .../Support/FakeCommentsService.swift | 9 +++++ 5 files changed, 81 insertions(+) diff --git a/Modules/Sources/WordPressComments/Services/CommentsService.swift b/Modules/Sources/WordPressComments/Services/CommentsService.swift index 3b1d9b428df5..b0dec3c12aba 100644 --- a/Modules/Sources/WordPressComments/Services/CommentsService.swift +++ b/Modules/Sources/WordPressComments/Services/CommentsService.swift @@ -56,6 +56,10 @@ protocol CommentsServiceProtocol: Sendable { /// Total number of replies to `id`, read from the list response's /// `X-WP-Total` header rather than the (unused) page of results. func numberOfReplies(for id: Int64) async throws -> Int + + /// Creates a reply to `parentID` on `postID` and returns the created + /// comment's detail. + func createReply(postID: Int64, parentID: Int64, content: String) async throws -> CommentDetail } /// Errors raised by `CommentsService` that don't originate from wordpress-rs. @@ -166,6 +170,18 @@ final class CommentsService: CommentsServiceProtocol { ) return Int(response.headerMap.wpTotal() ?? 0) } + + func createReply(postID: Int64, parentID: Int64, content: String) async throws -> CommentDetail { + // Core returns the create response in view context unless the caller + // has moderate_comments (create_item overrides any requested + // ?context=), and wordpress-rs decodes it as such, so the result never + // carries edit-only fields (`contentRaw`, email, IP). The reply chain + // only reads its status. + let response = try await client.api.comments.create( + params: CommentCreateParams(post: postID, content: content, parent: parentID) + ) + return CommentDetail(comment: response.data) + } } extension WpApiError { @@ -187,6 +203,26 @@ extension WpApiError { } } +/// Classification of a create/update failure for the composer's messaging. +/// `duplicate` is special-cased by the reply chain: core's duplicate check +/// (same post, parent, author, content) proves the content already exists +/// server-side, so it is late confirmation of an earlier send, not a failure. +enum CommentComposerRequestError: Equatable, Sendable { + case duplicate + case commentsClosed + case invalidContent + case other + + init(error: Error) { + switch (error as? WpApiError)?.wpErrorCode { + case .CommentDuplicate: self = .duplicate + case .CommentClosed: self = .commentsClosed + case .CommentContentInvalid: self = .invalidContent + default: self = .other + } + } +} + extension CommentsListFilter { static let pageSize: UInt32 = 20 diff --git a/Modules/Sources/WordPressComments/Views/PreviewSupport.swift b/Modules/Sources/WordPressComments/Views/PreviewSupport.swift index 1e09ae8112bf..69a4b5dc68f7 100644 --- a/Modules/Sources/WordPressComments/Views/PreviewSupport.swift +++ b/Modules/Sources/WordPressComments/Views/PreviewSupport.swift @@ -28,6 +28,7 @@ final class PreviewCommentsService: CommentsServiceProtocol { func trash(id: Int64) async throws {} func delete(id: Int64) async throws {} func numberOfReplies(for id: Int64) async throws -> Int { replyCount } + func createReply(postID: Int64, parentID: Int64, content: String) async throws -> CommentDetail { .preview() } } struct PreviewCapabilities: CommentsCapabilitiesProtocol { diff --git a/Modules/Tests/WordPressCommentsTests/CommentsServiceTests.swift b/Modules/Tests/WordPressCommentsTests/CommentsServiceTests.swift index aff009a87553..276574fc780f 100644 --- a/Modules/Tests/WordPressCommentsTests/CommentsServiceTests.swift +++ b/Modules/Tests/WordPressCommentsTests/CommentsServiceTests.swift @@ -1,5 +1,6 @@ import Testing import WordPressAPI +import WordPressAPIInternal @testable import WordPressComments struct CommentsServiceTests { @@ -17,3 +18,25 @@ struct CommentsServiceTests { } } } + +struct CommentComposerRequestErrorTests { + @Test func duplicateErrorClassification() { + #expect(CommentComposerRequestError(error: WpApiError.stub(code: .CommentDuplicate)) == .duplicate) + } + + @Test func closedErrorClassification() { + #expect(CommentComposerRequestError(error: WpApiError.stub(code: .CommentClosed)) == .commentsClosed) + } + + @Test func invalidContentErrorClassification() { + #expect( + CommentComposerRequestError(error: WpApiError.stub(code: .CommentContentInvalid)) == .invalidContent + ) + } + + @Test func unrelatedErrorClassifiesAsOther() { + struct Plain: Error {} + #expect(CommentComposerRequestError(error: Plain()) == .other) + #expect(CommentComposerRequestError(error: WpApiError.stub(code: .PostInvalidId)) == .other) + } +} diff --git a/Modules/Tests/WordPressCommentsTests/Support/BlockingCommentsService.swift b/Modules/Tests/WordPressCommentsTests/Support/BlockingCommentsService.swift index aee6a46ac911..db7244e438af 100644 --- a/Modules/Tests/WordPressCommentsTests/Support/BlockingCommentsService.swift +++ b/Modules/Tests/WordPressCommentsTests/Support/BlockingCommentsService.swift @@ -19,6 +19,9 @@ final class BlockingCommentsService: CommentsServiceProtocol { private var numberOfRepliesContinuations: [CheckedContinuation] = [] private(set) var numberOfRepliesInvocations: [Int64] = [] + private var createReplyContinuations: [CheckedContinuation] = [] + private(set) var createReplyInvocations: [(postID: Int64, parentID: Int64, content: String)] = [] + func listComments(filter: CommentsListFilter, nextPage: CommentsPageToken?) async throws -> CommentsPage { callCount += 1 return try await withCheckedThrowingContinuation { continuations.append($0) } @@ -75,4 +78,13 @@ final class BlockingCommentsService: CommentsServiceProtocol { func resolveFetch(callIndex: Int, with detail: CommentDetail) { fetchContinuations[callIndex].resume(returning: detail) } + + func createReply(postID: Int64, parentID: Int64, content: String) async throws -> CommentDetail { + createReplyInvocations.append((postID, parentID, content)) + return try await withCheckedThrowingContinuation { createReplyContinuations.append($0) } + } + + func resolveCreateReply(callIndex: Int, with detail: CommentDetail) { + createReplyContinuations[callIndex].resume(returning: detail) + } } diff --git a/Modules/Tests/WordPressCommentsTests/Support/FakeCommentsService.swift b/Modules/Tests/WordPressCommentsTests/Support/FakeCommentsService.swift index 82b2797164e7..6b961db4687c 100644 --- a/Modules/Tests/WordPressCommentsTests/Support/FakeCommentsService.swift +++ b/Modules/Tests/WordPressCommentsTests/Support/FakeCommentsService.swift @@ -30,6 +30,9 @@ final class FakeCommentsService: CommentsServiceProtocol { var numberOfRepliesResult: Result? private(set) var numberOfRepliesInvocations: [Int64] = [] + var createReplyResult: Result? + private(set) var createReplyInvocations: [(postID: Int64, parentID: Int64, content: String)] = [] + func listComments(filter: CommentsListFilter, nextPage: CommentsPageToken?) async throws -> CommentsPage { requests.append((filter, nextPage)) guard !queuedResults.isEmpty else { @@ -78,6 +81,12 @@ final class FakeCommentsService: CommentsServiceProtocol { guard let numberOfRepliesResult else { throw FakeServiceError() } return try numberOfRepliesResult.get() } + + func createReply(postID: Int64, parentID: Int64, content: String) async throws -> CommentDetail { + createReplyInvocations.append((postID, parentID, content)) + guard let createReplyResult else { throw FakeServiceError() } + return try createReplyResult.get() + } } func makePage(items: [CommentListItem], hasNext: Bool) -> CommentsPage { From 3cf8ff41327d8507004e1d9310c9fdd527a36f08 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Thu, 27 Aug 2026 14:11:50 +1200 Subject: [PATCH 02/10] Handle reply-created events in the list and detail screens --- .../Models/CommentChangeEvent.swift | 6 +++ .../ViewModels/CommentDetailViewModel.swift | 10 ++++ .../ViewModels/CommentsListViewModel.swift | 6 +++ .../CommentDetailViewModelTests.swift | 39 +++++++++++++++ .../CommentsListViewModelEventTests.swift | 48 +++++++++++++++++++ 5 files changed, 109 insertions(+) diff --git a/Modules/Sources/WordPressComments/Models/CommentChangeEvent.swift b/Modules/Sources/WordPressComments/Models/CommentChangeEvent.swift index a0e3e31c24dd..0cf6bd539d56 100644 --- a/Modules/Sources/WordPressComments/Models/CommentChangeEvent.swift +++ b/Modules/Sources/WordPressComments/Models/CommentChangeEvent.swift @@ -6,6 +6,11 @@ import Foundation enum CommentChangeEvent: Equatable, Sendable { case statusChanged(id: Int64, to: CommentListItem.Status) case deleted(id: Int64) + /// A reply was created under `parentID`. Carries the reply's own status + /// (not the parent's) so loaded tabs can decide whether it belongs to + /// them; stales rather than inserts because a paged list cannot know the + /// reply's correct position. + case replyCreated(parentID: Int64, replyStatus: CommentListItem.Status) } extension CommentChangeEvent { @@ -15,6 +20,7 @@ extension CommentChangeEvent { switch self { case .statusChanged(let id, _): id case .deleted(let id): id + case .replyCreated(let parentID, _): parentID } } } diff --git a/Modules/Sources/WordPressComments/ViewModels/CommentDetailViewModel.swift b/Modules/Sources/WordPressComments/ViewModels/CommentDetailViewModel.swift index 5a1ede3f41ae..ac25da7e76c5 100644 --- a/Modules/Sources/WordPressComments/ViewModels/CommentDetailViewModel.swift +++ b/Modules/Sources/WordPressComments/ViewModels/CommentDetailViewModel.swift @@ -268,6 +268,16 @@ final class CommentDetailViewModel: ObservableObject { // The comment is gone: a terminal state that turns the toolbar off // and dismisses the screen. isDeleted = true + case .replyCreated: + // The reply is a different comment; this screen's own status is + // corrected by the approve step's statusChanged event when + // relevant. Not a pure no-op, though: the cached reply count feeds + // `trashConfirmation`, so it must be bumped when known, or a parent + // that just gained its first reply could be trashed with no + // confirmation. + if let count = numberOfReplies { + numberOfReplies = count + 1 + } } } } diff --git a/Modules/Sources/WordPressComments/ViewModels/CommentsListViewModel.swift b/Modules/Sources/WordPressComments/ViewModels/CommentsListViewModel.swift index 63537be0a440..8673e8a2402a 100644 --- a/Modules/Sources/WordPressComments/ViewModels/CommentsListViewModel.swift +++ b/Modules/Sources/WordPressComments/ViewModels/CommentsListViewModel.swift @@ -136,6 +136,12 @@ final class CommentsListViewModel: ObservableObject { } case .deleted(let id): removeItem(id: id) + case .replyCreated(_, let replyStatus): + guard hasLoaded else { return } + // A new reply belongs at some position in the tabs matching its + // status; a paged list cannot insert at the right position, so + // mark those tabs stale (same reasoning as restore above). + if filter.matches(replyStatus) { markStale() } } } diff --git a/Modules/Tests/WordPressCommentsTests/CommentDetailViewModelTests.swift b/Modules/Tests/WordPressCommentsTests/CommentDetailViewModelTests.swift index 3200e956eaa2..aa815ab7926b 100644 --- a/Modules/Tests/WordPressCommentsTests/CommentDetailViewModelTests.swift +++ b/Modules/Tests/WordPressCommentsTests/CommentDetailViewModelTests.swift @@ -253,6 +253,23 @@ struct CommentDetailViewModelTests { #expect(vm.header?.status == .spam) } + @Test func replyCreatedEventLeavesDetailUntouched() async { + let service = FakeCommentsService() + service.fetchCommentResult = .success(makeDetail(id: 1, status: .approved, editContext: true)) + let coordinator = CommentsModerationCoordinator(service: FakeCommentsService()) + let vm = makeVM(service: service, coordinator: coordinator) + + await vm.onAppear() + let contentBeforeEvent = vm.content + let headerBeforeEvent = vm.header + + coordinator.events.send(.replyCreated(parentID: vm.commentID, replyStatus: .approved)) + + #expect(vm.content == contentBeforeEvent) + #expect(vm.header == headerBeforeEvent) + #expect(vm.isTerminated == false) + } + @Test func trashConfirmationVariants() async { // nil replies -> generic confirmation. let unknownService = FakeCommentsService() @@ -305,6 +322,28 @@ struct CommentDetailViewModelTests { #expect(vm.trashConfirmation == .withReplies) } + @Test func replyCreatedIncrementsKnownReplyCountAndUpdatesTrashConfirmation() async { + // Seed a zero known reply count via the load path's count fetch, so + // trashConfirmation starts at .none. + let service = FakeCommentsService() + service.fetchCommentResult = .success(makeDetail(id: 1, status: .approved, editContext: true)) + service.numberOfRepliesResult = .success(0) + let coordinator = CommentsModerationCoordinator(service: FakeCommentsService()) + let vm = makeVM(service: service, coordinator: coordinator) + + await vm.onAppear() + #expect(vm.numberOfReplies == 0) + #expect(vm.trashConfirmation == .none) + + // This screen's own comment just gained a reply: the cached count must + // be bumped, or a parent that just went from 0 to 1 replies could be + // trashed with no confirmation. + coordinator.events.send(.replyCreated(parentID: vm.commentID, replyStatus: .approved)) + + #expect(vm.numberOfReplies == 1) + #expect(vm.trashConfirmation == .withReplies) + } + @Test func parentPreviewLoadedForReply() async { let service = FakeCommentsService() service.fetchCommentResultsByID = [ diff --git a/Modules/Tests/WordPressCommentsTests/CommentsListViewModelEventTests.swift b/Modules/Tests/WordPressCommentsTests/CommentsListViewModelEventTests.swift index 5ffbfba00027..977780bec582 100644 --- a/Modules/Tests/WordPressCommentsTests/CommentsListViewModelEventTests.swift +++ b/Modules/Tests/WordPressCommentsTests/CommentsListViewModelEventTests.swift @@ -450,4 +450,52 @@ struct CommentsListViewModelEventTests { #expect(viewModel.state == .loaded) #expect(viewModel.items.map(\.id) == [1]) } + + // MARK: - Reply created: stales only tabs matching the reply's status + + @Test func replyCreatedApprovedStalesApprovedAndAllOnly() async { + for filter in CommentsListFilter.allCases { + let service = FakeCommentsService() + service.queuedResults = [.success(makePage(items: [makeItem(id: 1)], hasNext: false))] + let viewModel = CommentsListViewModel(filter: filter, service: service) + await viewModel.onAppear() + + viewModel.apply(.replyCreated(parentID: 1, replyStatus: .approved)) + + switch filter { + case .approved, .all: + #expect(viewModel.state == .awaitingReload) + case .pending, .spam, .trash: + #expect(viewModel.state == .loaded) + } + } + } + + @Test func replyCreatedPendingStalesPendingAndAllOnly() async { + for filter in CommentsListFilter.allCases { + let service = FakeCommentsService() + service.queuedResults = [.success(makePage(items: [makeItem(id: 1)], hasNext: false))] + let viewModel = CommentsListViewModel(filter: filter, service: service) + await viewModel.onAppear() + + viewModel.apply(.replyCreated(parentID: 1, replyStatus: .pending)) + + switch filter { + case .pending, .all: + #expect(viewModel.state == .awaitingReload) + case .approved, .spam, .trash: + #expect(viewModel.state == .loaded) + } + } + } + + @Test func replyCreatedBeforeLoadIsIgnored() async { + let service = FakeCommentsService() + let viewModel = CommentsListViewModel(filter: .all, service: service) + + viewModel.apply(.replyCreated(parentID: 1, replyStatus: .approved)) + + #expect(viewModel.state == .idle) + } + } From f588aa966444a687e623f958e5027c8a40148633 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Thu, 13 Aug 2026 12:51:00 +1200 Subject: [PATCH 03/10] Add a namespaced reply draft store --- .../Services/CommentDraftStore.swift | 58 ++++++++++++++++++ .../CommentDraftStoreTests.swift | 59 +++++++++++++++++++ 2 files changed, 117 insertions(+) create mode 100644 Modules/Sources/WordPressComments/Services/CommentDraftStore.swift create mode 100644 Modules/Tests/WordPressCommentsTests/CommentDraftStoreTests.swift diff --git a/Modules/Sources/WordPressComments/Services/CommentDraftStore.swift b/Modules/Sources/WordPressComments/Services/CommentDraftStore.swift new file mode 100644 index 000000000000..1ced8ba66944 --- /dev/null +++ b/Modules/Sources/WordPressComments/Services/CommentDraftStore.swift @@ -0,0 +1,58 @@ +import Foundation + +/// Persists in-progress reply drafts per (site, user, comment), matching the +/// legacy composer's behavior so a half-written reply survives cancel and +/// process death. Edit mode deliberately has no drafts (legacy parity). +@MainActor +public protocol CommentDraftStoring { + func loadDraft(commentID: Int64) -> String? + func saveDraft(_ text: String, commentID: Int64) + func deleteDraft(commentID: Int64) +} + +@MainActor +public final class UserDefaultsCommentDraftStore: CommentDraftStoring { + private let namespace: String + private let defaults: UserDefaults + + /// `namespace` identifies the (site, user) pair so drafts never leak + /// across sites or accounts; see `namespace(siteURL:username:)`. + public init(namespace: String, defaults: UserDefaults = .standard) { + self.namespace = namespace + self.defaults = defaults + } + + public convenience init(siteURL: URL, username: String, defaults: UserDefaults = .standard) { + self.init(namespace: Self.namespace(siteURL: siteURL, username: username), defaults: defaults) + } + + /// Keys drafts per (site, user): same person different site, or same + /// site different account, must never see each other's drafts. Only the + /// case-insensitive URL parts (scheme, host) are normalized; the path is + /// case-sensitive, so lowercasing the whole URL would collapse distinct + /// sites like /Blog and /blog and leak drafts between them. + static func namespace(siteURL: URL, username: String) -> String { + guard var components = URLComponents(url: siteURL, resolvingAgainstBaseURL: false) else { + return "\(siteURL.absoluteString)|\(username)" + } + components.scheme = components.scheme?.lowercased() + components.host = components.host?.lowercased() + return "\(components.string ?? siteURL.absoluteString)|\(username)" + } + + private func key(_ commentID: Int64) -> String { + "CommentsV2Draft.\(namespace).\(commentID)" + } + + public func loadDraft(commentID: Int64) -> String? { + defaults.string(forKey: key(commentID)) + } + + public func saveDraft(_ text: String, commentID: Int64) { + defaults.set(text, forKey: key(commentID)) + } + + public func deleteDraft(commentID: Int64) { + defaults.removeObject(forKey: key(commentID)) + } +} diff --git a/Modules/Tests/WordPressCommentsTests/CommentDraftStoreTests.swift b/Modules/Tests/WordPressCommentsTests/CommentDraftStoreTests.swift new file mode 100644 index 000000000000..d9794afbe1fd --- /dev/null +++ b/Modules/Tests/WordPressCommentsTests/CommentDraftStoreTests.swift @@ -0,0 +1,59 @@ +import Foundation +import Testing +@testable import WordPressComments + +@MainActor +struct CommentDraftStoreTests { + private func makeStore(namespace: String = "site|user") -> UserDefaultsCommentDraftStore { + let defaults = UserDefaults(suiteName: "CommentDraftStoreTests-\(UUID().uuidString)")! + return UserDefaultsCommentDraftStore(namespace: namespace, defaults: defaults) + } + + @Test func saveAndLoadRoundTrip() { + let store = makeStore() + store.saveDraft("hello", commentID: 7) + let loaded = store.loadDraft(commentID: 7) + #expect(loaded == "hello") + } + + @Test func loadMissingReturnsNil() { + let store = makeStore() + let loaded = store.loadDraft(commentID: 1) + #expect(loaded == nil) + } + + @Test func deleteRemovesDraft() { + let store = makeStore() + store.saveDraft("hello", commentID: 3) + store.deleteDraft(commentID: 3) + let loaded = store.loadDraft(commentID: 3) + #expect(loaded == nil) + } + + @Test func draftsAreScopedByCommentID() { + let store = makeStore() + store.saveDraft("hello", commentID: 1) + let loaded = store.loadDraft(commentID: 2) + #expect(loaded == nil) + } + + @Test func namespaceLowercasesSchemeAndHostButNotPath() { + let namespace = UserDefaultsCommentDraftStore.namespace( + siteURL: URL(string: "HTTPS://Example.COM/Blog")!, + username: "Admin" + ) + #expect(namespace == "https://example.com/Blog|Admin") + } + + @Test func draftsAreScopedByNamespace() { + let defaults = UserDefaults(suiteName: "CommentDraftStoreTests-\(UUID().uuidString)")! + let store1 = UserDefaultsCommentDraftStore(namespace: "site1|user1", defaults: defaults) + let store2 = UserDefaultsCommentDraftStore(namespace: "site2|user2", defaults: defaults) + + store1.saveDraft("draft1", commentID: 1) + store2.saveDraft("draft2", commentID: 1) + + #expect(store1.loadDraft(commentID: 1) == "draft1") + #expect(store2.loadDraft(commentID: 1) == "draft2") + } +} From fe771f7cf28271459f5d6d1c860da5d27440fb04 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Thu, 27 Aug 2026 14:12:29 +1200 Subject: [PATCH 04/10] Add the reply operation chain to the moderation coordinator --- .../CommentsModerationCoordinator.swift | 63 +++++++ .../Services/CommentsTracker.swift | 2 + .../CommentsModerationCoordinatorTests.swift | 165 ++++++++++++++++++ .../Support/BlockingCommentsService.swift | 4 + .../Comments/V2/CommentsTrackerAdapter.swift | 1 + 5 files changed, 235 insertions(+) diff --git a/Modules/Sources/WordPressComments/Services/CommentsModerationCoordinator.swift b/Modules/Sources/WordPressComments/Services/CommentsModerationCoordinator.swift index f7ed144a086e..c8d91875a006 100644 --- a/Modules/Sources/WordPressComments/Services/CommentsModerationCoordinator.swift +++ b/Modules/Sources/WordPressComments/Services/CommentsModerationCoordinator.swift @@ -26,6 +26,15 @@ enum CommentModerationAction: Hashable, Sendable { } } +/// The outcome of a successful `reply(to:content:approveParent:)` call. +struct ReplyOutcome: Equatable, Sendable { + let replyStatus: CommentListItem.Status + /// True when create returned comment_duplicate: the content already + /// existed server-side (an earlier send landed), so the composer words + /// its notice differently. + let alreadyPosted: Bool +} + /// Owns every comment mutation for the feature. All state changes flow through /// here so the coordinator can enforce two ordering rules the design requires: /// 1. One mutation in flight per comment. @@ -67,6 +76,50 @@ final class CommentsModerationCoordinator { await inFlight[id]?.value } + /// Creates a reply to `parent` and, when `approveParent`, approves the + /// pending parent afterwards, all inside one owning task holding the + /// parent's in-flight slot. Pessimistic: nothing is emitted until the + /// create outcome is known; a failure throws back to the composer. + func reply(to parent: CommentDetail, content: String, approveParent: Bool) async throws -> ReplyOutcome { + try await holdingSlot(for: parent.id, waitingForSlot: true) { [weak self] in + guard let self else { throw CancellationError() } + let created: CommentDetail? + do { + created = try await self.service.createReply( + postID: parent.postID, + parentID: parent.id, + content: content + ) + } catch { + // comment_duplicate proves this author already has this exact + // content on this post (core never accepts it twice), so an + // earlier send landed (timeout-after-commit). Continue the + // chain rather than failing, or the promised parent approval + // would be silently dropped on the retry path. + guard CommentComposerRequestError(error: error) == .duplicate else { throw error } + created = nil + } + // Reply is moderator-gated, so core auto-approves our replies; a + // duplicate (unknown landed status) assumes approved on the same + // basis. A plugin forcing moderation is corrected by the next + // list refetch (the event only marks tabs stale). + let replyStatus = created?.status ?? .approved + self.events.send(.replyCreated(parentID: parent.id, replyStatus: replyStatus)) + self.tracker?.track(.repliedTo(commentID: parent.id, postID: parent.postID)) + if approveParent { + // The approve runs pessimistically: its statusChanged event is + // emitted only after the request succeeds, so there is no + // optimistic emit to undo. It's possible the reply lands but the + // parent approval fails; the parent then remains Pending in list + // and detail, which is the true server state. We consider that + // an edge case and accept the risk; the user can approve + // manually. + try? await self.runModeration(.approve, on: parent) + } + return ReplyOutcome(replyStatus: replyStatus, alreadyPosted: created == nil) + } + } + /// Broadcasts a status change the detail screen observed on load (its seed /// status disagreed with the fetched truth) without running a mutation, so /// loaded list tabs reconcile the corrected status in place. @@ -90,10 +143,20 @@ final class CommentsModerationCoordinator { /// slot, so the mutation outlives a popped screen while the caller can /// still await its outcome. The slot claim happens synchronously before any /// suspension, so no second claimant can interleave. + /// + /// With `waitingForSlot`, a busy slot is awaited instead of being the + /// caller's problem. The check repeats after every wait rather than + /// awaiting once: two callers can both be suspended on the same in-flight + /// task and both resume once it completes. Looping lets only the first + /// claimant proceed; the second waits on that new claim instead. private func holdingSlot( for id: Int64, + waitingForSlot: Bool = false, _ body: @escaping @MainActor () async throws -> T ) async throws -> T { + while waitingForSlot, isMutating(id: id) { + await waitForPendingMutation(id: id) + } let chain = Task { try await body() } let slot = Task { [weak self] in _ = try? await chain.value diff --git a/Modules/Sources/WordPressComments/Services/CommentsTracker.swift b/Modules/Sources/WordPressComments/Services/CommentsTracker.swift index 54027d4609a8..e5aace0718ab 100644 --- a/Modules/Sources/WordPressComments/Services/CommentsTracker.swift +++ b/Modules/Sources/WordPressComments/Services/CommentsTracker.swift @@ -5,6 +5,8 @@ public enum CommentsTrackedEvent: Equatable, Sendable { case spammed(commentID: Int64, postID: Int64) case trashed(commentID: Int64, postID: Int64) // Permanent delete: legacy has no analytics event; deliberately untracked. + /// A reply was successfully created, matching legacy's reply-sent event. + case repliedTo(commentID: Int64, postID: Int64) } public protocol CommentsTracker: Sendable { diff --git a/Modules/Tests/WordPressCommentsTests/CommentsModerationCoordinatorTests.swift b/Modules/Tests/WordPressCommentsTests/CommentsModerationCoordinatorTests.swift index d727ca83f826..4f86d4383bcd 100644 --- a/Modules/Tests/WordPressCommentsTests/CommentsModerationCoordinatorTests.swift +++ b/Modules/Tests/WordPressCommentsTests/CommentsModerationCoordinatorTests.swift @@ -270,4 +270,169 @@ struct CommentsModerationCoordinatorTests { // A mapped success fires the same analytics event as a plain success. #expect(spy.trackedEvents == [.approved(commentID: 1, postID: 10)]) } + + // MARK: - Reply chain + + @Test func replyEmitsReplyCreatedWithLandedStatus() async throws { + let service = FakeCommentsService() + service.createReplyResult = .success(makeDetail(id: 99, status: .approved)) + let spy = SpyCommentsTracker() + let coordinator = CommentsModerationCoordinator(service: service, tracker: spy) + let recorder = EventRecorder(coordinator) + let parent = makeDetail(id: 1, status: .hold) + + let outcome = try await coordinator.reply(to: parent, content: "hi", approveParent: false) + + #expect(outcome.replyStatus == .approved) + #expect(!outcome.alreadyPosted) + #expect(recorder.events == [.replyCreated(parentID: 1, replyStatus: .approved)]) + #expect(spy.trackedEvents == [.repliedTo(commentID: 1, postID: 10)]) + #expect(!coordinator.isMutating(id: 1)) + } + + @Test func replyWithApproveParentInvokesSetStatus() async throws { + let service = FakeCommentsService() + service.createReplyResult = .success(makeDetail(id: 99, status: .approved)) + service.setStatusResult = .success(makeDetail(id: 1, status: .approved)) + let spy = SpyCommentsTracker() + let coordinator = CommentsModerationCoordinator(service: service, tracker: spy) + let recorder = EventRecorder(coordinator) + let parent = makeDetail(id: 1, status: .hold) + + let outcome = try await coordinator.reply(to: parent, content: "hi", approveParent: true) + + #expect(outcome.replyStatus == .approved) + #expect( + recorder.events == [ + .replyCreated(parentID: 1, replyStatus: .approved), + .statusChanged(id: 1, to: .approved) + ] + ) + // Regression test: a naive nested `perform(.approve, on: parent)` call + // would be dropped by the `isMutating` guard the chain itself holds, + // so `setStatus` must actually run via `runModeration` directly. + #expect(service.setStatusInvocations.map(\.status) == [.approved]) + #expect(spy.trackedEvents == [.repliedTo(commentID: 1, postID: 10), .approved(commentID: 1, postID: 10)]) + } + + @Test func replyCreateFailureThrowsWithoutEventsOrApprove() async { + let service = FakeCommentsService() + service.createReplyResult = .failure(FakeServiceError()) + let coordinator = CommentsModerationCoordinator(service: service) + let recorder = EventRecorder(coordinator) + let parent = makeDetail(id: 1, status: .hold) + + await #expect(throws: (any Error).self) { + try await coordinator.reply(to: parent, content: "hi", approveParent: true) + } + + #expect(recorder.events.isEmpty) + #expect(service.setStatusInvocations.isEmpty) + } + + @Test func replyDuplicateContinuesChainAndApproves() async throws { + let service = FakeCommentsService() + // comment_duplicate: an earlier send already landed server-side. + service.createReplyResult = .failure(WpApiError.stub(code: .CommentDuplicate)) + service.setStatusResult = .success(makeDetail(id: 1, status: .approved)) + let coordinator = CommentsModerationCoordinator(service: service) + let recorder = EventRecorder(coordinator) + let parent = makeDetail(id: 1, status: .hold) + + let outcome = try await coordinator.reply(to: parent, content: "hi", approveParent: true) + + #expect(outcome.alreadyPosted) + #expect(outcome.replyStatus == .approved) + #expect(recorder.events.contains(.replyCreated(parentID: 1, replyStatus: .approved))) + #expect(recorder.events.contains(.statusChanged(id: 1, to: .approved))) + #expect(service.setStatusInvocations.map(\.status) == [.approved]) + } + + @Test func replyApproveFailureStillSucceedsWithoutStatusEvent() async throws { + let service = FakeCommentsService() + service.createReplyResult = .success(makeDetail(id: 99, status: .approved)) + service.setStatusResult = .failure(FakeServiceError()) + let coordinator = CommentsModerationCoordinator(service: service) + let recorder = EventRecorder(coordinator) + let parent = makeDetail(id: 1, status: .hold) + + // The reply lands but the parent approval fails. The reply still + // succeeds, and the parent's status never changes: the approve emits + // its statusChanged only on success, so there is no optimistic emit to + // undo. The parent stays Pending, the true server state. + let outcome = try await coordinator.reply(to: parent, content: "hi", approveParent: true) + + #expect(outcome.replyStatus == .approved) + #expect(recorder.events == [.replyCreated(parentID: 1, replyStatus: .approved)]) + } + + @Test func toolbarActionDuringReplyChainIsDropped() async throws { + let service = BlockingCommentsService() + let coordinator = CommentsModerationCoordinator(service: service) + let recorder = EventRecorder(coordinator) + let parent = makeDetail(id: 1, status: .hold) + + async let replyOutcome = coordinator.reply(to: parent, content: "hi", approveParent: false) + await waitUntil { !service.createReplyInvocations.isEmpty } + + // The reply chain holds the slot, so a toolbar action is dropped by the + // isMutating guard: no request, no event. + try? await coordinator.perform(.trash, on: parent) + #expect(recorder.events.isEmpty) + #expect(coordinator.isMutating(id: 1)) + + service.resolveCreateReply(callIndex: 0, with: makeDetail(id: 99, status: .approved)) + let outcome = try await replyOutcome + + #expect(outcome.replyStatus == .approved) + #expect(recorder.events == [.replyCreated(parentID: 1, replyStatus: .approved)]) + } + + @Test func replyAwaitsPendingMutationOnParent() async throws { + let service = BlockingCommentsService() + service.createReplyResult = .success(makeDetail(id: 99, status: .approved)) + let coordinator = CommentsModerationCoordinator(service: service) + let parent = makeDetail(id: 1, status: .hold) + + let approve = Task { try? await coordinator.perform(.approve, on: parent) } + await waitUntil { !service.setStatusInvocations.isEmpty } + + async let replyOutcome = coordinator.reply(to: parent, content: "hi", approveParent: false) + // The reply call is suspended in waitForPendingMutation; the approve + // is still blocked on its continuation, so createReply must not have + // fired yet. + await Task.yield() + #expect(service.createReplyInvocations.isEmpty) + + service.resolveSetStatus(callIndex: 0, with: makeDetail(id: 1, status: .approved)) + let outcome = try await replyOutcome + _ = await approve.value + + #expect(outcome.replyStatus == .approved) + #expect(service.createReplyInvocations.count == 1) + } + + @Test func reentryFetchAwaitsReplyChain() async throws { + let service = BlockingCommentsService() + let coordinator = CommentsModerationCoordinator(service: service) + let parent = makeDetail(id: 5, status: .hold) + + async let replyOutcome = coordinator.reply(to: parent, content: "hi", approveParent: false) + await waitUntil { !service.createReplyInvocations.isEmpty } + + async let waiterResumed: Bool = { + await coordinator.waitForPendingMutation(id: 5) + return true + }() + + // Let the waiter reach its suspension point; the chain is still in + // flight, so it must not have resumed yet. + await Task.yield() + #expect(coordinator.isMutating(id: 5)) + + service.resolveCreateReply(callIndex: 0, with: makeDetail(id: 99, status: .approved)) + #expect(await waiterResumed) + #expect(!coordinator.isMutating(id: 5)) + _ = try await replyOutcome + } } diff --git a/Modules/Tests/WordPressCommentsTests/Support/BlockingCommentsService.swift b/Modules/Tests/WordPressCommentsTests/Support/BlockingCommentsService.swift index db7244e438af..eff57a2de807 100644 --- a/Modules/Tests/WordPressCommentsTests/Support/BlockingCommentsService.swift +++ b/Modules/Tests/WordPressCommentsTests/Support/BlockingCommentsService.swift @@ -21,6 +21,9 @@ final class BlockingCommentsService: CommentsServiceProtocol { private var createReplyContinuations: [CheckedContinuation] = [] private(set) var createReplyInvocations: [(postID: Int64, parentID: Int64, content: String)] = [] + /// When set, `createReply` returns immediately instead of blocking, for + /// tests that only need the call's timing relative to a blocked `setStatus`. + var createReplyResult: Result? func listComments(filter: CommentsListFilter, nextPage: CommentsPageToken?) async throws -> CommentsPage { callCount += 1 @@ -81,6 +84,7 @@ final class BlockingCommentsService: CommentsServiceProtocol { func createReply(postID: Int64, parentID: Int64, content: String) async throws -> CommentDetail { createReplyInvocations.append((postID, parentID, content)) + if let createReplyResult { return try createReplyResult.get() } return try await withCheckedThrowingContinuation { createReplyContinuations.append($0) } } diff --git a/WordPress/Classes/ViewRelated/Comments/V2/CommentsTrackerAdapter.swift b/WordPress/Classes/ViewRelated/Comments/V2/CommentsTrackerAdapter.swift index 1220f9248baf..dd592d1d1f5e 100644 --- a/WordPress/Classes/ViewRelated/Comments/V2/CommentsTrackerAdapter.swift +++ b/WordPress/Classes/ViewRelated/Comments/V2/CommentsTrackerAdapter.swift @@ -21,6 +21,7 @@ struct CommentsTrackerAdapter: CommentsTracker { case .unapproved(let c, let p): (.commentUnApproved, c, p) case .spammed(let c, let p): (.commentSpammed, c, p) case .trashed(let c, let p): (.commentTrashed, c, p) + case .repliedTo(let c, let p): (.commentRepliedTo, c, p) } WPAnalytics.track( analyticsEvent, From 4029f9c517f9c5435633bb3326396e6a247a616c Mon Sep 17 00:00:00 2001 From: Tony Li Date: Thu, 27 Aug 2026 14:12:38 +1200 Subject: [PATCH 05/10] Add composer and reply strings --- .../WordPressComments/Strings/Strings.swift | 84 +++++++++++++++++++ 1 file changed, 84 insertions(+) diff --git a/Modules/Sources/WordPressComments/Strings/Strings.swift b/Modules/Sources/WordPressComments/Strings/Strings.swift index 57b9edd13c93..18f17a1f327d 100644 --- a/Modules/Sources/WordPressComments/Strings/Strings.swift +++ b/Modules/Sources/WordPressComments/Strings/Strings.swift @@ -242,4 +242,88 @@ enum Strings { value: "Couldn't load this comment", comment: "Error state title when the comment detail fails to load" ) + + static let composerReplyTitle = NSLocalizedString( + "commentComposer.title.reply", + value: "Reply", + comment: "Title of the compose screen for replying to a comment" + ) + + static let composerPlaceholder = NSLocalizedString( + "commentComposer.placeholder", + value: "Leave a reply…", + comment: "Placeholder text in the composer text input field" + ) + + static let composerSend = NSLocalizedString( + "commentComposer.action.send", + value: "Send", + comment: "Button label to send a new reply" + ) + + static let composerCancel = NSLocalizedString( + "commentComposer.action.cancel", + value: "Cancel", + comment: "Button label to cancel composing or editing a comment" + ) + + static let composerApproveNote = NSLocalizedString( + "commentComposer.approveNote", + value: "Sending will also approve this comment.", + comment: "Note explaining that sending a reply will also approve the pending comment" + ) + + static let composerSaveDraft = NSLocalizedString( + "commentComposer.action.saveDraft", + value: "Save Draft", + comment: "Button label to save the current text as a draft" + ) + + static let composerDeleteDraft = NSLocalizedString( + "commentComposer.action.deleteDraft", + value: "Delete Draft", + comment: "Button label to delete a saved draft" + ) + + static let composerKeepEditing = NSLocalizedString( + "commentComposer.action.keepEditing", + value: "Keep Editing", + comment: "Button label to continue editing instead of discarding changes" + ) + + static let composerErrorClosed = NSLocalizedString( + "commentComposer.error.closed", + value: "Comments are closed for this post.", + comment: "Error message shown when comments are disabled for the post" + ) + + static let composerErrorReplyFailed = NSLocalizedString( + "commentComposer.error.replyFailed", + value: "Failed to send reply.", + comment: "Error message shown when sending a reply fails" + ) + + static let noticeReplySent = NSLocalizedString( + "commentComposer.notice.replySent", + value: "Reply sent.", + comment: "Notice shown after a reply is successfully sent" + ) + + static let noticeReplyPending = NSLocalizedString( + "commentComposer.notice.replyPending", + value: "Reply submitted for moderation.", + comment: "Notice shown when a reply is submitted and awaiting moderation" + ) + + static let noticeReplyAlreadyPosted = NSLocalizedString( + "commentComposer.notice.replyAlreadyPosted", + value: "This reply has already been posted.", + comment: "Notice shown when attempting to post a reply that was already submitted" + ) + + static let detailReply = NSLocalizedString( + "commentDetail.action.reply", + value: "Reply", + comment: "Button label to reply to a comment on the detail screen" + ) } From 71afb086782ade32fe747576fa157cfc25214dee Mon Sep 17 00:00:00 2001 From: Tony Li Date: Thu, 27 Aug 2026 14:13:17 +1200 Subject: [PATCH 06/10] Add the comment composer view model --- .../ViewModels/CommentComposerViewModel.swift | 146 ++++++++++++ .../CommentComposerViewModelTests.swift | 223 ++++++++++++++++++ .../Support/CommentDetailTestHelpers.swift | 7 +- .../Support/FakeCommentDraftStore.swift | 28 +++ 4 files changed, 402 insertions(+), 2 deletions(-) create mode 100644 Modules/Sources/WordPressComments/ViewModels/CommentComposerViewModel.swift create mode 100644 Modules/Tests/WordPressCommentsTests/CommentComposerViewModelTests.swift create mode 100644 Modules/Tests/WordPressCommentsTests/Support/FakeCommentDraftStore.swift diff --git a/Modules/Sources/WordPressComments/ViewModels/CommentComposerViewModel.swift b/Modules/Sources/WordPressComments/ViewModels/CommentComposerViewModel.swift new file mode 100644 index 000000000000..ede438dc3f0d --- /dev/null +++ b/Modules/Sources/WordPressComments/ViewModels/CommentComposerViewModel.swift @@ -0,0 +1,146 @@ +import Foundation +import WordPressShared + +/// Drives the reply composer sheet. Every mutation delegates to +/// `CommentsModerationCoordinator`. +@MainActor +final class CommentComposerViewModel: ObservableObject { + enum Mode: Equatable { + case reply(parent: CommentDetail) + } + + /// What the detail screen shows after the sheet dismisses. + enum Outcome: Equatable { + case replied(notice: String) + } + + @Published var text: String + @Published private(set) var isSending = false + @Published private(set) var errorMessage: String? + + let mode: Mode + /// The parent's author and one-line snippet, derived once (the sheet + /// re-renders on every keystroke). + let parentPreview: CommentListItem + + var title: String { + switch mode { + case .reply: Strings.composerReplyTitle + } + } + + var sendButtonTitle: String { + switch mode { + case .reply: Strings.composerSend + } + } + + var canSend: Bool { + guard !trimmedText.isEmpty else { return false } + switch mode { + case .reply: return true + } + } + + /// Reply mode only: sending will also approve the pending parent. + var showsApproveNote: Bool { + guard case .reply(let parent) = mode else { return false } + return parent.status == .pending + } + + /// Whether cancelling should ask before discarding: a reply with any + /// non-blank text. + var isDirty: Bool { + switch mode { + case .reply: !trimmedText.isEmpty + } + } + + private var trimmedText: String { + text.trim() + } + + private let coordinator: CommentsModerationCoordinator + private let draftStore: any CommentDraftStoring + private let tracker: (any CommentsTracker)? + + init( + mode: Mode, + coordinator: CommentsModerationCoordinator, + draftStore: any CommentDraftStoring, + tracker: (any CommentsTracker)? = nil + ) { + self.mode = mode + self.coordinator = coordinator + self.draftStore = draftStore + self.tracker = tracker + switch mode { + case .reply(let parent): + parentPreview = CommentListItem(detail: parent) + text = draftStore.loadDraft(commentID: parent.id) ?? "" + } + } + + func send() async -> Outcome? { + guard canSend, !isSending else { return nil } + errorMessage = nil + isSending = true + defer { isSending = false } + + let content = trimmedText + switch mode { + case .reply(let parent): + do { + // The composer is only reachable by moderators, so a pending + // parent always means "approve on send" (no separate consent + // step). + let outcome = try await coordinator.reply( + to: parent, + content: content, + approveParent: parent.status == .pending + ) + draftStore.deleteDraft(commentID: parent.id) + return .replied(notice: notice(for: outcome)) + } catch { + errorMessage = errorText(for: error, defaultMessage: Strings.composerErrorReplyFailed) + return nil + } + } + } + + /// Keeps the current text for the next time the composer opens on this + /// parent. + func saveDraft() { + guard case .reply(let parent) = mode else { return } + draftStore.saveDraft(text, commentID: parent.id) + } + + func deleteDraft() { + guard case .reply(let parent) = mode else { return } + draftStore.deleteDraft(commentID: parent.id) + } + + /// Words the post-send notice: a duplicate confirms an earlier send + /// already landed, a pending status means the reply itself needs + /// moderation, otherwise it posted straight away. + private func notice(for outcome: ReplyOutcome) -> String { + if outcome.alreadyPosted { + return Strings.noticeReplyAlreadyPosted + } + if outcome.replyStatus == .pending { + return Strings.noticeReplyPending + } + return Strings.noticeReplySent + } + + /// Classifies a thrown mutation error into the message the composer + /// shows. `.duplicate` never reaches here (the coordinator's reply chain + /// absorbs it into a successful outcome), so it falls back to + /// `defaultMessage` alongside `.invalidContent`/`.other`. + private func errorText(for error: Error, defaultMessage: String) -> String { + switch CommentComposerRequestError(error: error) { + case .commentsClosed: return Strings.composerErrorClosed + case .duplicate, .invalidContent, .other: return defaultMessage + } + } +} diff --git a/Modules/Tests/WordPressCommentsTests/CommentComposerViewModelTests.swift b/Modules/Tests/WordPressCommentsTests/CommentComposerViewModelTests.swift new file mode 100644 index 000000000000..bd7ac670f766 --- /dev/null +++ b/Modules/Tests/WordPressCommentsTests/CommentComposerViewModelTests.swift @@ -0,0 +1,223 @@ +import Foundation +import Testing +import WordPressAPI +import WordPressAPIInternal +@testable import WordPressComments + +@MainActor +private func makeCoordinator( + service: FakeCommentsService = FakeCommentsService(), + tracker: (any CommentsTracker)? = nil +) -> CommentsModerationCoordinator { + CommentsModerationCoordinator(service: service, tracker: tracker) +} + +@MainActor +struct CommentComposerViewModelTests { + + // MARK: - Construction and gating + + @Test func replyModeRestoresDraftOnInit() { + let store = FakeCommentDraftStore() + store.preloadDraft("draft", commentID: 1) + + let vm = CommentComposerViewModel( + mode: .reply(parent: makeDetail(id: 1)), + coordinator: makeCoordinator(), + draftStore: store + ) + + #expect(vm.text == "draft") + } + + @Test func approveNoteShownOnlyForPendingReplyParent() { + let pendingVM = CommentComposerViewModel( + mode: .reply(parent: makeDetail(status: .hold)), + coordinator: makeCoordinator(), + draftStore: FakeCommentDraftStore() + ) + #expect(pendingVM.showsApproveNote) + + let approvedVM = CommentComposerViewModel( + mode: .reply(parent: makeDetail(status: .approved)), + coordinator: makeCoordinator(), + draftStore: FakeCommentDraftStore() + ) + #expect(!approvedVM.showsApproveNote) + } + + @Test func canSendRequiresNonEmptyTrimmedText() { + let vm = CommentComposerViewModel( + mode: .reply(parent: makeDetail()), + coordinator: makeCoordinator(), + draftStore: FakeCommentDraftStore() + ) + + vm.text = " \n" + #expect(!vm.canSend) + + vm.text = "hi" + #expect(vm.canSend) + } + + // MARK: - Send: reply + + @Test func sendReplyPassesApproveParentForPendingParent() async { + let pendingService = FakeCommentsService() + pendingService.createReplyResult = .success(makeDetail(id: 99, status: .approved)) + pendingService.setStatusResult = .success(makeDetail(id: 1, status: .approved)) + let pendingVM = CommentComposerViewModel( + mode: .reply(parent: makeDetail(id: 1, status: .hold)), + coordinator: makeCoordinator(service: pendingService), + draftStore: FakeCommentDraftStore() + ) + pendingVM.text = "hi" + _ = await pendingVM.send() + #expect(pendingService.setStatusInvocations.map(\.status) == [.approved]) + + let approvedService = FakeCommentsService() + approvedService.createReplyResult = .success(makeDetail(id: 99, status: .approved)) + let approvedVM = CommentComposerViewModel( + mode: .reply(parent: makeDetail(id: 1, status: .approved)), + coordinator: makeCoordinator(service: approvedService), + draftStore: FakeCommentDraftStore() + ) + approvedVM.text = "hi" + _ = await approvedVM.send() + #expect(approvedService.setStatusInvocations.isEmpty) + } + + @Test func sendReplySuccessDeletesDraftAndReturnsNotice() async { + let service = FakeCommentsService() + service.createReplyResult = .success(makeDetail(id: 99, status: .approved)) + let spy = SpyCommentsTracker() + let store = FakeCommentDraftStore() + store.preloadDraft("draft", commentID: 1) + let vm = CommentComposerViewModel( + mode: .reply(parent: makeDetail(id: 1, status: .approved)), + coordinator: makeCoordinator(service: service, tracker: spy), + draftStore: store, + tracker: spy + ) + vm.text = "hi" + + let outcome = await vm.send() + + #expect(outcome == .replied(notice: Strings.noticeReplySent)) + #expect(store.deleted == [1]) + } + + @Test func sendReplyPendingStatusWordsNoticeAccordingly() async { + let service = FakeCommentsService() + service.createReplyResult = .success(makeDetail(id: 99, status: .hold)) + let vm = CommentComposerViewModel( + mode: .reply(parent: makeDetail(id: 1, status: .approved)), + coordinator: makeCoordinator(service: service), + draftStore: FakeCommentDraftStore() + ) + vm.text = "hi" + + let outcome = await vm.send() + + #expect(outcome == .replied(notice: Strings.noticeReplyPending)) + } + + @Test func sendReplyAlreadyPostedWordsNoticeAccordingly() async { + let service = FakeCommentsService() + service.createReplyResult = .failure(WpApiError.stub(code: .CommentDuplicate)) + let store = FakeCommentDraftStore() + store.preloadDraft("draft", commentID: 1) + let vm = CommentComposerViewModel( + mode: .reply(parent: makeDetail(id: 1, status: .approved)), + coordinator: makeCoordinator(service: service), + draftStore: store + ) + vm.text = "hi" + + let outcome = await vm.send() + + #expect(outcome == .replied(notice: Strings.noticeReplyAlreadyPosted)) + #expect(store.deleted == [1]) + } + + @Test func sendReplyFailureShowsErrorAndKeepsDraftIntact() async { + let service = FakeCommentsService() + service.createReplyResult = .failure(FakeServiceError()) + let store = FakeCommentDraftStore() + store.preloadDraft("draft", commentID: 1) + let vm = CommentComposerViewModel( + mode: .reply(parent: makeDetail(id: 1, status: .approved)), + coordinator: makeCoordinator(service: service), + draftStore: store + ) + vm.text = "hi" + + let outcome = await vm.send() + + #expect(outcome == nil) + #expect(vm.errorMessage == Strings.composerErrorReplyFailed) + #expect(!vm.isSending) + #expect(store.deleted.isEmpty) + } + + @Test func sendReplyClosedErrorShowsClosedMessage() async { + let service = FakeCommentsService() + service.createReplyResult = .failure(WpApiError.stub(code: .CommentClosed)) + let vm = CommentComposerViewModel( + mode: .reply(parent: makeDetail(id: 1, status: .approved)), + coordinator: makeCoordinator(service: service), + draftStore: FakeCommentDraftStore() + ) + vm.text = "hi" + + let outcome = await vm.send() + + #expect(outcome == nil) + #expect(vm.errorMessage == Strings.composerErrorClosed) + } + + // MARK: - Cancel flows + + @Test func replyIsDirtyOnlyWithNonBlankText() { + let vm = CommentComposerViewModel( + mode: .reply(parent: makeDetail(id: 1)), + coordinator: makeCoordinator(), + draftStore: FakeCommentDraftStore() + ) + #expect(!vm.isDirty) + + vm.text = " \n" + #expect(!vm.isDirty) + + vm.text = "hi" + #expect(vm.isDirty) + } + + @Test func saveDraftPersists() { + let store = FakeCommentDraftStore() + let vm = CommentComposerViewModel( + mode: .reply(parent: makeDetail(id: 1)), + coordinator: makeCoordinator(), + draftStore: store + ) + vm.text = "draft text" + + vm.saveDraft() + + #expect(store.saved[1] == "draft text") + } + + @Test func deleteDraftDeletes() { + let store = FakeCommentDraftStore() + let vm = CommentComposerViewModel( + mode: .reply(parent: makeDetail(id: 1)), + coordinator: makeCoordinator(), + draftStore: store + ) + + vm.deleteDraft() + + #expect(store.deleted.contains(1)) + } + +} diff --git a/Modules/Tests/WordPressCommentsTests/Support/CommentDetailTestHelpers.swift b/Modules/Tests/WordPressCommentsTests/Support/CommentDetailTestHelpers.swift index 7c9cbf93a3dc..b143ab47ce9a 100644 --- a/Modules/Tests/WordPressCommentsTests/Support/CommentDetailTestHelpers.swift +++ b/Modules/Tests/WordPressCommentsTests/Support/CommentDetailTestHelpers.swift @@ -12,10 +12,13 @@ func makeDetail( parent: Int64 = 0, post: Int64 = 10, status: CommentStatus = .approved, - editContext: Bool = false + editContext: Bool = false, + content: String = "raw" ) -> CommentDetail { if editContext { - return CommentDetail(comment: .editDetailBuilder(id: id, post: post, parent: parent, status: status)) + return CommentDetail( + comment: .editDetailBuilder(id: id, content: content, post: post, parent: parent, status: status) + ) } return CommentDetail(comment: .detailBuilder(id: id, post: post, parent: parent, status: status)) } diff --git a/Modules/Tests/WordPressCommentsTests/Support/FakeCommentDraftStore.swift b/Modules/Tests/WordPressCommentsTests/Support/FakeCommentDraftStore.swift new file mode 100644 index 000000000000..03b65e56e125 --- /dev/null +++ b/Modules/Tests/WordPressCommentsTests/Support/FakeCommentDraftStore.swift @@ -0,0 +1,28 @@ +@testable import WordPressComments + +/// In-memory draft store recording every save/delete call, so composer VM +/// tests can assert on draft lifecycle without touching `UserDefaults`. +@MainActor +final class FakeCommentDraftStore: CommentDraftStoring { + private var drafts: [Int64: String] = [:] + private(set) var saved: [Int64: String] = [:] + private(set) var deleted: [Int64] = [] + + func preloadDraft(_ text: String, commentID: Int64) { + drafts[commentID] = text + } + + func loadDraft(commentID: Int64) -> String? { + drafts[commentID] + } + + func saveDraft(_ text: String, commentID: Int64) { + saved[commentID] = text + drafts[commentID] = text + } + + func deleteDraft(commentID: Int64) { + deleted.append(commentID) + drafts[commentID] = nil + } +} From ba7358adebd7588541ce43f19cbab3c393bbc800 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Thu, 27 Aug 2026 14:14:23 +1200 Subject: [PATCH 07/10] Add the comment composer sheet and wire reply into the detail screen --- .../Services/CommentsDetailRouter.swift | 4 + .../ViewModels/CommentDetailViewModel.swift | 38 +++++ .../Views/CommentsHostingController.swift | 4 +- .../Views/Detail/CommentComposerView.swift | 139 ++++++++++++++++++ .../Views/Detail/CommentDetailView.swift | 20 +++ .../Views/PreviewSupport.swift | 7 + .../CommentDetailViewModelTests.swift | 134 ++++++++++++++++- .../CommentsDetailRouterTests.swift | 1 + .../Support/CommentDetailTestHelpers.swift | 2 + .../Comments/CommentsRouting.swift | 5 +- 10 files changed, 350 insertions(+), 4 deletions(-) create mode 100644 Modules/Sources/WordPressComments/Views/Detail/CommentComposerView.swift diff --git a/Modules/Sources/WordPressComments/Services/CommentsDetailRouter.swift b/Modules/Sources/WordPressComments/Services/CommentsDetailRouter.swift index 402e1a15c007..2110d9362cb9 100644 --- a/Modules/Sources/WordPressComments/Services/CommentsDetailRouter.swift +++ b/Modules/Sources/WordPressComments/Services/CommentsDetailRouter.swift @@ -14,6 +14,7 @@ final class CommentsDetailRouter { /// while the list loads and later screens read it synchronously. private let capabilities: CommentsCapabilityResolver private let coordinator: CommentsModerationCoordinator + private let draftStore: any CommentDraftStoring private let titleResolver: PostTitleResolver private let tracker: (any CommentsTracker)? private let noticePresenter: any NoticePresenting @@ -23,6 +24,7 @@ final class CommentsDetailRouter { service: any CommentsServiceProtocol, capabilities: any CommentsCapabilitiesProtocol, coordinator: CommentsModerationCoordinator, + draftStore: any CommentDraftStoring, titleResolver: PostTitleResolver, tracker: (any CommentsTracker)?, noticePresenter: any NoticePresenting, @@ -31,6 +33,7 @@ final class CommentsDetailRouter { self.service = service self.capabilities = CommentsCapabilityResolver(capabilities: capabilities) self.coordinator = coordinator + self.draftStore = draftStore self.titleResolver = titleResolver self.tracker = tracker self.noticePresenter = noticePresenter @@ -48,6 +51,7 @@ final class CommentsDetailRouter { service: service, capabilities: capabilities, coordinator: coordinator, + draftStore: draftStore, titleResolver: titleResolver, tracker: tracker, noticePresenter: noticePresenter diff --git a/Modules/Sources/WordPressComments/ViewModels/CommentDetailViewModel.swift b/Modules/Sources/WordPressComments/ViewModels/CommentDetailViewModel.swift index ac25da7e76c5..5f77fd4f3d40 100644 --- a/Modules/Sources/WordPressComments/ViewModels/CommentDetailViewModel.swift +++ b/Modules/Sources/WordPressComments/ViewModels/CommentDetailViewModel.swift @@ -49,6 +49,7 @@ final class CommentDetailViewModel: ObservableObject { /// spinner and gates the toolbar synchronously (the coordinator's /// `isMutating` only flips after the first suspension). @Published private(set) var pendingAction: CommentModerationAction? + @Published var composer: CommentComposerViewModel? // non-nil = the composer sheet is presented /// Set by a `.deleted` event (a confirmed delete), which turns the toolbar /// off and makes the view dismiss itself. Cleared by a successful /// authoritative fetch or a later status change proving the comment exists @@ -91,6 +92,18 @@ final class CommentDetailViewModel: ObservableObject { actionableDetail != nil && pendingAction == nil } + /// Reply is reachable only for an active (approved/pending) comment, and + /// only behind the moderation toolbar gate: the reply chain assumes a + /// moderator (core auto-approves the reply; a pending parent is approved + /// alongside it). Non-moderator reply is a follow-up that needs its own + /// capability check and a chain that doesn't assume auto-approval. + var canReply: Bool { + switch actionableDetail?.status { + case .approved, .pending: true + case .spam, .trash, .other, nil: false + } + } + var trashConfirmation: TrashConfirmation { switch numberOfReplies { case .none: .generic @@ -109,10 +122,33 @@ final class CommentDetailViewModel: ObservableObject { return loadedDetail } + /// Presents the composer in reply mode. A no-op unless `canReply` (which + /// already covers "no mutation in flight" and "no composer already + /// presented"). + func replyTapped() { + guard canReply, composer == nil, let detail = loadedDetail else { return } + composer = CommentComposerViewModel( + mode: .reply(parent: detail), + coordinator: coordinator, + draftStore: draftStore, + tracker: tracker + ) + } + + /// Dismisses the composer sheet. A successful reply also posts its + /// notice. + func composerFinished(_ outcome: CommentComposerViewModel.Outcome) { + composer = nil + if case .replied(let replyNotice) = outcome { + noticePresenter?.present(title: replyNotice) + } + } + private let seed: CommentListItem? private let service: any CommentsServiceProtocol private let capabilities: CommentsCapabilityResolver private let coordinator: CommentsModerationCoordinator + private let draftStore: any CommentDraftStoring private let titleResolver: PostTitleResolver /// Fires `.detailViewed` once per screen, on the first successful fetch. private let tracker: (any CommentsTracker)? @@ -129,6 +165,7 @@ final class CommentDetailViewModel: ObservableObject { service: any CommentsServiceProtocol, capabilities: CommentsCapabilityResolver, coordinator: CommentsModerationCoordinator, + draftStore: any CommentDraftStoring, titleResolver: PostTitleResolver, tracker: (any CommentsTracker)? = nil, noticePresenter: (any NoticePresenting)? = nil @@ -139,6 +176,7 @@ final class CommentDetailViewModel: ObservableObject { self.capabilities = capabilities canModerate = capabilities.canModerate self.coordinator = coordinator + self.draftStore = draftStore self.titleResolver = titleResolver self.tracker = tracker self.noticePresenter = noticePresenter diff --git a/Modules/Sources/WordPressComments/Views/CommentsHostingController.swift b/Modules/Sources/WordPressComments/Views/CommentsHostingController.swift index 89655f894031..eafaa026bb76 100644 --- a/Modules/Sources/WordPressComments/Views/CommentsHostingController.swift +++ b/Modules/Sources/WordPressComments/Views/CommentsHostingController.swift @@ -11,7 +11,8 @@ public enum CommentsHostingController { client: WordPressClient, makeContentRenderer: @escaping @MainActor () -> any CommentContentRendering, tracker: any CommentsTracker, - noticePresenter: any NoticePresenting + noticePresenter: any NoticePresenting, + draftStore: any CommentDraftStoring ) -> UIViewController { let service = CommentsService(client: client) let titleResolver = PostTitleResolver(fetcher: PostTitleResolver.liveFetcher(client: client)) @@ -24,6 +25,7 @@ public enum CommentsHostingController { service: service, capabilities: CommentsCapabilities(client: client), coordinator: coordinator, + draftStore: draftStore, titleResolver: titleResolver, tracker: tracker, noticePresenter: noticePresenter, diff --git a/Modules/Sources/WordPressComments/Views/Detail/CommentComposerView.swift b/Modules/Sources/WordPressComments/Views/Detail/CommentComposerView.swift new file mode 100644 index 000000000000..c134ab74d6b6 --- /dev/null +++ b/Modules/Sources/WordPressComments/Views/Detail/CommentComposerView.swift @@ -0,0 +1,139 @@ +import SwiftUI + +/// The reply composer sheet, presented from the detail screen. Cancelling a +/// dirty reply asks whether to keep or discard its draft. +struct CommentComposerView: View { + @ObservedObject var viewModel: CommentComposerViewModel + let onFinished: (CommentComposerViewModel.Outcome) -> Void + let onDismiss: () -> Void + + @FocusState private var editorFocused: Bool + @State private var isCancelConfirmationPresented = false + + var body: some View { + NavigationStack { + VStack(alignment: .leading, spacing: 0) { + parentSnippet(viewModel.parentPreview) + if viewModel.showsApproveNote { + approveNote + } + textEditor + if let error = viewModel.errorMessage { + errorBanner(error) + } + } + .navigationTitle(viewModel.title) + .navigationBarTitleDisplayMode(.inline) + .toolbar { toolbarContent } + .onAppear { editorFocused = true } + } + .interactiveDismissDisabled(viewModel.isDirty || viewModel.isSending) + .confirmationDialog("", isPresented: $isCancelConfirmationPresented) { + cancelConfirmationActions + } + } + + @ViewBuilder + private func parentSnippet(_ parent: CommentListItem) -> some View { + VStack(alignment: .leading, spacing: 2) { + Text(parent.authorName) + .font(.headline) + Text(parent.snippet) + .font(.subheadline) + .foregroundStyle(.secondary) + .lineLimit(1) + } + .frame(maxWidth: .infinity, alignment: .leading) + .padding() + Divider() + } + + private var approveNote: some View { + Label(Strings.composerApproveNote, systemImage: "info.circle") + .font(.footnote) + .foregroundStyle(.secondary) + .padding(.horizontal) + .padding(.top, 8) + } + + private var textEditor: some View { + TextEditor(text: $viewModel.text) + .focused($editorFocused) + .disabled(viewModel.isSending) + .padding(.horizontal, 12) + .overlay(alignment: .topLeading) { + if viewModel.text.isEmpty { + Text(Strings.composerPlaceholder) + .foregroundStyle(.tertiary) + .padding(.horizontal, 16) + .padding(.top, 8) + .allowsHitTesting(false) + } + } + } + + private func errorBanner(_ message: String) -> some View { + Text(message) + .font(.footnote) + .foregroundStyle(.red) + .padding(.horizontal) + .padding(.vertical, 8) + } + + @ToolbarContentBuilder + private var toolbarContent: some ToolbarContent { + ToolbarItem(placement: .cancellationAction) { + Button(Strings.composerCancel) { handleCancel() } + .disabled(viewModel.isSending) + } + ToolbarItem(placement: .confirmationAction) { + if viewModel.isSending { + ProgressView() + .accessibilityLabel(viewModel.sendButtonTitle) + } else { + Button(viewModel.sendButtonTitle) { + Task { + if let outcome = await viewModel.send() { + onFinished(outcome) + } + } + } + .disabled(!viewModel.canSend) + } + } + } + + /// A dirty reply offers to keep or discard its draft. + @ViewBuilder + private var cancelConfirmationActions: some View { + Button(Strings.composerSaveDraft) { + viewModel.saveDraft() + onDismiss() + } + Button(Strings.composerDeleteDraft, role: .destructive) { + viewModel.deleteDraft() + onDismiss() + } + Button(Strings.composerKeepEditing, role: .cancel) {} + } + + private func handleCancel() { + guard viewModel.isDirty else { + onDismiss() + return + } + isCancelConfirmationPresented = true + } +} + +#if DEBUG +#Preview("Reply") { + let coordinator = CommentsModerationCoordinator(service: PreviewCommentsService()) + let viewModel = CommentComposerViewModel( + mode: .reply(parent: .preview(status: .pending)), + coordinator: coordinator, + draftStore: PreviewCommentDraftStore() + ) + return CommentComposerView(viewModel: viewModel, onFinished: { _ in }, onDismiss: {}) +} +#endif diff --git a/Modules/Sources/WordPressComments/Views/Detail/CommentDetailView.swift b/Modules/Sources/WordPressComments/Views/Detail/CommentDetailView.swift index 52ed56e7488d..cb782c4619cd 100644 --- a/Modules/Sources/WordPressComments/Views/Detail/CommentDetailView.swift +++ b/Modules/Sources/WordPressComments/Views/Detail/CommentDetailView.swift @@ -42,6 +42,18 @@ struct CommentDetailView: View { .onChange(of: viewModel.isDeleted) { _, isDeleted in if isDeleted { dismiss() } } + .sheet( + isPresented: Binding(get: { viewModel.composer != nil }, set: { if !$0 { viewModel.composer = nil } }) + ) { + if let composer = viewModel.composer { + CommentComposerView( + viewModel: composer, + onFinished: { viewModel.composerFinished($0) }, + onDismiss: { viewModel.composer = nil } + ) + .presentationDetents([.large]) + } + } } private var fixedRegions: some View { @@ -111,6 +123,13 @@ struct CommentDetailView: View { @ToolbarContentBuilder private var trailingToolbarItems: some ToolbarContent { + ToolbarItem(placement: .topBarTrailing) { + if viewModel.canReply { + Button(Strings.detailReply, systemImage: "arrowshape.turn.up.left") { + viewModel.replyTapped() + } + } + } ToolbarItem(placement: .topBarTrailing) { let link = viewModel.loadedDetail?.link let menuAction = viewModel.toolbarModel.menuAction @@ -180,6 +199,7 @@ private final class StubContentRenderer: NSObject, CommentContentRendering { service: service, capabilities: CommentsCapabilityResolver(capabilities: PreviewCapabilities()), coordinator: coordinator, + draftStore: PreviewCommentDraftStore(), titleResolver: titleResolver ) return NavigationStack { diff --git a/Modules/Sources/WordPressComments/Views/PreviewSupport.swift b/Modules/Sources/WordPressComments/Views/PreviewSupport.swift index 69a4b5dc68f7..e77e212f6d35 100644 --- a/Modules/Sources/WordPressComments/Views/PreviewSupport.swift +++ b/Modules/Sources/WordPressComments/Views/PreviewSupport.swift @@ -34,4 +34,11 @@ final class PreviewCommentsService: CommentsServiceProtocol { struct PreviewCapabilities: CommentsCapabilitiesProtocol { func canModerateComments() async throws -> Bool { true } } + +@MainActor +final class PreviewCommentDraftStore: CommentDraftStoring { + func loadDraft(commentID: Int64) -> String? { nil } + func saveDraft(_ text: String, commentID: Int64) {} + func deleteDraft(commentID: Int64) {} +} #endif diff --git a/Modules/Tests/WordPressCommentsTests/CommentDetailViewModelTests.swift b/Modules/Tests/WordPressCommentsTests/CommentDetailViewModelTests.swift index aa815ab7926b..b7f514431e7a 100644 --- a/Modules/Tests/WordPressCommentsTests/CommentDetailViewModelTests.swift +++ b/Modules/Tests/WordPressCommentsTests/CommentDetailViewModelTests.swift @@ -267,7 +267,7 @@ struct CommentDetailViewModelTests { #expect(vm.content == contentBeforeEvent) #expect(vm.header == headerBeforeEvent) - #expect(vm.isTerminated == false) + #expect(!vm.isDeleted) } @Test func trashConfirmationVariants() async { @@ -452,4 +452,136 @@ struct CommentDetailViewModelTests { await waitUntil { !noticePresenter.presented.isEmpty } #expect(noticePresenter.presented == ["That action couldn't be completed. Please try again."]) } + + // MARK: - Reply/edit composer gating and presentation + + @Test func canReplyRequiresModerationAndActiveStatus() async { + func makeLoadedVM(status: CommentStatus) async -> CommentDetailViewModel { + let service = FakeCommentsService() + service.fetchCommentResult = .success(makeDetail(id: 1, status: status, editContext: true)) + let vm = makeVM(service: service) + await vm.onAppear() + return vm + } + + let approvedVM = await makeLoadedVM(status: .approved) + #expect(approvedVM.canReply == true) + + let pendingVM = await makeLoadedVM(status: .hold) + #expect(pendingVM.canReply == true) + + let spamVM = await makeLoadedVM(status: .spam) + #expect(spamVM.canReply == false) + + let trashVM = await makeLoadedVM(status: .trash) + #expect(trashVM.canReply == false) + + let otherVM = await makeLoadedVM(status: .custom("draft")) + #expect(otherVM.canReply == false) + + // A demoted capability hides the toolbar (and so blocks reply) + // regardless of the loaded status. + let cannotModerateCapabilities = FakeCommentsCapabilities() + cannotModerateCapabilities.canModerate = false + let cannotModerateService = FakeCommentsService() + cannotModerateService.fetchCommentResult = .success(makeDetail(id: 1, status: .approved, editContext: false)) + let cannotModerateVM = makeVM(service: cannotModerateService, capabilities: cannotModerateCapabilities) + await cannotModerateVM.onAppear() + #expect(cannotModerateVM.canReply == false) + + // Before the authoritative fetch lands (seed only), canReply is false. + let seed = makeItem(id: 1, status: .approved) + let unfetchedVM = makeVM(seed: seed, service: FakeCommentsService()) + #expect(unfetchedVM.canReply == false) + } + + // MARK: - Status change refreshes the loaded detail (not just the header) + + @Test(arguments: [CommentListItem.Status.spam, .trash]) + func statusChangeToSpamOrTrashRefreshesCanReplyGating(_ to: CommentListItem.Status) async { + let service = FakeCommentsService() + service.fetchCommentResult = .success(makeDetail(id: 1, status: .approved, editContext: true)) + let coordinator = CommentsModerationCoordinator(service: FakeCommentsService()) + let vm = makeVM(service: service, coordinator: coordinator) + + await vm.onAppear() + #expect(vm.canReply == true) + + coordinator.noteExternalStatus(id: 1, to: to) + + // The header (the screen's display source of truth) still tracks the + // new status, matching the existing header-tracking behavior. + #expect(vm.header?.status == to) + // canReply reads loadedDetail.status directly and excludes spam/trash; + // it must follow moderation instead of staying stuck on the + // pre-moderation status (Reply no longer shown for a spam/trash + // comment). + #expect(vm.canReply == false) + } + + @Test func statusChangeToPendingKeepsCanReplyTrue() async { + let service = FakeCommentsService() + service.fetchCommentResult = .success(makeDetail(id: 1, status: .approved, editContext: true)) + let coordinator = CommentsModerationCoordinator(service: FakeCommentsService()) + let vm = makeVM(service: service, coordinator: coordinator) + + await vm.onAppear() + #expect(vm.canReply == true) + + coordinator.noteExternalStatus(id: 1, to: .pending) + + #expect(vm.header?.status == .pending) + #expect(vm.canReply == true) + } + + @Test func replyTappedPresentsReplyComposer() async { + let service = FakeCommentsService() + let detail = makeDetail(id: 1, status: .approved, editContext: true) + service.fetchCommentResult = .success(detail) + let vm = makeVM(service: service) + await vm.onAppear() + #expect(vm.canReply == true) + + vm.replyTapped() + + #expect(vm.composer != nil) + #expect(vm.composer?.mode == .reply(parent: detail)) + } + + @Test func replyTappedIgnoredWhileMutating() async { + let coordinatorService = BlockingCommentsService() + let coordinator = CommentsModerationCoordinator(service: coordinatorService) + let service = FakeCommentsService() + service.fetchCommentResult = .success(makeDetail(id: 1, status: .approved, editContext: true)) + let vm = makeVM(service: service, coordinator: coordinator) + + await vm.onAppear() + let spam = Task { + try? await coordinator.perform(.spam, on: makeDetail(id: 1, status: .approved, editContext: true)) + } + await waitUntil { !coordinatorService.setStatusInvocations.isEmpty } + #expect(coordinator.isMutating(id: 1)) + + vm.replyTapped() + + #expect(vm.composer == nil) + + coordinatorService.resolveSetStatus(callIndex: 0, with: makeDetail(id: 1, status: .spam, editContext: true)) + _ = await spam.value + } + + @Test func composerFinishedRepliedPresentsNoticeAndDismisses() async { + let noticePresenter = FakeNoticePresenter() + let service = FakeCommentsService() + service.fetchCommentResult = .success(makeDetail(id: 1, status: .approved, editContext: true)) + let vm = makeVM(service: service, noticePresenter: noticePresenter) + await vm.onAppear() + vm.replyTapped() + #expect(vm.composer != nil) + + vm.composerFinished(.replied(notice: "Reply sent.")) + + #expect(vm.composer == nil) + #expect(noticePresenter.presented == ["Reply sent."]) + } } diff --git a/Modules/Tests/WordPressCommentsTests/CommentsDetailRouterTests.swift b/Modules/Tests/WordPressCommentsTests/CommentsDetailRouterTests.swift index b1f985e4e691..e97f710dfb57 100644 --- a/Modules/Tests/WordPressCommentsTests/CommentsDetailRouterTests.swift +++ b/Modules/Tests/WordPressCommentsTests/CommentsDetailRouterTests.swift @@ -39,6 +39,7 @@ struct CommentsDetailRouterTests { service: FakeCommentsService(), capabilities: capabilities, coordinator: CommentsModerationCoordinator(service: FakeCommentsService()), + draftStore: FakeCommentDraftStore(), titleResolver: PostTitleResolver(fetcher: { _ in .init(titles: [:]) }), tracker: nil, noticePresenter: FakeNoticePresenter(), diff --git a/Modules/Tests/WordPressCommentsTests/Support/CommentDetailTestHelpers.swift b/Modules/Tests/WordPressCommentsTests/Support/CommentDetailTestHelpers.swift index b143ab47ce9a..7203ea18f4e1 100644 --- a/Modules/Tests/WordPressCommentsTests/Support/CommentDetailTestHelpers.swift +++ b/Modules/Tests/WordPressCommentsTests/Support/CommentDetailTestHelpers.swift @@ -31,6 +31,7 @@ func makeVM( capabilities: FakeCommentsCapabilities = FakeCommentsCapabilities(), resolver: CommentsCapabilityResolver? = nil, coordinator: CommentsModerationCoordinator? = nil, + draftStore: any CommentDraftStoring = FakeCommentDraftStore(), tracker: (any CommentsTracker)? = nil, noticePresenter: (any NoticePresenting)? = nil ) -> CommentDetailViewModel { @@ -40,6 +41,7 @@ func makeVM( service: service, capabilities: resolver ?? CommentsCapabilityResolver(capabilities: capabilities), coordinator: coordinator ?? CommentsModerationCoordinator(service: FakeCommentsService()), + draftStore: draftStore, titleResolver: makeResolver(), tracker: tracker, noticePresenter: noticePresenter diff --git a/WordPress/Classes/ViewRelated/Comments/CommentsRouting.swift b/WordPress/Classes/ViewRelated/Comments/CommentsRouting.swift index 093f19f4a592..93e864511bb7 100644 --- a/WordPress/Classes/ViewRelated/Comments/CommentsRouting.swift +++ b/WordPress/Classes/ViewRelated/Comments/CommentsRouting.swift @@ -22,7 +22,7 @@ enum CommentsRouting { static func makeViewController(for blog: Blog) -> UIViewController? { guard FeatureFlag.commentsV2.enabled, let site = try? WordPressSite(blog: blog), - case .selfHosted = site.flavor + case .selfHosted(let credentials) = site.flavor else { return nil } @@ -31,7 +31,8 @@ enum CommentsRouting { client: client, makeContentRenderer: { CommentsWebContentRendererAdapter() }, tracker: CommentsTrackerAdapter(blogProperties: blog.analyticsProperties), - noticePresenter: NoticePresenterAdapter() + noticePresenter: NoticePresenterAdapter(), + draftStore: UserDefaultsCommentDraftStore(siteURL: site.siteURL, username: credentials.username) ) } } From 4e4554a145adf91046ed16f981931edb8f6b880f Mon Sep 17 00:00:00 2001 From: Tony Li Date: Mon, 31 Aug 2026 20:36:58 +1200 Subject: [PATCH 08/10] Drop a restored reply draft the user cleared Cancelling or swiping away a blank reply skipped the draft prompt and left the previously saved draft in storage, so it came back on the next open. The composer now deletes the stored draft when it closes with blank text. --- .../ViewModels/CommentComposerViewModel.swift | 8 ++++++++ .../Views/Detail/CommentComposerView.swift | 1 + .../CommentComposerViewModelTests.swift | 16 ++++++++++++++++ 3 files changed, 25 insertions(+) diff --git a/Modules/Sources/WordPressComments/ViewModels/CommentComposerViewModel.swift b/Modules/Sources/WordPressComments/ViewModels/CommentComposerViewModel.swift index ede438dc3f0d..77a644408c1b 100644 --- a/Modules/Sources/WordPressComments/ViewModels/CommentComposerViewModel.swift +++ b/Modules/Sources/WordPressComments/ViewModels/CommentComposerViewModel.swift @@ -120,6 +120,14 @@ final class CommentComposerViewModel: ObservableObject { draftStore.deleteDraft(commentID: parent.id) } + /// Runs when the sheet closes. The blank exits (Cancel, swipe-down) skip + /// the draft prompt, so a restored draft the user cleared is dropped here + /// instead of coming back on the next open. + func deleteDraftIfBlank() { + guard case .reply(let parent) = mode, trimmedText.isEmpty else { return } + draftStore.deleteDraft(commentID: parent.id) + } + /// Words the post-send notice: a duplicate confirms an earlier send /// already landed, a pending status means the reply itself needs /// moderation, otherwise it posted straight away. diff --git a/Modules/Sources/WordPressComments/Views/Detail/CommentComposerView.swift b/Modules/Sources/WordPressComments/Views/Detail/CommentComposerView.swift index c134ab74d6b6..1de9ea1d8061 100644 --- a/Modules/Sources/WordPressComments/Views/Detail/CommentComposerView.swift +++ b/Modules/Sources/WordPressComments/Views/Detail/CommentComposerView.swift @@ -28,6 +28,7 @@ struct CommentComposerView: View { .onAppear { editorFocused = true } } .interactiveDismissDisabled(viewModel.isDirty || viewModel.isSending) + .onDisappear { viewModel.deleteDraftIfBlank() } .confirmationDialog("", isPresented: $isCancelConfirmationPresented) { cancelConfirmationActions } diff --git a/Modules/Tests/WordPressCommentsTests/CommentComposerViewModelTests.swift b/Modules/Tests/WordPressCommentsTests/CommentComposerViewModelTests.swift index bd7ac670f766..5f87657af93c 100644 --- a/Modules/Tests/WordPressCommentsTests/CommentComposerViewModelTests.swift +++ b/Modules/Tests/WordPressCommentsTests/CommentComposerViewModelTests.swift @@ -220,4 +220,20 @@ struct CommentComposerViewModelTests { #expect(store.deleted.contains(1)) } + @Test func deleteDraftIfBlankDeletesOnlyWhenTextIsBlank() { + let store = FakeCommentDraftStore() + store.preloadDraft("draft", commentID: 1) + let vm = CommentComposerViewModel( + mode: .reply(parent: makeDetail(id: 1)), + coordinator: makeCoordinator(), + draftStore: store + ) + + vm.deleteDraftIfBlank() + #expect(store.deleted.isEmpty) + + vm.text = " \n" + vm.deleteDraftIfBlank() + #expect(store.deleted == [1]) + } } From e7c75f080ad0675f18e9fb8bdc4f17fe1d5c726c Mon Sep 17 00:00:00 2001 From: Tony Li Date: Wed, 2 Sep 2026 12:02:59 +1200 Subject: [PATCH 09/10] Show the detail Reply button disabled until the fetch lands Reply was hidden until the whole detail load finished, so it popped in late. It now renders from the seed status as soon as the moderation capability resolves, matching the bottom toolbar, and stays disabled until the authoritative fetch lands. --- .../ViewModels/CommentDetailViewModel.swift | 10 +++++ .../Views/Detail/CommentDetailView.swift | 3 +- .../CommentDetailViewModelTests.swift | 37 ++++++++++++++++++- 3 files changed, 48 insertions(+), 2 deletions(-) diff --git a/Modules/Sources/WordPressComments/ViewModels/CommentDetailViewModel.swift b/Modules/Sources/WordPressComments/ViewModels/CommentDetailViewModel.swift index 5f77fd4f3d40..de696da9f0e9 100644 --- a/Modules/Sources/WordPressComments/ViewModels/CommentDetailViewModel.swift +++ b/Modules/Sources/WordPressComments/ViewModels/CommentDetailViewModel.swift @@ -92,6 +92,16 @@ final class CommentDetailViewModel: ObservableObject { actionableDetail != nil && pendingAction == nil } + /// Whether the Reply button renders. Reads the same seed-or-fetched status + /// the toolbar does, so the button appears as soon as the capability + /// resolves instead of after the whole load; `canReply` enables it. + var showsReply: Bool { + switch toolbarModel { + case .approved, .pending: true + case .inBin, .hidden: false + } + } + /// Reply is reachable only for an active (approved/pending) comment, and /// only behind the moderation toolbar gate: the reply chain assumes a /// moderator (core auto-approves the reply; a pending parent is approved diff --git a/Modules/Sources/WordPressComments/Views/Detail/CommentDetailView.swift b/Modules/Sources/WordPressComments/Views/Detail/CommentDetailView.swift index cb782c4619cd..3e2ed1d2ad9e 100644 --- a/Modules/Sources/WordPressComments/Views/Detail/CommentDetailView.swift +++ b/Modules/Sources/WordPressComments/Views/Detail/CommentDetailView.swift @@ -124,10 +124,11 @@ struct CommentDetailView: View { @ToolbarContentBuilder private var trailingToolbarItems: some ToolbarContent { ToolbarItem(placement: .topBarTrailing) { - if viewModel.canReply { + if viewModel.showsReply { Button(Strings.detailReply, systemImage: "arrowshape.turn.up.left") { viewModel.replyTapped() } + .disabled(!viewModel.canReply) } } ToolbarItem(placement: .topBarTrailing) { diff --git a/Modules/Tests/WordPressCommentsTests/CommentDetailViewModelTests.swift b/Modules/Tests/WordPressCommentsTests/CommentDetailViewModelTests.swift index b7f514431e7a..aeabb9728fc6 100644 --- a/Modules/Tests/WordPressCommentsTests/CommentDetailViewModelTests.swift +++ b/Modules/Tests/WordPressCommentsTests/CommentDetailViewModelTests.swift @@ -439,7 +439,11 @@ struct CommentDetailViewModelTests { let noticePresenter = FakeNoticePresenter() let coordinatorService = BlockingCommentsService() let coordinator = CommentsModerationCoordinator(service: coordinatorService) - var vm: CommentDetailViewModel? = await makeLoadedVM(status: .hold, coordinator: coordinator, noticePresenter: noticePresenter) + var vm: CommentDetailViewModel? = await makeLoadedVM( + status: .hold, + coordinator: coordinator, + noticePresenter: noticePresenter + ) vm!.perform(.approve) await waitUntil { !coordinatorService.setStatusInvocations.isEmpty } @@ -495,6 +499,37 @@ struct CommentDetailViewModelTests { #expect(unfetchedVM.canReply == false) } + @Test func showsReplyRendersDisabledButtonBeforeFetchCompletes() async { + let service = BlockingCommentsService() + let vm = makeVM(seed: makeItem(id: 1, status: .approved), service: service) + // Capability unresolved: nothing renders yet. + #expect(!vm.showsReply) + + async let appear: Void = vm.onAppear() + await waitUntil { !service.fetchCommentInvocations.isEmpty } + + // Capability resolved, fetch in flight: the button renders from the + // seed status but stays disabled, like the toolbar. + #expect(vm.showsReply) + #expect(!vm.canReply) + + service.resolveFetch(callIndex: 0, with: makeDetail(id: 1, editContext: true)) + await appear + + #expect(vm.showsReply) + #expect(vm.canReply) + } + + @Test func showsReplyHiddenForBinnedAndCustomStatuses() async { + for status in [CommentStatus.spam, .trash, .custom("draft")] { + let service = FakeCommentsService() + service.fetchCommentResult = .success(makeDetail(id: 1, status: status, editContext: true)) + let vm = makeVM(seed: makeItem(id: 1, status: status), service: service) + await vm.onAppear() + #expect(!vm.showsReply, "\(status)") + } + } + // MARK: - Status change refreshes the loaded detail (not just the header) @Test(arguments: [CommentListItem.Status.spam, .trash]) From 8276878ab3e79262217b5f8059c312c05332c359 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Wed, 2 Sep 2026 17:06:50 +1200 Subject: [PATCH 10/10] Defer the list reload for a new reply to the next appearance A reply is only sent from the detail screen, so no list tab is visible when replyCreated lands. Reloading right away cost one page-one fetch per reply while the list was hidden; the tab now just marks itself stale and the root controller's appearance reloads it once on return. Moderation events keep reloading eagerly, since their tab can be visible. --- .../ViewModels/CommentsListViewModel.swift | 32 ++++++++++++------- .../Views/CommentsHostingController.swift | 14 ++++---- .../CommentsListViewModelEventTests.swift | 26 +++++++++++++++ 3 files changed, 53 insertions(+), 19 deletions(-) diff --git a/Modules/Sources/WordPressComments/ViewModels/CommentsListViewModel.swift b/Modules/Sources/WordPressComments/ViewModels/CommentsListViewModel.swift index 8673e8a2402a..ca681776694f 100644 --- a/Modules/Sources/WordPressComments/ViewModels/CommentsListViewModel.swift +++ b/Modules/Sources/WordPressComments/ViewModels/CommentsListViewModel.swift @@ -22,9 +22,9 @@ final class CommentsListViewModel: ObservableObject { /// A change event couldn't be reconciled in place (see `apply(_:)`); /// page one is refetching behind the outdated rows. case reloading - /// The rows are outdated and a page-one reload is pending: either - /// just scheduled by `markStale`, or deferred to the next appearance - /// after a reload attempt failed. + /// The rows are outdated and a page-one reload is pending: just + /// scheduled by `markStale`, deferred to the next appearance, or + /// deferred after a reload attempt failed. case awaitingReload } @@ -137,11 +137,15 @@ final class CommentsListViewModel: ObservableObject { case .deleted(let id): removeItem(id: id) case .replyCreated(_, let replyStatus): - guard hasLoaded else { return } + guard hasLoaded, filter.matches(replyStatus) else { return } // A new reply belongs at some position in the tabs matching its // status; a paged list cannot insert at the right position, so - // mark those tabs stale (same reasoning as restore above). - if filter.matches(replyStatus) { markStale() } + // mark those tabs stale (same reasoning as restore above). The + // reload waits for the next appearance: a reply is only sent from + // the detail screen, so no list tab is visible, and reloading + // right away would cost one fetch per reply while the list is + // hidden instead of one on return. + markStale(reloadNow: false) } } @@ -155,16 +159,20 @@ final class CommentsListViewModel: ObservableObject { invalidateInFlightFetches() } - /// Marks the tab stale and reloads page one right away, so a visible tab - /// doesn't wait for a tab switch. Callers guard `hasLoaded`. A reload - /// already in flight keeps its state: its fetch is invalidated below and - /// refetches itself, and `onAppear` ignores the extra kick. - private func markStale() { + /// Marks the tab stale and, with `reloadNow`, reloads page one right away + /// so a visible tab doesn't wait for a tab switch; otherwise the reload + /// runs on the next appearance (`reloadIfStale`). Callers guard + /// `hasLoaded`. A reload already in flight keeps its state: its fetch is + /// invalidated below and refetches itself, and `onAppear` ignores the + /// extra kick. + private func markStale(reloadNow: Bool = true) { if state == .loaded { state = .awaitingReload } invalidateInFlightFetches() - Task { await onAppear() } + if reloadNow { + Task { await onAppear() } + } } /// Bumps `generation` so any list fetch already in flight (which captured diff --git a/Modules/Sources/WordPressComments/Views/CommentsHostingController.swift b/Modules/Sources/WordPressComments/Views/CommentsHostingController.swift index eafaa026bb76..d7d6aa4bf0ad 100644 --- a/Modules/Sources/WordPressComments/Views/CommentsHostingController.swift +++ b/Modules/Sources/WordPressComments/Views/CommentsHostingController.swift @@ -56,13 +56,13 @@ public enum CommentsHostingController { } } -/// Hosts the tab view and, on each appearance, retries the list tabs whose -/// stale reload failed while the list was off screen (typically behind a -/// pushed detail screen). `markStale()` already reloads eagerly at event -/// time; this is only the retry for when that reload failed. It lives in the -/// controller because SwiftUI's `onAppear` and `.task` do not re-run when a -/// UIKit-pushed controller is popped back to this one (verified on a -/// simulator). +/// Hosts the tab view and, on each appearance, reloads the list tabs still +/// awaiting a stale reload: one deferred to this appearance (a reply sent +/// from the detail screen) or one that failed while the list was off screen +/// (typically behind a pushed detail screen). Moderation events reload +/// eagerly at event time. It lives in the controller because SwiftUI's +/// `onAppear` and `.task` do not re-run when a UIKit-pushed controller is +/// popped back to this one (verified on a simulator). final class CommentsRootHostingController: UIHostingController { private let listViewModels: [CommentsListViewModel] diff --git a/Modules/Tests/WordPressCommentsTests/CommentsListViewModelEventTests.swift b/Modules/Tests/WordPressCommentsTests/CommentsListViewModelEventTests.swift index 977780bec582..7860ec851eaf 100644 --- a/Modules/Tests/WordPressCommentsTests/CommentsListViewModelEventTests.swift +++ b/Modules/Tests/WordPressCommentsTests/CommentsListViewModelEventTests.swift @@ -489,6 +489,32 @@ struct CommentsListViewModelEventTests { } } + @Test func replyCreatedDefersTheReloadToTheNextAppearance() async { + let service = FakeCommentsService() + service.queuedResults = [ + .success(makePage(items: [makeItem(id: 1)], hasNext: false)), + .success(makePage(items: [makeItem(id: 2), makeItem(id: 1)], hasNext: false)) + ] + let viewModel = CommentsListViewModel(filter: .all, service: service) + await viewModel.onAppear() + + viewModel.apply(.replyCreated(parentID: 1, replyStatus: .approved)) + viewModel.apply(.replyCreated(parentID: 1, replyStatus: .approved)) + for _ in 0..<10 { await Task.yield() } + + // No list tab is visible while a reply is sent, so the stale tab + // keeps its rows and issues no fetch until it next appears. + #expect(viewModel.state == .awaitingReload) + #expect(viewModel.items.map(\.id) == [1]) + #expect(service.requests.count == 1) + + await viewModel.reloadIfStale() + + #expect(viewModel.state == .loaded) + #expect(viewModel.items.map(\.id) == [2, 1]) + #expect(service.requests.count == 2) + } + @Test func replyCreatedBeforeLoadIsIgnored() async { let service = FakeCommentsService() let viewModel = CommentsListViewModel(filter: .all, service: service)