Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hook to support STI subclasses of Tag in save_tags #413

Merged
merged 2 commits into from
Apr 18, 2014
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 27 additions & 1 deletion lib/acts_as_taggable_on/acts_as_taggable_on/core.rb
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ def save_tags
tag_list = tag_list_cache_on(context).uniq

# Find existing tags or create non-existing tags:
tags = load_tags(tag_list)
tags = find_or_create_tags_from_list_with_context(tag_list, context)

# Tag objects for currently assigned tags
current_tags = tags_on(context)
Expand Down Expand Up @@ -394,5 +394,31 @@ def save_tags

true
end

private

##
# Override this hook if you wish to subclass {ActsAsTaggableOn::Tag} --
# context is provided so that you may conditionally use a Tag subclass
# only for some contexts.
#
# @example Custom Tag class for one context
# class Company < ActiveRecord::Base
# acts_as_taggable_on :markets, :locations
#
# def find_or_create_tags_from_list_with_context(tag_list, context)
# if context.to_sym == :markets
# MarketTag.find_or_create_all_with_like_by_name(tag_list)
# else
# super
# end
# end
#
# @param [Array<String>] tag_list Tags to find or create
# @param [Symbol] context The tag context for the tag_list
def find_or_create_tags_from_list_with_context(tag_list, context)
load_tags(tag_list)
end
end
end

2 changes: 1 addition & 1 deletion lib/acts_as_taggable_on/acts_as_taggable_on/ownership.rb
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ def save_owned_tags
cached_owned_tag_list_on(context).each do |owner, tag_list|

# Find existing tags or create non-existing tags:
tags = ActsAsTaggableOn::Tag.find_or_create_all_with_like_by_name(tag_list.uniq)
tags = find_or_create_tags_from_list_with_context(tag_list.uniq, context)

# Tag objects for owned tags
owned_tags = owner_tags_on(owner, context)
Expand Down
4 changes: 2 additions & 2 deletions lib/acts_as_taggable_on/tag.rb
Original file line number Diff line number Diff line change
Expand Up @@ -71,13 +71,13 @@ def self.find_or_create_all_with_like_by_name(*list)

return [] if list.empty?

existing_tags = Tag.named_any(list)
existing_tags = named_any(list)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, this is definitely a good call. I missed that this was a subclassing issue unrelated to the STI issue.


list.map do |tag_name|
comparable_tag_name = comparable_name(tag_name)
existing_tag = existing_tags.find { |tag| comparable_name(tag.name) == comparable_tag_name }
begin
existing_tag || Tag.create(name: tag_name)
existing_tag || create(name: tag_name)
rescue ActiveRecord::RecordNotUnique
# Postgres aborts the current transaction with
# PG::InFailedSqlTransaction: ERROR: current transaction is aborted, commands ignored until end of transaction block
Expand Down
27 changes: 27 additions & 0 deletions spec/acts_as_taggable_on/single_table_inheritance_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -185,4 +185,31 @@
end
end

describe 'a subclass of Tag' do
let(:company) { Company.new(:name => 'Dewey, Cheatham & Howe') }
let(:user) { User.create! }

subject { Market.create! :name => 'finance' }

its(:type) { should eql 'Market' }

it 'sets STI type through string list' do
company.market_list = 'law, accounting'
company.save!
expect(Market.count).to eq(2)
end

it 'does not interfere with a normal Tag context on the same model' do
company.location_list = 'cambridge'
company.save!
ActsAsTaggableOn::Tag.where(name: 'cambridge', type: nil).should_not be_empty
end

it 'is returned with proper type through ownership' do
user.tag(company, :with => 'ripoffs, rackets', :on => :markets)
tags = company.owner_tags_on(user, :markets)
tags.all? { |tag| tag.is_a? Market }.should be_truthy
end
end
end

5 changes: 4 additions & 1 deletion spec/acts_as_taggable_on/taggable_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -525,7 +525,10 @@

describe 'grouped_column_names_for method' do
it 'should return all column names joined for Tag GROUP clause' do
expect(@taggable.grouped_column_names_for(ActsAsTaggableOn::Tag)).to eq('tags.id, tags.name, tags.taggings_count')
# NOTE: type column supports an STI Tag subclass in the test suite, though
# isn't included by default in the migration generator
expect(@taggable.grouped_column_names_for(ActsAsTaggableOn::Tag)).
to eq('tags.id, tags.name, tags.taggings_count, tags.type')
end

it 'should return all column names joined for TaggableModel GROUP clause' do
Expand Down
19 changes: 19 additions & 0 deletions spec/internal/app/models/models.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,25 @@ class AlteredInheritingTaggableModel < TaggableModel
acts_as_taggable_on :parts
end

class Market < ActsAsTaggableOn::Tag
end

class Company < ActiveRecord::Base
acts_as_taggable_on :locations, :markets

has_many :markets, :through => :market_taggings, :source => :tag

private

def find_or_create_tags_from_list_with_context(tag_list, context)
if context.to_sym == :markets
Market.find_or_create_all_with_like_by_name(tag_list)
else
super
end
end
end

class User < ActiveRecord::Base
acts_as_tagger
end
Expand Down
5 changes: 5 additions & 0 deletions spec/internal/db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
create_table :tags, force: true do |t|
t.string :name
t.integer :taggings_count, default: 0
t.string :type
end
add_index 'tags', ['name'], name: 'index_tags_on_name', unique: true

Expand Down Expand Up @@ -55,6 +56,10 @@
t.column :cached_glass_list, :string
end

create_table :companies, force: true do |t|
t.column :name, :string
end

create_table :users, force: true do |t|
t.column :name, :string
end
Expand Down