From aeb2076b807fbb4879eda70c0da2c5ba25d1390b Mon Sep 17 00:00:00 2001
From: Earlopain <14981592+Earlopain@users.noreply.github.com>
Date: Sun, 17 Mar 2024 19:06:37 +0100
Subject: [PATCH] [TagAliases] Fix locked artist preventing approval

Everything is already scoped to the approver. Switching to the creator doesn't make much sense here.

In addition, this sent the success message 5 times because the rename happened at the wrong place
---
 app/models/tag_alias.rb     | 20 ++++++--------------
 test/unit/tag_alias_test.rb | 21 +++++++++++++++++++++
 2 files changed, 27 insertions(+), 14 deletions(-)

diff --git a/app/models/tag_alias.rb b/app/models/tag_alias.rb
index 12fb6bbc3..65bcbcc93 100644
--- a/app/models/tag_alias.rb
+++ b/app/models/tag_alias.rb
@@ -128,8 +128,8 @@ class TagAlias < TagRelationship
       update_posts_locked_tags_undo
       update_blacklists_undo
       update_posts_undo
-      forum_updater.update(retirement_message, "UNDONE") if update_topic
       rename_artist_undo
+      forum_updater.update(retirement_message, "UNDONE") if update_topic
     end
     tag_rel_undos.update_all(applied: true)
   end
@@ -138,9 +138,7 @@ class TagAlias < TagRelationship
     Post.without_timeout do
       Post.where_ilike(:locked_tags, "*#{consequent_name}*").find_each(batch_size: 50) do |post|
         fixed_tags = TagAlias.to_aliased_query(post.locked_tags, overrides: {consequent_name => antecedent_name})
-        CurrentUser.scoped(creator, creator_ip_addr) do
-          post.update_column(:locked_tags, fixed_tags)
-        end
+        post.update_column(:locked_tags, fixed_tags)
       end
     end
   end
@@ -173,9 +171,7 @@ class TagAlias < TagRelationship
   def rename_artist_undo
     if consequent_tag.category == Tag.categories.artist
       if consequent_tag.artist.present? && antecedent_tag.artist.blank?
-        CurrentUser.scoped(creator, creator_ip_addr) do
-          consequent_tag.artist.update!(name: antecedent_name)
-        end
+        consequent_tag.artist.update!(name: antecedent_name)
       end
     end
   end
@@ -191,8 +187,8 @@ class TagAlias < TagRelationship
         update_posts_locked_tags
         update_blacklists
         update_posts
-        forum_updater.update(approval_message(approver), "APPROVED") if update_topic
         rename_artist
+        forum_updater.update(approval_message(approver), "APPROVED") if update_topic
         update(status: 'active', post_count: consequent_tag.post_count)
         # TODO: Race condition with indexing jobs here.
         antecedent_tag.fix_post_count if antecedent_tag
@@ -275,9 +271,7 @@ class TagAlias < TagRelationship
     Post.without_timeout do
       Post.where_ilike(:locked_tags, "*#{antecedent_name}*").find_each(batch_size: 50) do |post|
         fixed_tags = TagAlias.to_aliased_query(post.locked_tags)
-        CurrentUser.scoped(creator, creator_ip_addr) do
-          post.update_column(:locked_tags, fixed_tags)
-        end
+        post.update_column(:locked_tags, fixed_tags)
       end
     end
   end
@@ -297,9 +291,7 @@ class TagAlias < TagRelationship
   def rename_artist
     if antecedent_tag.category == Tag.categories.artist
       if antecedent_tag.artist.present? && consequent_tag.artist.blank?
-        CurrentUser.scoped(creator, creator_ip_addr) do
-          antecedent_tag.artist.update!(name: consequent_name)
-        end
+        antecedent_tag.artist.update!(name: consequent_name)
       end
     end
   end
diff --git a/test/unit/tag_alias_test.rb b/test/unit/tag_alias_test.rb
index ff8eb50de..0fc01e3f4 100644
--- a/test/unit/tag_alias_test.rb
+++ b/test/unit/tag_alias_test.rb
@@ -152,6 +152,17 @@ class TagAliasTest < ActiveSupport::TestCase
       assert_equal(3, tag2.reload.category)
     end
 
+    should "not fail if an artist with the same name is locked" do
+      ta = create(:tag_alias, antecedent_name: "aaa", consequent_name: "bbb")
+      artist = as(@admin) { create(:artist, name: "aaa", is_locked: true) }
+      artist.tag.update(category: Tag.categories.artist)
+
+      with_inline_jobs { ta.approve!(approver: @admin) }
+
+      assert_equal("active", ta.reload.status)
+      assert_equal("bbb", artist.reload.name)
+    end
+
     should "error on approve if its not valid anymore" do
       create(:tag_alias, antecedent_name: "aaa", consequent_name: "bbb", status: "active")
       ta = build(:tag_alias, antecedent_name: "aaa", consequent_name: "bbb", status: "pending", creator: @admin)
@@ -179,6 +190,16 @@ class TagAliasTest < ActiveSupport::TestCase
       assert_equal "deleted", ta.reload.status
     end
 
+    should "update locked tags on approve" do
+      ta = create(:tag_alias, antecedent_name: "aaa", consequent_name: "bbb", status: "pending")
+      post1 = create(:post, locked_tags: "aaa foo")
+      post2 = create(:post, locked_tags: "-aaa foo")
+      with_inline_jobs { ta.approve!(approver: @admin) }
+
+      assert_equal("bbb foo", post1.reload.locked_tags)
+      assert_equal("-bbb foo", post2.reload.locked_tags)
+    end
+
     context "with an associated forum topic" do
       setup do
         @admin = create(:admin_user)