Remove stream editor email duplication

ietf/doc/tests_conflict_review.py

@@ -14,7 +14,8 @@
 import debug    # pyflakes:ignore
 
 from ietf.doc.factories import IndividualDraftFactory, ConflictReviewFactory
-from ietf.doc.models import Document, DocEvent, NewRevisionDocEvent, BallotPositionDocEvent, TelechatDocEvent, State
+from ietf.doc.models import Document, DocEvent, NewRevisionDocEvent, BallotPositionDocEvent, \
+                            TelechatDocEvent, State, DocAlias
 from ietf.doc.utils import create_ballot_if_not_open
 from ietf.doc.views_conflict_review import default_approval_text
 from ietf.group.models import Person
@@ -80,6 +81,61 @@ def test_start_review_as_secretary(self):
         r = self.client.post(url,dict(ad="Areað Irector",create_in_state="Needs Shepherd",notify='ipu@ietf.org'))
         self.assertEqual(r.status_code, 404)
 
+    def test_start_review_as_secretary_with_stream_manager_in_notify(self):
+        ad_strpk = str(Person.objects.get(name='Areað Irector').pk)
+        state_strpk = str(State.objects.get(used=True, slug='needshep', type__slug='conflrev').pk)
+        self.client.login(username="secretary", password="secretary+password")
+        # This set of notification recipients should cover a bunch of cases for this test.
+        # It includes: 
+        #     * The address to remove being first in the list
+        #     * The address to remove being in the list twice
+        #     * The address to remove having a name in front of the email address
+        #     * The address to remove being bare
+        #     * Extra addresses not being removed
+        notify = ','.join(['<rfc-ise@rfc-editor.org>',
+                           'IRTF Chair <irtf-chair@irtf.org>',
+                           '<iesg@ietf.org>',
+                           'rfc-ise@rfc-editor.org',
+                           '<iab-chair@iab.org>'])
+        manager_emails = {'ise': 'rfc-ise@rfc-editor.org', 'irtf': 'irtf-chair@irtf.org'}
+
+        for stream_slug in ['ise', 'irtf']:
+            doc = Document.objects.create(
+                name=f"draft-{stream_slug}-imaginary-submission",
+                type_id='draft',
+                rev='00',
+                stream=StreamName.objects.get(slug=stream_slug),
+                title=f"Imaginary {stream_slug}: A flight of fancy through {stream_slug}"
+            )
+            DocAlias.objects.create(name=doc.name).docs.add(doc)
+            url = urlreverse('ietf.doc.views_conflict_review.start_review', kwargs=dict(name=doc.name))
+
+            # Choose which email address needs to be removed.
+            email = manager_emails[stream_slug]
+
+            # Get the email addresses which should remain
+            other_emails = [email for stream, email in manager_emails.items()
+                            if stream != stream_slug]
+
+            data = dict(
+                ad=ad_strpk, 
+                create_in_state=state_strpk,
+                notify=notify
+            )
+            r = self.client.post(
+                url,
+                data
+            )
+            self.assertEqual(r.status_code, 302)
+
+            review_doc = Document.objects.get(name=f"conflict-review-{stream_slug}-imaginary-submission")
+            self.assertNotIn(email, review_doc.notify)
+            self.assertIn('iesg@ietf.org', review_doc.notify)
+            self.assertIn('iab-chair@iab.org', review_doc.notify)
+            for other_email in other_emails:
+                self.assertIn(
+                    other_email, 
+                    review_doc.notify)
 
     def test_start_review_as_stream_owner(self):
 

ietf/doc/views_conflict_review.py

@@ -5,6 +5,7 @@
 import datetime
 import io
 import os
+import re
 
 from django import forms
 from django.shortcuts import render, get_object_or_404, redirect
@@ -25,6 +26,7 @@
 from ietf.group.models import Role, Group
 from ietf.iesg.models import TelechatDate
 from ietf.ietfauth.utils import has_role, role_required, is_authorized_in_doc_stream
+from ietf.mailtrigger.models import Recipient
 from ietf.person.models import Person
 from ietf.utils import log
 from ietf.utils.mail import send_mail_preformatted
@@ -369,14 +371,20 @@ def approve_conflict_review(request, name):
                                    form = form,
                                    ))
 
+
 class SimpleStartReviewForm(forms.Form):
-    notify = forms.CharField(max_length=255, label="Notice emails", help_text="Separate email addresses with commas.", required=False)
+    notify = forms.CharField(
+        max_length=255,
+        label="Notice emails",
+        help_text="Separate email addresses with commas. " + 
+                  "ISE and IRTF stream editors are notified automatically for their streams.",
+        required=False)
+
 
-class StartReviewForm(forms.Form):
+class StartReviewForm(SimpleStartReviewForm):
     ad = forms.ModelChoiceField(Person.objects.filter(role__name="ad", role__group__state="active",role__group__type='area').order_by('name'), 
                                 label="Shepherding AD", empty_label="(None)", required=True)
     create_in_state = forms.ModelChoiceField(State.objects.filter(used=True, type="conflrev", slug__in=("needshep", "adrev")), empty_label=None, required=False)
-    notify = forms.CharField(max_length=255, label="Notice emails", help_text="Separate email addresses with commas.", required=False)
     telechat_date = forms.TypedChoiceField(coerce=lambda x: datetime.datetime.strptime(x, '%Y-%m-%d').date(), empty_value=None, required=False, widget=forms.Select(attrs={'onchange':'make_bold()'}))
 
     def __init__(self, *args, **kwargs):
@@ -392,10 +400,52 @@ def __init__(self, *args, **kwargs):
 
 @role_required("Secretariat","IRTF Chair","ISE")
 def start_review(request, name):
-    if has_role(request.user,"Secretariat"):
-        return start_review_as_secretariat(request,name)
+    '''Start the conflict review process.
+
+    Start the conflict review process, setting the initial 
+    shepherding AD, and possibly putting the review on a telechat.'''
+
+    def cleaned_notify(notify, doc):
+        if doc.stream.slug not in ['irtf', 'ise']:
+            return notify
+        stream_managers = [
+            re.sub('([^<]*<)?(.*)(>.*)', '\2', r)
+            for r in Recipient(slug='stream_managers').gather(streams=[doc.stream.slug])
+        ]
+        notifications = notify.split(',')
+        for mgr in stream_managers:
+            goods = [n for n in notifications if mgr not in n]
+            notifications = goods
+        retval = ','.join(notifications)
+        return retval
+
+    doc_to_review = start_review_sanity_check(request, name)
+    if has_role(request.user, "Secretariat"):
+        form_class = StartReviewForm
     else:
-        return start_review_as_stream_owner(request,name)
+        form_class = SimpleStartReviewForm
+
+    if request.method == 'POST':
+        form = form_class(request.POST)
+        if form.is_valid():
+            form.cleaned_data['notify'] = cleaned_notify(form.cleaned_data['notify'], doc_to_review)
+            conflict_review = conflict_review_document(doc_to_review, form, request)
+            send_conflict_review_started_email(request, conflict_review)
+            return HttpResponseRedirect(conflict_review.get_absolute_url())
+    else:
+        notify_addresses = build_notify_addresses(doc_to_review)
+        init = {'notify': ', '.join(notify_addresses)}
+        if has_role(request.user, 'Secretariat'):
+            init["ad"] = Role.objects.filter(group__acronym='ietf', name='chair')[0].person.id
+        form = form_class(initial=init)
+
+    return render(
+        request,
+        'doc/conflict_review/start.html',
+        {'form': form,
+         'doc_to_review': doc_to_review}
+    )
+
 
 def start_review_sanity_check(request, name):
     doc_to_review = get_object_or_404(Document, type="draft", name=name)
@@ -409,13 +459,20 @@ def start_review_sanity_check(request, name):
 
     return doc_to_review
 
+
 def build_notify_addresses(doc_to_review):
     # Take care to do the right thing during ietf chair and stream owner transitions
     notify_addresses = []
-    notify_addresses.extend([r.formatted_email() for r in Role.objects.filter(group__acronym=doc_to_review.stream.slug, name='chair')])
+    stream_manager_addresses = Recipient(slug='stream_managers').gather(streams=['ise', 'irtf'])
+    notify_addresses.extend(
+        [r.formatted_email() 
+         for r in Role.objects.filter(group__acronym=doc_to_review.stream.slug, name='chair')
+         if r.formatted_email() not in stream_manager_addresses]
+    )
     notify_addresses.append("%s@%s" % (doc_to_review.name, settings.DRAFT_ALIAS_DOMAIN))
     return notify_addresses
 
+
 def build_conflict_review_document(login, doc_to_review, ad, notify, create_in_state):
     if doc_to_review.name.startswith('draft-'):
         review_name = 'conflict-review-'+doc_to_review.name[6:]
@@ -453,74 +510,26 @@ def build_conflict_review_document(login, doc_to_review, ad, notify, create_in_s
 
     return conflict_review
 
-def start_review_as_secretariat(request, name):
-    """Start the conflict review process, setting the initial shepherding AD, and possibly putting the review on a telechat."""
-
-    doc_to_review = start_review_sanity_check(request, name)
 
+def conflict_review_document(doc_to_review, form, request):
     login = request.user.person
-
-    if request.method == 'POST':
-        form = StartReviewForm(request.POST)
-        if form.is_valid():
-            conflict_review = build_conflict_review_document(login = login,
-                                                             doc_to_review = doc_to_review, 
-                                                             ad = form.cleaned_data['ad'],
-                                                             notify = form.cleaned_data['notify'],
-                                                             create_in_state = form.cleaned_data['create_in_state']
-                                                            )
-
-            tc_date = form.cleaned_data['telechat_date']
-            if tc_date:
-                update_telechat(request, conflict_review, login, tc_date)
-
-            send_conflict_review_started_email(request, conflict_review)
-
-            return HttpResponseRedirect(conflict_review.get_absolute_url())
-    else: 
-        notify_addresses = build_notify_addresses(doc_to_review)
-        init = { 
-                "ad" : Role.objects.filter(group__acronym='ietf',name='chair')[0].person.id,
-                "notify" : ', '.join(notify_addresses),
-               }
-        form = StartReviewForm(initial=init)
-
-    return render(request, 'doc/conflict_review/start.html',
-                              {'form':   form,
-                               'doc_to_review': doc_to_review,
-                              },
-                          )
-
-def start_review_as_stream_owner(request, name):
-    """Start the conflict review process using defaults for everything but notify and let the secretariat know"""
-
-    doc_to_review = start_review_sanity_check(request, name)
-
-    login = request.user.person
-
-    if request.method == 'POST':
-        form = SimpleStartReviewForm(request.POST)
-        if form.is_valid():
-            conflict_review = build_conflict_review_document(login = login,
-                                                             doc_to_review = doc_to_review, 
-                                                             ad = Role.objects.filter(group__acronym='ietf',name='chair')[0].person,
-                                                             notify = form.cleaned_data['notify'],
-                                                             create_in_state = State.objects.get(used=True,type='conflrev',slug='needshep')
-                                                            )
-
-            send_conflict_review_started_email(request, conflict_review)
-
-            return HttpResponseRedirect(conflict_review.get_absolute_url())
-    else: 
-        notify_addresses = build_notify_addresses(doc_to_review)
-
-        init = { 
-                "notify" : ', '.join(notify_addresses),
-               }
-        form = SimpleStartReviewForm(initial=init)
-
-    return render(request, 'doc/conflict_review/start.html',
-                              {'form':   form,
-                               'doc_to_review': doc_to_review,
-                              },
-                          )
+    if has_role(request.user, "Secretariat"):
+        ad = form.cleaned_data['ad']
+        create_in_state = form.cleaned_data['create_in_state']
+        tc_date = form.cleaned_data['telechat_date']
+    else:
+        ad = Role.objects.filter(group__acronym='ietf', name='chair')[0].person
+        create_in_state = State.objects.get(used=True, type='conflrev', slug='needshep')
+        tc_date = None
+
+    conflict_review = build_conflict_review_document(
+        login=login,
+        doc_to_review=doc_to_review,
+        ad=ad,
+        notify=form.cleaned_data['notify'],
+        create_in_state=create_in_state
+    )
+    if tc_date:
+        update_telechat(request, conflict_review, login, tc_date)
+
+    return conflict_review