Импорт CSV для тегов/групп тегов учитывает регистр, но на самом деле нет

Внести вклад Ошибка
1

Воспроизведение:

  • Есть тег с названием ABC
  • Создайте CSV-файл с тегом abc
  • Перейдите на страницу /tags и загрузите CSV-файл
  • Вы увидите ошибку: «Извините, произошла ошибка при загрузке foo.csv. Пожалуйста, попробуйте снова».

Это происходит потому, что теги учитывают регистр, но должны быть уникальными без учёта регистра.

Если у вас файл с 1000 тегами, найти место ошибки довольно трудоёмко.

В коде используется следующая логика:

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)

Это плохой паттерн, поскольку find_by_name чувствителен к регистру, а create! — нет [1], из-за чего возникает ошибка ActiveRecord::RecordInvalid : Validation failed: Name has already been taken.

Следовало бы использовать что-то вроде:

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

  1. потому что validates :name, presence: true, uniqueness: { case_sensitive: false } ↩︎

4 лайка
2

Спасибо за отчет, поставьте метку pr-welcome на этот запрос. Похоже, нам нужно изменить метод find_by_name, чтобы он выполнял поиск без учета регистра.

Учитывая, что у нас есть:

Исправление будет следующим:

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

В TagGroup отсутствует индекс, поэтому нам нужно добавить его, а затем сделать то же самое там.

4

Привет @RGJ

Мне не удалось воспроизвести вашу проблему с обычным тегом, но я получил ошибку, о которой вы упоминали, при использовании группы тегов.

Вот мой пример CSV:

ABC,TAGGROUP

ABC работает нормально, но с tag_group возникает ошибка. Если вы сможете предоставить пример CSV-файла, я с радостью помогу!

Я создал PR, исправляющий проблему с группами тегов. Также включена ли в вашем форуме настройка SiteSetting.force_lowercase_tags?

1 лайк
5

Нет, она не отмечена. Извините, теперь я понимаю, в чём дело.

Не совсем понятно, как вы пытаетесь воспроизвести проблему, но в CSV должен быть строчный abc, а настройка force_lowercase_tags должна быть снята.

1 лайк
6

Ничего страшного, это была настройка сайта, которую я обнаружил, изучая код, она может быть важной!

Ваш CSV содержит только abc без групп тегов?

Наш тестовый пример таков (и я тестировал локально, используя capitaltag2 в верхнем регистре, и это работало как ожидалось):

7

Действительно, воспроизвести проблему только по тегу не удалось.

Я допустил небольшую ошибку в своём первом сообщении:

Tag.find_by_name регистронезависим (хорошо), а Tag.find_by(name: ) регистрозависим (плохо).
Это связано с тем, что Дэвид уже исправил это для тегов 7 лет назад, переопределив метод find_by_name.

Для TagGroup это исправление так и не было применено.

2 лайка