Skip to content

Commit

Permalink
Merge pull request mbleigh#178 from bernardkroes/master
Browse files Browse the repository at this point in the history
use of #{primary_key} instead of .id
  • Loading branch information
Artem Kramarenko committed Aug 23, 2011
2 parents 16c8c50 + 5dae123 commit 8a7a7a0
Show file tree
Hide file tree
Showing 7 changed files with 114 additions and 12 deletions.
6 changes: 3 additions & 3 deletions lib/acts_as_taggable_on/acts_as_taggable_on/core.rb
Expand Up @@ -21,7 +21,7 @@ def initialize_acts_as_taggable_on_core

class_eval do
has_many context_taggings, :as => :taggable, :dependent => :destroy, :include => :tag, :class_name => "ActsAsTaggableOn::Tagging",
:conditions => ["#{ActsAsTaggableOn::Tagging.table_name}.tag_id = #{ActsAsTaggableOn::Tag.table_name}.id AND #{ActsAsTaggableOn::Tagging.table_name}.context = ?", tags_type]
:conditions => ["#{ActsAsTaggableOn::Tagging.table_name}.tag_id = #{ActsAsTaggableOn::Tag.table_name}.#{ActsAsTaggableOn::Tag.primary_key} AND #{ActsAsTaggableOn::Tagging.table_name}.context = ?", tags_type]
has_many context_tags, :through => context_taggings, :source => :tag, :class_name => "ActsAsTaggableOn::Tag"
end

Expand Down Expand Up @@ -79,7 +79,7 @@ def tagged_with(tags, options = {})

if options.delete(:exclude)
tags_conditions = tag_list.map { |t| sanitize_sql(["#{ActsAsTaggableOn::Tag.table_name}.name #{like_operator} ?", t]) }.join(" OR ")
conditions << "#{table_name}.#{primary_key} NOT IN (SELECT #{ActsAsTaggableOn::Tagging.table_name}.taggable_id FROM #{ActsAsTaggableOn::Tagging.table_name} JOIN #{ActsAsTaggableOn::Tag.table_name} ON #{ActsAsTaggableOn::Tagging.table_name}.tag_id = #{ActsAsTaggableOn::Tag.table_name}.id AND (#{tags_conditions}) WHERE #{ActsAsTaggableOn::Tagging.table_name}.taggable_type = #{quote_value(base_class.name)})"
conditions << "#{table_name}.#{primary_key} NOT IN (SELECT #{ActsAsTaggableOn::Tagging.table_name}.taggable_id FROM #{ActsAsTaggableOn::Tagging.table_name} JOIN #{ActsAsTaggableOn::Tag.table_name} ON #{ActsAsTaggableOn::Tagging.table_name}.tag_id = #{ActsAsTaggableOn::Tag.table_name}.#{ActsAsTaggableOn::Tag.primary_key} AND (#{tags_conditions}) WHERE #{ActsAsTaggableOn::Tagging.table_name}.taggable_type = #{quote_value(base_class.name)})"

elsif options.delete(:any)
# get tags, drop out if nothing returned (we need at least one)
Expand Down Expand Up @@ -256,7 +256,7 @@ def save_tags

if old_taggings.present?
# Destroy old taggings:
ActsAsTaggableOn::Tagging.destroy_all :id => old_taggings.map(&:id)
ActsAsTaggableOn::Tagging.destroy_all "#{ActsAsTaggableOn::Tagging.primary_key}".to_sym => old_taggings.map(&:id)
end

# Create new taggings:
Expand Down
14 changes: 7 additions & 7 deletions lib/acts_as_taggable_on/acts_as_taggable_on/related.rb
Expand Up @@ -44,30 +44,30 @@ module InstanceMethods
def matching_contexts_for(search_context, result_context, klass, options = {})
tags_to_find = tags_on(search_context).collect { |t| t.name }

exclude_self = "#{klass.table_name}.id != #{id} AND" if self.class == klass
exclude_self = "#{klass.table_name}.#{klass.primary_key} != #{id} AND" if self.class == klass

group_columns = ActsAsTaggableOn::Tag.using_postgresql? ? grouped_column_names_for(klass) : "#{klass.table_name}.#{klass.primary_key}"

klass.scoped({ :select => "#{klass.table_name}.*, COUNT(#{ActsAsTaggableOn::Tag.table_name}.id) AS count",
klass.scoped({ :select => "#{klass.table_name}.*, COUNT(#{ActsAsTaggableOn::Tag.table_name}.#{ActsAsTaggableOn::Tag.primary_key}) AS count",
:from => "#{klass.table_name}, #{ActsAsTaggableOn::Tag.table_name}, #{ActsAsTaggableOn::Tagging.table_name}",
:conditions => ["#{exclude_self} #{klass.table_name}.id = #{ActsAsTaggableOn::Tagging.table_name}.taggable_id AND #{ActsAsTaggableOn::Tagging.table_name}.taggable_type = '#{klass.to_s}' AND #{ActsAsTaggableOn::Tagging.table_name}.tag_id = #{ActsAsTaggableOn::Tag.table_name}.id AND #{ActsAsTaggableOn::Tag.table_name}.name IN (?) AND #{ActsAsTaggableOn::Tagging.table_name}.context = ?", tags_to_find, result_context],
:conditions => ["#{exclude_self} #{klass.table_name}.#{klass.primary_key} = #{ActsAsTaggableOn::Tagging.table_name}.taggable_id AND #{ActsAsTaggableOn::Tagging.table_name}.taggable_type = '#{klass.to_s}' AND #{ActsAsTaggableOn::Tagging.table_name}.tag_id = #{ActsAsTaggableOn::Tag.table_name}.#{ActsAsTaggableOn::Tag.primary_key} AND #{ActsAsTaggableOn::Tag.table_name}.name IN (?) AND #{ActsAsTaggableOn::Tagging.table_name}.context = ?", tags_to_find, result_context],
:group => group_columns,
:order => "count DESC" }.update(options))
end

def related_tags_for(context, klass, options = {})
tags_to_find = tags_on(context).collect { |t| t.name }

exclude_self = "#{klass.table_name}.id != #{id} AND" if self.class == klass
exclude_self = "#{klass.table_name}.#{klass.primary_key} != #{id} AND" if self.class == klass

group_columns = ActsAsTaggableOn::Tag.using_postgresql? ? grouped_column_names_for(klass) : "#{klass.table_name}.#{klass.primary_key}"

klass.scoped({ :select => "#{klass.table_name}.*, COUNT(#{ActsAsTaggableOn::Tag.table_name}.id) AS count",
klass.scoped({ :select => "#{klass.table_name}.*, COUNT(#{ActsAsTaggableOn::Tag.table_name}.#{ActsAsTaggableOn::Tag.primary_key}) AS count",
:from => "#{klass.table_name}, #{ActsAsTaggableOn::Tag.table_name}, #{ActsAsTaggableOn::Tagging.table_name}",
:conditions => ["#{exclude_self} #{klass.table_name}.id = #{ActsAsTaggableOn::Tagging.table_name}.taggable_id AND #{ActsAsTaggableOn::Tagging.table_name}.taggable_type = '#{klass.to_s}' AND #{ActsAsTaggableOn::Tagging.table_name}.tag_id = #{ActsAsTaggableOn::Tag.table_name}.id AND #{ActsAsTaggableOn::Tag.table_name}.name IN (?)", tags_to_find],
:conditions => ["#{exclude_self} #{klass.table_name}.#{klass.primary_key} = #{ActsAsTaggableOn::Tagging.table_name}.taggable_id AND #{ActsAsTaggableOn::Tagging.table_name}.taggable_type = '#{klass.to_s}' AND #{ActsAsTaggableOn::Tagging.table_name}.tag_id = #{ActsAsTaggableOn::Tag.table_name}.#{ActsAsTaggableOn::Tag.primary_key} AND #{ActsAsTaggableOn::Tag.table_name}.name IN (?)", tags_to_find],
:group => group_columns,
:order => "count DESC" }.update(options))
end
end
end
end
end
32 changes: 32 additions & 0 deletions spec/acts_as_taggable_on/acts_as_taggable_on_spec.rb
Expand Up @@ -95,6 +95,24 @@
taggable1.find_related_tags.should_not include(taggable2)
end

it "should find related objects based on tag names on context - non standard id" do
taggable1 = NonStandardIdTaggableModel.create!(:name => "Taggable 1")
taggable2 = NonStandardIdTaggableModel.create!(:name => "Taggable 2")
taggable3 = NonStandardIdTaggableModel.create!(:name => "Taggable 3")

taggable1.tag_list = "one, two"
taggable1.save

taggable2.tag_list = "three, four"
taggable2.save

taggable3.tag_list = "one, four"
taggable3.save

taggable1.find_related_tags.should include(taggable3)
taggable1.find_related_tags.should_not include(taggable2)
end

it "should find other related objects based on tag names on context" do
taggable1 = TaggableModel.create!(:name => "Taggable 1")
taggable2 = OtherTaggableModel.create!(:name => "Taggable 2")
Expand Down Expand Up @@ -126,6 +144,20 @@
taggable1.find_related_tags.should include(taggable2)
taggable1.find_related_tags.should_not include(taggable1)
end

it "should not include the object itself in the list of related objects - non standard id" do
taggable1 = NonStandardIdTaggableModel.create!(:name => "Taggable 1")
taggable2 = NonStandardIdTaggableModel.create!(:name => "Taggable 2")

taggable1.tag_list = "one"
taggable1.save

taggable2.tag_list = "one, two"
taggable2.save

taggable1.find_related_tags.should include(taggable2)
taggable1.find_related_tags.should_not include(taggable1)
end
end

describe "Matching Contexts" do
Expand Down
56 changes: 56 additions & 0 deletions spec/acts_as_taggable_on/taggable_spec.rb
Expand Up @@ -296,6 +296,10 @@
it "should return all column names joined for TaggableModel GROUP clause" do
@taggable.grouped_column_names_for(TaggableModel).should == "taggable_models.id, taggable_models.name, taggable_models.type"
end

it "should return all column names joined for NonStandardIdTaggableModel GROUP clause" do
@taggable.grouped_column_names_for(TaggableModel).should == "taggable_models.#{TaggableModel.primary_key}, taggable_models.name, taggable_models.type"
end
end

describe "Single Table Inheritance" do
Expand Down Expand Up @@ -345,4 +349,56 @@
@inherited_same.update_attributes! :name => 'foo'
end
end

describe "NonStandardIdTaggable" do
before(:each) do
clean_database!
@taggable = NonStandardIdTaggableModel.new(:name => "Bob Jones")
@taggables = [@taggable, NonStandardIdTaggableModel.new(:name => "John Doe")]
end

it "should have tag types" do
[:tags, :languages, :skills, :needs, :offerings].each do |type|
NonStandardIdTaggableModel.tag_types.should include type
end

@taggable.tag_types.should == NonStandardIdTaggableModel.tag_types
end

it "should have tag_counts_on" do
NonStandardIdTaggableModel.tag_counts_on(:tags).all.should be_empty

@taggable.tag_list = ["awesome", "epic"]
@taggable.save

NonStandardIdTaggableModel.tag_counts_on(:tags).length.should == 2
@taggable.tag_counts_on(:tags).length.should == 2
end

it "should be able to create tags" do
@taggable.skill_list = "ruby, rails, css"
@taggable.instance_variable_get("@skill_list").instance_of?(ActsAsTaggableOn::TagList).should be_true

lambda {
@taggable.save
}.should change(ActsAsTaggableOn::Tag, :count).by(3)

@taggable.reload
@taggable.skill_list.sort.should == %w(ruby rails css).sort
end

it "should be able to create tags through the tag list directly" do
@taggable.tag_list_on(:test).add("hello")
@taggable.tag_list_cache_on(:test).should_not be_empty
@taggable.tag_list_on(:test).should == ["hello"]

@taggable.save
@taggable.save_tags

@taggable.reload
@taggable.tag_list_on(:test).should == ["hello"]
end
end
end


2 changes: 1 addition & 1 deletion spec/database.yml.sample
Expand Up @@ -16,4 +16,4 @@ postgresql:
username: postgres
password:
database: acts_as_taggable_on
encoding: utf8
encoding: utf8
11 changes: 10 additions & 1 deletion spec/models.rb
Expand Up @@ -32,4 +32,13 @@ class TaggableUser < ActiveRecord::Base

class UntaggableModel < ActiveRecord::Base
belongs_to :taggable_model
end
end

class NonStandardIdTaggableModel < ActiveRecord::Base
set_primary_key "an_id"
acts_as_taggable
acts_as_taggable_on :languages
acts_as_taggable_on :skills
acts_as_taggable_on :needs, :offerings
has_many :untaggable_models
end
5 changes: 5 additions & 0 deletions spec/schema.rb
Expand Up @@ -21,6 +21,11 @@
t.column :type, :string
end

create_table :non_standard_id_taggable_models, :primary_key => "an_id", :force => true do |t|
t.column :name, :string
t.column :type, :string
end

create_table :untaggable_models, :force => true do |t|
t.column :taggable_model_id, :integer
t.column :name, :string
Expand Down

0 comments on commit 8a7a7a0

Please sign in to comment.