Commit Graph

1408 Commits

Author SHA1 Message Date
Alan Guo Xiang Tan 0d1d707213
FIX: Set `Jobs::BackfillSidebarSiteSettings` cluster concurrency to 1 (#22316)
What is the problem?

When an admin changes the default_sidebar_categories or default_sidebar_tags site settings and opts to backfill the setting,
we currently enqueue a sidekiq job to run the backfilling operation. When an admin changes those settings multiple times
within a short time frame, multiple sidekiq jobs with different backfilling parameters will be enqueued.
This is problematic if multiple jobs are executed concurrently as it may lead to situations where a job
with “outdated” site setting values is completed after a job with the “latest” site setting values.

What is the fix?

By setting `cluster_concurrency` to `1`, we ensure that only one of such
backfilling job will execute across all the sidekiq processes that are
deployed at any point in time. Since Sidekiq pops off job in the order
in which they are pushed, limiting the cluster concurrency here will
allow us to execute the enqueued `Jobs::BackfillSidebarSiteSettings`
jobs serially.
2023-06-28 13:07:46 +08:00
Alessio Cosenza 56718504ac
FEATURE: Add hooks for email poller plugins (#21384)
While we are unable to support OAUTH2 with pop3 (due to upstream dependency ruby/net-pop#16), we are adding the support for mail pollers plugin. Doing so, it would be possible to write a plugin which then uses other ways (microsoft graph sdk for example) to poll emails from a mailbox.

The idea is that a plugin would define a class which inherits from Email::Poller and defines a poll_mailbox static method which returns an array of strings. Then the plugin could call register_mail_poller(<class_name>) to have it registered. All the configuration (oauth2 tokens, email, etc) could be managed by sitesettings defined in the plugin.
2023-06-26 13:16:03 +08:00
Ted Johansson a183f14d09
DEV: Update display name in new quote format - Part 2 (#22104)
This change adds support retroactively updating display names in the new quote format when the user's name is changed. It happens through a background job that is triggered by a callback when a user is saved with a new name.
2023-06-26 11:01:59 +08:00
Sam 9e241e82e9
DEV: use HTML5 version of loofah (#21522)
https://meta.discourse.org/t/markdown-preview-and-result-differ/263878

The result of this markdown had different results in the composer preview and the post. This is solved by updating Loofah to the latest version and using html5 fragments like our user had reported. While the change was only needed in cooked_post_processor.rb for this fix, other areas also had to be updated due to various side effects.
2023-06-20 09:49:22 +08:00
Juan David Martínez Cubillos 0626b7c413
DEV: Fix warning when exporting Staff Actions (#22168) 2023-06-19 09:23:21 +08:00
Ted Johansson a674c6c4c2
DEV: Update username in new quote format - Part 1 (#22032)
When we introduced the new quote format with full-name display name:

```
[quote="Ted Johansson, post:1, topic:2, username:ted"]
we overlooked the code responsible for rewriting quotes when a user's name is changed.
```

The functional part of this change adds support for the new quote format in the code that updates quotes when a user's username changes. See the test case in `spec/services/username_changer_spec.rb` for the details.

In addition, this change adds a regression test for PrettyText to cover the new quote format, and extracts the code responsible for rewriting raw and cooked quotes into its own `QuoteRewriter` class. The functionality of the latter is tested through the tests in `spec/services/username_changer_spec.rb`.
2023-06-14 16:14:11 +08:00
Sam eb603b246b
PERF: limit anonymization to 1 per cluster (#21992)
Anonymization is among the most expensive operations we can perform with
extreme potential to impact the database. To mitigate risk we only allow a
single anonymization across the entire cluster concurrently.

This commit introduces support for `cluster_concurrency 1`. When you set that on a Job it will only allow 1 concurrent execution per cluster.
2023-06-14 08:30:23 +10:00
Juan David Martínez Cubillos 1a27e715ee
PERF: Increase performance of large csv exports for User Histories (#22008)
* PERF: Increase performance of large csv exports for User Histories

* added find_each for screened_email_export
2023-06-13 16:05:08 -05:00
Faizaan Gagan d1334a7aaf
FIX: consider users.created_at for inactive cleanup (#21688) 2023-05-23 13:41:23 +05:30
Faizaan Gagan efdfddf7fc
FIX: consider users with trashed topics/posts for inactive cleanup (#21678)
* FIX: consider users with trashed topics/posts for inactive cleanup

* defer checking for missing associations
2023-05-23 00:26:24 +05:30
Keegan George 082821c754
DEV: Remove legacy user menu (#21308) 2023-05-17 09:16:42 -07:00
Bianca Nenciu cc18a99105
FEATURE: Add new notification for admin problems (#21287)
Add new notification for admin problems to replace old PM-based flow.
2023-05-03 19:35:22 +03:00
Krzysztof Kotlarek a8e28060d1
FIX: rename notify_about_flags_after to notify_about_reviewable_item_after (#21320)
Change name and description for SiteSetting to make it easier to understand.
2023-05-02 08:08:22 +10:00
David Taylor cd88af8876
FIX: Ensure reviewable counts are updated correctly for new user menu (#21222)
On the client-side, message-bus subscriptions and reviewable count UI is based on the 'redesigned_user_menu_enabled' boolean. We need to use the same logic on the server-side to ensure things work correctly when legacy navigation is used alongside the new user menu.
2023-04-24 16:59:32 +01:00
David Taylor 93c33e02f0
PERF: Avoid full `posts` table scans during anonymisation (#21081)
2e78045a fixed the anonymization job so that it correctly updated self-mentions, which are not logged in the post_actions table. The solution was to scan the entire `posts` table with an `raw ILIKE` query. On sites with many posts, this can take a very long time.

This commit updates the job to take a two-pass approach:

First, we update posts based on the post_actions table. This is much more efficient than a full table scan, and takes care of all 'non-self' mentions.

Then, we make a second pass using the `raw ILIKE` approach. Since we already took care of most posts, we can scope this down to self-mentions only. By filtering the query to a specific posts.user_id, it is significantly more performant than a full table scan.
2023-04-12 18:39:10 +01:00
Loïc Guitaut d151f4ee9d
FIX: Don’t assume post is available in UserEmail job (#21054)
Currently, we’re performing a check when a user is suspended in the
`UserEmail` job and we’re assuming a `post` is always available, which
is not the case. The code indeed breaks when the job is called with the
`account_suspended` type option.

This patch fixes this issue by making the check use the safe navigation
operator, thus making it working when `post` is not provided.
2023-04-12 12:34:22 +10:00
Bianca Nenciu 142d2ab65e
FEATURE: Move bootstrap mode indicator to header (#20663)
The information about bootstrap mode has been moved from the notice to
the README topic for admins.
2023-03-24 15:59:03 +02:00
Loïc Guitaut 8a8f2758d5 DEV: Refactor `Jobs::UserEmail` a little 2023-03-14 09:23:06 +01:00
Loïc Guitaut 27f7cf18b1 FIX: Don’t email suspended users from group PM
Currently, when a suspended user belongs to a group PM (private message
with more than two people in it) and a staff member sends a message to
this group PM, then the suspended user will receive an email.
This happens because a suspended user can only receive emails from staff
members. But in this case, this can be seen as a bug as the expected
behavior would be instead to not send any email to the suspended user. A
staff member can participate in active discussions like any other
member and so their messages in this context shouldn’t be treated
differently than the ones from regular users.

This patch addresses this issue by checking if a suspended user receives
a message from a group PM or not. If that’s the case then an email won’t
be sent no matter if the post originated from a staff member or not.
2023-03-08 15:53:53 +01:00
chapoi e52bbc1230
UX/DEV: Review queue redesign fixes (#20239)
* UX: add type tag and design update

* UX: clarify status copy in reviewQ

* DEV: switch to selectKit

* UX: color approve/reject buttons in RQ

* DEV: regroup actions

* UX: add type tag and design update

* UX: clarify status copy in reviewQ

* Join questions for flagged post with "or" with new I18n function
* Move ReviewableScores component out of context
* Add CSS classes to reviewable-item based on human type

* UX: add table header for scoring

* UX: don't display % score

* UX: prefix modifier class with dash

* UX: reviewQ flag table styling

* UX: consistent use of ignore icon

* DEV: only show context question on pending status

* UX: only show table headers on pending status

* DEV: reviewQ regroup actions for hidden posts

* UX: reviewQ > approve/reject buttons

* UX: reviewQ add fadeout

* UX: reviewQ styling

* DEV: move scores back into component

* UX: reviewQ mobile styling

* UX: score table on mobile

* UX: reviewQ > move meta info outside table

* UX: reviewQ > score layout fixes

* DEV: readd `agree_and_keep` and fix the spec tests.

* Fix the spec tests

* fix the quint test

* DEV: readd deleting replies

* UX: reviewQ copy tweaks

* DEV: readd test for ignore + delete replies

* Remove old

* FIX: Add perform_ignore back in for backwards compat

* DEV: add an action alias `ignore` for `ignore_and_do_nothing`.

---------

Co-authored-by: Martin Brennan <martin@discourse.org>
Co-authored-by: Vinoth Kannan <svkn.87@gmail.com>
2023-03-02 16:40:53 +01:00
Sam 654959faa4
DEV: reduce amount of errors logged when notifying on flags (#20472)
Forcing distributed muted to raise when a notify reviewable job is running
leads to excessive errors in the logs under many conditions.

The new pattern

1. Optimises the counting of reviewables so it is a lot faster
2. Holds the distributed lock for 2 minutes (max)


The downside is the job queue can get blocked up when tons of notify
reviewables are running at the same time. However this should be very
rare in the real world, as we only notify when stuff is flagged which
is fairly infrequent.

This also give a fair bit more time for the notifications which may be
a little slow on large sites with tons of mods.
2023-03-01 08:58:32 +11:00
Loïc Guitaut f7c57fbc19 DEV: Enable `unless` cops
We discussed the use of `unless` internally and decided to enforce
available rules from rubocop to restrict its most problematic uses.
2023-02-21 10:30:48 +01:00
Ted Johansson 25a226279a
DEV: Replace #pluck_first freedom patch with AR #pick in core (#19893)
The #pluck_first freedom patch, first introduced by @danielwaterworth has served us well, and is used widely throughout both core and plugins. It seems to have been a common enough use case that Rails 6 introduced it's own method #pick with the exact same implementation. This allows us to retire the freedom patch and switch over to the built-in ActiveRecord method.

There is no replacement for #pluck_first!, but a quick search shows we are using this in a very limited capacity, and in some cases incorrectly (by assuming a nil return rather than an exception), which can quite easily be replaced with #pick plus some extra handling.
2023-02-13 12:39:45 +08:00
Selase Krakani 2e78045af1
FIX: Extend username updates to self-mentions (#20071)
Posts with self-mentions aren't updated with username updates. This happens
because mention `UserAction` entries aren't logged for self-mentions.

This change updates the lookup of `Post` and `PostRevision` with mentions to bypass
`UserAction` entries.
2023-02-02 12:33:42 +00:00
jbrw f8863b0f98
DEV: Limit concurrency of NotifyReviewables job (#19968)
Under scenarios of extremely high load where large numbers of `Reviewable*` items are being created, it has been observed that multiple instances of the `NotifyReviewable` job may run simultaneously.

These jobs will work satisfactorily if the concurrency is limited to 1, and the different types of jobs (items reviewable by admins, vs moderators, vs particular groups, etc.) are run eventually.

This change introduces a new option to `DistributedMutex` which allows the `max_get_lock_attempts` to be specified. If the number is exceeded an error will be raised, which will cause Sidekiq to requeue the job. Sidekiq has existing logic to back-off on retry times for jobs that have failed multiple times.
2023-01-25 15:19:11 -05:00
Daniel Waterworth 666536cbd1
DEV: Prefer \A and \z over ^ and $ in regexes (#19936) 2023-01-20 12:52:49 -06:00
Régis Hanol b704e338ef
DEV: extract anniversary badge query (#19716)
So it can easily be overwritten in a plugin for example.

### Added more tests to provide better coverage

We previously only had `u.silenced_till IS NULL` but I made it consistent with pretty much every other places where we check for "active" users.

These two new lines do change the query a tiny bit though. 

**Before** 

- You could not get the badge if you were currently silenced (no matter what period is being checked)
- You could get the badge if you were suspended 😬 

**After**

- You can't get the badge if you were silenced during the past year
- You can't get the badge if you were suspended during the past year


### Improved the performance of the query by using `NOT EXISTS` instead of `LEFT JOIN / COUNT() = 0`

There is no difference in behaviour between 

```sql
LEFT JOIN user_badges AS ub ON ub.user_id = u.id AND ...
[...]
HAVING COUNT(ub.*) = 0
```

and

```sql
NOT EXISTS (SELECT 1 FROM user_badges AS ub WHERE ub.user_id = u.id AND ...)
```

The only difference is performance-wise. The `NOT EXISTS` is 10-30% faster on very large databases (aka. posts and users in X millions). I checked on 3 of the largest datasets I could find.
2023-01-16 11:55:00 +01:00
Alan Guo Xiang Tan 8a7b62b126
DEV: Fix threading error when running jobs immediately in system tests (#19811)
```
class Jobs::DummyDelayedJob < Jobs::Base
  def execute(args = {})
  end
end

RSpec.describe "Jobs.run_immediately!" do
  before { Jobs.run_immediately! }

  it "explodes" do
    current_user = Fabricate(:user)
    Jobs.enqueue_in(1.seconds, :dummy_delayed_job)
    sign_in(current_user)
  end
end
```

The test above will fail with the following error if `ActiveRecord::Base.connection_handler.clear_active_connections!` is called before the configured Capybara server checks out a connection from the connection pool.

```
     ActiveRecord::ActiveRecordError:
       Cannot expire connection, it is owned by a different thread: #<Thread:0x00007f437391df58@puma srv tp 001 /home/tgxworld/.asdf/installs/ruby/3.1.3/lib/ruby/gems/3.1.0/gems/puma-6.0.2/lib/puma/thread_pool.rb:106 sleep_forever>. Current thread: #<Thread:0x00007f437d6cfc60 run>.
```

We're not exactly sure if this is an ActiveRecord bug or not but we've
invested too much time into investigating this problem. Fundamentally,
we also no longer understand why `ActiveRecord::Base.connection_handler.clear_active_connections!` is being called in an ensure block
within `Jobs::Base#perform` which was added in
ceddb6e0da 10 years ago. This
commit moves the logic for running jobs immediately out of the
`Jobs::Base#perform` method into another `Jobs::Base#perform_immediately` method such that
`ActiveRecord::Base.connection_handler.clear_active_connections!` is not
called. This change will only impact the test environment.
2023-01-10 13:41:25 +08:00
Daniel Waterworth dfc449a32e
FIX: Continue if checking themes if updating one fails (#19665) 2023-01-09 11:56:43 -06:00
David Taylor 5a003715d3
DEV: Apply syntax_tree formatting to `app/*` 2023-01-09 14:14:59 +00:00
Alan Guo Xiang Tan ab3a032b4b
SECURITY: BCC active user emails from group SMTP (#19725)
When sending emails out via group SMTP, if we
are sending them to non-staged users we want
to mask those emails with BCC, just so we don't
expose them to anyone we shouldn't. Staged users
are ones that have likely only interacted with
support via email, and will likely include other
people who were CC'd on the original email to the
group.

Co-authored-by: Martin Brennan <martin@discourse.org>
2023-01-05 06:07:50 +08:00
Selase Krakani 7ba115769a
DEV: Skip push notifications for active online users (#19502)
* DEV: Skip push notifications for active online users

Currently, users with active push subscriptions get push notifications
regardless of their "presence" on the site.

This change introduces a `push_notification_time_window_mins`
site setting which is used in conjunction with a user's `last_seen_at` to
determine if push notifications should be sent. A user is considered to
be actively online if their `last_seen_at` is within `push_notification_time_window_mins`
minutes. `push_notification_time_window_mins` is set to 10 by default.

* DEV: Remove client param for push_notification_time_window_mins site setting

Co-authored-by: Bianca Nenciu <nbianca@users.noreply.github.com>

Co-authored-by: Bianca Nenciu <nbianca@users.noreply.github.com>
2022-12-19 20:17:40 +00:00
Osama Sayegh 1c03d6f9b9
FEATURE: Send notifications to admins when new features are released (#19460)
This commit adds a new notification that gets sent to admins when the site gets new features after an upgrade/deploy. Clicking on the notification takes the admin to the admin dashboard at `/admin` where they can see the new features under the "New Features" section.

Internal topic: t/87166.
2022-12-15 20:12:53 +03:00
Alan Guo Xiang Tan fde9e6bc25
DEV: Migrate sidebar site settings (#19336)
This new site setting replaces the
`enable_experimental_sidebar_hamburger` and `enable_sidebar` site
settings as the sidebar feature exits the experimental phase.

Note that we're replacing this without depreciation since the previous
site setting was considered experimental.

Internal Ref: /t/86563
2022-12-08 09:44:29 +08:00
Osama Sayegh 3ff6f6a5e1
FIX: Exclude claimed reviewables from user menu (#19179)
Users who can access the review queue can claim a pending reviewable(s) which means that the claimed reviewable(s) can only be handled by the user who claimed it. Currently, we show claimed reviewables in the user menu, but this can be annoying for other reviewers because they can't do anything about a reviewable claimed by someone. So this PR makes sure that we only show in the user menu reviewables that are claimed by nobody or claimed by the current user.

Internal topic: t/77235.
2022-12-01 07:09:57 +08:00
Alan Guo Xiang Tan 7c321d3aad
PERF: Update `Group#user_count` counter cache outside DB transaction (#19256)
While load testing our user creation code path in production, we
identified that executing the DB statement to update the `Group#user_count` column within a
transaction is creating a bottleneck for us. This is because the
creation of a user and addition of the user to the relevant groups are
done in a transaction. When we execute the DB statement to update
`Group#user_count` for the relevant group, a row level lock is held
until the transaction completes. This row level lock acts like a global
lock when the server is creating users that will be added to the same
group in quick succession.

Instead of updating the counter cache within a transaction which the
default ActiveRecord `counter_cache` option does, we simply update the
counter cache outside of the committing transaction.

Co-authored-by: Rafael dos Santos Silva <xfalcox@gmail.com>

Co-authored-by: Rafael dos Santos Silva <xfalcox@gmail.com>
2022-11-30 11:52:08 -03:00
Selase Krakani c7ccb17433
FEATURE: Add cooked post to user archive exports (#18979)
This change allows easily accessible secure media URLs to be available
in the exported data.
2022-11-11 11:07:32 +00:00
Daniel Waterworth 167181f4b7
DEV: Quote values when constructing SQL (#18827)
All of these cases should already be safe, but still good to quote for
"defense in depth".
2022-11-01 14:05:13 -05:00
David Taylor 68b4fe4cf8
SECURITY: Expand and improve SSRF Protections (#18815)
See https://github.com/discourse/discourse/security/advisories/GHSA-rcc5-28r3-23rr

Co-authored-by: OsamaSayegh <asooomaasoooma90@gmail.com>
Co-authored-by: Daniel Waterworth <me@danielwaterworth.com>
2022-11-01 16:33:17 +00:00
Alan Guo Xiang Tan 1b56a55f50
DEV: Sidebar default tags and categories are determined at user creation (#18620)
The previous sidebar default tags and categories implementation did not
allow for a user to configure their sidebar to have no categories or
tags. This commit changes how the defaults are applied. When a user is being created,
we create the SidebarSectionLink records based on the `default_sidebar_categories` and
`default_sidebar_tags` site settings. SidebarSectionLink records are
only created for categories and tags which the user has visibility on at
the point of user creation.

With this change, we're also adding the ability for admins to apply
changes to the `default_sidebar_categories` and `default_sidebar_tags`
site settings historically when changing their site setting. When a new
category/tag has been added to the default, the new category/tag will be
added to the sidebar for all users if the admin elects to apply the changes historically.
Like wise when a tag/category is removed, the tag/category will be
removed from the sidebar for all users if the admin elects to apply the
changes historically.

Internal Ref: /t/73500
2022-10-27 06:38:50 +08:00
David Taylor 3115f38de2
PERF: Move dominant color calculation to separate job (#18501)
This will ensure that any potential problems with this process do not affect the performance or reliability of the PeriodicalUpdates job.
2022-10-06 13:26:08 +01:00
Krzysztof Kotlarek d5f6262c4f
FIX: watched topic overcome muted category (#18480)
Previously, when categories were not muted by default, we were sending message about unmuted topics (topics which user explicitly set notification level to watching)

The same mechanism can be used to fix a bug. When the user was explicitly watching topic, but category was muted, then the user was not informed about new reply.
2022-10-06 11:10:43 +11:00
Bianca Nenciu cf646b2061
FIX: Count resulting bulk invites correctly (#18461)
Skipped invites were not counted at all and some invites could generate
more than one error and resulted in a grand total that was not equal to
the count of bulk invites.
2022-10-04 18:41:06 +03:00
Martin Brennan 8ebd5edd1e
DEV: Rename secure_media to secure_uploads (#18376)
This commit renames all secure_media related settings to secure_uploads_* along with the associated functionality.

This is being done because "media" does not really cover it, we aren't just doing this for images and videos etc. but for all uploads in the site.

Additionally, in future we want to secure more types of uploads, and enable a kind of "mixed mode" where some uploads are secure and some are not, so keeping media in the name is just confusing.

This also keeps compatibility with the `secure-media-uploads` path, and changes new
secure URLs to be `secure-uploads`.

Deprecated settings:

* secure_media -> secure_uploads
* secure_media_allow_embed_images_in_emails -> secure_uploads_allow_embed_images_in_emails
* secure_media_max_email_embed_image_size_kb -> secure_uploads_max_email_embed_image_size_kb
2022-09-29 09:24:33 +10:00
Loïc Guitaut 26fe047724 DEV: Use AR enums in reviewables related code
This is the first patch of many to replace our custom enums in Ruby by
the ones provided by `ActiveRecord`.
2022-09-22 14:44:27 +02:00
David Taylor d0243f741e
UX: Use dominant color as image loading placeholder (#18248)
We previously had a system which would generate a 10x10px preview of images and add their URLs in a data-small-upload attribute. The client would then use that as the background-image of the `<img>` element. This works reasonably well on fast connections, but on slower connections it can take a few seconds for the placeholders to appear. The act of loading the placeholders can also break or delay the loading of the 'real' images.

This commit replaces the placeholder logic with a new approach. Instead of a 10x10px preview, we use imagemagick to calculate the average color of an image and store it in the database. The hex color value then added as a `data-dominant-color` attribute on the `<img>` element, and the client can use this as a `background-color` on the element while the real image is loading. That means no extra HTTP request is required, and so the placeholder color can appear instantly.

Dominant color will be calculated:
1. When a new upload is created
2. During a post rebake, if the dominant color is missing from an upload, it will be calculated and stored
3. Every 15 minutes, 25 old upload records are fetched and their dominant color calculated and stored. (part of the existing PeriodicalUpdates job)

Existing posts will continue to use the old 10x10px placeholder system until they are next rebaked
2022-09-20 10:28:17 +01:00
Alan Guo Xiang Tan 0f0048e8e3
DEV: Enable new user menu when experimental sidebar hamburger is enabled (#18133)
When `enable_experimental_sidebar_hamburger` site setting is enabled, we
will switch to rendering the new user menu.
2022-08-31 21:15:01 +03:00
jbrw ea5d303a07
DEV: Increase timeout when pulling hotlinked image (#18036)
We observed that some sites seemingly put us in a tarpit when we attempt to pull hotlinked images. Increasing the timeout will help in these situations.
2022-08-23 09:38:10 -04:00
Jarek Radosz 2361833844
FIX: Don't raise on deleted topic in UpdateHotlinkedRaw (#17901)
Fixes:

```
Job exception: undefined method `acting_user=' for nil:NilClass
```

in
```
/var/www/discourse/lib/post_revisor.rb:181:in `revise!'
/var/www/discourse/app/models/post.rb:646:in `revise'
/var/www/discourse/app/jobs/regular/update_hotlinked_raw.rb:24:in `execute'
```
2022-08-13 22:40:26 +02:00
Roman Rizzi 58b135d6d3
DEV:Using symbols is deprecated, use strings instead (#17874) 2022-08-11 11:33:41 -03:00