Skip to content

Commit ef1e800

Browse files
author
Sasha Romijn
committed
Fix ietf-tools#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
1 parent a7a2ee4 commit ef1e800

3 files changed

Lines changed: 301 additions & 17 deletions

File tree

‎ietf/dbtemplate/migrations/0005_adjust_assignment_email_summary_templates_2526.py‎

Lines changed: 283 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,283 @@
1+
# Copyright The IETF Trust 2019, All Rights Reserved
2+
# -*- coding: utf-8 -*-
3+
# Generated by Django 1.11.20 on 2019-03-13 13:41
4+
5+
6+
from __future__ import absolute_import, print_function, unicode_literals
7+
8+
from django.db import migrations
9+
10+
def forward(apps, schema_editor):
11+
DBTemplate = apps.get_model('dbtemplate','DBTemplate')
12+
13+
DBTemplate.objects.filter(pk=182).update(content="""{% autoescape off %}Subject: Open review assignments in {{group.acronym}}
14+
15+
The following reviewers have assignments:{% for r in review_assignments %}{% ifchanged r.section %}
16+
17+
{{r.section}}
18+
19+
{% if r.section == 'Early review requests:' %}Reviewer Due Draft{% else %}Reviewer LC end Draft{% endif %}{% endifchanged %}
20+
{{ 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 %}
21+
22+
{% if rotation_list %}Next in the reviewer rotation:
23+
24+
{% for p in rotation_list %} {{ p }}
25+
{% endfor %}{% endif %}{% endautoescape %}
26+
""")
27+
28+
DBTemplate.objects.filter(pk=183).update(content="""{% autoescape off %}Subject: Review Assignments
29+
30+
Hi all,
31+
32+
The following reviewers have assignments:{% for r in review_assignments %}{% ifchanged r.section %}
33+
34+
{{r.section}}
35+
36+
{% if r.section == 'Early review requests:' %}Reviewer Due Draft{% else %}Reviewer Type LC end Draft{% endif %}{% endifchanged %}
37+
{{ 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 %}
38+
39+
{% if rotation_list %}Next in the reviewer rotation:
40+
41+
{% for p in rotation_list %} {{ p }}
42+
{% endfor %}{% endif %}
43+
The LC and Telechat review templates are included below:
44+
-------------------------------------------------------
45+
46+
-- Begin LC Template --
47+
I am the assigned Gen-ART reviewer for this draft. The General Area
48+
Review Team (Gen-ART) reviews all IETF documents being processed
49+
by the IESG for the IETF Chair. Please treat these comments just
50+
like any other last call comments.
51+
52+
For more information, please see the FAQ at
53+
54+
<https://trac.ietf.org/trac/gen/wiki/GenArtfaq>.
55+
56+
Document:
57+
Reviewer:
58+
Review Date:
59+
IETF LC End Date:
60+
IESG Telechat date: (if known)
61+
62+
Summary:
63+
64+
Major issues:
65+
66+
Minor issues:
67+
68+
Nits/editorial comments:
69+
70+
-- End LC Template --
71+
72+
-- Begin Telechat Template --
73+
I am the assigned Gen-ART reviewer for this draft. The General Area
74+
Review Team (Gen-ART) reviews all IETF documents being processed
75+
by the IESG for the IETF Chair. Please wait for direction from your
76+
document shepherd or AD before posting a new version of the draft.
77+
78+
For more information, please see the FAQ at
79+
80+
<https://trac.ietf.org/trac/gen/wiki/GenArtfaq>.
81+
82+
Document:
83+
Reviewer:
84+
Review Date:
85+
IETF LC End Date:
86+
IESG Telechat date: (if known)
87+
88+
Summary:
89+
90+
Major issues:
91+
92+
Minor issues:
93+
94+
Nits/editorial comments:
95+
96+
-- End Telechat Template --
97+
{% endautoescape %}
98+
99+
""")
100+
101+
DBTemplate.objects.filter(pk=184).update(content="""{% autoescape off %}Subject: Assignments
102+
103+
Review instructions and related resources are at:
104+
http://tools.ietf.org/area/sec/trac/wiki/SecDirReview{% for r in review_assignments %}{% ifchanged r.section %}
105+
106+
{{r.section}}
107+
108+
{% if r.section == 'Early review requests:' %}Reviewer Due Draft{% else %}Reviewer LC end Draft{% endif %}{% endifchanged %}
109+
{{ 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 %}
110+
111+
{% if rotation_list %}Next in the reviewer rotation:
112+
113+
{% for p in rotation_list %} {{ p }}
114+
{% endfor %}{% endif %}{% endautoescape %}
115+
116+
""")
117+
118+
DBTemplate.objects.filter(pk=185).update(content="""{% autoescape off %}Subject: Open review assignments in {{group.acronym}}
119+
120+
Review instructions and related resources are at:
121+
<https://trac.ietf.org/trac/ops/wiki/Directorates>
122+
123+
The following reviewers have assignments:{% for r in review_assignments %}{% ifchanged r.section %}
124+
125+
{{r.section}}
126+
127+
{% if r.section == 'Early review requests:' %}Reviewer Due Draft{% else %}Reviewer LC end Draft{% endif %}{% endifchanged %}
128+
{{ 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 %}
129+
130+
{% if rotation_list %}Next in the reviewer rotation:
131+
132+
{% for p in rotation_list %} {{ p }}
133+
{% endfor %}{% endif %}{% endautoescape %}
134+
135+
""")
136+
137+
138+
def reverse(apps, schema_editor):
139+
DBTemplate = apps.get_model('dbtemplate','DBTemplate')
140+
141+
DBTemplate.objects.filter(pk=182).update(content="""{% autoescape off %}Subject: Open review assignments in {{group.acronym}}
142+
143+
The following reviewers have assignments:{% for r in review_assignments %}{% ifchanged r.section %}
144+
145+
{{r.section}}
146+
147+
{% if r.section == 'Early review requests:' %}Reviewer Due Draft{% else %}Reviewer LC end Draft{% endif %}{% endifchanged %}
148+
{{ 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 %}
149+
150+
* Other revision previously reviewed
151+
** This revision already reviewed
152+
153+
{% if rotation_list %}Next in the reviewer rotation:
154+
155+
{% for p in rotation_list %} {{ p }}
156+
{% endfor %}{% endif %}{% endautoescape %}
157+
""")
158+
159+
DBTemplate.objects.filter(pk=183).update(content="""{% autoescape off %}Subject: Review Assignments
160+
161+
Hi all,
162+
163+
The following reviewers have assignments:{% for r in review_assignments %}{% ifchanged r.section %}
164+
165+
{{r.section}}
166+
167+
{% if r.section == 'Early review requests:' %}Reviewer Due Draft{% else %}Reviewer Type LC end Draft{% endif %}{% endifchanged %}
168+
{{ 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 %}
169+
170+
* Other revision previously reviewed
171+
** This revision already reviewed
172+
173+
{% if rotation_list %}Next in the reviewer rotation:
174+
175+
{% for p in rotation_list %} {{ p }}
176+
{% endfor %}{% endif %}
177+
The LC and Telechat review templates are included below:
178+
-------------------------------------------------------
179+
180+
-- Begin LC Template --
181+
I am the assigned Gen-ART reviewer for this draft. The General Area
182+
Review Team (Gen-ART) reviews all IETF documents being processed
183+
by the IESG for the IETF Chair. Please treat these comments just
184+
like any other last call comments.
185+
186+
For more information, please see the FAQ at
187+
188+
<https://trac.ietf.org/trac/gen/wiki/GenArtfaq>.
189+
190+
Document:
191+
Reviewer:
192+
Review Date:
193+
IETF LC End Date:
194+
IESG Telechat date: (if known)
195+
196+
Summary:
197+
198+
Major issues:
199+
200+
Minor issues:
201+
202+
Nits/editorial comments:
203+
204+
-- End LC Template --
205+
206+
-- Begin Telechat Template --
207+
I am the assigned Gen-ART reviewer for this draft. The General Area
208+
Review Team (Gen-ART) reviews all IETF documents being processed
209+
by the IESG for the IETF Chair. Please wait for direction from your
210+
document shepherd or AD before posting a new version of the draft.
211+
212+
For more information, please see the FAQ at
213+
214+
<https://trac.ietf.org/trac/gen/wiki/GenArtfaq>.
215+
216+
Document:
217+
Reviewer:
218+
Review Date:
219+
IETF LC End Date:
220+
IESG Telechat date: (if known)
221+
222+
Summary:
223+
224+
Major issues:
225+
226+
Minor issues:
227+
228+
Nits/editorial comments:
229+
230+
-- End Telechat Template --
231+
{% endautoescape %}
232+
233+
""")
234+
235+
DBTemplate.objects.filter(pk=184).update(content="""{% autoescape off %}Subject: Assignments
236+
237+
Review instructions and related resources are at:
238+
http://tools.ietf.org/area/sec/trac/wiki/SecDirReview{% for r in review_assignments %}{% ifchanged r.section %}
239+
240+
{{r.section}}
241+
242+
{% if r.section == 'Early review requests:' %}Reviewer Due Draft{% else %}Reviewer LC end Draft{% endif %}{% endifchanged %}
243+
{{ 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 %}
244+
245+
{% if rotation_list %}Next in the reviewer rotation:
246+
247+
{% for p in rotation_list %} {{ p }}
248+
{% endfor %}{% endif %}{% endautoescape %}
249+
250+
""")
251+
252+
DBTemplate.objects.filter(pk=185).update(content="""{% autoescape off %}Subject: Open review assignments in {{group.acronym}}
253+
254+
Review instructions and related resources are at:
255+
<https://trac.ietf.org/trac/ops/wiki/Directorates>
256+
257+
The following reviewers have assignments:{% for r in review_assignments %}{% ifchanged r.section %}
258+
259+
{{r.section}}
260+
261+
{% if r.section == 'Early review requests:' %}Reviewer Due Draft{% else %}Reviewer LC end Draft{% endif %}{% endifchanged %}
262+
{{ 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 %}
263+
264+
* Other revision previously reviewed
265+
** This revision already reviewed
266+
267+
{% if rotation_list %}Next in the reviewer rotation:
268+
269+
{% for p in rotation_list %} {{ p }}
270+
{% endfor %}{% endif %}{% endautoescape %}
271+
272+
""")
273+
274+
275+
class Migration(migrations.Migration):
276+
277+
dependencies = [
278+
('dbtemplate', '0004_adjust_assignment_email_summary_templates'),
279+
]
280+
281+
operations = [
282+
migrations.RunPython(forward,reverse),
283+
]

‎ietf/group/tests_review.py‎

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -238,19 +238,24 @@ def test_manage_review_requests(self):
238238

239239
def test_email_open_review_assignments(self):
240240
review_req1 = ReviewRequestFactory()
241-
ReviewAssignmentFactory(review_request=review_req1,reviewer=EmailFactory(person__user__username='marschairman'))
241+
review_assignment_completed = ReviewAssignmentFactory(review_request=review_req1,reviewer=EmailFactory(person__user__username='marschairman'), state_id='completed', reviewed_rev=0)
242+
ReviewAssignmentFactory(review_request=review_req1,reviewer=review_assignment_completed.reviewer)
242243
DBTemplateFactory.create(path='/group/defaults/email/open_assignments.txt',
243244
type_id='django',
244245
content = """
245-
{% autoescape off %}
246-
Reviewer Deadline Draft
247-
{% 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 %}
248-
{% endfor %}
249-
{% if rotation_list %}Next in the reviewer rotation:
250-
251-
{% for p in rotation_list %} {{ p }}
252-
{% endfor %}{% endif %}
253-
{% endautoescape %}
246+
{% autoescape off %}Subject: Open review assignments in {{group.acronym}}
247+
248+
The following reviewers have assignments:{% for r in review_assignments %}{% ifchanged r.section %}
249+
250+
{{r.section}}
251+
252+
{% if r.section == 'Early review requests:' %}Reviewer Due Draft{% else %}Reviewer LC end Draft{% endif %}{% endifchanged %}
253+
{{ 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 %}
254+
255+
{% if rotation_list %}Next in the reviewer rotation:
256+
257+
{% for p in rotation_list %} {{ p }}
258+
{% endfor %}{% endif %}{% endautoescape %}
254259
""")
255260

256261
group = review_req1.team
@@ -266,6 +271,7 @@ def test_email_open_review_assignments(self):
266271
q = PyQuery(r.content)
267272
generated_text = q("[name=body]").text()
268273
self.assertTrue(review_req1.doc.name in generated_text)
274+
self.assertTrue('(-0 lc review)' in generated_text) # previous completed assignment
269275
self.assertTrue(six.text_type(Person.objects.get(user__username="marschairman")) in generated_text)
270276

271277
empty_outbox()

‎ietf/group/views.py‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,6 @@
5252
from django import forms
5353
from django.conf import settings
5454
from django.contrib.auth.decorators import login_required
55-
from django.db.models.aggregates import Max
5655
from django.db.models import Q, Count
5756
from django.http import HttpResponse, HttpResponseForbidden, Http404, HttpResponseRedirect, JsonResponse
5857
from django.shortcuts import render, redirect, get_object_or_404
@@ -1610,12 +1609,8 @@ def email_open_review_assignments(request, acronym, group_type=None):
16101609
r.lastcall_ends = e and e.expires.date().isoformat()
16111610
r.earlier_review = ReviewAssignment.objects.filter(review_request__doc=r.review_request.doc,reviewer__in=r.reviewer.person.email_set.all(),state="completed")
16121611
if r.earlier_review:
1613-
req_rev = r.review_request.requested_rev or r.review_request.doc.rev
1614-
earlier_review_rev = r.earlier_review.aggregate(Max('reviewed_rev'))['reviewed_rev__max']
1615-
if req_rev == earlier_review_rev:
1616-
r.earlier_review_mark = '**'
1617-
else:
1618-
r.earlier_review_mark = '*'
1612+
earlier_reviews_formatted = ['-{} {} completed'.format(ra.reviewed_rev, ra.review_request.type.slug) for ra in r.earlier_review]
1613+
r.earlier_reviews = '({})'.format(', '.join(earlier_reviews_formatted))
16191614

16201615
review_assignments.sort(key=lambda r: r.section_order)
16211616

0 commit comments

Comments
 (0)