diff --git a/plugins/chat/app/controllers/chat/api/channel_messages_controller.rb b/plugins/chat/app/controllers/chat/api/channel_messages_controller.rb index c57b52820a9..102ffaf0970 100644 --- a/plugins/chat/app/controllers/chat/api/channel_messages_controller.rb +++ b/plugins/chat/app/controllers/chat/api/channel_messages_controller.rb @@ -4,29 +4,51 @@ class Chat::Api::ChannelMessagesController < Chat::ApiController def index with_service(::Chat::ListChannelMessages) do on_success { render_serialized(result, ::Chat::MessagesSerializer, root: false) } + on_failure { render(json: failed_json, status: 422) } on_failed_policy(:can_view_channel) { raise Discourse::InvalidAccess } on_failed_policy(:target_message_exists) { raise Discourse::NotFound } on_model_not_found(:channel) { raise Discourse::NotFound } + on_failed_contract do |contract| + render(json: failed_json.merge(errors: contract.errors.full_messages), status: 400) + end end end def destroy - with_service(Chat::TrashMessage) { on_model_not_found(:message) { raise Discourse::NotFound } } + with_service(Chat::TrashMessage) do + on_success { render(json: success_json) } + on_failure { render(json: failed_json, status: 422) } + on_model_not_found(:message) { raise Discourse::NotFound } + on_failed_policy(:invalid_access) { raise Discourse::InvalidAccess } + on_failed_contract do |contract| + render(json: failed_json.merge(errors: contract.errors.full_messages), status: 400) + end + end end def restore with_service(Chat::RestoreMessage) do + on_success { render(json: success_json) } + on_failure { render(json: failed_json, status: 422) } + on_failed_policy(:invalid_access) { raise Discourse::InvalidAccess } on_model_not_found(:message) { raise Discourse::NotFound } + on_failed_contract do |contract| + render(json: failed_json.merge(errors: contract.errors.full_messages), status: 400) + end end end def update with_service(Chat::UpdateMessage) do on_success { render json: success_json.merge(message_id: result[:message].id) } + on_failure { render(json: failed_json, status: 422) } on_model_not_found(:message) { raise Discourse::NotFound } on_model_errors(:message) do |model| render_json_error(model.errors.map(&:full_message).join(", ")) end + on_failed_contract do |contract| + render(json: failed_json.merge(errors: contract.errors.full_messages), status: 400) + end end end @@ -38,6 +60,7 @@ class Chat::Api::ChannelMessagesController < Chat::ApiController with_service(Chat::CreateMessage) do on_success { render json: success_json.merge(message_id: result[:message_instance].id) } + on_failure { render(json: failed_json, status: 422) } on_failed_policy(:no_silenced_user) { raise Discourse::InvalidAccess } on_model_not_found(:channel) { raise Discourse::NotFound } on_failed_policy(:allowed_to_join_channel) { raise Discourse::InvalidAccess } @@ -55,6 +78,9 @@ class Chat::Api::ChannelMessagesController < Chat::ApiController on_model_errors(:message_instance) do |model| render_json_error(model.errors.map(&:full_message).join(", ")) end + on_failed_contract do |contract| + render(json: failed_json.merge(errors: contract.errors.full_messages), status: 400) + end end end end diff --git a/plugins/chat/app/controllers/chat/api/channel_thread_messages_controller.rb b/plugins/chat/app/controllers/chat/api/channel_thread_messages_controller.rb index 324069d9f8c..28a303fdb6e 100644 --- a/plugins/chat/app/controllers/chat/api/channel_thread_messages_controller.rb +++ b/plugins/chat/app/controllers/chat/api/channel_thread_messages_controller.rb @@ -12,11 +12,14 @@ class Chat::Api::ChannelThreadMessagesController < Chat::ApiController include_thread_original_message: false, ) end - + on_failure { render(json: failed_json, status: 422) } on_failed_policy(:ensure_thread_enabled) { raise Discourse::NotFound } on_failed_policy(:target_message_exists) { raise Discourse::NotFound } on_failed_policy(:can_view_thread) { raise Discourse::InvalidAccess } on_model_not_found(:thread) { raise Discourse::NotFound } + on_failed_contract do |contract| + render(json: failed_json.merge(errors: contract.errors.full_messages), status: 400) + end end end end diff --git a/plugins/chat/app/controllers/chat/api/channel_threads_controller.rb b/plugins/chat/app/controllers/chat/api/channel_threads_controller.rb index b21f6c58618..7252f25d69b 100644 --- a/plugins/chat/app/controllers/chat/api/channel_threads_controller.rb +++ b/plugins/chat/app/controllers/chat/api/channel_threads_controller.rb @@ -22,6 +22,10 @@ class Chat::Api::ChannelThreadsController < Chat::ApiController on_failed_policy(:can_view_channel) { raise Discourse::InvalidAccess } on_model_not_found(:channel) { raise Discourse::NotFound } on_model_not_found(:threads) { render json: success_json.merge(threads: []) } + on_failure { render(json: failed_json, status: 422) } + on_failed_contract do |contract| + render(json: failed_json.merge(errors: contract.errors.full_messages), status: 400) + end end end @@ -38,9 +42,13 @@ class Chat::Api::ChannelThreadsController < Chat::ApiController participants: result.participants, ) end - + on_failed_policy(:invalid_access) { raise Discourse::InvalidAccess } on_failed_policy(:threading_enabled_for_channel) { raise Discourse::NotFound } on_model_not_found(:thread) { raise Discourse::NotFound } + on_failure { render(json: failed_json, status: 422) } + on_failed_contract do |contract| + render(json: failed_json.merge(errors: contract.errors.full_messages), status: 400) + end end end @@ -53,6 +61,11 @@ class Chat::Api::ChannelThreadsController < Chat::ApiController on_failed_step(:update) do render json: failed_json.merge(errors: [result["result.step.update"].error]), status: 422 end + on_success { render(json: success_json) } + on_failure { render(json: failed_json, status: 422) } + on_failed_contract do |contract| + render(json: failed_json.merge(errors: contract.errors.full_messages), status: 400) + end end end @@ -73,6 +86,10 @@ class Chat::Api::ChannelThreadsController < Chat::ApiController render json: failed_json.merge(errors: [result["result.step.create_thread"].error]), status: 422 end + on_failure { render(json: failed_json, status: 422) } + on_failed_contract do |contract| + render(json: failed_json.merge(errors: contract.errors.full_messages), status: 400) + end end end end diff --git a/plugins/chat/app/controllers/chat/api/channel_threads_current_user_notifications_settings_controller.rb b/plugins/chat/app/controllers/chat/api/channel_threads_current_user_notifications_settings_controller.rb index e54d5665450..63b40652bf7 100644 --- a/plugins/chat/app/controllers/chat/api/channel_threads_current_user_notifications_settings_controller.rb +++ b/plugins/chat/app/controllers/chat/api/channel_threads_current_user_notifications_settings_controller.rb @@ -13,6 +13,10 @@ class Chat::Api::ChannelThreadsCurrentUserNotificationsSettingsController < Chat root: "membership", ) end + on_failure { render(json: failed_json, status: 422) } + on_failed_contract do |contract| + render(json: failed_json.merge(errors: contract.errors.full_messages), status: 400) + end end end end diff --git a/plugins/chat/app/controllers/chat/api/channels_controller.rb b/plugins/chat/app/controllers/chat/api/channels_controller.rb index 17d85dcb73f..b37b7899d43 100644 --- a/plugins/chat/app/controllers/chat/api/channels_controller.rb +++ b/plugins/chat/app/controllers/chat/api/channels_controller.rb @@ -34,7 +34,13 @@ class Chat::Api::ChannelsController < Chat::ApiController def destroy with_service Chat::TrashChannel do + on_failed_policy(:invalid_access) { raise Discourse::InvalidAccess } on_model_not_found(:channel) { raise ActiveRecord::RecordNotFound } + on_success { render(json: success_json) } + on_failure { render(json: failed_json, status: 422) } + on_failed_contract do |contract| + render(json: failed_json.merge(errors: contract.errors.full_messages), status: 400) + end end end @@ -75,6 +81,10 @@ class Chat::Api::ChannelsController < Chat::ApiController on_model_errors(:membership) do render_json_error(result.membership, type: :record_invalid, status: 422) end + on_failure { render(json: failed_json, status: 422) } + on_failed_contract do |contract| + render(json: failed_json.merge(errors: contract.errors.full_messages), status: 400) + end end end diff --git a/plugins/chat/app/controllers/chat/api/channels_current_user_membership_controller.rb b/plugins/chat/app/controllers/chat/api/channels_current_user_membership_controller.rb index 3ee9ac0af07..4d327e2e640 100644 --- a/plugins/chat/app/controllers/chat/api/channels_current_user_membership_controller.rb +++ b/plugins/chat/app/controllers/chat/api/channels_current_user_membership_controller.rb @@ -12,6 +12,13 @@ class Chat::Api::ChannelsCurrentUserMembershipController < Chat::Api::ChannelsCo end def destroy - with_service(Chat::LeaveChannel) { on_model_not_found(:channel) { raise Discourse::NotFound } } + with_service(Chat::LeaveChannel) do + on_success { render(json: success_json) } + on_failure { render(json: failed_json, status: 422) } + on_model_not_found(:channel) { raise Discourse::NotFound } + on_failed_contract do |contract| + render(json: failed_json.merge(errors: contract.errors.full_messages), status: 400) + end + end end end diff --git a/plugins/chat/app/controllers/chat/api/channels_current_user_membership_follows_controller.rb b/plugins/chat/app/controllers/chat/api/channels_current_user_membership_follows_controller.rb index cff2138e44d..64fbb27641a 100644 --- a/plugins/chat/app/controllers/chat/api/channels_current_user_membership_follows_controller.rb +++ b/plugins/chat/app/controllers/chat/api/channels_current_user_membership_follows_controller.rb @@ -11,6 +11,10 @@ class Chat::Api::ChannelsCurrentUserMembershipFollowsController < Chat::Api::Cha ) end on_model_not_found(:channel) { raise Discourse::NotFound } + on_failure { render(json: failed_json, status: 422) } + on_failed_contract do |contract| + render(json: failed_json.merge(errors: contract.errors.full_messages), status: 400) + end end end end diff --git a/plugins/chat/app/controllers/chat/api/channels_drafts_controller.rb b/plugins/chat/app/controllers/chat/api/channels_drafts_controller.rb index 6c2a51f543b..1f8a46187b8 100644 --- a/plugins/chat/app/controllers/chat/api/channels_drafts_controller.rb +++ b/plugins/chat/app/controllers/chat/api/channels_drafts_controller.rb @@ -2,6 +2,13 @@ class Chat::Api::ChannelsDraftsController < Chat::ApiController def create - with_service(Chat::UpsertDraft) { on_model_not_found(:channel) { raise Discourse::NotFound } } + with_service(Chat::UpsertDraft) do + on_success { render(json: success_json) } + on_failure { render(json: failed_json, status: 422) } + on_model_not_found(:channel) { raise Discourse::NotFound } + on_failed_contract do |contract| + render(json: failed_json.merge(errors: contract.errors.full_messages), status: 400) + end + end end end diff --git a/plugins/chat/app/controllers/chat/api/channels_invites_controller.rb b/plugins/chat/app/controllers/chat/api/channels_invites_controller.rb index fafdee8bac4..00b9b8a4cdb 100644 --- a/plugins/chat/app/controllers/chat/api/channels_invites_controller.rb +++ b/plugins/chat/app/controllers/chat/api/channels_invites_controller.rb @@ -3,8 +3,13 @@ class Chat::Api::ChannelsInvitesController < Chat::ApiController def create with_service(Chat::InviteUsersToChannel) do + on_success { render(json: success_json) } + on_failure { render(json: failed_json, status: 422) } on_failed_policy(:can_view_channel) { raise Discourse::InvalidAccess } on_model_not_found(:channel) { raise Discourse::NotFound } + on_failed_contract do |contract| + render(json: failed_json.merge(errors: contract.errors.full_messages), status: 400) + end end end end diff --git a/plugins/chat/app/controllers/chat/api/channels_memberships_controller.rb b/plugins/chat/app/controllers/chat/api/channels_memberships_controller.rb index a60769128e7..14ece64edcf 100644 --- a/plugins/chat/app/controllers/chat/api/channels_memberships_controller.rb +++ b/plugins/chat/app/controllers/chat/api/channels_memberships_controller.rb @@ -31,9 +31,14 @@ class Chat::Api::ChannelsMembershipsController < Chat::Api::ChannelsController def create with_service(Chat::AddUsersToChannel) do + on_success { render(json: success_json) } + on_failure { render(json: failed_json, status: 422) } on_failed_policy(:can_add_users_to_channel) do render_json_error(I18n.t("chat.errors.users_cant_be_added_to_channel")) end + on_failed_contract do |contract| + render(json: failed_json.merge(errors: contract.errors.full_messages), status: 400) + end end end end diff --git a/plugins/chat/app/controllers/chat/api/channels_messages_flags_controller.rb b/plugins/chat/app/controllers/chat/api/channels_messages_flags_controller.rb index 448c1ce2d75..03ae2b34a33 100644 --- a/plugins/chat/app/controllers/chat/api/channels_messages_flags_controller.rb +++ b/plugins/chat/app/controllers/chat/api/channels_messages_flags_controller.rb @@ -5,8 +5,13 @@ class Chat::Api::ChannelsMessagesFlagsController < Chat::ApiController RateLimiter.new(current_user, "flag_chat_message", 4, 1.minutes).performed! with_service(Chat::FlagMessage) do + on_success { render(json: success_json) } + on_failure { render(json: failed_json, status: 422) } on_model_not_found(:message) { raise Discourse::NotFound } on_failed_policy(:can_flag_message_in_channel) { raise Discourse::InvalidAccess } + on_failed_contract do |contract| + render(json: failed_json.merge(errors: contract.errors.full_messages), status: 400) + end end end end diff --git a/plugins/chat/app/controllers/chat/api/channels_messages_streaming_controller.rb b/plugins/chat/app/controllers/chat/api/channels_messages_streaming_controller.rb index cbbde2c393c..fed94d60342 100644 --- a/plugins/chat/app/controllers/chat/api/channels_messages_streaming_controller.rb +++ b/plugins/chat/app/controllers/chat/api/channels_messages_streaming_controller.rb @@ -3,9 +3,14 @@ class Chat::Api::ChannelsMessagesStreamingController < Chat::Api::ChannelsController def destroy with_service(Chat::StopMessageStreaming) do + on_success { render(json: success_json) } + on_failure { render(json: failed_json, status: 422) } on_model_not_found(:message) { raise Discourse::NotFound } on_failed_policy(:can_join_channel) { raise Discourse::InvalidAccess } on_failed_policy(:can_stop_streaming) { raise Discourse::InvalidAccess } + on_failed_contract do |contract| + render(json: failed_json.merge(errors: contract.errors.full_messages), status: 400) + end end end end diff --git a/plugins/chat/app/controllers/chat/api/channels_status_controller.rb b/plugins/chat/app/controllers/chat/api/channels_status_controller.rb index 38e83f23104..3b683c31c8b 100644 --- a/plugins/chat/app/controllers/chat/api/channels_status_controller.rb +++ b/plugins/chat/app/controllers/chat/api/channels_status_controller.rb @@ -6,6 +6,10 @@ class Chat::Api::ChannelsStatusController < Chat::Api::ChannelsController on_success { render_serialized(result.channel, Chat::ChannelSerializer, root: "channel") } on_model_not_found(:channel) { raise ActiveRecord::RecordNotFound } on_failed_policy(:check_channel_permission) { raise Discourse::InvalidAccess } + on_failure { render(json: failed_json, status: 422) } + on_failed_contract do |contract| + render(json: failed_json.merge(errors: contract.errors.full_messages), status: 400) + end end end end diff --git a/plugins/chat/app/controllers/chat/api/channels_threads_drafts_controller.rb b/plugins/chat/app/controllers/chat/api/channels_threads_drafts_controller.rb index a0366c5d21f..558c367e182 100644 --- a/plugins/chat/app/controllers/chat/api/channels_threads_drafts_controller.rb +++ b/plugins/chat/app/controllers/chat/api/channels_threads_drafts_controller.rb @@ -3,8 +3,13 @@ class Chat::Api::ChannelsThreadsDraftsController < Chat::ApiController def create with_service(Chat::UpsertDraft) do + on_success { render(json: success_json) } + on_failure { render(json: failed_json, status: 422) } on_model_not_found(:channel) { raise Discourse::NotFound } on_failed_step(:check_thread_exists) { raise Discourse::NotFound } + on_failed_contract do |contract| + render(json: failed_json.merge(errors: contract.errors.full_messages), status: 400) + end end end end diff --git a/plugins/chat/app/controllers/chat/api/chatables_controller.rb b/plugins/chat/app/controllers/chat/api/chatables_controller.rb index 7940a7221e5..bd48a585e2b 100644 --- a/plugins/chat/app/controllers/chat/api/chatables_controller.rb +++ b/plugins/chat/app/controllers/chat/api/chatables_controller.rb @@ -6,6 +6,10 @@ class Chat::Api::ChatablesController < Chat::ApiController def index with_service(::Chat::SearchChatable) do on_success { render_serialized(result, ::Chat::ChatablesSerializer, root: false) } + on_failure { render(json: failed_json, status: 422) } + on_failed_contract do |contract| + render(json: failed_json.merge(errors: contract.errors.full_messages), status: 400) + end end end end diff --git a/plugins/chat/app/controllers/chat/api/current_user_threads_controller.rb b/plugins/chat/app/controllers/chat/api/current_user_threads_controller.rb index 5a2b5577990..871ed8aacdf 100644 --- a/plugins/chat/app/controllers/chat/api/current_user_threads_controller.rb +++ b/plugins/chat/app/controllers/chat/api/current_user_threads_controller.rb @@ -19,6 +19,10 @@ class Chat::Api::CurrentUserThreadsController < Chat::ApiController ) end on_model_not_found(:threads) { render json: success_json.merge(threads: []) } + on_failure { render(json: failed_json, status: 422) } + on_failed_contract do |contract| + render(json: failed_json.merge(errors: contract.errors.full_messages), status: 400) + end end end end diff --git a/plugins/chat/app/controllers/chat/api/direct_messages_controller.rb b/plugins/chat/app/controllers/chat/api/direct_messages_controller.rb index fde4637d776..3dd8940d497 100644 --- a/plugins/chat/app/controllers/chat/api/direct_messages_controller.rb +++ b/plugins/chat/app/controllers/chat/api/direct_messages_controller.rb @@ -30,6 +30,10 @@ class Chat::Api::DirectMessagesController < Chat::ApiController on_model_errors(:channel) do |model| render_json_error(model, type: :record_invalid, status: 422) end + on_failure { render(json: failed_json, status: 422) } + on_failed_contract do |contract| + render(json: failed_json.merge(errors: contract.errors.full_messages), status: 400) + end end end end diff --git a/plugins/chat/app/controllers/chat/api/reads_controller.rb b/plugins/chat/app/controllers/chat/api/reads_controller.rb index 99a8dff412b..7d25138ac5d 100644 --- a/plugins/chat/app/controllers/chat/api/reads_controller.rb +++ b/plugins/chat/app/controllers/chat/api/reads_controller.rb @@ -5,12 +5,18 @@ class Chat::Api::ReadsController < Chat::ApiController params.require(%i[channel_id message_id]) with_service(Chat::UpdateUserLastRead) do + on_success { render(json: success_json) } + on_failure { render(json: failed_json, status: 422) } on_failed_policy(:ensure_message_id_recency) do raise Discourse::InvalidParameters.new(:message_id) end on_model_not_found(:message) { raise Discourse::NotFound } on_model_not_found(:active_membership) { raise Discourse::NotFound } on_model_not_found(:channel) { raise Discourse::NotFound } + on_failed_policy(:invalid_access) { raise Discourse::InvalidAccess } + on_failed_contract do |contract| + render(json: failed_json.merge(errors: contract.errors.full_messages), status: 400) + end end end @@ -19,6 +25,7 @@ class Chat::Api::ReadsController < Chat::ApiController on_success do render(json: success_json.merge(updated_memberships: result.updated_memberships)) end + on_failure { render(json: failed_json, status: 422) } end end end diff --git a/plugins/chat/app/controllers/chat/api/thread_reads_controller.rb b/plugins/chat/app/controllers/chat/api/thread_reads_controller.rb index dfb72d9b70f..66620d7ddff 100644 --- a/plugins/chat/app/controllers/chat/api/thread_reads_controller.rb +++ b/plugins/chat/app/controllers/chat/api/thread_reads_controller.rb @@ -5,7 +5,13 @@ class Chat::Api::ThreadReadsController < Chat::ApiController params.require(%i[channel_id thread_id]) with_service(Chat::UpdateUserThreadLastRead) do + on_success { render(json: success_json) } + on_failure { render(json: failed_json, status: 422) } on_model_not_found(:thread) { raise Discourse::NotFound } + on_failed_policy(:invalid_access) { raise Discourse::InvalidAccess } + on_failed_contract do |contract| + render(json: failed_json.merge(errors: contract.errors.full_messages), status: 400) + end end end end diff --git a/plugins/chat/app/controllers/chat/api_controller.rb b/plugins/chat/app/controllers/chat/api_controller.rb index 11fda64aabc..a3d0018d5ea 100644 --- a/plugins/chat/app/controllers/chat/api_controller.rb +++ b/plugins/chat/app/controllers/chat/api_controller.rb @@ -3,18 +3,5 @@ module Chat class ApiController < ::Chat::BaseController include Chat::WithServiceHelper - - private - - def default_actions_for_service - proc do - on_success { render(json: success_json) } - on_failure { render(json: failed_json, status: 422) } - on_failed_policy(:invalid_access) { raise Discourse::InvalidAccess } - on_failed_contract do |contract| - render(json: failed_json.merge(errors: contract.errors.full_messages), status: 400) - end - end - end end end diff --git a/plugins/chat/app/controllers/chat/incoming_webhooks_controller.rb b/plugins/chat/app/controllers/chat/incoming_webhooks_controller.rb index 52fe03e589f..2180da1fca1 100644 --- a/plugins/chat/app/controllers/chat/incoming_webhooks_controller.rb +++ b/plugins/chat/app/controllers/chat/incoming_webhooks_controller.rb @@ -63,6 +63,7 @@ module Chat incoming_chat_webhook: webhook, ) do on_success { render json: success_json } + on_failure { render(json: failed_json, status: 422) } on_failed_contract do |contract| raise Discourse::InvalidParameters.new(contract.errors.full_messages) end diff --git a/plugins/chat/app/helpers/chat/with_service_helper.rb b/plugins/chat/app/helpers/chat/with_service_helper.rb index 7647ced6206..7d273a2ac1e 100644 --- a/plugins/chat/app/helpers/chat/with_service_helper.rb +++ b/plugins/chat/app/helpers/chat/with_service_helper.rb @@ -8,14 +8,14 @@ module Chat # @param service [Class] A class including {Chat::Service::Base} # @param dependencies [kwargs] Any additional params to load into the service context, # in addition to controller @params. - def with_service(service, default_actions: true, **dependencies, &block) + def with_service(service, **dependencies, &block) object = self - merged_block = - proc do - instance_exec(&object.method(:default_actions_for_service).call) if default_actions - instance_exec(&(block || proc {})) - end - ServiceRunner.call(service, object, **dependencies, &merged_block) + ServiceRunner.call( + service, + object, + **dependencies, + &proc { instance_exec(&(block || proc {})) } + ) end def run_service(service, dependencies) @@ -24,9 +24,5 @@ module Chat @_result = service.call(params.to_unsafe_h.merge(guardian: self.try(:guardian) || nil, **dependencies)) end - - def default_actions_for_service - proc {} - end end end diff --git a/plugins/chat/app/jobs/regular/chat/auto_join_channel_batch.rb b/plugins/chat/app/jobs/regular/chat/auto_join_channel_batch.rb index a11211a57dd..9e82569c10f 100644 --- a/plugins/chat/app/jobs/regular/chat/auto_join_channel_batch.rb +++ b/plugins/chat/app/jobs/regular/chat/auto_join_channel_batch.rb @@ -5,6 +5,7 @@ module Jobs class AutoJoinChannelBatch < ServiceJob def execute(args) with_service(::Chat::AutoJoinChannelBatch, **args) do + on_failure { Rails.logger.error("Failed with unexpected error") } on_failed_contract do |contract| Rails.logger.error(contract.errors.full_messages.join(", ")) end diff --git a/plugins/chat/app/services/chat/lookup_channel_threads.rb b/plugins/chat/app/services/chat/lookup_channel_threads.rb index fa6efbd8a97..d20549efb00 100644 --- a/plugins/chat/app/services/chat/lookup_channel_threads.rb +++ b/plugins/chat/app/services/chat/lookup_channel_threads.rb @@ -42,6 +42,13 @@ module Chat validates :channel_id, presence: true attribute :limit, :integer + validates :limit, + numericality: { + less_than_or_equal_to: THREADS_LIMIT, + only_integer: true, + }, + allow_nil: true + attribute :offset, :integer end diff --git a/plugins/chat/lib/service_runner.rb b/plugins/chat/lib/service_runner.rb index 78ecff46998..7dbec6e7a67 100644 --- a/plugins/chat/lib/service_runner.rb +++ b/plugins/chat/lib/service_runner.rb @@ -27,8 +27,6 @@ # argument to their block. `on_model_errors` receives the actual model so it’s # easier to inspect it. # -# Default actions for each of these are defined in [Chat::ApiController#default_actions_for_service] -# # @example In a controller # def create # with_service MyService do 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 47e83919c8e..6f1393fdab5 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,14 @@ RSpec.describe Chat::Api::ChannelMessagesController do end end + context "when params are invalid" do + it "returns a 400" do + get "/chat/api/channels/#{channel.id}/messages?page_size=9999" + + expect(response.status).to eq(400) + end + end + context "when readonly mode" do fab!(:message_1) { Fabricate(:chat_message, chat_channel: channel) } diff --git a/plugins/chat/spec/requests/chat/api/channel_thread_messages_controller_spec.rb b/plugins/chat/spec/requests/chat/api/channel_thread_messages_controller_spec.rb index d3f052fcc00..0e5c38b9299 100644 --- a/plugins/chat/spec/requests/chat/api/channel_thread_messages_controller_spec.rb +++ b/plugins/chat/spec/requests/chat/api/channel_thread_messages_controller_spec.rb @@ -73,5 +73,13 @@ RSpec.describe Chat::Api::ChannelThreadMessagesController do expect(response.status).to eq(404) end end + + context "when params are invalid" do + it "returns a 400" do + get "/chat/api/channels/#{thread.channel.id}/threads/#{thread.id}/messages?page_size=9999" + + expect(response.status).to eq(400) + end + end end end diff --git a/plugins/chat/spec/requests/chat/api/channel_threads_controller_spec.rb b/plugins/chat/spec/requests/chat/api/channel_threads_controller_spec.rb index d766da3deff..6c2a432d951 100644 --- a/plugins/chat/spec/requests/chat/api/channel_threads_controller_spec.rb +++ b/plugins/chat/spec/requests/chat/api/channel_threads_controller_spec.rb @@ -168,6 +168,13 @@ RSpec.describe Chat::Api::ChannelThreadsController do expect(response.status).to eq(404) end end + + context "when params are invalid" do + it "returns a 400" do + get "/chat/api/channels/#{public_channel.id}/threads?limit=9999" + expect(response.status).to eq(400) + end + end end describe "update" do diff --git a/plugins/chat/spec/services/chat/lookup_channel_threads_spec.rb b/plugins/chat/spec/services/chat/lookup_channel_threads_spec.rb index 19146ec5ff0..d8a03d962fe 100644 --- a/plugins/chat/spec/services/chat/lookup_channel_threads_spec.rb +++ b/plugins/chat/spec/services/chat/lookup_channel_threads_spec.rb @@ -1,7 +1,13 @@ # frozen_string_literal: true RSpec.describe ::Chat::LookupChannelThreads::Contract, type: :model do - it { is_expected.to validate_presence_of :channel_id } + subject(:contract) { described_class.new(channel_id: 1) } + + it { is_expected.to validate_presence_of(:channel_id) } + it { is_expected.to allow_values(1, 0, nil, "a").for(:limit) } + it do + is_expected.not_to allow_values(::Chat::LookupChannelThreads::THREADS_LIMIT + 1).for(:limit) + end end RSpec.describe ::Chat::LookupChannelThreads do @@ -30,9 +36,7 @@ RSpec.describe ::Chat::LookupChannelThreads do context "when limit is over max" do let(:limit) { described_class::THREADS_LIMIT + 1 } - it "defaults to a max value" do - expect(result.limit).to eq(described_class::THREADS_LIMIT) - end + it { is_expected.to fail_a_contract } end context "when limit is under min" do