Related to issue ietf-tools#2186: The issue asked for changing review requests from state 'assigned' back to state 'requested' if the last associated assignment is withdrawn. However, the code to implement this makes the change for all assignment states, and also when there has been no state change. Changed to be more discerning.

levkowetz · levkowetz · commit 8ebef270268e · 2020-12-01T15:36:04.000Z
- Legacy-Id: 18722
diff --git a/ietf/review/models.py b/ietf/review/models.py
@@ -8,6 +8,8 @@
 
 from django.db import models
 
+import debug                            # pyflakes:ignore
+
 from ietf.doc.models import Document
 from ietf.group.models import Group
 from ietf.person.models import Person, Email
@@ -152,6 +154,12 @@ class ReviewAssignment(models.Model):
     result         = ForeignKey(ReviewResultName, blank=True, null=True)
     mailarch_url   = models.URLField(blank=True, null = True)
 
+    _original_state = None
+
+    def __init__(self, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+        self._original_state = self.state_id
+
     def __str__(self):
         return "Assignment for %s (%s) : %s %s of %s" % (self.reviewer.person, self.state, self.review_request.team.acronym, self.review_request.type, self.review_request.doc)
 
@@ -161,13 +169,15 @@ def save(self, *args, **kwargs):
         If the review request has no other active or completed reviews, the review request
         needs to be treated as an unassigned request, as it will need a new reviewer.
         """
-        super(ReviewAssignment, self).save(*args, **kwargs)
-        active_states = ['assigned', 'accepted', 'completed']
-        review_req_has_active_assignments = self.review_request.reviewassignment_set.filter(state__in=active_states)
-        if self.review_request.state_id == 'assigned' and not review_req_has_active_assignments:
-            self.review_request.state_id = 'requested'
-            self.review_request.save()
-            
+        super().save(*args, **kwargs)
+        if self._original_state != self.state_id:
+            if self.state_id in ['withdrawn', 'rejected', 'no-response', 'overtaken']:
+                active_req_states = ['assigned', 'accepted', 'completed']
+                review_req_has_active_assignments = self.review_request.reviewassignment_set.filter(state__in=active_req_states)
+                if self.review_request.state_id == 'assigned' and not review_req_has_active_assignments:
+                    self.review_request.state_id = 'requested'
+                    self.review_request.save()
+            self._original_state = self.state_id
 
 def get_default_review_types():
     return ReviewTypeName.objects.filter(slug__in=['early','lc','telechat'])
diff --git a/ietf/review/tests.py b/ietf/review/tests.py
@@ -24,15 +24,23 @@ def test_hash_list_message_id(self):
             
 
 class ReviewAssignmentTest(TestCase):
-    def test_update_review_req_status(self):
+    def do_test_update_review_req_status(self, assignment_state, expected_state):
         review_req = ReviewRequestFactory(state_id='assigned')
         ReviewAssignmentFactory(review_request=review_req, state_id='part-completed')
         assignment = ReviewAssignmentFactory(review_request=review_req)
 
-        assignment.state_id = 'no-response'
+        assignment.state_id = assignment_state
         assignment.save()
         review_req = reload_db_objects(review_req)
-        self.assertEqual(review_req.state_id, 'requested')
+        self.assertEqual(review_req.state_id, expected_state)
+
+    def test_update_review_req_status(self):
+        # Test change
+        for assignment_state in ['no-response', 'rejected', 'withdrawn', 'overtaken']:
+            self.do_test_update_review_req_status(assignment_state, 'requested')
+        # Test no-change
+        for assignment_state in ['accepted', 'assigned', 'completed', 'part-completed', 'unknown', ]:
+            self.do_test_update_review_req_status('', 'assigned')
 
     def test_no_update_review_req_status_when_other_active_assignment(self):
         # If there is another still active assignment, do not update review_req state
