DEV: cooks messages synchronously (#24510)

Mentions and other post processing (like images) are still done asynchronously in the background. This should ensure reloading a channel while the message has not been processed yet doesn’t renders a blank message.

As a followup, we could probably simplify the staged message logic, given we have the new cooked on send.

plugins/chat/app/jobs/regular/chat/process_message.rb

@@ -10,6 +10,7 @@ module Jobs
         ) do
           chat_message = ::Chat::Message.find_by(id: args[:chat_message_id])
           return if !chat_message
+
           processor =
             ::Chat::MessageProcessor.new(
               chat_message,
@@ -22,34 +23,19 @@ module Jobs
               cooked: processor.html,
               cooked_version: ::Chat::Message::BAKED_VERSION,
             )
-            chat_message.upsert_mentions
-
-            if args[:edit_timestamp]
-              ::Chat::Publisher.publish_edit!(chat_message.chat_channel, chat_message)
-              ::Chat::Notifier.new(chat_message, args[:edit_timestamp]).notify_edit
-              DiscourseEvent.trigger(
-                :chat_message_edited,
-                chat_message,
-                chat_message.chat_channel,
-                chat_message.user,
-              )
-            else
-              ::Chat::Publisher.publish_new!(
-                chat_message.chat_channel,
-                chat_message,
-                args[:staged_id],
-              )
-              ::Chat::Notifier.new(chat_message, chat_message.created_at).notify_new
-              DiscourseEvent.trigger(
-                :chat_message_created,
-                chat_message,
-                chat_message.chat_channel,
-                chat_message.user,
-              )
-            end
-
-            ::Chat::Publisher.publish_processed!(chat_message)
           end
+
+          # we dont process mentions when creating/updating message so we always have to do it
+          chat_message.upsert_mentions
+
+          # notifier should be idempotent and not re-notify
+          if args[:edit_timestamp]
+            ::Chat::Notifier.new(chat_message, args[:edit_timestamp]).notify_edit
+          else
+            ::Chat::Notifier.new(chat_message, chat_message.created_at).notify_new
+          end
+
+          ::Chat::Publisher.publish_processed!(chat_message)
         end
       end
     end

plugins/chat/app/services/chat/create_message.rb

@@ -132,6 +132,8 @@ module Chat
         message: contract.message,
         uploads: uploads,
         thread: thread,
+        cooked: ::Chat::Message.cook(contract.message, user_id: guardian.user.id),
+        cooked_version: ::Chat::Message::BAKED_VERSION,
       )
     end
 
@@ -182,6 +184,14 @@ module Chat
     end
 
     def process(channel:, message_instance:, contract:, **)
+      ::Chat::Publisher.publish_new!(channel, message_instance, contract.staged_id)
+      DiscourseEvent.trigger(
+        :chat_message_created,
+        message_instance,
+        channel,
+        message_instance.user,
+      )
+
       if contract.process_inline
         Jobs::Chat::ProcessMessage.new.execute(
           { chat_message_id: message_instance.id, staged_id: contract.staged_id },

plugins/chat/app/services/chat/publisher.rb

@@ -109,7 +109,7 @@ module Chat
       publish_to_targets!(
         message_bus_targets,
         chat_channel,
-        { type: :processed, chat_message: { id: chat_message.id, cooked: chat_message.cooked } },
+        serialize_message_with_type(chat_message, :processed),
       )
     end
 

plugins/chat/app/services/chat/update_message.rb

@@ -81,6 +81,7 @@ module Chat
     def modify_message(contract:, message:, guardian:, uploads:, **)
       message.message = contract.message
       message.last_editor_id = guardian.user.id
+      message.cook
 
       return if uploads&.size != contract.upload_ids.to_a.size
 
@@ -133,6 +134,9 @@ module Chat
     def publish(message:, guardian:, contract:, **)
       edit_timestamp = context.revision&.created_at&.iso8601(6) || Time.zone.now.iso8601(6)
 
+      ::Chat::Publisher.publish_edit!(message.chat_channel, message)
+      DiscourseEvent.trigger(:chat_message_edited, message, message.chat_channel, message.user)
+
       if contract.process_inline
         Jobs::Chat::ProcessMessage.new.execute(
           { chat_message_id: message.id, edit_timestamp: edit_timestamp },

plugins/chat/assets/javascripts/discourse/lib/chat-channel-subscription-manager.js

@@ -117,6 +117,7 @@ export default class ChatChannelSubscriptionManager {
     stagedMessage.excerpt = data.chat_message.excerpt;
     stagedMessage.channel = channel;
     stagedMessage.createdAt = new Date(data.chat_message.created_at);
+    stagedMessage.cooked = data.chat_message.cooked;
 
     return stagedMessage;
   }
@@ -139,6 +140,8 @@ export default class ChatChannelSubscriptionManager {
   handleEditMessage(data) {
     const message = this.messagesManager.findMessage(data.chat_message.id);
     if (message) {
+      message.message = data.chat_message.message;
+      message.cooked = data.chat_message.cooked;
       message.excerpt = data.chat_message.excerpt;
       message.uploads = cloneJSON(data.chat_message.uploads || []);
       message.edited = data.chat_message.edited;

plugins/chat/spec/services/chat/create_message_spec.rb

@@ -43,7 +43,6 @@ RSpec.describe Chat::CreateMessage do
       end
 
       it "cooks the message" do
-        Jobs.run_immediately!
         expect(message).to be_cooked
       end
 

plugins/chat/spec/services/chat/update_message_spec.rb

@@ -138,6 +138,14 @@ RSpec.describe Chat::UpdateMessage do
       expect(chat_message.reload.message).to eq(new_message)
     end
 
+    it "cooks the message" do
+      chat_message = create_chat_message(user1, "This will be changed", public_chat_channel)
+      new_message = "Change **to** this!"
+
+      described_class.call(guardian: guardian, message_id: chat_message.id, message: new_message)
+      expect(chat_message.reload.cooked).to eq("<p>Change <strong>to</strong> this!</p>")
+    end
+
     it "publishes a DiscourseEvent for updated messages" do
       chat_message = create_chat_message(user1, "This will be changed", public_chat_channel)
       events =
@@ -293,7 +301,7 @@ RSpec.describe Chat::UpdateMessage do
                 message: new_content,
               )
             end
-            .detect { |m| m.data["type"] == "edit" }
+            .detect { |m| m.data["type"] == "processed" }
             .data
 
         expect(processed_message["chat_message"]["mentioned_users"].count).to eq(1)
@@ -319,7 +327,7 @@ RSpec.describe Chat::UpdateMessage do
                 message: "Hey @#{user2.username}",
               )
             end
-            .detect { |m| m.data["type"] == "edit" }
+            .detect { |m| m.data["type"] == "processed" }
             .data
 
         expect(processed_message["chat_message"]["mentioned_users"].count).to be(1)