Fix #4168: Ignore validations when rejecting tag changes.

* Only check for conflicts with existing aliases/implications when
  requests are created or approved, not when requests are rejected.

* Use `update!(status: "deleted")` instead of `update(status: "deleted")`
  so that if rejecting the request fails we fail immediately instead of
  continuing on and updating the forum topic.

* Wrap `reject!` and `TagChangeRequestPruner.reject_expired` in
  transactions so that if updating either the request or the forum
  fails, they both get rolled back.

app/logical/tag_change_request_pruner.rb

@@ -29,14 +29,16 @@ class TagChangeRequestPruner
 
   def reject_expired(model)
     model.expired.pending.find_each do |tag_change|
-      if tag_change.forum_topic
+      transaction do
-        name = model.model_name.human.downcase
+        if tag_change.forum_topic
-        body = "This #{name} has been rejected because it was not approved within 60 days."
+          name = model.model_name.human.downcase
-        tag_change.forum_updater.update(body)
+          body = "This #{name} has been rejected because it was not approved within 60 days."
-      end
+          tag_change.forum_updater.update(body)
+        end
 
-      CurrentUser.as_system do
+        CurrentUser.as_system do
-        tag_change.reject!
+          tag_change.reject!
+        end
       end
     end
   end

app/models/tag_alias.rb

@@ -83,6 +83,8 @@ class TagAlias < TagRelationship
   end
 
   def absence_of_transitive_relation
+    return if is_rejected?
+
     # We don't want a -> b && b -> c chains if the b -> c alias was created first.
     # If the a -> b alias was created first, the new one will be allowed and the old one will be moved automatically instead.
     if TagAlias.active.exists?(antecedent_name: consequent_name)
@@ -172,11 +174,6 @@ class TagAlias < TagRelationship
     end
   end
 
-  def reject!(update_topic: true)
-    update(status: "deleted")
-    forum_updater.update(reject_message(CurrentUser.user), "REJECTED") if update_topic
-  end
-
   def wiki_pages_present
     if antecedent_wiki.present? && consequent_wiki.present?
       errors[:base] << conflict_message

app/models/tag_implication.rb

@@ -76,6 +76,8 @@ class TagImplication < TagRelationship
 
   module ValidationMethods
     def absence_of_circular_relation
+      return if is_rejected?
+
       # We don't want a -> b && b -> a chains
       if descendants.include?(antecedent_name)
         errors[:base] << "Tag implication can not create a circular relation with another tag implication"
@@ -84,6 +86,8 @@ class TagImplication < TagRelationship
 
     # If we already have a -> b -> c, don't allow a -> c.
     def absence_of_transitive_relation
+      return if is_rejected?
+
       # Find everything else the antecedent implies, not including the current implication.
       implications = TagImplication.active.where("antecedent_name = ? and consequent_name != ?", antecedent_name, consequent_name)
       implied_tags = implications.flat_map(&:descendant_names)
@@ -93,6 +97,8 @@ class TagImplication < TagRelationship
     end
 
     def antecedent_is_not_aliased
+      return if is_rejected?
+
       # We don't want to implicate a -> b if a is already aliased to c
       if TagAlias.active.exists?(["antecedent_name = ?", antecedent_name])
         errors[:base] << "Antecedent tag must not be aliased to another tag"
@@ -100,6 +106,8 @@ class TagImplication < TagRelationship
     end
 
     def consequent_is_not_aliased
+      return if is_rejected?
+
       # We don't want to implicate a -> b if b is already aliased to c
       if TagAlias.active.exists?(["antecedent_name = ?", consequent_name])
         errors[:base] << "Consequent tag must not be aliased to another tag"
@@ -165,11 +173,6 @@ class TagImplication < TagRelationship
       ProcessTagImplicationJob.perform_later(self, update_topic: update_topic)
     end
 
-    def reject!(update_topic: true)
-      update(status: "deleted")
-      forum_updater.update(reject_message(CurrentUser.user), "REJECTED") if update_topic
-    end
-
     def create_mod_action
       implication = %Q("tag implication ##{id}":[#{Rails.application.routes.url_helpers.tag_implication_path(self)}]: [[#{antecedent_name}]] -> [[#{consequent_name}]])
 

app/models/tag_relationship.rb

@@ -47,6 +47,10 @@ class TagRelationship < ApplicationRecord
     status.in?(%w[active processing queued])
   end
 
+  def is_rejected?
+    status.in?(%w[retired deleted])
+  end
+
   def is_retired?
     status == "retired"
   end
@@ -78,6 +82,13 @@ class TagRelationship < ApplicationRecord
     deletable_by?(user)
   end
 
+  def reject!(update_topic: true)
+    transaction do
+      update!(status: "deleted")
+      forum_updater.update(reject_message(CurrentUser.user), "REJECTED") if update_topic
+    end
+  end
+
   module SearchMethods
     def name_matches(name)
       where("(antecedent_name like ? escape E'\\\\' or consequent_name like ? escape E'\\\\')", name.mb_chars.downcase.to_escaped_for_sql_like, name.mb_chars.downcase.to_escaped_for_sql_like)

test/unit/tag_alias_test.rb

@@ -80,6 +80,16 @@ class TagAliasTest < ActiveSupport::TestCase
       end
     end
 
+    context "#reject!" do
+      should "not be blocked by validations" do
+        ta1 = create(:tag_alias, antecedent_name: "kitty", consequent_name: "kitten", status: "active")
+        ta2 = build(:tag_alias, antecedent_name: "cat", consequent_name: "kitty", status: "pending")
+
+        ta2.reject!
+        assert_equal("deleted", ta2.reload.status)
+      end
+    end
+
     context "on secondary validation" do
       should "warn about missing wiki pages" do
         ti = FactoryBot.build(:tag_alias, antecedent_name: "aaa", consequent_name: "bbb", skip_secondary_validations: false)

test/unit/tag_implication_test.rb

@@ -76,6 +76,16 @@ class TagImplicationTest < ActiveSupport::TestCase
       end
     end
 
+    context "#reject!" do
+      should "not be blocked by alias validations" do
+        ti = create(:tag_implication, antecedent_name: "cat", consequent_name: "animal", status: "pending")
+        ta = create(:tag_alias, antecedent_name: "cat", consequent_name: "kitty", status: "active")
+
+        ti.reject!
+        assert_equal("deleted", ti.reload.status)
+      end
+    end
+
     context "on secondary validation" do
       should "warn if either tag is missing a wiki" do
         ti = FactoryBot.build(:tag_implication, antecedent_name: "aaa", consequent_name: "bbb", skip_secondary_validations: false)