Related:
https://meta.discourse.org/t/yet-another-title-localization-issue/395469
### bug context
tldr; `Topic#fancy_title` saves a fancy_title to db when the fancy_title
is null.
This bug requires a certain incantation to trigger.
- Topic 395465 exists with `title = "Notification level button always
says \"tracking\""` and
`fancy_title = NULL` in the DB
- A `TopicLocalization` exists for this topic in `zh_CN`
- `content_localization_enabled` is on
When a crawler hits `GET /t/.../395465?tl=zh_CN`, localization
replacement happens on the topic, which writes the title attribute, so
the state is now
- `title` = Chinese (modified)
- `fancy_title` = NULL (untouched, still what was loaded from DB)
When serializing via `TopicViewSerializer` which uses
`LocalizedFancyTopicTitleMixin`, we call `topic.fancy_title`. The
`topic#fancy_title` generates the fancy_title from the title value, then
writes the chinese fancy title to db 😢https://github.com/discourse/discourse/blob/e935ed63b28a30ee7ae6a7783ae05fe33edf3367/app/models/topic.rb#L532-L545
### fix
This commit fixes the issue by ensuring the fancy_title is always written
along with the title, preventing the need for invoking
`topic#fancy_title`.
When multiple replies are collapsed into a single notification,
`PostAlerter` sets `display_username` to a translated reply count (e.g.,
"7 réponses") for use in the notification dropdown UI. The
`original_username` field always holds the actual poster's username.
However, if `original_username` is ever missing from the notification
data, `notification_email` falls back to `display_username`, which
causes the reply count string to be used as the email sender username.
This leads to errors during email rendering.
This updates the fallback chain in `notification_email` to prefer
`post.user.username` over `display_username`, ensuring a real username
is always used even when `original_username` is absent.
https://meta.discourse.org/t//395420
The sidebar "New category" action and the `/new-category` route were
checking `currentUser.admin` directly, which excluded moderators even
when the `moderators_manage_categories` site setting was enabled.
This is inconsistent with the backend Guardian which already allows
moderators to create categories when that setting is on.
Added `can_create_category` to `CurrentUserSerializer` (delegating to
`Guardian#can_create_category?`) and updated the sidebar component and
the new-category route to use it instead of manual role checks.
Ref - https://meta.discourse.org/t/395441
When pressing B then Enter to bookmark a topic, the bookmark icon next
to the topic title and the topic footer bookmark button did not update
until page reload.
The keyboard shortcut path opens the BookmarkModal directly via the
topic controller, bypassing the TopicBookmarkManager that the footer's
BookmarkMenu component relies on. The afterSave callback correctly sets
`topic.bookmarked = true`, but the `topicBookmarkManager` computed
property only depended on `"topic"` (the object identity), so it never
recreated the manager to pick up the new bookmark state.
Changed the dependent key to `"topic.bookmarked"` so the manager is
recreated whenever the bookmarked property changes, regardless of which
code path triggered it.
## Summary
The original commit added a new Discourse ID settings page with the
ability to enable/disable Discourse ID and regenerate credentials.
However, the `update_settings` action used direct assignment to
`SiteSetting.enable_discourse_id` which bypasses the staff action
logging mechanism, meaning changes to this setting were not being
recorded in the admin logs.
Followup 42da6860fd
When the upcoming change for "Impersonate without logout" was added, we
didn't take into account that the "stop impersonation" action would not
work if the Staff enabled option for the upcoming change was used.
This was happening because in the ImpersonateController#destroy action
we werecheckingif the current user had `impersonate_without_logout`
enabled, but we should be checking if the acting user had that
permission instead (i.e. the original admin not the user they are
impersonating)
c.f.
https://meta.discourse.org/t/new-bug-with-experimental-impersonation-interface/395621
This allows us to quickly link from the upcoming changes page
to any related settings, since sometimes after the change is enabled,
more settings may need to be configured. The current need for this
is the `enable_custom_splash_screen` setting.
We can use the status of upcoming changes to indicate
whether they are experimental or not, having experimental
in the setting name is redundant.
Migrates the settings and the upcoming change events,
updates code, and updates yaml translation keys.
Both the allowed_tags and allowed_tag_groups fields were not
correctly saving in the Tabs form when simplified category creation
was enabled. This commit fixes the issue and adds a spec to cover
it.
The UserSerializer exposes post_count as a staff attribute but does not
expose topic_count, even though the underlying data is available on the
UserStat model. This means consumers like the Discourse AI tool API's
discourse.getUser() function can retrieve how many replies a user has
made but not how many topics they have created, giving an incomplete
picture of user activity.
Add topic_count as a staff attribute in UserSerializer, mirroring the
existing post_count implementation. Also add spec coverage for all four
staff attributes (post_count, topic_count, can_be_deleted,
can_delete_all_posts) to verify they are included for staff scopes and
excluded for non-staff scopes.
Ref: https://meta.discourse.org/t/394878
When an admin deletes a user and blocks their email/IP, only primary
and secondary emails (from `user_emails`) were added to the screened
emails list. OAuth/social login emails (Google, Facebook, GitHub, etc.)
stored in `user_associated_accounts` were not blocked, allowing the
deleted user to re-register from a different IP using their OAuth
account.
Similarly, when anonymizing a user, invite cleanup, incoming email
deletion, and screened email IP anonymization only considered the
primary email, missing associated account emails entirely.
Collect emails from `user_associated_accounts.info['email']` alongside
`user_emails` using array union (`|`) for natural deduplication:
- `UserDestroyer`: block and clean up invites for all emails
- `UserAnonymizer`: capture associated emails before they are destroyed,
pass them to the `AnonymizeUser` job
- `AnonymizeUser` job: use all emails for invite, incoming email, and
screened email cleanup
Ref - t/173323
Introduces a minor accessibility improvement to the radio button group
in the wizard component and adds a corresponding system test to ensure
correct keyboard navigation behavior.
Ensures that no `ProblemCheckTracker` records are deleted when there are
no current targets (`not: []` will resolve to `1=1`) and adds a test to
verify this behavior.
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
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.
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.
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
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.
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.
When editing a topic via the composer, tags with parent tag requirements
weren't appearing in search results, even though the parent tag was
already selected on the topic. When the composer opens for editing, it
was using these serialized string tags. The mini-tag-chooser couldn't
extract tag IDs from strings
This commit fixes that. Also adds a system spec testing the composer and
title header scenarios.
The original commit added anchor fragment enhancement for onebox
descriptions, reusing the `find_anchor_target` method. However, this
method had two bugs: a broken string escaping pattern that corrupts
fragments containing single quotes, and a reference to an undefined
`CSS.escape` constant that would raise a `NameError` at runtime.
The original commit moved the topic merging operation into a `hijack`
block to avoid Unicorn worker timeouts when merging topics with many
posts. However, it omitted error handling that exists in the similar
`move_posts` method, which could lead to 500 errors with empty bodies in
production when `ActiveRecord::RecordInvalid` or
`ActiveRecord::RecordNotSaved` exceptions occur.
Without this we will get no information about why merging did not work
The 't' shortcut (in:title) modifies where to search but still
requires actual search terms to produce meaningful results. Unlike
'l' (latest) and 'r' (recent) which return results on their own,
't' alone should enforce the minimum search term length.
Renames `valid_search_shortcut?` to `min_length_bypass?` to better
describe the method's purpose.
When creating a new category and selecting a restricted parent category,
the visibility should automatically switch to "Private" mode.
Previously, if a user toggled between Public/Private before selecting a
restricted parent, the UI would incorrectly stay on "Public" even though
the parent category required restricted access.
This commit:
- Resets visibility state when selecting a restricted parent category
- Resets to public when removing the parent category
- Adds controlled mode to ConditionalContent component (when @onChange
is provided, the component respects external @activeName changes)
- Removes dead code (userModifiedPermissions)
- Uses DToggleSwitch page object in category specs
What is the problem?
The flagged posts count on user profiles only shows
`ReviewableFlaggedPost` items
and excludes other reviewable types like `ReviewableQueuedPost`,
`ReviewableUser`,
and plugin-provided reviewables (chat, AI, etc.).
`User#number_of_flagged_posts` queries only `ReviewableFlaggedPost`
records, and
the review queue link includes `type="ReviewableFlaggedPost"` as a query
parameter
to scope results. This has been confusing for staff who expect the count
to reflect
all pending review items for a user, not just flagged posts.
What is the solution?
Product has decided to change the label from "X flagged posts" to "X
flags" and
stop scoping to just `ReviewableFlaggedPost`, so the counter reflects
all
reviewables for the user.
1. Rename `User#number_of_flagged_posts` to `User#number_of_flags` and
query
all `Reviewable` records instead of just `ReviewableFlaggedPost`
2. Remove `type="ReviewableFlaggedPost"` from the review queue link
query params
so clicking through shows all reviewables matching the username filter
3. Rename the translation key from `flagged_posts` to `flags` and CSS
class from
`.flagged-posts` to `.flags`
Adds comprehensive system tests for the simplified category creation
feature covering all tabs: General, Security, Settings, Images, and
Tags.
Also includes:
- Fix FormKit `choose_conditional` to work with hidden radio inputs
- Add `toggle_advanced_settings` and `toggle_checkbox` helpers to
Category page object
Joffrey JAFFEUX
Natalie Tay
Régis Hanol
Sam
Martin Brennan
Renato Atilio
Alan Guo Xiang Tan
David Taylor
Jordan Vidrine
Kris
Osama Sayegh
Jarek Radosz