Merge remote-tracking branch 'origin/alex/bookmarks-minor-improvement…

…s' into alex/cmd-tab-show-x

DuckDuckGo/Bookmarks/Model/BookmarkManager.swift

@@ -32,7 +32,7 @@ protocol BookmarkManager: AnyObject {
     func getBookmarkFolder(withId id: String) -> BookmarkFolder?
     @discardableResult func makeBookmark(for url: URL, title: String, isFavorite: Bool, index: Int?, parent: BookmarkFolder?) -> Bookmark?
     func makeBookmarks(for websitesInfo: [WebsiteInfo], inNewFolderNamed folderName: String, withinParentFolder parent: ParentFolderType)
-    func makeFolder(for title: String, parent: BookmarkFolder?, completion: @escaping (Result<BookmarkFolder, Error>) -> Void)
+    func makeFolder(named title: String, parent: BookmarkFolder?, completion: @escaping (Result<BookmarkFolder, Error>) -> Void)
     func remove(bookmark: Bookmark, undoManager: UndoManager?)
     func remove(folder: BookmarkFolder, undoManager: UndoManager?)
     func remove(objectsWithUUIDs uuids: [String], undoManager: UndoManager?)
@@ -66,23 +66,14 @@ protocol BookmarkManager: AnyObject {
     var sortMode: BookmarksSortMode { get set }
 
     func requestSync()
-
 }
 extension BookmarkManager {
     @discardableResult func makeBookmark(for url: URL, title: String, isFavorite: Bool) -> Bookmark? {
         makeBookmark(for: url, title: title, isFavorite: isFavorite, index: nil, parent: nil)
     }
-    func makeFolder(for title: String, completion: @escaping (Result<BookmarkFolder, Error>) -> Void) {
-        makeFolder(for: title, parent: nil, completion: completion)
+    @discardableResult func makeBookmark(for url: URL, title: String, isFavorite: Bool, index: Int?, parent: BookmarkFolder?) -> Bookmark? {
+        makeBookmark(for: url, title: title, isFavorite: isFavorite, index: index, parent: parent)
     }
-    func makeFolder(named title: String, parent: BookmarkFolder? = nil) async throws -> BookmarkFolder {
-        try await withCheckedThrowingContinuation { continuation in
-            makeFolder(for: title, parent: parent) { result in
-                continuation.resume(with: result)
-            }
-        }
-    }
-
     func move(objectUUIDs: [String], toIndex index: Int?, withinParentFolder parent: ParentFolderType) {
         move(objectUUIDs: objectUUIDs, toIndex: index, withinParentFolder: parent) { _ in }
     }
@@ -181,7 +172,7 @@ final class LocalBookmarkManager: BookmarkManager {
         bookmarkStore.bookmarkFolder(withId: id)
     }
 
-    @discardableResult func makeBookmark(for url: URL, title: String, isFavorite: Bool, index: Int? = nil, parent: BookmarkFolder? = nil) -> Bookmark? {
+    @discardableResult func makeBookmark(for url: URL, title: String, isFavorite: Bool, index: Int?, parent: BookmarkFolder?) -> Bookmark? {
         guard list != nil else { return nil }
 
         guard !isUrlBookmarked(url: url) else {
@@ -193,8 +184,8 @@ final class LocalBookmarkManager: BookmarkManager {
         let bookmark = Bookmark(id: id, url: url.absoluteString, title: title, isFavorite: isFavorite, parentFolderUUID: parent?.id)
 
         list?.insert(bookmark)
-        bookmarkStore.save(bookmark: bookmark, index: index) { [weak self] success, _  in
-            guard success else {
+        bookmarkStore.save(bookmark: bookmark, index: index) { [weak self] error  in
+            if error != nil {
                 self?.list?.remove(bookmark)
                 return
             }
@@ -222,8 +213,8 @@ final class LocalBookmarkManager: BookmarkManager {
 
         undoManager?.registerUndoDeleteEntities([bookmark], bookmarkManager: self)
         list?.remove(latestBookmark)
-        bookmarkStore.remove(objectsWithUUIDs: [bookmark.id]) { [weak self] success, _ in
-            if !success {
+        bookmarkStore.remove(objectsWithUUIDs: [bookmark.id]) { [weak self] error in
+            if error != nil {
                 self?.list?.insert(bookmark)
             }
 
@@ -235,7 +226,7 @@ final class LocalBookmarkManager: BookmarkManager {
     @MainActor
     func remove(folder: BookmarkFolder, undoManager: UndoManager?) {
         undoManager?.registerUndoDeleteEntities([folder], bookmarkManager: self)
-        bookmarkStore.remove(objectsWithUUIDs: [folder.id]) { [weak self] _, _ in
+        bookmarkStore.remove(objectsWithUUIDs: [folder.id]) { [weak self] _ in
             self?.loadBookmarks()
             self?.requestSync()
         }
@@ -246,7 +237,7 @@ final class LocalBookmarkManager: BookmarkManager {
         if let undoManager, let entities = bookmarkStore.bookmarkEntities(withIds: uuids) {
             undoManager.registerUndoDeleteEntities(entities, bookmarkManager: self)
         }
-        bookmarkStore.remove(objectsWithUUIDs: uuids) { [weak self] _, _ in
+        bookmarkStore.remove(objectsWithUUIDs: uuids) { [weak self] _ in
             self?.loadBookmarks()
             self?.requestSync()
         }
@@ -350,10 +341,10 @@ final class LocalBookmarkManager: BookmarkManager {
 
     // MARK: - Folders
 
-    func makeFolder(for title: String, parent: BookmarkFolder?, completion: @escaping (Result<BookmarkFolder, Error>) -> Void) {
+    func makeFolder(named title: String, parent: BookmarkFolder?, completion: @escaping (Result<BookmarkFolder, Error>) -> Void) {
         let folder = BookmarkFolder(id: UUID().uuidString, title: title, parentFolderUUID: parent?.id, children: [])
 
-        bookmarkStore.save(folder: folder) { [weak self] _, error in
+        bookmarkStore.save(folder: folder) { [weak self] error  in
             if let error {
                 completion(.failure(error))
                 return
@@ -667,7 +658,6 @@ private extension String {
     private func removeAccents() -> String {
         // Normalize the string to NFD (Normalization Form Decomposition)
         let normalizedString = self as NSString
-        let range = NSRange(location: 0, length: normalizedString.length)
 
         // Apply the transform to remove diacritics
         let transformedString = normalizedString.applyingTransform(.toLatin, reverse: false) ?? ""

DuckDuckGo/Bookmarks/Services/BookmarkStore.swift

@@ -49,7 +49,7 @@ protocol BookmarkStore {
     func loadAll(type: BookmarkStoreFetchPredicateType, completion: @escaping ([BaseBookmarkEntity]?, Error?) -> Void)
     func save(entitiesAtIndices: [(entity: BaseBookmarkEntity, index: Int?)], completion: @escaping (Error?) -> Void)
     func saveBookmarks(for websitesInfo: [WebsiteInfo], inNewFolderNamed folderName: String, withinParentFolder parent: ParentFolderType)
-    func remove(objectsWithUUIDs: [String], completion: @escaping (Bool, Error?) -> Void)
+    func remove(objectsWithUUIDs: [String], completion: @escaping (Error?) -> Void)
     func update(bookmark: Bookmark)
     func bookmarkEntities(withIds ids: [String]) -> [BaseBookmarkEntity]?
     func update(folder: BookmarkFolder)
@@ -81,15 +81,15 @@ extension BookmarkStore {
         bookmarkEntities(withIds: [id])?.first as? BookmarkFolder
     }
 
-    func save(folder: BookmarkFolder, index: Int? = nil, completion: @escaping (Bool, Error?) -> Void) {
+    func save(folder: BookmarkFolder, index: Int? = nil, completion: @escaping (Error?) -> Void) {
         save(entitiesAtIndices: [(folder, index)]) { error in
-            completion(error == nil, error)
+            completion(error)
         }
     }
 
     func save(folder: BookmarkFolder, index: Int? = nil) async throws {
         return try await withCheckedThrowingContinuation { continuation in
-            save(folder: folder, index: index) { _, error in
+            save(folder: folder, index: index) { error in
                 if let error {
                     continuation.resume(throwing: error)
                     return
@@ -100,15 +100,15 @@ extension BookmarkStore {
         }
     }
 
-    func save(bookmark: Bookmark, index: Int? = nil, completion: @escaping (Bool, Error?) -> Void) {
+    func save(bookmark: Bookmark, index: Int? = nil, completion: @escaping (Error?) -> Void) {
         save(entitiesAtIndices: [(bookmark, index)]) { error in
-            completion(error == nil, error)
+            completion(error)
         }
     }
 
     func save(bookmark: Bookmark, index: Int? = nil) async throws {
         return try await withCheckedThrowingContinuation { continuation in
-            save(bookmark: bookmark, index: index) { _, error in
+            save(bookmark: bookmark, index: index) { error in
                 if let error {
                     continuation.resume(throwing: error)
                     return
@@ -121,7 +121,7 @@ extension BookmarkStore {
 
     func remove(objectsWithUUIDs uuids: [String]) async throws {
         return try await withCheckedThrowingContinuation { continuation in
-            remove(objectsWithUUIDs: uuids) { _, error in
+            remove(objectsWithUUIDs: uuids) { error in
                 if let error {
                     continuation.resume(throwing: error)
                     return

DuckDuckGo/Bookmarks/Services/BookmarkStoreMock.swift

@@ -25,11 +25,10 @@ final class BookmarkStoreMock: BookmarkStore {
 
     private let store: LocalBookmarkStore?
 
-    init(contextProvider: (() -> NSManagedObjectContext)? = nil, loadAllCalled: Bool = false, bookmarks: [BaseBookmarkEntity]? = nil, loadError: Error? = nil, removeSuccess: Bool = true, removeError: Error? = nil, updateBookmarkCalled: Bool = false, updateFolderCalled: Bool = false, addChildCalled: Bool = false, updateObjectsCalled: Bool = false, importBookmarksCalled: Bool = false, canMoveObjectWithUUIDCalled: Bool = false, moveObjectUUIDCalled: Bool = false, updateFavoriteIndexCalled: Bool = false) {
+    init(contextProvider: (() -> NSManagedObjectContext)? = nil, loadAllCalled: Bool = false, bookmarks: [BaseBookmarkEntity]? = nil, loadError: Error? = nil, removeError: Error? = nil, updateBookmarkCalled: Bool = false, updateFolderCalled: Bool = false, addChildCalled: Bool = false, updateObjectsCalled: Bool = false, importBookmarksCalled: Bool = false, canMoveObjectWithUUIDCalled: Bool = false, moveObjectUUIDCalled: Bool = false, updateFavoriteIndexCalled: Bool = false) {
         self.loadAllCalled = loadAllCalled
         self.bookmarks = bookmarks
         self.loadError = loadError
-        self.removeSuccess = removeSuccess
         self.removeError = removeError
         self.updateBookmarkCalled = updateBookmarkCalled
         self.updateFolderCalled = updateFolderCalled
@@ -138,19 +137,18 @@ final class BookmarkStoreMock: BookmarkStore {
 
     var removeCalled: Bool { removeCalledWithIds != nil }
     var removeCalledWithIds: [String]?
-    var removeSuccess = true
     var removeError: Error?
-    func remove(objectsWithUUIDs uuids: [String], completion: @escaping (Bool, Error?) -> Void) {
+    func remove(objectsWithUUIDs uuids: [String], completion: @escaping (Error?) -> Void) {
         removeCalledWithIds = uuids
 
         // For the purpose of the mock, only remove bookmarks if `removeSuccess` is true.
-        guard removeSuccess else {
-            completion(removeSuccess, removeError)
+        guard removeError == nil else {
+            completion(removeError)
             return
         }
 
         store?.remove(objectsWithUUIDs: uuids, completion: completion) ?? { bookmarks?.removeAll { uuids.contains($0.id) }
-            completion(removeSuccess, removeError)
+            completion(removeError)
         }()
     }
 

DuckDuckGo/Bookmarks/Services/LocalBookmarkStore.swift

@@ -276,7 +276,7 @@ final class LocalBookmarkStore: BookmarkStore {
         }
 
         let context = makeContext()
-        context.perform {
+        context.perform { [favoritesDisplayMode] in
             do {
                 let results: [BookmarkEntity]
 
@@ -298,7 +298,7 @@ final class LocalBookmarkStore: BookmarkStore {
                 let entities: [BaseBookmarkEntity] = results.compactMap { entity in
                     BaseBookmarkEntity.from(managedObject: entity,
                                             parentFolderUUID: entity.parent?.uuid,
-                                            favoritesDisplayMode: self.favoritesDisplayMode)
+                                            favoritesDisplayMode: favoritesDisplayMode)
                 }
 
                 mainQueueCompletion(bookmarks: entities, error: nil)
@@ -379,13 +379,9 @@ final class LocalBookmarkStore: BookmarkStore {
         }
     }
 
-    func remove(objectsWithUUIDs identifiers: [String], completion: @escaping (Bool, Error?) -> Void) {
-
-        applyChangesAndSave(changes: { [weak self] context in
-            guard self != nil else {
-                throw BookmarkStoreError.storeDeallocated
-            }
+    func remove(objectsWithUUIDs identifiers: [String], completion: @escaping (Error?) -> Void) {
 
+        applyChangesAndSave(changes: { context in
             let fetchRequest = BaseBookmarkEntity.entities(with: identifiers)
             let fetchResults = (try? context.fetch(fetchRequest)) ?? []
 
@@ -398,20 +394,16 @@ final class LocalBookmarkStore: BookmarkStore {
             }
         }, onError: { [weak self] error in
             self?.commonOnSaveErrorHandler(error)
-            DispatchQueue.main.async { completion(false, error) }
+            DispatchQueue.main.async { completion(error) }
         }, onDidSave: {
-            DispatchQueue.main.async { completion(true, nil) }
+            DispatchQueue.main.async { completion(nil) }
         })
     }
 
     func update(bookmark: Bookmark) {
 
         do {
-            try applyChangesAndSave(changes: { [weak self] context in
-                guard let self = self else {
-                    throw BookmarkStoreError.storeDeallocated
-                }
-
+            try applyChangesAndSave(changes: { [favoritesDisplayMode] context in
                 let bookmarkFetchRequest = BaseBookmarkEntity.singleEntity(with: bookmark.id)
                 let bookmarkFetchRequestResults = try? context.fetch(bookmarkFetchRequest)
 
@@ -538,9 +530,9 @@ final class LocalBookmarkStore: BookmarkStore {
 
             let favoritesFolders = BookmarkUtils.fetchFavoritesFolders(for: favoritesDisplayMode, in: context)
             bookmarkManagedObjects.forEach { managedObject in
-                if let entity = BaseBookmarkEntity.from(managedObject: managedObject, parentFolderUUID: nil, favoritesDisplayMode: self.favoritesDisplayMode) {
+                if let entity = BaseBookmarkEntity.from(managedObject: managedObject, parentFolderUUID: nil, favoritesDisplayMode: favoritesDisplayMode) {
                     update(entity)
-                    managedObject.update(with: entity, favoritesFoldersToAddFavorite: favoritesFolders, favoritesDisplayMode: self.favoritesDisplayMode)
+                    managedObject.update(with: entity, favoritesFoldersToAddFavorite: favoritesFolders, favoritesDisplayMode: favoritesDisplayMode)
                 }
             }
         }, onError: { [weak self] error in
@@ -660,11 +652,7 @@ final class LocalBookmarkStore: BookmarkStore {
 
     func moveFavorites(with objectUUIDs: [String], toIndex index: Int?, completion: @escaping (Error?) -> Void) {
 
-        applyChangesAndSave(changes: { [weak self] context in
-            guard let self = self else {
-                throw BookmarkStoreError.storeDeallocated
-            }
-
+        applyChangesAndSave(changes: { [favoritesDisplayMode] context in
             let displayedFavoritesFolderUUID = favoritesDisplayMode.displayedFolder.rawValue
             guard let displayedFavoritesFolder = BookmarkUtils.fetchFavoritesFolder(withUUID: displayedFavoritesFolderUUID, in: context) else {
                 throw BookmarkStoreError.missingFavoritesRoot
@@ -947,11 +935,8 @@ final class LocalBookmarkStore: BookmarkStore {
     // MARK: - Sync
 
     func handleFavoritesAfterDisablingSync() {
-        applyChangesAndSave { [weak self] context in
-            guard let self else {
-                return
-            }
-            if self.favoritesDisplayMode.isDisplayUnified {
+        applyChangesAndSave { [favoritesDisplayMode] context in
+            if favoritesDisplayMode.isDisplayUnified {
                 BookmarkUtils.copyFavorites(from: .unified, to: .desktop, clearingNonNativeFavoritesFolder: .mobile, in: context)
             } else {
                 BookmarkUtils.copyFavorites(from: .desktop, to: .unified, clearingNonNativeFavoritesFolder: .mobile, in: context)

DuckDuckGo/Bookmarks/ViewModel/AddBookmarkFolderPopoverViewModel.swift

@@ -72,7 +72,7 @@ final class AddBookmarkFolderPopoverViewModel: ObservableObject {
         }
 
         isDisabled = true
-        bookmarkManager.makeFolder(for: folderName.trimmingWhitespace(), parent: parent) { [completionHandler] result in
+        bookmarkManager.makeFolder(named: folderName.trimmingWhitespace(), parent: parent) { [completionHandler] result in
             completionHandler(try? result.get())
         }
     }

DuckDuckGo/Bookmarks/ViewModel/AddEditBookmarkFolderDialogViewModel.swift

@@ -139,8 +139,8 @@ private extension AddEditBookmarkFolderDialogViewModel {
     }
 
     func add(folderWithName name: String, to parent: BookmarkFolder?) {
-        bookmarkManager.makeFolder(for: name, parent: parent) { [weak self] result in
-            guard let bookmarkFolder = try? result.get() else { return }
+        bookmarkManager.makeFolder(named: name, parent: parent) { [weak self] bookmarkFolder in
+            guard case .success(let bookmarkFolder) = bookmarkFolder else { return }
             self?.addFolderSubject.send(bookmarkFolder)
         }
     }

DuckDuckGo/Common/Localizables/UserText.swift

@@ -452,7 +452,7 @@ struct UserText {
     static let passwordManagerAlertDuplicatePassword = NSLocalizedString("passsword.manager.alert.duplicate.password", value: "Duplicate Password", comment: "Title of the alert that the password inserted already exists")
     static let passwordManagerAlertDuplicatePasswordDescription = NSLocalizedString("passsword.manager.alert.duplicate.password.description", value: "You already have a password saved for this username and website.", comment: "Text of the alert that explains the password inserted already exists for a given website")
     static let thisActionCannotBeUndone = NSLocalizedString("action-cannot-be-undone", value: "This action cannot be undone.", comment: "Text used in alerts to warn user that a given action cannot be undone")
-    static let passwordManagerAlerDeleteButton = NSLocalizedString("passsword.manager.alert.delete", value: "Delete", comment: "Button of the alert that asks the user to confirm they want to delete an password, login or credential to actually delete")
+    static let passwordManagerAlertDeleteButton = NSLocalizedString("passsword.manager.alert.delete", value: "Delete", comment: "Button of the alert that asks the user to confirm they want to delete an password, login or credential to actually delete")
     static let passwordManagerAlertRemoveCardConfirmation = NSLocalizedString("passsword.manager.alert.remove-card.confirmation", value: "Are you sure you want to delete this saved credit card?", comment: "Text of the alert that asks the user to confirm they want to delete a credit card")
     static let passwordManagerAlertRemoveIdentityConfirmation = NSLocalizedString("passsword.manager.alert.remove-identity.confirmation", value: "Are you sure you want to delete this saved autofill info?", comment: "Text of the alert that asks the user to confirm they want to delete an identity")
     static let passwordManagerAlertRemoveNoteConfirmation = NSLocalizedString("passsword.manager.alert.remove-note.confirmation", value: "Are you sure you want to delete this note?", comment: "Text of the alert that asks the user to confirm they want to delete a note")