Fix off-by-one error in post likes (#23912)

* Fix formatting in getLikesFor

* Refactor ReaderDetailLikesView

* Simplify ReaderDetailLikesView further

* Update release notes

* Fix compilation in tests

RELEASE-NOTES.txt

@@ -19,6 +19,7 @@
 * [*] Reader: The post cover now uses the standard aspect ratio for covers, so there is no jumping. There are also a few minor improvements to the layout and animations of the cover and the header [#23897, #23909]
 * [*] Reader: Move the "Reading Preferences" button to the "More" menu [#23897]
 * [*] Reader: Hide post toolbar when reading an article and fix like button animations [#23909]
+* [*] Reader: Fix off-by-one error in post details like counter when post is liked by you [#23912]
 
 25.5
 -----

WordPress/Classes/ViewRelated/Reader/Detail/ReaderDetailCoordinator.swift

@@ -1,5 +1,6 @@
 import Foundation
 import WordPressShared
+import Combine
 
 class ReaderDetailCoordinator {
 
@@ -43,6 +44,8 @@ class ReaderDetailCoordinator {
     /// Called if the view controller's post fails to load
     var postLoadFailureBlock: (() -> Void)? = nil
 
+    private var likesAvatarURLs: [String]?
+
     /// An authenticator to ensure any request made to WP sites is properly authenticated
     lazy var authenticator: RequestAuthenticator? = {
         guard let account = try? WPAccount.lookupDefaultWordPressComAccount(in: coreDataStack.mainContext) else {
@@ -102,10 +105,7 @@ class ReaderDetailCoordinator {
     }
 
     private var followCommentsService: FollowCommentsService?
-
-    /// The total number of Likes for the post.
-    /// Passed to ReaderDetailLikesListController to display in the view title.
-    private var totalLikes = 0
+    private var likesObserver: AnyCancellable?
 
     /// Initialize the Reader Detail Coordinator
     ///
@@ -156,41 +156,50 @@ class ReaderDetailCoordinator {
         }
     }
 
-    /// Fetch Likes for the current post.
-    /// Returns `ReaderDetailLikesView.maxAvatarsDisplayed` number of Likes.
-    ///
     func fetchLikes(for post: ReaderPost) {
-        guard let postID = post.postID else {
-            return
-        }
+        guard let postID = post.postID else { return }
 
         // Fetch a full page of Likes but only return the `maxAvatarsDisplayed` number.
         // That way the first page will already be cached if the user displays the full Likes list.
-        postService.getLikesFor(postID: postID,
-                                siteID: post.siteID,
-                                success: { [weak self] users, totalLikes, _ in
-                                    var filteredUsers = users
-                                    var currentLikeUser: LikeUser? = nil
-                                    let totalLikesExcludingSelf = totalLikes - (post.isLiked ? 1 : 0)
-
-                                    // Split off current user's like from the list.
-                                    // Likes from self will always be placed in the last position, regardless of the when the post was liked.
-                                    if let userID = try? WPAccount.lookupDefaultWordPressComAccount(in: ContextManager.shared.mainContext)?.userID.int64Value,
-                                       let userIndex = filteredUsers.firstIndex(where: { $0.userID == userID }) {
-                                        currentLikeUser = filteredUsers.remove(at: userIndex)
-                                    }
-
-                                    self?.totalLikes = totalLikes
-                                    self?.view?.updateLikes(with: filteredUsers.prefix(ReaderDetailLikesView.maxAvatarsDisplayed).map { $0.avatarUrl },
-                                                            totalLikes: totalLikesExcludingSelf)
-                                    // Only pass current user's avatar when we know *for sure* that the post is liked.
-                                    // This is to work around a possible race condition that causes an unliked post to have current user's LikeUser, which
-                                    // would cause a display bug in ReaderDetailLikesView. The race condition issue will be investigated separately.
-                                    self?.view?.updateSelfLike(with: post.isLiked ? currentLikeUser?.avatarUrl : nil)
-                                }, failure: { [weak self] error in
-                                    self?.view?.updateLikes(with: [String](), totalLikes: 0)
-                                    DDLogError("Error fetching Likes for post detail: \(String(describing: error?.localizedDescription))")
-                                })
+        postService.getLikesFor(postID: postID, siteID: post.siteID, success: { [weak self] users, totalLikes, _ in
+            guard let self else { return }
+
+            var filteredUsers = users
+            if let userID = try? WPAccount.lookupDefaultWordPressComAccount(in: ContextManager.shared.mainContext)?.userID.int64Value,
+               let userIndex = filteredUsers.firstIndex(where: { $0.userID == userID }) {
+                filteredUsers.remove(at: userIndex)
+            }
+
+            self.likesAvatarURLs = filteredUsers.prefix(ReaderDetailLikesView.maxAvatarsDisplayed).map(\.avatarUrl)
+            self.updateLikesView()
+            self.startObservingLikes()
+        }, failure: { error in
+            DDLogError("Error fetching Likes for post detail: \(String(describing: error?.localizedDescription))")
+        })
+    }
+
+    private func startObservingLikes() {
+        guard let post else {
+            return wpAssertionFailure("post missing")
+        }
+
+        likesObserver = Publishers.CombineLatest(
+            post.publisher(for: \.likeCount, options: [.new]).removeDuplicates(),
+            post.publisher(for: \.isLiked, options: [.new]).removeDuplicates()
+        ).sink { [weak self] _, _ in
+            self?.updateLikesView()
+        }
+    }
+
+    private func updateLikesView() {
+        guard let post, let likesAvatarURLs else { return }
+
+        let viewModel = ReaderDetailLikesViewModel(
+            likeCount: post.likeCount.intValue,
+            avatarURLs: likesAvatarURLs,
+            selfLikeAvatarURL: post.isLiked ? try? WPAccount.lookupDefaultWordPressComAccount(in: ContextManager.shared.mainContext)?.avatarURL : nil
+        )
+        view?.updateLikesView(with: viewModel)
     }
 
     /// Fetch Comments for the current post.
@@ -590,8 +599,7 @@ class ReaderDetailCoordinator {
         guard let post else {
             return
         }
-
-        let controller = ReaderDetailLikesListController(post: post, totalLikes: totalLikes)
+        let controller = ReaderDetailLikesListController(post: post, totalLikes: post.likeCount.intValue)
         viewController?.navigationController?.pushViewController(controller, animated: true)
     }
 
@@ -712,30 +720,19 @@ extension ReaderDetailCoordinator: ReaderDetailHeaderViewDelegate {
     }
 }
 
-// MARK: - ReaderDetailFeaturedImageViewDelegate
 extension ReaderDetailCoordinator: ReaderDetailFeaturedImageViewDelegate {
     func didTapFeaturedImage(_ sender: AsyncImageView) {
         showFeaturedImage(sender)
     }
 }
 
-// MARK: - ReaderDetailLikesViewDelegate
 extension ReaderDetailCoordinator: ReaderDetailLikesViewDelegate {
     func didTapLikesView() {
         showLikesList()
     }
 }
 
-// MARK: - ReaderDetailToolbarDelegate
-extension ReaderDetailCoordinator: ReaderDetailToolbarDelegate {
-    func didTapLikeButton(isLiked: Bool) {
-        guard let userAvatarURL = try? WPAccount.lookupDefaultWordPressComAccount(in: ContextManager.shared.mainContext)?.avatarURL else {
-            return
-        }
-
-        self.view?.updateSelfLike(with: isLiked ? userAvatarURL : nil)
-    }
-}
+extension ReaderDetailCoordinator: ReaderDetailToolbarDelegate {}
 
 // MARK: - Private Definitions
 

WordPress/Classes/ViewRelated/Reader/Detail/ReaderDetailViewController.swift

@@ -12,20 +12,7 @@ protocol ReaderDetailView: AnyObject {
     func showErrorWithWebAction()
     func scroll(to: String)
     func updateHeader()
-
-    /// Shows likes view containing avatars of users that liked the post.
-    /// The number of avatars displayed is limited to `ReaderDetailView.maxAvatarDisplayed` plus the current user's avatar.
-    /// Note that the current user's avatar is displayed through a different method.
-    ///
-    /// - Seealso: `updateSelfLike(with avatarURLString: String?)`
-    /// - Parameters:
-    ///   - avatarURLStrings: A list of URL strings for the liking users' avatars.
-    ///   - totalLikes: The total number of likes for this post.
-    func updateLikes(with avatarURLStrings: [String], totalLikes: Int)
-
-    /// Updates the likes view to append an additional avatar for the current user, indicating that the post is liked by current user.
-    /// - Parameter avatarURLString: The URL string for the current user's avatar. Optional.
-    func updateSelfLike(with avatarURLString: String?)
+    func updateLikesView(with viewModel: ReaderDetailLikesViewModel)
 
     /// Updates comments table to display the post's comments.
     /// - Parameters:
@@ -374,38 +361,17 @@ class ReaderDetailViewController: UIViewController, ReaderDetailView {
         header.refreshFollowButton()
     }
 
-    func updateLikes(with avatarURLStrings: [String], totalLikes: Int) {
-        // always configure likes summary view first regardless of totalLikes, since it can affected by self likes.
-        likesSummary.configure(with: avatarURLStrings, totalLikes: totalLikes)
-
-        guard totalLikes > 0 else {
+    func updateLikesView(with viewModel: ReaderDetailLikesViewModel) {
+        guard viewModel.likeCount > 0 else {
             hideLikesView()
             return
         }
-
         if likesSummary.superview == nil {
             configureLikesSummary()
         }
-
         scrollView.layoutIfNeeded()
-    }
-
-    func updateSelfLike(with avatarURLString: String?) {
-        // only animate changes when the view is visible.
-        let shouldAnimate = isVisibleInScrollView(likesSummary)
-        guard let someURLString = avatarURLString else {
-            likesSummary.removeSelfAvatar(animated: shouldAnimate)
-            if likesSummary.totalLikesForDisplay == 0 {
-                hideLikesView()
-            }
-            return
-        }
-
-        if likesSummary.superview == nil {
-            configureLikesSummary()
-        }
 
-        likesSummary.addSelfAvatar(with: someURLString, animated: shouldAnimate)
+        likesSummary.configure(with: viewModel)
     }
 
     func updateComments(_ comments: [Comment], totalComments: Int) {