From fdb4c2a0559c30acd4182cf236f9527b43c3f7a1 Mon Sep 17 00:00:00 2001 From: Sasha Romijn Date: Thu, 3 Oct 2019 09:02:34 +0000 Subject: [PATCH] Fix #2186 - Return review req to 'requested' status if no review assignments are open. If a review assignment is rejected, withdrawn, marked no response, etc., and this leaves a review request without any assigned/accepted/completed review assignments, return the request state to "requested", which means it will be shown as an unassigned review in all interfaces. Commit ready for merge. - Legacy-Id: 16819 --- ietf/group/views.py | 2 +- ietf/review/models.py | 13 +++++++++++++ ietf/review/tests.py | 36 +++++++++++++++++++++++++++++++++++- 3 files changed, 49 insertions(+), 2 deletions(-) diff --git a/ietf/group/views.py b/ietf/group/views.py index d3dcef1a4..a39a9fa1f 100644 --- a/ietf/group/views.py +++ b/ietf/group/views.py @@ -1290,7 +1290,7 @@ def get_open_review_requests_for_team(team, assignment_status=None): Q(state_id='requested') | Q(state_id='assigned',reviewassignment__state__in=('assigned','accepted')) ).prefetch_related( "type", "state", "doc", "doc__states", - ).order_by("-time", "-id") + ).order_by("-time", "-id").distinct() if assignment_status == "unassigned": open_review_requests = suggested_review_requests_for_team(team) + list(open_review_requests.filter(state_id='requested')) diff --git a/ietf/review/models.py b/ietf/review/models.py index a5948c776..220ee3876 100644 --- a/ietf/review/models.py +++ b/ietf/review/models.py @@ -158,6 +158,19 @@ class ReviewAssignment(models.Model): 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) + def save(self, *args, **kwargs): + """ + Save the assignment, and check whether the review request status needs to be updated. + 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() + 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 5d599669a..f46e44bf9 100644 --- a/ietf/review/tests.py +++ b/ietf/review/tests.py @@ -4,7 +4,8 @@ from __future__ import absolute_import, print_function, unicode_literals -from ietf.utils.test_utils import TestCase +from ietf.review.factories import ReviewAssignmentFactory, ReviewRequestFactory +from ietf.utils.test_utils import TestCase, reload_db_objects from .mailarch import hash_list_message_id class HashTest(TestCase): @@ -23,3 +24,36 @@ class HashTest(TestCase): ): self.assertEqual(hash, hash_list_message_id(list, msgid)) + +class ReviewAssignmentTest(TestCase): + def test_update_review_req_status(self): + 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.save() + review_req = reload_db_objects(review_req) + self.assertEqual(review_req.state_id, 'requested') + + 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 + review_req = ReviewRequestFactory(state_id='assigned') + ReviewAssignmentFactory(review_request=review_req, state_id='assigned') + assignment = ReviewAssignmentFactory(review_request=review_req) + + assignment.state_id = 'no-response' + assignment.save() + review_req = reload_db_objects(review_req) + self.assertEqual(review_req.state_id, 'assigned') + + def test_no_update_review_req_status_when_review_req_withdrawn(self): + # review_req state must only be changed to "requested", if old state was "assigned", + # to prevent reviving dead review requests + review_req = ReviewRequestFactory(state_id='withdrawn') + assignment = ReviewAssignmentFactory(review_request=review_req) + + assignment.state_id = 'no-response' + assignment.save() + review_req = reload_db_objects(review_req) + self.assertEqual(review_req.state_id, 'withdrawn')