From 92bb845db2f6996cccd42fc96ffa7727c3af6912 Mon Sep 17 00:00:00 2001 From: Joffrey JAFFEUX Date: Thu, 11 May 2023 17:52:53 +0200 Subject: [PATCH] FIX: messages selection with shift + click (#21506) This commit fixes the shift+click multi selection in threads. We were not correctly using the manager of the message and would attempt to find messages in the channel instead of the thread. The `activeThread` was also not correctly set sometimes. Also adds tests for message selection in threads. --- .../discourse/lib/chat-message-interactor.js | 8 +- .../discourse/routes/chat-channel-thread.js | 8 +- .../system/channel_message_selection_spec.rb | 79 ++++++++++++++++--- .../system/page_objects/chat/chat_thread.rb | 6 ++ 4 files changed, 82 insertions(+), 19 deletions(-) diff --git a/plugins/chat/assets/javascripts/discourse/lib/chat-message-interactor.js b/plugins/chat/assets/javascripts/discourse/lib/chat-message-interactor.js index ea67d8b0f59..f838341b11e 100644 --- a/plugins/chat/assets/javascripts/discourse/lib/chat-message-interactor.js +++ b/plugins/chat/assets/javascripts/discourse/lib/chat-message-interactor.js @@ -213,17 +213,17 @@ export default class ChatMessageInteractor { } bulkSelect(checked) { - const channel = this.message.channel; - const lastSelectedIndex = channel.findIndexOfMessage( + const manager = this.message.manager; + const lastSelectedIndex = manager.findIndexOfMessage( this.pane.lastSelectedMessage.id ); - const newlySelectedIndex = channel.findIndexOfMessage(this.message.id); + const newlySelectedIndex = manager.findIndexOfMessage(this.message.id); const sortedIndices = [lastSelectedIndex, newlySelectedIndex].sort( (a, b) => a - b ); for (let i = sortedIndices[0]; i <= sortedIndices[1]; i++) { - channel.messages[i].selected = checked; + manager.messages[i].selected = checked; } } diff --git a/plugins/chat/assets/javascripts/discourse/routes/chat-channel-thread.js b/plugins/chat/assets/javascripts/discourse/routes/chat-channel-thread.js index 755b5cd291f..ab772ab4675 100644 --- a/plugins/chat/assets/javascripts/discourse/routes/chat-channel-thread.js +++ b/plugins/chat/assets/javascripts/discourse/routes/chat-channel-thread.js @@ -20,12 +20,12 @@ export default class ChatChannelThread extends DiscourseRoute { }); } - deactivate() { - this.chatChannelThreadPane.close(); + afterModel(model) { + this.chatChannelThreadPane.open(model); } - activate() { - this.chatChannelThreadPane.open(this.currentModel); + deactivate() { + this.chatChannelThreadPane.close(); } beforeModel(transition) { diff --git a/plugins/chat/spec/system/channel_message_selection_spec.rb b/plugins/chat/spec/system/channel_message_selection_spec.rb index 6c5e3752aae..d4bd6b30ae0 100644 --- a/plugins/chat/spec/system/channel_message_selection_spec.rb +++ b/plugins/chat/spec/system/channel_message_selection_spec.rb @@ -7,8 +7,9 @@ RSpec.describe "Channel message selection", type: :system, js: true do fab!(:message_2) { Fabricate(:chat_message, chat_channel: channel_1) } fab!(:message_3) { Fabricate(:chat_message, chat_channel: channel_1) } - let(:chat) { PageObjects::Pages::Chat.new } - let(:channel) { PageObjects::Pages::ChatChannel.new } + let(:chat_page) { PageObjects::Pages::Chat.new } + let(:channel_page) { PageObjects::Pages::ChatChannel.new } + let(:thread_page) { PageObjects::Pages::ChatThread.new } before do chat_system_bootstrap @@ -17,22 +18,22 @@ RSpec.describe "Channel message selection", type: :system, js: true do end it "can select multiple messages" do - chat.visit_channel(channel_1) - channel.select_message(message_1) + chat_page.visit_channel(channel_1) + channel_page.select_message(message_1) expect(page).to have_css(".chat-selection-management") - channel.message_by_id(message_2.id).find(".chat-message-selector").click + channel_page.message_by_id(message_2.id).find(".chat-message-selector").click expect(page).to have_css(".chat-message-selector:checked", count: 2) end it "can shift + click to select messages between the first and last" do - chat.visit_channel(channel_1) - channel.select_message(message_1) + chat_page.visit_channel(channel_1) + channel_page.select_message(message_1) expect(page).to have_css(".chat-selection-management") - channel.message_by_id(message_3.id).find(".chat-message-selector").click(:shift) + channel_page.message_by_id(message_3.id).find(".chat-message-selector").click(:shift) expect(page).to have_css(".chat-message-selector:checked", count: 3) end @@ -42,14 +43,70 @@ RSpec.describe "Channel message selection", type: :system, js: true do before { channel_2.add(current_user) } it "resets message selection" do - chat.visit_channel(channel_1) - channel.select_message(message_1) + chat_page.visit_channel(channel_1) + channel_page.select_message(message_1) expect(page).to have_css(".chat-selection-management") - chat.visit_channel(channel_2) + chat_page.visit_channel(channel_2) expect(page).to have_no_css(".chat-selection-management") end end + + context "when in a thread" do + fab!(:thread_message_1) do + Chat::MessageCreator.create( + chat_channel: channel_1, + in_reply_to_id: message_1.id, + user: Fabricate(:user), + content: Faker::Lorem.paragraph, + ).chat_message + end + + fab!(:thread_message_2) do + Chat::MessageCreator.create( + chat_channel: channel_1, + in_reply_to_id: message_1.id, + user: Fabricate(:user), + content: Faker::Lorem.paragraph, + ).chat_message + end + + fab!(:thread_message_3) do + Chat::MessageCreator.create( + chat_channel: channel_1, + in_reply_to_id: message_1.id, + user: Fabricate(:user), + content: Faker::Lorem.paragraph, + ).chat_message + end + + before do + SiteSetting.enable_experimental_chat_threaded_discussions = true + channel_1.update!(threading_enabled: true) + end + + it "can select multiple messages" do + chat_page.visit_thread(thread_message_1.thread) + thread_page.select_message(thread_message_1) + + expect(thread_page).to have_css(".chat-selection-management") + + thread_page.message_by_id(thread_message_2.id).find(".chat-message-selector").click + + expect(thread_page).to have_css(".chat-message-selector:checked", count: 2) + end + + it "can shift + click to select messages between the first and last" do + chat_page.visit_thread(thread_message_1.thread) + thread_page.select_message(thread_message_1) + + expect(thread_page).to have_css(".chat-selection-management") + + thread_page.message_by_id(thread_message_3.id).find(".chat-message-selector").click(:shift) + + expect(page).to have_css(".chat-message-selector:checked", count: 3) + end + end end diff --git a/plugins/chat/spec/system/page_objects/chat/chat_thread.rb b/plugins/chat/spec/system/page_objects/chat/chat_thread.rb index d53b743ab00..27ebd657504 100644 --- a/plugins/chat/spec/system/page_objects/chat/chat_thread.rb +++ b/plugins/chat/spec/system/page_objects/chat/chat_thread.rb @@ -88,6 +88,12 @@ module PageObjects ".chat-thread .chat-messages-container .chat-message-container[data-id=\"#{id}\"]" end + def select_message(message) + hover_message(message) + click_more_button + find("[data-value='select']").click + end + def has_deleted_message?(message, count: 1) has_css?( ".chat-thread .chat-message-container[data-id=\"#{message.id}\"] .chat-message-deleted",