From f5116c5ce2784537446a963ea19089fe39c2f5a8 Mon Sep 17 00:00:00 2001 From: evazion Date: Mon, 31 Dec 2018 17:07:14 -0600 Subject: [PATCH] aliases/implications: allow duplicate inactive aliases/implications. Allow multiple pending, deleted or retired aliases/implications for the same tag. This is so that deleted or retired aliases can be resubmitted as new pending requests. --- app/models/tag_alias.rb | 2 +- app/models/tag_implication.rb | 2 +- test/unit/tag_alias_test.rb | 12 ++++++++++++ test/unit/tag_implication_test.rb | 12 ++++++++++++ 4 files changed, 26 insertions(+), 2 deletions(-) diff --git a/app/models/tag_alias.rb b/app/models/tag_alias.rb index 78fe61930..000f616bf 100644 --- a/app/models/tag_alias.rb +++ b/app/models/tag_alias.rb @@ -3,7 +3,7 @@ class TagAlias < TagRelationship after_destroy :clear_all_cache after_save :clear_all_cache, if: ->(rec) {rec.is_retired?} after_save :create_mod_action - validates_uniqueness_of :antecedent_name + validates_uniqueness_of :antecedent_name, scope: :status, conditions: -> { active } validate :absence_of_transitive_relation validate :wiki_pages_present, on: :create, unless: :skip_secondary_validations validate :mininum_antecedent_count, on: :create, unless: :skip_secondary_validations diff --git a/app/models/tag_implication.rb b/app/models/tag_implication.rb index b10a4c65a..463387046 100644 --- a/app/models/tag_implication.rb +++ b/app/models/tag_implication.rb @@ -7,7 +7,7 @@ class TagImplication < TagRelationship after_save :update_descendant_names_for_parents after_destroy :update_descendant_names_for_parents after_save :create_mod_action - validates_uniqueness_of :antecedent_name, :scope => :consequent_name + validates_uniqueness_of :antecedent_name, scope: [:consequent_name, :status], conditions: -> { active } validate :absence_of_circular_relation validate :absence_of_transitive_relation validate :antecedent_is_not_aliased diff --git a/test/unit/tag_alias_test.rb b/test/unit/tag_alias_test.rb index a2028015e..7f86e43a8 100644 --- a/test/unit/tag_alias_test.rb +++ b/test/unit/tag_alias_test.rb @@ -44,6 +44,18 @@ class TagAliasTest < ActiveSupport::TestCase should_not allow_value(nil).for(:creator_id) should_not allow_value(-1).for(:creator_id).with_message("must exist", against: :creator) + + should "not allow duplicate active aliases" do + ta1 = FactoryBot.create(:tag_alias, antecedent_name: "aaa", consequent_name: "bbb", status: "active") + ta2 = FactoryBot.create(:tag_alias, antecedent_name: "aaa", consequent_name: "bbb", status: "retired") + ta3 = FactoryBot.create(:tag_alias, antecedent_name: "aaa", consequent_name: "bbb", status: "deleted") + ta4 = FactoryBot.create(:tag_alias, antecedent_name: "aaa", consequent_name: "bbb", status: "deleted") + ta5 = FactoryBot.create(:tag_alias, antecedent_name: "aaa", consequent_name: "bbb", status: "pending") + [ta1, ta2, ta3, ta4, ta5].each { |ta| assert(ta.valid?) } + + ta5.update(status: "active") + assert_includes(ta5.errors[:antecedent_name], "has already been taken") + end end context "on secondary validation" do diff --git a/test/unit/tag_implication_test.rb b/test/unit/tag_implication_test.rb index 2daad2fe3..a21005209 100644 --- a/test/unit/tag_implication_test.rb +++ b/test/unit/tag_implication_test.rb @@ -40,6 +40,18 @@ class TagImplicationTest < ActiveSupport::TestCase should_not allow_value(nil).for(:creator_id) should_not allow_value(-1).for(:creator_id).with_message("must exist", against: :creator) + + should "not allow duplicate active implications" do + ti1 = FactoryBot.create(:tag_implication, antecedent_name: "aaa", consequent_name: "bbb", status: "active") + ti2 = FactoryBot.create(:tag_implication, antecedent_name: "aaa", consequent_name: "bbb", status: "retired") + ti3 = FactoryBot.create(:tag_implication, antecedent_name: "aaa", consequent_name: "bbb", status: "deleted") + ti4 = FactoryBot.create(:tag_implication, antecedent_name: "aaa", consequent_name: "bbb", status: "deleted") + ti5 = FactoryBot.create(:tag_implication, antecedent_name: "aaa", consequent_name: "bbb", status: "pending") + [ti1, ti2, ti3, ti4, ti5].each { |ti| assert(ti.valid?) } + + ti5.update(status: "active") + assert_includes(ti5.errors[:antecedent_name], "has already been taken") + end end context "on secondary validation" do