diff --git a/app/services/flags/create_flag.rb b/app/services/flags/create_flag.rb index 09addd884ec..d0089b02469 100644 --- a/app/services/flags/create_flag.rb +++ b/app/services/flags/create_flag.rb @@ -3,28 +3,26 @@ class Flags::CreateFlag include Service::Base - contract - policy :invalid_access - policy :unique_name - model :flag, :instantiate_flag - - transaction do - step :create - step :log - end - - class Contract + contract do attribute :name, :string attribute :description, :string attribute :require_message, :boolean attribute :enabled, :boolean attribute :applies_to + validates :name, presence: true validates :description, presence: true validates :name, length: { maximum: Flag::MAX_NAME_LENGTH } validates :description, length: { maximum: Flag::MAX_DESCRIPTION_LENGTH } validates :applies_to, inclusion: { in: -> { Flag.valid_applies_to_types } }, allow_nil: false end + policy :invalid_access + policy :unique_name + model :flag, :instantiate_flag + transaction do + step :create + step :log + end private diff --git a/app/services/flags/reorder_flag.rb b/app/services/flags/reorder_flag.rb index a306a7cd32f..bd3ae1125e4 100644 --- a/app/services/flags/reorder_flag.rb +++ b/app/services/flags/reorder_flag.rb @@ -5,23 +5,21 @@ VALID_DIRECTIONS = %w[up down] class Flags::ReorderFlag include Service::Base - contract + contract do + attribute :flag_id, :integer + attribute :direction, :string + + validates :flag_id, presence: true + validates :direction, inclusion: { in: VALID_DIRECTIONS } + end model :flag policy :invalid_access policy :invalid_move - transaction do step :move step :log end - class Contract - attribute :flag_id, :integer - attribute :direction, :string - validates :flag_id, presence: true - validates :direction, inclusion: { in: VALID_DIRECTIONS } - end - private def fetch_flag(flag_id:) diff --git a/app/services/flags/toggle_flag.rb b/app/services/flags/toggle_flag.rb index d7c65258fd7..06afe4f4424 100644 --- a/app/services/flags/toggle_flag.rb +++ b/app/services/flags/toggle_flag.rb @@ -3,20 +3,18 @@ class Flags::ToggleFlag include Service::Base - contract + contract do + attribute :flag_id, :integer + + validates :flag_id, presence: true + end model :flag policy :invalid_access - transaction do step :toggle step :log end - class Contract - attribute :flag_id, :integer - validates :flag_id, presence: true - end - private def fetch_flag(flag_id:) diff --git a/app/services/flags/update_flag.rb b/app/services/flags/update_flag.rb index eaaee969d15..766d2bb701c 100644 --- a/app/services/flags/update_flag.rb +++ b/app/services/flags/update_flag.rb @@ -3,30 +3,28 @@ class Flags::UpdateFlag include Service::Base - contract - model :flag - policy :not_system - policy :not_used - policy :invalid_access - policy :unique_name - - transaction do - step :update - step :log - end - - class Contract + contract do attribute :name, :string attribute :description, :string attribute :require_message, :boolean attribute :enabled, :boolean attribute :applies_to + validates :name, presence: true validates :description, presence: true validates :name, length: { maximum: Flag::MAX_NAME_LENGTH } validates :description, length: { maximum: Flag::MAX_DESCRIPTION_LENGTH } validates :applies_to, inclusion: { in: -> { Flag.valid_applies_to_types } }, allow_nil: false end + model :flag + policy :not_system + policy :not_used + policy :invalid_access + policy :unique_name + transaction do + step :update + step :log + end private diff --git a/app/services/update_site_setting.rb b/app/services/update_site_setting.rb index 0b4f28ff1f3..70f2d1d6325 100644 --- a/app/services/update_site_setting.rb +++ b/app/services/update_site_setting.rb @@ -4,20 +4,18 @@ class UpdateSiteSetting include Service::Base policy :current_user_is_admin - contract - step :convert_name_to_sym - policy :setting_is_visible - policy :setting_is_configurable - step :cleanup_value - step :save - - class Contract + contract do attribute :setting_name attribute :new_value attribute :allow_changing_hidden, :boolean, default: false validates :setting_name, presence: true end + step :convert_name_to_sym + policy :setting_is_visible + policy :setting_is_configurable + step :cleanup_value + step :save private diff --git a/app/services/user/silence.rb b/app/services/user/silence.rb index 778afd4cceb..afc87131942 100644 --- a/app/services/user/silence.rb +++ b/app/services/user/silence.rb @@ -3,16 +3,7 @@ class User::Silence include Service::Base - contract - model :user - policy :not_silenced_already, class_name: User::Policy::NotAlreadySilenced - model :users - policy :can_silence_all_users - step :silence - model :post, optional: true - step :perform_post_action - - class Contract + contract do attribute :user_id, :integer attribute :reason, :string attribute :message, :string @@ -28,6 +19,13 @@ class User::Silence validates :other_user_ids, length: { maximum: User::MAX_SIMILAR_USERS } validates :post_action, inclusion: { in: %w[delete delete_replies edit] }, allow_blank: true end + model :user + policy :not_silenced_already, class_name: User::Policy::NotAlreadySilenced + model :users + policy :can_silence_all_users + step :silence + model :post, optional: true + step :perform_post_action private diff --git a/app/services/user/suspend.rb b/app/services/user/suspend.rb index 806f730db7b..bd08a6dc2e9 100644 --- a/app/services/user/suspend.rb +++ b/app/services/user/suspend.rb @@ -3,16 +3,7 @@ class User::Suspend include Service::Base - contract - model :user - policy :not_suspended_already, class_name: User::Policy::NotAlreadySuspended - model :users - policy :can_suspend_all_users - step :suspend - model :post, optional: true - step :perform_post_action - - class Contract + contract do attribute :user_id, :integer attribute :reason, :string attribute :message, :string @@ -28,6 +19,13 @@ class User::Suspend validates :other_user_ids, length: { maximum: User::MAX_SIMILAR_USERS } validates :post_action, inclusion: { in: %w[delete delete_replies edit] }, allow_blank: true end + model :user + policy :not_suspended_already, class_name: User::Policy::NotAlreadySuspended + model :users + policy :can_suspend_all_users + step :suspend + model :post, optional: true + step :perform_post_action private diff --git a/lib/service.rb b/lib/service.rb index 8f1ef6c6973..07d39252657 100644 --- a/lib/service.rb +++ b/lib/service.rb @@ -11,12 +11,12 @@ module Service # Currently, there are 5 types of steps: # # * +contract(name = :default)+: used to validate the input parameters, - # typically provided by a user calling an endpoint. A special embedded - # +Contract+ class has to be defined to holds 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. + # 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. # * +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 diff --git a/lib/service/base.rb b/lib/service/base.rb index cb7f38d5498..46559ddb858 100644 --- a/lib/service/base.rb +++ b/lib/service/base.rb @@ -78,10 +78,12 @@ module Service steps << ModelStep.new(name, step_name, optional: optional) end - def contract(name = :default, class_name: self::Contract, default_values_from: nil) + def contract(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( name, - class_name: class_name, + class_name: contract_class, default_values_from: default_values_from, ) end @@ -214,20 +216,6 @@ module Service included do # The global context which is available from any step. attr_reader :context - - # @!visibility private - # Internal class used to setup the base contract of the service. - self::Contract = - Class.new do - include ActiveModel::API - include ActiveModel::Attributes - include ActiveModel::AttributeMethods - include ActiveModel::Validations::Callbacks - - def raw_attributes - @attributes.values_before_type_cast - end - end end class_methods do @@ -306,10 +294,10 @@ module Service # end # @!scope class - # @!method contract(name = :default, class_name: self::Contract, default_values_from: nil) + # @!method contract(name = :default, default_values_from: nil, &block) # @param name [Symbol] name for this contract - # @param class_name [Class] a class defining the 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. # @@ -317,9 +305,7 @@ module Service # (can be customized by providing the +name+ argument). # # @example - # contract - # - # class Contract + # contract do # attribute :name # validates :name, presence: true # end diff --git a/lib/service/contract_base.rb b/lib/service/contract_base.rb new file mode 100644 index 00000000000..79a0413df2b --- /dev/null +++ b/lib/service/contract_base.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +class Service::ContractBase + include ActiveModel::API + include ActiveModel::Attributes + include ActiveModel::AttributeMethods + include ActiveModel::Validations::Callbacks + + def raw_attributes + @attributes.values_before_type_cast + 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 318459615fe..291c0d761e6 100644 --- a/plugins/chat/app/services/chat/add_users_to_channel.rb +++ b/plugins/chat/app/services/chat/add_users_to_channel.rb @@ -22,33 +22,28 @@ module Chat # @option params_to_create [Array] usernames # @option params_to_create [Array] groups # @return [Service::Base::Context] - contract - model :channel - policy :can_add_users_to_channel - model :target_users, optional: true - policy :satisfies_dms_max_users_limit, - class_name: Chat::DirectMessageChannel::Policy::MaxUsersExcess - - transaction do - step :upsert_memberships - step :recompute_users_count - step :notice_channel - end - - # @!visibility private - class Contract + contract do attribute :usernames, :array attribute :groups, :array - attribute :channel_id, :integer - validates :channel_id, presence: true + validates :channel_id, presence: true validate :target_presence def target_presence usernames.present? || groups.present? end end + model :channel + policy :can_add_users_to_channel + model :target_users, optional: true + policy :satisfies_dms_max_users_limit, + class_name: Chat::DirectMessageChannel::Policy::MaxUsersExcess + transaction do + step :upsert_memberships + step :recompute_users_count + step :notice_channel + end private @@ -128,20 +123,17 @@ module Chat return if added_users.blank? - result = - ::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, - ), - ) - - fail!(failure: "Failed to notice the channel") if result.failure? + ::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, + ), + ) { on_failure { fail!(failure: "Failed to notice the channel") } } end 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 6a95cc4d314..6518e6f2e53 100644 --- a/plugins/chat/app/services/chat/auto_join_channel_batch.rb +++ b/plugins/chat/app/services/chat/auto_join_channel_batch.rb @@ -14,13 +14,7 @@ module Chat class AutoJoinChannelBatch include Service::Base - contract - model :channel - step :create_memberships - step :recalculate_user_count - step :publish_new_channel - - class Contract + contract do # Backward-compatible attributes attribute :chat_channel_id, :integer attribute :starts_at, :integer @@ -41,6 +35,10 @@ module Chat self.end_user_id ||= ends_at end end + model :channel + step :create_memberships + step :recalculate_user_count + step :publish_new_channel private 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 fff835bc986..6da61967c1d 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 @@ -14,7 +14,11 @@ module Chat class HandleCategoryUpdated include Service::Base - contract + contract do + attribute :category_id, :integer + + validates :category_id, presence: true + end step :assign_defaults policy :chat_enabled model :category @@ -23,12 +27,6 @@ module Chat step :remove_users_without_channel_permission step :publish - class Contract - attribute :category_id, :integer - - validates :category_id, presence: true - end - private def assign_defaults 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 9d8e42b6c33..fc29b1152d0 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 @@ -21,7 +21,11 @@ module Chat class HandleDestroyedGroup include Service::Base - contract + contract do + attribute :destroyed_group_user_ids + + validates :destroyed_group_user_ids, presence: true + end step :assign_defaults policy :chat_enabled policy :not_everyone_allowed @@ -30,12 +34,6 @@ module Chat step :remove_users_without_channel_permission step :publish - class Contract - attribute :destroyed_group_user_ids - - validates :destroyed_group_user_ids, presence: true - end - private def assign_defaults 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 c6bdec7171e..5862ba696f8 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 @@ -20,7 +20,11 @@ module Chat class HandleUserRemovedFromGroup include Service::Base - contract + contract do + attribute :user_id, :integer + + validates :user_id, presence: true + end step :assign_defaults policy :chat_enabled policy :not_everyone_allowed @@ -30,12 +34,6 @@ module Chat step :remove_from_private_channels step :publish - class Contract - attribute :user_id, :integer - - validates :user_id, presence: true - end - private def assign_defaults diff --git a/plugins/chat/app/services/chat/create_category_channel.rb b/plugins/chat/app/services/chat/create_category_channel.rb index 5f98b74ac9f..179bcf7947b 100644 --- a/plugins/chat/app/services/chat/create_category_channel.rb +++ b/plugins/chat/app/services/chat/create_category_channel.rb @@ -29,17 +29,7 @@ module Chat policy :public_channels_enabled policy :can_create_channel - contract - model :category, :fetch_category - policy :category_channel_does_not_exist - transaction do - model :channel, :create_channel - model :membership, :create_membership - end - step :enforce_automatic_channel_memberships - - # @!visibility private - class Contract + contract do attribute :name, :string attribute :description, :string attribute :slug, :string @@ -55,6 +45,13 @@ module Chat validates :category_id, presence: true validates :name, length: { maximum: SiteSetting.max_topic_title_length } end + model :category + policy :category_channel_does_not_exist + transaction do + model :channel, :create_channel + model :membership, :create_membership + end + step :enforce_automatic_channel_memberships private 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 fd19a811a49..3af0d6df2c5 100644 --- a/plugins/chat/app/services/chat/create_direct_message_channel.rb +++ b/plugins/chat/app/services/chat/create_direct_message_channel.rb @@ -23,7 +23,18 @@ module Chat # @option params_to_create [Boolean] upsert # @return [Service::Base::Context] - contract + contract do + attribute :name, :string + attribute :target_usernames, :array + attribute :target_groups, :array + attribute :upsert, :boolean, default: false + + validate :target_presence + + def target_presence + target_usernames.present? || target_groups.present? + end + end model :target_users policy :can_create_direct_message policy :satisfies_dms_max_users_limit, @@ -38,20 +49,6 @@ module Chat step :update_memberships step :recompute_users_count - # @!visibility private - class Contract - attribute :name, :string - attribute :target_usernames, :array - attribute :target_groups, :array - attribute :upsert, :boolean, default: false - - validate :target_presence - - def target_presence - target_usernames.present? || target_groups.present? - end - end - private def can_create_direct_message(guardian:, target_users:) diff --git a/plugins/chat/app/services/chat/create_message.rb b/plugins/chat/app/services/chat/create_message.rb index 4b6170eb195..4aebe1471ed 100644 --- a/plugins/chat/app/services/chat/create_message.rb +++ b/plugins/chat/app/services/chat/create_message.rb @@ -22,36 +22,7 @@ module Chat # @param incoming_chat_webhook [Chat::IncomingWebhook] policy :no_silenced_user - contract - model :channel - step :enforce_membership - model :membership - policy :allowed_to_create_message_in_channel, class_name: Chat::Channel::Policy::MessageCreation - model :reply, optional: true - policy :ensure_reply_consistency - model :thread, optional: true - policy :ensure_valid_thread_for_channel - policy :ensure_thread_matches_parent - model :uploads, optional: true - step :clean_message - model :message_instance, :instantiate_message - - transaction do - step :create_excerpt - step :update_created_by_sdk - step :save_message - step :delete_drafts - step :post_process_thread - step :create_webhook_event - step :update_channel_last_message - step :update_membership_last_read - step :process_direct_message_channel - end - step :publish_new_thread - step :process - step :publish_user_tracking_state - - class Contract + contract do attribute :chat_channel_id, :string attribute :in_reply_to_id, :string attribute :context_topic_id, :integer @@ -71,6 +42,32 @@ module Chat validates :chat_channel_id, presence: true validates :message, presence: true, if: -> { upload_ids.blank? } end + model :channel + step :enforce_membership + model :membership + policy :allowed_to_create_message_in_channel, class_name: Chat::Channel::Policy::MessageCreation + model :reply, optional: true + policy :ensure_reply_consistency + model :thread, optional: true + policy :ensure_valid_thread_for_channel + policy :ensure_thread_matches_parent + model :uploads, optional: true + step :clean_message + model :message_instance, :instantiate_message + transaction do + step :create_excerpt + step :update_created_by_sdk + step :save_message + step :delete_drafts + step :post_process_thread + step :create_webhook_event + step :update_channel_last_message + step :update_membership_last_read + step :process_direct_message_channel + end + step :publish_new_thread + step :process + step :publish_user_tracking_state private diff --git a/plugins/chat/app/services/chat/create_thread.rb b/plugins/chat/app/services/chat/create_thread.rb index 0e1316de21e..e7b6f042b60 100644 --- a/plugins/chat/app/services/chat/create_thread.rb +++ b/plugins/chat/app/services/chat/create_thread.rb @@ -16,7 +16,14 @@ module Chat # @option params_to_create [String,nil] title # @return [Service::Base::Context] - contract + contract do + attribute :original_message_id, :integer + attribute :channel_id, :integer + attribute :title, :string + + validates :original_message_id, :channel_id, presence: true + validates :title, length: { maximum: Chat::Thread::MAX_TITLE_LENGTH } + end model :channel policy :can_view_channel policy :threading_enabled_for_channel @@ -29,16 +36,6 @@ module Chat step :trigger_chat_thread_created_event end - # @!visibility private - class Contract - attribute :original_message_id, :integer - attribute :channel_id, :integer - attribute :title, :string - - validates :original_message_id, :channel_id, presence: true - validates :title, length: { maximum: Chat::Thread::MAX_TITLE_LENGTH } - end - private def fetch_channel(contract:) diff --git a/plugins/chat/app/services/chat/flag_message.rb b/plugins/chat/app/services/chat/flag_message.rb index 0bdea682e13..ad610dddebe 100644 --- a/plugins/chat/app/services/chat/flag_message.rb +++ b/plugins/chat/app/services/chat/flag_message.rb @@ -26,27 +26,22 @@ module Chat # the force_review option. # @return [Service::Base::Context] - contract - model :message - policy :can_flag_message_in_channel - step :flag_message - - # @!visibility private - class Contract + contract do attribute :message_id, :integer - validates :message_id, presence: true - attribute :channel_id, :integer - validates :channel_id, presence: true - attribute :flag_type_id, :integer - validates :flag_type_id, inclusion: { in: ->(_flag_type) { ::ReviewableScore.types.values } } - attribute :message, :string attribute :is_warning, :boolean attribute :take_action, :boolean attribute :queue_for_review, :boolean + + validates :message_id, presence: true + validates :channel_id, presence: true + validates :flag_type_id, inclusion: { in: ->(_flag_type) { ::ReviewableScore.types.values } } end + model :message + policy :can_flag_message_in_channel + step :flag_message private 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 e2346f42f7a..1a492af4584 100644 --- a/plugins/chat/app/services/chat/invite_users_to_channel.rb +++ b/plugins/chat/app/services/chat/invite_users_to_channel.rb @@ -16,23 +16,19 @@ module Chat # @option optional_params [Integer, nil] message_id # @return [Service::Base::Context] - contract + contract do + attribute :user_ids, :array + attribute :channel_id, :integer + attribute :message_id, :integer + + validates :user_ids, presence: true + validates :channel_id, presence: true + end model :channel policy :can_view_channel model :users, optional: true step :send_invite_notifications - # @!visibility private - class Contract - attribute :user_ids, :array - validates :user_ids, presence: true - - attribute :channel_id, :integer - validates :channel_id, presence: true - - attribute :message_id, :integer - end - private def fetch_channel(contract:) diff --git a/plugins/chat/app/services/chat/leave_channel.rb b/plugins/chat/app/services/chat/leave_channel.rb index f905eb04ff6..63afd9ab3df 100644 --- a/plugins/chat/app/services/chat/leave_channel.rb +++ b/plugins/chat/app/services/chat/leave_channel.rb @@ -17,17 +17,15 @@ module Chat # @param [Integer] channel_id of the channel # @return [Service::Base::Context] - contract + contract do + attribute :channel_id, :integer + + validates :channel_id, presence: true + end model :channel step :leave step :recompute_users_count - # @!visibility private - class Contract - attribute :channel_id, :integer - validates :channel_id, presence: true - end - private def fetch_channel(contract:) diff --git a/plugins/chat/app/services/chat/list_channel_messages.rb b/plugins/chat/app/services/chat/list_channel_messages.rb index 62b3770ccad..1ec134f8e98 100644 --- a/plugins/chat/app/services/chat/list_channel_messages.rb +++ b/plugins/chat/app/services/chat/list_channel_messages.rb @@ -15,11 +15,34 @@ module Chat # @param [Guardian] guardian # @return [Service::Base::Context] - contract + contract do + attribute :channel_id, :integer + attribute :page_size, :integer + + # If this is not present, then we just fetch messages with page_size + # and direction. + attribute :target_message_id, :integer # (optional) + attribute :direction, :string # (optional) + attribute :fetch_from_last_read, :boolean # (optional) + attribute :target_date, :string # (optional) + + validates :channel_id, presence: true + validates :page_size, + numericality: { + less_than_or_equal_to: ::Chat::MessagesQuery::MAX_PAGE_SIZE, + only_integer: true, + }, + allow_nil: true + validates :direction, + inclusion: { + in: Chat::MessagesQuery::VALID_DIRECTIONS, + }, + allow_nil: true + end model :channel policy :can_view_channel - step :fetch_optional_membership + model :membership, optional: true step :enabled_threads? step :determine_target_message_id policy :target_message_exists @@ -31,40 +54,14 @@ module Chat step :update_membership_last_viewed_at step :update_user_last_channel - class Contract - attribute :channel_id, :integer - validates :channel_id, presence: true - - attribute :page_size, :integer - validates :page_size, - numericality: { - less_than_or_equal_to: ::Chat::MessagesQuery::MAX_PAGE_SIZE, - only_integer: true, - }, - allow_nil: true - - # If this is not present, then we just fetch messages with page_size - # and direction. - attribute :target_message_id, :integer # (optional) - attribute :direction, :string # (optional) - attribute :fetch_from_last_read, :boolean # (optional) - attribute :target_date, :string # (optional) - - validates :direction, - inclusion: { - in: Chat::MessagesQuery::VALID_DIRECTIONS, - }, - allow_nil: true - end - private def fetch_channel(contract:) ::Chat::Channel.includes(:chatable).find_by(id: contract.channel_id) end - def fetch_optional_membership(channel:, guardian:) - context.membership = channel.membership_for(guardian.user) + def fetch_membership(channel:, guardian:) + channel.membership_for(guardian.user) end def enabled_threads?(channel:) 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 152e4268678..63fc584ff57 100644 --- a/plugins/chat/app/services/chat/list_channel_thread_messages.rb +++ b/plugins/chat/app/services/chat/list_channel_thread_messages.rb @@ -15,19 +15,8 @@ module Chat # @option optional_params [Integer] thread_id # @return [Service::Base::Context] - contract - - model :thread - policy :can_view_thread - step :fetch_optional_membership - step :determine_target_message_id - policy :target_message_exists - step :fetch_messages - - class Contract + contract do attribute :thread_id, :integer - validates :thread_id, presence: true - # If this is not present, then we just fetch messages with page_size # and direction. attribute :target_message_id, :integer # (optional) @@ -38,6 +27,7 @@ module Chat attribute :fetch_from_first_message, :boolean # (optional) attribute :target_date, :string # (optional) + validates :thread_id, presence: true validates :direction, inclusion: { in: Chat::MessagesQuery::VALID_DIRECTIONS, @@ -50,13 +40,15 @@ module Chat }, allow_nil: true end + model :thread + policy :can_view_thread + model :membership, optional: true + step :determine_target_message_id + policy :target_message_exists + model :messages, optional: true private - def fetch_optional_membership(thread:, guardian:) - context.membership = thread.membership_for(guardian.user) - end - def fetch_thread(contract:) ::Chat::Thread.strict_loading.includes(channel: :chatable).find_by(id: contract.thread_id) end @@ -65,6 +57,10 @@ module Chat guardian.user == Discourse.system_user || guardian.can_preview_chat_channel?(thread.channel) end + def fetch_membership(thread:, guardian:) + thread.membership_for(guardian.user) + end + def determine_target_message_id(contract:, membership:, guardian:, thread:) if contract.fetch_from_last_message context.target_message_id = thread.last_message_id @@ -106,7 +102,7 @@ module Chat context.can_load_more_past = messages_data[:can_load_more_past] context.can_load_more_future = messages_data[:can_load_more_future] - context.messages = [ + [ messages_data[:messages], messages_data[:past_messages]&.reverse, messages_data[:target_message], diff --git a/plugins/chat/app/services/chat/lookup_channel_threads.rb b/plugins/chat/app/services/chat/lookup_channel_threads.rb index 261f35bb916..2c57dc76444 100644 --- a/plugins/chat/app/services/chat/lookup_channel_threads.rb +++ b/plugins/chat/app/services/chat/lookup_channel_threads.rb @@ -24,7 +24,19 @@ module Chat # @param [Integer] offset # @return [Service::Base::Context] - contract + contract do + attribute :channel_id, :integer + attribute :limit, :integer + attribute :offset, :integer + + validates :channel_id, presence: true + validates :limit, + numericality: { + less_than_or_equal_to: THREADS_LIMIT, + only_integer: true, + }, + allow_nil: true + end step :set_limit step :set_offset model :channel @@ -36,22 +48,6 @@ module Chat step :fetch_participants step :build_load_more_url - # @!visibility private - class Contract - attribute :channel_id, :integer - validates :channel_id, presence: true - - attribute :limit, :integer - validates :limit, - numericality: { - less_than_or_equal_to: THREADS_LIMIT, - only_integer: true, - }, - allow_nil: true - - attribute :offset, :integer - end - private def set_limit(contract:) diff --git a/plugins/chat/app/services/chat/lookup_thread.rb b/plugins/chat/app/services/chat/lookup_thread.rb index 937993bfcd5..f8aa427d3b5 100644 --- a/plugins/chat/app/services/chat/lookup_thread.rb +++ b/plugins/chat/app/services/chat/lookup_thread.rb @@ -16,20 +16,17 @@ module Chat # @param [Guardian] guardian # @return [Service::Base::Context] - contract - model :thread, :fetch_thread - policy :invalid_access - policy :threading_enabled_for_channel - step :fetch_membership - step :fetch_participants - - # @!visibility private - class Contract + contract do attribute :thread_id, :integer attribute :channel_id, :integer validates :thread_id, :channel_id, presence: true end + model :thread + policy :invalid_access + policy :threading_enabled_for_channel + model :membership, optional: true + model :participants, optional: true private @@ -50,11 +47,11 @@ module Chat end def fetch_membership(thread:, guardian:) - context.membership = thread.membership_for(guardian.user) + thread.membership_for(guardian.user) end def fetch_participants(thread:) - context.participants = ::Chat::ThreadParticipantQuery.call(thread_ids: [thread.id])[thread.id] + ::Chat::ThreadParticipantQuery.call(thread_ids: [thread.id])[thread.id] end end end diff --git a/plugins/chat/app/services/chat/lookup_user_threads.rb b/plugins/chat/app/services/chat/lookup_user_threads.rb index 75d311e1553..571c9f7c7d1 100644 --- a/plugins/chat/app/services/chat/lookup_user_threads.rb +++ b/plugins/chat/app/services/chat/lookup_user_threads.rb @@ -20,7 +20,10 @@ module Chat # @param [Integer] offset # @return [Service::Base::Context] - contract + contract do + attribute :limit, :integer + attribute :offset, :integer + end step :set_limit step :set_offset model :threads @@ -29,12 +32,6 @@ module Chat step :fetch_participants step :build_load_more_url - # @!visibility private - class Contract - attribute :limit, :integer - attribute :offset, :integer - end - private def set_limit(contract:) 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 bd102747818..4a03d07febf 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 @@ -21,19 +21,16 @@ module Chat # @param [Guardian] guardian # @return [Service::Base::Context] - contract - model :thread - policy :threading_enabled_for_channel - policy :can_view_channel - transaction { step :create_or_update_membership } - - # @!visibility private - class Contract + contract do attribute :thread_id, :integer attribute :channel_id, :integer validates :thread_id, :channel_id, presence: true end + model :thread + policy :threading_enabled_for_channel + policy :can_view_channel + transaction { step :create_or_update_membership } private diff --git a/plugins/chat/app/services/chat/restore_message.rb b/plugins/chat/app/services/chat/restore_message.rb index d5bf0d835fe..757d672ba4c 100644 --- a/plugins/chat/app/services/chat/restore_message.rb +++ b/plugins/chat/app/services/chat/restore_message.rb @@ -17,7 +17,13 @@ module Chat # @param [Guardian] guardian # @return [Service::Base::Context] - contract + contract do + attribute :message_id, :integer + attribute :channel_id, :integer + + validates :message_id, presence: true + validates :channel_id, presence: true + end model :message policy :invalid_access transaction do @@ -27,14 +33,6 @@ module Chat end step :publish_events - # @!visibility private - class Contract - attribute :message_id, :integer - attribute :channel_id, :integer - validates :message_id, presence: true - validates :channel_id, presence: true - end - private def fetch_message(contract:) diff --git a/plugins/chat/app/services/chat/search_chatable.rb b/plugins/chat/app/services/chat/search_chatable.rb index 37566821905..47c29aeb168 100644 --- a/plugins/chat/app/services/chat/search_chatable.rb +++ b/plugins/chat/app/services/chat/search_chatable.rb @@ -16,16 +16,7 @@ module Chat # @param [Guardian] guardian # @return [Service::Base::Context] - contract - step :clean_term - model :memberships, optional: true - model :users, optional: true - model :groups, optional: true - model :category_channels, optional: true - model :direct_message_channels, optional: true - - # @!visibility private - class Contract + contract do attribute :term, :string, default: "" attribute :include_users, :boolean, default: true attribute :include_groups, :boolean, default: true @@ -33,6 +24,12 @@ module Chat attribute :include_direct_message_channels, :boolean, default: true attribute :excluded_memberships_channel_id, :integer end + step :clean_term + model :memberships, optional: true + model :users, optional: true + model :groups, optional: true + model :category_channels, optional: true + model :direct_message_channels, optional: true private diff --git a/plugins/chat/app/services/chat/stop_message_streaming.rb b/plugins/chat/app/services/chat/stop_message_streaming.rb index 730c93b749d..ce35b04b70d 100644 --- a/plugins/chat/app/services/chat/stop_message_streaming.rb +++ b/plugins/chat/app/services/chat/stop_message_streaming.rb @@ -13,7 +13,11 @@ module Chat # @param [Integer] message_id # @param [Guardian] guardian # @return [Service::Base::Context] - contract + contract do + attribute :message_id, :integer + + validates :message_id, presence: true + end model :message step :enforce_membership model :membership @@ -21,13 +25,6 @@ module Chat step :stop_message_streaming step :publish_message_streaming_state - # @!visibility private - class Contract - attribute :message_id, :integer - - validates :message_id, presence: true - end - private def fetch_message(contract:) diff --git a/plugins/chat/app/services/chat/tracking_state.rb b/plugins/chat/app/services/chat/tracking_state.rb index 5bfd53ffe2a..7704b29821f 100644 --- a/plugins/chat/app/services/chat/tracking_state.rb +++ b/plugins/chat/app/services/chat/tracking_state.rb @@ -33,26 +33,17 @@ module Chat # @param [Guardian] guardian # @return [Service::Base::Context] - contract - step :cast_thread_and_channel_ids_to_integer - model :report - - # @!visibility private - class Contract - attribute :channel_ids, default: [] - attribute :thread_ids, default: [] + contract do + attribute :channel_ids, :array, default: [] + attribute :thread_ids, :array, default: [] attribute :include_missing_memberships, default: false attribute :include_threads, default: false attribute :include_read, default: true end + model :report private - def cast_thread_and_channel_ids_to_integer(contract:) - contract.thread_ids = contract.thread_ids.map(&:to_i) - contract.channel_ids = contract.channel_ids.map(&:to_i) - end - def fetch_report(contract:, guardian:) ::Chat::TrackingStateReportQuery.call( guardian: guardian, diff --git a/plugins/chat/app/services/chat/trash_message.rb b/plugins/chat/app/services/chat/trash_message.rb index 6bb8100bc1e..51be333c270 100644 --- a/plugins/chat/app/services/chat/trash_message.rb +++ b/plugins/chat/app/services/chat/trash_message.rb @@ -17,7 +17,13 @@ module Chat # @param [Guardian] guardian # @return [Service::Base::Context] - contract + contract do + attribute :message_id, :integer + attribute :channel_id, :integer + + validates :message_id, presence: true + validates :channel_id, presence: true + end model :message policy :invalid_access transaction do @@ -29,14 +35,6 @@ module Chat end step :publish_events - # @!visibility private - class Contract - attribute :message_id, :integer - attribute :channel_id, :integer - validates :message_id, presence: true - validates :channel_id, presence: true - end - private def fetch_message(contract:) diff --git a/plugins/chat/app/services/chat/trash_messages.rb b/plugins/chat/app/services/chat/trash_messages.rb index 1593752e793..eb278cde763 100644 --- a/plugins/chat/app/services/chat/trash_messages.rb +++ b/plugins/chat/app/services/chat/trash_messages.rb @@ -17,7 +17,13 @@ module Chat # @param [Guardian] guardian # @return [Service::Base::Context] - contract + contract do + attribute :channel_id, :integer + attribute :message_ids, :array + + validates :channel_id, presence: true + validates :message_ids, length: { minimum: 1, maximum: 50 } + end model :messages policy :can_delete_all_chat_messages transaction do @@ -29,14 +35,6 @@ module Chat end step :publish_events - # @!visibility private - class Contract - attribute :channel_id, :integer - attribute :message_ids, :array - validates :channel_id, presence: true - validates :message_ids, length: { minimum: 1, maximum: 50 } - end - private def fetch_messages(contract:) diff --git a/plugins/chat/app/services/chat/unfollow_channel.rb b/plugins/chat/app/services/chat/unfollow_channel.rb index 5f58c3ebb96..851ef0260d0 100644 --- a/plugins/chat/app/services/chat/unfollow_channel.rb +++ b/plugins/chat/app/services/chat/unfollow_channel.rb @@ -17,15 +17,13 @@ module Chat # @param [Integer] channel_id of the channel # @return [Service::Base::Context] - contract - model :channel - step :unfollow - - # @!visibility private - class Contract + contract do attribute :channel_id, :integer + validates :channel_id, presence: true end + model :channel + step :unfollow private diff --git a/plugins/chat/app/services/chat/update_channel.rb b/plugins/chat/app/services/chat/update_channel.rb index fbf920fbd89..38a3faad535 100644 --- a/plugins/chat/app/services/chat/update_channel.rb +++ b/plugins/chat/app/services/chat/update_channel.rb @@ -32,17 +32,9 @@ module Chat # @option params_to_edit [Boolean] allow_channel_wide_mentions Allow the use of @here and @all in the channel. # @return [Service::Base::Context] - model :channel, :fetch_channel + model :channel policy :check_channel_permission - contract default_values_from: :channel - step :update_channel - step :mark_all_threads_as_read_if_needed - step :update_site_settings_if_needed - step :publish_channel_update - step :auto_join_users_if_needed - - # @!visibility private - class Contract + contract(default_values_from: :channel) do attribute :name, :string attribute :description, :string attribute :slug, :string @@ -56,6 +48,11 @@ module Chat ) end end + step :update_channel + step :mark_all_threads_as_read_if_needed + step :update_site_settings_if_needed + step :publish_channel_update + step :auto_join_users_if_needed private diff --git a/plugins/chat/app/services/chat/update_channel_status.rb b/plugins/chat/app/services/chat/update_channel_status.rb index 8719406d07c..1816599846c 100644 --- a/plugins/chat/app/services/chat/update_channel_status.rb +++ b/plugins/chat/app/services/chat/update_channel_status.rb @@ -16,15 +16,13 @@ module Chat # @return [Service::Base::Context] model :channel, :fetch_channel - contract - policy :check_channel_permission - step :change_status - - # @!visibility private - class Contract + contract do attribute :status + validates :status, inclusion: { in: Chat::Channel.editable_statuses.keys } end + policy :check_channel_permission + step :change_status private diff --git a/plugins/chat/app/services/chat/update_message.rb b/plugins/chat/app/services/chat/update_message.rb index 2bf2ecbb746..8bbc5fc36b2 100644 --- a/plugins/chat/app/services/chat/update_message.rb +++ b/plugins/chat/app/services/chat/update_message.rb @@ -15,7 +15,17 @@ module Chat # @param message [String] # @param upload_ids [Array] IDs of uploaded documents - contract + 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? } + end model :message model :uploads, optional: true step :enforce_membership @@ -23,7 +33,6 @@ module Chat policy :can_modify_channel_message policy :can_modify_message step :clean_message - transaction do step :modify_message step :update_excerpt @@ -32,22 +41,6 @@ module Chat step :publish end - class Contract - attribute :message_id, :string - validates :message_id, presence: true - - attribute :message, :string - validates :message, presence: true, if: -> { upload_ids.blank? } - - attribute :upload_ids, :array - - attribute :streaming, :boolean, default: false - - attribute :strip_whitespaces, :boolean, default: true - - attribute :process_inline, :boolean, default: Rails.env.test? - end - private def enforce_membership(guardian:, message:) diff --git a/plugins/chat/app/services/chat/update_thread.rb b/plugins/chat/app/services/chat/update_thread.rb index dccd3160986..3d0db4a1059 100644 --- a/plugins/chat/app/services/chat/update_thread.rb +++ b/plugins/chat/app/services/chat/update_thread.rb @@ -19,22 +19,19 @@ module Chat # @option params_to_edit [String,nil] title # @return [Service::Base::Context] - contract - model :thread, :fetch_thread - policy :can_view_channel - policy :can_edit_thread - policy :threading_enabled_for_channel - step :update - step :publish_metadata - - # @!visibility private - class Contract + contract do attribute :thread_id, :integer attribute :title, :string validates :thread_id, presence: true validates :title, length: { maximum: Chat::Thread::MAX_TITLE_LENGTH } end + model :thread + policy :can_view_channel + policy :can_edit_thread + policy :threading_enabled_for_channel + step :update + step :publish_metadata private 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 6d7b1266f13..4cdeae35a68 100644 --- a/plugins/chat/app/services/chat/update_thread_notification_settings.rb +++ b/plugins/chat/app/services/chat/update_thread_notification_settings.rb @@ -23,25 +23,21 @@ module Chat # @param [Guardian] guardian # @return [Service::Base::Context] - contract - model :thread, :fetch_thread - policy :can_view_channel - policy :threading_enabled_for_channel - transaction { step :create_or_update_membership } - - # @!visibility private - class Contract + contract do attribute :thread_id, :integer attribute :channel_id, :integer attribute :notification_level, :integer validates :thread_id, :channel_id, :notification_level, presence: true - validates :notification_level, inclusion: { in: Chat::UserChatThreadMembership.notification_levels.values, } end + model :thread, :fetch_thread + policy :can_view_channel + policy :threading_enabled_for_channel + transaction { step :create_or_update_membership } private 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 94600d55ab8..45629daa820 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 @@ -15,7 +15,12 @@ module Chat # @param [Guardian] guardian # @return [Service::Base::Context] - contract + contract do + attribute :message_id, :integer + attribute :channel_id, :integer + + validates :message_id, :channel_id, presence: true + end model :channel model :membership policy :invalid_access @@ -27,14 +32,6 @@ module Chat end step :publish_new_last_read_to_clients - # @!visibility private - class Contract - attribute :message_id, :integer - attribute :channel_id, :integer - - validates :message_id, :channel_id, presence: true - end - private def fetch_channel(contract:) 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 5004416503a..6fcb9d0e73d 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 @@ -17,7 +17,13 @@ module Chat # @param [Guardian] guardian # @return [Service::Base::Context] - contract + contract do + attribute :channel_id, :integer + attribute :thread_id, :integer + attribute :message_id, :integer + + validates :thread_id, :channel_id, presence: true + end model :thread policy :invalid_access model :membership @@ -27,15 +33,6 @@ module Chat step :mark_thread_read step :publish_new_last_read_to_clients - # @!visibility private - class Contract - attribute :channel_id, :integer - attribute :thread_id, :integer - attribute :message_id, :integer - - validates :thread_id, :channel_id, presence: true - end - private def fetch_thread(contract:) diff --git a/plugins/chat/app/services/chat/upsert_draft.rb b/plugins/chat/app/services/chat/upsert_draft.rb index 392ef5621b3..d6b4a5db889 100644 --- a/plugins/chat/app/services/chat/upsert_draft.rb +++ b/plugins/chat/app/services/chat/upsert_draft.rb @@ -20,20 +20,17 @@ module Chat # @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 # @return [Service::Base::Context] - contract - model :channel - policy :can_upsert_draft - step :check_thread_exists - step :upsert_draft - - # @!visibility private - class Contract + contract do attribute :channel_id, :integer validates :channel_id, presence: true attribute :thread_id, :integer attribute :data, :string end + model :channel + policy :can_upsert_draft + step :check_thread_exists + step :upsert_draft private diff --git a/spec/lib/service/runner_spec.rb b/spec/lib/service/runner_spec.rb index 1d8cd6680bb..b9d98c66b03 100644 --- a/spec/lib/service/runner_spec.rb +++ b/spec/lib/service/runner_spec.rb @@ -38,18 +38,17 @@ RSpec.describe Service::Runner do class FailedContractService include Service::Base - class Contract + contract do attribute :test + validates :test, presence: true end - - contract end class SuccessContractService include Service::Base - contract + contract {} end class FailureWithModelService diff --git a/spec/lib/steps_inspector_spec.rb b/spec/lib/steps_inspector_spec.rb index 38227239a7d..dcc0917460b 100644 --- a/spec/lib/steps_inspector_spec.rb +++ b/spec/lib/steps_inspector_spec.rb @@ -6,18 +6,16 @@ RSpec.describe StepsInspector do model :model policy :policy - contract + contract do + attribute :parameter + + validates :parameter, presence: true + end transaction do step :in_transaction_step_1 step :in_transaction_step_2 end step :final_step - - class Contract - attribute :parameter - - validates :parameter, presence: true - end end subject(:inspector) { described_class.new(result) }