Merged in [17070] from sasha@dashcare.nl:

levkowetz · levkowetz · commit 2bc560698691 · 2019-11-26T20:22:56.000Z
Fix ietf-tools#2061 - Improve 'complete review' workflow for secretaries. When a secretary completes a review, 'link to a review message' is automatically selected, and the first non-reply mail is used to fill in the review details. The secretary can still modify all details. The order of fields for secretaries is also modified to fit this workflow. All cases where 'link to review message' is used, by reviewers or secretaries, now attempt to fill in the 'reviewed version' if found in the email subject. - Legacy-Id: 17105 Note: SVN reference [17070] has been migrated to Git commit dc95462
diff --git a/ietf/doc/tests_review.py b/ietf/doc/tests_review.py
@@ -621,13 +621,15 @@ def test_search_mail_archive(self):
             self.assertEqual(messages[0]["url"], "https://www.example.com/testmessage")
             self.assertTrue("John Doe" in messages[0]["content"])
             self.assertEqual(messages[0]["subject"], "Review of {}-01".format(review_req.doc.name))
+            self.assertEqual(messages[0]["revision_guess"], "01")
             self.assertEqual(messages[0]["splitfrom"], ["John Doe", "johndoe@example.com"])
             self.assertEqual(messages[0]["utcdate"][0], today.isoformat())
 
             self.assertEqual(messages[1]["url"], "https://www.example.com/testmessage2")
             self.assertTrue("Looks OK" in messages[1]["content"])
             self.assertTrue("<html>" not in messages[1]["content"])
             self.assertEqual(messages[1]["subject"], "Review of {}".format(review_req.doc.name))
+            self.assertFalse('revision_guess' in messages[1])
             self.assertEqual(messages[1]["splitfrom"], ["John Doe II", "johndoe2@example.com"])
             self.assertEqual(messages[1]["utcdate"][0], "")
 
diff --git a/ietf/doc/views_review.py b/ietf/doc/views_review.py
@@ -535,6 +535,11 @@ def __init__(self, assignment, doc, team, is_reviewer, *args, **kwargs):
 
         revising_review = assignment.state_id not in ["assigned", "accepted"] if assignment else False
 
+        if not is_reviewer:
+            new_field_order = ['review_submission', 'review_url', 'review_file', 'review_content']
+            new_field_order += [f for f in self.fields.keys() if f not in new_field_order]
+            self.order_fields(new_field_order)
+
         if not revising_review:
             self.fields["state"].choices = [
                 (slug, "{} - extra reviewer is to be assigned".format(label)) if slug == "part-completed" else (slug, label)
@@ -891,6 +896,7 @@ def complete_review(request, name, assignment_id=None, acronym=None):
         'revising_review': revising_review,
         'review_to': to,
         'review_cc': cc,
+        'is_reviewer': is_reviewer,
     })
 
 def search_mail_archive(request, name, acronym=None, assignment_id=None):
@@ -916,6 +922,12 @@ def search_mail_archive(request, name, acronym=None, assignment_id=None):
 
     try:
         res["messages"] = mailarch.retrieve_messages(res["query_data_url"])[:MAX_RESULTS]
+        for message in res["messages"]:
+            try:
+                revision_guess = message["subject"].split(name)[1].split('-')[1]
+                message["revision_guess"] = revision_guess if revision_guess.isnumeric() else None
+            except IndexError:
+                pass
     except KeyError as e:
         res["error"] = "No results found (%s)" % str(e)
     except Exception as e:
diff --git a/ietf/static/ietf/js/complete-review.js b/ietf/static/ietf/js/complete-review.js
@@ -47,6 +47,7 @@ $(document).ready(function () {
             if (!err && (!data.messages || !data.messages.length))
                 err = "No messages matching document name found in archive";
 
+            var non_reply_row = null;
             if (err) {
                 var errorDiv = mailArchiveSearch.find(".error");
                 errorDiv.removeClass("hidden");
@@ -74,7 +75,15 @@ $(document).ready(function () {
                     row.data("content", msg.content);
                     row.data("date", msg.utcdate[0]);
                     row.data("time", msg.utcdate[1]);
+                    row.data("revision_guess", msg.revision_guess);
                     results.append(row);
+                    if (msg.subject.toUpperCase().substr(0, 3) !== 'RE:') {
+                        non_reply_row = row;
+                    }
+                }
+                if (!isReviewer && non_reply_row) {
+                    // Automatically select the first non-reply.
+                    non_reply_row.click();
                 }
             }
         }, function () {
@@ -103,6 +112,7 @@ $(document).ready(function () {
         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"));
+        form.find("[name=reviewed_rev]").val(row.data("revision_guess"));
     });
 
 
@@ -133,4 +143,9 @@ $(document).ready(function () {
         if (val == "link")
             searchMailArchive();
     }).trigger("change");
+    
+    if (!isReviewer) {
+        // Select mail search by default for secretary completions.
+        form.find("[name=review_submission][value=link]").click()
+    }
 });
diff --git a/ietf/templates/doc/review/complete_review.html b/ietf/templates/doc/review/complete_review.html
@@ -115,6 +115,7 @@ <h1>{% if revising_review %}Revise{% elif assignment %}Complete{% else %}Submit
   {% else %}
     var searchMailArchiveUrl = "{% url "ietf.doc.views_review.search_mail_archive" name=doc.name acronym=team.acronym %}";
   {% endif %}
+  var isReviewer = {{ is_reviewer|yesno:'true,false' }};
   </script>
   <script src="{% static 'ietf/js/complete-review.js' %}"></script>
 {% endblock %}
