Validation problem?

[BenZhang]

Hi,

I am not quit sure whether this is a bug.

ActsAsTaggableOn::Tag.create(:name => "Awesome")
ActsAsTaggableOn::Tag.create(:name => "awesome")
@user = TaggableModel.create(:name => "Pablo")
@user.update_attributes skill_list: 'awesome, Awesome'
@user.reload.skill_list.count # => 0

When I look at @user.errors, it gives me nothing. Any suggestion would be appreciated.

[BenZhang]

I managed to repeat this by editing a test case. I have made some changes to make it work for my situation.
BenZhang@3f63dd6

[bf4]

@BenZhang Are you using the case insensitive setting? If not, that's a feature.

see https://github.com/mbleigh/acts-as-taggable-on#configuration

ActsAsTaggableOn.force_lowercase = true

What version of the gem are you on? Also ruby version, ruby engine, rails version, os platform...

[BenZhang]

@bf4 thanks for you suggestion. I realise this feature, but I don't want to convert all the users input to lower case. Digging into the source code, I've found it is caused by tagging duplication. I have fixed it by adding uniq to the tag array. Please review my commit whether it can be a pull request.

For example,

If a tag has been created already, say ruby, then when I do

@taggable.skill_list = "ruby, Ruby"

I found https://github.com/mbleigh/acts-as-taggable-on/blob/master/lib/acts_as_taggable_on/acts_as_taggable_on/core.rb#L345 would return

[<tag id=111 name="ruby">, <tag id=111 name="ruby">]

, which are two same tag. It will cause

taggings.create!(:tag_id => tag.id, :context => context.to_s, :taggable => self)

fails because of the uniqueness validation.

[bf4]

Would you mind writing a failing test for this?

[BenZhang]

@bf4 The failing test is included from my commit. https://github.com/BenZhang/acts-as-taggable-on/blob/3f63dd6daeaf295616dc3143cb1712ca7f8854c6/spec/acts_as_taggable_on/taggable_spec.rb#L176

https://github.com/BenZhang/acts-as-taggable-on/blob/3f63dd6daeaf295616dc3143cb1712ca7f8854c6/lib/acts_as_taggable_on/tag.rb#L79

existing_tag = existing_tags.detect { |tag| comparable_name(tag.name) == comparable_tag_name }

Looking into this line, even the ActsAsTaggableOn.strict_case_match is set to true, comparable_name will still convert tag name to downcase, therefore load_tags returns two same tags, 'ruby', instead of creating a new tag 'Ruby'.

[bf4]

@BenZhang You're right. I wonder why that is. That would appear to me as an untested bug.

[BenZhang]

@bf4 I guess this situation is quit rare. See the tests below.

before(:each) do
  @tag.name = "awesome"
  @tag.save
end

it "should find by name" do
  ActsAsTaggableOn::Tag.find_or_create_all_with_like_by_name("awesome").should == [@tag]
end
# => pass

it "should find by name case insensitive" do
  ActsAsTaggableOn::Tag.find_or_create_all_with_like_by_name("AWESOME").should == [@tag]
end
# => pass

it "should find by name case sensitive" do
  ActsAsTaggableOn.strict_case_match = true
   lambda {
     ActsAsTaggableOn::Tag.find_or_create_all_with_like_by_name("AWESOME")
    }.should change(ActsAsTaggableOn::Tag, :count).by(1)  
end
# => pass

it "should create by name" do
  lambda {
    ActsAsTaggableOn::Tag.find_or_create_all_with_like_by_name("epic")
  }.should change(ActsAsTaggableOn::Tag, :count).by(1)
end
# => pass

it "should find or create by name case sensitive" do
  ActsAsTaggableOn.strict_case_match = true
  lambda {
    ActsAsTaggableOn::Tag.find_or_create_all_with_like_by_name("AWESOME", 'awesome').map(&:name).should == ["AWESOME", "awesome"]
  }.should change(ActsAsTaggableOn::Tag, :count).by(1)
end
# => fail

It only happens when a user creates a new tag with the lower case tag already existed.
Do you mind I create a pull request to fix this bug. My project occurs this bug, I have already fixed it for my project. It would be great if you can review my commit BenZhang@1f79cca

[bf4]

Please!

[BenZhang]

Great. I have created a pull request. Please refer to here #440