We want to merge the theme component that allows admins to display extra groups on the about page. The settings for this are now under About your site.
All the code is lift-and-shift, with some minor adjustments, e.g. theme components can't use the group_list setting type, but it has been converted to that here.
Also the system tests for the admin controls are new.
This whole thing is gated behind a hidden site setting to avoid double rendering while we deprecate the theme component.
We were not correctly relying on capybara matchers leading to test
flakiness
```
ailure/Error: super
Capybara::Playwright::Node::StaleReferenceError:
Element is not attached to the DOM
/var/www/discourse/vendor/bundle/ruby/3.3.0/gems/capybara-playwright-driver-0.5.6/lib/capybara/playwright/node.rb:91:in `rescue in assert_element_not_stale'
/var/www/discourse/vendor/bundle/ruby/3.3.0/gems/capybara-playwright-driver-0.5.6/lib/capybara/playwright/node.rb:81:in `assert_element_not_stale'
/var/www/discourse/vendor/bundle/ruby/3.3.0/gems/capybara-playwright-driver-0.5.6/lib/capybara/playwright/node.rb:124:in `visible_text'
/var/www/discourse/vendor/bundle/ruby/3.3.0/gems/capybara-3.40.0/lib/capybara/node/element.rb:60:in `block in text'
/var/www/discourse/vendor/bundle/ruby/3.3.0/gems/capybara-3.40.0/lib/capybara/node/base.rb:84:in `synchronize'
./spec/rails_helper.rb:421:in `synchronize'
/var/www/discourse/vendor/bundle/ruby/3.3.0/gems/capybara-3.40.0/lib/capybara/node/element.rb:60:in `text'
/var/www/discourse/vendor/bundle/ruby/3.3.0/gems/capybara-3.40.0/lib/capybara/result.rb:44:in `each'
/var/www/discourse/vendor/bundle/ruby/3.3.0/gems/capybara-3.40.0/lib/capybara/result.rb:44:in `each'
./spec/system/page_objects/modals/sidebar_edit_categories.rb:45:in `map'
./spec/system/page_objects/modals/sidebar_edit_categories.rb:45:in `has_categories?'
./spec/system/editing_sidebar_categories_navigation_spec.rb:161:in `block (2 levels) in <main>'
```
While re-reviewing what I did in
https://github.com/discourse/discourse/pull/32706, I noticed that the
import for customize_show_schema in `admin_base.scss` file was missing.
Probably removed by mistake in the rebase.
**Description**
As part of a customer request, we have added the option to search groups
by ID when doing API calls. However, in doing so we have decided to
correct the confusion around the group's routes. Previously the route
would look like `g/:id` while taking the `name` of the group as the
param. For example, when getting a group the route would be like this:
```
GET /g/admins
```
This would make the code in the controller seem as if it was handling
the group IDs instead of names. With these changes, this should be
addressed.
This allows, in particular, for larger animated image uploads. Due to
optimizing (clientside and serverside), the actual size of the uploaded
(non-animated) image often ends up being smaller than the o riginal
size. I.e. it may already be optimized before hitting this limit.
In development mode, when 'DISCOURSE_DEV_ALLOW_ANON_TO_IMPERSONATE' is
enabled, and going to /session/:username/become, we will now show an
error message when trying to impersonate an inactive user.
This was not obvious why trying to impersonate a user wasn't working
locally because I would hit the URL and be redirected back to the index
without any error and without being logged in.
Ignore any files/directories in '.devenv' like we do for those in
'node_modules' to 1) speed-up watching files and 2) prevent files from
being watched multiples times (due to symlinks).
No specs since this is only a warning that is being shown in development
when using devenv.
---
Here's an example of the 100s of warning you get when using devenv
```plain
** ERROR: directory is already being watched! **
Directory: /Users/zogstrip/Poetry/discourse/plugins/discourse-login-client/.devenv/profile
is already being watched through: /nix/store/jp4qhxc2f98nyq90kia200mrygl2ndcy-devenv-profile
MORE INFO: https://github.com/guard/listen/blob/master/README.md
```
We were using a vanilla `button` tag here, but can use `DButton` now,
this helps us be more consistent with classes... we had a minor issue
where class differences changed the active state between buttons here...
Before (hamburger and avatar forced active):
After (hamburger and avatar forced active):
I also noticed we have a useless title attribute on the image here, the
title should be on the button and the image doesn't need one.
I've also cleaned up the titles:
* hamburger menu was previously just `menu` but we refer to this as
`Navigation menu` generally, and that provides a lot more context
* the notification menu was "Username's account" but more accurately
it's "Notifications and account" (and I don't think we need the username
present)
These reject/approve buttons were getting `btn-default` classes rather
than `btn-danger` or `btn-success`, which meant the styles for the
colors had to be applied separately to `.approve-post` and
`.reject-post`. This also meant the styles needed an additional override
in themes.
Moving to our common button classes helps centralize the styles and
avoids a manual override being needed in themes.
Button appearance is the same before/after
This strategy allows us to synchronously init the wasm, without needing
a patch on the npm package
---------
Co-authored-by: Jarek Radosz <jarek@cvx.dev>
This allows themes to add exceptions for menus that may be triggered
from the hamburger menu... like this
```js
api.registerValueTransformer(
"hamburger-dropdown-click-outside-exceptions",
({ value }) => {
return [...value, ".topic-drafts-menu-content"];
}
);
```
Running this spec locally I was getting an error:
```
1) translate accelerator plugins loads plural rules from plugins
Failure/Error: DiscoursePluginRegistry.unregister_locale("foo")
NoMethodError:
undefined method `unregister_locale' for class DiscoursePluginRegistry
# ./spec/lib/freedom_patches/translate_accelerator_spec.rb:113:in `block (3 levels) in <main>'
Finished in 0.17998 seconds (files took 1.88 seconds to load)
1 example, 1 failure
Failed examples:
rspec ./spec/lib/freedom_patches/translate_accelerator_spec.rb:117 # translate accelerator plugins loads plural rules from plugins
```
On top of this, this spec was flakey, Im not sure this is going to fix
flakyness, but this seems like a good first step.
The way playwright renders headed browser is different than selenium,
and depending on the size of your screen you could not be able to see
part of the app. To avoid this issue you can now use
`PLAYWRIGHT_NO_VIEWPORT=1` which will default to a smaller viewport.
Note that to ensure we don't fail randomly on CI, the viewport on CI
will always be 1400x1400, so if you develop with
`PLAYWRIGHT_NO_VIEWPORT=1` you might experience failures on CI.
Doing a full page load in system test is expensive and takes about 1
second on average. Since this shared example is being run across 70
official
plugins, the additional time to execute these full page loads adds up.
Therefore, we are trading off some readability here for CI runtime.
Before:
```
Randomized with seed 7202
..................
Finished in 21.26 seconds (files took 1.72 seconds to load)
18 examples, 0 failures
```
After:
```
Randomized with seed 7202
........
Finished in 14.28 seconds (files took 1.74 seconds to load)
8 examples, 0 failures
```
Switches to using `URL.createObjectURL` for better support of very large
images (30Mb+). Using blob URLs is more memory-efficient and on testing,
handles a 90Mb file just fine (previously, it would fail and skip
optimization).
These changes have been made for playwright as it was hard to test a
long press even on playwright given `click` won't trigger `touchstart`
even with `isMobile:true` and `hasTouch:true`. You have to use `tap`,
but you don't have the `delay` option on tap, so you can't make it a
long tap.
Sadly this code is apparently not working correctly on Android 15. This
commit will revert the modifier to what it was before and is relying on
native JS to trigger the fake long press in specs, which seems to work
nicely.
This commit also attempts to centralize the actions on messages in page
objects to avoid code duplication.
- When using the Glimmer Post Stream, ensure the status wrapper is only
rendered for mentions of users with a status set.
This prevents an empty wrapper from adding a small blank space at the
end of the mention.
- Ensures the post's user field in initialized with the
`avatar_template` in the PostStream to prevent missing avatar on small
actions
- Fix an issue where the path would be incorrectly linked on small
actions
- Fix an issue where the relative timestamp would be incorrectly
displayed in wiki posts
Previously when changing back to the same email (ie. change to new
email, then change back again) we can easily end up showing the
incorrect old email if changing back to the same email more than once.
Then passing the incorrect old email causes an error as it can't be found.
This happens because we only searched for a combination of `user_id` and
`new_email` which can load an older change from the database. By using
the current email address when finding / initializing the
`EmailChangeRequest` we can prevent this issue as we will be creating a
new entry rather than loading an outdated one.
