* UX: Apply admin table classes for consistent mobile styling on custom flags
* UX: Apply admin table classes for consistent mobile styling on custom flags
* UX: Apply admin table classes for consistent mobile styling on backups
* UX: Apply admin table classes for consistent mobile styling on plugins list
* DEV: tweaks on admin table
* UX: Apply admin table classes for consistent mobile styling on chat plugin
* apply prettier
* apply lint
* DEV: removed commented out code
* DEV: removed unnecessary div element
* scroll to the element
* remove the workaround
* revert
* add an extra assertion
* add enabled check
* improve switching
* rm
---------
Co-authored-by: Jarek Radosz <jradosz@gmail.com>
When rendering the initial search options, we re-use the `AssistantItem` component.
`AssistantItem` requires that you pass in the required params to define what _type_ of component it will be - category, tag, tag intersection, user, etc. This flexibility is nice, as we can just loop through all `@results` and pass in params, without having to predefine what _type_ of result it is.
It is is not very good when it comes to seperating the html strucutre of each unique _type_. This is an example of the initial search results:
<img width="408" alt="Screenshot 2024-10-23 at 9 04 18 AM" src="https://github.com/user-attachments/assets/46795697-6246-4b60-be18-fea200a57baa">
You can see that both categories **and** tags are being rendered. The HTML strcuture looks like so:
```html
<ul class="search-menu-assistant">
<li class="search-menu-assistant-item">
<a class="search-link" href="#"> CATEGORY </a>
</li>
<li class="search-menu-assistant-item">
<a class="search-link" href="#"> CATEGORY </a>
</li>
<li class="search-menu-assistant-item">
<a class="search-link" href="#"> TAG </a>
</li>
<li class="search-menu-assistant-item">
<a class="search-link" href="#"> TAG </a>
</li>
</ul>
```
There is no way to differentiate between the types, even though some are categories and others tags.
This PR adds a _typeClass_ to each component, that will be a additional class included at the top level of the component HTML structure.
```html
<ul class="search-menu-assistant">
<li class="category search-menu-assistant-item">
<a class="search-link" href="#"> CATEGORY </a>
</li>
<li class="category search-menu-assistant-item">
<a class="search-link" href="#"> CATEGORY </a>
</li>
<li class="tag search-menu-assistant-item">
<a class="search-link" href="#"> TAG </a>
</li>
<li class="tag search-menu-assistant-item">
<a class="search-link" href="#"> TAG </a>
</li>
</ul>
```
_See `.category` and `.tag` attached to each `search-menu-assistant-item`._
This will help us identify which _type_ it is, and allow devs to target and customize each element by _type_.
A followup to f05b984208
* modifiers to keep track of components' lifecycles, instead of did-insert/did-update/willDestroy
* proper glimmer-friendly tracking in related models
* caching
* `@outletArgs`
* gjs
We were using a modifier purely for its lifecycle hooks - not to modify an element. This commit switches to using a helper, which provides a similar lifecycle, but without needing to be attached to an element.
Bug introduced in this PR https://github.com/discourse/discourse/pull/29244
When the experiment toggle button was introduced, new features did not look right when the toggle button was not available.
In addition, the plugin name can be an empty string. In that case, information about new features should be displayed.
…or a tip with the highest priority.
This regressed in 597ef11195 where we got rid of `next()` calls, so we'd render the first tip we encounter.
The commit also adds a test and updates existing ones.
Moves the user-tip from the topic-timeline notifications button to the one at the bottom of the topic page.
Three reasons:
1. new users are more likely to use the button that has the full text (and description) rather than the icon-only one
2. we hide the timeline button when scrolled all the way to the bottom of the page, and then the tip doesn't seems to be attached to anything
3. we might be removing the timeline button altogether in the near future
The visitor stats on the /about page were previously showing as `NaN` immediately after enabling the `display_eu_visitor_stats` site setting because the stats for the /about page are cached and updated once every 30 minutes in a sidekiq job. The `NaN` would go away upon the next run of the relevant sidekiq job, but it's not good UX to display a cryptic `NaN` until the job runs. So, this commit ensures that the visitor stats is not displayed at all until the visitor stats is calculated and available.
Internal topic: t/128480.
Currently, when calling a service with its block form, a `#result`
method is automatically created on the caller object. Even if it never
clashed so far, this could happen.
This patch removes that method, and instead use a more classical way of
doing things: the result object is now provided as an argument to the
main block. This means if we need to access the result object in an
outcome block, it will be done like this from now on:
```ruby
MyService.call(params) do |result|
on_success do
# do something with the result object
do_something(result)
end
end
```
In the same vein, this patch introduces the ability to match keys from
the result object in the outcome blocks, like we already do with step
definitions in a service. For example:
```ruby
on_success do |model:, contract:|
do_something(model, contract)
end
```
Instead of
```ruby
on_success do
do_something(result.model, result.contract)
end
```
This PR is a follow-up to ea1473e532. When we initially added the experimental feature for automatically adding `[grid]` to images, we add the [grid] surrounding images after all the uploads have been completed.
This can lead to confusion when `[grid]` is delayed to be added in the composer, as users may try to add grid manually leading to breakage. This also leads to issues with Discourse AI's automatic image caption feature.
**In this PR**: we simply move the logic to be added when the images are uploaded and processing. This way, `[grid]` surrounding images is added immediately. We also apply a fix for an edge-case to prevent images from being wrapped in `[grid]` when they are already inside `[grid]` tags.
As part of #29272 we made a unique index work on PG13 by introducing a dummy string to represent "NULL".
We missed one spot, leading to a potential for duplicate admin notices for problems without a target.
This fixes that.
This commit brings back some reports hidden or changed
by the commit in 14b436923c if
the site setting `use_legacy_pageviews` is false.
* Unhide the old “Consolidated Pageviews” report and rename it
to “Legacy Consolidated Pageviews”
* Add a legacy_page_view_total_reqs report called “Legacy Pageviews”,
which calculates pageviews in the same way the old page_view_total_reqs
report did.
This will allow admins to better compare old and new pageview
stats which are based on browser detection if they have switched
over to _not_ use legacy pageviews.
Toggle the button to enable the experimental site setting from "What's new" announcement.
The toggle button is displayed when:
- site setting exists and is boolean;
- potentially required plugin is enabled.
This PR adds the feature where three or more image uploads in the composer will result in the images being surrounded by `[grid]` tags. This helps take advantage of the grid feature (https://github.com/discourse/discourse/pull/21513) and display images in a more appealing way immediately after upload.
* FIX: participating users statistics...
... was (mis-)counting
- bots
- anonymous users
- suspended users
There's now a "valid_users" function that holds the AR query for valid users and which is used in all "users", "active_users", and "participating_users" queries.
Internal ref - t/138435
These tweaks will help adoption of the non-mixin-based uppy patterns.
- Add `type:` to default arguments list
- Update pick-files-button to support explicit element registration
- Make `cancelSingleUpload` a public API, and add `cancelAllUploads`
- Remove `isDestroyed` logic - it doesn't do anything outside a component
- Add `@bind` to `setup()`
- Allow `additionalParams` to be a function
- Fix `autoStart` mixin shim
This commit simplifies the initial state of the invite modal when it's opened to make it one click away from creating an invite link. The existing options/fields within the invite modal are still available, but are now hidden behind an advanced mode which can be enabled.
On the technical front, this PR also switches the invite modal to use our FormKit library.
Internal topic: t/134023.
When a user is missing required fields, they are required to fill those up before continuing to interact with the forum. This applies to admins as well.
We keep a whitelist of paths that can still be visited in this mode: FAQ, About, 2FA setup, and any admin route for admins.
We concluded that admins should still be able to enable safe mode even with missing required fields. Since plugins etc. can potentially mess with the ability to fill those up.
When staff only mode is enabled - Discourse.enable_readonly_mode(Discourse::STAFF_WRITES_ONLY_MODE_KEY)
Staff members couldn't reset their password via the "forgot password" link.
This fixes it.
Internal ref. t/133990
We're seeing errors in logs due to some sites setting the reserved_usernames setting to nil. This is causing multiple use cases upstream of User#reserved_username? to error out.
This commit changes from using the raw #reserved_usernames to using the #reserved_usernames_map helper which exists on list-type site settings. It returns an empty array if the raw value is nil or empty string.
When adding or updating a custom user field to apply to all users (retroactively) we want to alert the admin that this will force all existing users to fill up the field before they are able to access the forum again.
However, we currently show this prompt when making changes only to other attributes on the custom field, i.e. the requirement hasn't changed.
This commit fixes that.
In #29169 we added a NULLS NOT DISTINCT option to the unique index on problem_check_trackers. This is to enforce uniqueness even when the target is NULL. (Postgres considers all NULLs to be distinct by default.)
However, this only works in PG15. In PG13 it does nothing.
This commit adds a default dummy string value __NULL__ to target. Since it's a string, PG13 will be able to correctly identify duplicate records.
We're expecting the period param to be something that neatly coerces into a symbol. If we receive something like a nested parameter, this will blow up.
This commit raises an InvalidParameters exception in the case of a non-stringy period parameter.
This component will soon be updated to remove the mixin entirely (and add a regression test for it). But for now, this is a quick fix to get it working again.
Since we recently blocked accidental serialization of AR models, we are getting a 500 error in some cases with thumbnails. We can fix this by serializing the thumbnail, previously we just returned a raw OptimizedImage object.
Thumbnails are now attached to the serializer in core, therefore we no longer need to use add_to_serializer within the chat plugin to use thumbnails within chat message uploads.
We're expecting the ID param to be something that neatly coerces into an ID. If we receive something like a nested parameter, this will blow up. (We already handle the case of arrays.)
This commit raises an InvalidParameters exception in the case of a nested ID.
We're expecting the page param to be something that neatly coerces into an integer. If we receive something like a nested parameter, this will blow up. (I'm sure there are other examples as well.)
This commit falls back to a page value of 1 if the coercion fails.
This commit is fixing the path which sets a default value to trigger. We were doing `if (!this.model.trigger)` but `this.model.trigger` can have `0` as value, which would trigger this codepath and this codepath was setting the first value of `badgeTriggers` as a default value for trigger.
The subcategories page was not paginated and it was using the
subcategory style from the category settings. The same page style should
be used for categories and subcategories page.
Theme modifiers can now be defined as theme settings, this allows for
site operators to override behavior of theme modifiers.
New syntax is:
```
{
...
"modifiers": {
"modifier_name": {
"type": "setting",
"value": "setting_name"
}
}
}
```
This also introduces a new theme modifier for serialize_post_user_badges. Name of badge must match the name of the badge in the badges table. The client-side is updated to load this new data from the post-stream serializer.
Co-authored-by: David Taylor <david@taylorhq.com>
When converting the user custom fields admin form in #29070, I accidentally removed the plugin outlet after-admin-user-fields. This is used by the discourse-authentication-validations plugin, which is now broken on main core.
This commit adds back the plugin outlet in core.
Add plugin outlets for each assistant result type - User, Tag and Categories, Groups, etc. This gives us the ability to add content for each _type_ of search suggestion:
<img width="216" alt="Screenshot 2024-10-16 at 10 01 09 AM" src="https://github.com/user-attachments/assets/fbbc71fe-a8fe-499b-8377-480dd0ed5f75">
I would have preferred to add a single plugin outlet at the top of the template and pass `this.suggestionType` and `@results`, but that would then require that we carry over a ton of core logic to plugins to calculate which _type_ is being rendered, and it would get unnecessarily messy quick. So instead I opted to create a plugin outlet for each _type_.
#29209 introduced a bug where columns to the directory added via add_directory_column are not being translated properly.
This fixes the issue and adds an integration test.
This commit replaces all uppy-related mixins with standalone classes. The main entrypoint is now lib/uppy/uppy-upload.js, which has a list of its config options listed at the top of the file. Functionality & logic is completely unchanged.
The uppy-upload mixin is replaced with a backwards-compatibility shim, which will allow us to migrate to the new pattern incrementally.
Constants should always be only assigned once. The logical OR assignment
of a constant is a relic of the past before we used zeitwerk for
autoloading and had bugs where a file could be loaded twice resulting in
constant redefinition warnings.
* UX: More additions
* UX: more
* DEV: Add admin/config/themes route
* UX: Use admin config card
* syntax merge fixes
* cleanup
* cleanup
* checkbox
* more
* error
* save on click
* more
* fix setter
* DEV: Implement vanilla checkbox
* cleanup
* UX: save themes as default
* DEV: Add component list to card
* DEV: Add placeholder for no screenshots
* DEV: Fix default theme reactivity
Also add content/optionalAction yields to config area
card and put the theme user selectable checkbox there,
along with adding styles.
* DEV: Change to generic "look and feel" config area
* DEV: Auto redirect to themes on base look and feel route
* UX: Remove computed from sorted themes
* linting
* UX: Turn update icon into button that routes to settings
* DEV: remove unused function
* UX: center icons with title
* DEV: Lint
* UX: Hook up theme preview button
* DEV: Minor fixes
---------
Co-authored-by: Martin Brennan <martin@discourse.org>
Co-authored-by: Joffrey JAFFEUX <j.jaffeux@gmail.com>
This adds dedicated routes for /login and /signup, replacing the use of modals. Currently, this is behind the experimental_full_page_login feature flag. It also includes some small consistency fixes related to formatting, spacing, icons, and the loading of certain elements
The `id` column of `notifications` table and `notification_id` columns
of the other tables have been migrated to bigint in previous commits
(for example, 799a45a).
In order to run the migrations with zero downtime, the data had to be
copied to new columns and swapped, but the old columns have been kept
to allow for rollback. They are no longer needed now.
Prior to this fix the format class was only applied to the child control, but in case of full we also need to ensure the parent wrapping field is also taking 100% width otherwise we are at risk of having a field of the width of its content.
theme-setting and theme-i18n are not needed in `.gjs` files. This commit adds more helpful error messages to direct developers to the modern alternatives.
Currently the topic timeline tip appears before the suggested topics one only due to a small implementation detail in the latter.
This ensures that tips appear in the expected order when more than one of these components is rendered at the same time.
If a plugin's JS fails to load for some reason, most commonly
ad blockers, the entire admin interface would break. This is because
we are adding links to the admin routes for plugins that define
them in the sidebar.
We have a fix for this already in the plugin list which shows a warning
to the admin. This fix just prevents the broken link from rendering
in the sidebar if the route is not valid.
Since the AdminPageHeader is H1, it is more semantically
correct to progress to H2 after it rather than skipping
a level to H3
Also amend style of H2 to make it the same size as H3
* Add migrations to ensure password hash is synced across users & user_passwords
* Persist password-related data in user_passwords instead of users
* Merge User#expire_old_email_tokens with User#expire_tokens_if_password_changed
* Add post deploy migration to mark password-related columns from users table as read-only
* Refactored UserPassword#confirm_password? and changes required to accommodate hashing the password after validations
In our production environment, we have been seeing Sidekiq processes
getting stuck randomly when a USR1 signal is sent to the Unicorn master
process. We have not been able to identify the root cause of why the
Sidekiq process gets stuck. We however noticed that when the Unicorn
master process receives a USR1 signal, it will reopen the logs for the
Unicorn master process first before sending a USR1 signal for the
Unicorn worker processes to reopen the logs. We figured that we should
do the same for the Sidekiq process as well when a USR1 signal.
In this commit, we introduce an arbitrary delay of 1 second before we
the Sidekiq process reopens its log files so as to allow enough time for the Unicorn
master to finish reopening it logs first.
We also do not send reopen logs for the Sidekiq process if the `DISCOURSE_LOG_SIDEKIQ`
env is not present because there is no need to reopen any logs.
`Date.today` relies on the system's timezone instead of
`Rails.configuration.time_zone`. This can cause tests to fail when we
assert against the date of a record. Just use `Time.zone.today` instead
so that we always follow `Rails.configuration.time_zone`.
Mobile app can capture event and launch a separate login flow. Should
help resolve issues with passkeys (which aren't available in webviews)
and non-local login methods.
While using `OpenStruct` is nice, it’s generally not a very good idea as
it usually leads to performance problems.
The `OpenStruct` source code even says basically to avoid it.
Since the context object is crucial in our services, this patch replaces
`OpenStruct` with a custom implementation instead.
This commit attempts to improve the mobile experience for
admin page header and subheader by automatically collapsing
all action buttons in these components into a DMenu when viewing
mobile.
This is done by using different "list" wrapper components and a
DMenu trigger and a DropdownMenu on mobile only, and uses has-block
to determine whether to render the DMenu trigger at all.
This also removes the `PluginOutlet` in `AdminPluginConfigPage`, it
was too inflexible for this `DropdownMenu` case, and since the `:actions`
were always rendering we couldn't rely on `has-block`. A new plugin API,
`registerPluginHeaderActionComponent`, has been introduced instead to
replace it.
Because of unreliability, the spec was temporarily disabled. However, it is ensuring that the custom flags system is working correctly. Therefore it would be great to enable it again.
I made a few fixes to try to mitigate this situation:
- Reduced amount of Redis calls;
- When deleting, ensure that the modal is closed before checking the result;
- Moved duplicated name tests to a separate block;
- Increased wait time to 3 times the default because I noticed that sometimes it gets stuck for a moment. Most of the time it is fast, but sometimes when I run tests in a loop 50 times I see slowness.
Dismissing admin notices is an admin-only action. This is enforced on the back-end both by a routing constraint and a policy in the relevant service.
However, we still unconditionally display the "Dismiss" button to anyone with access to the admin dashboard. When clicked, it results in a 404 modal (due to the routing constraint.)
With this change we only render the dismiss button for admins.
If you have the admin dashboard open, and one of the admin notices listed has already been dismissed (e.g. in another tab, or by another admin) we would show an ugly "FAILED" modal.
This change makes the admin dismiss endpoint idempotent. If the admin notice is already destroyed, then respond with 200. This will also correctly remove it from the list in the front-end.
When a post has some replies, and the user click on the button to show them, we would load ALL the replies. This could lead to DoS if there were a very large number of replies.
This adds support for pagination to these post replies.
Internal ref t/129773
FIX: Duplicated parent posts
DEV: Query refactor
Recently we updated the icon library from Font Awesome `5` to `6.6.0`. Since we were running Font Awesome 5 for a long time while 6 had already been released, we often specified in the codebase with the text _"FontAwesome 5"_. However, now that we are in the latest version, there is no need for our API's/comments to keep specifying for version 5. This PR updates all instances of FontAwesome 5 or FA5 and removes the version number to be the more generic: "FontAwesome"
This PR limits this feature:
On all devices:
- Browsers with OffScreenCanvas support
- Browsers with createImageBitmap
On Apple Safari
- At least version 18
It also adds a routine that terminates the worker after 5 uses on all devices to handle any WASM memory leaks. All this together fixes crashes that could occur on iPhones.
It still leaves the feature disabled by default on iOS, which will be revisited after testing this changes.
Currently in services, the `contract` step is only used to define where
the contract will be called in the execution flow. Then, a `Contract`
class has to be defined with validations in it.
This patch allows the `contract` step to take a block containing
validations, attributes, etc. directly. No need to then open a
`Contract` class later in the service.
It also has a nice side effect, as it’s now easy to define multiples
contracts inside the same service. Before, we had the `class_name:`
option, but it wasn’t really useful as you had to redefine a complete
new contract class.
Now, when using a name for the contract other than `default`, a new
contract will be created automatically using the provided name.
Example:
```ruby
contract(:user) do
attribute :user_id, :integer
validates :user_id, presence: true
end
```
This will create a `UserContract` class and use it, also putting the
resulting contract in `context[:user_contract]`.
We are going to start making section landing pages
for admin for each sidebar section. This lays the framework
with routes and simple components that can be further
refined by a designer, but I have taken the base CSS from
AI which Kris made.
The initial section landing items will be used in AI to replace
the placeholders added in this commit b8b3c61451
* Fixes big gap at the left of the plugins list.
* Fixes plugin settings list padding, the yellow overridden
dot was cut off on mobile.
* Also increased settings filter input size and settings
sidebar button margin.
After #28603, the options "agree and suspend" and "agree and silence" in the review queue weren't working. This was happening because the optionalService, when used as a decorator, needs a name argument to work properly. We were also lacking tests for this.
In 61c1d35f17 I added a
PluginOutlet to AdminPluginConfigPage. This was intended to be
used as a way to render actions buttons inside the header of
a plugin that has a custom admin UI page. This worked, but
since the outlet was generically named, as soon as one plugin
used it the button would show on all plugins.
This fixes the immediate issue by naming the outlet based
on the plugin, then having each plugin specify their own
outlet to render into. There may be a nicer way to do this,
but for now this stops the bleeding.
This commit introduces a feature that allows an admin to delete a user's
associated account. After deletion, a log will be recorded in staff
actions.
ref=t/136675
Permanently deleting posts that no longer have a user associated was not
working as expected because of UserAction.log which expected user_id to
be present.
This will help to enforce a consistent pattern for creating service
actions.
This patch also namespaces actions and policies, making everything
related to a service available directly in
`app/services/<concept-name>`, making things more consistent at that
level too.
In our test suite, we sometimes see ChunkLoadErrors. This plugin should cause those failed requests to be retried seamlessly. It'll also help clients with flaky internet connections in production.
When running checks, we look to the existing problem check trackers and try to grab their ProblemCheck classes.
In some cases this is no longer in the problem check repository, e.g. when the check was part of a plugin that has been uninstalled.
In the case where the check was scheduled, this would lead to an error in one of the jobs
the radical change in the implementation doesn't stem from the glimmer migration, but rather the fact that previously the component was single-use – changing any of its args didn't (and couldn't) be reflected because hljs was replacing the nodes so all the ember bookkeeping was gone.
Co-authored-by: David Taylor <david@taylorhq.com>
There is a risk of overriding and then deleting a prop of the context in case of a naming clash between localName and that prop, e.g.
```js
class Test {
item = "foo";
items = [1, 2];
}
const template = `
{{#each items as |item|}}
{{item}}
{{/each}}
`;
const compiledTemplate = compile(template);
const object = new Test();
// object.item === "foo"
const output = compiledTemplate(object, RUNTIME_OPTIONS);
// object.item === undefined
```
…but I think we can accept this risk and just be careful.`#each` isn't widely used in hbr anyway (as proven by the other long-standing and recently fixed bug) and hbr is on its way out anyway.
his is a new feature that lets admins dismiss notices from the dashboard. This helps with self-service in cases where a notice is "stuck", while we work on provisions to prevent "sticking" in the first place.
In some very rare cases, the header element doesn't yet have the bg
when this code is run. This PR adds a simple retry mechanism.
No tests because this relies on specific load timing from the browser.
In TopicController, in addition to ensure_can_move_posts!, we also
checked if the topic is private message in this line:
```ruby
raise Discourse::InvalidAccess if params[:archetype] == "private_message" && !guardian.is_staff?
```
However, this was not present in `guardian.can_move_posts?`. As a result,
the frontend topic view got an incorrect serialized result, thinking
that TL4 could move the private message post. In fact, once they tried
to move it, they got the `InvalidAccess` error message.
This commit fixes that TL4 will no longer sees the "move to" option in
the "select post" panel for a private message.
Anonymous users are "shadow" users created when an existing real user desires to post anonymously. This feature is off by default, but it can be enabled via the `allow_anonymous_posting` site setting. Those shadow users shouldn't be included in the users directory (`/u`).
In 14cf8eacf1, we added the
`user_search_similar_results` site setting which when enabled will use
trigram matching for similarity search in `UserSearch`. However, we
noted that adding the `index_users_on_username_lower_trgm` index is
causing the PG planner to not use the `index_users_on_username_lower`
index when the `=` operator is used against the `username_lower` column.
Based on the PG mailing list discussion where support for the `=`
operator in gist_trgm_ops was being considered, it stated that "I also have checked that btree_gist is preferred over pg_trgm gist
index for equality search." This is however quite different from reality
on our own PG clusters where the btree index is not preferred leading to
significantly slower queries when the `=` operator is used.
Since the pg_trgm gist index is only used for queries when the `user_search_similar_results` site setting
is enabled, we decided to drop the feature instead as it is hidden and
disabled by default. As such, we can consider it experiemental and drop
it without deprecation.
PG mailing list discussiong: https://www.postgresql.org/message-id/CAPpHfducQ0U8noyb2L3VChsyBMsc5V2Ej2whmEuxmAgHa2jVXg%40mail.gmail.com
Previously shortcuts added to toolbar buttons will automatically use the same action assigned to the button when clicked. This PR adds an additional optional key that can be passed when creating a new toolbar button: shortcutAction which allows for passing a custom action for the keyboard shortcut. This way a button can have a specific action when a keyboard shortcut is pressed that's different from when the button is clicked.
Followup to e25578d702
Using execCommand to replace the entire contents of the textarea is very slow for larger posts (it seems the browser does a reflow after every 'virtual keypress'.
This commit updates the `replaceText` function to be more surgical with its `insertAt` calls. Now it only selects & replaces the characters which are actually being replaced.
- fetch models inside services
- validate `user_id` in contracts
- use policy objects
- extract more logic to actions
- write specs for services and action
Previously for reviewables that could be claimed, we positioned
the "you can claim / you must claim" message and button underneath
the "Is there something wrong with this post?" message but _before_
the reviewable action buttons like Yes/No/Ignore. This was a confusing
flow.
This commit fixes the issue, and also makes it so if claiming is
required and the reviewable has not been claimed, we don't show
the "Is there something wrong with this post?" which was showing
with no buttons.
Followup 5a8e7c5f29
The admin report results need to be side by side
with the filter for the report, which sits on the
right. The previous commit made it stacked.
Followup 0323b366f3
This was happening because another spec was adding a
report using the plugin API, but there was nothing
resetting that, so later in the reports controller
when we did Report.singleton_methods, we ended up
with another report with no translation, causing another
error.
Currently, when the custom flag has the same name as the system flag (which is disabled) then it is not displayed. To fix the problem, `custom_` prefix as `name_key` is used to distinguish between the system and the custom flag.
I considered writing a migration to fix existing custom flags name key. However, at the end of migration I would need to run rails code to reset cache `Flag.reset_flag_settings!`. I decided to skip that step as it is a very edge case. If someone has the same flag name as the system flag, then all they have to do is edit the flag and click save.
In addition, I made 2 small fixes:
- edit flag title was missing translation;
- flag form UI was not showing that description is the required field.
This commit adds a new `about_page_hidden_groups` setting to exclude members of specific groups from the admin and moderator lists on the /about page.
Internal topic: t/137717.
