jaculabilis/danbooru
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

BURs: clean up parsing and error handling.

Browse Source 
* Don't raise exceptions when a BUR is invalid. Instead, use Rails
  validations to return errors. Fixes invalid BURs potentially raising
  exceptions in views. Also makes it so that each error in a BUR is
  reported, not just the first one.

* Revalidate the BUR whenever the script is edited, not just when the
  BUR is created. Ensures the BUR can't be broken by editing. Fixes a bug
  where forum threads could be broken by someone editing a BUR and
  breaking the syntax, thereby causing the BUR to raise an unparseable
  script error when the forum thread was viewed.

* Validate that removed aliases and implication actually exist.

* Validate that the tag actually exists when changing a tag's category.

* Combine bulk update request processor unit tests with main bulk update
  request unit tests.
This commit is contained in:
evazion
2020-08-24 16:41:52 -05:00
parent c295e233f2
commit 1ddcc661e1
5 changed files with 287 additions and 273 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
test/functional/bulk_update_requests_controller_test.rb
View File

@@ -7,7 +7,7 @@ class BulkUpdateRequestsControllerTest < ActionDispatch::IntegrationTest
@builder = create(:builder_user)
@admin = create(:admin_user)
as(@admin) { @forum_topic = create(:forum_topic, id: 100, category_id: 0) }
as(@user) { @bulk_update_request = create(:bulk_update_request, user: @user, forum_topic: @forum_topic) }
as(@user) { @bulk_update_request = create(:bulk_update_request, user: @user, forum_topic: @forum_topic, script: "create alias aaa -> bbb") }
end
context "#new" do
@@ -71,6 +71,12 @@ class BulkUpdateRequestsControllerTest < ActionDispatch::IntegrationTest
assert_response 403
assert_equal("create alias aaa -> bbb", @bulk_update_request.reload.script)
end
should "fail for an invalid script" do
put_auth bulk_update_request_path(@bulk_update_request.id), @user, params: { bulk_update_request: { script: "create alis gray -> grey" }}
assert_response :success
assert_equal("create alias aaa -> bbb", @bulk_update_request.reload.script)
end
end
context "#index" do

128
test/unit/bulk_update_request_processor_test.rb
View File

@@ -1,128 +0,0 @@
require 'test_helper'
class BulkUpdateRequestProcessorTest < ActiveSupport::TestCase
context "The bulk update request processor" do
setup do
CurrentUser.user = create(:admin_user)
CurrentUser.ip_addr = "127.0.0.1"
end
teardown do
CurrentUser.user = nil
CurrentUser.ip_addr = nil
end
context "category command" do
should "change the tag's category" do
@tag = create(:tag, name: "hello")
@script = "category hello -> artist\n"
@processor = BulkUpdateRequestProcessor.new(@script)
@processor.process!(CurrentUser.user)
assert_equal(Tag.categories.value_for("artist"), @tag.reload.category)
end
end
context "#affected_tags" do
setup do
create(:post, tag_string: "aaa")
create(:post, tag_string: "bbb")
create(:post, tag_string: "ccc")
create(:post, tag_string: "ddd")
create(:post, tag_string: "eee")
@script = "create alias aaa -> 000\n" +
"create implication bbb -> 111\n" +
"remove alias ccc -> 222\n" +
"remove implication ddd -> 333\n" +
"mass update eee -> 444\n"
end
should "return the correct tags" do
@processor = BulkUpdateRequestProcessor.new(@script)
assert_equal(%w(000 111 222 333 444 aaa bbb ccc ddd eee), @processor.affected_tags)
end
end
context "given a valid list" do
should "process it" do
@list = "create alias abc -> def\ncreate implication aaa -> bbb\n"
@processor = BulkUpdateRequestProcessor.new(@list)
@processor.process!(CurrentUser.user)
assert(TagAlias.exists?(antecedent_name: "abc", consequent_name: "def"))
assert(TagImplication.exists?(antecedent_name: "aaa", consequent_name: "bbb"))
end
end
context "given a list with an invalid command" do
should "throw an exception" do
@list = "zzzz abc -> def\n"
@processor = BulkUpdateRequestProcessor.new(@list)
assert_raises(BulkUpdateRequestProcessor::Error) do
@processor.process!(CurrentUser.user)
end
end
end
context "given a list with a logic error" do
should "throw an exception" do
@list = "remove alias zzz -> yyy\n"
@processor = BulkUpdateRequestProcessor.new(@list)
assert_raises(BulkUpdateRequestProcessor::Error) do
@processor.process!(CurrentUser.user)
end
end
end
should "rename an aliased tag's artist entry and wiki page" do
tag1 = create(:tag, name: "aaa", category: 1)
tag2 = create(:tag, name: "bbb")
wiki = create(:wiki_page, title: "aaa")
artist = create(:artist, name: "aaa")
@processor = BulkUpdateRequestProcessor.new("create alias aaa -> bbb")
@processor.process!(CurrentUser.user)
perform_enqueued_jobs
assert_equal("bbb", artist.reload.name)
assert_equal("bbb", wiki.reload.title)
end
context "remove alias and remove implication commands" do
setup do
@ta = create(:tag_alias, antecedent_name: "a", consequent_name: "b", status: "active")
@ti = create(:tag_implication, antecedent_name: "c", consequent_name: "d", status: "active")
@script = %{
remove alias a -> b
remove implication c -> d
}
@processor = BulkUpdateRequestProcessor.new(@script)
end
should "set aliases and implications as deleted" do
@processor.process!(CurrentUser.user)
assert_equal("deleted", @ta.reload.status)
assert_equal("deleted", @ti.reload.status)
end
should "create modactions for each removal" do
assert_difference(-> { ModAction.count }, 2) do
@processor.process!(CurrentUser.user)
end
end
should "only remove active aliases and implications" do
@ta2 = create(:tag_alias, antecedent_name: "a", consequent_name: "b", status: "pending")
@ti2 = create(:tag_implication, antecedent_name: "c", consequent_name: "d", status: "pending")
@processor.process!(CurrentUser.user)
assert_equal("pending", @ta2.reload.status)
assert_equal("pending", @ti2.reload.status)
end
end
end
end

272
test/unit/bulk_update_request_test.rb
View File

@@ -13,24 +13,209 @@ class BulkUpdateRequestTest < ActiveSupport::TestCase
CurrentUser.ip_addr = nil
end
should "parse the tags inside the script" do
@bur = create(:bulk_update_request, script: %{
create alias aaa -> 000
create implication bbb -> 111
remove alias ccc -> 222
remove implication ddd -> 333
mass update eee id:1 -fff ~ggg hhh* -> 444 -555
category iii -> meta
})
assert_equal(%w[000 111 222 333 444 aaa bbb ccc ddd eee iii], @bur.tags)
end
should_eventually "parse tags with tag type prefixes inside the script" do
@bur = create(:bulk_update_request, script: "mass update aaa -> artist:bbb")
assert_equal(%w[aaa bbb], @bur.tags)
end
context "when approving a BUR" do
context "the category command" do
should "change the tag's category" do
@tag = create(:tag, name: "hello")
@bur = create(:bulk_update_request, script: "category hello -> artist")
@bur.approve!(@admin)
assert_equal(true, @bur.valid?)
assert_equal(true, @tag.reload.artist?)
end
should "fail if the tag doesn't already exist" do
@bur = build(:bulk_update_request, script: "category hello -> artist")
assert_equal(false, @bur.valid?)
assert_equal(["Can't change category hello -> artist (the 'hello' tag doesn't exist)"], @bur.errors[:base])
end
end
context "the create alias command" do
setup do
@wiki = create(:wiki_page, title: "foo")
@artist = create(:artist, name: "foo")
@bur = create(:bulk_update_request, script: "create alias foo -> bar")
@bur.approve!(@admin)
perform_enqueued_jobs
end
should "create an alias" do
@alias = TagAlias.find_by(antecedent_name: "foo", consequent_name: "bar")
assert_equal(true, @alias.present?)
assert_equal(true, @alias.is_active?)
end
should "rename the aliased tag's artist entry and wiki page" do
assert_equal("bar", @artist.reload.name)
assert_equal("bar", @wiki.reload.title)
end
should "fail if the alias is invalid" do
create(:tag_alias, antecedent_name: "aaa", consequent_name: "bbb")
@bur = build(:bulk_update_request, script: "create alias bbb -> aaa")
assert_equal(false, @bur.valid?)
assert_equal(["Can't create alias bbb -> aaa (A tag alias for aaa already exists)"], @bur.errors.full_messages)
end
end
context "the create implication command" do
should "create an implication" do
@bur = create(:bulk_update_request, script: "create implication foo -> bar")
@bur.approve!(@admin)
perform_enqueued_jobs
@implication = TagImplication.find_by(antecedent_name: "foo", consequent_name: "bar")
assert_equal(true, @implication.present?)
assert_equal(true, @implication.is_active?)
end
should "fail for an implication that is redundant with an existing implication" do
create(:tag_implication, antecedent_name: "a", consequent_name: "b")
create(:tag_implication, antecedent_name: "b", consequent_name: "c")
@bur = build(:bulk_update_request, script: "imply a -> c")
assert_equal(false, @bur.valid?)
assert_equal(["Can't create implication a -> c (a already implies c through another implication)"], @bur.errors.full_messages)
end
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")
@bur = build(:bulk_update_request, script: "imply a -> b\nimply a -> c")
assert_equal(false, @bur.valid?)
assert_equal(["Can't create implication a -> c (a already implies c through another implication)"], @bur.errors.full_messages)
end
end
context "the remove alias command" do
should "remove an alias" do
create(:tag_alias, antecedent_name: "foo", consequent_name: "bar")
@bur = create(:bulk_update_request, script: "remove alias foo -> bar")
@bur.approve!(@admin)
@alias = TagAlias.find_by(antecedent_name: "foo", consequent_name: "bar")
assert_equal(true, @alias.present?)
assert_equal(true, @alias.is_deleted?)
end
should "fail if the alias isn't active" do
create(:tag_alias, antecedent_name: "foo", consequent_name: "bar", status: "pending")
@bur = build(:bulk_update_request, script: "remove alias foo -> bar")
assert_equal(false, @bur.valid?)
assert_equal(["Can't remove alias foo -> bar (alias doesn't exist)"], @bur.errors[:base])
end
should "fail if the alias doesn't already exist" do
@bur = build(:bulk_update_request, script: "remove alias foo -> bar")
assert_equal(false, @bur.valid?)
assert_equal(["Can't remove alias foo -> bar (alias doesn't exist)"], @bur.errors[:base])
end
end
context "the remove implication command" do
should "remove an implication" do
create(:tag_implication, antecedent_name: "foo", consequent_name: "bar", status: "active")
@bur = create(:bulk_update_request, script: "remove implication foo -> bar")
@bur.approve!(@admin)
@implication = TagImplication.find_by(antecedent_name: "foo", consequent_name: "bar")
assert_equal(true, @implication.present?)
assert_equal(true, @implication.is_deleted?)
end
should "fail if the implication isn't active" do
create(:tag_implication, antecedent_name: "foo", consequent_name: "bar", status: "pending")
@bur = build(:bulk_update_request, script: "remove implication foo -> bar")
assert_equal(false, @bur.valid?)
assert_equal(["Can't remove implication foo -> bar (implication doesn't exist)"], @bur.errors[:base])
end
should "fail if the implication doesn't already exist" do
@bur = build(:bulk_update_request, script: "remove implication foo -> bar")
assert_equal(false, @bur.valid?)
assert_equal(["Can't remove implication foo -> bar (implication doesn't exist)"], @bur.errors[:base])
end
end
context "the mass update command" do
setup do
@post = create(:post, tag_string: "foo")
@bur = create(:bulk_update_request, script: "mass update foo -> bar")
@bur.approve!(@admin)
perform_enqueued_jobs
end
should "update the tags" do
assert_equal("bar", @post.reload.tag_string)
end
end
end
context "when validating a script" do
context "an unparseable script" do
should "fail validation" do
@script = <<~EOS
create alias aaa -> 000
create alias bbb > 111
create alias ccc -> 222
EOS
@bur = build(:bulk_update_request, script: @script)
assert_equal(false, @bur.valid?)
assert_equal(["Invalid line: create alias bbb > 111"], @bur.errors[:base])
end
end
context "a script with extra whitespace" do
should "validate" do
@script = %{
create alias aaa -> 000
create alias bbb -> 111
}
@bur = create(:bulk_update_request, script: @script)
assert_equal(true, @bur.valid?)
end
end
end
context "when the script is updated" do
should "update the BUR's list of affected tags" do
create(:tag_alias, antecedent_name: "ccc", consequent_name: "222")
create(:tag_implication, antecedent_name: "ddd", consequent_name: "333")
create(:tag, name: "iii")
@script = <<~EOS
create alias aaa -> 000
create implication bbb -> 111
remove alias ccc -> 222
remove implication ddd -> 333
mass update eee id:1 -fff ~ggg hhh* -> 444 -555
category iii -> meta
EOS
@bur = create(:bulk_update_request, script: "create alias aaa -> bbb")
assert_equal(%w[aaa bbb], @bur.tags)
@bur.update!(script: @script)
assert_equal(%w(000 111 222 333 444 aaa bbb ccc ddd eee iii), @bur.tags)
end
end
context "on approval" do
setup do
@post = create(:post, tag_string: "foo aaa")
@@ -81,67 +266,6 @@ class BulkUpdateRequestTest < ActiveSupport::TestCase
assert_match(/zzz/, bur.forum_post.body)
end
context "that has an invalid alias" do
setup do
@alias1 = create(:tag_alias, antecedent_name: "aaa", consequent_name: "bbb", creator: @admin)
@req = FactoryBot.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 "for an implication that is redundant with an existing implication" do
should "not validate" do
FactoryBot.create(:tag_implication, :antecedent_name => "a", :consequent_name => "b")
FactoryBot.create(:tag_implication, :antecedent_name => "b", :consequent_name => "c")
bur = FactoryBot.build(:bulk_update_request, :script => "imply a -> c")
bur.save
assert_equal(["Error: a already implies c through another implication (create implication a -> c)"], bur.errors.full_messages)
end
end
context "for an implication that is redundant with another implication in the same BUR" do
setup do
FactoryBot.create(:tag_implication, :antecedent_name => "b", :consequent_name => "c")
@bur = FactoryBot.build(:bulk_update_request, :script => "imply a -> b\nimply a -> c")
@bur.save
end
should "not process" do
assert_no_difference("TagImplication.count") do
@bur.approve!(@admin)
end
end
should_eventually "not validate" do
assert_equal(["Error: a already implies c through another implication (create implication a -> c)"], @bur.errors.full_messages)
end
end
context "for a `category <tag> -> type` change" do
should "work" do
tag = Tag.find_or_create_by_name("tagme")
bur = FactoryBot.create(:bulk_update_request, :script => "category tagme -> meta")
bur.approve!(@admin)
assert_equal(Tag.categories.meta, tag.reload.category)
end
should "work for a new tag" do
bur = FactoryBot.create(:bulk_update_request, :script => "category new_tag -> meta")
bur.approve!(@admin)
assert_not_nil(Tag.find_by_name("new_tag"))
assert_equal(Tag.categories.meta, Tag.find_by_name("new_tag").category)
end
end
context "with an associated forum topic" do
setup do
@topic = create(:forum_topic, title: "[bulk] hoge", creator: @admin)