From dc9546211f91a6daf9ec21a06bf91ea3fc8a1276 Mon Sep 17 00:00:00 2001 From: Sasha Romijn Date: Wed, 20 Nov 2019 14:25:05 +0000 Subject: [PATCH] Fix #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. Commit ready for merge. - Legacy-Id: 17070 --- ietf/doc/tests_review.py | 2 ++ ietf/doc/views_review.py | 13 +++++++++++++ ietf/static/ietf/js/complete-review.js | 15 +++++++++++++++ ietf/templates/doc/review/complete_review.html | 1 + 4 files changed, 31 insertions(+) diff --git a/ietf/doc/tests_review.py b/ietf/doc/tests_review.py index a1d8fb909..d0f94fa36 100644 --- a/ietf/doc/tests_review.py +++ b/ietf/doc/tests_review.py @@ -596,6 +596,7 @@ class ReviewTests(TestCase): 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()) @@ -603,6 +604,7 @@ class ReviewTests(TestCase): self.assertTrue("Looks OK" in messages[1]["content"]) self.assertTrue("" 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 index 537548add..b2748f7db 100644 --- a/ietf/doc/views_review.py +++ b/ietf/doc/views_review.py @@ -531,6 +531,11 @@ class CompleteReviewForm(forms.Form): 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 @@ class CompleteReviewForm(forms.Form): # 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("{1}".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 index 90749a494..9f373a2e1 100644 --- 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 index b2e9b8d0e..d7cd23ee7 100644 --- a/ietf/templates/doc/review/complete_review.html +++ b/ietf/templates/doc/review/complete_review.html @@ -115,6 +115,7 @@ {% 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' }}; {% endblock %}