Commit 951501ccb2
951501ccb2e1f826d7b4573c7b6bce1b1582d600
parent: 77cd5ece16
Verified · cmc
cmc <hello@cleberg.net> · 2026-09-07 04:10 UTC
Fix review findings: cheap post-action reload, tautological and last-request test cleanup
RepoDetailViewModel gains reloadDetail(), re-reading repo show only
with try? so a transient failure leaves the last good state in place.
RepoView's bookmark/watch/mute actions call it instead of the full
load() (repo show + dashboard + tree + cat), cutting each action's
request count from 5 to 2. perform() now reuses reloadDetail() instead
of duplicating the read.
Removed noStateOffersAReturnToTheDefault, which held for any WatchAction
implementation regardless of correctness. Fixed
bookmarkingAndUnbookmarkingAreDifferentCommands to identify requests by
argv instead of stub.seen.last, and to assert exactly one request per
action.
Layout: unified · split
gitbay/Repos/RepoDetailViewModel.swift
+10 −3
| @@ -44,6 +44,15 @@ final class RepoDetailViewModel { |
| 44 | isPinned = dashboard.pinned.contains { $0.path == path } |
44 | isPinned = dashboard.pinned.contains { $0.path == path } |
| 45 | } |
45 | } |
| 46 | |
46 | |
| |
47 | /// Re-reads `repo show` only, leaving the last good state in place on |
| |
48 | /// failure. For refreshing after an action performed elsewhere |
| |
49 | /// (bookmark, watch, mute) without paying for a full screen load. |
| |
50 | func reloadDetail() async { |
| |
51 | if let detail = try? await client.read(["repo", "show", path], as: RepoDetail.self) { |
| |
52 | state = .loaded(detail) |
| |
53 | } |
| |
54 | } |
| |
55 | |
| 47 | // MARK: - Management actions |
56 | // MARK: - Management actions |
| 48 | |
57 | |
| 49 | func setPinned(_ pinned: Bool) async { |
58 | func setPinned(_ pinned: Bool) async { |
| @@ -61,9 +70,7 @@ final class RepoDetailViewModel { |
| 61 | defer { working = false } |
70 | defer { working = false } |
| 62 | do { |
71 | do { |
| 63 | try await client.run(argv) |
72 | try await client.run(argv) |
| 64 | if let detail = try? await client.read(["repo", "show", path], as: RepoDetail.self) { |
73 | await reloadDetail() |
| 65 | state = .loaded(detail) |
| |
| 66 | } |
| |
| 67 | } catch let error as GitbayError { |
74 | } catch let error as GitbayError { |
| 68 | actionError = error.userFacingMessage |
75 | actionError = error.userFacingMessage |
| 69 | } catch { |
76 | } catch { |
gitbay/Views/Repos/RepoView.swift
+3 −3
| @@ -152,7 +152,7 @@ struct RepoView: View { |
| 152 | Button { |
152 | Button { |
| 153 | Task { |
153 | Task { |
| 154 | await actionsModel.setBookmarked(!detail.bookmarked) |
154 | await actionsModel.setBookmarked(!detail.bookmarked) |
| 155 | await model.load() |
155 | await model.reloadDetail() |
| 156 | } |
156 | } |
| 157 | } label: { |
157 | } label: { |
| 158 | Label(detail.bookmarked ? "Unbookmark" : "Bookmark", |
158 | Label(detail.bookmarked ? "Unbookmark" : "Bookmark", |
| @@ -173,7 +173,7 @@ struct RepoView: View { |
| 173 | Button { |
173 | Button { |
| 174 | Task { |
174 | Task { |
| 175 | await actionsModel.watch() |
175 | await actionsModel.watch() |
| 176 | await model.load() |
176 | await model.reloadDetail() |
| 177 | } |
177 | } |
| 178 | } label: { |
178 | } label: { |
| 179 | Label("Watch this repository", systemImage: "eye") |
179 | Label("Watch this repository", systemImage: "eye") |
| @@ -183,7 +183,7 @@ struct RepoView: View { |
| 183 | Button { |
183 | Button { |
| 184 | Task { |
184 | Task { |
| 185 | await actionsModel.unwatch() |
185 | await actionsModel.unwatch() |
| 186 | await model.load() |
186 | await model.reloadDetail() |
| 187 | } |
187 | } |
| 188 | } label: { |
188 | } label: { |
| 189 | Label("Mute this repository", systemImage: "bell.slash") |
189 | Label("Mute this repository", systemImage: "bell.slash") |
gitbayTests/RepoActionTests.swift
+5 −2
| @@ -30,12 +30,15 @@ struct RepoActionsTests { |
| 30 | |
30 | |
| 31 | stub.enqueue(.init(status: 200, json: ok)) |
31 | stub.enqueue(.init(status: 200, json: ok)) |
| 32 | await model.setBookmarked(false) |
32 | await model.setBookmarked(false) |
| 33 | #expect(try argvOf(try #require(stub.seen.last)) == ["repo", "unbookmark", "krz/gitbay"]) |
33 | #expect(stub.seen.contains { (try? argvOf($0)) == ["repo", "unbookmark", "krz/gitbay"] }) |
| |
34 | #expect(stub.seen.count == 1) |
| 34 | |
35 | |
| 35 | let model2 = RepoActionsViewModel(client: client, repoPath: "krz/other") |
36 | let model2 = RepoActionsViewModel(client: client, repoPath: "krz/other") |
| |
37 | let before = stub.seen.count |
| 36 | stub.enqueue(.init(status: 200, json: ok)) |
38 | stub.enqueue(.init(status: 200, json: ok)) |
| 37 | await model2.setBookmarked(true) |
39 | await model2.setBookmarked(true) |
| 38 | #expect(try argvOf(try #require(stub.seen.last)) == ["repo", "bookmark", "krz/other"]) |
40 | #expect(stub.seen.contains { (try? argvOf($0)) == ["repo", "bookmark", "krz/other"] }) |
| |
41 | #expect(stub.seen.count == before + 1) |
| 39 | } |
42 | } |
| 40 | |
43 | |
| 41 | @Test func watchAndUnwatchSendTheirOwnCommands() async throws { |
44 | @Test func watchAndUnwatchSendTheirOwnCommands() async throws { |
gitbayTests/RepoStateTests.swift
−6
| @@ -53,10 +53,4 @@ struct WatchStateTests { |
| 53 | #expect(WatchState.muted.available == [.watch]) |
53 | #expect(WatchState.muted.available == [.watch]) |
| 54 | #expect(WatchState.default.available == [.watch, .mute]) |
54 | #expect(WatchState.default.available == [.watch, .mute]) |
| 55 | } |
55 | } |
| 56 | |
| |
| 57 | @Test func noStateOffersAReturnToTheDefault() { |
| |
| 58 | for state in [WatchState.watching, .muted, .default] { |
| |
| 59 | #expect(state.available.allSatisfy { $0 == .watch || $0 == .mute }) |
| |
| 60 | } |
| |
| 61 | } |
| |
| 62 | } |
56 | } |