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)