From 233bff8196d1e8ccc407224952700b0e3fee982e Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Mon, 18 Jan 2021 14:55:25 +0000 Subject: [PATCH] Improve handling of submissions for closed working groups. Fixes #3058. Commit ready for merge. - Legacy-Id: 18798 --- ietf/group/models.py | 38 +- ietf/group/views.py | 18 +- ...19_add_ad_approval_request_mailtriggers.py | 58 +++ ietf/mailtrigger/models.py | 21 + ietf/name/fixtures/names.json | 83 +++- ...21_add_ad_appr_draftsubmissionstatename.py | 70 ++++ ietf/submit/mail.py | 36 +- ietf/submit/models.py | 37 +- ietf/submit/tests.py | 373 +++++++++++++++++- ietf/submit/utils.py | 170 ++++++-- ietf/submit/views.py | 76 ++-- ietf/templates/group/active_wgs.html | 6 +- ietf/templates/submit/approval_request.txt | 8 +- ietf/templates/submit/submission_status.html | 8 +- 14 files changed, 877 insertions(+), 125 deletions(-) create mode 100644 ietf/mailtrigger/migrations/0019_add_ad_approval_request_mailtriggers.py create mode 100644 ietf/name/migrations/0021_add_ad_appr_draftsubmissionstatename.py diff --git a/ietf/group/models.py b/ietf/group/models.py index 22c98a590..aa6f55cec 100644 --- a/ietf/group/models.py +++ b/ietf/group/models.py @@ -49,6 +49,8 @@ class GroupInfo(models.Model): uses_milestone_dates = models.BooleanField(default=True) + ACTIVE_STATE_IDS = ('active', 'bof', 'proposed') # states considered "active" + def __str__(self): return self.name @@ -72,12 +74,39 @@ class GroupInfo(models.Model): def is_bof(self): return self.state_id in ["bof", "bof-conc"] + @property + def is_wg(self): + return self.type_id == 'wg' + + @property + def is_active(self): + # N.B., this has only been thought about for groups of type WG! + return self.state_id in self.ACTIVE_STATE_IDS + + @property + def is_individual(self): + return self.acronym == 'none' + + @property + def area(self): + if self.type_id == 'area': + return self + elif not self.is_individual and self.parent: + return self.parent + return None + class Meta: abstract = True class GroupManager(models.Manager): + def wgs(self): + return self.get_queryset().filter(type='wg') + def active_wgs(self): - return self.get_queryset().filter(type='wg', state__in=('bof','proposed','active')) + return self.wgs().filter(state__in=Group.ACTIVE_STATE_IDS) + + def closed_wgs(self): + return self.wgs().exclude(state__in=Group.ACTIVE_STATE_IDS) class Group(GroupInfo): objects = GroupManager() @@ -114,6 +143,13 @@ class Group(GroupInfo): chair = self.role_set.filter(name__slug='chair')[:1] return chair and chair[0] or None + @property + def ads(self): + return sorted( + self.role_set.filter(name="ad").select_related("email", "person"), + key=lambda role: role.person.name_parts()[3], # gets last name + ) + # these are copied to Group because it is still proxied. @property def upcase_acronym(self): diff --git a/ietf/group/views.py b/ietf/group/views.py index d12f7ef74..463a847cc 100644 --- a/ietf/group/views.py +++ b/ietf/group/views.py @@ -221,7 +221,6 @@ def wg_summary_area(request, group_type): raise Http404 areas = Group.objects.filter(type="area", state="active").order_by("name") for area in areas: - area.ads = sorted(roles(area, "ad"), key=extract_last_name) area.groups = Group.objects.filter(parent=area, type="wg", state="active").order_by("acronym") for group in area.groups: group.chairs = sorted(roles(group, "chair"), key=extract_last_name) @@ -250,13 +249,11 @@ def wg_charters(request, group_type): raise Http404 areas = Group.objects.filter(type="area", state="active").order_by("name") for area in areas: - area.ads = sorted(roles(area, "ad"), key=extract_last_name) area.groups = Group.objects.filter(parent=area, type="wg", state="active").order_by("name") for group in area.groups: fill_in_charter_info(group) fill_in_wg_roles(group) fill_in_wg_drafts(group) - group.area = area return render(request, 'group/1wg-charters.txt', { 'areas': areas }, content_type='text/plain; charset=UTF-8') @@ -265,17 +262,12 @@ def wg_charters(request, group_type): def wg_charters_by_acronym(request, group_type): if group_type != "wg": raise Http404 - areas = dict((a.id, a) for a in Group.objects.filter(type="area", state="active").order_by("name")) - - for area in areas.values(): - area.ads = sorted(roles(area, "ad"), key=extract_last_name) groups = Group.objects.filter(type="wg", state="active").exclude(parent=None).order_by("acronym") for group in groups: fill_in_charter_info(group) fill_in_wg_roles(group) fill_in_wg_drafts(group) - group.area = areas.get(group.parent_id) return render(request, 'group/1wg-charters-by-acronym.txt', { 'groups': groups }, content_type='text/plain; charset=UTF-8') @@ -313,7 +305,6 @@ def active_dirs(request): dirs = Group.objects.filter(type__in=['dir', 'review'], state="active").order_by("name") for group in dirs: group.chairs = sorted(roles(group, "chair"), key=extract_last_name) - group.ads = sorted(roles(group, "ad"), key=extract_last_name) group.secretaries = sorted(roles(group, "secr"), key=extract_last_name) return render(request, 'group/active_dirs.html', {'dirs' : dirs }) @@ -321,7 +312,6 @@ def active_review_dirs(request): dirs = Group.objects.filter(type="review", state="active").order_by("name") for group in dirs: group.chairs = sorted(roles(group, "chair"), key=extract_last_name) - group.ads = sorted(roles(group, "ad"), key=extract_last_name) group.secretaries = sorted(roles(group, "secr"), key=extract_last_name) return render(request, 'group/active_review_dirs.html', {'dirs' : dirs }) @@ -345,14 +335,15 @@ def active_wgs(request): areas = Group.objects.filter(type="area", state="active").order_by("name") for area in areas: # dig out information for template - area.ads = (list(sorted(roles(area, "ad"), key=extract_last_name)) - + list(sorted(roles(area, "pre-ad"), key=extract_last_name))) + area.ads_and_pre_ads = ( + list(area.ads) + list(sorted(roles(area, "pre-ad"), key=extract_last_name)) + ) area.groups = Group.objects.filter(parent=area, type="wg", state="active").order_by("acronym") area.urls = area.groupextresource_set.all().order_by("name") for group in area.groups: group.chairs = sorted(roles(group, "chair"), key=extract_last_name) - group.ad_out_of_area = group.ad_role() and group.ad_role().person not in [role.person for role in area.ads] + group.ad_out_of_area = group.ad_role() and group.ad_role().person not in [role.person for role in area.ads_and_pre_ads] # get the url for mailing list subscription if group.list_subscribe.startswith('http'): group.list_subscribe_url = group.list_subscribe @@ -378,7 +369,6 @@ def active_ags(request): groups = Group.objects.filter(type="ag", state="active").order_by("acronym") for group in groups: group.chairs = sorted(roles(group, "chair"), key=extract_last_name) - group.ads = sorted(roles(group, "ad"), key=extract_last_name) return render(request, 'group/active_ags.html', { 'groups': groups }) diff --git a/ietf/mailtrigger/migrations/0019_add_ad_approval_request_mailtriggers.py b/ietf/mailtrigger/migrations/0019_add_ad_approval_request_mailtriggers.py new file mode 100644 index 000000000..b839a01e6 --- /dev/null +++ b/ietf/mailtrigger/migrations/0019_add_ad_approval_request_mailtriggers.py @@ -0,0 +1,58 @@ +# Generated by Django 2.2.17 on 2020-11-23 09:54 + +from django.db import migrations + + +def forward(apps, schema_editor): + """Add new MailTrigger and Recipients, remove the one being replaced""" + MailTrigger = apps.get_model('mailtrigger', 'MailTrigger') + Recipient = apps.get_model('mailtrigger', 'Recipient') + + MailTrigger.objects.create( + slug='sub_director_approval_requested', + desc='Recipients for a message requesting AD approval of a revised draft submission', + ).to.add( + Recipient.objects.create( + pk='sub_group_parent_directors', + desc="ADs for the parent group of a submission" + ) + ) + + MailTrigger.objects.create( + slug='sub_replaced_doc_chair_approval_requested', + desc='Recipients for a message requesting chair approval of a replaced WG document', + ).to.add( + Recipient.objects.get(pk='doc_group_chairs') + ) + + MailTrigger.objects.create( + slug='sub_replaced_doc_director_approval_requested', + desc='Recipients for a message requesting AD approval of a replaced WG document', + ).to.add( + Recipient.objects.create( + pk='doc_group_parent_directors', + desc='ADs for the parent group of a doc' + ) + ) + + +def reverse(apps, schema_editor): + """Remove the MailTrigger and Recipient created in the forward migration""" + MailTrigger = apps.get_model('mailtrigger', 'MailTrigger') + MailTrigger.objects.filter(slug='sub_director_approval_requested').delete() + MailTrigger.objects.filter(slug='sub_replaced_doc_chair_approval_requested').delete() + MailTrigger.objects.filter(slug='sub_replaced_doc_director_approval_requested').delete() + Recipient = apps.get_model('mailtrigger', 'Recipient') + Recipient.objects.filter(pk='sub_group_parent_directors').delete() + Recipient.objects.filter(pk='doc_group_parent_directors').delete() + + +class Migration(migrations.Migration): + + dependencies = [ + ('mailtrigger', '0018_interim_approve_announce'), + ] + + operations = [ + migrations.RunPython(forward, reverse) + ] diff --git a/ietf/mailtrigger/models.py b/ietf/mailtrigger/models.py index 3df49bf45..3f8b2b0ea 100644 --- a/ietf/mailtrigger/models.py +++ b/ietf/mailtrigger/models.py @@ -242,6 +242,27 @@ class Recipient(models.Model): addrs.extend(Recipient.objects.get(slug='group_chairs').gather(**{'group':submission.group})) return addrs + def gather_sub_group_parent_directors(self, **kwargs): + addrs = [] + if 'submission' in kwargs: + submission = kwargs['submission'] + if submission.group and submission.group.parent: + addrs.extend( + Recipient.objects.get( + slug='group_responsible_directors').gather(group=submission.group.parent) + ) + return addrs + + def gather_doc_group_parent_directors(self, **kwargs): + addrs = [] + doc = kwargs.get('doc') + if doc and doc.group and doc.group.parent: + addrs.extend( + Recipient.objects.get( + slug='group_responsible_directors').gather(group=doc.group.parent) + ) + return addrs + def gather_submission_confirmers(self, **kwargs): """If a submitted document is revising an existing document, the confirmers are the authors of that existing document, and the chairs if the document is diff --git a/ietf/name/fixtures/names.json b/ietf/name/fixtures/names.json index 174712ba9..7081a1aff 100644 --- a/ietf/name/fixtures/names.json +++ b/ietf/name/fixtures/names.json @@ -4874,6 +4874,17 @@ "model": "mailtrigger.mailtrigger", "pk": "sub_confirmation_requested" }, + { + "fields": { + "cc": [], + "desc": "Recipients for a message requesting AD approval of a revised draft submission", + "to": [ + "sub_group_parent_directors" + ] + }, + "model": "mailtrigger.mailtrigger", + "pk": "sub_director_approval_requested" + }, { "fields": { "cc": [], @@ -4926,6 +4937,28 @@ "model": "mailtrigger.mailtrigger", "pk": "sub_new_wg_00" }, + { + "fields": { + "cc": [], + "desc": "Recipients for a message requesting chair approval of a replaced WG document", + "to": [ + "doc_group_chairs" + ] + }, + "model": "mailtrigger.mailtrigger", + "pk": "sub_replaced_doc_chair_approval_requested" + }, + { + "fields": { + "cc": [], + "desc": "Recipients for a message requesting AD approval of a replaced WG document", + "to": [ + "doc_group_parent_directors" + ] + }, + "model": "mailtrigger.mailtrigger", + "pk": "sub_replaced_doc_director_approval_requested" + }, { "fields": { "desc": "The person providing a comment to nomcom", @@ -5030,6 +5063,14 @@ "model": "mailtrigger.recipient", "pk": "doc_group_mail_list" }, + { + "fields": { + "desc": "ADs for the parent group of a doc", + "template": null + }, + "model": "mailtrigger.recipient", + "pk": "doc_group_parent_directors" + }, { "fields": { "desc": "The document's group's responsible AD(s) or IRTF chair", @@ -5518,6 +5559,14 @@ "model": "mailtrigger.recipient", "pk": "stream_managers" }, + { + "fields": { + "desc": "ADs for the parent group of a submission", + "template": null + }, + "model": "mailtrigger.recipient", + "pk": "sub_group_parent_directors" + }, { "fields": { "desc": "The authors of a submitted draft", @@ -9686,6 +9735,20 @@ "model": "name.docurltagname", "pk": "yang-module-metadata" }, + { + "fields": { + "desc": "", + "name": "Awaiting AD Approval", + "next_states": [ + "cancel", + "posted" + ], + "order": 5, + "used": true + }, + "model": "name.draftsubmissionstatename", + "pk": "ad-appr" + }, { "fields": { "desc": "", @@ -9721,7 +9784,7 @@ "desc": "", "name": "Cancelled", "next_states": [], - "order": 6, + "order": 7, "used": true }, "model": "name.draftsubmissionstatename", @@ -9763,7 +9826,7 @@ "cancel", "posted" ], - "order": 5, + "order": 6, "used": true }, "model": "name.draftsubmissionstatename", @@ -9774,7 +9837,7 @@ "desc": "", "name": "Posted", "next_states": [], - "order": 7, + "order": 8, "used": true }, "model": "name.draftsubmissionstatename", @@ -9805,7 +9868,7 @@ "cancel", "posted" ], - "order": 8, + "order": 9, "used": true }, "model": "name.draftsubmissionstatename", @@ -15008,7 +15071,7 @@ "fields": { "command": "xym", "switch": "--version", - "time": "2020-10-09T00:17:03.141", + "time": "2020-11-14T00:10:15.888", "used": true, "version": "xym 0.4.8" }, @@ -15019,9 +15082,9 @@ "fields": { "command": "pyang", "switch": "--version", - "time": "2020-10-09T00:17:04.888", + "time": "2020-11-14T00:10:17.069", "used": true, - "version": "pyang 2.3.2" + "version": "pyang 2.4.0" }, "model": "utils.versioninfo", "pk": 2 @@ -15030,7 +15093,7 @@ "fields": { "command": "yanglint", "switch": "--version", - "time": "2020-10-09T00:17:05.228", + "time": "2020-11-14T00:10:17.405", "used": true, "version": "yanglint SO 1.6.7" }, @@ -15041,9 +15104,9 @@ "fields": { "command": "xml2rfc", "switch": "--version", - "time": "2020-10-09T00:17:07.630", + "time": "2020-11-14T00:10:19.405", "used": true, - "version": "xml2rfc 3.2.1" + "version": "xml2rfc 3.4.0" }, "model": "utils.versioninfo", "pk": 4 diff --git a/ietf/name/migrations/0021_add_ad_appr_draftsubmissionstatename.py b/ietf/name/migrations/0021_add_ad_appr_draftsubmissionstatename.py new file mode 100644 index 000000000..825c93e6a --- /dev/null +++ b/ietf/name/migrations/0021_add_ad_appr_draftsubmissionstatename.py @@ -0,0 +1,70 @@ +# Generated by Django 2.2.17 on 2020-11-18 07:41 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('name', '0020_add_rescheduled_session_name'), + ] + + def up(apps, schema_editor): + DraftSubmissionStateName = apps.get_model('name', 'DraftSubmissionStateName') + new_state_name = DraftSubmissionStateName.objects.create( + slug='ad-appr', + name='Awaiting AD Approval', + used=True, + ) + new_state_name.next_states.add(DraftSubmissionStateName.objects.get(slug='posted')) + new_state_name.next_states.add(DraftSubmissionStateName.objects.get(slug='cancel')) + DraftSubmissionStateName.objects.get(slug='uploaded').next_states.add(new_state_name) + + # Order so the '-appr' states are together + for slug, order in ( + ('confirmed', 0), + ('uploaded', 1), + ('auth', 2), + ('aut-appr', 3), + ('grp-appr', 4), + ('ad-appr', 5), + ('manual', 6), + ('cancel', 7), + ('posted', 8), + ('waiting-for-draft', 9), + ): + state_name = DraftSubmissionStateName.objects.get(slug=slug) + state_name.order = order + state_name.save() + + def down(apps, schema_editor): + DraftSubmissionStateName = apps.get_model('name', 'DraftSubmissionStateName') + Submission = apps.get_model('submit', 'Submission') + + name_to_delete = DraftSubmissionStateName.objects.get(slug='ad-appr') + + # Refuse to migrate if there are any Submissions using the state we're about to remove + assert(Submission.objects.filter(state=name_to_delete).count() == 0) + + DraftSubmissionStateName.objects.get(slug='uploaded').next_states.remove(name_to_delete) + name_to_delete.delete() + + # restore original order + for slug, order in ( + ('confirmed', 0), + ('uploaded', 1), + ('auth', 2), + ('aut-appr', 3), + ('grp-appr', 4), + ('manual', 5), + ('cancel', 6), + ('posted', 7), + ('waiting-for-draft', 8), + ): + state_name = DraftSubmissionStateName.objects.get(slug=slug) + state_name.order = order + state_name.save() + + operations = [ + migrations.RunPython(up, down), + ] diff --git a/ietf/submit/mail.py b/ietf/submit/mail.py index 32dd89e25..b3e943de8 100644 --- a/ietf/submit/mail.py +++ b/ietf/submit/mail.py @@ -67,23 +67,49 @@ def send_full_url(request, submission): all_addrs.extend(cc) return all_addrs -def send_approval_request_to_group(request, submission): + +def send_approval_request(request, submission, replaced_doc=None): + """Send an approval request for a submission + + If replaced_doc is not None, requests will be sent to the wg chairs or ADs + responsible for that doc's group instead of the submission. + """ subject = 'New draft waiting for approval: %s' % submission.name from_email = settings.IDSUBMIT_FROM_EMAIL - (to_email,cc) = gather_address_lists('sub_chair_approval_requested',submission=submission) + + # Sort out which MailTrigger to use + mt_kwargs = dict(submission=submission) + if replaced_doc: + mt_kwargs['doc'] = replaced_doc + if submission.state_id == 'ad-appr': + approval_type = 'ad' + if replaced_doc: + mt_slug = 'sub_replaced_doc_director_approval_requested' + else: + mt_slug = 'sub_director_approval_requested' + else: + approval_type = 'chair' + if replaced_doc: + mt_slug = 'sub_replaced_doc_chair_approval_requested' + else: + mt_slug = 'sub_chair_approval_requested' + + (to_email,cc) = gather_address_lists(mt_slug, **mt_kwargs) if not to_email: return to_email - send_mail(request, to_email, from_email, subject, 'submit/approval_request.txt', + send_mail(request, to_email, from_email, subject, 'submit/approval_request.txt', { - 'submission': submission, - 'domain': Site.objects.get_current().domain, + 'submission': submission, + 'domain': Site.objects.get_current().domain, + 'approval_type': approval_type, }, cc=cc) all_addrs = to_email all_addrs.extend(cc) return all_addrs + def send_manual_post_request(request, submission, errors): subject = 'Manual Post Requested for %s' % submission.name from_email = settings.IDSUBMIT_FROM_EMAIL diff --git a/ietf/submit/models.py b/ietf/submit/models.py index 5dfb55863..3ecd06c13 100644 --- a/ietf/submit/models.py +++ b/ietf/submit/models.py @@ -77,7 +77,42 @@ class Submission(models.Model): def has_yang(self): return any ( [ c.checker=='yang validation' and c.passed is not None for c in self.latest_checks()] ) - + + @property + def replaces_names(self): + return self.replaces.split(',') + + @property + def area(self): + return self.group.area if self.group else None + + @property + def is_individual(self): + return self.group.is_individual if self.group else True + + @property + def revises_wg_draft(self): + return ( + self.rev != '00' + and self.group + and self.group.is_wg + ) + + @property + def active_wg_drafts_replaced(self): + return Document.objects.filter( + docalias__name__in=self.replaces.split(','), + group__in=Group.objects.active_wgs() + ) + + @property + def closed_wg_drafts_replaced(self): + return Document.objects.filter( + docalias__name__in=self.replaces.split(','), + group__in=Group.objects.closed_wgs() + ) + + class SubmissionCheck(models.Model): time = models.DateTimeField(default=datetime.datetime.now) submission = ForeignKey(Submission, related_name='checks') diff --git a/ietf/submit/tests.py b/ietf/submit/tests.py index 07beee539..0ee552c6e 100644 --- a/ietf/submit/tests.py +++ b/ietf/submit/tests.py @@ -379,6 +379,85 @@ class SubmitTests(TestCase): def text_submit_new_wg_txt_xml(self): self.submit_new_wg(["txt", "xml"]) + def test_submit_new_wg_as_author(self): + """A new WG submission by a logged-in author needs chair approval""" + # submit new -> supply submitter info -> approve + mars = GroupFactory(type_id='wg', acronym='mars') + draft = WgDraftFactory(group=mars) + setup_default_community_list_for_group(draft.group) + + name = "draft-ietf-mars-testing-tests" + rev = "00" + group = "mars" + + status_url, author = self.do_submission(name, rev, group) + username = author.user.email + + # supply submitter info, then draft should be in and ready for approval + mailbox_before = len(outbox) + self.client.login(username=username, password=username+'+password') # log in as the author + r = self.supply_extra_metadata(name, status_url, author.ascii, author.email().address.lower(), replaces='') + self.assertEqual(r.status_code, 302) + status_url = r["Location"] + + # Draft should be in the 'grp-appr' state to await approval by WG chair + submission = Submission.objects.get(name=name, rev=rev) + self.assertEqual(submission.state_id, 'grp-appr') + + # Approval request notification should be sent to the WG chair + self.assertEqual(len(outbox), mailbox_before + 1) + self.assertTrue("New draft waiting for approval" in outbox[-1]["Subject"]) + self.assertTrue(name in outbox[-1]["Subject"]) + print(outbox[-1]["To"]) + self.assertTrue('mars-chairs@ietf.org' in outbox[-1]['To']) + + # Status page should show that group chair approval is needed + r = self.client.get(status_url) + self.assertEqual(r.status_code, 200) + self.assertContains(r, 'The submission is pending approval by the group chairs.') + + def submit_new_concluded_wg_as_author(self, group_state_id='conclude'): + """A new concluded WG submission by a logged-in author needs AD approval""" + mars = GroupFactory(type_id='wg', acronym='mars', state_id=group_state_id) + draft = WgDraftFactory(group=mars) + setup_default_community_list_for_group(draft.group) + + name = "draft-ietf-mars-testing-tests" + rev = "00" + group = "mars" + + status_url, author = self.do_submission(name, rev, group) + username = author.user.email + + # Should receive an error because group is not active + self.client.login(username=username, password=username+'+password') # log in as the author + r = self.client.get(status_url) + self.assertEqual(r.status_code, 200) + self.assertContains(r, 'Group exists but is not an active group') + q = PyQuery(r.content) + post_button = q('[type=submit]:contains("Post")') + self.assertEqual(len(post_button), 0) # no UI option to post the submission in this state + + # Try to post anyway + r = self.client.post(status_url, + {'submitter-name': author.name, + 'submitter-email': username, + 'action': 'autopost', + 'replaces': ''}) + # Attempt should fail and draft should remain in the uploaded state + self.assertEqual(r.status_code, 403) + submission = Submission.objects.get(name=name, rev=rev) + self.assertEqual(submission.state_id, 'uploaded') + + def test_submit_new_concluded_wg_as_author(self): + self.submit_new_concluded_wg_as_author() + + def test_submit_new_bofconc_wg_as_author(self): + self.submit_new_concluded_wg_as_author('bof-conc') + + def test_submit_new_replaced_wg_as_author(self): + self.submit_new_concluded_wg_as_author('replaced') + 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 if stream_type == 'ietf': @@ -592,6 +671,67 @@ class SubmitTests(TestCase): def test_submit_existing_iab(self): self.submit_existing(["txt"],stream_type='iab', group_type='individ') + def do_submit_existing_concluded_wg_test(self, group_state_id='conclude', submit_as_author=False): + """A revision to an existing WG draft should go to AD for approval if WG is not active""" + ad = Person.objects.get(user__username='ad') + area = GroupFactory(type_id='area') + RoleFactory(name_id='ad', group=area, person=ad) + group = GroupFactory(type_id='wg', state_id=group_state_id, parent=area, acronym='mars') + author = PersonFactory() + draft = WgDraftFactory(group=group, authors=[author]) + draft.set_state(State.objects.get(type_id='draft-stream-ietf', slug='wg-doc')) + + name = draft.name + rev = "%02d" % (int(draft.rev) + 1) + + if submit_as_author: + username = author.user.email + self.client.login(username=username, password=username + '+password') + status_url, author = self.do_submission(name, rev, group, author=author) + mailbox_before = len(outbox) + + # Try to post anyway + r = self.client.post(status_url, + {'submitter-name': author.name, + 'submitter-email': 'submitter@example.com', + 'action': 'autopost', + 'replaces': ''}) + self.assertEqual(r.status_code, 302) + status_url = r["Location"] + + # Draft should be in the 'ad-appr' state to await approval + submission = Submission.objects.get(name=name, rev=rev) + self.assertEqual(submission.state_id, 'ad-appr') + + # Approval request notification should be sent to the AD for the group + self.assertEqual(len(outbox), mailbox_before + 1) + self.assertTrue("New draft waiting for approval" in outbox[-1]["Subject"]) + self.assertTrue(name in outbox[-1]["Subject"]) + self.assertTrue(ad.user.email in outbox[-1]['To']) + + # Status page should show that AD approval is needed + r = self.client.get(status_url) + self.assertEqual(r.status_code, 200) + self.assertContains(r, 'The submission is pending approval by the area director.') + + def test_submit_existing_concluded_wg(self): + self.do_submit_existing_concluded_wg_test() + + def test_submit_existing_concluded_wg_as_author(self): + self.do_submit_existing_concluded_wg_test(submit_as_author=True) + + def test_submit_existing_bofconc_wg(self): + self.do_submit_existing_concluded_wg_test(group_state_id='bof-conc') + + def test_submit_existing_bofconc_wg_as_author(self): + self.do_submit_existing_concluded_wg_test(group_state_id='bof-conc', submit_as_author=True) + + def test_submit_existing_replaced_wg(self): + self.do_submit_existing_concluded_wg_test(group_state_id='replaced') + + def test_submit_existing_replaced_wg_as_author(self): + self.do_submit_existing_concluded_wg_test(group_state_id='replaced', submit_as_author=True) + def submit_new_individual(self, formats): # submit new -> supply submitter info -> confirm @@ -737,6 +877,51 @@ class SubmitTests(TestCase): self.assertContains(r, draft.name) self.assertContains(r, draft.title) + def submit_new_individual_replacing_wg(self, logged_in=False, group_state_id='active', notify_ad=False): + """Chair of an active WG should be notified if individual draft is proposed to replace a WG draft""" + name = "draft-authorname-testing-tests" + rev = "00" + group = None + status_url, author = self.do_submission(name, rev, group) + + ad = Person.objects.get(user__username='ad') + replaced_draft = WgDraftFactory(group__state_id=group_state_id) + RoleFactory(name_id='ad', group=replaced_draft.group.parent, person=ad) + if logged_in: + username = author.user.email + self.client.login(username=username, password=username + '+password') + + mailbox_before = len(outbox) + self.supply_extra_metadata( + name, + status_url, + "Submitter Name", + "submitter@example.com", + replaces=str(replaced_draft.docalias.first().pk), + ) + + submission = Submission.objects.get(name=name, rev=rev) + self.assertEqual(submission.state_id, 'ad-appr' if notify_ad else 'grp-appr') + self.assertEqual(len(outbox), mailbox_before + 1) + notice = outbox[-1] + self.assertIn( + ad.user.email if notify_ad else '%s-chairs@ietf.org' % replaced_draft.group.acronym, + notice['To'] + ) + self.assertIn('New draft waiting for approval', notice['Subject']) + + def test_submit_new_individual_replacing_wg(self): + self.submit_new_individual_replacing_wg() + + def test_submit_new_individual_replacing_wg_logged_in(self): + self.submit_new_individual_replacing_wg(logged_in=True) + + def test_submit_new_individual_replacing_concluded_wg(self): + self.submit_new_individual_replacing_wg(group_state_id='conclude', notify_ad=True) + + def test_submit_new_individual_replacing_concluded_wg_logged_in(self): + self.submit_new_individual_replacing_wg(group_state_id='conclude', notify_ad=True, logged_in=True) + def test_submit_cancel_confirmation(self): ad=Person.objects.get(user__username='ad') # Group of None here does not reflect real individual submissions @@ -1338,12 +1523,110 @@ class SubmitTests(TestCase): """An existing SubmissionDocEvent with later rev should trigger manual processing""" self.submit_conflicting_submissiondocevent_rev('01', '02') + def do_wg_approval_auth_test(self, state, chair_can_approve=False): + """Helper to test approval authorization + + Assumes approval allowed by AD and secretary and, optionally, chair of WG + """ + class _SubmissionFactory: + """Helper class to generate fresh submissions""" + def __init__(self, author, state): + self.author = author + self.state = state + self.index = 0 + + def next(self): + self.index += 1 + sub = Submission.objects.create(name="draft-ietf-mars-bar-%d" % self.index, + group=Group.objects.get(acronym="mars"), + submission_date=datetime.date.today(), + authors=[dict(name=self.author.name, + email=self.author.user.email, + affiliation='affiliation', + country='country')], + rev="00", + state_id=self.state) + status_url = urlreverse('ietf.submit.views.submission_status', + kwargs=dict(submission_id=sub.pk, + access_token=sub.access_token())) + return sub, status_url + + def _assert_approval_refused(username, submission_factory, user_description): + """Helper to attempt to approve a document and check that it fails""" + if username: + self.client.login(username=username, password=username + '+password') + submission, status_url = submission_factory.next() + r = self.client.post(status_url, dict(action='approve')) + self.assertEqual(r.status_code, 403, '%s should not be able to approve' % user_description.capitalize()) + submission = Submission.objects.get(pk=submission.pk) # refresh from DB + self.assertEqual(submission.state_id, state, + 'Submission should still be awaiting approval after %s approval attempt fails' % user_description) + + def _assert_approval_allowed(username, submission_factory, user_description): + """Helper to attempt to approve a document and check that it succeeds""" + self.client.login(username=username, password=username + '+password') + submission, status_url = submission_factory.next() + r = self.client.post(status_url, dict(action='approve')) + self.assertEqual(r.status_code, 302, '%s should be able to approve' % user_description.capitalize()) + submission = Submission.objects.get(pk=submission.pk) # refresh from DB + self.assertEqual(submission.state_id, 'posted', + 'Submission should be posted after %s approves' % user_description) + + # create WGs + area = GroupFactory(type_id='area', acronym='area') + mars = GroupFactory(type_id='wg', acronym='mars', parent=area) # WG for submission + ames = GroupFactory(type_id='wg', acronym='ames', parent=area) # another WG + + # create / get users and roles + ad = Person.objects.get(user__username='ad') + RoleFactory(name_id='ad', group=area, person=ad) + RoleFactory(name_id='chair', group=mars, person__user__username='marschairman') + RoleFactory(name_id='chair', group=ames, person__user__username='ameschairman') + author = PersonFactory(user__username='author_user') + PersonFactory(user__username='ordinary_user') + + submission_factory = _SubmissionFactory(author, state) + + # Most users should not be allowed to approve + _assert_approval_refused(None, submission_factory, 'anonymous user') + _assert_approval_refused('ordinary_user', submission_factory, 'ordinary user') + _assert_approval_refused('author_user', submission_factory, 'author') + _assert_approval_refused('ameschairman', submission_factory, 'wrong WG chair') + + # chair of correct wg should be able to approve if chair_can_approve == True + if chair_can_approve: + _assert_approval_allowed('marschairman', submission_factory, 'WG chair') + else: + _assert_approval_refused('marschairman', submission_factory, 'WG chair') + + # ADs and secretaries can always approve + _assert_approval_allowed('ad', submission_factory, 'AD') + _assert_approval_allowed('secretary', submission_factory, 'secretary') + + def test_submit_wg_group_approval_auth(self): + """Group chairs should be able to approve submissions in grp-appr state""" + self.do_wg_approval_auth_test('grp-appr', chair_can_approve=True) + + def test_submit_wg_ad_approval_auth(self): + """Area directors should be able to approve submissions in ad-appr state""" + self.do_wg_approval_auth_test('ad-appr', chair_can_approve=False) class ApprovalsTestCase(TestCase): def test_approvals(self): - RoleFactory(name_id='chair', group__acronym='mars', person__user__username='marschairman') + RoleFactory(name_id='chair', + group__acronym='mars', + person__user__username='marschairman') + RoleFactory(name_id='chair', + group__acronym='ames', + person__user__username='ameschairman') + RoleFactory(name_id='ad', + group=Group.objects.get(acronym='mars').parent, + person=Person.objects.get(user__username='ad')) + RoleFactory(name_id='ad', + group=Group.objects.get(acronym='ames').parent, + person__user__username='other-ad') + url = urlreverse('ietf.submit.views.approvals') - self.client.login(username="marschairman", password="marschairman+password") Preapproval.objects.create(name="draft-ietf-mars-foo", by=Person.objects.get(user__username="marschairman")) Preapproval.objects.create(name="draft-ietf-mars-baz", by=Person.objects.get(user__username="marschairman")) @@ -1358,17 +1641,101 @@ class ApprovalsTestCase(TestCase): submission_date=datetime.date.today(), rev="00", state_id="grp-appr") + Submission.objects.create(name="draft-ietf-mars-quux", + group=Group.objects.get(acronym="mars"), + submission_date=datetime.date.today(), + rev="00", + state_id="ad-appr") - # get + # get as wg chair + self.client.login(username="marschairman", password="marschairman+password") r = self.client.get(url) self.assertEqual(r.status_code, 200) q = PyQuery(r.content) self.assertEqual(len(q('.approvals a:contains("draft-ietf-mars-foo")')), 0) self.assertEqual(len(q('.approvals a:contains("draft-ietf-mars-bar")')), 1) + self.assertEqual(len(q('.approvals a:contains("draft-ietf-mars-quux")')), 0) # wg chair does not see ad-appr self.assertEqual(len(q('.preapprovals td:contains("draft-ietf-mars-foo")')), 0) self.assertEqual(len(q('.preapprovals td:contains("draft-ietf-mars-baz")')), 1) + self.assertEqual(len(q('.preapprovals td:contains("draft-ietf-mars-bar")')), 0) + self.assertEqual(len(q('.preapprovals td:contains("draft-ietf-mars-quux")')), 0) self.assertEqual(len(q('.recently-approved a:contains("draft-ietf-mars-foo")')), 1) + self.assertEqual(len(q('.recently-approved a:contains("draft-ietf-mars-baz")')), 0) + self.assertEqual(len(q('.recently-approved a:contains("draft-ietf-mars-bar")')), 0) + self.assertEqual(len(q('.recently-approved a:contains("draft-ietf-mars-quux")')), 0) + + # get as AD - sees everything + self.client.login(username="ad", password="ad+password") + r = self.client.get(url) + self.assertEqual(r.status_code, 200) + q = PyQuery(r.content) + + self.assertEqual(len(q('.approvals a:contains("draft-ietf-mars-foo")')), 0) + self.assertEqual(len(q('.approvals a:contains("draft-ietf-mars-bar")')), 1) # AD sees grp-appr in their area + self.assertEqual(len(q('.approvals a:contains("draft-ietf-mars-quux")')), 1) # AD does see ad-appr + self.assertEqual(len(q('.preapprovals td:contains("draft-ietf-mars-foo")')), 0) + self.assertEqual(len(q('.preapprovals td:contains("draft-ietf-mars-baz")')), 1) + self.assertEqual(len(q('.preapprovals td:contains("draft-ietf-mars-bar")')), 0) + self.assertEqual(len(q('.preapprovals td:contains("draft-ietf-mars-quux")')), 0) + self.assertEqual(len(q('.recently-approved a:contains("draft-ietf-mars-foo")')), 1) + self.assertEqual(len(q('.recently-approved a:contains("draft-ietf-mars-baz")')), 0) + self.assertEqual(len(q('.recently-approved a:contains("draft-ietf-mars-bar")')), 0) + self.assertEqual(len(q('.recently-approved a:contains("draft-ietf-mars-quux")')), 0) + + # get as wg chair for a different group - should see nothing + self.client.login(username="ameschairman", password="ameschairman+password") + r = self.client.get(url) + self.assertEqual(r.status_code, 200) + q = PyQuery(r.content) + + self.assertEqual(len(q('.approvals a:contains("draft-ietf-mars-foo")')), 0) + self.assertEqual(len(q('.approvals a:contains("draft-ietf-mars-bar")')), 0) + self.assertEqual(len(q('.approvals a:contains("draft-ietf-mars-quux")')), 0) + self.assertEqual(len(q('.preapprovals td:contains("draft-ietf-mars-foo")')), 0) + self.assertEqual(len(q('.preapprovals td:contains("draft-ietf-mars-baz")')), 0) + self.assertEqual(len(q('.preapprovals td:contains("draft-ietf-mars-bar")')), 0) + self.assertEqual(len(q('.preapprovals td:contains("draft-ietf-mars-quux")')), 0) + self.assertEqual(len(q('.recently-approved a:contains("draft-ietf-mars-foo")')), 0) + self.assertEqual(len(q('.recently-approved a:contains("draft-ietf-mars-baz")')), 0) + self.assertEqual(len(q('.recently-approved a:contains("draft-ietf-mars-bar")')), 0) + self.assertEqual(len(q('.recently-approved a:contains("draft-ietf-mars-quux")')), 0) + + # get as AD for a different area - should see nothing + self.client.login(username="other-ad", password="other-ad+password") + r = self.client.get(url) + self.assertEqual(r.status_code, 200) + q = PyQuery(r.content) + + self.assertEqual(len(q('.approvals a:contains("draft-ietf-mars-foo")')), 0) + self.assertEqual(len(q('.approvals a:contains("draft-ietf-mars-bar")')), 0) + self.assertEqual(len(q('.approvals a:contains("draft-ietf-mars-quux")')), 0) + self.assertEqual(len(q('.preapprovals td:contains("draft-ietf-mars-foo")')), 0) + self.assertEqual(len(q('.preapprovals td:contains("draft-ietf-mars-baz")')), 0) + self.assertEqual(len(q('.preapprovals td:contains("draft-ietf-mars-bar")')), 0) + self.assertEqual(len(q('.preapprovals td:contains("draft-ietf-mars-quux")')), 0) + self.assertEqual(len(q('.recently-approved a:contains("draft-ietf-mars-foo")')), 0) + self.assertEqual(len(q('.recently-approved a:contains("draft-ietf-mars-baz")')), 0) + self.assertEqual(len(q('.recently-approved a:contains("draft-ietf-mars-bar")')), 0) + self.assertEqual(len(q('.recently-approved a:contains("draft-ietf-mars-quux")')), 0) + + # get as secretary - should see everything + self.client.login(username="secretary", password="secretary+password") + r = self.client.get(url) + self.assertEqual(r.status_code, 200) + q = PyQuery(r.content) + + self.assertEqual(len(q('.approvals a:contains("draft-ietf-mars-foo")')), 0) + self.assertEqual(len(q('.approvals a:contains("draft-ietf-mars-bar")')), 1) + self.assertEqual(len(q('.approvals a:contains("draft-ietf-mars-quux")')), 1) + self.assertEqual(len(q('.preapprovals td:contains("draft-ietf-mars-foo")')), 0) + self.assertEqual(len(q('.preapprovals td:contains("draft-ietf-mars-baz")')), 1) + self.assertEqual(len(q('.preapprovals td:contains("draft-ietf-mars-bar")')), 0) + self.assertEqual(len(q('.preapprovals td:contains("draft-ietf-mars-quux")')), 0) + self.assertEqual(len(q('.recently-approved a:contains("draft-ietf-mars-foo")')), 1) + self.assertEqual(len(q('.recently-approved a:contains("draft-ietf-mars-baz")')), 0) + self.assertEqual(len(q('.recently-approved a:contains("draft-ietf-mars-bar")')), 0) + self.assertEqual(len(q('.recently-approved a:contains("draft-ietf-mars-quux")')), 0) def test_add_preapproval(self): RoleFactory(name_id='chair', group__acronym='mars', person__user__username='marschairman') diff --git a/ietf/submit/utils.py b/ietf/submit/utils.py index e75d3f323..501b0f4a5 100644 --- a/ietf/submit/utils.py +++ b/ietf/submit/utils.py @@ -24,7 +24,7 @@ from ietf.doc.models import ( Document, State, DocAlias, DocEvent, SubmissionDoc from ietf.doc.models import NewRevisionDocEvent from ietf.doc.models import RelatedDocument, DocRelationshipName from ietf.doc.utils import add_state_change_event, rebuild_reference_relations -from ietf.doc.utils import set_replaces_for_document +from ietf.doc.utils import set_replaces_for_document, prettify_std_name from ietf.doc.mails import send_review_possibly_replaces_request from ietf.group.models import Group from ietf.ietfauth.utils import has_role @@ -32,7 +32,7 @@ from ietf.name.models import StreamName, FormalLanguageName from ietf.person.models import Person, Email from ietf.community.utils import update_name_contains_indexes_with_new_doc from ietf.submit.mail import ( announce_to_lists, announce_new_version, announce_to_authors, - send_approval_request_to_group, send_submission_confirmation, announce_new_wg_00 ) + announce_new_wg_00, send_approval_request, send_submission_confirmation ) from ietf.submit.models import Submission, SubmissionEvent, Preapproval, DraftSubmissionStateName, SubmissionCheck from ietf.utils import log from ietf.utils.accesstoken import generate_random_key @@ -604,12 +604,19 @@ def approvable_submissions_for_user(user): if not user.is_authenticated: return [] - res = Submission.objects.filter(state="grp-appr").order_by('-submission_date') + # Submissions that are group / AD approvable by someone + group_approvable = Submission.objects.filter(state="grp-appr") + ad_approvable = Submission.objects.filter(state="ad-appr") if has_role(user, "Secretariat"): - return res + return (group_approvable | ad_approvable).order_by('-submission_date') - # those we can reach as chair - return res.filter(group__role__name="chair", group__role__person__user=user) + # group-approvable that we can reach as chair plus group-approvable that we can reach as AD + # plus AD-approvable that we can reach as ad + return ( + group_approvable.filter(group__role__name="chair", group__role__person__user=user) + | group_approvable.filter(group__parent__role__name="ad", group__parent__role__person__user=user) + | ad_approvable.filter(group__parent__role__name="ad", group__parent__role__person__user=user) + ).order_by('-submission_date') def preapprovals_for_user(user): if not user.is_authenticated: @@ -620,7 +627,11 @@ def preapprovals_for_user(user): if has_role(user, "Secretariat"): return res - acronyms = [g.acronym for g in Group.objects.filter(role__person__user=user, type__features__req_subm_approval=True)] + accessible_groups = ( + Group.objects.filter(role__person__user=user, type__features__req_subm_approval=True) + | Group.objects.filter(parent__role__name='ad', parent__role__person__user=user, type__features__req_subm_approval=True) + ) + acronyms = [g.acronym for g in accessible_groups] res = res.filter(name__regex="draft-[^-]+-(%s)-.*" % "|".join(acronyms)) @@ -634,8 +645,11 @@ def recently_approved_by_user(user, since): if has_role(user, "Secretariat"): return res - # those we can reach as chair - return res.filter(group__role__name="chair", group__role__person__user=user) + # those we can reach as chair or ad + return ( + res.filter(group__role__name="chair", group__role__person__user=user) + | res.filter(group__parent__role__name="ad", group__parent__role__person__user=user) + ) def expirable_submissions(older_than_days): cutoff = datetime.date.today() - datetime.timedelta(days=older_than_days) @@ -793,26 +807,107 @@ def apply_checkers(submission, file_name): tau = time.time() - mark log.log(f"ran submission checks ({tau:.3}s) for {file_name}") -def send_confirmation_emails(request, submission, requires_group_approval, requires_prev_authors_approval): - docevent_from_submission(request, submission, desc="Uploaded new revision") +def accept_submission_requires_prev_auth_approval(submission): + """Does acceptance process require approval of previous authors?""" + return Document.objects.filter(name=submission.name).exists() - if requires_group_approval: +def accept_submission_requires_group_approval(submission): + """Does acceptance process require group approval? + + Depending on the state of the group, this approval may come from group chair or area director. + """ + return ( + submission.rev == '00' + and submission.group and submission.group.features.req_subm_approval + and not Preapproval.objects.filter(name=submission.name).exists() + ) + +def accept_submission(request, submission, autopost=False): + """Accept a submission and post or put in correct state to await approvals + + If autopost is True, will post draft if submitter is authorized to do so. + """ + doc = submission.existing_document() + prev_authors = [] if not doc else [ author.person for author in doc.documentauthor_set.all() ] + curr_authors = [ get_person_from_name_email(author["name"], author.get("email")) + for author in submission.authors ] + # Is the user authenticated as an author who can approve this submission? + user_is_author = ( + request.user.is_authenticated + and request.user.person in (prev_authors if submission.rev != '00' else curr_authors) # type: ignore + ) + + # If "who" is None, docevent_from_submission will pull it out of submission + docevent_from_submission( + request, + submission, + desc="Uploaded new revision", + who=request.user.person if user_is_author else None, + ) + + replaces = DocAlias.objects.filter(name__in=submission.replaces_names) + pretty_replaces = '(none)' if not replaces else ( + ', '.join(prettify_std_name(r.name) for r in replaces) + ) + + active_wg_drafts_replaced = submission.active_wg_drafts_replaced + closed_wg_drafts_replaced = submission.closed_wg_drafts_replaced + + # Determine which approvals are required + requires_prev_authors_approval = accept_submission_requires_prev_auth_approval(submission) + requires_group_approval = accept_submission_requires_group_approval(submission) + requires_ad_approval = submission.revises_wg_draft and not submission.group.is_active + if submission.is_individual: + requires_prev_group_approval = active_wg_drafts_replaced.exists() + requires_prev_ad_approval = closed_wg_drafts_replaced.exists() + else: + requires_prev_group_approval = False + requires_prev_ad_approval = False + + # Partial message for submission event + sub_event_desc = 'Set submitter to \"%s\", replaces to %s' % (submission.submitter, pretty_replaces) + docevent_desc = None + address_list = [] + if requires_ad_approval or requires_prev_ad_approval: + submission.state_id = "ad-appr" + submission.save() + + if closed_wg_drafts_replaced.exists(): + replaced_document = closed_wg_drafts_replaced.first() + else: + replaced_document = None + + address_list = send_approval_request( + request, + submission, + replaced_document, # may be None + ) + sent_to = ', '.join(address_list) + sub_event_desc += ' and sent approval email to AD: %s' % sent_to + docevent_desc = "Request for posting approval emailed to AD: %s" % sent_to + elif requires_group_approval or requires_prev_group_approval: submission.state = DraftSubmissionStateName.objects.get(slug="grp-appr") submission.save() - sent_to = send_approval_request_to_group(request, submission) - - desc = "sent approval email to group chairs: %s" % ", ".join(sent_to) - docDesc = "Request for posting approval emailed to group chairs: %s" % ", ".join(sent_to) + if active_wg_drafts_replaced.exists(): + replaced_document = active_wg_drafts_replaced.first() + else: + replaced_document = None + address_list = send_approval_request( + request, + submission, + replaced_document, # may be None + ) + sent_to = ', '.join(address_list) + sub_event_desc += ' and sent approval email to group chairs: %s' % sent_to + docevent_desc = "Request for posting approval emailed to group chairs: %s" % sent_to + elif user_is_author and autopost: + # go directly to posting submission + sub_event_desc = "New version accepted (logged-in submitter: %s)" % request.user.person # type: ignore + post_submission(request, submission, sub_event_desc, sub_event_desc) + sub_event_desc = None # do not create submission event below, post_submission() handles it else: - group_authors_changed = False - doc = submission.existing_document() - if doc and doc.group: - old_authors = [ author.person for author in doc.documentauthor_set.all() ] - new_authors = [ get_person_from_name_email(author["name"], author.get("email")) for author in submission.authors ] - group_authors_changed = set(old_authors)!=set(new_authors) - submission.auth_key = generate_random_key() if requires_prev_authors_approval: submission.state = DraftSubmissionStateName.objects.get(slug="aut-appr") @@ -820,14 +915,29 @@ def send_confirmation_emails(request, submission, requires_group_approval, requi submission.state = DraftSubmissionStateName.objects.get(slug="auth") submission.save() - sent_to = send_submission_confirmation(request, submission, chair_notice=group_authors_changed) + group_authors_changed = False + doc = submission.existing_document() + if doc and doc.group: + old_authors = [ author.person for author in doc.documentauthor_set.all() ] + new_authors = [ get_person_from_name_email(author["name"], author.get("email")) for author in submission.authors ] + group_authors_changed = set(old_authors)!=set(new_authors) + address_list = send_submission_confirmation( + request, + submission, + chair_notice=group_authors_changed + ) + sent_to = ', '.join(address_list) if submission.state_id == "aut-appr": - desc = "sent confirmation email to previous authors: %s" % ", ".join(sent_to) - docDesc = "Request for posting confirmation emailed to previous authors: %s" % ", ".join(sent_to) + sub_event_desc += " and sent confirmation email to previous authors: %s" % sent_to + docevent_desc = "Request for posting confirmation emailed to previous authors: %s" % sent_to else: - desc = "sent confirmation email to submitter and authors: %s" % ", ".join(sent_to) - docDesc = "Request for posting confirmation emailed to submitter and authors: %s" % ", ".join(sent_to) - return sent_to, desc, docDesc + sub_event_desc += " and sent confirmation email to submitter and authors: %s" % sent_to + docevent_desc = "Request for posting confirmation emailed to submitter and authors: %s" % sent_to - + if sub_event_desc: + create_submission_event(request, submission, sub_event_desc) + if docevent_desc: + docevent_from_submission(request, submission, docevent_desc, who=Person.objects.get(name="(System)")) + + return address_list \ No newline at end of file diff --git a/ietf/submit/views.py b/ietf/submit/views.py index d54db999a..ecf65ebd5 100644 --- a/ietf/submit/views.py +++ b/ietf/submit/views.py @@ -22,23 +22,23 @@ from django.views.decorators.csrf import csrf_exempt import debug # pyflakes:ignore from ietf.doc.models import Document, DocAlias, AddedMessageEvent -from ietf.doc.utils import prettify_std_name from ietf.group.models import Group from ietf.group.utils import group_features_group_filter from ietf.ietfauth.utils import has_role, role_required from ietf.mailtrigger.utils import gather_address_lists from ietf.message.models import Message, MessageAttachment -from ietf.person.models import Person, Email +from ietf.person.models import Email from ietf.submit.forms import ( SubmissionManualUploadForm, SubmissionAutoUploadForm, AuthorForm, SubmitterForm, EditSubmissionForm, PreapprovalForm, ReplacesForm, SubmissionEmailForm, MessageModelForm ) -from ietf.submit.mail import ( send_full_url, send_manual_post_request, add_submission_email, get_reply_to ) +from ietf.submit.mail import send_full_url, send_manual_post_request, add_submission_email, get_reply_to from ietf.submit.models import (Submission, Preapproval, DraftSubmissionStateName, SubmissionEmailEvent ) from ietf.submit.utils import ( approvable_submissions_for_user, preapprovals_for_user, recently_approved_by_user, validate_submission, create_submission_event, docevent_from_submission, post_submission, cancel_submission, rename_submission_files, remove_submission_files, get_draft_meta, - get_submission, fill_in_submission, apply_checkers, send_confirmation_emails, save_files, - get_person_from_name_email, check_submission_revision_consistency ) + get_submission, fill_in_submission, apply_checkers, save_files, + check_submission_revision_consistency, accept_submission, accept_submission_requires_group_approval, + accept_submission_requires_prev_auth_approval) from ietf.stats.utils import clean_country_name from ietf.utils.accesstoken import generate_access_token from ietf.utils.log import log @@ -174,18 +174,7 @@ def api_submit(request): raise ValidationError('Submitter %s is not one of the document authors' % user.username) submission.submitter = user.person.formatted_email() - docevent_from_submission(request, submission, desc="Uploaded new revision") - - requires_group_approval = (submission.rev == '00' - and submission.group and submission.group.features.req_subm_approval - and not Preapproval.objects.filter(name=submission.name).exists()) - requires_prev_authors_approval = Document.objects.filter(name=submission.name) - - sent_to, desc, docDesc = send_confirmation_emails(request, submission, requires_group_approval, requires_prev_authors_approval) - msg = "Set submitter to \"%s\" and %s" % (submission.submitter, desc) - create_submission_event(request, submission, msg) - docevent_from_submission(request, submission, docDesc, who=Person.objects.get(name="(System)")) - + sent_to = accept_submission(request, submission) return HttpResponse( "Upload of %s OK, confirmation requests sent to:\n %s" % (submission.name, ',\n '.join(sent_to)), @@ -253,10 +242,14 @@ def submission_status(request, submission_id, access_token=None): is_secretariat = has_role(request.user, "Secretariat") is_chair = submission.group and submission.group.has_role(request.user, "chair") + area = submission.area + is_ad = area and area.has_role(request.user, "ad") can_edit = can_edit_submission(request.user, submission, access_token) and submission.state_id == "uploaded" can_cancel = (key_matched or is_secretariat) and submission.state.next_states.filter(slug="cancel") - can_group_approve = (is_secretariat or is_chair) and submission.state_id == "grp-appr" + can_group_approve = (is_secretariat or is_ad or is_chair) and submission.state_id == "grp-appr" + can_ad_approve = (is_secretariat or is_ad) and submission.state_id == "ad-appr" + can_force_post = ( is_secretariat and submission.state.next_states.filter(slug="posted").exists() @@ -273,26 +266,18 @@ def submission_status(request, submission_id, access_token=None): # Convert from RFC 2822 format if needed confirmation_list = [ "%s <%s>" % parseaddr(a) for a in addresses ] - requires_group_approval = (submission.rev == '00' - and submission.group and submission.group.features.req_subm_approval - and not Preapproval.objects.filter(name=submission.name).exists()) - - requires_prev_authors_approval = Document.objects.filter(name=submission.name) - message = None - - - if submission.state_id == "cancel": message = ('error', 'This submission has been cancelled, modification is no longer possible.') elif submission.state_id == "auth": message = ('success', 'The submission is pending email authentication. An email has been sent to: %s' % ", ".join(confirmation_list)) elif submission.state_id == "grp-appr": message = ('success', 'The submission is pending approval by the group chairs.') + elif submission.state_id == "ad-appr": + message = ('success', 'The submission is pending approval by the area director.') elif submission.state_id == "aut-appr": message = ('success', 'The submission is pending approval by the authors of the previous version. An email has been sent to: %s' % ", ".join(confirmation_list)) - submitter_form = SubmitterForm(initial=submission.submitter_parsed(), prefix="submitter") replaces_form = ReplacesForm(name=submission.name,initial=DocAlias.objects.filter(name__in=submission.replaces.split(","))) @@ -313,6 +298,9 @@ def submission_status(request, submission_id, access_token=None): approvals_received = submitter_form.cleaned_data['approvals_received'] + if submission.rev == '00' and submission.group and not submission.group.is_active: + permission_denied(request, 'Posting a new draft for an inactive group is not permitted.') + if approvals_received: if not is_secretariat: permission_denied(request, 'You do not have permission to perform this action') @@ -324,26 +312,8 @@ def submission_status(request, submission_id, access_token=None): post_submission(request, submission, desc, desc) else: - doc = submission.existing_document() - prev_authors = [] if not doc else [ author.person for author in doc.documentauthor_set.all() ] - curr_authors = [ get_person_from_name_email(author["name"], author.get("email")) for author in submission.authors ] + accept_submission(request, submission, autopost=True) - if request.user.is_authenticated and request.user.person in (prev_authors if submission.rev != '00' else curr_authors): # type: ignore - # go directly to posting submission - docevent_from_submission(request, submission, desc="Uploaded new revision", who=request.user.person) # type: ignore - - desc = "New version accepted (logged-in submitter: %s)" % request.user.person # type: ignore - post_submission(request, submission, desc, desc) - - else: - sent_to, desc, docDesc = send_confirmation_emails(request, submission, requires_group_approval, requires_prev_authors_approval) - msg = "Set submitter to \"%s\", replaces to %s and %s" % ( - submission.submitter, - ", ".join(prettify_std_name(r.name) for r in replaces) if replaces else "(none)", - desc) - create_submission_event(request, submission, msg) - docevent_from_submission(request, submission, docDesc, who=Person.objects.get(name="(System)")) - if access_token: return redirect("ietf.submit.views.submission_status", submission_id=submission.pk, access_token=access_token) else: @@ -372,6 +342,13 @@ def submission_status(request, submission_id, access_token=None): return redirect("ietf.submit.views.submission_status", submission_id=submission_id) + elif action == "approve" and submission.state_id == "ad-appr": + if not can_ad_approve: + permission_denied(request, 'You do not have permission to perform this action.') + + post_submission(request, submission, "WG -00 approved", "Approved and posted submission") + + return redirect("ietf.doc.views_doc.document_main", name=submission.name) elif action == "approve" and submission.state_id == "grp-appr": if not can_group_approve: @@ -381,7 +358,6 @@ def submission_status(request, submission_id, access_token=None): return redirect("ietf.doc.views_doc.document_main", name=submission.name) - elif action == "forcepost" and submission.state.next_states.filter(slug="posted"): if not can_force_post: permission_denied(request, 'You do not have permission to perform this action.') @@ -416,8 +392,8 @@ def submission_status(request, submission_id, access_token=None): 'can_group_approve': can_group_approve, 'can_cancel': can_cancel, 'show_send_full_url': show_send_full_url, - 'requires_group_approval': requires_group_approval, - 'requires_prev_authors_approval': requires_prev_authors_approval, + 'requires_group_approval': accept_submission_requires_group_approval(submission), + 'requires_prev_authors_approval': accept_submission_requires_prev_auth_approval(submission), 'confirmation_list': confirmation_list, }) diff --git a/ietf/templates/group/active_wgs.html b/ietf/templates/group/active_wgs.html index 4a29b854d..6e5fd799f 100644 --- a/ietf/templates/group/active_wgs.html +++ b/ietf/templates/group/active_wgs.html @@ -24,10 +24,10 @@ {% for area in areas %}

{{ area.name }} ({{ area.acronym }})

- {% if area.ads %} -

{{ area.acronym }} Area Director{{ area.ads|pluralize }} (AD{{ area.ads|pluralize }})

+ {% if area.ads_and_pre_ads %} +

{{ area.acronym }} Area Director{{ area.ads_and_pre_ads|pluralize }} (AD{{ area.ads_and_pre_ads|pluralize }})