From f4fde4e49b03d5e8f01d02e2354c553c82f3402d Mon Sep 17 00:00:00 2001 From: Andrei Prigorshnev Date: Thu, 11 May 2023 20:05:59 +0400 Subject: [PATCH] DEV: When deleting a chat message, do not delete mention records (#21486) A chat message may be restored later, so we shouldn't be deleting `chat_mentions` records for it. But we still have to remove notifications (see 082cd139). --- plugins/chat/app/services/chat/trash_message.rb | 8 +++++--- plugins/chat/spec/services/chat/trash_message_spec.rb | 11 ++++++++--- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/plugins/chat/app/services/chat/trash_message.rb b/plugins/chat/app/services/chat/trash_message.rb index 80722b2519b..23ba5f0b212 100644 --- a/plugins/chat/app/services/chat/trash_message.rb +++ b/plugins/chat/app/services/chat/trash_message.rb @@ -22,7 +22,7 @@ module Chat policy :invalid_access transaction do step :trash_message - step :destroy_mentions + step :destroy_notifications step :update_tracking_state step :update_thread_reply_cache end @@ -53,8 +53,10 @@ module Chat message.trash! end - def destroy_mentions(message:, **) - Chat::Mention.where(chat_message: message).destroy_all + def destroy_notifications(message:, **) + ids = Chat::Mention.where(chat_message: message).pluck(:notification_id) + Notification.where(id: ids).destroy_all + Chat::Mention.where(chat_message: message).update_all(notification_id: nil) end def update_tracking_state(message:, **) diff --git a/plugins/chat/spec/services/chat/trash_message_spec.rb b/plugins/chat/spec/services/chat/trash_message_spec.rb index f102ee62224..30eedb5760d 100644 --- a/plugins/chat/spec/services/chat/trash_message_spec.rb +++ b/plugins/chat/spec/services/chat/trash_message_spec.rb @@ -43,10 +43,15 @@ RSpec.describe Chat::TrashMessage do expect(Chat::Message.find_by(id: message.id)).to be_nil end - it "destroys associated mentions" do - mention = Fabricate(:chat_mention, chat_message: message) + it "destroys notifications for mentions" do + notification = Fabricate(:notification) + mention = Fabricate(:chat_mention, chat_message: message, notification: notification) + result - expect(Chat::Mention.find_by(id: mention.id)).to be_nil + + mention = Chat::Mention.find_by(id: mention.id) + expect(mention).to be_present + expect(mention.notification_id).to be_nil end it "publishes associated Discourse and MessageBus events" do