diff --git a/lib/acts_as_taggable_on/taggable/core.rb b/lib/acts_as_taggable_on/taggable/core.rb index 237af58d8..bc7346513 100644 --- a/lib/acts_as_taggable_on/taggable/core.rb +++ b/lib/acts_as_taggable_on/taggable/core.rb @@ -100,6 +100,7 @@ def tagged_with(tags, options = {}) context = options.delete(:on) owned_by = options.delete(:owned_by) alias_base_name = undecorated_table_name.gsub('.', '_') + # FIXME use ActiveRecord's connection quote_column_name quote = ActsAsTaggableOn::Utils.using_postgresql? ? '"' : '' if options.delete(:exclude) @@ -117,7 +118,7 @@ def tagged_with(tags, options = {}) " AND #{ActsAsTaggableOn::Tagging.table_name}.taggable_type = #{quote_value(base_class.name, nil)}" + " AND #{ActsAsTaggableOn::Tagging.table_name}.tagger_id = #{quote_value(owned_by.id, nil)}" + " AND #{ActsAsTaggableOn::Tagging.table_name}.tagger_type = #{quote_value(owned_by.class.base_class.to_s, nil)}" - + joins << " AND " + sanitize_sql(["#{ActsAsTaggableOn::Tagging.table_name}.created_at >= ?", options.delete(:start_at)]) if options[:start_at] joins << " AND " + sanitize_sql(["#{ActsAsTaggableOn::Tagging.table_name}.created_at <= ?", options.delete(:end_at)]) if options[:end_at] end @@ -130,7 +131,7 @@ def tagged_with(tags, options = {}) ActsAsTaggableOn::Tag.named_any(tag_list) end - return empty_result unless tags.length > 0 + return empty_result if tags.length == 0 # setup taggings alias so we can chain, ex: items_locations_taggings_awesome_cool_123 # avoid ambiguous column name @@ -140,21 +141,22 @@ def tagged_with(tags, options = {}) "#{alias_base_name[0..4]}#{taggings_context[0..6]}_taggings_#{ActsAsTaggableOn::Utils.sha_prefix(tags.map(&:name).join('_'))}" ) - tagging_join = "JOIN #{ActsAsTaggableOn::Tagging.table_name} #{taggings_alias}" + - " ON #{taggings_alias}.taggable_id = #{quote}#{table_name}#{quote}.#{primary_key}" + + tagging_cond = "#{ActsAsTaggableOn::Tagging.table_name} #{taggings_alias}" + + " WHERE #{taggings_alias}.taggable_id = #{quote}#{table_name}#{quote}.#{primary_key}" + " AND #{taggings_alias}.taggable_type = #{quote_value(base_class.name, nil)}" - tagging_join << " AND " + sanitize_sql(["#{taggings_alias}.created_at >= ?", options.delete(:start_at)]) if options[:start_at] - tagging_join << " AND " + sanitize_sql(["#{taggings_alias}.created_at <= ?", options.delete(:end_at)]) if options[:end_at] + tagging_cond << " AND " + sanitize_sql(["#{taggings_alias}.created_at >= ?", options.delete(:start_at)]) if options[:start_at] + tagging_cond << " AND " + sanitize_sql(["#{taggings_alias}.created_at <= ?", options.delete(:end_at)]) if options[:end_at] - tagging_join << " AND " + sanitize_sql(["#{taggings_alias}.context = ?", context.to_s]) if context + tagging_cond << " AND " + sanitize_sql(["#{taggings_alias}.context = ?", context.to_s]) if context # don't need to sanitize sql, map all ids and join with OR logic - conditions << tags.map { |t| "#{taggings_alias}.tag_id = #{quote_value(t.id, nil)}" }.join(' OR ') + tag_ids = tags.map { |t| quote_value(t.id, nil) }.join(', ') + tagging_cond << " AND #{taggings_alias}.tag_id in (#{tag_ids})" select_clause << " #{table_name}.*" unless context and tag_types.one? if owned_by - tagging_join << ' AND ' + + tagging_cond << ' AND ' + sanitize_sql([ "#{taggings_alias}.tagger_id = ? AND #{taggings_alias}.tagger_type = ?", owned_by.id, @@ -162,10 +164,9 @@ def tagged_with(tags, options = {}) ]) end - joins << tagging_join - unless any == 'distinct' # Fix issue #544 - group = "#{table_name}.#{primary_key}" - select_clause << group + conditions << "EXISTS (SELECT 1 FROM #{tagging_cond})" + if options.delete(:order_by_matching_tag_count) + order_by << "(SELECT count(*) FROM #{tagging_cond}) desc" end else tags = ActsAsTaggableOn::Tag.named_any(tag_list) @@ -224,11 +225,11 @@ def tagged_with(tags, options = {}) query = self query = self.select(select_clause.join(',')) unless select_clause.empty? query.joins(joins.join(' ')) - .where(conditions.join(' AND ')) - .group(group) - .having(having) - .order(order_by.join(', ')) - .readonly(false) + .where(conditions.join(' AND ')) + .group(group) + .having(having) + .order(order_by.join(', ')) + .readonly(false) end def is_taggable? diff --git a/spec/acts_as_taggable_on/taggable_spec.rb b/spec/acts_as_taggable_on/taggable_spec.rb index 062ceaceb..beda3d231 100644 --- a/spec/acts_as_taggable_on/taggable_spec.rb +++ b/spec/acts_as_taggable_on/taggable_spec.rb @@ -261,7 +261,7 @@ @taggable.tag_list = 'bob, charlie' @taggable.save - expect(TaggableModel.group(:created_at).tagged_with(['bob', 'css'], :any => true).first).to eq(@taggable) + expect(TaggableModel.tagged_with(['bob', 'css'], :any => true).to_a).to eq([@taggable]) bob = TaggableModel.create(:name => 'Bob', :tag_list => 'ruby, rails, css') frank = TaggableModel.create(:name => 'Frank', :tag_list => 'ruby, rails')