Improved several views.

 - Legacy-Id: 16038

Author: rjsparks

ietf/doc/factories.py

@@ -209,6 +209,12 @@ def states(obj, create, extracted, **kwargs):
         else:
             obj.set_state(State.objects.get(type_id='conflrev',slug='iesgeval'))
 
+# TODO: This is too skeletal - improve it with, at least, a group generator that backs the object with a review team.
+class ReviewFactory(BaseDocumentFactory):
+    type_id = 'review'
+    name = factory.LazyAttribute(lambda o: 'review-doesnotexist-00-%s-%s'%(o.group.acronym,datetime.date.today().isoformat()))
+    group = factory.SubFactory('ietf.group.factories.GroupFactory',type_id='review')
+
 class DocAliasFactory(factory.DjangoModelFactory):
     class Meta:
         model = DocAlias

ietf/doc/tests_review.py

@@ -17,7 +17,7 @@
 import debug                            # pyflakes:ignore
 
 import ietf.review.mailarch
-from ietf.doc.factories import NewRevisionDocEventFactory, WgDraftFactory, WgRfcFactory
+from ietf.doc.factories import NewRevisionDocEventFactory, WgDraftFactory, WgRfcFactory, ReviewFactory
 from ietf.doc.models import DocumentAuthor, RelatedDocument, DocEvent, ReviewAssignmentDocEvent
 from ietf.group.factories import RoleFactory, ReviewTeamFactory
 from ietf.group.models import Group
@@ -335,6 +335,7 @@ def test_assign_reviewer(self):
             result_id='serious-issues',
             reviewer=reviewer_email,
             reviewed_rev="01",
+            review = ReviewFactory(),
             assigned_on=req.time,
             completed_on=req.time + datetime.timedelta(days=10),
         )

ietf/doc/views_review.py

@@ -360,6 +360,7 @@ def reject_reviewer_assignment(request, name, assignment_id):
     return render(request, 'doc/review/reject_reviewer_assignment.html', {
         'doc': doc,
         'review_req': review_assignment.review_request,
+        'assignments': review_assignment.review_request.reviewassignment_set.all(),
         'form': form,
     })
 

ietf/group/tests_review.py

@@ -100,6 +100,7 @@ def test_suggested_review_requests(self):
         review_req.state = ReviewRequestStateName.objects.get(slug="assigned")
         review_req.save()
         assignment.state = ReviewAssignmentStateName.objects.get(slug="completed")
+        assignment.reviewed_rev = review_req.doc.rev
         assignment.save()
 
         self.assertEqual(list(suggested_review_requests_for_team(team)), [])

ietf/review/models.py

@@ -136,11 +136,8 @@ class ReviewRequest(models.Model):
     def __unicode__(self):
         return u"%s review on %s by %s %s" % (self.type, self.doc, self.team, self.state)
 
-    def other_requests(self):
-        return self.doc.reviewrequest_set.exclude(id=self.id)
-
-    def other_completed_requests(self):
-        return self.other_requests().filter(state_id__in=['completed','part-completed'])
+    def all_completed_assignments_for_doc(self):
+        return ReviewAssignment.objects.filter(review_request__doc=self.doc, state__in=['completed','part-completed'])
 
     def request_closed_time(self):
         return self.doc.request_closed_time(self) or self.time

ietf/review/utils.py

@@ -450,10 +450,11 @@ def assign_review_request_to_reviewer(request, review_req, reviewer, add_skip=Fa
 
     # Note that assigning a review no longer unassigns other reviews
 
-    review_req.reviewassignment_set.create(state_id='assigned', reviewer = reviewer, assigned_on = datetime.datetime.now())
     if review_req.state_id != 'assigned':
         review_req.state_id = 'assigned'
         review_req.save()
+        
+    review_req.reviewassignment_set.create(state_id='assigned', reviewer = reviewer, assigned_on = datetime.datetime.now())
 
     if reviewer:
         possibly_advance_next_reviewer_for_team(review_req.team, reviewer.person_id, add_skip)
@@ -543,8 +544,9 @@ def close_review_request(request, review_req, close_state):
     suggested_req = review_req.pk is None
 
     review_req.state = close_state
-    if close_state.slug == "no-review-version":
-        review_req.reviewed_rev = review_req.requested_rev or review_req.doc.rev # save rev for later reference
+# This field no longer exists, and it's not clear what the later reference was...
+#    if close_state.slug == "no-review-version":
+#        review_req.reviewed_rev = review_req.requested_rev or review_req.doc.rev # save rev for later reference
     review_req.save()
 
     if not suggested_req:
@@ -681,8 +683,8 @@ def blocks(existing, request):
                    and existing.reviewassignment_set.filter(state_id__in=("assigned", "accepted")).exists()
                    and (not existing.requested_rev or existing.requested_rev == request.doc.rev))
         request_closed = existing.state_id not in ('requested','assigned')
-        # at least one assignment was completed for the requested version:
-        some_assignment_completed = existing.reviewassignment_set.filter(reviewed_rev=existing.requested_rev,state_id='completed').exists()
+        # at least one assignment was completed for the requested version or the current doc version if no specific version was requested:
+        some_assignment_completed = existing.reviewassignment_set.filter(reviewed_rev=existing.requested_rev or existing.doc.rev, state_id='completed').exists()
 
         return any([no_review_document, no_review_rev, pending, request_closed, some_assignment_completed])
 

ietf/templates/doc/review/reject_reviewer_assignment.html

@@ -10,7 +10,7 @@ <h1>Reject review assignment<br><small>{{ review_req.doc.name }}</small></h1>
 
   {% include "doc/review/request_info.html" %}
 
-  <p>{{ review_req.reviewer.person }} is currently assigned to do the review. Do you want to reject this assignment?</p>
+  <p>Do you want to reject this assignment?</p>
 
   <form method="post">
     {% csrf_token %}

ietf/templates/doc/review/request_info.html

@@ -78,12 +78,10 @@
 
       <tr>
         <th></th>
-        <th>Reviews from other teams</th>
+        <th>Completed reviews</th>
         <td>
-          {% for req in review_req.other_completed_requests %} {# TODO: align this with new models #}
-            {% if req.reviewer == review_req.reviewer %}<strong>{% endif %}
-            <a href="{% url "ietf.doc.views_review.review_request" name=req.doc request_id=req.id %}">{{req.team.acronym|capfirst}} {{req.type.name}} review of -{{req.reviewed_rev}} by {{req.reviewer.person.plain_name}}</a> {% if req.reviewed_rev != req.doc.rev %} (<a href="{{ rfcdiff_base_url }}?url1={{ req.doc.name }}-{{ req.reviewed_rev }}&amp;url2={{ req.doc.name }}-{{ req.doc.rev }}">diff</a>){% endif %}<br>
-            {% if req.reviewer == review_req.reviewer %}</strong>{% endif %}
+          {% for a in review_req.all_completed_assignments_for_doc %}
+            <a href="{% url "ietf.doc.views_doc.document_main" name=a.review.name %}">{{a.review_request.team.acronym|capfirst}} {{a.review_request.type.name}} review of -{{a.reviewed_rev}} by {{a.reviewer.person.plain_name}}</a> {% if a.reviewed_rev != a.review_request.doc.rev %} (<a href="{{ rfcdiff_base_url }}?url1={{ a.review_request.doc.name }}-{{ a.reviewed_rev }}&amp;url2={{ a.review_request.doc.name }}-{{ a.review_request.doc.rev }}">diff</a>){% endif %}<br>
           {% endfor %}
         </td>
       </tr>
@@ -134,7 +132,7 @@
           <tbody class="panel-meta">
             <tr>
               <th>State</th>
-              <td>{{ review_req.state.name }}
+              <td>{{ assignment.state.name }}
                 {% if snapshot %}
                    <span class="label label-warning">Snapshot</span>
                 {% endif %}