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
Follow up to e4401e587e98ac4020c2c4fd965e227146cf33d4.
In the previous change, when serialising a Reviewable post, we were appending the title to the post content whilst checking for watched word matches. This append action was acting upon the object itself, rather than just a copy of the string, causing the UI to display the title appended twice to the content.
Fortunately, since it was only happening in the serialiser, the incorrect data was never stored in the database, it was only happening when viewing the review queue.
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.
Fixes the custom homepage crawler output to include links to the site's
top menu.
This also provides a way to override that content via a plugin. Example
usage in a `plugin.rb` file:
```
register_html_builder("server:custom-homepage-crawler-view") do |c|
"<div>override</div>"
end
```
Currently we allow for 2 theme screenshots to be specified,
with a lightweight spec to allow both a light and dark
version of the screenshot. However, we were not storing this
screenshot name anywhere, so we would not be able to use it
for light/dark switching.
This commit fixes that issue, and also does some general refactoring
around theme screenshots, and adds more tests.
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>
This is a follow up to 6820622467ab3613e824f0cb6219def2a575bc1d.
This commit addresses migrations that uses `remove_index` with the
`if_exists: true` option to drop an existing index before creating an
index using the `concurrently` option.
This commit also reruns two migration which may have caused indexes to
be left in an `invalid` state.
### Reviewers Note
Plugin tests are failing due to
https://github.com/discourse/discourse-translator/pull/251
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.
This improves the new rake task to bulk delete posts based on feedback:
- Honour the `can_permanently_delete` site setting.
- Fix a warning about a scope order no being applied due to batching.
- Fix an issue where double delete would result in duplicate user
histories.
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.
We already support `/apple-app-site-association` at the root. Apple also
accepts `.well-known/apple-app-site-association` as a valid path so this
adds that as well, just in case.
The service worker isn't served via normal asset paths or the CDN.
Instead, the ERB was being compiled by sprockets, fished out of the
`public/` directory by the static_controller, and then the
sprockets-specific stuff like `sourceMappingUrl` was being removed.
Instead, we can put the ERB under `views/static/`, and have it evaluate
at runtime. There are only a couple of super-cheap interpolations, plus
the route is cached in nginx, so there is no performance concern.
This takes us one step closer to removing sprockets.
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.
This commit updates `Migration::SafeMigrate` to protect against unsafe
ways of adding a Postgres index concurrently.
Per postgres documentation:
If a problem arises while scanning the table, such as a deadlock or a
uniqueness violation in a unique index,
the CREATE INDEX command will fail but leave behind an "invalid" index.
This index will be ignored for querying
purposes because it might be incomplete; however it will still consume
update overhead. The recommended recovery
method in such cases is to drop the index and try again to perform
CREATE INDEX CONCURRENTLY .
Therefore, the simplest way for us to ensure that migrations that create
indexes concurrently are idempotent is to follow postgres'
recommendation of dropping the index first before trying to create the
index concurrently.
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).
Followup https://github.com/discourse/discourse/pull/31505/
When sending notification emails for system user responses for PMs,
we removed the part of the CTA where it says "to respond to xyz" in a
previous commit.
This commit takes it slightly further -- we now only show a "Visit
Topic"
or "Visit Message" button if the PM notification is from a system user,
it's a bit cleaner.
This commit also adds more in-depth tests, and refactors the message
builder a little.
Continues the work done on
https://github.com/discourse/discourse/pull/30815.
Adds a `onebox` and an `onebox_inline` node spec, their serializers, and
a plugin that automatically converts links to oneboxes.
This might not reduce the failures to zero but some screenshots of the
failures clearly show we were still on the success message page.
Same fix than: https://github.com/discourse/discourse/pull/31750
I suspect that sometimes the button was still not disabled yet when we
where checking for it. Checking for saved confirmation should be more
resilient.
- Remove `wait: 0` and base flow on site setting instead
- Ensure full-page-login has actually opened before running `go_back` -
otherwise we'll end up going back to `about:blank`
This list of all reports is needed in the admin search
controller as well, so this commit refactors it into
a service, adds specs, and also updates the admin
search code to use this new service & avoid a second
AJAX call to the server.
Currently, when converting raw hotlinked image urls to image upload
markdown, the `alt` attribute is left blank.
This change sets the `alt` to the original filename (which means it will
also appear in the lightbox).
This adds more context and accessibility to the image and is consistent
with how regular uploaded images default to use the filename.
For example:
`http://foo.bar/screenshot.jpg` becomes
``
