From 2b4dc975511532dd15b3565c58b95899a67f482a Mon Sep 17 00:00:00 2001 From: Martin Brennan Date: Wed, 24 May 2023 12:01:41 +0200 Subject: [PATCH] FIX: Do not error if admin/owner checks target message (#21721) In the ChannelViewBuilder, we introduced a check to see if the target message exists, which errors if the message has been trashed. However if the user is the creator of the message or admin then they are able to see trashed messages, so we need to take this into account. --- .../chat/app/services/chat/channel_view_builder.rb | 7 +++++-- .../spec/services/chat/channel_view_builder_spec.rb | 12 ++++++++++++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/plugins/chat/app/services/chat/channel_view_builder.rb b/plugins/chat/app/services/chat/channel_view_builder.rb index 5bbbbdeb200..3e86f0e1f8c 100644 --- a/plugins/chat/app/services/chat/channel_view_builder.rb +++ b/plugins/chat/app/services/chat/channel_view_builder.rb @@ -65,9 +65,12 @@ module Chat guardian.can_preview_chat_channel?(channel) end - def target_message_exists(contract:, **) + def target_message_exists(contract:, guardian:, **) return true if contract.target_message_id.blank? - Chat::Message.exists?(id: contract.target_message_id) + target_message = Chat::Message.unscoped.find_by(id: contract.target_message_id) + return false if target_message.blank? + return true if !target_message.trashed? + target_message.user_id == guardian.user.id || guardian.is_staff? end def determine_threads_enabled(channel:, **) diff --git a/plugins/chat/spec/services/chat/channel_view_builder_spec.rb b/plugins/chat/spec/services/chat/channel_view_builder_spec.rb index 62487fac01c..15890292a43 100644 --- a/plugins/chat/spec/services/chat/channel_view_builder_spec.rb +++ b/plugins/chat/spec/services/chat/channel_view_builder_spec.rb @@ -228,6 +228,18 @@ RSpec.describe Chat::ChannelViewBuilder do before { message.trash! } it { is_expected.to fail_a_policy(:target_message_exists) } + + context "when the user is the owner of the trashed message" do + before { message.update!(user: current_user) } + + it { is_expected.not_to fail_a_policy(:target_message_exists) } + end + + context "when the user is admin" do + before { current_user.update!(admin: true) } + + it { is_expected.not_to fail_a_policy(:target_message_exists) } + end end end end