From 197194a41c60fa6ff9a24ba348989eb1417cfe6d Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Mon, 14 Jun 2021 17:11:23 +0000 Subject: [PATCH] Update action holders when a new draft is submitted. Fixes #3281. Commit ready for merge. - Legacy-Id: 19122 --- ietf/submit/tests.py | 78 ++++++++++++++++++++++++++++++++------------ ietf/submit/utils.py | 10 ++++-- 2 files changed, 65 insertions(+), 23 deletions(-) diff --git a/ietf/submit/tests.py b/ietf/submit/tests.py index 210017796..31f704456 100644 --- a/ietf/submit/tests.py +++ b/ietf/submit/tests.py @@ -24,7 +24,7 @@ from ietf.submit.utils import expirable_submissions, expire_submission from ietf.doc.factories import DocumentFactory, WgDraftFactory, IndividualDraftFactory from ietf.doc.models import ( Document, DocAlias, DocEvent, State, BallotPositionDocEvent, DocumentAuthor, SubmissionDocEvent ) -from ietf.doc.utils import create_ballot_if_not_open, can_edit_docextresources +from ietf.doc.utils import create_ballot_if_not_open, can_edit_docextresources, update_action_holders from ietf.group.factories import GroupFactory, RoleFactory from ietf.group.models import Group from ietf.group.utils import setup_default_community_list_for_group @@ -479,6 +479,14 @@ class SubmitTests(TestCase): def submit_existing(self, formats, change_authors=True, group_type='wg', stream_type='ietf'): # submit new revision of existing -> supply submitter info -> prev authors confirm + + def _assert_authors_are_action_holders(draft, expect=True): + for author in draft.authors(): + if expect: + self.assertIn(author, draft.action_holders.all()) + else: + self.assertNotIn(author, draft.action_holders.all()) + if stream_type == 'ietf': ad = Person.objects.get(user__username='ad') if group_type == 'area': @@ -489,10 +497,18 @@ class SubmitTests(TestCase): RoleFactory(name_id='ad',group=area,person=ad) group = GroupFactory(type_id=group_type, parent=area, acronym='mars') draft = DocumentFactory(type_id='draft', group=group, stream_id=stream_type, ad=ad, authors=PersonFactory.create_batch(1)) - draft.set_state(State.objects.get(type_id='draft-stream-ietf',slug='wg-doc')) + wg_doc_state = State.objects.get(type_id='draft-stream-ietf',slug='wg-doc') + draft.set_state(wg_doc_state) + update_action_holders(draft, new_state=wg_doc_state) # pretend IANA reviewed it - draft.set_state(State.objects.get(used=True, type="draft-iana-review", slug="not-ok")) + not_ok_state = State.objects.get(used=True, type="draft-iana-review", slug="not-ok") + draft.set_state(not_ok_state) + update_action_holders( + draft, + prev_state=State.objects.get(used=True, type="draft-iana-review", slug="changed"), + new_state=not_ok_state, + ) # pretend it was approved to check that we notify the RFC Editor e = DocEvent(type="iesg_approved", doc=draft, rev=draft.rev) @@ -531,6 +547,7 @@ class SubmitTests(TestCase): # Set the revision needed tag draft.tags.add("need-rev") + update_action_holders(draft, new_tags=draft.tags.all()) name = draft.name rev = "%02d" % (int(draft.rev) + 1) @@ -542,9 +559,12 @@ class SubmitTests(TestCase): f.write("a" * 2000) old_docevents = list(draft.docevent_set.all()) + _assert_authors_are_action_holders(draft, True) # authors should be action holders prior to the test status_url, author = self.do_submission(name, rev, group, formats, author=prev_author.person) + _assert_authors_are_action_holders(draft, True) # still waiting for author confirmation + # supply submitter info, then previous authors get a confirmation email mailbox_before = len(outbox) r = self.supply_extra_metadata(name, status_url, "Submitter Name", "submitter@example.com", replaces="") @@ -553,6 +573,7 @@ class SubmitTests(TestCase): r = self.client.get(status_url) self.assertEqual(r.status_code, 200) self.assertContains(r, "The submission is pending approval by the authors") + _assert_authors_are_action_holders(draft, True) # still waiting for author confirmation self.assertEqual(len(outbox), mailbox_before + 1) confirm_email = outbox[-1] @@ -592,6 +613,7 @@ class SubmitTests(TestCase): self.assertEqual(r.status_code, 302) new_docevents = draft.docevent_set.exclude(pk__in=[event.pk for event in old_docevents]) + _assert_authors_are_action_holders(draft, False) # confirmed and posted, authors no longer hold action # check we have document events doc_events = new_docevents.filter(type__in=["new_submission", "added_comment"]) @@ -605,30 +627,44 @@ class SubmitTests(TestCase): # docevents = list(new_docevents.order_by("-time", "-id")) # Latest events are first (this is the default, but we make it explicit) - # Assert event content in chronological order: - def inspect_docevents(docevents, event_delta, type, be_in_desc, by_name): - self.assertEqual(docevents[event_delta].type, type) - self.assertIn(be_in_desc, docevents[event_delta].desc) - self.assertEqual(docevents[event_delta].by.name, by_name) + def inspect_docevents(docevents, event_delta, event_type, be_in_desc, by_name): + self.assertEqual(docevents[event_delta].type, event_type, + 'Unexpected event type for event_delta={}'.format(event_delta)) + self.assertIn(be_in_desc, docevents[event_delta].desc, + 'Expected text not found for event_delta={}'.format(event_delta)) + self.assertEqual(docevents[event_delta].by.name, by_name, + 'Unexpected name for event_delta={}'.format(event_delta)) if len(docevents) > event_delta + 1: - self.assertGreater(docevents[event_delta].id, docevents[event_delta+1].id) + self.assertGreater(docevents[event_delta].id, docevents[event_delta+1].id, + 'Event out of order for event_delta={}'.format(event_delta)) + # Assert event content in chronological order: if draft.stream_id == 'ietf': - inspect_docevents(docevents, 5, "new_submission", "Uploaded new revision", "Submitter Name") - inspect_docevents(docevents, 4, "new_submission", "Request for posting confirmation", "(System)") - inspect_docevents(docevents, 3, "new_submission", "New version approved", "(System)") - inspect_docevents(docevents, 2, "new_revision", "New version available", "Submitter Name") - inspect_docevents(docevents, 1, "changed_state", "IANA Review", "(System)") - inspect_docevents(docevents, 0, "changed_document", "AD Followup", "(System)") + expected_docevents = [ + ("new_submission", "Uploaded new revision", "Submitter Name"), + ("new_submission", "Request for posting confirmation", "(System)"), + ("new_submission", "New version approved", "(System)"), + ("new_revision", "New version available", "Submitter Name"), + ("changed_state", "IANA Review", "(System)"), + ("changed_document", "AD Followup", "(System)"), + ("changed_action_holders", "IESG state changed", "(System)"), + ] elif draft.stream_id in ('ise', 'irtf', 'iab'): - inspect_docevents(docevents, 4, "new_submission", "Uploaded new revision", "Submitter Name") - inspect_docevents(docevents, 3, "new_submission", "Request for posting confirmation", "(System)") - inspect_docevents(docevents, 2, "new_submission", "New version approved", "(System)") - inspect_docevents(docevents, 1, "new_revision", "New version available", "Submitter Name") - inspect_docevents(docevents, 0, "changed_document", "tag cleared", "(System)") + expected_docevents = [ + ("new_submission", "Uploaded new revision", "Submitter Name"), + ("new_submission", "Request for posting confirmation", "(System)"), + ("new_submission", "New version approved", "(System)"), + ("new_revision", "New version available", "Submitter Name"), + ("changed_document", "tag cleared", "(System)"), + ("changed_action_holders", "IESG state changed", "(System)"), + ] else: - pass + expected_docevents = [] # empty list will skip the docevent test entirely + + # go through event list in reverse so newest gets index 0 + for event_delta, (event_type, be_in_desc, by_name) in enumerate(expected_docevents[::-1]): + inspect_docevents(docevents, event_delta, event_type, be_in_desc, by_name) self.assertTrue(not os.path.exists(os.path.join(self.repository_dir, "%s-%s.txt" % (name, old_rev)))) self.assertTrue(os.path.exists(os.path.join(self.archive_dir, "%s-%s.txt" % (name, old_rev)))) diff --git a/ietf/submit/utils.py b/ietf/submit/utils.py index 1f16a4e98..672aec5f2 100644 --- a/ietf/submit/utils.py +++ b/ietf/submit/utils.py @@ -26,7 +26,7 @@ from ietf.doc.models import NewRevisionDocEvent from ietf.doc.models import RelatedDocument, DocRelationshipName, DocExtResource from ietf.doc.utils import add_state_change_event, rebuild_reference_relations from ietf.doc.utils import ( set_replaces_for_document, prettify_std_name, - update_doc_extresources, can_edit_docextresources, update_documentauthors ) + update_doc_extresources, can_edit_docextresources, update_documentauthors, update_action_holders ) from ietf.doc.mails import send_review_possibly_replaces_request, send_external_resource_change_request from ietf.group.models import Group from ietf.ietfauth.utils import has_role @@ -372,6 +372,8 @@ def post_submission(request, submission, approved_doc_desc, approved_subm_desc): state_change_msg = "" if not was_rfc and draft.tags.filter(slug="need-rev"): + tags_before = list(draft.tags.all()) + draft.tags.remove("need-rev") if draft.stream_id == 'ietf': draft.tags.add("ad-f-up") @@ -384,9 +386,13 @@ def post_submission(request, submission, approved_doc_desc, approved_subm_desc): e.by = system e.save() events.append(e) - state_change_msg = e.desc + # Changed tags - update action holders if necessary + e = update_action_holders(draft, prev_tags=tags_before, new_tags=draft.tags.all()) + if e is not None: + events.append(e) + if draft.stream_id == "ietf" and draft.group.type_id == "wg" and draft.rev == "00": # automatically set state "WG Document" draft.set_state(State.objects.get(used=True, type="draft-stream-%s" % draft.stream_id, slug="wg-doc"))