Add Schedule.origin to enable tracking where a schedule was copied

from. Add utility function and view for diff'ing two arbitrary
schedules, and modify the schedule list page to show the number of
differences between the origin of a schedule, linking to the diff, to
make it easy to see what's changed between work-in-progress schedules.
 - Legacy-Id: 18337

Author: OleLaursen

ietf/meeting/migrations/0031_add_session_origin.py

@@ -0,0 +1,20 @@
+# Generated by Django 2.0.13 on 2020-08-04 06:22
+
+from django.db import migrations
+import django.db.models.deletion
+import ietf.utils.models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('meeting', '0030_schedule_notes'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='schedule',
+            name='origin',
+            field=ietf.utils.models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, to='meeting.Schedule'),
+        ),
+    ]

ietf/meeting/models.py

@@ -626,7 +626,7 @@ class Schedule(models.Model):
     public   = models.BooleanField(default=True, help_text="Allow others to see this agenda.")
     badness  = models.IntegerField(null=True, blank=True)
     notes    = models.TextField(blank=True)
-    # considering copiedFrom = ForeignKey('Schedule', blank=True, null=True)
+    origin   = ForeignKey('Schedule', blank=True, null=True, on_delete=models.SET_NULL)
 
     def __str__(self):
         return u"%s:%s(%s)" % (self.meeting, self.name, self.owner)

ietf/meeting/tests_views.py

@@ -1359,6 +1359,77 @@ def test_list_schedules(self):
         r = self.client.get(url)
         self.assertTrue(r.status_code, 200)
 
+    def test_diff_schedules(self):
+        meeting = make_meeting_test_data()
+
+        url = urlreverse('ietf.meeting.views.diff_schedules',kwargs={'num':meeting.number})
+        login_testing_unauthorized(self,"secretary", url)
+        r = self.client.get(url)
+        self.assertTrue(r.status_code, 200)
+
+        from_schedule = Schedule.objects.get(meeting=meeting, name="test-unofficial-schedule")
+
+        session1 = Session.objects.filter(meeting=meeting, group__acronym='mars').first()
+        session2 = Session.objects.filter(meeting=meeting, group__acronym='ames').first()
+        session3 = Session.objects.create(meeting=meeting, group=Group.objects.get(acronym='mars'),
+                               attendees=10, requested_duration=datetime.timedelta(minutes=70),
+                               type_id='regular')
+        SchedulingEvent.objects.create(session=session3, status_id='schedw', by=Person.objects.first())
+
+        slot2 = TimeSlot.objects.filter(meeting=meeting, type='regular').order_by('-time').first()
+        slot3 = TimeSlot.objects.create(
+            meeting=meeting, type_id='regular', location=slot2.location,
+            duration=datetime.timedelta(minutes=60),
+            time=slot2.time + datetime.timedelta(minutes=60),
+        )
+
+        # copy
+        copy_url = urlreverse("ietf.meeting.views.copy_meeting_schedule", kwargs=dict(num=meeting.number, owner=from_schedule.owner_email(), name=from_schedule.name))
+        r = self.client.post(copy_url, {
+            'name': "newtest",
+            'public': "on",
+        })
+        self.assertNoFormPostErrors(r)
+
+        to_schedule = Schedule.objects.get(meeting=meeting, name='newtest')
+
+        # make some changes
+
+        edit_url = urlreverse("ietf.meeting.views.edit_meeting_schedule", kwargs=dict(num=meeting.number, owner=to_schedule.owner_email(), name=to_schedule.name))
+
+        # schedule
+        r = self.client.post(edit_url, {
+            'action': 'assign',
+            'timeslot': slot3.pk,
+            'session': session3.pk,
+        })
+        self.assertEqual(json.loads(r.content)['success'], True)
+
+        # unschedule
+        r = self.client.post(edit_url, {
+            'action': 'unassign',
+            'session': session1.pk,
+        })
+        self.assertEqual(json.loads(r.content)['success'], True)
+
+        # move
+        r = self.client.post(edit_url, {
+            'action': 'assign',
+            'timeslot': slot2.pk,
+            'session': session2.pk,
+        })
+        self.assertEqual(json.loads(r.content)['success'], True)
+
+        # get differences
+        r = self.client.get(url, {
+            'from_schedule': from_schedule.name,
+            'to_schedule': to_schedule.name,
+        })
+        self.assertTrue(r.status_code, 200)
+
+        q = PyQuery(r.content)
+        self.assertEqual(len(q(".schedule-diffs tr")), 3)
+
     def test_delete_schedule(self):
         url = urlreverse('ietf.meeting.views.delete_schedule',
                          kwargs={'num':self.mtg.number,

ietf/meeting/urls.py

@@ -48,6 +48,7 @@
     url(r'^agenda/by-type/(?P<type>[a-z]+)/ics$', views.agenda_by_type_ics),
     url(r'^agendas/list$', views.list_schedules),
     url(r'^agendas/edit$', RedirectView.as_view(pattern_name='ietf.meeting.views.list_schedules', permanent=True)),
+    url(r'^agendas/diff/$', views.diff_schedules),
     url(r'^timeslots/edit$',                     views.edit_timeslots),
     url(r'^timeslot/(?P<slot_id>\d+)/edittype$', views.edit_timeslot_type),
     url(r'^rooms$',                              ajax.timeslot_roomsurl),

ietf/meeting/utils.py

@@ -436,3 +436,80 @@ def format_constraint(c):
             formatted_constraints_for_sessions[s.pk][joint_with_groups_constraint_name] = [g.acronym for g in joint_groups]
 
     return constraints_for_sessions, formatted_constraints_for_sessions, constraint_names
+
+
+def diff_meeting_schedules(from_schedule, to_schedule):
+    """Compute the difference between the two meeting schedules as a list
+    describing the set of actions that will turn the schedule of from into
+    the schedule of to, like:
+
+    [
+      {'change': 'schedule', 'session': session_id, 'to': timeslot_id},
+      {'change': 'move', 'session': session_id, 'from': timeslot_id, 'to': timeslot_id2},
+      {'change': 'unschedule', 'session': session_id, 'from': timeslot_id},
+    ]
+
+    Uses .assignments.all() so that it can be prefetched.
+    """
+    diffs = []
+
+    from_session_timeslots = {
+        a.session_id: a.timeslot_id
+        for a in from_schedule.assignments.all()
+    }
+
+    session_ids_in_to = set()
+
+    for a in to_schedule.assignments.all():
+        session_ids_in_to.add(a.session_id)
+
+        from_timeslot_id = from_session_timeslots.get(a.session_id)
+
+        if from_timeslot_id is None:
+            diffs.append({'change': 'schedule', 'session': a.session_id, 'to': a.timeslot_id})
+        elif a.timeslot_id != from_timeslot_id:
+            diffs.append({'change': 'move', 'session': a.session_id, 'from': from_timeslot_id, 'to': a.timeslot_id})
+
+    for from_session_id, from_timeslot_id in from_session_timeslots.items():
+        if from_session_id not in session_ids_in_to:
+            diffs.append({'change': 'unschedule', 'session': from_session_id, 'from': from_timeslot_id})
+
+    return diffs
+
+
+def prefetch_schedule_diff_objects(diffs):
+    session_ids = set()
+    timeslot_ids = set()
+
+    for d in diffs:
+        session_ids.add(d['session'])
+
+        if d['change'] == 'schedule':
+            timeslot_ids.add(d['to'])
+        elif d['change'] == 'move':
+            timeslot_ids.add(d['from'])
+            timeslot_ids.add(d['to'])
+        elif d['change'] == 'unschedule':
+            timeslot_ids.add(d['from'])
+
+    session_lookup = {s.pk: s for s in Session.objects.filter(pk__in=session_ids)}
+    timeslot_lookup = {t.pk: t for t in TimeSlot.objects.filter(pk__in=timeslot_ids).prefetch_related('location')}
+
+    res = []
+    for d in diffs:
+        d_objs = {
+            'change': d['change'],
+            'session': session_lookup.get(d['session'])
+        }
+
+        if d['change'] == 'schedule':
+            d_objs['to'] = timeslot_lookup.get(d['to'])
+        elif d['change'] == 'move':
+            d_objs['from'] = timeslot_lookup.get(d['from'])
+            d_objs['to'] = timeslot_lookup.get(d['to'])
+        elif d['change'] == 'unschedule':
+            d_objs['from'] = timeslot_lookup.get(d['from'])
+
+        res.append(d_objs)
+
+    return res

ietf/meeting/views.py

@@ -77,6 +77,7 @@
 from ietf.meeting.utils import current_session_status
 from ietf.meeting.utils import data_for_meetings_overview
 from ietf.meeting.utils import preprocess_constraints_for_meeting_schedule_editor
+from ietf.meeting.utils import diff_meeting_schedules, prefetch_schedule_diff_objects
 from ietf.message.utils import infer_message
 from ietf.secr.proceedings.utils import handle_upload_file
 from ietf.secr.proceedings.proc_utils import (get_progress_stats, post_process, import_audio_files,
@@ -825,7 +826,9 @@ def natural_sort_key(s): # from https://stackoverflow.com/questions/4836710/is-t
 def list_schedules(request, num):
     meeting = get_meeting(num)
 
-    schedules = Schedule.objects.filter(meeting=meeting).prefetch_related('owner').order_by('owner', '-name', '-public').distinct()
+    schedules = Schedule.objects.filter(
+        meeting=meeting
+    ).prefetch_related('owner', 'assignments', 'origin', 'origin__assignments').order_by('owner', '-name', '-public').distinct()
     if not has_role(request.user, 'Secretariat'):
         schedules = schedules.filter(Q(visible=True) | Q(owner=request.user.person))
 
@@ -839,6 +842,9 @@ def list_schedules(request, num):
     for s in schedules:
         s.can_edit_properties = is_secretariat or user_is_person(request.user, s.owner)
 
+        if s.origin:
+            s.changes_from_origin = len(diff_meeting_schedules(s.origin, s))
+
         if s.pk == meeting.schedule_id:
             official_schedules.append(s)
         elif user_is_person(request.user, s.owner):
@@ -862,6 +868,58 @@ def list_schedules(request, num):
         'schedule_groups': schedule_groups,
     })
 
+class DiffSchedulesForm(forms.Form):
+    from_schedule = forms.ChoiceField()
+    to_schedule = forms.ChoiceField()
+
+    def __init__(self, meeting, user, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+
+        qs = Schedule.objects.filter(meeting=meeting).prefetch_related('owner').order_by('-public').distinct()
+
+        if not has_role(user, 'Secretariat'):
+            qs = qs.filter(Q(visible=True) | Q(owner__user=user))
+
+        sorted_schedules = sorted(qs, reverse=True, key=lambda s: natural_sort_key(s.name))
+
+        schedule_choices = [(schedule.name, "{} ({})".format(schedule.name, schedule.owner)) for schedule in sorted_schedules]
+
+        self.fields['from_schedule'].choices = schedule_choices
+        self.fields['to_schedule'].choices = schedule_choices
+
+@role_required('Area Director','Secretariat')
+def diff_schedules(request, num):
+    meeting = get_meeting(num)
+
+    diffs = None
+    from_schedule = None
+    to_schedule = None
+
+    if 'from_schedule' in request.GET:
+        form = DiffSchedulesForm(meeting, request.user, request.GET)
+        if form.is_valid():
+            from_schedule = get_object_or_404(Schedule, name=form.cleaned_data['from_schedule'], meeting=meeting)
+            to_schedule = get_object_or_404(Schedule, name=form.cleaned_data['to_schedule'], meeting=meeting)
+            raw_diffs = diff_meeting_schedules(from_schedule, to_schedule)
+
+            diffs = prefetch_schedule_diff_objects(raw_diffs)
+            for d in diffs:
+                s = d['session']
+                s.session_label = s.short_name
+                if s.requested_duration:
+                    s.session_label = "{} ({}h)".format(s.session_label, round(s.requested_duration.seconds / 60.0 / 60.0, 1))
+    else:
+        form = DiffSchedulesForm(meeting, request.user)
+
+    return render(request, "meeting/diff_schedules.html", {
+        'meeting': meeting,
+        'form': form,
+        'diffs': diffs,
+        'from_schedule': from_schedule,
+        'to_schedule': to_schedule,
+    })
+
+
 @ensure_csrf_cookie
 def agenda(request, num=None, name=None, base=None, ext=None, owner=None, utc=""):
     base = base if base else 'agenda'
@@ -2282,6 +2340,10 @@ def delete_schedule(request, num, owner, name):
         return HttpResponseForbidden("You may not delete other user's schedules")
 
     if request.method == 'POST':
+        # remove schedule from origin tree
+        replacement_origin = schedule.origin
+        Schedule.objects.filter(origin=schedule).update(origin=replacement_origin)
+
         schedule.delete()
         return HttpResponseRedirect(reverse('ietf.meeting.views.list_schedules',kwargs={'num':num}))
 

ietf/templates/meeting/diff_schedules.html

@@ -0,0 +1,40 @@
+{% extends "base.html" %}
+{# Copyright The IETF Trust 2020, All Rights Reserved #}
+{% load origin %}
+{% load ietf_filters %}
+{% load bootstrap3 %}
+
+{% block content %}
+  {% origin %}
+  <h1>{% block title %}Differences between Meeting Agendas for IETF {{ meeting.number }}{% endblock %}</h1>
+
+  <form method="get">
+    {% bootstrap_form form %}
+    <button type="submit">Show differences</button>
+  </form>
+
+  {% if diffs != None %}
+    <h2>Differences from {{ from_schedule.name }} ({{ from_schedule.owner }}) to {{ to_schedule.name }} ({{ to_schedule.owner }}) </h2>
+
+    {% if diffs %}
+      <table class="table table-condensed schedule-diffs">
+        {% for d in diffs %}
+          <tr>
+            <td>
+              {% if d.change == 'schedule' %}
+                Scheduled <b>{{ d.session.session_label }}</b> to <b>{{ d.to.time|date:"l G:i" }} at {{ d.to.location.name }}</b>
+              {% elif d.change == 'move' %}
+                Moved <b>{{ d.session.session_label }}</b> from {{ d.from.time|date:"l G:i" }} at {{ d.from.location.name }} to <b>{{ d.to.time|date:"l G:i" }} at {{ d.to.location.name }}</b>
+              {% elif d.change == 'unschedule' %}
+                Unscheduled <b>{{ d.session.session_label }}</b> from {{ d.from.time|date:"l G:i" }} at {{ d.from.location.name }}
+              {% endif %}
+            </td>
+          </tr>
+        {% endfor %}
+      </table>
+    {% else %}
+      No differences in scheduled sessions found!
+    {% endif %}
+  {% endif %}
+
+{% endblock content %}

ietf/templates/meeting/schedule_list.html

@@ -22,21 +22,28 @@ <h1>{% block title %}Possible Meeting Agendas for IETF {{ meeting.number }}{% en
             <tr>
               <th class="col-md-2">Name</th>
               <th class="col-md-2">Owner</th>
-              <th class="col-md-5">Notes</th>
+              <th class="col-md-1">Origin</th>
+              <th class="col-md-4">Notes</th>
               <th class="col-md-1">Visible</th>
               <th class="col-md-1">Public</th>
             </tr>
             {% for schedule in schedules %}
-            <tr>
-              <td>
-                <a href="{% url "ietf.meeting.views.edit_schedule" schedule.meeting.number schedule.owner_email schedule.name %}">{{ schedule.name }}</a>
-                {% if schedule.can_edit_properties %}
-                  <a class="edit-schedule-properties" href="{% url "ietf.meeting.views.edit_schedule_properties" schedule.meeting.number schedule.owner_email schedule.name %}?next={{ request.get_full_path|urlencode }}">
-                    <i title="Edit agenda properties" class="fa fa-edit"></i>
-                  </a>
+              <tr>
+                <td>
+                  <a href="{% url "ietf.meeting.views.edit_schedule" meeting.number schedule.owner_email schedule.name %}">{{ schedule.name }}</a>
+                  {% if schedule.can_edit_properties %}
+                    <a class="edit-schedule-properties" href="{% url "ietf.meeting.views.edit_schedule_properties" meeting.number schedule.owner_email schedule.name %}?next={{ request.get_full_path|urlencode }}">
+                      <i title="Edit agenda properties" class="fa fa-edit"></i>
+                    </a>
+                  {% endif %}
+                </td>
+                <td>{{ schedule.owner }}</td>
+                <td>
+                  {% if schedule.origin %}
+                    <a href="{% url "ietf.meeting.views.edit_schedule" meeting.number schedule.origin.owner_email schedule.name %}">{{ schedule.origin.name }}</a>
+                    <a href="{% url "ietf.meeting.views.diff_schedules" meeting.number %}?from_schedule={{ schedule.origin.name|urlencode }}&to_schedule={{ schedule.name|urlencode }}" title="{{ schedule.changes_from_origin }} change{{ schedule.changes_from_origin|pluralize }} from {{ schedule.origin.name }}">+{{ schedule.changes_from_origin }}</a>
                 {% endif %}
               </td>
-              <td>{{ schedule.owner }}</td>
               <td>{{ schedule.notes|linebreaksbr }}</td>
               <td>
                 {% if schedule.visible %}