diff --git a/ietf/doc/views_review.py b/ietf/doc/views_review.py index fa6e3a7ffe..f758032f95 100644 --- a/ietf/doc/views_review.py +++ b/ietf/doc/views_review.py @@ -389,9 +389,6 @@ def reject_reviewer_assignment(request, name, assignment_id): state=review_assignment.state, ) - policy = get_reviewer_queue_policy(review_assignment.review_request.team) - policy.return_reviewer_to_rotation_top(review_assignment.reviewer.person, form.cleaned_data['wants_to_be_next']) - msg = render_to_string("review/reviewer_assignment_rejected.txt", { "by": request.user.person, "message_to_secretary": form.cleaned_data.get("message_to_secretary"), @@ -441,7 +438,7 @@ def withdraw_reviewer_assignment(request, name, assignment_id): ) policy = get_reviewer_queue_policy(review_assignment.review_request.team) - policy.return_reviewer_to_rotation_top(review_assignment.reviewer.person, True) + policy.set_wants_to_be_next(review_assignment.reviewer.person) msg = "Review assignment withdrawn by %s"%request.user.person diff --git a/ietf/review/policies.py b/ietf/review/policies.py index fe6519a5ef..ac3cca2835 100644 --- a/ietf/review/policies.py +++ b/ietf/review/policies.py @@ -84,7 +84,7 @@ def default_reviewer_rotation_list(self, include_unavailable=False): rotation_list = self._filter_unavailable_reviewers(rotation_list) return rotation_list - def return_reviewer_to_rotation_top(self, reviewer_person, wants_to_be_next): + def set_wants_to_be_next(self, reviewer_person): """ Return a reviewer to the top of the rotation, e.g. because they rejected a review, and should retroactively not have been rotated over. @@ -475,14 +475,13 @@ def default_reviewer_rotation_list(self, include_unavailable=False): return reviewers[next_reviewer_index:] + reviewers[:next_reviewer_index] - def return_reviewer_to_rotation_top(self, reviewer_person, wants_to_be_next): + def set_wants_to_be_next(self, reviewer_person): # As RotateAlphabetically does not keep a full rotation list, # returning someone to a particular order is complex. # Instead, the "assign me next" flag is set. - if wants_to_be_next: - settings = self._reviewer_settings_for(reviewer_person) - settings.request_assignment_next = wants_to_be_next - settings.save() + settings = self._reviewer_settings_for(reviewer_person) + settings.request_assignment_next = True + settings.save() def _update_skip_next(self, rotation_pks, assignee_person): """Decrement skip_next for all users skipped @@ -570,14 +569,13 @@ def default_reviewer_rotation_list(self, include_unavailable=False): rotation_list += reviewers_with_assignment return rotation_list - def return_reviewer_to_rotation_top(self, reviewer_person, wants_to_be_next): + def set_wants_to_be_next(self, reviewer_person): # Reviewer rotation for this policy ignores rejected/withdrawn # reviews, so it automatically adjusts the position of someone # who rejected a review and no further action is needed. - if wants_to_be_next: - settings = self._reviewer_settings_for(reviewer_person) - settings.request_assignment_next = wants_to_be_next - settings.save() + settings = self._reviewer_settings_for(reviewer_person) + settings.request_assignment_next = True + settings.save() QUEUE_POLICY_NAME_MAPPING = { diff --git a/ietf/review/tests_policies.py b/ietf/review/tests_policies.py index d1d26997b4..ca687ff862 100644 --- a/ietf/review/tests_policies.py +++ b/ietf/review/tests_policies.py @@ -115,7 +115,7 @@ 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_return_reviewer_to_rotation_top(self): + def test_set_wants_to_be_next(self): # Subclass must implement this raise NotImplementedError @@ -507,11 +507,9 @@ def test_default_reviewer_rotation_list_with_nextreviewerinteam(self): rotation = self.policy.default_reviewer_rotation_list() self.assertEqual(rotation, available_reviewers[2:] + available_reviewers[:1]) - def test_return_reviewer_to_rotation_top(self): + def test_set_wants_to_be_next(self): reviewer = self.append_reviewer() - self.policy.return_reviewer_to_rotation_top(reviewer, False) - self.assertFalse(self.reviewer_settings_for(reviewer).request_assignment_next) - self.policy.return_reviewer_to_rotation_top(reviewer, True) + 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): @@ -725,11 +723,9 @@ def test_default_review_rotation_list_uses_assigned_on_date(self): self.assertEqual(self.policy.default_reviewer_rotation_list(), available_reviewers[2:] + [first_reviewer, second_reviewer]) - def test_return_reviewer_to_rotation_top(self): + def test_set_wants_to_be_next(self): reviewer = self.append_reviewer() - self.policy.return_reviewer_to_rotation_top(reviewer, False) - self.assertFalse(self.reviewer_settings_for(reviewer).request_assignment_next) - self.policy.return_reviewer_to_rotation_top(reviewer, True) + 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):