Skip to content

Commit f6202c5

Browse files
Notifications: Fix multiple with issues with the new like inline action (#22708)
2 parents a948db1 + bd38f0e commit f6202c5

File tree

8 files changed

+153
-37
lines changed

8 files changed

+153
-37
lines changed

WordPress/Classes/Services/NotificationSyncMediator.swift

Lines changed: 93 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -305,52 +305,56 @@ final class NotificationSyncMediator: NotificationSyncMediatorProtocol {
305305
/// Deletes the note with the given ID from Core Data.
306306
///
307307
func deleteNote(noteID: String) {
308-
Self.operationQueue.addOperation(AsyncBlockOperation { [contextManager] done in
308+
Self.operationQueue.addOperation(AsyncBlockOperation { [contextManager] operationCompletion in
309309
contextManager.performAndSave({ context in
310310
let predicate = NSPredicate(format: "(notificationId == %@)", noteID)
311311

312312
for orphan in context.allObjects(ofType: Notification.self, matching: predicate) {
313313
context.deleteObject(orphan)
314314
}
315-
}, completion: done, on: .main)
315+
}, completion: operationCompletion, on: .main)
316316
})
317317
}
318318

319319
/// Invalidates the local cache for the notification with the specified ID.
320320
///
321-
func invalidateCacheForNotification(_ noteID: String) {
322-
invalidateCacheForNotifications([noteID])
321+
func invalidateCacheForNotification(_ noteID: String,
322+
completion: (() -> Void)? = nil) {
323+
invalidateCacheForNotifications([noteID], completion: completion)
323324
}
324325

325326
/// Invalidates the local cache for all the notifications with specified ID's in the array.
326327
///
327-
func invalidateCacheForNotifications(_ noteIDs: [String]) {
328-
Self.operationQueue.addOperation(AsyncBlockOperation { [contextManager] done in
328+
func invalidateCacheForNotifications(_ noteIDs: [String],
329+
completion: (() -> Void)? = nil) {
330+
Self.operationQueue.addOperation(AsyncBlockOperation { [contextManager] operationCompletion in
329331
contextManager.performAndSave({ context in
330332
let predicate = NSPredicate(format: "(notificationId IN %@)", noteIDs)
331333
let notifications = context.allObjects(ofType: Notification.self, matching: predicate)
332334

333335
notifications.forEach { $0.notificationHash = nil }
334-
}, completion: done, on: .main)
336+
}, completion: {
337+
completion?()
338+
operationCompletion()
339+
}, on: .main)
335340
})
336341
}
337342

338343
func toggleLikeForPostNotification(isLike: Bool,
339344
postID: UInt,
340345
siteID: UInt,
341346
completion: @escaping (Result<Bool, Swift.Error>) -> Void) {
347+
let success = { [weak self] () -> Void in
348+
self?.updatePostLikeStatusLocally(isLike: isLike, postID: postID, siteID: siteID, completion: completion)
349+
}
342350
if isLike {
343-
readerRemoteService.likePost(postID, forSite: siteID) {
344-
completion(.success(isLike))
345-
} failure: { error in
351+
readerRemoteService.likePost(postID, forSite: siteID, success: success, failure: { error in
346352
completion(.failure(error ?? ServiceError.unknown))
347-
}
353+
})
348354
} else {
349-
readerRemoteService.unlikePost(postID, forSite: siteID) {
350-
completion(.success(isLike))
351-
} failure: { error in
355+
readerRemoteService.unlikePost(postID, forSite: siteID, success: success, failure: { error in
352356
completion(.failure(error ?? ServiceError.unknown))
353-
}
357+
})
354358
}
355359
}
356360

@@ -359,16 +363,15 @@ final class NotificationSyncMediator: NotificationSyncMediatorProtocol {
359363
siteID: UInt,
360364
completion: @escaping (Result<Bool, Swift.Error>) -> Void) {
361365
let commentService = commentRemoteFactory.restRemote(siteID: NSNumber(value: siteID), api: restAPI)
366+
let success = { [weak self] () -> Void in
367+
self?.updateCommentLikeStatusLocally(isLike: isLike, commentID: commentID, siteID: siteID, completion: completion)
368+
}
362369
if isLike {
363-
commentService.likeComment(withID: NSNumber(value: commentID)) {
364-
completion(.success(isLike))
365-
} failure: { error in
370+
commentService.likeComment(withID: NSNumber(value: commentID), success: success) { error in
366371
completion(.failure(error ?? ServiceError.unknown))
367372
}
368373
} else {
369-
commentService.unlikeComment(withID: NSNumber(value: commentID)) {
370-
completion(.success(isLike))
371-
} failure: { error in
374+
commentService.unlikeComment(withID: NSNumber(value: commentID), success: success) { error in
372375
completion(.failure(error ?? ServiceError.unknown))
373376
}
374377
}
@@ -386,7 +389,7 @@ private extension NotificationSyncMediator {
386389
/// - completion: Callback to be executed on completion
387390
///
388391
func determineUpdatedNotes(with remoteHashes: [RemoteNotification], completion: @escaping (([String]) -> Void)) {
389-
Self.operationQueue.addOperation(AsyncBlockOperation { [contextManager] done in
392+
Self.operationQueue.addOperation(AsyncBlockOperation { [contextManager] operationCompletion in
390393
contextManager.performAndSave({ context in
391394
let remoteIds = remoteHashes.map { $0.notificationId }
392395
let predicate = NSPredicate(format: "(notificationId IN %@)", remoteIds)
@@ -404,7 +407,7 @@ private extension NotificationSyncMediator {
404407
.map { $0.notificationId }
405408
}, completion: { outdatedIds in
406409
completion(outdatedIds)
407-
done()
410+
operationCompletion()
408411
}, on: .main)
409412
})
410413
}
@@ -417,7 +420,7 @@ private extension NotificationSyncMediator {
417420
/// - completion: Callback to be executed on completion
418421
///
419422
func updateLocalNotes(with remoteNotes: [RemoteNotification], completion: (() -> Void)? = nil) {
420-
Self.operationQueue.addOperation(AsyncBlockOperation { [contextManager] done in
423+
Self.operationQueue.addOperation(AsyncBlockOperation { [contextManager] operationCompletion in
421424
contextManager.performAndSave({ context in
422425
for remoteNote in remoteNotes {
423426
let predicate = NSPredicate(format: "(notificationId == %@)", remoteNote.notificationId)
@@ -426,7 +429,7 @@ private extension NotificationSyncMediator {
426429
localNote.update(with: remoteNote)
427430
}
428431
}, completion: {
429-
done()
432+
operationCompletion()
430433
DispatchQueue.main.async {
431434
completion?()
432435
}
@@ -440,7 +443,7 @@ private extension NotificationSyncMediator {
440443
/// - Parameter remoteHashes: Collection of remoteNotifications.
441444
///
442445
func deleteLocalMissingNotes(from remoteHashes: [RemoteNotification], completion: @escaping (() -> Void)) {
443-
Self.operationQueue.addOperation(AsyncBlockOperation { [contextManager] done in
446+
Self.operationQueue.addOperation(AsyncBlockOperation { [contextManager] operationCompletion in
444447
contextManager.performAndSave({ context in
445448
let remoteIds = remoteHashes.map { $0.notificationId }
446449
let predicate = NSPredicate(format: "NOT (notificationId IN %@)", remoteIds)
@@ -449,7 +452,7 @@ private extension NotificationSyncMediator {
449452
context.deleteObject(orphan)
450453
}
451454
}, completion: {
452-
done()
455+
operationCompletion()
453456
DispatchQueue.main.async {
454457
completion()
455458
}
@@ -495,11 +498,74 @@ private extension NotificationSyncMediator {
495498
let notificationCenter = NotificationCenter.default
496499
notificationCenter.post(name: Foundation.Notification.Name(rawValue: NotificationSyncMediatorDidUpdateNotifications), object: nil)
497500
}
501+
502+
/// Attempts to fetch a `Comment` object matching the comment and site IDs from the local cache
503+
/// If found, the like status is updated. If not found, nothing happens
504+
/// - Parameters:
505+
/// - isLike: Indicates whether this is a like or unlike
506+
/// - commentID: Comment identifier used to fetch the comment
507+
/// - siteID: Site identifier used to fetch the comment
508+
/// - completion: Callback block which is called when the local comment is updated.
509+
func updateCommentLikeStatusLocally(isLike: Bool,
510+
commentID: UInt,
511+
siteID: UInt,
512+
completion: @escaping (Result<Bool, Swift.Error>) -> Void) {
513+
contextManager.performAndSave({ context in
514+
do {
515+
let fetchRequest = NSFetchRequest<Comment>(entityName: Comment.entityName())
516+
fetchRequest.fetchLimit = 1
517+
let commentIDPredicate = NSPredicate(format: "\(#keyPath(Comment.commentID)) == %d", commentID)
518+
let siteIDPredicate = NSPredicate(format: "blog.blogID = %@", NSNumber(value: siteID))
519+
fetchRequest.predicate = NSCompoundPredicate(andPredicateWithSubpredicates: [commentIDPredicate, siteIDPredicate])
520+
if let comment = try context.fetch(fetchRequest).first {
521+
comment.isLiked = isLike
522+
comment.likeCount = comment.likeCount + (comment.isLiked ? 1 : -1)
523+
}
524+
}
525+
catch {
526+
completion(.failure(ServiceError.localPersistenceError))
527+
}
528+
}, completion: {
529+
completion(.success(isLike))
530+
}, on: .main)
531+
}
532+
533+
/// Attempts to fetch a `ReaderPost` object matching the post and site IDs from the local cache
534+
/// If found, the like status is updated. If not found, nothing happens
535+
/// - Parameters:
536+
/// - isLike: Indicates whether this is a like or unlike
537+
/// - postID: Post identifier used to fetch the post
538+
/// - siteID: Site identifier used to fetch the post
539+
/// - completion: Callback block which is called when the local post is updated.
540+
func updatePostLikeStatusLocally(isLike: Bool,
541+
postID: UInt,
542+
siteID: UInt,
543+
completion: @escaping (Result<Bool, Swift.Error>) -> Void) {
544+
contextManager.performAndSave({ context in
545+
do {
546+
let fetchRequest = NSFetchRequest<ReaderPost>(entityName: ReaderPost.entityName())
547+
fetchRequest.fetchLimit = 1
548+
let commentIDPredicate = NSPredicate(format: "\(#keyPath(ReaderPost.postID)) == %d", postID)
549+
let siteIDPredicate = NSPredicate(format: "\(#keyPath(ReaderPost.siteID)) = %@", NSNumber(value: siteID))
550+
fetchRequest.predicate = NSCompoundPredicate(andPredicateWithSubpredicates: [commentIDPredicate, siteIDPredicate])
551+
if let post = try context.fetch(fetchRequest).first {
552+
post.isLiked = isLike
553+
post.likeCount = NSNumber(value: post.likeCount.intValue + (post.isLiked ? 1 : -1))
554+
}
555+
}
556+
catch {
557+
completion(.failure(ServiceError.localPersistenceError))
558+
}
559+
}, completion: {
560+
completion(.success(isLike))
561+
}, on: .main)
562+
}
498563
}
499564

500565
extension NotificationSyncMediator {
501566

502567
enum ServiceError: Error {
503568
case unknown
569+
case localPersistenceError
504570
}
505571
}

WordPress/Classes/Utility/WPTableViewHandler.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,10 @@
8686
- (void)scrollViewWillEndDragging:(nonnull UIScrollView *)scrollView withVelocity:(CGPoint)velocity targetContentOffset:(nonnull inout CGPoint *)targetContentOffset;
8787
- (void)scrollViewDidEndDragging:(nonnull UIScrollView *)scrollView willDecelerate:(BOOL)decelerate;
8888

89+
#pragma mark - Customizing animations
90+
91+
- (BOOL)shouldCancelUpdateAnimation;
92+
8993
@end
9094

9195

WordPress/Classes/Utility/WPTableViewHandler.m

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -705,8 +705,14 @@ - (void)controller:(NSFetchedResultsController *)controller
705705
break;
706706
case NSFetchedResultsChangeUpdate:
707707
{
708-
[self invalidateCachedRowHeightAtIndexPath:indexPath];
709-
[self.tableView reloadRowsAtIndexPaths:@[indexPath] withRowAnimation:self.updateRowAnimation];
708+
BOOL shouldCancelUpdateAnimation =
709+
[self.delegate respondsToSelector:@selector(shouldCancelUpdateAnimation)]
710+
&& [self.delegate shouldCancelUpdateAnimation];
711+
712+
if (!shouldCancelUpdateAnimation) {
713+
[self invalidateCachedRowHeightAtIndexPath:indexPath];
714+
[self.tableView reloadRowsAtIndexPaths:@[indexPath] withRowAnimation:self.updateRowAnimation];
715+
}
710716
}
711717
break;
712718
case NSFetchedResultsChangeMove:

WordPress/Classes/ViewRelated/Comments/CommentDetailViewController.swift

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -678,7 +678,15 @@ private extension CommentDetailViewController {
678678
CommentAnalytics.trackCommentLiked(comment: comment)
679679
}
680680

681-
commentService.toggleLikeStatus(for: comment, siteID: siteID, success: {}, failure: { _ in
681+
commentService.toggleLikeStatus(for: comment, siteID: siteID, success: { [weak self] in
682+
guard let self, let notification = self.notification else {
683+
return
684+
}
685+
let mediator = NotificationSyncMediator()
686+
mediator?.invalidateCacheForNotification(notification.notificationId, completion: {
687+
mediator?.syncNote(with: notification.notificationId)
688+
})
689+
}, failure: { _ in
682690
self.refreshData() // revert the like button state.
683691
})
684692
}

WordPress/Classes/ViewRelated/Notifications/Controllers/NotificationsViewController/NotificationTableViewCell.swift

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,12 @@ final class NotificationTableViewCell: HostingTableViewCell<NotificationsTableVi
77

88
// MARK: - API
99

10-
func configure(with notification: Notification, deletionRequest: NotificationDeletionRequest, parent: UIViewController, onDeletionRequestCanceled: @escaping () -> Void) {
10+
func configure(with notification: Notification, deletionRequest: NotificationDeletionRequest, parent: NotificationsViewController, onDeletionRequestCanceled: @escaping () -> Void) {
1111
let style = NotificationsTableViewCellContent.Style.altered(.init(text: deletionRequest.kind.legendText, action: onDeletionRequestCanceled))
1212
self.host(.init(style: style), parent: parent)
1313
}
1414

15-
func configure(with viewModel: NotificationsViewModel, notification: Notification, parent: UIViewController) {
15+
func configure(with viewModel: NotificationsViewModel, notification: Notification, parent: NotificationsViewController) {
1616
let title: AttributedString? = {
1717
guard let attributedSubject = notification.renderSubject() else {
1818
return nil
@@ -36,13 +36,13 @@ final class NotificationTableViewCell: HostingTableViewCell<NotificationsTableVi
3636

3737
// MARK: - Private Methods
3838

39-
private func inlineAction(viewModel: NotificationsViewModel, notification: Notification, parent: UIViewController) -> NotificationsTableViewCellContent.InlineAction.Configuration? {
39+
private func inlineAction(viewModel: NotificationsViewModel, notification: Notification, parent: NotificationsViewController) -> NotificationsTableViewCellContent.InlineAction.Configuration? {
4040
let notification = notification.parsed()
4141
switch notification {
4242
case .comment(let notification):
43-
return commentLikeInlineAction(viewModel: viewModel, notification: notification)
43+
return commentLikeInlineAction(viewModel: viewModel, notification: notification, parent: parent)
4444
case .newPost(let notification):
45-
return postLikeInlineAction(viewModel: viewModel, notification: notification)
45+
return postLikeInlineAction(viewModel: viewModel, notification: notification, parent: parent)
4646
case .other(let notification):
4747
guard notification.kind == .like || notification.kind == .reblog else {
4848
return nil
@@ -71,11 +71,14 @@ final class NotificationTableViewCell: HostingTableViewCell<NotificationsTableVi
7171
)
7272
}
7373

74-
private func postLikeInlineAction(viewModel: NotificationsViewModel, notification: NewPostNotification) -> NotificationsTableViewCellContent.InlineAction.Configuration {
74+
private func postLikeInlineAction(viewModel: NotificationsViewModel,
75+
notification: NewPostNotification,
76+
parent: NotificationsViewController) -> NotificationsTableViewCellContent.InlineAction.Configuration {
7577
let action: () -> Void = { [weak self] in
7678
guard let self, let content = self.content, case let .regular(style) = content.style, let config = style.inlineAction else {
7779
return
7880
}
81+
parent.cancelNextUpdateAnimation()
7982
viewModel.likeActionTapped(with: notification, action: .postLike) { liked in
8083
let (image, color) = self.likeInlineActionIcon(filled: liked)
8184
config.icon = image
@@ -86,11 +89,14 @@ final class NotificationTableViewCell: HostingTableViewCell<NotificationsTableVi
8689
return .init(icon: image, color: color, action: action)
8790
}
8891

89-
private func commentLikeInlineAction(viewModel: NotificationsViewModel, notification: CommentNotification) -> NotificationsTableViewCellContent.InlineAction.Configuration {
92+
private func commentLikeInlineAction(viewModel: NotificationsViewModel,
93+
notification: CommentNotification,
94+
parent: NotificationsViewController) -> NotificationsTableViewCellContent.InlineAction.Configuration {
9095
let action: () -> Void = { [weak self] in
9196
guard let self, let content = self.content, case let .regular(style) = content.style, let config = style.inlineAction else {
9297
return
9398
}
99+
parent.cancelNextUpdateAnimation()
94100
viewModel.likeActionTapped(with: notification, action: .commentLike) { liked in
95101
let (image, color) = self.likeInlineActionIcon(filled: liked)
96102
config.icon = image

WordPress/Classes/ViewRelated/Notifications/Controllers/NotificationsViewController/NotificationsViewController.swift

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,8 @@ class NotificationsViewController: UIViewController, UIViewControllerRestoration
9494
///
9595
private var timestampBeforeUpdatesForSecondAlert: String?
9696

97+
private var shouldCancelNextUpdateAnimation = false
98+
9799
private lazy var notificationCommentDetailCoordinator: NotificationCommentDetailCoordinator = {
98100
return NotificationCommentDetailCoordinator(notificationsNavigationDataSource: self)
99101
}()
@@ -836,6 +838,7 @@ extension NotificationsViewController {
836838
let readerViewController = ReaderDetailViewController.controllerWithPostID(postID, siteID: siteID)
837839
readerViewController.navigationItem.largeTitleDisplayMode = .never
838840
readerViewController.hidesBottomBarWhenPushed = true
841+
readerViewController.coordinator?.notificationID = note.notificationId
839842
displayViewController(readerViewController)
840843
return
841844
}
@@ -969,6 +972,10 @@ extension NotificationsViewController {
969972

970973
present(navigationController, animated: true, completion: nil)
971974
}
975+
976+
func cancelNextUpdateAnimation() {
977+
shouldCancelNextUpdateAnimation = true
978+
}
972979
}
973980

974981
// MARK: - Notifications Deletion Mechanism
@@ -1466,6 +1473,10 @@ extension NotificationsViewController: WPTableViewHandlerDelegate {
14661473
}
14671474

14681475
func tableViewDidChangeContent(_ tableView: UITableView) {
1476+
guard shouldCancelNextUpdateAnimation == false else {
1477+
shouldCancelNextUpdateAnimation = false
1478+
return
1479+
}
14691480
refreshUnreadNotifications()
14701481

14711482
// Update NoResults View
@@ -1488,6 +1499,10 @@ extension NotificationsViewController: WPTableViewHandlerDelegate {
14881499
}
14891500
}
14901501

1502+
func shouldCancelUpdateAnimation() -> Bool {
1503+
return shouldCancelNextUpdateAnimation
1504+
}
1505+
14911506
// counts the new notifications for the second alert
14921507
private var newNotificationsForSecondAlert: Int {
14931508

0 commit comments

Comments
 (0)