From 2befff5101d27e1ccaaf3a7ab1d44f14fee2271a Mon Sep 17 00:00:00 2001 From: Joffrey JAFFEUX Date: Fri, 24 Nov 2023 11:46:00 +0100 Subject: [PATCH] FIX: nullifies target message id when not readable (#24540) This bug was very reproducible when your last read was a message you didn't read and an admin would delete it. When coming back to the channel you would get a not found, in this case we will now reset last read and present you the last message of the channel. We could be more fancy and try to detect the next readable message but that would be more code and complexity for such a rare case. --- plugins/chat/app/services/chat/list_channel_messages.rb | 9 ++++++++- .../spec/services/chat/list_channel_messages_spec.rb | 4 +++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/plugins/chat/app/services/chat/list_channel_messages.rb b/plugins/chat/app/services/chat/list_channel_messages.rb index 7fd4f2093f5..e5faca0095e 100644 --- a/plugins/chat/app/services/chat/list_channel_messages.rb +++ b/plugins/chat/app/services/chat/list_channel_messages.rb @@ -84,11 +84,18 @@ module Chat def target_message_exists(channel:, guardian:, **) return true if context.target_message_id.blank? + target_message = Chat::Message.with_deleted.find_by(id: context.target_message_id, chat_channel: channel) return false if target_message.blank? + return true if !target_message.trashed? - target_message.user_id == guardian.user.id || guardian.is_staff? + if target_message.trashed? && target_message.user_id == guardian.user.id || guardian.is_staff? + return true + end + + context.target_message_id = nil + true end def fetch_messages(channel:, contract:, guardian:, enabled_threads:, **) diff --git a/plugins/chat/spec/services/chat/list_channel_messages_spec.rb b/plugins/chat/spec/services/chat/list_channel_messages_spec.rb index e8dbb97dfcf..794519c502f 100644 --- a/plugins/chat/spec/services/chat/list_channel_messages_spec.rb +++ b/plugins/chat/spec/services/chat/list_channel_messages_spec.rb @@ -109,7 +109,9 @@ RSpec.describe Chat::ListChannelMessages do before { target_message.trash! } context "when user is regular" do - it { is_expected.to fail_a_policy(:target_message_exists) } + it "nullifies target_message_id" do + expect(result.target_message_id).to be_blank + end end context "when user is the message creator" do