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.
This commit is contained in:
Joffrey JAFFEUX 2023-08-07 16:34:22 +02:00 committed by GitHub
parent b8d6c9bd45
commit c996b7fe4b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 16 additions and 1 deletions

View File

@ -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

View File

@ -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 doesnt exist" do
it "returns a 404" do
get "/chat/api/channels/-999/messages"