We're seeing a lot of log noise coming from unhandled exceptions stemming from requests to TopicsController#show where id is passed in as an array.
In the implementation of the method, we assume that if id is present it will be a string. This is because one of the routes to this action uses :id as a URL fragment, and so must be a string. However, there are other routes that go to this endpoint as well. Some of them don't have this URL fragment, so you can pass an arbitrary id query parameter.
Instead of a downstream unhandled exception, we raise a Discourse::InvalidParameters upfront.
This commit improves `TopicsController#show` to not load suggested and
related topics unless it is the last page of the topic's view.
Previously, we avoided loading suggested and related topics by the use
of conditionals in the `TopicViewSerializer` to avoid calling
`TopicView#suggested_topics` and `TopicView#related_topics`. However,
this pattern is not reliable as the methods can still be called from
other spots in the code base. Instead, we ensure that
`TopicView#include_suggested` and `TopicView#include_related` is set
correctly on the instance of `TopicView` which ensures that for the
given instance, `TopicView#suggested_topics` and
`TopicView#related_topics` will be a noop.
When creating a shared draft, we're recording topic view stats on the draft and then pass those on when the draft is published, conflating the actual view count.
This fixes that by not registering topic views if the topic is a shared draft.
Similar to https://github.com/discourse/discourse/pull/28061, merging topics with many posts can exceed the 30 seconds timeout that Unicorn workers are limited to, so we should move the operation into a background thread to get around this limit.
Internal topic: t/133710.
Performing a bulk action on many topics can exceed the 30 seconds timeout that Unicorn workers have which results in the request failing and the operation getting aborted. To get around this 30 seconds timeout, we can push the operation into a background thread using the rack `hijack` API.
Internal topic: t/133779.
Fixed using next instead. It was causing this kind of errors:
```
Job exception: unexpected return
/var/www/discourse/app/controllers/topics_controller.rb:1304:in `block in defer_topic_view'
/var/www/discourse/lib/scheduler/defer.rb:115:in `block in do_work'
rails_multisite-6.0.0/lib/rails_multisite/connection_management/null_instance.rb:49:in `with_connection'
rails_multisite-6.0.0/lib/rails_multisite/connection_management.rb:21:in `with_connection'
/var/www/discourse/lib/scheduler/defer.rb:109:in `do_work'
/var/www/discourse/lib/scheduler/defer.rb:97:in `block (2 levels) in start_thread'
```
Previously, we did not log any topic slow mode changes. This allowed
some malicious (or just careless) TL4 users to delete slow modes created
by moderators at will. Administrators could not see who changed the slow
mode unless they had SQL knowledge and used Data Explorer.
This commit enables logging who turns slow mode on, off, or changes it.
Related meta topic: https://meta.discourse.org/t/why-is-there-no-record-of-who-added-or-removed-slow-mode/316354
Followup to 527f02e99f,
I had to introduce defer_track_visit_v2 because discourse-docs
relied on defer_track_visit. Now that discourse-docs
is using the new method as of
discourse/discourse-docs@0d9365571b,
we can rename it in core. Then we will need one more PR
in both core and docs to remove usage of the "v2" method.
Followup 2f2da72747
This commit moves topic view tracking from happening
every time a Topic is requested, which is susceptible
to inflating numbers of views from web crawlers, to
our request tracker middleware.
In this new location, topic views are only tracked when
the following headers are sent:
* HTTP_DISCOURSE_TRACK_VIEW - This is sent on every page navigation when
clicking around the ember app. We count these as browser page views
because we know it comes from the AJAX call in our app. The topic ID
is extracted from HTTP_DISCOURSE_TRACK_VIEW_TOPIC_ID
* HTTP_DISCOURSE_DEFERRED_TRACK_VIEW - Sent when MessageBus initializes
after first loading the page to count the initial page load view. The
topic ID is extracted from HTTP_DISCOURSE_DEFERRED_TRACK_VIEW.
This will bring topic views more in line with the change we
made to page views in the referenced commit and result in
more realistic topic view counts.
When converting a PM to a public topic (and vice versa), if there was a validation error (like a topic already used, or a tag required or not allowed) the error message wasn't bubbled up nor shown to the user.
This fix ensures we properly stop the conversion whenever a validation error happens and bubble up the errors back to the user so they can be informed.
Internal ref - t/128795
This commit introduces a few changes as a result of
customer issues with finding why a topic was relisted.
In one case, if a user edited the OP of a topic that was
unlisted and hidden because of too many flags, the topic
would get relisted by directly changing topic.visible,
instead of going via TopicStatusUpdater.
To improve tracking we:
* Introduce a visibility_reason_id to topic which functions
in a similar way to hidden_reason_id on post, this column is
set from the various places we change topic visibility
* Fix Post#unhide! which was directly modifying topic.visible,
instead we use TopicStatusUpdater which sets visibility_reason_id
and also makes a small action post
* Show the reason topic visibility changed when hovering the
unlisted icon in topic status on topic titles
This would allow a theme component (or an API call) to reset the bump
date of a topic to a given post's created_at date.
I picked `post_id` as the parameter here because it provides a bit of
extra protection against accidentally resetting the bump date to a date
that doesn't make sense.
Why this change?
When the URL `/t/1234?preview_theme_id=21` is loaded, we redirect to
`/t/<topic slug>/1234` stripping the `preview_theme_id` query params.
What does this change do?
This change builds on 61248652cd and
simply adds the `preview_theme_id` query param when redirecting.
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).
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.
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.
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 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 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.
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.
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.
* 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'
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
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 #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.
* 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 can no longer be used from the user interface and could be used to
generate useless topic invites notifications. This commit adds site
setting max_topic_invitations_per_minute to prevent invite spam.
Hard deleting topics that contained soft deleted posts or small actions
used to create orphan posts because only the first post was hard
deleted. This commit adds an error message if there are still posts left
in the topic that must be hard deleted first or hard deletes all small
actions too immediately (there is no other way of hard deleting a small
action because there is no wrench menu).
Before, whispers were only available for staff members.
Config has been changed to allow to configure privileged groups with access to whispers. Post migration was added to move from the old setting into the new one.
I considered having a boolean column `whisperer` on user model similar to `admin/moderator` for performance reason. Finally, I decided to keep looking for groups as queries are only done for current user and didn't notice any N+1 queries.
This commit migrates all bookmarks to be polymorphic (using the
bookmarkable_id and bookmarkable_type) columns. It also deletes
all the old code guarded behind the use_polymorphic_bookmarks setting
and changes that setting to true for all sites and by default for
the sake of plugins.
No data is deleted in the migrations, the old post_id and for_topic
columns for bookmarks will be dropped later on.
