mirror of
https://github.com/discourse/discourse.git
synced 2026-08-06 13:08:40 +08:00
2708818126Previously, several chat endpoints answered policy failures with a
generic 422 `{"failed":"FAILED"}`, rendered literally as "FAILED" in the
UI: an admin creating a channel while `enable_public_channels` was
disabled had no way to know why creation failed, and a user saving a
message edit after staff closed the channel hit the same dead end. The
root cause is structural — a service policy failing without a matching
`on_failed_policy` handler silently falls through to the catch-all
`on_failure`, and near-identical actor-shaped policy names made the
handler lists look exhaustive when they weren't.
This change makes policy failures on the channel-creation and
message-edit endpoints answer with either a 403 (authorization) or a 422
carrying an actionable reason (feature or channel state), and makes that
split visible in the code:
- Creating a channel while public channels are disabled now explains the
`enable public channels` site setting instead of failing blankly.
- Editing a message in a closed/read-only channel now explains the
channel status via a new `Chat::Channel::Policy::MessageModification`
reason, mirroring the existing `MessageCreation` pattern.
- Authorization policies run before feature/state policies on both
endpoints (spec-pinned), so unauthorized users keep getting a plain 403
and are never shown state guidance they cannot act on. This flips a few
edit-endpoint failures (non-author, silenced, lost channel access) from
the opaque 422 to a proper 403.
- State policies are renamed with the channel as the grammatical subject
— `channel_allows_message_creation`,
`channel_allows_message_modification` — to distinguish them from actor
checks like `can_edit_message`; the old actor-shaped names are how the
gaps went unnoticed.
- An audit of every handler block in the chat plugin found nine handlers
naming policies or models that no longer exist. Eight were dead code
(removed or repaired to the current names); one was a live bug:
`bulk_destroy` listened for `:invalid_access` while
`Chat::TrashMessages` declares `:can_delete_all_chat_messages`, so
unauthorized bulk deletions returned the generic 422 instead of 403.
That endpoint also gains its first request specs.
Ref - t/188375
114 lines
4.5 KiB
Ruby
Vendored
114 lines
4.5 KiB
Ruby
Vendored
# frozen_string_literal: true
|
||
|
||
module ChatSDK
|
||
class Thread
|
||
# Updates the title of a specified chat thread.
|
||
#
|
||
# @param title [String] The new title for the chat thread.
|
||
# @param thread_id [Integer] The ID of the chat thread to be updated.
|
||
# @param guardian [Guardian] The guardian object representing the user's permissions.
|
||
# @return [Chat::Thread] The updated thread object with the new title.
|
||
#
|
||
# @example Updating the title of a chat thread
|
||
# ChatSDK::Thread.update_title(title: "New Thread Title", thread_id: 1, guardian: Guardian.new)
|
||
#
|
||
def self.update_title(thread_id:, guardian:, title:)
|
||
new.update(thread_id:, guardian:, title:)
|
||
end
|
||
|
||
# Retrieves messages from a specified thread.
|
||
#
|
||
# @param thread_id [Integer] The ID of the chat thread from which to fetch messages.
|
||
# @param guardian [Guardian] The guardian object representing the user's permissions.
|
||
# @return [Array<Chat::Message>] An array of message objects from the specified thread.
|
||
#
|
||
# @example Fetching messages from a thread with additional parameters
|
||
# ChatSDK::Thread.messages(thread_id: 1, guardian: Guardian.new)
|
||
#
|
||
def self.messages(...)
|
||
new.messages(...)
|
||
end
|
||
|
||
# Fetches the first messages from a specified chat thread, starting from the first available message.
|
||
#
|
||
# @param thread_id [Integer] The ID of the chat thread from which to fetch messages.
|
||
# @param guardian [Guardian] The guardian object representing the user's permissions.
|
||
# @param page_size [Integer] (optional) The number of messages to fetch, defaults to 10.
|
||
# @return [Array<Chat::Message>] An array of message objects representing the first messages in the thread.
|
||
#
|
||
# @example Fetching the first 15 messages from a thread
|
||
# ChatSDK::Thread.first_messages(thread_id: 1, guardian: Guardian.new, page_size: 15)
|
||
#
|
||
def self.first_messages(thread_id:, guardian:, page_size: 10)
|
||
new.messages(
|
||
thread_id:,
|
||
guardian:,
|
||
page_size:,
|
||
direction: "future",
|
||
fetch_from_first_message: true,
|
||
)
|
||
end
|
||
|
||
# Fetches the last messages from a specified chat thread, starting from the last available message.
|
||
#
|
||
# @param thread_id [Integer] The ID of the chat thread from which to fetch messages.
|
||
# @param guardian [Guardian] The guardian object representing the user's permissions.
|
||
# @param page_size [Integer] (optional) The number of messages to fetch, defaults to 10.
|
||
# @return [Array<Chat::Message>] An array of message objects representing the last messages in the thread.
|
||
#
|
||
# @example Fetching the last 20 messages from a thread
|
||
# ChatSDK::Thread.last_messages(thread_id: 2, guardian: Guardian.new, page_size: 20)
|
||
#
|
||
def self.last_messages(thread_id:, guardian:, page_size: 10)
|
||
new.messages(
|
||
thread_id:,
|
||
guardian:,
|
||
page_size:,
|
||
direction: "past",
|
||
fetch_from_last_message: true,
|
||
)
|
||
end
|
||
|
||
def self.update(...)
|
||
new.update(...)
|
||
end
|
||
|
||
def messages(thread_id:, guardian:, channel_id: nil, direction: "future", **params)
|
||
channel_id ||= ::Chat::Thread.where(id: thread_id).pick(:channel_id)
|
||
Chat::ListChannelThreadMessages.call(
|
||
guardian:,
|
||
params: {
|
||
thread_id:,
|
||
channel_id:,
|
||
direction:,
|
||
**params,
|
||
},
|
||
) do
|
||
on_success { |messages:| messages }
|
||
on_failed_contract { |contract| raise contract.errors.full_messages.join(", ") }
|
||
on_failed_policy(:can_view_thread) { raise "Guardian can't view thread" }
|
||
on_failed_policy(:threading_enabled_for_channel) do
|
||
raise "Threading is not enabled for this channel"
|
||
end
|
||
on_failed_policy(:target_message_exists) { raise "Target message doesn't exist" }
|
||
on_failure { raise "Unexpected error" }
|
||
end
|
||
end
|
||
|
||
def update(guardian:, **params)
|
||
Chat::UpdateThread.call(guardian:, params:) do
|
||
on_model_not_found(:thread) do
|
||
raise "Couldn’t find thread with id: `#{params[:thread_id]}`"
|
||
end
|
||
on_failed_policy(:can_view_channel) { raise "Guardian can't view channel" }
|
||
on_failed_policy(:can_edit_thread) { raise "Guardian can't edit thread" }
|
||
on_failed_policy(:threading_enabled_for_channel) do
|
||
raise "Threading is not enabled for this channel"
|
||
end
|
||
on_failed_contract { |contract| raise contract.errors.full_messages.join(", ") }
|
||
on_success { |thread:| thread }
|
||
on_failure { raise "Unexpected error" }
|
||
end
|
||
end
|
||
end
|
||
end
|
