better validation for bulk update requests

app/logical/alias_and_implication_importer.rb

@@ -12,6 +12,11 @@ class AliasAndImplicationImporter
     parse(tokens)
   end
 
+  def validate!
+    tokens = AliasAndImplicationImporter.tokenize(text)
+    validate(tokens)
+  end
+
   def rename_aliased_pages?
     @rename_aliased_pages == "1"
   end
@@ -45,6 +50,30 @@ class AliasAndImplicationImporter
     end
   end
 
+  def validate(tokens)
+    tokens.map do |token|
+      case token[0]
+      when :create_alias
+        tag_alias = TagAlias.new(:forum_topic_id => forum_id, :status => "pending", :antecedent_name => token[1], :consequent_name => token[2])
+        unless tag_alias.valid?
+          raise "Error: #{tag_alias.errors.full_messages.join("; ")} (create alias #{tag_alias.antecedent_name} -> #{tag_alias.consequent_name})"
+        end
+
+      when :create_implication
+        tag_implication = TagImplication.new(:forum_topic_id => forum_id, :status => "pending", :antecedent_name => token[1], :consequent_name => token[2])
+        unless tag_implication.valid?
+          raise "Error: #{tag_implication.errors.full_messages.join("; ")} (create implication #{tag_implication.antecedent_name} -> #{tag_implication.consequent_name})"
+        end
+
+      when :remove_alias, :remove_implication, :mass_update
+        # okay
+
+      else
+        raise "Unknown token: #{token[0]}"
+      end
+    end
+  end
+
 private
 
   def parse(tokens)

app/models/bulk_update_request.rb

@@ -10,6 +10,7 @@ class BulkUpdateRequest < ActiveRecord::Base
   validates_inclusion_of :status, :in => %w(pending approved rejected)
   validate :script_formatted_correctly
   validate :forum_topic_id_not_invalid
+  validate :validate_script
   attr_accessible :user_id, :forum_topic_id, :script, :title, :reason
   attr_accessible :status, :as => [:admin]
   before_validation :initialize_attributes, :on => :create
@@ -130,7 +131,6 @@ class BulkUpdateRequest < ActiveRecord::Base
     end
   end
 
-
   def update_forum_topic_for_approve
     if forum_topic
       forum_topic.posts.create(
@@ -150,4 +150,15 @@ class BulkUpdateRequest < ActiveRecord::Base
   def normalize_text
     self.script = script.downcase
   end
+
+  def validate_script
+    begin
+      AliasAndImplicationImporter.new(script, forum_topic_id, "1").validate!
+    rescue RuntimeError => e
+      self.errors[:base] = e.message
+      return false
+    end
+
+    errors.empty?
+  end
 end

app/models/tag_alias.rb

@@ -8,6 +8,8 @@ class TagAlias < ActiveRecord::Base
   validates_uniqueness_of :antecedent_name
   validate :absence_of_transitive_relation
   validate :antecedent_and_consequent_are_different
+  # validate :consequent_has_wiki_page
+  # validate :mininum_antecedent_count
   belongs_to :creator, :class_name => "User"
   belongs_to :forum_topic
   attr_accessible :antecedent_name, :consequent_name, :forum_topic_id, :status
@@ -139,7 +141,7 @@ class TagAlias < ActiveRecord::Base
     # We don't want a -> b && b -> c chains if the b -> c alias was created first.
     # If the a -> b alias was created first, the new one will be allowed and the old one will be moved automatically instead.
     if self.class.active.exists?(["antecedent_name = ?", consequent_name])
-      self.errors[:base] << "Tag alias can not create a transitive relation with another tag alias"
+      self.errors[:base] << "A tag alias for #{consequent_name} already exists"
       false
     end
   end
@@ -290,6 +292,23 @@ class TagAlias < ActiveRecord::Base
     destroy
   end
 
+  def consequent_has_wiki_page
+    return if !Danbooru.config.strict_tag_requirements
+
+    unless WikiPage.titled(consequent_name).exists?
+      self.errors[:base] = "The #{consequent_name} tag needs a corresponding wiki page"
+      return false
+    end
+  end
+
+  def mininum_antecedent_count
+    return if !Danbooru.config.strict_tag_requirements
+
+    unless Post.fast_count(antecedent_name) >= 50
+      self.errors[:base] = "The #{antecedent_name} tag must have at least 50 posts for an alias to be created"
+    end
+  end
+
   def self.update_cached_post_counts_for_all
     TagAlias.without_timeout do
       execute_sql("UPDATE tag_aliases SET post_count = tags.post_count FROM tags WHERE tags.name = tag_aliases.consequent_name")

app/models/tag_implication.rb

@@ -12,6 +12,7 @@ class TagImplication < ActiveRecord::Base
   validate :antecedent_is_not_aliased
   validate :consequent_is_not_aliased
   validate :antecedent_and_consequent_are_different
+  # validate :wiki_pages_present
   attr_accessible :antecedent_name, :consequent_name, :descendant_names, :forum_topic_id, :status, :forum_topic
 
   module DescendantMethods
@@ -256,6 +257,20 @@ class TagImplication < ActiveRecord::Base
     end
   end
 
+  def wiki_pages_present
+    return if !Danbooru.config.strict_tag_requirements
+
+    unless WikiPage.titled(consequent_name).exists?
+      self.errors[:base] = "The #{consequent_name} tag needs a corresponding wiki page"
+      return false
+    end
+
+    unless WikiPage.titled(antecedent_name).exists?
+      self.errors[:base] = "The #{antecedent_name} tag needs a corresponding wiki page"
+      return false
+    end
+  end
+
   def reject!
     update_forum_topic_for_reject
     destroy

config/danbooru_default_config.rb

@@ -348,6 +348,11 @@ module Danbooru
       "zDMSATq0W3hmA5p3rKTgD"
     end
 
+    # impose additional requirements to create tag aliases and implications
+    def strict_tag_requirements
+      true
+    end
+
     # For downloads, if the host matches any of these IPs, block it
     def banned_ip_for_download?(ip_addr)
       raise ArgumentError unless ip_addr.is_a?(IPAddr)

test/unit/bulk_update_request_test.rb

@@ -4,10 +4,12 @@ class BulkUpdateRequestTest < ActiveSupport::TestCase
   context "creation" do
     setup do
       CurrentUser.user = FactoryGirl.create(:user)
+      CurrentUser.ip_addr = "127.0.0.1"
     end
 
     teardown do
       CurrentUser.user = nil
+      CurrentUser.ip_addr = nil
     end
 
     should "create a forum topic" do
@@ -16,6 +18,20 @@ class BulkUpdateRequestTest < ActiveSupport::TestCase
       end
     end
 
+    context "that has an invalid alias" do
+      setup do
+        @alias1 = FactoryGirl.create(:tag_alias)
+        @req = FactoryGirl.build(:bulk_update_request, :script => "create alias bbb -> aaa")
+      end
+
+      should "not validate" do
+        assert_difference("TagAlias.count", 0) do
+          @req.save
+        end
+        assert_equal(["Error: A tag alias for aaa already exists (create alias bbb -> aaa)"], @req.errors.full_messages)
+      end
+    end
+
     context "with an associated forum topic" do
       setup do
         @admin = FactoryGirl.create(:admin_user)

test/unit/tag_alias_test.rb

@@ -60,7 +60,7 @@ class TagAliasTest < ActiveSupport::TestCase
         ta2 = FactoryGirl.build(:tag_alias, :antecedent_name => "aaa", :consequent_name => "bbb")
         ta2.save
         assert(ta2.errors.any?, "Tag alias should be invalid")
-        assert_equal("Tag alias can not create a transitive relation with another tag alias", ta2.errors.full_messages.join)
+        assert_equal("A tag alias for bbb already exists", ta2.errors.full_messages.join)
       end
     end