diff --git a/app/logical/tag_mover.rb b/app/logical/tag_mover.rb index 8215e8baa..6c9c3a456 100644 --- a/app/logical/tag_mover.rb +++ b/app/logical/tag_mover.rb @@ -9,6 +9,8 @@ class TagMover def move! CurrentUser.scoped(user) do + move_aliases! + move_implications! move_cosplay_tag! move_tag_category! move_artist! @@ -57,6 +59,22 @@ class TagMover end end + def move_aliases! + old_tag.consequent_aliases.each do |tag_alias| + tag_alias.update!(consequent_name: new_tag.name) + end + end + + def move_implications! + old_tag.antecedent_implications.each do |tag_implication| + tag_implication.update!(antecedent_name: new_tag.name) + end + + old_tag.consequent_implications.each do |tag_implication| + tag_implication.update!(consequent_name: new_tag.name) + end + end + def move_cosplay_tag! old_cosplay_tag = "#{old_tag.name}_(cosplay)" new_cosplay_tag = "#{new_tag.name}_(cosplay)" diff --git a/app/models/tag_alias.rb b/app/models/tag_alias.rb index 6e7303d5d..2f0dbca6a 100644 --- a/app/models/tag_alias.rb +++ b/app/models/tag_alias.rb @@ -16,7 +16,6 @@ class TagAlias < TagRelationship def process!(approver) update!(approver: approver, status: "processing") - move_aliases_and_implications TagMover.new(antecedent_name, consequent_name, user: User.system).move! update!(status: "active") rescue Exception => e @@ -34,35 +33,6 @@ class TagAlias < TagRelationship end end - def move_aliases_and_implications - aliases = TagAlias.where(["consequent_name = ?", antecedent_name]) - aliases.each do |ta| - ta.consequent_name = self.consequent_name - success = ta.save - if !success && ta.errors.full_messages.join("; ") =~ /Cannot alias a tag to itself/ - ta.destroy - end - end - - implications = TagImplication.where(["antecedent_name = ?", antecedent_name]) - implications.each do |ti| - ti.antecedent_name = self.consequent_name - success = ti.save - if !success && ti.errors.full_messages.join("; ") =~ /Cannot implicate a tag to itself/ - ti.destroy - end - end - - implications = TagImplication.where(["consequent_name = ?", antecedent_name]) - implications.each do |ti| - ti.consequent_name = self.consequent_name - success = ti.save - if !success && ti.errors.full_messages.join("; ") =~ /Cannot implicate a tag to itself/ - ti.destroy - end - end - end - def create_mod_action alias_desc = %("tag alias ##{id}":[#{Rails.application.routes.url_helpers.tag_alias_path(self)}]: [[#{antecedent_name}]] -> [[#{consequent_name}]]) diff --git a/test/unit/bulk_update_request_test.rb b/test/unit/bulk_update_request_test.rb index ce7e05f8a..3500daca7 100644 --- a/test/unit/bulk_update_request_test.rb +++ b/test/unit/bulk_update_request_test.rb @@ -58,6 +58,38 @@ class BulkUpdateRequestTest < ActiveSupport::TestCase assert_equal("bar", @wiki.reload.title) end + should "move any active aliases from the old tag to the new tag" do + @bur1 = create(:bulk_update_request, script: "alias aaa -> bbb") + @bur2 = create(:bulk_update_request, script: "alias bbb -> ccc") + + @bur1.approve!(@admin) + @bur2.approve!(@admin) + perform_enqueued_jobs + + assert_equal(false, TagAlias.where(antecedent_name: "aaa", consequent_name: "bbb", status: "active").exists?) + assert_equal(true, TagAlias.where(antecedent_name: "bbb", consequent_name: "ccc", status: "active").exists?) + assert_equal(true, TagAlias.where(antecedent_name: "aaa", consequent_name: "ccc", status: "active").exists?) + end + + should "move any active implications from the old tag to the new tag" do + @bur1 = create(:bulk_update_request, script: "imply aaa -> bbb") + @bur2 = create(:bulk_update_request, script: "alias bbb -> ccc") + + @bur1.approve!(@admin) + @bur2.approve!(@admin) + perform_enqueued_jobs + + assert_equal(false, TagImplication.where(antecedent_name: "aaa", consequent_name: "bbb", status: "active").exists?) + assert_equal(true, TagImplication.where(antecedent_name: "aaa", consequent_name: "ccc", status: "active").exists?) + + @bur3 = create(:bulk_update_request, script: "alias aaa -> ddd") + @bur3.approve!(@admin) + perform_enqueued_jobs + + assert_equal(false, TagImplication.where(antecedent_name: "aaa", consequent_name: "ccc", status: "active").exists?) + assert_equal(true, TagImplication.where(antecedent_name: "ddd", consequent_name: "ccc", status: "active").exists?) + 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") diff --git a/test/unit/tag_alias_test.rb b/test/unit/tag_alias_test.rb index 9b36d76c9..a8fe21d20 100644 --- a/test/unit/tag_alias_test.rb +++ b/test/unit/tag_alias_test.rb @@ -263,28 +263,6 @@ class TagAliasTest < ActiveSupport::TestCase end end - - should "move existing aliases" do - ta1 = FactoryBot.create(:tag_alias, :antecedent_name => "aaa", :consequent_name => "bbb", :status => "pending") - ta2 = FactoryBot.create(:tag_alias, :antecedent_name => "bbb", :consequent_name => "ccc", :status => "pending") - - # XXX this is broken, it depends on the order the jobs are executed in. - ta2.approve!(@admin) - ta1.approve!(@admin) - perform_enqueued_jobs - - assert_equal("ccc", ta1.reload.consequent_name) - end - - should "move existing implications" do - ti = FactoryBot.create(:tag_implication, :antecedent_name => "aaa", :consequent_name => "bbb") - ta = FactoryBot.create(:tag_alias, :antecedent_name => "bbb", :consequent_name => "ccc") - ta.approve!(@admin) - perform_enqueued_jobs - - assert_equal("ccc", ti.reload.consequent_name) - end - should "push the consequent's category to the antecedent if the antecedent is general" do tag1 = create(:tag, name: "general", category: 0) tag2 = create(:tag, name: "artist", category: 1)