FIX: ensures removing a reaction doesn’t remove others (#20869)

plugins/chat/assets/javascripts/discourse/models/chat-message.js

@@ -151,18 +151,23 @@ export default class ChatMessage {
         }
         existingReaction.users.pushObject(actor);
       } else {
-        existingReaction.count = existingReaction.count - 1;
+        const existingUserReaction = existingReaction.users.find(
+          (user) => user.id === actor.id
+        );
+
+        if (!existingUserReaction) {
+          return;
+        }
 
         if (selfReaction) {
           existingReaction.reacted = false;
         }
 
-        if (existingReaction.count === 0) {
+        if (existingReaction.count === 1) {
           this.reactions.removeObject(existingReaction);
         } else {
-          existingReaction.users.removeObject(
+          existingReaction.count = existingReaction.count - 1;
-            existingReaction.users.find((user) => user.id === actor.id)
+          existingReaction.users.removeObject(existingUserReaction);
-          );
         }
       }
     } else {

plugins/chat/spec/system/page_objects/chat/chat_channel.rb

@@ -112,15 +112,13 @@ module PageObjects
         within(message_by_id(message.id)) { find(".chat-message-bookmarked") }
       end
 
-      def find_reaction(message, reaction)
+      def find_reaction(message, emoji)
-        within(message_reactions_list(message)) do
+        within(message_reactions_list(message)) { return find("[data-emoji-name=\"#{emoji}\"]") }
-          return find("[data-emoji-name=\"#{reaction.emoji}\"]")
-        end
       end
 
-      def has_reaction?(message, reaction, text = nil)
+      def has_reaction?(message, emoji, text = nil)
         within(message_reactions_list(message)) do
-          has_css?("[data-emoji-name=\"#{reaction.emoji}\"]", text: text)
+          has_css?("[data-emoji-name=\"#{emoji}\"]", text: text)
         end
       end
 
@@ -136,8 +134,8 @@ module PageObjects
         within(message_by_id(message.id)) { has_no_css?(".chat-message-reaction-list") }
       end
 
-      def click_reaction(message, reaction)
+      def click_reaction(message, emoji)
-        find_reaction(message, reaction).click
+        find_reaction(message, emoji).click
       end
 
       def open_action_menu

plugins/chat/spec/system/react_to_message_spec.rb

@@ -26,14 +26,14 @@ RSpec.describe "React to message", type: :system, js: true do
       it "shows existing reactions under the message" do
         sign_in(current_user)
         chat.visit_channel(category_channel_1)
-        expect(channel).to have_reaction(message_1, reaction_1)
+        expect(channel).to have_reaction(message_1, reaction_1.emoji)
       end
 
       it "increments when clicking it" do
         sign_in(current_user)
         chat.visit_channel(category_channel_1)
-        channel.click_reaction(message_1, reaction_1)
+        channel.click_reaction(message_1, reaction_1.emoji)
-        expect(channel).to have_reaction(message_1, reaction_1, 2)
+        expect(channel).to have_reaction(message_1, reaction_1.emoji, 2)
       end
     end
 
@@ -64,7 +64,7 @@ RSpec.describe "React to message", type: :system, js: true do
           find(".chat-message-react-btn").click
           find(".chat-emoji-picker [data-emoji=\"nerd_face\"]").click
 
-          expect(channel).to have_reaction(message_1, reaction_1)
+          expect(channel).to have_reaction(message_1, reaction_1.emoji)
         end
 
         context "when current user has multiple sessions" do
@@ -86,12 +86,12 @@ RSpec.describe "React to message", type: :system, js: true do
               find(".chat-message-react-btn").click
               find(".chat-emoji-picker [data-emoji=\"#{reaction.emoji}\"]").click
 
-              expect(channel).to have_reaction(message_1, reaction)
+              expect(channel).to have_reaction(message_1, reaction.emoji)
               session.quit
             end
 
             using_session(:tab_2) do |session|
-              expect(channel).to have_reaction(message_1, reaction)
+              expect(channel).to have_reaction(message_1, reaction.emoji)
               session.quit
             end
           end
@@ -107,7 +107,7 @@ RSpec.describe "React to message", type: :system, js: true do
             find(".chat-message-actions .react-btn").click
             find(".chat-emoji-picker [data-emoji=\"nerd_face\"]").click
 
-            expect(channel).to have_reaction(message_1, reaction_1)
+            expect(channel).to have_reaction(message_1, reaction_1.emoji)
           end
         end
 
@@ -127,6 +127,42 @@ RSpec.describe "React to message", type: :system, js: true do
     end
   end
 
+  context "when current user and another have reacted" do
+    fab!(:other_user) { Fabricate(:user) }
+
+    fab!(:reaction_1) do
+      Chat::MessageReactor.new(current_user, category_channel_1).react!(
+        message_id: message_1.id,
+        react_action: :add,
+        emoji: "female_detective",
+      )
+    end
+
+    fab!(:reaction_2) do
+      Chat::MessageReactor.new(other_user, category_channel_1).react!(
+        message_id: message_1.id,
+        react_action: :add,
+        emoji: "female_detective",
+      )
+    end
+
+    context "when removing the reaction" do
+      it "removes only the reaction from the current user" do
+        sign_in(current_user)
+        chat.visit_channel(category_channel_1)
+
+        expect(channel).to have_reaction(message_1, "female_detective", "2")
+
+        channel.click_reaction(message_1, "female_detective")
+
+        expect(channel).to have_reaction(message_1, "female_detective", "1")
+        expect(
+          channel.find_reaction(message_1, "female_detective")["data-tippy-content"],
+        ).to include(other_user.username)
+      end
+    end
+  end
+
   context "when current user has reacted" do
     fab!(:reaction_1) do
       Chat::MessageReactor.new(current_user, category_channel_1).react!(
@@ -140,13 +176,13 @@ RSpec.describe "React to message", type: :system, js: true do
       it "shows existing reactions under the message" do
         sign_in(current_user)
         chat.visit_channel(category_channel_1)
-        expect(channel).to have_reaction(message_1, reaction_1)
+        expect(channel).to have_reaction(message_1, reaction_1.emoji)
       end
 
       it "removes it when clicking it" do
         sign_in(current_user)
         chat.visit_channel(category_channel_1)
-        channel.click_reaction(message_1, reaction_1)
+        channel.click_reaction(message_1, reaction_1.emoji)
         expect(channel).to have_no_reactions(message_1)
       end
     end
@@ -177,7 +213,7 @@ RSpec.describe "React to message", type: :system, js: true do
         Chat::Publisher.publish_reaction!(category_channel_1, message_1, "add", user_1, "heart")
         channel.send_message("test") # cheap trick to ensure reaction has been processed
 
-        expect(channel).to have_reaction(message_1, reaction_2, "1")
+        expect(channel).to have_reaction(message_1, reaction_2.emoji, "1")
       end
     end
   end