Commit 6b87a902e8
Unsigned
Layout: unified · split
docs/superpowers/plans/2026-09-07-mr12-repo-state.md added +195
| @@ -0,0 +1,195 @@ | |||
| 1 | # MR 12: Render the repository state the server now reports | ||
| 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:** Replace the stateless watch/mute buttons with a control that shows where you actually are, drop the extra request the bookmark state costs, and say when a repository is a fork. | ||
| 6 | |||
| 7 | **Why now:** `krz/gitbay#178` — filed while building `!53`, because watch, bookmark and fork state were readable from no command — has been implemented upstream. `repo show` now carries all three. `!53` shipped a weaker affordance purely because of that gap, and the gap is gone. | ||
| 8 | |||
| 9 | **No parity row moves.** All three rows are already `yes`; this improves what closed them. | ||
| 10 | |||
| 11 | ## What the server now reports | ||
| 12 | |||
| 13 | From `internal/control/repo.go:288-299`, all three `omitempty`: | ||
| 14 | |||
| 15 | ```go | ||
| 16 | // ForkOf names the parent only when the caller can read it: a | ||
| 17 | // private parent is not confirmed to exist, here as anywhere. | ||
| 18 | ForkOf string `json:"fork_of,omitempty"` | ||
| 19 | // Watch and Bookmarked are the caller's own state, so a client | ||
| 20 | // can draw a toggle rather than two stateless buttons (#178). | ||
| 21 | Watch string `json:"watch,omitempty"` // watching, muted, or absent | ||
| 22 | Bookmarked bool `json:"bookmarked,omitempty"` | ||
| 23 | ``` | ||
| 24 | |||
| 25 | Three consequences that shape the whole design: | ||
| 26 | |||
| 27 | 1. **`watch` has three states, and only two are reachable.** `repo watch` sets `watching`; `repo unwatch` sets `muted` (`internal/control/notifications.go:177-181`). **No command returns you to the absent default.** So the control must show the current state and offer only the transitions that exist — never a "Default" option with no command behind it. | ||
| 28 | 2. **`bookmarked` is `omitempty` on a `Bool`, so absent means false.** Decoding must default it, not require it. | ||
| 29 | 3. **Absent `fork_of` does not mean "not a fork."** The server withholds it when the caller cannot read the parent — a private parent is not confirmed to exist. So the UI may say "forked from X" when present, and must say **nothing** when absent. It must never say "not a fork". | ||
| 30 | |||
| 31 | ## Global Constraints | ||
| 32 | |||
| 33 | - Swift 6 language mode, default `MainActor` isolation. Wire models are `nonisolated struct`s. | ||
| 34 | - Swift Testing only — never XCTest. | ||
| 35 | - `gitbayTests` is hermetic and offline; network goes through `StubProtocol`. | ||
| 36 | - New files under `gitbay/` and `gitbayTests/` need **no** `project.pbxproj` edit. | ||
| 37 | - The label model is `IssueLabel`; the notification model is `InboxNotification`. | ||
| 38 | - Never mention Claude, LLMs or AI in commits, comments, or the merge request. No `Co-Authored-By` trailer. | ||
| 39 | - Never commit to `main`. | ||
| 40 | - **Test discipline:** use the `until(_:_:)` helper in `gitbayTests/StubProtocol.swift` rather than fixed sleeps; assert on the request identified by its argv, not `stub.seen.last`; never index `stub.seen` directly, go through `#require`; provision a stub for every request each flow makes. | ||
| 41 | |||
| 42 | Baseline: **365 total, 364 passed, 1 skipped, 0 failed.** | ||
| 43 | |||
| 44 | --- | ||
| 45 | |||
| 46 | ## File Structure | ||
| 47 | |||
| 48 | | File | Responsibility | | ||
| 49 | |------|----------------| | ||
| 50 | | `gitbay/Repos/RepoModels.swift` (modify) | Decode `watch`, `bookmarked`, `fork_of` on `RepoDetail` | | ||
| 51 | | `gitbay/Repos/RepoActionsViewModel.swift` (modify) | Delete `loadBookmarkState`; take state from `RepoDetail` | | ||
| 52 | | `gitbayTests/RepoActionTests.swift` (modify) | Rewrite the bookmark-state tests | | ||
| 53 | | `gitbayTests/RepoStateTests.swift` (create) | Decoding and transition tests | | ||
| 54 | | `gitbay/Views/Repos/RepoView.swift` (modify) | The watch control, the bookmark toggle, the fork line | | ||
| 55 | |||
| 56 | --- | ||
| 57 | |||
| 58 | ### Task 1: Decode the state, and stop fetching it separately | ||
| 59 | |||
| 60 | **Files:** `RepoModels.swift`, `RepoActionsViewModel.swift`; tests in `gitbayTests/RepoStateTests.swift` and `RepoActionTests.swift` | ||
| 61 | |||
| 62 | **Interfaces produced:** | ||
| 63 | - `RepoDetail.watch: String?`, `.bookmarked: Bool`, `.forkOf: String?` | ||
| 64 | - `nonisolated enum WatchState: Sendable` — `.watching`, `.muted`, `.default`; `init(_ raw: String?)`; `var available: [WatchAction]` giving only the transitions that exist | ||
| 65 | - `RepoActionsViewModel` **loses** `isBookmarked` and `loadBookmarkState()`; the screen reads state from the loaded `RepoDetail` instead | ||
| 66 | |||
| 67 | **What to delete, and why.** `!53` added `loadBookmarkState()`, which calls `repo bookmarks` and tests membership — one extra read per repository screen, plus a `.refreshable` hook and a three-state `Bool?` to cover its failure. All of that existed only because the state was unreadable. `repo show` is already fetched by that screen, so the whole mechanism goes. Delete it; do not leave it dual-sourced. | ||
| 68 | |||
| 69 | Behaviour, each pinned by a test: | ||
| 70 | 1. `bookmarked` is `omitempty` on a Bool — **absent means false**, and must decode, not throw. | ||
| 71 | 2. `watch` absent means the default state, which is neither watching nor muted. | ||
| 72 | 3. `fork_of` absent means **either** not-a-fork **or** a parent you cannot read. The model exposes it as `String?` and says nothing more. | ||
| 73 | 4. `WatchState.available` returns `[.mute]` when watching, `[.watch]` when muted, and **both** when default. It never offers a transition to the default, because no command performs one. | ||
| 74 | 5. `watch()` and `unwatch()` still send `repo watch` / `repo unwatch` unchanged, and still reload so the new state is read back from the server rather than assumed. | ||
| 75 | |||
| 76 | - [ ] **Step 1: Write the failing tests** | ||
| 77 | |||
| 78 | ```swift | ||
| 79 | import Foundation | ||
| 80 | import Testing | ||
| 81 | @testable import gitbay | ||
| 82 | |||
| 83 | struct RepoStateDecodingTests { | ||
| 84 | |||
| 85 | private func decode(_ json: String) throws -> RepoDetail { | ||
| 86 | try JSONDecoder().decode(RepoDetail.self, from: Data(json.utf8)) | ||
| 87 | } | ||
| 88 | |||
| 89 | private let base = """ | ||
| 90 | "path":"krz/gitbay","visibility":"public","default_branch":"main" | ||
| 91 | """ | ||
| 92 | |||
| 93 | /// All three are omitempty. A repository you have expressed no | ||
| 94 | /// opinion about, which is not a fork, carries none of them. | ||
| 95 | @Test func aRepositoryWithNoStateDecodes() throws { | ||
| 96 | let repo = try decode("{\(base)}") | ||
| 97 | #expect(repo.watch == nil) | ||
| 98 | #expect(repo.bookmarked == false) | ||
| 99 | #expect(repo.forkOf == nil) | ||
| 100 | } | ||
| 101 | |||
| 102 | @Test func theCallersOwnStateDecodes() throws { | ||
| 103 | let repo = try decode(""" | ||
| 104 | {\(base),"watch":"watching","bookmarked":true,"fork_of":"upstream/thing"} | ||
| 105 | """) | ||
| 106 | #expect(repo.watch == "watching") | ||
| 107 | #expect(repo.bookmarked) | ||
| 108 | #expect(repo.forkOf == "upstream/thing") | ||
| 109 | } | ||
| 110 | |||
| 111 | @Test func mutedDecodes() throws { | ||
| 112 | #expect(try decode("{\(base),\"watch\":\"muted\"}").watch == "muted") | ||
| 113 | } | ||
| 114 | } | ||
| 115 | |||
| 116 | struct WatchStateTests { | ||
| 117 | |||
| 118 | @Test func theThreeStatesMapFromTheWire() { | ||
| 119 | #expect(WatchState("watching") == .watching) | ||
| 120 | #expect(WatchState("muted") == .muted) | ||
| 121 | #expect(WatchState(nil) == .default) | ||
| 122 | // An unknown value the server might add later must not crash or | ||
| 123 | // masquerade as watching. | ||
| 124 | #expect(WatchState("something-new") == .default) | ||
| 125 | } | ||
| 126 | |||
| 127 | /// Only two transitions exist. Nothing returns you to the default, | ||
| 128 | /// so nothing may offer it. | ||
| 129 | @Test func onlyTheReachableTransitionsAreOffered() { | ||
| 130 | #expect(WatchState.watching.available == [.mute]) | ||
| 131 | #expect(WatchState.muted.available == [.watch]) | ||
| 132 | #expect(WatchState.default.available == [.watch, .mute]) | ||
| 133 | } | ||
| 134 | |||
| 135 | @Test func noStateOffersAReturnToTheDefault() { | ||
| 136 | for state in [WatchState.watching, .muted, .default] { | ||
| 137 | #expect(state.available.allSatisfy { $0 == .watch || $0 == .mute }) | ||
| 138 | } | ||
| 139 | } | ||
| 140 | } | ||
| 141 | ``` | ||
| 142 | |||
| 143 | Add view-model tests confirming `watch()`/`unwatch()` still send their commands and reload, and **rewrite** `RepoActionTests`'s bookmark tests: the ones covering `loadBookmarkState` and the `repo bookmarks` argv go, since that path no longer exists. `setBookmarked` still sends `repo bookmark`/`repo unbookmark` and must keep its tests. | ||
| 144 | |||
| 145 | - [ ] **Step 2: Run to verify failure.** | ||
| 146 | - [ ] **Step 3: Implement, deleting `loadBookmarkState` and `isBookmarked` entirely.** | ||
| 147 | - [ ] **Step 4: Run the tests.** | ||
| 148 | - [ ] **Step 5: Commit** — `git commit -m "Take watch, bookmark and fork state from repo show"` | ||
| 149 | |||
| 150 | --- | ||
| 151 | |||
| 152 | ### Task 2: The controls | ||
| 153 | |||
| 154 | **Files:** `gitbay/Views/Repos/RepoView.swift` | ||
| 155 | |||
| 156 | No unit tests — UI. | ||
| 157 | |||
| 158 | **The watch control** replaces `!53`'s two stateless buttons. Show the current state and offer only what `WatchState.available` gives: | ||
| 159 | |||
| 160 | - Watching → "Watching" with a Mute action | ||
| 161 | - Muted → "Muted" with a Watch action | ||
| 162 | - Default → neither label claimed, both actions offered | ||
| 163 | |||
| 164 | Do **not** render a three-segment picker: selecting the default segment would have no command behind it. And drop `!53`'s caption saying the current setting is not shown — it is now. | ||
| 165 | |||
| 166 | **The bookmark toggle** reads `RepoDetail.bookmarked` directly. Remove the `.task` and `.refreshable` hooks that fed `loadBookmarkState`, and the "Checking Bookmark…" state, which existed only for a read that no longer happens. `setBookmarked` still reloads, so the toggle follows the server. | ||
| 167 | |||
| 168 | **The fork line:** when `forkOf` is present, show "Forked from `<path>`", linking to `RepoRoute.repo(forkOf)`. When absent, **show nothing** — absence means either not-a-fork or a parent you cannot see, and the screen must not claim the former. | ||
| 169 | |||
| 170 | - [ ] **Step 1: The watch control** | ||
| 171 | - [ ] **Step 2: The bookmark toggle, with the old load path removed** | ||
| 172 | - [ ] **Step 3: The fork line** | ||
| 173 | - [ ] **Step 4: Build and run the full suite.** | ||
| 174 | - [ ] **Step 5: Commit** — `git commit -m "Show watch, bookmark and fork state on the repository screen"` | ||
| 175 | |||
| 176 | --- | ||
| 177 | |||
| 178 | ### Task 3: Open the merge request | ||
| 179 | |||
| 180 | No parity rows move — all three are already `yes`. **Do not touch the wiki.** | ||
| 181 | |||
| 182 | - [ ] **Step 1:** Run the full suite via `xcresulttool`; record the real numbers. | ||
| 183 | - [ ] **Step 2:** Open the merge request, noting it closes the gap `krz/gitbay#178` described and that the issue is already closed upstream. | ||
| 184 | |||
| 185 | --- | ||
| 186 | |||
| 187 | ## Notes for whoever executes this | ||
| 188 | |||
| 189 | **No command returns you to the neutral watch state.** Only `repo watch` and `repo unwatch` exist. Any control offering a third option would be offering something the server cannot do. | ||
| 190 | |||
| 191 | **Absent `fork_of` is not "not a fork."** The server withholds it when you cannot read the parent. Say nothing rather than saying the wrong thing. | ||
| 192 | |||
| 193 | **`bookmarked` absent means false** — it is `omitempty` on a Bool. | ||
| 194 | |||
| 195 | **Delete the old path, do not dual-source it.** `loadBookmarkState` and `isBookmarked` existed only because the state was unreadable. Leaving both would mean two answers to one question. | ||
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/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/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 +42 −27
| @@ -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,34 +149,46 @@ 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.reloadDetail() |
| 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 { |
| 172 | Button { | 163 | let watchState = WatchState(detail.watch) |
| 173 | Task { await actionsModel.watch() } | 164 | switch watchState { |
| 174 | } label: { | 165 | case .watching: |
| 175 | Label("Watch this repository", systemImage: "eye") | 166 | Label("Watching", systemImage: "eye") |
| 167 | case .muted: | ||
| 168 | Label("Muted", systemImage: "bell.slash") | ||
| 169 | case .default: | ||
| 170 | EmptyView() | ||
| 176 | } | 171 | } |
| 177 | Button { | 172 | if watchState.available.contains(.watch) { |
| 178 | Task { await actionsModel.unwatch() } | 173 | Button { |
| 179 | } label: { | 174 | Task { |
| 180 | Label("Mute this repository", systemImage: "bell.slash") | 175 | await actionsModel.watch() |
| 176 | await model.reloadDetail() | ||
| 177 | } | ||
| 178 | } label: { | ||
| 179 | Label("Watch this repository", systemImage: "eye") | ||
| 180 | } | ||
| 181 | } | ||
| 182 | if watchState.available.contains(.mute) { | ||
| 183 | Button { | ||
| 184 | Task { | ||
| 185 | await actionsModel.unwatch() | ||
| 186 | await model.reloadDetail() | ||
| 187 | } | ||
| 188 | } label: { | ||
| 189 | Label("Mute this repository", systemImage: "bell.slash") | ||
| 190 | } | ||
| 181 | } | 191 | } |
| 182 | // Nothing reports which of these is current (krz/gitbay#178). | ||
| 183 | Text("The current setting isn't shown.") | ||
| 184 | } | 192 | } |
| 185 | Divider() | 193 | Divider() |
| 186 | Button { | 194 | Button { |
| @@ -244,6 +252,13 @@ struct RepoView: View { | |||
| 244 | } | 252 | } |
| 245 | .font(.gbSans(.caption)) | 253 | .font(.gbSans(.caption)) |
| 246 | .foregroundStyle(.secondary) | 254 | .foregroundStyle(.secondary) |
| 255 | if let forkOf = detail.forkOf { | ||
| 256 | NavigationLink(value: RepoRoute.repo(forkOf)) { | ||
| 257 | Text("Forked from \(forkOf)") | ||
| 258 | } | ||
| 259 | .font(.gbSans(.caption)) | ||
| 260 | .foregroundStyle(.secondary) | ||
| 261 | } | ||
| 247 | } | 262 | } |
| 248 | .padding(.vertical, 4) | 263 | .padding(.vertical, 4) |
| 249 | } | 264 | } |
gitbayTests/RepoActionTests.swift +8 −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,21 @@ 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(stub.seen.contains { (try? argvOf($0)) == ["repo", "unbookmark", "krz/gitbay"] }) |
| 85 | #expect(try argvOf(write) == ["repo", "unbookmark", "krz/gitbay"]) | 34 | #expect(stub.seen.count == 1) |
| 86 | 35 | ||
| 87 | let (client2, stub2) = try makeClient() | 36 | let model2 = RepoActionsViewModel(client: client, repoPath: "krz/other") |
| 88 | stub2.enqueue(.init(status: 200, json: bookmarks)) | 37 | let before = stub.seen.count |
| 89 | let model2 = RepoActionsViewModel(client: client2, repoPath: "krz/other") | 38 | stub.enqueue(.init(status: 200, json: ok)) |
| 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) | 39 | await model2.setBookmarked(true) |
| 94 | write = try #require(stub2.seen.first { $0.method == "POST" }) | 40 | #expect(stub.seen.contains { (try? argvOf($0)) == ["repo", "bookmark", "krz/other"] }) |
| 95 | #expect(try argvOf(write) == ["repo", "bookmark", "krz/other"]) | 41 | #expect(stub.seen.count == before + 1) |
| 96 | } | 42 | } |
| 97 | 43 | ||
| 98 | @Test func watchAndUnwatchSendTheirOwnCommands() async throws { | 44 | @Test func watchAndUnwatchSendTheirOwnCommands() async throws { |
| @@ -211,25 +157,4 @@ struct RepoActionsTests { | |||
| 211 | #expect(result == nil) | 157 | #expect(result == nil) |
| 212 | #expect(model.actionError?.isEmpty == false) | 158 | #expect(model.actionError?.isEmpty == false) |
| 213 | } | 159 | } |
| 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 | } | 160 | } |
gitbayTests/RepoStateTests.swift added +56
| @@ -0,0 +1,56 @@ | |||
| 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 | } | ||