Commit 1f32ceb67c
Verified · cmc
Layout: unified · split
gitbay/Repos/RepoActionsViewModel.swift +7 −25
| @@ -1,16 +1,14 @@ | |||
| 1 | import Foundation | 1 | import Foundation |
| 2 | import Observation | 2 | import Observation |
| 3 | 3 | ||
| 4 | /// Fork, watch, mute and bookmark — the repository-level actions the | 4 | /// Fork, watch, mute and bookmark — repository-level actions performed |
| 5 | /// server exposes but does not describe fully back: | 5 | /// against a `RepoDetail` already loaded elsewhere: |
| 6 | /// | 6 | /// |
| 7 | /// - Watch/mute state is reported by no command at all (krz/gitbay#178), | 7 | /// - Watch/mute/bookmark state now comes back on `repo show` |
| 8 | /// so `watch()`/`unwatch()` only send their command and report the | 8 | /// (`RepoDetail.watch`/`.bookmarked`), so this view model only sends |
| 9 | /// result; they never track or infer a current state. | 9 | /// the write commands; the caller rereads `repo show` to see the result. |
| 10 | /// - Bookmark state has to be derived by listing `repo bookmarks` and | 10 | /// - `fork_of` is also on `repo fork`'s own response, which is why `fork` |
| 11 | /// testing membership — there is no per-repo "is this bookmarked" call. | 11 | /// hands its result back to the caller directly. |
| 12 | /// - `fork_of` is only on `repo fork`'s own response, never on `repo | ||
| 13 | /// show`, which is why `fork` hands its result back to the caller. | ||
| 14 | @Observable | 12 | @Observable |
| 15 | @MainActor | 13 | @MainActor |
| 16 | final class RepoActionsViewModel { | 14 | final class RepoActionsViewModel { |
| @@ -26,9 +24,6 @@ final class RepoActionsViewModel { | |||
| 26 | } | 24 | } |
| 27 | } | 25 | } |
| 28 | 26 | ||
| 29 | /// `nil` until known or after a failed load. Never `false` for "could | ||
| 30 | /// not check" — that would draw the wrong control. | ||
| 31 | private(set) var isBookmarked: Bool? | ||
| 32 | private(set) var actionError: String? | 27 | private(set) var actionError: String? |
| 33 | private(set) var notice: String? | 28 | private(set) var notice: String? |
| 34 | private(set) var working = false | 29 | private(set) var working = false |
| @@ -43,21 +38,8 @@ final class RepoActionsViewModel { | |||
| 43 | 38 | ||
| 44 | // MARK: - Bookmarks | 39 | // MARK: - Bookmarks |
| 45 | 40 | ||
| 46 | /// `repo bookmarks` — plural, no repository argument. Membership in | ||
| 47 | /// the listing is the only signal the server gives for this repo. | ||
| 48 | func loadBookmarkState() async { | ||
| 49 | do { | ||
| 50 | let bookmarks = try await client.readList(["repo", "bookmarks"], of: RepoSummary.self) | ||
| 51 | isBookmarked = bookmarks.contains { $0.path == repoPath } | ||
| 52 | } catch { | ||
| 53 | // "We don't know" is not "not bookmarked". | ||
| 54 | isBookmarked = nil | ||
| 55 | } | ||
| 56 | } | ||
| 57 | |||
| 58 | func setBookmarked(_ bookmarked: Bool) async { | 41 | func setBookmarked(_ bookmarked: Bool) async { |
| 59 | await perform(["repo", bookmarked ? "bookmark" : "unbookmark", repoPath]) | 42 | await perform(["repo", bookmarked ? "bookmark" : "unbookmark", repoPath]) |
| 60 | await loadBookmarkState() | ||
| 61 | } | 43 | } |
| 62 | 44 | ||
| 63 | // MARK: - Watch / mute | 45 | // MARK: - Watch / mute |
gitbay/Repos/RepoModels.swift +60 −1
| @@ -28,16 +28,75 @@ nonisolated struct RepoDetail: Decodable, Sendable, Hashable { | |||
| 28 | let protectedBranches: [String]? | 28 | let protectedBranches: [String]? |
| 29 | let archived: Bool? | 29 | let archived: Bool? |
| 30 | let topics: [String]? | 30 | let topics: [String]? |
| 31 | /// The caller's own watch state: `"watching"`, `"muted"`, or absent | ||
| 32 | /// for the default (neither). | ||
| 33 | let watch: String? | ||
| 34 | /// `omitempty` on a Bool, so absent means false, not unknown. | ||
| 35 | let bookmarked: Bool | ||
| 36 | /// Absent means either not a fork, or a fork whose parent the caller | ||
| 37 | /// cannot read — the server withholds it rather than confirm a | ||
| 38 | /// private parent exists. Nothing further may be inferred from nil. | ||
| 39 | let forkOf: String? | ||
| 31 | 40 | ||
| 32 | enum CodingKeys: String, CodingKey { | 41 | enum CodingKeys: String, CodingKey { |
| 33 | case path, description, website, visibility, archived, topics | 42 | case path, description, website, visibility, archived, topics, watch, bookmarked |
| 34 | case defaultBranch = "default_branch" | 43 | case defaultBranch = "default_branch" |
| 35 | case protectedBranches = "protected_branches" | 44 | case protectedBranches = "protected_branches" |
| 45 | case forkOf = "fork_of" | ||
| 46 | } | ||
| 47 | |||
| 48 | init(from decoder: any Decoder) throws { | ||
| 49 | let container = try decoder.container(keyedBy: CodingKeys.self) | ||
| 50 | path = try container.decode(String.self, forKey: .path) | ||
| 51 | description = try container.decodeIfPresent(String.self, forKey: .description) | ||
| 52 | website = try container.decodeIfPresent(String.self, forKey: .website) | ||
| 53 | visibility = try container.decode(String.self, forKey: .visibility) | ||
| 54 | defaultBranch = try container.decode(String.self, forKey: .defaultBranch) | ||
| 55 | protectedBranches = try container.decodeIfPresent([String].self, forKey: .protectedBranches) | ||
| 56 | archived = try container.decodeIfPresent(Bool.self, forKey: .archived) | ||
| 57 | topics = try container.decodeIfPresent([String].self, forKey: .topics) | ||
| 58 | watch = try container.decodeIfPresent(String.self, forKey: .watch) | ||
| 59 | bookmarked = try container.decodeIfPresent(Bool.self, forKey: .bookmarked) ?? false | ||
| 60 | forkOf = try container.decodeIfPresent(String.self, forKey: .forkOf) | ||
| 36 | } | 61 | } |
| 37 | 62 | ||
| 38 | var isArchived: Bool { archived ?? false } | 63 | var isArchived: Bool { archived ?? false } |
| 39 | } | 64 | } |
| 40 | 65 | ||
| 66 | /// The caller's watch/mute state on a repository, from `RepoDetail.watch`. | ||
| 67 | /// Only two transitions are reachable from the server: `repo watch` sets | ||
| 68 | /// `.watching`, `repo unwatch` sets `.muted`. Nothing sets `.default` — | ||
| 69 | /// it is only ever the starting state — so `available` never offers it. | ||
| 70 | nonisolated enum WatchState: Sendable, Hashable { | ||
| 71 | case watching | ||
| 72 | case muted | ||
| 73 | case `default` | ||
| 74 | |||
| 75 | /// Unrecognized raw values (a future server addition) fall back to | ||
| 76 | /// `.default` rather than crashing or masquerading as watching. | ||
| 77 | init(_ raw: String?) { | ||
| 78 | switch raw { | ||
| 79 | case "watching": self = .watching | ||
| 80 | case "muted": self = .muted | ||
| 81 | default: self = .default | ||
| 82 | } | ||
| 83 | } | ||
| 84 | |||
| 85 | var available: [WatchAction] { | ||
| 86 | switch self { | ||
| 87 | case .watching: [.mute] | ||
| 88 | case .muted: [.watch] | ||
| 89 | case .default: [.watch, .mute] | ||
| 90 | } | ||
| 91 | } | ||
| 92 | } | ||
| 93 | |||
| 94 | /// A transition `WatchState.available` may offer. | ||
| 95 | nonisolated enum WatchAction: Sendable, Hashable { | ||
| 96 | case watch | ||
| 97 | case mute | ||
| 98 | } | ||
| 99 | |||
| 41 | /// `repo refs <owner/name>` — the branches and tags the server knows. | 100 | /// `repo refs <owner/name>` — the branches and tags the server knows. |
| 42 | nonisolated struct RepoRefs: Decodable, Sendable, Hashable { | 101 | nonisolated struct RepoRefs: Decodable, Sendable, Hashable { |
| 43 | let branches: [RepoRef] | 102 | let branches: [RepoRef] |
gitbay/Views/Repos/RepoView.swift +8 −17
| @@ -101,11 +101,7 @@ struct RepoView: View { | |||
| 101 | .navigationBarTitleDisplayMode(.inline) | 101 | .navigationBarTitleDisplayMode(.inline) |
| 102 | .toolbar { toolbar } | 102 | .toolbar { toolbar } |
| 103 | .task { await model.load() } | 103 | .task { await model.load() } |
| 104 | .task { await actionsModel.loadBookmarkState() } | 104 | .refreshable { await model.load() } |
| 105 | .refreshable { | ||
| 106 | await model.load() | ||
| 107 | await actionsModel.loadBookmarkState() | ||
| 108 | } | ||
| 109 | .navigationDestination(item: $forkDestination) { route in | 105 | .navigationDestination(item: $forkDestination) { route in |
| 110 | if case .repo(let forkedPath) = route { | 106 | if case .repo(let forkedPath) = route { |
| 111 | RepoView(client: client, path: forkedPath) | 107 | RepoView(client: client, path: forkedPath) |
| @@ -153,19 +149,14 @@ struct RepoView: View { | |||
| 153 | systemImage: pinned ? "pin.slash" : "pin") | 149 | systemImage: pinned ? "pin.slash" : "pin") |
| 154 | } | 150 | } |
| 155 | } | 151 | } |
| 156 | if let bookmarked = actionsModel.isBookmarked { | 152 | Button { |
| 157 | Button { | 153 | Task { |
| 158 | Task { await actionsModel.setBookmarked(!bookmarked) } | 154 | await actionsModel.setBookmarked(!detail.bookmarked) |
| 159 | } label: { | 155 | await model.load() |
| 160 | Label(bookmarked ? "Unbookmark" : "Bookmark", | ||
| 161 | systemImage: bookmarked ? "bookmark.slash" : "bookmark") | ||
| 162 | } | ||
| 163 | } else { | ||
| 164 | Label { | ||
| 165 | Text("Checking Bookmark…") | ||
| 166 | } icon: { | ||
| 167 | ProgressView() | ||
| 168 | } | 156 | } |
| 157 | } label: { | ||
| 158 | Label(detail.bookmarked ? "Unbookmark" : "Bookmark", | ||
| 159 | systemImage: detail.bookmarked ? "bookmark.slash" : "bookmark") | ||
| 169 | } | 160 | } |
| 170 | Divider() | 161 | Divider() |
| 171 | Section { | 162 | Section { |
gitbayTests/RepoActionTests.swift +5 −83
| @@ -12,20 +12,11 @@ private func makeClient() throws -> (GitbayClient, StubProtocol.Box) { | |||
| 12 | return (client, box) | 12 | return (client, box) |
| 13 | } | 13 | } |
| 14 | 14 | ||
| 15 | private func argvFrom(_ url: URL) -> [String] { | ||
| 16 | URLComponents(url: url, resolvingAgainstBaseURL: false)? | ||
| 17 | .queryItems?.filter { $0.name == "argv" }.compactMap(\.value) ?? [] | ||
| 18 | } | ||
| 19 | |||
| 20 | private func argvOf(_ seen: StubProtocol.Seen) throws -> [String] { | 15 | private func argvOf(_ seen: StubProtocol.Seen) throws -> [String] { |
| 21 | let body = try #require(try JSONSerialization.jsonObject(with: seen.body) as? [String: Any]) | 16 | let body = try #require(try JSONSerialization.jsonObject(with: seen.body) as? [String: Any]) |
| 22 | return try #require(body["argv"] as? [String]) | 17 | return try #require(body["argv"] as? [String]) |
| 23 | } | 18 | } |
| 24 | 19 | ||
| 25 | private let bookmarks = """ | ||
| 26 | {"protocol_version":1,"data":[{"path":"krz/gitbay","visibility":"public"},\ | ||
| 27 | {"path":"krz/solar","visibility":"public"}],"exit_code":0} | ||
| 28 | """ | ||
| 29 | private let ok = """ | 20 | private let ok = """ |
| 30 | {"protocol_version":1,"exit_code":0} | 21 | {"protocol_version":1,"exit_code":0} |
| 31 | """ | 22 | """ |
| @@ -33,66 +24,18 @@ private let ok = """ | |||
| 33 | @MainActor | 24 | @MainActor |
| 34 | struct RepoActionsTests { | 25 | struct RepoActionsTests { |
| 35 | 26 | ||
| 36 | /// `repo bookmarks` is plural and takes NO repository argument. | ||
| 37 | @Test func theBookmarkListingTakesNoArgument() async throws { | ||
| 38 | let (client, stub) = try makeClient() | ||
| 39 | stub.enqueue(.init(status: 200, json: bookmarks)) | ||
| 40 | let model = RepoActionsViewModel(client: client, repoPath: "krz/gitbay") | ||
| 41 | await model.loadBookmarkState() | ||
| 42 | |||
| 43 | #expect(argvFrom(try #require(stub.seen.last).url) == ["repo", "bookmarks"]) | ||
| 44 | #expect(model.isBookmarked == true) | ||
| 45 | } | ||
| 46 | |||
| 47 | @Test func aRepoAbsentFromTheListingIsNotBookmarked() async throws { | ||
| 48 | let (client, stub) = try makeClient() | ||
| 49 | stub.enqueue(.init(status: 200, json: bookmarks)) | ||
| 50 | let model = RepoActionsViewModel(client: client, repoPath: "krz/other") | ||
| 51 | await model.loadBookmarkState() | ||
| 52 | |||
| 53 | #expect(model.isBookmarked == false) | ||
| 54 | } | ||
| 55 | |||
| 56 | /// "We don't know" is not "not bookmarked". A failed load must reset | ||
| 57 | /// the state to unknown rather than leaving a stale prior value in | ||
| 58 | /// place — this proves the reset by establishing a known `true` | ||
| 59 | /// first, then failing a second load. | ||
| 60 | @Test func aFailedListingLeavesTheStateUnknown() async throws { | ||
| 61 | let (client, stub) = try makeClient() | ||
| 62 | stub.enqueue(.init(status: 200, json: bookmarks)) | ||
| 63 | let model = RepoActionsViewModel(client: client, repoPath: "krz/gitbay") | ||
| 64 | await model.loadBookmarkState() | ||
| 65 | #expect(model.isBookmarked == true) | ||
| 66 | |||
| 67 | stub.enqueue(.init(status: 200, json: """ | ||
| 68 | {"protocol_version":1,"error":"denied","exit_code":4} | ||
| 69 | """)) | ||
| 70 | await model.loadBookmarkState() | ||
| 71 | |||
| 72 | #expect(model.isBookmarked == nil) | ||
| 73 | } | ||
| 74 | |||
| 75 | @Test func bookmarkingAndUnbookmarkingAreDifferentCommands() async throws { | 27 | @Test func bookmarkingAndUnbookmarkingAreDifferentCommands() async throws { |
| 76 | let (client, stub) = try makeClient() | 28 | let (client, stub) = try makeClient() |
| 77 | stub.enqueue(.init(status: 200, json: bookmarks)) | ||
| 78 | let model = RepoActionsViewModel(client: client, repoPath: "krz/gitbay") | 29 | let model = RepoActionsViewModel(client: client, repoPath: "krz/gitbay") |
| 79 | await model.loadBookmarkState() | ||
| 80 | 30 | ||
| 81 | stub.enqueue(.init(status: 200, json: ok)) | 31 | stub.enqueue(.init(status: 200, json: ok)) |
| 82 | stub.enqueue(.init(status: 200, json: bookmarks)) | ||
| 83 | await model.setBookmarked(false) | 32 | await model.setBookmarked(false) |
| 84 | var write = try #require(stub.seen.first { $0.method == "POST" }) | 33 | #expect(try argvOf(try #require(stub.seen.last)) == ["repo", "unbookmark", "krz/gitbay"]) |
| 85 | #expect(try argvOf(write) == ["repo", "unbookmark", "krz/gitbay"]) | 34 | |
| 86 | 35 | let model2 = RepoActionsViewModel(client: client, repoPath: "krz/other") | |
| 87 | let (client2, stub2) = try makeClient() | 36 | stub.enqueue(.init(status: 200, json: ok)) |
| 88 | stub2.enqueue(.init(status: 200, json: bookmarks)) | ||
| 89 | let model2 = RepoActionsViewModel(client: client2, repoPath: "krz/other") | ||
| 90 | await model2.loadBookmarkState() | ||
| 91 | stub2.enqueue(.init(status: 200, json: ok)) | ||
| 92 | stub2.enqueue(.init(status: 200, json: bookmarks)) | ||
| 93 | await model2.setBookmarked(true) | 37 | await model2.setBookmarked(true) |
| 94 | write = try #require(stub2.seen.first { $0.method == "POST" }) | 38 | #expect(try argvOf(try #require(stub.seen.last)) == ["repo", "bookmark", "krz/other"]) |
| 95 | #expect(try argvOf(write) == ["repo", "bookmark", "krz/other"]) | ||
| 96 | } | 39 | } |
| 97 | 40 | ||
| 98 | @Test func watchAndUnwatchSendTheirOwnCommands() async throws { | 41 | @Test func watchAndUnwatchSendTheirOwnCommands() async throws { |
| @@ -211,25 +154,4 @@ struct RepoActionsTests { | |||
| 211 | #expect(result == nil) | 154 | #expect(result == nil) |
| 212 | #expect(model.actionError?.isEmpty == false) | 155 | #expect(model.actionError?.isEmpty == false) |
| 213 | } | 156 | } |
| 214 | |||
| 215 | /// A bookmark write must be followed by a re-read of `repo | ||
| 216 | /// bookmarks` — that re-read is what keeps `isBookmarked` honest | ||
| 217 | /// instead of an optimistic local guess. | ||
| 218 | @Test func bookmarkingRereadsTheListingAfterTheWrite() async throws { | ||
| 219 | let (client, stub) = try makeClient() | ||
| 220 | stub.enqueue(.init(status: 200, json: bookmarks)) | ||
| 221 | let model = RepoActionsViewModel(client: client, repoPath: "krz/other") | ||
| 222 | await model.loadBookmarkState() | ||
| 223 | #expect(model.isBookmarked == false) | ||
| 224 | |||
| 225 | stub.enqueue(.init(status: 200, json: ok)) | ||
| 226 | stub.enqueue(.init(status: 200, json: bookmarks)) | ||
| 227 | await model.setBookmarked(true) | ||
| 228 | |||
| 229 | let calls = stub.seen.suffix(2) | ||
| 230 | let write = try #require(calls.first) | ||
| 231 | let reread = try #require(calls.last) | ||
| 232 | #expect(try argvOf(write) == ["repo", "bookmark", "krz/other"]) | ||
| 233 | #expect(argvFrom(reread.url) == ["repo", "bookmarks"]) | ||
| 234 | } | ||
| 235 | } | 157 | } |
gitbayTests/RepoStateTests.swift added +62
| @@ -0,0 +1,62 @@ | |||
| 1 | import Foundation | ||
| 2 | import Testing | ||
| 3 | @testable import gitbay | ||
| 4 | |||
| 5 | struct RepoStateDecodingTests { | ||
| 6 | |||
| 7 | private func decode(_ json: String) throws -> RepoDetail { | ||
| 8 | try JSONDecoder().decode(RepoDetail.self, from: Data(json.utf8)) | ||
| 9 | } | ||
| 10 | |||
| 11 | private let base = """ | ||
| 12 | "path":"krz/gitbay","visibility":"public","default_branch":"main" | ||
| 13 | """ | ||
| 14 | |||
| 15 | /// All three are omitempty. A repository you have expressed no | ||
| 16 | /// opinion about, which is not a fork, carries none of them. | ||
| 17 | @Test func aRepositoryWithNoStateDecodes() throws { | ||
| 18 | let repo = try decode("{\(base)}") | ||
| 19 | #expect(repo.watch == nil) | ||
| 20 | #expect(repo.bookmarked == false) | ||
| 21 | #expect(repo.forkOf == nil) | ||
| 22 | } | ||
| 23 | |||
| 24 | @Test func theCallersOwnStateDecodes() throws { | ||
| 25 | let repo = try decode(""" | ||
| 26 | {\(base),"watch":"watching","bookmarked":true,"fork_of":"upstream/thing"} | ||
| 27 | """) | ||
| 28 | #expect(repo.watch == "watching") | ||
| 29 | #expect(repo.bookmarked) | ||
| 30 | #expect(repo.forkOf == "upstream/thing") | ||
| 31 | } | ||
| 32 | |||
| 33 | @Test func mutedDecodes() throws { | ||
| 34 | #expect(try decode("{\(base),\"watch\":\"muted\"}").watch == "muted") | ||
| 35 | } | ||
| 36 | } | ||
| 37 | |||
| 38 | struct WatchStateTests { | ||
| 39 | |||
| 40 | @Test func theThreeStatesMapFromTheWire() { | ||
| 41 | #expect(WatchState("watching") == .watching) | ||
| 42 | #expect(WatchState("muted") == .muted) | ||
| 43 | #expect(WatchState(nil) == .default) | ||
| 44 | // An unknown value the server might add later must not crash or | ||
| 45 | // masquerade as watching. | ||
| 46 | #expect(WatchState("something-new") == .default) | ||
| 47 | } | ||
| 48 | |||
| 49 | /// Only two transitions exist. Nothing returns you to the default, | ||
| 50 | /// so nothing may offer it. | ||
| 51 | @Test func onlyTheReachableTransitionsAreOffered() { | ||
| 52 | #expect(WatchState.watching.available == [.mute]) | ||
| 53 | #expect(WatchState.muted.available == [.watch]) | ||
| 54 | #expect(WatchState.default.available == [.watch, .mute]) | ||
| 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 | } | ||