Permalink
Browse files

Fixed major bug in Tag.with_type_scope

Because we are doing a lot of joins and selecting all columns, ActiveRecord was using the ID from one of the other tables in some cases (at least on SQLite). You can see that in the example below it is using the ID from Tagging.

e.g. (before fix then after)

SQL

$ script/dbconsole
sqlite> SELECT distinct * FROM "tags" left outer join taggings on taggings.tag_id = tags.id WHERE ("tags"."id" = 4) AND (taggable_type = 'Message') GROUP BY name ORDER BY name ASC;
4|wtf|1|5|4|9|Message|
sqlite> SELECT distinct tags.* FROM "tags" left outer join taggings on taggings.tag_id = tags.id WHERE ("tags"."id" = 4) AND (taggable_type = 'Message') GROUP BY name ORDER BY name ASC;
4|wtf|1

ActiveRecord

$ script/console
>> # BEFORE
>> Tag.with_type_scope('Comment') do
?>   Tag.find(4)
>> end
=> #<Tag id: 5, name: "wtf", taggings_count: 1>
>>
?> #AFTER
>> Tag.with_type_scope('Comment') do
?>   Tag.find(4)
>> end
=> #<Tag id: 4, name: "wtf", taggings_count: 1>

>> Tagging.find(5)
=> #<Tagging id: 5, tag_id: 4, taggable_id: 9, taggable_type: "Message", user_id: nil>

Signed-off-by: Wesley Beary <me@geemus.com>
  • Loading branch information...
1 parent a22b9b9 commit 8d032f5ac86a36d31478b384226b715192614d5e @bjeanes bjeanes committed with geemus Jun 12, 2009
Showing with 1 addition and 1 deletion.
  1. +1 −1 lib/tag.rb
View
@@ -41,7 +41,7 @@ def self.with_type_scope(taggable_type, user = nil)
if taggable_type
conditions = sanitize_sql(["taggable_type = ?", taggable_type])
conditions += sanitize_sql([" AND #{Tagging.table_name}.user_id = ?", user.id]) if user
- with_scope(:find => {:select => 'distinct *', :joins => "left outer join #{Tagging.table_name} on #{Tagging.table_name}.tag_id = #{Tag.table_name}.id", :conditions => conditions, :group => "name"}) { yield }
+ with_scope(:find => {:select => "distinct #{Tag.table_name}.*", :joins => "left outer join #{Tagging.table_name} on #{Tagging.table_name}.tag_id = #{Tag.table_name}.id", :conditions => conditions, :group => "name"}) { yield }
else
yield
end

0 comments on commit 8d032f5

Please sign in to comment.