From 12dff5c210ac6edb82e70b93664c48e792868dcd Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Fri, 10 Jun 2022 17:36:03 -0300 Subject: [PATCH 1/7] fix: only use non-empty Q object as interim group filter --- ietf/meeting/forms.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/ietf/meeting/forms.py b/ietf/meeting/forms.py index 3785cee4ab..9e5cbbf1e4 100644 --- a/ietf/meeting/forms.py +++ b/ietf/meeting/forms.py @@ -201,8 +201,13 @@ def set_group_options(self): q_objects.add(Q(type="rg", state__in=("active", "proposed"), role__person=self.person, role__name="chair"), Q.OR) if has_role(self.user, "Program Lead") or has_role(self.user, "Program Chair"): q_objects.add(Q(type="program", state__in=("active", "proposed"), role__person=self.person, role__name__in=["chair", "lead"]), Q.OR) - - queryset = Group.objects.filter(q_objects).distinct().order_by('acronym') + + # Only apply q_objects if it is not empty, otherwise filter(Q()) passes everything through! + if q_objects: + queryset = Group.objects.filter(q_objects).distinct().order_by('acronym') + else: + queryset = Group.objects.none() + self.fields['group'].queryset = queryset # if there's only one possibility make it the default From 88869d42f2e56a68aa2d5c5ea5c84c33544a04b7 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Mon, 13 Jun 2022 17:26:53 -0300 Subject: [PATCH 2/7] refactor: add with_meetings queryset to GroupManager --- ietf/group/models.py | 4 ++++ ietf/meeting/forms.py | 5 +---- ietf/meeting/tests_views.py | 4 ++-- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/ietf/group/models.py b/ietf/group/models.py index 7deb0b3924..40d5954a4c 100644 --- a/ietf/group/models.py +++ b/ietf/group/models.py @@ -110,6 +110,10 @@ def active_wgs(self): def closed_wgs(self): return self.wgs().exclude(state__in=Group.ACTIVE_STATE_IDS) + def with_meetings(self): + return self.get_queryset().filter(type__features__has_meetings=True) + + class Group(GroupInfo): objects = GroupManager() diff --git a/ietf/meeting/forms.py b/ietf/meeting/forms.py index 9e5cbbf1e4..42cd16e5c9 100644 --- a/ietf/meeting/forms.py +++ b/ietf/meeting/forms.py @@ -106,10 +106,7 @@ def clean(self): class InterimMeetingModelForm(forms.ModelForm): group = GroupModelChoiceField( - queryset=Group.objects.filter( - type_id__in=GroupFeatures.objects.filter( - has_meetings=True - ).values_list('type_id',flat=True), + queryset=Group.objects.with_meetings().filter( state__in=('active', 'proposed', 'bof') ).order_by('acronym'), required=False, diff --git a/ietf/meeting/tests_views.py b/ietf/meeting/tests_views.py index 5c35da300e..f7299f3d6a 100644 --- a/ietf/meeting/tests_views.py +++ b/ietf/meeting/tests_views.py @@ -4612,8 +4612,8 @@ def test_interim_request_options(self): r = self.client.get("/meeting/interim/request/") self.assertEqual(r.status_code, 200) q = PyQuery(r.content) - Group.objects.filter(type_id__in=GroupFeatures.objects.filter(has_meetings=True).values_list('type_id',flat=True), state__in=('active', 'proposed', 'bof')) - self.assertEqual(Group.objects.filter(type_id__in=GroupFeatures.objects.filter(has_meetings=True).values_list('type_id',flat=True), state__in=('active', 'proposed', 'bof')).count(), + Group.objects.has_meetings().filter(state__in=('active', 'proposed', 'bof')) + self.assertEqual(Group.objects.has_meetings().filter(state__in=('active', 'proposed', 'bof')).count(), len(q("#id_group option")) - 1) # -1 for options placeholder self.client.logout() From 9796ae9815c43270f2f14484ef07ae9083c5b218 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Mon, 13 Jun 2022 13:19:12 -0300 Subject: [PATCH 3/7] test: users can only request interims for managed groups --- ietf/meeting/tests_forms.py | 18 ++++++++++++++++-- ietf/meeting/tests_views.py | 6 ++++-- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/ietf/meeting/tests_forms.py b/ietf/meeting/tests_forms.py index eb9b5049aa..9ae1c413aa 100644 --- a/ietf/meeting/tests_forms.py +++ b/ietf/meeting/tests_forms.py @@ -3,9 +3,12 @@ """Tests of forms in the Meeting application""" from django.conf import settings from django.core.files.uploadedfile import SimpleUploadedFile -from django.test import override_settings +from django.test import override_settings, RequestFactory -from ietf.meeting.forms import FileUploadForm, ApplyToAllFileUploadForm, InterimSessionModelForm +from ietf.group.factories import GroupFactory, RoleFactory +from ietf.meeting.forms import (FileUploadForm, ApplyToAllFileUploadForm, InterimSessionModelForm, + InterimMeetingModelForm) +from ietf.person.factories import PersonFactory from ietf.utils.test_utils import TestCase @@ -119,3 +122,14 @@ def test_remote_participation_options(self): choice_vals = [choice[0] for choice in form.fields['remote_participation'].choices] self.assertNotIn('meetecho', choice_vals) self.assertIn('manual', choice_vals) + + +class InterimMeetingModelFormTests(TestCase): + def test_enforces_authroles(self): + """User can only request sessions for groups they can manage""" + GroupFactory(type_id='wg', state_id='active') + request = RequestFactory().get('/some/url') + request.user = PersonFactory().user + form = InterimMeetingModelForm(request) + self.assertEqual(form.fields['group'].queryset.count(), 0, + 'person with no roles cannot request interims for any group') diff --git a/ietf/meeting/tests_views.py b/ietf/meeting/tests_views.py index f7299f3d6a..80d24793ef 100644 --- a/ietf/meeting/tests_views.py +++ b/ietf/meeting/tests_views.py @@ -4612,8 +4612,8 @@ def test_interim_request_options(self): r = self.client.get("/meeting/interim/request/") self.assertEqual(r.status_code, 200) q = PyQuery(r.content) - Group.objects.has_meetings().filter(state__in=('active', 'proposed', 'bof')) - self.assertEqual(Group.objects.has_meetings().filter(state__in=('active', 'proposed', 'bof')).count(), + Group.objects.with_meetings().filter(state__in=('active', 'proposed', 'bof')) + self.assertEqual(Group.objects.with_meetings().filter(state__in=('active', 'proposed', 'bof')).count(), len(q("#id_group option")) - 1) # -1 for options placeholder self.client.logout() @@ -6504,8 +6504,10 @@ def test_can_request_interim(self): url = urlreverse('ietf.meeting.views.interim_request') for gf in GroupFeatures.objects.filter(has_meetings=True): + debug.show('gf') meeting_count = 0 for role in gf.groupman_roles: + debug.show('role') role = RoleFactory(group__type_id=gf.type_id, name_id=role) self.do_request_interim(url, role.group, role.person.user, meeting_count) for authrole in gf.groupman_authroles: From cbf4e6a69585201556b58ddd79a5ef694aa27897 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Wed, 15 Jun 2022 12:37:03 -0300 Subject: [PATCH 4/7] fix: find managed groups from groupman_roles/authroles --- ietf/group/utils.py | 22 ++++++++++++++++++++++ ietf/meeting/forms.py | 34 +++++++++------------------------- 2 files changed, 31 insertions(+), 25 deletions(-) diff --git a/ietf/group/utils.py b/ietf/group/utils.py index e6628ca41e..8a5fd37288 100644 --- a/ietf/group/utils.py +++ b/ietf/group/utils.py @@ -120,6 +120,28 @@ def can_manage_group(user, group): return True return group.has_role(user, group.features.groupman_roles) +def groups_managed_by(user, group_queryset=None): + """Find groups user can manage""" + if group_queryset is None: + group_queryset = Group.objects.all() + query_terms = Q(pk__in=[]) # ensure empty set is returned if no other terms are added + if user.is_authenticated or user.person: + # find the GroupTypes entirely managed by this user based on groupman_authroles + types_can_manage = [] + for type_id, groupman_authroles in GroupFeatures.objects.values_list('type_id', 'groupman_authroles'): + if has_role(user, groupman_authroles): + types_can_manage.append(type_id) + query_terms |= Q(type_id__in=types_can_manage) + # find the Groups managed by this user based on groupman_roles + groups_can_manage = [] + for group_id, role_name, groupman_roles in user.person.role_set.values_list( + 'group_id', 'name_id', 'group__type__features__groupman_roles' + ): + if role_name in groupman_roles: + groups_can_manage.append(group_id) + query_terms |= Q(pk__in=groups_can_manage) + return group_queryset.filter(query_terms) + def milestone_reviewer_for_group_type(group_type): if group_type == "rg": return "IRTF Chair" diff --git a/ietf/meeting/forms.py b/ietf/meeting/forms.py index 42cd16e5c9..04885cfc8b 100644 --- a/ietf/meeting/forms.py +++ b/ietf/meeting/forms.py @@ -14,15 +14,14 @@ from django.conf import settings from django.core import validators from django.core.exceptions import ValidationError -from django.db.models import Q from django.forms import BaseInlineFormSet from django.utils.functional import cached_property import debug # pyflakes:ignore from ietf.doc.models import Document, DocAlias, State, NewRevisionDocEvent -from ietf.group.models import Group, GroupFeatures -from ietf.ietfauth.utils import has_role +from ietf.group.models import Group +from ietf.group.utils import groups_managed_by from ietf.meeting.models import Session, Meeting, Schedule, countries, timezones, TimeSlot, Room from ietf.meeting.helpers import get_next_interim_number, make_materials_directories from ietf.meeting.helpers import is_interim_meeting_approved, get_next_agenda_name @@ -184,29 +183,14 @@ def is_virtual(self): return True def set_group_options(self): - '''Set group options based on user accessing the form''' - if has_role(self.user, "Secretariat"): - return # don't reduce group options - q_objects = Q() - if has_role(self.user, "Area Director"): - q_objects.add(Q(type__in=["wg", "ag", "team"], state__in=("active", "proposed", "bof")), Q.OR) - if has_role(self.user, "IRTF Chair"): - q_objects.add(Q(type__in=["rg", "rag"], state__in=("active", "proposed")), Q.OR) - if has_role(self.user, "WG Chair"): - q_objects.add(Q(type="wg", state__in=("active", "proposed", "bof"), role__person=self.person, role__name="chair"), Q.OR) - if has_role(self.user, "RG Chair"): - q_objects.add(Q(type="rg", state__in=("active", "proposed"), role__person=self.person, role__name="chair"), Q.OR) - if has_role(self.user, "Program Lead") or has_role(self.user, "Program Chair"): - q_objects.add(Q(type="program", state__in=("active", "proposed"), role__person=self.person, role__name__in=["chair", "lead"]), Q.OR) - - # Only apply q_objects if it is not empty, otherwise filter(Q()) passes everything through! - if q_objects: - queryset = Group.objects.filter(q_objects).distinct().order_by('acronym') - else: - queryset = Group.objects.none() - + """Set group options based on user accessing the form""" + queryset = groups_managed_by( + self.user, + Group.objects.with_meetings(), + ).filter( + state_id__in=['active', 'proposed', 'bof'] + ).order_by('acronym') self.fields['group'].queryset = queryset - # if there's only one possibility make it the default if len(queryset) == 1: self.fields['group'].initial = queryset[0] From 69313b35528957a4942fee4f57a193b9d933b699 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Wed, 15 Jun 2022 12:37:58 -0300 Subject: [PATCH 5/7] feat: let chair manage directorate groups --- .../0056_dir_chair_groupman_role.py | 30 +++++++++++++++++++ ietf/name/fixtures/names.json | 2 +- 2 files changed, 31 insertions(+), 1 deletion(-) create mode 100644 ietf/group/migrations/0056_dir_chair_groupman_role.py diff --git a/ietf/group/migrations/0056_dir_chair_groupman_role.py b/ietf/group/migrations/0056_dir_chair_groupman_role.py new file mode 100644 index 0000000000..b69eb03ae8 --- /dev/null +++ b/ietf/group/migrations/0056_dir_chair_groupman_role.py @@ -0,0 +1,30 @@ +# Generated by Django 2.2.28 on 2022-06-14 13:14 + +from django.db import migrations + + +def forward(apps, schema_editor): + GroupFeatures = apps.get_model('group', 'GroupFeatures') + features = GroupFeatures.objects.get(type_id='dir') + if 'chair' not in features.groupman_roles: + features.groupman_roles.append('chair') + features.save() + + +def reverse(apps, schema_editor): + GroupFeatures = apps.get_model('group', 'GroupFeatures') + features = GroupFeatures.objects.get(type_id='dir') + if 'chair' in features.groupman_roles: + features.groupman_roles.remove('chair') + features.save() + + +class Migration(migrations.Migration): + + dependencies = [ + ('group', '0055_editorial_stream'), + ] + + operations = [ + migrations.RunPython(forward, reverse), + ] diff --git a/ietf/name/fixtures/names.json b/ietf/name/fixtures/names.json index 260e40da53..59d23a419b 100644 --- a/ietf/name/fixtures/names.json +++ b/ietf/name/fixtures/names.json @@ -2757,7 +2757,7 @@ "default_used_roles": "[\n \"ad\",\n \"chair\",\n \"reviewer\",\n \"secr\",\n \"delegate\"\n]", "docman_roles": "[\n \"chair\"\n]", "groupman_authroles": "[\n \"Secretariat\"\n]", - "groupman_roles": "[\n \"ad\",\n \"secr\",\n \"delegate\"\n]", + "groupman_roles": "[\n \"ad\",\n \"secr\",\n \"delegate\",\n \"chair\"\n]", "has_chartering_process": false, "has_default_jabber": false, "has_documents": false, From b2b41a710aeae068fac8b6ed9b86607aa370ae94 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Wed, 15 Jun 2022 13:01:16 -0300 Subject: [PATCH 6/7] test: remove debug statements and unused imports --- ietf/meeting/tests_forms.py | 2 +- ietf/meeting/tests_views.py | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/ietf/meeting/tests_forms.py b/ietf/meeting/tests_forms.py index 9ae1c413aa..4a06d77867 100644 --- a/ietf/meeting/tests_forms.py +++ b/ietf/meeting/tests_forms.py @@ -5,7 +5,7 @@ from django.core.files.uploadedfile import SimpleUploadedFile from django.test import override_settings, RequestFactory -from ietf.group.factories import GroupFactory, RoleFactory +from ietf.group.factories import GroupFactory from ietf.meeting.forms import (FileUploadForm, ApplyToAllFileUploadForm, InterimSessionModelForm, InterimMeetingModelForm) from ietf.person.factories import PersonFactory diff --git a/ietf/meeting/tests_views.py b/ietf/meeting/tests_views.py index 80d24793ef..f34f2c94eb 100644 --- a/ietf/meeting/tests_views.py +++ b/ietf/meeting/tests_views.py @@ -6504,10 +6504,8 @@ def test_can_request_interim(self): url = urlreverse('ietf.meeting.views.interim_request') for gf in GroupFeatures.objects.filter(has_meetings=True): - debug.show('gf') meeting_count = 0 for role in gf.groupman_roles: - debug.show('role') role = RoleFactory(group__type_id=gf.type_id, name_id=role) self.do_request_interim(url, role.group, role.person.user, meeting_count) for authrole in gf.groupman_authroles: From 6bcbcbb16c801ea0484649f2d82eb170cfcfee11 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Mon, 20 Jun 2022 14:09:22 -0300 Subject: [PATCH 7/7] test: remove do-nothing code from test --- ietf/meeting/tests_views.py | 1 - 1 file changed, 1 deletion(-) diff --git a/ietf/meeting/tests_views.py b/ietf/meeting/tests_views.py index f34f2c94eb..020dc64599 100644 --- a/ietf/meeting/tests_views.py +++ b/ietf/meeting/tests_views.py @@ -4612,7 +4612,6 @@ def test_interim_request_options(self): r = self.client.get("/meeting/interim/request/") self.assertEqual(r.status_code, 200) q = PyQuery(r.content) - Group.objects.with_meetings().filter(state__in=('active', 'proposed', 'bof')) self.assertEqual(Group.objects.with_meetings().filter(state__in=('active', 'proposed', 'bof')).count(), len(q("#id_group option")) - 1) # -1 for options placeholder self.client.logout()