When a user chooses to move a topic/message to an existing topic/message, they can now opt to merge the posts chronologically (using a checkbox in the UI).
What is this change required?
I noticed that actions in `SidebarSectionsController` resulted in
lots of N+1 queries problem and I wanted a solution to
prevent such problems without having to write N+1 queries tests. I have
also used strict loading for `SidebarSection` queries in performance
sensitive spots.
Note that in this commit, I have also set `config.active_record.action_on_strict_loading_violation = :log`
for the production environment so that we have more visibility of
potential N+1 queries problem in the logs. In development and test
environment, we're sticking with the default of raising an error.
What is the problem?
In the test environement, we were calling `SiteSetting.setting` directly
to introduce new site settings. However, this leads to changes in state of the SiteSettings
hash that is stored in memory as test runs. Changing or leaking states
when running tests is one of the major contributors of test flakiness.
An example of how this resulted in test flakiness is our `spec/integrity/i18n_spec.rb` spec file which
had a test case that would fail because a new "plugin_setting" site
setting was registered in another test case but the site setting did not
have translations for the site setting set.
What is the fix?
There are a couple of changes being introduced in this commit:
1. Make `SiteSetting.setting` a private method as it is not safe to be
exposed as a public method of the `SiteSetting` class
2. Change test cases to use existing site settings in Discourse instead
of creating custom site settings. Existing site settings are not
removed often so we don't really need to dynamically add new site
settings in test cases. Even if the site settings being used in test
cases are removed, updating the test cases to rely on other site
settings is a very easy change.
3. Set up a plugin instance in the test environment as a "fixture"
instead of having each test create its own plugin instance.
This patch is a followup of
https://github.com/discourse/discourse/pull/21504 where limits on custom
message for an invite were introduced.
This had a side effect of making some existing invites invalid and with
the current code, they can’t be invalidated anymore.
This patch takes the approach of skipping the validations when invites
are invalidated since the important thing here is to mark the invite as
invalidated regardless of its actual state in the DB. (no other
attributes are updated at the same time anyway)
* FIX: Video thumbnails can have duplicates
It's possible that a duplicate video or even a very similar video could
generate the same video thumbnail. Because video thumbnails are mapped
to their corresponding video by using the video sha1 in the thumbnail
filename we need to allow for duplicate thumbnails otherwise even when a
thumbnail has been generated for a topic it will not be mapped
correctly.
This will also allow you to re-upload a video on the same topic to
regenerate the thumbnail.
* fix typo
The welcome topic user tip was for admins only, but in general, user
tips should be used for guiding new users through the features that
Discourse offers. For this reason, we decided to remove the user tip.
This commit also includes a few more copy tweaks to the welcome topic.
The old method updated only existing records, without considering that
new tags might have been created or some tags might not exist anymore.
This was usually not a problem because the stats were also updated by
other code paths.
However, the ensure consistency job should be more solid and help when
other code paths fail or after importing data.
Also, update category tag stats too should happen when updating other
category stats as well.
### What is this change?
The lounge category was replaced with the general category in https://github.com/discourse/discourse/pull/18097.
However, there are still a few references to the lounge category in code. In particular, `Category#seeded?` is erroring out in production looking for `SiteSetting.lounge_category_id`.
In #21498, we split `BaseStore#download` into a "safe" version which returns nil on errors, and an "unsafe" version which raises an exception, which was the old behaviour of `#download`.
This change updates call sites that used the old `#download`, which raised exceptions, to use the new `#download!` to preserve behaviour (and silence deprecation warnings.)
It also silences the deprecation warning in tests.
- Update welcome topic copy
- Edit the welcome topic automatically when the title or description changes
- Remove “Create your Welcome Topic” banner/CTA
- Add "edit welcome topic" user tip
### Background
When SSRF detection fails, the exception bubbles all the way up, causing a log alert. This isn't actionable, and should instead be ignored. The existing `rescue` does already ignore network errors, but fails to account for SSRF exceptions coming from `FinalDestination`.
### What is this change?
This PR does two things.
---
Firstly, it introduces a common root exception class, `FinalDestination::SSRFError` for SSRF errors. This serves two functions: 1) it makes it easier to rescue both errors at once, which is generally what one wants to do and 2) prevents having to dig deep into the class hierarchy for the constant.
This change is fully backwards compatible thanks to how inheritance and exception handling works.
---
Secondly, it rescues this new exception in `UserAvatar.import_url_for_user`, which is causing sporadic errors to be logged in production. After this SSRF errors are handled the same as network errors.
This fixes a bug in the create invite API where if you passed in an
integer for the group_ids field it would fail to add the user to the
specified group.
This commit fixes an issue where the Likes Received notification
count in the user digest email was not affected by the
since/last_seen date for the user, which meant that no matter
how long it had been since the user visited the count was
always constant.
Now instead for the Likes Received count, we only count the
unread notifications of that type since the user was last
seen.
The current limit (250 characters) is too low, as we have some
translations used for our badge descriptions that result in a
description length of 264 characters.
To be on the safe side, the limit is now set to 500 characters.
A category's slug can be encoded when
`SiteSetting.slug_generation_method` has been set to "encoded". As a
result, we have to support non ASCII characters as well.
This amends it so our cached counting reliant specs run in synchronize mode
When running async there are situations where data is left over in the table
after a transactional test. This means that repeat runs of the test suite
fail.
* DEV: move sidebar community section to database
Before, community section was hard-coded. In the future, we are planning to allow admins to edit it. Therefore, it has to be moved to database to `custom_sections` table.
Few steps and simplifications has to be made:
- custom section was hidden behind `enable_custom_sidebar_sections` feature flag. It has to be deleted so all forums, see community section;
- migration to add `section_type` column to sidebar section to show it is a special type;
- migration to add `segment` column to sidebar links to determine if link should be displayed in primary section or in more section;
- simplify more section to have one level only (secondary section links are merged);
- ensure that links like `everything` are correctly tracking state;
- make user an anonymous links position consistence. For example, from now on `faq` link for user and anonymous is visible in more tab;
- delete old community-section template.
Before this fix if the underlying model of a reviewable was changed, the filter wouldn't work anymore as it was expecting a 1:1 relation between filter type and model name.
This commit also relies on the `Reviewable.types` array to check against valid types instead of a regex not checking much.
Finally this commit adds a spec to ensure chat reviewables are listable from the review index page.
Many blog posts use these to illustrate and images were previously omitted
Additionally strip superfluous HTML and BODY tags from embed HTML.
This was incorrectly returned from server.
Group user event webhooks filtered by group fail silently
because the `group_ids` job arg wasn't being passed into the job.
This change add's `group_ids` to the `EmitWebHookEvent` jobs queued for
`user_added_to_group` and `user_removed_from_group` events.
Currently, only user badge grants emit webhook events. This change
extends the `user_badge` webhook to emit user badge revocation events.
A new `user_badge_revoked` event has been introduced instead of relying
on the existing `user_badge_removed` event. `user_badge_removed` emitted
just the `badge_id` and `user_id` which aren't helpful for generating a
meaningful webhook payload for revoked(deleted) user badges.
The new event emits the user badge object.
When revising a post, if the topic that post belonged to did not have a category attached it would error with
> NoMethodError (undefined method `read_restricted' for nil:NilClass)
* FIX: Do not overwrite existing thumbnails
When auto generating video thumbnails they should not overwrite any
existing topic thumbnails.
This also addresses an issue with capitalized file extensions like .MOV
that were being excluded.
* Update app/models/post.rb
Remove comment
Co-authored-by: Penar Musaraj <pmusaraj@gmail.com>
---------
Co-authored-by: Penar Musaraj <pmusaraj@gmail.com>
Responding to negative behaviour tends to solicit more of the same. Common wisdom states: "don't feed the trolls".
This change codifies that advice by introducing a new nudge when hitting the reply button on a flagged post. It will be shown if either the current user, or two other users (configurable via a site setting) have flagged the post.
This PR adds the ability to destroy drafts for a passed user via the API. This was not possible before as this action was reserved for only your personal drafts.
If a user is an admin and calls the `#destroy` action from the API they are able to destroy a draft for a passed user. A user can be targeted by passed either their:
- username
- external_id (for SSO)
to the request.
In the case you attempt to destroy a non-personal draft and
- You are not an admin
- You do not access the `#destroy` action via the API
you will raise a `Discourse::InvalidAccess` (403) and will not succeed in destroying the draft.
This feature will allow sites to define which emoji are not allowed. Emoji in this list should be excluded from the set we show in the core emoji picker used in the composer for posts when emoji are enabled. And they should not be allowed to be chosen to be added to messages or as reactions in chat.
This feature prevents denied emoji from appearing in the following scenarios:
- topic title and page title
- private messages (topic title and body)
- inserting emojis into a chat
- reacting to chat messages
- using the emoji picker (composer, user status etc)
- using search within emoji picker
It also takes into account the various ways that emojis can be accessed, such as:
- emoji autocomplete suggestions
- emoji favourites (auto populates when adding to emoji deny list for example)
- emoji inline translations
- emoji skintones (ie. for certain hand gestures)
Previously, Discourse's password hashing was hard-coded to a specific algorithm and parameters. Any changes to the algorithm or parameters would essentially invalidate all existing user passwords.
This commit introduces a new `password_algorithm` column on the `users` table. This persists the algorithm/parameters which were use to generate the hash for a given user. All existing rows in the users table are assumed to be using Discourse's current algorithm/parameters. With this data stored per-user in the database, we'll be able to keep existing passwords working while adjusting the algorithm/parameters for newly hashed passwords.
Passwords which were hashed with an old algorithm will be automatically re-hashed with the new algorithm when the user next logs in.
Values in the `password_algorithm` column are based on the PHC string format (https://github.com/P-H-C/phc-string-format/blob/master/phc-sf-spec.md). Discourse's existing algorithm is described by the string `$pbkdf2-sha256$i=64000,l=32$`
To introduce a new algorithm and start using it, make sure it's implemented in the `PasswordHasher` library, then update `User::TARGET_PASSWORD_ALGORITHM`.
Previously, public custom sections were only visible to logged-in users. In this PR, we are making them visible to anonymous as well.
The reason is that Community Section will be moved into custom section model to be easily editable by admins.
This commit adds support for filtering for topics in specific
subcategories via the categories filter query language.
For example: `category:documentation:admins` will filter for topics and
subcategory topics in
the category with slug "admins" whose parent category has the slug
"documentation".
The `=` prefix can also be used such that
`=category:documentation:admins` will exclude subcategory topics of the
category with slug "admins" whose parent category has the slug
"documentation".
`Rails.application.routes.recognize_path(value)` was not working for /admin paths because StaffConstraint.new requires user to check permission.
This validation is not bringing much value, and the easiest way is to drop it. In the worse case scenario, a user will have an incorrect link in their sidebar.
Bug reported: https://meta.discourse.org/t/custom-sidebar-sections-being-tested-on-meta/255303/66
There is no need to validate the user's emails when
promoting/demoting their trust level, this can cause
issues in things like Jobs::Tl3Promotions, we don't
need to fail in that case when all we are doing is changing
trust level.
`default_categories_*` site settings will update the category preferences on user creation. But it shouldn't update the user's category preference if a group's setting already updated it for that user.
We perform lookups on sidebar section links based on sidebar_section_id
totally ignoring user. This ensures we have an index to work with.
This removes the previous index `links_user_id_section_id_position` which
partially doubled up `idx_unique_sidebar_section_links`
Before, incorrectly filled fields were marked with red border. Now, additional information under the field is displayed to notify the user what is incorrect.
/t/93696
There are many situations that may cause users to lose permission to
send messages in a chat channel. Until now we have relied on security
checks in `Chat::ChatChannelFetcher` to remove channels which the
user may have a `UserChatChannelMembership` record for but which
they do not have access to.
This commit takes a more proactive approach. Now any of these following
`DiscourseEvent` triggers may cause `UserChatChannelMembership`
records to be deleted:
* `category_updated` - Permissions of the category changed
(i.e. CategoryGroup records changed)
* `user_removed_from_group` - Means the user may not be able to access the
channel based on `GroupUser` or also `chat_allowed_groups`
* `site_setting_changed` - The `chat_allowed_groups` was updated, some
users may no longer be in groups that can access chat.
* `group_destroyed` - Means the user may not be able to access the
channel based on `GroupUser` or also `chat_allowed_groups`
All of these are handled in a distinct service run in a background
job. Users removed are logged via `StaffActionLog` and then we
publish messages on a per-channel basis to users who had their
memberships deleted.
When the user has a channel they are kicked from open, we show
a dialog saying "You no longer have access to this channel".
When they click OK we redirect them either:
* To their first other public channel, if they have any followed
* The chat browse page if they don't
This is to save on tons of requests from kicked out users getting messages
from other channels.
When the user does not have the kicked channel open, we can just
silently yoink it out of their sidebar and turn off subscriptions.
Followup to 184ce647ea,
this just implements Bianca's suggestion on the original
PR and catches the NameError, which was not necessary
before as we were not actually resolving any class from
bookmarkable_type.
Prior to this change `registered_bookmarkable` would return `nil` as `type` in `Bookmark.registered_bookmarkable_from_type(type)` would be `ChatMessage` and we registered a `Chat::Message` class.
This commit will now properly rely on each model `polymorphic_class_for(name)` to help us infer the proper type from a a `bookmarkable_type`.
Tests have also been added to ensure that creating/destroying chat message bookmarks is working correctly.
---
Longer explanation
Currently when you save a bookmark in the database, it's associated to another object through a polymorphic relationship, which will is represented by two columns: `bookmarkable_id` and `bookmarkable_type`. The `bookmarkable_id` contains the id of the relationship (a post ID for example) and the `bookmarkable_type` contains the type of the object as a string by default, (`"Post"` for example).
Chat plugin just started namespacing objects, as a result a model named `ChatMessage` is now named `Chat::Message`, to avoid complex and risky migrations we rely on methods provided by rails to alter the `bookmarkable_type` when we save it: we want to still save it as `"ChatMessage"` and not `"Chat::Message"`. And, to retrieve the correct model when we load the bookmark from the database: we want `"ChatMessage"` to load the `Chat::Message` model and not the `ChatMessage`model which doesn't exist anymore.
On top of this the bookmark codepath is allowing plugins to register types and will check against these types, so we alter this code path to be able to do a similar ChatMessage <-> Chat::Message dance and allow to check the type is valid. In the specific case of this commit, we were retrieving a `"ChatMessage"` bookmarkable_type from the DB and looking for it in the registered bookmarkable types which contain `Chat::Message` and not `ChatMessage`.
This commit main goal was to comply with Zeitwerk and properly rely on autoloading. To achieve this, most resources have been namespaced under the `Chat` module.
- Given all models are now namespaced with `Chat::` and would change the stored types in DB when using polymorphism or STI (single table inheritance), this commit uses various Rails methods to ensure proper class is loaded and the stored name in DB is unchanged, eg: `Chat::Message` model will be stored as `"ChatMessage"`, and `"ChatMessage"` will correctly load `Chat::Message` model.
- Jobs are now using constants only, eg: `Jobs::Chat::Foo` and should only be enqueued this way
Notes:
- This commit also used this opportunity to limit the number of registered css files in plugin.rb
- `discourse_dev` support has been removed within this commit and will be reintroduced later
<!-- NOTE: All pull requests should have tests (rspec in Ruby, qunit in JavaScript). If your code does not include test coverage, please include an explanation of why it was omitted. -->
Using `create_or_find_by!`, followed by `update_all!` requires two or three queries (two when the row doesn't already exist, three when it does). Instead, we can use postgres's native `INSERT ... ON CONFLICT ... DO UPDATE SET` feature to do the logic in a single atomic call.
If you happen to delete the general category before editing the welcome
topic, the banner will still display. This fix adds a after destroy hook
that will clear the entries for the welcome topic banner in the redis
cache.
Currently the auto-bump cooldown is hard-coded to 24 hours.
This change makes the highlighted 24 hours part configurable (defaulting to 24 hours), and the rest of the process remains the same.
This uses the new CategorySetting model associated with Category. We decided to add this because we want to move away from custom fields due to the lack of type casting and validations, but we want to keep the loading of these optional as they are not needed for almost all of the flows.
Category settings will be back-filled to all categories as part of this change, and creating a new category will now also create a category setting.
* FEATURE: Generate thumbnail images for uploaded videos
Topics in Discourse have a topic thumbnail feature which allows themes
to show a preview image before viewing the actual Topic.
This PR allows for the ability to generate a thumbnail image from an
uploaded video that can be use for the topic preview.
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.
That column is obsolete since we added the `granted_title_badge_id` column in 2019 (56d3e29a69). Having both columns can lead to inconsistencies (mostly due to old data from before 2019).
For example, `BadgeGranter.revoke_ungranted_titles!` doesn't work correctly if `badge_granted_title` is `false` while `granted_title_badge_id` points to the badge that is used as title.
Similar spirit to e195e6f614,
this moves the Bookmarkable registration to DiscoursePluginRegistry
so plugins which are not enabled do not register additional
bookmarkable classes.
What is the problem?
We have a hidden site setting `show_category_definitions_in_topic_lists`
which is set to false by default. What this means is that category
definition topics are not shown in the topic list by default. Only the
category definition topic for the category being viewed will be shown.
However, we have a bug where we would show that a category has new
topics when a new child category along with its category definition
topic is created even though the topic list does not list the child
category's category definition topic.
What is the fix here?
This commit fixes the problem by shipping down an additional
`is_category_topic` attribute in `TopicTrackingStateItemSerializer` when
the `show_category_definitions_in_topic_lists` site setting has been set
to false. With the new attribute, we can then exclude counting child
categories' category definition topics when counting new and unread
counts for a category.
* 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>
Fixes a small issue where allowed user removes themselves from a private message before the post activity (small action) is created.
I also added some test coverage to prevent regression.
/t/92811
Follow up to 098ab29d41. Since
we just used a `cattr_reader` on `About` this was not safe
for multisite, since some sites could have the chat plugin
enabled and some may not. Using `DiscoursePluginRegistry` gets
around this issue, and makes it so the chat stats only show
for a site if `chat_enabled` is true.
We currently apply type: :link watched words to custom user fields. This makes the user card pretty ugly because we don't allow html / links there. Additionally, the admin UI also does not say that we apply this to custom user fields, but only words in posts.
So this PR is to remove the replacement of link-type watch words for custom user fields.
This commit allows the user to set their preference vis-a-vis
the chat icon in the header of the page. There are three options:
- All New (default) - This maintains the existing behaviour where
all new messages in the channel show a blue dot on the icon
- Direct Messages and Mentions - Only show the green dot on the
icon when you are directly messaged or mentioned, the blue dot
is never shown
- Never - Never show any dot on the chat icon, for those who
want tractor-beam-laser-focus
