From 5626388c6c774f1200692436223aa1778a994d11 Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Thu, 14 Mar 2024 10:03:18 +0100 Subject: [PATCH 01/10] Switch to BSK from a branch --- DuckDuckGo.xcodeproj/project.pbxproj | 4 ++-- .../project.xcworkspace/xcshareddata/swiftpm/Package.resolved | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/DuckDuckGo.xcodeproj/project.pbxproj b/DuckDuckGo.xcodeproj/project.pbxproj index ccafd9b0dd..b41869b3c1 100644 --- a/DuckDuckGo.xcodeproj/project.pbxproj +++ b/DuckDuckGo.xcodeproj/project.pbxproj @@ -13949,8 +13949,8 @@ isa = XCRemoteSwiftPackageReference; repositoryURL = "https://github.com/duckduckgo/BrowserServicesKit"; requirement = { - kind = exactVersion; - version = 126.1.0; + branch = "dominik/sync-field-validation"; + kind = branch; }; }; AA06B6B52672AF8100F541C5 /* XCRemoteSwiftPackageReference "Sparkle" */ = { diff --git a/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved b/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved index 7a11e2163c..14ad334696 100644 --- a/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved +++ b/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved @@ -32,8 +32,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/duckduckgo/BrowserServicesKit", "state" : { - "revision" : "f4894b9c00dd7514c66d6b929c12315e0cd9c151", - "version" : "126.1.0" + "branch" : "dominik/sync-field-validation", + "revision" : "9a323f6c60bc4ba5d7f62a404014cdec0de5d37f" } }, { From 8341eaa6e9b3913aa748c46e81e7fc89053e5d1c Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Thu, 14 Mar 2024 17:27:58 +0100 Subject: [PATCH 02/10] Show info about invalid bookmarks and credentials that failed to be synced --- .../xcshareddata/swiftpm/Package.resolved | 2 +- .../Preferences/Model/SyncPreferences.swift | 26 +++++++ .../View/PreferencesRootView.swift | 11 +-- DuckDuckGo/Sync/SyncBookmarksAdapter.swift | 3 +- DuckDuckGo/Sync/SyncCredentialsAdapter.swift | 1 + DuckDuckGo/Sync/SyncSettingsAdapter.swift | 1 + .../SyncUI/Resources/Localizable.xcstrings | 72 +++++++++++++++++++ .../ViewModels/ManagementViewModel.swift | 3 + .../ManagementView/SyncEnabledView.swift | 42 +++++++++++ .../Sources/SyncUI/internal/UserText.swift | 42 +++++++++++ 10 files changed, 197 insertions(+), 6 deletions(-) diff --git a/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved b/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved index 14ad334696..6f61da0fd3 100644 --- a/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved +++ b/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved @@ -33,7 +33,7 @@ "location" : "https://github.com/duckduckgo/BrowserServicesKit", "state" : { "branch" : "dominik/sync-field-validation", - "revision" : "9a323f6c60bc4ba5d7f62a404014cdec0de5d37f" + "revision" : "614537e0fbcfb2446182c97ad1e99b5ccbf716a1" } }, { diff --git a/DuckDuckGo/Preferences/Model/SyncPreferences.swift b/DuckDuckGo/Preferences/Model/SyncPreferences.swift index 6c2df2e7ac..2560b41a7b 100644 --- a/DuckDuckGo/Preferences/Model/SyncPreferences.swift +++ b/DuckDuckGo/Preferences/Model/SyncPreferences.swift @@ -39,6 +39,8 @@ extension SyncDevice { } final class SyncPreferences: ObservableObject, SyncUI.ManagementViewModel { + @Published var invalidBookmarksTitles: [String] = [] + @Published var invalidCredentialsTitles: [String] = [] struct Consts { static let syncPausedStateChanged = Notification.Name("com.duckduckgo.app.SyncPausedStateChanged") @@ -117,12 +119,14 @@ final class SyncPreferences: ObservableObject, SyncUI.ManagementViewModel { init( syncService: DDGSyncing, syncBookmarksAdapter: SyncBookmarksAdapter, + syncCredentialsAdapter: SyncCredentialsAdapter, appearancePreferences: AppearancePreferences = .shared, managementDialogModel: ManagementDialogModel = ManagementDialogModel(), userAuthenticator: UserAuthenticating = DeviceAuthenticator.shared ) { self.syncService = syncService self.syncBookmarksAdapter = syncBookmarksAdapter + self.syncCredentialsAdapter = syncCredentialsAdapter self.appearancePreferences = appearancePreferences self.syncFeatureFlags = syncService.featureFlags self.userAuthenticator = userAuthenticator @@ -138,6 +142,17 @@ final class SyncPreferences: ObservableObject, SyncUI.ManagementViewModel { updateSyncFeatureFlags(self.syncFeatureFlags) setUpObservables() setUpSyncOptionsObservables(apperancePreferences: appearancePreferences) + + updateInvalidObjects() + } + + private func updateInvalidObjects() { + invalidBookmarksTitles = syncBookmarksAdapter.provider? + .fetchTitlesForObjectsThatFailedValidation() + .map { $0.truncated(length: 15) } ?? [] + + let invalidCredentialsObjects: [String] = (try? syncCredentialsAdapter.provider?.fetchTitlesForObjectsThatFailedValidation()) ?? [] + invalidCredentialsTitles = invalidCredentialsObjects.map({ $0.truncated(length: 15) }) } private func setUpObservables() { @@ -157,6 +172,16 @@ final class SyncPreferences: ObservableObject, SyncUI.ManagementViewModel { } .store(in: &cancellables) + syncService.isSyncInProgressPublisher + .removeDuplicates() + .filter { !$0 } + .asVoid() + .receive(on: DispatchQueue.main) + .sink { [weak self] in + self?.updateInvalidObjects() + } + .store(in: &cancellables) + $syncErrorMessage .map { $0 != nil } .receive(on: DispatchQueue.main) @@ -351,6 +376,7 @@ final class SyncPreferences: ObservableObject, SyncUI.ManagementViewModel { private let syncService: DDGSyncing private let syncBookmarksAdapter: SyncBookmarksAdapter + private let syncCredentialsAdapter: SyncCredentialsAdapter private let appearancePreferences: AppearancePreferences private var cancellables = Set() private var connector: RemoteConnecting? diff --git a/DuckDuckGo/Preferences/View/PreferencesRootView.swift b/DuckDuckGo/Preferences/View/PreferencesRootView.swift index 76c6d00af1..0269395452 100644 --- a/DuckDuckGo/Preferences/View/PreferencesRootView.swift +++ b/DuckDuckGo/Preferences/View/PreferencesRootView.swift @@ -173,10 +173,13 @@ struct SyncView: View { var body: some View { if let syncService = NSApp.delegateTyped.syncService, let syncDataProviders = NSApp.delegateTyped.syncDataProviders { - SyncUI.ManagementView(model: SyncPreferences(syncService: syncService, syncBookmarksAdapter: syncDataProviders.bookmarksAdapter)) - .onAppear { - requestSync() - } + let syncPreferences = SyncPreferences( + syncService: syncService, + syncBookmarksAdapter: syncDataProviders.bookmarksAdapter, + syncCredentialsAdapter: syncDataProviders.credentialsAdapter + ) + SyncUI.ManagementView(model: syncPreferences) + .onAppear { requestSync() } } else { FailedAssertionView("Failed to initialize Sync Management View") } diff --git a/DuckDuckGo/Sync/SyncBookmarksAdapter.swift b/DuckDuckGo/Sync/SyncBookmarksAdapter.swift index 8d90de82f8..d7a9651d14 100644 --- a/DuckDuckGo/Sync/SyncBookmarksAdapter.swift +++ b/DuckDuckGo/Sync/SyncBookmarksAdapter.swift @@ -112,6 +112,7 @@ final class SyncBookmarksAdapter { database: database, metadataStore: metadataStore, metricsEvents: metricsEventsHandler, + log: OSLog.sync, syncDidUpdateData: { [weak self] in LocalBookmarkManager.shared.loadBookmarks() self?.isSyncBookmarksPaused = false @@ -132,7 +133,7 @@ final class SyncBookmarksAdapter { if !didMigrateToImprovedListsHandling { didMigrateToImprovedListsHandling = true - provider.lastSyncTimestamp = nil + provider.updateSyncTimestamps(server: nil, local: nil) } bindSyncErrorPublisher(provider) diff --git a/DuckDuckGo/Sync/SyncCredentialsAdapter.swift b/DuckDuckGo/Sync/SyncCredentialsAdapter.swift index 4eec486d2b..444e7a1872 100644 --- a/DuckDuckGo/Sync/SyncCredentialsAdapter.swift +++ b/DuckDuckGo/Sync/SyncCredentialsAdapter.swift @@ -73,6 +73,7 @@ final class SyncCredentialsAdapter { secureVaultErrorReporter: SecureVaultErrorReporter.shared, metadataStore: metadataStore, metricsEvents: metricsEventsHandler, + log: OSLog.sync, syncDidUpdateData: { [weak self] in self?.syncDidCompleteSubject.send() self?.isSyncCredentialsPaused = false diff --git a/DuckDuckGo/Sync/SyncSettingsAdapter.swift b/DuckDuckGo/Sync/SyncSettingsAdapter.swift index d5939b7ac9..96fc030a0d 100644 --- a/DuckDuckGo/Sync/SyncSettingsAdapter.swift +++ b/DuckDuckGo/Sync/SyncSettingsAdapter.swift @@ -53,6 +53,7 @@ final class SyncSettingsAdapter { metadataStore: metadataStore, settingsHandlers: [FavoritesDisplayModeSyncHandler(), EmailProtectionSyncHandler(emailManager: emailManager)], metricsEvents: metricsEventsHandler, + log: OSLog.sync, syncDidUpdateData: { [weak self] in self?.syncDidCompleteSubject.send() } diff --git a/LocalPackages/SyncUI/Sources/SyncUI/Resources/Localizable.xcstrings b/LocalPackages/SyncUI/Sources/SyncUI/Resources/Localizable.xcstrings index 230beb36fa..994dd8b787 100644 --- a/LocalPackages/SyncUI/Sources/SyncUI/Resources/Localizable.xcstrings +++ b/LocalPackages/SyncUI/Sources/SyncUI/Resources/Localizable.xcstrings @@ -4739,6 +4739,78 @@ } } }, + "prefrences.sync.invalid-bookmarks-present-description-many" : { + "comment" : "Alert message for multiple invalid bookmark being filtered out of synced data", + "extractionState" : "extracted_with_value", + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "new", + "value" : "Your bookmarks for %1$@ and other sites (%2$@) can't sync because some of their fields exceed the character limit." + } + } + } + }, + "prefrences.sync.invalid-bookmarks-present-description-one" : { + "comment" : "Alert message for 1 invalid bookmark being filtered out of synced data", + "extractionState" : "extracted_with_value", + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "new", + "value" : "Your bookmark for %@ can't sync because one of its fields exceeds the character limit." + } + } + } + }, + "prefrences.sync.invalid-bookmarks-present-title" : { + "comment" : "Alert title for invalid bookmarks being filtered out of synced data", + "extractionState" : "extracted_with_value", + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "new", + "value" : "Some bookmarks are not syncing due to excessively long content in certain fields." + } + } + } + }, + "prefrences.sync.invalid-credentials-present-description-many" : { + "comment" : "Alert message for multiple invalid logins being filtered out of synced data", + "extractionState" : "extracted_with_value", + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "new", + "value" : "Your passwords for %1$@ and other sites (%2$@) can't sync because some of their fields exceed the character limit." + } + } + } + }, + "prefrences.sync.invalid-credentials-present-description-one" : { + "comment" : "Alert message for 1 invalid login being filtered out of synced data", + "extractionState" : "extracted_with_value", + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "new", + "value" : "Your password for %@ can't sync because one of its fields exceeds the character limit." + } + } + } + }, + "prefrences.sync.invalid-credentials-present-title" : { + "comment" : "Alert title for invalid logins being filtered out of synced data", + "extractionState" : "extracted_with_value", + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "new", + "value" : "Some logins are not syncing due to excessively long content in certain fields." + } + } + } + }, "prefrences.sync.keep-favicons-updated" : { "comment" : "Title of the confirmation button for favicons fetching", "extractionState" : "extracted_with_value", diff --git a/LocalPackages/SyncUI/Sources/SyncUI/ViewModels/ManagementViewModel.swift b/LocalPackages/SyncUI/Sources/SyncUI/ViewModels/ManagementViewModel.swift index 26580aee97..0ad1a50478 100644 --- a/LocalPackages/SyncUI/Sources/SyncUI/ViewModels/ManagementViewModel.swift +++ b/LocalPackages/SyncUI/Sources/SyncUI/ViewModels/ManagementViewModel.swift @@ -33,6 +33,9 @@ public protocol ManagementViewModel: ObservableObject { var isSyncBookmarksPaused: Bool { get } var isSyncCredentialsPaused: Bool { get } + var invalidBookmarksTitles: [String] { get } + var invalidCredentialsTitles: [String] { get } + var recoveryCode: String? { get } var codeToDisplay: String? { get } var devices: [SyncDevice] { get } diff --git a/LocalPackages/SyncUI/Sources/SyncUI/Views/ManagementView/SyncEnabledView.swift b/LocalPackages/SyncUI/Sources/SyncUI/Views/ManagementView/SyncEnabledView.swift index f11064d54f..ac251788dd 100644 --- a/LocalPackages/SyncUI/Sources/SyncUI/Views/ManagementView/SyncEnabledView.swift +++ b/LocalPackages/SyncUI/Sources/SyncUI/Views/ManagementView/SyncEnabledView.swift @@ -32,6 +32,12 @@ struct SyncEnabledView: View where ViewModel: ManagementViewModel { if model.isSyncCredentialsPaused { syncPaused(for: .credentials) } + if !model.invalidBookmarksTitles.isEmpty { + syncHasInvalidItems(for: .bookmarks) + } + if !model.invalidCredentialsTitles.isEmpty { + syncHasInvalidItems(for: .credentials) + } } // Sync Enabled @@ -107,6 +113,42 @@ struct SyncEnabledView: View where ViewModel: ManagementViewModel { } } + @ViewBuilder + func syncHasInvalidItems(for itemType: LimitedItemType) -> some View { + var title: String { + switch itemType { + case .bookmarks: + return UserText.invalidBookmarksPresentTitle + case .credentials: + return UserText.invalidCredentialsPresentTitle + } + } + var description: String { + switch itemType { + case .bookmarks: + return UserText.invalidBookmarksPresentDescription(model.invalidBookmarksTitles) + case .credentials: + return UserText.invalidCredentialsPresentDescription(model.invalidCredentialsTitles) + } + } + var actionTitle: String { + switch itemType { + case .bookmarks: + return UserText.bookmarksLimitExceededAction + case .credentials: + return UserText.credentialsLimitExceededAction + } + } + SyncWarningMessage(title: title, message: description, buttonTitle: actionTitle) { + switch itemType { + case .bookmarks: + model.manageBookmarks() + case .credentials: + model.manageLogins() + } + } + } + @ViewBuilder fileprivate func syncUnavailableView() -> some View { if model.isDataSyncingAvailable { diff --git a/LocalPackages/SyncUI/Sources/SyncUI/internal/UserText.swift b/LocalPackages/SyncUI/Sources/SyncUI/internal/UserText.swift index 3d9356af3e..c2b9c9203a 100644 --- a/LocalPackages/SyncUI/Sources/SyncUI/internal/UserText.swift +++ b/LocalPackages/SyncUI/Sources/SyncUI/internal/UserText.swift @@ -148,6 +148,48 @@ enum UserText { static let credentialsLimitExceededDescription = NSLocalizedString("prefrences.sync.credentials-limit-exceeded-description", bundle: Bundle.module, value: "Logins limit exceeded. Delete some to resume syncing.", comment: "Description for sync credentials limits exceeded warning") static let bookmarksLimitExceededAction = NSLocalizedString("prefrences.sync.bookmarks-limit-exceeded-action", bundle: Bundle.module, value: "Manage Bookmarks", comment: "Button title for sync bookmarks limits exceeded warning to go to manage bookmarks") static let credentialsLimitExceededAction = NSLocalizedString("prefrences.sync.credentials-limit-exceeded-action", bundle: Bundle.module, value: "Manage passwords…", comment: "Button title for sync credentials limits exceeded warning to go to manage passwords") + static let invalidBookmarksPresentTitle = NSLocalizedString("prefrences.sync.invalid-bookmarks-present-title", bundle: Bundle.module, value: "Some bookmarks are not syncing due to excessively long content in certain fields.", comment: "Alert title for invalid bookmarks being filtered out of synced data") + static let invalidCredentialsPresentTitle = NSLocalizedString("prefrences.sync.invalid-credentials-present-title", bundle: Bundle.module, value: "Some logins are not syncing due to excessively long content in certain fields.", comment: "Alert title for invalid logins being filtered out of synced data") + static func invalidBookmarksPresentDescription(_ invalidItemsTitles: [String]) -> String { + assert(!invalidItemsTitles.isEmpty) + let localized: String = { + if invalidItemsTitles.count == 1 { + return NSLocalizedString( + "prefrences.sync.invalid-bookmarks-present-description-one", + bundle: Bundle.module, + value: "Your bookmark for %@ can't sync because one of its fields exceeds the character limit.", + comment: "Alert message for 1 invalid bookmark being filtered out of synced data" + ) + } + return NSLocalizedString( + "prefrences.sync.invalid-bookmarks-present-description-many", + bundle: Bundle.module, + value: "Your bookmarks for %@ and other sites (%@) can't sync because some of their fields exceed the character limit.", + comment: "Alert message for multiple invalid bookmark being filtered out of synced data" + ) + }() + return String(format: localized, invalidItemsTitles.first ?? "", invalidItemsTitles.count - 1) + } + static func invalidCredentialsPresentDescription(_ invalidItemsTitles: [String]) -> String { + assert(!invalidItemsTitles.isEmpty) + let localized: String = { + if invalidItemsTitles.count == 1 { + return NSLocalizedString( + "prefrences.sync.invalid-credentials-present-description-one", + bundle: Bundle.module, + value: "Your password for %@ can't sync because one of its fields exceeds the character limit.", + comment: "Alert message for 1 invalid login being filtered out of synced data" + ) + } + return NSLocalizedString( + "prefrences.sync.invalid-credentials-present-description-many", + bundle: Bundle.module, + value: "Your passwords for %@ and other sites (%@) can't sync because some of their fields exceed the character limit.", + comment: "Alert message for multiple invalid logins being filtered out of synced data" + ) + }() + return String(format: localized, invalidItemsTitles.first ?? "", invalidItemsTitles.count - 1) + } static let syncErrorAlertTitle = NSLocalizedString("alert.sync-error", bundle: Bundle.module, value: "Sync & Backup Error", comment: "Title for sync error alert") static let unableToSyncToServerDescription = NSLocalizedString("alert.unable-to-sync-to-server-description", bundle: Bundle.module, value: "Unable to connect to the server.", comment: "Description for unable to sync to server error") static let unableToSyncWithAnotherDeviceDescription = NSLocalizedString("alert.unable-to-sync-with-another-device-description", bundle: Bundle.module, value: "Unable to Sync with another device.", comment: "Description for unable to sync with another device error") From f5a190a9fa45bfe3839d4ad0ef5564ffc8946984 Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Mon, 18 Mar 2024 08:31:44 +0100 Subject: [PATCH 03/10] Update Package.resolved --- .../project.xcworkspace/xcshareddata/swiftpm/Package.resolved | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved b/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved index 6f61da0fd3..68fc2e89c3 100644 --- a/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved +++ b/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved @@ -33,7 +33,7 @@ "location" : "https://github.com/duckduckgo/BrowserServicesKit", "state" : { "branch" : "dominik/sync-field-validation", - "revision" : "614537e0fbcfb2446182c97ad1e99b5ccbf716a1" + "revision" : "41589a8c5551a3e116775eb30b76dacca7421848" } }, { From b7981c97179ebc41217d382783beb6e849bf0ef3 Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Mon, 18 Mar 2024 09:32:10 +0100 Subject: [PATCH 04/10] Update naming --- DuckDuckGo/Preferences/Model/SyncPreferences.swift | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/DuckDuckGo/Preferences/Model/SyncPreferences.swift b/DuckDuckGo/Preferences/Model/SyncPreferences.swift index 2560b41a7b..7ed5ea10c9 100644 --- a/DuckDuckGo/Preferences/Model/SyncPreferences.swift +++ b/DuckDuckGo/Preferences/Model/SyncPreferences.swift @@ -148,13 +148,14 @@ final class SyncPreferences: ObservableObject, SyncUI.ManagementViewModel { private func updateInvalidObjects() { invalidBookmarksTitles = syncBookmarksAdapter.provider? - .fetchTitlesForObjectsThatFailedValidation() + .fetchDescriptionsForObjectsThatFailedValidation() .map { $0.truncated(length: 15) } ?? [] - let invalidCredentialsObjects: [String] = (try? syncCredentialsAdapter.provider?.fetchTitlesForObjectsThatFailedValidation()) ?? [] + let invalidCredentialsObjects: [String] = (try? syncCredentialsAdapter.provider?.fetchDescriptionsForObjectsThatFailedValidation()) ?? [] invalidCredentialsTitles = invalidCredentialsObjects.map({ $0.truncated(length: 15) }) } + // swiftlint:disable:next function_body_length private func setUpObservables() { syncService.featureFlagsPublisher .dropFirst() From 0f88400fa0b53e641817006b4a7e95d34d5299a3 Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Mon, 18 Mar 2024 15:56:52 +0100 Subject: [PATCH 05/10] Update user strings --- .../xcshareddata/swiftpm/Package.resolved | 2 +- .../Preferences/Model/SyncPreferences.swift | 5 +++-- .../SyncUI/Resources/Localizable.xcstrings | 4 ++-- .../ManagementView/SyncEnabledView.swift | 9 ++++++-- .../Sources/SyncUI/internal/UserText.swift | 21 ++++++++++--------- 5 files changed, 24 insertions(+), 17 deletions(-) diff --git a/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved b/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved index 68fc2e89c3..9a13c9300d 100644 --- a/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved +++ b/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved @@ -33,7 +33,7 @@ "location" : "https://github.com/duckduckgo/BrowserServicesKit", "state" : { "branch" : "dominik/sync-field-validation", - "revision" : "41589a8c5551a3e116775eb30b76dacca7421848" + "revision" : "987b93da5aff25b2d010d463f3359245c47936dd" } }, { diff --git a/DuckDuckGo/Preferences/Model/SyncPreferences.swift b/DuckDuckGo/Preferences/Model/SyncPreferences.swift index 7ed5ea10c9..d35d78c0fd 100644 --- a/DuckDuckGo/Preferences/Model/SyncPreferences.swift +++ b/DuckDuckGo/Preferences/Model/SyncPreferences.swift @@ -39,8 +39,6 @@ extension SyncDevice { } final class SyncPreferences: ObservableObject, SyncUI.ManagementViewModel { - @Published var invalidBookmarksTitles: [String] = [] - @Published var invalidCredentialsTitles: [String] = [] struct Consts { static let syncPausedStateChanged = Notification.Name("com.duckduckgo.app.SyncPausedStateChanged") @@ -88,6 +86,9 @@ final class SyncPreferences: ObservableObject, SyncUI.ManagementViewModel { @Published var isSyncCredentialsPaused: Bool + @Published var invalidBookmarksTitles: [String] = [] + @Published var invalidCredentialsTitles: [String] = [] + private var shouldRequestSyncOnFavoritesOptionChange: Bool = true private var isScreenLocked: Bool = false private var recoveryKey: SyncCode.RecoveryKey? diff --git a/LocalPackages/SyncUI/Sources/SyncUI/Resources/Localizable.xcstrings b/LocalPackages/SyncUI/Sources/SyncUI/Resources/Localizable.xcstrings index 994dd8b787..7b94029086 100644 --- a/LocalPackages/SyncUI/Sources/SyncUI/Resources/Localizable.xcstrings +++ b/LocalPackages/SyncUI/Sources/SyncUI/Resources/Localizable.xcstrings @@ -4746,7 +4746,7 @@ "en" : { "stringUnit" : { "state" : "new", - "value" : "Your bookmarks for %1$@ and other sites (%2$@) can't sync because some of their fields exceed the character limit." + "value" : "Your bookmarks for %1$@ and other sites (%2$d) can't sync because some of their fields exceed the character limit." } } } @@ -4782,7 +4782,7 @@ "en" : { "stringUnit" : { "state" : "new", - "value" : "Your passwords for %1$@ and other sites (%2$@) can't sync because some of their fields exceed the character limit." + "value" : "Your passwords for %1$@ and other sites (%2$d) can't sync because some of their fields exceed the character limit." } } } diff --git a/LocalPackages/SyncUI/Sources/SyncUI/Views/ManagementView/SyncEnabledView.swift b/LocalPackages/SyncUI/Sources/SyncUI/Views/ManagementView/SyncEnabledView.swift index ac251788dd..1255fdf962 100644 --- a/LocalPackages/SyncUI/Sources/SyncUI/Views/ManagementView/SyncEnabledView.swift +++ b/LocalPackages/SyncUI/Sources/SyncUI/Views/ManagementView/SyncEnabledView.swift @@ -126,9 +126,14 @@ struct SyncEnabledView: View where ViewModel: ManagementViewModel { var description: String { switch itemType { case .bookmarks: - return UserText.invalidBookmarksPresentDescription(model.invalidBookmarksTitles) + assert(!model.invalidBookmarksTitles.isEmpty) + let firstInvalidBookmarkTitle = model.invalidBookmarksTitles.first ?? "" + return UserText.invalidBookmarksPresentDescription(firstInvalidBookmarkTitle, numberOfOtherInvalidItems: model.invalidBookmarksTitles.count - 1) + case .credentials: - return UserText.invalidCredentialsPresentDescription(model.invalidCredentialsTitles) + assert(!model.invalidCredentialsTitles.isEmpty) + let firstInvalidCredentialTitle = model.invalidCredentialsTitles.first ?? "" + return UserText.invalidCredentialsPresentDescription(firstInvalidCredentialTitle, numberOfOtherInvalidItems: model.invalidCredentialsTitles.count - 1) } } var actionTitle: String { diff --git a/LocalPackages/SyncUI/Sources/SyncUI/internal/UserText.swift b/LocalPackages/SyncUI/Sources/SyncUI/internal/UserText.swift index c2b9c9203a..1fee316481 100644 --- a/LocalPackages/SyncUI/Sources/SyncUI/internal/UserText.swift +++ b/LocalPackages/SyncUI/Sources/SyncUI/internal/UserText.swift @@ -150,10 +150,10 @@ enum UserText { static let credentialsLimitExceededAction = NSLocalizedString("prefrences.sync.credentials-limit-exceeded-action", bundle: Bundle.module, value: "Manage passwords…", comment: "Button title for sync credentials limits exceeded warning to go to manage passwords") static let invalidBookmarksPresentTitle = NSLocalizedString("prefrences.sync.invalid-bookmarks-present-title", bundle: Bundle.module, value: "Some bookmarks are not syncing due to excessively long content in certain fields.", comment: "Alert title for invalid bookmarks being filtered out of synced data") static let invalidCredentialsPresentTitle = NSLocalizedString("prefrences.sync.invalid-credentials-present-title", bundle: Bundle.module, value: "Some logins are not syncing due to excessively long content in certain fields.", comment: "Alert title for invalid logins being filtered out of synced data") - static func invalidBookmarksPresentDescription(_ invalidItemsTitles: [String]) -> String { - assert(!invalidItemsTitles.isEmpty) + + static func invalidBookmarksPresentDescription(_ invalidItemTitle: String, numberOfOtherInvalidItems: Int) -> String { let localized: String = { - if invalidItemsTitles.count == 1 { + guard numberOfOtherInvalidItems > 0 else { return NSLocalizedString( "prefrences.sync.invalid-bookmarks-present-description-one", bundle: Bundle.module, @@ -164,16 +164,16 @@ enum UserText { return NSLocalizedString( "prefrences.sync.invalid-bookmarks-present-description-many", bundle: Bundle.module, - value: "Your bookmarks for %@ and other sites (%@) can't sync because some of their fields exceed the character limit.", + value: "Your bookmarks for %@ and other sites (%d) can't sync because some of their fields exceed the character limit.", comment: "Alert message for multiple invalid bookmark being filtered out of synced data" ) }() - return String(format: localized, invalidItemsTitles.first ?? "", invalidItemsTitles.count - 1) + return String(format: localized, invalidItemTitle, numberOfOtherInvalidItems) } - static func invalidCredentialsPresentDescription(_ invalidItemsTitles: [String]) -> String { - assert(!invalidItemsTitles.isEmpty) + + static func invalidCredentialsPresentDescription(_ invalidItemTitle: String, numberOfOtherInvalidItems: Int) -> String { let localized: String = { - if invalidItemsTitles.count == 1 { + guard numberOfOtherInvalidItems > 0 else { return NSLocalizedString( "prefrences.sync.invalid-credentials-present-description-one", bundle: Bundle.module, @@ -184,12 +184,13 @@ enum UserText { return NSLocalizedString( "prefrences.sync.invalid-credentials-present-description-many", bundle: Bundle.module, - value: "Your passwords for %@ and other sites (%@) can't sync because some of their fields exceed the character limit.", + value: "Your passwords for %@ and other sites (%d) can't sync because some of their fields exceed the character limit.", comment: "Alert message for multiple invalid logins being filtered out of synced data" ) }() - return String(format: localized, invalidItemsTitles.first ?? "", invalidItemsTitles.count - 1) + return String(format: localized, invalidItemTitle, numberOfOtherInvalidItems) } + static let syncErrorAlertTitle = NSLocalizedString("alert.sync-error", bundle: Bundle.module, value: "Sync & Backup Error", comment: "Title for sync error alert") static let unableToSyncToServerDescription = NSLocalizedString("alert.unable-to-sync-to-server-description", bundle: Bundle.module, value: "Unable to connect to the server.", comment: "Description for unable to sync to server error") static let unableToSyncWithAnotherDeviceDescription = NSLocalizedString("alert.unable-to-sync-with-another-device-description", bundle: Bundle.module, value: "Unable to Sync with another device.", comment: "Description for unable to sync with another device error") From 170923df198474d676e8616cc5ec9d7dadbe6618 Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Tue, 19 Mar 2024 16:28:54 +0100 Subject: [PATCH 06/10] Honor feature flag --- .../xcshareddata/swiftpm/Package.resolved | 2 +- .../Preferences/Model/SyncPreferences.swift | 22 +++++++++---------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved b/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved index 9a13c9300d..f087473a43 100644 --- a/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved +++ b/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved @@ -33,7 +33,7 @@ "location" : "https://github.com/duckduckgo/BrowserServicesKit", "state" : { "branch" : "dominik/sync-field-validation", - "revision" : "987b93da5aff25b2d010d463f3359245c47936dd" + "revision" : "2f979f9a7c9c3736b4490682dbb531eb06666f79" } }, { diff --git a/DuckDuckGo/Preferences/Model/SyncPreferences.swift b/DuckDuckGo/Preferences/Model/SyncPreferences.swift index d35d78c0fd..28be6ba74d 100644 --- a/DuckDuckGo/Preferences/Model/SyncPreferences.swift +++ b/DuckDuckGo/Preferences/Model/SyncPreferences.swift @@ -143,8 +143,6 @@ final class SyncPreferences: ObservableObject, SyncUI.ManagementViewModel { updateSyncFeatureFlags(self.syncFeatureFlags) setUpObservables() setUpSyncOptionsObservables(apperancePreferences: appearancePreferences) - - updateInvalidObjects() } private func updateInvalidObjects() { @@ -174,15 +172,17 @@ final class SyncPreferences: ObservableObject, SyncUI.ManagementViewModel { } .store(in: &cancellables) - syncService.isSyncInProgressPublisher - .removeDuplicates() - .filter { !$0 } - .asVoid() - .receive(on: DispatchQueue.main) - .sink { [weak self] in - self?.updateInvalidObjects() - } - .store(in: &cancellables) + if DDGSync.isFieldValidationEnabled { + syncService.isSyncInProgressPublisher + .removeDuplicates() + .filter { !$0 } + .asVoid() + .receive(on: DispatchQueue.main) + .sink { [weak self] in + self?.updateInvalidObjects() + } + .store(in: &cancellables) + } $syncErrorMessage .map { $0 != nil } From 23386cdb5a9b607de71d278928553b540e92ae8b Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Tue, 19 Mar 2024 17:05:10 +0100 Subject: [PATCH 07/10] Update BSK ref --- .../project.xcworkspace/xcshareddata/swiftpm/Package.resolved | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved b/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved index f087473a43..6928f83343 100644 --- a/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved +++ b/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved @@ -33,7 +33,7 @@ "location" : "https://github.com/duckduckgo/BrowserServicesKit", "state" : { "branch" : "dominik/sync-field-validation", - "revision" : "2f979f9a7c9c3736b4490682dbb531eb06666f79" + "revision" : "cdeed8ae54b3b8558ccf8bf5964476c81cb095a0" } }, { From 2201517876b0bcc1ac06746b408243469e7f0eae Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Wed, 20 Mar 2024 00:32:18 +0100 Subject: [PATCH 08/10] Fix tests compilation --- UnitTests/DataExport/MockSecureVault.swift | 8 ++++++++ UnitTests/Sync/SyncPreferencesTests.swift | 3 +++ 2 files changed, 11 insertions(+) diff --git a/UnitTests/DataExport/MockSecureVault.swift b/UnitTests/DataExport/MockSecureVault.swift index d9dd3ad32d..d0242c9b3e 100644 --- a/UnitTests/DataExport/MockSecureVault.swift +++ b/UnitTests/DataExport/MockSecureVault.swift @@ -174,6 +174,10 @@ final class MockSecureVault: AutofillSecureVault { [] } + func accountTitlesForSyncableCredentials(modifiedBefore date: Date) throws -> [String] { + [] + } + func deleteSyncableCredentials(_ syncableCredentials: SecureVaultModels.SyncableCredentials, in database: Database) throws { if let accountId = syncableCredentials.metadata.objectId { try deleteWebsiteCredentialsFor(accountId: accountId) @@ -413,6 +417,10 @@ class MockDatabaseProvider: AutofillDatabaseProvider { [] } + func modifiedSyncableCredentials(before date: Date) throws -> [SecureVaultModels.SyncableCredentials] { + [] + } + func syncableCredentialsForSyncIds(_ syncIds: any Sequence, in database: Database) throws -> [SecureVaultModels.SyncableCredentials] { [] } diff --git a/UnitTests/Sync/SyncPreferencesTests.swift b/UnitTests/Sync/SyncPreferencesTests.swift index 76cfc93428..1a0557f5b3 100644 --- a/UnitTests/Sync/SyncPreferencesTests.swift +++ b/UnitTests/Sync/SyncPreferencesTests.swift @@ -41,6 +41,7 @@ final class SyncPreferencesTests: XCTestCase { let managementDialogModel = ManagementDialogModel() var ddgSyncing: MockDDGSyncing! var syncBookmarksAdapter: SyncBookmarksAdapter! + var syncCredentialsAdapter: SyncCredentialsAdapter! var appearancePersistor = MockPersistor() var appearancePreferences: AppearancePreferences! var syncPreferences: SyncPreferences! @@ -55,10 +56,12 @@ final class SyncPreferencesTests: XCTestCase { ddgSyncing = MockDDGSyncing(authState: .inactive, scheduler: scheduler, isSyncInProgress: false) syncBookmarksAdapter = SyncBookmarksAdapter(database: bookmarksDatabase, appearancePreferences: appearancePreferences) + syncCredentialsAdapter = SyncCredentialsAdapter(secureVaultFactory: AutofillSecureVaultFactory) syncPreferences = SyncPreferences( syncService: ddgSyncing, syncBookmarksAdapter: syncBookmarksAdapter, + syncCredentialsAdapter: syncCredentialsAdapter, appearancePreferences: appearancePreferences, managementDialogModel: managementDialogModel, userAuthenticator: MockUserAuthenticator() From d0e84de98599bf872f04cd989fe8b254dfd718c2 Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Fri, 22 Mar 2024 17:11:02 +0100 Subject: [PATCH 09/10] Update copy after review --- .../xcshareddata/swiftpm/Package.resolved | 2 +- .../ManagementView/SyncEnabledView.swift | 4 +- .../Sources/SyncUI/internal/UserText.swift | 62 +++++++++---------- 3 files changed, 33 insertions(+), 35 deletions(-) diff --git a/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved b/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved index 6928f83343..211df4dab3 100644 --- a/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved +++ b/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved @@ -33,7 +33,7 @@ "location" : "https://github.com/duckduckgo/BrowserServicesKit", "state" : { "branch" : "dominik/sync-field-validation", - "revision" : "cdeed8ae54b3b8558ccf8bf5964476c81cb095a0" + "revision" : "0c1d85fdbd60beb148393f2caa9b59ecd9406e05" } }, { diff --git a/LocalPackages/SyncUI/Sources/SyncUI/Views/ManagementView/SyncEnabledView.swift b/LocalPackages/SyncUI/Sources/SyncUI/Views/ManagementView/SyncEnabledView.swift index 1255fdf962..4affcab8b7 100644 --- a/LocalPackages/SyncUI/Sources/SyncUI/Views/ManagementView/SyncEnabledView.swift +++ b/LocalPackages/SyncUI/Sources/SyncUI/Views/ManagementView/SyncEnabledView.swift @@ -128,12 +128,12 @@ struct SyncEnabledView: View where ViewModel: ManagementViewModel { case .bookmarks: assert(!model.invalidBookmarksTitles.isEmpty) let firstInvalidBookmarkTitle = model.invalidBookmarksTitles.first ?? "" - return UserText.invalidBookmarksPresentDescription(firstInvalidBookmarkTitle, numberOfOtherInvalidItems: model.invalidBookmarksTitles.count - 1) + return UserText.invalidBookmarksPresentDescription(firstInvalidBookmarkTitle, numberOfInvalidItems: model.invalidBookmarksTitles.count) case .credentials: assert(!model.invalidCredentialsTitles.isEmpty) let firstInvalidCredentialTitle = model.invalidCredentialsTitles.first ?? "" - return UserText.invalidCredentialsPresentDescription(firstInvalidCredentialTitle, numberOfOtherInvalidItems: model.invalidCredentialsTitles.count - 1) + return UserText.invalidCredentialsPresentDescription(firstInvalidCredentialTitle, numberOfInvalidItems: model.invalidCredentialsTitles.count) } } var actionTitle: String { diff --git a/LocalPackages/SyncUI/Sources/SyncUI/internal/UserText.swift b/LocalPackages/SyncUI/Sources/SyncUI/internal/UserText.swift index 1fee316481..ad9b47b13e 100644 --- a/LocalPackages/SyncUI/Sources/SyncUI/internal/UserText.swift +++ b/LocalPackages/SyncUI/Sources/SyncUI/internal/UserText.swift @@ -151,44 +151,42 @@ enum UserText { static let invalidBookmarksPresentTitle = NSLocalizedString("prefrences.sync.invalid-bookmarks-present-title", bundle: Bundle.module, value: "Some bookmarks are not syncing due to excessively long content in certain fields.", comment: "Alert title for invalid bookmarks being filtered out of synced data") static let invalidCredentialsPresentTitle = NSLocalizedString("prefrences.sync.invalid-credentials-present-title", bundle: Bundle.module, value: "Some logins are not syncing due to excessively long content in certain fields.", comment: "Alert title for invalid logins being filtered out of synced data") - static func invalidBookmarksPresentDescription(_ invalidItemTitle: String, numberOfOtherInvalidItems: Int) -> String { - let localized: String = { - guard numberOfOtherInvalidItems > 0 else { - return NSLocalizedString( - "prefrences.sync.invalid-bookmarks-present-description-one", - bundle: Bundle.module, - value: "Your bookmark for %@ can't sync because one of its fields exceeds the character limit.", - comment: "Alert message for 1 invalid bookmark being filtered out of synced data" - ) - } - return NSLocalizedString( - "prefrences.sync.invalid-bookmarks-present-description-many", + static func invalidBookmarksPresentDescription(_ invalidItemTitle: String, numberOfInvalidItems: Int) -> String { + guard numberOfInvalidItems > 1 else { + let message = NSLocalizedString( + "prefrences.sync.invalid-bookmarks-present-description-one", bundle: Bundle.module, - value: "Your bookmarks for %@ and other sites (%d) can't sync because some of their fields exceed the character limit.", - comment: "Alert message for multiple invalid bookmark being filtered out of synced data" + value: "Your bookmark for %@ can't sync because one of its fields exceeds the character limit.", + comment: "Alert message for 1 invalid bookmark being filtered out of synced data" ) - }() - return String(format: localized, invalidItemTitle, numberOfOtherInvalidItems) + return String(format: message, invalidItemTitle) + } + let message = NSLocalizedString( + "prefrences.sync.invalid-bookmarks-present-description-many", + bundle: Bundle.module, + value: "Some bookmarks (%d) can't sync because some of their fields exceed the character limit.", + comment: "Alert message for multiple invalid bookmark being filtered out of synced data" + ) + return String(format: message, numberOfInvalidItems) } - static func invalidCredentialsPresentDescription(_ invalidItemTitle: String, numberOfOtherInvalidItems: Int) -> String { - let localized: String = { - guard numberOfOtherInvalidItems > 0 else { - return NSLocalizedString( - "prefrences.sync.invalid-credentials-present-description-one", - bundle: Bundle.module, - value: "Your password for %@ can't sync because one of its fields exceeds the character limit.", - comment: "Alert message for 1 invalid login being filtered out of synced data" - ) - } - return NSLocalizedString( - "prefrences.sync.invalid-credentials-present-description-many", + static func invalidCredentialsPresentDescription(_ invalidItemTitle: String, numberOfInvalidItems: Int) -> String { + guard numberOfInvalidItems > 1 else { + let message = NSLocalizedString( + "prefrences.sync.invalid-credentials-present-description-one", bundle: Bundle.module, - value: "Your passwords for %@ and other sites (%d) can't sync because some of their fields exceed the character limit.", - comment: "Alert message for multiple invalid logins being filtered out of synced data" + value: "Your password for %@ can't sync because one of its fields exceeds the character limit.", + comment: "Alert message for 1 invalid login being filtered out of synced data" ) - }() - return String(format: localized, invalidItemTitle, numberOfOtherInvalidItems) + return String(format: message, invalidItemTitle) + } + let message = NSLocalizedString( + "prefrences.sync.invalid-credentials-present-description-many", + bundle: Bundle.module, + value: "Some passwords (n) can't sync because some of their fields exceed the character limit.", + comment: "Alert message for multiple invalid logins being filtered out of synced data" + ) + return String(format: message, numberOfInvalidItems) } static let syncErrorAlertTitle = NSLocalizedString("alert.sync-error", bundle: Bundle.module, value: "Sync & Backup Error", comment: "Title for sync error alert") From cb16f8abb9c98f5a5e797f4d3b5710e0a5345e6b Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Sun, 24 Mar 2024 22:12:11 +0100 Subject: [PATCH 10/10] Update Localizable.xcstrings --- .../SyncUI/Sources/SyncUI/Resources/Localizable.xcstrings | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/LocalPackages/SyncUI/Sources/SyncUI/Resources/Localizable.xcstrings b/LocalPackages/SyncUI/Sources/SyncUI/Resources/Localizable.xcstrings index 7b94029086..f4b5506ac5 100644 --- a/LocalPackages/SyncUI/Sources/SyncUI/Resources/Localizable.xcstrings +++ b/LocalPackages/SyncUI/Sources/SyncUI/Resources/Localizable.xcstrings @@ -4746,7 +4746,7 @@ "en" : { "stringUnit" : { "state" : "new", - "value" : "Your bookmarks for %1$@ and other sites (%2$d) can't sync because some of their fields exceed the character limit." + "value" : "Some bookmarks (%d) can't sync because some of their fields exceed the character limit." } } } @@ -4782,7 +4782,7 @@ "en" : { "stringUnit" : { "state" : "new", - "value" : "Your passwords for %1$@ and other sites (%2$d) can't sync because some of their fields exceed the character limit." + "value" : "Some passwords (n) can't sync because some of their fields exceed the character limit." } } }