Fix ietf-tools#2061 - Improve "complete review" workflow for secretaries.

Sasha Romijn · Sasha Romijn · commit dc9546211f91 · 2019-11-20T14:25:05.000Z
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.

Commit ready for merge.
 - Legacy-Id: 17070
diff --git a/ietf/doc/tests_review.py b/ietf/doc/tests_review.py
@@ -596,13 +596,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
@@ -531,6 +531,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)
@@ -556,6 +561,7 @@ def __init__(self, assignment, doc, team, is_reviewer, *args, **kwargs):
             # If it is users own review, then default to latest version
             if is_reviewer:
                 kwargs["initial"]["reviewed_rev"] = last_version
+            
 
             self.fields["reviewed_rev"].help_text = mark_safe(
                 " ".join("<a class=\"rev label label-default {0}\" title=\"{2:%Y-%m-%d}\">{1}</a>".format(reviewed_rev_class[i], *r)
@@ -887,6 +893,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):
@@ -912,6 +919,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 %}
