diff --git a/ietf/doc/tests_review.py b/ietf/doc/tests_review.py index e98f729c9..67a663b76 100644 --- a/ietf/doc/tests_review.py +++ b/ietf/doc/tests_review.py @@ -15,8 +15,8 @@ import debug # pyflakes:ignore 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 @@ class ReviewTests(TestCase): 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() diff --git a/ietf/doc/views_doc.py b/ietf/doc/views_doc.py index 3317748ea..ba6fde043 100644 --- a/ietf/doc/views_doc.py +++ b/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, diff --git a/ietf/group/views.py b/ietf/group/views.py index 221e0587e..8894e6486 100644 --- a/ietf/group/views.py +++ b/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"), diff --git a/ietf/group/views_review.py b/ietf/group/views_review.py index 483b269a3..47b607ccf 100644 --- a/ietf/group/views_review.py +++ b/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 = [] diff --git a/ietf/review/utils.py b/ietf/review/utils.py index 955231806..05d84aa60 100644 --- a/ietf/review/utils.py +++ b/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 suggested_review_requests_for_team(team): 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 diff --git a/ietf/templates/doc/document_draft.html b/ietf/templates/doc/document_draft.html index 8e6a22189..790908580 100644 --- a/ietf/templates/doc/document_draft.html +++ b/ietf/templates/doc/document_draft.html @@ -199,7 +199,7 @@ {% 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 %} diff --git a/ietf/templates/doc/document_review.html b/ietf/templates/doc/document_review.html index 0745a8c42..1d83e0c00 100644 --- a/ietf/templates/doc/document_review.html +++ b/ietf/templates/doc/document_review.html @@ -100,7 +100,7 @@ {% 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 %} diff --git a/ietf/templates/doc/review_request_summary.html b/ietf/templates/doc/review_request_summary.html index 4631dd256..22bf04dcd 100644 --- a/ietf/templates/doc/review_request_summary.html +++ b/ietf/templates/doc/review_request_summary.html @@ -1,7 +1,7 @@
{% if review_request.state_id == "completed" or review_request.state_id == "part-completed" %} - {{ 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 }} {% else %} diff --git a/ietf/templates/group/review_requests.html b/ietf/templates/group/review_requests.html index eccb631eb..8e1df839a 100644 --- a/ietf/templates/group/review_requests.html +++ b/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 %} @@ -29,7 +29,7 @@ {% for r in open_review_requests %} - {{ r.doc.name }}{% if r.requested_rev %}-{{ r.requested_rev }}{% endif %} + {{ r.doc.name }}-{% if r.requested_rev %}{{ r.requested_rev }}{% else %}{{ r.doc.rev }}{% endif %} {{ r.type.name }} {% if r.time %}{{ r.time|date:"Y-m-d" }}{% else %}auto-suggested{% endif %}