From c080ac0094dbd2f8030d9174f9a3a85bcf7b8e44 Mon Sep 17 00:00:00 2001 From: Joffrey JAFFEUX Date: Tue, 9 Jul 2024 18:34:35 +0200 Subject: [PATCH] FIX: show too long message error on client (#27794) Prior to this fix we would show the message after a round trip to the server. If you had a too long message error, at this point your input would be empty and we would show an error in chat. It's important to have this server side safety net, but we can have a better UX by showing an error on the frontend before sending the message, that way you can correct your message before sending it and not lose it. --- .../discourse/components/chat-channel.gjs | 14 +++++ .../discourse/components/chat-thread.gjs | 13 +++++ plugins/chat/config/locales/client.en.yml | 1 + .../chat/spec/system/message_errors_spec.rb | 57 ++++++++++++++----- .../system/page_objects/chat/chat_channel.rb | 4 +- .../system/page_objects/chat/chat_thread.rb | 4 +- .../page_objects/chat/components/composer.rb | 4 ++ 7 files changed, 81 insertions(+), 16 deletions(-) diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-channel.gjs b/plugins/chat/assets/javascripts/discourse/components/chat-channel.gjs index 11abaee3c04..0b2f14ba3a3 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-channel.gjs +++ b/plugins/chat/assets/javascripts/discourse/components/chat-channel.gjs @@ -18,6 +18,7 @@ import { import i18n from "discourse-common/helpers/i18n"; import discourseDebounce from "discourse-common/lib/debounce"; import { bind } from "discourse-common/utils/decorators"; +import I18n from "I18n"; import ChatChannelStatus from "discourse/plugins/chat/discourse/components/chat-channel-status"; import firstVisibleMessageId from "discourse/plugins/chat/discourse/helpers/first-visible-message-id"; import ChatChannelSubscriptionManager from "discourse/plugins/chat/discourse/lib/chat-channel-subscription-manager"; @@ -62,9 +63,11 @@ export default class ChatChannel extends Component { @service("chat-channel-composer") composer; @service("chat-channel-pane") pane; @service currentUser; + @service dialog; @service messageBus; @service router; @service site; + @service siteSettings; @tracked sending = false; @tracked showChatQuoteSuccess = false; @@ -496,6 +499,17 @@ export default class ChatChannel extends Component { @action async onSendMessage(message) { + if ( + message.message.length > this.siteSettings.chat_maximum_message_length + ) { + this.dialog.alert( + I18n.t("chat.message_too_long", { + count: this.siteSettings.chat_maximum_message_length, + }) + ); + return; + } + await message.cook(); if (message.editing) { await this.#sendEditMessage(message); diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-thread.gjs b/plugins/chat/assets/javascripts/discourse/components/chat-thread.gjs index 1bbd1d063e7..ebfd382dee7 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-thread.gjs +++ b/plugins/chat/assets/javascripts/discourse/components/chat-thread.gjs @@ -11,6 +11,7 @@ import { popupAjaxError } from "discourse/lib/ajax-error"; import { NotificationLevels } from "discourse/lib/notification-levels"; import discourseDebounce from "discourse-common/lib/debounce"; import { bind } from "discourse-common/utils/decorators"; +import I18n from "I18n"; import ChatThreadTitlePrompt from "discourse/plugins/chat/discourse/components/chat-thread-title-prompt"; import firstVisibleMessageId from "discourse/plugins/chat/discourse/helpers/first-visible-message-id"; import ChatChannelThreadSubscriptionManager from "discourse/plugins/chat/discourse/lib/chat-channel-thread-subscription-manager"; @@ -49,6 +50,7 @@ export default class ChatThread extends Component { @service chatDraftsManager; @service chatThreadComposer; @service chatThreadPane; + @service dialog; @service currentUser; @service router; @service siteSettings; @@ -359,6 +361,17 @@ export default class ChatThread extends Component { @action async onSendMessage(message) { + if ( + message.message.length > this.siteSettings.chat_maximum_message_length + ) { + this.dialog.alert( + I18n.t("chat.message_too_long", { + count: this.siteSettings.chat_maximum_message_length, + }) + ); + return; + } + await message.cook(); if (message.editing) { await this.#sendEditMessage(message); diff --git a/plugins/chat/config/locales/client.en.yml b/plugins/chat/config/locales/client.en.yml index 2550f725af7..7d1f101e653 100644 --- a/plugins/chat/config/locales/client.en.yml +++ b/plugins/chat/config/locales/client.en.yml @@ -259,6 +259,7 @@ en: copy_link: "Copy link" copy_text: "Copy text" rebake_message: "Rebuild HTML" + message_too_long: "Message is too long, messages must be a maximum of %{count} characters." retry_staged_message: title: "Network error" action: "Send again?" diff --git a/plugins/chat/spec/system/message_errors_spec.rb b/plugins/chat/spec/system/message_errors_spec.rb index 8b2336aa45a..5d751cbc52e 100644 --- a/plugins/chat/spec/system/message_errors_spec.rb +++ b/plugins/chat/spec/system/message_errors_spec.rb @@ -1,26 +1,55 @@ # frozen_string_literal: true RSpec.describe "Message errors", type: :system do - fab!(:current_user) { Fabricate(:admin) } - let(:chat_page) { PageObjects::Pages::Chat.new } - let(:channel_page) { PageObjects::Pages::ChatChannel.new } - let(:max_length) { SiteSetting.chat_maximum_message_length } - - before { chat_system_bootstrap } - context "when message is too long" do - fab!(:channel) { Fabricate(:chat_channel) } + let(:chat_page) { PageObjects::Pages::Chat.new } + let(:dialog_page) { PageObjects::Components::Dialog.new } + let(:max_length) { SiteSetting.chat_maximum_message_length } + let(:message) { "atoolongmessage" + "a" * max_length } - before { channel.add(current_user) } + fab!(:current_user) { Fabricate(:admin) } - it "only shows the error, not the message" do + before do + chat_system_bootstrap sign_in(current_user) - chat_page.visit_channel(channel) + end - channel_page.send_message("atoolongmessage" + "a" * max_length) + context "when in channel" do + fab!(:channel) { Fabricate(:chat_channel) } - expect(page).to have_content(I18n.t("chat.errors.message_too_long", count: max_length)) - expect(page).to have_no_content("atoolongmessage") + let(:channel_page) { PageObjects::Pages::ChatChannel.new } + + before { channel.add(current_user) } + + it "shows a dialog with the error and keeps the message in the input" do + chat_page.visit_channel(channel) + + channel_page.send_message(message) + + expect(dialog_page).to have_content( + I18n.t("chat.errors.message_too_long", count: max_length), + ) + expect(channel_page.composer).to have_value(message) + end + end + + context "when in thread" do + fab!(:thread) { Fabricate(:chat_thread) } + + let(:thread_page) { PageObjects::Pages::ChatThread.new } + + before { thread.add(current_user) } + + it "shows a dialog with the error and keeps the message in the input" do + chat_page.visit_thread(thread) + + thread_page.send_message(message) + + expect(dialog_page).to have_content( + I18n.t("chat.errors.message_too_long", count: max_length), + ) + expect(thread_page.composer).to have_value(message) + end end end end 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 92c514ea6eb..15b31e7a473 100644 --- a/plugins/chat/spec/system/page_objects/chat/chat_channel.rb +++ b/plugins/chat/spec/system/page_objects/chat/chat_channel.rb @@ -35,7 +35,9 @@ module PageObjects end def click_composer - find(".chat-channel .chat-composer__input").click # ensures autocomplete is closed and not masking anything + if has_no_css?(".dialog-overlay", wait: 0) # we can't click composer if a dialog is open, in case of error for exampel + find(".chat-channel .chat-composer__input").click # ensures autocomplete is closed and not masking anything + end end def click_send_message 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 5ec9364c9be..600426b0d22 100644 --- a/plugins/chat/spec/system/page_objects/chat/chat_thread.rb +++ b/plugins/chat/spec/system/page_objects/chat/chat_thread.rb @@ -94,7 +94,9 @@ module PageObjects end def click_composer - find(".chat-thread .chat-composer__input").click # ensures autocomplete is closed and not masking anything + if has_no_css?(".dialog-overlay", wait: 0) # we can't click composer if a dialog is open, in case of error for exampel + find(".chat-thread .chat-composer__input").click # ensures autocomplete is closed and not masking anything + end end def send_message(text = nil) 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 07695148542..255fd8f11fd 100644 --- a/plugins/chat/spec/system/page_objects/chat/components/composer.rb +++ b/plugins/chat/spec/system/page_objects/chat/components/composer.rb @@ -48,6 +48,10 @@ module PageObjects input.value end + def has_value?(expected) + value == expected + end + def reply_to_last_message_shortcut input.click input.send_keys(%i[shift arrow_up])