From 0f351106812854e9880f3ac9e3ffe2c4b002ac77 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Wed, 2 Sep 2026 12:11:14 +1200 Subject: [PATCH 1/2] Use the octagon glyph for the comment Spam action --- .../Views/Detail/CommentModerationToolbar.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/Sources/WordPressComments/Views/Detail/CommentModerationToolbar.swift b/Modules/Sources/WordPressComments/Views/Detail/CommentModerationToolbar.swift index 3eb4ec87800d..ca7dc872b6b0 100644 --- a/Modules/Sources/WordPressComments/Views/Detail/CommentModerationToolbar.swift +++ b/Modules/Sources/WordPressComments/Views/Detail/CommentModerationToolbar.swift @@ -50,7 +50,7 @@ extension CommentModerationAction { switch self { case .approve: "checkmark" case .unapprove: "clock" - case .spam: "exclamationmark.bubble" + case .spam: "exclamationmark.octagon" case .trash: "trash" case .restore: "arrow.uturn.backward" case .delete: "trash" From 1b59c060931ac5ef0a513baf5474b6bd0ec96589 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Wed, 2 Sep 2026 17:38:03 +1200 Subject: [PATCH 2/2] Resolve the comment moderation capability through one shared resolver The detail screen awaited the moderation capability before it could render its toolbar and nav bar controls, so they missed the first frame and appeared after the push transition. CommentsCapabilityResolver now owns the lookup: the router prefetches it once while the list loads and caches the answer, and each detail view model seeds from that cached value so its controls render on the first frame, then awaits it on load. The lookup throws so a failure is never cached; it retries on the next open and degrades the screen to read-only. --- .../Services/CommentsCapabilities.swift | 13 ++--- .../Services/CommentsCapabilityResolver.swift | 42 ++++++++++++++ .../Services/CommentsDetailRouter.swift | 8 ++- .../ViewModels/CommentDetailViewModel.swift | 15 +++-- .../Views/Detail/CommentDetailView.swift | 2 +- .../Views/PreviewSupport.swift | 2 +- .../CommentsCapabilityResolverTests.swift | 57 +++++++++++++++++++ .../CommentsDetailRouterTests.swift | 36 +++++++++--- .../Support/CommentDetailTestHelpers.swift | 14 ++++- .../Support/FakeCommentsCapabilities.swift | 8 ++- 10 files changed, 170 insertions(+), 27 deletions(-) create mode 100644 Modules/Sources/WordPressComments/Services/CommentsCapabilityResolver.swift create mode 100644 Modules/Tests/WordPressCommentsTests/CommentsCapabilityResolverTests.swift diff --git a/Modules/Sources/WordPressComments/Services/CommentsCapabilities.swift b/Modules/Sources/WordPressComments/Services/CommentsCapabilities.swift index 872fe271d11d..540e9a6a3d9b 100644 --- a/Modules/Sources/WordPressComments/Services/CommentsCapabilities.swift +++ b/Modules/Sources/WordPressComments/Services/CommentsCapabilities.swift @@ -1,18 +1,15 @@ import WordPressCore protocol CommentsCapabilitiesProtocol: Sendable { - /// Whether the current user can moderate comments. Resolved once per - /// detail screen; false also when the lookup fails, which degrades the - /// screen to read-only (view-context fetch, no author email or IP). - func canModerateComments() async -> Bool + /// Whether the current user can moderate comments. Throws when the lookup + /// fails, so a caller can decide whether to cache the answer. + func canModerateComments() async throws -> Bool } struct CommentsCapabilities: CommentsCapabilitiesProtocol { let client: WordPressClient - func canModerateComments() async -> Bool { - // A failed current-user request must not block a readable - // view-context detail screen. - (try? await client.currentUserCan(.moderateComments)) ?? false + func canModerateComments() async throws -> Bool { + try await client.currentUserCan(.moderateComments) } } diff --git a/Modules/Sources/WordPressComments/Services/CommentsCapabilityResolver.swift b/Modules/Sources/WordPressComments/Services/CommentsCapabilityResolver.swift new file mode 100644 index 000000000000..bc89e61bde18 --- /dev/null +++ b/Modules/Sources/WordPressComments/Services/CommentsCapabilityResolver.swift @@ -0,0 +1,42 @@ +/// Resolves the moderation capability once per comments session and caches +/// the answer, so every detail screen can read it synchronously at init (its +/// toolbar and nav bar items then render on the first frame and take part in +/// the push transition). Concurrent callers share one lookup. A failed lookup +/// is not cached: the value stays nil and the next call retries. +@MainActor +final class CommentsCapabilityResolver { + /// The capability once a lookup succeeded; nil while unknown. + private(set) var canModerate: Bool? + + private let capabilities: any CommentsCapabilitiesProtocol + private var lookup: Task? + + init(capabilities: any CommentsCapabilitiesProtocol) { + self.capabilities = capabilities + } + + /// Starts a lookup unless the answer is known or one is already running. + func prefetch() { + guard canModerate == nil else { return } + _ = lookupIfNeeded() + } + + /// The cached answer, else the result of the running or a fresh lookup. + /// Nil when that lookup fails. + func resolve() async -> Bool? { + if let canModerate { return canModerate } + return await lookupIfNeeded().value + } + + private func lookupIfNeeded() -> Task { + if let lookup { return lookup } + let task = Task { [weak self, capabilities] () -> Bool? in + let result = try? await capabilities.canModerateComments() + self?.canModerate = result + self?.lookup = nil + return result + } + lookup = task + return task + } +} diff --git a/Modules/Sources/WordPressComments/Services/CommentsDetailRouter.swift b/Modules/Sources/WordPressComments/Services/CommentsDetailRouter.swift index 89d6d670ffb4..402e1a15c007 100644 --- a/Modules/Sources/WordPressComments/Services/CommentsDetailRouter.swift +++ b/Modules/Sources/WordPressComments/Services/CommentsDetailRouter.swift @@ -10,7 +10,9 @@ final class CommentsDetailRouter { weak var host: UIViewController? private let service: any CommentsServiceProtocol - private let capabilities: any CommentsCapabilitiesProtocol + /// Shared by every detail view model, so the capability resolves once + /// while the list loads and later screens read it synchronously. + private let capabilities: CommentsCapabilityResolver private let coordinator: CommentsModerationCoordinator private let titleResolver: PostTitleResolver private let tracker: (any CommentsTracker)? @@ -27,17 +29,19 @@ final class CommentsDetailRouter { makeContentRenderer: @escaping @MainActor () -> any CommentContentRendering ) { self.service = service - self.capabilities = capabilities + self.capabilities = CommentsCapabilityResolver(capabilities: capabilities) self.coordinator = coordinator self.titleResolver = titleResolver self.tracker = tracker self.noticePresenter = noticePresenter self.makeContentRenderer = makeContentRenderer + self.capabilities.prefetch() } /// Builds the detail view model and screen for `id` (seeded from the list /// row when available) and pushes it onto the shared navigation stack. func open(id: Int64, seed: CommentListItem?) { + capabilities.prefetch() let viewModel = CommentDetailViewModel( commentID: id, seed: seed, diff --git a/Modules/Sources/WordPressComments/ViewModels/CommentDetailViewModel.swift b/Modules/Sources/WordPressComments/ViewModels/CommentDetailViewModel.swift index 66ca0a2d29f0..5a1ede3f41ae 100644 --- a/Modules/Sources/WordPressComments/ViewModels/CommentDetailViewModel.swift +++ b/Modules/Sources/WordPressComments/ViewModels/CommentDetailViewModel.swift @@ -2,8 +2,10 @@ import Combine import Foundation import WordPressShared -/// Drives the comment detail and moderation screen. Resolves the moderation -/// capability once, runs the authoritative detail/parent/reply-count fetches, +/// Drives the comment detail and moderation screen. Reads the moderation +/// capability from the shared resolver when already known (so the toolbar and +/// nav bar items render on the first frame), else awaits it once; runs the +/// authoritative detail/parent/reply-count fetches, /// and enforces the ordering rules the design requires: no action lands before /// the authoritative fetch, the toolbar is disabled while a mutation is in /// flight, and an open screen still hears late status changes by subscribing to @@ -109,7 +111,7 @@ final class CommentDetailViewModel: ObservableObject { private let seed: CommentListItem? private let service: any CommentsServiceProtocol - private let capabilities: any CommentsCapabilitiesProtocol + private let capabilities: CommentsCapabilityResolver private let coordinator: CommentsModerationCoordinator private let titleResolver: PostTitleResolver /// Fires `.detailViewed` once per screen, on the first successful fetch. @@ -125,7 +127,7 @@ final class CommentDetailViewModel: ObservableObject { commentID: Int64, seed: CommentListItem?, service: any CommentsServiceProtocol, - capabilities: any CommentsCapabilitiesProtocol, + capabilities: CommentsCapabilityResolver, coordinator: CommentsModerationCoordinator, titleResolver: PostTitleResolver, tracker: (any CommentsTracker)? = nil, @@ -135,6 +137,7 @@ final class CommentDetailViewModel: ObservableObject { self.seed = seed self.service = service self.capabilities = capabilities + canModerate = capabilities.canModerate self.coordinator = coordinator self.titleResolver = titleResolver self.tracker = tracker @@ -183,7 +186,9 @@ final class CommentDetailViewModel: ObservableObject { isLoading = true defer { isLoading = false } if canModerate == nil { - canModerate = await capabilities.canModerateComments() + // A failed lookup degrades this screen to read-only (view-context + // fetch, no author email or IP) without blocking it. + canModerate = await capabilities.resolve() ?? false } await runFetch() } diff --git a/Modules/Sources/WordPressComments/Views/Detail/CommentDetailView.swift b/Modules/Sources/WordPressComments/Views/Detail/CommentDetailView.swift index 5b90b60b2725..52ed56e7488d 100644 --- a/Modules/Sources/WordPressComments/Views/Detail/CommentDetailView.swift +++ b/Modules/Sources/WordPressComments/Views/Detail/CommentDetailView.swift @@ -178,7 +178,7 @@ private final class StubContentRenderer: NSObject, CommentContentRendering { commentID: 1, seed: nil, service: service, - capabilities: PreviewCapabilities(), + capabilities: CommentsCapabilityResolver(capabilities: PreviewCapabilities()), coordinator: coordinator, titleResolver: titleResolver ) diff --git a/Modules/Sources/WordPressComments/Views/PreviewSupport.swift b/Modules/Sources/WordPressComments/Views/PreviewSupport.swift index 6cfaace80b3b..1e09ae8112bf 100644 --- a/Modules/Sources/WordPressComments/Views/PreviewSupport.swift +++ b/Modules/Sources/WordPressComments/Views/PreviewSupport.swift @@ -31,6 +31,6 @@ final class PreviewCommentsService: CommentsServiceProtocol { } struct PreviewCapabilities: CommentsCapabilitiesProtocol { - func canModerateComments() async -> Bool { true } + func canModerateComments() async throws -> Bool { true } } #endif diff --git a/Modules/Tests/WordPressCommentsTests/CommentsCapabilityResolverTests.swift b/Modules/Tests/WordPressCommentsTests/CommentsCapabilityResolverTests.swift new file mode 100644 index 000000000000..370a9fa04e4d --- /dev/null +++ b/Modules/Tests/WordPressCommentsTests/CommentsCapabilityResolverTests.swift @@ -0,0 +1,57 @@ +import Testing +@testable import WordPressComments + +@MainActor +struct CommentsCapabilityResolverTests { + @Test func resolveCachesASuccessfulLookup() async { + let capabilities = FakeCommentsCapabilities() + capabilities.canModerate = false + let resolver = CommentsCapabilityResolver(capabilities: capabilities) + #expect(resolver.canModerate == nil) + + #expect(await resolver.resolve() == false) + #expect(await resolver.resolve() == false) + + #expect(resolver.canModerate == false) + #expect(capabilities.invocations == 1) + } + + @Test func concurrentResolvesShareOneLookup() async { + let capabilities = FakeCommentsCapabilities() + let resolver = CommentsCapabilityResolver(capabilities: capabilities) + + async let first = resolver.resolve() + async let second = resolver.resolve() + let results = await [first, second] + + #expect(results == [true, true]) + #expect(capabilities.invocations == 1) + } + + @Test func prefetchStartsOneLookupThatResolveAwaits() async { + let capabilities = FakeCommentsCapabilities() + let resolver = CommentsCapabilityResolver(capabilities: capabilities) + + resolver.prefetch() + resolver.prefetch() + #expect(await resolver.resolve() == true) + + #expect(capabilities.invocations == 1) + // Known: a later prefetch is a no-op. + resolver.prefetch() + #expect(capabilities.invocations == 1) + } + + @Test func failedLookupIsNotCachedAndRetries() async { + let capabilities = FakeCommentsCapabilities() + capabilities.error = FakeServiceError() + let resolver = CommentsCapabilityResolver(capabilities: capabilities) + + #expect(await resolver.resolve() == nil) + #expect(resolver.canModerate == nil) + + capabilities.error = nil + #expect(await resolver.resolve() == true) + #expect(capabilities.invocations == 2) + } +} diff --git a/Modules/Tests/WordPressCommentsTests/CommentsDetailRouterTests.swift b/Modules/Tests/WordPressCommentsTests/CommentsDetailRouterTests.swift index af97c03840a9..b1f985e4e691 100644 --- a/Modules/Tests/WordPressCommentsTests/CommentsDetailRouterTests.swift +++ b/Modules/Tests/WordPressCommentsTests/CommentsDetailRouterTests.swift @@ -8,19 +8,41 @@ struct CommentsDetailRouterTests { @Test func openPushesDetailOntoHostNavigationStack() { let host = UIViewController() let navigation = UINavigationController(rootViewController: host) - let router = CommentsDetailRouter( + let router = makeRouter(capabilities: FakeCommentsCapabilities()) + router.host = host + + router.open(id: 1, seed: nil) + + #expect(navigation.viewControllers.count == 2) + } + + @Test func resolvesCapabilityOnceAndRetriesAfterFailure() async { + let capabilities = FakeCommentsCapabilities() + capabilities.error = FakeServiceError() + let router = makeRouter(capabilities: capabilities) + router.host = UINavigationController(rootViewController: UIViewController()).viewControllers[0] + + // Resolved while the list loads; the failure is not cached. + await waitUntil { capabilities.invocations == 1 } + capabilities.error = nil + router.open(id: 1, seed: nil) + await waitUntil { capabilities.invocations == 2 } + + // Once known, later opens reuse the answer. + router.open(id: 2, seed: nil) + for _ in 0..<10 { await Task.yield() } + #expect(capabilities.invocations == 2) + } + + private func makeRouter(capabilities: FakeCommentsCapabilities) -> CommentsDetailRouter { + CommentsDetailRouter( service: FakeCommentsService(), - capabilities: FakeCommentsCapabilities(), + capabilities: capabilities, coordinator: CommentsModerationCoordinator(service: FakeCommentsService()), titleResolver: PostTitleResolver(fetcher: { _ in .init(titles: [:]) }), tracker: nil, noticePresenter: FakeNoticePresenter(), makeContentRenderer: { FakeContentRenderer() } ) - router.host = host - - router.open(id: 1, seed: nil) - - #expect(navigation.viewControllers.count == 2) } } diff --git a/Modules/Tests/WordPressCommentsTests/Support/CommentDetailTestHelpers.swift b/Modules/Tests/WordPressCommentsTests/Support/CommentDetailTestHelpers.swift index e980048f67ff..7c9cbf93a3dc 100644 --- a/Modules/Tests/WordPressCommentsTests/Support/CommentDetailTestHelpers.swift +++ b/Modules/Tests/WordPressCommentsTests/Support/CommentDetailTestHelpers.swift @@ -26,6 +26,7 @@ func makeVM( seed: CommentListItem? = nil, service: any CommentsServiceProtocol, capabilities: FakeCommentsCapabilities = FakeCommentsCapabilities(), + resolver: CommentsCapabilityResolver? = nil, coordinator: CommentsModerationCoordinator? = nil, tracker: (any CommentsTracker)? = nil, noticePresenter: (any NoticePresenting)? = nil @@ -34,7 +35,7 @@ func makeVM( commentID: commentID, seed: seed, service: service, - capabilities: capabilities, + capabilities: resolver ?? CommentsCapabilityResolver(capabilities: capabilities), coordinator: coordinator ?? CommentsModerationCoordinator(service: FakeCommentsService()), titleResolver: makeResolver(), tracker: tracker, @@ -42,6 +43,17 @@ func makeVM( ) } +/// A resolver whose lookup has already landed with `canModerate`, standing in +/// for the router's prefetch finishing before a detail screen opens. +@MainActor +func makeResolvedCapabilities(canModerate: Bool) async -> CommentsCapabilityResolver { + let capabilities = FakeCommentsCapabilities() + capabilities.canModerate = canModerate + let resolver = CommentsCapabilityResolver(capabilities: capabilities) + _ = await resolver.resolve() + return resolver +} + /// A view model whose authoritative fetch has landed with edit context at /// `status`, so the toolbar is enabled and actions run through `coordinator`. @MainActor diff --git a/Modules/Tests/WordPressCommentsTests/Support/FakeCommentsCapabilities.swift b/Modules/Tests/WordPressCommentsTests/Support/FakeCommentsCapabilities.swift index 14a5ee3a0619..798bb7c582bc 100644 --- a/Modules/Tests/WordPressCommentsTests/Support/FakeCommentsCapabilities.swift +++ b/Modules/Tests/WordPressCommentsTests/Support/FakeCommentsCapabilities.swift @@ -3,8 +3,12 @@ @MainActor final class FakeCommentsCapabilities: CommentsCapabilitiesProtocol { var canModerate = true + var error: Error? + private(set) var invocations = 0 - func canModerateComments() async -> Bool { - canModerate + func canModerateComments() async throws -> Bool { + invocations += 1 + if let error { throw error } + return canModerate } }