From 25cba710bf45d73363893b25ee422ccdc0ca2bb2 Mon Sep 17 00:00:00 2001 From: evazion Date: Thu, 12 Nov 2020 19:37:55 -0600 Subject: [PATCH] BURs: don't allow requesting implications that already exist. Fix it being possible to request duplicate implications. --- app/logical/bulk_update_request_processor.rb | 2 +- app/models/tag_implication.rb | 2 +- config/application.rb | 1 + config/locales/en.yml | 7 +++++++ test/unit/bulk_update_request_test.rb | 8 ++++++++ test/unit/tag_implication_test.rb | 4 ++-- 6 files changed, 20 insertions(+), 4 deletions(-) diff --git a/app/logical/bulk_update_request_processor.rb b/app/logical/bulk_update_request_processor.rb index 2d072640d..1e369bb84 100644 --- a/app/logical/bulk_update_request_processor.rb +++ b/app/logical/bulk_update_request_processor.rb @@ -48,7 +48,7 @@ class BulkUpdateRequestProcessor end when :create_implication - tag_implication = TagImplication.new(creator: User.system, antecedent_name: args[0], consequent_name: args[1], skip_secondary_validations: skip_secondary_validations) + tag_implication = TagImplication.new(creator: User.system, antecedent_name: args[0], consequent_name: args[1], skip_secondary_validations: skip_secondary_validations, status: "active") if tag_implication.invalid? errors[:base] << "Can't create implication #{tag_implication.antecedent_name} -> #{tag_implication.consequent_name} (#{tag_implication.errors.full_messages.join("; ")})" end diff --git a/app/models/tag_implication.rb b/app/models/tag_implication.rb index a4e7649ab..69f09781c 100644 --- a/app/models/tag_implication.rb +++ b/app/models/tag_implication.rb @@ -3,7 +3,7 @@ class TagImplication < TagRelationship has_many :parent_implications, class_name: "TagImplication", primary_key: :antecedent_name, foreign_key: :consequent_name after_save :create_mod_action - validates_uniqueness_of :antecedent_name, scope: [:consequent_name, :status], conditions: -> { active } + validates :antecedent_name, uniqueness: { scope: [:consequent_name, :status], conditions: -> { active }} validate :absence_of_circular_relation validate :absence_of_transitive_relation validate :antecedent_is_not_aliased diff --git a/config/application.rb b/config/application.rb index 28d80a21f..5f2e046ac 100644 --- a/config/application.rb +++ b/config/application.rb @@ -44,6 +44,7 @@ module Danbooru config.autoload_paths += %W(#{config.root}/app/presenters #{config.root}/app/logical/concerns #{config.root}/app/logical #{config.root}/app/mailers) config.plugins = [:all] config.time_zone = 'Eastern Time (US & Canada)' + config.active_model.i18n_customize_full_message = true raise "Danbooru.config.secret_key_base not configured" if Danbooru.config.secret_key_base.blank? config.secret_key_base = Danbooru.config.secret_key_base diff --git a/config/locales/en.yml b/config/locales/en.yml index 0dd40d4e4..eee817cb7 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -68,3 +68,10 @@ en: saved_search: user: "You" user_id: "You" + errors: + models: + tag_implication: + attributes: + antecedent_name: + taken: "Implication already exists" + format: "%{message}" diff --git a/test/unit/bulk_update_request_test.rb b/test/unit/bulk_update_request_test.rb index 3500daca7..27ba2b842 100644 --- a/test/unit/bulk_update_request_test.rb +++ b/test/unit/bulk_update_request_test.rb @@ -129,6 +129,14 @@ class BulkUpdateRequestTest < ActiveSupport::TestCase assert_equal(["Can't create implication a -> c (a already implies c through another implication)"], @bur.errors.full_messages) end + should "fail for an implication that is a duplicate of an existing implication" do + create(:tag_implication, antecedent_name: "a", consequent_name: "b") + @bur = build(:bulk_update_request, script: "imply a -> b") + + assert_equal(false, @bur.valid?) + assert_equal(["Can't create implication a -> b (Implication already exists)"], @bur.errors.full_messages) + end + should_eventually "fail for an implication that is redundant with another implication in the same BUR" do create(:tag_implication, antecedent_name: "b", consequent_name: "c") @bur = build(:bulk_update_request, script: "imply a -> b\nimply a -> c") diff --git a/test/unit/tag_implication_test.rb b/test/unit/tag_implication_test.rb index 85d7c6293..263529cb5 100644 --- a/test/unit/tag_implication_test.rb +++ b/test/unit/tag_implication_test.rb @@ -47,7 +47,7 @@ class TagImplicationTest < ActiveSupport::TestCase [ti1, ti2, ti3, ti4, ti5].each { |ti| assert(ti.valid?) } ti5.update(status: "active") - assert_includes(ti5.errors[:antecedent_name], "has already been taken") + assert_includes(ti5.errors[:antecedent_name], "Implication already exists") end end @@ -115,7 +115,7 @@ class TagImplicationTest < ActiveSupport::TestCase ti2 = FactoryBot.build(:tag_implication, :antecedent_name => "aaa", :consequent_name => "bbb") ti2.save assert(ti2.errors.any?, "Tag implication should not have validated.") - assert_includes(ti2.errors.full_messages, "Antecedent name has already been taken") + assert_includes(ti2.errors.full_messages, "Implication already exists") end should "not validate if its antecedent or consequent are aliased to another tag" do