FEATURE: Allow to bulk delete chat messages (#26586)

This commit is contained in:
Jan Cernik 2024-05-22 08:57:00 -03:00 committed by GitHub
parent e5d040ef61
commit 9889547475
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
13 changed files with 494 additions and 3 deletions

View File

@ -26,6 +26,18 @@ class Chat::Api::ChannelMessagesController < Chat::ApiController
end
end
def bulk_destroy
with_service(Chat::TrashMessages) do
on_success { render(json: success_json) }
on_failure { render(json: failed_json, status: 422) }
on_model_not_found(:messages) { raise Discourse::NotFound }
on_failed_policy(:invalid_access) { raise Discourse::InvalidAccess }
on_failed_contract do |contract|
render(json: failed_json.merge(errors: contract.errors.full_messages), status: 400)
end
end
end
def restore
with_service(Chat::RestoreMessage) do
on_success { render(json: success_json) }

View File

@ -80,7 +80,7 @@ module Chat
message.chat_channel.update_last_message_id!
end
def publish_events(guardian:, message:)
def publish_events(contract:, guardian:, message:)
DiscourseEvent.trigger(:chat_message_trashed, message, message.chat_channel, guardian.user)
Chat::Publisher.publish_delete!(message.chat_channel, message)

View File

@ -0,0 +1,97 @@
# frozen_string_literal: true
module Chat
# Service responsible for trashing multiple chat messages
# for a channel and ensuring that their client and read state
# is updated.
#
# @example
# Chat::TrashMessages.call(message_ids: [2, 3], channel_id: 1, guardian: guardian)
#
class TrashMessages
include Service::Base
# @!method call(message_ids:, channel_id:, guardian:)
# @param [Array<Integer>] message_ids
# @param [Integer] channel_id
# @param [Guardian] guardian
# @return [Service::Base::Context]
contract
model :messages
policy :can_delete_all_chat_messages
transaction do
step :trash_messages
step :destroy_notifications
step :update_last_message_ids
step :update_tracking_states
step :update_thread_reply_cache
end
step :publish_events
# @!visibility private
class Contract
attribute :channel_id, :integer
attribute :message_ids, :array
validates :channel_id, presence: true
validates :message_ids, length: { minimum: 1, maximum: 50 }
end
private
def fetch_messages(contract:)
Chat::Message.includes(chat_channel: :chatable).where(
id: contract.message_ids,
chat_channel_id: contract.channel_id,
)
end
def can_delete_all_chat_messages(guardian:, messages:)
messages.all? { |message| guardian.can_delete_chat?(message, message.chat_channel.chatable) }
end
def trash_messages(guardian:, messages:)
messages.each { |message| message.trash!(guardian.user) }
end
def destroy_notifications(messages:)
Notification.where(
id:
Chat::Mention
.where(chat_message_id: messages.map(&:id))
.joins(:notifications)
.select("notifications.id"),
).destroy_all
end
def update_last_message_ids(messages:)
messages.each do |message|
message.thread&.update_last_message_id!
message.chat_channel.update_last_message_id!
end
end
def update_tracking_states(messages:)
messages.each do |message|
::Chat::Action::ResetUserLastReadChannelMessage.call(
[message.id],
[message.chat_channel_id],
)
if message.thread_id.present?
::Chat::Action::ResetUserLastReadThreadMessage.call([message.id], [message.thread_id])
end
end
end
def update_thread_reply_cache(messages:)
messages.each { |message| message.thread&.decrement_replies_count_cache }
end
def publish_events(contract:, guardian:, messages:)
messages.each do |message|
DiscourseEvent.trigger(:chat_message_trashed, message, message.chat_channel, guardian.user)
end
Chat::Publisher.publish_bulk_delete!(messages.first.chat_channel, contract.message_ids)
end
end
end

View File

@ -738,6 +738,7 @@ export default class ChatChannel extends Component {
@channel.canModerate
}}
@pane={{this.pane}}
@messagesManager={{this.messagesManager}}
/>
{{else}}
{{#if (and (not @channel.isFollowing) @channel.isCategoryChannel)}}

View File

@ -573,7 +573,10 @@ export default class ChatThread extends Component {
/>
{{#if this.chatThreadPane.selectingMessages}}
<ChatSelectionManager @pane={{this.chatThreadPane}} />
<ChatSelectionManager
@pane={{this.chatThreadPane}}
@messagesManager={{this.messagesManager}}
/>
{{else}}
<ChatComposerThread
@channel={{@channel}}

View File

@ -0,0 +1,46 @@
import Component from "@glimmer/component";
import { action } from "@ember/object";
import { service } from "@ember/service";
import DButton from "discourse/components/d-button";
import DModal from "discourse/components/d-modal";
import DModalCancel from "discourse/components/d-modal-cancel";
import { popupAjaxError } from "discourse/lib/ajax-error";
import i18n from "discourse-common/helpers/i18n";
export default class DeleteMessagesConfirm extends Component {
@service chatApi;
@action
async delete() {
try {
await this.chatApi.trashMessages(
this.args.model.sourceChannel.id,
this.args.model.selectedMessageIds
);
} catch (error) {
popupAjaxError(error);
} finally {
this.args.closeModal();
}
}
<template>
<DModal @closeModal={{@closeModal}} @headerClass="hidden">
<:body>
{{i18n
"chat.delete_messages.confirm"
count=@model.selectedMessageIds.length
}}
</:body>
<:footer>
<DButton
class="btn-primary"
@action={{this.delete}}
@label="delete"
@icon="trash-alt"
/>
<DModalCancel @close={{@closeModal}} />
</:footer>
</DModal>
</template>
}

View File

@ -2,21 +2,25 @@ import Component from "@glimmer/component";
import { getOwner } from "@ember/application";
import { action } from "@ember/object";
import { service } from "@ember/service";
import { not } from "truth-helpers";
import { not, or } from "truth-helpers";
import DButton from "discourse/components/d-button";
import { popupAjaxError } from "discourse/lib/ajax-error";
import { clipboardCopyAsync } from "discourse/lib/utilities";
import { isTesting } from "discourse-common/config/environment";
import { bind } from "discourse-common/utils/decorators";
import I18n from "discourse-i18n";
import DeleteMessagesConfirm from "discourse/plugins/chat/discourse/components/chat/modal/delete-messages-confirm";
import ChatModalMoveMessageToChannel from "discourse/plugins/chat/discourse/components/chat/modal/move-message-to-channel";
const DELETE_COUNT_LIMIT = 50;
export default class ChatSelectionManager extends Component {
@service("composer") topicComposer;
@service router;
@service modal;
@service site;
@service toasts;
@service currentUser;
@service("chat-api") api;
get enableMove() {
@ -27,6 +31,40 @@ export default class ChatSelectionManager extends Component {
return this.args.pane.selectedMessageIds.length > 0;
}
get deleteCountLimitReached() {
return this.args.pane.selectedMessageIds.length > DELETE_COUNT_LIMIT;
}
get canDeleteMessages() {
return this.args.pane.selectedMessageIds.every((id) => {
return this.canDeleteMessage(id);
});
}
canDeleteMessage(id) {
const message = this.args.messagesManager?.findMessage(id);
if (message) {
const canDelete =
this.currentUser.id === message.user.id
? message.channel?.canDeleteSelf
: message.channel?.canDeleteOthers;
return (
canDelete &&
!message.deletedAt &&
message.channel?.canModifyMessages?.(this.currentUser)
);
}
}
get deleteButtonTitle() {
return I18n.t("chat.selection.delete", {
selectionCount: this.args.pane.selectedMessageIds.length,
totalCount: DELETE_COUNT_LIMIT,
});
}
@bind
async generateQuote() {
const { markdown } = await this.api.generateQuote(
@ -47,6 +85,16 @@ export default class ChatSelectionManager extends Component {
});
}
@action
openDeleteMessagesModal() {
this.modal.show(DeleteMessagesConfirm, {
model: {
sourceChannel: this.args.pane.channel,
selectedMessageIds: this.args.pane.selectedMessageIds,
},
});
}
@action
async quoteMessages() {
let quoteMarkdown;
@ -140,6 +188,18 @@ export default class ChatSelectionManager extends Component {
/>
{{/if}}
<DButton
@icon="trash-alt"
@translatedLabel={{this.deleteButtonTitle}}
@disabled={{or
(not this.anyMessagesSelected)
(not this.canDeleteMessages)
this.deleteCountLimitReached
}}
@action={{this.openDeleteMessagesModal}}
id="chat-delete-btn"
/>
<DButton
@icon="times"
@label="chat.selection.cancel"

View File

@ -215,6 +215,18 @@ export default class ChatApi extends Service {
return this.#deleteRequest(`/channels/${channelId}/messages/${messageId}`);
}
/**
* Trashes (soft deletes) multiple chat messages.
* @param {number} channelId - ID of the channel.
* @param {Array.<number>} messageIds - IDs of the messages to delete.
* @returns {Promise}
*/
trashMessages(channelId, messageIds) {
return this.#deleteRequest(`/channels/${channelId}/messages`, {
message_ids: messageIds,
});
}
/**
* Creates a channel archive.
* @param {number} channelId - The ID of the channel.

View File

@ -58,6 +58,10 @@ en:
one: "You are moving <strong>%{count}</strong> message. Select a destination channel. A placeholder message will be created in the <strong>%{channelTitle}</strong> channel to indicate that this message has been moved. Note that reply chains will not be preserved in the new channel, and messages in the old channel will no longer show as replying to any moved messages."
other: "You are moving <strong>%{count}</strong> messages. Select a destination channel. A placeholder message will be created in the <strong>%{channelTitle}</strong> channel to indicate that these messages have been moved. Note that reply chains will not be preserved in the new channel, and messages in the old channel will no longer show as replying to any moved messages."
confirm_move: "Move Messages"
delete_messages:
confirm:
one: "Are you sure you want to delete this message?"
other: "Are you sure you want to delete these %{count} messages?"
channel_settings:
title: "Channel settings"
edit: "Edit"
@ -552,6 +556,7 @@ en:
cancel: "Cancel"
quote_selection: "Quote in Topic"
copy: "Copy"
delete: "Delete (%{selectionCount}/%{totalCount})"
move_selection_to_channel: "Move to Channel"
error: "There was an error moving the chat messages"
title: "Move Chat to Topic"

View File

@ -58,6 +58,7 @@ Chat::Engine.routes.draw do
put "/channels/:channel_id/messages/:message_id/restore" => "channel_messages#restore"
delete "/channels/:channel_id/messages/:message_id" => "channel_messages#destroy"
delete "/channels/:channel_id/messages" => "channel_messages#bulk_destroy"
get "/channels/:channel_id/summarize" => "summaries#get_summary"
end

View File

@ -0,0 +1,236 @@
# frozen_string_literal: true
RSpec.describe Chat::TrashMessages do
fab!(:current_user) { Fabricate(:user) }
let!(:guardian) { Guardian.new(current_user) }
fab!(:chat_channel) { Fabricate(:chat_channel) }
fab!(:message1) { Fabricate(:chat_message, user: current_user, chat_channel: chat_channel) }
fab!(:message2) { Fabricate(:chat_message, user: current_user, chat_channel: chat_channel) }
describe ".call" do
subject(:result) { described_class.call(params) }
context "when params are not valid" do
let(:params) { { guardian: guardian } }
it { is_expected.to fail_a_contract }
end
context "when params are valid" do
let(:params) do
{ guardian: guardian, message_ids: [message1.id, message2.id], channel_id: chat_channel.id }
end
context "when the user does not have permission to delete" do
before { message1.update!(user: Fabricate(:admin)) }
it { is_expected.to fail_a_policy(:can_delete_all_chat_messages) }
end
context "when the channel does not match the message" do
let(:params) do
{
guardian: guardian,
message_ids: [message1.id, message2.id],
channel_id: Fabricate(:chat_channel).id,
}
end
it { is_expected.to fail_to_find_a_model(:messages) }
end
context "when the user has permission to delete" do
it "sets the service result as successful" do
expect(result).to be_a_success
end
it "trashes the messages" do
result
[message1, message2].each do |message|
expect(Chat::Message.find_by(id: message.id)).to be_nil
deleted_message = Chat::Message.unscoped.find_by(id: message.id)
expect(deleted_message.deleted_by_id).to eq(current_user.id)
expect(deleted_message.deleted_at).to be_within(1.minute).of(Time.zone.now)
end
end
it "destroys notifications for mentions" do
mention1 =
Fabricate(
:user_chat_mention,
chat_message: message1,
notifications: [Fabricate(:notification)],
)
mention2 =
Fabricate(
:user_chat_mention,
chat_message: message2,
notifications: [Fabricate(:notification)],
)
result
[mention1, mention2].each do |mention|
mention = Chat::Mention.find_by(id: mention.id)
expect(mention).to be_present
expect(mention.notifications).to be_empty
end
end
it "publishes associated Discourse and MessageBus events for multiple messages" do
freeze_time
messages = nil
events =
DiscourseEvent
.track_events { messages = MessageBus.track_publish { result } }
.select { |e| e[:event_name] == :chat_message_trashed }
[message1, message2].each do |message|
event = events.find { |e| e[:params].first.id == message.id }
expect(event).to be_present
expect(event[:params]).to eq([message, message.chat_channel, current_user])
end
message_data = messages.find { |m| m.channel == "/chat/#{chat_channel.id}" }.data
expect(message_data).to eq(
{
"type" => "bulk_delete",
"deleted_ids" => [message1.id, message2.id],
"deleted_at" => message1.reload.deleted_at.iso8601(3),
},
)
end
it "updates the tracking to the last non-deleted channel message for users whose last_read_message_id was the trashed message" do
other_message = Fabricate(:chat_message, chat_channel: chat_channel)
membership_1 =
Fabricate(
:user_chat_channel_membership,
chat_channel: chat_channel,
last_read_message: message1,
)
membership_2 =
Fabricate(
:user_chat_channel_membership,
chat_channel: chat_channel,
last_read_message: message2,
)
membership_3 =
Fabricate(
:user_chat_channel_membership,
chat_channel: chat_channel,
last_read_message: other_message,
)
result
expect(membership_1.reload.last_read_message_id).to eq(other_message.id)
expect(membership_2.reload.last_read_message_id).to eq(other_message.id)
expect(membership_3.reload.last_read_message_id).to eq(other_message.id)
end
it "updates the tracking to nil when there are no other messages left in the channnel" do
membership_1 =
Fabricate(
:user_chat_channel_membership,
chat_channel: chat_channel,
last_read_message: message1,
)
membership_2 =
Fabricate(
:user_chat_channel_membership,
chat_channel: chat_channel,
last_read_message: message2,
)
result
expect(membership_1.reload.last_read_message_id).to be_nil
expect(membership_2.reload.last_read_message_id).to be_nil
end
it "updates the channel last_message_id to the previous message in the channel" do
message3 = Fabricate(:chat_message, chat_channel: chat_channel, user: current_user)
params[:message_ids] = [message2.id, message3.id]
chat_channel.update!(last_message: message3)
result
expect(chat_channel.reload.last_message).to eq(message1)
end
context "when the message has a thread" do
fab!(:thread) { Fabricate(:chat_thread, channel: chat_channel) }
before do
message1.update!(thread: thread)
message2.update!(thread: thread, created_at: message1.created_at - 1.hour)
thread.update!(last_message: message1)
thread.original_message.update!(created_at: message1.created_at - 2.hours)
end
it "decrements the thread reply count" do
thread.set_replies_count_cache(5)
result
expect(thread.replies_count_cache).to eq(3)
end
it "updates the tracking to the last non-deleted thread message for users whose last_read_message_id was the trashed message" do
other_message = Fabricate(:chat_message, chat_channel: chat_channel, thread: thread)
membership_1 =
Fabricate(:user_chat_thread_membership, thread: thread, last_read_message: message1)
membership_2 =
Fabricate(:user_chat_thread_membership, thread: thread, last_read_message: message2)
membership_3 =
Fabricate(
:user_chat_thread_membership,
thread: thread,
last_read_message: other_message,
)
result
expect(membership_1.reload.last_read_message_id).to eq(other_message.id)
expect(membership_2.reload.last_read_message_id).to eq(other_message.id)
expect(membership_3.reload.last_read_message_id).to eq(other_message.id)
end
it "updates the tracking to nil when there are no other messages left in the thread" do
membership_1 =
Fabricate(:user_chat_thread_membership, thread: thread, last_read_message: message1)
membership_2 =
Fabricate(:user_chat_thread_membership, thread: thread, last_read_message: message2)
result
expect(membership_1.reload.last_read_message_id).to be_nil
expect(membership_2.reload.last_read_message_id).to be_nil
end
it "updates the thread last_message_id to the previous message in the thread" do
next_message =
Fabricate(
:chat_message,
thread: thread,
user: current_user,
chat_channel: chat_channel,
)
params[:message_ids] = [message2.id, next_message.id]
thread.update!(last_message: next_message)
result
expect(thread.reload.last_message).to eq(message1)
end
context "when there are no other messages left in the thread except the original message" do
it "updates the thread last_message_id to the original message" do
expect(thread.last_message).to eq(message1)
result
expect(thread.reload.last_message).to eq(thread.original_message)
end
end
end
context "when all messages are already deleted" do
before do
message1.trash!
message2.trash!
end
it { is_expected.to fail_to_find_a_model(:messages) }
end
end
end
end
end

View File

@ -80,6 +80,18 @@ RSpec.describe "Deleted message", type: :system do
fab!(:message_5) { Fabricate(:chat_message, chat_channel: channel_1) }
fab!(:message_6) { Fabricate(:chat_message, chat_channel: channel_1) }
it "allows user to bulk delete" do
chat_page.visit_channel(channel_1)
channel_page.messages.select(message_2)
channel_page.messages.select(message_4)
channel_page.messages.select(message_6)
channel_page.selection_management.delete
click_button(I18n.t("js.delete"))
expect(channel_page.messages).to have_deleted_messages(message_2, message_4, message_6)
end
it "groups them" do
chat_page.visit_channel(channel_1)

View File

@ -48,6 +48,10 @@ module PageObjects
click_button("move")
end
def delete
click_button("delete")
end
private
def selector_for(action)
@ -60,6 +64,8 @@ module PageObjects
"chat-cancel-selection-btn"
when "move"
"chat-move-to-channel-btn"
when "delete"
"chat-delete-btn"
end
end