Skip to content

Commit

Permalink
Merge pull request #904 from reamaze/remove-redundant-join
Browse files Browse the repository at this point in the history
tags_count only need to join on the taggable's table if using STI
  • Loading branch information
seuros authored Jun 30, 2018
2 parents 59dca73 + eb3967f commit f6c4646
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 8 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@ Each change should fall into categories that would affect whether the release is

As such, a _Feature_ would map to either major or minor. A _bug fix_ to a patch. And _misc_ is either minor or patch, the difference being kind of fuzzy for the purposes of history. Adding tests would be patch level.

### [6.0.1 / 2018-06-27](https://github.com/mbleigh/acts-as-taggable-on/compare/v6.0.0...v6.0.1)
* Fixes
* [@hengwoon tags_count only need to join on the taggable's table if using STI ](https://github.com/mbleigh/acts-as-taggable-on/pull/904)

### [6.0.0 / 2018-06-19](https://github.com/mbleigh/acts-as-taggable-on/compare/v5.0.0...v6.0.0)
* Breaking Changes
* [@Fodoj Drop support for Rails 4.2](https://github.com/mbleigh/acts-as-taggable-on/pull/887)
Expand Down
14 changes: 8 additions & 6 deletions lib/acts_as_taggable_on/taggable/collection.rb
Original file line number Diff line number Diff line change
Expand Up @@ -94,16 +94,18 @@ def all_tag_counts(options = {})
## Generate conditions:
options[:conditions] = sanitize_sql(options[:conditions]) if options[:conditions]

## Generate joins:
taggable_join = "INNER JOIN #{table_name} ON #{table_name}.#{primary_key} = #{ActsAsTaggableOn::Tagging.table_name}.taggable_id"
taggable_join << " AND #{table_name}.#{inheritance_column} = '#{name}'" unless descends_from_active_record? # Current model is STI descendant, so add type checking to the join condition

## Generate scope:
tagging_scope = ActsAsTaggableOn::Tagging.select("#{ActsAsTaggableOn::Tagging.table_name}.tag_id, COUNT(#{ActsAsTaggableOn::Tagging.table_name}.tag_id) AS tags_count")
tag_scope = ActsAsTaggableOn::Tag.select("#{ActsAsTaggableOn::Tag.table_name}.*, #{ActsAsTaggableOn::Tagging.table_name}.tags_count AS count").order(options[:order]).limit(options[:limit])

# Joins and conditions
tagging_scope = tagging_scope.joins(taggable_join)
# Current model is STI descendant, so add type checking to the join condition
unless descends_from_active_record?
taggable_join = "INNER JOIN #{table_name} ON #{table_name}.#{primary_key} = #{ActsAsTaggableOn::Tagging.table_name}.taggable_id"
taggable_join << " AND #{table_name}.#{inheritance_column} = '#{name}'"
tagging_scope = tagging_scope.joins(taggable_join)
end

# Conditions
tagging_conditions(options).each { |condition| tagging_scope = tagging_scope.where(condition) }
tag_scope = tag_scope.where(options[:conditions])

Expand Down
3 changes: 1 addition & 2 deletions lib/acts_as_taggable_on/version.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
module ActsAsTaggableOn
VERSION = '6.0.0'
VERSION = '6.0.1'
end

5 changes: 5 additions & 0 deletions spec/acts_as_taggable_on/single_table_inheritance_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,11 @@

inheriting_model.update_attributes! name: 'foo'
end

it "should only join with taggable's table to check type for inherited models" do
expect(TaggableModel.tag_counts_on(:tags).to_sql).to_not match /INNER JOIN taggable_models ON/
expect(InheritingTaggableModel.tag_counts_on(:tags).to_sql).to match /INNER JOIN taggable_models ON/
end
end

describe 'ownership' do
Expand Down

0 comments on commit f6c4646

Please sign in to comment.