From 69de5b161f53b2649b695891c3969de74d3a61a2 Mon Sep 17 00:00:00 2001 From: Joffrey JAFFEUX Date: Wed, 29 Mar 2023 08:39:52 +0200 Subject: [PATCH] =?UTF-8?q?FIX:=20ensures=20removing=20a=20reaction=20does?= =?UTF-8?q?n=E2=80=99t=20remove=20others=20(#20869)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../discourse/models/chat-message.js | 15 +++-- .../system/page_objects/chat/chat_channel.rb | 14 ++--- .../chat/spec/system/react_to_message_spec.rb | 56 +++++++++++++++---- 3 files changed, 62 insertions(+), 23 deletions(-) diff --git a/plugins/chat/assets/javascripts/discourse/models/chat-message.js b/plugins/chat/assets/javascripts/discourse/models/chat-message.js index 1187c37f37b..7475c17e1ec 100644 --- a/plugins/chat/assets/javascripts/discourse/models/chat-message.js +++ b/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.users.find((user) => user.id === actor.id) - ); + existingReaction.count = existingReaction.count - 1; + existingReaction.users.removeObject(existingUserReaction); } } } else { diff --git a/plugins/chat/spec/system/page_objects/chat/chat_channel.rb b/plugins/chat/spec/system/page_objects/chat/chat_channel.rb index 1d093778b8a..1d5ce7a615f 100644 --- a/plugins/chat/spec/system/page_objects/chat/chat_channel.rb +++ b/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) - within(message_reactions_list(message)) do - return find("[data-emoji-name=\"#{reaction.emoji}\"]") - end + def find_reaction(message, emoji) + within(message_reactions_list(message)) { return find("[data-emoji-name=\"#{emoji}\"]") } 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) - find_reaction(message, reaction).click + def click_reaction(message, emoji) + find_reaction(message, emoji).click end def open_action_menu diff --git a/plugins/chat/spec/system/react_to_message_spec.rb b/plugins/chat/spec/system/react_to_message_spec.rb index 416f0a5028f..30bf15c9a56 100644 --- a/plugins/chat/spec/system/react_to_message_spec.rb +++ b/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) - expect(channel).to have_reaction(message_1, reaction_1, 2) + channel.click_reaction(message_1, reaction_1.emoji) + 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