BURs: refactor implication wiki page validations.
Move the validation that the tags in an implication must have wiki pages back into the TagImplication model. Use validation contexts to only run the validation when the BUR is created, not when the BUR is approved.
This commit is contained in:
@@ -40,30 +40,27 @@ class BulkUpdateRequestProcessor
|
||||
end
|
||||
end
|
||||
|
||||
# validation_context will be either :request (when the BUR is first created
|
||||
# or edited) or :approval (when the BUR is approved). Certain validations
|
||||
# only run when the BUR is requested, not when it's approved.
|
||||
def validate_script
|
||||
BulkUpdateRequest.transaction(requires_new: true) do
|
||||
commands.each do |command, *args|
|
||||
case command
|
||||
when :create_alias
|
||||
tag_alias = TagAlias.create(creator: User.system, antecedent_name: args[0], consequent_name: args[1])
|
||||
if tag_alias.invalid?
|
||||
tag_alias = TagAlias.new(creator: User.system, antecedent_name: args[0], consequent_name: args[1])
|
||||
tag_alias.save(context: validation_context)
|
||||
if tag_alias.errors.present?
|
||||
errors[:base] << "Can't create alias #{tag_alias.antecedent_name} -> #{tag_alias.consequent_name} (#{tag_alias.errors.full_messages.join("; ")})"
|
||||
end
|
||||
|
||||
when :create_implication
|
||||
tag_implication = TagImplication.create(creator: User.system, antecedent_name: args[0], consequent_name: args[1], status: "active")
|
||||
if tag_implication.invalid?
|
||||
tag_implication = TagImplication.new(creator: User.system, antecedent_name: args[0], consequent_name: args[1], status: "active")
|
||||
tag_implication.save(context: validation_context)
|
||||
if tag_implication.errors.present?
|
||||
errors[:base] << "Can't create implication #{tag_implication.antecedent_name} -> #{tag_implication.consequent_name} (#{tag_implication.errors.full_messages.join("; ")})"
|
||||
end
|
||||
|
||||
if !tag_implication.antecedent_tag.empty? && tag_implication.antecedent_wiki.blank?
|
||||
errors[:base] << "'#{tag_implication.antecedent_tag.name}' must have a wiki page"
|
||||
end
|
||||
|
||||
if !tag_implication.consequent_tag.empty? && tag_implication.consequent_wiki.blank?
|
||||
errors[:base] << "'#{tag_implication.consequent_tag.name}' must have a wiki page"
|
||||
end
|
||||
|
||||
when :remove_alias
|
||||
tag_alias = TagAlias.active.find_by(antecedent_name: args[0], consequent_name: args[1])
|
||||
if tag_alias.nil?
|
||||
@@ -110,8 +107,6 @@ class BulkUpdateRequestProcessor
|
||||
|
||||
def process!(approver)
|
||||
ActiveRecord::Base.transaction do
|
||||
validate!
|
||||
|
||||
commands.map do |command, *args|
|
||||
case command
|
||||
when :create_alias
|
||||
|
||||
@@ -73,6 +73,7 @@ class BulkUpdateRequest < ApplicationRecord
|
||||
def approve!(approver)
|
||||
transaction do
|
||||
CurrentUser.scoped(approver) do
|
||||
processor.validate!(:approval)
|
||||
processor.process!(approver)
|
||||
update!(status: "approved", approver: approver)
|
||||
forum_updater.update("The #{bulk_update_request_link} (forum ##{forum_post.id}) has been approved by @#{approver.name}.")
|
||||
@@ -107,7 +108,7 @@ class BulkUpdateRequest < ApplicationRecord
|
||||
end
|
||||
|
||||
def validate_script
|
||||
if processor.invalid?
|
||||
if processor.invalid?(:request)
|
||||
errors[:base] << processor.errors.full_messages.join("; ")
|
||||
end
|
||||
end
|
||||
|
||||
@@ -7,6 +7,7 @@ class TagImplication < TagRelationship
|
||||
validate :absence_of_transitive_relation
|
||||
validate :antecedent_is_not_aliased
|
||||
validate :consequent_is_not_aliased
|
||||
validate :has_wiki_page, on: :request
|
||||
|
||||
concerning :HierarchyMethods do
|
||||
class_methods do
|
||||
@@ -91,6 +92,16 @@ class TagImplication < TagRelationship
|
||||
errors[:base] << "Consequent tag must not be aliased to another tag"
|
||||
end
|
||||
end
|
||||
|
||||
def has_wiki_page
|
||||
if !antecedent_tag.empty? && antecedent_wiki.blank?
|
||||
errors[:base] << "'#{antecedent_name}' must have a wiki page"
|
||||
end
|
||||
|
||||
if !consequent_tag.empty? && consequent_wiki.blank?
|
||||
errors[:base] << "'#{consequent_name}' must have a wiki page"
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
concerning :ApprovalMethods do
|
||||
|
||||
@@ -335,7 +335,7 @@ class BulkUpdateRequestTest < ActiveSupport::TestCase
|
||||
@bur = build(:bulk_update_request, script: "imply a -> b")
|
||||
|
||||
assert_equal(false, @bur.valid?)
|
||||
assert_equal(["'a' must have a wiki page; 'b' must have a wiki page"], @bur.errors.full_messages)
|
||||
assert_equal(["Can't create implication a -> b ('a' must have a wiki page; 'b' must have a wiki page)"], @bur.errors.full_messages)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
Reference in New Issue
Block a user