Merge pull request #570 from stiff/master

Fixed generated SQL for tagged_with :any

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,32 +141,32 @@ 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,
                                  owned_by.class.base_class.to_s
                              ])
           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?

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')