Skip to content

Commit a1b1a91

Browse files
committed
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 ietf-tools#2996. - Legacy-Id: 18448 Note: SVN reference [18373] has been migrated to Git commit d105f8b
2 parents 95bca1d + d105f8b commit a1b1a91

2 files changed

Lines changed: 32 additions & 2 deletions

File tree

ietf/review/policies.py

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -126,8 +126,25 @@ def setup_reviewer_field(self, field, review_req):
126126
The field should be an instance similar to
127127
PersonEmailChoiceField(label="Assign Reviewer", empty_label="(None)")
128128
"""
129-
field.queryset = field.queryset.filter(role__name="reviewer", role__group=review_req.team)
130-
one_assignment = review_req.reviewassignment_set.first()
129+
130+
# Collect a set of person IDs for people who have either not responded
131+
# to or outright rejected reviewing this document in the past
132+
rejecting_reviewer_ids = review_req.doc.reviewrequest_set.filter(
133+
reviewassignment__state__slug__in=('rejected', 'no-response')
134+
).values_list(
135+
'reviewassignment__reviewer__person_id', flat=True
136+
)
137+
138+
# Query the Email objects for reviewers who haven't rejected or
139+
# not responded to this document in the past
140+
field.queryset = field.queryset.filter(
141+
role__name="reviewer",
142+
role__group=review_req.team
143+
).exclude( person_id__in=rejecting_reviewer_ids )
144+
145+
one_assignment = (review_req.reviewassignment_set
146+
.exclude(state__slug__in=('rejected', 'no-response'))
147+
.first())
131148
if one_assignment:
132149
field.initial = one_assignment.reviewer_id
133150

ietf/review/tests_policies.py

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,11 +91,24 @@ def test_setup_reviewer_field(self):
9191
policy = RotateAlphabeticallyReviewerQueuePolicy(team)
9292
reviewer_0 = create_person(team, "reviewer", name="Test Reviewer-0", username="testreviewer0")
9393
reviewer_1 = create_person(team, "reviewer", name="Test Reviewer-1", username="testreviewer1")
94+
reviewer_2 = create_person(team, "reviewer", name="Test Reviewer-2", username="testreviewer2")
95+
reviewer_3 = create_person(team, "reviewer", name="Test Reviewer-3", username="testreviewer3")
9496
review_req = ReviewRequestFactory(team=team, type_id='early')
97+
review_req_2 = ReviewRequestFactory(team=team, type_id='early', doc=review_req.doc)
98+
review_req_3 = ReviewRequestFactory(team=team, type_id='early', doc=review_req.doc)
9599
ReviewAssignmentFactory(review_request=review_req, reviewer=reviewer_1.email(), state_id='part-completed')
100+
ReviewAssignmentFactory(review_request=review_req_2, reviewer=reviewer_2.email(), state_id='rejected')
101+
ReviewAssignmentFactory(review_request=review_req_3, reviewer=reviewer_3.email(), state_id='no-response')
96102
field = PersonEmailChoiceField(label="Assign Reviewer", empty_label="(None)", required=False)
97103

98104
policy.setup_reviewer_field(field, review_req)
105+
addresses = list( map( lambda choice: choice[0], field.choices ) )
106+
self.assertNotIn(
107+
str(reviewer_2.email()), addresses,
108+
"Reviews should not suggest people who have rejected this request in the past")
109+
self.assertNotIn(
110+
str(reviewer_3.email()), addresses,
111+
"Reviews should not suggest people who have not responded to this request in the past.")
99112
self.assertEqual(field.choices[0], ('', '(None)'))
100113
self.assertEqual(field.choices[1][0], str(reviewer_0.email()))
101114
self.assertEqual(field.choices[2][0], str(reviewer_1.email()))

0 commit comments

Comments
 (0)