Skip to content

Commit

Permalink
Refactored editing the notify field to remove redundant code.
Browse files Browse the repository at this point in the history
Changed the default notification list to include .all for documents, and the wg list for wg documents.
Allowed recalculating the notification list for all document types.
Improved the calculated notification list value for charters, conflict-reviews, and status-changes.
Adds shepherds to the notification list when they are assigned to a document.
Adds the working group email list to the notification list when a document is adopted.

Fixes #1438
Commit ready for merge.
 - Legacy-Id: 8293
  • Loading branch information
rjsparks committed Aug 20, 2014
1 parent 7c56f8b commit 640c5eb
Show file tree
Hide file tree
Showing 20 changed files with 216 additions and 283 deletions.
6 changes: 5 additions & 1 deletion ietf/doc/forms.py
Expand Up @@ -34,4 +34,8 @@ def __init__(self, *args, **kwargs):
self.fields['ad'].choices = list(choices) + [("", "-------"), (ad_pk, Person.objects.get(pk=ad_pk).plain_name())]

class NotifyForm(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, help_text="List of email addresses to receive state notifications, separated by comma", label="Notification list", required=False)

def clean_notify(self):
addrspecs = [x.strip() for x in self.cleaned_data["notify"].split(',')]
return ', '.join(addrspecs)
2 changes: 2 additions & 0 deletions ietf/doc/models.py
Expand Up @@ -178,6 +178,8 @@ def meeting_related(self):
class Meta:
abstract = True

STATUSCHANGE_RELATIONS = ('tops','tois','tohist','toinf','tobcp','toexp')

class RelatedDocument(models.Model):
source = models.ForeignKey('Document')
target = models.ForeignKey('DocAlias')
Expand Down
16 changes: 14 additions & 2 deletions ietf/doc/tests_charter.py
Expand Up @@ -181,11 +181,23 @@ def test_edit_notify(self):

# post
self.assertTrue(not charter.notify)
r = self.client.post(url, dict(notify="someone@example.com, someoneelse@example.com"))
newlist = "someone@example.com, someoneelse@example.com"
r = self.client.post(url, dict(notify=newlist,save_addresses="1"))
self.assertEqual(r.status_code, 302)

charter = Document.objects.get(name=charter.name)
self.assertEqual(charter.notify, "someone@example.com, someoneelse@example.com")
self.assertEqual(charter.notify, newlist)

# Ask the form to regenerate the list
r = self.client.post(url,dict(regenerate_addresses="1"))
self.assertEqual(r.status_code,200)
charter= Document.objects.get(name=charter.name)
# Regenerate does not save!
self.assertEqual(charter.notify,newlist)
q = PyQuery(r.content)
formlist = q('form input[name=notify]')[0].value
self.assertTrue('marschairman@ietf.org' in formlist)
self.assertFalse('someone@example.com' in formlist)

def test_edit_ad(self):
make_test_data()
Expand Down
12 changes: 11 additions & 1 deletion ietf/doc/tests_conflict_review.py
Expand Up @@ -176,12 +176,22 @@ def test_edit_notices(self):

# change notice list
newlist = '"Foo Bar" <foo@bar.baz.com>'
r = self.client.post(url,dict(notify=newlist))
r = self.client.post(url,dict(notify=newlist,save_addresses="1"))
self.assertEqual(r.status_code,302)
doc = Document.objects.get(name='conflict-review-imaginary-irtf-submission')
self.assertEqual(doc.notify,newlist)
self.assertTrue(doc.latest_event(DocEvent,type="added_comment").desc.startswith('Notification list changed'))

# Ask the form to regenerate the list
r = self.client.post(url,dict(regenerate_addresses="1"))
self.assertEqual(r.status_code,200)
doc = Document.objects.get(name='conflict-review-imaginary-irtf-submission')
# Regenerate does not save!
self.assertEqual(doc.notify,newlist)
q = PyQuery(r.content)
self.assertTrue('draft-imaginary-irtf-submission@tools.ietf.org' in q('form input[name=notify]')[0].value)
self.assertTrue('irtf-chair@ietf.org' in q('form input[name=notify]')[0].value)
self.assertTrue('foo@bar.baz.com' not in q('form input[name=notify]')[0].value)

def test_edit_ad(self):
doc = Document.objects.get(name='conflict-review-imaginary-irtf-submission')
Expand Down
11 changes: 8 additions & 3 deletions ietf/doc/tests_draft.py
Expand Up @@ -841,7 +841,9 @@ def test_doc_change_shepherd(self):
self.assertEqual(r.status_code,302)
self.doc = Document.objects.get(name=self.docname)
self.assertEqual(self.doc.shepherd,plain)
self.assertTrue(self.doc.latest_event(DocEvent,type="added_comment").desc.startswith('Document shepherd changed to Plain Man'))
comments = '::'.join([x.desc for x in self.doc.docevent_set.filter(time=self.doc.time,type="added_comment")])
self.assertTrue('Document shepherd changed to Plain Man' in comments)
self.assertTrue('Notification list changed' in comments)

ad = Person.objects.get(name='Aread Irector')
two_answers = "%s,%s" % (plain_email, ad.email_set.all()[0])
Expand Down Expand Up @@ -1021,16 +1023,19 @@ def test_adopt_document(self):
# adopt in mars WG
mailbox_before = len(outbox)
events_before = draft.docevent_set.count()
mars = Group.objects.get(acronym="mars")
r = self.client.post(url,
dict(comment="some comment",
group=Group.objects.get(acronym="mars").pk,
group=mars.pk,
weeks="10"))
self.assertEqual(r.status_code, 302)

draft = Document.objects.get(pk=draft.pk)
self.assertEqual(draft.group.acronym, "mars")
self.assertEqual(draft.stream_id, "ietf")
self.assertEqual(draft.docevent_set.count() - events_before, 4)
self.assertEqual(draft.docevent_set.count() - events_before, 5)
self.assertTrue(mars.list_email in draft.notify)
self.assertTrue('draft-ietf-mars-test.all@tools.ietf.org' in draft.notify)
self.assertEqual(len(outbox), mailbox_before + 1)
self.assertTrue("state changed" in outbox[-1]["Subject"].lower())
self.assertTrue("marschairman@ietf.org" in unicode(outbox[-1]))
Expand Down
18 changes: 17 additions & 1 deletion ietf/doc/tests_status_change.py
Expand Up @@ -139,12 +139,28 @@ def test_edit_notices(self):

# change notice list
newlist = '"Foo Bar" <foo@bar.baz.com>'
r = self.client.post(url,dict(notify=newlist))
r = self.client.post(url,dict(notify=newlist,save_addresses="1"))
self.assertEqual(r.status_code,302)
doc = Document.objects.get(name='status-change-imaginary-mid-review')
self.assertEqual(doc.notify,newlist)
self.assertTrue(doc.latest_event(DocEvent,type="added_comment").desc.startswith('Notification list changed'))

# Some additional setup so there's something to put in a generated notify list
doc.relateddocument_set.create(target=DocAlias.objects.get(name='rfc9999'),relationship_id='tois')
doc.relateddocument_set.create(target=DocAlias.objects.get(name='rfc9998'),relationship_id='tohist')

# Ask the form to regenerate the list
r = self.client.post(url,dict(regenerate_addresses="1"))
self.assertEqual(r.status_code,200)
doc = Document.objects.get(name='status-change-imaginary-mid-review')
# Regenerate does not save!
self.assertEqual(doc.notify,newlist)
q = PyQuery(r.content)
formlist = q('form input[name=notify]')[0].value
self.assertTrue('draft-ietf-random-thing@ietf.org' in formlist)
self.assertTrue('draft-ietf-random-otherthing@ietf.org' in formlist)
self.assertFalse('foo@bar.baz.com' in formlist)

def test_edit_title(self):
doc = Document.objects.get(name='status-change-imaginary-mid-review')
url = urlreverse('status_change_title',kwargs=dict(name=doc.name))
Expand Down
2 changes: 1 addition & 1 deletion ietf/doc/urls.py
Expand Up @@ -74,7 +74,7 @@

url(r'^(?P<name>[A-Za-z0-9._+-]+)/edit/stream/$', views_draft.change_stream, name='doc_change_stream'),
url(r'^(?P<name>[A-Za-z0-9._+-]+)/edit/replaces/$', views_draft.replaces, name='doc_change_replaces'),
url(r'^(?P<name>[A-Za-z0-9._+-]+)/edit/notify/$', views_draft.edit_notices, name='doc_change_notify'),
url(r'^(?P<name>[A-Za-z0-9._+-]+)/edit/notify/$', views_doc.edit_notify, name='doc_change_notify'),
url(r'^(?P<name>[A-Za-z0-9._+-]+)/edit/status/$', views_draft.change_intention, name='doc_change_intended_status'),
url(r'^(?P<name>[A-Za-z0-9._+-]+)/edit/telechat/$', views_doc.telechat_date, name='doc_change_telechat_date'),
url(r'^(?P<name>[A-Za-z0-9._+-]+)/edit/iesgnote/$', views_draft.edit_iesg_note, name='doc_change_iesg_note'),
Expand Down
2 changes: 1 addition & 1 deletion ietf/doc/urls_charter.py
Expand Up @@ -6,7 +6,7 @@
url(r'^state/$', "ietf.doc.views_charter.change_state", name='charter_change_state'),
url(r'^(?P<option>initcharter|recharter|abandon)/$', "ietf.doc.views_charter.change_state", name='charter_startstop_process'),
url(r'^telechat/$', "ietf.doc.views_doc.telechat_date", name='charter_telechat_date'),
url(r'^notify/$', "ietf.doc.views_charter.edit_notify", name='charter_edit_notify'),
url(r'^notify/$', "ietf.doc.views_doc.edit_notify", name='charter_edit_notify'),
url(r'^ad/$', "ietf.doc.views_charter.edit_ad", name='charter_edit_ad'),
url(r'^(?P<ann>action|review)/$', "ietf.doc.views_charter.announcement_text", name="charter_edit_announcement"),
url(r'^ballotwriteupnotes/$', "ietf.doc.views_charter.ballot_writeupnotes"),
Expand Down
2 changes: 1 addition & 1 deletion ietf/doc/urls_conflict_review.py
Expand Up @@ -3,14 +3,14 @@
urlpatterns = patterns('ietf.doc.views_conflict_review',
url(r'^state/$', "change_state", name='conflict_review_change_state'),
url(r'^submit/$', "submit", name='conflict_review_submit'),
url(r'^notices/$', "edit_notices", name='conflict_review_notices'),
url(r'^ad/$', "edit_ad", name='conflict_review_ad'),
url(r'^approve/$', "approve", name='conflict_review_approve'),
url(r'^start_conflict_review/$', "start_review", name='conflict_review_start'),
)

urlpatterns += patterns('ietf.doc.views_doc',
url(r'^telechat/$', "telechat_date", name='conflict_review_telechat_date'),
url(r'^notices/$', "edit_notify", name='conflict_review_notices'),
)


2 changes: 1 addition & 1 deletion ietf/doc/urls_status_change.py
Expand Up @@ -3,7 +3,6 @@
urlpatterns = patterns('ietf.doc.views_status_change',
url(r'^state/$', "change_state", name='status_change_change_state'),
url(r'^submit/$', "submit", name='status_change_submit'),
url(r'^notices/$', "edit_notices", name='status_change_notices'),
url(r'^ad/$', "edit_ad", name='status_change_ad'),
url(r'^title/$', "edit_title", name='status_change_title'),
url(r'^approve/$', "approve", name='status_change_approve'),
Expand All @@ -13,6 +12,7 @@

urlpatterns += patterns('ietf.doc.views_doc',
url(r'^telechat/$', "telechat_date", name='status_change_telechat_date'),
url(r'^notices/$', "edit_notify", name='status_change_notices'),
)


72 changes: 71 additions & 1 deletion ietf/doc/utils.py
Expand Up @@ -5,16 +5,19 @@
import datetime

from django.conf import settings
from django.db.models import Q
from django.db.models.query import EmptyQuerySet
from django.forms import ValidationError
from django.utils.html import strip_tags
from django.utils.html import strip_tags, escape

from ietf.utils import markup_txt
from ietf.doc.models import Document, DocHistory
from ietf.doc.models import DocAlias, RelatedDocument, BallotType, DocReminder
from ietf.doc.models import DocEvent, BallotDocEvent, NewRevisionDocEvent, StateDocEvent
from ietf.doc.models import save_document_in_history, STATUSCHANGE_RELATIONS
from ietf.name.models import DocReminderTypeName, DocRelationshipName
from ietf.group.models import Role
from ietf.person.models import Email
from ietf.ietfauth.utils import has_role
from ietf.utils import draft
from ietf.utils.mail import send_mail
Expand Down Expand Up @@ -333,6 +336,29 @@ def has_same_ballot(doc, date1, date2=datetime.date.today()):
ballot2 = doc.latest_event(BallotDocEvent,type='created_ballot',time__lt=date2+datetime.timedelta(days=1))
return ballot1==ballot2

def make_notify_changed_event(request, doc, by, new_notify, time=None):

# FOR REVIEW: This preserves the behavior from when
# drafts and charters had separate edit_notify
# functions. If it should be unified, there should
# also be a migration function cause historic
# events to match
if doc.type.slug=='charter':
event_type = 'changed_document'
save_document_in_history(doc)
else:
event_type = 'added_comment'

e = DocEvent(type=event_type, doc=doc, by=by)
e.desc = "Notification list changed to %s" % (escape(new_notify) or "none")
if doc.notify:
e.desc += " from %s" % escape(doc.notify)
if time:
e.time = time
e.save()

return e

def update_telechat(request, doc, by, new_telechat_date, new_returning_item=None):
from ietf.doc.models import TelechatDocEvent

Expand Down Expand Up @@ -447,3 +473,47 @@ def check_common_doc_name_rules(name):

if errors:
raise ValidationError(errors)

def get_initial_notify(doc,extra=None):
# set change state notice to something sensible
receivers = []

if doc.type.slug=='draft':
if doc.group.type_id in ("individ", "area"):
for a in doc.authors.all():
receivers.append(a.address)
else:
receivers.append("%s-chairs@%s" % (doc.group.acronym, settings.TOOLS_SERVER))
for editor in Email.objects.filter(role__name="editor", role__group=doc.group):
receivers.append(editor.address)

if doc.group.list_email:
receivers.append(doc.group.list_email)

receivers.append("%s.all@%s" % (doc.name, settings.TOOLS_SERVER))

elif doc.type.slug=='charter':
receivers.extend([role.person.formatted_email() for role in doc.group.role_set.filter(name__slug__in=['ad','chair','secr','techadv'])])

else:
pass

for relation in doc.relateddocument_set.filter(Q(relationship='conflrev')|Q(relationship__in=STATUSCHANGE_RELATIONS)):
if relation.relationship.slug=='conflrev':
doc_to_review = relation.target.document
receivers.extend([x.person.formatted_email() for x in Role.objects.filter(group__acronym=doc_to_review.stream.slug,name='chair')])
receivers.append("%s@%s" % (doc_to_review.name, settings.TOOLS_SERVER))
elif relation.relationship.slug in STATUSCHANGE_RELATIONS:
affected_doc = relation.target.document
if affected_doc.notify:
receivers.extend(affected_doc.notify.split(','))

if doc.shepherd:
receivers.append(doc.shepherd.email_address())

if extra:
if isinstance(extra,basestring):
extra = extra.split(', ')
receivers.extend(extra)

return ", ".join(set([x.strip() for x in receivers]))
43 changes: 0 additions & 43 deletions ietf/doc/views_charter.py
Expand Up @@ -5,7 +5,6 @@
from django.core.urlresolvers import reverse as urlreverse
from django.template import RequestContext
from django import forms
from django.utils.html import escape
from django.utils.safestring import mark_safe
from django.conf import settings
from django.contrib import messages
Expand Down Expand Up @@ -225,48 +224,6 @@ def state_pk(slug):
),
context_instance=RequestContext(request))

class NotifyForm(forms.Form):
notify = forms.CharField(max_length=255, help_text="List of email addresses to receive state notifications, separated by comma", label="Notification list", required=False)

def clean_notify(self):
return self.cleaned_data["notify"].strip()

@role_required("Area Director", "Secretariat")
def edit_notify(request, name):
doc = get_object_or_404(Document, type="charter", name=name)
login = request.user.person

init = {'notify': doc.notify}

if request.method == "POST":
form = NotifyForm(request.POST, initial=init)
if form.is_valid():
n = form.cleaned_data["notify"]
if n != doc.notify:
save_document_in_history(doc)

e = DocEvent(doc=doc, by=login)
e.desc = "Notification list changed to %s" % (escape(n) or "none")
if doc.notify:
e.desc += " from %s" % escape(doc.notify)
e.type = "changed_document"
e.save()

doc.notify = n
doc.time = e.time
doc.save()

return redirect("doc_view", name=doc.name)
else:
form = NotifyForm(initial=init)

return render_to_response('doc/charter/edit_notify.html',
dict(doc=doc,
form=form,
user=request.user,
login=login),
context_instance=RequestContext(request))

class AdForm(forms.Form):
ad = forms.ModelChoiceField(Person.objects.filter(role__name="ad", role__group__state="active").order_by('name'),
label="Responsible AD", empty_label="(None)", required=True)
Expand Down
37 changes: 1 addition & 36 deletions ietf/doc/views_conflict_review.py
Expand Up @@ -13,7 +13,7 @@
from ietf.doc.utils import ( add_state_change_event, close_open_ballots,
create_ballot_if_not_open, get_document_content, update_telechat )
from ietf.doc.mails import email_iana
from ietf.doc.forms import AdForm, NotifyForm
from ietf.doc.forms import AdForm
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
Expand Down Expand Up @@ -205,41 +205,6 @@ def submit(request, name):
},
context_instance=RequestContext(request))


@role_required("Area Director", "Secretariat")
def edit_notices(request, name):
"""Change the set of email addresses document change notificaitions go to."""

review = get_object_or_404(Document, type="conflrev", name=name)

if request.method == 'POST':
form = NotifyForm(request.POST)
if form.is_valid():

review.notify = form.cleaned_data['notify']
review.save()

login = request.user.person
c = DocEvent(type="added_comment", doc=review, by=login)
c.desc = "Notification list changed to : "+review.notify
c.save()

return redirect('doc_view', name=review.name)

else:

init = { "notify" : review.notify }
form = NotifyForm(initial=init)

conflictdoc = review.relateddocument_set.get(relationship__slug='conflrev').target.document
titletext = 'the conflict review of %s-%s' % (conflictdoc.canonical_name(),conflictdoc.rev)
return render_to_response('doc/notify.html',
{'form': form,
'doc': review,
'titletext' : titletext
},
context_instance = RequestContext(request))

@role_required("Area Director", "Secretariat")
def edit_ad(request, name):
"""Change the shepherding Area Director for this review."""
Expand Down

0 comments on commit 640c5eb

Please sign in to comment.