Merged in [19121] from jennifer@painless-security.com:

Display rooms in blocks with identical timeslots. Add a timeslot label row above each. Fixes #3220.
 - Legacy-Id: 19126
Note: SVN reference [19121] has been migrated to Git commit b42f1a2d7f

ietf/meeting/factories.py

@@ -131,6 +131,14 @@ class RoomFactory(factory.DjangoModelFactory):
     meeting = factory.SubFactory(MeetingFactory)
     name = factory.Faker('name')
 
+    @factory.post_generation
+    def session_types(obj, create, extracted, **kwargs): # pylint: disable=no-self-argument
+        """Prep session types m2m relationship for room, defaulting to 'regular'"""
+        if create:
+            session_types = extracted if extracted is not None else ['regular']
+            for st in session_types:
+                obj.session_types.add(st)
+
 
 class TimeSlotFactory(factory.DjangoModelFactory):
     class Meta:

ietf/meeting/tests_js.py

@@ -24,7 +24,7 @@ from ietf.group import colors
 from ietf.person.models import Person
 from ietf.group.models import Group
 from ietf.group.factories import GroupFactory
-from ietf.meeting.factories import MeetingFactory, SessionFactory, TimeSlotFactory
+from ietf.meeting.factories import MeetingFactory, RoomFactory, SessionFactory, TimeSlotFactory
 from ietf.meeting.test_data import make_meeting_test_data, make_interim_meeting
 from ietf.meeting.models import (Schedule, SchedTimeSessAssignment, Session,
                                  Room, TimeSlot, Constraint, ConstraintName,
@@ -273,6 +273,9 @@ class EditMeetingScheduleTests(IetfSeleniumTestCase):
         # Create an IETF meeting...
         meeting = MeetingFactory(type_id='ietf')
 
+        # ...add a room that has no timeslots to be sure it's handled...
+        RoomFactory(meeting=meeting)
+
         # ...and sessions for the groups. Use durations that are in a different order than
         # area or name. The wgs list is in ascending acronym order, so use descending durations.
         sessions = []
@@ -297,7 +300,6 @@ class EditMeetingScheduleTests(IetfSeleniumTestCase):
         self.login('secretary')
         self.driver.get(url)
 
-
         select = self.driver.find_element_by_name('sort_unassigned')
         options = {
             opt.get_attribute('value'): opt

ietf/meeting/tests_views.py

@@ -49,7 +49,7 @@ from ietf.utils.text import xslugify
 from ietf.person.factories import PersonFactory
 from ietf.group.factories import GroupFactory, GroupEventFactory, RoleFactory
 from ietf.meeting.factories import ( SessionFactory, SessionPresentationFactory, ScheduleFactory,
-    MeetingFactory, FloorPlanFactory, TimeSlotFactory, SlideSubmissionFactory )
+    MeetingFactory, FloorPlanFactory, TimeSlotFactory, SlideSubmissionFactory, RoomFactory )
 from ietf.doc.factories import DocumentFactory, WgDraftFactory
 from ietf.submit.tests import submission_file
 from ietf.utils.test_utils import assert_ical_response_is_valid
@@ -891,6 +891,86 @@ class MeetingTests(TestCase):
             self.assertFalse(q('ul li a:contains("%s")' % slide.title))
 
 
+class EditMeetingScheduleTests(TestCase):
+    """Tests of the meeting editor view
+
+    This has tests in tests_js.py as well.
+    """
+    def test_room_grouping(self):
+        """Blocks of rooms in the editor should have identical timeslots"""
+        # set up a meeting, but we'll construct our own timeslots/rooms
+        meeting = MeetingFactory(type_id='ietf', populate_schedule=False)
+        sched = ScheduleFactory(meeting=meeting)
+
+        # Make groups of rooms with timeslots identical within a group, distinct between groups
+        times = [
+            [datetime.time(11,0), datetime.time(12,0), datetime.time(13,0)],
+            [datetime.time(11,0), datetime.time(12,0), datetime.time(13,0)],  # same times, but durations will differ
+            [datetime.time(11,30), datetime.time(12, 0), datetime.time(13,0)],  # different time
+            [datetime.time(12,0)],  # different number of timeslots
+        ]
+        durations = [
+            [30, 60, 90],
+            [60, 60, 90],
+            [30, 60, 90],
+            [60],
+        ]
+        # check that times and durations are same-sized arrays
+        self.assertEqual(len(times), len(durations))
+        for time_row, duration_row in zip(times, durations):
+            self.assertEqual(len(time_row), len(duration_row))
+
+        # Create an array of room groups, each with rooms_per_group Rooms in it.
+        # Assign TimeSlots according to the times/durations above to each Room.
+        room_groups = []
+        rooms_in_group = 1  # will be incremented with each group
+        for time_row, duration_row in zip(times, durations):
+            room_groups.append(RoomFactory.create_batch(rooms_in_group, meeting=meeting))
+            rooms_in_group += 1  # put a different number of rooms in each group to help identify errors in grouping
+            for time, duration in zip(time_row, duration_row):
+                for room in room_groups[-1]:
+                    TimeSlotFactory(
+                        meeting=meeting,
+                        location=room,
+                        time=datetime.datetime.combine(meeting.date, time),
+                        duration=datetime.timedelta(minutes=duration),
+                    )
+
+        # Now retrieve the edit meeting schedule page
+        url = urlreverse('ietf.meeting.views.edit_meeting_schedule',
+                         kwargs=dict(num=meeting.number, owner=sched.owner.email(), name=sched.name))
+        r = self.client.get(url)
+        self.assertEqual(r.status_code, 200)
+
+        q = PyQuery(r.content)
+        day_divs = q('div.day')
+        # There's only one day with TimeSlots. This means there will be two divs with class 'day':
+        # the first is the room label column, the second is the TimeSlot grid.
+        # Using eq() instead of [] gives us PyQuery objects instead of Elements
+        label_divs = day_divs.eq(0).find('div.room-group')
+        self.assertEqual(len(label_divs), len(room_groups))
+        room_group_divs = day_divs.eq(1).find('div.room-group')
+        self.assertEqual(len(room_group_divs), len(room_groups))
+        for rg, l_div, rg_div in zip(
+                room_groups,
+                label_divs.items(),  # items() gives us PyQuery objects
+                room_group_divs.items(),  # items() gives us PyQuery objects
+        ):
+            # Check that room labels are correctly grouped
+            self.assertCountEqual(
+                [div.text() for div in l_div.find('div.room-name').items()],
+                [room.name for room in rg],
+            )
+
+            # And that the time labels are correct. Just check that the individual timeslot labels agree with
+            # the time-header above each room group.
+            time_header_labels = rg_div.find('div.time-header div.time-label').text()
+            timeslot_rows = rg_div.find('div.timeslots')
+            for row in timeslot_rows.items():
+                time_labels = row.find('div.time-label').text()
+                self.assertEqual(time_labels, time_header_labels)
+
+
 class ReorderSlidesTests(TestCase):
 
     def test_add_slides_to_session(self):

ietf/meeting/views.py

@@ -16,7 +16,6 @@ import tarfile
 import tempfile
 import markdown2
 
-
 from calendar import timegm
 from collections import OrderedDict, Counter, deque, defaultdict
 from urllib.parse import unquote
@@ -495,8 +494,6 @@ def edit_meeting_schedule(request, num=None, owner=None, name=None):
     for a in assignments:
         assignments_by_session[a.session_id].append(a)
 
-    rooms = meeting.room_set.filter(session_types__slug='regular').distinct().order_by("capacity")
-
     tombstone_states = ['canceled', 'canceledpa', 'resched']
 
     sessions = add_event_info_to_session_qs(
@@ -581,6 +578,145 @@ def edit_meeting_schedule(request, num=None, owner=None, name=None):
 
             s.readonly = s.current_status in tombstone_states or any(a.schedule_id != schedule.pk for a in assignments_by_session.get(s.pk, []))
 
+    def prepare_timeslots_for_display(timeslots, rooms):
+        """Prepare timeslot data for template
+
+        Prepares timeslots for display by sorting into groups in a structure
+        that can be rendered by the template and by adding some data to the timeslot
+        instances. Currently adds a 'layout_width' property to each timeslot instance.
+        The layout_width is the width, in em, that should be used to style the timeslot's
+        width.
+
+        Rooms are partitioned into groups that have identical sets of timeslots
+        for the entire meeting.
+
+        The result of this method is an OrderedDict, days, keyed by the Date
+        of each day that has at least one timeslot. The value of days[day] is a
+        list with one entry for each group of rooms. Each entry is a list of
+        dicts with keys 'room' and 'timeslots'. The 'room' value is the room
+        instance and 'timeslots' is a list of timeslot instances for that room.
+
+        The format is more easily illustrated than explained:
+
+        days = OrderedDict(
+          Date(2021, 5, 27): [
+            [  # room group 1
+              {'room': <room1>, 'timeslots': [<room1 timeslot1>, <room1 timeslot2>]},
+              {'room': <room2>, 'timeslots': [<room2 timeslot1>, <room2 timeslot2>]},
+              {'room': <room3>, 'timeslots': [<room3 timeslot1>, <room3 timeslot2>]},
+            ],
+            [  # room group 2
+              {'room': <room4>, 'timeslots': [<room4 timeslot1>]},
+            ],
+          ],
+          Date(2021, 5, 28): [
+            [ # room group 1
+              {'room': <room1>, 'timeslots': [<room1 timeslot3>]},
+              {'room': <room2>, 'timeslots': [<room2 timeslot3>]},
+              {'room': <room3>, 'timeslots': [<room3 timeslot3>]},
+            ],
+            [ # room group 2
+              {'room': <room4>, 'timeslots': []},
+            ],
+          ],
+        )
+        """
+
+        # Populate room_data. This collects the timeslots for each room binned by
+        # day, plus data needed for sorting the rooms for display.
+        room_data = dict()
+        all_days = set()
+        # timeslots_qs is already sorted by location, name, and time
+        for t in timeslots:
+            if t.location not in rooms:
+                continue
+
+            t.layout_width = timedelta_to_css_ems(t.duration)
+            if t.location_id not in room_data:
+                room_data[t.location_id] = dict(
+                    timeslots_by_day=dict(),
+                    timeslot_count=0,
+                    start_and_duration=[],
+                    first_timeslot = t,
+                )
+            rd = room_data[t.location_id]
+            rd['timeslot_count'] += 1
+            rd['start_and_duration'].append((t.time, t.duration))
+            ttd = t.time.date()
+            all_days.add(ttd)
+            if ttd not in rd['timeslots_by_day']:
+                rd['timeslots_by_day'][ttd] = []
+            rd['timeslots_by_day'][ttd].append(t)
+
+        all_days = sorted(all_days)  # changes set to a list
+        # Note the maximum timeslot count for any room
+        max_timeslots = max(rd['timeslot_count'] for rd in room_data.values())
+
+        # Partition rooms into groups with identical timeslot arrangements.
+        # Start by discarding any roos that have no timeslots.
+        rooms_with_timeslots = [r for r in rooms if r.pk in room_data]
+        # Then sort the remaining rooms.
+        sorted_rooms = sorted(
+            rooms_with_timeslots,
+            key=lambda room: (
+                # First, sort regular session rooms ahead of others - these will usually
+                # have more timeslots than other room types.
+                0 if room_data[room.pk]['timeslot_count'] == max_timeslots else 1,
+                # Sort rooms with earlier timeslots ahead of later
+                room_data[room.pk]['first_timeslot'].time,
+                # Sort rooms with more sessions ahead of rooms with fewer
+                0 - room_data[room.pk]['timeslot_count'],
+                # Sort by list of starting time and duration so that groups with identical
+                # timeslot structure will be neighbors. The grouping algorithm relies on this!
+                room_data[room.pk]['start_and_duration'],
+                # Within each group, sort higher capacity rooms first.
+                room.capacity,
+                # Finally, sort alphabetically by name
+                room.name
+            )
+        )
+
+        # Rooms are now ordered so rooms with identical timeslot arrangements are neighbors.
+        # Walk the list, splitting these into groups.
+        room_groups = []
+        last_start_and_duration = None  # Used to watch for changes in start_and_duration
+        for room in sorted_rooms:
+            if last_start_and_duration != room_data[room.pk]['start_and_duration']:
+                room_groups.append([])  # start a new room_group
+                last_start_and_duration = room_data[room.pk]['start_and_duration']
+            room_groups[-1].append(room)
+
+        # Next, build the structure that will hold the data for the view. This makes it
+        # easier to arrange that every room has an entry for every day, even if there is
+        # no timeslot for that day. This makes the HTML template much easier to write.
+        # Use OrderedDicts instead of lists so that we can easily put timeslot data in the
+        # right place.
+        days = OrderedDict(
+            (
+                day,  # key in the Ordered Dict
+                [
+                    # each value is an OrderedDict of room group data
+                    OrderedDict(
+                        (room.pk, dict(room=room, timeslots=[]))
+                        for room in rg
+                    ) for rg in room_groups
+                ]
+            ) for day in all_days
+        )
+
+        # With the structure's skeleton built, now fill in the data. The loops must
+        # preserve the order of room groups and rooms within each group.
+        for rg_num, rgroup in enumerate(room_groups):
+            for room in rgroup:
+                for day, ts_for_day in room_data[room.pk]['timeslots_by_day'].items():
+                    days[day][rg_num][room.pk]['timeslots'] = ts_for_day
+
+        # Now convert the OrderedDict entries into lists since we don't need to
+        # do lookup by pk any more.
+        for day in days.keys():
+            days[day] = [list(rg.values()) for rg in days[day]]
+
+        return days
 
     if request.method == 'POST':
         if not can_edit:
@@ -660,34 +796,11 @@ def edit_meeting_schedule(request, num=None, owner=None, name=None):
 
         return HttpResponse("Invalid parameters", status=400)
 
-    # prepare timeslot layout
+    # Show only rooms that have regular sessions
+    rooms = meeting.room_set.filter(session_types__slug='regular')
 
-    timeslots_by_room_and_day = defaultdict(list)
+    # Construct timeslot data for the template to render
-    room_has_timeslots = set()
+    days = prepare_timeslots_for_display(timeslots_qs, rooms)
-    for t in timeslots_qs:
-        room_has_timeslots.add(t.location_id)
-        timeslots_by_room_and_day[(t.location_id, t.time.date())].append(t)
-
-    days = []
-    for day in sorted(set(t.time.date() for t in timeslots_qs)):
-        room_timeslots = []
-        for r in rooms:
-            if r.pk not in room_has_timeslots:
-                continue
-
-            timeslots = []
-            for t in timeslots_by_room_and_day.get((r.pk, day), []):
-                t.layout_width = timedelta_to_css_ems(t.end_time() - t.time)
-                timeslots.append(t)
-
-            room_timeslots.append((r, timeslots))
-
-        days.append({
-            'day': day,
-            'room_timeslots': room_timeslots,
-        })
-
-    room_labels = [[r for r in rooms if r.pk in room_has_timeslots] for i in range(len(days))]
 
     # possible timeslot start/ends
     timeslot_groups = defaultdict(set)
@@ -761,7 +874,6 @@ def edit_meeting_schedule(request, num=None, owner=None, name=None):
         'can_edit_properties': can_edit or secretariat,
         'secretariat': secretariat,
         'days': days,
-        'room_labels': room_labels,
         'timeslot_groups': sorted((d, list(sorted(t_groups))) for d, t_groups in timeslot_groups.items()),
         'unassigned_sessions': unassigned_sessions,
         'session_parents': session_parents,

ietf/static/ietf/css/ietf.css

@@ -1067,6 +1067,30 @@ a.fc-event, .fc-event, .fc-content, .fc-title, .fc-event-container {
   justify-content: flex-start;
 }
 
+.edit-meeting-schedule .edit-grid .room-group:not(:last-child) {
+    margin-bottom: 1em;
+}
+
+.edit-meeting-schedule .edit-grid .time-header {
+    position: relative;
+    height: 1.5em;
+    padding-bottom: 0.15em;
+}
+
+.edit-meeting-schedule .edit-grid .time-header .time-label {
+    display: inline-block;
+    position: relative;
+    width: 100%;
+    align-items: center;
+}
+
+.edit-meeting-schedule .edit-grid .time-header .time-label span {
+    display: inline-block;
+    width: 100%;
+    text-align: center;
+    color: #444444;
+}
+
 .edit-meeting-schedule .edit-grid .timeslots {
   position: relative;
   height: 4.5em;

ietf/templates/meeting/edit_meeting_schedule.html

@@ -60,19 +60,24 @@
 
       {# using the same markup in both room labels and the actual days ensures they are aligned #}
       <div class="room-label-column">
-        {% for labels in room_labels %}
+        {% for day_data in days.values %}
           <div class="day">
             <div class="day-label">
               <strong>&nbsp;</strong><br>
               &nbsp;
             </div>
 
-            {% for room in labels %}
+            {% for rgroup in day_data %}
-              <div class="timeslots">
+              <div class="room-group">
-                <div class="room-name">
+                <div class="time-header"><div class="time-label"></div></div>
-                  <strong>{{ room.name }}</strong><br>
+                {% for room_data in rgroup %}{% with room_data.room as room %}
-                  {% if room.capacity %}{{ room.capacity }} <i class="fa fa-user-o"></i>{% endif %}
+                  <div class="timeslots">
-                </div>
+                    <div class="room-name">
+                      <strong>{{ room.name }}</strong><br>
+                      {% if room.capacity %}{{ room.capacity }} <i class="fa fa-user-o"></i>{% endif %}
+                    </div>
+                  </div>
+                {% endwith %}{% endfor %}
               </div>
             {% endfor %}
           </div>
@@ -80,29 +85,38 @@
       </div>
 
       <div class="day-flow">
-        {% for day in days %}
+        {% for day, day_data in days.items %}
           <div class="day">
             <div class="day-label">
-              <strong>{{ day.day|date:"l" }}</strong> <i class="fa fa-exchange swap-days" data-dayid="{{ day.day.isoformat }}" data-toggle="modal" data-target="#swap-days-modal"></i><br>
+              <strong>{{ day|date:"l" }}</strong> <i class="fa fa-exchange swap-days" data-dayid="{{ day.isoformat }}" data-toggle="modal" data-target="#swap-days-modal"></i><br>
-              {{ day.day|date:"N j, Y" }}
+              {{ day|date:"N j, Y" }}
             </div>
 
-            {% for room, timeslots in day.room_timeslots %}
+            {% for rgroup in day_data %}
-              <div class="timeslots" data-roomcapacity="{{ room.capacity }}">
+              <div class="room-group">
+                <div class="time-header">
+                  {# All rooms in a group have same timeslots; grab the first for the labels #}
+                  {% for t in rgroup.0.timeslots %}
+                    <div class="time-label" style="width: {{ t.layout_width }}rem"><span>{{ t.time|date:"G:i" }} - {{ t.end_time|date:"G:i" }}</span></div>
+                  {% endfor %}
+                </div>
+                {% for room_data in rgroup %}{% with room_data.room as room %}
+                  <div class="timeslots" data-roomcapacity="{{ room.capacity }}">
+                    {% for t in room_data.timeslots %}
+                      <div id="timeslot{{ t.pk }}" class="timeslot {{ t.start_end_group }}" data-start="{{ t.time.isoformat }}" data-end="{{ t.end_time.isoformat }}" data-duration="{{ t.duration.total_seconds }}" data-scheduledatlabel="{{ t.time|date:"l G:i" }}-{{ t.end_time|date:"G:i" }}" style="width: {{ t.layout_width }}rem;">
+                        <div class="time-label">
+                          {{ t.time|date:"G:i" }} - {{ t.end_time|date:"G:i" }}
+                        </div>
 
-                {% for t in timeslots %}
+                        <div class="drop-target">
-                  <div id="timeslot{{ t.pk }}" class="timeslot {{ t.start_end_group }}" data-start="{{ t.time.isoformat }}" data-end="{{ t.end_time.isoformat }}" data-duration="{{ t.duration.total_seconds }}" data-scheduledatlabel="{{ t.time|date:"l G:i" }}-{{ t.end_time|date:"G:i" }}" style="width: {{ t.layout_width }}rem;">
+                          {% for assignment, session in t.session_assignments %}
-                    <div class="time-label">
+                            {% include "meeting/edit_meeting_schedule_session.html" %}
-                      {{ t.time|date:"G:i" }} - {{ t.end_time|date:"G:i" }}
+                          {% endfor %}
-                    </div>
+                        </div>
-
+                      </div>
-                    <div class="drop-target">
+                    {% endfor %}
-                      {% for assignment, session in t.session_assignments %}
-                        {% include "meeting/edit_meeting_schedule_session.html" %}
-                      {% endfor %}
-                    </div>
                   </div>
-                {% endfor %}
+                {% endwith %}{% endfor %}
               </div>
             {% endfor %}
           </div>
@@ -192,7 +206,7 @@
           <div class="modal-body">
             {% for day in days %}
               <label>
-                <input type="radio" name="target_day" value="{{ day.day.isoformat }}"> {{ day.day|date:"l, N j, Y" }}
+                <input type="radio" name="target_day" value="{{ day.isoformat }}"> {{ day|date:"l, N j, Y" }}
               </label>
             {% endfor %}
           </div>