Previously, I thought it was better to drop the site into reading mode
when Redis has failed over to the replica but it created more errors
while Redis is in readonly mode since ActiveRecord would prevent us from
writing to PG even though PG is up.
The risk here is that the database for one site goes down in the multisite setup and we drop everything to readonly mode. However, I discussed this with Sam and we agree that one database having problem is very rare. Most of the time, it is the entire DB cluster that goes down.
This reverts commit 20780a1eee.
* SECURITY: re-adds accidentally reverted commit:
03d26cd6: ensure embed_url contains valid http(s) uri
* when the merge commit e62a85cf was reverted, git chose the 2660c2e2 parent to land on
instead of the 03d26cd6 parent (which contains security fixes)
Upgrades Rails to latest, this version has better compatibility
with Ruby 2.7
During the upgrade we needed a new cleaner mechanism for configuring
message bus.
All tests are green.
If anything weird pops up please revert.
We have the `# frozen_string_literal: true` comment on all our
files. This means all string literals are frozen. There is no need
to call #freeze on any literals.
For files with `# frozen_string_literal: true`
```
puts %w{a b}[0].frozen?
=> true
puts "hi".frozen?
=> true
puts "a #{1} b".frozen?
=> true
puts ("a " + "b").frozen?
=> false
puts (-("a " + "b")).frozen?
=> true
```
For more details see: https://samsaffron.com/archive/2018/02/16/reducing-string-duplication-in-ruby
* DEV: Use `render_json_error` (Adds specs for Admin::GroupsController)
* DEV: Use a specific error on blank category slug (Fixes a `render_json_error` warning)
* DEV: Use a specific error on reviewable claim conflict (Fixes a `render_json_error` warning)
* DEV: Use specific errors in Admin::UsersController (Fixes `render_json_error` warnings)
* FIX: PublishedPages error responses
* FIX: TopicsController error responses (There was an issue of two separate `Topic` instances for the same record. This makes sure there's only one up-to-date instance.)
Previously we would consider a user "present" and "last seen" if the
browser window was visible.
This has many edge cases, you could be considered present and around for
days just by having a window open and no screensaver on.
Instead we now also check that you either clicked, transitioned around app
or scrolled the page in the last minute in combination with window
visibility
This will lead to more reliable notifications via email and reduce load of
message bus for cases where a user walks away from the terminal
If the “secure media” site setting is enabled then ALL files uploaded to Discourse (images, video, audio, pdf, txt, zip etc. etc.) will follow the secure media rules. The “prevent anons from downloading files” setting will no longer have any bearing on upload security. Basically, the feature will more appropriately be called “secure uploads” instead of “secure media”.
This is being done because there are communities out there that would like all attachments and media to be secure based on category rules but still allow anonymous users to download attachments in public places, which is not possible in the current arrangement.
Rails calls I18n.translate during initialization and by default translation overrides are used. Database migrations would fail if the system tried to migrate from an old version that didn't have the `translation_overrides` table with all its columns yet.
This makes restoring really old backups work again. Running `DISABLE_TRANSLATION_OVERRIDES=1 rake db:migrate` will allow you to upgrade such an old database as well.
* Remove some `.es6` from comments where it does not matter
* Use a post processor for transpilation
This will allow us to eventually use the directory structure to
transpile rather than the extension.
* FIX: Some errors and clean up in confirm-new-email
It would throw an error if the webauthn element wasn't present.
Also I changed things so that no-module is not explicitly
referenced.
* Remove `no-module`
Instead we allow a magic comment: `// discourse-skip-module` to prevent
the asset pipeline from creating a module.
* DEV: Enable babel transpilation based on directory
If it's in `app/assets/javascripts/dicourse` it will be transpiled
even without the `.es6` extension.
* REFACTOR: Remove Tilt/ES6ModuleTranspiler
* FIX: We need to skip users with associated reviewables when auto-approving them
* Update spec/initializers/track_setting_changes_spec.rb
* Update spec/initializers/track_setting_changes_spec.rb
Co-authored-by: Robin Ward <robin.ward@gmail.com>
Tracking down concurrency issues from backtraces and manual repros is a fraught process.
Sometimes you've just got to get your hands dirty and do a live debug.
Previously we had many places in the app that called `hostname` to get
hostname of a server. This commit replaces the pattern in 2 ways
1. We cache the result in `Discourse.os_hostname` so it is only ever called once
2. We prefer to use Socket.gethostname which avoids making a shell command
This improves performance as we are not spawning hostname processes throughout
the app lifetime
This is not used in core or official plugins, and has been printing a deprecation notice since v2.3.0beta4. All OpenID 2.0 code and dependencies have been dropped. The user_open_ids table remains for now, in case anyone has missed the deprecation notice, and needs to migrate their data.
Context at https://meta.discourse.org/t/-/113249
Out-of-the-box Oj uses :object mode, this shifts us to use :compat mode
by default which is safer.
It means any de-serialization going forward will default to this mode.
If we wish to serialize or deserialize arbitrary objects going forward with
no json interfaces we will have to opt in.
- Refactor source_url to avoid using eval in development
- Precompile handlebars in development
- Include template compilers when running qunit
- Remove unsafe-eval in development CSP
- Include unsafe-eval only for qunit routes in development
According to the [Rails
Source](https://github.com/rails/rails/blob/master/activerecord/lib/active_record/railties/databases.rake#L20)
the `ActiveRecord::Migrator.migrations_paths` are overwritten with the
value of `ActiveRecord::Tasks::DatabaseTasks.migrations_paths` every
time the config is loaded.
This caused a bug for Discourse development where if you ran:
`rake db:drop db:create db:migrate` in one line, you would not get our
post migrations, as those had a custom value for `migrations_paths`.
The fix is to use `ActiveRecord::Tasks::DatabaseTasks.migrations_paths`
to set up all our custom paths. Everything seems to work as expected.
Post-zeitwerk, rails has deprecated autoloading modules during
initialization and forces all autoloaded modules to be reloaded after
initialization.
Requiring the file explicitly prevents autoloading and therefore
prevents the state on SiteSetting being trashed which was causing the
problem here.
Zeitwerk simplifies working with dependencies in dev and makes it easier reloading class chains.
We no longer need to use Rails "require_dependency" anywhere and instead can just use standard
Ruby patterns to require files.
This is a far reaching change and we expect some followups here.
Under extreme load on large databases certain regular jobs can take quite
a while to run. We need to ensure we never starve a sidekiq from running
mini scheduler, cause without it we are unable to queue stuff such as
heartbeat jobs.
This adds a 1 minute rate limit to all JS error reporting per IP. Previously
we would only use the global rate limit.
This also introduces DISCOURSE_ENABLE_JS_ERROR_REPORTING, if it is set to
false then no JS error reporting will be allowed on the site.
Previously the default stack suppressor in rack-mini-profiler was excluding
the plugin directory.
This made islolating issues more complicated cause you needed to defer to
pp=full-backtrace which is both slow and noisy
If enabled, this will fire a webhook whenever a user's notification has
been created. This could potentially be a lot of data depending on your
forum, and should be used carefully since it includes everything all users
will see in their feeds.
The message_bus performs a fair amount of work prior to hijacking requests
this change ensures that if there is a situation where the server is flooded
message_bus will inform client to back off for 30 seconds + random(120 secs)
This back-off is ultra cheap and happens very early in the middleware.
It corrects a situation where a flood to message bus could cause the app
to become unresponsive
MessageBus update is here to ensure message_bus gem properly respects
Retry-After header and status 429.
Under normal state this code should never trigger, to disable raise the
value of DISCOURSE_REJECT_MESSAGE_BUS_QUEUE_SECONDS, default is to tell
message bus to go away if we are queueing for 100ms or longer
This is useful when a backup is restored on a staging site or in a development environment. It also deletes all existing push subscriptions because they get invalid when the keys change.
Sometimes we would like to create a base image without any DB access, this
assists in creating custom base images with custom plugins that already
includes `public/assets`
Following this change set you can run:
```
SPROCKETS_CONCURRENT=1 DONT_PRECOMPILE_CSS=1 SKIP_DB_AND_REDIS=1 RAILS_ENV=production bin/rake assets:precompile
```
Then it is straight forward to create a base image without needing a DB or
Redis.
* Support private uploads in S3
* Use localStore for local avatars
* Add job to update private upload ACL on S3
* Test multisite paths
* update ACL for private uploads in migrate_to_s3 task
This adds support for DISCOURSE_ENABLE_PERFORMANCE_HTTP_HEADERS
when set to `true` this will turn on performance related headers
```text
X-Redis-Calls: 10 # number of redis calls
X-Redis-Time: 1.02 # redis time in seconds
X-Sql-Commands: 102 # number of SQL commands
X-Sql-Time: 1.02 # duration in SQL in seconds
X-Queue-Time: 1.01 # time the request sat in queue (depends on NGINX)
```
To get queue time NGINX must provide: HTTP_X_REQUEST_START
We do not recommend you enable this without thinking, it exposes information
about what your page is doing, usually you would only enable this if you
intend to strip off the headers further down the stream in a proxy
This backend is a bit faster and well tested, this is part of a longer
term plan to have a `backend: :memory, threaded: false` type config for
message bus which we can use in test.
The threading in message bus causes all sorts of surprises in test, it will
be nice not to be beholden to them.
Adds `DISCOURSE_MESSAGE_BUS_REDIS_ENABLED` env var, that when set
to true, will allow Discourse to connect to a different redis
instance for MessageBus needs.
When enabled you can configure the same env vars user for redis,
but prefixed by `MESSAGE_BUS`, eg:
`DISCOURSE_MESSAGE_BUS_REDIS_HOST`
This reduces chances of errors where consumers of strings mutate inputs
and reduces memory usage of the app.
Test suite passes now, but there may be some stuff left, so we will run
a few sites on a branch prior to merging
Benchmarking:
```
Benchmark.ips do |b|
b.report("simple") do
User.first
end
end
ActiveSupport::Notifications.notifier.listeners_for("sql.active_record").clear
Benchmark.ips do |b|
b.report("simple") do
User.first
end
end
```
```
sam@arch discourse % RAILS_ENV=production ruby script/micro_bench.rb
Before
Calculating -------------------------------------
simple 3.289k (± 4.4%) i/s - 16.575k in 5.049771s
After
Calculating -------------------------------------
simple 3.491k (± 3.6%) i/s - 17.442k in 5.002226s
````
This change automatically resizes icons for various purposes. Admins can now upload `logo` and `logo_small`, and everything else will be auto-generated. Specific icons can still be uploaded separately if required.
## Core
- Adds an SiteIconManager module which manages automatic resizing and fallback
- Icons are looked up in the OptimizedImage table at runtime, and then cached in Redis. If the resized version is missing for some reason, then most icons will fall back to the original files. Some icons (e.g. PWA Manifest) will return `nil` (because an incorrectly sized icon is worse than a missing icon).
- `SiteSetting.site_large_icon_url` will return the optimized version, including any fallback. `SiteSetting.large_icon` continues to return the upload object. This means that (almost) no changes are required in core/plugins to support this new system.
- Icons are resized whenever a relevant site setting is changed, and during post-deploy migrations
## Wizard
- Allows `requiresRefresh` wizard steps to reload data via AJAX instead of a full page reload
- Add placeholders to the **icons** step of the wizard, which automatically update from the "Square Logo"
- Various copy updates to support the changes
- Remove the "upload-time" resizing for `large_icon`. This is no longer required.
## Site Settings UX
- Move logo/icon settings under a new "Branding" tab
- Various copy changes to support the changes
- Adds placeholder support to the `image-uploader` component
- Automatically reloads site settings after saving. This allows setting placeholders to change based on changes to other settings
- Upload site settings will be assigned a placeholder if SiteIconManager `responds_to?` an icon of the same name
## Dashboard Warnings
- Remove PWA icon and PWA title warnings. Both are now handled automatically.
## Bonus
- Updated the sketch logos to use @awesomerobot's new high-res designs
Without forcing a reload on start internal state in the accelerator can be
off. In Rails 5 not translation is being called so this is not an issue but
in 6 it is called earlier on.
* 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.
Since Rails 5.2, the behavior of `attribute_changed?` inside `after_save` callbacks has changed, so we need to use `saved_change_to_attribute` instead. The site setting local_process_provider in test mode was covering up the issue.
If you turn it on now, default all users to approved since they were
previously. Also support approving a user that doesn't have a reviewable
record (it will be created first.)
This also includes a refactor to move class method calls to
`DiscourseEvent` into an initializer. Otherwise the load order of
classes makes a difference in the test environment and some settings
might be triggered and others not, randomly.
Includes support for flags, reviewable users and queued posts, with REST API
backwards compatibility.
Co-Authored-By: romanrizzi <romanalejandro@gmail.com>
Co-Authored-By: jjaffeux <j.jaffeux@gmail.com>
This cleans up logster configuration a bit cause we no longer have to
check if we respond_to anything and keeps the logster limit properly
documented
Followup on da578e92
Ruby 2.5.3 has an upatched issue that crashes unicorn after fork:
https://bugs.ruby-lang.org/issues/14634
This will be patched in 2.5.4 however for now just warn people dev is slower
and disable async logging on the older rubies
On the first migration, trying to access the users table will throw an
error in PostgreSQL's log which has been confusing since users will
report it to us when rebuild fails.
This avoids require dependency on method_profiler and anon cache.
It means that if there is any change to these files the reloader will not pick it up.
Previously the reloader was picking up the anon cache twice causing it to double load on boot.
This caused warnings.
Long term my plan is to give up on require dependency and instead use:
https://github.com/Shopify/autoload_reloader
This provides us with instrumentation missing after rails upgrade
Latest version of rails uses exec_params internally which is no longer
routed to intercepted methods in mini profiler 1.0.0
ActiveRecord defines automatic scopes for enums, the Poll model defines
an enum for `{open: 1}` this mean Rails wants the scope `Poll.all.open`
to work which in turn means it has to override `open` which is defined
privately.
Rails feature req exists for: https://github.com/rails/rails/issues/34599
which will allow us to define enums without scopes which would resolve this
a lot more cleaner.
* Add missing icons to set
* Revert FA5 revert
This reverts commit 42572ff
* use new SVG syntax in locales
* Noscript page changes (remove login button, center "powered by" footer text)
* Cast wider net for SVG icons in settings
- include any _icon setting for SVG registry (offers better support for plugin settings)
- let themes store multiple pipe-delimited icons in a setting
- also replaces broken onebox image icon with SVG reference in cooked post processor
* interpolate icons in locales
* Fix composer whisper icon alignment
* Add support for stacked icons
* SECURITY: enforce hostname to match discourse hostname
This ensures that the hostname rails uses for various helpers always matches
the Discourse hostname
* load SVG sprite with pre-initializers
* FIX: enable caching on SVG sprites
* PERF: use JSONP for SVG sprites so they are served from CDN
This avoids needing to deal with CORS for loading of the SVG
Note, added the svg- prefix to the filename so we can quickly tell in
dev tools what the file is
* Add missing SVG sprite JSONP script to CSP
* Upgrade to FA 5.5.0
* Add support for all FA4.7 icons
- adds complete frontend and backend for renamed FA4.7 icons
- improves performance of SvgSprite.bundle and SvgSprite.all_icons
* Fix group avatar flair preview
- adds an endpoint at /svg-sprites/search/:keyword
- adds frontend ajax call that pulls icon in avatar flair preview even when it is not in subset
* Remove FA 4.7 font files
* First take on subsetting svg icons
* FontAwesome 5 svg subset WIP
* Include icons from plugins/badges into svg sprite subset
* add svg icon support to themes
* Add spec for SvgSprite
* Misc. SVG icon fixes
* Use FA5 svgs in local-dates plugin
* CSS adjustments, fix SVG icons in group flair
* Use SVG icons in poll plugin
* Add SVG icons to /wizard
This moves us away from the delayed drops pattern which
was problematic on two counts. First, it uses a hardcoded "delay for"
duration which may be too short for certain deployment strategies.
Second, delayed drop doesn't ensure that it only runs after
the latest application code has been deployed. If the migration runs
and the application code fails to deploy, running the migration after
"delay for" has been met will cause the application to blow up.
The new strategy allows post deployment migrations to be skipped if the
env `SKIP_POST_DEPLOYMENT_MIGRATIONS` is provided.
```
SKIP_POST_DEPLOYMENT_MIGRATIONS=1 rake db:migrate
-> deploy app servers
SKIP_POST_DEPLOYMENT_MIGRATIONS=0 rake db:migrate
```
To aid with the generation of a post deployment migration, a generator
has been added. Simply run `rails generate post_migration`.
This commit also cleans up a bunch of pointless noise each time we boot app
- narrative was loading i18n cause redefinition of consts
- discourse.rb was loaded twice as was auth
- bin/unicorn now does all the smart things and boots unicron in dev
- bin/rails s will boot unicorn with no params
- remove bin/puma which only causes confusion
Introduce new patterns for direct sql that are safe and fast.
MiniSql is not prone to memory bloat that can happen with direct PG usage.
It also has an extremely fast materializer and very a convenient API
- DB.exec(sql, *params) => runs sql returns row count
- DB.query(sql, *params) => runs sql returns usable objects (not a hash)
- DB.query_hash(sql, *params) => runs sql returns an array of hashes
- DB.query_single(sql, *params) => runs sql and returns a flat one dimensional array
- DB.build(sql) => returns a sql builder
See more at: https://github.com/discourse/mini_sql
* Feature: Push notifications for Android
Notification config for desktop and mobile are merged.
Desktop notifications stay as they are for desktop views.
If mobile mode, push notifications are enabled.
Added push notification subscriptions in their own table, rather than through
custom fields.
Notification banner prompts appear for both mobile and desktop when enabled.
Daniel Waterworth
Penar Musaraj
Bianca Nenciu
Guo Xiang Tan
Guo Xiang Tan
Michael Brown
Jeff Atwood
Sam Saffron
David Taylor
Jarek Radosz
Dan Ungureanu
Kane York
Martin Brennan
Gerhard Schlager
Robin Ward
Roman Rizzi
Matt Palmer
OsamaSayegh
Vinoth Kannan
Joffrey JAFFEUX
Arpit Jalan
Krzysztof Kotlarek
Bianca Nenciu
Rafael dos Santos Silva
Kyle Zhao
Davide Porrovecchio
Vinoth Kannan
CheshireVillageSoftware
Neil Lalonde
Dax74
Jeff Wong
scossar