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

Fix #2354 - Make review_completed configurable per team and review type

Browse source 
This includes a migration to change mailtrigger slugs to be up to 64
characters instead of 32, as some slugs would not fit and require clunky
abbreviations.
A data migration creates triggers for existing teams, and they are also
created on the fly if a trigger does not exist yet, providing a safe
fallback for new review teams. The review_completed mailtrigger serves
as the template for new triggers.
This commit also includes tests for gather_address_lists(), as none
existed.

Commit ready for merge.
 - Legacy-Id: 16680
This commit is contained in:
Sasha Romijn 2019-08-30 17:40:55 +00:00
parent 88b7b45b0e
commit 1390ae073c
7 changed files with 165 additions and 7 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

17
ietf/doc/views_review.py
View file

@ -571,7 +571,17 @@ def complete_review(request, name, assignment_id):
if not (is_reviewer or can_manage_request):
return HttpResponseForbidden("You do not have permission to perform this action")
(to, cc) = gather_address_lists('review_completed', review_req = assignment.review_request)
team_acronym = assignment.review_request.team.acronym.lower()
request_type = assignment.review_request.type
mailtrigger_slug = 'review_completed_{}_{}'.format(team_acronym, request_type.slug)
# Description is only used if the mailtrigger does not exist yet.
mailtrigger_desc = 'Recipients when a {} {} review is completed'.format(team_acronym, request_type)
to, cc = gather_address_lists(
mailtrigger_slug,
create_from_slug_if_not_exists='review_completed',
desc_if_not_exists=mailtrigger_desc,
review_req=assignment.review_request
)
if request.method == "POST":
form = CompleteReviewForm(assignment, is_reviewer,
@ -588,7 +598,7 @@ def complete_review(request, name, assignment_id):
strip_prefix(assignment.review_request.doc.name, "draft-"),
form.cleaned_data["reviewed_rev"],
assignment.review_request.team.acronym,
assignment.review_request.type.slug,
request_type.slug,
xslugify(assignment.reviewer.person.ascii_parts()[3]),
datetime.date.today().isoformat(),
]
@ -761,7 +771,8 @@ def complete_review(request, name, assignment_id):
}
try:
initial['review_content'] = render_to_string('/group/%s/review/content_templates/%s.txt' % (assignment.review_request.team.acronym, assignment.review_request.type.slug), {'assignment':assignment,'today':datetime.date.today()})
initial['review_content'] = render_to_string('/group/%s/review/content_templates/%s.txt' % (assignment.review_request.team.acronym,
request_type.slug), {'assignment':assignment, 'today':datetime.date.today()})
except TemplateDoesNotExist:
pass

32
ietf/mailtrigger/migrations/0008_lengthen_mailtrigger_slug.py Normal file
View file

@ -0,0 +1,32 @@
# Copyright The IETF Trust 2015-2019, All Rights Reserved
# -*- coding: utf-8 -*-
# Generated by Django 1.11.23 on 2019-08-30 09:02
from __future__ import unicode_literals
from django.db import migrations, models
class Migration(migrations.Migration):
dependencies = [
('mailtrigger', '0007_add_review_mailtriggers'),
]
operations = [
migrations.AlterField(
model_name='mailtrigger',
name='slug',
field=models.CharField(max_length=64, primary_key=True, serialize=False),
),
# The above migration will not update the ManyToMany tables, which also reference
# the mailtrigger pk as varchar(32), so manual SQL is used.
# https://code.djangoproject.com/ticket/25012
migrations.RunSQL(
sql='ALTER TABLE `mailtrigger_mailtrigger_to` MODIFY `mailtrigger_id` varchar(64);',
reverse_sql='ALTER TABLE `mailtrigger_mailtrigger_to` MODIFY `mailtrigger_id` varchar(32);',
),
migrations.RunSQL(
sql='ALTER TABLE `mailtrigger_mailtrigger_cc` MODIFY `mailtrigger_id` varchar(64);',
reverse_sql='ALTER TABLE `mailtrigger_mailtrigger_cc` MODIFY `mailtrigger_id` varchar(32);',
),
]

52
ietf/mailtrigger/migrations/0009_custom_review_complete_mailtriggers.py Normal file
View file

@ -0,0 +1,52 @@
# Copyright The IETF Trust 2019, All Rights Reserved
# -*- coding: utf-8 -*-
from __future__ import absolute_import, print_function, unicode_literals
from django.db import migrations
def forward(apps, schema_editor):
ReviewTeamSettings = apps.get_model('review', 'ReviewTeamSettings')
MailTrigger = apps.get_model('mailtrigger', 'Mailtrigger')
Group = apps.get_model('group', 'Group')
GroupFeatures = apps.get_model('group', 'GroupFeatures')
template = MailTrigger.objects.get(slug='review_completed')
template.desc = 'Default template for recipients when an review is completed - ' \
'customised mail triggers are used/created per team and review type.'
template.save()
for group in Group.objects.all().only('pk', 'type', 'acronym'):
if not GroupFeatures.objects.get(type=group.type).has_reviews:
continue
try:
review_team = ReviewTeamSettings.objects.get(group=group.pk)
except ReviewTeamSettings.DoesNotExist:
continue
team_acronym = group.acronym.lower()
for review_type in review_team.review_types.all():
slug = 'review_completed_{}_{}'.format(team_acronym, review_type.slug)
desc = 'Recipients when a {} {} review is completed'.format(team_acronym, review_type)
if MailTrigger.objects.filter(slug=slug):
# Never overwrite existing triggers
continue
mailtrigger = MailTrigger.objects.create(slug=slug, desc=desc)
mailtrigger.to.set(template.to.all())
mailtrigger.cc.set(template.cc.all())
def reverse(apps, schema_editor):
pass
class Migration(migrations.Migration):
dependencies = [
('mailtrigger', '0008_lengthen_mailtrigger_slug'),
('review', '0014_document_primary_key_cleanup'),
('group', '0019_rename_field_document2'),
]
operations = [
migrations.RunPython(forward, reverse)
]

2
ietf/mailtrigger/models.py
View file

@ -35,7 +35,7 @@ def clean_duplicates(addrlist):
@python_2_unicode_compatible
class MailTrigger(models.Model):
slug = models.CharField(max_length=32, primary_key=True)
slug = models.CharField(max_length=64, primary_key=True)
desc = models.TextField(blank=True)
to = models.ManyToManyField('Recipient', blank=True, related_name='used_in_to')
cc = models.ManyToManyField('Recipient', blank=True, related_name='used_in_cc')

46
ietf/mailtrigger/test_utils.py Normal file
View file

@ -0,0 +1,46 @@
# Copyright The IETF Trust 2015-2019, All Rights Reserved
# -*- coding: utf-8 -*-
from __future__ import absolute_import, print_function, unicode_literals
from ietf.doc.factories import WgDraftFactory
from ietf.mailtrigger.models import MailTrigger
from .utils import gather_address_lists
from ietf.utils.test_utils import TestCase
import six
class GatherAddressListsTests(TestCase):
def setUp(self):
self.doc = WgDraftFactory(group__acronym='mars', rev='01')
self.author_address = self.doc.name + '@ietf.org'
def test_regular_trigger(self):
to, cc = gather_address_lists('doc_pulled_from_rfc_queue', doc=self.doc)
# Despite its name, assertCountEqual also compares content, but does not care for ordering
six.assertCountEqual(self, to, ['iana@iana.org', 'rfc-editor@rfc-editor.org'])
six.assertCountEqual(self, cc, ['The IESG <iesg@ietf.org>', self.author_address,
'mars-chairs@ietf.org', 'iesg-secretary@ietf.org'])
def test_skipped_recipient(self):
to, cc = gather_address_lists('doc_pulled_from_rfc_queue', doc=self.doc,
skipped_recipients=['mars-chairs@ietf.org', 'iana@iana.org'])
six.assertCountEqual(self, to, ['rfc-editor@rfc-editor.org'])
six.assertCountEqual(self, cc, ['The IESG <iesg@ietf.org>', self.author_address,
'iesg-secretary@ietf.org'])
def test_trigger_does_not_exist(self):
with self.assertRaises(MailTrigger.DoesNotExist):
gather_address_lists('this-does-not-exist______', doc=self.doc)
def test_create_if_not_exists(self):
new_slug = 'doc_pulled_from_rfc_special_autocreated'
new_desc = 'Autocreated mailtrigger from doc_pulled_from_rfc_queue'
to, cc = gather_address_lists(new_slug, doc=self.doc, desc_if_not_exists=new_desc,
create_from_slug_if_not_exists='doc_pulled_from_rfc_queue')
six.assertCountEqual(self, to, ['iana@iana.org', 'rfc-editor@rfc-editor.org'])
six.assertCountEqual(self, cc, ['The IESG <iesg@ietf.org>', self.author_address,
'mars-chairs@ietf.org', 'iesg-secretary@ietf.org'])
new_trigger = MailTrigger.objects.get(slug=new_slug)
self.assertEqual(new_trigger.desc, new_desc)

21
ietf/mailtrigger/utils.py
View file

@ -19,8 +19,10 @@ class AddrLists(namedtuple('AddrLists',['to','cc'])):
return namedtuple('AddrListsAsStrings',['to','cc'])(to=to_string,cc=cc_string)
def gather_address_lists(slug, skipped_recipients=None, **kwargs):
mailtrigger = MailTrigger.objects.get(slug=slug)
def gather_address_lists(slug, skipped_recipients=None, create_from_slug_if_not_exists=None,
desc_if_not_exists=None, **kwargs):
mailtrigger = get_mailtrigger(slug, create_from_slug_if_not_exists, desc_if_not_exists)
to = set()
for recipient in mailtrigger.to.all():
@ -38,6 +40,21 @@ def gather_address_lists(slug, skipped_recipients=None, **kwargs):
return AddrLists(to=list(to),cc=list(cc))
def get_mailtrigger(slug, create_from_slug_if_not_exists, desc_if_not_exists):
try:
mailtrigger = MailTrigger.objects.get(slug=slug)
except MailTrigger.DoesNotExist:
if create_from_slug_if_not_exists and desc_if_not_exists:
template = MailTrigger.objects.get(slug=create_from_slug_if_not_exists)
mailtrigger = MailTrigger.objects.create(slug=slug, desc=desc_if_not_exists)
mailtrigger.to.set(template.to.all())
mailtrigger.cc.set(template.cc.all())
else:
raise
return mailtrigger
def gather_relevant_expansions(**kwargs):
def starts_with(prefix):

2
ietf/name/fixtures/names.json
View file

@ -3902,7 +3902,7 @@
"review_doc_all_parties",
"review_doc_group_mail_list"
],
"desc": "Recipients when an review is completed",
"desc": "Default template for recipients when an review is completed - customised mail triggers are used/created per team and review type.",
"to": [
"review_team_mail_list"
]