fix: address PR review feedback for iCloud sync

Author: datlechin

TablePro.xcodeproj/project.pbxproj

@@ -1735,7 +1735,7 @@
 				AUTOMATION_APPLE_EVENTS = NO;
 				CODE_SIGN_ENTITLEMENTS = TablePro/TablePro.entitlements;
 				CODE_SIGN_IDENTITY = "Apple Development";
-				"CODE_SIGN_IDENTITY[sdk=macosx*]" = "Apple Development";
+				"CODE_SIGN_IDENTITY[sdk=macosx*]" = "-";
 				CODE_SIGN_STYLE = Automatic;
 				COPY_PHASE_STRIP = YES;
 				CURRENT_PROJECT_VERSION = 33;

TablePro/Core/Storage/AppSettingsStorage.swift

@@ -184,6 +184,7 @@ final class AppSettingsStorage {
         saveTabs(.default)
         saveKeyboard(.default)
         saveAI(.default)
+        saveSync(.default)
     }
 
     // MARK: - Helpers

TablePro/Core/Storage/ConnectionStorage.swift

@@ -60,16 +60,14 @@ final class ConnectionStorage {
         } catch {
             Self.logger.error("Failed to save connections: \(error)")
         }
-
-        // Mark all saved connections as dirty for sync
-        SyncChangeTracker.shared.markDirty(.connection, ids: connections.map { $0.id.uuidString })
     }
 
     /// Add a new connection
     func addConnection(_ connection: DatabaseConnection, password: String? = nil) {
         var connections = loadConnections()
         connections.append(connection)
         saveConnections(connections)
+        SyncChangeTracker.shared.markDirty(.connection, id: connection.id.uuidString)
 
         if let password = password, !password.isEmpty {
             savePassword(password, for: connection.id)
@@ -82,6 +80,7 @@ final class ConnectionStorage {
         if let index = connections.firstIndex(where: { $0.id == connection.id }) {
             connections[index] = connection
             saveConnections(connections)
+            SyncChangeTracker.shared.markDirty(.connection, id: connection.id.uuidString)
 
             if let password = password {
                 if password.isEmpty {
@@ -135,6 +134,7 @@ final class ConnectionStorage {
         var connections = loadConnections()
         connections.append(duplicate)
         saveConnections(connections)
+        SyncChangeTracker.shared.markDirty(.connection, id: duplicate.id.uuidString)
 
         // Copy all passwords from source to duplicate
         if let password = loadPassword(for: connection.id) {

TablePro/Core/Sync/CloudKitSyncEngine.swift

@@ -48,9 +48,15 @@ actor CloudKitSyncEngine {
         do {
             _ = try await database.save(zone)
             Self.logger.trace("Created or confirmed sync zone: \(Self.zoneName)")
-        } catch let error as CKError where error.code == .serverRejectedRequest {
-            // Zone already exists, which is fine
-            Self.logger.trace("Sync zone already exists")
+        } catch let error as CKError {
+            // Zone save can fail if zone already exists — that's fine.
+            // Only re-throw if the error is something else.
+            let ignorableCodes: Set<CKError.Code> = [.serverRejectedRequest]
+            if ignorableCodes.contains(error.code) {
+                Self.logger.trace("Sync zone already exists")
+            } else {
+                throw error
+            }
         }
     }
 

TablePro/Core/Sync/SyncChangeTracker.swift

@@ -29,7 +29,7 @@ final class SyncChangeTracker {
     func markDirty(_ type: SyncRecordType, id: String) {
         guard !isSuppressed else { return }
         metadataStorage.addDirty(type: type, id: id)
-        print("[Sync] Marked dirty: \(type.rawValue)/\(id)")
+        Self.logger.info("Marked dirty: \(type.rawValue)/\(id)")
         postChangeNotification()
     }
 

TablePro/Core/Sync/SyncCoordinator.swift

@@ -74,7 +74,7 @@ final class SyncCoordinator {
     /// Manual full sync (push then pull)
     func syncNow() async {
         guard canSync() else {
-            print("[Sync] syncNow: canSync() returned false, skipping")
+            Self.logger.info("syncNow: canSync() returned false, skipping")
             return
         }
 
@@ -108,14 +108,15 @@ final class SyncCoordinator {
 
     /// Called when user enables sync in settings
     func enableSync() {
-        print("[Sync] enableSync() called")
+        Self.logger.info("enableSync() called")
 
         // Clear token to force a full fetch on first sync after enabling
         metadataStorage.clearSyncToken()
 
         // Mark ALL existing local data as dirty so it gets pushed on first sync
         markAllLocalDataDirty()
-        print("[Sync] enableSync() dirty marking done, dirty connections: \(changeTracker.dirtyRecords(for: .connection))")
+        let dirtyCount = changeTracker.dirtyRecords(for: .connection).count
+        Self.logger.info("enableSync() dirty marking done, dirty connections: \(dirtyCount)")
 
         Task {
             await checkAccountStatus()
@@ -150,7 +151,7 @@ final class SyncCoordinator {
             changeTracker.markDirty(.settings, id: category)
         }
 
-        print("[Sync] Marked all local data dirty: \(connections.count) connections, \(groups.count) groups, \(tags.count) tags, 8 settings categories")
+        Self.logger.info("Marked all local data dirty: \(connections.count) connections, \(groups.count) groups, \(tags.count) tags, 8 settings categories")
     }
 
     /// Called when user disables sync in settings
@@ -222,7 +223,8 @@ final class SyncCoordinator {
         var recordsToSave: [CKRecord] = []
         var recordIDsToDelete: [CKRecord.ID] = []
         let zoneID = await engine.zoneID
-        print("[Sync] performPush: syncConnections=\(settings.syncConnections), dirty connections=\(changeTracker.dirtyRecords(for: .connection))")
+        let dirtyConnectionCount = changeTracker.dirtyRecords(for: .connection).count
+        Self.logger.info("performPush: syncConnections=\(settings.syncConnections), dirty connections=\(dirtyConnectionCount)")
 
         // Collect dirty connections
         if settings.syncConnections {
@@ -281,7 +283,7 @@ final class SyncCoordinator {
                 }
             }
 
-            print("[Sync] Push completed: \(recordsToSave.count) saved, \(recordIDsToDelete.count) deleted")
+            Self.logger.info("Push completed: \(recordsToSave.count) saved, \(recordIDsToDelete.count) deleted")
         } catch let error as CKError where error.code == .serverRecordChanged {
             Self.logger.warning("Server record changed during push — conflicts detected")
             handlePushConflicts(error)
@@ -294,15 +296,16 @@ final class SyncCoordinator {
 
     private func performPull() async {
         let token = metadataStorage.loadSyncToken()
-        print("[Sync] Pull starting, token: \(token == nil ? "nil (full fetch)" : "present (delta)")")
+        let tokenStatus = token == nil ? "nil (full fetch)" : "present (delta)"
+        Self.logger.info("Pull starting, token: \(tokenStatus)")
 
         do {
             let result = try await engine.pull(since: token)
 
-            print("[Sync] Pull fetched: \(result.changedRecords.count) changed, \(result.deletedRecordIDs.count) deleted")
+            Self.logger.info("Pull fetched: \(result.changedRecords.count) changed, \(result.deletedRecordIDs.count) deleted")
 
             for record in result.changedRecords {
-                print("[Sync]   Pulled record: \(record.recordType)/\(record.recordID.recordName)")
+                Self.logger.info("Pulled record: \(record.recordType)/\(record.recordID.recordName)")
             }
 
             if let newToken = result.newToken {
@@ -581,7 +584,6 @@ final class SyncCoordinator {
     }
 
     private func applySettingsData(_ data: Data, for category: String) {
-        let storage = AppSettingsStorage.shared
         let manager = AppSettingsManager.shared
         let decoder = JSONDecoder()
 

TablePro/Core/Sync/SyncRecordMapper.swift

@@ -104,7 +104,7 @@ struct SyncRecordMapper {
         }
 
         let host = record["host"] as? String ?? "localhost"
-        let port = (record["port"] as? Int64).map { Int($0) } ?? 3_306
+        let port = (record["port"] as? Int64).map { Int($0) } ?? 0
         let database = record["database"] as? String ?? ""
         let username = record["username"] as? String ?? ""
         let colorRaw = record["color"] as? String ?? ConnectionColor.none.rawValue

TablePro/Models/Settings/ProFeature.swift

@@ -8,7 +8,7 @@
 import Foundation
 
 /// Features that require a Pro (active) license
-enum ProFeature: String, CaseIterable {
+internal enum ProFeature: String, CaseIterable {
     case iCloudSync
 
     var displayName: String {
@@ -34,7 +34,7 @@ enum ProFeature: String, CaseIterable {
 }
 
 /// Result of checking Pro feature availability
-enum ProFeatureAccess {
+internal enum ProFeatureAccess {
     case available
     case unlicensed
     case expired

TablePro/Views/Connection/ConnectionFormView.swift

@@ -1025,13 +1025,15 @@ struct ConnectionFormView: View { // swiftlint:disable:this type_body_length
         if isNew {
             savedConnections.append(connectionToSave)
             storage.saveConnections(savedConnections)
+            SyncChangeTracker.shared.markDirty(.connection, id: connectionToSave.id.uuidString)
             // Close and connect to database
             NSApplication.shared.closeWindows(withId: "connection-form")
             connectToDatabase(connectionToSave)
         } else {
             if let index = savedConnections.firstIndex(where: { $0.id == connectionToSave.id }) {
                 savedConnections[index] = connectionToSave
                 storage.saveConnections(savedConnections)
+                SyncChangeTracker.shared.markDirty(.connection, id: connectionToSave.id.uuidString)
             }
             NSApplication.shared.closeWindows(withId: "connection-form")
             NotificationCenter.default.post(name: .connectionUpdated, object: nil)
@@ -1040,6 +1042,7 @@ struct ConnectionFormView: View { // swiftlint:disable:this type_body_length
 
     private func deleteConnection() {
         guard let id = connectionId else { return }
+        SyncChangeTracker.shared.markDeleted(.connection, id: id.uuidString)
         var savedConnections = storage.loadConnections()
         savedConnections.removeAll { $0.id == id }
         storage.saveConnections(savedConnections)