What is the problem?
Discourse allows admins to mark a tag as a synonym of another tag. For
example, "brunch" can be made a synonym of "lunch". When this happens,
all topics tagged with "brunch" are automatically retagged with "lunch",
and the `tags.target_tag_id` column on the synonym tag record is set to
point to the target tag.
However, several code paths did not account for synonyms:
1. **Search:** `Search#search_tags` and the hashtag advanced filter did
not resolve synonyms. When a user searched using a synonym name (e.g.
`tags:brunch`, `tags:brunch+eggs`, or `#brunch`), no results were
returned because:
- The `tags:` comma path queries `topic_tags` joined with `tags` by
name, but topics are tagged with the target tag "lunch", not the
synonym "brunch".
- The `tags:` plus path aggregates tag names per topic into a
tsvector and matches against the searched name, but the aggregated
names are target tag names, so "brunch" never matches.
- The `#` hashtag path picks the synonym tag's own `id` and queries
`topic_tags` by that ID, but topics store the target tag's ID.
2. **Filter route:** `TopicsFilter#tag_ids_from_tag_names` concatenated
both the synonym's own ID and the target tag ID. For match-all
queries (e.g. `tag:brunch`), this required a topic to have both IDs
in `topic_tags`, which never happens since only the target tag ID is
stored. For negation queries (e.g. `-tag:brunch`), the exclusion
targeted the synonym ID rather than the target, so no topics were
excluded.
What is the solution?
In `Search#search_tags`, add a synonym resolution step before the
existing comma/plus branching logic. It splits the match string into
individual tag names, queries for any that are synonyms via
`Tag.where_name(tag_names).where.not(target_tag_id: nil)`, builds a
name mapping, and replaces synonym names with their target tag names.
The replacement operates on the split array elements rather than using
substring replacement to avoid corrupting tag names that may contain
other tag names as substrings.
In the hashtag advanced filter, pick both `:id` and `:target_tag_id`
from the tag lookup and prefer `target_tag_id` when present, so the
query uses the target tag's ID instead of the synonym's own ID.
In `TopicsFilter#tag_ids_from_tag_names`, replace the transpose/concat
approach with `.map { |id, target_id| target_id || id }` to resolve
each tag to its canonical ID — the target for synonyms, or the tag's
own ID otherwise.
A partial index on `tags.target_tag_id` is added (scoped to
`WHERE target_tag_id IS NOT NULL`) to support efficient synonym lookups.
In https://github.com/discourse/discourse/pull/36678 we introduced the
`tag_ids` param for topic creation and updating to add tags to a topic,
however, this does not cater for new tags which are allowed based on the
UI and previous API.
Furthermore, the frontend wasn't even using the new param, making the
new `tag_ids` implementation on the controller moot.
This commit
- removes this `tag_id` param
- updates the deprecation note to the `tags` param to require `[{ id: 1,
name: "old1" }, { name: "new1" }]` rather than the old `["old1",
"new1"]`.
- including IDs here eliminates the case where John renames `"old1"` to
`"old2"`, but Mary sends `["old1", "new1"]`, thus creating the `"old1"`
tag again when it was already renamed.
- keeps us consistent with other endpoints that take in `tags`
Reviewer note: The `tag_topic_by_names` method is [extremely
complicated](https://github.com/discourse/discourse/blob/2b24fc91c1f46e57913d6acb7aee0c05041f64be/lib/discourse_tagging.rb#L19-L248).
Ideally, we would like to have a `tag_topics` method that does not incur
that many `Tag` queries, but to prevent a large refactor here, we are
re-using `tag_topic_by_names`.
The /drafts endpoint returns a 500 error when any draft contains HTML
with excessive nesting depth or too many attributes per element.
Nokogiri::HTML5.fragment raises ArgumentError when these limits are
exceeded, and PrettyText.excerpt had no error handling for this. A
previous fix in PostItemExcerpt only caught the tree depth variant,
leaving the attributes limit unhandled, and only protecting one of the
13+ callers.
Rescue ArgumentError around the Nokogiri::HTML5.fragment call in
PrettyText.excerpt and return "" on failure. This is consistent with the
existing blank-input guard and protects all callers at once. The
now-redundant rescue in PostItemExcerpt is removed.
Ref - t/173858
MiniMime.lookup_by_filename can return nil if the file extension is not
recognized. The move_to_final_destination method was calling .content_type
directly on the result without safe navigation, causing NoMethodError when
processing backup files with unrecognized extensions.
This is the same bug pattern that was fixed in e817cf6f47 for
BackupRestore::Creator. The fallback to "application/gzip" is appropriate
since this method is used for backup uploads which are .tar.gz or .sql.gz.
Background: When using form-templates, the controls on any uploaded
images (edit title/delete image) were not working.
Details:
- Enabled modifier forwarding in the preview component so initialization
hooks run correctly when the preview is rendered directly.
- Hardened preview initialization logic to support cases where the
preview wrapper itself is the root element, ensuring image action
listeners attach reliably.
- Kept composer markdown in sync during preview updates, allowing image
edit/delete actions to correctly locate and modify the underlying
markdown.
- Moved image replacement handling to the input components themselves,
ensuring the correct field updates when preview actions occur.
- Resolved a race condition during preview refreshes by deferring
updates until after the UI has fully rendered.
- Synchronized upload state with preview deletions, keeping the uploaded
files list consistent when images are removed.
Together, these changes ensure image controls in Form Template previews
behave consistently with the standard composer and no longer fail or
revert unexpectedly.
Bug report:
https://meta.discourse.org/t/topic-content-does-not-take-up-full-width-for-very-short-text/395371
With Foundation theme, when screen width <= 924px, .container.posts has
`grid-template-columns: auto` (see
app/assets/stylesheets/common/base/topic.scss). But in Horizon theme, it
is overriden with `grid-template-columns: auto auto`. However there is
not another column here and it makes the child `.row` fail to take up
full width if its content is smaller than screen width.
Turns out SMIL animations in SVGs get paused by javascript execution,
and therefore can appear very stuttery! (thanks @davidtaylorhq!)
So this removes the various pieces that allowed SMIL animation, and
switches animated SVGs to be directly part of the DOM. This way CSS
transformation and opacity animations end up in the compositor layer and
aren't blocked by JS, making them less prone to stutter.
https://github.com/discourse/discourse/commit/c35e7366 improved handling
of pasted tables in the ProseMirror rich editor by normalizing column
counts and supporting nested tables. However, it introduced a potential
crash when pasting tables with empty tbody elements or rows with zero
cells.
This PR fixes this edge case.
On the rich editor automatic download of base64 image data, image
positions retrieved from `dataURIMap` are now sorted in descending order
before replacement. This prevents issues when replacing multiple images
with the same data URI by ensuring that later positions are replaced
first, avoiding offset errors.
`GitUtils` determines the Discourse version information which is shown
in the user interface. This commit introduces an optional
`config/git-utils-overrides.json` file, which hosting providers can use
to override this information. For example, if the hosting provider is
needs to apply hosting-platform-specific patches or embargoed security
fixes on top of a normal release branch.
Previously there were no specs for this file. So this commit introduces
specs for the default behaviour, and the new overrides feature.
Bulk tags feature introduced in
https://github.com/discourse/discourse/pull/36645 was lower-casing tags
being sent, regardless of the `force_lowercase_tags` setting.
This commit ensures the lowercase is done only when that setting is enabled.
This is a follow up to the removal of chat action buttons for non
channel members in #37060
Previously we allowed non channel members to react to posts, now the
user can still remove reactions they left in the past (if they were once
a channel member and then left), but attempting to add new reactions via
the API or by clicking other user reactions to increment them would
result in an error message stating that you need to join the channel.
From the API request to add a reaction they would get a 403 response
code with a message `You must be a member of this channel`.
If clicking an existing reaction from the UI they will get an error
dialog:
<img width="550" alt="When non members click existing reactions"
src="https://github.com/user-attachments/assets/57b7a205-e5b7-426d-a64d-19dd04803686"
/>
Internal ref - /t/160400
What is the problem?
When creating or editing a tag group, users can type new tag names into
the `TagChooser` (which has `allowAny=true`). The select-kit component
assigns a string-based ID to these new tags. `TagGroupsController#tag_groups_params`
blindly extracts `id` from each tag object, producing the string name
instead of a database ID for new tags. `TagGroup#tag_ids=` then silently
ignores these invalid values, so the new tags are dropped without error.
The same issue affects parent tags.
This was introduced in 9e99066b07 which changed tags from string arrays
to object arrays but did not handle the case where new tags have no
numeric ID.
What is the solution?
On the frontend, the `TagGroupsForm#save` action now strips non-numeric
IDs from tags and parent tags before sending to the backend via a private
`#serializeTag` method. Tags with a numeric `id` (existing) keep both
`id` and `name`; tags with a string `id` (new) are sent with only `name`.
On the backend, `TagGroupsController#tag_groups_params` now splits the
tags array into existing (have `id`) and new (no `id`). New tag names are
passed to `DiscourseTagging.find_or_create_tags!` which validates names via
`tags_for_saving` and creates them. The same logic applies to parent
tags.
After editing a category and then attempting to create a new one,
sometimes you'd end up back to the one you were editing when clicking
tabs in the navigation. This resets the data to avoid the issue.
Follow-up to #37540. This continues the code cleanup work for the
upsert-category components.
Changes:
- consolidate 8 upload actions into a single pair using fn partial
application to reduce repetition
- move appearance-related properties (views, sorts, periods, filters,
subcategory list styles) from settings to appearance where they actually
belong
- use site.periods as the single source of truth for top periods instead
of hardcoding values
- add none option to ComboBoxes so values can be cleared back to default
- remove enable_simplified_category_creation guard from text_color and
slug fields since they should always be available
- remove unnecessary placementStrategy="absolute" from ComboBoxes
- add missing "hot" to available default views
- pass form to category-email-in plugin outlet so plugins can use it
- rename properties for consistency (eg. availableSorts → sortOrders,
parentIsRestricted → isParentRestricted)
Ref - t/173336
Two issues addressed:
1. Require icon selection when style type is "icon" - previously the new
category interface allowed creating categories without an icon, unlike
the old interface which required one.
https://meta.discourse.org/t/395223
2. Reset security settings when navigating from edit to new category -
the categoryVisibilityState was being reused across different
categories, causing the security tab to show the previous category's
settings.
https://meta.discourse.org/t/395224
The sidebar plugin API documents `hoverType`, `hoverValue`,
`hoverAction`, and `hoverTitle` on `BaseCustomSidebarSectionLink`, but
these only work when using `route`. Links using `href` silently ignore
these properties because the template's href branch was never updated to
include them.
This affects plugins that need sidebar links which don't navigate (e.g.,
triggering actions on click) but still want hover menus like chat has.
### Why href?
Some sidebar links can't use `route` because clicking them doesn't
navigate anywhere - they trigger actions (like joining a voice room,
toggling a state, etc). These use `href="#"` with a click handler
intercept. The API suggests hover features should work, but they don't.
### The lint-disable
The `route` branch already has a button inside `LinkTo` (which renders
as `<a>`). The linter doesn't flag this because it only sees the
component, not the rendered output. Adding the same pattern to the
`href` branch requires the disable comment for parity.
An alternative would be moving the hover button outside both link
elements, but that changes the DOM structure and breaks existing styles.
I think the original intent was to only target directional icons, but
things fall through the cracks (see
https://meta.discourse.org/t/reverse-reply-arrow-icon-for-rtl/385565) so
this commit aims to:
* apply a catch-all to icons
* use `transform: scaleX(-1);` which provides a more correct horizontal
flip
As a downside, the transform now gets applied to all icons, but this
should come at a negligible performance cost.
- don't rely on cached, it's almost never necessary
- rely on default form-kit behavior where possible
- prefer concatClass to getters when possible to have the css directly
in the template
When a color scheme has a base_scheme, the base scheme's
color_scheme_colors were not being eager loaded, causing strict loading
violations.
This updates the includes statement to properly load the nested
association `base_scheme: :color_scheme_colors` instead of just
`:base_scheme`.
Also adds a test that creates color schemes with base_scheme
relationships to verify the associations are properly loaded without
strict loading errors.
The sidebar becomes a floating-overlay element at size = md, so by
updating the grid layout we avoid an invisible element interfering with
grid gap properties.
This shouldn't change anything visually and have no theme-impact afaict.
This is a follow-up to fd04f690.
The original commit only protected potentially illegal reviewables from
being auto-approved in one code path (direct post deletion). However,
when deleting a post through a reviewable action with
`notify_users_after_responses_deleted_on_flagged_post` enabled, the code
would still call `#ignore` on any flagged reply posts without checking
`#potentially_illegal?`.
Technically there appear to also be a two and many... but this at least
removes some of the bug.
```
def wrong(n)
n != 13 || n != 14
end
def right(n)
n != 13 && n != 14
end
[13, 14, 15].each do |n|
puts "#{n}: wrong=#{wrong(n)}, right=#{right(n)}"
end
```
15 is (not 13) OR (not 14)
15 is not (not 13) AND (not 14)
apologies for the headache.
Discourse Translator Bot
Alan Guo Xiang Tan
Natalie Tay
Kris
Régis Hanol
Jordan Vidrine
Jarek Radosz
Jake Goldsborough
Bannon Tanner
方觉
Renato Atilio
David Taylor
David Battersby
Martin BrennanandLoïc Guitaut
chapoi
Rafael dos Santos Silva
Joffrey JAFFEUX
Osama Sayegh
Sam