The `using_wait_time` DSL did not work when used in the `around` context
as the Capybara's session has not been initialized.
Follow-up to afde7cc1727a751e4d87be2ceee17f2d82edf89c
I'm not sure what is causing the tests to be flaky so I need more
information to determine where the problem is. For now, I'm doubling the
wait time to rule out any server side problem.
We are not waiting for asynchronous operations to complete before
executing our assertions.
### Reviewer notes
Example test failure:
https://github.com/discourse/discourse/actions/runs/14100970402/job/39496976787
```
Failure/Error: measurement = Benchmark.measure { example.run }
expected `Chat::Channel.count` to have changed by 1, but was changed by 0
[Screenshot Image]: /__w/discourse/discourse/tmp/capybara/failures_r_spec_example_groups_chat_new_message_from_params_with_multiple_users_creates_a_dm_channel_when_none_exists_626.png
~~~~~~~ JS LOGS ~~~~~~~
~~~~~ END JS LOGS ~~~~~
./plugins/chat/spec/system/chat_new_message_spec.rb:57:in `block (3 levels) in <main>'
./spec/rails_helper.rb:619:in `block (3 levels) in <top (required)>'
/var/www/discourse/vendor/bundle/ruby/3.3.0/gems/benchmark-0.4.0/lib/benchmark.rb:304:in `measure'
./spec/rails_helper.rb:619:in `block (2 levels) in <top (required)>'
./spec/rails_helper.rb:580:in `block (3 levels) in <top (required)>'
/var/www/discourse/vendor/bundle/ruby/3.3.0/gems/timeout-0.4.3/lib/timeout.rb:185:in `block in timeout'
/var/www/discourse/vendor/bundle/ruby/3.3.0/gems/timeout-0.4.3/lib/timeout.rb:192:in `timeout'
./spec/rails_helper.rb:570:in `block (2 levels) in <top (required)>'
./spec/rails_helper.rb:527:in `block (2 levels) in <top (required)>'
/var/www/discourse/vendor/bundle/ruby/3.3.0/gems/webmock-3.25.1/lib/webmock/rspec.rb:39:in `block (2 levels) in <top (required)>'
```
Some site settings support backfilling if the user specified it. This works fine for singular site settings sent to the SiteSettingsController#update endpoint, but with bulk save we need to support this for a list of settings as well.
This change alters the params format for SiteSetting::Update.
It also moves the backfill logic into the service.
A system test in `system/search_spec.rb` was failing with the following
error frequently on CI:
```
Failure/Error: expect(search_page).to have_heading_text("Search")
expected `#<PageObjects::Pages::Search:0x00007fb9fcd3f028>.has_heading_text?("Search")` to be truthy, got false
[Screenshot Image]: /__w/discourse/discourse/tmp/capybara/failures_r_spec_example_groups_search_when_using_full_page_search_on_mobile_works_and_clears_search_page_state_912.png
~~~~~~~ JS LOGS ~~~~~~~
(no logs)
~~~~~ END JS LOGS ~~~~~
./spec/system/search_spec.rb:42:in `block (3 levels) in <main>'
./spec/rails_helper.rb:619:in `block (3 levels) in <top (required)>'
/var/www/discourse/vendor/bundle/ruby/3.3.0/gems/benchmark-0.4.0/lib/benchmark.rb:304:in `measure'
./spec/rails_helper.rb:619:in `block (2 levels) in <top (required)>'
./spec/rails_helper.rb:580:in `block (3 levels) in <top (required)>'
/var/www/discourse/vendor/bundle/ruby/3.3.0/gems/timeout-0.4.3/lib/timeout.rb:185:in `block in timeout'
/var/www/discourse/vendor/bundle/ruby/3.3.0/gems/timeout-0.4.3/lib/timeout.rb:192:in `timeout'
./spec/rails_helper.rb:570:in `block (2 levels) in <top (required)>'
./spec/rails_helper.rb:527:in `block (2 levels) in <top (required)>'
/var/www/discourse/vendor/bundle/ruby/3.3.0/gems/webmock-3.25.1/lib/webmock/rspec.rb:39:in `block (2 levels) in <top (required)>'
```
The failure screenshot shows that the "user" is on the homepage even
though we have already clicked the search icon and ensured that the user
can see the search container. I suspect there is some sort of race
condition here since Capybara executes clicks in quick sucession where
we clicked on both the homepage logo and the search icon. It may be
possible that Ember redirected the user to the search page first
before the browser was able to finish navigating the user to the `/`
href.
### Reviewer notes
Test flaked in
https://github.com/discourse/discourse/actions/runs/14085443789/job/39448197089
with the following failure screenshot:
This is to provide diagnostic information for some flaky system tests
which we are currently investigating. The previous attempt in
6f92f42eb83fea61b2712aaaefcc124e4939365d only dumped threads that
contains the `puma` gem in the backtrace but that is not enough. Since I
am now printing all thread backtraces, I added the
`dump_threads_on_failure` metadata
which will be used to determine if all the thread backtraces should be
printed out in the spec's failure output.
* Prevents tag separators (commas by default) from wrapping separately
by using absolute positioning so they don't impact the width (thus a
comma can't wrap on its own, because a tag can't influence the width)
* Removes some whitespace creating extra space between categories and
tags (to better match desktop)
* Removes a redundant `flex-wrap: wrap` as `.discourse-tags` already
carries this
* Improves vertical alignment of commas (they were too high on mobile,
which is avoided on desktop with baseline alignment in flex)
* Fixes an issue where tag and separator color could be mismatched
because of a too-broad color being applied to all links
Before:
After:
Small alignment and sizing improvements for emojis/icons added to
categories.
We are also fixing the private category locked icon to retain the grey
color.
This patch adds a new shared example to be used as a smoke test in
plugins and themes.
A `skip_examples` argument is available to easily opt-out from a
category of tests.
Example:
```rb
RSpec.describe "Testing core features", type: :system do
it_behaves_like "having working core features", skip_examples: %i[search login]
end
```
When a Capybara helper times out after hitting
`Capybara.default_max_wait_time`, we will now
dump the puma server threads backtraces. This information will help us
diagnose test failures due to requests not completing within
`Capybara.default_max_wait_time`.
Example test failure output:
```
Failures:
1) User resetting password when desktop when user has multi-factor authentication configured when user has TOTP, security key and backup codes configured should allow a user to toggle from security key to TOTP and between TOTP and backup codes
Failure/Error: expect(page).to have_current_path("/u/#{user.username}/preferences/second-factor")
expected `#<Capybara::Session>.has_current_path?("/u/john/preferences/second-factor")` to be truthy, got false
[Screenshot Image]: /Users/tgxworld/work/discourse/tmp/capybara/failures_r_spec_example_groups_user_resetting_password_when_desktop_when_user_has_multi_factor_authentication_configured_when_user_has_totp_security_key_and_backup_codes_configured_should_allow_a_user_to_toggl_874.png
~~~~~~~ SERVER THREADS BACKTRACES ~~~~~~~
/Users/tgxworld/work/discourse/app/controllers/users_controller.rb:2216:in 'Kernel#sleep'
/Users/tgxworld/work/discourse/app/controllers/users_controller.rb:2216:in 'UsersController#confirm_secure_session'
/Users/tgxworld/work/discourse/app/controllers/users_controller.rb:1543:in 'UsersController#confirm_session'
/Users/tgxworld/work/discourse/app/controllers/application_controller.rb:427:in 'block in ApplicationController#with_resolved_locale'
/Users/tgxworld/work/discourse/app/controllers/application_controller.rb:427:in 'ApplicationController#with_resolved_locale'
/Users/tgxworld/work/discourse/lib/middleware/omniauth_bypass_middleware.rb:35:in 'Middleware::OmniauthBypassMiddleware#call'
/Users/tgxworld/work/discourse/lib/content_security_policy/middleware.rb:12:in 'ContentSecurityPolicy::Middleware#call'
/Users/tgxworld/work/discourse/lib/middleware/anonymous_cache.rb:368:in 'Middleware::AnonymousCache#call'
/Users/tgxworld/work/discourse/lib/middleware/csp_script_nonce_injector.rb:12:in 'Middleware::CspScriptNonceInjector#call'
/Users/tgxworld/work/discourse/spec/rails_helper.rb:48:in 'RspecErrorTracker#call'
/Users/tgxworld/work/discourse/config/initializers/008-rack-cors.rb:14:in 'Discourse::Cors#call'
/Users/tgxworld/work/discourse/lib/middleware/default_headers.rb:13:in 'Middleware::DefaultHeaders#call'
/Users/tgxworld/work/discourse/config/initializers/100-quiet_logger.rb:20:in 'DiscourseRackQuietAssetsLogger#call'
/Users/tgxworld/work/discourse/config/initializers/100-silence_logger.rb:29:in 'SilenceLogger#call'
/Users/tgxworld/work/discourse/lib/middleware/enforce_hostname.rb:24:in 'Middleware::EnforceHostname#call'
/Users/tgxworld/work/discourse/lib/middleware/processing_request.rb:12:in 'Middleware::ProcessingRequest#call'
/Users/tgxworld/work/discourse/lib/middleware/request_tracker.rb:385:in 'Middleware::RequestTracker#call'
/Users/tgxworld/work/discourse/config/initializers/200-first_middlewares.rb:30:in 'TestMultisiteMiddleware#call'
/Users/tgxworld/work/discourse/config/initializers/200-first_middlewares.rb:77:in 'BlockRequestsMiddleware#call'
~~~~~~~ END SERVER THREADS BACKTRACES ~~~~~~~
~~~~~~~ JS LOGS ~~~~~~~
(no logs)
~~~~~ END JS LOGS ~~~~~
```
### Reviewer notes
In this [flaky test
example](https://github.com/discourse/discourse/actions/runs/14087382392/job/39454911943),
the test failed because a capybara assertion did not pass within
`Capybara.default_max_wait_time`. From the screenshot below, we see that
the submit button is still loading which indicates that the request is
still being processed by the server. However, we need to know why the
server is taking more than 10 seconds to generate a response.
```
Failure/Error: expect(page).to have_current_path("/u/#{user.username}/preferences/second-factor")
expected `#<Capybara::Session>.has_current_path?("/u/john/preferences/second-factor")` to be truthy, got false
[Screenshot Image]: /__w/discourse/discourse/tmp/capybara/failures_r_spec_example_groups_user_resetting_password_when_desktop_when_user_has_multi_factor_authentication_configured_when_user_only_has_security_key_configured_should_allow_a_user_to_reset_password_with_a__75.png
~~~~~~~ JS LOGS ~~~~~~~
(no logs)
~~~~~ END JS LOGS ~~~~~
Shared Example Group: "forgot password scenarios" called from ./spec/system/forgot_password_spec.rb:213
./spec/system/page_objects/pages/user_preferences_security.rb:15:in `visit_second_factor'
./spec/system/forgot_password_spec.rb:30:in `create_user_security_key'
./spec/system/forgot_password_spec.rb:79:in `block (4 levels) in <main>'
./spec/rails_helper.rb:619:in `block (3 levels) in <top (required)>'
/var/www/discourse/vendor/bundle/ruby/3.3.0/gems/benchmark-0.4.0/lib/benchmark.rb:304:in `measure'
./spec/rails_helper.rb:619:in `block (2 levels) in <top (required)>'
./spec/rails_helper.rb:580:in `block (3 levels) in <top (required)>'
/var/www/discourse/vendor/bundle/ruby/3.3.0/gems/timeout-0.4.3/lib/timeout.rb:185:in `block in timeout'
/var/www/discourse/vendor/bundle/ruby/3.3.0/gems/timeout-0.4.3/lib/timeout.rb:192:in `timeout'
./spec/rails_helper.rb:570:in `block (2 levels) in <top (required)>'
./spec/rails_helper.rb:527:in `block (2 levels) in <top (required)>'
/var/www/discourse/vendor/bundle/ruby/3.3.0/gems/webmock-3.25.1/lib/webmock/rspec.rb:39:in `block (2 levels) in <top (required)>'
```
This change disables the save and undo buttons for site settings while the setting is being saved. This gives some visual indication that saving is underway, and prevents unnecessarily sending more than one request (which will be no-ops anyway.)
This update adds a new value transformer to the `PostTextSelection`
component. This allows for dynamically setting a `preventClose`
property. This is useful to prevent the `onSelectionChange` listener
from firing a toolbar close. In particular, we want to use this in
Discourse AI to prevent the selection change from closing the toolbar
when selecting new text inside the explain popup
(https://github.com/discourse/discourse-ai/pull/1221)
This was accidentally reverted as part of the rebasing/merging of the
gjs conversions in b29e0b6e1b6001890ce92f96448258550f80132b
Co-authored-by: Robert <35533304+merefield@users.noreply.github.com>
Follow-up to bbea7c3b11054b8e7faeee85a0336ecc45b84890
This behavior should only be applied to mobile view, otherwise there are
some scenarios (Android tablets) where the composer toolbar will become
inaccessible.
This commit improves said method to ensure that user is redirected to
the right page before returning.
### Reviewer notes
Example of test flakiness:
https://github.com/discourse/discourse/actions/runs/14081653020/job/39435797236
```
Failure/Error: raise capybara_timeout_error
CapybaraTimeoutExtension::CapybaraTimedOut:
This spec passed, but capybara waited for the full wait duration (10s) at least once. This will slow down the test suite. Beware of negating the result of selenium's RSpec matchers.
[Screenshot Image]: /__w/discourse/discourse/tmp/capybara/failures_r_spec_example_groups_user_resetting_password_when_desktop_when_user_has_multi_factor_authentication_configured_when_user_has_security_key_and_backup_codes_configured_should_allow_a_user_to_reset_pass_261.png
~~~~~~~ JS LOGS ~~~~~~~
~~~~~ END JS LOGS ~~~~~
Shared Example Group: "forgot password scenarios" called from ./spec/system/forgot_password_spec.rb:213
./spec/rails_helper.rb:426:in `block (3 levels) in <top (required)>'
./spec/rails_helper.rb:619:in `block (3 levels) in <top (required)>'
/var/www/discourse/vendor/bundle/ruby/3.3.0/gems/benchmark-0.4.0/lib/benchmark.rb:304:in `measure'
./spec/rails_helper.rb:619:in `block (2 levels) in <top (required)>'
./spec/rails_helper.rb:580:in `block (3 levels) in <top (required)>'
/var/www/discourse/vendor/bundle/ruby/3.3.0/gems/timeout-0.4.3/lib/timeout.rb:185:in `block in timeout'
/var/www/discourse/vendor/bundle/ruby/3.3.0/gems/timeout-0.4.3/lib/timeout.rb:192:in `timeout'
./spec/rails_helper.rb:570:in `block (2 levels) in <top (required)>'
./spec/rails_helper.rb:527:in `block (2 levels) in <top (required)>'
/var/www/discourse/vendor/bundle/ruby/3.3.0/gems/webmock-3.25.1/lib/webmock/rspec.rb:39:in `block (2 levels) in <top (required)>'
```
