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.
This commit is contained in:
Joffrey JAFFEUX 2024-07-09 18:34:35 +02:00 committed by GitHub
parent 866f6b910b
commit c080ac0094
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 81 additions and 16 deletions

View File

@ -18,6 +18,7 @@ import {
import i18n from "discourse-common/helpers/i18n"; import i18n from "discourse-common/helpers/i18n";
import discourseDebounce from "discourse-common/lib/debounce"; import discourseDebounce from "discourse-common/lib/debounce";
import { bind } from "discourse-common/utils/decorators"; import { bind } from "discourse-common/utils/decorators";
import I18n from "I18n";
import ChatChannelStatus from "discourse/plugins/chat/discourse/components/chat-channel-status"; import ChatChannelStatus from "discourse/plugins/chat/discourse/components/chat-channel-status";
import firstVisibleMessageId from "discourse/plugins/chat/discourse/helpers/first-visible-message-id"; import firstVisibleMessageId from "discourse/plugins/chat/discourse/helpers/first-visible-message-id";
import ChatChannelSubscriptionManager from "discourse/plugins/chat/discourse/lib/chat-channel-subscription-manager"; 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-composer") composer;
@service("chat-channel-pane") pane; @service("chat-channel-pane") pane;
@service currentUser; @service currentUser;
@service dialog;
@service messageBus; @service messageBus;
@service router; @service router;
@service site; @service site;
@service siteSettings;
@tracked sending = false; @tracked sending = false;
@tracked showChatQuoteSuccess = false; @tracked showChatQuoteSuccess = false;
@ -496,6 +499,17 @@ export default class ChatChannel extends Component {
@action @action
async onSendMessage(message) { 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(); await message.cook();
if (message.editing) { if (message.editing) {
await this.#sendEditMessage(message); await this.#sendEditMessage(message);

View File

@ -11,6 +11,7 @@ import { popupAjaxError } from "discourse/lib/ajax-error";
import { NotificationLevels } from "discourse/lib/notification-levels"; import { NotificationLevels } from "discourse/lib/notification-levels";
import discourseDebounce from "discourse-common/lib/debounce"; import discourseDebounce from "discourse-common/lib/debounce";
import { bind } from "discourse-common/utils/decorators"; import { bind } from "discourse-common/utils/decorators";
import I18n from "I18n";
import ChatThreadTitlePrompt from "discourse/plugins/chat/discourse/components/chat-thread-title-prompt"; 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 firstVisibleMessageId from "discourse/plugins/chat/discourse/helpers/first-visible-message-id";
import ChatChannelThreadSubscriptionManager from "discourse/plugins/chat/discourse/lib/chat-channel-thread-subscription-manager"; 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 chatDraftsManager;
@service chatThreadComposer; @service chatThreadComposer;
@service chatThreadPane; @service chatThreadPane;
@service dialog;
@service currentUser; @service currentUser;
@service router; @service router;
@service siteSettings; @service siteSettings;
@ -359,6 +361,17 @@ export default class ChatThread extends Component {
@action @action
async onSendMessage(message) { 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(); await message.cook();
if (message.editing) { if (message.editing) {
await this.#sendEditMessage(message); await this.#sendEditMessage(message);

View File

@ -259,6 +259,7 @@ en:
copy_link: "Copy link" copy_link: "Copy link"
copy_text: "Copy text" copy_text: "Copy text"
rebake_message: "Rebuild HTML" rebake_message: "Rebuild HTML"
message_too_long: "Message is too long, messages must be a maximum of %{count} characters."
retry_staged_message: retry_staged_message:
title: "Network error" title: "Network error"
action: "Send again?" action: "Send again?"

View File

@ -1,26 +1,55 @@
# frozen_string_literal: true # frozen_string_literal: true
RSpec.describe "Message errors", type: :system do 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 context "when message is too long" do
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 }
fab!(:current_user) { Fabricate(:admin) }
before do
chat_system_bootstrap
sign_in(current_user)
end
context "when in channel" do
fab!(:channel) { Fabricate(:chat_channel) } fab!(:channel) { Fabricate(:chat_channel) }
let(:channel_page) { PageObjects::Pages::ChatChannel.new }
before { channel.add(current_user) } before { channel.add(current_user) }
it "only shows the error, not the message" do it "shows a dialog with the error and keeps the message in the input" do
sign_in(current_user)
chat_page.visit_channel(channel) chat_page.visit_channel(channel)
channel_page.send_message("atoolongmessage" + "a" * max_length) channel_page.send_message(message)
expect(page).to have_content(I18n.t("chat.errors.message_too_long", count: max_length)) expect(dialog_page).to have_content(
expect(page).to have_no_content("atoolongmessage") 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 end
end end

View File

@ -35,8 +35,10 @@ module PageObjects
end end
def click_composer def click_composer
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 find(".chat-channel .chat-composer__input").click # ensures autocomplete is closed and not masking anything
end end
end
def click_send_message def click_send_message
find(".chat-composer.is-send-enabled .chat-composer-button.-send").click find(".chat-composer.is-send-enabled .chat-composer-button.-send").click

View File

@ -94,8 +94,10 @@ module PageObjects
end end
def click_composer def click_composer
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 find(".chat-thread .chat-composer__input").click # ensures autocomplete is closed and not masking anything
end end
end
def send_message(text = nil) def send_message(text = nil)
text ||= Faker::Lorem.characters(number: SiteSetting.chat_minimum_message_length) text ||= Faker::Lorem.characters(number: SiteSetting.chat_minimum_message_length)

View File

@ -48,6 +48,10 @@ module PageObjects
input.value input.value
end end
def has_value?(expected)
value == expected
end
def reply_to_last_message_shortcut def reply_to_last_message_shortcut
input.click input.click
input.send_keys(%i[shift arrow_up]) input.send_keys(%i[shift arrow_up])