mbleigh / acts-as-taggable-on

A tagging plugin for Rails applications that allows for custom tagging along dynamic contexts.
http://mbleigh.lighthouseapp.com/projects/10116-acts-as-taggable-on
MIT License
4.96k stars 1.19k forks source link

Add support for a list of taggable IDs in the tagging conditions #1053

Closed moliver-hemasystems closed 2 years ago

moliver-hemasystems commented 2 years ago

In the event of trying to fetch tags for multiple records, where the IDs of those records are already known, adding support for the :id option to accept an array of IDs to use directly, instead of using a subquery to find the same list of IDs.

I had run into a situation, where I was trying to get the tags for a series of records associated to the ones I was listing, and SQL resulting from calling tag_counts_on was using a subquery to fetch the IDs that were already being used in the query chain.

For example, say I was listing cases for multiple projects, and wanted to be able to filter those cases by tags on the projects. If I did:

Project.where(id: cases.distinct.pluck(:project_id)).tag_counts_on(:tags)

... it would result in a SQL that used the following sub-query:

AND taggings.taggable_id IN (
  SELECT "projects"."id"
  FROM "projects"
  WHERE "projects"."id" IN ({list of IDs})
)

This is causing the database to do extra work. There's an option that can be used to pass in an ID to tag_counts_on, but it currently only works with a single ID. This PR aims to extend that option to support an array of IDs as well, so the following could be used:

Project.tag_counts_on(:tags, id: cases.distinct.pluck(:project_id))

... and have the generated SQL be this instead:

AND taggings.taggable_id IN ({list of IDs})
seuros commented 2 years ago

@moliver-hemasystems Thank you for your contribution, can you rebase your PR ? I will release it in version 9.0.0

moliver-hemasystems commented 2 years ago

I rebased the PR. I didn't think of it at the time, but did you want me to adjust the changelog/version?

seuros commented 2 years ago

Yes, please.