`bin/lint` had several ways to print "All lints passed" without a linter
ever running. That is worse than failing: it tells you your code is
clean
when nothing looked at it.
Crashes and no-ops:
- `Set#exclude?` is an ActiveSupport method, and this script loads only
optparse, open3, pathname and shellwords. Every path under `plugins/`
raised `NoMethodError`, so no plugin file could be linted at all.
- `lib/` contains a `plugin.rb`, so it was mistaken for an external
plugin. `--recent` ran `bundle install` inside `lib/` and linted 135
files with the wrong configuration.
- The script never moved to the repository root, so any invocation from
a
subdirectory resolved paths against the wrong base and linted nothing.
- A mistyped path was dropped in silence. Bad paths are now reported,
and
the remaining valid paths are still linted.
- An empty result set claimed success. It now says "Nothing was linted".
- A directory argument built an argv larger than `ARG_MAX`. `system`
returns nil rather than raising there, which was recorded as a lint
failure with no explanation. The `--file` arguments are now batched.
Wrong file sets:
- `--recent` intersected the last 50 commits with tracked files, so an
uncommitted change to a file nobody had committed recently was never
linted. It now includes the working tree.
- Directory expansion walked the filesystem, so build output that git
ignores was linted and files deleted from the working tree were handed
to the linters. It now asks git, and falls back to a walk for
directories git ignores, such as unbundled plugins.
- `lintable_file?` compared substrings, so the `.git` test excluded all
of
`.github/`, and any path containing `tmp` or `vendor` anywhere was
skipped. It now compares whole path segments.
Unwanted side effects:
- Checking an external plugin ran `bundle install` and `pnpm i` even
without `--fix`, which can rewrite `Gemfile.lock` and `pnpm-lock.yaml`.
Both installs are now frozen unless `--fix` is given, so a read-only
check stays read-only.
- A failed dependency install called `abort`, discarding the results
already collected for other plugins. It is now recorded as a failure.
Finally, `lefthook.yml` routes everything under `bin/` to the Ruby
linters, but the exclude list was missing `bin/dev` (node) and
`bin/system_rspec` (bash), so `syntax_tree` tried to parse them as Ruby.
---------
Co-authored-by: Sam Saffron <sam.saffron@gmail.com>
`NodeType` exposed six of its filter helpers twice: a class method holding
the implementation, and a private instance method whose entire body
forwarded to it. Three of those pairs existed for no reason - nothing in
either plugin called `matches_category_ids?`, `matches_user_groups?` or
`matches_reviewable_types?` at class level.
The pairs are not pointless in general. `stale_topic` filters from
`self.trigger_data_for` rather than from an instance, so
`category_ids_parameter`, `normalize_tag_names`, `normalize_category_ids`
and `expand_subcategory_ids` genuinely need a class form and keep it. The
rule is "a class method when a class method needs it", and these three were
the exceptions to it.
Their spec reached for the class form because that was the only public way
in; it now exercises them through a node, which is how nodes use them.
The node types controller renders what the list service returns, with no
transformation of its own, so its request spec had grown into a second copy
of the service spec expressed in JSON: the same property schemas, the same
UI controls, the same operation lists, asserted twice. Touching a node
schema meant updating both.
The request spec now covers what only it can - the admin constraint, and
that the endpoint renders - plus the one case that genuinely belongs at
this layer, where metadata must not be preloaded for options that depend on
node parameters.
A few examples in the service spec asserted facts that neighbouring
examples in the same file already covered, and are gone too.
The post action resolved all thirty-odd of its parameters for every input
item, whatever it had been asked to do. A delete reads two of them; a
create reads six. The other twenty-five went through the parameter resolver
- and any expression the user had left in a field belonging to a different
operation got evaluated too - only to be thrown away.
Resolving on first read instead keeps the cost proportional to the
operation, and means adding an operation no longer means adding a line to a
hash that already listed every field on the node.
Nine trigger nodes declared the same `category_ids`, `include_subcategories`
and `tag_names` properties, byte for byte, across two plugins. Adding a UI
control, changing a default or introducing a new filter meant nine edits and
a good chance of drift.
The behaviour behind these properties already lives on `NodeType`, so the
declarations belong there too. Nodes splat in the sets they need, which
keeps each node's own property order - and therefore the order of the fields
in the editor - unchanged.
The per-node translations are untouched: they key off each node's i18n
scope rather than the property hash, so nodes keep describing their own
filters in their own words.
The identically named property on the topic and tag group actions is a
different thing - a list of tags to apply, not to filter by - and is left
alone.
Workflows could react to a post being created, edited or moved, but not to
one being deleted or restored. Anything that mirrors or syncs posts
elsewhere could therefore only ever accumulate: it had no way to learn that
the source had gone away, and no way to act on it if it did.
This adds `trigger:post_destroyed` and `trigger:post_recovered`, and the
matching `delete` and `recover` operations on the post action so a workflow
can complete the loop. Both events already existed in core and were
unclaimed.
The two triggers are the same node twice over, so their shared behaviour
lives in a `PostLifecycle` mixin. It cannot be a base class: `NodeType`
registers every subclass, so an abstract parent would register itself and
fail on `identifier`.
Notes on the edges:
- The acting user in the payload is whoever deleted or restored the post,
not its author, which is the opposite of the create and edit triggers.
- Both triggers default to skipping personal messages, so a workflow
cannot forward private content by accident.
- Deleting through the action always really deletes. Without that,
PostDestroyer leaves an author's own post as a stub and the workflow
reports success having changed nothing.
- Editing now always records its own revision. An edit by a different user
already forced one, so this only changes the case where a workflow edits
a post it just wrote, where collapsing the two into one revision loses
the audit trail.
An error workflow was told which workflow failed, where it failed and why,
but not what it was working on. That is enough to announce a failure and
not enough to do anything about it: the post, topic or user that the
original run was handed is exactly what a retry, a dead-letter record or a
targeted notification needs.
Passing the failed run's trigger data through makes those patterns
expressible. It is omitted when the failing run was itself an error
workflow, so a chain of error workflows cannot nest the payload on every
hop.
Deleting the first post of a topic takes the topic down with it, and
`Topic` is trashable, so `post.topic` then resolves to nil under the
default scope. Every topic-derived field a workflow reads off a post -
title, slug, category and tags - came back nil, and `post_url` fell
through `Post#url`'s nil-topic branch and became "/404".
That already affected `action:post` reading a post in a deleted topic, and
becomes far more visible with triggers that fire while a topic is on its
way out.
Resolve the topic through `Topic.with_deleted` for staff scopes, which is
what core's own post serializer does, and build the url from the resolved
topic instead of delegating.
Every trigger that filters on a topic carried its own copy of the same
matching logic. `matches_tags?` and `topic_tag_names` existed seven times,
`matches_topic_type?` three times and `matches_group_inbox?` twice - once
per node, plus another copy in discourse-topic-voting. They only differed
in whether the topic arrived as an argument or was read from an ivar.
`NodeType` already hosts `matches_category_ids?`, `matches_user_groups?`
and `normalize_tag_names` for exactly this reason, so these belong beside
them. Nodes now pass their topic explicitly, which is what the majority of
the copies already did, and is unambiguous for `post_moved` where two
topics are in play.
`topic_tag_names` memoises per topic id rather than in a single ivar, so a
node that inspects more than one topic cannot silently read another
topic's tags.
Posts created on behalf of a user by automation - a plugin, a scheduled
job, or a workflow acting as the person who triggered it - are subject to
the same per-user rate limits as someone typing in the composer. When the
automation runs in response to that user's own post, it reliably trips
`rate_limit_create_post` and the post is silently dropped. Only staff are
exempt, so trust level 4 members hit this too.
Until now the only way out was `skip_validations`, which is far broader:
it also skips content validation, the host spam check, the suspended user
check and the review queue. Callers that only need the rate limit lifted
had to give up all of that.
`skip_rate_limits` lifts exactly the rate limit and nothing else, for both
post and topic creation.
Previously, `site_contact_group_name` was written as a group name by the
About config page but as a group id by the all-settings page, which grew
a group picker in #40981 — and `GroupSettingValidator` was relaxed in
the
same commit to accept either, so the mismatched value saved cleanly and
the row still displayed the right group. `SystemMessage` looks the value
up by name, so an admin who set the contact group from All site settings
silently stopped it being invited to automated personal messages, and
the
About page then showed no group selected at all.
This change makes the id the stored format everywhere, because a group
name can be renamed or localized while an id cannot. `TypeSupervisor`
converts a name to its id on write, so console, API and plugin callers
that pass a name keep working; the validator now accepts ids only; and a
post-deploy migration converts the values sites have already stored,
post-deploy because the old code can only resolve a name. The About page
writes and reads the id, and `SystemMessage` resolves it through a new
`Discourse.site_contact_group`, which still accepts a name for sites
that
configure this through a `DISCOURSE_SITE_CONTACT_GROUP_NAME` global
override, since no migration can reach those. Both conversions go
through
`Group.find_by_id_or_name`.
The lookups guard on a digits-only match rather than casting, because
Rails turns `Group.find_by(id: "0support")` into `WHERE id = 0` — the
everyone group.
Two behaviour changes are worth calling out. Renaming the contact group
no longer breaks the setting; a spec asserted the opposite, which was
the
bug written down as intent. And the migration matches names
case-insensitively, so a site that stored `Staff` for a group named
`staff` — which `Group.exists?(name:)` never matched — starts inviting
that group again.
`site_settings.errors.invalid_group` now says "There's no such group",
since it fires for an unknown name as well as an unknown id and is
shared
with `AtLeastOneGroupValidator`, which has only ever validated ids.
Finally, the setting keeps its name. Renaming it would change a key that
self-hosters set through the environment and that third parties read,
and
the deprecated-settings alias does not cover global overrides, so that
belongs in its own change rather than in a fix.
Previously, the admin plugins page only exposed an outlet below the
list, so a plugin had nowhere to put a notice admins see before
scrolling through every installed plugin.
This change adds a matching `admin-above-plugins-index` outlet between
the page header and the filter controls, with the same `model` outlet
argument.
Previously, the events plugin had no way to drive a workflow from an
RSVP or from an event finishing, and a trigger node contributed by any
plugin was added to the registry but never subscribed to its
`DiscourseEvent` — so it saved fine, showed up in the palette, and
silently never fired.
This change wires the subscription into node registration, which drops
the hand-rolled `on(...)` workarounds in assign, topic voting and chat,
and adds `trigger:event_participation_changed` and
`trigger:event_ended`, both scopable to a single topic.
---
Split into two commits, since the first touches shared infrastructure:
- **`FIX: Subscribe trigger events for workflow nodes added by
plugins`** — claiming a node now registers *and* subscribes it, and
contributing plugins are flushed before the host claims its own, so
ownership lands on them. Because the subscription goes through
`Plugin::Instance#on`, a node stops listening while *its own* plugin is
disabled rather than following the host's setting. This also drops a
duplicate registration that let `trigger:chat_message_created` show in
the palette while chat was disabled.
- **`FEATURE: Workflow triggers for event participation and event end`**
— the two triggers. No `on(...)` wiring needed in the events plugin
thanks to the commit above.
Three behaviour changes in the events plugin, each deliberate:
- Withdrawing an RSVP destroyed the record without publishing anything,
so attendance state built from these triggers could never recover from
someone leaving. Removal now publishes, and reads as `status: null` with
`removed: true`. The livestream chat sync skips removals, so its
follow/unfollow behaviour is unchanged.
- The ended occurrence travels with `:discourse_post_event_event_ended`.
`set_next_date` moves the event on immediately afterwards, and
`Event#starts_at` returns `nil` once a bounded series is past
`recurrence_until`, so the event alone cannot say which occurrence
ended.
- Re-submitting an unchanged RSVP still publishes, so the trigger
ignores it rather than running a workflow twice for one decision.
Payload is `{event, post, topic, stats}`, plus `{user, participation}`
on the participation trigger — modelled on
`WebHook.build_calendar_event_payload` rather than `EventSerializer`,
which goes admin-truthy under a system guardian. No JS, no core changes,
no new site settings.
### Known gaps, documented rather than fixed
Bulk invite, `Event#create_invitees` (`insert_all!`),
`reset_invitee_notifications` (`update_all`),
`enforce_private_invitees!` (`delete_all`) and event/user destroy stay
silent. `create_attendance!` swallows `RecordNotUnique`, so a concurrent
first RSVP fires nothing. `event_ended` can re-fire if an edit resets
`finished_at`, and is missed when an event is closed early:
`EventDate.pending` merges `Event.open`, so the in-flight occurrence
leaves the job's scope.
`EventListener` does not rescue `new`/`valid?`/`matches?` and
`DiscourseEvent.trigger` re-raises, so a raising subscriber aborts
`MonitorEventDates` mid-`find_each`. Adding `continue_on_error:` there
breaks existing `track_events` assertions, so it is left for its own
commit.
### Testing
Full `discourse-workflows` and `discourse-events` backend suites pass
(4005 examples), plus the workflow specs in chat, assign and topic
voting. Verified at runtime that all 22 trigger nodes have exactly one
subscription and there are no duplicate registrations, and that both new
nodes disappear from the palette and stop dispatching when
`discourse_events_enabled` is off.
Meta:
https://meta.discourse.org/t/discourse-calendar-events-webhook-triggers-automations-plugin/409623
---------
Co-authored-by: Joffrey JAFFEUX <j.jaffeux@gmail.com>
Every reason an upload never reaches the model -- unsupported format,
failed conversion, missing file -- only ever went to the Rails log, so a
caller that wanted to tell someone their attachment was ignored had no
way to find out.
Encoding now appends those reasons to an optional collector, which the
LLM takes from the execution context, so anything running a completion
can read them back after the fact. Nothing changes for callers that do
not pass one.
Stacked on #43011.
Filtering ran a guardian check per upload, and each one lazily loaded
that upload's access control post, so a post with a dozen secure images
cost a dozen extra queries. Preload them instead.
The preload has to happen inside `Post.unscoped`, because `Upload` only
unscopes in its own association reader: preloading outside it would
resolve a deleted post to nil and hand the model an upload the guardian
would otherwise have hidden. There's a regression test for that.
Stacked on #43010.
Reported on [AI triage supporting check of user
profiles](https://dev.discourse.org/t/144011/9): an `.avif` avatar never
reached the model and no review was generated, silently.
Four copies of the supported image extension list had drifted apart, so
what counted as an image depended on which one you hit. They are now one
list on `UploadEncoder`, which also lets us add AVIF: everything that is
not JPEG is already transcoded to PNG before it leaves us, so nothing
downstream has to learn the format, and provider-by-provider format
support stays irrelevant.
An image in a format we cannot transcode used to be treated as a
document, where it either got base64'd as an opaque blob or was dropped
citing attachment types. It is now recognised as an image and skipped
with a reason naming the formats we accept. The other silent image drops
-- unknown dimensions, failed transcode, missing file -- say so in the
logs too.
Stacked on #43009.
`.html` uploads already resolved to an attachment type, but nothing
could convert them, so they fell through to the raw path, which only
accepts PDFs, and were dropped. Route them through `HtmlToMarkdown` so
the model sees the readable content instead of nothing.
Stacked on #43008.
`UploadEncoder` had grown into 400 lines where the image path was a
tenth of the file and every new document format meant another
near-identical `*_to_text_payload` method plus another branch in a long
if/elsif chain.
Documents now live in `DocumentEncoder`, which drives the converters off
a table keyed by attachment type, and each converter gets its text
normalization from a shared `TextNormalization` module instead of
redefining `force_utf8` and friends.
Behaviour is unchanged; the extraction limit the converters cap at is
now one constant instead of a `100_001` literal repeated in six files.
Stacked on #43007.
`Upload.find` raises when the row is gone, which made the `blank?` guard
right below it dead code and turned a stale upload id into a 500 for the
whole completion. Loading the batch up front also drops the query per
upload.
Stacked on #43006.
Previously, `Chat::ChannelFetcher.structured` capped the sidebar payload
at 100 followed public channels (ordered alphabetically) and 75 direct
message channels (ordered by activity), so a starred channel past either
cutoff never reached the client and vanished from the Starred section.
This change fetches starred channels with a dedicated query and merges
them into the result, running it only when the base list actually hit
its limit so the extra queries cost nothing for the vast majority of
users.
Meta:
https://meta.discourse.org/t/starred-chats-disappear-if-user-is-in-more-than-100-chats/411040
`TopicGuardian#can_recover_topic?` asked `can_recover_post?` about
`topic.ordered_posts.first`, but `ordered_posts` is trashable-scoped, so
once the first post is deleted that query can never return it — it
returns the earliest *surviving* post instead.
`TopicsController#recover` meanwhile acts on
`posts.with_deleted.order(:post_number).first`, the real first post. The
guardian was answering a question about one record while the controller
acted on another.
### Reproduction
On a default install:
1. Alice (TL1) creates a topic, replies to it, and deletes her own
reply.
2. A moderator deletes the topic.
3. Alice sends `PUT /t/:id/recover.json` → **200**.
Her self-deleted reply is `user_deleted` with `deleted_at` unset, which
is exactly the shape `can_recover_post?` approves — so the guardian
approved recovering the topic on the strength of a reply.
### Why the state it leaves behind is worse than the permission slip
`user_recovered` bails out on a post staff trashed, since `user_deleted`
is false. `@topic.recover!` ran anyway, keyed on which branch was
*selected* rather than on whether it did anything. The topic came back
listed with no opening post, and Alice could not repair it —
`can_recover_post?` on the real first post is false for her.
That state is also load-bearing elsewhere: `Jobs::DeleteTopic`,
`DestroyTask` and `PostMover` all reach for `ordered_posts.first` and
hand the result to `PostDestroyer`, so on an affected topic they delete
a reply and leave the topic alive, reporting success. Those call sites
are left for a follow-up — `PostDestroyer#destroy` is not idempotent on
an already-trashed post, so converting them needs its own change.
### The guard
Pointing the guardian at the real first post would, on its own, hand TL4
a recover button for an opening post a moderator deleted on a live topic
— `can_recover_post?` grants that through `can_moderate_topic?`, and
`details.can_recover` flips with it. `posts#recover` refuses that same
request through `ensure_can_see!` → `can_see_deleted_post?`, so the
guard keeps both routes to one answer. A TL4 recovering a post they
trashed themselves stays allowed, exactly as `posts#recover` already
allowed.
`first_post_with_deleted` reads `post_number: 1` rather than the lowest
surviving number, so it agrees with `Post#is_first_post?` and cannot
substitute a reply the way the code it replaces did.
### No backfill
A live topic whose first post is trashed is also a legitimate state — a
moderator deleting the opening post of a live multi-post topic produces
it. Nothing can tell that apart from the corruption, so existing rows
are left alone.
Previously, pipes inside complete Markdown links and images were treated
as table column separators before inline parsing, corrupting rows and
rich-editor round trips.
This change protects those pipes during block parsing and escapes
serialized table cells, preserving hand-authored and generated Markdown.
`CategoriesController#index` built and serialized a topic list on every
request, including crawler ones. The crawler layout renders categories
only and never emits preloaded data, so that work was always discarded.
`/categories` and `/c/*path/subcategories` both route here, and both are
heavily crawled — every one of those hits paid for a `TopicQuery` plus a
`MultiJson.dump(TopicListSerializer)` that nothing read.
### Why it is safe
`@topic_list` is assigned only inside `fetch_topic_list` and read only
by the preload that this change guards. No view touches it —
`app/views/categories/index.html.erb` renders
`@category_list.categories`, and that is also what the application
layout's noscript `yield` renders. `#data-preloaded` is emitted from
`app/views/layouts/application.html.erb` only, never from the crawler
layout.
`use_crawler_layout?` also covers `?print` and the browser-update path.
Both render the crawler layout, so skipping is correct there too.
### Why this stops at crawlers
The same waste happens on narrow viewports — `MobileCategoryPageStyle`
has no latest/top style, so the client discards the list there too
(`PreloadStore.remove("topic_list")` in `discovery/categories.js`). That
one is not safe to fix the same way: the server only has UA detection,
while `site.mobileView` is a live viewport check, so a narrow desktop
window disagrees with it. Skipping the preload on a UA guess would trade
guaranteed-correct wasted server work for an extra client request on
real users, which is what e7a84948b9 set out to remove.
### Testing
New request spec asserts `fetch_topic_list` is never invoked for a
crawler request. Observed failing before the change (`unexpected
invocation: CategoriesController.fetch_topic_list`). The existing
"properly preloads topic list" spec covers the unchanged non-crawler
path.
> Stacked on #42729.
A moderator reviewing a flagged profile sees the account's *current*
avatar, which is no longer the one anybody objected to once it has been
removed. With #42724 giving staff a Remove avatar action and #42726
letting a workflow reset one automatically, a reviewable can now
describe an image that exists nowhere on screen.
Snapshots the avatar into the reviewable payload, alongside the name,
email, bio and website already captured there, and renders it.
### Notes
**One payload builder instead of three.** `Jobs::CreateUserReviewable`,
`Jobs::EnqueueSuspectUsers` and the workflows flag node each built this
hash independently, and had already drifted — the signup path omitted
`bio`, which the review UI renders. Adding a field meant editing all
three. They now share `ReviewableUser.payload_for`, so signup
reviewables gain the `bio` the UI was already trying to show.
**The upload url, not the avatar template.** `avatar_template` resolves
through `uploaded_avatar_id` at request time, so it would stop pointing
at the flagged image the moment the avatar is removed — exactly when the
snapshot has to work.
**The snapshot is referenced, not just recorded.** Once
`uploaded_avatar_id` and `custom_upload_id` are nulled the upload has no
`UploadReference` and no non-post relation, which makes it eligible for
`Jobs::CleanUpUploads` after the grace period — the evidence would
disappear an hour after remediation. An `after_create` on the reviewable
references the upload, so cleanup spares it. Covered by a spec that
removes the avatar and then runs the cleanup job.
**Scrubbing.** `ReviewableUser#scrub` already replaces the whole
payload, so the url goes with it.
> Stacked on #42726.
#42723 added a spec pinning one direction of schema drift, for the user
constants only. This extends it to the rest, and to the direction it
deliberately skipped.
Two things drift, and the second is the interesting one:
- A property **declared but never emitted** becomes an expression the
picker offers and that resolves to nil at runtime. `TOPIC_PROPERTIES`
documents both topic serializers, but only the full one has
`first_post_id`, so nodes backed by `TopicListItemSerializer` offer
exactly that dead path today.
- A field **emitted but never declared** reaches item data without
surfacing anywhere an admin can see it. `WebHookGroupSerializer`
currently carries `incoming_email`, `bio_raw`, `can_admin_group`,
`has_messages` and `display_name` into workflow items undeclared, all
guardian-dependent. It is a core serializer this plugin does not own, so
core can widen that surface without anyone here noticing.
Neither is automatically wrong — schemas are curated views,
`TOPIC_PROPERTIES` hides around twenty fields on purpose — so this
records the differences rather than forbidding them.
### Why subset checks rather than exact matching
Plenty of these fields are `include_*?`-gated and only appear under
conditions the spec does not set up: `excerpt`, `display_name`,
`can_edit_group`. Exact matching made the spec depend on whether a
fabricated record happened to trigger them. Each check is now a subset
test — a difference already recorded passes either way, anything new
fails.
Payloads are built with the system user's guardian, so this pins the
widest surface a workflow can see rather than what an anonymous scope
happens to expose.
Also corrects one source attribution: `BASIC_GROUP_PROPERTIES` documents
the hash the membership triggers build by hand, not
`BasicGroupSerializer`. Comparing it against the serializer suggested 25
undeclared fields that were not real.
Mutation-tested by adding an attribute to
`DiscourseWorkflows::UserSerializer` and confirming the new direction
fails.
> Stacked on #42725.
A workflow that decided a profile was a problem could only flag the user
and wait, even when the offending part was a single field. The user
node's edit operation could set the bio and title, and nothing else on
the profile.
### Changes
The edit operation gains:
- `website` and `location` — already supported by `UserUpdater`, just
not exposed
- `remove_profile_background` and `remove_card_background` — map to the
blank url `UserUpdater` already treats as "clear it"
- `remove_avatar` — goes through the same `User#reset_avatar!` as the
review queue action, so the image cannot be re-selected from the picker,
and records a staff action because the actor is configurable
Booleans are cast rather than tested for truthiness, so an expression
resolving to the string `"false"` does not remove someone's avatar.
### Note for workflow authors
Removing an avatar itself triggers `:user_updated`, so a workflow that
both scans and remediates should branch on the avatar being present.
That check is free — an absent avatar has nothing to scan — but it is
the difference between a workflow that terminates and one that does not.
This completes the loop the earlier PRs in the stack open up: read the
profile images, decide, then either flag for a human or fix the field
directly.
> Stacked on #42724.
The user updated trigger fired for every profile save with no indication
of what changed. A workflow scanning avatars therefore also ran on every
notification preference save, and could only tell the difference after
paying for the job and a lookup.
### Changes
`:user_updated` now carries the columns that persisted. Every call site
reports:
| call site | reports |
|---|---|
| `User#trigger_user_updated_event` | `uploaded_avatar_id` (the callback
already fires only for avatars) |
| `UserUpdater` | what actually persisted on the user and its profile |
| `EmailUpdater`, `UsersController#update_primary_email`,
`#destroy_email` | `email` |
| `Jobs::UpdateUsername` | `username` |
The trigger node gains a **Changed fields** filter. It is applied while
matching, before anything is enqueued, so an irrelevant save costs
nothing rather than being filtered downstream by an `If` node after the
job has already run.
Columns are an implementation detail and a poor vocabulary for a filter
— a bio edit persists `bio_raw`, `bio_cooked` and `bio_cooked_version` —
so the node folds them into the names admins reason about: avatar, bio,
name, username, email, website, location, title, profile images. The
mapping lives in the node rather than in core, so core does not learn
the plugin's naming.
### Compatibility
Additive. `DiscourseEvent.trigger` calls stored blocks, and a non-lambda
`Proc` drops surplus arguments, so existing one-argument listeners are
unaffected. There is one other listener in the tree
(`plugins/automation`), and it is a one-argument block.
Call sites that do not report what changed pass nil, which matches
everything. An unreported change is better handled twice than missed.
Previously, RSS items were generally treated as raw HTML and “Show full
post” scraped the linked page, so plain-text, image-only, excerpted, and
expanded content could render inconsistently or fail silently.
This change infers rendering and truncation from each feed item, expands
cached feed content instead of an unrelated page scrape, and surfaces
expansion failures while preserving legacy behavior.
Previously, the client used whatever topic list the server had preloaded
without checking it was the list it asked for, so any server/client
disagreement about the filter silently rendered the wrong list on a full
page load — and only on a full page load.
This change compares the preloaded list's `filter` against the requested
one and refetches when they differ, and clamps a category's
`default_view` on the client to the filters the visitor could actually
request.
---
Follow-up to #42736, which fixed the one instance of this we knew about.
This closes the underlying seam so the next one self-heals.
### Why the check is needed
`TopicList#preload_key` has been the constant `"topic_list"` since
e7a84948b9, and `TopicListAdapter#find` consumes that key regardless of
the filter it requested, then stamps the requested filter onto the
payload. Before that commit the key included the filter, so a mismatch
missed the store and triggered a real request — that was the safety net.
Since then the same failure mode has come up twice: `default_list_filter
= "none"` (patched narrowly in b5721b7b4f, whose message notes *"it was
still using the preloaded data for the old route. This has been
happening since e7a84948"*) and `default_view` (#42736). Both were
diagnosed from scratch. With #42736's `topic_list.filter` on the wire,
the client can now just notice.
The check fails open when either side can't be identified, so an older
server payload without `filter` is still used.
### Why `filterTypeForMode` isn't reused
`serverFilterForMode` looks like a duplicate of `filterTypeForMode`
(`split("/").pop()`) but isn't a substitute for it: for
`tags/intersection/<a>/<b>`, `topics/created-by/<user>` and private
message lists the last segment is a *name*, not a filter, so comparing
it would discard a perfectly good preload and issue a needless request
on every load. The new helper only answers when the mode is a bare
filter or contains `/l/<filter>`, and returns `undefined` otherwise. It
also handles a category whose slug is itself `l` (`c/l/1/l/latest`).
### The client-side `default_view` clamp
This is not optional polish. `build-category-route.js` resolved
`default_view` with no allow-list at all, so an anonymous visitor to a
category with `default_view = "unread"` requested `/l/unread` and got a
**403 and a blank page** on in-app navigation. That is reproducible on
`main` today; the preload was masking it on direct loads. Adding the
filter check without the clamp would have turned a working (if
mislabelled) direct load into a blank one too.
The clamp mirrors `ListController#category_default_view` and reads a new
`Site#anonymous_list_filters`. Both now go through
`Discourse.anonymous_list_filters`, so the rule has one owner.
### Testing
New unit tests cover the normaliser, including every shape that must
fail open. New acceptance tests cover match / mismatch /
payload-without-filter, and the clamp in both directions (anonymous
falls back to `latest`, logged in keeps `unread`).
Manually, on a dev instance: forcing `category_default_view` to return
`"latest"` makes a direct load of a `default_view = "votes"` category
refetch `/l/votes.json` and render correctly, where it previously
rendered activity order with no request at all. With the server
behaving, a cold load of every list route — homepage, latest, top, hot,
categories, category, category `/none`, tag, tag intersection,
tag-in-category, filter, unread, new, bookmarks — issues no extra
request.
`site_response.json` is updated because it is `additionalProperties:
false`.
The profile and card background uploads were reconsidered on **every**
`UserUpdater` call, whether or not the caller mentioned them:
```ruby
if attributes[:profile_background_upload_url] == "" ||
!guardian.can_upload_profile_header?(user)
user_profile.profile_background_upload_id = nil
```
With the key absent the first test is false, so the permission check
decides — and when it fails the upload is discarded. An update that only
touched a bio could take the backgrounds with it.
### Reproduced
| scenario | before | after |
|---|---|---|
| permitted user updates their own bio | kept | kept |
| user has since dropped out of `profile_background_allowed_groups`,
updates their bio | **cleared** | kept |
| non-staff actor updates another user's bio | **both cleared** | kept |
The third is the one that bites without any setting change:
`can_upload_profile_header?` is `(is_me? && in allowed groups) ||
is_staff?`, so it is false for *any* other non-staff actor, and each
background has its own independent guard.
### Fix
Both blocks are now entered only when the caller actually submitted the
field. `permit` drops keys that were not sent, so the controller path is
unaffected for real submissions.
Submitting a background the actor may not upload still clears it. That
is the existing behaviour for a request that asks, and changing it is a
separate judgement call.
Found while checking whether the workflows user node could clear a
background unintentionally. It could not — that node defaults its actor
to `system`, which is staff — so this is unrelated to that work and
branches off `main`.
> Stacked on #42723.
Reviewing a user flagged for an inappropriate profile picture offered
only **Approve** and **Delete user**. Deleting the account is the wrong
remedy when the image is the only problem, so in practice the image
could only be dealt with by editing the user out of band, and the
reviewable stayed open meanwhile.
### Changes
- A **Remove avatar** action on pending user reviewables, shown only
while the user still has one. It deliberately leaves the reviewable
pending: removing the image is remediation, not a verdict on the
account.
- `User#reset_avatar!`. Unlike picking the system avatar, this also
drops `custom_upload_id` and `gravatar_upload_id`, so the removed image
cannot simply be re-selected from the avatar picker. A gravatar can
still be restored by its owner, since that image lives outside
Discourse.
- A `removed_avatar` staff action, since the actor is not always the
person reviewing.
The upload itself is kept, so the record of what was removed survives
for whoever handles the account.
No frontend work: reviewable actions only need client code when they
declare `client_action`, and this one is a plain server round-trip.
Workflow user payloads carried no reference to any user-supplied image.
The AI agent node already accepts `upload_ids` and routes them to a
vision-enabled agent, but nothing in a workflow could produce an
avatar's upload id, so profile images were unreachable.
### Changes
- `uploaded_avatar_id` and `avatar_template` on the shared user
serializer, which reaches every user-emitting node at once
(`user_created`, `user_updated`, `user_seen`, the group membership
triggers, `badge_granted`, `action:user`, `action:user_search`).
`User#avatar_template` is string interpolation over `username` and
`uploaded_avatar_id`, so this adds no query to the trigger path.
- `website`, `profile_background_upload_id` and
`card_background_upload_id` on the user node, behind the same
profile-detail guard as `bio_raw`. `user_profile` is already loaded
there.
`uploaded_avatar_id` is nil for system and letter avatars, so the nil
check a workflow has to do is also the "only look at user-supplied
images" filter.
### On the spec
Schema constants drive the builder's output pane and expression picker,
but nothing at runtime forces them to agree with the payloads they
document, and `match_node_output_schema` does not catch it either. A
property that is declared but never emitted becomes an expression that
silently resolves to nil.
This adds a spec pinning that direction for the user constants. Only
that direction: schemas are curated views that deliberately omit fields
their source emits (`TOPIC_PROPERTIES` hides around twenty), so extra
payload keys are not a failure and `additionalProperties: false` would
be wrong here.
The user constants were the only ones in the file already in sync both
ways, so this pins them before the first new field goes in. The same
spec extends to the post, topic and group constants later, with
allowlists for their conditional fields.
Previously, the `category`, `group`, `tag_list`, `tag_group_list` and
`icon` site settings were the last choosers on the admin site settings
page still rendered by a bespoke control, separate from the shared
FormKit field infrastructure the text, number, boolean and enum
families already go through.
This change adds a shared renderer for each of them under
`app/components/setting-field/` and marks the five types admin-ready,
continuing the per-type rollout. They wrap the same select-kit
components the legacy controls did instead of resolving to a native
FormKit control, because the built-in tag chooser hands its callback
raw select-kit items — which the row would serialize as
`[object Object]` — and cannot express `allowAny=false`, and there is
no built-in control for a category or a single group at all. Each
renderer owns its own pipe-delimited wire format, so no type ever hands
an array to the row's serializer.
`icon` does use the native control, but through a renderer rather than
the bare fallback so that it can pass `@onlyAvailable={{false}}`. The
control otherwise offers only icons already in the site's sprite,
whereas picking an icon is precisely what adds it there — without it an
admin could never choose an icon the site does not already render.
Two things a category setting got wrong now work. The chooser was
handed the raw wire value, a string, while select-kit compares category
ids strictly, so the stored category was never highlighted in the
dropdown and the header's tooltip and accessible name were the raw id
rather than the category name. On sites with lazy category loading it
also warned and re-fetched the category on every render.
Pressing Enter in a select-kit filter no longer submits the surrounding
form. Each settings row is its own form and the chooser's filter is its
only text field, so as soon as nothing matched what an admin was typing
the browser's implicit submission saved the setting out from under
them.
Finally, the renderers forward the field's disabled state, which the
category and group controls never did. As with every earlier step of
this rollout the legacy controls are left in place for a single cleanup
at the end: theme rows opt out of change tracking, and both `icon` and
the list variants of the tag types are reachable as theme settings, so
they still resolve them.
Previously, deleting an agent's bot user left `ai_agents.user_id`
dangling and released the id, so the next bot user created reused it and
every stale reference silently resolved to the new bot — which is how
enabling the discover feature re-attributed translation activity to
`discover_bot`, and deleting that bot left the older entries
unattributed.
This change allocates bot user ids below every id already claimed and
never reuses them, clears the reference when the user is destroyed, and
stops offering a bot user to the agents a feature only invokes
internally.
Reported in dev/t/190355.
Stacked on #42783 — without it, showing the action would offer group
members a delete the server silently no-ops.
The server grants bulk delete to members of
`delete_all_posts_and_topics_allowed_groups`, but the menu item was
offered to staff only, so a TL4 member of the group could bulk select
topics and never see a delete action the server would honor. Staff lose
nothing: the setting's `mandatory_values: "1|2"` keep them in the
capability.
Deliberately out of scope: non-staff, non-TL4 group members still cannot
enter bulk-select mode at all (`canBulkSelect` is staff/TL4-gated in the
discovery, user-topics, and search controllers). Widening that gate
would surface other menu items whose visibility still assumes staff —
the same "UI offers what the server refuses" class this stack fixes — so
it deserves its own pass.
Previously, the workflows "Modal" step showed its modal in every tab the
target user had open: answering or dismissing it in one tab left stale
copies open everywhere else, and acting on a leftover copy surfaced a
misleading "not found" error, since a response token can no longer be
matched once its execution has resumed.
This change tags every shown modal with a unique id and broadcasts a
`close_modal` message on the user's channel whenever the modal is
answered or dismissed, closing the remaining copies in all tabs; a
response from a leftover copy is now acknowledged as already handled
instead of erroring.
Reported at https://meta.discourse.org/t/410129
Stacked on #42776. Follow-up 2 of 2 from the bulk-actions investigation
([meta
t/410444](https://meta.discourse.org/t/bulk-actions-confusion-delete-topics/410444)).
`PostRevisor` silently refuses tag and category changes the user lacks
permission for, yet the bulk layer gated only on `can_edit?` and counted
every topic as changed: a green "Completed" toast, zero changes applied
— and each refused revise still bumped `post.version` and wrote an empty
`PostRevision`, permanently stamping a bogus "1 edit" indicator on up to
1000 topics per click. The UI also offered "Manage tags" to users the
server would then refuse.
`reset_bump_dates` checked permissions once, globally, so a TL4 user
could reset bump dates on topics in restricted categories they cannot
see — the single-topic route fixed this exact hole in aac9494fb3
(#37798), but the bulk path was missed. `destroy_post_timing` had no
permission check at all, and could drive `posts.reads` negative on
topics where the caller had no timings to destroy.
Stacked on #42776. Follow-up 1 of 2 from the bulk-actions investigation
([meta
t/410444](https://meta.discourse.org/t/bulk-actions-confusion-delete-topics/410444)).
The guardians grant delete and recover on membership of
`delete_all_posts_and_topics_allowed_groups`, but `PostDestroyer` only
ever consulted the staff / TL4 / category-moderator checks. For a
non-staff member of the group — exactly who the setting advertises
("Groups allowed to delete posts and topics created by other users") —
every delete endpoint returned 200 with nothing deleted, while
irreversible side effects (activity stream removal, webhooks, events)
fired anyway.
A refused recover was worse: it still resurrected the topic, leaving a
live topic whose first post is trashed — a corrupt state many downstream
consumers choke on, and one `PostOwnerChanger` could also produce by
recovering a topic without its first post.
Two deliberate consequences for these group members: their deletions of
other users' content now appear in the staff action log, and they remain
subject to `max_post_deletions_per_minute`/`_per_day` since the
rate-limiter exemption still keys on `can_moderate_topic?`.
Reported in [Bulk actions confusion - Delete
Topics](https://meta.discourse.org/t/410444).
Selecting topics and choosing **Delete** asks you to confirm:
> Permanently delete the selected topics. This action cannot be undone.
…and then soft deletes them, so they can be recovered. Not
search-specific — the same modal is used by every topic list.
There is no configuration where the copy is true.
`TopicsBulkAction#delete` builds `PostDestroyer` with an empty options
hash, so `permanent?` is never satisfied, and `PUT /topics/bulk` doesn't
permit `force_destroy` in the first place. `can_permanently_delete` has
no bearing on this path. The description was added with the copy pass in
4b6169028f and never matched the behaviour.
### Also fixed
Two long-standing bugs in the same six lines:
- `delete` was the only operation in `TopicsBulkAction` that never
appended to `@changed_ids`, so the endpoint always answered
`{"topic_ids":[]}`. A bulk delete where the guardian skipped every topic
looked exactly like a successful one, toast included.
- `ordered_posts` is scoped by `Trashable`, so for a topic whose first
post was already deleted it returned post 2. `is_first_post?` was then
false, `@topic.trash!` never ran, and an unrelated reply was deleted
while the topic stayed alive — reported as a success.
`TopicsController#destroy` already gets this right with
`ordered_posts.with_deleted.first`.
### Follow-ups, not in this PR
- `ordered_posts.first` feeding `PostDestroyer` carries the same latent
bug at five other call sites: `Jobs::DeleteTopic`, three in
`DestroyTask`, and `TopicGuardian#can_recover_topic?`. Worth a `Topic`
accessor rather than a sixth hand-rolled copy.
- Search results give no hint whether a hit is a topic or a reply, which
is the other half of the report. `post_number` is already serialized and
`search.post_format` already exists, but is gated to the in-topic
dropdown.
- Translations of this key still assert permanence until the pipeline
catches up.
Moving from one reviewable to another — by clicking through the review
queue list in the user menu, for example — is a change of model on the
same route, so the whole `ReviewableItem` tree is re-rendered in place
rather than torn down. Any state those components hold for a single
reviewable therefore survives the transition.
`ReviewableTimeline` copied `reviewable_notes` into a tracked field in
its constructor, which as a result only ever ran for the first
reviewable rendered. A note added to one reviewable was then shown on
every reviewable visited afterwards, and a reviewable's own note was
missing whenever it wasn't the first one opened. Notes are now read
from, and written to, the reviewable itself.
`ReviewableItem` kept `editing` and its pending `_updates` as well, so
leaving a reviewable mid-edit landed on the next one still in edit mode,
and saving from there wrote the previous reviewable's changes to it.
---
### Reproducing
1. Flag a few posts so the review queue has several pending items.
2. As a moderator, open the user menu, go to the review queue tab, and
click one of the items.
3. Add a note.
4. Open the user menu again and click a different item.
The note from the first item is shown on the second. Doing it in the
other order — opening a reviewable with no notes first — hides the note
on the reviewable that actually owns it. The review queue list itself
has always been correct, because it renders each reviewable in an
`{{#each}}` and so gives each one its own components.
For the edit half, do the same with two queued posts and click "Edit
Post" before navigating away.
### `ReviewableItem` becomes a glimmer component
`didUpdateAttrs` was the only place this state could be reset, and it is
classic-component-only. The state belonging to a single reviewable now
lives in a `@cached` `state` getter that reads `this.args.reviewable`,
so a different reviewable produces a fresh `trackedObject` and nothing
carries over — the reset is a consequence of the argument changing
rather than a hook that has to remember every field. The `@computed`
getters become plain getters, `this.set` gives way to assignment, and
`{{mut}}` on `claimed_by` gives way to a callback.
Plain getters only track what the model declares, so `Reviewable` now
declares its server payload as `@tracked`, the way `Post` already does.
`reviewable_notes` is an `@autoTrackedArray` seeded to `[]`, which is
what lets the timeline append to it directly instead of bridging through
`get`/`set` from `@ember/object`.
### Test coverage
None of this reproduces on a full page load, so `visit_reviewable` — how
every reviewable system spec navigates — cannot exercise it. The review
page object grows a `visit_reviewable_from_user_menu` that stays within
the running app.
`shows correct IP when navigating between reviewables` now uses it.
Worth flagging: that spec was added alongside the `didUpdateAttrs` fix
it was written for, and because it navigated with `visit_reviewable` it
had been passing either way ever since. It only becomes a real guard
here.
The component tests swap a tracked `@reviewable` on a mounted component,
which is the mechanism at fault. Each one kills a specific mutation —
removing the argument read from `state()`, or dropping
`@autoTrackedArray` — except `keeps a pending edit when the reviewable
it belongs to is refreshed`, which is deliberately the other direction:
`store.find` updates a record in place after every `perform`, so it
guards against a cache key that invalidates too eagerly.
`removes a deleted note from the timeline` covers a path that had no
test in either suite.
### Notes
`updating` and `disabled` are deliberately left outside `state`. They
belong to an in-flight `perform()` request whose `.finally()` clears
them — resetting them on navigation would clear the flag out from under
a request started on the new reviewable.
The bare `this.args.reviewable;` in `state()` is what ties the cache to
a single reviewable; deleting it makes both reset tests fail.
`no-unused-expressions` only runs on `.ts`/`.gts` here, so unlike the
equivalent in `block-outlet-root-container.gts` nothing catches it,
hence the comment.
Previously, `enum` and `locale_enum` settings were the last large family
on the admin site settings page still rendered by a bespoke select-kit
control, even though the shared FormKit registry already had an enum
renderer at parity that the category type, plugin feature and workflow
forms use.
This change marks both types admin-ready so they render through that
shared renderer, and fixes two things a native `<select>` does not get
for free. A stored value that is no longer one of the choices is now
offered as its own option: select-kit synthesized a fallback item for
this case, whereas a native select with no matching option silently
falls back to the first one, so a setting pointing at a deleted record
or at a locale removed with its plugin would have displayed — and, on
the next save, persisted — the wrong value. And `locale_enum` gets its
own renderer that labels each option through the language name lookup,
because the shared valid-values projection translates the locale name
and drops the native one, which would have turned "Inglés (English)"
into "Inglés" for every admin.
The setting preview also moves into the FormKit branch of the row. Only
one setting in core ships a `preview`, it is an enum, and nothing
asserted on it, so converting the type would have deleted the feature
unnoticed; a test now covers it.
Finally, the enum page object helper moves from select-kit to `DSelect`,
and the locale system spec now asserts on the select's value instead of
the row's text — with every locale rendered as an option, a text match
passed no matter which one was selected.
---
<sub>Stack created with <a
href="https://github.com/github/gh-stack">GitHub Stacks CLI</a> • <a
href="https://gh.io/stacks-feedback">Give Feedback 💬</a></sub>
Previously, `ListController#category_default` only honoured `hot`,
`latest` and `top`, so a category whose `default_view` was any other
registered filter — like `votes` from `discourse-topic-voting` — was
served the `latest` list on a direct link, even though the client
highlighted the matching tab.
This change resolves `default_view` against `Discourse.filters`,
restricted to `Discourse.anonymous_filters` when logged out, and routes
`/c/<slug>/<id>/none` through the same resolver instead of hardcoding
`latest`.
---
### Why it only broke on a direct link
The client has always resolved `default_view` correctly, so an in-app
transition requested `/c/<slug>/<id>/l/votes` and got the right list. On
a full page load the server preloads its list under the fixed key
`topic_list` (`TopicList#preload_key`), and `TopicListAdapter#find`
consumes that key without checking it matches the filter it asked for —
so the `latest` list was used and relabelled as `votes`. That is also
why clicking "Latest" afterwards appeared to do nothing: the list was
already `latest`.
Crawlers never recover from this, since there is no client-side
correction for them.
### `/c/<slug>/<id>/none`
That route was hardcoded to `category_none_latest`, while the matching
Ember route (`discovery.category-none`) has always resolved
`default_view`. This is broken today for core views too — a category
with `default_list_filter = "none"` and `default_view = "hot"` renders
`latest` there, no plugin involved.
`:as => "category_none_default"` on the route is load-bearing:
`construct_url_with` builds `"#{action_name}_path"` on every request.
### Behaviour change worth flagging
A logged-in member visiting a category whose `default_view` is a
member-relative filter (`unread`, `new`, `bookmarks`, …) now gets that
filter instead of `latest`, which may be an empty list. This matches
what in-app navigation already does — making the two agree is the point
of the fix. Anonymous visitors still fall back to `latest`, and none of
these values are reachable from the category settings UI.
### `TopicListSerializer#filter`
Exposing the list's filter makes the ordering assertable in specs — the
existing `category_default` examples only checked status and
`for_period`, and passed either way. It also gives the client something
to compare a preloaded list against, should we want to close the preload
mismatch above properly. `category_topics_response.json` is
`additionalProperties: false`, so the schema is updated to match.
Previously, the `prioritize_recently_used_tags` upcoming change was
`conceptual`, so it was hidden from the upcoming changes admin page and
admins had no way to preview it.
This change promotes it to `experimental` and adds the `learn_more_url`
pointing at the meta topic, so admins can opt themselves, staff, or
specific groups in.
Previously, the admin site settings page rendered every text field
setting — `string`, `float`, `username`, `email`, and the `textarea` and
`secret` variants of those — with a bespoke control, separate from the
shared FormKit field infrastructure that `bool` and `integer` were
already converted to.
This change renders them through that shared infrastructure, continuing
the per-type `adminReady` rollout so that every type not yet converted
keeps its current control. Each converted type gets its own registry
entry rather than relying on the registry's `default` fallback: that
fallback is the catch-all for every unregistered type, including more
than twenty that still have bespoke controls, so marking it ready would
have silently degraded all of them to bare text inputs.
The `textarea` and `secret` variants resolve through a `subtype` derived
on the setting model, rather than by teaching the shared registry two
site-setting-only flags. This keeps the registry dispatching on a single
axis and puts the two ordering rules next to the data that produces
them: `textarea` wins over `secret`, so a multi-line credential such as
a PEM key stays readable instead of collapsing into a masked single-line
input, and any `list_type` vetoes both, so the delimited `key|secret`
pairs of a `secret_list` are never rewritten as a single value. Settings
embedded in plugin admin pages now mask their secrets as a result, where
before they were rendered in cleartext.
Two behaviours the legacy control provided move into the FormKit
controls themselves rather than into setting-specific wrappers, since
both apply to every consumer: `FKControlInput` sets `dir` when
`support_mixed_text_direction` is enabled, restoring the bidi handling
these inputs used to inherit from `DTextField`, and `FKControlPassword`
sets `autocomplete="new-password"` so that browser password managers
stop offering to autofill credentials into API key fields.
Finally, pressing Enter on a text field setting now submits through the
row's full save path, including the confirmation dialog and the backfill
prompt, instead of bypassing them — the same fix already made for
`integer`.
Previously, inline `code` only got its chip styling when its direct
parent was `p`, `li`, `strong`, `em` or `a` — every other container
(`small`, `b`, `i`, `sub`, `sup`, `span`, `del`, headings, table cells,
…) fell through to the code-block rule, whose light-mode background is a
3% blend of primary over secondary and is therefore indistinguishable
from a quote's 5% blend, and *exactly identical* inside a nested quote.
Fenced code blocks sat on the same collision, so they vanished in quotes
too.
This change makes the inline style the default for `code` and scopes the
block style to `pre > code`, and derives `$hljs-bg` from
`$inline-code-bg` so the two code surfaces match by default and neither
disappears against a quote background. `--hljs-bg` and
`--inline-code-bg` both keep their names and meanings, so themes that
differentiate them are unaffected.
Reported in
https://meta.discourse.org/t/code-is-invisible-solely-when-directly-inside-small/410078
### Screenshots
<img width="3200" height="1538" alt="comparison-dark"
src="https://github.com/user-attachments/assets/28263913-6d94-46b5-a8e6-4178a601521f"
/>
<img width="3200" height="1538" alt="comparison-light"
src="https://github.com/user-attachments/assets/86ab9405-f53e-423c-8e31-6b438bf2e5c5"
/>
Previously, only *generated* node names were unique. The rename field
accepted any non-blank string and file import concatenated names
verbatim, so renaming two nodes to the same value — or importing a
workflow into one that already shares a node name, or importing a node
with no name at all — produced a graph the server rejected on every
subsequent save, surfaced as a generic toast with no indication of which
node was at fault.
This change validates the name inline while renaming (the save button
stays disabled with an explanatory message) and uniques imported names
against the existing graph, falling back to the node type's label when
an imported node has no usable name.
Stacked on #42627, which adds the `takenNodeNames` helper this uses.
Voting in a poll whose results are hidden (`results=on_close` while
open, `results=staff_only` for non-staff) produced almost no feedback:
the ballot stayed on screen with "Vote now!" still enabled, so it felt
like the vote had not registered — and re-clicking re-sent it.
The reasoning behind each change:
- **The ballot becomes a "You voted for:" summary after casting**
because a view change is the confirmation voters already know from
visible-results polls, and their own choices are the only thing that can
be shown while results must stay hidden.
- **The info column swaps the selection help text for "Your vote has
been recorded."** because that instruction no longer applies once you
have voted, and replacing it in place keeps the column from reflowing;
the "Results will be shown once **closed**." hint stays untouched
because that fact has not changed.
- **The resting state offers only "Change vote"** because a confirmation
should not carry a destructive action; **"Undo vote" only appears on the
reopened ballot**, next to "Update vote", so the two vote actions are
never separated by the "Results" toggle.
- **"Update vote" stays disabled until the selection differs from the
recorded vote** so clicking it "just to be sure" is a no-op instead of a
duplicate request.
- **"Back" discards tentative changes** because reopening the ballot to
double-check your vote must be risk-free.
- **Re-clicking your current choice while changing a single-choice vote
keeps it** (instead of the usual tap-to-remove) because the ballot was
reopened to reconsider, and removal already has an explicit button
there.
- **All view state derives from the persisted vote**, via the same
per-poll stashes as `showResultsToggle`/`inProgressVote`, because poll
components are destroyed and recreated on scroll — anything transient
would forget the vote. `votedChoices` reads through the tracked
`hasSavedVote` because `post.polls_votes` can be a plain, untracked
object.
- **The vote payload is snapshotted before the request and guarded
against re-entry** because a selection edited during a slow request must
not be recorded as saved, and a double-click must not cast twice.
- **Focus is handed to the next control on cast/change/undo** because
the clicked button unrenders, which would otherwise drop keyboard focus
to `<body>`.
- **The summary flips to results when the poll is closed while on
screen, but only once vote counts arrive**, because the results view
cannot render without them; "Back" works even on a closed poll so a
voter is never trapped on the ballot.
- **The dead "Closes in …" / "Closed … ago" info row is revived** — it
referenced properties that no longer exist — with fixed interpolation, a
single date parse in the parent, and no countdown on manually closed
polls, because a countdown on a closed poll is a false promise.
### BEFORE / AFTER screenshots
<img width="1222" height="478" alt="pr-shot_autoclose"
src="https://github.com/user-attachments/assets/ebf2233a-2cee-49e2-ba7b-57c17f30b2db"
/>
<img width="1222" height="715" alt="pr-shot_big"
src="https://github.com/user-attachments/assets/eeae2444-704a-4358-8f13-97cf74e84a39"
/>
<img width="1222" height="424" alt="pr-shot_number"
src="https://github.com/user-attachments/assets/6d4e9bb9-0578-41ac-8037-86d544efb4a8"
/>
<img width="1222" height="462" alt="pr-shot_ranked"
src="https://github.com/user-attachments/assets/dcc51619-ac86-401f-a112-b3739acd5bb5"
/>
<img width="1222" height="424" alt="pr-shot_single"
src="https://github.com/user-attachments/assets/0ce5eaff-082b-4f22-b656-86f02e87d942"
/>
<img width="1222" height="443" alt="pr-shot_staff"
src="https://github.com/user-attachments/assets/31402444-4966-4431-a8b3-e6732b9c4e5f"
/>
Previously, `serializeConnections` accumulated into a plain object
literal keyed by node name, so a node named `__proto__` routed the write
to the prototype accessor instead of an own property: the node's
connections silently vanished from the save payload and
`Object.prototype` gained a stray `main` property for the rest of the
session.
This change accumulates into a null-prototype object, so a node name can
never resolve to an inherited property. Node names are user supplied —
typed in the rename field, or arriving via paste and file import — so
nothing upstream constrains them to safe keys.
Previously, pasting a node in the workflow editor threw away its name
and regenerated one from the node type, so a node named "Fetch topic"
came back as "HTTP request 1".
This change keeps the copied name and appends a counter only on
collision. Because a pasted name can now be anything the user typed,
node naming also reserves the name sticky notes serialize under — the
server rejects a graph where an executable node shares it, and that
reservation has to hold before the workflow has any sticky note, not
just once one exists.
https://meta.discourse.org/t/copy-workflow-step-names/409410
Previously, the icon pickers for badges and group flair only offered
icons already in the site's SVG sprite: `only_available` arrived as a
query-string value where both `"false"` and `""` are truthy in Ruby, so
every picker had been silently restricted since 2023 — and even
unrestricted, the endpoint capped results at the first 500 of 2,000+
icons alphabetically. A badge could not use a new icon until it was
manually added to `svg_icon_subset`
(https://meta.discourse.org/t/discourse-fa-seeding/409783).
This change lets the pickers search the entire icon set: browsing lists
the sprite's icons first, search covers everything, and the grid loads
pages of 100 as it scrolls. The server reports `has_more` so clients
carry no page-size knowledge, ships `<symbol>` markup only for icons the
viewer's theme sprite cannot render, and browsed symbols stay scoped to
the picker — only a picked icon is added to the page sprite, so it keeps
rendering until saving adds it to the sprite for everyone.
### Before / after
**Searching an icon outside the subset**
<img width="990" height="165" alt="01-search-non-subset-icon"
src="https://github.com/user-attachments/assets/d35562f0-66e6-49c7-b584-497a24cae9e1"
/>
**Scrolling past the old 500-icon cap**
<img width="990" height="395" alt="02-infinite-scroll"
src="https://github.com/user-attachments/assets/73fd724f-ee0e-47b8-930d-7d2cefa08797"
/>
**A picked non-subset icon rendering in the trigger**
<img width="602" height="160" alt="03-picked-icon-in-trigger"
src="https://github.com/user-attachments/assets/8d41d1be-8aeb-48f7-b9bc-d00a1f33dfb5"
/>
---------
Co-authored-by: David Taylor <david@taylorhq.com>
Previously, clearing a field could silently do nothing: form controls
represented "cleared" as `undefined`, which `JSON.stringify` drops, so
the key never reached the server and any endpoint that treats an absent
key as "leave this column alone" kept the old value. It stayed hidden
because it only bites JSON bodies — jQuery's `$.param` encodes both
`null` and `undefined` as `key=`, so form-encoded requests always
transmitted the clear correctly.
This change has the form controls emit `null`, which survives
serialization, and repairs the places where a clear was being discarded
or was crashing on arrival.
The category case is the one that prompted this: setting "Topic list
sort by" or "Default topic list" back to **Default** went dirty,
reported success, then snapped back. It regressed when those controls
moved off select-kit's ComboBox (which emitted `null`) onto the FormKit
select; the read path was taught to normalize both at the time, the
write path was not.
Commits, each independently reviewable:
- `DEV:` form controls emit `null` — `fk/control/input.gjs` already did,
so this brings the stragglers in line
- `FIX:` category appearance settings — plus the dangling sort
direction, stored values no option provides, and a `nil` guard for
subcategory list style
- `FIX:` a category's minimum required tags — `nil&.blank?` is `nil`, so
the guard skipped the case that reached the NOT NULL column
- `FIX:` a tag's custom slug
- `FIX:` a topic's featured link — client rename plus `PostRevisor`,
which rejected a write that only removes one
- `FIX:` edit conflict detection for topic titles and tags — the payload
was built from wire names, so both fields were always `undefined`
- `FIX:` permission check when a topic's category is cleared —
API-reachable only
- `FIX:` AI tool RAG chunk sizes — 500 instead of a validation error
One caveat for bisecting: between the first two commits there is a
window where selects emit `null` but the subcategory-list-style `nil`
guard has not landed, so clearing that one field would error. The series
is correct as a whole. Happy to squash the first two if preferred.
Previously, values typed into the event builder's custom fields were
cleared as soon as another field changed (location, image, name, …), and
toggling between the compact and advanced screens could permanently
revert them to stale values.
This change commits custom field values through the form's `set` (a
field's `@onSet` replaces FormKit's default write, so skipping it left
the typed value out of the form data), replaces the `customFields`
reference on every write (mutating inside the object never invalidated
`compactInitialState`, so the compact screen served stale values back),
and tracks `formData` (untracked, the advanced screen always
reinitialized from the modal's construction-time snapshot).
Reported in
https://meta.discourse.org/t/potential-bug-where-data-in-custom-fields-is-cleared-if-certain-fields-change/410050
Previously, automatic AI bot conversation titles were saved through
`PostRevisor` without bypassing the edit rate limiter, so each one
counted against the bot account's daily allowance. Agent bot users are
TL4 but not staff, so unlike the system user they are not exempt — once
a busy bot spent its allowance the limiter raised inside the enclosing
transaction, the title write was rolled back, and conversations were
left as "[Untitled AI bot PM]" while replies carried on working.
Reported by a customer who noticed exactly `max_edits_per_day` × the TL4
multiplier successful titles in a day, and none after.
This change passes `bypass_rate_limiter` on the title and
regenerate-reply paths, moves `bypass_bump`/`skip_validations` in the
LLM tagger and triage automations out of the fields hash and into the
options hash where `PostRevisor` actually reads them, and makes title
generation resilient to an empty or over-long model response by falling
back to an excerpt of the member's own first post, truncating to a valid
length, and only announcing the title over MessageBus once the save
succeeded.
FloatKit gives a tooltip a fixed `maxWidth` and no height limit at all.
That is fine for the short labels most tooltips carry, but it breaks
down as soon as one holds user content — footnotes, chat reaction lists
and event descriptions all render arbitrary-length HTML into a tooltip,
and the result is a popup bigger than the screen it has to fit on.
Neither axis recovers on its own, because floating-ui's `shift` can only
*move* a float, never shrink it.
**Vertically** it clamps the popup flush under the header and leaves the
rest below the fold. Because the float is re-anchored to its trigger on
every scroll, its viewport rectangle never changes, so the hidden part
cannot be reached by scrolling — the page just slides underneath it.
Measured on a 390×664 phone with a long footnote: a 1557px popup with
945px permanently unreachable, and swiping inside it scrolled the page
from 871 to 1381 while revealing nothing.
**Horizontally** the same clamp pins a 350px popup 10px from the left
edge of a 320px screen, so the overflow joins the document's scrollable
area and the whole page — header included — can be panned sideways
(42px, growing with each pan).
## What changed
- `.fk-d-tooltip__inner-content` is bounded to `60dvh` and scrolls,
which is what menus have done since they gained `overflow: auto`.
Tooltips kept the `overflow: hidden` they were born with and were simply
never revisited, so the asymmetry was drift rather than a decision.
- Alignment becomes `align-items: safe center` at the same time. The
cross axis is the one being capped, so plain `center` would centre
content that is too tall and push its first lines above the scrollport,
where scrolling cannot follow — measured at 830px out of reach.
- `maxWidth` is clamped against the space the viewport actually leaves,
derived from the same padding `shift` keeps clear so the two cannot
disagree.
- Only *numeric* `maxWidth` values are clamped. The option also accepts
CSS keywords (the user card passes `unset`), and wrapping a keyword in
`min()` is invalid CSS — the browser would drop the declaration and hand
the float to whatever stylesheet rule was previously being overridden.
`overflow-x` stays `hidden` rather than becoming `auto`: the same rule
carries `overflow-wrap: break-word`, so `auto` on both axes would give
tooltips a horizontal scrollbar for unbreakable content.
## Before / after
A long footnote on a 390px-wide phone, and the same popup after swiping
inside it:
| | Before | After |
|---|---|---|
| popup height | 1557px | 400px |
| clipped below the fold | 945px | 0 |
| scrollable | no | yes (1157px of scroll) |
| swiping inside it | scrolls the page, popup unchanged | scrolls the
footnote, page stays put |
| 320px screen | 352px wide, page pans 42px | 302px wide, no pan |
<img width="844" height="751" alt="footnote-height-before-after"
src="https://github.com/user-attachments/assets/64a8107d-47c2-45f9-9a77-75f121ec8b8f"
/>
<img width="844" height="782" alt="footnote-narrow-before-after"
src="https://github.com/user-attachments/assets/6dbfffaf-2885-4836-b020-3ee2d31ecfcb"
/>
<img width="844" height="751" alt="footnote-scroll-before-after"
src="https://github.com/user-attachments/assets/9b0dade9-0238-4e44-b10f-2d48bbe0da57"
/>
## Testing
- `d-tooltip-test.gjs` gains a height/scroll test and a
keyword-`maxWidth` test, and the two numeric `maxWidth` cases are folded
into one. Each new assertion was checked to fail against the specific
declaration it guards: dropping `safe`, `box-sizing`, or `max-height`
each turn the scroll test red, and wrapping keywords in `min()` turns
the keyword test red.
- A footnote system spec covers the end-to-end mobile case at 320px —
real cooked content, real touch scroll, and the document-level
horizontal overflow that a rendering test cannot observe.
- Ran the tooltip- and menu-heavy core system specs (cards, user tips,
filter navigation, styleguide, localization menus, composer, bookmarks,
chat reactions, calendar) — ~200 examples, no new failures.
Stacked on #42469 — review that one first. This PR's diff against `main`
will show both; the second commit is the one to look at.
None of the code removed here was reachable.
### Stylesheets
- FullCalendar v4 class names (`.fc-unthemed`,
`.fc-list-item-add-to-calendar`) left behind by the v6 upgrade
- The pre-Glimmer `widget-dropdown` blocks, in two places
- `.event-invitees .header` and its subtree — the component has never
rendered a `.header` child. It was `display: none` on itself, which is
the tell.
- `.event-actions .event-status` — a descendant selector, but both class
names have always been on the *same* element, so it has never matched
- `&.auto` / `&.small` / `&.medium` / `&.large` on `.group-timezones` —
the markdown rule emits `data-size`, and the sanitiser allowlist permits
only that attribute, so the class form is unreachable even in old cooked
HTML
- `.invitee .status`, `.combo-box.user-timezone`, `.event-dates
.participants`, `.event-dates .separator`, and a few other selectors
with no emitter
### JavaScript
`event-relative-date` is the one worth a look. Its initializer ran a 60
second timer for the lifetime of the page to recompute
`.event-relative-date.topic-list` elements — but the component that
renders that class has never emitted `topic-list`, so every tick walked
an empty NodeList. Removing the initializer takes the recurring timer
with it.
The rest have no callers: a helper, a model, an options builder and an
empty route/controller pair that nothing imports, plus three getters in
the event builder that are shadowed by the compact editor's own copies.
### Ruby
The `EventStarted` job is never enqueued — the `DiscourseEvent` it
triggers is fired directly by `monitor_event_dates`.
### How this was checked
Every candidate was checked against dynamic class construction
(`concat`, template literals, BEM helpers), cooked post HTML and the
markdown sanitiser allowlists, third-party DOM (FullCalendar, chat,
core), and both shipped themes, before being removed. Anything that
could not be positively ruled out was left in place.
261 JS tests and the plugin's system, job and model specs pass. Three
failures in the suite reproduce identically on a clean checkout — two
are local test-database pollution and one is a local ImageMagick font
issue.
### Deliberately left alone
- Serializer attributes (`is_ongoing`, `is_private`, `is_public`,
`capacity`) — externally visible API, a local grep can't clear them
- The `split_grouped_events_by_timezone_threshold` site setting — needs
the deprecation path
- ~16 apparently unused i18n keys — worth a separate pass, since a
couple need per-key checks against similarly named symbols and there are
translation-sync implications
- Two `replaceIcon` notification registrations — removing a live one
silently breaks an icon for no gain
Previously, claiming a topic recorded a history entry on every
reviewable it contained, including ones resolved long ago, and the
transient lock taken whenever a moderator opens a confirmation dialog or
an action modal was recorded the same way — even on the sites where
claiming is disabled, which is the default. A flag that stayed open
therefore accumulated an endless list of claim/unclaim pairs, most of
them recording activity on other flags entirely.
This change records only deliberate claims, and only against reviewables
that are still pending, so a resolved item's timeline stops changing and
the entries that remain correspond to real moderation decisions.
Previously, the admin site settings page rendered every setting with
bespoke per-type controls layered on a buffered proxy, separate from the
shared FormKit field infrastructure (`SettingDefinitionField` and the
setting-field registry) that category-type and plugin settings already
use.
This change renders `bool` and `integer` settings through that shared
infrastructure — gated per-type by an `adminReady` registry flag so the
remaining types keep their current controls until they are converted in
turn — by wrapping each row's control in a single-field `<Form>` whose
`@onSet` writes back into the existing buffered proxy, so dirty
tracking, the changes banner, and the route guard keep working
unchanged. It also fixes two latent issues it surfaced: pressing Enter
now submits through the row's full save path (including the confirmation
dialog) instead of bypassing it, and `FKControlInput` no longer keeps a
stale raw-text buffer that stopped Cancel and Reset from reverting a
number field.
Every endpoint that lists tags for browsing hand-rolled its own filter
chain, and they had drifted apart.
The tags page filtered synonyms out when tags were listed flat but not
when they were listed by group, so a synonym of a tag belonging to no
tag group was shown next to its target — a duplicate entry whose page
only redirects back to the tag it duplicates. Synonyms of grouped tags
were hidden by accident rather than by design: creating a synonym copies
the target's tag group memberships onto it, which happens to satisfy the
"belongs to no tag group" condition the ungrouped list is built from.
Non-admins were shielded by a second accident, since a synonym normally
ends up with a zero topic count — so any synonym that kept its topics,
whether from an import, a plugin, or assigning `target_tag_id` directly,
was listed to everyone.
Tag visibility has two independent mechanisms: tag group permissions,
and the categories a tag is attached to. Only the second one can apply
to a tag that belongs to no tag group, and neither the grouped tags page
nor the tag group search endpoint applied it, so the names of tags
restricted to a category the viewer cannot read were listed to them.
`TagGroup.visible` gates which groups are returned, never the tags
inside them.
The navigation menu tag picker filtered out neither synonyms nor tags
only used in personal messages, and gated on staff where the tags page
gates on admin, so moderators were served the entire tag table. A
synonym picked there was saved as a sidebar link and rendered from then
on. Top tags and tag search had the same gap.
Paths that resolve a tag by name keep matching synonyms on purpose: the
composer offers them so that typing a retired name finds the tag that
replaced it, and hashtags in posts cooked before a rename have to keep
working. Serializers that echo configuration back are left alone,
because filtering a value the client posts straight back would silently
delete it.
Display rules compare the parameter they are anchored on against the
literals they expect, which an expression can never equal. Everything
gated on a dynamic parameter therefore behaved as if the gate had
definitely failed: the field disappeared from the configurator, so a
value the node still needs at runtime could no longer be entered, and
its required-field check disappeared with it instead of being waived on
purpose. Output contracts were chosen the same way, so the node ended up
declaring no schema at all and downstream nodes had nothing to pick
from.
An anchor holding an expression is not a rule that failed, it is a rule
that cannot be answered yet, and that deserves a state of its own: the
target stays visible with its hard requirements suspended until the gate
is known, and every variant still in contention contributes to the
output schema rather than none of them.
The editor and the server judged all of this independently, so the two
drew different conclusions from the same workflow — and the server did
not even agree with itself, carrying three copies of the matching.
Sharing one is also what stops a credential slot gated on an expression
from being discarded on save while the editor still shows it.
All of that because I wanted the `operation` parameter of a "add user to
group" node to be dynamic, and when doing that, it would hide the `user`
parameter 😅
<img width="1372" height="720" alt="before-after"
src="https://github.com/user-attachments/assets/ff854611-c809-421a-a904-b40ccb91ede4"
/>
Previously, RSVPing "Going" to a livestream event hid the first post's
avatar — a leftover from the original theater-mode design carried over
when the standalone livestream plugin was folded into
discourse-calendar.
This change keeps the avatar visible and swaps the post body's `width:
100%` for `flex: 1`, so the body still fills the remaining width in
theater mode without squeezing avatars (the post row is a flex
container) or overriding the narrower width embedded posts expect.
Internal ref - t/188846
Previously, the custom sidebar section editor and the server assumed a
section's base title and link names were written in the site's current
*default* locale, so changing the default locale scrambled which
language each stored value mapped to and offered the wrong set of
translation targets ([meta](https://meta.discourse.org/t/408327)).
This change keys the editor, the same-locale de-duplication and the
collision cleanup off each record's own source locale — now surfaced as
an editable "Section language" — and reorganises the translation UI
around languages rather than fields.
### Notable behaviour changes
- Links keep their own source language, so saving a section no longer
relabels a link authored in a different one.
- A localization colliding with its record's source is now destroyed
instead of being left behind to shadow the base string. The collision is
matched **exactly**, so a regional variant like `en_GB` under an `en`
source is preserved.
- Localization permissions are evaluated against the visibility being
*submitted*, so making a section public and translating it in one save
no longer 403s and loses the edit.
- `create` now enforces localization permissions, which it previously
skipped entirely — an admin could write localizations onto a private
section. A non-admin submitting localizations on create now gets a `403`
instead of a silent drop, matching `update`.
- A submitted locale is validated at the request boundary: an
unsupported or over-long value returns `400` rather than an unhandled
`500`. A value already stored on the record is still accepted, so
AI-detected locales outside the supported list stay editable.
### UI
<img width="821" height="646" alt="2026-07-29 @ 14 56 10"
src="https://github.com/user-attachments/assets/19dad480-9f77-4f5b-b1a6-ed29689839a8"
/>
<img width="820" height="666" alt="2026-07-29 @ 14 56 16"
src="https://github.com/user-attachments/assets/a9377282-d4b2-43af-91a6-700e10c50ce6"
/>
Translations moved from inline per-field rows to a single "Manage
translations" entry point opening a language-major panel — one group per
language holding the section title and every link name. The old layout
rendered `1 + links × languages` rows with an add button per field,
which stopped being usable at three links and two languages.
A blank field means "not translated yet" rather than invalid, and
clearing a saved translation now asks for confirmation before removing
it.
Previously, a `Service::Runner` action block could declare a keyword
argument with a default (e.g. `on_success do |table_sizes: {}|`), but
the runner only fills **required** keyword arguments from the service
result — so the default always won and the service's computed value was
silently dropped, producing 200 responses with subtly wrong data (see
#42263 where this happened twice in the workflows plugin).
Following review feedback, instead of raising on defaulted keyword
arguments (the initial approach), the runner now fills them from the
result exactly like required ones. The default only applies when the
service didn't set the key at all, making it a proper fallback for keys
that are only set on some paths — a model fetched by a step that didn't
run, or steps wrapped in an `only_if` block. This matters for outcome
blocks in particular, since the caller (possibly a plugin) doesn't
always own the service it calls and can't make it always set a key.
Also documents required vs defaulted keyword arguments in the service
objects developer guide.
Previously, the workflows admin showed "0 Bytes" for every data table
and the executions list never loaded more than the first page, because
`Service::Runner` only fills **required** keyword arguments of
`on_success` blocks from the service result — defaulted kwargs like
`table_sizes: {}` and `load_more_url: nil` silently kept their defaults,
dropping the values the services computed.
This change declares both keyword arguments as required, and moves
`Execution::List`'s `load_more_url` step out of `only_if(:has_more)`
(guarding inside the compute method instead, like `Workflow::List`) so
the context key always exists and the required kwarg can never raise.
Previously, the topic payloads emitted by workflow triggers and nodes
did not include the topic owner, so a condition like "was this post
written by the topic owner?" required an extra `action:topic` lookup
just to fetch `user_id`.
This change adds `user_id` to the topic payload of every trigger and
node — via a plugin-level `TopicListItemSerializer` — so expressions can
compare `$json.topic.user_id` to `$json.post.user_id` directly, and
consolidates the duplicated per-node `topic_data` helpers into a single
`NodeType` helper.
Previously, enabling `cache_control_bfcache_compatibility` (the
experiment currently running on meta, #38763) made HTML documents
storable in the browser HTTP cache, and browsers skip revalidation on
history navigations — so back/forward, session restore, and
discarded-tab reloads could resurrect a days-old document: stale topic
lists that only get older, and logged-in UI shown to logged-out sessions
(and vice versa). Reported in https://meta.discourse.org/t/400459.
This change keeps the documents out of shared caches (`no-cache,
private`), reloads any document that was served from the HTTP cache on a
history navigation (navigation entry with `type === "back_forward"` and
`transferSize === 0` — a forced reload gets type `"reload"`, so it
cannot loop), and validates the session on `pageshow` restores from the
back/forward cache, reloading when the logged-in user no longer matches
the one the page booted with. Both client-side checks only run when the
setting is enabled; the full rationale (browser-engine specifics, why
`transferSize` rather than `deliveryType`, why `fetch` rather than
`ajax`) is in the commit message.
Reproduced and verified end-to-end in Chromium and Firefox: with the
setting enabled, `goBack()` served `/latest` with zero network contact —
stale list, wrong login state, and the exact `403
/u/:username/private-message-topic-tracking-state` errors from the meta
report; with this change the same navigation heals with a single
automatic reload, and a control run with the setting disabled behaves as
before.
Previously, suspect-user reviewables ("user needs approval" flags) and
queued-post reviewables could only be resolved by deleting the user or
rejecting the post outright — a reviewer mistake on an automated false
positive was irreversible, and sites with no-deletion policies had to
resolve the flag and then manually suspend the user from their admin
page. Requested in [meta t/408891](https://meta.discourse.org/t/408891)
and [meta t/225660](https://meta.discourse.org/t/225660).
This change adds guardian-gated **Silence user** / **Suspend user**
resolutions to both queues via a shared `build_penalty_actions` helper,
using the same penalize-modal flow the flagged-post and
review-every-post queues already use (staff log linked back to the
reviewable). Along the way it:
- hides penalty actions that are already active everywhere (previously a
resolve-then-409 dead end), and fixes the penalize modal's unawaited
`before()` race so the penalty is only applied once the reviewable
action succeeded;
- narrows the rejected-user **scrub** affordance to records whose
identity snapshot is the last remaining copy (user deleted, or
renamed/anonymized after a failed deletion), admin-gated to match the
endpoint;
- shows an active penalty (localized end date and reason) on the
reviewable user card, so it's clear why a penalty option is absent;
- preloads `anonymous_user_master` on the queue since the new
`silenced?` gates would otherwise lazy-load it per row;
- rewrites the action descriptions in one consistent voice.
<img width="460" height="313" alt="2026-07-31 @ 07 46 57"
src="https://github.com/user-attachments/assets/87d5bf49-97e0-47a4-939e-16da3f55c6e1"
/>
Previously, in Safari, a `DSelect` whose options were re-rendered
visually reset to its first option — WebKit deselects a freshly inserted
`<option selected>` while the old selected node is still present (the
spec's "last selected option wins" arbitration), then falls back to the
first option once it's removed. Nothing re-asserts the value afterwards,
since the `value` binding on the `<select>` can't
(https://github.com/emberjs/ember.js/issues/19115). In the workflows
Filter node this looked like changing one condition's operator updated
every condition of the same type: each rebuilt select showed its first
option, which happened to be the operator the user had just picked.
This change renders `DSelectOption` as a single `<option>` driven by the
`selected` DOM property — a dynamic binding for in-place updates plus a
`claimSelectedAfterRender` modifier that re-claims selection after the
render settles, i.e. once the old option nodes are gone — which survives
WebKit's arbitration and fixes every `DSelect`/FormKit select in Safari.
The `selected` attribute consequently no longer appears in the DOM, so
the one test asserting it now checks the select's value. CI runs Chrome
only (where the freshly inserted option already wins), so the new "keeps
the selection when options are rebuilt" test pins the invariant
cross-browser while the WebKit behavior was verified directly.
Previously, reviewable reasons from custom flag producers were treated
as inline text, and pre-escaping prevented Markdown and line-break
formatting from being recognized.
This change cooks a constrained Markdown subset at serialization so
every producer shares one sanitization boundary, renders cooked block
HTML directly, and keeps watched-word matches literal because their
Markdown metacharacters are data rather than formatting.
Previously, the `reduce_cooked` handler appended the `.poll-title`
element and then the inner HTML of `.poll-container`. Since the markdown
rule nests the title inside that container, every poll's title showed up
twice in HTML emails — and, since they share the same code path, in
digests, embedded comments and RSS feeds too.
This change drops the redundant append, leaving the container as the
single source of the poll's content, and tightens the email specs to
assert on the whole reduced fragment rather than with `include`, which a
duplicate satisfies just as well as a single copy.
https://meta.discourse.org/t/408875
Previously, a nested list pasted or written in Word was flattened and
its last item was glued to the item that followed it, because Word
closes the `<li>` before opening the sublist — leaving the sublist a
sibling of the item it belongs to, with no enclosing `<li>` to indent it
and no visitor emitting the separating newline.
This change moves such sublists into the preceding `<li>`, matching how
browsers already render them, so the hierarchy the sender saw is
preserved.
Previously, a reply sent from Outlook was truncated at its first
numbered list — `extract_from_word` treats any child of `.WordSection1`
that is neither a `<p>` nor a `<ul>` as the start of a signature or
forwarded message, and Outlook renders numbered lists as `<ol>`, so the
list and everything after it was elided and (outside of private
messages, or with `always_show_trimmed_content` disabled) dropped from
the post with no error.
This change excludes `<ol>` as well, so numbered lists are kept along
with the content that follows them, while the `<div>`/`<table>`
signature and forwarded-message blocks the method exists to strip are
still elided.
Previously, a site in read only or staff only mode still offered
anonymous visitors every way in: the header "Sign Up" button, the signup
call to action at the end of a topic, the create account link on the
login page, and the server-rendered header — including the one on the
read only error page itself. Account creation is blocked in both modes,
so all of them were dead ends. The banner made it worse by describing
what members lose (replying, likes) rather than what an anonymous
visitor is actually unable to do.
This change gates the JS `canSignUp` getter and its Ruby twin
`can_sign_up?` on read only state, which covers every signup entry point
at once. `canSignUp` had to drop its `@computed` decorator to do so:
with no dependent keys it cached permanently, so the new term would have
been evaluated once at boot and then frozen — leaving the button visible
if read only mode started later, and hidden long after it ended.
The "Log In" button deliberately stays, because staff can still log in
during staff only mode. Anonymous visitors now get banner copy naming
signup and login instead, a refused login says only staff can log in
rather than claiming login is disabled outright, and the email and code
login forms show that inline instead of a generic "an error occurred"
dialog.
It also fixes a pre-existing blank page. Both `/login` and `/signup`
aborted the transition when read only, which on a direct URL load left
the application template unrendered — and with it the dialog holder, so
the explanation never appeared either. They now redirect home when there
is no route to stay on, and keep aborting when there is.
Meta ref: /t/408703
Previously, a markdown link in an event's location rendered correctly in
email notifications but showed as raw `[text](url)` markup on the event
card. Every surface (card, email, topic excerpt) re-implemented its own
rendering of the same fields, and `extract_events` HTML-escaped the
stored location one level further on every edit.
This change cooks `location` and `description` once, server side, with a
restricted inline pipeline (links and emoji only, so onebox embeds
cannot return), and every surface renders through that shared cook — the
card via new `location_html`/`description_html` serializer attributes.
It stops escaping the stored location, with a migration healing existing
rows, and moves the composer's `[event]` parsing onto core's
`parseBBCodeTag` so quoted attribute values containing `]` survive
round-trips.
Ref - t/188447
Previously, the header search field appeared as soon as the welcome
banner was no longer fully visible, leaving both search bars on screen
and interactive at once; since they share one search service, a single
search could show its results in both panels, the banner's open results
panel could overlay and swallow clicks aimed at the header field, and
typing into the superseded banner input ran searches with no visible
results.
This change bases the switch on the banner's search bar itself: the
banner remains the only search UI while any part of its input is below
the header, and the header search replaces it exactly when the input is
fully tucked away — with focus and the open results panel following the
handoff in both directions, stray focus on the superseded input
redirected to the active one, and hidden menus closed so a dismissed
panel stays dismissed.
Note the intentional behavior change: the header search field now
appears slightly later when scrolling (once the banner search bar is
fully under the header, rather than on the first pixel of scroll).
Internal topic: t/188511
Previously, on sites that require staff to approve new members,
moderators were not reminded about people waiting for approval until 8
hours (480 minutes) after they registered, leaving new members waiting
far longer than necessary.
This change adds an `update_pending_users_reminder_default` upcoming
change that lowers the `pending_users_reminder_delay_minutes` default to
30 when enabled, so staff are notified promptly. It uses the virtual
default-override mechanism rather than writing to the DB, so an admin
who has customized the setting keeps their value and disabling the
change restores the previous default. The change is only surfaced on
sites with `must_approve_users` enabled, since the reminder never fires
otherwise.
## Summary
- When `edit_history_visible_to_public` is disabled, category group
moderators using the review panel get an `invalid_access` error when
viewing edit history for posts in their moderated categories
- Allows category group moderators to view edit history for posts in
categories they moderate
- Updates the `edit_history_visible_to_public` setting description to
reflect this change
- Adds guardian specs plus request specs verifying category group
moderators can only view revisions in their moderated categories, and
that hidden revisions remain staff-only (both on the revisions endpoints
and the `?version=` post endpoint)
Based on #37025 by @small-lovely-cat.
Co-authored-by: small-lovely-cat <77799160+small-lovely-cat@users.noreply.github.com>
Previously, `sync_sso` rebuilt its query string from the already
form-decoded `params[:sso]` and let `DiscourseConnect.parse` decode it a
second time, so any literal `+` in the base64 payload became a space and
a correctly-signed request was rejected with a generic 422 "Login
Error".
This change rebuilds the query with `Rack::Utils.build_query` — the
exact inverse of the `parse_query` the parser runs — so the payload is
decoded exactly once. The second commit applies the same reasoning to
`CookedPostProcessor#remove_user_ids`, which rebuilt link query strings
by hand and corrupted encoded values (e.g. `%26`) when stripping the
`u=` param.
Reported at https://meta.discourse.org/t/407426.
Previously, the duplicate topic title check compared new titles against
every topic on the site regardless of visibility: users were blocked by
titles in categories they couldn't see or unlisted topics they couldn't
find, the bare "Title has already been used" error gave no way to locate
the conflict (and doubled as an existence oracle for hidden titles), and
the behavior was controlled by two entangled boolean settings.
This change scopes the check to the destination category plus whatever
the acting user can actually see, links the conflicting topic in the
error — safe by construction, since being blocked now implies being able
to see it:
> This title has already been used by [another topic]().
It also consolidates the two booleans into a single
`duplicate_topic_titles` enum (`disallowed` /
`allowed_across_categories` / `allowed`), with existing values migrated
and the old names kept as hidden deprecated aliases that admin search
still resolves.
Reported in
https://meta.discourse.org/t/title-has-already-been-used-in-a-secure-category/123047
Note for self-hosters: env-provided settings can't be migrated —
`DISCOURSE_ALLOW_DUPLICATE_TOPIC_TITLES=true` configs need to switch to
`DISCOURSE_DUPLICATE_TOPIC_TITLES=allowed`.
Previously, public poll voters were displayed alphabetically by username
while pagination selected them chronologically — a regression from the
ranked choice feature (bae492efee) — so each "show more" appended an
alphabetically-sorted chunk of 25 and the overall list read as random.
This change orders voters by the same `ROW_NUMBER()` over `created_at`
that already drives pagination, restoring the pre-2024 chronological
display and aligning display order with page selection. A `user_id`
tiebreaker makes the order deterministic when votes share a timestamp
(e.g. bulk imports), and the now unused `username` column is dropped
from the query. The specs previously sorted voters by id before
asserting — which is how the regression went unnoticed — and now assert
the actual order, with vote order deliberately different from id order
so any alphabetical or id-based ordering fails.
Previously, 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
Previously, a tag whose only usage was in personal messages was silently
dropped by `TagsController.tag_counts_json` — a display rule from 2020
meant to keep such tags off the `/tags` browse page for users who cannot
tag messages (and `pm_tags_allowed_for_groups` has no staff bypass, so
by default that includes admins). Every surface reusing that method as a
plain serializer inherited the rule by accident:
- the composer tag search treated the missing row as unauthorized and
showed the tag disabled with a bogus **"Can't be used in this
category"** reason (the reported bug),
- every "show all tags" chooser (tag groups, synonyms, watched tags,
category allowed tags, webhooks, automations, …) silently refused to
offer such tags at all,
- the `#` autocomplete would not suggest a tag that nonetheless cooked
into a working hashtag link when typed in full.
This change makes `tag_counts_json` a pure serializer and moves the rule
into an explicit, named helper (`DiscourseTagging.without_pm_only_tags`)
applied only where it belongs — the `/tags` browse lists — with an
exemption for the admin "show all tags" view so the admin inventory is
complete. Selection and search surfaces now offer every tag the user is
allowed to use, and tag-group visibility rules still apply everywhere.
It also fixes two adjacent inconsistencies uncovered along the way:
- **Topic→message conversion counter drift.** Converting only adjusted
`public_topic_count`, so a converted topic's tags kept working until the
periodic consistency job recounted them into the broken state — the
"worked at first, broke a day later" in the report. The converter now
moves all three counters immediately, and rolls back cleanly when the
underlying post revision fails (its return value was previously ignored,
and `Topic#valid?` clears the errors it adds, so a failed conversion
still applied its side effects).
- **Crawler/print tag leak.** The crawler layout leaked a message's tag
names in the page title and `og:article:tag` metadata to participants
the serializer already hides tags from; both now flow through
`TopicView#visible_tags`, gated on `guardian.can_see_tags?`.
Reported in https://meta.discourse.org/t/407050
Previously, picking a group in the bulk assign modal always failed with
a bodyless 500: the client only sent `username`, `group_name` was not a
permitted bulk action parameter, and the bulk operation only ever
resolved a `User` — so `Assigner` received `nil`, treated it as a
`Group`, and crashed dereferencing it.
This change routes both the bulk and single-topic paths through one
`DiscourseAssign::AssigneeResolver`, makes `Assigner#assign` return a
reason instead of raising for caller-supplied input, and surfaces
per-topic refusals through `@errors` so a partially applied bulk assign
explains itself instead of reporting success.
Ref - t/188206
Previously, a member who turned chat off in their preferences saw their
existing chat notifications render as blank rows, and any chat link
silently bounced them to the homepage.
This change registers the chat notification renderers whenever chat is
enabled site-wide so those notifications still render correctly, and
sends chat links to a new "chat is disabled" page that explains how to
turn it back on.
**BEFORE**
(the notification are "blank")
<img width="1400" height="1200" alt="before-notifications"
src="https://github.com/user-attachments/assets/d8dcef2e-98eb-4b30-bf33-4a1f8c2c4626"
/>
**AFTER**
(the notification are there)
<img width="1400" height="1200" alt="after-notifications"
src="https://github.com/user-attachments/assets/72b17123-8f82-4c5e-a74d-7d5347de623a"
/>
(the "blank slate" page that is displayed when you click a #chat
notification after you've disabled #chat in your preferences)
<img width="1400" height="1200" alt="after-disabled-page"
src="https://github.com/user-attachments/assets/1905ecb5-7b8c-4682-87a9-1559ca4ec569"
/>
Previously, `PostMover` detected a full merge by comparing the sizes of
two differently-filtered post sets — a topic-wide census (`regular OR
(whisper AND action_code != 'split_topic')`, with no content filter and
a NULL-unsafe `!=`) against the movable set (which excludes
`small_action` and blank-`raw` posts). Any post the two filters
classified differently threw the counts off, so a topic that had ever
been assigned (its blank-`raw` tracking whisper is counted but never
movable) would never close on a full merge, was never scheduled for
deletion, and — via discourse-topic-voting, which only transfers votes
once the source topic closes — left its votes stranded. The same
asymmetry ran the other way for ordinary content whispers (dropped from
the census by the NULL comparison), so one left behind could wrongly
close and delete the source with its content still inside.
This change computes `@full_move` directly as "no close-preventing post
is left out of the move" — a set difference between the close-preventing
posts and the moved posts — using a single NULL-safe predicate
(`regular`/`whisper`, `raw <> ''`, `action_code IS DISTINCT FROM
'split_topic'`). Fully merged topics now reliably close and transfer
their votes, while a topic still holding whisper content correctly stays
open.
Previously, the change tracker reached directly into each setting's
buffered proxy, which couples the "unsaved changes" banner to one
specific row implementation — and hid two bugs: the banner's save-all
never triggered the live page refresh for reload-requiring settings (it
read `setting.afterSave`, which nothing ever assigns), and every setting
row subscribed to a backfill MessageBus channel because `if
(this.canSubscribeToSettingsJobs)` tested a method reference instead of
calling it.
This change narrows the tracker to a small handle interface
(`pendingValue` / `commit()` / `rollback()`) so that FormKit-based rows
can register alongside buffered rows during the site settings conversion
(t/179889), makes save-all refresh fonts/logos live like a row-level
save does, subscribes only `default_categories_*`/`default_tags_*` rows
to job progress, and keeps theme rows out of the tracker so an abandoned
theme edit can no longer leak into the site settings bulk save.
Previously, the shared `SettingDefinitionField` renderers lacked
behaviors their legacy `site-settings/*` counterparts enforce — most
notably `group_list` skipped the everyone/logged_in_users aliasing and
`disallowed_groups`/`mandatory_values`, so the AI feature settings page
could silently store the wrong group.
This change ports those behaviors into the shared renderers — group
aliasing (extracted to `discourse/lib/group-list-setting-aliasing` so
there is a single implementation), `compact_list` custom entries /
`allow_any` / `mandatory_values`, `enum` none-gating and non-string
value matching, and the `category_list` concurrent-lookup guard — so
they can back the admin site settings pages next (t/179889).
Previously, affiliate link rewriting only ran via the
`:post_process_cooked` event, which fires exclusively when baking a
post's original cooked HTML — so readers viewing a translated post were
served untagged links.
This change subscribes the same rewrite to the
`:post_process_localized_cooked` event fired by
`LocalizedCookedPostProcessor`, so translated posts get affiliate links
too. Existing localizations pick up the tags on their next rebake, since
the recook path re-runs the localized post processor.
Reported at https://meta.discourse.org/t/407688
Previously, when a site behind a bot-protection service answered a
onebox fetch with a JavaScript challenge — IMDb, for example, currently
returns `202 Accepted` with an AWS WAF challenge page — the preview
showed the baffling "the web server returned an error code of 202", a
success code presented as an error with no hint of the actual cause.
This change detects the documented challenge headers (AWS WAF's
`x-amzn-waf-action` and Cloudflare's `cf-mitigated`) in
`FinalDestination` and shows a message explaining that the site requires
visitors to pass a verification step in a web browser. Accepting 2xx
statuses beyond 200 instead would not help: the challenge response
carries no OpenGraph data, so the onebox would just come out silently
blank.
Reported in https://meta.discourse.org/t/407725
**BEFORE**
<img width="726" height="605" alt="2026-07-16 @ 09 51 32"
src="https://github.com/user-attachments/assets/929b116d-d07a-4aca-a655-6c4b1d0db607"
/>
**AFTER**
<img width="718" height="685" alt="2026-07-16 @ 09 52 10"
src="https://github.com/user-attachments/assets/7b555557-6cb4-40ac-a7dc-32bf2515e357"
/>
Previously, the admin dashboard rendered `{{setting:...}}` references in
problem messages as raw text, because `AdminNotice#message`
re-translated each notice without expanding the markers and its
sanitizer stripped the `class`/`data-setting-*` attributes the frontend
needs to link each setting.
This change expands the markers at that render seam (widening the
sanitizer allowlist via a shared constant on
`SiteSettings::LabelFormatter` so it can't drift from what `linkify`
emits) and applies the `linkifySettingLinks` modifier in both the
classic and redesigned dashboards, so setting references resolve to
their own config pages.
Ref - t/148687
**BEFORE**
<img width="1958" height="1380" alt="2026-07-16 @ 09 02 12"
src="https://github.com/user-attachments/assets/4a83deba-ae10-40e3-a91b-ca66d5178c08"
/>
**AFTER**
<img width="1958" height="1380" alt="2026-07-16 @ 09 01 53"
src="https://github.com/user-attachments/assets/8cd71db8-95d7-445a-9dde-f49af21b3446"
/>
Previously, the upcoming-events sidebar block and the category events
calendar ignored a category's "subcategories"/"no subcategories" filter
— the block was locked to a static per-site parameter, and the calendar
always included subcategory events.
This change derives subcategory inclusion from the current category
route (matching core's `includeSubcategories === !noSubcategories`), so
both surfaces show exactly the events the list's filter implies.
Reported at
https://meta.discourse.org/t/have-the-upcoming-events-block-respect-the-subcategory-filter-when-in-category-lists/407540
### Why
Saving `github_linkback_access_token` validates the token against every
repository listed in `github_badges_repos`, and saving
`github_badges_repos` validates the format of each entry. When one entry
failed, both validators returned a generic error:
- _"You must provide a valid GitHub linkback access token which has
access to the badge repositories you have provided."_
- _"You must provide a GitHub URL or the repository name in the format
github_user/repository_name"_
Neither message said **which** repository was at fault. On a site with a
long list of repositories, an admin had no way to identify the offending
entry from the UI and had to run a console script to find it.
### What changed
Both validators now capture the first failing entry and interpolate its
name into the error message, so the problem repository can be fixed
straight from the settings page, e.g.:
> The GitHub linkback access token could not access the
'acme/private-repo' repository. Make sure the token is valid and that
the repository name is correct and accessible.
> 'not a repo' is not a valid GitHub URL or repository name. Use the
format github_user/repository_name.
The generic messages are kept as a fallback.
While reworking the token validator, a repository that is missing or
private (GitHub answers both with a `404`) was previously left unrescued
and turned the save into a `500`. That case is now treated as a
validation failure, alongside an unauthorized (`401`) token, and
reported with the same repository-naming message.
### Testing
Added specs covering the `401` and `404` paths for the token validator
and the "name the first invalid entry" path for the badges-repo
validator, plus the generic-message fallback for both.
Previously, S3 backups and uploads larger than the multipart threshold
crashed with `undefined method 'downcase' for nil` whenever
`AWS_REQUEST_CHECKSUM_CALCULATION` was set to `when_required` (a common
setup for S3-compatible providers such as Cloudflare R2, Backblaze B2,
and MinIO), because of a bug in `aws-sdk-s3` 1.182.0's multipart
uploader.
This change bumps `aws-sdk-s3` to 1.227.0 — which respects
`when_required` in multipart uploads — and adapts to its API changes:
moving off the newly-deprecated
`Aws::S3::Object#upload_file`/`#download_file` onto
`Aws::S3::TransferManager`, and reading the ETag back from the
destination now that multipart copies no longer return a response.
When topic voting vote limits are disabled, the "Notify me about new
posts"
prompt shown after voting never appeared. That prompt is part of the
menu the
vote button opens, but the menu was only rendered when vote limits were
enabled — with limits off the button had no menu at all, so both the
notification prompt and the remove-vote action were unreachable.
The fix renders the menu for any signed-in user rather than gating it on
vote
limits. The remaining-votes rows stay tied to limits being enabled,
while
remove-vote and watch-topic show whenever the user has voted. Removing a
vote
with limits disabled now goes through the menu, matching the behavior
when
limits are enabled.
Reported at
https://meta.discourse.org/t/disabling-vote-limits-seems-to-break-notification-prompt-in-topic-voting/407512
The second commit is follow-up housekeeping on the plugin, kept separate
from
the fix: removing dead code and duplication in the vote components and
initializers, counting a user's votes with `COUNT` instead of loading
every
row on each current-user serialization, and deleting orphaned i18n keys
and a
route-map filename left over from the plugin's former name.
### Testing
- Added a system spec covering the notification prompt appearing after
voting
with vote limits disabled.
- Existing topic-voting system and JS specs pass; the model spec still
covers
the trust-level-0 lock via `reached_voting_limit?`.
Previously, the tag picker shown when creating or editing a topic only
suggested the forum's most-used tags — which on many sites are dominated
by automated-topic tags and rarely match what a member actually reaches
for.
This change adds an experimental `prioritize_recently_used_tags`
upcoming change that surfaces the tags a member has recently used on
their own topics (from their last 10 created topics) at the top of the
picker, falling back to the popular tags for members with little
history.
Previously, the title on the not-found/private (404) and forbidden (403)
pages was resolved using the site's default locale while the rest of the
page used the visitor's interface language, so the heading could appear
in a different language than the rest of the page.
This change resolves the title in the same locale as the page it appears
on — on both the full-page render and the SPA-injected error panel
(`topics#show` / `categories#find_by_slug`) — so the whole page renders
consistently.
Régis HanolandSam Saffron
