From 23944a17946c2fbc10c1ca9fb5a330c0a0f3253d Mon Sep 17 00:00:00 2001 From: evazion Date: Wed, 26 Aug 2020 19:25:14 -0500 Subject: [PATCH] Fix #4491: Have tag rename option for bulk update requests. * Add a `rename A -> B` command for bulk update requests. * Change mass updates to only retag the posts, not to move saved searches or blacklists. A tag rename does the same thing an alias does, except it doesn't create a permanent alias. More precisely, a tag rename: * Moves the wiki. * Moves the artist entry. * Moves saved searches. * Moves blacklists. * Merges the wikis, if both tags have wiki pages. * Merges the artist entries, if both tags have artist pages. * Fixes links in wiki pages to point to the new tag. * Retags the posts. --- app/helpers/bulk_update_requests_helper.rb | 11 ++-- app/jobs/tag_batch_change_job.rb | 54 +------------------- app/jobs/tag_rename_job.rb | 7 +++ app/logical/bulk_update_request_processor.rb | 19 +++++-- test/jobs/tag_batch_change_job_test.rb | 33 +----------- test/unit/bulk_update_request_test.rb | 29 +++++++++++ 6 files changed, 61 insertions(+), 92 deletions(-) create mode 100644 app/jobs/tag_rename_job.rb diff --git a/app/helpers/bulk_update_requests_helper.rb b/app/helpers/bulk_update_requests_helper.rb index ed90ad6e3..1b007db9d 100644 --- a/app/helpers/bulk_update_requests_helper.rb +++ b/app/helpers/bulk_update_requests_helper.rb @@ -1,13 +1,14 @@ module BulkUpdateRequestsHelper def bur_script_example <<~EOS - create alias kitty -> cat - remove alias kitty -> cat + create alias bunny -> rabbit + remove alias bunny -> rabbit - create implication cat -> animal - remove implication cat -> animal + create implication bunny -> animal + remove implication bunny -> animal + + rename bunny -> rabbit - mass update kitty -> cat category touhou -> copyright EOS end diff --git a/app/jobs/tag_batch_change_job.rb b/app/jobs/tag_batch_change_job.rb index 8667a37a1..934d1bdc5 100644 --- a/app/jobs/tag_batch_change_job.rb +++ b/app/jobs/tag_batch_change_job.rb @@ -1,19 +1,12 @@ class TagBatchChangeJob < ApplicationJob - class Error < StandardError; end - queue_as :bulk_update - def perform(antecedent, consequent, updater, updater_ip_addr) - raise Error.new("antecedent is missing") if antecedent.blank? - + def perform(antecedent, consequent) normalized_antecedent = PostQueryBuilder.new(antecedent).split_query normalized_consequent = PostQueryBuilder.new(consequent).parse_tag_edit - CurrentUser.scoped(updater, updater_ip_addr) do + CurrentUser.scoped(User.system) do migrate_posts(normalized_antecedent, normalized_consequent) - migrate_saved_searches(normalized_antecedent, normalized_consequent) - migrate_blacklists(normalized_antecedent, normalized_consequent) - ModAction.log("processed mass update: #{antecedent} -> #{consequent}", :mass_update) end end @@ -26,47 +19,4 @@ class TagBatchChangeJob < ApplicationJob end end end - - def migrate_saved_searches(normalized_antecedent, normalized_consequent) - tags = PostQueryBuilder.new(normalized_antecedent.join(" ")).split_query - - # https://www.postgresql.org/docs/current/static/functions-array.html - saved_searches = SavedSearch.where("string_to_array(query, ' ') @> ARRAY[?]", tags) - saved_searches.find_each do |ss| - ss.query = (ss.query.split - tags + normalized_consequent).uniq.join(" ") - ss.save - end - end - - # this can't handle negated tags or other special cases - def migrate_blacklists(normalized_antecedent, normalized_consequent) - query = normalized_antecedent - adds = normalized_consequent - arel = query.inject(User.none) do |scope, x| - scope.or(User.where_like(:blacklisted_tags, "*#{x}*")) - end - - arel.find_each do |user| - changed = false - - begin - repl = user.blacklisted_tags.split(/\r\n|\r|\n/).map do |line| - list = PostQueryBuilder.new(line).split_query - - if (list & query).size != query.size - next line - end - - changed = true - (list - query + adds).join(" ") - end - - if changed - user.update(blacklisted_tags: repl.join("\n")) - end - rescue Exception => e - DanbooruLogger.log(e) - end - end - end end diff --git a/app/jobs/tag_rename_job.rb b/app/jobs/tag_rename_job.rb new file mode 100644 index 000000000..2a8fd34ac --- /dev/null +++ b/app/jobs/tag_rename_job.rb @@ -0,0 +1,7 @@ +class TagRenameJob < ApplicationJob + queue_as :bulk_update + + def perform(old_tag_name, new_tag_name) + TagMover.new(old_tag_name, new_tag_name, user: User.system).move! + end +end diff --git a/app/logical/bulk_update_request_processor.rb b/app/logical/bulk_update_request_processor.rb index a64ba7d02..ecbd5997c 100644 --- a/app/logical/bulk_update_request_processor.rb +++ b/app/logical/bulk_update_request_processor.rb @@ -26,6 +26,8 @@ class BulkUpdateRequestProcessor [:remove_alias, Tag.normalize_name($1), Tag.normalize_name($2)] when /\A(?:remove implication|unimply) (\S+) -> (\S+)\z/i [:remove_implication, Tag.normalize_name($1), Tag.normalize_name($2)] + when /\Arename (\S+) -> (\S+)\z/i + [:rename, Tag.normalize_name($1), Tag.normalize_name($2)] when /\A(?:mass update|update) (.+?) -> (.*)\z/i [:mass_update, $1, $2] when /\Acategory (\S+) -> (#{Tag.categories.regexp})\z/i @@ -69,6 +71,12 @@ class BulkUpdateRequestProcessor errors[:base] << "Can't change category #{args[0]} -> #{args[1]} (the '#{args[0]}' tag doesn't exist)" end + when :rename + tag = Tag.find_by_name(args[0]) + if tag.nil? + errors[:base] << "Can't rename #{args[0]} -> #{args[1]} (the '#{args[0]}' tag doesn't exist)" + end + when :mass_update # okay @@ -105,7 +113,10 @@ class BulkUpdateRequestProcessor tag_implication.reject! when :mass_update - TagBatchChangeJob.perform_later(args[0], args[1], User.system, "127.0.0.1") + TagBatchChangeJob.perform_later(args[0], args[1]) + + when :rename + TagRenameJob.perform_later(args[0], args[1]) when :change_category tag = Tag.find_or_create_by_name(args[0]) @@ -122,7 +133,7 @@ class BulkUpdateRequestProcessor def affected_tags commands.flat_map do |command, *args| case command - when :create_alias, :remove_alias, :create_implication, :remove_implication + when :create_alias, :remove_alias, :create_implication, :remove_implication, :rename [args[0], args[1]] when :mass_update tags = PostQueryBuilder.new(args[0]).tags + PostQueryBuilder.new(args[1]).tags @@ -136,7 +147,7 @@ class BulkUpdateRequestProcessor def is_tag_move_allowed? commands.all? do |command, *args| case command - when :create_alias + when :create_alias, :rename BulkUpdateRequestProcessor.is_tag_move_allowed?(args[0], args[1]) when :mass_update lhs = PostQueryBuilder.new(args[0]) @@ -152,7 +163,7 @@ class BulkUpdateRequestProcessor def to_dtext commands.map do |command, *args| case command - when :create_alias, :create_implication, :remove_alias, :remove_implication + when :create_alias, :create_implication, :remove_alias, :remove_implication, :rename "#{command.to_s.tr("_", " ")} [[#{args[0]}]] -> [[#{args[1]}]]" when :mass_update "mass update {{#{args[0]}}} -> #{args[1]}" diff --git a/test/jobs/tag_batch_change_job_test.rb b/test/jobs/tag_batch_change_job_test.rb index 07b183556..40b3a6ced 100644 --- a/test/jobs/tag_batch_change_job_test.rb +++ b/test/jobs/tag_batch_change_job_test.rb @@ -8,42 +8,13 @@ class TagBatchChangeJobTest < ActiveJob::TestCase end should "execute" do - TagBatchChangeJob.perform_now("aaa", "bbb", @user, "127.0.0.1") + TagBatchChangeJob.perform_now("aaa", "bbb") assert_equal("bbb", @post.reload.tag_string) end - should "move saved searches" do - ss = create(:saved_search, user: @user, query: "123 ... 456") - TagBatchChangeJob.perform_now("...", "bbb", @user, "127.0.0.1") - assert_equal("123 456 bbb", ss.reload.normalized_query) - end - - should "move blacklists" do - @user.update(blacklisted_tags: "123 456\n789\n") - TagBatchChangeJob.perform_now("456", "xxx", @user, "127.0.0.1") - - assert_equal("123 xxx\n789", @user.reload.blacklisted_tags) - end - - should "move only saved searches that match the mass update exactly" do - ss = create(:saved_search, user: @user, query: "123 ... 456") - - TagBatchChangeJob.perform_now("1", "bbb", @user, "127.0.0.1") - assert_equal("... 123 456", ss.reload.normalized_query, "expected '123' to remain unchanged") - - TagBatchChangeJob.perform_now("123 456", "789", @user, "127.0.0.1") - assert_equal("... 789", ss.reload.normalized_query, "expected '123 456' to be changed to '789'") - end - should "log a modaction" do - TagBatchChangeJob.perform_now("1", "2", @user, "127.0.0.1") + TagBatchChangeJob.perform_now("1", "2") assert_equal("mass_update", ModAction.last.category) end - - should "raise an error if there is no predicate" do - assert_raises(TagBatchChangeJob::Error) do - TagBatchChangeJob.perform_now("", "bbb", @user, "127.0.0.1") - end - end end end diff --git a/test/unit/bulk_update_request_test.rb b/test/unit/bulk_update_request_test.rb index d4154e127..762f80444 100644 --- a/test/unit/bulk_update_request_test.rb +++ b/test/unit/bulk_update_request_test.rb @@ -182,6 +182,35 @@ class BulkUpdateRequestTest < ActiveSupport::TestCase assert_equal("Imageboard", @post.reload.source) end end + + context "the rename command" do + setup do + @artist = create(:artist, name: "foo") + @wiki = create(:wiki_page, title: "foo", body: "[[foo]]") + @post = create(:post, tag_string: "foo blah") + @bur = create(:bulk_update_request, script: "rename foo -> bar") + @bur.approve!(@admin) + perform_enqueued_jobs + end + + should "rename the tags" do + assert_equal("approved", @bur.status) + assert_equal("bar blah", @post.reload.tag_string) + end + + should "move the tag's artist entry and wiki page" do + assert_equal("bar", @artist.reload.name) + assert_equal("bar", @wiki.reload.title) + assert_equal("[[bar]]", @wiki.body) + end + + should "fail if the old tag doesn't exist" do + @bur = build(:bulk_update_request, script: "rename aaa -> bbb") + + assert_equal(false, @bur.valid?) + assert_equal(["Can't rename aaa -> bbb (the 'aaa' tag doesn't exist)"], @bur.errors.full_messages) + end + end end context "when validating a script" do