L'importation CSV de balises / TagGroup respecte la casse, mais ce n'est pas le cas

Bug
1

Reproduction :

  • Avoir une balise nommée ABC
  • Créer un CSV avec la balise abc
  • Aller sur /tags et téléverser le CSV
  • Voir l’échec avec « Désolé, une erreur s’est produite lors du téléversement de foo.csv. Veuillez réessayer. »

Cela se produit car les balises sont sensibles à la casse mais doivent être uniques en ignorant la casse.

Si vous avez un fichier avec 1000 balises, il est assez difficile de déterminer où cela pose problème.

Le code fait ceci

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)

ce qui est un mauvais schéma, puisque find_by_name est sensible à la casse et create! ne l’est pas [^1], il échoue donc avec ActiveRecord::RecordInvalid : Validation failed: Name has already been taken

Cela devrait ressembler à ceci :

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

[^1] : parce que validates :name, presence: true, uniqueness: { case_sensitive: false }

3 « J'aime »
2

Merci d’avoir signalé, ajoutez un pr-welcome à cela, je suppose que nous devons changer ce find_by_name pour effectuer une recherche insensible à la casse.

Étant donné que nous avons :

La correction serait :

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

TagGroup n’a pas l’index, nous devrons donc l’ajouter et faire de même là-bas.

4

Salut @RGJ

Je n’ai pas pu reproduire votre problème avec une balise normale mais j’ai obtenu l’erreur que vous avez mentionnée avec un groupe de balises.

Voici mon exemple de CSV :

ABC,TAGGROUP

ABC fonctionne bien, mais avec le groupe de balises, cela génère une erreur. Si vous pouviez partager un exemple de CSV, je serais ravi de vous aider !

J’ai créé une PR pour corriger le problème du groupe de balises. De plus, SiteSetting.force_lowercase_tags est-il activé sur votre forum ?

1 « J'aime »
5

Non, il est décoché. Désolé, je vois maintenant que cela a de l’importance.

Je ne suis pas sûr de la façon dont vous essayez de reproduire le problème, mais le CSV devrait contenir un abc en minuscules et force_lowercase_tags devrait être décoché.

1 « J'aime »
6

Pas de souci, c’était un paramètre du site que j’ai trouvé en fouillant dans le code, cela pourrait être important !

Votre CSV ne contient que le abc sans groupes d’étiquettes ?

Notre exemple de test est (et j’ai testé localement, avec capitaltag2 en majuscules, mais cela a fonctionné comme prévu) :

7

Je ne peux effectivement pas reproduire avec une balise seule.

J’ai fait une petite erreur dans mon message initial :

Tag.find_by_name ne tient pas compte de la casse (bien) et Tag.find_by(name: ) tient compte de la casse (mal).
Ceci est dû au fait que David l’a déjà corrigé pour les balises il y a 7 ans en surchargeant la méthode find_by_name.

Pour TagGroup, cette correction n’a jamais été appliquée.

2 « J'aime »