From e11583a87f492953e9406ca1f5a0b36ca5bafe3c Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Fri, 12 Feb 2021 20:31:00 +0000 Subject: [PATCH] Allow assignment of Person as "action holder" for a Doc, plus rudimentary automation of assignment. Fixes #3146. Commit ready for merge. - Legacy-Id: 18829 --- ietf/doc/admin.py | 16 +- ietf/doc/expire.py | 5 +- ietf/doc/factories.py | 9 +- ietf/doc/forms.py | 11 + ietf/doc/lastcall.py | 9 +- ietf/doc/mails.py | 17 ++ ...dd_changed_action_holders_docevent_type.py | 19 ++ .../0041_add_documentactionholder.py | 35 +++ ietf/doc/models.py | 47 ++++ ietf/doc/resources.py | 21 +- ietf/doc/templatetags/ietf_filters.py | 32 +++ ietf/doc/tests.py | 101 ++++++++- ietf/doc/tests_ballot.py | 30 ++- ietf/doc/tests_draft.py | 191 ++++++++++++++-- ietf/doc/tests_utils.py | 207 ++++++++++++++++++ ietf/doc/urls.py | 2 + ietf/doc/utils.py | 90 +++++++- ietf/doc/views_ballot.py | 60 +++-- ietf/doc/views_doc.py | 160 +++++++++++++- ietf/doc/views_draft.py | 20 +- ietf/group/tests_info.py | 15 +- .../0021_email_remind_action_holders.py | 38 ++++ ietf/name/fixtures/names.json | 19 ++ ietf/person/fields.py | 28 ++- ietf/secr/telechat/tests.py | 62 +++++- ietf/secr/telechat/views.py | 14 +- ietf/settings.py | 3 + ietf/static/ietf/js/select2-field.js | 28 ++- ietf/sync/rfceditor.py | 21 +- ietf/sync/tests.py | 29 ++- ietf/templates/doc/document_draft.html | 23 ++ .../templates/doc/drafts_in_iesg_process.html | 8 +- ietf/templates/doc/edit_action_holders.html | 138 ++++++++++++ .../doc/mail/remind_action_holders_mail.txt | 14 ++ ietf/templates/doc/remind_action_holders.html | 28 +++ ietf/templates/doc/search/status_columns.html | 8 +- 36 files changed, 1458 insertions(+), 100 deletions(-) create mode 100644 ietf/doc/migrations/0040_add_changed_action_holders_docevent_type.py create mode 100644 ietf/doc/migrations/0041_add_documentactionholder.py create mode 100644 ietf/doc/tests_utils.py create mode 100644 ietf/mailtrigger/migrations/0021_email_remind_action_holders.py create mode 100644 ietf/templates/doc/edit_action_holders.html create mode 100644 ietf/templates/doc/mail/remind_action_holders_mail.txt create mode 100644 ietf/templates/doc/remind_action_holders.html diff --git a/ietf/doc/admin.py b/ietf/doc/admin.py index 67c5bcab6..b10b061b2 100644 --- a/ietf/doc/admin.py +++ b/ietf/doc/admin.py @@ -11,7 +11,7 @@ from .models import (StateType, State, RelatedDocument, DocumentAuthor, Document StateDocEvent, ConsensusDocEvent, BallotType, BallotDocEvent, WriteupDocEvent, LastCallDocEvent, TelechatDocEvent, BallotPositionDocEvent, ReviewRequestDocEvent, InitialReviewDocEvent, AddedMessageEvent, SubmissionDocEvent, DeletedEvent, EditedAuthorsDocEvent, DocumentURL, - ReviewAssignmentDocEvent, IanaExpertDocEvent, IRSGBallotDocEvent, DocExtResource ) + ReviewAssignmentDocEvent, IanaExpertDocEvent, IRSGBallotDocEvent, DocExtResource, DocumentActionHolder ) from ietf.utils.validators import validate_external_resource_value @@ -34,6 +34,11 @@ class DocAuthorInline(admin.TabularInline): raw_id_fields = ['person', 'email'] extra = 1 +class DocActionHolderInline(admin.TabularInline): + model = DocumentActionHolder + raw_id_fields = ['person'] + extra = 1 + class RelatedDocumentInline(admin.TabularInline): model = RelatedDocument def this(self, instance): @@ -72,7 +77,7 @@ class DocumentAdmin(admin.ModelAdmin): search_fields = ['name'] list_filter = ['type'] raw_id_fields = ['group', 'shepherd', 'ad'] - inlines = [DocAuthorInline, RelatedDocumentInline, AdditionalUrlInLine] + inlines = [DocAuthorInline, DocActionHolderInline, RelatedDocumentInline, AdditionalUrlInLine] form = DocumentForm def save_model(self, request, obj, form, change): @@ -137,6 +142,13 @@ class BallotTypeAdmin(admin.ModelAdmin): list_display = ["slug", "doc_type", "name", "question"] admin.site.register(BallotType, BallotTypeAdmin) + +class DocumentActionHolderAdmin(admin.ModelAdmin): + list_display = ['id', 'document', 'person', 'time_added'] + raw_id_fields = ['document', 'person'] +admin.site.register(DocumentActionHolder, DocumentActionHolderAdmin) + + # events class DocEventAdmin(admin.ModelAdmin): diff --git a/ietf/doc/expire.py b/ietf/doc/expire.py index f7efd00f1..19ddf775e 100644 --- a/ietf/doc/expire.py +++ b/ietf/doc/expire.py @@ -15,7 +15,7 @@ from ietf.utils.mail import send_mail from ietf.doc.models import Document, DocEvent, State, IESG_SUBSTATE_TAGS from ietf.person.models import Person from ietf.meeting.models import Meeting -from ietf.doc.utils import add_state_change_event +from ietf.doc.utils import add_state_change_event, update_action_holders from ietf.mailtrigger.utils import gather_address_lists @@ -171,6 +171,9 @@ def expire_draft(doc): e = add_state_change_event(doc, system, prev_state, new_state, prev_tags=prev_tags, new_tags=[]) if e: events.append(e) + e = update_action_holders(doc, prev_state, new_state, prev_tags=prev_tags, new_tags=[]) + if e: + events.append(e) events.append(DocEvent.objects.create(doc=doc, rev=doc.rev, by=system, type="expired_document", desc="Document has expired")) diff --git a/ietf/doc/factories.py b/ietf/doc/factories.py index 1e8359f14..2f74fb60b 100644 --- a/ietf/doc/factories.py +++ b/ietf/doc/factories.py @@ -12,7 +12,8 @@ from typing import Optional # pyflakes:ignore from django.conf import settings from ietf.doc.models import ( Document, DocEvent, NewRevisionDocEvent, DocAlias, State, DocumentAuthor, - StateDocEvent, BallotPositionDocEvent, BallotDocEvent, BallotType, IRSGBallotDocEvent, TelechatDocEvent) + StateDocEvent, BallotPositionDocEvent, BallotDocEvent, BallotType, IRSGBallotDocEvent, TelechatDocEvent, + DocumentActionHolder) from ietf.group.models import Group def draft_name_generator(type_id,group,n): @@ -358,3 +359,9 @@ class BallotPositionDocEventFactory(DocEventFactory): balloter = factory.SubFactory('ietf.person.factories.PersonFactory') pos_id = 'discuss' +class DocumentActionHolderFactory(factory.DjangoModelFactory): + class Meta: + model = DocumentActionHolder + + document = factory.SubFactory(WgDraftFactory) + person = factory.SubFactory('ietf.person.factories.PersonFactory') diff --git a/ietf/doc/forms.py b/ietf/doc/forms.py index 18d72587d..f9b1024fe 100644 --- a/ietf/doc/forms.py +++ b/ietf/doc/forms.py @@ -10,6 +10,8 @@ from ietf.doc.fields import SearchableDocAliasesField, SearchableDocAliasField from ietf.doc.models import RelatedDocument from ietf.iesg.models import TelechatDate from ietf.iesg.utils import telechat_page_count +from ietf.person.fields import SearchablePersonsField + class TelechatForm(forms.Form): telechat_date = forms.TypedChoiceField(coerce=lambda x: datetime.datetime.strptime(x, '%Y-%m-%d').date(), empty_value=None, required=False, help_text="Page counts are the current page counts for the telechat, before this telechat date edit is made.") @@ -54,6 +56,15 @@ class NotifyForm(forms.Form): addrspecs = [x.strip() for x in self.cleaned_data["notify"].split(',')] return ', '.join(addrspecs) +class ActionHoldersForm(forms.Form): + action_holders = SearchablePersonsField(required=False) + reason = forms.CharField( + label='Reason for change', + required=False, + max_length=255, + strip=True, + ) + IESG_APPROVED_STATE_LIST = ("ann", "rfcqueue", "pub") class AddDownrefForm(forms.Form): diff --git a/ietf/doc/lastcall.py b/ietf/doc/lastcall.py index a00fc16d2..fab3001b1 100644 --- a/ietf/doc/lastcall.py +++ b/ietf/doc/lastcall.py @@ -7,7 +7,7 @@ from django.db.models import Q from ietf.doc.models import Document, State, DocEvent, LastCallDocEvent, WriteupDocEvent from ietf.doc.models import IESG_SUBSTATE_TAGS from ietf.person.models import Person -from ietf.doc.utils import add_state_change_event +from ietf.doc.utils import add_state_change_event, update_action_holders from ietf.doc.mails import generate_ballot_writeup, generate_approval_mail, generate_last_call_announcement from ietf.doc.mails import send_last_call_request, email_last_call_expired, email_last_call_expired_with_downref @@ -60,9 +60,14 @@ def expire_last_call(doc): doc.tags.remove(*prev_tags) system = Person.objects.get(name="(System)") + events = [] e = add_state_change_event(doc, system, prev_state, new_state, prev_tags=prev_tags, new_tags=[]) if e: - doc.save_with_history([e]) + events.append(e) + e = update_action_holders(doc, prev_state, new_state, prev_tags=prev_tags, new_tags=[]) + if e: + events.append(e) + doc.save_with_history(events) email_last_call_expired(doc) diff --git a/ietf/doc/mails.py b/ietf/doc/mails.py index c3ec97210..8b2168cf7 100644 --- a/ietf/doc/mails.py +++ b/ietf/doc/mails.py @@ -15,6 +15,7 @@ from django.utils.encoding import force_text import debug # pyflakes:ignore from ietf.doc.templatetags.mail_filters import std_level_prompt +from ietf.utils import log from ietf.utils.mail import send_mail, send_mail_text from ietf.ipr.utils import iprs_from_docs, related_docs from ietf.doc.models import WriteupDocEvent, LastCallDocEvent, DocAlias, ConsensusDocEvent @@ -127,6 +128,22 @@ def email_iesg_processing_document(request, doc, changes): url=settings.IDTRACKER_BASE_URL + doc.get_absolute_url()), cc=addrs.cc) +def email_remind_action_holders(request, doc, note=None): + addrs = gather_address_lists('doc_remind_action_holders', doc=doc) + log.assertion( + 'not doc.action_holders.exclude(email__in=addrs.to).exists()', + note='All action holders should receive a reminder email. Failed for %s.' % doc.name, + ) + send_mail(request, addrs.to, None, + 'Reminder: action needed for %s' % doc.display_name(), + 'doc/mail/remind_action_holders_mail.txt', + dict( + doc=doc, + doc_url=settings.IDTRACKER_BASE_URL + doc.get_absolute_url(), + note=note, + ), + cc=addrs.cc) + def html_to_text(html): return strip_tags(html.replace("<", "<").replace(">", ">").replace("&", "&").replace("
", "\n")) diff --git a/ietf/doc/migrations/0040_add_changed_action_holders_docevent_type.py b/ietf/doc/migrations/0040_add_changed_action_holders_docevent_type.py new file mode 100644 index 000000000..3aa127871 --- /dev/null +++ b/ietf/doc/migrations/0040_add_changed_action_holders_docevent_type.py @@ -0,0 +1,19 @@ +# Generated by Django 2.2.17 on 2021-01-15 12:50 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('group', '0040_lengthen_used_roles_fields'), # only needed for schema vs data ordering + ('doc', '0039_auto_20201109_0439'), + ] + + operations = [ + migrations.AlterField( + model_name='docevent', + name='type', + field=models.CharField(choices=[('new_revision', 'Added new revision'), ('new_submission', 'Uploaded new revision'), ('changed_document', 'Changed document metadata'), ('added_comment', 'Added comment'), ('added_message', 'Added message'), ('edited_authors', 'Edited the documents author list'), ('deleted', 'Deleted document'), ('changed_state', 'Changed state'), ('changed_stream', 'Changed document stream'), ('expired_document', 'Expired document'), ('extended_expiry', 'Extended expiry of document'), ('requested_resurrect', 'Requested resurrect'), ('completed_resurrect', 'Completed resurrect'), ('changed_consensus', 'Changed consensus'), ('published_rfc', 'Published RFC'), ('added_suggested_replaces', 'Added suggested replacement relationships'), ('reviewed_suggested_replaces', 'Reviewed suggested replacement relationships'), ('changed_action_holders', 'Changed action holders for document'), ('changed_group', 'Changed group'), ('changed_protocol_writeup', 'Changed protocol writeup'), ('changed_charter_milestone', 'Changed charter milestone'), ('initial_review', 'Set initial review time'), ('changed_review_announcement', 'Changed WG Review text'), ('changed_action_announcement', 'Changed WG Action text'), ('started_iesg_process', 'Started IESG process on document'), ('created_ballot', 'Created ballot'), ('closed_ballot', 'Closed ballot'), ('sent_ballot_announcement', 'Sent ballot announcement'), ('changed_ballot_position', 'Changed ballot position'), ('changed_ballot_approval_text', 'Changed ballot approval text'), ('changed_ballot_writeup_text', 'Changed ballot writeup text'), ('changed_rfc_editor_note_text', 'Changed RFC Editor Note text'), ('changed_last_call_text', 'Changed last call text'), ('requested_last_call', 'Requested last call'), ('sent_last_call', 'Sent last call'), ('scheduled_for_telechat', 'Scheduled for telechat'), ('iesg_approved', 'IESG approved document (no problem)'), ('iesg_disapproved', 'IESG disapproved document (do not publish)'), ('approved_in_minute', 'Approved in minute'), ('iana_review', 'IANA review comment'), ('rfc_in_iana_registry', 'RFC is in IANA registry'), ('rfc_editor_received_announcement', 'Announcement was received by RFC Editor'), ('requested_publication', 'Publication at RFC Editor requested'), ('sync_from_rfc_editor', 'Received updated information from RFC Editor'), ('requested_review', 'Requested review'), ('assigned_review_request', 'Assigned review request'), ('closed_review_request', 'Closed review request'), ('closed_review_assignment', 'Closed review assignment'), ('downref_approved', 'Downref approved'), ('posted_related_ipr', 'Posted related IPR'), ('removed_related_ipr', 'Removed related IPR')], max_length=50), + ), + ] diff --git a/ietf/doc/migrations/0041_add_documentactionholder.py b/ietf/doc/migrations/0041_add_documentactionholder.py new file mode 100644 index 000000000..3832a5603 --- /dev/null +++ b/ietf/doc/migrations/0041_add_documentactionholder.py @@ -0,0 +1,35 @@ +# Generated by Django 2.2.17 on 2021-01-15 12:50 + +import datetime +from django.db import migrations, models +import django.db.models.deletion +import ietf.utils.models + + +class Migration(migrations.Migration): + + dependencies = [ + ('person', '0018_auto_20201109_0439'), + ('doc', '0040_add_changed_action_holders_docevent_type'), + ] + + operations = [ + migrations.CreateModel( + name='DocumentActionHolder', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('time_added', models.DateTimeField(default=datetime.datetime.now)), + ('document', ietf.utils.models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='doc.Document')), + ('person', ietf.utils.models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='person.Person')), + ], + ), + migrations.AddField( + model_name='document', + name='action_holders', + field=models.ManyToManyField(blank=True, through='doc.DocumentActionHolder', to='person.Person'), + ), + migrations.AddConstraint( + model_name='documentactionholder', + constraint=models.UniqueConstraint(fields=('document', 'person'), name='unique_action_holder'), + ), + ] diff --git a/ietf/doc/models.py b/ietf/doc/models.py index 93625cd15..e07f27c1c 100644 --- a/ietf/doc/models.py +++ b/ietf/doc/models.py @@ -630,6 +630,45 @@ class DocumentAuthor(DocumentAuthorInfo): return u"%s %s (%s)" % (self.document.name, self.person, self.order) +class DocumentActionHolder(models.Model): + """Action holder for a document""" + document = ForeignKey('Document') + person = ForeignKey(Person) + time_added = models.DateTimeField(default=datetime.datetime.now) + + CLEAR_ACTION_HOLDERS_STATES = ['approved', 'ann', 'rfcqueue', 'pub', 'dead'] # draft-iesg state slugs + GROUP_ROLES_OF_INTEREST = ['chair', 'techadv', 'editor', 'secr'] + + def __str__(self): + return str(self.person) + + class Meta: + constraints = [ + models.UniqueConstraint(fields=['document', 'person'], name='unique_action_holder') + ] + + def role_for_doc(self): + """Brief string description of this person's relationship to the doc""" + roles = [] + if self.person in self.document.authors(): + roles.append('Author') + if self.person == self.document.ad: + roles.append('Responsible AD') + if self.document.shepherd and self.person == self.document.shepherd.person: + roles.append('Shepherd') + if self.document.group: + roles.extend([ + 'Group %s' % role.name.name + for role in self.document.group.role_set.filter( + name__in=self.GROUP_ROLES_OF_INTEREST, + person=self.person, + ) + ]) + + if not roles: + roles.append('Action Holder') + return ', '.join(roles) + validate_docname = RegexValidator( r'^[-a-z0-9]+$', "Provide a valid document name consisting of lowercase letters, numbers and hyphens.", @@ -638,6 +677,8 @@ validate_docname = RegexValidator( class Document(DocumentInfo): name = models.CharField(max_length=255, validators=[validate_docname,], unique=True) # immutable + + action_holders = models.ManyToManyField(Person, through=DocumentActionHolder, blank=True) def __str__(self): return self.name @@ -869,6 +910,11 @@ class Document(DocumentInfo): return dh + def action_holders_enabled(self): + """Is the action holder list active for this document?""" + iesg_state = self.get_state('draft-iesg') + return iesg_state and iesg_state.slug != 'idexists' + class DocumentURL(models.Model): doc = ForeignKey(Document) tag = ForeignKey(DocUrlTagName) @@ -1000,6 +1046,7 @@ EVENT_TYPES = [ ("published_rfc", "Published RFC"), ("added_suggested_replaces", "Added suggested replacement relationships"), ("reviewed_suggested_replaces", "Reviewed suggested replacement relationships"), + ("changed_action_holders", "Changed action holders for document"), # WG events ("changed_group", "Changed group"), diff --git a/ietf/doc/resources.py b/ietf/doc/resources.py index 946667b97..b4f75fc55 100644 --- a/ietf/doc/resources.py +++ b/ietf/doc/resources.py @@ -17,7 +17,7 @@ from ietf.doc.models import (BallotType, DeletedEvent, StateType, State, Documen InitialReviewDocEvent, DocHistoryAuthor, BallotDocEvent, RelatedDocument, RelatedDocHistory, BallotPositionDocEvent, AddedMessageEvent, SubmissionDocEvent, ReviewRequestDocEvent, ReviewAssignmentDocEvent, EditedAuthorsDocEvent, DocumentURL, - IanaExpertDocEvent, IRSGBallotDocEvent, DocExtResource ) + IanaExpertDocEvent, IRSGBallotDocEvent, DocExtResource, DocumentActionHolder ) from ietf.name.resources import BallotPositionNameResource, DocTypeNameResource class BallotTypeResource(ModelResource): @@ -787,3 +787,22 @@ class DocExtResourceResource(ModelResource): "name": ALL_WITH_RELATIONS, } api.doc.register(DocExtResourceResource()) + + +from ietf.person.resources import PersonResource +class DocumentActionHolderResource(ModelResource): + document = ToOneField(DocumentResource, 'document') + person = ToOneField(PersonResource, 'person') + class Meta: + queryset = DocumentActionHolder.objects.all() + serializer = api.Serializer() + cache = SimpleCache() + #resource_name = 'documentactionholder' + ordering = ['id', ] + filtering = { + "id": ALL, + "time_added": ALL, + "document": ALL_WITH_RELATIONS, + "person": ALL_WITH_RELATIONS, + } +api.doc.register(DocumentActionHolderResource()) diff --git a/ietf/doc/templatetags/ietf_filters.py b/ietf/doc/templatetags/ietf_filters.py index 948db9238..9074b96ee 100644 --- a/ietf/doc/templatetags/ietf_filters.py +++ b/ietf/doc/templatetags/ietf_filters.py @@ -581,3 +581,35 @@ def can_ballot(user,doc): return has_role(user,'IRSG Member') else: return user.person.role_set.filter(name="ad", group__type="area", group__state="active") + +@register.filter +def action_holder_badge(action_holder): + """Add a warning tag if action holder age exceeds limit + + >>> from ietf.doc.factories import DocumentActionHolderFactory + >>> old_limit = settings.DOC_ACTION_HOLDER_AGE_LIMIT_DAYS + >>> settings.DOC_ACTION_HOLDER_AGE_LIMIT_DAYS = 15 + >>> action_holder_badge(DocumentActionHolderFactory()) + '' + + >>> action_holder_badge(DocumentActionHolderFactory(time_added=datetime.datetime.now() - datetime.timedelta(days=15))) + '' + + >>> action_holder_badge(DocumentActionHolderFactory(time_added=datetime.datetime.now() - datetime.timedelta(days=16))) + 'for 16 days' + + >>> action_holder_badge(DocumentActionHolderFactory(time_added=datetime.datetime.now() - datetime.timedelta(days=30))) + 'for 30 days' + + >>> settings.DOC_ACTION_HOLDER_AGE_LIMIT_DAYS = old_limit + """ + age_limit = settings.DOC_ACTION_HOLDER_AGE_LIMIT_DAYS + age = (datetime.datetime.now() - action_holder.time_added).days + if age > age_limit: + return mark_safe('for %d day%s' % ( + age_limit, + age, + 's' if age != 1 else '')) + else: + return '' # no alert needed + diff --git a/ietf/doc/tests.py b/ietf/doc/tests.py index ec6546618..e6a316e5e 100644 --- a/ietf/doc/tests.py +++ b/ietf/doc/tests.py @@ -9,6 +9,7 @@ import io import lxml import sys import bibtexparser +import mock if sys.version_info[0] == 2 and sys.version_info[1] < 7: import unittest2 as unittest @@ -224,6 +225,7 @@ class SearchTests(TestCase): def test_docs_for_ad(self): ad = RoleFactory(name_id='ad',group__type_id='area',group__state_id='active').person draft = IndividualDraftFactory(ad=ad) + draft.action_holders.set([PersonFactory()]) draft.set_state(State.objects.get(type='draft-iesg', slug='lc')) rfc = IndividualDraftFactory(ad=ad) rfc.set_state(State.objects.get(type='draft', slug='rfc')) @@ -243,6 +245,7 @@ class SearchTests(TestCase): r = self.client.get(urlreverse('ietf.doc.views_search.docs_for_ad', kwargs=dict(name=ad.full_name_as_key()))) self.assertEqual(r.status_code, 200) self.assertContains(r, draft.name) + self.assertContains(r, draft.action_holders.first().plain_name()) self.assertContains(r, rfc.canonical_name()) self.assertContains(r, conflrev.name) self.assertContains(r, statchg.name) @@ -265,18 +268,22 @@ class SearchTests(TestCase): def test_drafts_in_last_call(self): draft = IndividualDraftFactory(pages=1) + draft.action_holders.set([PersonFactory()]) draft.set_state(State.objects.get(type="draft-iesg", slug="lc")) r = self.client.get(urlreverse('ietf.doc.views_search.drafts_in_last_call')) self.assertEqual(r.status_code, 200) self.assertContains(r, draft.title) + self.assertContains(r, draft.action_holders.first().plain_name()) def test_in_iesg_process(self): doc_in_process = IndividualDraftFactory() + doc_in_process.action_holders.set([PersonFactory()]) doc_in_process.set_state(State.objects.get(type='draft-iesg', slug='lc')) doc_not_in_process = IndividualDraftFactory() r = self.client.get(urlreverse('ietf.doc.views_search.drafts_in_iesg_process')) self.assertEqual(r.status_code, 200) self.assertContains(r, doc_in_process.title) + self.assertContains(r, doc_in_process.action_holders.first().plain_name()) self.assertNotContains(r, doc_not_in_process.title) def test_indexes(self): @@ -323,6 +330,7 @@ class SearchTests(TestCase): drafts = WgDraftFactory.create_batch(3,states=[('draft','active'),('draft-iesg','ad-eval')]) for index, draft in enumerate(drafts): StateDocEventFactory(doc=draft, state=('draft-iesg','ad-eval'), time=datetime.datetime.now()-datetime.timedelta(days=[1,15,29][index])) + draft.action_holders.set([PersonFactory()]) # And one draft that should not show (with the default of 7 days to view) old = WgDraftFactory() @@ -335,7 +343,9 @@ class SearchTests(TestCase): q = PyQuery(r.content) self.assertEqual(len(q('td.doc')),3) self.assertEqual(q('td.status span.label-warning').text(),"for 15 days") - self.assertEqual(q('td.status span.label-danger').text(),"for 29 days") + self.assertEqual(q('td.status span.label-danger').text(),"for 29 days") + for ah in [draft.action_holders.first() for draft in drafts]: + self.assertContains(r, ah.plain_name()) class DocDraftTestCase(TestCase): draft_text = """ @@ -775,6 +785,95 @@ Man Expires September 22, 2015 [Page 3] msg_prefix='Non-WG-like group %s (%s) should not include group type in link' % (group.acronym, group.type), ) + @staticmethod + def _pyquery_select_action_holder_string(q, s): + """Helper to use PyQuery to find an action holder in the draft HTML""" + # selector grabs the action holders heading and finds siblings with a div containing the search string + return q('th:contains("Action Holders") ~ td>div:contains("%s")' % s) + + @mock.patch.object(Document, 'action_holders_enabled', return_value=False, new_callable=mock.PropertyMock) + def test_document_draft_hides_action_holders(self, mock_method): + """Draft should not show action holders when appropriate""" + draft = WgDraftFactory() + url = urlreverse("ietf.doc.views_doc.document_main", kwargs=dict(name=draft.name)) + r = self.client.get(url) + self.assertNotContains(r, 'Action Holders') # should not show action holders... + + draft.action_holders.set([PersonFactory()]) + r = self.client.get(url) + self.assertNotContains(r, 'Action Holders') # ...even if they are assigned + + @mock.patch.object(Document, 'action_holders_enabled', return_value=True, new_callable=mock.PropertyMock) + def test_document_draft_shows_action_holders(self, mock_method): + """Draft should show action holders when appropriate""" + draft = WgDraftFactory() + url = urlreverse("ietf.doc.views_doc.document_main", kwargs=dict(name=draft.name)) + + # No action holders case should be shown properly + r = self.client.get(url) + self.assertContains(r, 'Action Holders') # should show action holders + q = PyQuery(r.content) + self.assertEqual(len(self._pyquery_select_action_holder_string(q, '(None)')), 1) + + # Action holders should be listed when assigned + draft.action_holders.set(PersonFactory.create_batch(3)) + + # Make one action holder "old" + old_action_holder = draft.documentactionholder_set.first() + old_action_holder.time_added -= datetime.timedelta(days=30) + old_action_holder.save() + + with self.settings(DOC_ACTION_HOLDER_AGE_LIMIT_DAYS=20): + r = self.client.get(url) + + self.assertContains(r, 'Action Holders') # should still be shown + q = PyQuery(r.content) + self.assertEqual(len(self._pyquery_select_action_holder_string(q, '(None)')), 0) + for person in draft.action_holders.all(): + self.assertEqual(len(self._pyquery_select_action_holder_string(q, person.plain_name())), 1) + # check that one action holder was marked as old + self.assertEqual(len(self._pyquery_select_action_holder_string(q, 'for 30 days')), 1) + + @mock.patch.object(Document, 'action_holders_enabled', return_value=True, new_callable=mock.PropertyMock) + def test_document_draft_action_holders_buttons(self, mock_method): + """Buttons for action holders should be shown when AD or secretary""" + draft = WgDraftFactory() + draft.action_holders.set([PersonFactory()]) + + url = urlreverse('ietf.doc.views_doc.document_main', kwargs=dict(name=draft.name)) + edit_ah_url = urlreverse('ietf.doc.views_doc.edit_action_holders', kwargs=dict(name=draft.name)) + remind_ah_url = urlreverse('ietf.doc.views_doc.remind_action_holders', kwargs=dict(name=draft.name)) + + def _run_test(username=None, expect_buttons=False): + if username: + self.client.login(username=username, password=username + '+password') + r = self.client.get(url) + q = PyQuery(r.content) + + self.assertEqual( + len(q('th:contains("Action Holders") ~ td a[href="%s"]' % edit_ah_url)), + 1 if expect_buttons else 0, + '%s should%s see the edit action holders button but %s' % ( + username if username else 'unauthenticated user', + '' if expect_buttons else ' not', + 'did not' if expect_buttons else 'did', + ) + ) + self.assertEqual( + len(q('th:contains("Action Holders") ~ td a[href="%s"]' % remind_ah_url)), + 1 if expect_buttons else 0, + '%s should%s see the remind action holders button but %s' % ( + username if username else 'unauthenticated user', + '' if expect_buttons else ' not', + 'did not' if expect_buttons else 'did', + ) + ) + + _run_test(None, False) + _run_test('plain', False) + _run_test('ad', True) + _run_test('secretary', True) + def test_draft_group_link(self): """Link to group 'about' page should have correct format""" for group_type_id in ['wg', 'rg', 'ag']: diff --git a/ietf/doc/tests_ballot.py b/ietf/doc/tests_ballot.py index 2c5e47a48..cfcaf89ff 100644 --- a/ietf/doc/tests_ballot.py +++ b/ietf/doc/tests_ballot.py @@ -339,6 +339,8 @@ class BallotWriteupsTests(TestCase): send_last_call_request="1")) draft = Document.objects.get(name=draft.name) self.assertEqual(draft.get_state_slug("draft-iesg"), "lc-req") + self.assertCountEqual(draft.action_holders.all(), [ad]) + self.assertIn('Changed action holders', draft.latest_event(type='changed_action_holders').desc) self.assertEqual(len(outbox), mailbox_before + 1) self.assertTrue("Last Call" in outbox[-1]['Subject']) self.assertTrue(draft.name in outbox[-1]['Subject']) @@ -501,6 +503,7 @@ class BallotWriteupsTests(TestCase): self.assertEqual(len(q('textarea[name=ballot_writeup]')), 1) self.assertFalse(q('[class=help-block]:contains("not completed IETF Last Call")')) self.assertTrue(q('[type=submit]:contains("Save")')) + self.assertCountEqual(draft.action_holders.all(), []) # save r = self.client.post(url, dict( @@ -508,7 +511,8 @@ class BallotWriteupsTests(TestCase): issue_ballot="1")) self.assertEqual(r.status_code, 200) d = Document.objects.get(name=draft.name) - self.assertTrue('iesg-eva' == d.get_state_slug('draft-iesg')) + self.assertTrue('iesg-eva' == d.get_state_slug('draft-iesg')) + self.assertCountEqual(draft.action_holders.all(), [ad]) def test_issue_ballot_warn_if_early(self): ad = Person.objects.get(user__username="ad") @@ -727,13 +731,17 @@ class ApproveBallotTests(TestCase): self.assertTrue(not outbox[-1]['CC']) self.assertTrue('drafts-approval@icann.org' in outbox[-1]['To']) self.assertTrue("Protocol Action" in draft.message_set.order_by("-time")[0].subject) + # in 'ann' state, action holders should be empty + self.assertCountEqual(draft.action_holders.all(), []) def test_disapprove_ballot(self): # This tests a codepath that is not used in production # and that has already had some drift from usefulness (it results in a # older-style conflict review response). - draft = IndividualDraftFactory() + ad = Person.objects.get(name="Areað Irector") + draft = IndividualDraftFactory(ad=ad) draft.set_state(State.objects.get(used=True, type="draft-iesg", slug="nopubadw")) + draft.action_holders.set([ad]) url = urlreverse('ietf.doc.views_ballot.approve_ballot', kwargs=dict(name=draft.name)) login_testing_unauthorized(self, "secretary", url) @@ -748,6 +756,8 @@ class ApproveBallotTests(TestCase): self.assertEqual(draft.get_state_slug("draft-iesg"), "dead") self.assertEqual(len(outbox), mailbox_before + 1) self.assertTrue("NOT be published" in str(outbox[-1])) + self.assertCountEqual(draft.action_holders.all(), []) + self.assertIn('Removed all action holders', draft.latest_event(type='changed_action_holders').desc) def test_clear_ballot(self): draft = IndividualDraftFactory() @@ -846,6 +856,7 @@ class MakeLastCallTests(TestCase): draft = Document.objects.get(name=draft.name) self.assertEqual(draft.get_state_slug("draft-iesg"), "lc") self.assertEqual(draft.latest_event(LastCallDocEvent, "sent_last_call").expires.strftime("%Y-%m-%d"), expire_date) + self.assertCountEqual(draft.action_holders.all(), [ad]) self.assertEqual(len(outbox), mailbox_before + 2) @@ -940,7 +951,12 @@ class DeferUndeferTestCase(TestCase): if doc.type_id in defer_states: self.assertEqual(doc.get_state(defer_states[doc.type_id][0]).slug,defer_states[doc.type_id][1]) self.assertTrue(doc.active_defer_event()) - + if doc.type_id == 'draft': + self.assertCountEqual(doc.action_holders.all(), [doc.ad]) + self.assertIn('Changed action holders', doc.latest_event(type='changed_action_holders').desc) + else: + self.assertIsNone(doc.latest_event(type='changed_action_holders')) + self.assertEqual(len(outbox), mailbox_before + 2) self.assertTrue('Telechat update' in outbox[-2]['Subject']) @@ -1000,6 +1016,11 @@ class DeferUndeferTestCase(TestCase): if doc.type_id in undefer_states: self.assertEqual(doc.get_state(undefer_states[doc.type_id][0]).slug,undefer_states[doc.type_id][1]) self.assertFalse(doc.active_defer_event()) + if doc.type_id == 'draft': + self.assertCountEqual(doc.action_holders.all(), [doc.ad]) + self.assertIn('Changed action holders', doc.latest_event(type='changed_action_holders').desc) + else: + self.assertIsNone(doc.latest_event(type='changed_action_holders')) self.assertEqual(len(outbox), mailbox_before + 2) self.assertTrue("Telechat update" in outbox[-2]['Subject']) self.assertTrue('iesg-secretary@' in outbox[-2]['To']) @@ -1035,7 +1056,8 @@ class DeferUndeferTestCase(TestCase): # when charters support being deferred, be sure to test them here def setUp(self): - IndividualDraftFactory(name='draft-ietf-mars-test',states=[('draft','active'),('draft-iesg','iesg-eva')]) + IndividualDraftFactory(name='draft-ietf-mars-test',states=[('draft','active'),('draft-iesg','iesg-eva')], + ad=Person.objects.get(user__username='ad')) DocumentFactory(type_id='statchg',name='status-change-imaginary-mid-review',states=[('statchg','iesgeval')]) DocumentFactory(type_id='conflrev',name='conflict-review-imaginary-irtf-submission',states=[('conflrev','iesgeval')]) diff --git a/ietf/doc/tests_draft.py b/ietf/doc/tests_draft.py index af06a8eb0..67b25f94c 100644 --- a/ietf/doc/tests_draft.py +++ b/ietf/doc/tests_draft.py @@ -25,7 +25,7 @@ from ietf.doc.utils import get_tags_for_stream_id, create_ballot_if_not_open from ietf.name.models import StreamName, DocTagName from ietf.group.factories import GroupFactory, RoleFactory from ietf.group.models import Group, Role -from ietf.person.factories import PersonFactory +from ietf.person.factories import PersonFactory, EmailFactory from ietf.person.models import Person, Email from ietf.meeting.models import Meeting, MeetingTypeName from ietf.iesg.models import TelechatDate @@ -41,6 +41,7 @@ class ChangeStateTests(TestCase): draft = WgDraftFactory(ad=ad,states=[('draft','active'),('draft-iesg','iesg-eva')]) DocEventFactory(type='started_iesg_process',by=ad,doc=draft,rev=draft.rev,desc="Started IESG Process") draft.tags.add("ad-f-up") + draft.action_holders.add(ad) url = urlreverse('ietf.doc.views_draft.change_state', kwargs=dict(name=draft.name)) login_testing_unauthorized(self, "ad", url) @@ -67,10 +68,12 @@ class ChangeStateTests(TestCase): self.assertEqual(draft.get_state_slug("draft-iesg"), "approved") self.assertTrue(not draft.tags.filter(slug="approved")) self.assertFalse(draft.tags.exists()) - self.assertEqual(draft.docevent_set.count(), events_before + 2) + self.assertEqual(draft.docevent_set.count(), events_before + 3) self.assertTrue("Test comment" in draft.docevent_set.all()[0].desc) - self.assertTrue("IESG state changed" in draft.docevent_set.all()[1].desc) - + self.assertTrue("Removed all action holders" in draft.docevent_set.all()[1].desc) + self.assertTrue("IESG state changed" in draft.docevent_set.all()[2].desc) + self.assertCountEqual(draft.action_holders.all(), []) + # should have sent two emails, the second one to the iesg with approved message self.assertEqual(len(outbox), mailbox_before + 2) self.assertTrue("Approved: " in outbox[-1]['Subject']) @@ -79,8 +82,15 @@ class ChangeStateTests(TestCase): def test_change_state(self): ad = Person.objects.get(user__username="ad") - draft = WgDraftFactory(name='draft-ietf-mars-test',group__acronym='mars',ad=ad,states=[('draft','active'),('draft-iesg','ad-eval')]) + draft = WgDraftFactory( + name='draft-ietf-mars-test', + group__acronym='mars', + ad=ad, + authors=PersonFactory.create_batch(3), + states=[('draft','active'),('draft-iesg','ad-eval')] + ) DocEventFactory(type='started_iesg_process',by=ad,doc=draft,rev=draft.rev,desc="Started IESG Process") + draft.action_holders.add(ad) url = urlreverse('ietf.doc.views_draft.change_state', kwargs=dict(name=draft.name)) login_testing_unauthorized(self, "secretary", url) @@ -105,7 +115,7 @@ class ChangeStateTests(TestCase): self.assertTrue(len(q('form .has-error')) > 0) draft = Document.objects.get(name=draft.name) self.assertEqual(draft.get_state("draft-iesg"), first_state) - + self.assertCountEqual(draft.action_holders.all(), [ad]) # change state events_before = draft.docevent_set.count() @@ -122,9 +132,11 @@ class ChangeStateTests(TestCase): self.assertEqual(draft.get_state_slug("draft-iesg"), "review-e") self.assertTrue(not draft.tags.filter(slug="ad-f-up")) self.assertTrue(draft.tags.filter(slug="need-rev")) - self.assertEqual(draft.docevent_set.count(), events_before + 2) + self.assertCountEqual(draft.action_holders.all(), [ad] + draft.authors()) + self.assertEqual(draft.docevent_set.count(), events_before + 3) self.assertTrue("Test comment" in draft.docevent_set.all()[0].desc) - self.assertTrue("IESG state changed" in draft.docevent_set.all()[1].desc) + self.assertTrue("Changed action holders" in draft.docevent_set.all()[1].desc) + self.assertTrue("IESG state changed" in draft.docevent_set.all()[2].desc) self.assertEqual(len(outbox), mailbox_before + 1) self.assertTrue("State Update Notice" in outbox[-1]['Subject']) self.assertTrue('draft-ietf-mars-test@' in outbox[-1]['To']) @@ -139,8 +151,13 @@ class ChangeStateTests(TestCase): def test_pull_from_rfc_queue(self): ad = Person.objects.get(user__username="ad") - draft = WgDraftFactory(ad=ad,states=[('draft-iesg','rfcqueue')]) + draft = WgDraftFactory( + ad=ad, + authors=PersonFactory.create_batch(3), + states=[('draft-iesg','rfcqueue')], + ) DocEventFactory(type='started_iesg_process',by=ad,doc=draft,rev=draft.rev,desc="Started IESG Process") + draft.action_holders.add(*(draft.authors())) url = urlreverse('ietf.doc.views_draft.change_state', kwargs=dict(name=draft.name)) login_testing_unauthorized(self, "secretary", url) @@ -156,7 +173,7 @@ class ChangeStateTests(TestCase): draft = Document.objects.get(name=draft.name) self.assertEqual(draft.get_state_slug("draft-iesg"), "review-e") - + self.assertCountEqual(draft.action_holders.all(), [ad]) self.assertEqual(len(outbox), mailbox_before + 2) self.assertTrue(draft.name in outbox[-1]['Subject']) @@ -234,8 +251,13 @@ class ChangeStateTests(TestCase): def test_request_last_call(self): ad = Person.objects.get(user__username="ad") - draft = WgDraftFactory(ad=ad,states=[('draft-iesg','ad-eval')]) + draft = WgDraftFactory( + ad=ad, + authors=PersonFactory.create_batch(3), + states=[('draft-iesg','ad-eval')], + ) DocEventFactory(type='started_iesg_process',by=ad,doc=draft,rev=draft.rev,desc="Started IESG Process") + draft.action_holders.add(*(draft.authors())) self.client.login(username="secretary", password="secretary+password") url = urlreverse('ietf.doc.views_draft.change_state', kwargs=dict(name=draft.name)) @@ -247,6 +269,8 @@ class ChangeStateTests(TestCase): self.assertEqual(r.status_code,200) self.assertContains(r, "Your request to issue") + draft = Document.objects.get(name=draft.name) + # last call text e = draft.latest_event(WriteupDocEvent, type="changed_last_call_text") self.assertTrue(e) @@ -279,6 +303,9 @@ class ChangeStateTests(TestCase): # comment self.assertTrue("Last call was requested" in draft.latest_event().desc) + # action holders + self.assertCountEqual(draft.action_holders.all(), [ad]) + class EditInfoTests(TestCase): def test_edit_info(self): @@ -449,9 +476,10 @@ class EditInfoTests(TestCase): self.assertEqual(draft.ad, ad) self.assertEqual(draft.note, "This is a note") self.assertTrue(not draft.latest_event(TelechatDocEvent, type="scheduled_for_telechat")) - self.assertEqual(draft.docevent_set.count(), events_before + 4) + self.assertEqual(draft.docevent_set.count(), events_before + 5) + self.assertCountEqual(draft.action_holders.all(), [draft.ad]) events = list(draft.docevent_set.order_by('time', 'id')) - self.assertEqual(events[-4].type, "started_iesg_process") + self.assertEqual(events[-5].type, "started_iesg_process") self.assertEqual(len(outbox), mailbox_before+1) self.assertTrue('IESG processing' in outbox[-1]['Subject']) self.assertTrue('draft-ietf-mars-test2@' in outbox[-1]['To']) @@ -460,6 +488,7 @@ class EditInfoTests(TestCase): draft.set_state(State.objects.get(type_id='draft-iesg', slug='idexists')) draft.set_state(State.objects.get(type='draft-stream-ietf',slug='writeupw')) draft.stream = StreamName.objects.get(slug='ietf') + draft.action_holders.clear() draft.save_with_history([DocEvent.objects.create(doc=draft, rev=draft.rev, type="changed_stream", by=Person.objects.get(user__username="secretary"), desc="Test")]) r = self.client.post(url, dict(intended_std_level=str(draft.intended_std_level_id), @@ -473,6 +502,7 @@ class EditInfoTests(TestCase): draft = Document.objects.get(name=draft.name) self.assertEqual(draft.get_state_slug('draft-iesg'),'pub-req') self.assertEqual(draft.get_state_slug('draft-stream-ietf'),'sub-pub') + self.assertCountEqual(draft.action_holders.all(), [draft.ad]) def test_edit_consensus(self): draft = WgDraftFactory() @@ -668,8 +698,9 @@ class ExpireIDsTests(DraftFileMixin, TestCase): def test_expire_drafts(self): mars = GroupFactory(type_id='wg',acronym='mars') - ad_role = RoleFactory(group=mars, name_id='ad', person=Person.objects.get(user__username='ad')) - draft = WgDraftFactory(name='draft-ietf-mars-test',group=mars) + ad = Person.objects.get(user__username='ad') + ad_role = RoleFactory(group=mars, name_id='ad', person=ad) + draft = WgDraftFactory(name='draft-ietf-mars-test',group=mars,ad=ad) DocEventFactory(type='started_iesg_process',by=ad_role.person,doc=draft,rev=draft.rev,desc="Started IESG Process") self.assertEqual(len(list(get_expired_drafts())), 0) @@ -689,6 +720,8 @@ class ExpireIDsTests(DraftFileMixin, TestCase): self.assertEqual(len(list(get_expired_drafts())), 0) + draft.action_holders.set([draft.ad]) + # test notice mailbox_before = len(outbox) @@ -710,6 +743,8 @@ class ExpireIDsTests(DraftFileMixin, TestCase): self.assertEqual(draft.get_state_slug(), "expired") self.assertEqual(draft.get_state_slug("draft-iesg"), "dead") self.assertTrue(draft.latest_event(type="expired_document")) + self.assertCountEqual(draft.action_holders.all(), []) + self.assertIn('Removed all action holders', draft.latest_event(type='changed_action_holders').desc) self.assertTrue(not os.path.exists(os.path.join(self.id_dir, txt))) self.assertTrue(os.path.exists(os.path.join(self.archive_dir, txt))) @@ -815,12 +850,14 @@ class ExpireLastCallTests(TestCase): # expire it mailbox_before = len(outbox) events_before = draft.docevent_set.count() - + expire_last_call(drafts[0]) draft = Document.objects.get(name=draft.name) self.assertEqual(draft.get_state_slug("draft-iesg"), "writeupw") - self.assertEqual(draft.docevent_set.count(), events_before + 1) + self.assertEqual(draft.docevent_set.count(), events_before + 2) + self.assertCountEqual(draft.action_holders.all(), [ad]) + self.assertIn('Changed action holders', draft.latest_event(type='changed_action_holders').desc) self.assertEqual(len(outbox), mailbox_before + 1) self.assertTrue("Last Call Expired" in outbox[-1]["Subject"]) self.assertTrue('iesg-secretary@' in outbox[-1]['Cc']) @@ -844,13 +881,15 @@ class ExpireLastCallTests(TestCase): drafts = list(get_expired_last_calls()) self.assertEqual(len(drafts), 1) - mailbox_before = len(outbox) + mailbox_before = len(outbox) expire_last_call(drafts[0]) d = Document.objects.get(name=draft.name) self.assertEqual(len(outbox), mailbox_before + 2) self.assertTrue("Review Downrefs From Expired Last Call" in outbox[-1]["Subject"]) self.assertTrue(d.ad.email().address in outbox[-1]['To']) + self.assertCountEqual(d.action_holders.all(), [ad]) + self.assertIn('Changed action holders', d.latest_event(type='changed_action_holders').desc) class IndividualInfoFormsTests(TestCase): @@ -1202,12 +1241,117 @@ class IndividualInfoFormsTests(TestCase): self.assertEqual(doc.docextresource_set.get(name__slug='github_repo').display_name, 'Some display text') self.assertIn(doc.docextresource_set.first().name.slug,str(doc.docextresource_set.first())) + # This is in views_doc, not views_draft, but there's already mixing and this keeps it with similar tests + def do_doc_change_action_holders_test(self, username): + # Set up people related to the doc to be sure shortcut buttons appear. + doc = Document.objects.get(name=self.docname) + doc.documentauthor_set.create(person=PersonFactory()) + doc.ad = Person.objects.get(user__username='ad') + doc.shepherd = EmailFactory() + doc.save_with_history([DocEvent.objects.create(doc=doc, rev=doc.rev, type="changed_shepherd", by=Person.objects.get(user__username="secretary"), desc="Test")]) + RoleFactory(name_id='chair', person=PersonFactory(), group=doc.group) + RoleFactory(name_id='techadv', person=PersonFactory(), group=doc.group) + RoleFactory(name_id='editor', person=PersonFactory(), group=doc.group) + RoleFactory(name_id='secr', person=PersonFactory(), group=doc.group) + + url = urlreverse('ietf.doc.views_doc.edit_action_holders', kwargs=dict(name=doc.name)) + + login_testing_unauthorized(self, username, url) + + r = self.client.get(url) + self.assertEqual(r.status_code, 200) + q = PyQuery(r.content) + self.assertEqual(len(q('form input[id=id_reason]')), 1) + self.assertEqual(len(q('form input[id=id_action_holders]')), 1) + for role_name in [ + 'Author', + 'Responsible AD', + 'Shepherd', + 'Group Chair', + 'Group Tech Advisor', + 'Group Editor', + 'Group Secretary', + ]: + self.assertEqual(len(q('button:contains("Add %s")' % role_name)), 1, + 'Expected "Add %s" button' % role_name) + self.assertEqual(len(q('button:contains("Remove %s")' % role_name)), 1, + 'Expected "Remove %s" button for' % role_name) + + def _test_changing_ah(action_holders, reason): + r = self.client.post(url, dict( + reason=reason, + action_holders=','.join([str(p.pk) for p in action_holders]), + )) + self.assertEqual(r.status_code, 302) + doc = Document.objects.get(name=self.docname) + self.assertCountEqual(doc.action_holders.all(), action_holders) + event = doc.latest_event(type='changed_action_holders') + self.assertIn(reason, event.desc) + if action_holders: + for ah in action_holders: + self.assertIn(ah.plain_name(), event.desc) + else: + self.assertIn('Removed all', event.desc) + + _test_changing_ah([doc.ad, doc.shepherd.person], 'this is a first test') + _test_changing_ah([doc.ad], 'this is a second test') + _test_changing_ah(doc.authors(), 'authors can do it, too') + _test_changing_ah([], 'clear it back out') + + def test_doc_change_action_holders_as_secretary(self): + self.do_doc_change_action_holders_test('secretary') + + def test_doc_change_action_holders_as_ad(self): + self.do_doc_change_action_holders_test('ad') + + def do_doc_remind_action_holders_test(self, username): + doc = Document.objects.get(name=self.docname) + doc.action_holders.set(PersonFactory.create_batch(3)) + + url = urlreverse('ietf.doc.views_doc.remind_action_holders', kwargs=dict(name=doc.name)) + + login_testing_unauthorized(self, username, url) + r = self.client.get(url) + self.assertEqual(r.status_code, 200) + q = PyQuery(r.content) + self.assertEqual(len(q('form textarea[id=id_note]')), 1) + self.assertEqual(len(q('button:contains("Send")')), 1) + for ah in doc.action_holders.all(): + self.assertContains(r, ah.plain_name()) + + empty_outbox() + r = self.client.post(url, dict(note='this is my note')) # note should be < 78 chars to avoid wrapping + self.assertEqual(r.status_code, 302) + + self.assertEqual(len(outbox), 1) + for ah in doc.action_holders.all(): + self.assertIn('Reminder: action needed', outbox[0]['Subject']) + self.assertIn(ah.email_address(), outbox[0]['To']) + self.assertIn(doc.display_name(), outbox[0].as_string()) + self.assertIn(doc.get_absolute_url(), outbox[0].as_string()) + self.assertIn('this is my note', outbox[0].as_string()) + + # check that nothing is sent when no action holders + doc.action_holders.clear() + self.client.post(url) + self.assertEqual(len(outbox), 1) # still 1 + + def test_doc_remind_action_holders_as_ad(self): + self.do_doc_remind_action_holders_test('ad') + + def test_doc_remind_action_holders_as_secretary(self): + self.do_doc_remind_action_holders_test('secretary') class SubmitToIesgTests(TestCase): def setUp(self): role=RoleFactory(group__acronym='mars',name_id='chair',person=PersonFactory(user__username='marschairman')) - doc=WgDraftFactory(name='draft-ietf-mars-test',group=role.group,ad=Person.objects.get(user__username='ad')) + doc=WgDraftFactory( + name='draft-ietf-mars-test', + group=role.group, + ad=Person.objects.get(user__username='ad'), + authors=PersonFactory.create_batch(3), + ) self.docname=doc.name def test_verify_permissions(self): @@ -1242,6 +1386,7 @@ class SubmitToIesgTests(TestCase): doc = Document.objects.get(name=self.docname) self.assertEqual(doc.get_state_slug('draft-iesg'),'idexists') + self.assertCountEqual(doc.action_holders.all(), []) def test_confirm_submission(self): url = urlreverse('ietf.doc.views_draft.to_iesg', kwargs=dict(name=self.docname)) @@ -1258,16 +1403,14 @@ class SubmitToIesgTests(TestCase): self.assertTrue(doc.get_state('draft-iesg').slug=='pub-req') self.assertTrue(doc.get_state('draft-stream-ietf').slug=='sub-pub') - # It's not clear what this testing - the view can certainly - # leave the document without an ad. This line as written only - # checks whether the setup document had an ad or not. - self.assertTrue(doc.ad!=None) + self.assertCountEqual(doc.action_holders.all(), [doc.ad]) new_docevents = set(doc.docevent_set.all()) - docevents_pre - self.assertEqual(len(new_docevents),3) + self.assertEqual(len(new_docevents), 4) new_docevent_type_count = Counter([e.type for e in new_docevents]) self.assertEqual(new_docevent_type_count['changed_state'],2) self.assertEqual(new_docevent_type_count['started_iesg_process'],1) + self.assertEqual(new_docevent_type_count['changed_action_holders'], 1) self.assertEqual(len(outbox), mailbox_before + 1) self.assertTrue("Publication has been requested" in outbox[-1]['Subject']) diff --git a/ietf/doc/tests_utils.py b/ietf/doc/tests_utils.py new file mode 100644 index 000000000..7bc5adab6 --- /dev/null +++ b/ietf/doc/tests_utils.py @@ -0,0 +1,207 @@ +# Copyright The IETF Trust 2020, All Rights Reserved +import datetime + +from ietf.group.factories import GroupFactory, RoleFactory +from ietf.name.models import DocTagName +from ietf.person.factories import PersonFactory +from ietf.utils.test_utils import TestCase +from ietf.person.models import Person +from ietf.doc.factories import DocumentFactory +from ietf.doc.models import State, DocumentActionHolder +from ietf.doc.utils import update_action_holders, add_state_change_event + + +class ActionHoldersTests(TestCase): + + def setUp(self): + """Set up helper for the update_action_holders tests""" + self.authors = PersonFactory.create_batch(3) + self.ad = Person.objects.get(user__username='ad') + self.group = GroupFactory() + RoleFactory(name_id='ad', group=self.group, person=self.ad) + + def doc_in_iesg_state(self, slug): + return DocumentFactory(authors=self.authors, group=self.group, ad=self.ad, states=[('draft-iesg', slug)]) + + def update_doc_state(self, doc, new_state, add_tags=None, remove_tags=None): + """Update document state/tags, create change event, and save""" + prev_tags = list(doc.tags.all()) # list to make sure we retrieve now + # prev_action_holders = list(doc.action_holders.all()) + + prev_state = doc.get_state(new_state.type_id) + if new_state != prev_state: + doc.set_state(new_state) + + if add_tags: + doc.tags.add(*DocTagName.objects.filter(slug__in=add_tags)) + if remove_tags: + doc.tags.remove(*DocTagName.objects.filter(slug__in=remove_tags)) + new_tags = list(doc.tags.all()) + + events = [] + e = add_state_change_event( + doc, + Person.objects.get(name='(System)'), + prev_state, new_state, + prev_tags, new_tags) + self.assertIsNotNone(e, 'Test logic error') + events.append(e) + e = update_action_holders(doc, prev_state, new_state, prev_tags, new_tags) + if e: + events.append(e) + doc.save_with_history(events) + + + def test_update_action_holders_by_state(self): + """Doc action holders should auto-update correctly on state change""" + # Test the transition from every state to each of its 'next_states' + + for initial_state in State.objects.filter(type__slug='draft-iesg'): + for next_state in initial_state.next_states.all(): + # Test with no action holders initially + doc = DocumentFactory( + authors=self.authors, + group=self.group, + ad=self.ad, + states=[('draft-iesg', initial_state.slug)], + ) + docevents_before = set(doc.docevent_set.all()) + + self.update_doc_state(doc, next_state) + + new_docevents = set(doc.docevent_set.all()).difference(docevents_before) + self.assertIn(doc.latest_event(type='changed_state'), new_docevents) + + if next_state.slug in DocumentActionHolder.CLEAR_ACTION_HOLDERS_STATES: + self.assertCountEqual(doc.action_holders.all(), []) + self.assertEqual(len(new_docevents), 1) + else: + self.assertCountEqual( + doc.action_holders.all(), [doc.ad], + 'AD should be only action holder after transition to %s' % next_state.slug) + + self.assertEqual(len(new_docevents), 2) + change_event = doc.latest_event(type='changed_action_holders') + self.assertIn(change_event, new_docevents) + self.assertIn('Changed action holders', change_event.desc) + self.assertIn(doc.ad.name, change_event.desc) + doc.delete() # clean up for next iteration + + # Test with action holders initially + doc = DocumentFactory( + authors=self.authors, + group=self.group, + ad=self.ad, + states=[('draft-iesg', initial_state.slug)], + ) + doc.action_holders.add(*self.authors) # adds all authors + docevents_before = set(doc.docevent_set.all()) + + self.update_doc_state(doc, next_state) + + new_docevents = set(doc.docevent_set.all()).difference(docevents_before) + self.assertEqual(len(new_docevents), 2) + self.assertIn(doc.latest_event(type='changed_state'), new_docevents) + change_event = doc.latest_event(type='changed_action_holders') + self.assertIn(change_event, new_docevents) + + if next_state.slug in DocumentActionHolder.CLEAR_ACTION_HOLDERS_STATES: + self.assertCountEqual(doc.action_holders.all(), []) + self.assertIn('Removed all action holders', change_event.desc) + else: + self.assertCountEqual( + doc.action_holders.all(), [doc.ad], + 'AD should be only action holder after transition to %s' % next_state.slug) + self.assertIn('Changed action holders', change_event.desc) + self.assertIn(doc.ad.name, change_event.desc) + doc.delete() # clean up for next iteration + + def test_update_action_holders_with_no_ad(self): + """A document with no AD should be handled gracefully""" + doc = self.doc_in_iesg_state('idexists') + doc.ad = None + doc.save() + + docevents_before = set(doc.docevent_set.all()) + self.update_doc_state(doc, State.objects.get(slug='pub-req')) + new_docevents = set(doc.docevent_set.all()).difference(docevents_before) + self.assertEqual(len(new_docevents), 1) + self.assertIn(doc.latest_event(type='changed_state'), new_docevents) + self.assertCountEqual(doc.action_holders.all(), []) + + def test_update_action_holders_resets_age(self): + """Action holder age should reset when document state changes""" + doc = self.doc_in_iesg_state('pub-req') + doc.action_holders.set([self.ad]) + dah = doc.documentactionholder_set.get(person=self.ad) + dah.time_added = datetime.datetime(2020, 1, 1) # arbitrary date in the past + dah.save() + + self.assertNotEqual(doc.documentactionholder_set.get(person=self.ad).time_added.date(), datetime.date.today()) + self.update_doc_state(doc, State.objects.get(slug='ad-eval')) + self.assertEqual(doc.documentactionholder_set.get(person=self.ad).time_added.date(), datetime.date.today()) + + def test_update_action_holders_add_tag_need_rev(self): + """Adding need-rev tag adds authors as action holders""" + doc = self.doc_in_iesg_state('pub-req') + first_author = self.authors[0] + doc.action_holders.add(first_author) + self.assertCountEqual(doc.action_holders.all(), [first_author]) + self.update_doc_state(doc, + doc.get_state('draft-iesg'), + add_tags=['need-rev'], + remove_tags=None) + self.assertCountEqual(doc.action_holders.all(), self.authors) + + def test_update_action_holders_add_tag_need_rev_no_dups(self): + """Adding need-rev tag does not duplicate existing action holders""" + doc = self.doc_in_iesg_state('pub-req') + self.assertCountEqual(doc.action_holders.all(), []) + self.update_doc_state(doc, + doc.get_state('draft-iesg'), + add_tags=['need-rev'], + remove_tags=None) + self.assertCountEqual(doc.action_holders.all(), self.authors) + + def test_update_action_holders_remove_tag_need_rev(self): + """Removing need-rev tag drops authors as action holders""" + doc = self.doc_in_iesg_state('pub-req') + doc.tags.add(DocTagName.objects.get(slug='need-rev')) + self.assertEqual(doc.action_holders.count(), 0) + self.update_doc_state(doc, + doc.get_state('draft-iesg'), + add_tags=None, + remove_tags=['need-rev']) + self.assertEqual(doc.action_holders.count(), 0) + + def test_update_action_holders_add_tag_need_rev_ignores_non_authors(self): + """Adding need-rev tag does not affect existing action holders""" + doc = self.doc_in_iesg_state('pub-req') + doc.action_holders.add(self.ad) + self.assertCountEqual(doc.action_holders.all(),[self.ad]) + self.update_doc_state(doc, + doc.get_state('draft-iesg'), + add_tags=['need-rev'], + remove_tags=None) + self.assertCountEqual(doc.action_holders.all(), [self.ad] + self.authors) + + def test_update_action_holders_remove_tag_need_rev_ignores_non_authors(self): + """Removing need-rev tag does not affect non-author action holders""" + doc = self.doc_in_iesg_state('pub-req') + doc.tags.add(DocTagName.objects.get(slug='need-rev')) + doc.action_holders.add(self.ad) + self.assertCountEqual(doc.action_holders.all(), [self.ad]) + self.update_doc_state(doc, + doc.get_state('draft-iesg'), + add_tags=None, + remove_tags=['need-rev']) + self.assertCountEqual(doc.action_holders.all(), [self.ad]) + + def test_doc_action_holders_enabled(self): + """Action holders should only be enabled in certain states""" + doc = self.doc_in_iesg_state('idexists') + self.assertFalse(doc.action_holders_enabled()) + + for state in State.objects.filter(type='draft-iesg').exclude(slug='idexists'): + doc.set_state(state) + self.assertTrue(doc.action_holders_enabled()) \ No newline at end of file diff --git a/ietf/doc/urls.py b/ietf/doc/urls.py index b9aa9b2f8..bec7d3524 100644 --- a/ietf/doc/urls.py +++ b/ietf/doc/urls.py @@ -104,6 +104,8 @@ urlpatterns = [ url(r'^%(name)s/edit/stream/$' % settings.URL_REGEXPS, views_draft.change_stream), url(r'^%(name)s/edit/replaces/$' % settings.URL_REGEXPS, views_draft.replaces), url(r'^%(name)s/edit/notify/$' % settings.URL_REGEXPS, views_doc.edit_notify), + url(r'^%(name)s/edit/actionholders/$' % settings.URL_REGEXPS, views_doc.edit_action_holders), + url(r'^%(name)s/edit/remindactionholders/$' % settings.URL_REGEXPS, views_doc.remind_action_holders), url(r'^%(name)s/edit/suggested-replaces/$' % settings.URL_REGEXPS, views_draft.review_possibly_replaces), url(r'^%(name)s/edit/status/$' % settings.URL_REGEXPS, views_draft.change_intention), url(r'^%(name)s/edit/telechat/$' % settings.URL_REGEXPS, views_doc.telechat_date), diff --git a/ietf/doc/utils.py b/ietf/doc/utils.py index 055e16876..93b9187e7 100644 --- a/ietf/doc/utils.py +++ b/ietf/doc/utils.py @@ -26,7 +26,7 @@ from ietf.community.utils import docs_tracked_by_community_list from ietf.doc.models import Document, DocHistory, State, DocumentAuthor, DocHistoryAuthor from ietf.doc.models import DocAlias, RelatedDocument, RelatedDocHistory, BallotType, DocReminder from ietf.doc.models import DocEvent, ConsensusDocEvent, BallotDocEvent, IRSGBallotDocEvent, NewRevisionDocEvent, StateDocEvent -from ietf.doc.models import TelechatDocEvent +from ietf.doc.models import TelechatDocEvent, DocumentActionHolder from ietf.name.models import DocReminderTypeName, DocRelationshipName from ietf.group.models import Role, Group from ietf.ietfauth.utils import has_role @@ -406,11 +406,15 @@ def get_document_content(key, filename, split=True, markup=True): def tags_suffix(tags): return ("::" + "::".join(t.name for t in tags)) if tags else "" -def add_state_change_event(doc, by, prev_state, new_state, prev_tags=[], new_tags=[], timestamp=None): +def add_state_change_event(doc, by, prev_state, new_state, prev_tags=None, new_tags=None, timestamp=None): """Add doc event to explain that state change just happened.""" if prev_state and new_state: assert prev_state.type_id == new_state.type_id + # convert default args to empty lists + prev_tags = prev_tags or [] + new_tags = new_tags or [] + if prev_state == new_state and set(prev_tags) == set(new_tags): return None @@ -426,6 +430,88 @@ def add_state_change_event(doc, by, prev_state, new_state, prev_tags=[], new_tag e.save() return e + +def add_action_holder_change_event(doc, by, prev_set, reason=None): + set_changed = False + if doc.documentactionholder_set.exclude(person__in=prev_set).exists(): + set_changed = True # doc has an action holder not in the old set + # If set_changed is still False, then all of the current action holders were in + # prev_set. Either the sets are the same or the prev_set contains at least one + # Person not in the current set, so just check length. + if doc.documentactionholder_set.count() != len(prev_set): + set_changed = True + + if not set_changed: + return None + + if doc.action_holders.exists(): + ah_names = [person.plain_name() for person in doc.action_holders.all()] + description = 'Changed action holders to %s' % ', '.join(ah_names) + else: + description = 'Removed all action holders' + if reason: + description += ' (%s)' % reason + + return DocEvent.objects.create( + type='changed_action_holders', + doc=doc, + by=by, + rev=doc.rev, + desc=description, + ) + + +def update_action_holders(doc, prev_state=None, new_state=None, prev_tags=None, new_tags=None): + """Update the action holders for doc based on state transition + + Returns an event describing the change which should be passed to doc.save_with_history() + + Only cares about draft-iesg state changes. Places where other state types are updated + may not call this method. If you add rules for updating action holders on other state + types, be sure this is called in the places that change that state. + """ + # Should not call this with different state types + if prev_state and new_state: + assert prev_state.type_id == new_state.type_id + + # Convert tags to sets of slugs + prev_tag_slugs = {t.slug for t in (prev_tags or [])} + new_tag_slugs = {t.slug for t in (new_tags or [])} + + # Do nothing if state / tag have not changed + if (prev_state == new_state) and (prev_tag_slugs == new_tag_slugs): + return None + + # Remember original list of action holders to later check if it changed + prev_set = list(doc.action_holders.all()) + # Only draft-iesg states are of interest (for now) + if (prev_state != new_state) and (getattr(new_state, 'type_id') == 'draft-iesg'): + # Clear the action_holders list on a state change. This will reset the age of any that get added back. + doc.action_holders.clear() + if doc.ad and new_state.slug not in DocumentActionHolder.CLEAR_ACTION_HOLDERS_STATES: + # Default to responsible AD for states other than these + doc.action_holders.add(doc.ad) + + if prev_tag_slugs != new_tag_slugs: + # If we have added or removed the need-rev tag, add or remove authors as action holders + if ('need-rev' in prev_tag_slugs) and ('need-rev' not in new_tag_slugs): + # Removed the 'need-rev' tag - drop authors from the action holders list + DocumentActionHolder.objects.filter(document=doc, person__in=doc.authors()).delete() + elif ('need-rev' not in prev_tag_slugs) and ('need-rev' in new_tag_slugs): + # Added the 'need-rev' tag - add authors to the action holders list + for auth in doc.authors(): + if not doc.action_holders.filter(pk=auth.pk).exists(): + doc.action_holders.add(auth) + + # Now create an event if we changed the set + return add_action_holder_change_event( + doc, + Person.objects.get(name='(System)'), + prev_set, + reason='IESG state changed', + ) + + def update_reminder(doc, reminder_type_slug, event, due_date): reminder_type = DocReminderTypeName.objects.get(slug=reminder_type_slug) diff --git a/ietf/doc/views_ballot.py b/ietf/doc/views_ballot.py index 71f03dce6..be9223b78 100644 --- a/ietf/doc/views_ballot.py +++ b/ietf/doc/views_ballot.py @@ -22,7 +22,7 @@ from ietf.doc.models import ( Document, State, DocEvent, BallotDocEvent, IRSGBallotDocEvent, BallotPositionDocEvent, LastCallDocEvent, WriteupDocEvent, IESG_SUBSTATE_TAGS, RelatedDocument, BallotType ) from ietf.doc.utils import ( add_state_change_event, close_ballot, close_open_ballots, - create_ballot_if_not_open, update_telechat ) + create_ballot_if_not_open, update_telechat, update_action_holders ) from ietf.doc.mails import ( email_ballot_deferred, email_ballot_undeferred, extra_automation_headers, generate_last_call_announcement, generate_issue_ballot_mail, generate_ballot_writeup, generate_ballot_rfceditornote, @@ -79,9 +79,12 @@ def do_undefer_ballot(request, doc): doc.tags.remove(*prev_tags) events = [] - state_change_event = add_state_change_event(doc, by, prev_state, new_state, prev_tags=prev_tags, new_tags=new_tags) - if state_change_event: - events.append(state_change_event) + e = add_state_change_event(doc, by, prev_state, new_state, prev_tags=prev_tags, new_tags=new_tags) + if e: + events.append(e) + e = update_action_holders(doc, prev_state, new_state, prev_tags=prev_tags, new_tags=new_tags) + if e: + events.append(e) e = update_telechat(request, doc, by, telechat_date) if e: @@ -453,9 +456,12 @@ def defer_ballot(request, name): events = [] - state_change_event = add_state_change_event(doc, login, prev_state, new_state, prev_tags=prev_tags, new_tags=new_tags) - if state_change_event: - events.append(state_change_event) + e = add_state_change_event(doc, login, prev_state, new_state, prev_tags=prev_tags, new_tags=new_tags) + if e: + events.append(e) + e = update_action_holders(doc, prev_state, new_state, prev_tags=prev_tags, new_tags=new_tags) + if e: + events.append(e) e = update_telechat(request, doc, login, telechat_date) if e: @@ -547,10 +553,16 @@ def lastcalltext(request, name): doc.set_state(new_state) doc.tags.remove(*prev_tags) + events = [] e = add_state_change_event(doc, login, prev_state, new_state, prev_tags=prev_tags, new_tags=[]) - if e: - doc.save_with_history([e]) + events.append(e) + e = update_action_holders(doc, prev_state, new_state, prev_tags=prev_tags, new_tags=[]) + if e: + events.append(e) + + if events: + doc.save_with_history(events) request_last_call(request, doc) @@ -628,9 +640,15 @@ def ballot_writeupnotes(request, name): doc.set_state(new_state) doc.tags.remove(*prev_tags) - sce = add_state_change_event(doc, login, prev_state, new_state, prev_tags=prev_tags, new_tags=[]) - if sce: - doc.save_with_history([sce]) + events = [] + e = add_state_change_event(doc, login, prev_state, new_state, prev_tags=prev_tags, new_tags=[]) + if e: + events.append(e) + e = update_action_holders(doc, prev_state, new_state, prev_tags=prev_tags, new_tags=[]) + if e: + events.append(e) + if events: + doc.save_with_history(events) if not ballot_already_approved: e = create_ballot_if_not_open(request, doc, login, "approve") # pyflakes:ignore @@ -893,9 +911,11 @@ def approve_ballot(request, name): e.desc = "IESG has approved the document" e.save() events.append(e) - - e = add_state_change_event(doc, login, prev_state, new_state, prev_tags=prev_tags, new_tags=[]) + e = add_state_change_event(doc, login, prev_state, new_state, prev_tags=prev_tags, new_tags=[]) + if e: + events.append(e) + e = update_action_holders(doc, prev_state, new_state, prev_tags=prev_tags, new_tags=[]) if e: events.append(e) @@ -1038,6 +1058,9 @@ def make_last_call(request, name): doc.tags.remove(*prev_tags) e = add_state_change_event(doc, login, prev_state, new_state, prev_tags=prev_tags, new_tags=new_tags) + if e: + events.append(e) + e = update_action_holders(doc, prev_state, new_state, prev_tags=prev_tags, new_tags=new_tags) if e: events.append(e) expiration_date = form.cleaned_data['last_call_expiration_date'] @@ -1129,9 +1152,12 @@ def issue_irsg_ballot(request, name): doc.tags.remove(*prev_tags) events = [] - state_change_event = add_state_change_event(doc, by, prev_state, new_state, prev_tags=prev_tags, new_tags=new_tags) - if state_change_event: - events.append(state_change_event) + e = add_state_change_event(doc, by, prev_state, new_state, prev_tags=prev_tags, new_tags=new_tags) + if e: + events.append(e) + e = update_action_holders(doc, prev_state, new_state, prev_tags=prev_tags, new_tags=new_tags) + if e: + events.append(e) if events: doc.save_with_history(events) diff --git a/ietf/doc/views_doc.py b/ietf/doc/views_doc.py index 3a32a5f19..8caaaeb6b 100644 --- a/ietf/doc/views_doc.py +++ b/ietf/doc/views_doc.py @@ -54,21 +54,21 @@ import debug # pyflakes:ignore from ietf.doc.models import ( Document, DocAlias, DocHistory, DocEvent, BallotDocEvent, BallotType, ConsensusDocEvent, NewRevisionDocEvent, TelechatDocEvent, WriteupDocEvent, IanaExpertDocEvent, - IESG_BALLOT_ACTIVE_STATES, STATUSCHANGE_RELATIONS ) + IESG_BALLOT_ACTIVE_STATES, STATUSCHANGE_RELATIONS, DocumentActionHolder ) from ietf.doc.utils import (add_links_in_new_revision_events, augment_events_with_revision, can_adopt_draft, can_unadopt_draft, get_chartering_type, get_tags_for_stream_id, needed_ballot_positions, nice_consensus, prettify_std_name, update_telechat, has_same_ballot, get_initial_notify, make_notify_changed_event, make_rev_history, default_consensus, add_events_message_info, get_unicode_document_content, build_doc_meta_block, - augment_docs_and_user_with_user_info, irsg_needed_ballot_positions ) + augment_docs_and_user_with_user_info, irsg_needed_ballot_positions, add_action_holder_change_event ) from ietf.group.models import Role, Group from ietf.group.utils import can_manage_group_type, can_manage_materials, group_features_role_filter from ietf.ietfauth.utils import ( has_role, is_authorized_in_doc_stream, user_is_person, role_required, is_individual_draft_author) from ietf.name.models import StreamName, BallotPositionName from ietf.utils.history import find_history_active_at -from ietf.doc.forms import TelechatForm, NotifyForm -from ietf.doc.mails import email_comment +from ietf.doc.forms import TelechatForm, NotifyForm, ActionHoldersForm +from ietf.doc.mails import email_comment, email_remind_action_holders from ietf.mailtrigger.utils import gather_relevant_expansions from ietf.meeting.models import Session from ietf.meeting.utils import group_sessions, get_upcoming_manageable_sessions, sort_sessions, add_event_info_to_session_qs @@ -1275,6 +1275,14 @@ def telechat_date(request, name): warnings=warnings, login=login)) + +def doc_titletext(doc): + if doc.type.slug=='conflrev': + conflictdoc = doc.relateddocument_set.get(relationship__slug='conflrev').target.document + return 'the conflict review of %s' % conflictdoc.canonical_name() + return doc.canonical_name() + + def edit_notify(request, name): """Change the set of email addresses document change notificaitions go to.""" @@ -1311,18 +1319,150 @@ def edit_notify(request, name): init = { "notify" : doc.notify } form = NotifyForm(initial=init) - if doc.type.slug=='conflrev': - conflictdoc = doc.relateddocument_set.get(relationship__slug='conflrev').target.document - titletext = 'the conflict review of %s' % conflictdoc.canonical_name() - else: - titletext = '%s' % doc.canonical_name() return render(request, 'doc/edit_notify.html', {'form': form, 'doc': doc, - 'titletext': titletext, + 'titletext': doc_titletext(doc), }, ) + +@role_required('Area Director', 'Secretariat') +def edit_action_holders(request, name): + """Change the set of action holders for a doc""" + doc = get_object_or_404(Document, name=name) + + if request.method == 'POST': + form = ActionHoldersForm(request.POST) + if form.is_valid() and 'action_holders' in request.POST: + new_action_holders = form.cleaned_data['action_holders'] # Person queryset + prev_action_holders = list(doc.action_holders.all()) + + # Now update the action holders. We can't use the simple approach of clearing + # the set and then adding back the entire new_action_holders. If we did that, + # the timestamps that track when each person became an action holder would + # reset every time the list was modified. So we need to be careful only + # to delete the ones that are really being removed. + # + # Also need to take care not to delete the people! doc.action_holders.all() + # (and other querysets) give the Person objects. We only want to add/delete + # the DocumentActionHolder 'through' model objects. That means working directly + # with the model or using doc.action_holders.add() and .remove(), which take + # Person objects as arguments. + existing = DocumentActionHolder.objects.filter(document=doc) # through model + to_remove = existing.exclude(person__in=new_action_holders) # through model + to_remove.delete() # deletes the DocumentActionHolder objects, leaves the Person objects + + # Get all the Persons who do not have a DocumentActionHolder for this document + added_people = new_action_holders.exclude(documentactionholder__document=doc) + doc.action_holders.add(*added_people) + + add_action_holder_change_event(doc, request.user.person, prev_action_holders, + form.cleaned_data['reason']) + + return redirect('ietf.doc.views_doc.document_main', name=doc.name) + + # When not a POST + # Data for quick add/remove of various related Persons + doc_role_labels = [] # labels for document-related roles + group_role_labels = [] # labels for group-related roles + role_ids = dict() # maps role slug to list of Person IDs (assumed numeric in the JavaScript) + extra_prefetch = [] # list of Person objects to prefetch for select2 field + + if len(doc.authors()) > 0: + doc_role_labels.append(dict(slug='authors', label='Authors')) + authors = doc.authors() + role_ids['authors'] = [p.pk for p in authors] + extra_prefetch += authors + + if doc.ad: + doc_role_labels.append(dict(slug='ad', label='Responsible AD')) + role_ids['ad'] = [doc.ad.pk] + extra_prefetch.append(doc.ad) + + if doc.shepherd: + # doc.shepherd is an Email, which is allowed not to have a Person. + # The Emails used for shepherds should always have one, though. If not, log the + # event and move on without the shepherd. This just means there will not be + # add/remove shepherd buttons. + log.assertion('doc.shepherd.person', + note="A document's shepherd should always have a Person'. Failed for %s"%doc.name) + if doc.shepherd.person: + doc_role_labels.append(dict(slug='shep', label='Shepherd')) + role_ids['shep'] = [doc.shepherd.person.pk] + extra_prefetch.append(doc.shepherd.person) + + if doc.group: + # UI buttons to add / remove will appear in same order as this list + group_roles = doc.group.role_set.filter( + name__in=DocumentActionHolder.GROUP_ROLES_OF_INTEREST, + ).select_related('name', 'person') # name is a RoleName + + # Gather all the roles for this group + for role in group_roles: + key = 'group_%s' % role.name.slug + existing_list = role_ids.get(key) + if existing_list: + existing_list.append(role.person.pk) + else: + role_ids[key] = [role.person.pk] + group_role_labels.append(dict( + sort_order=DocumentActionHolder.GROUP_ROLES_OF_INTEREST.index(role.name.slug), + slug=key, + label='Group ' + role.name.name, # friendly role name + )) + extra_prefetch.append(role.person) + + # Ensure group role button order is stable + group_role_labels.sort(key=lambda r: r['sort_order']) + + form = ActionHoldersForm(initial={'action_holders': doc.action_holders.all()}) + form.fields['action_holders'].extra_prefetch = extra_prefetch + form.fields['action_holders'].widget.attrs["data-role-ids"] = json.dumps(role_ids) + + return render( + request, + 'doc/edit_action_holders.html', + { + 'form': form, + 'doc': doc, + 'titletext': doc_titletext(doc), + 'role_labels': doc_role_labels + group_role_labels, + } + ) + + +class ReminderEmailForm(forms.Form): + note = forms.CharField( + widget=forms.Textarea, + label='Note to action holders', + help_text='Optional message to the action holders', + required=False, + strip=True, + ) + +@role_required('Area Director', 'Secretariat') +def remind_action_holders(request, name): + doc = get_object_or_404(Document, name=name) + + if request.method == 'POST': + form = ReminderEmailForm(request.POST) + if form.is_valid(): + email_remind_action_holders(request, doc, form.cleaned_data['note']) + return redirect('ietf.doc.views_doc.document_main', name=doc.canonical_name()) + + form = ReminderEmailForm() + return render( + request, + 'doc/remind_action_holders.html', + { + 'form': form, + 'doc': doc, + 'titletext': doc_titletext(doc), + } + ) + + def email_aliases(request,name=''): doc = get_object_or_404(Document, name=name) if name else None if not name: diff --git a/ietf/doc/views_draft.py b/ietf/doc/views_draft.py index 0584142c8..89cc51a2c 100644 --- a/ietf/doc/views_draft.py +++ b/ietf/doc/views_draft.py @@ -33,7 +33,7 @@ from ietf.doc.mails import ( email_pulled_from_rfc_queue, email_resurrect_reques email_iesg_processing_document, email_ad_approved_doc, email_iana_expert_review_state_changed ) from ietf.doc.utils import ( add_state_change_event, can_adopt_draft, can_unadopt_draft, - get_tags_for_stream_id, nice_consensus, + get_tags_for_stream_id, nice_consensus, update_action_holders, update_reminder, update_telechat, make_notify_changed_event, get_initial_notify, set_replaces_for_document, default_consensus, tags_suffix, ) from ietf.doc.lastcall import request_last_call @@ -115,6 +115,7 @@ def change_state(request, name): events = [] + e = add_state_change_event(doc, login, prev_state, new_state, prev_tags=prev_tags, new_tags=new_tags) @@ -124,6 +125,10 @@ def change_state(request, name): events.append(e) + e = update_action_holders(doc, prev_state, new_state, prev_tags=prev_tags, new_tags=new_tags) + if e: + events.append(e) + if comment: c = DocEvent(type="added_comment") c.doc = doc @@ -596,6 +601,9 @@ def to_iesg(request,name): new_state = target_state[target_map[state_type]] if not prev_state==new_state: doc.set_state(new_state) + e = update_action_holders(doc, prev_state, new_state) + if e: + events.append(e) events.append(add_state_change_event(doc=doc,by=by,prev_state=prev_state,new_state=new_state)) if not doc.ad == ad : @@ -764,6 +772,16 @@ def edit_info(request, name): doc.save_with_history(events) + if new_document: + # If we created a new doc, update the action holders as though it + # started in idexists and moved to its create_in_state. Do this + # after the doc has been updated so, e.g., doc.ad is set. + update_action_holders( + doc, + State.objects.get(type='draft-iesg', slug='idexists'), + r['create_in_state'] + ) + if changes: email_iesg_processing_document(request, doc, changes) diff --git a/ietf/group/tests_info.py b/ietf/group/tests_info.py index e2d162976..8de4c7eb6 100644 --- a/ietf/group/tests_info.py +++ b/ietf/group/tests_info.py @@ -205,19 +205,32 @@ class GroupPagesTests(TestCase): group = GroupFactory() setup_default_community_list_for_group(group) draft = WgDraftFactory(group=group) + draft.action_holders.set([PersonFactory()]) draft2 = WgDraftFactory(group=group) + draft3 = WgDraftFactory(group=group) + draft3.set_state(State.objects.get(type='draft-iesg', slug='pub-req')) + draft3.action_holders.set(PersonFactory.create_batch(2)) + old_dah = draft3.documentactionholder_set.first() + old_dah.time_added -= datetime.timedelta(days=173) # make an "old" action holder + old_dah.save() clist = CommunityList.objects.get(group=group) related_docs_rule = clist.searchrule_set.get(rule_type='name_contains') reset_name_contains_index_for_rule(related_docs_rule) for url in group_urlreverse_list(group, 'ietf.group.views.group_documents'): - r = self.client.get(url) + with self.settings(DOC_ACTION_HOLDER_MAX_AGE_DAYS=20): + r = self.client.get(url) self.assertEqual(r.status_code, 200) self.assertContains(r, draft.name) self.assertContains(r, group.name) self.assertContains(r, group.acronym) + self.assertNotContains(r, draft.action_holders.first().plain_name()) self.assertContains(r, draft2.name) + self.assertContains(r, draft3.name) + for ah in draft3.action_holders.all(): + self.assertContains(r, ah.plain_name()) + self.assertContains(r, 'for 173 days', count=1) # the old_dah should be tagged # Make sure that a logged in user is presented with an opportunity to add results to their community list self.client.login(username="secretary", password="secretary+password") diff --git a/ietf/mailtrigger/migrations/0021_email_remind_action_holders.py b/ietf/mailtrigger/migrations/0021_email_remind_action_holders.py new file mode 100644 index 000000000..704b452a5 --- /dev/null +++ b/ietf/mailtrigger/migrations/0021_email_remind_action_holders.py @@ -0,0 +1,38 @@ +# Copyright The IETF Trust 2020 All Rights Reserved + +from django.db import migrations + + +def forward(apps,schema_editor): + MailTrigger = apps.get_model('mailtrigger', 'MailTrigger') + Recipient = apps.get_model('mailtrigger', 'Recipient') + + (doc_action_holders, _) = Recipient.objects.get_or_create( + slug='doc_action_holders', + desc='Action holders for a document', + template='{% for action_holder in doc.action_holders.all %}{% if doc.shepherd and action_holder == doc.shepherd.person %}{{ doc.shepherd }}{% else %}{{ action_holder.email }}{% endif %}{% if not forloop.last %},{%endif %}{% endfor %}', + ) + (doc_remind_action_holders, _) = MailTrigger.objects.get_or_create( + slug='doc_remind_action_holders', + desc='Recipients when sending a reminder email to action holders for a document', + ) + doc_remind_action_holders.to.set([doc_action_holders]) + + +def reverse(apps,schema_editor): + MailTrigger = apps.get_model('mailtrigger', 'MailTrigger') + Recipient = apps.get_model('mailtrigger', 'Recipient') + + MailTrigger.objects.filter(slug='doc_remind_action_holders').delete() + Recipient.objects.filter(slug='doc_action_holders').delete() + + +class Migration(migrations.Migration): + + dependencies = [ + ('mailtrigger', '0020_add_ad_approval_request_mailtriggers'), + ] + + operations = [ + migrations.RunPython(forward, reverse) + ] diff --git a/ietf/name/fixtures/names.json b/ietf/name/fixtures/names.json index 592711472..6195215f2 100644 --- a/ietf/name/fixtures/names.json +++ b/ietf/name/fixtures/names.json @@ -3573,6 +3573,17 @@ "model": "mailtrigger.mailtrigger", "pk": "doc_pulled_from_rfc_queue" }, + { + "fields": { + "cc": [], + "desc": "Recipients when sending a reminder email to action holders for a document", + "to": [ + "doc_action_holders" + ] + }, + "model": "mailtrigger.mailtrigger", + "pk": "doc_remind_action_holders" + }, { "fields": { "cc": [], @@ -4999,6 +5010,14 @@ "model": "mailtrigger.recipient", "pk": "conflict_review_stream_manager" }, + { + "fields": { + "desc": "Action holders for a document", + "template": "{% for action_holder in doc.action_holders.all %}{% if doc.shepherd and action_holder == doc.shepherd.person %}{{ doc.shepherd }}{% else %}{{ action_holder.email }}{% endif %}{% if not forloop.last %},{%endif %}{% endfor %}" + }, + "model": "mailtrigger.recipient", + "pk": "doc_action_holders" + }, { "fields": { "desc": "The document's responsible Area Director", diff --git a/ietf/person/fields.py b/ietf/person/fields.py index a90d0a62c..969450bd7 100644 --- a/ietf/person/fields.py +++ b/ietf/person/fields.py @@ -16,7 +16,7 @@ import debug # pyflakes:ignore from ietf.person.models import Email, Person -def select2_id_name_json(objs): +def select2_id_name(objs): def format_email(e): return escape("%s <%s>" % (e.person.name, e.address)) def format_person(p): @@ -33,10 +33,13 @@ def select2_id_name_json(objs): for p in objs: p.name_count = c[p.name] - formatter = format_email if objs and isinstance(objs[0], Email) else format_person + return [{ "id": o.pk, "text": formatter(o) } for o in objs if o] + + +def select2_id_name_json(objs): + return json.dumps(select2_id_name(objs)) - return json.dumps([{ "id": o.pk, "text": formatter(o) } for o in objs if o]) class SearchablePersonsField(forms.CharField): """Server-based multi-select field for choosing @@ -48,12 +51,19 @@ class SearchablePersonsField(forms.CharField): The field uses a comma-separated list of primary keys in a CharField element as its API with some extra attributes used by - the Javascript part.""" + the Javascript part. + + If the field will be programmatically updated, any model instances + that may be added to the initial set should be included in the extra_prefetch + list. These can then be added by updating val() and triggering the 'change' + event on the select2 field in JavaScript. + """ def __init__(self, max_entries=None, # max number of selected objs only_users=False, # only select persons who also have a user all_emails=False, # select only active email addresses + extra_prefetch=None, # extra data records to include in prefetch model=Person, # or Email hint_text="Type in name to search for person.", *args, **kwargs): @@ -70,6 +80,9 @@ class SearchablePersonsField(forms.CharField): self.widget.attrs["data-placeholder"] = hint_text if self.max_entries != None: self.widget.attrs["data-max-entries"] = self.max_entries + + self.extra_prefetch = extra_prefetch or [] + assert all([isinstance(obj, self.model) for obj in self.extra_prefetch]) def parse_select2_value(self, value): return [x.strip() for x in value.split(",") if x.strip()] @@ -96,7 +109,12 @@ class SearchablePersonsField(forms.CharField): if isinstance(value, self.model): value = [value] - self.widget.attrs["data-pre"] = select2_id_name_json(value) + # data-pre is a map from ID to full data. It includes records needed by the + # initial value of the field plus any added via extra_prefetch. + prefetch_set = set(value).union(set(self.extra_prefetch)) # eliminate duplicates + self.widget.attrs["data-pre"] = json.dumps({ + d['id']: d for d in select2_id_name(list(prefetch_set)) + }) # doing this in the constructor is difficult because the URL # patterns may not have been fully constructed there yet diff --git a/ietf/secr/telechat/tests.py b/ietf/secr/telechat/tests.py index c707675fc..f515187b6 100644 --- a/ietf/secr/telechat/tests.py +++ b/ietf/secr/telechat/tests.py @@ -10,11 +10,12 @@ import debug # pyflakes:ignore from django.urls import reverse from ietf.doc.factories import WgDraftFactory, IndividualRfcFactory, CharterFactory -from ietf.doc.models import BallotDocEvent, BallotType, BallotPositionDocEvent +from ietf.doc.models import BallotDocEvent, BallotType, BallotPositionDocEvent, State, Document from ietf.doc.utils import update_telechat, create_ballot_if_not_open from ietf.utils.test_utils import TestCase from ietf.iesg.models import TelechatDate from ietf.person.models import Person +from ietf.person.factories import PersonFactory from ietf.secr.telechat.views import get_next_telechat_date SECR_USER='secretary' @@ -180,3 +181,62 @@ class SecrTelechatTestCase(TestCase): ) self.assertEqual(response.status_code,302) self.assertEqual(charter.get_state('charter').slug,'notrev') + + def test_doc_detail_post_update_state_action_holder_automation(self): + """Updating IESG state of a draft should update action holders""" + by = Person.objects.get(name='(System)') + draft = WgDraftFactory( + states=[('draft-iesg', 'iesg-eva')], + ad=Person.objects.get(user__username='ad'), + authors=PersonFactory.create_batch(3), + ) + last_week = datetime.date.today()-datetime.timedelta(days=7) + BallotDocEvent.objects.create(type='created_ballot',by=by,doc=draft, rev=draft.rev, + ballot_type=BallotType.objects.get(doc_type=draft.type,slug='approve'), + time=last_week) + d = get_next_telechat_date() + date = d.strftime('%Y-%m-%d') + update_telechat(None, draft, by, d) + url = reverse('ietf.secr.telechat.views.doc_detail', kwargs={'date':date, 'name':draft.name}) + self.client.login(username="secretary", password="secretary+password") + + # Check that there are no action holder DocEvents yet + self.assertEqual(draft.docevent_set.filter(type='changed_action_holders').count(), 0) + + # setting to defer should add AD, adding need-rev should add authors + response = self.client.post(url,{ + 'submit': 'update_state', + 'state': State.objects.get(type_id='draft-iesg', slug='defer').pk, + 'substate': 'need-rev', + }) + self.assertEqual(response.status_code,302) + draft = Document.objects.get(name=draft.name) + self.assertEqual(draft.get_state('draft-iesg').slug,'defer') + self.assertCountEqual(draft.action_holders.all(), [draft.ad] + draft.authors()) + self.assertEqual(draft.docevent_set.filter(type='changed_action_holders').count(), 1) + + # Removing need-rev should remove authors + response = self.client.post(url,{ + 'submit': 'update_state', + 'state': State.objects.get(type_id='draft-iesg', slug='iesg-eva').pk, + 'substate': '', + }) + self.assertEqual(response.status_code,302) + draft = Document.objects.get(name=draft.name) + self.assertEqual(draft.get_state('draft-iesg').slug,'iesg-eva') + self.assertCountEqual(draft.action_holders.all(), [draft.ad]) + self.assertEqual(draft.docevent_set.filter(type='changed_action_holders').count(), 2) + + # Setting to approved should remove all action holders + # noinspection DjangoOrm + draft.action_holders.add(*(draft.authors())) # add() with through model ok in Django 2.2+ + response = self.client.post(url,{ + 'submit': 'update_state', + 'state': State.objects.get(type_id='draft-iesg', slug='approved').pk, + 'substate': '', + }) + self.assertEqual(response.status_code,302) + draft = Document.objects.get(name=draft.name) + self.assertEqual(draft.get_state('draft-iesg').slug,'approved') + self.assertCountEqual(draft.action_holders.all(), []) + self.assertEqual(draft.docevent_set.filter(type='changed_action_holders').count(), 3) diff --git a/ietf/secr/telechat/views.py b/ietf/secr/telechat/views.py index 8359d5bb3..3c0e9a198 100644 --- a/ietf/secr/telechat/views.py +++ b/ietf/secr/telechat/views.py @@ -12,7 +12,7 @@ from django.utils.functional import curry import debug # pyflakes:ignore from ietf.doc.models import DocEvent, Document, BallotDocEvent, BallotPositionDocEvent, BallotType, WriteupDocEvent -from ietf.doc.utils import add_state_change_event +from ietf.doc.utils import add_state_change_event, update_action_holders from ietf.person.models import Person from ietf.doc.lastcall import request_last_call from ietf.doc.mails import email_state_changed @@ -284,12 +284,18 @@ def doc_detail(request, date, name): doc.tags.remove(*prev_tags) doc.tags.add(*new_tags) - e = add_state_change_event(doc, login, prev_state, new_state, + events = [] + sce = add_state_change_event(doc, login, prev_state, new_state, prev_tags=prev_tags, new_tags=new_tags) + if sce: + events.append(sce) + e = update_action_holders(doc, prev_state, new_state, prev_tags=prev_tags, new_tags=new_tags) if e: - doc.save_with_history([e]) + events.append(e) + if events: + doc.save_with_history(events) - email_state_changed(request, doc, e.desc, 'doc_state_edited') + email_state_changed(request, doc, sce.desc, 'doc_state_edited') if new_state.slug == "lc-req": request_last_call(request, doc) diff --git a/ietf/settings.py b/ietf/settings.py index e5ca7c9a6..50a008e65 100644 --- a/ietf/settings.py +++ b/ietf/settings.py @@ -705,6 +705,9 @@ DOC_HREFS = { # e.g. a charter or a review. Must be a tuple, not a list. DOC_TEXT_FILE_VALID_UPLOAD_MIME_TYPES = ('text/plain', 'text/markdown', 'text/x-rst', 'text/x-markdown', ) +# Age limit before action holders are flagged in the document display +DOC_ACTION_HOLDER_AGE_LIMIT_DAYS = 20 + # Override this in settings_local.py if needed CACHE_MIDDLEWARE_SECONDS = 300 CACHE_MIDDLEWARE_KEY_PREFIX = '' diff --git a/ietf/static/ietf/js/select2-field.js b/ietf/static/ietf/js/select2-field.js index 993b57f89..c4efe6f7a 100644 --- a/ietf/static/ietf/js/select2-field.js +++ b/ietf/static/ietf/js/select2-field.js @@ -6,7 +6,7 @@ function setupSelect2Field(e) { return; var maxEntries = e.data("max-entries"); - var multiple = maxEntries != 1; + var multiple = maxEntries !== 1; var prefetched = e.data("pre"); e.select2({ multiple: multiple, @@ -27,7 +27,7 @@ function setupSelect2Field(e) { results: function (results) { return { results: results, - more: results.length == 10 + more: results.length === 10 }; } }, @@ -35,11 +35,27 @@ function setupSelect2Field(e) { return m; }, initSelection: function (element, cb) { - if (!multiple && prefetched.length > 0) - cb(prefetched[0]); - else - cb(prefetched); + element = $(element); // jquerify + // The original data set will contain any values looked up via ajax + var data = element.select2('data'); + var data_map = {}; + + // map id to its data representation + for (var ii = 0; ii < data.length; ii++) { + var this_item = data[ii]; + data_map[this_item.id] = this_item; + } + + // convert values to data objects, letting element data supersede prefetch + var ids = element.val().split(','); + if (!multiple && ids.length > 0) { + cb(data_map[ids[0]] || prefetched[ids[0]]); + } else { + cb(ids.map(function(id) { + return data_map[id] || prefetched[id]; + })); + } }, dropdownCssClass: "bigdrop" }); diff --git a/ietf/sync/rfceditor.py b/ietf/sync/rfceditor.py index 3d8912ea6..f41554c03 100644 --- a/ietf/sync/rfceditor.py +++ b/ietf/sync/rfceditor.py @@ -18,7 +18,7 @@ import debug # pyflakes:ignore from ietf.doc.models import ( Document, DocAlias, State, StateType, DocEvent, DocRelationshipName, DocTagName, DocTypeName, RelatedDocument ) from ietf.doc.expire import move_draft_files_to_archive -from ietf.doc.utils import add_state_change_event, prettify_std_name +from ietf.doc.utils import add_state_change_event, prettify_std_name, update_action_holders from ietf.group.models import Group from ietf.name.models import StdLevelName, StreamName from ietf.person.models import Person @@ -186,6 +186,9 @@ def update_drafts_from_queue(drafts): d.set_state(next_iesg_state) e = add_state_change_event(d, system, prev_iesg_state, next_iesg_state) + if e: + events.append(e) + e = update_action_holders(d, prev_iesg_state, next_iesg_state) if e: events.append(e) changed.add(name) @@ -457,12 +460,16 @@ def update_docs_from_rfc_index(index_data, errata_data, skip_older_than_date=Non rfc_published = True for t in ("draft-iesg", "draft-stream-iab", "draft-stream-irtf", "draft-stream-ise"): - slug = doc.get_state_slug(t) - if slug and slug not in ("pub", "idexists"): - new_state = State.objects.select_related("type").get(used=True, type=t, slug="pub") - doc.set_state(new_state) - changes.append("changed %s to %s" % (new_state.type.label, new_state)) - if t == 'draft-iesg' and not slug: + prev_state = doc.get_state(t) + if prev_state is not None: + if prev_state.slug not in ("pub", "idexists"): + new_state = State.objects.select_related("type").get(used=True, type=t, slug="pub") + doc.set_state(new_state) + changes.append("changed %s to %s" % (new_state.type.label, new_state)) + e = update_action_holders(doc, prev_state, new_state) + if e: + events.append(e) + elif t == 'draft-iesg': doc.set_state(State.objects.get(type_id='draft-iesg', slug='idexists')) def parse_relation_list(l): diff --git a/ietf/sync/tests.py b/ietf/sync/tests.py index 5f5ed4224..f0badf1d5 100644 --- a/ietf/sync/tests.py +++ b/ietf/sync/tests.py @@ -239,9 +239,14 @@ class RFCSyncTests(TestCase): def test_rfc_index(self): area = GroupFactory(type_id='area') - doc = WgDraftFactory(group__parent=area,states=[('draft-iesg','rfcqueue'),('draft-stream-ise','rfc-edit')]) + doc = WgDraftFactory( + group__parent=area, + states=[('draft-iesg','rfcqueue'),('draft-stream-ise','rfc-edit')], + ad=Person.objects.get(user__username='ad'), + ) # it's a bit strange to have draft-stream-ise set when draft-iesg is set # too, but for testing purposes ... + doc.action_holders.add(doc.ad) # not normally set, but add to be sure it's cleared updated_doc = Document.objects.create(name="draft-ietf-something") DocAlias.objects.create(name=updated_doc.name).docs.add(updated_doc) @@ -358,9 +363,11 @@ class RFCSyncTests(TestCase): doc = Document.objects.get(name=doc.name) - self.assertEqual(doc.docevent_set.all()[0].type, "sync_from_rfc_editor") - self.assertEqual(doc.docevent_set.all()[1].type, "published_rfc") - self.assertEqual(doc.docevent_set.all()[1].time.date(), today) + events = doc.docevent_set.all() + self.assertEqual(events[0].type, "sync_from_rfc_editor") + self.assertEqual(events[1].type, "changed_action_holders") + self.assertEqual(events[2].type, "published_rfc") + self.assertEqual(events[2].time.date(), today) self.assertTrue("errata" in doc.tags.all().values_list("slug", flat=True)) self.assertTrue(DocAlias.objects.filter(name="rfc1234", docs=doc)) self.assertTrue(DocAlias.objects.filter(name="bcp1", docs=doc)) @@ -371,6 +378,7 @@ class RFCSyncTests(TestCase): self.assertEqual(doc.abstract, "This is some interesting text.") self.assertEqual(doc.get_state_slug(), "rfc") self.assertEqual(doc.get_state_slug("draft-iesg"), "pub") + self.assertCountEqual(doc.action_holders.all(), []) self.assertEqual(doc.get_state_slug("draft-stream-ise"), "pub") self.assertEqual(doc.std_level_id, "ps") self.assertEqual(doc.pages, 42) @@ -413,7 +421,9 @@ class RFCSyncTests(TestCase): return t def test_rfc_queue(self): - draft = WgDraftFactory(states=[('draft-iesg','ann')]) + draft = WgDraftFactory(states=[('draft-iesg','ann')], ad=Person.objects.get(user__username='ad')) + draft.action_holders.add(draft.ad) # add an action holder so we can test that it's removed later + expected_auth48_url = "http://www.rfc-editor.org/auth48/rfc1234" t = self._generate_rfc_queue_xml(draft, state='EDIT*R*A(1G)', @@ -433,10 +443,13 @@ class RFCSyncTests(TestCase): draft = Document.objects.get(pk=draft.pk) self.assertEqual(draft.get_state_slug("draft-rfceditor"), "edit") self.assertEqual(draft.get_state_slug("draft-iesg"), "rfcqueue") + self.assertCountEqual(draft.action_holders.all(), []) self.assertEqual(set(draft.tags.all()), set(DocTagName.objects.filter(slug__in=("iana", "ref")))) - self.assertEqual(draft.docevent_set.all()[0].type, "changed_state") # changed draft-iesg state - self.assertEqual(draft.docevent_set.all()[1].type, "changed_state") # changed draft-rfceditor state - self.assertEqual(draft.docevent_set.all()[2].type, "rfc_editor_received_announcement") + events = draft.docevent_set.all() + self.assertEqual(events[0].type, "changed_state") # changed draft-iesg state + self.assertEqual(events[1].type, "changed_action_holders") + self.assertEqual(events[2].type, "changed_state") # changed draft-rfceditor state + self.assertEqual(events[3].type, "rfc_editor_received_announcement") self.assertEqual(len(outbox), mailbox_before + 1) self.assertTrue("RFC Editor queue" in outbox[-1]["Subject"]) diff --git a/ietf/templates/doc/document_draft.html b/ietf/templates/doc/document_draft.html index e051366a3..5d6600e79 100644 --- a/ietf/templates/doc/document_draft.html +++ b/ietf/templates/doc/document_draft.html @@ -457,6 +457,29 @@ + {% if doc.action_holders_enabled %} + + + Action Holders + + {% if can_edit %} + Edit + {% endif %} + + +
+ {% if doc.action_holders.exists %} + {% for action_holder in doc.documentactionholder_set.all %} +
{% person_link action_holder.person title=action_holder.role_for_doc %} {{ action_holder|action_holder_badge }}
+ {% endfor %} + {% if can_edit %} Send reminder email {% endif %} + {% else %} + (None) + {% endif %} +
+ + {% endif %} + {% if consensus and doc.stream_id == 'ietf' %} diff --git a/ietf/templates/doc/drafts_in_iesg_process.html b/ietf/templates/doc/drafts_in_iesg_process.html index e015f109a..f8e59977b 100644 --- a/ietf/templates/doc/drafts_in_iesg_process.html +++ b/ietf/templates/doc/drafts_in_iesg_process.html @@ -2,7 +2,7 @@ {# Copyright The IETF Trust 2015, All Rights Reserved #} {% load origin %} {% load ietf_filters static %} -{% load textfilters %} +{% load textfilters person_filters %} {% block pagehead %} @@ -43,6 +43,12 @@ {{ doc.name }}
{{ doc.title }} + {% if doc.action_holders_enabled and doc.action_holders.exists %} +
Action holders: + {% for action_holder in doc.documentactionholder_set.all %} + {% person_link action_holder.person title=action_holder.role_for_doc %}{{ action_holder|action_holder_badge }}{% if not forloop.last %},{% endif %} + {% endfor %} + {% endif %} {% if doc.note %}
Note: {{ doc.note|linkify|linebreaksbr }} {% endif %} diff --git a/ietf/templates/doc/edit_action_holders.html b/ietf/templates/doc/edit_action_holders.html new file mode 100644 index 000000000..c7b8d5632 --- /dev/null +++ b/ietf/templates/doc/edit_action_holders.html @@ -0,0 +1,138 @@ +{% extends "base.html" %} +{# Copyright The IETF Trust 2020, All Rights Reserved #} +{% load origin %} +{% load static %} +{% load bootstrap3 %} + +{% block title %} + Edit action holders for {{ titletext }} +{% endblock %} + +{% block pagehead %} + + +{% endblock %} + +{% block content %} + {% origin %} +

Edit action holders
{{titletext}}

+ +
+ {% csrf_token %} + {% bootstrap_form form %} + +
+ + +
+ + {% buttons %} + + Back + {% endbuttons %} +
+ +{% endblock %} + +{% block js %} + + + +{% endblock %} diff --git a/ietf/templates/doc/mail/remind_action_holders_mail.txt b/ietf/templates/doc/mail/remind_action_holders_mail.txt new file mode 100644 index 000000000..6f1ce8657 --- /dev/null +++ b/ietf/templates/doc/mail/remind_action_holders_mail.txt @@ -0,0 +1,14 @@ +{% autoescape off %} +Reminder: your action is needed to allow the publication process for + + {{ doc.display_name }} + +to proceed. This document can be found at + + {{ doc_url }} +{% if note %} +Please note: + +{{ note|wordwrap:78 }} +{% endif %} +{% endautoescape %} \ No newline at end of file diff --git a/ietf/templates/doc/remind_action_holders.html b/ietf/templates/doc/remind_action_holders.html new file mode 100644 index 000000000..93658ecb1 --- /dev/null +++ b/ietf/templates/doc/remind_action_holders.html @@ -0,0 +1,28 @@ +{% extends "base.html" %} +{# Copyright The IETF Trust 2020, All Rights Reserved #} +{% load origin %} +{% load static %} +{% load bootstrap3 %} +{% load person_filters %} + +{% block title %} + Send reminder to action holders for {{ titletext }} +{% endblock %} + +{% block content %} + {% origin %} +

Send reminder to action holders
{{ titletext }}

+ +
+ {% csrf_token %} + {% bootstrap_form form %} + +

This reminder will be sent to + {% for person in doc.action_holders.all %} + {% if forloop.last and not forloop.first %} and {% endif %}{% person_link person %}{% if not forloop.last %}, {% endif %}{% endfor %}.

+ {% buttons %} + + Cancel + {% endbuttons %} +
+{% endblock %} \ No newline at end of file diff --git a/ietf/templates/doc/search/status_columns.html b/ietf/templates/doc/search/status_columns.html index 698a703aa..c1abec656 100644 --- a/ietf/templates/doc/search/status_columns.html +++ b/ietf/templates/doc/search/status_columns.html @@ -1,5 +1,5 @@ {# Copyright The IETF Trust 2015, All Rights Reserved #}{% load origin %}{% origin %} -{% load ietf_filters ballot_icon %} +{% load ietf_filters ballot_icon person_filters %}
@@ -61,6 +61,12 @@ {{ m.due|date:"M Y" }}{% if not forloop.last %}, {% endif %} {% endfor %} + {% if doc.action_holders_enabled and doc.action_holders.exists %} +
Action Holders: + {% for action_holder in doc.documentactionholder_set.all %} + {% person_link action_holder.person title=action_holder.role_for_doc %}{{ action_holder|action_holder_badge }}{% if not forloop.last %}, {% endif %} + {% endfor %} + {% endif %} {% else %}{# RFC #} {{ doc.std_level|safe }} RFC