diff --git a/apps/swift-ios/Features/Shared/FeatureToolModels.swift b/apps/swift-ios/Features/Shared/FeatureToolModels.swift index fc03f6f00b4..102ba572f5d 100644 --- a/apps/swift-ios/Features/Shared/FeatureToolModels.swift +++ b/apps/swift-ios/Features/Shared/FeatureToolModels.swift @@ -825,6 +825,19 @@ public struct FeaturePullRequest: Sendable, Equatable, Hashable, Codable { self.state = state self.url = url } + + /// Only hand credential-free web destinations to the system browser. + public var safeExternalURL: URL? { + guard let url, + let scheme = url.scheme?.lowercased(), + scheme == "http" || scheme == "https", + url.host?.isEmpty == false, + url.user == nil, + url.password == nil else { + return nil + } + return url + } } public enum FeatureSourceControlAction: String, CaseIterable, Sendable, Codable { diff --git a/apps/swift-ios/Features/Workspace/HomeThreadCollectionView.swift b/apps/swift-ios/Features/Workspace/HomeThreadCollectionView.swift index 90b048fa804..003ac3e56f1 100644 --- a/apps/swift-ios/Features/Workspace/HomeThreadCollectionView.swift +++ b/apps/swift-ios/Features/Workspace/HomeThreadCollectionView.swift @@ -59,6 +59,7 @@ struct HomeThreadCollectionView: UIViewRepresentable { static func dismantleUIView(_ collectionView: UICollectionView, coordinator: Coordinator) { coordinator.invalidateTimer() + coordinator.invalidatePullRequestLookups() collectionView.delegate = nil } @@ -77,6 +78,12 @@ struct HomeThreadCollectionView: UIViewRepresentable { private var timer: Timer? private var timerTick = 0 private var timerInterval: TimeInterval = 0 + private var pullRequestResolutions: [ + HomeThreadPullRequestLookupKey: HomeThreadPullRequestResolution + ] = [:] + private var pullRequestTasks: [ + HomeThreadPullRequestLookupKey: (token: UUID, task: Task) + ] = [:] init(parent: HomeThreadCollectionView) { self.parent = parent @@ -117,6 +124,7 @@ struct HomeThreadCollectionView: UIViewRepresentable { seenIdentifiers.insert(item.id).inserted } itemsByID = Dictionary(uniqueKeysWithValues: items.map { ($0.id, $0) }) + prunePullRequestLookups(to: Set(items.compactMap(\.pullRequestLookupKey))) // After items land: picks 1 Hz when a working thread is present, // 60s otherwise, and is a no-op when the interval is unchanged. startTimer() @@ -151,6 +159,12 @@ struct HomeThreadCollectionView: UIViewRepresentable { timer = nil } + func invalidatePullRequestLookups() { + pullRequestTasks.values.forEach { $0.task.cancel() } + pullRequestTasks.removeAll() + pullRequestResolutions.removeAll() + } + func collectionView(_ collectionView: UICollectionView, didSelectItemAt indexPath: IndexPath) { guard let item = item(at: indexPath) else { return } switch item { @@ -250,12 +264,21 @@ struct HomeThreadCollectionView: UIViewRepresentable { now: Date ) { guard let item = itemsByID[identifier] else { return } + let pullRequest: HomeThreadPullRequestPresentation? + if case let .thread(thread, _, _, _, _) = item { + loadPullRequestIfNeeded(for: thread) + pullRequest = HomeThreadPullRequestLookupKey(thread: thread) + .flatMap { pullRequestResolutions[$0]?.presentation } + } else { + pullRequest = nil + } cell.contentConfiguration = UIHostingConfiguration { HomeCollectionCellContent( item: item, projectFaviconClient: parent.projectFaviconClient, isSelected: identifier.threadID == selectedThreadID, - now: now + now: now, + pullRequest: pullRequest ) } .margins(.all, 0) @@ -268,6 +291,7 @@ struct HomeThreadCollectionView: UIViewRepresentable { } private func configureAccessibility(_ cell: HomeCollectionCell, item: HomeCollectionItem) { + cell.accessibilityCustomActions = nil switch item { case let .thread(thread, context, _, _, _): cell.isAccessibilityElement = true @@ -275,8 +299,19 @@ struct HomeThreadCollectionView: UIViewRepresentable { ? [.button, .selected] : .button cell.accessibilityLabel = thread.title - cell.accessibilityValue = threadAccessibilityValue(thread, context: context) + let baseValue = threadAccessibilityValue(thread, context: context) + cell.accessibilityValue = baseValue cell.accessibilityHint = "Opens task" + if let key = HomeThreadPullRequestLookupKey(thread: thread), + let pullRequest = pullRequestResolutions[key]?.presentation { + cell.accessibilityValue = pullRequest.accessibilityValue(appending: baseValue) + cell.accessibilityCustomActions = [ + UIAccessibilityCustomAction(name: pullRequest.accessibilityActionName) { _ in + UIApplication.shared.open(pullRequest.destination) + return true + }, + ] + } cell.onAccessibilityActivate = { [weak self] in guard let self else { return } let previousSelection = self.selectedThreadID @@ -325,6 +360,51 @@ struct HomeThreadCollectionView: UIViewRepresentable { } } + private func loadPullRequestIfNeeded(for thread: FeatureThread) { + guard let key = HomeThreadPullRequestLookupKey(thread: thread), + pullRequestResolutions[key] == nil, + pullRequestTasks[key] == nil else { return } + let token = UUID() + let client = parent.projectFaviconClient + let task = Task { [weak self] in + let presentation: HomeThreadPullRequestPresentation? + do { + let status = try await client.sourceControlStatus(threadID: thread.id) + presentation = HomeThreadPullRequestPresentation(thread: thread, status: status) + } catch { + guard let self, pullRequestTasks[key]?.token == token else { return } + pullRequestTasks[key] = nil + return + } + guard !Task.isCancelled, let self, + pullRequestTasks[key]?.token == token else { return } + pullRequestTasks[key] = nil + pullRequestResolutions[key] = .resolved(presentation) + reconfigureThreadRows(matching: key) + } + pullRequestTasks[key] = (token, task) + } + + private func prunePullRequestLookups(to activeKeys: Set) { + pullRequestResolutions = pullRequestResolutions.filter { activeKeys.contains($0.key) } + let inactiveTasks = pullRequestTasks.filter { !activeKeys.contains($0.key) } + for (key, lookup) in inactiveTasks { + lookup.task.cancel() + pullRequestTasks[key] = nil + } + } + + private func reconfigureThreadRows(matching key: HomeThreadPullRequestLookupKey) { + guard let dataSource else { return } + let identifiers = itemsByID.compactMap { identifier, item in + item.pullRequestLookupKey == key ? identifier : nil + } + guard !identifiers.isEmpty else { return } + var snapshot = dataSource.snapshot() + snapshot.reconfigureItems(identifiers) + dataSource.apply(snapshot, animatingDifferences: false) + } + private func threadAccessibilityValue( _ thread: FeatureThread, context: HomeThreadRowContext @@ -645,6 +725,7 @@ private struct HomeCollectionCellContent: View { let projectFaviconClient: any FeatureClient let isSelected: Bool let now: Date + let pullRequest: HomeThreadPullRequestPresentation? @ViewBuilder var body: some View { @@ -657,7 +738,8 @@ private struct HomeCollectionCellContent: View { isSelected: isSelected, style: style, now: now, - allowsMultilineTitle: allowsMultilineTitle + allowsMultilineTitle: allowsMultilineTitle, + pullRequest: pullRequest ) case let .shelfHeader(shelf, count, isExpanded): HomeShelfHeader( @@ -697,6 +779,40 @@ private struct HomeCollectionCellContent: View { } } +struct HomeThreadPullRequestLookupKey: Hashable { + let environmentID: String? + let projectID: String + let branch: String + let worktreePath: String? + + init?(thread: FeatureThread) { + guard let branch = thread.branch?.trimmingCharacters(in: .whitespacesAndNewlines), + !branch.isEmpty else { return nil } + environmentID = thread.environmentID + projectID = thread.projectID + self.branch = branch + let path = thread.worktreePath?.trimmingCharacters(in: .whitespacesAndNewlines) + worktreePath = path?.isEmpty == false ? path : nil + } +} + +private enum HomeThreadPullRequestResolution { + case resolved(HomeThreadPullRequestPresentation?) + + var presentation: HomeThreadPullRequestPresentation? { + switch self { + case let .resolved(presentation): presentation + } + } +} + +private extension HomeCollectionItem { + var pullRequestLookupKey: HomeThreadPullRequestLookupKey? { + guard case let .thread(thread, _, _, _, _) = self else { return nil } + return HomeThreadPullRequestLookupKey(thread: thread) + } +} + private extension Optional where Wrapped == [IndexPath] { var orEmpty: [IndexPath] { self ?? [] } } diff --git a/apps/swift-ios/Features/Workspace/WorkspaceView.swift b/apps/swift-ios/Features/Workspace/WorkspaceView.swift index 910c0c6c64c..1b30f401202 100644 --- a/apps/swift-ios/Features/Workspace/WorkspaceView.swift +++ b/apps/swift-ios/Features/Workspace/WorkspaceView.swift @@ -857,6 +857,45 @@ struct HomeThreadRowContext: Equatable { } } +struct HomeThreadPullRequestPresentation: Equatable { + let number: Int + let state: String + let destination: URL + + var shortLabel: String { "#\(number)" } + var accessibilityLabel: String { "Pull request \(number), \(state)" } + var accessibilityActionName: String { "Open pull request \(number), \(state)" } + func accessibilityValue(appending baseValue: String) -> String { + "\(baseValue). \(accessibilityLabel)." + } + + init?( + thread: FeatureThread, + status: FeatureSourceControlStatus + ) { + guard let threadBranch = Self.normalizedBranch(thread.branch), + let statusBranch = Self.normalizedBranch(status.branch), + threadBranch == statusBranch, + let pullRequest = status.pullRequest, + pullRequest.number > 0, + let destination = pullRequest.safeExternalURL else { + return nil + } + let normalizedState = pullRequest.state.trimmingCharacters(in: .whitespacesAndNewlines) + number = pullRequest.number + state = normalizedState.isEmpty ? "unknown state" : normalizedState + self.destination = destination + } + + private static func normalizedBranch(_ branch: String?) -> String? { + guard let branch = branch?.trimmingCharacters(in: .whitespacesAndNewlines), + !branch.isEmpty else { + return nil + } + return branch + } +} + struct FeatureThreadRow: View { enum Style: Equatable { case rich @@ -870,6 +909,7 @@ struct FeatureThreadRow: View { let style: Style let now: Date let allowsMultilineTitle: Bool + let pullRequest: HomeThreadPullRequestPresentation? init( thread: FeatureThread, @@ -878,7 +918,8 @@ struct FeatureThreadRow: View { isSelected: Bool = false, style: Style = .rich, now: Date = .now, - allowsMultilineTitle: Bool = false + allowsMultilineTitle: Bool = false, + pullRequest: HomeThreadPullRequestPresentation? = nil ) { self.thread = thread self.context = context @@ -887,6 +928,7 @@ struct FeatureThreadRow: View { self.style = style self.now = now self.allowsMultilineTitle = allowsMultilineTitle + self.pullRequest = pullRequest } var body: some View { @@ -941,6 +983,7 @@ struct FeatureThreadRow: View { .foregroundStyle(T3Colors.syntaxProperty) } Spacer(minLength: 8) + pullRequestLink if let environmentLabel { HStack(spacing: 4) { Image(systemName: environmentIcon) @@ -982,6 +1025,7 @@ struct FeatureThreadRow: View { .foregroundStyle(T3Colors.textSecondary) .lineLimit(allowsMultilineTitle ? 2 : 1) Spacer(minLength: 8) + pullRequestLink if thread.pinnedAt != nil { Image(systemName: "pin.fill") .font(.system(size: 9, weight: .semibold)) @@ -1001,6 +1045,35 @@ struct FeatureThreadRow: View { ) } + @ViewBuilder + private var pullRequestLink: some View { + if let pullRequest { + Link(destination: pullRequest.destination) { + HStack(spacing: 3) { + Text(pullRequest.shortLabel) + .monospacedDigit() + .lineLimit(1) + .fixedSize(horizontal: true, vertical: false) + Image(systemName: "arrow.up.right") + .font(.system(size: 8, weight: .bold)) + } + .font(T3Typography.homeMetadata.weight(.semibold)) + .foregroundStyle(T3Colors.accent) + .contentShape(Rectangle()) + } + .padding(.horizontal, 7) + .padding(.vertical, 15) + .contentShape(Rectangle()) + .padding(.horizontal, -7) + .padding(.vertical, -15) + .buttonStyle(.plain) + .layoutPriority(1) + .accessibilityLabel(pullRequest.accessibilityLabel) + .accessibilityHint("Opens pull request in the browser") + .accessibilityIdentifier("thread-\(thread.id)-pull-request") + } + } + @ViewBuilder private func status(at now: Date) -> some View { let label = thread.homeStatusLabel diff --git a/apps/swift-ios/Tests/FeatureTests/HomeThreadMetadataTests.swift b/apps/swift-ios/Tests/FeatureTests/HomeThreadMetadataTests.swift index c63745ee727..621c1164064 100644 --- a/apps/swift-ios/Tests/FeatureTests/HomeThreadMetadataTests.swift +++ b/apps/swift-ios/Tests/FeatureTests/HomeThreadMetadataTests.swift @@ -195,4 +195,190 @@ struct HomeThreadMetadataTests { ) == nil ) } + + @Test + func pullRequestPresentationCarriesSafeDestinationNumberStateAndAccessibility() throws { + let thread = pullRequestThread(branch: " feature/pr-links ") + let destination = try #require(URL(string: "https://github.com/pingdotgg/t3code/pull/5804")) + let presentation = try #require(HomeThreadPullRequestPresentation( + thread: thread, + status: FeatureSourceControlStatus( + branch: "feature/pr-links", + pullRequest: FeaturePullRequest( + number: 5804, + title: "Link thread rows", + state: " open ", + url: destination + ) + ) + )) + + #expect(presentation.number == 5804) + #expect(presentation.state == "open") + #expect(presentation.destination == destination) + #expect(presentation.shortLabel == "#5804") + #expect(presentation.accessibilityLabel == "Pull request 5804, open") + #expect(presentation.accessibilityActionName == "Open pull request 5804, open") + #expect( + presentation.accessibilityValue(appending: "Ready. Project t3code") + == "Ready. Project t3code. Pull request 5804, open." + ) + } + + @Test + func pullRequestPresentationRejectsMissingMismatchedOrMalformedRemoteData() { + let thread = pullRequestThread(branch: "feature/pr-links") + let validPullRequest = FeaturePullRequest( + number: 5804, + title: "Link thread rows", + state: "open", + url: URL(string: "https://github.com/pingdotgg/t3code/pull/5804") + ) + + #expect(HomeThreadPullRequestPresentation( + thread: thread, + status: FeatureSourceControlStatus(branch: "feature/other", pullRequest: validPullRequest) + ) == nil) + #expect(HomeThreadPullRequestPresentation( + thread: thread, + status: FeatureSourceControlStatus(branch: nil, pullRequest: validPullRequest) + ) == nil) + #expect(HomeThreadPullRequestPresentation( + thread: thread, + status: FeatureSourceControlStatus(branch: "feature/pr-links", pullRequest: nil) + ) == nil) + + for unsafeURL in [ + nil, + URL(string: "t3code-swiftui://pull/5804"), + URL(string: "https://token@example.com/pull/5804"), + URL(string: "https:///pull/5804"), + URL(string: "not a remote URL"), + ] { + var pullRequest = validPullRequest + pullRequest.url = unsafeURL + #expect(HomeThreadPullRequestPresentation( + thread: thread, + status: FeatureSourceControlStatus( + branch: "feature/pr-links", + pullRequest: pullRequest + ) + ) == nil) + } + + var invalidNumber = validPullRequest + invalidNumber.number = 0 + #expect(HomeThreadPullRequestPresentation( + thread: thread, + status: FeatureSourceControlStatus( + branch: "feature/pr-links", + pullRequest: invalidNumber + ) + ) == nil) + + var missingBranch = thread + missingBranch.branch = " " + #expect(HomeThreadPullRequestPresentation( + thread: missingBranch, + status: FeatureSourceControlStatus( + branch: "feature/pr-links", + pullRequest: validPullRequest + ) + ) == nil) + } + + @Test + func pullRequestLookupKeysAndDestinationsDoNotFollowRecycledThreadIDs() throws { + let first = pullRequestThread( + id: "first-row", + environmentID: "mac", + branch: " feature/pr-links ", + worktreePath: " /worktrees/pr-links " + ) + let recycled = pullRequestThread( + id: "recycled-row", + environmentID: "mac", + branch: "feature/pr-links", + worktreePath: "/worktrees/pr-links" + ) + let otherCheckout = pullRequestThread( + id: "other-row", + environmentID: "mac", + branch: "feature/other", + worktreePath: "/worktrees/other" + ) + let firstKey = try #require(HomeThreadPullRequestLookupKey(thread: first)) + let recycledKey = try #require(HomeThreadPullRequestLookupKey(thread: recycled)) + let otherKey = try #require(HomeThreadPullRequestLookupKey(thread: otherCheckout)) + + #expect(firstKey == recycledKey) + #expect(firstKey != otherKey) + + let firstDestination = URL(string: "https://github.com/pingdotgg/t3code/pull/5804") + let otherDestination = URL(string: "https://github.com/pingdotgg/t3code/pull/5999") + let firstPresentation = try #require(HomeThreadPullRequestPresentation( + thread: first, + status: FeatureSourceControlStatus( + branch: "feature/pr-links", + pullRequest: FeaturePullRequest( + number: 5804, + title: "First", + state: "open", + url: firstDestination + ) + ) + )) + let otherPresentation = try #require(HomeThreadPullRequestPresentation( + thread: otherCheckout, + status: FeatureSourceControlStatus( + branch: "feature/other", + pullRequest: FeaturePullRequest( + number: 5999, + title: "Other", + state: "merged", + url: otherDestination + ) + ) + )) + + #expect(firstPresentation.destination == firstDestination) + #expect(otherPresentation.destination == otherDestination) + #expect(firstPresentation.accessibilityActionName != otherPresentation.accessibilityActionName) + #expect(HomeThreadPullRequestLookupKey(thread: pullRequestThread(branch: nil)) == nil) + } + + @Test + func blankPullRequestStateHasATruthfulAccessibilityFallback() throws { + let presentation = try #require(HomeThreadPullRequestPresentation( + thread: pullRequestThread(branch: "feature/pr-links"), + status: FeatureSourceControlStatus( + branch: "feature/pr-links", + pullRequest: FeaturePullRequest( + number: 5804, + title: "Link thread rows", + state: " ", + url: URL(string: "http://127.0.0.1/pull/5804") + ) + ) + )) + + #expect(presentation.state == "unknown state") + #expect(presentation.accessibilityLabel == "Pull request 5804, unknown state") + } + + private func pullRequestThread( + id: String = "thread", + environmentID: String? = "environment", + branch: String?, + worktreePath: String? = "/worktrees/pr-links" + ) -> FeatureThread { + FeatureThread( + id: id, + projectID: "project", + environmentID: environmentID, + title: "Build", + branch: branch, + worktreePath: worktreePath + ) + } }