Tag / TagGroup CSV import is case aware, but it's not

Bug
1

Reproduction:

  • Have a tag called ABC
  • Create a CSV with tag abc
  • Go to /tags and upload CSV
  • See it fail with “Sorry, there was an error uploading foo.csv. Please try again.”

This is happening because tags are case aware but they must be unique ignoring the case.

If you have a file with 1000 tags it is quite a job to figure out where it goes wrong.

The code does this

tag = Tag.find_by_name(tag_name) || Tag.create!(name: tag_name)
...
tag_group = TagGroup.find_by(name: tag_group_name) || TagGroup.create!(name: tag_group_name)

which is a bad pattern, since find_by_name is case sensitive and create! is not [1] so it errors out with ActiveRecord::RecordInvalid : Validation failed: Name has already been taken

This should be something like

tag = Tag.where('name ILIKE ?', tag_name).first || Tag.create!(name: tag_name)

  1. because validates :name, presence: true, uniqueness: { case_sensitive: false } ↩︎

3 Likes
2

Thanks for reporting, put a pr-welcome on this, I guess we need to change that find_by_name to do a case insensitive lookup.

Given we have:

The fix would be:

tag = Tag.where('lower(name) = ?', tag_name.downcase).first

TagGroup is missing the index, so we would need to add it and then do the same there.

4

Hey @RGJ

I could not reproduce your issue with a regular tag but I got the error you mentioned with a tag group

This is my sample CSV:

ABC,TAGGROUP

ABC works fine, but with the tag_group, it would error. If you could share a sample CSV, I’m more than happy to help!

I’ve made a PR fixing the tag group issue. Also, is SiteSetting.force_lowercase_tags enabled in your forum?

1 Like
5

No, it’s unchecked. Sorry, I now see that matters.

Not sure how you’re trying to repro, but the CSV should have a lowercase abc and force_lowercase_tags should be unchecked.

1 Like
6

No worries, it was a site setting that I’ve found while digging in the code, it might be important!

Your CSV is just the abc without tag groups ?

Our testing example is(and I’ve tested locally, having capitaltag2 all caps, but it did work as expected):

7

I indeed cannot reproduce with a tag alone.

I made a small mistake in my OP:

Tag.find_by_name is case insensitive (good) and Tag.find_by(name: ) is case sensitive (bad).
This is because David already fixed it for tags 7 years ago by overriding the find_by_name method.

For TagGroup, that fix was never applied.

2 Likes