feat: Return to expected schedule after editing sessions/timeslots (#…

…5158)

* fix: Return to the expected schedule after editing session dteails

* style: Avoid confusing nested double-quotes in html template

* feat: When possible, add "back to agenda" button to edit_timeslots

* chore: Propagate 'sched' to links/includes in timeslot_edit.html

* chore: Propagate 'sched' to links/includes in edit_timeslot() view

* chore: Propagate 'sched' to links/includes in create_timeslot() view

* test: Test sched param propagation in timeslot edit views

* test: Test sched param propagation in session edit view

* test: Fix URL in test_edit_meeting_schedule

ietf/meeting/tests_views.py

@@ -2156,24 +2156,30 @@ def test_edit_timeslot(self):
         )
 
         self.login()
+        url = self.edit_timeslot_url(ts)
+
+        # check that sched parameter is preserved
+        r = self.client.get(url)
+        self.assertNotContains(r, '?sched=', status_code=200)
+        r = self.client.get(url + '?sched=1234')
+        self.assertContains(r, '?sched=1234', status_code=200)  # could check in more detail
+
         name_after = 'New Name (tm)'
         type_after = 'plenary'
         time_after = (time_utc + datetime.timedelta(days=1, hours=2)).astimezone(meeting.tz())
         duration_after = duration_before * 2
         show_location_after = False
         location_after = meeting.room_set.last()
-        r = self.client.post(
-            self.edit_timeslot_url(ts),
-            data=dict(
-                name=name_after,
-                type=type_after,
-                time_0=time_after.strftime('%Y-%m-%d'),  # date for SplitDateTimeField
-                time_1=time_after.strftime('%H:%M'),  # time for SplitDateTimeField
-                duration=str(duration_after),
-                # show_location=show_location_after,  # False values are omitted from form
-                location=location_after.pk,
-            )
-        )
+        post_data = dict(
+            name=name_after,
+            type=type_after,
+            time_0=time_after.strftime('%Y-%m-%d'),  # date for SplitDateTimeField
+            time_1=time_after.strftime('%H:%M'),  # time for SplitDateTimeField
+            duration=str(duration_after),
+            # show_location=show_location_after,  # False values are omitted from form
+            location=location_after.pk,
+        )
+        r = self.client.post(url, data=post_data)
         self.assertEqual(r.status_code, 302)  # expect redirect to timeslot edit url
         self.assertEqual(r['Location'], self.edit_timeslots_url(meeting),
                          'Expected to be redirected to meeting timeslots edit page')
@@ -2194,6 +2200,12 @@ def test_edit_timeslot(self):
         self.assertEqual(ts.show_location, show_location_after)
         self.assertEqual(ts.location, location_after)
 
+        # and check with sched param set
+        r = self.client.post(url + '?sched=1234', data=post_data)
+        self.assertEqual(r.status_code, 302)  # expect redirect to timeslot edit url
+        self.assertEqual(r['Location'], self.edit_timeslots_url(meeting) + '?sched=1234',
+                         'Expected to be redirected to meeting timeslots edit page with sched param set')
+
     def test_invalid_edit_timeslot(self):
         meeting = self.create_bare_meeting()
         ts: TimeSlot = TimeSlotFactory(meeting=meeting, name='slot')  # n.b., colon indicates type hinting
@@ -2316,6 +2328,7 @@ def test_create_single_timeslot(self):
         meeting = self.create_meeting()
         timeslots_before = set(ts.pk for ts in meeting.timeslot_set.all())
 
+        url = self.create_timeslots_url(meeting)
         post_data = dict(
             name='some name',
             type='regular',
@@ -2326,10 +2339,14 @@ def test_create_single_timeslot(self):
             locations=str(meeting.room_set.first().pk),
         )
         self.login()
-        r = self.client.post(
-            self.create_timeslots_url(meeting),
-            data=post_data,
-        )
+
+        # check that sched parameter is preserved
+        r = self.client.get(url)
+        self.assertNotContains(r, '?sched=', status_code=200)
+        r = self.client.get(url + '?sched=1234')
+        self.assertContains(r, '?sched=1234', status_code=200)  # could check in more detail
+
+        r = self.client.post(url, data=post_data)
         self.assertEqual(r.status_code, 302)
         self.assertEqual(r['Location'], self.edit_timeslots_url(meeting),
                          'Expected to be redirected to meeting timeslots edit page')
@@ -2344,6 +2361,12 @@ def test_create_single_timeslot(self):
         self.assertEqual(ts.show_location, post_data['show_location'])
         self.assertEqual(str(ts.location.pk), post_data['locations'])
 
+        # check again with sched parameter
+        r = self.client.post(url + '?sched=1234', data=post_data)
+        self.assertEqual(r.status_code, 302)
+        self.assertEqual(r['Location'], self.edit_timeslots_url(meeting) + '?sched=1234',
+                         'Expected to be redirected to meeting timeslots edit page with sched parameter set')
+
     def test_create_single_timeslot_outside_meeting_days(self):
         """Creating a single timeslot outside the official meeting days should work"""
         meeting = self.create_meeting()
@@ -2627,6 +2650,17 @@ def test_create_bulk_timeslots(self):
             day_locs.discard((ts.time.date(), ts.location))
         self.assertEqual(day_locs, set(), 'Not all day/location combinations created')
 
+    def test_sched_param_preserved(self):
+        meeting = MeetingFactory(type_id='ietf')
+        url = urlreverse('ietf.meeting.views.edit_timeslots', kwargs={'num': meeting.number})
+        self.client.login(username='secretary', password='secretary+password')
+        r = self.client.get(url)
+        self.assertNotContains(r, '?sched=', status_code=200)
+        self.assertNotContains(r, "Back to agenda")
+        r = self.client.get(url + '?sched=1234')
+        self.assertContains(r, '?sched=1234', status_code=200)  # could check in more detail
+        self.assertContains(r, "Back to agenda")
+
     def test_ajax_delete_timeslot(self):
         """AJAX call to delete timeslot should work"""
         meeting = self.create_bare_meeting()
@@ -3197,10 +3231,11 @@ def test_edit_meeting_schedule(self):
             e = q("#session{}".format(s.pk))
 
             # should be link to edit/cancel session
+            edit_session_url = urlreverse(
+                'ietf.meeting.views.edit_session', kwargs={'session_id': s.pk}
+            ) + f'?sched={meeting.schedule.pk}'
             self.assertTrue(
-                e.find('a[href="{}"]'.format(
-                    urlreverse('ietf.meeting.views.edit_session', kwargs={'session_id': s.pk}),
-                ))
+                e.find(f'a[href="{edit_session_url}"]')
             )
             self.assertTrue(
                 e.find('a[href="{}?sched={}"]'.format(
@@ -3768,11 +3803,15 @@ def test_new_meeting_schedule_rejects_invalid_names(self):
 
     def test_edit_session(self):
         session = SessionFactory(meeting__type_id='ietf', group__type_id='team')  # type determines allowed session purposes
+        edit_meeting_url = urlreverse('ietf.meeting.views.edit_meeting_schedule', kwargs={'num': session.meeting.number})
         self.client.login(username='secretary', password='secretary+password')
         url = urlreverse('ietf.meeting.views.edit_session', kwargs={'session_id': session.pk})
         r = self.client.get(url)
         self.assertContains(r, 'Edit session', status_code=200)
-        r = self.client.post(url, {
+        pq = PyQuery(r.content)
+        back_button = pq(f'a[href="{edit_meeting_url}"]')
+        self.assertEqual(len(back_button), 1)
+        post_data = {
             'name': 'this is a name',
             'short': 'tian',
             'purpose': 'coding',
@@ -3782,10 +3821,10 @@ def test_edit_session(self):
             'remote_instructions': 'Do this do that',
             'attendees': '103',
             'comments': 'So much to say',
-        })
+        }
+        r = self.client.post(url, post_data)
         self.assertNoFormPostErrors(r)
-        self.assertRedirects(r, urlreverse('ietf.meeting.views.edit_meeting_schedule',
-                                           kwargs={'num': session.meeting.number}))
+        self.assertRedirects(r, edit_meeting_url)
         session = Session.objects.get(pk=session.pk)  # refresh objects from DB
         self.assertEqual(session.name, 'this is a name')
         self.assertEqual(session.short, 'tian')
@@ -3797,6 +3836,23 @@ def test_edit_session(self):
         self.assertEqual(session.attendees, 103)
         self.assertEqual(session.comments, 'So much to say')
 
+        # Verify return to correct schedule when sched query parameter is present
+        other_schedule = ScheduleFactory(meeting=session.meeting)
+        r = self.client.get(url + f'?sched={other_schedule.pk}')
+        edit_meeting_url = urlreverse(
+            'ietf.meeting.views.edit_meeting_schedule',
+            kwargs={
+                'num': session.meeting.number,
+                'owner': other_schedule.owner.email(),
+                'name': other_schedule.name,
+            },
+        )
+        pq = PyQuery(r.content)
+        back_button = pq(f'a[href="{edit_meeting_url}"]')
+        self.assertEqual(len(back_button), 1)
+        r = self.client.post(url + f'?sched={other_schedule.pk}', post_data)
+        self.assertRedirects(r, edit_meeting_url)
+
     def test_cancel_session(self):
         # session for testing with official schedule
         session = SessionFactory(meeting__type_id='ietf')

ietf/meeting/views.py

@@ -17,7 +17,7 @@
 
 from calendar import timegm
 from collections import OrderedDict, Counter, deque, defaultdict, namedtuple
-from urllib.parse import unquote
+from urllib.parse import parse_qs, unquote, urlencode, urlsplit, urlunsplit
 from tempfile import mkstemp
 from wsgiref.handlers import format_date_time
 
@@ -281,8 +281,13 @@ def materials_editable_groups(request, num=None):
 
 @role_required('Secretariat')
 def edit_timeslots(request, num=None):
-
     meeting = get_meeting(num)
+    if 'sched' in request.GET:
+        schedule = Schedule.objects.filter(pk=request.GET.get('sched', None)).first()
+        schedule_edit_url = _schedule_edit_url(meeting, schedule)
+    else:
+        schedule_edit_url = None
+
     with timezone.override(meeting.tz()):
         if request.method == 'POST':
             # handle AJAX requests
@@ -333,6 +338,7 @@ def edit_timeslots(request, num=None):
                                               "slot_slices": slots,
                                               "date_slices":date_slices,
                                               "meeting":meeting,
+                                              "schedule_edit_url": schedule_edit_url,
                                               "ts_list":ts_list,
                                               "ts_with_official_assignments": ts_with_official_assignments,
                                               "ts_with_any_assignments": ts_with_any_assignments,
@@ -4123,7 +4129,15 @@ def edit_timeslot(request, num, slot_id):
             form = TimeSlotEditForm(instance=timeslot, data=request.POST)
             if form.is_valid():
                 form.save()
-                return HttpResponseRedirect(reverse('ietf.meeting.views.edit_timeslots', kwargs={'num': num}))
+                redirect_to = reverse('ietf.meeting.views.edit_timeslots', kwargs={'num': num})
+                if 'sched' in request.GET:
+                    # Preserve 'sched' as a query parameter
+                    urlparts = list(urlsplit(redirect_to))
+                    query = parse_qs(urlparts[3])
+                    query['sched'] = request.GET['sched']
+                    urlparts[3] = urlencode(query)
+                    redirect_to = urlunsplit(urlparts)
+                return HttpResponseRedirect(redirect_to)
         else:
             form = TimeSlotEditForm(instance=timeslot)
 
@@ -4156,7 +4170,15 @@ def create_timeslot(request, num):
                     show_location=form.cleaned_data['show_location'],
                 )
             )
-            return HttpResponseRedirect(reverse('ietf.meeting.views.edit_timeslots',kwargs={'num':num}))
+            redirect_to = reverse('ietf.meeting.views.edit_timeslots',kwargs={'num':num})
+            if 'sched' in request.GET:
+                # Preserve 'sched' as a query parameter
+                urlparts = list(urlsplit(redirect_to))
+                query = parse_qs(urlparts[3])
+                query['sched'] = request.GET['sched']
+                urlparts[3] = urlencode(query)
+                redirect_to = urlunsplit(urlparts)
+            return HttpResponseRedirect(redirect_to)
     else:
         form = TimeSlotCreateForm(meeting)
 
@@ -4171,19 +4193,19 @@ def create_timeslot(request, num):
 @role_required('Secretariat')
 def edit_session(request, session_id):
     session = get_object_or_404(Session, pk=session_id)
+    schedule = Schedule.objects.filter(pk=request.GET.get('sched', None)).first()
+    editor_url = _schedule_edit_url(session.meeting, schedule)
     if request.method == 'POST':
         form = SessionEditForm(instance=session, data=request.POST)
         if form.is_valid():
             form.save()
-            return HttpResponseRedirect(
-                reverse('ietf.meeting.views.edit_meeting_schedule',
-                        kwargs={'num': form.instance.meeting.number}))
+            return HttpResponseRedirect(editor_url)
     else:
         form = SessionEditForm(instance=session)
     return render(
         request,
         'meeting/edit_session.html',
-        {'session': session, 'form': form},
+        {'session': session, 'form': form, 'editor_url': editor_url},
     )
 
 def _schedule_edit_url(meeting, schedule):

ietf/templates/meeting/create_timeslot.html

@@ -12,7 +12,7 @@ <h1>Create timeslot for {{ meeting }}</h1>
         {% bootstrap_form form %}
         <button type="submit" class="btn btn-primary">Save</button>
         <a class="btn btn-secondary float-end"
-           href="{% url 'ietf.meeting.views.edit_timeslots' num=meeting.number %}">Back</a>
+           href="{% url 'ietf.meeting.views.edit_timeslots' num=meeting.number %}{% if 'sched' in request.GET %}?sched={{ request.GET.sched }}{% endif %}">Back</a>
     </form>
 {% endblock %}
 {% block js %}

ietf/templates/meeting/edit_meeting_schedule.html

@@ -40,22 +40,22 @@ <h1>
             <div class="my-3">
                 {% if can_edit_properties %}
                     <a class="btn btn-primary"
-                       href="{% url "ietf.meeting.views.edit_schedule_properties" schedule.meeting.number schedule.owner_email schedule.name %}">
+                       href="{% url 'ietf.meeting.views.edit_schedule_properties' schedule.meeting.number schedule.owner_email schedule.name %}">
                         Edit properties
                     </a>
                 {% endif %}
                 {% if user|has_role:"Secretariat" %}
                     <a class="btn btn-primary"
-                       href="{% url "ietf.meeting.views.edit_timeslots" num=meeting.number %}">
+                       href="{% url 'ietf.meeting.views.edit_timeslots' num=meeting.number %}?sched={{ schedule.pk }}">
                         Edit timeslots
                     </a>
                 {% endif %}
                 <a class="btn btn-primary"
-                   href="{% url "ietf.meeting.views.new_meeting_schedule" num=meeting.number owner=schedule.owner_email name=schedule.name %}">
+                   href="{% url 'ietf.meeting.views.new_meeting_schedule' num=meeting.number owner=schedule.owner_email name=schedule.name %}">
                     Copy agenda
                 </a>
                 <a class="btn btn-primary"
-                   href="{% url "ietf.meeting.views.list_schedules" num=meeting.number %}">
+                   href="{% url 'ietf.meeting.views.list_schedules' num=meeting.number %}">
                   Other agendas
                 </a>
             </div>
@@ -64,7 +64,7 @@ <h1>
                     You can't edit this schedule.
                     {% if schedule.is_official_record %}This is the official schedule for a meeting in the past.{% endif %}
                     Make a
-                    <a href="{% url "ietf.meeting.views.new_meeting_schedule" num=meeting.number owner=schedule.owner_email name=schedule.name %}">
+                    <a href="{% url 'ietf.meeting.views.new_meeting_schedule' num=meeting.number owner=schedule.owner_email name=schedule.name %}">
                         new agenda from this</a>.
                 </div>
             {% endif %}
@@ -74,7 +74,8 @@ <h1>
                   No timeslots exist for this meeting yet.
                 </p>
                 <p>
-                  <a class="btn btn-primary" href="{% url "ietf.meeting.views.edit_timeslots" num=meeting.number %}">
+                  <a class="btn btn-primary"
+                     href="{% url 'ietf.meeting.views.edit_timeslots' num=meeting.number %}?sched={{ schedule.pk }}">
                     Edit timeslots
                   </a>
                 </p>

ietf/templates/meeting/edit_meeting_schedule_session.html

@@ -104,7 +104,7 @@
         {% endfor %}
         {% if secretariat %}
             <a class="btn btn-primary btn-sm mt-2"
-               href="{% url 'ietf.meeting.views.edit_session' session_id=session.pk %}">
+               href="{% url 'ietf.meeting.views.edit_session' session_id=session.pk %}?sched={{ schedule.pk }}">
                 Edit session
             </a>
             <a class="btn btn-danger btn-sm mt-2"

ietf/templates/meeting/edit_session.html

@@ -15,10 +15,7 @@ <h1>
         {% csrf_token %}
         {% bootstrap_form form %}
         <button type="submit" class="btn btn-primary">Save</button>
-        <a class="btn btn-secondary float-end"
-           href="{% url 'ietf.meeting.views.edit_meeting_schedule' num=session.meeting.number %}">
-            Back
-        </a>
+        <a class="btn btn-secondary float-end" href="{{ editor_url }}">Back</a>
     </form>
 {% endblock %}
 {% block js %}{{ form.media.js }}{% endblock %}

ietf/templates/meeting/edit_timeslot.html

@@ -25,7 +25,7 @@ <h1>
         {% bootstrap_form form %}
         <button type="submit" class="btn btn-primary">Save</button>
         <a class="btn btn-secondary float-end"
-           href="{% url 'ietf.meeting.views.edit_timeslots' num=timeslot.meeting.number %}">
+           href="{% url 'ietf.meeting.views.edit_timeslots' num=timeslot.meeting.number %}{% if "sched" in request.GET %}?sched={{ request.GET.sched }}{% endif %}">
             Back
         </a>
     </form>