diff --git a/plugins/chat/app/models/chat/channel.rb b/plugins/chat/app/models/chat/channel.rb index f002185a8a6..9749be0e56c 100644 --- a/plugins/chat/app/models/chat/channel.rb +++ b/plugins/chat/app/models/chat/channel.rb @@ -127,6 +127,16 @@ module Chat SQL end + def latest_not_deleted_message_id + DB.query_single(<<~SQL, channel_id: self.id).first + SELECT id FROM chat_messages + WHERE chat_channel_id = :channel_id + AND deleted_at IS NULL + ORDER BY created_at DESC, id DESC + LIMIT 1 + SQL + end + private def change_status(acting_user, target_status) diff --git a/plugins/chat/app/models/chat/thread.rb b/plugins/chat/app/models/chat/thread.rb index 3c245be26ce..6730dd82f30 100644 --- a/plugins/chat/app/models/chat/thread.rb +++ b/plugins/chat/app/models/chat/thread.rb @@ -52,6 +52,17 @@ 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 + SELECT id FROM chat_messages + WHERE chat_channel_id = :channel_id + AND thread_id = :thread_id + AND deleted_at IS NULL + ORDER BY created_at DESC, id DESC + LIMIT 1 + SQL + end + def self.grouped_messages(thread_ids: nil, message_ids: nil, include_original_message: true) DB.query(<<~SQL, message_ids: message_ids, thread_ids: thread_ids) SELECT thread_id, diff --git a/plugins/chat/app/services/chat/publisher.rb b/plugins/chat/app/services/chat/publisher.rb index a1348177316..3458b034a1b 100644 --- a/plugins/chat/app/services/chat/publisher.rb +++ b/plugins/chat/app/services/chat/publisher.rb @@ -137,10 +137,22 @@ module Chat def self.publish_delete!(chat_channel, chat_message) message_bus_targets = calculate_publish_targets(chat_channel, chat_message) + 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 + else + chat_channel.latest_not_deleted_message_id + end publish_to_targets!( message_bus_targets, chat_channel, - { type: "delete", deleted_id: chat_message.id, deleted_at: chat_message.deleted_at }, + { + type: "delete", + deleted_id: chat_message.id, + deleted_at: chat_message.deleted_at, + latest_not_deleted_message_id: latest_not_deleted_message_id, + }, ) end diff --git a/plugins/chat/assets/javascripts/discourse/services/chat-channel-pane-subscriptions-manager.js b/plugins/chat/assets/javascripts/discourse/services/chat-channel-pane-subscriptions-manager.js index 14d5ebc8262..cf8e85f336f 100644 --- a/plugins/chat/assets/javascripts/discourse/services/chat-channel-pane-subscriptions-manager.js +++ b/plugins/chat/assets/javascripts/discourse/services/chat-channel-pane-subscriptions-manager.js @@ -29,4 +29,11 @@ export default class ChatChannelPaneSubscriptionsManager extends ChatPaneBaseSub message.threadTitle = data.title; } } + + _afterDeleteMessage(targetMsg, data) { + if (this.model.currentUserMembership.lastReadMessageId === targetMsg.id) { + this.model.currentUserMembership.lastReadMessageId = + data.latest_not_deleted_message_id; + } + } } diff --git a/plugins/chat/assets/javascripts/discourse/services/chat-channel-thread-pane-subscriptions-manager.js b/plugins/chat/assets/javascripts/discourse/services/chat-channel-thread-pane-subscriptions-manager.js index c0ffc6aa17d..0a3114951e2 100644 --- a/plugins/chat/assets/javascripts/discourse/services/chat-channel-thread-pane-subscriptions-manager.js +++ b/plugins/chat/assets/javascripts/discourse/services/chat-channel-thread-pane-subscriptions-manager.js @@ -33,4 +33,8 @@ export default class ChatChannelThreadPaneSubscriptionsManager extends ChatPaneB handleThreadOriginalMessageUpdate() { return; } + + _afterDeleteMessage() { + // TODO (martin) Handle this once we have lastReadMessageId for thread memberships. + } } 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 180946c26d6..b87953d9c77 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 @@ -182,6 +182,8 @@ export default class ChatPaneBaseSubscriptionsManager extends Service { } else { this.messagesManager.removeMessage(targetMsg); } + + this._afterDeleteMessage(targetMsg, data); } handleRestoreMessage(data) { @@ -249,4 +251,8 @@ export default class ChatPaneBaseSubscriptionsManager extends Service { handleThreadOriginalMessageUpdate() { throw "not implemented"; } + + _afterDeleteMessage() { + throw "not implemented"; + } } diff --git a/plugins/chat/spec/services/chat/publisher_spec.rb b/plugins/chat/spec/services/chat/publisher_spec.rb index 6a70614974c..95b43660b41 100644 --- a/plugins/chat/spec/services/chat/publisher_spec.rb +++ b/plugins/chat/spec/services/chat/publisher_spec.rb @@ -6,6 +6,39 @@ describe Chat::Publisher do fab!(:channel) { Fabricate(:category_channel) } fab!(:message) { Fabricate(:chat_message, chat_channel: channel) } + describe ".publish_delete!" do + fab!(:message_2) { Fabricate(:chat_message, chat_channel: channel) } + before { message.trash! } + + it "publishes the correct data" do + data = MessageBus.track_publish { described_class.publish_delete!(channel, message) }[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["type"]).to eq("delete") + 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) + 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 + + 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["type"]).to eq("delete") + end + end + end + describe ".publish_refresh!" do it "publishes the message" do data = MessageBus.track_publish { described_class.publish_refresh!(channel, message) }[0].data diff --git a/plugins/chat/spec/services/chat/trash_message_spec.rb b/plugins/chat/spec/services/chat/trash_message_spec.rb index 30eedb5760d..a88388aca54 100644 --- a/plugins/chat/spec/services/chat/trash_message_spec.rb +++ b/plugins/chat/spec/services/chat/trash_message_spec.rb @@ -66,6 +66,7 @@ RSpec.describe Chat::TrashMessage do "type" => "delete", "deleted_id" => message.id, "deleted_at" => message.reload.deleted_at.iso8601(3), + "latest_not_deleted_message_id" => nil, }, ) end diff --git a/plugins/chat/spec/system/deleted_message_spec.rb b/plugins/chat/spec/system/deleted_message_spec.rb index 7c10a55e08b..c32bff814ab 100644 --- a/plugins/chat/spec/system/deleted_message_spec.rb +++ b/plugins/chat/spec/system/deleted_message_spec.rb @@ -3,6 +3,7 @@ RSpec.describe "Deleted message", type: :system, js: true do let(:chat_page) { PageObjects::Pages::Chat.new } let(:channel_page) { PageObjects::Pages::ChatChannel.new } + let(:sidebar_component) { PageObjects::Components::Sidebar.new } fab!(:current_user) { Fabricate(:admin) } fab!(:channel_1) { Fabricate(:category_channel) } @@ -28,6 +29,22 @@ RSpec.describe "Deleted message", type: :system, js: true do count: 1, ) end + + it "does not error when coming back to the channel from another channel" do + message = Fabricate(:chat_message, chat_channel: channel_1) + channel_2 = Fabricate(:category_channel, name: "other channel") + channel_2.add(current_user) + channel_1 + .user_chat_channel_memberships + .find_by(user: current_user) + .update!(last_read_message_id: message.id) + chat_page.visit_channel(channel_1) + channel_page.delete_message(message) + expect(channel_page).to have_deleted_message(message, count: 1) + sidebar_component.click_link(channel_2.name) + sidebar_component.click_link(channel_1.name) + expect(channel_page).to have_deleted_message(message, count: 1) + end end context "when deleting multiple messages" do