This commit removes the filter that would remove themeable
site settings from all setting lists, so they are easier to
find for admins.
To do this, we show the value of the site's default theme
for that theme site setting, disable the setting component,
and provide a link to the site's default theme for quick
editing.
---------
Co-authored-by: awesomerobot <kris.aubuchon@discourse.org>
Followup 5b236ccc07
The humanize_name method was introducing some slowness and also more
objects in memory to GC. This commit improves the performance by
memoizing and moving work to hash lookup instead of array.include?
checks, and making sure we compile regexes only once.
As well as this, we memoize the humanized names for all settings
so we don't have to recompute them every time we call all_settings.
Results are as follows:
**Current perf:**
```
Results:
Total time: 36.8178 seconds
Average time per call: 368.1783 ms
Calls per second: 2.72
Detailed timing:
User CPU time: 36.432 seconds
System CPU time: 0.2607 seconds
Total CPU time: 36.6926 seconds
Real time: 36.8178 seconds
Memory Report:
Total allocations: 10932505
Total retained: 985
```
**Humanize improvements:**
```
Results:
Total time: 27.7318 seconds
Average time per call: 277.3178 ms
Calls per second: 3.61
Detailed timing:
User CPU time: 27.3635 seconds
System CPU time: 0.2078 seconds
Total CPU time: 27.5713 seconds
Real time: 27.7318 seconds
Memory Report:
Total allocations: 5535404
Total retained: 1098
```
**Humanize improvements + memoization of humanized names:**
```
Results:
Total time: 24.5924 seconds
Average time per call: 245.9237 ms
Calls per second: 4.07
Detailed timing:
User CPU time: 24.2941 seconds
System CPU time: 0.2019 seconds
Total CPU time: 24.496 seconds
Real time: 24.5924 seconds
Memory Report:
Total allocations: 4417243
Total retained: 1184
```
This commit reverts the changes made to `SvgSprite.settings_icons` in
19af83d39e
which resulted in significantly slower boot times on multisite clusters
since `SvgSprite.settings_icons` is called during application preload.
The test cases added in 470b2891c5 caused
core backend tests to start timing out on CI because MessageBus messages
were being processed in a background thread outside of the current
database transation.
We are currently working around the problem by updating the test cases
to just assert that the right MessageBus message is being published.
This commit fixes an issue in multi-process setups where the following
would happen:
1. Admin changes a theme site setting. The new value is stored in memory
in the process that made the change. For this example, changing
`enable_welcome_banner` from `true` to `false`
2. Another user visits the site and hits another process, which has the
old value of `enable_welcome_banner` still set to `true` in memory
3. The other user's value for `enable_welcome_banner` is cached in
the cache for theme site settings, which is shared across all
processes
The issue is fixed by doing the same thing as site settings: when the
new value is saved, we send a MessageBus message to all processes via
`SiteSettingExtension` which calls `refresh!` and reloads the in-memory
cache of theme site settings from the DB.
c.f.
https://meta.discourse.org/t/difficulty-understanding-the-enable-welcome-banner-setting/377321
This PR adds localized language names to settings. The language names
are localized in the frontend, not the backend, due to setting
initialization complexity.
This change affects these areas:
- `SiteSetting.available_locales`
- this "setting" is a lookup table to get language names. use
`languageNameLookup` service to get the name for a locale
- it returns an object that looks like the following, then gets
re-hydrated with client localized values when initializing the
`siteSettingService` in the frontend.
```
[
{"native_name":"اللغة العربية","value":"ar","name":"languages.ar.name"},
...
]
```
- `SiteSetting.default_locale`
- this is a single-value `enum` setting that has always been hardcoded.
This caused quite an issue as it is not initialized the same way as
other site settings in the yml file. It has always relied on reading
directly from a `names.yml` file to load native language names, thus
bypassing the need for I18n to be initialized from the backend. A new
locale_enum type has been introduced for this setting, and any future
settings.
- `SiteSetting.content_localization_supported_locales` - this is a
`enum_list` setting,
- enum_list is introduced, leveraging both `list` and `enum`
- theme translations
- site texts
- Wizard's default_locale choices
- it was set up from the backend using `LocaleSiteSetting.value`. This
proved problematic, as a Japanese user would be getting the locales in
English because the values are initialized using English even without
memoization
- therefore we're now initializing the choices in the frontend using
`available_locales` as defined above
- content localization meta data
- post language in the composer, localization composer, post history
modal, post language tooltip, language switcher
/t/151409
There are a few legit scenarios where the current
theme ID may be blank, such as safe mode. In this
case it will be better to return default site setting
values rather than to cause random/undefined behaviour
in the UI.
Also clears the theme site setting cache when the
theme ID is null, along with the other fix here this
was probably causing issues in system spec runs
Followup 19af83d39e
There was another edge case here, where this scenario happened:
* A site setting is deprecated _without_ an override
* This site setting is being shadowed by a global setting
* This would cause an argument error when getting the deprecated
setting value in SiteSetting.client_settings_json_uncached
An example of this setting was `external_system_avatars_enabled`.
The shadowed setting was blowing up because it was missing the
`scoped_to` argument that was added for themeable site settings
in the referenced commit.
This commit fixes the issue and adds a spec to cover this scenario,
followup commit will add stronger handling of shadowed global
settings for theme site settings...this is not allowed.
Followup 19af83d39e
In some scenarios, the client_settings_json_uncached
was silently failing and returning "", which caused
problems down the line in mini_racer when this code
was produced:
```
__optInput = {};
__optInput.siteSettings = ;
```
The problem was only happening in rare cases with interaction
between core and other plugins, because the length of the
array we were trying to make a Hash with wasn't an even length.
To address this, we can use filter_map to ensure correct behavior,
and add some specs and also raise errors when parsing client settings
JSON
in specs, so we don't miss this in the future.
This commit introduces the concept of themeable site settings,
which is a new tool for theme authors that lives alongside theme
modifiers and theme settings. Here is a quick summary:
* Theme settings - These are custom settings used to control UI and functionality within your theme or component and provide configuration options. These cannot change core Discourse functionality.
* Theme modifiers - Allows a theme or a component to modify selected server-side functionality of core Discourse as an alternative to building a plugin.
* Themeable site settings (new) - Allows a theme (not components) to override a small subset of core site settings, which generally control parts of the UI and other minor functionality. This allows themes to have a greater control over the full site experience.
Themeable site settings will be shown for all themes, whether the theme
changes
the value or not, and have a similar UI to custom theme settings.
We are also introducing a new page at
`/admin/config/theme-site-settings` that
allows admins to see all possible themeable site settings, and which
themes
are changing the value from the default.
### Configuration
Theme authors can configure initial values themeable site settings using
a section in the `about.json` file like so:
```json
"theme_site_settings": {
"search_experience": "search_field"
}
```
These values will not change when the theme updates, because we cannot
know if admins have manually changed them.
### Limitations
Themeable site settings are only really intended to control elements of
the UI, and when retrieving their value we require a theme ID, so these
limitations apply:
- Themeable site settings cannot be used in Sidekiq jobs
- Themeable site settings cannot be used in markdown rules
- Themeable site settings will be cached separately to client site
settings using theme ID as a key
- Themeable site settings will override keys on the `siteSettings`
service on the client using the application preloader
- `SiteSetting.client_settings_json` will not include themeable site
settings, instead you can call `SiteSetting.theme_site_settings_json`
with a theme ID
### Initial settings
There are only two site settings that will be themeable to begin with:
* `enable_welcome_banner`
* `search_experience`
And our new Horizon theme will take advantage of both. Over time, more
settings that control elements of the UI will be exposed this way.
This commit moves the humanization of site setting names from the client
side to the server side, since this is becoming more widely used and
will be useful for specs to be able to access the same humanized name
formatting without duplicating the logic on the server.
There are only a couple of places in the admin UI that we send this
down in the JSON, the `site.sitSettings` object does not include these
humanized names as they are not needed.
This feature allows admins to find what they are
looking for in the admin interface via a search modal.
This replaces the admin sidebar filter
as the focus of the Ctrl+/ command, but the sidebar
filter can also still be used. Perhaps at some point
we may remove it or change the shortcut.
The search modal presents the following data for filtering:
* A list of all admin pages, the same as the sidebar,
except also showing "third level" pages like
"Email > Skipped"
* All site settings
* Themes
* Components
* Reports
Admins can also filter which types of items are shown in the modal,
for example hiding Settings if they know they are looking for a Page.
In this PR, I also have the following fixes:
* Site setting filters now clear when moving between
filtered site setting pages, previously it was super
sticky from Ember
* Many translations were moved around, instead of being
in various namespaces for the sidebar links and the admin
page titles and descriptions, now everything is under
`admin.config` namespace, this makes it way easier to reuse
this text for pages, search, and sidebar, and if you change it
in one place then it is changed everywhere.
---------
Co-authored-by: Ella <ella.estigoy@gmail.com>
This commit contains two changes to how our site setting
keyword system works:
1. Crowdin, our translation provider, does not support YAML lists,
so we are changing site setting keywords in server.en.yml to
be pipe-separated (|)
2. It's unclear to translators what they are supposed to do with
aliases of site settings where the name has changed, e.g.
min_trust_level_for_here_mention. Instead of getting these as
keywords from the yml file, we can discern these from
SiteSettings::DeprecatedSettings automatically, and still use
them for client-side search
These changes should help improve the situation for translators.
Constants should always be only assigned once. The logical OR assignment
of a constant is a relic of the past before we used zeitwerk for
autoloading and had bugs where a file could be loaded twice resulting in
constant redefinition warnings.
A new setting attribute is used to define the areas (separated by `|`).
In addition, endpoint `/admin/config/site_settings.json` accepts new `filter_area` data.
* DEV: Upgrade Rails to 7.1
* FIX: Remove references to `Rails.logger.chained`
`Rails.logger.chained` was provided by Logster before Rails 7.1
introduced their broadcast logger. Now all the loggers are added to
`Rails.logger.broadcasts`.
Some code in our initializers was still using `chained` instead of
`broadcasts`.
* DEV: Make parameters optional to all FakeLogger methods
* FIX: Set `override_level` on Logster loggers (#27519)
A followup to f595d599dd
* FIX: Don’t duplicate Rack response
---------
Co-authored-by: Jarek Radosz <jradosz@gmail.com>
* DEV: Upgrade Rails to 7.1
* FIX: Remove references to `Rails.logger.chained`
`Rails.logger.chained` was provided by Logster before Rails 7.1
introduced their broadcast logger. Now all the loggers are added to
`Rails.logger.broadcasts`.
Some code in our initializers was still using `chained` instead of
`broadcasts`.
* DEV: Make parameters optional to all FakeLogger methods
* FIX: Set `override_level` on Logster loggers (#27519)
A followup to f595d599dd
* FIX: Don’t duplicate Rack response
---------
Co-authored-by: Jarek Radosz <jradosz@gmail.com>
Many site settings can be distructive or have huge side-effects
for a site that the admin may not be aware of when changing it.
This commit introduces a `requires_confirmation` attribute that
can be added to any site setting. When it is true, a confirmation
dialog will open if that setting is changed in the admin UI,
optionally with a custom message that is defined in client.en.yml.
If the admin does not confirm, we reset the setting to its previous
clean value and do not save the new value.
* Revert "FIX: Set `override_level` on Logster loggers (#27519)"
This reverts commit c1b0488c54.
* Revert "DEV: Make parameters optional to all FakeLogger methods"
This reverts commit 3318dad7b4.
* Revert "FIX: Remove references to `Rails.logger.chained`"
This reverts commit f595d599dd.
* Revert "DEV: Upgrade Rails to 7.1"
This reverts commit 081b00391e.
This commit adds ability to fetch a subset of site settings from the `/admin/site_settings` endpoint so that it can be used in all places where the client app needs access to a subset of the site settings.
Additionally, this commit also introduces a new service class called `UpdateSiteSetting` that encapsulates all the logic that surrounds updating a site setting so that it can be used to update site setting(s) anywhere in the backend. This service comes in handy with, for example, the controller for the flags admin config area which may need to update some site settings related to flags.
Internal topic: t/130713.
When we turn on settings automatically for customers,
we sometimes use `.set_and_log` which will make a staff
action log for the site setting change. This is fine, but
there is no context for customers.
This change allows setting a message with `.set_and_log`, which
will be stored in the `details` column of the staff action log
created, which will show up on `/admin/logs/staff_action_logs`
---------
Co-authored-by: Kelv <kelv@discourse.org>
Automatically add `moderators` and `admins` auto groups to specific site settings.
In the new group-based permissions systems, we just want to check the user’s groups since it more accurately reflects reality
Affected settings:
- tag_topic_allowed_groups
- create_tag_allowed_groups
- send_email_messages_allowed_groups
- personal_message_enabled_groups
- here_mention_allowed_groups
- approve_unless_allowed_groups
- approve_new_topics_unless_allowed_groups
- skip_review_media_groups
- email_in_allowed_groups
- create_topic_allowed_groups
- edit_wiki_post_allowed_groups
- edit_post_allowed_groups
- self_wiki_allowed_groups
- flag_post_allowed_groups
- post_links_allowed_groups
- embedded_media_post_allowed_groups
- profile_background_allowed_groups
- user_card_background_allowed_groups
- invite_allowed_groups
- ignore_allowed_groups
- user_api_key_allowed_groups
When a user is manually deactivated, they should not be deleted by our background job that purges inactive users.
In addition, site settings keywords should accept an array of keywords.
This commit makes it so the site settings filter controls and
the list of settings input editors themselves can be used elsewhere
in the admin UI outside of /admin/site_settings
This allows us to provide more targeted groups of settings in different
UI areas where it makes sense to provide them, such as on plugin pages.
You could open a single page for a plugin where you can see information
about that plugin, change settings, and configure it with custom UIs
in the one place.
In future we will do this in "config areas" for other parts of the
admin UI.
For deprecated site settings, we log out a warning when
the old setting is used. However when we convert all the client
settings to JSON, we are creating a lot of log noise like this:
> Deprecation notice: `SiteSetting.anonymous_posting_min_trust_level` has been deprecated.
We don't need to do this because we are just dumping the JSON.
Plugins can use a new modifier to change which site settings are hidden using the :hidden_site_settings modifier. For example:
```
register_modifier(:hidden_site_settings) do |hidden|
(hidden + [:invite_only, :login_required]).uniq
end
```
* FEATURE: Add keywords support for site_settings search
This change allows for a new `keywords` field that can be added to site
settings in order to help with searching. Keywords are not visible in
the UI, but site settings matching one of the contained keywords will
appear when searching for that keyword.
Keywords can be added for site settings inside of the
`config/locales/server.en.yml` file under the new `keywords` key.
```
site_settings
example_1: "fancy description"
example_2: "another description"
keywords:
example_1: "capybara"
```
* Add keywords entry for a recently changed site setting and add system specs
* Use page.visit now that we have our own visit
Plugins can use a new modifier to change which site settings are
hidden using the :hidden_site_settings modifier. For example:
register_modifier(:hidden_site_settings) do |hidden|
(hidden + [:invite_only, :login_required]).uniq
end
Followup to eea74e0e32. Site settings
which are a list without a list_type should also have the _map
extension added which returns an array based on split("|").
For example:
```
SiteSetting.post_menu_map
=> ["read", "like"]
```
What is the problem?
In the test environement, we were calling `SiteSetting.setting` directly
to introduce new site settings. However, this leads to changes in state of the SiteSettings
hash that is stored in memory as test runs. Changing or leaking states
when running tests is one of the major contributors of test flakiness.
An example of how this resulted in test flakiness is our `spec/integrity/i18n_spec.rb` spec file which
had a test case that would fail because a new "plugin_setting" site
setting was registered in another test case but the site setting did not
have translations for the site setting set.
What is the fix?
There are a couple of changes being introduced in this commit:
1. Make `SiteSetting.setting` a private method as it is not safe to be
exposed as a public method of the `SiteSetting` class
2. Change test cases to use existing site settings in Discourse instead
of creating custom site settings. Existing site settings are not
removed often so we don't really need to dynamically add new site
settings in test cases. Even if the site settings being used in test
cases are removed, updating the test cases to rely on other site
settings is a very easy change.
3. Set up a plugin instance in the test environment as a "fixture"
instead of having each test create its own plugin instance.
Martin Brennanandawesomerobot
Alan Guo Xiang Tan
Natalie Tay
Jarek Radosz
Krzysztof Kotlarek
Blake Erickson
Sam
Régis Hanol
Loïc GuitautandJarek Radosz
Osama Sayegh
Neil Lalonde
Blake Erickson
Bianca Nenciu
Joffrey JAFFEUX