From a0fd974c924a93156465d9d6d7a22afabf7b4ec1 Mon Sep 17 00:00:00 2001 From: Robert Sparks Date: Thu, 6 Feb 2014 04:25:33 +0000 Subject: [PATCH] Allow the IRTF Chair and the ISE to request a conflict review directly through the tracker. Notify the Secretariat when someone other than the secretariat initiates a conflict review. Notify IANA when anyone initiates a conflict review. Fixes tickets #1287 and #1289 Commit ready for merge - Legacy-Id: 7210 --- ietf/doc/tests_conflict_review.py | 50 +++++- ietf/doc/views_conflict_review.py | 165 +++++++++++++----- ietf/doc/views_doc.py | 2 +- ietf/ietfauth/utils.py | 1 + .../doc/conflict_review/review_started.txt | 11 ++ ietf/templates/doc/conflict_review/start.html | 4 + 6 files changed, 186 insertions(+), 47 deletions(-) create mode 100644 ietf/templates/doc/conflict_review/review_started.txt diff --git a/ietf/doc/tests_conflict_review.py b/ietf/doc/tests_conflict_review.py index 595cf9549..e842bac19 100644 --- a/ietf/doc/tests_conflict_review.py +++ b/ietf/doc/tests_conflict_review.py @@ -23,7 +23,7 @@ from ietf.iesg.models import TelechatDate class ConflictReviewTests(TestCase): - def test_start_review(self): + def test_start_review_as_secretary(self): doc = Document.objects.get(name='draft-imaginary-independent-submission') url = urlreverse('conflict_review_start',kwargs=dict(name=doc.name)) @@ -71,11 +71,59 @@ class ConflictReviewTests(TestCase): self.assertTrue(review_doc.latest_event(DocEvent,type="added_comment").desc.startswith("IETF conflict review requested")) self.assertTrue(doc.latest_event(DocEvent,type="added_comment").desc.startswith("IETF conflict review initiated")) + self.assertTrue('Conflict Review requested' in outbox[-1]['Subject']) + self.assertTrue(settings.IANA_EVAL_EMAIL in outbox[-1]['To']) # verify you can't start a review when a review is already in progress r = self.client.post(url,dict(ad="Aread Irector",create_in_state="Needs Shepherd",notify='ipu@ietf.org')) self.assertEqual(r.status_code, 404) + + def test_start_review_as_stream_owner(self): + + doc = Document.objects.get(name='draft-imaginary-independent-submission') + url = urlreverse('conflict_review_start',kwargs=dict(name=doc.name)) + + login_testing_unauthorized(self, "ise-chair", url) + + # can't start conflict reviews on documents not in a stream + r = self.client.get(url) + self.assertEquals(r.status_code, 404) + + + # can't start conflict reviews on documents in some other stream + doc.stream=StreamName.objects.get(slug='irtf') + doc.save() + r = self.client.get(url) + self.assertEquals(r.status_code, 404) + + # successful get + doc.stream=StreamName.objects.get(slug='ise') + doc.save() + r = self.client.get(url) + self.assertEquals(r.status_code, 200) + q = PyQuery(r.content) + self.assertEquals(len(q('form input[name=notify]')),1) + self.assertEquals(len(q('form select[name=ad]')),0) + + # successfully starts a review, and notifies the secretariat + messages_before = len(outbox) + r = self.client.post(url,dict(notify='ipu@ietf.org')) + self.assertEquals(r.status_code, 302) + review_doc = Document.objects.get(name='conflict-review-imaginary-independent-submission') + self.assertEquals(review_doc.get_state('conflrev').slug,'needshep') + self.assertEquals(review_doc.rev,u'00') + self.assertEquals(review_doc.telechat_date(),None) + self.assertEquals(review_doc.ad.name,u'Ietf Chair') + self.assertEquals(review_doc.notify,u'ipu@ietf.org') + doc = Document.objects.get(name='draft-imaginary-independent-submission') + self.assertTrue(doc in [x.target.document for x in review_doc.relateddocument_set.filter(relationship__slug='conflrev')]) + self.assertEqual(len(outbox), messages_before + 2) + self.assertTrue('Conflict Review requested' in outbox[-1]['Subject']) + self.assertTrue(any('iesg-secretary@ietf.org' in x['To'] for x in outbox[-2:])) + self.assertTrue(any(settings.IANA_EVAL_EMAIL in x['To'] for x in outbox[-2:])) + + def test_change_state(self): doc = Document.objects.get(name='conflict-review-imaginary-irtf-submission') diff --git a/ietf/doc/views_conflict_review.py b/ietf/doc/views_conflict_review.py index 7a0a1c09e..9da832de1 100644 --- a/ietf/doc/views_conflict_review.py +++ b/ietf/doc/views_conflict_review.py @@ -11,7 +11,7 @@ from django.conf import settings from ietf.doc.utils import add_state_change_event, update_telechat from ietf.doc.models import save_document_in_history from ietf.doc.utils import create_ballot_if_not_open, close_open_ballots, get_document_content -from ietf.ietfauth.utils import has_role, role_required +from ietf.ietfauth.utils import has_role, role_required, is_authorized_in_doc_stream from ietf.utils.textupload import get_cleaned_text_file_content from ietf.utils.mail import send_mail_preformatted from ietf.doc.mails import email_iana @@ -88,6 +88,22 @@ def change_state(request, name, option=None): ), context_instance=RequestContext(request)) +def send_conflict_review_started_email(request, review): + msg = render_to_string("doc/conflict_review/review_started.txt", + dict(frm = settings.DEFAULT_FROM_EMAIL, + by = request.user.person, + review = review, + reviewed_doc = review.relateddocument_set.get(relationship__slug='conflrev').target.document, + review_url = settings.IDTRACKER_BASE_URL+review.get_absolute_url(), + ) + ) + if not has_role(request.user,"Secretariat"): + send_mail_preformatted(request,msg) + email_iana(request, + review.relateddocument_set.get(relationship__slug='conflrev').target.document, + settings.IANA_EVAL_EMAIL, + msg) + def send_conflict_eval_email(request,review): msg = render_to_string("doc/eval_email.txt", dict(doc=review, @@ -345,6 +361,9 @@ def approve(request, name): ), context_instance=RequestContext(request)) +class SimpleStartReviewForm(forms.Form): + notify = forms.CharField(max_length=255, label="Notice emails", help_text="Separate email addresses with commas", required=False) + class StartReviewForm(forms.Form): ad = forms.ModelChoiceField(Person.objects.filter(role__name="ad", role__group__state="active").order_by('name'), label="Shepherding AD", empty_label="(None)", required=True) @@ -363,73 +382,96 @@ class StartReviewForm(forms.Form): self.fields['telechat_date'].choices = [("", "(not on agenda)")] + [(d, d.strftime("%Y-%m-%d")) for d in dates] -@role_required("Secretariat") +@role_required("Secretariat","IRTF Chair","ISE") def start_review(request, name): - """Start the conflict review process, setting the initial shepherding AD, and possibly putting the review on a telechat.""" + if has_role(request.user,"Secretariat"): + return start_review_as_secretariat(request,name) + else: + return start_review_as_stream_owner(request,name) +def start_review_sanity_check(request, name): doc_to_review = get_object_or_404(Document, type="draft", name=name) - if not doc_to_review.stream_id in ('ise','irtf'): + if ( not doc_to_review.stream_id in ('ise','irtf') ) or ( not is_authorized_in_doc_stream(request.user,doc_to_review)): raise Http404 # sanity check that there's not already a conflict review document for this document if [ rel.source for alias in doc_to_review.docalias_set.all() for rel in alias.relateddocument_set.filter(relationship='conflrev') ]: raise Http404 - login = request.user.person + 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([x.person.formatted_email() for x in Role.objects.filter(group__acronym=doc_to_review.stream.slug,name='chair')]) + notify_addresses.append("%s@%s" % (doc_to_review.name, settings.TOOLS_SERVER)) + 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:] + else: + # This is a failsafe - and might be treated better as an error + review_name = 'conflict-review-'+doc_to_review.name + + iesg_group = Group.objects.get(acronym='iesg') + + conflict_review=Document( type_id = "conflrev", + title = "IETF conflict review for %s" % doc_to_review.name, + name = review_name, + rev = "00", + ad = ad, + notify = notify, + stream_id = 'ietf', + group = iesg_group, + ) + conflict_review.save() + conflict_review.set_state(create_in_state) + + DocAlias.objects.create( name=review_name , document=conflict_review ) + + conflict_review.relateddocument_set.create(target=DocAlias.objects.get(name=doc_to_review.name),relationship_id='conflrev') + + c = DocEvent(type="added_comment", doc=conflict_review, by=login) + c.desc = "IETF conflict review requested" + c.save() + + c = DocEvent(type="added_comment", doc=doc_to_review, by=login) + # Is it really OK to put html tags into comment text? + c.desc = 'IETF conflict review initiated - see %s' % (reverse('doc_view', kwargs={'name':conflict_review.name}),conflict_review.name) + c.save() + + 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) + + 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'] + ) - if doc_to_review.name.startswith('draft-'): - review_name = 'conflict-review-'+doc_to_review.name[6:] - else: - # This is a failsafe - and might be treated better as an error - review_name = 'conflict-review-'+doc_to_review.name - - iesg_group = Group.objects.get(acronym='iesg') - - conflict_review=Document( type_id = "conflrev", - title = "IETF conflict review for %s" % doc_to_review.name, - name = review_name, - rev = "00", - ad = form.cleaned_data['ad'], - notify = form.cleaned_data['notify'], - stream_id = 'ietf', - group = iesg_group, - ) - conflict_review.save() - conflict_review.set_state(form.cleaned_data['create_in_state']) - - DocAlias.objects.create( name=review_name , document=conflict_review ) - - conflict_review.relateddocument_set.create(target=DocAlias.objects.get(name=doc_to_review.name),relationship_id='conflrev') - - c = DocEvent(type="added_comment", doc=conflict_review, by=login) - c.desc = "IETF conflict review requested" - c.save() - - c = DocEvent(type="added_comment", doc=doc_to_review, by=login) - # Is it really OK to put html tags into comment text? - c.desc = 'IETF conflict review initiated - see %s' % (reverse('doc_view', kwargs={'name':conflict_review.name}),conflict_review.name) - c.save() - 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: - # Take care to do the right thing during ietf chair and stream owner transitions - ietf_chair_id = Role.objects.filter(group__acronym='ietf',name='chair')[0].person.id - notify_addresses = [] - notify_addresses.extend([x.person.formatted_email() for x in Role.objects.filter(group__acronym=doc_to_review.stream.slug,name='chair')]) - notify_addresses.append("%s@%s" % (name, settings.TOOLS_SERVER)) - + notify_addresses = build_notify_addresses(doc_to_review) init = { - "ad" : ietf_chair_id, + "ad" : Role.objects.filter(group__acronym='ietf',name='chair')[0].person.id, "notify" : u', '.join(notify_addresses), } form = StartReviewForm(initial=init) @@ -440,6 +482,39 @@ def start_review(request, name): }, context_instance = RequestContext(request)) +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" : u', '.join(notify_addresses), + } + form = SimpleStartReviewForm(initial=init) + + return render_to_response('doc/conflict_review/start.html', + {'form': form, + 'doc_to_review': doc_to_review, + }, + context_instance = RequestContext(request)) @role_required("Area Director", "Secretariat") def telechat_date(request, name): diff --git a/ietf/doc/views_doc.py b/ietf/doc/views_doc.py index cfd54b49c..3c243addd 100644 --- a/ietf/doc/views_doc.py +++ b/ietf/doc/views_doc.py @@ -294,7 +294,7 @@ def document_main(request, name, rev=None): actions.append(("Resurrect", urlreverse('doc_resurrect', kwargs=dict(name=doc.name)))) if (doc.get_state_slug() != "expired" and doc.stream_id in ("ise", "irtf") - and has_role(request.user, ("Secretariat",)) and not conflict_reviews): + and can_edit_stream_info and not conflict_reviews): label = "Begin IETF Conflict Review" if not doc.intended_std_level: label += " (note that intended status is not set)" diff --git a/ietf/ietfauth/utils.py b/ietf/ietfauth/utils.py index 13bef2fdf..220f775cd 100644 --- a/ietf/ietfauth/utils.py +++ b/ietf/ietfauth/utils.py @@ -46,6 +46,7 @@ def has_role(user, role_names, *args, **kwargs): "Secretariat": Q(person=person, name="secr", group__acronym="secretariat"), "IANA": Q(person=person, name="auth", group__acronym="iana"), "RFC Editor": Q(person=person, name="auth", group__acronym="rfceditor"), + "ISE" : Q(person=person, name="chair", group__acronym="ise"), "IAD": Q(person=person, name="admdir", group__acronym="ietf"), "IETF Chair": Q(person=person, name="chair", group__acronym="ietf"), "IRTF Chair": Q(person=person, name="chair", group__acronym="irtf"), diff --git a/ietf/templates/doc/conflict_review/review_started.txt b/ietf/templates/doc/conflict_review/review_started.txt new file mode 100644 index 000000000..5161e287f --- /dev/null +++ b/ietf/templates/doc/conflict_review/review_started.txt @@ -0,0 +1,11 @@ +{% load mail_filters %}{% autoescape off %}To: IESG Secretary +From: {{ frm }} +Subject: Conflict Review requested for {{reviewed_doc.name}} + +{{ by.name }} has requested a conflict review for: + {{ reviewed_doc.name }} + {{ reviewed_doc.title }} + +The conflict review is being tracked at <{{ review_url }}> + +{% endautoescape%} diff --git a/ietf/templates/doc/conflict_review/start.html b/ietf/templates/doc/conflict_review/start.html index 6272442fd..8ce73400d 100644 --- a/ietf/templates/doc/conflict_review/start.html +++ b/ietf/templates/doc/conflict_review/start.html @@ -1,5 +1,7 @@ {% extends "base.html" %} +{% load ietf_filters %} + {% block title %}Begin IETF conflict review : {{doc_to_review.canonical_name}}-{{doc_to_review.rev}}{% endblock %} {% block morecss %} @@ -18,7 +20,9 @@ form.start-conflict-review .actions { {% block content %}

Begin IETF conflict review for {{doc_to_review.canonical_name}}-{{doc_to_review.rev}}

+{% if user|has_role:"Secretariat" %}

For help on the initial state choice, see the state table.

+{% endif %}
{% csrf_token %}