ci: merge main to release

ietf/doc/migrations/0022_remove_dochistory_internal_comments_and_more.py

@@ -0,0 +1,21 @@
+# Generated by Django 4.2.15 on 2024-08-16 16:43
+
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("doc", "0021_narrativeminutes"),
+    ]
+
+    operations = [
+        migrations.RemoveField(
+            model_name="dochistory",
+            name="internal_comments",
+        ),
+        migrations.RemoveField(
+            model_name="document",
+            name="internal_comments",
+        ),
+    ]

ietf/doc/models.py

@@ -122,7 +122,6 @@ class DocumentInfo(models.Model):
     external_url = models.URLField(blank=True)
     uploaded_filename = models.TextField(blank=True)
     note = models.TextField(blank=True)
-    internal_comments = models.TextField(blank=True)
     rfc_number = models.PositiveIntegerField(blank=True, null=True)  # only valid for type="rfc"
 
     def file_extension(self):

ietf/doc/resources.py

@@ -130,7 +130,6 @@ class Meta:
             "external_url": ALL,
             "uploaded_filename": ALL,
             "note": ALL,
-            "internal_comments": ALL,
             "name": ALL,
             "type": ALL_WITH_RELATIONS,
             "stream": ALL_WITH_RELATIONS,
@@ -247,7 +246,6 @@ class Meta:
             "external_url": ALL,
             "uploaded_filename": ALL,
             "note": ALL,
-            "internal_comments": ALL,
             "name": ALL,
             "type": ALL_WITH_RELATIONS,
             "stream": ALL_WITH_RELATIONS,

ietf/doc/templatetags/ietf_filters.py

@@ -856,10 +856,10 @@ def badgeify(blob):
     Add an appropriate bootstrap badge around "text", based on its contents.
     """
     config = [
-        (r"rejected|not ready", "danger", "x-lg"),
+        (r"rejected|not ready|serious issues", "danger", "x-lg"),
         (r"complete|accepted|ready", "success", ""),
         (r"has nits|almost ready", "info", "info-lg"),
-        (r"has issues", "warning", "exclamation-lg"),
+        (r"has issues|on the right track", "warning", "exclamation-lg"),
         (r"assigned", "info", "person-plus-fill"),
         (r"will not review|overtaken by events|withdrawn", "secondary", "dash-lg"),
         (r"no response", "warning", "question-lg"),

ietf/doc/tests_ballot.py

@@ -20,6 +20,7 @@
                                 BallotPositionDocEventFactory, BallotDocEventFactory, IRSGBallotDocEventFactory)
 from ietf.doc.templatetags.ietf_filters import can_defer
 from ietf.doc.utils import create_ballot_if_not_open
+from ietf.doc.views_ballot import parse_ballot_edit_return_point
 from ietf.doc.views_doc import document_ballot_content
 from ietf.group.models import Group, Role
 from ietf.group.factories import GroupFactory, RoleFactory, ReviewTeamFactory
@@ -1451,3 +1452,32 @@ def test_document_ballot_content_without_send_email_values(self):
         self._assertBallotMessage(q, balloters[0], 'No discuss send log available')
         self._assertBallotMessage(q, balloters[1], 'No comment send log available')
         self._assertBallotMessage(q, old_balloter, 'No ballot position send log available')
+
+class ReturnToUrlTests(TestCase):
+    def test_invalid_return_to_url(self):
+        self.assertRaises(
+            Exception,
+            lambda: parse_ballot_edit_return_point('/doc/', 'draft-ietf-opsawg-ipfix-tcpo-v6eh', '998718'),
+        )
+        self.assertRaises(
+            Exception,
+            lambda: parse_ballot_edit_return_point('/a-route-that-does-not-exist/', 'draft-ietf-opsawg-ipfix-tcpo-v6eh', '998718'),
+        )
+        self.assertRaises(
+            Exception,
+            lambda: parse_ballot_edit_return_point('https://example.com/phishing', 'draft-ietf-opsawg-ipfix-tcpo-v6eh', '998718'),
+        )
+
+    def test_valid_default_return_to_url(self):
+        self.assertEqual(parse_ballot_edit_return_point(
+            None,
+            'draft-ietf-opsawg-ipfix-tcpo-v6eh',
+            '998718'
+        ), '/doc/draft-ietf-opsawg-ipfix-tcpo-v6eh/ballot/998718/')
+
+    def test_valid_return_to_url(self):
+        self.assertEqual(parse_ballot_edit_return_point(
+            '/doc/draft-ietf-opsawg-ipfix-tcpo-v6eh/ballot/998718/',
+            'draft-ietf-opsawg-ipfix-tcpo-v6eh',
+            '998718'
+        ), '/doc/draft-ietf-opsawg-ipfix-tcpo-v6eh/ballot/998718/')

ietf/doc/views_ballot.py

@@ -8,13 +8,14 @@
 
 from django import forms
 from django.conf import settings
-from django.http import HttpResponse, HttpResponseRedirect, Http404
+from django.http import HttpResponse, HttpResponseRedirect, Http404, HttpResponseBadRequest
 from django.shortcuts import render, get_object_or_404, redirect
 from django.template.defaultfilters import striptags
 from django.template.loader import render_to_string
 from django.urls import reverse as urlreverse
 from django.views.decorators.csrf import csrf_exempt
 from django.utils.html import escape
+from urllib.parse import urlencode as urllib_urlencode
 
 import debug                            # pyflakes:ignore
 
@@ -39,6 +40,7 @@
 from ietf.name.models import BallotPositionName, DocTypeName
 from ietf.person.models import Person
 from ietf.utils.fields import ModelMultipleChoiceField
+from ietf.utils.http import validate_return_to_path
 from ietf.utils.mail import send_mail_text, send_mail_preformatted
 from ietf.utils.decorators import require_api_key
 from ietf.utils.response import permission_denied
@@ -185,11 +187,11 @@ def edit_position(request, name, ballot_id):
 
     balloter = login = request.user.person
 
-    if 'ballot_edit_return_point' in request.session:
-        return_to_url = request.session['ballot_edit_return_point']
-    else:
-        return_to_url = urlreverse("ietf.doc.views_doc.document_ballot", kwargs=dict(name=doc.name, ballot_id=ballot_id))
-
+    try:
+        return_to_url = parse_ballot_edit_return_point(request.GET.get('ballot_edit_return_point'), doc.name, ballot_id)
+    except ValueError:
+        return HttpResponseBadRequest('ballot_edit_return_point is invalid')
+    
     # if we're in the Secretariat, we can select a balloter to act as stand-in for
     if has_role(request.user, "Secretariat"):
         balloter_id = request.GET.get('balloter')
@@ -209,9 +211,14 @@ def edit_position(request, name, ballot_id):
             save_position(form, doc, ballot, balloter, login, send_mail)
 
             if send_mail:
-                qstr=""
+                query = {}
                 if request.GET.get('balloter'):
-                    qstr += "?balloter=%s" % request.GET.get('balloter')
+                    query['balloter'] = request.GET.get('balloter')
+                if request.GET.get('ballot_edit_return_point'):
+                    query['ballot_edit_return_point'] = request.GET.get('ballot_edit_return_point')
+                qstr = ""
+                if len(query) > 0:
+                    qstr = "?" + urllib_urlencode(query, safe='/')
                 return HttpResponseRedirect(urlreverse('ietf.doc.views_ballot.send_ballot_comment', kwargs=dict(name=doc.name, ballot_id=ballot_id)) + qstr)
             elif request.POST.get("Defer") and doc.stream.slug != "irtf":
                 return redirect('ietf.doc.views_ballot.defer_ballot', name=doc)
@@ -337,11 +344,11 @@ def send_ballot_comment(request, name, ballot_id):
 
     balloter = request.user.person
 
-    if 'ballot_edit_return_point' in request.session:
-        return_to_url = request.session['ballot_edit_return_point']
-    else:
-        return_to_url = urlreverse("ietf.doc.views_doc.document_ballot", kwargs=dict(name=doc.name, ballot_id=ballot_id))
-
+    try:
+        return_to_url = parse_ballot_edit_return_point(request.GET.get('ballot_edit_return_point'), doc.name, ballot_id)
+    except ValueError:
+        return HttpResponseBadRequest('ballot_edit_return_point is invalid')
+    
     if 'HTTP_REFERER' in request.META:
         back_url = request.META['HTTP_REFERER']
     else:
@@ -1302,3 +1309,15 @@ def rsab_ballot_status(request):
     # Possible TODO: add a menu item to show this? Maybe only if you're in rsab or an rswg chair?
     # There will be so few of these that the general community would follow them from the rswg docs page.
     # Maybe the view isn't actually needed at all...
+
+
+def parse_ballot_edit_return_point(path, doc_name, ballot_id):
+    get_default_path = lambda: urlreverse("ietf.doc.views_doc.document_ballot", kwargs=dict(name=doc_name, ballot_id=ballot_id))
+    allowed_path_handlers = {
+        "ietf.doc.views_doc.document_ballot",
+        "ietf.doc.views_doc.document_irsg_ballot",
+        "ietf.doc.views_doc.document_rsab_ballot",
+        "ietf.iesg.views.agenda",
+        "ietf.iesg.views.agenda_documents",
+    }
+    return validate_return_to_path(path, get_default_path, allowed_path_handlers)

ietf/doc/views_doc.py

@@ -1538,7 +1538,6 @@ def document_ballot(request, name, ballot_id=None):
     top = render_document_top(request, doc, ballot_tab, name)
 
     c = document_ballot_content(request, doc, ballot.id, editable=True)
-    request.session['ballot_edit_return_point'] = request.path_info
 
     return render(request, "doc/document_ballot.html",
                               dict(doc=doc,
@@ -1556,8 +1555,6 @@ def document_irsg_ballot(request, name, ballot_id=None):
 
     c = document_ballot_content(request, doc, ballot_id, editable=True)
 
-    request.session['ballot_edit_return_point'] = request.path_info
-
     return render(request, "doc/document_ballot.html",
                               dict(doc=doc,
                                    top=top,
@@ -1575,8 +1572,6 @@ def document_rsab_ballot(request, name, ballot_id=None):
 
     c = document_ballot_content(request, doc, ballot_id, editable=True)
 
-    request.session['ballot_edit_return_point'] = request.path_info
-
     return render(
         request,
         "doc/document_ballot.html",

ietf/iesg/views.py

@@ -209,7 +209,6 @@ def agenda(request, date=None):
                 urlreverse("ietf.iesg.views.telechat_agenda_content_view", kwargs={"section": "minutes"})
             ))
 
-    request.session['ballot_edit_return_point'] = request.path_info
     return render(request, "iesg/agenda.html", {
             "date": data["date"],
             "sections": sorted(data["sections"].items(), key=lambda x:[int(p) for p in x[0].split('.')]),
@@ -398,7 +397,7 @@ def agenda_documents(request):
                 "sections": sorted((num, section) for num, section in sections.items()
                                    if "2" <= num < "5")
                 })
-    request.session['ballot_edit_return_point'] = request.path_info
+
     return render(request, 'iesg/agenda_documents.html', { 'telechats': telechats })
 
 def past_documents(request):

ietf/ipr/forms.py

@@ -112,7 +112,7 @@ def clean(self):
         if not document:
             self.add_error("document", "Identifying the Internet-Draft or RFC for this disclosure is required.")
         elif not document.name.startswith("rfc"):
-            if revisions.strip() == "":
+            if revisions is None or revisions.strip() == "":
                 self.add_error("revisions", "Revisions of this Internet-Draft for which this disclosure is relevant must be specified.")
         return cleaned_data
 

ietf/ipr/tests.py

@@ -28,9 +28,11 @@
 from ietf.ipr.factories import (
     HolderIprDisclosureFactory,
     GenericIprDisclosureFactory,
+    IprDisclosureBaseFactory,
     IprDocRelFactory,
     IprEventFactory
 )
+from ietf.ipr.forms import DraftForm
 from ietf.ipr.mail import (process_response_email, get_reply_to, get_update_submitter_emails,
     get_pseudo_submitter, get_holders, get_update_cc_addrs)
 from ietf.ipr.models import (IprDisclosureBase,GenericIprDisclosure,HolderIprDisclosure,
@@ -935,3 +937,61 @@ def test_no_revisions_message(self):
             no_revisions_message(iprdocrel),
             "No revisions for this Internet-Draft were specified in this disclosure. However, there is only one revision of this Internet-Draft."
         )
+
+
+class DraftFormTests(TestCase):
+    def setUp(self):
+        super().setUp()
+        self.disclosure = IprDisclosureBaseFactory()
+        self.draft = WgDraftFactory.create_batch(10)[-1]
+        self.rfc = RfcFactory()
+
+    def test_revisions_valid(self):
+        post_data = {
+            # n.b., "document" is a SearchableDocumentField, which is a multiple choice field limited
+            # to a single choice. Its value must be an array of pks with one element.
+            "document": [str(self.draft.pk)],
+            "disclosure": str(self.disclosure.pk),
+        }
+        # The revisions field is just a char field that allows descriptions of the applicable
+        # document revisions. It's usually just a rev or "00-02", but the form allows anything
+        # not empty. The secretariat will review the value before the disclosure is posted so
+        # minimal validation is ok here.
+        self.assertTrue(DraftForm(post_data | {"revisions": "00"}).is_valid())
+        self.assertTrue(DraftForm(post_data | {"revisions": "00-02"}).is_valid())
+        self.assertTrue(DraftForm(post_data | {"revisions": "01,03, 05"}).is_valid())
+        self.assertTrue(DraftForm(post_data | {"revisions": "all but 01"}).is_valid())
+        # RFC instead of draft - allow empty / missing revisions
+        post_data["document"] = [str(self.rfc.pk)]
+        self.assertTrue(DraftForm(post_data).is_valid())
+        self.assertTrue(DraftForm(post_data | {"revisions": ""}).is_valid())
+
+    def test_revisions_invalid(self):
+        missing_rev_error_msg = (
+            "Revisions of this Internet-Draft for which this disclosure is relevant must be specified."
+        )
+        null_char_error_msg = "Null characters are not allowed."
+
+        post_data = {
+            # n.b., "document" is a SearchableDocumentField, which is a multiple choice field limited
+            # to a single choice. Its value must be an array of pks with one element.
+            "document": [str(self.draft.pk)],
+            "disclosure": str(self.disclosure.pk),
+        }
+        self.assertFormError(
+            DraftForm(post_data), "revisions", missing_rev_error_msg
+        )
+        self.assertFormError(
+            DraftForm(post_data | {"revisions": ""}), "revisions", missing_rev_error_msg
+        )
+        self.assertFormError(
+            DraftForm(post_data | {"revisions": "1\x00"}),
+            "revisions",
+            [null_char_error_msg, missing_rev_error_msg],
+        )
+        # RFC instead of draft still validates the revisions field
+        self.assertFormError(
+            DraftForm(post_data | {"document": [str(self.rfc.pk)], "revisions": "1\x00"}),
+            "revisions",
+            null_char_error_msg,
+        )

ietf/meeting/migrations/0008_remove_schedtimesessassignment_notes.py

@@ -0,0 +1,17 @@
+# Generated by Django 4.2.15 on 2024-08-16 13:49
+
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("meeting", "0007_attended_origin_attended_time"),
+    ]
+
+    operations = [
+        migrations.RemoveField(
+            model_name="schedtimesessassignment",
+            name="notes",
+        ),
+    ]

ietf/meeting/models.py

@@ -788,7 +788,6 @@ class SchedTimeSessAssignment(models.Model):
     schedule = ForeignKey('Schedule', null=False, blank=False, related_name='assignments')
     extendedfrom = ForeignKey('self', null=True, default=None, help_text="Timeslot this session is an extension of.")
     modified = models.DateTimeField(auto_now=True)
-    notes    = models.TextField(blank=True)
     badness  = models.IntegerField(default=0, blank=True, null=True)
     pinned   = models.BooleanField(default=False, help_text="Do not move session during automatic placement.")
 
@@ -1423,7 +1422,7 @@ class MeetingHost(models.Model):
                 validate_file_extension,
                 settings.MEETING_VALID_UPLOAD_EXTENSIONS['meetinghostlogo'],
             ),
-            WrappedValidator(
+   WrappedValidator(
                 validate_mime_type,
                 settings.MEETING_VALID_UPLOAD_MIME_TYPES['meetinghostlogo'],
                 True,

ietf/meeting/resources.py

@@ -269,7 +269,6 @@ class Meta:
         filtering = { 
             "id": ALL,
             "modified": ALL,
-            "notes": ALL,
             "badness": ALL,
             "pinned": ALL,
             "timeslot": ALL_WITH_RELATIONS,