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