Skip to content

Commit

Permalink
Fixed major bug in Tag.with_type_scope
Browse files Browse the repository at this point in the history
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
bjeanes authored and Wesley Beary committed Jun 12, 2009
1 parent a22b9b9 commit 8d032f5
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion lib/tag.rb
Expand Up @@ -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
Expand Down

0 comments on commit 8d032f5

Please sign in to comment.