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

@@ -536,8 +536,8 @@ public final class Account: DisplayNameProvider, UnreadCountProvider, Container,
 		addOPMLItems(OPMLNormalizer.normalize(items))		
 	}
 	
-	public func markArticles(_ articles: Set<Article>, statusKey: ArticleStatus.Key, flag: Bool) {
+	public func markArticles(_ articles: Set<Article>, statusKey: ArticleStatus.Key, flag: Bool, completion: @escaping (Result<Void, Error>) -> Void) {
-		delegate.markArticles(for: self, articles: articles, statusKey: statusKey, flag: flag)
+		delegate.markArticles(for: self, articles: articles, statusKey: statusKey, flag: flag, completion: completion)
 	}
 
 	func existingContainer(withExternalID externalID: String) -> Container? {

Account/Sources/Account/AccountDelegate.swift

@@ -44,7 +44,7 @@ protocol AccountDelegate {
 	func restoreWebFeed(for account: Account, feed: WebFeed, container: Container, completion: @escaping (Result<Void, Error>) -> Void)
 	func restoreFolder(for account: Account, folder: Folder, completion: @escaping (Result<Void, Error>) -> Void)
 
-	func markArticles(for account: Account, articles: Set<Article>, statusKey: ArticleStatus.Key, flag: Bool)
+	func markArticles(for account: Account, articles: Set<Article>, statusKey: ArticleStatus.Key, flag: Bool, completion: @escaping (Result<Void, Error>) -> Void)
 
 	// Called at the end of account’s init method.
 	func accountDidInitialize(_ account: Account)

Account/Sources/Account/CloudKit/CloudKitAccountDelegate.swift

@@ -387,7 +387,7 @@ final class CloudKitAccountDelegate: AccountDelegate {
 		}
 	}
 
-	func markArticles(for account: Account, articles: Set<Article>, statusKey: ArticleStatus.Key, flag: Bool) {
+	func markArticles(for account: Account, articles: Set<Article>, statusKey: ArticleStatus.Key, flag: Bool, completion: @escaping (Result<Void, Error>) -> 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))
 			}
 		}
 	}

Account/Sources/Account/FeedWrangler/FeedWranglerAccountDelegate.swift

@@ -454,7 +454,7 @@ final class FeedWranglerAccountDelegate: AccountDelegate {
 		fatalError()
 	}
 	
-	func markArticles(for account: Account, articles: Set<Article>, statusKey: ArticleStatus.Key, flag: Bool) {
+	func markArticles(for account: Account, articles: Set<Article>, statusKey: ArticleStatus.Key, flag: Bool, completion: @escaping (Result<Void, Error>) -> 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))
 			}
 		}
 	}

Account/Sources/Account/Feedbin/FeedbinAccountDelegate.swift

@@ -536,7 +536,7 @@ final class FeedbinAccountDelegate: AccountDelegate {
 		
 	}
 	
-	func markArticles(for account: Account, articles: Set<Article>, statusKey: ArticleStatus.Key, flag: Bool) {
+	func markArticles(for account: Account, articles: Set<Article>, statusKey: ArticleStatus.Key, flag: Bool, completion: @escaping (Result<Void, Error>) -> 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))
 			}
 		}
 	}

Account/Sources/Account/Feedly/FeedlyAccountDelegate.swift

@@ -487,7 +487,7 @@ final class FeedlyAccountDelegate: AccountDelegate {
 		}
 	}
 	
-	func markArticles(for account: Account, articles: Set<Article>, statusKey: ArticleStatus.Key, flag: Bool) {
+	func markArticles(for account: Account, articles: Set<Article>, statusKey: ArticleStatus.Key, flag: Bool, completion: @escaping (Result<Void, Error>) -> 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))
 			}
 		}
 	}

Account/Sources/Account/LocalAccount/LocalAccountDelegate.swift

@@ -209,8 +209,14 @@ final class LocalAccountDelegate: AccountDelegate {
 		completion(.success(()))
 	}
 
-	func markArticles(for account: Account, articles: Set<Article>, statusKey: ArticleStatus.Key, flag: Bool) {
+	func markArticles(for account: Account, articles: Set<Article>, statusKey: ArticleStatus.Key, flag: Bool, completion: @escaping (Result<Void, Error>) -> Void) {
-		account.update(articles, statusKey: statusKey, flag: flag) { _ in }
+		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) {

Account/Sources/Account/NewsBlur/NewsBlurAccountDelegate.swift

@@ -564,7 +564,7 @@ final class NewsBlurAccountDelegate: AccountDelegate {
 		}
 	}
 
-	func markArticles(for account: Account, articles: Set<Article>, statusKey: ArticleStatus.Key, flag: Bool) {
+	func markArticles(for account: Account, articles: Set<Article>, statusKey: ArticleStatus.Key, flag: Bool, completion: @escaping (Result<Void, Error>) -> 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))
 			}
 		}
 	}

Account/Sources/Account/ReaderAPI/ReaderAPIAccountDelegate.swift

@@ -529,7 +529,7 @@ final class ReaderAPIAccountDelegate: AccountDelegate {
 		
 	}
 
-	func markArticles(for account: Account, articles: Set<Article>, statusKey: ArticleStatus.Key, flag: Bool) {
+	func markArticles(for account: Account, articles: Set<Article>, statusKey: ArticleStatus.Key, flag: Bool, completion: @escaping (Result<Void, Error>) -> 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))
 			}
 		}
 	}

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, completion: completion)
-        markArticles(articles, statusKey: statusKey, flag: flag)
+		completion = nil
     }
 
 	static private let markReadActionName = NSLocalizedString("Mark Read", comment: "command")

Shared/Extensions/ArticleUtilities.swift

@@ -13,15 +13,24 @@ import Account
 
 // These handle multiple accounts.
 
-func markArticles(_ articles: Set<Article>, statusKey: ArticleStatus.Key, flag: Bool) {
+func markArticles(_ articles: Set<Article>, statusKey: ArticleStatus.Key, flag: Bool, completion: (() -> Void)? = nil) {
 	
 	let d: [String: Set<Article>] = 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?()
 	}
 }
 

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, *) {

iOS/RootSplitViewController.swift

@@ -58,8 +58,9 @@ class RootSplitViewController: UISplitViewController {
 	}
 	
 	@objc func markAllAsReadAndGoToNextUnread(_ sender: Any?) {
-		coordinator.markAllAsReadInTimeline()
+		coordinator.markAllAsReadInTimeline() {
-		coordinator.selectNextUnread()
+			self.coordinator.selectNextUnread()
+		}
 	}
 
 	@objc func markAboveAsRead(_ sender: Any?) {

iOS/SceneCoordinator.swift

@@ -997,13 +997,15 @@ class SceneCoordinator: NSObject, UndoableCommandRunner, UnreadCountProvider {
 		}
 	}
 	
-	func markAllAsRead(_ articles: [Article]) {
+	func markAllAsRead(_ articles: [Article], completion: (() -> Void)? = nil) {
-		markArticlesWithUndo(articles, statusKey: .read, flag: true)
+		markArticlesWithUndo(articles, statusKey: .read, flag: true, completion: completion)
 	}
 	
-	func markAllAsReadInTimeline() {
+	func markAllAsReadInTimeline(completion: (() -> Void)? = nil) {
-		markAllAsRead(articles)
+		markAllAsRead(articles) {
-		masterNavigationController.popViewController(animated: true)
+			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) {
+	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) else {
+		guard let undoManager = undoManager,
+			  let markReadCommand = MarkStatusCommand(initialArticles: articles, statusKey: statusKey, flag: flag, undoManager: undoManager, completion: completion) else {
 			return
 		}
 		runCommand(markReadCommand)