altf4arnold/datatracker
1
0
Fork 0
Code Issues Pull requests Actions 5 Packages Projects Releases Wiki Activity

Merged in [17217] from olau@iola.dk:

Browse source 
Tighten validation of session conflicts in the session request tool,
preventing conflicts where source = target. Make sure the 'use
previous session' button doesn't carry forward conflicts that are no
longer valid.
Also fix a bug with the group list in the dropdowns being computed
statically at module import time instead of being queried dynamically
upon each page request.
 - Legacy-Id: 17222
Note: SVN reference [17217] has been migrated to Git commit c34fec535f
This commit is contained in:
Henrik Levkowetz 2020-01-13 14:14:31 +00:00
parent 428525d8c4 c34fec535f
commit 2eafa85b46
3 changed files with 93 additions and 29 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

39
ietf/secr/sreq/forms.py
View file

@ -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):

49
ietf/secr/sreq/tests.py
View file

@ -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')

34
ietf/secr/sreq/views.py
View file

@ -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, prune_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 prune_conflicts or valid_conflict(c))
initial['conflict2'] = ' '.join(c.target.acronym for c in conflicts.filter(name__slug='conflic2') if not prune_conflicts or valid_conflict(c))
initial['conflict3'] = ' '.join(c.target.acronym for c in conflicts.filter(name__slug='conflic3') if not prune_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, prune_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,