Skip to content

Commit c5ecfab

Browse files
author
Sasha Romijn
committed
Account for previous reviews of ancestor documents (see [16981])
- Legacy-Id: 17046 Note: SVN reference [16981] has been migrated to Git commit f740adc
1 parent c36fcdc commit c5ecfab

2 files changed

Lines changed: 33 additions & 24 deletions

File tree

ietf/review/policies.py

Lines changed: 25 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import six
88

99
from ietf.doc.models import DocumentAuthor, DocAlias
10+
from ietf.doc.utils import extract_complete_replaces_ancestor_mapping_for_docs
1011
from ietf.group.models import Role
1112
from ietf.person.models import Person
1213
import debug # pyflakes:ignore
@@ -261,11 +262,13 @@ def _connections_with_doc(self, doc, person_ids):
261262

262263
def _persons_with_previous_review(self, review_req, possible_person_ids):
263264
"""
264-
Collect anyone in possible_person_ids that have reviewed the request before.
265+
Collect anyone in possible_person_ids that have reviewed the document before,
266+
or an ancestor document.
265267
Returns a set with Person IDs of anyone who has.
266268
"""
269+
doc_names = {review_req.doc.name}.union(*extract_complete_replaces_ancestor_mapping_for_docs([review_req.doc.name]).values())
267270
has_reviewed_previous = ReviewRequest.objects.filter(
268-
doc=review_req.doc,
271+
doc__name__in=doc_names,
269272
reviewassignment__reviewer__person__in=possible_person_ids,
270273
reviewassignment__state="completed",
271274
team=self.team,
@@ -313,30 +316,33 @@ def reviewer_settings_for(person):
313316
print('with skipped {}'.format([r for r in rotation_list]))
314317
print('without skip {}'.format([r for r in rotation_list_without_skip]))
315318
print('skip counts {}'.format([(r, reviewer_settings_for(r).skip_next) for r in rotation_list]))
319+
# In order means: assigned to the first person in the rotation list with skip_next=0
320+
# If the assignment is not in order, skip_next and NextReviewerInTeam are not modified.
316321
in_order_assignment = rotation_list_without_skip[0] == assignee_person
317322
print('in order: {}'.format(in_order_assignment))
318323

319324
# Loop through the list until finding the first person with skip_next=0,
320325
# who is not the current assignee. Anyone with skip_next>0 encountered before
321326
# has their skip_next decreased.
322327
current_idx = 0
323-
while in_order_assignment:
324-
current_idx_person = reviewer_at_index(current_idx)
325-
settings = reviewer_settings_for(current_idx_person)
326-
print('evaluating {} with skip_next {}, assignee {}'.format(current_idx_person, settings.skip_next, assignee_person))
327-
if settings.skip_next > 0:
328-
print('dropping skip_next')
329-
settings.skip_next -= 1
330-
settings.save()
331-
elif current_idx_person != assignee_person:
332-
print('nr appointed')
333-
nr = NextReviewerInTeam.objects.filter(team=self.team).first() or NextReviewerInTeam(
334-
team=self.team)
335-
nr.next_reviewer = current_idx_person
336-
nr.save()
337-
338-
break
339-
current_idx += 1
328+
if in_order_assignment:
329+
while True:
330+
current_idx_person = reviewer_at_index(current_idx)
331+
settings = reviewer_settings_for(current_idx_person)
332+
print('evaluating {} with skip_next {}, assignee {}'.format(current_idx_person, settings.skip_next, assignee_person))
333+
if settings.skip_next > 0:
334+
print('dropping skip_next')
335+
settings.skip_next -= 1
336+
settings.save()
337+
elif current_idx_person != assignee_person:
338+
print('nr appointed')
339+
nr = NextReviewerInTeam.objects.filter(team=self.team).first() or NextReviewerInTeam(
340+
team=self.team)
341+
nr.next_reviewer = current_idx_person
342+
nr.save()
343+
344+
break
345+
current_idx += 1
340346

341347
if add_skip:
342348
print('raising skip count for assignee')

ietf/review/test_policies.py

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
# Copyright The IETF Trust 2016-2019, All Rights Reserved
22

3-
from ietf.doc.factories import WgDraftFactory
3+
from ietf.doc.factories import WgDraftFactory, IndividualDraftFactory
44
from ietf.group.factories import ReviewTeamFactory
55
from ietf.group.models import Group, Role
66
from ietf.person.fields import PersonEmailChoiceField
@@ -177,15 +177,18 @@ def test_determine_ranking(self):
177177
# This reviewer should be ignored because it is not in the rotation list.
178178
create_person(team, "reviewer", name="Test Reviewer-out-of-rotation", username="testreviewer-out-of-rotation")
179179

180-
# Trigger author check, AD check and group check
181-
doc = WgDraftFactory(group__acronym='mars', rev='01', authors=[reviewer_low], ad=reviewer_low, shepherd=reviewer_low.email())
180+
# Create a document with ancestors, that also triggers author check, AD check and group check
181+
doc_individual = IndividualDraftFactory()
182+
doc_wg = WgDraftFactory(relations=[('replaces', doc_individual)])
183+
doc_middle_wg = WgDraftFactory(relations=[('replaces', doc_wg)])
184+
doc = WgDraftFactory(group__acronym='mars', rev='01', authors=[reviewer_low], ad=reviewer_low, shepherd=reviewer_low.email(), relations=[('replaces', doc_middle_wg)])
182185
Role.objects.create(group=doc.group, person=reviewer_low, email=reviewer_low.email(), name_id='advisor')
183186

184187
review_req = ReviewRequestFactory(doc=doc, team=team, type_id='early')
185188
rotation_list = [reviewer_low, reviewer_high, reviewer_unavailable]
186189

187-
# Trigger previous review check and completed review stats - TODO: something something related documents
188-
ReviewAssignmentFactory(review_request__team=team, review_request__doc=doc, reviewer=reviewer_high.email(), state_id='completed')
190+
# Trigger previous review check (including finding ancestor documents) and completed review stats.
191+
ReviewAssignmentFactory(review_request__team=team, review_request__doc=doc_individual, reviewer=reviewer_high.email(), state_id='completed')
189192
# Trigger other review stats
190193
ReviewAssignmentFactory(review_request__team=team, review_request__doc=doc, reviewer=reviewer_high.email(), state_id='no-response')
191194
ReviewAssignmentFactory(review_request__team=team, review_request__doc=doc, reviewer=reviewer_high.email(), state_id='part-completed')

0 commit comments

Comments
 (0)