SearchIndexer is only automatically disabled in `before_all` and `before` blocks which means at the start
of test runs. Enabling the SearchIndexer in one `fab!` block will affect
all other `fab!` blocks which is not ideal as we may be indexing stuff
for search when we don't need to.
`TopicQuery#latest_results` which was being used by
`TopicQuery#list_filter` defaults to ordering by `Topic#bumped_at` in
descending order and that was taking precedent over the order scopes
being applied by `TopicsFilter`.
The value field of ThemeField is only used when viewing a diff in the staff action logs and local theme editing. value is being serialized into the theme index as well, which is not used. It's a huge amount of JSON that we can cut by removing it.
This also breaks up the various theme serializers into separate classes so they autoload properly (or at least restart the server on edit)
We are seeing issues with the composer not being able to close due to the addition of a error message when rescuing from `Draft::OutOfSequence`. This PR will revert to the original solution implemented prior to https://github.com/discourse/discourse/pull/21148 that just silently rescues from `Draft::OutOfSequence`
This PR adds the ability to destroy reviewables for a passed user via the API. This was not possible before as this action was reserved for reviewables for you created only.
If a user is an admin and calls the `#destroy` action from the API they are able to destroy a reviewable 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 reviewable 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 reviewable.
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.
* FEATURE: add a setting to allowlist DiscourseConnect return path domains
This commit adds a site setting to allowlist DiscourseConnect return
path domains. The setting needs supports exact domain or wildcard
character (*) to allow for any domain as return path.
* Add more specs to clarify what is allowed in site setting
* Update setting description to explain what is allowed
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)
This commit adds support for the `created-by:<username>` query filter
which will return topics created by the specified user. Multiple
usernames can be specified by comma seperating the usernames like so:
`created-by:username1,username2`. This will filter for topics created by
either of the specified users. Multiple `created-by:<username>` can also
be composed together. `created-by:username1 created-by:username2` is
equivalent to `created-by:username1,username2`.
This commit adds support for the `in:<topic notification level>` query
filter. As an example, `in:tracking` will filter for topics that the
user is watching. Filtering for multiple topic notification levels can
be done by comma separating the topic notification level keys. For
example, `in:muted,tracking` or `in:muted,tracking,watching`.
Alternatively, the user can also compose multiple filters with `in:muted
in:tracking` which translates to the same behaviour as
`in:muted,tracking`.
This commit adds some more detailed information about how to actually
get the number of likes for a post.
Also refactors some requests and responses into json schema files to
clean up the specs a bit.
See https://meta.discourse.org/t/69017/4?u=blake
topic.url is a dangerous method to use, it is incompatible with our `sign_in`
helper.
This removes a bunch of places this was erroneously used and helps to avoid
future cargo culting.
The following are the changes being introduced in this commit:
1. Instead of mapping the query language to various query params on the
client side, we've decided that the benefits of having a more robust
query language far outweighs the benefits of having a more human readable query params in the URL.
As such, the `/filter` route will just accept a single `q` query param
and the query string will be parsed on the server side.
1. On the `/filter` route, the tags filtering query language is now
supported in the input per the example provided below:
```
tags:bug+feature tagged both bug and feature
tags:bug,feature tagged either bug or feature
-tags:bug+feature excluding topics tagged bug and feature
-tags:bug,feature excluding topics tagged bug or feature
```
The `tags` filter can also be specified multiple
times in the query string like so `tags:bug tags:feature` which will
filter topics that contain both the `bug` tag and `feature` tag. More
complex query like `tags:bug+feature -tags:experimental` will also work.
Our schema allows `category.topic_id` to be NULL. Null values shouldn't actually happen in production, but it is very common in tests because `Fabricate(:category)` skips creating the definition topic to improve performance. Before this commit, a NULL category.topic_id would cause all subcategory topics to be excluded from a TopicQuery result. This is because, in postgres, `NULL <> anything` is falsy. Instead, we can use `IS DISTINCT FROM`, which will return true when NULL is compared to a non-NULL value.
Why is this change required?
Prior to this change, we would list all group messages that a user
has access to in the user menu messages notifications panel dropdown.
However, this did not respect the topic's notification level setting and
group messages which the user has set to 'normal' notification level were
being displayed
What does this commit do?
With this commit, we no longer display all group messages that a user
has access to. Instead, we only display group messages that a user is
watching in the user menu messages notifications panel dropdown.
Internal Ref: /t/94392
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.
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.
Way back when this was introduced way back in b96c10a903
I didn't have any frame of reference for what these max rate
limit numbers should be, so 10 seemed like a reasonable limit
until a real world case where this did not make sense came
along.
The time has come.
Moving these into site settings, which are hidden since in most
cases there is no need to change these.
Similar spirit to e195e6f614,
this moves the Bookmarkable registration to DiscoursePluginRegistry
so plugins which are not enabled do not register additional
bookmarkable classes.
* 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>
This commit e62e93f83a slowed
down the spec, making it take ~4m in CI and a long time
locally.
This commit fixes the issue by just not doing the work in
the before block, it was not necessary, all we really need
to do is make sure everyone can PM.
54.37s -> 22.98s locally
This commit implements many changes to topic and comments embedding. It
deprecates the class_name field from EmbeddableHost and suggests using
the className parameter. discourse_username parameter has been
deprecated and it will fetch it from embedded site from the author or
discourse-username meta.
See the updated code sample from Admin > Customize > Embedding page.
* FEATURE: Add className parameter for Discourse embed
* DEV: Hide class_name from EmbeddableHost
* DEV: Deprecate class_name field of EmbeddableHost
* FEATURE: Use either author or discourse-username meta tag
* DEV: Deprecate discourse_username parameter
* DEV: Improve embed code sample
* FIX: Use pluralized string
* REFACTOR: Fix misuse of pluralized string
* REFACTOR: Fix misuse of pluralized string
* DEV: Remove linting of `one` key in MessageFormat string, it doesn't work
* REFACTOR: Fix misuse of pluralized string
This also ensures that the URL works on subfolder and shows the site setting link only for admins instead of staff. The string is quite complicated, so the best option was to switch to MessageFormat.
* REFACTOR: Fix misuse of pluralized string
* FIX: Use pluralized string
This also ensures that the URL works on subfolder and shows the site setting link only for admins instead of staff.
* REFACTOR: Correctly pluralize reaction tooltips in chat
This also ensures that maximum 5 usernames are shown and fixes the number of "others" which was off by 1 if the current user reacted on a message.
* REFACTOR: Use translatable string as comma separator
* DEV: Add comment to translation to clarify the meaning of `%{identifier}`
* REFACTOR: Use translatable comma separator and use explicit interpolation keys
* REFACTOR: Don't interpolate lowercase channel status
* REFACTOR: Fix misuse of pluralized string
* REFACTOR: Don't interpolate channel status
* REFACTOR: Use %{count} interpolation key
* REFACTOR: Fix misuse of pluralized string
* REFACTOR: Correctly pluralize DM chat channel titles
Currently, clicking on the unsubscribe link with a `key` associated with a
deleted topic results in an HTTP 500 response.
This change fixes that by skipping any attempt to run topic related flow if topic
isn't present.
Previously, a user avatar redirect had a lifetime of 24h. That means that a change to the S3 CDN URL would take up to 24h to propagate to clients and intermediate CDNs.
This commit reduces the max age to 1 hour, but also introduces a `stale-while-revalidate` directive. This allows clients and CDNs to use a 'stale' value if it was received between 1h and 24h ago, as long as they make a background request to update the cache. This should reduce the impact of S3 URL changes. 1 hour after the change, the CDN will start serving updated values. Plus, if users have cached bad responses, their browser will automatically fetch the correct version and use it on the next page load.
When a post is created using the API and goes into the review queue, we
would return a 'null' string in the response which isn't valid JSON.
Internal ref: /t/92419
Co-authored-by: Leonardo Mosquera <ldmosquera@gmail.com>
Improvements for this PR: https://github.com/discourse/discourse/pull/20057
What was fixed:
- [x] Use ember transitions instead of full reload
- [x] Link was inaccurately kept active
- [x] "+ save" renamed to just "save"
- [x] Render emojis in link name
- [x] UI to set icon
- [x] Delete link is trash icon instead of "x"
- [x] Add another link to on the left and rewording
- [x] Raname "link name" -> "name", "points to" -> link
- [x] Add limits to fields
- [x] Move add section button to the bottom
* FIX: Remove action buttons if post has already been reviewed
* Change the approve to reject test to expect an error
* Adds a controller spec to ensure you can't edit a non-pending review item
* Remove unnessary conditional
This commits adds the ability to add a header to the embedded comments
view. One use case for this is to allow `postMessage` communication
between the comments iframe and the parent frame, for example, when
toggling the theme of the parent webpage.
The algorithm failed to find the correct category by slug when there are multiple sub-sub-categories with the same child-category name and the first child doesn't have the correct grandchild.
So, searching for "child / grandchild" worked in the following case, it found (3):
- (1) parent 1
- (2) child
- (3) grandchild
- (4) parent 2
- (5) child
- (6) grandchild
But it failed to find the grandchild in the following case:
- (1) parent 1
- (2) child
- (4) parent 2
- (5) child
- (6) grandchild
And this also fixes a flaky spec by forcing categories to always order by by `parent_category_id` and `id`.
This makes it possible to partly revert 60990aab55
Allows users to configure their own custom sidebar sections with links withing Discourse instance. Links can be passed as relative path, for example "/tags" or full URL.
Only path is saved in DB, so when Discourse domain is changed, links will be still valid.
Feature is hidden behind SiteSetting.enable_custom_sidebar_sections. This hidden setting determines the group which members have access to this new feature.
1. What is the problem here?
When a user's reviewables count changes, the changes are published via
MessageBus in a background Sidekiq job which means there is a delay before the
client receives the MessageBus message with the updated count. During
the time the reviewables count for a user has been updated and the time
when the client receives the MessageBus message with the updated count,
a user may view the reviewables list in the user menu. When that happens, the number of
reviewables in the list may be out of sync with the count shown.
2. What is the fix?
Going forward, the response for the `ReviewablesController#user_menu_list` action will include the user's reviewables count as
the `reviewables_count` attribute. This is then used by the client side
to update the user's reviewables count to ensure that the reviewables
list and count are kept in sync.
When we introduce new color scheme colors, they are not immediately persisted to the database for all color schemes. Previously, this meant that they would be unavailable in the admin UI for editing. The only way to work with the new colors was to create a new color scheme.
This commit updates the serializer so that all colors are serialized, even if they are not yet persisted to the database for the current scheme. This means that they now show up in the admin UI and can be edited.
Currently `Topic#pm_topic_count` is a count of all personal messages tagged for a given tag. As a result, any user with access to PM tags can poll a sensitive tag to determine if a new personal message has been created using that tag even if the user does not have access to the personal message. We classify this as a minor leak in sensitive information.
With this commit, `Topic#pm_topic_count` is hidden from users by default unless the `display_personal_messages_tag_counts` site setting is enabled.
In some situations, these HTTP calls would cause some cache to warmup and send a `/distributed_hash` message-bus message. We can avoid tracking those by passing a specific channel name to `track_publish`.
This commits adds a database migration to limit the user status to 100
characters, limits the user status in the UI and makes sure that the
emoji is valid.
Follow up to commit b6f75e231c.
When the `tags_listed_by_group` site setting is disable, we were seeing
the N+1 queries problem when multiple `CategoryTag` records are listed.
This commit fixes that by ensuring that we are not filtering through the
category `tags` association after the association has been eager loaded.
This assertion was failing in internal builds. I can repro locally if I
set `foobarbaz` to be created after `quxbarbaz`.
For now, I think this complication in the test is unnecessary, hence this
removes the `quxbarbaz` case.
When creating a group membership request, there is no character
limit on the 'reason' field. This can be potentially be used by
an attacker to create enormous amount of data in the database.
Co-authored-by: Ted Johansson <ted@discourse.org>
This is just cleaning up a TODO I had to add more specs
to this controller -- there are more thorough tests on the
actual HashtagService class and the type-specific hashtag
classes.
This commit allows us to set the channel slug when creating new chat
channels. As well as this, it introduces a new `SlugsController` which can
generate a slug using `Slug.for` and a name string for input. We call this
after the user finishes typing the channel name (debounced) and fill in
the autogenerated slug in the background, and update the slug input
placeholder.
This autogenerated slug is used by default, but if the user writes anything
else in the input it will be used instead.
Currently, `Tag#topic_count` is a count of all regular topics regardless of whether the topic is in a read restricted category or not. As a result, any users can technically poll a sensitive tag to determine if a new topic is created in a category which the user has not excess to. We classify this as a minor leak in sensitive information.
The following changes are introduced in this commit:
1. Introduce `Tag#public_topic_count` which only count topics which have been tagged with a given tag in public categories.
2. Rename `Tag#topic_count` to `Tag#staff_topic_count` which counts the same way as `Tag#topic_count`. In other words, it counts all topics tagged with a given tag regardless of the category the topic is in. The rename is also done so that we indicate that this column contains sensitive information.
3. Change all previous spots which relied on `Topic#topic_count` to rely on `Tag.topic_column_count(guardian)` which will return the right "topic count" column to use based on the current scope.
4. Introduce `SiteSetting.include_secure_categories_in_tag_counts` site setting to allow site administrators to always display the tag topics count using `Tag#staff_topic_count` instead.
Prior to this change, we were parsing `Post#cooked` every time we
serialize a post to extract the usernames of mentioned users in the
post. However, the only reason we have to do this is to support
displaying a user's status beside each mention in a post on the client side when
the `enable_user_status` site setting is enabled. When
`enable_user_status` is disabled, we should avoid having to parse
`Post#cooked` since there is no point in doing so.
Our JS files reference sourcemaps relative to their current path. On sites with non-S3 CDN setups, we use a special path for brotli assets (39a524aa). This caused the sourcemap requests to 404.
This commit fixes the issue by allowing the `.map` files to be accessed under `/brotli_asset/*`.
When the `tags_listed_by_group` site setting is enabled, we were seeing
the N+1 queries problem when multiple `TagGroup` records are listed.
This commit fixes that by ensuring that we are not filtering through the
`tags` association after the association has been eager loaded.
Added in c2013865d7,
this migration was supposed to only turn off the hashtag
setting for existing sites (since that was the old default)
but its doing it for new ones too because we run all migrations
on new sites.
Instead, we should only run this if the first migration was
only just created, meaning its a new site.
In "GlobalSetting.redirect_avatar_requests" mode, when the application gets
an avatar request it returns a "redirect" to the S3 CDN.
This shields the application from caching avatars and downloading from S3.
However clients will make 2 requests per avatar. (one to get redirect,
second to get avatar)
A one hour cache on a redirect means there may be an increase in CDN
traffic, given more clients will ask for the redirect every hour.
This may also lead to an increase in origin requests to the application.
To mitigate lets cache the CDN URL for 1 day.
The downside is that any changes to S3 CDN need extra care to allow for
the extra 1 day delay. (leave data around for 1 extra day)
If a secure upload's access_control_post was trashed, and an anon user
tried to look at that upload, they would get a 500 error rather than
the correct 403 because of an error inside the PostGuardian logic.