altf4arnold/datatracker
1
0
Fork 0
Code Issues Pull requests Actions 3 Packages Projects Releases Wiki Activity

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

Browse source 
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
This commit is contained in:
Sasha Romijn 2019-11-20 14:25:05 +00:00
parent 65d84155b6
commit dc9546211f
4 changed files with 31 additions and 0 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
ietf/doc/tests_review.py
View file

@ -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("<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], "")

13
ietf/doc/views_review.py
View file

@ -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("<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:

15
ietf/static/ietf/js/complete-review.js
View file

@ -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()
}
});

1
ietf/templates/doc/review/complete_review.html
View file

@ -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' }};
</script>
<script src="{% static 'ietf/js/complete-review.js' %}"></script>
{% endblock %}