From f59399ed67134e85e1c058f5f9129b7080c4e0d9 Mon Sep 17 00:00:00 2001
From: Earlopain <14981592+Earlopain@users.noreply.github.com>
Date: Thu, 22 Feb 2024 17:38:51 +0100
Subject: [PATCH] [TagRelationships] Fix aliases/implications being left in
 queued state

This wasn't going through the error handler
---
 app/models/tag_alias.rb           |  8 ++------
 app/models/tag_implication.rb     |  8 ++------
 test/factories/tag_implication.rb |  1 +
 test/unit/tag_alias_test.rb       | 13 +++++++++++--
 test/unit/tag_implication_test.rb |  9 +++++++++
 5 files changed, 25 insertions(+), 14 deletions(-)

diff --git a/app/models/tag_alias.rb b/app/models/tag_alias.rb
index 7d18f7c60..e59192113 100644
--- a/app/models/tag_alias.rb
+++ b/app/models/tag_alias.rb
@@ -179,15 +179,11 @@ class TagAlias < TagRelationship
   end
 
   def process!(update_topic: true)
-    unless valid?
-      raise errors.full_messages.join("; ")
-    end
-
     tries = 0
 
     begin
       CurrentUser.scoped(approver) do
-        update(status: "processing")
+        update!(status: "processing")
         move_aliases_and_implications
         ensure_category_consistency
         update_posts_locked_tags
@@ -210,7 +206,7 @@ class TagAlias < TagRelationship
 
       CurrentUser.scoped(approver) do
         forum_updater.update(failure_message(e), "FAILED") if update_topic
-        update(status: "error: #{e}")
+        update_columns(status: "error: #{e}")
       end
 
       DanbooruLogger.log(e, tag_alias_id: id, antecedent_name: antecedent_name, consequent_name: consequent_name)
diff --git a/app/models/tag_implication.rb b/app/models/tag_implication.rb
index 0e65a43a3..69c80e2c5 100644
--- a/app/models/tag_implication.rb
+++ b/app/models/tag_implication.rb
@@ -115,15 +115,11 @@ class TagImplication < TagRelationship
 
   module ApprovalMethods
     def process!(update_topic: true)
-      unless valid?
-        raise errors.full_messages.join("; ")
-      end
-
       tries = 0
 
       begin
         CurrentUser.scoped(approver) do
-          update(status: "processing")
+          update!(status: "processing")
           update_posts
           update(status: "active")
           update_descendant_names_for_parents
@@ -137,7 +133,7 @@ class TagImplication < TagRelationship
         end
 
         forum_updater.update(failure_message(e), "FAILED") if update_topic
-        update(status: "error: #{e}")
+        update_columns(status: "error: #{e}")
 
         DanbooruLogger.log(e, tag_implication_id: id, antecedent_name: antecedent_name, consequent_name: consequent_name)
       end
diff --git a/test/factories/tag_implication.rb b/test/factories/tag_implication.rb
index 44ccb1feb..3b0148f4d 100644
--- a/test/factories/tag_implication.rb
+++ b/test/factories/tag_implication.rb
@@ -3,5 +3,6 @@ FactoryBot.define do
     antecedent_name { "aaa" }
     consequent_name { "bbb" }
     status { "active" }
+    creator_ip_addr { "127.0.0.1" }
   end
 end
diff --git a/test/unit/tag_alias_test.rb b/test/unit/tag_alias_test.rb
index 90d784845..36901cf5b 100644
--- a/test/unit/tag_alias_test.rb
+++ b/test/unit/tag_alias_test.rb
@@ -127,7 +127,7 @@ class TagAliasTest < ActiveSupport::TestCase
       tag1 = create(:tag, name: "aaa", category: 1)
       tag2 = create(:tag, name: "bbb", category: 3)
       ta = create(:tag_alias, antecedent_name: "aaa", consequent_name: "bbb")
-      ta.approve!(approver: @admin)
+      with_inline_jobs { ta.approve!(approver: @admin) }
 
       assert_equal(3, tag2.reload.category)
     end
@@ -145,11 +145,20 @@ class TagAliasTest < ActiveSupport::TestCase
       tag1 = create(:tag, name: "aaa", category: 1)
       tag2 = create(:tag, name: "bbb", category: 3, is_locked: true)
       ta = create(:tag_alias, antecedent_name: "aaa", consequent_name: "bbb")
-      ta.approve!(approver: @admin)
+      with_inline_jobs { ta.approve!(approver: @admin) }
 
       assert_equal(3, tag2.reload.category)
     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", creator: @admin)
+      ta.save(validate: false)
+      with_inline_jobs { ta.approve!(approver: @admin) }
+
+      assert_match "error", ta.reload.status
+    end
+
     context "with an associated forum topic" do
       setup do
         @admin = create(:admin_user)
diff --git a/test/unit/tag_implication_test.rb b/test/unit/tag_implication_test.rb
index a034c628c..d93ec5872 100644
--- a/test/unit/tag_implication_test.rb
+++ b/test/unit/tag_implication_test.rb
@@ -220,6 +220,15 @@ class TagImplicationTest < ActiveSupport::TestCase
       assert_equal("aaa bbb ccc xxx yyy", p1.reload.tag_string)
     end
 
+    should "error on approve if its not valid anymore" do
+      create(:tag_implication, antecedent_name: "aaa", consequent_name: "bbb", status: "active")
+      ti = build(:tag_implication, antecedent_name: "aaa", consequent_name: "bbb", creator: @user)
+      ti.save(validate: false)
+      with_inline_jobs { ti.approve!(approver: @user) }
+
+      assert_match "error", ti.reload.status
+    end
+
     context "with an associated forum topic" do
       setup do
         @admin = create(:admin_user)