From 79a260a6bbcfbfc5d1d38fa32fc9e7b1171ddf22 Mon Sep 17 00:00:00 2001 From: Joffrey JAFFEUX Date: Thu, 15 Jun 2023 11:27:31 +0200 Subject: [PATCH] FIX: allows selection of messages in threads (#22119) This commit fixes the selection of message in threads and also applies various refactorings - improves specs and especially page objects/components - makes the channel/thread panes responsible of the state - adds an animationend modifier - continues to follow the logic of "state" should be displayed as data attributes on component by having a new `data-selected` attribute on chat messages --- .../discourse/components/chat-channel.hbs | 12 +- .../discourse/components/chat-message.hbs | 1 + .../components/chat-selection-manager.js | 133 ------------------ .../discourse/components/chat-thread.hbs | 10 +- .../selection-manager.hbs} | 35 ++--- .../components/chat/selection-manager.js | 103 ++++++++++++++ .../discourse/models/chat-channel.js | 4 + .../discourse/models/chat-thread.js | 4 + .../modifiers/chat/on-animation-end.js | 34 +++++ .../discourse/services/chat-api.js | 13 ++ .../discourse/services/chat-channel-pane.js | 25 ++-- .../discourse/services/chat-thread-pane.js | 17 +-- .../common/chat-selection-manager.scss | 4 +- .../mobile/chat-selection-manager.scss | 2 +- .../system/channel_message_selection_spec.rb | 112 --------------- .../system/move_message_to_channel_spec.rb | 16 +-- .../system/page_objects/chat/chat_channel.rb | 19 ++- .../system/page_objects/chat/chat_thread.rb | 15 +- .../page_objects/chat/components/message.rb | 74 ++++++++-- .../page_objects/chat/components/messages.rb | 28 +++- .../chat/components/selection_management.rb | 72 ++++++++++ .../system/select_message/channel_spec.rb | 52 +++++++ .../spec/system/select_message/thread_spec.rb | 53 +++++++ plugins/chat/spec/system/transcript_spec.rb | 48 ++----- 24 files changed, 501 insertions(+), 385 deletions(-) delete mode 100644 plugins/chat/assets/javascripts/discourse/components/chat-selection-manager.js rename plugins/chat/assets/javascripts/discourse/components/{chat-selection-manager.hbs => chat/selection-manager.hbs} (52%) create mode 100644 plugins/chat/assets/javascripts/discourse/components/chat/selection-manager.js create mode 100644 plugins/chat/assets/javascripts/discourse/modifiers/chat/on-animation-end.js delete mode 100644 plugins/chat/spec/system/channel_message_selection_spec.rb create mode 100644 plugins/chat/spec/system/page_objects/chat/components/selection_management.rb create mode 100644 plugins/chat/spec/system/select_message/channel_spec.rb create mode 100644 plugins/chat/spec/system/select_message/thread_spec.rb diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-channel.hbs b/plugins/chat/assets/javascripts/discourse/components/chat-channel.hbs index 091a65368ec..715ddb54ec9 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-channel.hbs +++ b/plugins/chat/assets/javascripts/discourse/components/chat-channel.hbs @@ -68,14 +68,12 @@ /> {{#if this.pane.selectingMessages}} - {{else}} {{#if (or @channel.isDraft @channel.isFollowing)}} diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-message.hbs b/plugins/chat/assets/javascripts/discourse/components/chat-message.hbs index 79eae17fb63..67d197b600c 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-message.hbs +++ b/plugins/chat/assets/javascripts/discourse/components/chat-message.hbs @@ -30,6 +30,7 @@ }} data-id={{@message.id}} data-thread-id={{@message.thread.id}} + data-selected={{@message.selected}} {{chat/track-message (hash didEnterViewport=(fn @messageDidEnterViewport @message) diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-selection-manager.js b/plugins/chat/assets/javascripts/discourse/components/chat-selection-manager.js deleted file mode 100644 index 2bff51427d7..00000000000 --- a/plugins/chat/assets/javascripts/discourse/components/chat-selection-manager.js +++ /dev/null @@ -1,133 +0,0 @@ -import Component from "@ember/component"; -import { action, computed } from "@ember/object"; -import showModal from "discourse/lib/show-modal"; -import { clipboardCopyAsync } from "discourse/lib/utilities"; -import { getOwner } from "discourse-common/lib/get-owner"; -import { ajax } from "discourse/lib/ajax"; -import { isTesting } from "discourse-common/config/environment"; -import { popupAjaxError } from "discourse/lib/ajax-error"; -import { schedule } from "@ember/runloop"; -import { inject as service } from "@ember/service"; -import getURL from "discourse-common/lib/get-url"; -import { bind } from "discourse-common/utils/decorators"; -import { MESSAGE_CONTEXT_THREAD } from "discourse/plugins/chat/discourse/components/chat-message"; - -export default class ChatSelectionManager extends Component { - @service router; - tagName = ""; - chatChannel = null; - context = null; - selectedMessageIds = null; - chatCopySuccess = false; - showChatCopySuccess = false; - cancelSelecting = null; - - @computed("selectedMessageIds.length") - get anyMessagesSelected() { - return this.selectedMessageIds.length > 0; - } - - @computed("chatChannel.isDirectMessageChannel", "chatChannel.canModerate") - get showMoveMessageButton() { - return ( - this.context !== MESSAGE_CONTEXT_THREAD && - !this.chatChannel.isDirectMessageChannel && - this.chatChannel.canModerate - ); - } - - @bind - async generateQuote() { - const response = await ajax( - getURL(`/chat/${this.chatChannel.id}/quote.json`), - { - data: { message_ids: this.selectedMessageIds }, - type: "POST", - } - ); - - return new Blob([response.markdown], { - type: "text/plain", - }); - } - - @action - openMoveMessageModal() { - showModal("chat-message-move-to-channel-modal").setProperties({ - sourceChannel: this.chatChannel, - selectedMessageIds: this.selectedMessageIds, - }); - } - - @action - async quoteMessages() { - let quoteMarkdown; - - try { - const quoteMarkdownBlob = await this.generateQuote(); - quoteMarkdown = await quoteMarkdownBlob.text(); - } catch (error) { - popupAjaxError(error); - } - - const container = getOwner(this); - const composer = container.lookup("controller:composer"); - const openOpts = {}; - - if (this.chatChannel.isCategoryChannel) { - openOpts.categoryId = this.chatChannel.chatableId; - } - - if (this.site.mobileView) { - // go to the relevant chatable (e.g. category) and open the - // composer to insert text - if (this.chatChannel.chatableUrl) { - this.router.transitionTo(this.chatChannel.chatableUrl); - } - - await composer.focusComposer({ - fallbackToNewTopic: true, - insertText: quoteMarkdown, - openOpts, - }); - } else { - // open the composer and insert text, reply to the current - // topic if there is one, use the active draft if there is one - const topic = container.lookup("controller:topic"); - await composer.focusComposer({ - fallbackToNewTopic: true, - topic: topic?.model, - insertText: quoteMarkdown, - openOpts, - }); - } - } - - @action - async copyMessages() { - try { - this.set("chatCopySuccess", false); - - if (!isTesting()) { - // clipboard API throws errors in tests - await clipboardCopyAsync(this.generateQuote); - this.set("chatCopySuccess", true); - } - - this.set("showChatCopySuccess", true); - - schedule("afterRender", () => { - const element = document.querySelector(".chat-selection-message"); - element?.addEventListener("animationend", () => { - if (this.isDestroying || this.isDestroyed) { - return; - } - - this.set("showChatCopySuccess", false); - }); - }); - } catch (error) { - popupAjaxError(error); - } - } -} diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-thread.hbs b/plugins/chat/assets/javascripts/discourse/components/chat-thread.hbs index 4a5cb3d8bab..24725e3ed9f 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-thread.hbs +++ b/plugins/chat/assets/javascripts/discourse/components/chat-thread.hbs @@ -39,15 +39,7 @@ {{#if this.chatThreadPane.selectingMessages}} - + {{else}} -
+
+
{{#if this.site.desktopView}} {{/if}} - {{#if this.showMoveMessageButton}} + {{#if this.enableMove}} {{/if}} @@ -44,14 +33,16 @@ @icon="times" @class="btn-secondary cancel-btn" @label="chat.selection.cancel" - @title="chat.selection.cancel" - @action={{this.cancelSelecting}} + @action={{@pane.cancelSelecting}} />
- {{#if this.showChatCopySuccess}} -
+ {{#if this.showCopySuccess}} + {{i18n "chat.quote.copy_success"}} -
+ {{/if}}
\ No newline at end of file diff --git a/plugins/chat/assets/javascripts/discourse/components/chat/selection-manager.js b/plugins/chat/assets/javascripts/discourse/components/chat/selection-manager.js new file mode 100644 index 00000000000..86d3ca66f00 --- /dev/null +++ b/plugins/chat/assets/javascripts/discourse/components/chat/selection-manager.js @@ -0,0 +1,103 @@ +import Component from "@glimmer/component"; +import { action } from "@ember/object"; +import showModal from "discourse/lib/show-modal"; +import { clipboardCopyAsync } from "discourse/lib/utilities"; +import { getOwner } from "discourse-common/lib/get-owner"; +import { isTesting } from "discourse-common/config/environment"; +import { popupAjaxError } from "discourse/lib/ajax-error"; +import { inject as service } from "@ember/service"; +import { bind } from "discourse-common/utils/decorators"; +import { tracked } from "@glimmer/tracking"; + +export default class ChatSelectionManager extends Component { + @service("composer") topicComposer; + @service router; + @service site; + @service("chat-api") api; + + @tracked showCopySuccess = false; + + get enableMove() { + return this.args.enableMove ?? false; + } + + get anyMessagesSelected() { + return this.args.pane.selectedMessageIds.length > 0; + } + + @bind + async generateQuote() { + const { markdown } = await this.api.generateQuote( + this.args.pane.channel.id, + this.args.pane.selectedMessageIds + ); + + return new Blob([markdown], { type: "text/plain" }); + } + + @action + openMoveMessageModal() { + showModal("chat-message-move-to-channel-modal").setProperties({ + sourceChannel: this.args.pane.channel, + selectedMessageIds: this.args.pane.selectedMessageIds, + }); + } + + @action + async quoteMessages() { + let quoteMarkdown; + + try { + const quoteMarkdownBlob = await this.generateQuote(); + quoteMarkdown = await quoteMarkdownBlob.text(); + } catch (error) { + popupAjaxError(error); + } + + const openOpts = {}; + if (this.args.pane.channel.isCategoryChannel) { + openOpts.categoryId = this.args.pane.channel.chatableId; + } + + if (this.site.mobileView) { + // go to the relevant chatable (e.g. category) and open the + // composer to insert text + if (this.args.pane.channel.chatableUrl) { + this.router.transitionTo(this.args.pane.channel.chatableUrl); + } + + await this.topicComposer.focusComposer({ + fallbackToNewTopic: true, + insertText: quoteMarkdown, + openOpts, + }); + } else { + // open the composer and insert text, reply to the current + // topic if there is one, use the active draft if there is one + const container = getOwner(this); + const topic = container.lookup("controller:topic"); + await this.topicComposer.focusComposer({ + fallbackToNewTopic: true, + topic: topic?.model, + insertText: quoteMarkdown, + openOpts, + }); + } + } + + @action + async copyMessages() { + try { + this.showCopySuccess = false; + + if (!isTesting()) { + // clipboard API throws errors in tests + await clipboardCopyAsync(this.generateQuote); + } + + this.showCopySuccess = true; + } catch (error) { + popupAjaxError(error); + } + } +} diff --git a/plugins/chat/assets/javascripts/discourse/models/chat-channel.js b/plugins/chat/assets/javascripts/discourse/models/chat-channel.js index 19193446015..289eb54ddd2 100644 --- a/plugins/chat/assets/javascripts/discourse/models/chat-channel.js +++ b/plugins/chat/assets/javascripts/discourse/models/chat-channel.js @@ -336,4 +336,8 @@ export default class ChatChannel { method: "PUT", }); } + + clearSelectedMessages() { + this.selectedMessages.forEach((message) => (message.selected = false)); + } } diff --git a/plugins/chat/assets/javascripts/discourse/models/chat-thread.js b/plugins/chat/assets/javascripts/discourse/models/chat-thread.js index 614ee06e727..070bf01d987 100644 --- a/plugins/chat/assets/javascripts/discourse/models/chat-thread.js +++ b/plugins/chat/assets/javascripts/discourse/models/chat-thread.js @@ -79,6 +79,10 @@ export default class ChatThread { return this.messagesManager.findLastUserMessage(user); } + clearSelectedMessages() { + this.selectedMessages.forEach((message) => (message.selected = false)); + } + get routeModels() { return [...this.channel.routeModels, this.id]; } diff --git a/plugins/chat/assets/javascripts/discourse/modifiers/chat/on-animation-end.js b/plugins/chat/assets/javascripts/discourse/modifiers/chat/on-animation-end.js new file mode 100644 index 00000000000..68325d87285 --- /dev/null +++ b/plugins/chat/assets/javascripts/discourse/modifiers/chat/on-animation-end.js @@ -0,0 +1,34 @@ +import Modifier from "ember-modifier"; +import { registerDestructor } from "@ember/destroyable"; +import { cancel, schedule } from "@ember/runloop"; +import { bind } from "discourse-common/utils/decorators"; + +export default class ChatOnAnimationEnd extends Modifier { + constructor(owner, args) { + super(owner, args); + registerDestructor(this, (instance) => instance.cleanup()); + } + + modify(element, [fn]) { + this.element = element; + this.fn = fn; + + this.handler = schedule("afterRender", () => { + this.element.addEventListener("animationend", this.handleAnimationEnd); + }); + } + + @bind + handleAnimationEnd() { + if (this.isDestroying || this.isDestroyed) { + return; + } + + this.fn?.(this.element); + } + + cleanup() { + cancel(this.handler); + this.element?.removeEventListener("animationend", this.handleAnimationEnd); + } +} diff --git a/plugins/chat/assets/javascripts/discourse/services/chat-api.js b/plugins/chat/assets/javascripts/discourse/services/chat-api.js index 6420cfd5472..cc332084da9 100644 --- a/plugins/chat/assets/javascripts/discourse/services/chat-api.js +++ b/plugins/chat/assets/javascripts/discourse/services/chat-api.js @@ -431,6 +431,19 @@ export default class ChatApi extends Service { return this.#putRequest(`/channels/${channelId}/threads/${threadId}`, data); } + /** + * Generate a quote for a list of messages. + * + * @param {number} channelId - The ID of the channel containing the messages. + * @param {Array} messageIds - The IDs of the messages to quote. + */ + generateQuote(channelId, messageIds) { + return ajax(`/chat/${channelId}/quote`, { + type: "POST", + data: { message_ids: messageIds }, + }); + } + get #basePath() { return "/chat/api"; } diff --git a/plugins/chat/assets/javascripts/discourse/services/chat-channel-pane.js b/plugins/chat/assets/javascripts/discourse/services/chat-channel-pane.js index 7f825d3a655..405a9dca07b 100644 --- a/plugins/chat/assets/javascripts/discourse/services/chat-channel-pane.js +++ b/plugins/chat/assets/javascripts/discourse/services/chat-channel-pane.js @@ -10,21 +10,22 @@ export default class ChatChannelPane extends Service { @tracked lastSelectedMessage = null; @tracked sending = false; - get selectedMessageIds() { - return this.chat.activeChannel?.selectedMessages?.mapBy("id") || []; - } - get channel() { return this.chat.activeChannel; } - @action - cancelSelecting(selectedMessages) { - this.selectingMessages = false; + get selectedMessages() { + return this.channel?.selectedMessages; + } - selectedMessages.forEach((message) => { - message.selected = false; - }); + get selectedMessageIds() { + return this.selectedMessages.mapBy("id"); + } + + @action + cancelSelecting() { + this.selectingMessages = false; + this.channel.clearSelectedMessages(); } @action @@ -32,8 +33,4 @@ export default class ChatChannelPane extends Service { this.lastSelectedMessage = message; this.selectingMessages = true; } - - get lastMessage() { - return this.chat.activeChannel.messages.lastObject; - } } diff --git a/plugins/chat/assets/javascripts/discourse/services/chat-thread-pane.js b/plugins/chat/assets/javascripts/discourse/services/chat-thread-pane.js index 186c5557e7a..df0c743bfc0 100644 --- a/plugins/chat/assets/javascripts/discourse/services/chat-thread-pane.js +++ b/plugins/chat/assets/javascripts/discourse/services/chat-thread-pane.js @@ -5,15 +5,20 @@ export default class ChatThreadPane extends ChatChannelPane { @service chat; @service router; + get thread() { + return this.channel?.activeThread; + } + get isOpened() { return this.router.currentRoute.name === "chat.channel.thread"; } + get selectedMessages() { + return this.thread?.selectedMessages; + } + async close() { - await this.router.transitionTo( - "chat.channel", - ...this.chat.activeChannel.routeModels - ); + await this.router.transitionTo("chat.channel", ...this.channel.routeModels); } async open(thread) { @@ -22,8 +27,4 @@ export default class ChatThreadPane extends ChatChannelPane { ...thread.routeModels ); } - - get selectedMessageIds() { - return this.chat.activeChannel.activeThread.selectedMessages.mapBy("id"); - } } diff --git a/plugins/chat/assets/stylesheets/common/chat-selection-manager.scss b/plugins/chat/assets/stylesheets/common/chat-selection-manager.scss index 959fad11334..2d3e4bab323 100644 --- a/plugins/chat/assets/stylesheets/common/chat-selection-manager.scss +++ b/plugins/chat/assets/stylesheets/common/chat-selection-manager.scss @@ -8,7 +8,7 @@ flex-direction: column; } - .chat-selection-management-buttons { + &__buttons { display: flex; gap: 0.5rem; @@ -18,7 +18,7 @@ } } - .chat-selection-message { + &__copy-success { animation: chat-quote-message-background-fade-highlight 2s ease-out 3s; animation-fill-mode: forwards; background-color: var(--success-low); diff --git a/plugins/chat/assets/stylesheets/mobile/chat-selection-manager.scss b/plugins/chat/assets/stylesheets/mobile/chat-selection-manager.scss index 9774a36e294..82d69177b03 100644 --- a/plugins/chat/assets/stylesheets/mobile/chat-selection-manager.scss +++ b/plugins/chat/assets/stylesheets/mobile/chat-selection-manager.scss @@ -1,5 +1,5 @@ .chat-selection-management { - .chat-selection-management-buttons { + &__buttons { display: flex; flex-direction: column; width: 100%; diff --git a/plugins/chat/spec/system/channel_message_selection_spec.rb b/plugins/chat/spec/system/channel_message_selection_spec.rb deleted file mode 100644 index 3a3da9c5992..00000000000 --- a/plugins/chat/spec/system/channel_message_selection_spec.rb +++ /dev/null @@ -1,112 +0,0 @@ -# frozen_string_literal: true - -RSpec.describe "Channel message selection", type: :system do - fab!(:current_user) { Fabricate(:user) } - fab!(:channel_1) { Fabricate(:chat_channel) } - fab!(:message_1) { Fabricate(:chat_message, chat_channel: channel_1) } - fab!(:message_2) { Fabricate(:chat_message, chat_channel: channel_1) } - fab!(:message_3) { Fabricate(:chat_message, chat_channel: channel_1) } - - 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 - channel_1.add(current_user) - sign_in(current_user) - end - - it "can select multiple messages" do - chat_page.visit_channel(channel_1) - channel_page.select_message(message_1) - - expect(page).to have_css(".chat-selection-management") - - 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_page.visit_channel(channel_1) - channel_page.select_message(message_1) - - expect(page).to have_css(".chat-selection-management") - - 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 - - context "when visiting another channel" do - fab!(:channel_2) { Fabricate(:chat_channel) } - - before { channel_2.add(current_user) } - - it "resets message selection" do - chat_page.visit_channel(channel_1) - channel_page.select_message(message_1) - - expect(page).to have_css(".chat-selection-management") - - 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/move_message_to_channel_spec.rb b/plugins/chat/spec/system/move_message_to_channel_spec.rb index 3ee9894e024..73d9c665831 100644 --- a/plugins/chat/spec/system/move_message_to_channel_spec.rb +++ b/plugins/chat/spec/system/move_message_to_channel_spec.rb @@ -18,9 +18,9 @@ RSpec.describe "Move message to channel", type: :system do it "is not available" do chat_page.visit_channel(channel_1) - channel_page.select_message(message_1) + channel_page.messages.select(message_1) - expect(page).to have_no_content(I18n.t("js.chat.selection.move_selection_to_channel")) + expect(channel_page.selection_management).to have_no_move_action end context "when can moderate channel" do @@ -37,9 +37,9 @@ RSpec.describe "Move message to channel", type: :system do it "is available" do chat_page.visit_channel(channel_1) - channel_page.select_message(message_1) + channel_page.messages.select(message_1) - expect(page).to have_content(I18n.t("js.chat.selection.move_selection_to_channel")) + expect(channel_page.selection_management).to have_move_action end end end @@ -57,9 +57,9 @@ RSpec.describe "Move message to channel", type: :system do it "is not available" do chat_page.visit_channel(dm_channel_1) - channel_page.select_message(message_1) + channel_page.messages.select(message_1) - expect(page).to have_no_content(I18n.t("js.chat.selection.move_selection_to_channel")) + expect(channel_page.selection_management).to have_no_move_action end end @@ -77,8 +77,8 @@ RSpec.describe "Move message to channel", type: :system do it "moves the message" do chat_page.visit_channel(channel_1) - channel_page.select_message(message_1) - click_button(I18n.t("js.chat.selection.move_selection_to_channel")) + channel_page.messages.select(message_1) + channel_page.selection_management.move find(".chat-move-message-channel-chooser").click find("[data-value='#{channel_2.id}']").click click_button(I18n.t("js.chat.move_to_channel.confirm_move")) 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 9514c23c253..c2a53a8c488 100644 --- a/plugins/chat/spec/system/page_objects/chat/chat_channel.rb +++ b/plugins/chat/spec/system/page_objects/chat/chat_channel.rb @@ -11,6 +11,15 @@ module PageObjects @messages ||= PageObjects::Components::Chat::Messages.new(".chat-channel") end + def selection_management + @selection_management ||= + PageObjects::Components::Chat::SelectionManagement.new(".chat-channel") + end + + def has_selected_messages?(*messages) + self.messages.has_selected_messages?(*messages) + end + def replying_to?(message) find(".chat-channel .chat-reply", text: message.message) end @@ -76,16 +85,6 @@ module PageObjects end end - def select_message(message) - if page.has_css?("html.mobile-view", wait: 0) - click_message_action_mobile(message, "select") - else - hover_message(message) - click_more_button - find("[data-value='select']").click - end - end - def click_more_button find(".more-buttons").click 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 8f4d0d0bdf2..a9995b42ec9 100644 --- a/plugins/chat/spec/system/page_objects/chat/chat_thread.rb +++ b/plugins/chat/spec/system/page_objects/chat/chat_thread.rb @@ -20,6 +20,15 @@ module PageObjects @header ||= PageObjects::Components::Chat::ThreadHeader.new(".chat-thread") end + def selection_management + @selection_management ||= + PageObjects::Components::Chat::SelectionManagement.new(".chat-channel") + end + + def has_selected_messages?(*messages) + self.messages.has_selected_messages?(*messages) + end + def close header.find(".chat-thread__close").click end @@ -110,12 +119,6 @@ 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", diff --git a/plugins/chat/spec/system/page_objects/chat/components/message.rb b/plugins/chat/spec/system/page_objects/chat/components/message.rb index 147693ef229..1a9e89766f7 100644 --- a/plugins/chat/spec/system/page_objects/chat/components/message.rb +++ b/plugins/chat/spec/system/page_objects/chat/components/message.rb @@ -5,6 +5,7 @@ module PageObjects module Chat class Message < PageObjects::Components::Base attr_reader :context + attr_reader :component SELECTOR = ".chat-message-container" @@ -16,31 +17,80 @@ module PageObjects exists?(**args, does_not_exist: true) end - def exists?(**args) - text = args[:text] + def hover + message_by_id(message.id).hover + end - selectors = SELECTOR - selectors += "[data-id=\"#{args[:id]}\"]" if args[:id] - selectors += ".is-persisted" if args[:persisted] - selectors += ".is-staged" if args[:staged] + def select(shift: false) + if component[:class].include?("selecting-message") + message_selector = component.find(".chat-message-selector") + if shift + message_selector.click(:shift) + else + message_selector.click + end - if args[:deleted] - selectors += ".is-deleted" - text = I18n.t("js.chat.deleted", count: args[:deleted]) + return end + if page.has_css?("html.mobile-view", wait: 0) + component.click(delay: 0.6) + page.find(".chat-message-actions [data-id=\"select\"]").click + else + component.hover + click_more_button + page.find("[data-value='select']").click + end + end + + def find(**args) + selector = build_selector(**args) + text = args[:text] + text = I18n.t("js.chat.deleted", count: args[:deleted]) if args[:deleted] + + if text + @component = + find(context).find("#{selector} .chat-message-text", text: /#{Regexp.escape(text)}/) + else + @component = page.find(context).find(selector) + end + + self + end + + def exists?(**args) + selector = build_selector(**args) + text = args[:text] + text = I18n.t("js.chat.deleted", count: args[:deleted]) if args[:deleted] + selector_method = args[:does_not_exist] ? :has_no_selector? : :has_selector? if text - find(context).send( + page.find(context).send( selector_method, - selectors + " " + ".chat-message-text", + selector + " " + ".chat-message-text", text: /#{Regexp.escape(text)}/, ) else - find(context).send(selector_method, selectors) + page.find(context).send(selector_method, selector) end end + + private + + def click_more_button + page.find(".more-buttons").click + end + + def build_selector(**args) + selector = SELECTOR + selector += "[data-id=\"#{args[:id]}\"]" if args[:id] + selector += "[data-selected]" if args[:selected] + selector += ".is-persisted" if args[:persisted] + selector += ".is-staged" if args[:staged] + selector += ".is-deleted" if args[:deleted] + selector + end end end end diff --git a/plugins/chat/spec/system/page_objects/chat/components/messages.rb b/plugins/chat/spec/system/page_objects/chat/components/messages.rb index a496369a5db..a823eab7590 100644 --- a/plugins/chat/spec/system/page_objects/chat/components/messages.rb +++ b/plugins/chat/spec/system/page_objects/chat/components/messages.rb @@ -13,11 +13,23 @@ module PageObjects end def component - find(context) + page.find(context) end - def message - PageObjects::Components::Chat::Message.new(context + " " + SELECTOR) + def select(args) + find(args).select + end + + def shift_select(args) + find(args).select(shift: true) + end + + def find(args) + if args.is_a?(Hash) + message.find(**args) + else + message.find(id: args.id) + end end def has_message?(**args) @@ -27,6 +39,16 @@ module PageObjects def has_no_message?(**args) message.does_not_exist?(**args) end + + def has_selected_messages?(*messages) + messages.all? { |message| has_message?(id: message.id, selected: true) } + end + + private + + def message + PageObjects::Components::Chat::Message.new("#{context} #{SELECTOR}") + end end end end diff --git a/plugins/chat/spec/system/page_objects/chat/components/selection_management.rb b/plugins/chat/spec/system/page_objects/chat/components/selection_management.rb new file mode 100644 index 00000000000..3d89b6e6696 --- /dev/null +++ b/plugins/chat/spec/system/page_objects/chat/components/selection_management.rb @@ -0,0 +1,72 @@ +# frozen_string_literal: true + +module PageObjects + module Components + module Chat + class SelectionManagement < PageObjects::Components::Base + attr_reader :context + + SELECTOR = ".chat-selection-management" + + def initialize(context) + @context = context + end + + def visible? + find(context).has_css?(SELECTOR) + end + + def not_visible? + find(context).has_no_css?(SELECTOR) + end + + def has_no_move_action? + has_no_button?(selector_for("move")) + end + + def has_move_action? + has_button?(selector_for("move")) + end + + def component + find(context).find(SELECTOR) + end + + def cancel + click_button("cancel") + end + + def quote + click_button("quote") + end + + def copy + click_button("copy") + end + + def move + click_button("move") + end + + private + + def selector_for(action) + case action + when "quote" + "chat-quote-btn" + when "copy" + "chat-copy-btn" + when "cancel" + "chat-cancel-selection-btn" + when "move" + "chat-move-to-channel-btn" + end + end + + def click_button(action) + find_button(selector_for(action), disabled: false).click + end + end + end + end +end diff --git a/plugins/chat/spec/system/select_message/channel_spec.rb b/plugins/chat/spec/system/select_message/channel_spec.rb new file mode 100644 index 00000000000..6eb83d7c6e4 --- /dev/null +++ b/plugins/chat/spec/system/select_message/channel_spec.rb @@ -0,0 +1,52 @@ +# frozen_string_literal: true + +RSpec.describe "Chat | Select message | channel", type: :system do + fab!(:current_user) { Fabricate(:user) } + fab!(:channel_1) { Fabricate(:chat_channel) } + fab!(:message_1) { Fabricate(:chat_message, chat_channel: channel_1) } + fab!(:message_2) { Fabricate(:chat_message, chat_channel: channel_1) } + fab!(:message_3) { Fabricate(:chat_message, chat_channel: channel_1) } + + let(:chat_page) { PageObjects::Pages::Chat.new } + let(:channel_page) { PageObjects::Pages::ChatChannel.new } + + before do + chat_system_bootstrap + channel_1.add(current_user) + sign_in(current_user) + end + + it "can select multiple messages" do + chat_page.visit_channel(channel_1) + + channel_page.messages.select(message_1) + channel_page.messages.select(message_2) + + expect(channel_page).to have_selected_messages(message_1, message_2) + end + + it "can shift + click to select messages between the first and last" do + chat_page.visit_channel(channel_1) + channel_page.messages.select(message_1) + channel_page.messages.shift_select(message_3) + + expect(channel_page).to have_selected_messages(message_1, message_2, message_3) + end + + context "when visiting another channel" do + fab!(:channel_2) { Fabricate(:chat_channel) } + + before { channel_2.add(current_user) } + + it "resets message selection" do + chat_page.visit_channel(channel_1) + channel_page.messages.select(message_1) + + expect(channel_page.selection_management).to be_visible + + chat_page.visit_channel(channel_2) + + expect(channel_page.selection_management).to be_not_visible + end + end +end diff --git a/plugins/chat/spec/system/select_message/thread_spec.rb b/plugins/chat/spec/system/select_message/thread_spec.rb new file mode 100644 index 00000000000..31f086299ef --- /dev/null +++ b/plugins/chat/spec/system/select_message/thread_spec.rb @@ -0,0 +1,53 @@ +# frozen_string_literal: true + +RSpec.describe "Chat | Select message | thread", type: :system do + fab!(:current_user) { Fabricate(:user) } + fab!(:channel_1) { Fabricate(:chat_channel, threading_enabled: true) } + fab!(:original_message) { Fabricate(:chat_message, chat_channel: channel_1) } + + let(:chat_page) { PageObjects::Pages::Chat.new } + let(:channel_page) { PageObjects::Pages::ChatChannel.new } + let(:thread_page) { PageObjects::Pages::ChatThread.new } + + before do + SiteSetting.enable_experimental_chat_threaded_discussions = true + chat_system_bootstrap + channel_1.add(current_user) + sign_in(current_user) + end + + fab!(:thread_message_1) do + Fabricate(:chat_message, chat_channel: channel_1, in_reply_to: original_message) + end + fab!(:thread_message_2) do + Fabricate(:chat_message, chat_channel: channel_1, in_reply_to: original_message) + end + fab!(:thread_message_3) do + Fabricate(:chat_message, chat_channel: channel_1, in_reply_to: original_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.messages.select(thread_message_1) + thread_page.messages.select(thread_message_2) + + expect(thread_page).to have_selected_messages(thread_message_1, thread_message_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.messages.select(thread_message_1) + thread_page.messages.shift_select(thread_message_3) + + expect(thread_page).to have_selected_messages( + thread_message_1, + thread_message_2, + thread_message_3, + ) + end +end diff --git a/plugins/chat/spec/system/transcript_spec.rb b/plugins/chat/spec/system/transcript_spec.rb index 05a00e75f2b..93d30c37451 100644 --- a/plugins/chat/spec/system/transcript_spec.rb +++ b/plugins/chat/spec/system/transcript_spec.rb @@ -7,7 +7,7 @@ RSpec.describe "Quoting chat message transcripts", type: :system do let(:cdp) { PageObjects::CDP.new } let(:chat_page) { PageObjects::Pages::Chat.new } - let(:chat_channel_page) { PageObjects::Pages::ChatChannel.new } + let(:channel_page) { PageObjects::Pages::ChatChannel.new } let(:topic_page) { PageObjects::Pages::Topic.new } before do @@ -16,38 +16,11 @@ RSpec.describe "Quoting chat message transcripts", type: :system do sign_in(current_user) end - def select_message_desktop(message) - if page.has_css?(".chat-message-container.selecting-messages", wait: 0) - chat_channel_page.message_by_id(message.id).find(".chat-message-selector").click - else - chat_channel_page.message_by_id(message.id).hover - expect(page).to have_css(".chat-message-actions .more-buttons") - find(".chat-message-actions .more-buttons").click - find(".select-kit-row[data-value=\"select\"]").click - end - end - - def click_selection_button(button) - selector = - case button - when "quote" - "chat-quote-btn" - when "copy" - "chat-copy-btn" - when "cancel" - "chat-cancel-selection-btn" - when "move" - "chat-move-to-channel-btn" - end - find_button(selector, disabled: false, wait: 5).click - end - def copy_messages_to_clipboard(messages) messages = Array.wrap(messages) - messages.each { |message| select_message_desktop(message) } - expect(chat_channel_page).to have_selection_management - click_selection_button("copy") - expect(page).to have_selector(".chat-copy-success") + messages.each { |message| channel_page.messages.select(message) } + channel_page.selection_management.copy + expect(page).to have_selector(".chat-selection-management__copy-success") clip_text = cdp.read_clipboard expect(clip_text.chomp).to eq(generate_transcript(messages, current_user)) clip_text @@ -140,8 +113,8 @@ RSpec.describe "Quoting chat message transcripts", type: :system do chat_page.visit_channel(chat_channel_1) clip_text = copy_messages_to_clipboard(message_1) - click_selection_button("cancel") - chat_channel_page.send_message(clip_text) + channel_page.selection_management.cancel + channel_page.send_message(clip_text) expect(page).to have_selector(".chat-message", count: 2) expect(page).to have_css(".chat-transcript") @@ -155,9 +128,8 @@ RSpec.describe "Quoting chat message transcripts", type: :system do it "opens the topic composer with correct state" do chat_page.visit_channel(chat_channel_1) - - select_message_desktop(message_1) - click_selection_button("quote") + channel_page.messages.select(message_1) + channel_page.selection_management.quote expect(topic_page).to have_expanded_composer expect(topic_page).to have_composer_content(generate_transcript(message_1, current_user)) @@ -181,8 +153,8 @@ RSpec.describe "Quoting chat message transcripts", type: :system do it "first navigates to the channel's category before opening the topic composer with the quote prefilled", mobile: true do chat_page.visit_channel(chat_channel_1) - chat_channel_page.select_message(message_1) - click_selection_button("quote") + channel_page.messages.select(message_1) + channel_page.selection_management.quote expect(topic_page).to have_expanded_composer expect(topic_page).to have_composer_content(generate_transcript(message_1, current_user))