In the past we would build the stack of Omniauth providers at boot, which meant that plugins had to register any authenticators in the root of their plugin.rb (i.e. not in an `after_initialize` block). This could be frustrating because many features are not available that early in boot (e.g. Zeitwerk autoloading).
Now that we build the omniauth strategy stack 'just in time', it is safe for plugins to register their auth methods in an `after_initialize` block. This commit relaxes the old restrictions so that plugin authors have the option to move things around.
Previously, we would build the stack of omniauth authenticators once on boot. That meant that all strategies had to be included, even if they were disabled. We then used the `before_request_phase` to ensure disabled strategies could not be used. This works well, but it means that omniauth is often doing unnecessary work running logic in disabled strategies.
This commit refactors things so that we build the stack of strategies on each request. That means we only need to include the enabled strategies in the stack - disabled strategies are totally ignored. Building the stack on-demand like this does add some overhead to auth requests, but on the majority of sites that will be significantly outweighed by the fact we're now skipping logic for disabled authenticators.
As well as the slight performance improvement, this new approach means that:
- Broken (i.e. exception-raising) strategies cannot cause issues on a site if they're disabled
- `other_phase` of disabled strategies will never appear in the backtrace of other authentication errors
No plugins or themes rely on anonymous_posting_min_trust_level so we
can just switch straight over to anonymous_posting_allowed_groups
This also adds an AUTO_GROUPS const which can be imported in JS
tests which is analogous to the one defined in group.rb. This can be used
to set the current user's groups where JS tests call for checking these groups
against site settings.
Finally a AtLeastOneGroupValidator validator is added for group_list site
settings which ensures that at least one group is always selected, since if
you want to allow all users to use a feature in this way you can just use
the everyone group.
Using Wizard.exclude_steps applies to all sites in a multisite cluster.
In order to exclude steps for individual sites at run-time, a new
instance method `remove_step` is being added.
* DEV: refactor rake asset precompile tasks
add a separate ember build task that does not depend on rails env
allowing us to compile assets without db+redis connections
rename EMBER_CLI_COMPILE_DONE to SKIP_EMBER_CLI_COMPILE
better semantics in build steps
This PR addresses the push to unify the icon representing AI throughout Discourse, by using the discourse-sparkles icon.
The icon is being moved to core to make changes with dependencies included in core that were using the "magic" icon instead.
In 2 places "magic" -> "discourse-sparkles,
1. topic summaries
2. (unreleased) chat summaries example
The UrlHelper#escape_uri helper has been deprecated and replaced by UrlHelper#normalized_encode, and was marked for removal in 3.0. This PR removes the method.
* FIX: Secure upload post processing race condition
This commit fixes a couple of issues.
A little background -- when uploads are created in the composer
for posts, regardless of whether the upload will eventually be
marked secure or not, if secure_uploads is enabled we always mark
the upload secure at first. This is so the upload is by default
protected, regardless of post type (regular or PM) or category.
This was causing issues in some rare occasions though because
of the order of operations of our post creation and processing
pipeline. When creating a post, we enqueue a sidekiq job to
post-process the post which does various things including
converting images to lightboxes. We were also enqueuing a job
to update the secure status for all uploads in that post.
Sometimes the secure status job would run before the post process
job, marking uploads as _not secure_ in the background and changing
their ACL before the post processor ran, which meant the users
would see a broken image in their posts. This commit fixes that issue
by always running the upload security changes inline _within_ the
cooked_post_processor job.
The other issue was that the lightbox wrapper link for images in
the post would end up with a URL like this:
```
href="/secure-uploads/original/2X/4/4e1f00a40b6c952198bbdacae383ba77932fc542.jpeg"
```
Since we weren't actually using the `upload.url` to pass to
`UrlHelper.cook_url` here, we weren't converting this href to the CDN
URL if the post was not in a secure context (the UrlHelper does not
know how to convert a secure-uploads URL to a CDN one). Now we
always end up with the correct lightbox href. This was less of an issue
than the other one, since the secure-uploads URL works even when the
upload has become non-secure, but it was a good inconsistency to fix
anyway.
As of #23867 this is now a real package, so updating the imports to
use the real package name, rather than relying on the alias. The
name change in the package name is because `I18n` is not a valid
name as NPM packages must be all lowercase.
This commit also introduces an eslint rule to prevent importing from
the old I18n path.
For themes/plugins, the old 'i18n' name remains functional.
There are cases where a user can copy image markdown from a public
post (such as via the discourse-templates plugin) into a PM which
is then sent via an email. Since a PM is a secure context (via the
.with_secure_uploads? check on Post), the image will get a secure
URL in the PM post even though the backing upload is not secure.
This fixes the bug in that case where the image would be stripped
from the email (since it had a /secure-uploads/ URL) but not re-attached
further down the line using the secure_uploads_allow_embed_images_in_emails
setting because the upload itself was not secure.
The flow in Email::Sender for doing this is still not ideal, but
there are chicken and egg problems around when to strip the images,
how to fit in with other attachments and email size limits, and
when to apply the images inline via Email::Styles. It's convoluted,
but at least this fixes the Template use case for now.
Why this change?
This ensures that malicious requests cannot end up causing the logs to
quickly fill up. The default chosen is sufficient for most legitimate
requests to the Discourse application.
When truncation happens, parsing of logs in supported format like
lograge may break down.
Why this change?
The `PostsController#create` action allows arbitrary topic custom fields
to be set by any user that can create a topic. Without any restrictions,
this opens us up to potential security issues where plugins may be using
topic custom fields in security sensitive areas.
What does this change do?
1. This change introduces the `register_editable_topic_custom_field` plugin
API which allows plugins to register topic custom fields that are
editable either by staff users only or all users. The registered
editable topic custom fields are stored in `DiscoursePluginRegistry` and
is called by a new method `Topic#editable_custom_fields` which is then
used in the `PostsController#create` controller action. When an unpermitted custom fields is present in the `meta_data` params,
a 400 response code is returned.
2. Removes all reference to `meta_data` on a topic as it is confusing
since we actually mean topic custom fields instead.
Preloading just metadata is not always respected by browsers, and
sometimes the whole video will be downloaded. This switches to using a
placeholder image for the video and only loads the video when the play
button is clicked.
Currently, `window.I18n` is defined in an old school hand written
script, inlined into locale/*.js by the Rails asset pipeline, and
then the global variable is shimmed into a pseudo AMD module later
in `module-shims.js`.
This approach has some problems – for one thing, when we add a new
V2 addon (e.g. in #23859), Embroider/Webpack is stricter about its
dependencies and won't let you `import from "I18n";` when `"I18n"`
isn't listed as one of its `dependencies` or `peerDependencies`.
This moves `I18n` into a real package – `discourse-i18n`. (I was
originally planning to keep the `I18n` name since it's a private
package anyway, but NPM packages are supposed to have lower case
names and that may cause problems with other tools.)
This package defines and exports a regular class, but also defines
the default global instance for backwards compatibility. We should
use the exported class in tests to make one-off instances without
mutating the global instance and having to clean it up after the
test run. However, I did not attempt that refactor in this PR.
Since `discourse-i18n` is now included by the app, the locale
scripts needs to be loaded after the app chunks. Since no "real"
work happens until later on when we kick things off in the boot
script, the order in which the script tags appear shouldn't be a
problem. Alternatively, we can rework the locale bundles to be more
lazy like everything else, and require/import them into the app.
I avoided renaming the imports in this commit since that would be
quite noisy and drowns out the actual changes here. Instead, I used
a Webpack alias to redirect the current `"I18n"` import to the new
package for the time being. In a separate commit later on, I'll
rename all the imports in oneshot and remove the alias. As always,
plugins and the legacy bundles (admin/wizard) still relies on the
runtime AMD shims regardless.
For the most part, I avoided refactoring the actual I18n code too
much other than making it a class, and some light stuff like `var`
into `let`.
However, now that it is in a reasonable format to work with (no
longer inside the global script context!) it may also be a good
opportunity to refactor and make clear what is intended to be
public API vs internal implementation details.
Speaking of, I took the librety to make `PLACEHOLDER`, `SEPARATOR`
and `I18nMissingInterpolationArgument` actual constants since it
seemed pretty clear to me those were just previously stashed on to
the `I18n` global to avoid polluting the global namespace, rather
than something we expect the consumers to set/replace.
This is part 2 (of 3) for passkeys support.
This adds a hidden site setting plus routes and controller actions.
1. registering passkeys
Passkeys are registered in a two-step process. First, `create_passkey`
returns details for the browser to create a passkey. This includes
- a challenge
- the relying party ID and Origin
- the user's secure identifier
- the supported algorithms
- the user's existing passkeys (if any)
Then the browser creates a key with this information, and submits it to
the server via `register_passkey`.
2. authenticating passkeys
A similar process happens here as well. First, a challenge is created
and sent to the browser. Then the browser makes a public key credential
and submits it to the server via `passkey_auth_perform`.
3. renaming/deleting passkeys
These routes allow changing the name of a key and deleting it.
4. checking if session is trusted for sensitive actions
Since a passkey is a password replacement, we want to make sure to confirm the user's identity before allowing adding/deleting passkeys. The u/trusted-session GET route returns success if user has confirmed their session (and failed if user hasn't). In the frontend (in the next PR), we're using these routes to show the password confirmation screen.
The `/u/confirm-session` route allows the user to confirm their session with a password. The latter route's functionality already existed in core, under the 2FA flow, but it has been abstracted into its own here so it can be used independently.
Co-authored-by: Alan Guo Xiang Tan <gxtan1990@gmail.com>
For the admin plugin list we want to be able to link to
a meta topic for plugins, but we have no standard way to
do this at the moment. This adds support for meta_topic_id
alongside other plugin metadata like authors, URL etc,
that gets built into a Meta topic URL in the serializer.
To match discourse_theme CLI behavior, we should skip hidden files/directories (e.g. `.git`), and two regular directories: `node_modules/` and `src/`.
Without these excludes, it's very easy for a theme to hit the file count limit. e.g. when trying this with discourse-kanban-board, I got:
> The number of files (20366) in the theme has exceeded the maximum allowed number of files (1024)
We have a custom implementation of #symbolize_keys in our Onebox helpers. This is likely a legacy from when Onebox was a standalone gem. This change replaces all usages with either #deep_symbolize_keys from ActiveSupport, or appropriate option to the JSON parser gem used.
We have a custom implementation of #blank? in our Onebox helpers. This is likely a legacy from when Onebox was a standalone gem. This change replaces all usages with respective incarnations of #blank?, #present?, and #presence from ActiveSupport. It changes a bunch of "unless blank" to "if present" as well.
This is part 1 of 3, split up of PR #23529. This PR refactors the
webauthn code to support passkey authentication/registration.
Passkeys aren't used yet, that is coming in PRs 2 and 3.
Co-authored-by: Alan Guo Xiang Tan <gxtan1990@gmail.com>
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
If a user somehow is looking at an old version of the page and attempts
to like a post they already like. Display a more reasonable error message.
Previously we would display:
> You are not permitted to view the requested resource.
New error message is:
> Oops! You already performed this action. Can you try refreshing the page?
Triggering this error condition is very tricky, you need to stop the
message bus. A possible reason for it could be bad network connectivity.
We will soon be dropping support for `/theme-qunit` in production, so this will start failing if we don't remove it. Plus, we now have system specs which verify the end-to-end functionality of the Theme QUnit system.
This was the last thing which was using the legacy `run-qunit` script, so that can also be dropped.
* FIX: min_personal_message_post_length not applying to first post
Due to the way PostCreator is wired, we were not applying min_personal_message_post_length
to the first post.
This meant that admins could not configure it so PMs have different
limits.
The code was already pretending that this works, but had no reliable way
of figuring out if we were dealing with a private message
This commit adds limits to themes and theme components on the:
- file size of about.json and .discourse-compatibility
- file size of theme assets
- number of files in a theme
This extends search so it can have consumers that:
1. Can split off "term" from various advanced filters and orders
2. Can build a relation of either order or filter
It also moves a lot of stuff around in the search class for clarity.
Two new APIs are exposed:
`.apply_filter` to apply all the special filters to a posts/topics relation
`.apply_order` to force a particular order (eg: order:latest)
This can then be used by semantic search in Discourse AI
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.
This comment isn't necessarily on a line by itself, so we need to remove the `^` from the regex. This will fix `EMBER_ENV=development bin/rake assets:precompile`
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.
This method is used by assets:precompile to decide whether to apply `terser` to a file. Embroider chunks do not necessarily start with `chunk.`, and so they were incorrectly being re-terser'd by our assets:precompile task. This is inefficient, and also led to broken sourcemaps on some assets.
This is a follow up to 9caba30d5c
In that commit, we were migrating the database but we didn't actually
ensure that the database was created and that plugins were updated
before the databases were migrated.
## What is the context here?
The `docker.rake` Rakefile contains Rake tasks that are meant to be run
in the `discourse/discourse_test:release` Docker image. For example, we
have the `docker:test` Rake task that makes it easier to run the test
suite for a particular Discourse commit.
Why are we introducing a `docker:test:setup` Rake task?
While we have the `docker:test` Rake task, it is very limited in the
test commands that can be executed. It is very useful for automated
testing but not very useful for running tests in the development
environment. Therefore, we are introducing a `docker:test:setup` rake
task that can be used to set up the test environment for running tests.
The envisioned example usage is something like this:
```
docker run -d --name=discourse_test --entrypoint=/sbin/boot discourse/discourse_test:release
docker exec -u discourse:discourse discourse_test ruby script/docker_test.rb --no-tests
docker exec -u discourse:discourse discourse_test bundle exec rake docker:test:setup
docker exec -u discourse:discourse discourse_test bundle exec rspec <path to file>
```
Currently, if the review queue has both a flagged post and a flagged chat message, one of the two will have some of the labels of their actions replaced by those of the other. In other words, the labels are getting mixed up. For example, a flagged chat message might show up with an action labelled "Delete post".
This is happening because when using bundles, we are sending along the actions in a separate part of the response, so they can be shared by many reviewables. The bundles then index into this bag of actions by their ID, which is something generic describing the server action, e.g. "agree_and_delete".
The problem here is the same action can have different labels depending on the type of reviewable. Now that the bag of actions contains multiple actions with the same ID, which one is chosen is arbitrary. I.e. it doesn't distinguish based on the type of the reviewable.
This change adds an additional field to the actions, server_action, which now contains what used to be the ID. Meanwhile, the ID has been turned into a concatenation of the reviewable type and the server action, e.g. post-agree_and_delete.
This still provides the upside of denormalizing the actions while allowing for different reviewable types to have different labels and descriptions.
At first I thought I would prepend the reviewable type to the ID, but this doesn't work well because the ID is used on the server-side to determine which actions are possible, and these need to be shared between different reviewables. Hence the introduction of server_action, which now serves that purpose.
I also thought about changing the way that the bundle indexes into the bag of actions, but this is happening through some EmberJS mechanism, so we don't own that code.
This was causing the following notice to be printed out when running system specs:
```
I did no detect a custom `config/dev.yml` file, creating one for you where you can amend defaults.
```
(since 61571bee43)
This adds a new secure_uploads_pm_only site setting. When secure_uploads
is true with this setting, only uploads created in PMs will be marked
secure; no uploads in secure categories will be marked as secure, and
the login_required site setting has no bearing on upload security
either.
This is meant to be a stopgap solution to prevent secure uploads
in a single place (private messages) for sensitive admin data exports.
Ideally we would want a more comprehensive way of saying that certain
upload types get secured which is a hybrid/mixed mode secure uploads,
but for now this will do the trick.
Admins are always able to send PMs, so it doesn't make
sense that they shouldn't be able to convert topics just
because they aren't in personal_message_enabled_groups.
Previously we would respect it if the filter was `nil`, but if `default` was explicitly passed then it would ignore the category order settings. This explicit passing of `filter=default` happens for some types of navigations in the JS app.
This extends the fix from 92bc61b4be
The theme tests we use for the smoke-test typically take 3-4 seconds to complete. This commit reduces the timeout from 10 minutes to 20 seconds, so that failures are detected more quickl
Our Ember build compiles assets into multiple chunks. In the past, we used the output from ember-auto-import-chunks-json-generator to give Rails a map of those chunks. However, that addon is specific to ember-auto-import, and is not compatible with Embroider.
Instead, we can switch to parsing the html files which are output by ember-cli. These are guaranteed to have the correct JS files in the correct place. A <discourse-chunked-script> will allow us to easily identify which chunks belong to which entrypoint.
In future, as we update more entrypoints to be compiled by Embroider/Webpack, we can easily introduce new wrappers.
Previously applied in 2c58d45 and reverted in 24d46fd. This version has been updated for subfolder support.
New headers were added to upload PUT requests as part of a MinIO update (cf42466). This change updates the asset bucket CORS ruleset to allow the new headers in the preflight request.
See https://dev.discourse.org/t/111136
Co-authored-by: Sam Saffron <sam.saffron@gmail.com>
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.
Our Ember build compiles assets into multiple chunks. In the past, we used the output from `ember-auto-import-chunks-json-generator` to give Rails a map of those chunks. However, that addon is specific to ember-auto-import, and is not compatible with Embroider.
Instead, we can switch to parsing the html files which are output by ember-cli. These are guaranteed to have the correct JS files in the correct place. A `<discourse-chunked-script>` will allow us to easily identify which chunks belong to which entrypoint.
In future, as we update more entrypoints to be compiled by Embroider/Webpack, we can easily introduce new wrappers.
Followup to eea74e0e32. Site settings
which are a list without a list_type should also have the _map
extension added which returns an array based on split("|").
For example:
```
SiteSetting.post_menu_map
=> ["read", "like"]
```
* DEV: Add rake command to help detect dead settings
Some Site Settings may still exist but are no longer being used in the
core discourse code or in related plugins. This rake task will help
identify any unused (aka: dead) settings by using the `rg` command to
search for them.
You can execute the rake task by using this command:
`LOAD_PLUGINS=1 bin/rails "site_settings:find_dead"`
* Add env variable, apply feedback
When navigating around, we make ajax requests with a parameter like `?filter=latest`. This results in the TopicQuery being set up with `filter: "latest"` as a string. The logic introduced in fd9a5bc0 checks for equality with `:latest` and `:unseen` symbols, which didn't work correctly in this situation
This commit makes the logic detect both strings and symbols, and adds a spec for the behaviour.
This adds support for oneboxing WEBP and AVIF images in posts and fixing
oneboxing fixes download remote images for those formats too.
Reported in https://meta.discourse.org/t/-/276433?u=falco
Reverts e2705df and re-lands #23187 and #23219.
The issue was incorrect order of execution of Rails' `assets:precompile` task in our own precompilation stack.
Co-authored-by: David Taylor <david@taylorhq.com>
This commit adds some system specs to test uploads with
direct to S3 single and multipart uploads via uppy. This
is done with minio as a local S3 replacement. We are doing
this to catch regressions when uppy dependencies need to
be upgraded or we change uppy upload code, since before
this there was no way to know outside manual testing whether
these changes would cause regressions.
Minio's server lifecycle and the installed binaries are managed
by the https://github.com/discourse/minio_runner gem, though the
binaries are already installed on the discourse_test image we run
GitHub CI from.
These tests will only run in CI unless you specifically use the
CI=1 or RUN_S3_SYSTEM_SPECS=1 env vars.
For a history of experimentation here see https://github.com/discourse/discourse/pull/22381
Related PRs:
* https://github.com/discourse/minio_runner/pull/1
* https://github.com/discourse/minio_runner/pull/2
* https://github.com/discourse/minio_runner/pull/3
Manipulating theme module paths means that the paths you author are not the ones used at runtime. This can lead to some very unexpected behavior and potential module name clashes. It also meant that the refactor in 16c6ab8661 was unable to correctly match up theme connector js/templates.
While this could technically be a breaking change, I think it is reasonably safe because:
1. Themes are already forced to use relative paths when referencing their own modules (since they're namespaced based on the site-specific id). The only time this might be problematic is when theme tests reference modules in the theme's main `javascripts` directory
2. For things like components/services/controllers/etc. our custom Ember resolver works backwards from the end of the path, so adding `discourse/` in the middle will not affect resolution.
`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.
This PR adds a new toggle to switch the (new) /new list between showing topics with new replies (a.k.a unread topics), new topics, or everything mixed together.
The category feature that automatically closes topics does it silently
This amends it so `rake topics:apply_autoclose` which does retroactive
closing will also do so silently.
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.
Prior to this fix we would output an image with no width/height which would then bypass a large part of `CookedProcessorMixin` and have no aspect ratio. As a result, an image with no size would cause layout shift.
It also removes a fix for oneboxes in chat messages due to this case.
This adds a new `loaderShim()` function to ensure certain modules
are present in the `loader.js` registry and therefore runtime
`require()`-able.
Currently, the classic build pipeline puts a lot of things in the
runtime `loader.js` registry automatically. For example, all of
the ember-auto-import packages are in there.
Going forward, and especially as we switch to the Embroider build
pipeline, this will not be guarenteed. We need to keep an eye on
what modules (packages) our "external" bundles (admin, wizard,
markdown-it, plugins, etc) are expecting to be present and put
them into the registry proactively.
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.
What is the context of this change?
Before 7c6a8f1c74, we were using
`preload(:tags)` on the topics relation but that was accidentally
removed in the refactor. This was discovered and fixed in
5bec894a8c but insteadl of using
`preload(:tags)` we ended up using `includes(:tags)`. The problem here
is that `includes(:tags)` can either result in `preload(:tags)` or
`eager_load(:tags)` but for some reason ActiveRecord is deciding to
`eager_load(:tags)` resulting in a joins to the `topic_tags` and `tags`
table which is not necessarily and leads to more inefficient queries.
When `includes(:tags)` is used, listing the latest topics ended up
generating the following sample queries to fetch the list of topics to display.
```
SELECT DISTINCT "topics"."pinned_at" AS alias_0, "topics"."id" FROM "topics" LEFT OUTER JOIN "categories" ON "categories"."id" = "topics"."category_id" LEFT OUTER JOIN "topic_tags" ON "topic_tags"."topic_id" = "topics"."id" LEFT OUTER JOIN "tags" ON "tags"."id" = "topic_tags"."tag_id" LEFT OUTER JOIN topic_users AS tu ON (topics.id = tu.topic_id AND tu.user_id = 29) LEFT JOIN category_users ON category_users.category_id = topics.category_id AND category_users.user_id = 29 WHERE "topics"."deleted_at" IS NULL AND (topics.archetype <> 'private_message') AND (COALESCE(categories.topic_id, 0) <> topics.id) AND (COALESCE(tu.notification_level,1) > 0) AND (topics.category_id = -1
OR
(COALESCE(category_users.notification_level, 1) <> 0 AND (topics.category_id IS NULL OR topics.category_id NOT IN(-1)))
OR tu.notification_level > 1) AND (pinned_globally AND pinned_at IS NOT NULL AND (topics.pinned_at > tu.cleared_pinned_at OR tu.cleared_pinned_at IS NULL)) ORDER BY "topics"."pinned_at" DESC LIMIT 30
SELECT "topics"."id" AS t0_r0, "topics"."title" AS t0_r1, "topics"."last_posted_at" AS t0_r2, "topics"."created_at" AS t0_r3, "topics"."updated_at" AS t0_r4, "topics"."views" AS t0_r5, "topics"."posts_count" AS t0_r6, "topics"."user_id" AS t0_r7, "topics"."last_post_user_id" AS t0_r8, "topics"."reply_count" AS t0_r9, "topics"."featured_user1_id" AS t0_r10, "topics"."featured_user2_id" AS t0_r11, "topics"."featured_user3_id" AS t0_r12, "topics"."deleted_at" AS t0_r13, "topics"."highest_post_number" AS t0_r14, "topics"."like_count" AS t0_r15, "topics"."incoming_link_count" AS t0_r16, "topics"."category_id" AS t0_r17, "topics"."visible" AS t0_r18, "topics"."moderator_posts_count" AS t0_r19, "topics"."closed" AS t0_r20, "topics"."archived" AS t0_r21, "topics"."bumped_at" AS t0_r22, "topics"."has_summary" AS t0_r23, "topics"."archetype" AS t0_r24, "topics"."featured_user4_id" AS t0_r25, "topics"."notify_moderators_count" AS t0_r26, "topics"."spam_count" AS t0_r27, "topics"."pinned_at" AS t0_r28, "topics"."score" AS t0_r29, "topics"."percent_rank" AS t0_r30, "topics"."subtype" AS t0_r31, "topics"."slug" AS t0_r32, "topics"."deleted_by_id" AS t0_r33, "topics"."participant_count" AS t0_r34, "topics"."word_count" AS t0_r35, "topics"."excerpt" AS t0_r36, "topics"."pinned_globally" AS t0_r37, "topics"."pinned_until" AS t0_r38, "topics"."fancy_title" AS t0_r39, "topics"."highest_staff_post_number" AS t0_r40, "topics"."featured_link" AS t0_r41, "topics"."reviewable_score" AS t0_r42, "topics"."image_upload_id" AS t0_r43, "topics"."slow_mode_seconds" AS t0_r44, "topics"."bannered_until" AS t0_r45, "topics"."external_id" AS t0_r46, "categories"."id" AS t1_r0, "categories"."name" AS t1_r1, "categories"."color" AS t1_r2, "categories"."topic_id" AS t1_r3, "categories"."topic_count" AS t1_r4, "categories"."created_at" AS t1_r5, "categories"."updated_at" AS t1_r6, "categories"."user_id" AS t1_r7, "categories"."topics_year" AS t1_r8, "categories"."topics_month" AS t1_r9, "categories"."topics_week" AS t1_r10, "categories"."slug" AS t1_r11, "categories"."description" AS t1_r12, "categories"."text_color" AS t1_r13, "categories"."read_restricted" AS t1_r14, "categories"."auto_close_hours" AS t1_r15, "categories"."post_count" AS t1_r16, "categories"."latest_post_id" AS t1_r17, "categories"."latest_topic_id" AS t1_r18, "categories"."position" AS t1_r19, "categories"."parent_category_id" AS t1_r20, "categories"."posts_year" AS t1_r21, "categories"."posts_month" AS t1_r22, "categories"."posts_week" AS t1_r23, "categories"."email_in" AS t1_r24, "categories"."email_in_allow_strangers" AS t1_r25, "categories"."topics_day" AS t1_r26, "categories"."posts_day" AS t1_r27, "categories"."allow_badges" AS t1_r28, "categories"."name_lower" AS t1_r29, "categories"."auto_close_based_on_last_post" AS t1_r30, "categories"."topic_template" AS t1_r31, "categories"."contains_messages" AS t1_r32, "categories"."sort_order" AS t1_r33, "categories"."sort_ascending" AS t1_r34, "categories"."uploaded_logo_id" AS t1_r35, "categories"."uploaded_background_id" AS t1_r36, "categories"."topic_featured_link_allowed" AS t1_r37, "categories"."all_topics_wiki" AS t1_r38, "categories"."show_subcategory_list" AS t1_r39, "categories"."num_featured_topics" AS t1_r40, "categories"."default_view" AS t1_r41, "categories"."subcategory_list_style" AS t1_r42, "categories"."default_top_period" AS t1_r43, "categories"."mailinglist_mirror" AS t1_r44, "categories"."minimum_required_tags" AS t1_r45, "categories"."navigate_to_first_post_after_read" AS t1_r46, "categories"."search_priority" AS t1_r47, "categories"."allow_global_tags" AS t1_r48, "categories"."reviewable_by_group_id" AS t1_r49, "categories"."read_only_banner" AS t1_r50, "categories"."default_list_filter" AS t1_r51, "categories"."allow_unlimited_owner_edits_on_first_post" AS t1_r52, "categories"."default_slow_mode_seconds" AS t1_r53, "categories"."uploaded_logo_dark_id" AS t1_r54, "tags"."id" AS t2_r0, "tags"."name" AS t2_r1, "tags"."created_at" AS t2_r2, "tags"."updated_at" AS t2_r3, "tags"."pm_topic_count" AS t2_r4, "tags"."target_tag_id" AS t2_r5, "tags"."description" AS t2_r6, "tags"."public_topic_count" AS t2_r7, "tags"."staff_topic_count" AS t2_r8 FROM "topics" LEFT OUTER JOIN "categories" ON "categories"."id" = "topics"."category_id" LEFT OUTER JOIN "topic_tags" ON "topic_tags"."topic_id" = "topics"."id" LEFT OUTER JOIN "tags" ON "tags"."id" = "topic_tags"."tag_id" LEFT OUTER JOIN topic_users AS tu ON (topics.id = tu.topic_id AND tu.user_id = 29) LEFT JOIN category_users ON category_users.category_id = topics.category_id AND category_users.user_id = 29 WHERE "topics"."deleted_at" IS NULL AND (topics.archetype <> 'private_message') AND (COALESCE(categories.topic_id, 0) <> topics.id) AND (COALESCE(tu.notification_level,1) > 0) AND (topics.category_id = -1
OR
(COALESCE(category_users.notification_level, 1) <> 0 AND (topics.category_id IS NULL OR topics.category_id NOT IN(-1)))
OR tu.notification_level > 1) AND (pinned_globally AND pinned_at IS NOT NULL AND (topics.pinned_at > tu.cleared_pinned_at OR tu.cleared_pinned_at IS NULL)) AND "topics"."id" = 7 ORDER BY "topics"."pinned_at" DESC
SELECT DISTINCT topics.bumped_at AS alias_0, "topics"."id" FROM "topics" LEFT OUTER JOIN "categories" ON "categories"."id" = "topics"."category_id" LEFT OUTER JOIN "topic_tags" ON "topic_tags"."topic_id" = "topics"."id" LEFT OUTER JOIN "tags" ON "tags"."id" = "topic_tags"."tag_id" LEFT OUTER JOIN topic_users AS tu ON (topics.id = tu.topic_id AND tu.user_id = 29) LEFT JOIN category_users ON category_users.category_id = topics.category_id AND category_users.user_id = 29 WHERE "topics"."deleted_at" IS NULL AND (topics.archetype <> 'private_message') AND (COALESCE(categories.topic_id, 0) <> topics.id) AND (COALESCE(tu.notification_level,1) > 0) AND (topics.category_id = -1
OR
(COALESCE(category_users.notification_level, 1) <> 0 AND (topics.category_id IS NULL OR topics.category_id NOT IN(-1)))
OR tu.notification_level > 1) AND (NOT ( pinned_globally AND pinned_at IS NOT NULL AND (topics.pinned_at > tu.cleared_pinned_at OR tu.cleared_pinned_at IS NULL) )) ORDER BY topics.bumped_at DESC LIMIT 30
SELECT "topics"."id" AS t0_r0, "topics"."title" AS t0_r1, "topics"."last_posted_at" AS t0_r2, "topics"."created_at" AS t0_r3, "topics"."updated_at" AS t0_r4, "topics"."views" AS t0_r5, "topics"."posts_count" AS t0_r6, "topics"."user_id" AS t0_r7, "topics"."last_post_user_id" AS t0_r8, "topics"."reply_count" AS t0_r9, "topics"."featured_user1_id" AS t0_r10, "topics"."featured_user2_id" AS t0_r11, "topics"."featured_user3_id" AS t0_r12, "topics"."deleted_at" AS t0_r13, "topics"."highest_post_number" AS t0_r14, "topics"."like_count" AS t0_r15, "topics"."incoming_link_count" AS t0_r16, "topics"."category_id" AS t0_r17, "topics"."visible" AS t0_r18, "topics"."moderator_posts_count" AS t0_r19, "topics"."closed" AS t0_r20, "topics"."archived" AS t0_r21, "topics"."bumped_at" AS t0_r22, "topics"."has_summary" AS t0_r23, "topics"."archetype" AS t0_r24, "topics"."featured_user4_id" AS t0_r25, "topics"."notify_moderators_count" AS t0_r26, "topics"."spam_count" AS t0_r27, "topics"."pinned_at" AS t0_r28, "topics"."score" AS t0_r29, "topics"."percent_rank" AS t0_r30, "topics"."subtype" AS t0_r31, "topics"."slug" AS t0_r32, "topics"."deleted_by_id" AS t0_r33, "topics"."participant_count" AS t0_r34, "topics"."word_count" AS t0_r35, "topics"."excerpt" AS t0_r36, "topics"."pinned_globally" AS t0_r37, "topics"."pinned_until" AS t0_r38, "topics"."fancy_title" AS t0_r39, "topics"."highest_staff_post_number" AS t0_r40, "topics"."featured_link" AS t0_r41, "topics"."reviewable_score" AS t0_r42, "topics"."image_upload_id" AS t0_r43, "topics"."slow_mode_seconds" AS t0_r44, "topics"."bannered_until" AS t0_r45, "topics"."external_id" AS t0_r46, "categories"."id" AS t1_r0, "categories"."name" AS t1_r1, "categories"."color" AS t1_r2, "categories"."topic_id" AS t1_r3, "categories"."topic_count" AS t1_r4, "categories"."created_at" AS t1_r5, "categories"."updated_at" AS t1_r6, "categories"."user_id" AS t1_r7, "categories"."topics_year" AS t1_r8, "categories"."topics_month" AS t1_r9, "categories"."topics_week" AS t1_r10, "categories"."slug" AS t1_r11, "categories"."description" AS t1_r12, "categories"."text_color" AS t1_r13, "categories"."read_restricted" AS t1_r14, "categories"."auto_close_hours" AS t1_r15, "categories"."post_count" AS t1_r16, "categories"."latest_post_id" AS t1_r17, "categories"."latest_topic_id" AS t1_r18, "categories"."position" AS t1_r19, "categories"."parent_category_id" AS t1_r20, "categories"."posts_year" AS t1_r21, "categories"."posts_month" AS t1_r22, "categories"."posts_week" AS t1_r23, "categories"."email_in" AS t1_r24, "categories"."email_in_allow_strangers" AS t1_r25, "categories"."topics_day" AS t1_r26, "categories"."posts_day" AS t1_r27, "categories"."allow_badges" AS t1_r28, "categories"."name_lower" AS t1_r29, "categories"."auto_close_based_on_last_post" AS t1_r30, "categories"."topic_template" AS t1_r31, "categories"."contains_messages" AS t1_r32, "categories"."sort_order" AS t1_r33, "categories"."sort_ascending" AS t1_r34, "categories"."uploaded_logo_id" AS t1_r35, "categories"."uploaded_background_id" AS t1_r36, "categories"."topic_featured_link_allowed" AS t1_r37, "categories"."all_topics_wiki" AS t1_r38, "categories"."show_subcategory_list" AS t1_r39, "categories"."num_featured_topics" AS t1_r40, "categories"."default_view" AS t1_r41, "categories"."subcategory_list_style" AS t1_r42, "categories"."default_top_period" AS t1_r43, "categories"."mailinglist_mirror" AS t1_r44, "categories"."minimum_required_tags" AS t1_r45, "categories"."navigate_to_first_post_after_read" AS t1_r46, "categories"."search_priority" AS t1_r47, "categories"."allow_global_tags" AS t1_r48, "categories"."reviewable_by_group_id" AS t1_r49, "categories"."read_only_banner" AS t1_r50, "categories"."default_list_filter" AS t1_r51, "categories"."allow_unlimited_owner_edits_on_first_post" AS t1_r52, "categories"."default_slow_mode_seconds" AS t1_r53, "categories"."uploaded_logo_dark_id" AS t1_r54, "tags"."id" AS t2_r0, "tags"."name" AS t2_r1, "tags"."created_at" AS t2_r2, "tags"."updated_at" AS t2_r3, "tags"."pm_topic_count" AS t2_r4, "tags"."target_tag_id" AS t2_r5, "tags"."description" AS t2_r6, "tags"."public_topic_count" AS t2_r7, "tags"."staff_topic_count" AS t2_r8 FROM "topics" LEFT OUTER JOIN "categories" ON "categories"."id" = "topics"."category_id" LEFT OUTER JOIN "topic_tags" ON "topic_tags"."topic_id" = "topics"."id" LEFT OUTER JOIN "tags" ON "tags"."id" = "topic_tags"."tag_id" LEFT OUTER JOIN topic_users AS tu ON (topics.id = tu.topic_id AND tu.user_id = 29) LEFT JOIN category_users ON category_users.category_id = topics.category_id AND category_users.user_id = 29 WHERE "topics"."deleted_at" IS NULL AND (topics.archetype <> 'private_message') AND (COALESCE(categories.topic_id, 0) <> topics.id) AND (COALESCE(tu.notification_level,1) > 0) AND (topics.category_id = -1
OR
(COALESCE(category_users.notification_level, 1) <> 0 AND (topics.category_id IS NULL OR topics.category_id NOT IN(-1)))
OR tu.notification_level > 1) AND (NOT ( pinned_globally AND pinned_at IS NOT NULL AND (topics.pinned_at > tu.cleared_pinned_at OR tu.cleared_pinned_at IS NULL) )) AND "topics"."id" IN (477, 481, 480, 479, 478, 467, 466, 230, 209, 183, 173, 179, 168, 139, 102, 144, 150, 118, 126, 88, 63, 46, 117, 171, 45, 77, 154, 158, 43, 79) ORDER BY topics.bumped_at DESC
```
Note how there are two extra queries which has to select `DISTINCT
topics.pinned_at` and `DISTINCT topics.bumped_at` because of the
unnecessary left joins to the `topic_tags` and `tags` table result in
duplicated rows in the topic tables. As a result, PG is not able to
use our indexes to effectively execute the query.
Comparing this to the queries being executed when `preload(:tags)` is
used.
```
SELECT "topics"."id" AS t0_r0, "topics"."title" AS t0_r1, "topics"."last_posted_at" AS t0_r2, "topics"."created_at" AS t0_r3, "topics"."updated_at" AS t0_r4, "topics"."views" AS t0_r5, "topics"."posts_count" AS t0_r6, "topics"."user_id" AS t0_r7, "topics"."last_post_user_id" AS t0_r8, "topics"."reply_count" AS t0_r9, "topics"."featured_user1_id" AS t0_r10, "topics"."featured_user2_id" AS t0_r11, "topics"."featured_user3_id" AS t0_r12, "topics"."deleted_at" AS t0_r13, "topics"."highest_post_number" AS t0_r14, "topics"."like_count" AS t0_r15, "topics"."incoming_link_count" AS t0_r16, "topics"."category_id" AS t0_r17, "topics"."visible" AS t0_r18, "topics"."moderator_posts_count" AS t0_r19, "topics"."closed" AS t0_r20, "topics"."archived" AS t0_r21, "topics"."bumped_at" AS t0_r22, "topics"."has_summary" AS t0_r23, "topics"."archetype" AS t0_r24, "topics"."featured_user4_id" AS t0_r25, "topics"."notify_moderators_count" AS t0_r26, "topics"."spam_count" AS t0_r27, "topics"."pinned_at" AS t0_r28, "topics"."score" AS t0_r29, "topics"."percent_rank" AS t0_r30, "topics"."subtype" AS t0_r31, "topics"."slug" AS t0_r32, "topics"."deleted_by_id" AS t0_r33, "topics"."participant_count" AS t0_r34, "topics"."word_count" AS t0_r35, "topics"."excerpt" AS t0_r36, "topics"."pinned_globally" AS t0_r37, "topics"."pinned_until" AS t0_r38, "topics"."fancy_title" AS t0_r39, "topics"."highest_staff_post_number" AS t0_r40, "topics"."featured_link" AS t0_r41, "topics"."reviewable_score" AS t0_r42, "topics"."image_upload_id" AS t0_r43, "topics"."slow_mode_seconds" AS t0_r44, "topics"."bannered_until" AS t0_r45, "topics"."external_id" AS t0_r46, "categories"."id" AS t1_r0, "categories"."name" AS t1_r1, "categories"."color" AS t1_r2, "categories"."topic_id" AS t1_r3, "categories"."topic_count" AS t1_r4, "categories"."created_at" AS t1_r5, "categories"."updated_at" AS t1_r6, "categories"."user_id" AS t1_r7, "categories"."topics_year" AS t1_r8, "categories"."topics_month" AS t1_r9, "categories"."topics_week" AS t1_r10, "categories"."slug" AS t1_r11, "categories"."description" AS t1_r12, "categories"."text_color" AS t1_r13, "categories"."read_restricted" AS t1_r14, "categories"."auto_close_hours" AS t1_r15, "categories"."post_count" AS t1_r16, "categories"."latest_post_id" AS t1_r17, "categories"."latest_topic_id" AS t1_r18, "categories"."position" AS t1_r19, "categories"."parent_category_id" AS t1_r20, "categories"."posts_year" AS t1_r21, "categories"."posts_month" AS t1_r22, "categories"."posts_week" AS t1_r23, "categories"."email_in" AS t1_r24, "categories"."email_in_allow_strangers" AS t1_r25, "categories"."topics_day" AS t1_r26, "categories"."posts_day" AS t1_r27, "categories"."allow_badges" AS t1_r28, "categories"."name_lower" AS t1_r29, "categories"."auto_close_based_on_last_post" AS t1_r30, "categories"."topic_template" AS t1_r31, "categories"."contains_messages" AS t1_r32, "categories"."sort_order" AS t1_r33, "categories"."sort_ascending" AS t1_r34, "categories"."uploaded_logo_id" AS t1_r35, "categories"."uploaded_background_id" AS t1_r36, "categories"."topic_featured_link_allowed" AS t1_r37, "categories"."all_topics_wiki" AS t1_r38, "categories"."show_subcategory_list" AS t1_r39, "categories"."num_featured_topics" AS t1_r40, "categories"."default_view" AS t1_r41, "categories"."subcategory_list_style" AS t1_r42, "categories"."default_top_period" AS t1_r43, "categories"."mailinglist_mirror" AS t1_r44, "categories"."minimum_required_tags" AS t1_r45, "categories"."navigate_to_first_post_after_read" AS t1_r46, "categories"."search_priority" AS t1_r47, "categories"."allow_global_tags" AS t1_r48, "categories"."reviewable_by_group_id" AS t1_r49, "categories"."read_only_banner" AS t1_r50, "categories"."default_list_filter" AS t1_r51, "categories"."allow_unlimited_owner_edits_on_first_post" AS t1_r52, "categories"."default_slow_mode_seconds" AS t1_r53, "categories"."uploaded_logo_dark_id" AS t1_r54 FROM "topics" LEFT OUTER JOIN "categories" ON "categories"."id" = "topics"."category_id" LEFT OUTER JOIN topic_users AS tu ON (topics.id = tu.topic_id AND tu.user_id = 29) LEFT JOIN category_users ON category_users.category_id = topics.category_id AND category_users.user_id = 29 WHERE "topics"."deleted_at" IS NULL AND (topics.archetype <> 'private_message') AND (COALESCE(categories.topic_id, 0) <> topics.id) AND (COALESCE(tu.notification_level,1) > 0) AND (topics.category_id = -1
OR
(COALESCE(category_users.notification_level, 1) <> 0 AND (topics.category_id IS NULL OR topics.category_id NOT IN(-1)))
OR tu.notification_level > 1) AND (pinned_globally AND pinned_at IS NOT NULL AND (topics.pinned_at > tu.cleared_pinned_at OR tu.cleared_pinned_at IS NULL)) ORDER BY "topics"."pinned_at" DESC LIMIT 30
SELECT "topic_tags".* FROM "topic_tags" WHERE "topic_tags"."topic_id" = 7
SELECT "topics"."id" AS t0_r0, "topics"."title" AS t0_r1, "topics"."last_posted_at" AS t0_r2, "topics"."created_at" AS t0_r3, "topics"."updated_at" AS t0_r4, "topics"."views" AS t0_r5, "topics"."posts_count" AS t0_r6, "topics"."user_id" AS t0_r7, "topics"."last_post_user_id" AS t0_r8, "topics"."reply_count" AS t0_r9, "topics"."featured_user1_id" AS t0_r10, "topics"."featured_user2_id" AS t0_r11, "topics"."featured_user3_id" AS t0_r12, "topics"."deleted_at" AS t0_r13, "topics"."highest_post_number" AS t0_r14, "topics"."like_count" AS t0_r15, "topics"."incoming_link_count" AS t0_r16, "topics"."category_id" AS t0_r17, "topics"."visible" AS t0_r18, "topics"."moderator_posts_count" AS t0_r19, "topics"."closed" AS t0_r20, "topics"."archived" AS t0_r21, "topics"."bumped_at" AS t0_r22, "topics"."has_summary" AS t0_r23, "topics"."archetype" AS t0_r24, "topics"."featured_user4_id" AS t0_r25, "topics"."notify_moderators_count" AS t0_r26, "topics"."spam_count" AS t0_r27, "topics"."pinned_at" AS t0_r28, "topics"."score" AS t0_r29, "topics"."percent_rank" AS t0_r30, "topics"."subtype" AS t0_r31, "topics"."slug" AS t0_r32, "topics"."deleted_by_id" AS t0_r33, "topics"."participant_count" AS t0_r34, "topics"."word_count" AS t0_r35, "topics"."excerpt" AS t0_r36, "topics"."pinned_globally" AS t0_r37, "topics"."pinned_until" AS t0_r38, "topics"."fancy_title" AS t0_r39, "topics"."highest_staff_post_number" AS t0_r40, "topics"."featured_link" AS t0_r41, "topics"."reviewable_score" AS t0_r42, "topics"."image_upload_id" AS t0_r43, "topics"."slow_mode_seconds" AS t0_r44, "topics"."bannered_until" AS t0_r45, "topics"."external_id" AS t0_r46, "categories"."id" AS t1_r0, "categories"."name" AS t1_r1, "categories"."color" AS t1_r2, "categories"."topic_id" AS t1_r3, "categories"."topic_count" AS t1_r4, "categories"."created_at" AS t1_r5, "categories"."updated_at" AS t1_r6, "categories"."user_id" AS t1_r7, "categories"."topics_year" AS t1_r8, "categories"."topics_month" AS t1_r9, "categories"."topics_week" AS t1_r10, "categories"."slug" AS t1_r11, "categories"."description" AS t1_r12, "categories"."text_color" AS t1_r13, "categories"."read_restricted" AS t1_r14, "categories"."auto_close_hours" AS t1_r15, "categories"."post_count" AS t1_r16, "categories"."latest_post_id" AS t1_r17, "categories"."latest_topic_id" AS t1_r18, "categories"."position" AS t1_r19, "categories"."parent_category_id" AS t1_r20, "categories"."posts_year" AS t1_r21, "categories"."posts_month" AS t1_r22, "categories"."posts_week" AS t1_r23, "categories"."email_in" AS t1_r24, "categories"."email_in_allow_strangers" AS t1_r25, "categories"."topics_day" AS t1_r26, "categories"."posts_day" AS t1_r27, "categories"."allow_badges" AS t1_r28, "categories"."name_lower" AS t1_r29, "categories"."auto_close_based_on_last_post" AS t1_r30, "categories"."topic_template" AS t1_r31, "categories"."contains_messages" AS t1_r32, "categories"."sort_order" AS t1_r33, "categories"."sort_ascending" AS t1_r34, "categories"."uploaded_logo_id" AS t1_r35, "categories"."uploaded_background_id" AS t1_r36, "categories"."topic_featured_link_allowed" AS t1_r37, "categories"."all_topics_wiki" AS t1_r38, "categories"."show_subcategory_list" AS t1_r39, "categories"."num_featured_topics" AS t1_r40, "categories"."default_view" AS t1_r41, "categories"."subcategory_list_style" AS t1_r42, "categories"."default_top_period" AS t1_r43, "categories"."mailinglist_mirror" AS t1_r44, "categories"."minimum_required_tags" AS t1_r45, "categories"."navigate_to_first_post_after_read" AS t1_r46, "categories"."search_priority" AS t1_r47, "categories"."allow_global_tags" AS t1_r48, "categories"."reviewable_by_group_id" AS t1_r49, "categories"."read_only_banner" AS t1_r50, "categories"."default_list_filter" AS t1_r51, "categories"."allow_unlimited_owner_edits_on_first_post" AS t1_r52, "categories"."default_slow_mode_seconds" AS t1_r53, "categories"."uploaded_logo_dark_id" AS t1_r54 FROM "topics" LEFT OUTER JOIN "categories" ON "categories"."id" = "topics"."category_id" LEFT OUTER JOIN topic_users AS tu ON (topics.id = tu.topic_id AND tu.user_id = 29) LEFT JOIN category_users ON category_users.category_id = topics.category_id AND category_users.user_id = 29 WHERE "topics"."deleted_at" IS NULL AND (topics.archetype <> 'private_message') AND (COALESCE(categories.topic_id, 0) <> topics.id) AND (COALESCE(tu.notification_level,1) > 0) AND (topics.category_id = -1
OR
(COALESCE(category_users.notification_level, 1) <> 0 AND (topics.category_id IS NULL OR topics.category_id NOT IN(-1)))
OR tu.notification_level > 1) AND (NOT ( pinned_globally AND pinned_at IS NOT NULL AND (topics.pinned_at > tu.cleared_pinned_at OR tu.cleared_pinned_at IS NULL) )) ORDER BY topics.bumped_at DESC LIMIT 30
SELECT "topic_tags".* FROM "topic_tags" WHERE "topic_tags"."topic_id" IN (477, 481, 480, 479, 478, 467, 466, 230, 209, 183, 173, 179, 168, 139, 102, 144, 150, 118, 126, 88, 63, 46, 117, 171, 45, 77, 154, 158, 43, 79)
SELECT "tags"."id", "tags"."name", "tags"."created_at", "tags"."updated_at", "tags"."pm_topic_count", "tags"."target_tag_id", "tags"."description", "tags"."public_topic_count", "tags"."staff_topic_count" FROM "tags" WHERE "tags"."id" IN (10, 20, 26, 7, 27, 28, 30, 19, 9, 4, 15, 29, 14, 18, 11, 25, 1, 21, 8, 22, 5, 32)
```
We end up with queries that are much more efficient as those queries can
effectively use the indexes.
This commit introduces the :push_notification event and deprecates :post_notification_alert.
The old :post_notification_alert event was not triggered when pushing chat notifications and did not respect when the user was in "do not disturb" mode.
The new event fixes these issues.
This plugin is no longer supported, and so we no longer need to run its tests in CI
(removing the comment and the 'Canned Replies' value from the array caused syntax_tree to change to the `%w` syntax)
Why this change?
This is a follow up to e8f7b62752.
Tracking of GC stats didn't really belong in the `MethodProfiler` class
so we want to extract that concern into its own class.
As part of this PR, the `track_gc_stat_per_request` site setting has
also been renamed to `instrument_gc_stat_per_request`.
What does this change do?
This change adds a hidden `track_gc_stat_per_request` site setting which
when enabled will track the time spent in GC, major GC count and minor
GC count during a request.
Why is this change needed?
We have plans to tune our GC in production but without any
instrumentation, we will not be able to know if our tuning is effective
or not. This commit takes the first step at instrumenting some basic GC
stats in core during a request which can then be consumed by the discourse-prometheus plugin.
Linking to the #feedback category can break if the category gets renamed or a different site locale is used. By using the correct hashtag (at the time of seeding) this issues can be avoided.
PresenceChannel configuration is cached using redis. That cache is used, and sometimes repopulated, during normal GET requests. When the primary redis server was readonly, that `redis.set` call would raise an error and cause the entire request to fail. Instead, we should ignore the failure and continue without populating the cache.
This commit introduces five rake tasks to help us with version bump procedures:
- `version_bump:beta` and `version_bump:minor_stable` are for our minor releases
- `version_bump:major_stable_prepare` and `version_bump:major_stable_merge` are for our major release process
- `version_bump:stage_security_fixes` is to collate multiple security fixes from private branches into a single branch for release
The scripts will stage the necessary commits in a branch and prompt you to create a PR for review. No changes to release branches or tags will be made without the PR being approved, and explicit confirmation of prompts in the scripts.
To avoid polluting the operator's primary working tree, the scripts create a temporary git worktree in a temporary directory and perform all checkouts/commits there.
A previous change updated `ReviewableQueuedPost`'s `created_by`
to be consistent with other reviewable types. It assigns
the the creator of the post being queued to `target_created_by` and sets
the `created_by` to the creator of the reviewable itself.
This fix updates some of the `created_by` references missed during the
intial fix.
Internal oneboxes to posts that contained oneboxed github links to
commits or PRs with long enough commit messages to have the `show-more`
and the `excerpt hidden` classes in their html were being stripped of
their content resulting in empty internal oneboxes.
see: https://meta.discourse.org/t/269436
This fixes a regression introduced in:
0b3cf83e3c
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.
For the Discourse 3.2 beta series, we intend to use a `-dev` suffix while beta versions are being developed in `main`/`tests-passed`. When a beta version is ready, it will be 'released' without the `-dev` suffix.
This commit adds support for the `-dev` suffix, and also refactors `Discourse::VERSION` so that the canonical representation is a simple human-readable string. Constants for each segment are derived from that, so the interface remains unchanged.
Embed Motoko service's primary URL is transiting from embed.smartcontracts.org to embed.motoko.org, this PR updates the Onebox logic to work for either domain.
This adds support for the `<=` and `<` version operators in `.discourse-compatibility` files. This allows for more flexibility (e.g. targeting the entire 3.1.x stable release via `< 3.2.0.beta1`), and should also make compatibility files to be more readable.
If an operator is not specified we default to `<=`, which matches the old behavior.
We recently added a "don't feed the trolls" feature which warns you about interacting with posts that have been flagged and are pending review. The problem is the warning persists even if an admin reviews the post and rejects the flag.
After this change we only consider active flags when deciding whether to show the warning or not.
We're seeing unhandled errors in production when web push notifications are failing with an SSL error. This is happening for a few users, but generating a large amount of log noise due to the sheer number of notifications.
This adds handling of SSL errors in two places:
1. In FinalDestination::HTTP, this is handled the same as a timeout error, and gives a chance to recover.
2. In PushNotificationPusher. This will cause the notification to retry a number of times, and if it keeps failing, disable push notifications for the user. (Existing behaviour.)
I wanted to wrap the SSL error in e.g. WebPush::RequestError, but the gem doesn't have request error handling, so didn't want to have the freedom patch diverge from the gem as well. Instead just propagating the raw SSL error.
The parameter ascending was deprecated (replaced by asc) and marked for deletion in 2.9. This PR removes it. Since the resulting code was a simple one-liner, the method body was inlined instead.
Allow anonymous users (logged-in, but set to anonymous posting) to like posts
---------
Co-authored-by: Emmett Ling <eling@zendesk.com>
Co-authored-by: Nat <natalie.tay@discourse.org>
We deprecated the keywords method, route, and format (replaced with methods, actions, and formats respectively) as parameters to Plugin::Instance#add_api_parameter_route, marked for removal in 2.7. This PR deletes them.
These methods were deprecated and marked for removal in 2.6. This change deletes them.
These deprecations use raise_error: true, so the fallbacks are at this point unreachable and can't be used anyway.
- Convert `admin-incoming-email` modal to component-based API
- Testing that the modal was working in local development was extremely challenging due to the need for `rejected` and `bounced` emails. Something that is not easy to stub in a local dev environment. To make this process more smooth for future developers I have added a new rake task:
```
desc "Creates sample email logs"
task "email_logs:populate" => ["db:load_config"] do |_, args|
DiscourseDev::EmailLog.populate!
end
```
That will generate fully functional email logs in development to be toyed with.
<img width="787" alt="Screenshot 2023-07-20 at 3 27 04 PM" src="https://github.com/discourse/discourse/assets/50783505/47b3fe34-cd7e-49a5-8fe6-768c0fbd1aa2">
* DEV: Skip srcset for onebox thumbnails
In an effort to preserve bandwidth especially for mobile devices this
change will prevent upscaled srcset attributes from being added to
onebox thumbnail images.
Besides checking the html for onebox classes, our database structure for
uploads does not distinguish between regular images and onebox thumbnail
images, but all upload images in discourse do have a thumbnail. By
default this thumbnail is what is used for the non-upscaled image for
onebox images, so we should only use that thumbnail. Because the
rendered onebox image size is likely smaller than the upload thumbnail
size there really shouldn't be a need to upscale.
Followup to b583872eed
and 54001060ea
Another place where we need to filter hashtag types to
only enabled ones is PrettyText, though the latter PR
above should also already make it so the correct priority
types are passed.
This is causing errors in the email processing workflow
for some customers (presumably ones with tagging disabled).
Performing a `Delete User`/`Delete and Block User` reviewable actions for a
queued post reviewable from the `review.show` route results in an error
popup even if the action completes successfully.
This happens because unlike other reviewable types, a user delete action
on a queued post reviewable results in the deletion of the reviewable
itself. A subsequent attempt to reload the reviewable record results in
404. The deletion happens as part of the call to `UserDestroyer` which
includes a step for destroying reviewables created by the user being
destroyed. At the root of this is the creator of the queued post
being set as the creator of the reviewable as instead of the system
user.
This change assigns the creator of the reviewable to the system user and
uses the more approapriate `target_created_by` column for the creator of the
post being queued.
Wikimedia provides a thumbnail url for its images, so we should use that
for oneboxes instead of the full-size image. Because the size of the
onebox image we display is quite small anyways the thumbnail wikimedia
provides should suffice and will save bandwidth.
See: https://meta.discourse.org/t/264039
The primary motivation is to simplify `eagerLoadRawTemplateModules` which curently introspects the module dependencies (the `imports` at runtime). This is no longer supported in Embroider as the AMD shims do not have any dependencies (since it's managed internally with webpack).
Previously we had three query parameters to control which tests would be run. The default was to run all core/plugin tests together, which would almost always lead to errors and does not match the way we run tests in CI.
This commit removes the three old parameters (skip_core, skip_plugins and single_plugin), and introduces a new 'target' parameter. This can have a value of 'core', 'plugins', 'all', or a specific plugin name. The default is 'core'. Attempting to use the old parameters will raise an error.
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.
Instead of having to remember every time, just always wait until the
current transaction (if it exists) has committed before clearing any
DistributedCache.
The only exception to this is caches that aren't caching things from
postgres.
This means we have to do the test setup after setting the test
transaction, because doing the test setup involves clearing caches.
Reapplying this - it now doesn't use after_commit if skip_db is set
* 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.
These avatar-related helper functions are used in pretty-text, which currently means we load the entire `discourse/lib/utilities` module into the mini-racer when running pretty-text on the server side. This stops us adding any logic or imports to discourse/lib/utilities which may depend on other `discourse/` namespace features.
This commit moves the avatar-related utils into a dedicated module in the `discourse-common` namespace, adds backwards-compatibility shims, and updates the pretty-text config accordingly.
This adds an option to allow non-image s3 files to be downloaded through CDN URL.
Addresses the issues in:
* meta.discourse.org/t/s3-cdn-url-not-being-used-on-non-image-uploads/175332
* meta.discourse.org/t/s3-uploads-using-cdn-for-pdfs/213218
Instead of having to remember every time, just always wait until the
current transaction (if it exists) has committed before clearing any
DistributedCache.
The only exception to this is caches that aren't caching things from
postgres.
This means we have to do the test setup after setting the test
transaction, because doing the test setup involves clearing caches.
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).
Why this change?
The process's pid is useful when we're trying to link output from
different processes together. In this case, we want to be able to link
the Rails server logs to the right rspec process.
Before:
[2] Viewing sidebar mobile collapses the sidebar when clicking outside of it
After:
[2] (#176342) Viewing sidebar mobile collapses the sidebar when clicking outside of it
Recently, site setting watched_precedence_over_muted was introduced - https://github.com/discourse/discourse/pull/22252
In this PR, we are allowing users to override it. The option is only displayed when the user has watched categories and muted tags, or vice versa.
This will be used when we move the channel creation for DMs
to happen when we first send a message in a DM channel to avoid
a double-request. For now we can just have a new API endpoint
for creating this that the existing frontend code can use,
that uses the new service pattern.
This also uses the new policy pattern for services where the policy
can be defined in a class so a more dynamic reason for the policy
failing can be sent to the controller.
Co-authored-by: Loïc Guitaut <loic@discourse.org>
Twitter is now redirecting anonymous users (with a browser-like user agent, which FinalDestination uses) to the login page. Skipping redirect-following for twitter.com will allow us to continue oneboxing tweets via the OpenGraph data and the API (when credentials are present).
https://meta.discourse.org/t/269371/17
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.
New setting which allow admin to define behavior when topic is in watched category and muted topic and vice versa.
If watched_precedence_over_muted setting is true, that topic is still visible in list of topics and notification is created.
If watched_precedence_over_muted setting is false, that topic is not still visible in list of topics and notification is skipped as well.
While we are unable to support OAUTH2 with pop3 (due to upstream dependency ruby/net-pop#16), we are adding the support for mail pollers plugin. Doing so, it would be possible to write a plugin which then uses other ways (microsoft graph sdk for example) to poll emails from a mailbox.
The idea is that a plugin would define a class which inherits from Email::Poller and defines a poll_mailbox static method which returns an array of strings. Then the plugin could call register_mail_poller(<class_name>) to have it registered. All the configuration (oauth2 tokens, email, etc) could be managed by sitesettings defined in the plugin.
This change adds support retroactively updating display names in the new quote format when the user's name is changed. It happens through a background job that is triggered by a callback when a user is saved with a new name.
Under exceptional cases people may need to resize the notification table.
This only happens on forums with a total of more than 2.5 billion notifications.
This rake task can be used to convert all the notification columns to
bigint to make more room.
https://meta.discourse.org/t/markdown-preview-and-result-differ/263878
The result of this markdown had different results in the composer preview and the post. This is solved by updating Loofah to the latest version and using html5 fragments like our user had reported. While the change was only needed in cooked_post_processor.rb for this fix, other areas also had to be updated due to various side effects.
