This commit adds support for an optional `prompt` parameter in the
payload of the /session/sso_provider endpoint. If an SSO Consumer
adds a `prompt=none` parameter to the encoded/signed `sso` payload,
then Discourse will avoid trying to login a not-logged-in user:
* If the user is already logged in, Discourse will immediately
redirect back to the Consumer with the user's credentials in a
signed payload, as usual.
* If the user is not logged in, Discourse will immediately redirect
back to the Consumer with a signed payload bearing the parameter
`failed=true`.
This allows the SSO Consumer to simply test whether or not a user is
logged in, without forcing the user to try to log in. This is useful
when the SSO Consumer allows both anonymous and authenticated access.
(E.g., users that are already logged-in to Discourse can be seamlessly
logged-in to the Consumer site, and anonymous users can remain
anonymous until they explicitly ask to log in.)
This feature is similar to the `prompt=none` functionality in an
OpenID Connect Authentication Request; see
https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest
Some sites have a large number of categories and fetching the category
IDs or category topic IDs just to build another query can take a long
time or resources (i.e. memory).
* FIX: Return 403 instead of redirect on username routes when hidding profiles
* Updated raised error to better reflect the problem to the user
* implemented suggested changes
Previously, a "`some_not_allowed`" warning message was appeared in composer even when all the users mentioned via group are already invited to the private message directly or via other groups.
The hidden site setting max_drafts_per_user defaults to 10_000 drafts per user.
The longest key should be "topic_<MAX_BIG_INT>" which is 25 characters.
In #20135 we prevented invalid inputs from being accepted in category setting form fields on the front-end. We didn't do anything on the back-end at that time, because we were still discussing which path we wanted to take. Eventually we decided we want to move this to a new CategorySetting model.
This PR moves the require_topic_approval and require_reply_approval from custom fields to the new CategorySetting model.
This PR is nearly identical to #20580, which migrated num_auto_bump_daily, but since these are slightly more sensitive, they are moved after the previous one is verified.
Previous to this change when both `normalize_emails` and `hide_email_address_taken`
is enabled the expected `account_exists` email was only sent on exact email
matches.
This expands it so it also sends an email to the canonical email owner.
Why this change?
Currently, we do not have an easy way to test themes and theme components
using Rails system tests. While we support QUnit acceptance tests for
themes and theme components, QUnit acceptance tests stubs out the server
and setting up the fixtures for server responses is difficult and can lead to a
frustrating experience. System tests on the other hand allow authors to
set up the test fixtures using our fabricator system which is much
easier to use.
What does this change do?
In order for us to allow authors to run system tests with their themes
installed, we are adding a `upload_theme` helper that is made available
when writing system tests. The `upload_theme` helper requires a single
`directory` parameter where `directory` is the directory of the theme
locally and returns a `Theme` record.
Until now, we have allowed testing themes in production environments via `/theme-qunit`. This was made possible by hacking the ember-cli build so that it would create the `tests.js` bundle in production. However, this is fundamentally problematic because a number of test-specific things are still optimized out of the Ember build in production mode. It also makes asset compilation significantly slower, and makes it more difficult for us to update our build pipeline (e.g. to introduce Embroider).
This commit removes the ability to run qunit tests in production builds of the JS app when the Embdroider flag is enabled. If a production instance of Discourse exists exclusively for the development of themes (e.g. discourse.theme-creator.io) then they can add `EMBER_ENV: development` to their `app.yml` file. This will build the entire app in development mode, and has a significant performance impact. This must not be used for real production sites.
This commit also refactors many of the request specs into system specs. This means that the tests are guaranteed to have Ember assets built, and is also a better end-to-end test than simply checking for the presence of certain `<script>` tags in the HTML.
We're seeing a large number of log noise from this endpoint due to malicious scanners that are trying to send clever params and seeing if they can break something.
This change simply rescues any NoMethodError during parameter parsing and re-raises a Discourse::InvalidParameters exception, which will be caught and render a 400.
They're both constant per-instance values, there is no need to store them
in the session. This also makes the code a bit more readable by moving
the `session_challenge_key` method up to the `DiscourseWebauthn` module.
Why this change?
As part of our ongoing efforts to security harden the Discourse
application, we are adding the `cross_origin_opener_policy_header` site setting
which allows the `Cross-Origin-Opener-Policy` response header to be set on requests
that preloads the Discourse application. In more technical terms, only
GET requests that are not json or xhr will have the response header set.
The `cross_origin_opener_policy_header` site setting is hidden for now
for testing purposes and will either be released as a public site
setting or be remove if we decide to be opinionated and ship a default
for the `Cross-Origin-Opener-Policy` response header.
Streaming doesn't work for anonymous users because we scope updates to the current user. Since they can only see cached summaries, we can skip the streaming parameter and update it directly with the ajax response.
`ReviewableQueuedPost` got refactored a while back to use the more
appropriate `target_created_by` for the user of the post being queued
instead of `created_by`. The change was not extended to the `DELETE
/review/:id` endpoint leading to error responses for a user attempting
to deleting their own queued post.
This fix extends the `Reviewable` lookup implementation in
`ReviewablesController#destroy` and Guardian implementation to account
for this change.
When we receive the stream parameter, we'll queue a job that periodically publishes partial updates, and after the summarization finishes, a final one with the completed version, plus metadata.
`summary-box` listens to these updates via MessageBus, and updates state accordingly.
This commit removes any logic in the app and in specs around
enable_experimental_hashtag_autocomplete and deletes some
old category hashtag code that is no longer necessary.
It also adds a `slug_ref` category instance method, which
will generate a reference like `parent:child` for a category,
with an optional depth, which hashtags use. Also refactors
PostRevisor which was using CategoryHashtagDataSource directly
which is a no-no.
Deletes the old hashtag markdown rule as well.
In #20135 we prevented invalid inputs from being accepted in category setting form fields on the front-end. We didn't do anything on the back-end at that time, because we were still discussing which path we wanted to take. Eventually we decided we want to move this to a new CategorySetting model.
This PR moves the num_auto_bump_daily from custom fields to the new CategorySetting model.
In addition it sets the default value to 0, which exhibits the same behaviour as when the value is NULL.
This is a similar fix to 32d4810e2b
Why this change?
Prior to this change, there is a bug in `TopicsController#bulk`
where it does not dismiss new unred posts in sub-subcategories when the
`category_id` and `include_subcategories=true` params are present. This
is because the controller did not account for sub-subcategories when
fetching the category ids of the new topics that should be dismissed.
This commit fixes the problem by relying on the `Category.subcategory_ids` class
method which accounts for sub-subcategories.
What is the problem here?
In multiple controllers, we are accepting a `limit` params but do not
impose any upper bound on the values being accepted. Without an upper
bound, we may be allowing arbituary users from generating DB queries
which may end up exhausing the resources on the server.
What is the fix here?
A new `fetch_limit_from_params` helper method is introduced in
`ApplicationController` that can be used by controller actions to safely
get the limit from the params as a default limit and maximum limit has
to be set. When an invalid limit params is encountered, the server will
respond with the 400 response code.
What is the context for this change?
Prior to this change, there is a bug in `TopicsController#reset_new`
where it does not dismiss new topics in sub-subcategories when the
`category_id` and `include_subcategories=true` params are present. This
is because the controller did not account for sub-subcategories when
fetching the category ids of the new topics that should be dismissed.
This commit fixes the problem by relying on the `Category.subcategory_ids` class
method which accounts for sub-subcategories.
Recently we started giving admins a notice in the advice panel when their translations have become outdated due to changes in core. However, we didn't include any additional information.
This PR adds more information about the outdated translation inside the site text edit page, together with an option to dismiss the warning.
We have a number of raw comments indicating that certain methods and classes are deprecated and marked for removal. This change turn those comments into deprecation warnings so that we can 1) see them in the logs of our own hosting and 2) give some warning to self hosters.
Why this change?
Prior to this change, we would only return tags that are used in at
least one public topic. However, this is confusing for users because the
tag could be used in a restricted category and that is not considered a
"public" topic. Instead, we will just display all the tags in the edit
tags navigation modal as long as it is visible to the user.
Why this change?
Prior to this change, dismissing unreads posts did not publish the
changes across clients for the same user. As a result, users can end up
seeing an unread count being present but saw no topics being loaded when
visiting the `/unread` route.
* FEATURE: Inline topic summary. Cached version accessible to everyone.
Anons and non-members of the `custom_summarization_allowed_groups_map` groups can see cached summaries for any accessible topic. After the first 12 hours and if the posts to summarize have changed, allowed users clicking on the button will automatically re-generate it.
* Ensure chat summaries work and prevent model hallucinations when there are no messages.
This PR adds a feature to help admins stay up-to-date with their translations. We already have protections preventing admins from problems when they update their overrides. This change adds some protection in the other direction (where translations change in core due to an upgrade) by creating a notice for admins when defaults have changed.
Terms:
- In the case where Discourse core changes the default translation, the translation override is considered "outdated".
- In the case above where interpolation keys were changed from the ones the override is using, it is considered "invalid".
- If none of the above applies, the override is considered "up to date".
How does it work?
There are a few pieces that makes this work:
- When an admin creates or updates a translation override, we store the original translation at the time of write. (This is used to detect changes later on.)
- There is a background job that runs once every day and checks for outdated and invalid overrides, and marks them as such.
- When there are any outdated or invalid overrides, a notice is shown in admin dashboard with a link to the text customization page.
Known limitations
The link from the dashboard links to the default locale text customization page. Given there might be invalid overrides in multiple languages, I'm not sure what we could do here. Consideration for future improvement.
Some plugins call require_plugin with a wrong argument instead of the
plugin name. In a production environment that used to be a warning, but
there is no reason to keep it like that in a testing environment
because the issue will continue to be ignored.
Follow-up to b27e12445d
This commit adds 2 new site settings `default_sidebar_link_to_filtered_list` and `default_sidebar_show_count_of_new_items` to control the default values for the navigation menu preferences that were added in the linked commit (`sidebar_link_to_filtered_list` and `sidebar_show_count_of_new_items` respectively).
What is the problem?
Before this change, we were relying on the `/tags` endpoint which
returned all the tags that are visible to a give user on the site leading to potential performance problems.
The attribute keys of the response also changes based on the `tags_listed_by_group` site setting.
What is the fix?
This commit fixes the problems listed above by creating a dedicate `#list` action in the
`TagsController` to handle the listing of the tags in the edit
navigation menu tags modal. This is because the `TagsController#index`
action was created specifically for the `/tags` route and the response
body does not really map well to what we need. The `TagsController#list`
action added here is also much safer since the response is paginated and
we avoid loading a whole bunch of tags upfront.
]When changing fonts in the `/wizard/steps/styling` step of
the wizard, users would not see the font loaded straight away,
having to switch to another one then back to the original to
see the result. This is because we are using canvas to render
the style preview and this fails with a Chrome-based intervention
when font loading is taking too long:
> [Intervention] Slow network is detected. See
https://www.chromestatus.com/feature/5636954674692096 for more details.
Fallback font will be used while loading:
https://sea2.discourse-cdn.com/business7/fonts/Roboto-Bold.ttf?v=0.0.9
We can get around this by manually loading the fonts selected using
the FontFace JS API when the user selects them and before rerendering
the canvas. This just requires preloading more information about the
fonts if the user is admin so the wizard can query this data.
This is the first of a number of PRs aimed at helping admins manage their translation overrides. It simply adds a list of available interpolation keys below the input field when editing an override.
It also includes custom interpolation key.
Updates the interface for implementing summarization strategies and adds a cache layer to summarize topics once.
The cache stores the final summary and each chunk used to build it, which will be useful when we have to extend or rebuild it.
Communities can use sidebar or header dropdown, therefore navigation menu is a better name settings in 2 places:
- Old user sidebar preferences;
- Site setting about default tags and categories.
* FEATURE: Content custom summarization strategies.
This PR establishes a pattern for plugins to register alternative ways of summarizing content by extending a class that defines an interface.
Core controls which strategy we'll use and who has access to it through the `summarization_strategy` and `custom_summarization_allowed_groups`. It also defines the UI for summarizing topics.
Other plugins can access this summarization mechanism and implement their features, removing cross-plugin customizations, as it currently happens between chat and the discourse-ai plugin.
* Group membership validation and rate limiting
* Work with objects instead of classes
* Port summarization feature from discourse-ai to chat
* Rename available summaries to 'Top Replies' and 'Summary'
1. ember proxy stuff still isn't in a great shape, live-reload doesn't work yet, uploads made w/o subfolder won't work, custom fonts don't work, service worker doesn't work. But otherwise it's fine :P
2. I don't know why `HTTP_IF_MODIFIED_SINCE` can be an empty string. Don't have time to investigate, and fast_blank makes this fix an easy solution ;)
* DEV: Implement staff logs for user columns edits
* deleted extra space in staff logger detail string, deleted string when no changes are made, added basic test coverage for EditDirectoryColumnsController
* fixed change made to #self.staff_actions un UserHistory
* implemented a method that builds the details, previous_values and new_values in a dynamic way
* removed details of changes
* refactored small merge
Calling pluck is instantly making a SELECT, while passing the relationship allows rails to build a correct query.
Before (2 selects):
```
pry(main)> Post.where(topic_id: Topic.where(id: [1,3,4]).pluck(:id)).count
(1.3ms) SELECT "topics"."id" FROM "topics" WHERE "topics"."deleted_at" IS NULL AND "topics"."id" IN (1, 3, 4)
Post Count (0.5ms) SELECT COUNT(*) FROM "posts" WHERE "posts"."deleted_at" IS NULL AND "posts"."topic_id" IN (1, 3, 4)
```
After (1 select):
```
pry(main)> Post.where(topic_id: Topic.where(id: [1,3,4])).count
Post Count (2.7ms) SELECT COUNT(*) FROM "posts" WHERE "posts"."deleted_at" IS NULL AND "posts"."topic_id" IN (SELECT "topics"."id" FROM "topics" WHERE "topics"."deleted_at" IS NULL AND "topics"."id" IN (1, 3, 4))
```
Display modal for combined new and unread view with options:
- [x] Dismiss new topics
- [x] Dismiss new posts
- [ ] Stop tracking these topics so they stop appearing in my new list
What is the problem?
The user messages routes are currently routed by the server to
`UserActionsController#private_messages`. However, the method is
essentially a no-op and does not do any preloading. As a result, when we
load the user private messages routes, the client ends up having to
issue another request to the server to get more information about the
user profile currently being viewed. This extra request is triggered by
the `user` model's `findDetails` method that is called from the `user`
route in the `afterModel` hook.
What is the solution?
The `user` model's `findDetails` method actually checks the preload
store to see if the `user_${username}` key is present in the store and
if it is, it will use the preloaded data instead of triggering another
request. Since the user private messages routes are nested under the
user route on the client side, we have to rely on the
`UsersController#show` controller action on the server side for the user private
messages route as the `UsersController#show` controller action preloads
the required user information for the client side.
Prior to this commit, we didn't have RTL versions of our admin and plugins CSS bundles and we always served LTR versions of those bundles even when users used an RTL locale, causing admin and plugins UI elements to never look as good as when an LTR locale was used. Example of UI issues prior to this commit were: missing margins, borders on the wrong side and buttons too close to each other etc.
This commit creates an RTL version for the admin CSS bundle as well as RTL bundles for all the installed plugins and serves those RTL bundles to users/sites who use RTL locales.
This commit adds modifiers that allow plugins to change how categories and groups are prefetched into the application and listed in the respective controllers.
Possible use cases:
- prevent some categories/groups from being prefetched when the application loads for performance reasons.
- prevent some categories/groups from being listed in their respective index pages.
Allow admins to edit Community section. This includes drag and drop reorder, change names, delete and reset to default.
Visual improvements introduced in edit community section modal are available in edit custom section form as well. For example:
- drag and drop links to change their position;
- smaller icon picker.
This commit introduces a new `within_user_updater_transaction` event that's triggered inside the transaction that saves user updates in `UserUpdater`. Plugins can hook into the transaction using the event to include custom changes in the transaction. Callbacks for this event receive 2 arguments:
1. the user being saved
2. the changed attributes that are passed to `UserUpdater`.
There's also new modifier in this commit called `users_controller_update_user_params` to allow plugins to allowlist custom params in the `UsersController` which eventually end up getting passed as attributes to the `UserUpdater` and the new `within_user_updater_transaction` event where they can be used to perform additional updates using the custom params.
-----
New API is used in https://github.com/discourse/discourse-mailinglist-integration/pull/1.
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).
The field more_topic_url is already included in the response preloaded in categories#index
However this field was missing if a request was subsequently made to update the page using
the end-points /categories_and_latest or /categories_and_top. This could lead the client
app to display incorrect information if it relied on this information to update the UI.
This commit deprecates the `modify_user_params` method in `UsersController` in favor of a new modifier that replaces that method whose entire purpose is to allow plugins to monkey-patch it to permit custom params in the controller. We now have the "modifier" system which can achieve the same results but in a safer and easier way. The modifier that replaces the deprecated method is included in PR https://github.com/discourse/discourse/pull/21737.
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.
When uploading images via direct to S3 upload, we were
assuming that we could not pre-emptively check the file
size because the client may do preprocessing to reduce
the size, and UploadCreator could also further reduce the
size.
This, however, is not true of gifs, so we would have an
issue where you upload a gif > the max_image_size_kb
setting and had to wait until the upload completed for
this error to show.
Now, instead, when we direct upload gifs to S3, we check
the size straight away and present a file size error to
the user rather than making them wait. This will increase
meme efficiency by approximately 1000%.
What is the problem?
In `SvgSpriteController#search` and `SvgSpriteController#icon_picker_search`, the controller actions
was using the `RailsMultisite::ConnectionManagement.with_hostname` API
but `params[:hostname]` was always `nil` because the routes does not
have a `:hostname` param component and the client does not ever pass the
`:hostname` param when making the request. When `RailsMultisite::ConnectionManagement.with_hostname` is
used with a `nil` argument, it ends up connecting to the default
multisite database. Usually this would be bad because we're allowing a
site in a multisite setup to connect to another site but thankfully no
private data is being leaked here.
What is the fix?
Since `SvgSpriteController#search` and `SvgSpriteController#icon_picker_search` are login required route,
there is no need for us to switch database connections. The fix here is
to simply remove the use of `RailsMultisite::ConnectionManagement.with_hostname`.
### What is the problem?
It is possible to pass an arbitrary value to the limit parameter in `TagsController#search`, and have it flow through `DiscourseTagging.filter_allowed_tags` where it will raise an error deep in the database driver. MiniSql ensures there's no injection happening, but that ultimately results in an invalid query.
### How does this fix it?
This change checks more strictly that the parameter can be cleanly converted to an integer by replacing the loose `#to_i` conversion semantics with the stronger `Kernel#Integer` ones.
**Example:**
```ruby
"1; SELECT 1".to_i
#=> 1
Integer("1; SELECT 1")
#=> ArgumentError
```
As part of the change, I also went ahead to disallow a limit of "0", as that doesn't seem to be a useful option. Previously only negative limits were disallowed.
* 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.
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
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.
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.
This corrects two issues:
1. We were double serializing topic tracking state (as_json calls were not cached)
2. We were inefficiently serializing items by instantiating extra objects
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.
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
What does this change do?
This commit the client to override the navigation menu setting
configured by the site temporarily based on the value of the
`navigation_menu` query param. The new query param replaces the old
`enable_sidebar` query param.
Why do we need this change?
The motivation here is to allow theme maintainers to quickly preview
what the site will look like with the various navigation menu site
setting.
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.
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.
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
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.
## Why do we need this change?
When loading the ember app, [MessageBus does not start polling immediately](f31f0b70f8/app/assets/javascripts/discourse/app/initializers/message-bus.js (L71-L81)) and instead waits for `document.readyState` to be `complete`. What this means is that if there are new messages being created while we have yet to start polling, those messages will not be received by the client.
With sidebar being the default navigation menu, the counts derived from `topic-tracking-state.js` on the client side is prominently displayed on every page. Therefore, we want to ensure that we are not dropping any messages on the channels that `topic-tracking-state.js` subscribes to.
## What does this change do?
This includes the `MessageBus.last_id`s for the MessageBus channels which `topic-tracking-state.js` subscribes to as part of the preloaded data when loading a page. The last ids are then used when we subscribe the MessageBus channels so that messages which are published before MessageBus starts polling will not be missed.
## Review Notes
1. See https://github.com/discourse/message_bus#client-support for documentation about subscribing from a given message id.
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.
A while back the definition of TL was changed but many
areas in the codebase still use the term 'Regular user'
despite it having some implicit meaning (TL2).
See 20140905055251_rename_trust_level_badges.rb
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.
* FIX: Ensure soft-deleted topics can be deleted
The topic was not found during the deletion process because it was
deleted and `@post.topic` was nil.
* DEV: Use @topic instead of finding the topic every time
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.
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.
This is a very subtle one. Setting the redirect URL is done by passing
a hash through a Discourse event. This is broken on Ruby 2 since the
support for keyword arguments in events was added.
In Ruby 2 the last argument is cast to keyword arguments if it is a
hash. The key point here is that creates a new copy of the hash, so
what the plugin is modifying is not the hash that was passed.
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)
