From 5cb21883aff60c0313df0f8a987f46ae3b6b97d1 Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Thu, 29 Oct 2026 10:40:29 -0400 Subject: [PATCH] fix(notifications): title each row with its check name in every state A warning or fail used to replace the title with the problem, so "Notifications Allowed" became "Notifications are turned off". The title now stays the check name, and the problem moves to the detail's first line, with the explanation, if any, on the second. The checking registration row is "Registered for Push" too, over "Contacting Flipcash". --- .../Push/NotificationStatus.swift | 35 +++++++++------- .../Push/NotificationStatusTests.swift | 41 +++++++++++-------- .../NotificationStatusViewModelTests.swift | 3 +- 3 files changed, 47 insertions(+), 32 deletions(-) diff --git a/FlipcashCore/Sources/FlipcashCore/Push/NotificationStatus.swift b/FlipcashCore/Sources/FlipcashCore/Push/NotificationStatus.swift index fed0f5596..008c0d98d 100644 --- a/FlipcashCore/Sources/FlipcashCore/Push/NotificationStatus.swift +++ b/FlipcashCore/Sources/FlipcashCore/Push/NotificationStatus.swift @@ -184,7 +184,7 @@ extension NotificationStatus { /// i4 case registration - /// The check's name, used in the report and the passing row. + /// The check's name: the row's title in every state, and the report's label. public var name: String { switch self { case .permission: "Notifications Allowed" @@ -229,7 +229,9 @@ extension NotificationStatus { public struct Row: Equatable, Identifiable, Sendable { public var check: Check public var status: Status + /// The check's name, whatever the status. public var title: String + /// On a warning or fail, the problem on the first line and any explanation on the second. public var detail: String? public var fix: Fix? /// The raw value behind the status, for the report. @@ -347,9 +349,9 @@ extension NotificationStatus { case .authorized, .provisional, .ephemeral: return Row(check: .permission, status: .pass, title: Check.permission.name, detail: "Allowed in iOS Settings", raw: raw) case .denied: - return Row(check: .permission, status: .fail, title: "Notifications are turned off", detail: "Turned off for Flipcash in iOS Settings", fix: .openNotificationSettings, raw: raw) + return Row(check: .permission, status: .fail, title: Check.permission.name, detail: problem("Notifications are turned off", "Turned off for Flipcash in iOS Settings"), fix: .openNotificationSettings, raw: raw) case .notDetermined: - return Row(check: .permission, status: .fail, title: "Notifications aren't turned on yet", detail: "Flipcash hasn't asked yet", fix: .requestAuthorization, raw: raw) + return Row(check: .permission, status: .fail, title: Check.permission.name, detail: problem("Notifications aren't turned on yet", "Flipcash hasn't asked yet"), fix: .requestAuthorization, raw: raw) } } @@ -364,16 +366,16 @@ extension NotificationStatus { let raw = off.isEmpty ? nil : off.joined(separator: ", ") if bannersOff { - return Row(check: .display, status: .warning, title: "Banners are off", detail: "New notifications only go to Notification Center", fix: .openNotificationSettings, raw: raw) + return Row(check: .display, status: .warning, title: Check.display.name, detail: problem("Banners are off", "New notifications only go to Notification Center"), fix: .openNotificationSettings, raw: raw) } if lockScreenOff { - return Row(check: .display, status: .warning, title: "Hidden on the Lock Screen", fix: .openNotificationSettings, raw: raw) + return Row(check: .display, status: .warning, title: Check.display.name, detail: problem("Hidden on the Lock Screen"), fix: .openNotificationSettings, raw: raw) } return Row(check: .display, status: .pass, title: Check.display.name, detail: "Both on", raw: raw) } static func soundsAndBadgesRow(_ snapshot: Snapshot) -> Row { - let title: String? = switch (snapshot.sound.isOff, snapshot.badge.isOff) { + let problem: String? = switch (snapshot.sound.isOff, snapshot.badge.isOff) { case (true, true): "Sounds and badges are off" case (true, false): "Sounds are off" case (false, true): "Badges are off" @@ -385,31 +387,31 @@ extension NotificationStatus { if snapshot.badge.isOff { off.append("badge disabled") } let raw = off.isEmpty ? nil : off.joined(separator: ", ") - guard let title else { + guard let problem else { return Row(check: .soundsAndBadges, status: .pass, title: Check.soundsAndBadges.name, detail: "Both on", raw: raw) } - return Row(check: .soundsAndBadges, status: .warning, title: title, fix: .openNotificationSettings, raw: raw) + return Row(check: .soundsAndBadges, status: .warning, title: Check.soundsAndBadges.name, detail: Self.problem(problem), fix: .openNotificationSettings, raw: raw) } static func registrationRow(_ registration: Registration, saved: RegistrationRecord?, now: Date, locale: Locale) -> Row { let name = Check.registration.name switch registration { case .checking: - return Row(check: .registration, status: .checking, title: "Checking registration…", detail: "Contacting Flipcash", raw: "checking") + return Row(check: .registration, status: .checking, title: name, detail: "Contacting Flipcash", raw: "checking") case .noAPNsToken: - return Row(check: .registration, status: .fail, title: "This phone isn't registered with Apple", fix: .retryRegistration, raw: "noAPNsToken") + return Row(check: .registration, status: .fail, title: name, detail: problem("This phone isn't registered with Apple"), fix: .retryRegistration, raw: "noAPNsToken") case .finished(.ok): return Row(check: .registration, status: .pass, title: name, detail: "Confirmed just now", raw: RegistrationResult.ok.rawValue) case .finished(.invalidPushToken): - return Row(check: .registration, status: .fail, title: "Flipcash rejected this phone's push token", fix: .retryRegistrationDeletingToken, raw: RegistrationResult.invalidPushToken.rawValue) + return Row(check: .registration, status: .fail, title: name, detail: problem("Flipcash rejected this phone's push token"), fix: .retryRegistrationDeletingToken, raw: RegistrationResult.invalidPushToken.rawValue) case .finished(.transport): let raw = RegistrationResult.transport.rawValue if let okAt = saved?.lastRegistrationOkAt { - return Row(check: .registration, status: .warning, title: "Couldn't reach Flipcash", detail: "Last confirmed \(relative(okAt, to: now, locale: locale))", fix: .retryRegistration, raw: raw) + return Row(check: .registration, status: .warning, title: name, detail: problem("Couldn't reach Flipcash", "Last confirmed \(relative(okAt, to: now, locale: locale))"), fix: .retryRegistration, raw: raw) } - return Row(check: .registration, status: .fail, title: "Couldn't reach Flipcash", detail: "Check your connection", fix: .retryRegistration, raw: raw) + return Row(check: .registration, status: .fail, title: name, detail: problem("Couldn't reach Flipcash", "Check your connection"), fix: .retryRegistration, raw: raw) case .finished(.unknown): - return Row(check: .registration, status: .fail, title: "Registration failed", fix: .retryRegistration, raw: RegistrationResult.unknown.rawValue) + return Row(check: .registration, status: .fail, title: name, detail: problem("Registration failed"), fix: .retryRegistration, raw: RegistrationResult.unknown.rawValue) } } @@ -435,6 +437,11 @@ extension NotificationStatus { return Summary(status: .pass, title: "Notifications are working", detail: checked) } + /// A problem row's detail: the problem, then the explanation on its own line. + static func problem(_ problem: String, _ explanation: String? = nil) -> String { + [problem, explanation].compactMap { $0 }.joined(separator: "\n") + } + static func plural(_ count: Int, _ one: String, _ many: String) -> String { "\(count) \(count == 1 ? one : many)" } diff --git a/FlipcashCore/Tests/FlipcashCoreTests/Push/NotificationStatusTests.swift b/FlipcashCore/Tests/FlipcashCoreTests/Push/NotificationStatusTests.swift index 8004cb367..705f8a44c 100644 --- a/FlipcashCore/Tests/FlipcashCoreTests/Push/NotificationStatusTests.swift +++ b/FlipcashCore/Tests/FlipcashCoreTests/Push/NotificationStatusTests.swift @@ -55,6 +55,8 @@ struct NotificationStatusTests { #expect(page.summary.status == .fail) #expect(page.summary.title == "Notifications are off") #expect(page.rows.map(\.check) == [.permission, .registration]) + #expect(Self.row(.permission, in: page)?.title == "Notifications Allowed") + #expect(Self.row(.permission, in: page)?.detail == "Notifications are turned off\nTurned off for Flipcash in iOS Settings") #expect(Self.row(.permission, in: page)?.fix == .openNotificationSettings) #expect(Self.row(.permission, in: page)?.fixTitle == "Turn On") #expect(page.isTestEnabled == false) @@ -66,8 +68,8 @@ struct NotificationStatusTests { let row = Self.row(.permission, in: Self.page(.init(authorization: .notDetermined))) #expect(row?.status == .fail) - #expect(row?.title == "Notifications aren't turned on yet") - #expect(row?.detail == "Flipcash hasn't asked yet") + #expect(row?.title == "Notifications Allowed") + #expect(row?.detail == "Notifications aren't turned on yet\nFlipcash hasn't asked yet") #expect(row?.fix == .requestAuthorization) #expect(row?.fixTitle == "Turn On") } @@ -78,7 +80,8 @@ struct NotificationStatusTests { let row = Self.row(.display, in: page) #expect(row?.status == .warning) - #expect(row?.title == "Banners are off") + #expect(row?.title == "Banners and Lock Screen") + #expect(row?.detail == "Banners are off\nNew notifications only go to Notification Center") #expect(row?.fix == .openNotificationSettings) #expect(row?.fixTitle == "Fix") #expect(page.summary.status == .warning) @@ -92,7 +95,8 @@ struct NotificationStatusTests { let row = Self.row(.display, in: Self.page(.init(authorization: .authorized, lockScreen: .disabled))) #expect(row?.status == .warning) - #expect(row?.title == "Hidden on the Lock Screen") + #expect(row?.title == "Banners and Lock Screen") + #expect(row?.detail == "Hidden on the Lock Screen") } @Test("Sounds and badges name what is off", arguments: [ @@ -100,11 +104,12 @@ struct NotificationStatusTests { (.enabled, .disabled, "Badges are off"), (.disabled, .disabled, "Sounds and badges are off"), ]) - func soundsAndBadgesOff(sound: NotificationStatus.Snapshot.Setting, badge: NotificationStatus.Snapshot.Setting, title: String) { + func soundsAndBadgesOff(sound: NotificationStatus.Snapshot.Setting, badge: NotificationStatus.Snapshot.Setting, problem: String) { let row = Self.row(.soundsAndBadges, in: Self.page(.init(authorization: .authorized, sound: sound, badge: badge))) #expect(row?.status == .warning) - #expect(row?.title == title) + #expect(row?.title == "Sounds and Badges") + #expect(row?.detail == problem) } @Test("10: an invalid push token fails and its retry deletes the local token first") @@ -113,7 +118,8 @@ struct NotificationStatusTests { let row = Self.row(.registration, in: page) #expect(row?.status == .fail) - #expect(row?.title == "Flipcash rejected this phone's push token") + #expect(row?.title == "Registered for Push") + #expect(row?.detail == "Flipcash rejected this phone's push token") #expect(row?.fix == .retryRegistrationDeletingToken) #expect(row?.fixTitle == "Try Again") #expect(page.summary.title == "1 thing to fix") @@ -125,8 +131,8 @@ struct NotificationStatusTests { let row = Self.row(.registration, in: Self.page(registration: .finished(.transport), saved: saved)) #expect(row?.status == .fail) - #expect(row?.title == "Couldn't reach Flipcash") - #expect(row?.detail == "Check your connection") + #expect(row?.title == "Registered for Push") + #expect(row?.detail == "Couldn't reach Flipcash\nCheck your connection") #expect(row?.fix == .retryRegistration) } @@ -137,8 +143,8 @@ struct NotificationStatusTests { let row = Self.row(.registration, in: page) #expect(row?.status == .warning) - #expect(row?.title == "Couldn't reach Flipcash") - #expect(row?.detail == "Last confirmed 5 minutes ago") + #expect(row?.title == "Registered for Push") + #expect(row?.detail == "Couldn't reach Flipcash\nLast confirmed 5 minutes ago") #expect(page.summary.title == "1 thing to check") } @@ -148,7 +154,7 @@ struct NotificationStatusTests { let row = Self.row(.registration, in: Self.page(registration: .finished(.transport), saved: saved)) #expect(row?.status == .warning) - #expect(row?.detail == "Last confirmed 2 hours ago") + #expect(row?.detail == "Couldn't reach Flipcash\nLast confirmed 2 hours ago") } @Test("A save under a minute old reads just now, not 0 minutes ago", arguments: [0.0, -1, -59]) @@ -156,7 +162,7 @@ struct NotificationStatusTests { let saved = NotificationStatus.RegistrationRecord(at: Self.now.addingTimeInterval(offset), result: .ok, source: .refresh, lastRegistrationOkAt: Self.now.addingTimeInterval(offset)) let page = Self.page(registration: .finished(.transport), saved: saved) - #expect(Self.row(.registration, in: page)?.detail == "Last confirmed just now") + #expect(Self.row(.registration, in: page)?.detail == "Couldn't reach Flipcash\nLast confirmed just now") } @Test("A save a minute old reads in minutes") @@ -164,7 +170,7 @@ struct NotificationStatusTests { let saved = NotificationStatus.RegistrationRecord(at: Self.now.addingTimeInterval(-60), result: .ok, source: .refresh, lastRegistrationOkAt: Self.now.addingTimeInterval(-60)) let page = Self.page(registration: .finished(.transport), saved: saved) - #expect(Self.row(.registration, in: page)?.detail == "Last confirmed 1 minute ago") + #expect(Self.row(.registration, in: page)?.detail == "Couldn't reach Flipcash\nLast confirmed 1 minute ago") } @Test("Group headers are title case") @@ -179,7 +185,7 @@ struct NotificationStatusTests { let row = Self.row(.registration, in: page) #expect(row?.status == .checking) - #expect(row?.title == "Checking registration…") + #expect(row?.title == "Registered for Push") #expect(row?.detail == "Contacting Flipcash") #expect(page.summary.status == .checking) #expect(page.summary.title == "Checking…") @@ -192,7 +198,7 @@ struct NotificationStatusTests { #expect(page.rows.map(\.check) == NotificationStatus.Check.allCases) #expect(page.rows.allSatisfy { $0.status == .checking }) - #expect(Self.row(.registration, in: page)?.title == "Checking registration…") + #expect(page.rows.map(\.title) == NotificationStatus.Check.allCases.map(\.name)) #expect(page.summary.status == .checking) #expect(page.isTestEnabled == false) #expect(page.foldedTitle == nil) @@ -252,7 +258,8 @@ struct NotificationStatusTests { func noAPNsToken() { let page = Self.page(registration: .noAPNsToken) #expect(page.summary.title == "1 thing to fix") - #expect(Self.row(.registration, in: page)?.title == "This phone isn't registered with Apple") + #expect(Self.row(.registration, in: page)?.title == "Registered for Push") + #expect(Self.row(.registration, in: page)?.detail == "This phone isn't registered with Apple") } @Test("14: notSupported settings are not counted as off") diff --git a/FlipcashTests/Settings/NotificationStatusViewModelTests.swift b/FlipcashTests/Settings/NotificationStatusViewModelTests.swift index 795234645..9860e8a15 100644 --- a/FlipcashTests/Settings/NotificationStatusViewModelTests.swift +++ b/FlipcashTests/Settings/NotificationStatusViewModelTests.swift @@ -148,7 +148,8 @@ struct NotificationStatusViewModelTests { let row = try #require(model.page?.rows.first { $0.check == .registration }) #expect(store.record?.result == .transport) #expect(row.status == .warning) - #expect(row.detail == "Last confirmed 5 minutes ago") + #expect(row.title == "Registered for Push") + #expect(row.detail == "Couldn't reach Flipcash\nLast confirmed 5 minutes ago") #expect(model.page?.summary.status == .warning) }