Saving comma separated tags is incorrect

[jure]

Here's a mystery. I tried adding "WATFIV, WATFOR" as a tag, but it gets saved as WATFIV and WATFOR separately, even though tag_list correctly parses the string into 1 tag.

irb(main):079:0> jure.knowledge_list ='"WATFIV, WATFOR", hello'
=> "\"WATFIV, WATFOR\", hello"
irb(main):080:0> jure.knowledge_list
=> ["WATFIV, WATFOR", "hello"]
irb(main):081:0> jure.knowledge_list.size
=> 2
irb(main):082:0> jure.save
   (0.4ms)  BEGIN
  User Exists (0.6ms)  SELECT 1 AS one FROM "users" WHERE ("users"."name" = 'jure' AND "users"."id" != 2) LIMIT 1
   (0.6ms)  UPDATE "users" SET "updated_at" = '2012-10-16 11:28:14.610106' WHERE "users"."id" = 2
  ActsAsTaggableOn::Tag Load (5.8ms)  SELECT "tags".* FROM "tags" WHERE (lower(name) = 'watfiv' OR lower(name) = 'watfor' OR lower(name) = 'hello')
  ActsAsTaggableOn::Tag Load (0.7ms)  SELECT "tags".* FROM "tags" INNER JOIN "taggings" ON "tags"."id" = "taggings"."tag_id" WHERE "taggings"."taggable_id" = 2 AND "taggings"."taggable_type" = 'User' AND (taggings.context = 'knowledge' AND taggings.tagger_id IS NULL)
  ActsAsTaggableOn::Tagging Exists (0.5ms)  SELECT 1 AS one FROM "taggings" WHERE ("taggings"."tag_id" = 1977 AND "taggings"."taggable_type" = 'User' AND "taggings"."taggable_id" = 2 AND "taggings"."context" = 'knowledge' AND "taggings"."tagger_id" IS NULL AND "taggings"."tagger_type" IS NULL) LIMIT 1
  SQL (0.5ms)  INSERT INTO "taggings" ("context", "created_at", "tag_id", "taggable_id", "taggable_type", "tagger_id", "tagger_type") VALUES ($1, $2, $3, $4, $5, $6, $7) RETURNING "id"  [["context", "knowledge"], ["created_at", Tue, 16 Oct 2012 11:28:14 UTC +00:00], ["tag_id", 1977], ["taggable_id", 2], ["taggable_type", "User"], ["tagger_id", nil], ["tagger_type", nil]]
  ActsAsTaggableOn::Tagging Exists (0.5ms)  SELECT 1 AS one FROM "taggings" WHERE ("taggings"."tag_id" = 1978 AND "taggings"."taggable_type" = 'User' AND "taggings"."taggable_id" = 2 AND "taggings"."context" = 'knowledge' AND "taggings"."tagger_id" IS NULL AND "taggings"."tagger_type" IS NULL) LIMIT 1
  SQL (0.4ms)  INSERT INTO "taggings" ("context", "created_at", "tag_id", "taggable_id", "taggable_type", "tagger_id", "tagger_type") VALUES ($1, $2, $3, $4, $5, $6, $7) RETURNING "id"  [["context", "knowledge"], ["created_at", Tue, 16 Oct 2012 11:28:14 UTC +00:00], ["tag_id", 1978], ["taggable_id", 2], ["taggable_type", "User"], ["tagger_id", nil], ["tagger_type", nil]]
  ActsAsTaggableOn::Tagging Exists (0.5ms)  SELECT 1 AS one FROM "taggings" WHERE ("taggings"."tag_id" = 1979 AND "taggings"."taggable_type" = 'User' AND "taggings"."taggable_id" = 2 AND "taggings"."context" = 'knowledge' AND "taggings"."tagger_id" IS NULL AND "taggings"."tagger_type" IS NULL) LIMIT 1
  SQL (0.4ms)  INSERT INTO "taggings" ("context", "created_at", "tag_id", "taggable_id", "taggable_type", "tagger_id", "tagger_type") VALUES ($1, $2, $3, $4, $5, $6, $7) RETURNING "id"  [["context", "knowledge"], ["created_at", Tue, 16 Oct 2012 11:28:14 UTC +00:00], ["tag_id", 1979], ["taggable_id", 2], ["taggable_type", "User"], ["tagger_id", nil], ["tagger_type", nil]]
  ActsAsTaggableOn::Tag Load (0.7ms)  SELECT "tags".* FROM "tags" INNER JOIN "taggings" ON "tags"."id" = "taggings"."tag_id" WHERE "taggings"."taggable_id" = 2 AND "taggings"."taggable_type" = 'User' AND (taggings.context = 'interests' AND taggings.tagger_id IS NULL)
   (0.6ms)  COMMIT
=> true
irb(main):083:0> jure.reload
  User Load (1.5ms)  SELECT "users".* FROM "users" WHERE "users"."id" = $1 LIMIT 1  [["id", 2]]
=> #<User id: 2, email: "FILTERED", encrypted_password: "FILTERED", reset_password_token: nil, reset_password_sent_at: nil, remember_created_at: nil, sign_in_count: 2, current_sign_in_at: "2012-10-15 00:48:02", last_sign_in_at: "2012-10-14 23:54:19", current_sign_in_ip: "193.77.90.229", last_sign_in_ip: "FILTERED", created_at: "2012-10-14 23:54:19", updated_at: "2012-10-16 11:28:14", name: "jure">
irb(main):084:0> jure.knowledge_list
  ActsAsTaggableOn::Tag Load (1.2ms)  SELECT "tags".* FROM "tags" INNER JOIN "taggings" ON "tags"."id" = "taggings"."tag_id" WHERE "taggings"."taggable_id" = 2 AND "taggings"."taggable_type" = 'User' AND (taggings.context = 'knowledge' AND taggings.tagger_id IS NULL)
=> ["Watfiv", "watfor", "Hello"]
irb(main):085:0> jure.knowledge_list.size
=> 3

So even though the knowledge_list reader correctly parses the tags, the .save calls something else that parses the string incorrectly and splits the tags on that comma. Any ideas? I'll dig a bit deeper myself.

[tilsammans]

It seems you need to pass an option in order to parse tags with commas:

jure.knowledge_list ='"WATFIV, WATFOR", hello', :parse => true

but it doesn't seem to be documented anywhere and I think it's confusing. But does it work for you?

[jure]

@tilsammans This is not valid Ruby?

My issue also isn't that the tags get parsed wrongly (they are), but that once the object they get re-parsed instead of remaining as they were, when the tags were set. It has to do with saving the tags to the database, if they contain a comma.

I'll look into it more when I have time, but it seems that tags generally should just not contain commas in any case :)

[tilsammans]

Well, it should be fine. I think the logic goes like this:

jure.knowledge_list = "these, are, normal, tags"
jure.knowledge_list = '"this, is, a, single, tag", and, these, are separate'

So, the split is always by delimiter, unless there are quotes, in which case the quotes win.

But I see not a lot of test cases testing this behavior. So, it would be good to start adding failing specs. Are you in a position to add specs? If not, could you supply me with real IRB output that demonstrates the bugs? If you can please include the creation of the models too and leave out the database output. I am sure I can at least fix the most egregious bugs still in 2.4.x.

[jure]

Here's a single failing spec. Hope that helps. If you don't have time, I'll see what's up over the weekend.

[ches]

I think #386 is the valid Ruby you were looking for 😉

[bf4]

Would you mind rebasing against master and force-pushing? Thanks

[jure]

Done. Hope this helps you. Still fails on current master:

  1) Taggable NonStandardIdTaggable should be able to create tags containing a comma
     Failure/Error: @taggable.skill_list.should == ['Watfiv, watfor', 'hello']
       expected: ["Watfiv, watfor", "hello"]
            got: ["Watfiv", "watfor", "hello"] (using ==)

[bf4]

I believe this is a duplicate of #394

[mshappe]

Assuming this is still an open issue, I should point out that the documentation suggests that

foo_list = "bar, baz, quux"

should parse the tags out. It does not, however, say what happens when you mix and match both a CSV-string and an array of additional values, as in the OP above:

foo_list = "bar, baz, quux", quuux, quuuux

I personally think it's perfectly reasonable to assume that this does in fact mean to treat "bar, baz, quux" as a single tag with commas in it (that is, that we're adding an array of 3 tags, not 5). So I'm not really sure there's a bug here at all.

[jure]

I wish it would do that. In your example, i.e.:

foo_list = '"bar, baz, quux", quuux, quuuux'

You would get tags ['bar', 'baz', 'quux', 'quuux', 'quuuux'] (5) instead of ['bar, baz, quux', 'quuux', 'quuux'] (3).

I think that's a bug and what you're describing is what I think the correct behavior should be.

[mshappe]

@jure -- Not quite sure we're speaking to the same example, really. You've just introduced another case!

Case #1: setting only an array

foo_list = ['bar', 'baz', 'quux', 'quuux', 'quuuux']

or

foo_list = 'bar', 'baz', 'quux', 'quuux', 'quuuux'

Will add five tags.

Case #2: Add just a string

foo_list = 'bar, baz, quux, quuux, quuuux'

Will add five tags

**Case #3: Adding a CSV string as part of an array

foo_list = 'bar, baz, quux', 'quuux', 'quuuux'

or, to be a bit more syntactically clear

foo_list = ['bar, baz, quux', 'quuux', 'quuuux']

Will add three tags: 'bar, baz, quux', 'quuux', and 'quuuux'

**Case #4: Adding a CSV within a CSV

foo_list = "'bar, baz, quux', quuux, quuuux"

Personally, I would expect this to behave exactly like Case #3 above -- three tags, one of which contains commas.

[jure]

We are. In the bug I reported above, case #3 results in 5 tags (after .save and .reload). Before the .save and .reload it parses it correctly, i.e. into 3 tags, one of which contains commas. But they get saved as 5. I described this in my initial comment.

[mshappe]

@jure OK. I get it now. Thanks for clarifying. I agree that that is a bug (or if it isn't, the documentation should be updated to clarify all four cases, but I think it is).

I'm taking an interest in the gem since I'm giving serious thought to using it in a project. If time allows, I may tackle this issue.

[jure]

Cool, I would appreciate it!

[bf4]

@jure would you mind creating a failing test for this issue? And update the issue with a clearer description of the bug, as discussion here? And include an example in the README?