From c996b7fe4b18b18e76d4208cdc9052f16c546df5 Mon Sep 17 00:00:00 2001 From: Joffrey JAFFEUX Date: Mon, 7 Aug 2023 16:34:22 +0200 Subject: [PATCH] FIX: prevents readonly mode to crash channel_messages#index (#22987) Prior to this fix `context.membership&.update!(last_viewed_at: Time.zone.now)` would generate an update statement from a GET request which is not permitted by default when in readonly mode. The usual fix in this case is to check for readonly or rescue an error, however, this common pattern of updating "last seen" or similar can be better handled in a `Schedule::Defer` block, which won't raise the `ActiveRecord::ReadOnlyError` when in readonly and will also prevent the controller to wait for this operation. --- .../chat/app/services/chat/list_channel_messages.rb | 4 +++- .../chat/api/channel_messages_controller_spec.rb | 13 +++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/plugins/chat/app/services/chat/list_channel_messages.rb b/plugins/chat/app/services/chat/list_channel_messages.rb index 6c1f5155a49..affcfabbe07 100644 --- a/plugins/chat/app/services/chat/list_channel_messages.rb +++ b/plugins/chat/app/services/chat/list_channel_messages.rb @@ -158,7 +158,9 @@ module Chat end def update_membership_last_viewed_at(guardian:, **) - context.membership&.update!(last_viewed_at: Time.zone.now) + Scheduler::Defer.later "Chat::ListChannelMessages - defer update_membership_last_viewed_at" do + context.membership&.update!(last_viewed_at: Time.zone.now) + end end end end diff --git a/plugins/chat/spec/requests/chat/api/channel_messages_controller_spec.rb b/plugins/chat/spec/requests/chat/api/channel_messages_controller_spec.rb index d91840d2342..a31ab9e5ce7 100644 --- a/plugins/chat/spec/requests/chat/api/channel_messages_controller_spec.rb +++ b/plugins/chat/spec/requests/chat/api/channel_messages_controller_spec.rb @@ -28,6 +28,19 @@ RSpec.describe Chat::Api::ChannelMessagesController do end end + context "when readonly mode" do + fab!(:message_1) { Fabricate(:chat_message, chat_channel: channel) } + + before { Discourse.enable_readonly_mode } + after { Discourse.disable_readonly_mode } + + it "works" do + get "/chat/api/channels/#{channel.id}/messages" + + expect(response.status).to eq(200) + end + end + context "when channnel doesn’t exist" do it "returns a 404" do get "/chat/api/channels/-999/messages"