From 274820d2479b16227c3bde7cc4fb0688a4e6025a Mon Sep 17 00:00:00 2001
From: Joffrey JAFFEUX <j.jaffeux@gmail.com>
Date: Sun, 16 Apr 2023 10:30:33 +0200
Subject: [PATCH] FIX: prevents exception when publishing processed (#21104)

This regression happened in https://github.com/discourse/discourse/commit/bd5c5c4b5f7b33a64cc12e2ba13e81767ac00edc and is due to `message_bus_targets = calculate_publish_targets(chat_channel, chat_message)` expecting a `chat_channel` which was only defined after.

Example exception in logs:

```
Job exception: undefined local variable or method `chat_channel' for Chat::Publisher:Module

/var/www/discourse/plugins/chat/app/services/chat/publisher.rb:91:in `publish_processed!'
/var/www/discourse/plugins/chat/app/jobs/regular/chat/process_message.rb:21:in `block in execute'
/var/www/discourse/lib/distributed_mutex.rb:53:in `block in synchronize'
/var/www/discourse/lib/distributed_mutex.rb:49:in `synchronize'
/var/www/discourse/lib/distributed_mutex.rb:49:in `synchronize'
/var/www/discourse/lib/distributed_mutex.rb:34:in `synchronize'
/var/www/discourse/plugins/chat/app/jobs/regular/chat/process_message.rb:7:in `execute'
/var/www/discourse/app/jobs/base.rb:249:in `block (2 levels) in perform'
```

This commit also:
- adds a spec to ensure oneboxing is not regressing anymore
- increment the version on message processed to ensure callbacks are correctly ran

Note we should also have more tests in `Chat::Publisher`, this will be done when we move it to a proper service.

<!-- NOTE: All pull requests should have tests (rspec in Ruby, qunit in JavaScript). If your code does not include test coverage, please include an explanation of why it was omitted. -->
---
 plugins/chat/app/services/chat/publisher.rb   |  2 +-
 .../chat-pane-base-subscriptions-manager.js   |  1 +
 .../spec/system/chat_message_onebox_spec.rb   | 45 +++++++++++++++++++
 3 files changed, 47 insertions(+), 1 deletion(-)
 create mode 100644 plugins/chat/spec/system/chat_message_onebox_spec.rb

diff --git a/plugins/chat/app/services/chat/publisher.rb b/plugins/chat/app/services/chat/publisher.rb
index 5aa87e9ad05..81e3fd4c736 100644
--- a/plugins/chat/app/services/chat/publisher.rb
+++ b/plugins/chat/app/services/chat/publisher.rb
@@ -88,9 +88,9 @@ module Chat
     end
 
     def self.publish_processed!(chat_message)
+      chat_channel = chat_message.chat_channel
       message_bus_targets = calculate_publish_targets(chat_channel, chat_message)
 
-      chat_channel = chat_message.chat_channel
       content = {
         type: :processed,
         chat_message: {
diff --git a/plugins/chat/assets/javascripts/discourse/services/chat-pane-base-subscriptions-manager.js b/plugins/chat/assets/javascripts/discourse/services/chat-pane-base-subscriptions-manager.js
index 6e102f9db5b..3a09348d6a0 100644
--- a/plugins/chat/assets/javascripts/discourse/services/chat-pane-base-subscriptions-manager.js
+++ b/plugins/chat/assets/javascripts/discourse/services/chat-pane-base-subscriptions-manager.js
@@ -142,6 +142,7 @@ export default class ChatPaneBaseSubscriptionsManager extends Service {
     const message = this.messagesManager.findMessage(data.chat_message.id);
     if (message) {
       message.cooked = data.chat_message.cooked;
+      message.incrementVersion();
       this.afterProcessedMessage(message);
     }
   }
diff --git a/plugins/chat/spec/system/chat_message_onebox_spec.rb b/plugins/chat/spec/system/chat_message_onebox_spec.rb
new file mode 100644
index 00000000000..d697ac9959a
--- /dev/null
+++ b/plugins/chat/spec/system/chat_message_onebox_spec.rb
@@ -0,0 +1,45 @@
+# frozen_string_literal: true
+
+RSpec.describe "Chat message onebox", type: :system, js: true do
+  let(:chat_page) { PageObjects::Pages::Chat.new }
+  let(:channel_page) { PageObjects::Pages::ChatChannel.new }
+
+  fab!(:current_user) { Fabricate(:user) }
+  fab!(:channel_1) { Fabricate(:category_channel) }
+
+  before do
+    chat_system_bootstrap
+    channel_1.add(current_user)
+    sign_in(current_user)
+  end
+
+  context "when sending a message with a link" do
+    before do
+      SiteSetting.enable_inline_onebox_on_all_domains = true
+
+      full_onebox_html = <<~HTML.chomp
+        <aside class="onebox wikipedia" data-onebox-src="https://en.wikipedia.org/wiki/Hyperlink">
+          <article class="onebox-body">
+            <p>This is a test</p>
+          </article>
+        </aside>
+      HTML
+      Oneboxer
+        .stubs(:cached_onebox)
+        .with("https://en.wikipedia.org/wiki/Hyperlink")
+        .returns(full_onebox_html)
+
+      stub_request(:get, "https://en.wikipedia.org/wiki/Hyperlink").to_return(
+        status: 200,
+        body: "<html><head><title>a</title></head></html>",
+      )
+    end
+
+    it "is oneboxed" do
+      chat_page.visit_channel(channel_1)
+      channel_page.send_message("https://en.wikipedia.org/wiki/Hyperlink")
+
+      expect(page).to have_content("This is a test")
+    end
+  end
+end