datatracker/ietf/review/tests_policies.py
Lars Eggert 57f23f5198
chore: feat/dark-mode <- main (#6103)
* chore: Remove unused "rendertest" stuff (#6015)

* fix: restore ability to create status change documents (#5963)

* fix: restore ability to create status change documents

Fixes #5962

* chore: address review comment

* fix: Provide human-friendly status in submission status API response (#6011)

Co-authored-by: nectostr <bastinda96@gmail.com>

* fix: Make name/email lookups case-insensitive (#5972) (#6007)

* fix: Make name/email lookups case-insensitive (#5972)

Use icontains so that looking up name or email is case insensitive
Added a test

Fixes: 5972

* fix: Use __iexact not __icontains

* fix: Clarify no-action-needed (#5918) (#6020)

When a draft is submitted for manual processing, clarify that
no action is needed; the Secretariat has the next steps.

Fixes: #5918

* fix: Fix menu hover issue (#6019)

* fix: Fix menu hover issue

Fixes #5702

* Fix leftmenu hover issue

* fix: Server error from api_get_session_materials() (#6025)

Fixes #5877

* fix: Clarify Questionnaire label (#4688) (#6017)

When filtering nominees, `Questionnaire` implies `Accepted == yes`
so fix the dropdown test tosay that.

Fixes: #4688

* chore: Merge from @martinthomson's rfc-txt-html (#6023)

* fix:no history entry when changing RFC Editor note for doc (#6021)

* fix:no history entry when changing RFC Editor note for doc

* fix:no history entry when changing RFC Editor note for doc

---------

Co-authored-by: Priyanka Narkar <priyankanarkar@dhcp-91f8.meeting.ietf.org>

* fix: avoid deprecation warning on view_list() for objs without CommunityList

Fixes #5942

* fix: return 404 for non-existing revisions (#6014)

* fix: return 404 for non-existing revisions
Links to non-existing revisions to docs should return 404

* fix: change rfc/rev and search behaviour

* refactor: fix tab level

* fix: return 404 for rfc revision for bibtex

* fix: provide date for revisions in bibtex output (#6029)

* fix: provide date for revisions in bibtex output

* refactor: change walrus to if's

* fix: specify particular revision for events

* fix: review refactoring issue

fixes #5447

* fix:  Remove automatically suggested document for document that is already has review request (fixes #3211) (#5425)

* Added check that if there is already review request for the document
in question, ignore the automatic suggestion for that document.
Fixes #3211.

* fix: dont block on open requests for a previous version. Add tests

---------

Co-authored-by: Nicolas Giard <github@ngpixel.com>
Co-authored-by: Robert Sparks <rjsparks@nostrum.com>

* feat: IAB statements (#5940)

* feat: support iab and iesg statements. Import iab statements. (#5895)

* feat: infrastructure for statements doctype

* chore: basic test framework

* feat: basic statement document view

* feat: show replaced statements

* chore: black

* fix: state help for statements

* fix: cleanout non-relevant email expansions

* feat: import iab statements, provide group statements tab

* fix: guard against running import twice

* feat: build redirect csv for iab statements

* fix: set document state on import

* feat: show published date on main doc view

* feat: handle pdf statements

* feat: create new and update statements

* chore: copyright block updates

* chore: remove flakes

* chore: black

* feat: add edit/new buttons for the secretariat

* fix: address PR #5895 review comments

* fix: pin pydantic until inflect catches up (#5901) (#5902)

* chore: re-un-pin pydantic

* feat: include submitter in email about submitted slides (#6033)

* feat: include submitter in email about submitted slides

fixes #6031

* chore: remove unintended whitespace change

* chore(dev): update .vscode/settings.json with new taskExplorer settings

* fix: Add editorial stream to proceedings (#6027)

* fix: Add editorial stream to proceedings

Fixes #5717

* fix: Move editorial stream after the irtf in proceedings

* fix: Add editorial stream to meeting materials (#6047)

Fixes #6042

* fix: Shows requested reviews for doc fixes (#6022)

* Fix: Shows requested reviews for doc

* Changed template includes to only give required variables to them.

* feat: allow openId to choose an unactive email if there are none active (#6041)

* feat: allow openId to choose an unactive email if there are no active ones

* chore: correct typo

* chore: rename unactive to inactive

* fix: Make review table more responsive (#6053)

* fix: Improve layout of review table

* Progress

* Progress

* Final changes

* Fix tests

* Remove fluff

* Undo commits

* ci: add --validate-html-harder to tests

* ci: add  --validate-html-harder to build.yml workflow

* fix: Set colspan to actual number of columns (#6069)

* fix: Clean up view_feedback_pending (#6070)

- Remove "Unclassified" column header, which caused misalignment in the table body.

- Show the message author - previously displayed as `(None)`.

* docs: Update LICENSE year

* fix: Remove IESG state edit button when state is 'dead' (#6051) (#6065)

* fix: Correctly order "last call requested" column in the IESG dashboard (#6079)

* ci: update dev sandbox init script to start memcached

* feat: Reclassify nomcom feedback (#6002)

* fix: Clean up view_feedback_pending

- Remove "Unclassified" column header, which caused misalignment in the table body.

- Show the message author - previously displayed as `(None)`.

* feat: Reclassify nomcom feedback (#4669)

- There's a new `Chair/Advisor Tasks` menu item `Reclassify feedback`.

- I overloaded `view_feedback*` URLs with a `?reclassify` parameter.

- This adds a checkbox to each feedback message, and a `Reclassify` button
at the bottom of each feedback page.

- "Reclassifying" basically de-classifies the feedback, and punts it back
to the "Pending emails" view for reclassification.

- If a feedback has been applied to multiple nominees, declassifying it
from one nominee removes it from all.

* fix: Remove unused local variables

* fix: Fix some missing and mis-nested html

* test: Add tests for reclassifying feedback

* refactor: Substantial redesign of feedback reclassification

- Break out reclassify_feedback* as their own URLs and views,
  and revert changes to view_feedback*.html.

- Replace checkboxes with a Reclassify button on each message.

* fix: Remember to clear the feedback associations when reclassifying

* feat: Add an 'Overcome by events' feedback type

* refactor: When invoking reclassification from a view-feedback page, load the corresponding reclassify-feedback page

* fix: De-conflict migration with 0004_statements

Also change the coding style to match, and add a reverse migration.

* fix: Fix a test case to account for new feedback type

* fix: 842e730 broke the Back button

* refactor: Reclassify feedback directly instead of putting it back in the work queue

* fix: Adjust tests to new workflow

* refactor: Further refine reclassification to avoid redirects

* refactor: Impose a FeedbackTypeName ordering

Also add FeedbackTypeName.legend field, rather than synthesizing it every
time we classify or reclassify feedback.

In the reclassification forms, only show the relevant feedback types.

* refactor: Merge reclassify_feedback_* back into view_feedback_*

This means the "Reclassify" button is always present, but eliminates some
complexity.

* refactor: Add filter(used=True) on FeedbackTypeName querysets

* refactor: Add the new FeedbackTypeName to the reclassification success message

* fix: Secure reclassification against rogue nomcom members

* fix: Print decoded key and fully clean up test nomcom (#6094)

* fix: Delete Person records when deleting a test nomcom

* fix: Decode test nomcom private key before printing

* test: Use correct time zone for test_statement_doc_view (#6064)

* chore(deps): update all npm dependencies for playwright (#6061)

Co-authored-by: depfu[bot] <23717796+depfu[bot]@users.noreply.github.com>

* chore(deps): update all npm dependencies for dev/diff (#6062)

Co-authored-by: depfu[bot] <23717796+depfu[bot]@users.noreply.github.com>

* chore(deps): update all npm dependencies for dev/coverage-action (#6063)

Co-authored-by: depfu[bot] <23717796+depfu[bot]@users.noreply.github.com>

* fix: Hash cache key for default memcached cache (#6089)

* feat: Show docs that an AD hasn't balloted on that need ballots to progress (#6075)

* fix(doc): Unify help texts for document states (#6060)

* Fix IESG State help text link (only)

* Intermediate checkpoint

* Correct URL filtering of state descriptions

* Unify help texts for document states

* Remove redundant load static from template

---------

Co-authored-by: Robert Sparks <rjsparks@nostrum.com>

* ci: fix sandbox start.sh memcached user

* fix: refactor how settings handles cache definitions (#6099)

* fix: refactor how settings handles cache definitions

* chore: more english-speaker readable expression

* fix: Cast cache key to str before calling encode (#6100)

---------

Co-authored-by: Robert Sparks <rjsparks@nostrum.com>
Co-authored-by: Liubov Kurafeeva <liubov.kurafeeva@gmail.com>
Co-authored-by: nectostr <bastinda96@gmail.com>
Co-authored-by: Rich Salz <rsalz@akamai.com>
Co-authored-by: PriyankaN <priyanka@amsl.com>
Co-authored-by: Priyanka Narkar <priyankanarkar@dhcp-91f8.meeting.ietf.org>
Co-authored-by: Ali <alireza83@gmail.com>
Co-authored-by: Roman Beltiukov <maybe.hello.world@gmail.com>
Co-authored-by: Tero Kivinen <kivinen@iki.fi>
Co-authored-by: Nicolas Giard <github@ngpixel.com>
Co-authored-by: Kesara Rathnayake <kesara@fq.nz>
Co-authored-by: Jennifer Richards <jennifer@staff.ietf.org>
Co-authored-by: Paul Selkirk <paul@painless-security.com>
Co-authored-by: depfu[bot] <23717796+depfu[bot]@users.noreply.github.com>
Co-authored-by: Jim Fenton <fenton@bluepopcorn.net>
2023-08-10 09:49:15 -05:00

844 lines
44 KiB
Python

# Copyright The IETF Trust 2016-2021, All Rights Reserved
import debug # pyflakes:ignore
import datetime
from django.utils import timezone
from ietf.doc.factories import WgDraftFactory, IndividualDraftFactory
from ietf.group.factories import ReviewTeamFactory
from ietf.group.models import Group, Role
from ietf.name.models import ReviewerQueuePolicyName
from ietf.person.factories import PersonFactory
from ietf.person.fields import PersonEmailChoiceField
from ietf.person.models import Email
from ietf.review.factories import ReviewAssignmentFactory, ReviewRequestFactory
from ietf.review.models import ReviewerSettings, NextReviewerInTeam, UnavailablePeriod, ReviewWish, \
ReviewTeamSettings
from ietf.review.policies import (AssignmentOrderResolver, LeastRecentlyUsedReviewerQueuePolicy,
get_reviewer_queue_policy, QUEUE_POLICY_NAME_MAPPING)
from ietf.utils.test_data import create_person
from ietf.utils.test_utils import TestCase
class GetReviewerQueuePolicyTest(TestCase):
def test_valid_policy(self):
team = ReviewTeamFactory(acronym="rotationteam", name="Review Team", list_email="rotationteam@ietf.org", parent=Group.objects.get(acronym="farfut"), settings__reviewer_queue_policy_id='LeastRecentlyUsed')
policy = get_reviewer_queue_policy(team)
self.assertEqual(policy.__class__, LeastRecentlyUsedReviewerQueuePolicy)
def test_missing_settings(self):
team = ReviewTeamFactory(acronym="rotationteam", name="Review Team", list_email="rotationteam@ietf.org", parent=Group.objects.get(acronym="farfut"))
ReviewTeamSettings.objects.all().delete()
with self.assertRaises(ValueError):
get_reviewer_queue_policy(team)
def test_invalid_policy_name(self):
ReviewerQueuePolicyName.objects.create(slug='invalid')
team = ReviewTeamFactory(acronym="rotationteam", name="Review Team", list_email="rotationteam@ietf.org", parent=Group.objects.get(acronym="farfut"), settings__reviewer_queue_policy_id='invalid')
with self.assertRaises(ValueError):
get_reviewer_queue_policy(team)
class _Wrapper(TestCase):
"""Wrapper class - exists to prevent UnitTest from trying to run the base class tests"""
def test_all_reviewer_queue_policies_have_tests(self):
"""Every ReviewerQueuePolicy should be tested"""
rqp_test_classes = self.ReviewerQueuePolicyTestCase.__subclasses__()
self.assertCountEqual(
QUEUE_POLICY_NAME_MAPPING.keys(),
[cls.reviewer_queue_policy_id for cls in rqp_test_classes],
)
class ReviewerQueuePolicyTestCase(TestCase):
"""Parent class to define interface / default tests for QueuePolicy implementation tests
To add tests for a new AbstractReviewerQueuePolicy class, you need to:
1. Subclass _Wrapper.ReviewerQueuePolicyTestCase (i.e., this class)
2. Define the reviewer_queue_policy_id class variable in your new class
3. (Maybe) implement a class-specific append_reviewer() method to add a new
reviewer that sorts to the end of default_reviewer_rotation_list()
4. Fill in any tests that raise NotImplemented exceptions
5. Override any other tests that should have different behavior for your new policy
6. Add any policy-specific tests
When adding tests to this default class, be careful not to make assumptions about
the ordering of reviewers. The only guarantee is that append_reviewer() adds a
new reviewer who is later in the default rotation for the next assignment. Once that
assignment is made, the rotation order is entirely unknown! If you need to make
such assumptions, call policy.default_reviewer_rotation_list() or move the test
into a policy-specific subclass.
"""
# Must define reviewer_queue_policy_id in test subclass
reviewer_queue_policy_id = ''
def setUp(self):
super().setUp()
self.team = ReviewTeamFactory(acronym="rotationteam",
name="Review Team",
list_email="rotationteam@ietf.org",
parent=Group.objects.get(acronym="farfut"))
self.team.reviewteamsettings.reviewer_queue_policy_id = self.reviewer_queue_policy_id
self.team.reviewteamsettings.save()
self.policy = get_reviewer_queue_policy(self.team)
self.reviewers = []
def append_reviewer(self, skip_count=None):
"""Create a reviewer who will appear in the assignee options list
Newly added reviewer must come later in the default_reviewer_rotation_list. The default
implementation creates users whose names are in lexicographic order.
"""
index = len(self.reviewers)
assert(index < 100) # ordering by label will fail if > 100 reviewers are created
label = '{:02d}'.format(index)
reviewer = create_person(self.team,
'reviewer',
name='Test Reviewer{}'.format(label),
username='testreviewer{}'.format(label))
self.reviewers.append(reviewer)
if skip_count is not None:
settings = self.reviewer_settings_for(reviewer)
settings.skip_next = skip_count
settings.save()
return reviewer
def create_old_review_assignment(self, reviewer, **kwargs):
"""Create a review that won't disturb the ordering of reviewers"""
return ReviewAssignmentFactory(reviewer=reviewer.email(), **kwargs)
def reviewer_settings_for(self, person):
return (ReviewerSettings.objects.filter(team=self.team, person=person).first()
or ReviewerSettings(team=self.team, person=person))
def test_set_wants_to_be_next(self):
# Subclass must implement this
raise NotImplementedError
def test_default_reviewer_rotation_list_ignores_out_of_team_reviewers(self):
available_reviewers, _ = self.set_up_default_reviewer_rotation_list_test()
# This reviewer has an assignment, but is no longer in the team and should not be in rotation.
out_of_team_reviewer = PersonFactory()
ReviewAssignmentFactory(review_request__team=self.team, reviewer=out_of_team_reviewer.email())
# No known assignments, order in PK order.
rotation = self.policy.default_reviewer_rotation_list()
self.assertNotIn(out_of_team_reviewer, rotation)
self.assertEqual(rotation, available_reviewers)
def test_assign_reviewer(self):
"""assign_reviewer() should create a review assignment for the correct user"""
review_req = ReviewRequestFactory(team=self.team)
for _ in range(3):
self.append_reviewer()
self.assertFalse(review_req.reviewassignment_set.exists())
reviewer = self.reviewers[0]
self.policy.assign_reviewer(review_req, reviewer.email(), add_skip=False)
self.assertCountEqual(
review_req.reviewassignment_set.all().values_list('reviewer', flat=True),
[str(reviewer.email())]
)
self.assertEqual(self.reviewer_settings_for(reviewer).skip_next, 0)
def test_assign_reviewer_and_add_skip(self):
"""assign_reviewer() should create a review assignment for the correct user"""
review_req = ReviewRequestFactory(team=self.team)
for _ in range(3):
self.append_reviewer()
self.assertFalse(review_req.reviewassignment_set.exists())
reviewer = self.reviewers[0]
self.policy.assign_reviewer(review_req, reviewer.email(), add_skip=True)
self.assertCountEqual(
review_req.reviewassignment_set.all().values_list('reviewer', flat=True),
[str(reviewer.email())]
)
self.assertEqual(self.reviewer_settings_for(reviewer).skip_next, 1)
def test_assign_reviewer_updates_skip_next_minimal(self):
"""If we skip the first reviewer, their skip_next value should decrement
Different policies handle skipping in different ways.
The only assumption we make in the base test class is that an in-order assignment
to a non-skipped reviewer will decrement the skip_next for any reviewers we skipped.
Any other tests are policy-specific (e.g., the RotateAlphabetically policy will
also decrement any users skipped between the assignee and the next reviewer in the
rotation)
"""
review_req = ReviewRequestFactory(team=self.team)
reviewer_to_skip = self.append_reviewer()
settings = self.reviewer_settings_for(reviewer_to_skip)
settings.skip_next = 1
settings.save()
another_reviewer_to_skip = self.append_reviewer()
settings = self.reviewer_settings_for(another_reviewer_to_skip)
settings.skip_next = 1
settings.save()
reviewer_to_assign = self.append_reviewer()
reviewer_to_ignore = self.append_reviewer()
# Check test assumptions
self.assertEqual(
self.policy.default_reviewer_rotation_list(),
[
reviewer_to_skip,
another_reviewer_to_skip,
reviewer_to_assign,
reviewer_to_ignore,
],
)
self.assertEqual(self.reviewer_settings_for(reviewer_to_skip).skip_next, 1)
self.assertEqual(self.reviewer_settings_for(another_reviewer_to_skip).skip_next, 1)
self.assertEqual(self.reviewer_settings_for(reviewer_to_assign).skip_next, 0)
self.policy.assign_reviewer(review_req, reviewer_to_assign.email(), add_skip=False)
# Check results
self.assertEqual(self.reviewer_settings_for(reviewer_to_skip).skip_next, 0,
'skip_next not updated for first skipped reviewer')
self.assertEqual(self.reviewer_settings_for(another_reviewer_to_skip).skip_next, 0,
'skip_next not updated for second skipped reviewer')
def test_assign_reviewer_updates_skip_next_with_add_skip(self):
"""Skipping reviewers with add_skip=True should update skip_counts properly
Subclasses must implement
"""
raise NotImplementedError
def test_assign_reviewer_updates_skip_next_without_add_skip(self):
"""Skipping reviewers with add_skip=False should update skip_counts properly
Subclasses must implement
"""
raise NotImplementedError
def test_assign_reviewer_ignores_skip_next_on_out_of_order_assignment(self):
"""If assignment is not in-order, skip_next values should not change"""
review_req = ReviewRequestFactory(team=self.team)
reviewer_to_skip = self.append_reviewer()
settings = self.reviewer_settings_for(reviewer_to_skip)
settings.skip_next = 1
settings.save()
reviewer_to_ignore = self.append_reviewer()
reviewer_to_assign = self.append_reviewer()
another_reviewer_to_skip = self.append_reviewer()
settings = self.reviewer_settings_for(another_reviewer_to_skip)
settings.skip_next = 3
settings.save()
# Check test assumptions
self.assertEqual(
self.policy.default_reviewer_rotation_list(),
[
reviewer_to_skip,
reviewer_to_ignore,
reviewer_to_assign,
another_reviewer_to_skip,
],
)
self.assertEqual(self.reviewer_settings_for(reviewer_to_skip).skip_next, 1)
self.assertEqual(self.reviewer_settings_for(reviewer_to_ignore).skip_next, 0)
self.assertEqual(self.reviewer_settings_for(reviewer_to_assign).skip_next, 0)
self.assertEqual(self.reviewer_settings_for(another_reviewer_to_skip).skip_next, 3)
self.policy.assign_reviewer(review_req, reviewer_to_assign.email(), add_skip=False)
# Check results
self.assertEqual(self.reviewer_settings_for(reviewer_to_skip).skip_next, 1,
'skip_next changed unexpectedly for first skipped reviewer')
self.assertEqual(self.reviewer_settings_for(reviewer_to_ignore).skip_next, 0,
'skip_next changed unexpectedly for ignored reviewer')
self.assertEqual(self.reviewer_settings_for(reviewer_to_assign).skip_next, 0,
'skip_next changed unexpectedly for assigned reviewer')
self.assertEqual(self.reviewer_settings_for(another_reviewer_to_skip).skip_next, 3,
'skip_next changed unexpectedly for second skipped reviewer')
def test_assign_reviewer_updates_skip_next_when_canfinish_other_doc(self):
"""Should update skip_next when 'canfinish' set for someone unrelated to this doc"""
completed_req = ReviewRequestFactory(team=self.team, state_id='assigned')
assigned_req = ReviewRequestFactory(team=self.team, state_id='assigned')
new_req = ReviewRequestFactory(team=self.team, doc=assigned_req.doc)
reviewer_to_skip = self.append_reviewer()
settings = self.reviewer_settings_for(reviewer_to_skip)
settings.skip_next = 1
settings.save()
# Has completed a review of some other document - unavailable for current req
canfinish_reviewer = self.append_reviewer()
UnavailablePeriod.objects.create(
team=self.team,
person=canfinish_reviewer,
start_date='2000-01-01',
availability='canfinish',
)
self.create_old_review_assignment(
reviewer=canfinish_reviewer,
review_request=completed_req,
state_id='completed',
)
# Has no review assignments at all
canfinish_reviewer_no_review = self.append_reviewer()
UnavailablePeriod.objects.create(
team=self.team,
person=canfinish_reviewer_no_review,
start_date='2000-01-01',
availability='canfinish',
)
# Has accepted but not completed a review of this document
canfinish_reviewer_no_completed = self.append_reviewer()
UnavailablePeriod.objects.create(
team=self.team,
person=canfinish_reviewer_no_completed,
start_date='2000-01-01',
availability='canfinish',
)
self.create_old_review_assignment(
reviewer=canfinish_reviewer_no_completed,
review_request=assigned_req,
state_id='accepted',
)
reviewer_to_assign = self.append_reviewer()
self.assertEqual(
self.policy.default_reviewer_rotation_list(),
[
reviewer_to_skip,
canfinish_reviewer,
canfinish_reviewer_no_review,
canfinish_reviewer_no_completed,
reviewer_to_assign
],
'Test logic error - reviewers not in expected starting order'
)
# assign the review
self.policy.assign_reviewer(new_req, reviewer_to_assign.email(), add_skip=False)
# Check results
self.assertEqual(self.reviewer_settings_for(reviewer_to_skip).skip_next, 0,
'skip_next not updated for skipped reviewer')
self.assertEqual(self.reviewer_settings_for(canfinish_reviewer).skip_next, 0,
'skip_next changed unexpectedly for "canfinish" unavailable reviewer')
self.assertEqual(self.reviewer_settings_for(canfinish_reviewer_no_review).skip_next, 0,
'skip_next changed unexpectedly for "canfinish" unavailable reviewer with no review')
self.assertEqual(self.reviewer_settings_for(canfinish_reviewer_no_completed).skip_next, 0,
'skip_next changed unexpectedly for "canfinish" unavailable reviewer with no completed review')
self.assertEqual(self.reviewer_settings_for(reviewer_to_assign).skip_next, 0,
'skip_next changed unexpectedly for assigned reviewer')
def test_assign_reviewer_ignores_skip_next_when_canfinish_this_doc(self):
"""Should not update skip_next when 'canfinish' set for prior reviewer of current req
If a reviewer is unavailable but 'canfinish' and has previously completed a review of this
doc, they are a candidate to be assigned to it. In that case, when skip_next == 0, skipping
over them means the assignment was not 'in order' and skip_next should not be updated.
"""
completed_req = ReviewRequestFactory(team=self.team, state_id='assigned')
new_req = ReviewRequestFactory(team=self.team, doc=completed_req.doc)
reviewer_to_skip = self.append_reviewer()
settings = self.reviewer_settings_for(reviewer_to_skip)
settings.skip_next = 1
settings.save()
canfinish_reviewer = self.append_reviewer()
UnavailablePeriod.objects.create(
team=self.team,
person=canfinish_reviewer,
start_date='2000-01-01',
availability='canfinish',
)
self.create_old_review_assignment(
reviewer=canfinish_reviewer,
review_request=completed_req,
state_id='completed',
)
reviewer_to_assign = self.append_reviewer()
self.assertEqual(self.policy.default_reviewer_rotation_list(),
[reviewer_to_skip, canfinish_reviewer, reviewer_to_assign],
'Test logic error - reviewers not in expected starting order')
# assign the review
self.policy.assign_reviewer(new_req, reviewer_to_assign.email(), add_skip=False)
# Check results
self.assertEqual(self.reviewer_settings_for(reviewer_to_skip).skip_next, 1,
'skip_next changed unexpectedly for skipped reviewer')
self.assertEqual(self.reviewer_settings_for(canfinish_reviewer).skip_next, 0,
'skip_next changed unexpectedly for "canfinish" reviewer')
self.assertEqual(self.reviewer_settings_for(reviewer_to_assign).skip_next, 0,
'skip_next changed unexpectedly for assigned reviewer')
def set_up_default_reviewer_rotation_list_test(self):
"""Helper to set up for the test_default_reviewer_rotation_list test and related tests"""
for i in range(5):
self.append_reviewer()
# This reviewer should never be included.
unavailable_reviewer = self.append_reviewer()
UnavailablePeriod.objects.create(
team=self.team,
person=unavailable_reviewer,
start_date='2000-01-01',
availability='unavailable',
)
# This should not have any impact. Canfinish unavailable reviewers are included in
# the default rotation, and filtered further when making assignment choices.
UnavailablePeriod.objects.create(
team=self.team,
person=self.reviewers[1],
start_date='2000-01-01',
availability='canfinish',
)
return (
[r for r in self.reviewers if r is not unavailable_reviewer], # available reviewers
unavailable_reviewer,
)
def test_default_reviewer_rotation_list(self):
available_reviewers, unavailable_reviewer = self.set_up_default_reviewer_rotation_list_test()
rotation = self.policy.default_reviewer_rotation_list()
self.assertNotIn(unavailable_reviewer, rotation)
self.assertEqual(rotation, available_reviewers)
def test_recommended_assignment_order(self):
reviewer_low = self.append_reviewer()
reviewer_high = self.append_reviewer()
# reviewer_high appears later in the default rotation, but reviewer_low is the author
doc = WgDraftFactory(group__acronym='mars', rev='01', authors=[reviewer_low])
review_req = ReviewRequestFactory(doc=doc, team=self.team, type_id='early')
order = self.policy.recommended_assignment_order(Email.objects.all(), review_req)
self.assertEqual(order[0][0], str(reviewer_high.email()))
self.assertEqual(order[1][0], str(reviewer_low.email()))
self.assertIn('{}: #2'.format(reviewer_high.name), order[0][1])
self.assertIn('{}: is author of document; #1'.format(reviewer_low.name), order[1][1])
with self.assertRaises(ValueError):
review_req_other_team = ReviewRequestFactory(doc=doc, type_id='early')
self.policy.recommended_assignment_order(Email.objects.all(), review_req_other_team)
def test_setup_reviewer_field(self):
review_req = ReviewRequestFactory(team=self.team, type_id='early')
reviewer = self.append_reviewer()
partial_reviewer = self.append_reviewer()
ReviewAssignmentFactory(review_request=review_req,
reviewer=partial_reviewer.email(),
state_id='part-completed')
rejected_reviewer = self.append_reviewer()
ReviewAssignmentFactory(review_request=ReviewRequestFactory(team=self.team,
type_id='early',
doc=review_req.doc),
reviewer=rejected_reviewer.email(),
state_id='rejected')
no_response_reviewer = self.append_reviewer()
ReviewAssignmentFactory(review_request=ReviewRequestFactory(team=self.team,
type_id='early',
doc=review_req.doc),
reviewer=no_response_reviewer.email(),
state_id='no-response')
field = PersonEmailChoiceField(label="Assign Reviewer", empty_label="(None)", required=False)
self.policy.setup_reviewer_field(field, review_req)
addresses = list( map( lambda choice: choice[0], field.choices ) )
self.assertNotIn(
str(rejected_reviewer.email()), addresses,
"Reviews should not suggest people who have rejected this request in the past")
self.assertNotIn(
str(no_response_reviewer.email()), addresses,
"Reviews should not suggest people who have not responded to this request in the past.")
self.assertEqual(field.initial, str(partial_reviewer.email()))
self.assertEqual(field.choices[0], ('', '(None)'))
self.assertEqual(field.choices[1][0], str(reviewer.email()))
self.assertEqual(field.choices[2][0], str(partial_reviewer.email()))
self.assertIn('{}: #1'.format(reviewer.name), field.choices[1][1])
self.assertIn('{}: #2'.format(partial_reviewer.name), field.choices[2][1])
self.assertIn('1 partially complete', field.choices[2][1])
class RotateAlphabeticallyReviewerQueuePolicyTest(_Wrapper.ReviewerQueuePolicyTestCase):
reviewer_queue_policy_id = 'RotateAlphabetically'
def test_default_reviewer_rotation_list_with_nextreviewerinteam(self):
available_reviewers, _ = self.set_up_default_reviewer_rotation_list_test()
assert(len(available_reviewers) > 4)
# Policy with a current NextReviewerInTeam
NextReviewerInTeam.objects.create(team=self.team, next_reviewer=available_reviewers[3])
rotation = self.policy.default_reviewer_rotation_list()
self.assertEqual(rotation, available_reviewers[3:] + available_reviewers[:3])
# Policy with a NextReviewerInTeam that has left the team.
Role.objects.get(person=available_reviewers[1]).delete()
NextReviewerInTeam.objects.filter(team=self.team).update(next_reviewer=available_reviewers[1])
rotation = self.policy.default_reviewer_rotation_list()
self.assertEqual(rotation, available_reviewers[2:] + available_reviewers[:1])
def test_set_wants_to_be_next(self):
reviewer = self.append_reviewer()
self.policy.set_wants_to_be_next(reviewer)
self.assertTrue(self.reviewer_settings_for(reviewer).request_assignment_next)
def test_update_policy_state_for_assignment(self):
# make a bunch of reviewers
review_req = ReviewRequestFactory(team=self.team)
for i in range(5):
self.append_reviewer()
reviewers = self.reviewers
self.assertEqual(reviewers, self.policy.default_reviewer_rotation_list())
def get_skip_next(person):
return self.reviewer_settings_for(person).skip_next
# Regular in-order assignment without skips
reviewer0_settings = self.reviewer_settings_for(reviewers[0])
reviewer0_settings.request_assignment_next = True
reviewer0_settings.save()
self.policy.update_policy_state_for_assignment(review_req, assignee_person=reviewers[0], add_skip=False)
self.assertEqual(NextReviewerInTeam.objects.get(team=self.team).next_reviewer, reviewers[1])
self.assertEqual(get_skip_next(reviewers[0]), 0)
self.assertEqual(get_skip_next(reviewers[1]), 0)
self.assertEqual(get_skip_next(reviewers[2]), 0)
self.assertEqual(get_skip_next(reviewers[3]), 0)
self.assertEqual(get_skip_next(reviewers[4]), 0)
# request_assignment_next should be reset after any assignment
self.assertFalse(self.reviewer_settings_for(reviewers[0]).request_assignment_next)
# In-order assignment with add_skip
self.policy.update_policy_state_for_assignment(review_req, assignee_person=reviewers[1], add_skip=True)
self.assertEqual(NextReviewerInTeam.objects.get(team=self.team).next_reviewer, reviewers[2])
self.assertEqual(get_skip_next(reviewers[0]), 0)
self.assertEqual(get_skip_next(reviewers[1]), 1) # from current add_skip=True
self.assertEqual(get_skip_next(reviewers[2]), 0)
self.assertEqual(get_skip_next(reviewers[3]), 0)
self.assertEqual(get_skip_next(reviewers[4]), 0)
# In-order assignment to 2, but 3 has a skip_next, so 4 should be assigned.
# 3 has skip_next decreased as it is skipped over, 1 retains its skip_next
reviewer3_settings = self.reviewer_settings_for(reviewers[3])
reviewer3_settings.skip_next = 2
reviewer3_settings.save()
self.policy.update_policy_state_for_assignment(review_req, assignee_person=reviewers[2], add_skip=False)
self.assertEqual(NextReviewerInTeam.objects.get(team=self.team).next_reviewer, reviewers[4])
self.assertEqual(get_skip_next(reviewers[0]), 0)
self.assertEqual(get_skip_next(reviewers[1]), 1) # from previous add_skip=true
self.assertEqual(get_skip_next(reviewers[2]), 0)
self.assertEqual(get_skip_next(reviewers[3]), 1) # from manually set skip_next - 1
self.assertEqual(get_skip_next(reviewers[4]), 0)
# Out of order assignments, nothing should change,
# except the add_skip=True should still apply
self.policy.update_policy_state_for_assignment(review_req, assignee_person=reviewers[3], add_skip=False)
self.policy.update_policy_state_for_assignment(review_req, assignee_person=reviewers[2], add_skip=False)
self.policy.update_policy_state_for_assignment(review_req, assignee_person=reviewers[1], add_skip=False)
self.policy.update_policy_state_for_assignment(review_req, assignee_person=reviewers[0], add_skip=True)
self.assertEqual(NextReviewerInTeam.objects.get(team=self.team).next_reviewer, reviewers[4])
self.assertEqual(get_skip_next(reviewers[0]), 1) # from current add_skip=True
self.assertEqual(get_skip_next(reviewers[1]), 1)
self.assertEqual(get_skip_next(reviewers[2]), 0)
self.assertEqual(get_skip_next(reviewers[3]), 1)
self.assertEqual(get_skip_next(reviewers[4]), 0)
# Regular assignment, testing wrap-around
self.policy.update_policy_state_for_assignment(review_req, assignee_person=reviewers[4], add_skip=False)
self.assertEqual(NextReviewerInTeam.objects.get(team=self.team).next_reviewer, reviewers[2])
self.assertEqual(get_skip_next(reviewers[0]), 0) # skipped over with this assignment
self.assertEqual(get_skip_next(reviewers[1]), 0) # skipped over with this assignment
self.assertEqual(get_skip_next(reviewers[2]), 0)
self.assertEqual(get_skip_next(reviewers[3]), 1)
self.assertEqual(get_skip_next(reviewers[4]), 0)
# Leave only a single reviewer remaining, which should not trigger an infinite loop.
# The deletion also causes NextReviewerInTeam to be deleted.
[reviewer.delete() for reviewer in reviewers[1:]]
self.assertEqual([reviewers[0]], self.policy.default_reviewer_rotation_list())
self.policy.update_policy_state_for_assignment(review_req, assignee_person=reviewers[0], add_skip=False)
# No NextReviewerInTeam should be created, the only possible next is the excluded assignee.
self.assertFalse(NextReviewerInTeam.objects.filter(team=self.team))
self.assertEqual([reviewers[0]], self.policy.default_reviewer_rotation_list())
def test_assign_reviewer_updates_skip_next_without_add_skip(self):
"""Skipping reviewers with add_skip=False should update skip_counts properly"""
review_req = ReviewRequestFactory(team=self.team)
reviewer_to_skip = self.append_reviewer(skip_count=1)
reviewer_to_assign = self.append_reviewer(skip_count=0)
reviewer_to_skip_later = self.append_reviewer(skip_count=1)
# Check test assumptions
self.assertEqual(
self.policy.default_reviewer_rotation_list(),
[reviewer_to_skip, reviewer_to_assign, reviewer_to_skip_later],
)
self.policy.assign_reviewer(review_req, reviewer_to_assign.email(), add_skip=False)
# Check results
self.assertEqual(self.reviewer_settings_for(reviewer_to_skip).skip_next, 0,
'skip_next not updated for skipped reviewer')
self.assertEqual(self.reviewer_settings_for(reviewer_to_assign).skip_next, 0,
'skip_next changed unexpectedly for assigned reviewer')
# Expect to skip the later reviewer when updating NextReviewerInTeam
self.assertEqual(self.reviewer_settings_for(reviewer_to_skip_later).skip_next, 0,
'skip_next not updated for reviewer to skip later')
def test_assign_reviewer_updates_skip_next_with_add_skip(self):
"""Skipping reviewers with add_skip=True should update skip_counts properly"""
review_req = ReviewRequestFactory(team=self.team)
reviewer_to_skip = self.append_reviewer(skip_count=1)
reviewer_to_assign = self.append_reviewer(skip_count=0)
reviewer_to_skip_later = self.append_reviewer(skip_count=1)
# Check test assumptions
self.assertEqual(
self.policy.default_reviewer_rotation_list(),
[reviewer_to_skip, reviewer_to_assign, reviewer_to_skip_later],
)
self.policy.assign_reviewer(review_req, reviewer_to_assign.email(), add_skip=True)
# Check results
self.assertEqual(self.reviewer_settings_for(reviewer_to_skip).skip_next, 0,
'skip_next not updated for skipped reviewer')
self.assertEqual(self.reviewer_settings_for(reviewer_to_assign).skip_next, 1,
'skip_next not updated for assigned reviewer')
# Expect to skip the later reviewer when updating NextReviewerInTeam
self.assertEqual(self.reviewer_settings_for(reviewer_to_skip_later).skip_next, 0,
'skip_next not updated for reviewer to skip later')
class LeastRecentlyUsedReviewerQueuePolicyTest(_Wrapper.ReviewerQueuePolicyTestCase):
"""
These tests only cover where this policy deviates from
RotateAlphabeticallyReviewerQueuePolicy - the common behaviour
inherited from AbstractReviewerQueuePolicy is covered in
RotateAlphabeticallyReviewerQueuePolicyTest.
"""
reviewer_queue_policy_id = 'LeastRecentlyUsed'
def setUp(self):
super(LeastRecentlyUsedReviewerQueuePolicyTest, self).setUp()
self.last_assigned_on = timezone.now() - datetime.timedelta(days=365)
def append_reviewer(self, skip_count=None):
"""Create a reviewer who will appear in the assignee options list
New reviewer will be last in the default reviewer rotation list.
"""
reviewer = super(LeastRecentlyUsedReviewerQueuePolicyTest, self).append_reviewer(skip_count)
self.create_reviewer_assignment(reviewer)
return reviewer
def create_reviewer_assignment(self, reviewer):
"""Assign reviewer as most recent assignee
Calling this will move a reviewer to the end of the LRU order.
"""
if self.last_assigned_on is None:
assignment = ReviewAssignmentFactory(review_request__team=self.team, reviewer=reviewer.email())
else:
assignment = ReviewAssignmentFactory(
review_request__team=self.team,
reviewer=reviewer.email(),
assigned_on=self.last_assigned_on + datetime.timedelta(days=1)
)
self.last_assigned_on = assignment.assigned_on
return assignment
def create_old_review_assignment(self, reviewer, **kwargs):
"""Create a review that won't disturb the ordering of reviewers"""
# Make a review older than our oldest review
assert('assigned_on' not in kwargs)
kwargs['assigned_on'] = timezone.now() - datetime.timedelta(days=400)
return super(LeastRecentlyUsedReviewerQueuePolicyTest, self).create_old_review_assignment(reviewer, **kwargs)
def test_default_reviewer_rotation_list_uses_latest_assignment(self):
available_reviewers, _ = self.set_up_default_reviewer_rotation_list_test()
assert(len(available_reviewers) > 2) # need enough to avoid wrapping around in a way that invalidates tests
# Give the first reviewer, who would normally appear at the top of the list, a newer assignment
first_reviewer = available_reviewers[0]
self.create_reviewer_assignment(first_reviewer) # creates a new assignment, later assigned_on than all others
self.assertEqual(self.policy.default_reviewer_rotation_list(), available_reviewers[1:] + [first_reviewer])
def test_default_reviewer_rotation_list_ignores_rejected(self):
available_reviewers, _ = self.set_up_default_reviewer_rotation_list_test()
assert(len(available_reviewers) > 2) # need enough to avoid wrapping around in a way that invalidates tests
first_reviewer = available_reviewers[0]
rejected_assignment = self.create_reviewer_assignment(first_reviewer) # assigned_on later than all others...
rejected_assignment.state_id = 'rejected' #... but marked as rejected
rejected_assignment.save()
self.assertEqual(self.policy.default_reviewer_rotation_list(), available_reviewers) # order unchanged
def test_default_review_rotation_list_uses_assigned_on_date(self):
available_reviewers, _ = self.set_up_default_reviewer_rotation_list_test()
assert(len(available_reviewers) > 2) # need enough to avoid wrapping around in a way that invalidates tests
first_reviewer, second_reviewer = available_reviewers[:2]
completed_assignment = self.create_reviewer_assignment(first_reviewer) # moves to the end...
second_reviewer_assignment = self.create_reviewer_assignment(second_reviewer) # moves to the end...
# Mark first_reviewer's assignment as completed after second_reviewer's was assigned
completed_assignment.state_id = 'completed'
completed_assignment.completed_on = second_reviewer_assignment.assigned_on + datetime.timedelta(days=1)
completed_assignment.save()
# The completed_on timestamp should not have changed the order - second_reviewer still at the end
self.assertEqual(self.policy.default_reviewer_rotation_list(),
available_reviewers[2:] + [first_reviewer, second_reviewer])
def test_set_wants_to_be_next(self):
reviewer = self.append_reviewer()
self.policy.set_wants_to_be_next(reviewer)
self.assertTrue(self.reviewer_settings_for(reviewer).request_assignment_next)
def test_assign_reviewer_updates_skip_next_without_add_skip(self):
"""Skipping reviewers with add_skip=False should update skip_counts properly"""
review_req = ReviewRequestFactory(team=self.team)
reviewer_to_skip = self.append_reviewer(skip_count=1)
reviewer_to_assign = self.append_reviewer(skip_count=0)
reviewer_to_skip_later = self.append_reviewer(skip_count=1)
# Check test assumptions
self.assertEqual(
self.policy.default_reviewer_rotation_list(),
[reviewer_to_skip, reviewer_to_assign, reviewer_to_skip_later],
)
self.policy.assign_reviewer(review_req, reviewer_to_assign.email(), add_skip=False)
# Check results
self.assertEqual(self.reviewer_settings_for(reviewer_to_skip).skip_next, 0,
'skip_next not updated for skipped reviewer')
self.assertEqual(self.reviewer_settings_for(reviewer_to_assign).skip_next, 0,
'skip_next changed unexpectedly for assigned reviewer')
self.assertEqual(self.reviewer_settings_for(reviewer_to_skip_later).skip_next, 1,
'skip_next changed unexpectedly for reviewer to skip later')
def test_assign_reviewer_updates_skip_next_with_add_skip(self):
"""Skipping reviewers with add_skip=True should update skip_counts properly"""
review_req = ReviewRequestFactory(team=self.team)
reviewer_to_skip = self.append_reviewer(skip_count=1)
reviewer_to_assign = self.append_reviewer(skip_count=0)
reviewer_to_skip_later = self.append_reviewer(skip_count=1)
# Check test assumptions
self.assertEqual(
self.policy.default_reviewer_rotation_list(),
[reviewer_to_skip, reviewer_to_assign, reviewer_to_skip_later],
)
self.policy.assign_reviewer(review_req, reviewer_to_assign.email(), add_skip=True)
# Check results
self.assertEqual(self.reviewer_settings_for(reviewer_to_skip).skip_next, 0,
'skip_next not updated for skipped reviewer')
self.assertEqual(self.reviewer_settings_for(reviewer_to_assign).skip_next, 1,
'skip_next not updated for assigned reviewer')
self.assertEqual(self.reviewer_settings_for(reviewer_to_skip_later).skip_next, 1,
'skip_next changed unexpectedly for reviewer to skip later')
class AssignmentOrderResolverTests(TestCase):
def test_determine_ranking(self):
# reviewer_high is second in the default rotation, reviewer_low is first
# however, reviewer_high hits every score increase, reviewer_low hits every score decrease
team = ReviewTeamFactory(acronym="rotationteam", name="Review Team", list_email="rotationteam@ietf.org", parent=Group.objects.get(acronym="farfut"))
reviewer_high = create_person(team, "reviewer", name="Test Reviewer-high", username="testreviewerhigh")
reviewer_low = create_person(team, "reviewer", name="Test Reviewer-low", username="testreviewerlow")
# This reviewer should be entirely ignored because it is not in the rotation list.
create_person(team, "reviewer", name="Test Reviewer-out-of-rotation", username="testreviewer-out-of-rotation")
# Create a document with ancestors, that also triggers author check, AD check and group check
doc_individual = IndividualDraftFactory()
doc_wg = WgDraftFactory(relations=[('replaces', doc_individual)])
doc_middle_wg = WgDraftFactory(relations=[('replaces', doc_wg)])
doc = WgDraftFactory(group__acronym='mars', rev='01', authors=[reviewer_low], ad=reviewer_low, shepherd=reviewer_low.email(), relations=[('replaces', doc_middle_wg)])
Role.objects.create(group=doc.group, person=reviewer_low, email=reviewer_low.email(), name_id='advisor')
# Trigger previous review check (including finding ancestor documents) and completed review stats.
ReviewAssignmentFactory(review_request__team=team, review_request__doc=doc_individual, reviewer=reviewer_high.email(), state_id='completed')
# Trigger other review stats
ReviewAssignmentFactory(review_request__team=team, review_request__doc=doc, reviewer=reviewer_high.email(), state_id='no-response')
ReviewAssignmentFactory(review_request__team=team, review_request__doc=doc, reviewer=reviewer_high.email(), state_id='part-completed')
# Trigger review wish check
ReviewWish.objects.create(team=team, doc=doc, person=reviewer_high)
# This period should not have an impact, because it is the canfinish type,
# and this reviewer has reviewed previously.
UnavailablePeriod.objects.create(
team=team,
person=reviewer_high,
start_date='2000-01-01',
availability='canfinish',
)
# Trigger "reviewer has rejected before"
ReviewAssignmentFactory(review_request__team=team, review_request__doc=doc, reviewer=reviewer_low.email(), state_id='rejected')
# Trigger max frequency and open review stats
ReviewAssignmentFactory(review_request__team=team, reviewer=reviewer_low.email(), state_id='assigned', review_request__doc__pages=10)
# Trigger skip_next, max frequency, filter_re
ReviewerSettings.objects.create(
team=team,
person=reviewer_low,
filter_re='.*draft.*',
skip_next=2,
min_interval=91,
)
# Trigger "assign me next"
ReviewerSettings.objects.create(
team=team,
person=reviewer_high,
request_assignment_next=True,
)
review_req = ReviewRequestFactory(doc=doc, team=team, type_id='early')
rotation_list = [reviewer_low, reviewer_high]
order = AssignmentOrderResolver(Email.objects.all(), review_req, rotation_list)
ranking = order.determine_ranking()
self.assertEqual(len(ranking), 2)
self.assertEqual(ranking[0]['email'], reviewer_high.email())
self.assertEqual(ranking[1]['email'], reviewer_low.email())
# These scores follow the ordering of https://github.com/ietf-tools/datatracker/wiki/ReviewerQueuePolicy,
self.assertEqual(ranking[0]['scores'], [ 1, 1, 1, 1, 1, 1, 0, 0, -1])
self.assertEqual(ranking[1]['scores'], [-1, -1, -1, -1, -1, -1, -91, -2, 0])
self.assertEqual(ranking[0]['label'], 'Test Reviewer-high: unavailable indefinitely (Can do follow-ups); requested to be selected next for assignment; reviewed document before; wishes to review document; #2; 1 no response, 1 partially complete, 1 fully completed')
self.assertEqual(ranking[1]['label'], 'Test Reviewer-low: rejected review of document before; is author of document; filter regexp matches; max frequency exceeded, ready in 91 days; skip next 2; #1; currently 1 open, 10 pages')