diff --git a/ietf/secr/sreq/forms.py b/ietf/secr/sreq/forms.py index 43c622401..ffd9fb9ea 100644 --- a/ietf/secr/sreq/forms.py +++ b/ietf/secr/sreq/forms.py @@ -1,4 +1,4 @@ -# Copyright The IETF Trust 2013-2019, All Rights Reserved +# Copyright The IETF Trust 2013-2020, All Rights Reserved # -*- coding: utf-8 -*- @@ -20,20 +20,24 @@ from ietf.person.fields import SearchablePersonsField NUM_SESSION_CHOICES = (('','--Please select'),('1','1'),('2','2')) # LENGTH_SESSION_CHOICES = (('','--Please select'),('1800','30 minutes'),('3600','1 hour'),('5400','1.5 hours'), ('7200','2 hours'),('9000','2.5 hours')) LENGTH_SESSION_CHOICES = (('','--Please select'),('1800','30 minutes'),('3600','1 hour'),('5400','1.5 hours'), ('7200','2 hours')) -WG_CHOICES = list( Group.objects.filter(type__in=('wg','rg','ag'),state__in=('bof','proposed','active')).values_list('acronym','acronym').order_by('acronym')) # type:ignore -WG_CHOICES.insert(0,('','--Select WG(s)')) # type:ignore # ------------------------------------------------- # Helper Functions # ------------------------------------------------- -def check_conflict(groups): +def allowed_conflicting_groups(): + return Group.objects.filter(type__in=['wg', 'ag', 'rg'], state__in=['bof', 'proposed', 'active']) + +def check_conflict(groups, source_group): ''' Takes a string which is a list of group acronyms. Checks that they are all active groups ''' # convert to python list (allow space or comma separated lists) items = groups.replace(',',' ').split() - active_groups = Group.objects.filter(type__in=('wg','ag','rg'), state__in=('bof','proposed','active')) + active_groups = allowed_conflicting_groups() for group in items: + if group == source_group.acronym: + raise forms.ValidationError("Cannot declare a conflict with the same group: %s" % group) + if not active_groups.filter(acronym=group): raise forms.ValidationError("Invalid or inactive group acronym: %s" % group) @@ -67,28 +71,39 @@ class SessionForm(forms.Form): length_session2 = forms.ChoiceField(choices=LENGTH_SESSION_CHOICES,required=False) length_session3 = forms.ChoiceField(choices=LENGTH_SESSION_CHOICES,required=False) attendees = forms.IntegerField() + # FIXME: it would cleaner to have these be + # ModelMultipleChoiceField, and just customize the widgetry, that + # way validation comes for free conflict1 = forms.CharField(max_length=255,required=False) conflict2 = forms.CharField(max_length=255,required=False) conflict3 = forms.CharField(max_length=255,required=False) comments = forms.CharField(max_length=200,required=False) - wg_selector1 = forms.ChoiceField(choices=WG_CHOICES,required=False) - wg_selector2 = forms.ChoiceField(choices=WG_CHOICES,required=False) - wg_selector3 = forms.ChoiceField(choices=WG_CHOICES,required=False) + wg_selector1 = forms.ChoiceField(choices=[],required=False) + wg_selector2 = forms.ChoiceField(choices=[],required=False) + wg_selector3 = forms.ChoiceField(choices=[],required=False) third_session = forms.BooleanField(required=False) resources = forms.MultipleChoiceField(widget=forms.CheckboxSelectMultiple,required=False) bethere = SearchablePersonsField(label="Must be present", required=False) - def __init__(self, *args, **kwargs): + def __init__(self, group, *args, **kwargs): if 'hidden' in kwargs: self.hidden = kwargs.pop('hidden') else: self.hidden = False + + self.group = group + super(SessionForm, self).__init__(*args, **kwargs) self.fields['num_session'].widget.attrs['onChange'] = "stat_ls(this.selectedIndex);" self.fields['length_session1'].widget.attrs['onClick'] = "if (check_num_session(1)) this.disabled=true;" self.fields['length_session2'].widget.attrs['onClick'] = "if (check_num_session(2)) this.disabled=true;" self.fields['length_session3'].widget.attrs['onClick'] = "if (check_third_session()) { this.disabled=true;}" self.fields['comments'].widget = forms.Textarea(attrs={'rows':'6','cols':'65'}) + + group_acronym_choices = [('','--Select WG(s)')] + list(allowed_conflicting_groups().exclude(pk=group.pk).values_list('acronym','acronym').order_by('acronym')) + for i in range(1, 4): + self.fields['wg_selector{}'.format(i)].choices = group_acronym_choices + # disabling handleconflictfield (which only enables or disables form elements) while we're hacking the meaning of the three constraints currently in use: #self.fields['wg_selector1'].widget.attrs['onChange'] = "document.form_post.conflict1.value=document.form_post.conflict1.value + ' ' + this.options[this.selectedIndex].value; return handleconflictfield(1);" #self.fields['wg_selector2'].widget.attrs['onChange'] = "document.form_post.conflict2.value=document.form_post.conflict2.value + ' ' + this.options[this.selectedIndex].value; return handleconflictfield(2);" @@ -117,17 +132,17 @@ class SessionForm(forms.Form): def clean_conflict1(self): conflict = self.cleaned_data['conflict1'] - check_conflict(conflict) + check_conflict(conflict, self.group) return conflict def clean_conflict2(self): conflict = self.cleaned_data['conflict2'] - check_conflict(conflict) + check_conflict(conflict, self.group) return conflict def clean_conflict3(self): conflict = self.cleaned_data['conflict3'] - check_conflict(conflict) + check_conflict(conflict, self.group) return conflict def clean(self): diff --git a/ietf/secr/sreq/tests.py b/ietf/secr/sreq/tests.py index 949df471f..24d9009a2 100644 --- a/ietf/secr/sreq/tests.py +++ b/ietf/secr/sreq/tests.py @@ -1,4 +1,4 @@ -# Copyright The IETF Trust 2013-2019, All Rights Reserved +# Copyright The IETF Trust 2013-2020, All Rights Reserved # -*- coding: utf-8 -*- @@ -13,7 +13,7 @@ import debug # pyflakes:ignore from ietf.utils.test_utils import TestCase from ietf.group.factories import GroupFactory, RoleFactory -from ietf.meeting.models import Session, ResourceAssociation, SchedulingEvent +from ietf.meeting.models import Session, ResourceAssociation, SchedulingEvent, Constraint from ietf.meeting.factories import MeetingFactory, SessionFactory from ietf.person.models import Person from ietf.utils.mail import outbox, empty_outbox @@ -154,6 +154,51 @@ class SubmitRequestCase(TestCase): self.assertEqual(len(q('#session-request-form')),1) self.assertContains(r, 'You must enter a length for all sessions') + def test_submit_request_check_constraints(self): + m1 = MeetingFactory(type_id='ietf', date=datetime.date.today() - datetime.timedelta(days=100)) + MeetingFactory(type_id='ietf', date=datetime.date.today()) + ad = Person.objects.get(user__username='ad') + area = RoleFactory(name_id='ad', person=ad, group__type_id='area').group + group = GroupFactory(parent=area) + still_active_group = GroupFactory(parent=area) + Constraint.objects.create( + meeting=m1, + source=group, + target=still_active_group, + name_id='conflict', + ) + inactive_group = GroupFactory(parent=area, state_id='conclude') + inactive_group.save() + Constraint.objects.create( + meeting=m1, + source=group, + target=inactive_group, + name_id='conflict', + ) + SessionFactory(group=group, meeting=m1) + + self.client.login(username="secretary", password="secretary+password") + + url = reverse('ietf.secr.sreq.views.new',kwargs={'acronym':group.acronym}) + r = self.client.get(url + '?previous') + self.assertEqual(r.status_code, 200) + q = PyQuery(r.content) + conflict1 = q('[name="conflict1"]').val() + self.assertIn(still_active_group.acronym, conflict1) + self.assertNotIn(inactive_group.acronym, conflict1) + + post_data = {'num_session':'1', + 'length_session1':'3600', + 'attendees':'10', + 'conflict1': group.acronym, + 'comments':'need projector', + 'submit': 'Continue'} + r = self.client.post(url,post_data) + self.assertEqual(r.status_code, 200) + q = PyQuery(r.content) + self.assertEqual(len(q('#session-request-form')),1) + self.assertContains(r, "Cannot declare a conflict with the same group") + def test_request_notification(self): meeting = MeetingFactory(type_id='ietf', date=datetime.date.today()) ad = Person.objects.get(user__username='ad') diff --git a/ietf/secr/sreq/views.py b/ietf/secr/sreq/views.py index 255146233..c8b685a21 100644 --- a/ietf/secr/sreq/views.py +++ b/ietf/secr/sreq/views.py @@ -1,4 +1,4 @@ -# Copyright The IETF Trust 2013-2019, All Rights Reserved +# Copyright The IETF Trust 2013-2020, All Rights Reserved # -*- coding: utf-8 -*- @@ -21,7 +21,7 @@ from ietf.meeting.models import Meeting, Session, Constraint, ResourceAssociatio from ietf.meeting.helpers import get_meeting from ietf.meeting.utils import add_event_info_to_session_qs from ietf.name.models import SessionStatusName, ConstraintName -from ietf.secr.sreq.forms import SessionForm, ToolStatusForm +from ietf.secr.sreq.forms import SessionForm, ToolStatusForm, allowed_conflicting_groups from ietf.secr.utils.decorators import check_permissions from ietf.secr.utils.group import get_my_groups from ietf.utils.mail import send_mail @@ -45,13 +45,13 @@ def check_app_locked(meeting=None): meeting = get_meeting() return bool(meeting.session_request_lock_message) -def get_initial_session(sessions): +def get_initial_session(sessions, clean_conflicts=False): ''' This function takes a queryset of sessions ordered by 'id' for consistency. It returns a dictionary to be used as the initial for a legacy session form ''' initial = {} - if(len(sessions) == 0): + if len(sessions) == 0: return initial meeting = sessions[0].meeting @@ -70,9 +70,13 @@ def get_initial_session(sessions): except IndexError: pass initial['attendees'] = sessions[0].attendees - initial['conflict1'] = ' '.join([ c.target.acronym for c in conflicts.filter(name__slug='conflict') ]) - initial['conflict2'] = ' '.join([ c.target.acronym for c in conflicts.filter(name__slug='conflic2') ]) - initial['conflict3'] = ' '.join([ c.target.acronym for c in conflicts.filter(name__slug='conflic3') ]) + + def valid_conflict(conflict): + return conflict.target != sessions[0].group and allowed_conflicting_groups().filter(pk=conflict.target_id).exists() + + initial['conflict1'] = ' '.join(c.target.acronym for c in conflicts.filter(name__slug='conflict') if not clean_conflicts or valid_conflict(c)) + initial['conflict2'] = ' '.join(c.target.acronym for c in conflicts.filter(name__slug='conflic2') if not clean_conflicts or valid_conflict(c)) + initial['conflict3'] = ' '.join(c.target.acronym for c in conflicts.filter(name__slug='conflic3') if not clean_conflicts or valid_conflict(c)) initial['comments'] = sessions[0].comments initial['resources'] = sessions[0].resources.all() initial['bethere'] = [x.person for x in sessions[0].constraints().filter(name='bethere').select_related("person")] @@ -243,10 +247,10 @@ def confirm(request, acronym): to confirm for submission. ''' # FIXME: this should be using form.is_valid/form.cleaned_data - invalid input will make it crash - form = SessionForm(request.POST, hidden=True) + group = get_object_or_404(Group,acronym=acronym) + form = SessionForm(group, request.POST, hidden=True) form.is_valid() meeting = get_meeting() - group = get_object_or_404(Group,acronym=acronym) login = request.user.person # check if request already exists for this group @@ -376,7 +380,7 @@ def edit(request, acronym, num=None): if button_text == 'Cancel': return redirect('ietf.secr.sreq.views.view', acronym=acronym) - form = SessionForm(request.POST,initial=initial) + form = SessionForm(group, request.POST,initial=initial) if form.is_valid(): if form.has_changed(): # might be cleaner to simply delete and rewrite all records (but maintain submitter?) @@ -485,7 +489,7 @@ def edit(request, acronym, num=None): else: if not sessions: return redirect('ietf.secr.sreq.views.new', acronym=acronym) - form = SessionForm(initial=initial) + form = SessionForm(group, initial=initial) return render(request, 'sreq/edit.html', { 'is_locked': is_locked, @@ -583,7 +587,7 @@ def new(request, acronym): if button_text == 'Cancel': return redirect('ietf.secr.sreq.views.main') - form = SessionForm(request.POST) + form = SessionForm(group, request.POST) if form.is_valid(): return confirm(request, acronym) @@ -596,16 +600,16 @@ def new(request, acronym): messages.warning(request, 'This group did not meet at %s' % previous_meeting) return redirect('ietf.secr.sreq.views.new', acronym=acronym) - initial = get_initial_session(previous_sessions) + initial = get_initial_session(previous_sessions, clean_conflicts=True) add_essential_people(group,initial) if 'resources' in initial: initial['resources'] = [x.pk for x in initial['resources']] - form = SessionForm(initial=initial) + form = SessionForm(group, initial=initial) else: initial={} add_essential_people(group,initial) - form = SessionForm(initial=initial) + form = SessionForm(group, initial=initial) return render(request, 'sreq/new.html', { 'meeting': meeting,