diff --git a/plugins/chat/app/controllers/chat_controller.rb b/plugins/chat/app/controllers/chat_controller.rb index 31ad38e02a6..3ac2770f5f2 100644 --- a/plugins/chat/app/controllers/chat_controller.rb +++ b/plugins/chat/app/controllers/chat_controller.rb @@ -260,13 +260,9 @@ class Chat::ChatController < Chat::ChatBaseController def delete guardian.ensure_can_delete_chat!(@message, @chatable) - updated = @message.trash!(current_user) - if updated - ChatPublisher.publish_delete!(@chat_channel, @message) - render json: success_json - else - render_json_error(@message) - end + ChatMessageDestroyer.new.trash_message(@message, current_user) + + head :ok end def restore diff --git a/plugins/chat/app/models/chat_mention.rb b/plugins/chat/app/models/chat_mention.rb index e334acae473..649303ca0a9 100644 --- a/plugins/chat/app/models/chat_mention.rb +++ b/plugins/chat/app/models/chat_mention.rb @@ -3,7 +3,7 @@ class ChatMention < ActiveRecord::Base belongs_to :user belongs_to :chat_message - belongs_to :notification + belongs_to :notification, dependent: :destroy end # == Schema Information diff --git a/plugins/chat/app/services/chat_message_destroyer.rb b/plugins/chat/app/services/chat_message_destroyer.rb index f5f159b8169..542c968c704 100644 --- a/plugins/chat/app/services/chat_message_destroyer.rb +++ b/plugins/chat/app/services/chat_message_destroyer.rb @@ -11,6 +11,18 @@ class ChatMessageDestroyer end end + def trash_message(message, actor) + ChatMessage.transaction do + message.trash!(actor) + ChatMention.where(chat_message: message).destroy_all + + # FIXME: We should do something to prevent the blue/green bubble + # of other channel members from getting out of sync when a message + # gets deleted. + ChatPublisher.publish_delete!(message.chat_channel, message) + end + end + private def reset_last_read(message_ids) diff --git a/plugins/chat/spec/requests/chat_controller_spec.rb b/plugins/chat/spec/requests/chat_controller_spec.rb index 2b9c3e5e3b6..7c400f1c231 100644 --- a/plugins/chat/spec/requests/chat_controller_spec.rb +++ b/plugins/chat/spec/requests/chat_controller_spec.rb @@ -611,7 +611,7 @@ RSpec.describe Chat::ChatController do end before do - ChatMessage.create(user: user, message: "this is a message", chat_channel: chat_channel) + ChatMessage.create!(user: user, message: "this is a message", chat_channel: chat_channel) end describe "for category" do diff --git a/plugins/chat/spec/services/chat_message_destroyer_spec.rb b/plugins/chat/spec/services/chat_message_destroyer_spec.rb index e6016b7bca3..c991c067d8a 100644 --- a/plugins/chat/spec/services/chat_message_destroyer_spec.rb +++ b/plugins/chat/spec/services/chat_message_destroyer_spec.rb @@ -43,4 +43,48 @@ RSpec.describe ChatMessageDestroyer do expect(message_2.reload).to be_present end end + + describe "#trash_message" do + fab!(:message_1) { Fabricate(:chat_message) } + fab!(:actor) { Discourse.system_user } + + it "trashes the message" do + described_class.new.trash_message(message_1, actor) + + expect(ChatMessage.find_by(id: message_1.id)).to be_blank + expect(ChatMessage.with_deleted.find_by(id: message_1.id)).to be_present + end + + context "when the message has associated notifications" do + context "when notification has the chat_mention type" do + it "deletes associated notification and chat mention relations" do + notification = + Fabricate(:notification, notification_type: Notification.types[:chat_mention]) + chat_mention = + Fabricate(:chat_mention, chat_message: message_1, notification: notification) + + described_class.new.trash_message(message_1, actor) + + expect { notification.reload }.to raise_error(ActiveRecord::RecordNotFound) + expect { chat_mention.reload }.to raise_error(ActiveRecord::RecordNotFound) + end + end + end + + it "publishes a MB message to update clients" do + delete_message = + MessageBus + .track_publish("/chat/#{message_1.chat_channel_id}") do + described_class.new.trash_message(message_1, actor) + end + .first + + expect(delete_message).to be_present + message_data = delete_message.data + + expect(message_data[:type]).to eq("delete") + expect(message_data[:deleted_id]).to eq(message_1.id) + expect(message_data[:deleted_at]).to be_present + end + end end