From 429a7d09e2059a55181d5ca77f8a9ba6c1220e44 Mon Sep 17 00:00:00 2001 From: Andrei Prigorshnev Date: Tue, 30 Jan 2024 14:37:11 +0000 Subject: [PATCH] FIX: Chat messages exporter (#25461) We usually don't enforce foreign key relationships on the database level. Because of that, occasionally it's possible to see a chat message that references to a non-existent chat_channel or user. MessagesExporter failed in such case before, this PR fixes that. --- plugins/chat/lib/chat/messages_exporter.rb | 8 ++-- .../spec/lib/chat/messages_exporter_spec.rb | 38 +++++++++++++++++-- 2 files changed, 38 insertions(+), 8 deletions(-) diff --git a/plugins/chat/lib/chat/messages_exporter.rb b/plugins/chat/lib/chat/messages_exporter.rb index 7b04cddc36b..2702d660169 100644 --- a/plugins/chat/lib/chat/messages_exporter.rb +++ b/plugins/chat/lib/chat/messages_exporter.rb @@ -48,10 +48,10 @@ module Chat yield( [ chat_message.id, - chat_message.chat_channel.id, - chat_message.chat_channel.name, - chat_message.user.id, - chat_message.user.username, + chat_message.chat_channel&.id, + chat_message.chat_channel&.name, + chat_message.user&.id, + chat_message.user&.username, chat_message.message, chat_message.cooked, chat_message.created_at, diff --git a/plugins/chat/spec/lib/chat/messages_exporter_spec.rb b/plugins/chat/spec/lib/chat/messages_exporter_spec.rb index 5e67bc9d44f..6895dcbf71b 100644 --- a/plugins/chat/spec/lib/chat/messages_exporter_spec.rb +++ b/plugins/chat/spec/lib/chat/messages_exporter_spec.rb @@ -56,13 +56,43 @@ describe Chat::MessagesExporter do end end + context "with corrupted data" do + fab!(:message) { Fabricate(:chat_message) } + + it "exports a message even its chat_channel doesn't exist" do + nonexistent_channel_id = Chat::Channel.pluck(:id).max + 1 + message.chat_channel_id = nonexistent_channel_id + message.save! + + exporter = Class.new.extend(Chat::MessagesExporter) + result = [] + exporter.chat_message_export { |data_row| result << data_row } + + expect(result.length).to be(1) + assert_exported_message(result[0], message) + end + + it "exports a message even its author doesn't exist" do + nonexistent_user_id = User.pluck(:id).max + 1 + message.user_id = nonexistent_user_id + message.save! + + exporter = Class.new.extend(Chat::MessagesExporter) + result = [] + exporter.chat_message_export { |data_row| result << data_row } + + expect(result.length).to be(1) + assert_exported_message(result[0], message) + end + end + def assert_exported_message(data_row, message) Chat::Channel.unscoped do expect(data_row[0]).to eq(message.id) - expect(data_row[1]).to eq(message.chat_channel.id) - expect(data_row[2]).to eq(message.chat_channel.name) - expect(data_row[3]).to eq(message.user.id) - expect(data_row[4]).to eq(message.user.username) + expect(data_row[1]).to eq(message.chat_channel&.id) + expect(data_row[2]).to eq(message.chat_channel&.name) + expect(data_row[3]).to eq(message.user&.id) + expect(data_row[4]).to eq(message.user&.username) expect(data_row[5]).to eq(message.message) expect(data_row[6]).to eq(message.cooked) expect(data_row[7]).to eq_time(message.created_at)