Use Roles instead of dedicated model for liaison statement group contacts. Fixes #3100. Commit ready for merge.

- Legacy-Id: 18828

ietf/bin/set_admin_permissions

@@ -61,8 +61,6 @@ def main():
              'group.add_groupevent','group.change_groupevent','group.delete_groupevent',
              'iesg.add_telechatagendaitem','iesg.change_telechatagendaitem','iesg.delete_telechatagendaitem',
              'iesg.add_telechatdate','iesg.change_telechatdate','iesg.delete_telechatdate',
-             'liaisons.add_liaisonstatementgroupcontacts','liaisons.change_liaisonstatementgroupcontacts',
-             'liaisons.delete_liaisonstatementgroupcontacts',
              'mailinglists.add_list','mailinglists.change_list','mailinglists.delete_list',
              'mailtrigger.add_mailtrigger','mailtrigger.change_mailtrigger','mailtrigger.delete_mailtrigger',
              'mailtrigger.add_recipient','mailtrigger.change_recipient','mailtrigger.delete_recipient',

ietf/group/migrations/0040_lengthen_used_roles_fields.py

@@ -0,0 +1,35 @@
+# Generated by Django 2.2.17 on 2020-12-11 08:48
+
+from django.db import migrations
+import jsonfield.fields
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('group', '0039_remove_historicalgroupfeatures'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='group',
+            name='used_roles',
+            field=jsonfield.fields.JSONField(blank=True, default=[], help_text="Leave an empty list to get the group_type's default used roles", max_length=256),
+        ),
+        migrations.AlterField(
+            model_name='groupfeatures',
+            name='default_used_roles',
+            field=jsonfield.fields.JSONField(default=[], max_length=256),
+        ),
+        migrations.AlterField(
+            model_name='grouphistory',
+            name='used_roles',
+            field=jsonfield.fields.JSONField(blank=True, default=[], help_text="Leave an empty list to get the group_type's default used roles", max_length=256),
+        ),
+        # historicalgroupfeatures has been removed
+        # migrations.AlterField(
+        #     model_name='historicalgroupfeatures',
+        #     name='default_used_roles',
+        #     field=jsonfield.fields.JSONField(default=[], max_length=256),
+        # ),
+    ]

ietf/group/migrations/0041_create_liaison_contact_roles.py

@@ -0,0 +1,158 @@
+# Generated by Django 2.2.17 on 2020-12-09 06:59
+
+from django.db import migrations
+
+from ietf.person.name import plain_name
+from ietf.utils.mail import parseaddr
+
+
+def find_or_create_email(email_model, person_model, formatted_email, group):
+    """Look up an email address or create if needed
+    
+    Also creates a Person if the email does not have one. Created Email will have
+    the origin field set to the origin parameter to this method.
+    """
+    name, address = parseaddr(formatted_email)
+    if not address:
+        raise ValueError('Could not parse email "%s"' % formatted_email)
+    email, _ = email_model.objects.get_or_create(
+        address=address,
+        defaults=dict(origin='liaison contact: ' + group.acronym)
+    )
+
+    if not email.person:
+        person = person_model.objects.create(name=name if name else address)
+        email.person = person
+        email.save()
+
+    # Display an alert if the formatted address sent from the Role will differ
+    # from what was in the original contacts list
+    if not email.person.plain and email.person.name == email.address:
+        recreated_contact_email = email.address
+    else:
+        person_plain = email.person.plain if email.person.plain else plain_name(email.person.name)
+        recreated_contact_email = "%s <%s>" % (person_plain, email.address)
+    if recreated_contact_email != formatted_email:
+        print('>> Note: address "%s" is now "%s" (%s)' % (
+            formatted_email,
+            recreated_contact_email,
+            group.acronym,
+        ))    
+    return email
+
+
+def forward(apps, schema_editor):
+    """Perform forward migration
+    
+    Creates liaison_contact and liaison_cc_contact Roles corresponding to existing
+    LiaisonStatementGroupContact instances.
+    """
+    Group = apps.get_model('group', 'Group')
+    Role = apps.get_model('group', 'Role')
+    Email = apps.get_model('person', 'Email')
+    Person = apps.get_model('person', 'Person')
+    
+    RoleName = apps.get_model('name', 'RoleName')
+    contact_role_name = RoleName.objects.get(slug='liaison_contact')
+    cc_contact_role_name = RoleName.objects.get(slug='liaison_cc_contact')
+    
+    print()
+    LiaisonStatementGroupContacts = apps.get_model('liaisons', 'LiaisonStatementGroupContacts')
+    for lsgc in LiaisonStatementGroupContacts.objects.all():
+        group = lsgc.group
+        for contact_email in lsgc.contacts.split(','):
+            if contact_email:
+                email = find_or_create_email(Email, Person, 
+                                             contact_email.strip(), 
+                                             group)
+                Role.objects.create(
+                    group=group,
+                    name=contact_role_name,
+                    person=email.person,
+                    email=email,
+                )
+
+        for contact_email in lsgc.cc_contacts.split(','):
+            if contact_email:
+                email = find_or_create_email(Email, Person, 
+                                             contact_email.strip(), 
+                                             group)
+                Role.objects.create(
+                    group=group,
+                    name=cc_contact_role_name,
+                    person=email.person,
+                    email=email,
+                )
+            
+    # Now validate that we got them all. As much as possible, use independent code
+    # to avoid replicating any bugs from the original migration.
+    for group in Group.objects.all():
+        lsgc = LiaisonStatementGroupContacts.objects.filter(group_id=group.pk).first()
+        
+        if not lsgc:
+            if group.role_set.filter(name__in=[contact_role_name, cc_contact_role_name]).exists():
+                raise ValueError('%s group has contact roles after migration but had no LiaisonStatementGroupContacts' % (
+                    group.acronym,
+                ))
+        else:
+            contacts = group.role_set.filter(name=contact_role_name)
+            num_lsgc_contacts = len(lsgc.contacts.split(',')) if lsgc.contacts else 0
+            if len(contacts) != num_lsgc_contacts:
+                raise ValueError(
+                    '%s group has %d contact(s) but only %d address(es) in its LiaisonStatementGroupContacts (contact addresses = "%s", LSGC.contacts="%s")' % (
+                        group.acronym, len(contacts), num_lsgc_contacts,
+                        '","'.join([c.email.address for c in contacts]),
+                        lsgc.contacts,
+                    )
+                )
+            for contact in contacts:
+                email = contact.email.address
+                if email.lower() not in lsgc.contacts.lower():
+                    raise ValueError(
+                        '%s group has "%s" contact but not found in LiaisonStatementGroupContacts.contacts = "%s"' % (
+                            group.acronym, email, lsgc.contacts,
+                        )
+                    )
+
+            cc_contacts = group.role_set.filter(name=cc_contact_role_name)
+            num_lsgc_cc_contacts = len(lsgc.cc_contacts.split(',')) if lsgc.cc_contacts else 0
+            if len(cc_contacts) != num_lsgc_cc_contacts:
+                raise ValueError(
+                    '%s group has %d CC contact(s) but %d address(es) in its LiaisonStatementGroupContacts (cc_contact addresses = "%s", LSGC.cc_contacts="%s")' % (
+                        group.acronym, len(cc_contacts), num_lsgc_cc_contacts,
+                        '","'.join([c.email.address for c in cc_contacts]),
+                        lsgc.cc_contacts,
+                    )
+                )
+            for cc_contact in cc_contacts:
+                email = cc_contact.email.address
+                if email.lower() not in lsgc.cc_contacts.lower():
+                    raise ValueError(
+                        '%s group has "%s" CC contact but not found in LiaisonStatementGroupContacts.cc_contacts = "%s"' % (
+                            group.acronym, email, lsgc.cc_contacts,
+                        )
+                    )
+
+def reverse(apps, schema_editor):
+    """Perform reverse migration
+    
+    Removes liaison_contact and liaison_cc_contact Roles. The forward migration creates missing
+    Email and Person instances, but these are not removed because it's difficult to do this
+    safely and correctly.
+    """
+    Role = apps.get_model('group', 'Role')
+    Role.objects.filter(
+        name_id__in=['liaison_contact', 'liaison_cc_contact']
+    ).delete()
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('group', '0040_lengthen_used_roles_fields'),
+        ('name', '0022_add_liaison_contact_rolenames'),
+    ]
+
+    operations = [
+        migrations.RunPython(forward, reverse),
+    ]

ietf/group/migrations/0042_add_liaison_contact_roles_to_used_roles.py

@@ -0,0 +1,63 @@
+# Generated by Django 2.2.17 on 2020-12-11 08:52
+
+from django.db import migrations
+
+
+def forward(apps, schema_editor):
+    role_names_to_add = ['liaison_contact', 'liaison_cc_contact']
+    
+    Group = apps.get_model('group', 'Group')
+    GroupFeatures = apps.get_model('group', 'GroupFeatures')
+    Role = apps.get_model('group', 'Role')
+    
+    # Add new liaison contact roles to default_used_fields for wg, sdo, and area groups
+    for group_type in ['wg', 'sdo', 'area']:
+        gf = GroupFeatures.objects.get(type_id=group_type)
+        for role_name in role_names_to_add:
+            if role_name not in gf.default_used_roles:
+                gf.default_used_roles.append(role_name)
+        gf.save()
+        
+        # Add new role names to any groups that both have liaison contacts
+        # and use a custom used_roles list.
+        for group in Group.objects.filter(type_id=group_type):
+            used_roles_is_set = len(group.used_roles) > 0
+            has_contacts = Role.objects.filter(name_id__in=role_names_to_add).exists()
+            if used_roles_is_set and has_contacts:
+                for role_name in role_names_to_add:
+                    if role_name not in group.used_roles:
+                        print('>> Adding %s to used_roles for %s' % (role_name, group.acronym))
+                        group.used_roles.append(role_name)
+                group.save()
+
+
+def reverse(apps, schema_editor):
+    role_names_to_remove = ['liaison_contact', 'liaison_cc_contact']
+    
+    Group = apps.get_model('group', 'Group')
+    GroupFeatures = apps.get_model('group', 'GroupFeatures')
+
+    for group in Group.objects.all():
+        for role_name in role_names_to_remove:
+            if role_name in group.used_roles:
+                print('>> Removing %s from used_roles for %s' % (role_name, group.acronym))
+                group.used_roles.remove(role_name)
+                group.save()
+
+    for gf in GroupFeatures.objects.all():
+        for role_name in role_names_to_remove:
+            if role_name in gf.default_used_roles:
+                print('>> Removing %s from default_used_roles for %s' % (role_name, gf.type_id))
+                gf.default_used_roles.remove(role_name)
+                gf.save()
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('group', '0041_create_liaison_contact_roles'),
+    ]
+
+    operations = [
+        migrations.RunPython(forward, reverse),
+    ]

ietf/group/models.py

@@ -45,7 +45,7 @@ class GroupInfo(models.Model):
     unused_states = models.ManyToManyField('doc.State', help_text="Document states that have been disabled for the group.", blank=True)
     unused_tags = models.ManyToManyField(DocTagName, help_text="Document tags that have been disabled for the group.", blank=True)
 
-    used_roles = jsonfield.JSONField(max_length=128, blank=True, default=[], help_text="Leave an empty list to get the group_type's default used roles")
+    used_roles = jsonfield.JSONField(max_length=256, blank=True, default=[], help_text="Leave an empty list to get the group_type's default used roles")
 
     uses_milestone_dates = models.BooleanField(default=True)
 
@@ -272,7 +272,7 @@ class GroupFeatures(models.Model):
     about_page              = models.CharField(max_length=64, blank=False, default="ietf.group.views.group_about" )
     default_tab             = models.CharField(max_length=64, blank=False, default="ietf.group.views.group_about" )
     material_types          = jsonfield.JSONField(max_length=64, blank=False, default=["slides"])
-    default_used_roles      = jsonfield.JSONField(max_length=128, blank=False, default=[])
+    default_used_roles      = jsonfield.JSONField(max_length=256, blank=False, default=[])
     admin_roles             = jsonfield.JSONField(max_length=64, blank=False, default=["chair"]) # Trac Admin
     docman_roles            = jsonfield.JSONField(max_length=128, blank=False, default=["ad","chair","delegate","secr"])
     groupman_roles          = jsonfield.JSONField(max_length=128, blank=False, default=["ad","chair",])

ietf/group/tests_info.py

@@ -31,9 +31,9 @@ from ietf.group.factories import (GroupFactory, RoleFactory, GroupEventFactory,
 from ietf.group.models import Group, GroupEvent, GroupMilestone, GroupStateTransitions, Role
 from ietf.group.utils import save_group_in_history, setup_default_community_list_for_group
 from ietf.meeting.factories import SessionFactory
-from ietf.name.models import DocTagName, GroupStateName, GroupTypeName, ExtResourceName
+from ietf.name.models import DocTagName, GroupStateName, GroupTypeName, ExtResourceName, RoleName
 from ietf.person.models import Person, Email
-from ietf.person.factories import PersonFactory
+from ietf.person.factories import PersonFactory, EmailFactory
 from ietf.review.factories import ReviewRequestFactory, ReviewAssignmentFactory
 from ietf.utils.mail import outbox, empty_outbox, get_payload_text
 from ietf.utils.test_utils import login_testing_unauthorized, TestCase, unicontent, reload_db_objects
@@ -309,7 +309,6 @@ class GroupPagesTests(TestCase):
         for group in test_groups:
 
             for url in [group.about_url(),] + group_urlreverse_list(group, 'ietf.group.views.group_about'):
-                url = group.about_url()
                 r = self.client.get(url)
                 self.assertEqual(r.status_code, 200)
                 self.assertContains(r, group.name)
@@ -324,6 +323,19 @@ class GroupPagesTests(TestCase):
                 for username in list(set(interesting_users)-set(can_edit[group.type_id])):
                     verify_cannot_edit_group(url, group, username)
 
+    def test_group_about_personnel(self):
+        """Correct personnel should appear on the group About page"""
+        group = GroupFactory()
+        for role_name in group.features.default_used_roles:
+            RoleFactory.create_batch(2, group=group, name=RoleName.objects.get(slug=role_name))
+
+        for url in [group.about_url(),] + group_urlreverse_list(group, 'ietf.group.views.group_about'):
+            r = self.client.get(url)
+            self.assertEqual(r.status_code, 200)
+            
+            for role in group.role_set.all():
+                self.assertContains(r, role.person.plain_name())
+
     def test_materials(self):
         group = GroupFactory(type_id="team", acronym="testteam", name="Test Team", state_id="active")
 
@@ -606,6 +618,8 @@ class GroupEditTests(TestCase):
                                   state=state.pk,
                                   chair_roles="aread@example.org, ad1@example.org",
                                   secr_roles="aread@example.org, ad1@example.org, ad2@example.org",
+                                  liaison_contact_roles="ad1@example.org",
+                                  liaison_cc_contact_roles="aread@example.org, ad2@example.org",
                                   techadv_roles="aread@example.org",
                                   delegate_roles="ad2@example.org",
                                   list_email="mars@mail",
@@ -618,8 +632,13 @@ class GroupEditTests(TestCase):
         self.assertEqual(group.name, "Mars Not Special Interest Group")
         self.assertEqual(group.parent, area)
         self.assertEqual(group.ad_role().person, ad)
-        for k in ("chair", "secr", "techadv"):
+        for k in ("chair", "secr", "techadv", "liaison_cc_contact"):
             self.assertTrue(group.role_set.filter(name=k, email__address="aread@example.org"))
+        self.assertTrue(group.role_set.filter(name='liaison_contact', email__address='ad1@example.org'))
+        self.assertFalse(group.role_set.filter(name='liaison_contact', email__address='aread@example.org'))
+        self.assertFalse(group.role_set.filter(name='liaison_contact', email__address='ad2@example.org'))
+        self.assertFalse(group.role_set.filter(name='liaison_cc_contact', email__address='ad1@example.org'))
+        self.assertTrue(group.role_set.filter(name='liaison_cc_contact', email__address='ad2@example.org'))
         self.assertTrue(group.role_set.filter(name="delegate", email__address="ad2@example.org"))
         self.assertEqual(group.list_email, "mars@mail")
         self.assertEqual(group.list_subscribe, "subscribe.mars")
@@ -682,40 +701,47 @@ class GroupEditTests(TestCase):
 
 
     def test_edit_field(self):
-        group = GroupFactory(acronym="mars")
-
-        # Edit name
-        url = urlreverse('ietf.group.views.edit', kwargs=dict(acronym=group.acronym, action="edit", field="name"))
+        def _test_field(group, field_name, field_content, prohibited_form_names):
+            url = urlreverse('ietf.group.views.edit', 
+                             kwargs=dict(
+                                 acronym=group.acronym, 
+                                 action="edit", 
+                                 field=field_name
+                             ))
             login_testing_unauthorized(self, "secretary", url)
-        # normal get
             r = self.client.get(url)
             self.assertEqual(r.status_code, 200)
+
             q = PyQuery(r.content)
-        self.assertEqual(len(q('div#content > form input[name=name]')), 1)
-        self.assertEqual(len(q('form input[name=acronym]')), 0)
+            self.assertEqual(len(q('div#content > form input[name=%s]' % field_name)), 1)
+            for prohibited_name in prohibited_form_names:
+                self.assertEqual(len(q('div#content > form input[name=%s]' % prohibited_name)), 0)
+
             # edit info
-        r = self.client.post(url, dict(name="Mars Not Special Interest Group"))
+            r = self.client.post(url, {field_name: field_content})
             self.assertEqual(r.status_code, 302)
+
             #
-        group = Group.objects.get(acronym="mars")
-        self.assertEqual(group.name, "Mars Not Special Interest Group")
+            group = Group.objects.get(acronym=group.acronym)
+            if field_name.endswith('_roles'):
+                role_name = field_name[:-len('_roles')]
+                self.assertSetEqual(
+                    {fc.strip() for fc in field_content.split(',')},
+                    set(group.role_set.filter(name=role_name).values_list('email', flat=True))
+                )
+            else:
+                self.assertEqual(getattr(group, field_name), field_content)
+            self.client.logout()
 
+        group = GroupFactory(acronym="mars")
+        EmailFactory(address='user@example.com')
+        EmailFactory(address='other_user@example.com')
 
-        # Edit list email
-        url = urlreverse('ietf.group.views.edit', kwargs=dict(group_type=group.type_id, acronym=group.acronym, action="edit", field="list_email"))
-        # normal get
-        r = self.client.get(url)
-        self.assertEqual(r.status_code, 200)
-        q = PyQuery(r.content)
-        self.assertEqual(len(q('div#content > form input[name=list_email]')), 1)
-        self.assertEqual(len(q('div#content > form input[name=name]')), 0)
-        # edit info
-        r = self.client.post(url, dict(list_email="mars@mail"))
-        self.assertEqual(r.status_code, 302)
-        #
-        group = Group.objects.get(acronym="mars")
-        self.assertEqual(group.list_email, "mars@mail")
-
+        # Test various fields
+        _test_field(group, 'name', 'Mars Not Special Interest Group', ['acronym'])
+        _test_field(group, 'list_email', 'mars@mail', ['name'])
+        _test_field(group, 'liaison_contact_roles', 'user@example.com, other_user@example.com', ['list_email'])
+        _test_field(group, 'liaison_cc_contact_roles', 'user@example.com, other_user@example.com', ['liaison_contact'])
 
     def test_edit_reviewers(self):
         group=GroupFactory(type_id='review',parent=GroupFactory(type_id='area'))

ietf/liaisons/admin.py

@@ -6,7 +6,7 @@ from django.contrib import admin
 from django.urls import reverse
 
 from ietf.liaisons.models import  ( LiaisonStatement, LiaisonStatementEvent,
-    LiaisonStatementGroupContacts, RelatedLiaisonStatement, LiaisonStatementAttachment )
+    RelatedLiaisonStatement, LiaisonStatementAttachment )
 
 
 class RelatedLiaisonStatementInline(admin.TabularInline):
@@ -49,12 +49,5 @@ class LiaisonStatementEventAdmin(admin.ModelAdmin):
     search_fields = ["statement__title", "by__name"]
     raw_id_fields = ["statement", "by"]
 
-class LiaisonStatementGroupContactsAdmin(admin.ModelAdmin):
-    list_display = ["group", "contacts"]
-    raw_id_fields = ["group"]
-    search_fields = ["group__acronym", "contacts"]
-    ordering = ["group__name"]
-
 admin.site.register(LiaisonStatement, LiaisonStatementAdmin)
 admin.site.register(LiaisonStatementEvent, LiaisonStatementEventAdmin)
-admin.site.register(LiaisonStatementGroupContacts, LiaisonStatementGroupContactsAdmin)

ietf/liaisons/migrations/0008_purge_liaisonstatementgroupcontacts_data.py

@@ -0,0 +1,62 @@
+# Generated by Django 2.2.17 on 2020-12-10 06:21
+
+from django.db import migrations
+
+from ietf.person.name import plain_name
+
+
+def forward(apps, schema_editor):
+    """Delete LiaisonStatementGroupContacts records"""
+    LiaisonStatementGroupContacts = apps.get_model('liaisons', 'LiaisonStatementGroupContacts')
+    LiaisonStatementGroupContacts.objects.all().delete()
+
+
+def contacts_from_roles(roles):
+    """Create contacts string from Role queryset"""
+    emails = []
+    for r in roles:
+        if not r.person.plain and r.person.name == r.email.address:
+            # Person was just a stand-in for a bare email address, so just return a bare email address
+            emails.append(r.email.address)
+        else:
+            # Person had a name of some sort, use that as the friendly name
+            person_name = r.person.plain if r.person.plain else plain_name(r.person.name)
+            emails.append('{} <{}>'.format(person_name,r.email.address))
+    return ','.join(emails)
+
+def reverse(apps, schema_editor):
+    """Recreate LiaisonStatementGroupContacts records
+    
+    Note that this does not exactly reproduce the original contents. In particular, email addresses
+    in contacts or cc_contacts may have had different real names than those in the corresponding
+    email.person.name field. In this case, the record will be reconstructed with the name from
+    the Person model. The email addresses should be unchanged, though. 
+    """
+    LiaisonStatementGroupContacts = apps.get_model('liaisons', 'LiaisonStatementGroupContacts')
+    Group = apps.get_model('group', 'Group')
+    Role = apps.get_model('group', 'Role')
+    RoleName=apps.get_model('name', 'RoleName')
+    
+    contact_role_name = RoleName.objects.get(slug='liaison_contact')
+    cc_contact_role_name = RoleName.objects.get(slug='liaison_cc_contact')
+    
+    for group in Group.objects.all():
+        contacts = Role.objects.filter(name=contact_role_name, group=group)
+        cc_contacts = Role.objects.filter(name=cc_contact_role_name, group=group)
+        if contacts.exists() or cc_contacts.exists():
+            LiaisonStatementGroupContacts.objects.create(
+                group_id=group.pk,
+                contacts=contacts_from_roles(contacts),
+                cc_contacts=contacts_from_roles(cc_contacts),
+            )
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('liaisons', '0007_auto_20201109_0439'),
+        ('group', '0041_create_liaison_contact_roles'),
+    ]
+
+    operations = [
+        migrations.RunPython(forward, reverse),
+    ]

ietf/liaisons/migrations/0009_delete_liaisonstatementgroupcontacts_model.py

@@ -0,0 +1,16 @@
+# Generated by Django 2.2.17 on 2020-12-10 10:05
+
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('liaisons', '0008_purge_liaisonstatementgroupcontacts_data'),
+    ]
+
+    operations = [
+        migrations.DeleteModel(
+            name='LiaisonStatementGroupContacts',
+        ),
+    ]

ietf/liaisons/models.py

@@ -217,15 +217,6 @@ class RelatedLiaisonStatement(models.Model):
         return "%s %s %s" % (self.source.title, self.relationship.name.lower(), self.target.title)
 
 
-class LiaisonStatementGroupContacts(models.Model):
-    group = ForeignKey(Group, unique=True, null=True)
-    contacts = models.CharField(max_length=255,blank=True)
-    cc_contacts = models.CharField(max_length=255,blank=True)
-
-    def __str__(self):
-        return "%s" % self.group.name
-
-
 class LiaisonStatementEvent(models.Model):
     time = models.DateTimeField(auto_now_add=True)
     type = ForeignKey(LiaisonStatementEventTypeName)

ietf/liaisons/resources.py

@@ -11,8 +11,8 @@ from tastypie.cache import SimpleCache
 
 from ietf import api
 
-from ietf.liaisons.models import (LiaisonStatement, LiaisonStatementGroupContacts,
-    LiaisonStatementEvent, LiaisonStatementAttachment, RelatedLiaisonStatement)
+from ietf.liaisons.models import (LiaisonStatement, LiaisonStatementEvent, LiaisonStatementAttachment,
+                                  RelatedLiaisonStatement)
 
 
 from ietf.person.resources import EmailResource
@@ -56,23 +56,6 @@ class LiaisonStatementResource(ModelResource):
         }
 api.liaisons.register(LiaisonStatementResource())
 
-from ietf.group.resources import GroupResource
-class LiaisonStatementGroupContactsResource(ModelResource):
-    group            = ToOneField(GroupResource, 'group')
-    class Meta:
-        cache = SimpleCache()
-        queryset = LiaisonStatementGroupContacts.objects.all()
-        serializer = api.Serializer()
-        #resource_name = 'liaisonstatementgroupcontacts'
-        ordering = ['id', ]
-        filtering = { 
-            "id": ALL,
-            "contacts": ALL,
-            "cc_contacts": ALL,
-            "group": ALL_WITH_RELATIONS,
-        }
-api.liaisons.register(LiaisonStatementGroupContactsResource())
-
 from ietf.person.resources import PersonResource
 from ietf.name.resources import LiaisonStatementEventTypeNameResource
 class LiaisonStatementEventResource(ModelResource):

ietf/liaisons/tests.py

@@ -23,11 +23,11 @@ from ietf.group.factories import GroupFactory, RoleFactory
 from ietf.liaisons.factories import ( LiaisonStatementFactory, 
     LiaisonStatementEventFactory, LiaisonStatementAttachmentFactory, )
 from ietf.liaisons.models import (LiaisonStatement, LiaisonStatementPurposeName,
-    LiaisonStatementGroupContacts, LiaisonStatementAttachment)
+    LiaisonStatementAttachment)
 from ietf.person.models import Person
 from ietf.group.models import Group
 from ietf.liaisons.mails import send_sdo_reminder, possibly_send_deadline_reminder
-from ietf.liaisons.views import contacts_from_roles
+from ietf.liaisons.views import contacts_from_roles, contact_email_from_role
 
 # -------------------------------------------------
 # Helper Functions
@@ -132,15 +132,20 @@ class UnitTests(TestCase):
         sdo = RoleFactory(name_id='liaiman',group__type_id='sdo',).group
         cc = get_cc(sdo)
         self.assertTrue(contacts_from_roles([sdo.role_set.filter(name='liaiman').first()]) in cc)
+        # test a cc_contact role
+        cc_contact_role = RoleFactory(name_id='liaison_cc_contact', group=sdo)
+        cc = get_cc(sdo)
+        self.assertIn(contact_email_from_role(cc_contact_role), cc)
 
     def test_get_contacts_for_group(self):
         from ietf.liaisons.views import get_contacts_for_group, EMAIL_ALIASES
 
         # test explicit
         sdo = GroupFactory(type_id='sdo')
-        LiaisonStatementGroupContacts.objects.create(group=sdo,contacts='bob@world.com')
+        contact_email = RoleFactory(name_id='liaison_contact', group=sdo).email.address
         contacts = get_contacts_for_group(sdo)
-        self.assertTrue('bob@world.com' in contacts)
+        self.assertIsNotNone(contact_email)
+        self.assertIn(contact_email, contacts)
         # test area
         area = Group.objects.filter(type='area').first()
         contacts = get_contacts_for_group(area)
@@ -204,14 +209,14 @@ class AjaxTests(TestCase):
         area = RoleFactory(name_id='ad',group__type_id='area').group
         group = GroupFactory(parent=area)
         LiaisonStatementFactory(to_groups=[group,])
-        LiaisonStatementGroupContacts.objects.create(group=group,contacts='test@example.com')
+        contact_email = contact_email_from_role(RoleFactory(name_id='liaison_contact', group=group))
 
         url = urlreverse('ietf.liaisons.views.ajax_get_liaison_info') + "?to_groups={}&from_groups=".format(group.pk)
         self.client.login(username="secretary", password="secretary+password")
         r = self.client.get(url)
         self.assertEqual(r.status_code, 200)
         data = r.json()
-        self.assertEqual(data["to_contacts"],['test@example.com'])
+        self.assertEqual(data["to_contacts"],[contact_email])
 
     def test_ajax_select2_search_liaison_statements(self):
         liaison = LiaisonStatementFactory()

ietf/liaisons/views.py

@@ -89,9 +89,12 @@ def _find_person_in_emails(liaison, person):
 
     return False
 
+def contact_email_from_role(role):
+    return '{} <{}>'.format(role.person.plain_name(), role.email.address)
+
 def contacts_from_roles(roles):
     '''Returns contact string for given roles'''
-    emails = [ '{} <{}>'.format(r.person.plain_name(),r.email.address) for r in roles ]
+    emails = [ contact_email_from_role(r) for r in roles ]
     return ','.join(emails)
 
 def get_cc(group):
@@ -111,34 +114,34 @@ def get_cc(group):
     elif group.type_id == 'area':
         emails.append(EMAIL_ALIASES['IETFCHAIR'])
         ad_roles = group.role_set.filter(name='ad')
-        emails.extend([ '{} <{}>'.format(r.person.plain_name(),r.email.address) for r in ad_roles ])
+        emails.extend([ contact_email_from_role(r) for r in ad_roles ])
     elif group.type_id == 'wg':
         ad_roles = group.parent.role_set.filter(name='ad')
-        emails.extend([ '{} <{}>'.format(r.person.plain_name(),r.email.address) for r in ad_roles ])
+        emails.extend([ contact_email_from_role(r) for r in ad_roles ])
         chair_roles = group.role_set.filter(name='chair')
-        emails.extend([ '{} <{}>'.format(r.person.plain_name(),r.email.address) for r in chair_roles ])
+        emails.extend([ contact_email_from_role(r) for r in chair_roles ])
         if group.list_email:
             emails.append('{} Discussion List <{}>'.format(group.name,group.list_email))
     elif group.type_id == 'sdo':
         liaiman_roles = group.role_set.filter(name='liaiman')
-        emails.extend([ '{} <{}>'.format(r.person.plain_name(),r.email.address) for r in liaiman_roles ])
+        emails.extend([ contact_email_from_role(r) for r in liaiman_roles ])
 
     # explicit CCs
-    if group.liaisonstatementgroupcontacts_set.exists() and group.liaisonstatementgroupcontacts_set.first().cc_contacts:
-        emails = emails + group.liaisonstatementgroupcontacts_set.first().cc_contacts.split(',')
+    liaison_cc_roles = group.role_set.filter(name='liaison_cc_contact')
+    emails.extend([ contact_email_from_role(r) for r in liaison_cc_roles ])
 
     return emails
 
 def get_contacts_for_group(group):
     '''Returns default contacts for groups as a comma separated string'''
-    contacts = []
-
     # use explicit default contacts if defined
-    if group.liaisonstatementgroupcontacts_set.first():
-        contacts.append(group.liaisonstatementgroupcontacts_set.first().contacts)
+    explicit_contacts = contacts_from_roles(group.role_set.filter(name='liaison_contact'))
+    if explicit_contacts:
+        return explicit_contacts
 
     # otherwise construct based on group type
-    elif group.type_id == 'area':
+    contacts = []
+    if group.type_id == 'area':
         roles = group.role_set.filter(name='ad')
         contacts.append(contacts_from_roles(roles))
     elif group.type_id == 'wg':

ietf/name/fixtures/names.json

@@ -2579,7 +2579,7 @@
       "custom_group_roles": true,
       "customize_workflow": false,
       "default_tab": "ietf.group.views.group_about",
-      "default_used_roles": "[\n    \"ad\"\n]",
+      "default_used_roles": "[\n    \"ad\",\n    \"liaison_contact\",\n    \"liaison_cc_contact\"\n]",
       "docman_roles": "[\n    \"ad\",\n    \"delegate\",\n    \"secr\"\n]",
       "groupman_authroles": "[\n    \"Secretariat\"\n]",
       "groupman_roles": "[\n    \"ad\"\n]",
@@ -3091,7 +3091,7 @@
       "custom_group_roles": true,
       "customize_workflow": false,
       "default_tab": "ietf.group.views.group_about",
-      "default_used_roles": "[\n    \"liaiman\",\n    \"ceo\",\n    \"coord\",\n    \"auth\",\n    \"chair\"\n]",
+      "default_used_roles": "[\n    \"liaiman\",\n    \"ceo\",\n    \"coord\",\n    \"auth\",\n    \"chair\",\n    \"liaison_contact\",\n    \"liaison_cc_contact\"\n]",
       "docman_roles": "[\n    \"liaiman\",\n    \"matman\"\n]",
       "groupman_authroles": "[\n    \"Secretariat\"\n]",
       "groupman_roles": "[]",
@@ -3155,7 +3155,7 @@
       "custom_group_roles": false,
       "customize_workflow": true,
       "default_tab": "ietf.group.views.group_documents",
-      "default_used_roles": "[\n    \"ad\",\n    \"editor\",\n    \"delegate\",\n    \"secr\",\n    \"chair\",\n    \"matman\",\n    \"techadv\"\n]",
+      "default_used_roles": "[\n    \"ad\",\n    \"editor\",\n    \"delegate\",\n    \"secr\",\n    \"chair\",\n    \"matman\",\n    \"techadv\",\n    \"liaison_contact\",\n    \"liaison_cc_contact\"\n]",
       "docman_roles": "[\n    \"chair\",\n    \"delegate\",\n    \"secr\"\n]",
       "groupman_authroles": "[\n    \"Secretariat\",\n    \"Area Director\"\n]",
       "groupman_roles": "[\n    \"ad\",\n    \"chair\",\n    \"delegate\",\n    \"secr\"\n]",
@@ -11910,6 +11910,26 @@
     "model": "name.rolename",
     "pk": "liaison"
   },
+  {
+    "fields": {
+      "desc": "",
+      "name": "Liaison CC Contact",
+      "order": 0,
+      "used": true
+    },
+    "model": "name.rolename",
+    "pk": "liaison_cc_contact"
+  },
+  {
+    "fields": {
+      "desc": "",
+      "name": "Liaison Contact",
+      "order": 0,
+      "used": true
+    },
+    "model": "name.rolename",
+    "pk": "liaison_contact"
+  },
   {
     "fields": {
       "desc": "",

ietf/name/migrations/0022_add_liaison_contact_rolenames.py

@@ -0,0 +1,41 @@
+# Generated by Django 2.2.17 on 2020-12-09 07:02
+
+from django.db import migrations
+
+
+def forward(apps, schema_editor):
+    """Perform forward migration
+    
+    Adds RoleNames for liaison contacts
+    """
+    RoleName = apps.get_model('name', 'RoleName')
+    RoleName.objects.create(
+        slug='liaison_contact',
+        name='Liaison Contact',
+    )
+    RoleName.objects.create(
+        slug='liaison_cc_contact',
+        name='Liaison CC Contact',
+    )
+    
+
+def reverse(apps, schema_editor):
+    """Perform reverse migration
+    
+    Removes RoleNames for liaison contacts
+    """
+    RoleName = apps.get_model('name', 'RoleName')
+    RoleName.objects.filter(
+        slug__in=['liaison_contact', 'liaison_cc_contact']
+    ).delete()
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('name', '0021_add_ad_appr_draftsubmissionstatename'),
+        ('group', '0040_lengthen_used_roles_fields'),
+    ]
+
+    operations = [
+        migrations.RunPython(forward, reverse),
+    ]

ietf/secr/groups/views.py

@@ -8,6 +8,7 @@ from ietf.ietfauth.utils import role_required
 from ietf.person.models import Person
 from ietf.secr.groups.forms import RoleForm, SearchForm
 from ietf.secr.utils.meeting import get_current_meeting
+from ietf.liaisons.views import contacts_from_roles
 
 # -------------------------------------------------
 # Helper Functions
@@ -42,7 +43,12 @@ def add_legacy_fields(group):
     group.techadvisors = group.role_set.filter(name="techadv")
     group.editors = group.role_set.filter(name="editor")
     group.secretaries = group.role_set.filter(name="secr")
-    group.liaison_contacts = group.liaisonstatementgroupcontacts_set.first()
+    # Note: liaison_contacts is now a dict instead of a model instance with fields. In
+    # templates, the dict can still be accessed using '.contacts' and .cc_contacts', though.
+    group.liaison_contacts = dict(
+        contacts=contacts_from_roles(group.role_set.filter(name='liaison_contact')),
+        cc_contacts=contacts_from_roles(group.role_set.filter(name='liaison_cc_contact')),
+    )
 
     #fill_in_charter_info(group)