diff --git a/ietf/bin/set_admin_permissions b/ietf/bin/set_admin_permissions index 79560ce2f..0ecc0737e 100755 --- a/ietf/bin/set_admin_permissions +++ b/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', diff --git a/ietf/group/migrations/0040_lengthen_used_roles_fields.py b/ietf/group/migrations/0040_lengthen_used_roles_fields.py new file mode 100644 index 000000000..d33310495 --- /dev/null +++ b/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), + # ), + ] diff --git a/ietf/group/migrations/0041_create_liaison_contact_roles.py b/ietf/group/migrations/0041_create_liaison_contact_roles.py new file mode 100644 index 000000000..0194cc077 --- /dev/null +++ b/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), + ] diff --git a/ietf/group/migrations/0042_add_liaison_contact_roles_to_used_roles.py b/ietf/group/migrations/0042_add_liaison_contact_roles_to_used_roles.py new file mode 100644 index 000000000..2b16d7485 --- /dev/null +++ b/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), + ] diff --git a/ietf/group/models.py b/ietf/group/models.py index 02c860708..b693371cb 100644 --- a/ietf/group/models.py +++ b/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",]) diff --git a/ietf/group/tests_info.py b/ietf/group/tests_info.py index ea848a183..e2d162976 100644 --- a/ietf/group/tests_info.py +++ b/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): + 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) + r = self.client.get(url) + self.assertEqual(r.status_code, 200) + + q = PyQuery(r.content) + 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, {field_name: field_content}) + self.assertEqual(r.status_code, 302) + + # + 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 name - url = urlreverse('ietf.group.views.edit', kwargs=dict(acronym=group.acronym, action="edit", 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) - # edit info - r = self.client.post(url, dict(name="Mars Not Special Interest Group")) - self.assertEqual(r.status_code, 302) - # - group = Group.objects.get(acronym="mars") - self.assertEqual(group.name, "Mars Not Special Interest Group") - - - # 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')) diff --git a/ietf/liaisons/admin.py b/ietf/liaisons/admin.py index 8159e4866..bedb1d69f 100644 --- a/ietf/liaisons/admin.py +++ b/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) diff --git a/ietf/liaisons/migrations/0008_purge_liaisonstatementgroupcontacts_data.py b/ietf/liaisons/migrations/0008_purge_liaisonstatementgroupcontacts_data.py new file mode 100644 index 000000000..4bec1a4f0 --- /dev/null +++ b/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), + ] diff --git a/ietf/liaisons/migrations/0009_delete_liaisonstatementgroupcontacts_model.py b/ietf/liaisons/migrations/0009_delete_liaisonstatementgroupcontacts_model.py new file mode 100644 index 000000000..c4f654f1d --- /dev/null +++ b/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', + ), + ] diff --git a/ietf/liaisons/models.py b/ietf/liaisons/models.py index 66d817606..6302bea77 100644 --- a/ietf/liaisons/models.py +++ b/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) diff --git a/ietf/liaisons/resources.py b/ietf/liaisons/resources.py index 6ca4188ba..8f31ea3a6 100644 --- a/ietf/liaisons/resources.py +++ b/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): diff --git a/ietf/liaisons/tests.py b/ietf/liaisons/tests.py index c149abd38..7c7d5b8fe 100644 --- a/ietf/liaisons/tests.py +++ b/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() diff --git a/ietf/liaisons/views.py b/ietf/liaisons/views.py index 676a302e4..a8e80a519 100644 --- a/ietf/liaisons/views.py +++ b/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': diff --git a/ietf/name/fixtures/names.json b/ietf/name/fixtures/names.json index de8acbe19..592711472 100644 --- a/ietf/name/fixtures/names.json +++ b/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": "", diff --git a/ietf/name/migrations/0022_add_liaison_contact_rolenames.py b/ietf/name/migrations/0022_add_liaison_contact_rolenames.py new file mode 100644 index 000000000..e5edb2399 --- /dev/null +++ b/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), + ] diff --git a/ietf/secr/groups/views.py b/ietf/secr/groups/views.py index 7f72dce43..16b41a55a 100644 --- a/ietf/secr/groups/views.py +++ b/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)