These tests are still flaky (order dependence) just that now it doesn't fail the test, instead it creates an infinite loop. Skipping these for now. We know they work because they pass, but they leak into other tests. I think we can re-enable locally and either fix or remove this once TL migration is done.
We're changing the implementation of trust levels to use groups. Part of this is to have site settings that reference trust levels use groups instead. It converts the min_trust_to_allow_self_wiki site setting to self_wiki_allowed_groups.
Nothing of note here. This is used in exactly one place, and there's no fallout.
There's a leaky test that breaks some controller tests if run first, creating an order-dependent flake.
This change fixes that, but in doing so also skips a low-value test that breaks from the fix. (Verified manually that it's working.)
Why this change?
On CI, we have been seeing the "handles job concurrency" job timing out
on CI after 45 seconds. Upon closer inspection of `Jobs::Base#perform`
when cluster concurrency has been set, we see that a thread is spun up
to extend the expiring of a redis key by 120 seconds every 60 seconds
while the job is still being executed. The thread looks like this before
the fix:
```
keepalive_thread =
Thread.new do
while parent_thread.alive? && !finished
Discourse.redis.without_namespace.expire(cluster_concurrency_redis_key, 120)
sleep 60
end
end
```
In an ensure block of `Jobs::Base#perform`, the thread is stop by doing
something like this:
```
finished = true
keepalive_thread.wakeup
keepalive_thread.join
```
If the thread is sleeping, `keepalive_thread.wakeup` will stop the
`sleep` method and run the next iteration causing the thread to
complete. However, there is a timing issue at play here. If
`keepalive_thread.wakeup` is called at a time when the thread is not
sleeping, it will have no effect and the thread may end up sleeping for
60 seconds which is longer than our timeout on CI of 45 seconds.
What does this change do?
1. Change `sleep 60` to sleep in intervals of 1 second checking if the
job has been finished each time.
2. Add `use_redis_snapshotting` to `Jobs::Base` spec since Redis is
involved in scheduling and we want to ensure we don't leak Redis
keys.
3. Add `ConcurrentJob.stop!` and `thread.join` to `ensure` block in "handles job concurrency"
test since a failing expectation will cause us to not clean up the
thread we created in the test.
When setting an old TL based site setting in the console e.g.:
SiteSetting.min_trust_level_to_allow_ignore = TrustLevel[3]
We will silently convert this to the corresponding Group::AUTO_GROUP. And vice-versa, when we read the value on the old setting, we will automatically get the lowest trust level corresponding to the lowest auto group for the new setting in the database.
Why this change?
When running system tests on our CI, we have been occasionally seeing
server errors like:
```
Error encountered while proccessing /stylesheets/desktop_e58cf7f686aab173f9b778797f241913c2833c39.css
NoMethodError: undefined method `+' for nil:NilClass
/__w/discourse/discourse/vendor/bundle/ruby/3.2.0/gems/actionpack-7.0.7/lib/action_dispatch/journey/path/pattern.rb:139:in `[]'
/__w/discourse/discourse/vendor/bundle/ruby/3.2.0/gems/actionpack-7.0.7/lib/action_dispatch/journey/router.rb:127:in `block (2 levels) in find_routes'
/__w/discourse/discourse/vendor/bundle/ruby/3.2.0/gems/actionpack-7.0.7/lib/action_dispatch/journey/router.rb:126:in `each'
/__w/discourse/discourse/vendor/bundle/ruby/3.2.0/gems/actionpack-7.0.7/lib/action_dispatch/journey/router.rb:126:in `each_with_index'
/__w/discourse/discourse/vendor/bundle/ruby/3.2.0/gems/actionpack-7.0.7/lib/action_dispatch/journey/router.rb:126:in `block in find_routes'
/__w/discourse/discourse/vendor/bundle/ruby/3.2.0/gems/actionpack-7.0.7/lib/action_dispatch/journey/router.rb:123:in `map!'
/__w/discourse/discourse/vendor/bundle/ruby/3.2.0/gems/actionpack-7.0.7/lib/action_dispatch/journey/router.rb:123:in `find_routes'
/__w/discourse/discourse/vendor/bundle/ruby/3.2.0/gems/actionpack-7.0.7/lib/action_dispatch/journey/router.rb:32:in `serve'
/__w/discourse/discourse/vendor/bundle/ruby/3.2.0/gems/actionpack-7.0.7/lib/action_dispatch/routing/route_set.rb:852:in `call'
```
While looking through various Rails issues related to the error above, I
came across https://github.com/rails/rails/pull/27647 which is a fix to
fully initialize routes before the first request is handled. However,
the routes are only fully initialize only if `config.eager_load` is set
to `true`. There is no reason why `config.eager_load` shouldn't be `true` in the
CI environment and this is what a new Rails 7.1 app is generated with.
What does this change do?
Enable `config.eager_load` when `env["CI"]` is present
A bug that allowed TL1 to convert other's posts to wiki.
The issue was introduced in this PR: https://github.com/discourse/discourse/pull/24999/files
The wiki can be created if a user is TL3 and it is their own post - default 3 for setting `SiteSetting.min_trust_to_allow_self_wiki`
In addition, a wiki can be created by staff and TL4 users for any post.
Follow-up to f5ca96528d
Why this change?
`RSpec.current_example.metadata[:extra_failure_lines]` can be `nil` and
calling `<<` on `nil` is not a good idea.
What does this change do?
Set `RSpec.current_example.metadata[:extra_failure_lines]` to `""` as
long as there are exceptions.
Why this change?
When running system tests with all official plugins installed, we have
encountered instances where the system tests will hang. When dumping the
backtraces of the threads, we can see that the main thread running the
tests is stuck in a deadlock with the puma thread while serving a
request.
The deadlock happens when the main thread acquires the `ActiveSupport::Concurrency::LoadInterlockAwareMonitor`
lock first in `ActiveRecord::ConnectionAdapters::AbstractAdapter` before acquring another `Monitor` lock in
`ActiveRecord::ModelSchema`. In the Puma thread, it acquires the
`Monitor` lock in `ActiveRecord::ModelSchema` first before acquring the
`ActiveSupport::Concurrency::LoadInterlockAwareMonitor` lock.
What does this change do?
To workaround this problem, we will preload all model schema cache
before running system tests such that the `Monitor` lock in `ActiveRecord::ModelSchema`
will not be acquired.
This changes the Plugins link in the admin sidebar to
be a section instead, which then shows all enabled plugin
admin routes (which are custom routes some plugins e.g.
chat define).
This is done via adding some special preloaded data for
all controllers based on AdminController, and also specifically
on Admin::PluginsController, to have the routes loaded without
additional requests on page load.
We just use a cog for all the route icons for now...we don't
have anything better.
Meta topic: https://meta.discourse.org/t/reseting-robots-txt-override-doesnt-seem-to-work-as-expected/287880?u=osama
Discourse provides a default version for `/robots.txt` which can be customized by admins in `/admin/customize/robots`. In that page, there's a button to reset back to the default version that Discourse provides. However, there's currently a bug with the reset button where the content appears to change to some HTML document instead of the default `robots.txt` version when clicking the button. Refreshing the page shows the true/correct content of `robots.txt` which is the default version, so the reset button actually works but there's a display problem.
What causes this display problem is that we use Rails' `render_to_string` method to generate the default content for `robots.txt` from the template, and what we get from that method is the `robots.txt` content wrapped in the application layout. To fix this issue, we need to pass `layout: false` to the `render_to_string` method so that it renders the template without any layouts.
(extracted from #23678)
* Move Wizard back into main app, remove Wizard addon
* Remove Wizard-related resolver or build hacks
* Install and enable `@embroider/router`
* Add "wizard" to `splitAtRoutes`
In a fully optimized Embroider app, route-based code splitting more
or less Just Work™ – install `@embroider/router`, subclass from it,
configure which routes you want to split and that's about it.
However, our app is not "fully optimized", by which I mean we are
not able to turn on all the `static*` flags.
In Embroider, "static" means "statically analyzable". Specifically
it means that all inter-dependencies between modules (files) are
explicitly expressed as `import`s, as opposed to `{{i18n ...}}`
magically means "look for the default export in app/helpers/i18n.js"
or something even more dynamic with the resolver.
Without turning on those flags, Embroider behaves conservatively,
slurps up all `app` files eagerly into the primary bundle/chunks.
So, while you _could_ turn on route-based code splitting, there
won't be much to split.
The commits leading up to this involves a bunch of refactors and
cleanups that 1) works perfectly fine in the classic build, 2) are
good and useful in their own right, but also 3) re-arranged things
such that most dependencies are now explicit.
With those in place, I was able to move all the wizard code into
the "app/static" folder. Embroider does not eagerly pull things from
this folder into any bundle, unless something explicitly "asks" for
them via `imports`. Conversely, things from this folder are not
registered with the resolver and are not added to the `loader.js`
registry.
In conjunction with route-based code splitting, we now have the
ability to split out islands of on-demand functionalities from the
main app bundle.
When you split a route in Embroider, it automatically creates a
bundle/entrypoint with the relevant routes/templates/controllers
matching that route prefix. Anything they import will be added to
the bundle as well, assuming they are not already in the main app
bundle, which is where the "app/static" folder comes into play.
The "app/static" folder name is not special. It is configured in
ember-cli-build.js. Alternatively, we could have left everything
in their normal locations, and add more fine-grained paths to the
`staticAppPaths` array. I just thought it would be easy to manage
and scale, and less error-prone to do it this way.
Note that putting things in `app/static` does not guarantee that
it would not be part of the main app bundle. For example, if we
were to add an `import ... from "app/static/wizard/...";` in a
main bundle file (say, `app.js`), then that chunk of the module
graph would be pulled in. (Consider using `await import(...)`?)
Overtime, we can build better tooling (e.g. lint rules and babel
macros to make things less repetitive) as we expand the use of
this pattern, but this is a start.
Co-authored-by: Godfrey Chan <godfreykfc@gmail.com>
Why this change?
Previously, we were attaching any server exception to the RSpec
example's `Exception#cause` by doing `example.exception.cause =
RspecErrorTracker.last_exception`. However, this is problematic because
it relies on RSpec internal implementation details where RSpec will
print out the exception's cause. The other problem is that when RSpec
prints out the exception cause, it only includes a single line of
backtrace which isn't very helpful sometimes.
While this change of tracking the last exception works OK for request
specs, it doesn't not work for system specs where multiple requests can
be triggered in an example potentially leading to multiple exceptions.
Knowing all the exceptions which happened in the request is important
for us when it comes to debugging system test failures.
What does this change do?
`RspecErrorTracker` now tracks all exceptions that occurs during an
RSpec example run. All the exceptions including the fullback trace of
each exception is printed out as part of the example's `extra_failure_lines` metadata.
Example:
```
Failures:
1) Shortcuts | mark all read when chat is open when pressing shift+esc marks all channels read
Failure/Error: expect(page).to have_content("all read messagasd")
expected to find text "all read messagasd" in "Topics\nMy Posts\nReview\nAdmin\nMore\nCategories\nAmazing Category 0\nAmazing Category 1\nAmazing Category 2\nUncategorized\nAll categories\nConfigure defaults\nMessages\nInbox\nMy threads\nChannels\nKino Buffs 2\nMusic Lodge 0\nMusic Lodge 1\nPersonal chat\nMusic Lodge 1\nChat settings have been set to retain channel messages for 90 days.\nToday\nbruce6\n2:46 pm\nall read message 0\nbruce7\n2:46 pm\nall read message 1\nbruce8\n2:46 pm\nall read message 2\nbruce9\n2:46 pm\nall read message 3\nbruce10\n2:46 pm\nall read message 4\nbruce11\n2:46 pm\nall read message 5\nbruce12\n2:46 pm\nall read message 6\nbruce13\n2:46 pm\nall read message 7\nbruce14\n2:46 pm\nall read message 8\nbruce15\n2:46 pm\nall read message 9\nShowing all messages"
[Screenshot Image]: /home/tgxworld/work/discourse/tmp/capybara/failures_r_spec_example_groups_shortcuts_mark_all_read_when_chat_is_open_when_pressing_shift_esc_marks_all_channels_read_236.png
~~~~~~~ SERVER EXCEPTIONS ~~~~~~~
Error encountered while proccessing /stylesheets/desktop_theme_1_5dba82f48b7d6e4a9d54ffd915712811591356b7.css
RuntimeError: boom
/home/tgxworld/work/discourse/app/controllers/application_controller.rb:996:in `set_cross_origin_opener_policy_header'
/home/tgxworld/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/activesupport-7.0.7/lib/active_support/callbacks.rb:400:in `block in make_lambda'
/home/tgxworld/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/activesupport-7.0.7/lib/active_support/callbacks.rb:236:in `block in halting_and_conditional'
/home/tgxworld/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/activesupport-7.0.7/lib/active_support/callbacks.rb:599:in `block in invoke_after'
/home/tgxworld/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/activesupport-7.0.7/lib/active_support/callbacks.rb:599:in `each'
/home/tgxworld/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/activesupport-7.0.7/lib/active_support/callbacks.rb:599:in `invoke_after'
/home/tgxworld/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/activesupport-7.0.7/lib/active_support/callbacks.rb:133:in `block in run_callbacks'
/home/tgxworld/work/discourse/app/controllers/application_controller.rb:423:in `block in with_resolved_locale'
/home/tgxworld/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/i18n-1.14.1/lib/i18n.rb:322:in `with_locale'
/home/tgxworld/work/discourse/app/controllers/application_controller.rb:423:in `with_resolved_locale'
/home/tgxworld/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/activesupport-7.0.7/lib/active_support/callbacks.rb:127:in `block in run_callbacks'
/home/tgxworld/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/activesupport-7.0.7/lib/active_support/callbacks.rb:138:in `run_callbacks'
/home/tgxworld/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/actionpack-7.0.7/lib/abstract_controller/callbacks.rb:233:in `process_action'
/home/tgxworld/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/actionpack-7.0.7/lib/action_controller/metal/rescue.rb:23:in `process_action'
/home/tgxworld/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/actionpack-7.0.7/lib/action_controller/metal/instrumentation.rb:67:in `block in process_action'
/home/tgxworld/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/activesupport-7.0.7/lib/active_support/notifications.rb:206:in `block in instrument'
/home/tgxworld/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/activesupport-7.0.7/lib/active_support/notifications/instrumenter.rb:24:in `instrument'
/home/tgxworld/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/activesupport-7.0.7/lib/active_support/notifications.rb:206:in `instrument'
/home/tgxworld/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/actionpack-7.0.7/lib/action_controller/metal/instrumentation.rb:66:in `process_action'
/home/tgxworld/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/actionpack-7.0.7/lib/action_controller/metal/params_wrapper.rb:259:in `process_action'
/home/tgxworld/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/activerecord-7.0.7/lib/active_record/railties/controller_runtime.rb:27:in `process_action'
/home/tgxworld/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/actionpack-7.0.7/lib/abstract_controller/base.rb:151:in `process'
/home/tgxworld/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/actionview-7.0.7/lib/action_view/rendering.rb:39:in `process'
/home/tgxworld/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/actionpack-7.0.7/lib/action_controller/metal.rb:188:in `dispatch'
/home/tgxworld/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/actionpack-7.0.7/lib/action_controller/metal.rb:251:in `dispatch'
/home/tgxworld/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/actionpack-7.0.7/lib/action_dispatch/routing/route_set.rb:49:in `dispatch'
/home/tgxworld/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/actionpack-7.0.7/lib/action_dispatch/routing/route_set.rb:32:in `serve'
/home/tgxworld/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/actionpack-7.0.7/lib/action_dispatch/journey/router.rb:50:in `block in serve'
/home/tgxworld/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/actionpack-7.0.7/lib/action_dispatch/journey/router.rb:32:in `each'
/home/tgxworld/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/actionpack-7.0.7/lib/action_dispatch/journey/router.rb:32:in `serve'
/home/tgxworld/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/actionpack-7.0.7/lib/action_dispatch/routing/route_set.rb:852:in `call'
/home/tgxworld/work/discourse/lib/middleware/omniauth_bypass_middleware.rb:64:in `call'
/home/tgxworld/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/rack-2.2.8/lib/rack/tempfile_reaper.rb:15:in `call'
/home/tgxworld/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/rack-2.2.8/lib/rack/conditional_get.rb:27:in `call'
/home/tgxworld/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/rack-2.2.8/lib/rack/head.rb:12:in `call'
/home/tgxworld/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/actionpack-7.0.7/lib/action_dispatch/http/permissions_policy.rb:38:in `call'
/home/tgxworld/work/discourse/lib/content_security_policy/middleware.rb:12:in `call'
/home/tgxworld/work/discourse/lib/middleware/anonymous_cache.rb:351:in `call'
/home/tgxworld/work/discourse/lib/middleware/gtm_script_nonce_injector.rb:10:in `call'
/home/tgxworld/work/discourse/spec/rails_helper.rb:47:in `call'
/home/tgxworld/work/discourse/config/initializers/008-rack-cors.rb:14:in `call'
/home/tgxworld/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/rack-2.2.8/lib/rack/session/abstract/id.rb:266:in `context'
/home/tgxworld/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/rack-2.2.8/lib/rack/session/abstract/id.rb:260:in `call'
/home/tgxworld/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/actionpack-7.0.7/lib/action_dispatch/middleware/cookies.rb:704:in `call'
/home/tgxworld/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/actionpack-7.0.7/lib/action_dispatch/middleware/callbacks.rb:27:in `block in call'
/home/tgxworld/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/activesupport-7.0.7/lib/active_support/callbacks.rb:99:in `run_callbacks'
/home/tgxworld/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/actionpack-7.0.7/lib/action_dispatch/middleware/callbacks.rb:26:in `call'
/home/tgxworld/work/discourse/plugins/discourse-geoblocking/lib/geoblocking_middleware.rb:24:in `call'
/home/tgxworld/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/actionpack-7.0.7/lib/action_dispatch/middleware/debug_exceptions.rb:28:in `call'
/home/tgxworld/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/actionpack-7.0.7/lib/action_dispatch/middleware/show_exceptions.rb:29:in `call'
/home/tgxworld/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/railties-7.0.7/lib/rails/rack/logger.rb:40:in `call_app'
/home/tgxworld/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/railties-7.0.7/lib/rails/rack/logger.rb:27:in `call'
/home/tgxworld/work/discourse/config/initializers/100-quiet_logger.rb:20:in `call'
/home/tgxworld/work/discourse/config/initializers/100-silence_logger.rb:29:in `call'
/home/tgxworld/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/actionpack-7.0.7/lib/action_dispatch/middleware/remote_ip.rb:93:in `call'
/home/tgxworld/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/actionpack-7.0.7/lib/action_dispatch/middleware/request_id.rb:26:in `call'
/home/tgxworld/work/discourse/lib/middleware/enforce_hostname.rb:24:in `call'
/home/tgxworld/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/rack-2.2.8/lib/rack/method_override.rb:24:in `call'
/home/tgxworld/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/actionpack-7.0.7/lib/action_dispatch/middleware/executor.rb:14:in `call'
/home/tgxworld/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/actionpack-7.0.7/lib/action_dispatch/middleware/static.rb:23:in `call'
/home/tgxworld/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/rack-2.2.8/lib/rack/sendfile.rb:110:in `call'
/home/tgxworld/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/actionpack-7.0.7/lib/action_dispatch/middleware/host_authorization.rb:131:in `call'
/home/tgxworld/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/message_bus-4.3.8/lib/message_bus/rack/middleware.rb:60:in `call'
/home/tgxworld/work/discourse/lib/middleware/request_tracker.rb:233:in `call'
/home/tgxworld/work/discourse/config/initializers/200-first_middlewares.rb:27:in `call'
/home/tgxworld/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/railties-7.0.7/lib/rails/engine.rb:530:in `call'
/home/tgxworld/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/rack-2.2.8/lib/rack/urlmap.rb:74:in `block in call'
/home/tgxworld/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/rack-2.2.8/lib/rack/urlmap.rb:58:in `each'
/home/tgxworld/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/rack-2.2.8/lib/rack/urlmap.rb:58:in `call'
/home/tgxworld/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/rack-2.2.8/lib/rack/builder.rb:244:in `call'
/home/tgxworld/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/capybara-3.39.2/lib/capybara/server/animation_disabler.rb:25:in `call'
/home/tgxworld/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/capybara-3.39.2/lib/capybara/server/middleware.rb:60:in `call'
/home/tgxworld/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/puma-6.4.0/lib/puma/configuration.rb:272:in `call'
/home/tgxworld/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/puma-6.4.0/lib/puma/request.rb💯in `block in handle_request'
/home/tgxworld/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/puma-6.4.0/lib/puma/thread_pool.rb:378:in `with_force_shutdown'
/home/tgxworld/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/puma-6.4.0/lib/puma/request.rb:99:in `handle_request'
/home/tgxworld/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/puma-6.4.0/lib/puma/server.rb:443:in `process_client'
/home/tgxworld/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/puma-6.4.0/lib/puma/server.rb:241:in `block in run'
/home/tgxworld/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/puma-6.4.0/lib/puma/thread_pool.rb:155:in `block in spawn_thread'
~~~~~~~ END SERVER EXCEPTIONS ~~~~~~~
~~~~~~~ JS LOGS ~~~~~~~
http://localhost:31337/stylesheets/desktop_theme_1_5dba82f48b7d6e4a9d54ffd915712811591356b7.css?__ws=localhost - Failed to load resource: the server responded with a status of 500 (Internal Server Error)
~~~~~ END JS LOGS ~~~~~
```
Why this change?
This is what `Capybara::Session#quit` does:
```
def quit
@driver.quit if @driver.respond_to? :quit
@document = @driver = nil
@touched = false
@server&.reset_error!
end
```
One notable thing is that it resets server errors which means that any
server errors encountered by a session is cleared. That is not what we
want since it hides errors even though `Capybara.raise_server_errors`
has been set to `true`.
Why this change?
This is part of our efforts to harden the security of the Discourse
application. Setting the `CROSS_ORIGIN_OPENER_POLICY` header to `same-origin-allow-popups`
by default makes the application safer. We have opted to make this a
hidden site setting because most admins will never have to care about
this setting so we're are opting not to show it. If they do have to
change it, they can still do so by setting the
`DISCOURSE_CROSS_ORIGIN_OPENER_POLICY` env.
Adds an API scope for accessing Logster's routes. This one is a bit
different than routes from core because it is mounted like
```
mount Logster::Web => "/logs"
```
and doesn't have all the route info a traditional rails app/engine does.
Ability to automatically generate migration when site setting is changed from trust level to groups.
Example usage:
rails generate site_setting_move_to_groups_migration min_trust_to_create_topic create_topic_allowed_groups
Settings that are using the new `file_size_restriction` types like the
`max_image_size_kb` setting need to have their values saved as integers.
This was a recent regression in 00209f03e6
that caused these values to be saved as strings.
This change also removes negatives from the validation regex because
file sizes can't be negative anyways.
Bug report: https://meta.discourse.org/t/289037
This commit refactor CategoryList to remove usage of EmberObject,
hopefully make the code more readable and fixes various edge cases with
lazy loaded categories (third level subcategories not being visible,
subcategories not being visible on category page, requesting for more
pages even if the last one did not return any results, etc).
The problems have always been here, but were not visible because a lot
of the processing was handled by the server and then the result was
serialized. With more of these being moved to the client side for the
lazy category loading, the problems became more obvious.
We're changing the implementation of trust levels to use groups. Part of this is to have site settings that reference trust levels use groups instead. It converts the min_trust_level_to_allow_ignore site setting to ignore_allowed_groups.
This PR maintains backwards compatibility until we can update plugins and themes using this.
We're changing the implementation of trust levels to use groups. Part of this is to have site settings that reference trust levels use groups instead. It converts the min_trust_level_to_allow_invite site setting to invite_allowed_groups.
Nothing much of note. This is used in one place and there's no fallout.
Why this change?
By default, `Capybara.default_max_wait_time` is set to `2`. However,
this is not a high enough default for Discourse as certain requests like
creating a post can take upwards of 2 seconds even on a high end desktop
CPU like the Ryzen 5950x. Therefore, we have decided to double the default max wait time.
Using min_trust_to_create_topic and create_topic_allowed_groups together was part of #24740
Now, when plugins specs are fixed, we can safely remove that part of logic.
This is v0 of admin sidebar navigation, which moves
all of the top-level admin nav from the top of the page
into a sidebar. This is hidden behind a enable_admin_sidebar_navigation
site setting, and is opt-in for now.
This sidebar is dynamically shown whenever the user enters an
admin route in the UI, and is hidden and replaced with either
the:
* Main forum sidebar
* Chat sidebar
Depending on where they navigate to. For now, custom sections
are not supported in the admin sidebar.
This commit removes the experimental admin sidebar generation rake
task but keeps the experimental sidebar UI for now for further
testing; it just uses the real nav as the default now.
Before, when needed to get stats in a plugin, we called Core classes directly.
Introducing plugin API will decouple plugins from Core and give as more freedom
in refactoring stats in Core. Without this API, I wasn't able to do all refactorings
I wanted when working on d91456f.
We're changing the implementation of trust levels to use groups. Part of this is to have site settings that reference trust levels use groups instead. It converts the min_trust_level_to_allow_user_card_background site setting to user_card_background_allowed_groups.
Nothing of note here. This is used in exactly one place, and there's no fallout.
This validator is used for site settings where one or more groups are to be input.
At the moment this validator just checks that the value isn't blank. This PR adds a validation for the existence of the groups passed in.
We're changing the implementation of trust levels to use groups. Part of this is to have site settings that reference trust levels use groups instead. It converts the tl4_delete_posts_and_topics site setting to delete_all_posts_and_topics_allowed_groups.
This one is a bit different from previous ones, as it's a boolean flag, and the default should be no group. Pay special attention to the migration during review.
* FEATURE: core code, tests for feature to allow backups to removed based on a time window
* FEATURE: getting tests working for time-based backup
* FEATURE: getting tests running
* FEATURE: linting
We're changing the implementation of trust levels to use groups. Part of this is to have site settings that reference trust levels use groups instead. It converts the min_trust_to_flag_posts site setting to flag_post_allowed_groups.
Note: In the original setting, "posts" is plural. I have changed this to "post" singular in the new setting to match others.
We're changing the implementation of trust levels to use groups. Part of this is to have site settings that reference trust levels use groups instead. It converts the min_trust_to_edit_post site setting to edit_post_allowed_groups.
The old implementation will co-exist for a short period while I update any references in plugins and themes.
This change converts the min_trust_to_create_topic site setting to
create_topic_allowed_groups.
See: https://meta.discourse.org/t/283408
- Hides the old setting
- Adds the new site setting
- Add a deprecation warning
- Updates to use the new setting
- Adds a migration to fill in the new setting if the old setting was
changed
- Adds an entry to the site_setting.keywords section
- Updates tests to account for the new change
- After a couple of months, we will remove the min_trust_to_create_topicsetting entirely.
Internal ref: /t/117248
This change converts the allow_uploaded_avatars site setting to uploaded_avatars_allowed_groups.
See: https://meta.discourse.org/t/283408
Hides the old setting
Adds the new site setting
Adds a deprecation warning
Updates to use the new setting
Adds a migration to fill in the new setting if the old setting was changed
Adds an entry to the site_setting.keywords section
Updates tests to account for the new change
After a couple of months, we will remove the allow_uploaded_avatars setting entirely.
Internal ref: /t/117248
What motivated this change?
Our builds on Github actions have been extremely flaky mostly due to system tests. This has led to a drop in confidence
in our test suite where our developers tend to assume that a failed job is due to a flaky system test. As a result, we
have had occurrences where changes that resulted in legitimate test failures are merged into the `main` branch because developers
assumed it was a flaky test.
What does this change do?
This change seeks to reduce the flakiness of our builds on Github Actions by automatically re-running RSpec tests once when
they fail. If a failed test passes subsequently in the re-run, we mark the test as flaky by logging it into a file on disk
which is then uploaded as an artifact of the Github workflow run. We understand that automatically re-runs will lead to
lower accuracy of our tests but we accept this as an acceptable trade-off since a fragile build has a much greater impact
on our developers' time. Internally, the Discourse development team will be running a service to fetch the flaky tests
which have been logged for internal monitoring.
How is the change implemented?
1. A `--retry-and-log-flaky-tests` CLI flag is added to the `bin/turbo_rspec` CLI which will then initialize `TurboTests::Runner`
with the `retry_and_log_flaky_tests` kwarg set to `true`.
2. When the `retry_and_log_flaky_tests` kwarg is set to `true` for `TurboTests::Runner`, we will register an additional
formatter `Flaky::FailuresLoggerFormatter` to the `TurboTests::Reporter` in the `TurboTests::Runner#run` method.
The `Flaky::FailuresLoggerFormatter` has a simple job of logging all failed examples to a file on disk when running all the
tests. The details of the failed example which are logged can be found in `TurboTests::Flaky::FailedExample.to_h`.
3. Once all the tests have been run once, we check the result for any failed examples and if there are, we read the file on
disk to fetch the `location_rerun_location` of the failed examples which is then used to run the tests in a new RSpec process.
In the rerun, we configure a `TurboTests::Flaky::FlakyDetectorFormatter` with RSpec which removes all failed examples from the log file on disk since those examples are not flaky tests. Note that if there are too many failed examples on the first run, we will deem the failures to likely not be due to flaky tests and not re-run the test failures. As of writing, the threshold of failed examples is set to 10. If there are more than 10 failed examples, we will not re-run the failures.
Ability to automatically generate migration when site setting name is changed.
Example usage: `rails generate site_setting_rename_migration site_description contact_email`
Applies the embed_unlisted site setting consistently across topic embeds, including those created via the WP Discourse plugin. Relatedly, adds a embed exception to can_create_unlisted_topic? check. Users creating embedded topics are not always staff.
Why this change?
The code changes introduced in 5b91dc1844
resulted in errors being raised when `session.quit` is called when using
multiple sessions. From my debugging, this seems to be attributed to the
fact that the change introduced resulted in multiple sessions sharing
the same instance of `Selenium::WebDriver::Remote::Http::Default`. While
sharing the same instance in theory should be fine, but the problem is
that `Selenium::WebDriver::Driver` will mutate the `server_url` of the
client in `Selenium::WebDriver::Remote::Bridge`. This is problematic
because each session created by capbyara relies on a different server
URL and this mutation causes all sorts of weird errors to occur.
To reproduce the problem, run `LOAD_PLUGINS=1 rspec plugins/chat/spec/system/send_message_spec.rb:76`
locally while excluding the changes in this commit.
This change converts the min_trust_to_edit_wiki_post site setting to edit_wiki_post_allowed_groups.
See: https://meta.discourse.org/t/283408
Hides the old setting
Adds the new site setting
Add a deprecation warning
Updates to use the new setting
Adds a migration to fill in the new setting if the old setting was changed
Adds an entry to the site_setting.keywords section
Updates tests to account for the new change
After a couple of months, we will remove the email_in_min_trust setting entirely.
Internal ref: /t/117248
A lot of work has been put in the select kits used for selecting
categories: CategorySelector, CategoryChooser, CategoryDrop, however
they still do not work as expected when these selectors already have
values set, because the category were still looked up in the list of
categories stored on the client-side Categrories.list().
This PR fixes that by looking up the categories when the selector is
initialized. This required altering the /categories/find.json endpoint
to accept a list of IDs that need to be looked up. The API is called
using Category.asyncFindByIds on the client-side.
CategorySelector was also updated to receive a list of category IDs as
attribute, instead of the list of categories, because the list of
categories may have not been loaded.
During this development, I noticed that SiteCategorySerializer did not
serializer all fields (such as permission and notification_level)
which are not a property of category, but a property of the relationship
between users and categories. To make this more efficient, the
preload_user_fields! method was implemented that can be used to
preload these attributes for a user and a list of categories.
When rebaking and in various other places for posts, we
run through the uploads and call `update_secure_status` on
each of them.
However, if the secure status didn't change, we were still
calling S3 to change the ACL, which would have been a noop
in many cases and takes ~1 second per call, slowing things
down a lot.
Also, we didn't account for the s3_acls_enabled site setting
being false here, and in the specs doing an assertion
that `Discourse.store.update_ACL` is not called doesn't
work; `Discourse.store` isn't a singleton, it re-initializes
`FileStore::S3Store.new` every single time.
Why this change?
The tags modal loads more tags via infinite loading based on when the last tag in the
given page appears in the viewport for the user. When it comes in to
view, a request is then triggered to fetch additional tags. To ensure
that we are only loading a single page of tags each time the modal is
opened, we previously set a max height on the modal's body to ensure
that the last tag which appears in the modal will be outside of the view
port in the initial load. However, this has regressed recently due to
unknown reasons and resulted in multiple pages of tags being loaded
immediately from the server as the modal's height was not restricted.
This regression was caught by an existing test but was unfortunately
determined as flaky.
What does this change do?
This change restores the max height on the edit navigation menu tags
modal on dekstop.
I took the wrong approach here, need to rethink.
* Revert "FIX: Use Guardian.basic_user instead of new (anon) (#24705)"
This reverts commit 9057272ee2.
* Revert "DEV: Remove unnecessary method_missing from GuardianUser (#24735)"
This reverts commit a5d4bf6dd2.
* Revert "DEV: Improve Guardian devex (#24706)"
This reverts commit 77b6a038ba.
* Revert "FIX: Introduce Guardian::BasicUser for oneboxing checks (#24681)"
This reverts commit de983796e1.
c.f. de983796e1
There will soon be additional login_required checks
for Guardian, and the intent of many checks by automated
systems is better fulfilled by using BasicUser, which
simulates a logged in TL0 forum user, rather than an
anon user.
In some cases the use of anon still makes sense (e.g.
anonymous_cache), and in that case the more explicit
`Guardian.anon_user` is used
The modified test used to be the same as the test above. The bad test
was introduced in commit 77d4c4d8dc,
during a refactoring.
This was not a serious problem because the same behavior was still
tested partially by the other tests below.
Through internal discussion, it has become clear that
we need a conceptual Guardian user that bridges the
gap between anon users and a logged in forum user with
an absolute baseline level of access to public topics,
which can be used in cases where:
1. Automated systems are running which shouldn't see any
private data
1. A baseline level of user access is needed
In this case we are fixing the latter; when oneboxing a local
topic, and we are linking to a topic in another category from
the current one, we need to operate off a baseline level of
access, since not all users have access to the same categories,
and we don't want e.g. editing a post with an internal link to
expose sensitive internal information.
We add `Access-Control-Allow-Origin: *` to all asset requests which are requested via a configured CDN. This is particularly important now that we're using browser-native `import()` to load the highlightjs bundle. Unfortunately, user-configurable 'cors_origins' site setting was overriding the wldcard value on CDN assets and causing CORS errors.
This commit updates the logic to give the `*` value precedence, and adds a spec for the situation. It also invalidates the cache of hljs assets (because CDNs will have cached the bad Access-Control-Allow-Origin header).
The rack-cors middleware is also slightly tweaked so that it is always inserted. This makes things easier to test and more consistent.
Why was the problem?
ActiveRecord's query cache for the connection pool wasn't disabled after the
`with a fake provider runs 'other_phase' for enabled auth methods` test
in `omniauth_callbacks_controller_spec.rb` was run. This was because the
Rack response body in `FakeAuthenticator::Strategy::other_phase` did not
adhere to the expected Rack body format which is "typically an Array of
String instances". Because this expectation was broken, it cascaded the
problem down where it resulted in the ActiveRecord's query cache for the
connection pool not being disabled as it normally should when the
response body is closed.
When the query cache is left enabled, common assertions pattern in RSpec
like `expect { something }.to change { Group.count }` will fail since
the query cache is enabled and the call first call to `Group.count` will
cache the result to be reused later on.
To see the bug in action, one can run the following command:
`bundle exec rspec --seed 44747
spec/requests/omniauth_callbacks_controller_spec.rb:1150
spec/models/group_spec.rb:283`
Followup e37fb3042d
* Automatically remove the prefix `Discourse ` from all the plugin titles to avoid repetition
* Remove the :discourse_dev: icon from the author. Consider a "By Discourse" with no labels as official
* We add a `label` metadata to plugin.rb
* Only plugins made by us in `discourse` and `discourse-org` GitHub organizations will show these in the list
* Make the plugin author font size a little smaller
* Make the commit sha look like a link so it's more obvious it goes to the code
Also I added some validation and truncation for plugin metadata
parsing since currently you can put absolutely anything in there
and it will show on the plugin list.
The category drop was rerendered after every category async change
because it updated the categories list. This is not necessary and
categories can be referenced indirectly by ID instead.
This change refactors the check `user.groups.any?` and instead uses
`user.staged?` to check if the user is staged or not.
Also fixes several tests to ensure the users have their auto trust level
groups created.
Follow up to:
- 8a45f84277
- 447d9b2105
- c89edd9e86
Why this change?
Asserting against records of the database in system tests can be flaky
because those assertions can run against the database before the server
has actually saved the necessary changes to the database.
What does this change do?
While the assertion is not ideal, we are working around this as a
temporary fix by using `try_until_success` which will retry the
assertion up till the default capybara timeout.
This value is included when generating static asset URLs. Updating the value will allow site operators to invalidate all asset urls to recover from configuration issues which may have been cached by CDNs/browsers.
When sending SMTP for group SMTP functionality, we
are running into timeouts for both read and open
when sending mail occassionally, which can cause issues
like the email only being sent to _some_ of the recipients
or to fail altogether.
The defaults of 5s are too low, so bumping them up to
the defaults of the `net-smtp` gem.
When we check upload security, one of the checks is to
run `access_control_post.with_secure_uploads?`. The problem
here is that the `topic` for the post could be deleted,
which would make the check return `nil` sometimes instead
of false because of safe navigation. We just need to be
more explicit.
Admin can add tag description up to 1000 characters.
Full description is displayed on tag page, however on topic list it is truncated to 80 characters.
Why this change?
In the `invites_controller_spec.rb` file, we had several tests that were
checking for assets path in the response's body to determine which
layout has been rendered. However, those test fails if `bin/ember-cli
--build` has been run locally.
What does this change do?
Instead of checking for asset paths to determine the layout that has
been rendered, this change relies on the fact that the `no_ember` layout
has a `no-ember` class on the `body` element. This is more deterministic
as compared to relying on the different asset paths that are rendered in
the response.
Followup to 2443446e62
We introduced video placeholders which prevent preloading
metadata for videos in posts. The structure looks like this
in HTML when the post is cooked:
```
<div class="video-placeholder-container" data-video-src="http://some-url.com/video.mp4" dir="ltr" style="cursor: pointer;">
<div class="video-placeholder-wrapper">
<div class="video-placeholder-overlay">
<svg class="fa d-icon d-icon-play svg-icon svg-string" xmlns="http://www.w3.org/2000/svg">
<use href="#play"></use>
</svg>
</div>
</div>
</div>
```
However, we did not update the code that links post uploads
to the post via UploadReference, so any videos uploaded since
this change are essentially dangling and liable to be deleted.
This also causes some uploads to be marked secure when they
shouldn't be, because they are not picked up and analysed in the
CookedPostProcessor flow.
Followup to e37fb3042d,
in some cases we cannot get git information for the
plugin folder (e.g. permission issues), so we need
to only try and get information about it if
commit_hash is present.
Reverts
- DEV: maxmind license checking failing tests #24534
- UX: Show if MaxMind key is missing on IP lookup #18993
These changes are leading to surprising results, our logs are now filling up with warnings on dev environments
We need the change to be redone
This improves the implementation of #18993
1. Error message displayed to user is clearer
2. open_db will also be called, even if license key is blank, as it was previously
3. This in turn means no need to keep stubbing 'maxmind_license_key'
The parent category needs to be serialized before the child category
because they are parsed in order. Otherwise the client will not build
the parent-child relationship correctly.
This change converts the `email_in_min_trust` site setting to
`email_in_allowed_groups`.
See: https://meta.discourse.org/t/283408
- Hides the old setting
- Adds the new site setting
- Add a deprecation warning
- Updates to use the new setting
- Adds a migration to fill in the new setting if the old setting was
changed
- Adds an entry to the site_setting.keywords section
- Updates tests to account for the new change
After a couple of months we will remove the
`email_in_min_trust` setting entirely.
Internal ref: /t/115696
Followup to e37fb3042d
Some plugins like discourse-ai and discourse-saml do not
nicely change from kebab-case to Title Case (e.g. Ai, Saml),
and anyway this method of getting the plugin name is not
translated either.
Better to use the plugin setting category if it exists,
since that is written by a human and is translated.
* DEV: Convert approve_new_topics_unless_trust_level to groups
This change converts the `approve_new_topics_unless_trust_level` site
setting to `approve_new_topics_unless_allowed_groups`.
See: https://meta.discourse.org/t/283408
- Hides the old setting
- Adds the new site setting
- Add a deprecation warning
- Updates to use the new setting
- Adds a migration to fill in the new setting if the old setting was
changed
- Adds an entry to the site_setting.keywords section
- Updates tests to account for the new change
After a couple of months we will remove the
`approve_new_topics_unless_trust_level` setting entirely.
Internal ref: /t/115696
* add missing translation
* Add keyword entry
* Add migration
This commit extracts the storage part of the route-scroll-manager into a dedicated service. This provides a key/value store which will reset for each navigation, and restore previous values when the user uses the back/forward buttons in their browser.
This gives us a reliable replacement for the old `DiscourseRoute.isPoppedState` function, which would not work under all situations.
Previously reverted in e6370decfd. This version has been significantly refactored, and includes an additional system spec for the issue we identified.
We were throwing ArgumentError in UrlHelper.normalised_encode,
but it was incorrect -- we were passing ArgumentError.new
2 arguments which is not supported. Fix this and have a hint
of which URL is causing the issue for debugging.
This change converts the `approve_unless_trust_level` site setting to
`approve_unless_allowed_groups`.
See: https://meta.discourse.org/t/283408
- Adds the new site setting
- Adds a deprecation warning
- Updates core to use the new settings.
- Adds a migration to fill in the new setting of the old setting was
changed
- Adds an entry to the site_setting.keywords section
- Updates many tests to account for the new change
After a couple of months we will remove the `approve_unless_trust_level`
setting entirely.
Internal ref: /t/115696
Why this change?
The test was randomly failing in
https://github.com/discourse/discourse/actions/runs/6936264158/job/18868087113
with the following failure:
```
expect do user.update_ip_address!("127.0.0.1") end.to change {
UserIpAddressHistory.where(user_id: user.id).count
}.by(1)
expected `UserIpAddressHistory.where(user_id: user.id).count` to have changed by 1, but was changed by 0
```
This is due to the fact that ActiveRecord will actually cache the result
of `UserIpAddressHistory.where(user_id: user.id).count`. However,
`User.update_ip_address!` relies on mini_sql and does not go through
ActiveRecord. As a result, the query cache is not cleared and hence the
flakiness.
What does this change do?
This change uses the `uncached` method provided by ActiveRecord when
we are fetching the count.
* Remove checkmark for official plugins
* Add author for plugin, which is By Discourse for all discourse
and discourse-org github plugins
* Link to meta topic instead of github repo
* Add experimental flag for plugin metadata and show this as a
badge on the plugin list if present
---------
Co-authored-by: chapoi <101828855+chapoi@users.noreply.github.com>
In the long term we should aim to modernize these places, but for now this change will make them compatible with Ember 5.x (while maintaining compatibility with Ember 3.28)
This commit adds a new `search_default_sort_order` site setting,
set to "relevance" by default, that controls the default sort order
for the full page /search route.
If the user changes the order in the dropdown on that page, we remember
their preference automatically, and it takes precedence over the site
setting as a default from then on. This way people who prefer e.g.
Latest Post as their default can make it so.
Currently to use a limit in the notifications index, you have to also pass recent: true as a param.
This PR:
Adds optional limit param to be used in the notifications query, regardless of the presence of recent
Raises the max limit of the response with recent present from 50 -> 60. It is super weird we have a hard-limit of 50 before with recent param, and 60 without the param.
config.after(:suite) which stops minio server is called every time one
of the groups of parallel tests complete. This works fine most of the
time with parallel spec runs, but sometimes one of these
MinioRunner.stop calls happens while a spec is running in another
process that expects the minio server to be running.
Skipping these tests to avoid flakys for now.
Why this change?
The test became flaky due to d208396c5c.
In that commit, we introduced `page.has_no_css?("div.menu-panel.animating")` to `PageObjects::Components::NavigationMenu::Sidebar#open_on_mobile` but
it did not work as intended because `page.has_no_css?("div.menu-panel.animating")` can return `true` immediately as the `animating` class has not been added
to the element.
What does this change do?
Switch to the `wait_for_animation` system helper to ensure that all
animations have ended on the element.
When we started using NumberField for integer site settings
in e113eff663, we did not end up
passing down a min/max value for the integer to the field, which
meant that for some fields where negative numbers were allowed
we were not accepting that as valid input.
This commit passes down the min/max options from the server for
integer settings then in turn passes them down to NumberField.
c.f. https://meta.discourse.org/t/delete-user-self-max-post-count-not-accepting-1-to-disable/285162
This PR refactors the following:
* leaving all the CSS applied to the old `modal-body` classes in their respective files
* made new clean styling for `.d-modal` and refactored the template to use the new BEM classes
* `inner-`, `middle-`, `outer-` container classes are gone and replaced with simplified `wrapper` and `container` classes
* use standardised max-sizes with modifiers `-large` and `-max`
* lighter backdrop,
* min-width to prevent puny modals
* other styling changes regarding padding, close button,…
* pulled out all modal overrides into a general `modal-overrides` file + cleanup of outdated CSS
* pulled out login and create account modal styling into their own file, cause it's such a big override
* removed old general login.scss file for mobile & desktop
* only kept some remainders I don't want to touch in `app/assets/stylesheets/common/base/login.scss`
Previously we would only recompile a theme locale when its own data changes. However, the output also includes fallback data from other locales, so we need to invalidate all locales when fallback locale data is changed. Building a list of dependent locales is tricky, so let's just invalidate them all.
We ask users to confirm their session if they are making a sensitive
action, such as adding/updating second factors or passkeys. This
commit adds the ability to confirm sessions with passkeys as an option
to the password confirmation.
The `src` of js files is now dependent on the ember-cli/webpack build, so it's not a good thing to check in specs. In CI it passes because the ember-cli build is not run. But locally it would fail if you had a build in `app/assets/javascripts/discourse/dist`.
This commit updates the specs to check for the presence of a stable data attribute instead.
