From 8ebef270268ebe878cccdd4e79e094a4a3686755 Mon Sep 17 00:00:00 2001
From: Henrik Levkowetz <henrik@levkowetz.com>
Date: Tue, 1 Dec 2020 15:36:04 +0000
Subject: [PATCH] Related to issue #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.  - Legacy-Id: 18722

---
 ietf/review/models.py | 24 +++++++++++++++++-------
 ietf/review/tests.py  | 14 +++++++++++---
 2 files changed, 28 insertions(+), 10 deletions(-)

diff --git a/ietf/review/models.py b/ietf/review/models.py
index 95d89a740..a33bda065 100644
--- a/ietf/review/models.py
+++ b/ietf/review/models.py
@@ -8,6 +8,8 @@ from simple_history.models import HistoricalRecords
 
 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 @@ class ReviewAssignment(models.Model):
         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
index bb8cea89e..a1cd017e7 100644
--- a/ietf/review/tests.py
+++ b/ietf/review/tests.py
@@ -24,15 +24,23 @@ class HashTest(TestCase):
             
 
 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