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.

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 manager = this.message.manager;
-    const lastSelectedIndex = channel.findIndexOfMessage(
+    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;
     }
   }
 

plugins/chat/assets/javascripts/discourse/routes/chat-channel-thread.js

@@ -20,12 +20,12 @@ export default class ChatChannelThread extends DiscourseRoute {
       });
   }
 
-  deactivate() {
+  afterModel(model) {
-    this.chatChannelThreadPane.close();
+    this.chatChannelThreadPane.open(model);
   }
 
-  activate() {
+  deactivate() {
-    this.chatChannelThreadPane.open(this.currentModel);
+    this.chatChannelThreadPane.close();
   }
 
   beforeModel(transition) {

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(:chat_page) { PageObjects::Pages::Chat.new }
-  let(:channel) { PageObjects::Pages::ChatChannel.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)
+    chat_page.visit_channel(channel_1)
-    channel.select_message(message_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)
+    chat_page.visit_channel(channel_1)
-    channel.select_message(message_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)
+      chat_page.visit_channel(channel_1)
-      channel.select_message(message_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

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",