Filter out reviewers who have rejected reviewing this document in the past from the list of suggested reviewers. Fixes ietf-tools#2996. Commit ready for merge.

meadmaker · meadmaker · commit d105f8ba5bda · 2020-08-13T15:03:54.000Z
- Legacy-Id: 18373
diff --git a/ietf/review/policies.py b/ietf/review/policies.py
@@ -3,6 +3,7 @@
 
 import re
 
+from django.db.models import Q
 from django.db.models.aggregates import Max
 
 from ietf.doc.models import DocumentAuthor, DocAlias
@@ -126,8 +127,26 @@ def setup_reviewer_field(self, field, review_req):
         The field should be an instance similar to
             PersonEmailChoiceField(label="Assign Reviewer", empty_label="(None)")
         """
-        field.queryset = field.queryset.filter(role__name="reviewer", role__group=review_req.team)
-        one_assignment = review_req.reviewassignment_set.first()
+
+        # Collect a set of person IDs for people who have either not responded
+        # to or outright rejected reviewing this document in the past
+        rejecting_reviewer_ids = review_req.doc.reviewrequest_set.filter(
+            reviewassignment__state__slug__in=('rejected', 'no-response')
+        ).values_list(
+            'reviewassignment__reviewer__person_id', flat=True
+        )
+
+        # Query the Email objects for reviewers who haven't rejected or
+        # not responded to this document in the past
+        field.queryset = field.queryset.filter(
+            role__name="reviewer",
+            role__group=review_req.team
+        ).filter(
+            ~Q( person_id__in=rejecting_reviewer_ids )
+        )
+        one_assignment = review_req.reviewassignment_set.filter(
+            ~Q(state__slug__in = ('rejected', 'no-response') )
+        ).first()
         if one_assignment:
             field.initial = one_assignment.reviewer_id
 
diff --git a/ietf/review/tests_policies.py b/ietf/review/tests_policies.py
@@ -91,11 +91,24 @@ def test_setup_reviewer_field(self):
         policy = RotateAlphabeticallyReviewerQueuePolicy(team)
         reviewer_0 = create_person(team, "reviewer", name="Test Reviewer-0", username="testreviewer0")
         reviewer_1 = create_person(team, "reviewer", name="Test Reviewer-1", username="testreviewer1")
+        reviewer_2 = create_person(team, "reviewer", name="Test Reviewer-2", username="testreviewer2")
+        reviewer_3 = create_person(team, "reviewer", name="Test Reviewer-3", username="testreviewer3")
         review_req = ReviewRequestFactory(team=team, type_id='early')
+        review_req_2 = ReviewRequestFactory(team=team, type_id='early', doc=review_req.doc)
+        review_req_3 = ReviewRequestFactory(team=team, type_id='early', doc=review_req.doc)
         ReviewAssignmentFactory(review_request=review_req, reviewer=reviewer_1.email(), state_id='part-completed')
+        ReviewAssignmentFactory(review_request=review_req_2, reviewer=reviewer_2.email(), state_id='rejected')
+        ReviewAssignmentFactory(review_request=review_req_3, reviewer=reviewer_3.email(), state_id='no-response')
         field = PersonEmailChoiceField(label="Assign Reviewer", empty_label="(None)", required=False)
         
         policy.setup_reviewer_field(field, review_req)
+        addresses = list( map( lambda choice: choice[0], field.choices ) )
+        self.assertNotIn(
+            str(reviewer_2.email()), addresses,
+            "Reviews should not suggest people who have rejected this request in the past")
+        self.assertNotIn(
+            str(reviewer_3.email()), addresses,
+            "Reviews should not suggest people who have not responded to this request in the past.")
         self.assertEqual(field.choices[0], ('', '(None)'))
         self.assertEqual(field.choices[1][0], str(reviewer_0.email()))
         self.assertEqual(field.choices[2][0], str(reviewer_1.email()))
