Allow external resources to be set/suggested during submission process. Fixes ietf-tools#3068. Commit ready for merge.

 - Legacy-Id: 18960

Author: jennifer-richards

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 @@ def clean_notify(self):
         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 @@ def clean(self):
             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)

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),)

ietf/doc/models.py

@@ -922,15 +922,57 @@ 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
     def __str__(self):
         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")

ietf/doc/utils.py

@@ -29,7 +29,7 @@
 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

ietf/doc/views_draft.py

@@ -13,7 +13,6 @@
 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.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) )

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": [],