feat: only offer IAB/IESG members for bofreq responsible leadership (#4276)

* refactor: avoid using select2 data-* attrs on html elements

Using "data-ajax--url" shadows explicit configuration in our
select2.js wrapper. Use "data-select2-ajax-url" to avoid this.

Also add ability to omit the ajax setup entirely by returning
None from ajax_url().

* chore: hook up a flag to disable ajax for SearchablePersonsField

* refactor: send select2 prefetch data as array and allow config of min input length

* feat: only offer IAB/IESG members for bofreq responsible leadership

* test: area directors/IAB members should be options for bofreq responsible leaders

* test: update tests to match changes to SearchableField

* fix: clean up SearchablePersonsField breakage when searching by email address

* chore: finish incomplete comment

ietf/doc/tests.py

@@ -2468,12 +2468,12 @@ class FieldTests(TestCase):
             decoded = json.loads(json_data)
         except json.JSONDecodeError as e:
             self.fail('data-pre contained invalid JSON data: %s' % str(e))
-        decoded_ids = list(decoded.keys())
-        self.assertCountEqual(decoded_ids, [str(doc.id) for doc in docs])
+        decoded_ids = [item['id'] for item in decoded]
+        self.assertEqual(decoded_ids, [doc.id for doc in docs])
         for doc in docs:
             self.assertEqual(
                 dict(id=doc.pk, selected=True, url=doc.get_absolute_url(), text=escape(uppercase_std_abbreviated_name(doc.name))),
-                decoded[str(doc.pk)],
+                decoded[decoded_ids.index(doc.pk)],
             )
 
 class MaterialsTests(TestCase):

ietf/doc/tests_bofreq.py

@@ -2,6 +2,7 @@
 
 import datetime
 import debug    # pyflakes:ignore
+import json
 import os
 
 from pathlib import Path
@@ -18,7 +19,9 @@ from ietf.group.factories import RoleFactory
 from ietf.doc.factories import BofreqFactory, NewRevisionDocEventFactory
 from ietf.doc.models import State, Document, DocAlias, NewRevisionDocEvent
 from ietf.doc.utils_bofreq import bofreq_editors, bofreq_responsible
+from ietf.ietfauth.utils import has_role
 from ietf.person.factories import PersonFactory
+from ietf.person.models import Person
 from ietf.utils.mail import outbox, empty_outbox
 from ietf.utils.test_utils import TestCase, reload_db_objects, unicontent, login_testing_unauthorized
 from ietf.utils.text import xslugify
@@ -270,6 +273,25 @@ This test section has some text.
         for p in bad_batch:
             self.assertIn(p.plain_name(), error_text)
 
+    def test_change_responsible_options(self):
+        """Only valid options should be offered for responsible leadership field"""
+        doc = BofreqFactory()
+        url = urlreverse('ietf.doc.views_bofreq.change_responsible', kwargs={'name': doc.name})
+        self.client.login(username='secretary', password='secretary+password')
+        r = self.client.get(url)
+        self.assertEqual(r.status_code, 200)
+        q = PyQuery(r.content)
+        option_ids = [opt['id'] for opt in json.loads(q('#id_responsible').attr('data-pre'))]
+        ad_people = [p for p in Person.objects.all() if has_role(p.user, 'Area Director')]
+        iab_people = [p for p in Person.objects.all() if has_role(p.user, 'IAB')]
+        self.assertGreater(len(ad_people), 0, 'Need at least one AD')
+        self.assertGreater(len(iab_people), 0, 'Need at least one IAB member')
+        self.assertGreater(Person.objects.count(), len(ad_people) + len(iab_people),
+                           'Need at least one Person not an AD nor IAB member')
+        # Next line will fail if there's overlap between ad_people and iab_people. This is by design.
+        # If the test setup changes and overlap is expected, need to separately check that area directors
+        # and IAB members wind up in the options list.
+        self.assertCountEqual(option_ids, [p.pk for p in ad_people + iab_people])
 
     def test_submit(self):
         doc = BofreqFactory()

ietf/doc/views_bofreq.py

@@ -7,6 +7,7 @@ import io
 from django import forms
 from django.conf import settings
 from django.contrib.auth.decorators import login_required
+from django.db.models import Q
 from django.shortcuts import get_object_or_404, redirect, render
 from django.template.loader import render_to_string
 from django.urls import reverse as urlreverse
@@ -20,6 +21,7 @@ from ietf.doc.utils import add_state_change_event
 from ietf.doc.utils_bofreq import bofreq_editors, bofreq_responsible
 from ietf.ietfauth.utils import has_role, role_required
 from ietf.person.fields import SearchablePersonsField
+from ietf.person.models import Person
 from ietf.utils import markdown
 from ietf.utils.response import permission_denied
 from ietf.utils.text import xslugify
@@ -224,7 +226,22 @@ def change_editors(request, name):
 
 
 class ChangeResponsibleForm(forms.Form):
-    responsible = SearchablePersonsField(required=False)
+    def __init__(self, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+
+        # The queryset in extra_prefetch finds users for whom has_role(u, 'Area Director') or
+        # has_role(u, 'IAB') is True. It needs to match the queries in the has_role() method.
+        # Disable ajax requests because the SearchablePersonsField cannot enforce the queryset filter.
+        self.fields['responsible'] = SearchablePersonsField(
+            required=False,
+            extra_prefetch=Person.objects.filter(
+                Q(role__name='member', role__group__acronym='iab')
+                | Q(role__name__in=('pre-ad', 'ad'), role__group__type='area', role__group__state='active')
+            ).distinct(),
+            disable_ajax=True,  # only use the prefetched options
+            min_search_length=0,  # do not require typing to display options
+        )
+
     def clean_responsible(self):
         responsible = self.cleaned_data['responsible']
         not_leadership = list()

ietf/liaisons/tests.py

@@ -1014,7 +1014,7 @@ class LiaisonManagementTests(TestCase):
         self.assertEqual(q('#id_from_groups').find('option:selected').val(),reply_from_group_id)
         self.assertEqual(q('#id_to_groups').find('option:selected').val(),reply_to_group_id)
         pre = json.loads(q('#id_related_to').attr("data-pre"))
-        self.assertEqual(pre[str(liaison.pk)]['id'], liaison.pk)
+        self.assertEqual(pre[0]['id'], liaison.pk)
 
     def test_search(self):
         # Statement 1

ietf/person/fields.py

@@ -9,6 +9,7 @@ from urllib.parse import urlencode
 
 from typing import Type # pyflakes:ignore
 
+import unidecode
 from django import forms
 from django.core.validators import validate_email
 from django.db import models # pyflakes:ignore
@@ -62,6 +63,9 @@ class SearchablePersonsField(SearchableField):
     that may be added to the initial set should be included in the extra_prefetch
     list. These can then be added by updating val() and triggering the 'change'
     event on the select2 field in JavaScript.
+
+    If disable_ajax is True, only objects that are prefetched can be selected. This
+    will be any currently selected items plus any in extra_prefetch.
     """
     model = Person # type: Type[models.Model]
     default_hint_text = "Type name to search for person."
@@ -69,12 +73,14 @@ class SearchablePersonsField(SearchableField):
                  only_users=False, # only select persons who also have a user
                  all_emails=False, # select only active email addresses
                  extra_prefetch=None, # extra data records to include in prefetch
+                 disable_ajax=False, # use ajax to search outside of prefetched set
                  *args, **kwargs):
         super(SearchablePersonsField, self).__init__(*args, **kwargs)
         self.only_users = only_users
         self.all_emails = all_emails
         self.extra_prefetch = extra_prefetch or []
         assert all([isinstance(obj, self.model) for obj in self.extra_prefetch])
+        self.disable_ajax = disable_ajax
 
     def validate_pks(self, pks):
         """Validate format of PKs"""
@@ -87,21 +93,27 @@ class SearchablePersonsField(SearchableField):
         # via the extra_prefetch property.
         prefetch_set = set(model_instances) if model_instances else set()
         prefetch_set = prefetch_set.union(set(self.extra_prefetch))  # eliminate duplicates
-        return select2_id_name(list(prefetch_set))
+        return sorted(
+            select2_id_name(list(prefetch_set)),
+            key=lambda item: unidecode.unidecode(item['text']),
+        )
 
     def ajax_url(self):
-        url = urlreverse(
-            "ietf.person.views.ajax_select2_search",
-            kwargs={ "model_name": self.model.__name__.lower() }
-        )
-        query_args = {}
-        if self.only_users:
-            query_args["user"] = "1"
-        if self.all_emails:
-            query_args["a"] = "1"
-        if len(query_args) > 0:
-            url += '?%s' % urlencode(query_args)
-        return url
+        if self.disable_ajax:
+            return None
+        else:
+            url = urlreverse(
+                "ietf.person.views.ajax_select2_search",
+                kwargs={ "model_name": self.model.__name__.lower() }
+            )
+            query_args = {}
+            if self.only_users:
+                query_args["user"] = "1"
+            if self.all_emails:
+                query_args["a"] = "1"
+            if len(query_args) > 0:
+                url += '?%s' % urlencode(query_args)
+            return url
 
 
 class SearchablePersonField(SearchablePersonsField):

ietf/static/js/select2.js

@@ -22,12 +22,19 @@ function prettify_tz(x) {
 // Copyright The IETF Trust 2015-2021, All Rights Reserved
 // JS for ietf.utils.fields.SearchableField subclasses
 window.setupSelect2Field = function (e) {
-    var url = e.data("ajax--url");
-    var maxEntries = e.data("max-entries");
-    var result_key = e.data("result-key");
-    var options = e.data("pre");
-    for (var id in options) {
-        e.append(new Option(options[id].text, options[id].id, false, options[id].selected));
+    // Avoid using data attributes that match any of the search2 option attributes.
+    // These will override settings in the options object that we use below.
+    // (see https://select2.org/configuration/data-attributes)
+    // Note: e.data('k') returns undefined if there is no data-k attribute.
+    let url = e.data("select2-ajax-url");
+    let maxEntries = e.data("max-entries");
+    let minSearchLength = e.data("min-search-length");
+    let result_key = e.data("result-key");
+    let options = e.data("pre");
+    if (options) {
+        options.forEach(
+          opt => e.append(new Option(opt.text, opt.id, false, opt.selected))
+        );
     }
 
     template_modify = e.hasClass("tz-select") ? prettify_tz : undefined;
@@ -35,6 +42,7 @@ window.setupSelect2Field = function (e) {
     e.select2({
         multiple: maxEntries !== 1,
         maximumSelectionSize: maxEntries,
+        minimumInputLength: minSearchLength,
         templateResult: template_modify,
         templateSelection: template_modify,
         ajax: url ? {

ietf/templates/base.html

@@ -61,7 +61,7 @@
                     <label aria-label="Document search">
                     <input class="form-control select2-field"
                            id="navbar-doc-search"
-                           data-ajax--url="{% url 'ietf.doc.views_search.ajax_select2_search_docs' model_name='docalias' doc_type='draft' %}"
+                           data-select2-ajax-url="{% url 'ietf.doc.views_search.ajax_select2_search_docs' model_name='docalias' doc_type='draft' %}"
                            type="text"
                            data-placeholder="Document search">
                    </label>

ietf/templates/doc/status_change/edit_related_rows.html

@@ -5,7 +5,7 @@
 {% for rfc,choice_slug in form.relations.items %}
     <div class="input-group mb-3">
         <select class="form-control select2-field"
-                data-ajax--url="{% url 'ietf.doc.views_search.ajax_select2_search_docs' model_name='document' doc_type='draft' %}"
+                data-select2-ajax-url="{% url 'ietf.doc.views_search.ajax_select2_search_docs' model_name='document' doc_type='draft' %}"
                 data-max-entries="1"
                 data-width="resolve"
                 data-result-key="text"
@@ -39,7 +39,7 @@
             id="new_relation_row_rfc"
             aria-label="Enter new affected RFC"
             class="form-control select2-field"
-            data-ajax--url="{% url 'ietf.doc.views_search.ajax_select2_search_docs' model_name='docalias' doc_type='draft' %}"
+            data-select2-ajax-url="{% url 'ietf.doc.views_search.ajax_select2_search_docs' model_name='docalias' doc_type='draft' %}"
             data-result-key="text"
             data-max-entries="1"
             data-width="resolve"

ietf/utils/fields.py

@@ -201,6 +201,7 @@ class SearchableField(forms.MultipleChoiceField):
     model = None  # type:Optional[Type[models.Model]]
 #    max_entries = None  # may be overridden in __init__
     max_entries = None # type: Optional[int] 
+    min_search_length = None # type: Optional[int]
     default_hint_text = 'Type a value to search'
     
     def __init__(self, hint_text=None, *args, **kwargs):
@@ -210,6 +211,8 @@ class SearchableField(forms.MultipleChoiceField):
         # not setting the parameter at all.
         if 'max_entries' in kwargs:
             self.max_entries = kwargs.pop('max_entries')
+        if 'min_search_length' in kwargs:
+            self.min_search_length = kwargs.pop('min_search_length')
 
         super(SearchableField, self).__init__(*args, **kwargs)
 
@@ -217,6 +220,8 @@ class SearchableField(forms.MultipleChoiceField):
         self.widget.attrs["data-placeholder"] = self.hint_text
         if self.max_entries is not None:
             self.widget.attrs["data-max-entries"] = self.max_entries
+        if self.min_search_length is not None:
+            self.widget.attrs["data-min-search-length"] = self.min_search_length
 
     def make_select2_data(self, model_instances):
         """Get select2 data items
@@ -281,13 +286,13 @@ class SearchableField(forms.MultipleChoiceField):
                 d["selected"] = any([v.pk == d["id"] for v in value])
             else:
                 d["selected"] = value.exists() and value.filter(pk__in=[d["id"]]).exists()
-        self.widget.attrs["data-pre"] = json.dumps({
-            d['id']: d for d in pre
-        })
+        self.widget.attrs["data-pre"] = json.dumps(list(pre))
 
         # doing this in the constructor is difficult because the URL
         # patterns may not have been fully constructed there yet
-        self.widget.attrs["data-ajax--url"] = self.ajax_url()
+        ajax_url = self.ajax_url()
+        if ajax_url is not None:
+            self.widget.attrs["data-select2-ajax-url"] = ajax_url
 
         result = value
         return result