Skip to content

Commit

Permalink
Notifications: Fix multiple with issues with the new like inline acti…
Browse files Browse the repository at this point in the history
…on (#22708)
  • Loading branch information
hassaanelgarem authored Feb 27, 2024
2 parents a948db1 + bd38f0e commit f6202c5
Show file tree
Hide file tree
Showing 8 changed files with 153 additions and 37 deletions.
120 changes: 93 additions & 27 deletions WordPress/Classes/Services/NotificationSyncMediator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -305,52 +305,56 @@ final class NotificationSyncMediator: NotificationSyncMediatorProtocol {
/// Deletes the note with the given ID from Core Data.
///
func deleteNote(noteID: String) {
Self.operationQueue.addOperation(AsyncBlockOperation { [contextManager] done in
Self.operationQueue.addOperation(AsyncBlockOperation { [contextManager] operationCompletion in
contextManager.performAndSave({ context in
let predicate = NSPredicate(format: "(notificationId == %@)", noteID)

for orphan in context.allObjects(ofType: Notification.self, matching: predicate) {
context.deleteObject(orphan)
}
}, completion: done, on: .main)
}, completion: operationCompletion, on: .main)
})
}

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

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

notifications.forEach { $0.notificationHash = nil }
}, completion: done, on: .main)
}, completion: {
completion?()
operationCompletion()
}, on: .main)
})
}

func toggleLikeForPostNotification(isLike: Bool,
postID: UInt,
siteID: UInt,
completion: @escaping (Result<Bool, Swift.Error>) -> Void) {
let success = { [weak self] () -> Void in
self?.updatePostLikeStatusLocally(isLike: isLike, postID: postID, siteID: siteID, completion: completion)
}
if isLike {
readerRemoteService.likePost(postID, forSite: siteID) {
completion(.success(isLike))
} failure: { error in
readerRemoteService.likePost(postID, forSite: siteID, success: success, failure: { error in
completion(.failure(error ?? ServiceError.unknown))
}
})
} else {
readerRemoteService.unlikePost(postID, forSite: siteID) {
completion(.success(isLike))
} failure: { error in
readerRemoteService.unlikePost(postID, forSite: siteID, success: success, failure: { error in
completion(.failure(error ?? ServiceError.unknown))
}
})
}
}

Expand All @@ -359,16 +363,15 @@ final class NotificationSyncMediator: NotificationSyncMediatorProtocol {
siteID: UInt,
completion: @escaping (Result<Bool, Swift.Error>) -> Void) {
let commentService = commentRemoteFactory.restRemote(siteID: NSNumber(value: siteID), api: restAPI)
let success = { [weak self] () -> Void in
self?.updateCommentLikeStatusLocally(isLike: isLike, commentID: commentID, siteID: siteID, completion: completion)
}
if isLike {
commentService.likeComment(withID: NSNumber(value: commentID)) {
completion(.success(isLike))
} failure: { error in
commentService.likeComment(withID: NSNumber(value: commentID), success: success) { error in
completion(.failure(error ?? ServiceError.unknown))
}
} else {
commentService.unlikeComment(withID: NSNumber(value: commentID)) {
completion(.success(isLike))
} failure: { error in
commentService.unlikeComment(withID: NSNumber(value: commentID), success: success) { error in
completion(.failure(error ?? ServiceError.unknown))
}
}
Expand All @@ -386,7 +389,7 @@ private extension NotificationSyncMediator {
/// - completion: Callback to be executed on completion
///
func determineUpdatedNotes(with remoteHashes: [RemoteNotification], completion: @escaping (([String]) -> Void)) {
Self.operationQueue.addOperation(AsyncBlockOperation { [contextManager] done in
Self.operationQueue.addOperation(AsyncBlockOperation { [contextManager] operationCompletion in
contextManager.performAndSave({ context in
let remoteIds = remoteHashes.map { $0.notificationId }
let predicate = NSPredicate(format: "(notificationId IN %@)", remoteIds)
Expand All @@ -404,7 +407,7 @@ private extension NotificationSyncMediator {
.map { $0.notificationId }
}, completion: { outdatedIds in
completion(outdatedIds)
done()
operationCompletion()
}, on: .main)
})
}
Expand All @@ -417,7 +420,7 @@ private extension NotificationSyncMediator {
/// - completion: Callback to be executed on completion
///
func updateLocalNotes(with remoteNotes: [RemoteNotification], completion: (() -> Void)? = nil) {
Self.operationQueue.addOperation(AsyncBlockOperation { [contextManager] done in
Self.operationQueue.addOperation(AsyncBlockOperation { [contextManager] operationCompletion in
contextManager.performAndSave({ context in
for remoteNote in remoteNotes {
let predicate = NSPredicate(format: "(notificationId == %@)", remoteNote.notificationId)
Expand All @@ -426,7 +429,7 @@ private extension NotificationSyncMediator {
localNote.update(with: remoteNote)
}
}, completion: {
done()
operationCompletion()
DispatchQueue.main.async {
completion?()
}
Expand All @@ -440,7 +443,7 @@ private extension NotificationSyncMediator {
/// - Parameter remoteHashes: Collection of remoteNotifications.
///
func deleteLocalMissingNotes(from remoteHashes: [RemoteNotification], completion: @escaping (() -> Void)) {
Self.operationQueue.addOperation(AsyncBlockOperation { [contextManager] done in
Self.operationQueue.addOperation(AsyncBlockOperation { [contextManager] operationCompletion in
contextManager.performAndSave({ context in
let remoteIds = remoteHashes.map { $0.notificationId }
let predicate = NSPredicate(format: "NOT (notificationId IN %@)", remoteIds)
Expand All @@ -449,7 +452,7 @@ private extension NotificationSyncMediator {
context.deleteObject(orphan)
}
}, completion: {
done()
operationCompletion()
DispatchQueue.main.async {
completion()
}
Expand Down Expand Up @@ -495,11 +498,74 @@ private extension NotificationSyncMediator {
let notificationCenter = NotificationCenter.default
notificationCenter.post(name: Foundation.Notification.Name(rawValue: NotificationSyncMediatorDidUpdateNotifications), object: nil)
}

/// Attempts to fetch a `Comment` object matching the comment and site IDs from the local cache
/// If found, the like status is updated. If not found, nothing happens
/// - Parameters:
/// - isLike: Indicates whether this is a like or unlike
/// - commentID: Comment identifier used to fetch the comment
/// - siteID: Site identifier used to fetch the comment
/// - completion: Callback block which is called when the local comment is updated.
func updateCommentLikeStatusLocally(isLike: Bool,
commentID: UInt,
siteID: UInt,
completion: @escaping (Result<Bool, Swift.Error>) -> Void) {
contextManager.performAndSave({ context in
do {
let fetchRequest = NSFetchRequest<Comment>(entityName: Comment.entityName())
fetchRequest.fetchLimit = 1
let commentIDPredicate = NSPredicate(format: "\(#keyPath(Comment.commentID)) == %d", commentID)
let siteIDPredicate = NSPredicate(format: "blog.blogID = %@", NSNumber(value: siteID))
fetchRequest.predicate = NSCompoundPredicate(andPredicateWithSubpredicates: [commentIDPredicate, siteIDPredicate])
if let comment = try context.fetch(fetchRequest).first {
comment.isLiked = isLike
comment.likeCount = comment.likeCount + (comment.isLiked ? 1 : -1)
}
}
catch {
completion(.failure(ServiceError.localPersistenceError))
}
}, completion: {
completion(.success(isLike))
}, on: .main)
}

/// Attempts to fetch a `ReaderPost` object matching the post and site IDs from the local cache
/// If found, the like status is updated. If not found, nothing happens
/// - Parameters:
/// - isLike: Indicates whether this is a like or unlike
/// - postID: Post identifier used to fetch the post
/// - siteID: Site identifier used to fetch the post
/// - completion: Callback block which is called when the local post is updated.
func updatePostLikeStatusLocally(isLike: Bool,
postID: UInt,
siteID: UInt,
completion: @escaping (Result<Bool, Swift.Error>) -> Void) {
contextManager.performAndSave({ context in
do {
let fetchRequest = NSFetchRequest<ReaderPost>(entityName: ReaderPost.entityName())
fetchRequest.fetchLimit = 1
let commentIDPredicate = NSPredicate(format: "\(#keyPath(ReaderPost.postID)) == %d", postID)
let siteIDPredicate = NSPredicate(format: "\(#keyPath(ReaderPost.siteID)) = %@", NSNumber(value: siteID))
fetchRequest.predicate = NSCompoundPredicate(andPredicateWithSubpredicates: [commentIDPredicate, siteIDPredicate])
if let post = try context.fetch(fetchRequest).first {
post.isLiked = isLike
post.likeCount = NSNumber(value: post.likeCount.intValue + (post.isLiked ? 1 : -1))
}
}
catch {
completion(.failure(ServiceError.localPersistenceError))
}
}, completion: {
completion(.success(isLike))
}, on: .main)
}
}

extension NotificationSyncMediator {

enum ServiceError: Error {
case unknown
case localPersistenceError
}
}
4 changes: 4 additions & 0 deletions WordPress/Classes/Utility/WPTableViewHandler.h
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,10 @@
- (void)scrollViewWillEndDragging:(nonnull UIScrollView *)scrollView withVelocity:(CGPoint)velocity targetContentOffset:(nonnull inout CGPoint *)targetContentOffset;
- (void)scrollViewDidEndDragging:(nonnull UIScrollView *)scrollView willDecelerate:(BOOL)decelerate;

#pragma mark - Customizing animations

- (BOOL)shouldCancelUpdateAnimation;

@end


Expand Down
10 changes: 8 additions & 2 deletions WordPress/Classes/Utility/WPTableViewHandler.m
Original file line number Diff line number Diff line change
Expand Up @@ -705,8 +705,14 @@ - (void)controller:(NSFetchedResultsController *)controller
break;
case NSFetchedResultsChangeUpdate:
{
[self invalidateCachedRowHeightAtIndexPath:indexPath];
[self.tableView reloadRowsAtIndexPaths:@[indexPath] withRowAnimation:self.updateRowAnimation];
BOOL shouldCancelUpdateAnimation =
[self.delegate respondsToSelector:@selector(shouldCancelUpdateAnimation)]
&& [self.delegate shouldCancelUpdateAnimation];

if (!shouldCancelUpdateAnimation) {
[self invalidateCachedRowHeightAtIndexPath:indexPath];
[self.tableView reloadRowsAtIndexPaths:@[indexPath] withRowAnimation:self.updateRowAnimation];
}
}
break;
case NSFetchedResultsChangeMove:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -678,7 +678,15 @@ private extension CommentDetailViewController {
CommentAnalytics.trackCommentLiked(comment: comment)
}

commentService.toggleLikeStatus(for: comment, siteID: siteID, success: {}, failure: { _ in
commentService.toggleLikeStatus(for: comment, siteID: siteID, success: { [weak self] in
guard let self, let notification = self.notification else {
return
}
let mediator = NotificationSyncMediator()
mediator?.invalidateCacheForNotification(notification.notificationId, completion: {
mediator?.syncNote(with: notification.notificationId)
})
}, failure: { _ in
self.refreshData() // revert the like button state.
})
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,12 @@ final class NotificationTableViewCell: HostingTableViewCell<NotificationsTableVi

// MARK: - API

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

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

// MARK: - Private Methods

private func inlineAction(viewModel: NotificationsViewModel, notification: Notification, parent: UIViewController) -> NotificationsTableViewCellContent.InlineAction.Configuration? {
private func inlineAction(viewModel: NotificationsViewModel, notification: Notification, parent: NotificationsViewController) -> NotificationsTableViewCellContent.InlineAction.Configuration? {
let notification = notification.parsed()
switch notification {
case .comment(let notification):
return commentLikeInlineAction(viewModel: viewModel, notification: notification)
return commentLikeInlineAction(viewModel: viewModel, notification: notification, parent: parent)
case .newPost(let notification):
return postLikeInlineAction(viewModel: viewModel, notification: notification)
return postLikeInlineAction(viewModel: viewModel, notification: notification, parent: parent)
case .other(let notification):
guard notification.kind == .like || notification.kind == .reblog else {
return nil
Expand Down Expand Up @@ -71,11 +71,14 @@ final class NotificationTableViewCell: HostingTableViewCell<NotificationsTableVi
)
}

private func postLikeInlineAction(viewModel: NotificationsViewModel, notification: NewPostNotification) -> NotificationsTableViewCellContent.InlineAction.Configuration {
private func postLikeInlineAction(viewModel: NotificationsViewModel,
notification: NewPostNotification,
parent: NotificationsViewController) -> NotificationsTableViewCellContent.InlineAction.Configuration {
let action: () -> Void = { [weak self] in
guard let self, let content = self.content, case let .regular(style) = content.style, let config = style.inlineAction else {
return
}
parent.cancelNextUpdateAnimation()
viewModel.likeActionTapped(with: notification, action: .postLike) { liked in
let (image, color) = self.likeInlineActionIcon(filled: liked)
config.icon = image
Expand All @@ -86,11 +89,14 @@ final class NotificationTableViewCell: HostingTableViewCell<NotificationsTableVi
return .init(icon: image, color: color, action: action)
}

private func commentLikeInlineAction(viewModel: NotificationsViewModel, notification: CommentNotification) -> NotificationsTableViewCellContent.InlineAction.Configuration {
private func commentLikeInlineAction(viewModel: NotificationsViewModel,
notification: CommentNotification,
parent: NotificationsViewController) -> NotificationsTableViewCellContent.InlineAction.Configuration {
let action: () -> Void = { [weak self] in
guard let self, let content = self.content, case let .regular(style) = content.style, let config = style.inlineAction else {
return
}
parent.cancelNextUpdateAnimation()
viewModel.likeActionTapped(with: notification, action: .commentLike) { liked in
let (image, color) = self.likeInlineActionIcon(filled: liked)
config.icon = image
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,8 @@ class NotificationsViewController: UIViewController, UIViewControllerRestoration
///
private var timestampBeforeUpdatesForSecondAlert: String?

private var shouldCancelNextUpdateAnimation = false

private lazy var notificationCommentDetailCoordinator: NotificationCommentDetailCoordinator = {
return NotificationCommentDetailCoordinator(notificationsNavigationDataSource: self)
}()
Expand Down Expand Up @@ -836,6 +838,7 @@ extension NotificationsViewController {
let readerViewController = ReaderDetailViewController.controllerWithPostID(postID, siteID: siteID)
readerViewController.navigationItem.largeTitleDisplayMode = .never
readerViewController.hidesBottomBarWhenPushed = true
readerViewController.coordinator?.notificationID = note.notificationId
displayViewController(readerViewController)
return
}
Expand Down Expand Up @@ -969,6 +972,10 @@ extension NotificationsViewController {

present(navigationController, animated: true, completion: nil)
}

func cancelNextUpdateAnimation() {
shouldCancelNextUpdateAnimation = true
}
}

// MARK: - Notifications Deletion Mechanism
Expand Down Expand Up @@ -1466,6 +1473,10 @@ extension NotificationsViewController: WPTableViewHandlerDelegate {
}

func tableViewDidChangeContent(_ tableView: UITableView) {
guard shouldCancelNextUpdateAnimation == false else {
shouldCancelNextUpdateAnimation = false
return
}
refreshUnreadNotifications()

// Update NoResults View
Expand All @@ -1488,6 +1499,10 @@ extension NotificationsViewController: WPTableViewHandlerDelegate {
}
}

func shouldCancelUpdateAnimation() -> Bool {
return shouldCancelNextUpdateAnimation
}

// counts the new notifications for the second alert
private var newNotificationsForSecondAlert: Int {

Expand Down
Loading

0 comments on commit f6202c5

Please sign in to comment.