Commit 3233092acd
Unsigned
Layout: unified · split
docs/superpowers/plans/2026-09-06-mr04-mr-lifecycle.md added +409
| @@ -0,0 +1,409 @@ | ||
| 1 | # MR 4: Merge request lifecycle Implementation Plan | |
| 2 | ||
| 3 | > **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. | |
| 4 | ||
| 5 | **Goal:** Close four parity rows at once — `draft, ready`, `retarget`, `request a review`, and `stacked merge requests`. | |
| 6 | ||
| 7 | **Architecture:** All four are already on the wire and none needs a server change. `mr show` and `mr list` carry `draft`, `review_requests`, `stacked_on` and `stacked` as `omitempty` fields the iOS models simply do not decode yet. So this is decoding plus four write actions on a screen that already has a write path. | |
| 8 | ||
| 9 | **Tech Stack:** Swift 6, default `MainActor` isolation, iOS 26.5, SwiftUI, Swift Testing, `@Observable`. | |
| 10 | ||
| 11 | **Spec:** `docs/superpowers/specs/2026-09-06-ios-parity-design.md` | |
| 12 | ||
| 13 | ## Global Constraints | |
| 14 | ||
| 15 | - Swift 6 language mode, default `MainActor` isolation. Wire models are `nonisolated struct`s. | |
| 16 | - Swift Testing only — never XCTest. | |
| 17 | - `gitbayTests` is hermetic and offline; network goes through `StubProtocol`. | |
| 18 | - New files under `gitbay/` and `gitbayTests/` need **no** `project.pbxproj` edit. | |
| 19 | - The label model is `IssueLabel`, never `Label`. | |
| 20 | - Never mention Claude, LLMs or AI in commits, comments, or the merge request. No `Co-Authored-By` trailer. | |
| 21 | - Never commit to `main`. | |
| 22 | ||
| 23 | **The commands, verbatim from the registry:** | |
| 24 | ||
| 25 | ``` | |
| 26 | mr draft <owner/name> <n> | |
| 27 | mr ready <owner/name> <n> | |
| 28 | mr retarget <owner/name> <n> <branch> | |
| 29 | mr review request <owner/name> <n> [--add <user>]... [--remove <user>]... | |
| 30 | mr create <target owner/name> --source [owner/name:]<branch> --target <branch> | |
| 31 | --title <t> [--body <b> | --file -] [--format md|org] [--draft] | |
| 32 | repo refs <owner/name> | |
| 33 | ``` | |
| 34 | ||
| 35 | Note `mr review request` is a **three-word path** — `["mr", "review", "request", repo, n]`. The app already sends `["mr", "review", verdict, ...]` for approvals, so getting this wrong silently posts a review instead of requesting one. | |
| 36 | ||
| 37 | **The wire fields**, from `internal/control/mr.go:337-357` — all `omitempty`, all currently undecoded by iOS: | |
| 38 | ||
| 39 | ```go | |
| 40 | Draft bool `json:"draft,omitempty"` | |
| 41 | ReviewRequests []string `json:"review_requests,omitempty"` | |
| 42 | StackedOn *stackRef `json:"stacked_on,omitempty"` // {number, title} | |
| 43 | Stacked []stackRef `json:"stacked,omitempty"` // [{number, title}] | |
| 44 | ``` | |
| 45 | ||
| 46 | `mr list` rows carry `draft` and `stacked_on`; `mr show` carries all four. | |
| 47 | ||
| 48 | **Build and test command:** | |
| 49 | ||
| 50 | ```bash | |
| 51 | xcodebuild -project gitbay.xcodeproj -scheme gitbay \ | |
| 52 | -destination 'platform=iOS Simulator,name=iPhone 17' \ | |
| 53 | test -only-testing:gitbayTests | |
| 54 | ``` | |
| 55 | ||
| 56 | Baseline: **258 passed, 0 failed, 1 skipped** (`LiveInstanceTests` — expected). | |
| 57 | ||
| 58 | --- | |
| 59 | ||
| 60 | ## File Structure | |
| 61 | ||
| 62 | | File | Responsibility | | |
| 63 | |------|----------------| | |
| 64 | | `gitbay/MRs/MRModels.swift` (modify) | Decode `draft`, `review_requests`, `stacked_on`, `stacked` | | |
| 65 | | `gitbay/MRs/MRDetailViewModel.swift` (modify) | `setDraft`, `retarget`, `requestReview`, `removeReviewRequest`, branch list | | |
| 66 | | `gitbay/MRs/MRCreateViewModel.swift` (modify) | `--draft` on create | | |
| 67 | | `gitbayTests/MRLifecycleTests.swift` (create) | Every test in this plan | | |
| 68 | | `gitbay/Views/MRs/MRView.swift` (modify) | Draft badge, stack section, reviewer rows, the three actions | | |
| 69 | | `gitbay/Views/MRs/MRListView.swift` (modify) | Draft badge, "stacked on" hint | | |
| 70 | ||
| 71 | --- | |
| 72 | ||
| 73 | ### Task 1: Decode the four fields | |
| 74 | ||
| 75 | **Files:** Modify `gitbay/MRs/MRModels.swift`; test in `gitbayTests/MRLifecycleTests.swift` | |
| 76 | ||
| 77 | **Interfaces produced:** | |
| 78 | - `MergeRequest.draft: Bool` (defaulting false when absent) and `MergeRequest.stackedOn: StackRef?` | |
| 79 | - `MRDetail.draft: Bool`, `.reviewRequests: [String]`, `.stackedOn: StackRef?`, `.stacked: [StackRef]` | |
| 80 | - `nonisolated struct StackRef: Decodable, Sendable, Hashable, Identifiable` — `number: Int64`, `title: String`, `id` is `number` | |
| 81 | ||
| 82 | **The subtlety this task exists to get right.** All four keys are `omitempty`, so **absence is the common case**: a non-draft merge request has no `draft` key at all, not `"draft": false`. `Bool` is not `Optional`, so a plain `let draft: Bool` fails to decode the ordinary merge request. Either declare them optional and expose non-optional accessors, or write `init(from:)` with `decodeIfPresent` and a default. The tests below pin the absent case for every field, because that is what production sends most of the time. | |
| 83 | ||
| 84 | `draft` is a flag, not a fifth state: `state` stays `"open"`. A draft merge request is open but not asking — it does not merge and does not appear in a review queue. `isOpen` must keep meaning what it means. | |
| 85 | ||
| 86 | - [ ] **Step 1: Write the failing tests** | |
| 87 | ||
| 88 | Create `gitbayTests/MRLifecycleTests.swift`: | |
| 89 | ||
| 90 | ```swift | |
| 91 | import Foundation | |
| 92 | import Testing | |
| 93 | @testable import gitbay | |
| 94 | ||
| 95 | private func decodeDetail(_ json: String) throws -> MRDetail { | |
| 96 | let decoder = JSONDecoder() | |
| 97 | decoder.dateDecodingStrategy = .iso8601 | |
| 98 | return try decoder.decode(MRDetail.self, from: Data(json.utf8)) | |
| 99 | } | |
| 100 | ||
| 101 | private func decodeRow(_ json: String) throws -> MergeRequest { | |
| 102 | let decoder = JSONDecoder() | |
| 103 | decoder.dateDecodingStrategy = .iso8601 | |
| 104 | return try decoder.decode(MergeRequest.self, from: Data(json.utf8)) | |
| 105 | } | |
| 106 | ||
| 107 | private let plainDetail = """ | |
| 108 | {"number":7,"title":"a change","state":"open","author":"cmc",\ | |
| 109 | "source":"feat","target_ref":"main","head_sha":"abc123",\ | |
| 110 | "created_at":"2026-09-01T00:00:00Z"} | |
| 111 | """ | |
| 112 | ||
| 113 | struct MRLifecycleDecodingTests { | |
| 114 | ||
| 115 | /// Every one of these keys is omitempty. The ordinary merge request | |
| 116 | /// carries none of them, so absence must decode, not throw. | |
| 117 | @Test func anOrdinaryMergeRequestHasNoneOfTheNewKeys() throws { | |
| 118 | let mr = try decodeDetail(plainDetail) | |
| 119 | #expect(mr.draft == false) | |
| 120 | #expect(mr.reviewRequests.isEmpty) | |
| 121 | #expect(mr.stackedOn == nil) | |
| 122 | #expect(mr.stacked.isEmpty) | |
| 123 | } | |
| 124 | ||
| 125 | @Test func aDraftDecodesAndStaysOpen() throws { | |
| 126 | let mr = try decodeDetail(""" | |
| 127 | {"number":7,"title":"a change","state":"open","draft":true,"author":"cmc",\ | |
| 128 | "source":"feat","target_ref":"main","head_sha":"abc123",\ | |
| 129 | "created_at":"2026-09-01T00:00:00Z"} | |
| 130 | """) | |
| 131 | #expect(mr.draft) | |
| 132 | // Draft is a flag, not a fifth state. | |
| 133 | #expect(mr.state == "open") | |
| 134 | #expect(mr.isOpen) | |
| 135 | } | |
| 136 | ||
| 137 | @Test func reviewRequestsDecode() throws { | |
| 138 | let mr = try decodeDetail(""" | |
| 139 | {"number":7,"title":"a change","state":"open","author":"cmc",\ | |
| 140 | "review_requests":["rae","sam"],\ | |
| 141 | "source":"feat","target_ref":"main","head_sha":"abc123",\ | |
| 142 | "created_at":"2026-09-01T00:00:00Z"} | |
| 143 | """) | |
| 144 | #expect(mr.reviewRequests == ["rae", "sam"]) | |
| 145 | } | |
| 146 | ||
| 147 | @Test func bothHalvesOfAStackDecode() throws { | |
| 148 | let mr = try decodeDetail(""" | |
| 149 | {"number":7,"title":"a change","state":"open","author":"cmc",\ | |
| 150 | "stacked_on":{"number":6,"title":"the one below"},\ | |
| 151 | "stacked":[{"number":8,"title":"one above"},{"number":9,"title":"another"}],\ | |
| 152 | "source":"feat","target_ref":"main","head_sha":"abc123",\ | |
| 153 | "created_at":"2026-09-01T00:00:00Z"} | |
| 154 | """) | |
| 155 | #expect(mr.stackedOn?.number == 6) | |
| 156 | #expect(mr.stackedOn?.title == "the one below") | |
| 157 | #expect(mr.stacked.map(\.number) == [8, 9]) | |
| 158 | } | |
| 159 | ||
| 160 | @Test func aListRowCarriesDraftAndStackedOn() throws { | |
| 161 | let row = try decodeRow(""" | |
| 162 | {"number":7,"title":"a change","state":"open","draft":true,"author":"cmc",\ | |
| 163 | "stacked_on":{"number":6,"title":"below"},\ | |
| 164 | "source":"feat","target_ref":"main","head_sha":"abc123",\ | |
| 165 | "created_at":"2026-09-01T00:00:00Z"} | |
| 166 | """) | |
| 167 | #expect(row.draft) | |
| 168 | #expect(row.stackedOn?.number == 6) | |
| 169 | } | |
| 170 | ||
| 171 | @Test func aPlainListRowHasNeither() throws { | |
| 172 | let row = try decodeRow(""" | |
| 173 | {"number":7,"title":"a change","state":"open","author":"cmc",\ | |
| 174 | "source":"feat","target_ref":"main","head_sha":"abc123",\ | |
| 175 | "created_at":"2026-09-01T00:00:00Z"} | |
| 176 | """) | |
| 177 | #expect(row.draft == false) | |
| 178 | #expect(row.stackedOn == nil) | |
| 179 | } | |
| 180 | } | |
| 181 | ``` | |
| 182 | ||
| 183 | - [ ] **Step 2: Run to verify failure.** | |
| 184 | - [ ] **Step 3: Implement.** Add `StackRef` to `MRModels.swift`, add the four fields to `MRDetail` and the two to `MergeRequest`, and extend both `CodingKeys`. Use whichever of the two approaches above is cleaner, but the non-optional accessors (`draft: Bool`, `reviewRequests: [String]`, `stacked: [StackRef]`) are what Tasks 2 and 3 consume — do not push optionality onto callers. | |
| 185 | - [ ] **Step 4: Run the tests.** Expect PASS and the full suite green. | |
| 186 | - [ ] **Step 5: Commit** — `git commit -m "Decode draft, review requests and the merge request stack"` | |
| 187 | ||
| 188 | --- | |
| 189 | ||
| 190 | ### Task 2: The four write actions | |
| 191 | ||
| 192 | **Files:** Modify `gitbay/MRs/MRDetailViewModel.swift` and `gitbay/MRs/MRCreateViewModel.swift`; test in `gitbayTests/MRLifecycleTests.swift` | |
| 193 | ||
| 194 | **Interfaces produced:** | |
| 195 | - `MRDetailViewModel.setDraft(_ draft: Bool) async` — `mr draft` when true, `mr ready` when false | |
| 196 | - `MRDetailViewModel.retarget(to branch: String) async` | |
| 197 | - `MRDetailViewModel.requestReview(from user: String) async` / `.removeReviewRequest(_ user: String) async` | |
| 198 | - `MRDetailViewModel.branches: [String]` and `func loadBranches() async` — from `repo refs`, for the retarget picker | |
| 199 | - `MRCreateViewModel` gains a `draft: Bool` input that appends `--draft` | |
| 200 | ||
| 201 | Follow the existing `perform(argv:stdin:)` in `MRDetailViewModel` — it already sets `working`, clears `actionError`, runs, and reloads. Do not restructure it. | |
| 202 | ||
| 203 | **`mr review request` is a three-word path.** The argv is `["mr", "review", "request", repoPath, number, "--add", user]`. The existing approve path sends `["mr", "review", verdict, ...]`, so a two-word mistake here posts a review rather than requesting one — silently, with a 200. The tests below assert the exact argv for this reason. | |
| 204 | ||
| 205 | `repo refs` returns `{branches: [{name, sha}], tags: [...]}`. `RepoRefs` and `RepoRef` already exist and `RefsViewModel.load` already reads this command — reuse both models rather than writing a second pair. | |
| 206 | ||
| 207 | - [ ] **Step 1: Write the failing tests** — append to `gitbayTests/MRLifecycleTests.swift`: | |
| 208 | ||
| 209 | ```swift | |
| 210 | @MainActor | |
| 211 | struct MRLifecycleActionTests { | |
| 212 | ||
| 213 | private func loaded() async throws -> (MRDetailViewModel, StubProtocol.Box) { | |
| 214 | let box = StubProtocol.box() | |
| 215 | let client = GitbayClient( | |
| 216 | instance: try GitbayInstance(url: "https://gitbay.org"), | |
| 217 | token: "test-token", | |
| 218 | session: box.session() | |
| 219 | ) | |
| 220 | box.enqueue(.init(status: 200, json: """ | |
| 221 | {"protocol_version":1,"data":\(plainDetail),"exit_code":0} | |
| 222 | """)) | |
| 223 | let model = MRDetailViewModel(client: client, repoPath: "krz/gitbay", number: 7) | |
| 224 | await model.load() | |
| 225 | return (model, box) | |
| 226 | } | |
| 227 | ||
| 228 | private func argvOf(_ seen: StubProtocol.Seen) throws -> [String] { | |
| 229 | let body = try #require(try JSONSerialization.jsonObject(with: seen.body) as? [String: Any]) | |
| 230 | return try #require(body["argv"] as? [String]) | |
| 231 | } | |
| 232 | ||
| 233 | private func ok(_ box: StubProtocol.Box) { | |
| 234 | box.enqueue(.init(status: 200, json: """ | |
| 235 | {"protocol_version":1,"exit_code":0} | |
| 236 | """)) | |
| 237 | box.enqueue(.init(status: 200, json: """ | |
| 238 | {"protocol_version":1,"data":\(plainDetail),"exit_code":0} | |
| 239 | """)) | |
| 240 | } | |
| 241 | ||
| 242 | @Test func markingDraftAndReadyAreDifferentCommands() async throws { | |
| 243 | let (model, box) = try await loaded() | |
| 244 | ok(box) | |
| 245 | await model.setDraft(true) | |
| 246 | var write = try #require(box.seen.first { $0.method == "POST" }) | |
| 247 | #expect(try argvOf(write) == ["mr", "draft", "krz/gitbay", "7"]) | |
| 248 | ||
| 249 | let (model2, box2) = try await loaded() | |
| 250 | ok(box2) | |
| 251 | await model2.setDraft(false) | |
| 252 | write = try #require(box2.seen.first { $0.method == "POST" }) | |
| 253 | #expect(try argvOf(write) == ["mr", "ready", "krz/gitbay", "7"]) | |
| 254 | } | |
| 255 | ||
| 256 | @Test func retargetPassesTheBranchPositionally() async throws { | |
| 257 | let (model, box) = try await loaded() | |
| 258 | ok(box) | |
| 259 | await model.retarget(to: "release") | |
| 260 | let write = try #require(box.seen.first { $0.method == "POST" }) | |
| 261 | #expect(try argvOf(write) == ["mr", "retarget", "krz/gitbay", "7", "release"]) | |
| 262 | } | |
| 263 | ||
| 264 | /// `mr review request` is a THREE-word path. Two words posts a | |
| 265 | /// review instead — silently, with a 200. | |
| 266 | @Test func requestingAReviewUsesTheThreeWordPath() async throws { | |
| 267 | let (model, box) = try await loaded() | |
| 268 | ok(box) | |
| 269 | await model.requestReview(from: "rae") | |
| 270 | let write = try #require(box.seen.first { $0.method == "POST" }) | |
| 271 | #expect(try argvOf(write) | |
| 272 | == ["mr", "review", "request", "krz/gitbay", "7", "--add", "rae"]) | |
| 273 | } | |
| 274 | ||
| 275 | @Test func removingAReviewRequestUsesRemoveNotAdd() async throws { | |
| 276 | let (model, box) = try await loaded() | |
| 277 | ok(box) | |
| 278 | await model.removeReviewRequest("rae") | |
| 279 | let write = try #require(box.seen.first { $0.method == "POST" }) | |
| 280 | #expect(try argvOf(write) | |
| 281 | == ["mr", "review", "request", "krz/gitbay", "7", "--remove", "rae"]) | |
| 282 | } | |
| 283 | ||
| 284 | @Test func branchesComeFromRepoRefs() async throws { | |
| 285 | let (model, box) = try await loaded() | |
| 286 | box.enqueue(.init(status: 200, json: """ | |
| 287 | {"protocol_version":1,"data":{"branches":[{"name":"main","sha":"a"},\ | |
| 288 | {"name":"release","sha":"b"}],"tags":[]},"exit_code":0} | |
| 289 | """)) | |
| 290 | await model.loadBranches() | |
| 291 | #expect(model.branches == ["main", "release"]) | |
| 292 | } | |
| 293 | ||
| 294 | @Test func aRefusalSurfacesAndDoesNotReload() async throws { | |
| 295 | let (model, box) = try await loaded() | |
| 296 | box.enqueue(.init(status: 200, json: """ | |
| 297 | {"protocol_version":1,"error":"cannot retarget a merged merge request",\ | |
| 298 | "exit_code":1} | |
| 299 | """)) | |
| 300 | await model.retarget(to: "release") | |
| 301 | #expect(model.actionError?.isEmpty == false) | |
| 302 | #expect(model.working == false) | |
| 303 | } | |
| 304 | } | |
| 305 | ||
| 306 | @MainActor | |
| 307 | struct MRCreateDraftTests { | |
| 308 | ||
| 309 | @Test func creatingAsADraftAppendsTheFlag() async throws { | |
| 310 | let box = StubProtocol.box() | |
| 311 | let client = GitbayClient( | |
| 312 | instance: try GitbayInstance(url: "https://gitbay.org"), | |
| 313 | token: "test-token", | |
| 314 | session: box.session() | |
| 315 | ) | |
| 316 | box.enqueue(.init(status: 200, json: """ | |
| 317 | {"protocol_version":1,"data":{"number":7},"exit_code":0} | |
| 318 | """)) | |
| 319 | let model = MRCreateViewModel(client: client, repoPath: "krz/gitbay") | |
| 320 | model.draft = true | |
| 321 | _ = await model.create(source: "feat", target: "main", title: "a change", body: "") | |
| 322 | ||
| 323 | let write = try #require(box.seen.first { $0.method == "POST" }) | |
| 324 | let body = try #require( | |
| 325 | try JSONSerialization.jsonObject(with: write.body) as? [String: Any] | |
| 326 | ) | |
| 327 | let argv = try #require(body["argv"] as? [String]) | |
| 328 | #expect(argv.contains("--draft")) | |
| 329 | } | |
| 330 | ||
| 331 | @Test func creatingWithoutDraftOmitsTheFlag() async throws { | |
| 332 | let box = StubProtocol.box() | |
| 333 | let client = GitbayClient( | |
| 334 | instance: try GitbayInstance(url: "https://gitbay.org"), | |
| 335 | token: "test-token", | |
| 336 | session: box.session() | |
| 337 | ) | |
| 338 | box.enqueue(.init(status: 200, json: """ | |
| 339 | {"protocol_version":1,"data":{"number":7},"exit_code":0} | |
| 340 | """)) | |
| 341 | let model = MRCreateViewModel(client: client, repoPath: "krz/gitbay") | |
| 342 | _ = await model.create(source: "feat", target: "main", title: "a change", body: "") | |
| 343 | ||
| 344 | let write = try #require(box.seen.first { $0.method == "POST" }) | |
| 345 | let body = try #require( | |
| 346 | try JSONSerialization.jsonObject(with: write.body) as? [String: Any] | |
| 347 | ) | |
| 348 | #expect((body["argv"] as? [String])?.contains("--draft") == false) | |
| 349 | } | |
| 350 | } | |
| 351 | ``` | |
| 352 | ||
| 353 | `MRCreateViewModel.create(source:target:title:body:)` is the existing signature and the tests above match it. Add `draft` as a settable property rather than a fifth parameter, so the call sites do not all change. | |
| 354 | ||
| 355 | - [ ] **Step 2: Run to verify failure.** | |
| 356 | - [ ] **Step 3: Implement the five methods and the `draft` input.** | |
| 357 | - [ ] **Step 4: Run the tests.** | |
| 358 | - [ ] **Step 5: Commit** — `git commit -m "Draft, retarget and review requests on a merge request"` | |
| 359 | ||
| 360 | --- | |
| 361 | ||
| 362 | ### Task 3: The UI | |
| 363 | ||
| 364 | **Files:** Modify `gitbay/Views/MRs/MRView.swift` and `gitbay/Views/MRs/MRListView.swift` | |
| 365 | ||
| 366 | No unit tests — UI. Verification is the build plus the suite staying green. | |
| 367 | ||
| 368 | **On the list row:** a `GBChip("draft", .secondary)` when `mr.draft`, and a quiet "stacked on !N" line when `mr.stackedOn != nil`. Match the row's existing density; do not add a line to every row. | |
| 369 | ||
| 370 | **On the detail screen:** | |
| 371 | - Draft badge in the header beside the state. | |
| 372 | - A **Stack** section when `stackedOn != nil || !stacked.isEmpty`, listing both directions — what this is stacked on, and what is stacked on it — each row a `NavigationLink(value: MRRoute.mr(repo:number:))`. Say which direction each is; "stacked on !6" and "!8 is stacked on this" read differently and the user needs to know which. | |
| 373 | - A **Reviewers** section listing `reviewRequests`, each removable, with a field to add one. Reuse the add/remove shape `IssueView` already uses for labels and assignees rather than inventing another. | |
| 374 | - Actions, placed with the existing merge/close controls: **Mark as draft** / **Mark as ready** (whichever the current state implies), and **Retarget**, which presents a branch picker fed by `loadBranches()`. | |
| 375 | ||
| 376 | **Two things the UI must say, because the server enforces them and a silent refusal is worse than a warning:** | |
| 377 | 1. **Retargeting stales the reviews** — an approval was of the diff against the old branch. Say so in the confirmation. | |
| 378 | 2. **A squash or rebase merge is refused while anything is stacked on this merge request**, since it would rewrite the commits the stack builds on. The merge sheet offers Squash, Fast-forward and Rebase at `MRView.swift:63-65`; when `!stacked.isEmpty`, offer only Fast-forward. | |
| 379 | ||
| 380 | A draft merge request does not merge. When `mr.draft`, the merge control should say so rather than presenting a button the server will refuse. | |
| 381 | ||
| 382 | - [ ] **Step 1: List row — draft chip and stacked hint** | |
| 383 | - [ ] **Step 2: Detail — draft badge, Stack section, Reviewers section** | |
| 384 | - [ ] **Step 3: Detail — draft/ready and retarget actions, with the two warnings above** | |
| 385 | - [ ] **Step 4: Build and run the full suite.** Expect no drop from 258 + Tasks 1-2's additions. | |
| 386 | - [ ] **Step 5: Commit** — `git commit -m "Draft, stack and reviewers on the merge request screen"` | |
| 387 | ||
| 388 | --- | |
| 389 | ||
| 390 | ### Task 4: Flip four parity rows and open the merge request | |
| 391 | ||
| 392 | - [ ] **Step 1:** In the Merge requests table, set `draft, ready`, `retarget`, `request a review` and `stacked merge requests` to `yes` for iOS. Touch no other row — in particular leave `choose body markup`, which is MR 11. | |
| 393 | ||
| 394 | Land it via a worktree off `origin/main` in `krz/gitbay`, merged `--strategy ff`. That repo requires signed commits, so squash and rebase merges are refused, and its working tree usually holds unrelated work — never switch its branch. | |
| 395 | ||
| 396 | - [ ] **Step 2:** Run the full suite; record the real number. | |
| 397 | - [ ] **Step 3:** Open the merge request. | |
| 398 | ||
| 399 | --- | |
| 400 | ||
| 401 | ## Notes for whoever executes this | |
| 402 | ||
| 403 | **`mr review request` is three words.** `["mr", "review", "request", ...]`. The approve path is `["mr", "review", verdict, ...]`. A two-word slip posts a review and returns 200, so nothing surfaces — which is exactly why two tests assert the full argv. | |
| 404 | ||
| 405 | **Absence is the common case for all four new fields.** They are `omitempty`. Most merge requests carry none of them, so the decoding must default rather than require. Every test above that pins an absent field is pinning the ordinary case, not an edge case. | |
| 406 | ||
| 407 | **Draft is a flag, not a state.** `state` stays `"open"`. Do not add a `.draft` case to any state enum, and do not let `isOpen` change meaning — every `state = 'open'` rule on the server still means what it did. | |
| 408 | ||
| 409 | **Do not offer a merge strategy the server will refuse.** Squash and rebase are refused while anything is stacked on the merge request. | |
gitbay/MRs/MRCreateViewModel.swift +2
| @@ -13,6 +13,7 @@ final class MRCreateViewModel { | ||
| 13 | 13 | private(set) var working = false |
| 14 | 14 | private(set) var errorMessage: String? |
| 15 | 15 | private(set) var defaultBranch: String? |
| 16 | var draft = false | |
| 16 | 17 | |
| 17 | 18 | private let client: GitbayClient |
| 18 | 19 | let repoPath: String |
| @@ -45,6 +46,7 @@ final class MRCreateViewModel { | ||
| 45 | 46 | argv.append(contentsOf: ["--file", "-"]) |
| 46 | 47 | stdin = body |
| 47 | 48 | } |
| 49 | if draft { argv.append("--draft") } | |
| 48 | 50 | let created = try await client.run(argv, stdin: stdin, as: Created.self) |
| 49 | 51 | return created?.number |
| 50 | 52 | } catch let error as GitbayError { |
gitbay/MRs/MRDetailViewModel.swift +42
| @@ -108,6 +108,48 @@ final class MRDetailViewModel { | ||
| 108 | 108 | ) |
| 109 | 109 | } |
| 110 | 110 | |
| 111 | func setDraft(_ draft: Bool) async { | |
| 112 | await perform(["mr", draft ? "draft" : "ready"] + ref) | |
| 113 | } | |
| 114 | ||
| 115 | func retarget(to branch: String) async { | |
| 116 | await perform(["mr", "retarget"] + ref + [branch]) | |
| 117 | } | |
| 118 | ||
| 119 | func requestReview(from user: String) async { | |
| 120 | await perform(["mr", "review", "request"] + ref + ["--add", user]) | |
| 121 | } | |
| 122 | ||
| 123 | func removeReviewRequest(_ user: String) async { | |
| 124 | await perform(["mr", "review", "request"] + ref + ["--remove", user]) | |
| 125 | } | |
| 126 | ||
| 127 | /// Branches for the retarget picker, fetched on demand from `repo refs`. | |
| 128 | /// `nil` means "not yet loaded" so the picker can tell that apart from | |
| 129 | /// a repository that genuinely has no other branches; a failed fetch | |
| 130 | /// leaves `branches` nil and sets `branchesError` instead of silently | |
| 131 | /// showing an empty list. | |
| 132 | private(set) var branches: [String]? | |
| 133 | private(set) var branchesError: String? | |
| 134 | ||
| 135 | /// Cached for the life of this view model, same as `loadMilestones()`: | |
| 136 | /// branches can go stale after a push, but re-opening the retarget | |
| 137 | /// picker should not re-fetch every time. A pull-to-refresh of the | |
| 138 | /// whole screen doesn't clear this cache either — retarget is a | |
| 139 | /// secondary action, not the primary content `load()` refreshes. | |
| 140 | func loadBranches() async { | |
| 141 | guard branches == nil else { return } | |
| 142 | branchesError = nil | |
| 143 | do { | |
| 144 | let refs = try await client.read(["repo", "refs", repoPath], as: RepoRefs.self) | |
| 145 | branches = refs.branches.map(\.name) | |
| 146 | } catch let error as GitbayError { | |
| 147 | branchesError = error.userFacingMessage | |
| 148 | } catch { | |
| 149 | branchesError = GitbayError.transport(error).userFacingMessage | |
| 150 | } | |
| 151 | } | |
| 152 | ||
| 111 | 153 | private func perform(_ argv: [String], stdin: String? = nil) async { |
| 112 | 154 | working = true |
| 113 | 155 | actionError = nil |
gitbay/MRs/MRModels.swift +64 −2
| @@ -1,5 +1,13 @@ | ||
| 1 | 1 | import Foundation |
| 2 | 2 | |
| 3 | /// The MR below or above a stack member: `mr show` sends `{number, title}`. | |
| 4 | nonisolated struct StackRef: Decodable, Sendable, Hashable, Identifiable { | |
| 5 | let number: Int64 | |
| 6 | let title: String | |
| 7 | ||
| 8 | var id: Int64 { number } | |
| 9 | } | |
| 10 | ||
| 3 | 11 | /// One row of `mr list`, and the header half of `mr show`. |
| 4 | 12 | nonisolated struct MergeRequest: Decodable, Sendable, Hashable, Identifiable { |
| 5 | 13 | let number: Int64 |
| @@ -13,12 +21,31 @@ nonisolated struct MergeRequest: Decodable, Sendable, Hashable, Identifiable { | ||
| 13 | 21 | let headSHA: String |
| 14 | 22 | let body: String? |
| 15 | 23 | let createdAt: Date |
| 24 | /// Open but not asking: does not merge, does not appear in a review queue. | |
| 25 | let draft: Bool | |
| 26 | let stackedOn: StackRef? | |
| 16 | 27 | |
| 17 | 28 | enum CodingKeys: String, CodingKey { |
| 18 | case number, title, state, author, source, body | |
| 29 | case number, title, state, author, source, body, draft | |
| 19 | 30 | case targetRef = "target_ref" |
| 20 | 31 | case headSHA = "head_sha" |
| 21 | 32 | case createdAt = "created_at" |
| 33 | case stackedOn = "stacked_on" | |
| 34 | } | |
| 35 | ||
| 36 | init(from decoder: Decoder) throws { | |
| 37 | let container = try decoder.container(keyedBy: CodingKeys.self) | |
| 38 | number = try container.decode(Int64.self, forKey: .number) | |
| 39 | title = try container.decode(String.self, forKey: .title) | |
| 40 | state = try container.decode(String.self, forKey: .state) | |
| 41 | author = try container.decode(String.self, forKey: .author) | |
| 42 | source = try container.decode(String.self, forKey: .source) | |
| 43 | targetRef = try container.decode(String.self, forKey: .targetRef) | |
| 44 | headSHA = try container.decode(String.self, forKey: .headSHA) | |
| 45 | body = try container.decodeIfPresent(String.self, forKey: .body) | |
| 46 | createdAt = try container.decode(Date.self, forKey: .createdAt) | |
| 47 | draft = try container.decodeIfPresent(Bool.self, forKey: .draft) ?? false | |
| 48 | stackedOn = try container.decodeIfPresent(StackRef.self, forKey: .stackedOn) | |
| 22 | 49 | } |
| 23 | 50 | |
| 24 | 51 | var id: Int64 { number } |
| @@ -49,9 +76,14 @@ nonisolated struct MRDetail: Decodable, Sendable, Hashable { | ||
| 49 | 76 | let commits: [MRCommit]? |
| 50 | 77 | let comments: [MRComment]? |
| 51 | 78 | let reviews: [Review]? |
| 79 | /// Open but not asking: does not merge, does not appear in a review queue. | |
| 80 | let draft: Bool | |
| 81 | let reviewRequests: [String] | |
| 82 | let stackedOn: StackRef? | |
| 83 | let stacked: [StackRef] | |
| 52 | 84 | |
| 53 | 85 | enum CodingKeys: String, CodingKey { |
| 54 | case number, title, state, author, source, body, milestone, checks, commits, comments, reviews | |
| 86 | case number, title, state, author, source, body, milestone, checks, commits, comments, reviews, draft, stacked | |
| 55 | 87 | case targetRef = "target_ref" |
| 56 | 88 | case headSHA = "head_sha" |
| 57 | 89 | case createdAt = "created_at" |
| @@ -61,6 +93,36 @@ nonisolated struct MRDetail: Decodable, Sendable, Hashable { | ||
| 61 | 93 | case closedBy = "closed_by" |
| 62 | 94 | case checksCombined = "checks_combined" |
| 63 | 95 | case unresolvedThreads = "unresolved_threads" |
| 96 | case reviewRequests = "review_requests" | |
| 97 | case stackedOn = "stacked_on" | |
| 98 | } | |
| 99 | ||
| 100 | init(from decoder: Decoder) throws { | |
| 101 | let container = try decoder.container(keyedBy: CodingKeys.self) | |
| 102 | number = try container.decode(Int64.self, forKey: .number) | |
| 103 | title = try container.decode(String.self, forKey: .title) | |
| 104 | state = try container.decode(String.self, forKey: .state) | |
| 105 | author = try container.decode(String.self, forKey: .author) | |
| 106 | source = try container.decode(String.self, forKey: .source) | |
| 107 | targetRef = try container.decode(String.self, forKey: .targetRef) | |
| 108 | headSHA = try container.decode(String.self, forKey: .headSHA) | |
| 109 | body = try container.decodeIfPresent(String.self, forKey: .body) | |
| 110 | milestone = try container.decodeIfPresent(String.self, forKey: .milestone) | |
| 111 | createdAt = try container.decode(Date.self, forKey: .createdAt) | |
| 112 | mergedAt = try container.decodeIfPresent(Date.self, forKey: .mergedAt) | |
| 113 | mergedBy = try container.decodeIfPresent(String.self, forKey: .mergedBy) | |
| 114 | closedAt = try container.decodeIfPresent(Date.self, forKey: .closedAt) | |
| 115 | closedBy = try container.decodeIfPresent(String.self, forKey: .closedBy) | |
| 116 | checks = try container.decodeIfPresent([Check].self, forKey: .checks) | |
| 117 | checksCombined = try container.decodeIfPresent(String.self, forKey: .checksCombined) | |
| 118 | unresolvedThreads = try container.decodeIfPresent(Int.self, forKey: .unresolvedThreads) | |
| 119 | commits = try container.decodeIfPresent([MRCommit].self, forKey: .commits) | |
| 120 | comments = try container.decodeIfPresent([MRComment].self, forKey: .comments) | |
| 121 | reviews = try container.decodeIfPresent([Review].self, forKey: .reviews) | |
| 122 | draft = try container.decodeIfPresent(Bool.self, forKey: .draft) ?? false | |
| 123 | reviewRequests = try container.decodeIfPresent([String].self, forKey: .reviewRequests) ?? [] | |
| 124 | stackedOn = try container.decodeIfPresent(StackRef.self, forKey: .stackedOn) | |
| 125 | stacked = try container.decodeIfPresent([StackRef].self, forKey: .stacked) ?? [] | |
| 64 | 126 | } |
| 65 | 127 | |
| 66 | 128 | var isOpen: Bool { state == "open" } |
gitbay/Views/MRs/MRListView.swift +14
| @@ -116,6 +116,12 @@ private struct MRCreateSheet: View { | ||
| 116 | 116 | .autocorrectionDisabled() |
| 117 | 117 | .accessibilityIdentifier("mr-body") |
| 118 | 118 | } |
| 119 | Section { | |
| 120 | Toggle("Draft", isOn: Bindable(model).draft) | |
| 121 | .accessibilityIdentifier("mr-draft") | |
| 122 | } footer: { | |
| 123 | Text("A draft merge request is open but not asking for review: it won't merge and won't show up in a review queue.") | |
| 124 | } | |
| 119 | 125 | if let error = model.errorMessage { |
| 120 | 126 | Section { |
| 121 | 127 | GBNotice(error) |
| @@ -177,6 +183,9 @@ struct MRRow: View { | ||
| 177 | 183 | } |
| 178 | 184 | HStack(spacing: 6) { |
| 179 | 185 | MRStateBadge(state: mr.state) |
| 186 | if mr.draft { | |
| 187 | GBChip("draft", .secondary) | |
| 188 | } | |
| 180 | 189 | Text(mr.source.isEmpty ? "(source gone)" : mr.source) |
| 181 | 190 | .lineLimit(1) |
| 182 | 191 | Image(systemName: "arrow.right") |
| @@ -188,6 +197,11 @@ struct MRRow: View { | ||
| 188 | 197 | } |
| 189 | 198 | .font(.gbSans(.caption)) |
| 190 | 199 | .foregroundStyle(.secondary) |
| 200 | if let stackedOn = mr.stackedOn { | |
| 201 | Text("stacked on !\(stackedOn.number)") | |
| 202 | .font(.gbSans(.caption2)) | |
| 203 | .foregroundStyle(.tertiary) | |
| 204 | } | |
| 191 | 205 | } |
| 192 | 206 | .padding(.vertical, 2) |
| 193 | 207 | } |
gitbay/Views/MRs/MRView.swift +181 −6
| @@ -9,6 +9,9 @@ struct MRView: View { | ||
| 9 | 9 | @State private var editing = false |
| 10 | 10 | @State private var draftTitle = "" |
| 11 | 11 | @State private var draftBody = "" |
| 12 | @State private var editingReviewer = "" | |
| 13 | @State private var retargeting = false | |
| 14 | @State private var pendingRetarget: String? | |
| 12 | 15 | |
| 13 | 16 | init(client: GitbayClient, repo: String, number: Int64) { |
| 14 | 17 | _model = State(initialValue: MRDetailViewModel( |
| @@ -35,6 +38,8 @@ struct MRView: View { | ||
| 35 | 38 | } |
| 36 | 39 | |
| 37 | 40 | milestoneSection(mr) |
| 41 | stackSection(mr) | |
| 42 | reviewersSection(mr) | |
| 38 | 43 | diffSection |
| 39 | 44 | |
| 40 | 45 | if let commits = mr.commits, !commits.isEmpty { |
| @@ -59,18 +64,50 @@ struct MRView: View { | ||
| 59 | 64 | .task { await model.load() } |
| 60 | 65 | .refreshable { await model.load() } |
| 61 | 66 | .confirmationDialog("Merge !\(model.number)?", isPresented: $confirmingMerge) { |
| 62 | Button("Merge") { Task { await model.merge() } } | |
| 63 | Button("Squash") { Task { await model.merge(strategy: "squash") } } | |
| 64 | Button("Fast-forward") { Task { await model.merge(strategy: "ff") } } | |
| 65 | Button("Rebase") { Task { await model.merge(strategy: "rebase") } } | |
| 67 | if let mr = model.state.value, !mr.stacked.isEmpty { | |
| 68 | Button("Merge") { Task { await model.merge() } } | |
| 69 | Button("Fast-forward") { Task { await model.merge(strategy: "ff") } } | |
| 70 | } else { | |
| 71 | Button("Merge") { Task { await model.merge() } } | |
| 72 | Button("Squash") { Task { await model.merge(strategy: "squash") } } | |
| 73 | Button("Fast-forward") { Task { await model.merge(strategy: "ff") } } | |
| 74 | Button("Rebase") { Task { await model.merge(strategy: "rebase") } } | |
| 75 | } | |
| 66 | 76 | Button("Cancel", role: .cancel) {} |
| 67 | 77 | } message: { |
| 68 | Text("The server enforces approvals, threads and checks — a refusal will say why.") | |
| 78 | if let mr = model.state.value, !mr.stacked.isEmpty { | |
| 79 | Text("Merge requests are stacked on this one. Squash and rebase are unavailable — they would rewrite the commits the stack builds on.") | |
| 80 | } else { | |
| 81 | Text("The server enforces approvals, threads and checks — a refusal will say why.") | |
| 82 | } | |
| 69 | 83 | } |
| 70 | 84 | .confirmationDialog("Close !\(model.number) without merging?", isPresented: $confirmingClose) { |
| 71 | 85 | Button("Close", role: .destructive) { Task { await model.close() } } |
| 72 | 86 | Button("Cancel", role: .cancel) {} |
| 73 | 87 | } |
| 88 | .confirmationDialog( | |
| 89 | "Retarget to \(pendingRetarget ?? "")?", | |
| 90 | isPresented: .init( | |
| 91 | get: { pendingRetarget != nil }, | |
| 92 | set: { if !$0 { pendingRetarget = nil } } | |
| 93 | ), | |
| 94 | titleVisibility: .visible | |
| 95 | ) { | |
| 96 | Button("Retarget") { | |
| 97 | guard let branch = pendingRetarget else { return } | |
| 98 | pendingRetarget = nil | |
| 99 | Task { await model.retarget(to: branch) } | |
| 100 | } | |
| 101 | Button("Cancel", role: .cancel) { pendingRetarget = nil } | |
| 102 | } message: { | |
| 103 | Text("Existing approvals were of the diff against the old target and will no longer apply.") | |
| 104 | } | |
| 105 | .sheet(isPresented: $retargeting) { | |
| 106 | RetargetSheet(model: model) { branch in | |
| 107 | retargeting = false | |
| 108 | pendingRetarget = branch | |
| 109 | } | |
| 110 | } | |
| 74 | 111 | .sheet(isPresented: $editing) { |
| 75 | 112 | ComposeSheet( |
| 76 | 113 | heading: "Edit !\(model.number)", |
| @@ -98,6 +135,9 @@ struct MRView: View { | ||
| 98 | 135 | .font(.gbSans(.headline)) |
| 99 | 136 | HStack(spacing: 6) { |
| 100 | 137 | MRStateBadge(state: mr.state) |
| 138 | if mr.draft { | |
| 139 | GBChip("draft", .secondary) | |
| 140 | } | |
| 101 | 141 | Text(mr.source.isEmpty ? "(source gone)" : mr.source) |
| 102 | 142 | .lineLimit(1) |
| 103 | 143 | Image(systemName: "arrow.right") |
| @@ -170,6 +210,79 @@ struct MRView: View { | ||
| 170 | 210 | } |
| 171 | 211 | } |
| 172 | 212 | |
| 213 | /// Shown only when this merge request is part of a stack. Each row | |
| 214 | /// spells out its own direction — "stacked on" vs. "is stacked on | |
| 215 | /// this" mean opposite things and look alike if abbreviated. | |
| 216 | @ViewBuilder | |
| 217 | private func stackSection(_ mr: MRDetail) -> some View { | |
| 218 | if mr.stackedOn != nil || !mr.stacked.isEmpty { | |
| 219 | Section("Stack") { | |
| 220 | if let base = mr.stackedOn { | |
| 221 | NavigationLink(value: MRRoute.mr(repo: model.repoPath, number: base.number)) { | |
| 222 | Label("stacked on !\(base.number) — \(base.title)", systemImage: "arrow.up.to.line") | |
| 223 | .font(.gbSans(.subheadline)) | |
| 224 | .lineLimit(1) | |
| 225 | } | |
| 226 | } | |
| 227 | ForEach(mr.stacked) { ref in | |
| 228 | NavigationLink(value: MRRoute.mr(repo: model.repoPath, number: ref.number)) { | |
| 229 | Label("!\(ref.number) is stacked on this — \(ref.title)", systemImage: "arrow.down.to.line") | |
| 230 | .font(.gbSans(.subheadline)) | |
| 231 | .lineLimit(1) | |
| 232 | } | |
| 233 | } | |
| 234 | } | |
| 235 | } | |
| 236 | } | |
| 237 | ||
| 238 | private func reviewersSection(_ mr: MRDetail) -> some View { | |
| 239 | Section("Reviewers") { | |
| 240 | reviewerFlow(mr.reviewRequests) | |
| 241 | HStack { | |
| 242 | TextField("Request review", text: $editingReviewer) | |
| 243 | .autocorrectionDisabled() | |
| 244 | .textInputAutocapitalization(.never) | |
| 245 | Button { | |
| 246 | let user = editingReviewer.trimmingCharacters(in: .whitespaces) | |
| 247 | editingReviewer = "" | |
| 248 | Task { await model.requestReview(from: user) } | |
| 249 | } label: { | |
| 250 | Image(systemName: "plus.circle.fill") | |
| 251 | } | |
| 252 | .disabled(editingReviewer.trimmingCharacters(in: .whitespaces).isEmpty || model.working) | |
| 253 | } | |
| 254 | } | |
| 255 | } | |
| 256 | ||
| 257 | /// The same add/remove chip shape `IssueView` uses for labels and | |
| 258 | /// assignees. | |
| 259 | @ViewBuilder | |
| 260 | private func reviewerFlow(_ items: [String]) -> some View { | |
| 261 | if !items.isEmpty { | |
| 262 | ScrollView(.horizontal, showsIndicators: false) { | |
| 263 | HStack(spacing: 6) { | |
| 264 | ForEach(items, id: \.self) { item in | |
| 265 | HStack(spacing: 3) { | |
| 266 | Text(item) | |
| 267 | Button { | |
| 268 | Task { await model.removeReviewRequest(item) } | |
| 269 | } label: { | |
| 270 | Image(systemName: "xmark.circle.fill") | |
| 271 | .foregroundStyle(.tertiary) | |
| 272 | } | |
| 273 | .disabled(model.working) | |
| 274 | } | |
| 275 | .font(.gbSans(.caption)) | |
| 276 | .padding(.horizontal, 8) | |
| 277 | .padding(.vertical, 3) | |
| 278 | .background(Color.secondary.opacity(0.07), in: gbChipShape) | |
| 279 | .overlay(gbChipShape.stroke(Color.secondary.opacity(0.35), lineWidth: 1)) | |
| 280 | } | |
| 281 | } | |
| 282 | } | |
| 283 | } | |
| 284 | } | |
| 285 | ||
| 173 | 286 | private var diffSection: some View { |
| 174 | 287 | Section { |
| 175 | 288 | NavigationLink(value: MRRoute.diff(repo: model.repoPath, number: model.number)) { |
| @@ -327,11 +440,25 @@ struct MRView: View { | ||
| 327 | 440 | Label("Request Changes", systemImage: "exclamationmark.circle") |
| 328 | 441 | } |
| 329 | 442 | Divider() |
| 443 | Button { | |
| 444 | Task { await model.setDraft(!mr.draft) } | |
| 445 | } label: { | |
| 446 | Label(mr.draft ? "Mark as ready" : "Mark as draft", | |
| 447 | systemImage: mr.draft ? "checkmark.circle" : "pencil.circle") | |
| 448 | } | |
| 449 | Button { | |
| 450 | retargeting = true | |
| 451 | } label: { | |
| 452 | Label("Retarget", systemImage: "arrow.triangle.branch") | |
| 453 | } | |
| 454 | Divider() | |
| 330 | 455 | Button { |
| 331 | 456 | confirmingMerge = true |
| 332 | 457 | } label: { |
| 333 | Label("Merge", systemImage: "arrow.triangle.merge") | |
| 458 | Label(mr.draft ? "Merge (mark as ready first)" : "Merge", | |
| 459 | systemImage: "arrow.triangle.merge") | |
| 334 | 460 | } |
| 461 | .disabled(mr.draft) | |
| 335 | 462 | Button(role: .destructive) { |
| 336 | 463 | confirmingClose = true |
| 337 | 464 | } label: { |
| @@ -430,3 +557,51 @@ struct ReviewThreadView: View { | ||
| 430 | 557 | .padding(.vertical, 4) |
| 431 | 558 | } |
| 432 | 559 | } |
| 560 | ||
| 561 | /// A single-purpose branch picker for retarget. `model.branches` is | |
| 562 | /// `nil` while loading and `model.branchesError` distinguishes a failed | |
| 563 | /// `repo refs` fetch from a repository that genuinely has no other | |
| 564 | /// branches — an empty list must not read as a failed load. | |
| 565 | private struct RetargetSheet: View { | |
| 566 | ||
| 567 | let model: MRDetailViewModel | |
| 568 | let onPick: (String) -> Void | |
| 569 | ||
| 570 | @Environment(\.dismiss) private var dismiss | |
| 571 | ||
| 572 | var body: some View { | |
| 573 | NavigationStack { | |
| 574 | Group { | |
| 575 | if let error = model.branchesError { | |
| 576 | ContentUnavailableView { | |
| 577 | Label("Could not load branches", systemImage: "wifi.exclamationmark") | |
| 578 | } description: { | |
| 579 | Text(error) | |
| 580 | } | |
| 581 | } else if let branches = model.branches { | |
| 582 | if branches.isEmpty { | |
| 583 | ContentUnavailableView { | |
| 584 | Label("No other branches", systemImage: "arrow.triangle.branch") | |
| 585 | } description: { | |
| 586 | Text("This repository has no other branches to retarget to.") | |
| 587 | } | |
| 588 | } else { | |
| 589 | List(branches, id: \.self) { branch in | |
| 590 | Button(branch) { onPick(branch) } | |
| 591 | } | |
| 592 | } | |
| 593 | } else { | |
| 594 | ProgressView() | |
| 595 | } | |
| 596 | } | |
| 597 | .navigationTitle("Retarget !\(model.number)") | |
| 598 | .navigationBarTitleDisplayMode(.inline) | |
| 599 | .toolbar { | |
| 600 | ToolbarItem(placement: .cancellationAction) { | |
| 601 | Button("Cancel") { dismiss() } | |
| 602 | } | |
| 603 | } | |
| 604 | .task { await model.loadBranches() } | |
| 605 | } | |
| 606 | } | |
| 607 | } | |
gitbayTests/MRLifecycleTests.swift added +247
| @@ -0,0 +1,247 @@ | ||
| 1 | import Foundation | |
| 2 | import Testing | |
| 3 | @testable import gitbay | |
| 4 | ||
| 5 | private func decodeDetail(_ json: String) throws -> MRDetail { | |
| 6 | let decoder = JSONDecoder() | |
| 7 | decoder.dateDecodingStrategy = .iso8601 | |
| 8 | return try decoder.decode(MRDetail.self, from: Data(json.utf8)) | |
| 9 | } | |
| 10 | ||
| 11 | private func decodeRow(_ json: String) throws -> MergeRequest { | |
| 12 | let decoder = JSONDecoder() | |
| 13 | decoder.dateDecodingStrategy = .iso8601 | |
| 14 | return try decoder.decode(MergeRequest.self, from: Data(json.utf8)) | |
| 15 | } | |
| 16 | ||
| 17 | private let plainDetail = """ | |
| 18 | {"number":7,"title":"a change","state":"open","author":"cmc",\ | |
| 19 | "source":"feat","target_ref":"main","head_sha":"abc123",\ | |
| 20 | "created_at":"2026-09-01T00:00:00Z"} | |
| 21 | """ | |
| 22 | ||
| 23 | struct MRLifecycleDecodingTests { | |
| 24 | ||
| 25 | /// Every one of these keys is omitempty. The ordinary merge request | |
| 26 | /// carries none of them, so absence must decode, not throw. | |
| 27 | @Test func anOrdinaryMergeRequestHasNoneOfTheNewKeys() throws { | |
| 28 | let mr = try decodeDetail(plainDetail) | |
| 29 | #expect(mr.draft == false) | |
| 30 | #expect(mr.reviewRequests.isEmpty) | |
| 31 | #expect(mr.stackedOn == nil) | |
| 32 | #expect(mr.stacked.isEmpty) | |
| 33 | } | |
| 34 | ||
| 35 | @Test func aDraftDecodesAndStaysOpen() throws { | |
| 36 | let mr = try decodeDetail(""" | |
| 37 | {"number":7,"title":"a change","state":"open","draft":true,"author":"cmc",\ | |
| 38 | "source":"feat","target_ref":"main","head_sha":"abc123",\ | |
| 39 | "created_at":"2026-09-01T00:00:00Z"} | |
| 40 | """) | |
| 41 | #expect(mr.draft) | |
| 42 | // Draft is a flag, not a fifth state. | |
| 43 | #expect(mr.state == "open") | |
| 44 | #expect(mr.isOpen) | |
| 45 | } | |
| 46 | ||
| 47 | @Test func reviewRequestsDecode() throws { | |
| 48 | let mr = try decodeDetail(""" | |
| 49 | {"number":7,"title":"a change","state":"open","author":"cmc",\ | |
| 50 | "review_requests":["rae","sam"],\ | |
| 51 | "source":"feat","target_ref":"main","head_sha":"abc123",\ | |
| 52 | "created_at":"2026-09-01T00:00:00Z"} | |
| 53 | """) | |
| 54 | #expect(mr.reviewRequests == ["rae", "sam"]) | |
| 55 | } | |
| 56 | ||
| 57 | @Test func bothHalvesOfAStackDecode() throws { | |
| 58 | let mr = try decodeDetail(""" | |
| 59 | {"number":7,"title":"a change","state":"open","author":"cmc",\ | |
| 60 | "stacked_on":{"number":6,"title":"the one below"},\ | |
| 61 | "stacked":[{"number":8,"title":"one above"},{"number":9,"title":"another"}],\ | |
| 62 | "source":"feat","target_ref":"main","head_sha":"abc123",\ | |
| 63 | "created_at":"2026-09-01T00:00:00Z"} | |
| 64 | """) | |
| 65 | #expect(mr.stackedOn?.number == 6) | |
| 66 | #expect(mr.stackedOn?.title == "the one below") | |
| 67 | #expect(mr.stacked.map(\.number) == [8, 9]) | |
| 68 | } | |
| 69 | ||
| 70 | @Test func aListRowCarriesDraftAndStackedOn() throws { | |
| 71 | let row = try decodeRow(""" | |
| 72 | {"number":7,"title":"a change","state":"open","draft":true,"author":"cmc",\ | |
| 73 | "stacked_on":{"number":6,"title":"below"},\ | |
| 74 | "source":"feat","target_ref":"main","head_sha":"abc123",\ | |
| 75 | "created_at":"2026-09-01T00:00:00Z"} | |
| 76 | """) | |
| 77 | #expect(row.draft) | |
| 78 | #expect(row.stackedOn?.number == 6) | |
| 79 | } | |
| 80 | ||
| 81 | @Test func aPlainListRowHasNeither() throws { | |
| 82 | let row = try decodeRow(""" | |
| 83 | {"number":7,"title":"a change","state":"open","author":"cmc",\ | |
| 84 | "source":"feat","target_ref":"main","head_sha":"abc123",\ | |
| 85 | "created_at":"2026-09-01T00:00:00Z"} | |
| 86 | """) | |
| 87 | #expect(row.draft == false) | |
| 88 | #expect(row.stackedOn == nil) | |
| 89 | } | |
| 90 | } | |
| 91 | ||
| 92 | @MainActor | |
| 93 | struct MRLifecycleActionTests { | |
| 94 | ||
| 95 | private func loaded() async throws -> (MRDetailViewModel, StubProtocol.Box) { | |
| 96 | let box = StubProtocol.box() | |
| 97 | let client = GitbayClient( | |
| 98 | instance: try GitbayInstance(url: "https://gitbay.org"), | |
| 99 | token: "test-token", | |
| 100 | session: box.session() | |
| 101 | ) | |
| 102 | box.enqueue(.init(status: 200, json: """ | |
| 103 | {"protocol_version":1,"data":\(plainDetail),"exit_code":0} | |
| 104 | """)) | |
| 105 | let model = MRDetailViewModel(client: client, repoPath: "krz/gitbay", number: 7) | |
| 106 | await model.load() | |
| 107 | return (model, box) | |
| 108 | } | |
| 109 | ||
| 110 | private func argvOf(_ seen: StubProtocol.Seen) throws -> [String] { | |
| 111 | let body = try #require(try JSONSerialization.jsonObject(with: seen.body) as? [String: Any]) | |
| 112 | return try #require(body["argv"] as? [String]) | |
| 113 | } | |
| 114 | ||
| 115 | private func ok(_ box: StubProtocol.Box) { | |
| 116 | box.enqueue(.init(status: 200, json: """ | |
| 117 | {"protocol_version":1,"exit_code":0} | |
| 118 | """)) | |
| 119 | box.enqueue(.init(status: 200, json: """ | |
| 120 | {"protocol_version":1,"data":\(plainDetail),"exit_code":0} | |
| 121 | """)) | |
| 122 | } | |
| 123 | ||
| 124 | /// `load()` issues three GETs: `mr show`, `mr diff`, `mr threads`. | |
| 125 | /// `loaded()` runs it once (3 GETs, the last two unstubbed and failed), | |
| 126 | /// so a reload after a write brings the total to 6. | |
| 127 | private func gets(_ box: StubProtocol.Box) -> Int { | |
| 128 | box.seen.count { $0.method == "GET" } | |
| 129 | } | |
| 130 | ||
| 131 | @Test func markingDraftAndReadyAreDifferentCommands() async throws { | |
| 132 | let (model, box) = try await loaded() | |
| 133 | ok(box) | |
| 134 | await model.setDraft(true) | |
| 135 | var write = try #require(box.seen.first { $0.method == "POST" }) | |
| 136 | #expect(try argvOf(write) == ["mr", "draft", "krz/gitbay", "7"]) | |
| 137 | #expect(gets(box) == 6) | |
| 138 | ||
| 139 | let (model2, box2) = try await loaded() | |
| 140 | ok(box2) | |
| 141 | await model2.setDraft(false) | |
| 142 | write = try #require(box2.seen.first { $0.method == "POST" }) | |
| 143 | #expect(try argvOf(write) == ["mr", "ready", "krz/gitbay", "7"]) | |
| 144 | #expect(gets(box2) == 6) | |
| 145 | } | |
| 146 | ||
| 147 | @Test func retargetPassesTheBranchPositionally() async throws { | |
| 148 | let (model, box) = try await loaded() | |
| 149 | ok(box) | |
| 150 | await model.retarget(to: "release") | |
| 151 | let write = try #require(box.seen.first { $0.method == "POST" }) | |
| 152 | #expect(try argvOf(write) == ["mr", "retarget", "krz/gitbay", "7", "release"]) | |
| 153 | #expect(gets(box) == 6) | |
| 154 | } | |
| 155 | ||
| 156 | /// `mr review request` is a THREE-word path. Two words posts a | |
| 157 | /// review instead — silently, with a 200. | |
| 158 | @Test func requestingAReviewUsesTheThreeWordPath() async throws { | |
| 159 | let (model, box) = try await loaded() | |
| 160 | ok(box) | |
| 161 | await model.requestReview(from: "rae") | |
| 162 | let write = try #require(box.seen.first { $0.method == "POST" }) | |
| 163 | #expect(try argvOf(write) | |
| 164 | == ["mr", "review", "request", "krz/gitbay", "7", "--add", "rae"]) | |
| 165 | #expect(gets(box) == 6) | |
| 166 | } | |
| 167 | ||
| 168 | @Test func removingAReviewRequestUsesRemoveNotAdd() async throws { | |
| 169 | let (model, box) = try await loaded() | |
| 170 | ok(box) | |
| 171 | await model.removeReviewRequest("rae") | |
| 172 | let write = try #require(box.seen.first { $0.method == "POST" }) | |
| 173 | #expect(try argvOf(write) | |
| 174 | == ["mr", "review", "request", "krz/gitbay", "7", "--remove", "rae"]) | |
| 175 | #expect(gets(box) == 6) | |
| 176 | } | |
| 177 | ||
| 178 | @Test func branchesComeFromRepoRefs() async throws { | |
| 179 | let (model, box) = try await loaded() | |
| 180 | box.enqueue(.init(status: 200, json: """ | |
| 181 | {"protocol_version":1,"data":{"branches":[{"name":"main","sha":"a"},\ | |
| 182 | {"name":"release","sha":"b"}],"tags":[]},"exit_code":0} | |
| 183 | """)) | |
| 184 | await model.loadBranches() | |
| 185 | #expect(model.branches == ["main", "release"]) | |
| 186 | } | |
| 187 | ||
| 188 | @Test func aRefusalSurfacesAndDoesNotReload() async throws { | |
| 189 | let (model, box) = try await loaded() | |
| 190 | box.enqueue(.init(status: 200, json: """ | |
| 191 | {"protocol_version":1,"error":"cannot retarget a merged merge request",\ | |
| 192 | "exit_code":1} | |
| 193 | """)) | |
| 194 | await model.retarget(to: "release") | |
| 195 | #expect(model.actionError?.isEmpty == false) | |
| 196 | #expect(model.working == false) | |
| 197 | // A refusal throws before `perform` reaches `load()`: still the | |
| 198 | // 3 GETs from `loaded()`, none from a reload. | |
| 199 | #expect(gets(box) == 3) | |
| 200 | } | |
| 201 | } | |
| 202 | ||
| 203 | @MainActor | |
| 204 | struct MRCreateDraftTests { | |
| 205 | ||
| 206 | @Test func creatingAsADraftAppendsTheFlag() async throws { | |
| 207 | let box = StubProtocol.box() | |
| 208 | let client = GitbayClient( | |
| 209 | instance: try GitbayInstance(url: "https://gitbay.org"), | |
| 210 | token: "test-token", | |
| 211 | session: box.session() | |
| 212 | ) | |
| 213 | box.enqueue(.init(status: 200, json: """ | |
| 214 | {"protocol_version":1,"data":{"number":7},"exit_code":0} | |
| 215 | """)) | |
| 216 | let model = MRCreateViewModel(client: client, repoPath: "krz/gitbay") | |
| 217 | model.draft = true | |
| 218 | _ = await model.create(source: "feat", target: "main", title: "a change", body: "") | |
| 219 | ||
| 220 | let write = try #require(box.seen.first { $0.method == "POST" }) | |
| 221 | let body = try #require( | |
| 222 | try JSONSerialization.jsonObject(with: write.body) as? [String: Any] | |
| 223 | ) | |
| 224 | let argv = try #require(body["argv"] as? [String]) | |
| 225 | #expect(argv.contains("--draft")) | |
| 226 | } | |
| 227 | ||
| 228 | @Test func creatingWithoutDraftOmitsTheFlag() async throws { | |
| 229 | let box = StubProtocol.box() | |
| 230 | let client = GitbayClient( | |
| 231 | instance: try GitbayInstance(url: "https://gitbay.org"), | |
| 232 | token: "test-token", | |
| 233 | session: box.session() | |
| 234 | ) | |
| 235 | box.enqueue(.init(status: 200, json: """ | |
| 236 | {"protocol_version":1,"data":{"number":7},"exit_code":0} | |
| 237 | """)) | |
| 238 | let model = MRCreateViewModel(client: client, repoPath: "krz/gitbay") | |
| 239 | _ = await model.create(source: "feat", target: "main", title: "a change", body: "") | |
| 240 | ||
| 241 | let write = try #require(box.seen.first { $0.method == "POST" }) | |
| 242 | let body = try #require( | |
| 243 | try JSONSerialization.jsonObject(with: write.body) as? [String: Any] | |
| 244 | ) | |
| 245 | #expect((body["argv"] as? [String])?.contains("--draft") == false) | |
| 246 | } | |
| 247 | } | |