FIX: Delete associated notifications when trashing chat messages. (#20144)

Deleting a message with a mention doesn't clear the associated notification, confusing the mentioned user.

There are different chat notification types, but we only care about `chat_mentioned` since `chat_quoted` is associated with a post, and `chat_message` is only for push notifications.

Unfortunately, this change doesn't fix the chat bubble getting out of sync when a message gets deleted since we track unread/mentions count with an integer, making it a bit hard to manipulate. We can follow up later if we consider it necessary.

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)
+    ChatMessageDestroyer.new.trash_message(@message, current_user)
-    if updated
+
-      ChatPublisher.publish_delete!(@chat_channel, @message)
+    head :ok
-      render json: success_json
-    else
-      render_json_error(@message)
-    end
   end
 
   def restore

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

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)

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

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