Swap timeslot columns in addition to full days in schedule editor. Fixes ietf-tools#3216. Commit ready for merge.

 - Legacy-Id: 19138

Author: jennifer-richards

ietf/meeting/forms.py

@@ -17,7 +17,7 @@
 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()

ietf/meeting/tests_js.py

@@ -50,7 +50,9 @@ def test_edit_meeting_schedule(self):
         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 @@ def test_edit_meeting_schedule(self):
         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 @@ def test_edit_meeting_schedule(self):
         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

ietf/meeting/tests_views.py

@@ -994,6 +994,187 @@ def test_bof_session_tag(self):
         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):
 

ietf/meeting/views.py

@@ -56,7 +56,7 @@
 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 prepare_timeslots_for_display(timeslots, rooms):
 
             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

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;