Add support for revising a closed review, mostly to allow reviewers to

correct historic entries with missing data
 - Legacy-Id: 12314

Author: OleLaursen

ietf/doc/tests_review.py

@@ -19,7 +19,7 @@
 from ietf.person.models import Email, Person
 from ietf.name.models import ReviewResultName, ReviewRequestStateName, ReviewTypeName, DocRelationshipName
 from ietf.group.models import Group
-from ietf.doc.models import DocumentAuthor, Document, DocAlias, RelatedDocument, DocEvent
+from ietf.doc.models import DocumentAuthor, Document, DocAlias, RelatedDocument, DocEvent, ReviewRequestDocEvent
 from ietf.utils.test_utils import TestCase
 from ietf.utils.test_data import make_test_data, make_review_data, create_person
 from ietf.utils.test_utils import login_testing_unauthorized, unicontent, reload_db_objects
@@ -553,7 +553,6 @@ def test_complete_review_enter_content(self):
 
         login_testing_unauthorized(self, review_req.reviewer.person.user.username, url)
 
-        # complete by uploading file
         empty_outbox()
 
         r = self.client.post(url, data={
@@ -584,7 +583,6 @@ def test_complete_review_link_to_mailing_list(self):
 
         login_testing_unauthorized(self, review_req.reviewer.person.user.username, url)
 
-        # complete by uploading file
         empty_outbox()
 
         r = self.client.post(url, data={
@@ -664,3 +662,58 @@ def test_partially_complete_review(self):
         second_review = review_req.review
         self.assertTrue(first_review.name != second_review.name)
         self.assertTrue(second_review.name.endswith("-2")) # uniquified
+
+    def test_revise_review_enter_content(self):
+        review_req, url = self.setup_complete_review_test()
+        review_req.state = ReviewRequestStateName.objects.get(slug="no-response")
+        review_req.save()
+
+        login_testing_unauthorized(self, review_req.reviewer.person.user.username, url)
+
+        empty_outbox()
+
+        r = self.client.post(url, data={
+            "result": ReviewResultName.objects.get(resultusedinreviewteam__team=review_req.team, slug="ready").pk,
+            "state": ReviewRequestStateName.objects.get(slug="completed").pk,
+            "reviewed_rev": review_req.doc.rev,
+            "review_submission": "enter",
+            "review_content": "This is a review\nwith two lines",
+            "review_url": "",
+            "review_file": "",
+            "completion_date": "2012-12-24",
+            "completion_time": "12:13:14",
+        })
+        self.assertEqual(r.status_code, 302)
+
+        review_req = reload_db_objects(review_req)
+        self.assertEqual(review_req.state_id, "completed")
+        event = ReviewRequestDocEvent.objects.get(type="closed_review_request", review_request=review_req)
+        self.assertEqual(event.time, datetime.datetime(2012, 12, 24, 12, 13, 14))
+
+        with open(os.path.join(self.review_subdir, review_req.review.name + ".txt")) as f:
+            self.assertEqual(f.read(), "This is a review\nwith two lines")
+
+        self.assertEqual(len(outbox), 0)
+
+        # revise again
+        empty_outbox()
+        r = self.client.post(url, data={
+            "result": ReviewResultName.objects.get(resultusedinreviewteam__team=review_req.team, slug="ready").pk,
+            "state": ReviewRequestStateName.objects.get(slug="part-completed").pk,
+            "reviewed_rev": review_req.doc.rev,
+            "review_submission": "enter",
+            "review_content": "This is a revised review",
+            "review_url": "",
+            "review_file": "",
+            "completion_date": "2013-12-24",
+            "completion_time": "11:11:11",
+        })
+        self.assertEqual(r.status_code, 302)
+
+        review_req = reload_db_objects(review_req)
+        self.assertEqual(review_req.review.rev, "01")
+        event = ReviewRequestDocEvent.objects.get(type="closed_review_request", review_request=review_req)
+        self.assertEqual(event.time, datetime.datetime(2013, 12, 24, 11, 11, 11))
+
+        self.assertEqual(len(outbox), 0)
+        

ietf/doc/views_review.py

@@ -165,10 +165,10 @@ def review_request(request, name, request_id):
                                       and review_req.reviewer
                                       and (is_reviewer or can_manage_request))
 
-    can_complete_review = (review_req.state_id in ["requested", "accepted"]
+    can_complete_review = (review_req.state_id in ["requested", "accepted", "overtaken", "no-response", "part-completed", "completed"]
                            and review_req.reviewer
                            and (is_reviewer or can_manage_request))
-    
+
     if request.method == "POST" and request.POST.get("action") == "accept" and can_accept_reviewer_assignment:
         review_req.state = ReviewRequestStateName.objects.get(slug="accepted")
         review_req.save()
@@ -331,6 +331,8 @@ class CompleteReviewForm(forms.Form):
     review_url = forms.URLField(label="Link to message", required=False)
     review_file = forms.FileField(label="Text file to upload", required=False)
     review_content = forms.CharField(widget=forms.Textarea, required=False)
+    completion_date = DatepickerDateField(date_format="yyyy-mm-dd", picker_settings={ "autoclose": "1" }, initial=datetime.date.today, help_text="Date of announcement of the results of this review")
+    completion_time = forms.TimeField(widget=forms.HiddenInput, initial=datetime.time.min)
     cc = forms.CharField(required=False, help_text="Email addresses to send to in addition to the review team list")
 
     def __init__(self, review_req, *args, **kwargs):
@@ -342,20 +344,33 @@ def __init__(self, review_req, *args, **kwargs):
 
         known_revisions = NewRevisionDocEvent.objects.filter(doc=doc).order_by("time", "id").values_list("rev", flat=True)
 
-        self.fields["state"].choices = [
-            (slug, "{} - extra reviewer is to be assigned".format(label)) if slug == "part-completed" else (slug, label)
-            for slug, label in self.fields["state"].choices
-        ]
+        revising_review = review_req.state_id not in ["requested", "accepted"]
+
+        if not revising_review:
+            self.fields["state"].choices = [
+                (slug, "{} - extra reviewer is to be assigned".format(label)) if slug == "part-completed" else (slug, label)
+                for slug, label in self.fields["state"].choices
+            ]
 
         self.fields["reviewed_rev"].help_text = mark_safe(
             " ".join("<a class=\"rev label label-default\">{}</a>".format(r)
                      for r in known_revisions))
 
         self.fields["result"].queryset = self.fields["result"].queryset.filter(resultusedinreviewteam__team=review_req.team)
-        self.fields["review_submission"].choices = [
-            (k, label.format(mailing_list=review_req.team.list_email or "[error: team has no mailing list set]"))
-            for k, label in self.fields["review_submission"].choices
-        ]
+
+        def format_submission_choice(label):
+            if revising_review:
+                label = label.replace(" (automatically posts to {mailing_list})", "")
+
+            return label.format(mailing_list=review_req.team.list_email or "[error: team has no mailing list set]")
+
+        self.fields["review_submission"].choices = [ (k, format_submission_choice(label)) for k, label in self.fields["review_submission"].choices]
+
+        if revising_review:
+            del self.fields["cc"]
+        else:
+            del self.fields["completion_date"]
+            del self.fields["completion_time"]
 
     def clean_reviewed_rev(self):
         return clean_doc_revision(self.review_req.doc, self.cleaned_data.get("reviewed_rev"))
@@ -386,7 +401,9 @@ def require_field(f):
 @login_required
 def complete_review(request, name, request_id):
     doc = get_object_or_404(Document, name=name)
-    review_req = get_object_or_404(ReviewRequest, pk=request_id, state__in=["requested", "accepted"])
+    review_req = get_object_or_404(ReviewRequest, pk=request_id)
+
+    revising_review = review_req.state_id not in ["requested", "accepted"]
 
     if not review_req.reviewer:
         return redirect(review_request, name=review_req.doc.name, request_id=review_req.pk)
@@ -402,30 +419,34 @@ def complete_review(request, name, request_id):
         if form.is_valid():
             review_submission = form.cleaned_data['review_submission']
 
-            # create review doc
-            for i in range(1, 100):
-                name_components = [
-                    "review",
-                    strip_prefix(review_req.doc.name, "draft-"),
-                    form.cleaned_data["reviewed_rev"],
-                    review_req.team.acronym,
-                    review_req.type.slug,
-                    xslugify(review_req.reviewer.person.ascii_parts()[3]),
-                    datetime.date.today().isoformat(),
-                ]
-                if i > 1:
-                    name_components.append(str(i))
-
-                name = "-".join(c for c in name_components if c).lower()
-                if not Document.objects.filter(name=name).exists():
-                    review = Document.objects.create(name=name)
-                    DocAlias.objects.create(document=review, name=review.name)
-                    break
-
-            review.type = DocTypeName.objects.get(slug="review")
-            review.rev = "00"
+            review = review_req.review
+            if not review:
+                # create review doc
+                for i in range(1, 100):
+                    name_components = [
+                        "review",
+                        strip_prefix(review_req.doc.name, "draft-"),
+                        form.cleaned_data["reviewed_rev"],
+                        review_req.team.acronym,
+                        review_req.type.slug,
+                        xslugify(review_req.reviewer.person.ascii_parts()[3]),
+                        datetime.date.today().isoformat(),
+                    ]
+                    if i > 1:
+                        name_components.append(str(i))
+
+                    name = "-".join(c for c in name_components if c).lower()
+                    if not Document.objects.filter(name=name).exists():
+                        review = Document.objects.create(name=name)
+                        DocAlias.objects.create(document=review, name=review.name)
+                        break
+
+                review.type = DocTypeName.objects.get(slug="review")
+                review.group = review_req.team
+
+            review.rev = "00" if not review.rev else "{:02}".format(int(review.rev) + 1)
             review.title = "{} Review of {}-{}".format(review_req.type.name, review_req.doc.name, form.cleaned_data["reviewed_rev"])
-            review.group = review_req.team
+            review.time = datetime.datetime.now()
             if review_submission == "link":
                 review.external_url = form.cleaned_data['review_url']
 
@@ -459,7 +480,7 @@ def complete_review(request, name, request_id):
             review_req.review = review
             review_req.save()
 
-            need_to_email_review = review_submission != "link" and review_req.team.list_email
+            need_to_email_review = review_submission != "link" and review_req.team.list_email and not revising_review
 
             desc = "Request for {} review by {} {}: {}. Reviewer: {}.".format(
                 review_req.type.name,
@@ -471,16 +492,22 @@ def complete_review(request, name, request_id):
             if need_to_email_review:
                 desc += " " + "Sent review to list."
 
-            close_event = ReviewRequestDocEvent.objects.create(
-                type="closed_review_request",
-                doc=review_req.doc,
-                by=request.user.person,
-                desc=desc,
-                review_request=review_req,
-                state=review_req.state,
-            )
+            completion_datetime = datetime.datetime.now()
+            if "completion_date" in form.cleaned_data:
+                completion_datetime = datetime.datetime.combine(form.cleaned_data["completion_date"], form.cleaned_data.get("completion_time") or datetime.time.min)
+
+            close_event = ReviewRequestDocEvent.objects.filter(type="closed_review_request", review_request=review_req).first()
+            if not close_event:
+                close_event = ReviewRequestDocEvent(type="closed_review_request", review_request=review_req)
+
+            close_event.doc = review_req.doc
+            close_event.by = request.user.person
+            close_event.desc = desc
+            close_event.state = review_req.state
+            close_event.time = completion_datetime
+            close_event.save()
 
-            if review_req.state_id == "part-completed":
+            if review_req.state_id == "part-completed" and not revising_review:
                 existing_open_reqs = ReviewRequest.objects.filter(doc=review_req.doc, team=review_req.team, state__in=("requested", "accepted"))
 
                 new_review_req_url = new_review_req = None
@@ -519,7 +546,10 @@ def complete_review(request, name, request_id):
 
             return redirect("doc_view", name=review_req.review.name)
     else:
-        form = CompleteReviewForm(review_req)
+        form = CompleteReviewForm(review_req, initial={
+            "reviewed_rev": review_req.reviewed_rev,
+            "result": review_req.result_id
+        })
 
     mail_archive_query_urls = mailarch.construct_query_urls(review_req)
 
@@ -528,11 +558,12 @@ def complete_review(request, name, request_id):
         'review_req': review_req,
         'form': form,
         'mail_archive_query_urls': mail_archive_query_urls,
+        'revising_review': revising_review,
     })
 
 def search_mail_archive(request, name, request_id):
     #doc = get_object_or_404(Document, name=name)
-    review_req = get_object_or_404(ReviewRequest, pk=request_id, state__in=["requested", "accepted"])
+    review_req = get_object_or_404(ReviewRequest, pk=request_id)
 
     is_reviewer = user_is_person(request.user, review_req.reviewer.person)
     can_manage_request = can_manage_review_requests_for_team(request.user, review_req.team)

ietf/static/ietf/js/complete-review.js

@@ -72,6 +72,8 @@ $(document).ready(function () {
                     row.find(".from").text(msg.splitfrom[0]);
                     row.data("url", msg.url);
                     row.data("content", msg.content);
+                    row.data("date", msg.utcdate[0]);
+                    row.data("time", msg.utcdate[1]);
                     results.append(row);
                 }
             }
@@ -99,6 +101,8 @@ $(document).ready(function () {
 
         form.find("[name=review_url]").val(row.data("url"));
         form.find("[name=review_content]").val(row.data("content")).prop("scrollTop", 0);
+        form.find("[name=completion_date]").val(row.data("date"));
+        form.find("[name=completion_time]").val(row.data("time"));
     });
 
 

ietf/templates/doc/review/complete_review.html

@@ -2,26 +2,34 @@
 {# Copyright The IETF Trust 2016, All Rights Reserved #}
 {% load origin bootstrap3 static %}
 
-{% block title %}Complete review of {{ review_req.doc.name }}{% endblock %}
+{% block title %}{% if revising_review %}Revise{% else %}Complete{% endif %} review of {{ review_req.doc.name }}{% endblock %}
+
+{% block pagehead %}
+  <link rel="stylesheet" href="{% static 'bootstrap-datepicker/css/bootstrap-datepicker3.min.css' %}">
+{% endblock %}
 
 {% block content %}
   {% origin %}
-  <h1>Complete review<br><small>{{ review_req.doc.name }}</small></h1>
-
-  <p>The review findings should be made available here and the review
-    posted to the mailing list. If you enter the findings below, the
-    system will post the review for you. If you already have posted
-    the review, you can try to let the system find the link to the
-    archive and retrieve the email body.</p>
+  <h1>{% if revising_review %}Revise{% else %}Complete{% endif %} review<br><small>{{ review_req.doc.name }}</small></h1>
 
+  {% if not revising_review %}
+    <p>The review findings should be made available here and the review
+      posted to the mailing list. If you enter the findings below, the
+      system will post the review for you. If you already have posted
+      the review, you can try to let the system find the link to the
+      archive and retrieve the email body.</p>
+  {% else %}
+    <p>You can revise this review by entering the results below.</p>
+  {% endif %}
+    
   <form class="complete-review form-horizontal" method="post" enctype="multipart/form-data">
     {% csrf_token %}
 
     {% bootstrap_form form layout="horizontal" %}
 
     {% buttons %}
     <a class="btn btn-default" href="{% url "ietf.doc.views_review.review_request" name=doc.canonical_name request_id=review_req.pk %}">Cancel</a>
-    <button type="submit" class="btn btn-primary">Complete review</button>
+    <button type="submit" class="btn btn-primary">{% if revising_review %}Revise{% else %}Complete{% endif %} review</button>
     {% endbuttons %}
 
     <div class="template" style="display:none">
@@ -76,6 +84,7 @@ <h1>Complete review<br><small>{{ review_req.doc.name }}</small></h1>
 {% endblock %}
 
 {% block js %}
+  <script src="{% static 'bootstrap-datepicker/js/bootstrap-datepicker.min.js' %}"></script>
   <script>
    var searchMailArchiveUrl = "{% url "ietf.doc.views_review.search_mail_archive" name=review_req.doc.name request_id=review_req.pk %}";
   </script>

ietf/templates/doc/review/review_request.html

@@ -124,7 +124,7 @@ <h1>Review request<br><small>{{ review_req.doc.name }}</small></h1>
           {% endif %}
 
           {% if can_complete_review %}
-            <a class="btn btn-primary btn-xs" href="{% url "ietf.doc.views_review.complete_review" name=doc.name request_id=review_req.pk %}"><span class="fa fa-pencil-square-o"></span> Complete review</a>
+            <a class="btn btn-primary btn-xs" href="{% url "ietf.doc.views_review.complete_review" name=doc.name request_id=review_req.pk %}"><span class="fa fa-pencil-square-o"></span> {% if review_req.state_id == "requested" or review_req.state_id == "accepted" %}Complete review{% else %}Correct review{% endif %}</a>
           {% endif %}
         </td>
       </tr>