Skip to content

Commit 0a76688

Browse files
committed
Merged in [15675] from rjsparks@nostrum.com:
Let review teams opt in to poking a responsible AD when unhappy reviews are submitted. Fixes ietf-tools#2544. - Legacy-Id: 15678 Note: SVN reference [15675] has been migrated to Git commit c7bf147
2 parents 542a85d + c7bf147 commit 0a76688

10 files changed

Lines changed: 178 additions & 15 deletions

File tree

ietf/doc/tests_review.py

Lines changed: 31 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -573,7 +573,7 @@ def test_complete_review_upload_content(self):
573573
test_file.name = "unnamed"
574574

575575
r = self.client.post(url, data={
576-
"result": ReviewResultName.objects.get(reviewteamsettings__group=review_req.team, slug="ready").pk,
576+
"result": ReviewResultName.objects.get(reviewteamsettings_review_results_set__group=review_req.team, slug="ready").pk,
577577
"state": ReviewRequestStateName.objects.get(slug="completed").pk,
578578
"reviewed_rev": review_req.doc.rev,
579579
"review_submission": "upload",
@@ -627,7 +627,7 @@ def test_complete_review_enter_content(self):
627627
empty_outbox()
628628

629629
r = self.client.post(url, data={
630-
"result": ReviewResultName.objects.get(reviewteamsettings__group=review_req.team, slug="ready").pk,
630+
"result": ReviewResultName.objects.get(reviewteamsettings_review_results_set__group=review_req.team, slug="ready").pk,
631631
"state": ReviewRequestStateName.objects.get(slug="completed").pk,
632632
"reviewed_rev": review_req.doc.rev,
633633
"review_submission": "enter",
@@ -649,6 +649,30 @@ def test_complete_review_enter_content(self):
649649

650650
self.assertTrue(settings.MAILING_LIST_ARCHIVE_URL in review_req.review.external_url)
651651

652+
def test_complete_notify_ad(self):
653+
review_req, url = self.setup_complete_review_test()
654+
review_req.team.reviewteamsettings.notify_ad_when.add(ReviewResultName.objects.get(slug='issues'))
655+
# TODO - it's a little surprising that the factories so far didn't give this doc an ad
656+
review_req.doc.ad = PersonFactory()
657+
review_req.doc.save_with_history([DocEvent.objects.create(doc=review_req.doc, rev=review_req.doc.rev, by=review_req.reviewer.person, type='changed_document',desc='added an AD')])
658+
login_testing_unauthorized(self, review_req.reviewer.person.user.username, url)
659+
660+
empty_outbox()
661+
662+
r = self.client.post(url, data={
663+
"result": ReviewResultName.objects.get(reviewteamsettings_review_results_set__group=review_req.team, slug="issues").pk,
664+
"state": ReviewRequestStateName.objects.get(slug="completed").pk,
665+
"reviewed_rev": review_req.doc.rev,
666+
"review_submission": "enter",
667+
"review_content": "This is a review\nwith two lines",
668+
"review_url": "",
669+
"review_file": "",
670+
})
671+
self.assertEqual(r.status_code, 302)
672+
673+
self.assertEqual(len(outbox), 2)
674+
self.assertIn('Has Issues', outbox[-1]['Subject'])
675+
652676
@patch('requests.get')
653677
def test_complete_review_link_to_mailing_list(self, mock):
654678
# Mock up the url response for the request.get() call to retrieve the mailing list url
@@ -665,7 +689,7 @@ def test_complete_review_link_to_mailing_list(self, mock):
665689
empty_outbox()
666690

667691
r = self.client.post(url, data={
668-
"result": ReviewResultName.objects.get(reviewteamsettings__group=review_req.team, slug="ready").pk,
692+
"result": ReviewResultName.objects.get(reviewteamsettings_review_results_set__group=review_req.team, slug="ready").pk,
669693
"state": ReviewRequestStateName.objects.get(slug="completed").pk,
670694
"reviewed_rev": review_req.doc.rev,
671695
"review_submission": "link",
@@ -693,7 +717,7 @@ def test_partially_complete_review(self):
693717
empty_outbox()
694718

695719
r = self.client.post(url, data={
696-
"result": ReviewResultName.objects.get(reviewteamsettings__group=review_req.team, slug="ready").pk,
720+
"result": ReviewResultName.objects.get(reviewteamsettings_review_results_set__group=review_req.team, slug="ready").pk,
697721
"state": ReviewRequestStateName.objects.get(slug="part-completed").pk,
698722
"reviewed_rev": review_req.doc.rev,
699723
"review_submission": "enter",
@@ -734,7 +758,7 @@ def test_partially_complete_review(self):
734758
url = urlreverse('ietf.doc.views_review.complete_review', kwargs={ "name": review_req.doc.name, "request_id": review_req.pk })
735759

736760
r = self.client.post(url, data={
737-
"result": ReviewResultName.objects.get(reviewteamsettings__group=review_req.team, slug="ready").pk,
761+
"result": ReviewResultName.objects.get(reviewteamsettings_review_results_set__group=review_req.team, slug="ready").pk,
738762
"state": ReviewRequestStateName.objects.get(slug="completed").pk,
739763
"reviewed_rev": review_req.doc.rev,
740764
"review_submission": "enter",
@@ -766,7 +790,7 @@ def test_revise_review_enter_content(self):
766790
empty_outbox()
767791

768792
r = self.client.post(url, data={
769-
"result": ReviewResultName.objects.get(reviewteamsettings__group=review_req.team, slug="ready").pk,
793+
"result": ReviewResultName.objects.get(reviewteamsettings_review_results_set__group=review_req.team, slug="ready").pk,
770794
"state": ReviewRequestStateName.objects.get(slug="completed").pk,
771795
"reviewed_rev": review_req.doc.rev,
772796
"review_submission": "enter",
@@ -791,7 +815,7 @@ def test_revise_review_enter_content(self):
791815
# revise again
792816
empty_outbox()
793817
r = self.client.post(url, data={
794-
"result": ReviewResultName.objects.get(reviewteamsettings__group=review_req.team, slug="ready").pk,
818+
"result": ReviewResultName.objects.get(reviewteamsettings_review_results_set__group=review_req.team, slug="ready").pk,
795819
"state": ReviewRequestStateName.objects.get(slug="part-completed").pk,
796820
"reviewed_rev": review_req.doc.rev,
797821
"review_submission": "enter",

ietf/doc/views_review.py

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
from django.http import HttpResponseForbidden, JsonResponse
1313
from django.shortcuts import render, get_object_or_404, redirect
1414
from django import forms
15+
from django.conf import settings
1516
from django.contrib.auth.decorators import login_required
1617
from django.utils.html import mark_safe
1718
from django.core.exceptions import ValidationError
@@ -25,6 +26,7 @@
2526
from ietf.group.models import Group
2627
from ietf.ietfauth.utils import is_authorized_in_doc_stream, user_is_person, has_role
2728
from ietf.message.models import Message
29+
from ietf.message.utils import infer_message
2830
from ietf.person.fields import PersonEmailChoiceField, SearchablePersonField
2931
from ietf.review.utils import (active_review_teams, assign_review_request_to_reviewer,
3032
can_request_review_of_doc, can_manage_review_requests_for_team,
@@ -423,7 +425,7 @@ def __init__(self, review_req, is_reviewer, *args, **kwargs):
423425
" ".join("<a class=\"rev label label-default {0}\" title=\"{2:%Y-%m-%d}\">{1}</a>".format('', *r)
424426
for i, r in enumerate(known_revisions)))
425427

426-
self.fields["result"].queryset = self.fields["result"].queryset.filter(reviewteamsettings__group=review_req.team)
428+
self.fields["result"].queryset = self.fields["result"].queryset.filter(reviewteamsettings_review_results_set__group=review_req.team)
427429

428430
def format_submission_choice(label):
429431
if revising_review:
@@ -643,6 +645,19 @@ def complete_review(request, name, request_id):
643645
review.external_url = mailarch.construct_message_url(list_name, email.utils.unquote(msg["Message-ID"]))
644646
review.save_with_history([close_event])
645647

648+
if review_req.result in review_req.team.reviewteamsettings.notify_ad_when.all():
649+
(to, cc) = gather_address_lists('review_notify_ad',review_req = review_req)
650+
msg_txt = render_to_string("review/notify_ad.txt", {
651+
"to": to,
652+
"cc": cc,
653+
"review_req": review_req,
654+
"settings": settings,
655+
})
656+
msg = infer_message(msg_txt)
657+
msg.by = request.user.person
658+
msg.save()
659+
send_mail_message(request, msg)
660+
646661
return redirect("ietf.doc.views_doc.document_main", name=review_req.review.name)
647662
else:
648663
initial={
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
# -*- coding: utf-8 -*-
2+
# Generated by Django 1.11.16 on 2018-11-02 11:34
3+
from __future__ import unicode_literals
4+
5+
from django.db import migrations
6+
7+
def forward(apps, schema_editor):
8+
MailTrigger = apps.get_model('mailtrigger','MailTrigger')
9+
Recipient = apps.get_model('mailtrigger', 'Recipient')
10+
11+
Recipient.objects.create(
12+
slug = 'review_doc_ad',
13+
desc = "The reviewed document's responsible area director",
14+
template = '{% if review_req.doc.ad %}{{review_req.doc.ad.email_address}}{% endif %}'
15+
)
16+
17+
review_notify_ad = MailTrigger.objects.create(
18+
slug = 'review_notify_ad',
19+
desc = 'Recipients when a team notifies area directors when a review with one of a certain set of results (typically results indicating problem) is submitted',
20+
)
21+
review_notify_ad.to.set(Recipient.objects.filter(slug='review_doc_ad'))
22+
23+
24+
def reverse(apps, schema_editor):
25+
MailTrigger = apps.get_model('mailtrigger','MailTrigger')
26+
Recipient = apps.get_model('mailtrigger', 'Recipient')
27+
28+
MailTrigger.objects.filter(slug='review_notify_ad').delete()
29+
Recipient.objects.filter(slug='review_doc_ad').delete()
30+
31+
class Migration(migrations.Migration):
32+
33+
dependencies = [
34+
('mailtrigger', '0002_conflrev_changes'),
35+
('review', '0003_add_notify_ad_when'),
36+
]
37+
38+
operations = [
39+
migrations.RunPython(forward, reverse)
40+
]

ietf/name/fixtures/names.json

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3569,6 +3569,17 @@
35693569
"model": "mailtrigger.mailtrigger",
35703570
"pk": "review_completed"
35713571
},
3572+
{
3573+
"fields": {
3574+
"cc": [],
3575+
"desc": "Recipients when a team notifies area directors when a review with one of a certain set of results (typically results indicating problem) is submitted",
3576+
"to": [
3577+
"review_doc_ad"
3578+
]
3579+
},
3580+
"model": "mailtrigger.mailtrigger",
3581+
"pk": "review_notify_ad"
3582+
},
35723583
{
35733584
"fields": {
35743585
"cc": [
@@ -4181,6 +4192,14 @@
41814192
"model": "mailtrigger.recipient",
41824193
"pk": "nominee"
41834194
},
4195+
{
4196+
"fields": {
4197+
"desc": "The reviewed document's responsible area director",
4198+
"template": "{% if review_req.doc.ad %}{{review_req.doc.ad.email_address}}{% endif %}"
4199+
},
4200+
"model": "mailtrigger.recipient",
4201+
"pk": "review_doc_ad"
4202+
},
41844203
{
41854204
"fields": {
41864205
"desc": "The .all alias for the document being reviewed",
@@ -10475,7 +10494,7 @@
1047510494
"fields": {
1047610495
"command": "xym",
1047710496
"switch": "--version",
10478-
"time": "2018-10-19T00:08:03.034",
10497+
"time": "2018-11-01T00:08:56.002",
1047910498
"used": true,
1048010499
"version": "xym 0.4"
1048110500
},
@@ -10486,7 +10505,7 @@
1048610505
"fields": {
1048710506
"command": "pyang",
1048810507
"switch": "--version",
10489-
"time": "2018-10-19T00:08:03.673",
10508+
"time": "2018-11-01T00:08:57.320",
1049010509
"used": true,
1049110510
"version": "pyang 1.7.5"
1049210511
},
@@ -10497,7 +10516,7 @@
1049710516
"fields": {
1049810517
"command": "yanglint",
1049910518
"switch": "--version",
10500-
"time": "2018-10-19T00:08:03.887",
10519+
"time": "2018-11-01T00:08:57.507",
1050110520
"used": true,
1050210521
"version": "yanglint 0.14.80"
1050310522
},
@@ -10508,9 +10527,9 @@
1050810527
"fields": {
1050910528
"command": "xml2rfc",
1051010529
"switch": "--version",
10511-
"time": "2018-10-19T00:08:04.519",
10530+
"time": "2018-11-01T00:08:59.405",
1051210531
"used": true,
10513-
"version": "xml2rfc 2.11.1"
10532+
"version": "xml2rfc 2.12.3"
1051410533
},
1051510534
"model": "utils.versioninfo",
1051610535
"pk": 4

ietf/review/admin.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,6 @@ class ReviewTeamSettingsAdmin(admin.ModelAdmin):
6363
list_display = ["group", ]
6464
search_fields = ["group__acronym", ]
6565
raw_id_fields = ["group", ]
66-
filter_horizontal = ["review_types", "review_results", ]
66+
filter_horizontal = ["review_types", "review_results", "notify_ad_when"]
6767

6868
admin.site.register(ReviewTeamSettings, ReviewTeamSettingsAdmin)
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
# -*- coding: utf-8 -*-
2+
# Generated by Django 1.11.16 on 2018-11-02 10:10
3+
from __future__ import unicode_literals
4+
5+
from django.db import migrations, models
6+
import ietf.review.models
7+
8+
9+
class Migration(migrations.Migration):
10+
11+
dependencies = [
12+
('name', '0004_add_prefix_to_doctypenames'),
13+
('review', '0002_unavailableperiod_reason'),
14+
]
15+
16+
operations = [
17+
migrations.AddField(
18+
model_name='reviewteamsettings',
19+
name='notify_ad_when',
20+
field=models.ManyToManyField(related_name='reviewteamsettings_notify_ad_set', to='name.ReviewResultName'),
21+
),
22+
migrations.AlterField(
23+
model_name='reviewteamsettings',
24+
name='review_results',
25+
field=models.ManyToManyField(default=ietf.review.models.get_default_review_results, related_name='reviewteamsettings_review_results_set', to='name.ReviewResultName'),
26+
),
27+
]
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
# -*- coding: utf-8 -*-
2+
# Generated by Django 1.11.16 on 2018-11-02 10:20
3+
from __future__ import unicode_literals
4+
5+
from django.db import migrations
6+
7+
def forward(apps, schema_editor):
8+
ReviewTeamSettings = apps.get_model('review','ReviewTeamSettings')
9+
ReviewTeamSettings.objects.get(group__acronym='secdir').notify_ad_when.set(['serious-issues', 'issues', 'not-ready'])
10+
11+
def reverse(apps, schema_editor):
12+
ReviewTeamSettings = apps.get_model('review','ReviewTeamSettings')
13+
ReviewTeamSettings.objects.get(group__acronym='secdir').notify_ad_when.set([])
14+
15+
class Migration(migrations.Migration):
16+
17+
dependencies = [
18+
('review', '0003_add_notify_ad_when'),
19+
]
20+
21+
operations = [
22+
migrations.RunPython(forward, reverse)
23+
]

ietf/review/models.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,8 @@ class ReviewTeamSettings(models.Model):
160160
group = OneToOneField(Group)
161161
autosuggest = models.BooleanField(default=True, verbose_name="Automatically suggest possible review requests")
162162
review_types = models.ManyToManyField(ReviewTypeName, default=get_default_review_types)
163-
review_results = models.ManyToManyField(ReviewResultName, default=get_default_review_results)
163+
review_results = models.ManyToManyField(ReviewResultName, default=get_default_review_results, related_name='reviewteamsettings_review_results_set')
164+
notify_ad_when = models.ManyToManyField(ReviewResultName, related_name='reviewteamsettings_notify_ad_set')
164165

165166
def __unicode__(self):
166167
return u"%s" % (self.group.acronym,)

ietf/review/resources.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,7 @@ class ReviewTeamSettingsResource(ModelResource):
157157
group = ToOneField(GroupResource, 'group')
158158
review_types = ToManyField(ReviewTypeNameResource, 'review_types', null=True)
159159
review_results = ToManyField(ReviewResultNameResource, 'review_results', null=True)
160+
notify_ad_when = ToManyField(ReviewResultNameResource, 'notify_ad_when', null = True)
160161
class Meta:
161162
queryset = ReviewTeamSettings.objects.all()
162163
serializer = api.Serializer()
@@ -168,6 +169,7 @@ class Meta:
168169
"group": ALL_WITH_RELATIONS,
169170
"review_types": ALL_WITH_RELATIONS,
170171
"review_results": ALL_WITH_RELATIONS,
172+
"notify_ad_when": ALL_WITH_RELATIONS,
171173
}
172174
api.review.register(ReviewTeamSettingsResource())
173175

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
{% load ietf_filters %}{% autoescape off %}From: {{settings.DEFAULT_FROM_EMAIL}}
2+
To: {{to}}{% if cc %}
3+
Cc: {{cc}}{% endif %}
4+
Subject: "{{review_req.result}}" review submitted for {{review_req.doc}}{% if review_req.reviewed_rev %}-{{review_req.reviewed_rev}}{% endif %}
5+
6+
{{review_req.reviewer.person.name}} has submitted a "{{review_req.result}}" review result for {{review_req.doc}}{% if review_req.reviewed_rev %}-{{review_req.reviewed_rev}}{% endif %}.
7+
8+
The review is available at {{settings.IDTRACKER_BASE_URL}}{% url 'ietf.doc.views_doc.document_main' name=review_req.review.name %}
9+
10+
The document is available at {{settings.IDTRACKER_BASE_URL}}{% url 'ietf.doc.views_doc.document_main' name=review_req.doc.name %}
11+
12+
{% endautoescape %}

0 commit comments

Comments
 (0)