From 35d26e92e9fa1553365489df35d035f406640475 Mon Sep 17 00:00:00 2001 From: evazion Date: Thu, 3 Dec 2020 15:31:27 -0600 Subject: [PATCH] BURs: don't update OP when approving BUR. When approving or rejecting a BUR, don't edit the OP forum post to add an EDIT: line stating the request has been approved. Instead just let the embedded BUR state who it was approved by, and post a reply saying that the request has been approved. --- app/logical/d_text.rb | 2 +- app/logical/forum_updater.rb | 21 +++------------ app/models/bulk_update_request.rb | 9 +------ test/unit/bulk_update_request_pruner_test.rb | 2 +- test/unit/bulk_update_request_test.rb | 27 -------------------- 5 files changed, 6 insertions(+), 55 deletions(-) diff --git a/app/logical/d_text.rb b/app/logical/d_text.rb index 015539d19..d0964a129 100644 --- a/app/logical/d_text.rb +++ b/app/logical/d_text.rb @@ -109,7 +109,7 @@ class DText end if obj.is_approved? - "The \"bulk update request ##{obj.id}\":/bulk_update_requests/#{obj.id} is active.\n\n#{embedded_script}" + "The \"bulk update request ##{obj.id}\":/bulk_update_requests/#{obj.id} has been approved by <@#{obj.approver.pretty_name}>.\n\n#{embedded_script}" elsif obj.is_pending? "The \"bulk update request ##{obj.id}\":/bulk_update_requests/#{obj.id} is pending approval.\n\n#{embedded_script}" elsif obj.is_rejected? diff --git a/app/logical/forum_updater.rb b/app/logical/forum_updater.rb index 6ea465183..423b3a62b 100644 --- a/app/logical/forum_updater.rb +++ b/app/logical/forum_updater.rb @@ -1,28 +1,13 @@ class ForumUpdater - attr_reader :forum_topic, :forum_post + attr_reader :forum_topic - def initialize(forum_topic, options = {}) + def initialize(forum_topic) @forum_topic = forum_topic - @forum_post = options[:forum_post] end def update(message) - return if forum_topic.nil? - CurrentUser.scoped(User.system) do - create_response(message) - - if forum_post - update_post(message) - end + forum_topic.forum_posts.create(body: message, skip_mention_notifications: true, creator: User.system) end end - - def create_response(body) - forum_topic.forum_posts.create(body: body, skip_mention_notifications: true, creator: User.system) - end - - def update_post(body) - forum_post.update(body: "#{forum_post.body}\n\nEDIT: #{body}", skip_mention_notifications: true) - end end diff --git a/app/models/bulk_update_request.rb b/app/models/bulk_update_request.rb index 3b2e9a0df..a1d2d5c31 100644 --- a/app/models/bulk_update_request.rb +++ b/app/models/bulk_update_request.rb @@ -60,14 +60,7 @@ class BulkUpdateRequest < ApplicationRecord module ApprovalMethods def forum_updater - @forum_updater ||= begin - post = if forum_topic - forum_post || forum_topic.forum_posts.first - else - nil - end - ForumUpdater.new(forum_topic, forum_post: post) - end + @forum_updater ||= ForumUpdater.new(forum_topic) end def approve!(approver) diff --git a/test/unit/bulk_update_request_pruner_test.rb b/test/unit/bulk_update_request_pruner_test.rb index d6f222682..7af809345 100644 --- a/test/unit/bulk_update_request_pruner_test.rb +++ b/test/unit/bulk_update_request_pruner_test.rb @@ -19,7 +19,7 @@ class BulkUpdateRequestPrunerTest < ActiveSupport::TestCase BulkUpdateRequestPruner.reject_expired assert_equal("rejected", bur.reload.status) - assert_match(/rejected because it was not approved within 60 days/, bur.forum_post.body) + assert_match(/rejected because it was not approved within 60 days/, ForumPost.second.body) end end end diff --git a/test/unit/bulk_update_request_test.rb b/test/unit/bulk_update_request_test.rb index 7e35ca6bf..4343cbb70 100644 --- a/test/unit/bulk_update_request_test.rb +++ b/test/unit/bulk_update_request_test.rb @@ -422,10 +422,6 @@ class BulkUpdateRequestTest < ActiveSupport::TestCase @ti = TagImplication.where(:antecedent_name => "bar", :consequent_name => "baz").first end - should "reference the approver in the automated message" do - assert_match(Regexp.compile(@admin.name), @bur.forum_post.body) - end - should "set the BUR approver" do assert_equal(@admin.id, @bur.approver.id) end @@ -481,29 +477,6 @@ class BulkUpdateRequestTest < ActiveSupport::TestCase assert_equal("pending", @req.status) end - should "update the topic when processed" do - assert_difference("ForumPost.count") do - @req.approve!(@admin) - end - - assert_match(/approved/, @post.reload.body) - end - - should "update the topic when rejected" do - @req.approver_id = @admin.id - - assert_difference("ForumPost.count") do - @req.reject!(@admin) - end - - assert_match(/rejected/, @post.reload.body) - end - - should "reference the rejector in the automated message" do - @req.reject!(@admin) - assert_match(Regexp.compile(@admin.name), @req.forum_post.body) - end - should "not send @mention dmails to the approver" do assert_no_difference("Dmail.count") do @req.approve!(@admin)