From 12ddbf26ce8472d1fdbcda0057884a9388678fd5 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Mon, 12 Oct 2026 19:40:08 +0700 Subject: [PATCH 1/2] refactor(settings)!: remove the per-kind notification toggles --- CHANGELOG.md | 1 + .../OperationCompletionPolicy.swift | 8 ++-- .../Settings/NotificationSettings.swift | 25 +--------- .../Settings/NotificationsSettingsView.swift | 32 ------------- .../OperationCompletionPolicyTests.swift | 46 +++++++++++-------- 5 files changed, 33 insertions(+), 79 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3944d16251..0e62f676bd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Interface, Sidebar and Toolbar color groups from the theme editor and the theme file. - Fonts from the theme file. +- Per-kind toggles in Settings > Notifications. Kinds you turned off notify again while notifications are on. ### Fixed diff --git a/TablePro/Core/Services/Operations/OperationCompletionPolicy.swift b/TablePro/Core/Services/Operations/OperationCompletionPolicy.swift index 2005f64beb..13b9de4697 100644 --- a/TablePro/Core/Services/Operations/OperationCompletionPolicy.swift +++ b/TablePro/Core/Services/Operations/OperationCompletionPolicy.swift @@ -9,13 +9,13 @@ internal enum CompletionSuppression: Equatable, Sendable { case cancelled case belowThreshold case resultOnScreen - case kindDisabled + case notificationsOff } /// What should happen now that an operation has finished. /// /// The tab marker and the notification are separate channels on purpose. The marker is the -/// in-app one and does not depend on notification permission, on the per-kind toggles, or on +/// in-app one and does not depend on notification permission, on the notification switch, or on /// the user ever having seen a banner, so a user who denied notifications still gets told which /// tab finished while they were away. internal struct CompletionDecision: Equatable { @@ -72,9 +72,9 @@ internal enum OperationCompletionPolicy { /// keyed on a tab change and returning to the app is not one. let marksOwnerUnseen = !visibility.ownerIsSelectedInWindow - guard settings.isEnabled(for: completion.kind) else { + guard settings.isEnabled else { return CompletionDecision( - notification: nil, marksOwnerUnseen: marksOwnerUnseen, suppression: .kindDisabled + notification: nil, marksOwnerUnseen: marksOwnerUnseen, suppression: .notificationsOff ) } diff --git a/TablePro/Models/Settings/NotificationSettings.swift b/TablePro/Models/Settings/NotificationSettings.swift index 2d186c363c..9a45184ec9 100644 --- a/TablePro/Models/Settings/NotificationSettings.swift +++ b/TablePro/Models/Settings/NotificationSettings.swift @@ -5,12 +5,7 @@ import Foundation -/// Which finished operations are worth a notification, and how long is long enough to count. -/// -/// Kinds are stored as the ones the user turned OFF rather than the ones left on, so a kind added -/// by a later release arrives enabled instead of silently missing from an older stored set. They -/// are stored as raw strings for the same reason in the other direction: a build that does not -/// know a kind must skip it, not fail to decode the whole settings record. +/// Whether a finished operation is worth a notification, and how long is long enough to count. struct NotificationSettings: Codable, Equatable { static let minimumThresholdSeconds = 5 static let maximumThresholdSeconds = 600 @@ -18,7 +13,6 @@ struct NotificationSettings: Codable, Equatable { var isEnabled: Bool var thresholdSeconds: Int - var disabledKindIds: Set static let `default` = NotificationSettings() @@ -26,26 +20,12 @@ struct NotificationSettings: Codable, Equatable { min(max(thresholdSeconds, Self.minimumThresholdSeconds), Self.maximumThresholdSeconds) } - func isEnabled(for kind: TrackedOperationKind) -> Bool { - isEnabled && !disabledKindIds.contains(kind.rawValue) - } - - mutating func setEnabled(_ enabled: Bool, for kind: TrackedOperationKind) { - if enabled { - disabledKindIds.remove(kind.rawValue) - } else { - disabledKindIds.insert(kind.rawValue) - } - } - init( isEnabled: Bool = true, - thresholdSeconds: Int = NotificationSettings.defaultThresholdSeconds, - disabledKindIds: Set = [] + thresholdSeconds: Int = NotificationSettings.defaultThresholdSeconds ) { self.isEnabled = isEnabled self.thresholdSeconds = thresholdSeconds - self.disabledKindIds = disabledKindIds } init(from decoder: Decoder) throws { @@ -53,6 +33,5 @@ struct NotificationSettings: Codable, Equatable { isEnabled = try container.decodeIfPresent(Bool.self, forKey: .isEnabled) ?? true thresholdSeconds = try container.decodeIfPresent(Int.self, forKey: .thresholdSeconds) ?? Self.defaultThresholdSeconds - disabledKindIds = try container.decodeIfPresent(Set.self, forKey: .disabledKindIds) ?? [] } } diff --git a/TablePro/Views/Settings/NotificationsSettingsView.swift b/TablePro/Views/Settings/NotificationsSettingsView.swift index 300a7beb7a..6a4e147837 100644 --- a/TablePro/Views/Settings/NotificationsSettingsView.swift +++ b/TablePro/Views/Settings/NotificationsSettingsView.swift @@ -32,13 +32,6 @@ struct NotificationsSettingsView: View { .foregroundStyle(.secondary) } - Section("Notify about") { - ForEach(TrackedOperationKind.allCases, id: \.self) { kind in - Toggle(kind.settingsLabel, isOn: binding(for: kind)) - .disabled(!settings.isEnabled) - } - } - if authorizationStatus == .denied { Section { HStack(spacing: 8) { @@ -57,13 +50,6 @@ struct NotificationsSettingsView: View { } } - private func binding(for kind: TrackedOperationKind) -> Binding { - Binding( - get: { !settings.disabledKindIds.contains(kind.rawValue) }, - set: { settings.setEnabled($0, for: kind) } - ) - } - private static let thresholdChoices = [5, 10, 20, 30, 60, 120, 300, 600] private static func thresholdLabel(_ seconds: Int) -> String { @@ -79,21 +65,3 @@ struct NotificationsSettingsView: View { NSWorkspace.shared.open(url) } } - -extension TrackedOperationKind { - var settingsLabel: String { - switch self { - case .query: return String(localized: "Queries") - case .queryBatch: return String(localized: "Statement batches") - case .rowSave: return String(localized: "Row edits") - case .schemaChange: return String(localized: "Structure changes") - case .dataImport: return String(localized: "Imports") - case .dataExport: return String(localized: "Exports") - case .objectCopy: return String(localized: "Object copies") - case .backup: return String(localized: "Backups") - case .fetchAll: return String(localized: "Fetch all rows") - case .mcpQuery: return String(localized: "AI and MCP queries") - case .scriptQuery: return String(localized: "AppleScript queries") - } - } -} diff --git a/TableProTests/Core/Operations/OperationCompletionPolicyTests.swift b/TableProTests/Core/Operations/OperationCompletionPolicyTests.swift index a26ba5238d..f2444ed79c 100644 --- a/TableProTests/Core/Operations/OperationCompletionPolicyTests.swift +++ b/TableProTests/Core/Operations/OperationCompletionPolicyTests.swift @@ -100,20 +100,8 @@ struct OperationCompletionPolicyTests { } } - @Test("A disabled kind still marks the tab but posts nothing") - func disabledKindStillMarksTheTab() { - var settings = NotificationSettings() - settings.setEnabled(false, for: .query) - let decision = OperationCompletionPolicy.decide( - completion: completion(), visibility: .offScreen, settings: settings - ) - #expect(decision.notification == nil) - #expect(decision.marksOwnerUnseen) - #expect(decision.suppression == .kindDisabled) - } - - @Test("Turning notifications off entirely still marks the tab") - func masterSwitchStillMarksTheTab() { + @Test("Turning notifications off still marks the tab") + func switchOffStillMarksTheTab() { let decision = OperationCompletionPolicy.decide( completion: completion(), visibility: .offScreen, @@ -121,6 +109,17 @@ struct OperationCompletionPolicyTests { ) #expect(decision.notification == nil) #expect(decision.marksOwnerUnseen) + #expect(decision.suppression == .notificationsOff) + } + + @Test("With notifications on, every kind posts") + func everyKindPostsWhileOn() { + for kind in TrackedOperationKind.allCases { + let decision = OperationCompletionPolicy.decide( + completion: completion(kind: kind), visibility: .offScreen, settings: .default + ) + #expect(decision.notification != nil, "\(kind.rawValue)") + } } /// One tab owns its query, its structure changes, its row saves and its fetch-all. Keying the @@ -182,12 +181,19 @@ struct OperationCompletionPolicyTests { #expect(NotificationSettings(thresholdSeconds: 99_999).validatedThresholdSeconds == 600) } - /// Stored as the kinds that are OFF, so a kind added by a later release arrives enabled - /// instead of being silently absent from an older stored set. - @Test("A kind the stored settings have never heard of is enabled") - func unknownKindDefaultsToEnabled() { - let settings = NotificationSettings(disabledKindIds: ["somethingOlderBuildsNeverKnew"]) - #expect(settings.isEnabled(for: .backup)) + @Test("Stored settings with per-kind choices load with the switch and the threshold intact") + func storedKindChoicesAreIgnored() throws { + let suiteName = "notifications-\(UUID().uuidString)" + let defaults = try #require(UserDefaults(suiteName: suiteName)) + defer { defaults.removePersistentDomain(forName: suiteName) } + defaults.set( + Data(#"{"isEnabled":false,"thresholdSeconds":60,"disabledKindIds":["query","backup"]}"#.utf8), + forKey: "com.TablePro.settings.notifications" + ) + + let settings = AppSettingsStorage(userDefaults: defaults).loadNotifications() + + #expect(settings == NotificationSettings(isEnabled: false, thresholdSeconds: 60)) } /// Automatic grammar agreement returns its own markup verbatim when the catalog has no entry From 843e6f28599ddf67db11d58a1833cb720544828e Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Mon, 12 Oct 2026 21:16:08 +0700 Subject: [PATCH 2/2] test(coordinator): fix the chrome snapshot build and re-record it --- TableProTests/Core/Menu/ChromeSnapshot.json | 52 +++++++++---------- .../Core/Menu/ChromeSnapshotTests.swift | 15 +----- 2 files changed, 27 insertions(+), 40 deletions(-) diff --git a/TableProTests/Core/Menu/ChromeSnapshot.json b/TableProTests/Core/Menu/ChromeSnapshot.json index 01f51f6556..687191d302 100644 --- a/TableProTests/Core/Menu/ChromeSnapshot.json +++ b/TableProTests/Core/Menu/ChromeSnapshot.json @@ -63,7 +63,6 @@ {"path": "File", "separator": true}, {"path": "File > Backup Dump…", "action": "backupDatabase:"}, {"path": "File > Restore Dump…", "action": "restoreDatabase:"}, - {"path": "File > Server-Side Export…", "action": "serverSideExport:"}, {"path": "Edit"}, {"path": "Edit > Undo", "identifier": "shortcut.undo", "action": "undo:", "key": "z", "modifiers": ["command"]}, {"path": "Edit > Redo", "identifier": "shortcut.redo", "action": "redo:", "key": "z", "modifiers": ["shift", "command"]}, @@ -96,35 +95,35 @@ {"path": "Edit", "separator": true}, {"path": "Edit > Restore Previous Values…", "identifier": "shortcut.restorePreviousValues", "action": "restorePreviousValues:"}, {"path": "Edit", "separator": true}, - {"path": "Edit > Data"}, - {"path": "Edit > Data > Insert Row Above", "action": "dataFileInsertRowAbove:"}, - {"path": "Edit > Data > Insert Row Below", "action": "dataFileInsertRowBelow:"}, - {"path": "Edit > Data > Delete Rows", "action": "dataFileDeleteSelectedRows:"}, + {"path": "Edit > Data", "identifier": "edit.data", "hidden": true}, + {"path": "Edit > Data > Insert Row Above", "action": "dataFileInsertRowAbove:", "hidden": true}, + {"path": "Edit > Data > Insert Row Below", "action": "dataFileInsertRowBelow:", "hidden": true}, + {"path": "Edit > Data > Delete Rows", "action": "dataFileDeleteSelectedRows:", "hidden": true}, {"path": "Edit > Data", "separator": true}, - {"path": "Edit > Data > Add Column…", "action": "dataFileAddColumn:"}, - {"path": "Edit > Data > Insert Column Left", "action": "dataFileInsertColumnLeft:"}, - {"path": "Edit > Data > Insert Column Right", "action": "dataFileInsertColumnRight:"}, - {"path": "Edit > Data > Rename Column…", "action": "dataFileRenameColumn:"}, - {"path": "Edit > Data > Split Column…", "action": "dataFileSplitColumn:"}, - {"path": "Edit > Data > Merge Columns…", "action": "dataFileMergeColumns:"}, - {"path": "Edit > Data > Delete Column", "action": "dataFileDeleteColumn:"}, + {"path": "Edit > Data > Add Column…", "action": "dataFileAddColumn:", "hidden": true}, + {"path": "Edit > Data > Insert Column Left", "action": "dataFileInsertColumnLeft:", "hidden": true}, + {"path": "Edit > Data > Insert Column Right", "action": "dataFileInsertColumnRight:", "hidden": true}, + {"path": "Edit > Data > Rename Column…", "action": "dataFileRenameColumn:", "hidden": true}, + {"path": "Edit > Data > Split Column…", "action": "dataFileSplitColumn:", "hidden": true}, + {"path": "Edit > Data > Merge Columns…", "action": "dataFileMergeColumns:", "hidden": true}, + {"path": "Edit > Data > Delete Column", "action": "dataFileDeleteColumn:", "hidden": true}, {"path": "Edit > Data", "separator": true}, - {"path": "Edit > Data > Fill Down", "action": "dataFileFillDown:"}, - {"path": "Edit > Data > Set Cells to Value…", "action": "dataFileSetCellsToValue:"}, - {"path": "Edit > Data > Trim Whitespace", "action": "dataFileTrimWhitespace:"}, - {"path": "Edit > Data > Change Case"}, - {"path": "Edit > Data > Change Case > UPPERCASE", "action": "dataFileChangeCase:"}, - {"path": "Edit > Data > Change Case > lowercase", "action": "dataFileChangeCase:"}, - {"path": "Edit > Data > Change Case > Title Case", "action": "dataFileChangeCase:"}, - {"path": "Edit > Data > Replace in Column…", "action": "dataFileReplaceInColumn:"}, - {"path": "Edit > Data > Remove Duplicate Rows…", "action": "dataFileRemoveDuplicates:"}, + {"path": "Edit > Data > Fill Down", "action": "dataFileFillDown:", "hidden": true}, + {"path": "Edit > Data > Set Cells to Value…", "action": "dataFileSetCellsToValue:", "hidden": true}, + {"path": "Edit > Data > Trim Whitespace", "action": "dataFileTrimWhitespace:", "hidden": true}, + {"path": "Edit > Data > Change Case", "hidden": true}, + {"path": "Edit > Data > Change Case > UPPERCASE", "action": "dataFileChangeCase:", "hidden": true}, + {"path": "Edit > Data > Change Case > lowercase", "action": "dataFileChangeCase:", "hidden": true}, + {"path": "Edit > Data > Change Case > Title Case", "action": "dataFileChangeCase:", "hidden": true}, + {"path": "Edit > Data > Replace in Column…", "action": "dataFileReplaceInColumn:", "hidden": true}, + {"path": "Edit > Data > Remove Duplicate Rows…", "action": "dataFileRemoveDuplicates:", "hidden": true}, {"path": "Edit > Data", "separator": true}, - {"path": "Edit > Data > Column Statistics…", "action": "dataFileShowStatistics:"}, - {"path": "Edit > Data > Search All Columns", "action": "dataFileSearchAllColumns:"}, + {"path": "Edit > Data > Column Statistics…", "action": "dataFileShowStatistics:", "hidden": true}, + {"path": "Edit > Data > Search All Columns", "action": "dataFileSearchAllColumns:", "hidden": true}, {"path": "Edit > Data", "separator": true}, - {"path": "Edit > Data > Switch First Row Between Header and Data", "identifier": "shortcut.toggleHeaderRow", "action": "dataFileToggleHeaderRow:", "key": "h", "modifiers": ["shift", "command"]}, - {"path": "Edit > Data > File Properties…", "action": "dataFileShowProperties:"}, - {"path": "Edit > Data > Import into Table…", "action": "dataFileImportIntoTable:"}, + {"path": "Edit > Data > Switch First Row Between Header and Data", "identifier": "shortcut.toggleHeaderRow", "action": "dataFileToggleHeaderRow:", "key": "h", "modifiers": ["shift", "command"], "hidden": true}, + {"path": "Edit > Data > File Properties…", "action": "dataFileShowProperties:", "hidden": true}, + {"path": "Edit > Data > Import into Table…", "action": "dataFileImportIntoTable:", "hidden": true}, {"path": "View"}, {"path": "View > Show Sidebar", "identifier": "shortcut.toggleTableBrowser", "action": "toggleSidebar:", "key": "0", "modifiers": ["command"]}, {"path": "View > Show Inspector", "identifier": "shortcut.toggleInspector", "action": "toggleInspector:", "key": "i", "modifiers": ["option", "command"]}, @@ -347,7 +346,6 @@ "com.TablePro.toolbar.results", "com.TablePro.toolbar.export", "com.TablePro.toolbar.import", - "com.TablePro.toolbar.dashboard", "com.TablePro.toolbar.history", "com.TablePro.toolbar.addRow", "com.TablePro.toolbar.restorePreviousValues", diff --git a/TableProTests/Core/Menu/ChromeSnapshotTests.swift b/TableProTests/Core/Menu/ChromeSnapshotTests.swift index 1da1440d16..f4f91676ff 100644 --- a/TableProTests/Core/Menu/ChromeSnapshotTests.swift +++ b/TableProTests/Core/Menu/ChromeSnapshotTests.swift @@ -94,20 +94,9 @@ private enum ChromeSnapshot { """ } - // MainMenuBuilder.build assigns NSApp's Services, Window and Help menus. The app's own are put - // back so the test host keeps the menu bar it launched with. private static func menuBarEntries() -> [String] { - let app = NSApplication.shared - let services = app.servicesMenu - let windows = app.windowsMenu - let help = app.helpMenu - defer { - app.servicesMenu = services - app.windowsMenu = windows - app.helpMenu = help - } - let menu = MainMenuBuilder.build(keyboard: KeyboardSettings()) - return entries(in: menu, path: [], appKitOwned: app.servicesMenu) + let built = MainMenuBuilder.build(keyboard: KeyboardSettings()) + return entries(in: built.bar, path: [], appKitOwned: built.servicesMenu) } // AppKit fills the Services menu and appends open windows to the Window menu, so neither is