BURs: rename AliasAndImplicationImporter to BulkUpdateRequestProcessor.
The name AliasAndImplicationImporter is a holdover from the time before bulk update requests existed. This was a bad name because it doesn't do any actual importing, instead it's used for parsing and executing bulk update requests.
This commit is contained in:
@@ -1,65 +1,50 @@
|
||||
class AliasAndImplicationImporter
|
||||
class Error < RuntimeError; end
|
||||
attr_accessor :text, :commands, :forum_id, :skip_secondary_validations
|
||||
class BulkUpdateRequestProcessor
|
||||
extend Memoist
|
||||
|
||||
def initialize(text, forum_id, skip_secondary_validations = true)
|
||||
@forum_id = forum_id
|
||||
class Error < StandardError; end
|
||||
attr_accessor :text, :forum_topic_id, :skip_secondary_validations
|
||||
|
||||
def initialize(text, forum_topic_id: nil, skip_secondary_validations: true)
|
||||
@forum_topic_id = forum_topic_id
|
||||
@text = text
|
||||
@skip_secondary_validations = skip_secondary_validations
|
||||
end
|
||||
|
||||
def process!(approver = CurrentUser.user)
|
||||
tokens = AliasAndImplicationImporter.tokenize(text)
|
||||
parse(tokens, approver)
|
||||
end
|
||||
|
||||
def validate!
|
||||
tokens = AliasAndImplicationImporter.tokenize(text)
|
||||
validate(tokens)
|
||||
end
|
||||
|
||||
def self.tokenize(text)
|
||||
def tokens
|
||||
text.split(/\r\n|\r|\n/).reject(&:blank?).map do |line|
|
||||
line = line.gsub(/[[:space:]]+/, " ").strip
|
||||
|
||||
if line =~ /^(?:create alias|aliasing|alias) (\S+) -> (\S+)$/i
|
||||
[:create_alias, $1, $2]
|
||||
|
||||
elsif line =~ /^(?:create implication|implicating|implicate|imply) (\S+) -> (\S+)$/i
|
||||
[:create_implication, $1, $2]
|
||||
|
||||
elsif line =~ /^(?:remove alias|unaliasing|unalias) (\S+) -> (\S+)$/i
|
||||
[:remove_alias, $1, $2]
|
||||
|
||||
elsif line =~ /^(?:remove implication|unimplicating|unimplicate|unimply) (\S+) -> (\S+)$/i
|
||||
[:remove_implication, $1, $2]
|
||||
|
||||
elsif line =~ /^(?:mass update|updating|update|change) (.+?) -> (.*)$/i
|
||||
[:mass_update, $1, $2]
|
||||
|
||||
elsif line =~ /^category (\S+) -> (#{Tag.categories.regexp})/
|
||||
[:change_category, $1, $2]
|
||||
|
||||
elsif line.strip.empty?
|
||||
# do nothing
|
||||
|
||||
else
|
||||
raise Error, "Unparseable line: #{line}"
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
def validate(tokens)
|
||||
def validate!
|
||||
tokens.map do |token|
|
||||
case token[0]
|
||||
when :create_alias
|
||||
tag_alias = TagAlias.new(creator: User.system, forum_topic_id: forum_id, status: "pending", antecedent_name: token[1], consequent_name: token[2], skip_secondary_validations: skip_secondary_validations)
|
||||
tag_alias = TagAlias.new(creator: User.system, forum_topic_id: forum_topic_id, status: "pending", antecedent_name: token[1], consequent_name: token[2], skip_secondary_validations: skip_secondary_validations)
|
||||
unless tag_alias.valid?
|
||||
raise Error, "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(creator: User.system, forum_topic_id: forum_id, status: "pending", antecedent_name: token[1], consequent_name: token[2], skip_secondary_validations: skip_secondary_validations)
|
||||
tag_implication = TagImplication.new(creator: User.system, forum_topic_id: forum_topic_id, status: "pending", antecedent_name: token[1], consequent_name: token[2], skip_secondary_validations: skip_secondary_validations)
|
||||
unless tag_implication.valid?
|
||||
raise Error, "Error: #{tag_implication.errors.full_messages.join("; ")} (create implication #{tag_implication.antecedent_name} -> #{tag_implication.consequent_name})"
|
||||
end
|
||||
@@ -73,37 +58,19 @@ class AliasAndImplicationImporter
|
||||
end
|
||||
end
|
||||
|
||||
def affected_tags
|
||||
AliasAndImplicationImporter.tokenize(text).flat_map do |type, *args|
|
||||
case type
|
||||
when :create_alias, :remove_alias, :create_implication, :remove_implication
|
||||
[args[0], args[1]]
|
||||
when :mass_update
|
||||
tags = PostQueryBuilder.new(args[0]).tags + PostQueryBuilder.new(args[1]).tags
|
||||
tags.reject(&:negated).reject(&:optional).reject(&:wildcard).map(&:name)
|
||||
when :change_category
|
||||
args[0]
|
||||
end
|
||||
end.sort.uniq
|
||||
rescue Error
|
||||
[]
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def parse(tokens, approver)
|
||||
def process!(approver)
|
||||
ActiveRecord::Base.transaction do
|
||||
tokens.map do |token|
|
||||
case token[0]
|
||||
when :create_alias
|
||||
tag_alias = TagAlias.create(creator: approver, forum_topic_id: forum_id, status: "pending", antecedent_name: token[1], consequent_name: token[2], skip_secondary_validations: skip_secondary_validations)
|
||||
tag_alias = TagAlias.create(creator: approver, forum_topic_id: forum_topic_id, status: "pending", antecedent_name: token[1], consequent_name: token[2], skip_secondary_validations: skip_secondary_validations)
|
||||
unless tag_alias.valid?
|
||||
raise Error, "Error: #{tag_alias.errors.full_messages.join("; ")} (create alias #{tag_alias.antecedent_name} -> #{tag_alias.consequent_name})"
|
||||
end
|
||||
tag_alias.approve!(approver: approver)
|
||||
|
||||
when :create_implication
|
||||
tag_implication = TagImplication.create(creator: approver, forum_topic_id: forum_id, status: "pending", antecedent_name: token[1], consequent_name: token[2], skip_secondary_validations: skip_secondary_validations)
|
||||
tag_implication = TagImplication.create(creator: approver, forum_topic_id: forum_topic_id, status: "pending", antecedent_name: token[1], consequent_name: token[2], skip_secondary_validations: skip_secondary_validations)
|
||||
unless tag_implication.valid?
|
||||
raise Error, "Error: #{tag_implication.errors.full_messages.join("; ")} (create implication #{tag_implication.antecedent_name} -> #{tag_implication.consequent_name})"
|
||||
end
|
||||
@@ -133,4 +100,37 @@ class AliasAndImplicationImporter
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
def affected_tags
|
||||
tokens.flat_map do |type, *args|
|
||||
case type
|
||||
when :create_alias, :remove_alias, :create_implication, :remove_implication
|
||||
[args[0], args[1]]
|
||||
when :mass_update
|
||||
tags = PostQueryBuilder.new(args[0]).tags + PostQueryBuilder.new(args[1]).tags
|
||||
tags.reject(&:negated).reject(&:optional).reject(&:wildcard).map(&:name)
|
||||
when :change_category
|
||||
args[0]
|
||||
end
|
||||
end.sort.uniq
|
||||
rescue Error
|
||||
[]
|
||||
end
|
||||
|
||||
def to_dtext
|
||||
tokens.map do |token|
|
||||
case token[0]
|
||||
when :create_alias, :create_implication, :remove_alias, :remove_implication
|
||||
"#{token[0].to_s.tr("_", " ")} [[#{token[1]}]] -> [[#{token[2]}]]"
|
||||
when :mass_update
|
||||
"mass update {{#{token[1]}}} -> #{token[2]}"
|
||||
when :change_category
|
||||
"category [[#{token[1]}]] -> #{token[2]}"
|
||||
else
|
||||
raise "Unknown token: #{token[0]}"
|
||||
end
|
||||
end.join("\n")
|
||||
end
|
||||
|
||||
memoize :tokens
|
||||
end
|
||||
@@ -105,9 +105,9 @@ class DText
|
||||
end
|
||||
elsif obj.is_a?(BulkUpdateRequest)
|
||||
if obj.script.size < 700
|
||||
embedded_script = obj.script_with_links
|
||||
embedded_script = obj.processor.to_dtext
|
||||
else
|
||||
embedded_script = "[expand]#{obj.script_with_links}[/expand]"
|
||||
embedded_script = "[expand]#{obj.processor.to_dtext}[/expand]"
|
||||
end
|
||||
|
||||
if obj.is_approved?
|
||||
|
||||
@@ -75,12 +75,12 @@ class BulkUpdateRequest < ApplicationRecord
|
||||
def approve!(approver)
|
||||
transaction do
|
||||
CurrentUser.scoped(approver) do
|
||||
AliasAndImplicationImporter.new(script, forum_topic_id, true).process!
|
||||
processor.process!(approver)
|
||||
update!(status: "approved", approver: approver, skip_secondary_validations: true)
|
||||
forum_updater.update("The #{bulk_update_request_link} (forum ##{forum_post.id}) has been approved by @#{approver.name}.")
|
||||
end
|
||||
end
|
||||
rescue AliasAndImplicationImporter::Error => x
|
||||
rescue BulkUpdateRequestProcessor::Error => x
|
||||
self.approver = approver
|
||||
CurrentUser.scoped(approver) do
|
||||
forum_updater.update("The #{bulk_update_request_link} (forum ##{forum_post.id}) has failed: #{x}")
|
||||
@@ -110,8 +110,8 @@ class BulkUpdateRequest < ApplicationRecord
|
||||
|
||||
module ValidationMethods
|
||||
def validate_script
|
||||
AliasAndImplicationImporter.new(script, forum_topic_id, skip_secondary_validations).validate!
|
||||
rescue AliasAndImplicationImporter::Error => e
|
||||
processor.validate!
|
||||
rescue BulkUpdateRequestProcessor::Error => e
|
||||
errors[:base] << e.message
|
||||
end
|
||||
end
|
||||
@@ -120,38 +120,22 @@ class BulkUpdateRequest < ApplicationRecord
|
||||
include ApprovalMethods
|
||||
include ValidationMethods
|
||||
|
||||
def script_with_links
|
||||
tokens = AliasAndImplicationImporter.tokenize(script)
|
||||
lines = tokens.map do |token|
|
||||
case token[0]
|
||||
when :create_alias, :create_implication, :remove_alias, :remove_implication
|
||||
"#{token[0].to_s.tr("_", " ")} [[#{token[1]}]] -> [[#{token[2]}]]"
|
||||
|
||||
when :mass_update
|
||||
"mass update {{#{token[1]}}} -> #{token[2]}"
|
||||
|
||||
when :change_category
|
||||
"category [[#{token[1]}]] -> #{token[2]}"
|
||||
|
||||
else
|
||||
raise "Unknown token: #{token[0]}"
|
||||
end
|
||||
end
|
||||
lines.join("\n")
|
||||
end
|
||||
|
||||
def normalize_text
|
||||
self.script = script.downcase
|
||||
end
|
||||
|
||||
def update_tags
|
||||
self.tags = AliasAndImplicationImporter.new(script, nil).affected_tags
|
||||
self.tags = processor.affected_tags
|
||||
end
|
||||
|
||||
def skip_secondary_validations=(v)
|
||||
@skip_secondary_validations = v.to_s.truthy?
|
||||
end
|
||||
|
||||
def processor
|
||||
@processor ||= BulkUpdateRequestProcessor.new(script, forum_topic_id: forum_topic_id, skip_secondary_validations: skip_secondary_validations)
|
||||
end
|
||||
|
||||
def is_pending?
|
||||
status == "pending"
|
||||
end
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
<% dtext_data = DText.preprocess(bulk_update_requests.map(&:script_with_links)) %>
|
||||
<% dtext_data = DText.preprocess(bulk_update_requests.map(&:processor).map(&:to_dtext)) %>
|
||||
|
||||
<%= table_for bulk_update_requests, width: "100%" do |t| %>
|
||||
<% t.column "Request" do |request| %>
|
||||
@@ -9,7 +9,7 @@
|
||||
<% end %>
|
||||
|
||||
<div class="prose">
|
||||
<%= format_text(request.script_with_links, data: dtext_data) %>
|
||||
<%= format_text(request.processor.to_dtext, data: dtext_data) %>
|
||||
</div>
|
||||
<% end %>
|
||||
<% t.column "Votes" do |request| %>
|
||||
|
||||
@@ -14,7 +14,7 @@
|
||||
<div style="margin: 1em 0;">
|
||||
<h2>Script</h2>
|
||||
<div class="prose">
|
||||
<%= format_text @bulk_update_request.script_with_links %>
|
||||
<%= format_text @bulk_update_request.processor.to_dtext %>
|
||||
</div>
|
||||
|
||||
<%= render "bur_edit_links", bur: @bulk_update_request %>
|
||||
|
||||
@@ -4,7 +4,7 @@ require_relative "../../config/environment"
|
||||
|
||||
BulkUpdateRequest.transaction do
|
||||
BulkUpdateRequest.find_each do |request|
|
||||
request.tags = AliasAndImplicationImporter.new(request.script, nil).affected_tags
|
||||
request.tags = request.processor.affected_tags
|
||||
request.save!(validate: false)
|
||||
puts "bur id=#{request.id} tags=#{request.tags}"
|
||||
end
|
||||
|
||||
@@ -1,9 +1,9 @@
|
||||
require 'test_helper'
|
||||
|
||||
class AliasAndImplicationImporterTest < ActiveSupport::TestCase
|
||||
context "The alias and implication importer" do
|
||||
class BulkUpdateRequestProcessorTest < ActiveSupport::TestCase
|
||||
context "The bulk update request processor" do
|
||||
setup do
|
||||
CurrentUser.user = FactoryBot.create(:admin_user)
|
||||
CurrentUser.user = create(:admin_user)
|
||||
CurrentUser.ip_addr = "127.0.0.1"
|
||||
end
|
||||
|
||||
@@ -13,26 +13,23 @@ class AliasAndImplicationImporterTest < ActiveSupport::TestCase
|
||||
end
|
||||
|
||||
context "category command" do
|
||||
setup do
|
||||
@tag = Tag.find_or_create_by_name("hello")
|
||||
@list = "category hello -> artist\n"
|
||||
@importer = AliasAndImplicationImporter.new(@list, nil)
|
||||
end
|
||||
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)
|
||||
|
||||
should "work" do
|
||||
@importer.process!
|
||||
@tag.reload
|
||||
assert_equal(Tag.categories.value_for("artist"), @tag.category)
|
||||
assert_equal(Tag.categories.value_for("artist"), @tag.reload.category)
|
||||
end
|
||||
end
|
||||
|
||||
context "#affected_tags" do
|
||||
setup do
|
||||
FactoryBot.create(:post, tag_string: "aaa")
|
||||
FactoryBot.create(:post, tag_string: "bbb")
|
||||
FactoryBot.create(:post, tag_string: "ccc")
|
||||
FactoryBot.create(:post, tag_string: "ddd")
|
||||
FactoryBot.create(:post, tag_string: "eee")
|
||||
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" +
|
||||
@@ -41,48 +38,41 @@ class AliasAndImplicationImporterTest < ActiveSupport::TestCase
|
||||
"mass update eee -> 444\n"
|
||||
end
|
||||
|
||||
subject { AliasAndImplicationImporter.new(@script, nil) }
|
||||
|
||||
should "return the correct tags" do
|
||||
assert_equal(%w(aaa 000 bbb 111 ccc 222 ddd 333 eee 444), subject.affected_tags)
|
||||
@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
|
||||
setup do
|
||||
@list = "create alias abc -> def\ncreate implication aaa -> bbb\n"
|
||||
@importer = AliasAndImplicationImporter.new(@list, nil)
|
||||
end
|
||||
|
||||
should "process it" do
|
||||
@importer.process!
|
||||
@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
|
||||
setup do
|
||||
@list = "zzzz abc -> def\n"
|
||||
@importer = AliasAndImplicationImporter.new(@list, nil)
|
||||
end
|
||||
|
||||
should "throw an exception" do
|
||||
assert_raises(RuntimeError) do
|
||||
@importer.process!
|
||||
@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
|
||||
setup do
|
||||
@list = "remove alias zzz -> yyy\n"
|
||||
@importer = AliasAndImplicationImporter.new(@list, nil)
|
||||
end
|
||||
|
||||
should "throw an exception" do
|
||||
assert_raises(RuntimeError) do
|
||||
@importer.process!
|
||||
@list = "remove alias zzz -> yyy\n"
|
||||
@processor = BulkUpdateRequestProcessor.new(@list)
|
||||
|
||||
assert_raises(BulkUpdateRequestProcessor::Error) do
|
||||
@processor.process!(CurrentUser.user)
|
||||
end
|
||||
end
|
||||
end
|
||||
@@ -92,26 +82,28 @@ class AliasAndImplicationImporterTest < ActiveSupport::TestCase
|
||||
tag2 = create(:tag, name: "bbb")
|
||||
wiki = create(:wiki_page, title: "aaa")
|
||||
artist = create(:artist, name: "aaa")
|
||||
@importer = AliasAndImplicationImporter.new("create alias aaa -> bbb", "")
|
||||
@importer.process!
|
||||
|
||||
@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 = FactoryBot.create(:tag_alias, antecedent_name: "a", consequent_name: "b", status: "active")
|
||||
@ti = FactoryBot.create(:tag_implication, antecedent_name: "c", consequent_name: "d", status: "active")
|
||||
@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
|
||||
}
|
||||
@importer = AliasAndImplicationImporter.new(@script, nil)
|
||||
@processor = BulkUpdateRequestProcessor.new(@script)
|
||||
end
|
||||
|
||||
should "set aliases and implications as deleted" do
|
||||
@importer.process!
|
||||
@processor.process!(CurrentUser.user)
|
||||
|
||||
assert_equal("deleted", @ta.reload.status)
|
||||
assert_equal("deleted", @ti.reload.status)
|
||||
@@ -119,15 +111,15 @@ class AliasAndImplicationImporterTest < ActiveSupport::TestCase
|
||||
|
||||
should "create modactions for each removal" do
|
||||
assert_difference(-> { ModAction.count }, 2) do
|
||||
@importer.process!
|
||||
@processor.process!(CurrentUser.user)
|
||||
end
|
||||
end
|
||||
|
||||
should "only remove active aliases and implications" do
|
||||
@ta2 = FactoryBot.create(:tag_alias, antecedent_name: "a", consequent_name: "b", status: "pending")
|
||||
@ti2 = FactoryBot.create(:tag_implication, antecedent_name: "c", consequent_name: "d", status: "pending")
|
||||
@ta2 = create(:tag_alias, antecedent_name: "a", consequent_name: "b", status: "pending")
|
||||
@ti2 = create(:tag_implication, antecedent_name: "c", consequent_name: "d", status: "pending")
|
||||
|
||||
@importer.process!
|
||||
@processor.process!(CurrentUser.user)
|
||||
assert_equal("pending", @ta2.reload.status)
|
||||
assert_equal("pending", @ti2.reload.status)
|
||||
end
|
||||
@@ -150,7 +150,7 @@ class BulkUpdateRequestTest < ActiveSupport::TestCase
|
||||
end
|
||||
|
||||
should "gracefully handle validation errors during approval" do
|
||||
@req.stubs(:update!).raises(AliasAndImplicationImporter::Error.new("blah"))
|
||||
@req.stubs(:update!).raises(BulkUpdateRequestProcessor::Error.new("blah"))
|
||||
assert_difference("ForumPost.count", 1) do
|
||||
@req.approve!(@admin)
|
||||
end
|
||||
|
||||
Reference in New Issue
Block a user