diff --git a/app/logical/post_edit.rb b/app/logical/post_edit.rb index c0ddf7880..a84b7265d 100644 --- a/app/logical/post_edit.rb +++ b/app/logical/post_edit.rb @@ -42,7 +42,6 @@ class PostEdit def tag_names tag_names = current_tag_names + effective_added_tag_names - user_removed_tag_names tag_names = post.add_automatic_tags(tag_names) - tag_names = ::Tag.convert_cosplay_tags(tag_names) tag_names += ::Tag.automatic_tags_for(tag_names) tag_names += TagImplication.tags_implied_by(tag_names).map(&:name) tag_names.uniq.sort diff --git a/app/logical/tag_name_validator.rb b/app/logical/tag_name_validator.rb index 3a1341316..dfff576ca 100644 --- a/app/logical/tag_name_validator.rb +++ b/app/logical/tag_name_validator.rb @@ -46,12 +46,13 @@ class TagNameValidator < ActiveModel::EachValidator when "new", "search", "and", "or", "not" record.errors.add(attribute, "'#{value}' is a reserved name and cannot be used") when /\A(.+)_\(cosplay\)\z/i - # XXX don't allow aliases here? - tag_name = TagAlias.to_aliased([$1]).first - tag = Tag.find_by_name(tag_name) + tag_name = $1; + char_tag = Tag.find_by_name(tag_name) - if tag.present? && !tag.empty? && !tag.character? - record.errors.add(attribute, "#{tag_name} must be a character tag") + if char_tag.present? && char_tag.antecedent_alias.present? + record.errors.add(attribute, "'#{value}' is not allowed because '#{tag_name}' is aliased to '#{char_tag.antecedent_alias.consequent_name}'") + elsif char_tag.present? && !char_tag.empty? && !char_tag.character? + record.errors.add(attribute, "'#{value}' is not allowed because '#{tag_name}' is not a character tag") end end end diff --git a/app/models/tag.rb b/app/models/tag.rb index 59fdb11fd..79f1e058e 100644 --- a/app/models/tag.rb +++ b/app/models/tag.rb @@ -362,7 +362,7 @@ class Tag < ApplicationRecord def self.automatic_tags_for(names) tags = [] - tags += names.grep(/\A(.+)_\(cosplay\)\z/i) { TagAlias.to_aliased([$1]).first } + tags += names.grep(/\A(.+)_\(cosplay\)\z/i) { $1 } tags << "cosplay" if names.any?(/_\(cosplay\)\z/i) tags << "school_uniform" if names.any?(/_school_uniform\z/i) tags << "meme" if names.any?(/_\(meme\)\z/i) @@ -370,13 +370,6 @@ class Tag < ApplicationRecord end concerning :CosplayTagMethods do - class_methods do - def convert_cosplay_tags(tags) - cosplay_tags, other_tags = tags.partition {|tag| tag.match(/\A(.+)_\(cosplay\)\Z/) } - cosplay_tags.grep(/\A(.+)_\(cosplay\)\Z/) { "#{TagAlias.to_aliased([$1]).first}_(cosplay)" } + other_tags - end - end - def create_character_tag_for_cosplay_tag character_name = name.delete_suffix("_(cosplay)") Tag.find_or_create_by_name("char:#{character_name}") diff --git a/test/unit/bulk_update_request_test.rb b/test/unit/bulk_update_request_test.rb index 13ba17b6b..094c2b230 100644 --- a/test/unit/bulk_update_request_test.rb +++ b/test/unit/bulk_update_request_test.rb @@ -392,7 +392,6 @@ class BulkUpdateRequestTest < ActiveSupport::TestCase should "move the *_(cosplay) tag as well" do @post = create(:post, tag_string: "toosaka_rin_(cosplay)") @wiki = create(:wiki_page, title: "toosaka_rin_(cosplay)") - @ta = create(:tag_alias, antecedent_name: "toosaka_rin", consequent_name: "tohsaka_rin") create_bur!("rename toosaka_rin -> tohsaka_rin", @admin) @@ -401,6 +400,18 @@ class BulkUpdateRequestTest < ActiveSupport::TestCase end end + context "when aliasing a character tag with a *_(cosplay) tag" do + should "move the *_(cosplay) tag as well" do + @post = create(:post, tag_string: "toosaka_rin_(cosplay)") + @wiki = create(:wiki_page, title: "toosaka_rin_(cosplay)") + + create_bur!("alias toosaka_rin -> tohsaka_rin", @admin) + + assert_equal("cosplay tohsaka_rin tohsaka_rin_(cosplay)", @post.reload.tag_string) + assert_equal("tohsaka_rin_(cosplay)", @wiki.reload.title) + end + end + context "when renaming an artist tag with a *_(style) tag" do should "move the *_(style) tag as well" do create(:tag, name: "tanaka_takayuki", category: Tag.categories.artist) diff --git a/test/unit/post_test.rb b/test/unit/post_test.rb index 20028bdad..697febcf3 100644 --- a/test/unit/post_test.rb +++ b/test/unit/post_test.rb @@ -577,7 +577,11 @@ class PostTest < ActiveSupport::TestCase @post.add_tag("jim_(cosplay)") @post.save - assert(@post.has_tag?("james"), "expected 'jim' to be aliased to 'james'") + assert_equal(false, @post.has_tag?("jim_(cosplay)")) + assert_equal(false, @post.has_tag?("james_(cosplay)")) + assert_equal(false, @post.has_tag?("jim")) + assert_equal(false, @post.has_tag?("james")) + assert_match(/'jim_\(cosplay\)' is not allowed because 'jim' is aliased to 'james'/, @post.warnings.full_messages.join) end should "apply implications after the character tag is added" do @@ -1213,6 +1217,7 @@ class PostTest < ActiveSupport::TestCase refute(@post.has_tag?("little_red_riding_hood")) refute(@post.has_tag?("cosplay")) assert(@post.warnings[:base].grep(/Couldn't add tag/).present?) + assert_match(/'little_red_riding_hood_\(cosplay\)' is not allowed because 'little_red_riding_hood' is not a character tag/, @post.warnings.full_messages.join) end should "allow creating a _(cosplay) tag for an empty general tag" do diff --git a/test/unit/tag_test.rb b/test/unit/tag_test.rb index 9aea0f416..9b22b43f0 100644 --- a/test/unit/tag_test.rb +++ b/test/unit/tag_test.rb @@ -195,11 +195,13 @@ class TagTest < ActiveSupport::TestCase setup do create(:tag, name: "bkub", category: Tag.categories.artist) create(:tag, name: "fumimi", category: Tag.categories.character) + create(:tag_alias, antecedent_name: "orin", consequent_name: "kaenbyou_rin") end should allow_value("fumimi_(cosplay)").for(:name) should allow_value("new_tag_(cosplay)").for(:name) should_not allow_value("bkub_(cosplay)").for(:name) + should_not allow_value("orin_(cosplay)").for(:name) end end end