Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Alessandro/address bookmarks feedback folder logic #2221

Merged
30 changes: 30 additions & 0 deletions DuckDuckGo.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -2406,6 +2406,14 @@
9F514F912B7D88AD001832A9 /* AddEditBookmarkFolderDialogView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9F514F902B7D88AD001832A9 /* AddEditBookmarkFolderDialogView.swift */; };
9F514F922B7D88AD001832A9 /* AddEditBookmarkFolderDialogView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9F514F902B7D88AD001832A9 /* AddEditBookmarkFolderDialogView.swift */; };
9F514F932B7D88AD001832A9 /* AddEditBookmarkFolderDialogView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9F514F902B7D88AD001832A9 /* AddEditBookmarkFolderDialogView.swift */; };
9F56CFA92B82DC4300BB7F11 /* AddEditBookmarkFolderView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9F56CFA82B82DC4300BB7F11 /* AddEditBookmarkFolderView.swift */; };
9F56CFAA2B82DC4300BB7F11 /* AddEditBookmarkFolderView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9F56CFA82B82DC4300BB7F11 /* AddEditBookmarkFolderView.swift */; };
9F56CFAB2B82DC4300BB7F11 /* AddEditBookmarkFolderView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9F56CFA82B82DC4300BB7F11 /* AddEditBookmarkFolderView.swift */; };
9F982F0D2B8224BF00231028 /* AddEditBookmarkFolderDialogViewModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9F982F0C2B8224BE00231028 /* AddEditBookmarkFolderDialogViewModel.swift */; };
9F982F0E2B8224BF00231028 /* AddEditBookmarkFolderDialogViewModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9F982F0C2B8224BE00231028 /* AddEditBookmarkFolderDialogViewModel.swift */; };
9F982F0F2B8224BF00231028 /* AddEditBookmarkFolderDialogViewModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9F982F0C2B8224BE00231028 /* AddEditBookmarkFolderDialogViewModel.swift */; };
9F982F132B822B7B00231028 /* AddEditBookmarkFolderDialogViewModelTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9F982F112B82268F00231028 /* AddEditBookmarkFolderDialogViewModelTests.swift */; };
9F982F142B822C7400231028 /* AddEditBookmarkFolderDialogViewModelTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9F982F112B82268F00231028 /* AddEditBookmarkFolderDialogViewModelTests.swift */; };
9FA173DA2B79BD8A00EE4E6E /* BookmarkDialogContainerView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9FA173D92B79BD8A00EE4E6E /* BookmarkDialogContainerView.swift */; };
9FA173DB2B79BD8A00EE4E6E /* BookmarkDialogContainerView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9FA173D92B79BD8A00EE4E6E /* BookmarkDialogContainerView.swift */; };
9FA173DC2B79BD8A00EE4E6E /* BookmarkDialogContainerView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9FA173D92B79BD8A00EE4E6E /* BookmarkDialogContainerView.swift */; };
Expand Down Expand Up @@ -3949,6 +3957,9 @@
9F3910612B68C35600CB5112 /* DownloadsTabExtensionTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DownloadsTabExtensionTests.swift; sourceTree = "<group>"; };
9F3910682B68D87B00CB5112 /* ProgressExtensionTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ProgressExtensionTests.swift; sourceTree = "<group>"; };
9F514F902B7D88AD001832A9 /* AddEditBookmarkFolderDialogView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AddEditBookmarkFolderDialogView.swift; sourceTree = "<group>"; };
9F56CFA82B82DC4300BB7F11 /* AddEditBookmarkFolderView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AddEditBookmarkFolderView.swift; sourceTree = "<group>"; };
9F982F0C2B8224BE00231028 /* AddEditBookmarkFolderDialogViewModel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AddEditBookmarkFolderDialogViewModel.swift; sourceTree = "<group>"; };
9F982F112B82268F00231028 /* AddEditBookmarkFolderDialogViewModelTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AddEditBookmarkFolderDialogViewModelTests.swift; sourceTree = "<group>"; };
9FA173D92B79BD8A00EE4E6E /* BookmarkDialogContainerView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BookmarkDialogContainerView.swift; sourceTree = "<group>"; };
9FA173DE2B7A0EFE00EE4E6E /* BookmarkDialogButtonsView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BookmarkDialogButtonsView.swift; sourceTree = "<group>"; };
9FA173E22B7A12B600EE4E6E /* BookmarkDialogFolderManagementView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BookmarkDialogFolderManagementView.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -6548,6 +6559,14 @@
path = DuckDuckGoDBPBackgroundAgent;
sourceTree = "<group>";
};
9F982F102B82264400231028 /* ViewModels */ = {
isa = PBXGroup;
children = (
9F982F112B82268F00231028 /* AddEditBookmarkFolderDialogViewModelTests.swift */,
);
path = ViewModels;
sourceTree = "<group>";
};
9FA173DD2B7A0ECE00EE4E6E /* Dialog */ = {
isa = PBXGroup;
children = (
Expand All @@ -6558,6 +6577,7 @@
9FA173E22B7A12B600EE4E6E /* BookmarkDialogFolderManagementView.swift */,
9FA173EA2B7B232200EE4E6E /* AddEditBookmarkDialogView.swift */,
9F514F902B7D88AD001832A9 /* AddEditBookmarkFolderDialogView.swift */,
9F56CFA82B82DC4300BB7F11 /* AddEditBookmarkFolderView.swift */,
);
path = Dialog;
sourceTree = "<group>";
Expand Down Expand Up @@ -6905,6 +6925,7 @@
AA652CAB25DD820D009059CC /* Bookmarks */ = {
isa = PBXGroup;
children = (
9F982F102B82264400231028 /* ViewModels */,
AA652CAE25DD8228009059CC /* Model */,
AA652CAF25DD822C009059CC /* Services */,
);
Expand Down Expand Up @@ -7312,6 +7333,7 @@
AAB549DE25DAB8F80058460B /* BookmarkViewModel.swift */,
B6F9BDD72B45B7D900677B33 /* AddBookmarkModalViewModel.swift */,
B6F9BDDF2B45C1A800677B33 /* AddBookmarkFolderModalViewModel.swift */,
9F982F0C2B8224BE00231028 /* AddEditBookmarkFolderDialogViewModel.swift */,
);
path = ViewModel;
sourceTree = "<group>";
Expand Down Expand Up @@ -9503,6 +9525,7 @@
4B41EDA82B1543C9001EEDF4 /* PreferencesVPNView.swift in Sources */,
3706FA7B293F65D500E42796 /* FaviconUserScript.swift in Sources */,
3706FA7E293F65D500E42796 /* LottieAnimationCache.swift in Sources */,
9F982F0E2B8224BF00231028 /* AddEditBookmarkFolderDialogViewModel.swift in Sources */,
3706FA7F293F65D500E42796 /* TabIndex.swift in Sources */,
3706FA80293F65D500E42796 /* TabLazyLoaderDataSource.swift in Sources */,
3706FA81293F65D500E42796 /* LoginImport.swift in Sources */,
Expand Down Expand Up @@ -9799,6 +9822,7 @@
37CBCA9B2A8966E60050218F /* SyncSettingsAdapter.swift in Sources */,
3707C72A294B5D2900682A9F /* URLExtension.swift in Sources */,
3706FB76293F65D500E42796 /* ASN1Parser.swift in Sources */,
9F56CFAA2B82DC4300BB7F11 /* AddEditBookmarkFolderView.swift in Sources */,
37FD78122A29EBD100B36DB1 /* SyncErrorHandler.swift in Sources */,
987799F42999993C005D8EB6 /* LegacyBookmarksStoreMigration.swift in Sources */,
3706FB7A293F65D500E42796 /* FileDownloadManager.swift in Sources */,
Expand Down Expand Up @@ -10435,6 +10459,7 @@
3706FE7B293F661700E42796 /* HistoryStoringMock.swift in Sources */,
562984702AC4610100AC20EB /* SyncPreferencesTests.swift in Sources */,
3706FE7C293F661700E42796 /* LocalBookmarkStoreTests.swift in Sources */,
9F982F142B822C7400231028 /* AddEditBookmarkFolderDialogViewModelTests.swift in Sources */,
B6CA4825298CE4B70067ECCE /* AdClickAttributionTabExtensionTests.swift in Sources */,
3707C72D294B5D4100682A9F /* EmptyAttributionRulesProver.swift in Sources */,
376E2D2629428353001CD31B /* PrivacyReferenceTestHelper.swift in Sources */,
Expand Down Expand Up @@ -10896,6 +10921,7 @@
4B957A202AC7AE700062CA31 /* CancellableExtension.swift in Sources */,
4B957A212AC7AE700062CA31 /* PinnedTabsHostingView.swift in Sources */,
4B957A222AC7AE700062CA31 /* FirefoxBookmarksReader.swift in Sources */,
9F982F0F2B8224BF00231028 /* AddEditBookmarkFolderDialogViewModel.swift in Sources */,
4B0526622B1D55320054955A /* VPNFeedbackSender.swift in Sources */,
4B957A232AC7AE700062CA31 /* DeviceIdleStateDetector.swift in Sources */,
4B957A242AC7AE700062CA31 /* FlatButton.swift in Sources */,
Expand Down Expand Up @@ -11332,6 +11358,7 @@
4B957BA12AC7AE700062CA31 /* UserDefaults+NetworkProtectionShared.swift in Sources */,
4B957BA22AC7AE700062CA31 /* NavigationActionPolicyExtension.swift in Sources */,
4B957BA32AC7AE700062CA31 /* CIImageExtension.swift in Sources */,
9F56CFAB2B82DC4300BB7F11 /* AddEditBookmarkFolderView.swift in Sources */,
9FA173DC2B79BD8A00EE4E6E /* BookmarkDialogContainerView.swift in Sources */,
4B957BA42AC7AE700062CA31 /* NSMenuExtension.swift in Sources */,
4B957BA52AC7AE700062CA31 /* MainWindowController.swift in Sources */,
Expand Down Expand Up @@ -11563,6 +11590,7 @@
B602E7CF2A93A5FF00F12201 /* WKBackForwardListExtension.swift in Sources */,
4B59024026B35F3600489384 /* ChromiumDataImporter.swift in Sources */,
B62B48562ADE730D000DECE5 /* FileImportView.swift in Sources */,
9F982F0D2B8224BF00231028 /* AddEditBookmarkFolderDialogViewModel.swift in Sources */,
AAA0CC3C25337FAB0079BC96 /* WKBackForwardListItemViewModel.swift in Sources */,
1D43EB3429297D760065E5D6 /* BWNotRespondingAlert.swift in Sources */,
4BB88B4525B7B55C006F6B06 /* DebugUserScript.swift in Sources */,
Expand Down Expand Up @@ -12052,6 +12080,7 @@
AAC5E4E425D6BA9C007F5990 /* NSSizeExtension.swift in Sources */,
AA6820EB25503D6A005ED0D5 /* Fire.swift in Sources */,
3158B1492B0BF73000AF130C /* DBPHomeViewController.swift in Sources */,
9F56CFA92B82DC4300BB7F11 /* AddEditBookmarkFolderView.swift in Sources */,
37445F9C2A1569F00029F789 /* SyncBookmarksAdapter.swift in Sources */,
B6AAAC3E26048F690029438D /* RandomAccessCollectionExtension.swift in Sources */,
4B9292AF26670F5300AD2C21 /* NSOutlineViewExtensions.swift in Sources */,
Expand Down Expand Up @@ -12186,6 +12215,7 @@
B6619F062B17138D00CD9186 /* DataImportSourceViewModelTests.swift in Sources */,
4BBF0917282DD6EF00EE1418 /* TemporaryFileHandlerTests.swift in Sources */,
B6A5A27925B93FFF00AA7ADA /* StateRestorationManagerTests.swift in Sources */,
9F982F132B822B7B00231028 /* AddEditBookmarkFolderDialogViewModelTests.swift in Sources */,
B630E7FE29C887ED00363609 /* NSErrorAdditionalInfo.swift in Sources */,
4B9292BB2667103100AD2C21 /* BookmarkNodeTests.swift in Sources */,
4B0219A825E0646500ED7DEA /* WebsiteDataStoreTests.swift in Sources */,
Expand Down
7 changes: 7 additions & 0 deletions DuckDuckGo/Bookmarks/Model/BookmarkManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ protocol BookmarkManager: AnyObject {
func remove(objectsWithUUIDs uuids: [String])
func update(bookmark: Bookmark)
func update(folder: BookmarkFolder)
func update(folder: BookmarkFolder, andMoveToParent parent: ParentFolderType)
@discardableResult func updateUrl(of bookmark: Bookmark, to newUrl: URL) -> Bookmark?
func add(bookmark: Bookmark, to parent: BookmarkFolder?, completion: @escaping (Error?) -> Void)
func add(objectsWithUUIDs uuids: [String], to parent: BookmarkFolder?, completion: @escaping (Error?) -> Void)
Expand Down Expand Up @@ -217,6 +218,12 @@ final class LocalBookmarkManager: BookmarkManager {
requestSync()
}

func update(folder: BookmarkFolder, andMoveToParent parent: ParentFolderType) {
bookmarkStore.update(folder: folder, andMoveToParent: parent)
loadBookmarks()
requestSync()
}

func updateUrl(of bookmark: Bookmark, to newUrl: URL) -> Bookmark? {
guard list != nil else { return nil }
guard getBookmark(forUrl: bookmark.url) != nil else {
Expand Down
1 change: 1 addition & 0 deletions DuckDuckGo/Bookmarks/Services/BookmarkStore.swift
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ protocol BookmarkStore {
func remove(objectsWithUUIDs: [String], completion: @escaping (Bool, Error?) -> Void)
func update(bookmark: Bookmark)
func update(folder: BookmarkFolder)
func update(folder: BookmarkFolder, andMoveToParent parent: ParentFolderType)
func add(objectsWithUUIDs: [String], to parent: BookmarkFolder?, completion: @escaping (Error?) -> Void)
func update(objectsWithUUIDs uuids: [String], update: @escaping (BaseBookmarkEntity) -> Void, completion: @escaping (Error?) -> Void)
func canMoveObjectWithUUID(objectUUID uuid: String, to parent: BookmarkFolder) -> Bool
Expand Down
30 changes: 30 additions & 0 deletions DuckDuckGo/Bookmarks/Services/BookmarkStoreMock.swift
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,10 @@ public final class BookmarkStoreMock: BookmarkStore {
self.updateFavoriteIndexCalled = updateFavoriteIndexCalled
}

var capturedFolder: BookmarkFolder?
var capturedParentFolder: BookmarkFolder?
var capturedParentFolderType: ParentFolderType?

var loadAllCalled = false
var bookmarks: [BaseBookmarkEntity]?
var loadError: Error?
Expand All @@ -68,6 +72,8 @@ public final class BookmarkStoreMock: BookmarkStore {
var saveFolderError: Error?
func save(folder: BookmarkFolder, parent: BookmarkFolder?, completion: @escaping (Bool, Error?) -> Void) {
saveFolderCalled = true
capturedFolder = folder
capturedParentFolder = parent
completion(saveFolderSuccess, saveFolderError)
}

Expand Down Expand Up @@ -97,6 +103,14 @@ public final class BookmarkStoreMock: BookmarkStore {
var updateFolderCalled = false
func update(folder: BookmarkFolder) {
updateFolderCalled = true
capturedFolder = folder
}

var updateFolderAndMoveToParentCalled = false
func update(folder: BookmarkFolder, andMoveToParent parent: ParentFolderType) {
updateFolderAndMoveToParentCalled = true
capturedFolder = folder
capturedParentFolderType = parent
}

var addChildCalled = false
Expand All @@ -122,8 +136,11 @@ public final class BookmarkStoreMock: BookmarkStore {
}

var moveObjectUUIDCalled = false
var capturedObjectUUIDs: [String]?
func move(objectUUIDs: [String], toIndex: Int?, withinParentFolder: ParentFolderType, completion: @escaping (Error?) -> Void) {
moveObjectUUIDCalled = true
capturedObjectUUIDs = objectUUIDs
capturedParentFolderType = withinParentFolder
}

var updateFavoriteIndexCalled = false
Expand All @@ -135,4 +152,17 @@ public final class BookmarkStoreMock: BookmarkStore {
func handleFavoritesAfterDisablingSync() {}
}

extension ParentFolderType: Equatable {
public static func == (lhs: Self, rhs: Self) -> Bool {
switch (lhs, rhs) {
case (.root, .root):
return true
case (.parent(let lhsValue), .parent(let rhsValue)):
return lhsValue == rhsValue
default:
return false
}
}
}

#endif
94 changes: 60 additions & 34 deletions DuckDuckGo/Bookmarks/Services/LocalBookmarkStore.swift
Original file line number Diff line number Diff line change
Expand Up @@ -391,18 +391,21 @@ final class LocalBookmarkStore: BookmarkStore {
}

func update(folder: BookmarkFolder) {

do {
_ = try applyChangesAndSave(changes: { context in
let folderFetchRequest = BaseBookmarkEntity.singleEntity(with: folder.id)
let folderFetchRequestResults = try? context.fetch(folderFetchRequest)

guard let bookmarkFolderMO = folderFetchRequestResults?.first else {
assertionFailure("LocalBookmarkStore: Failed to get BookmarkEntity from the context")
throw BookmarkStoreError.missingEntity
}
try update(folder: folder, in: context)
})
} catch {
let error = error as NSError
commonOnSaveErrorHandler(error)
}
}

bookmarkFolderMO.update(with: folder)
func update(folder: BookmarkFolder, andMoveToParent parent: ParentFolderType) {
do {
_ = try applyChangesAndSave(changes: { context in
let folderEntity = try update(folder: folder, in: context)
try move(entities: [folderEntity], toIndex: nil, withinParentFolderType: parent, in: context)
})
} catch {
let error = error as NSError
Expand Down Expand Up @@ -566,39 +569,15 @@ final class LocalBookmarkStore: BookmarkStore {
throw BookmarkStoreError.storeDeallocated
}

guard let rootFolder = self.bookmarksRoot(in: context) else {
throw BookmarkStoreError.missingRoot
}

// Guarantee that bookmarks are fetched in the same order as the UUIDs. In the future, this should fetch all objects at once with a
// batch fetch request and have them sorted in the correct order.
let bookmarkManagedObjects: [BookmarkEntity] = objectUUIDs.compactMap { uuid in
let entityFetchRequest = BaseBookmarkEntity.singleEntity(with: uuid)
return (try? context.fetch(entityFetchRequest))?.first
}

let newParentFolder: BookmarkEntity

switch type {
case .root: newParentFolder = rootFolder
case .parent(let newParentUUID):
let bookmarksFetchRequest = BaseBookmarkEntity.singleEntity(with: newParentUUID)

if let fetchedParent = try context.fetch(bookmarksFetchRequest).first, fetchedParent.isFolder {
newParentFolder = fetchedParent
} else {
throw BookmarkStoreError.missingEntity
}
}
try move(entities: bookmarkManagedObjects, toIndex: index, withinParentFolderType: type, in: context)

if let index = index, index < newParentFolder.childrenArray.count {
self.move(entities: bookmarkManagedObjects, to: index, within: newParentFolder)
} else {
for bookmarkManagedObject in bookmarkManagedObjects {
bookmarkManagedObject.parent = nil
newParentFolder.addToChildren(bookmarkManagedObject)
}
}
}, onError: { [weak self] error in
self?.commonOnSaveErrorHandler(error)
DispatchQueue.main.async { completion(error) }
Expand Down Expand Up @@ -996,6 +975,53 @@ final class LocalBookmarkStore: BookmarkStore {

}

private extension LocalBookmarkStore {

@discardableResult
func update(folder: BookmarkFolder, in context: NSManagedObjectContext) throws -> BookmarkEntity {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This logic was already existing, I just moved into its own function so that I can use either when the folder is updated (title changes) and when the folder moves within another parent folder

let folderFetchRequest = BaseBookmarkEntity.singleEntity(with: folder.id)
let folderFetchRequestResults = try? context.fetch(folderFetchRequest)

guard let bookmarkFolderMO = folderFetchRequestResults?.first else {
assertionFailure("LocalBookmarkStore: Failed to get BookmarkEntity from the context")
throw BookmarkStoreError.missingEntity
}

bookmarkFolderMO.update(with: folder)
return bookmarkFolderMO
}

func move(entities: [BookmarkEntity], toIndex index: Int?, withinParentFolderType type: ParentFolderType, in context: NSManagedObjectContext) throws {
guard let rootFolder = bookmarksRoot(in: context) else {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above, I just moved this existing logic in its own function so I can use it when doing an update and move in one go.

throw BookmarkStoreError.missingRoot
}

let newParentFolder: BookmarkEntity

switch type {
case .root: newParentFolder = rootFolder
case .parent(let newParentUUID):
let bookmarksFetchRequest = BaseBookmarkEntity.singleEntity(with: newParentUUID)

if let fetchedParent = try context.fetch(bookmarksFetchRequest).first, fetchedParent.isFolder {
newParentFolder = fetchedParent
} else {
throw BookmarkStoreError.missingEntity
}
}

if let index = index, index < newParentFolder.childrenArray.count {
self.move(entities: entities, to: index, within: newParentFolder)
} else {
for bookmarkManagedObject in entities {
bookmarkManagedObject.parent = nil
newParentFolder.addToChildren(bookmarkManagedObject)
}
}
}

}

extension LocalBookmarkStore.BookmarkStoreError: CustomNSError {

var errorCode: Int {
Expand Down
Loading
Loading