Followup c7e471d35a
It is currently possible to add a bundle (which is a collection
of actions used for a dropdown on the client) for a reviewable
via actions.add_bundle and then never add any actions to it.
This causes the client to explode, as seen in the referenced
commit, because of the way our store expects to resolve objects
referenced by ID that are passed down by the serializer, which
then causes Ember to have an unrecoverable render error.
Fixing this on the serializer level is not really possible because
of all the ActiveModel::Serializer magic that serializes
objects by ID reference when doing things like has_many.
`Reviewable#actions_for` is a better place to do this anyway,
because this is the main location where the bundles and actions
are built for every action via the serializer.
Currently only system flags are translated. When we send message to the user that their post was deleted because of custom flag, we should default to custom flag name.
* DEV: unsilence deprecation warnings for old Font Awesome icon names
* update fa-user to user font awesome icon name
* update pencil-alt to pencil font awesome 6 icon name
In order to limit issues with duplicate inline CSS definitions, this will now deduplicate inline CSS styles with the "last-to-be-defined-wins" strategy.
Also removes unecessary whitespaces in inline styles.
Context - https://meta.discourse.org/t/resolve-final-styles-in-email-notifications/310219
Co-authored-by: Thomas Kalka <thomas.kalka@gmail.com>
When serializing the `body_changes` in the `PostRevisionSerializer`, we create two diffs: one for the `cooked` and another one for the `raw` version of the post.
Inside `DiscourseDiff`, we generate both `html` and `markdown` diffs when we only need the `html` diffs for the `cooked` version of the post and the `markdown` diff for the `raw` version of the post.
This solves the issue repored in https://meta.discourse.org/t/server-error-accessing-topic-revisions-on-a-specific-topic/339185 where some revisions would return 500 because of a `ArgumentError : Attributes per element limit exceeded` exception when trying to generate the `html` diff on a very large `raw`.
In some cases in CI env, it seems the AR connection isn’t available and
the `ensure` block is executed. It’s calling `#verify!` on the
connection, so it can fail sometimes. This is probably why
`#clear_active_connections!` was failing too sometimes.
Here, we just check the connection is present before clearing the
connections.
Spec was flaky cause work could still be in pipeline after the defer
length is 0. Our length denotes the backlog, not the in progress
count.
This adds a mechanism for gracefully stopping the queue and avoids
wait_for callse
We already add the "delete user" and "delete and block user" options to the drop-down for potential spam, but we should do this for potentially illegal posts as well.
This is entirely based on the implementation for the potential spam one, including caching the status on the reviewable record.
Also note that just as for potential spam, the user must be "deletable" for the option to appear.
I also took the liberty to move the options in the drop-down to what I think is a more intuitive place. (Between delete post and suspend/silence user.)
Sometimes changes to "What's new?" feed items are made or the feed items are
removed altogether, and the polling interval to check for new features is 1 day.
This is quite long, so this commit introduces a "Check for updates"
button for admins to click on the "What's new?" page which will bust
the cache for the feed and check again at the new features endpoint.
This is limited to 5 times per minute to avoid rapid sending of
requests.
This fix handles the case where an In-Reply-To mail header
can contain multiple Message-IDs. We use this header to
try look up an EmailLog record to find the post to reply
to in the group email inbox flow.
Since the case where multiple In-Reply-To Message-IDs is
rare (we've only seen a couple of instances of this causing
errors in the wild), we are just going to use the first one
in the array.
Also, Discourse does not support replying to multiple posts
at once, so it doesn't really make sense to use multiple
In-Reply-To Message-IDs anyway.
* DEV: Gracefully handle `regex_replace` violations of column length constraints
This is a follow-up to the `remap` [refactor](9b0cfa99c5).
Similar to `remap`, the entire `regex_replace` operation fails if the new content exceeds the column’s max length.
This change introduces an optional mode, controlled by the new `skip_max_length_violations` param
to skip records eligible for `regex_replace` where the new content violates the max column length constraint.
It also includes updates to the exception message raised when `regex_replace` fails to include more details
* DEV: Remove string escapes in heredoc text
Uploads that are linked to site settings shouldn't be flagged as secure in login-required sites that enable secure uploads. However, in order for site setting uploads to not be marked secured, the frontend uploader has to include 2 params in the upload request: `for_site_setting: true` and `type: "site_setting"`.
Since these 2 params are semantically identical, we want the `type: "site_setting"` param alone to make the upload correctly treated as a site setting upload. To achieve that, we need to include the `site_setting` type in the public types list because the `for_site_setting` param has the same effect — it marks the upload as a public type.
b138eaf9e5/lib/upload_security.rb (L128-L131)
Following a recent commit (cb4b8146a3),
the benchmark script wasn’t working anymore (and the related rake task).
This patch fixes it. It also adds some information about Ruby YJIT being
enabled or not.
This patch adds a new step to services named `try`.
It’s useful to rescue exceptions that some steps could raise. That way,
if an exception is caught, the service will stop its execution and can
be inspected like with any other steps.
Just wrap the steps that can raise with a `try` block:
```ruby
try do
step :step_that_can_raise
step :another_step_that_can_raise
end
```
By default, `try` will catch any exception inheriting from
`StandardError`, but we can specify what exceptions to catch:
```ruby
try(ArgumentError, RuntimeError) do
step :will_raise
end
```
An outcome matcher has been added: `on_exceptions`. By default it will
be executed for any exception caught by the `try` step.
Here also, we can specify what exceptions to catch:
```ruby
on_exceptions(ArgumentError, RuntimeError) do |exception|
…
end
```
Finally, an RSpec matcher has been added:
```ruby
it { is_expected.to fail_with_exception }
# or
it { is_expected.to fail_with_exception(ArgumentError) }
```
* DEV: Gracefully handle remaps which violate DB column constraints
This change implements length constraint enforcement to skip remaps
which exceed column max lengths
* DEV: Only perform skipped column stats lookup when verbose is true
* DEV: Tidy up specs
* DEV: Make skipping violating remap behaviour opt-in
This change introduces a new `skip_max_length_violations` param for
`remap`, set to `false` by default to ensure we still continue to fail
hard when max lenth constraints are violated.
To aid in quick resolution when remaps fail, this change also
adds more context to the exception message to include the offending table
and column information
* Apply suggestions from code review
Co-authored-by: Gerhard Schlager <gerhard.schlager@discourse.org>
* FIX: Various fixes
- Linter errors
- Remap status "logger" early return condition
---------
Co-authored-by: Gerhard Schlager <gerhard.schlager@discourse.org>
Firstly, we need to understand that ActiveRecord can be
connected to a role which prevent writes and this happens in Discourse when a
replica database has been setup for failover purposes. When a role
prevent writes from happening, ActiveRecord will raise the
`ActiveRecord::ReadOnlyError` if a write query is attempted.
Secondly, theme fields are baked at runtime within GET requests. The
baking process involves writing the baked value to the
`ThemeField#baked_value` column in the database.
If we combine the two points above, we can see how the writing of the
baked value to the database will trigger a `ActiveRecord::ReadOnlyError`
in a GET requests when the database is connected to a role preventing
writes. However, failing to bake a theme is not the end of the world and
should not cause GET requests to fail. Therefore, this commit adds a rescue
for `ActiveRecord::ReadOnlyError` in the `ThemeField#ensure_baked!`
method.
It splits the hide_profile_and_presence user option and the default_hide_profile_and_presence site setting for more granular control. It keeps the option to hide the profile under /u/username/preferences/interface and adds the presence toggle in the quick user menu.
Co-authored-by: Régis Hanol <regis@hanol.fr>
This reverts commit 5a00a041f1.
Implementation is currently not correct. Multiple uploads can share the
same etag but have different paths in the S3 bucket.
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.
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"
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.
The `categories_only_optimized` category page style has been introduced
in commit d37a0d401c. This commit makes
sure that style is enforced for users who can see over 1000 categories
in order to keep `/categories` page functional.
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.
