diff --git a/app/logical/bulk_update_request_processor.rb b/app/logical/bulk_update_request_processor.rb index 9601e95ef..af8536637 100644 --- a/app/logical/bulk_update_request_processor.rb +++ b/app/logical/bulk_update_request_processor.rb @@ -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 diff --git a/app/models/bulk_update_request.rb b/app/models/bulk_update_request.rb index 9e0784851..3b2e9a0df 100644 --- a/app/models/bulk_update_request.rb +++ b/app/models/bulk_update_request.rb @@ -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 diff --git a/app/models/tag_implication.rb b/app/models/tag_implication.rb index 6c5da1d4f..aa583c87d 100644 --- a/app/models/tag_implication.rb +++ b/app/models/tag_implication.rb @@ -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 diff --git a/test/unit/bulk_update_request_test.rb b/test/unit/bulk_update_request_test.rb index 77031626f..4bcba51a1 100644 --- a/test/unit/bulk_update_request_test.rb +++ b/test/unit/bulk_update_request_test.rb @@ -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