Commit 888a5c286c

888a5c286c37590fd91351de07b52e6e4084a9e5

parent: 26e787b104

Verified · cmc

cmc <hello@cleberg.net> · 2026-09-07 01:41 UTC

Fix final-review findings in profile set/edit

- ProfileEdit.Link gets an id: UUID and Identifiable, and the edit
  sheet's links section binds ForEach($edit.links) to it instead of
  index-based enumeration, which could subscript out of range against
  the already-shortened array on delete.
- Move the label pipe-character check into ProfileEdit.validationError
  so saveProfile refuses it regardless of caller, not just the sheet's
  own disabled-state check.
- Remove the unused ProfileEdit.isEmpty.
- Make aboutStdin a plain String; it never returned nil.
- Add tests for ProfileEdit(from:), including the org about_format and
  the md fallback, a pipe-in-label validation test, and fix the reload
  test's fixture so it can distinguish the server's value from the
  typed one.

Layout: unified · split

gitbay/Discovery/ProfileEdit.swift +11 −8
@@ -8,7 +8,8 @@ import Foundation
8/// would leave the old value alone. 8/// would leave the old value alone.
9nonisolated struct ProfileEdit: Equatable, Sendable { 9nonisolated struct ProfileEdit: Equatable, Sendable {
10 10
11 nonisolated struct Link: Equatable, Sendable { 11 nonisolated struct Link: Equatable, Sendable, Identifiable {
12 var id = UUID()
12 var label: String 13 var label: String
13 var url: String 14 var url: String
14 } 15 }
@@ -39,17 +40,19 @@ nonisolated struct ProfileEdit: Equatable, Sendable {
39 links = (profile.links ?? []).map { Link(label: $0.label ?? "", url: $0.url) } 40 links = (profile.links ?? []).map { Link(label: $0.label ?? "", url: $0.url) }
40 } 41 }
41 42
42 var isEmpty: Bool { 43 /// At most 5 links and every link needs a URL — the server enforces
43 description.isEmpty && website.isEmpty && about.isEmpty && links.isEmpty 44 /// both, but the form should not let a save reach it that way. A
44 } 45 /// label may not contain `|` either: the server splits a link on its
45 46 /// first pipe, so a label carrying one would silently corrupt the URL
46 /// At most 5 links, and every link needs a URL — the server enforces 47 /// rather than be rejected.
47 /// both, but the form should not let a save reach it that way.
48 var validationError: String? { 48 var validationError: String? {
49 if links.count > 5 { return "At most 5 links." } 49 if links.count > 5 { return "At most 5 links." }
50 if links.contains(where: { $0.url.trimmingCharacters(in: .whitespaces).isEmpty }) { 50 if links.contains(where: { $0.url.trimmingCharacters(in: .whitespaces).isEmpty }) {
51 return "A link needs a URL." 51 return "A link needs a URL."
52 } 52 }
53 if links.contains(where: { $0.label.contains("|") }) {
54 return "A link label can't contain \"|\" — that's what separates it from the URL."
55 }
53 return nil 56 return nil
54 } 57 }
55 58
@@ -70,5 +73,5 @@ nonisolated struct ProfileEdit: Equatable, Sendable {
70 73
71 /// The about text, sent over stdin — never in argv, since it is 74 /// The about text, sent over stdin — never in argv, since it is
72 /// long-form and may hold newlines. 75 /// long-form and may hold newlines.
73 var aboutStdin: String? { about } 76 var aboutStdin: String { about }
74} 77}
gitbay/Views/Discovery/ProfileEditSheet.swift +5 −14
@@ -19,17 +19,7 @@ struct ProfileEditSheet: View {
19 _edit = State(initialValue: ProfileEdit(from: profile)) 19 _edit = State(initialValue: ProfileEdit(from: profile))
20 } 20 }
21 21
22 /// The server splits a link on the first `|`, so a label carrying one 22 private var localError: String? { edit.validationError }
23 /// would silently push part of itself into the URL. Caught here,
24 /// ahead of `ProfileEdit.validationError`, so the message is specific
25 /// rather than a generic refusal.
26 private var pipeError: String? {
27 edit.links.contains(where: { $0.label.contains("|") })
28 ? "A link label can't contain \"|\" — that's what separates it from the URL."
29 : nil
30 }
31
32 private var localError: String? { pipeError ?? edit.validationError }
33 23
34 var body: some View { 24 var body: some View {
35 NavigationStack { 25 NavigationStack {
@@ -91,12 +81,13 @@ struct ProfileEditSheet: View {
91 81
92 private var linksSection: some View { 82 private var linksSection: some View {
93 Section { 83 Section {
94 ForEach(Array(edit.links.enumerated()), id: \.offset) { index, _ in 84 ForEach($edit.links) { $link in
85 let index = edit.links.firstIndex { $0.id == link.id } ?? 0
95 VStack(alignment: .leading, spacing: 4) { 86 VStack(alignment: .leading, spacing: 4) {
96 TextField("Label", text: $edit.links[index].label) 87 TextField("Label", text: $link.label)
97 .autocorrectionDisabled() 88 .autocorrectionDisabled()
98 .accessibilityIdentifier("profile-edit-link-label-\(index)") 89 .accessibilityIdentifier("profile-edit-link-label-\(index)")
99 TextField("https://example.com", text: $edit.links[index].url) 90 TextField("https://example.com", text: $link.url)
100 .keyboardType(.URL) 91 .keyboardType(.URL)
101 .autocorrectionDisabled() 92 .autocorrectionDisabled()
102 .textInputAutocapitalization(.never) 93 .textInputAutocapitalization(.never)
gitbayTests/ProfileEditTests.swift +52 −3
@@ -84,6 +84,55 @@ struct ProfileEditFlagTests {
84 @Test func aLinkWithoutAUrlIsInvalid() { 84 @Test func aLinkWithoutAUrlIsInvalid() {
85 #expect(edit(links: [.init(label: "blog", url: " ")]).validationError != nil) 85 #expect(edit(links: [.init(label: "blog", url: " ")]).validationError != nil)
86 } 86 }
87
88 /// The server splits a link on its first `|`, so a label carrying one
89 /// would silently corrupt the URL rather than be rejected — caught
90 /// here, not just in the view.
91 @Test func aLabelContainingAPipeIsInvalid() {
92 #expect(edit(links: [.init(label: "a|b", url: "https://x.test")]).validationError != nil)
93 }
94}
95
96struct ProfileEditFromProfileTests {
97
98 private let orgProfileJSON = """
99 {"name":"cmc","kind":"user",\
100 "description":"about me","website":"https://cleberg.net",\
101 "about":"* heading\\n\\n| a | b |","about_format":"org",\
102 "links":[{"label":null,"url":"https://a.test"},{"label":"blog","url":"https://b.test"}],\
103 "repos":[],"activity_total":0}
104 """
105
106 /// The real profile on gitbay.org is org, with tables and links that
107 /// render as flat text under markdown — the stored format must round
108 /// trip, not default away from what the server holds.
109 @Test func seedsFromAnOrgProfile() throws {
110 let data = try #require(orgProfileJSON.data(using: .utf8))
111 let profile = try JSONDecoder().decode(ProfileViewModel.Profile.self, from: data)
112
113 let edit = ProfileEdit(from: profile)
114
115 #expect(edit.aboutFormat == "org")
116 #expect(edit.description == "about me")
117 #expect(edit.website == "https://cleberg.net")
118 #expect(edit.about == "* heading\n\n| a | b |")
119 #expect(edit.links.map(\.url) == ["https://a.test", "https://b.test"])
120 #expect(edit.links.map(\.label) == ["", "blog"])
121 }
122
123 /// Absent `about_format`, seeding falls back to markdown.
124 @Test func seedingFallsBackToMarkdownWhenFormatIsAbsent() throws {
125 let json = """
126 {"name":"cmc","kind":"user",\
127 "description":"hi","repos":[],"activity_total":0}
128 """
129 let data = try #require(json.data(using: .utf8))
130 let profile = try JSONDecoder().decode(ProfileViewModel.Profile.self, from: data)
131
132 let edit = ProfileEdit(from: profile)
133
134 #expect(edit.aboutFormat == "md")
135 }
87} 136}
88 137
89private func makeClient() throws -> (GitbayClient, StubProtocol.Box) { 138private func makeClient() throws -> (GitbayClient, StubProtocol.Box) {
@@ -105,7 +154,7 @@ private let okJSON = #"{"protocol_version":1,"exit_code":0}"#
105 154
106private let profileJSON = """ 155private let profileJSON = """
107 {"protocol_version":1,"data":{"name":"cmc","kind":"user",\ 156 {"protocol_version":1,"data":{"name":"cmc","kind":"user",\
108 "description":"hi","website":"https://cleberg.net","repos":[],\ 157 "description":"server value","website":"https://cleberg.net","repos":[],\
109 "activity_total":0},"exit_code":0} 158 "activity_total":0},"exit_code":0}
110 """ 159 """
111 160
@@ -149,11 +198,11 @@ struct ProfileSaveTests {
149 stub.enqueue(.init(status: 200, json: profileJSON, match: "argv=show")) 198 stub.enqueue(.init(status: 200, json: profileJSON, match: "argv=show"))
150 let model = ProfileViewModel(client: client, name: "cmc") 199 let model = ProfileViewModel(client: client, name: "cmc")
151 200
152 let saved = await model.saveProfile(ProfileEdit(description: "hi", website: "https://cleberg.net")) 201 let saved = await model.saveProfile(ProfileEdit(description: "typed value", website: "https://cleberg.net"))
153 202
154 #expect(saved) 203 #expect(saved)
155 let profile = try #require(model.state.value) 204 let profile = try #require(model.state.value)
156 #expect(profile.description == "hi") 205 #expect(profile.description == "server value")
157 } 206 }
158 207
159 @Test func aRefusedSaveReturnsFalseAndSurfaces() async throws { 208 @Test func aRefusedSaveReturnsFalseAndSurfaces() async throws {