diff --git a/ietf/doc/forms.py b/ietf/doc/forms.py index f9b1024fe..a0a76e0c0 100644 --- a/ietf/doc/forms.py +++ b/ietf/doc/forms.py @@ -5,13 +5,16 @@ import datetime import debug #pyflakes:ignore from django import forms +from django.core.exceptions import ObjectDoesNotExist, ValidationError from ietf.doc.fields import SearchableDocAliasesField, SearchableDocAliasField -from ietf.doc.models import RelatedDocument +from ietf.doc.models import RelatedDocument, DocExtResource from ietf.iesg.models import TelechatDate from ietf.iesg.utils import telechat_page_count from ietf.person.fields import SearchablePersonsField +from ietf.name.models import ExtResourceName +from ietf.utils.validators import validate_external_resource_value 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.") @@ -57,7 +60,7 @@ class NotifyForm(forms.Form): return ', '.join(addrspecs) class ActionHoldersForm(forms.Form): - action_holders = SearchablePersonsField(required=False) + action_holders = SearchablePersonsField(required=False) reason = forms.CharField( label='Reason for change', required=False, @@ -125,3 +128,77 @@ class AddDownrefForm(forms.Form): if v_err_refnorm: v_err_refnorm_prefix = "There does not seem to be a normative reference to RFC " + rfc.document.rfc_number() + " by " raise forms.ValidationError(v_err_refnorm_prefix + v_err_refnorm) + + +class ExtResourceForm(forms.Form): + resources = forms.CharField(widget=forms.Textarea, label="Additional Resources", required=False, + help_text=("Format: 'tag value (Optional description)'." + " Separate multiple entries with newline. When the value is a URL, use https:// where possible.") ) + + def __init__(self, *args, initial=None, extresource_model=None, **kwargs): + self.extresource_model = extresource_model + if initial: + kwargs = kwargs.copy() + resources = initial.get('resources') + if resources is not None and not isinstance(resources, str): + initial = initial.copy() + # Convert objects to string representation + initial['resources'] = self.format_resources(resources) + kwargs['initial'] = initial + super(ExtResourceForm, self).__init__(*args, **kwargs) + + @staticmethod + def format_resources(resources, fs="\n"): + # Might be better to shift to a formset instead of parsing these lines. + return fs.join([r.to_form_entry_str() for r in resources]) + + def clean_resources(self): + """Clean the resources field + + The resources field is a newline-separated set of resource entries. Each entry + should be " " or " ()" with any whitespace + delimiting the components. This clean only validates that the tag and value are + present and valid - tag must be a recognized ExtResourceName and value is + validated using validate_external_resource_value(). Further interpretation of + the resource is performed int he clean() method. + """ + lines = [x.strip() for x in self.cleaned_data["resources"].splitlines() if x.strip()] + errors = [] + for l in lines: + parts = l.split() + if len(parts) == 1: + errors.append("Too few fields: Expected at least tag and value: '%s'" % l) + elif len(parts) >= 2: + name_slug = parts[0] + try: + name = ExtResourceName.objects.get(slug=name_slug) + except ObjectDoesNotExist: + errors.append("Bad tag in '%s': Expected one of %s" % (l, ', '.join([ o.slug for o in ExtResourceName.objects.all() ]))) + continue + value = parts[1] + try: + validate_external_resource_value(name, value) + except ValidationError as e: + e.message += " : " + value + errors.append(e) + if errors: + raise ValidationError(errors) + return lines + + def clean(self): + """Clean operations after all other fields are cleaned by clean_ methods + + Converts resource strings into ExtResource model instances. + """ + cleaned_data = super(ExtResourceForm, self).clean() + cleaned_resources = [] + cls = self.extresource_model or DocExtResource + for crs in cleaned_data.get('resources', []): + cleaned_resources.append( + cls.from_form_entry_str(crs) + ) + cleaned_data['resources'] = cleaned_resources + + @staticmethod + def valid_resource_tags(): + return ExtResourceName.objects.all().order_by('slug').values_list('slug', flat=True) \ No newline at end of file diff --git a/ietf/doc/mails.py b/ietf/doc/mails.py index 8b2168cf7..d86b88597 100644 --- a/ietf/doc/mails.py +++ b/ietf/doc/mails.py @@ -672,3 +672,20 @@ def email_iana_expert_review_state_changed(request, events): dict(event=events[0], url=settings.IDTRACKER_BASE_URL + events[0].doc.get_absolute_url() ), cc = addrs.cc, ) + +def send_external_resource_change_request(request, doc, submitter_info, requested_resources): + """Send an email to requesting changes to a draft's external resources""" + addrs = gather_address_lists('doc_external_resource_change_requested', doc=doc) + to = set(addrs.to) + cc = set(addrs.cc) + + send_mail(request, list(to), settings.DEFAULT_FROM_EMAIL, + 'External resource change requested for %s' % doc.name, + 'doc/mail/external_resource_change_request.txt', + dict( + doc=doc, + submitter_info=submitter_info, + requested_resources=requested_resources, + doc_url=settings.IDTRACKER_BASE_URL + doc.get_absolute_url(), + ), + cc=list(cc),) diff --git a/ietf/doc/models.py b/ietf/doc/models.py index c43127aea..ba49b0741 100644 --- a/ietf/doc/models.py +++ b/ietf/doc/models.py @@ -922,8 +922,7 @@ class DocumentURL(models.Model): desc = models.CharField(max_length=255, default='', blank=True) url = models.URLField(max_length=2083) # 2083 is the legal max for URLs -class DocExtResource(models.Model): - doc = ForeignKey(Document) # Should this really be to DocumentInfo rather than Document? +class ExtResource(models.Model): name = models.ForeignKey(ExtResourceName, on_delete=models.CASCADE) display_name = models.CharField(max_length=255, default='', blank=True) value = models.CharField(max_length=2083) # 2083 is the maximum legal URL length @@ -931,6 +930,49 @@ class DocExtResource(models.Model): priority = self.display_name or self.name.name return u"%s (%s) %s" % (priority, self.name.slug, self.value) + class Meta: + abstract = True + + # The to_form_entry_str() and matching from_form_entry_str() class method are + # defined here to ensure that change request emails suggest resources in the + # correct format to cut-and-paste into the current textarea on the external + # resource form. If that is changed to a formset or other non-text entry field, + # these methods really should not be needed. + def to_form_entry_str(self): + """Serialize as a string suitable for entry in a form""" + if self.display_name: + return "%s %s (%s)" % (self.name.slug, self.value, self.display_name.strip('()')) + else: + return "%s %s" % (self.name.slug, self.value) + + @classmethod + def from_form_entry_str(cls, s): + """Create an instance from the form_entry_str format + + Expected format is " [ ()]" + Any text after the value is treated as the display name, with whitespace replaced by + spaces and leading/trailing parentheses stripped. + """ + parts = s.split(None, 2) + display_name = ' '.join(parts[2:]).strip('()') + kwargs = dict(name_id=parts[0], value=parts[1]) + if display_name: + kwargs['display_name'] = display_name + return cls(**kwargs) + + @classmethod + def from_sibling_class(cls, sib): + """Create an instance with same base attributes as another subclass instance""" + kwargs = dict() + for field in ExtResource._meta.get_fields(): + value = getattr(sib, field.name, None) + if value: + kwargs[field.name] = value + return cls(**kwargs) + +class DocExtResource(ExtResource): + doc = ForeignKey(Document) # Should this really be to DocumentInfo rather than Document? + class RelatedDocHistory(models.Model): source = ForeignKey('DocHistory') target = ForeignKey('DocAlias', related_name="reversely_related_document_history_set") diff --git a/ietf/doc/utils.py b/ietf/doc/utils.py index 1bda84dd9..76aa40ea5 100644 --- a/ietf/doc/utils.py +++ b/ietf/doc/utils.py @@ -29,7 +29,7 @@ from ietf.doc.models import DocEvent, ConsensusDocEvent, BallotDocEvent, IRSGBal 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 +from ietf.ietfauth.utils import has_role, is_authorized_in_doc_stream, is_individual_draft_author from ietf.person.models import Person from ietf.review.models import ReviewWish from ietf.utils import draft, text @@ -149,6 +149,11 @@ def can_unadopt_draft(user, doc): else: return False +def can_edit_docextresources(user, doc): + return (has_role(user, ("Secretariat", "Area Director")) + or is_authorized_in_doc_stream(user, doc) + or is_individual_draft_author(user, doc)) + def two_thirds_rule( recused=0 ): # For standards-track, need positions from 2/3 of the non-recused current IESG. active = Role.objects.filter(name="ad",group__type="area",group__state="active").count() @@ -1131,3 +1136,22 @@ def augment_docs_and_user_with_user_info(docs, user): for d in docs: d.tracked_in_personal_community_list = d.pk in tracked d.has_review_wish = d.pk in review_wished + + +def update_doc_extresources(doc, new_resources, by): + old_res_strs = '\n'.join(sorted(r.to_form_entry_str() for r in doc.docextresource_set.all())) + new_res_strs = '\n'.join(sorted(r.to_form_entry_str() for r in new_resources)) + + if old_res_strs == new_res_strs: + return False # no change + + doc.docextresource_set.all().delete() + for new_res in new_resources: + new_res.doc = doc + new_res.save() + e = DocEvent(doc=doc, rev=doc.rev, by=by, type='changed_document') + e.desc = "Changed document external resources from:\n\n%s\n\nto:\n\n%s" % ( + old_res_strs, new_res_strs) + e.save() + doc.save_with_history([e]) + return True \ No newline at end of file diff --git a/ietf/doc/views_draft.py b/ietf/doc/views_draft.py index 89cc51a2c..f7ae4592c 100644 --- a/ietf/doc/views_draft.py +++ b/ietf/doc/views_draft.py @@ -13,7 +13,6 @@ from django import forms from django.conf import settings from django.contrib import messages from django.contrib.auth.decorators import login_required -from django.core.exceptions import ValidationError, ObjectDoesNotExist from django.db.models import Q from django.http import HttpResponseRedirect, Http404 from django.shortcuts import render, get_object_or_404, redirect @@ -35,21 +34,22 @@ from ietf.doc.mails import ( email_pulled_from_rfc_queue, email_resurrect_reques from ietf.doc.utils import ( add_state_change_event, can_adopt_draft, can_unadopt_draft, 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, ) + set_replaces_for_document, default_consensus, tags_suffix, can_edit_docextresources, + update_doc_extresources ) from ietf.doc.lastcall import request_last_call from ietf.doc.fields import SearchableDocAliasesField +from ietf.doc.forms import ExtResourceForm from ietf.group.models import Group, Role, GroupFeatures from ietf.iesg.models import TelechatDate -from ietf.ietfauth.utils import has_role, is_authorized_in_doc_stream, user_is_person, is_individual_draft_author +from ietf.ietfauth.utils import has_role, is_authorized_in_doc_stream, user_is_person from ietf.ietfauth.utils import role_required from ietf.mailtrigger.utils import gather_address_lists from ietf.message.models import Message -from ietf.name.models import IntendedStdLevelName, DocTagName, StreamName, ExtResourceName +from ietf.name.models import IntendedStdLevelName, DocTagName, StreamName from ietf.person.fields import SearchableEmailField from ietf.person.models import Person, Email from ietf.utils.mail import send_mail, send_mail_message, on_behalf_of from ietf.utils.textupload import get_cleaned_text_file_content -from ietf.utils.validators import validate_external_resource_value from ietf.utils import log from ietf.utils.response import permission_denied @@ -1205,81 +1205,23 @@ def edit_consensus(request, name): def edit_doc_extresources(request, name): - class DocExtResourceForm(forms.Form): - resources = forms.CharField(widget=forms.Textarea, label="Additional Resources", required=False, - help_text=("Format: 'tag value (Optional description)'." - " Separate multiple entries with newline. When the value is a URL, use https:// where possible.") ) - - def clean_resources(self): - lines = [x.strip() for x in self.cleaned_data["resources"].splitlines() if x.strip()] - errors = [] - for l in lines: - parts = l.split() - if len(parts) == 1: - errors.append("Too few fields: Expected at least tag and value: '%s'" % l) - elif len(parts) >= 2: - name_slug = parts[0] - try: - name = ExtResourceName.objects.get(slug=name_slug) - except ObjectDoesNotExist: - errors.append("Bad tag in '%s': Expected one of %s" % (l, ', '.join([ o.slug for o in ExtResourceName.objects.all() ]))) - continue - value = parts[1] - try: - validate_external_resource_value(name, value) - except ValidationError as e: - e.message += " : " + value - errors.append(e) - if errors: - raise ValidationError(errors) - return lines - - def format_resources(resources, fs="\n"): - res = [] - for r in resources: - if r.display_name: - res.append("%s %s (%s)" % (r.name.slug, r.value, r.display_name.strip('()'))) - else: - res.append("%s %s" % (r.name.slug, r.value)) - # TODO: This is likely problematic if value has spaces. How then to delineate value and display_name? Perhaps in the short term move to comma or pipe separation. - # Might be better to shift to a formset instead of parsing these lines. - return fs.join(res) - doc = get_object_or_404(Document, name=name) - if not (has_role(request.user, ("Secretariat", "Area Director")) - or is_authorized_in_doc_stream(request.user, doc) - or is_individual_draft_author(request.user, doc)): + if not can_edit_docextresources(request.user, doc): permission_denied(request, "You do not have the necessary permissions to view this page.") - old_resources = format_resources(doc.docextresource_set.all()) - if request.method == 'POST': - form = DocExtResourceForm(request.POST) + form = ExtResourceForm(request.POST) if form.is_valid(): - old_resources = sorted(old_resources.splitlines()) - new_resources = sorted(form.cleaned_data['resources']) - if old_resources != new_resources: - doc.docextresource_set.all().delete() - for u in new_resources: - parts = u.split(None, 2) - name = parts[0] - value = parts[1] - display_name = ' '.join(parts[2:]).strip('()') - doc.docextresource_set.create(value=value, name_id=name, display_name=display_name) - new_resources = format_resources(doc.docextresource_set.all()) - e = DocEvent(doc=doc, rev=doc.rev, by=request.user.person, type='changed_document') - e.desc = "Changed document external resources from:\n\n%s\n\nto:\n\n%s" % (old_resources, new_resources) - e.save() - doc.save_with_history([e]) + if update_doc_extresources(doc, form.cleaned_data['resources'], by=request.user.person): messages.success(request,"Document resources updated.") else: messages.info(request,"No change in Document resources.") return redirect('ietf.doc.views_doc.document_main', name=doc.name) else: - form = DocExtResourceForm(initial={'resources': old_resources, }) + form = ExtResourceForm(initial={'resources': doc.docextresource_set.all()}) - info = "Valid tags:

%s" % ', '.join([ o.slug for o in ExtResourceName.objects.all().order_by('slug') ]) + info = "Valid tags:

%s" % ', '.join(form.valid_resource_tags()) # May need to explain the tags more - probably more reason to move to a formset. title = "Additional document resources" return render(request, 'doc/edit_field.html',dict(doc=doc, form=form, title=title, info=info) ) diff --git a/ietf/name/fixtures/names.json b/ietf/name/fixtures/names.json index 6195215f2..c2d4aa03f 100644 --- a/ietf/name/fixtures/names.json +++ b/ietf/name/fixtures/names.json @@ -3507,6 +3507,21 @@ "model": "mailtrigger.mailtrigger", "pk": "doc_expires_soon" }, + { + "fields": { + "cc": [], + "desc": "Recipients when a change to the external resources for a document is requested.", + "to": [ + "doc_ad", + "doc_group_chairs", + "doc_group_delegates", + "doc_shepherd", + "doc_stream_manager" + ] + }, + "model": "mailtrigger.mailtrigger", + "pk": "doc_external_resource_change_requested" + }, { "fields": { "cc": [], diff --git a/ietf/submit/admin.py b/ietf/submit/admin.py index 05ee2480c..703ad5860 100644 --- a/ietf/submit/admin.py +++ b/ietf/submit/admin.py @@ -2,9 +2,12 @@ from django.urls import reverse as urlreverse from django.contrib import admin from django.conf import settings +from django import forms +from ietf.submit.models import (Preapproval, Submission, SubmissionEvent, + SubmissionCheck, SubmissionEmailEvent, SubmissionExtResource) +from ietf.utils.validators import validate_external_resource_value -from ietf.submit.models import Preapproval, Submission, SubmissionEvent, SubmissionCheck, SubmissionEmailEvent class SubmissionAdmin(admin.ModelAdmin): list_display = ['id', 'rev', 'draft_link', 'status_link', 'submission_date',] @@ -48,4 +51,15 @@ admin.site.register(Preapproval, PreapprovalAdmin) class SubmissionEmailEventAdmin(admin.ModelAdmin): list_display = ['id', 'submission', 'time', 'by', 'message', 'desc', ] -admin.site.register(SubmissionEmailEvent, SubmissionEmailEventAdmin) +admin.site.register(SubmissionEmailEvent, SubmissionEmailEventAdmin) + +class SubmissionExtResourceAdminForm(forms.ModelForm): + def clean(self): + validate_external_resource_value(self.cleaned_data['name'],self.cleaned_data['value']) + +class SubmissionExtResourceAdmin(admin.ModelAdmin): + form = SubmissionExtResourceAdminForm + list_display = ['id', 'submission', 'name', 'display_name', 'value',] + search_fields = ['submission__name', 'value', 'display_name', 'name__slug',] + raw_id_fields = ['submission', ] +admin.site.register(SubmissionExtResource, SubmissionExtResourceAdmin) diff --git a/ietf/submit/factories.py b/ietf/submit/factories.py new file mode 100644 index 000000000..143634723 --- /dev/null +++ b/ietf/submit/factories.py @@ -0,0 +1,34 @@ +# Copyright The IETF Trust 2020, All Rights Reserved +# -*- coding: utf-8 -*- + + +import debug # pyflakes:ignore +import factory + +from ietf.doc.factories import draft_name_generator +from ietf.name.models import ExtResourceName +from ietf.submit.models import Submission, SubmissionExtResource +from ietf.utils.accesstoken import generate_random_key + + +class SubmissionExtResourceFactory(factory.DjangoModelFactory): + name = factory.Iterator(ExtResourceName.objects.all()) + value = factory.Faker('url') + submission = factory.SubFactory('ietf.submit.factories.SubmissionFactory') + + class Meta: + model = SubmissionExtResource + +class SubmissionFactory(factory.DjangoModelFactory): + state_id = 'uploaded' + + @factory.lazy_attribute_sequence + def name(self, n): + return draft_name_generator('draft', getattr(self, 'group', None), n) + + @factory.lazy_attribute + def auth_key(self): + return generate_random_key() + + class Meta: + model = Submission diff --git a/ietf/submit/migrations/0008_submissionextresource.py b/ietf/submit/migrations/0008_submissionextresource.py new file mode 100644 index 000000000..be51e2c6d --- /dev/null +++ b/ietf/submit/migrations/0008_submissionextresource.py @@ -0,0 +1,29 @@ +# Generated by Django 2.2.17 on 2021-01-27 12:23 + +from django.db import migrations, models +import django.db.models.deletion +import ietf.utils.models + + +class Migration(migrations.Migration): + + dependencies = [ + ('name', '0020_add_rescheduled_session_name'), + ('submit', '0007_auto_20201109_0439'), + ] + + operations = [ + migrations.CreateModel( + name='SubmissionExtResource', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('display_name', models.CharField(blank=True, default='', max_length=255)), + ('value', models.CharField(max_length=2083)), + ('name', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='name.ExtResourceName')), + ('submission', ietf.utils.models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='external_resources', to='submit.Submission')), + ], + options={ + 'abstract': False, + }, + ), + ] diff --git a/ietf/submit/models.py b/ietf/submit/models.py index 3ecd06c13..4a09c0323 100644 --- a/ietf/submit/models.py +++ b/ietf/submit/models.py @@ -10,7 +10,7 @@ from django.db import models import debug # pyflakes:ignore -from ietf.doc.models import Document +from ietf.doc.models import Document, ExtResource from ietf.person.models import Person from ietf.group.models import Group from ietf.message.models import Message @@ -167,3 +167,6 @@ class SubmissionEmailEvent(SubmissionEvent): class Meta: ordering = ['-time', '-id'] + +class SubmissionExtResource(ExtResource): + submission = ForeignKey(Submission, related_name='external_resources') \ No newline at end of file diff --git a/ietf/submit/resources.py b/ietf/submit/resources.py index 2d5d47308..bf4959f24 100644 --- a/ietf/submit/resources.py +++ b/ietf/submit/resources.py @@ -10,7 +10,7 @@ from tastypie.cache import SimpleCache from ietf import api from ietf.submit.models import ( Preapproval, SubmissionCheck, Submission, - SubmissionEmailEvent, SubmissionEvent ) + SubmissionEmailEvent, SubmissionEvent, SubmissionExtResource ) from ietf.person.resources import PersonResource @@ -139,3 +139,23 @@ class SubmissionEmailEventResource(ModelResource): } api.submit.register(SubmissionEmailEventResource()) + + +from ietf.name.resources import ExtResourceNameResource +class SubmissionExtResourceResource(ModelResource): + name = ToOneField(ExtResourceNameResource, 'name') + submission = ToOneField(SubmissionResource, 'submission') + class Meta: + queryset = SubmissionExtResource.objects.all() + serializer = api.Serializer() + cache = SimpleCache() + resource_name = 'submissionextresource' + ordering = ['id', ] + filtering = { + "id": ALL, + "display_name": ALL, + "value": ALL, + "name": ALL_WITH_RELATIONS, + "submission": ALL_WITH_RELATIONS, + } +api.submit.register(SubmissionExtResourceResource()) diff --git a/ietf/submit/tests.py b/ietf/submit/tests.py index cba53cdf0..d04bc0cdb 100644 --- a/ietf/submit/tests.py +++ b/ietf/submit/tests.py @@ -9,7 +9,7 @@ import os import re import shutil import sys - +import mock from io import StringIO from pyquery import PyQuery @@ -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 +from ietf.doc.utils import create_ballot_if_not_open, can_edit_docextresources from ietf.group.factories import GroupFactory, RoleFactory from ietf.group.models import Group from ietf.group.utils import setup_default_community_list_for_group @@ -34,14 +34,15 @@ from ietf.message.models import Message from ietf.name.models import FormalLanguageName from ietf.person.models import Person from ietf.person.factories import UserFactory, PersonFactory, EmailFactory -from ietf.submit.models import Submission, Preapproval +from ietf.submit.factories import SubmissionFactory, SubmissionExtResourceFactory +from ietf.submit.models import Submission, Preapproval, SubmissionExtResource from ietf.submit.mail import add_submission_email, process_response_email +from ietf.utils.accesstoken import generate_access_token from ietf.utils.mail import outbox, empty_outbox, get_payload_text from ietf.utils.models import VersionInfo from ietf.utils.test_utils import login_testing_unauthorized, TestCase from ietf.utils.draft import Draft - def submission_file(name, rev, group, format, templatename, author=None, email=None, title=None, year=None, ascii=True): # construct appropriate text draft f = io.open(os.path.join(settings.BASE_DIR, "submit", templatename)) @@ -217,7 +218,7 @@ class SubmitTests(TestCase): return status_url, author - def supply_extra_metadata(self, name, status_url, submitter_name, submitter_email, replaces): + def supply_extra_metadata(self, name, status_url, submitter_name, submitter_email, replaces, extresources=None): # check the page r = self.client.get(status_url) q = PyQuery(r.content) @@ -225,19 +226,30 @@ class SubmitTests(TestCase): self.assertEqual(len(post_button), 1) action = post_button.parents("form").find('input[type=hidden][name="action"]').val() - # post submitter info - r = self.client.post(status_url, { + post_data = { "action": action, "submitter-name": submitter_name, "submitter-email": submitter_email, "replaces": replaces, - }) + 'resources': '\n'.join(r.to_form_entry_str() for r in extresources) if extresources else '', + } + + # post submitter info + r = self.client.post(status_url, post_data) if r.status_code == 302: submission = Submission.objects.get(name=name) self.assertEqual(submission.submitter, email.utils.formataddr((submitter_name, submitter_email))) - self.assertEqual(submission.replaces, ",".join(d.name for d in DocAlias.objects.filter(pk__in=replaces.split(",") if replaces else []))) - + self.assertEqual(submission.replaces, + ",".join( + d.name for d in DocAlias.objects.filter( + pk__in=replaces.split(",") if replaces else [] + ) + )) + self.assertCountEqual( + [str(r) for r in submission.external_resources.all()], + [str(r) for r in extresources] if extresources else [], + ) return r def extract_confirmation_url(self, confirmation_email): @@ -457,6 +469,9 @@ class SubmitTests(TestCase): def test_submit_new_replaced_wg_as_author(self): self.submit_new_concluded_wg_as_author('replaced') + def test_submit_new_wg_with_extresources(self): + self.submit_new_draft_with_extresources(group=GroupFactory()) + 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': @@ -655,18 +670,33 @@ class SubmitTests(TestCase): def test_submit_existing_txt_preserve_authors(self): self.submit_existing(["txt"], change_authors=False) + def test_submit_existing_wg_with_extresources(self): + self.submit_existing_with_extresources(group_type='wg') + def test_submit_existing_rg(self): self.submit_existing(["txt"],group_type='rg', stream_type='irtf') + def test_submit_existing_rg_with_extresources(self): + self.submit_existing_with_extresources(group_type='rg', stream_type='irtf') + def test_submit_existing_ag(self): self.submit_existing(["txt"],group_type='ag') + def test_submit_existing_ag_with_extresources(self): + self.submit_existing_with_extresources(group_type='ag') + def test_submit_existing_area(self): self.submit_existing(["txt"],group_type='area') + def test_submit_existing_area_with_extresources(self): + self.submit_existing_with_extresources(group_type='area') + def test_submit_existing_ise(self): self.submit_existing(["txt"],stream_type='ise', group_type='individ') + def test_submit_existing_ise_with_extresources(self): + self.submit_existing_with_extresources(stream_type='ise', group_type='individ') + def test_submit_existing_iab(self): self.submit_existing(["txt"],stream_type='iab', group_type='individ') @@ -731,6 +761,9 @@ class SubmitTests(TestCase): 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 test_submit_existing_iab_with_extresources(self): + self.submit_existing_with_extresources(stream_type='iab', group_type='individ') + def submit_new_individual(self, formats): # submit new -> supply submitter info -> confirm @@ -749,6 +782,8 @@ class SubmitTests(TestCase): r = self.client.get(status_url) self.assertEqual(r.status_code, 200) self.assertContains(r, "The submission is pending email authentication") + self._assert_extresources_in_table(r, []) + self._assert_extresources_form_not_present(r) self.assertEqual(len(outbox), mailbox_before + 1) confirm_email = outbox[-1] @@ -786,6 +821,87 @@ class SubmitTests(TestCase): def test_submit_new_individual_txt_xml(self): self.submit_new_individual(["txt", "xml"]) + def _assert_extresources_in_table(self, response, extresources, th_label=None): + """Assert that external resources are properly shown on the submission_status table""" + q = PyQuery(response.content) + + # Find the that labels the resource list + th = q('th:contains("%s")' % (th_label or 'Submission additional resources')) + self.assertEqual(len(th), 1) + + # Find the element that holds the resource list + td_siblings = th.siblings('td') + self.assertEqual(len(td_siblings), 1) + td = td_siblings.eq(0) + td_html = td.html() + + if extresources: + for res in extresources: + # If the value is present, that's good enough. Don't test the detailed format. + self.assertIn(res.value, td_html, 'Value of resource %s not found' % (res)) + else: + self.assertIn('None', td_html) + + def _assert_extresources_form(self, response, expected_extresources): + """Assert that the form for editing external resources is present and has expected contents""" + q = PyQuery(response.content) + + # The external resources form is currently just a text area. Find it by its ID and check + # that it has the expected contents. + elems = q('form textarea#id_resources') + self.assertEqual(len(elems), 1) + + text_area = elems.eq(0) + contents = text_area.text() + if len(expected_extresources) == 0: + self.assertEqual(contents.strip(), '') + else: + res_strings = [rs for rs in contents.split('\n') if len(rs.strip()) > 0] # ignore empty lines + self.assertCountEqual( + res_strings, + [r.to_form_entry_str() for r in expected_extresources], + ) + + def _assert_extresources_form_not_present(self, response): + q=PyQuery(response.content) + self.assertEqual(len(q('form textarea#id_resources')), 0) + + def _assert_extresource_change_event(self, doc, is_present=True): + """Assert that an external resource change event is (or is not) present for the doc""" + event = doc.latest_event(type='changed_document', desc__contains='Changed document external resources') + if is_present: + self.assertIsNotNone(event, 'External resource change event was not created properly') + else: + self.assertIsNone(event, 'External resource change event was unexpectedly created') + + def submit_new_draft_with_extresources(self, group): + name = 'draft-testing-with-extresources' + + status_url, author = self.do_submission(name, rev='00', group=group) + + # Check that the submission starts with no external resources + r = self.client.get(status_url) + self.assertEqual(r.status_code, 200) + self._assert_extresources_in_table(r, []) + self._assert_extresources_form(r, []) + + resources = [ + SubmissionExtResource(name_id='faq', value='https://faq.example.com/'), + SubmissionExtResource(name_id='wiki', value='https://wiki.example.com', display_name='Test Wiki'), + ] + r = self.supply_extra_metadata(name, status_url, 'Submitter name', 'submitter@example.com', replaces='', + extresources=resources) + self.assertEqual(r.status_code, 302) + status_url = r['Location'] + + r = self.client.get(status_url) + self.assertEqual(r.status_code, 200) + self._assert_extresources_in_table(r, resources) + self._assert_extresources_form_not_present(r) + + def test_submit_new_individual_with_extresources(self): + self.submit_new_draft_with_extresources(group=None) + def submit_new_individual_logged_in(self, formats): # submit new -> supply submitter info -> done @@ -808,6 +924,8 @@ class SubmitTests(TestCase): r = self.client.get(status_url) self.assertEqual(r.status_code, 200) self.assertContains(r, "New version accepted") + self._assert_extresources_in_table(r, []) + self._assert_extresources_form_not_present(r) self.assertEqual(len(outbox), mailbox_before+2) announcement_email = outbox[-2] @@ -821,9 +939,11 @@ class SubmitTests(TestCase): draft = Document.objects.get(docalias__name=name) self.assertEqual(draft.rev, rev) + self.assertEqual(draft.docextresource_set.count(), 0) new_revision = draft.latest_event() self.assertEqual(new_revision.type, "new_revision") self.assertEqual(new_revision.by.name, author.name) + self._assert_extresource_change_event(draft, is_present=False) # Check submission settings self.assertEqual(draft.submission().xml_version, "3" if 'xml' in formats else None) @@ -834,6 +954,43 @@ class SubmitTests(TestCase): def test_submit_new_logged_in_xml(self): self.submit_new_individual_logged_in(["xml"]) + def test_submit_new_logged_in_with_extresources(self): + """Logged-in author of individual draft can set external resources""" + name = 'draft-individual-testing-with-extresources' + author = PersonFactory() + username = author.user.email + + self.client.login(username=username, password=username+'+password') + status_url, author = self.do_submission(name, rev='00', author=author) + + # Check that the submission starts with no external resources + r = self.client.get(status_url) + self.assertEqual(r.status_code, 200) + self._assert_extresources_in_table(r, []) + self._assert_extresources_form(r, []) + + resources = [ + SubmissionExtResource(name_id='faq', value='https://faq.example.com/'), + SubmissionExtResource(name_id='wiki', value='https://wiki.example.com', display_name='Test Wiki'), + ] + r = self.supply_extra_metadata(name, status_url, author.name, username, replaces='', + extresources=resources) + self.assertEqual(r.status_code, 302) + status_url = r['Location'] + + r = self.client.get(status_url) + self.assertEqual(r.status_code, 200) + self._assert_extresources_in_table(r, resources) + self._assert_extresources_form_not_present(r) + + # Check that the draft itself got the resources + draft = Document.objects.get(docalias__name=name) + self.assertCountEqual( + [str(r) for r in draft.docextresource_set.all()], + [str(r) for r in resources], + ) + self._assert_extresource_change_event(draft, is_present=True) + def test_submit_update_individual(self): IndividualDraftFactory(name='draft-ietf-random-thing', states=[('draft','rfc')], other_aliases=['rfc9999',], pages=5) ad=Person.objects.get(user__username='ad') @@ -844,23 +1001,36 @@ class SubmitTests(TestCase): rev = '%02d'%(int(draft.rev)+1) status_url, author = self.do_submission(name,rev) mailbox_before = len(outbox) + replaced_alias = draft.docalias.first() r = self.supply_extra_metadata(name, status_url, "Submitter Name", "author@example.com", replaces=str(replaced_alias.pk)) self.assertEqual(r.status_code, 200) self.assertContains(r, 'cannot replace itself') + self._assert_extresources_in_table(r, []) + self._assert_extresources_form(r, []) + replaced_alias = DocAlias.objects.get(name='draft-ietf-random-thing') r = self.supply_extra_metadata(name, status_url, "Submitter Name", "author@example.com", replaces=str(replaced_alias.pk)) self.assertEqual(r.status_code, 200) self.assertContains(r, 'cannot replace an RFC') + self._assert_extresources_in_table(r, []) + self._assert_extresources_form(r, []) + replaced_alias.document.set_state(State.objects.get(type='draft-iesg',slug='approved')) replaced_alias.document.set_state(State.objects.get(type='draft',slug='active')) r = self.supply_extra_metadata(name, status_url, "Submitter Name", "author@example.com", replaces=str(replaced_alias.pk)) self.assertEqual(r.status_code, 200) self.assertContains(r, 'approved by the IESG and cannot') + self._assert_extresources_in_table(r, []) + self._assert_extresources_form(r, []) + r = self.supply_extra_metadata(name, status_url, "Submitter Name", "author@example.com", replaces='') self.assertEqual(r.status_code, 302) status_url = r["Location"] r = self.client.get(status_url) + self._assert_extresources_in_table(r, []) + self._assert_extresources_form_not_present(r) + self.assertEqual(len(outbox), mailbox_before + 1) confirmation_url = self.extract_confirmation_url(outbox[-1]) self.assertFalse("chairs have been copied" in str(outbox[-1])) @@ -871,10 +1041,55 @@ class SubmitTests(TestCase): draft = Document.objects.get(docalias__name=name) self.assertEqual(draft.rev, rev) self.assertEqual(draft.relateddocument_set.filter(relationship_id='replaces').count(), replaces_count) + self.assertEqual(draft.docextresource_set.count(), 0) # r = self.client.get(urlreverse('ietf.doc.views_search.recent_drafts')) self.assertContains(r, draft.name) self.assertContains(r, draft.title) + self._assert_extresource_change_event(draft, is_present=False) + + def submit_existing_with_extresources(self, group_type, stream_type='ietf'): + """Submit a draft with external resources + + Unlike some other tests in this module, does not confirm draft if this would be required. + """ + orig_draft = DocumentFactory( + type_id='draft', + group=GroupFactory(type_id=group_type) if group_type else None, + stream_id=stream_type, + ) # type: Document + name = orig_draft.name + group = orig_draft.group + new_rev = '%02d' % (int(orig_draft.rev) + 1) + author = PersonFactory() # type: Person + DocumentAuthor.objects.create(person=author, document=orig_draft) + orig_draft.docextresource_set.create(name_id='faq', value='https://faq.example.com/') + orig_draft.docextresource_set.create(name_id='wiki', value='https://wiki.example.com', display_name='Test Wiki') + orig_extresources = list(orig_draft.docextresource_set.all()) + + status_url, _ = self.do_submission(name=name, rev=new_rev, author=author, group=group) + + # Make sure the submission status inherits the original draft's external resources + r = self.client.get(status_url) + self.assertEqual(r.status_code, 200) + self._assert_extresources_in_table(r, orig_extresources) + self._assert_extresources_form(r, orig_extresources) + + # Update with an empty set of resources + r = self.supply_extra_metadata(orig_draft.name, status_url, author.name, author.user.email, + replaces='', extresources=[]) + self.assertEqual(r.status_code, 302) + status_url = r['Location'] + + # Should now see the submission's resources and the set currently assigned to the document + r = self.client.get(status_url) + self.assertEqual(r.status_code, 200) + self._assert_extresources_in_table(r, []) + self._assert_extresources_in_table(r, orig_extresources, 'Current document additional resources') + self._assert_extresources_form_not_present(r) + + def test_submit_update_individual_with_extresources(self): + self.submit_existing_with_extresources(group_type=None, stream_type='ietf') 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""" @@ -1085,6 +1300,7 @@ class SubmitTests(TestCase): draft = Document.objects.get(docalias__name=name) self.assertEqual(draft.rev, rev) + self.assertEqual(draft.docextresource_set.count(), 0) def test_search_for_submission_and_edit_as_secretariat(self): # submit -> edit @@ -1609,7 +1825,186 @@ class SubmitTests(TestCase): 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) + def do_approval_with_extresources_test(self, submission, url, action, permitted): + """Helper for submission approval external resource testing + + Only intended to test the permissions handling for external resources. Assumes + the permissions defined by can_edit_docextresources() are tested separately. + + Checks that the submission's external_resources are added / not added based on + permitted. Also checks that a suggestion email is not sent / sent. + """ + mailbox_before = len(outbox) + with mock.patch('ietf.submit.utils.can_edit_docextresources', return_value=permitted,) as mocked_permission_check: + r = self.client.post(url, dict(action=action)) + self.assertEqual(r.status_code, 302) + self.assertTrue(mocked_permission_check.called, 'Permissions were not checked') + draft = Document.objects.get(name=submission.name) + self.assertCountEqual( + [str(r) for r in draft.docextresource_set.all()], + [str(r) for r in submission.external_resources.all()] if permitted else [], + ) + + expected_other_emails = 1 # confirmation / approval email + if permitted: + self._assert_extresource_change_event(draft, is_present=True) + self.assertEqual(len(outbox), mailbox_before + expected_other_emails) + else: + self._assert_extresource_change_event(draft, is_present=False) + self.assertEqual(len(outbox), mailbox_before + 1 + expected_other_emails) + new_mail = outbox[mailbox_before:] + subject = 'External resource change requested for %s' % submission.name + suggestion_email = [m for m in new_mail + if m['Subject'] == subject] + self.assertEqual(len(suggestion_email), 1) + body = str(suggestion_email[0]) + for res in submission.external_resources.all(): + self.assertIn(res.to_form_entry_str(), body) + + def group_approve_with_extresources(self, permitted): + group = GroupFactory() + # someone to be notified of resource suggestion when permission not granted + RoleFactory(group=group, person=PersonFactory(), name_id='chair') + submission = SubmissionFactory(state_id='grp-appr', group=group) # type: Submission + SubmissionExtResourceFactory(submission=submission) + + # use secretary user to ensure we have permission to approve + self.client.login(username='secretary', password='secretary+password') + url = urlreverse('ietf.submit.views.submission_status', + kwargs=dict(submission_id=submission.pk)) + self.do_approval_with_extresources_test(submission, url, 'approve', permitted) + + def test_group_approve_with_extresources(self): + """Doc external resources should be updated when approved by group""" + self.group_approve_with_extresources(permitted=True) + self.group_approve_with_extresources(permitted=False) + + def confirm_with_extresources(self, state, permitted): + group = GroupFactory() + # someone to be notified of resource suggestion when permission not granted + RoleFactory(group=group, person=PersonFactory(), name_id='chair') + submission = SubmissionFactory(state_id=state, group=group) # type: Submission + SubmissionExtResourceFactory(submission=submission) + + url = urlreverse( + 'ietf.submit.views.confirm_submission', + kwargs=dict(submission_id=submission.pk, + auth_token=generate_access_token(submission.auth_key)) + ) + + self.do_approval_with_extresources_test(submission, url, 'confirm', permitted) + + def test_confirm_with_extresources(self): + """Doc external resources should be updated when confirmed by author""" + self.confirm_with_extresources('aut-appr', permitted=True) + self.confirm_with_extresources('aut-appr', permitted=False) + self.confirm_with_extresources('auth', permitted=True) + self.confirm_with_extresources('auth', permitted=False) + + def test_can_edit_docextresources(self): + """The can_edit_docextresources method should authorize correctly + + Tests that is_authorized_in_doc_stream() being True grants access, but does not + do detailed testing of that method. + """ + author = PersonFactory() + plain = PersonFactory() + secretary = Person.objects.get(user__username='secretary') + ad = Person.objects.get(user__username='ad') + wg_chair = PersonFactory() + wg = GroupFactory() + RoleFactory(person=wg_chair, group=wg, name_id='chair') + + wg_doc = WgDraftFactory(authors=[author], group=wg) + self.assertFalse(can_edit_docextresources(author.user, wg_doc)) + self.assertTrue(can_edit_docextresources(secretary.user, wg_doc)) + self.assertTrue(can_edit_docextresources(ad.user, wg_doc)) + self.assertTrue(can_edit_docextresources(wg_chair.user, wg_doc)) + self.assertFalse(can_edit_docextresources(plain.user, wg_doc)) + with mock.patch('ietf.doc.utils.is_authorized_in_doc_stream', return_value=True): + self.assertTrue(can_edit_docextresources(plain.user, wg_doc)) + + individ_doc = IndividualDraftFactory(authors=[author]) + self.assertTrue(can_edit_docextresources(author.user, individ_doc)) + self.assertTrue(can_edit_docextresources(secretary.user, individ_doc)) + self.assertTrue(can_edit_docextresources(ad.user, individ_doc)) + self.assertFalse(can_edit_docextresources(wg_chair.user, individ_doc)) + self.assertFalse(can_edit_docextresources(plain.user, individ_doc)) + with mock.patch('ietf.doc.utils.is_authorized_in_doc_stream', return_value=True): + self.assertTrue(can_edit_docextresources(plain.user, individ_doc)) + + def test_forcepost_with_extresources(self): + # state needs to be one that has 'posted' as a next state + submission = SubmissionFactory(state_id='grp-appr') # type: Submission + SubmissionExtResourceFactory(submission=submission) + + url = urlreverse( + 'ietf.submit.views.submission_status', + kwargs=dict(submission_id=submission.pk), + ) + + self.client.login(username='secretary', password='secretary+password') + r = self.client.post(url, dict(action='forcepost')) + self.assertEqual(r.status_code, 302) + draft = Document.objects.get(name=submission.name) + self._assert_extresource_change_event(draft, is_present=True) + self.assertCountEqual( + [str(r) for r in draft.docextresource_set.all()], + [str(r) for r in submission.external_resources.all()], + ) + + def test_submission_status_labels_extresource_changes(self): + """Added or removed labels should be present for changed external resources""" + draft = WgDraftFactory(rev='00') + draft.docextresource_set.create( + name_id='faq', + value='https://example.com/faq-removed', + display_name='Resource to be removed', + ) + draft.docextresource_set.create( + name_id='faq', + value='https://example.com/faq-kept', + display_name='Resource to be kept', + ) + + submission = SubmissionFactory(name=draft.name, rev='01') + submission.external_resources.create( + name_id='faq', + value='https://example.com/faq-kept', + display_name='Resource to be kept', + ) + submission.external_resources.create( + name_id='faq', + value='https://example.com/faq-added', + display_name='Resource to be added', + ) + + url = urlreverse('ietf.submit.views.submission_status', + kwargs=dict(submission_id=submission.pk)) + r = self.client.get(url) + self.assertEqual(r.status_code, 200) + + q = PyQuery(r.content) + + # The removed resource should appear once (for the doc current value), tagged as removed + removed_div = q('td>div:contains("Resource to be removed")') + self.assertEqual(len(removed_div), 1) + self.assertEqual(len(removed_div('span.label:contains("Removed")')), 1) + self.assertEqual(len(removed_div('span.label:contains("New")')), 0) + + # The added resource should appear once (for the submission), tagged as new + added_div = q('td>div:contains("Resource to be added")') + self.assertEqual(len(added_div), 1) + self.assertEqual(len(added_div('span.label:contains("Removed")')), 0) + self.assertEqual(len(added_div('span.label:contains("New")')), 1) + + # The kept resource should appear twice (once for the doc, once for the submission), with no tag + kept_div = q('td>div:contains("Resource to be kept")') + self.assertEqual(len(kept_div), 2) + self.assertEqual(len(kept_div('span.label:contains("Removed")')), 0) + self.assertEqual(len(kept_div('span.label:contains("New")')), 0) + class ApprovalsTestCase(TestCase): def test_approvals(self): RoleFactory(name_id='chair', @@ -2380,6 +2775,33 @@ class ApiSubmitTests(TestCase): expected = "Document date must be within 3 days of submission date" self.assertContains(r, expected, status_code=400) + def test_api_submit_keeps_extresources(self): + """API submit should not disturb doc external resources + + Tests that the submission inherits the existing doc's docextresource_set. + Relies on separate testing that Submission external_resources will be + handled appropriately. + """ + draft = WgDraftFactory() + + # add an external resource + self.assertEqual(draft.docextresource_set.count(), 0) + extres = draft.docextresource_set.create( + name_id='faq', + display_name='this is a display name', + value='https://example.com/faq-for-test.html', + ) + + r, _, __ = self.do_post_submission('01', name=draft.name) + self.assertEqual(r.status_code, 200) + # draft = Document.objects.get(pk=draft.pk) # update the draft + sub = Submission.objects.get(name=draft.name) + self.assertEqual( + [str(r) for r in sub.external_resources.all()], + [str(extres)], + ) + + class RefsTests(TestCase): def test_draft_refs_identification(self): diff --git a/ietf/submit/utils.py b/ietf/submit/utils.py index 2c04fc230..27a14420f 100644 --- a/ietf/submit/utils.py +++ b/ietf/submit/utils.py @@ -23,18 +23,19 @@ import debug # pyflakes:ignore from ietf.doc.models import ( Document, State, DocAlias, DocEvent, SubmissionDocEvent, DocumentAuthor, AddedMessageEvent ) from ietf.doc.models import NewRevisionDocEvent -from ietf.doc.models import RelatedDocument, DocRelationshipName +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 -from ietf.doc.mails import send_review_possibly_replaces_request +from ietf.doc.utils import set_replaces_for_document, prettify_std_name, update_doc_extresources, can_edit_docextresources +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 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, - announce_new_wg_00, send_approval_request, send_submission_confirmation ) -from ietf.submit.models import Submission, SubmissionEvent, Preapproval, DraftSubmissionStateName, SubmissionCheck + send_approval_request, send_submission_confirmation, announce_new_wg_00 ) +from ietf.submit.models import ( Submission, SubmissionEvent, Preapproval, DraftSubmissionStateName, + SubmissionCheck, SubmissionExtResource ) from ietf.utils import log from ietf.utils.accesstoken import generate_random_key from ietf.utils.draft import Draft @@ -408,10 +409,24 @@ def post_submission(request, submission, approved_doc_desc, approved_subm_desc): log.log(f"{submission.name}: moved files") new_replaces, new_possibly_replaces = update_replaces_from_submission(request, submission, draft) - update_name_contains_indexes_with_new_doc(draft) log.log(f"{submission.name}: updated replaces and indexes") + # See whether a change to external resources is requested. Test for equality of sets is ugly, + # but works. + draft_resources = '\n'.join(sorted(str(r) for r in draft.docextresource_set.all())) + submission_resources = '\n'.join(sorted(str(r) for r in submission.external_resources.all())) + if draft_resources != submission_resources: + if can_edit_docextresources(request.user, draft): + update_docextresources_from_submission(request, submission, draft) + log.log(f"{submission.name}: updated external resources") + else: + send_external_resource_change_request(request, + draft, + submitter_info, + submission.external_resources.all()) + log.log(f"{submission.name}: sent email suggesting external resources") + announce_to_lists(request, submission) if submission.group and submission.group.type_id == 'wg' and draft.rev == '00': announce_new_wg_00(request, submission) @@ -482,6 +497,12 @@ def update_replaces_from_submission(request, submission, draft): return approved, suggested +def update_docextresources_from_submission(request, submission, draft): + doc_resources = [DocExtResource.from_sibling_class(res) + for res in submission.external_resources.all()] + by = request.user.person if request.user.is_authenticated else Person.objects.get(name='(System)') + update_doc_extresources(draft, doc_resources, by) + def get_person_from_name_email(name, email): # try email if email and (email.startswith('unknown-email-') or is_valid_email(email)): @@ -781,6 +802,7 @@ def fill_in_submission(form, submission, authors, abstract, file_size): submission.save() submission.formal_languages.set(FormalLanguageName.objects.filter(slug__in=form.parsed_draft.get_formal_languages())) + set_extresources_from_existing_draft(submission) def apply_checkers(submission, file_name): # run submission checkers @@ -814,7 +836,7 @@ def accept_submission_requires_prev_auth_approval(submission): 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 ( @@ -825,7 +847,7 @@ def accept_submission_requires_group_approval(submission): 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() @@ -940,5 +962,24 @@ def accept_submission(request, submission, autopost=False): 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 + + return address_list + +def set_extresources_from_existing_draft(submission): + """Replace a submission's external resources with values from previous revision + + If there is no previous revision, clears the external resource list for the submission. + """ + doc = submission.existing_document() + if doc: + update_submission_external_resources( + submission, + [SubmissionExtResource.from_sibling_class(res) + for res in doc.docextresource_set.all()] + ) + +def update_submission_external_resources(submission, new_resources): + submission.external_resources.all().delete() + for new_res in new_resources: + new_res.submission = submission + new_res.save() diff --git a/ietf/submit/views.py b/ietf/submit/views.py index c939603f2..4a61a4eeb 100644 --- a/ietf/submit/views.py +++ b/ietf/submit/views.py @@ -6,7 +6,7 @@ import re import base64 import datetime -from typing import Optional # pyflakes:ignore +from typing import Optional, cast # pyflakes:ignore from django.conf import settings from django.contrib import messages @@ -22,6 +22,7 @@ from django.views.decorators.csrf import csrf_exempt import debug # pyflakes:ignore from ietf.doc.models import Document, DocAlias, AddedMessageEvent +from ietf.doc.forms import ExtResourceForm from ietf.group.models import Group from ietf.group.utils import group_features_group_filter from ietf.ietfauth.utils import has_role, role_required @@ -31,14 +32,14 @@ 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.models import (Submission, Preapproval, +from ietf.submit.models import (Submission, Preapproval, SubmissionExtResource, 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, save_files, check_submission_revision_consistency, accept_submission, accept_submission_requires_group_approval, - accept_submission_requires_prev_auth_approval) + accept_submission_requires_prev_auth_approval, update_submission_external_resources ) from ietf.stats.utils import clean_country_name from ietf.utils.accesstoken import generate_access_token from ietf.utils.log import log @@ -267,6 +268,7 @@ def submission_status(request, submission_id, access_token=None): confirmation_list = [ "%s <%s>" % parseaddr(a) for a in addresses ] message = None + if submission.state_id == "cancel": message = ('error', 'This submission has been cancelled, modification is no longer possible.') elif submission.state_id == "auth": @@ -278,8 +280,58 @@ def submission_status(request, submission_id, access_token=None): 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)) + existing_doc = submission.existing_document() + + # Sort out external resources + external_resources = [ + dict(res=r, added=False) + for r in submission.external_resources.order_by('name__slug', 'value', 'display_name') + ] + + # Show comparison of resources with current doc resources. If not posted or canceled, + # determine which resources were added / removed. In the output, submission resources + # will be marked as "new" if they were not present on the existing document. Document + # resources will be marked as "removed" if they are not present in the submission. + # + # To classify the resources, start by assuming that every submission resource already + # existed (the "added=False" above) and that every existing document resource was + # removed (the "removed=True" below). Then check every submission resource for a + # matching resource on the existing document that is still marked as "removed". If one + # exists, change the existing resource to "not removed" and leave the submission resource + # as "not added." If there is no matching removed resource, then mark the submission + # resource as "added." + # + show_resource_changes = submission.state_id not in ['posted', 'cancel'] + doc_external_resources = [dict(res=r, removed=True) + for r in existing_doc.docextresource_set.all()] if existing_doc else [] + if show_resource_changes: + for item in external_resources: + er = cast(SubmissionExtResource, item['res']) # cast to help type checker with the dict typing + # get first matching resource still marked as 'removed' from previous rev resources + existing_item = next( + filter( + lambda r: (r['removed'] + and er.name == r['res'].name + and er.value == r['res'].value + and er.display_name == r['res'].display_name), + doc_external_resources + ), + None + ) # type: ignore + if existing_item is None: + item['added'] = True + else: + existing_item['removed'] = False + doc_external_resources.sort( + key=lambda d: (d['res'].name.slug, d['res'].value, d['res'].display_name) + ) + submitter_form = SubmitterForm(initial=submission.submitter_parsed(), prefix="submitter") replaces_form = ReplacesForm(name=submission.name,initial=DocAlias.objects.filter(name__in=submission.replaces.split(","))) + extresources_form = ExtResourceForm( + initial=dict(resources=[er['res'] for er in external_resources]), + extresource_model=SubmissionExtResource, + ) if request.method == 'POST': action = request.POST.get('action') @@ -289,13 +341,23 @@ def submission_status(request, submission_id, access_token=None): submitter_form = SubmitterForm(request.POST, prefix="submitter") replaces_form = ReplacesForm(request.POST, name=submission.name) - validations = [submitter_form.is_valid(), replaces_form.is_valid()] + extresources_form = ExtResourceForm( + request.POST, extresource_model=SubmissionExtResource + ) + validations = [ + submitter_form.is_valid(), + replaces_form.is_valid(), + extresources_form.is_valid(), + ] if all(validations): submission.submitter = submitter_form.cleaned_line() replaces = replaces_form.cleaned_data.get("replaces", []) submission.replaces = ",".join(o.name for o in replaces) + extresources = extresources_form.cleaned_data.get('resources', []) + update_submission_external_resources(submission, extresources) + approvals_received = submitter_form.cleaned_data['approvals_received'] if submission.rev == '00' and submission.group and not submission.group.is_active: @@ -388,6 +450,12 @@ def submission_status(request, submission_id, access_token=None): 'passes_checks': passes_checks, 'submitter_form': submitter_form, 'replaces_form': replaces_form, + 'extresources_form': extresources_form, + 'external_resources': { + 'current': external_resources, # dict with 'res' and 'added' as keys + 'previous': doc_external_resources, # dict with 'res' and 'removed' as keys + 'show_changes': show_resource_changes, + }, 'message': message, 'can_edit': can_edit, 'can_force_post': can_force_post, diff --git a/ietf/templates/doc/mail/external_resource_change_request.txt b/ietf/templates/doc/mail/external_resource_change_request.txt new file mode 100644 index 000000000..5d0a12c35 --- /dev/null +++ b/ietf/templates/doc/mail/external_resource_change_request.txt @@ -0,0 +1,18 @@ +{% autoescape off %}{% with current_resources=doc.docextresource_set.all %} +{{ submitter_info }} has requested changes to the additional resources for +{{ doc }}: + +{% if current_resources %}Currently, the additional resources for this document are: +{% for res in current_resources %} +{{ res.to_form_entry_str }}{% endfor %}{% else %}Currently, this document has no additional resources.{% endif %} + +{% if requested_resources %}It is requested that the additional resources be changed to: +{% for res in requested_resources %} +{{ res.to_form_entry_str }}{% endfor %}{% else %}It is requested that the additional resources be removed.{% endif %} + +If this is an acceptable change, please visit + + {{ doc_url }} + +and edit the additional resources accordingly. +{% endwith %}{% endautoescape %} diff --git a/ietf/templates/submit/extresources_form.html b/ietf/templates/submit/extresources_form.html new file mode 100644 index 000000000..beaf3e668 --- /dev/null +++ b/ietf/templates/submit/extresources_form.html @@ -0,0 +1,4 @@ +{% load bootstrap3 %} +

Additional resource information

+{% bootstrap_form extresources_form %} +

Valid tags: {{ extresources_form.valid_resource_tags|join:", " }}

diff --git a/ietf/templates/submit/submission_status.html b/ietf/templates/submit/submission_status.html index 2037244a5..66dbf352c 100644 --- a/ietf/templates/submit/submission_status.html +++ b/ietf/templates/submit/submission_status.html @@ -31,7 +31,7 @@

{% endif %} - {% if submitter_form.errors or replaces_form.errors %} + {% if submitter_form.errors or replaces_form.errors or extresources_form.errors %}

Please fix errors in the form below.

{% endif %} @@ -269,6 +269,42 @@ {% if errors.formal_languages %}

{{ errors.formal_languages }}

{% endif %} + + + Submission additional resources + + {% for r in external_resources.current %}{% with res=r.res added=r.added %} +
+ {{ res.name.name }}: {{ res.value }} + {% if res.display_name %} (as "{{ res.display_name }}") {% endif %} + {% if external_resources.show_changes and added %} + New + {% endif %} +
+ {% endwith %} + {% empty %} + None + {% endfor %} + + + + {% if external_resources.show_changes %} + Current document additional resources + + {% for r in external_resources.previous %}{% with res=r.res removed=r.removed %} +
+ {{ res.name.name }}: {{ res.value }} + {% if res.display_name %} (as "{{ res.display_name }}") {% endif %} + {% if removed %} + Removed + {% endif %} +
+ {% endwith %} + {% empty %} + None + {% endfor %} + + {% endif %} {% if can_edit %} @@ -287,6 +323,7 @@ {% csrf_token %} {% include "submit/submitter_form.html" %} {% include "submit/replaces_form.html" %} + {% include "submit/extresources_form.html" %}

Post submission