From b4dc7487eee10334cc76f9ad5b57c975638c259f Mon Sep 17 00:00:00 2001 From: evazion Date: Sun, 15 Aug 2021 04:51:48 -0500 Subject: [PATCH] BURs: reduce autorejection timeout from 60 days to 45 days. --- app/logical/bulk_update_request_pruner.rb | 26 +++++++++++++++----- app/models/bulk_update_request.rb | 2 -- app/models/tag_relationship.rb | 5 ---- test/unit/bulk_update_request_pruner_test.rb | 6 ++--- 4 files changed, 23 insertions(+), 16 deletions(-) diff --git a/app/logical/bulk_update_request_pruner.rb b/app/logical/bulk_update_request_pruner.rb index 88aa75937..113544785 100644 --- a/app/logical/bulk_update_request_pruner.rb +++ b/app/logical/bulk_update_request_pruner.rb @@ -1,12 +1,18 @@ -# Rejects bulk update requests that haven't been approved in 60 days. +# Rejects bulk update requests that haven't been approved in 45 days. module BulkUpdateRequestPruner module_function + # How many days before a bulk update request should be automatically rejected. + EXPIRATION_PERIOD = 45.days + + # How many days before we should warn about upcoming rejections. + WARNING_PERIOD = 5.days + # Posts a warning when a bulk update request is pending automatic rejection in 5 days. def warn_old - BulkUpdateRequest.old.pending.find_each do |bulk_update_request| + upcoming_expired_requests.find_each do |bulk_update_request| if bulk_update_request.forum_topic - body = "This bulk update request is pending automatic rejection in 5 days." + body = "This bulk update request is pending automatic rejection in #{WARNING_PERIOD.inspect}." unless bulk_update_request.forum_topic.forum_posts.where(creator_id: User.system.id, body: body).exists? bulk_update_request.forum_updater.update(body) end @@ -14,12 +20,12 @@ module BulkUpdateRequestPruner end end - # Rejects bulk update requests that haven't been approved in 60 days. + # Rejects bulk update requests that haven't been approved in 45 days. def reject_expired - BulkUpdateRequest.expired.pending.find_each do |bulk_update_request| + expired_requests.find_each do |bulk_update_request| ApplicationRecord.transaction do if bulk_update_request.forum_topic - body = "This bulk update request has been rejected because it was not approved within 60 days." + body = "This bulk update request has been rejected because it was not approved within #{EXPIRATION_PERIOD.inspect}." bulk_update_request.forum_updater.update(body) end @@ -27,4 +33,12 @@ module BulkUpdateRequestPruner end end end + + def expired_requests + BulkUpdateRequest.pending.where("created_at < ?", EXPIRATION_PERIOD.ago) + end + + def upcoming_expired_requests + BulkUpdateRequest.pending.where("created_at >= ? and created_at < ?", EXPIRATION_PERIOD.ago, (EXPIRATION_PERIOD - WARNING_PERIOD).ago) + end end diff --git a/app/models/bulk_update_request.rb b/app/models/bulk_update_request.rb index fc0a5f8e8..95562a465 100644 --- a/app/models/bulk_update_request.rb +++ b/app/models/bulk_update_request.rb @@ -21,8 +21,6 @@ class BulkUpdateRequest < ApplicationRecord scope :approved, -> { where(status: "approved") } scope :rejected, -> { where(status: "rejected") } scope :has_topic, -> { where.not(forum_topic: nil) } - scope :expired, -> {where("created_at < ?", TagRelationship::EXPIRY.days.ago)} - scope :old, -> {where("created_at between ? and ?", TagRelationship::EXPIRY.days.ago, TagRelationship::EXPIRY_WARNING.days.ago)} module SearchMethods def default_order diff --git a/app/models/tag_relationship.rb b/app/models/tag_relationship.rb index 1ffe0130d..316ad3ac8 100644 --- a/app/models/tag_relationship.rb +++ b/app/models/tag_relationship.rb @@ -1,9 +1,6 @@ class TagRelationship < ApplicationRecord self.abstract_class = true - EXPIRY = 60 - EXPIRY_WARNING = 55 - belongs_to :creator, class_name: "User" belongs_to :approver, class_name: "User", optional: true belongs_to :forum_post, optional: true @@ -15,8 +12,6 @@ class TagRelationship < ApplicationRecord scope :active, -> {where(status: "active")} scope :deleted, -> {where(status: "deleted")} - scope :expired, -> {where("created_at < ?", EXPIRY.days.ago)} - scope :old, -> {where("created_at >= ? and created_at < ?", EXPIRY.days.ago, EXPIRY_WARNING.days.ago)} scope :retired, -> {where(status: "retired")} before_validation :normalize_names diff --git a/test/unit/bulk_update_request_pruner_test.rb b/test/unit/bulk_update_request_pruner_test.rb index 7af809345..0914dfc63 100644 --- a/test/unit/bulk_update_request_pruner_test.rb +++ b/test/unit/bulk_update_request_pruner_test.rb @@ -4,7 +4,7 @@ class BulkUpdateRequestPrunerTest < ActiveSupport::TestCase context '#warn_old' do should "update the forum topic for a bulk update request" do forum_topic = as(create(:user)) { create(:forum_topic) } - bur = create(:bulk_update_request, status: "pending", forum_topic: forum_topic, created_at: (TagRelationship::EXPIRY_WARNING + 1).days.ago) + bur = create(:bulk_update_request, status: "pending", forum_topic: forum_topic, created_at: (BulkUpdateRequestPruner::EXPIRATION_PERIOD - 1.day).ago) BulkUpdateRequestPruner.warn_old assert_equal("pending", bur.reload.status) @@ -15,11 +15,11 @@ class BulkUpdateRequestPrunerTest < ActiveSupport::TestCase context '#reject_expired' do should "reject the bulk update request" do forum_topic = as(create(:user)) { create(:forum_topic) } - bur = create(:bulk_update_request, status: "pending", forum_topic: forum_topic, created_at: (TagRelationship::EXPIRY + 1).days.ago) + bur = create(:bulk_update_request, status: "pending", forum_topic: forum_topic, created_at: (BulkUpdateRequestPruner::EXPIRATION_PERIOD + 1.day).ago) BulkUpdateRequestPruner.reject_expired assert_equal("rejected", bur.reload.status) - assert_match(/rejected because it was not approved within 60 days/, ForumPost.second.body) + assert_match(/rejected because it was not approved within \d+ days/, ForumPost.second.body) end end end