From 584424594e1a8baeceb2e501ce47979224fa70a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Guitaut?= Date: Wed, 23 Oct 2024 17:57:48 +0200 Subject: [PATCH] DEV: Replace `params` by the contract object in services MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This patch replaces the parameters provided to a service through `params` by the contract object. That way, it allows better consistency when accessing input params. For example, if you have a service without a contract, to access a parameter, you need to use `params[:my_parameter]`. But with a contract, you do this through `contract.my_parameter`. Now, with this patch, you’ll be able to access it through `params.my_parameter` or `params[:my_parameter]`. Some methods have been added to the contract object to better mimic a Hash. That way, when accessing/using `params`, you don’t have to think too much about it: - `params.my_key` is also accessible through `params[:my_key]`. - `params.my_key = value` can also be done through `params[:my_key] = value`. - `#slice` and `#merge` are available. - `#to_hash` has been implemented, so the contract object will be automatically cast as a hash by Ruby depending on the context. For example, with an AR model, you can do this: `user.update(**params)`. --- .../admin/site_settings_controller.rb | 4 +- app/controllers/admin/users_controller.rb | 4 +- app/services/experiments/toggle.rb | 12 ++-- app/services/flags/create_flag.rb | 10 ++-- app/services/flags/reorder_flag.rb | 20 +++---- app/services/flags/toggle_flag.rb | 6 +- app/services/flags/update_flag.rb | 14 ++--- app/services/site_setting/update.rb | 16 ++--- app/services/user/action/silence_all.rb | 4 +- app/services/user/action/suspend_all.rb | 4 +- .../user/action/trigger_post_action.rb | 6 +- app/services/user/silence.rb | 22 +++---- app/services/user/suspend.rb | 22 +++---- lib/service.rb | 8 +-- lib/service/base.rb | 12 ++-- lib/service/contract_base.rb | 14 +++++ lib/service/steps_inspector.rb | 9 ++- .../regular/chat/auto_join_channel_batch.rb | 4 +- .../create_memberships_for_auto_join.rb | 6 +- .../app/services/chat/add_users_to_channel.rb | 12 ++-- .../services/chat/auto_join_channel_batch.rb | 12 ++-- .../auto_remove/handle_category_updated.rb | 6 +- .../handle_chat_allowed_groups_change.rb | 12 ++-- .../auto_remove/handle_destroyed_group.rb | 6 +- .../handle_user_removed_from_group.rb | 6 +- .../services/chat/create_category_channel.rb | 18 +++--- .../chat/create_direct_message_channel.rb | 18 +++--- .../chat/app/services/chat/create_message.rb | 59 +++++++++---------- .../chat/app/services/chat/create_thread.rb | 32 +++++----- .../chat/app/services/chat/flag_message.rb | 27 ++++----- .../services/chat/invite_users_to_channel.rb | 16 ++--- .../chat/app/services/chat/leave_channel.rb | 6 +- .../services/chat/list_channel_messages.rb | 26 ++++---- .../chat/list_channel_thread_messages.rb | 32 +++++----- .../services/chat/lookup_channel_threads.rb | 18 +++--- .../chat/app/services/chat/lookup_thread.rb | 6 +- .../app/services/chat/lookup_user_threads.rb | 12 ++-- .../chat/mark_thread_title_prompt_seen.rb | 8 +-- .../chat/app/services/chat/restore_message.rb | 6 +- .../chat/app/services/chat/search_chatable.rb | 54 ++++++++--------- .../services/chat/stop_message_streaming.rb | 8 +-- .../chat/app/services/chat/tracking_state.rb | 18 +++--- .../chat/app/services/chat/trash_message.rb | 10 ++-- .../chat/app/services/chat/trash_messages.rb | 12 ++-- .../app/services/chat/unfollow_channel.rb | 6 +- .../chat/app/services/chat/update_channel.rb | 6 +- .../services/chat/update_channel_status.rb | 12 ++-- .../chat/app/services/chat/update_message.rb | 31 +++++----- .../chat/app/services/chat/update_thread.rb | 10 ++-- .../update_thread_notification_settings.rb | 10 ++-- .../chat/update_user_channel_last_read.rb | 10 ++-- .../chat/update_user_thread_last_read.rb | 22 +++---- .../chat/app/services/chat/upsert_draft.rb | 31 +++++----- .../create_memberships_for_auto_join_spec.rb | 4 +- .../services/chat/search_chatable_spec.rb | 4 +- spec/lib/service/runner_spec.rb | 4 +- spec/lib/service/steps_inspector_spec.rb | 20 +++---- .../user/action/trigger_post_action_spec.rb | 4 +- spec/services/user/silence_spec.rb | 2 +- spec/services/user/suspend_spec.rb | 2 +- 60 files changed, 410 insertions(+), 405 deletions(-) diff --git a/app/controllers/admin/site_settings_controller.rb b/app/controllers/admin/site_settings_controller.rb index cde55031c65..db8e95cde78 100644 --- a/app/controllers/admin/site_settings_controller.rb +++ b/app/controllers/admin/site_settings_controller.rb @@ -40,9 +40,9 @@ class Admin::SiteSettingsController < Admin::AdminController previous_value = value_or_default(SiteSetting.get(id)) if update_existing_users SiteSetting::Update.call(params: { setting_name: id, new_value: value }, guardian:) do - on_success do |contract:| + on_success do |params:| if update_existing_users - SiteSettingUpdateExistingUsers.call(id, contract.new_value, previous_value) + SiteSettingUpdateExistingUsers.call(id, params.new_value, previous_value) end render body: nil end diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb index ca0223dd5b0..f291c36cee5 100644 --- a/app/controllers/admin/users_controller.rb +++ b/app/controllers/admin/users_controller.rb @@ -121,10 +121,10 @@ class Admin::UsersController < Admin::StaffController def suspend User::Suspend.call(service_params) do - on_success do |contract:, user:, full_reason:| + on_success do |params:, user:, full_reason:| render_json_dump( suspension: { - suspend_reason: contract.reason, + suspend_reason: params.reason, full_suspend_reason: full_reason, suspended_till: user.suspended_till, suspended_at: user.suspended_at, diff --git a/app/services/experiments/toggle.rb b/app/services/experiments/toggle.rb index f41dbfa747c..bd2e38a372d 100644 --- a/app/services/experiments/toggle.rb +++ b/app/services/experiments/toggle.rb @@ -4,7 +4,7 @@ class Experiments::Toggle include Service::Base policy :current_user_is_admin - contract do + params do attribute :setting_name, :string validates :setting_name, presence: true end @@ -17,14 +17,14 @@ class Experiments::Toggle guardian.is_admin? end - def setting_is_available(contract:) - SiteSetting.respond_to?(contract.setting_name) + def setting_is_available(params:) + SiteSetting.respond_to?(params[:setting_name]) end - def toggle(contract:, guardian:) + def toggle(params:, guardian:) SiteSetting.set_and_log( - contract.setting_name, - !SiteSetting.public_send(contract.setting_name), + params[:setting_name], + !SiteSetting.public_send(params[:setting_name]), guardian.user, ) end diff --git a/app/services/flags/create_flag.rb b/app/services/flags/create_flag.rb index b0d1b385dbc..f1f8075df23 100644 --- a/app/services/flags/create_flag.rb +++ b/app/services/flags/create_flag.rb @@ -4,7 +4,7 @@ class Flags::CreateFlag include Service::Base policy :invalid_access - contract do + params do attribute :name, :string attribute :description, :string attribute :require_message, :boolean @@ -31,12 +31,12 @@ class Flags::CreateFlag guardian.can_create_flag? end - def unique_name(contract:) - !Flag.custom.where(name: contract.name).exists? + def unique_name(params:) + !Flag.custom.where(name: params[:name]).exists? end - def instantiate_flag(contract:) - Flag.new(contract.attributes.merge(notify_type: true)) + def instantiate_flag(params:) + Flag.new(params.merge(notify_type: true)) end def create(flag:) diff --git a/app/services/flags/reorder_flag.rb b/app/services/flags/reorder_flag.rb index 5fc55dc17e0..df7384a44b5 100644 --- a/app/services/flags/reorder_flag.rb +++ b/app/services/flags/reorder_flag.rb @@ -3,7 +3,7 @@ class Flags::ReorderFlag include Service::Base - contract do + params do attribute :flag_id, :integer attribute :direction, :string @@ -21,8 +21,8 @@ class Flags::ReorderFlag private - def fetch_flag(contract:) - Flag.find_by(id: contract.flag_id) + def fetch_flag(params:) + Flag.find_by(id: params[:flag_id]) end def invalid_access(guardian:, flag:) @@ -33,25 +33,25 @@ class Flags::ReorderFlag Flag.where.not(name_key: "notify_user").order(:position).to_a end - def invalid_move(flag:, contract:, all_flags:) - return false if all_flags.first == flag && contract.direction == "up" - return false if all_flags.last == flag && contract.direction == "down" + def invalid_move(flag:, params:, all_flags:) + return false if all_flags.first == flag && params[:direction] == "up" + return false if all_flags.last == flag && params[:direction] == "down" true end - def move(flag:, contract:, all_flags:) + def move(flag:, params:, all_flags:) old_position = flag.position index = all_flags.index(flag) - target_flag = all_flags[contract.direction == "up" ? index - 1 : index + 1] + target_flag = all_flags[params[:direction] == "up" ? index - 1 : index + 1] flag.update!(position: target_flag.position) target_flag.update!(position: old_position) end - def log(guardian:, flag:, contract:) + def log(guardian:, flag:, params:) StaffActionLogger.new(guardian.user).log_custom( "move_flag", - { flag: flag.name, direction: contract.direction }, + { flag: flag.name, direction: params[:direction] }, ) end end diff --git a/app/services/flags/toggle_flag.rb b/app/services/flags/toggle_flag.rb index d8c61701f46..a8f8cb7211d 100644 --- a/app/services/flags/toggle_flag.rb +++ b/app/services/flags/toggle_flag.rb @@ -4,7 +4,7 @@ class Flags::ToggleFlag include Service::Base policy :invalid_access - contract do + params do attribute :flag_id, :integer validates :flag_id, presence: true @@ -21,8 +21,8 @@ class Flags::ToggleFlag guardian.can_toggle_flag? end - def fetch_flag(contract:) - Flag.find_by(id: contract.flag_id) + def fetch_flag(params:) + Flag.find_by(id: params[:flag_id]) end def toggle(flag:) diff --git a/app/services/flags/update_flag.rb b/app/services/flags/update_flag.rb index 1a52d3d8925..49d457ec4b4 100644 --- a/app/services/flags/update_flag.rb +++ b/app/services/flags/update_flag.rb @@ -3,7 +3,7 @@ class Flags::UpdateFlag include Service::Base - contract do + params do attribute :id, :integer attribute :name, :string attribute :description, :string @@ -31,8 +31,8 @@ class Flags::UpdateFlag private - def fetch_flag(contract:) - Flag.find_by(id: contract.id) + def fetch_flag(params:) + Flag.find_by(id: params[:id]) end def not_system(flag:) @@ -47,12 +47,12 @@ class Flags::UpdateFlag guardian.can_edit_flag?(flag) end - def unique_name(contract:) - !Flag.custom.where(name: contract.name).where.not(id: contract.id).exists? + def unique_name(params:) + !Flag.custom.where(name: params[:name]).where.not(id: params[:id]).exists? end - def update(flag:, contract:) - flag.update!(contract.attributes) + def update(flag:, params:) + flag.update!(**params) end def log(guardian:, flag:) diff --git a/app/services/site_setting/update.rb b/app/services/site_setting/update.rb index 1762fef875a..95511e2a48a 100644 --- a/app/services/site_setting/update.rb +++ b/app/services/site_setting/update.rb @@ -6,7 +6,7 @@ class SiteSetting::Update options { attribute :allow_changing_hidden, :boolean, default: false } policy :current_user_is_admin - contract do + params do attribute :setting_name attribute :new_value @@ -44,17 +44,17 @@ class SiteSetting::Update guardian.is_admin? end - def setting_is_visible(contract:, options:) - options.allow_changing_hidden || !SiteSetting.hidden_settings.include?(contract.setting_name) + def setting_is_visible(params:, options:) + options.allow_changing_hidden || !SiteSetting.hidden_settings.include?(params[:setting_name]) end - def setting_is_configurable(contract:) - return true if !SiteSetting.plugins[contract.setting_name] + def setting_is_configurable(params:) + return true if !SiteSetting.plugins[params[:setting_name]] - Discourse.plugins_by_name[SiteSetting.plugins[contract.setting_name]].configurable? + Discourse.plugins_by_name[SiteSetting.plugins[params[:setting_name]]].configurable? end - def save(contract:, guardian:) - SiteSetting.set_and_log(contract.setting_name, contract.new_value, guardian.user) + def save(params:, guardian:) + SiteSetting.set_and_log(params[:setting_name], params[:new_value], guardian.user) end end diff --git a/app/services/user/action/silence_all.rb b/app/services/user/action/silence_all.rb index aa5d76a8f0e..4136972a807 100644 --- a/app/services/user/action/silence_all.rb +++ b/app/services/user/action/silence_all.rb @@ -3,9 +3,9 @@ class User::Action::SilenceAll < Service::ActionBase option :users, [] option :actor - option :contract + option :params - delegate :message, :post_id, :silenced_till, :reason, to: :contract, private: true + delegate :message, :post_id, :silenced_till, :reason, to: :params, private: true def call silenced_users.first.try(:user_history).try(:details) diff --git a/app/services/user/action/suspend_all.rb b/app/services/user/action/suspend_all.rb index f9eaa40dc5c..791feb3cff7 100644 --- a/app/services/user/action/suspend_all.rb +++ b/app/services/user/action/suspend_all.rb @@ -3,9 +3,9 @@ class User::Action::SuspendAll < Service::ActionBase option :users, [] option :actor - option :contract + option :params - delegate :message, :post_id, :suspend_until, :reason, to: :contract, private: true + delegate :message, :post_id, :suspend_until, :reason, to: :params, private: true def call suspended_users.first.try(:user_history).try(:details) diff --git a/app/services/user/action/trigger_post_action.rb b/app/services/user/action/trigger_post_action.rb index a258397eebd..fd310849ae6 100644 --- a/app/services/user/action/trigger_post_action.rb +++ b/app/services/user/action/trigger_post_action.rb @@ -3,9 +3,9 @@ class User::Action::TriggerPostAction < Service::ActionBase option :guardian option :post - option :contract + option :params - delegate :post_action, to: :contract, private: true + delegate :post_action, to: :params, private: true delegate :user, to: :guardian, private: true def call @@ -30,7 +30,7 @@ class User::Action::TriggerPostAction < Service::ActionBase # Take what the moderator edited in as gospel PostRevisor.new(post).revise!( user, - { raw: contract.post_edit }, + { raw: params.post_edit }, skip_validations: true, skip_revision: true, ) diff --git a/app/services/user/silence.rb b/app/services/user/silence.rb index afc87131942..ecf1fc153d3 100644 --- a/app/services/user/silence.rb +++ b/app/services/user/silence.rb @@ -3,7 +3,7 @@ class User::Silence include Service::Base - contract do + params do attribute :user_id, :integer attribute :reason, :string attribute :message, :string @@ -29,27 +29,27 @@ class User::Silence private - def fetch_user(contract:) - User.find_by(id: contract.user_id) + def fetch_user(params:) + User.find_by(id: params[:user_id]) end - def fetch_users(user:, contract:) - [user, *User.where(id: contract.other_user_ids.to_a.uniq).to_a] + def fetch_users(user:, params:) + [user, *User.where(id: params[:other_user_ids].to_a.uniq).to_a] end def can_silence_all_users(guardian:, users:) users.all? { guardian.can_silence_user?(_1) } end - def silence(guardian:, users:, contract:) - context[:full_reason] = User::Action::SilenceAll.call(users:, actor: guardian.user, contract:) + def silence(guardian:, users:, params:) + context[:full_reason] = User::Action::SilenceAll.call(users:, actor: guardian.user, params:) end - def fetch_post(contract:) - Post.find_by(id: contract.post_id) + def fetch_post(params:) + Post.find_by(id: params[:post_id]) end - def perform_post_action(guardian:, post:, contract:) - User::Action::TriggerPostAction.call(guardian:, post:, contract:) + def perform_post_action(guardian:, post:, params:) + User::Action::TriggerPostAction.call(guardian:, post:, params:) end end diff --git a/app/services/user/suspend.rb b/app/services/user/suspend.rb index bd08a6dc2e9..63de8bde7bd 100644 --- a/app/services/user/suspend.rb +++ b/app/services/user/suspend.rb @@ -3,7 +3,7 @@ class User::Suspend include Service::Base - contract do + params do attribute :user_id, :integer attribute :reason, :string attribute :message, :string @@ -29,27 +29,27 @@ class User::Suspend private - def fetch_user(contract:) - User.find_by(id: contract.user_id) + def fetch_user(params:) + User.find_by(id: params[:user_id]) end - def fetch_users(user:, contract:) - [user, *User.where(id: contract.other_user_ids.to_a.uniq).to_a] + def fetch_users(user:, params:) + [user, *User.where(id: params[:other_user_ids].to_a.uniq).to_a] end def can_suspend_all_users(guardian:, users:) users.all? { guardian.can_suspend?(_1) } end - def suspend(guardian:, users:, contract:) - context[:full_reason] = User::Action::SuspendAll.call(users:, actor: guardian.user, contract:) + def suspend(guardian:, users:, params:) + context[:full_reason] = User::Action::SuspendAll.call(users:, actor: guardian.user, params:) end - def fetch_post(contract:) - Post.find_by(id: contract.post_id) + def fetch_post(params:) + Post.find_by(id: params[:post_id]) end - def perform_post_action(guardian:, post:, contract:) - User::Action::TriggerPostAction.call(guardian:, post:, contract:) + def perform_post_action(guardian:, post:, params:) + User::Action::TriggerPostAction.call(guardian:, post:, params:) end end diff --git a/lib/service.rb b/lib/service.rb index 7057fc0017d..52bcbc90d2b 100644 --- a/lib/service.rb +++ b/lib/service.rb @@ -10,13 +10,11 @@ module Service # # Currently, there are 5 types of steps: # - # * +contract(name = :default)+: used to validate the input parameters, + # * +params(name = :default)+: used to validate the input parameters, # typically provided by a user calling an endpoint. A block has to be # defined to hold the validations. If the validations fail, the step will # fail. Otherwise, the resulting contract will be available in - # +context[:contract]+. When calling +step(name)+ or +model(name = :model)+ - # methods after validating a contract, the contract should be used as an - # argument instead of context attributes. + # +context[:params]+. # * +model(name = :model)+: used to instantiate a model (either by building # it or fetching it from the DB). If a falsy value is returned, then the # step will fail. Otherwise the resulting object will be assigned in @@ -82,7 +80,7 @@ module Service # include Service::Base # # model :channel - # contract do + # params do # attribute :status # validates :status, inclusion: { in: Chat::Channel.editable_statuses.keys } # end diff --git a/lib/service/base.rb b/lib/service/base.rb index e25a23c4015..d8def9c1481 100644 --- a/lib/service/base.rb +++ b/lib/service/base.rb @@ -94,7 +94,7 @@ module Service steps << ModelStep.new(name, step_name, optional: optional) end - def contract(name = :default, default_values_from: nil, &block) + def params(name = :default, default_values_from: nil, &block) contract_class = Class.new(Service::ContractBase).tap { _1.class_eval(&block) } const_set("#{name.to_s.classify.sub("Default", "")}Contract", contract_class) steps << ContractStep.new( @@ -137,7 +137,7 @@ module Service object = class_name&.new(context) method = object&.method(:call) || instance.method(method_name) if method.parameters.any? { _1[0] != :keyreq } - raise "In #{type} '#{name}': default values in step implementations are not allowed. Maybe they could be defined in a contract?" + raise "In #{type} '#{name}': default values in step implementations are not allowed. Maybe they could be defined in a params or options block?" end args = context.slice(*method.parameters.select { _1[0] == :keyreq }.map(&:last)) context[result_key] = Context.build(object: object) @@ -214,7 +214,7 @@ module Service private def contract_name - return :contract if default? + return :params if default? :"#{name}_contract" end @@ -328,18 +328,18 @@ module Service # end # @!scope class - # @!method contract(name = :default, default_values_from: nil, &block) + # @!method params(name = :default, default_values_from: nil, &block) # @param name [Symbol] name for this contract # @param default_values_from [Symbol] name of the model to get default values from # @param block [Proc] a block containing validations # Checks the validity of the input parameters. # Implements ActiveModel::Validations and ActiveModel::Attributes. # - # It stores the resulting contract in +context[:contract]+ by default + # It stores the resulting contract in +context[:params]+ by default # (can be customized by providing the +name+ argument). # # @example - # contract do + # params do # attribute :name # validates :name, presence: true # end diff --git a/lib/service/contract_base.rb b/lib/service/contract_base.rb index 79a0413df2b..dfd40452d72 100644 --- a/lib/service/contract_base.rb +++ b/lib/service/contract_base.rb @@ -6,6 +6,20 @@ class Service::ContractBase include ActiveModel::AttributeMethods include ActiveModel::Validations::Callbacks + delegate :slice, :merge, to: :to_hash + + def [](key) + public_send(key) + end + + def []=(key, value) + public_send("#{key}=", value) + end + + def to_hash + attributes.symbolize_keys + end + def raw_attributes @attributes.values_before_type_cast end diff --git a/lib/service/steps_inspector.rb b/lib/service/steps_inspector.rb index 6c7b47aff81..c0c03527ca2 100644 --- a/lib/service/steps_inspector.rb +++ b/lib/service/steps_inspector.rb @@ -27,6 +27,7 @@ class Service::StepsInspector def type self.class.name.split("::").last.downcase end + alias inspect_type type def emoji "#{result_emoji}#{unexpected_result_emoji}" @@ -37,7 +38,7 @@ class Service::StepsInspector end def inspect - "#{" " * nesting_level}[#{type}] '#{name}' #{emoji}".rstrip + "#{" " * nesting_level}[#{inspect_type}] '#{name}' #{emoji}".rstrip end private @@ -75,6 +76,10 @@ class Service::StepsInspector def error "#{step_result.errors.inspect}\n\nProvided parameters: #{step_result.parameters.pretty_inspect}" end + + def inspect_type + "params" + end end # @!visibility private @@ -113,7 +118,7 @@ class Service::StepsInspector # Inspect the provided result object. # Example output: # [1/4] [model] 'channel' ✅ - # [2/4] [contract] 'default' ✅ + # [2/4] [params] 'default' ✅ # [3/4] [policy] 'check_channel_permission' ❌ # [4/4] [step] 'change_status' # @return [String] the steps of the result object with their state 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 2016a68916c..230b2a3f926 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 @@ -9,8 +9,8 @@ module Jobs on_failed_contract do |contract| Rails.logger.error(contract.errors.full_messages.join(", ")) end - on_model_not_found(:channel) do |contract:| - Rails.logger.error("Channel not found (id=#{contract.channel_id})") + on_model_not_found(:channel) do |params:| + Rails.logger.error("Channel not found (id=#{params.channel_id})") end end end diff --git a/plugins/chat/app/services/chat/action/create_memberships_for_auto_join.rb b/plugins/chat/app/services/chat/action/create_memberships_for_auto_join.rb index 63a878bb7ec..986a75859a1 100644 --- a/plugins/chat/app/services/chat/action/create_memberships_for_auto_join.rb +++ b/plugins/chat/app/services/chat/action/create_memberships_for_auto_join.rb @@ -4,7 +4,7 @@ module Chat module Action class CreateMembershipsForAutoJoin < Service::ActionBase option :channel - option :contract + option :params def call ::DB.query_single(<<~SQL, query_args) @@ -42,8 +42,8 @@ module Chat def query_args { chat_channel_id: channel.id, - start: contract.start_user_id, - end: contract.end_user_id, + start: params.start_user_id, + end: params.end_user_id, suspended_until: Time.zone.now, last_seen_at: 3.months.ago, channel_category: channel.category.id, 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 2c8ed8164aa..109b83e0520 100644 --- a/plugins/chat/app/services/chat/add_users_to_channel.rb +++ b/plugins/chat/app/services/chat/add_users_to_channel.rb @@ -24,7 +24,7 @@ module Chat # @option params [Array] :usernames # @option params [Array] :groups # @return [Service::Base::Context] - contract do + params do attribute :usernames, :array attribute :groups, :array attribute :channel_id, :integer @@ -49,8 +49,8 @@ module Chat private - def fetch_channel(contract:) - ::Chat::Channel.includes(:chatable).find_by(id: contract.channel_id) + def fetch_channel(params:) + ::Chat::Channel.includes(:chatable).find_by(id: params[:channel_id]) end def can_add_users_to_channel(guardian:, channel:) @@ -58,10 +58,10 @@ module Chat channel.direct_message_channel? && channel.chatable.group end - def fetch_target_users(contract:, channel:) + def fetch_target_users(params:, channel:) ::Chat::UsersFromUsernamesAndGroupsQuery.call( - usernames: contract.usernames, - groups: contract.groups, + usernames: params[:usernames], + groups: params[:groups], excluded_user_ids: channel.chatable.direct_message_users.pluck(:user_id), ) 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 2ec508d2607..102d3f245f6 100644 --- a/plugins/chat/app/services/chat/auto_join_channel_batch.rb +++ b/plugins/chat/app/services/chat/auto_join_channel_batch.rb @@ -16,7 +16,7 @@ module Chat class AutoJoinChannelBatch include Service::Base - contract do + params do # Backward-compatible attributes attribute :chat_channel_id, :integer attribute :starts_at, :integer @@ -44,14 +44,14 @@ module Chat private - def fetch_channel(contract:) - ::Chat::CategoryChannel.find_by(id: contract.channel_id, auto_join_users: true) + def fetch_channel(params:) + ::Chat::CategoryChannel.find_by(id: params[:channel_id], auto_join_users: true) end - def create_memberships(channel:, contract:) + def create_memberships(channel:, params:) context[:added_user_ids] = ::Chat::Action::CreateMembershipsForAutoJoin.call( - channel: channel, - contract: contract, + channel:, + params:, ) end diff --git a/plugins/chat/app/services/chat/auto_remove/handle_category_updated.rb b/plugins/chat/app/services/chat/auto_remove/handle_category_updated.rb index 909c6855704..2a60cc9344c 100644 --- a/plugins/chat/app/services/chat/auto_remove/handle_category_updated.rb +++ b/plugins/chat/app/services/chat/auto_remove/handle_category_updated.rb @@ -15,7 +15,7 @@ module Chat include Service::Base policy :chat_enabled - contract do + params do attribute :category_id, :integer validates :category_id, presence: true @@ -37,8 +37,8 @@ module Chat SiteSetting.chat_enabled end - def fetch_category(contract:) - Category.find_by(id: contract.category_id) + def fetch_category(params:) + Category.find_by(id: params[:category_id]) end def fetch_category_channel_ids(category:) diff --git a/plugins/chat/app/services/chat/auto_remove/handle_chat_allowed_groups_change.rb b/plugins/chat/app/services/chat/auto_remove/handle_chat_allowed_groups_change.rb index fd212f63c61..cacb00c2227 100644 --- a/plugins/chat/app/services/chat/auto_remove/handle_chat_allowed_groups_change.rb +++ b/plugins/chat/app/services/chat/auto_remove/handle_chat_allowed_groups_change.rb @@ -19,7 +19,7 @@ module Chat include Service::Base policy :chat_enabled - contract { attribute :new_allowed_groups, :array } + params { attribute :new_allowed_groups, :array } policy :not_everyone_allowed model :users model :memberships_to_remove @@ -32,11 +32,11 @@ module Chat SiteSetting.chat_enabled end - def not_everyone_allowed(contract:) - contract.new_allowed_groups.exclude?(Group::AUTO_GROUPS[:everyone]) + def not_everyone_allowed(params:) + params[:new_allowed_groups].exclude?(Group::AUTO_GROUPS[:everyone]) end - def fetch_users(contract:) + def fetch_users(params:) User .real .activated @@ -46,8 +46,8 @@ module Chat .joins(:user_chat_channel_memberships) .distinct .then do |users| - break users if contract.new_allowed_groups.blank? - users.where(<<~SQL, contract.new_allowed_groups) + break users if params[:new_allowed_groups].blank? + users.where(<<~SQL, params[:new_allowed_groups]) users.id NOT IN ( SELECT DISTINCT group_users.user_id FROM group_users diff --git a/plugins/chat/app/services/chat/auto_remove/handle_destroyed_group.rb b/plugins/chat/app/services/chat/auto_remove/handle_destroyed_group.rb index 18e7d6a6dd0..16303071d1a 100644 --- a/plugins/chat/app/services/chat/auto_remove/handle_destroyed_group.rb +++ b/plugins/chat/app/services/chat/auto_remove/handle_destroyed_group.rb @@ -22,7 +22,7 @@ module Chat include Service::Base policy :chat_enabled - contract do + params do attribute :destroyed_group_user_ids, :array validates :destroyed_group_user_ids, presence: true @@ -48,7 +48,7 @@ module Chat !SiteSetting.chat_allowed_groups_map.include?(Group::AUTO_GROUPS[:everyone]) end - def fetch_scoped_users(contract:) + def fetch_scoped_users(params:) User .real .activated @@ -56,7 +56,7 @@ module Chat .not_staged .includes(:group_users) .where("NOT admin AND NOT moderator") - .where(id: contract.destroyed_group_user_ids) + .where(id: params[:destroyed_group_user_ids]) .joins(:user_chat_channel_memberships) .distinct end diff --git a/plugins/chat/app/services/chat/auto_remove/handle_user_removed_from_group.rb b/plugins/chat/app/services/chat/auto_remove/handle_user_removed_from_group.rb index 5d497d5d850..f9ebb7227d8 100644 --- a/plugins/chat/app/services/chat/auto_remove/handle_user_removed_from_group.rb +++ b/plugins/chat/app/services/chat/auto_remove/handle_user_removed_from_group.rb @@ -21,7 +21,7 @@ module Chat include Service::Base policy :chat_enabled - contract do + params do attribute :user_id, :integer validates :user_id, presence: true @@ -48,8 +48,8 @@ module Chat !SiteSetting.chat_allowed_groups_map.include?(Group::AUTO_GROUPS[:everyone]) end - def fetch_user(contract:) - User.find_by(id: contract.user_id) + def fetch_user(params:) + User.find_by(id: params[:user_id]) end def user_not_staff(user:) diff --git a/plugins/chat/app/services/chat/create_category_channel.rb b/plugins/chat/app/services/chat/create_category_channel.rb index 642399a042e..bb9aaf018f5 100644 --- a/plugins/chat/app/services/chat/create_category_channel.rb +++ b/plugins/chat/app/services/chat/create_category_channel.rb @@ -31,7 +31,7 @@ module Chat policy :public_channels_enabled policy :can_create_channel - contract do + params do attribute :name, :string attribute :description, :string attribute :slug, :string @@ -65,22 +65,18 @@ module Chat guardian.can_create_chat_channel? end - def fetch_category(contract:) - Category.find_by(id: contract.category_id) + def fetch_category(params:) + Category.find_by(id: params[:category_id]) end - def category_channel_does_not_exist(category:, contract:) - !Chat::Channel.exists?(chatable: category, name: contract.name) + def category_channel_does_not_exist(category:, params:) + !Chat::Channel.exists?(chatable: category, name: params[:name]) end - def create_channel(category:, contract:) + def create_channel(category:, params:) category.create_chat_channel( - name: contract.name, - slug: contract.slug, - description: contract.description, user_count: 1, - auto_join_users: contract.auto_join_users, - threading_enabled: contract.threading_enabled, + **params.slice(:name, :slug, :description, :auto_join_users, :threading_enabled), ) end 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 359d2c56780..0e8aa1f6686 100644 --- a/plugins/chat/app/services/chat/create_direct_message_channel.rb +++ b/plugins/chat/app/services/chat/create_direct_message_channel.rb @@ -25,7 +25,7 @@ module Chat # @option params [Boolean] :upsert # @return [Service::Base::Context] - contract do + params do attribute :name, :string attribute :target_usernames, :array attribute :target_groups, :array @@ -62,10 +62,10 @@ module Chat ) end - def fetch_target_users(guardian:, contract:) + def fetch_target_users(guardian:, params:) ::Chat::UsersFromUsernamesAndGroupsQuery.call( - usernames: [*contract.target_usernames, guardian.user.username], - groups: contract.target_groups, + usernames: [*params[:target_usernames], guardian.user.username], + groups: params[:target_groups], ) end @@ -77,11 +77,11 @@ module Chat !user_comm_screener.actor_disallowing_all_pms? end - def fetch_or_create_direct_message(target_users:, contract:) + def fetch_or_create_direct_message(target_users:, params:) ids = target_users.map(&:id) - is_group = ids.size > 2 || contract.name.present? + is_group = ids.size > 2 || params[:name].present? - if contract.upsert || !is_group + if params[:upsert] || !is_group ::Chat::DirectMessage.for_user_ids(ids, group: is_group) || ::Chat::DirectMessage.create(user_ids: ids, group: is_group) else @@ -93,8 +93,8 @@ module Chat ::Chat::DirectMessageChannel.find_or_create_by(chatable: direct_message) end - def set_optional_name(channel:, contract:) - channel.update!(name: contract.name) if contract.name&.length&.positive? + def set_optional_name(channel:, params:) + channel.update!(params.slice(:name)) if params[:name]&.size&.positive? end def update_memberships(channel:, target_users:) diff --git a/plugins/chat/app/services/chat/create_message.rb b/plugins/chat/app/services/chat/create_message.rb index ab7ddc49df8..4d70ffda7fd 100644 --- a/plugins/chat/app/services/chat/create_message.rb +++ b/plugins/chat/app/services/chat/create_message.rb @@ -34,7 +34,7 @@ module Chat end policy :no_silenced_user - contract do + params do attribute :chat_channel_id, :string attribute :in_reply_to_id, :string attribute :context_topic_id, :integer @@ -80,8 +80,8 @@ module Chat !guardian.is_silenced? end - def fetch_channel(contract:) - Chat::Channel.find_by_id_or_slug(contract.chat_channel_id) + def fetch_channel(params:) + Chat::Channel.find_by_id_or_slug(params[:chat_channel_id]) end def enforce_membership(guardian:, channel:, options:) @@ -98,17 +98,17 @@ module Chat channel.membership_for(guardian.user) end - def fetch_reply(contract:) - Chat::Message.find_by(id: contract.in_reply_to_id) + def fetch_reply(params:) + Chat::Message.find_by(id: params[:in_reply_to_id]) end - def ensure_reply_consistency(channel:, contract:, reply:) - return true if contract.in_reply_to_id.blank? + def ensure_reply_consistency(channel:, params:, reply:) + return true if params[:in_reply_to_id].blank? reply&.chat_channel == channel end - def fetch_thread(contract:, reply:, channel:, options:) - return Chat::Thread.find_by(id: contract.thread_id) if contract.thread_id.present? + def fetch_thread(params:, reply:, channel:, options:) + return Chat::Thread.find_by(id: params[:thread_id]) if params[:thread_id].present? return unless reply reply.thread || reply.build_thread( @@ -119,8 +119,8 @@ module Chat ) end - def ensure_valid_thread_for_channel(thread:, contract:, channel:) - return true if contract.thread_id.blank? + def ensure_valid_thread_for_channel(thread:, params:, channel:) + return true if params[:thread_id].blank? thread&.channel == channel end @@ -129,29 +129,28 @@ module Chat reply.thread == thread end - def fetch_uploads(contract:, guardian:) + def fetch_uploads(params:, guardian:) return [] if !SiteSetting.chat_allow_uploads - guardian.user.uploads.where(id: contract.upload_ids) + guardian.user.uploads.where(id: params[:upload_ids]) end - def clean_message(contract:, options:) - contract.message = - TextCleaner.clean( - contract.message, - strip_whitespaces: options.strip_whitespaces, - strip_zero_width_spaces: true, - ) + def clean_message(params:, options:) + params[:message] = TextCleaner.clean( + params[:message], + strip_whitespaces: options.strip_whitespaces, + strip_zero_width_spaces: true, + ) end - def instantiate_message(channel:, guardian:, contract:, uploads:, thread:, reply:, options:) + def instantiate_message(channel:, guardian:, params:, uploads:, thread:, reply:, options:) channel.chat_messages.new( user: guardian.user, last_editor: guardian.user, in_reply_to: reply, - message: contract.message, + message: params[:message], uploads: uploads, thread: thread, - cooked: ::Chat::Message.cook(contract.message, user_id: guardian.user.id), + cooked: ::Chat::Message.cook(params[:message], user_id: guardian.user.id), cooked_version: ::Chat::Message::BAKED_VERSION, streaming: options.streaming, ) @@ -199,14 +198,14 @@ module Chat Chat::Action::PublishAndFollowDirectMessageChannel.call(channel_membership: membership) end - def publish_new_thread(reply:, contract:, channel:, thread:) + def publish_new_thread(reply:, channel:, thread:) return unless channel.threading_enabled? || thread&.force return unless reply&.thread_id_previously_changed?(from: nil) Chat::Publisher.publish_thread_created!(channel, reply, thread.id) end - def process(channel:, message_instance:, contract:, thread:, options:) - ::Chat::Publisher.publish_new!(channel, message_instance, contract.staged_id) + def process(channel:, message_instance:, params:, thread:, options:) + ::Chat::Publisher.publish_new!(channel, message_instance, params[:staged_id]) DiscourseEvent.trigger( :chat_message_created, @@ -217,20 +216,20 @@ module Chat thread: thread, thread_replies_count: thread&.replies_count_cache || 0, context: { - post_ids: contract.context_post_ids, - topic_id: contract.context_topic_id, + post_ids: params[:context_post_ids], + topic_id: params[:context_topic_id], }, }, ) if options.process_inline Jobs::Chat::ProcessMessage.new.execute( - { chat_message_id: message_instance.id, staged_id: contract.staged_id }, + { chat_message_id: message_instance.id, staged_id: params[:staged_id] }, ) else Jobs.enqueue( Jobs::Chat::ProcessMessage, - { chat_message_id: message_instance.id, staged_id: contract.staged_id }, + { chat_message_id: message_instance.id, staged_id: params[:staged_id] }, ) end end diff --git a/plugins/chat/app/services/chat/create_thread.rb b/plugins/chat/app/services/chat/create_thread.rb index 84ad808a32a..bc3f930aeb4 100644 --- a/plugins/chat/app/services/chat/create_thread.rb +++ b/plugins/chat/app/services/chat/create_thread.rb @@ -17,7 +17,7 @@ module Chat # @option params [String,nil] :title # @return [Service::Base::Context] - contract do + params do attribute :original_message_id, :integer attribute :channel_id, :integer attribute :title, :string @@ -39,14 +39,14 @@ module Chat private - def fetch_channel(contract:) - ::Chat::Channel.find_by(id: contract.channel_id) + def fetch_channel(params:) + ::Chat::Channel.find_by(id: params[:channel_id]) end - def fetch_original_message(channel:, contract:) + def fetch_original_message(channel:, params:) ::Chat::Message.find_by( - id: contract.original_message_id, - chat_channel_id: contract.channel_id, + id: params[:original_message_id], + chat_channel_id: params[:channel_id], ) end @@ -58,33 +58,33 @@ module Chat channel.threading_enabled end - def find_or_create_thread(channel:, original_message:, contract:) + def find_or_create_thread(channel:, original_message:, params:) if original_message.thread_id.present? return context[:thread] = ::Chat::Thread.find_by(id: original_message.thread_id) end context[:thread] = channel.threads.create( - title: contract.title, + title: params[:title], original_message: original_message, original_message_user: original_message.user, ) fail!(context.thread.errors.full_messages.join(", ")) if context.thread.invalid? end - def associate_thread_to_message(original_message:) - original_message.update(thread: context.thread) + def associate_thread_to_message(original_message:, thread:) + original_message.update(thread:) end - def fetch_membership(guardian:) - context[:membership] = context.thread.membership_for(guardian.user) + def fetch_membership(guardian:, thread:) + context[:membership] = thread.membership_for(guardian.user) end - def publish_new_thread(channel:, original_message:) - ::Chat::Publisher.publish_thread_created!(channel, original_message, context.thread.id) + def publish_new_thread(channel:, original_message:, thread:) + ::Chat::Publisher.publish_thread_created!(channel, original_message, thread.id) end - def trigger_chat_thread_created_event - ::DiscourseEvent.trigger(:chat_thread_created, context.thread) + def trigger_chat_thread_created_event(thread:) + ::DiscourseEvent.trigger(:chat_thread_created, thread) end end end diff --git a/plugins/chat/app/services/chat/flag_message.rb b/plugins/chat/app/services/chat/flag_message.rb index dbc9a1e649f..567dd864e20 100644 --- a/plugins/chat/app/services/chat/flag_message.rb +++ b/plugins/chat/app/services/chat/flag_message.rb @@ -27,7 +27,7 @@ module Chat # @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 + params do attribute :message_id, :integer attribute :channel_id, :integer attribute :flag_type_id, :integer @@ -46,36 +46,29 @@ module Chat private - def fetch_message(contract:) + def fetch_message(params:) Chat::Message.includes(:chat_channel, :revisions).find_by( - id: contract.message_id, - chat_channel_id: contract.channel_id, + id: params[:message_id], + chat_channel_id: params[:channel_id], ) end - def can_flag_message_in_channel(guardian:, contract:, message:) + def can_flag_message_in_channel(guardian:, params:, message:) guardian.can_join_chat_channel?(message.chat_channel) && guardian.can_flag_chat_message?(message) && guardian.can_flag_message_as?( message, - contract.flag_type_id, - { - queue_for_review: contract.queue_for_review, - take_action: contract.take_action, - is_warning: contract.is_warning, - }, + params[:flag_type_id], + params.slice(:queue_for_review, :take_action, :is_warning), ) end - def flag_message(message:, contract:, guardian:) + def flag_message(message:, params:, guardian:) Chat::ReviewQueue.new.flag_message( message, guardian, - contract.flag_type_id, - message: contract.message, - is_warning: contract.is_warning, - take_action: contract.take_action, - queue_for_review: contract.queue_for_review, + params[:flag_type_id], + **params.slice(:message, :is_warning, :take_action, :queue_for_review), ) end end 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 307421a7444..9effedd3c8f 100644 --- a/plugins/chat/app/services/chat/invite_users_to_channel.rb +++ b/plugins/chat/app/services/chat/invite_users_to_channel.rb @@ -17,7 +17,7 @@ module Chat # @option params [Integer, nil] :message_id # @return [Service::Base::Context] - contract do + params do attribute :user_ids, :array attribute :channel_id, :integer attribute :message_id, :integer @@ -32,24 +32,24 @@ module Chat private - def fetch_channel(contract:) - ::Chat::Channel.find_by(id: contract.channel_id) + def fetch_channel(params:) + ::Chat::Channel.find_by(id: params[:channel_id]) end def can_view_channel(guardian:, channel:) guardian.can_preview_chat_channel?(channel) end - def fetch_users(contract:) + def fetch_users(params:) ::User .joins(:user_option) .where(user_options: { chat_enabled: true }) .not_suspended - .where(id: contract.user_ids) + .where(id: params[:user_ids]) .limit(50) end - def send_invite_notifications(channel:, guardian:, users:, contract:) + def send_invite_notifications(channel:, guardian:, users:, params:) users&.each do |invited_user| next if !invited_user.guardian.can_join_chat_channel?(channel) @@ -59,8 +59,8 @@ module Chat chat_channel_title: channel.title(invited_user), chat_channel_slug: channel.slug, invited_by_username: guardian.user.username, - } - data[:chat_message_id] = contract.message_id if contract.message_id + chat_message_id: params[:message_id], + }.compact invited_user.notifications.create( notification_type: ::Notification.types[:chat_invitation], diff --git a/plugins/chat/app/services/chat/leave_channel.rb b/plugins/chat/app/services/chat/leave_channel.rb index fac02a54cdf..e68ac4ddf52 100644 --- a/plugins/chat/app/services/chat/leave_channel.rb +++ b/plugins/chat/app/services/chat/leave_channel.rb @@ -20,7 +20,7 @@ module Chat # @option params [Integer] :channel_id ID of the channel # @return [Service::Base::Context] - contract do + params do attribute :channel_id, :integer validates :channel_id, presence: true @@ -31,8 +31,8 @@ module Chat private - def fetch_channel(contract:) - Chat::Channel.find_by(id: contract.channel_id) + def fetch_channel(params:) + Chat::Channel.find_by(id: params[:channel_id]) end def leave(channel:, guardian:) diff --git a/plugins/chat/app/services/chat/list_channel_messages.rb b/plugins/chat/app/services/chat/list_channel_messages.rb index 872cf453ee0..0f39a26ecf7 100644 --- a/plugins/chat/app/services/chat/list_channel_messages.rb +++ b/plugins/chat/app/services/chat/list_channel_messages.rb @@ -16,7 +16,7 @@ module Chat # @option params [Integer] :channel_id # @return [Service::Base::Context] - contract do + params do attribute :channel_id, :integer attribute :page_size, :integer @@ -56,8 +56,8 @@ module Chat private - def fetch_channel(contract:) - ::Chat::Channel.includes(:chatable).find_by(id: contract.channel_id) + def fetch_channel(params:) + ::Chat::Channel.includes(:chatable).find_by(id: params[:channel_id]) end def fetch_membership(channel:, guardian:) @@ -72,11 +72,11 @@ module Chat guardian.can_preview_chat_channel?(channel) end - def determine_target_message_id(contract:, membership:) - if contract.fetch_from_last_read + def determine_target_message_id(params:, membership:) + if params[:fetch_from_last_read] context[:target_message_id] = membership&.last_read_message_id else - context[:target_message_id] = contract.target_message_id + context[:target_message_id] = params[:target_message_id] end end @@ -96,16 +96,16 @@ module Chat true end - def fetch_messages(channel:, contract:, guardian:, enabled_threads:) + def fetch_messages(channel:, params:, guardian:, enabled_threads:, target_message_id:) messages_data = ::Chat::MessagesQuery.call( - channel: channel, - guardian: guardian, - target_message_id: context.target_message_id, + channel:, + guardian:, + target_message_id:, include_thread_messages: !enabled_threads, - page_size: contract.page_size || Chat::MessagesQuery::MAX_PAGE_SIZE, - direction: contract.direction, - target_date: contract.target_date, + page_size: params[:page_size] || Chat::MessagesQuery::MAX_PAGE_SIZE, + direction: params[:direction], + target_date: params[:target_date], ) context[:can_load_more_past] = messages_data[:can_load_more_past] 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 5c467e44633..b9a31e2b603 100644 --- a/plugins/chat/app/services/chat/list_channel_thread_messages.rb +++ b/plugins/chat/app/services/chat/list_channel_thread_messages.rb @@ -16,7 +16,7 @@ module Chat # @option params [Integer] :thread_id # @return [Service::Base::Context] - contract do + params do attribute :thread_id, :integer # If this is not present, then we just fetch messages with page_size # and direction. @@ -50,8 +50,8 @@ module Chat private - def fetch_thread(contract:) - ::Chat::Thread.strict_loading.includes(channel: :chatable).find_by(id: contract.thread_id) + def fetch_thread(params:) + ::Chat::Thread.strict_loading.includes(channel: :chatable).find_by(id: params[:thread_id]) end def can_view_thread(guardian:, thread:) @@ -62,42 +62,42 @@ module Chat thread.membership_for(guardian.user) end - def determine_target_message_id(contract:, membership:, guardian:, thread:) - if contract.fetch_from_last_message + def determine_target_message_id(params:, membership:, guardian:, thread:) + if params[:fetch_from_last_message] context[:target_message_id] = thread.last_message_id - elsif contract.fetch_from_first_message + elsif params[:fetch_from_first_message] context[:target_message_id] = thread.original_message_id - elsif contract.fetch_from_last_read || !contract.target_message_id + elsif params[:fetch_from_last_read] || !params[:target_message_id] context[:target_message_id] = membership&.last_read_message_id - elsif contract.target_message_id - context[:target_message_id] = contract.target_message_id + elsif params[:target_message_id] + context[:target_message_id] = params[:target_message_id] end end - def target_message_exists(contract:, guardian:) + def target_message_exists(params:, guardian:) return true if context.target_message_id.blank? target_message = ::Chat::Message.with_deleted.find_by( id: context.target_message_id, - thread_id: contract.thread_id, + thread_id: params[:thread_id], ) return false if target_message.blank? return true if !target_message.trashed? target_message.user_id == guardian.user.id || guardian.is_staff? end - def fetch_messages(thread:, guardian:, contract:) + def fetch_messages(thread:, guardian:, params:) messages_data = ::Chat::MessagesQuery.call( channel: thread.channel, guardian: guardian, target_message_id: context.target_message_id, thread_id: thread.id, - page_size: contract.page_size || Chat::MessagesQuery::MAX_PAGE_SIZE, - direction: contract.direction, - target_date: contract.target_date, + page_size: params[:page_size] || Chat::MessagesQuery::MAX_PAGE_SIZE, + direction: params[:direction], + target_date: params[:target_date], include_target_message_id: - contract.fetch_from_first_message || contract.fetch_from_last_message, + params[:fetch_from_first_message] || params[:fetch_from_last_message], ) context[:can_load_more_past] = messages_data[:can_load_more_past] diff --git a/plugins/chat/app/services/chat/lookup_channel_threads.rb b/plugins/chat/app/services/chat/lookup_channel_threads.rb index 0917a8e7049..bc1a3894826 100644 --- a/plugins/chat/app/services/chat/lookup_channel_threads.rb +++ b/plugins/chat/app/services/chat/lookup_channel_threads.rb @@ -25,7 +25,7 @@ module Chat # @option params [Integer] :offset # @return [Service::Base::Context] - contract do + params do attribute :channel_id, :integer attribute :limit, :integer attribute :offset, :integer @@ -51,16 +51,16 @@ module Chat private - def set_limit(contract:) - context[:limit] = (contract.limit || THREADS_LIMIT).to_i.clamp(1, THREADS_LIMIT) + def set_limit(params:) + context[:limit] = (params[:limit] || THREADS_LIMIT).to_i.clamp(1, THREADS_LIMIT) end - def set_offset(contract:) - context[:offset] = [contract.offset || 0, 0].max + def set_offset(params:) + context[:offset] = [params[:offset] || 0, 0].max end - def fetch_channel(contract:) - ::Chat::Channel.strict_loading.includes(:chatable).find_by(id: contract.channel_id) + def fetch_channel(params:) + ::Chat::Channel.strict_loading.includes(:chatable).find_by(id: params[:channel_id]) end def threading_enabled_for_channel(channel:) @@ -137,10 +137,10 @@ module Chat context[:participants] = ::Chat::ThreadParticipantQuery.call(thread_ids: threads.map(&:id)) end - def build_load_more_url(contract:) + def build_load_more_url(channel:) load_more_params = { offset: context.offset + context.limit }.to_query context[:load_more_url] = ::URI::HTTP.build( - path: "/chat/api/channels/#{contract.channel_id}/threads", + path: "/chat/api/channels/#{channel.id}/threads", query: load_more_params, ).request_uri end diff --git a/plugins/chat/app/services/chat/lookup_thread.rb b/plugins/chat/app/services/chat/lookup_thread.rb index 30f35a56ace..52411333bdb 100644 --- a/plugins/chat/app/services/chat/lookup_thread.rb +++ b/plugins/chat/app/services/chat/lookup_thread.rb @@ -17,7 +17,7 @@ module Chat # @option params [Integer] :channel_id # @return [Service::Base::Context] - contract do + params do attribute :thread_id, :integer attribute :channel_id, :integer @@ -31,12 +31,12 @@ module Chat private - def fetch_thread(contract:) + def fetch_thread(params:) Chat::Thread.includes( :channel, original_message_user: :user_status, original_message: :chat_webhook_event, - ).find_by(id: contract.thread_id, channel_id: contract.channel_id) + ).find_by(id: params[:thread_id], channel_id: params[:channel_id]) end def invalid_access(guardian:, thread:) diff --git a/plugins/chat/app/services/chat/lookup_user_threads.rb b/plugins/chat/app/services/chat/lookup_user_threads.rb index 139c6ec26b9..c2c21e386fd 100644 --- a/plugins/chat/app/services/chat/lookup_user_threads.rb +++ b/plugins/chat/app/services/chat/lookup_user_threads.rb @@ -21,7 +21,7 @@ module Chat # @option params [Integer] :offset # @return [Service::Base::Context] - contract do + params do attribute :limit, :integer attribute :offset, :integer end @@ -35,12 +35,12 @@ module Chat private - def set_limit(contract:) - context[:limit] = (contract.limit || THREADS_LIMIT).to_i.clamp(1, THREADS_LIMIT) + def set_limit(params:) + context[:limit] = (params[:limit] || THREADS_LIMIT).to_i.clamp(1, THREADS_LIMIT) end - def set_offset(contract:) - context[:offset] = [contract.offset || 0, 0].max + def set_offset(params:) + context[:offset] = [params[:offset] || 0, 0].max end def fetch_threads(guardian:) @@ -131,7 +131,7 @@ module Chat context[:participants] = ::Chat::ThreadParticipantQuery.call(thread_ids: threads.map(&:id)) end - def build_load_more_url(contract:) + def build_load_more_url load_more_params = { limit: context.limit, offset: context.offset + context.limit }.to_query context[:load_more_url] = ::URI::HTTP.build( 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 7672288f098..90f75069aa5 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 @@ -24,7 +24,7 @@ module Chat # @option params [Integer] :channel_id # @return [Service::Base::Context] - contract do + params do attribute :thread_id, :integer attribute :channel_id, :integer @@ -37,8 +37,8 @@ module Chat private - def fetch_thread(contract:) - Chat::Thread.find_by(id: contract.thread_id, channel_id: contract.channel_id) + def fetch_thread(params:) + Chat::Thread.find_by(id: params[:thread_id], channel_id: params[:channel_id]) end def can_view_channel(guardian:, thread:) @@ -49,7 +49,7 @@ module Chat thread.channel.threading_enabled end - def create_or_update_membership(thread:, guardian:, contract:) + def create_or_update_membership(thread:, guardian:) membership = thread.membership_for(guardian.user) membership = thread.add( diff --git a/plugins/chat/app/services/chat/restore_message.rb b/plugins/chat/app/services/chat/restore_message.rb index 1d6f094ea45..d20735082e3 100644 --- a/plugins/chat/app/services/chat/restore_message.rb +++ b/plugins/chat/app/services/chat/restore_message.rb @@ -18,7 +18,7 @@ module Chat # @option params [Integer] :channel_id # @return [Service::Base::Context] - contract do + params do attribute :message_id, :integer attribute :channel_id, :integer @@ -36,11 +36,11 @@ module Chat private - def fetch_message(contract:) + def fetch_message(params:) Chat::Message .with_deleted .includes(chat_channel: :chatable) - .find_by(id: contract.message_id, chat_channel_id: contract.channel_id) + .find_by(id: params[:message_id], chat_channel_id: params[:channel_id]) end def invalid_access(guardian:, message:) diff --git a/plugins/chat/app/services/chat/search_chatable.rb b/plugins/chat/app/services/chat/search_chatable.rb index 95680e654b7..aab3a0dd996 100644 --- a/plugins/chat/app/services/chat/search_chatable.rb +++ b/plugins/chat/app/services/chat/search_chatable.rb @@ -17,7 +17,7 @@ module Chat # @option params [String] :term # @return [Service::Base::Context] - contract do + params do attribute :term, :string, default: "" attribute :include_users, :boolean, default: true attribute :include_groups, :boolean, default: true @@ -34,42 +34,42 @@ module Chat private - def clean_term(contract:) - contract.term = contract.term&.downcase&.strip&.gsub(/^[@#]+/, "") + def clean_term(params:) + params[:term] = params[:term]&.downcase&.strip&.gsub(/^[@#]+/, "") end def fetch_memberships(guardian:) ::Chat::ChannelMembershipManager.all_for_user(guardian.user) end - def fetch_users(guardian:, contract:) - return unless contract.include_users + def fetch_users(guardian:, params:) + return unless params[:include_users] return unless guardian.can_create_direct_message? - search_users(contract, guardian) + search_users(params, guardian) end - def fetch_groups(guardian:, contract:) - return unless contract.include_groups + def fetch_groups(guardian:, params:) + return unless params[:include_groups] return unless guardian.can_create_direct_message? - search_groups(contract, guardian) + search_groups(params, guardian) end - def fetch_category_channels(guardian:, contract:) - return unless contract.include_category_channels + def fetch_category_channels(guardian:, params:) + return unless params[:include_category_channels] return unless SiteSetting.enable_public_channels - search_category_channels(contract, guardian) + search_category_channels(params, guardian) end - def fetch_direct_message_channels(guardian:, contract:, users:) - return unless contract.include_direct_message_channels + def fetch_direct_message_channels(guardian:, params:, users:) + return unless params[:include_direct_message_channels] return unless guardian.can_create_direct_message? - search_direct_message_channels(guardian, contract, users) + search_direct_message_channels(guardian, params, users) end - def search_users(contract, guardian) - user_search = ::UserSearch.new(contract.term, limit: SEARCH_RESULT_LIMIT) + def search_users(params, guardian) + user_search = ::UserSearch.new(params[:term], limit: SEARCH_RESULT_LIMIT) - if contract.term.blank? + if params[:term].blank? user_search = user_search.scoped_users else user_search = user_search.search @@ -81,51 +81,51 @@ module Chat user_search = user_search.real(allowed_bot_user_ids: allowed_bot_user_ids) user_search = user_search.includes(:user_option) - if contract.excluded_memberships_channel_id + if params[:excluded_memberships_channel_id] user_search = user_search.where( "NOT EXISTS (SELECT 1 FROM user_chat_channel_memberships WHERE user_id = users.id AND chat_channel_id = ?)", - contract.excluded_memberships_channel_id, + params[:excluded_memberships_channel_id], ) end user_search end - def search_groups(contract, guardian) + def search_groups(params, guardian) Group .visible_groups(guardian.user) .includes(users: :user_option) .where( "groups.name ILIKE :term_like OR groups.full_name ILIKE :term_like", - term_like: "%#{contract.term}%", + term_like: "%#{params[:term]}%", ) .limit(SEARCH_RESULT_LIMIT) end - def search_category_channels(contract, guardian) + def search_category_channels(params, guardian) ::Chat::ChannelFetcher.secured_public_channel_search( guardian, status: :open, - filter: contract.term, + filter: params[:term], filter_on_category_name: false, match_filter_on_starts_with: false, limit: SEARCH_RESULT_LIMIT, ) end - def search_direct_message_channels(guardian, contract, users) + def search_direct_message_channels(guardian, params, users) channels = ::Chat::ChannelFetcher.secured_direct_message_channels_search( guardian.user.id, guardian, - filter: contract.term, + filter: params[:term], match_filter_on_starts_with: false, limit: SEARCH_RESULT_LIMIT, ) || [] # skip 1:1s when search returns users - if contract.include_users && users.present? + if params[:include_users] && users.present? channels.reject! do |channel| other_user_ids = channel.allowed_user_ids - [guardian.user.id] other_user_ids.size <= 1 diff --git a/plugins/chat/app/services/chat/stop_message_streaming.rb b/plugins/chat/app/services/chat/stop_message_streaming.rb index 46758278775..cdad91954fb 100644 --- a/plugins/chat/app/services/chat/stop_message_streaming.rb +++ b/plugins/chat/app/services/chat/stop_message_streaming.rb @@ -14,7 +14,7 @@ module Chat # @param [Hash] params # @option params [Integer] :message_id # @return [Service::Base::Context] - contract do + params do attribute :message_id, :integer validates :message_id, presence: true @@ -28,8 +28,8 @@ module Chat private - def fetch_message(contract:) - ::Chat::Message.find_by(id: contract.message_id) + def fetch_message(params:) + ::Chat::Message.find_by(id: params[:message_id]) end def enforce_membership(guardian:, message:) @@ -49,7 +49,7 @@ module Chat message.update!(streaming: false) end - def publish_message_streaming_state(guardian:, message:, contract:) + def publish_message_streaming_state(guardian:, message:) ::Chat::Publisher.publish_edit!(message.chat_channel, message) end end diff --git a/plugins/chat/app/services/chat/tracking_state.rb b/plugins/chat/app/services/chat/tracking_state.rb index 7c590493bc7..b4e0b6375bc 100644 --- a/plugins/chat/app/services/chat/tracking_state.rb +++ b/plugins/chat/app/services/chat/tracking_state.rb @@ -34,7 +34,7 @@ module Chat # @option params [Integer] :channel_ids # @return [Service::Base::Context] - contract do + params do attribute :channel_ids, :array, default: [] attribute :thread_ids, :array, default: [] attribute :include_missing_memberships, default: false @@ -45,14 +45,16 @@ module Chat private - def fetch_report(contract:, guardian:) + def fetch_report(params:, guardian:) ::Chat::TrackingStateReportQuery.call( - guardian: guardian, - channel_ids: contract.channel_ids, - thread_ids: contract.thread_ids, - include_missing_memberships: contract.include_missing_memberships, - include_threads: contract.include_threads, - include_read: contract.include_read, + guardian:, + **params.slice( + :channel_ids, + :thread_ids, + :include_missing_memberships, + :include_threads, + :include_read, + ), ) end end diff --git a/plugins/chat/app/services/chat/trash_message.rb b/plugins/chat/app/services/chat/trash_message.rb index 9322fea9fff..b157413f50b 100644 --- a/plugins/chat/app/services/chat/trash_message.rb +++ b/plugins/chat/app/services/chat/trash_message.rb @@ -18,7 +18,7 @@ module Chat # @option params [Integer] :channel_id # @return [Service::Base::Context] - contract do + params do attribute :message_id, :integer attribute :channel_id, :integer @@ -38,10 +38,10 @@ module Chat private - def fetch_message(contract:) + def fetch_message(params:) Chat::Message.includes(chat_channel: :chatable).find_by( - id: contract.message_id, - chat_channel_id: contract.channel_id, + id: params[:message_id], + chat_channel_id: params[:channel_id], ) end @@ -79,7 +79,7 @@ module Chat message.chat_channel.update_last_message_id! end - def publish_events(contract:, guardian:, message:) + def publish_events(guardian:, message:) DiscourseEvent.trigger(:chat_message_trashed, message, message.chat_channel, guardian.user) Chat::Publisher.publish_delete!(message.chat_channel, message) diff --git a/plugins/chat/app/services/chat/trash_messages.rb b/plugins/chat/app/services/chat/trash_messages.rb index a6fb6dd4935..7abf4479b7c 100644 --- a/plugins/chat/app/services/chat/trash_messages.rb +++ b/plugins/chat/app/services/chat/trash_messages.rb @@ -18,7 +18,7 @@ module Chat # @option params [Integer] :channel_id # @return [Service::Base::Context] - contract do + params do attribute :channel_id, :integer attribute :message_ids, :array @@ -38,10 +38,10 @@ module Chat private - def fetch_messages(contract:) + def fetch_messages(params:) Chat::Message.includes(chat_channel: :chatable).where( - id: contract.message_ids, - chat_channel_id: contract.channel_id, + id: params[:message_ids], + chat_channel_id: params[:channel_id], ) end @@ -86,11 +86,11 @@ module Chat messages.each { |message| message.thread&.decrement_replies_count_cache } end - def publish_events(contract:, guardian:, messages:) + def publish_events(params:, guardian:, messages:) messages.each do |message| DiscourseEvent.trigger(:chat_message_trashed, message, message.chat_channel, guardian.user) end - Chat::Publisher.publish_bulk_delete!(messages.first.chat_channel, contract.message_ids) + Chat::Publisher.publish_bulk_delete!(messages.first.chat_channel, params[:message_ids]) end end end diff --git a/plugins/chat/app/services/chat/unfollow_channel.rb b/plugins/chat/app/services/chat/unfollow_channel.rb index b23d81915b4..c23cd44536d 100644 --- a/plugins/chat/app/services/chat/unfollow_channel.rb +++ b/plugins/chat/app/services/chat/unfollow_channel.rb @@ -20,7 +20,7 @@ module Chat # @option params [Integer] :channel_id ID of the channel # @return [Service::Base::Context] - contract do + params do attribute :channel_id, :integer validates :channel_id, presence: true @@ -30,8 +30,8 @@ module Chat private - def fetch_channel(contract:) - Chat::Channel.find_by(id: contract.channel_id) + def fetch_channel(params:) + Chat::Channel.find_by(id: params[:channel_id]) end def unfollow(channel:, guardian:) diff --git a/plugins/chat/app/services/chat/update_channel.rb b/plugins/chat/app/services/chat/update_channel.rb index ed39c2892c7..3539555f656 100644 --- a/plugins/chat/app/services/chat/update_channel.rb +++ b/plugins/chat/app/services/chat/update_channel.rb @@ -36,7 +36,7 @@ module Chat model :channel policy :check_channel_permission - contract(default_values_from: :channel) do + params(default_values_from: :channel) do attribute :name, :string attribute :description, :string attribute :slug, :string @@ -66,8 +66,8 @@ module Chat guardian.can_preview_chat_channel?(channel) && guardian.can_edit_chat_channel?(channel) end - def update_channel(channel:, contract:) - channel.update!(contract.attributes) + def update_channel(channel:, params:) + channel.update!(**params) end def mark_all_threads_as_read_if_needed(channel:) diff --git a/plugins/chat/app/services/chat/update_channel_status.rb b/plugins/chat/app/services/chat/update_channel_status.rb index 845b21cac8a..309b4649f4c 100644 --- a/plugins/chat/app/services/chat/update_channel_status.rb +++ b/plugins/chat/app/services/chat/update_channel_status.rb @@ -16,8 +16,8 @@ module Chat # @option params [String] :status # @return [Service::Base::Context] - model :channel, :fetch_channel - contract do + model :channel + params do attribute :status, :string validates :status, inclusion: { in: Chat::Channel.editable_statuses.keys } @@ -31,13 +31,13 @@ module Chat Chat::Channel.find_by(id: params[:channel_id]) end - def check_channel_permission(guardian:, channel:, contract:) + def check_channel_permission(guardian:, channel:, params:) guardian.can_preview_chat_channel?(channel) && - guardian.can_change_channel_status?(channel, contract.status.to_sym) + guardian.can_change_channel_status?(channel, params[:status].to_sym) end - def change_status(channel:, contract:, guardian:) - channel.public_send("#{contract.status}!", guardian.user) + def change_status(channel:, params:, guardian:) + channel.public_send("#{params[:status]}!", guardian.user) end end end diff --git a/plugins/chat/app/services/chat/update_message.rb b/plugins/chat/app/services/chat/update_message.rb index 275bc0efcef..0a9168df1f6 100644 --- a/plugins/chat/app/services/chat/update_message.rb +++ b/plugins/chat/app/services/chat/update_message.rb @@ -26,7 +26,7 @@ module Chat attribute :process_inline, :boolean, default: -> { Rails.env.test? } end - contract do + params do attribute :message_id, :string attribute :message, :string attribute :upload_ids, :array @@ -55,7 +55,7 @@ module Chat message.chat_channel.add(guardian.user) if guardian.user.bot? end - def fetch_message(contract:) + def fetch_message(params:) ::Chat::Message.includes( :chat_mentions, :bookmarks, @@ -70,16 +70,16 @@ module Chat chatable: [:topic_only_relative_url, direct_message_users: [user: :user_option]], ], user: :user_status, - ).find_by(id: contract.message_id) + ).find_by(id: params[:message_id]) end def fetch_membership(guardian:, message:) message.chat_channel.membership_for(guardian.user) end - def fetch_uploads(contract:, guardian:) + def fetch_uploads(params:, guardian:) return if !SiteSetting.chat_allow_uploads - guardian.user.uploads.where(id: contract.upload_ids) + guardian.user.uploads.where(id: params[:upload_ids]) end def can_modify_channel_message(guardian:, message:) @@ -90,21 +90,20 @@ module Chat guardian.can_edit_chat?(message) end - def clean_message(contract:, options:) - contract.message = - TextCleaner.clean( - contract.message, - strip_zero_width_spaces: true, - strip_whitespaces: options.strip_whitespaces, - ) + def clean_message(params:, options:) + params[:message] = TextCleaner.clean( + params[:message], + strip_zero_width_spaces: true, + strip_whitespaces: options.strip_whitespaces, + ) end - def modify_message(contract:, message:, guardian:, uploads:) - message.message = contract.message + def modify_message(params:, message:, guardian:, uploads:) + message.message = params[:message] message.last_editor_id = guardian.user.id message.cook - return if uploads&.size != contract.upload_ids.to_a.size + return if uploads&.size != params[:upload_ids].to_a.size new_upload_ids = uploads.map(&:id) existing_upload_ids = message.upload_ids @@ -157,7 +156,7 @@ module Chat chars_edited > max_edited_chars end - def publish(message:, guardian:, contract:, options:) + def publish(message:, guardian:, options:) edit_timestamp = context[:revision]&.created_at&.iso8601(6) || Time.zone.now.iso8601(6) ::Chat::Publisher.publish_edit!(message.chat_channel, message) diff --git a/plugins/chat/app/services/chat/update_thread.rb b/plugins/chat/app/services/chat/update_thread.rb index e3fcdcb6f12..c42c9ee0c92 100644 --- a/plugins/chat/app/services/chat/update_thread.rb +++ b/plugins/chat/app/services/chat/update_thread.rb @@ -19,7 +19,7 @@ module Chat # @option params [Integer] :channel_id # @return [Service::Base::Context] - contract do + params do attribute :thread_id, :integer attribute :title, :string @@ -35,8 +35,8 @@ module Chat private - def fetch_thread(contract:) - Chat::Thread.find_by(id: contract.thread_id) + def fetch_thread(params:) + Chat::Thread.find_by(id: params[:thread_id]) end def can_view_channel(guardian:, thread:) @@ -51,8 +51,8 @@ module Chat thread.channel.threading_enabled end - def update(thread:, contract:) - thread.update(title: contract.title) + def update(thread:, params:) + thread.update(params.slice(:title)) fail!(thread.errors.full_messages.join(", ")) if thread.invalid? end 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 f991bc21b14..57c3caaf4bd 100644 --- a/plugins/chat/app/services/chat/update_thread_notification_settings.rb +++ b/plugins/chat/app/services/chat/update_thread_notification_settings.rb @@ -26,7 +26,7 @@ module Chat # @option params [Integer] :notification_level # @return [Service::Base::Context] - contract do + params do attribute :thread_id, :integer attribute :channel_id, :integer attribute :notification_level, :integer @@ -44,8 +44,8 @@ module Chat private - def fetch_thread(contract:) - Chat::Thread.find_by(id: contract.thread_id, channel_id: contract.channel_id) + def fetch_thread(params:) + Chat::Thread.find_by(id: params[:thread_id], channel_id: params[:channel_id]) end def can_view_channel(guardian:, thread:) @@ -56,13 +56,13 @@ module Chat thread.channel.threading_enabled end - def create_or_update_membership(thread:, guardian:, contract:) + def create_or_update_membership(thread:, guardian:, params:) membership = thread.membership_for(guardian.user) if !membership membership = thread.add(guardian.user) membership.update!(last_read_message_id: thread.last_message_id) end - membership.update!(notification_level: contract.notification_level) + membership.update!(notification_level: params[:notification_level]) context[:membership] = membership end end 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 f39d7af14a7..580f2679fc0 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 @@ -16,7 +16,7 @@ module Chat # @option params [Integer] :message_id # @return [Service::Base::Context] - contract do + params do attribute :message_id, :integer attribute :channel_id, :integer @@ -35,8 +35,8 @@ module Chat private - def fetch_channel(contract:) - ::Chat::Channel.find_by(id: contract.channel_id) + def fetch_channel(params:) + ::Chat::Channel.find_by(id: params[:channel_id]) end def fetch_membership(guardian:, channel:) @@ -47,8 +47,8 @@ module Chat guardian.can_join_chat_channel?(membership.chat_channel) end - def fetch_message(channel:, contract:) - ::Chat::Message.with_deleted.find_by(chat_channel_id: channel.id, id: contract.message_id) + def fetch_message(channel:, params:) + ::Chat::Message.with_deleted.find_by(chat_channel_id: channel.id, id: params[:message_id]) end def ensure_message_id_recency(message:, membership:) 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 1afda59c70a..8e10dd0b44c 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 @@ -18,7 +18,7 @@ module Chat # @option params [Integer] :message_id # @return [Service::Base::Context] - contract do + params do attribute :channel_id, :integer attribute :thread_id, :integer attribute :message_id, :integer @@ -36,24 +36,24 @@ module Chat private - def fetch_thread(contract:) - ::Chat::Thread.find_by(id: contract.thread_id, channel_id: contract.channel_id) + def fetch_thread(params:) + ::Chat::Thread.find_by(id: params[:thread_id], channel_id: params[:channel_id]) end - def fetch_message(contract:, thread:) - ::Chat::Message.with_deleted.find_by( - id: contract.message_id || thread.last_message_id, - thread_id: contract.thread_id, - chat_channel_id: contract.channel_id, - ) + def invalid_access(guardian:, thread:) + guardian.can_join_chat_channel?(thread.channel) end def fetch_membership(guardian:, thread:) thread.membership_for(guardian.user) end - def invalid_access(guardian:, thread:) - guardian.can_join_chat_channel?(thread.channel) + def fetch_message(params:, thread:) + ::Chat::Message.with_deleted.find_by( + id: params[:message_id] || thread.last_message_id, + thread: thread, + chat_channel: thread.channel, + ) end def ensure_valid_message(message:, membership:) diff --git a/plugins/chat/app/services/chat/upsert_draft.rb b/plugins/chat/app/services/chat/upsert_draft.rb index ba2e0517397..c47313d1fa3 100644 --- a/plugins/chat/app/services/chat/upsert_draft.rb +++ b/plugins/chat/app/services/chat/upsert_draft.rb @@ -24,12 +24,12 @@ module Chat # @option params [Integer] :thread_id ID of the thread # @return [Service::Base::Context] - contract do + params do attribute :channel_id, :integer - validates :channel_id, presence: true - attribute :thread_id, :integer attribute :data, :string + + validates :channel_id, presence: true end model :channel policy :can_upsert_draft @@ -38,36 +38,35 @@ module Chat private - def fetch_channel(contract:) - Chat::Channel.find_by(id: contract.channel_id) + def fetch_channel(params:) + Chat::Channel.find_by(id: params[:channel_id]) end def can_upsert_draft(guardian:, channel:) guardian.can_chat? && guardian.can_join_chat_channel?(channel) end - def check_thread_exists(contract:, channel:) - if contract.thread_id.present? - fail!("Thread not found") if !channel.threads.exists?(id: contract.thread_id) - end + def check_thread_exists(params:, channel:) + return if params[:thread_id].blank? + fail!("Thread not found") if !channel.threads.exists?(id: params[:thread_id]) end - def upsert_draft(contract:, guardian:) - if contract.data.present? + def upsert_draft(params:, guardian:) + if params[:data].present? draft = Chat::Draft.find_or_initialize_by( user_id: guardian.user.id, - chat_channel_id: contract.channel_id, - thread_id: contract.thread_id, + chat_channel_id: params[:channel_id], + thread_id: params[:thread_id], ) - draft.data = contract.data + draft.data = params[:data] draft.save! else # when data is empty, we destroy the draft Chat::Draft.where( user: guardian.user, - chat_channel_id: contract.channel_id, - thread_id: contract.thread_id, + chat_channel_id: params[:channel_id], + thread_id: params[:thread_id], ).destroy_all end end diff --git a/plugins/chat/spec/services/actions/create_memberships_for_auto_join_spec.rb b/plugins/chat/spec/services/actions/create_memberships_for_auto_join_spec.rb index dbc15d92b7c..83674a3b3d4 100644 --- a/plugins/chat/spec/services/actions/create_memberships_for_auto_join_spec.rb +++ b/plugins/chat/spec/services/actions/create_memberships_for_auto_join_spec.rb @@ -1,14 +1,14 @@ # frozen_string_literal: true RSpec.describe Chat::Action::CreateMembershipsForAutoJoin do - subject(:action) { described_class.call(channel: channel, contract: contract) } + subject(:action) { described_class.call(channel:, params:) } fab!(:channel) { Fabricate(:chat_channel, auto_join_users: true) } fab!(:user_1) { Fabricate(:user, last_seen_at: 15.minutes.ago) } let(:start_user_id) { user_1.id } let(:end_user_id) { user_1.id } - let(:contract) { OpenStruct.new(start_user_id: start_user_id, end_user_id: end_user_id) } + let(:params) { OpenStruct.new(start_user_id: start_user_id, end_user_id: end_user_id) } it "adds correct members" do expect(action).to eq([user_1.id]) diff --git a/plugins/chat/spec/services/chat/search_chatable_spec.rb b/plugins/chat/spec/services/chat/search_chatable_spec.rb index e2c153e28ca..87f16bdd1ce 100644 --- a/plugins/chat/spec/services/chat/search_chatable_spec.rb +++ b/plugins/chat/spec/services/chat/search_chatable_spec.rb @@ -47,10 +47,10 @@ RSpec.describe Chat::SearchChatable do it "cleans the term" do params[:term] = "#bob" - expect(result.contract.term).to eq("bob") + expect(result.params[:term]).to eq("bob") params[:term] = "@bob" - expect(result.contract.term).to eq("bob") + expect(result.params[:term]).to eq("bob") end it "fetches user memberships" do diff --git a/spec/lib/service/runner_spec.rb b/spec/lib/service/runner_spec.rb index 919e769668a..c5bd871937d 100644 --- a/spec/lib/service/runner_spec.rb +++ b/spec/lib/service/runner_spec.rb @@ -38,7 +38,7 @@ RSpec.describe Service::Runner do class FailedContractService include Service::Base - contract do + params do attribute :test validates :test, presence: true @@ -48,7 +48,7 @@ RSpec.describe Service::Runner do class SuccessContractService include Service::Base - contract {} + params {} end class FailureWithModelService diff --git a/spec/lib/service/steps_inspector_spec.rb b/spec/lib/service/steps_inspector_spec.rb index fb420e1eab5..5f845db45ef 100644 --- a/spec/lib/service/steps_inspector_spec.rb +++ b/spec/lib/service/steps_inspector_spec.rb @@ -11,7 +11,7 @@ RSpec.describe Service::StepsInspector do model :model policy :policy - contract do + params do attribute :parameter validates :parameter, presence: true @@ -45,7 +45,7 @@ RSpec.describe Service::StepsInspector do [1/8] [options] 'default' ✅ [2/8] [model] 'model' ✅ [3/8] [policy] 'policy' ✅ - [4/8] [contract] 'default' ✅ + [4/8] [params] 'default' ✅ [5/8] [transaction] [6/8] [step] 'in_transaction_step_1' ✅ [7/8] [step] 'in_transaction_step_2' ✅ @@ -68,7 +68,7 @@ RSpec.describe Service::StepsInspector do [1/8] [options] 'default' ✅ [2/8] [model] 'model' ❌ [3/8] [policy] 'policy' - [4/8] [contract] 'default' + [4/8] [params] 'default' [5/8] [transaction] [6/8] [step] 'in_transaction_step_1' [7/8] [step] 'in_transaction_step_2' @@ -91,7 +91,7 @@ RSpec.describe Service::StepsInspector do [1/8] [options] 'default' ✅ [2/8] [model] 'model' ✅ [3/8] [policy] 'policy' ❌ - [4/8] [contract] 'default' + [4/8] [params] 'default' [5/8] [transaction] [6/8] [step] 'in_transaction_step_1' [7/8] [step] 'in_transaction_step_2' @@ -100,7 +100,7 @@ RSpec.describe Service::StepsInspector do end end - context "when the contract step is failing" do + context "when the params step is failing" do let(:parameter) { nil } it "shows the failing step" do @@ -108,7 +108,7 @@ RSpec.describe Service::StepsInspector do [1/8] [options] 'default' ✅ [2/8] [model] 'model' ✅ [3/8] [policy] 'policy' ✅ - [4/8] [contract] 'default' ❌ + [4/8] [params] 'default' ❌ [5/8] [transaction] [6/8] [step] 'in_transaction_step_1' [7/8] [step] 'in_transaction_step_2' @@ -131,7 +131,7 @@ RSpec.describe Service::StepsInspector do [1/8] [options] 'default' ✅ [2/8] [model] 'model' ✅ [3/8] [policy] 'policy' ✅ - [4/8] [contract] 'default' ✅ + [4/8] [params] 'default' ✅ [5/8] [transaction] [6/8] [step] 'in_transaction_step_1' ✅ [7/8] [step] 'in_transaction_step_2' ❌ @@ -149,7 +149,7 @@ RSpec.describe Service::StepsInspector do [1/8] [options] 'default' ✅ [2/8] [model] 'model' ✅ [3/8] [policy] 'policy' ✅ ⚠️ <= expected to return false but got true instead - [4/8] [contract] 'default' ✅ + [4/8] [params] 'default' ✅ [5/8] [transaction] [6/8] [step] 'in_transaction_step_1' ✅ [7/8] [step] 'in_transaction_step_2' ✅ @@ -173,7 +173,7 @@ RSpec.describe Service::StepsInspector do [1/8] [options] 'default' ✅ [2/8] [model] 'model' ✅ [3/8] [policy] 'policy' ❌ ⚠️ <= expected to return true but got false instead - [4/8] [contract] 'default' + [4/8] [params] 'default' [5/8] [transaction] [6/8] [step] 'in_transaction_step_1' [7/8] [step] 'in_transaction_step_2' @@ -223,7 +223,7 @@ RSpec.describe Service::StepsInspector do end end - context "when the contract step is failing" do + context "when the params step is failing" do let(:parameter) { nil } it "returns an error related to the contract" do diff --git a/spec/services/user/action/trigger_post_action_spec.rb b/spec/services/user/action/trigger_post_action_spec.rb index 5b5c5895994..b732f20ff5f 100644 --- a/spec/services/user/action/trigger_post_action_spec.rb +++ b/spec/services/user/action/trigger_post_action_spec.rb @@ -2,13 +2,13 @@ RSpec.describe User::Action::TriggerPostAction do describe ".call" do - subject(:action) { described_class.call(guardian:, post:, contract:) } + subject(:action) { described_class.call(guardian:, post:, params:) } fab!(:post) fab!(:admin) let(:guardian) { admin.guardian } - let(:contract) { User::Suspend::Contract.new(post_action:, post_edit:) } + let(:params) { User::Suspend::Contract.new(post_action:, post_edit:) } let(:post_action) { nil } let(:post_edit) { nil } diff --git a/spec/services/user/silence_spec.rb b/spec/services/user/silence_spec.rb index 26023224f84..89f55672941 100644 --- a/spec/services/user/silence_spec.rb +++ b/spec/services/user/silence_spec.rb @@ -79,7 +79,7 @@ RSpec.describe User::Silence do expect(User::Action::TriggerPostAction).to have_received(:call).with( guardian:, post: nil, - contract: result[:contract], + params: result[:params], ) end end diff --git a/spec/services/user/suspend_spec.rb b/spec/services/user/suspend_spec.rb index b99c78fd6d2..243c9b9900f 100644 --- a/spec/services/user/suspend_spec.rb +++ b/spec/services/user/suspend_spec.rb @@ -73,7 +73,7 @@ RSpec.describe User::Suspend do expect(User::Action::TriggerPostAction).to have_received(:call).with( guardian:, post: nil, - contract: result[:contract], + params: result[:params], ) end