From 66b9c41dcc715d63f5e50c2bb5ef0c72bb225fb5 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Wed, 9 Jun 2021 19:36:27 +0000 Subject: [PATCH] Add ConstraintNames for chair, tech, and key participant conflicts. Replace temporary UI workaround with proper conflict type handling. Fixes #3083. Commit ready for merge. - Legacy-Id: 19103 --- .../management/commands/generate_schedule.py | 2 +- .../0041_assign_correct_constraintnames.py | 47 +++ ietf/meeting/models.py | 20 ++ ietf/name/fixtures/names.json | 46 +++ .../0024_constraintname_is_group_conflict.py | 18 ++ ...25_set_constraintname_is_group_conflict.py | 25 ++ .../0026_add_conflict_constraintnames.py | 75 +++++ ietf/name/models.py | 1 + ietf/secr/sreq/forms.py | 192 +++++++----- ietf/secr/sreq/tests.py | 163 +++++++--- ietf/secr/sreq/views.py | 112 ++++--- ietf/secr/static/secr/css/custom.css | 4 +- ietf/secr/static/secr/js/sessions.js | 285 ++++++++---------- .../includes/sessions_request_form.html | 43 +-- .../includes/sessions_request_view.html | 14 +- .../static/ietf/js/agenda/agenda_listeners.js | 2 +- ietf/static/ietf/js/agenda/agenda_objects.js | 139 ++++----- 17 files changed, 750 insertions(+), 438 deletions(-) create mode 100644 ietf/meeting/migrations/0041_assign_correct_constraintnames.py create mode 100644 ietf/name/migrations/0024_constraintname_is_group_conflict.py create mode 100644 ietf/name/migrations/0025_set_constraintname_is_group_conflict.py create mode 100644 ietf/name/migrations/0026_add_conflict_constraintnames.py diff --git a/ietf/meeting/management/commands/generate_schedule.py b/ietf/meeting/management/commands/generate_schedule.py index 6a99cdd3f..2799fa04f 100644 --- a/ietf/meeting/management/commands/generate_schedule.py +++ b/ietf/meeting/management/commands/generate_schedule.py @@ -583,7 +583,7 @@ class Session(object): self.last_cost = None for constraint_db in constraints_db: - if constraint_db.name.slug in ['conflict', 'conflic2', 'conflic3']: + if constraint_db.name.is_group_conflict: self.conflict_groups[constraint_db.target.acronym] += constraint_db.name.penalty elif constraint_db.name.slug == 'bethere': self.conflict_people.add(constraint_db.person.pk) diff --git a/ietf/meeting/migrations/0041_assign_correct_constraintnames.py b/ietf/meeting/migrations/0041_assign_correct_constraintnames.py new file mode 100644 index 000000000..a5f5cf130 --- /dev/null +++ b/ietf/meeting/migrations/0041_assign_correct_constraintnames.py @@ -0,0 +1,47 @@ +# Generated by Django 2.2.20 on 2021-05-13 07:51 + +from django.db import migrations + + +replacement_slugs = ( + ('conflict', 'chair_conflict'), + ('conflic2', 'tech_overlap'), + ('conflic3', 'key_participant'), +) + + +def affected(constraint_qs): + """Filter constraints, keeping only those to be updated""" + # The constraints were renamed in the UI in commit 16699 on 2019-09-03. + # This was between meetings 105 and 106. Assuming this migration and + # the new conflict types are in place before meeting 111, these + # are the meetings for which the UI disagreed with the constraint + # type actually created. + affected_meetings = ['106', '107', '108', '109', '110'] + return constraint_qs.filter(meeting__number__in=affected_meetings) + + +def forward(apps, schema_editor): + Constraint = apps.get_model('meeting', 'Constraint') + affected_constraints = affected(Constraint.objects.all()) + for old, new in replacement_slugs: + affected_constraints.filter(name_id=old).update(name_id=new) + + +def reverse(apps, schema_editor): + Constraint = apps.get_model('meeting', 'Constraint') + affected_constraints = affected(Constraint.objects.all()) + for old, new in replacement_slugs: + affected_constraints.filter(name_id=new).update(name_id=old) + + +class Migration(migrations.Migration): + + dependencies = [ + ('meeting', '0040_auto_20210130_1027'), + ('name', '0026_add_conflict_constraintnames'), + ] + + operations = [ + migrations.RunPython(forward, reverse), + ] diff --git a/ietf/meeting/models.py b/ietf/meeting/models.py index 3e14a76c9..39c9c39af 100644 --- a/ietf/meeting/models.py +++ b/ietf/meeting/models.py @@ -207,6 +207,26 @@ class Meeting(models.Model): else: return None + @property + def session_constraintnames(self): + """Gets a list of the constraint names that should be used for this meeting + + Anticipated that this will soon become a many-to-many relationship with ConstraintName + (see issue #2770). Making this a @property allows use of the .all(), .filter(), etc, + so that other code should not need changes when this is replaced. + """ + try: + mtg_num = int(self.number) + except ValueError: + mtg_num = None # should not come up, but this method should not fail + if mtg_num is None or mtg_num >= 106: + # These meetings used the old 'conflic?' constraint types labeled as though + # they were the new types. + slugs = ('chair_conflict', 'tech_overlap', 'key_participant') + else: + slugs = ('conflict', 'conflic2', 'conflic3') + return ConstraintName.objects.filter(slug__in=slugs) + def json_url(self): return "/meeting/%s/json" % (self.number, ) diff --git a/ietf/name/fixtures/names.json b/ietf/name/fixtures/names.json index ce101e8ae..9b344014f 100644 --- a/ietf/name/fixtures/names.json +++ b/ietf/name/fixtures/names.json @@ -5990,6 +5990,7 @@ "fields": { "desc": "", "editor_label": "{count}", + "is_group_conflict": false, "name": "Person must be present", "order": 4, "penalty": 10000, @@ -5998,10 +5999,24 @@ "model": "name.constraintname", "pk": "bethere" }, + { + "fields": { + "desc": "Indicates other WGs the chairs also lead or will be active participants in", + "editor_label": "", + "is_group_conflict": true, + "name": "Chair conflict", + "order": 8, + "penalty": 100000, + "used": true + }, + "model": "name.constraintname", + "pk": "chair_conflict" + }, { "fields": { "desc": "", "editor_label": "2", + "is_group_conflict": true, "name": "Conflicts with (secondary)", "order": 2, "penalty": 10000, @@ -6014,6 +6029,7 @@ "fields": { "desc": "", "editor_label": "3", + "is_group_conflict": true, "name": "Conflicts with (tertiary)", "order": 3, "penalty": 100000, @@ -6026,6 +6042,7 @@ "fields": { "desc": "", "editor_label": "1", + "is_group_conflict": true, "name": "Conflicts with", "order": 1, "penalty": 100000, @@ -6034,10 +6051,37 @@ "model": "name.constraintname", "pk": "conflict" }, + { + "fields": { + "desc": "Indicates WGs with which key participants (presenter, secretary, etc.) may overlap", + "editor_label": "", + "is_group_conflict": true, + "name": "Key participant conflict", + "order": 10, + "penalty": 100000, + "used": true + }, + "model": "name.constraintname", + "pk": "key_participant" + }, + { + "fields": { + "desc": "Indicates WGs with a related technology or a closely related charter", + "editor_label": "", + "is_group_conflict": true, + "name": "Technology overlap", + "order": 9, + "penalty": 10000, + "used": true + }, + "model": "name.constraintname", + "pk": "tech_overlap" + }, { "fields": { "desc": "", "editor_label": "Δ", + "is_group_conflict": false, "name": "Preference for time between sessions", "order": 6, "penalty": 1000, @@ -6050,6 +6094,7 @@ "fields": { "desc": "", "editor_label": "", + "is_group_conflict": false, "name": "Can't meet within timerange", "order": 5, "penalty": 1000000, @@ -6062,6 +6107,7 @@ "fields": { "desc": "", "editor_label": "", + "is_group_conflict": false, "name": "Request for adjacent scheduling with another WG", "order": 7, "penalty": 1000, diff --git a/ietf/name/migrations/0024_constraintname_is_group_conflict.py b/ietf/name/migrations/0024_constraintname_is_group_conflict.py new file mode 100644 index 000000000..47810c6da --- /dev/null +++ b/ietf/name/migrations/0024_constraintname_is_group_conflict.py @@ -0,0 +1,18 @@ +# Generated by Django 2.2.20 on 2021-05-19 09:45 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ('name', '0023_change_stream_descriptions'), + ] + + operations = [ + migrations.AddField( + model_name='constraintname', + name='is_group_conflict', + field=models.BooleanField(default=False, + help_text='Does this constraint capture a conflict between groups?'), + ), + ] diff --git a/ietf/name/migrations/0025_set_constraintname_is_group_conflict.py b/ietf/name/migrations/0025_set_constraintname_is_group_conflict.py new file mode 100644 index 000000000..86deb9bd2 --- /dev/null +++ b/ietf/name/migrations/0025_set_constraintname_is_group_conflict.py @@ -0,0 +1,25 @@ +# Generated by Django 2.2.20 on 2021-05-19 09:55 + +from django.db import migrations + + +def forward(apps, schema_editor): + """Set is_group_conflict for ConstraintNames that need it to be True""" + ConstraintName = apps.get_model('name', 'ConstraintName') + ConstraintName.objects.filter( + slug__in=['conflict', 'conflic2', 'conflic3'] + ).update(is_group_conflict=True) + + +def reverse(apps, schema_editor): + pass # nothing to be done + + +class Migration(migrations.Migration): + dependencies = [ + ('name', '0024_constraintname_is_group_conflict'), + ] + + operations = [ + migrations.RunPython(forward, reverse), + ] diff --git a/ietf/name/migrations/0026_add_conflict_constraintnames.py b/ietf/name/migrations/0026_add_conflict_constraintnames.py new file mode 100644 index 000000000..1ee0acbc3 --- /dev/null +++ b/ietf/name/migrations/0026_add_conflict_constraintnames.py @@ -0,0 +1,75 @@ +# Generated by Django 2.2.20 on 2021-05-05 10:05 + +from collections import namedtuple +from django.db import migrations +from django.db.models import Max + + +# Simple type for representing constraint name data that will be +# modified. +ConstraintInfo = namedtuple( + 'ConstraintInfo', + ['replaces', 'slug', 'name', 'desc', 'editor_label'], +) + +constraint_names_to_add = [ + ConstraintInfo( + replaces='conflict', + slug='chair_conflict', + name='Chair conflict', + desc='Indicates other WGs the chairs also lead or will be active participants in', + editor_label='', + ), + ConstraintInfo( + replaces='conflic2', + slug='tech_overlap', + name='Technology overlap', + desc='Indicates WGs with a related technology or a closely related charter', + editor_label='', + ), + ConstraintInfo( + replaces='conflic3', + slug='key_participant', + name='Key participant conflict', + desc='Indicates WGs with which key participants (presenter, secretary, etc.) may overlap', + editor_label='', + ) +] + + +def forward(apps, schema_editor): + ConstraintName = apps.get_model('name', 'ConstraintName') + max_order = ConstraintName.objects.all().aggregate(Max('order'))['order__max'] + + for index, new_constraint in enumerate(constraint_names_to_add): + # hack_constraint is the constraint type relabeled by the hack fix in #2754 + hack_constraint = ConstraintName.objects.get(slug=new_constraint.replaces) + ConstraintName.objects.create( + slug=new_constraint.slug, + name=new_constraint.name, + desc=new_constraint.desc, + used=hack_constraint.used, + order=max_order + index + 1, + penalty=hack_constraint.penalty, + editor_label=new_constraint.editor_label, + is_group_conflict=True, + ) + + +def reverse(apps, schema_editor): + ConstraintName = apps.get_model('name', 'ConstraintName') + for new_constraint in constraint_names_to_add: + ConstraintName.objects.filter(slug=new_constraint.slug).delete() + +class Migration(migrations.Migration): + dependencies = [ + ('name', '0025_set_constraintname_is_group_conflict'), + # Reversing this migration requires that the 'day' field be removed from + # the Constraint model, so we indirectly depend on the migration that + # removed it. + ('meeting', '0027_add_constraint_options_and_joint_groups'), + ] + + operations = [ + migrations.RunPython(forward, reverse), + ] diff --git a/ietf/name/models.py b/ietf/name/models.py index 22a265f3a..3117487f4 100644 --- a/ietf/name/models.py +++ b/ietf/name/models.py @@ -71,6 +71,7 @@ class ConstraintName(NameModel): """conflict, conflic2, conflic3, bethere, timerange, time_relation, wg_adjacent""" penalty = models.IntegerField(default=0, help_text="The penalty for violating this kind of constraint; for instance 10 (small penalty) or 10000 (large penalty)") editor_label = models.CharField(max_length=64, blank=True, help_text="Very short label for producing warnings inline in the sessions in the schedule editor.") + is_group_conflict = models.BooleanField(default=False, help_text="Does this constraint capture a conflict between groups?") class TimerangeName(NameModel): """(monday|tuesday|wednesday|thursday|friday)-(morning|afternoon-early|afternoon-late)""" class LiaisonStatementPurposeName(NameModel): diff --git a/ietf/secr/sreq/forms.py b/ietf/secr/sreq/forms.py index 1aba94fd0..64b3e3dbb 100644 --- a/ietf/secr/sreq/forms.py +++ b/ietf/secr/sreq/forms.py @@ -11,6 +11,7 @@ from ietf.group.models import Group from ietf.meeting.models import ResourceAssociation, Constraint from ietf.person.fields import SearchablePersonsField from ietf.utils.html import clean_text_field +from ietf.utils import log # ------------------------------------------------- # Globals @@ -42,18 +43,6 @@ def check_conflict(groups, source_group): if not active_groups.filter(acronym=group): raise forms.ValidationError("Invalid or inactive group acronym: %s" % group) - -def join_conflicts(data): - ''' - Takes a dictionary (ie. data dict from a form) and concatenates all - conflict fields into one list - ''' - conflicts = [] - for groups in (data['conflict1'],data['conflict2'],data['conflict3']): - # convert to python list (allow space or comma separated lists) - items = groups.replace(',',' ').split() - conflicts.extend(items) - return conflicts # ------------------------------------------------- # Forms @@ -82,17 +71,12 @@ class SessionForm(forms.Form): 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) + # way validation comes for free (applies to this CharField and the + # constraints dynamically instantiated in __init__()) joint_with_groups = forms.CharField(max_length=255,required=False) + joint_with_groups_selector = forms.ChoiceField(choices=[], required=False) # group select widget for prev field joint_for_session = forms.ChoiceField(choices=JOINT_FOR_SESSION_CHOICES, required=False) comments = forms.CharField(max_length=200,required=False) - wg_selector1 = forms.ChoiceField(choices=[],required=False) - wg_selector2 = forms.ChoiceField(choices=[],required=False) - wg_selector3 = forms.ChoiceField(choices=[],required=False) - wg_selector4 = 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) @@ -100,7 +84,7 @@ class SessionForm(forms.Form): queryset=TimerangeName.objects.all()) adjacent_with_wg = forms.ChoiceField(required=False) - def __init__(self, group, *args, **kwargs): + def __init__(self, group, meeting, *args, **kwargs): if 'hidden' in kwargs: self.hidden = kwargs.pop('hidden') else: @@ -109,31 +93,40 @@ class SessionForm(forms.Form): 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['num_session'].widget.attrs['onChange'] = "ietf_sessions.stat_ls(this.selectedIndex);" + self.fields['length_session1'].widget.attrs['onClick'] = "if (ietf_sessions.check_num_session(1)) this.disabled=true;" + self.fields['length_session2'].widget.attrs['onClick'] = "if (ietf_sessions.check_num_session(2)) this.disabled=true;" + self.fields['length_session3'].widget.attrs['onClick'] = "if (ietf_sessions.check_third_session()) { this.disabled=true;}" self.fields['comments'].widget = forms.Textarea(attrs={'rows':'3','cols':'65'}) other_groups = list(allowed_conflicting_groups().exclude(pk=group.pk).values_list('acronym', 'acronym').order_by('acronym')) self.fields['adjacent_with_wg'].choices = [('', '--No preference')] + other_groups group_acronym_choices = [('','--Select WG(s)')] + other_groups - for i in range(1, 5): - self.fields['wg_selector{}'.format(i)].choices = group_acronym_choices + self.fields['joint_with_groups_selector'].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);" - #self.fields['wg_selector3'].widget.attrs['onChange'] = "document.form_post.conflict3.value=document.form_post.conflict3.value + ' ' + this.options[this.selectedIndex].value; return handleconflictfield(3);" - self.fields['wg_selector1'].widget.attrs['onChange'] = "document.form_post.conflict1.value=document.form_post.conflict1.value + ' ' + this.options[this.selectedIndex].value; return 1;" - self.fields['wg_selector2'].widget.attrs['onChange'] = "document.form_post.conflict2.value=document.form_post.conflict2.value + ' ' + this.options[this.selectedIndex].value; return 1;" - self.fields['wg_selector3'].widget.attrs['onChange'] = "document.form_post.conflict3.value=document.form_post.conflict3.value + ' ' + this.options[this.selectedIndex].value; return 1;" - self.fields['wg_selector4'].widget.attrs['onChange'] = "document.form_post.joint_with_groups.value=document.form_post.joint_with_groups.value + ' ' + this.options[this.selectedIndex].value; return 1;" + # Set up constraints for the meeting + self._wg_field_data = [] + for constraintname in meeting.session_constraintnames.all(): + # two fields for each constraint: a CharField for the group list and a selector to add entries + constraint_field = forms.CharField(max_length=255, required=False) + constraint_field.widget.attrs['data-slug'] = constraintname.slug + constraint_field.widget.attrs['data-constraint-name'] = str(constraintname).title() + self._add_widget_class(constraint_field.widget, 'wg_constraint') - # disabling check_prior_conflict javascript while we're hacking the meaning of the three constraints currently in use - #self.fields['wg_selector2'].widget.attrs['onClick'] = "return check_prior_conflict(2);" - #self.fields['wg_selector3'].widget.attrs['onClick'] = "return check_prior_conflict(3);" - + selector_field = forms.ChoiceField(choices=group_acronym_choices, required=False) + selector_field.widget.attrs['data-slug'] = constraintname.slug # used by onChange handler + self._add_widget_class(selector_field.widget, 'wg_constraint_selector') + + cfield_id = 'constraint_{}'.format(constraintname.slug) + cselector_id = 'wg_selector_{}'.format(constraintname.slug) + # keep an eye out for field name conflicts + log.assertion('cfield_id not in self.fields') + log.assertion('cselector_id not in self.fields') + self.fields[cfield_id] = constraint_field + self.fields[cselector_id] = selector_field + self._wg_field_data.append((constraintname, cfield_id, cselector_id)) + + self.fields['joint_with_groups_selector'].widget.attrs['onChange'] = "document.form_post.joint_with_groups.value=document.form_post.joint_with_groups.value + ' ' + this.options[this.selectedIndex].value; return 1;" self.fields['third_session'].widget.attrs['onClick'] = "if (document.form_post.num_session.selectedIndex < 2) { alert('Cannot use this field - Number of Session is not set to 2'); return false; } else { if (this.checked==true) { document.form_post.length_session3.disabled=false; } else { document.form_post.length_session3.value=0;document.form_post.length_session3.disabled=true; } }" self.fields["resources"].choices = [(x.pk,x.desc) for x in ResourceAssociation.objects.filter(name__used=True).order_by('name__order') ] @@ -149,20 +142,53 @@ class SessionForm(forms.Form): self.fields['resources'].widget = forms.MultipleHiddenInput() self.fields['timeranges'].widget = forms.MultipleHiddenInput() - def clean_conflict1(self): - conflict = self.cleaned_data['conflict1'] - check_conflict(conflict, self.group) - return conflict - - def clean_conflict2(self): - conflict = self.cleaned_data['conflict2'] - check_conflict(conflict, self.group) - return conflict - - def clean_conflict3(self): - conflict = self.cleaned_data['conflict3'] - check_conflict(conflict, self.group) - return conflict + def wg_constraint_fields(self): + """Iterates over wg constraint fields + + Intended for use in the template. + """ + for cname, cfield_id, cselector_id in self._wg_field_data: + yield cname, self[cfield_id], self[cselector_id] + + def wg_constraint_field_ids(self): + """Iterates over wg constraint field IDs""" + for cname, cfield_id, _ in self._wg_field_data: + yield cname, cfield_id + + @staticmethod + def _add_widget_class(widget, new_class): + """Add a new class, taking care in case some already exist""" + existing_classes = widget.attrs.get('class', '').split() + widget.attrs['class'] = ' '.join(existing_classes + [new_class]) + + def _join_conflicts(self, cleaned_data, slugs): + """Concatenate constraint fields from cleaned data into a single list""" + conflicts = [] + for cname, cfield_id, _ in self._wg_field_data: + if cname.slug in slugs and cfield_id in cleaned_data: + groups = cleaned_data[cfield_id] + # convert to python list (allow space or comma separated lists) + items = groups.replace(',',' ').split() + conflicts.extend(items) + return conflicts + + def _validate_duplicate_conflicts(self, cleaned_data): + """Validate that no WGs appear in more than one constraint that does not allow duplicates + + Raises ValidationError + """ + # Only the older constraints (conflict, conflic2, conflic3) need to be mutually exclusive. + all_conflicts = self._join_conflicts(cleaned_data, ['conflict', 'conflic2', 'conflic3']) + seen = [] + duplicated = [] + errors = [] + for c in all_conflicts: + if c not in seen: + seen.append(c) + elif c not in duplicated: # only report once + duplicated.append(c) + errors.append(forms.ValidationError('%s appears in conflicts more than once' % c)) + return errors def clean_joint_with_groups(self): groups = self.cleaned_data['joint_with_groups'] @@ -175,37 +201,51 @@ class SessionForm(forms.Form): def clean(self): super(SessionForm, self).clean() data = self.cleaned_data + + # Validate the individual conflict fields + for _, cfield_id, _ in self._wg_field_data: + try: + check_conflict(data[cfield_id], self.group) + except forms.ValidationError as e: + self.add_error(cfield_id, e) + + # Skip remaining tests if individual field tests had errors, if self.errors: - return self.cleaned_data - - # error if conflits contain dupes - all_conflicts = join_conflicts(data) - temp = [] - for c in all_conflicts: - if c not in temp: - temp.append(c) - else: - raise forms.ValidationError('%s appears in conflicts more than once' % c) - + return data + + # error if conflicts contain disallowed dupes + for error in self._validate_duplicate_conflicts(data): + self.add_error(None, error) + # verify session_length and num_session correspond # if default (empty) option is selected, cleaned_data won't include num_session key if data.get('num_session','') == '2': if not data['length_session2']: - raise forms.ValidationError('You must enter a length for all sessions') + self.add_error('length_session2', forms.ValidationError('You must enter a length for all sessions')) else: if data.get('session_time_relation'): - raise forms.ValidationError('Time between sessions can only be used when two ' - 'sessions are requested.') - if data['joint_for_session'] == '2': - raise forms.ValidationError('The second session can not be the joint session, ' - 'because you have not requested a second session.') + self.add_error( + 'session_time_relation', + forms.ValidationError('Time between sessions can only be used when two sessions are requested.') + ) + if data.get('joint_for_session') == '2': + self.add_error( + 'joint_for_session', + forms.ValidationError( + 'The second session can not be the joint session, because you have not requested a second session.' + ) + ) - if data.get('third_session',False): - if not data['length_session2'] or not data.get('length_session3',None): - raise forms.ValidationError('You must enter a length for all sessions') - elif data['joint_for_session'] == '3': - raise forms.ValidationError('The third session can not be the joint session, ' - 'because you have not requested a third session.') + if data.get('third_session', False): + if not data.get('length_session3',None): + self.add_error('length_session3', forms.ValidationError('You must enter a length for all sessions')) + elif data.get('joint_for_session') == '3': + self.add_error( + 'joint_for_session', + forms.ValidationError( + 'The third session can not be the joint session, because you have not requested a third session.' + ) + ) return data diff --git a/ietf/secr/sreq/tests.py b/ietf/secr/sreq/tests.py index 8f943d572..9b6ffea79 100644 --- a/ietf/secr/sreq/tests.py +++ b/ietf/secr/sreq/tests.py @@ -94,7 +94,7 @@ class SessionRequestTestCase(TestCase): 'length_session1':'3600', 'length_session2':'3600', 'attendees':'10', - 'conflict1':iabprog.acronym, + 'constraint_conflict': iabprog.acronym, 'comments':'need lights', 'session_time_relation': 'subsequent-days', 'adjacent_with_wg': group2.acronym, @@ -134,7 +134,7 @@ class SessionRequestTestCase(TestCase): 'length_session1':'3600', 'length_session2':'3600', 'attendees':'10', - 'conflict1':'', + 'constraint_conflict':'', 'comments':'need lights', 'joint_with_groups': group2.acronym, 'joint_for_session': '1', @@ -164,8 +164,71 @@ class SessionRequestTestCase(TestCase): self.assertEqual(r.status_code, 200) r = self.client.post(url, {'message':'locked', 'submit':'Lock'}) self.assertRedirects(r,reverse('ietf.secr.sreq.views.main')) - + + def test_new_req_constraint_types(self): + """ITEF meetings 106 and later use different constraint types + + Relies on SessionForm representing constraint values with element IDs + like id_constraint_ + """ + should_have_pre106 = ['conflict', 'conflic2', 'conflic3'] + should_have = ['chair_conflict', 'tech_overlap', 'key_participant'] + + meeting = MeetingFactory(type_id='ietf', date=datetime.date.today()) + RoleFactory(name_id='chair', person__user__username='marschairman', group__acronym='mars') + url = reverse('ietf.secr.sreq.views.new', kwargs=dict(acronym='mars')) + self.client.login(username="marschairman", password="marschairman+password") + + for meeting_number in ['95', '100', '105', '106', '111', '125']: + meeting.number = meeting_number + meeting.save() + + r = self.client.get(url) + self.assertEqual(r.status_code, 200) + q = PyQuery(r.content) + expected = should_have if int(meeting.number) >= 106 else should_have_pre106 + self.assertCountEqual( + [elt.attr('id') for elt in q.items('*[id^=id_constraint_]')], + ['id_constraint_{}'.format(conf_name) for conf_name in expected], + 'Unexpected constraints for meeting number {}'.format(meeting_number), + ) + + def test_edit_req_constraint_types(self): + """Editing a request constraint should show the expected constraints""" + should_have_pre106 = ['conflict', 'conflic2', 'conflic3'] + should_have = ['chair_conflict', 'tech_overlap', 'key_participant'] + + meeting = MeetingFactory(type_id='ietf', date=datetime.date.today()) + SessionFactory(group__acronym='mars', + status_id='schedw', + meeting=meeting, + add_to_schedule=False) + RoleFactory(name_id='chair', person__user__username='marschairman', group__acronym='mars') + + url = reverse('ietf.secr.sreq.views.edit', kwargs=dict(acronym='mars')) + self.client.login(username='marschairman', password='marschairman+password') + + for meeting_number in ['95', '100', '105', '106', '111', '125']: + meeting.number = meeting_number + meeting.save() + + r = self.client.get(url) + self.assertEqual(r.status_code, 200) + q = PyQuery(r.content) + expected = should_have if int(meeting.number) >= 106 else should_have_pre106 + self.assertCountEqual( + [elt.attr('id') for elt in q.items('*[id^=id_constraint_]')], + ['id_constraint_{}'.format(conf_name) for conf_name in expected], + 'Unexpected constraints for meeting number {}'.format(meeting_number), + ) + class SubmitRequestCase(TestCase): + def setUp(self): + super(SubmitRequestCase, self).setUp() + # Ensure meeting numbers are predictable. Temporarily needed while basing + # constraint types on meeting number, expected to go away when #2770 is resolved. + MeetingFactory.reset_sequence(0) + def test_submit_request(self): meeting = MeetingFactory(type_id='ietf', date=datetime.date.today()) ad = Person.objects.get(user__username='ad') @@ -181,7 +244,7 @@ class SubmitRequestCase(TestCase): post_data = {'num_session':'1', 'length_session1':'3600', 'attendees':'10', - 'conflict1':'', + 'constraint_conflict':'', 'comments':'need projector', 'adjacent_with_wg': group2.acronym, 'timeranges': ['thursday-afternoon-early', 'thursday-afternoon-late'], @@ -227,7 +290,7 @@ class SubmitRequestCase(TestCase): post_data = {'num_session':'2', 'length_session1':'3600', 'attendees':'10', - 'conflict1':'', + 'constraint_conflict':'', 'comments':'need projector'} self.client.login(username="secretary", password="secretary+password") r = self.client.post(url,post_data) @@ -265,14 +328,14 @@ class SubmitRequestCase(TestCase): r = self.client.get(url + '?previous') self.assertEqual(r.status_code, 200) q = PyQuery(r.content) - conflict1 = q('[name="conflict1"]').val() + conflict1 = q('[name="constraint_conflict"]').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, + 'constraint_conflict': group.acronym, 'comments':'need projector', 'submit': 'Continue'} r = self.client.post(url,post_data) @@ -280,7 +343,7 @@ class SubmitRequestCase(TestCase): 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') @@ -303,7 +366,7 @@ class SubmitRequestCase(TestCase): 'length_session2':'3600', 'attendees':'10', 'bethere':str(ad.pk), - 'conflict1':'', + 'constraint_conflict':'', 'comments':'', 'resources': resource.pk, 'session_time_relation': 'subsequent-days', @@ -422,6 +485,11 @@ class RetrievePreviousCase(TestCase): class SessionFormTest(TestCase): def setUp(self): + # Ensure meeting numbers are predictable. Temporarily needed while basing + # constraint types on meeting number, expected to go away when #2770 is resolved. + MeetingFactory.reset_sequence(0) + + self.meeting = MeetingFactory(type_id='ietf') self.group1 = GroupFactory() self.group2 = GroupFactory() self.group3 = GroupFactory() @@ -436,9 +504,9 @@ class SessionFormTest(TestCase): 'length_session2': '3600', 'length_session3': '3600', 'attendees': '10', - 'conflict1': self.group2.acronym, - 'conflict2': self.group3.acronym, - 'conflict3': self.group4.acronym, + 'constraint_conflict': self.group2.acronym, + 'constraint_conflic2': self.group3.acronym, + 'constraint_conflic3': self.group4.acronym, 'comments': 'need lights', 'session_time_relation': 'subsequent-days', 'adjacent_with_wg': self.group5.acronym, @@ -450,7 +518,7 @@ class SessionFormTest(TestCase): def test_valid(self): # Test with three sessions - form = SessionForm(data=self.valid_form_data, group=self.group1) + form = SessionForm(data=self.valid_form_data, group=self.group1, meeting=self.meeting) self.assertTrue(form.is_valid()) # Test with two sessions @@ -459,7 +527,7 @@ class SessionFormTest(TestCase): 'third_session': '', 'joint_for_session': '2' }) - form = SessionForm(data=self.valid_form_data, group=self.group1) + form = SessionForm(data=self.valid_form_data, group=self.group1, meeting=self.meeting) self.assertTrue(form.is_valid()) # Test with one session @@ -469,14 +537,14 @@ class SessionFormTest(TestCase): 'joint_for_session': '1', 'session_time_relation': '', }) - form = SessionForm(data=self.valid_form_data, group=self.group1) + form = SessionForm(data=self.valid_form_data, group=self.group1, meeting=self.meeting) self.assertTrue(form.is_valid()) def test_invalid_groups(self): new_form_data = { - 'conflict1': 'doesnotexist', - 'conflict2': 'doesnotexist', - 'conflict3': 'doesnotexist', + 'constraint_conflict': 'doesnotexist', + 'constraint_conflic2': 'doesnotexist', + 'constraint_conflic3': 'doesnotexist', 'adjacent_with_wg': 'doesnotexist', 'joint_with_groups': 'doesnotexist', } @@ -485,15 +553,15 @@ class SessionFormTest(TestCase): def test_invalid_group_appears_in_multiple_conflicts(self): new_form_data = { - 'conflict1': self.group2.acronym, - 'conflict2': self.group2.acronym, + 'constraint_conflict': self.group2.acronym, + 'constraint_conflic2': self.group2.acronym, } form = self._invalid_test_helper(new_form_data) self.assertEqual(form.non_field_errors(), ['%s appears in conflicts more than once' % self.group2.acronym]) def test_invalid_conflict_with_self(self): new_form_data = { - 'conflict1': self.group1.acronym, + 'constraint_conflict': self.group1.acronym, } self._invalid_test_helper(new_form_data) @@ -504,8 +572,11 @@ class SessionFormTest(TestCase): 'num_session': 1, 'joint_for_session': '1', }) - self.assertEqual(form.non_field_errors(), ['Time between sessions can only be used when two ' - 'sessions are requested.']) + self.assertEqual(form.errors, + { + 'session_time_relation': ['Time between sessions can only be used when two ' + 'sessions are requested.'] + }) def test_invalid_joint_for_session(self): form = self._invalid_test_helper({ @@ -513,8 +584,11 @@ class SessionFormTest(TestCase): 'num_session': 2, 'joint_for_session': '3', }) - self.assertEqual(form.non_field_errors(), ['The third session can not be the joint session, ' - 'because you have not requested a third session.']) + self.assertEqual(form.errors, + { + 'joint_for_session': ['The third session can not be the joint session, ' + 'because you have not requested a third session.'] + }) form = self._invalid_test_helper({ 'third_session': '', @@ -523,24 +597,45 @@ class SessionFormTest(TestCase): 'joint_for_session': '2', 'session_time_relation': '', }) - self.assertEqual(form.non_field_errors(), ['The second session can not be the joint session, ' - 'because you have not requested a second session.']) + self.assertEqual(form.errors, + { + 'joint_for_session': ['The second session can not be the joint session, ' + 'because you have not requested a second session.'] + }) def test_invalid_missing_session_length(self): form = self._invalid_test_helper({ 'length_session2': '', - 'third_session': 'true', + 'third_session': 'false', + 'joint_for_session': None, }) - self.assertEqual(form.non_field_errors(), ['You must enter a length for all sessions']) + self.assertEqual(form.errors, + { + 'length_session2': ['You must enter a length for all sessions'], + }) - form = self._invalid_test_helper({'length_session2': ''}) - self.assertEqual(form.non_field_errors(), ['You must enter a length for all sessions']) + form = self._invalid_test_helper({ + 'length_session2': '', + 'length_session3': '', + 'joint_for_session': None, + }) + self.assertEqual(form.errors, + { + 'length_session2': ['You must enter a length for all sessions'], + 'length_session3': ['You must enter a length for all sessions'], + }) - form = self._invalid_test_helper({'length_session3': ''}) - self.assertEqual(form.non_field_errors(), ['You must enter a length for all sessions']) + form = self._invalid_test_helper({ + 'length_session3': '', + 'joint_for_session': None, + }) + self.assertEqual(form.errors, + { + 'length_session3': ['You must enter a length for all sessions'], + }) def _invalid_test_helper(self, new_form_data): form_data = dict(self.valid_form_data, **new_form_data) - form = SessionForm(data=form_data, group=self.group1) + form = SessionForm(data=form_data, group=self.group1, meeting=self.meeting) self.assertFalse(form.is_valid()) return form \ No newline at end of file diff --git a/ietf/secr/sreq/views.py b/ietf/secr/sreq/views.py index 8450a0688..369fab42a 100644 --- a/ietf/secr/sreq/views.py +++ b/ietf/secr/sreq/views.py @@ -3,7 +3,7 @@ import datetime -from collections import defaultdict +from collections import defaultdict, OrderedDict from django.conf import settings from django.contrib import messages @@ -56,7 +56,9 @@ def get_initial_session(sessions, prune_conflicts=False): meeting = sessions[0].meeting group = sessions[0].group - conflicts = group.constraint_source_set.filter(meeting=meeting) + + constraints = group.constraint_source_set.filter(meeting=meeting) # all constraints with this group as source + conflicts = constraints.filter(name__is_group_conflict=True) # only the group conflict constraints # even if there are three sessions requested, the old form has 2 in this field initial['num_session'] = min(sessions.count(), 2) @@ -73,19 +75,23 @@ def get_initial_session(sessions, prune_conflicts=False): def valid_conflict(conflict): return conflict.target != sessions[0].group and allowed_conflicting_groups().filter(pk=conflict.target_id).exists() + if prune_conflicts: + conflicts = [c for c in conflicts if valid_conflict(c)] + + conflict_name_ids = set(c.name_id for c in conflicts) + for name_id in conflict_name_ids: + target_acros = [c.target.acronym for c in conflicts if c.name_id == name_id] + initial['constraint_{}'.format(name_id)] = ' '.join(target_acros) - 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")] - wg_adjacent = conflicts.filter(name__slug='wg_adjacent') + wg_adjacent = constraints.filter(name__slug='wg_adjacent') initial['adjacent_with_wg'] = wg_adjacent[0].target.acronym if wg_adjacent else None - time_relation = conflicts.filter(name__slug='time_relation') + time_relation = constraints.filter(name__slug='time_relation') initial['session_time_relation'] = time_relation[0].time_relation if time_relation else None initial['session_time_relation_display'] = time_relation[0].get_time_relation_display if time_relation else None - timeranges = conflicts.filter(name__slug='timerange') + timeranges = constraints.filter(name__slug='timerange') initial['timeranges'] = timeranges[0].timeranges.all() if timeranges else [] initial['timeranges_display'] = [t.desc for t in initial['timeranges']] for idx, session in enumerate(sessions): @@ -180,13 +186,14 @@ def send_notification(group,meeting,login,session,action): context, cc=cc_list) -def session_conflicts_as_string(group, meeting): +def inbound_session_conflicts_as_string(group, meeting): ''' Takes a Group object and Meeting object and returns a string of other groups which have a conflict with this one ''' - groups = group.constraint_target_set.filter(meeting=meeting, name__in=['conflict', 'conflic2', 'conflic3']) - group_list = [g.source.acronym for g in groups] + constraints = group.constraint_target_set.filter(meeting=meeting, name__is_group_conflict=True) + group_set = set(constraints.values_list('source__acronym', flat=True)) # set to de-dupe + group_list = sorted(group_set) # give a consistent order return ', '.join(group_list) # ------------------------------------------------- @@ -272,7 +279,7 @@ def confirm(request, acronym): meeting = get_meeting(days=14) FormClass = get_session_form_class() - form = FormClass(group, request.POST, hidden=True) + form = FormClass(group, meeting, request.POST, hidden=True) form.is_valid() login = request.user.person @@ -293,11 +300,13 @@ def confirm(request, acronym): if form.cleaned_data.get('timeranges'): session_data['timeranges_display'] = [t.desc for t in form.cleaned_data['timeranges']] session_data['resources'] = [ ResourceAssociation.objects.get(pk=pk) for pk in request.POST.getlist('resources') ] - - button_text = request.POST.get('submit', '') - if button_text == 'Cancel': - messages.success(request, 'Session Request has been cancelled') - return redirect('ietf.secr.sreq.views.main') + + # extract wg conflict constraint data for the view + outbound_conflicts = [] + for conflictname, cfield_id in form.wg_constraint_field_ids(): + conflict_groups = form.cleaned_data[cfield_id] + if len(conflict_groups) > 0: + outbound_conflicts.append(dict(name=conflictname, groups=conflict_groups)) button_text = request.POST.get('submit', '') if button_text == 'Cancel': @@ -341,9 +350,8 @@ def confirm(request, acronym): session_changed(new_session) # write constraint records - save_conflicts(group,meeting,form.data.get('conflict1',''),'conflict') - save_conflicts(group,meeting,form.data.get('conflict2',''),'conflic2') - save_conflicts(group,meeting,form.data.get('conflict3',''),'conflic3') + for conflictname, cfield_id in form.wg_constraint_field_ids(): + save_conflicts(group, meeting, form.data.get(cfield_id, ''), conflictname.slug) save_conflicts(group, meeting, form.data.get('adjacent_with_wg', ''), 'wg_adjacent') if form.cleaned_data.get('session_time_relation'): @@ -372,7 +380,10 @@ def confirm(request, acronym): return redirect('ietf.secr.sreq.views.main') # POST from request submission - session_conflicts = session_conflicts_as_string(group, meeting) + session_conflicts = dict( + outbound=outbound_conflicts, # each is a dict with name and groups as keys + inbound=inbound_session_conflicts_as_string(group, meeting), + ) return render(request, 'sreq/confirm.html', { 'form': form, @@ -418,8 +429,11 @@ def edit(request, acronym, num=None): is_locked = check_app_locked(meeting=meeting) if is_locked: messages.warning(request, "The Session Request Tool is closed") - - session_conflicts = session_conflicts_as_string(group, meeting) + + # Only need the inbound conflicts here, the form itself renders the outbound + session_conflicts = dict( + inbound=inbound_session_conflicts_as_string(group, meeting), + ) login = request.user.person session = Session() @@ -431,7 +445,7 @@ def edit(request, acronym, num=None): if button_text == 'Cancel': return redirect('ietf.secr.sreq.views.view', acronym=acronym) - form = FormClass(group, request.POST, initial=initial) + form = FormClass(group, meeting, 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?) @@ -524,15 +538,13 @@ def edit(request, acronym, num=None): sessions.update(attendees=form.cleaned_data['attendees']) if 'comments' in form.changed_data: sessions.update(comments=form.cleaned_data['comments']) - if 'conflict1' in form.changed_data: - Constraint.objects.filter(meeting=meeting,source=group,name='conflict').delete() - save_conflicts(group,meeting,form.cleaned_data['conflict1'],'conflict') - if 'conflict2' in form.changed_data: - Constraint.objects.filter(meeting=meeting,source=group,name='conflic2').delete() - save_conflicts(group,meeting,form.cleaned_data['conflict2'],'conflic2') - if 'conflict3' in form.changed_data: - Constraint.objects.filter(meeting=meeting,source=group,name='conflic3').delete() - save_conflicts(group,meeting,form.cleaned_data['conflict3'],'conflic3') + + # Handle constraints + for cname, cfield_id in form.wg_constraint_field_ids(): + if cfield_id in form.changed_data: + Constraint.objects.filter(meeting=meeting, source=group, name=cname.slug).delete() + save_conflicts(group, meeting, form.cleaned_data[cfield_id], cname.slug) + if 'adjacent_with_wg' in form.changed_data: Constraint.objects.filter(meeting=meeting, source=group, name='wg_adjacent').delete() save_conflicts(group, meeting, form.cleaned_data['adjacent_with_wg'], 'wg_adjacent') @@ -577,10 +589,17 @@ def edit(request, acronym, num=None): messages.success(request, 'Session Request updated') return redirect('ietf.secr.sreq.views.view', acronym=acronym) - else: + else: # method is not POST + # gather outbound conflicts for initial value + outbound_constraints = defaultdict(list) + for obc in group.constraint_source_set.filter(meeting=meeting, name__is_group_conflict=True): + outbound_constraints[obc.name.slug].append(obc.target.acronym) + for slug, groups in outbound_constraints.items(): + initial['constraint_{}'.format(slug)] = ' '.join(groups) + if not sessions: return redirect('ietf.secr.sreq.views.new', acronym=acronym) - form = FormClass(group, initial=initial) + form = FormClass(group, meeting, initial=initial) return render(request, 'sreq/edit.html', { 'is_locked': is_locked, @@ -666,7 +685,7 @@ def new(request, acronym): ''' group = get_object_or_404(Group, acronym=acronym) meeting = get_meeting(days=14) - session_conflicts = session_conflicts_as_string(group, meeting) + session_conflicts = dict(inbound=inbound_session_conflicts_as_string(group, meeting)) is_virtual = meeting.number in settings.SECR_VIRTUAL_MEETINGS, FormClass = get_session_form_class() @@ -681,7 +700,7 @@ def new(request, acronym): if button_text == 'Cancel': return redirect('ietf.secr.sreq.views.main') - form = FormClass(group, request.POST) + form = FormClass(group, meeting, request.POST) if form.is_valid(): return confirm(request, acronym) @@ -705,12 +724,12 @@ def new(request, acronym): add_essential_people(group,initial) if 'resources' in initial: initial['resources'] = [x.pk for x in initial['resources']] - form = FormClass(group, initial=initial) + form = FormClass(group, meeting, initial=initial) else: initial={} add_essential_people(group,initial) - form = FormClass(group, initial=initial) + form = FormClass(group, meeting, initial=initial) return render(request, 'sreq/new.html', { 'meeting': meeting, @@ -840,8 +859,19 @@ def view(request, acronym, num = None): 'act_by': e.by, } for e in sessions[0].schedulingevent_set.select_related('status', 'by')] - # other groups that list this group in their conflicts - session_conflicts = session_conflicts_as_string(group, meeting) + # gather outbound conflicts + outbound_dict = OrderedDict() + for obc in group.constraint_source_set.filter(meeting=meeting, name__is_group_conflict=True): + if obc.name.slug not in outbound_dict: + outbound_dict[obc.name.slug] = [] + outbound_dict[obc.name.slug].append(obc.target.acronym) + + session_conflicts = dict( + inbound=inbound_session_conflicts_as_string(group, meeting), + outbound=[dict(name=ConstraintName.objects.get(slug=slug), groups=' '.join(groups)) + for slug, groups in outbound_dict.items()], + ) + show_approve_button = False # if sessions include a 3rd session waiting approval and the user is a secretariat or AD of the group diff --git a/ietf/secr/static/secr/css/custom.css b/ietf/secr/static/secr/css/custom.css index c3cc4078a..b7e8e6078 100644 --- a/ietf/secr/static/secr/css/custom.css +++ b/ietf/secr/static/secr/css/custom.css @@ -655,9 +655,7 @@ table#sessions-new-table td { width: 3em; } -#id_conflict1 { width: 37em; } -#id_conflict2 { width: 37em; } -#id_conflict3 { width: 37em; } +input.wg_constraint { width: 37em; } ul.session-buttons { padding-left: 2px; diff --git a/ietf/secr/static/secr/js/sessions.js b/ietf/secr/static/secr/js/sessions.js index 7d087eb13..4635c5603 100644 --- a/ietf/secr/static/secr/js/sessions.js +++ b/ietf/secr/static/secr/js/sessions.js @@ -1,180 +1,137 @@ -function resetfieldstat () { - if (document.form_post.p_num_session.value > 0) { - document.form_post.length_session1.disabled=false; +// Copyright The IETF Trust 2015-2021, All Rights Reserved +var ietf_sessions; // public interface + +(function() { + 'use strict'; + + function stat_ls (val){ + if (val == 0) { + document.form_post.length_session1.disabled = true; + document.form_post.length_session2.disabled = true; + if (document.form_post.length_session3) { document.form_post.length_session3.disabled = true; } + document.form_post.session_time_relation.disabled = true; + document.form_post.joint_for_session.disabled = true; + document.form_post.length_session1.value = 0; + document.form_post.length_session2.value = 0; + document.form_post.length_session3.value = 0; + document.form_post.session_time_relation.value = ''; + document.form_post.joint_for_session.value = ''; + document.form_post.third_session.checked=false; + } + if (val == 1) { + document.form_post.length_session1.disabled = false; + document.form_post.length_session2.disabled = true; + if (document.form_post.length_session3) { document.form_post.length_session3.disabled = true; } + document.form_post.session_time_relation.disabled = true; + document.form_post.joint_for_session.disabled = true; + document.form_post.length_session2.value = 0; + document.form_post.length_session3.value = 0; + document.form_post.session_time_relation.value = ''; + document.form_post.joint_for_session.value = '1'; + document.form_post.third_session.checked=false; + } + if (val == 2) { + document.form_post.length_session1.disabled = false; + document.form_post.length_session2.disabled = false; + if (document.form_post.length_session3) { document.form_post.length_session3.disabled = false; } + document.form_post.session_time_relation.disabled = false; + document.form_post.joint_for_session.disabled = false; + } } - if (document.form_post.p_num_session.value > 1) { - document.form_post.length_session2.disabled=false; + + function check_num_session (val) { + if (document.form_post.num_session.value < val) { + alert("Please change the value in the Number of Sessions to use this field"); + document.form_post.num_session.focused = true; + return true; + } + return false; } - if (document.form_post.prev_third_session.value > 0) { - document.form_post.length_session3.disabled=false; + + function check_third_session () { + if (document.form_post.third_session.checked == false) { + + return true; + } + return false; } - return 1; -} -function stat_ls (val){ - if (val == 0) { - document.form_post.length_session1.disabled = true; - document.form_post.length_session2.disabled = true; - if (document.form_post.length_session3) { document.form_post.length_session3.disabled = true; } - document.form_post.session_time_relation.disabled = true; - document.form_post.joint_for_session.disabled = true; - document.form_post.length_session1.value = 0; - document.form_post.length_session2.value = 0; - document.form_post.length_session3.value = 0; - document.form_post.session_time_relation.value = ''; - document.form_post.joint_for_session.value = ''; - document.form_post.third_session.checked=false; + function delete_last_joint_with_groups () { + var b = document.form_post.joint_with_groups.value; + var temp = b.split(' '); + temp.pop(); + b = temp.join(' '); + document.form_post.joint_with_groups.value = b; + document.form_post.joint_with_groups_selector.selectedIndex=0; } - if (val == 1) { - document.form_post.length_session1.disabled = false; - document.form_post.length_session2.disabled = true; - if (document.form_post.length_session3) { document.form_post.length_session3.disabled = true; } - document.form_post.session_time_relation.disabled = true; - document.form_post.joint_for_session.disabled = true; - document.form_post.length_session2.value = 0; - document.form_post.length_session3.value = 0; - document.form_post.session_time_relation.value = ''; - document.form_post.joint_for_session.value = '1'; - document.form_post.third_session.checked=false; + +/*******************************************************************/ +// WG constraint UI support + +// get the constraint field element for a given slug + function constraint_field(slug) { + return document.getElementById('id_constraint_' + slug); } - if (val == 2) { - document.form_post.length_session1.disabled = false; - document.form_post.length_session2.disabled = false; - if (document.form_post.length_session3) { document.form_post.length_session3.disabled = false; } - document.form_post.session_time_relation.disabled = false; - document.form_post.joint_for_session.disabled = false; + +// get the wg selection element for a given slug + function constraint_selector(slug) { + return document.getElementById('id_wg_selector_' + slug); } -} -function check_num_session (val) { - if (document.form_post.num_session.value < val) { - alert("Please change the value in the Number of Sessions to use this field"); - document.form_post.num_session.focused = true; - return true; + /** + * Handler for constraint select input 'change' event + */ + function wg_constraint_selector_changed() { + let slug = this.getAttribute('data-slug'); + let cfield = constraint_field(slug); + // add selected value to constraint_field + cfield.value += ' ' + this.options[this.selectedIndex].value; } - return false; -} -function check_third_session () { - if (document.form_post.third_session.checked == false) { - - return true; + /** + * Remove the last group in a WG constraint field + * + * @param slug ConstraintName slug + */ + function delete_last_wg_constraint(slug) { + let cfield = constraint_field(slug); + if (cfield) { + var b = cfield.value; + var temp = b.split(' '); + temp.pop(); + b = temp.join(' '); + cfield.value = b; + constraint_selector(slug).selectedIndex = 0; + } } - return false; -} -// All calls to handleconflictfield are being disabled while we hack on the meaning of the three constraint fields -// function handleconflictfield (val) { -// if (val==1) { -// if (document.form_post.conflict1.value.length > 0) { -// document.form_post.conflict2.disabled=false; -// if (document.form_post.conflict2.value.length > 0) { -// document.form_post.conflict3.disabled=false; -// } -// return 1; -// } else { -// if (document.form_post.conflict2.value.length > 0 || document.form_post.conflict3.value.length > 0) { -// alert("Second and Third Conflicts to Avoid fields are being disabled"); -// document.form_post.conflict2.disabled=true; -// document.form_post.conflict3.disabled=true; -// return 0; -// } -// } -// } else { -// if (document.form_post.conflict2.value.length > 0) { -// document.form_post.conflict3.disabled=false; -// return 1; -// } else { -// if (document.form_post.conflict3.value.length > 0) { -// alert("Third Conflicts to Avoid field is being disabled"); -// document.form_post.conflict3.disabled=true; -// return 0; -// } -// } -// } -// return 1; -// } + /** + * Handle click event on a WG constraint's delete button + * + * @param slug ConstraintName slug + */ + function delete_wg_constraint_clicked(slug) { + delete_last_wg_constraint(slug); + } -function delete_last1 () { - var b = document.form_post.conflict1.value; - var temp = new Array(); - temp = b.split(' '); - temp.pop(); - b = temp.join(' '); - document.form_post.conflict1.value = b; - document.form_post.wg_selector1.selectedIndex=0; -} -function delete_last2 () { - var b = document.form_post.conflict2.value; - var temp = new Array(); - temp = b.split(' '); - temp.pop(); - b = temp.join(' '); - document.form_post.conflict2.value = b; - document.form_post.wg_selector2.selectedIndex=0; -} -function delete_last3 () { - var b = document.form_post.conflict3.value; - var temp = new Array(); - temp = b.split(' '); - temp.pop(); - b = temp.join(' '); - document.form_post.conflict3.value = b; - document.form_post.wg_selector3.selectedIndex=0; -} -function delete_last_joint_with_groups () { - var b = document.form_post.joint_with_groups.value; - var temp = new Array(); - temp = b.split(' '); - temp.pop(); - b = temp.join(' '); - document.form_post.joint_with_groups.value = b; - document.form_post.wg_selector4.selectedIndex=0; -} + function on_load() { + // Attach event handlers to constraint selectors + let selectors = document.getElementsByClassName('wg_constraint_selector'); + for (let index = 0; index < selectors.length; index++) { + selectors[index].addEventListener('change', wg_constraint_selector_changed, false) + } -// Not calling check_prior_confict (see ietf/secr/sreq/forms.py definition of SessionForm) -// while we are hacking the use of the current three constraint types around. We could bring -// this back in when we solve the general case of what constraints to use at what meeting. -// When we do, the else should explicitly check for a value of 3. -// function check_prior_conflict(val) { -// if (val == 2) { -// if (document.form_post.conflict1.value=="") { -// alert("Please specify your First Priority prior to using this field"); -// document.form_post.conflict2.disabled=true; -// document.form_post.conflict3.disabled=true; -// document.form_post.wg_selector1.focus(); -// return 0; -// } -// } -// else { -// if (document.form_post.conflict2.value=="" && document.form_post.conflict1.value=="") { -// alert("Please specify your First and Second Priority prior to using this field"); -// document.form_post.conflict3.disabled=true; -// document.form_post.wg_selector1.focus(); -// return 0; -// } else { -// if (document.form_post.conflict2.value=="") { -// alert("Please specify your Second Priority prior to using this field"); -// document.form_post.conflict3.disabled=true; -// document.form_post.wg_selector2.focus(); -// return 0; -// } -// } -// } + } -// return 1; -// } + // initialize after page loads + window.addEventListener('load', on_load, false); -function retrieve_data () { - document.form_post.num_session.selectedIndex = document.form_post.prev_num_session.value; - document.form_post.length_session1.selectedIndex = document.form_post.prev_length_session1.value; - document.form_post.length_session2.selectedIndex = document.form_post.prev_length_session2.value; - document.form_post.length_session3.selectedIndex = document.form_post.prev_length_session3.value; - document.form_post.number_attendee.value = document.form_post.prev_number_attendee.value; - document.form_post.conflict1.value = document.form_post.prev_conflict1.value; - document.form_post.conflict2.value = document.form_post.prev_conflict2.value; - document.form_post.conflict3.value = document.form_post.prev_conflict3.value; - document.form_post.conflict_other.value = document.form_post.prev_conflict_other.value; - document.form_post.special_req.value = document.form_post.prev_special_req.value; - return 1; -} + // expose public interface methods + ietf_sessions = { + stat_ls: stat_ls, + check_num_session: check_num_session, + check_third_session: check_third_session, + delete_last_joint_with_groups: delete_last_joint_with_groups, + delete_wg_constraint_clicked: delete_wg_constraint_clicked + } +})(); \ No newline at end of file diff --git a/ietf/secr/templates/includes/sessions_request_form.html b/ietf/secr/templates/includes/sessions_request_form.html index 4ed8f255b..adfc1c3af 100755 --- a/ietf/secr/templates/includes/sessions_request_form.html +++ b/ietf/secr/templates/includes/sessions_request_form.html @@ -23,33 +23,18 @@ - - - - - - - - - - - - - - + + {% for cname, cfield, cselector in form.wg_constraint_fields %} + + {% if forloop.first %}{% endif %} + + + + {% endfor %} @@ -81,8 +66,8 @@ Joint session with:
(To request one session for multiple WGs together.) - @@ -96,7 +81,7 @@ {% endif %} - (limit 200 characters) +
Other WGs that included {{ group.name }} in their conflict lists:{{ session_conflicts }}
WG Sessions:
You may select multiple WGs within each priority		Chair Conflict:{{ form.wg_selector1 }} - {% comment %}
{% endcomment %} -
- {{ form.conflict1.errors }}{{ form.conflict1 }} -
Technology Overlap:{{ form.wg_selector2 }} - {% comment %}
{% endcomment %} -
- {{ form.conflict2.errors }}{{ form.conflict2 }} -
Key Participant Conflict:{{ form.wg_selector3 }} - {% comment %}
{% endcomment %} -
- {{ form.conflict3.errors }}{{ form.conflict3 }} - 		{{ session_conflicts.inbound }}
WG Sessions:
You may select multiple WGs within each category		{{ cname|title }}{{ cselector }} +
+ {{ cfield.errors }}{{ cfield }} +
BOF Sessions: If the sessions can not be found in the fields above, please enter free form requests in the Special Requests field below.{{ form.wg_selector4 }} -
+ 		{{ form.joint_with_groups_selector }} +
{{ form.joint_with_groups.errors }}{{ form.joint_with_groups }}
Special Requests:
 
i.e. restrictions on meeting times / days, etc.		Special Requests:
 
i.e. restrictions on meeting times / days, etc.
(limit 200 characters)		 {{ form.comments.errors }}{{ form.comments }}
diff --git a/ietf/secr/templates/includes/sessions_request_view.html b/ietf/secr/templates/includes/sessions_request_view.html index 7f31df0bb..595b69e3d 100644 --- a/ietf/secr/templates/includes/sessions_request_view.html +++ b/ietf/secr/templates/includes/sessions_request_view.html @@ -18,16 +18,18 @@ Conflicts to Avoid: - - {% if session.conflict1 %}{% endif %} - {% if session.conflict2 %}{% endif %} - {% if session.conflict3 %}{% endif %} -
Chair Conflict: {{ session.conflict1 }}
Technology Overlap: {{ session.conflict2 }}
Key Participant Conflict: {{ session.conflict3 }}
+ {% if session_conflicts.outbound %} + + {% for conflict in session_conflicts.outbound %} + + {% endfor %} +
{{ conflict.name|title }}: {{ conflict.groups }}
+ {% else %}None{% endif %} Other WGs that included {{ group }} in their conflict list: - {% if session_conflicts %}{{ session_conflicts }}{% else %}None so far{% endif %} + {% if session_conflicts.inbound %}{{ session_conflicts.inbound }}{% else %}None so far{% endif %} {% if not is_virtual %} diff --git a/ietf/static/ietf/js/agenda/agenda_listeners.js b/ietf/static/ietf/js/agenda/agenda_listeners.js index f5bee200f..3ee70b5de 100644 --- a/ietf/static/ietf/js/agenda/agenda_listeners.js +++ b/ietf/static/ietf/js/agenda/agenda_listeners.js @@ -839,7 +839,7 @@ function group_name_or_empty(constraint) { function draw_constraints(session) { if("conflicts" in session) { - var display = { 'conflict':'1' , 'conflic2':'2' , 'conflic3':'3' }; + var display = agenda_globals.group_conflict_labels; var group_icons = ""; var group_set = {}; $.each(session.conflicts, function(index) { diff --git a/ietf/static/ietf/js/agenda/agenda_objects.js b/ietf/static/ietf/js/agenda/agenda_objects.js index 0b42aee30..441f27b81 100644 --- a/ietf/static/ietf/js/agenda/agenda_objects.js +++ b/ietf/static/ietf/js/agenda/agenda_objects.js @@ -31,6 +31,19 @@ function AgendaGlobals() { this.assignment_promise = undefined; this.timeslot_promise = undefined; this.__debug_session_move = false; + /* Group_conflict_labels defines constraint names that are group conflicts and + * the symbols used to represent them. Shadow the old 1-2-3 labels to maintain + * behavior from before the new conflict types were introduced. This may need + * to be changed if the old editor is not phased out. */ + this.group_conflict_labels = { + 'conflict':'1' , + 'conflic2':'2' , + 'conflic3':'3', + 'chair_conflict': '1', + 'tech_overlap': '2', + 'key_participant': '3' + }; + } function createLine(x1,y1, x2,y2){ @@ -160,53 +173,29 @@ function find_and_populate_conflicts(session_obj) { var room_tag = null; session_obj.reset_conflicts(); - for(ccn in session_obj.column_class_list) { - var vertical_location = session_obj.column_class_list[ccn].column_tag; - var room_tag = session_obj.column_class_list[ccn].room_tag; + for(let ccn in session_obj.column_class_list) { + if (session_obj.column_class_list.hasOwnProperty(ccn)) { + var vertical_location = session_obj.column_class_list[ccn].column_tag; + var room_tag = session_obj.column_class_list[ccn].room_tag; + $.each(Object.keys(agenda_globals.group_conflict_labels), (i, conflict_name) => { + if (session_obj.constraints[conflict_name]) { + $.each(session_obj.constraints[conflict_name], (j, conflict) => { + calculate_real_conflict(conflict, vertical_location, room_tag, session_obj); + }); + } + if(session_obj.theirconstraints.conflict){ + $.each(session_obj.theirconstraints.conflict, (i, conflict) => { + calculate_real_conflict(conflict, vertical_location, room_tag, session_obj); + }); + } + } + ); - if(session_obj.constraints.conflict != null){ - $.each(session_obj.constraints.conflict, function(i){ - var conflict = session_obj.constraints.conflict[i]; - calculate_real_conflict(conflict, vertical_location, room_tag, session_obj); - }); + /* bethere constraints are processed in another loop */ } - if(session_obj.constraints.conflic2 != null){ - $.each(session_obj.constraints.conflic2, function(i){ - var conflict = session_obj.constraints.conflic2[i]; - calculate_real_conflict(conflict, vertical_location, room_tag, session_obj); - }); - } - if(session_obj.constraints.conflic3 != null){ - $.each(session_obj.constraints.conflic3, function(i){ - var conflict = session_obj.constraints.conflic3[i]; - calculate_real_conflict(conflict, vertical_location, room_tag, session_obj); - }); - } - - if(session_obj.theirconstraints.conflict != null){ - $.each(session_obj.theirconstraints.conflict, function(i){ - var conflict = session_obj.theirconstraints.conflict[i]; - calculate_real_conflict(conflict, vertical_location, room_tag, session_obj); - }); - } - if(session_obj.theirconstraints.conflic2 != null){ - $.each(session_obj.theirconstraints.conflic2, function(i){ - var conflict = session_obj.theirconstraints.conflic2[i]; - calculate_real_conflict(conflict, vertical_location, room_tag, session_obj); - }); - } - if(session_obj.theirconstraints.conflic3 != null){ - $.each(session_obj.theirconstraints.conflic3, function(i){ - var conflict = session_obj.theirconstraints.conflic3[i]; - calculate_real_conflict(conflict, vertical_location, room_tag, session_obj); - }); - } - - /* bethere constraints are processed in another loop */ } } - function show_non_conflicting_spots(ss_id){ var conflict_spots = [] $.each(conflict_classes, function(key){ @@ -961,7 +950,7 @@ Session.prototype.show_conflict = function() { if(_conflict_debug) { console.log("showing conflict for", this.title, this.conflict_level['ours'],this.conflict_level['theirs']); } - var display = { 'conflict':'1' , 'conflic2':'2' , 'conflic3':'3' }; + var display = agenda_globals.group_conflict_labels; if (this.conflicted) { if ('ours' in this.conflict_level) { this.element().find('.ourconflicts').text('->'+display[this.conflict_level.ours]); @@ -1329,36 +1318,18 @@ Session.prototype.fill_in_constraints = function(constraint_list) { // here we can sort the constraints by group name. // make a single list. this.constraints is not an array, can not use concat. this.conflicts = []; - if("conflict" in this.constraints) { - $.each(this.constraints["conflict"], function(index) { - session_obj.conflicts.push(session_obj.constraints["conflict"][index]); - }); - } - if("conflic2" in this.constraints) { - $.each(this.constraints["conflic2"], function(index) { - session_obj.conflicts.push(session_obj.constraints["conflic2"][index]); - }); - } - if("conflic3" in this.constraints) { - $.each(this.constraints["conflic3"], function(index) { - session_obj.conflicts.push(session_obj.constraints["conflic3"][index]); - }); - } - if("conflict" in this.theirconstraints) { - $.each(this.theirconstraints["conflict"], function(index) { - session_obj.conflicts.push(session_obj.theirconstraints["conflict"][index]); - }); - } - if("conflic2" in this.theirconstraints) { - $.each(this.theirconstraints["conflic2"], function(index) { - session_obj.conflicts.push(session_obj.theirconstraints["conflic2"][index]); - }); - } - if("conflic3" in this.theirconstraints) { - $.each(this.theirconstraints["conflic3"], function(index) { - session_obj.conflicts.push(session_obj.theirconstraints["conflic3"][index]); - }); - } + $.each(Object.keys(agenda_globals.group_conflict_labels), (i, conflict_name) => { + if (conflict_name in this.constraints) { + $.each(this.constraints[conflict_name], (j, conflict) => { + session_obj.conflicts.push(conflict); + }); + } + if (conflict_name in this.theirconstraints) { + $.each(this.theirconstraints[conflict_name], (j, conflict) => { + session_obj.conflicts.push(conflict); + }); + } + }); this.calculate_bethere(); this.conflicts = sort_conflict_list(this.conflicts) }; @@ -1532,9 +1503,9 @@ var conflict_classes = {}; function clear_conflict_classes() { // remove all conflict boxes from before - $(".show_conflict_specific_box").removeClass("show_conflict_specific_box"); - $(".show_conflic2_specific_box").removeClass("show_conflic2_specific_box"); - $(".show_conflic3_specific_box").removeClass("show_conflic3_specific_box"); + $.each(Object.keys(agenda_globals.group_conflict_labels), (i, conflict_name) => { + $(".show_" + conflict_name + "_specific_box").removeClass("show_" + conflict_name + "_specific_box"); + }); // reset all column headings $(".show_conflict_view_highlight").removeClass("show_conflict_view_highlight"); @@ -1547,26 +1518,28 @@ Constraint.prototype.column_class_list = function() { return this.othergroup.column_class_list; }; +/* N.B., handling new conflict types as equivalent to the originals for prioritization. + * If this editor is not replaced by the new one, it might be worth sorting out how to + * properly prioritize. Otherwise, this maintains the behavior seen prior to the introduction + * of the new conflicts. */ Constraint.prototype.conflict1P = function() { - return (this.conflict_type == "conflict") + return ((this.conflict_type === "conflict") || (this.conflict_type === "chair_conflict")); }; Constraint.prototype.conflict2P = function() { - return (this.conflict_type == "conflic2") + return ((this.conflict_type === "conflic2") || (this.conflict_type === "tech_overlap")); }; Constraint.prototype.conflict3P = function() { - return (this.conflict_type == "conflic3") + return ((this.conflict_type === "conflic3") || (this.conflict_type === "key_participant")); }; Constraint.prototype.conflict_groupP = function() { - return (this.conflict_type == "conflict" || - this.conflict_type == "conflic2" || - this.conflict_type == "conflic3"); + return this.conflict_type in agenda_globals.group_conflict_labels; }; Constraint.prototype.conflict_peopleP = function() { - return (this.conflict_type == "bethere") + return (this.conflict_type === "bethere") }; Constraint.prototype.conflict_compare = function(oflict) {