We had quite a few cases in core where inputs are being mutated as a side
effect of calling a method.
This handles all the cases where specs caught this.
Mutating inputs makes code harder to reason about. Eg:
```
frog = "frog"
jump(frog)
puts frog
"fly" # ?????
```
This commit is part of a followup commit that adds # frozen_string_literal
to all our specs.
* DEV: Replace site_setting_saved DiscourseEvent with site_setting_changed
site_setting_saved is confusing for a few reasons:
- It is attached to the after_save of the ActiveRecord model. This is confusing because it only works 'properly' with the db_provider
- It passes the activerecord model as a parameter, which is confusing because you get access to the 'database' version of the setting, rather than the ruby setting. For example, booleans appear as 'y' or 'n' strings.
- When the event is called, the local process cache has not yet been updated. So if you call SiteSetting.setting_name inside the event handler, you will receive the old site setting value
I have deprecated that event, and added a new site_setting_changed event. It passes three parameters:
- Setting name (symbol)
- Old value (in ruby format)
- New value (in ruby format)
It is triggered after the setting has been persisted, and the local process cache has been updated.
This commit also includes a test case which describes the confusing behavior. This can be removed once site_setting_saved is removed.
SiteSettingExtension triggers message bus which re-establishes a
DB connection in `SiteSettingExtension#process_message`. That happens
concurrently and a test that requires a connection to the db will
fail when the reconnection is happening.
This splits off the logic between SSO keys used incoming vs outgoing, it allows to far better restrict who is allowed to log in using a site.
This allows for better auditing of the SSO provider feature
This refactors it so "Defaults provider" is only responsible for "defaults"
Locale handling and management of locale settings is moved back into
SiteSettingExtension
This eliminates complex state management using DistributedCache and makes
it way easier to test SiteSettingExtension
* `rescue nil` is a really bad pattern to use in our code base.
We should rescue errors that we expect the code to throw and
not rescue everything because we're unsure of what errors the
code would throw. This would reduce the amount of pain we face
when debugging why something isn't working as expexted. I've
been bitten countless of times by errors being swallowed as a
result during debugging sessions.
This change-set allows setting different defaults for different locales.
It also:
- Adds extensive testing around site setting validation
- raises deprecation error if site setting has the default property based on env
- relocated site settings for dev and tests in the initializer
- deprecated client_setting in the site setting's loading process
- ensure it raises when a enum site setting being set
- default_locale is promoted to `required` category.
- fixes incorrect default setting and validation
- fixes ensure type check for site settings
- creates a benchmark for site setting
- sets reasonable defaults for Chinese
FEATURE: new 'max_oneboxes_per_post' site setting
FEATURE: change onebox whitelist to a blacklist
PERF: debounce the loading of oneboxes
PERF: improve perf of mention links in preview
FIX: sort loading of custom oneboxer
Due to https://github.com/docker/docker/issues/9298 it is a huge pain
to remove ENV vars when composing images, allow us to simply treat "blank"
as a ENV var that is not being shadowed. In general we always supply a
value to ENV vars we are shadowing.
Sam Saffron
David Taylor
Guo Xiang Tan
Guo Xiang Tan
Gerhard Schlager
Maja Komel
Régis Hanol
OsamaSayegh
Arpit Jalan
Erick Guan
Robin Ward
Neil Lalonde
Kane York