From 0e7632ed8a1269ec84cb751700d541581cfbaad8 Mon Sep 17 00:00:00 2001 From: evazion Date: Tue, 10 Mar 2020 20:48:08 -0500 Subject: [PATCH] aliases/implications: remove forum topic updating code. Remove code for updating forum topics when an alias or implication is approved or rejected. This code was only used when approving single alias or implication requests. This is no longer used now that all alias/implication requests are done through BURs. --- app/jobs/process_tag_alias_job.rb | 4 +- app/jobs/process_tag_implication_job.rb | 4 +- app/logical/alias_and_implication_importer.rb | 8 +-- app/logical/forum_updater.rb | 2 - app/logical/tag_alias_request.rb | 9 --- app/logical/tag_implication_request.rb | 9 --- app/models/bulk_update_request.rb | 7 +-- app/models/tag_alias.rb | 31 ++------- app/models/tag_implication.rb | 23 +------ app/models/tag_relationship.rb | 34 +--------- test/unit/tag_alias_test.rb | 63 +------------------ test/unit/tag_implication_test.rb | 28 --------- 12 files changed, 20 insertions(+), 202 deletions(-) delete mode 100644 app/logical/tag_alias_request.rb delete mode 100644 app/logical/tag_implication_request.rb diff --git a/app/jobs/process_tag_alias_job.rb b/app/jobs/process_tag_alias_job.rb index 4a6881565..9098dd14e 100644 --- a/app/jobs/process_tag_alias_job.rb +++ b/app/jobs/process_tag_alias_job.rb @@ -1,7 +1,7 @@ class ProcessTagAliasJob < ApplicationJob queue_as :bulk_update - def perform(tag_alias, update_topic: true) - tag_alias.process!(update_topic: update_topic) + def perform(tag_alias) + tag_alias.process! end end diff --git a/app/jobs/process_tag_implication_job.rb b/app/jobs/process_tag_implication_job.rb index 330f7acf3..41bff5c45 100644 --- a/app/jobs/process_tag_implication_job.rb +++ b/app/jobs/process_tag_implication_job.rb @@ -1,7 +1,7 @@ class ProcessTagImplicationJob < ApplicationJob queue_as :bulk_update - def perform(tag_implication, update_topic: true) - tag_implication.process!(update_topic: update_topic) + def perform(tag_implication) + tag_implication.process! end end diff --git a/app/logical/alias_and_implication_importer.rb b/app/logical/alias_and_implication_importer.rb index c55979399..479f410ab 100644 --- a/app/logical/alias_and_implication_importer.rb +++ b/app/logical/alias_and_implication_importer.rb @@ -114,24 +114,24 @@ class AliasAndImplicationImporter raise Error, "Error: #{tag_alias.errors.full_messages.join("; ")} (create alias #{tag_alias.antecedent_name} -> #{tag_alias.consequent_name})" end tag_alias.rename_wiki_and_artist if rename_aliased_pages? - tag_alias.approve!(approver: approver, update_topic: false) + 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) unless tag_implication.valid? raise Error, "Error: #{tag_implication.errors.full_messages.join("; ")} (create implication #{tag_implication.antecedent_name} -> #{tag_implication.consequent_name})" end - tag_implication.approve!(approver: approver, update_topic: false) + tag_implication.approve!(approver: approver) when :remove_alias tag_alias = TagAlias.active.find_by(antecedent_name: token[1], consequent_name: token[2]) raise Error, "Alias for #{token[1]} not found" if tag_alias.nil? - tag_alias.reject!(update_topic: false) + tag_alias.reject! when :remove_implication tag_implication = TagImplication.active.find_by(antecedent_name: token[1], consequent_name: token[2]) raise Error, "Implication for #{token[1]} not found" if tag_implication.nil? - tag_implication.reject!(update_topic: false) + tag_implication.reject! when :mass_update TagBatchChangeJob.perform_later(token[1], token[2], User.system, "127.0.0.1") diff --git a/app/logical/forum_updater.rb b/app/logical/forum_updater.rb index 0f274dec5..eeea0652d 100644 --- a/app/logical/forum_updater.rb +++ b/app/logical/forum_updater.rb @@ -5,7 +5,6 @@ class ForumUpdater @forum_topic = forum_topic @forum_post = options[:forum_post] @expected_title = options[:expected_title] - @skip_update = options[:skip_update] end def update(message, title_tag = nil) @@ -32,7 +31,6 @@ class ForumUpdater end def update_post(body) - return if @skip_update forum_post.update(body: "#{forum_post.body}\n\nEDIT: #{body}", skip_mention_notifications: true) end end diff --git a/app/logical/tag_alias_request.rb b/app/logical/tag_alias_request.rb deleted file mode 100644 index f85393785..000000000 --- a/app/logical/tag_alias_request.rb +++ /dev/null @@ -1,9 +0,0 @@ -class TagAliasRequest - def self.command_string(antecedent_name, consequent_name, id = nil) - if id - return "[ta:#{id}]" - end - - "create alias [[#{antecedent_name}]] -> [[#{consequent_name}]]" - end -end diff --git a/app/logical/tag_implication_request.rb b/app/logical/tag_implication_request.rb deleted file mode 100644 index bfa91c682..000000000 --- a/app/logical/tag_implication_request.rb +++ /dev/null @@ -1,9 +0,0 @@ -class TagImplicationRequest - def self.command_string(antecedent_name, consequent_name, id = nil) - if id - return "[ti:#{id}]" - end - - "create implication [[#{antecedent_name}]] -> [[#{consequent_name}]]" - end -end diff --git a/app/models/bulk_update_request.rb b/app/models/bulk_update_request.rb index 24505ab34..ed8f30e0e 100644 --- a/app/models/bulk_update_request.rb +++ b/app/models/bulk_update_request.rb @@ -67,8 +67,7 @@ class BulkUpdateRequest < ApplicationRecord ForumUpdater.new( forum_topic, forum_post: post, - expected_title: title, - skip_update: !TagRelationship::SUPPORT_HARD_CODED + expected_title: title ) end end @@ -88,10 +87,6 @@ class BulkUpdateRequest < ApplicationRecord end end - def date_timestamp - Time.now.strftime("%Y-%m-%d") - end - def create_forum_topic CurrentUser.as(user) do self.forum_topic = ForumTopic.create(title: title, category_id: 1, creator: user) unless forum_topic.present? diff --git a/app/models/tag_alias.rb b/app/models/tag_alias.rb index 58e6ab5db..0d0fa9b4f 100644 --- a/app/models/tag_alias.rb +++ b/app/models/tag_alias.rb @@ -5,32 +5,13 @@ class TagAlias < TagRelationship validate :wiki_pages_present, on: :create, unless: :skip_secondary_validations module ApprovalMethods - def approve!(approver: CurrentUser.user, update_topic: true) + def approve!(approver: CurrentUser.user) update(approver: approver, status: "queued") - ProcessTagAliasJob.perform_later(self, update_topic: update_topic) - end - end - - module ForumMethods - def forum_updater - @forum_updater ||= begin - post = if forum_topic - forum_post || forum_topic.posts.where("body like ?", TagAliasRequest.command_string(antecedent_name, consequent_name, id) + "%").last - else - nil - end - ForumUpdater.new( - forum_topic, - forum_post: post, - expected_title: "Tag alias: #{antecedent_name} -> #{consequent_name}", - skip_update: !TagRelationship::SUPPORT_HARD_CODED - ) - end + ProcessTagAliasJob.perform_later(self) end end include ApprovalMethods - include ForumMethods def self.to_aliased(names) names = Array(names).map(&:to_s) @@ -39,7 +20,7 @@ class TagAlias < TagRelationship names.map { |name| aliases[name] || name } end - def process!(update_topic: true) + def process! unless valid? raise errors.full_messages.join("; ") end @@ -50,13 +31,11 @@ class TagAlias < TagRelationship move_saved_searches ensure_category_consistency update_posts - forum_updater.update(approval_message(approver), "APPROVED") if update_topic rename_wiki_and_artist update!(status: "active") end rescue Exception => e CurrentUser.scoped(approver) do - forum_updater.update(failure_message(e), "FAILED") if update_topic update(status: "error: #{e}") end @@ -122,8 +101,6 @@ class TagAlias < TagRelationship if antecedent_wiki.present? if WikiPage.titled(consequent_name).blank? antecedent_wiki.update!(title: consequent_name) - else - forum_updater.update(conflict_message) end end @@ -136,7 +113,7 @@ class TagAlias < TagRelationship def wiki_pages_present if antecedent_wiki.present? && consequent_wiki.present? - errors[:base] << conflict_message + errors[:base] << "The tag alias [[#{antecedent_name}]] -> [[#{consequent_name}]] has conflicting wiki pages. [[#{consequent_name}]] should be updated to include information from [[#{antecedent_name}]] if necessary." elsif antecedent_wiki.blank? && consequent_wiki.blank? errors[:base] << "The #{consequent_name} tag needs a corresponding wiki page" end diff --git a/app/models/tag_implication.rb b/app/models/tag_implication.rb index 9317dc5dd..e4c0321b6 100644 --- a/app/models/tag_implication.rb +++ b/app/models/tag_implication.rb @@ -120,7 +120,7 @@ class TagImplication < TagRelationship end module ApprovalMethods - def process!(update_topic: true) + def process! unless valid? raise errors.full_messages.join("; ") end @@ -129,18 +129,15 @@ class TagImplication < TagRelationship update(status: "processing") update_posts update(status: "active") - forum_updater.update(approval_message(approver), "APPROVED") if update_topic end rescue Exception => e - forum_updater.update(failure_message(e), "FAILED") if update_topic update(status: "error: #{e}") - DanbooruLogger.log(e, tag_implication_id: id, antecedent_name: antecedent_name, consequent_name: consequent_name) end - def approve!(approver: CurrentUser.user, update_topic: true) + def approve!(approver: CurrentUser.user) update(approver: approver, status: "queued") - ProcessTagImplicationJob.perform_later(self, update_topic: update_topic) + ProcessTagImplicationJob.perform_later(self) end def create_mod_action @@ -162,20 +159,6 @@ class TagImplication < TagRelationship ModAction.log("updated #{implication}\n#{change_desc}", :tag_implication_update) end end - - def forum_updater - post = if forum_topic - forum_post || forum_topic.posts.where("body like ?", TagImplicationRequest.command_string(antecedent_name, consequent_name, id) + "%").last - else - nil - end - ForumUpdater.new( - forum_topic, - forum_post: post, - expected_title: "Tag implication: #{antecedent_name} -> #{consequent_name}", - skip_update: !TagRelationship::SUPPORT_HARD_CODED - ) - end end include DescendantMethods diff --git a/app/models/tag_relationship.rb b/app/models/tag_relationship.rb index b339683d9..ee14ecce6 100644 --- a/app/models/tag_relationship.rb +++ b/app/models/tag_relationship.rb @@ -1,7 +1,6 @@ class TagRelationship < ApplicationRecord self.abstract_class = true - SUPPORT_HARD_CODED = true EXPIRY = 60 EXPIRY_WARNING = 55 @@ -69,11 +68,8 @@ class TagRelationship < ApplicationRecord user.is_admin? end - def reject!(update_topic: true) - transaction do - update!(status: "deleted") - forum_updater.update(reject_message(CurrentUser.user), "REJECTED") if update_topic - end + def reject! + update!(status: "deleted") end module SearchMethods @@ -145,32 +141,8 @@ class TagRelationship < ApplicationRecord self.class.name.underscore.tr("_", " ") end - def approval_message(approver) - "The #{relationship} [[#{antecedent_name}]] -> [[#{consequent_name}]] #{forum_link} has been approved by @#{approver.name}." - end - - def failure_message(e = nil) - "The #{relationship} [[#{antecedent_name}]] -> [[#{consequent_name}]] #{forum_link} failed during processing. Reason: #{e}" - end - - def reject_message(rejector) - "The #{relationship} [[#{antecedent_name}]] -> [[#{consequent_name}]] #{forum_link} has been rejected by @#{rejector.name}." - end - def retirement_message - "The #{relationship} [[#{antecedent_name}]] -> [[#{consequent_name}]] #{forum_link} has been retired." - end - - def conflict_message - "The tag alias [[#{antecedent_name}]] -> [[#{consequent_name}]] #{forum_link} has conflicting wiki pages. [[#{consequent_name}]] should be updated to include information from [[#{antecedent_name}]] if necessary." - end - - def date_timestamp - Time.now.strftime("%Y-%m-%d") - end - - def forum_link - "(forum ##{forum_post.id})" if forum_post.present? + "The #{relationship} [[#{antecedent_name}]] -> [[#{consequent_name}]] has been retired." end end diff --git a/test/unit/tag_alias_test.rb b/test/unit/tag_alias_test.rb index 7f56f7734..e0cad9af8 100644 --- a/test/unit/tag_alias_test.rb +++ b/test/unit/tag_alias_test.rb @@ -92,7 +92,7 @@ class TagAliasTest < ActiveSupport::TestCase ti = FactoryBot.build(:tag_alias, antecedent_name: "aaa", consequent_name: "bbb", skip_secondary_validations: false) assert(ti.invalid?) - assert_includes(ti.errors[:base], "The tag alias [[aaa]] -> [[bbb]] has conflicting wiki pages. [[bbb]] should be updated to include information from [[aaa]] if necessary.") + assert_includes(ti.errors[:base], "The tag alias [[aaa]] -> [[bbb]] has conflicting wiki pages. [[bbb]] should be updated to include information from [[aaa]] if necessary.") end end @@ -198,66 +198,5 @@ class TagAliasTest < ActiveSupport::TestCase assert_equal(1, tag2.reload.category) end - - context "with an associated forum topic" do - setup do - @admin = FactoryBot.create(:admin_user) - CurrentUser.scoped(@admin) do - @topic = FactoryBot.create(:forum_topic, :title => "Tag alias: aaa -> bbb") - @post = FactoryBot.create(:forum_post, :topic_id => @topic.id, :body => TagAliasRequest.command_string("aaa", "bbb")) - @alias = FactoryBot.create(:tag_alias, :antecedent_name => "aaa", :consequent_name => "bbb", :forum_topic => @topic, :forum_post => @post, :status => "pending") - end - end - - context "and conflicting wiki pages" do - setup do - CurrentUser.scoped(@admin) do - @wiki1 = FactoryBot.create(:wiki_page, :title => "aaa") - @wiki2 = FactoryBot.create(:wiki_page, :title => "bbb") - @alias.approve!(approver: @admin) - perform_enqueued_jobs - end - end - - should "update the forum topic when approved" do - assert_equal("[APPROVED] Tag alias: aaa -> bbb", @topic.reload.title) - assert_match(/The tag alias .* been approved/m, @topic.posts.second.body) - end - - should "warn about conflicting wiki pages when approved" do - assert_match(/has conflicting wiki pages/m, @topic.posts.third.body) - end - end - - should "update the topic when processed" do - assert_difference("ForumPost.count") do - @alias.approve!(approver: @admin) - perform_enqueued_jobs - end - end - - should "update the parent post" do - previous = @post.body - @alias.approve!(approver: @admin) - perform_enqueued_jobs - assert_not_equal(previous, @post.reload.body) - end - - should "update the topic when rejected" do - assert_difference("ForumPost.count") do - @alias.reject! - end - end - - should "update the topic when failed" do - @alias.stubs(:sleep).returns(true) - @alias.stubs(:update_posts).raises(Exception, "oh no") - @alias.process! - - assert_equal("[FAILED] Tag alias: aaa -> bbb", @topic.reload.title) - assert_match(/error: oh no/, @alias.status) - assert_match(/The tag alias .* failed during processing/, @topic.posts.last.body) - end - end end end diff --git a/test/unit/tag_implication_test.rb b/test/unit/tag_implication_test.rb index 6e907ef08..d6d01693f 100644 --- a/test/unit/tag_implication_test.rb +++ b/test/unit/tag_implication_test.rb @@ -181,33 +181,5 @@ class TagImplicationTest < ActiveSupport::TestCase assert_equal([], TagImplication.tags_implied_by("d").map(&:name)) end end - - context "with an associated forum topic" do - setup do - @topic = FactoryBot.create(:forum_topic, :title => "Tag implication: aaa -> bbb") - @post = FactoryBot.create(:forum_post, topic_id: @topic.id, :body => TagImplicationRequest.command_string("aaa", "bbb")) - @implication = FactoryBot.create(:tag_implication, :antecedent_name => "aaa", :consequent_name => "bbb", :forum_topic => @topic, :forum_post => @post, :status => "pending") - end - - should "update the topic when processed" do - assert_difference("ForumPost.count") do - @implication.approve! - perform_enqueued_jobs - end - - assert_match(/The tag implication .* has been approved/, @post.reload.body) - assert_equal("[APPROVED] Tag implication: aaa -> bbb", @topic.reload.title) - end - - should "update the topic when rejected" do - assert_difference("ForumPost.count") do - @implication.reject! - end - @post.reload - @topic.reload - assert_match(/The tag implication .* has been rejected/, @post.body) - assert_equal("[REJECTED] Tag implication: aaa -> bbb", @topic.title) - end - end end end