From deb133970a404671ae1d034f7f9a2d2a1e56c4d2 Mon Sep 17 00:00:00 2001
From: Jennifer Richards <jennifer@staff.ietf.org>
Date: Fri, 2 May 2025 11:54:32 -0300
Subject: [PATCH 01/19] refactor: clean up outgoing LS from field init

---
 ietf/liaisons/forms.py | 84 +++++++++++++++++++++++++++---------------
 1 file changed, 55 insertions(+), 29 deletions(-)

diff --git a/ietf/liaisons/forms.py b/ietf/liaisons/forms.py
index 1af29044b3..ec6917c1fb 100644
--- a/ietf/liaisons/forms.py
+++ b/ietf/liaisons/forms.py
@@ -15,7 +15,6 @@
 from django.core.exceptions import ObjectDoesNotExist, ValidationError
 from django.forms.utils import ErrorList
 from django.db.models import Q
-#from django.forms.widgets import RadioFieldRenderer
 from django.core.validators import validate_email
 from django_stubs_ext import QuerySetAny
 
@@ -51,17 +50,22 @@
 def liaison_manager_sdos(person):
     return Group.objects.filter(type="sdo", state="active", role__person=person, role__name="liaiman").distinct()
 
+
 def flatten_choices(choices):
-    '''Returns a flat choice list given one with option groups defined'''
+    """Returns a flat choice list given one with option groups defined"""
+    # TODO this does not handle mixed grouped and ungrouped options properly
     flat = []
-    for optgroup,options in choices:
+    for optgroup, options in choices:
         flat.extend(options)
     return flat
-    
+
+
 def get_internal_choices(user):
-    '''Returns the set of internal IETF groups the user has permissions for, as a list
-    of choices suitable for use in a select widget.  If user == None, all active internal
-    groups are included.'''
+    """Get choices list for internal IETF groups user is authorized to select
+    
+    Returns a grouped list of choices suitable for use with a ChoiceField. If user is None,
+    includes all groups.
+    """
     choices = []
     groups = get_groups_for_person(user.person if user else None)
     main = [ (g.pk, 'The {}'.format(g.acronym.upper())) for g in groups.filter(acronym__in=('ietf','iesg','iab')) ]
@@ -72,6 +76,7 @@ def get_internal_choices(user):
     choices.append(('IETF Working Groups', wgs ))
     return choices
 
+
 def get_groups_for_person(person):
     '''Returns queryset of internal Groups the person has interesting roles in.
     This is a refactor of IETFHierarchyManager.get_entities_for_person().  If Person
@@ -216,8 +221,6 @@ class LiaisonModelForm(forms.ModelForm):
     '''Specify fields which require a custom widget or that are not part of the model.
     '''
     from_groups = ModelMultipleChoiceField(queryset=Group.objects.all(),label='Groups',required=False)
-    from_groups.widget.attrs["class"] = "select2-field"
-    from_groups.widget.attrs['data-minimum-input-length'] = 0
     from_contact = forms.EmailField()   # type: Union[forms.EmailField, SearchableEmailField]
     to_contacts = forms.CharField(label="Contacts", widget=forms.Textarea(attrs={'rows':'3', }), strip=False)
     to_groups = ModelMultipleChoiceField(queryset=Group.objects,label='Groups',required=False)
@@ -245,11 +248,13 @@ def __init__(self, user, *args, **kwargs):
         self.person = get_person_for_user(user)
         self.is_new = not self.instance.pk
 
+        self.fields["from_groups"].widget.attrs["class"] = "select2-field"
+        self.fields["from_groups"].widget.attrs["data-minimum-input-length"] = 0
         self.fields["from_groups"].widget.attrs["data-placeholder"] = "Type in name to search for group"
         self.fields["to_groups"].widget.attrs["data-placeholder"] = "Type in name to search for group"
         self.fields["to_contacts"].label = 'Contacts'
         self.fields["other_identifiers"].widget.attrs["rows"] = 2
-        
+
         # add email validators
         for field in ['from_contact','to_contacts','technical_contacts','action_holder_contacts','cc_contacts']:
             if field in self.fields:
@@ -472,31 +477,52 @@ class Meta:
     def is_approved(self):
         return self.cleaned_data['approved']
 
+    @staticmethod
+    def from_groups_choices(user):
+        return get_internal_choices(user)
+
+    @staticmethod
+    def to_groups_choices(user):
+        return Group.objects.none()
+
+    @staticmethod
+    def from_contact_queryset(person):
+        if person.role_set.filter(name='liaiman',group__state='active'):
+            email = person.role_set.filter(name='liaiman',group__state='active').first.email
+        elif person.role_set.filter(name__in=('ad','chair'),group__state='active'):
+            email = person.role_set.filter(name__in=('ad','chair'),group__state='active').first().email
+        else:
+            email = person.email()
+        return Email.objects.filter(pk=email)
+
+    @staticmethod
+    def to_contact_queryset(person):
+        return Email.objects.none()
+
     def set_from_fields(self):
-        '''Set from_groups and from_contact options and initial value based on user
-        accessing the form'''
-        choices = get_internal_choices(self.user)
-        self.fields['from_groups'].choices = choices
+        """Configure from from_groups and from_contact based on user roles"""
+        self.set_from_groups_field()
+        self.set_from_contact_field()
         
-        # set initial value if only one entry 
-        flat_choices = flatten_choices(choices)
+    def set_from_groups_field(self):
+        """Configure the from_groups field based on user roles"""
+        grouped_choices = self.from_groups_choices(self.user)
+        flat_choices = flatten_choices(grouped_choices)
         if len(flat_choices) == 1:
-            self.fields['from_groups'].initial = [flat_choices[0][0]]
-        
+            self.fields["from_groups"].choices = flat_choices
+            self.fields["from_groups"].initial = [flat_choices[0][0]]
+        else:
+            self.fields["from_groups"].choices = grouped_choices
+
+    def set_from_contact_field(self):
+        """Configure the from_contact field based on user roles"""
         if has_role(self.user, "Secretariat"):
             self.fields['from_contact'] = SearchableEmailField(only_users=True)  # secretariat can edit this field!
-            return
-
-        if self.person.role_set.filter(name='liaiman',group__state='active'):
-            email = self.person.role_set.filter(name='liaiman',group__state='active').first().email.address
-        elif self.person.role_set.filter(name__in=('ad','chair'),group__state='active'):
-            email = self.person.role_set.filter(name__in=('ad','chair'),group__state='active').first().email.address
         else:
-            email = self.person.email_address()
-
-        # Non-secretariat user cannot change the from_contact field. Fill in its value.
-        self.fields['from_contact'].disabled = True
-        self.fields['from_contact'].initial = email
+            # Non-secretariat user cannot change the from_contact field. Fill in its value.
+            allowed_from_emails = self.from_contact_queryset(self.person)
+            self.fields['from_contact'].disabled = True
+            self.fields['from_contact'].initial = allowed_from_emails.first().address  # todo actually allow choice
 
     def set_to_fields(self):
         '''Set to_groups and to_contacts options and initial value based on user

From 9b67ed6dea624f6d6c5a9501b9adc5ee9eafa3ff Mon Sep 17 00:00:00 2001
From: Jennifer Richards <jennifer@staff.ietf.org>
Date: Fri, 2 May 2025 15:57:28 -0300
Subject: [PATCH 02/19] feat: additional LS "from" groups for IETF/IAB
 Chair+ADs

---
 ietf/liaisons/forms.py | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/ietf/liaisons/forms.py b/ietf/liaisons/forms.py
index ec6917c1fb..0f73acd24e 100644
--- a/ietf/liaisons/forms.py
+++ b/ietf/liaisons/forms.py
@@ -82,7 +82,10 @@ def get_groups_for_person(person):
     This is a refactor of IETFHierarchyManager.get_entities_for_person().  If Person
     is None or Secretariat or Liaison Manager all internal IETF groups are returned.
     '''
-    if person == None or has_role(person.user, "Secretariat") or has_role(person.user, "Liaison Manager"):
+    if person is None or has_role(
+        person.user,
+        ("Secretariat", "IETF Chair", "IAB Chair", "Liaison Manager"),  # todo liaison coordinator as well
+    ):
         # collect all internal IETF groups
         queries = [Q(acronym__in=('ietf','iesg','iab')),
                    Q(type='area',state='active'),
@@ -94,6 +97,8 @@ def get_groups_for_person(person):
                    Q(role__person=person,role__name='ad',type='area',state='active'),
                    Q(role__person=person,role__name__in=('chair','secretary'),type='wg',state='active'),
                    Q(parent__role__person=person,parent__role__name='ad',type='wg',state='active')]
+        if has_role(person.user, "Area Director"):
+            queries.append(Q(acronym__in=("ietf", "iesg")))
     return Group.objects.filter(reduce(operator.or_,queries)).order_by('acronym').distinct()
 
 def liaison_form_factory(request, type=None, **kwargs):

From a38e86ca8484e7d3143e3e4335ee1cae4fc7374c Mon Sep 17 00:00:00 2001
From: Jennifer Richards <jennifer@staff.ietf.org>
Date: Fri, 2 May 2025 16:28:29 -0300
Subject: [PATCH 03/19] refactor: reduce queries in get_internal_choices

---
 ietf/liaisons/forms.py | 25 ++++++++++++++++++-------
 1 file changed, 18 insertions(+), 7 deletions(-)

diff --git a/ietf/liaisons/forms.py b/ietf/liaisons/forms.py
index 0f73acd24e..c21f3f0f53 100644
--- a/ietf/liaisons/forms.py
+++ b/ietf/liaisons/forms.py
@@ -5,6 +5,7 @@
 import io
 import os
 import operator
+from itertools import groupby
 
 from typing import Union            # pyflakes:ignore
 
@@ -66,14 +67,24 @@ def get_internal_choices(user):
     Returns a grouped list of choices suitable for use with a ChoiceField. If user is None,
     includes all groups.
     """
-    choices = []
     groups = get_groups_for_person(user.person if user else None)
-    main = [ (g.pk, 'The {}'.format(g.acronym.upper())) for g in groups.filter(acronym__in=('ietf','iesg','iab')) ]
-    areas = [ (g.pk, '{} - {}'.format(g.acronym,g.name)) for g in groups.filter(type='area') ]
-    wgs = [ (g.pk, '{} - {}'.format(g.acronym,g.name)) for g in groups.filter(type='wg') ]
-    choices.append(('Main IETF Entities', main))
-    choices.append(('IETF Areas', areas))
-    choices.append(('IETF Working Groups', wgs ))
+    main = []
+    areas = []
+    wgs = []
+    for g in groups.order_by("acronym"):
+        if g.acronym in ("ietf", "iesg", "iab"):
+            main.append((g.pk, f"The {g.acronym.upper()}"))
+        elif g.type_id == "area":
+            areas.append((g.pk, f"{g.acronym} - {g.name}"))
+        elif g.type_id == "wg":
+            areas.append((g.pk, f"{g.acronym} - {g.name}"))
+    choices = []
+    if len(main) > 0:
+        choices.append(("Main IETF Entities", main))
+    if len(areas) > 0:
+        choices.append(("IETF Areas", areas))
+    if len(wgs) > 0:
+        choices.append(("IETF Working Groups", wgs))
     return choices
 
 

From ba1319cf6aa53015c5b1535602f4653578da25c4 Mon Sep 17 00:00:00 2001
From: Jennifer Richards <jennifer@staff.ietf.org>
Date: Fri, 2 May 2025 17:03:30 -0300
Subject: [PATCH 04/19] refactor: break down / rename get_groups_for_person

---
 ietf/liaisons/forms.py | 74 +++++++++++++++++++++++++-----------------
 1 file changed, 44 insertions(+), 30 deletions(-)

diff --git a/ietf/liaisons/forms.py b/ietf/liaisons/forms.py
index c21f3f0f53..9fa6b6e0fb 100644
--- a/ietf/liaisons/forms.py
+++ b/ietf/liaisons/forms.py
@@ -15,7 +15,7 @@
 from django.conf import settings
 from django.core.exceptions import ObjectDoesNotExist, ValidationError
 from django.forms.utils import ErrorList
-from django.db.models import Q
+from django.db.models import Q, QuerySet
 from django.core.validators import validate_email
 from django_stubs_ext import QuerySetAny
 
@@ -61,17 +61,16 @@ def flatten_choices(choices):
     return flat
 
 
-def get_internal_choices(user):
+def choices_from_group_queryset(groups: QuerySet[Group]):
     """Get choices list for internal IETF groups user is authorized to select
     
     Returns a grouped list of choices suitable for use with a ChoiceField. If user is None,
     includes all groups.
     """
-    groups = get_groups_for_person(user.person if user else None)
     main = []
     areas = []
     wgs = []
-    for g in groups.order_by("acronym"):
+    for g in groups.distinct().order_by("acronym"):
         if g.acronym in ("ietf", "iesg", "iab"):
             main.append((g.pk, f"The {g.acronym.upper()}"))
         elif g.type_id == "area":
@@ -88,29 +87,44 @@ def get_internal_choices(user):
     return choices
 
 
-def get_groups_for_person(person):
-    '''Returns queryset of internal Groups the person has interesting roles in.
-    This is a refactor of IETFHierarchyManager.get_entities_for_person().  If Person
-    is None or Secretariat or Liaison Manager all internal IETF groups are returned.
-    '''
+def all_internal_groups():
+    """Get a queryset of all IETF groups suitable for LS To/From assignment"""
+    return Group.objects.filter(
+        Q(acronym__in=("ietf", "iesg", "iab"))
+        | Q(type="area", state="active")
+        | Q(type="wg", state="active")
+    ).distinct()
+
+
+def internal_groups_for_person(person):
+    """Get a queryset of IETF groups suitable for LS To/From assignment by person"""
     if person is None or has_role(
         person.user,
         ("Secretariat", "IETF Chair", "IAB Chair", "Liaison Manager"),  # todo liaison coordinator as well
     ):
-        # collect all internal IETF groups
-        queries = [Q(acronym__in=('ietf','iesg','iab')),
-                   Q(type='area',state='active'),
-                   Q(type='wg',state='active')]
-    else:
-        # Interesting roles, as Group queries
-        queries = [Q(role__person=person,role__name='chair',acronym='ietf'),
-                   Q(role__person=person,role__name__in=('chair','execdir'),acronym='iab'),
-                   Q(role__person=person,role__name='ad',type='area',state='active'),
-                   Q(role__person=person,role__name__in=('chair','secretary'),type='wg',state='active'),
-                   Q(parent__role__person=person,parent__role__name='ad',type='wg',state='active')]
-        if has_role(person.user, "Area Director"):
-            queries.append(Q(acronym__in=("ietf", "iesg")))
-    return Group.objects.filter(reduce(operator.or_,queries)).order_by('acronym').distinct()
+        return all_internal_groups()
+    # Interesting roles, as Group queries
+    queries = [
+        Q(role__person=person, role__name="chair", acronym="ietf"),
+        Q(role__person=person, role__name__in=("chair", "execdir"), acronym="iab"),
+        Q(role__person=person, role__name="ad", type="area", state="active"),
+        Q(
+            role__person=person,
+            role__name__in=("chair", "secretary"),
+            type="wg",
+            state="active",
+        ),
+        Q(
+            parent__role__person=person,
+            parent__role__name="ad",
+            type="wg",
+            state="active",
+        ),
+    ]
+    if has_role(person.user, "Area Director"):
+        queries.append(Q(acronym__in=("ietf", "iesg")))  # AD can also choose these
+    return Group.objects.filter(reduce(operator.or_, queries)).distinct()
+
 
 def liaison_form_factory(request, type=None, **kwargs):
     """Returns appropriate Liaison entry form"""
@@ -480,7 +494,7 @@ def set_to_fields(self):
         '''Set to_groups and to_contacts options and initial value based on user
         accessing the form.  For incoming Liaisons, to_groups choices is the full set.
         '''
-        self.fields['to_groups'].choices = get_internal_choices(None)
+        self.fields['to_groups'].choices = choices_from_group_queryset(all_internal_groups())
 
 
 class OutgoingLiaisonForm(LiaisonModelForm):
@@ -494,8 +508,8 @@ def is_approved(self):
         return self.cleaned_data['approved']
 
     @staticmethod
-    def from_groups_choices(user):
-        return get_internal_choices(user)
+    def from_groups_choices(person):
+        return choices_from_group_queryset(internal_groups_for_person(person))
 
     @staticmethod
     def to_groups_choices(user):
@@ -521,8 +535,8 @@ def set_from_fields(self):
         self.set_from_contact_field()
         
     def set_from_groups_field(self):
-        """Configure the from_groups field based on user roles"""
-        grouped_choices = self.from_groups_choices(self.user)
+        """Configure the from_groups field based on roles"""
+        grouped_choices = self.from_groups_choices(self.person)
         flat_choices = flatten_choices(grouped_choices)
         if len(flat_choices) == 1:
             self.fields["from_groups"].choices = flat_choices
@@ -578,7 +592,7 @@ def set_from_fields(self):
         '''Set from_groups and from_contact options and initial value based on user
         accessing the form.'''
         if self.instance.is_outgoing():
-            self.fields['from_groups'].choices = get_internal_choices(self.user)
+            self.fields['from_groups'].choices = choices_from_group_queryset(internal_groups_for_person(self.person))
         else:
             if has_role(self.user, "Secretariat"):
                 queryset = Group.objects.filter(type="sdo").order_by('name')
@@ -600,7 +614,7 @@ def set_to_fields(self):
                 queryset = Group.objects.filter(type="sdo").order_by('name')
             self.fields['to_groups'].queryset = queryset
         else:
-            self.fields['to_groups'].choices = get_internal_choices(None)
+            self.fields['to_groups'].choices = choices_from_group_queryset(all_internal_groups())
 
 
 class EditAttachmentForm(forms.Form):

From 032802013060703a7d56d97663a1806c28ccbd5e Mon Sep 17 00:00:00 2001
From: Jennifer Richards <jennifer@staff.ietf.org>
Date: Fri, 2 May 2025 17:16:33 -0300
Subject: [PATCH 05/19] refactor: inline / remove unneeded methods

---
 ietf/liaisons/forms.py | 13 +------------
 1 file changed, 1 insertion(+), 12 deletions(-)

diff --git a/ietf/liaisons/forms.py b/ietf/liaisons/forms.py
index 9fa6b6e0fb..2dfd7625cc 100644
--- a/ietf/liaisons/forms.py
+++ b/ietf/liaisons/forms.py
@@ -5,7 +5,6 @@
 import io
 import os
 import operator
-from itertools import groupby
 
 from typing import Union            # pyflakes:ignore
 
@@ -19,8 +18,6 @@
 from django.core.validators import validate_email
 from django_stubs_ext import QuerySetAny
 
-import debug                            # pyflakes:ignore
-
 from ietf.ietfauth.utils import has_role
 from ietf.name.models import DocRelationshipName
 from ietf.liaisons.utils import get_person_for_user,is_authorized_individual
@@ -507,14 +504,6 @@ class Meta:
     def is_approved(self):
         return self.cleaned_data['approved']
 
-    @staticmethod
-    def from_groups_choices(person):
-        return choices_from_group_queryset(internal_groups_for_person(person))
-
-    @staticmethod
-    def to_groups_choices(user):
-        return Group.objects.none()
-
     @staticmethod
     def from_contact_queryset(person):
         if person.role_set.filter(name='liaiman',group__state='active'):
@@ -536,7 +525,7 @@ def set_from_fields(self):
         
     def set_from_groups_field(self):
         """Configure the from_groups field based on roles"""
-        grouped_choices = self.from_groups_choices(self.person)
+        grouped_choices = choices_from_group_queryset(internal_groups_for_person(self.person))
         flat_choices = flatten_choices(grouped_choices)
         if len(flat_choices) == 1:
             self.fields["from_groups"].choices = flat_choices

From 3f8c28ddd29d65c4f964f6c372cfdbf2c26d3f5f Mon Sep 17 00:00:00 2001
From: Jennifer Richards <jennifer@staff.ietf.org>
Date: Fri, 2 May 2025 18:13:36 -0300
Subject: [PATCH 06/19] refactor: colocate similar field config

---
 ietf/liaisons/forms.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ietf/liaisons/forms.py b/ietf/liaisons/forms.py
index 2dfd7625cc..27c5de9c08 100644
--- a/ietf/liaisons/forms.py
+++ b/ietf/liaisons/forms.py
@@ -251,8 +251,6 @@ class LiaisonModelForm(forms.ModelForm):
     from_contact = forms.EmailField()   # type: Union[forms.EmailField, SearchableEmailField]
     to_contacts = forms.CharField(label="Contacts", widget=forms.Textarea(attrs={'rows':'3', }), strip=False)
     to_groups = ModelMultipleChoiceField(queryset=Group.objects,label='Groups',required=False)
-    to_groups.widget.attrs["class"] = "select2-field"
-    to_groups.widget.attrs['data-minimum-input-length'] = 0
     deadline = DatepickerDateField(date_format="yyyy-mm-dd", picker_settings={"autoclose": "1" }, label='Deadline', required=True)
     related_to = SearchableLiaisonStatementsField(label='Related Liaison Statement', required=False)
     submitted_date = DatepickerDateField(date_format="yyyy-mm-dd", picker_settings={"autoclose": "1" }, label='Submission date', required=True, initial=lambda: date_today(DEADLINE_TZINFO))
@@ -278,6 +276,8 @@ def __init__(self, user, *args, **kwargs):
         self.fields["from_groups"].widget.attrs["class"] = "select2-field"
         self.fields["from_groups"].widget.attrs["data-minimum-input-length"] = 0
         self.fields["from_groups"].widget.attrs["data-placeholder"] = "Type in name to search for group"
+        self.fields["to_groups"].widget.attrs["class"] = "select2-field"
+        self.fields["to_groups"].widget.attrs["data-minimum-input-length"] = 0
         self.fields["to_groups"].widget.attrs["data-placeholder"] = "Type in name to search for group"
         self.fields["to_contacts"].label = 'Contacts'
         self.fields["other_identifiers"].widget.attrs["rows"] = 2

From 0ae84634480e67216774920822717c4a8934ef94 Mon Sep 17 00:00:00 2001
From: Jennifer Richards <jennifer@staff.ietf.org>
Date: Fri, 2 May 2025 19:01:27 -0300
Subject: [PATCH 07/19] refactor: unify role logic for LS To fields

---
 ietf/liaisons/forms.py | 37 ++++++++++++++++++++-----------------
 ietf/liaisons/utils.py | 20 +++++++++++++++++---
 2 files changed, 37 insertions(+), 20 deletions(-)

diff --git a/ietf/liaisons/forms.py b/ietf/liaisons/forms.py
index 27c5de9c08..6c90681911 100644
--- a/ietf/liaisons/forms.py
+++ b/ietf/liaisons/forms.py
@@ -20,7 +20,8 @@
 
 from ietf.ietfauth.utils import has_role
 from ietf.name.models import DocRelationshipName
-from ietf.liaisons.utils import get_person_for_user,is_authorized_individual
+from ietf.liaisons.utils import get_person_for_user, is_authorized_individual, OUTGOING_LIAISON_ROLES, \
+    INCOMING_LIAISON_ROLES
 from ietf.liaisons.widgets import ButtonWidget,ShowAttachmentsWidget
 from ietf.liaisons.models import (LiaisonStatement,
     LiaisonStatementEvent,LiaisonStatementAttachment,LiaisonStatementPurposeName)
@@ -123,6 +124,17 @@ def internal_groups_for_person(person):
     return Group.objects.filter(reduce(operator.or_, queries)).distinct()
 
 
+def external_groups_for_person(person):
+    filter_expr = Q(pk__in=[])  # start with no groups
+    # These roles can add all external sdo groups
+    if has_role(person.user, set(INCOMING_LIAISON_ROLES + OUTGOING_LIAISON_ROLES) - {"Liaison Manager"}):
+        filter_expr |= Q(type="sdo")
+    else:
+        # The person cannot add all external sdo groups; add any for which they are Liaison Manager
+        filter_expr |= Q(type="sdo", role__person=person, role__name="liaiman")
+    return Group.objects.filter(state="active").filter(filter_expr).distinct()
+    
+
 def liaison_form_factory(request, type=None, **kwargs):
     """Returns appropriate Liaison entry form"""
     user = request.user
@@ -514,12 +526,8 @@ def from_contact_queryset(person):
             email = person.email()
         return Email.objects.filter(pk=email)
 
-    @staticmethod
-    def to_contact_queryset(person):
-        return Email.objects.none()
-
     def set_from_fields(self):
-        """Configure from from_groups and from_contact based on user roles"""
+        """Configure from "From" fields based on user roles"""
         self.set_from_groups_field()
         self.set_from_contact_field()
         
@@ -544,20 +552,15 @@ def set_from_contact_field(self):
             self.fields['from_contact'].initial = allowed_from_emails.first().address  # todo actually allow choice
 
     def set_to_fields(self):
-        '''Set to_groups and to_contacts options and initial value based on user
-        accessing the form'''
-        # set options. if the user is a Liaison Manager and nothing more, reduce set to his SDOs
-        if has_role(self.user, "Liaison Manager") and not self.person.role_set.filter(name__in=('ad','chair'),group__state='active'):
-            queryset = Group.objects.filter(type="sdo", state="active", role__person=self.person, role__name="liaiman").distinct().order_by('name')
-        else:
-            # get all outgoing entities
-            queryset = Group.objects.filter(type="sdo", state="active").order_by('name')
-
-        self.fields['to_groups'].queryset = queryset
+        """Configure the "To" fields based on user roles"""
+        qs = external_groups_for_person(self.person).order_by("name")
+        self.fields['to_groups'].queryset = qs 
 
         # set initial
         if has_role(self.user, "Liaison Manager"):
-            self.fields['to_groups'].initial = [queryset.first()]
+            self.fields['to_groups'].initial = [
+                qs.filter(role__person=self.person, role__name="liaiman").first()
+            ]
 
 
 class EditLiaisonForm(LiaisonModelForm):
diff --git a/ietf/liaisons/utils.py b/ietf/liaisons/utils.py
index df48831917..05c7c8a85c 100644
--- a/ietf/liaisons/utils.py
+++ b/ietf/liaisons/utils.py
@@ -4,6 +4,21 @@
 from ietf.liaisons.models import LiaisonStatement
 from ietf.ietfauth.utils import has_role, passes_test_decorator
 
+# Roles allowed to create and manage outgoing liaison statements.
+OUTGOING_LIAISON_ROLES = [
+    "Area Director",
+    "IAB Chair",
+    "IAB Executive Director",
+    "IETF Chair",
+    "Liaison Manager",
+    "Secretariat",
+    "WG Chair",
+    "WG Secretary",
+]
+
+# Roles allowed to create and manage incoming liaison statements.
+INCOMING_LIAISON_ROLES = ["Authorized Individual", "Liaison Manager", "Secretariat"]
+
 can_submit_liaison_required = passes_test_decorator(
     lambda u, *args, **kwargs: can_add_liaison(u),
     "Restricted to participants who are authorized to submit liaison statements on behalf of the various IETF entities")
@@ -59,11 +74,10 @@ def get_person_for_user(user):
         return None
 
 def can_add_outgoing_liaison(user):
-    return has_role(user, ["Area Director","WG Chair","WG Secretary","IETF Chair","IAB Chair",
-        "IAB Executive Director","Liaison Manager","Secretariat"])
+    return has_role(user, OUTGOING_LIAISON_ROLES)
 
 def can_add_incoming_liaison(user):
-    return has_role(user, ["Liaison Manager","Authorized Individual","Secretariat"])
+    return has_role(user, INCOMING_LIAISON_ROLES)
 
 def can_add_liaison(user):
     return can_add_incoming_liaison(user) or can_add_outgoing_liaison(user)

From f216b216abbf60832f82cf01b7b1c036be35939f Mon Sep 17 00:00:00 2001
From: Jennifer Richards <jennifer@staff.ietf.org>
Date: Fri, 2 May 2025 19:01:42 -0300
Subject: [PATCH 08/19] fix: typo

---
 ietf/liaisons/forms.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ietf/liaisons/forms.py b/ietf/liaisons/forms.py
index 6c90681911..ee943f738d 100644
--- a/ietf/liaisons/forms.py
+++ b/ietf/liaisons/forms.py
@@ -519,7 +519,7 @@ def is_approved(self):
     @staticmethod
     def from_contact_queryset(person):
         if person.role_set.filter(name='liaiman',group__state='active'):
-            email = person.role_set.filter(name='liaiman',group__state='active').first.email
+            email = person.role_set.filter(name='liaiman',group__state='active').first().email
         elif person.role_set.filter(name__in=('ad','chair'),group__state='active'):
             email = person.role_set.filter(name__in=('ad','chair'),group__state='active').first().email
         else:

From a55e654aa874a504d4f80875ecfe771d8ab29414 Mon Sep 17 00:00:00 2001
From: Jennifer Richards <jennifer@staff.ietf.org>
Date: Fri, 2 May 2025 19:17:46 -0300
Subject: [PATCH 09/19] refactor: update EditLiaisonForm to match changes

---
 ietf/liaisons/forms.py | 26 +++++++-------------------
 1 file changed, 7 insertions(+), 19 deletions(-)

diff --git a/ietf/liaisons/forms.py b/ietf/liaisons/forms.py
index ee943f738d..b4ba87edc0 100644
--- a/ietf/liaisons/forms.py
+++ b/ietf/liaisons/forms.py
@@ -132,7 +132,7 @@ def external_groups_for_person(person):
     else:
         # The person cannot add all external sdo groups; add any for which they are Liaison Manager
         filter_expr |= Q(type="sdo", role__person=person, role__name="liaiman")
-    return Group.objects.filter(state="active").filter(filter_expr).distinct()
+    return Group.objects.filter(state="active").filter(filter_expr).distinct().order_by("name")
     
 
 def liaison_form_factory(request, type=None, **kwargs):
@@ -553,7 +553,7 @@ def set_from_contact_field(self):
 
     def set_to_fields(self):
         """Configure the "To" fields based on user roles"""
-        qs = external_groups_for_person(self.person).order_by("name")
+        qs = external_groups_for_person(self.person)
         self.fields['to_groups'].queryset = qs 
 
         # set initial
@@ -581,30 +581,18 @@ def save(self, *args, **kwargs):
         return self.instance
 
     def set_from_fields(self):
-        '''Set from_groups and from_contact options and initial value based on user
-        accessing the form.'''
+        """Configure from "From" fields based on user roles"""
         if self.instance.is_outgoing():
             self.fields['from_groups'].choices = choices_from_group_queryset(internal_groups_for_person(self.person))
         else:
-            if has_role(self.user, "Secretariat"):
-                queryset = Group.objects.filter(type="sdo").order_by('name')
-            else:
-                queryset = Group.objects.filter(type="sdo", role__person=self.person, role__name__in=("liaiman", "auth")).distinct().order_by('name')
+            self.fields["from_groups"].queryset = external_groups_for_person(self.person)
+            if not has_role(self.user, "Secretariat"):
                 self.fields['from_contact'].widget.attrs['disabled'] = True
-            self.fields['from_groups'].queryset = queryset
 
     def set_to_fields(self):
-        '''Set to_groups and to_contacts options and initial value based on user
-        accessing the form.  For incoming Liaisons, to_groups choices is the full set.
-        '''
+        """Configure the "To" fields based on user roles"""
         if self.instance.is_outgoing():
-            # if the user is a Liaison Manager and nothing more, reduce to set to his SDOs
-            if has_role(self.user, "Liaison Manager") and not self.person.role_set.filter(name__in=('ad','chair'),group__state='active'):
-                queryset = Group.objects.filter(type="sdo", role__person=self.person, role__name="liaiman").distinct().order_by('name')
-            else:
-                # get all outgoing entities
-                queryset = Group.objects.filter(type="sdo").order_by('name')
-            self.fields['to_groups'].queryset = queryset
+            self.fields['to_groups'].queryset = external_groups_for_person(self.person)
         else:
             self.fields['to_groups'].choices = choices_from_group_queryset(all_internal_groups())
 

From 2232869fda91e9bf6c44c708820d984e74202c20 Mon Sep 17 00:00:00 2001
From: Jennifer Richards <jennifer@staff.ietf.org>
Date: Fri, 2 May 2025 19:26:46 -0300
Subject: [PATCH 10/19] refactor: update IncomingLiaisonForm to match

---
 ietf/liaisons/forms.py | 23 ++++++++++-------------
 1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/ietf/liaisons/forms.py b/ietf/liaisons/forms.py
index b4ba87edc0..b2feea192b 100644
--- a/ietf/liaisons/forms.py
+++ b/ietf/liaisons/forms.py
@@ -484,20 +484,17 @@ def get_post_only(self):
         return True
 
     def set_from_fields(self):
-        '''Set from_groups and from_contact options and initial value based on user
-        accessing the form.'''
-        if has_role(self.user, "Secretariat"):
-            queryset = Group.objects.filter(type="sdo", state="active").order_by('name')
-        else:
-            queryset = Group.objects.filter(type="sdo", state="active", role__person=self.person, role__name__in=("liaiman", "auth")).distinct().order_by('name')
-            self.fields['from_contact'].initial = self.person.role_set.filter(group=queryset[0]).first().email.address
-            self.fields['from_contact'].widget.attrs['disabled'] = True
-        self.fields['from_groups'].queryset = queryset
-        self.fields['from_groups'].widget.submitter = str(self.person)
-
+        """Configure from "From" fields based on user roles"""
+        qs = external_groups_for_person(self.person)
+        self.fields["from_groups"].queryset = qs
+        self.fields["from_groups"].widget.submitter = str(self.person)
         # if there's only one possibility make it the default
-        if len(queryset) == 1:
-            self.fields['from_groups'].initial = queryset
+        if len(qs) == 1:
+            self.fields['from_groups'].initial = qs
+
+        if not has_role(self.user, "Secretariat"):
+            self.fields["from_contact"].initial = self.person.role_set.filter(group=qs[0]).first().email.address
+            self.fields["from_contact"].widget.attrs["disabled"] = True
 
     def set_to_fields(self):
         '''Set to_groups and to_contacts options and initial value based on user

From db8c46b63b40ca4e56da75dcc38aa3449d254a64 Mon Sep 17 00:00:00 2001
From: Jennifer Richards <jennifer@staff.ietf.org>
Date: Fri, 2 May 2025 19:35:48 -0300
Subject: [PATCH 11/19] fix: typo / add docstring

---
 ietf/liaisons/forms.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/ietf/liaisons/forms.py b/ietf/liaisons/forms.py
index b2feea192b..8538d4d829 100644
--- a/ietf/liaisons/forms.py
+++ b/ietf/liaisons/forms.py
@@ -74,7 +74,7 @@ def choices_from_group_queryset(groups: QuerySet[Group]):
         elif g.type_id == "area":
             areas.append((g.pk, f"{g.acronym} - {g.name}"))
         elif g.type_id == "wg":
-            areas.append((g.pk, f"{g.acronym} - {g.name}"))
+            wgs.append((g.pk, f"{g.acronym} - {g.name}"))
     choices = []
     if len(main) > 0:
         choices.append(("Main IETF Entities", main))
@@ -125,6 +125,7 @@ def internal_groups_for_person(person):
 
 
 def external_groups_for_person(person):
+    """Get a queryset of external groups suitable for LS To/From assignment by person"""
     filter_expr = Q(pk__in=[])  # start with no groups
     # These roles can add all external sdo groups
     if has_role(person.user, set(INCOMING_LIAISON_ROLES + OUTGOING_LIAISON_ROLES) - {"Liaison Manager"}):

From f95d000586b277ea6c83655fd6e040d3bbe9ec96 Mon Sep 17 00:00:00 2001
From: Jennifer Richards <jennifer@staff.ietf.org>
Date: Mon, 5 May 2025 12:39:51 -0300
Subject: [PATCH 12/19] test: framing for new tests; test_flatten_choices()

---
 ietf/liaisons/forms.py       |  7 ++++--
 ietf/liaisons/tests_forms.py | 42 ++++++++++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+), 2 deletions(-)
 create mode 100644 ietf/liaisons/tests_forms.py

diff --git a/ietf/liaisons/forms.py b/ietf/liaisons/forms.py
index 8538d4d829..f7c001a684 100644
--- a/ietf/liaisons/forms.py
+++ b/ietf/liaisons/forms.py
@@ -51,8 +51,11 @@ def liaison_manager_sdos(person):
 
 
 def flatten_choices(choices):
-    """Returns a flat choice list given one with option groups defined"""
-    # TODO this does not handle mixed grouped and ungrouped options properly
+    """Returns a flat choice list given one with option groups defined
+    
+    n.b., Django allows mixing grouped options and top-level options. This helper only supports
+    the non-mixed case where every option is in an option group.
+    """
     flat = []
     for optgroup, options in choices:
         flat.extend(options)
diff --git a/ietf/liaisons/tests_forms.py b/ietf/liaisons/tests_forms.py
new file mode 100644
index 0000000000..cffe489491
--- /dev/null
+++ b/ietf/liaisons/tests_forms.py
@@ -0,0 +1,42 @@
+# Copyright The IETF Trust 2025, All Rights Reserved
+from ietf.liaisons.forms import flatten_choices
+from ietf.utils.test_utils import TestCase
+
+
+class HelperTests(TestCase):
+    def test_choices_from_group_queryset(self):
+        raise NotImplementedError()
+
+    def test_all_internal_groups(self):
+        raise NotImplementedError()
+
+    def test_internal_groups_for_person(self):
+        raise NotImplementedError()
+
+    def test_external_groups_for_person(self):
+        raise NotImplementedError()
+
+    def test_flatten_choices(self):
+        self.assertEqual(flatten_choices([]), [])
+        self.assertEqual(
+            flatten_choices(
+                (
+                    ("group A", ()),
+                    ("group B", (("val0", "label0"), ("val1", "label1"))),
+                    ("group C", (("val2", "label2"),)),
+                )
+            ),
+            [("val0", "label0"), ("val1", "label1"), ("val2", "label2")],
+        )
+
+
+class IncomingLiaisonFormTests(TestCase):
+    pass
+
+
+class OutgoingLiaisonFormTests(TestCase):
+    pass
+
+
+class EditLiaisonFormTests(TestCase):
+    pass

From e43fbb3909c2b18a8f625f1d697d99ed5fe4744d Mon Sep 17 00:00:00 2001
From: Jennifer Richards <jennifer@staff.ietf.org>
Date: Mon, 5 May 2025 13:13:20 -0300
Subject: [PATCH 13/19] test: test_choices_from_group_queryset()

---
 ietf/liaisons/tests_forms.py | 73 +++++++++++++++++++++++++++++++++++-
 1 file changed, 71 insertions(+), 2 deletions(-)

diff --git a/ietf/liaisons/tests_forms.py b/ietf/liaisons/tests_forms.py
index cffe489491..d9a447cddd 100644
--- a/ietf/liaisons/tests_forms.py
+++ b/ietf/liaisons/tests_forms.py
@@ -1,11 +1,80 @@
 # Copyright The IETF Trust 2025, All Rights Reserved
-from ietf.liaisons.forms import flatten_choices
+from ietf.group.factories import GroupFactory
+from ietf.group.models import Group
+from ietf.liaisons.forms import flatten_choices, choices_from_group_queryset
 from ietf.utils.test_utils import TestCase
 
 
 class HelperTests(TestCase):
+    @staticmethod
+    def _alphabetically_by_acronym(group_list):
+        return sorted(group_list, key=lambda item: item.acronym)
+
     def test_choices_from_group_queryset(self):
-        raise NotImplementedError()
+        main_groups = list(Group.objects.filter(acronym__in=["ietf", "iab"]))
+        areas = GroupFactory.create_batch(2, type_id="area")
+        wgs = GroupFactory.create_batch(2)
+
+        # No groups
+        self.assertEqual(
+            choices_from_group_queryset(Group.objects.none()),
+            [],
+        )
+
+        # Main groups only
+        choices = choices_from_group_queryset(
+            Group.objects.filter(pk__in=[g.pk for g in main_groups])
+        )
+        self.assertEqual(len(choices), 1, "show one optgroup, hide empty ones")
+        self.assertEqual(choices[0][0], "Main IETF Entities")
+        self.assertEqual(
+            [val for val, _ in choices[0][1]],  # extract the choice value
+            [g.pk for g in self._alphabetically_by_acronym(main_groups)],
+        )
+
+        # Area groups only
+        choices = choices_from_group_queryset(
+            Group.objects.filter(pk__in=[g.pk for g in areas])
+        )
+        self.assertEqual(len(choices), 1, "show one optgroup, hide empty ones")
+        self.assertEqual(choices[0][0], "IETF Areas")
+        self.assertEqual(
+            [val for val, _ in choices[0][1]],  # extract the choice value
+            [g.pk for g in self._alphabetically_by_acronym(areas)],
+        )
+
+        # WGs only
+        choices = choices_from_group_queryset(
+            Group.objects.filter(pk__in=[g.pk for g in wgs])
+        )
+        self.assertEqual(len(choices), 1, "show one optgroup, hide empty ones")
+        self.assertEqual(choices[0][0], "IETF Working Groups")
+        self.assertEqual(
+            [val for val, _ in choices[0][1]],  # extract the choice value
+            [g.pk for g in self._alphabetically_by_acronym(wgs)],
+        )
+
+        # All together
+        choices = choices_from_group_queryset(
+            Group.objects.filter(pk__in=[g.pk for g in main_groups + areas + wgs])
+        )
+        self.assertEqual(len(choices), 3, "show all three optgroups")
+        self.assertEqual(
+            [optgroup_label for optgroup_label, _ in choices],
+            ["Main IETF Entities", "IETF Areas", "IETF Working Groups"],
+        )
+        self.assertEqual(
+            [val for val, _ in choices[0][1]],  # extract the choice value
+            [g.pk for g in self._alphabetically_by_acronym(main_groups)],
+        )
+        self.assertEqual(
+            [val for val, _ in choices[1][1]],  # extract the choice value
+            [g.pk for g in self._alphabetically_by_acronym(areas)],
+        )
+        self.assertEqual(
+            [val for val, _ in choices[2][1]],  # extract the choice value
+            [g.pk for g in self._alphabetically_by_acronym(wgs)],
+        )
 
     def test_all_internal_groups(self):
         raise NotImplementedError()

From 418f8ada3ff71db43da43e8000cbee88f30c2210 Mon Sep 17 00:00:00 2001
From: Jennifer Richards <jennifer@staff.ietf.org>
Date: Mon, 5 May 2025 13:20:20 -0300
Subject: [PATCH 14/19] test: test_all_internal_groups()

---
 ietf/liaisons/tests_forms.py | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/ietf/liaisons/tests_forms.py b/ietf/liaisons/tests_forms.py
index d9a447cddd..da73c9db00 100644
--- a/ietf/liaisons/tests_forms.py
+++ b/ietf/liaisons/tests_forms.py
@@ -1,7 +1,11 @@
 # Copyright The IETF Trust 2025, All Rights Reserved
 from ietf.group.factories import GroupFactory
 from ietf.group.models import Group
-from ietf.liaisons.forms import flatten_choices, choices_from_group_queryset
+from ietf.liaisons.forms import (
+    flatten_choices,
+    choices_from_group_queryset,
+    all_internal_groups,
+)
 from ietf.utils.test_utils import TestCase
 
 
@@ -77,7 +81,11 @@ def test_choices_from_group_queryset(self):
         )
 
     def test_all_internal_groups(self):
-        raise NotImplementedError()
+        # relies on the data created in ietf.utils.test_data.make_immutable_test_data()
+        self.assertCountEqual(
+            all_internal_groups().values_list("acronym", flat=True),
+            {"ietf", "iab", "iesg", "farfut", "ops", "sops"},
+        )
 
     def test_internal_groups_for_person(self):
         raise NotImplementedError()

From 75055765ce1e79de91878575ae15ec01a4bc39f1 Mon Sep 17 00:00:00 2001
From: Jennifer Richards <jennifer@staff.ietf.org>
Date: Mon, 5 May 2025 13:31:30 -0300
Subject: [PATCH 15/19] fix: no person = no internal groups for LS

---
 ietf/liaisons/forms.py | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/ietf/liaisons/forms.py b/ietf/liaisons/forms.py
index f7c001a684..dbcc72d4ae 100644
--- a/ietf/liaisons/forms.py
+++ b/ietf/liaisons/forms.py
@@ -6,9 +6,8 @@
 import os
 import operator
 
-from typing import Union            # pyflakes:ignore
-
 from email.utils import parseaddr
+from typing import Union, Optional  # pyflakes:ignore
 
 from django import forms
 from django.conf import settings
@@ -27,7 +26,7 @@
     LiaisonStatementEvent,LiaisonStatementAttachment,LiaisonStatementPurposeName)
 from ietf.liaisons.fields import SearchableLiaisonStatementsField
 from ietf.group.models import Group
-from ietf.person.models import Email
+from ietf.person.models import Email, Person
 from ietf.person.fields import SearchableEmailField
 from ietf.doc.models import Document
 from ietf.utils.fields import DatepickerDateField, ModelMultipleChoiceField
@@ -97,9 +96,12 @@ def all_internal_groups():
     ).distinct()
 
 
-def internal_groups_for_person(person):
+def internal_groups_for_person(person: Optional[Person]):
     """Get a queryset of IETF groups suitable for LS To/From assignment by person"""
-    if person is None or has_role(
+    if person is None:
+        return Group.objects.none()  # no person = no roles
+
+    if has_role(
         person.user,
         ("Secretariat", "IETF Chair", "IAB Chair", "Liaison Manager"),  # todo liaison coordinator as well
     ):
@@ -137,7 +139,7 @@ def external_groups_for_person(person):
         # The person cannot add all external sdo groups; add any for which they are Liaison Manager
         filter_expr |= Q(type="sdo", role__person=person, role__name="liaiman")
     return Group.objects.filter(state="active").filter(filter_expr).distinct().order_by("name")
-    
+
 
 def liaison_form_factory(request, type=None, **kwargs):
     """Returns appropriate Liaison entry form"""

From 35573cea4a583cd2d1bae69d3d2d53aba701934b Mon Sep 17 00:00:00 2001
From: Jennifer Richards <jennifer@staff.ietf.org>
Date: Mon, 5 May 2025 15:23:35 -0300
Subject: [PATCH 16/19] test: test_all_internal_groups()

---
 ietf/liaisons/tests_forms.py | 48 +++++++++++++++++++++++++++++++++---
 1 file changed, 45 insertions(+), 3 deletions(-)

diff --git a/ietf/liaisons/tests_forms.py b/ietf/liaisons/tests_forms.py
index da73c9db00..20b7c0a397 100644
--- a/ietf/liaisons/tests_forms.py
+++ b/ietf/liaisons/tests_forms.py
@@ -1,11 +1,14 @@
 # Copyright The IETF Trust 2025, All Rights Reserved
-from ietf.group.factories import GroupFactory
+from ietf.group.factories import GroupFactory, RoleFactory
 from ietf.group.models import Group
 from ietf.liaisons.forms import (
     flatten_choices,
     choices_from_group_queryset,
     all_internal_groups,
+    internal_groups_for_person,
 )
+from ietf.person.factories import PersonFactory
+from ietf.person.models import Person
 from ietf.utils.test_utils import TestCase
 
 
@@ -81,14 +84,53 @@ def test_choices_from_group_queryset(self):
         )
 
     def test_all_internal_groups(self):
-        # relies on the data created in ietf.utils.test_data.make_immutable_test_data()
+        # test relies on the data created in ietf.utils.test_data.make_immutable_test_data()
         self.assertCountEqual(
             all_internal_groups().values_list("acronym", flat=True),
             {"ietf", "iab", "iesg", "farfut", "ops", "sops"},
         )
 
     def test_internal_groups_for_person(self):
-        raise NotImplementedError()
+        # test relies on the data created in ietf.utils.test_data.make_immutable_test_data()
+        # todo add liaison coordinator when modeled
+        # todo ensure that all roles that can add LS have a group to choose from
+        self.assertQuerysetEqual(
+            internal_groups_for_person(None),
+            Group.objects.none(),
+            msg="no Person means no groups",
+        )
+        self.assertQuerysetEqual(
+            internal_groups_for_person(PersonFactory()),
+            Group.objects.none(),
+            msg="no Role means no groups",
+        )
+
+        for username in ("secretary", "ietf-chair", "iab-chair"):
+            returned_queryset = internal_groups_for_person(
+                Person.objects.get(user__username=username)
+            )
+            self.assertCountEqual(
+                returned_queryset.values_list("acronym", flat=True),
+                {"ietf", "iab", "iesg", "farfut", "ops", "sops"},
+                f"{username} should get all groups",
+            )
+
+        # "ops-ad" user is the AD of the "ops" area, which contains the "sops" wg
+        self.assertCountEqual(
+            internal_groups_for_person(
+                Person.objects.get(user__username="ops-ad")
+            ).values_list("acronym", flat=True),
+            {"ietf", "iesg", "ops", "sops"},
+            "area director should get only their area, its wgs, and the ietf/iesg groups",
+        )
+
+        self.assertCountEqual(
+            internal_groups_for_person(
+                Person.objects.get(user__username="sopschairman"),
+            ).values_list("acronym", flat=True),
+            {"sops"},
+            "wg chair should get only their wg",
+        )
 
     def test_external_groups_for_person(self):
         raise NotImplementedError()

From fe1ed3a19302059fef9aa7e2445639ed0e0d9d0e Mon Sep 17 00:00:00 2001
From: Jennifer Richards <jennifer@staff.ietf.org>
Date: Mon, 5 May 2025 15:47:24 -0300
Subject: [PATCH 17/19] test: test_external_groups_for_person()

---
 ietf/liaisons/tests_forms.py | 39 +++++++++++++++++++++++++++++++++++-
 1 file changed, 38 insertions(+), 1 deletion(-)

diff --git a/ietf/liaisons/tests_forms.py b/ietf/liaisons/tests_forms.py
index 20b7c0a397..c9e7ce5ccb 100644
--- a/ietf/liaisons/tests_forms.py
+++ b/ietf/liaisons/tests_forms.py
@@ -6,6 +6,7 @@
     choices_from_group_queryset,
     all_internal_groups,
     internal_groups_for_person,
+    external_groups_for_person,
 )
 from ietf.person.factories import PersonFactory
 from ietf.person.models import Person
@@ -133,7 +134,43 @@ def test_internal_groups_for_person(self):
         )
 
     def test_external_groups_for_person(self):
-        raise NotImplementedError()
+        liaison_manager = RoleFactory(
+            name_id="liaiman", group__type_id="sdo", group__acronym="the-sdo"
+        ).person
+        other_sdo = GroupFactory(acronym="other-sdo", type_id="sdo")
+        for username in (
+            "secretary",
+            "ietf-chair",
+            "iab-chair",
+            "ad",
+            "sopschairman",
+            "sopssecretary",
+        ):
+            person = Person.objects.get(user__username=username)
+            self.assertCountEqual(
+                external_groups_for_person(
+                    person,
+                ).values_list("acronym", flat=True),
+                {"the-sdo", "other-sdo"},
+                f"{username} should get all SDO groups",
+            )
+            tmp_role = RoleFactory(name_id="chair", group__type_id="wg", person=person)
+            self.assertCountEqual(
+                external_groups_for_person(
+                    person,
+                ).values_list("acronym", flat=True),
+                {"the-sdo", "other-sdo"},
+                f"{username} should still get all SDO groups when they also a liaison manager",
+            )
+            tmp_role.delete()
+
+        self.assertCountEqual(
+            external_groups_for_person(liaison_manager).values_list(
+                "acronym", flat=True
+            ),
+            {"the-sdo"},
+            f"{username} should get only their SDO group",
+        )
 
     def test_flatten_choices(self):
         self.assertEqual(flatten_choices([]), [])

From a272ae0926347c4d58fa78ecb31cd458eb8f7062 Mon Sep 17 00:00:00 2001
From: Jennifer Richards <jennifer@staff.ietf.org>
Date: Mon, 5 May 2025 16:26:36 -0300
Subject: [PATCH 18/19] chore: adjust iab execdir and sdo auth'd individs

Need to confirm what is actually needed, but without
these users may be able to create/edit LS's they can't
actually fill in.
---
 ietf/liaisons/forms.py       | 13 ++++++++---
 ietf/liaisons/tests_forms.py | 43 ++++++++++++++++++++++++++++++------
 2 files changed, 46 insertions(+), 10 deletions(-)

diff --git a/ietf/liaisons/forms.py b/ietf/liaisons/forms.py
index dbcc72d4ae..d579011b6c 100644
--- a/ietf/liaisons/forms.py
+++ b/ietf/liaisons/forms.py
@@ -103,7 +103,14 @@ def internal_groups_for_person(person: Optional[Person]):
 
     if has_role(
         person.user,
-        ("Secretariat", "IETF Chair", "IAB Chair", "Liaison Manager"),  # todo liaison coordinator as well
+        (
+                "Secretariat",
+                "IETF Chair",
+                "IAB Chair",
+                "IAB Executive Director",
+                "Liaison Manager",
+                "Authorized Individual",
+        ),  # todo liaison coordinator as well
     ):
         return all_internal_groups()
     # Interesting roles, as Group queries
@@ -133,11 +140,11 @@ def external_groups_for_person(person):
     """Get a queryset of external groups suitable for LS To/From assignment by person"""
     filter_expr = Q(pk__in=[])  # start with no groups
     # These roles can add all external sdo groups
-    if has_role(person.user, set(INCOMING_LIAISON_ROLES + OUTGOING_LIAISON_ROLES) - {"Liaison Manager"}):
+    if has_role(person.user, set(INCOMING_LIAISON_ROLES + OUTGOING_LIAISON_ROLES) - {"Liaison Manager", "Authorized Individual"}):
         filter_expr |= Q(type="sdo")
     else:
         # The person cannot add all external sdo groups; add any for which they are Liaison Manager
-        filter_expr |= Q(type="sdo", role__person=person, role__name="liaiman")
+        filter_expr |= Q(type="sdo", role__person=person, role__name__in=["auth", "liaiman"])
     return Group.objects.filter(state="active").filter(filter_expr).distinct().order_by("name")
 
 
diff --git a/ietf/liaisons/tests_forms.py b/ietf/liaisons/tests_forms.py
index c9e7ce5ccb..965c4f82db 100644
--- a/ietf/liaisons/tests_forms.py
+++ b/ietf/liaisons/tests_forms.py
@@ -94,7 +94,18 @@ def test_all_internal_groups(self):
     def test_internal_groups_for_person(self):
         # test relies on the data created in ietf.utils.test_data.make_immutable_test_data()
         # todo add liaison coordinator when modeled
-        # todo ensure that all roles that can add LS have a group to choose from
+        RoleFactory(
+            name_id="execdir",
+            group=Group.objects.get(acronym="iab"),
+            person__user__username="iab-execdir",
+        )
+        RoleFactory(
+            name_id="auth",
+            group__type_id="sdo",
+            group__acronym="sdo",
+            person__user__username="sdo-authperson",
+        )
+
         self.assertQuerysetEqual(
             internal_groups_for_person(None),
             Group.objects.none(),
@@ -106,7 +117,13 @@ def test_internal_groups_for_person(self):
             msg="no Role means no groups",
         )
 
-        for username in ("secretary", "ietf-chair", "iab-chair"):
+        for username in (
+            "secretary",
+            "ietf-chair",
+            "iab-chair",
+            "iab-execdir",
+            "sdo-authperson",
+        ):
             returned_queryset = internal_groups_for_person(
                 Person.objects.get(user__username=username)
             )
@@ -134,14 +151,21 @@ def test_internal_groups_for_person(self):
         )
 
     def test_external_groups_for_person(self):
-        liaison_manager = RoleFactory(
-            name_id="liaiman", group__type_id="sdo", group__acronym="the-sdo"
-        ).person
-        other_sdo = GroupFactory(acronym="other-sdo", type_id="sdo")
+        RoleFactory(
+            name_id="execdir",
+            group=Group.objects.get(acronym="iab"),
+            person__user__username="iab-execdir",
+        )
+        the_sdo = GroupFactory(type_id="sdo", acronym="the-sdo")
+        liaison_manager = RoleFactory(name_id="liaiman", group=the_sdo).person
+        authperson = RoleFactory(name_id="auth", group=the_sdo).person
+
+        GroupFactory(acronym="other-sdo", type_id="sdo")
         for username in (
             "secretary",
             "ietf-chair",
             "iab-chair",
+            "iab-execdir",
             "ad",
             "sopschairman",
             "sopssecretary",
@@ -169,7 +193,12 @@ def test_external_groups_for_person(self):
                 "acronym", flat=True
             ),
             {"the-sdo"},
-            f"{username} should get only their SDO group",
+            f"liaison manager should get only their SDO group",
+        )
+        self.assertCountEqual(
+            external_groups_for_person(authperson).values_list("acronym", flat=True),
+            {"the-sdo"},
+            f"authorized individual should get only their SDO group",
         )
 
     def test_flatten_choices(self):

From 3dff1eeb0225549f92bc0bb6ceb5fa91183e01ee Mon Sep 17 00:00:00 2001
From: Robert Sparks <rjsparks@nostrum.com>
Date: Fri, 23 May 2025 14:32:03 -0500
Subject: [PATCH 19/19] fix: f-string flakes

---
 ietf/liaisons/tests_forms.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ietf/liaisons/tests_forms.py b/ietf/liaisons/tests_forms.py
index 965c4f82db..512cf40d07 100644
--- a/ietf/liaisons/tests_forms.py
+++ b/ietf/liaisons/tests_forms.py
@@ -193,12 +193,12 @@ def test_external_groups_for_person(self):
                 "acronym", flat=True
             ),
             {"the-sdo"},
-            f"liaison manager should get only their SDO group",
+            "liaison manager should get only their SDO group",
         )
         self.assertCountEqual(
             external_groups_for_person(authperson).values_list("acronym", flat=True),
             {"the-sdo"},
-            f"authorized individual should get only their SDO group",
+            "authorized individual should get only their SDO group",
         )
 
     def test_flatten_choices(self):