From 4de1d3952b46dc25e2e4d075cadb42933df880d1 Mon Sep 17 00:00:00 2001 From: Joffrey JAFFEUX Date: Wed, 24 May 2023 15:36:46 +0200 Subject: [PATCH] FIX: improves draft for channels (#21724) This commit attempts to correctly change draft when the channel changes. It moves responsibility to the composer instead of the channel. A new service `chatDraftsManager` is being introduced here to allow finer control and pave the way for future thread draft support. These changes also now allow an editing message to be stored as a draft. --- .../discourse/components/chat-channel.js | 9 +- .../chat-composer-message-details.hbs | 6 +- .../discourse/components/chat-composer.hbs | 2 + .../discourse/components/chat-composer.js | 34 ++-- .../components/chat/composer/channel.js | 3 + .../discourse/models/chat-channel.js | 1 - .../discourse/models/chat-message.js | 12 +- .../services/chat-channel-composer.js | 8 + .../services/chat-channel-thread-composer.js | 2 +- .../discourse/services/chat-composer.js | 18 +- .../discourse/services/chat-drafts-manager.js | 25 +++ .../javascripts/discourse/services/chat.js | 24 +-- .../spec/system/chat_composer_draft_spec.rb | 135 ++++++++++++++ .../chat/spec/system/chat_composer_spec.rb | 165 +++++------------- .../page_objects/chat/components/composer.rb | 8 + .../components/composer_message_details.rb | 10 +- .../unit/services/chat-drafts-manager-test.js | 53 ++++++ 17 files changed, 338 insertions(+), 177 deletions(-) create mode 100644 plugins/chat/assets/javascripts/discourse/services/chat-drafts-manager.js create mode 100644 plugins/chat/spec/system/chat_composer_draft_spec.rb create mode 100644 plugins/chat/test/javascripts/unit/services/chat-drafts-manager-test.js diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-channel.js b/plugins/chat/assets/javascripts/discourse/components/chat-channel.js index 6f558a3902e..ce2ccc9aa68 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-channel.js +++ b/plugins/chat/assets/javascripts/discourse/components/chat-channel.js @@ -43,6 +43,7 @@ export default class ChatLivePane extends Component { @service appEvents; @service messageBus; @service site; + @service chatDraftsManager; @tracked loading = false; @tracked loadingMorePast = false; @@ -116,11 +117,6 @@ export default class ChatLivePane extends Component { if (this._loadedChannelId !== this.args.channel?.id) { this.unsubscribeToUpdates(this._loadedChannelId); this.chatChannelPane.selectingMessages = false; - - if (this.args.channel.draft) { - this.chatChannelComposer.message = this.args.channel.draft; - } - this._loadedChannelId = this.args.channel?.id; } @@ -642,6 +638,7 @@ export default class ChatLivePane extends Component { .editMessage(this.args.channel.id, message.id, data) .catch(popupAjaxError) .finally(() => { + this.chatDraftsManager.remove({ channelId: this.args.channel.id }); this.chatChannelPane.sending = false; }); } @@ -704,7 +701,7 @@ export default class ChatLivePane extends Component { return; } - this.args.channel.draft = null; + this.chatDraftsManager.remove({ channelId: this.args.channel.id }); this.chatChannelPane.sending = false; }); } diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-composer-message-details.hbs b/plugins/chat/assets/javascripts/discourse/components/chat-composer-message-details.hbs index d0426eede5f..20286950731 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-composer-message-details.hbs +++ b/plugins/chat/assets/javascripts/discourse/components/chat-composer-message-details.hbs @@ -1,4 +1,8 @@ -
+
{{d-icon (if @message.editing "pencil-alt" "reply")}} diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-composer.hbs b/plugins/chat/assets/javascripts/discourse/components/chat-composer.hbs index 7540dfed5ba..ff45d3db85c 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-composer.hbs +++ b/plugins/chat/assets/javascripts/discourse/components/chat-composer.hbs @@ -26,6 +26,8 @@ {{did-update this.didUpdateMessage this.currentMessage}} {{did-update this.didUpdateInReplyTo this.currentMessage.inReplyTo}} {{did-insert this.setup}} + {{did-insert this.didUpdateChannel}} + {{did-update this.didUpdateChannel @channel.id}} {{will-destroy this.teardown}} {{will-destroy this.cancelPersistDraft}} > diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-composer.js b/plugins/chat/assets/javascripts/discourse/components/chat-composer.js index bcd9cab45dc..771efb0053d 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-composer.js +++ b/plugins/chat/assets/javascripts/discourse/components/chat-composer.js @@ -17,8 +17,8 @@ import I18n from "I18n"; import { translations } from "pretty-text/emoji/data"; import { setupHashtagAutocomplete } from "discourse/lib/hashtag-autocomplete"; import { isEmpty, isPresent } from "@ember/utils"; -import ChatMessage from "discourse/plugins/chat/discourse/models/chat-message"; import { Promise } from "rsvp"; +import ChatMessage from "discourse/plugins/chat/discourse/models/chat-message"; import User from "discourse/models/user"; export default class ChatComposer extends Component { @@ -33,6 +33,7 @@ export default class ChatComposer extends Component { @service chatEmojiPickerManager; @service currentUser; @service chatApi; + @service chatDraftsManager; @tracked isFocused = false; @tracked inProgressUploadsCount = 0; @@ -78,15 +79,8 @@ export default class ChatComposer extends Component { } @action - sendMessage(raw) { - const message = ChatMessage.createDraftMessage(this.args.channel, { - user: this.currentUser, - message: raw, - }); - - this.args.onSendMessage(message); - - return Promise.resolve(); + sendMessage() { + this.args.onSendMessage(this.currentMessage); } @action @@ -107,13 +101,15 @@ export default class ChatComposer extends Component { @action didUpdateMessage() { - cancel(this._persistHandler); + this.cancelPersistDraft(); this.textareaInteractor.value = this.currentMessage.message || ""; this.textareaInteractor.focus({ refreshHeight: true }); + this.persistDraft(); } @action didUpdateInReplyTo() { + this.cancelPersistDraft(); this.textareaInteractor.focus({ ensureAtEnd: true, refreshHeight: true }); this.persistDraft(); } @@ -148,11 +144,21 @@ export default class ChatComposer extends Component { ); } + @action + didUpdateChannel() { + if (!this.args.channel) { + this.composer.message = null; + } + + this.composer.message = + this.chatDraftsManager.get({ channelId: this.args.channel.id }) || + ChatMessage.createDraftMessage(this.args.channel, { + user: this.currentUser, + }); + } + @action setup() { - this.composer.message = ChatMessage.createDraftMessage(this.args.channel, { - user: this.currentUser, - }); this.appEvents.on("chat:modify-selection", this, "modifySelection"); this.appEvents.on( "chat:open-insert-link-modal", diff --git a/plugins/chat/assets/javascripts/discourse/components/chat/composer/channel.js b/plugins/chat/assets/javascripts/discourse/components/chat/composer/channel.js index 26f5ea2ebd3..7ec57d2a1f3 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat/composer/channel.js +++ b/plugins/chat/assets/javascripts/discourse/components/chat/composer/channel.js @@ -7,6 +7,7 @@ import { action } from "@ember/object"; export default class ChatComposerChannel extends ChatComposer { @service("chat-channel-composer") composer; @service("chat-channel-pane") pane; + @service chatDraftsManager; context = "channel"; @@ -23,6 +24,8 @@ export default class ChatComposerChannel extends ChatComposer { return; } + this.chatDraftsManager.add(this.currentMessage); + this._persistHandler = discourseDebounce( this, this._debouncedPersistDraft, diff --git a/plugins/chat/assets/javascripts/discourse/models/chat-channel.js b/plugins/chat/assets/javascripts/discourse/models/chat-channel.js index 76302c66ff9..a5728bf651c 100644 --- a/plugins/chat/assets/javascripts/discourse/models/chat-channel.js +++ b/plugins/chat/assets/javascripts/discourse/models/chat-channel.js @@ -81,7 +81,6 @@ export default class ChatChannel { @tracked canFlag; @tracked canModerate; @tracked userSilenced; - @tracked draft = null; @tracked meta; @tracked chatableType; @tracked chatableUrl; diff --git a/plugins/chat/assets/javascripts/discourse/models/chat-message.js b/plugins/chat/assets/javascripts/discourse/models/chat-message.js index f462688f679..5f88728d716 100644 --- a/plugins/chat/assets/javascripts/discourse/models/chat-message.js +++ b/plugins/chat/assets/javascripts/discourse/models/chat-message.js @@ -138,8 +138,12 @@ export default class ChatMessage { } cook() { + const site = getOwner(this).lookup("service:site"); + next(() => { - const site = getOwner(this).lookup("service:site"); + if (this.isDestroyed || this.isDestroying) { + return; + } const markdownOptions = { featuresOverride: @@ -248,6 +252,12 @@ export default class ChatMessage { }; } + if (this.editing) { + data.editing = true; + data.id = this.id; + data.excerpt = this.excerpt; + } + return JSON.stringify(data); } diff --git a/plugins/chat/assets/javascripts/discourse/services/chat-channel-composer.js b/plugins/chat/assets/javascripts/discourse/services/chat-channel-composer.js index 4be7139cd56..98506f32976 100644 --- a/plugins/chat/assets/javascripts/discourse/services/chat-channel-composer.js +++ b/plugins/chat/assets/javascripts/discourse/services/chat-channel-composer.js @@ -1,11 +1,19 @@ import { inject as service } from "@ember/service"; import { action } from "@ember/object"; import ChatComposer from "./chat-composer"; +import ChatMessage from "discourse/plugins/chat/discourse/models/chat-message"; export default class ChatChannelComposer extends ChatComposer { @service chat; @service router; + @action + reset(channel) { + this.message = ChatMessage.createDraftMessage(channel, { + user: this.currentUser, + }); + } + @action replyTo(message) { this.chat.activeMessage = null; diff --git a/plugins/chat/assets/javascripts/discourse/services/chat-channel-thread-composer.js b/plugins/chat/assets/javascripts/discourse/services/chat-channel-thread-composer.js index e6145df394b..d9b08e6e885 100644 --- a/plugins/chat/assets/javascripts/discourse/services/chat-channel-thread-composer.js +++ b/plugins/chat/assets/javascripts/discourse/services/chat-channel-thread-composer.js @@ -7,7 +7,7 @@ export default class ChatChannelThreadComposer extends ChatComposer { reset(channel, thread) { this.message = ChatMessage.createDraftMessage(channel, { user: this.currentUser, + thread, }); - this.message.thread = thread; } } diff --git a/plugins/chat/assets/javascripts/discourse/services/chat-composer.js b/plugins/chat/assets/javascripts/discourse/services/chat-composer.js index 74f01157f04..8c62361ae8c 100644 --- a/plugins/chat/assets/javascripts/discourse/services/chat-composer.js +++ b/plugins/chat/assets/javascripts/discourse/services/chat-composer.js @@ -1,13 +1,12 @@ import { tracked } from "@glimmer/tracking"; import Service, { inject as service } from "@ember/service"; import { action } from "@ember/object"; -import ChatMessage from "discourse/plugins/chat/discourse/models/chat-message"; export default class ChatComposer extends Service { @service chat; @service currentUser; - @tracked _message; + @tracked message; @action cancel() { @@ -18,13 +17,6 @@ export default class ChatComposer extends Service { } } - @action - reset(channel) { - this.message = ChatMessage.createDraftMessage(channel, { - user: this.currentUser, - }); - } - @action clear() { this.message.message = ""; @@ -41,12 +33,4 @@ export default class ChatComposer extends Service { onCancelEditing() { this.reset(); } - - get message() { - return this._message; - } - - set message(message) { - this._message = message; - } } diff --git a/plugins/chat/assets/javascripts/discourse/services/chat-drafts-manager.js b/plugins/chat/assets/javascripts/discourse/services/chat-drafts-manager.js new file mode 100644 index 00000000000..00662f03edc --- /dev/null +++ b/plugins/chat/assets/javascripts/discourse/services/chat-drafts-manager.js @@ -0,0 +1,25 @@ +import Service from "@ember/service"; +import ChatMessage from "discourse/plugins/chat/discourse/models/chat-message"; +export default class ChatDraftsManager extends Service { + drafts = {}; + + add(message) { + if (message instanceof ChatMessage) { + this.drafts[message.channel.id] = message; + } else { + throw new Error("message must be an instance of ChatMessage"); + } + } + + get({ channelId }) { + return this.drafts[channelId]; + } + + remove({ channelId }) { + delete this.drafts[channelId]; + } + + reset() { + this.drafts = {}; + } +} diff --git a/plugins/chat/assets/javascripts/discourse/services/chat.js b/plugins/chat/assets/javascripts/discourse/services/chat.js index 39f3bce2de4..f61df665d93 100644 --- a/plugins/chat/assets/javascripts/discourse/services/chat.js +++ b/plugins/chat/assets/javascripts/discourse/services/chat.js @@ -27,6 +27,7 @@ export default class Chat extends Service { @service chatNotificationManager; @service chatSubscriptionsManager; @service chatStateManager; + @service chatDraftsManager; @service presence; @service router; @service site; @@ -167,19 +168,18 @@ export default class Chat extends Service { [...channels.public_channels, ...channels.direct_message_channels].forEach( (channelObject) => { const channel = this.chatChannelsManager.store(channelObject); + const storedDraft = (this.currentUser?.chat_drafts || []).find( + (draft) => draft.channel_id === channel.id + ); - if (this.currentUser.chat_drafts) { - const storedDraft = this.currentUser.chat_drafts.find( - (draft) => draft.channel_id === channel.id - ); - - channel.draft = ChatMessage.createDraftMessage( - channel, - Object.assign( - { - user: this.currentUser, - }, - storedDraft ? JSON.parse(storedDraft.data) : {} + if (storedDraft) { + this.chatDraftsManager.add( + ChatMessage.createDraftMessage( + channel, + Object.assign( + { user: this.currentUser }, + JSON.parse(storedDraft.data) + ) ) ); } diff --git a/plugins/chat/spec/system/chat_composer_draft_spec.rb b/plugins/chat/spec/system/chat_composer_draft_spec.rb new file mode 100644 index 00000000000..7dfff4b4ef7 --- /dev/null +++ b/plugins/chat/spec/system/chat_composer_draft_spec.rb @@ -0,0 +1,135 @@ +# frozen_string_literal: true + +RSpec.describe "Chat composer draft", type: :system, js: true do + fab!(:current_user) { Fabricate(:user) } + fab!(:channel_1) { Fabricate(:chat_channel) } + fab!(:message_1) { Fabricate(:chat_message, chat_channel: channel_1) } + + let(:chat_page) { PageObjects::Pages::Chat.new } + let(:channel_page) { PageObjects::Pages::ChatChannel.new } + + before { chat_system_bootstrap } + + context "when loading a channel with a draft" do + fab!(:draft_1) do + Chat::Draft.create!( + chat_channel: channel_1, + user: current_user, + data: { message: "draft" }.to_json, + ) + end + + before do + channel_1.add(current_user) + sign_in(current_user) + end + + it "loads the draft" do + chat_page.visit_channel(channel_1) + + expect(channel_page.composer.value).to eq("draft") + end + + context "when loading another channel and back" do + fab!(:channel_2) { Fabricate(:chat_channel) } + + fab!(:draft_2) do + Chat::Draft.create!( + chat_channel: channel_2, + user: current_user, + data: { message: "draft2" }.to_json, + ) + end + + before { channel_2.add(current_user) } + + it "loads the correct drafts" do + chat_page.visit_channel(channel_1) + + expect(channel_page.composer.value).to eq("draft") + + chat_page.visit_channel(channel_2) + + expect(channel_page.composer.value).to eq("draft2") + + chat_page.visit_channel(channel_1) + + expect(channel_page.composer.value).to eq("draft") + end + end + + context "with editing" do + fab!(:draft_1) do + Chat::Draft.create!( + chat_channel: channel_1, + user: current_user, + data: { message: message_1.message, id: message_1.id, editing: true }.to_json, + ) + end + + it "loads the draft with the editing state" do + chat_page.visit_channel(channel_1) + + expect(channel_page.composer).to be_editing_message(message_1) + end + end + + context "with uploads" do + fab!(:upload_1) do + Fabricate( + :upload, + url: "/images/logo-dark.png", + original_filename: "logo_dark.png", + width: 400, + height: 300, + extension: "png", + ) + end + + fab!(:draft_1) do + Chat::Draft.create!( + chat_channel: channel_1, + user: current_user, + data: { message: "draft", uploads: [upload_1] }.to_json, + ) + end + + it "loads the draft with the upload" do + chat_page.visit_channel(channel_1) + + expect(channel_page.composer.value).to eq("draft") + expect(page).to have_selector(".chat-composer-upload--image", count: 1) + end + end + + context "when replying" do + fab!(:draft_1) do + Chat::Draft.create!( + chat_channel: channel_1, + user: current_user, + data: { + message: "draft", + replyToMsg: { + id: message_1.id, + excerpt: message_1.excerpt, + user: { + id: message_1.user.id, + name: nil, + avatar_template: message_1.user.avatar_template, + username: message_1.user.username, + }, + }, + }.to_json, + ) + end + + it "loads the draft with replied to mesage" do + chat_page.visit_channel(channel_1) + + expect(channel_page.composer.value).to eq("draft") + expect(page).to have_selector(".chat-reply__username", text: message_1.user.username) + expect(page).to have_selector(".chat-reply__excerpt", text: message_1.excerpt) + end + end + end +end diff --git a/plugins/chat/spec/system/chat_composer_spec.rb b/plugins/chat/spec/system/chat_composer_spec.rb index a505b59ccc6..1495913c85d 100644 --- a/plugins/chat/spec/system/chat_composer_spec.rb +++ b/plugins/chat/spec/system/chat_composer_spec.rb @@ -5,90 +5,11 @@ RSpec.describe "Chat composer", type: :system, js: true do fab!(:channel_1) { Fabricate(:chat_channel) } fab!(:message_1) { 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 } before { chat_system_bootstrap } - context "when loading a channel with a draft" do - fab!(:draft_1) do - Chat::Draft.create!( - chat_channel: channel_1, - user: current_user, - data: { message: "draft" }.to_json, - ) - end - - before do - channel_1.add(current_user) - sign_in(current_user) - end - - it "loads the draft" do - chat.visit_channel(channel_1) - - expect(find(".chat-composer__input").value).to eq("draft") - end - - context "with uploads" do - fab!(:upload_1) do - Fabricate( - :upload, - url: "/images/logo-dark.png", - original_filename: "logo_dark.png", - width: 400, - height: 300, - extension: "png", - ) - end - - fab!(:draft_1) do - Chat::Draft.create!( - chat_channel: channel_1, - user: current_user, - data: { message: "draft", uploads: [upload_1] }.to_json, - ) - end - - it "loads the draft with the upload" do - chat.visit_channel(channel_1) - - expect(find(".chat-composer__input").value).to eq("draft") - expect(page).to have_selector(".chat-composer-upload--image", count: 1) - end - end - - context "when replying" do - fab!(:draft_1) do - Chat::Draft.create!( - chat_channel: channel_1, - user: current_user, - data: { - message: "draft", - replyToMsg: { - id: message_1.id, - excerpt: message_1.excerpt, - user: { - id: message_1.user.id, - name: nil, - avatar_template: message_1.user.avatar_template, - username: message_1.user.username, - }, - }, - }.to_json, - ) - end - - it "loads the draft with replied to mesage" do - chat.visit_channel(channel_1) - - expect(find(".chat-composer__input").value).to eq("draft") - expect(page).to have_selector(".chat-reply__username", text: message_1.user.username) - expect(page).to have_selector(".chat-reply__excerpt", text: message_1.excerpt) - end - end - end - context "when replying to a message" do before do channel_1.add(current_user) @@ -96,8 +17,8 @@ RSpec.describe "Chat composer", type: :system, js: true do end it "adds the reply indicator to the composer" do - chat.visit_channel(channel_1) - channel.reply_to(message_1) + chat_page.visit_channel(channel_1) + channel_page.reply_to(message_1) expect(page).to have_selector( ".chat-composer-message-details .chat-reply__username", @@ -109,8 +30,8 @@ RSpec.describe "Chat composer", type: :system, js: true do before { message_1.update!(message: "not marked") } it "renders text in the details" do - chat.visit_channel(channel_1) - channel.reply_to(message_1) + chat_page.visit_channel(channel_1) + channel_page.reply_to(message_1) expect( find(".chat-composer-message-details .chat-reply__excerpt")["innerHTML"].strip, @@ -128,47 +49,47 @@ RSpec.describe "Chat composer", type: :system, js: true do end it "adds the edit indicator" do - chat.visit_channel(channel_1) - channel.edit_message(message_2) + chat_page.visit_channel(channel_1) + channel_page.edit_message(message_2) expect(page).to have_selector( ".chat-composer-message-details .chat-reply__username", text: current_user.username, ) - expect(find(".chat-composer__input").value).to eq(message_2.message) + expect(channel_page.composer.value).to eq(message_2.message) end it "updates the message instantly" do - chat.visit_channel(channel_1) + chat_page.visit_channel(channel_1) page.driver.browser.network_conditions = { offline: true } - channel.edit_message(message_2) + channel_page.edit_message(message_2) find(".chat-composer__input").send_keys("instant") - channel.click_send_message + channel_page.click_send_message - expect(channel).to have_message(text: message_2.message + "instant") + expect(channel_page).to have_message(text: message_2.message + "instant") page.driver.browser.network_conditions = { offline: false } end context "when pressing escape" do it "cancels editing" do - chat.visit_channel(channel_1) - channel.edit_message(message_2) + chat_page.visit_channel(channel_1) + channel_page.edit_message(message_2) find(".chat-composer__input").send_keys(:escape) expect(page).to have_no_selector(".chat-composer-message-details .chat-reply__username") - expect(find(".chat-composer__input").value).to eq("") + expect(channel_page.composer.value).to eq("") end end context "when closing edited message" do it "cancels editing" do - chat.visit_channel(channel_1) - channel.edit_message(message_2) + chat_page.visit_channel(channel_1) + channel_page.edit_message(message_2) find(".cancel-message-action").click expect(page).to have_no_selector(".chat-composer-message-details .chat-reply__username") - expect(find(".chat-composer__input").value).to eq("") + expect(channel_page.composer.value).to eq("") end end end @@ -180,19 +101,19 @@ RSpec.describe "Chat composer", type: :system, js: true do end xit "adds the emoji to the composer" do - chat.visit_channel(channel_1) - channel.open_action_menu - channel.click_action_button("emoji") + chat_page.visit_channel(channel_1) + channel_page.open_action_menu + channel_page.click_action_button("emoji") find("[data-emoji='grimacing']").click(wait: 0.5) - expect(find(".chat-composer__input").value).to eq(":grimacing:") + expect(channel_page.composer.value).to eq(":grimacing:") end it "removes denied emojis from insert emoji picker" do SiteSetting.emoji_deny_list = "monkey|peach" - chat.visit_channel(channel_1) - channel.composer.open_emoji_picker + chat_page.visit_channel(channel_1) + channel_page.composer.open_emoji_picker expect(page).to have_no_selector("[data-emoji='monkey']") expect(page).to have_no_selector("[data-emoji='peach']") @@ -206,16 +127,16 @@ RSpec.describe "Chat composer", type: :system, js: true do end it "adds the emoji to the composer" do - chat.visit_channel(channel_1) + chat_page.visit_channel(channel_1) find(".chat-composer__input").fill_in(with: ":gri") find(".emoji-shortname", text: "grimacing").click - expect(find(".chat-composer__input").value).to eq(":grimacing: ") + expect(channel_page.composer.value).to eq(":grimacing: ") end it "doesn't suggest denied emojis and aliases" do SiteSetting.emoji_deny_list = "peach|poop" - chat.visit_channel(channel_1) + chat_page.visit_channel(channel_1) find(".chat-composer__input").fill_in(with: ":peac") expect(page).to have_no_selector(".emoji-shortname", text: "peach") @@ -232,7 +153,7 @@ RSpec.describe "Chat composer", type: :system, js: true do end xit "prefills the emoji picker filter input" do - chat.visit_channel(channel_1) + chat_page.visit_channel(channel_1) find(".chat-composer__input").fill_in(with: ":gri") click_link(I18n.t("js.composer.more_emoji")) @@ -241,7 +162,7 @@ RSpec.describe "Chat composer", type: :system, js: true do end xit "filters with the prefilled input" do - chat.visit_channel(channel_1) + chat_page.visit_channel(channel_1) find(".chat-composer__input").fill_in(with: ":fr") click_link(I18n.t("js.composer.more_emoji")) @@ -258,19 +179,19 @@ RSpec.describe "Chat composer", type: :system, js: true do end it "propagates keys to composer" do - chat.visit_channel(channel_1) + chat_page.visit_channel(channel_1) find("body").send_keys("b") - expect(find(".chat-composer__input").value).to eq("b") + expect(channel_page.composer.value).to eq("b") find("body").send_keys("b") - expect(find(".chat-composer__input").value).to eq("bb") + expect(channel_page.composer.value).to eq("bb") find("body").send_keys(:enter) # special case - expect(find(".chat-composer__input").value).to eq("bb") + expect(channel_page.composer.value).to eq("bb") end end @@ -288,7 +209,7 @@ RSpec.describe "Chat composer", type: :system, js: true do element.setSelectionRange(0, element.value.length) JS - chat.visit_channel(channel_1) + chat_page.visit_channel(channel_1) find("body").send_keys("https://www.discourse.org") page.execute_script(select_text, ".chat-composer__input") @@ -301,7 +222,7 @@ RSpec.describe "Chat composer", type: :system, js: true do page.send_keys [modifier, "v"] - expect(find(".chat-composer__input").value).to eq("[discourse](https://www.discourse.org)") + expect(channel_page.composer.value).to eq("[discourse](https://www.discourse.org)") end end @@ -313,11 +234,11 @@ RSpec.describe "Chat composer", type: :system, js: true do end it "works" do - chat.visit_channel(channel_1) + chat_page.visit_channel(channel_1) find("body").send_keys("1") - channel.click_send_message + channel_page.click_send_message - expect(channel).to have_message(text: "1") + expect(channel_page).to have_message(text: "1") end end @@ -329,7 +250,7 @@ RSpec.describe "Chat composer", type: :system, js: true do end it "doesn’t allow to send" do - chat.visit_channel(channel_1) + chat_page.visit_channel(channel_1) find("body").send_keys("1") expect(page).to have_css(".chat-composer.is-send-disabled") @@ -343,14 +264,14 @@ RSpec.describe "Chat composer", type: :system, js: true do end it "doesn’t allow to send" do - chat.visit_channel(channel_1) + chat_page.visit_channel(channel_1) page.driver.browser.network_conditions = { latency: 20_000 } file_path = file_from_fixtures("logo.png", "images").path attach_file(file_path) do - channel.open_action_menu - channel.click_action_button("chat-upload-btn") + channel_page.open_action_menu + channel_page.click_action_button("chat-upload-btn") end expect(page).to have_css(".chat-composer-upload--in-progress") diff --git a/plugins/chat/spec/system/page_objects/chat/components/composer.rb b/plugins/chat/spec/system/page_objects/chat/components/composer.rb index bba098582ac..3c3306893fa 100644 --- a/plugins/chat/spec/system/page_objects/chat/components/composer.rb +++ b/plugins/chat/spec/system/page_objects/chat/components/composer.rb @@ -12,6 +12,10 @@ module PageObjects @context = context end + def message_details + @message_details ||= PageObjects::Components::Chat::ComposerMessageDetails.new(context) + end + def input find(context).find(SELECTOR).find(".chat-composer__input") end @@ -31,6 +35,10 @@ module PageObjects def open_emoji_picker find(context).find(SELECTOR).find(".chat-composer-button__btn.emoji").click end + + def editing_message?(message) + value == message.message && message_details.editing_message?(message) + end end end end diff --git a/plugins/chat/spec/system/page_objects/chat/components/composer_message_details.rb b/plugins/chat/spec/system/page_objects/chat/components/composer_message_details.rb index c064b0cec10..694f1012401 100644 --- a/plugins/chat/spec/system/page_objects/chat/components/composer_message_details.rb +++ b/plugins/chat/spec/system/page_objects/chat/components/composer_message_details.rb @@ -12,13 +12,19 @@ module PageObjects @context = context end - def has_message?(message) - find(context).find(SELECTOR + "[data-id=\"#{message.id}\"]") + def has_message?(message, action: nil) + data_attributes = "[data-id=\"#{message.id}\"]" + data_attributes << "[data-action=\"#{action}\"]" if action + find(context).find(SELECTOR + data_attributes) end def has_no_message? find(context).has_no_css?(SELECTOR) end + + def editing_message?(message) + has_message?(message, action: :edit) + end end end end diff --git a/plugins/chat/test/javascripts/unit/services/chat-drafts-manager-test.js b/plugins/chat/test/javascripts/unit/services/chat-drafts-manager-test.js new file mode 100644 index 00000000000..9c5338cddf9 --- /dev/null +++ b/plugins/chat/test/javascripts/unit/services/chat-drafts-manager-test.js @@ -0,0 +1,53 @@ +import { module, test } from "qunit"; +import { setupTest } from "ember-qunit"; +import { getOwner } from "discourse-common/lib/get-owner"; +import fabricators from "discourse/plugins/chat/discourse/lib/fabricators"; + +module( + "Discourse Chat | Unit | Service | chat-drafts-manager", + function (hooks) { + setupTest(hooks); + + hooks.beforeEach(function () { + this.subject = getOwner(this).lookup("service:chat-drafts-manager"); + }); + + hooks.afterEach(function () { + this.subject.reset(); + }); + + test("storing and retrieving message", function (assert) { + const message1 = fabricators.message(); + this.subject.add(message1); + + assert.strictEqual( + this.subject.get({ channelId: message1.channel.id }), + message1 + ); + + const message2 = fabricators.message(); + this.subject.add(message2); + + assert.strictEqual( + this.subject.get({ channelId: message2.channel.id }), + message2 + ); + }); + + test("stores only chat messages", function (assert) { + assert.throws(function () { + this.subject.add({ foo: "bar" }); + }, /instance of ChatMessage/); + }); + + test("#reset", function (assert) { + this.subject.add(fabricators.message()); + + assert.strictEqual(Object.keys(this.subject.drafts).length, 1); + + this.subject.reset(); + + assert.strictEqual(Object.keys(this.subject.drafts).length, 0); + }); + } +);