From 065609ff4fa5bec4a8ca2c91e480c58118004524 Mon Sep 17 00:00:00 2001
From: evazion <noizave@gmail.com>
Date: Thu, 1 Aug 2019 20:11:27 -0500
Subject: [PATCH] artists: prevent creating artist entries for non-artist tags
 (#4107, 2717).

Validate that artist entries belong to an artist tag. Don't allow
creating artist entries for character/copyright/meta tags, but do allow
entries for general tags (the gentag will be automatically changed to an
artist tag).
---
 app/models/artist.rb     | 14 +++++++++-----
 test/unit/artist_test.rb | 40 ++++++++++++++++++++++++++++------------
 2 files changed, 37 insertions(+), 17 deletions(-)

diff --git a/app/models/artist.rb b/app/models/artist.rb
index 94f3666ec..0f0af0d2d 100644
--- a/app/models/artist.rb
+++ b/app/models/artist.rb
@@ -8,9 +8,9 @@ class Artist < ApplicationRecord
   before_validation :normalize_name
   before_validation :normalize_other_names
   after_save :create_version
-  after_save :categorize_tag
   after_save :update_wiki
   after_save :clear_url_string_changed
+  validate :validate_tag_category
   validates :name, tag_name: true, uniqueness: true
   belongs_to_creator
   has_many :members, :class_name => "Artist", :foreign_key => "group_name", :primary_key => "name"
@@ -18,7 +18,7 @@ class Artist < ApplicationRecord
   has_many :versions, -> {order("artist_versions.id ASC")}, :class_name => "ArtistVersion"
   has_one :wiki_page, :foreign_key => "title", :primary_key => "name"
   has_one :tag_alias, :foreign_key => "antecedent_name", :primary_key => "name"
-  has_one :tag, :foreign_key => "name", :primary_key => "name"
+  belongs_to :tag, foreign_key: "name", primary_key: "name", default: -> { Tag.new(name: name, category: Tag.categories.artist) }
   attribute :notes, :string
 
   scope :active, -> { where(is_active: true) }
@@ -405,9 +405,13 @@ class Artist < ApplicationRecord
       Tag.category_for(name)
     end
 
-    def categorize_tag
-      if new_record? || saved_change_to_name?
-        Tag.find_or_create_by_name("artist:#{name}")
+    def validate_tag_category
+      return unless is_active? && name_changed?
+
+      if tag.category_name == "General"
+        tag.update(category: Tag.categories.artist)
+      elsif tag.category_name != "Artist"
+        errors[:base] << "'#{name}' is a #{tag.category_name.downcase} tag; artist entries can only be created for artist tags"
       end
     end
   end
diff --git a/test/unit/artist_test.rb b/test/unit/artist_test.rb
index 9eea19b53..14ba8535e 100644
--- a/test/unit/artist_test.rb
+++ b/test/unit/artist_test.rb
@@ -469,20 +469,36 @@ class ArtistTest < ActiveSupport::TestCase
       assert_equal(%w[yyy], artist.other_names)
     end
 
-    should "update the category of the tag when created" do
-      tag = FactoryBot.create(:tag, :name => "abc")
-      artist = FactoryBot.create(:artist, :name => "abc")
-      tag.reload
-      assert_equal(Tag.categories.artist, tag.category)
+    context "when creating" do
+      should "create a new artist tag if one does not already exist" do
+        FactoryBot.create(:artist, name: "bkub")
+        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)
+        artist = FactoryBot.create(:artist, name: "abc")
+
+        assert_equal(Tag.categories.artist, tag.reload.category)
+      end
+
+      should "not allow creating artist entries for non-artist tags" do
+        tag = FactoryBot.create(:tag, name: "touhou", category: Tag.categories.copyright)
+        artist = FactoryBot.build(:artist, name: "touhou")
+
+        assert(artist.invalid?)
+        assert_match(/'touhou' is a copyright tag/, artist.errors.full_messages.join)
+      end
     end
 
-    should "update the category of the tag when renamed" do
-      tag = FactoryBot.create(:tag, :name => "def")
-      artist = FactoryBot.create(:artist, :name => "abc")
-      artist.name = "def"
-      artist.save
-      tag.reload
-      assert_equal(Tag.categories.artist, tag.category)
+    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)
+        artist = FactoryBot.create(:artist, name: "abc")
+        artist.update(name: "def")
+
+        assert_equal(Tag.categories.artist, tag.reload.category)
+      end
     end
 
     context "when saving" do