Add tests of suggested review requests, make suggestions take more

things into account
 - Legacy-Id: 11528

Author: OleLaursen

ietf/group/tests_review.py

@@ -6,11 +6,73 @@
 
 from ietf.utils.test_data import make_test_data, make_review_data
 from ietf.utils.test_utils import login_testing_unauthorized, TestCase, unicontent, reload_db_objects
-from ietf.review.models import ReviewRequest
+from ietf.review.models import ReviewRequest, ReviewRequestStateName
+from ietf.doc.models import TelechatDocEvent
+from ietf.iesg.models import TelechatDate
 from ietf.person.models import Email, Person
+from ietf.review.utils import suggested_review_requests_for_team
 import ietf.group.views_review
 
 class ReviewTests(TestCase):
+    def test_suggested_review_requests(self):
+        doc = make_test_data()
+        review_req = make_review_data(doc)
+        team = review_req.team
+
+        # put on telechat
+        TelechatDocEvent.objects.create(
+            type="scheduled_for_telechat",
+            by=Person.objects.get(name="(System)"),
+            doc=doc,
+            telechat_date=TelechatDate.objects.all().first().date,
+        )
+        doc.rev = "10"
+        doc.save()
+
+        prev_rev = "{:02}".format(int(doc.rev) - 1)
+
+        # blocked by existing request
+        review_req.requested_rev = ""
+        review_req.save()
+
+        self.assertEqual(len(suggested_review_requests_for_team(team)), 0)
+
+        # ... but not to previous version
+        review_req.requested_rev = prev_rev
+        review_req.save()
+        suggestions = suggested_review_requests_for_team(team)
+        self.assertEqual(len(suggestions), 1)
+        self.assertEqual(suggestions[0].doc, doc)
+        self.assertEqual(suggestions[0].team, team)
+
+        # blocked by non-versioned refusal
+        review_req.requested_rev = ""
+        review_req.state = ReviewRequestStateName.objects.get(slug="no-review-document")
+        review_req.save()
+
+        self.assertEqual(list(suggested_review_requests_for_team(team)), [])
+
+        # blocked by versioned refusal
+        review_req.reviewed_rev = doc.rev
+        review_req.state = ReviewRequestStateName.objects.get(slug="no-review-document")
+        review_req.save()
+
+        self.assertEqual(list(suggested_review_requests_for_team(team)), [])
+
+        # blocked by completion
+        review_req.state = ReviewRequestStateName.objects.get(slug="completed")
+        review_req.save()
+
+        self.assertEqual(list(suggested_review_requests_for_team(team)), [])
+
+        # ... but not to previous version
+        review_req.reviewed_rev = prev_rev
+        review_req.state = ReviewRequestStateName.objects.get(slug="completed")
+        review_req.save()
+
+        self.assertEqual(len(suggested_review_requests_for_team(team)), 1)
+        
+
     def test_manage_review_requests(self):
         doc = make_test_data()
         review_req1 = make_review_data(doc)

ietf/group/views_review.py

@@ -3,7 +3,7 @@
 from django.contrib.auth.decorators import login_required
 from django import forms
 
-from ietf.review.models import ReviewRequest, ReviewRequestStateName
+from ietf.review.models import ReviewRequest
 from ietf.review.utils import (can_manage_review_requests_for_team, close_review_request_states,
                                extract_revision_ordered_review_requests_for_documents,
                                assign_review_request_to_reviewer,
@@ -62,11 +62,8 @@ def __init__(self, review_req, *args, **kwargs):
         self.fields["reviewer"].widget.attrs["class"] = "form-control input-sm"
 
         if self.is_bound:
-            action = self.data.get("action")
-            if action == "close":
+            if self.data.get("action") == "close":
                 self.fields["close"].required = True
-            elif action == "assign":
-                self.fields["reviewer"].required = True
 
 
 @login_required

ietf/name/fixtures/names.json

@@ -1827,6 +1827,26 @@
  "fields": {
   "order": 7,
   "used": true,
+  "name": "No Review of Version",
+  "desc": ""
+ },
+ "model": "name.reviewrequeststatename",
+ "pk": "no-review-version"
+},
+{
+ "fields": {
+  "order": 8,
+  "used": true,
+  "name": "No Review of Document",
+  "desc": ""
+ },
+ "model": "name.reviewrequeststatename",
+ "pk": "no-review-document"
+},
+{
+ "fields": {
+  "order": 9,
+  "used": true,
   "name": "Partially Completed",
   "desc": ""
  },
@@ -1835,7 +1855,7 @@
 },
 {
  "fields": {
-  "order": 8,
+  "order": 10,
   "used": true,
   "name": "Completed",
   "desc": ""

ietf/review/utils.py

@@ -153,6 +153,8 @@ def fixup_deadline(d):
 
     requests = {}
 
+    requested_state = ReviewRequestStateName.objects.get(slug="requested", used=True)
+
     if True: # FIXME
         # in Last Call
         last_call_type = ReviewTypeName.objects.get(slug="lc")
@@ -171,6 +173,7 @@ def fixup_deadline(d):
                 team=team,
                 deadline=deadline,
                 requested_by=system_person,
+                state=requested_state,
             )
 
             seen_deadlines[doc.pk] = deadline
@@ -200,6 +203,7 @@ def fixup_deadline(d):
                 team=team,
                 deadline=deadline,
                 requested_by=system_person,
+                state=requested_state,
             )
 
             seen_deadlines[doc.pk] = deadline
@@ -210,11 +214,19 @@ def fixup_deadline(d):
         existing_requests[r.doc_id].append(r)
 
     def blocks(existing, request):
-        return (existing.doc_id == request.doc_id
-                and existing.reviewed_rev == request.doc.rev
-                and existing.state_id not in ("part-completed", "rejected", "overtaken"))
+        if existing.doc_id != request.doc_id:
+            return False
+
+        no_review_document = existing.state_id == "no-review-document"
+        pending = (existing.state_id in ("requested", "accepted")
+                   and (not existing.requested_rev or existing.requested_rev == request.doc.rev))
+        completed_or_closed = (existing.state_id not in ("part-completed", "rejected", "overtaken", "no-response")
+                               and existing.reviewed_rev == request.doc.rev)
+
+        return no_review_document or pending or completed_or_closed
 
-    res = [r for r in requests.itervalues() if not any(blocks(e, r) for e in existing_requests[r.doc_id])]
+    res = [r for r in requests.itervalues()
+           if not any(blocks(e, r) for e in existing_requests[r.doc_id])]
     res.sort(key=lambda r: (r.deadline, r.doc_id))
     return res