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)>'
```
This commit adds a loading state to the confirm button in
`confirm-session` dialog.
This also unskips the flaky system tests in
`spec/system/forgot_password_spec.rb` as this change will allow us to
gather more information about why the test is flaky. The screenshots
which we have gathered when the test flakes does not allow us to know if
the button has been clicked or not before the test times out.
This switches the signup/login UI to the full page experience by
default. This has been in use by many sites for multiple months and we
have ironed out many fixes in the meantime.
The `full_page_login` setting is also marked for removal in about
1.5mths, by the end of April 2025.
This commit unskips 3 flaky system tests and gives up on asserting that
redirecting is done correctly. This is because we have invested
considerable effort into this and cannot figure it out. The redirect is
tested by the client side anyway so there is still some test coverage.
Tight lists are lists that look like this:
* Item 1
* Item 2
* Item 3
Loose lists look like this:
* Item 1
* Item 2
* Item 3
There is a place for the latter, but the former is more
common default behaviour for writing apps and widgets, so
we are overriding the prosemirror default to use tight lists.
Eventually we will have a shortcut or other special behaviour
to switch between the list styles.
When editing a color palette via the new page introduced in
https://github.com/discourse/discourse/pull/31742, it should apply the
color changes for the admin making the change automatically upon save.
Internal topic: t/148628/12.
The tests being fixed in this commit are not waiting for the
asynchronous behaviour of bulk closing topic to complete before running
some of the assertions. This commit updates the tests to check for the
asynchronous behaviour to complete so that the following assertions will
pass with more reliability.
This commit removes an assertion for the redirect after 2FA
authentication is success message because the message is flashed briefly
before a route transition happens. A proper fix would require us to
redesign when/how the flash message which we can address in the future.
Meta:
https://meta.discourse.org/t/cant-erase-the-bookmark-search-input/357861/4
> When we fill in the bookmark search input and send the request, then
we can’t delete the input’s content.
After removing the last character, the `searchTerm` getter is called. At
this point, `_searchTerm` is empty.
However, `this._searchTerm || this.q` will treat the empty string as
_falsy_, and the `q` value is displayed instead.
It makes it impossible to clear the input.
To fix this, we check specifically on the initial state of
`_searchTerm,` which is _undefined_, to include an empty string as a
valid value.
Note: because of `@computed`, the issue is not triggered when the
content is selected and deleted.
This commit updates the 2FA token submission page to disable the submit
button when the 2FA token is not valid and to also set the submit button
to be in the loading state after the submit button has been clicked.
The UX issues were discovered while I was investigating a flaky test
which has been unskipped in this commit as well. I am not sure if this
will completely resolve the flakiness but we have to unskip it to know
if it continues to be flaky.
Now we have the search input showing in a few
different configurations:
* Welcome banner
* Header field
* Header icon
And we can get to the search with both `/` and
`Ctrl+F` shortcuts. These configurations can
be used together, and we need to focus on the right
search input at the right time.
This commit fixes the shortcuts not working
or showing the wrong thing in some cases,
and adds a comprehensive system spec for all
the variants.
Fixes an incorrect logic on `transformPasted` that was causing a
`TypeError: Cannot read properties of null (reading 'marks')` when only
part of a paragraph was selected.
Adds tests
Follow-up to https://github.com/discourse/discourse/pull/30953
This PR is a partial revert of the linked PR — it changes the "Themes
and components" link in the admin sidebar back to the legacy
`/admin/customize/themes` page and adds the themes list/sidebar back to
the left hand side of the page. The new `/admin/config/customize/` route
is still available, but it's not linked from anywhere. When accessing
the new page and then navigating to a theme, the old components (e.g.
the themes list) of the page are hidden. This allows us to iterate on
the new page and improve until we're more ready to make it available
more widely.
Both tests being unskipped here failed previosly with the following
error:
```
Failure/Error: expect(page).to have_current_path("/u/#{user.username}/preferences/account")
expected "/u/confirm-new-email/f42a416fcbca40d66788d65a8837ad49" to equal "/u/bruce306/preferences/account"
./spec/system/email_change_spec.rb:49:in `block (2 levels) in <main>'
```
The error indicates that the transition was not successful and I
suspect that it may be due to the use of the `/my` route prefix which
is just a nice to have and not necessary.
The change made in #31854 introduced a regression when editing groups, preventing saving when no auto membership e-mail domains are entered.
This change fixes that and adds a system test.
Before this commit,
`PageObjects::Components::SelectKit#expanded_component` might not expand
the component if `is_collapsed?` returns true which can return true if
the method was called before the select-kit component appears on the
screen. When that happens, we end up not expanding the `select-kit`
because we think it is collapsed when in fact the select-kit component
is not rendered yet. This lead to system test failures with errors like:
```
Capybara::ElementNotFound:
Unable to find css "#add-synonyms.is-expanded"
```
This commit updates `PageObjects::Components::SelectKit#is_collapsed?`
to check that the select-kit component has rendered first before
checking if the component is not expanded.
### Reviewer notes
Instances of test flakiness due to this bug:
1.
https://github.com/discourse/discourse/actions/runs/13905226478/job/38906569777
2.
https://github.com/discourse/discourse/actions/runs/13848357836/job/38751122333
When performing an action in the review queue, this change makes two improvements:
- The buttons on the reviewable item are disabled, so you can't accidentally multi-click.
- A toast is displayed when the action is complete, as a success indication.
Test is flaking in CI with:
```
Failure/Error: expect(theme_translations_settings_editor.get_input_value).to have_content("Bonjour!")
expected to find text "Bonjour!" in "Hello there!"
[Screenshot Image]: /__w/discourse/discourse/tmp/capybara/failures_r_spec_example_groups_admin_customize_themes_when_editing_theme_translations_should_allow_admin_to_edit_and_save_the_theme_translations_from_other_languages_797.png
~~~~~~~ JS LOGS ~~~~~~~
(no logs)
~~~~~ END JS LOGS ~~~~~
./spec/system/admin_customize_themes_spec.rb:158:in `block (3 levels) in <main>'
```
This is a stripped-back version of the Search Banner
component https://meta.discourse.org/t/search-banner/122939,
which will be renamed to Advanced Search Banner,
see https://github.com/discourse/discourse-search-banner/pull/84.
This welcome banner interacts with the header search.
When `search_experience` is set to `search_field`, we only
show the header search after the welcome banner scrolls
out of view, and vice-versa.
Only new sites will get this feature turned on by default,
existing sites have a migration to disable it.
---------
Co-authored-by: Joffrey JAFFEUX <j.jaffeux@gmail.com>
Co-authored-by: Jordan Vidrine <jordan@jordanvidrine.com>
The "Tag synonyms when visiting edit tag page allows an admin to create
a new tag as synonym when tag does not exist" system
test was flaky with the following error:
```
Failure/Error: super
Capybara::ElementNotFound:
Unable to find css "#add-synonyms.is-expanded"
```
Backtrace points to the following location:
```
...
./spec/system/page_objects/components/select_kit.rb:30:in `expanded_component'
./spec/system/page_objects/components/select_kit.rb:88:in `search'
...
```
What I noticed is that
`PageObjects::Components::SelectKit#expanded_component` has already
found the expanded element when it calls `#expand`. Therefore, there is
no need for us to search for it again.
The "Homepage allows users to pick their homepage" system test was flaky
because it was assuming that waiting 5 seconds was enough for the
request to save the user's preferences to complete. This may be true in
development where we have powerful development machines. In production,
Capybara's default wait time is actually 10 seconds so we should respect
that.
The "Admin editing objects type theme setting when editing a theme
setting of objects type allows an admin to edit a theme setting of
objects type via the settings editor"
system test was flaky because of the way we were filling in content into
AceEditor. It is not a normal input and does not seem to function like a
normal input. Using Capybara's `fill_in` method was not reliable so we
will just use AceEditor's API directly to update its input.
When a user typed `:emoji` using the autocomplete on the rich editor,
but then used the "more" option to open the emoji picker and picked an
emoji from there, we were leaving the dangling `:emoji` there and just
added the emoji node after it.
We now check if there's a partial emoji exactly at the caret position,
and replace it too.
When copying images from cooked, we have a `data-base62-sha1` attribute
instead of a `data-orig-src` that we were expecting.
This PR fixes it and adds a system test.
There are a number of minor changes in this commit :
1. Combine the "Themes" and "Components" links in the admin sidebar into
a single tab labelled "Themes and components"
2. The combined tab links to the `/admin/config/customize/themes` page
(titled as "Themes and components")
3. Add a new "Components" tab to the "Themes and components" page.
There's already an existing "Themes" tab
4. Add a "back to" link at the top of individual theme/component page to
navigate back to the respective tab in the "Themes and components" page
5. Remove the themes/components list/sidebar that currently serves for
navigating between themes/components
6. Remove the header in the theme/component page
Changes 4–6 apply only if the admin sidebar is enabled; they have no
effect otherwise.
Internal topic: t/146006.
When you are uploading, it just causes weird problems
when you switch between markdown/rich editors, and it's
not something we really want to support. This commit
disables the Composer::ToggleSwitch while uploading.
Adds support to typing `---`, `___` or `***` to create a horizontal
rule.
Converting when typing `---` is actually written here as an en-dash +
`-`, because the typographer replacements extension turns `--` into an
en-dash first.
`___` and `***` are only triggered after a whitespace, because they
could also mean bold+italic.
These specs are causing far more flakes and trouble than they are worth,
I think it's just the killer
combination of relying on messagebus and background jobs along with the
specs being quite big. Let's just get rid of them...
A new user joining a community via DiscourseHub and logging in via oauth
goes through this process. This would break down for two reasons.
Reason 1: in some cases, especially on Safari mobile, the redirect in
the omniauth callback was happening too early. A new user may not be
signed in yet by that point, which means the redirect to
`/user-api-key/new` triggers a redirect to `/login` which ends up in a
bit of an infinite loop. Not all browsers exhibited this behaviour, but
Safari definitely did.
Reason 2: `/user-api-key/new` is gated via group membership using the
`user_api_key_allowed_groups` site setting. By default that is set to
include `trust_level_0`, however, auto group assignment wasn't taking
place for all user `create` events (only some that go through staged
users).
