diff --git a/plugins/chat/app/models/chat/channel.rb b/plugins/chat/app/models/chat/channel.rb index 9749be0e56c..76af00560cc 100644 --- a/plugins/chat/app/models/chat/channel.rb +++ b/plugins/chat/app/models/chat/channel.rb @@ -127,11 +127,12 @@ module Chat SQL end - def latest_not_deleted_message_id - DB.query_single(<<~SQL, channel_id: self.id).first + def latest_not_deleted_message_id(anchor_message_id: nil) + DB.query_single(<<~SQL, channel_id: self.id, anchor_message_id: anchor_message_id).first SELECT id FROM chat_messages WHERE chat_channel_id = :channel_id AND deleted_at IS NULL + #{anchor_message_id ? "AND id < :anchor_message_id" : ""} ORDER BY created_at DESC, id DESC LIMIT 1 SQL diff --git a/plugins/chat/app/models/chat/thread.rb b/plugins/chat/app/models/chat/thread.rb index 6730dd82f30..bb10e6111e5 100644 --- a/plugins/chat/app/models/chat/thread.rb +++ b/plugins/chat/app/models/chat/thread.rb @@ -52,15 +52,21 @@ module Chat original_message.rich_excerpt(max_length: EXCERPT_LENGTH) end - def latest_not_deleted_message_id - DB.query_single(<<~SQL, channel_id: self.channel_id, thread_id: self.id).first + def latest_not_deleted_message_id(anchor_message_id: nil) + DB.query_single( + <<~SQL, SELECT id FROM chat_messages WHERE chat_channel_id = :channel_id AND thread_id = :thread_id AND deleted_at IS NULL + #{anchor_message_id ? "AND id < :anchor_message_id" : ""} ORDER BY created_at DESC, id DESC LIMIT 1 SQL + channel_id: self.channel_id, + thread_id: self.id, + anchor_message_id: anchor_message_id, + ).first end def self.grouped_messages(thread_ids: nil, message_ids: nil, include_original_message: true) diff --git a/plugins/chat/app/services/chat/publisher.rb b/plugins/chat/app/services/chat/publisher.rb index 3458b034a1b..17512b6d312 100644 --- a/plugins/chat/app/services/chat/publisher.rb +++ b/plugins/chat/app/services/chat/publisher.rb @@ -140,9 +140,9 @@ module Chat latest_not_deleted_message_id = if chat_message.thread_reply? && chat_channel.threading_enabled && SiteSetting.enable_experimental_chat_threaded_discussions - chat_message.thread.latest_not_deleted_message_id + chat_message.thread.latest_not_deleted_message_id(anchor_message_id: chat_message.id) else - chat_channel.latest_not_deleted_message_id + chat_channel.latest_not_deleted_message_id(anchor_message_id: chat_message.id) end publish_to_targets!( message_bus_targets, diff --git a/plugins/chat/spec/services/chat/publisher_spec.rb b/plugins/chat/spec/services/chat/publisher_spec.rb index 95b43660b41..68a2c69633f 100644 --- a/plugins/chat/spec/services/chat/publisher_spec.rb +++ b/plugins/chat/spec/services/chat/publisher_spec.rb @@ -4,36 +4,47 @@ require "rails_helper" describe Chat::Publisher do fab!(:channel) { Fabricate(:category_channel) } - fab!(:message) { Fabricate(:chat_message, chat_channel: channel) } + fab!(:message_1) { Fabricate(:chat_message, chat_channel: channel) } describe ".publish_delete!" do fab!(:message_2) { Fabricate(:chat_message, chat_channel: channel) } - before { message.trash! } + before { message_2.trash! } it "publishes the correct data" do - data = MessageBus.track_publish { described_class.publish_delete!(channel, message) }[0].data + data = + MessageBus.track_publish { described_class.publish_delete!(channel, message_2) }[0].data - expect(data["deleted_at"]).to eq(message.deleted_at.iso8601(3)) - expect(data["deleted_id"]).to eq(message.id) - expect(data["latest_not_deleted_message_id"]).to eq(message_2.id) + expect(data["deleted_at"]).to eq(message_2.deleted_at.iso8601(3)) + expect(data["deleted_id"]).to eq(message_2.id) + expect(data["latest_not_deleted_message_id"]).to eq(message_1.id) expect(data["type"]).to eq("delete") end + context "when there are no earlier messages in the channel to send as latest_not_deleted_message_id" do + it "publishes nil" do + data = + MessageBus.track_publish { described_class.publish_delete!(channel, message_1) }[0].data + + expect(data["latest_not_deleted_message_id"]).to eq(nil) + end + end + context "when the message is in a thread and the channel has threading_enabled" do before do SiteSetting.enable_experimental_chat_threaded_discussions = true thread = Fabricate(:chat_thread, channel: channel) - message.update!(thread: thread) + message_1.update!(thread: thread) + message_2.update!(thread: thread) channel.update!(threading_enabled: true) end it "publishes the correct latest not deleted message id" do data = - MessageBus.track_publish { described_class.publish_delete!(channel, message) }[0].data + MessageBus.track_publish { described_class.publish_delete!(channel, message_2) }[0].data - expect(data["deleted_at"]).to eq(message.deleted_at.iso8601(3)) - expect(data["deleted_id"]).to eq(message.id) - expect(data["latest_not_deleted_message_id"]).to eq(message.thread.original_message_id) + expect(data["deleted_at"]).to eq(message_2.deleted_at.iso8601(3)) + expect(data["deleted_id"]).to eq(message_2.id) + expect(data["latest_not_deleted_message_id"]).to eq(message_1.id) expect(data["type"]).to eq("delete") end end @@ -41,9 +52,10 @@ describe Chat::Publisher do describe ".publish_refresh!" do it "publishes the message" do - data = MessageBus.track_publish { described_class.publish_refresh!(channel, message) }[0].data + data = + MessageBus.track_publish { described_class.publish_refresh!(channel, message_1) }[0].data - expect(data["chat_message"]["id"]).to eq(message.id) + expect(data["chat_message"]["id"]).to eq(message_1.id) expect(data["type"]).to eq("refresh") end end @@ -53,10 +65,10 @@ describe Chat::Publisher do before { SiteSetting.enable_experimental_chat_threaded_discussions = false } context "when the message is the original message of a thread" do - fab!(:thread) { Fabricate(:chat_thread, original_message: message, channel: channel) } + fab!(:thread) { Fabricate(:chat_thread, original_message: message_1, channel: channel) } it "generates the correct targets" do - targets = described_class.calculate_publish_targets(channel, message) + targets = described_class.calculate_publish_targets(channel, message_1) expect(targets).to contain_exactly("/chat/#{channel.id}") end end @@ -70,17 +82,17 @@ describe Chat::Publisher do ) end - before { message.update!(thread: thread) } + before { message_1.update!(thread: thread) } it "generates the correct targets" do - targets = described_class.calculate_publish_targets(channel, message) + targets = described_class.calculate_publish_targets(channel, message_1) expect(targets).to contain_exactly("/chat/#{channel.id}") end end context "when the message is not part of a thread" do it "generates the correct targets" do - targets = described_class.calculate_publish_targets(channel, message) + targets = described_class.calculate_publish_targets(channel, message_1) expect(targets).to contain_exactly("/chat/#{channel.id}") end end @@ -93,10 +105,10 @@ describe Chat::Publisher do end context "when the message is the original message of a thread" do - fab!(:thread) { Fabricate(:chat_thread, original_message: message, channel: channel) } + fab!(:thread) { Fabricate(:chat_thread, original_message: message_1, channel: channel) } it "generates the correct targets" do - targets = described_class.calculate_publish_targets(channel, message) + targets = described_class.calculate_publish_targets(channel, message_1) expect(targets).to contain_exactly("/chat/#{channel.id}") end end @@ -110,17 +122,17 @@ describe Chat::Publisher do ) end - before { message.update!(thread: thread) } + before { message_1.update!(thread: thread) } it "generates the correct targets" do - targets = described_class.calculate_publish_targets(channel, message) + targets = described_class.calculate_publish_targets(channel, message_1) expect(targets).to contain_exactly("/chat/#{channel.id}") end end context "when the message is not part of a thread" do it "generates the correct targets" do - targets = described_class.calculate_publish_targets(channel, message) + targets = described_class.calculate_publish_targets(channel, message_1) expect(targets).to contain_exactly("/chat/#{channel.id}") end end @@ -133,10 +145,10 @@ describe Chat::Publisher do end context "when the message is the original message of a thread" do - fab!(:thread) { Fabricate(:chat_thread, original_message: message, channel: channel) } + fab!(:thread) { Fabricate(:chat_thread, original_message: message_1, channel: channel) } it "generates the correct targets" do - targets = described_class.calculate_publish_targets(channel, message) + targets = described_class.calculate_publish_targets(channel, message_1) expect(targets).to contain_exactly( "/chat/#{channel.id}", "/chat/#{channel.id}/thread/#{thread.id}", @@ -153,13 +165,13 @@ describe Chat::Publisher do ) end - before { message.update!(thread: thread) } + before { message_1.update!(thread: thread) } it "generates the correct targets" do targets = described_class.calculate_publish_targets( channel, - message, + message_1, staged_thread_id: "stagedthreadid", ) @@ -179,17 +191,17 @@ describe Chat::Publisher do ) end - before { message.update!(thread: thread) } + before { message_1.update!(thread: thread) } it "generates the correct targets" do - targets = described_class.calculate_publish_targets(channel, message) + targets = described_class.calculate_publish_targets(channel, message_1) expect(targets).to contain_exactly("/chat/#{channel.id}/thread/#{thread.id}") end end context "when the message is not part of a thread" do it "generates the correct targets" do - targets = described_class.calculate_publish_targets(channel, message) + targets = described_class.calculate_publish_targets(channel, message_1) expect(targets).to contain_exactly("/chat/#{channel.id}") end end @@ -203,14 +215,14 @@ describe Chat::Publisher do it "publishes to the new_messages_message_bus_channel" do messages = MessageBus.track_publish(described_class.new_messages_message_bus_channel(channel.id)) do - described_class.publish_new!(channel, message, staged_id) + described_class.publish_new!(channel, message_1, staged_id) end expect(messages.first.data).to eq( { channel_id: channel.id, - message_id: message.id, - user_id: message.user_id, - username: message.user.username, + message_id: message_1.id, + user_id: message_1.user_id, + username: message_1.user.username, thread_id: nil, }, ) @@ -226,7 +238,7 @@ describe Chat::Publisher do ) end - before { message.update!(thread: thread) } + before { message_1.update!(thread: thread) } context "if enable_experimental_chat_threaded_discussions is false" do before { SiteSetting.enable_experimental_chat_threaded_discussions = false } @@ -235,7 +247,7 @@ describe Chat::Publisher do messages = MessageBus.track_publish( described_class.new_messages_message_bus_channel(channel.id), - ) { described_class.publish_new!(channel, message, staged_id) } + ) { described_class.publish_new!(channel, message_1, staged_id) } expect(messages).not_to be_empty end end @@ -250,7 +262,7 @@ describe Chat::Publisher do messages = MessageBus.track_publish( described_class.new_messages_message_bus_channel(channel.id), - ) { described_class.publish_new!(channel, message, staged_id) } + ) { described_class.publish_new!(channel, message_1, staged_id) } expect(messages).not_to be_empty end end @@ -262,7 +274,7 @@ describe Chat::Publisher do messages = MessageBus.track_publish( described_class.new_messages_message_bus_channel(channel.id), - ) { described_class.publish_new!(channel, message, staged_id) } + ) { described_class.publish_new!(channel, message_1, staged_id) } expect(messages).to be_empty end end diff --git a/plugins/chat/spec/system/deleted_message_spec.rb b/plugins/chat/spec/system/deleted_message_spec.rb index c32bff814ab..82d31715b07 100644 --- a/plugins/chat/spec/system/deleted_message_spec.rb +++ b/plugins/chat/spec/system/deleted_message_spec.rb @@ -42,6 +42,8 @@ RSpec.describe "Deleted message", type: :system, js: true do channel_page.delete_message(message) expect(channel_page).to have_deleted_message(message, count: 1) sidebar_component.click_link(channel_2.name) + expect(channel_page).to have_no_loading_skeleton + sidebar_component.click_link(channel_1.name) expect(channel_page).to have_deleted_message(message, count: 1) end