From 919f71537e1f89c1d4f833282888617ede803a37 Mon Sep 17 00:00:00 2001 From: Rafael dos Santos Silva Date: Mon, 9 May 2022 17:23:39 -0300 Subject: [PATCH] FIX: Background like count update didn't account for own user actions (#16688) This fixes a corner case of the perf optimization in d4e35f5. When you have the the same post showing in multiple tab/devices and like said post in one place, we updated the like count but didn't flip the `acted` bool in the front-end. This caused a small visual desync. Co-authored-by: Penar Musaraj --- .../discourse/app/controllers/topic.js | 10 +++++++-- .../discourse/app/models/post-stream.js | 4 ++-- .../javascripts/discourse/app/models/post.js | 22 +++++++++---------- lib/post_action_creator.rb | 2 +- lib/post_action_destroyer.rb | 2 +- spec/lib/post_action_creator_spec.rb | 13 +++++++++++ spec/lib/post_action_destroyer_spec.rb | 3 ++- 7 files changed, 37 insertions(+), 19 deletions(-) diff --git a/app/assets/javascripts/discourse/app/controllers/topic.js b/app/assets/javascripts/discourse/app/controllers/topic.js index f0fd230fcf9..ab42d954fa3 100644 --- a/app/assets/javascripts/discourse/app/controllers/topic.js +++ b/app/assets/javascripts/discourse/app/controllers/topic.js @@ -1712,9 +1712,15 @@ export default Controller.extend(bufferedProperty("model"), { .then(() => refresh({ id: data.id, refreshLikes: true })); break; } - case "liked": { + case "liked": + case "unliked": { postStream - .triggerLikedPost(data.id, data.likes_count) + .triggerLikedPost( + data.id, + data.likes_count, + data.user_id, + data.type + ) .then(() => refresh({ id: data.id, refreshLikes: true })); break; } diff --git a/app/assets/javascripts/discourse/app/models/post-stream.js b/app/assets/javascripts/discourse/app/models/post-stream.js index e85d80de64e..7c509f75d8f 100644 --- a/app/assets/javascripts/discourse/app/models/post-stream.js +++ b/app/assets/javascripts/discourse/app/models/post-stream.js @@ -847,12 +847,12 @@ export default RestModel.extend({ return resolved; }, - triggerLikedPost(postId, likesCount) { + triggerLikedPost(postId, likesCount, userID, eventType) { const resolved = Promise.resolve(); const post = this.findLoadedPost(postId); if (post) { - post.updateLikeCount(likesCount); + post.updateLikeCount(likesCount, userID, eventType); this.storePost(post); } diff --git a/app/assets/javascripts/discourse/app/models/post.js b/app/assets/javascripts/discourse/app/models/post.js index 16ea1d81532..0d52a034721 100644 --- a/app/assets/javascripts/discourse/app/models/post.js +++ b/app/assets/javascripts/discourse/app/models/post.js @@ -355,21 +355,18 @@ const Post = RestModel.extend({ } }, - updateLikeCount(count) { + updateLikeCount(count, userId, eventType) { + let ownLike = User.current()?.id === userId && eventType === "liked"; let current_actions_summary = this.get("actions_summary"); let likeActionID = Site.current().post_action_types.find( (a) => a.name_key === "like" ).id; + const newActionObject = { id: likeActionID, count, acted: ownLike }; if (!this.actions_summary.find((entry) => entry.id === likeActionID)) { let json = Post.munge({ id: this.id, - actions_summary: [ - { - id: likeActionID, - count, - }, - ], + actions_summary: [newActionObject], }); this.set( "actions_summary", @@ -378,11 +375,12 @@ const Post = RestModel.extend({ this.set("actionByName", json.actionByName); this.set("likeAction", json.likeAction); } else { - this.actions_summary.find( - (entry) => entry.id === likeActionID - ).count = count; - this.actionByName["like"] = count; - this.likeAction.count = count; + Object.assign( + this.actions_summary.find((entry) => entry.id === likeActionID), + newActionObject + ); + Object.assign(this.actionByName["like"], newActionObject); + Object.assign(this.likeAction, newActionObject); } }, diff --git a/lib/post_action_creator.rb b/lib/post_action_creator.rb index 910078766f8..7b8256b380f 100644 --- a/lib/post_action_creator.rb +++ b/lib/post_action_creator.rb @@ -161,7 +161,7 @@ private def notify_subscribers if @post_action_name == :like - @post.publish_change_to_clients! :liked, { likes_count: @post.like_count + 1 } + @post.publish_change_to_clients! :liked, { likes_count: @post.like_count + 1, user_id: @created_by.id } elsif self.class.notify_types.include?(@post_action_name) @post.publish_change_to_clients! :acted end diff --git a/lib/post_action_destroyer.rb b/lib/post_action_destroyer.rb index d215f1616e5..697047de906 100644 --- a/lib/post_action_destroyer.rb +++ b/lib/post_action_destroyer.rb @@ -65,7 +65,7 @@ protected def notify_subscribers name = PostActionType.types[@post_action_type_id] if name == :like - @post.publish_change_to_clients!(:liked, { likes_count: @post.like_count }) + @post.publish_change_to_clients!(:unliked, { likes_count: @post.like_count, user_id: @destroyed_by.id }) elsif self.class.notify_types.include?(name) @post.publish_change_to_clients!(:acted) end diff --git a/spec/lib/post_action_creator_spec.rb b/spec/lib/post_action_creator_spec.rb index 483fa9ffe1b..c5f201f2b59 100644 --- a/spec/lib/post_action_creator_spec.rb +++ b/spec/lib/post_action_creator_spec.rb @@ -60,6 +60,19 @@ describe PostActionCreator do expect(result.post_action.post_action_type_id).to eq(like_type_id) end + it 'notifies subscribers' do + expect(post.reload.like_count).to eq(0) + + messages = MessageBus.track_publish do + PostActionCreator.new(user, post, like_type_id).perform + end + + message = messages.last.data + expect(message[:type]).to eq(:liked) + expect(message[:likes_count]).to eq(1) + expect(message[:user_id]).to eq(user.id) + end + it 'does not create an invalid post action' do result = PostActionCreator.new(user, nil, like_type_id).perform expect(result.failed?).to eq(true) diff --git a/spec/lib/post_action_destroyer_spec.rb b/spec/lib/post_action_destroyer_spec.rb index 1b333f86a48..ad0d2f7db00 100644 --- a/spec/lib/post_action_destroyer_spec.rb +++ b/spec/lib/post_action_destroyer_spec.rb @@ -26,8 +26,9 @@ describe PostActionDestroyer do end message = messages.last.data - expect(message[:type]).to eq(:liked) + expect(message[:type]).to eq(:unliked) expect(message[:likes_count]).to eq(0) + expect(message[:user_id]).to eq(user.id) end end