From f2bbacc871e8ab49032409064f6bd9493857af50 Mon Sep 17 00:00:00 2001 From: Maurice Parker Date: Mon, 12 Apr 2021 19:41:01 -0500 Subject: [PATCH 1/4] Add completion callbacks so that we can ensure that unreads have been marked before determining the next unread. Fixes #2993 --- Account/Sources/Account/Account.swift | 4 ++-- Account/Sources/Account/AccountDelegate.swift | 2 +- .../CloudKit/CloudKitAccountDelegate.swift | 6 +++--- .../FeedWranglerAccountDelegate.swift | 6 +++--- .../Feedbin/FeedbinAccountDelegate.swift | 6 +++--- .../Account/Feedly/FeedlyAccountDelegate.swift | 6 +++--- .../LocalAccount/LocalAccountDelegate.swift | 10 ++++++++-- .../NewsBlur/NewsBlurAccountDelegate.swift | 6 +++--- .../ReaderAPI/ReaderAPIAccountDelegate.swift | 6 +++--- Shared/Commands/MarkStatusCommand.swift | 10 +++++----- Shared/Extensions/ArticleUtilities.swift | 13 +++++++++++-- iOS/AppDelegate.swift | 4 ++-- iOS/RootSplitViewController.swift | 5 +++-- iOS/SceneCoordinator.swift | 17 ++++++++++------- 14 files changed, 60 insertions(+), 41 deletions(-) diff --git a/Account/Sources/Account/Account.swift b/Account/Sources/Account/Account.swift index 2db24eb0d..c802ac572 100644 --- a/Account/Sources/Account/Account.swift +++ b/Account/Sources/Account/Account.swift @@ -536,8 +536,8 @@ public final class Account: DisplayNameProvider, UnreadCountProvider, Container, addOPMLItems(OPMLNormalizer.normalize(items)) } - public func markArticles(_ articles: Set
, statusKey: ArticleStatus.Key, flag: Bool) { - delegate.markArticles(for: self, articles: articles, statusKey: statusKey, flag: flag) + public func markArticles(_ articles: Set
, statusKey: ArticleStatus.Key, flag: Bool, completion: @escaping (Result) -> Void) { + delegate.markArticles(for: self, articles: articles, statusKey: statusKey, flag: flag, completion: completion) } func existingContainer(withExternalID externalID: String) -> Container? { diff --git a/Account/Sources/Account/AccountDelegate.swift b/Account/Sources/Account/AccountDelegate.swift index f86368053..1f8584406 100644 --- a/Account/Sources/Account/AccountDelegate.swift +++ b/Account/Sources/Account/AccountDelegate.swift @@ -44,7 +44,7 @@ protocol AccountDelegate { func restoreWebFeed(for account: Account, feed: WebFeed, 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) + func markArticles(for account: Account, articles: Set
, statusKey: ArticleStatus.Key, flag: Bool, completion: @escaping (Result) -> Void) // Called at the end of account’s init method. func accountDidInitialize(_ account: Account) diff --git a/Account/Sources/Account/CloudKit/CloudKitAccountDelegate.swift b/Account/Sources/Account/CloudKit/CloudKitAccountDelegate.swift index 143b90e6c..c7371079d 100644 --- a/Account/Sources/Account/CloudKit/CloudKitAccountDelegate.swift +++ b/Account/Sources/Account/CloudKit/CloudKitAccountDelegate.swift @@ -387,7 +387,7 @@ final class CloudKitAccountDelegate: AccountDelegate { } } - func markArticles(for account: Account, articles: Set
, statusKey: ArticleStatus.Key, flag: Bool) { + func markArticles(for account: Account, articles: Set
, statusKey: ArticleStatus.Key, flag: Bool, completion: @escaping (Result) -> Void) { account.update(articles, statusKey: statusKey, flag: flag) { result in switch result { case .success(let articles): @@ -398,12 +398,12 @@ final class CloudKitAccountDelegate: AccountDelegate { self.database.insertStatuses(syncStatuses) { _ in self.database.selectPendingCount { result in if let count = try? result.get(), count > 100 { - self.sendArticleStatus(for: account, showProgress: false) { _ in } + self.sendArticleStatus(for: account, showProgress: false, completion: completion) } } } case .failure(let error): - os_log(.error, log: self.log, "Error marking article status: %@", error.localizedDescription) + completion(.failure(error)) } } } diff --git a/Account/Sources/Account/FeedWrangler/FeedWranglerAccountDelegate.swift b/Account/Sources/Account/FeedWrangler/FeedWranglerAccountDelegate.swift index 06edc663f..057585745 100644 --- a/Account/Sources/Account/FeedWrangler/FeedWranglerAccountDelegate.swift +++ b/Account/Sources/Account/FeedWrangler/FeedWranglerAccountDelegate.swift @@ -454,7 +454,7 @@ final class FeedWranglerAccountDelegate: AccountDelegate { fatalError() } - func markArticles(for account: Account, articles: Set
, statusKey: ArticleStatus.Key, flag: Bool) { + func markArticles(for account: Account, articles: Set
, statusKey: ArticleStatus.Key, flag: Bool, completion: @escaping (Result) -> Void) { account.update(articles, statusKey: statusKey, flag: flag) { result in switch result { case .success(let articles): @@ -465,12 +465,12 @@ final class FeedWranglerAccountDelegate: AccountDelegate { self.database.insertStatuses(syncStatuses) { _ in self.database.selectPendingCount { result in if let count = try? result.get(), count > 100 { - self.sendArticleStatus(for: account) { _ in } + self.sendArticleStatus(for: account, completion: completion) } } } case .failure(let error): - os_log(.error, log: self.log, "Error marking article status: %@", error.localizedDescription) + completion(.failure(error)) } } } diff --git a/Account/Sources/Account/Feedbin/FeedbinAccountDelegate.swift b/Account/Sources/Account/Feedbin/FeedbinAccountDelegate.swift index 406a34218..678c73a2e 100644 --- a/Account/Sources/Account/Feedbin/FeedbinAccountDelegate.swift +++ b/Account/Sources/Account/Feedbin/FeedbinAccountDelegate.swift @@ -536,7 +536,7 @@ final class FeedbinAccountDelegate: AccountDelegate { } - func markArticles(for account: Account, articles: Set
, statusKey: ArticleStatus.Key, flag: Bool) { + func markArticles(for account: Account, articles: Set
, statusKey: ArticleStatus.Key, flag: Bool, completion: @escaping (Result) -> Void) { account.update(articles, statusKey: statusKey, flag: flag) { result in switch result { case .success(let articles): @@ -547,12 +547,12 @@ final class FeedbinAccountDelegate: AccountDelegate { self.database.insertStatuses(syncStatuses) { _ in self.database.selectPendingCount { result in if let count = try? result.get(), count > 100 { - self.sendArticleStatus(for: account) { _ in } + self.sendArticleStatus(for: account, completion: completion) } } } case .failure(let error): - os_log(.error, log: self.log, "Error marking article status: %@", error.localizedDescription) + completion(.failure(error)) } } } diff --git a/Account/Sources/Account/Feedly/FeedlyAccountDelegate.swift b/Account/Sources/Account/Feedly/FeedlyAccountDelegate.swift index 56b7533d1..e6672472b 100644 --- a/Account/Sources/Account/Feedly/FeedlyAccountDelegate.swift +++ b/Account/Sources/Account/Feedly/FeedlyAccountDelegate.swift @@ -487,7 +487,7 @@ final class FeedlyAccountDelegate: AccountDelegate { } } - func markArticles(for account: Account, articles: Set
, statusKey: ArticleStatus.Key, flag: Bool) { + func markArticles(for account: Account, articles: Set
, statusKey: ArticleStatus.Key, flag: Bool, completion: @escaping (Result) -> Void) { account.update(articles, statusKey: statusKey, flag: flag) { result in switch result { case .success(let articles): @@ -498,12 +498,12 @@ final class FeedlyAccountDelegate: AccountDelegate { self.database.insertStatuses(syncStatuses) { _ in self.database.selectPendingCount { result in if let count = try? result.get(), count > 100 { - self.sendArticleStatus(for: account) { _ in } + self.sendArticleStatus(for: account, completion: completion) } } } case .failure(let error): - os_log(.error, log: self.log, "Error marking article status: %@", error.localizedDescription) + completion(.failure(error)) } } } diff --git a/Account/Sources/Account/LocalAccount/LocalAccountDelegate.swift b/Account/Sources/Account/LocalAccount/LocalAccountDelegate.swift index 2ae29f7f7..2006f2973 100644 --- a/Account/Sources/Account/LocalAccount/LocalAccountDelegate.swift +++ b/Account/Sources/Account/LocalAccount/LocalAccountDelegate.swift @@ -209,8 +209,14 @@ final class LocalAccountDelegate: AccountDelegate { completion(.success(())) } - func markArticles(for account: Account, articles: Set
, statusKey: ArticleStatus.Key, flag: Bool) { - account.update(articles, statusKey: statusKey, flag: flag) { _ in } + func markArticles(for account: Account, articles: Set
, statusKey: ArticleStatus.Key, flag: Bool, completion: @escaping (Result) -> Void) { + account.update(articles, statusKey: statusKey, flag: flag) { result in + if case .failure(let error) = result { + completion(.failure(error)) + } else { + completion(.success(())) + } + } } func accountDidInitialize(_ account: Account) { diff --git a/Account/Sources/Account/NewsBlur/NewsBlurAccountDelegate.swift b/Account/Sources/Account/NewsBlur/NewsBlurAccountDelegate.swift index 71f4f22b3..85f41a2ab 100644 --- a/Account/Sources/Account/NewsBlur/NewsBlurAccountDelegate.swift +++ b/Account/Sources/Account/NewsBlur/NewsBlurAccountDelegate.swift @@ -564,7 +564,7 @@ final class NewsBlurAccountDelegate: AccountDelegate { } } - func markArticles(for account: Account, articles: Set
, statusKey: ArticleStatus.Key, flag: Bool) { + func markArticles(for account: Account, articles: Set
, statusKey: ArticleStatus.Key, flag: Bool, completion: @escaping (Result) -> Void) { account.update(articles, statusKey: statusKey, flag: flag) { result in switch result { case .success(let articles): @@ -575,12 +575,12 @@ final class NewsBlurAccountDelegate: AccountDelegate { self.database.insertStatuses(syncStatuses) { _ in self.database.selectPendingCount { result in if let count = try? result.get(), count > 100 { - self.sendArticleStatus(for: account) { _ in } + self.sendArticleStatus(for: account, completion: completion) } } } case .failure(let error): - os_log(.error, log: self.log, "Error marking article status: %@", error.localizedDescription) + completion(.failure(error)) } } } diff --git a/Account/Sources/Account/ReaderAPI/ReaderAPIAccountDelegate.swift b/Account/Sources/Account/ReaderAPI/ReaderAPIAccountDelegate.swift index 182db4203..060ba2bbb 100644 --- a/Account/Sources/Account/ReaderAPI/ReaderAPIAccountDelegate.swift +++ b/Account/Sources/Account/ReaderAPI/ReaderAPIAccountDelegate.swift @@ -529,7 +529,7 @@ final class ReaderAPIAccountDelegate: AccountDelegate { } - func markArticles(for account: Account, articles: Set
, statusKey: ArticleStatus.Key, flag: Bool) { + func markArticles(for account: Account, articles: Set
, statusKey: ArticleStatus.Key, flag: Bool, completion: @escaping (Result) -> Void) { account.update(articles, statusKey: statusKey, flag: flag) { result in switch result { case .success(let articles): @@ -540,12 +540,12 @@ final class ReaderAPIAccountDelegate: AccountDelegate { self.database.insertStatuses(syncStatuses) { _ in self.database.selectPendingCount { result in if let count = try? result.get(), count > 100 { - self.sendArticleStatus(for: account) { _ in } + self.sendArticleStatus(for: account, completion: completion) } } } case .failure(let error): - os_log(.error, log: self.log, "Error marking article status: %@", error.localizedDescription) + completion(.failure(error)) } } } diff --git a/Shared/Commands/MarkStatusCommand.swift b/Shared/Commands/MarkStatusCommand.swift index c513413fd..b1e5f4c5c 100644 --- a/Shared/Commands/MarkStatusCommand.swift +++ b/Shared/Commands/MarkStatusCommand.swift @@ -20,8 +20,9 @@ final class MarkStatusCommand: UndoableCommand { let undoManager: UndoManager let flag: Bool let statusKey: ArticleStatus.Key + var completion: (() -> Void)? = nil - init?(initialArticles: [Article], statusKey: ArticleStatus.Key, flag: Bool, undoManager: UndoManager) { + init?(initialArticles: [Article], statusKey: ArticleStatus.Key, flag: Bool, undoManager: UndoManager, completion: (() -> Void)? = nil) { // Filter out articles that already have the desired status or can't be marked. let articlesToMark = MarkStatusCommand.filteredArticles(initialArticles, statusKey, flag) @@ -33,6 +34,7 @@ final class MarkStatusCommand: UndoableCommand { self.flag = flag self.statusKey = statusKey self.undoManager = undoManager + self.completion = completion let actionName = MarkStatusCommand.actionName(statusKey, flag) self.undoActionName = actionName @@ -40,12 +42,10 @@ final class MarkStatusCommand: UndoableCommand { } convenience init?(initialArticles: [Article], markingRead: Bool, undoManager: UndoManager) { - self.init(initialArticles: initialArticles, statusKey: .read, flag: markingRead, undoManager: undoManager) } convenience init?(initialArticles: [Article], markingStarred: Bool, undoManager: UndoManager) { - self.init(initialArticles: initialArticles, statusKey: .starred, flag: markingStarred, undoManager: undoManager) } @@ -63,8 +63,8 @@ final class MarkStatusCommand: UndoableCommand { private extension MarkStatusCommand { func mark(_ statusKey: ArticleStatus.Key, _ flag: Bool) { - - markArticles(articles, statusKey: statusKey, flag: flag) + markArticles(articles, statusKey: statusKey, flag: flag, completion: completion) + completion = nil } static private let markReadActionName = NSLocalizedString("Mark Read", comment: "command") diff --git a/Shared/Extensions/ArticleUtilities.swift b/Shared/Extensions/ArticleUtilities.swift index 6d9f23963..5e1822cba 100644 --- a/Shared/Extensions/ArticleUtilities.swift +++ b/Shared/Extensions/ArticleUtilities.swift @@ -13,15 +13,24 @@ import Account // These handle multiple accounts. -func markArticles(_ articles: Set
, statusKey: ArticleStatus.Key, flag: Bool) { +func markArticles(_ articles: Set
, statusKey: ArticleStatus.Key, flag: Bool, completion: (() -> Void)? = nil) { let d: [String: Set
] = accountAndArticlesDictionary(articles) + let group = DispatchGroup() + for (accountID, accountArticles) in d { guard let account = AccountManager.shared.existingAccount(with: accountID) else { continue } - account.markArticles(accountArticles, statusKey: statusKey, flag: flag) + group.enter() + account.markArticles(accountArticles, statusKey: statusKey, flag: flag) { _ in + group.leave() + } + } + + group.notify(queue: .main) { + completion?() } } diff --git a/iOS/AppDelegate.swift b/iOS/AppDelegate.swift index da8c4dafd..6e1bab493 100644 --- a/iOS/AppDelegate.swift +++ b/iOS/AppDelegate.swift @@ -428,7 +428,7 @@ private extension AppDelegate { os_log(.debug, "No article found from search using %@", articleID) return } - account!.markArticles(article!, statusKey: .read, flag: true) + account!.markArticles(article!, statusKey: .read, flag: true) { _ in } self.prepareAccountsForBackground() account!.syncArticleStatus(completion: { [weak self] _ in if !AccountManager.shared.isSuspended { @@ -458,7 +458,7 @@ private extension AppDelegate { os_log(.debug, "No article found from search using %@", articleID) return } - account!.markArticles(article!, statusKey: .starred, flag: true) + account!.markArticles(article!, statusKey: .starred, flag: true) { _ in } account!.syncArticleStatus(completion: { [weak self] _ in if !AccountManager.shared.isSuspended { if #available(iOS 14, *) { diff --git a/iOS/RootSplitViewController.swift b/iOS/RootSplitViewController.swift index e84a8241f..90b20f3c4 100644 --- a/iOS/RootSplitViewController.swift +++ b/iOS/RootSplitViewController.swift @@ -58,8 +58,9 @@ class RootSplitViewController: UISplitViewController { } @objc func markAllAsReadAndGoToNextUnread(_ sender: Any?) { - coordinator.markAllAsReadInTimeline() - coordinator.selectNextUnread() + coordinator.markAllAsReadInTimeline() { + self.coordinator.selectNextUnread() + } } @objc func markAboveAsRead(_ sender: Any?) { diff --git a/iOS/SceneCoordinator.swift b/iOS/SceneCoordinator.swift index 62361afd7..02346c5f7 100644 --- a/iOS/SceneCoordinator.swift +++ b/iOS/SceneCoordinator.swift @@ -997,13 +997,15 @@ class SceneCoordinator: NSObject, UndoableCommandRunner, UnreadCountProvider { } } - func markAllAsRead(_ articles: [Article]) { - markArticlesWithUndo(articles, statusKey: .read, flag: true) + func markAllAsRead(_ articles: [Article], completion: (() -> Void)? = nil) { + markArticlesWithUndo(articles, statusKey: .read, flag: true, completion: completion) } - func markAllAsReadInTimeline() { - markAllAsRead(articles) - masterNavigationController.popViewController(animated: true) + func markAllAsReadInTimeline(completion: (() -> Void)? = nil) { + markAllAsRead(articles) { + self.masterNavigationController.popViewController(animated: true) + completion?() + } } func canMarkAboveAsRead(for article: Article) -> Bool { @@ -1372,8 +1374,9 @@ extension SceneCoordinator: UINavigationControllerDelegate { private extension SceneCoordinator { - func markArticlesWithUndo(_ articles: [Article], statusKey: ArticleStatus.Key, flag: Bool) { - guard let undoManager = undoManager, let markReadCommand = MarkStatusCommand(initialArticles: articles, statusKey: statusKey, flag: flag, undoManager: undoManager) else { + func markArticlesWithUndo(_ articles: [Article], statusKey: ArticleStatus.Key, flag: Bool, completion: (() -> Void)? = nil) { + guard let undoManager = undoManager, + let markReadCommand = MarkStatusCommand(initialArticles: articles, statusKey: statusKey, flag: flag, undoManager: undoManager, completion: completion) else { return } runCommand(markReadCommand) From 8f0519ca47c2966f6b88d013560a26c994a1747a Mon Sep 17 00:00:00 2001 From: Maurice Parker Date: Mon, 12 Apr 2021 21:09:34 -0500 Subject: [PATCH 2/4] Fix regression where marking all as unread wouldn't take you back to the sidebar --- Account/Sources/Account/CloudKit/CloudKitAccountDelegate.swift | 3 ++- .../Account/FeedWrangler/FeedWranglerAccountDelegate.swift | 3 ++- Account/Sources/Account/Feedbin/FeedbinAccountDelegate.swift | 3 ++- Account/Sources/Account/Feedly/FeedlyAccountDelegate.swift | 3 ++- Account/Sources/Account/NewsBlur/NewsBlurAccountDelegate.swift | 3 ++- .../Sources/Account/ReaderAPI/ReaderAPIAccountDelegate.swift | 3 ++- iOS/SceneCoordinator.swift | 1 + 7 files changed, 13 insertions(+), 6 deletions(-) diff --git a/Account/Sources/Account/CloudKit/CloudKitAccountDelegate.swift b/Account/Sources/Account/CloudKit/CloudKitAccountDelegate.swift index c7371079d..808d2add8 100644 --- a/Account/Sources/Account/CloudKit/CloudKitAccountDelegate.swift +++ b/Account/Sources/Account/CloudKit/CloudKitAccountDelegate.swift @@ -398,8 +398,9 @@ final class CloudKitAccountDelegate: AccountDelegate { self.database.insertStatuses(syncStatuses) { _ in self.database.selectPendingCount { result in if let count = try? result.get(), count > 100 { - self.sendArticleStatus(for: account, showProgress: false, completion: completion) + self.sendArticleStatus(for: account, showProgress: false) { _ in } } + completion(.success(())) } } case .failure(let error): diff --git a/Account/Sources/Account/FeedWrangler/FeedWranglerAccountDelegate.swift b/Account/Sources/Account/FeedWrangler/FeedWranglerAccountDelegate.swift index 057585745..11e7d130b 100644 --- a/Account/Sources/Account/FeedWrangler/FeedWranglerAccountDelegate.swift +++ b/Account/Sources/Account/FeedWrangler/FeedWranglerAccountDelegate.swift @@ -465,8 +465,9 @@ final class FeedWranglerAccountDelegate: AccountDelegate { self.database.insertStatuses(syncStatuses) { _ in self.database.selectPendingCount { result in if let count = try? result.get(), count > 100 { - self.sendArticleStatus(for: account, completion: completion) + self.sendArticleStatus(for: account) { _ in } } + completion(.success(())) } } case .failure(let error): diff --git a/Account/Sources/Account/Feedbin/FeedbinAccountDelegate.swift b/Account/Sources/Account/Feedbin/FeedbinAccountDelegate.swift index 678c73a2e..78d81d4ee 100644 --- a/Account/Sources/Account/Feedbin/FeedbinAccountDelegate.swift +++ b/Account/Sources/Account/Feedbin/FeedbinAccountDelegate.swift @@ -547,8 +547,9 @@ final class FeedbinAccountDelegate: AccountDelegate { self.database.insertStatuses(syncStatuses) { _ in self.database.selectPendingCount { result in if let count = try? result.get(), count > 100 { - self.sendArticleStatus(for: account, completion: completion) + self.sendArticleStatus(for: account) { _ in } } + completion(.success(())) } } case .failure(let error): diff --git a/Account/Sources/Account/Feedly/FeedlyAccountDelegate.swift b/Account/Sources/Account/Feedly/FeedlyAccountDelegate.swift index e6672472b..1cd3e2aed 100644 --- a/Account/Sources/Account/Feedly/FeedlyAccountDelegate.swift +++ b/Account/Sources/Account/Feedly/FeedlyAccountDelegate.swift @@ -498,8 +498,9 @@ final class FeedlyAccountDelegate: AccountDelegate { self.database.insertStatuses(syncStatuses) { _ in self.database.selectPendingCount { result in if let count = try? result.get(), count > 100 { - self.sendArticleStatus(for: account, completion: completion) + self.sendArticleStatus(for: account) { _ in } } + completion(.success(())) } } case .failure(let error): diff --git a/Account/Sources/Account/NewsBlur/NewsBlurAccountDelegate.swift b/Account/Sources/Account/NewsBlur/NewsBlurAccountDelegate.swift index 85f41a2ab..b13ba7da1 100644 --- a/Account/Sources/Account/NewsBlur/NewsBlurAccountDelegate.swift +++ b/Account/Sources/Account/NewsBlur/NewsBlurAccountDelegate.swift @@ -575,8 +575,9 @@ final class NewsBlurAccountDelegate: AccountDelegate { self.database.insertStatuses(syncStatuses) { _ in self.database.selectPendingCount { result in if let count = try? result.get(), count > 100 { - self.sendArticleStatus(for: account, completion: completion) + self.sendArticleStatus(for: account) { _ in } } + completion(.success(())) } } case .failure(let error): diff --git a/Account/Sources/Account/ReaderAPI/ReaderAPIAccountDelegate.swift b/Account/Sources/Account/ReaderAPI/ReaderAPIAccountDelegate.swift index 060ba2bbb..6afb25e87 100644 --- a/Account/Sources/Account/ReaderAPI/ReaderAPIAccountDelegate.swift +++ b/Account/Sources/Account/ReaderAPI/ReaderAPIAccountDelegate.swift @@ -540,8 +540,9 @@ final class ReaderAPIAccountDelegate: AccountDelegate { self.database.insertStatuses(syncStatuses) { _ in self.database.selectPendingCount { result in if let count = try? result.get(), count > 100 { - self.sendArticleStatus(for: account, completion: completion) + self.sendArticleStatus(for: account) { _ in } } + completion(.success(())) } } case .failure(let error): diff --git a/iOS/SceneCoordinator.swift b/iOS/SceneCoordinator.swift index 02346c5f7..87d9ba4d1 100644 --- a/iOS/SceneCoordinator.swift +++ b/iOS/SceneCoordinator.swift @@ -1377,6 +1377,7 @@ private extension SceneCoordinator { func markArticlesWithUndo(_ articles: [Article], statusKey: ArticleStatus.Key, flag: Bool, completion: (() -> Void)? = nil) { guard let undoManager = undoManager, let markReadCommand = MarkStatusCommand(initialArticles: articles, statusKey: statusKey, flag: flag, undoManager: undoManager, completion: completion) else { + completion?() return } runCommand(markReadCommand) From 92fd016b7a6e027f5b6843391ea79ba1e89aa1f8 Mon Sep 17 00:00:00 2001 From: Maurice Parker Date: Thu, 15 Apr 2021 14:13:15 -0500 Subject: [PATCH 3/4] Exclude Inoreader from article status syncs --- Account/Sources/Account/Account.swift | 16 +------------ Account/Sources/Account/AccountDelegate.swift | 1 + .../CloudKit/CloudKitAccountDelegate.swift | 18 +++++++++++++++ .../FeedWranglerAccountDelegate.swift | 18 +++++++++++++++ .../Feedbin/FeedbinAccountDelegate.swift | 18 +++++++++++++++ .../Feedly/FeedlyAccountDelegate.swift | 18 +++++++++++++++ .../LocalAccount/LocalAccountDelegate.swift | 4 ++++ .../NewsBlur/NewsBlurAccountDelegate.swift | 18 +++++++++++++++ .../ReaderAPI/ReaderAPIAccountDelegate.swift | 23 +++++++++++++++++++ 9 files changed, 119 insertions(+), 15 deletions(-) diff --git a/Account/Sources/Account/Account.swift b/Account/Sources/Account/Account.swift index c802ac572..0cb9fb67b 100644 --- a/Account/Sources/Account/Account.swift +++ b/Account/Sources/Account/Account.swift @@ -443,21 +443,7 @@ public final class Account: DisplayNameProvider, UnreadCountProvider, Container, } public func syncArticleStatus(completion: ((Result) -> Void)? = nil) { - delegate.sendArticleStatus(for: self) { [unowned self] result in - switch result { - case .success: - self.delegate.refreshArticleStatus(for: self) { result in - switch result { - case .success: - completion?(.success(())) - case .failure(let error): - completion?(.failure(error)) - } - } - case .failure(let error): - completion?(.failure(error)) - } - } + delegate.syncArticleStatus(for: self, completion: completion) } public func importOPML(_ opmlFile: URL, completion: @escaping (Result) -> Void) { diff --git a/Account/Sources/Account/AccountDelegate.swift b/Account/Sources/Account/AccountDelegate.swift index 1f8584406..93a998561 100644 --- a/Account/Sources/Account/AccountDelegate.swift +++ b/Account/Sources/Account/AccountDelegate.swift @@ -26,6 +26,7 @@ protocol AccountDelegate { func receiveRemoteNotification(for account: Account, userInfo: [AnyHashable : Any], completion: @escaping () -> Void) func refreshAll(for account: Account, completion: @escaping (Result) -> Void) + func syncArticleStatus(for account: Account, completion: ((Result) -> Void)?) func sendArticleStatus(for account: Account, completion: @escaping ((Result) -> Void)) func refreshArticleStatus(for account: Account, completion: @escaping ((Result) -> Void)) diff --git a/Account/Sources/Account/CloudKit/CloudKitAccountDelegate.swift b/Account/Sources/Account/CloudKit/CloudKitAccountDelegate.swift index 808d2add8..98baf7565 100644 --- a/Account/Sources/Account/CloudKit/CloudKitAccountDelegate.swift +++ b/Account/Sources/Account/CloudKit/CloudKitAccountDelegate.swift @@ -92,6 +92,24 @@ final class CloudKitAccountDelegate: AccountDelegate { standardRefreshAll(for: account, completion: completion) } + func syncArticleStatus(for account: Account, completion: ((Result) -> Void)? = nil) { + sendArticleStatus(for: account) { result in + switch result { + case .success: + self.refreshArticleStatus(for: account) { result in + switch result { + case .success: + completion?(.success(())) + case .failure(let error): + completion?(.failure(error)) + } + } + case .failure(let error): + completion?(.failure(error)) + } + } + } + func sendArticleStatus(for account: Account, completion: @escaping ((Result) -> Void)) { sendArticleStatus(for: account, showProgress: false, completion: completion) } diff --git a/Account/Sources/Account/FeedWrangler/FeedWranglerAccountDelegate.swift b/Account/Sources/Account/FeedWrangler/FeedWranglerAccountDelegate.swift index 11e7d130b..1be0ec832 100644 --- a/Account/Sources/Account/FeedWrangler/FeedWranglerAccountDelegate.swift +++ b/Account/Sources/Account/FeedWrangler/FeedWranglerAccountDelegate.swift @@ -150,6 +150,24 @@ final class FeedWranglerAccountDelegate: AccountDelegate { } } + func syncArticleStatus(for account: Account, completion: ((Result) -> Void)? = nil) { + sendArticleStatus(for: account) { result in + switch result { + case .success: + self.refreshArticleStatus(for: account) { result in + switch result { + case .success: + completion?(.success(())) + case .failure(let error): + completion?(.failure(error)) + } + } + case .failure(let error): + completion?(.failure(error)) + } + } + } + func refreshArticles(for account: Account, page: Int = 0, completion: @escaping ((Result) -> Void)) { os_log(.debug, log: log, "Refreshing articles, page: %d...", page) diff --git a/Account/Sources/Account/Feedbin/FeedbinAccountDelegate.swift b/Account/Sources/Account/Feedbin/FeedbinAccountDelegate.swift index 78d81d4ee..418711849 100644 --- a/Account/Sources/Account/Feedbin/FeedbinAccountDelegate.swift +++ b/Account/Sources/Account/Feedbin/FeedbinAccountDelegate.swift @@ -112,6 +112,24 @@ final class FeedbinAccountDelegate: AccountDelegate { } + func syncArticleStatus(for account: Account, completion: ((Result) -> Void)? = nil) { + sendArticleStatus(for: account) { result in + switch result { + case .success: + self.refreshArticleStatus(for: account) { result in + switch result { + case .success: + completion?(.success(())) + case .failure(let error): + completion?(.failure(error)) + } + } + case .failure(let error): + completion?(.failure(error)) + } + } + } + func sendArticleStatus(for account: Account, completion: @escaping ((Result) -> Void)) { os_log(.debug, log: log, "Sending article statuses...") diff --git a/Account/Sources/Account/Feedly/FeedlyAccountDelegate.swift b/Account/Sources/Account/Feedly/FeedlyAccountDelegate.swift index 1cd3e2aed..e2df06bf3 100644 --- a/Account/Sources/Account/Feedly/FeedlyAccountDelegate.swift +++ b/Account/Sources/Account/Feedly/FeedlyAccountDelegate.swift @@ -145,6 +145,24 @@ final class FeedlyAccountDelegate: AccountDelegate { operationQueue.add(syncAllOperation) } + func syncArticleStatus(for account: Account, completion: ((Result) -> Void)? = nil) { + sendArticleStatus(for: account) { result in + switch result { + case .success: + self.refreshArticleStatus(for: account) { result in + switch result { + case .success: + completion?(.success(())) + case .failure(let error): + completion?(.failure(error)) + } + } + case .failure(let error): + completion?(.failure(error)) + } + } + } + func sendArticleStatus(for account: Account, completion: @escaping ((Result) -> Void)) { // Ensure remote articles have the same status as they do locally. let send = FeedlySendArticleStatusesOperation(database: database, service: caller, log: log) diff --git a/Account/Sources/Account/LocalAccount/LocalAccountDelegate.swift b/Account/Sources/Account/LocalAccount/LocalAccountDelegate.swift index 2006f2973..456c8b738 100644 --- a/Account/Sources/Account/LocalAccount/LocalAccountDelegate.swift +++ b/Account/Sources/Account/LocalAccount/LocalAccountDelegate.swift @@ -96,6 +96,10 @@ final class LocalAccountDelegate: AccountDelegate { } + func syncArticleStatus(for account: Account, completion: ((Result) -> Void)? = nil) { + completion?(.success(())) + } + func sendArticleStatus(for account: Account, completion: @escaping ((Result) -> Void)) { completion(.success(())) } diff --git a/Account/Sources/Account/NewsBlur/NewsBlurAccountDelegate.swift b/Account/Sources/Account/NewsBlur/NewsBlurAccountDelegate.swift index b13ba7da1..b415d03d6 100644 --- a/Account/Sources/Account/NewsBlur/NewsBlurAccountDelegate.swift +++ b/Account/Sources/Account/NewsBlur/NewsBlurAccountDelegate.swift @@ -114,6 +114,24 @@ final class NewsBlurAccountDelegate: AccountDelegate { } } + func syncArticleStatus(for account: Account, completion: ((Result) -> Void)? = nil) { + sendArticleStatus(for: account) { result in + switch result { + case .success: + self.refreshArticleStatus(for: account) { result in + switch result { + case .success: + completion?(.success(())) + case .failure(let error): + completion?(.failure(error)) + } + } + case .failure(let error): + completion?(.failure(error)) + } + } + } + func sendArticleStatus(for account: Account, completion: @escaping (Result) -> ()) { os_log(.debug, log: log, "Sending story statuses...") diff --git a/Account/Sources/Account/ReaderAPI/ReaderAPIAccountDelegate.swift b/Account/Sources/Account/ReaderAPI/ReaderAPIAccountDelegate.swift index 6afb25e87..84d8b7353 100644 --- a/Account/Sources/Account/ReaderAPI/ReaderAPIAccountDelegate.swift +++ b/Account/Sources/Account/ReaderAPI/ReaderAPIAccountDelegate.swift @@ -131,6 +131,29 @@ final class ReaderAPIAccountDelegate: AccountDelegate { } + func syncArticleStatus(for account: Account, completion: ((Result) -> Void)? = nil) { + guard variant != .inoreader else { + completion?(.success(())) + return + } + + sendArticleStatus(for: account) { result in + switch result { + case .success: + self.refreshArticleStatus(for: account) { result in + switch result { + case .success: + completion?(.success(())) + case .failure(let error): + completion?(.failure(error)) + } + } + case .failure(let error): + completion?(.failure(error)) + } + } + } + func sendArticleStatus(for account: Account, completion: @escaping ((Result) -> Void)) { os_log(.debug, log: log, "Sending article statuses...") From 3955151daf450fded74385f4c49ad19cc2a078e5 Mon Sep 17 00:00:00 2001 From: Maurice Parker Date: Thu, 15 Apr 2021 14:29:49 -0500 Subject: [PATCH 4/4] Make sure mark as read completes before searching for the next unread. Fixes #2952 --- Mac/AppDelegate.swift | 4 ++-- Mac/MainWindow/MainWindowController.swift | 5 +++-- Mac/MainWindow/Timeline/TimelineViewController.swift | 4 ++-- Shared/Commands/MarkStatusCommand.swift | 8 ++++---- 4 files changed, 11 insertions(+), 10 deletions(-) diff --git a/Mac/AppDelegate.swift b/Mac/AppDelegate.swift index 9b80e8b54..cf4af585a 100644 --- a/Mac/AppDelegate.swift +++ b/Mac/AppDelegate.swift @@ -843,7 +843,7 @@ private extension AppDelegate { os_log(.debug, "No article found from search using %@", articleID) return } - account!.markArticles(article!, statusKey: .read, flag: true) + account!.markArticles(article!, statusKey: .read, flag: true) { _ in } } func handleMarkAsStarred(userInfo: [AnyHashable: Any]) { @@ -862,6 +862,6 @@ private extension AppDelegate { os_log(.debug, "No article found from search using %@", articleID) return } - account!.markArticles(article!, statusKey: .starred, flag: true) + account!.markArticles(article!, statusKey: .starred, flag: true) { _ in } } } diff --git a/Mac/MainWindow/MainWindowController.swift b/Mac/MainWindow/MainWindowController.swift index 4db72dd1e..84e73ef21 100644 --- a/Mac/MainWindow/MainWindowController.swift +++ b/Mac/MainWindow/MainWindowController.swift @@ -397,8 +397,9 @@ class MainWindowController : NSWindowController, NSUserInterfaceValidations { } @IBAction func markAllAsReadAndGoToNextUnread(_ sender: Any?) { - markAllAsRead(sender) - nextUnread(sender) + currentTimelineViewController?.markAllAsRead() { + self.nextUnread(sender) + } } @IBAction func markUnreadAndGoToNextUnread(_ sender: Any?) { diff --git a/Mac/MainWindow/Timeline/TimelineViewController.swift b/Mac/MainWindow/Timeline/TimelineViewController.swift index 18da6aeb8..f8ff8f37b 100644 --- a/Mac/MainWindow/Timeline/TimelineViewController.swift +++ b/Mac/MainWindow/Timeline/TimelineViewController.swift @@ -233,8 +233,8 @@ final class TimelineViewController: NSViewController, UndoableCommandRunner, Unr // MARK: - API - func markAllAsRead() { - guard let undoManager = undoManager, let markReadCommand = MarkStatusCommand(initialArticles: articles, markingRead: true, undoManager: undoManager) else { + func markAllAsRead(completion: (() -> Void)? = nil) { + guard let undoManager = undoManager, let markReadCommand = MarkStatusCommand(initialArticles: articles, markingRead: true, undoManager: undoManager, completion: completion) else { return } runCommand(markReadCommand) diff --git a/Shared/Commands/MarkStatusCommand.swift b/Shared/Commands/MarkStatusCommand.swift index b1e5f4c5c..e6eae4b54 100644 --- a/Shared/Commands/MarkStatusCommand.swift +++ b/Shared/Commands/MarkStatusCommand.swift @@ -41,12 +41,12 @@ final class MarkStatusCommand: UndoableCommand { self.redoActionName = actionName } - convenience init?(initialArticles: [Article], markingRead: Bool, undoManager: UndoManager) { - self.init(initialArticles: initialArticles, statusKey: .read, flag: markingRead, undoManager: undoManager) + convenience init?(initialArticles: [Article], markingRead: Bool, undoManager: UndoManager, completion: (() -> Void)? = nil) { + self.init(initialArticles: initialArticles, statusKey: .read, flag: markingRead, undoManager: undoManager, completion: completion) } - convenience init?(initialArticles: [Article], markingStarred: Bool, undoManager: UndoManager) { - self.init(initialArticles: initialArticles, statusKey: .starred, flag: markingStarred, undoManager: undoManager) + convenience init?(initialArticles: [Article], markingStarred: Bool, undoManager: UndoManager, completion: (() -> Void)? = nil) { + self.init(initialArticles: initialArticles, statusKey: .starred, flag: markingStarred, undoManager: undoManager, completion: completion) } func perform() {