From 23b43ecc0758ee7187b03ebac6ebc2c9b7503cb6 Mon Sep 17 00:00:00 2001 From: Maurice Parker Date: Thu, 9 May 2019 16:09:21 -0500 Subject: [PATCH] Add undo for feeds --- Frameworks/Account/Account.swift | 75 +++++++-------- Frameworks/Account/AccountDelegate.swift | 5 +- .../Feedbin/FeedbinAccountDelegate.swift | 96 ++++++++++++++++++- .../LocalAccount/LocalAccountDelegate.swift | 16 +++- .../AddFeed/AddFeedController.swift | 2 +- Mac/Scriptability/Feed+Scriptability.swift | 2 +- Shared/Commands/DeleteCommand.swift | 53 +++++----- 7 files changed, 178 insertions(+), 71 deletions(-) diff --git a/Frameworks/Account/Account.swift b/Frameworks/Account/Account.swift index 2b4edeb62..82cc5cd0c 100644 --- a/Frameworks/Account/Account.swift +++ b/Frameworks/Account/Account.swift @@ -354,30 +354,16 @@ public final class Account: DisplayNameProvider, UnreadCountProvider, Container, return ensureFolder(with: folderName) } - public func canAddFeed(_ feed: Feed, to folder: Folder?) -> Bool { - - // If folder is nil, then it should go at the top level. - // The same feed in multiple folders is allowed. - // But the same feed can’t appear twice in the same folder - // (or at the top level). - - return true // TODO - } - - 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 addFeed(container: Container, feed: Feed, completion: @escaping (Result) -> Void) { delegate.addFeed(for: self, to: container, with: feed, completion: completion) } - public func createFeed(with name: String?, url: String, completion: @escaping (Result) -> Void) { - delegate.createFeed(for: self, with: name, url: url, completion: completion) + 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, completion: @escaping (Result) -> Void) { + delegate.createFeed(for: self, url: url, completion: completion) } func createFeed(with name: String?, url: String, feedID: String, homePageURL: String?) -> Feed { @@ -391,29 +377,38 @@ public final class Account: DisplayNameProvider, UnreadCountProvider, Container, } + public func deleteFeed(_ feed: Feed, completion: @escaping (Result) -> Void) { + feedMetadata[feed.url] = nil + delegate.deleteFeed(for: self, with: feed, completion: completion) + } + public func renameFeed(_ feed: Feed, to name: String, completion: @escaping (Result) -> Void) { delegate.renameFeed(for: self, with: feed, to: name, completion: completion) } - @discardableResult - public func addFolder(_ folder: Folder, to parentFolder: Folder?) -> Bool { - - // TODO: support subfolders, maybe, some day, if one of the sync systems - // supports subfolders. But, for now, parentFolder is ignored. - if folders!.contains(folder) { - return true - } - folders!.insert(folder) - postChildrenDidChangeNotification() - structureDidChange() - return true + public func restoreFeed(_ feed: Feed, folder: Folder?, completion: @escaping (Result) -> Void) { + delegate.restoreFeed(for: self, feed: feed, folder: folder, completion: completion) + } + + public func deleteFolder(_ folder: Folder, completion: @escaping (Result) -> Void) { + delegate.deleteFolder(for: self, with: folder, completion: completion) } public func renameFolder(_ folder: Folder, to name: String, completion: @escaping (Result) -> Void) { delegate.renameFolder(for: self, with: folder, to: name, completion: completion) } - public func importOPML(_ opmlDocument: RSOPMLDocument) { + public func restoreFolder(_ folder: Folder, completion: @escaping (Result) -> Void) { + delegate.restoreFolder(for: self, folder: folder, completion: completion) + } + + func addFolder(_ folder: Folder) { + folders!.insert(folder) + postChildrenDidChangeNotification() + structureDidChange() + } + + public func importOPML(_ opmlDocument: RSOPMLDocument) { guard let children = opmlDocument.children else { return @@ -582,12 +577,12 @@ public final class Account: DisplayNameProvider, UnreadCountProvider, Container, return _flattenedFeeds } - public func deleteFeed(_ feed: Feed, completion: @escaping (Result) -> Void) { - delegate.deleteFeed(for: self, with: feed, completion: completion) + public func removeFeed(_ feed: Feed, completion: @escaping (Result) -> Void) { + delegate.removeFeed(for: self, from: self, with: feed, completion: completion) } - - func removeFeed(_ feed: Feed, from container: Container, completion: @escaping (Result) -> Void) { - delegate.removeFeed(for: self, from: container, 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) { @@ -602,10 +597,6 @@ public final class Account: DisplayNameProvider, UnreadCountProvider, Container, postChildrenDidChangeNotification() } - public func deleteFolder(_ folder: Folder, completion: @escaping (Result) -> Void) { - delegate.deleteFolder(for: self, with: folder, completion: completion) - } - func deleteFolder(_ folder: Folder) { folders?.remove(folder) structureDidChange() diff --git a/Frameworks/Account/AccountDelegate.swift b/Frameworks/Account/AccountDelegate.swift index 2a0cb20ba..fed819eb0 100644 --- a/Frameworks/Account/AccountDelegate.swift +++ b/Frameworks/Account/AccountDelegate.swift @@ -24,13 +24,16 @@ 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, with name: String?, url: String, completion: @escaping (Result) -> Void) + func createFeed(for account: Account, url: String, 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 restoreFolder(for account: Account, folder: Folder, completion: @escaping (Result) -> Void) + // Called at the end of account’s init method. func accountDidInitialize(_ account: Account) diff --git a/Frameworks/Account/Feedbin/FeedbinAccountDelegate.swift b/Frameworks/Account/Feedbin/FeedbinAccountDelegate.swift index c7279c265..67597f952 100644 --- a/Frameworks/Account/Feedbin/FeedbinAccountDelegate.swift +++ b/Frameworks/Account/Feedbin/FeedbinAccountDelegate.swift @@ -18,6 +18,7 @@ import os.log public enum FeedbinAccountDelegateError: String, Error { case invalidParameter = "There was an invalid parameter passed." + case invalidResponse = "An invalid response was received from the service." } final class FeedbinAccountDelegate: AccountDelegate { @@ -114,7 +115,7 @@ final class FeedbinAccountDelegate: AccountDelegate { } - func createFeed(for account: Account, with name: String?, url: String, completion: @escaping (Result) -> Void) { + func createFeed(for account: Account, url: String, completion: @escaping (Result) -> Void) { caller.createSubscription(url: url) { result in switch result { @@ -255,7 +256,100 @@ final class FeedbinAccountDelegate: AccountDelegate { } } + + func restoreFeed(for account: Account, feed: Feed, folder: Folder?, completion: @escaping (Result) -> Void) { + + let editedName = feed.editedName + + createFeed(for: account, url: feed.url) { [weak self] result in + + switch result { + case .success(let createResult): + + switch createResult { + case .created(let feed): + self?.processRestoredFeed(for: account, feed: feed, editedName: editedName, folder: folder, completion: completion) + default: + DispatchQueue.main.async { + completion(.failure(FeedbinAccountDelegateError.invalidResponse)) + } + } + + case .failure(let error): + DispatchQueue.main.async { + completion(.failure(error)) + } + } + + } + + } + + private 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) { [weak self] result in + + switch result { + case .success: + + if editedName != nil { + DispatchQueue.main.async { + folder.addFeed(feed) + } + self?.processRestoredFeedName(for: account, feed: feed, editedName: editedName!, completion: completion) + } else { + DispatchQueue.main.async { + folder.addFeed(feed) + completion(.success(())) + } + } + + case .failure(let error): + DispatchQueue.main.async { + completion(.failure(error)) + } + } + + } + + } else { + + DispatchQueue.main.async { + account.addFeed(feed) + } + + processRestoredFeedName(for: account, feed: feed, editedName: editedName!, completion: completion) + } + + } + + private 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 restoreFolder(for account: Account, folder: Folder, completion: @escaping (Result) -> Void) { + account.addFolder(folder) + completion(.success(())) + } + func accountDidInitialize(_ account: Account) { credentials = try? account.retrieveBasicCredentials() accountMetadata = account.metadata diff --git a/Frameworks/Account/LocalAccount/LocalAccountDelegate.swift b/Frameworks/Account/LocalAccount/LocalAccountDelegate.swift index 3f2938fac..fc55e0d4b 100644 --- a/Frameworks/Account/LocalAccount/LocalAccountDelegate.swift +++ b/Frameworks/Account/LocalAccount/LocalAccountDelegate.swift @@ -46,7 +46,7 @@ final class LocalAccountDelegate: AccountDelegate { completion(.success(())) } - func createFeed(for account: Account, with name: String?, url urlString: String, completion: @escaping (Result) -> Void) { + func createFeed(for account: Account, url urlString: String, completion: @escaping (Result) -> Void) { guard let url = URL(string: urlString) else { completion(.failure(LocalAccountDelegateError.invalidParameter)) @@ -107,6 +107,20 @@ 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 restoreFolder(for account: Account, folder: Folder, completion: @escaping (Result) -> Void) { + account.addFolder(folder) + completion(.success(())) + } + func accountDidInitialize(_ account: Account) { } diff --git a/Mac/MainWindow/AddFeed/AddFeedController.swift b/Mac/MainWindow/AddFeed/AddFeedController.swift index 2264f588a..45a32f939 100644 --- a/Mac/MainWindow/AddFeed/AddFeedController.swift +++ b/Mac/MainWindow/AddFeed/AddFeedController.swift @@ -59,7 +59,7 @@ class AddFeedController: AddFeedWindowControllerDelegate { return } - account.createFeed(with: nil, url: url.absoluteString) { [weak self] result in + account.createFeed(url: url.absoluteString) { [weak self] result in self?.endShowingProgress() diff --git a/Mac/Scriptability/Feed+Scriptability.swift b/Mac/Scriptability/Feed+Scriptability.swift index 33cb3b04b..9a9151a9d 100644 --- a/Mac/Scriptability/Feed+Scriptability.swift +++ b/Mac/Scriptability/Feed+Scriptability.swift @@ -100,7 +100,7 @@ class ScriptableFeed: NSObject, UniqueIdScriptingObject, ScriptingObjectContaine // suspendExecution(). When we get the callback, we can supply the event result and call resumeExecution() command.suspendExecution() - account.createFeed(with: nil, url: url) { result in + account.createFeed(url: url) { result in switch result { case .success(let createFeedResult): diff --git a/Shared/Commands/DeleteCommand.swift b/Shared/Commands/DeleteCommand.swift index cbd0d69e7..bf38155d7 100644 --- a/Shared/Commands/DeleteCommand.swift +++ b/Shared/Commands/DeleteCommand.swift @@ -136,29 +136,11 @@ private struct SidebarItemSpecifier { if let feed = feed { account?.deleteFeed(feed) { result in - switch result { - case .success(): - break - case .failure(let error): - #if os(macOS) - NSApplication.shared.presentError(error) - #else - UIApplication.shared.presentError(error) - #endif - } + self.checkResult(result) } } else if let folder = folder { account?.deleteFolder(folder) { result in - switch result { - case .success(): - break - case .failure(let error): - #if os(macOS) - NSApplication.shared.presentError(error) - #else - UIApplication.shared.presentError(error) - #endif - } + self.checkResult(result) } } } @@ -178,7 +160,11 @@ private struct SidebarItemSpecifier { guard let account = account, let feed = feed else { return } -// account.addFeed(feed, to: resolvedFolder()) + + account.restoreFeed(feed, folder: resolvedFolder()) { result in + self.checkResult(result) + } + } private func restoreFolder() { @@ -186,17 +172,36 @@ private struct SidebarItemSpecifier { guard let account = account, let folder = folder else { return } - account.addFolder(folder, to: nil) + + account.restoreFolder(folder) { result in + self.checkResult(result) + } + } private func resolvedFolder() -> Folder? { - return path.resolveContainer() as? Folder } + + private func checkResult(_ result: Result) { + + switch result { + case .success: + break + case .failure(let error): + #if os(macOS) + NSApplication.shared.presentError(error) + #else + UIApplication.shared.presentError(error) + #endif + } + + } + } private extension Node { - + func parentFolder() -> Folder? { guard let parentNode = self.parent else {