From 493abbb609585d4c176c9577db043bb2661e4624 Mon Sep 17 00:00:00 2001 From: Maurice Parker Date: Tue, 28 May 2019 09:45:02 -0500 Subject: [PATCH 01/28] Refactor create feed functionality to increase code reuse and encapsulation --- Frameworks/Account/Account.swift | 8 +- Frameworks/Account/AccountDelegate.swift | 4 +- .../Feedbin/FeedbinAccountDelegate.swift | 158 ++++++------------ .../LocalAccount/LocalAccountDelegate.swift | 35 ++-- .../AddFeed/AddFeedController.swift | 54 +----- Mac/Scriptability/Feed+Scriptability.swift | 27 +-- Shared/Commands/DeleteCommand.swift | 8 +- iOS/Add/AddFeedViewController.swift | 66 ++------ 8 files changed, 111 insertions(+), 249 deletions(-) diff --git a/Frameworks/Account/Account.swift b/Frameworks/Account/Account.swift index 4bb5df495..4a53ee32f 100644 --- a/Frameworks/Account/Account.swift +++ b/Frameworks/Account/Account.swift @@ -377,8 +377,8 @@ public final class Account: DisplayNameProvider, UnreadCountProvider, Container, delegate.removeFeed(for: self, from: container, with: feed, completion: completion) } - public func createFeed(url: String, completion: @escaping (Result) -> Void) { - delegate.createFeed(for: self, url: url, completion: completion) + public func createFeed(url: String, name: String?, container: Container, completion: @escaping (Result) -> Void) { + delegate.createFeed(for: self, url: url, name: name, container: container, completion: completion) } func createFeed(with name: String?, url: String, feedID: String, homePageURL: String?) -> Feed { @@ -401,8 +401,8 @@ public final class Account: DisplayNameProvider, UnreadCountProvider, Container, delegate.renameFeed(for: self, with: feed, to: name, completion: completion) } - public func restoreFeed(_ feed: Feed, folder: Folder?, completion: @escaping (Result) -> Void) { - delegate.restoreFeed(for: self, feed: feed, folder: folder, completion: completion) + public func restoreFeed(_ feed: Feed, container: Container, completion: @escaping (Result) -> Void) { + delegate.restoreFeed(for: self, feed: feed, container: container, completion: completion) } public func deleteFolder(_ folder: Folder, completion: @escaping (Result) -> Void) { diff --git a/Frameworks/Account/AccountDelegate.swift b/Frameworks/Account/AccountDelegate.swift index 314a944c5..33dace1a3 100644 --- a/Frameworks/Account/AccountDelegate.swift +++ b/Frameworks/Account/AccountDelegate.swift @@ -31,14 +31,14 @@ protocol AccountDelegate { func renameFolder(for account: Account, with folder: Folder, to name: String, completion: @escaping (Result) -> Void) func deleteFolder(for account: Account, with folder: Folder, completion: @escaping (Result) -> Void) - func createFeed(for account: Account, url: String, completion: @escaping (Result) -> Void) + func createFeed(for account: Account, url: String, name: String?, container: Container, completion: @escaping (Result) -> Void) func renameFeed(for account: Account, with feed: Feed, to name: String, completion: @escaping (Result) -> Void) func deleteFeed(for account: Account, with feed: Feed, completion: @escaping (Result) -> Void) func addFeed(for account: Account, to container: Container, with: Feed, completion: @escaping (Result) -> Void) func removeFeed(for account: Account, from container: Container, with: Feed, completion: @escaping (Result) -> Void) - func restoreFeed(for account: Account, feed: Feed, folder: Folder?, completion: @escaping (Result) -> Void) + func restoreFeed(for account: Account, feed: Feed, container: Container, completion: @escaping (Result) -> Void) func restoreFolder(for account: Account, folder: Folder, completion: @escaping (Result) -> Void) func markArticles(for account: Account, articles: Set
, statusKey: ArticleStatus.Key, flag: Bool) -> Set
? diff --git a/Frameworks/Account/Feedbin/FeedbinAccountDelegate.swift b/Frameworks/Account/Feedbin/FeedbinAccountDelegate.swift index eddcd43b1..0a9b223d5 100644 --- a/Frameworks/Account/Feedbin/FeedbinAccountDelegate.swift +++ b/Frameworks/Account/Feedbin/FeedbinAccountDelegate.swift @@ -284,16 +284,16 @@ final class FeedbinAccountDelegate: AccountDelegate { } - func createFeed(for account: Account, url: String, completion: @escaping (Result) -> Void) { + func createFeed(for account: Account, url: String, name: String?, container: Container, completion: @escaping (Result) -> Void) { caller.createSubscription(url: url) { result in switch result { case .success(let subResult): switch subResult { case .created(let subscription): - self.createFeed(account: account, subscription: subscription, completion: completion) + self.createFeed(account: account, subscription: subscription, name: name, container: container, completion: completion) case .multipleChoice(let choices): - self.decideBestFeedChoice(account: account, url: url, choices: choices, completion: completion) + self.decideBestFeedChoice(account: account, url: url, name: name, container: container, choices: choices, completion: completion) case .alreadySubscribed: DispatchQueue.main.async { completion(.failure(AccountError.createErrorAlreadySubscribed)) @@ -424,19 +424,14 @@ final class FeedbinAccountDelegate: AccountDelegate { } - func restoreFeed(for account: Account, feed: Feed, folder: Folder?, completion: @escaping (Result) -> Void) { + func restoreFeed(for account: Account, feed: Feed, container: Container, completion: @escaping (Result) -> Void) { - let editedName = feed.editedName - - createFeed(for: account, url: feed.url) { result in + createFeed(for: account, url: feed.url, name: feed.editedName, container: container) { result in switch result { - case .success(let feed): - self.processRestoredFeed(for: account, feed: feed, editedName: editedName, folder: folder, completion: completion) + case .success: + completion(.success(())) case .failure(let error): - DispatchQueue.main.async { - let wrappedError = AccountError.wrappedError(error: error, account: account) - completion(.failure(wrappedError)) - } + completion(.failure(error)) } } @@ -824,74 +819,6 @@ private extension FeedbinAccountDelegate { } - func processRestoredFeed(for account: Account, feed: Feed, editedName: String?, folder: Folder?, completion: @escaping (Result) -> Void) { - - if let folder = folder { - - addFeed(for: account, to: folder, with: feed) { result in - - switch result { - case .success: - - if editedName != nil { - DispatchQueue.main.async { - account.removeFeed(feed) - folder.addFeed(feed) - } - self.processRestoredFeedName(for: account, feed: feed, editedName: editedName!, completion: completion) - } else { - DispatchQueue.main.async { - account.removeFeed(feed) - folder.addFeed(feed) - completion(.success(())) - } - } - - case .failure(let error): - DispatchQueue.main.async { - completion(.failure(error)) - } - } - - } - - } else { - - DispatchQueue.main.async { - account.addFeed(feed) - } - - if editedName != nil { - processRestoredFeedName(for: account, feed: feed, editedName: editedName!, completion: completion) - } else { - DispatchQueue.main.async { - completion(.success(())) - } - } - - } - - } - - func processRestoredFeedName(for account: Account, feed: Feed, editedName: String, completion: @escaping (Result) -> Void) { - - renameFeed(for: account, with: feed, to: editedName) { result in - switch result { - case .success: - DispatchQueue.main.async { - feed.editedName = editedName - completion(.success(())) - } - case .failure(let error): - DispatchQueue.main.async { - completion(.failure(error)) - } - } - - } - - } - func clearFolderRelationship(for feed: Feed, withFolderName folderName: String) { if var folderRelationship = feed.folderRelationship { folderRelationship[folderName] = nil @@ -908,7 +835,7 @@ private extension FeedbinAccountDelegate { } } - func decideBestFeedChoice(account: Account, url: String, choices: [FeedbinSubscriptionChoice], completion: @escaping (Result) -> Void) { + func decideBestFeedChoice(account: Account, url: String, name: String?, container: Container, choices: [FeedbinSubscriptionChoice], completion: @escaping (Result) -> Void) { let feedSpecifiers: [FeedSpecifier] = choices.map { choice in let source = url == choice.url ? FeedSpecifier.Source.UserEntered : FeedSpecifier.Source.HTMLLink @@ -918,7 +845,7 @@ private extension FeedbinAccountDelegate { if let bestSpecifier = FeedSpecifier.bestFeed(in: Set(feedSpecifiers)) { if let bestSubscription = choices.filter({ bestSpecifier.urlString == $0.url }).first { - createFeed(for: account, url: bestSubscription.url, completion: completion) + createFeed(for: account, url: bestSubscription.url, name: name, container: container, completion: completion) } else { DispatchQueue.main.async { completion(.failure(FeedbinAccountDelegateError.invalidParameter)) @@ -932,44 +859,67 @@ private extension FeedbinAccountDelegate { } - func createFeed( account: Account, subscription sub: FeedbinSubscription, completion: @escaping (Result) -> Void) { + func createFeed( account: Account, subscription sub: FeedbinSubscription, name: String?, container: Container, completion: @escaping (Result) -> Void) { + DispatchQueue.main.async { let feed = account.createFeed(with: sub.name, url: sub.url, feedID: String(sub.feedID), homePageURL: sub.homePageURL) feed.subscriptionID = String(sub.subscriptionID) - // Download the initial articles - self.caller.retrieveEntries(feedID: feed.feedID) { result in - + container.addFeed(feed) { result in switch result { - case .success(let (entries, page)): - - self.processEntries(account: account, entries: entries) { - self.refreshArticles(account, page: page) { - self.refreshArticleStatus(for: account) { - self.refreshMissingArticles(account) { - DispatchQueue.main.async { - completion(.success(feed)) - } - } + case .success: + if let name = name { + account.renameFeed(feed, to: name) { result in + switch result { + case .success: + self.initialFeedDownload(account: account, feed: feed, completion: completion) + case .failure(let error): + completion(.failure(error)) } } + } else { + self.initialFeedDownload(account: account, feed: feed, completion: completion) } - case .failure(let error): - os_log(.error, log: self.log, "Initial articles download failed: %@.", error.localizedDescription) - DispatchQueue.main.async { - completion(.success(feed)) - } + completion(.failure(error)) } - } - + } } + func initialFeedDownload( account: Account, feed: Feed, completion: @escaping (Result) -> Void) { + + // Download the initial articles + self.caller.retrieveEntries(feedID: feed.feedID) { result in + + switch result { + case .success(let (entries, page)): + + self.processEntries(account: account, entries: entries) { + self.refreshArticles(account, page: page) { + self.refreshArticleStatus(for: account) { + self.refreshMissingArticles(account) { + DispatchQueue.main.async { + completion(.success(feed)) + } + } + } + } + } + + case .failure(let error): + completion(.failure(error)) + } + + } + + + } + func refreshArticles(_ account: Account, completion: @escaping (() -> Void)) { os_log(.debug, log: log, "Refreshing articles...") diff --git a/Frameworks/Account/LocalAccount/LocalAccountDelegate.swift b/Frameworks/Account/LocalAccount/LocalAccountDelegate.swift index f9efdbc31..10e5ce446 100644 --- a/Frameworks/Account/LocalAccount/LocalAccountDelegate.swift +++ b/Frameworks/Account/LocalAccount/LocalAccountDelegate.swift @@ -27,6 +27,8 @@ final class LocalAccountDelegate: AccountDelegate { private weak var account: Account? private var feedFinder: FeedFinder? + private var createFeedName: String? + private var createFeedContainer: Container? private var createFeedCompletion: ((Result) -> Void)? private let refresher = LocalAccountRefresher() @@ -99,7 +101,7 @@ final class LocalAccountDelegate: AccountDelegate { completion(.success(())) } - func createFeed(for account: Account, url urlString: String, completion: @escaping (Result) -> Void) { + func createFeed(for account: Account, url urlString: String, name: String?, container: Container, completion: @escaping (Result) -> Void) { guard let url = URL(string: urlString) else { completion(.failure(LocalAccountDelegateError.invalidParameter)) @@ -107,8 +109,9 @@ final class LocalAccountDelegate: AccountDelegate { } self.account = account + createFeedName = name + createFeedContainer = container createFeedCompletion = completion - feedFinder = FeedFinder(url: url, delegate: self) } @@ -143,10 +146,8 @@ final class LocalAccountDelegate: AccountDelegate { func addFeed(for account: Account, to container: Container, with feed: Feed, completion: @escaping (Result) -> Void) { if let folder = container as? Folder { folder.addFeed(feed) - feed.account = folder.account } else if let account = container as? Account { account.addFeed(feed) - feed.account = account } completion(.success(())) } @@ -161,13 +162,8 @@ final class LocalAccountDelegate: AccountDelegate { completion(.success(())) } - func restoreFeed(for account: Account, feed: Feed, folder: Folder?, completion: @escaping (Result) -> Void) { - if let folder = folder { - folder.addFeed(feed) - } else { - account.addFeed(feed) - } - completion(.success(())) + func restoreFeed(for account: Account, feed: Feed, container: Container, completion: @escaping (Result) -> Void) { + container.addFeed(feed, completion: completion) } func restoreFolder(for account: Account, folder: Folder, completion: @escaping (Result) -> Void) { @@ -216,11 +212,24 @@ extension LocalAccountDelegate: FeedFinderDelegate { } let feed = account.createFeed(with: nil, url: url.absoluteString, feedID: url.absoluteString, homePageURL: nil) - InitialFeedDownloader.download(url) { [weak self] parsedFeed in + + InitialFeedDownloader.download(url) { parsedFeed in + if let parsedFeed = parsedFeed { account.update(feed, with: parsedFeed, {}) } - self?.createFeedCompletion!(.success(feed)) + + feed.editedName = self.createFeedName + + self.createFeedContainer?.addFeed(feed) { result in + switch result { + case .success: + self.createFeedCompletion?(.success(feed)) + case .failure(let error): + self.createFeedCompletion?(.failure(error)) + } + } + } } diff --git a/Mac/MainWindow/AddFeed/AddFeedController.swift b/Mac/MainWindow/AddFeed/AddFeedController.swift index 8b1babdbe..7475be1b3 100644 --- a/Mac/MainWindow/AddFeed/AddFeedController.swift +++ b/Mac/MainWindow/AddFeed/AddFeedController.swift @@ -52,7 +52,6 @@ class AddFeedController: AddFeedWindowControllerDelegate { return } let account = accountAndFolderSpecifier.account - let folder = accountAndFolderSpecifier.folder if account.hasFeed(withURL: url.absoluteString) { showAlreadySubscribedError(url.absoluteString) @@ -61,20 +60,20 @@ class AddFeedController: AddFeedWindowControllerDelegate { BatchUpdate.shared.start() - account.createFeed(url: url.absoluteString) { [weak self] result in - - self?.endShowingProgress() + account.createFeed(url: url.absoluteString, name: title, container: container) { result in + self.endShowingProgress() + BatchUpdate.shared.end() + switch result { case .success(let feed): - self?.processFeed(feed, account: account, folder: folder, url: url, title: title) + NotificationCenter.default.post(name: .UserDidAddFeed, object: self, userInfo: [UserInfoKey.feed: feed]) case .failure(let error): - BatchUpdate.shared.end() switch error { case AccountError.createErrorAlreadySubscribed: - self?.showAlreadySubscribedError(url.absoluteString) + self.showAlreadySubscribedError(url.absoluteString) case AccountError.createErrorNotFound: - self?.showNoFeedsErrorMessage() + self.showNoFeedsErrorMessage() default: NSApplication.shared.presentError(error) } @@ -125,45 +124,6 @@ private extension AddFeedController { } } - func processFeed(_ feed: Feed, account: Account, folder: Folder?, url: URL, title: String?) { - - if let title = title { - account.renameFeed(feed, to: title) { result in - switch result { - case .success: - break - case .failure(let error): - NSApplication.shared.presentError(error) - } - } - } - - if let folder = folder { - folder.addFeed(feed) { result in - switch result { - case .success: - BatchUpdate.shared.end() - NotificationCenter.default.post(name: .UserDidAddFeed, object: self, userInfo: [UserInfoKey.feed: feed]) - case .failure(let error): - BatchUpdate.shared.end() - NSApplication.shared.presentError(error) - } - } - } else { - account.addFeed(feed) { result in - switch result { - case .success: - BatchUpdate.shared.end() - NotificationCenter.default.post(name: .UserDidAddFeed, object: self, userInfo: [UserInfoKey.feed: feed]) - case .failure(let error): - BatchUpdate.shared.end() - NSApplication.shared.presentError(error) - } - } - } - - } - // MARK: Errors func showAlreadySubscribedError(_ urlString: String) { diff --git a/Mac/Scriptability/Feed+Scriptability.swift b/Mac/Scriptability/Feed+Scriptability.swift index db385b312..42fefde87 100644 --- a/Mac/Scriptability/Feed+Scriptability.swift +++ b/Mac/Scriptability/Feed+Scriptability.swift @@ -91,7 +91,9 @@ class ScriptableFeed: NSObject, UniqueIdScriptingObject, ScriptingObjectContaine if let existingFeed = account.existingFeed(withURL:url) { return self.scriptableFeed(existingFeed, account:account, folder:folder) } - + + let container: Container = folder != nil ? folder! : account + // at this point, we need to download the feed and parse it. // RS Parser does the callback for the download on the main thread (which it probably shouldn't?) // because we can't wait here (on the main thread, maybe) for the callback, we have to return from this function @@ -100,27 +102,12 @@ class ScriptableFeed: NSObject, UniqueIdScriptingObject, ScriptingObjectContaine // suspendExecution(). When we get the callback, we can supply the event result and call resumeExecution() command.suspendExecution() - account.createFeed(url: url) { result in + account.createFeed(url: url, name: titleFromArgs, container: container) { result in switch result { case .success(let feed): - - if let editedName = titleFromArgs { - account.renameFeed(feed, to: editedName) { result in - } - } - - // add the feed, putting it in a folder if needed - account.addFeed(feed) { result in - switch result { - case .success: - NotificationCenter.default.post(name: .UserDidAddFeed, object: self, userInfo: [UserInfoKey.feed: feed]) - let scriptableFeed = self.scriptableFeed(feed, account:account, folder:folder) - command.resumeExecution(withResult:scriptableFeed.objectSpecifier) - case .failure: - command.resumeExecution(withResult:nil) - } - } - + NotificationCenter.default.post(name: .UserDidAddFeed, object: self, userInfo: [UserInfoKey.feed: feed]) + let scriptableFeed = self.scriptableFeed(feed, account:account, folder:folder) + command.resumeExecution(withResult:scriptableFeed.objectSpecifier) case .failure: command.resumeExecution(withResult:nil) } diff --git a/Shared/Commands/DeleteCommand.swift b/Shared/Commands/DeleteCommand.swift index afd07f1f2..309a8c8f0 100644 --- a/Shared/Commands/DeleteCommand.swift +++ b/Shared/Commands/DeleteCommand.swift @@ -161,12 +161,12 @@ private struct SidebarItemSpecifier { private func restoreFeed() { - guard let account = account, let feed = feed else { + guard let account = account, let feed = feed, let container = path.resolveContainer() else { return } BatchUpdate.shared.start() - account.restoreFeed(feed, folder: resolvedFolder()) { result in + account.restoreFeed(feed, container: container) { result in BatchUpdate.shared.end() self.checkResult(result) } @@ -187,10 +187,6 @@ private struct SidebarItemSpecifier { } - private func resolvedFolder() -> Folder? { - return path.resolveContainer() as? Folder - } - private func checkResult(_ result: Result) { switch result { diff --git a/iOS/Add/AddFeedViewController.swift b/iOS/Add/AddFeedViewController.swift index b2b84cee0..b2dc3464b 100644 --- a/iOS/Add/AddFeedViewController.swift +++ b/iOS/Add/AddFeedViewController.swift @@ -80,13 +80,10 @@ class AddFeedViewController: UITableViewController, AddContainerViewControllerCh let container = pickerData.containers[folderPickerView.selectedRow(inComponent: 0)] var account: Account? - var folder: Folder? if let containerAccount = container as? Account { account = containerAccount - } - if let containerFolder = container as? Folder, let containerAccount = containerFolder.account { + } else if let containerFolder = container as? Folder, let containerAccount = containerFolder.account { account = containerAccount - folder = containerFolder } if account!.hasFeed(withURL: url.absoluteString) { @@ -94,26 +91,28 @@ class AddFeedViewController: UITableViewController, AddContainerViewControllerCh return } - let title = nameTextField.text - delegate?.processingDidBegin() + BatchUpdate.shared.start() + + account!.createFeed(url: url.absoluteString, name: nameTextField.text, container: container) { result in - account!.createFeed(url: url.absoluteString) { [weak self] result in + BatchUpdate.shared.end() switch result { case .success(let feed): - self?.processFeed(feed, account: account!, folder: folder, url: url, title: title) + self.delegate?.processingDidEnd() + NotificationCenter.default.post(name: .UserDidAddFeed, object: self, userInfo: [UserInfoKey.feed: feed]) case .failure(let error): switch error { case AccountError.createErrorAlreadySubscribed: - self?.showAlreadySubscribedError() - self?.delegate?.processingDidCancel() + self.showAlreadySubscribedError() + self.delegate?.processingDidCancel() case AccountError.createErrorNotFound: - self?.showNoFeedsErrorMessage() - self?.delegate?.processingDidCancel() + self.showNoFeedsErrorMessage() + self.delegate?.processingDidCancel() default: - self?.presentError(error) - self?.delegate?.processingDidCancel() + self.presentError(error) + self.delegate?.processingDidCancel() } } @@ -178,45 +177,6 @@ private extension AddFeedViewController { presentError(title: title, message: message as String) } - func processFeed(_ feed: Feed, account: Account, folder: Folder?, url: URL, title: String?) { - - if let title = title { - account.renameFeed(feed, to: title) { [weak self] result in - switch result { - case .success: - break - case .failure(let error): - self?.presentError(error) - } - } - } - - if let folder = folder { - folder.addFeed(feed) { [weak self] result in - switch result { - case .success: - self?.delegate?.processingDidEnd() - NotificationCenter.default.post(name: .UserDidAddFeed, object: self, userInfo: [UserInfoKey.feed: feed]) - case .failure(let error): - self?.delegate?.processingDidEnd() - self?.presentError(error) - } - } - } else { - account.addFeed(feed) { [weak self] result in - switch result { - case .success: - self?.delegate?.processingDidEnd() - NotificationCenter.default.post(name: .UserDidAddFeed, object: self, userInfo: [UserInfoKey.feed: feed]) - case .failure(let error): - self?.delegate?.processingDidEnd() - self?.presentError(error) - } - } - } - - } - } extension AddFeedViewController: UITextFieldDelegate { From b3c4c8de5930ec8f74fc18c2755e32714486caff Mon Sep 17 00:00:00 2001 From: Maurice Parker Date: Tue, 28 May 2019 10:59:06 -0500 Subject: [PATCH 02/28] Correct how feeds were copied and moved between accounts to eliminate shared objects. --- .../Sidebar/SidebarOutlineDataSource.swift | 89 +++++++++++++------ 1 file changed, 60 insertions(+), 29 deletions(-) diff --git a/Mac/MainWindow/Sidebar/SidebarOutlineDataSource.swift b/Mac/MainWindow/Sidebar/SidebarOutlineDataSource.swift index fed4cda2d..864f7bdfe 100644 --- a/Mac/MainWindow/Sidebar/SidebarOutlineDataSource.swift +++ b/Mac/MainWindow/Sidebar/SidebarOutlineDataSource.swift @@ -243,27 +243,7 @@ private extension SidebarOutlineDataSource { return accounts } - private func copy(node: Node, to parentNode: Node) { - guard let feed = node.representedObject as? Feed else { - return - } - - let destination = parentNode.representedObject as? Container - - BatchUpdate.shared.start() - destination?.addFeed(feed) { result in - switch result { - case .success: - BatchUpdate.shared.end() - break - case .failure(let error): - BatchUpdate.shared.end() - NSApplication.shared.presentError(error) - } - } - } - - private func move(node: Node, to parentNode: Node) { + private func moveInAccount(node: Node, to parentNode: Node) { guard let feed = node.representedObject as? Feed else { return } @@ -279,7 +259,6 @@ private extension SidebarOutlineDataSource { switch result { case .success: BatchUpdate.shared.end() - break case .failure(let error): // If the second part of the move failed, try to put the feed back source?.addFeed(feed) { result in} @@ -294,6 +273,53 @@ private extension SidebarOutlineDataSource { } } + private func copyBetweenAccounts(node: Node, to parentNode: Node) { + guard let feed = node.representedObject as? Feed, + let destinationAccount = nodeAccount(parentNode), + let destinationContainer = parentNode.representedObject as? Container else { + return + } + + BatchUpdate.shared.start() + destinationAccount.createFeed(url: feed.url, name: feed.editedName, container: destinationContainer) { result in + switch result { + case .success: + BatchUpdate.shared.end() + case .failure(let error): + BatchUpdate.shared.end() + NSApplication.shared.presentError(error) + } + } + } + + private func moveBetweenAccounts(node: Node, to parentNode: Node) { + guard let feed = node.representedObject as? Feed, + let sourceAccount = nodeAccount(node), + let destinationAccount = nodeAccount(parentNode), + let destinationContainer = parentNode.representedObject as? Container else { + return + } + + BatchUpdate.shared.start() + destinationAccount.createFeed(url: feed.url, name: feed.editedName, container: destinationContainer) { result in + switch result { + case .success: + sourceAccount.deleteFeed(feed) { result in + switch result { + case .success: + BatchUpdate.shared.end() + case .failure(let error): + BatchUpdate.shared.end() + NSApplication.shared.presentError(error) + } + } + case .failure(let error): + BatchUpdate.shared.end() + NSApplication.shared.presentError(error) + } + } + } + func acceptLocalFeedsDrop(_ outlineView: NSOutlineView, _ draggedFeeds: Set, _ parentNode: Node, _ index: Int) -> Bool { guard let draggedNodes = draggedNodes else { return false @@ -303,11 +329,11 @@ private extension SidebarOutlineDataSource { draggedNodes.forEach { node in if sameAccount(node, parentNode) { - move(node: node, to: parentNode) + moveInAccount(node: node, to: parentNode) } else if NSApplication.shared.currentEvent?.modifierFlags.contains(.option) ?? false { - copy(node: node, to: parentNode) + copyBetweenAccounts(node: node, to: parentNode) } else { - move(node: node, to: parentNode) + moveBetweenAccounts(node: node, to: parentNode) } } @@ -432,16 +458,21 @@ private extension SidebarOutlineDataSource { return false } - func nodeAccountID(_ node: Node) -> String? { + func nodeAccount(_ node: Node) -> Account? { if let account = node.representedObject as? Account { - return account.accountID + return account } else if let folder = node.representedObject as? Folder { - return folder.account?.accountID + return folder.account } else if let feed = node.representedObject as? Feed { - return feed.account?.accountID + return feed.account } else { return nil } + + } + + func nodeAccountID(_ node: Node) -> String? { + return nodeAccount(node)?.accountID } func nodeHasChildRepresentingAnyDraggedFeed(_ parentNode: Node, _ draggedFeeds: Set) -> Bool { From 78c19bda43afedd8994495017e4d0891e59d2a78 Mon Sep 17 00:00:00 2001 From: Maurice Parker Date: Tue, 28 May 2019 11:09:47 -0500 Subject: [PATCH 03/28] Remove restriction on only moving and copying between local accounts. --- .../Sidebar/SidebarOutlineDataSource.swift | 30 ------------------- 1 file changed, 30 deletions(-) diff --git a/Mac/MainWindow/Sidebar/SidebarOutlineDataSource.swift b/Mac/MainWindow/Sidebar/SidebarOutlineDataSource.swift index 864f7bdfe..b9a0b0b6f 100644 --- a/Mac/MainWindow/Sidebar/SidebarOutlineDataSource.swift +++ b/Mac/MainWindow/Sidebar/SidebarOutlineDataSource.swift @@ -173,9 +173,6 @@ private extension SidebarOutlineDataSource { guard let dropTargetNode = ancestorThatCanAcceptLocalFeed(parentNode) else { return SidebarOutlineDataSource.dragOperationNone } - if !allParticipantsAreLocalAccounts(dropTargetNode, Set([draggedFeed])) { - return SidebarOutlineDataSource.dragOperationNone - } if nodeHasChildRepresentingDraggedFeed(dropTargetNode, draggedFeed) { return SidebarOutlineDataSource.dragOperationNone } @@ -195,9 +192,6 @@ private extension SidebarOutlineDataSource { guard let dropTargetNode = ancestorThatCanAcceptLocalFeed(parentNode) else { return SidebarOutlineDataSource.dragOperationNone } - if !allParticipantsAreLocalAccounts(dropTargetNode, draggedFeeds) { - return SidebarOutlineDataSource.dragOperationNone - } if nodeHasChildRepresentingAnyDraggedFeed(dropTargetNode, draggedFeeds) { return SidebarOutlineDataSource.dragOperationNone } @@ -411,30 +405,6 @@ private extension SidebarOutlineDataSource { return false } - func allParticipantsAreLocalAccounts(_ parentNode: Node, _ draggedFeeds: Set) -> Bool { - - if let account = parentNode.representedObject as? Account { - if account.type != .onMyMac { - return false - } - } else if let folder = parentNode.representedObject as? Folder { - if folder.account?.type != .onMyMac { - return false - } - } else { - return false - } - - for draggedFeed in draggedFeeds { - if draggedFeed.accountType != .onMyMac { - return false - } - } - - return true - - } - func allParticipantsAreSameAccount(_ parentNode: Node, _ draggedFeeds: Set) -> Bool { guard let parentAccountID = nodeAccountID(parentNode) else { return false From 112702020b5bcec266a2ebd3576ae7704893d9a9 Mon Sep 17 00:00:00 2001 From: Maurice Parker Date: Tue, 28 May 2019 13:11:29 -0500 Subject: [PATCH 04/28] Enable same account copying. --- .../Sidebar/SidebarOutlineDataSource.swift | 64 +++++++++---------- 1 file changed, 30 insertions(+), 34 deletions(-) diff --git a/Mac/MainWindow/Sidebar/SidebarOutlineDataSource.swift b/Mac/MainWindow/Sidebar/SidebarOutlineDataSource.swift index b9a0b0b6f..903373e54 100644 --- a/Mac/MainWindow/Sidebar/SidebarOutlineDataSource.swift +++ b/Mac/MainWindow/Sidebar/SidebarOutlineDataSource.swift @@ -202,9 +202,6 @@ private extension SidebarOutlineDataSource { } func localFeedsDropOperation(_ dropTargetNode: Node, _ draggedFeeds: Set) -> NSDragOperation { - if allParticipantsAreSameAccount(dropTargetNode, draggedFeeds) { - return .move - } if NSApplication.shared.currentEvent?.modifierFlags.contains(.option) ?? false { return .copy } else { @@ -237,6 +234,23 @@ private extension SidebarOutlineDataSource { return accounts } + private func copyInAccount(node: Node, to parentNode: Node) { + guard let feed = node.representedObject as? Feed else { + return + } + + let destination = parentNode.representedObject as? Container + + destination?.addFeed(feed) { result in + switch result { + case .success: + break + case .failure(let error): + NSApplication.shared.presentError(error) + } + } + } + private func moveInAccount(node: Node, to parentNode: Node) { guard let feed = node.representedObject as? Feed else { return @@ -245,25 +259,20 @@ private extension SidebarOutlineDataSource { let source = node.parent?.representedObject as? Container let destination = parentNode.representedObject as? Container - BatchUpdate.shared.start() source?.removeFeed(feed) { result in switch result { case .success: destination?.addFeed(feed) { result in switch result { case .success: - BatchUpdate.shared.end() + break case .failure(let error): - // If the second part of the move failed, try to put the feed back - source?.addFeed(feed) { result in} - BatchUpdate.shared.end() NSApplication.shared.presentError(error) } } case .failure(let error): NSApplication.shared.presentError(error) } - } } @@ -274,13 +283,11 @@ private extension SidebarOutlineDataSource { return } - BatchUpdate.shared.start() destinationAccount.createFeed(url: feed.url, name: feed.editedName, container: destinationContainer) { result in switch result { case .success: - BatchUpdate.shared.end() + break case .failure(let error): - BatchUpdate.shared.end() NSApplication.shared.presentError(error) } } @@ -294,21 +301,18 @@ private extension SidebarOutlineDataSource { return } - BatchUpdate.shared.start() destinationAccount.createFeed(url: feed.url, name: feed.editedName, container: destinationContainer) { result in switch result { case .success: sourceAccount.deleteFeed(feed) { result in switch result { case .success: - BatchUpdate.shared.end() + break case .failure(let error): - BatchUpdate.shared.end() NSApplication.shared.presentError(error) } } case .failure(let error): - BatchUpdate.shared.end() NSApplication.shared.presentError(error) } } @@ -323,11 +327,17 @@ private extension SidebarOutlineDataSource { draggedNodes.forEach { node in if sameAccount(node, parentNode) { - moveInAccount(node: node, to: parentNode) - } else if NSApplication.shared.currentEvent?.modifierFlags.contains(.option) ?? false { - copyBetweenAccounts(node: node, to: parentNode) + if NSApplication.shared.currentEvent?.modifierFlags.contains(.option) ?? false { + copyInAccount(node: node, to: parentNode) + } else { + moveInAccount(node: node, to: parentNode) + } } else { - moveBetweenAccounts(node: node, to: parentNode) + if NSApplication.shared.currentEvent?.modifierFlags.contains(.option) ?? false { + copyBetweenAccounts(node: node, to: parentNode) + } else { + moveBetweenAccounts(node: node, to: parentNode) + } } } @@ -405,20 +415,6 @@ private extension SidebarOutlineDataSource { return false } - func allParticipantsAreSameAccount(_ parentNode: Node, _ draggedFeeds: Set) -> Bool { - guard let parentAccountID = nodeAccountID(parentNode) else { - return false - } - - for draggedFeed in draggedFeeds { - if draggedFeed.accountID != parentAccountID { - return false - } - } - - return true - } - func sameAccount(_ node: Node, _ parentNode: Node) -> Bool { if let accountID = nodeAccountID(node), let parentAccountID = nodeAccountID(parentNode) { if accountID == parentAccountID { From cf016c5d7d455ce7deb574f498d634bb9610391c Mon Sep 17 00:00:00 2001 From: Maurice Parker Date: Tue, 28 May 2019 13:38:40 -0500 Subject: [PATCH 05/28] Prevent Feedbin in account copy from putting a feed in both the root account and a folder at the same time. --- Frameworks/Account/Account.swift | 6 ++++++ Frameworks/Account/Feedbin/FeedbinAccountDelegate.swift | 4 +++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/Frameworks/Account/Account.swift b/Frameworks/Account/Account.swift index 4a53ee32f..5972c28d0 100644 --- a/Frameworks/Account/Account.swift +++ b/Frameworks/Account/Account.swift @@ -675,6 +675,12 @@ public final class Account: DisplayNameProvider, UnreadCountProvider, Container, postChildrenDidChangeNotification() } + func addFeedIfNotInAnyFolder(_ feed: Feed) { + if !flattenedFeeds().contains(feed) { + addFeed(feed) + } + } + func deleteFolder(_ folder: Folder) { folders?.remove(folder) structureDidChange() diff --git a/Frameworks/Account/Feedbin/FeedbinAccountDelegate.swift b/Frameworks/Account/Feedbin/FeedbinAccountDelegate.swift index 0a9b223d5..7520d0642 100644 --- a/Frameworks/Account/Feedbin/FeedbinAccountDelegate.swift +++ b/Frameworks/Account/Feedbin/FeedbinAccountDelegate.swift @@ -377,6 +377,7 @@ final class FeedbinAccountDelegate: AccountDelegate { case .success(let taggingID): DispatchQueue.main.async { self.saveFolderRelationship(for: feed, withFolderName: folder.name ?? "", id: String(taggingID)) + account.removeFeed(feed) folder.addFeed(feed) completion(.success(())) } @@ -389,7 +390,7 @@ final class FeedbinAccountDelegate: AccountDelegate { } } else { if let account = container as? Account { - account.addFeed(feed) + account.addFeedIfNotInAnyFolder(feed) } DispatchQueue.main.async { completion(.success(())) @@ -406,6 +407,7 @@ final class FeedbinAccountDelegate: AccountDelegate { case .success: DispatchQueue.main.async { folder.removeFeed(feed) + account.addFeedIfNotInAnyFolder(feed) completion(.success(())) } case .failure(let error): From 01d5a95241f58505e8af2e3fe83decc4343f3da6 Mon Sep 17 00:00:00 2001 From: Maurice Parker Date: Tue, 28 May 2019 16:31:03 -0500 Subject: [PATCH 06/28] Remove redundant private keyword usage --- .../Sidebar/SidebarOutlineDataSource.swift | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Mac/MainWindow/Sidebar/SidebarOutlineDataSource.swift b/Mac/MainWindow/Sidebar/SidebarOutlineDataSource.swift index 903373e54..e304e30c0 100644 --- a/Mac/MainWindow/Sidebar/SidebarOutlineDataSource.swift +++ b/Mac/MainWindow/Sidebar/SidebarOutlineDataSource.swift @@ -209,7 +209,7 @@ private extension SidebarOutlineDataSource { } } - private func accountForNode(_ node: Node) -> Account? { + func accountForNode(_ node: Node) -> Account? { if let account = node.representedObject as? Account { return account } @@ -222,7 +222,7 @@ private extension SidebarOutlineDataSource { return nil } - private func commonAccountsFor(_ nodes: Set) -> Set { + func commonAccountsFor(_ nodes: Set) -> Set { var accounts = Set() for node in nodes { @@ -234,7 +234,7 @@ private extension SidebarOutlineDataSource { return accounts } - private func copyInAccount(node: Node, to parentNode: Node) { + func copyInAccount(node: Node, to parentNode: Node) { guard let feed = node.representedObject as? Feed else { return } @@ -251,7 +251,7 @@ private extension SidebarOutlineDataSource { } } - private func moveInAccount(node: Node, to parentNode: Node) { + func moveInAccount(node: Node, to parentNode: Node) { guard let feed = node.representedObject as? Feed else { return } @@ -276,7 +276,7 @@ private extension SidebarOutlineDataSource { } } - private func copyBetweenAccounts(node: Node, to parentNode: Node) { + func copyBetweenAccounts(node: Node, to parentNode: Node) { guard let feed = node.representedObject as? Feed, let destinationAccount = nodeAccount(parentNode), let destinationContainer = parentNode.representedObject as? Container else { @@ -293,7 +293,7 @@ private extension SidebarOutlineDataSource { } } - private func moveBetweenAccounts(node: Node, to parentNode: Node) { + func moveBetweenAccounts(node: Node, to parentNode: Node) { guard let feed = node.representedObject as? Feed, let sourceAccount = nodeAccount(node), let destinationAccount = nodeAccount(parentNode), From 83652c40dea11229fdc3ecb55fbb0e138427ce20 Mon Sep 17 00:00:00 2001 From: Maurice Parker Date: Tue, 28 May 2019 16:46:16 -0500 Subject: [PATCH 07/28] Handle scenario where moved/copied feed already exists somewhere else in account --- .../Sidebar/SidebarOutlineDataSource.swift | 67 ++++++++++++++----- 1 file changed, 50 insertions(+), 17 deletions(-) diff --git a/Mac/MainWindow/Sidebar/SidebarOutlineDataSource.swift b/Mac/MainWindow/Sidebar/SidebarOutlineDataSource.swift index e304e30c0..aa85636de 100644 --- a/Mac/MainWindow/Sidebar/SidebarOutlineDataSource.swift +++ b/Mac/MainWindow/Sidebar/SidebarOutlineDataSource.swift @@ -283,12 +283,23 @@ private extension SidebarOutlineDataSource { return } - destinationAccount.createFeed(url: feed.url, name: feed.editedName, container: destinationContainer) { result in - switch result { - case .success: - break - case .failure(let error): - NSApplication.shared.presentError(error) + if let existingFeed = destinationAccount.existingFeed(withURL: feed.url) { + destinationContainer.addFeed(existingFeed) { result in + switch result { + case .success: + break + case .failure(let error): + NSApplication.shared.presentError(error) + } + } + } else { + destinationAccount.createFeed(url: feed.url, name: feed.editedName, container: destinationContainer) { result in + switch result { + case .success: + break + case .failure(let error): + NSApplication.shared.presentError(error) + } } } } @@ -301,20 +312,42 @@ private extension SidebarOutlineDataSource { return } - destinationAccount.createFeed(url: feed.url, name: feed.editedName, container: destinationContainer) { result in - switch result { - case .success: - sourceAccount.deleteFeed(feed) { result in - switch result { - case .success: - break - case .failure(let error): - NSApplication.shared.presentError(error) + if let existingFeed = destinationAccount.existingFeed(withURL: feed.url) { + + destinationContainer.addFeed(existingFeed) { result in + switch result { + case .success: + sourceAccount.deleteFeed(feed) { result in + switch result { + case .success: + break + case .failure(let error): + NSApplication.shared.presentError(error) + } } + case .failure(let error): + NSApplication.shared.presentError(error) } - case .failure(let error): - NSApplication.shared.presentError(error) } + + } else { + + destinationAccount.createFeed(url: feed.url, name: feed.editedName, container: destinationContainer) { result in + switch result { + case .success: + sourceAccount.deleteFeed(feed) { result in + switch result { + case .success: + break + case .failure(let error): + NSApplication.shared.presentError(error) + } + } + case .failure(let error): + NSApplication.shared.presentError(error) + } + } + } } From 0648053417fb275d43dfa637e8a28849fd072ec7 Mon Sep 17 00:00:00 2001 From: Maurice Parker Date: Tue, 28 May 2019 17:42:19 -0500 Subject: [PATCH 08/28] Enforce tag specific drop validation (can't copy to the account level) --- Frameworks/Account/Account.swift | 4 +++ Frameworks/Account/AccountDelegate.swift | 1 + .../Feedbin/FeedbinAccountDelegate.swift | 1 + .../LocalAccount/LocalAccountDelegate.swift | 1 + .../Sidebar/SidebarOutlineDataSource.swift | 29 +++++++++++++++++++ 5 files changed, 36 insertions(+) diff --git a/Frameworks/Account/Account.swift b/Frameworks/Account/Account.swift index 5972c28d0..3d60bda46 100644 --- a/Frameworks/Account/Account.swift +++ b/Frameworks/Account/Account.swift @@ -167,6 +167,10 @@ public final class Account: DisplayNameProvider, UnreadCountProvider, Container, } } + public var usesTags: Bool { + return delegate.usesTags + } + var refreshInProgress = false { didSet { if refreshInProgress != oldValue { diff --git a/Frameworks/Account/AccountDelegate.swift b/Frameworks/Account/AccountDelegate.swift index 33dace1a3..cfb685f64 100644 --- a/Frameworks/Account/AccountDelegate.swift +++ b/Frameworks/Account/AccountDelegate.swift @@ -14,6 +14,7 @@ protocol AccountDelegate { // Local account does not; some synced accounts might. var supportsSubFolders: Bool { get } + var usesTags: Bool { get } var opmlImportInProgress: Bool { get } var server: String? { get } diff --git a/Frameworks/Account/Feedbin/FeedbinAccountDelegate.swift b/Frameworks/Account/Feedbin/FeedbinAccountDelegate.swift index 7520d0642..798abeee3 100644 --- a/Frameworks/Account/Feedbin/FeedbinAccountDelegate.swift +++ b/Frameworks/Account/Feedbin/FeedbinAccountDelegate.swift @@ -31,6 +31,7 @@ final class FeedbinAccountDelegate: AccountDelegate { private var log = OSLog(subsystem: Bundle.main.bundleIdentifier!, category: "Feedbin") let supportsSubFolders = false + let usesTags = true let server: String? = "api.feedbin.com" var opmlImportInProgress = false diff --git a/Frameworks/Account/LocalAccount/LocalAccountDelegate.swift b/Frameworks/Account/LocalAccount/LocalAccountDelegate.swift index 10e5ce446..34f1c86aa 100644 --- a/Frameworks/Account/LocalAccount/LocalAccountDelegate.swift +++ b/Frameworks/Account/LocalAccount/LocalAccountDelegate.swift @@ -19,6 +19,7 @@ public enum LocalAccountDelegateError: String, Error { final class LocalAccountDelegate: AccountDelegate { let supportsSubFolders = false + let usesTags = false let opmlImportInProgress = false let server: String? = nil diff --git a/Mac/MainWindow/Sidebar/SidebarOutlineDataSource.swift b/Mac/MainWindow/Sidebar/SidebarOutlineDataSource.swift index aa85636de..19090392c 100644 --- a/Mac/MainWindow/Sidebar/SidebarOutlineDataSource.swift +++ b/Mac/MainWindow/Sidebar/SidebarOutlineDataSource.swift @@ -176,6 +176,9 @@ private extension SidebarOutlineDataSource { if nodeHasChildRepresentingDraggedFeed(dropTargetNode, draggedFeed) { return SidebarOutlineDataSource.dragOperationNone } + if violatesTagSpecificBehavior(dropTargetNode, draggedFeed) { + return SidebarOutlineDataSource.dragOperationNone + } let dragOperation: NSDragOperation = localFeedsDropOperation(dropTargetNode, Set([draggedFeed])) if parentNode == dropTargetNode && index == NSOutlineViewDropOnItemIndex { return dragOperation @@ -195,6 +198,9 @@ private extension SidebarOutlineDataSource { if nodeHasChildRepresentingAnyDraggedFeed(dropTargetNode, draggedFeeds) { return SidebarOutlineDataSource.dragOperationNone } + if violatesTagSpecificBehavior(dropTargetNode, draggedFeeds) { + return SidebarOutlineDataSource.dragOperationNone + } if parentNode !== dropTargetNode || index != NSOutlineViewDropOnItemIndex { outlineView.setDropItem(dropTargetNode, dropChildIndex: NSOutlineViewDropOnItemIndex) } @@ -483,6 +489,29 @@ private extension SidebarOutlineDataSource { return false } + func violatesTagSpecificBehavior(_ parentNode: Node, _ draggedFeed: PasteboardFeed) -> Bool { + return violatesTagSpecificBehavior(parentNode, Set([draggedFeed])) + } + + func violatesTagSpecificBehavior(_ parentNode: Node, _ draggedFeeds: Set) -> Bool { + guard let parentAccount = nodeAccount(parentNode), parentAccount.usesTags else { + return false + } + + for draggedFeed in draggedFeeds { + if parentAccount.accountID != draggedFeed.accountID { + return false + } + } + + // Can't copy to the account when using tags + if parentNode.representedObject is Account && (NSApplication.shared.currentEvent?.modifierFlags.contains(.option) ?? false) { + return true + } + + return false + } + func indexWhereDraggedFeedWouldAppear(_ parentNode: Node, _ draggedFeed: PasteboardFeed) -> Int { let draggedFeedWrapper = PasteboardFeedObjectWrapper(pasteboardFeed: draggedFeed) let draggedFeedNode = Node(representedObject: draggedFeedWrapper, parent: nil) From ce81b5d78ccdb8d14fc20bbb855181df62ff22f0 Mon Sep 17 00:00:00 2001 From: Brent Simmons Date: Tue, 28 May 2019 21:56:29 -0700 Subject: [PATCH 09/28] Update RSParser. --- submodules/RSParser | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/submodules/RSParser b/submodules/RSParser index 718f27db5..93b481897 160000 --- a/submodules/RSParser +++ b/submodules/RSParser @@ -1 +1 @@ -Subproject commit 718f27db5016298a9cc650764d5d92ce54ce1e1a +Subproject commit 93b481897d84849345daa965bd8e11860c9422e7 From 8fc6e81ddf1ebb2d5f13237801532532b3e5f906 Mon Sep 17 00:00:00 2001 From: Maurice Parker Date: Wed, 29 May 2019 10:24:30 -0500 Subject: [PATCH 10/28] Update the Today timeline and unread count when the day changes. Issue #627 --- Mac/MainWindow/Sidebar/SidebarViewController.swift | 7 +++++++ .../Timeline/TimelineViewController.swift | 13 +++++++++++++ 2 files changed, 20 insertions(+) diff --git a/Mac/MainWindow/Sidebar/SidebarViewController.swift b/Mac/MainWindow/Sidebar/SidebarViewController.swift index cb890af36..c206be8aa 100644 --- a/Mac/MainWindow/Sidebar/SidebarViewController.swift +++ b/Mac/MainWindow/Sidebar/SidebarViewController.swift @@ -60,6 +60,7 @@ protocol SidebarDelegate: class { NotificationCenter.default.addObserver(self, selector: #selector(feedSettingDidChange(_:)), name: .FeedSettingDidChange, object: nil) NotificationCenter.default.addObserver(self, selector: #selector(displayNameDidChange(_:)), name: .DisplayNameDidChange, object: nil) NotificationCenter.default.addObserver(self, selector: #selector(userDidRequestSidebarSelection(_:)), name: .UserDidRequestSidebarSelection, object: nil) + NotificationCenter.default.addObserver(self, selector: #selector(calendarDayChanged(_:)), name: .NSCalendarDayChanged, object: nil) outlineView.reloadData() @@ -165,6 +166,12 @@ protocol SidebarDelegate: class { revealAndSelectRepresentedObject(feed as AnyObject) } + @objc func calendarDayChanged(_ note: Notification) { + DispatchQueue.main.async { + SmartFeedsController.shared.todayFeed.fetchUnreadCounts() + } + } + // MARK: - Actions @IBAction func delete(_ sender: AnyObject?) { diff --git a/Mac/MainWindow/Timeline/TimelineViewController.swift b/Mac/MainWindow/Timeline/TimelineViewController.swift index 1aa349a86..3a2ca3ea9 100644 --- a/Mac/MainWindow/Timeline/TimelineViewController.swift +++ b/Mac/MainWindow/Timeline/TimelineViewController.swift @@ -147,6 +147,7 @@ final class TimelineViewController: NSViewController, UndoableCommandRunner { NotificationCenter.default.addObserver(self, selector: #selector(accountStateDidChange(_:)), name: .AccountStateDidChange, object: nil) NotificationCenter.default.addObserver(self, selector: #selector(accountsDidChange(_:)), name: .AccountsDidChange, object: nil) NotificationCenter.default.addObserver(self, selector: #selector(userDefaultsDidChange(_:)), name: UserDefaults.didChangeNotification, object: nil) + NotificationCenter.default.addObserver(self, selector: #selector(calendarDayChanged(_:)), name: .NSCalendarDayChanged, object: nil) didRegisterForNotifications = true } @@ -511,6 +512,14 @@ final class TimelineViewController: NSViewController, UndoableCommandRunner { self.fontSize = AppDefaults.timelineFontSize self.sortDirection = AppDefaults.timelineSortDirection } + + @objc func calendarDayChanged(_ note: Notification) { + if representedObjectsContainsTodayFeed() { + DispatchQueue.main.async { [weak self] in + self?.fetchArticles() + } + } + } // MARK: - Reloading Data @@ -966,6 +975,10 @@ private extension TimelineViewController { return representedObjects?.contains(where: { $0 is PseudoFeed}) ?? false } + func representedObjectsContainsTodayFeed() -> Bool { + return representedObjects?.contains(where: { $0 === SmartFeedsController.shared.todayFeed }) ?? false + } + func representedObjectsContainsAnyFeed(_ feeds: Set) -> Bool { // Return true if there’s a match or if a folder contains (recursively) one of feeds From b1bd8d2d90cc06a9d788efd2f29e46d4a736ab82 Mon Sep 17 00:00:00 2001 From: Maurice Parker Date: Wed, 29 May 2019 15:43:33 -0500 Subject: [PATCH 11/28] Enable folder dragging between accounts --- .../Sidebar/FolderPasteboardWriter.swift | 82 ----------- Mac/MainWindow/Sidebar/PasteboardFolder.swift | 137 +++++++++++++++++ .../Sidebar/SidebarOutlineDataSource.swift | 138 +++++++++++++----- NetNewsWire.xcodeproj/project.pbxproj | 8 +- 4 files changed, 244 insertions(+), 121 deletions(-) delete mode 100644 Mac/MainWindow/Sidebar/FolderPasteboardWriter.swift create mode 100644 Mac/MainWindow/Sidebar/PasteboardFolder.swift diff --git a/Mac/MainWindow/Sidebar/FolderPasteboardWriter.swift b/Mac/MainWindow/Sidebar/FolderPasteboardWriter.swift deleted file mode 100644 index b87ade020..000000000 --- a/Mac/MainWindow/Sidebar/FolderPasteboardWriter.swift +++ /dev/null @@ -1,82 +0,0 @@ -// -// FolderPasteboardWriter.swift -// NetNewsWire -// -// Created by Brent Simmons on 2/11/18. -// Copyright © 2018 Ranchero Software. All rights reserved. -// - -import AppKit -import Account -import RSCore - -extension Folder: PasteboardWriterOwner { - - public var pasteboardWriter: NSPasteboardWriting { - return FolderPasteboardWriter(folder: self) - } -} - -@objc final class FolderPasteboardWriter: NSObject, NSPasteboardWriting { - - private let folder: Folder - static let folderUTIInternal = "com.ranchero.NetNewsWire-Evergreen.internal.folder" - static let folderUTIInternalType = NSPasteboard.PasteboardType(rawValue: folderUTIInternal) - - init(folder: Folder) { - - self.folder = folder - } - - // MARK: - NSPasteboardWriting - - func writableTypes(for pasteboard: NSPasteboard) -> [NSPasteboard.PasteboardType] { - - return [.string, FolderPasteboardWriter.folderUTIInternalType] - } - - func pasteboardPropertyList(forType type: NSPasteboard.PasteboardType) -> Any? { - - let plist: Any? - - switch type { - case .string: - plist = folder.nameForDisplay - case FolderPasteboardWriter.folderUTIInternalType: - plist = internalDictionary() - default: - plist = nil - } - - return plist - } -} - -private extension FolderPasteboardWriter { - - private struct Key { - - static let name = "name" - - // Internal - static let accountID = "accountID" - static let folderID = "folderID" - } - - func internalDictionary() -> [String: Any] { - - var d = [String: Any]() - - d[Key.folderID] = folder.folderID - if let name = folder.name { - d[Key.name] = name - } - if let accountID = folder.account?.accountID { - d[Key.accountID] = accountID - } - - return d - - } -} - diff --git a/Mac/MainWindow/Sidebar/PasteboardFolder.swift b/Mac/MainWindow/Sidebar/PasteboardFolder.swift new file mode 100644 index 000000000..2a7e14eb8 --- /dev/null +++ b/Mac/MainWindow/Sidebar/PasteboardFolder.swift @@ -0,0 +1,137 @@ +// +// FolderPasteboardWriter.swift +// NetNewsWire +// +// Created by Brent Simmons on 2/11/18. +// Copyright © 2018 Ranchero Software. All rights reserved. +// + +import AppKit +import Account +import RSCore + +typealias PasteboardFolderDictionary = [String: String] + +struct PasteboardFolder: Hashable { + + private struct Key { + static let name = "name" + // Internal + static let folderID = "folderID" + static let accountID = "accountID" + } + + + let name: String + let folderID: String? + let accountID: String? + + init(name: String, folderID: String?, accountID: String?) { + self.name = name + self.folderID = folderID + self.accountID = accountID + } + + // MARK: - Reading + + init?(dictionary: PasteboardFolderDictionary) { + guard let name = dictionary[Key.name] else { + return nil + } + + let folderID = dictionary[Key.folderID] + let accountID = dictionary[Key.accountID] + + self.init(name: name, folderID: folderID, accountID: accountID) + } + + init?(pasteboardItem: NSPasteboardItem) { + var pasteboardType: NSPasteboard.PasteboardType? + if pasteboardItem.types.contains(FolderPasteboardWriter.folderUTIInternalType) { + pasteboardType = FolderPasteboardWriter.folderUTIInternalType + } + + if let foundType = pasteboardType { + if let folderDictionary = pasteboardItem.propertyList(forType: foundType) as? PasteboardFeedDictionary { + self.init(dictionary: folderDictionary) + return + } + } + + return nil + } + + static func pasteboardFolders(with pasteboard: NSPasteboard) -> Set? { + guard let items = pasteboard.pasteboardItems else { + return nil + } + let folders = items.compactMap { PasteboardFolder(pasteboardItem: $0) } + return folders.isEmpty ? nil : Set(folders) + } + + // MARK: - Writing + + func internalDictionary() -> PasteboardFolderDictionary { + var d = PasteboardFeedDictionary() + d[PasteboardFolder.Key.name] = name + if let folderID = folderID { + d[PasteboardFolder.Key.folderID] = folderID + } + if let accountID = accountID { + d[PasteboardFolder.Key.accountID] = accountID + } + return d + } +} + +extension Folder: PasteboardWriterOwner { + + public var pasteboardWriter: NSPasteboardWriting { + return FolderPasteboardWriter(folder: self) + } +} + +@objc final class FolderPasteboardWriter: NSObject, NSPasteboardWriting { + + private let folder: Folder + static let folderUTIInternal = "com.ranchero.NetNewsWire-Evergreen.internal.folder" + static let folderUTIInternalType = NSPasteboard.PasteboardType(rawValue: folderUTIInternal) + + init(folder: Folder) { + + self.folder = folder + } + + // MARK: - NSPasteboardWriting + + func writableTypes(for pasteboard: NSPasteboard) -> [NSPasteboard.PasteboardType] { + + return [.string, FolderPasteboardWriter.folderUTIInternalType] + } + + func pasteboardPropertyList(forType type: NSPasteboard.PasteboardType) -> Any? { + + let plist: Any? + + switch type { + case .string: + plist = folder.nameForDisplay + case FolderPasteboardWriter.folderUTIInternalType: + plist = internalDictionary + default: + plist = nil + } + + return plist + } +} + +private extension FolderPasteboardWriter { + var pasteboardFolder: PasteboardFolder { + return PasteboardFolder(name: folder.name ?? "", folderID: String(folder.folderID), accountID: folder.account?.accountID) + } + + var internalDictionary: PasteboardFeedDictionary { + return pasteboardFolder.internalDictionary() + } +} diff --git a/Mac/MainWindow/Sidebar/SidebarOutlineDataSource.swift b/Mac/MainWindow/Sidebar/SidebarOutlineDataSource.swift index 19090392c..040a7369d 100644 --- a/Mac/MainWindow/Sidebar/SidebarOutlineDataSource.swift +++ b/Mac/MainWindow/Sidebar/SidebarOutlineDataSource.swift @@ -54,46 +54,66 @@ import Account } func outlineView(_ outlineView: NSOutlineView, validateDrop info: NSDraggingInfo, proposedItem item: Any?, proposedChildIndex index: Int) -> NSDragOperation { - guard let draggedFeeds = PasteboardFeed.pasteboardFeeds(with: info.draggingPasteboard), !draggedFeeds.isEmpty else { + let draggedFolders = PasteboardFolder.pasteboardFolders(with: info.draggingPasteboard) + let draggedFeeds = PasteboardFeed.pasteboardFeeds(with: info.draggingPasteboard) + if (draggedFolders == nil && draggedFeeds == nil) || (draggedFolders != nil && draggedFeeds != nil) { return SidebarOutlineDataSource.dragOperationNone } - let parentNode = nodeForItem(item) - let contentsType = draggedFeedContentsType(draggedFeeds) - switch contentsType { - case .singleNonLocal: - let draggedNonLocalFeed = singleNonLocalFeed(from: draggedFeeds)! - return validateSingleNonLocalFeedDrop(outlineView, draggedNonLocalFeed, parentNode, index) - case .singleLocal: - let draggedFeed = draggedFeeds.first! - return validateSingleLocalFeedDrop(outlineView, draggedFeed, parentNode, index) - case .multipleLocal: - return validateLocalFeedsDrop(outlineView, draggedFeeds, parentNode, index) - case .multipleNonLocal, .mixed, .empty: - return SidebarOutlineDataSource.dragOperationNone + if let draggedFolders = draggedFolders { + if draggedFolders.count == 1 { + return validateLocalFolderDrop(outlineView, draggedFolders.first!, parentNode, index) + } else { + return validateLocalFoldersDrop(outlineView, draggedFolders, parentNode, index) + } } + + if let draggedFeeds = draggedFeeds { + let contentsType = draggedFeedContentsType(draggedFeeds) + + switch contentsType { + case .singleNonLocal: + let draggedNonLocalFeed = singleNonLocalFeed(from: draggedFeeds)! + return validateSingleNonLocalFeedDrop(outlineView, draggedNonLocalFeed, parentNode, index) + case .singleLocal: + let draggedFeed = draggedFeeds.first! + return validateSingleLocalFeedDrop(outlineView, draggedFeed, parentNode, index) + case .multipleLocal: + return validateLocalFeedsDrop(outlineView, draggedFeeds, parentNode, index) + case .multipleNonLocal, .mixed, .empty: + return SidebarOutlineDataSource.dragOperationNone + } + } + + return SidebarOutlineDataSource.dragOperationNone } func outlineView(_ outlineView: NSOutlineView, acceptDrop info: NSDraggingInfo, item: Any?, childIndex index: Int) -> Bool { - guard let draggedFeeds = PasteboardFeed.pasteboardFeeds(with: info.draggingPasteboard), !draggedFeeds.isEmpty else { + let draggedFolders = PasteboardFolder.pasteboardFolders(with: info.draggingPasteboard) + let draggedFeeds = PasteboardFeed.pasteboardFeeds(with: info.draggingPasteboard) + if (draggedFolders == nil && draggedFeeds == nil) || (draggedFolders != nil && draggedFeeds != nil) { return false } - let parentNode = nodeForItem(item) - let contentsType = draggedFeedContentsType(draggedFeeds) - switch contentsType { - case .singleNonLocal: - let draggedNonLocalFeed = singleNonLocalFeed(from: draggedFeeds)! - return acceptSingleNonLocalFeedDrop(outlineView, draggedNonLocalFeed, parentNode, index) - case .singleLocal: - return acceptLocalFeedsDrop(outlineView, draggedFeeds, parentNode, index) - case .multipleLocal: - return acceptLocalFeedsDrop(outlineView, draggedFeeds, parentNode, index) - case .multipleNonLocal, .mixed, .empty: - return false + if let draggedFeeds = draggedFeeds { + let contentsType = draggedFeedContentsType(draggedFeeds) + + switch contentsType { + case .singleNonLocal: + let draggedNonLocalFeed = singleNonLocalFeed(from: draggedFeeds)! + return acceptSingleNonLocalFeedDrop(outlineView, draggedNonLocalFeed, parentNode, index) + case .singleLocal: + return acceptLocalFeedsDrop(outlineView, draggedFeeds, parentNode, index) + case .multipleLocal: + return acceptLocalFeedsDrop(outlineView, draggedFeeds, parentNode, index) + case .multipleNonLocal, .mixed, .empty: + return false + } } + + return false } } @@ -109,11 +129,10 @@ private extension SidebarOutlineDataSource { } func nodeRepresentsDraggableItem(_ node: Node) -> Bool { - // Don’t allow PseudoFeed or Folder to be dragged. + // Don’t allow PseudoFeed to be dragged. // This will have to be revisited later. For instance, // user-created smart feeds should be draggable, maybe. - // And we might allow dragging folders between accounts. - return node.representedObject is Feed + return node.representedObject is Folder || node.representedObject is Feed } // MARK: - Drag and Drop @@ -179,15 +198,14 @@ private extension SidebarOutlineDataSource { if violatesTagSpecificBehavior(dropTargetNode, draggedFeed) { return SidebarOutlineDataSource.dragOperationNone } - let dragOperation: NSDragOperation = localFeedsDropOperation(dropTargetNode, Set([draggedFeed])) if parentNode == dropTargetNode && index == NSOutlineViewDropOnItemIndex { - return dragOperation + return localDragOperation() } let updatedIndex = indexWhereDraggedFeedWouldAppear(dropTargetNode, draggedFeed) if parentNode !== dropTargetNode || index != updatedIndex { outlineView.setDropItem(dropTargetNode, dropChildIndex: updatedIndex) } - return dragOperation + return localDragOperation() } func validateLocalFeedsDrop(_ outlineView: NSOutlineView, _ draggedFeeds: Set, _ parentNode: Node, _ index: Int) -> NSDragOperation { @@ -204,10 +222,10 @@ private extension SidebarOutlineDataSource { if parentNode !== dropTargetNode || index != NSOutlineViewDropOnItemIndex { outlineView.setDropItem(dropTargetNode, dropChildIndex: NSOutlineViewDropOnItemIndex) } - return localFeedsDropOperation(dropTargetNode, draggedFeeds) + return localDragOperation() } - func localFeedsDropOperation(_ dropTargetNode: Node, _ draggedFeeds: Set) -> NSDragOperation { + func localDragOperation() -> NSDragOperation { if NSApplication.shared.currentEvent?.modifierFlags.contains(.option) ?? false { return .copy } else { @@ -240,6 +258,32 @@ private extension SidebarOutlineDataSource { return accounts } + func validateLocalFolderDrop(_ outlineView: NSOutlineView, _ draggedFolder: PasteboardFolder, _ parentNode: Node, _ index: Int) -> NSDragOperation { + guard let dropAccount = parentNode.representedObject as? Account, dropAccount.accountID != draggedFolder.accountID else { + return SidebarOutlineDataSource.dragOperationNone + } + let updatedIndex = indexWhereDraggedFolderWouldAppear(parentNode, draggedFolder) + if index != updatedIndex { + outlineView.setDropItem(parentNode, dropChildIndex: updatedIndex) + } + return localDragOperation() + } + + func validateLocalFoldersDrop(_ outlineView: NSOutlineView, _ draggedFolders: Set, _ parentNode: Node, _ index: Int) -> NSDragOperation { + guard let dropAccount = parentNode.representedObject as? Account else { + return SidebarOutlineDataSource.dragOperationNone + } + for draggedFolder in draggedFolders { + if dropAccount.accountID == draggedFolder.accountID { + return SidebarOutlineDataSource.dragOperationNone + } + } + if index != NSOutlineViewDropOnItemIndex { + outlineView.setDropItem(parentNode, dropChildIndex: NSOutlineViewDropOnItemIndex) + } + return localDragOperation() + } + func copyInAccount(node: Node, to parentNode: Node) { guard let feed = node.representedObject as? Feed else { return @@ -522,6 +566,18 @@ private extension SidebarOutlineDataSource { let index = sortedNodes.firstIndex(of: draggedFeedNode)! return index } + + func indexWhereDraggedFolderWouldAppear(_ parentNode: Node, _ draggedFolder: PasteboardFolder) -> Int { + let draggedFolderWrapper = PasteboardFolderObjectWrapper(pasteboardFolder: draggedFolder) + let draggedFolderNode = Node(representedObject: draggedFolderWrapper, parent: nil) + draggedFolderNode.canHaveChildNodes = true + let nodes = parentNode.childNodes + [draggedFolderNode] + + // Revisit if the tree controller can ever be sorted in some other way. + let sortedNodes = nodes.sortedAlphabeticallyWithFoldersAtEnd() + let index = sortedNodes.firstIndex(of: draggedFolderNode)! + return index + } } final class PasteboardFeedObjectWrapper: DisplayNameProvider { @@ -535,3 +591,15 @@ final class PasteboardFeedObjectWrapper: DisplayNameProvider { self.pasteboardFeed = pasteboardFeed } } + +final class PasteboardFolderObjectWrapper: DisplayNameProvider { + + var nameForDisplay: String { + return pasteboardFolder.name + } + let pasteboardFolder: PasteboardFolder + + init(pasteboardFolder: PasteboardFolder) { + self.pasteboardFolder = pasteboardFolder + } +} diff --git a/NetNewsWire.xcodeproj/project.pbxproj b/NetNewsWire.xcodeproj/project.pbxproj index bb3af139d..5ffe67acf 100644 --- a/NetNewsWire.xcodeproj/project.pbxproj +++ b/NetNewsWire.xcodeproj/project.pbxproj @@ -233,7 +233,7 @@ 84A37CB5201ECD610087C5AF /* RenameWindowController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 84A37CB4201ECD610087C5AF /* RenameWindowController.swift */; }; 84A3EE5F223B667F00557320 /* DefaultFeeds.opml in Resources */ = {isa = PBXBuildFile; fileRef = 84A3EE52223B667F00557320 /* DefaultFeeds.opml */; }; 84A3EE61223B667F00557320 /* DefaultFeeds.opml in Resources */ = {isa = PBXBuildFile; fileRef = 84A3EE52223B667F00557320 /* DefaultFeeds.opml */; }; - 84AD1EAA2031617300BC20B7 /* FolderPasteboardWriter.swift in Sources */ = {isa = PBXBuildFile; fileRef = 84AD1EA92031617300BC20B7 /* FolderPasteboardWriter.swift */; }; + 84AD1EAA2031617300BC20B7 /* PasteboardFolder.swift in Sources */ = {isa = PBXBuildFile; fileRef = 84AD1EA92031617300BC20B7 /* PasteboardFolder.swift */; }; 84AD1EBA2031649C00BC20B7 /* SmartFeedPasteboardWriter.swift in Sources */ = {isa = PBXBuildFile; fileRef = 84AD1EB92031649C00BC20B7 /* SmartFeedPasteboardWriter.swift */; }; 84AD1EBC2032AF5C00BC20B7 /* SidebarOutlineDataSource.swift in Sources */ = {isa = PBXBuildFile; fileRef = 84AD1EBB2032AF5C00BC20B7 /* SidebarOutlineDataSource.swift */; }; 84B7178C201E66580091657D /* SidebarViewController+ContextualMenus.swift in Sources */ = {isa = PBXBuildFile; fileRef = 84B7178B201E66580091657D /* SidebarViewController+ContextualMenus.swift */; }; @@ -839,7 +839,7 @@ 84A1500420048DDF0046AD9A /* SendToMarsEditCommand.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SendToMarsEditCommand.swift; sourceTree = ""; }; 84A37CB4201ECD610087C5AF /* RenameWindowController.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = RenameWindowController.swift; sourceTree = ""; }; 84A3EE52223B667F00557320 /* DefaultFeeds.opml */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.xml; path = DefaultFeeds.opml; sourceTree = ""; }; - 84AD1EA92031617300BC20B7 /* FolderPasteboardWriter.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FolderPasteboardWriter.swift; sourceTree = ""; }; + 84AD1EA92031617300BC20B7 /* PasteboardFolder.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PasteboardFolder.swift; sourceTree = ""; }; 84AD1EB92031649C00BC20B7 /* SmartFeedPasteboardWriter.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SmartFeedPasteboardWriter.swift; sourceTree = ""; }; 84AD1EBB2032AF5C00BC20B7 /* SidebarOutlineDataSource.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SidebarOutlineDataSource.swift; sourceTree = ""; }; 84B7178B201E66580091657D /* SidebarViewController+ContextualMenus.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = "SidebarViewController+ContextualMenus.swift"; sourceTree = ""; }; @@ -1369,7 +1369,7 @@ 849A97601ED9EB96007D329B /* SidebarOutlineView.swift */, 849A97631ED9EB96007D329B /* UnreadCountView.swift */, 848D578D21543519005FFAD5 /* PasteboardFeed.swift */, - 84AD1EA92031617300BC20B7 /* FolderPasteboardWriter.swift */, + 84AD1EA92031617300BC20B7 /* PasteboardFolder.swift */, 849A97821ED9EC63007D329B /* SidebarStatusBarView.swift */, 844B5B6A1FEA224000C7C76A /* Keyboard */, 845A29251FC928C7007B49E3 /* Cell */, @@ -2468,7 +2468,7 @@ 51E3EB33229AB02C00645299 /* ErrorHandler.swift in Sources */, 8472058120142E8900AD578B /* FeedInspectorViewController.swift in Sources */, 5144EA382279FC6200D19003 /* AccountsAddLocalWindowController.swift in Sources */, - 84AD1EAA2031617300BC20B7 /* FolderPasteboardWriter.swift in Sources */, + 84AD1EAA2031617300BC20B7 /* PasteboardFolder.swift in Sources */, 5144EA51227B8E4500D19003 /* AccountsFeedbinWindowController.swift in Sources */, 84AD1EBC2032AF5C00BC20B7 /* SidebarOutlineDataSource.swift in Sources */, 845A29241FC9255E007B49E3 /* SidebarCellAppearance.swift in Sources */, From fa6b6a47694f6a33ef8aa61f61e09455a57b47f4 Mon Sep 17 00:00:00 2001 From: Maurice Parker Date: Wed, 29 May 2019 17:08:41 -0500 Subject: [PATCH 12/28] Corrected move BatchUpdate usage to make move animation smoother --- .../Sidebar/SidebarOutlineDataSource.swift | 32 ++++++++++--------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/Mac/MainWindow/Sidebar/SidebarOutlineDataSource.swift b/Mac/MainWindow/Sidebar/SidebarOutlineDataSource.swift index 040a7369d..81f9c45d9 100644 --- a/Mac/MainWindow/Sidebar/SidebarOutlineDataSource.swift +++ b/Mac/MainWindow/Sidebar/SidebarOutlineDataSource.swift @@ -309,10 +309,12 @@ private extension SidebarOutlineDataSource { let source = node.parent?.representedObject as? Container let destination = parentNode.representedObject as? Container + BatchUpdate.shared.start() source?.removeFeed(feed) { result in switch result { case .success: destination?.addFeed(feed) { result in + BatchUpdate.shared.end() switch result { case .success: break @@ -364,10 +366,12 @@ private extension SidebarOutlineDataSource { if let existingFeed = destinationAccount.existingFeed(withURL: feed.url) { + BatchUpdate.shared.start() destinationContainer.addFeed(existingFeed) { result in switch result { case .success: sourceAccount.deleteFeed(feed) { result in + BatchUpdate.shared.end() switch result { case .success: break @@ -382,10 +386,12 @@ private extension SidebarOutlineDataSource { } else { + BatchUpdate.shared.start() destinationAccount.createFeed(url: feed.url, name: feed.editedName, container: destinationContainer) { result in switch result { case .success: sourceAccount.deleteFeed(feed) { result in + BatchUpdate.shared.end() switch result { case .success: break @@ -406,24 +412,20 @@ private extension SidebarOutlineDataSource { return false } - BatchUpdate.shared.perform { - - draggedNodes.forEach { node in - if sameAccount(node, parentNode) { - if NSApplication.shared.currentEvent?.modifierFlags.contains(.option) ?? false { - copyInAccount(node: node, to: parentNode) - } else { - moveInAccount(node: node, to: parentNode) - } + draggedNodes.forEach { node in + if sameAccount(node, parentNode) { + if NSApplication.shared.currentEvent?.modifierFlags.contains(.option) ?? false { + copyInAccount(node: node, to: parentNode) } else { - if NSApplication.shared.currentEvent?.modifierFlags.contains(.option) ?? false { - copyBetweenAccounts(node: node, to: parentNode) - } else { - moveBetweenAccounts(node: node, to: parentNode) - } + moveInAccount(node: node, to: parentNode) + } + } else { + if NSApplication.shared.currentEvent?.modifierFlags.contains(.option) ?? false { + copyBetweenAccounts(node: node, to: parentNode) + } else { + moveBetweenAccounts(node: node, to: parentNode) } } - } let allReferencedNodes = draggedNodes.union(Set([parentNode])) From bead6ae1231971198652ddb41ea09af94ceaeaf3 Mon Sep 17 00:00:00 2001 From: Maurice Parker Date: Wed, 29 May 2019 17:14:50 -0500 Subject: [PATCH 13/28] Remove now unnecessary call to account structureDidChange in drop --- Mac/MainWindow/Sidebar/SidebarOutlineDataSource.swift | 4 ---- 1 file changed, 4 deletions(-) diff --git a/Mac/MainWindow/Sidebar/SidebarOutlineDataSource.swift b/Mac/MainWindow/Sidebar/SidebarOutlineDataSource.swift index 81f9c45d9..94a805071 100644 --- a/Mac/MainWindow/Sidebar/SidebarOutlineDataSource.swift +++ b/Mac/MainWindow/Sidebar/SidebarOutlineDataSource.swift @@ -428,10 +428,6 @@ private extension SidebarOutlineDataSource { } } - let allReferencedNodes = draggedNodes.union(Set([parentNode])) - let accounts = commonAccountsFor(allReferencedNodes) - accounts.forEach { $0.structureDidChange() } - return true } From 5e3fcfd95546a04197bc61b13205e3a70a18f580 Mon Sep 17 00:00:00 2001 From: Maurice Parker Date: Wed, 29 May 2019 17:56:26 -0500 Subject: [PATCH 14/28] Correct how feeds were deleted so that only the feed in the correct container was deleted --- Frameworks/Account/Account.swift | 4 ++-- Frameworks/Account/AccountDelegate.swift | 2 +- .../Account/Feedbin/FeedbinAccountDelegate.swift | 2 +- .../LocalAccount/LocalAccountDelegate.swift | 14 +------------- .../Sidebar/SidebarOutlineDataSource.swift | 5 +++-- Mac/Scriptability/Account+Scriptability.swift | 6 +++++- Mac/Scriptability/Folder+Scriptability.swift | 2 +- Shared/Commands/DeleteCommand.swift | 2 +- 8 files changed, 15 insertions(+), 22 deletions(-) diff --git a/Frameworks/Account/Account.swift b/Frameworks/Account/Account.swift index 3d60bda46..1014d0d8f 100644 --- a/Frameworks/Account/Account.swift +++ b/Frameworks/Account/Account.swift @@ -396,9 +396,9 @@ public final class Account: DisplayNameProvider, UnreadCountProvider, Container, } - public func deleteFeed(_ feed: Feed, completion: @escaping (Result) -> Void) { + public func deleteFeed(_ feed: Feed, from container: Container?, completion: @escaping (Result) -> Void) { feedMetadata[feed.url] = nil - delegate.deleteFeed(for: self, with: feed, completion: completion) + delegate.deleteFeed(for: self, with: feed, from: container, completion: completion) } public func renameFeed(_ feed: Feed, to name: String, completion: @escaping (Result) -> Void) { diff --git a/Frameworks/Account/AccountDelegate.swift b/Frameworks/Account/AccountDelegate.swift index cfb685f64..0d92a4f77 100644 --- a/Frameworks/Account/AccountDelegate.swift +++ b/Frameworks/Account/AccountDelegate.swift @@ -34,7 +34,7 @@ protocol AccountDelegate { func createFeed(for account: Account, url: String, name: String?, container: Container, completion: @escaping (Result) -> Void) func renameFeed(for account: Account, with feed: Feed, to name: String, completion: @escaping (Result) -> Void) - func deleteFeed(for account: Account, with feed: Feed, completion: @escaping (Result) -> Void) + func deleteFeed(for account: Account, with feed: Feed, from container: Container?, completion: @escaping (Result) -> Void) func addFeed(for account: Account, to container: Container, with: Feed, completion: @escaping (Result) -> Void) func removeFeed(for account: Account, from container: Container, with: Feed, completion: @escaping (Result) -> Void) diff --git a/Frameworks/Account/Feedbin/FeedbinAccountDelegate.swift b/Frameworks/Account/Feedbin/FeedbinAccountDelegate.swift index 798abeee3..8866ff7bf 100644 --- a/Frameworks/Account/Feedbin/FeedbinAccountDelegate.swift +++ b/Frameworks/Account/Feedbin/FeedbinAccountDelegate.swift @@ -340,7 +340,7 @@ final class FeedbinAccountDelegate: AccountDelegate { } - func deleteFeed(for account: Account, with feed: Feed, completion: @escaping (Result) -> Void) { + func deleteFeed(for account: Account, with feed: Feed, from container: Container?, completion: @escaping (Result) -> Void) { // This error should never happen guard let subscriptionID = feed.subscriptionID else { diff --git a/Frameworks/Account/LocalAccount/LocalAccountDelegate.swift b/Frameworks/Account/LocalAccount/LocalAccountDelegate.swift index 34f1c86aa..2cbfe6d67 100644 --- a/Frameworks/Account/LocalAccount/LocalAccountDelegate.swift +++ b/Frameworks/Account/LocalAccount/LocalAccountDelegate.swift @@ -122,8 +122,7 @@ final class LocalAccountDelegate: AccountDelegate { completion(.success(())) } - func deleteFeed(for account: Account, from container: Container, feed: Feed, completion: @escaping (Result) -> Void) { - + func deleteFeed(for account: Account, with feed: Feed, from container: Container?, completion: @escaping (Result) -> Void) { if let account = container as? Account { account.removeFeed(feed) } @@ -131,17 +130,6 @@ final class LocalAccountDelegate: AccountDelegate { folder.removeFeed(feed) } completion(.success(())) - - } - - func deleteFeed(for account: Account, with feed: Feed, completion: @escaping (Result) -> Void) { - account.removeFeed(feed) - if let folders = account.folders { - for folder in folders { - folder.removeFeed(feed) - } - } - completion(.success(())) } func addFeed(for account: Account, to container: Container, with feed: Feed, completion: @escaping (Result) -> Void) { diff --git a/Mac/MainWindow/Sidebar/SidebarOutlineDataSource.swift b/Mac/MainWindow/Sidebar/SidebarOutlineDataSource.swift index 94a805071..214f8bd7d 100644 --- a/Mac/MainWindow/Sidebar/SidebarOutlineDataSource.swift +++ b/Mac/MainWindow/Sidebar/SidebarOutlineDataSource.swift @@ -359,6 +359,7 @@ private extension SidebarOutlineDataSource { func moveBetweenAccounts(node: Node, to parentNode: Node) { guard let feed = node.representedObject as? Feed, let sourceAccount = nodeAccount(node), + let sourceContainer = node.parent?.representedObject as? Container, let destinationAccount = nodeAccount(parentNode), let destinationContainer = parentNode.representedObject as? Container else { return @@ -370,7 +371,7 @@ private extension SidebarOutlineDataSource { destinationContainer.addFeed(existingFeed) { result in switch result { case .success: - sourceAccount.deleteFeed(feed) { result in + sourceAccount.deleteFeed(feed, from: sourceContainer) { result in BatchUpdate.shared.end() switch result { case .success: @@ -390,7 +391,7 @@ private extension SidebarOutlineDataSource { destinationAccount.createFeed(url: feed.url, name: feed.editedName, container: destinationContainer) { result in switch result { case .success: - sourceAccount.deleteFeed(feed) { result in + sourceAccount.deleteFeed(feed, from: sourceContainer) { result in BatchUpdate.shared.end() switch result { case .success: diff --git a/Mac/Scriptability/Account+Scriptability.swift b/Mac/Scriptability/Account+Scriptability.swift index 7c9e9c73c..f0ce08e22 100644 --- a/Mac/Scriptability/Account+Scriptability.swift +++ b/Mac/Scriptability/Account+Scriptability.swift @@ -55,7 +55,11 @@ class ScriptableAccount: NSObject, UniqueIdScriptingObject, ScriptingObjectConta } } else if let scriptableFeed = element as? ScriptableFeed { BatchUpdate.shared.perform { - account.deleteFeed(scriptableFeed.feed) { result in + var container: Container? = nil + if let scriptableFolder = scriptableFeed.container as? ScriptableFolder { + container = scriptableFolder.folder + } + account.deleteFeed(scriptableFeed.feed, from: container) { result in } } } diff --git a/Mac/Scriptability/Folder+Scriptability.swift b/Mac/Scriptability/Folder+Scriptability.swift index bd215f855..07e358407 100644 --- a/Mac/Scriptability/Folder+Scriptability.swift +++ b/Mac/Scriptability/Folder+Scriptability.swift @@ -53,7 +53,7 @@ class ScriptableFolder: NSObject, UniqueIdScriptingObject, ScriptingObjectContai func deleteElement(_ element:ScriptingObject) { if let scriptableFeed = element as? ScriptableFeed { BatchUpdate.shared.perform { - folder.account?.deleteFeed(scriptableFeed.feed) { result in } + folder.account?.deleteFeed(scriptableFeed.feed, from: folder) { result in } } } } diff --git a/Shared/Commands/DeleteCommand.swift b/Shared/Commands/DeleteCommand.swift index 309a8c8f0..cc3b5eac9 100644 --- a/Shared/Commands/DeleteCommand.swift +++ b/Shared/Commands/DeleteCommand.swift @@ -136,7 +136,7 @@ private struct SidebarItemSpecifier { if let feed = feed { BatchUpdate.shared.start() - account?.deleteFeed(feed) { result in + account?.deleteFeed(feed, from: path.resolveContainer()) { result in BatchUpdate.shared.end() self.checkResult(result) } From f4bc17c8f1b96f159462e5b37bba7c2675d09468 Mon Sep 17 00:00:00 2001 From: Maurice Parker Date: Wed, 29 May 2019 20:47:52 -0500 Subject: [PATCH 15/28] Refactor addFeed and removeFeed usages to be more consistent --- Frameworks/Account/Account.swift | 19 ++++----- Frameworks/Account/Container.swift | 5 ++- .../Feedbin/FeedbinAccountDelegate.swift | 2 +- Frameworks/Account/Folder.swift | 12 +----- .../LocalAccount/LocalAccountDelegate.swift | 42 ++++++------------- .../Sidebar/SidebarOutlineDataSource.swift | 21 ++++------ 6 files changed, 35 insertions(+), 66 deletions(-) diff --git a/Frameworks/Account/Account.swift b/Frameworks/Account/Account.swift index 1014d0d8f..d1313beb3 100644 --- a/Frameworks/Account/Account.swift +++ b/Frameworks/Account/Account.swift @@ -61,6 +61,9 @@ public final class Account: DisplayNameProvider, UnreadCountProvider, Container, return defaultName }() + public var account: Account? { + return self + } public let accountID: String public let type: AccountType public var nameForDisplay: String { @@ -373,11 +376,11 @@ public final class Account: DisplayNameProvider, UnreadCountProvider, Container, return feed } - func addFeed(container: Container, feed: Feed, completion: @escaping (Result) -> Void) { + public func addFeed(_ feed: Feed, to container: Container, completion: @escaping (Result) -> Void) { delegate.addFeed(for: self, to: container, with: feed, completion: completion) } - func removeFeed(_ feed: Feed, from container: Container, completion: @escaping (Result) -> Void) { + public func removeFeed(_ feed: Feed, from container: Container, completion: @escaping (Result) -> Void) { delegate.removeFeed(for: self, from: container, with: feed, completion: completion) } @@ -659,21 +662,13 @@ public final class Account: DisplayNameProvider, UnreadCountProvider, Container, return _flattenedFeeds } - public func removeFeed(_ feed: Feed, completion: @escaping (Result) -> Void) { - delegate.removeFeed(for: self, from: self, with: feed, completion: completion) - } - - public func addFeed(_ feed: Feed, completion: @escaping (Result) -> Void) { - delegate.addFeed(for: self, to: self, with: feed, completion: completion) - } - - func removeFeed(_ feed: Feed) { + public func removeFeed(_ feed: Feed) { topLevelFeeds.remove(feed) structureDidChange() postChildrenDidChangeNotification() } - func addFeed(_ feed: Feed) { + public func addFeed(_ feed: Feed) { topLevelFeeds.insert(feed) structureDidChange() postChildrenDidChangeNotification() diff --git a/Frameworks/Account/Container.swift b/Frameworks/Account/Container.swift index 78afa189d..bd3092894 100644 --- a/Frameworks/Account/Container.swift +++ b/Frameworks/Account/Container.swift @@ -18,6 +18,7 @@ extension Notification.Name { public protocol Container: class { + var account: Account? { get } var topLevelFeeds: Set { get set } var folders: Set? { get set } @@ -27,8 +28,8 @@ public protocol Container: class { func hasChildFolder(with: String) -> Bool func childFolder(with: String) -> Folder? - func removeFeed(_ feed: Feed, completion: @escaping (Result) -> Void) - func addFeed(_ feed: Feed, completion: @escaping (Result) -> Void) + func removeFeed(_ feed: Feed) + func addFeed(_ feed: Feed) //Recursive — checks subfolders func flattenedFeeds() -> Set diff --git a/Frameworks/Account/Feedbin/FeedbinAccountDelegate.swift b/Frameworks/Account/Feedbin/FeedbinAccountDelegate.swift index 8866ff7bf..58f5b5ebb 100644 --- a/Frameworks/Account/Feedbin/FeedbinAccountDelegate.swift +++ b/Frameworks/Account/Feedbin/FeedbinAccountDelegate.swift @@ -870,7 +870,7 @@ private extension FeedbinAccountDelegate { let feed = account.createFeed(with: sub.name, url: sub.url, feedID: String(sub.feedID), homePageURL: sub.homePageURL) feed.subscriptionID = String(sub.subscriptionID) - container.addFeed(feed) { result in + account.addFeed(feed, to: container) { result in switch result { case .success: if let name = name { diff --git a/Frameworks/Account/Folder.swift b/Frameworks/Account/Folder.swift index 5bf961fb1..f2ee0551f 100644 --- a/Frameworks/Account/Folder.swift +++ b/Frameworks/Account/Folder.swift @@ -95,20 +95,12 @@ public final class Folder: DisplayNameProvider, Renamable, Container, UnreadCoun return topLevelFeeds.contains(feed) } - public func addFeed(_ feed: Feed, completion: @escaping (Result) -> Void) { - account?.addFeed(container: self, feed: feed, completion: completion) - } - - public func removeFeed(_ feed: Feed, completion: @escaping (Result) -> Void) { - account?.removeFeed(feed, from: self, completion: completion) - } - - func addFeed(_ feed: Feed) { + public func addFeed(_ feed: Feed) { topLevelFeeds.insert(feed) postChildrenDidChangeNotification() } - func removeFeed(_ feed: Feed) { + public func removeFeed(_ feed: Feed) { topLevelFeeds.remove(feed) postChildrenDidChangeNotification() } diff --git a/Frameworks/Account/LocalAccount/LocalAccountDelegate.swift b/Frameworks/Account/LocalAccount/LocalAccountDelegate.swift index 2cbfe6d67..8ce2848aa 100644 --- a/Frameworks/Account/LocalAccount/LocalAccountDelegate.swift +++ b/Frameworks/Account/LocalAccount/LocalAccountDelegate.swift @@ -123,36 +123,26 @@ final class LocalAccountDelegate: AccountDelegate { } func deleteFeed(for account: Account, with feed: Feed, from container: Container?, completion: @escaping (Result) -> Void) { - if let account = container as? Account { - account.removeFeed(feed) - } - if let folder = container as? Folder { - folder.removeFeed(feed) - } + container?.removeFeed(feed) completion(.success(())) } func addFeed(for account: Account, to container: Container, with feed: Feed, completion: @escaping (Result) -> Void) { + container.addFeed(feed) + completion(.success(())) + } + + func removeFeed(for account: Account, from container: Container, with feed: Feed, completion: @escaping (Result) -> Void) { + container.removeFeed(feed) + completion(.success(())) + } + + func restoreFeed(for account: Account, feed: Feed, container: Container, completion: @escaping (Result) -> Void) { if let folder = container as? Folder { folder.addFeed(feed) } else if let account = container as? Account { account.addFeed(feed) } - completion(.success(())) - } - - func removeFeed(for account: Account, from container: Container, with feed: Feed, completion: @escaping (Result) -> Void) { - if let account = container as? Account { - account.removeFeed(feed) - } - if let folder = container as? Folder { - folder.removeFeed(feed) - } - completion(.success(())) - } - - func restoreFeed(for account: Account, feed: Feed, container: Container, completion: @escaping (Result) -> Void) { - container.addFeed(feed, completion: completion) } func restoreFolder(for account: Account, folder: Folder, completion: @escaping (Result) -> Void) { @@ -210,14 +200,8 @@ extension LocalAccountDelegate: FeedFinderDelegate { feed.editedName = self.createFeedName - self.createFeedContainer?.addFeed(feed) { result in - switch result { - case .success: - self.createFeedCompletion?(.success(feed)) - case .failure(let error): - self.createFeedCompletion?(.failure(error)) - } - } + self.createFeedContainer?.addFeed(feed) + self.createFeedCompletion?(.success(feed)) } diff --git a/Mac/MainWindow/Sidebar/SidebarOutlineDataSource.swift b/Mac/MainWindow/Sidebar/SidebarOutlineDataSource.swift index 214f8bd7d..8c3ecea0e 100644 --- a/Mac/MainWindow/Sidebar/SidebarOutlineDataSource.swift +++ b/Mac/MainWindow/Sidebar/SidebarOutlineDataSource.swift @@ -285,13 +285,11 @@ private extension SidebarOutlineDataSource { } func copyInAccount(node: Node, to parentNode: Node) { - guard let feed = node.representedObject as? Feed else { + guard let feed = node.representedObject as? Feed, let destination = parentNode.representedObject as? Container else { return } - let destination = parentNode.representedObject as? Container - - destination?.addFeed(feed) { result in + destination.account?.addFeed(feed, to: destination) { result in switch result { case .success: break @@ -302,18 +300,17 @@ private extension SidebarOutlineDataSource { } func moveInAccount(node: Node, to parentNode: Node) { - guard let feed = node.representedObject as? Feed else { + guard let feed = node.representedObject as? Feed, + let source = node.parent?.representedObject as? Container, + let destination = parentNode.representedObject as? Container else { return } - let source = node.parent?.representedObject as? Container - let destination = parentNode.representedObject as? Container - BatchUpdate.shared.start() - source?.removeFeed(feed) { result in + source.account?.removeFeed(feed, from: source) { result in switch result { case .success: - destination?.addFeed(feed) { result in + destination.account?.addFeed(feed, to: destination) { result in BatchUpdate.shared.end() switch result { case .success: @@ -336,7 +333,7 @@ private extension SidebarOutlineDataSource { } if let existingFeed = destinationAccount.existingFeed(withURL: feed.url) { - destinationContainer.addFeed(existingFeed) { result in + destinationAccount.addFeed(existingFeed, to: destinationContainer) { result in switch result { case .success: break @@ -368,7 +365,7 @@ private extension SidebarOutlineDataSource { if let existingFeed = destinationAccount.existingFeed(withURL: feed.url) { BatchUpdate.shared.start() - destinationContainer.addFeed(existingFeed) { result in + destinationAccount.addFeed(existingFeed, to: destinationContainer) { result in switch result { case .success: sourceAccount.deleteFeed(feed, from: sourceContainer) { result in From 527e6779346c998520543f731e5f4a3bb9c9ebff Mon Sep 17 00:00:00 2001 From: Maurice Parker Date: Wed, 29 May 2019 20:53:00 -0500 Subject: [PATCH 16/28] Rename deleteFolder to removeFolder to make the API more consistent --- Frameworks/Account/Account.swift | 6 +++--- Frameworks/Account/AccountDelegate.swift | 2 +- Frameworks/Account/Feedbin/FeedbinAccountDelegate.swift | 8 ++++---- .../Account/LocalAccount/LocalAccountDelegate.swift | 4 ++-- Mac/Scriptability/Account+Scriptability.swift | 2 +- Shared/Commands/DeleteCommand.swift | 2 +- 6 files changed, 12 insertions(+), 12 deletions(-) diff --git a/Frameworks/Account/Account.swift b/Frameworks/Account/Account.swift index d1313beb3..bcd4a61ab 100644 --- a/Frameworks/Account/Account.swift +++ b/Frameworks/Account/Account.swift @@ -412,8 +412,8 @@ public final class Account: DisplayNameProvider, UnreadCountProvider, Container, delegate.restoreFeed(for: self, feed: feed, container: container, completion: completion) } - public func deleteFolder(_ folder: Folder, completion: @escaping (Result) -> Void) { - delegate.deleteFolder(for: self, with: folder, completion: completion) + public func removeFolder(_ folder: Folder, completion: @escaping (Result) -> Void) { + delegate.removeFolder(for: self, with: folder, completion: completion) } public func renameFolder(_ folder: Folder, to name: String, completion: @escaping (Result) -> Void) { @@ -680,7 +680,7 @@ public final class Account: DisplayNameProvider, UnreadCountProvider, Container, } } - func deleteFolder(_ folder: Folder) { + func removeFolder(_ folder: Folder) { folders?.remove(folder) structureDidChange() postChildrenDidChangeNotification() diff --git a/Frameworks/Account/AccountDelegate.swift b/Frameworks/Account/AccountDelegate.swift index 0d92a4f77..01eb15e7f 100644 --- a/Frameworks/Account/AccountDelegate.swift +++ b/Frameworks/Account/AccountDelegate.swift @@ -30,7 +30,7 @@ protocol AccountDelegate { func importOPML(for account:Account, opmlFile: URL, completion: @escaping (Result) -> Void) func renameFolder(for account: Account, with folder: Folder, to name: String, completion: @escaping (Result) -> Void) - func deleteFolder(for account: Account, with folder: Folder, completion: @escaping (Result) -> Void) + func removeFolder(for account: Account, with folder: Folder, completion: @escaping (Result) -> Void) func createFeed(for account: Account, url: String, name: String?, container: Container, completion: @escaping (Result) -> Void) func renameFeed(for account: Account, with feed: Feed, to name: String, completion: @escaping (Result) -> Void) diff --git a/Frameworks/Account/Feedbin/FeedbinAccountDelegate.swift b/Frameworks/Account/Feedbin/FeedbinAccountDelegate.swift index 58f5b5ebb..621e3093a 100644 --- a/Frameworks/Account/Feedbin/FeedbinAccountDelegate.swift +++ b/Frameworks/Account/Feedbin/FeedbinAccountDelegate.swift @@ -250,11 +250,11 @@ final class FeedbinAccountDelegate: AccountDelegate { } - func deleteFolder(for account: Account, with folder: Folder, completion: @escaping (Result) -> Void) { + func removeFolder(for account: Account, with folder: Folder, completion: @escaping (Result) -> Void) { // Feedbin uses tags and if at least one feed isn't tagged, then the folder doesn't exist on their system guard folder.hasAtLeastOneFeed() else { - account.deleteFolder(folder) + account.removeFolder(folder) return } @@ -270,7 +270,7 @@ final class FeedbinAccountDelegate: AccountDelegate { account.addFeed(feed) self.clearFolderRelationship(for: feed, withFolderName: folder.name ?? "") } - account.deleteFolder(folder) + account.removeFolder(folder) } completion(.success(())) } @@ -570,7 +570,7 @@ private extension FeedbinAccountDelegate { account.addFeed(feed) clearFolderRelationship(for: feed, withFolderName: folder.name ?? "") } - account.deleteFolder(folder) + account.removeFolder(folder) } } } diff --git a/Frameworks/Account/LocalAccount/LocalAccountDelegate.swift b/Frameworks/Account/LocalAccount/LocalAccountDelegate.swift index 8ce2848aa..d1fe5f84e 100644 --- a/Frameworks/Account/LocalAccount/LocalAccountDelegate.swift +++ b/Frameworks/Account/LocalAccount/LocalAccountDelegate.swift @@ -97,8 +97,8 @@ final class LocalAccountDelegate: AccountDelegate { completion(.success(())) } - func deleteFolder(for account: Account, with folder: Folder, completion: @escaping (Result) -> Void) { - account.deleteFolder(folder) + func removeFolder(for account: Account, with folder: Folder, completion: @escaping (Result) -> Void) { + account.removeFolder(folder) completion(.success(())) } diff --git a/Mac/Scriptability/Account+Scriptability.swift b/Mac/Scriptability/Account+Scriptability.swift index f0ce08e22..3647282e3 100644 --- a/Mac/Scriptability/Account+Scriptability.swift +++ b/Mac/Scriptability/Account+Scriptability.swift @@ -50,7 +50,7 @@ class ScriptableAccount: NSObject, UniqueIdScriptingObject, ScriptingObjectConta func deleteElement(_ element:ScriptingObject) { if let scriptableFolder = element as? ScriptableFolder { BatchUpdate.shared.perform { - account.deleteFolder(scriptableFolder.folder) { result in + account.removeFolder(scriptableFolder.folder) { result in } } } else if let scriptableFeed = element as? ScriptableFeed { diff --git a/Shared/Commands/DeleteCommand.swift b/Shared/Commands/DeleteCommand.swift index cc3b5eac9..f35ce5c75 100644 --- a/Shared/Commands/DeleteCommand.swift +++ b/Shared/Commands/DeleteCommand.swift @@ -142,7 +142,7 @@ private struct SidebarItemSpecifier { } } else if let folder = folder { BatchUpdate.shared.start() - account?.deleteFolder(folder) { result in + account?.removeFolder(folder) { result in BatchUpdate.shared.end() self.checkResult(result) } From 51284b5aa4e6f8cc5dc0c7299d6ab8af7fea9bd6 Mon Sep 17 00:00:00 2001 From: Maurice Parker Date: Wed, 29 May 2019 21:04:44 -0500 Subject: [PATCH 17/28] Rename deleteFeed to removeFeed to be more consistent with other API's --- Frameworks/Account/Account.swift | 10 +++------- Frameworks/Account/AccountDelegate.swift | 5 ++--- .../Account/Feedbin/FeedbinAccountDelegate.swift | 6 +++--- .../Account/LocalAccount/LocalAccountDelegate.swift | 9 ++------- Mac/MainWindow/Sidebar/SidebarOutlineDataSource.swift | 4 ++-- Mac/Scriptability/Account+Scriptability.swift | 2 +- Mac/Scriptability/Folder+Scriptability.swift | 2 +- Shared/Commands/DeleteCommand.swift | 2 +- 8 files changed, 15 insertions(+), 25 deletions(-) diff --git a/Frameworks/Account/Account.swift b/Frameworks/Account/Account.swift index bcd4a61ab..6134a34b2 100644 --- a/Frameworks/Account/Account.swift +++ b/Frameworks/Account/Account.swift @@ -377,13 +377,9 @@ public final class Account: DisplayNameProvider, UnreadCountProvider, Container, } public func addFeed(_ feed: Feed, to container: Container, completion: @escaping (Result) -> Void) { - delegate.addFeed(for: self, to: container, with: feed, completion: completion) + delegate.addFeed(for: self, with: feed, to: container, completion: completion) } - public func removeFeed(_ feed: Feed, from container: Container, completion: @escaping (Result) -> Void) { - delegate.removeFeed(for: self, from: container, with: feed, completion: completion) - } - public func createFeed(url: String, name: String?, container: Container, completion: @escaping (Result) -> Void) { delegate.createFeed(for: self, url: url, name: name, container: container, completion: completion) } @@ -399,9 +395,9 @@ public final class Account: DisplayNameProvider, UnreadCountProvider, Container, } - public func deleteFeed(_ feed: Feed, from container: Container?, completion: @escaping (Result) -> Void) { + public func removeFeed(_ feed: Feed, from container: Container?, completion: @escaping (Result) -> Void) { feedMetadata[feed.url] = nil - delegate.deleteFeed(for: self, with: feed, from: container, completion: completion) + delegate.removeFeed(for: self, with: feed, from: container, completion: completion) } public func renameFeed(_ feed: Feed, to name: String, completion: @escaping (Result) -> Void) { diff --git a/Frameworks/Account/AccountDelegate.swift b/Frameworks/Account/AccountDelegate.swift index 01eb15e7f..3bf3ff1c4 100644 --- a/Frameworks/Account/AccountDelegate.swift +++ b/Frameworks/Account/AccountDelegate.swift @@ -34,10 +34,9 @@ protocol AccountDelegate { func createFeed(for account: Account, url: String, name: String?, container: Container, completion: @escaping (Result) -> Void) func renameFeed(for account: Account, with feed: Feed, to name: String, completion: @escaping (Result) -> Void) - func deleteFeed(for account: Account, with feed: Feed, from container: Container?, completion: @escaping (Result) -> Void) + func addFeed(for account: Account, with: Feed, to container: Container, completion: @escaping (Result) -> Void) + func removeFeed(for account: Account, with feed: Feed, from container: Container?, completion: @escaping (Result) -> Void) - func addFeed(for account: Account, to container: Container, with: Feed, completion: @escaping (Result) -> Void) - func removeFeed(for account: Account, from container: Container, with: Feed, completion: @escaping (Result) -> Void) func restoreFeed(for account: Account, feed: Feed, container: Container, completion: @escaping (Result) -> Void) func restoreFolder(for account: Account, folder: Folder, completion: @escaping (Result) -> Void) diff --git a/Frameworks/Account/Feedbin/FeedbinAccountDelegate.swift b/Frameworks/Account/Feedbin/FeedbinAccountDelegate.swift index 621e3093a..47d5cccb9 100644 --- a/Frameworks/Account/Feedbin/FeedbinAccountDelegate.swift +++ b/Frameworks/Account/Feedbin/FeedbinAccountDelegate.swift @@ -340,7 +340,7 @@ final class FeedbinAccountDelegate: AccountDelegate { } - func deleteFeed(for account: Account, with feed: Feed, from container: Container?, completion: @escaping (Result) -> Void) { + func removeFeed(for account: Account, with feed: Feed, from container: Container?, completion: @escaping (Result) -> Void) { // This error should never happen guard let subscriptionID = feed.subscriptionID else { @@ -370,7 +370,7 @@ final class FeedbinAccountDelegate: AccountDelegate { } - func addFeed(for account: Account, to container: Container, with feed: Feed, completion: @escaping (Result) -> Void) { + func addFeed(for account: Account, with feed: Feed, to container: Container, completion: @escaping (Result) -> Void) { if let folder = container as? Folder, let feedID = Int(feed.feedID) { caller.createTagging(feedID: feedID, name: folder.name ?? "") { result in @@ -448,7 +448,7 @@ final class FeedbinAccountDelegate: AccountDelegate { for feed in folder.topLevelFeeds { group.enter() - addFeed(for: account, to: folder, with: feed) { result in + addFeed(for: account, with: feed, to: folder) { result in if account.topLevelFeeds.contains(feed) { account.removeFeed(feed) } diff --git a/Frameworks/Account/LocalAccount/LocalAccountDelegate.swift b/Frameworks/Account/LocalAccount/LocalAccountDelegate.swift index d1fe5f84e..313c058f9 100644 --- a/Frameworks/Account/LocalAccount/LocalAccountDelegate.swift +++ b/Frameworks/Account/LocalAccount/LocalAccountDelegate.swift @@ -122,21 +122,16 @@ final class LocalAccountDelegate: AccountDelegate { completion(.success(())) } - func deleteFeed(for account: Account, with feed: Feed, from container: Container?, completion: @escaping (Result) -> Void) { + func removeFeed(for account: Account, with feed: Feed, from container: Container?, completion: @escaping (Result) -> Void) { container?.removeFeed(feed) completion(.success(())) } - func addFeed(for account: Account, to container: Container, with feed: Feed, completion: @escaping (Result) -> Void) { + func addFeed(for account: Account, with feed: Feed, to container: Container, completion: @escaping (Result) -> Void) { container.addFeed(feed) completion(.success(())) } - func removeFeed(for account: Account, from container: Container, with feed: Feed, completion: @escaping (Result) -> Void) { - container.removeFeed(feed) - completion(.success(())) - } - func restoreFeed(for account: Account, feed: Feed, container: Container, completion: @escaping (Result) -> Void) { if let folder = container as? Folder { folder.addFeed(feed) diff --git a/Mac/MainWindow/Sidebar/SidebarOutlineDataSource.swift b/Mac/MainWindow/Sidebar/SidebarOutlineDataSource.swift index 8c3ecea0e..dafd0d01b 100644 --- a/Mac/MainWindow/Sidebar/SidebarOutlineDataSource.swift +++ b/Mac/MainWindow/Sidebar/SidebarOutlineDataSource.swift @@ -368,7 +368,7 @@ private extension SidebarOutlineDataSource { destinationAccount.addFeed(existingFeed, to: destinationContainer) { result in switch result { case .success: - sourceAccount.deleteFeed(feed, from: sourceContainer) { result in + sourceAccount.removeFeed(feed, from: sourceContainer) { result in BatchUpdate.shared.end() switch result { case .success: @@ -388,7 +388,7 @@ private extension SidebarOutlineDataSource { destinationAccount.createFeed(url: feed.url, name: feed.editedName, container: destinationContainer) { result in switch result { case .success: - sourceAccount.deleteFeed(feed, from: sourceContainer) { result in + sourceAccount.removeFeed(feed, from: sourceContainer) { result in BatchUpdate.shared.end() switch result { case .success: diff --git a/Mac/Scriptability/Account+Scriptability.swift b/Mac/Scriptability/Account+Scriptability.swift index 3647282e3..80637f6be 100644 --- a/Mac/Scriptability/Account+Scriptability.swift +++ b/Mac/Scriptability/Account+Scriptability.swift @@ -59,7 +59,7 @@ class ScriptableAccount: NSObject, UniqueIdScriptingObject, ScriptingObjectConta if let scriptableFolder = scriptableFeed.container as? ScriptableFolder { container = scriptableFolder.folder } - account.deleteFeed(scriptableFeed.feed, from: container) { result in + account.removeFeed(scriptableFeed.feed, from: container) { result in } } } diff --git a/Mac/Scriptability/Folder+Scriptability.swift b/Mac/Scriptability/Folder+Scriptability.swift index 07e358407..a5ea88618 100644 --- a/Mac/Scriptability/Folder+Scriptability.swift +++ b/Mac/Scriptability/Folder+Scriptability.swift @@ -53,7 +53,7 @@ class ScriptableFolder: NSObject, UniqueIdScriptingObject, ScriptingObjectContai func deleteElement(_ element:ScriptingObject) { if let scriptableFeed = element as? ScriptableFeed { BatchUpdate.shared.perform { - folder.account?.deleteFeed(scriptableFeed.feed, from: folder) { result in } + folder.account?.removeFeed(scriptableFeed.feed, from: folder) { result in } } } } diff --git a/Shared/Commands/DeleteCommand.swift b/Shared/Commands/DeleteCommand.swift index f35ce5c75..6a8a15a5d 100644 --- a/Shared/Commands/DeleteCommand.swift +++ b/Shared/Commands/DeleteCommand.swift @@ -136,7 +136,7 @@ private struct SidebarItemSpecifier { if let feed = feed { BatchUpdate.shared.start() - account?.deleteFeed(feed, from: path.resolveContainer()) { result in + account?.removeFeed(feed, from: path.resolveContainer()) { result in BatchUpdate.shared.end() self.checkResult(result) } From 7bec55c90b92616a1137816ed6217ebb55f754a9 Mon Sep 17 00:00:00 2001 From: Maurice Parker Date: Wed, 29 May 2019 21:39:53 -0500 Subject: [PATCH 18/28] Add missing completion call that was causing restored feeds to not show --- Frameworks/Account/LocalAccount/LocalAccountDelegate.swift | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/Frameworks/Account/LocalAccount/LocalAccountDelegate.swift b/Frameworks/Account/LocalAccount/LocalAccountDelegate.swift index 313c058f9..d4169bfb1 100644 --- a/Frameworks/Account/LocalAccount/LocalAccountDelegate.swift +++ b/Frameworks/Account/LocalAccount/LocalAccountDelegate.swift @@ -133,11 +133,8 @@ final class LocalAccountDelegate: AccountDelegate { } func restoreFeed(for account: Account, feed: Feed, container: Container, completion: @escaping (Result) -> Void) { - if let folder = container as? Folder { - folder.addFeed(feed) - } else if let account = container as? Account { - account.addFeed(feed) - } + container.addFeed(feed) + completion(.success(())) } func restoreFolder(for account: Account, folder: Folder, completion: @escaping (Result) -> Void) { From 1352dda8aa399a550656fbb266988fef4626eec7 Mon Sep 17 00:00:00 2001 From: Maurice Parker Date: Thu, 30 May 2019 10:12:34 -0500 Subject: [PATCH 19/28] Modify Feedbin feed deletes so that they emulate how the local account feed deletes work. --- Frameworks/Account/Account.swift | 5 +- Frameworks/Account/AccountDelegate.swift | 2 +- .../Feedbin/FeedbinAccountDelegate.swift | 132 +++++++++++------- .../LocalAccount/LocalAccountDelegate.swift | 6 + .../Sidebar/SidebarOutlineDataSource.swift | 12 +- 5 files changed, 91 insertions(+), 66 deletions(-) diff --git a/Frameworks/Account/Account.swift b/Frameworks/Account/Account.swift index 6134a34b2..71e6d67bd 100644 --- a/Frameworks/Account/Account.swift +++ b/Frameworks/Account/Account.swift @@ -396,10 +396,13 @@ public final class Account: DisplayNameProvider, UnreadCountProvider, Container, } public func removeFeed(_ feed: Feed, from container: Container?, completion: @escaping (Result) -> Void) { - feedMetadata[feed.url] = nil delegate.removeFeed(for: self, with: feed, from: container, completion: completion) } + public func moveFeed(_ feed: Feed, from: Container, to: Container, completion: @escaping (Result) -> Void) { + delegate.moveFeed(for: self, with: feed, from: from, to: to, completion: completion) + } + public func renameFeed(_ feed: Feed, to name: String, completion: @escaping (Result) -> Void) { delegate.renameFeed(for: self, with: feed, to: name, completion: completion) } diff --git a/Frameworks/Account/AccountDelegate.swift b/Frameworks/Account/AccountDelegate.swift index 3bf3ff1c4..34b41db8f 100644 --- a/Frameworks/Account/AccountDelegate.swift +++ b/Frameworks/Account/AccountDelegate.swift @@ -36,7 +36,7 @@ protocol AccountDelegate { func renameFeed(for account: Account, with feed: Feed, to name: String, completion: @escaping (Result) -> Void) func addFeed(for account: Account, with: Feed, to container: Container, completion: @escaping (Result) -> Void) func removeFeed(for account: Account, with feed: Feed, from container: Container?, completion: @escaping (Result) -> Void) - + func moveFeed(for account: Account, with feed: Feed, from: Container, to: Container, completion: @escaping (Result) -> Void) func restoreFeed(for account: Account, feed: Feed, container: Container, completion: @escaping (Result) -> Void) func restoreFolder(for account: Account, folder: Folder, completion: @escaping (Result) -> Void) diff --git a/Frameworks/Account/Feedbin/FeedbinAccountDelegate.swift b/Frameworks/Account/Feedbin/FeedbinAccountDelegate.swift index 47d5cccb9..1ec25ca63 100644 --- a/Frameworks/Account/Feedbin/FeedbinAccountDelegate.swift +++ b/Frameworks/Account/Feedbin/FeedbinAccountDelegate.swift @@ -341,35 +341,28 @@ final class FeedbinAccountDelegate: AccountDelegate { } func removeFeed(for account: Account, with feed: Feed, from container: Container?, completion: @escaping (Result) -> Void) { - - // This error should never happen - guard let subscriptionID = feed.subscriptionID else { - completion(.failure(FeedbinAccountDelegateError.invalidParameter)) - return + if feed.folderRelationship?.count ?? 0 > 1 { + deleteTagging(for: account, with: feed, from: container, completion: completion) + } else { + deleteSubscription(for: account, with: feed, from: container, completion: completion) } - - caller.deleteSubscription(subscriptionID: subscriptionID) { result in - switch result { - case .success: - DispatchQueue.main.async { - account.removeFeed(feed) - if let folders = account.folders { - for folder in folders { - folder.removeFeed(feed) - } - } - completion(.success(())) - } - case .failure(let error): - DispatchQueue.main.async { - let wrappedError = AccountError.wrappedError(error: error, account: account) - completion(.failure(wrappedError)) + } + + func moveFeed(for account: Account, with feed: Feed, from: Container, to: Container, completion: @escaping (Result) -> Void) { + if from is Account { + addFeed(for: account, with: feed, to: to, completion: completion) + } else { + deleteTagging(for: account, with: feed, from: from) { result in + switch result { + case .success: + self.addFeed(for: account, with: feed, to: to, completion: completion) + case .failure(let error): + completion(.failure(error)) } } } - } - + func addFeed(for account: Account, with feed: Feed, to container: Container, completion: @escaping (Result) -> Void) { if let folder = container as? Folder, let feedID = Int(feed.feedID) { @@ -390,43 +383,16 @@ final class FeedbinAccountDelegate: AccountDelegate { } } } else { - if let account = container as? Account { - account.addFeedIfNotInAnyFolder(feed) - } DispatchQueue.main.async { + if let account = container as? Account { + account.addFeedIfNotInAnyFolder(feed) + } completion(.success(())) } } } - func removeFeed(for account: Account, from container: Container, with feed: Feed, completion: @escaping (Result) -> Void) { - - if let folder = container as? Folder, let feedTaggingID = feed.folderRelationship?[folder.name ?? ""] { - caller.deleteTagging(taggingID: feedTaggingID) { result in - switch result { - case .success: - DispatchQueue.main.async { - folder.removeFeed(feed) - account.addFeedIfNotInAnyFolder(feed) - completion(.success(())) - } - case .failure(let error): - DispatchQueue.main.async { - let wrappedError = AccountError.wrappedError(error: error, account: account) - completion(.failure(wrappedError)) - } - } - } - } else { - if let account = container as? Account { - account.removeFeed(feed) - } - completion(.success(())) - } - - } - func restoreFeed(for account: Account, feed: Feed, container: Container, completion: @escaping (Result) -> Void) { createFeed(for: account, url: feed.url, name: feed.editedName, container: container) { result in @@ -1148,5 +1114,63 @@ private extension FeedbinAccountDelegate { } } + + func deleteTagging(for account: Account, with feed: Feed, from container: Container?, completion: @escaping (Result) -> Void) { + + if let folder = container as? Folder, let feedTaggingID = feed.folderRelationship?[folder.name ?? ""] { + caller.deleteTagging(taggingID: feedTaggingID) { result in + switch result { + case .success: + DispatchQueue.main.async { + feed.folderRelationship?.removeValue(forKey: folder.name ?? "") + folder.removeFeed(feed) + account.addFeedIfNotInAnyFolder(feed) + completion(.success(())) + } + case .failure(let error): + DispatchQueue.main.async { + let wrappedError = AccountError.wrappedError(error: error, account: account) + completion(.failure(wrappedError)) + } + } + } + } else { + if let account = container as? Account { + account.removeFeed(feed) + } + completion(.success(())) + } + + } + + func deleteSubscription(for account: Account, with feed: Feed, from container: Container?, completion: @escaping (Result) -> Void) { + + // This error should never happen + guard let subscriptionID = feed.subscriptionID else { + completion(.failure(FeedbinAccountDelegateError.invalidParameter)) + return + } + + caller.deleteSubscription(subscriptionID: subscriptionID) { result in + switch result { + case .success: + DispatchQueue.main.async { + account.removeFeed(feed) + if let folders = account.folders { + for folder in folders { + folder.removeFeed(feed) + } + } + completion(.success(())) + } + case .failure(let error): + DispatchQueue.main.async { + let wrappedError = AccountError.wrappedError(error: error, account: account) + completion(.failure(wrappedError)) + } + } + } + + } } diff --git a/Frameworks/Account/LocalAccount/LocalAccountDelegate.swift b/Frameworks/Account/LocalAccount/LocalAccountDelegate.swift index d4169bfb1..759c411b0 100644 --- a/Frameworks/Account/LocalAccount/LocalAccountDelegate.swift +++ b/Frameworks/Account/LocalAccount/LocalAccountDelegate.swift @@ -127,6 +127,12 @@ final class LocalAccountDelegate: AccountDelegate { completion(.success(())) } + func moveFeed(for account: Account, with feed: Feed, from: Container, to: Container, completion: @escaping (Result) -> Void) { + from.removeFeed(feed) + to.addFeed(feed) + completion(.success(())) + } + func addFeed(for account: Account, with feed: Feed, to container: Container, completion: @escaping (Result) -> Void) { container.addFeed(feed) completion(.success(())) diff --git a/Mac/MainWindow/Sidebar/SidebarOutlineDataSource.swift b/Mac/MainWindow/Sidebar/SidebarOutlineDataSource.swift index dafd0d01b..00eb7711a 100644 --- a/Mac/MainWindow/Sidebar/SidebarOutlineDataSource.swift +++ b/Mac/MainWindow/Sidebar/SidebarOutlineDataSource.swift @@ -307,18 +307,10 @@ private extension SidebarOutlineDataSource { } BatchUpdate.shared.start() - source.account?.removeFeed(feed, from: source) { result in + source.account?.moveFeed(feed, from: source, to: destination) { result in switch result { case .success: - destination.account?.addFeed(feed, to: destination) { result in - BatchUpdate.shared.end() - switch result { - case .success: - break - case .failure(let error): - NSApplication.shared.presentError(error) - } - } + BatchUpdate.shared.end() case .failure(let error): NSApplication.shared.presentError(error) } From 30c21bb1259a58e4a4ab034f46c3c79d658c2995 Mon Sep 17 00:00:00 2001 From: Maurice Parker Date: Thu, 30 May 2019 14:36:21 -0500 Subject: [PATCH 20/28] Enable folders to be dropped in a move or copy between accounts --- Frameworks/Account/Account.swift | 4 + Frameworks/Account/AccountDelegate.swift | 1 + .../Account/Feedbin/FeedbinAPICaller.swift | 19 ---- .../Feedbin/FeedbinAccountDelegate.swift | 50 +++++----- .../LocalAccount/LocalAccountDelegate.swift | 28 ++++-- .../Sidebar/SidebarOutlineDataSource.swift | 94 +++++++++++++++++-- 6 files changed, 135 insertions(+), 61 deletions(-) diff --git a/Frameworks/Account/Account.swift b/Frameworks/Account/Account.swift index 71e6d67bd..5bc4cdfd0 100644 --- a/Frameworks/Account/Account.swift +++ b/Frameworks/Account/Account.swift @@ -411,6 +411,10 @@ public final class Account: DisplayNameProvider, UnreadCountProvider, Container, delegate.restoreFeed(for: self, feed: feed, container: container, completion: completion) } + public func addFolder(_ name: String, completion: @escaping (Result) -> Void) { + delegate.addFolder(for: self, name: name, completion: completion) + } + public func removeFolder(_ folder: Folder, completion: @escaping (Result) -> Void) { delegate.removeFolder(for: self, with: folder, completion: completion) } diff --git a/Frameworks/Account/AccountDelegate.swift b/Frameworks/Account/AccountDelegate.swift index 34b41db8f..56005273a 100644 --- a/Frameworks/Account/AccountDelegate.swift +++ b/Frameworks/Account/AccountDelegate.swift @@ -29,6 +29,7 @@ protocol AccountDelegate { func importOPML(for account:Account, opmlFile: URL, completion: @escaping (Result) -> Void) + func addFolder(for account: Account, name: String, completion: @escaping (Result) -> Void) func renameFolder(for account: Account, with folder: Folder, to name: String, completion: @escaping (Result) -> Void) func removeFolder(for account: Account, with folder: Folder, completion: @escaping (Result) -> Void) diff --git a/Frameworks/Account/Feedbin/FeedbinAPICaller.swift b/Frameworks/Account/Feedbin/FeedbinAPICaller.swift index ba1e0fa94..d8a8bc555 100644 --- a/Frameworks/Account/Feedbin/FeedbinAPICaller.swift +++ b/Frameworks/Account/Feedbin/FeedbinAPICaller.swift @@ -143,25 +143,6 @@ final class FeedbinAPICaller: NSObject { transport.send(request: request, method: HTTPMethod.post, payload: payload, completion: completion) } - func deleteTag(name: String, completion: @escaping (Result<[FeedbinTagging]?, Error>) -> Void) { - - let callURL = feedbinBaseURL.appendingPathComponent("tags.json") - let request = URLRequest(url: callURL, credentials: credentials) - let payload = FeedbinDeleteTag(name: name) - - transport.send(request: request, method: HTTPMethod.delete, payload: payload, resultType: [FeedbinTagging].self) { result in - - switch result { - case .success(let (_, taggings)): - completion(.success(taggings)) - case .failure(let error): - completion(.failure(error)) - } - - } - - } - func retrieveSubscriptions(completion: @escaping (Result<[FeedbinSubscription]?, Error>) -> Void) { let callURL = feedbinBaseURL.appendingPathComponent("subscriptions.json") diff --git a/Frameworks/Account/Feedbin/FeedbinAccountDelegate.swift b/Frameworks/Account/Feedbin/FeedbinAccountDelegate.swift index 1ec25ca63..fbd830e9e 100644 --- a/Frameworks/Account/Feedbin/FeedbinAccountDelegate.swift +++ b/Frameworks/Account/Feedbin/FeedbinAccountDelegate.swift @@ -231,6 +231,14 @@ final class FeedbinAccountDelegate: AccountDelegate { } + func addFolder(for account: Account, name: String, completion: @escaping (Result) -> Void) { + if let folder = account.ensureFolder(with: name) { + completion(.success(folder)) + } else { + completion(.failure(FeedbinAccountDelegateError.invalidParameter)) + } + } + func renameFolder(for account: Account, with folder: Folder, to name: String, completion: @escaping (Result) -> Void) { caller.renameTag(oldName: folder.name ?? "", newName: name) { result in @@ -258,31 +266,26 @@ final class FeedbinAccountDelegate: AccountDelegate { return } - // After we successfully delete at Feedbin, we add all the feeds to the account to save them. We then - // delete the folder. We then sync the taggings we received on the delete to remove any feeds from - // the account that might be in another folder. - caller.deleteTag(name: folder.name ?? "") { result in - switch result { - case .success(let taggings): - DispatchQueue.main.sync { - BatchUpdate.shared.perform { - for feed in folder.topLevelFeeds { - account.addFeed(feed) - self.clearFolderRelationship(for: feed, withFolderName: folder.name ?? "") - } - account.removeFolder(folder) - } - completion(.success(())) - } - self.syncTaggings(account, taggings) - case .failure(let error): - DispatchQueue.main.async { - let wrappedError = AccountError.wrappedError(error: error, account: account) - completion(.failure(wrappedError)) + let group = DispatchGroup() + + for feed in folder.topLevelFeeds { + group.enter() + removeFeed(for: account, with: feed, from: folder) { result in + group.leave() + switch result { + case .success: + break + case .failure(let error): + os_log(.error, log: self.log, "Remove feed error: %@.", error.localizedDescription) } } } + group.notify(queue: DispatchQueue.main) { + account.removeFolder(folder) + completion(.success(())) + } + } func createFeed(for account: Account, url: String, name: String?, container: Container, completion: @escaping (Result) -> Void) { @@ -885,8 +888,7 @@ private extension FeedbinAccountDelegate { } } - - + } func refreshArticles(_ account: Account, completion: @escaping (() -> Void)) { @@ -1122,7 +1124,7 @@ private extension FeedbinAccountDelegate { switch result { case .success: DispatchQueue.main.async { - feed.folderRelationship?.removeValue(forKey: folder.name ?? "") + self.clearFolderRelationship(for: feed, withFolderName: folder.name ?? "") folder.removeFeed(feed) account.addFeedIfNotInAnyFolder(feed) completion(.success(())) diff --git a/Frameworks/Account/LocalAccount/LocalAccountDelegate.swift b/Frameworks/Account/LocalAccount/LocalAccountDelegate.swift index 759c411b0..d0d4c5435 100644 --- a/Frameworks/Account/LocalAccount/LocalAccountDelegate.swift +++ b/Frameworks/Account/LocalAccount/LocalAccountDelegate.swift @@ -91,16 +91,6 @@ final class LocalAccountDelegate: AccountDelegate { completion(.success(())) } - - func renameFolder(for account: Account, with folder: Folder, to name: String, completion: @escaping (Result) -> Void) { - folder.name = name - completion(.success(())) - } - - func removeFolder(for account: Account, with folder: Folder, completion: @escaping (Result) -> Void) { - account.removeFolder(folder) - completion(.success(())) - } func createFeed(for account: Account, url urlString: String, name: String?, container: Container, completion: @escaping (Result) -> Void) { @@ -143,6 +133,24 @@ final class LocalAccountDelegate: AccountDelegate { completion(.success(())) } + func addFolder(for account: Account, name: String, completion: @escaping (Result) -> Void) { + if let folder = account.ensureFolder(with: name) { + completion(.success(folder)) + } else { + completion(.failure(FeedbinAccountDelegateError.invalidParameter)) + } + } + + func renameFolder(for account: Account, with folder: Folder, to name: String, completion: @escaping (Result) -> Void) { + folder.name = name + completion(.success(())) + } + + func removeFolder(for account: Account, with folder: Folder, completion: @escaping (Result) -> Void) { + account.removeFolder(folder) + completion(.success(())) + } + func restoreFolder(for account: Account, folder: Folder, completion: @escaping (Result) -> Void) { account.addFolder(folder) completion(.success(())) diff --git a/Mac/MainWindow/Sidebar/SidebarOutlineDataSource.swift b/Mac/MainWindow/Sidebar/SidebarOutlineDataSource.swift index 00eb7711a..1eff773aa 100644 --- a/Mac/MainWindow/Sidebar/SidebarOutlineDataSource.swift +++ b/Mac/MainWindow/Sidebar/SidebarOutlineDataSource.swift @@ -97,6 +97,10 @@ import Account } let parentNode = nodeForItem(item) + if let draggedFolders = draggedFolders { + return acceptLocalFoldersDrop(outlineView, draggedFolders, parentNode, index) + } + if let draggedFeeds = draggedFeeds { let contentsType = draggedFeedContentsType(draggedFeeds) @@ -284,7 +288,7 @@ private extension SidebarOutlineDataSource { return localDragOperation() } - func copyInAccount(node: Node, to parentNode: Node) { + func copyFeedInAccount(node: Node, to parentNode: Node) { guard let feed = node.representedObject as? Feed, let destination = parentNode.representedObject as? Container else { return } @@ -299,7 +303,7 @@ private extension SidebarOutlineDataSource { } } - func moveInAccount(node: Node, to parentNode: Node) { + func moveFeedInAccount(node: Node, to parentNode: Node) { guard let feed = node.representedObject as? Feed, let source = node.parent?.representedObject as? Container, let destination = parentNode.representedObject as? Container else { @@ -317,7 +321,7 @@ private extension SidebarOutlineDataSource { } } - func copyBetweenAccounts(node: Node, to parentNode: Node) { + func copyFeedBetweenAccounts(node: Node, to parentNode: Node) { guard let feed = node.representedObject as? Feed, let destinationAccount = nodeAccount(parentNode), let destinationContainer = parentNode.representedObject as? Container else { @@ -345,7 +349,7 @@ private extension SidebarOutlineDataSource { } } - func moveBetweenAccounts(node: Node, to parentNode: Node) { + func moveFeedBetweenAccounts(node: Node, to parentNode: Node) { guard let feed = node.representedObject as? Feed, let sourceAccount = nodeAccount(node), let sourceContainer = node.parent?.representedObject as? Container, @@ -405,15 +409,15 @@ private extension SidebarOutlineDataSource { draggedNodes.forEach { node in if sameAccount(node, parentNode) { if NSApplication.shared.currentEvent?.modifierFlags.contains(.option) ?? false { - copyInAccount(node: node, to: parentNode) + copyFeedInAccount(node: node, to: parentNode) } else { - moveInAccount(node: node, to: parentNode) + moveFeedInAccount(node: node, to: parentNode) } } else { if NSApplication.shared.currentEvent?.modifierFlags.contains(.option) ?? false { - copyBetweenAccounts(node: node, to: parentNode) + copyFeedBetweenAccounts(node: node, to: parentNode) } else { - moveBetweenAccounts(node: node, to: parentNode) + moveFeedBetweenAccounts(node: node, to: parentNode) } } } @@ -453,6 +457,80 @@ private extension SidebarOutlineDataSource { return ancestorThatCanAcceptNonLocalFeed(parentNode) } + func copyFolderBetweenAccounts(node: Node, to parentNode: Node) { + guard let sourceFolder = node.representedObject as? Folder, + let destinationAccount = nodeAccount(parentNode) else { + return + } + replicateFolder(sourceFolder, destinationAccount: destinationAccount, completion: {}) + } + + func moveFolderBetweenAccounts(node: Node, to parentNode: Node) { + guard let sourceFolder = node.representedObject as? Folder, + let sourceAccount = nodeAccount(node), + let destinationAccount = nodeAccount(parentNode) else { + return + } + + BatchUpdate.shared.start() + replicateFolder(sourceFolder, destinationAccount: destinationAccount) { + sourceAccount.removeFolder(sourceFolder) { result in + BatchUpdate.shared.end() + switch result { + case .success: + break + case .failure(let error): + NSApplication.shared.presentError(error) + } + } + } + } + + func replicateFolder(_ folder: Folder, destinationAccount: Account, completion: @escaping () -> Void) { + destinationAccount.addFolder(folder.name ?? "") { result in + switch result { + case .success(let destinationFolder): + let group = DispatchGroup() + for feed in folder.topLevelFeeds { + group.enter() + destinationAccount.createFeed(url: feed.url, name: feed.editedName, container: destinationFolder) { result in + group.leave() + switch result { + case .success: + break + case .failure(let error): + NSApplication.shared.presentError(error) + } + } + } + group.notify(queue: DispatchQueue.main) { + completion() + } + case .failure(let error): + NSApplication.shared.presentError(error) + } + } + + } + + func acceptLocalFoldersDrop(_ outlineView: NSOutlineView, _ draggedFolders: Set, _ parentNode: Node, _ index: Int) -> Bool { + guard let draggedNodes = draggedNodes else { + return false + } + + draggedNodes.forEach { node in + if !sameAccount(node, parentNode) { + if NSApplication.shared.currentEvent?.modifierFlags.contains(.option) ?? false { + copyFolderBetweenAccounts(node: node, to: parentNode) + } else { + moveFolderBetweenAccounts(node: node, to: parentNode) + } + } + } + + return true + } + func acceptSingleNonLocalFeedDrop(_ outlineView: NSOutlineView, _ draggedFeed: PasteboardFeed, _ parentNode: Node, _ index: Int) -> Bool { guard nodeIsDropTarget(parentNode), index == NSOutlineViewDropOnItemIndex else { return false From a8f090656d8177c5f7aa03ea2794ce6c573c14be Mon Sep 17 00:00:00 2001 From: Maurice Parker Date: Thu, 30 May 2019 14:44:13 -0500 Subject: [PATCH 21/28] Handle when a feed in a folder being copied/moved already is subscribed in target account --- .../Sidebar/SidebarOutlineDataSource.swift | 30 ++++++++++++++----- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/Mac/MainWindow/Sidebar/SidebarOutlineDataSource.swift b/Mac/MainWindow/Sidebar/SidebarOutlineDataSource.swift index 1eff773aa..d81048636 100644 --- a/Mac/MainWindow/Sidebar/SidebarOutlineDataSource.swift +++ b/Mac/MainWindow/Sidebar/SidebarOutlineDataSource.swift @@ -492,14 +492,27 @@ private extension SidebarOutlineDataSource { case .success(let destinationFolder): let group = DispatchGroup() for feed in folder.topLevelFeeds { - group.enter() - destinationAccount.createFeed(url: feed.url, name: feed.editedName, container: destinationFolder) { result in - group.leave() - switch result { - case .success: - break - case .failure(let error): - NSApplication.shared.presentError(error) + if let existingFeed = destinationAccount.existingFeed(withURL: feed.url) { + group.enter() + destinationAccount.addFeed(existingFeed, to: destinationFolder) { result in + group.leave() + switch result { + case .success: + break + case .failure(let error): + NSApplication.shared.presentError(error) + } + } + } else { + group.enter() + destinationAccount.createFeed(url: feed.url, name: feed.editedName, container: destinationFolder) { result in + group.leave() + switch result { + case .success: + break + case .failure(let error): + NSApplication.shared.presentError(error) + } } } } @@ -508,6 +521,7 @@ private extension SidebarOutlineDataSource { } case .failure(let error): NSApplication.shared.presentError(error) + completion() } } From 0ddb47aa321c1088f4acec9db796c615a7e5354a Mon Sep 17 00:00:00 2001 From: Maurice Parker Date: Thu, 30 May 2019 17:35:08 -0500 Subject: [PATCH 22/28] Refactor feed finder to make it threadsafe --- .../Account/FeedFinder/FeedFinder.swift | 183 +++++++----------- .../LocalAccount/LocalAccountDelegate.swift | 93 ++++----- 2 files changed, 106 insertions(+), 170 deletions(-) diff --git a/Frameworks/Account/FeedFinder/FeedFinder.swift b/Frameworks/Account/FeedFinder/FeedFinder.swift index 89ef24602..c6c2f660e 100644 --- a/Frameworks/Account/FeedFinder/FeedFinder.swift +++ b/Frameworks/Account/FeedFinder/FeedFinder.swift @@ -11,38 +11,54 @@ import RSParser import RSWeb import RSCore -protocol FeedFinderDelegate: class { - - func feedFinder(_: FeedFinder, didFindFeeds: Set) -} - class FeedFinder { + + static func find(url: URL, completion: @escaping (Result, Error>) -> Void) { - private weak var delegate: FeedFinderDelegate? - private var feedSpecifiers = [String: FeedSpecifier]() - private var didNotifyDelegate = false - - var initialDownloadError: Error? - var initialDownloadStatusCode = -1 - - init(url: URL, delegate: FeedFinderDelegate) { - - self.delegate = delegate - - DispatchQueue.main.async() { () -> Void in - - self.findFeeds(url) + downloadUsingCache(url) { (data, response, error) in + + if response?.forcedStatusCode == 404 { + completion(.failure(AccountError.createErrorNotFound)) + return + } + + if let error = error { + completion(.failure(error)) + return + } + + guard let data = data, let response = response else { + completion(.failure(AccountError.createErrorNotFound)) + return + } + + if !response.statusIsOK || data.isEmpty { + completion(.failure(AccountError.createErrorNotFound)) + return + } + + if FeedFinder.isFeed(data, url.absoluteString) { + let feedSpecifier = FeedSpecifier(title: nil, urlString: url.absoluteString, source: .UserEntered) + completion(.success(Set([feedSpecifier]))) + return + } + + if !FeedFinder.isHTML(data) { + completion(.failure(AccountError.createErrorNotFound)) + return + } + + FeedFinder.findFeedsInHTMLPage(htmlData: data, urlString: url.absoluteString, completion: completion) + } + } - deinit { - notifyDelegateIfNeeded() - } } private extension FeedFinder { - func addFeedSpecifier(_ feedSpecifier: FeedSpecifier) { + static func addFeedSpecifier(_ feedSpecifier: FeedSpecifier, feedSpecifiers: inout [String: FeedSpecifier]) { // If there’s an existing feed specifier, merge the two so that we have the best data. If one has a title and one doesn’t, use that non-nil title. Use the better source. @@ -55,7 +71,7 @@ private extension FeedFinder { } } - func findFeedsInHTMLPage(htmlData: Data, urlString: String) { + static func findFeedsInHTMLPage(htmlData: Data, urlString: String, completion: @escaping (Result, Error>) -> Void) { // Feeds in the section we automatically assume are feeds. // If there are none from the section, @@ -63,31 +79,35 @@ private extension FeedFinder { // and added once we determine they are feeds. let possibleFeedSpecifiers = possibleFeedsInHTMLPage(htmlData: htmlData, urlString: urlString) + var feedSpecifiers = [String: FeedSpecifier]() var feedSpecifiersToDownload = Set() var didFindFeedInHTMLHead = false for oneFeedSpecifier in possibleFeedSpecifiers { if oneFeedSpecifier.source == .HTMLHead { - addFeedSpecifier(oneFeedSpecifier) + addFeedSpecifier(oneFeedSpecifier, feedSpecifiers: &feedSpecifiers) didFindFeedInHTMLHead = true } else { - if !feedSpecifiersContainsURLString(oneFeedSpecifier.urlString) { + if feedSpecifiers[oneFeedSpecifier.urlString] == nil { feedSpecifiersToDownload.insert(oneFeedSpecifier) } } } - if didFindFeedInHTMLHead || feedSpecifiersToDownload.isEmpty { - stopFinding() - } - else { - downloadFeedSpecifiers(feedSpecifiersToDownload) + if didFindFeedInHTMLHead { + completion(.success(Set(feedSpecifiers.values))) + return + } else if feedSpecifiersToDownload.isEmpty { + completion(.failure(AccountError.createErrorNotFound)) + return + } else { + downloadFeedSpecifiers(feedSpecifiersToDownload, feedSpecifiers: feedSpecifiers, completion: completion) } } - func possibleFeedsInHTMLPage(htmlData: Data, urlString: String) -> Set { + static func possibleFeedsInHTMLPage(htmlData: Data, urlString: String) -> Set { let parserData = ParserData(url: urlString, data: htmlData) var feedSpecifiers = HTMLFeedFinder(parserData: parserData).feedSpecifiers @@ -109,105 +129,42 @@ private extension FeedFinder { return feedSpecifiers } - func feedSpecifiersContainsURLString(_ urlString: String) -> Bool { - - if let _ = feedSpecifiers[urlString] { - return true - } - return false - } - - func isHTML(_ data: Data) -> Bool { - + static func isHTML(_ data: Data) -> Bool { return (data as NSData).rs_dataIsProbablyHTML() } - func findFeeds(_ initialURL: URL) { + static func downloadFeedSpecifiers(_ downloadFeedSpecifiers: Set, feedSpecifiers: [String: FeedSpecifier], completion: @escaping (Result, Error>) -> Void) { - downloadInitialFeed(initialURL) - } + var resultFeedSpecifiers = feedSpecifiers + let group = DispatchGroup() + + for downloadFeedSpecifier in downloadFeedSpecifiers { - func downloadInitialFeed(_ initialURL: URL) { - - downloadUsingCache(initialURL) { (data, response, error) in - - self.initialDownloadStatusCode = response?.forcedStatusCode ?? -1 - - if let error = error { - self.initialDownloadError = error - self.stopFinding() - return - } - guard let data = data, let response = response else { - self.stopFinding() - return - } - - if !response.statusIsOK || data.isEmpty { - self.stopFinding() - return - } - - if self.isFeed(data, initialURL.absoluteString) { - let feedSpecifier = FeedSpecifier(title: nil, urlString: initialURL.absoluteString, source: .UserEntered) - self.addFeedSpecifier(feedSpecifier) - self.stopFinding() - return - } - - if !self.isHTML(data) { - self.stopFinding() - return - } - - self.findFeedsInHTMLPage(htmlData: data, urlString: initialURL.absoluteString) - } - } - - func downloadFeedSpecifiers(_ feedSpecifiers: Set) { - - var pendingDownloads = feedSpecifiers - - for oneFeedSpecifier in feedSpecifiers { - - guard let url = URL(string: oneFeedSpecifier.urlString) else { - pendingDownloads.remove(oneFeedSpecifier) + guard let url = URL(string: downloadFeedSpecifier.urlString) else { continue } - + + group.enter() downloadUsingCache(url) { (data, response, error) in - - pendingDownloads.remove(oneFeedSpecifier) - if let data = data, let response = response, response.statusIsOK, error == nil { - if self.isFeed(data, oneFeedSpecifier.urlString) { - self.addFeedSpecifier(oneFeedSpecifier) + if self.isFeed(data, downloadFeedSpecifier.urlString) { + addFeedSpecifier(downloadFeedSpecifier, feedSpecifiers: &resultFeedSpecifiers) } } - - if pendingDownloads.isEmpty { - self.stopFinding() - } + group.leave() } + } - } - func stopFinding() { - - notifyDelegateIfNeeded() - } - - func notifyDelegateIfNeeded() { - - if !didNotifyDelegate { - delegate?.feedFinder(self, didFindFeeds: Set(feedSpecifiers.values)) - didNotifyDelegate = true + group.notify(queue: DispatchQueue.main) { + completion(.success(Set(resultFeedSpecifiers.values))) } + } - func isFeed(_ data: Data, _ urlString: String) -> Bool { - + static func isFeed(_ data: Data, _ urlString: String) -> Bool { let parserData = ParserData(url: urlString, data: data) return FeedParser.canParse(parserData) } + } diff --git a/Frameworks/Account/LocalAccount/LocalAccountDelegate.swift b/Frameworks/Account/LocalAccount/LocalAccountDelegate.swift index d0d4c5435..aab4c1f56 100644 --- a/Frameworks/Account/LocalAccount/LocalAccountDelegate.swift +++ b/Frameworks/Account/LocalAccount/LocalAccountDelegate.swift @@ -26,12 +26,6 @@ final class LocalAccountDelegate: AccountDelegate { var credentials: Credentials? var accountMetadata: AccountMetadata? - private weak var account: Account? - private var feedFinder: FeedFinder? - private var createFeedName: String? - private var createFeedContainer: Container? - private var createFeedCompletion: ((Result) -> Void)? - private let refresher = LocalAccountRefresher() var refreshProgress: DownloadProgress { @@ -99,11 +93,42 @@ final class LocalAccountDelegate: AccountDelegate { return } - self.account = account - createFeedName = name - createFeedContainer = container - createFeedCompletion = completion - feedFinder = FeedFinder(url: url, delegate: self) + FeedFinder.find(url: url) { result in + + switch result { + case .success(let feedSpecifiers): + + guard let bestFeedSpecifier = FeedSpecifier.bestFeed(in: feedSpecifiers), + let url = URL(string: bestFeedSpecifier.urlString) else { + completion(.failure(AccountError.createErrorNotFound)) + return + } + + if account.hasFeed(withURL: bestFeedSpecifier.urlString) { + completion(.failure(AccountError.createErrorAlreadySubscribed)) + return + } + + let feed = account.createFeed(with: nil, url: url.absoluteString, feedID: url.absoluteString, homePageURL: nil) + + InitialFeedDownloader.download(url) { parsedFeed in + + if let parsedFeed = parsedFeed { + account.update(feed, with: parsedFeed, {}) + } + + feed.editedName = name + + container.addFeed(feed) + completion(.success(feed)) + + } + + case .failure(let error): + completion(.failure(error)) + } + + } } @@ -168,49 +193,3 @@ final class LocalAccountDelegate: AccountDelegate { } } - -extension LocalAccountDelegate: FeedFinderDelegate { - - // MARK: FeedFinderDelegate - - public func feedFinder(_ feedFinder: FeedFinder, didFindFeeds feedSpecifiers: Set) { - - if let error = feedFinder.initialDownloadError { - if feedFinder.initialDownloadStatusCode == 404 { - createFeedCompletion!(.failure(AccountError.createErrorNotFound)) - } else { - createFeedCompletion!(.failure(error)) - } - return - } - - guard let bestFeedSpecifier = FeedSpecifier.bestFeed(in: feedSpecifiers), - let url = URL(string: bestFeedSpecifier.urlString), - let account = account else { - createFeedCompletion!(.failure(AccountError.createErrorNotFound)) - return - } - - if account.hasFeed(withURL: bestFeedSpecifier.urlString) { - createFeedCompletion!(.failure(AccountError.createErrorAlreadySubscribed)) - return - } - - let feed = account.createFeed(with: nil, url: url.absoluteString, feedID: url.absoluteString, homePageURL: nil) - - InitialFeedDownloader.download(url) { parsedFeed in - - if let parsedFeed = parsedFeed { - account.update(feed, with: parsedFeed, {}) - } - - feed.editedName = self.createFeedName - - self.createFeedContainer?.addFeed(feed) - self.createFeedCompletion?(.success(feed)) - - } - - } - -} From e7c339fb093deb328f94e0d45e55e6ee7a3674f2 Mon Sep 17 00:00:00 2001 From: Maurice Parker Date: Thu, 30 May 2019 17:41:56 -0500 Subject: [PATCH 23/28] Update iOS to work with the latest Account API --- iOS/MasterFeed/MasterFeedViewController.swift | 21 +++++++------------ 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/iOS/MasterFeed/MasterFeedViewController.swift b/iOS/MasterFeed/MasterFeedViewController.swift index 73a654501..4f01ed04d 100644 --- a/iOS/MasterFeed/MasterFeedViewController.swift +++ b/iOS/MasterFeed/MasterFeedViewController.swift @@ -373,22 +373,17 @@ class MasterFeedViewController: ProgressTableViewController, UndoableCommandRunn }() // Move the Feed - let source = sourceNode.parent?.representedObject as? Container - let destination = destParentNode?.representedObject as? Container - source?.removeFeed(feed) { [weak self] result in + guard let source = sourceNode.parent?.representedObject as? Container, let destination = destParentNode?.representedObject as? Container else { + return + } + + BatchUpdate.shared.start() + source.account?.moveFeed(feed, from: source, to: destination) { result in switch result { case .success: - destination?.addFeed(feed) { result in - switch result { - case .success: - break - case .failure(let error): - source?.addFeed(feed) { result in } - self?.presentError(error) - } - } + BatchUpdate.shared.end() case .failure(let error): - self?.presentError(error) + self.presentError(error) } } From beacad1aebb257a8a6ce545f4587549828b42d06 Mon Sep 17 00:00:00 2001 From: Maurice Parker Date: Thu, 30 May 2019 17:57:06 -0500 Subject: [PATCH 24/28] Validate folder drop to make sure no folders with the same name are already in the target account --- .../Sidebar/SidebarOutlineDataSource.swift | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/Mac/MainWindow/Sidebar/SidebarOutlineDataSource.swift b/Mac/MainWindow/Sidebar/SidebarOutlineDataSource.swift index d81048636..935b97458 100644 --- a/Mac/MainWindow/Sidebar/SidebarOutlineDataSource.swift +++ b/Mac/MainWindow/Sidebar/SidebarOutlineDataSource.swift @@ -262,10 +262,22 @@ private extension SidebarOutlineDataSource { return accounts } + func accountHasFolderRepresentingAnyDraggedFolders(_ account: Account, _ draggedFolders: Set) -> Bool { + for draggedFolder in draggedFolders { + if account.existingFolder(with: draggedFolder.name) != nil { + return true + } + } + return false + } + func validateLocalFolderDrop(_ outlineView: NSOutlineView, _ draggedFolder: PasteboardFolder, _ parentNode: Node, _ index: Int) -> NSDragOperation { guard let dropAccount = parentNode.representedObject as? Account, dropAccount.accountID != draggedFolder.accountID else { return SidebarOutlineDataSource.dragOperationNone } + if accountHasFolderRepresentingAnyDraggedFolders(dropAccount, Set([draggedFolder])) { + return SidebarOutlineDataSource.dragOperationNone + } let updatedIndex = indexWhereDraggedFolderWouldAppear(parentNode, draggedFolder) if index != updatedIndex { outlineView.setDropItem(parentNode, dropChildIndex: updatedIndex) @@ -277,6 +289,9 @@ private extension SidebarOutlineDataSource { guard let dropAccount = parentNode.representedObject as? Account else { return SidebarOutlineDataSource.dragOperationNone } + if accountHasFolderRepresentingAnyDraggedFolders(dropAccount, draggedFolders) { + return SidebarOutlineDataSource.dragOperationNone + } for draggedFolder in draggedFolders { if dropAccount.accountID == draggedFolder.accountID { return SidebarOutlineDataSource.dragOperationNone From d602f894f63a12b4f080e9fe3863573542a487c3 Mon Sep 17 00:00:00 2001 From: Maurice Parker Date: Fri, 31 May 2019 07:22:28 -0500 Subject: [PATCH 25/28] Fix link parsing bug that causes crash --- Frameworks/Account/Feedbin/FeedbinAPICaller.swift | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Frameworks/Account/Feedbin/FeedbinAPICaller.swift b/Frameworks/Account/Feedbin/FeedbinAPICaller.swift index d8a8bc555..9389df59a 100644 --- a/Frameworks/Account/Feedbin/FeedbinAPICaller.swift +++ b/Frameworks/Account/Feedbin/FeedbinAPICaller.swift @@ -532,10 +532,11 @@ extension FeedbinAPICaller { } if let lowerBound = link.range(of: "page=")?.upperBound { - if let upperBound = link.range(of: "&")?.lowerBound { + let partialLink = link[lowerBound..")?.lowerBound { + if let upperBound = partialLink.range(of: ">")?.lowerBound { return Int(link[lowerBound.. Date: Fri, 31 May 2019 07:47:05 -0500 Subject: [PATCH 26/28] Clear the feed metadata on Feedbin feed delete --- Frameworks/Account/Account.swift | 4 ++++ Frameworks/Account/Feedbin/FeedbinAccountDelegate.swift | 1 + 2 files changed, 5 insertions(+) diff --git a/Frameworks/Account/Account.swift b/Frameworks/Account/Account.swift index 5bc4cdfd0..ae57dc285 100644 --- a/Frameworks/Account/Account.swift +++ b/Frameworks/Account/Account.swift @@ -427,6 +427,10 @@ public final class Account: DisplayNameProvider, UnreadCountProvider, Container, delegate.restoreFolder(for: self, folder: folder, completion: completion) } + func clearFeedMetadata(_ feed: Feed) { + feedMetadata[feed.url] = nil + } + func addFolder(_ folder: Folder) { folders!.insert(folder) postChildrenDidChangeNotification() diff --git a/Frameworks/Account/Feedbin/FeedbinAccountDelegate.swift b/Frameworks/Account/Feedbin/FeedbinAccountDelegate.swift index fbd830e9e..f7788af52 100644 --- a/Frameworks/Account/Feedbin/FeedbinAccountDelegate.swift +++ b/Frameworks/Account/Feedbin/FeedbinAccountDelegate.swift @@ -347,6 +347,7 @@ final class FeedbinAccountDelegate: AccountDelegate { if feed.folderRelationship?.count ?? 0 > 1 { deleteTagging(for: account, with: feed, from: container, completion: completion) } else { + account.clearFeedMetadata(feed) deleteSubscription(for: account, with: feed, from: container, completion: completion) } } From fec3c69f4a9b95540c1c02288b78d47650994fb5 Mon Sep 17 00:00:00 2001 From: Maurice Parker Date: Fri, 31 May 2019 07:54:12 -0500 Subject: [PATCH 27/28] Remove Feedbin api workaround for unpropagated mode=extended parameter --- Frameworks/Account/Feedbin/FeedbinAPICaller.swift | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/Frameworks/Account/Feedbin/FeedbinAPICaller.swift b/Frameworks/Account/Feedbin/FeedbinAPICaller.swift index 9389df59a..ffbb2646c 100644 --- a/Frameworks/Account/Feedbin/FeedbinAPICaller.swift +++ b/Frameworks/Account/Feedbin/FeedbinAPICaller.swift @@ -419,13 +419,12 @@ final class FeedbinAPICaller: NSObject { func retrieveEntries(page: String, completion: @escaping (Result<([FeedbinEntry]?, String?), Error>) -> Void) { - guard let url = URL(string: page), var callComponents = URLComponents(url: url, resolvingAgainstBaseURL: false) else { + guard let url = URL(string: page) else { completion(.success((nil, nil))) return } - callComponents.queryItems?.append(URLQueryItem(name: "mode", value: "extended")) - let request = URLRequest(url: callComponents.url!, credentials: credentials) + let request = URLRequest(url: url, credentials: credentials) transport.send(request: request, resultType: [FeedbinEntry].self) { result in From 88b2775076c16e9e8403a7591b1e08250265a838 Mon Sep 17 00:00:00 2001 From: Maurice Parker Date: Fri, 31 May 2019 08:05:26 -0500 Subject: [PATCH 28/28] Make sure that an account doesn't try persist to the disk in the time that it is logically deleted and when it is actually deallocated --- Frameworks/Account/Account.swift | 8 +++++--- Frameworks/Account/AccountManager.swift | 1 + Mac/MainWindow/AddFeed/AddFeedWindowController.swift | 2 +- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/Frameworks/Account/Account.swift b/Frameworks/Account/Account.swift index ae57dc285..8d5564215 100644 --- a/Frameworks/Account/Account.swift +++ b/Frameworks/Account/Account.swift @@ -61,6 +61,8 @@ public final class Account: DisplayNameProvider, UnreadCountProvider, Container, return defaultName }() + public var isDeleted = false + public var account: Account? { return self } @@ -760,19 +762,19 @@ public final class Account: DisplayNameProvider, UnreadCountProvider, Container, @objc func saveToDiskIfNeeded() { - if dirty { + if dirty && !isDeleted { saveToDisk() } } @objc func saveFeedMetadataIfNeeded() { - if feedMetadataDirty { + if feedMetadataDirty && !isDeleted { saveFeedMetadata() } } @objc func saveAccountMetadataIfNeeded() { - if metadataDirty { + if metadataDirty && !isDeleted { saveAccountMetadata() } } diff --git a/Frameworks/Account/AccountManager.swift b/Frameworks/Account/AccountManager.swift index 7f72d5dd9..f707ff6c1 100644 --- a/Frameworks/Account/AccountManager.swift +++ b/Frameworks/Account/AccountManager.swift @@ -127,6 +127,7 @@ public final class AccountManager: UnreadCountProvider { } accountsDictionary.removeValue(forKey: account.accountID) + account.isDeleted = true do { try FileManager.default.removeItem(atPath: account.dataFolder) diff --git a/Mac/MainWindow/AddFeed/AddFeedWindowController.swift b/Mac/MainWindow/AddFeed/AddFeedWindowController.swift index 8c5946f67..5c05b2af5 100644 --- a/Mac/MainWindow/AddFeed/AddFeedWindowController.swift +++ b/Mac/MainWindow/AddFeed/AddFeedWindowController.swift @@ -29,7 +29,7 @@ class AddFeedWindowController : NSWindowController { private var urlString: String? private var initialName: String? - private var initialAccount: Account? + private weak var initialAccount: Account? private var initialFolder: Folder? private weak var delegate: AddFeedWindowControllerDelegate? private var folderTreeController: TreeController!