This commit fixes two issues:
1. The wrong exception was being printed as the 'cause' in turbo_rspec output. This was happening because RSpec [expects exceptions to be subclasses of `Exception`](d6e320dc11/lib/rspec/core/formatters/exception_presenter.rb (L102)). This commit resolves the issue by replacing the `FakeException` `Struct` with a subclass of `Exception`.
2. The `full_cause_backtrace` option we set in `rails_helper.rb` does not carry through to the RSpec formatters running in the turbo_rspec reporter process. To fix that, this commit duplicates the necessary config in `lib/turbo_tests.rb`.
Example before - note that the cause is a duplicate of the original exception, and only has three lines of backtrace:
```
Failure/Error: raise capybara_timeout_error
CapybaraTimeoutExtension::CapybaraTimedOut:
This spec passed, but capybara waited for the full wait duration (4s) at least once. This will slow down the test suite. Beware of negating the result of selenium's RSpec matchers.
[Screenshot Image]: /Users/david/discourse/discourse/tmp/capybara/failures_r_spec_example_groups_glimmer_header_when_cmd_f_keyboard_shortcut_pressed_when_within_a_topic_with_less_than20_posts_does_not_open_search_484.png
~~~~~~~ JS LOGS ~~~~~~~
~~~~~ END JS LOGS ~~~~~
# ./spec/rails_helper.rb:372:in `block (3 levels) in <top (required)>'
# ./spec/rails_helper.rb:472:in `block (2 levels) in <top (required)>'
# /Users/david/.rvm/gems/ruby-3.2.1/gems/webmock-3.23.0/lib/webmock/rspec.rb:39:in `block (2 levels) in <top (required)>'
# ------------------
# --- Caused by: ---
# CapybaraTimeoutExtension::CapybaraTimedOut:
# This spec passed, but capybara waited for the full wait duration (4s) at least once. This will slow down the test suite. Beware of negating the result of selenium's RSpec matchers.
# ./spec/rails_helper.rb:372:in `block (3 levels) in <top (required)>'
# ./spec/rails_helper.rb:472:in `block (2 levels) in <top (required)>'
# /Users/david/.rvm/gems/ruby-3.2.1/gems/webmock-3.23.0/lib/webmock/rspec.rb:39:in `block (2 levels) in <top (required)>'
```
After - note correct causing exception, and the full backtrace 🎉
```
Failure/Error: raise capybara_timeout_error
CapybaraTimeoutExtension::CapybaraTimedOut:
This spec passed, but capybara waited for the full wait duration (4s) at least once. This will slow down the test suite. Beware of negating the result of selenium's RSpec matchers.
[Screenshot Image]: /Users/david/discourse/discourse/tmp/capybara/failures_r_spec_example_groups_glimmer_header_when_cmd_f_keyboard_shortcut_pressed_when_within_a_topic_with_less_than20_posts_does_not_open_search_61.png
~~~~~~~ JS LOGS ~~~~~~~
~~~~~ END JS LOGS ~~~~~
# ./spec/rails_helper.rb:372:in `block (3 levels) in <top (required)>'
# ./spec/rails_helper.rb:472:in `block (2 levels) in <top (required)>'
# /Users/david/.rvm/gems/ruby-3.2.1/gems/webmock-3.23.0/lib/webmock/rspec.rb:39:in `block (2 levels) in <top (required)>'
# ------------------
# --- Caused by: ---
# Capybara::ExpectationNotMet:
# expected to find css ".search-menu .search-menu-panel" but there were no matches
# /Users/david/.rvm/gems/ruby-3.2.1/gems/capybara-3.40.0/lib/capybara/node/matchers.rb:112:in `block in assert_selector'
# /Users/david/.rvm/gems/ruby-3.2.1/gems/capybara-3.40.0/lib/capybara/node/matchers.rb:869:in `block in _verify_selector_result'
# /Users/david/.rvm/gems/ruby-3.2.1/gems/capybara-3.40.0/lib/capybara/node/base.rb:84:in `synchronize'
# ./spec/rails_helper.rb:345:in `synchronize'
# /Users/david/.rvm/gems/ruby-3.2.1/gems/capybara-3.40.0/lib/capybara/node/matchers.rb:868:in `_verify_selector_result'
# /Users/david/.rvm/gems/ruby-3.2.1/gems/capybara-3.40.0/lib/capybara/node/matchers.rb:110:in `assert_selector'
# /Users/david/.rvm/gems/ruby-3.2.1/gems/capybara-3.40.0/lib/capybara/node/matchers.rb:39:in `block in has_selector?'
# /Users/david/.rvm/gems/ruby-3.2.1/gems/capybara-3.40.0/lib/capybara/node/matchers.rb:902:in `make_predicate'
# /Users/david/.rvm/gems/ruby-3.2.1/gems/capybara-3.40.0/lib/capybara/node/matchers.rb:39:in `has_selector?'
# /Users/david/.rvm/gems/ruby-3.2.1/gems/capybara-3.40.0/lib/capybara/session.rb:774:in `has_selector?'
# ./spec/system/page_objects/pages/search.rb:46:in `has_search_menu_visible?'
# ./spec/system/header_spec.rb:206:in `block (4 levels) in <main>'
# ./spec/rails_helper.rb:472:in `block (2 levels) in <top (required)>'
# /Users/david/.rvm/gems/ruby-3.2.1/gems/webmock-3.23.0/lib/webmock/rspec.rb:39:in `block (2 levels) in <top (required)>'
```
When "lazy load categories" is enabled, the CategoryDrop component will
render at most 15 categories. If there are more categories, a "Show
more" link pointing to the categories page will be displayed.
Previously services would let you define a high level default `def default_actions_for_service; end` which would define various handlers like `on_success`, after months of usage we consider the cons are superior to the pros here.
Two mains cons:
- people would often not understand where the handling was coming from
- it's easy to miss a case when you write your specs
Why this change?
When creating a new theme setting that does not have a corresponding row
in the `theme_settings` table, we end up writing to the database twice
because `ActiveRecord::Base#save!` is called once before the `value`
or `json_value` column is updated again with another database query with
another call to `ActiveRecord::Base#save!`.
What does this change do?
Adds the column to be updated to argument for the `ActiveRecord::Base#create!`
method call so that we only have one write query to the database.
Why this change?
Assertions against the database in a system test is not reliable because
the request sent from the client side may not have been processed when
the query to the database has been run.
The test was added to prevent a regression for 63119144ff
but it turns out that the test will still prevent the regression even if
we do not assert against the state in the database.
The build is broken due to some changes not being staged when I pushed the previous PR. The assertions that check that a job has been scheduled needs to be updated to reflect the new name.
Doing the following renames:
Jobs::ProblemChecks → Jobs::RunProblemChecks
Jobs::ProblemCheck → Jobs::RunProblemCheck
This is to disambiguate the ProblemCheck class name, ease fuzzy finding, and avoid needing to use :: in a bunch of places.
Before, the `back to forum` link was part of experimental admin navigation. It means that the link could be filtered out.
Because it is essential navigation, it should not be part of sidebar links and should be moved above the filter.
This option was introduced at some point in the past, but was removed
during the work necessary to make Discourse work with a large number of
categories.
Follow up to commit 2e68ead45b.
Having minitest as a direct dependency causes ruby-lsp to use it as our test runner (per https://github.com/Shopify/ruby-lsp/blob/d1da8858a1/lib/ruby_lsp/requests/support/dependency_detector.rb#L40-L55). This makes VSCode's test explorer incorrectly display Minitest 'run' buttons above all our tests.
We were only using it in `emoji.rake`... and that wasn't even working with the latest version of Minitest. This commit refactors `emoji.rake` to work without minitest, and removes the dependency.
Sometimes we add scripts outside of Rails. This commit provides a way to generate a nonce placeholder even if you don't have access to an ApplicationController instance.
Forcing a thread will work even in channel which don't have `threading_enabled` or in direct message channels.
For now this feature is only available through the `ChatSDK`:
```ruby
ChatSDK::Message.create(in_reply_to_id: 1, guardian: guardian, raw: "foo bar baz", channel_id: 2, force_thread: true)
```
Why this change?
There are two problematic queries in question here when loading
notifications in various tabs in the user menu:
```
SELECT "notifications".*
FROM "notifications"
LEFT JOIN topics ON notifications.topic_id = topics.id
WHERE "notifications"."user_id" = 1338 AND (topics.id IS NULL OR topics.deleted_at IS NULL)
ORDER BY notifications.high_priority AND NOT notifications.read DESC,
NOT notifications.read AND notifications.notification_type NOT IN (5,19,25) DESC,
notifications.created_at DESC
LIMIT 30;
```
and
```
EXPLAIN ANALYZE SELECT "notifications".*
FROM "notifications"
LEFT JOIN topics ON notifications.topic_id = topics.id
WHERE "notifications"."user_id" = 1338
AND (topics.id IS NULL OR topics.deleted_at IS NULL)
AND "notifications"."notification_type" IN (5, 19, 25)
ORDER BY notifications.high_priority AND NOT notifications.read DESC, NOT notifications.read DESC, notifications.created_at DESC LIMIT 30;
```
For a particular user, the queries takes about 40ms and 26ms
respectively on one of our production instance where the user has 10K notifications while the site has 600K notifications in total.
What does this change do?
1. Adds the `index_notifications_user_menu_ordering` index to the `notifications` table which is
indexed on `(user_id, (high_priority AND NOT read) DESC, (NOT read)
DESC, created_at DESC)`.
1. Adds a second index `index_notifications_user_menu_ordering_deprioritized_likes` to the `notifications`
table which is indexed on `(user_id, (high_priority AND NOT read) DESC, (NOT read AND notification_type NOT IN (5,19,25)) DESC, created_at DESC)`. Note that we have to hardcode the like typed notifications type here as it is being used in an ordering clause.
With the two indexes above, both queries complete in roughly 0.2ms. While I acknowledge that there will be some overhead in insert,update or delete operations. I believe this trade-off is worth it since viewing notifications in the user menu is something that is at the core of using a Discourse forum so we should optimise this experience as much as possible.
Why this change?
When a site's default locale is changed, Discobot's `UserProfile#bio_raw` is not
changed and we have gotten reports about this.
What does this change do?
This change adds a `DiscourseEvent.on(:site_setting_changed)` callback
which watches for changes to the `default_locale` site setting and
updates Discobot's `UserProfile#bio_raw` when it changes.
Why this change?
Instead of manually loading files, we should just structure the plugin
so that it relies on Rails autoload strategy and avoid all the manual
`require_relative`s.
What does this change do?
1. Structure the plugin to use Rails autoloading convention
2. Remove onceff jobs that were added 5-6 years ago. There is no need to
carry these jobs anymore after such a long time.
3. Move setting of `SiteSetting.discourse_narrative_bot_enabled` to
`false` in the test environment from core into the plugin.
* DEV: add topic-map-expanded glimmer component
* DEV: remove topic-map-expanded widgets from topic-map
* DEV: add noreferrer for _blank target and add table grouping to template
* DEV: negate base styling of tbody element so expanded topic map retains current look
* DEV: pass in title to TopicParticipants as internationalized string instead of renderable HTML and set TRUNCATED_LINKS_LIMIT constant
Why this change?
The `/admin/customize/themes/:id/schema/name` route is a work in
progress but we want to be able to start navigating to it from the
`/admin/customize/themes/:id` route.
What does this change do?
1. Move `adminCustomizeThemes.schema` to a child route of
`adminCustomizeThemes.show`. This is because we need the model
from the parent route and if it isn't a child route we end up
having to load the theme model again from the server.
1. Add the `objects_schema` attribute to `ThemeSettingsSerializer`
1. Refactor `SiteSettingComponent` to be able to render a button
so that we don't have to hardcode the button rendering into the
`SiteSettings::String` component
