Commit `aa1e2d22c6`

aa1e2d22c6b9c16b338c90983577510755a4f949

parent: 21e79f09ba

Verified · cmc

cmc <hello@cleberg.net> · 2026-09-20 23:25 UTC

push: re-read authorization on activation, stop reloading the inbox

Two fixes from review. The denied row went stale after the Settings
round trip it exists to send the user on: nothing re-read iOS
authorization on return, so a user who just granted permission still
saw the disabled toggle and the turned-off notice until they left the
screen and came back. Re-read it on scene activation and on pull to
refresh.

Flipping the toggle or removing a device also reused perform(), whose
reload is the inbox list — that fetch set the list to .loading and
blanked every row behind a spinner for a write the user was watching.
Split perform() into the shared write-and-error-handling and the
inbox reload it layers on for markRead/markAllRead; the push callers
use the write alone and re-read only their own state afterward. Also
guarded setPush/removeDevice against a fast double-tap, matching
markRead's existing guard.

Ref krz/gitbay#89

Layout: unified · split

gitbay/Account/NotificationsViewModel.swift +24 −3

             ["notifications", "device", "list"], of: PushDevice.self)) ?? []
     }
+    /// Unlike `markRead`/`markAllRead`, this does not reload the inbox —
+    /// only its own state, so the list on screen does not blank out
+    /// behind a spinner for an unrelated fetch.
     func setPush(_ on: Bool) async {
-        await perform(["notifications", "settings", "push", on ? "on" : "off"])
+        guard !working else { return }
+        await performWrite(["notifications", "settings", "push", on ? "on" : "off"])
         await loadPushSettings()
     }
+    /// See `setPush` — no inbox reload here either.
     func removeDevice(_ id: Int64) async {
-        await perform(["notifications", "device", "remove", String(id)])
+        guard !working else { return }
+        await performWrite(["notifications", "device", "remove", String(id)])
         await loadPushSettings()
     }
         list.argv = ["notifications", "list"] + (showAll ? ["--all"] : [])
     }
+    /// `markRead`/`markAllRead`'s write: the shared error handling, plus
+    /// the inbox reload that makes sense for them but not for the push
+    /// settings below.
     private func perform(_ argv: [String]) async {
+        if await performWrite(argv) {
+            await load()
+        }
+    }
+    /// The write and its error handling alone, shared by every caller in
+    /// this file. Returns whether it succeeded, so a caller can decide
+    /// what to reload.
+    @discardableResult
+    private func performWrite(_ argv: [String]) async -> Bool {
         working = true
         actionError = nil
         defer { working = false }
         do {
             try await client.run(argv)
-            await load()
+            return true
         } catch let error as GitbayError {
             actionError = error.userFacingMessage
+            return false
         } catch {
             actionError = GitbayError.transport(error).userFacingMessage
+            return false
         }
     }
 }

gitbay/Views/Account/NotificationsView.swift +10

     @State private var model: NotificationsViewModel
     @Environment(PushRegistrar.self) private var registrar
+    @Environment(\.scenePhase) private var scenePhase
     @State private var authorization: UNAuthorizationStatus = .notDetermined
     init(client: GitbayClient) {
         .task { await model.load() }
         .task { await model.loadPushSettings() }
         .task { authorization = await registrar.authorizationStatus() }
+        // Settings is exactly where the denied row sends the user, and
+        // returning from it is a scene activation, not a refresh — this
+        // is what actually clears a stale "denied" once they flip the
+        // switch there.
+        .onChange(of: scenePhase) { _, newPhase in
+            guard newPhase == .active else { return }
+            Task { authorization = await registrar.authorizationStatus() }
+        }
         .refreshable {
             await model.load()
             await model.loadPushSettings()
+            authorization = await registrar.authorizationStatus()
         }
     }

gitbayTests/NotificationTests.swift +31 −4

         #expect(model.devices.isEmpty)
     }
-    @Test func settingPushSendsOnAndReloadsTheState() async throws {
+    /// Flipping the toggle must not touch the inbox: three requests
+    /// total (the write, then the two reads `loadPushSettings` makes for
+    /// its own state), never a fourth for `notifications list`. That
+    /// count is what stops the notification list on screen from
+    /// blanking out behind a spinner for an unrelated fetch.
+    @Test func settingPushReloadsItsOwnStateOnlyNotTheInbox() async throws {
         let (client, stub) = try makeClient()
         let model = NotificationsViewModel(client: client)
         stub.enqueue(.init(status: 200, json: """
             {"protocol_version":1,"exit_code":0}
             """))
-        stub.enqueue(.init(status: 200, json: page))
         stub.enqueue(.init(status: 200, json: settingsPushOn))
         stub.enqueue(.init(status: 200, json: noDevices))
         await model.setPush(true)
+        #expect(stub.seen.count == 3)
         let write = try #require(stub.seen.first { $0.method == "POST" })
         #expect(try argvOf(write) == ["notifications", "settings", "push", "on"])
         #expect(model.pushEnabled)
     }
-    @Test func removingADeviceSendsItsId() async throws {
+    /// See `settingPushReloadsItsOwnStateOnlyNotTheInbox` — the same
+    /// reasoning applies to a device removal.
+    @Test func removingADeviceSendsItsIdAndDoesNotReloadTheInbox() async throws {
         let (client, stub) = try makeClient()
         let model = NotificationsViewModel(client: client)
         stub.enqueue(.init(status: 200, json: """
             {"protocol_version":1,"exit_code":0}
             """))
-        stub.enqueue(.init(status: 200, json: page))
         stub.enqueue(.init(status: 200, json: """
             {"protocol_version":1,"data":{"mail":true,"watch":false,"push":false},"exit_code":0}
             """))
         stub.enqueue(.init(status: 200, json: noDevices))
         await model.removeDevice(7)
+        #expect(stub.seen.count == 3)
         let write = try #require(stub.seen.first { $0.method == "POST" })
         #expect(try argvOf(write) == ["notifications", "device", "remove", "7"])
         #expect(model.devices.isEmpty)
     }
+    /// A fast double-tap must not fire the write twice — `working` guards
+    /// it, the same pattern `markRead` already uses.
+    @Test func aSecondSetPushWhileWorkingIsIgnored() async throws {
+        let (client, stub) = try makeClient()
+        let model = NotificationsViewModel(client: client)
+        stub.enqueue(.init(status: 200, json: """
+            {"protocol_version":1,"exit_code":0}
+            """))
+        stub.enqueue(.init(status: 200, json: settingsPushOn))
+        stub.enqueue(.init(status: 200, json: noDevices))
+        async let first: Void = model.setPush(true)
+        await model.setPush(true)
+        _ = await first
+        // Only the first call's three requests; the guard drops the rest.
+        #expect(stub.seen.count == 3)
+    }
 }

Commit aa1e2d22c6

Commit `aa1e2d22c6`