The browse-more message was not consuming the tracking state’s
invalidation
token, so its new and unread counts remained stale after reading a
topic.
Recompute the message whenever the topic tracking state changes.
Reduces RSpec startup time by avoiding work that the selected spec does
not need.
The main changes are:
- Mark development/test dependencies as require: false where they can be
loaded safely at their point of use.
- Avoid loading Capybara, the Playwright driver, system helpers, and
page objects unless the selected specs need them.
- Autoload several large spec helper modules and load Rails tasks only
for task specs.
- Load OAuth providers, schema libraries, API documentation support, and
QR-code support at their actual call sites.
- Preload production dependencies before workers fork to preserve
copy-on-write sharing.
- Register parallel_tests Rake tasks only in local environments so
production bundles do not require test-only gems.
- Make rbtrace opt-in via RBTRACE=1.
- Avoid creating an unrelated top-level fixture in user_spec.rb.
- Explicitly load Capybara in the nginx integration spec, which sits
outside the system spec directory.
The new lazy support files work as follows:
- lazy_fabricators.rb indexes core fabricator definitions without
executing every file. When an unregistered fabricator is first
requested, only the file defining it is loaded. Plugin suites retain
eager fabricator loading for compatibility.
- lazy_faker.rb intercepts a missing Faker constant, loads the full
Faker gem on first use, and retries the lookup.
- lazy_pry.rb provides a temporary pry method that loads pry and
pry-rails on first use, then delegates to the real implementation.
On my machine, startup for bin/rspec spec/models/user_spec.rb:20
decreased from approximately 2.1 seconds to 1.1 seconds—a reduction of
roughly 47%.
Reduces RSpec startup time by avoiding work that the selected spec does
not need.
The main changes are:
- Mark development/test dependencies as require: false where they can be
loaded safely at their point of use.
- Avoid loading Capybara, the Playwright driver, system helpers, and
page objects unless system or request specs need them.
- Autoload several large spec helper modules and load Rails tasks only
for task specs.
- Load OAuth providers, schema libraries, API documentation support, and
QR-code support at their actual call sites.
- Make rbtrace opt-in via RBTRACE=1.
- Avoid creating an unrelated top-level fixture in user_spec.rb.
The new lazy support files work as follows:
- lazy_fabricators.rb indexes core fabricator definitions without
executing every file. When an unregistered fabricator is first
requested, only the file defining it is loaded. Plugin suites retain
eager fabricator loading for compatibility.
- lazy_faker.rb intercepts a missing Faker constant, loads the full
Faker gem on first use, and retries the lookup.
- lazy_pry.rb provides a temporary pry method that loads pry and
pry-rails on first use, then delegates to the real implementation.
On my machine, startup for bin/rspec spec/models/user_spec.rb:20
decreased from approximately 2.1 seconds to 1.1 seconds—a reduction of
roughly 47%.
Boards queried the database during startup to register user-selected
icons, breaking database-less tasks such as asset precompilation. It
also caused stale per-process state and could leak icons between sites.
Load plugin icon sources when building the site-scoped SVG sprite
instead.
Issue is that plugins/boards/plugin.rb:46 hits the database at
after_initialize (data_source_exists?(:discourse_kanban_columns)) to
pre-register column icons. The Publish Assets job runs rake
assets:precompile with no Postgres container, so the connection fails
and boot aborts.
The rescue there was meant to cover this but misses the actual class:
PG's failure surfaces as ActiveRecord::ConnectionNotEstablished. The
listed classes are NoDatabaseError, StatementInvalid, and
DatabaseConnectionError — and DatabaseConnectionError is a subclass of
ConnectionNotEstablished, not its parent, so it doesn't catch it.
The fix is to widen the rescue to ActiveRecord::ConnectionNotEstablished
(covers NoDatabaseError and DatabaseConnectionError) alongside
StatementInvalid
Moves the https://github.com/discourse/discourse-kanban plugin into
core as `boards`, with `Boards` as the engine const.
Table names are unchanged, but setting names are changed and
we create new setting records in a migration, along with duplicating
ACL rows with the new `Boards::Board` target class.
The following fixes are made here to make the transition to
`granular_anonymous_and_logged_in_groups_permissions` being
permanently enabled easier for people.
- **DEV: Show logged_in_users and anonymous_users in setting list**
- Previously, these groups would only show if
`granular_anonymous_and_logged_in_groups_permissions` is enabled,
but they are usable now everywhere in core, and this will help ease the
transition for themes + settings.
- **DEV: Add everyone -> logged_in_users map for theme settings**
- Theme settings now copy the same pattern as site settings, where
previously selected `everyone`
is dynamically changed to `logged_in_users` when
`granular_anonymous_and_logged_in_groups_permissions`
is enabled.
- **DEV: Show everyone group as everyone (legacy) in site setting group
list pickers**
- Makes it clearer this group will soon not be supported
- **DEV: Allow anonymous_users in AnonymousUser#in_any_groups?**
- Anon users should return true for `in_any_groups?` for
`anonymous_users` regardless of whether
`granular_anonymous_and_logged_in_groups_permissions` is on
In some situations, you want to have something like a submenu
where the child menu can close either from:
* The user moving their mouse away from the submenu or
* The user clicking on an item in the submenu
In this case, if the parent has an onClose handler, and
some logic to close itself when the child closes, you
want to be able to pass an argument from the child to
the parent in case the parent should only close
if the child closes from a click, not just because the
mouse moved out of the child submenu.
Then you can do something like:
```
onClose(data) {
if (data?.reason === 'saved') {
this.close();
}
}
```
ExternalLinter runs from the plugin root, but previously passed absolute
file paths through the symlink under discourse/plugins. RuboCop expands
exclusion patterns relative to the plugin's real root, so those paths do
not match.
For example, discourse-kanban system specs incorrectly triggered
RSpec/DescribeClass even though spec/system is excluded. Pass file paths
relative to the plugin root so linter configuration matches symlinked
plugins correctly.
Fixes an issue found in the DAccessControl component inside
a modal (in the kanban plugin). Selecting an option from the
permission dropdown for an ACL row on mobile would close the
whole modal. Now, this only happens when `@modalForMobile={{true}}`
is used on a DMenu outside a modal.
Renaming this array to user.visibleGroups/currentUser.visibleGroups to
make it clear that this is not the full list of groups a user belongs
to, but only the ones that are visible to the current user.
Adds a `deprecated` warning when calling `user.groups` on the client,
and
adds a `user.visibleGroups` alias to the new property.
Similar to the old CHROME_DEVTOOLS="POSITION" command,
this automatically opens the devtools to the right,
you can't control the position in playwright.
These have been stable for a long time and are mostly cosmetic,
bumping to permanent, removing code branches, and incorporating
CSS changes into the main stylesheets.
These are not very common, but if there was a multi-day livestream
event with Zoom, we would show the "You can join the webinar closer
to the event start time message" even if the current time was within
the multiple days.
Now we will only show this message before the grace period before
the start time of the first day of the event.
<img width="476" height="453" alt="image"
src="https://github.com/user-attachments/assets/ce575fbd-9a88-4bc4-a1b2-ba867da8113e"
/>
Fixes the root cause of the giant YouTube videos in chat,
when the iframe is used instead of lazy video (such as in
cases where the URL is a weird format or the video is for
some reason not available and returns a 401).
Gives the iframe video the same max-width as lazy video/
images, and changes the title to YouTube instead of the
URL to the generic youtube thumbnail.
**Before**
<img width="1872" height="703" alt="image"
src="https://github.com/user-attachments/assets/219977d6-e984-4a3c-aa1b-780a3ba3f49d"
/>
**After**
<img width="786" height="370" alt="image"
src="https://github.com/user-attachments/assets/fdbb47ae-91dd-4e59-aeb7-6dc7b227132f"
/>
Regis already added this in 62bc006ea6 to
the MiniTagChooser, so this commit brings parity, and also adds
the same option to be passed down via FKControlTagChooser
Brings a feature of the old markdown editor code blocks
to the ProseMirror RTE code blocks. You can select a portion
of code in a code block and do Tab to indent it and Shift+Tab
to outdent it, which makes copying code + formatting it in
the editor a lot nicer. Ctrl+Z works with this too.
https://github.com/user-attachments/assets/f09cd3f9-e106-451e-955d-4b605b41cd75
For logged in users, MessageBus needs to be able to handle
publish directives with group_ids including the new
anonymous_users and logged_in_users pseudogroups. This commit updates
the
message bus configuration to recognize these pseudogroups and ensures
that
they are correctly processed when publishing messages to the bus.
If granular_anonymous_and_logged_in_groups_permissions is disabled,
then the everyone pseudogroup will also be used for logged in users.
The use case for this was Kanban boards publishing card updates,
some board permissions have logged_in_users (group ID 5) in the board
permissions which we use for MessageBus group_ids.
Not much point in allowing this, and I doubt this is the case
anywhere in the wild. I also doubt more than a handful of sites
even have this plugin enabled, it's more of a developer tool.
Followup a0de88db7b
This extends the /evaluate endpoint for ACLs to handle the case
where, when creating something like a kanban board, the user
would end up with insufficient permissions (determined by
the ACL target) to further access the new entity.
For example, if a regular user can create a kanban board,
but they do not add themselves or a group they are in to
the Manage permissions ACL, then they will not be able to access
the board after creation.
Adds the following schedule payload items:
* date, which is the date of the schedule run in YYYY-MM-DD
* month_number, which is the month number of the schedule run (1-12)
* time, which is the combined 24 hour time of the schedule run in
HH:MM:SS format
This can be helpful when using the schedule data to get e.g.
a week/month/time that adds or subtracts from the schedule run
date/time.
It's currently very easy to lose your permission to an ACL target
by removing permissions, for example here if you're part of the
Pals group and not an admin, you can remove Pals and lose access
to e.g. the kanban board:
<img width="629" height="694" alt="image"
src="https://github.com/user-attachments/assets/9465d4a6-603d-421c-aaef-23eb3724083c"
/>
This PR adds an `/access-control/evaluate` endpoint and a FormKit
field for `DAccessControl` (called `DAccessControlField`) that performs
a validation using the new
ACL proposed and the current user's groups and user ID to check
if they will still have access after save. If not, we show this
confirmation
dialog:
<img width="1107" height="738" alt="image"
src="https://github.com/user-attachments/assets/b52d95e2-91f9-4227-b119-fda813339fd1"
/>
In addition to a general "no access left" check, kanban targets can
define
a `loss_warning_permissions` class method that says "if these
permissions would be lost,
warn the user", defining the error message to show under server.en.yml
in
`access_control_list.errors.#{target_class.acl_target_key}_user_will_lose_permission`
If you press OK then we save and you lose your access. If you press
Cancel,
you can reconfigure the ACL before trying again.
Also adds a `preventSubmit` function on FormKit when validating a field.
Instead of calling `addError` which shows an error message,
`preventSubmit`
can stop the field from proceeding if there is e.g. a validation that
requires
user confirmation.
This will be Cmd+Shift+D on Mac and Ctrl+Shift+D on Windows/Linux.
It will insert a details block at the current cursor position in the
RTE,
and the shortcut shows in the (+) menu.
**Disclaimer: Claude was the driver here mostly, but I tested
before/after locally**
Attempts to fix some issues with the Zoom livestream integration
when a user clicks Join before the webinar has started by the host.
This shows a 30s countdown timer and then attempts to join the webinar
when the countdown ends.
However, when the tab wasn't focused/visible, Chrome (and assuming
other browsers) do some throttling of timers, requestAnimationFrame,
and ResizeObserver callbacks. When the user switches back to the tab,
the Zoom video frame appeared to not be rendered.
The Zoom frame was `display: none` when `init()` measured it, the
desktop rule setting that had a higher specificity than the `--visible`
class meant to reveal it, so the class never applied and the element
only became visible once Zoom wrote an inline `display` of its own.
`computeZoomViewSize` read a zero width and clamped to its 240px
minimum, meaning every session configured Zoom at the wrong size.
That went unnoticed because the post-join sync corrected it
immediately. But that sync runs in a frame callback, and both it and
the modifier's resize observer are suspended while the document is
hidden.
A retry that landed in a background tab never got the
correction, and the single queued frame callback fired on return
racing Zoom's remount, so the meeting rendered at 240px with none of
the layout heights applied, making it seem like the video didn't appear.
Moves the base `display: none` to where `--visible` outranks it, so
the frame is laid out before Zoom is initialized; falls back to the
container's width rather than the minimum when the root measures zero;
and skips the frame callbacks while hidden, re-running them on
`visibilitychange` instead.
Adds groupName and groupFullName helper methods to site
model, this is something we commonly use in the codebase,
and this lets us avoid calling groupsById[id]?.name needing
safe navigation.
This commit fixes `can_create_discourse_post_event?` manually checking
a user's groups, which doesn't account for pseudogroups like
`logged_in_users`. We should always use `user.in_any_groups?` for this.
In addition, the opportunity is taken here for some cleanup, moving
all the guardian extensions from plugin.rb into a proper
`GuardianExtensions` file for calendar, adding extra specs along the
way.
Finally, I've added a `Group.refresh_automatic_groups_for_user!`
method as a single-user variant of `Group.refresh_automatic_groups!`
that can use in fabricators for testing, to make this more
thorough/reliable
than manually adding group user records in `after_create` in the
fabricators
The methods group_has_permission? and user_has_permission? were
returning nil, not false, if the entity did not have permission even
though they are framed as questions. Let's return false as expected,
which is also what we do for the multiple versions of these which
are group_has_any_permission? and user_has_any_permission?
When `solved_topics_auto_close_hours` is null/missing in the DB for
Support type categories, the UI defaults to 48 hours for this setting,
but this can be surprising for admins when changing some other support
category setting, as it will go from 0 -> 48 hours in the background.
This commit fixes the issue by backfilling a new custom field
value for `solved_topics_auto_close_hours`, setting it to `0`
on categories with `enable_accepted_answers` set to true
and no existing value for the field.
New support categories are unaffected, as we set 48 hours by
default there.
For upcoming changes, we were having a problem where we
were notifying admins of upcoming changes in brand new sites.
A previous fix 2e5afb46e2f5c0547bc298b2bd1a9c39e813dc17 tried
to remedy this, but since it relies on the `new_site?` method, which
isn't true after 1 hour, the next time the scheduled notifications are
run for an admin they will be sent them even if their site is relatively
new.
This commit attempts to conclusively fix the issue by inserting
upcoming change `admins_notified_automatic_promotion` and
`admins_notified_available_change` events via `BackfillNotifiedEvents`
for changes that exist in particular statuses when the site is first
created. These events will have `event_data: {"backfilled" => true}`
to distinguish them from others.
It also fixes an adjacent issue where an admin was notified of a
plugin's upcoming changes as soon as they enable the plugin,
which isn't useful as the intent of upcoming changes is to notify
of changes for things the admin has already been using.
**Before this change, on brand new site with current upcoming change
state**
<img width="401" height="429" alt="image"
src="https://github.com/user-attachments/assets/a89cd641-59aa-4c48-a57c-9692021df176"
/>
**After this change**
No notification to show :) But when I moved an upcoming change to `beta`
it showed
the notification for the enabled upcoming change correctly:
<img width="378" height="174" alt="image"
src="https://github.com/user-attachments/assets/20e4096f-e83f-4c06-970d-a9b43561b12e"
/>
* Do not show the direct Zoom link when the livestream
is enabled for the event, we want to keep people on the
Discourse site
<img width="942" height="637" alt="image"
src="https://github.com/user-attachments/assets/6267ce8d-7848-4a7d-8769-78f82dd87417"
/>
* Prompt anonymous users to log in when Join Zoom is clicked
* Prevent unnecessary Hash#merge in DefaultsProvider#all_clean,
Fixes an unnecessary merge which can sometimes cause performance
overhead when calling SiteSetting.defaults_provider.all_clean.
We don't need to merge the defaults when locale provided is the
same as the default locale
* Do not check can_lazy_load_categories? in loop,
This is a small perf optimization, we don't need to check
can_lazy_load_categories? for every category in the site,
which looks at the user's groups and upcoming changes every
time, we just need to do it once.
This commit uses `DFilterControls` for emoji list filtering, and sorts
the
emoji by group then by name, with default always at the top:
<img width="1101" height="803" alt="image"
src="https://github.com/user-attachments/assets/77e384c1-33b1-4741-bd55-ca86cd5f2a03"
/>
I've also refactored the create + delete endpoints into services.
These changes don't need to show the scary warning to
admins when they become stable, as they are still optional
features even when the upcoming change is gone
Followup e66d6c56e2
I was referring to `result` outside the block, and also realised
that should_notify_admins policy was not being checked, which is
why we were ending up with the N/A error in the first place
The `enable_discourse_workflows` site setting is both the plugin enabled
setting AND an upcoming change, which are always hidden settings.
Admins, when enabling the plugin, would see an error like this
since the setting was hidden:
> You are not allowed to change hidden settings:
enable_discourse_workflows
This commit fixes this by allowing hidden settings to be changed if the
setting is an upcoming change
`ModelStep` revalidates changed models by calling `invalid?`, which
clears errors
added by the model producer before validations run. For example,
`UploadCreator`
can return an unsaved `Upload` containing an image-processing error;
`ModelStep`
then clears that error and may treat the `Upload` as valid.
Treat models that already contain errors as invalid without revalidating
them.
This preserves operation-specific errors for `on_model_errors` while
continuing
to validate changed models that do not already contain errors.
---------
Co-authored-by: Loïc Guitaut <loic@discourse.org>
This kind of error and backtrace that is unexpected and
has no error backtrace info is happening for some sites in
production, but I can't tell why:
> [CheckUpcomingChanges (SITE)] Failed to notify about promotion of
'enable_simplified_category_creation': with backtrace: N/A
Add more logging to try get a better reason
When we define banned_acls on an ACL target, there is a
possible case where all permission types for that ACL target
are banned for a particular grantee (group/user). For example,
if you do not want anonymous_users to view, edit, or manage
a certain target.
In this case, instead of hiding the grantee row in the chooser we
will disable it and show a message to the user so it's clear why it's
disabled
<img width="687" height="614" alt="image"
src="https://github.com/user-attachments/assets/d8aa60c6-a2f1-4821-b715-55595ec0a03d"
/>
Our UpcomingChangeStableOptedOut problem check was showing
problems on brand new sites, when it should only show a
problem after admin has opted out of a stable upcoming change.
This was happening because on some sites a plugin might be disabled
or not configurable, so it would return false when calling
`SiteSetting.send(change_name)`, but crucially we weren't filtering
out upcoming changes that shouldn't display on the site.
This commit fixes the issue by returning no problem if the upcoming
change is not configured to display on the site.
Followup bd3906ce64
Converts the new endpoints made in the original PR into
service classes, using AI to drive implementation based
on the service authoring skill:
* CustomEmoji::Export
* CustomEmoji::PreviewImport
* CustomEmoji::ConfirmImport
In addition, some supporting action, lib, and serializer
classes have been added. This fixed a couple of
bugs as well:
* Import names now go through `Emoji.sanitize_emoji_name` like
single-emoji create
* No-group imports no longer persist a literal "default" string
* Two 500s (suspicious zip entries, malformed CSV) became proper 422s
* Duplicate CSV names are flagged at preview instead of exploding the
confirm transaction
*` Emoji.clear_cache `moved outside the transaction
Partial followup of 7ff30d81a3
The previous commit totally hid the group name
in the DAccessCopntrol grantee chooser (derived from
EmailGroupUserChooser) in favour of full name.
However in some cases it is useful to see the group name
as it may be different from the full name. This commit brings
back the group name after the full name, and adds an option
to not show the name if it's the same as the full name.
<img width="680" height="760" alt="image"
src="https://github.com/user-attachments/assets/127187fe-4e23-4689-a97d-f7e65870a353"
/>
Fixes the following error when generating the upcoming changes status
report
on a weekly basis in GitHub actions:
```
/__w/discourse/discourse/lib/upcoming_changes/status_report.rb:56:in 'UpcomingChanges::StatusReport::Git#capture': git show 45535887f1231b40f8a21b8c154e58cef8670063:plugins/discourse-workflows/config/settings.yml failed: fatal: path 'plugins/discourse-workflows/config/settings.yml' exists on disk, but not in '45535887f1231b40f8a21b8c154e58cef8670063' (RuntimeError)
from /__w/discourse/discourse/lib/upcoming_changes/status_report.rb:47:in 'UpcomingChanges::StatusReport::Git#show_file'
from /__w/discourse/discourse/lib/upcoming_changes/status_report.rb:227:in 'UpcomingChanges::StatusReport::GitHistory#statuses_at'
from /__w/discourse/discourse/lib/upcoming_changes/status_report.rb:215:in 'block in UpcomingChanges::StatusReport::GitHistory#add_history_for_settings_file'
from /__w/discourse/discourse/lib/upcoming_changes/status_report.rb:214:in 'Array#reverse_each'
from /__w/discourse/discourse/lib/upcoming_changes/status_report.rb:214:in 'UpcomingChanges::StatusReport::GitHistory#add_history_for_settings_file'
from /__w/discourse/discourse/lib/upcoming_changes/status_report.rb:205:in 'block in UpcomingChanges::StatusReport::GitHistory#by_change'
from /__w/discourse/discourse/lib/upcoming_changes/status_report.rb:202:in 'Hash#each'
from /__w/discourse/discourse/lib/upcoming_changes/status_report.rb:202:in 'Enumerable#each_with_object'
from /__w/discourse/discourse/lib/upcoming_changes/status_report.rb:202:in 'UpcomingChanges::StatusReport::GitHistory#by_change'
from /__w/discourse/discourse/lib/upcoming_changes/status_report.rb:422:in 'UpcomingChanges::StatusReport#history_by_change'
from /__w/discourse/discourse/lib/upcoming_changes/status_report.rb:363:in 'block in UpcomingChanges::StatusReport#report'
from /__w/discourse/discourse/lib/upcoming_changes/status_report.rb:362:in 'Hash#each'
from /__w/discourse/discourse/lib/upcoming_changes/status_report.rb:362:in 'Enumerable#map'
from /__w/discourse/discourse/lib/upcoming_changes/status_report.rb:362:in 'UpcomingChanges::StatusReport#report'
from /__w/discourse/discourse/lib/upcoming_changes/status_report.rb:499:in 'UpcomingChanges::StatusReport::CLI.run'
from /__w/discourse/discourse/script/upcoming_changes_status_report:9:in '<main>'
```
This happened because we were trying to run `git sho` on a file that was
reverted in a commit. Now, we check whether the file exists for the
specific commit hash before running git show, and continue with the
next valid commit for the file.
Followup 7e77ce4bd3
We need to automatically resolve group membership into a boolean
for theme object type settings which are of the group type, similar
to what we did in the original commit above for group list type
settings.
This behaves in the same way -- for an object setting schema like this:
```
menu_sections:
type: objects
default:
- name: section 1
groups:
- 1
- 3
schema:
name: menu section
properties:
name:
type: string
groups:
type: groups
resolve_group_membership: true
```
We replace `groups` with a boolean `user_in_groups` (groups is just
the property name, it could be foo_bar etc.) and then you can
do this on the client:
```
for (const section of settings.menu_sections) {
if (section.user_in_groups) {
// User is in at least one selected group for this section.
}
}
```
Rather than inspecting the `currentUser.groups`, which only includes
visible groups, not all groups the user is a member of. This allows
for more accurate permission checks for theme settings that are group
based.
Also c.f.
https://meta.discourse.org/t/granular-group-based-permissions-for-anonymous-and-logged-in-users/402273/18?u=martin
This commit adds support for embedding a Zoom webinar
livestream directly within a Discourse Event via a URL in
the location field of the event in the topic.
Admins must supply a Zoom API key and secret in the plugin
settings for this functionality to be available using
these settings:
* livestream_zoom_enabled
* livestream_zoom_sdk_key
* livestream_zoom_sdk_secret
When a topic is opened with a Zoom URL in the event location,
and the Livestream toggle is enabled, the
Zoom library dependencies will be loaded asyncrhonously
and the Zoom webinar will be embedded within the topic page
using the Zoom component view.
On mobile, a fullscreen Zoom client using the Zoom client view
will be used, with a floating Chat button that opens the Discourse
livestream chat in a modal which slides up from the bottom
of the screen in an overlay. This client view is at `/t/:id/:slug/zoom`
### Webinar event date/time not reached
<img width="880" height="612" alt="image"
src="https://github.com/user-attachments/assets/4b112b59-eed0-4d63-b166-da825d8ebd0b"
/>
### Within 30 minutes before and 10 minutes after the event date/time
<img width="879" height="613" alt="image"
src="https://github.com/user-attachments/assets/373191f8-6245-4b41-b2a8-161c935166be"
/>
### Attendee joined before host, we will retry join every 30s
<img width="735" height="817" alt="image"
src="https://github.com/user-attachments/assets/519afa51-8099-4af0-954f-08da4c7e3297"
/>
### Attendee joined
<img width="733" height="831" alt="image"
src="https://github.com/user-attachments/assets/f633d21f-956e-4bec-a2f8-f7dc8b95d974"
/>
---------
Co-authored-by: David Taylor <david@taylorhq.com>
Co-authored-by: awesomerobot <kris.aubuchon@discourse.org>
The `AdminFilterControls` component isn't really admin specific,
and I want to use it in more places. Renaming to `DFilterControls`.
No backwards compat needed, it's only used in core + core plugins.
Also add `customEmptyState` to `DFilterControls`, which
allows caller to show a custom empty state for `DFilterControls`
in a yield when the array of result items is empty, for cases
where you might want to show e.g. a different message and CTA
when the filter is empty.
E.g. used in Kanban:
<img width="1107" height="439" alt="image"
src="https://github.com/user-attachments/assets/1466e4d9-ac4d-4c59-81e5-6e0e3e259c6f"
/>
ProblemCheck#run was calling translation_data with no args while
persisting tracker/admin notice details. For targeted checks like
`group_email_credentials` and `upcoming_change_stable_opted_out`,
`translation_data` requires the target, so scheduled runs crashed after
call returned a problem. This was causing an error like this in
production:
```
A scheduled admin dashboard problem check (group_email_credentials) errored. : ArgumentError : wrong number of arguments (given 0, expected 1)
/var/www/discourse/app/services/problem_check/group_email_credentials.rb:35:in 'translation_data'
/var/www/discourse/app/models/problem_check.rb:203:in 'ProblemCheck#run'
/var/www/discourse/app/jobs/regular/run_problem_check.rb:23:in 'Jobs::RunProblemCheck#execute'
/var/www/discourse/app/jobs/base.rb:313:in 'block (2 levels) in Jobs::Base#perform'
```
The intended flow after the fix now:
* targets provides the scheduler/tracker key.
* problem(target) is where target-specific translation data is computed.
* Problem#details carries that computed translation data plus
override_data.
* run persists problem.details instead of recalculating
translation_data.
c.f.
https://meta.discourse.org/t/argumenterror-wrong-number-of-arguments-given-0-expected-1-in-upcoming-change-stable-opted-out-problem-check-flooding-logs/407351
Sometimes for upcoming changes we need to tell the admin about
some settings that must be enabled for the change to take effect.
Currently the only way to do this is with a note in the change
description which is not ideal.
This commit introduces a way to display the depends_on settings
for the upcoming change in the admin upcoming changes page. The
warning only shows if any of the settings are not enabled, if all
dependencies are satisfied we do not show the warning to the admin.
There is a link to each setting from the upcoming changes list
to make it easy for the admin to enable the setting if needed.
<img width="1280" height="720" alt="image"
src="https://github.com/user-attachments/assets/724762a9-cc40-45b4-b080-d6ee87dc57ca"
/>
Upcoming changes conditional display needs to be able to
work for plugin-defined upcoming changes in some cases.
Currently the only way to do this is to extend the
UpcomingChanges::ConditionalDisplay class, which is not ideal.
This commit adds a plugin registry for upcoming changes conditional
display, allowing plugins to register their own conditional display
methods per change.
Upcoming changes can be registered by plugins, but nothing in the
upcoming changes framework knew whether the owning plugin was actually
configurable on a given site.
For unavailble plugins on our hosting, `SiteSettingExtension#setting`
already
forces the plugin's `enabled_site_setting` to false, and `all_settings`
already filters the change out of the Upcoming Changes page.
The notification paths for upcoming changes did not
follow suit...`UpcomingChanges::NotifyPromotion` and
`Jobs::NotifyAdminsOfAvailableUpcomingChanges` both gate only on
`ConditionalDisplay`, so admins were told a change had been enabled by
default for a plugin they cannot use and that is in fact still off.
`UpcomingChanges.enabled?` had the same blind spot for a different
reason, in that it reads the settings provider directly, bypassing the
getter's
configurability guard, so it returned true where the setting itself read
false.
Adds `UpcomingChanges.owning_plugin_configurable?` and short-circuits
both `ConditionalDisplay.should_display?` and `UpcomingChanges.enabled?`
on it. Core changes have no owning plugin and return early, so hot paths
such as `settings_hidden_while_enabled` are unaffected.
The gate keys on `configurable?` alone rather than mirroring the
getter's
narrower `enabled_site_setting == name` condition, a change gating a
sub-feature of an unavailable plugin is equally unavailable.
Currently the /u/username/activity/assigns list sorts by
topic bump date for assignments, whereas the user menu for
assigns list sorts by read/unread and then the created_at
date for the notification.
The latter doesn't make much sense, most of the time when
you are working on assignments the ones you are working on
now which are being bumped somewhat frequently are the
ones you want to see first, not in created_at order.
This commit keeps the unread assignments at the top of the
list, but sorts the rest by topic bump date.
Currently, theme settings with `type: list` and `list_type: group`
require client-side permission checks, but `currentUser.groups` only
includes visible groups, not all groups the user belongs to. This makes
permission checks unreliable and can leak hidden group membership.
To address this, this commit adds an opt-in `resolve_group_membership:
true` option that replaces the group ID list with a user_in_SETTING_NAME
boolean resolved server-side via `guardian.in_any_groups?`. The original
group list is removed from the frontend payload to prevent leaking group
IDs.
Since theme settings are cached per-theme (not per-user), the resolution
happens after the cache lookup during per-request serialization in
`ApplicationLayoutPreloader#activated_themes_json`.
Example YAML:
```yaml
copy_button_allowed_groups:
type: list
list_type: group
resolve_group_membership: true
default: "1|3"
```
Frontend usage:
```javascript
if (!settings.user_in_copy_button_allowed_groups) return;
```
With the ACL changes that are ongoing, regular users
need to be able to see the pseudogroups in certain situations,
like the ACL group selection dropdown. Pseudogroups are the
everyone, logged_in_users, and anonymous_users groups. They
are currently limited to staff only, but this change will allow
all logged on users to see them as well.
This commit also does some minor fixes related to
granular_anonymous_and_logged_in_groups_permissions
upcoming change. If this is enabled, the everyone group should not
be selectable in Site.groups in the UI, that will just lead to
confusion in the long term especially because this upcoming change
will eventually remove the everyone group.
Permanent changes show on `/whats-new` but not the upcoming changes
page.
This PR removes the Permanent filter from the dropdown on the upcoming
changes page to avoid confusion.
We think this system is tested well enough now to make the default
promotion status for upcoming changes to Beta instead of Stable. This
will mostly affect self-hosters as on our own hosting we set
different promotion statuses based on billing tier.
When an upcoming change was automatically enabled, the notification
linked admins to the upcoming changes config page. But once a change
reaches permanent status it no longer appears there (that page only
lists experimental through stable) and instead surfaces on the What's
New page. So if the notification was left unread until the change became
permanent, clicking it dropped the admin on an empty filtered list.
This resolves the change's status at click time rather than baking it
into the notification. The permanent change names are now exposed to
staff via the site serializer, and the notification's link checks them:
if every referenced change is now permanent, it sends the admin to
What's New scrolled to that change's card; otherwise it keeps the
existing upcoming changes page where the still non-permanent changes
remain actionable.
To support the scroll, every What's New item now has a stable id anchor,
and the page accepts a scrollTo query param that scrolls to and briefly
highlights the matching card once the feed loads.
When showing groups in the DAccessList component, we need
to fall back to name when full_name is not defined for a group,
so let's call this a single property "display_name", which user
ACLs will use too.
Fixes this:
<img width="616" height="486" alt="image"
src="https://github.com/user-attachments/assets/2c5d21d7-3f33-4666-8244-23b94bac57f7"
/>
The Horizon theme doesn't show the empty box on unsolved
topics even if empty_box_on_unsolved is enabled for the
category, so we should hide that setting if the Horizon
theme is default for the site.
Introduces a server-side definition of banned_acl,
similar to mandatory_acl from 5823e4e3b2
This allows AclTarget implementing classes to define
which ACLs cannot be used for certain types. For example:
```
def self.banned_acl
[{ type: :group, id: Group::AUTO_GROUPS[:anonymous_users], permission: "edit" }]
end
```
This prevents anonymous users group from being able to have the Edit
permission on the target, which is practical because the anonymous
user is not logged in and generally cannot create/edit anything.
This restriction is passed to the DAccessControl component via the Site
serializer, same as mandatory_acl, and is used to prevent the user from
selecting the banned ACLs in the UI.
Then, server-side this rule is enforced in `AccessControlListManager`
service as a policy.
**Before**
<img width="971" height="287" alt="image"
src="https://github.com/user-attachments/assets/af4ace0f-7b11-4005-8344-774d554693f0"
/>
**After**
<img width="914" height="194" alt="image"
src="https://github.com/user-attachments/assets/b756b976-f6e6-4331-b27d-c9a2d13997aa"
/>
Introduces a new way of handling permissions for a target entity
within Discourse, including integration with `Guardian` and `User`
models. For this initial PR, only group-based permissions will work
with this new model, a followup PR will introduce user-based
permissions.
The initial use case for this is securing Kanban Boards in Discourse
Kanban,
see https://github.com/discourse/discourse-kanban/pull/58 for the sister
PR.
In future, we will use this in more places in core, like Chat, Category,
etc.
The new model is called `AccessControlList`, here is the schema:
* `target_id`/`target_type` - Polymorphic columns, can point to any
other model
* `owner` - A string indicating whether the permission is owned by
`core` or a plugin e.g. `discourse-kanban`
* `permission` - A free text field, which can be whatever the target
requires, but defaults are `edit`, `view`, `manage`, and `own`. `own` at
this time is a special permission that should be added to whatever user
creates the ACL at first, but should not be shown in any UI
* `allowed_group_ids` - An array of group IDs which have this permission
* `allowed_user_ids` - An array of group IDs which have this permission
A component, called `DAccessControl`, is also introduced to display
these permissions
and the groups (and soon users) who have them. The component allows a
custom description
label, and the list of permissions can have their text and description
modified,
and can also have permissions removed or added as needed for the target:
<img width="612" height="222" alt="image"
src="https://github.com/user-attachments/assets/88f8d70a-f965-4ef2-9e64-5f8b2bf3fa12"
/>
<img width="598" height="380" alt="image"
src="https://github.com/user-attachments/assets/db90b956-86c2-4ca7-9b63-4c3eb0bad5ef"
/>
Each target entity can define their own `mandatory_acl` array which
is similar to how `mandatory_values` for site settings. For example,
Kanban Boards have a mandatory ACL of the admins group ID having
the `manage` permission for the board, since admins always need to
be able to see + manage boards. This allows us to avoid hardcoding
admin/staff escape hatches in guardian/permissions code.
These will be shown as disabled rows in the `DAccessControl` component:
<img width="577" height="248" alt="image"
src="https://github.com/user-attachments/assets/d9bf1754-0530-4edb-b89f-28887cd613d6"
/>
This commit also introduces a `full_name` calculation for automatic
groups, so we can display a nicer version of the name for things
like `admins`, `staff`, `trust_level_0` in the UI.
The shared JSON endpoint examples used to validate schemas with a
boolean
assertion and print one ad hoc hint to stdout. A failure was hard to act
on:
```
VALUE AT "/access_control": {"mandatory_acl" => {}}
POSSIBLE ISSUE W/: /access_control
expected: true
got: false
```
That output hid the real validator error, only showed the first failure,
and did not explain whether the response or schema needed to change.
Build a proper RSpec failure message from all JSONSchemer validation
results
instead. Each error now includes the issue, validator error, data path,
schema path, offending value, optional parent/details context, and a
concrete
suggested fix.
For unexpected properties:
```
JSON schema validation failed with 1 error:
1. Unexpected property at /access_control
Error: object property at `/access_control` is a disallowed additional property
Data path: /access_control
Schema path: /additionalProperties
Value:
{
"mandatory_acl": {}
}
Suggested fix: If this response/request field is intentional, add this entry
to the parent schema's `properties` object:
{
"access_control": {
"type": "object",
"additionalProperties": false,
"properties": {
"mandatory_acl": {
"type": "object",
"additionalProperties": true
}
},
"required": [
"mandatory_acl"
]
}
}
If the field is always present, also add "access_control" to the parent
schema's `required` array.
```
For missing required properties, the message now groups missing keys and
shows
the response root plus validator details:
```
Missing required properties default_archetype, notification_types at root
Error: object at root is missing required properties: ...
Details:
{
"missing_keys": [
"default_archetype",
"notification_types"
]
}
Suggested fix: Add default_archetype, notification_types to the
response/request, or remove them from `required` at root schema.
```
For type mismatches, the message now points at the mismatched schema
node and
suggests an inferred replacement shape:
```
Error: value at `/access_control` is not an array
Data path: /access_control
Schema path: /properties/access_control
Value:
{
"mandatory_acl": {}
}
Suggested fix: Update the payload to match the documented `type`, or replace
the schema at /properties/access_control with:
{
"type": "object",
"additionalProperties": false,
"properties": {
"mandatory_acl": {
"type": "object",
"additionalProperties": true
}
},
"required": [
"mandatory_acl"
]
}
```
When an admin changes a category's slug or the parent category,
we need to update posts referencing the category with a `#hashtag`
and rebake it with the new hashtag reference.
Now, we enqueue a background job when a category slug or parent
changes to remap matching hashtag references in posts.
- Add a new `discourse-admin-ui` skill distilling
https://meta.discourse.org/t/creating-consistent-admin-interfaces/326780?tl=en
for creating and reviewing consistent Discourse admin interfaces.
- Split guidance into focused references for page shells, content
patterns, filtered settings pages, and plugin admin UIs.
- Include local code anchors so agents can follow current core and
plugin examples without rereading the Meta guide.
Some upcoming changes only display based on some condition.
We also need to respect that condition for sending notifications,
since otherwise admins are notified about something they can't
see in the UI.
c.f. https://meta.discourse.org/t/-/405009
Martin Brennan
