diff --git a/ietf/group/utils.py b/ietf/group/utils.py
index dbce474db..8d27507db 100644
--- a/ietf/group/utils.py
+++ b/ietf/group/utils.py
@@ -7,6 +7,7 @@ import os
from django.db.models import Q
from django.shortcuts import get_object_or_404
+from django.utils.html import format_html
from django.utils.safestring import mark_safe
from django.urls import reverse as urlreverse
@@ -15,9 +16,9 @@ import debug # pyflakes:ignore
from ietf.community.models import CommunityList, SearchRule
from ietf.community.utils import reset_name_contains_index_for_rule, can_manage_community_list
from ietf.doc.models import Document, State
-from ietf.group.models import Group, RoleHistory, Role, GroupFeatures
+from ietf.group.models import Group, RoleHistory, Role, GroupFeatures, GroupEvent
from ietf.ietfauth.utils import has_role
-from ietf.name.models import GroupTypeName
+from ietf.name.models import GroupTypeName, RoleName
from ietf.person.models import Email
from ietf.review.utils import can_manage_review_requests_for_team
from ietf.utils import log
@@ -279,4 +280,45 @@ def group_features_role_filter(roles, person, feature):
return roles.none()
q = reduce(lambda a,b:a|b, [ Q(person=person, name__slug__in=getattr(t.features, feature)) for t in group_types ])
return roles.filter(q)
-
+
+
+def group_attribute_change_desc(attr, new, old=None):
+ if old is None:
+ return format_html('{} changed to {}', attr, new)
+ else:
+ return format_html('{} changed to {} from {}', attr, new, old)
+
+
+def update_role_set(group, role_name, new_value, by):
+ """Alter role_set for a group
+
+ Updates the value and creates history events.
+ """
+ if isinstance(role_name, str):
+ role_name = RoleName.objects.get(slug=role_name)
+ new = set(new_value)
+ old = set(r.email for r in group.role_set.filter(name=role_name).distinct().select_related("person"))
+ removed = old - new
+ added = new - old
+ if added or removed:
+ GroupEvent.objects.create(
+ group=group,
+ by=by,
+ type='info_changed',
+ desc=group_attribute_change_desc(
+ role_name.name,
+ ", ".join(sorted(x.get_name() for x in new)),
+ ", ".join(sorted(x.get_name() for x in old)),
+ )
+ )
+
+ group.role_set.filter(name=role_name, email__in=removed).delete()
+ for email in added:
+ group.role_set.create(name=role_name, email=email, person=email.person)
+
+ for e in new:
+ if not e.origin or (e.person.user and e.origin == e.person.user.username):
+ e.origin = "role: %s %s" % (group.acronym, role_name.slug)
+ e.save()
+
+ return added, removed
diff --git a/ietf/group/views.py b/ietf/group/views.py
index fcb3318ae..0cfd38259 100644
--- a/ietf/group/views.py
+++ b/ietf/group/views.py
@@ -77,9 +77,9 @@ from ietf.group.models import ( Group, Role, GroupEvent, GroupStateTransitions,
ChangeStateGroupEvent, GroupFeatures )
from ietf.group.utils import (get_charter_text, can_manage_all_groups_of_type,
milestone_reviewer_for_group_type, can_provide_status_update,
- can_manage_materials,
+ can_manage_materials, group_attribute_change_desc,
construct_group_menu_context, get_group_materials,
- save_group_in_history, can_manage_group,
+ save_group_in_history, can_manage_group, update_role_set,
get_group_or_404, setup_default_community_list_for_group, )
#
from ietf.ietfauth.utils import has_role, is_authorized_in_group
@@ -873,13 +873,6 @@ def group_photos(request, group_type=None, acronym=None):
def edit(request, group_type=None, acronym=None, action="edit", field=None):
"""Edit or create a group, notifying parties as
necessary and logging changes as group events."""
- def desc(attr, new, old):
- entry = "%(attr)s changed to %(new)s from %(old)s"
- if new_group:
- entry = "%(attr)s changed to %(new)s"
-
- return entry % dict(attr=attr, new=new, old=old)
-
def format_resources(resources, fs="\n"):
res = []
for r in resources:
@@ -896,7 +889,11 @@ def edit(request, group_type=None, acronym=None, action="edit", field=None):
return
v = getattr(group, attr)
if clean[attr] != v:
- changes.append((attr, clean[attr], desc(name, clean[attr], v)))
+ changes.append((
+ attr,
+ clean[attr],
+ group_attribute_change_desc(name, clean[attr], v if v else None)
+ ))
setattr(group, attr, clean[attr])
if action == "edit":
@@ -972,47 +969,33 @@ def edit(request, group_type=None, acronym=None, action="edit", field=None):
title = f.label
- new = clean[attr]
- old = Email.objects.filter(role__group=group, role__name=slug).select_related("person")
- if set(new) != set(old):
- changes.append((attr, new, desc(title,
- ", ".join(sorted(x.get_name() for x in new)),
- ", ".join(sorted(x.get_name() for x in old)))))
- group.role_set.filter(name=slug).delete()
- for e in new:
- Role.objects.get_or_create(name_id=slug, email=e, group=group, person=e.person)
- if not e.origin or (e.person.user and e.origin == e.person.user.username):
- e.origin = "role: %s %s" % (group.acronym, slug)
- e.save()
+ added, deleted = update_role_set(group, slug, clean[attr], request.user.person)
+ changed_personnel.update(added | deleted)
+ if added:
+ change_text=title + ' added: ' + ", ".join(x.name_and_email() for x in added)
+ personnel_change_text+=change_text+"\n"
+ if deleted:
+ change_text=title + ' deleted: ' + ", ".join(x.name_and_email() for x in deleted)
+ personnel_change_text+=change_text+"\n"
+
+ today = datetime.date.today()
+ for deleted_email in deleted:
+ # Verify the person doesn't have a separate reviewer role for the group with a different address
+ if not group.role_set.filter(name_id='reviewer',person=deleted_email.person).exists():
+ active_assignments = ReviewAssignment.objects.filter(
+ review_request__team=group,
+ reviewer__person=deleted_email.person,
+ state_id__in=['accepted', 'assigned'],
+ )
+ for assignment in active_assignments:
+ if assignment.review_request.deadline > today:
+ assignment.state_id = 'withdrawn'
+ else:
+ assignment.state_id = 'no-response'
+ # save() will update review_request state to 'requested'
+ # if needed, so that the review can be assigned to someone else
+ assignment.save()
- added = set(new) - set(old)
- deleted = set(old) - set(new)
- if added:
- change_text=title + ' added: ' + ", ".join(x.name_and_email() for x in added)
- personnel_change_text+=change_text+"\n"
- if deleted:
- change_text=title + ' deleted: ' + ", ".join(x.name_and_email() for x in deleted)
- personnel_change_text+=change_text+"\n"
-
- today = datetime.date.today()
- for deleted_email in deleted:
- # Verify the person doesn't have a separate reviewer role for the group with a different address
- if not group.role_set.filter(name_id='reviewer',person=deleted_email.person).exists():
- active_assignments = ReviewAssignment.objects.filter(
- review_request__team=group,
- reviewer__person=deleted_email.person,
- state_id__in=['accepted', 'assigned'],
- )
- for assignment in active_assignments:
- if assignment.review_request.deadline > today:
- assignment.state_id = 'withdrawn'
- else:
- assignment.state_id = 'no-response'
- # save() will update review_request state to 'requested'
- # if needed, so that the review can be assigned to someone else
- assignment.save()
-
- changed_personnel.update(set(old)^set(new))
if personnel_change_text!="":
changed_personnel = [ str(p) for p in changed_personnel ]
@@ -1030,7 +1013,15 @@ def edit(request, group_type=None, acronym=None, action="edit", field=None):
value = parts[1]
display_name = ' '.join(parts[2:]).strip('()')
group.groupextresource_set.create(value=value, name_id=name, display_name=display_name)
- changes.append(('resources', new_resources, desc('Resources', ", ".join(new_resources), ", ".join(old_resources))))
+ changes.append((
+ 'resources',
+ new_resources,
+ group_attribute_change_desc(
+ 'Resources',
+ ", ".join(new_resources),
+ ", ".join(old_resources) if old_resources else None
+ )
+ ))
group.time = datetime.datetime.now()
diff --git a/ietf/nomcom/forms.py b/ietf/nomcom/forms.py
index 642ec5d5f..6e5d3fa45 100644
--- a/ietf/nomcom/forms.py
+++ b/ietf/nomcom/forms.py
@@ -86,9 +86,23 @@ class MultiplePositionNomineeField(forms.MultipleChoiceField, PositionNomineeFie
return result
-class NewEditMembersForm(forms.Form):
+class EditMembersForm(forms.Form):
+ members = SearchableEmailsField(only_users=True, all_emails=True, required=False)
+ liaisons = SearchableEmailsField(only_users=True, all_emails=True, required=False)
+
+ def __init__(self, nomcom, *args, **kwargs):
+ initial = kwargs.setdefault('initial', {})
+ roles = nomcom.group.role_set.filter(
+ name__slug__in=('member', 'liaison')
+ ).order_by('email__person__name').select_related('email')
+ initial['members'] = [
+ r.email for r in roles if r.name.slug == 'member'
+ ]
+ initial['liaisons'] = [
+ r.email for r in roles if r.name.slug =='liaison'
+ ]
+ super().__init__(*args, **kwargs)
- members = SearchableEmailsField(only_users=True,all_emails=True)
class EditNomcomForm(forms.ModelForm):
diff --git a/ietf/nomcom/tests.py b/ietf/nomcom/tests.py
index bca655c51..a498a9e56 100644
--- a/ietf/nomcom/tests.py
+++ b/ietf/nomcom/tests.py
@@ -408,9 +408,14 @@ class NomcomViewsTest(TestCase):
self.client.logout()
- def change_members(self, members):
- members_emails = ','.join(['%s%s' % (member, EMAIL_DOMAIN) for member in members])
- test_data = {'members': members_emails,}
+ def change_members(self, members=None, liaisons=None):
+ test_data = {}
+ if members is not None:
+ members_emails = ','.join(['%s%s' % (member, EMAIL_DOMAIN) for member in members])
+ test_data['members'] = members_emails
+ if liaisons is not None:
+ liaisons_emails = ','.join(['%s%s' % (liaison, EMAIL_DOMAIN) for liaison in liaisons])
+ test_data['liaisons'] = liaisons_emails
self.client.post(self.edit_members_url, test_data)
def test_edit_members_view(self):
@@ -432,6 +437,54 @@ class NomcomViewsTest(TestCase):
self.check_url_status(self.private_index_url, 403)
self.client.logout()
+ def test_edit_members_only_removes_member_roles(self):
+ """Removing a member or liaison should not affect other roles"""
+ # log in and set up members/liaisons lists
+ self.access_chair_url(self.edit_members_url)
+ self.change_members(
+ members=[CHAIR_USER, COMMUNITY_USER],
+ liaisons=[CHAIR_USER, COMMUNITY_USER],
+ )
+ nomcom_group = Group.objects.get(acronym=f'nomcom{self.year}')
+ self.assertCountEqual(
+ nomcom_group.role_set.filter(name='member').values_list('email__address', flat=True),
+ [CHAIR_USER + EMAIL_DOMAIN, COMMUNITY_USER + EMAIL_DOMAIN],
+ )
+ self.assertCountEqual(
+ nomcom_group.role_set.filter(name='liaison').values_list('email__address', flat=True),
+ [CHAIR_USER + EMAIL_DOMAIN, COMMUNITY_USER + EMAIL_DOMAIN],
+ )
+
+ # remove a member who is also a liaison and check that the liaisons list is unchanged
+ self.change_members(
+ members=[COMMUNITY_USER],
+ liaisons=[CHAIR_USER, COMMUNITY_USER],
+ )
+ nomcom_group = Group.objects.get(pk=nomcom_group.pk) # refresh from db
+ self.assertCountEqual(
+ nomcom_group.role_set.filter(name='member').values_list('email__address', flat=True),
+ [COMMUNITY_USER + EMAIL_DOMAIN],
+ )
+ self.assertCountEqual(
+ nomcom_group.role_set.filter(name='liaison').values_list('email__address', flat=True),
+ [CHAIR_USER + EMAIL_DOMAIN, COMMUNITY_USER + EMAIL_DOMAIN],
+ )
+
+ # remove a liaison who is also a member and check that the members list is unchanged
+ self.change_members(
+ members=[COMMUNITY_USER],
+ liaisons=[CHAIR_USER],
+ )
+ nomcom_group = Group.objects.get(pk=nomcom_group.pk) # refresh from db
+ self.assertCountEqual(
+ nomcom_group.role_set.filter(name='member').values_list('email__address', flat=True),
+ [COMMUNITY_USER + EMAIL_DOMAIN],
+ )
+ self.assertCountEqual(
+ nomcom_group.role_set.filter(name='liaison').values_list('email__address', flat=True),
+ [CHAIR_USER + EMAIL_DOMAIN],
+ )
+
def test_edit_nomcom_view(self):
r = self.access_chair_url(self.edit_nomcom_url)
q = PyQuery(r.content)
diff --git a/ietf/nomcom/views.py b/ietf/nomcom/views.py
index 5ddeec24d..f69d65910 100644
--- a/ietf/nomcom/views.py
+++ b/ietf/nomcom/views.py
@@ -22,7 +22,8 @@ from django.utils.encoding import force_bytes, force_text
from ietf.dbtemplate.models import DBTemplate
from ietf.dbtemplate.views import group_template_edit, group_template_show
from ietf.name.models import NomineePositionStateName, FeedbackTypeName
-from ietf.group.models import Group, GroupEvent, Role
+from ietf.group.models import Group, GroupEvent, Role
+from ietf.group.utils import update_role_set
from ietf.message.models import Message
from ietf.nomcom.decorators import nomcom_private_key_required
@@ -31,7 +32,7 @@ from ietf.nomcom.forms import (NominateForm, NominateNewPersonForm, FeedbackForm
PrivateKeyForm, EditNomcomForm, EditNomineeForm,
PendingFeedbackForm, ReminderDatesForm, FullFeedbackFormSet,
FeedbackEmailForm, NominationResponseCommentForm, TopicForm,
- NewEditMembersForm, VolunteerForm, )
+ EditMembersForm, VolunteerForm, )
from ietf.nomcom.models import (Position, NomineePosition, Nominee, Feedback, NomCom, ReminderDates,
FeedbackLastSeen, Topic, TopicFeedbackLastSeen, )
from ietf.nomcom.utils import (get_nomcom_by_year, store_nomcom_private_key, suggest_affiliation,
@@ -1230,18 +1231,14 @@ def edit_members(request, year):
if nomcom.group.state_id=='conclude':
permission_denied(request, 'This nomcom is closed.')
- old_members_email = [r.email for r in nomcom.group.role_set.filter(name='member')]
-
if request.method=='POST':
- form = NewEditMembersForm(data=request.POST)
+ form = EditMembersForm(nomcom, data=request.POST)
if form.is_valid():
- new_members_email = form.cleaned_data['members']
- nomcom.group.role_set.filter( email__in=set(old_members_email)-set(new_members_email) ).delete()
- for email in set(new_members_email)-set(old_members_email):
- nomcom.group.role_set.create(email=email,person=email.person,name_id='member')
+ update_role_set(nomcom.group, 'member', form.cleaned_data['members'], request.user.person)
+ update_role_set(nomcom.group, 'liaison', form.cleaned_data['liaisons'], request.user.person)
return HttpResponseRedirect(reverse('ietf.nomcom.views.private_index',kwargs={'year':year}))
else:
- form = NewEditMembersForm(initial={ 'members' : old_members_email })
+ form = EditMembersForm(nomcom)
return render(request, 'nomcom/new_edit_members.html',
{'nomcom' : nomcom,