DEV: Remove hash-like access from service contracts

We decided to keep only one way to access values from a contract. This
patch thus removes the hash-like access from contracts.
This commit is contained in:
Loïc Guitaut 2024-10-28 17:21:59 +01:00 committed by Loïc Guitaut
parent 4d0ed2e146
commit 2f334964f2
50 changed files with 293 additions and 257 deletions

View File

@ -4,6 +4,11 @@ class AdminNotices::Dismiss
include Service::Base
policy :invalid_access
params do
attribute :id, :integer
validates :id, presence: true
end
model :admin_notice, optional: true
transaction do
step :destroy
@ -17,7 +22,7 @@ class AdminNotices::Dismiss
end
def fetch_admin_notice(params:)
AdminNotice.find_by(id: params[:id])
AdminNotice.find_by(id: params.id)
end
def destroy(admin_notice:)

View File

@ -18,13 +18,13 @@ class Experiments::Toggle
end
def setting_is_available(params:)
SiteSetting.respond_to?(params[:setting_name])
SiteSetting.respond_to?(params.setting_name)
end
def toggle(params:, guardian:)
SiteSetting.set_and_log(
params[:setting_name],
!SiteSetting.public_send(params[:setting_name]),
params.setting_name,
!SiteSetting.public_send(params.setting_name),
guardian.user,
)
end

View File

@ -32,7 +32,7 @@ class Flags::CreateFlag
end
def unique_name(params:)
!Flag.custom.where(name: params[:name]).exists?
!Flag.custom.where(name: params.name).exists?
end
def instantiate_flag(params:)

View File

@ -3,6 +3,11 @@
class Flags::DestroyFlag
include Service::Base
params do
attribute :id, :integer
validates :id, presence: true
end
model :flag
policy :not_system
policy :not_used
@ -15,7 +20,7 @@ class Flags::DestroyFlag
private
def fetch_flag(params:)
Flag.find_by(id: params[:id])
Flag.find_by(id: params.id)
end
def not_system(flag:)

View File

@ -22,7 +22,7 @@ class Flags::ReorderFlag
private
def fetch_flag(params:)
Flag.find_by(id: params[:flag_id])
Flag.find_by(id: params.flag_id)
end
def invalid_access(guardian:, flag:)
@ -34,15 +34,15 @@ class Flags::ReorderFlag
end
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"
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:, params:, all_flags:)
old_position = flag.position
index = all_flags.index(flag)
target_flag = all_flags[params[: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)
@ -51,7 +51,7 @@ class Flags::ReorderFlag
def log(guardian:, flag:, params:)
StaffActionLogger.new(guardian.user).log_custom(
"move_flag",
{ flag: flag.name, direction: params[:direction] },
{ flag: flag.name, direction: params.direction },
)
end
end

View File

@ -22,7 +22,7 @@ class Flags::ToggleFlag
end
def fetch_flag(params:)
Flag.find_by(id: params[:flag_id])
Flag.find_by(id: params.flag_id)
end
def toggle(flag:)

View File

@ -32,7 +32,7 @@ class Flags::UpdateFlag
private
def fetch_flag(params:)
Flag.find_by(id: params[:id])
Flag.find_by(id: params.id)
end
def not_system(flag:)
@ -48,7 +48,7 @@ class Flags::UpdateFlag
end
def unique_name(params:)
!Flag.custom.where(name: params[:name]).where.not(id: params[:id]).exists?
!Flag.custom.where(name: params.name).where.not(id: params.id).exists?
end
def update(flag:, params:)

View File

@ -45,16 +45,16 @@ class SiteSetting::Update
end
def setting_is_visible(params:, options:)
options.allow_changing_hidden || !SiteSetting.hidden_settings.include?(params[:setting_name])
options.allow_changing_hidden || !SiteSetting.hidden_settings.include?(params.setting_name)
end
def setting_is_configurable(params:)
return true if !SiteSetting.plugins[params[:setting_name]]
return true if !SiteSetting.plugins[params.setting_name]
Discourse.plugins_by_name[SiteSetting.plugins[params[:setting_name]]].configurable?
Discourse.plugins_by_name[SiteSetting.plugins[params.setting_name]].configurable?
end
def save(params:, guardian:)
SiteSetting.set_and_log(params[:setting_name], params[:new_value], guardian.user)
SiteSetting.set_and_log(params.setting_name, params.new_value, guardian.user)
end
end

View File

@ -30,11 +30,11 @@ class User::Silence
private
def fetch_user(params:)
User.find_by(id: params[:user_id])
User.find_by(id: params.user_id)
end
def fetch_users(user:, params:)
[user, *User.where(id: params[:other_user_ids].to_a.uniq).to_a]
[user, *User.where(id: params.other_user_ids.to_a.uniq).to_a]
end
def can_silence_all_users(guardian:, users:)
@ -46,7 +46,7 @@ class User::Silence
end
def fetch_post(params:)
Post.find_by(id: params[:post_id])
Post.find_by(id: params.post_id)
end
def perform_post_action(guardian:, post:, params:)

View File

@ -30,11 +30,11 @@ class User::Suspend
private
def fetch_user(params:)
User.find_by(id: params[:user_id])
User.find_by(id: params.user_id)
end
def fetch_users(user:, params:)
[user, *User.where(id: params[:other_user_ids].to_a.uniq).to_a]
[user, *User.where(id: params.other_user_ids.to_a.uniq).to_a]
end
def can_suspend_all_users(guardian:, users:)
@ -46,7 +46,7 @@ class User::Suspend
end
def fetch_post(params:)
Post.find_by(id: params[:post_id])
Post.find_by(id: params.post_id)
end
def perform_post_action(guardian:, post:, params:)

View File

@ -17,10 +17,6 @@ class Service::ContractBase
@__options__
end
def [](key)
public_send(key)
end
def to_hash
attributes.symbolize_keys
end

View File

@ -50,7 +50,7 @@ module Chat
private
def fetch_channel(params:)
::Chat::Channel.includes(:chatable).find_by(id: params[:channel_id])
::Chat::Channel.includes(:chatable).find_by(id: params.channel_id)
end
def can_add_users_to_channel(guardian:, channel:)
@ -60,8 +60,8 @@ module Chat
def fetch_target_users(params:, channel:)
::Chat::UsersFromUsernamesAndGroupsQuery.call(
usernames: params[:usernames],
groups: params[:groups],
usernames: params.usernames,
groups: params.groups,
excluded_user_ids: channel.chatable.direct_message_users.pluck(:user_id),
)
end

View File

@ -45,7 +45,7 @@ module Chat
private
def fetch_channel(params:)
::Chat::CategoryChannel.find_by(id: params[:channel_id], auto_join_users: true)
::Chat::CategoryChannel.find_by(id: params.channel_id, auto_join_users: true)
end
def create_memberships(channel:, params:)

View File

@ -38,7 +38,7 @@ module Chat
end
def fetch_category(params:)
Category.find_by(id: params[:category_id])
Category.find_by(id: params.category_id)
end
def fetch_category_channel_ids(category:)

View File

@ -33,7 +33,7 @@ module Chat
end
def not_everyone_allowed(params:)
params[:new_allowed_groups].exclude?(Group::AUTO_GROUPS[:everyone])
params.new_allowed_groups.exclude?(Group::AUTO_GROUPS[:everyone])
end
def fetch_users(params:)
@ -46,8 +46,8 @@ module Chat
.joins(:user_chat_channel_memberships)
.distinct
.then do |users|
break users if params[:new_allowed_groups].blank?
users.where(<<~SQL, params[: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

View File

@ -56,7 +56,7 @@ module Chat
.not_staged
.includes(:group_users)
.where("NOT admin AND NOT moderator")
.where(id: params[:destroyed_group_user_ids])
.where(id: params.destroyed_group_user_ids)
.joins(:user_chat_channel_memberships)
.distinct
end

View File

@ -49,7 +49,7 @@ module Chat
end
def fetch_user(params:)
User.find_by(id: params[:user_id])
User.find_by(id: params.user_id)
end
def user_not_staff(user:)

View File

@ -66,11 +66,11 @@ module Chat
end
def fetch_category(params:)
Category.find_by(id: params[:category_id])
Category.find_by(id: params.category_id)
end
def category_channel_does_not_exist(category:, params:)
!Chat::Channel.exists?(chatable: category, name: params[:name])
!Chat::Channel.exists?(chatable: category, name: params.name)
end
def create_channel(category:, params:)

View File

@ -64,8 +64,8 @@ module Chat
def fetch_target_users(guardian:, params:)
::Chat::UsersFromUsernamesAndGroupsQuery.call(
usernames: [*params[:target_usernames], guardian.user.username],
groups: params[:target_groups],
usernames: [*params.target_usernames, guardian.user.username],
groups: params.target_groups,
)
end
@ -79,9 +79,9 @@ module Chat
def fetch_or_create_direct_message(target_users:, params:)
ids = target_users.map(&:id)
is_group = ids.size > 2 || params[:name].present?
is_group = ids.size > 2 || params.name.present?
if params[: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
@ -94,7 +94,7 @@ module Chat
end
def set_optional_name(channel:, params:)
channel.update!(params.slice(:name)) if params[:name]&.size&.positive?
channel.update!(params.slice(:name)) if params.name&.size&.positive?
end
def update_memberships(channel:, target_users:)

View File

@ -90,7 +90,7 @@ module Chat
end
def fetch_channel(params:)
Chat::Channel.find_by_id_or_slug(params[:chat_channel_id])
Chat::Channel.find_by_id_or_slug(params.chat_channel_id)
end
def enforce_membership(guardian:, channel:, options:)
@ -108,16 +108,16 @@ module Chat
end
def fetch_reply(params:)
Chat::Message.find_by(id: params[:in_reply_to_id])
Chat::Message.find_by(id: params.in_reply_to_id)
end
def ensure_reply_consistency(channel:, params:, reply:)
return true if params[:in_reply_to_id].blank?
return true if params.in_reply_to_id.blank?
reply&.chat_channel == channel
end
def fetch_thread(params:, reply:, channel:, options:)
return Chat::Thread.find_by(id: params[:thread_id]) if params[:thread_id].present?
return Chat::Thread.find_by(id: params.thread_id) if params.thread_id.present?
return unless reply
reply.thread ||
reply.build_thread(
@ -129,7 +129,7 @@ module Chat
end
def ensure_valid_thread_for_channel(thread:, params:, channel:)
return true if params[:thread_id].blank?
return true if params.thread_id.blank?
thread&.channel == channel
end
@ -140,7 +140,7 @@ module Chat
def fetch_uploads(params:, guardian:)
return [] if !SiteSetting.chat_allow_uploads
guardian.user.uploads.where(id: params[:upload_ids])
guardian.user.uploads.where(id: params.upload_ids)
end
def instantiate_message(channel:, guardian:, params:, uploads:, thread:, reply:, options:)
@ -148,10 +148,10 @@ module Chat
user: guardian.user,
last_editor: guardian.user,
in_reply_to: reply,
message: params[:message],
message: params.message,
uploads: uploads,
thread: thread,
cooked: ::Chat::Message.cook(params[: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,
)
@ -206,7 +206,7 @@ module Chat
end
def process(channel:, message_instance:, params:, thread:, options:)
::Chat::Publisher.publish_new!(channel, message_instance, params[:staged_id])
::Chat::Publisher.publish_new!(channel, message_instance, params.staged_id)
DiscourseEvent.trigger(
:chat_message_created,
@ -217,20 +217,20 @@ module Chat
thread: thread,
thread_replies_count: thread&.replies_count_cache || 0,
context: {
post_ids: params[:context_post_ids],
topic_id: params[: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: params[: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: params[:staged_id] },
{ chat_message_id: message_instance.id, staged_id: params.staged_id },
)
end
end

View File

@ -40,14 +40,11 @@ module Chat
private
def fetch_channel(params:)
::Chat::Channel.find_by(id: params[:channel_id])
::Chat::Channel.find_by(id: params.channel_id)
end
def fetch_original_message(channel:, params:)
::Chat::Message.find_by(
id: params[:original_message_id],
chat_channel_id: params[:channel_id],
)
::Chat::Message.find_by(id: params.original_message_id, chat_channel_id: params.channel_id)
end
def can_view_channel(guardian:, channel:)
@ -64,7 +61,7 @@ module Chat
end
context[:thread] = channel.threads.create(
title: params[:title],
title: params.title,
original_message: original_message,
original_message_user: original_message.user,
)

View File

@ -48,8 +48,8 @@ module Chat
def fetch_message(params:)
Chat::Message.includes(:chat_channel, :revisions).find_by(
id: params[:message_id],
chat_channel_id: params[:channel_id],
id: params.message_id,
chat_channel_id: params.channel_id,
)
end
@ -58,7 +58,7 @@ module Chat
guardian.can_flag_chat_message?(message) &&
guardian.can_flag_message_as?(
message,
params[:flag_type_id],
params.flag_type_id,
params.slice(:queue_for_review, :take_action, :is_warning),
)
end
@ -67,7 +67,7 @@ module Chat
Chat::ReviewQueue.new.flag_message(
message,
guardian,
params[:flag_type_id],
params.flag_type_id,
**params.slice(:message, :is_warning, :take_action, :queue_for_review),
)
end

View File

@ -33,7 +33,7 @@ module Chat
private
def fetch_channel(params:)
::Chat::Channel.find_by(id: params[:channel_id])
::Chat::Channel.find_by(id: params.channel_id)
end
def can_view_channel(guardian:, channel:)
@ -45,7 +45,7 @@ module Chat
.joins(:user_option)
.where(user_options: { chat_enabled: true })
.not_suspended
.where(id: params[:user_ids])
.where(id: params.user_ids)
.limit(50)
end
@ -59,7 +59,7 @@ module Chat
chat_channel_title: channel.title(invited_user),
chat_channel_slug: channel.slug,
invited_by_username: guardian.user.username,
chat_message_id: params[:message_id],
chat_message_id: params.message_id,
}.compact
invited_user.notifications.create(

View File

@ -32,7 +32,7 @@ module Chat
private
def fetch_channel(params:)
Chat::Channel.find_by(id: params[:channel_id])
Chat::Channel.find_by(id: params.channel_id)
end
def leave(channel:, guardian:)

View File

@ -57,7 +57,7 @@ module Chat
private
def fetch_channel(params:)
::Chat::Channel.includes(:chatable).find_by(id: params[:channel_id])
::Chat::Channel.includes(:chatable).find_by(id: params.channel_id)
end
def fetch_membership(channel:, guardian:)
@ -73,10 +73,10 @@ module Chat
end
def determine_target_message_id(params:, membership:)
if params[:fetch_from_last_read]
if params.fetch_from_last_read
context[:target_message_id] = membership&.last_read_message_id
else
context[:target_message_id] = params[:target_message_id]
context[:target_message_id] = params.target_message_id
end
end
@ -103,9 +103,9 @@ module Chat
guardian:,
target_message_id:,
include_thread_messages: !enabled_threads,
page_size: params[:page_size] || Chat::MessagesQuery::MAX_PAGE_SIZE,
direction: params[:direction],
target_date: params[: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]

View File

@ -51,7 +51,7 @@ module Chat
private
def fetch_thread(params:)
::Chat::Thread.strict_loading.includes(channel: :chatable).find_by(id: params[:thread_id])
::Chat::Thread.strict_loading.includes(channel: :chatable).find_by(id: params.thread_id)
end
def can_view_thread(guardian:, thread:)
@ -63,14 +63,14 @@ module Chat
end
def determine_target_message_id(params:, membership:, guardian:, thread:)
if params[:fetch_from_last_message]
if params.fetch_from_last_message
context[:target_message_id] = thread.last_message_id
elsif params[:fetch_from_first_message]
elsif params.fetch_from_first_message
context[:target_message_id] = thread.original_message_id
elsif params[:fetch_from_last_read] || !params[:target_message_id]
elsif params.fetch_from_last_read || !params.target_message_id
context[:target_message_id] = membership&.last_read_message_id
elsif params[:target_message_id]
context[:target_message_id] = params[:target_message_id]
elsif params.target_message_id
context[:target_message_id] = params.target_message_id
end
end
@ -79,7 +79,7 @@ module Chat
target_message =
::Chat::Message.with_deleted.find_by(
id: context.target_message_id,
thread_id: params[:thread_id],
thread_id: params.thread_id,
)
return false if target_message.blank?
return true if !target_message.trashed?
@ -93,11 +93,11 @@ module Chat
guardian: guardian,
target_message_id: context.target_message_id,
thread_id: thread.id,
page_size: params[:page_size] || Chat::MessagesQuery::MAX_PAGE_SIZE,
direction: params[:direction],
target_date: params[:target_date],
page_size: params.page_size || Chat::MessagesQuery::MAX_PAGE_SIZE,
direction: params.direction,
target_date: params.target_date,
include_target_message_id:
params[:fetch_from_first_message] || params[: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]

View File

@ -37,9 +37,12 @@ module Chat
only_integer: true,
},
allow_nil: true
after_validation do
self.limit = (limit || THREADS_LIMIT).to_i.clamp(1, THREADS_LIMIT)
self.offset = [offset || 0, 0].max
end
end
step :set_limit
step :set_offset
model :channel
policy :threading_enabled_for_channel
policy :can_view_channel
@ -51,16 +54,8 @@ module Chat
private
def set_limit(params:)
context[:limit] = (params[:limit] || THREADS_LIMIT).to_i.clamp(1, THREADS_LIMIT)
end
def set_offset(params:)
context[:offset] = [params[:offset] || 0, 0].max
end
def fetch_channel(params:)
::Chat::Channel.strict_loading.includes(:chatable).find_by(id: params[:channel_id])
::Chat::Channel.strict_loading.includes(:chatable).find_by(id: params.channel_id)
end
def threading_enabled_for_channel(channel:)
@ -71,7 +66,7 @@ module Chat
guardian.can_preview_chat_channel?(channel)
end
def fetch_threads(guardian:, channel:)
def fetch_threads(guardian:, channel:, params:)
::Chat::Thread
.includes(
:channel,
@ -114,8 +109,8 @@ module Chat
.order(
"CASE WHEN user_chat_thread_memberships.last_read_message_id IS NULL OR user_chat_thread_memberships.last_read_message_id < chat_threads.last_message_id THEN true ELSE false END DESC, last_message.created_at DESC",
)
.limit(context.limit)
.offset(context.offset)
.limit(params.limit)
.offset(params.offset)
end
def fetch_tracking(guardian:, threads:)
@ -137,8 +132,8 @@ module Chat
context[:participants] = ::Chat::ThreadParticipantQuery.call(thread_ids: threads.map(&:id))
end
def build_load_more_url(channel:)
load_more_params = { offset: context.offset + context.limit }.to_query
def build_load_more_url(channel:, params:)
load_more_params = { offset: params.offset + params.limit }.to_query
context[:load_more_url] = ::URI::HTTP.build(
path: "/chat/api/channels/#{channel.id}/threads",
query: load_more_params,

View File

@ -36,7 +36,7 @@ module Chat
:channel,
original_message_user: :user_status,
original_message: :chat_webhook_event,
).find_by(id: params[:thread_id], channel_id: params[:channel_id])
).find_by(id: params.thread_id, channel_id: params.channel_id)
end
def invalid_access(guardian:, thread:)

View File

@ -24,9 +24,12 @@ module Chat
params do
attribute :limit, :integer
attribute :offset, :integer
after_validation do
self.limit = (limit || THREADS_LIMIT).to_i.clamp(1, THREADS_LIMIT)
self.offset = [offset || 0, 0].max
end
end
step :set_limit
step :set_offset
model :threads
step :fetch_tracking
step :fetch_memberships
@ -35,15 +38,7 @@ module Chat
private
def set_limit(params:)
context[:limit] = (params[:limit] || THREADS_LIMIT).to_i.clamp(1, THREADS_LIMIT)
end
def set_offset(params:)
context[:offset] = [params[:offset] || 0, 0].max
end
def fetch_threads(guardian:)
def fetch_threads(guardian:, params:)
::Chat::Thread
.includes(
:channel,
@ -108,8 +103,8 @@ module Chat
.order(
"CASE WHEN user_chat_thread_memberships.last_read_message_id IS NULL OR user_chat_thread_memberships.last_read_message_id < chat_threads.last_message_id THEN true ELSE false END DESC, last_message.created_at DESC",
)
.limit(context.limit)
.offset(context.offset)
.limit(params.limit)
.offset(params.offset)
end
def fetch_tracking(guardian:, threads:)
@ -131,8 +126,8 @@ module Chat
context[:participants] = ::Chat::ThreadParticipantQuery.call(thread_ids: threads.map(&:id))
end
def build_load_more_url
load_more_params = { limit: context.limit, offset: context.offset + context.limit }.to_query
def build_load_more_url(params:)
load_more_params = { limit: params.limit, offset: params.offset + params.limit }.to_query
context[:load_more_url] = ::URI::HTTP.build(
path: "/chat/api/me/threads",

View File

@ -38,7 +38,7 @@ module Chat
private
def fetch_thread(params:)
Chat::Thread.find_by(id: params[:thread_id], channel_id: params[:channel_id])
Chat::Thread.find_by(id: params.thread_id, channel_id: params.channel_id)
end
def can_view_channel(guardian:, thread:)

View File

@ -40,7 +40,7 @@ module Chat
Chat::Message
.with_deleted
.includes(chat_channel: :chatable)
.find_by(id: params[:message_id], chat_channel_id: params[:channel_id])
.find_by(id: params.message_id, chat_channel_id: params.channel_id)
end
def invalid_access(guardian:, message:)

View File

@ -40,33 +40,33 @@ module Chat
end
def fetch_users(guardian:, params:)
return unless params[:include_users]
return unless params.include_users
return unless guardian.can_create_direct_message?
search_users(params, guardian)
end
def fetch_groups(guardian:, params:)
return unless params[:include_groups]
return unless params.include_groups
return unless guardian.can_create_direct_message?
search_groups(params, guardian)
end
def fetch_category_channels(guardian:, params:)
return unless params[:include_category_channels]
return unless params.include_category_channels
return unless SiteSetting.enable_public_channels
search_category_channels(params, guardian)
end
def fetch_direct_message_channels(guardian:, params:, users:)
return unless params[:include_direct_message_channels]
return unless params.include_direct_message_channels
return unless guardian.can_create_direct_message?
search_direct_message_channels(guardian, params, users)
end
def search_users(params, guardian)
user_search = ::UserSearch.new(params[:term], limit: SEARCH_RESULT_LIMIT)
user_search = ::UserSearch.new(params.term, limit: SEARCH_RESULT_LIMIT)
if params[:term].blank?
if params.term.blank?
user_search = user_search.scoped_users
else
user_search = user_search.search
@ -78,11 +78,11 @@ module Chat
user_search = user_search.real(allowed_bot_user_ids: allowed_bot_user_ids)
user_search = user_search.includes(:user_option)
if params[: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 = ?)",
params[:excluded_memberships_channel_id],
params.excluded_memberships_channel_id,
)
end
@ -95,7 +95,7 @@ module Chat
.includes(users: :user_option)
.where(
"groups.name ILIKE :term_like OR groups.full_name ILIKE :term_like",
term_like: "%#{params[:term]}%",
term_like: "%#{params.term}%",
)
.limit(SEARCH_RESULT_LIMIT)
end
@ -104,7 +104,7 @@ module Chat
::Chat::ChannelFetcher.secured_public_channel_search(
guardian,
status: :open,
filter: params[:term],
filter: params.term,
filter_on_category_name: false,
match_filter_on_starts_with: false,
limit: SEARCH_RESULT_LIMIT,
@ -116,13 +116,13 @@ module Chat
::Chat::ChannelFetcher.secured_direct_message_channels_search(
guardian.user.id,
guardian,
filter: params[:term],
filter: params.term,
match_filter_on_starts_with: false,
limit: SEARCH_RESULT_LIMIT,
) || []
# skip 1:1s when search returns users
if params[: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

View File

@ -29,7 +29,7 @@ module Chat
private
def fetch_message(params:)
::Chat::Message.find_by(id: params[:message_id])
::Chat::Message.find_by(id: params.message_id)
end
def enforce_membership(guardian:, message:)

View File

@ -18,7 +18,12 @@ module Chat
DELETE_CHANNEL_LOG_KEY = "chat_channel_delete"
model :channel, :fetch_channel
params do
attribute :channel_id, :integer
validates :channel_id, presence: true
end
model :channel
policy :invalid_access
transaction do
step :prevents_slug_collision
@ -30,7 +35,7 @@ module Chat
private
def fetch_channel(params:)
Chat::Channel.find_by(id: params[:channel_id])
Chat::Channel.find_by(id: params.channel_id)
end
def invalid_access(guardian:, channel:)

View File

@ -40,8 +40,8 @@ module Chat
def fetch_message(params:)
Chat::Message.includes(chat_channel: :chatable).find_by(
id: params[:message_id],
chat_channel_id: params[:channel_id],
id: params.message_id,
chat_channel_id: params.channel_id,
)
end

View File

@ -40,8 +40,8 @@ module Chat
def fetch_messages(params:)
Chat::Message.includes(chat_channel: :chatable).where(
id: params[:message_ids],
chat_channel_id: params[:channel_id],
id: params.message_ids,
chat_channel_id: params.channel_id,
)
end
@ -90,7 +90,7 @@ module Chat
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, params[:message_ids])
Chat::Publisher.publish_bulk_delete!(messages.first.chat_channel, params.message_ids)
end
end
end

View File

@ -31,7 +31,7 @@ module Chat
private
def fetch_channel(params:)
Chat::Channel.find_by(id: params[:channel_id])
Chat::Channel.find_by(id: params.channel_id)
end
def unfollow(channel:, guardian:)

View File

@ -16,28 +16,30 @@ module Chat
# @option params [String] :status
# @return [Service::Base::Context]
model :channel
params do
attribute :channel_id, :integer
attribute :status, :string
validates :channel_id, presence: true
validates :status, inclusion: { in: Chat::Channel.editable_statuses.keys }
end
model :channel
policy :check_channel_permission
step :change_status
private
def fetch_channel(params:)
Chat::Channel.find_by(id: params[:channel_id])
Chat::Channel.find_by(id: params.channel_id)
end
def check_channel_permission(guardian:, channel:, params:)
guardian.can_preview_chat_channel?(channel) &&
guardian.can_change_channel_status?(channel, params[:status].to_sym)
guardian.can_change_channel_status?(channel, params.status.to_sym)
end
def change_status(channel:, params:, guardian:)
channel.public_send("#{params[:status]}!", guardian.user)
channel.public_send("#{params.status}!", guardian.user)
end
end
end

View File

@ -79,7 +79,7 @@ module Chat
chatable: [:topic_only_relative_url, direct_message_users: [user: :user_option]],
],
user: :user_status,
).find_by(id: params[:message_id])
).find_by(id: params.message_id)
end
def fetch_membership(guardian:, message:)
@ -88,7 +88,7 @@ module Chat
def fetch_uploads(params:, guardian:)
return if !SiteSetting.chat_allow_uploads
guardian.user.uploads.where(id: params[:upload_ids])
guardian.user.uploads.where(id: params.upload_ids)
end
def can_modify_channel_message(guardian:, message:)
@ -100,11 +100,11 @@ module Chat
end
def modify_message(params:, message:, guardian:, uploads:)
message.message = params[:message]
message.message = params.message
message.last_editor_id = guardian.user.id
message.cook
return if uploads&.size != params[: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

View File

@ -36,7 +36,7 @@ module Chat
private
def fetch_thread(params:)
Chat::Thread.find_by(id: params[:thread_id])
Chat::Thread.find_by(id: params.thread_id)
end
def can_view_channel(guardian:, thread:)

View File

@ -45,7 +45,7 @@ module Chat
private
def fetch_thread(params:)
Chat::Thread.find_by(id: params[:thread_id], channel_id: params[:channel_id])
Chat::Thread.find_by(id: params.thread_id, channel_id: params.channel_id)
end
def can_view_channel(guardian:, thread:)
@ -62,7 +62,7 @@ module Chat
membership = thread.add(guardian.user)
membership.update!(last_read_message_id: thread.last_message_id)
end
membership.update!(notification_level: params[:notification_level])
membership.update!(notification_level: params.notification_level)
context[:membership] = membership
end
end

View File

@ -36,7 +36,7 @@ module Chat
private
def fetch_channel(params:)
::Chat::Channel.find_by(id: params[:channel_id])
::Chat::Channel.find_by(id: params.channel_id)
end
def fetch_membership(guardian:, channel:)
@ -48,7 +48,7 @@ module Chat
end
def fetch_message(channel:, params:)
::Chat::Message.with_deleted.find_by(chat_channel_id: channel.id, id: params[:message_id])
::Chat::Message.with_deleted.find_by(chat_channel_id: channel.id, id: params.message_id)
end
def ensure_message_id_recency(message:, membership:)

View File

@ -37,7 +37,7 @@ module Chat
private
def fetch_thread(params:)
::Chat::Thread.find_by(id: params[:thread_id], channel_id: params[:channel_id])
::Chat::Thread.find_by(id: params.thread_id, channel_id: params.channel_id)
end
def invalid_access(guardian:, thread:)
@ -50,7 +50,7 @@ module Chat
def fetch_message(params:, thread:)
::Chat::Message.with_deleted.find_by(
id: params[:message_id] || thread.last_message_id,
id: params.message_id || thread.last_message_id,
thread: thread,
chat_channel: thread.channel,
)

View File

@ -39,7 +39,7 @@ module Chat
private
def fetch_channel(params:)
Chat::Channel.find_by(id: params[:channel_id])
Chat::Channel.find_by(id: params.channel_id)
end
def can_upsert_draft(guardian:, channel:)
@ -47,26 +47,26 @@ module Chat
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])
return if params.thread_id.blank?
fail!("Thread not found") if !channel.threads.exists?(id: params.thread_id)
end
def upsert_draft(params:, guardian:)
if params[:data].present?
if params.data.present?
draft =
Chat::Draft.find_or_initialize_by(
user_id: guardian.user.id,
chat_channel_id: params[:channel_id],
thread_id: params[:thread_id],
chat_channel_id: params.channel_id,
thread_id: params.thread_id,
)
draft.data = params[: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: params[:channel_id],
thread_id: params[:thread_id],
chat_channel_id: params.channel_id,
thread_id: params.thread_id,
).destroy_all
end
end

View File

@ -30,7 +30,7 @@ RSpec.describe ::Chat::LookupChannelThreads do
let(:limit) { nil }
it "defaults to a max value" do
expect(result.limit).to eq(described_class::THREADS_LIMIT)
expect(result.params.limit).to eq(described_class::THREADS_LIMIT)
end
end
@ -44,7 +44,7 @@ RSpec.describe ::Chat::LookupChannelThreads do
let(:limit) { 0 }
it "defaults to a max value" do
expect(result.limit).to eq(1)
expect(result.params.limit).to eq(1)
end
end
end
@ -57,7 +57,7 @@ RSpec.describe ::Chat::LookupChannelThreads do
let(:offset) { nil }
it "defaults to zero" do
expect(result.offset).to eq(0)
expect(result.params.offset).to eq(0)
end
end
@ -65,7 +65,7 @@ RSpec.describe ::Chat::LookupChannelThreads do
let(:offset) { -99 }
it "defaults to a min value" do
expect(result.offset).to eq(0)
expect(result.params.offset).to eq(0)
end
end
end

View File

@ -47,10 +47,10 @@ RSpec.describe Chat::SearchChatable do
it "cleans the term" do
params[:term] = "#bob"
expect(result.params[:term]).to eq("bob")
expect(result.params.term).to eq("bob")
params[:term] = "@bob"
expect(result.params[:term]).to eq("bob")
expect(result.params.term).to eq("bob")
end
it "fetches user memberships" do

View File

@ -1,30 +1,40 @@
# frozen_string_literal: true
RSpec.describe Chat::TrashChannel do
subject(:result) { described_class.call(params:, **dependencies) }
fab!(:current_user) { Fabricate(:admin) }
fab!(:channel) { Fabricate(:chat_channel) }
let(:params) { { channel_id: } }
let(:dependencies) { { guardian: } }
let(:guardian) { Guardian.new(current_user) }
let(:channel_id) { channel.id }
context "when channel_id is not provided" do
let(:channel_id) { nil }
it { is_expected.to fail_to_find_a_model(:channel) }
describe described_class::Contract, type: :model do
it { is_expected.to validate_presence_of(:channel_id) }
end
context "when channel_id is provided" do
describe ".call" do
subject(:result) { described_class.call(params:, **dependencies) }
fab!(:current_user) { Fabricate(:admin) }
fab!(:channel) { Fabricate(:chat_channel) }
let(:params) { { channel_id: } }
let(:dependencies) { { guardian: } }
let(:guardian) { Guardian.new(current_user) }
let(:channel_id) { channel.id }
context "when data is invalid" do
let(:channel_id) { nil }
it { is_expected.to fail_a_contract }
end
context "when model is not found" do
let(:channel_id) { 0 }
it { is_expected.to fail_to_find_a_model(:channel) }
end
context "when user is not allowed to perform the action" do
let!(:current_user) { Fabricate(:user) }
it { is_expected.to fail_a_policy(:invalid_access) }
end
context "when user is allowed to perform the action" do
context "when everythings ok" do
it { is_expected.to run_successfully }
it "trashes the channel" do

View File

@ -2,6 +2,7 @@
RSpec.describe(Chat::UpdateChannelStatus) do
describe described_class::Contract, type: :model do
it { is_expected.to validate_presence_of(:channel_id) }
it do
is_expected.to validate_inclusion_of(:status).in_array(Chat::Channel.editable_statuses.keys)
end
@ -19,8 +20,8 @@ RSpec.describe(Chat::UpdateChannelStatus) do
let(:status) { "open" }
let(:channel_id) { channel.id }
context "when no channel_id is given" do
let(:channel_id) { nil }
context "when model is not found" do
let(:channel_id) { 0 }
it { is_expected.to fail_to_find_a_model(:channel) }
end

View File

@ -1,36 +1,49 @@
# frozen_string_literal: true
RSpec.describe(AdminNotices::Dismiss) do
subject(:result) { described_class.call(params:, **dependencies) }
fab!(:current_user) { Fabricate(:admin) }
fab!(:admin_notice) { Fabricate(:admin_notice, identifier: "problem.test") }
fab!(:problem_check) { Fabricate(:problem_check_tracker, identifier: "problem.test", blips: 3) }
let(:params) { { id: admin_notice.id } }
let(:dependencies) { { guardian: current_user.guardian } }
context "when user is not allowed to perform the action" do
fab!(:current_user) { Fabricate(:user) }
it { is_expected.to fail_a_policy(:invalid_access) }
describe described_class::Contract, type: :model do
it { is_expected.to validate_presence_of(:id) }
end
context "when the admin notice has already been dismissed" do
before { admin_notice.destroy! }
describe ".call" do
subject(:result) { described_class.call(params:, **dependencies) }
it { is_expected.to run_successfully }
end
fab!(:current_user) { Fabricate(:admin) }
fab!(:admin_notice) { Fabricate(:admin_notice, identifier: "problem.test") }
fab!(:problem_check) { Fabricate(:problem_check_tracker, identifier: "problem.test", blips: 3) }
context "when everything's ok" do
it { is_expected.to run_successfully }
let(:params) { { id: notice_id } }
let(:notice_id) { admin_notice.id }
let(:dependencies) { { guardian: current_user.guardian } }
it "destroys the admin notice" do
expect { result }.to change { AdminNotice.count }.from(1).to(0)
context "when user is not allowed to perform the action" do
fab!(:current_user) { Fabricate(:user) }
it { is_expected.to fail_a_policy(:invalid_access) }
end
it "resets any associated problem check" do
expect { result }.to change { problem_check.reload.blips }.from(3).to(0)
context "when data is invalid" do
let(:notice_id) { nil }
it { is_expected.to fail_a_contract }
end
context "when the admin notice has already been dismissed" do
before { admin_notice.destroy! }
it { is_expected.to run_successfully }
end
context "when everything's ok" do
it { is_expected.to run_successfully }
it "destroys the admin notice" do
expect { result }.to change { AdminNotice.count }.from(1).to(0)
end
it "resets any associated problem check" do
expect { result }.to change { problem_check.reload.blips }.from(3).to(0)
end
end
end
end

View File

@ -1,57 +1,69 @@
# frozen_string_literal: true
RSpec.describe(Flags::DestroyFlag) do
subject(:result) { described_class.call(params:, **dependencies) }
fab!(:current_user) { Fabricate(:admin) }
fab!(:flag)
let(:params) { { id: flag_id } }
let(:dependencies) { { guardian: current_user.guardian } }
let(:flag_id) { flag.id }
# DO NOT REMOVE: flags have side effects and their state will leak to
# other examples otherwise.
after { flag.destroy! }
context "when model is not found" do
let(:flag_id) { 0 }
it { is_expected.to fail_to_find_a_model(:flag) }
describe described_class::Contract, type: :model do
it { is_expected.to validate_presence_of(:id) }
end
context "when the flag is a system one" do
let(:flag) { Flag.first }
describe ".call" do
subject(:result) { described_class.call(params:, **dependencies) }
it { is_expected.to fail_a_policy(:not_system) }
end
fab!(:current_user) { Fabricate(:admin) }
fab!(:flag)
context "when the flag has been used" do
let!(:post_action) { Fabricate(:post_action, post_action_type_id: flag.id) }
let(:params) { { id: flag_id } }
let(:dependencies) { { guardian: current_user.guardian } }
let(:flag_id) { flag.id }
it { is_expected.to fail_a_policy(:not_used) }
end
# DO NOT REMOVE: flags have side effects and their state will leak to
# other examples otherwise.
after { flag.destroy! }
context "when user is not allowed to perform the action" do
fab!(:current_user) { Fabricate(:user) }
context "when data is invalid" do
let(:flag_id) { nil }
it { is_expected.to fail_a_policy(:invalid_access) }
end
context "when everything's ok" do
it { is_expected.to run_successfully }
it "destroys the flag" do
expect { result }.to change { Flag.where(id: flag).count }.by(-1)
it { is_expected.to fail_a_contract }
end
it "logs the action" do
expect { result }.to change { UserHistory.count }.by(1)
expect(UserHistory.last).to have_attributes(
custom_type: "delete_flag",
details:
"name: offtopic\ndescription: \napplies_to: [\"Post\", \"Chat::Message\"]\nenabled: true",
)
context "when model is not found" do
let(:flag_id) { 0 }
it { is_expected.to fail_to_find_a_model(:flag) }
end
context "when the flag is a system one" do
let(:flag) { Flag.first }
it { is_expected.to fail_a_policy(:not_system) }
end
context "when the flag has been used" do
let!(:post_action) { Fabricate(:post_action, post_action_type_id: flag.id) }
it { is_expected.to fail_a_policy(:not_used) }
end
context "when user is not allowed to perform the action" do
fab!(:current_user) { Fabricate(:user) }
it { is_expected.to fail_a_policy(:invalid_access) }
end
context "when everything's ok" do
it { is_expected.to run_successfully }
it "destroys the flag" do
expect { result }.to change { Flag.where(id: flag).count }.by(-1)
end
it "logs the action" do
expect { result }.to change { UserHistory.count }.by(1)
expect(UserHistory.last).to have_attributes(
custom_type: "delete_flag",
details:
"name: offtopic\ndescription: \napplies_to: [\"Post\", \"Chat::Message\"]\nenabled: true",
)
end
end
end
end