When performing bulk dismissal in Unread and New views, the dismiss button stays at the top of the UI. Because of this we want to provide the dismiss action also in the "sticky" menu that's always in view, even when scrolling a long list of topics.
This reverts commit 5a00a041f1.
Implementation is currently not correct. Multiple uploads can share the
same etag but have different paths in the S3 bucket.
Follow-up to a5497b74be
In the linked commit, as part of simplifying the invite modal, we removed the option to skip sending an email when creating an invite restricted to a specific address. This has caused confusion about whether an email will be sent by Discourse or not, so we're adding back the option to create a restricted invite without emailing.
Internal topic: t/134023/48.
Followup 0568d36133
S3 itself and other S3-compatible providers do not
allow using an S3 custom endpoint and dualstack at
the same time, so this commit fixes that by not using
dualstack when the endpoint is present.
A "bad upload" in this context is a upload with a mismatched URL. This can happen when changing the S3 bucket used for uploads and the upload records in the database have not been remapped correctly.
"Resume editing" would do nothing when going through the `/new-message` flow.
This seems to be broken since [this commit](b0f6d074be). which moved `this._setModel` calls around – the same we're doing now, but to different places: the first one needs to happen after the `draft.data` has been set , while the second needs to happen before the `this.open` call.
When we added direct S3 uploads to Discourse, which use
presigned URLs, we never took into account the dualstack
endpoints for IPv6 on S3.
This commit fixes the issue by using the dualstack endpoints
for presigned URLs and requests, which are used in the
get-presigned-put and batch-presign-urls endpoints used when
directly uploading to S3.
It also makes regular S3 requests for `put` and so on use
dualstack URLs. It doesn't seem like there is a downside to
doing this, but a bunch of specs needed to be updated to reflect this.
This PR adds a small visual change to the new feature item on the `/admin/whats-new` page. When features are marked with an experimental site setting, they should show an indication on the feature item that it is "Experimental"
Currently, if an association is added as a tracked field in
`PostRevisor`, the `PostRevisionSerializer` class will try to serialize
it somehow. This will raise an error as ActiveRecord collection proxies
can't be serialized.
This patch addresses this issue by skipping any association tracked by
the `PostRevisor` class.
Prior to Uppy, the `uploads#create` endpoint used to receive a `type` param that indicated the purpose/target of the upload, such as `avatar`, `site_setting` and so on. With the introduction of Uppy, the `type` param became the MIME type of the file being uploaded, and the purpose/target of the upload became a new param called `upload_type`, however the backend could still use the `type` param (which now contains MIME type) as the purpose/target of the upload if `upload_type` is absent.
We technically don't need to send the MIME type over the network, but it seems like it's done by Uppy and we have no control over the `type` param that Uppy includes:
758de8167b/app/assets/javascripts/discourse/app/lib/uppy/uppy-upload.js (L146-L151)
This commit does a couple of things:
1. It amends the `uploads#create` endpoint so it always requires the `upload_type` param and doesn't fallback to `type` if `upload_type` is absent
2. It forces consumers of the `UppyUpload` class (and by extension `UppyImageUploader`) to specify `type` of the upload
Internal topic: t/140945.
The normalize_emails setting makes it so that only canonical e-mails are considered for validation purposes. This means disallowing "plus addressing". For example, with this enabled, bob@discourse.org and bob+foo@discourse.org are considered the same address, and you can only sign up with one of them.
Currently this is disabled by default, leading to a lot of spam sign-ups. It's healthier to consider this an opt-out setting.
This commit adds an API `upload_image` to `FormKitField` page object for setting an image file on an `Image` field in FormKit. Usage is like this:
```ruby
form.field("image_field").upload_image(image_path)
```
The `value` API also now supports `Image` fields; it returns an `Upload` record if the field has an uploaded image.
Recently we added a new feature for automatically gridding images in the composer (https://github.com/discourse/discourse/pull/29260). After testing this feature under a setting for a short period of time, the feature is no longer experimental anymore.
This PR removes the site setting `experimental_auto_grid_images`.
The primary key is usually a bigint column, but the foreign key columns
are usually of integer type. This can lead to issues when joining these
columns due to mismatched types and different value ranges.
This was using a temporary plugin / test API to make tests pass. After
more careful consideration, we concluded that it is safe to alter the
tables directly.
Docking is a leftover from older header code, it looks like it is no
longer used in the app. This helper was registering a scroll event
listener to check if the header should be docked or not. Initially, a
"docked" class was added to the body element. This class persisted
through the lifecycle of the app and the scroll event was doing no
useful work.
Some older themes may still use it in CSS, that will cause a regression,
from a quick look at existing code, the surface area should be small
(2-3 themes). It's worth removing the event listener for performance
reasons. We could possibly add the class "docked" statically to the body
element, but it's redundant. It's best to clean up the relevant CSS in
themes, where applicable.
This commit adds a new "Community title" field to the about config page. This field controls the `short_site_description` setting, which is shown in the browser tab for key pages such categories pages and topic lists.
Internal topic: t/140812.
This commit changes the uploads:secure_upload_analyse_and_update
and uploads:disable_secure_uploads to no longer rebake affected
posts inline. This just took way too long, and if the task stalled
you couldn't be sure if the rest of it completed.
Instead, we can update the baked_version of affected posts and
utilize our PeriodicalUpdates job to gradually rebake them. I added
warnings about increasing the site setting rebake_old_posts_count and
the global setting max_old_rebakes_per_15_minutes before doing this
as well.
For good measure, the affected post IDs are written to a JSON file too.
This commit contains two changes to how our site setting
keyword system works:
1. Crowdin, our translation provider, does not support YAML lists,
so we are changing site setting keywords in server.en.yml to
be pipe-separated (|)
2. It's unclear to translators what they are supposed to do with
aliases of site settings where the name has changed, e.g.
min_trust_level_for_here_mention. Instead of getting these as
keywords from the yml file, we can discern these from
SiteSettings::DeprecatedSettings automatically, and still use
them for client-side search
These changes should help improve the situation for translators.
Followup bd4e8422fe
In the previous commit, we introduced the `page_view_legacy_total_reqs`
report. However this was not tested properly, and due to a typo
the report returned no data.
This commit fixes the issue and adds a spec to catch this.
This commit adds a new "Invite" link to the sidebar for all users who can invite to the site. Clicking the link opens the invite modal without changing the current route the user is on. Admins can customize the new link or remove it entirely if they wish by editing the sidebar section.
Internal topic: t/129752.
This commit removes the feature flag for the new /about page, enabling it for all sites, and removes the code for old the /about page.
Internal topic: t/140413.
Followup 30fdd7738e
Adds a new site setting and corresponding user preference
to disable smart lists. By default they are enabled, because
this is a better experience for most users. A small number of
users would prefer to not have this enabled.
Smart lists automatically append new items to each
list started in the composer when enter is pressed. If
enter is pressed on an empty list item, it is cleared.
This setting will be removed when the new composer is complete.
This commit allows themes to define up to 2 screenshots
in about.json. These should be paths within the theme's
git repository, images with a 1MB max file size and max width 3840x2160.
These screenshots will be downloaded and stored against a theme
field, and we will use these in the redesigned theme grid UI.
These screenshots will be updated when the theme is updated
in the same way the additional theme files are.
For now this is gated behind a hidden `theme_download_screenshots`
site setting, to allow us to test this on a small number of sites without
making other sites make unnecessary uploads.
**Future considerations:**
* We may want to have a specialized naming system for screenshots. E.g. having light.png/dark.png/some_palette.png
* We may want to show more than one screenshot for the theme, maybe in a carousel or reacting to dark mode or color palette changes
* We may want to allow clicking on the theme screenshot to show a lightbox
* We may want to make an optimized thumbnail image for the theme grid
---------
Co-authored-by: Ted Johansson <ted@discourse.org>
Followup 9762e65758
When we added the Revise... option for posts/new topics
in the review queue, which sends a PM to the user, we used
`SystemMessage.create_from_system_user`, which always sends
the PM from the system user. However, this makes it so if the
user replies to the PM, which they are encouraged to do,
no one will see it unless they actively monitor the system inbox.
This commit changes it so `SystemMessage.create` is used,
which uses the `site_contact_username` and `site_contact_group`
site settings as participants in the sent PM. Then, when the
user replies, it will send to that inbox instead.
If `site_contact_username` is blank, the system user is used.
# Context
Add `disableDefaultKeyboardShortcuts` function to the plugin API to allow for disabling [default bindings](e4941278b2/app/assets/javascripts/discourse/app/lib/keyboard-shortcuts.js (L49)).
# Details
This function is used to disable a "default" keyboard shortcut. You can pass an array of shortcut bindings as strings to disable them.
**Please note that this function must be called from a pre-initializer.**
Example:
```js
api.disableDefaultKeyboardShortcuts(['command+f', 'shift+c']);
```
- Added system spec, displaying intended behavior
This patch replaces the parameters provided to a service through
`params` by the contract object.
That way, it allows better consistency when accessing input params. For
example, if you have a service without a contract, to access a
parameter, you need to use `params[:my_parameter]`. But with a contract,
you do this through `contract.my_parameter`. Now, with this patch,
you’ll be able to access it through `params.my_parameter` or
`params[:my_parameter]`.
Some methods have been added to the contract object to better mimic a
Hash. That way, when accessing/using `params`, you don’t have to think
too much about it:
- `params.my_key` is also accessible through `params[:my_key]`.
- `params.my_key = value` can also be done through `params[:my_key] =
value`.
- `#slice` and `#merge` are available.
- `#to_hash` has been implemented, so the contract object will be
automatically cast as a hash by Ruby depending on the context. For
example, with an AR model, you can do this: `user.update(**params)`.
Currently in services, we don’t make a distinction between input
parameters, options and dependencies.
This can lead to user input modifying the service behavior, whereas it
was not the developer intention.
This patch addresses the issue by changing how data is provided to
services:
- `params` is now used to hold all data coming from outside (typically
user input from a controller) and a contract will take its values from
`params`.
- `options` is a new key to provide options to a service. This typically
allows changing a service behavior at runtime. It is, of course,
totally optional.
- `dependencies` is actually anything else provided to the service (like
`guardian`) and available directly from the context object.
The `service_params` helper in controllers has been updated to reflect
those changes, so most of the existing services didn’t need specific
changes.
The options block has the same DSL as contracts, as it’s also based on
`ActiveModel`. There aren’t any validations, though. Here’s an example:
```ruby
options do
attribute :allow_changing_hidden, :boolean, default: false
end
```
And here’s an example of how to call a service with the new keys:
```ruby
MyService.call(params: { key1: value1, … }, options: { my_option: true }, guardian:, …)
```
Background
When creating webhooks on a site without the Discourse Category Experts plugin installed, the category_experts_unapproved_event and category_experts_approved_event webhook events are getting automatically added to webhooks without a way to disable them.
The category_experts_unapproved_event and category_experts_approved_event webhook events are associated with the Discourse Category Experts plugin so I am moving these webhook events into the Category Experts plugin.
Changes
This PR deletes Category Experts plugin specific webhook event types added into core.
The new style is called `categories_only_optimized` and it is designed
to show only the parent categories, without any subcategories. This
works best for communities with many categories (over a thousand).
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.
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
```
Database dumps sometimes reference functions in the `discourse_functions` schema. It's possible that some of these functions have been dropped in a newer version of Discourse. In that case, restoring an older backup will fail with a `ERROR: function discourse_functions.something_something() does not exist` error. The restore functionality contains a workaround for that problem, but it didn't work with functions created in plugin migrations.
This commit adds support for temporarily creating missing `discourse_functions` from plugins. And it adds a simple check if the DB migration file even contains the required `DROPPED_TABLES` or `DROPPED_COLUMNS` constant. We don't need to create an instance of the DB migration class unless one of those constants is used. This makes the restore slightly faster and works around a problem with migrations that execute without `up` or `down` methods (e.g. `BackfillChatChannelAndThreadLastMessageIdsPostMigrate`).
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.
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
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.
Creating or updating flags generates global side effects. Sometimes it
seems the state can leak from the flag specs.
This is probably related to the use of `fab!`. This patch replaces those
calls with standard `let`s. While the overall performances of these
tests will be a little less good, their state should not leak anymore.
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.
We want to allow lightboxing of smaller images, even if they are below the minimum size for image thumbnail generation.
This change sets a minimum threshold of 100 x 100 pixels for triggering the lightbox.
---------
Co-authored-by: Régis Hanol <regis@hanol.fr>
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.
This patch improves the custom `array` type available in contracts.
It’s now able to split strings on `|` on top of `,`, and to be more
consistent, it also tries to cast the resulting items to integers.
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>
Technically we don't show the edit custom section button on mobile, but the button is present so I just fixed it so the finder works on mobile. We should probably remove this test or find a way to make the button visible on mobile.
Also used `mobile: true` instead of manual url.
Adding the directory item test causes the default test to fail randomly due to directory items not getting removed properly.
Removing this for now, and also moving this test to the common system folder instead of system/user_page
#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.
- Uses a temporary, clean, per-test-process directory for minio data
- Runs a separate minio instance for each test process
- Unskips minio-based tests in CI
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.
- limits security key deletes to second factor keys
- also deletes backup codes (lingering backup codes break login flow entirely)
* Add spec for rake task to disable 2FA for a user
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
Currently, when the MessageFormat compiler fails on some translations,
we just have the raw output from the compiler in the logs and that’s not
always very helpful.
Now, when there is an error, we iterate over the translation keys and
try to compile them one by one. When we detect one that is failing, it’s
added to a list that is now outputted in the logs. That way, it’s easier
to know which keys are not properly translated, and the problems can be
addressed quicker.
---
The previous implementation of this patch had a bug: it wasn’t handling
locales with country/region code properly. So instead of iterating over
the problematic keys, it was raising an error.
In ed6c9d1545, we started flushing
Redis's database at the end of each test. However, we had something like
this:
```
config.after(:each, type: :system) { teardown system test stuff }
config.after(:each) { # flush redis }
```
When stuff was defined in this order, flushing redis was called before
the teardown of system test. Instead we have to switch the order around
which is what this commit does.
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.
This helps uncover issues with bigint columns that are joined with int
columns. It also introduces a temporary API for plugins to migrate int
columns to bigint in test environment to make tests pass.
We some times get the following failure on Github CI:
```
expected StandardError with message matching /some.host/, got #<Socket::ResolutionError: getaddrinfo: Temporary failure in name resolution> with backtrace:
```
* 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
There have been too many flaky tests as a result of leaking state in
Redis so it is easier to resolve them by ensuring we flush Redis'
database.
Locally on my machine, calling `Discourse.redis.flushdb` takes around
0.1ms which means this change will have very little impact on test
runtimes.
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.
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
XHR requests are handled differently by the application and the
responses do not have any preloaded data so the cache key needs to
differntiate between those requests.
Currently, when the MessageFormat compiler fails on some translations,
we just have the raw output from the compiler in the logs and that’s not
always very helpful.
Now, when there is an error, we iterate over the translation keys and
try to compile them one by one. When we detect one that is failing, it’s
added to a list that is now outputted in the logs. That way, it’s easier
to know which keys are not properly translated, and the problems can be
addressed quicker.
I think the check for the bookmark icon is too optimistic,
so the DB might not be updated by the time we check. Using
try_until_success should fix this, we also don't have a
toast to check against via AJAX success, by design.
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]`.
Previously admins could still click on topics when `suppress_secured_categories_from_admin` was set
This change improves the block so admins without permission will not be allowed to click through till they add themselves to appropriate groups
Keep in mind this setting is a quality of life setting and not a SECURITY
setting, admins have an infinite way of bypassing visiblity limits
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.
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.
With the current implementation, a service step can be written as:
```ruby
def my_step(a_default_value: 2)
…
end
```
That’s a pattern we want to avoid as default values (if needed) should
be probably defined in a contract.
This patch makes a service raise an exception if a default value is
encountered.
Currently, when certain search terms are provided, this can lead to
`Search.need_segmenting?` raising an error because it makes `URI#path`
to return `nil` instead of a string.
This patch forces a cast to string so it won’t raise anymore.
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.
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
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 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
The user directory (`/u`) excludes inactive and silenced users from the list, so for the sake parity, it makes sense to also exclude those users from the /about page stats.
Internal topic: t/70928.
- fetch models inside services
- validate `user_id` in contracts
- use policy objects
- extract more logic to actions
- write specs for services and action
Adds support for `input-date` field when calling
`fill_in` on a FormKit field. Capybara supports passing
a Date object to `fill_in(with: value)` for date inputs,
so there is nothing fancy that needs to be done to support this.
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.
The user option 'hide_profile_and_presence' is necessary to figure out
if the user status has to be displayed or not. In order to avoid N+1s
generated by `include_status?` method, both `user_status` and
`user_option` relations have to be included.
`track_sql_queries` only returned queries that were executed by
ActiveRecord. All queries executed through DB.exec, DB.query and others
were not returned.
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.
The Report model spec was directly adding methods
to the Report class, which was causing errors in the
admin reports controller because it would look for
a translation of the report name (e.g. report_timeout_test)
like so `I18n.t("reports.#{type}.title")`, then get an
error because the translation did not exist.
This is fixed by using `Report.stubs` instead, which is
cleaned up after every test.
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.
### UI changes
All of the UI changes described are gated behind the `use_legacy_pageviews`
site setting.
This commit changes the admin dashboard pageviews report to
use the "Consolidated Pageviews with Browser Detection" report
introduced in 2f2da72747 with
the following changes:
* The report name is changed to "Site traffic"
* The pageview count on the dashboard is counting only using the new method
* The old "Consolidated Pageviews" report is renamed as "Consolidated Legacy Pageviews"
* By default "known crawlers" and "other" sources of pageviews are hidden on the report
When `use_legacy_pageviews` is `true`, we do not show or allow running
the "Site traffic" report for admins. When `use_legacy_pageviews` is `false`,
we do not show or allow running the following legacy reports:
* consolidated_page_views
* consolidated_page_views_browser_detection
* page_view_anon_reqs
* page_view_logged_in_reqs
### Historical data changes
Also part of this change is that, since we introduced our new "Consolidated
Pageviews with Browser Detection" report, some admins are confused at either:
* The lack of data before a certain date , which didn’t exist before
we started collecting it
* Comparing this and the current "Consolidated Pageviews" report data,
which rolls up "Other Pageviews" into "Anonymous Browser" and so it
appears inaccurate
All pageview data in the new report before the date where the _first_
anon or logged in browser pageview was recorded is now hidden.
This upgrade is designed to be fully backwards-compatible. Any icon names which have changed will be automatically remapped to the new name. For now, this will happen silently. In future, once core & official themes/plugins have been updated, we will start raising deprecation errors to help theme/plugin authors update their code.
Extracted from https://github.com/discourse/discourse/pull/28715
Announcement at https://meta.discourse.org/t/were-upgrading-our-icons-to-font-awesome-6/325349
Co-authored-by: awesomerobot <kris.aubuchon@discourse.org>
* FEATURE: Log tag group changes in staff action log
This commit records every change (add, change, delete) to a tag group in
the staff action log.
It uses a modal that was originally called ThemeChangeModal to display
changes, allowing staffs to see the specific changes clearly. The modal
is renamed to StaffActionLogChangeModal in this PR.
ref: https://meta.discourse.org/t/-/325011/14
Co-authored-by: Keegan George <kgeorge13@gmail.com>
This was added 10 years ago, but currently there's not a single use in our public and private plugins and no reference in third-party plugins on github
* FEATURE: Add user to topic_tags_changed event
Add user to topic_tags_changed event context
Update automation plugin with new arguments in event
Update tests for new arguments
relates to https://github.com/discourse/discourse-chat-integration/pull/214
* DEV: change variable name for better readability
changed `tags` to be payload and used `values_at` to get the values of the keys
This commit adds a link to the top of the new /about page, shown to admins only, to allow them to easily navigate to `/admin/config/about` where they can edit the /about page.
Internal topic: t/137546.
This patch removes the `with_service` helper from the code base.
Instead, we can pass a block with actions directly to the `.call` method
of a service.
This simplifies how to use services:
- use `.call` without a block to run the service and get its result
object.
- use `.call` with a block of actions to run the service and execute
arbitrary code depending on the service outcome.
It also means a service is now “self-contained” and can be used anywhere
without having to include a helper or whatever.
Currently, categories support designating only 1 group as a moderation group on the category. This commit removes the one group limitation and makes it possible to designate multiple groups as mods on a category.
Internal topic: t/124648.
This commit introduces a new hidden site setting: `group_pm_user_limit`, default to `1000` which will raise an error when attempting to create a PM target a large group.
This will bring significant improvements to install speed & storage requirements. For information on how it may affect you, see https://meta.discourse.org/t/324521
This commit:
- removes the `yarn.lock` and replaces with `pnpm-lock.yaml`
- updates workspaces to pnpm format
- adjusts package dependencies to work with pnpm's stricter resolution strategy
- updates Rails app to load modules from more specific node_modules directories
- adds a `.pnpmfile` which automatically cleans up old yarn-managed `node_modules` directories
- updates various scripts to call `pnpm` instead of `yarn`
- updates patches to use pnpm's native patch system instead of patch-package
- adds a patch for licensee to support pnpm
Currently, when the default locale is Japanese, the search for a topic
using its URL, path or ID doesn’t work as expected. It will either
return wrong results or no result at all.
The problem lies with how we process the provided terms in Japanese
mode. For example, if `http://localhost/t/-/55` is provided, currently
this will result in `http localhost t 5 5` to be searched for.
This patch addresses the issue by checking whether the provided term
needs segmenting. If the provided term is a number, or a path or a full
URL, then it doesn’t need segmenting. When that happens we skip the
processing we normally apply for Japanese, making the search return the
expected results.
TagUser.rb is used to set user notification levels for a tag, we don't have a unique index on the notification level itself. This means that there might be some weird case where a user may have multiple of the same notification level on a tag.
This PR adds a migration which de-duplicates this based on defaults, where we keep the earliest record in the event there is multiple notification level per-user-per-tag.
* add data migration to keep only unexpired or most recently expired user password
* refactor to 1:1 relationship between User and UserPassword
* add migration to remove redundant indexes on user passwords
A new setting attribute is used to define the areas (separated by `|`).
In addition, endpoint `/admin/config/site_settings.json` accepts new `filter_area` data.
A previous refactor has moved this function in the controller instead of the route making it inaccessible to the modal.
This commit is fixing this and also adding a spec.
This change introduces a new thread notification level allowing users to get notified when someone replies to the thread.
Users who watch a thread will get a green notification on the chat icon and a user notification (blue). User notifications are consolidated based on thread id to prevent cluttering the original users notification area.
---------
Co-authored-by: Régis Hanol <regis@hanol.fr>
This commit will allow plugin developers to enable/disable the custom homepage.
Usage:
```ruby
register_modifier(:custom_homepage_enabled) do |enabled, args|
true
end
```
Args might contain request and/or current_user.
This commit introduces a little bit of duplication
since the old plugin UIs not using the new plugin show
page look different from ones like AI and Gamification
which have been converted. We can use the new admin
header component on the plugins list, but for the other
pages we are manually rendering a breadcrumb trail and
the list of plugin tabs.
Over time as we convert more plugins to use the new UI
guidelines and show page we can get rid of this duplication.
* DEV: Split slow test in multiple smaller tests
This might be faster because the smaller chunks of the test may run in
parallel.
* DEV: Fabricate reviewables only once
* Do not delete created external upload stubs for 2 days
instead of 1 hour if enable_upload_debug_mode is true,
this aids with server-side debugging.
* If using an API call, return the detailed error message
if enable_upload_debug_mode is true. In this case the user
is not using the UI, so a more detailed message is appropriate.
* Add a prefix to log messages in ExternalUploadHelpers, to
make it easier to find these in logster.
Prior to this fix we were calling `fetch_stats` which is never checking if we have a cache entry. This call is making a lot of SQL calls, so it's better to use the cache.
We're seeing a lot of log noise coming from unhandled exceptions stemming from requests to TopicsController#show where id is passed in as an array.
In the implementation of the method, we assume that if id is present it will be a string. This is because one of the routes to this action uses :id as a URL fragment, and so must be a string. However, there are other routes that go to this endpoint as well. Some of them don't have this URL fragment, so you can pass an arbitrary id query parameter.
Instead of a downstream unhandled exception, we raise a Discourse::InvalidParameters upfront.
In this PR we introduced a new setting `enforce_second_factor_on_external_auth` which disables enforce 2FA when the user is authenticated with an external provider.
https://github.com/discourse/discourse/pull/27506
However, with the first registration with an external provider, we authenticate the user right after activation. In that case, we need to also keep information that the user was authenticated with an external OAuth provider.
We were not updating `searchTerm` when changing the input which was making us always send an empty q parameter.
This commit is also adding tests for:
- initial url with q param
- filtering the bookmarks through the input
Makes it easier to reach the group from the category security
tab, and moves the trash button to the right to avoid misclicks.
Also converts the category permission row to gjs
This is a follow-up of d749227e87.
This patch checks if the key `not_found` is present on the result object
instead of calling `#blank?` on the model, as it can trigger an
`ActiveRecord` relation.
To test the restricted routing when filling up required fields, we fill up the field and then navigate to the root path and checking that we're not redirected.
This is somewhat flaky, and the screenshot shows we are back at the profile page, without any prompt to fill up fields.
My hypothesis is in cases where the backend is "slow" to respond, we're navigating away from the page before the request finishes (which will redirect back to the profile page.)
This PR adds an expectation after saving, to wait until the unrestricted profile page is rendered, before navigating away.
This commit changes the cutoff number for the admins and mods lists on the new /about page from 12 to 6. If the admins or mods lists are bigger than 6, the about page will display the 6 most recently seen admins/mods, and tuck the rest away behind a "view more" button.
Angus McLeod
Ted Johansson
Alan Guo Xiang Tan
Osama Sayegh
Martin Brennan
Kris
Bianca Nenciu
Renato Atilio
Keegan George
Jean
Gabriel Grubba
Loïc Guitaut
Krzysztof Kotlarek
benj
Régis Hanol
Penar Musaraj
Jan Cernik
Isaac Janzen
Guhyoun Nam
Jarek Radosz
Gerhard Schlager
Kelv
Roman Rizzi
David Battersby
Joffrey JAFFEUX
Sam
Natalie Tay
Juan David Martínez Cubillos
David Taylor
Jeff Wong
Yuvaraj J
Mark VanLandingham
Bianca Nenciu
Daniel Waterworth
Linca
Sérgio Saquetim