add secondary validations to aliases+implications+requests

This commit is contained in:
r888888888
2016-02-10 14:52:26 -08:00
parent 01eac1e587
commit d75546a4e4
18 changed files with 239 additions and 89 deletions

View File

@@ -1,17 +1,17 @@
class TagAliasRequestsController < ApplicationController
before_filter :member_only
rescue_from TagAliasRequest::ValidationError, :with => :rescue_exception
def new
end
def create
@tag_alias_request = TagAliasRequest.new(
params[:tag_alias_request][:antecedent_name],
params[:tag_alias_request][:consequent_name],
params[:tag_alias_request][:reason]
)
@tag_alias_request = TagAliasRequest.new(params[:tag_alias_request])
@tag_alias_request.create
redirect_to(forum_topic_path(@tag_alias_request.forum_topic))
if @tag_alias_request.invalid?
render :action => "new"
else
redirect_to forum_topic_path(@tag_alias_request.forum_topic)
end
end
end

View File

@@ -1,17 +1,17 @@
class TagImplicationRequestsController < ApplicationController
before_filter :member_only
rescue_from TagImplicationRequest::ValidationError, :with => :rescue_exception
def new
end
def create
@tag_implication_request = TagImplicationRequest.new(
params[:tag_implication_request][:antecedent_name],
params[:tag_implication_request][:consequent_name],
params[:tag_implication_request][:reason]
)
@tag_implication_request = TagImplicationRequest.new(params[:tag_implication_request])
@tag_implication_request.create
redirect_to(forum_topic_path(@tag_implication_request.forum_topic))
if @tag_implication_request.invalid?
render :action => "new"
else
redirect_to forum_topic_path(@tag_implication_request.forum_topic)
end
end
end

View File

@@ -1,10 +1,11 @@
class AliasAndImplicationImporter
attr_accessor :text, :commands, :forum_id, :rename_aliased_pages
attr_accessor :text, :commands, :forum_id, :rename_aliased_pages, :skip_secondary_validations
def initialize(text, forum_id, rename_aliased_pages = "0")
def initialize(text, forum_id, rename_aliased_pages = "0", skip_secondary_validations = true)
@forum_id = forum_id
@text = text
@rename_aliased_pages = rename_aliased_pages
@skip_secondary_validations = skip_secondary_validations
end
def process!
@@ -54,13 +55,13 @@ class AliasAndImplicationImporter
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])
tag_alias = TagAlias.new(:forum_topic_id => forum_id, :status => "pending", :antecedent_name => token[1], :consequent_name => token[2], :skip_secondary_validations => skip_secondary_validations)
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])
tag_implication = TagImplication.new(:forum_topic_id => forum_id, :status => "pending", :antecedent_name => token[1], :consequent_name => token[2], :skip_secondary_validations => skip_secondary_validations)
unless tag_implication.valid?
raise "Error: #{tag_implication.errors.full_messages.join("; ")} (create implication #{tag_implication.antecedent_name} -> #{tag_implication.consequent_name})"
end
@@ -81,7 +82,7 @@ private
tokens.map do |token|
case token[0]
when :create_alias
tag_alias = TagAlias.create(:forum_topic_id => forum_id, :status => "pending", :antecedent_name => token[1], :consequent_name => token[2])
tag_alias = TagAlias.create(:forum_topic_id => forum_id, :status => "pending", :antecedent_name => token[1], :consequent_name => token[2], :skip_secondary_validations => skip_secondary_validations)
unless tag_alias.valid?
raise "Error: #{tag_alias.errors.full_messages.join("; ")} (create alias #{tag_alias.antecedent_name} -> #{tag_alias.consequent_name})"
end
@@ -89,7 +90,7 @@ private
tag_alias.delay(:queue => "default").process!(false)
when :create_implication
tag_implication = TagImplication.create(:forum_topic_id => forum_id, :status => "pending", :antecedent_name => token[1], :consequent_name => token[2])
tag_implication = TagImplication.create(:forum_topic_id => forum_id, :status => "pending", :antecedent_name => token[1], :consequent_name => token[2], :skip_secondary_validations => skip_secondary_validations)
unless tag_implication.valid?
raise "Error: #{tag_implication.errors.full_messages.join("; ")} (create implication #{tag_implication.antecedent_name} -> #{tag_implication.consequent_name})"
end

View File

@@ -1,40 +1,73 @@
class TagAliasRequest
class ValidationError < Exception ; end
include ActiveModel::Validations
attr_reader :antecedent_name, :consequent_name, :reason, :tag_alias, :forum_topic
attr_reader :antecedent_name, :consequent_name, :reason, :skip_secondary_validations, :tag_alias, :forum_topic
def initialize(antecedent_name, consequent_name, reason)
@antecedent_name = antecedent_name.strip.tr(" ", "_")
@consequent_name = consequent_name.strip.tr(" ", "_")
@reason = reason
validate :validate_tag_alias
validate :validate_forum_topic
def initialize(attributes)
@antecedent_name = attributes[:antecedent_name].strip.tr(" ", "_")
@consequent_name = attributes[:consequent_name].strip.tr(" ", "_")
@reason = attributes[:reason]
self.skip_secondary_validations = attributes[:skip_secondary_validations]
end
def create
return false if invalid?
TagAlias.transaction do
create_alias
create_forum_topic
@tag_alias = build_tag_alias
@tag_alias.save
@forum_topic = build_forum_topic(@tag_alias.id)
@forum_topic.save
@tag_alias.update_attribute(:forum_topic_id, @forum_topic.id)
end
end
def create_alias
@tag_alias = TagAlias.create(:antecedent_name => antecedent_name, :consequent_name => consequent_name, :status => "pending")
if @tag_alias.errors.any?
raise ValidationError.new(@tag_alias.errors.full_messages.join("; "))
end
def build_tag_alias
TagAlias.new(
:antecedent_name => antecedent_name,
:consequent_name => consequent_name,
:status => "pending",
:skip_secondary_validations => skip_secondary_validations
)
end
def create_forum_topic
@forum_topic = ForumTopic.create(
def build_forum_topic(tag_alias_id)
ForumTopic.new(
:title => "Tag alias: #{antecedent_name} -> #{consequent_name}",
:original_post_attributes => {
:body => "create alias [[#{antecedent_name}]] -> [[#{consequent_name}]]\n\n\"Link to alias\":/tag_aliases?search[id]=#{tag_alias.id}\n\n#{reason}"
:body => "create alias [[#{antecedent_name}]] -> [[#{consequent_name}]]\n\n\"Link to alias\":/tag_aliases?search[id]=#{tag_alias_id}\n\n#{reason}"
},
:category_id => 1
)
if @forum_topic.errors.any?
raise ValidationError.new(@forum_topic.errors.full_messages.join("; "))
end
end
tag_alias.update_attribute(:forum_topic_id, @forum_topic.id)
def validate_tag_alias
ta = @tag_alias || build_tag_alias
if ta.invalid?
self.errors.add(:base, ta.errors.full_messages.join("; "))
return false
end
end
def validate_forum_topic
ft = @forum_topic || build_forum_topic(nil)
if ft.invalid?
self.errors.add(:base, ft.errors.full_messages.join("; "))
return false
end
end
def skip_secondary_validations=(v)
if v == "1" or v == true
@skip_secondary_validations = true
else
@skip_secondary_validations = false
end
end
end

View File

@@ -1,40 +1,73 @@
class TagImplicationRequest
class ValidationError < Exception ; end
include ActiveModel::Validations
attr_reader :antecedent_name, :consequent_name, :reason, :tag_implication, :forum_topic
attr_reader :antecedent_name, :consequent_name, :reason, :tag_implication, :forum_topic, :skip_secondary_validations
def initialize(antecedent_name, consequent_name, reason)
@antecedent_name = antecedent_name.strip.tr(" ", "_")
@consequent_name = consequent_name.strip.tr(" ", "_")
@reason = reason
validate :validate_tag_implication
validate :validate_forum_topic
def initialize(attributes)
@antecedent_name = attributes[:antecedent_name].strip.tr(" ", "_")
@consequent_name = attributes[:consequent_name].strip.tr(" ", "_")
@reason = attributes[:reason]
self.skip_secondary_validations = attributes[:skip_secondary_validations]
end
def create
return false if invalid?
TagImplication.transaction do
create_implication
create_forum_topic
@tag_implication = build_tag_implication
@tag_implication.save
@forum_topic = build_forum_topic(@tag_implication.id)
@forum_topic.save
@tag_implication.update_attribute(:forum_topic_id, @forum_topic.id)
end
end
def create_implication
@tag_implication = TagImplication.create(:antecedent_name => antecedent_name, :consequent_name => consequent_name, :status => "pending")
if @tag_implication.errors.any?
raise ValidationError.new(@tag_implication.errors.full_messages.join("; "))
end
def build_tag_implication
TagImplication.new(
:antecedent_name => antecedent_name,
:consequent_name => consequent_name,
:status => "pending",
:skip_secondary_validations => skip_secondary_validations
)
end
def create_forum_topic
@forum_topic = ForumTopic.create(
def build_forum_topic(tag_implication_id)
ForumTopic.new(
:title => "Tag implication: #{antecedent_name} -> #{consequent_name}",
:original_post_attributes => {
:body => "create implication [[#{antecedent_name}]] -> [[#{consequent_name}]]\n\n\"Link to implication\":/tag_implications?search[id]=#{tag_implication.id}\n\n#{reason}"
:body => "create implication [[#{antecedent_name}]] -> [[#{consequent_name}]]\n\n\"Link to implication\":/tag_implications?search[id]=#{tag_implication_id}\n\n#{reason}"
},
:category_id => 1
)
if @forum_topic.errors.any?
raise ValidationError.new(@forum_topic.errors.full_messages.join("; "))
end
end
tag_implication.update_attribute(:forum_topic_id, @forum_topic.id)
def validate_tag_implication
ti = @tag_implication || build_tag_implication
if ti.invalid?
self.errors.add(:base, ti.errors.full_messages.join("; "))
return false
end
end
def validate_forum_topic
ft = @forum_topic || build_forum_topic(nil)
if ft.invalid?
self.errors.add(:base, ft.errors.full_messages.join("; "))
return false
end
end
def skip_secondary_validations=(v)
if v == "1" or v == true
@skip_secondary_validations = true
else
@skip_secondary_validations = false
end
end
end

View File

@@ -1,5 +1,5 @@
class BulkUpdateRequest < ActiveRecord::Base
attr_accessor :title, :reason
attr_accessor :title, :reason, :skip_secondary_validations
belongs_to :user
belongs_to :forum_topic
@@ -11,7 +11,7 @@ class BulkUpdateRequest < ActiveRecord::Base
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 :user_id, :forum_topic_id, :script, :title, :reason, :skip_secondary_validations
attr_accessible :status, :as => [:admin]
before_validation :initialize_attributes, :on => :create
before_validation :normalize_text
@@ -33,7 +33,7 @@ class BulkUpdateRequest < ActiveRecord::Base
extend SearchMethods
def approve!
AliasAndImplicationImporter.new(script, forum_topic_id, "1").process!
AliasAndImplicationImporter.new(script, forum_topic_id, "1", true).process!
update_forum_topic_for_approve
update_attribute(:status, "approved")
@@ -151,9 +151,17 @@ class BulkUpdateRequest < ActiveRecord::Base
self.script = script.downcase
end
def skip_secondary_validations=(v)
if v == "1" or v == true
@skip_secondary_validations = true
else
@skip_secondary_validations = false
end
end
def validate_script
begin
AliasAndImplicationImporter.new(script, forum_topic_id, "1").validate!
AliasAndImplicationImporter.new(script, forum_topic_id, "1", skip_secondary_validations).validate!
rescue RuntimeError => e
self.errors[:base] = e.message
return false

View File

@@ -1,4 +1,6 @@
class TagAlias < ActiveRecord::Base
attr_accessor :skip_secondary_validations
before_save :ensure_tags_exist
after_save :clear_all_cache
after_destroy :clear_all_cache
@@ -8,11 +10,11 @@ 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
validate :consequent_has_wiki_page, :on => :create
validate :mininum_antecedent_count, :on => :create
belongs_to :creator, :class_name => "User"
belongs_to :forum_topic
attr_accessible :antecedent_name, :consequent_name, :forum_topic_id, :status
attr_accessible :antecedent_name, :consequent_name, :forum_topic_id, :status, :skip_secondary_validations
module SearchMethods
def name_matches(name)
@@ -293,7 +295,7 @@ class TagAlias < ActiveRecord::Base
end
def consequent_has_wiki_page
return if !Danbooru.config.strict_tag_requirements
return if skip_secondary_validations
unless WikiPage.titled(consequent_name).exists?
self.errors[:base] = "The #{consequent_name} tag needs a corresponding wiki page"
@@ -302,7 +304,7 @@ class TagAlias < ActiveRecord::Base
end
def mininum_antecedent_count
return if !Danbooru.config.strict_tag_requirements
return if skip_secondary_validations
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"

View File

@@ -1,4 +1,6 @@
class TagImplication < ActiveRecord::Base
attr_accessor :skip_secondary_validations
before_save :update_descendant_names
after_save :update_descendant_names_for_parents
after_destroy :update_descendant_names_for_parents
@@ -12,8 +14,8 @@ 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
validate :wiki_pages_present, :on => :create
attr_accessible :antecedent_name, :consequent_name, :descendant_names, :forum_topic_id, :status, :forum_topic, :skip_secondary_validations
module DescendantMethods
extend ActiveSupport::Concern
@@ -258,7 +260,7 @@ class TagImplication < ActiveRecord::Base
end
def wiki_pages_present
return if !Danbooru.config.strict_tag_requirements
return if skip_secondary_validations
unless WikiPage.titled(consequent_name).exists?
self.errors[:base] = "The #{consequent_name} tag needs a corresponding wiki page"

View File

@@ -20,6 +20,16 @@ update aaa -> bbb
<%= dtext_field "bulk_update_request", "reason", :name => "Reason" %>
</div>
<% if @bulk_update_request.errors.any? %>
<div class="input">
<label class="checkbox optional" for="bulk_update_request_skip_secondary_validations">
<%= check_box "bulk_update_request", "skip_secondary_validations" %>
Skip validations
</label>
<p class="hint">You can ignore the wiki page and minimum count requirements</p>
</div>
<% end %>
<%= f.input :forum_topic_id, :hint => " (optional)" %>
<%= f.button :submit, :value => "Submit" %>
<%= dtext_preview_button "bulk_update_request", "reason" %>

View File

@@ -20,7 +20,7 @@
<%= link_to "Approve", approve_bulk_update_request_path(request), :remote => true, :method => :post %> |
<% end %>
<% if request.editable?(CurrentUser.user) %>
<%= link_to "Delete", bulk_update_request_path(request), :method => :delete, :remote => true, :data => {:confirm => "Are you sure you want to delete this bulk update request?"} %> |
<%= link_to "Reject", bulk_update_request_path(request), :method => :delete, :remote => true, :data => {:confirm => "Are you sure you want to reject this bulk update request?"} %> |
<%= link_to "Edit", edit_bulk_update_request_path(request) %>
<% end %>
</td>

View File

@@ -2,6 +2,8 @@
<div id="a-request-new">
<h1>Tag Alias Request</h1>
<%= error_messages_for :tag_alias_request %>
<p>You can request a new tag alias be created. This will create a corresponding forum topic for community review.</p>
<%= form_tag(tag_alias_request_path, :class => "simple_form") do %>
@@ -19,6 +21,14 @@
<%= dtext_field "tag_alias_request", "reason", :name => "Reason" %>
</div>
<div class="input">
<label class="checkbox optional" for="tag_alias_request_skip_secondary_validations">
<%= check_box "tag_alias_request", "skip_secondary_validations" %>
Skip validations
</label>
<p class="hint">You can ignore the wiki page and minimum count requirements</p>
</div>
<div class="input">
<%= submit_tag "Submit" %>
<%= dtext_preview_button "tag_alias_request", "reason" %>

View File

@@ -2,6 +2,8 @@
<div id="a-request-new">
<h1>Tag Implication Request</h1>
<%= error_messages_for :tag_implication_request %>
<p>You can request a new tag implication be created. This will create a corresponding forum topic for community review.</p>
<%= form_tag(tag_implication_request_path, :class => "simple_form") do %>
@@ -19,6 +21,14 @@
<%= dtext_field "tag_implication_request", "reason", :name => "Reason" %>
</div>
<div class="input">
<label class="checkbox optional" for="tag_implication_request_skip_secondary_validations">
<%= check_box "tag_implication_request", "skip_secondary_validations" %>
Skip validations
</label>
<p class="hint">You can ignore the wiki page and minimum count requirements</p>
</div>
<div class="input">
<%= submit_tag "Submit" %>
<%= dtext_preview_button "tag_implication_request", "reason" %>

View File

@@ -2,5 +2,6 @@ FactoryGirl.define do
factory(:bulk_update_request) do |f|
title "xxx"
script "create alias aaa -> bbb"
skip_secondary_validations true
end
end

View File

@@ -3,6 +3,7 @@ FactoryGirl.define do
antecedent_name "aaa"
consequent_name "bbb"
status "active"
skip_secondary_validations true
after(:create) do |tag_alias|
unless tag_alias.status == "pending"

View File

@@ -3,7 +3,8 @@ FactoryGirl.define do
antecedent_name "aaa"
consequent_name "bbb"
status "active"
skip_secondary_validations true
after(:create) do |tag_implication|
unless tag_implication.status == "pending"
tag_implication.process!

View File

@@ -17,11 +17,31 @@ class BulkUpdateRequestsControllerTest < ActionController::TestCase
context "#create" do
should "succeed" do
assert_difference("BulkUpdateRequest.count", 1) do
post :create, {:bulk_update_request => {:script => "create alias aaa -> bbb", :title => "xxx"}}, {:user_id => @user.id}
post :create, {:bulk_update_request => {:skip_secondary_validations => "1", :script => "create alias aaa -> bbb", :title => "xxx"}}, {:user_id => @user.id}
end
end
end
context "#update" do
setup do
CurrentUser.scoped(@user) do
@bulk_update_request = FactoryGirl.create(:bulk_update_request)
end
end
should "still handle enabled secondary validations correctly" do
post :update, {:id => @bulk_update_request.id, :bulk_update_request => {:script => "create alias zzz -> 222", :skip_secondary_validations => "0"}}, {:user_id => @user.id}
@bulk_update_request.reload
assert_equal("create alias aaa -> bbb", @bulk_update_request.script)
end
should "still handle disabled secondary validations correctly" do
post :update, {:id => @bulk_update_request.id, :bulk_update_request => {:script => "create alias zzz -> 222", :skip_secondary_validations => "1"}}, {:user_id => @user.id}
@bulk_update_request.reload
assert_equal("create alias zzz -> 222", @bulk_update_request.script)
end
end
context "#index" do
setup do
CurrentUser.scoped(@user) do

View File

@@ -16,28 +16,37 @@ class TagAliasRequestTest < ActiveSupport::TestCase
CurrentUser.ip_addr = nil
end
should "raise an exception if invalid" do
assert_raises(TagAliasRequest::ValidationError) do
TagAliasRequest.new("", "", "reason").create
end
should "handle invalid attributes" do
tar = TagAliasRequest.new(:antecedent_name => "", :consequent_name => "", :reason => "reason", :skip_secondary_validations => true)
tar.create
assert(tar.invalid?)
end
should "handle secondary validations" do
tar = TagAliasRequest.new(:antecedent_name => "aaa", :consequent_name => "bbb", :reason => "reason", :skip_secondary_validations => false)
tar.create
assert(tar.invalid?)
end
should "create a tag alias" do
assert_difference("TagAlias.count", 1) do
TagAliasRequest.new("aaa", "bbb", "reason").create
tar = TagAliasRequest.new(:antecedent_name => "aaa", :consequent_name => "bbb", :reason => "reason", :skip_secondary_validations => true)
tar.create
end
assert_equal("pending", TagAlias.last.status)
end
should "create a forum topic" do
assert_difference("ForumTopic.count", 1) do
TagAliasRequest.new("aaa", "bbb", "reason").create
tar = TagAliasRequest.new(:antecedent_name => "aaa", :consequent_name => "bbb", :reason => "reason", :skip_secondary_validations => true)
tar.create
end
end
should "create a forum post" do
assert_difference("ForumPost.count", 1) do
TagAliasRequest.new("aaa", "bbb", "reason").create
tar = TagAliasRequest.new(:antecedent_name => "aaa", :consequent_name => "bbb", :reason => "reason", :skip_secondary_validations => true)
tar.create
end
end
end

View File

@@ -16,28 +16,37 @@ class TagImplicationRequestTest < ActiveSupport::TestCase
CurrentUser.ip_addr = nil
end
should "raise an exception if invalid" do
assert_raises(TagImplicationRequest::ValidationError) do
TagImplicationRequest.new("", "", "reason").create
end
should "handle invalid attributes" do
tir = TagImplicationRequest.new(:antecedent_name => "", :consequent_name => "", :reason => "reason", :skip_secondary_validations => true)
tir.create
assert(tir.invalid?)
end
should "handle secondary validations" do
tir = TagImplicationRequest.new(:antecedent_name => "aaa", :consequent_name => "bbb", :reason => "reason", :skip_secondary_validations => false)
tir.create
assert(tir.invalid?)
end
should "create a tag implication" do
assert_difference("TagImplication.count", 1) do
TagImplicationRequest.new("aaa", "bbb", "reason").create
tir = TagImplicationRequest.new(:antecedent_name => "aaa", :consequent_name => "bbb", :reason => "reason", :skip_secondary_validations => true)
tir.create
end
assert_equal("pending", TagImplication.last.status)
end
should "create a forum topic" do
assert_difference("ForumTopic.count", 1) do
TagImplicationRequest.new("aaa", "bbb", "reason").create
tir = TagImplicationRequest.new(:antecedent_name => "aaa", :consequent_name => "bbb", :reason => "reason", :skip_secondary_validations => true)
tir.create
end
end
should "create a forum post" do
assert_difference("ForumPost.count", 1) do
TagImplicationRequest.new("aaa", "bbb", "reason").create
tir = TagImplicationRequest.new(:antecedent_name => "aaa", :consequent_name => "bbb", :reason => "reason", :skip_secondary_validations => true)
tir.create
end
end
end