FIX: prevents exception when publishing processed (#21104)

This regression happened in bd5c5c4b5f 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. -->
This commit is contained in:
Joffrey JAFFEUX 2023-04-16 10:30:33 +02:00 committed by GitHub
parent dd98ecb7d1
commit 274820d247
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 47 additions and 1 deletions

View File

@ -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: {

View File

@ -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);
}
}

View File

@ -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