Merged in [18373] from mark@painless-security.com:

Filter out reviewers who have rejected reviewing this document in the past from the list of suggested reviewers.  Fixes #2996.
 - Legacy-Id: 18448
Note: SVN reference [18373] has been migrated to Git commit d105f8ba5b
This commit is contained in:
Henrik Levkowetz 2020-08-31 11:21:07 +00:00
commit a1b1a919ab
2 changed files with 32 additions and 2 deletions

View file

@ -126,8 +126,25 @@ class AbstractReviewerQueuePolicy:
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
).exclude( person_id__in=rejecting_reviewer_ids )
one_assignment = (review_req.reviewassignment_set
.exclude(state__slug__in=('rejected', 'no-response'))
.first())
if one_assignment:
field.initial = one_assignment.reviewer_id

View file

@ -91,11 +91,24 @@ class RotateAlphabeticallyReviewerAndGenericQueuePolicyTest(TestCase):
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()))