Commit 5c2efd71c6

5c2efd71c6bfd5b1fa90a16d02fbd0309dd62feb

parent: f321cd5b62

Verified · cmc

cmc <hello@cleberg.net> · 2026-09-06 23:14 UTC

Fix notification inbox review findings

- Pin the dashboard unread count in the fixture and assert it decodes,
  so a wrong key name can't silently fall back to 0.
- Gate swipe-to-mark-read on `working`, matching the toolbar action.
- Clear `actionError` on load and on filter change, not just at the
  start of the next action.
- Give the dashboard bell an accessibility label that states the
  unread count as one sentence, not a bare number after "bell".
- Use fractional-second timestamps in notification fixtures to match
  the server and the rest of the suite; add a cursor-reset test for a
  notifications filter change.

Layout: unified · split

gitbay/Account/NotificationsViewModel.swift +3
@@ -60,6 +60,7 @@ final class NotificationsViewModel {
60 var showAll = false { 60 var showAll = false {
61 didSet { 61 didSet {
62 guard showAll != oldValue else { return } 62 guard showAll != oldValue else { return }
63 actionError = nil
63 configureList() 64 configureList()
64 reloadTask?.cancel() 65 reloadTask?.cancel()
65 reloadTask = Task { await list.reload() } 66 reloadTask = Task { await list.reload() }
@@ -85,10 +86,12 @@ final class NotificationsViewModel {
85 86
86 func load() async { 87 func load() async {
87 reloadTask?.cancel() 88 reloadTask?.cancel()
89 actionError = nil
88 await list.reload() 90 await list.reload()
89 } 91 }
90 92
91 func markRead(_ id: Int64) async { 93 func markRead(_ id: Int64) async {
94 guard !working else { return }
92 await perform(["notifications", "read", String(id)]) 95 await perform(["notifications", "read", String(id)])
93 } 96 }
94 97
gitbay/Views/Dashboard/DashboardView.swift +6
@@ -55,6 +55,7 @@ struct DashboardView: View {
55 } 55 }
56 } 56 }
57 .accessibilityIdentifier("dashboard-notifications-button") 57 .accessibilityIdentifier("dashboard-notifications-button")
58 .accessibilityLabel(bellLabel)
58 } 59 }
59 ToolbarItem(placement: .topBarTrailing) { 60 ToolbarItem(placement: .topBarTrailing) {
60 Button { 61 Button {
@@ -78,6 +79,11 @@ struct DashboardView: View {
78 .refreshable { await model.load() } 79 .refreshable { await model.load() }
79 } 80 }
80 81
82 private var bellLabel: String {
83 guard let unread = model.state.value?.unread, unread > 0 else { return "Notifications" }
84 return "Notifications, \(unread) unread"
85 }
86
81 private func itemSection( 87 private func itemSection(
82 _ title: String, 88 _ title: String,
83 items: [DashboardItem], 89 items: [DashboardItem],
gitbayTests/DashboardViewModelTests.swift +3 −1
@@ -27,7 +27,8 @@ private let dashboardJSON = """
27 "data":{"number":2},"created_at":"2026-08-27T10:30:00.000Z"}],\ 27 "data":{"number":2},"created_at":"2026-08-27T10:30:00.000Z"}],\
28 "builds":[{"repo":"krz/gitbay","number":9,"job":"ci","status":"success",\ 28 "builds":[{"repo":"krz/gitbay","number":9,"job":"ci","status":"success",\
29 "sha":"65ba14e0000000000000","ref":"refs/heads/main",\ 29 "sha":"65ba14e0000000000000","ref":"refs/heads/main",\
30 "created_at":"2026-08-27T09:00:00.000Z","finished_at":"2026-08-27T09:05:00.000Z"}]},\ 30 "created_at":"2026-08-27T09:00:00.000Z","finished_at":"2026-08-27T09:05:00.000Z"}],\
31 "unread":3},\
31 "exit_code":0} 32 "exit_code":0}
32 """ 33 """
33 34
@@ -54,6 +55,7 @@ struct DashboardViewModelTests {
54 #expect(data.openIssues.first?.number == 11) 55 #expect(data.openIssues.first?.number == 11)
55 #expect(data.recentActivity.first?.phrase == "merged !2") 56 #expect(data.recentActivity.first?.phrase == "merged !2")
56 #expect(data.builds.first?.status == "success") 57 #expect(data.builds.first?.status == "success")
58 #expect(data.unread == 3)
57 // The whole screen cost exactly one request. 59 // The whole screen cost exactly one request.
58 #expect(stub.seen.count == 1) 60 #expect(stub.seen.count == 1)
59 #expect(stub.seen.first?.url.query() == "argv=dashboard") 61 #expect(stub.seen.first?.url.query() == "argv=dashboard")
gitbayTests/NotificationTests.swift +27 −6
@@ -31,7 +31,7 @@ private func decode(_ json: String) throws -> InboxNotification {
31private let unreadBuild = """ 31private let unreadBuild = """
32 {"id":15,"repo":"krz/gitbay","kind":"build","actor":"ci",\ 32 {"id":15,"repo":"krz/gitbay","kind":"build","actor":"ci",\
33 "summary":"build 966 failed","path":"krz/gitbay/builds/966",\ 33 "summary":"build 966 failed","path":"krz/gitbay/builds/966",\
34 "created_at":"2026-09-06T22:14:46Z"} 34 "created_at":"2026-09-06T22:14:46.000Z"}
35 """ 35 """
36 36
37struct NotificationDecodingTests { 37struct NotificationDecodingTests {
@@ -48,7 +48,7 @@ struct NotificationDecodingTests {
48 let n = try decode(""" 48 let n = try decode("""
49 {"id":15,"repo":"krz/gitbay","kind":"build","actor":"ci",\ 49 {"id":15,"repo":"krz/gitbay","kind":"build","actor":"ci",\
50 "summary":"x","path":"krz/gitbay/builds/966",\ 50 "summary":"x","path":"krz/gitbay/builds/966",\
51 "created_at":"2026-09-06T22:14:46Z","read_at":"2026-09-06T22:20:24Z"} 51 "created_at":"2026-09-06T22:14:46.000Z","read_at":"2026-09-06T22:20:24.000Z"}
52 """) 52 """)
53 #expect(n.readAt != nil) 53 #expect(n.readAt != nil)
54 #expect(n.isUnread == false) 54 #expect(n.isUnread == false)
@@ -57,14 +57,14 @@ struct NotificationDecodingTests {
57 @Test func everyKindRoutesToItsScreen() throws { 57 @Test func everyKindRoutesToItsScreen() throws {
58 let issue = try decode(""" 58 let issue = try decode("""
59 {"id":1,"repo":"krz/gitbay","kind":"issue","actor":"cmc","summary":"x",\ 59 {"id":1,"repo":"krz/gitbay","kind":"issue","actor":"cmc","summary":"x",\
60 "path":"krz/gitbay/issues/168","created_at":"2026-09-06T22:14:46Z"} 60 "path":"krz/gitbay/issues/168","created_at":"2026-09-06T22:14:46.000Z"}
61 """) 61 """)
62 #expect(issue.destination == .issue(repo: "krz/gitbay", number: 168)) 62 #expect(issue.destination == .issue(repo: "krz/gitbay", number: 168))
63 63
64 // The segment is "mrs", not "merge_requests". 64 // The segment is "mrs", not "merge_requests".
65 let mr = try decode(""" 65 let mr = try decode("""
66 {"id":2,"repo":"krz/gitbay","kind":"mr","actor":"cmc","summary":"x",\ 66 {"id":2,"repo":"krz/gitbay","kind":"mr","actor":"cmc","summary":"x",\
67 "path":"krz/gitbay/mrs/282","created_at":"2026-09-06T22:14:46Z"} 67 "path":"krz/gitbay/mrs/282","created_at":"2026-09-06T22:14:46.000Z"}
68 """) 68 """)
69 #expect(mr.destination == .mr(repo: "krz/gitbay", number: 282)) 69 #expect(mr.destination == .mr(repo: "krz/gitbay", number: 282))
70 70
@@ -79,7 +79,7 @@ struct NotificationDecodingTests {
79 "too/short", "krz/gitbay/issues/1/extra", ""] { 79 "too/short", "krz/gitbay/issues/1/extra", ""] {
80 let n = try decode(""" 80 let n = try decode("""
81 {"id":1,"repo":"krz/gitbay","kind":"issue","actor":"cmc","summary":"x",\ 81 {"id":1,"repo":"krz/gitbay","kind":"issue","actor":"cmc","summary":"x",\
82 "path":"\(path)","created_at":"2026-09-06T22:14:46Z"} 82 "path":"\(path)","created_at":"2026-09-06T22:14:46.000Z"}
83 """) 83 """)
84 #expect(n.destination == nil, "\(path) should not route") 84 #expect(n.destination == nil, "\(path) should not route")
85 } 85 }
@@ -90,7 +90,7 @@ struct NotificationDecodingTests {
90 @Test func theDestinationRepoComesFromThePath() throws { 90 @Test func theDestinationRepoComesFromThePath() throws {
91 let n = try decode(""" 91 let n = try decode("""
92 {"id":1,"repo":"other/repo","kind":"issue","actor":"cmc","summary":"x",\ 92 {"id":1,"repo":"other/repo","kind":"issue","actor":"cmc","summary":"x",\
93 "path":"krz/gitbay/issues/7","created_at":"2026-09-06T22:14:46Z"} 93 "path":"krz/gitbay/issues/7","created_at":"2026-09-06T22:14:46.000Z"}
94 """) 94 """)
95 #expect(n.destination == .issue(repo: "krz/gitbay", number: 7)) 95 #expect(n.destination == .issue(repo: "krz/gitbay", number: 7))
96 } 96 }
@@ -103,6 +103,10 @@ struct NotificationsViewModelTests {
103 {"protocol_version":1,"data":{"items":[\(unreadBuild)]},"exit_code":0} 103 {"protocol_version":1,"data":{"items":[\(unreadBuild)]},"exit_code":0}
104 """ 104 """
105 105
106 private let pageWithCursor = """
107 {"protocol_version":1,"data":{"items":[\(unreadBuild)],"next":"c1"},"exit_code":0}
108 """
109
106 @Test func theDefaultListingIsUnreadOnly() async throws { 110 @Test func theDefaultListingIsUnreadOnly() async throws {
107 let (client, stub) = try makeClient() 111 let (client, stub) = try makeClient()
108 stub.enqueue(.init(status: 200, json: page)) 112 stub.enqueue(.init(status: 200, json: page))
@@ -128,6 +132,23 @@ struct NotificationsViewModelTests {
128 #expect(argvFrom(try #require(stub.seen.last).url).contains("--all")) 132 #expect(argvFrom(try #require(stub.seen.last).url).contains("--all"))
129 } 133 }
130 134
135 /// A filter change reloads rather than pages, so it must not carry
136 /// forward a cursor from whatever page was on screen.
137 @Test func aFilterChangeDoesNotCarryAStaleCursor() async throws {
138 let (client, stub) = try makeClient()
139 stub.enqueue(.init(status: 200, json: pageWithCursor))
140 let model = NotificationsViewModel(client: client)
141 await model.load()
142 #expect(model.list.hasMore)
143
144 stub.enqueue(.init(status: 200, json: page))
145 model.showAll = true
146 try await Task.sleep(for: .milliseconds(150))
147
148 let argv = argvFrom(try #require(stub.seen.last).url)
149 #expect(argv.contains("--cursor") == false)
150 }
151
131 @Test func markingOneReadSendsItsId() async throws { 152 @Test func markingOneReadSendsItsId() async throws {
132 let (client, stub) = try makeClient() 153 let (client, stub) = try makeClient()
133 stub.enqueue(.init(status: 200, json: page)) 154 stub.enqueue(.init(status: 200, json: page))