Commit f7f6759a51
Unsigned
Layout: unified · split
gitbay/MRs/MRDetailViewModel.swift +26 −2
| @@ -98,8 +98,32 @@ final class MRDetailViewModel { | |||
| 98 | await perform(argv) | 98 | await perform(argv) |
| 99 | } | 99 | } |
| 100 | 100 | ||
| 101 | func close() async { | 101 | /// `--by` records which merge request replaces this one; the server |
| 102 | await perform(["mr", "close"] + ref) | 102 | /// refuses a closed, merged or self reference. |
| 103 | func close(inFavourOf successor: Int64? = nil) async { | ||
| 104 | var argv = ["mr", "close"] + ref | ||
| 105 | if let successor { argv.append(contentsOf: ["--by", String(successor)]) } | ||
| 106 | await perform(argv) | ||
| 107 | } | ||
| 108 | |||
| 109 | /// The repository's other open merge requests, for the close-in- | ||
| 110 | /// favour picker. Fetched on demand and cached like `loadBranches()`; | ||
| 111 | /// this merge request is dropped since it cannot supersede itself. | ||
| 112 | private(set) var openMergeRequests: [MergeRequest]? | ||
| 113 | private(set) var openMergeRequestsError: String? | ||
| 114 | |||
| 115 | func loadOpenMergeRequests() async { | ||
| 116 | guard openMergeRequests == nil else { return } | ||
| 117 | openMergeRequestsError = nil | ||
| 118 | do { | ||
| 119 | let rows = try await client.readList( | ||
| 120 | ["mr", "list", repoPath, "--state", "open"], of: MergeRequest.self) | ||
| 121 | openMergeRequests = rows.filter { $0.number != number } | ||
| 122 | } catch let error as GitbayError { | ||
| 123 | openMergeRequestsError = error.userFacingMessage | ||
| 124 | } catch { | ||
| 125 | openMergeRequestsError = GitbayError.transport(error).userFacingMessage | ||
| 126 | } | ||
| 103 | } | 127 | } |
| 104 | 128 | ||
| 105 | func setResolved(_ thread: ReviewThread, _ resolved: Bool) async { | 129 | func setResolved(_ thread: ReviewThread, _ resolved: Bool) async { |
gitbay/MRs/MRModels.swift +5
| @@ -90,6 +90,9 @@ nonisolated struct MRDetail: Decodable, Sendable, Hashable { | |||
| 90 | let mergedBy: String? | 90 | let mergedBy: String? |
| 91 | let closedAt: Date? | 91 | let closedAt: Date? |
| 92 | let closedBy: String? | 92 | let closedBy: String? |
| 93 | /// The merge request this one was closed in favour of. Omitted by | ||
| 94 | /// the server when there is none. | ||
| 95 | let supersededBy: Int64? | ||
| 93 | let checks: [Check]? | 96 | let checks: [Check]? |
| 94 | let checksCombined: String? | 97 | let checksCombined: String? |
| 95 | let unresolvedThreads: Int? | 98 | let unresolvedThreads: Int? |
| @@ -115,6 +118,7 @@ nonisolated struct MRDetail: Decodable, Sendable, Hashable { | |||
| 115 | case mergedBy = "merged_by" | 118 | case mergedBy = "merged_by" |
| 116 | case closedAt = "closed_at" | 119 | case closedAt = "closed_at" |
| 117 | case closedBy = "closed_by" | 120 | case closedBy = "closed_by" |
| 121 | case supersededBy = "superseded_by" | ||
| 118 | case checksCombined = "checks_combined" | 122 | case checksCombined = "checks_combined" |
| 119 | case unresolvedThreads = "unresolved_threads" | 123 | case unresolvedThreads = "unresolved_threads" |
| 120 | case reviewRequests = "review_requests" | 124 | case reviewRequests = "review_requests" |
| @@ -138,6 +142,7 @@ nonisolated struct MRDetail: Decodable, Sendable, Hashable { | |||
| 138 | mergedBy = try container.decodeIfPresent(String.self, forKey: .mergedBy) | 142 | mergedBy = try container.decodeIfPresent(String.self, forKey: .mergedBy) |
| 139 | closedAt = try container.decodeIfPresent(Date.self, forKey: .closedAt) | 143 | closedAt = try container.decodeIfPresent(Date.self, forKey: .closedAt) |
| 140 | closedBy = try container.decodeIfPresent(String.self, forKey: .closedBy) | 144 | closedBy = try container.decodeIfPresent(String.self, forKey: .closedBy) |
| 145 | supersededBy = try container.decodeIfPresent(Int64.self, forKey: .supersededBy) | ||
| 141 | checks = try container.decodeIfPresent([Check].self, forKey: .checks) | 146 | checks = try container.decodeIfPresent([Check].self, forKey: .checks) |
| 142 | checksCombined = try container.decodeIfPresent(String.self, forKey: .checksCombined) | 147 | checksCombined = try container.decodeIfPresent(String.self, forKey: .checksCombined) |
| 143 | unresolvedThreads = try container.decodeIfPresent(Int.self, forKey: .unresolvedThreads) | 148 | unresolvedThreads = try container.decodeIfPresent(Int.self, forKey: .unresolvedThreads) |
gitbay/Views/MRs/MRView.swift +94 −1
| @@ -14,6 +14,8 @@ struct MRView: View { | |||
| 14 | @State private var editingReviewer = "" | 14 | @State private var editingReviewer = "" |
| 15 | @State private var retargeting = false | 15 | @State private var retargeting = false |
| 16 | @State private var pendingRetarget: String? | 16 | @State private var pendingRetarget: String? |
| 17 | @State private var superseding = false | ||
| 18 | @State private var pendingSuccessor: Int64? | ||
| 17 | 19 | ||
| 18 | init(client: GitbayClient, repo: String, number: Int64) { | 20 | init(client: GitbayClient, repo: String, number: Int64) { |
| 19 | _model = State(initialValue: MRDetailViewModel( | 21 | _model = State(initialValue: MRDetailViewModel( |
| @@ -41,6 +43,7 @@ struct MRView: View { | |||
| 41 | 43 | ||
| 42 | milestoneSection(mr) | 44 | milestoneSection(mr) |
| 43 | stackSection(mr) | 45 | stackSection(mr) |
| 46 | supersededSection(mr) | ||
| 44 | reviewersSection(mr) | 47 | reviewersSection(mr) |
| 45 | diffSection | 48 | diffSection |
| 46 | if !model.revisions.isEmpty { | 49 | if !model.revisions.isEmpty { |
| @@ -91,8 +94,32 @@ struct MRView: View { | |||
| 91 | } | 94 | } |
| 92 | .confirmationDialog("Close !\(model.number) without merging?", isPresented: $confirmingClose) { | 95 | .confirmationDialog("Close !\(model.number) without merging?", isPresented: $confirmingClose) { |
| 93 | Button("Close", role: .destructive) { Task { await model.close() } } | 96 | Button("Close", role: .destructive) { Task { await model.close() } } |
| 97 | Button("Close in favour of another…") { superseding = true } | ||
| 94 | Button("Cancel", role: .cancel) {} | 98 | Button("Cancel", role: .cancel) {} |
| 95 | } | 99 | } |
| 100 | .confirmationDialog( | ||
| 101 | "Close !\(model.number) in favour of !\(pendingSuccessor ?? 0)?", | ||
| 102 | isPresented: .init( | ||
| 103 | get: { pendingSuccessor != nil }, | ||
| 104 | set: { if !$0 { pendingSuccessor = nil } } | ||
| 105 | ), | ||
| 106 | titleVisibility: .visible | ||
| 107 | ) { | ||
| 108 | Button("Close", role: .destructive) { | ||
| 109 | guard let successor = pendingSuccessor else { return } | ||
| 110 | pendingSuccessor = nil | ||
| 111 | Task { await model.close(inFavourOf: successor) } | ||
| 112 | } | ||
| 113 | Button("Cancel", role: .cancel) { pendingSuccessor = nil } | ||
| 114 | } message: { | ||
| 115 | Text("The page will name the merge request that replaces this one, and that one will list this as superseded.") | ||
| 116 | } | ||
| 117 | .sheet(isPresented: $superseding) { | ||
| 118 | SupersedeSheet(model: model) { successor in | ||
| 119 | superseding = false | ||
| 120 | pendingSuccessor = successor | ||
| 121 | } | ||
| 122 | } | ||
| 96 | .confirmationDialog( | 123 | .confirmationDialog( |
| 97 | "Retarget to \(pendingRetarget ?? "")?", | 124 | "Retarget to \(pendingRetarget ?? "")?", |
| 98 | isPresented: .init( | 125 | isPresented: .init( |
| @@ -175,7 +202,8 @@ struct MRView: View { | |||
| 175 | if let at = mr.mergedAt { | 202 | if let at = mr.mergedAt { |
| 176 | stampLine("merged", by: mr.mergedBy, at: at) | 203 | stampLine("merged", by: mr.mergedBy, at: at) |
| 177 | } else if let at = mr.closedAt { | 204 | } else if let at = mr.closedAt { |
| 178 | stampLine("closed without merging", by: mr.closedBy, at: at) | 205 | stampLine(mr.supersededBy.map { "closed in favour of !\($0)" } ?? "closed without merging", |
| 206 | by: mr.closedBy, at: at) | ||
| 179 | } | 207 | } |
| 180 | } | 208 | } |
| 181 | 209 | ||
| @@ -222,6 +250,20 @@ struct MRView: View { | |||
| 222 | /// Shown only when this merge request is part of a stack. Each row | 250 | /// Shown only when this merge request is part of a stack. Each row |
| 223 | /// spells out its own direction — "stacked on" vs. "is stacked on | 251 | /// spells out its own direction — "stacked on" vs. "is stacked on |
| 224 | /// this" mean opposite things and look alike if abbreviated. | 252 | /// this" mean opposite things and look alike if abbreviated. |
| 253 | /// The merge request that replaced this one, as a link; the stamp | ||
| 254 | /// line names it, this row reaches it. | ||
| 255 | @ViewBuilder | ||
| 256 | private func supersededSection(_ mr: MRDetail) -> some View { | ||
| 257 | if let successor = mr.supersededBy { | ||
| 258 | Section { | ||
| 259 | NavigationLink(value: MRRoute.mr(repo: model.repoPath, number: successor)) { | ||
| 260 | Label("Superseded by !\(successor)", systemImage: "arrow.right.to.line") | ||
| 261 | .font(.gbSans(.subheadline)) | ||
| 262 | } | ||
| 263 | } | ||
| 264 | } | ||
| 265 | } | ||
| 266 | |||
| 225 | @ViewBuilder | 267 | @ViewBuilder |
| 226 | private func stackSection(_ mr: MRDetail) -> some View { | 268 | private func stackSection(_ mr: MRDetail) -> some View { |
| 227 | if mr.stackedOn != nil || !mr.stacked.isEmpty { | 269 | if mr.stackedOn != nil || !mr.stacked.isEmpty { |
| @@ -664,6 +706,57 @@ struct ReviewThreadView: View { | |||
| 664 | /// `nil` while loading and `model.branchesError` distinguishes a failed | 706 | /// `nil` while loading and `model.branchesError` distinguishes a failed |
| 665 | /// `repo refs` fetch from a repository that genuinely has no other | 707 | /// `repo refs` fetch from a repository that genuinely has no other |
| 666 | /// branches — an empty list must not read as a failed load. | 708 | /// branches — an empty list must not read as a failed load. |
| 709 | /// The repository's other open merge requests, one of which will be | ||
| 710 | /// recorded as replacing this one. | ||
| 711 | private struct SupersedeSheet: View { | ||
| 712 | |||
| 713 | let model: MRDetailViewModel | ||
| 714 | let onPick: (Int64) -> Void | ||
| 715 | |||
| 716 | @Environment(\.dismiss) private var dismiss | ||
| 717 | |||
| 718 | var body: some View { | ||
| 719 | NavigationStack { | ||
| 720 | Group { | ||
| 721 | if let error = model.openMergeRequestsError { | ||
| 722 | ContentUnavailableView { | ||
| 723 | Label("Could not load merge requests", systemImage: "wifi.exclamationmark") | ||
| 724 | } description: { | ||
| 725 | Text(error) | ||
| 726 | } | ||
| 727 | } else if let candidates = model.openMergeRequests { | ||
| 728 | if candidates.isEmpty { | ||
| 729 | ContentUnavailableView { | ||
| 730 | Label("No other open merge requests", systemImage: "arrow.triangle.pull") | ||
| 731 | } description: { | ||
| 732 | Text("Only an open merge request in this repository can supersede this one.") | ||
| 733 | } | ||
| 734 | } else { | ||
| 735 | List(candidates) { mr in | ||
| 736 | Button { | ||
| 737 | onPick(mr.number) | ||
| 738 | } label: { | ||
| 739 | MRRow(mr: mr) | ||
| 740 | } | ||
| 741 | .tint(.primary) | ||
| 742 | } | ||
| 743 | } | ||
| 744 | } else { | ||
| 745 | ProgressView() | ||
| 746 | } | ||
| 747 | } | ||
| 748 | .navigationTitle("Close !\(model.number) in favour of") | ||
| 749 | .navigationBarTitleDisplayMode(.inline) | ||
| 750 | .toolbar { | ||
| 751 | ToolbarItem(placement: .cancellationAction) { | ||
| 752 | Button("Cancel") { dismiss() } | ||
| 753 | } | ||
| 754 | } | ||
| 755 | .task { await model.loadOpenMergeRequests() } | ||
| 756 | } | ||
| 757 | } | ||
| 758 | } | ||
| 759 | |||
| 667 | private struct RetargetSheet: View { | 760 | private struct RetargetSheet: View { |
| 668 | 761 | ||
| 669 | let model: MRDetailViewModel | 762 | let model: MRDetailViewModel |
gitbayTests/MRLifecycleTests.swift +49
| @@ -85,6 +85,19 @@ struct MRLifecycleDecodingTests { | |||
| 85 | #expect(row.draft == false) | 85 | #expect(row.draft == false) |
| 86 | #expect(row.stackedOn == nil) | 86 | #expect(row.stackedOn == nil) |
| 87 | } | 87 | } |
| 88 | |||
| 89 | /// `superseded_by` is omitempty and 0 means none, so the ordinary row | ||
| 90 | /// carries no key at all. | ||
| 91 | @Test func supersededByDecodesWhenPresent() throws { | ||
| 92 | #expect(try decodeDetail(plainDetail).supersededBy == nil) | ||
| 93 | let mr = try decodeDetail(""" | ||
| 94 | {"number":7,"title":"a change","state":"closed","author":"cmc",\ | ||
| 95 | "closed_at":"2026-09-02T00:00:00Z","closed_by":"cmc","superseded_by":9,\ | ||
| 96 | "source":"feat","target_ref":"main","head_sha":"abc123",\ | ||
| 97 | "created_at":"2026-09-01T00:00:00Z"} | ||
| 98 | """) | ||
| 99 | #expect(mr.supersededBy == 9) | ||
| 100 | } | ||
| 88 | } | 101 | } |
| 89 | 102 | ||
| 90 | @MainActor | 103 | @MainActor |
| @@ -197,6 +210,42 @@ struct MRLifecycleActionTests { | |||
| 197 | // 4 GETs from `loaded()`, none from a reload. | 210 | // 4 GETs from `loaded()`, none from a reload. |
| 198 | #expect(gets(box) == 4) | 211 | #expect(gets(box) == 4) |
| 199 | } | 212 | } |
| 213 | |||
| 214 | @Test func closingWithoutASuccessorSendsNoBy() async throws { | ||
| 215 | let (model, box) = try await loaded() | ||
| 216 | ok(box) | ||
| 217 | await model.close() | ||
| 218 | let write = try #require(box.seen.first { $0.method == "POST" }) | ||
| 219 | #expect(try argvOf(write) == ["mr", "close", "krz/gitbay", "7"]) | ||
| 220 | } | ||
| 221 | |||
| 222 | @Test func closingInFavourOfAnotherPassesItsNumberAsBy() async throws { | ||
| 223 | let (model, box) = try await loaded() | ||
| 224 | ok(box) | ||
| 225 | await model.close(inFavourOf: 9) | ||
| 226 | let write = try #require(box.seen.first { $0.method == "POST" }) | ||
| 227 | #expect(try argvOf(write) == ["mr", "close", "krz/gitbay", "7", "--by", "9"]) | ||
| 228 | #expect(gets(box) == 8) | ||
| 229 | } | ||
| 230 | |||
| 231 | /// The picker offers the repository's other open merge requests. This | ||
| 232 | /// one cannot supersede itself, so it is dropped before the list is | ||
| 233 | /// shown rather than left for the server to refuse. | ||
| 234 | @Test func openMergeRequestsForThePickerExcludeThisOne() async throws { | ||
| 235 | let (model, box) = try await loaded() | ||
| 236 | box.enqueue(.init(status: 200, json: """ | ||
| 237 | {"protocol_version":1,"data":[\ | ||
| 238 | {"number":7,"title":"a change","state":"open","author":"cmc",\ | ||
| 239 | "source":"feat","target_ref":"main","head_sha":"abc123","created_at":"2026-09-01T00:00:00Z"},\ | ||
| 240 | {"number":9,"title":"the redo","state":"open","author":"cmc",\ | ||
| 241 | "source":"feat2","target_ref":"main","head_sha":"def456","created_at":"2026-09-02T00:00:00Z"}],\ | ||
| 242 | "exit_code":0} | ||
| 243 | """)) | ||
| 244 | await model.loadOpenMergeRequests() | ||
| 245 | let read = try #require(box.seen.last { $0.method == "GET" }) | ||
| 246 | #expect(read.url.query() == "argv=mr&argv=list&argv=krz/gitbay&argv=--state&argv=open") | ||
| 247 | #expect(model.openMergeRequests?.map(\.number) == [9]) | ||
| 248 | } | ||
| 200 | } | 249 | } |
| 201 | 250 | ||
| 202 | @MainActor | 251 | @MainActor |