From 41584ab40c59dbc95273536147ab2c29e51017f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Guitaut?= Date: Fri, 18 Oct 2024 17:45:47 +0200 Subject: [PATCH] DEV: Provide user input to services using `params` key MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently in services, we don’t make a distinction between input parameters, options and dependencies. This can lead to user input modifying the service behavior, whereas it was not the developer intention. This patch addresses the issue by changing how data is provided to services: - `params` is now used to hold all data coming from outside (typically user input from a controller) and a contract will take its values from `params`. - `options` is a new key to provide options to a service. This typically allows changing a service behavior at runtime. It is, of course, totally optional. - `dependencies` is actually anything else provided to the service (like `guardian`) and available directly from the context object. The `service_params` helper in controllers has been updated to reflect those changes, so most of the existing services didn’t need specific changes. The options block has the same DSL as contracts, as it’s also based on `ActiveModel`. There aren’t any validations, though. Here’s an example: ```ruby options do attribute :allow_changing_hidden, :boolean, default: false end ``` And here’s an example of how to call a service with the new keys: ```ruby MyService.call(params: { key1: value1, … }, options: { my_option: true }, guardian:, …) ``` --- .../admin/config/about_controller.rb | 20 +- .../admin/site_settings_controller.rb | 2 +- app/controllers/application_controller.rb | 2 +- app/services/admin_notices/dismiss.rb | 4 +- app/services/experiments/toggle.rb | 2 +- app/services/flags/destroy_flag.rb | 4 +- app/services/site_setting/update.rb | 7 +- lib/service.rb | 16 +- lib/service/base.rb | 37 +- lib/service/options_base.rb | 7 + lib/service/steps_inspector.rb | 4 + .../chat/api/channel_messages_controller.rb | 3 +- .../chat/api/channels_controller.rb | 4 +- .../chat/incoming_webhooks_controller.rb | 8 +- .../regular/chat/auto_join_channel_batch.rb | 4 +- ...move_membership_handle_category_updated.rb | 2 +- ...rship_handle_chat_allowed_groups_change.rb | 2 +- ...emove_membership_handle_destroyed_group.rb | 2 +- ...mbership_handle_user_removed_from_group.rb | 2 +- .../app/services/chat/add_users_to_channel.rb | 34 +- .../services/chat/auto_join_channel_batch.rb | 8 +- .../services/chat/create_category_channel.rb | 28 +- .../chat/create_direct_message_channel.rb | 14 +- .../chat/app/services/chat/create_message.rb | 71 ++-- .../chat/app/services/chat/create_thread.rb | 11 +- .../chat/app/services/chat/flag_message.rb | 27 +- .../services/chat/invite_users_to_channel.rb | 11 +- .../chat/app/services/chat/leave_channel.rb | 11 +- .../services/chat/list_channel_messages.rb | 7 +- .../chat/list_channel_thread_messages.rb | 7 +- .../app/services/chat/list_user_channels.rb | 4 +- .../services/chat/lookup_channel_threads.rb | 11 +- .../chat/app/services/chat/lookup_thread.rb | 9 +- .../app/services/chat/lookup_user_threads.rb | 9 +- .../chat/mark_all_user_channels_read.rb | 2 +- .../chat/mark_thread_title_prompt_seen.rb | 13 +- plugins/chat/app/services/chat/publisher.rb | 6 +- .../chat/app/services/chat/restore_message.rb | 9 +- .../chat/app/services/chat/search_chatable.rb | 7 +- .../services/chat/stop_message_streaming.rb | 7 +- .../chat/app/services/chat/tracking_state.rb | 9 +- .../chat/app/services/chat/trash_channel.rb | 11 +- .../chat/app/services/chat/trash_message.rb | 9 +- .../chat/app/services/chat/trash_messages.rb | 9 +- .../app/services/chat/unfollow_channel.rb | 11 +- .../chat/app/services/chat/update_channel.rb | 36 +- .../services/chat/update_channel_status.rb | 13 +- .../chat/app/services/chat/update_message.rb | 32 +- .../chat/app/services/chat/update_thread.rb | 10 +- .../update_thread_notification_settings.rb | 17 +- .../chat/update_user_channel_last_read.rb | 9 +- .../chat/update_user_thread_last_read.rb | 11 +- .../chat/app/services/chat/upsert_draft.rb | 18 +- plugins/chat/lib/chat/channel_fetcher.rb | 10 +- plugins/chat/lib/chat/message_mover.rb | 20 +- plugins/chat/lib/chat_sdk/channel.rb | 13 +- plugins/chat/lib/chat_sdk/message.rb | 57 +-- plugins/chat/lib/chat_sdk/thread.rb | 35 +- plugins/chat/plugin.rb | 6 +- .../chat/spec/fabricators/chat_fabricator.rb | 20 +- ...hread_replies_count_cache_accuracy_spec.rb | 16 +- .../regular/chat/notify_mentioned_spec.rb | 4 +- plugins/chat/spec/lib/chat/notifier_spec.rb | 10 +- .../chat/spec/lib/chat/review_queue_spec.rb | 6 +- plugins/chat/spec/plugin_helper.rb | 48 ++- .../handle_category_updated_spec.rb | 2 +- .../handle_chat_allowed_groups_change_spec.rb | 4 +- .../handle_destroyed_group_spec.rb | 2 +- .../handle_user_removed_from_group_spec.rb | 2 +- .../chat/add_users_to_channel_spec.rb | 7 +- .../chat/auto_join_channel_batch_spec.rb | 2 +- .../chat/create_category_channel_spec.rb | 22 +- .../create_direct_message_channel_spec.rb | 16 +- .../spec/services/chat/create_message_spec.rb | 44 ++- .../spec/services/chat/create_thread_spec.rb | 18 +- .../spec/services/chat/flag_message_spec.rb | 4 +- .../chat/invite_users_to_channel_spec.rb | 2 +- .../spec/services/chat/leave_channel_spec.rb | 8 +- .../chat/list_channel_messages_spec.rb | 5 +- .../chat/list_channel_thread_messages_spec.rb | 278 +++++++------- .../services/chat/list_user_channels_spec.rb | 5 +- .../chat/lookup_channel_threads_spec.rb | 5 +- .../spec/services/chat/lookup_thread_spec.rb | 7 +- .../services/chat/lookup_user_threads_spec.rb | 5 +- .../chat/mark_all_user_channels_read_spec.rb | 5 +- .../mark_thread_title_prompt_seen_spec.rb | 7 +- .../services/chat/restore_message_spec.rb | 14 +- .../services/chat/search_chatable_spec.rb | 16 +- .../chat/stop_message_streaming_spec.rb | 5 +- .../spec/services/chat/tracking_state_spec.rb | 10 +- .../spec/services/chat/trash_channel_spec.rb | 20 +- .../spec/services/chat/trash_message_spec.rb | 2 +- .../spec/services/chat/trash_messages_spec.rb | 2 +- .../services/chat/unfollow_channel_spec.rb | 12 +- .../spec/services/chat/update_channel_spec.rb | 4 +- .../chat/update_channel_status_spec.rb | 2 +- .../spec/services/chat/update_message_spec.rb | 343 +++++++++++------- ...pdate_thread_notification_settings_spec.rb | 4 +- .../spec/services/chat/update_thread_spec.rb | 5 +- .../update_user_channel_last_read_spec.rb | 7 +- .../chat/update_user_thread_last_read_spec.rb | 18 +- .../spec/services/chat/upsert_draft_spec.rb | 13 +- .../system/invite_users_to_channel_spec.rb | 14 +- spec/lib/service/runner_spec.rb | 10 +- spec/lib/service/steps_inspector_spec.rb | 112 +++--- spec/services/admin_notices/dismiss_spec.rb | 2 +- spec/services/experiments/toggle_spec.rb | 86 +++-- spec/services/flags/create_flag_spec.rb | 2 +- spec/services/flags/destroy_flag_spec.rb | 2 +- spec/services/flags/reorder_flag_spec.rb | 4 +- spec/services/flags/toggle_flag_spec.rb | 2 +- spec/services/flags/update_flag_spec.rb | 2 +- spec/services/site_setting/update_spec.rb | 2 +- spec/services/user/silence_spec.rb | 2 +- spec/services/user/suspend_spec.rb | 2 +- 115 files changed, 1152 insertions(+), 895 deletions(-) create mode 100644 lib/service/options_base.rb diff --git a/app/controllers/admin/config/about_controller.rb b/app/controllers/admin/config/about_controller.rb index e92b1b657d9..b3b86120b71 100644 --- a/app/controllers/admin/config/about_controller.rb +++ b/app/controllers/admin/config/about_controller.rb @@ -38,14 +38,18 @@ class Admin::Config::AboutController < Admin::AdminController settings_map.each do |name, value| SiteSetting::Update.call( guardian:, - setting_name: name, - new_value: value, - allow_changing_hidden: %i[ - extended_site_description - extended_site_description_cooked - about_banner_image - community_owner - ].include?(name), + params: { + setting_name: name, + new_value: value, + }, + options: { + allow_changing_hidden: %i[ + extended_site_description + extended_site_description_cooked + about_banner_image + community_owner + ].include?(name), + }, ) end render json: success_json diff --git a/app/controllers/admin/site_settings_controller.rb b/app/controllers/admin/site_settings_controller.rb index cd80e41d669..cde55031c65 100644 --- a/app/controllers/admin/site_settings_controller.rb +++ b/app/controllers/admin/site_settings_controller.rb @@ -39,7 +39,7 @@ class Admin::SiteSettingsController < Admin::AdminController previous_value = value_or_default(SiteSetting.get(id)) if update_existing_users - SiteSetting::Update.call(service_params.merge(setting_name: id, new_value: value)) do + SiteSetting::Update.call(params: { setting_name: id, new_value: value }, guardian:) do on_success do |contract:| if update_existing_users SiteSettingUpdateExistingUsers.call(id, contract.new_value, previous_value) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 5982404e6e4..2e1b6a74c16 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -1164,6 +1164,6 @@ class ApplicationController < ActionController::Base end def service_params - params.to_unsafe_h.merge(guardian:) + { params: params.to_unsafe_h, guardian: } end end diff --git a/app/services/admin_notices/dismiss.rb b/app/services/admin_notices/dismiss.rb index 0d2f76484e7..b4543f973ea 100644 --- a/app/services/admin_notices/dismiss.rb +++ b/app/services/admin_notices/dismiss.rb @@ -16,8 +16,8 @@ class AdminNotices::Dismiss guardian.is_admin? end - def fetch_admin_notice(id:) - AdminNotice.find_by(id: id) + def fetch_admin_notice(params:) + AdminNotice.find_by(id: params[:id]) end def destroy(admin_notice:) diff --git a/app/services/experiments/toggle.rb b/app/services/experiments/toggle.rb index 84b6b709016..f41dbfa747c 100644 --- a/app/services/experiments/toggle.rb +++ b/app/services/experiments/toggle.rb @@ -24,7 +24,7 @@ class Experiments::Toggle def toggle(contract:, guardian:) SiteSetting.set_and_log( contract.setting_name, - !SiteSetting.send(contract.setting_name), + !SiteSetting.public_send(contract.setting_name), guardian.user, ) end diff --git a/app/services/flags/destroy_flag.rb b/app/services/flags/destroy_flag.rb index 114bc6074b8..2ee02d75a17 100644 --- a/app/services/flags/destroy_flag.rb +++ b/app/services/flags/destroy_flag.rb @@ -14,8 +14,8 @@ class Flags::DestroyFlag private - def fetch_flag(id:) - Flag.find_by(id: id) + def fetch_flag(params:) + Flag.find_by(id: params[:id]) end def not_system(flag:) diff --git a/app/services/site_setting/update.rb b/app/services/site_setting/update.rb index 47a7ced2272..1762fef875a 100644 --- a/app/services/site_setting/update.rb +++ b/app/services/site_setting/update.rb @@ -3,11 +3,12 @@ class SiteSetting::Update include Service::Base + options { attribute :allow_changing_hidden, :boolean, default: false } + policy :current_user_is_admin contract do attribute :setting_name attribute :new_value - attribute :allow_changing_hidden, :boolean, default: false before_validation do self.setting_name = setting_name&.to_sym @@ -43,8 +44,8 @@ class SiteSetting::Update guardian.is_admin? end - def setting_is_visible(contract:) - contract.allow_changing_hidden || !SiteSetting.hidden_settings.include?(contract.setting_name) + def setting_is_visible(contract:, options:) + options.allow_changing_hidden || !SiteSetting.hidden_settings.include?(contract.setting_name) end def setting_is_configurable(contract:) diff --git a/lib/service.rb b/lib/service.rb index 07d39252657..7057fc0017d 100644 --- a/lib/service.rb +++ b/lib/service.rb @@ -40,9 +40,9 @@ module Service # # @example An example from the {TrashChannel} service # class TrashChannel - # include Base + # include Service::Base # - # model :channel, :fetch_channel + # model :channel # policy :invalid_access # transaction do # step :prevents_slug_collision @@ -79,17 +79,15 @@ module Service # end # @example An example from the {UpdateChannelStatus} service which uses a contract # class UpdateChannelStatus - # include Base + # include Service::Base # - # model :channel, :fetch_channel - # contract - # policy :check_channel_permission - # step :change_status - # - # class Contract + # model :channel + # contract do # attribute :status # validates :status, inclusion: { in: Chat::Channel.editable_statuses.keys } # end + # policy :check_channel_permission + # step :change_status # # … # end diff --git a/lib/service/base.rb b/lib/service/base.rb index 1845be2aeb7..e25a23c4015 100644 --- a/lib/service/base.rb +++ b/lib/service/base.rb @@ -18,7 +18,7 @@ module Service # Simple structure to hold the context of the service during its whole lifecycle. class Context - delegate :slice, to: :store + delegate :slice, :dig, to: :store def initialize(context = {}) @store = context.symbolize_keys @@ -115,6 +115,12 @@ module Service def transaction(&block) steps << TransactionStep.new(&block) end + + def options(&block) + klass = Class.new(Service::OptionsBase).tap { _1.class_eval(&block) } + const_set("Options", klass) + steps << OptionsStep.new(:default, class_name: klass) + end end # @!visibility private @@ -196,7 +202,7 @@ module Service attributes = class_name.attribute_names.map(&:to_sym) default_values = {} default_values = context[default_values_from].slice(*attributes) if default_values_from - contract = class_name.new(default_values.merge(context.slice(*attributes))) + contract = class_name.new(default_values.merge(context[:params].slice(*attributes))) context[contract_name] = contract context[result_key] = Context.build if contract.invalid? @@ -208,9 +214,13 @@ module Service private def contract_name - return :contract if name.to_sym == :default + return :contract if default? :"#{name}_contract" end + + def default? + name.to_sym == :default + end end # @!visibility private @@ -229,6 +239,14 @@ module Service end end + # @!visibility private + class OptionsStep < Step + def call(instance, context) + context[result_key] = Context.build + context[:options] = class_name.new(context[:options]) + end + end + included do # The global context which is available from any step. attr_reader :context @@ -263,7 +281,7 @@ module Service # customized by providing the +name+ argument). # # @example - # model :channel, :fetch_channel + # model :channel # # private # @@ -361,6 +379,17 @@ module Service # step :log_channel_deletion # end + # @!scope class + # @!method options(&block) + # @param block [Proc] a block containing options definition + # This is used to define options allowing to parameterize the service + # behavior. The resulting options are available in `context[:options]`. + # + # @example + # options do + # attribute :my_option, :boolean, default: false + # end + # @!visibility private def initialize(initial_context = {}) @context = Context.build(initial_context.merge(__steps__: self.class.steps)) diff --git a/lib/service/options_base.rb b/lib/service/options_base.rb new file mode 100644 index 00000000000..fb42dfc411e --- /dev/null +++ b/lib/service/options_base.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class Service::OptionsBase + include ActiveModel::API + include ActiveModel::Attributes + include ActiveModel::AttributeMethods +end diff --git a/lib/service/steps_inspector.rb b/lib/service/steps_inspector.rb index 6b230c63607..6c7b47aff81 100644 --- a/lib/service/steps_inspector.rb +++ b/lib/service/steps_inspector.rb @@ -98,6 +98,10 @@ class Service::StepsInspector nil end end + # + # @!visibility private + class Options < Step + end attr_reader :steps, :result 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 d988ff628b7..f98d693acf5 100644 --- a/plugins/chat/app/controllers/chat/api/channel_messages_controller.rb +++ b/plugins/chat/app/controllers/chat/api/channel_messages_controller.rb @@ -67,8 +67,7 @@ class Chat::Api::ChannelMessagesController < Chat::ApiController def create Chat::MessageRateLimiter.run!(current_user) - # users can't force a thread through JSON API - Chat::CreateMessage.call(service_params.merge(force_thread: false)) do + Chat::CreateMessage.call(service_params) do on_success do |message_instance:| render json: success_json.merge(message_id: message_instance.id) end diff --git a/plugins/chat/app/controllers/chat/api/channels_controller.rb b/plugins/chat/app/controllers/chat/api/channels_controller.rb index cd5f203f769..df88bb3b51b 100644 --- a/plugins/chat/app/controllers/chat/api/channels_controller.rb +++ b/plugins/chat/app/controllers/chat/api/channels_controller.rb @@ -55,7 +55,7 @@ class Chat::Api::ChannelsController < Chat::ApiController # 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. Chat::CreateCategoryChannel.call( - service_params.merge(channel_params.merge(category_id: channel_params[:chatable_id])), + service_params.merge(params: channel_params.merge(category_id: channel_params[:chatable_id])), ) do on_success do |channel:, membership:| render_serialized(channel, Chat::ChannelSerializer, root: "channel", membership:) @@ -95,7 +95,7 @@ class Chat::Api::ChannelsController < Chat::ApiController auto_join_limiter(channel_from_params).performed! end - Chat::UpdateChannel.call(service_params.merge(params_to_edit)) do + Chat::UpdateChannel.call(service_params.deep_merge(params: params_to_edit.to_unsafe_h)) do on_success do |channel:| render_serialized( channel, diff --git a/plugins/chat/app/controllers/chat/incoming_webhooks_controller.rb b/plugins/chat/app/controllers/chat/incoming_webhooks_controller.rb index 7e7e0700732..9c58de23a04 100644 --- a/plugins/chat/app/controllers/chat/incoming_webhooks_controller.rb +++ b/plugins/chat/app/controllers/chat/incoming_webhooks_controller.rb @@ -56,12 +56,12 @@ module Chat webhook.chat_channel.add(Discourse.system_user) Chat::CreateMessage.call( - service_params.merge( + params: { chat_channel_id: webhook.chat_channel_id, - guardian: Discourse.system_user.guardian, message: text, - incoming_chat_webhook: webhook, - ), + }, + guardian: Discourse.system_user.guardian, + incoming_chat_webhook: webhook, ) do on_success { render json: success_json } on_failure { render(json: failed_json, status: 422) } 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 c7c5d6014d0..2016a68916c 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 @@ -3,8 +3,8 @@ module Jobs module Chat class AutoJoinChannelBatch < ::Jobs::Base - def execute(*) - ::Chat::AutoJoinChannelBatch.call(*) do + def execute(args) + ::Chat::AutoJoinChannelBatch.call(params: args) 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/app/jobs/regular/chat/auto_remove_membership_handle_category_updated.rb b/plugins/chat/app/jobs/regular/chat/auto_remove_membership_handle_category_updated.rb index fa1e6504fbf..be135d94100 100644 --- a/plugins/chat/app/jobs/regular/chat/auto_remove_membership_handle_category_updated.rb +++ b/plugins/chat/app/jobs/regular/chat/auto_remove_membership_handle_category_updated.rb @@ -4,7 +4,7 @@ module Jobs module Chat class AutoRemoveMembershipHandleCategoryUpdated < ::Jobs::Base def execute(args) - ::Chat::AutoRemove::HandleCategoryUpdated.call(**args) + ::Chat::AutoRemove::HandleCategoryUpdated.call(params: args) end end end diff --git a/plugins/chat/app/jobs/regular/chat/auto_remove_membership_handle_chat_allowed_groups_change.rb b/plugins/chat/app/jobs/regular/chat/auto_remove_membership_handle_chat_allowed_groups_change.rb index 94ac518b35d..d37daef4100 100644 --- a/plugins/chat/app/jobs/regular/chat/auto_remove_membership_handle_chat_allowed_groups_change.rb +++ b/plugins/chat/app/jobs/regular/chat/auto_remove_membership_handle_chat_allowed_groups_change.rb @@ -4,7 +4,7 @@ module Jobs module Chat class AutoRemoveMembershipHandleChatAllowedGroupsChange < ::Jobs::Base def execute(args) - ::Chat::AutoRemove::HandleChatAllowedGroupsChange.call(**args) + ::Chat::AutoRemove::HandleChatAllowedGroupsChange.call(params: args) end end end diff --git a/plugins/chat/app/jobs/regular/chat/auto_remove_membership_handle_destroyed_group.rb b/plugins/chat/app/jobs/regular/chat/auto_remove_membership_handle_destroyed_group.rb index e24f87247ee..0e47496e591 100644 --- a/plugins/chat/app/jobs/regular/chat/auto_remove_membership_handle_destroyed_group.rb +++ b/plugins/chat/app/jobs/regular/chat/auto_remove_membership_handle_destroyed_group.rb @@ -4,7 +4,7 @@ module Jobs module Chat class AutoRemoveMembershipHandleDestroyedGroup < ::Jobs::Base def execute(args) - ::Chat::AutoRemove::HandleDestroyedGroup.call(**args) + ::Chat::AutoRemove::HandleDestroyedGroup.call(params: args) end end end diff --git a/plugins/chat/app/jobs/regular/chat/auto_remove_membership_handle_user_removed_from_group.rb b/plugins/chat/app/jobs/regular/chat/auto_remove_membership_handle_user_removed_from_group.rb index bd599a840d2..f47ff9e911f 100644 --- a/plugins/chat/app/jobs/regular/chat/auto_remove_membership_handle_user_removed_from_group.rb +++ b/plugins/chat/app/jobs/regular/chat/auto_remove_membership_handle_user_removed_from_group.rb @@ -4,7 +4,7 @@ module Jobs module Chat class AutoRemoveMembershipHandleUserRemovedFromGroup < ::Jobs::Base def execute(args) - ::Chat::AutoRemove::HandleUserRemovedFromGroup.call(**args) + ::Chat::AutoRemove::HandleUserRemovedFromGroup.call(params: args) end end end diff --git a/plugins/chat/app/services/chat/add_users_to_channel.rb b/plugins/chat/app/services/chat/add_users_to_channel.rb index 585f44dd372..2c8ed8164aa 100644 --- a/plugins/chat/app/services/chat/add_users_to_channel.rb +++ b/plugins/chat/app/services/chat/add_users_to_channel.rb @@ -8,19 +8,21 @@ module Chat # @example # ::Chat::AddUsersToChannel.call( # guardian: guardian, - # channel_id: 1, - # usernames: ["bob", "alice"] + # params: { + # channel_id: 1, + # usernames: ["bob", "alice"], + # } # ) # class AddUsersToChannel include Service::Base - # @!method call(guardian:, **params_to_create) + # @!method self.call(guardian:, params:) # @param [Guardian] guardian - # @param [Integer] id of the channel - # @param [Hash] params_to_create - # @option params_to_create [Array] usernames - # @option params_to_create [Array] groups + # @param [Hash] params + # @option params [Integer] :channel_id ID of the channel + # @option params [Array] :usernames + # @option params [Array] :groups # @return [Service::Base::Context] contract do attribute :usernames, :array @@ -123,14 +125,16 @@ module Chat ::Chat::CreateMessage.call( guardian: Discourse.system_user.guardian, - chat_channel_id: channel.id, - message: - I18n.t( - "chat.channel.users_invited_to_channel", - invited_users: added_users.map { |u| "@#{u.username}" }.join(", "), - inviting_user: "@#{guardian.user.username}", - count: added_users.count, - ), + params: { + chat_channel_id: channel.id, + message: + I18n.t( + "chat.channel.users_invited_to_channel", + invited_users: added_users.map { |u| "@#{u.username}" }.join(", "), + inviting_user: "@#{guardian.user.username}", + count: added_users.count, + ), + }, ) { on_failure { fail!(failure: "Failed to notice the channel") } } end end diff --git a/plugins/chat/app/services/chat/auto_join_channel_batch.rb b/plugins/chat/app/services/chat/auto_join_channel_batch.rb index e892fcb2a0c..2ec508d2607 100644 --- a/plugins/chat/app/services/chat/auto_join_channel_batch.rb +++ b/plugins/chat/app/services/chat/auto_join_channel_batch.rb @@ -6,9 +6,11 @@ module Chat # # @example # Chat::AutoJoinChannelBatch.call( - # channel_id: 1, - # start_user_id: 27, - # end_user_id: 58, + # params: { + # channel_id: 1, + # start_user_id: 27, + # end_user_id: 58, + # } # ) # class AutoJoinChannelBatch diff --git a/plugins/chat/app/services/chat/create_category_channel.rb b/plugins/chat/app/services/chat/create_category_channel.rb index 179bcf7947b..642399a042e 100644 --- a/plugins/chat/app/services/chat/create_category_channel.rb +++ b/plugins/chat/app/services/chat/create_category_channel.rb @@ -6,25 +6,27 @@ module Chat # @example # Service::Chat::CreateCategoryChannel.call( # guardian: guardian, - # name: "SuperChannel", - # description: "This is the best channel", - # slug: "super-channel", - # category_id: category.id, - # threading_enabled: true, + # params: { + # name: "SuperChannel", + # description: "This is the best channel", + # slug: "super-channel", + # category_id: category.id, + # threading_enabled: true, + # } # ) # class CreateCategoryChannel include Service::Base - # @!method call(guardian:, **params_to_create) + # @!method self.call(guardian:, params:) # @param [Guardian] guardian - # @param [Hash] params_to_create - # @option params_to_create [String] name - # @option params_to_create [String] description - # @option params_to_create [String] slug - # @option params_to_create [Boolean] auto_join_users - # @option params_to_create [Integer] category_id - # @option params_to_create [Boolean] threading_enabled + # @param [Hash] params + # @option params [String] :name + # @option params [String] :description + # @option params [String] :slug + # @option params [Boolean] :auto_join_users + # @option params [Integer] :category_id + # @option params [Boolean] :threading_enabled # @return [Service::Base::Context] policy :public_channels_enabled diff --git a/plugins/chat/app/services/chat/create_direct_message_channel.rb b/plugins/chat/app/services/chat/create_direct_message_channel.rb index b1dd503854c..359d2c56780 100644 --- a/plugins/chat/app/services/chat/create_direct_message_channel.rb +++ b/plugins/chat/app/services/chat/create_direct_message_channel.rb @@ -9,18 +9,20 @@ module Chat # @example # ::Chat::CreateDirectMessageChannel.call( # guardian: guardian, - # target_usernames: ["bob", "alice"] + # params: { + # target_usernames: ["bob", "alice"], + # }, # ) # class CreateDirectMessageChannel include Service::Base - # @!method call(guardian:, **params_to_create) + # @!method self.call(guardian:, params:) # @param [Guardian] guardian - # @param [Hash] params_to_create - # @option params_to_create [Array] target_usernames - # @option params_to_create [Array] target_groups - # @option params_to_create [Boolean] upsert + # @param [Hash] params + # @option params [Array] :target_usernames + # @option params [Array] :target_groups + # @option params [Boolean] :upsert # @return [Service::Base::Context] contract do diff --git a/plugins/chat/app/services/chat/create_message.rb b/plugins/chat/app/services/chat/create_message.rb index 4aebe1471ed..ab7ddc49df8 100644 --- a/plugins/chat/app/services/chat/create_message.rb +++ b/plugins/chat/app/services/chat/create_message.rb @@ -4,22 +4,34 @@ module Chat # Service responsible for creating a new message. # # @example - # Chat::CreateMessage.call(chat_channel_id: 2, guardian: guardian, message: "A new message") + # Chat::CreateMessage.call(params: { chat_channel_id: 2, message: "A new message" }, guardian: guardian) # class CreateMessage include Service::Base - # @!method call(chat_channel_id:, guardian:, in_reply_to_id:, message:, staged_id:, upload_ids:, thread_id:, incoming_chat_webhook:) + # @!method self.call(guardian:, params:, options:) # @param guardian [Guardian] - # @param chat_channel_id [Integer] - # @param message [String] - # @param in_reply_to_id [Integer] ID of a message to reply to - # @param thread_id [Integer] ID of a thread to reply to - # @param upload_ids [Array] IDs of uploaded documents - # @param context_topic_id [Integer] ID of the currently visible topic in drawer mode - # @param context_post_ids [Array] IDs of the currently visible posts in drawer mode - # @param staged_id [String] arbitrary string that will be sent back to the client - # @param incoming_chat_webhook [Chat::IncomingWebhook] + # @param [Hash] params + # @option params [Integer] :chat_channel_id + # @option params [String] :message + # @option params [Integer] :in_reply_to_id ID of a message to reply to + # @option params [Integer] :thread_id ID of a thread to reply to + # @option params [Array] :upload_ids IDs of uploaded documents + # @option params [Integer] :context_topic_id ID of the currently visible topic in drawer mode + # @option params [Array] :context_post_ids IDs of the currently visible posts in drawer mode + # @option params [String] :staged_id arbitrary string that will be sent back to the client + # @param [Hash] options + # @option options [Chat::IncomingWebhook] :incoming_chat_webhook + # @return [Service::Base::Context] + + options do + attribute :streaming, :boolean, default: false + attribute :enforce_membership, :boolean, default: false + attribute :process_inline, :boolean, default: -> { Rails.env.test? } + attribute :force_thread, :boolean, default: false + attribute :strip_whitespaces, :boolean, default: true + attribute :created_by_sdk, :boolean, default: false + end policy :no_silenced_user contract do @@ -31,13 +43,6 @@ module Chat attribute :staged_id, :string attribute :upload_ids, :array attribute :thread_id, :string - attribute :streaming, :boolean, default: false - attribute :enforce_membership, :boolean, default: false - attribute :incoming_chat_webhook - attribute :process_inline, :boolean, default: Rails.env.test? - attribute :force_thread, :boolean, default: false - attribute :strip_whitespaces, :boolean, default: true - attribute :created_by_sdk, :boolean, default: false validates :chat_channel_id, presence: true validates :message, presence: true, if: -> { upload_ids.blank? } @@ -79,8 +84,8 @@ module Chat Chat::Channel.find_by_id_or_slug(contract.chat_channel_id) end - def enforce_membership(guardian:, channel:, contract:) - if guardian.user.bot? || contract.enforce_membership + def enforce_membership(guardian:, channel:, options:) + if guardian.user.bot? || options.enforce_membership channel.add(guardian.user) if channel.direct_message_channel? @@ -102,7 +107,7 @@ module Chat reply&.chat_channel == channel end - def fetch_thread(contract:, reply:, channel:) + def fetch_thread(contract:, reply:, channel:, options:) return Chat::Thread.find_by(id: contract.thread_id) if contract.thread_id.present? return unless reply reply.thread || @@ -110,7 +115,7 @@ module Chat original_message: reply, original_message_user: reply.user, channel: channel, - force: contract.force_thread, + force: options.force_thread, ) end @@ -129,16 +134,16 @@ module Chat guardian.user.uploads.where(id: contract.upload_ids) end - def clean_message(contract:) + def clean_message(contract:, options:) contract.message = TextCleaner.clean( contract.message, - strip_whitespaces: contract.strip_whitespaces, + strip_whitespaces: options.strip_whitespaces, strip_zero_width_spaces: true, ) end - def instantiate_message(channel:, guardian:, contract:, uploads:, thread:, reply:) + def instantiate_message(channel:, guardian:, contract:, uploads:, thread:, reply:, options:) channel.chat_messages.new( user: guardian.user, last_editor: guardian.user, @@ -148,7 +153,7 @@ module Chat thread: thread, cooked: ::Chat::Message.cook(contract.message, user_id: guardian.user.id), cooked_version: ::Chat::Message::BAKED_VERSION, - streaming: contract.streaming, + streaming: options.streaming, ) end @@ -169,10 +174,10 @@ module Chat thread.add(thread.original_message_user) end - def create_webhook_event(contract:, message_instance:) - return if contract.incoming_chat_webhook.blank? + def create_webhook_event(message_instance:) + return if context[:incoming_chat_webhook].blank? message_instance.create_chat_webhook_event( - incoming_chat_webhook: contract.incoming_chat_webhook, + incoming_chat_webhook: context[:incoming_chat_webhook], ) end @@ -186,8 +191,8 @@ module Chat membership.update!(last_read_message: message_instance) end - def update_created_by_sdk(message_instance:, contract:) - message_instance.created_by_sdk = contract.created_by_sdk + def update_created_by_sdk(message_instance:, options:) + message_instance.created_by_sdk = options.created_by_sdk end def process_direct_message_channel(membership:) @@ -200,7 +205,7 @@ module Chat Chat::Publisher.publish_thread_created!(channel, reply, thread.id) end - def process(channel:, message_instance:, contract:, thread:) + def process(channel:, message_instance:, contract:, thread:, options:) ::Chat::Publisher.publish_new!(channel, message_instance, contract.staged_id) DiscourseEvent.trigger( @@ -218,7 +223,7 @@ module Chat }, ) - if contract.process_inline + if options.process_inline Jobs::Chat::ProcessMessage.new.execute( { chat_message_id: message_instance.id, staged_id: contract.staged_id }, ) diff --git a/plugins/chat/app/services/chat/create_thread.rb b/plugins/chat/app/services/chat/create_thread.rb index baac11e6446..84ad808a32a 100644 --- a/plugins/chat/app/services/chat/create_thread.rb +++ b/plugins/chat/app/services/chat/create_thread.rb @@ -4,16 +4,17 @@ module Chat # Creates a thread. # # @example - # Chat::CreateThread.call(channel_id: 2, original_message_id: 3, guardian: guardian, title: "Restaurant for Saturday") + # Chat::CreateThread.call(guardian: guardian, params: { channel_id: 2, original_message_id: 3, title: "Restaurant for Saturday" }) # class CreateThread include Service::Base - # @!method call(thread_id:, channel_id:, guardian:, **params_to_create) - # @param [Integer] original_message_id - # @param [Integer] channel_id + # @!method self.call(guardian:, params:) # @param [Guardian] guardian - # @option params_to_create [String,nil] title + # @param [Hash] params + # @option params [Integer] :original_message_id + # @option params [Integer] :channel_id + # @option params [String,nil] :title # @return [Service::Base::Context] contract do diff --git a/plugins/chat/app/services/chat/flag_message.rb b/plugins/chat/app/services/chat/flag_message.rb index dabb638f1ee..dbc9a1e649f 100644 --- a/plugins/chat/app/services/chat/flag_message.rb +++ b/plugins/chat/app/services/chat/flag_message.rb @@ -6,26 +6,27 @@ module Chat # @example # ::Chat::FlagMessage.call( # guardian: guardian, - # channel_id: 1, - # message_id: 43, + # params: { + # channel_id: 1, + # message_id: 43, + # } # ) # class FlagMessage include Service::Base - # @!method call(guardian:, channel_id:, data:) + # @!method self.call(guardian:, params:) # @param [Guardian] guardian - # @param [Integer] channel_id of the channel - # @param [Integer] message_id of the message - # @param [Integer] flag_type_id - Type of flag to create - # @param [String] optional message - Used when the flag type is notify_user or notify_moderators and we have to create - # a separate PM. - # @param [Boolean] optional is_warning - Staff can send warnings when using the notify_user flag. - # @param [Boolean] optional take_action - Automatically approves the created reviewable and deletes the chat message. - # @param [Boolean] optional queue_for_review - Adds a special reason to the reviewable score and creates the reviewable using - # the force_review option. - + # @param [Hash] params + # @option params [Integer] :channel_id of the channel + # @option params [Integer] :message_id of the message + # @option params [Integer] :flag_type_id Type of flag to create + # @option params [String] :message (optional) Used when the flag type is notify_user or notify_moderators and we have to create a separate PM. + # @option params [Boolean] :is_warning (optional) Staff can send warnings when using the notify_user flag. + # @option params [Boolean] :take_action (optional) Automatically approves the created reviewable and deletes the chat message. + # @option params [Boolean] :queue_for_review (optional) Adds a special reason to the reviewable score and creates the reviewable using the force_review option. # @return [Service::Base::Context] + contract do attribute :message_id, :integer attribute :channel_id, :integer diff --git a/plugins/chat/app/services/chat/invite_users_to_channel.rb b/plugins/chat/app/services/chat/invite_users_to_channel.rb index 1a492af4584..307421a7444 100644 --- a/plugins/chat/app/services/chat/invite_users_to_channel.rb +++ b/plugins/chat/app/services/chat/invite_users_to_channel.rb @@ -4,16 +4,17 @@ module Chat # Invites users to a channel. # # @example - # Chat::InviteUsersToChannel.call(channel_id: 2, user_ids: [2, 43], guardian: guardian, **optional_params) + # Chat::InviteUsersToChannel.call(params: { channel_id: 2, user_ids: [2, 43] }, guardian: guardian) # class InviteUsersToChannel include Service::Base - # @!method call(user_ids:, channel_id:, guardian:) - # @param [Array] user_ids - # @param [Integer] channel_id + # @!method self.call(guardian:, params:) # @param [Guardian] guardian - # @option optional_params [Integer, nil] message_id + # @param [Hash] params + # @option params [Array] :user_ids + # @option params [Integer] :channel_id + # @option params [Integer, nil] :message_id # @return [Service::Base::Context] contract do diff --git a/plugins/chat/app/services/chat/leave_channel.rb b/plugins/chat/app/services/chat/leave_channel.rb index 6316963980c..fac02a54cdf 100644 --- a/plugins/chat/app/services/chat/leave_channel.rb +++ b/plugins/chat/app/services/chat/leave_channel.rb @@ -6,17 +6,20 @@ module Chat # @example # ::Chat::LeaveChannel.call( # guardian: guardian, - # channel_id: 1, + # params: { + # channel_id: 1, + # } # ) # class LeaveChannel include Service::Base - # @!method call(guardian:, channel_id:,) + # @!method self.call(guardian:, params:) # @param [Guardian] guardian - # @param [Integer] channel_id of the channel - + # @param [Hash] params + # @option params [Integer] :channel_id ID of the channel # @return [Service::Base::Context] + contract do attribute :channel_id, :integer diff --git a/plugins/chat/app/services/chat/list_channel_messages.rb b/plugins/chat/app/services/chat/list_channel_messages.rb index 6f77b71e306..872cf453ee0 100644 --- a/plugins/chat/app/services/chat/list_channel_messages.rb +++ b/plugins/chat/app/services/chat/list_channel_messages.rb @@ -5,14 +5,15 @@ module Chat # or fetching paginated messages from last read. # # @example - # Chat::ListChannelMessages.call(channel_id: 2, guardian: guardian, **optional_params) + # Chat::ListChannelMessages.call(params: { channel_id: 2, **optional_params }, guardian: guardian) # class ListChannelMessages include Service::Base - # @!method call(guardian:) - # @param [Integer] channel_id + # @!method self.call(guardian:, params:) # @param [Guardian] guardian + # @param [Hash] params + # @option params [Integer] :channel_id # @return [Service::Base::Context] contract do diff --git a/plugins/chat/app/services/chat/list_channel_thread_messages.rb b/plugins/chat/app/services/chat/list_channel_thread_messages.rb index ee02332eca9..5c467e44633 100644 --- a/plugins/chat/app/services/chat/list_channel_thread_messages.rb +++ b/plugins/chat/app/services/chat/list_channel_thread_messages.rb @@ -5,14 +5,15 @@ module Chat # or fetching paginated messages from last read. # # @example - # Chat::ListThreadMessages.call(thread_id: 2, guardian: guardian, **optional_params) + # Chat::ListThreadMessages.call(params: { thread_id: 2, **optional_params }, guardian: guardian) # class ListChannelThreadMessages include Service::Base - # @!method call(guardian:) + # @!method self.call(guardian:, params:) # @param [Guardian] guardian - # @option optional_params [Integer] thread_id + # @param [Hash] params + # @option params [Integer] :thread_id # @return [Service::Base::Context] contract do diff --git a/plugins/chat/app/services/chat/list_user_channels.rb b/plugins/chat/app/services/chat/list_user_channels.rb index 65a4eae7996..3dc5eed57af 100644 --- a/plugins/chat/app/services/chat/list_user_channels.rb +++ b/plugins/chat/app/services/chat/list_user_channels.rb @@ -4,12 +4,12 @@ module Chat # List of the channels a user is tracking # # @example - # Chat::ListUserChannels.call(guardian: guardian, **optional_params) + # Chat::ListUserChannels.call(guardian:) # class ListUserChannels include Service::Base - # @!method call(guardian:) + # @!method self.call(guardian:) # @param [Guardian] guardian # @return [Service::Base::Context] diff --git a/plugins/chat/app/services/chat/lookup_channel_threads.rb b/plugins/chat/app/services/chat/lookup_channel_threads.rb index 128e5130ccf..0917a8e7049 100644 --- a/plugins/chat/app/services/chat/lookup_channel_threads.rb +++ b/plugins/chat/app/services/chat/lookup_channel_threads.rb @@ -10,18 +10,19 @@ module Chat # of normal or tracking will be returned. # # @example - # Chat::LookupChannelThreads.call(channel_id: 2, guardian: guardian, limit: 5, offset: 2) + # Chat::LookupChannelThreads.call(params: { channel_id: 2, limit: 5, offset: 2 }, guardian: guardian) # class LookupChannelThreads include Service::Base THREADS_LIMIT = 10 - # @!method call(channel_id:, guardian:, limit: nil, offset: nil) - # @param [Integer] channel_id + # @!method self.call(guardian:, params:) # @param [Guardian] guardian - # @param [Integer] limit - # @param [Integer] offset + # @param [Hash] params + # @option params [Integer] :channel_id + # @option params [Integer] :limit + # @option params [Integer] :offset # @return [Service::Base::Context] contract do diff --git a/plugins/chat/app/services/chat/lookup_thread.rb b/plugins/chat/app/services/chat/lookup_thread.rb index f8aa427d3b5..30f35a56ace 100644 --- a/plugins/chat/app/services/chat/lookup_thread.rb +++ b/plugins/chat/app/services/chat/lookup_thread.rb @@ -5,15 +5,16 @@ module Chat # match, and the channel must specifically have threading enabled. # # @example - # Chat::LookupThread.call(thread_id: 88, channel_id: 2, guardian: guardian) + # Chat::LookupThread.call(params: { thread_id: 88, channel_id: 2 }, guardian: guardian) # class LookupThread include Service::Base - # @!method call(thread_id:, channel_id:, guardian:) - # @param [Integer] thread_id - # @param [Integer] channel_id + # @!method self.call(guardian:, params:) # @param [Guardian] guardian + # @param [Hash] params + # @option params [Integer] :thread_id + # @option params [Integer] :channel_id # @return [Service::Base::Context] contract do diff --git a/plugins/chat/app/services/chat/lookup_user_threads.rb b/plugins/chat/app/services/chat/lookup_user_threads.rb index 58e72809630..139c6ec26b9 100644 --- a/plugins/chat/app/services/chat/lookup_user_threads.rb +++ b/plugins/chat/app/services/chat/lookup_user_threads.rb @@ -7,17 +7,18 @@ module Chat # of normal or tracking will be returned. # # @example - # Chat::LookupUserThreads.call(guardian: guardian, limit: 5, offset: 2) + # Chat::LookupUserThreads.call(guardian: guardian, params: { limit: 5, offset: 2 }) # class LookupUserThreads include Service::Base THREADS_LIMIT = 10 - # @!method call(guardian:, limit: nil, offset: nil) + # @!method self.call(guardian:, params:) # @param [Guardian] guardian - # @param [Integer] limit - # @param [Integer] offset + # @param [Hash] params + # @option params [Integer] :limit + # @option params [Integer] :offset # @return [Service::Base::Context] contract do diff --git a/plugins/chat/app/services/chat/mark_all_user_channels_read.rb b/plugins/chat/app/services/chat/mark_all_user_channels_read.rb index e6fb6c74044..c5bcc93949f 100644 --- a/plugins/chat/app/services/chat/mark_all_user_channels_read.rb +++ b/plugins/chat/app/services/chat/mark_all_user_channels_read.rb @@ -10,7 +10,7 @@ module Chat class MarkAllUserChannelsRead include ::Service::Base - # @!method call(guardian:) + # @!method self.call(guardian:) # @param [Guardian] guardian # @return [Service::Base::Context] diff --git a/plugins/chat/app/services/chat/mark_thread_title_prompt_seen.rb b/plugins/chat/app/services/chat/mark_thread_title_prompt_seen.rb index 4d2287600eb..7672288f098 100644 --- a/plugins/chat/app/services/chat/mark_thread_title_prompt_seen.rb +++ b/plugins/chat/app/services/chat/mark_thread_title_prompt_seen.rb @@ -7,18 +7,21 @@ module Chat # # @example # Chat::MarkThreadTitlePromptSeen.call( - # thread_id: 88, - # channel_id: 2, + # params: { + # thread_id: 88, + # channel_id: 2, + # }, # guardian: guardian, # ) # class MarkThreadTitlePromptSeen include Service::Base - # @!method call(thread_id:, channel_id:, guardian:) - # @param [Integer] thread_id - # @param [Integer] channel_id + # @!method self.call(guardian:, params:) # @param [Guardian] guardian + # @param [Hash] params + # @option params [Integer] :thread_id + # @option params [Integer] :channel_id # @return [Service::Base::Context] contract do diff --git a/plugins/chat/app/services/chat/publisher.rb b/plugins/chat/app/services/chat/publisher.rb index 4955edf50fd..132a2e35ed3 100644 --- a/plugins/chat/app/services/chat/publisher.rb +++ b/plugins/chat/app/services/chat/publisher.rb @@ -322,8 +322,10 @@ module Chat tracking_data = Chat::TrackingState.call( guardian: Guardian.new(user), - channel_ids: channel_last_read_map.keys, - include_missing_memberships: true, + params: { + channel_ids: channel_last_read_map.keys, + include_missing_memberships: true, + }, ) if tracking_data.failure? raise StandardError, diff --git a/plugins/chat/app/services/chat/restore_message.rb b/plugins/chat/app/services/chat/restore_message.rb index 757d672ba4c..1d6f094ea45 100644 --- a/plugins/chat/app/services/chat/restore_message.rb +++ b/plugins/chat/app/services/chat/restore_message.rb @@ -6,15 +6,16 @@ module Chat # updated. # # @example - # Chat::RestoreMessage.call(message_id: 2, channel_id: 1, guardian: guardian) + # Chat::RestoreMessage.call(params: { message_id: 2, channel_id: 1 }, guardian: guardian) # class RestoreMessage include Service::Base - # @!method call(message_id:, channel_id:, guardian:) - # @param [Integer] message_id - # @param [Integer] channel_id + # @!method self.call(guardian:, params:) # @param [Guardian] guardian + # @param [Hash] params + # @option params [Integer] :message_id + # @option params [Integer] :channel_id # @return [Service::Base::Context] contract do diff --git a/plugins/chat/app/services/chat/search_chatable.rb b/plugins/chat/app/services/chat/search_chatable.rb index dbe2e41e1c0..95680e654b7 100644 --- a/plugins/chat/app/services/chat/search_chatable.rb +++ b/plugins/chat/app/services/chat/search_chatable.rb @@ -4,16 +4,17 @@ module Chat # Returns a list of chatables (users, groups ,category channels, direct message channels) that can be chatted with. # # @example - # Chat::SearchChatable.call(term: "@bob", guardian: guardian) + # Chat::SearchChatable.call(params: { term: "@bob" }, guardian: guardian) # class SearchChatable include Service::Base SEARCH_RESULT_LIMIT ||= 10 - # @!method call(term:, guardian:) - # @param [String] term + # @!method self.call(guardian:, params:) # @param [Guardian] guardian + # @param [Hash] params + # @option params [String] :term # @return [Service::Base::Context] contract do diff --git a/plugins/chat/app/services/chat/stop_message_streaming.rb b/plugins/chat/app/services/chat/stop_message_streaming.rb index ce35b04b70d..46758278775 100644 --- a/plugins/chat/app/services/chat/stop_message_streaming.rb +++ b/plugins/chat/app/services/chat/stop_message_streaming.rb @@ -4,14 +4,15 @@ module Chat # Service responsible for stopping streaming of a message. # # @example - # Chat::StopMessageStreaming.call(message_id: 3, guardian: guardian) + # Chat::StopMessageStreaming.call(params: { message_id: 3 }, guardian: guardian) # class StopMessageStreaming include ::Service::Base - # @!method call(message_id:, guardian:) - # @param [Integer] message_id + # @!method self.call(guardian:, params:) # @param [Guardian] guardian + # @param [Hash] params + # @option params [Integer] :message_id # @return [Service::Base::Context] contract do attribute :message_id, :integer diff --git a/plugins/chat/app/services/chat/tracking_state.rb b/plugins/chat/app/services/chat/tracking_state.rb index 7704b29821f..7c590493bc7 100644 --- a/plugins/chat/app/services/chat/tracking_state.rb +++ b/plugins/chat/app/services/chat/tracking_state.rb @@ -22,15 +22,16 @@ module Chat # Only channels with threads enabled will return thread tracking state. # # @example - # Chat::TrackingState.call(channel_ids: [2, 3], thread_ids: [6, 7], guardian: guardian) + # Chat::TrackingState.call(params: { channel_ids: [2, 3], thread_ids: [6, 7] }, guardian: guardian) # class TrackingState include Service::Base - # @!method call(thread_ids:, channel_ids:, guardian:) - # @param [Integer] thread_ids - # @param [Integer] channel_ids + # @!method self.call(guardian:, params:) # @param [Guardian] guardian + # @param [Hash] params + # @option params [Integer] :thread_ids + # @option params [Integer] :channel_ids # @return [Service::Base::Context] contract do diff --git a/plugins/chat/app/services/chat/trash_channel.rb b/plugins/chat/app/services/chat/trash_channel.rb index f3c144f613f..6a094962ab3 100644 --- a/plugins/chat/app/services/chat/trash_channel.rb +++ b/plugins/chat/app/services/chat/trash_channel.rb @@ -5,14 +5,15 @@ module Chat # Note the slug is modified to prevent collisions. # # @example - # Chat::TrashChannel.call(channel_id: 2, guardian: guardian) + # Chat::TrashChannel.call(params: { channel_id: 2 }, guardian: guardian) # class TrashChannel include Service::Base - # @!method call(channel_id:, guardian:) - # @param [Integer] channel_id + # @!method self.call(guardian:, params:) # @param [Guardian] guardian + # @param [Hash] params + # @option params [Integer] :channel_id # @return [Service::Base::Context] DELETE_CHANNEL_LOG_KEY = "chat_channel_delete" @@ -28,8 +29,8 @@ module Chat private - def fetch_channel(channel_id:) - Chat::Channel.find_by(id: channel_id) + def fetch_channel(params:) + Chat::Channel.find_by(id: params[:channel_id]) end def invalid_access(guardian:, channel:) diff --git a/plugins/chat/app/services/chat/trash_message.rb b/plugins/chat/app/services/chat/trash_message.rb index 51be333c270..9322fea9fff 100644 --- a/plugins/chat/app/services/chat/trash_message.rb +++ b/plugins/chat/app/services/chat/trash_message.rb @@ -6,15 +6,16 @@ module Chat # updated. # # @example - # Chat::TrashMessage.call(message_id: 2, channel_id: 1, guardian: guardian) + # Chat::TrashMessage.call(params: { message_id: 2, channel_id: 1 }, guardian: guardian) # class TrashMessage include Service::Base - # @!method call(message_id:, channel_id:, guardian:) - # @param [Integer] message_id - # @param [Integer] channel_id + # @!method self.call(guardian:, params:) # @param [Guardian] guardian + # @param [Hash] params + # @option params [Integer] :message_id + # @option params [Integer] :channel_id # @return [Service::Base::Context] contract do diff --git a/plugins/chat/app/services/chat/trash_messages.rb b/plugins/chat/app/services/chat/trash_messages.rb index a97a2666687..a6fb6dd4935 100644 --- a/plugins/chat/app/services/chat/trash_messages.rb +++ b/plugins/chat/app/services/chat/trash_messages.rb @@ -6,15 +6,16 @@ module Chat # is updated. # # @example - # Chat::TrashMessages.call(message_ids: [2, 3], channel_id: 1, guardian: guardian) + # Chat::TrashMessages.call(params: { message_ids: [2, 3], channel_id: 1 }, guardian: guardian) # class TrashMessages include Service::Base - # @!method call(message_ids:, channel_id:, guardian:) - # @param [Array] message_ids - # @param [Integer] channel_id + # @!method self.call(guardian:, params:) # @param [Guardian] guardian + # @param [Hash] params + # @option params [Array] :message_ids + # @option params [Integer] :channel_id # @return [Service::Base::Context] contract do diff --git a/plugins/chat/app/services/chat/unfollow_channel.rb b/plugins/chat/app/services/chat/unfollow_channel.rb index 4083ca10d08..b23d81915b4 100644 --- a/plugins/chat/app/services/chat/unfollow_channel.rb +++ b/plugins/chat/app/services/chat/unfollow_channel.rb @@ -6,17 +6,20 @@ module Chat # @example # ::Chat::UnfollowChannel.call( # guardian: guardian, - # channel_id: 1, + # params: { + # channel_id: 1, + # } # ) # class UnfollowChannel include Service::Base - # @!method call(guardian:, channel_id:,) + # @!method self.call(guardian:, params:) # @param [Guardian] guardian - # @param [Integer] channel_id of the channel - + # @param [Hash] params + # @option params [Integer] :channel_id ID of the channel # @return [Service::Base::Context] + contract do attribute :channel_id, :integer diff --git a/plugins/chat/app/services/chat/update_channel.rb b/plugins/chat/app/services/chat/update_channel.rb index 1898a5eba6d..ed39c2892c7 100644 --- a/plugins/chat/app/services/chat/update_channel.rb +++ b/plugins/chat/app/services/chat/update_channel.rb @@ -8,28 +8,30 @@ module Chat # # @example # ::Chat::UpdateChannel.call( - # channel_id: 2, # guardian: guardian, - # name: "SuperChannel", - # description: "This is the best channel", - # slug: "super-channel", - # threading_enabled: true, + # params:{ + # channel_id: 2, + # name: "SuperChannel", + # description: "This is the best channel", + # slug: "super-channel", + # threading_enabled: true + # }, # ) # + class UpdateChannel include Service::Base - # @!method call(channel_id:, guardian:, **params_to_edit) - # @param [Integer] channel_id + # @!method self.call(params:, guardian:) # @param [Guardian] guardian - # @param [Hash] params_to_edit - # @option params_to_edit [String,nil] name - # @option params_to_edit [String,nil] description - # @option params_to_edit [String,nil] slug - # @option params_to_edit [Boolean] threading_enabled - # @option params_to_edit [Boolean] auto_join_users Only valid for {CategoryChannel}. Whether active users - # with permission to see the category should automatically join the channel. - # @option params_to_edit [Boolean] allow_channel_wide_mentions Allow the use of @here and @all in the channel. + # @param [Hash] params + # @option params [Integer] :channel_id The channel ID + # @option params [String,nil] :name + # @option params [String,nil] :description + # @option params [String,nil] :slug + # @option params [Boolean] :threading_enabled + # @option params [Boolean] :auto_join_users Only valid for {CategoryChannel}. Whether active users with permission to see the category should automatically join the channel. + # @option params [Boolean] :allow_channel_wide_mentions Allow the use of @here and @all in the channel. # @return [Service::Base::Context] model :channel @@ -56,8 +58,8 @@ module Chat private - def fetch_channel(channel_id:) - Chat::Channel.find_by(id: channel_id) + def fetch_channel(params:) + Chat::Channel.find_by(id: params[:channel_id]) end def check_channel_permission(guardian:, channel:) diff --git a/plugins/chat/app/services/chat/update_channel_status.rb b/plugins/chat/app/services/chat/update_channel_status.rb index cf3b2892ce5..845b21cac8a 100644 --- a/plugins/chat/app/services/chat/update_channel_status.rb +++ b/plugins/chat/app/services/chat/update_channel_status.rb @@ -4,15 +4,16 @@ module Chat # Service responsible for updating a chat channel status. # # @example - # Chat::UpdateChannelStatus.call(channel_id: 2, guardian: guardian, status: "open") + # Chat::UpdateChannelStatus.call(guardian: guardian, params: { status: "open", channel_id: 2 }) # class UpdateChannelStatus include Service::Base - # @!method call(channel_id:, guardian:, status:) - # @param [Integer] channel_id + # @!method self.call(guardian:, params:) # @param [Guardian] guardian - # @param [String] status + # @param [Hash] params + # @option params [Integer] :channel_id + # @option params [String] :status # @return [Service::Base::Context] model :channel, :fetch_channel @@ -26,8 +27,8 @@ module Chat private - def fetch_channel(channel_id:) - Chat::Channel.find_by(id: channel_id) + def fetch_channel(params:) + Chat::Channel.find_by(id: params[:channel_id]) end def check_channel_permission(guardian:, channel:, contract:) diff --git a/plugins/chat/app/services/chat/update_message.rb b/plugins/chat/app/services/chat/update_message.rb index 8bf1e8cbd3a..275bc0efcef 100644 --- a/plugins/chat/app/services/chat/update_message.rb +++ b/plugins/chat/app/services/chat/update_message.rb @@ -4,24 +4,32 @@ module Chat # Service responsible for updating a message. # # @example - # Chat::UpdateMessage.call(message_id: 2, guardian: guardian, message: "A new message") + # Chat::UpdateMessage.call(guardian: guardian, params: { message: "A new message", message_id: 2 }) # + class UpdateMessage include Service::Base - # @!method call(message_id:, guardian:, message:, upload_ids:) + # @!method self.call(guardian:, params:, options:) # @param guardian [Guardian] - # @param message_id [Integer] - # @param message [String] - # @param upload_ids [Array] IDs of uploaded documents + # @param [Hash] params + # @option params [Integer] :message_id + # @option params [String] :message + # @option params [Array] :upload_ids IDs of uploaded documents + # @param [Hash] options + # @option options [Boolean] (true) :strip_whitespaces + # @option options [Boolean] :process_inline + # @return [Service::Base::Context] + + options do + attribute :strip_whitespaces, :boolean, default: true + attribute :process_inline, :boolean, default: -> { Rails.env.test? } + end contract do attribute :message_id, :string attribute :message, :string attribute :upload_ids, :array - attribute :streaming, :boolean, default: false - attribute :strip_whitespaces, :boolean, default: true - attribute :process_inline, :boolean, default: Rails.env.test? validates :message_id, presence: true validates :message, presence: true, if: -> { upload_ids.blank? } @@ -82,12 +90,12 @@ module Chat guardian.can_edit_chat?(message) end - def clean_message(contract:) + def clean_message(contract:, options:) contract.message = TextCleaner.clean( contract.message, - strip_whitespaces: contract.strip_whitespaces, strip_zero_width_spaces: true, + strip_whitespaces: options.strip_whitespaces, ) end @@ -149,14 +157,14 @@ module Chat chars_edited > max_edited_chars end - def publish(message:, guardian:, contract:) + def publish(message:, guardian:, contract:, options:) edit_timestamp = context[:revision]&.created_at&.iso8601(6) || Time.zone.now.iso8601(6) ::Chat::Publisher.publish_edit!(message.chat_channel, message) DiscourseEvent.trigger(:chat_message_edited, message, message.chat_channel, message.user) - if contract.process_inline + if options.process_inline Jobs::Chat::ProcessMessage.new.execute( { chat_message_id: message.id, edit_timestamp: edit_timestamp }, ) diff --git a/plugins/chat/app/services/chat/update_thread.rb b/plugins/chat/app/services/chat/update_thread.rb index 3d0db4a1059..e3fcdcb6f12 100644 --- a/plugins/chat/app/services/chat/update_thread.rb +++ b/plugins/chat/app/services/chat/update_thread.rb @@ -7,16 +7,16 @@ module Chat # Only the thread title can be updated. # # @example - # Chat::UpdateThread.call(thread_id: 88, guardian: guardian, title: "Restaurant for Saturday") + # Chat::UpdateThread.call(guardian: guardian, params: { thread_id: 88, title: "Restaurant for Saturday" }) # class UpdateThread include Service::Base - # @!method call(thread_id:, channel_id:, guardian:, **params_to_edit) - # @param [Integer] thread_id - # @param [Integer] channel_id + # @!method self.call(guardian:, params:) # @param [Guardian] guardian - # @option params_to_edit [String,nil] title + # @param [Hash] params + # @option params [Integer] :thread_id + # @option params [Integer] :channel_id # @return [Service::Base::Context] contract do diff --git a/plugins/chat/app/services/chat/update_thread_notification_settings.rb b/plugins/chat/app/services/chat/update_thread_notification_settings.rb index feffa839f11..f991bc21b14 100644 --- a/plugins/chat/app/services/chat/update_thread_notification_settings.rb +++ b/plugins/chat/app/services/chat/update_thread_notification_settings.rb @@ -7,20 +7,23 @@ module Chat # # @example # Chat::UpdateThreadNotificationSettings.call( - # thread_id: 88, - # channel_id: 2, + # params: { + # thread_id: 88, + # channel_id: 2, + # notification_level: notification_level, + # }, # guardian: guardian, - # notification_level: notification_level, # ) # class UpdateThreadNotificationSettings include Service::Base - # @!method call(thread_id:, channel_id:, guardian:, notification_level:) - # @param [Integer] thread_id - # @param [Integer] channel_id - # @param [Integer] notification_level + # @!method self.call(guardian:, params:) # @param [Guardian] guardian + # @param [Hash] params + # @option params [Integer] :thread_id + # @option params [Integer] :channel_id + # @option params [Integer] :notification_level # @return [Service::Base::Context] contract do diff --git a/plugins/chat/app/services/chat/update_user_channel_last_read.rb b/plugins/chat/app/services/chat/update_user_channel_last_read.rb index 45629daa820..f39d7af14a7 100644 --- a/plugins/chat/app/services/chat/update_user_channel_last_read.rb +++ b/plugins/chat/app/services/chat/update_user_channel_last_read.rb @@ -4,15 +4,16 @@ module Chat # Service responsible for updating the last read message id of a membership. # # @example - # Chat::UpdateUserChannelLastRead.call(channel_id: 2, message_id: 3, guardian: guardian) + # Chat::UpdateUserChannelLastRead.call(params: { channel_id: 2, message_id: 3 }, guardian: guardian) # class UpdateUserChannelLastRead include ::Service::Base - # @!method call(channel_id:, message_id:, guardian:) - # @param [Integer] channel_id - # @param [Integer] message_id + # @!method self.call(guardian:, params:) # @param [Guardian] guardian + # @param [Hash] params + # @option params [Integer] :channel_id + # @option params [Integer] :message_id # @return [Service::Base::Context] contract do diff --git a/plugins/chat/app/services/chat/update_user_thread_last_read.rb b/plugins/chat/app/services/chat/update_user_thread_last_read.rb index 6fcb9d0e73d..1afda59c70a 100644 --- a/plugins/chat/app/services/chat/update_user_thread_last_read.rb +++ b/plugins/chat/app/services/chat/update_user_thread_last_read.rb @@ -5,16 +5,17 @@ module Chat # as read. # # @example - # Chat::UpdateUserThreadLastRead.call(channel_id: 2, thread_id: 3, message_id: 4, guardian: guardian) + # Chat::UpdateUserThreadLastRead.call(params: { channel_id: 2, thread_id: 3, message_id: 4 }, guardian: guardian) # class UpdateUserThreadLastRead include ::Service::Base - # @!method call(channel_id:, thread_id:, guardian:) - # @param [Integer] channel_id - # @param [Integer] thread_id - # @param [Integer] message_id + # @!method self.call(guardian:, params:) # @param [Guardian] guardian + # @param [Hash] params + # @option params [Integer] :channel_id + # @option params [Integer] :thread_id + # @option params [Integer] :message_id # @return [Service::Base::Context] contract do diff --git a/plugins/chat/app/services/chat/upsert_draft.rb b/plugins/chat/app/services/chat/upsert_draft.rb index d6b4a5db889..ba2e0517397 100644 --- a/plugins/chat/app/services/chat/upsert_draft.rb +++ b/plugins/chat/app/services/chat/upsert_draft.rb @@ -6,20 +6,24 @@ module Chat # @example # ::Chat::UpsertDraft.call( # guardian: guardian, - # channel_id: 1, - # thread_id: 1, - # data: { message: "foo" } + # params: { + # channel_id: 1, + # thread_id: 1, + # data: { message: "foo" } + # } # ) # class UpsertDraft include Service::Base - # @!method call(guardian:, channel_id:, thread_id:, data:) + # @!method self.call(guardian:, params:) # @param [Guardian] guardian - # @param [Integer] channel_id of the channel - # @param [String] json object as string containing the data of the draft (message, uploads, replyToMsg and editing keys) - # @option [Integer] thread_id of the channel + # @param [Hash] params + # @option params [Integer] :channel_id ID of the channel + # @option params [String] :data JSON object as string containing the data of the draft (message, uploads, replyToMsg and editing keys) + # @option params [Integer] :thread_id ID of the thread # @return [Service::Base::Context] + contract do attribute :channel_id, :integer validates :channel_id, presence: true diff --git a/plugins/chat/lib/chat/channel_fetcher.rb b/plugins/chat/lib/chat/channel_fetcher.rb index 73e79c9f0eb..161908a0a16 100644 --- a/plugins/chat/lib/chat/channel_fetcher.rb +++ b/plugins/chat/lib/chat/channel_fetcher.rb @@ -246,10 +246,12 @@ module Chat def self.tracking_state(channel_ids, guardian, include_threads: false) Chat::TrackingState.call( - channel_ids: channel_ids, - guardian: guardian, - include_missing_memberships: true, - include_threads: include_threads, + guardian:, + params: { + include_missing_memberships: true, + channel_ids:, + include_threads:, + }, ).report end diff --git a/plugins/chat/lib/chat/message_mover.rb b/plugins/chat/lib/chat/message_mover.rb index fa7de0640a4..5c74a300497 100644 --- a/plugins/chat/lib/chat/message_mover.rb +++ b/plugins/chat/lib/chat/message_mover.rb @@ -236,16 +236,18 @@ module Chat def add_moved_placeholder(destination_channel, first_moved_message) @source_channel.add(Discourse.system_user) Chat::CreateMessage.call( - chat_channel_id: @source_channel.id, guardian: Discourse.system_user.guardian, - message: - I18n.t( - "chat.channel.messages_moved", - count: @source_message_ids.length, - acting_username: @acting_user.username, - channel_name: destination_channel.title(@acting_user), - first_moved_message_url: first_moved_message.url, - ), + params: { + chat_channel_id: @source_channel.id, + message: + I18n.t( + "chat.channel.messages_moved", + count: @source_message_ids.length, + acting_username: @acting_user.username, + channel_name: destination_channel.title(@acting_user), + first_moved_message_url: first_moved_message.url, + ), + }, ) end diff --git a/plugins/chat/lib/chat_sdk/channel.rb b/plugins/chat/lib/chat_sdk/channel.rb index 5a0b914b3b2..169f492dffb 100644 --- a/plugins/chat/lib/chat_sdk/channel.rb +++ b/plugins/chat/lib/chat_sdk/channel.rb @@ -11,12 +11,19 @@ module ChatSDK # @example Fetching messages from a channel with additional parameters # ChatSDK::Channel.messages(channel_id: 1, guardian: Guardian.new) # - def self.messages(channel_id:, guardian:, **params) - new.messages(channel_id:, guardian:, **params) + def self.messages(...) + new.messages(...) end def messages(channel_id:, guardian:, **params) - Chat::ListChannelMessages.call(channel_id:, guardian:, **params, direction: "future") do + Chat::ListChannelMessages.call( + guardian:, + params: { + channel_id:, + direction: "future", + **params, + }, + ) do on_success { |messages:| 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 952b9fe13ae..135c09ca916 100644 --- a/plugins/chat/lib/chat_sdk/message.rb +++ b/plugins/chat/lib/chat_sdk/message.rb @@ -24,8 +24,8 @@ module ChatSDK # ChatSDK::Message.create_with_stream(raw: "Streaming message", channel_id: 1, guardian: Guardian.new) do |helper, message| # helper.stream(raw: "Continuation of the message") # end - def self.create(**params, &block) - new.create(**params, &block) + def self.create(...) + new.create(...) end # Creates a new message with streaming enabled by default. @@ -46,8 +46,8 @@ module ChatSDK # @return [Chat::Message] The message object. # @example Streaming a message # ChatSDK::Message.stream(message_id: 42, guardian: guardian, raw: "text") - def self.stream(raw:, message_id:, guardian:, &block) - new.stream(raw: raw, message_id: message_id, guardian: guardian, &block) + def self.stream(...) + new.stream(...) end # Starts streaming for a specific chat message. @@ -57,8 +57,8 @@ module ChatSDK # @return [Chat::Message] The message object. # @example Starting the streaming of a message # ChatSDK::Message.start_stream(message_id: 42, guardian: guardian) - def self.start_stream(message_id:, guardian:) - new.start_stream(message_id: message_id, guardian: guardian) + def self.start_stream(...) + new.start_stream(...) end # Stops streaming for a specific chat message. @@ -68,8 +68,8 @@ module ChatSDK # @return [Chat::Message] The message object. # @example Stopping the streaming of a message # ChatSDK::Message.stop_stream(message_id: 42, guardian: guardian) - def self.stop_stream(message_id:, guardian:) - new.stop_stream(message_id: message_id, guardian: guardian) + def self.stop_stream(...) + new.stop_stream(...) end def start_stream(message_id:, guardian:) @@ -89,7 +89,7 @@ module ChatSDK end def stop_stream(message_id:, guardian:) - Chat::StopMessageStreaming.call(message_id:, guardian:) do + Chat::StopMessageStreaming.call(params: { message_id: }, guardian:) do on_success { |message:| message } on_model_not_found(:message) { raise "Couldn't find message with id: `#{message_id}`" } on_model_not_found(:membership) do @@ -121,18 +121,22 @@ module ChatSDK ) message = Chat::CreateMessage.call( - message: raw, - guardian: guardian, - chat_channel_id: channel_id, - in_reply_to_id: in_reply_to_id, - thread_id: thread_id, - upload_ids: upload_ids, - streaming: streaming, - enforce_membership: enforce_membership, - force_thread: force_thread, - strip_whitespaces: strip_whitespaces, - created_by_sdk: true, - **params, + params: { + message: raw, + chat_channel_id: channel_id, + in_reply_to_id:, + thread_id:, + upload_ids:, + **params, + }, + options: { + created_by_sdk: true, + streaming:, + enforce_membership:, + force_thread:, + strip_whitespaces:, + }, + guardian:, ) do on_model_not_found(:channel) { raise "Couldn't find channel with id: `#{channel_id}`" } on_model_not_found(:membership) do @@ -176,11 +180,14 @@ module ChatSDK return false if !message.streaming || !raw Chat::UpdateMessage.call( - message_id: message.id, - message: message.message + raw, guardian: guardian, - streaming: true, - strip_whitespaces: false, + params: { + message_id: message.id, + message: message.message + raw, + }, + options: { + strip_whitespaces: false, + }, ) { on_failure { raise "Unexpected error" } } message diff --git a/plugins/chat/lib/chat_sdk/thread.rb b/plugins/chat/lib/chat_sdk/thread.rb index 9e038728a07..3422afe7207 100644 --- a/plugins/chat/lib/chat_sdk/thread.rb +++ b/plugins/chat/lib/chat_sdk/thread.rb @@ -13,7 +13,7 @@ module ChatSDK # ChatSDK::Thread.update_title(title: "New Thread Title", thread_id: 1, guardian: Guardian.new) # def self.update_title(thread_id:, guardian:, title:) - new.update(title: title, thread_id: thread_id, guardian: guardian) + new.update(thread_id:, guardian:, title:) end # Retrieves messages from a specified thread. @@ -25,8 +25,8 @@ module ChatSDK # @example Fetching messages from a thread with additional parameters # ChatSDK::Thread.messages(thread_id: 1, guardian: Guardian.new) # - def self.messages(thread_id:, guardian:, **params) - new.messages(thread_id: thread_id, guardian: guardian, **params) + def self.messages(...) + new.messages(...) end # Fetches the first messages from a specified chat thread, starting from the first available message. @@ -41,9 +41,9 @@ module ChatSDK # def self.first_messages(thread_id:, guardian:, page_size: 10) new.messages( - thread_id: thread_id, - guardian: guardian, - page_size: page_size, + thread_id:, + guardian:, + page_size:, direction: "future", fetch_from_first_message: true, ) @@ -61,20 +61,27 @@ module ChatSDK # def self.last_messages(thread_id:, guardian:, page_size: 10) new.messages( - thread_id: thread_id, - guardian: guardian, - page_size: page_size, + thread_id:, + guardian:, + page_size:, direction: "past", fetch_from_last_message: true, ) end - def self.update(**params) - new.update(**params) + def self.update(...) + new.update(...) end def messages(thread_id:, guardian:, direction: "future", **params) - Chat::ListChannelThreadMessages.call(thread_id:, guardian:, direction:, **params) do + Chat::ListChannelThreadMessages.call( + guardian:, + params: { + thread_id:, + direction:, + **params, + }, + ) do on_success { |messages:| 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" } @@ -82,8 +89,8 @@ module ChatSDK end end - def update(**params) - Chat::UpdateThread.call(params) do + def update(guardian:, **params) + Chat::UpdateThread.call(guardian:, params:) do on_model_not_found(:channel) do raise "Couldn’t find channel with id: `#{params[:channel_id]}`" end diff --git a/plugins/chat/plugin.rb b/plugins/chat/plugin.rb index aa6f3a855b8..314b49a1fcf 100644 --- a/plugins/chat/plugin.rb +++ b/plugins/chat/plugin.rb @@ -472,9 +472,11 @@ after_initialize do creator = ::Chat::CreateMessage.call( - chat_channel_id: channel.id, guardian: sender.guardian, - message: utils.apply_placeholders(fields.dig("message", "value"), placeholders), + params: { + chat_channel_id: channel.id, + message: utils.apply_placeholders(fields.dig("message", "value"), placeholders), + }, ) if creator.failure? diff --git a/plugins/chat/spec/fabricators/chat_fabricator.rb b/plugins/chat/spec/fabricators/chat_fabricator.rb index 0a26dabfc40..ee32819a44a 100644 --- a/plugins/chat/spec/fabricators/chat_fabricator.rb +++ b/plugins/chat/spec/fabricators/chat_fabricator.rb @@ -93,16 +93,20 @@ Fabricator(:chat_message_with_service, class_name: "Chat::CreateMessage") do result = resolved_class.call( - chat_channel_id: channel.id, + params: { + chat_channel_id: channel.id, + message: + transients[:message] || + Faker::Alphanumeric.alpha(number: SiteSetting.chat_minimum_message_length), + thread_id: transients[:thread]&.id, + in_reply_to_id: transients[:in_reply_to]&.id, + upload_ids: transients[:upload_ids], + }, + options: { + process_inline: true, + }, guardian: user.guardian, - message: - transients[:message] || - Faker::Alphanumeric.alpha(number: SiteSetting.chat_minimum_message_length), - thread_id: transients[:thread]&.id, - in_reply_to_id: transients[:in_reply_to]&.id, - upload_ids: transients[:upload_ids], incoming_chat_webhook: transients[:incoming_chat_webhook], - process_inline: true, ) if result.failure? diff --git a/plugins/chat/spec/integration/thread_replies_count_cache_accuracy_spec.rb b/plugins/chat/spec/integration/thread_replies_count_cache_accuracy_spec.rb index 5e4280039ac..4ef7cdd6d28 100644 --- a/plugins/chat/spec/integration/thread_replies_count_cache_accuracy_spec.rb +++ b/plugins/chat/spec/integration/thread_replies_count_cache_accuracy_spec.rb @@ -24,10 +24,12 @@ RSpec.describe "Chat::Thread replies_count cache accuracy" do # Create 5 replies 5.times do |i| Chat::CreateMessage.call( - chat_channel_id: thread.channel_id, guardian: guardian, - thread_id: thread.id, - message: "Hello world #{i}", + params: { + chat_channel_id: thread.channel_id, + thread_id: thread.id, + message: "Hello world #{i}", + }, ) end @@ -39,10 +41,12 @@ RSpec.describe "Chat::Thread replies_count cache accuracy" do # Travel to the future so the cache expires. travel_to 6.minutes.from_now Chat::CreateMessage.call( - chat_channel_id: thread.channel_id, guardian: guardian, - thread_id: thread.id, - message: "Hello world now that time has passed", + params: { + chat_channel_id: thread.channel_id, + thread_id: thread.id, + message: "Hello world now that time has passed", + }, ) expect(thread.replies_count_cache).to eq(6) expect(thread.reload.replies_count).to eq(6) diff --git a/plugins/chat/spec/jobs/regular/chat/notify_mentioned_spec.rb b/plugins/chat/spec/jobs/regular/chat/notify_mentioned_spec.rb index 2fb4753ca34..b29539a5737 100644 --- a/plugins/chat/spec/jobs/regular/chat/notify_mentioned_spec.rb +++ b/plugins/chat/spec/jobs/regular/chat/notify_mentioned_spec.rb @@ -15,7 +15,9 @@ describe Jobs::Chat::NotifyMentioned do result = Chat::CreateDirectMessageChannel.call( guardian: user_1.guardian, - target_usernames: [user_1.username, user_2.username], + params: { + target_usernames: [user_1.username, user_2.username], + }, ) service_failed!(result) if result.failure? diff --git a/plugins/chat/spec/lib/chat/notifier_spec.rb b/plugins/chat/spec/lib/chat/notifier_spec.rb index e8f5df4a49e..73bb8d4272a 100644 --- a/plugins/chat/spec/lib/chat/notifier_spec.rb +++ b/plugins/chat/spec/lib/chat/notifier_spec.rb @@ -145,8 +145,10 @@ describe Chat::Notifier do Chat::UpdateMessage.call( guardian: user_1.guardian, - message_id: msg.id, - message: "hello @all", + params: { + message_id: msg.id, + message: "hello @all", + }, ) described_class.new(msg, msg.created_at).notify_edit @@ -425,7 +427,9 @@ describe Chat::Notifier do result = Chat::CreateDirectMessageChannel.call( guardian: user_1.guardian, - target_usernames: [user_1.username, user_2.username], + params: { + target_usernames: [user_1.username, user_2.username], + }, ) service_failed!(result) if result.failure? result.channel diff --git a/plugins/chat/spec/lib/chat/review_queue_spec.rb b/plugins/chat/spec/lib/chat/review_queue_spec.rb index 4a15f54fb3f..1ed3f2dfa08 100644 --- a/plugins/chat/spec/lib/chat/review_queue_spec.rb +++ b/plugins/chat/spec/lib/chat/review_queue_spec.rb @@ -115,8 +115,10 @@ describe Chat::ReviewQueue do it "ignores the cooldown window when the message is edited" do Chat::UpdateMessage.call( guardian: Guardian.new(message.user), - message_id: message.id, - message: "I'm editing this message. Please flag it.", + params: { + message_id: message.id, + message: "I'm editing this message. Please flag it.", + }, ) expect(second_flag_result).to include success: true diff --git a/plugins/chat/spec/plugin_helper.rb b/plugins/chat/spec/plugin_helper.rb index a15918f73d7..2982b1769e7 100644 --- a/plugins/chat/spec/plugin_helper.rb +++ b/plugins/chat/spec/plugin_helper.rb @@ -38,11 +38,13 @@ module ChatSystemHelpers last_user = ((users - [last_user]).presence || users).sample creator = Chat::CreateMessage.call( - chat_channel_id: channel.id, - in_reply_to_id: in_reply_to, - thread_id: thread_id, guardian: last_user.guardian, - message: Faker::Alphanumeric.alpha(number: SiteSetting.chat_minimum_message_length), + params: { + chat_channel_id: channel.id, + in_reply_to_id: in_reply_to, + thread_id: thread_id, + message: Faker::Alphanumeric.alpha(number: SiteSetting.chat_minimum_message_length), + }, ) raise "#{creator.inspect_steps.inspect}\n\n#{creator.inspect_steps.error}" if creator.failure? @@ -69,10 +71,14 @@ module ChatSpecHelpers def update_message!(message, text: nil, user: Discourse.system_user, upload_ids: nil) Chat::UpdateMessage.call( guardian: user.guardian, - message_id: message.id, - upload_ids: upload_ids, - message: text, - process_inline: true, + params: { + message_id: message.id, + upload_ids: upload_ids, + message: text, + }, + options: { + process_inline: true, + }, ) do |result| on_success { result.message_instance } on_failure { service_failed!(result) } @@ -81,8 +87,10 @@ module ChatSpecHelpers def trash_message!(message, user: Discourse.system_user) Chat::TrashMessage.call( - message_id: message.id, - channel_id: message.chat_channel_id, + params: { + message_id: message.id, + channel_id: message.chat_channel_id, + }, guardian: user.guardian, ) do |result| on_success { result } @@ -92,8 +100,10 @@ module ChatSpecHelpers def restore_message!(message, user: Discourse.system_user) Chat::RestoreMessage.call( - message_id: message.id, - channel_id: message.chat_channel_id, + params: { + message_id: message.id, + channel_id: message.chat_channel_id, + }, guardian: user.guardian, ) do |result| on_success { result } @@ -104,8 +114,10 @@ module ChatSpecHelpers def add_users_to_channel(users, channel, user: Discourse.system_user) ::Chat::AddUsersToChannel.call( guardian: user.guardian, - channel_id: channel.id, - usernames: Array(users).map(&:username), + params: { + channel_id: channel.id, + usernames: Array(users).map(&:username), + }, ) do |result| on_success { result } on_failure { service_failed!(result) } @@ -121,9 +133,11 @@ module ChatSpecHelpers ::Chat::UpsertDraft.call( guardian: user.guardian, - channel_id: channel.id, - thread_id: thread&.id, - data: data.to_json, + params: { + channel_id: channel.id, + thread_id: thread&.id, + data: data.to_json, + }, ) do |result| on_success { result } on_failure { service_failed!(result) } diff --git a/plugins/chat/spec/services/auto_remove/handle_category_updated_spec.rb b/plugins/chat/spec/services/auto_remove/handle_category_updated_spec.rb index 76700891df5..917ea868160 100644 --- a/plugins/chat/spec/services/auto_remove/handle_category_updated_spec.rb +++ b/plugins/chat/spec/services/auto_remove/handle_category_updated_spec.rb @@ -2,7 +2,7 @@ RSpec.describe Chat::AutoRemove::HandleCategoryUpdated do describe ".call" do - subject(:result) { described_class.call(params) } + subject(:result) { described_class.call(params:) } let(:params) { { category_id: updated_category.id } } diff --git a/plugins/chat/spec/services/auto_remove/handle_chat_allowed_groups_change_spec.rb b/plugins/chat/spec/services/auto_remove/handle_chat_allowed_groups_change_spec.rb index e771f813ea0..f89c045d3aa 100644 --- a/plugins/chat/spec/services/auto_remove/handle_chat_allowed_groups_change_spec.rb +++ b/plugins/chat/spec/services/auto_remove/handle_chat_allowed_groups_change_spec.rb @@ -2,9 +2,9 @@ RSpec.describe Chat::AutoRemove::HandleChatAllowedGroupsChange do describe ".call" do - subject(:result) { described_class.call(params) } + subject(:result) { described_class.call(params:) } - let(:params) { { new_allowed_groups: new_allowed_groups } } + let(:params) { { new_allowed_groups: } } fab!(:user_1) { Fabricate(:user, refresh_auto_groups: true) } fab!(:user_2) { Fabricate(:user, refresh_auto_groups: true) } fab!(:admin_1) { Fabricate(:admin) } diff --git a/plugins/chat/spec/services/auto_remove/handle_destroyed_group_spec.rb b/plugins/chat/spec/services/auto_remove/handle_destroyed_group_spec.rb index 7b84ac10654..f750ff10fe0 100644 --- a/plugins/chat/spec/services/auto_remove/handle_destroyed_group_spec.rb +++ b/plugins/chat/spec/services/auto_remove/handle_destroyed_group_spec.rb @@ -6,7 +6,7 @@ RSpec.describe Chat::AutoRemove::HandleDestroyedGroup do end describe ".call" do - subject(:result) { described_class.call(params) } + subject(:result) { described_class.call(params:) } let(:params) { { destroyed_group_user_ids: [admin_1.id, admin_2.id, user_1.id, user_2.id] } } fab!(:user_1) { Fabricate(:user, refresh_auto_groups: true) } diff --git a/plugins/chat/spec/services/auto_remove/handle_user_removed_from_group_spec.rb b/plugins/chat/spec/services/auto_remove/handle_user_removed_from_group_spec.rb index 387d4a166d0..1f5f4c293d3 100644 --- a/plugins/chat/spec/services/auto_remove/handle_user_removed_from_group_spec.rb +++ b/plugins/chat/spec/services/auto_remove/handle_user_removed_from_group_spec.rb @@ -2,7 +2,7 @@ RSpec.describe Chat::AutoRemove::HandleUserRemovedFromGroup do describe ".call" do - subject(:result) { described_class.call(params) } + subject(:result) { described_class.call(params:) } let(:params) { { user_id: removed_user.id } } fab!(:removed_user) { Fabricate(:user) } diff --git a/plugins/chat/spec/services/chat/add_users_to_channel_spec.rb b/plugins/chat/spec/services/chat/add_users_to_channel_spec.rb index b825e1dae03..b0df6089a94 100644 --- a/plugins/chat/spec/services/chat/add_users_to_channel_spec.rb +++ b/plugins/chat/spec/services/chat/add_users_to_channel_spec.rb @@ -10,7 +10,7 @@ RSpec.describe Chat::AddUsersToChannel do end describe ".call" do - subject(:result) { described_class.call(params) } + subject(:result) { described_class.call(params:, **dependencies) } fab!(:current_user) { Fabricate(:user) } fab!(:users) { Fabricate.times(5, :user) } @@ -21,9 +21,8 @@ RSpec.describe Chat::AddUsersToChannel do fab!(:group) { Fabricate(:public_group, users: [group_user_1, group_user_2]) } let(:guardian) { Guardian.new(current_user) } - let(:params) do - { guardian: guardian, channel_id: channel.id, usernames: users.map(&:username) } - end + let(:params) { { channel_id: channel.id, usernames: users.map(&:username) } } + let(:dependencies) { { guardian: } } context "when all steps pass" do before { channel.add(current_user) } diff --git a/plugins/chat/spec/services/chat/auto_join_channel_batch_spec.rb b/plugins/chat/spec/services/chat/auto_join_channel_batch_spec.rb index 37853130c3e..8a464a6729e 100644 --- a/plugins/chat/spec/services/chat/auto_join_channel_batch_spec.rb +++ b/plugins/chat/spec/services/chat/auto_join_channel_batch_spec.rb @@ -45,7 +45,7 @@ describe Chat::AutoJoinChannelBatch do end describe ".call" do - subject(:result) { described_class.call(params) } + subject(:result) { described_class.call(params:) } fab!(:channel) { Fabricate(:chat_channel, auto_join_users: true) } diff --git a/plugins/chat/spec/services/chat/create_category_channel_spec.rb b/plugins/chat/spec/services/chat/create_category_channel_spec.rb index 5fbc574bbd1..8bedc5b13c5 100644 --- a/plugins/chat/spec/services/chat/create_category_channel_spec.rb +++ b/plugins/chat/spec/services/chat/create_category_channel_spec.rb @@ -1,35 +1,21 @@ # frozen_string_literal: true RSpec.describe Chat::CreateCategoryChannel do - describe Chat::CreateCategoryChannel::Contract, type: :model do + describe described_class::Contract, type: :model do it { is_expected.to validate_presence_of :category_id } it { is_expected.to validate_length_of(:name).is_at_most(SiteSetting.max_topic_title_length) } end describe ".call" do - subject(:result) { described_class.call(params) } + subject(:result) { described_class.call(params:, **dependencies) } fab!(:current_user) { Fabricate(:admin) } fab!(:category) let(:category_id) { category.id } let(:guardian) { Guardian.new(current_user) } - let(:params) { { guardian: guardian, category_id: category_id, name: "cool channel" } } - - it "can create several channels with empty slugs" do - SiteSetting.slug_generation_method = "none" - expect do - described_class.call(params.merge(name: "channel 1", slug: nil)) - end.not_to raise_error - expect do - described_class.call(params.merge(name: "channel 2", slug: nil)) - end.not_to raise_error - end - - it "can create several channels with unicode names" do - expect do described_class.call(params.merge(name: "マイキ")) end.not_to raise_error - expect do described_class.call(params.merge(name: "境界")) end.not_to raise_error - end + let(:params) { { category_id:, name: "cool channel" } } + let(:dependencies) { { guardian: } } context "when public channels are disabled" do fab!(:current_user) { Fabricate(:user) } diff --git a/plugins/chat/spec/services/chat/create_direct_message_channel_spec.rb b/plugins/chat/spec/services/chat/create_direct_message_channel_spec.rb index 7c57b9ea3fa..cdb34c44194 100644 --- a/plugins/chat/spec/services/chat/create_direct_message_channel_spec.rb +++ b/plugins/chat/spec/services/chat/create_direct_message_channel_spec.rb @@ -29,7 +29,7 @@ RSpec.describe Chat::CreateDirectMessageChannel do end describe ".call" do - subject(:result) { described_class.call(params) } + subject(:result) { described_class.call(params:, **dependencies) } fab!(:current_user) { Fabricate(:user, username: "guybrush", refresh_auto_groups: true) } fab!(:user_1) { Fabricate(:user, username: "lechuck") } @@ -40,7 +40,8 @@ RSpec.describe Chat::CreateDirectMessageChannel do let(:guardian) { Guardian.new(current_user) } let(:target_usernames) { [user_1.username, user_2.username] } let(:name) { "" } - let(:params) { { guardian: guardian, target_usernames: target_usernames, name: name } } + let(:params) { { target_usernames:, name: } } + let(:dependencies) { { guardian: } } context "when all steps pass" do it { is_expected.to run_successfully } @@ -117,7 +118,7 @@ RSpec.describe Chat::CreateDirectMessageChannel do let(:name) { "Monkey Island" } it "creates a second channel" do - described_class.call(params) + described_class.call(params:, **dependencies) expect { result }.to change { Chat::Channel.count }.and change { Chat::DirectMessage.count @@ -129,7 +130,7 @@ RSpec.describe Chat::CreateDirectMessageChannel do let(:target_usernames) { [user_1.username, user_2.username] } it "creates a second channel" do - described_class.call(params) + described_class.call(params:, **dependencies) expect { result }.to change { Chat::Channel.count }.and change { Chat::DirectMessage.count @@ -141,7 +142,7 @@ RSpec.describe Chat::CreateDirectMessageChannel do let(:target_usernames) { [user_1.username] } it "reuses the existing channel" do - existing_channel = described_class.call(params).channel + existing_channel = described_class.call(params:, **dependencies).channel expect(result.channel.id).to eq(existing_channel.id) end @@ -151,8 +152,9 @@ RSpec.describe Chat::CreateDirectMessageChannel do let(:target_usernames) { [user_1.username] } it "returns the non group existing channel" do - group_channel = described_class.call(params.merge(name: "cats")).channel - channel = described_class.call(params).channel + group_channel = + described_class.call(params: params.merge(name: "cats"), **dependencies).channel + channel = described_class.call(params:, **dependencies).channel expect(result.channel.id).to_not eq(group_channel.id) expect(result.channel.id).to eq(channel.id) diff --git a/plugins/chat/spec/services/chat/create_message_spec.rb b/plugins/chat/spec/services/chat/create_message_spec.rb index 3e7f666a962..9e4765481f9 100644 --- a/plugins/chat/spec/services/chat/create_message_spec.rb +++ b/plugins/chat/spec/services/chat/create_message_spec.rb @@ -20,7 +20,7 @@ RSpec.describe Chat::CreateMessage do end describe ".call" do - subject(:result) { described_class.call(params) } + subject(:result) { described_class.call(params:, options:, **dependencies) } fab!(:user) fab!(:other_user) { Fabricate(:user) } @@ -35,16 +35,15 @@ RSpec.describe Chat::CreateMessage do let(:context_post_ids) { nil } let(:params) do { - enforce_membership: false, - guardian: guardian, chat_channel_id: channel.id, message: content, upload_ids: [upload.id], context_topic_id: context_topic_id, context_post_ids: context_post_ids, - force_thread: false, } end + let(:options) { { enforce_membership: false, force_thread: false } } + let(:dependencies) { { guardian: } } let(:message) { result[:message_instance].reload } before { channel.add(guardian.user) } @@ -74,9 +73,12 @@ RSpec.describe Chat::CreateMessage do end context "when strip_whitespace is disabled" do - it "doesn't strip newlines" do - params[:strip_whitespaces] = false + before do + options[:strip_whitespaces] = false params[:message] = "aaaaaaa\n" + end + + it "doesn't strip newlines" do expect(message.message).to eq("aaaaaaa\n") end end @@ -84,7 +86,7 @@ RSpec.describe Chat::CreateMessage do context "when coming from a webhook" do let(:incoming_webhook) { Fabricate(:incoming_chat_webhook, chat_channel: channel) } - before { params[:incoming_chat_webhook] = incoming_webhook } + before { dependencies[:incoming_chat_webhook] = incoming_webhook } it "creates a webhook event" do expect { result }.to change { Chat::WebhookEvent.count }.by(1) @@ -104,15 +106,21 @@ RSpec.describe Chat::CreateMessage do result end - it "can enqueue a job to process message" do - params[:process_inline] = false - expect_enqueued_with(job: Jobs::Chat::ProcessMessage) { result } + context "when process_inline is false" do + before { options[:process_inline] = false } + + it "enqueues a job to process message" do + expect_enqueued_with(job: Jobs::Chat::ProcessMessage) { result } + end end - it "can process a message inline" do - params[:process_inline] = true - Jobs::Chat::ProcessMessage.any_instance.expects(:execute).once - expect_not_enqueued_with(job: Jobs::Chat::ProcessMessage) { result } + context "when process_inline is true" do + before { options[:process_inline] = true } + + it "processes a message inline" do + Jobs::Chat::ProcessMessage.any_instance.expects(:execute).once + expect_not_enqueued_with(job: Jobs::Chat::ProcessMessage) { result } + end end it "triggers a Discourse event" do @@ -127,11 +135,11 @@ RSpec.describe Chat::CreateMessage do result end - context "when context given" do + context "when a context is given" do let(:context_post_ids) { [1, 2] } let(:context_topic_id) { 3 } - it "triggers a Discourse event with context if given" do + it "triggers a Discourse event with context" do DiscourseEvent.expects(:trigger).with( :chat_message_created, instance_of(Chat::Message), @@ -245,7 +253,7 @@ RSpec.describe Chat::CreateMessage do before do SiteSetting.chat_allowed_groups = Group::AUTO_GROUPS[:everyone] - params[:enforce_membership] = true + options[:enforce_membership] = true end it { is_expected.to run_successfully } @@ -345,7 +353,7 @@ RSpec.describe Chat::CreateMessage do end context "when thread is forced" do - before { params[:force_thread] = true } + before { options[:force_thread] = true } it "publishes the new thread" do Chat::Publisher.expects(:publish_thread_created!).with( diff --git a/plugins/chat/spec/services/chat/create_thread_spec.rb b/plugins/chat/spec/services/chat/create_thread_spec.rb index e76e1f69411..83cc6d37bd4 100644 --- a/plugins/chat/spec/services/chat/create_thread_spec.rb +++ b/plugins/chat/spec/services/chat/create_thread_spec.rb @@ -8,7 +8,7 @@ RSpec.describe Chat::CreateThread do end describe ".call" do - subject(:result) { described_class.call(params) } + subject(:result) { described_class.call(params:, **dependencies) } fab!(:current_user) { Fabricate(:user) } fab!(:channel_1) { Fabricate(:chat_channel, threading_enabled: true) } @@ -16,14 +16,8 @@ RSpec.describe Chat::CreateThread do let(:guardian) { Guardian.new(current_user) } let(:title) { nil } - let(:params) do - { - guardian: guardian, - original_message_id: message_1.id, - channel_id: channel_1.id, - title: title, - } - end + let(:params) { { original_message_id: message_1.id, channel_id: channel_1.id, title: } } + let(:dependencies) { { guardian: } } context "when all steps pass" do it { is_expected.to run_successfully } @@ -101,8 +95,10 @@ RSpec.describe Chat::CreateThread do before do Chat::CreateThread.call( guardian: current_user.guardian, - original_message_id: message_1.id, - channel_id: channel_1.id, + params: { + original_message_id: message_1.id, + channel_id: channel_1.id, + }, ) end diff --git a/plugins/chat/spec/services/chat/flag_message_spec.rb b/plugins/chat/spec/services/chat/flag_message_spec.rb index f382984a2c6..aeeab7175aa 100644 --- a/plugins/chat/spec/services/chat/flag_message_spec.rb +++ b/plugins/chat/spec/services/chat/flag_message_spec.rb @@ -11,7 +11,7 @@ RSpec.describe Chat::FlagMessage do end describe ".call" do - subject(:result) { described_class.call(params) } + subject(:result) { described_class.call(params:, **dependencies) } fab!(:current_user) { Fabricate(:user) } fab!(:channel_1) { Fabricate(:chat_channel) } @@ -26,7 +26,6 @@ RSpec.describe Chat::FlagMessage do let(:take_action) { nil } let(:params) do { - guardian: guardian, channel_id: channel_id, message_id:, flag_type_id: flag_type_id, @@ -35,6 +34,7 @@ RSpec.describe Chat::FlagMessage do take_action: take_action, } end + let(:dependencies) { { guardian: } } before { SiteSetting.direct_message_enabled_groups = Group::AUTO_GROUPS[:everyone] } diff --git a/plugins/chat/spec/services/chat/invite_users_to_channel_spec.rb b/plugins/chat/spec/services/chat/invite_users_to_channel_spec.rb index 02682d14836..82e370b69d2 100644 --- a/plugins/chat/spec/services/chat/invite_users_to_channel_spec.rb +++ b/plugins/chat/spec/services/chat/invite_users_to_channel_spec.rb @@ -7,7 +7,7 @@ RSpec.describe Chat::InviteUsersToChannel do end describe ".call" do - subject(:result) { described_class.call(**params, **dependencies) } + subject(:result) { described_class.call(params:, **dependencies) } fab!(:current_user) { Fabricate(:admin) } fab!(:user_1) { Fabricate(:user) } diff --git a/plugins/chat/spec/services/chat/leave_channel_spec.rb b/plugins/chat/spec/services/chat/leave_channel_spec.rb index 37b33d21b2f..36cfa3fa54d 100644 --- a/plugins/chat/spec/services/chat/leave_channel_spec.rb +++ b/plugins/chat/spec/services/chat/leave_channel_spec.rb @@ -2,24 +2,22 @@ RSpec.describe Chat::LeaveChannel do describe described_class::Contract, type: :model do - subject(:contract) { described_class.new } - it { is_expected.to validate_presence_of(:channel_id) } end describe ".call" do - subject(:result) { described_class.call(params) } + subject(:result) { described_class.call(params:, **dependencies) } fab!(:channel_1) { Fabricate(:chat_channel) } fab!(:current_user) { Fabricate(:user) } let(:guardian) { Guardian.new(current_user) } let(:channel_id) { channel_1.id } + let(:params) { { channel_id: } } + let(:dependencies) { { guardian: } } before { SiteSetting.direct_message_enabled_groups = Group::AUTO_GROUPS[:everyone] } - let(:params) { { guardian: guardian, channel_id: channel_id } } - context "when all steps pass" do context "when category channel" do context "with existing membership" do diff --git a/plugins/chat/spec/services/chat/list_channel_messages_spec.rb b/plugins/chat/spec/services/chat/list_channel_messages_spec.rb index a10c9d23ac2..968e1a66d45 100644 --- a/plugins/chat/spec/services/chat/list_channel_messages_spec.rb +++ b/plugins/chat/spec/services/chat/list_channel_messages_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true RSpec.describe Chat::ListChannelMessages do - subject(:result) { described_class.call(params) } + subject(:result) { described_class.call(params:, **dependencies) } fab!(:user) fab!(:channel) { Fabricate(:chat_channel) } @@ -9,7 +9,8 @@ RSpec.describe Chat::ListChannelMessages do let(:guardian) { Guardian.new(user) } let(:channel_id) { channel.id } let(:optional_params) { {} } - let(:params) { { guardian: guardian, channel_id: channel_id }.merge(optional_params) } + let(:params) { { channel_id: }.merge(optional_params) } + let(:dependencies) { { guardian: } } before { channel.add(user) } diff --git a/plugins/chat/spec/services/chat/list_channel_thread_messages_spec.rb b/plugins/chat/spec/services/chat/list_channel_thread_messages_spec.rb index 059a93b7f01..90357a0fa40 100644 --- a/plugins/chat/spec/services/chat/list_channel_thread_messages_spec.rb +++ b/plugins/chat/spec/services/chat/list_channel_thread_messages_spec.rb @@ -1,168 +1,184 @@ # frozen_string_literal: true RSpec.describe Chat::ListChannelThreadMessages do - subject(:result) { described_class.call(params) } - - fab!(:user) - fab!(:thread) { Fabricate(:chat_thread, channel: Fabricate(:chat_channel)) } - - let(:guardian) { Guardian.new(user) } - let(:thread_id) { thread.id } - let(:optional_params) { {} } - let(:params) { { guardian: guardian, thread_id: thread_id }.merge(optional_params) } - - before { thread.channel.add(user) } - - context "when contract" do - context "when thread_id is not present" do - let(:thread_id) { nil } - - it { is_expected.to fail_a_contract } + describe described_class::Contract, type: :model do + it { is_expected.to validate_presence_of(:thread_id) } + it do + is_expected.to validate_inclusion_of(:direction).in_array( + Chat::MessagesQuery::VALID_DIRECTIONS, + ).allow_nil end + it do + is_expected.to allow_values(Chat::MessagesQuery::MAX_PAGE_SIZE, 1, "1", nil).for(:page_size) + end + it { is_expected.not_to allow_values(Chat::MessagesQuery::MAX_PAGE_SIZE + 1).for(:page_size) } end - context "when fetch_thread" do - context "when thread doesn’t exist" do - let(:thread_id) { -1 } + describe ".call" do + subject(:result) { described_class.call(params:, **dependencies) } - it { is_expected.to fail_to_find_a_model(:thread) } - end + fab!(:user) + fab!(:thread) { Fabricate(:chat_thread, channel: Fabricate(:chat_channel)) } - context "when thread exists" do - it { is_expected.to run_successfully } + let(:guardian) { Guardian.new(user) } + let(:thread_id) { thread.id } + let(:optional_params) { {} } + let(:params) { { thread_id: }.merge(optional_params) } + let(:dependencies) { { guardian: } } - it "finds the correct channel" do - expect(result.thread).to eq(thread) + before { thread.channel.add(user) } + + context "when contract" do + context "when thread_id is not present" do + let(:thread_id) { nil } + + it { is_expected.to fail_a_contract } end end - end - context "when can_view_thread" do - context "when channel is private" do - fab!(:thread) { Fabricate(:chat_thread, channel: Fabricate(:private_category_channel)) } + context "when fetch_thread" do + context "when thread doesn’t exist" do + let(:thread_id) { -1 } - it { is_expected.to fail_a_policy(:can_view_thread) } + it { is_expected.to fail_to_find_a_model(:thread) } + end - context "with system user" do - fab!(:user) { Discourse.system_user } + context "when thread exists" do + it { is_expected.to run_successfully } + it "finds the correct channel" do + expect(result.thread).to eq(thread) + end + end + end + + context "when can_view_thread" do + context "when channel is private" do + fab!(:thread) { Fabricate(:chat_thread, channel: Fabricate(:private_category_channel)) } + + it { is_expected.to fail_a_policy(:can_view_thread) } + + context "with system user" do + fab!(:user) { Discourse.system_user } + + it { is_expected.to run_successfully } + end + end + end + + context "when determine_target_message_id" do + let(:optional_params) { { fetch_from_last_message: true } } + + context "when fetch_from_last_message is true" do + it "sets target_message_id to last thread message id" do + expect(result.target_message_id).to eq(thread.chat_messages.last.id) + end + end + + context "when fetch_from_first_message is true" do + it "sets target_message_id to first thread message id" do + expect(result.target_message_id).to eq(thread.chat_messages.first.id) + end + end + + context "when fetch_from_last_read is true" do + let(:optional_params) { { fetch_from_last_read: true } } + + before do + thread.add(user) + thread.membership_for(guardian.user).update!(last_read_message_id: 1) + end + + it "sets target_message_id to last_read_message_id" do + expect(result.target_message_id).to eq(1) + end + end + end + + context "when target_message_exists" do + context "when no target_message_id is given" do it { is_expected.to run_successfully } end - end - end - context "when determine_target_message_id" do - let(:optional_params) { { fetch_from_last_message: true } } + context "when target message is not found" do + let(:optional_params) { { target_message_id: -1 } } - context "when fetch_from_last_message is true" do - it "sets target_message_id to last thread message id" do - expect(result.target_message_id).to eq(thread.chat_messages.last.id) - end - end - - context "when fetch_from_first_message is true" do - it "sets target_message_id to first thread message id" do - expect(result.target_message_id).to eq(thread.chat_messages.first.id) - end - end - - context "when fetch_from_last_read is true" do - let(:optional_params) { { fetch_from_last_read: true } } - - before do - thread.add(user) - thread.membership_for(guardian.user).update!(last_read_message_id: 1) - end - - it "sets target_message_id to last_read_message_id" do - expect(result.target_message_id).to eq(1) - end - end - end - - context "when target_message_exists" do - context "when no target_message_id is given" do - it { is_expected.to run_successfully } - end - - context "when target message is not found" do - let(:optional_params) { { target_message_id: -1 } } - - it { is_expected.to fail_a_policy(:target_message_exists) } - end - - context "when target message is found" do - fab!(:target_message) do - Fabricate(:chat_message, chat_channel: thread.channel, thread: thread) - end - let(:optional_params) { { target_message_id: target_message.id } } - - it { is_expected.to run_successfully } - end - - context "when target message is trashed" do - fab!(:target_message) do - Fabricate(:chat_message, chat_channel: thread.channel, thread: thread) - end - let(:optional_params) { { target_message_id: target_message.id } } - - before { target_message.trash! } - - context "when user is regular" do it { is_expected.to fail_a_policy(:target_message_exists) } end - context "when user is the message creator" do + context "when target message is found" do fab!(:target_message) do - Fabricate(:chat_message, chat_channel: thread.channel, thread: thread, user: user) + Fabricate(:chat_message, chat_channel: thread.channel, thread: thread) + end + let(:optional_params) { { target_message_id: target_message.id } } + + it { is_expected.to run_successfully } + end + + context "when target message is trashed" do + fab!(:target_message) do + Fabricate(:chat_message, chat_channel: thread.channel, thread: thread) + end + let(:optional_params) { { target_message_id: target_message.id } } + + before { target_message.trash! } + + context "when user is regular" do + it { is_expected.to fail_a_policy(:target_message_exists) } end - it { is_expected.to run_successfully } - end + context "when user is the message creator" do + fab!(:target_message) do + Fabricate(:chat_message, chat_channel: thread.channel, thread: thread, user: user) + end - context "when user is admin" do - fab!(:user) { Fabricate(:admin) } + it { is_expected.to run_successfully } + end - it { is_expected.to run_successfully } - end - end - end + context "when user is admin" do + fab!(:user) { Fabricate(:admin) } - context "when fetch_messages" do - context "with not params" do - fab!(:messages) do - Fabricate.times(20, :chat_message, chat_channel: thread.channel, thread: thread) - end - - it "returns messages" do - expect(result.can_load_more_past).to eq(false) - expect(result.can_load_more_future).to eq(false) - expect(result.messages).to contain_exactly(thread.original_message, *messages) + it { is_expected.to run_successfully } + end end end - context "when target_date is provided" do - fab!(:past_message) do - Fabricate( - :chat_message, - chat_channel: thread.channel, - thread: thread, - created_at: 1.days.from_now, - ) - end - fab!(:future_message) do - Fabricate( - :chat_message, - chat_channel: thread.channel, - thread: thread, - created_at: 3.days.from_now, - ) + context "when fetch_messages" do + context "with not params" do + fab!(:messages) do + Fabricate.times(20, :chat_message, chat_channel: thread.channel, thread: thread) + end + + it "returns messages" do + expect(result.can_load_more_past).to eq(false) + expect(result.can_load_more_future).to eq(false) + expect(result.messages).to contain_exactly(thread.original_message, *messages) + end end - let(:optional_params) { { target_date: 2.days.ago } } + context "when target_date is provided" do + fab!(:past_message) do + Fabricate( + :chat_message, + chat_channel: thread.channel, + thread: thread, + created_at: 1.days.from_now, + ) + end + fab!(:future_message) do + Fabricate( + :chat_message, + chat_channel: thread.channel, + thread: thread, + created_at: 3.days.from_now, + ) + end - it "includes past and future messages" do - expect(result.messages).to eq([thread.original_message, past_message, future_message]) + let(:optional_params) { { target_date: 2.days.ago } } + + it "includes past and future messages" do + expect(result.messages).to eq([thread.original_message, past_message, future_message]) + end end end end diff --git a/plugins/chat/spec/services/chat/list_user_channels_spec.rb b/plugins/chat/spec/services/chat/list_user_channels_spec.rb index 6dc66947f63..82036f6425c 100644 --- a/plugins/chat/spec/services/chat/list_user_channels_spec.rb +++ b/plugins/chat/spec/services/chat/list_user_channels_spec.rb @@ -1,13 +1,14 @@ # frozen_string_literal: true RSpec.describe Chat::ListUserChannels do - subject(:result) { described_class.call(params) } + subject(:result) { described_class.call(params:, **dependencies) } fab!(:current_user) { Fabricate(:user) } fab!(:channel_1) { Fabricate(:chat_channel) } let(:guardian) { Guardian.new(current_user) } - let(:params) { { guardian: guardian } } + let(:params) { {} } + let(:dependencies) { { guardian: } } before { channel_1.add(current_user) } 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 05507470519..40926884e80 100644 --- a/plugins/chat/spec/services/chat/lookup_channel_threads_spec.rb +++ b/plugins/chat/spec/services/chat/lookup_channel_threads_spec.rb @@ -11,7 +11,7 @@ RSpec.describe ::Chat::LookupChannelThreads::Contract, type: :model do end RSpec.describe ::Chat::LookupChannelThreads do - subject(:result) { described_class.call(params) } + subject(:result) { described_class.call(params:, **dependencies) } fab!(:current_user) { Fabricate(:user) } @@ -19,7 +19,8 @@ RSpec.describe ::Chat::LookupChannelThreads do let(:channel_id) { nil } let(:limit) { 10 } let(:offset) { 0 } - let(:params) { { guardian: guardian, channel_id: channel_id, limit: limit, offset: offset } } + let(:params) { { channel_id:, limit:, offset: } } + let(:dependencies) { { guardian: } } describe "step - set_limit" do fab!(:channel_1) { Fabricate(:chat_channel) } diff --git a/plugins/chat/spec/services/chat/lookup_thread_spec.rb b/plugins/chat/spec/services/chat/lookup_thread_spec.rb index 65d9c190d96..5437759fc53 100644 --- a/plugins/chat/spec/services/chat/lookup_thread_spec.rb +++ b/plugins/chat/spec/services/chat/lookup_thread_spec.rb @@ -1,13 +1,13 @@ # frozen_string_literal: true RSpec.describe Chat::LookupThread do - describe Chat::LookupThread::Contract, type: :model do + describe described_class::Contract, type: :model do it { is_expected.to validate_presence_of :channel_id } it { is_expected.to validate_presence_of :thread_id } end describe ".call" do - subject(:result) { described_class.call(params) } + subject(:result) { described_class.call(params:, **dependencies) } fab!(:current_user) { Fabricate(:user) } fab!(:channel) { Fabricate(:chat_channel, threading_enabled: true) } @@ -16,7 +16,8 @@ RSpec.describe Chat::LookupThread do fab!(:other_thread) { Fabricate(:chat_thread) } let(:guardian) { Guardian.new(current_user) } - let(:params) { { guardian: guardian, thread_id: thread.id, channel_id: thread.channel_id } } + let(:params) { { thread_id: thread.id, channel_id: thread.channel_id } } + let(:dependencies) { { guardian: } } context "when all steps pass" do it { is_expected.to run_successfully } diff --git a/plugins/chat/spec/services/chat/lookup_user_threads_spec.rb b/plugins/chat/spec/services/chat/lookup_user_threads_spec.rb index a20758af42d..516dc0334ad 100644 --- a/plugins/chat/spec/services/chat/lookup_user_threads_spec.rb +++ b/plugins/chat/spec/services/chat/lookup_user_threads_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true RSpec.describe ::Chat::LookupUserThreads do - subject(:result) { described_class.call(params) } + subject(:result) { described_class.call(params:, **dependencies) } fab!(:current_user) { Fabricate(:user) } fab!(:channel_1) { Fabricate(:chat_channel, threading_enabled: true) } @@ -11,7 +11,8 @@ RSpec.describe ::Chat::LookupUserThreads do let(:channel_id) { channel_1.id } let(:limit) { 10 } let(:offset) { 0 } - let(:params) { { guardian: guardian, limit: limit, offset: offset } } + let(:params) { { limit:, offset: } } + let(:dependencies) { { guardian: } } before do channel_1.add(current_user) diff --git a/plugins/chat/spec/services/chat/mark_all_user_channels_read_spec.rb b/plugins/chat/spec/services/chat/mark_all_user_channels_read_spec.rb index a54a142f3b6..75a8a8675d3 100644 --- a/plugins/chat/spec/services/chat/mark_all_user_channels_read_spec.rb +++ b/plugins/chat/spec/services/chat/mark_all_user_channels_read_spec.rb @@ -2,9 +2,10 @@ RSpec.describe Chat::MarkAllUserChannelsRead do describe ".call" do - subject(:result) { described_class.call(params) } + subject(:result) { described_class.call(params:, **dependencies) } - let(:params) { { guardian: guardian } } + let(:params) { {} } + let(:dependencies) { { guardian: } } let(:guardian) { Guardian.new(current_user) } fab!(:current_user) { Fabricate(:user) } diff --git a/plugins/chat/spec/services/chat/mark_thread_title_prompt_seen_spec.rb b/plugins/chat/spec/services/chat/mark_thread_title_prompt_seen_spec.rb index 49b7b28cbff..ca1575057cf 100644 --- a/plugins/chat/spec/services/chat/mark_thread_title_prompt_seen_spec.rb +++ b/plugins/chat/spec/services/chat/mark_thread_title_prompt_seen_spec.rb @@ -1,13 +1,13 @@ # frozen_string_literal: true RSpec.describe Chat::MarkThreadTitlePromptSeen do - describe Chat::MarkThreadTitlePromptSeen::Contract, type: :model do + describe described_class::Contract, type: :model do it { is_expected.to validate_presence_of :channel_id } it { is_expected.to validate_presence_of :thread_id } end describe ".call" do - subject(:result) { described_class.call(params) } + subject(:result) { described_class.call(params:, **dependencies) } fab!(:current_user) { Fabricate(:user) } fab!(:channel) { Fabricate(:chat_channel, threading_enabled: true) } @@ -18,7 +18,8 @@ RSpec.describe Chat::MarkThreadTitlePromptSeen do fab!(:last_reply) { Fabricate(:chat_message, thread: thread, chat_channel: channel) } let(:guardian) { Guardian.new(current_user) } - let(:params) { { guardian: guardian, thread_id: thread.id, channel_id: thread.channel_id } } + let(:params) { { thread_id: thread.id, channel_id: thread.channel_id } } + let(:dependencies) { { guardian: } } before { thread.update!(last_message: last_reply) } diff --git a/plugins/chat/spec/services/chat/restore_message_spec.rb b/plugins/chat/spec/services/chat/restore_message_spec.rb index 3f8e4452f3c..0832df5a243 100644 --- a/plugins/chat/spec/services/chat/restore_message_spec.rb +++ b/plugins/chat/spec/services/chat/restore_message_spec.rb @@ -13,18 +13,18 @@ RSpec.describe Chat::RestoreMessage do end describe ".call" do - subject(:result) { described_class.call(params) } + subject(:result) { described_class.call(params:, **dependencies) } + + let(:dependencies) { { guardian: } } context "when params are not valid" do - let(:params) { { guardian: guardian } } + let(:params) { {} } it { is_expected.to fail_a_contract } end context "when params are valid" do - let(:params) do - { guardian: guardian, message_id: message.id, channel_id: message.chat_channel_id } - end + let(:params) { { message_id: message.id, channel_id: message.chat_channel_id } } context "when the user does not have permission to restore" do before { message.update!(user: Fabricate(:admin)) } @@ -33,9 +33,7 @@ RSpec.describe Chat::RestoreMessage do end context "when the channel does not match the message" do - let(:params) do - { guardian: guardian, message_id: message.id, channel_id: Fabricate(:chat_channel).id } - end + let(:params) { { message_id: message.id, channel_id: Fabricate(:chat_channel).id } } it { is_expected.to fail_to_find_a_model(:message) } end diff --git a/plugins/chat/spec/services/chat/search_chatable_spec.rb b/plugins/chat/spec/services/chat/search_chatable_spec.rb index 174d395c217..e2c153e28ca 100644 --- a/plugins/chat/spec/services/chat/search_chatable_spec.rb +++ b/plugins/chat/spec/services/chat/search_chatable_spec.rb @@ -2,7 +2,7 @@ RSpec.describe Chat::SearchChatable do describe ".call" do - subject(:result) { described_class.call(params) } + subject(:result) { described_class.call(params:, **dependencies) } fab!(:current_user) { Fabricate(:user, username: "bob-user") } fab!(:sam) { Fabricate(:user, username: "sam-user") } @@ -25,15 +25,15 @@ RSpec.describe Chat::SearchChatable do let(:excluded_memberships_channel_id) { nil } let(:params) do { - guardian: guardian, - term: term, - include_users: include_users, - include_groups: include_groups, - include_category_channels: include_category_channels, - include_direct_message_channels: include_direct_message_channels, - excluded_memberships_channel_id: excluded_memberships_channel_id, + term:, + include_users:, + include_groups:, + include_category_channels:, + include_direct_message_channels:, + excluded_memberships_channel_id:, } end + let(:dependencies) { { guardian: } } before do SiteSetting.direct_message_enabled_groups = Group::AUTO_GROUPS[:everyone] diff --git a/plugins/chat/spec/services/chat/stop_message_streaming_spec.rb b/plugins/chat/spec/services/chat/stop_message_streaming_spec.rb index 8e7b21a33fc..6aa56cfb4e0 100644 --- a/plugins/chat/spec/services/chat/stop_message_streaming_spec.rb +++ b/plugins/chat/spec/services/chat/stop_message_streaming_spec.rb @@ -6,10 +6,11 @@ RSpec.describe Chat::StopMessageStreaming do end describe ".call" do - subject(:result) { described_class.call(params) } + subject(:result) { described_class.call(params:, **dependencies) } let(:guardian) { Guardian.new(current_user) } - let(:params) { { guardian: guardian, message_id: message_1.id } } + let(:params) { { message_id: message_1.id } } + let(:dependencies) { { guardian: guardian } } fab!(:current_user) { Fabricate(:user) } fab!(:channel_1) { Fabricate(:chat_channel) } diff --git a/plugins/chat/spec/services/chat/tracking_state_spec.rb b/plugins/chat/spec/services/chat/tracking_state_spec.rb index f51e979da99..a484f1793de 100644 --- a/plugins/chat/spec/services/chat/tracking_state_spec.rb +++ b/plugins/chat/spec/services/chat/tracking_state_spec.rb @@ -2,7 +2,7 @@ RSpec.describe ::Chat::TrackingState do describe ".call" do - subject(:result) { described_class.call(params) } + subject(:result) { described_class.call(params:, **dependencies) } fab!(:current_user) { Fabricate(:user) } fab!(:channel_1) { Fabricate(:chat_channel, threading_enabled: true) } @@ -17,12 +17,8 @@ RSpec.describe ::Chat::TrackingState do let(:include_threads) { true } let(:include_missing_memberships) { nil } - let(:params) do - id_params.merge(guardian: guardian).merge( - include_threads: include_threads, - include_missing_memberships: include_missing_memberships, - ) - end + let(:params) { id_params.merge(include_threads:, include_missing_memberships:) } + let(:dependencies) { { guardian: } } fab!(:channel_1_membership) do Fabricate(:user_chat_channel_membership, chat_channel: channel_1, user: current_user) diff --git a/plugins/chat/spec/services/chat/trash_channel_spec.rb b/plugins/chat/spec/services/chat/trash_channel_spec.rb index d5fa1009636..097f1824dd1 100644 --- a/plugins/chat/spec/services/chat/trash_channel_spec.rb +++ b/plugins/chat/spec/services/chat/trash_channel_spec.rb @@ -1,30 +1,30 @@ # frozen_string_literal: true -RSpec.describe(Chat::TrashChannel) do - subject(:result) { described_class.call(guardian: guardian) } +RSpec.describe Chat::TrashChannel do + subject(:result) { described_class.call(params:, **dependencies) } + fab!(:current_user) { Fabricate(:admin) } + fab!(:channel) { Fabricate(:chat_channel) } + + let(:params) { { channel_id: } } + let(:dependencies) { { guardian: } } let(:guardian) { Guardian.new(current_user) } + let(:channel_id) { channel.id } context "when channel_id is not provided" do - fab!(:current_user) { Fabricate(:admin) } + let(:channel_id) { nil } it { is_expected.to fail_to_find_a_model(:channel) } end context "when channel_id is provided" do - subject(:result) { described_class.call(channel_id: channel.id, guardian: guardian) } - - fab!(:channel) { Fabricate(:chat_channel) } - context "when user is not allowed to perform the action" do - fab!(:current_user) { Fabricate(:user) } + let!(:current_user) { Fabricate(:user) } it { is_expected.to fail_a_policy(:invalid_access) } end context "when user is allowed to perform the action" do - fab!(:current_user) { Fabricate(:admin) } - it { is_expected.to run_successfully } it "trashes the channel" do diff --git a/plugins/chat/spec/services/chat/trash_message_spec.rb b/plugins/chat/spec/services/chat/trash_message_spec.rb index d9dff95ec00..a0bcd5c2264 100644 --- a/plugins/chat/spec/services/chat/trash_message_spec.rb +++ b/plugins/chat/spec/services/chat/trash_message_spec.rb @@ -7,7 +7,7 @@ RSpec.describe Chat::TrashMessage do end describe ".call" do - subject(:result) { described_class.call(**params, **dependencies) } + subject(:result) { described_class.call(params:, **dependencies) } fab!(:current_user) { Fabricate(:user) } fab!(:message) { Fabricate(:chat_message, user: current_user) } diff --git a/plugins/chat/spec/services/chat/trash_messages_spec.rb b/plugins/chat/spec/services/chat/trash_messages_spec.rb index c0dae7e7aac..8961682ee85 100644 --- a/plugins/chat/spec/services/chat/trash_messages_spec.rb +++ b/plugins/chat/spec/services/chat/trash_messages_spec.rb @@ -8,7 +8,7 @@ RSpec.describe Chat::TrashMessages do end describe ".call" do - subject(:result) { described_class.call(**params, **dependencies) } + subject(:result) { described_class.call(params:, **dependencies) } fab!(:current_user) { Fabricate(:user) } fab!(:chat_channel) { Fabricate(:chat_channel) } diff --git a/plugins/chat/spec/services/chat/unfollow_channel_spec.rb b/plugins/chat/spec/services/chat/unfollow_channel_spec.rb index 3199852f132..b5bf056671d 100644 --- a/plugins/chat/spec/services/chat/unfollow_channel_spec.rb +++ b/plugins/chat/spec/services/chat/unfollow_channel_spec.rb @@ -8,27 +8,27 @@ RSpec.describe Chat::UnfollowChannel do end describe ".call" do - subject(:result) { described_class.call(params) } + subject(:result) { described_class.call(params:, **dependencies) } fab!(:channel_1) { Fabricate(:chat_channel) } fab!(:current_user) { Fabricate(:user) } + let(:params) { { channel_id: } } + let(:dependencies) { { guardian: } } let(:guardian) { Guardian.new(current_user) } let(:channel_id) { channel_1.id } before { SiteSetting.direct_message_enabled_groups = Group::AUTO_GROUPS[:everyone] } - let(:params) { { guardian: guardian, channel_id: channel_id } } - context "when all steps pass" do context "with existing membership" do + let(:membership) { channel_1.membership_for(current_user) } + before { channel_1.add(current_user) } it { is_expected.to run_successfully } it "unfollows the channel" do - membership = channel_1.membership_for(current_user) - expect { result }.to change { membership.reload.following }.from(true).to(false) end end @@ -43,7 +43,7 @@ RSpec.describe Chat::UnfollowChannel do end context "when channel is not found" do - before { params[:channel_id] = -999 } + let(:channel_id) { -999 } it { is_expected.to fail_to_find_a_model(:channel) } end diff --git a/plugins/chat/spec/services/chat/update_channel_spec.rb b/plugins/chat/spec/services/chat/update_channel_spec.rb index 2f6f49cb536..046163b4c4a 100644 --- a/plugins/chat/spec/services/chat/update_channel_spec.rb +++ b/plugins/chat/spec/services/chat/update_channel_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true RSpec.describe Chat::UpdateChannel do - subject(:result) { described_class.call(guardian: guardian, channel_id: channel.id, **params) } + subject(:result) { described_class.call(params:, **dependencies) } fab!(:channel) { Fabricate(:chat_channel) } fab!(:current_user) { Fabricate(:admin) } @@ -9,6 +9,7 @@ RSpec.describe Chat::UpdateChannel do let(:guardian) { Guardian.new(current_user) } let(:params) do { + channel_id: channel.id, name: "cool channel", description: "a channel description", slug: "snail", @@ -16,6 +17,7 @@ RSpec.describe Chat::UpdateChannel do auto_join_users: false, } end + let(:dependencies) { { guardian: } } context "when the user cannot edit the channel" do fab!(:current_user) { Fabricate(:user) } diff --git a/plugins/chat/spec/services/chat/update_channel_status_spec.rb b/plugins/chat/spec/services/chat/update_channel_status_spec.rb index 9df907e8d2f..e7a6e143fb6 100644 --- a/plugins/chat/spec/services/chat/update_channel_status_spec.rb +++ b/plugins/chat/spec/services/chat/update_channel_status_spec.rb @@ -8,7 +8,7 @@ RSpec.describe(Chat::UpdateChannelStatus) do end describe ".call" do - subject(:result) { described_class.call(**params, **dependencies) } + subject(:result) { described_class.call(params:, **dependencies) } fab!(:channel) { Fabricate(:chat_channel) } fab!(:current_user) { Fabricate(:admin) } diff --git a/plugins/chat/spec/services/chat/update_message_spec.rb b/plugins/chat/spec/services/chat/update_message_spec.rb index edda8fa1b8e..625be4852c9 100644 --- a/plugins/chat/spec/services/chat/update_message_spec.rb +++ b/plugins/chat/spec/services/chat/update_message_spec.rb @@ -64,7 +64,13 @@ RSpec.describe Chat::UpdateMessage do new_message = "2 short" expect do - described_class.call(guardian: guardian, message_id: chat_message.id, message: new_message) + described_class.call( + guardian: guardian, + params: { + message_id: chat_message.id, + message: new_message, + }, + ) end.to raise_error(ActiveRecord::RecordInvalid).with_message( "Validation failed: " + I18n.t( @@ -83,7 +89,13 @@ RSpec.describe Chat::UpdateMessage do new_message = "2 long" * 100 expect do - described_class.call(guardian: guardian, message_id: chat_message.id, message: new_message) + described_class.call( + guardian: guardian, + params: { + message_id: chat_message.id, + message: new_message, + }, + ) end.to raise_error(ActiveRecord::RecordInvalid).with_message( "Validation failed: " + I18n.t( @@ -95,46 +107,17 @@ RSpec.describe Chat::UpdateMessage do expect(chat_message.reload.message).to eq(og_message) end - it "errors when a blank message is sent" do - og_message = "This won't be changed!" - chat_message = create_chat_message(user1, og_message, public_chat_channel) - new_message = " " - - updater = - described_class.call(guardian: guardian, message_id: chat_message.id, message: new_message) - - expect(updater.contract).not_to be_valid - expect(updater.contract.errors.added?(:message, :blank)).to be_truthy - expect(chat_message.reload.message).to eq(og_message) - end - - it "errors if a user other than the message user is trying to edit the message" do - og_message = "This won't be changed!" - chat_message = create_chat_message(user1, og_message, public_chat_channel) - new_message = "2 short" - updater = - described_class.call( - guardian: Guardian.new(Fabricate(:user)), - message_id: chat_message.id, - message: new_message, - ) - - expect(updater.message.reload.message).not_to eq(new_message) - end - - it "updates a message's content" do - chat_message = create_chat_message(user1, "This will be changed", public_chat_channel) - new_message = "Change to this!" - - described_class.call(guardian: guardian, message_id: chat_message.id, message: new_message) - expect(chat_message.reload.message).to eq(new_message) - end - it "cleans message's content" do chat_message = create_chat_message(user1, "This will be changed", public_chat_channel) new_message = "bbbbb\n" - described_class.call(guardian: guardian, message_id: chat_message.id, message: new_message) + described_class.call( + guardian: guardian, + params: { + message_id: chat_message.id, + message: new_message, + }, + ) expect(chat_message.reload.message).to eq("bbbbb") end @@ -145,9 +128,13 @@ RSpec.describe Chat::UpdateMessage do described_class.call( guardian: guardian, - message_id: chat_message.id, - message: new_message, - strip_whitespaces: false, + options: { + strip_whitespaces: false, + }, + params: { + message_id: chat_message.id, + message: new_message, + }, ) expect(chat_message.reload.message).to eq("bbbbb\n") end @@ -157,7 +144,13 @@ RSpec.describe Chat::UpdateMessage do chat_message = create_chat_message(user1, "This will be changed", public_chat_channel) new_message = "Change **to** this!" - described_class.call(guardian: guardian, message_id: chat_message.id, message: new_message) + described_class.call( + guardian: guardian, + params: { + message_id: chat_message.id, + message: new_message, + }, + ) expect(chat_message.reload.cooked).to eq("

Change to this!

") end @@ -166,8 +159,10 @@ RSpec.describe Chat::UpdateMessage do described_class.call( guardian: guardian, - message_id: chat_message.id, - message: "Change to this!", + params: { + message_id: chat_message.id, + message: "Change to this!", + }, ) expect(chat_message.reload.excerpt).to eq("Change to this!") end @@ -178,8 +173,10 @@ RSpec.describe Chat::UpdateMessage do DiscourseEvent.track_events do described_class.call( guardian: guardian, - message_id: chat_message.id, - message: "Change to this!", + params: { + message_id: chat_message.id, + message: "Change to this!", + }, ) end expect(events.map { _1[:event_name] }).to include(:chat_message_edited) @@ -194,8 +191,10 @@ RSpec.describe Chat::UpdateMessage do .track_publish("/chat/#{public_chat_channel.id}") do described_class.call( guardian: guardian, - message_id: chat_message.id, - message: new_content, + params: { + message_id: chat_message.id, + message: new_content, + }, ) end .detect { |m| m.data["type"] == "edit" } @@ -210,8 +209,10 @@ RSpec.describe Chat::UpdateMessage do described_class.call( guardian: guardian, - message_id: message.id, - message: "Mentioning @#{user2.username} and @#{user3.username}", + params: { + message_id: message.id, + message: "Mentioning @#{user2.username} and @#{user3.username}", + }, ) mention = user3.chat_mentions.where(chat_message: message.id).first @@ -224,8 +225,10 @@ RSpec.describe Chat::UpdateMessage do expect { described_class.call( guardian: guardian, - message_id: chat_message.id, - message: message + " editedddd", + params: { + message_id: chat_message.id, + message: message + " editedddd", + }, ) }.not_to change { Chat::Mention.count } end @@ -236,8 +239,10 @@ RSpec.describe Chat::UpdateMessage do described_class.call( guardian: guardian, - message_id: chat_message.id, - message: message + " @#{user_without_memberships.username}", + params: { + message_id: chat_message.id, + message: message + " @#{user_without_memberships.username}", + }, ) mention = user_without_memberships.chat_mentions.where(chat_message: chat_message).first @@ -254,8 +259,10 @@ RSpec.describe Chat::UpdateMessage do expect { described_class.call( guardian: guardian, - message_id: chat_message.id, - message: "ping @#{user3.username}", + params: { + message_id: chat_message.id, + message: "ping @#{user3.username}", + }, ) }.to change { user2.chat_mentions.count }.by(-1).and not_change { user3.chat_mentions.count @@ -271,8 +278,10 @@ RSpec.describe Chat::UpdateMessage do ) described_class.call( guardian: guardian, - message_id: chat_message.id, - message: "ping @#{user3.username} @#{user4.username}", + params: { + message_id: chat_message.id, + message: "ping @#{user3.username} @#{user4.username}", + }, ) expect(user2.chat_mentions.where(chat_message: chat_message)).not_to be_present @@ -284,7 +293,9 @@ RSpec.describe Chat::UpdateMessage do result = Chat::CreateDirectMessageChannel.call( guardian: user1.guardian, - target_usernames: [user1.username, user2.username], + params: { + target_usernames: [user1.username, user2.username], + }, ) service_failed!(result) if result.failure? direct_message_channel = result.channel @@ -292,8 +303,10 @@ RSpec.describe Chat::UpdateMessage do described_class.call( guardian: guardian, - message_id: message.id, - message: "ping @#{admin1.username}", + params: { + message_id: message.id, + message: "ping @#{admin1.username}", + }, ) mention = admin1.chat_mentions.where(chat_message_id: message.id).first @@ -304,7 +317,13 @@ RSpec.describe Chat::UpdateMessage do chat_message = create_chat_message(user1, "I will mention myself soon", public_chat_channel) new_content = "hello @#{user1.username}" - described_class.call(guardian: guardian, message_id: chat_message.id, message: new_content) + described_class.call( + guardian: guardian, + params: { + message_id: chat_message.id, + message: new_content, + }, + ) mention = user1.chat_mentions.where(chat_message: chat_message).first expect(mention).to be_present @@ -323,8 +342,10 @@ RSpec.describe Chat::UpdateMessage do .track_publish("/chat/#{public_chat_channel.id}") do described_class.call( guardian: guardian, - message_id: chat_message.id, - message: new_content, + params: { + message_id: chat_message.id, + message: new_content, + }, ) end .detect { |m| m.data["type"] == "processed" } @@ -349,8 +370,10 @@ RSpec.describe Chat::UpdateMessage do .track_publish("/chat/#{public_chat_channel.id}") do described_class.call( guardian: guardian, - message_id: chat_message.id, - message: "Hey @#{user2.username}", + params: { + message_id: chat_message.id, + message: "Hey @#{user2.username}", + }, ) end .detect { |m| m.data["type"] == "processed" } @@ -368,8 +391,10 @@ RSpec.describe Chat::UpdateMessage do described_class.call( guardian: guardian, - message_id: chat_message.id, - message: "ping @#{user3.username}", + params: { + message_id: chat_message.id, + message: "ping @#{user3.username}", + }, ) user2_mentions = user2.chat_mentions.where(chat_message: chat_message) @@ -386,8 +411,10 @@ RSpec.describe Chat::UpdateMessage do described_class.call( guardian: guardian, - message_id: chat_message.id, - message: "ping @#{user2.username} @#{user2.username} edited", + params: { + message_id: chat_message.id, + message: "ping @#{user2.username} @#{user2.username} edited", + }, ) expect(user2.chat_mentions.where(chat_message: chat_message).count).to eq(1) @@ -415,8 +442,10 @@ RSpec.describe Chat::UpdateMessage do described_class.call( guardian: guardian, - message_id: chat_message.id, - message: "ping @#{group_1.name}", + params: { + message_id: chat_message.id, + message: "ping @#{group_1.name}", + }, ) expect(group_1.chat_mentions.where(chat_message: chat_message).count).to be(1) @@ -429,8 +458,10 @@ RSpec.describe Chat::UpdateMessage do described_class.call( guardian: guardian, - message_id: chat_message.id, - message: "ping @#{group_2.name}", + params: { + message_id: chat_message.id, + message: "ping @#{group_2.name}", + }, ) expect(chat_message.reload.group_mentions.map(&:target_id)).to contain_exactly(group_2.id) @@ -441,8 +472,10 @@ RSpec.describe Chat::UpdateMessage do described_class.call( guardian: guardian, - message_id: chat_message.id, - message: "ping nobody anymore!", + params: { + message_id: chat_message.id, + message: "ping nobody anymore!", + }, ) expect(group_1.chat_mentions.where(chat_message: chat_message).count).to be(0) @@ -465,8 +498,10 @@ RSpec.describe Chat::UpdateMessage do described_class.call( guardian: guardian, - message_id: chat_message.id, - message: "Update the message and still mention the same group @#{group.name}", + params: { + message_id: chat_message.id, + message: "Update the message and still mention the same group @#{group.name}", + }, ) expect(group_user.notifications.count).to be(1) # no new notifications has been created @@ -486,8 +521,10 @@ RSpec.describe Chat::UpdateMessage do described_class.call( guardian: guardian, - message_id: chat_message.id, - message: "Update the message and still mention @here", + params: { + message_id: chat_message.id, + message: "Update the message and still mention @here", + }, ) expect(user.notifications.count).to be(1) # no new notifications have been created @@ -505,8 +542,10 @@ RSpec.describe Chat::UpdateMessage do described_class.call( guardian: guardian, - message_id: chat_message.id, - message: "Update the message and still mention @all", + params: { + message_id: chat_message.id, + message: "Update the message and still mention @all", + }, ) expect(user.notifications.count).to be(1) # no new notifications have been created @@ -522,7 +561,13 @@ RSpec.describe Chat::UpdateMessage do old_message = "It's a thrsday!" new_message = "Today is Thursday, it's almost the weekend already!" chat_message = create_chat_message(user1, old_message, public_chat_channel) - described_class.call(guardian: guardian, message_id: chat_message.id, message: new_message) + described_class.call( + guardian: guardian, + params: { + message_id: chat_message.id, + message: new_message, + }, + ) revision = chat_message.revisions.last expect(revision.old_message).to eq(old_message) @@ -556,8 +601,10 @@ RSpec.describe Chat::UpdateMessage do expect do described_class.call( guardian: guardian, - message_id: chat_message_1.id, - message: "another different chat message here", + params: { + message_id: chat_message_1.id, + message: "another different chat message here", + }, ) end.to raise_error(ActiveRecord::RecordInvalid).with_message( "Validation failed: " + I18n.t("chat.errors.duplicate_message"), @@ -577,9 +624,11 @@ RSpec.describe Chat::UpdateMessage do updater = described_class.call( guardian: guardian, - message_id: chat_message.id, - message: "this is some chat message", - upload_ids: [upload2.id], + params: { + message_id: chat_message.id, + message: "this is some chat message", + upload_ids: [upload2.id], + }, ) expect(updater.message).to be_valid expect(chat_message.reload.uploads.count).to eq(1) @@ -601,9 +650,11 @@ RSpec.describe Chat::UpdateMessage do expect { described_class.call( guardian: guardian, - message_id: chat_message.id, - message: "I guess this is different", - upload_ids: [upload2.id, upload1.id], + params: { + message_id: chat_message.id, + message: "I guess this is different", + upload_ids: [upload2.id, upload1.id], + }, ) }.to not_change { UploadReference.count } end @@ -620,9 +671,11 @@ RSpec.describe Chat::UpdateMessage do expect { described_class.call( guardian: guardian, - message_id: chat_message.id, - message: "I guess this is different", - upload_ids: [upload1.id], + params: { + message_id: chat_message.id, + message: "I guess this is different", + upload_ids: [upload1.id], + }, ) }.to change { UploadReference.where(upload_id: upload2.id).count }.by(-1) end @@ -639,9 +692,11 @@ RSpec.describe Chat::UpdateMessage do expect { described_class.call( guardian: guardian, - message_id: chat_message.id, - message: "I guess this is different", - upload_ids: [], + params: { + message_id: chat_message.id, + message: "I guess this is different", + upload_ids: [], + }, ) }.to change { UploadReference.where(target: chat_message).count }.by(-2) end @@ -651,9 +706,11 @@ RSpec.describe Chat::UpdateMessage do expect { described_class.call( guardian: guardian, - message_id: chat_message.id, - message: "I guess this is different", - upload_ids: [upload1.id], + params: { + message_id: chat_message.id, + message: "I guess this is different", + upload_ids: [upload1.id], + }, ) }.to change { UploadReference.where(target: chat_message).count }.by(1) end @@ -663,9 +720,11 @@ RSpec.describe Chat::UpdateMessage do expect { described_class.call( guardian: guardian, - message_id: chat_message.id, - message: "I guess this is different", - upload_ids: [upload1.id, upload2.id], + params: { + message_id: chat_message.id, + message: "I guess this is different", + upload_ids: [upload1.id, upload2.id], + }, ) }.to change { UploadReference.where(target: chat_message).count }.by(2) end @@ -676,9 +735,11 @@ RSpec.describe Chat::UpdateMessage do expect { described_class.call( guardian: guardian, - message_id: chat_message, - message: "I guess this is different", - upload_ids: [0], + params: { + message_id: chat_message, + message: "I guess this is different", + upload_ids: [0], + }, ) }.to not_change { UploadReference.where(target: chat_message).count } end @@ -689,9 +750,11 @@ RSpec.describe Chat::UpdateMessage do expect { described_class.call( guardian: guardian, - message_id: chat_message.id, - message: "I guess this is different", - upload_ids: [upload1.id, upload2.id], + params: { + message_id: chat_message.id, + message: "I guess this is different", + upload_ids: [upload1.id, upload2.id], + }, ) }.to not_change { UploadReference.where(target: chat_message).count } end @@ -708,9 +771,11 @@ RSpec.describe Chat::UpdateMessage do expect { described_class.call( guardian: guardian, - message_id: chat_message.id, - message: "I guess this is different", - upload_ids: [], + params: { + message_id: chat_message.id, + message: "I guess this is different", + upload_ids: [], + }, ) }.to not_change { UploadReference.where(target: chat_message).count } end @@ -727,9 +792,11 @@ RSpec.describe Chat::UpdateMessage do new_message = "hi :)" described_class.call( guardian: guardian, - message_id: chat_message.id, - message: new_message, - upload_ids: [upload1.id], + params: { + message_id: chat_message.id, + message: new_message, + upload_ids: [upload1.id], + }, ) expect(chat_message.reload.message).to eq(new_message) end @@ -750,8 +817,10 @@ RSpec.describe Chat::UpdateMessage do MessageBus.track_publish("/chat/#{public_chat_channel.id}") do described_class.call( guardian: guardian, - message_id: message.id, - message: "some new updated content", + params: { + message_id: message.id, + message: "some new updated content", + }, ) end expect( @@ -773,8 +842,10 @@ RSpec.describe Chat::UpdateMessage do expect do described_class.call( guardian: guardian, - message_id: chat_message.id, - message: "bad word - #{watched_word.word}", + params: { + message_id: chat_message.id, + message: "bad word - #{watched_word.word}", + }, ) end.to raise_error(ActiveRecord::RecordInvalid).with_message(msg) @@ -786,8 +857,10 @@ RSpec.describe Chat::UpdateMessage do described_class.call( guardian: guardian, - message_id: chat_message.id, - message: "bad word - #{censored_word.word}", + params: { + message_id: chat_message.id, + message: "bad word - #{censored_word.word}", + }, ) expect(chat_message.reload.excerpt).to eq("bad word - ■■■■") @@ -801,8 +874,10 @@ RSpec.describe Chat::UpdateMessage do message.update!(user: user) described_class.call( guardian: Guardian.new(user), - message_id: message.id, - message: "I guess this is different", + params: { + message_id: message.id, + message: "I guess this is different", + }, ) end @@ -810,7 +885,7 @@ RSpec.describe Chat::UpdateMessage do before { public_chat_channel.update(status: :closed) } it "errors when trying to update the message for non-staff" do - updater = update_message(user1) + update_message(user1) expect(message.reload.message).not_to eq("I guess this is different") end @@ -824,10 +899,10 @@ RSpec.describe Chat::UpdateMessage do before { public_chat_channel.update(status: :read_only) } it "errors when trying to update the message for all users" do - updater = update_message(user1) + update_message(user1) expect(message.reload.message).not_to eq("I guess this is different") - updater = update_message(admin1) + update_message(admin1) expect(message.reload.message).not_to eq("I guess this is different") end end @@ -836,10 +911,10 @@ RSpec.describe Chat::UpdateMessage do before { public_chat_channel.update(status: :archived) } it "errors when trying to update the message for all users" do - updater = update_message(user1) + update_message(user1) expect(message.reload.message).not_to eq("I guess this is different") - updater = update_message(admin1) + update_message(admin1) expect(message.reload.message).not_to eq("I guess this is different") end end @@ -847,7 +922,7 @@ RSpec.describe Chat::UpdateMessage do end describe ".call" do - subject(:result) { described_class.call(params) } + subject(:result) { described_class.call(params:, options:, **dependencies) } fab!(:current_user) { Fabricate(:user) } fab!(:channel_1) { Fabricate(:chat_channel) } @@ -866,9 +941,9 @@ RSpec.describe Chat::UpdateMessage do let(:message) { "new" } let(:message_id) { message_1.id } let(:upload_ids) { [upload_1.id] } - let(:params) do - { guardian: guardian, message_id: message_id, message: message, upload_ids: upload_ids } - end + let(:params) { { message_id: message_id, message: message, upload_ids: upload_ids } } + let(:dependencies) { { guardian: guardian } } + let(:options) { {} } before do SiteSetting.chat_editing_grace_period = 30 @@ -905,12 +980,12 @@ RSpec.describe Chat::UpdateMessage do end it "can enqueue a job to process message" do - params[:process_inline] = false + options[:process_inline] = false expect_enqueued_with(job: Jobs::Chat::ProcessMessage) { result } end it "can process a message inline" do - params[:process_inline] = true + options[:process_inline] = true Jobs::Chat::ProcessMessage.any_instance.expects(:execute).once expect_not_enqueued_with(job: Jobs::Chat::ProcessMessage) { result } end diff --git a/plugins/chat/spec/services/chat/update_thread_notification_settings_spec.rb b/plugins/chat/spec/services/chat/update_thread_notification_settings_spec.rb index ff72bfbfa94..31ad68abd2d 100644 --- a/plugins/chat/spec/services/chat/update_thread_notification_settings_spec.rb +++ b/plugins/chat/spec/services/chat/update_thread_notification_settings_spec.rb @@ -11,7 +11,7 @@ RSpec.describe Chat::UpdateThreadNotificationSettings do end describe ".call" do - subject(:result) { described_class.call(params) } + subject(:result) { described_class.call(params:, **dependencies) } fab!(:current_user) { Fabricate(:user) } fab!(:channel) { Fabricate(:chat_channel, threading_enabled: true) } @@ -22,12 +22,12 @@ RSpec.describe Chat::UpdateThreadNotificationSettings do let(:guardian) { Guardian.new(current_user) } let(:params) do { - guardian: guardian, thread_id: thread.id, channel_id: thread.channel_id, notification_level: Chat::UserChatThreadMembership.notification_levels[:normal], } end + let(:dependencies) { { guardian: } } before { thread.update!(last_message: last_reply) } diff --git a/plugins/chat/spec/services/chat/update_thread_spec.rb b/plugins/chat/spec/services/chat/update_thread_spec.rb index 00f8f48fbbd..8e20208b312 100644 --- a/plugins/chat/spec/services/chat/update_thread_spec.rb +++ b/plugins/chat/spec/services/chat/update_thread_spec.rb @@ -7,7 +7,7 @@ RSpec.describe Chat::UpdateThread do end describe ".call" do - subject(:result) { described_class.call(params) } + subject(:result) { described_class.call(params:, **dependencies) } fab!(:current_user) { Fabricate(:user) } fab!(:channel) { Fabricate(:chat_channel, threading_enabled: true) } @@ -17,7 +17,8 @@ RSpec.describe Chat::UpdateThread do let(:guardian) { Guardian.new(current_user) } let(:title) { "some new title :D" } - let(:params) { { guardian: guardian, thread_id: thread.id, title: title } } + let(:params) { { thread_id: thread.id, title: } } + let(:dependencies) { { guardian: } } context "when all steps pass" do it { is_expected.to run_successfully } diff --git a/plugins/chat/spec/services/chat/update_user_channel_last_read_spec.rb b/plugins/chat/spec/services/chat/update_user_channel_last_read_spec.rb index e3b045fc8c6..0e11c629a09 100644 --- a/plugins/chat/spec/services/chat/update_user_channel_last_read_spec.rb +++ b/plugins/chat/spec/services/chat/update_user_channel_last_read_spec.rb @@ -1,13 +1,13 @@ # frozen_string_literal: true RSpec.describe Chat::UpdateUserChannelLastRead do - describe Chat::UpdateUserChannelLastRead::Contract, type: :model do + describe described_class::Contract, type: :model do it { is_expected.to validate_presence_of :channel_id } it { is_expected.to validate_presence_of :message_id } end describe ".call" do - subject(:result) { described_class.call(params) } + subject(:result) { described_class.call(params:, **dependencies) } fab!(:chatters) { Fabricate(:group) } fab!(:current_user) { Fabricate(:user, group_ids: [chatters.id]) } @@ -18,7 +18,8 @@ RSpec.describe Chat::UpdateUserChannelLastRead do let(:message_1) { Fabricate(:chat_message, chat_channel: membership.chat_channel) } let(:guardian) { Guardian.new(current_user) } - let(:params) { { guardian: guardian, channel_id: channel.id, message_id: message_1.id } } + let(:params) { { channel_id: channel.id, message_id: message_1.id } } + let(:dependencies) { { guardian: } } before { SiteSetting.chat_allowed_groups = chatters } diff --git a/plugins/chat/spec/services/chat/update_user_thread_last_read_spec.rb b/plugins/chat/spec/services/chat/update_user_thread_last_read_spec.rb index f7251a16b04..97ab7d23e73 100644 --- a/plugins/chat/spec/services/chat/update_user_thread_last_read_spec.rb +++ b/plugins/chat/spec/services/chat/update_user_thread_last_read_spec.rb @@ -1,13 +1,13 @@ # frozen_string_literal: true RSpec.describe Chat::UpdateUserThreadLastRead do - describe Chat::UpdateUserThreadLastRead::Contract, type: :model do + describe described_class::Contract, type: :model do it { is_expected.to validate_presence_of :channel_id } it { is_expected.to validate_presence_of :thread_id } end describe ".call" do - subject(:result) { described_class.call(params) } + subject(:result) { described_class.call(params:, **dependencies) } fab!(:chatters) { Fabricate(:group) } fab!(:current_user) { Fabricate(:user, group_ids: [chatters.id]) } @@ -15,14 +15,8 @@ RSpec.describe Chat::UpdateUserThreadLastRead do fab!(:reply_1) { Fabricate(:chat_message, thread: thread, chat_channel_id: thread.channel.id) } let(:guardian) { Guardian.new(current_user) } - let(:params) do - { - message_id: reply_1.id, - guardian: guardian, - channel_id: thread.channel.id, - thread_id: thread.id, - } - end + let(:params) { { message_id: reply_1.id, channel_id: thread.channel.id, thread_id: thread.id } } + let(:dependencies) { { guardian: } } before do thread.add(current_user) @@ -117,7 +111,7 @@ RSpec.describe Chat::UpdateUserThreadLastRead do it "marks notifications as read" do params[:message_id] = reply_2.id - expect { described_class.call(params) }.to change { + expect { described_class.call(params:, **dependencies) }.to change { ::Notification .where(notification_type: Notification.types[:chat_mention]) .where(user: current_user) @@ -127,7 +121,7 @@ RSpec.describe Chat::UpdateUserThreadLastRead do params[:message_id] = reply_3.id - expect { described_class.call(params) }.to change { + expect { described_class.call(params:, **dependencies) }.to change { ::Notification .where(notification_type: Notification.types[:chat_mention]) .where(user: current_user) diff --git a/plugins/chat/spec/services/chat/upsert_draft_spec.rb b/plugins/chat/spec/services/chat/upsert_draft_spec.rb index 4dcc5004bd3..a2988c6f0ca 100644 --- a/plugins/chat/spec/services/chat/upsert_draft_spec.rb +++ b/plugins/chat/spec/services/chat/upsert_draft_spec.rb @@ -6,7 +6,7 @@ RSpec.describe Chat::UpsertDraft do end describe ".call" do - subject(:result) { described_class.call(params) } + subject(:result) { described_class.call(params:, **dependencies) } fab!(:current_user) { Fabricate(:user) } fab!(:channel_1) { Fabricate(:chat_channel) } @@ -16,9 +16,8 @@ RSpec.describe Chat::UpsertDraft do let(:data) { nil } let(:channel_id) { channel_1.id } let(:thread_id) { nil } - let(:params) do - { guardian: guardian, channel_id: channel_id, thread_id: thread_id, data: data } - end + let(:params) { { channel_id:, thread_id:, data: } } + let(:dependencies) { { guardian: } } before do SiteSetting.chat_enabled = true @@ -39,7 +38,7 @@ RSpec.describe Chat::UpsertDraft do it "updates draft if data provided and existing draft" do params[:data] = MultiJson.dump(message: "a") - described_class.call(**params) + described_class.call(params:, **dependencies) params[:data] = MultiJson.dump(message: "b") @@ -50,7 +49,7 @@ RSpec.describe Chat::UpsertDraft do it "destroys draft if empty data provided and existing draft" do params[:data] = MultiJson.dump(message: "a") - described_class.call(**params) + described_class.call(params:, **dependencies) params[:data] = "" @@ -60,7 +59,7 @@ RSpec.describe Chat::UpsertDraft do it "destroys draft if no data provided and existing draft" do params[:data] = MultiJson.dump(message: "a") - described_class.call(**params) + described_class.call(params:, **dependencies) params[:data] = nil diff --git a/plugins/chat/spec/system/invite_users_to_channel_spec.rb b/plugins/chat/spec/system/invite_users_to_channel_spec.rb index 299264ee3cb..9f2a54a044b 100644 --- a/plugins/chat/spec/system/invite_users_to_channel_spec.rb +++ b/plugins/chat/spec/system/invite_users_to_channel_spec.rb @@ -17,9 +17,11 @@ RSpec.describe "Invite users to channel", type: :system do context "when the invitation is linking to a channel" do before do Chat::InviteUsersToChannel.call( - channel_id: channel_1.id, - user_ids: [user_1.id], guardian: Guardian.new(Fabricate(:admin)), + params: { + channel_id: channel_1.id, + user_ids: [user_1.id], + }, ) end @@ -37,10 +39,12 @@ RSpec.describe "Invite users to channel", type: :system do before do Chat::InviteUsersToChannel.call( - channel_id: channel_1.id, - user_ids: [user_1.id], guardian: Guardian.new(Fabricate(:admin)), - message_id: message_1.id, + params: { + channel_id: channel_1.id, + user_ids: [user_1.id], + message_id: message_1.id, + }, ) end diff --git a/spec/lib/service/runner_spec.rb b/spec/lib/service/runner_spec.rb index 7128b82b11f..919e769668a 100644 --- a/spec/lib/service/runner_spec.rb +++ b/spec/lib/service/runner_spec.rb @@ -148,7 +148,7 @@ RSpec.describe Service::Runner do end describe ".call" do - subject(:runner) { described_class.call(service, &actions_block) } + subject(:runner) { described_class.call(service, dependencies, &actions_block) } let(:actions_block) { object.instance_eval(actions) } let(:service) { SuccessWithModelService } @@ -157,19 +157,13 @@ RSpec.describe Service::Runner do on_success { |fake_model:| [result, fake_model] } end BLOCK + let(:dependencies) { { guardian: stub, params: {} } } let(:object) do Class .new(ApplicationController) do def request OpenStruct.new end - - def params - ActionController::Parameters.new - end - - def guardian - end end .new end diff --git a/spec/lib/service/steps_inspector_spec.rb b/spec/lib/service/steps_inspector_spec.rb index ef6e197e383..fb420e1eab5 100644 --- a/spec/lib/service/steps_inspector_spec.rb +++ b/spec/lib/service/steps_inspector_spec.rb @@ -4,6 +4,11 @@ RSpec.describe Service::StepsInspector do class DummyService include Service::Base + options do + attribute :my_option, :boolean, default: true + attribute :my_other_option, :integer, default: 1 + end + model :model policy :policy contract do @@ -21,7 +26,7 @@ RSpec.describe Service::StepsInspector do subject(:inspector) { described_class.new(result) } let(:parameter) { "present" } - let(:result) { DummyService.call(parameter: parameter) } + let(:result) { DummyService.call(params: { parameter: parameter }) } before do class DummyService @@ -37,13 +42,14 @@ RSpec.describe Service::StepsInspector do context "when service runs without error" do it "outputs all the steps of the service" do expect(output).to eq <<~OUTPUT.chomp - [1/7] [model] 'model' ✅ - [2/7] [policy] 'policy' ✅ - [3/7] [contract] 'default' ✅ - [4/7] [transaction] - [5/7] [step] 'in_transaction_step_1' ✅ - [6/7] [step] 'in_transaction_step_2' ✅ - [7/7] [step] 'final_step' ✅ + [1/8] [options] 'default' ✅ + [2/8] [model] 'model' ✅ + [3/8] [policy] 'policy' ✅ + [4/8] [contract] 'default' ✅ + [5/8] [transaction] + [6/8] [step] 'in_transaction_step_1' ✅ + [7/8] [step] 'in_transaction_step_2' ✅ + [8/8] [step] 'final_step' ✅ OUTPUT end end @@ -59,13 +65,14 @@ RSpec.describe Service::StepsInspector do it "shows the failing step" do expect(output).to eq <<~OUTPUT.chomp - [1/7] [model] 'model' ❌ - [2/7] [policy] 'policy' - [3/7] [contract] 'default' - [4/7] [transaction] - [5/7] [step] 'in_transaction_step_1' - [6/7] [step] 'in_transaction_step_2' - [7/7] [step] 'final_step' + [1/8] [options] 'default' ✅ + [2/8] [model] 'model' ❌ + [3/8] [policy] 'policy' + [4/8] [contract] 'default' + [5/8] [transaction] + [6/8] [step] 'in_transaction_step_1' + [7/8] [step] 'in_transaction_step_2' + [8/8] [step] 'final_step' OUTPUT end end @@ -81,13 +88,14 @@ RSpec.describe Service::StepsInspector do it "shows the failing step" do expect(output).to eq <<~OUTPUT.chomp - [1/7] [model] 'model' ✅ - [2/7] [policy] 'policy' ❌ - [3/7] [contract] 'default' - [4/7] [transaction] - [5/7] [step] 'in_transaction_step_1' - [6/7] [step] 'in_transaction_step_2' - [7/7] [step] 'final_step' + [1/8] [options] 'default' ✅ + [2/8] [model] 'model' ✅ + [3/8] [policy] 'policy' ❌ + [4/8] [contract] 'default' + [5/8] [transaction] + [6/8] [step] 'in_transaction_step_1' + [7/8] [step] 'in_transaction_step_2' + [8/8] [step] 'final_step' OUTPUT end end @@ -97,13 +105,14 @@ RSpec.describe Service::StepsInspector do it "shows the failing step" do expect(output).to eq <<~OUTPUT.chomp - [1/7] [model] 'model' ✅ - [2/7] [policy] 'policy' ✅ - [3/7] [contract] 'default' ❌ - [4/7] [transaction] - [5/7] [step] 'in_transaction_step_1' - [6/7] [step] 'in_transaction_step_2' - [7/7] [step] 'final_step' + [1/8] [options] 'default' ✅ + [2/8] [model] 'model' ✅ + [3/8] [policy] 'policy' ✅ + [4/8] [contract] 'default' ❌ + [5/8] [transaction] + [6/8] [step] 'in_transaction_step_1' + [7/8] [step] 'in_transaction_step_2' + [8/8] [step] 'final_step' OUTPUT end end @@ -119,13 +128,14 @@ RSpec.describe Service::StepsInspector do it "shows the failing step" do expect(output).to eq <<~OUTPUT.chomp - [1/7] [model] 'model' ✅ - [2/7] [policy] 'policy' ✅ - [3/7] [contract] 'default' ✅ - [4/7] [transaction] - [5/7] [step] 'in_transaction_step_1' ✅ - [6/7] [step] 'in_transaction_step_2' ❌ - [7/7] [step] 'final_step' + [1/8] [options] 'default' ✅ + [2/8] [model] 'model' ✅ + [3/8] [policy] 'policy' ✅ + [4/8] [contract] 'default' ✅ + [5/8] [transaction] + [6/8] [step] 'in_transaction_step_1' ✅ + [7/8] [step] 'in_transaction_step_2' ❌ + [8/8] [step] 'final_step' OUTPUT end end @@ -136,13 +146,14 @@ RSpec.describe Service::StepsInspector do it "adapts its output accordingly" do expect(output).to eq <<~OUTPUT.chomp - [1/7] [model] 'model' ✅ - [2/7] [policy] 'policy' ✅ ⚠️ <= expected to return false but got true instead - [3/7] [contract] 'default' ✅ - [4/7] [transaction] - [5/7] [step] 'in_transaction_step_1' ✅ - [6/7] [step] 'in_transaction_step_2' ✅ - [7/7] [step] 'final_step' ✅ + [1/8] [options] 'default' ✅ + [2/8] [model] 'model' ✅ + [3/8] [policy] 'policy' ✅ ⚠️ <= expected to return false but got true instead + [4/8] [contract] 'default' ✅ + [5/8] [transaction] + [6/8] [step] 'in_transaction_step_1' ✅ + [7/8] [step] 'in_transaction_step_2' ✅ + [8/8] [step] 'final_step' ✅ OUTPUT end end @@ -159,13 +170,14 @@ RSpec.describe Service::StepsInspector do it "adapts its output accordingly" do expect(output).to eq <<~OUTPUT.chomp - [1/7] [model] 'model' ✅ - [2/7] [policy] 'policy' ❌ ⚠️ <= expected to return true but got false instead - [3/7] [contract] 'default' - [4/7] [transaction] - [5/7] [step] 'in_transaction_step_1' - [6/7] [step] 'in_transaction_step_2' - [7/7] [step] 'final_step' + [1/8] [options] 'default' ✅ + [2/8] [model] 'model' ✅ + [3/8] [policy] 'policy' ❌ ⚠️ <= expected to return true but got false instead + [4/8] [contract] 'default' + [5/8] [transaction] + [6/8] [step] 'in_transaction_step_1' + [7/8] [step] 'in_transaction_step_2' + [8/8] [step] 'final_step' OUTPUT end end diff --git a/spec/services/admin_notices/dismiss_spec.rb b/spec/services/admin_notices/dismiss_spec.rb index 821ed029ca2..c2606acb11d 100644 --- a/spec/services/admin_notices/dismiss_spec.rb +++ b/spec/services/admin_notices/dismiss_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true RSpec.describe(AdminNotices::Dismiss) do - subject(:result) { described_class.call(**params, **dependencies) } + subject(:result) { described_class.call(params:, **dependencies) } fab!(:current_user) { Fabricate(:admin) } fab!(:admin_notice) { Fabricate(:admin_notice, identifier: "problem.test") } diff --git a/spec/services/experiments/toggle_spec.rb b/spec/services/experiments/toggle_spec.rb index d7035383f2e..92e84c01c2a 100644 --- a/spec/services/experiments/toggle_spec.rb +++ b/spec/services/experiments/toggle_spec.rb @@ -1,56 +1,64 @@ # frozen_string_literal: true RSpec.describe Experiments::Toggle do - subject(:result) { described_class.call(params) } - describe described_class::Contract, type: :model do - subject(:contract) { described_class.new } - it { is_expected.to validate_presence_of :setting_name } end - fab!(:admin) - let(:params) { { setting_name:, guardian: } } - let(:setting_name) { :experimental_form_templates } - let(:guardian) { admin.guardian } + describe ".call" do + subject(:result) { described_class.call(params:, **dependencies) } - context "when setting_name is blank" do - let(:setting_name) { nil } + fab!(:admin) + let(:params) { { setting_name: } } + let(:setting_name) { :experimental_form_templates } + let(:dependencies) { { guardian: } } + let(:guardian) { admin.guardian } - it { is_expected.to fail_a_contract } - end + context "when data is invalid" do + let(:setting_name) { nil } - context "when setting_name is invalid" do - let(:setting_name) { "wrong_value" } - - it { is_expected.to fail_a_policy(:setting_is_available) } - end - - context "when a non-admin user tries to change a setting" do - let(:guardian) { Guardian.new } - - it { is_expected.to fail_a_policy(:current_user_is_admin) } - end - - context "when the admin toggles the feature" do - it { is_expected.to run_successfully } - - it "enables the specified setting" do - expect { result }.to change { SiteSetting.experimental_form_templates }.to(true) + it { is_expected.to fail_a_contract } end - it "disables the specified setting" do - SiteSetting.experimental_form_templates = true - expect { result }.to change { SiteSetting.experimental_form_templates }.to(false) + context "when setting_name is invalid" do + let(:setting_name) { "wrong_value" } + + it { is_expected.to fail_a_policy(:setting_is_available) } end - it "creates an entry in the staff action logs" do - expect { result }.to change { - UserHistory.where( - action: UserHistory.actions[:change_site_setting], - subject: "experimental_form_templates", - ).count - }.by(1) + context "when a non-admin user tries to change a setting" do + let(:guardian) { Guardian.new } + + it { is_expected.to fail_a_policy(:current_user_is_admin) } + end + + context "when everything's ok" do + it { is_expected.to run_successfully } + + context "when the setting is enabled" do + before { SiteSetting.experimental_form_templates = true } + + it "disables the specified setting" do + expect { result }.to change { SiteSetting.experimental_form_templates }.to(false) + end + end + + context "when the setting is disabled" do + before { SiteSetting.experimental_form_templates = false } + + it "enables the specified setting" do + expect { result }.to change { SiteSetting.experimental_form_templates }.to(true) + end + end + + it "creates an entry in the staff action logs" do + expect { result }.to change { + UserHistory.where( + action: UserHistory.actions[:change_site_setting], + subject: "experimental_form_templates", + ).count + }.by(1) + end end end end diff --git a/spec/services/flags/create_flag_spec.rb b/spec/services/flags/create_flag_spec.rb index 1da6cbbb212..7569ad3bbd8 100644 --- a/spec/services/flags/create_flag_spec.rb +++ b/spec/services/flags/create_flag_spec.rb @@ -10,7 +10,7 @@ RSpec.describe(Flags::CreateFlag) do end describe ".call" do - subject(:result) { described_class.call(**params, **dependencies) } + subject(:result) { described_class.call(params:, **dependencies) } fab!(:current_user) { Fabricate(:admin) } diff --git a/spec/services/flags/destroy_flag_spec.rb b/spec/services/flags/destroy_flag_spec.rb index 028b5fa0990..1fc39df27aa 100644 --- a/spec/services/flags/destroy_flag_spec.rb +++ b/spec/services/flags/destroy_flag_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true RSpec.describe(Flags::DestroyFlag) do - subject(:result) { described_class.call(**params, **dependencies) } + subject(:result) { described_class.call(params:, **dependencies) } fab!(:current_user) { Fabricate(:admin) } fab!(:flag) diff --git a/spec/services/flags/reorder_flag_spec.rb b/spec/services/flags/reorder_flag_spec.rb index e8352506ab4..25fd5b49ce1 100644 --- a/spec/services/flags/reorder_flag_spec.rb +++ b/spec/services/flags/reorder_flag_spec.rb @@ -7,7 +7,7 @@ RSpec.describe(Flags::ReorderFlag) do end describe ".call" do - subject(:result) { described_class.call(**params, **dependencies) } + subject(:result) { described_class.call(params:, **dependencies) } fab!(:current_user) { Fabricate(:admin) } @@ -44,7 +44,7 @@ RSpec.describe(Flags::ReorderFlag) do context "when everything's ok" do # DO NOT REMOVE: flags have side effects and their state will leak to # other examples otherwise. - after { described_class.call(**params.merge(direction: "down"), **dependencies) } + after { described_class.call(params: params.merge(direction: "down"), **dependencies) } it { is_expected.to run_successfully } diff --git a/spec/services/flags/toggle_flag_spec.rb b/spec/services/flags/toggle_flag_spec.rb index 3d21bd733b5..d22ca119a7a 100644 --- a/spec/services/flags/toggle_flag_spec.rb +++ b/spec/services/flags/toggle_flag_spec.rb @@ -6,7 +6,7 @@ RSpec.describe(Flags::ToggleFlag) do end describe ".call" do - subject(:result) { described_class.call(**params, **dependencies) } + subject(:result) { described_class.call(params:, **dependencies) } fab!(:current_user) { Fabricate(:admin) } fab!(:flag) diff --git a/spec/services/flags/update_flag_spec.rb b/spec/services/flags/update_flag_spec.rb index c7bc2452678..c9c6a19bca1 100644 --- a/spec/services/flags/update_flag_spec.rb +++ b/spec/services/flags/update_flag_spec.rb @@ -11,7 +11,7 @@ RSpec.describe(Flags::UpdateFlag) do end describe ".call" do - subject(:result) { described_class.call(**params, **dependencies) } + subject(:result) { described_class.call(params:, **dependencies) } fab!(:current_user) { Fabricate(:admin) } fab!(:flag) diff --git a/spec/services/site_setting/update_spec.rb b/spec/services/site_setting/update_spec.rb index 509373fd9b1..bf89f484366 100644 --- a/spec/services/site_setting/update_spec.rb +++ b/spec/services/site_setting/update_spec.rb @@ -6,7 +6,7 @@ RSpec.describe SiteSetting::Update do end describe ".call" do - subject(:result) { described_class.call(**params, **options, **dependencies) } + subject(:result) { described_class.call(params:, options:, **dependencies) } fab!(:admin) let(:params) { { setting_name:, new_value: } } diff --git a/spec/services/user/silence_spec.rb b/spec/services/user/silence_spec.rb index 102255650d0..26023224f84 100644 --- a/spec/services/user/silence_spec.rb +++ b/spec/services/user/silence_spec.rb @@ -19,7 +19,7 @@ RSpec.describe User::Silence do end describe ".call" do - subject(:result) { described_class.call(**params, **dependencies) } + subject(:result) { described_class.call(params:, **dependencies) } fab!(:admin) fab!(:user) diff --git a/spec/services/user/suspend_spec.rb b/spec/services/user/suspend_spec.rb index 8b823f7bf41..b99c78fd6d2 100644 --- a/spec/services/user/suspend_spec.rb +++ b/spec/services/user/suspend_spec.rb @@ -19,7 +19,7 @@ RSpec.describe User::Suspend do end describe ".call" do - subject(:result) { described_class.call(**params, **dependencies) } + subject(:result) { described_class.call(params:, **dependencies) } fab!(:admin) fab!(:user)