From e94707acdf4595052c64ceddadce1c8870c639ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Guitaut?= Date: Tue, 3 Sep 2024 18:30:22 +0200 Subject: [PATCH] DEV: Drop `WithServiceHelper` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This patch removes the `with_service` helper from the code base. Instead, we can pass a block with actions directly to the `.call` method of a service. This simplifies how to use services: - use `.call` without a block to run the service and get its result object. - use `.call` with a block of actions to run the service and execute arbitrary code depending on the service outcome. It also means a service is now “self-contained” and can be used anywhere without having to include a helper or whatever. --- app/controllers/admin/admin_controller.rb | 2 - .../admin/config/about_controller.rb | 4 +- .../admin/config/flags_controller.rb | 10 ++-- .../admin/site_settings_controller.rb | 10 ++-- app/controllers/admin/staff_controller.rb | 2 - app/controllers/admin/users_controller.rb | 10 +--- app/helpers/with_service_helper.rb | 27 ----------- app/jobs/service_job.rb | 9 ---- app/services/service/base.rb | 5 +- lib/service_runner.rb | 48 ++++++++++++------- .../chat/api/channel_messages_controller.rb | 16 +++---- .../api/channel_thread_messages_controller.rb | 2 +- .../chat/api/channel_threads_controller.rb | 8 ++-- ..._user_notifications_settings_controller.rb | 2 +- ...rrent_user_title_prompt_seen_controller.rb | 2 +- .../chat/api/channels_controller.rb | 9 ++-- ...nels_current_user_membership_controller.rb | 2 +- ...rent_user_membership_follows_controller.rb | 2 +- .../chat/api/channels_drafts_controller.rb | 2 +- .../chat/api/channels_invites_controller.rb | 2 +- .../api/channels_memberships_controller.rb | 2 +- .../api/channels_messages_flags_controller.rb | 2 +- .../channels_messages_streaming_controller.rb | 2 +- .../chat/api/channels_read_controller.rb | 4 +- .../chat/api/channels_status_controller.rb | 2 +- .../api/channels_threads_drafts_controller.rb | 2 +- .../api/channels_threads_read_controller.rb | 2 +- .../chat/api/chatables_controller.rb | 2 +- .../api/current_user_channels_controller.rb | 2 +- .../api/current_user_threads_controller.rb | 2 +- .../chat/api/direct_messages_controller.rb | 2 +- .../app/controllers/chat/api_controller.rb | 1 - .../chat/incoming_webhooks_controller.rb | 5 +- .../regular/chat/auto_join_channel_batch.rb | 6 +-- plugins/chat/lib/chat_sdk/channel.rb | 12 +---- plugins/chat/lib/chat_sdk/message.rb | 23 ++++----- plugins/chat/lib/chat_sdk/thread.rb | 12 +---- spec/lib/service_runner_spec.rb | 8 ++-- spec/rails_helper.rb | 1 - 39 files changed, 99 insertions(+), 167 deletions(-) delete mode 100644 app/helpers/with_service_helper.rb delete mode 100644 app/jobs/service_job.rb diff --git a/app/controllers/admin/admin_controller.rb b/app/controllers/admin/admin_controller.rb index 2595961acf9..2220a511e77 100644 --- a/app/controllers/admin/admin_controller.rb +++ b/app/controllers/admin/admin_controller.rb @@ -1,8 +1,6 @@ # frozen_string_literal: true class Admin::AdminController < ApplicationController - include WithServiceHelper - requires_login before_action :ensure_admin diff --git a/app/controllers/admin/config/about_controller.rb b/app/controllers/admin/config/about_controller.rb index f98046e97b9..0e0cb0acd3a 100644 --- a/app/controllers/admin/config/about_controller.rb +++ b/app/controllers/admin/config/about_controller.rb @@ -36,8 +36,8 @@ class Admin::Config::AboutController < Admin::AdminController end settings_map.each do |name, value| - with_service( - UpdateSiteSetting, + UpdateSiteSetting.call( + guardian: guardian, setting_name: name, new_value: value, allow_changing_hidden: %i[ diff --git a/app/controllers/admin/config/flags_controller.rb b/app/controllers/admin/config/flags_controller.rb index 4d0915953cf..2389be1127b 100644 --- a/app/controllers/admin/config/flags_controller.rb +++ b/app/controllers/admin/config/flags_controller.rb @@ -2,7 +2,7 @@ class Admin::Config::FlagsController < Admin::AdminController def toggle - with_service(Flags::ToggleFlag) do + Flags::ToggleFlag.call do on_success do Discourse.request_refresh! render(json: success_json) @@ -26,7 +26,7 @@ class Admin::Config::FlagsController < Admin::AdminController end def create - with_service(Flags::CreateFlag) do + Flags::CreateFlag.call do on_success do Discourse.request_refresh! render json: result.flag, serializer: FlagSerializer, used_flag_ids: Flag.used_flag_ids @@ -40,7 +40,7 @@ class Admin::Config::FlagsController < Admin::AdminController end def update - with_service(Flags::UpdateFlag) do + Flags::UpdateFlag.call do on_success do Discourse.request_refresh! render json: result.flag, serializer: FlagSerializer, used_flag_ids: Flag.used_flag_ids @@ -57,7 +57,7 @@ class Admin::Config::FlagsController < Admin::AdminController end def reorder - with_service(Flags::ReorderFlag) do + Flags::ReorderFlag.call do on_success do Discourse.request_refresh! render(json: success_json) @@ -73,7 +73,7 @@ class Admin::Config::FlagsController < Admin::AdminController end def destroy - with_service(Flags::DestroyFlag) do + Flags::DestroyFlag.call do on_success do Discourse.request_refresh! render(json: success_json) diff --git a/app/controllers/admin/site_settings_controller.rb b/app/controllers/admin/site_settings_controller.rb index 94f8d7faf76..45c2b9c7ec9 100644 --- a/app/controllers/admin/site_settings_controller.rb +++ b/app/controllers/admin/site_settings_controller.rb @@ -39,18 +39,16 @@ class Admin::SiteSettingsController < Admin::AdminController previous_value = value_or_default(SiteSetting.get(id)) if update_existing_users - with_service(UpdateSiteSetting, setting_name: id, new_value: value) do + UpdateSiteSetting.call(setting_name: id, new_value: value) do on_success do - value = result.new_value - SiteSettingUpdateExistingUsers.call(id, value, previous_value) if update_existing_users - + if update_existing_users + SiteSettingUpdateExistingUsers.call(id, result.new_value, previous_value) + end render body: nil end - on_failed_policy(:setting_is_visible) do raise Discourse::InvalidParameters, I18n.t("errors.site_settings.site_setting_is_hidden") end - on_failed_policy(:setting_is_configurable) do raise Discourse::InvalidParameters, I18n.t("errors.site_settings.site_setting_is_unconfigurable") diff --git a/app/controllers/admin/staff_controller.rb b/app/controllers/admin/staff_controller.rb index 39205fdff34..01d18fc92c4 100644 --- a/app/controllers/admin/staff_controller.rb +++ b/app/controllers/admin/staff_controller.rb @@ -1,8 +1,6 @@ # frozen_string_literal: true class Admin::StaffController < ApplicationController - include WithServiceHelper - requires_login before_action :ensure_staff end diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb index 1cdf4428bcf..e2c9ad1386c 100644 --- a/app/controllers/admin/users_controller.rb +++ b/app/controllers/admin/users_controller.rb @@ -119,7 +119,7 @@ class Admin::UsersController < Admin::StaffController end def suspend - with_service(SuspendUser, user: @user) do + SuspendUser.call(user: @user) do on_success do render_json_dump( suspension: { @@ -131,9 +131,7 @@ class Admin::UsersController < Admin::StaffController }, ) end - on_failed_policy(:can_suspend) { raise Discourse::InvalidAccess.new } - on_failed_policy(:not_suspended_already) do suspend_record = @user.suspend_record message = @@ -149,7 +147,6 @@ class Admin::UsersController < Admin::StaffController ) render json: failed_json.merge(message: message), status: 409 end - on_failed_contract do |contract| render json: failed_json.merge(errors: contract.errors.full_messages), status: 400 end @@ -328,7 +325,7 @@ class Admin::UsersController < Admin::StaffController end def silence - with_service(SilenceUser, user: @user) do + SilenceUser.call(user: @user) do on_success do render_json_dump( silence: { @@ -340,9 +337,7 @@ class Admin::UsersController < Admin::StaffController }, ) end - on_failed_policy(:can_silence) { raise Discourse::InvalidAccess.new } - on_failed_policy(:not_silenced_already) do silenced_record = @user.silenced_record message = @@ -358,7 +353,6 @@ class Admin::UsersController < Admin::StaffController ) render json: failed_json.merge(message: message), status: 409 end - on_failed_contract do |contract| render json: failed_json.merge(errors: contract.errors.full_messages), status: 400 end diff --git a/app/helpers/with_service_helper.rb b/app/helpers/with_service_helper.rb deleted file mode 100644 index c7e36ee5f66..00000000000 --- a/app/helpers/with_service_helper.rb +++ /dev/null @@ -1,27 +0,0 @@ -# frozen_string_literal: true - -module WithServiceHelper - def result - @_result - end - - # @param service [Class] A class including {Service::Base} - # @param dependencies [kwargs] Any additional params to load into the service context, - # in addition to controller @params. - def with_service(service, **dependencies, &block) - object = self - ServiceRunner.call( - service, - object, - **dependencies, - &proc { instance_exec(&(block || proc {})) } - ) - end - - def run_service(service, dependencies) - params = self.try(:params) || ActionController::Parameters.new - - @_result = - service.call(params.to_unsafe_h.merge(guardian: self.try(:guardian) || nil, **dependencies)) - end -end diff --git a/app/jobs/service_job.rb b/app/jobs/service_job.rb deleted file mode 100644 index 3f2ca9aa598..00000000000 --- a/app/jobs/service_job.rb +++ /dev/null @@ -1,9 +0,0 @@ -# frozen_string_literal: true - -class ServiceJob < ::Jobs::Base - include WithServiceHelper - - def run_service(service, dependencies) - @_result = service.call(dependencies) - end -end diff --git a/app/services/service/base.rb b/app/services/service/base.rb index 08f60c6a487..fee2cff02e2 100644 --- a/app/services/service/base.rb +++ b/app/services/service/base.rb @@ -230,8 +230,9 @@ module Service class_methods do include StepsHelpers - def call(context = {}) - new(context).tap(&:run).context + def call(context = {}, &actions) + return new(context).tap(&:run).context unless block_given? + ServiceRunner.call(self, context, &actions) end def call!(context = {}) diff --git a/lib/service_runner.rb b/lib/service_runner.rb index 05c9f507709..a18593f7fe4 100644 --- a/lib/service_runner.rb +++ b/lib/service_runner.rb @@ -2,11 +2,11 @@ # # = ServiceRunner # -# This class is to be used via its helper +with_service+ in any class. Its -# main purpose is to ease how actions can be run upon a service completion. -# Since a service will likely return the same kind of things over and over, -# this allows us to not have to repeat the same boilerplate code in every -# object. +# This class is automatically used when passing a block to the `.call` method +# of a service. Its main purpose is to ease how actions can be run upon a +# service completion. Since a service will likely return the same kind of +# things over and over, this allows us to not have to repeat the same +# boilerplate code in every object. # # There are several available actions and we can add new ones very easily: # @@ -29,7 +29,7 @@ # # @example In a controller # def create -# with_service MyService do +# MyService.call do # on_success do # flash[:notice] = "Success!" # redirect_to a_path @@ -39,19 +39,18 @@ # end # end # -# @example In a job (inheriting from +ServiceJob+) -# def execute(args = {}) -# with_service(MyService, **args) do +# @example In a job +# def execute(*) +# MyService.call(*) do # on_success { Rails.logger.info "SUCCESS" } # on_failure { Rails.logger.error "FAILURE" } # end # end # # The actions will be evaluated in the order they appear. So even if the -# service will ultimately fail with a failed policy, in this example only the -# +on_failed_policy+ action will be executed and not the +on_failure+ one. -# The only exception to this being +on_failure+ as it will always be executed -# last. +# service ultimately fails with a failed policy, in this example only the +# +on_failed_policy+ action will be executed and not the +on_failure+ one. The +# only exception to this being +on_failure+ as it will always be executed last. # class ServiceRunner @@ -101,7 +100,7 @@ class ServiceRunner delegate :result, to: :object # @!visibility private - def initialize(service, object, **dependencies) + def initialize(service, object, dependencies) @service = service @object = object @dependencies = dependencies @@ -109,16 +108,17 @@ class ServiceRunner end # @param service [Class] a class including {Service::Base} + # @param dependencies [Hash] dependencies to be provided to the service # @param block [Proc] a block containing the steps to match on # @return [void] - def self.call(service, object, **dependencies, &block) - new(service, object, **dependencies).call(&block) + def self.call(service, dependencies = {}, &block) + new(service, block.binding.eval("self"), dependencies).call(&block) end # @!visibility private def call(&block) instance_eval(&block) - object.run_service(service, dependencies) + setup_and_run_service # Always have `on_failure` as the last action ( actions @@ -132,8 +132,20 @@ class ServiceRunner attr_reader :actions + def setup_and_run_service + runner = self + params = object.try(:params) || ActionController::Parameters.new + object.instance_eval do + def result = @_result + @_result = + runner.service.call( + params.to_unsafe_h.merge(guardian: try(:guardian), **runner.dependencies), + ) + end + end + def failure_for?(key) - object.result[key]&.failure? + result[key]&.failure? end def add_action(name, *args, &block) 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 42ca5c8f727..a715ed9b111 100644 --- a/plugins/chat/app/controllers/chat/api/channel_messages_controller.rb +++ b/plugins/chat/app/controllers/chat/api/channel_messages_controller.rb @@ -2,7 +2,7 @@ class Chat::Api::ChannelMessagesController < Chat::ApiController def index - with_service(::Chat::ListChannelMessages) do + ::Chat::ListChannelMessages.call 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 } @@ -15,7 +15,7 @@ class Chat::Api::ChannelMessagesController < Chat::ApiController end def destroy - with_service(Chat::TrashMessage) do + Chat::TrashMessage.call do on_success { render(json: success_json) } on_failure { render(json: failed_json, status: 422) } on_model_not_found(:message) { raise Discourse::NotFound } @@ -27,7 +27,7 @@ class Chat::Api::ChannelMessagesController < Chat::ApiController end def bulk_destroy - with_service(Chat::TrashMessages) do + Chat::TrashMessages.call do on_success { render(json: success_json) } on_failure { render(json: failed_json, status: 422) } on_model_not_found(:messages) { raise Discourse::NotFound } @@ -39,7 +39,7 @@ class Chat::Api::ChannelMessagesController < Chat::ApiController end def restore - with_service(Chat::RestoreMessage) do + Chat::RestoreMessage.call do on_success { render(json: success_json) } on_failure { render(json: failed_json, status: 422) } on_failed_policy(:invalid_access) { raise Discourse::InvalidAccess } @@ -51,7 +51,7 @@ class Chat::Api::ChannelMessagesController < Chat::ApiController end def update - with_service(Chat::UpdateMessage) do + Chat::UpdateMessage.call 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 } @@ -65,12 +65,10 @@ class Chat::Api::ChannelMessagesController < Chat::ApiController end def create - # users can't force a thread through JSON API - params[:force_thread] = false - Chat::MessageRateLimiter.run!(current_user) - with_service(Chat::CreateMessage) do + # users can't force a thread through JSON API + Chat::CreateMessage.call(force_thread: false) 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 } 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 befbdb2ec69..b487b77bb8e 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 @@ -2,7 +2,7 @@ class Chat::Api::ChannelThreadMessagesController < Chat::ApiController def index - with_service(::Chat::ListChannelThreadMessages) do + ::Chat::ListChannelThreadMessages.call do on_success do render_serialized( result, 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 7252f25d69b..98588d34479 100644 --- a/plugins/chat/app/controllers/chat/api/channel_threads_controller.rb +++ b/plugins/chat/app/controllers/chat/api/channel_threads_controller.rb @@ -2,7 +2,7 @@ class Chat::Api::ChannelThreadsController < Chat::ApiController def index - with_service(::Chat::LookupChannelThreads) do + ::Chat::LookupChannelThreads.call do on_success do render_serialized( ::Chat::ThreadsView.new( @@ -30,7 +30,7 @@ class Chat::Api::ChannelThreadsController < Chat::ApiController end def show - with_service(::Chat::LookupThread) do + ::Chat::LookupThread.call do on_success do render_serialized( result.thread, @@ -53,7 +53,7 @@ class Chat::Api::ChannelThreadsController < Chat::ApiController end def update - with_service(::Chat::UpdateThread) do + ::Chat::UpdateThread.call do on_failed_policy(:threading_enabled_for_channel) { raise Discourse::NotFound } on_failed_policy(:can_view_channel) { raise Discourse::InvalidAccess } on_failed_policy(:can_edit_thread) { raise Discourse::InvalidAccess } @@ -70,7 +70,7 @@ class Chat::Api::ChannelThreadsController < Chat::ApiController end def create - with_service(::Chat::CreateThread) do + ::Chat::CreateThread.call do on_success do render_serialized( result.thread, 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 63b40652bf7..73008eddcb0 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 @@ -2,7 +2,7 @@ class Chat::Api::ChannelThreadsCurrentUserNotificationsSettingsController < Chat::ApiController def update - with_service(Chat::UpdateThreadNotificationSettings) do + Chat::UpdateThreadNotificationSettings.call do on_failed_policy(:threading_enabled_for_channel) { raise Discourse::NotFound } on_failed_policy(:can_view_channel) { raise Discourse::InvalidAccess } on_model_not_found(:thread) { raise Discourse::NotFound } diff --git a/plugins/chat/app/controllers/chat/api/channel_threads_current_user_title_prompt_seen_controller.rb b/plugins/chat/app/controllers/chat/api/channel_threads_current_user_title_prompt_seen_controller.rb index eac3244c5fb..7f276856745 100644 --- a/plugins/chat/app/controllers/chat/api/channel_threads_current_user_title_prompt_seen_controller.rb +++ b/plugins/chat/app/controllers/chat/api/channel_threads_current_user_title_prompt_seen_controller.rb @@ -2,7 +2,7 @@ class Chat::Api::ChannelThreadsCurrentUserTitlePromptSeenController < Chat::ApiController def update - with_service(Chat::MarkThreadTitlePromptSeen) do + Chat::MarkThreadTitlePromptSeen.call do on_failed_policy(:threading_enabled_for_channel) { raise Discourse::NotFound } on_failed_policy(:can_view_channel) { raise Discourse::InvalidAccess } on_model_not_found(:thread) { raise Discourse::NotFound } diff --git a/plugins/chat/app/controllers/chat/api/channels_controller.rb b/plugins/chat/app/controllers/chat/api/channels_controller.rb index b37b7899d43..5ea7a649699 100644 --- a/plugins/chat/app/controllers/chat/api/channels_controller.rb +++ b/plugins/chat/app/controllers/chat/api/channels_controller.rb @@ -33,7 +33,7 @@ class Chat::Api::ChannelsController < Chat::ApiController end def destroy - with_service Chat::TrashChannel do + Chat::TrashChannel.call do on_failed_policy(:invalid_access) { raise Discourse::InvalidAccess } on_model_not_found(:channel) { raise ActiveRecord::RecordNotFound } on_success { render(json: success_json) } @@ -58,9 +58,8 @@ class Chat::Api::ChannelsController < Chat::ApiController # NOTE: We don't allow creating channels for anything but category chatable types # at the moment. This may change in future, at which point we will need to pass in # a chatable_type param as well and switch to the correct service here. - with_service( - Chat::CreateCategoryChannel, - **channel_params.merge(category_id: channel_params[:chatable_id]), + Chat::CreateCategoryChannel.call( + channel_params.merge(category_id: channel_params[:chatable_id]), ) do on_success do render_serialized( @@ -105,7 +104,7 @@ class Chat::Api::ChannelsController < Chat::ApiController auto_join_limiter(channel_from_params).performed! end - with_service(Chat::UpdateChannel, **params_to_edit) do + Chat::UpdateChannel.call(params_to_edit) do on_success do render_serialized( result.channel, 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 4d327e2e640..00f5e6e51e0 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,7 +12,7 @@ class Chat::Api::ChannelsCurrentUserMembershipController < Chat::Api::ChannelsCo end def destroy - with_service(Chat::LeaveChannel) do + Chat::LeaveChannel.call do on_success { render(json: success_json) } on_failure { render(json: failed_json, status: 422) } on_model_not_found(:channel) { raise Discourse::NotFound } 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 64fbb27641a..47f0dcaeb06 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 @@ -2,7 +2,7 @@ class Chat::Api::ChannelsCurrentUserMembershipFollowsController < Chat::Api::ChannelsController def destroy - with_service(Chat::UnfollowChannel) do + Chat::UnfollowChannel.call do on_success do render_serialized( result.membership, 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 1f8a46187b8..997a4f3336e 100644 --- a/plugins/chat/app/controllers/chat/api/channels_drafts_controller.rb +++ b/plugins/chat/app/controllers/chat/api/channels_drafts_controller.rb @@ -2,7 +2,7 @@ class Chat::Api::ChannelsDraftsController < Chat::ApiController def create - with_service(Chat::UpsertDraft) do + Chat::UpsertDraft.call do on_success { render(json: success_json) } on_failure { render(json: failed_json, status: 422) } on_model_not_found(:channel) { raise Discourse::NotFound } 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 00b9b8a4cdb..f5b1469df5b 100644 --- a/plugins/chat/app/controllers/chat/api/channels_invites_controller.rb +++ b/plugins/chat/app/controllers/chat/api/channels_invites_controller.rb @@ -2,7 +2,7 @@ class Chat::Api::ChannelsInvitesController < Chat::ApiController def create - with_service(Chat::InviteUsersToChannel) do + Chat::InviteUsersToChannel.call do on_success { render(json: success_json) } on_failure { render(json: failed_json, status: 422) } on_failed_policy(:can_view_channel) { raise Discourse::InvalidAccess } 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 0a805df1a53..116ec7a7629 100644 --- a/plugins/chat/app/controllers/chat/api/channels_memberships_controller.rb +++ b/plugins/chat/app/controllers/chat/api/channels_memberships_controller.rb @@ -30,7 +30,7 @@ class Chat::Api::ChannelsMembershipsController < Chat::Api::ChannelsController end def create - with_service(Chat::AddUsersToChannel) do + Chat::AddUsersToChannel.call do on_success { render(json: success_json) } on_failure { render(json: failed_json, status: 422) } on_failed_policy(:can_add_users_to_channel) do 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 03ae2b34a33..09429d52046 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 @@ -4,7 +4,7 @@ class Chat::Api::ChannelsMessagesFlagsController < Chat::ApiController def create RateLimiter.new(current_user, "flag_chat_message", 4, 1.minutes).performed! - with_service(Chat::FlagMessage) do + Chat::FlagMessage.call do on_success { render(json: success_json) } on_failure { render(json: failed_json, status: 422) } on_model_not_found(:message) { raise Discourse::NotFound } 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 af2241f5a9f..930ffd1d777 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 @@ -2,7 +2,7 @@ class Chat::Api::ChannelsMessagesStreamingController < Chat::Api::ChannelsController def destroy - with_service(Chat::StopMessageStreaming) do + Chat::StopMessageStreaming.call do on_success { render(json: success_json) } on_failure { render(json: failed_json, status: 422) } on_model_not_found(:message) { raise Discourse::NotFound } diff --git a/plugins/chat/app/controllers/chat/api/channels_read_controller.rb b/plugins/chat/app/controllers/chat/api/channels_read_controller.rb index 0d34f09af13..115e4b61cfb 100644 --- a/plugins/chat/app/controllers/chat/api/channels_read_controller.rb +++ b/plugins/chat/app/controllers/chat/api/channels_read_controller.rb @@ -2,7 +2,7 @@ class Chat::Api::ChannelsReadController < Chat::ApiController def update - with_service(Chat::UpdateUserChannelLastRead) do + Chat::UpdateUserChannelLastRead.call do on_success { render(json: success_json) } on_failure { render(json: failed_json, status: 422) } on_failed_policy(:ensure_message_id_recency) do @@ -19,7 +19,7 @@ class Chat::Api::ChannelsReadController < Chat::ApiController end def update_all - with_service(Chat::MarkAllUserChannelsRead) do + Chat::MarkAllUserChannelsRead.call do on_success do render(json: success_json.merge(updated_memberships: result.updated_memberships)) 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 3b683c31c8b..de25b2d6027 100644 --- a/plugins/chat/app/controllers/chat/api/channels_status_controller.rb +++ b/plugins/chat/app/controllers/chat/api/channels_status_controller.rb @@ -2,7 +2,7 @@ class Chat::Api::ChannelsStatusController < Chat::Api::ChannelsController def update - with_service(Chat::UpdateChannelStatus) do + Chat::UpdateChannelStatus.call do 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 } 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 558c367e182..24ebb9bd93d 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 @@ -2,7 +2,7 @@ class Chat::Api::ChannelsThreadsDraftsController < Chat::ApiController def create - with_service(Chat::UpsertDraft) do + Chat::UpsertDraft.call do on_success { render(json: success_json) } on_failure { render(json: failed_json, status: 422) } on_model_not_found(:channel) { raise Discourse::NotFound } diff --git a/plugins/chat/app/controllers/chat/api/channels_threads_read_controller.rb b/plugins/chat/app/controllers/chat/api/channels_threads_read_controller.rb index f957d06991c..cf4bcf7631c 100644 --- a/plugins/chat/app/controllers/chat/api/channels_threads_read_controller.rb +++ b/plugins/chat/app/controllers/chat/api/channels_threads_read_controller.rb @@ -2,7 +2,7 @@ class Chat::Api::ChannelsThreadsReadController < Chat::ApiController def update - with_service(Chat::UpdateUserThreadLastRead) do + Chat::UpdateUserThreadLastRead.call do on_success { render(json: success_json) } on_failure { render(json: failed_json, status: 422) } on_model_not_found(:thread) { raise Discourse::NotFound } diff --git a/plugins/chat/app/controllers/chat/api/chatables_controller.rb b/plugins/chat/app/controllers/chat/api/chatables_controller.rb index bd48a585e2b..b22fd62a043 100644 --- a/plugins/chat/app/controllers/chat/api/chatables_controller.rb +++ b/plugins/chat/app/controllers/chat/api/chatables_controller.rb @@ -4,7 +4,7 @@ class Chat::Api::ChatablesController < Chat::ApiController before_action :ensure_logged_in def index - with_service(::Chat::SearchChatable) do + ::Chat::SearchChatable.call do on_success { render_serialized(result, ::Chat::ChatablesSerializer, root: false) } on_failure { render(json: failed_json, status: 422) } on_failed_contract do |contract| diff --git a/plugins/chat/app/controllers/chat/api/current_user_channels_controller.rb b/plugins/chat/app/controllers/chat/api/current_user_channels_controller.rb index 02660ef5cc5..a508ac9276c 100644 --- a/plugins/chat/app/controllers/chat/api/current_user_channels_controller.rb +++ b/plugins/chat/app/controllers/chat/api/current_user_channels_controller.rb @@ -2,7 +2,7 @@ class Chat::Api::CurrentUserChannelsController < Chat::ApiController def index - with_service(Chat::ListUserChannels) do + Chat::ListUserChannels.call do on_success do render_serialized( result.structured, 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 871ed8aacdf..02de0a3fd3d 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 @@ -2,7 +2,7 @@ class Chat::Api::CurrentUserThreadsController < Chat::ApiController def index - with_service(::Chat::LookupUserThreads) do + ::Chat::LookupUserThreads.call do on_success do render_serialized( ::Chat::ThreadsView.new( 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 7a4e2d34437..62b427977f9 100644 --- a/plugins/chat/app/controllers/chat/api/direct_messages_controller.rb +++ b/plugins/chat/app/controllers/chat/api/direct_messages_controller.rb @@ -2,7 +2,7 @@ class Chat::Api::DirectMessagesController < Chat::ApiController def create - with_service(Chat::CreateDirectMessageChannel) do + Chat::CreateDirectMessageChannel.call do on_success do render_serialized( result.channel, diff --git a/plugins/chat/app/controllers/chat/api_controller.rb b/plugins/chat/app/controllers/chat/api_controller.rb index 96ac3e9bcdd..18f6f89c4dd 100644 --- a/plugins/chat/app/controllers/chat/api_controller.rb +++ b/plugins/chat/app/controllers/chat/api_controller.rb @@ -2,6 +2,5 @@ module Chat class ApiController < ::Chat::BaseController - include WithServiceHelper 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 7d5d33b0edc..a995084cce0 100644 --- a/plugins/chat/app/controllers/chat/incoming_webhooks_controller.rb +++ b/plugins/chat/app/controllers/chat/incoming_webhooks_controller.rb @@ -2,8 +2,6 @@ module Chat class IncomingWebhooksController < ::ApplicationController - include WithServiceHelper - requires_plugin Chat::PLUGIN_NAME WEBHOOK_MESSAGES_PER_MINUTE_LIMIT = 10 @@ -57,8 +55,7 @@ module Chat webhook = find_and_rate_limit_webhook(key) webhook.chat_channel.add(Discourse.system_user) - with_service( - Chat::CreateMessage, + Chat::CreateMessage.call( chat_channel_id: webhook.chat_channel_id, guardian: Discourse.system_user.guardian, message: text, 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 9e82569c10f..fb3f7e2166a 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 @@ -2,9 +2,9 @@ module Jobs module Chat - class AutoJoinChannelBatch < ServiceJob - def execute(args) - with_service(::Chat::AutoJoinChannelBatch, **args) do + class AutoJoinChannelBatch < ::Jobs::Base + def execute(*) + ::Chat::AutoJoinChannelBatch.call(*) do on_failure { Rails.logger.error("Failed with unexpected error") } on_failed_contract do |contract| Rails.logger.error(contract.errors.full_messages.join(", ")) diff --git a/plugins/chat/lib/chat_sdk/channel.rb b/plugins/chat/lib/chat_sdk/channel.rb index 23b23e75110..1bf85714a1f 100644 --- a/plugins/chat/lib/chat_sdk/channel.rb +++ b/plugins/chat/lib/chat_sdk/channel.rb @@ -2,8 +2,6 @@ module ChatSDK class Channel - include WithServiceHelper - # Retrieves messages from a specified channel. # # @param channel_id [Integer] The ID of the chat channel from which to fetch messages. @@ -14,17 +12,11 @@ module ChatSDK # ChatSDK::Channel.messages(channel_id: 1, guardian: Guardian.new) # def self.messages(channel_id:, guardian:, **params) - new.messages(channel_id: channel_id, guardian: guardian, **params) + new.messages(channel_id:, guardian:, **params) end def messages(channel_id:, guardian:, **params) - with_service( - Chat::ListChannelMessages, - channel_id: channel_id, - guardian: guardian, - **params, - direction: "future", - ) do + Chat::ListChannelMessages.call(channel_id:, guardian:, **params, direction: "future") do on_success { result.messages } on_failure { raise "Unexpected error" } on_failed_policy(:can_view_channel) { raise "Guardian can't view channel" } diff --git a/plugins/chat/lib/chat_sdk/message.rb b/plugins/chat/lib/chat_sdk/message.rb index d5808abfbf3..72f46ed44fd 100644 --- a/plugins/chat/lib/chat_sdk/message.rb +++ b/plugins/chat/lib/chat_sdk/message.rb @@ -2,8 +2,6 @@ module ChatSDK class Message - include WithServiceHelper - # Creates a new message in a chat channel. # # @param raw [String] The content of the message. @@ -91,7 +89,7 @@ module ChatSDK end def stop_stream(message_id:, guardian:) - with_service(Chat::StopMessageStreaming, message_id: message_id, guardian: guardian) do + Chat::StopMessageStreaming.call(message_id:, guardian:) do on_success { result.message } on_model_not_found(:message) { raise "Couldn't find message with id: `#{message_id}`" } on_model_not_found(:membership) do @@ -121,8 +119,7 @@ module ChatSDK &block ) message = - with_service( - Chat::CreateMessage, + Chat::CreateMessage.call( message: raw, guardian: guardian, chat_channel_id: channel_id, @@ -165,8 +162,6 @@ module ChatSDK end class StreamHelper - include WithServiceHelper - attr_reader :message attr_reader :guardian @@ -176,19 +171,17 @@ module ChatSDK end def stream(raw: nil) - return false if !self.message.streaming - return false if !raw + return false if !message.streaming || !raw - with_service( - Chat::UpdateMessage, - message_id: self.message.id, - message: self.message.message + raw, - guardian: self.guardian, + Chat::UpdateMessage.call( + message_id: message.id, + message: message.message + raw, + guardian: guardian, streaming: true, strip_whitespaces: false, ) { on_failure { raise "Unexpected error" } } - self.message + message end end end diff --git a/plugins/chat/lib/chat_sdk/thread.rb b/plugins/chat/lib/chat_sdk/thread.rb index d687169c76e..a8bfedd1cbf 100644 --- a/plugins/chat/lib/chat_sdk/thread.rb +++ b/plugins/chat/lib/chat_sdk/thread.rb @@ -2,8 +2,6 @@ module ChatSDK class Thread - include WithServiceHelper - # Updates the title of a specified chat thread. # # @param title [String] The new title for the chat thread. @@ -76,13 +74,7 @@ module ChatSDK end def messages(thread_id:, guardian:, direction: "future", **params) - with_service( - Chat::ListChannelThreadMessages, - thread_id: thread_id, - guardian: guardian, - direction: direction, - **params, - ) do + Chat::ListChannelThreadMessages.call(thread_id:, guardian:, direction:, **params) do on_success { result.messages } on_failed_policy(:can_view_thread) { raise "Guardian can't view thread" } on_failed_policy(:target_message_exists) { raise "Target message doesn't exist" } @@ -91,7 +83,7 @@ module ChatSDK end def update(**params) - with_service(Chat::UpdateThread, **params) do + Chat::UpdateThread.call(params) do on_model_not_found(:channel) do raise "Couldn’t find channel with id: `#{params[:channel_id]}`" end diff --git a/spec/lib/service_runner_spec.rb b/spec/lib/service_runner_spec.rb index bfd6eb653c4..e9dec57c8dc 100644 --- a/spec/lib/service_runner_spec.rb +++ b/spec/lib/service_runner_spec.rb @@ -148,8 +148,8 @@ RSpec.describe ServiceRunner do end end - describe ".call(service, &block)" do - subject(:runner) { described_class.call(service, object, &actions_block) } + describe ".call" do + subject(:runner) { described_class.call(service, &actions_block) } let(:result) { object.result } let(:actions_block) { object.instance_eval(actions) } @@ -158,8 +158,6 @@ RSpec.describe ServiceRunner do let(:object) do Class .new(ApplicationController) do - include WithServiceHelper - def request OpenStruct.new end @@ -450,7 +448,7 @@ RSpec.describe ServiceRunner do end context "when running in the context of a job" do - let(:object) { Class.new(ServiceJob).new } + let(:object) { Class.new(Jobs::Base).new } let(:actions) { <<-BLOCK } proc do on_success { :success } diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index 2485d266b82..214455db16b 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -214,7 +214,6 @@ RSpec.configure do |config| config.include BackupsHelpers config.include OneboxHelpers config.include FastImageHelpers - config.include WithServiceHelper config.include ServiceMatchers config.include I18nHelpers