From c32f9fbdb8ab8cef1c736935c8a6ee6bb05fd8e3 Mon Sep 17 00:00:00 2001
From: evazion <noizave@gmail.com>
Date: Tue, 7 Jan 2020 20:24:17 -0600
Subject: [PATCH] approver prune: don't prune recently promoted approvers.

Don't prune people who were just promoted in the last 45 days. Also fix
bug where approvers with zero approvals weren't pruned.
---
 app/logical/approver_pruner.rb    | 12 +++++++++---
 test/unit/approver_pruner_test.rb | 29 +++++++++++++++++++++++++++++
 2 files changed, 38 insertions(+), 3 deletions(-)
 create mode 100644 test/unit/approver_pruner_test.rb

diff --git a/app/logical/approver_pruner.rb b/app/logical/approver_pruner.rb
index 061034248..03172f50f 100644
--- a/app/logical/approver_pruner.rb
+++ b/app/logical/approver_pruner.rb
@@ -5,9 +5,15 @@ module ApproverPruner
   MINIMUM_APPROVALS = 30
 
   def inactive_approvers
-    approvals = PostApproval.where("created_at >= ?", APPROVAL_PERIOD.ago)
-    approvers = User.where("bit_prefs & ? > 0", User.flag_value_for("can_approve_posts")).where("level < ?", User::Levels::MODERATOR)
-    approvers.where(id: approvals.group(:user_id).having("count(*) < ?", MINIMUM_APPROVALS).select(:user_id))
+    approvers = User.where("bit_prefs & ? > 0", User.flag_value_for("can_approve_posts"))
+    approvers = approvers.where("level < ?", User::Levels::MODERATOR)
+
+    recently_promoted_approvers = UserFeedback.where("created_at >= ?", APPROVAL_PERIOD.ago).where_like(:body, "*You gained the ability to approve posts*").select(:user_id)
+    approvers = approvers.where.not(id: recently_promoted_approvers)
+
+    approvers.select do |approver|
+      approver.post_approvals.where("created_at >= ?", APPROVAL_PERIOD.ago).count < MINIMUM_APPROVALS
+    end
   end
 
   def prune!
diff --git a/test/unit/approver_pruner_test.rb b/test/unit/approver_pruner_test.rb
new file mode 100644
index 000000000..1289678a2
--- /dev/null
+++ b/test/unit/approver_pruner_test.rb
@@ -0,0 +1,29 @@
+require 'test_helper'
+
+class ApproverPrunerTest < ActiveSupport::TestCase
+  context "ApproverPruner" do
+    setup do
+      @approver = create(:user, can_approve_posts: true)
+    end
+
+    should "demote inactive approvers" do
+      assert_equal([@approver.id], ApproverPruner.inactive_approvers.map(&:id))
+    end
+
+    should "not demote active approvers" do
+      posts = create_list(:post, ApproverPruner::MINIMUM_APPROVALS + 1, is_pending: true)
+      posts.each { |post| post.approve!(@approver) }
+
+      assert_equal([], ApproverPruner.inactive_approvers.map(&:id))
+    end
+
+    should "not demote recently promoted approvers" do
+      as(create(:admin_user)) do
+        @user = create(:user)
+        @user.promote_to!(User::Levels::BUILDER, can_approve_posts: true)
+      end
+
+      assert_not_includes(ApproverPruner.inactive_approvers.map(&:id), @user.id)
+    end
+  end
+end