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.
Followup 0568d36133
Followup 97cf069a06
Due to the S3 dualstack endpoint change, sites with
S3 backups configured but _not_ S3 uploads were erroring,
with admins unable to access the backups page. This
commit fixes the error by not enabling S3 dualstack
endpoints if S3 uploads have not been enabled, backups
don't need to use them.
c.f. https://meta.discourse.org/t/unable-to-backup-or-navigate-to-backups/335899
* FEATURE: Add skip notification option to group invite to topic
* DEV: rename `skip_notification` to `should_notify`
* DEV: update `should_notify` param to be default `true` in controllers
* DEV: update spec to use `greater than` instead of `equal to` to prevent flakiness
* Update app/controllers/topics_controller.rb
Co-authored-by: David Taylor <david@taylorhq.com>
* DEV: merged two `#invite_group` specs into one
* DEV: Added test case for `invite-group` in requests spec
---------
Co-authored-by: David Taylor <david@taylorhq.com>
Redesign the permalinks page to follow the UX guide. In addition, the ability to edit permalinks was added.
This change includes:
- move to RestModel
- added Validations
- update endpoint and clear old values after the update
- system specs and improvements for unit tests
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 is a very simple change, which creates a permanent table in the DB, rather than generating a temporary table when moving posts. This change is about capturing data and any usage will appear in a follow-up.
I did include a new column created_new_topic in the new table, so that it can be easily audited without having to compare destination topic created_at with moved_post records.
This commit removes the new optimized category style introduced in
previous commits (d37a0d40, 9a80d718 and 430c42ac), in favour of the
existent `categories_only`.
This reverts commit 5a00a041f1.
Implementation is currently not correct. Multiple uploads can share the
same etag but have different paths in the S3 bucket.
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.
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 commit switches the route for the about config page to retrieve the list of site settings that control the /about page by their area instead of their individual names.
Internal topic: t/136384.
I have been unable to figure out a way of testing this usefully (as I fear it would require creating several thousands of objects), but existing tests pass and a manual test with ~400k topics succeeds after the fix, while it would hang indefinitely and/or consume all disk space before the fix.
I have reported the initial problem and my findings in https://meta.discourse.org/t/topic-reset-all-highest-exhausts-all-available-disk-space/333837
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.
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.
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 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.
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).
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.
* 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
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.
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.
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>
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.
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.
* 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
`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`.
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.
