From bfc31acbed18e17087a60c43899b200ab004bfdd Mon Sep 17 00:00:00 2001 From: evazion Date: Tue, 7 Jul 2020 12:47:18 -0500 Subject: [PATCH] artists: fix accidental gentag category changes. Bug: if you created an artist with the name of an existing general tag, then the gentag would be changed to an artist tag, no matter how big the gentag was. Now we only allow creating artist entries for non-artist tags if the tag is empty. Ref: https://danbooru.donmai.us/forum_topics/17095 --- app/models/artist.rb | 20 ++++++++++++++------ test/unit/artist_test.rb | 10 +++++----- 2 files changed, 19 insertions(+), 11 deletions(-) diff --git a/app/models/artist.rb b/app/models/artist.rb index 6f7675d4d..e40820fef 100644 --- a/app/models/artist.rb +++ b/app/models/artist.rb @@ -8,10 +8,12 @@ class Artist < ApplicationRecord before_validation :normalize_name before_validation :normalize_other_names - after_save :create_version - after_save :clear_url_string_changed validate :validate_tag_category validates :name, tag_name: true, uniqueness: true + before_save :update_tag_category + after_save :create_version + after_save :clear_url_string_changed + has_many :members, :class_name => "Artist", :foreign_key => "group_name", :primary_key => "name" has_many :urls, :dependent => :destroy, :class_name => "ArtistUrl", :autosave => true has_many :versions, -> {order("artist_versions.id ASC")}, :class_name => "ArtistVersion" @@ -151,14 +153,20 @@ class Artist < ApplicationRecord module TagMethods def validate_tag_category - return unless !is_deleted? && name_changed? + return unless !is_deleted? && name_changed? && tag.present? - if tag.category_name == "General" - tag.update(category: Tag.categories.artist) - elsif tag.category_name != "Artist" + if tag.category_name != "Artist" && !tag.empty? errors[:base] << "'#{name}' is a #{tag.category_name.downcase} tag; artist entries can only be created for artist tags" end end + + def update_tag_category + return unless !is_deleted? && name_changed? && tag.present? + + if tag.category_name != "Artist" && tag.empty? + tag.update!(category: Tag.categories.artist) + end + end end module BanMethods diff --git a/test/unit/artist_test.rb b/test/unit/artist_test.rb index 58cd234ea..d82beccca 100644 --- a/test/unit/artist_test.rb +++ b/test/unit/artist_test.rb @@ -62,8 +62,8 @@ class ArtistTest < ActiveSupport::TestCase context "that has been banned" do setup do - @post = FactoryBot.create(:post, :tag_string => "aaa") @artist = FactoryBot.create(:artist, :name => "aaa") + @post = FactoryBot.create(:post, :tag_string => "aaa") @admin = FactoryBot.create(:admin_user) @artist.ban!(banner: @admin) @post.reload @@ -407,9 +407,9 @@ class ArtistTest < ActiveSupport::TestCase end should "search on has_tag and return matches" do - post = FactoryBot.create(:post, tag_string: "bkub") bkub = FactoryBot.create(:artist, name: "bkub") none = FactoryBot.create(:artist, name: "none") + post = FactoryBot.create(:post, tag_string: "bkub") assert_equal(bkub.id, Artist.search(has_tag: "true").first.id) assert_equal(none.id, Artist.search(has_tag: "false").first.id) @@ -443,8 +443,8 @@ class ArtistTest < ActiveSupport::TestCase assert(Tag.exists?(name: "bkub", category: Tag.categories.artist)) end - should "change the tag to an artist tag if it was a gentag" do - tag = FactoryBot.create(:tag, name: "abc", category: Tag.categories.general) + should "change the tag to an artist tag if it was an empty gentag" do + tag = FactoryBot.create(:tag, name: "abc", category: Tag.categories.general, post_count: 0) artist = FactoryBot.create(:artist, name: "abc") assert_equal(Tag.categories.artist, tag.reload.category) @@ -461,7 +461,7 @@ class ArtistTest < ActiveSupport::TestCase context "when renaming" do should "change the new tag to an artist tag if it was a gentag" do - tag = FactoryBot.create(:tag, name: "def", category: Tag.categories.general) + tag = FactoryBot.create(:tag, name: "def", category: Tag.categories.general, post_count: 0) artist = FactoryBot.create(:artist, name: "abc") artist.update(name: "def")