Commit 31e5f22879
Verified · cmc
Layout: unified · split
gitbay/MRs/MRModels.swift +27
| @@ -37,6 +37,12 @@ nonisolated struct MRDetail: Decodable, Sendable, Hashable { | |||
| 37 | let body: String? | 37 | let body: String? |
| 38 | let milestone: String? | 38 | let milestone: String? |
| 39 | let createdAt: Date | 39 | let createdAt: Date |
| 40 | /// Set once the merge request is resolved. The actor is absent when | ||
| 41 | /// the account is gone, or the row was imported. | ||
| 42 | let mergedAt: Date? | ||
| 43 | let mergedBy: String? | ||
| 44 | let closedAt: Date? | ||
| 45 | let closedBy: String? | ||
| 40 | let checks: [Check]? | 46 | let checks: [Check]? |
| 41 | let checksCombined: String? | 47 | let checksCombined: String? |
| 42 | let unresolvedThreads: Int? | 48 | let unresolvedThreads: Int? |
| @@ -49,6 +55,10 @@ nonisolated struct MRDetail: Decodable, Sendable, Hashable { | |||
| 49 | case targetRef = "target_ref" | 55 | case targetRef = "target_ref" |
| 50 | case headSHA = "head_sha" | 56 | case headSHA = "head_sha" |
| 51 | case createdAt = "created_at" | 57 | case createdAt = "created_at" |
| 58 | case mergedAt = "merged_at" | ||
| 59 | case mergedBy = "merged_by" | ||
| 60 | case closedAt = "closed_at" | ||
| 61 | case closedBy = "closed_by" | ||
| 52 | case checksCombined = "checks_combined" | 62 | case checksCombined = "checks_combined" |
| 53 | case unresolvedThreads = "unresolved_threads" | 63 | case unresolvedThreads = "unresolved_threads" |
| 54 | } | 64 | } |
| @@ -59,6 +69,16 @@ nonisolated struct MRDetail: Decodable, Sendable, Hashable { | |||
| 59 | let context: String | 69 | let context: String |
| 60 | let state: String | 70 | let state: String |
| 61 | let url: String? | 71 | let url: String? |
| 72 | /// When the check last changed state. | ||
| 73 | let updatedAt: Date? | ||
| 74 | /// How long the build behind the check ran, as the server | ||
| 75 | /// formats it ("1m12s"). Only CI checks have a build to time. | ||
| 76 | let duration: String? | ||
| 77 | |||
| 78 | enum CodingKeys: String, CodingKey { | ||
| 79 | case context, state, url, duration | ||
| 80 | case updatedAt = "updated_at" | ||
| 81 | } | ||
| 62 | } | 82 | } |
| 63 | 83 | ||
| 64 | nonisolated struct MRCommit: Decodable, Sendable, Hashable, Identifiable { | 84 | nonisolated struct MRCommit: Decodable, Sendable, Hashable, Identifiable { |
| @@ -86,6 +106,13 @@ nonisolated struct MRDetail: Decodable, Sendable, Hashable { | |||
| 86 | let verdict: String | 106 | let verdict: String |
| 87 | /// The review predates the current head — it approved older code. | 107 | /// The review predates the current head — it approved older code. |
| 88 | let stale: Bool | 108 | let stale: Bool |
| 109 | let createdAt: Date? | ||
| 110 | |||
| 111 | enum CodingKeys: String, CodingKey { | ||
| 112 | case reviewer, verdict, stale | ||
| 113 | case createdAt = "created_at" | ||
| 114 | } | ||
| 115 | |||
| 89 | var id: String { reviewer } | 116 | var id: String { reviewer } |
| 90 | } | 117 | } |
| 91 | } | 118 | } |
gitbay/Views/MRs/MRView.swift +69 −18
| @@ -113,11 +113,38 @@ struct MRView: View { | |||
| 113 | } | 113 | } |
| 114 | .font(.gbSans(.caption)) | 114 | .font(.gbSans(.caption)) |
| 115 | .foregroundStyle(.secondary) | 115 | .foregroundStyle(.secondary) |
| 116 | resolution(mr) | ||
| 116 | } | 117 | } |
| 117 | .padding(.vertical, 2) | 118 | .padding(.vertical, 2) |
| 118 | } | 119 | } |
| 119 | } | 120 | } |
| 120 | 121 | ||
| 122 | /// What became of the merge request, once it is no longer open. The | ||
| 123 | /// actor is absent for imported rows, which carry only a time. | ||
| 124 | @ViewBuilder | ||
| 125 | private func resolution(_ mr: MRDetail) -> some View { | ||
| 126 | if let at = mr.mergedAt { | ||
| 127 | stampLine("merged", by: mr.mergedBy, at: at) | ||
| 128 | } else if let at = mr.closedAt { | ||
| 129 | stampLine("closed without merging", by: mr.closedBy, at: at) | ||
| 130 | } | ||
| 131 | } | ||
| 132 | |||
| 133 | private func stampLine(_ verb: String, by actor: String?, at: Date) -> some View { | ||
| 134 | HStack(spacing: 6) { | ||
| 135 | Text(actor.map { "\(verb) by \($0)" } ?? verb) | ||
| 136 | Text(at, format: Self.stamp) | ||
| 137 | .foregroundStyle(.tertiary) | ||
| 138 | } | ||
| 139 | .font(.gbSans(.caption)) | ||
| 140 | .foregroundStyle(.secondary) | ||
| 141 | } | ||
| 142 | |||
| 143 | /// Reviews and checks are read for when they happened, so they carry | ||
| 144 | /// the date rather than "3 days ago". | ||
| 145 | private static let stamp = Date.FormatStyle.dateTime | ||
| 146 | .year().month(.abbreviated).day().hour().minute() | ||
| 147 | |||
| 121 | private func milestoneSection(_ mr: MRDetail) -> some View { | 148 | private func milestoneSection(_ mr: MRDetail) -> some View { |
| 122 | Section("Milestone") { | 149 | Section("Milestone") { |
| 123 | Menu { | 150 | Menu { |
| @@ -179,15 +206,18 @@ struct MRView: View { | |||
| 179 | private func checksSection(_ checks: [MRDetail.Check], combined: String?) -> some View { | 206 | private func checksSection(_ checks: [MRDetail.Check], combined: String?) -> some View { |
| 180 | Section("Checks" + (combined.map { " — \($0)" } ?? "")) { | 207 | Section("Checks" + (combined.map { " — \($0)" } ?? "")) { |
| 181 | ForEach(checks, id: \.context) { check in | 208 | ForEach(checks, id: \.context) { check in |
| 182 | HStack { | 209 | VStack(alignment: .leading, spacing: 2) { |
| 183 | Image(systemName: checkIcon(check.state)) | 210 | HStack { |
| 184 | .foregroundStyle(checkColor(check.state)) | 211 | Image(systemName: checkIcon(check.state)) |
| 185 | Text(check.context) | 212 | .foregroundStyle(checkColor(check.state)) |
| 186 | .font(.gbSans(.subheadline)) | 213 | Text(check.context) |
| 187 | Spacer() | 214 | .font(.gbSans(.subheadline)) |
| 188 | Text(check.state) | 215 | Spacer() |
| 189 | .font(.gbSans(.caption)) | 216 | Text(check.state) |
| 190 | .foregroundStyle(.secondary) | 217 | .font(.gbSans(.caption)) |
| 218 | .foregroundStyle(.secondary) | ||
| 219 | } | ||
| 220 | timing(check.updatedAt, duration: check.duration) | ||
| 191 | } | 221 | } |
| 192 | } | 222 | } |
| 193 | } | 223 | } |
| @@ -196,18 +226,39 @@ struct MRView: View { | |||
| 196 | private func reviewsSection(_ reviews: [MRDetail.Review]) -> some View { | 226 | private func reviewsSection(_ reviews: [MRDetail.Review]) -> some View { |
| 197 | Section("Reviews") { | 227 | Section("Reviews") { |
| 198 | ForEach(reviews) { review in | 228 | ForEach(reviews) { review in |
| 199 | HStack { | 229 | VStack(alignment: .leading, spacing: 2) { |
| 200 | Image(systemName: review.verdict == "approve" | 230 | HStack { |
| 201 | ? "checkmark.circle.fill" : "exclamationmark.circle.fill") | 231 | Image(systemName: review.verdict == "approve" |
| 202 | .foregroundStyle(review.verdict == "approve" ? Color.gbOK : Color.gbWarn) | 232 | ? "checkmark.circle.fill" : "exclamationmark.circle.fill") |
| 203 | Text(review.reviewer) | 233 | .foregroundStyle(review.verdict == "approve" ? Color.gbOK : Color.gbWarn) |
| 204 | .font(.gbSans(.subheadline)) | 234 | Text(review.reviewer) |
| 205 | Spacer() | 235 | .font(.gbSans(.subheadline)) |
| 206 | if review.stale { | 236 | Spacer() |
| 207 | GBChip("stale", .gbWarn) | 237 | if review.stale { |
| 238 | GBChip("stale", .gbWarn) | ||
| 239 | } | ||
| 208 | } | 240 | } |
| 241 | timing(review.createdAt) | ||
| 242 | } | ||
| 243 | } | ||
| 244 | } | ||
| 245 | } | ||
| 246 | |||
| 247 | /// When a review or check last said something, and for a CI check how | ||
| 248 | /// long its build ran. Nothing renders against a server that does not | ||
| 249 | /// report the time. | ||
| 250 | @ViewBuilder | ||
| 251 | private func timing(_ at: Date?, duration: String? = nil) -> some View { | ||
| 252 | if let at { | ||
| 253 | HStack(spacing: 6) { | ||
| 254 | Text(at, format: Self.stamp) | ||
| 255 | if let duration { | ||
| 256 | Text("·") | ||
| 257 | Text(duration) | ||
| 209 | } | 258 | } |
| 210 | } | 259 | } |
| 260 | .font(.gbSans(.caption)) | ||
| 261 | .foregroundStyle(.tertiary) | ||
| 211 | } | 262 | } |
| 212 | } | 263 | } |
| 213 | 264 | ||
gitbayTests/MRViewModelTests.swift +49 −2
| @@ -27,11 +27,21 @@ private let mrShowJSON = """ | |||
| 27 | {"protocol_version":1,"data":{"number":7,"title":"client: envelope decoding",\ | 27 | {"protocol_version":1,"data":{"number":7,"title":"client: envelope decoding",\ |
| 28 | "state":"open","author":"cmc","source":"client-envelope","target_ref":"main",\ | 28 | "state":"open","author":"cmc","source":"client-envelope","target_ref":"main",\ |
| 29 | "head_sha":"aabbcc","body":"Speaks both surfaces.","created_at":"2026-08-20T10:00:00.000Z",\ | 29 | "head_sha":"aabbcc","body":"Speaks both surfaces.","created_at":"2026-08-20T10:00:00.000Z",\ |
| 30 | "checks":[{"context":"build","state":"success"}],"checks_combined":"success",\ | 30 | "checks":[{"context":"ci/build","state":"success","updated_at":"2026-08-20T10:05:00.000Z",\ |
| 31 | "duration":"1m12s"},{"context":"external/lint","state":"success",\ | ||
| 32 | "updated_at":"2026-08-20T10:06:00.000Z"}],"checks_combined":"success",\ | ||
| 31 | "unresolved_threads":1,\ | 33 | "unresolved_threads":1,\ |
| 32 | "commits":[{"sha":"aabbcc00000000000000","subject":"client: envelope"}],\ | 34 | "commits":[{"sha":"aabbcc00000000000000","subject":"client: envelope"}],\ |
| 33 | "comments":[{"author":"krz","body":"looks right","created_at":"2026-08-20T11:00:00.000Z"}],\ | 35 | "comments":[{"author":"krz","body":"looks right","created_at":"2026-08-20T11:00:00.000Z"}],\ |
| 34 | "reviews":[{"reviewer":"krz","verdict":"approve","stale":false}]},"exit_code":0} | 36 | "reviews":[{"reviewer":"krz","verdict":"approve","stale":false,\ |
| 37 | "created_at":"2026-08-20T10:30:00.000Z"}]},"exit_code":0} | ||
| 38 | """ | ||
| 39 | |||
| 40 | private let mrMergedJSON = """ | ||
| 41 | {"protocol_version":1,"data":{"number":7,"title":"client: envelope decoding",\ | ||
| 42 | "state":"merged","author":"cmc","source":"client-envelope","target_ref":"main",\ | ||
| 43 | "head_sha":"aabbcc","created_at":"2026-08-20T10:00:00.000Z",\ | ||
| 44 | "merged_at":"2026-08-21T09:15:00.000Z","merged_by":"cmc"},"exit_code":0} | ||
| 35 | """ | 45 | """ |
| 36 | 46 | ||
| 37 | private let threadsJSON = """ | 47 | private let threadsJSON = """ |
| @@ -194,11 +204,48 @@ struct MRDetailViewModelTests { | |||
| 194 | #expect(mr.title == "client: envelope decoding") | 204 | #expect(mr.title == "client: envelope decoding") |
| 195 | #expect(mr.checksCombined == "success") | 205 | #expect(mr.checksCombined == "success") |
| 196 | #expect(mr.reviews?.first?.verdict == "approve") | 206 | #expect(mr.reviews?.first?.verdict == "approve") |
| 207 | #expect(mr.reviews?.first?.createdAt != nil) | ||
| 197 | #expect(model.diff?.files.count == 2) | 208 | #expect(model.diff?.files.count == 2) |
| 198 | #expect(model.threads.count == 2) | 209 | #expect(model.threads.count == 2) |
| 199 | #expect(model.unresolvedCount == 1) | 210 | #expect(model.unresolvedCount == 1) |
| 200 | } | 211 | } |
| 201 | 212 | ||
| 213 | /// A CI check reports how long its build ran; anything else reports | ||
| 214 | /// only when it last changed. Neither may borrow the other's timing. | ||
| 215 | @Test func checksCarryTheirTiming() async throws { | ||
| 216 | let (model, _) = try await loadedModel() | ||
| 217 | |||
| 218 | let checks = try #require(model.state.value?.checks) | ||
| 219 | let ci = try #require(checks.first { $0.context == "ci/build" }) | ||
| 220 | #expect(ci.duration == "1m12s") | ||
| 221 | #expect(ci.updatedAt != nil) | ||
| 222 | let external = try #require(checks.first { $0.context == "external/lint" }) | ||
| 223 | #expect(external.duration == nil) | ||
| 224 | #expect(external.updatedAt != nil) | ||
| 225 | } | ||
| 226 | |||
| 227 | /// A merged MR says who merged it and when. An open one has no stamp | ||
| 228 | /// to show, and must not invent one. | ||
| 229 | @Test func resolutionStampsDecode() async throws { | ||
| 230 | let (client, stub) = try makeClient() | ||
| 231 | stub.enqueue(.init(status: 200, json: mrShowJSON, match: "argv=show")) | ||
| 232 | stub.enqueue(.init(status: 200, json: diffEnvelope(), match: "argv=diff")) | ||
| 233 | stub.enqueue(.init(status: 200, json: threadsJSON, match: "argv=threads")) | ||
| 234 | let open = MRDetailViewModel(client: client, repoPath: "krz/gitbay", number: 7) | ||
| 235 | await open.load() | ||
| 236 | #expect(open.state.value?.mergedAt == nil) | ||
| 237 | #expect(open.state.value?.closedAt == nil) | ||
| 238 | |||
| 239 | let (mergedClient, mergedStub) = try makeClient() | ||
| 240 | mergedStub.enqueue(.init(status: 200, json: mrMergedJSON, match: "argv=show")) | ||
| 241 | mergedStub.enqueue(.init(status: 200, json: diffEnvelope(), match: "argv=diff")) | ||
| 242 | mergedStub.enqueue(.init(status: 200, json: threadsJSON, match: "argv=threads")) | ||
| 243 | let merged = MRDetailViewModel(client: mergedClient, repoPath: "krz/gitbay", number: 7) | ||
| 244 | await merged.load() | ||
| 245 | #expect(merged.state.value?.mergedBy == "cmc") | ||
| 246 | #expect(merged.state.value?.mergedAt != nil) | ||
| 247 | } | ||
| 248 | |||
| 202 | @Test func approveSendsTheWriteThenReloads() async throws { | 249 | @Test func approveSendsTheWriteThenReloads() async throws { |
| 203 | let (model, stub) = try await loadedModel() | 250 | let (model, stub) = try await loadedModel() |
| 204 | stub.enqueue(.init(status: 200, json: #"{"protocol_version":1,"data":{},"exit_code":0}"#)) | 251 | stub.enqueue(.init(status: 200, json: #"{"protocol_version":1,"data":{},"exit_code":0}"#)) |