fix: review refactoring issue

fixes #5447

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
 

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 = {

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):