From 47f233003c4378b84bdfc2822c4219e3303adb63 Mon Sep 17 00:00:00 2001
From: evazion <noizave@gmail.com>
Date: Mon, 27 Apr 2020 13:57:19 -0500
Subject: [PATCH] Fix #4424: Cross-category alias incorrectly changes
 destination category.

The rule now is that if one of the tags is a general tag then its
category will be changed to match the other tag.
---
 app/models/tag_alias.rb     |  6 ++++--
 test/unit/tag_alias_test.rb | 41 ++++++++++++++++++++++++++-----------
 2 files changed, 33 insertions(+), 14 deletions(-)

diff --git a/app/models/tag_alias.rb b/app/models/tag_alias.rb
index 0d0fa9b4f..b72f8ebac 100644
--- a/app/models/tag_alias.rb
+++ b/app/models/tag_alias.rb
@@ -91,8 +91,10 @@ class TagAlias < TagRelationship
   end
 
   def ensure_category_consistency
-    if antecedent_tag.category != consequent_tag.category && antecedent_tag.category != Tag.categories.general
-      consequent_tag.update_attribute(:category, antecedent_tag.category)
+    if antecedent_tag.category == Tag.categories.general && consequent_tag.category != Tag.categories.general
+      antecedent_tag.update!(category: consequent_tag.category)
+    elsif consequent_tag.category == Tag.categories.general && antecedent_tag.category != Tag.categories.general
+      consequent_tag.update!(category: antecedent_tag.category)
     end
   end
 
diff --git a/test/unit/tag_alias_test.rb b/test/unit/tag_alias_test.rb
index e0cad9af8..43f9d6bd9 100644
--- a/test/unit/tag_alias_test.rb
+++ b/test/unit/tag_alias_test.rb
@@ -180,23 +180,40 @@ class TagAliasTest < ActiveSupport::TestCase
       assert_equal("ccc", ti.reload.consequent_name)
     end
 
-    should "not push the antecedent's category to the consequent if the antecedent is general" do
-      tag1 = FactoryBot.create(:tag, :name => "aaa")
-      tag2 = FactoryBot.create(:tag, :name => "bbb", :category => 1)
-      ta = FactoryBot.create(:tag_alias, :antecedent_name => "aaa", :consequent_name => "bbb")
-      tag2.reload
-      assert_equal(1, tag2.category)
-    end
-
-    should "push the antecedent's category to the consequent" do
-      tag1 = FactoryBot.create(:tag, :name => "aaa", :category => 1)
-      tag2 = FactoryBot.create(:tag, :name => "bbb", :category => 0)
-      ta = FactoryBot.create(:tag_alias, :antecedent_name => "aaa", :consequent_name => "bbb")
+    should "push the consequent's category to the antecedent if the antecedent is general" do
+      tag1 = create(:tag, name: "general", category: 0)
+      tag2 = create(:tag, name: "artist", category: 1)
+      ta = create(:tag_alias, antecedent_name: "general", consequent_name: "artist")
 
       ta.approve!(approver: @admin)
       perform_enqueued_jobs
 
+      assert_equal(1, tag1.reload.category)
       assert_equal(1, tag2.reload.category)
     end
+
+    should "push the antecedent's category to the consequent if the consequent is general" do
+      tag1 = create(:tag, name: "artist", category: 1)
+      tag2 = create(:tag, name: "general", category: 0)
+      ta = create(:tag_alias, antecedent_name: "artist", consequent_name: "general")
+
+      ta.approve!(approver: @admin)
+      perform_enqueued_jobs
+
+      assert_equal(1, tag1.reload.category)
+      assert_equal(1, tag2.reload.category)
+    end
+
+    should "not change either tag category when neither the antecedent or consequent are general" do
+      tag1 = create(:tag, name: "character", category: 4)
+      tag2 = create(:tag, name: "copyright", category: 3)
+      ta = create(:tag_alias, antecedent_name: "character", consequent_name: "copyright")
+
+      ta.approve!(approver: @admin)
+      perform_enqueued_jobs
+
+      assert_equal(4, tag1.reload.category)
+      assert_equal(3, tag2.reload.category)
+    end
   end
 end