Follow replacements when displaying reviews for a draft on the

document page, too. Add a test to check that recursive replacements
are handled correctly. Polish the display a bit.
 - Legacy-Id: 11847

Author: OleLaursen

ietf/doc/tests_review.py

@@ -15,8 +15,8 @@
 from ietf.review.models import ReviewRequest, ReviewTeamResult, ReviewerSettings
 import ietf.review.mailarch
 from ietf.person.models import Email, Person
-from ietf.name.models import ReviewResultName, ReviewRequestStateName, ReviewTypeName
-from ietf.doc.models import DocumentAuthor
+from ietf.name.models import ReviewResultName, ReviewRequestStateName, ReviewTypeName, DocRelationshipName
+from ietf.doc.models import DocumentAuthor, Document, DocAlias, RelatedDocument
 from ietf.utils.test_utils import TestCase
 from ietf.utils.test_data import make_test_data, make_review_data
 from ietf.utils.test_utils import login_testing_unauthorized, unicontent, reload_db_objects
@@ -70,8 +70,23 @@ def test_request_review(self):
         self.assertEqual(doc.latest_event().type, "requested_review")
 
     def test_doc_page(self):
-        # FIXME: fill in
-        pass
+        doc = make_test_data()
+        review_req = make_review_data(doc)
+
+        # move the review request to a doubly-replaced document to
+        # check we can fish it out
+        old_doc = Document.objects.get(name="draft-foo-mars-test")
+        older_doc = Document.objects.create(name="draft-older")
+        older_docalias = DocAlias.objects.create(name=older_doc.name, document=older_doc)
+        RelatedDocument.objects.create(source=old_doc, target=older_docalias, relationship=DocRelationshipName.objects.get(slug='replaces'))
+        review_req.doc = older_doc
+        review_req.save()
+
+        url = urlreverse('doc_view', kwargs={ "name": doc.name })
+        r = self.client.get(url)
+        self.assertEqual(r.status_code, 200)
+        content = unicontent(r)
+        self.assertTrue("{} Review".format(review_req.type.name) in content)
 
     def test_review_request(self):
         doc = make_test_data()

ietf/doc/views_doc.py

@@ -581,7 +581,7 @@ def document_main(request, name, rev=None):
 
         other_reviews = []
         if review_req:
-            other_reviews = review_requests_to_list_for_doc(review_req.doc).exclude(pk=review_req.pk)
+            other_reviews = [r for r in review_requests_to_list_for_doc(review_req.doc) if r != review_req]
 
         return render(request, "doc/document_review.html",
                       dict(doc=doc,

ietf/group/views.py

@@ -662,7 +662,7 @@ def review_requests(request, acronym, group_type=None):
         team=group,
     ).exclude(
         state__in=("requested", "accepted")
-    ).prefetch_related("reviewer", "type", "state").order_by("-time", "-id")
+    ).prefetch_related("reviewer", "type", "state", "doc").order_by("-time", "-id")
 
     since_choices = [
         (None, "1 month"),

ietf/group/views_review.py

@@ -82,7 +82,8 @@ def manage_review_requests(request, acronym, group_type=None):
         set(r.doc_id for r in review_requests),
     )
 
-    # we need a mutable query dict
+    # we need a mutable query dict for resetting upon saving with
+    # conflicts
     query_dict = request.POST.copy() if request.method == "POST" else None
     for req in review_requests:
         l = []

ietf/review/utils.py

@@ -34,9 +34,12 @@ def can_manage_review_requests_for_team(user, team, allow_non_team_personnel=Tru
             or (allow_non_team_personnel and has_role(user, "Secretariat")))
 
 def review_requests_to_list_for_doc(doc):
-    return ReviewRequest.objects.filter(doc=doc).exclude(
-        state__in=["withdrawn", "rejected", "overtaken", "no-response"]
-    ).order_by("-time", "-id")
+    return extract_revision_ordered_review_requests_for_documents(
+        ReviewRequest.objects.exclude(
+            state__in=["withdrawn", "rejected", "overtaken", "no-response"],
+        ).prefetch_related("result"),
+        [doc.name]
+    ).get(doc.pk, [])
 
 def make_new_review_request_from_existing(review_req):
     obj = ReviewRequest()
@@ -234,13 +237,15 @@ def blocks(existing, request):
     res.sort(key=lambda r: (r.deadline, r.doc_id), reverse=True)
     return res
 
-def extract_revision_ordered_review_requests_for_documents(queryset, names):
+def extract_revision_ordered_review_requests_for_documents(review_request_queryset, names):
+    """Extracts all review requests for document names (including replaced ancestors)."""
+
     names = set(names)
 
     replaces = extract_complete_replaces_ancestor_mapping_for_docs(names)
 
     requests_for_each_doc = defaultdict(list)
-    for r in queryset.filter(doc__in=set(e for l in replaces.itervalues() for e in l) | names).order_by("-reviewed_rev", "-time", "-id").iterator():
+    for r in review_request_queryset.filter(doc__in=set(e for l in replaces.itervalues() for e in l) | names).order_by("-reviewed_rev", "-time", "-id").iterator():
         requests_for_each_doc[r.doc_id].append(r)
 
     # now collect in breadth-first order to keep the revision order intact

ietf/templates/doc/document_draft.html

@@ -199,7 +199,7 @@
 	<td class="edit"></td>
         <td>
           {% for review_request in review_requests %}
-            {% include "doc/review_request_summary.html" with current_rev=doc.rev %}
+            {% include "doc/review_request_summary.html" with current_doc_name=doc.name current_rev=doc.rev %}
           {% endfor %}
 
           {% if can_request_review %}

ietf/templates/doc/document_review.html

@@ -100,7 +100,7 @@
         <td class="edit"></td>
         <td>
           {% for review_request in other_reviews %}
-            {% include "doc/review_request_summary.html" with current_rev=review_req.reviewed_rev %}
+            {% include "doc/review_request_summary.html" with current_doc_name=review_req.doc_id current_rev=review_req.reviewed_rev %}
           {% endfor %}
         </td>
       </tr>

ietf/templates/doc/review_request_summary.html

@@ -1,7 +1,7 @@
 <div class="review-request-summary">
   {% if review_request.state_id == "completed" or review_request.state_id == "part-completed" %}
     <a href="{% if review_request.review %}{% url "doc_view" review_request.review.name %}{% else %}{% url "ietf.doc.views_review.review_request" review_request.doc_id review_request.pk %}{% endif %}">
-      {{ review_request.team.acronym|upper }} {{ review_request.type.name }} Review{% if review_request.reviewed_rev and review_request.reviewed_rev != current_rev %} (of -{{ review_request.reviewed_rev }}){% endif %}:
+      {{ review_request.team.acronym|upper }} {{ review_request.type.name }} Review{% if review_request.reviewed_rev and review_request.reviewed_rev != current_rev or review_request.doc_id != current_doc_name %} (of {% if review_request.doc_id != current_doc_name %}{{ review_request.doc_id }}{% endif %}-{{ review_request.reviewed_rev }}){% endif %}:
       {{ review_request.result.name }} {% if review_request.state_id == "part-completed" %}(partially completed){% endif %}
       - reviewer: {{ review_request.reviewer.person }}</a>
   {% else %}

ietf/templates/group/review_requests.html

@@ -4,7 +4,7 @@
 
 {% load ietf_filters staticfiles bootstrap3 %}
 
-{% block group_subtitle %}Reviews for {{ group.name }}{% endblock %}
+{% block group_subtitle %}Review requests{% endblock %}
 
 {% block pagehead %}
   <link rel="stylesheet" href="{% static "jquery.tablesorter/css/theme.bootstrap.min.css" %}">
@@ -29,7 +29,7 @@ <h2>Open review requests</h2>
       <tbody>
         {% for r in open_review_requests %}
           <tr>
-            <td><a {% if r.pk != None %}href="{% url "ietf.doc.views_review.review_request" name=r.doc.name request_id=r.pk %}"{% endif %}>{{ r.doc.name }}{% if r.requested_rev %}-{{ r.requested_rev }}{% endif %}</a></td>
+            <td><a {% if r.pk != None %}href="{% url "ietf.doc.views_review.review_request" name=r.doc.name request_id=r.pk %}"{% endif %}>{{ r.doc.name }}-{% if r.requested_rev %}{{ r.requested_rev }}{% else %}{{ r.doc.rev }}{% endif %}</a></td>
             <td>{{ r.type.name }}</td>
             <td>{% if r.time %}{{ r.time|date:"Y-m-d" }}{% else %}<em>auto-suggested</em>{% endif %}</td>
             <td>