mirror of
https://github.com/discourse/discourse.git
synced 2026-08-06 07:23:30 +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
|
||
|---|---|---|
| .. | ||
| components/chat | ||
| db/post_migrate | ||
| fabricators | ||
| integration | ||
| jobs | ||
| lib | ||
| mailers | ||
| models | ||
| queries/chat | ||
| requests | ||
| serializer | ||
| services | ||
| support | ||
| system | ||
| validators | ||
| plugin_helper.rb | ||
| plugin_spec.rb | ||
