BURs: don't allow requesting implications that already exist.

Fix it being possible to request duplicate implications.
This commit is contained in:
evazion
2020-11-12 19:37:55 -06:00
parent 654d2175b6
commit 25cba710bf
6 changed files with 20 additions and 4 deletions

View File

@@ -48,7 +48,7 @@ class BulkUpdateRequestProcessor
end end
when :create_implication 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? if tag_implication.invalid?
errors[:base] << "Can't create implication #{tag_implication.antecedent_name} -> #{tag_implication.consequent_name} (#{tag_implication.errors.full_messages.join("; ")})" errors[:base] << "Can't create implication #{tag_implication.antecedent_name} -> #{tag_implication.consequent_name} (#{tag_implication.errors.full_messages.join("; ")})"
end end

View File

@@ -3,7 +3,7 @@ class TagImplication < TagRelationship
has_many :parent_implications, class_name: "TagImplication", primary_key: :antecedent_name, foreign_key: :consequent_name has_many :parent_implications, class_name: "TagImplication", primary_key: :antecedent_name, foreign_key: :consequent_name
after_save :create_mod_action 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_circular_relation
validate :absence_of_transitive_relation validate :absence_of_transitive_relation
validate :antecedent_is_not_aliased validate :antecedent_is_not_aliased

View File

@@ -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.autoload_paths += %W(#{config.root}/app/presenters #{config.root}/app/logical/concerns #{config.root}/app/logical #{config.root}/app/mailers)
config.plugins = [:all] config.plugins = [:all]
config.time_zone = 'Eastern Time (US & Canada)' 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? raise "Danbooru.config.secret_key_base not configured" if Danbooru.config.secret_key_base.blank?
config.secret_key_base = Danbooru.config.secret_key_base config.secret_key_base = Danbooru.config.secret_key_base

View File

@@ -68,3 +68,10 @@ en:
saved_search: saved_search:
user: "You" user: "You"
user_id: "You" user_id: "You"
errors:
models:
tag_implication:
attributes:
antecedent_name:
taken: "Implication already exists"
format: "%{message}"

View File

@@ -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) assert_equal(["Can't create implication a -> c (a already implies c through another implication)"], @bur.errors.full_messages)
end 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 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") create(:tag_implication, antecedent_name: "b", consequent_name: "c")
@bur = build(:bulk_update_request, script: "imply a -> b\nimply a -> c") @bur = build(:bulk_update_request, script: "imply a -> b\nimply a -> c")

View File

@@ -47,7 +47,7 @@ class TagImplicationTest < ActiveSupport::TestCase
[ti1, ti2, ti3, ti4, ti5].each { |ti| assert(ti.valid?) } [ti1, ti2, ti3, ti4, ti5].each { |ti| assert(ti.valid?) }
ti5.update(status: "active") 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
end end
@@ -115,7 +115,7 @@ class TagImplicationTest < ActiveSupport::TestCase
ti2 = FactoryBot.build(:tag_implication, :antecedent_name => "aaa", :consequent_name => "bbb") ti2 = FactoryBot.build(:tag_implication, :antecedent_name => "aaa", :consequent_name => "bbb")
ti2.save ti2.save
assert(ti2.errors.any?, "Tag implication should not have validated.") 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 end
should "not validate if its antecedent or consequent are aliased to another tag" do should "not validate if its antecedent or consequent are aliased to another tag" do