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
2023-02-16 11:57:18 -04:00 · 2023-02-16 11:57:18 -04:00 · f9c2376d27
parent 8734193974
commit f9c2376d27
9 changed files with 129 additions and 48 deletions
--- a/ietf/meeting/tests_views.py
+++ b/ietf/meeting/tests_views.py
@ -2156,24 +2156,30 @@ class EditTimeslotsTests(TestCase):
        )

        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 @@ class EditTimeslotsTests(TestCase):
        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 @@ class EditTimeslotsTests(TestCase):
        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 @@ class EditTimeslotsTests(TestCase):
            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 @@ class EditTimeslotsTests(TestCase):
        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 @@ class EditTimeslotsTests(TestCase):
            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 @@ class EditTests(TestCase):
            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 @@ class EditTests(TestCase):

    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 @@ class EditTests(TestCase):
            '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 @@ class EditTests(TestCase):
        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')
--- a/ietf/meeting/views.py
+++ b/ietf/meeting/views.py
@ -17,7 +17,7 @@ import tempfile

 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):
--- a/ietf/templates/meeting/create_timeslot.html
+++ b/ietf/templates/meeting/create_timeslot.html
@ -12,7 +12,7 @@
        {% 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 %}
--- a/ietf/templates/meeting/edit_meeting_schedule.html
+++ b/ietf/templates/meeting/edit_meeting_schedule.html
@ -40,22 +40,22 @@
            <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 @@
                    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 @@
                  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>
--- a/ietf/templates/meeting/edit_meeting_schedule_session.html
+++ b/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"
--- a/ietf/templates/meeting/edit_session.html
+++ b/ietf/templates/meeting/edit_session.html
@ -15,10 +15,7 @@
        {% 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 %}
--- a/ietf/templates/meeting/edit_timeslot.html
+++ b/ietf/templates/meeting/edit_timeslot.html
@ -25,7 +25,7 @@
        {% 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>
--- a/ietf/templates/meeting/timeslot_edit.html
+++ b/ietf/templates/meeting/timeslot_edit.html
@ -35,7 +35,7 @@ a.new-timeslot-link { color: lightgray; font-size: large;}
    </h1>
    <div class="my-3">
        <a class="btn btn-primary"
-           href="{% url "ietf.meeting.views.create_timeslot" num=meeting.number %}">
+           href="{% url "ietf.meeting.views.create_timeslot" num=meeting.number %}{% if "sched" in request.GET %}?sched={{ request.GET.sched }}{% endif %}">
            New timeslot
        </a>
        {% if meeting.schedule %}
@ -51,6 +51,9 @@ a.new-timeslot-link { color: lightgray; font-size: large;}
           href="{% url "ietf.meeting.views.list_schedules" num=meeting.number %}">
            Agenda list
        </a>
+        {% if schedule_edit_url %}
+            <a class="btn btn-secondary" href="{{ schedule_edit_url }}">Back to agenda</a>
+        {% endif %}
    </div>
   <div class="timeslot-edit">
        {% if rooms|length == 0 %}
@ -126,7 +129,9 @@ a.new-timeslot-link { color: lightgray; font-size: large;}
                                        <p>
                                            No timeslots exist for this meeting yet.
                                        </p>
-                                        <a href="{% url "ietf.meeting.views.create_timeslot" num=meeting.number %}">Create a timeslot.</a>
+                                        <a href="{% url "ietf.meeting.views.create_timeslot" num=meeting.number %}{% if "sched" in request.GET %}?sched={{ request.GET.sched }}{% endif %}">
+                                            Create a timeslot.
+                                        </a>
                                    </td>
                                {% else %}
                                    {% for day in time_slices %}
@ -137,11 +142,11 @@ a.new-timeslot-link { color: lightgray; font-size: large;}
                                                    <td class="tscell {% if cell_ts|length > 1 %}timeslot-collision{% endif %}{% for ts in cell_ts %} tstype_{{ ts.type.slug }}{% endfor %}">
                                                        {% if cell_ts %}
                                                            {% for ts in cell_ts %}
-                                                                {% include 'meeting/timeslot_edit_timeslot.html' with ts=ts in_use=ts_with_any_assignments in_official_use=ts_with_official_assignments only %}
+                                                                {% include 'meeting/timeslot_edit_timeslot.html' with ts=ts in_use=ts_with_any_assignments in_official_use=ts_with_official_assignments request=request only %}
                                                            {% endfor %}
                                                        {% endif %}
                                                        <a class="new-timeslot-link text-reset text-decoration-none {% if cell_ts %}hidden{% endif %}" aria-label="+"
-                                                           href="{% url 'ietf.meeting.views.create_timeslot' num=meeting.number %}?day={{ day.toordinal }}&amp;date={{ day|date:'Y-m-d' }}&amp;location={{ room.pk }}&amp;time={{ slot.time|date:'H:i' }}&amp;duration={{ slot.duration }}">
+                                                           href="{% url 'ietf.meeting.views.create_timeslot' num=meeting.number %}?day={{ day.toordinal }}&amp;date={{ day|date:'Y-m-d' }}&amp;location={{ room.pk }}&amp;time={{ slot.time|date:'H:i' }}&amp;duration={{ slot.duration }}{% if "sched" in request.GET %}&amp;sched={{ request.GET.sched }}{% endif %}">
                                                            <i class="bi bi-plus-square"></i>
                                                        </a>
                                                    </td>
--- a/ietf/templates/meeting/timeslot_edit_timeslot.html
+++ b/ietf/templates/meeting/timeslot_edit_timeslot.html
@ -16,7 +16,7 @@ data-official-use="{% if ts in in_official_use %}true{% else %}false{% endif %}"
 </span>
 </div>
 <div class="timeslot-buttons">
-<a class="text-reset text-decoration-none" aria-label="Edit" href="{% url 'ietf.meeting.views.edit_timeslot' num=ts.meeting.number slot_id=ts.id %}">
+<a class="text-reset text-decoration-none" aria-label="Edit" href="{% url 'ietf.meeting.views.edit_timeslot' num=ts.meeting.number slot_id=ts.id %}{% if "sched" in request.GET %}?sched={{ request.GET.sched }}{% endif %}">
 <i class="bi bi-pencil"></i>
 </a>
 <i class="bi bi-trash delete-button" data-delete-scope="timeslot" title="Delete this timeslot"></i>