From 34891213ff89a94c91aa04496e810b397f18c3aa Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Tue, 22 Jun 2021 16:33:04 +0000 Subject: [PATCH] Swap timeslot columns in addition to full days in schedule editor. Fixes #3216. Commit ready for merge. - Legacy-Id: 19138 --- ietf/meeting/forms.py | 57 +++++- ietf/meeting/tests_js.py | 23 ++- ietf/meeting/tests_views.py | 181 ++++++++++++++++++ ietf/meeting/views.py | 38 +++- ietf/static/ietf/css/ietf.css | 15 ++ ietf/static/ietf/js/edit-meeting-schedule.js | 77 ++++++-- .../meeting/edit_meeting_schedule.html | 59 +++++- 7 files changed, 422 insertions(+), 28 deletions(-) diff --git a/ietf/meeting/forms.py b/ietf/meeting/forms.py index 5f87e38a8..91c43b481 100644 --- a/ietf/meeting/forms.py +++ b/ietf/meeting/forms.py @@ -17,7 +17,7 @@ import debug # pyflakes:ignore from ietf.doc.models import Document, DocAlias, State, NewRevisionDocEvent from ietf.group.models import Group, GroupFeatures from ietf.ietfauth.utils import has_role -from ietf.meeting.models import Session, Meeting, Schedule, countries, timezones +from ietf.meeting.models import Session, Meeting, Schedule, countries, timezones, TimeSlot, Room from ietf.meeting.helpers import get_next_interim_number, make_materials_directories from ietf.meeting.helpers import is_interim_meeting_approved, get_next_agenda_name from ietf.message.models import Message @@ -362,3 +362,58 @@ class RequestMinutesForm(forms.Form): cc = MultiEmailField(required=False) subject = forms.CharField() body = forms.CharField(widget=forms.Textarea,strip=False) + + +class SwapDaysForm(forms.Form): + source_day = forms.DateField(required=True) + target_day = forms.DateField(required=True) + + +class CsvModelPkInput(forms.TextInput): + """Text input that expects a CSV list of PKs of a model instances""" + def format_value(self, value): + """Convert value to contents of input text widget + + Value is a list of pks, or None + """ + return '' if value is None else ','.join(str(v) for v in value) + + def value_from_datadict(self, data, files, name): + """Convert data back to list of PKs""" + value = super(CsvModelPkInput, self).value_from_datadict(data, files, name) + return value.split(',') + + +class SwapTimeslotsForm(forms.Form): + """Timeslot swap form + + Interface uses timeslot instances rather than time/duration to simplify handling in + the JavaScript. This might make more sense with a DateTimeField and DurationField for + origin/target. Instead, grabs time and duration from a TimeSlot. + + This is not likely to be practical as a rendered form. Current use is to validate + data from an ad hoc form. In an ideal world, this would be refactored to use a complex + custom widget, but unless it proves to be reused that would be a poor investment of time. + """ + origin_timeslot = forms.ModelChoiceField( + required=True, + queryset=TimeSlot.objects.none(), # default to none, fill in when we have a meeting + widget=forms.TextInput, + ) + target_timeslot = forms.ModelChoiceField( + required=True, + queryset=TimeSlot.objects.none(), # default to none, fill in when we have a meeting + widget=forms.TextInput, + ) + rooms = forms.ModelMultipleChoiceField( + required=True, + queryset=Room.objects.none(), # default to none, fill in when we have a meeting + widget=CsvModelPkInput, + ) + + def __init__(self, meeting, *args, **kwargs): + super(SwapTimeslotsForm, self).__init__(*args, **kwargs) + self.meeting = meeting + self.fields['origin_timeslot'].queryset = meeting.timeslot_set.all() + self.fields['target_timeslot'].queryset = meeting.timeslot_set.all() + self.fields['rooms'].queryset = meeting.room_set.all() diff --git a/ietf/meeting/tests_js.py b/ietf/meeting/tests_js.py index d2ebc836b..573ec4701 100644 --- a/ietf/meeting/tests_js.py +++ b/ietf/meeting/tests_js.py @@ -50,7 +50,9 @@ class EditMeetingScheduleTests(IetfSeleniumTestCase): schedule = Schedule.objects.filter(meeting=meeting, owner__user__username="plain").first() room1 = Room.objects.get(name="Test Room") - slot1 = TimeSlot.objects.filter(meeting=meeting, location=room1).order_by('time').first() + slot1 = TimeSlot.objects.filter(meeting=meeting, location=room1, type='regular').order_by('time').first() + slot1b = TimeSlot.objects.filter(meeting=meeting, location=room1, type='regular').order_by('time').last() + self.assertNotEqual(slot1.pk, slot1b.pk) room2 = Room.objects.create(meeting=meeting, name="Test Room2", capacity=1) room2.session_types.add('regular') @@ -251,7 +253,6 @@ class EditMeetingScheduleTests(IetfSeleniumTestCase): self.assertIn('selected', s1_element.get_attribute('class'), 'Session should be selectable when parent enabled') - # hide timeslots self.driver.find_element_by_css_selector(".timeslot-group-toggles button").click() self.assertTrue(self.driver.find_element_by_css_selector("#timeslot-group-toggles-modal").is_displayed()) @@ -260,12 +261,26 @@ class EditMeetingScheduleTests(IetfSeleniumTestCase): self.assertTrue(not self.driver.find_element_by_css_selector("#timeslot-group-toggles-modal").is_displayed()) # swap days - self.driver.find_element_by_css_selector(".day [data-target=\"#swap-days-modal\"][data-dayid=\"{}\"]".format(slot4.time.date().isoformat())).click() + self.driver.find_element_by_css_selector(".day .swap-days[data-dayid=\"{}\"]".format(slot4.time.date().isoformat())).click() self.assertTrue(self.driver.find_element_by_css_selector("#swap-days-modal").is_displayed()) self.driver.find_element_by_css_selector("#swap-days-modal input[name=\"target_day\"][value=\"{}\"]".format(slot1.time.date().isoformat())).click() self.driver.find_element_by_css_selector("#swap-days-modal button[type=\"submit\"]").click() - self.assertTrue(self.driver.find_elements_by_css_selector('#timeslot{} #session{}'.format(slot4.pk, s1.pk))) + self.assertTrue(self.driver.find_elements_by_css_selector('#timeslot{} #session{}'.format(slot4.pk, s1.pk)), + 'Session s1 should have moved to second meeting day') + + # swap timeslot column - put session in a differently-timed timeslot + self.driver.find_element_by_css_selector( + '.day .swap-timeslot-col[data-timeslot-pk="{}"]'.format(slot1b.pk) + ).click() # open modal on the second timeslot for room1 + self.assertTrue(self.driver.find_element_by_css_selector("#swap-timeslot-col-modal").is_displayed()) + self.driver.find_element_by_css_selector( + '#swap-timeslot-col-modal input[name="target_timeslot"][value="{}"]'.format(slot4.pk) + ).click() # select room1 timeslot that has a session in it + self.driver.find_element_by_css_selector('#swap-timeslot-col-modal button[type="submit"]').click() + + self.assertTrue(self.driver.find_elements_by_css_selector('#timeslot{} #session{}'.format(slot1b.pk, s1.pk)), + 'Session s1 should have moved to second timeslot on first meeting day') def test_unassigned_sessions_sort(self): """Unassigned session sorting should behave correctly diff --git a/ietf/meeting/tests_views.py b/ietf/meeting/tests_views.py index 632d29e75..85aa9dd31 100644 --- a/ietf/meeting/tests_views.py +++ b/ietf/meeting/tests_views.py @@ -994,6 +994,187 @@ class EditMeetingScheduleTests(TestCase): self.assertIn('BoF', bof_tags.eq(0).text(), 'BoF tag should contain text "BoF"') + def _setup_for_swap_timeslots(self): + """Create a meeting, rooms, and schedule for swap_timeslots testing + + Creates two groups of rooms with disjoint timeslot sets, modeling the room grouping in + the edit_meeting_schedule view. + """ + # Meeting must be in the future so it can be edited + meeting = MeetingFactory( + type_id='ietf', + date=datetime.date.today() + datetime.timedelta(days=7), + populate_schedule=False, + ) + meeting.schedule = ScheduleFactory(meeting=meeting) + meeting.save() + + # Create room groups + room_groups = [ + RoomFactory.create_batch(2, meeting=meeting), + RoomFactory.create_batch(2, meeting=meeting), + ] + + # Set up different sets of timeslots + t0 = datetime.datetime.combine(meeting.date, datetime.time(11, 0)) + dur = datetime.timedelta(hours=2) + for room in room_groups[0]: + TimeSlotFactory(meeting=meeting, location=room, duration=dur, time=t0) + TimeSlotFactory(meeting=meeting, location=room, duration=dur, time=t0 + datetime.timedelta(days=1, hours=2)) + TimeSlotFactory(meeting=meeting, location=room, duration=dur, time=t0 + datetime.timedelta(days=2, hours=4)) + + for room in room_groups[1]: + TimeSlotFactory(meeting=meeting, location=room, duration=dur, time=t0 + datetime.timedelta(hours=1)) + TimeSlotFactory(meeting=meeting, location=room, duration=dur, time=t0 + datetime.timedelta(days=1, hours=3)) + TimeSlotFactory(meeting=meeting, location=room, duration=dur, time=t0 + datetime.timedelta(days=2, hours=5)) + + # And now put sessions in the timeslots + for ts in meeting.timeslot_set.all(): + SessionFactory( + meeting=meeting, + name=str(ts.pk), # label to identify where it started + add_to_schedule=False, + ).timeslotassignments.create( + timeslot=ts, + schedule=meeting.schedule, + ) + return meeting, room_groups + + def test_swap_timeslots(self): + """Schedule timeslot groups should swap properly + + This tests the case currently exercised by the UI - where the rooms are grouped according to + entirely equivalent sets of timeslots. Thus, there is always a matching timeslot for every (or no) + room as long as the rooms parameter to the ajax call includes only one group. + """ + meeting, room_groups = self._setup_for_swap_timeslots() + + url = urlreverse('ietf.meeting.views.edit_meeting_schedule', kwargs=dict(num=meeting.number)) + username = meeting.schedule.owner.user.username + self.client.login(username=username, password=username + '+password') + + # Swap group 0's first and last sessions + r = self.client.post( + url, + dict( + action='swaptimeslots', + origin_timeslot=str(room_groups[0][0].timeslot_set.first().pk), + target_timeslot=str(room_groups[0][0].timeslot_set.last().pk), + rooms=','.join([str(room.pk) for room in room_groups[0]]), + ) + ) + self.assertEqual(r.status_code, 302) + + # Validate results + for index, room in enumerate(room_groups[0]): + timeslots = list(room.timeslot_set.all()) + self.assertEqual(timeslots[0].session.name, str(timeslots[-1].pk), + 'Session from last timeslot in room (0, {}) should now be in first'.format(index)) + self.assertEqual(timeslots[-1].session.name, str(timeslots[0].pk), + 'Session from first timeslot in room (0, {}) should now be in last'.format(index)) + self.assertEqual( + [ts.session.name for ts in timeslots[1:-1]], + [str(ts.pk) for ts in timeslots[1:-1]], + 'Sessions in middle timeslots should be unchanged' + ) + for index, room in enumerate(room_groups[1]): + timeslots = list(room.timeslot_set.all()) + self.assertFalse( + any(ts.session is None for ts in timeslots), + "Sessions in other room group's timeslots should still be assigned" + ) + self.assertEqual( + [ts.session.name for ts in timeslots], + [str(ts.pk) for ts in timeslots], + "Sessions in other room group's timeslots should be unchanged" + ) + + def test_swap_timeslots_handles_unmatched(self): + """Sessions in unmatched timeslots should be unassigned when swapped + + This more generally tests the back end by exercising the situation where a timeslot in the + affected rooms does not have an equivalent timeslot target. This is not used by the UI as of + now (2021-06-22), but should function correctly. + """ + meeting, room_groups = self._setup_for_swap_timeslots() + + # Remove a timeslot and session from only one room in group 0 + ts_to_remove = room_groups[0][1].timeslot_set.last() + ts_to_remove.session.delete() + ts_to_remove.delete() # our object still exists but has no db object + + # Add a matching timeslot to group 1 so we can be sure it's being ignored. + # If not, this session will be unassigned when we swap timeslots on group 0. + new_ts = TimeSlotFactory( + meeting=meeting, + location=room_groups[1][0], + duration=ts_to_remove.duration, + time=ts_to_remove.time, + ) + SessionFactory( + meeting=meeting, + name=str(new_ts.pk), + add_to_schedule=False, + ).timeslotassignments.create( + timeslot=new_ts, + schedule=meeting.schedule, + ) + + url = urlreverse('ietf.meeting.views.edit_meeting_schedule', kwargs=dict(num=meeting.number)) + username = meeting.schedule.owner.user.username + self.client.login(username=username, password=username + '+password') + + # Now swap between first and last timeslots in group 0 + r = self.client.post( + url, + dict( + action='swaptimeslots', + origin_timeslot=str(room_groups[0][0].timeslot_set.first().pk), + target_timeslot=str(room_groups[0][0].timeslot_set.last().pk), + rooms=','.join([str(room.pk) for room in room_groups[0]]), + ) + ) + self.assertEqual(r.status_code, 302) + + # Validate results + for index, room in enumerate(room_groups[0]): + timeslots = list(room.timeslot_set.all()) + if index == 1: + # special case - this has no matching timeslot because we deleted it above + self.assertIsNone(timeslots[0].session, 'Unmatched timeslot should be empty after swap') + session_that_should_be_unassigned = Session.objects.get(name=str(timeslots[0].pk)) + self.assertEqual(session_that_should_be_unassigned.timeslotassignments.count(), 0, + 'Session that was in an unmatched timeslot should now be unassigned') + # check from 2nd timeslot to the last since we deleted the original last timeslot + self.assertEqual( + [ts.session.name for ts in timeslots[1:]], + [str(ts.pk) for ts in timeslots[1:]], + 'Sessions in middle timeslots should be unchanged' + ) + else: + self.assertEqual(timeslots[0].session.name, str(timeslots[-1].pk), + 'Session from last timeslot in room (0, {}) should now be in first'.format(index)) + self.assertEqual(timeslots[-1].session.name, str(timeslots[0].pk), + 'Session from first timeslot in room (0, {}) should now be in last'.format(index)) + self.assertEqual( + [ts.session.name for ts in timeslots[1:-1]], + [str(ts.pk) for ts in timeslots[1:-1]], + 'Sessions in middle timeslots should be unchanged' + ) + + # Still should have no effect on other rooms, even if they matched a timeslot + for index, room in enumerate(room_groups[1]): + timeslots = list(room.timeslot_set.all()) + self.assertFalse( + any(ts.session is None for ts in timeslots), + "Sessions in other room group's timeslots should still be assigned" + ) + self.assertEqual( + [ts.session.name for ts in timeslots], + [str(ts.pk) for ts in timeslots], + "Sessions in other room group's timeslots should be unchanged" + ) + class ReorderSlidesTests(TestCase): diff --git a/ietf/meeting/views.py b/ietf/meeting/views.py index 52ed79502..986bbe66d 100644 --- a/ietf/meeting/views.py +++ b/ietf/meeting/views.py @@ -56,7 +56,7 @@ from ietf.ietfauth.utils import role_required, has_role, user_is_person from ietf.mailtrigger.utils import gather_address_lists from ietf.meeting.models import Meeting, Session, Schedule, FloorPlan, SessionPresentation, TimeSlot, SlideSubmission from ietf.meeting.models import SessionStatusName, SchedulingEvent, SchedTimeSessAssignment, Room, TimeSlotTypeName -from ietf.meeting.forms import CustomDurationField +from ietf.meeting.forms import CustomDurationField, SwapDaysForm, SwapTimeslotsForm from ietf.meeting.helpers import get_areas, get_person_by_email, get_schedule_by_name from ietf.meeting.helpers import build_all_agenda_slices, get_wg_name_list from ietf.meeting.helpers import get_all_assignments_from_schedule @@ -455,11 +455,6 @@ def new_meeting_schedule(request, num, owner=None, name=None): 'form': form, }) - -class SwapDaysForm(forms.Form): - source_day = forms.DateField(required=True) - target_day = forms.DateField(required=True) - @ensure_csrf_cookie def edit_meeting_schedule(request, num=None, owner=None, name=None): meeting = get_meeting(num) @@ -794,6 +789,37 @@ def edit_meeting_schedule(request, num=None, owner=None, name=None): return HttpResponseRedirect(request.get_full_path()) + elif action == 'swaptimeslots': + # Swap sets of timeslots with equal start/end time for a given set of rooms. + # Gets start and end times from TimeSlot instances for the origin and target, + # then swaps all timeslots for the requested rooms whose start/end match those. + # The origin/target timeslots do not need to be the same duration. + swap_timeslots_form = SwapTimeslotsForm(meeting, request.POST) + if not swap_timeslots_form.is_valid(): + return HttpResponse("Invalid swap: {}".format(swap_timeslots_form.errors), status=400) + + affected_rooms = swap_timeslots_form.cleaned_data['rooms'] + origin_timeslot = swap_timeslots_form.cleaned_data['origin_timeslot'] + target_timeslot = swap_timeslots_form.cleaned_data['target_timeslot'] + + origin_timeslots = meeting.timeslot_set.filter( + location__in=affected_rooms, + time=origin_timeslot.time, + duration=origin_timeslot.duration, + ) + target_timeslots = meeting.timeslot_set.filter( + location__in=affected_rooms, + time=target_timeslot.time, + duration=target_timeslot.duration, + ) + swap_meeting_schedule_timeslot_assignments( + schedule, + list(origin_timeslots), + list(target_timeslots), + target_timeslot.time - origin_timeslot.time, + ) + return HttpResponseRedirect(request.get_full_path()) + return HttpResponse("Invalid parameters", status=400) # Show only rooms that have regular sessions diff --git a/ietf/static/ietf/css/ietf.css b/ietf/static/ietf/css/ietf.css index adfc7ead5..2f8a0d6d5 100644 --- a/ietf/static/ietf/css/ietf.css +++ b/ietf/static/ietf/css/ietf.css @@ -1356,6 +1356,21 @@ a.fc-event, .fc-event, .fc-content, .fc-title, .fc-event-container { cursor: pointer; } +.edit-meeting-schedule .modal .day-options { + display: flex; + flex-flow: row wrap; +} + +.edit-meeting-schedule .modal .timeslot-options { + display: flex; + flex-flow: column nowrap; + justify-content: flex-start; +} + +.edit-meeting-schedule .modal .room-group { + margin: 2em; +} + .edit-meeting-schedule .scheduling-panel .session-info-container { padding-left: 0.5em; flex: 0 0 25em; diff --git a/ietf/static/ietf/js/edit-meeting-schedule.js b/ietf/static/ietf/js/edit-meeting-schedule.js index 1936b7b46..a6f36ee6a 100644 --- a/ietf/static/ietf/js/edit-meeting-schedule.js +++ b/ietf/static/ietf/js/edit-meeting-schedule.js @@ -290,31 +290,80 @@ jQuery(document).ready(function () { } }); + + // Helpers for swap days / timeslots + // Enable or disable a swap modal's submit button + let updateSwapSubmitButton = function (modal, inputName) { + modal.find("button[type=submit]").prop( + "disabled", + modal.find("input[name='" + inputName + "']:checked").length === 0 + ); + }; + + // Disable a particular swap modal radio input + let updateSwapRadios = function (labels, radios, disableValue) { + labels.removeClass('text-muted'); + radios.prop('disabled', false); + radios.prop('checked', false); + let disableInput = radios.filter('[value="' + disableValue + '"]'); + if (disableInput) { + disableInput.parent().addClass('text-muted'); + disableInput.prop('disabled', true); + } + return disableInput; // return the input that was disabled, if any + }; + // swap days + let swapDaysModal = content.find("#swap-days-modal"); + let swapDaysLabels = swapDaysModal.find(".modal-body label"); + let swapDaysRadios = swapDaysLabels.find('input[name=target_day]'); + let updateSwapDaysSubmitButton = function () { + updateSwapSubmitButton(swapDaysModal, 'target_day') + }; + // handler to prep and open the modal content.find(".swap-days").on("click", function () { let originDay = this.dataset.dayid; - let modal = content.find("#swap-days-modal"); - let radios = modal.find(".modal-body label"); - radios.removeClass("text-muted"); - radios.find("input[name=target_day]").prop("disabled", false).prop("checked", false); + let originRadio = updateSwapRadios(swapDaysLabels, swapDaysRadios, originDay); - let originRadio = radios.find("input[name=target_day][value=" + originDay + "]"); - originRadio.parent().addClass("text-muted"); - originRadio.prop("disabled", true); + // Fill in label in the modal title + swapDaysModal.find(".modal-title .day").text(jQuery.trim(originRadio.parent().text())); - modal.find(".modal-title .day").text(jQuery.trim(originRadio.parent().text())); - modal.find("input[name=source_day]").val(originDay); + // Fill in the hidden form fields + swapDaysModal.find("input[name=source_day]").val(originDay); updateSwapDaysSubmitButton(); + swapDaysModal.modal('show'); // show via JS so it won't open until it is initialized }); + swapDaysRadios.on("change", function () {updateSwapDaysSubmitButton()}); - function updateSwapDaysSubmitButton() { - content.find("#swap-days-modal button[type=submit]").prop("disabled", content.find("#swap-days-modal input[name=target_day]:checked").length == 0); - } + // swap timeslot columns + let swapTimeslotsModal = content.find('#swap-timeslot-col-modal'); + let swapTimeslotsLabels = swapTimeslotsModal.find(".modal-body label"); + let swapTimeslotsRadios = swapTimeslotsLabels.find('input[name=target_timeslot]'); + let updateSwapTimeslotsSubmitButton = function () { + updateSwapSubmitButton(swapTimeslotsModal, 'target_timeslot'); + }; + // handler to prep and open the modal + content.find('.swap-timeslot-col').on('click', function() { + let roomGroup = this.closest('.room-group').dataset; + updateSwapRadios(swapTimeslotsLabels, swapTimeslotsRadios, this.dataset.timeslotPk) - content.find("#swap-days-modal input[name=target_day]").on("change", function () { - updateSwapDaysSubmitButton(); + // show only options for this room group + swapTimeslotsModal.find('.room-group').hide(); + swapTimeslotsModal.find('.room-group-' + roomGroup.index).show(); + + // Fill in label in the modal title + swapTimeslotsModal.find('.modal-title .origin-label').text(this.dataset.originLabel); + + // Fill in the hidden form fields + swapTimeslotsModal.find('input[name="origin_timeslot"]').val(this.dataset.timeslotPk); + swapTimeslotsModal.find('input[name="rooms"]').val(roomGroup.rooms); + + // Open the modal via JS so it won't open until it is initialized + updateSwapTimeslotsSubmitButton(); + swapTimeslotsModal.modal('show'); }); + swapTimeslotsRadios.on("change", function () {updateSwapTimeslotsSubmitButton()}); } // hints for the current schedule diff --git a/ietf/templates/meeting/edit_meeting_schedule.html b/ietf/templates/meeting/edit_meeting_schedule.html index 4b24c3a35..16ffea363 100644 --- a/ietf/templates/meeting/edit_meeting_schedule.html +++ b/ietf/templates/meeting/edit_meeting_schedule.html @@ -90,16 +90,25 @@ {% for day, day_data in days.items %}
- {{ day|date:"l" }}
+ {{ day|date:"l" }}
{{ day|date:"N j, Y" }}
{% for rgroup in day_data %} -
+
{# All rooms in a group have same timeslots; grab the first for the labels #} {% for t in rgroup.0.timeslots %} -
{{ t.time|date:"G:i" }} - {{ t.end_time|date:"G:i" }}
+
+ + {{ t.time|date:"G:i" }} - {{ t.end_time|date:"G:i" }} + + +
{% endfor %}
{% for room_data in rgroup %}{% with room_data.room as room %} @@ -221,5 +230,49 @@
+ +
{% endblock %}