From 0eceec357b7ef5251ed0ae9d42b513c17380e0af Mon Sep 17 00:00:00 2001 From: Christian Cleberg Date: Wed, 15 Jul 2026 21:48:58 -0500 Subject: feat: review patchsets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Patchsets are how contributions reach sourcehut, and Hutch had no reference to them anywhere. This adds review and triage: read a series, see its checks and version chain, and set its status. Two schema facts shaped the design. MailingList exposes no patchsets field, so a list's patchsets cannot be queried directly. They are reachable only through thread roots, so the existing threads query now also selects root.patchset — no extra request — and the Patches tab is derived from that. It appears only on lists that actually carry patches. Patch carries no diff. index, count, version, prefix, subject, and trailers are all it has; the diff exists only inside the email body. Patch bodies are split with the same InboxThreadUtilities.segmentMessageBody the inbox uses and rendered through the existing DiffView. Patches are ordered by their [PATCH n/m] index rather than receipt order, since mail arrives out of sequence. Patches with no index are kept at the end rather than dropped, because a one-off patch has no prefix. updatePatchset is nullable, so a null response is treated as a declined change and the local status is left alone rather than advanced optimistically. UNKNOWN and SUPERSEDED are not offered: the first is a sentinel, the second is set by the server when a newer version lands. Patch submission stays out of scope. It is a git send-email flow, not a GraphQL mutation. --- Hutch/Views/Projects/ProjectMailingListView.swift | 124 +++++++++++++++++++++- 1 file changed, 123 insertions(+), 1 deletion(-) (limited to 'Hutch/Views/Projects/ProjectMailingListView.swift') diff --git a/Hutch/Views/Projects/ProjectMailingListView.swift b/Hutch/Views/Projects/ProjectMailingListView.swift index d466121..3933bf3 100644 --- a/Hutch/Views/Projects/ProjectMailingListView.swift +++ b/Hutch/Views/Projects/ProjectMailingListView.swift @@ -24,12 +24,25 @@ private struct ProjectMailingListRootPayload: Decodable, Sendable { let id: Int let messageID: String let patch: InboxPatchPreview? + /// Null unless the thread's root email opens a patchset. `MailingList` has no + /// patchsets field, so this is the only way to enumerate a list's patchsets. + let patchset: PatchsetSummaryPayload? +} + +private struct PatchsetSummaryPayload: Decodable, Sendable { + let id: Int + let subject: String + let version: Int + let prefix: String? + let status: PatchsetStatus } @Observable @MainActor final class MailingListDetailViewModel { private(set) var threads: [InboxThreadSummary] = [] + /// Patchsets on this list, derived from thread roots — see the query below. + private(set) var patchsets: [PatchsetSummary] = [] private(set) var isLoading = false var error: String? var searchText = "" @@ -52,6 +65,13 @@ final class MailingListDetailViewModel { id messageID patch { subject } + patchset { + id + subject + version + prefix + status + } } } } @@ -87,11 +107,46 @@ final class MailingListDetailViewModel { threads = deduplicateThreads( response.list.threads.results.map(makeSummary(from:)) ) + patchsets = Self.patchsets(from: response.list.threads.results) } catch { self.error = "Failed to load mailing list" } } + var filteredPatchsets: [PatchsetSummary] { + let query = searchText.trimmingCharacters(in: .whitespacesAndNewlines).lowercased() + guard !query.isEmpty else { return patchsets } + return patchsets.filter { $0.subject.lowercased().contains(query) } + } + + /// Collects the patchsets opened by these threads, newest first. + /// + /// A revised series arrives as its own thread, so the same subject can appear + /// at several versions; they are kept as distinct patchsets and the version + /// chain is shown in the detail view. + private nonisolated static func patchsets( + from threads: [ProjectMailingListThreadPayload] + ) -> [PatchsetSummary] { + var seenIDs = Set() + var results: [PatchsetSummary] = [] + + for thread in threads { + guard let payload = thread.root.patchset, !seenIDs.contains(payload.id) else { continue } + seenIDs.insert(payload.id) + results.append( + PatchsetSummary( + id: payload.id, + subject: payload.subject, + version: payload.version, + prefix: payload.prefix, + status: payload.status + ) + ) + } + + return results + } + func markThreadRead(_ thread: InboxThreadSummary) { let viewedAt = max(Date(), thread.lastActivityAt) InboxReadStateStore.markViewed(viewedAt, for: thread.threadGroupingKey, defaults: defaults) @@ -257,12 +312,25 @@ final class MailingListDetailViewModel { } } +enum MailingListScope: String, CaseIterable, Hashable { + case threads + case patches + + var displayName: String { + switch self { + case .threads: "Threads" + case .patches: "Patches" + } + } +} + struct MailingListDetailView: View { let mailingList: InboxMailingListReference @Environment(AppState.self) private var appState @State private var viewModel: MailingListDetailViewModel? @State private var pinChangeCount = 0 + @State private var scope: MailingListScope = .threads private var currentUserKey: String? { appState.currentUser?.canonicalName @@ -337,6 +405,27 @@ struct MailingListDetailView: View { @Bindable var vm = viewModel List { + // Only offered when the list actually carries patches, so discussion + // lists do not grow an empty tab. + if !viewModel.patchsets.isEmpty { + Picker("Scope", selection: $scope) { + ForEach(MailingListScope.allCases, id: \.self) { scope in + Text(scope.displayName).tag(scope) + } + } + .pickerStyle(.segmented) + .listRowInsets(EdgeInsets(top: 4, leading: 12, bottom: 4, trailing: 12)) + .themedRow() + } + + if showingPatches(viewModel) { + ForEach(viewModel.filteredPatchsets) { patchset in + NavigationLink(value: MoreRoute.patchset(id: patchset.id, listName: mailingList.name)) { + PatchsetRow(patchset: patchset) + } + .themedRow() + } + } else { ForEach(viewModel.filteredThreads) { thread in NavigationLink { ThreadDetailView( @@ -373,13 +462,14 @@ struct MailingListDetailView: View { } } .themedRow() + } } .themedList() .listStyle(.plain) .searchable( text: $vm.searchText, placement: .navigationBarDrawer(displayMode: .always), - prompt: "Search messages" + prompt: showingPatches(viewModel) ? "Search patches" : "Search messages" ) .overlay { if viewModel.isLoading, viewModel.threads.isEmpty { @@ -390,6 +480,10 @@ struct MailingListDetailView: View { message: error, retryAction: { await viewModel.loadThreads() } ) + } else if showingPatches(viewModel) { + if !viewModel.patchsets.isEmpty, viewModel.filteredPatchsets.isEmpty { + ContentUnavailableView.search(text: viewModel.searchText) + } } else if !viewModel.threads.isEmpty, viewModel.filteredThreads.isEmpty { ContentUnavailableView.search(text: viewModel.searchText) } else if viewModel.threads.isEmpty { @@ -405,6 +499,34 @@ struct MailingListDetailView: View { } .srhtErrorBanner(error: $vm.error) } + + private func showingPatches(_ viewModel: MailingListDetailViewModel) -> Bool { + scope == .patches && !viewModel.patchsets.isEmpty + } +} + +struct PatchsetRow: View { + let patchset: PatchsetSummary + + var body: some View { + VStack(alignment: .leading, spacing: 6) { + Text(patchset.subject) + .font(.subheadline.weight(.medium)) + .lineLimit(2) + + HStack(spacing: 8) { + PatchsetStatusBadge(status: patchset.status) + if let versionLabel = patchset.versionLabel { + Text(versionLabel) + .font(.caption.weight(.medium)) + .foregroundStyle(.secondary) + } + } + } + .padding(.vertical, 2) + .accessibilityElement(children: .combine) + .accessibilityLabel("\(patchset.subject), \(patchset.status.displayName)") + } } struct ProjectMailingListView: View { -- cgit v1.2.3 From 575e62f6dab44b0c9836623fe8b7d17a219f8e0f Mon Sep 17 00:00:00 2001 From: Christian Cleberg Date: Wed, 15 Jul 2026 21:56:56 -0500 Subject: fix: push patchset views directly instead of by route Tapping a patch failed with "no matching navigationDestination declaration visible from the location of the link". MailingListDetailView is presented from four places, but only the More tab and Lookup declare a MoreRoute destination. Reached from a project, via ProjectMailingListView, there is no such destination in the surrounding stack, so a NavigationLink carrying MoreRoute.patchset had nowhere to resolve. The thread rows beside it already use the closure form for exactly this reason. Push PatchsetDetailView directly, from the rows and from the version-chain links inside the detail view, which inherits whatever stack presented it. That leaves MoreRoute.patchset with no users, so it and its two destinations are removed rather than left as a route nothing links to. Neither the compiler nor the tests catch this: it is a runtime SwiftUI resolution failure. --- Hutch/App/RootView.swift | 3 --- Hutch/Views/Lookup/LookupView.swift | 2 -- Hutch/Views/Patchsets/PatchsetDetailView.swift | 12 ++++++++++-- Hutch/Views/Projects/ProjectMailingListView.swift | 6 +++++- 4 files changed, 15 insertions(+), 8 deletions(-) (limited to 'Hutch/Views/Projects/ProjectMailingListView.swift') diff --git a/Hutch/App/RootView.swift b/Hutch/App/RootView.swift index 59d7caa..1ae4651 100644 --- a/Hutch/App/RootView.swift +++ b/Hutch/App/RootView.swift @@ -439,7 +439,6 @@ enum MoreRoute: Hashable { case projectDashboard(id: String, title: String?) case mailingList(InboxMailingListReference) case thread(InboxThreadSummary) - case patchset(id: Int, listName: String?) case manPageBrowser case manPage(URL) } @@ -473,8 +472,6 @@ private struct MoreNavigationRoot: View { ProjectDashboardDeepLinkView(projectID: id, title: title) case .mailingList(let mailingList): MailingListDetailView(mailingList: mailingList) - case .patchset(let id, let listName): - PatchsetDetailView(patchsetID: id, listName: listName) case .thread(let thread): ThreadDetailView( thread: thread, diff --git a/Hutch/Views/Lookup/LookupView.swift b/Hutch/Views/Lookup/LookupView.swift index b52bb3f..2a26282 100644 --- a/Hutch/Views/Lookup/LookupView.swift +++ b/Hutch/Views/Lookup/LookupView.swift @@ -465,8 +465,6 @@ struct LookupView: View { ProjectDashboardDeepLinkView(projectID: id, title: title) case .mailingList(let mailingList): MailingListDetailView(mailingList: mailingList) - case .patchset(let id, let listName): - PatchsetDetailView(patchsetID: id, listName: listName) case .thread(let thread): ThreadDetailView( thread: thread, diff --git a/Hutch/Views/Patchsets/PatchsetDetailView.swift b/Hutch/Views/Patchsets/PatchsetDetailView.swift index d76aab4..7bc5630 100644 --- a/Hutch/Views/Patchsets/PatchsetDetailView.swift +++ b/Hutch/Views/Patchsets/PatchsetDetailView.swift @@ -122,8 +122,14 @@ struct PatchsetDetailView: View { // The version chain matters during review: a superseded series should // usually be read at its newest version instead. + // + // Pushed directly rather than by value, for the same reason as the rows + // that lead here — this view inherits whatever stack presented it, and + // not all of them declare a MoreRoute destination. if let supersededBy = patchset.supersededBy { - NavigationLink(value: MoreRoute.patchset(id: supersededBy, listName: listName)) { + NavigationLink { + PatchsetDetailView(patchsetID: supersededBy, listName: listName) + } label: { SwiftUI.Label("Superseded by a newer version", systemImage: "arrow.right.circle") .font(.subheadline) } @@ -131,7 +137,9 @@ struct PatchsetDetailView: View { } if let supersedes = patchset.supersedes { - NavigationLink(value: MoreRoute.patchset(id: supersedes, listName: listName)) { + NavigationLink { + PatchsetDetailView(patchsetID: supersedes, listName: listName) + } label: { SwiftUI.Label("Revises an earlier version", systemImage: "arrow.left.circle") .font(.subheadline) } diff --git a/Hutch/Views/Projects/ProjectMailingListView.swift b/Hutch/Views/Projects/ProjectMailingListView.swift index 3933bf3..696cf6b 100644 --- a/Hutch/Views/Projects/ProjectMailingListView.swift +++ b/Hutch/Views/Projects/ProjectMailingListView.swift @@ -420,7 +420,11 @@ struct MailingListDetailView: View { if showingPatches(viewModel) { ForEach(viewModel.filteredPatchsets) { patchset in - NavigationLink(value: MoreRoute.patchset(id: patchset.id, listName: mailingList.name)) { + // Pushed directly rather than by value: this view is also shown + // from a project, whose stack declares no MoreRoute destination. + NavigationLink { + PatchsetDetailView(patchsetID: patchset.id, listName: mailingList.name) + } label: { PatchsetRow(patchset: patchset) } .themedRow() -- cgit v1.2.3