diff --git a/app/logical/tag_change_request_pruner.rb b/app/logical/tag_change_request_pruner.rb index 1d1d628fc..b67096002 100644 --- a/app/logical/tag_change_request_pruner.rb +++ b/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 - name = model.model_name.human.downcase - body = "This #{name} has been rejected because it was not approved within 60 days." - tag_change.forum_updater.update(body) - end + transaction do + if tag_change.forum_topic + name = model.model_name.human.downcase + body = "This #{name} has been rejected because it was not approved within 60 days." + tag_change.forum_updater.update(body) + end - CurrentUser.as_system do - tag_change.reject! + CurrentUser.as_system do + tag_change.reject! + end end end end diff --git a/app/models/tag_alias.rb b/app/models/tag_alias.rb index 5e9c29adf..40424c1af 100644 --- a/app/models/tag_alias.rb +++ b/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 diff --git a/app/models/tag_implication.rb b/app/models/tag_implication.rb index 4d700ee9d..b72513f42 100644 --- a/app/models/tag_implication.rb +++ b/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}]]) diff --git a/app/models/tag_relationship.rb b/app/models/tag_relationship.rb index ceb224407..de18effda 100644 --- a/app/models/tag_relationship.rb +++ b/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) diff --git a/test/unit/tag_alias_test.rb b/test/unit/tag_alias_test.rb index 9dc32d392..069ddaa77 100644 --- a/test/unit/tag_alias_test.rb +++ b/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) diff --git a/test/unit/tag_implication_test.rb b/test/unit/tag_implication_test.rb index 3ccd7e50f..1f9d813c3 100644 --- a/test/unit/tag_implication_test.rb +++ b/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)