From b08110b8381794b521b0f79ba5e57db5fc82d290 Mon Sep 17 00:00:00 2001
From: Jennifer Richards <jennifer@painless-security.com>
Date: Mon, 19 Apr 2021 20:21:19 +0000
Subject: [PATCH] Allow external resources to be set/suggested during
submission process. Fixes #3068. Commit ready for merge. - Legacy-Id: 18960
---
ietf/doc/forms.py | 81 +++-
ietf/doc/mails.py | 17 +
ietf/doc/models.py | 46 +-
ietf/doc/utils.py | 26 +-
ietf/doc/views_draft.py | 78 +---
ietf/name/fixtures/names.json | 15 +
ietf/submit/admin.py | 18 +-
ietf/submit/factories.py | 34 ++
.../migrations/0008_submissionextresource.py | 29 ++
ietf/submit/models.py | 5 +-
ietf/submit/resources.py | 22 +-
ietf/submit/tests.py | 442 +++++++++++++++++-
ietf/submit/utils.py | 61 ++-
ietf/submit/views.py | 76 ++-
.../mail/external_resource_change_request.txt | 18 +
ietf/templates/submit/extresources_form.html | 4 +
ietf/templates/submit/submission_status.html | 39 +-
17 files changed, 909 insertions(+), 102 deletions(-)
create mode 100644 ietf/submit/factories.py
create mode 100644 ietf/submit/migrations/0008_submissionextresource.py
create mode 100644 ietf/templates/doc/mail/external_resource_change_request.txt
create mode 100644 ietf/templates/submit/extresources_form.html
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 "<tag> <value>" or "<tag> <value> (<display name>)" 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_<field> 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 "<tag> <value>[ (<display name>)]"
+ 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:<br><br> %s" % ', '.join([ o.slug for o in ExtResourceName.objects.all().order_by('slug') ])
+ info = "Valid tags:<br><br> %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 <th> that labels the resource list
+ th = q('th:contains("%s")' % (th_label or 'Submission additional resources'))
+ self.assertEqual(len(th), 1)
+
+ # Find the <td> 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 %}
+<h3>Additional resource information</h3>
+{% bootstrap_form extresources_form %}
+<p><b>Valid tags:</b> {{ extresources_form.valid_resource_tags|join:", " }}</p>
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 @@
</p>
{% endif %}
- {% if submitter_form.errors or replaces_form.errors %}
+ {% if submitter_form.errors or replaces_form.errors or extresources_form.errors %}
<p class="alert alert-danger">Please fix errors in the form below.</p>
{% endif %}
@@ -269,6 +269,42 @@
{% if errors.formal_languages %}<p class="text-danger bg-danger"><b>{{ errors.formal_languages }}</b></p>{% endif %}
</td>
</tr>
+
+ <tr>
+ <th>Submission additional resources</th>
+ <td>
+ {% for r in external_resources.current %}{% with res=r.res added=r.added %}
+ <div>
+ {{ res.name.name }}: {{ res.value }}
+ {% if res.display_name %} (as "{{ res.display_name }}") {% endif %}
+ {% if external_resources.show_changes and added %}
+ <span class="label label-success">New</span>
+ {% endif %}
+ </div>
+ {% endwith %}
+ {% empty %}
+ None
+ {% endfor %}
+ </td>
+ </tr>
+
+ {% if external_resources.show_changes %}<tr>
+ <th>Current document additional resources</th>
+ <td>
+ {% for r in external_resources.previous %}{% with res=r.res removed=r.removed %}
+ <div>
+ {{ res.name.name }}: {{ res.value }}
+ {% if res.display_name %} (as "{{ res.display_name }}") {% endif %}
+ {% if removed %}
+ <span class="label label-warning">Removed</span>
+ {% endif %}
+ </div>
+ {% endwith %}
+ {% empty %}
+ None
+ {% endfor %}
+ </td>
+ </tr>{% endif %}
</table>
{% if can_edit %}
@@ -287,6 +323,7 @@
{% csrf_token %}
{% include "submit/submitter_form.html" %}
{% include "submit/replaces_form.html" %}
+ {% include "submit/extresources_form.html" %}
<input type="hidden" name="action" value="autopost">
<h3>Post submission</h3>
<button class="btn btn-primary" type="submit">Post submission</button>