From ef1e8006f0bb11f0c888574cb2dbd2f6efe2bcb7 Mon Sep 17 00:00:00 2001 From: Sasha Romijn Date: Mon, 21 Oct 2019 19:11:56 +0000 Subject: [PATCH] Fix #2526 - List previous reviews in assignments email. This includes a migration to update the database templates. Note that not all templates currently included them (secdir at least), and previous reviews are only listed in templates that included */** before. As before, previous reviews are only included if they are done by the same reviewer(s) as the current assignment. This also fixes a bug in database template 182 (/group/defaults/email/open_assignments.txt) which referred to r.review_request..doc.rev and r.review_request..requested_rev in the template, and updates the test to use the current version of template 182. Commit ready for merge. - Legacy-Id: 16895 --- ...assignment_email_summary_templates_2526.py | 283 ++++++++++++++++++ ietf/group/tests_review.py | 24 +- ietf/group/views.py | 9 +- 3 files changed, 300 insertions(+), 16 deletions(-) create mode 100644 ietf/dbtemplate/migrations/0005_adjust_assignment_email_summary_templates_2526.py diff --git a/ietf/dbtemplate/migrations/0005_adjust_assignment_email_summary_templates_2526.py b/ietf/dbtemplate/migrations/0005_adjust_assignment_email_summary_templates_2526.py new file mode 100644 index 000000000..06465460a --- /dev/null +++ b/ietf/dbtemplate/migrations/0005_adjust_assignment_email_summary_templates_2526.py @@ -0,0 +1,283 @@ +# Copyright The IETF Trust 2019, All Rights Reserved +# -*- coding: utf-8 -*- +# Generated by Django 1.11.20 on 2019-03-13 13:41 + + +from __future__ import absolute_import, print_function, unicode_literals + +from django.db import migrations + +def forward(apps, schema_editor): + DBTemplate = apps.get_model('dbtemplate','DBTemplate') + + DBTemplate.objects.filter(pk=182).update(content="""{% autoescape off %}Subject: Open review assignments in {{group.acronym}} + +The following reviewers have assignments:{% for r in review_assignments %}{% ifchanged r.section %} + +{{r.section}} + +{% if r.section == 'Early review requests:' %}Reviewer Due Draft{% else %}Reviewer LC end Draft{% endif %}{% endifchanged %} +{{ r.reviewer.person.plain_name|ljust:"22" }} {% if r.section == 'Early review requests:' %}{{ r.review_request.deadline|date:"Y-m-d" }}{% else %}{{ r.lastcall_ends|default:"None " }}{% endif %} {{ r.review_request.doc.name }}-{% if r.review_request.requested_rev %}{{ r.review_request.requested_rev }}{% else %}{{ r.review_request.doc.rev }}{% endif %} {{ r.earlier_reviews }}{% endfor %} + +{% if rotation_list %}Next in the reviewer rotation: + +{% for p in rotation_list %} {{ p }} +{% endfor %}{% endif %}{% endautoescape %} +""") + + DBTemplate.objects.filter(pk=183).update(content="""{% autoescape off %}Subject: Review Assignments + +Hi all, + +The following reviewers have assignments:{% for r in review_assignments %}{% ifchanged r.section %} + +{{r.section}} + +{% if r.section == 'Early review requests:' %}Reviewer Due Draft{% else %}Reviewer Type LC end Draft{% endif %}{% endifchanged %} +{{ r.reviewer.person.plain_name|ljust:"22" }} {% if r.section == 'Early review requests:' %}{{ r.review_request.deadline|date:"Y-m-d" }}{% else %}{{r.review_request.type.name|ljust:"10"}}{{ r.lastcall_ends|default:"None " }}{% endif %} {{ r.review_request.doc.name }}-{% if r.review_request.requested_rev %}{{ r.review_request.requested_rev }}{% else %}{{ r.review_request.doc.rev }}{% endif %}{% if r.earlier_reviews %} {{ r.earlier_reviews }}{% endif %}{% endfor %} + +{% if rotation_list %}Next in the reviewer rotation: + +{% for p in rotation_list %} {{ p }} +{% endfor %}{% endif %} +The LC and Telechat review templates are included below: +------------------------------------------------------- + +-- Begin LC Template -- +I am the assigned Gen-ART reviewer for this draft. The General Area +Review Team (Gen-ART) reviews all IETF documents being processed +by the IESG for the IETF Chair. Please treat these comments just +like any other last call comments. + +For more information, please see the FAQ at + +. + +Document: +Reviewer: +Review Date: +IETF LC End Date: +IESG Telechat date: (if known) + +Summary: + +Major issues: + +Minor issues: + +Nits/editorial comments: + +-- End LC Template -- + +-- Begin Telechat Template -- +I am the assigned Gen-ART reviewer for this draft. The General Area +Review Team (Gen-ART) reviews all IETF documents being processed +by the IESG for the IETF Chair. Please wait for direction from your +document shepherd or AD before posting a new version of the draft. + +For more information, please see the FAQ at + +. + +Document: +Reviewer: +Review Date: +IETF LC End Date: +IESG Telechat date: (if known) + +Summary: + +Major issues: + +Minor issues: + +Nits/editorial comments: + +-- End Telechat Template -- +{% endautoescape %} + +""") + + DBTemplate.objects.filter(pk=184).update(content="""{% autoescape off %}Subject: Assignments + +Review instructions and related resources are at: +http://tools.ietf.org/area/sec/trac/wiki/SecDirReview{% for r in review_assignments %}{% ifchanged r.section %} + +{{r.section}} + +{% if r.section == 'Early review requests:' %}Reviewer Due Draft{% else %}Reviewer LC end Draft{% endif %}{% endifchanged %} +{{ r.reviewer.person.plain_name|ljust:"22" }}{{ r.earlier_review|yesno:'R, , ' }}{% if r.section == 'Early review requests:' %}{{ r.review_request.deadline|date:"Y-m-d" }}{% else %}{{ r.lastcall_ends|default:"None " }}{% endif %} {{ r.review_request.doc.name }}-{% if r.review_request.requested_rev %}{{ r.review_request.requested_rev }}{% else %}{{ r.review_request.doc.rev }}{% endif %}{% endfor %} + +{% if rotation_list %}Next in the reviewer rotation: + +{% for p in rotation_list %} {{ p }} +{% endfor %}{% endif %}{% endautoescape %} + +""") + + DBTemplate.objects.filter(pk=185).update(content="""{% autoescape off %}Subject: Open review assignments in {{group.acronym}} + +Review instructions and related resources are at: + + +The following reviewers have assignments:{% for r in review_assignments %}{% ifchanged r.section %} + +{{r.section}} + +{% if r.section == 'Early review requests:' %}Reviewer Due Draft{% else %}Reviewer LC end Draft{% endif %}{% endifchanged %} +{{ r.reviewer.person.plain_name|ljust:"22" }} {% if r.section == 'Early review requests:' %}{{ r.review_request.deadline|date:"Y-m-d" }}{% else %}{{ r.lastcall_ends|default:"None " }}{% endif %} {{ r.review_request.doc.name }}-{% if r.review_request.requested_rev %}{{ r.review_request.requested_rev }}{% else %}{{ r.review_request.doc.rev }}{% endif %} {{ r.earlier_reviews }}{% endfor %} + +{% if rotation_list %}Next in the reviewer rotation: + +{% for p in rotation_list %} {{ p }} +{% endfor %}{% endif %}{% endautoescape %} + +""") + + +def reverse(apps, schema_editor): + DBTemplate = apps.get_model('dbtemplate','DBTemplate') + + DBTemplate.objects.filter(pk=182).update(content="""{% autoescape off %}Subject: Open review assignments in {{group.acronym}} + +The following reviewers have assignments:{% for r in review_assignments %}{% ifchanged r.section %} + +{{r.section}} + +{% if r.section == 'Early review requests:' %}Reviewer Due Draft{% else %}Reviewer LC end Draft{% endif %}{% endifchanged %} +{{ r.reviewer.person.plain_name|ljust:"22" }} {% if r.section == 'Early review requests:' %}{{ r.review_request.deadline|date:"Y-m-d" }}{% else %}{{ r.lastcall_ends|default:"None " }}{% endif %} {{ r.review_request.doc_id }}-{% if r.review_request..requested_rev %}{{ r.review_request.requested_rev }}{% else %}{{ r.review_request..doc.rev }}{% endif %} {{ r.earlier_review_mark }}{% endfor %} + +* Other revision previously reviewed +** This revision already reviewed + +{% if rotation_list %}Next in the reviewer rotation: + +{% for p in rotation_list %} {{ p }} +{% endfor %}{% endif %}{% endautoescape %} +""") + + DBTemplate.objects.filter(pk=183).update(content="""{% autoescape off %}Subject: Review Assignments + +Hi all, + +The following reviewers have assignments:{% for r in review_assignments %}{% ifchanged r.section %} + +{{r.section}} + +{% if r.section == 'Early review requests:' %}Reviewer Due Draft{% else %}Reviewer Type LC end Draft{% endif %}{% endifchanged %} +{{ r.reviewer.person.plain_name|ljust:"22" }} {% if r.section == 'Early review requests:' %}{{ r.review_request.deadline|date:"Y-m-d" }}{% else %}{{r.review_request.type.name|ljust:"10"}}{{ r.lastcall_ends|default:"None " }}{% endif %} {{ r.review_request.doc.name }}-{% if r.review_request.requested_rev %}{{ r.review_request.requested_rev }}{% else %}{{ r.review_request.doc.rev }}{% endif %}{% if r.earlier_review_mark %} {{ r.earlier_review_mark }}{% endif %}{% endfor %} + +* Other revision previously reviewed +** This revision already reviewed + +{% if rotation_list %}Next in the reviewer rotation: + +{% for p in rotation_list %} {{ p }} +{% endfor %}{% endif %} +The LC and Telechat review templates are included below: +------------------------------------------------------- + +-- Begin LC Template -- +I am the assigned Gen-ART reviewer for this draft. The General Area +Review Team (Gen-ART) reviews all IETF documents being processed +by the IESG for the IETF Chair. Please treat these comments just +like any other last call comments. + +For more information, please see the FAQ at + +. + +Document: +Reviewer: +Review Date: +IETF LC End Date: +IESG Telechat date: (if known) + +Summary: + +Major issues: + +Minor issues: + +Nits/editorial comments: + +-- End LC Template -- + +-- Begin Telechat Template -- +I am the assigned Gen-ART reviewer for this draft. The General Area +Review Team (Gen-ART) reviews all IETF documents being processed +by the IESG for the IETF Chair. Please wait for direction from your +document shepherd or AD before posting a new version of the draft. + +For more information, please see the FAQ at + +. + +Document: +Reviewer: +Review Date: +IETF LC End Date: +IESG Telechat date: (if known) + +Summary: + +Major issues: + +Minor issues: + +Nits/editorial comments: + +-- End Telechat Template -- +{% endautoescape %} + +""") + + DBTemplate.objects.filter(pk=184).update(content="""{% autoescape off %}Subject: Assignments + +Review instructions and related resources are at: +http://tools.ietf.org/area/sec/trac/wiki/SecDirReview{% for r in review_assignments %}{% ifchanged r.section %} + +{{r.section}} + +{% if r.section == 'Early review requests:' %}Reviewer Due Draft{% else %}Reviewer LC end Draft{% endif %}{% endifchanged %} +{{ r.reviewer.person.plain_name|ljust:"22" }}{{ r.earlier_review|yesno:'R, , ' }}{% if r.section == 'Early review requests:' %}{{ r.review_request.deadline|date:"Y-m-d" }}{% else %}{{ r.lastcall_ends|default:"None " }}{% endif %} {{ r.review_request.doc.name }}-{% if r.review_request.requested_rev %}{{ r.review_request.requested_rev }}{% else %}{{ r.review_request.doc.rev }}{% endif %}{% endfor %} + +{% if rotation_list %}Next in the reviewer rotation: + +{% for p in rotation_list %} {{ p }} +{% endfor %}{% endif %}{% endautoescape %} + +""") + + DBTemplate.objects.filter(pk=185).update(content="""{% autoescape off %}Subject: Open review assignments in {{group.acronym}} + +Review instructions and related resources are at: + + +The following reviewers have assignments:{% for r in review_assignments %}{% ifchanged r.section %} + +{{r.section}} + +{% if r.section == 'Early review requests:' %}Reviewer Due Draft{% else %}Reviewer LC end Draft{% endif %}{% endifchanged %} +{{ r.reviewer.person.plain_name|ljust:"22" }} {% if r.section == 'Early review requests:' %}{{ r.review_request.deadline|date:"Y-m-d" }}{% else %}{{ r.lastcall_ends|default:"None " }}{% endif %} {{ r.review_request.doc.name }}-{% if r.review_request.requested_rev %}{{ r.review_request.requested_rev }}{% else %}{{ r.review_request.doc.rev }}{% endif %} {{ r.earlier_review_mark }}{% endfor %} + +* Other revision previously reviewed +** This revision already reviewed + +{% if rotation_list %}Next in the reviewer rotation: + +{% for p in rotation_list %} {{ p }} +{% endfor %}{% endif %}{% endautoescape %} + +""") + + +class Migration(migrations.Migration): + + dependencies = [ + ('dbtemplate', '0004_adjust_assignment_email_summary_templates'), + ] + + operations = [ + migrations.RunPython(forward,reverse), + ] diff --git a/ietf/group/tests_review.py b/ietf/group/tests_review.py index b351ac032..2b0d159c0 100644 --- a/ietf/group/tests_review.py +++ b/ietf/group/tests_review.py @@ -238,19 +238,24 @@ class ReviewTests(TestCase): def test_email_open_review_assignments(self): review_req1 = ReviewRequestFactory() - ReviewAssignmentFactory(review_request=review_req1,reviewer=EmailFactory(person__user__username='marschairman')) + review_assignment_completed = ReviewAssignmentFactory(review_request=review_req1,reviewer=EmailFactory(person__user__username='marschairman'), state_id='completed', reviewed_rev=0) + ReviewAssignmentFactory(review_request=review_req1,reviewer=review_assignment_completed.reviewer) DBTemplateFactory.create(path='/group/defaults/email/open_assignments.txt', type_id='django', content = """ - {% autoescape off %} - Reviewer Deadline Draft - {% for r in review_assignments %}{{ r.reviewer.person.plain_name|ljust:"22" }} {{ r.review_request.deadline|date:"Y-m-d" }} {{ r.review_request.doc.name }}-{% if r.review_request.requested_rev %}{{ r.review_request.requested_rev }}{% else %}{{ r.review_request.doc.rev }}{% endif %} - {% endfor %} - {% if rotation_list %}Next in the reviewer rotation: + {% autoescape off %}Subject: Open review assignments in {{group.acronym}} - {% for p in rotation_list %} {{ p }} - {% endfor %}{% endif %} - {% endautoescape %} + The following reviewers have assignments:{% for r in review_assignments %}{% ifchanged r.section %} + + {{r.section}} + + {% if r.section == 'Early review requests:' %}Reviewer Due Draft{% else %}Reviewer LC end Draft{% endif %}{% endifchanged %} + {{ r.reviewer.person.plain_name|ljust:"22" }} {% if r.section == 'Early review requests:' %}{{ r.review_request.deadline|date:"Y-m-d" }}{% else %}{{ r.lastcall_ends|default:"None " }}{% endif %} {{ r.review_request.doc.name }}-{% if r.review_request.requested_rev %}{{ r.review_request.requested_rev }}{% else %}{{ r.review_request..doc.rev }}{% endif %} {{ r.earlier_reviews }}{% endfor %} + + {% if rotation_list %}Next in the reviewer rotation: + + {% for p in rotation_list %} {{ p }} + {% endfor %}{% endif %}{% endautoescape %} """) group = review_req1.team @@ -266,6 +271,7 @@ class ReviewTests(TestCase): q = PyQuery(r.content) generated_text = q("[name=body]").text() self.assertTrue(review_req1.doc.name in generated_text) + self.assertTrue('(-0 lc review)' in generated_text) # previous completed assignment self.assertTrue(six.text_type(Person.objects.get(user__username="marschairman")) in generated_text) empty_outbox() diff --git a/ietf/group/views.py b/ietf/group/views.py index 371126ec5..b93c49c0f 100644 --- a/ietf/group/views.py +++ b/ietf/group/views.py @@ -52,7 +52,6 @@ from simple_history.utils import update_change_reason from django import forms from django.conf import settings from django.contrib.auth.decorators import login_required -from django.db.models.aggregates import Max from django.db.models import Q, Count from django.http import HttpResponse, HttpResponseForbidden, Http404, HttpResponseRedirect, JsonResponse from django.shortcuts import render, redirect, get_object_or_404 @@ -1610,12 +1609,8 @@ def email_open_review_assignments(request, acronym, group_type=None): r.lastcall_ends = e and e.expires.date().isoformat() r.earlier_review = ReviewAssignment.objects.filter(review_request__doc=r.review_request.doc,reviewer__in=r.reviewer.person.email_set.all(),state="completed") if r.earlier_review: - req_rev = r.review_request.requested_rev or r.review_request.doc.rev - earlier_review_rev = r.earlier_review.aggregate(Max('reviewed_rev'))['reviewed_rev__max'] - if req_rev == earlier_review_rev: - r.earlier_review_mark = '**' - else: - r.earlier_review_mark = '*' + earlier_reviews_formatted = ['-{} {} completed'.format(ra.reviewed_rev, ra.review_request.type.slug) for ra in r.earlier_review] + r.earlier_reviews = '({})'.format(', '.join(earlier_reviews_formatted)) review_assignments.sort(key=lambda r: r.section_order)