fix: better handle corner cases for pre-IETF-64 agenda redirects (ietf-tools#3775)

Fixes ietf-tools#3669

Author: jennifer-richards

ietf/meeting/helpers.py

@@ -55,11 +55,12 @@ def get_current_ietf_meeting():
     return meetings.first()
 
 def get_current_ietf_meeting_num():
-    return get_current_ietf_meeting().number
+    cur = get_current_ietf_meeting()
+    return cur.number if cur else None
 
 def get_ietf_meeting(num=None):
     if num:
-        meeting = Meeting.objects.filter(number=num).first()
+        meeting = Meeting.objects.filter(type='ietf', number=num).first()
     else:
         meeting = get_current_ietf_meeting()
     return meeting

ietf/meeting/tests_helpers.py

@@ -1,5 +1,7 @@
 # Copyright The IETF Trust 2020, All Rights Reserved
 # -*- coding: utf-8 -*-
+import datetime
+
 from unittest.mock import patch, Mock
 
 from django.conf import settings
@@ -11,7 +13,7 @@
 from ietf.meeting.factories import SessionFactory, MeetingFactory, TimeSlotFactory
 from ietf.meeting.helpers import (AgendaFilterOrganizer, AgendaKeywordTagger,
     delete_interim_session_conferences, sessions_post_save, sessions_post_cancel,
-    create_interim_session_conferences)
+    create_interim_session_conferences, get_ietf_meeting)
 from ietf.meeting.models import SchedTimeSessAssignment, Session
 from ietf.meeting.test_data import make_meeting_test_data
 from ietf.utils.meetecho import Conference
@@ -615,3 +617,18 @@ def test_sessions_post_save_creates_meetecho_conferences(self, mock_create_metho
         msgs = [str(msg) for msg in messages]
         self.assertEqual(len(msgs), 1)
         self.assertIn('An error occurred', msgs[0])
+
+
+class HelperTests(TestCase):
+    def test_get_ietf_meeting(self):
+        """get_ietf_meeting() should only return IETF meetings"""
+        # put the IETF far in the past so it's not "current"
+        ietf = MeetingFactory(type_id='ietf', date=datetime.date.today() - datetime.timedelta(days=5 * 365))
+        # put the interim meeting now so it will be picked up as "current" if there's a bug
+        interim = MeetingFactory(type_id='interim', date=datetime.date.today())
+        self.assertEqual(get_ietf_meeting(ietf.number), ietf, 'Return IETF meeting by number')
+        self.assertIsNone(get_ietf_meeting(interim.number), 'Ignore non-IETF meetings')
+        self.assertIsNone(get_ietf_meeting(), 'Return None if there is no current IETF meeting')
+        ietf.date = datetime.date.today()
+        ietf.save()
+        self.assertEqual(get_ietf_meeting(), ietf, 'Return current meeting if there is one')

ietf/meeting/tests_views.py

@@ -280,6 +280,54 @@ def test_meeting_agenda(self):
         self.assertContains(r, session.group.acronym)
         self.assertContains(r, slot.location.name)
 
+    @override_settings(PROCEEDINGS_V1_BASE_URL='https://example.com/{meeting.number}')
+    def test_agenda_redirects_for_old_meetings(self):
+        """Meetings before 64 should be forwarded to their proceedings"""
+        # meeting with record but no schedule
+        MeetingFactory(type_id='ietf', number='35', populate_schedule=False)
+        r = self.client.get(
+            urlreverse(
+                'ietf.meeting.views.agenda',
+                kwargs={'num': '35', 'ext': '.html'},
+            ))
+        self.assertRedirects(r, 'https://example.com/35', fetch_redirect_response=False)
+
+        # meeting with record and schedule but no assignments
+        meeting_with_schedule = MeetingFactory(type_id='ietf', number='36', populate_schedule=True)
+        r = self.client.get(
+            urlreverse(
+                'ietf.meeting.views.agenda',
+                kwargs={'num': '36', 'ext': '.html'},
+            ))
+        self.assertRedirects(r, 'https://example.com/36', fetch_redirect_response=False)
+
+        # meeting with an assignment
+        SessionFactory(meeting=meeting_with_schedule)
+        r = self.client.get(
+                    urlreverse(
+                        'ietf.meeting.views.agenda',
+                        kwargs={'num': '36', 'ext': '.html'},
+                    ))
+        self.assertRedirects(r, 'https://example.com/36', fetch_redirect_response=False)
+
+    def test_agenda_for_nonexistent_meeting(self):
+        """Return a 404 for a bad IETF meeting number"""
+        # Meetings pre-64 are redirected, but should be a 404 if there is no Meeting instance
+        r = self.client.get(
+            urlreverse(
+                'ietf.meeting.views.agenda',
+                kwargs={'num': '32', 'ext': '.html'},
+            ))
+        self.assertEqual(r.status_code, 404)
+        # Check a post-64 meeting as well
+        r = self.client.get(
+            urlreverse(
+                'ietf.meeting.views.agenda',
+                kwargs={'num': '150', 'ext': '.html'},
+            ))
+        self.assertEqual(r.status_code, 404)
+
+
     def test_meeting_agenda_filters_ignored(self):
         """The agenda view should ignore filter querystrings
         

ietf/meeting/views.py

@@ -1496,16 +1496,16 @@ def agenda(request, num=None, name=None, base=None, ext=None, owner=None, utc=""
         raise Http404('Extension not allowed')
 
     # We do not have the appropriate data in the datatracker for IETF 64 and earlier.
-    # So that we're not producing misleading pages...
-
-    assert num is None or num.isdigit()
-
+    # So that we're not producing misleading pages, redirect to their proceedings.
+    # The datatracker DB does include a Meeting instance for every IETF meeting, though,
+    # so we can use that to validate that num is a valid meeting number.
     meeting = get_ietf_meeting(num)
-    if not meeting or (meeting.number.isdigit() and int(meeting.number) <= 64 and (not meeting.schedule or not meeting.schedule.assignments.exists())):
-        if ext == '.html' or (meeting and meeting.number.isdigit() and 0 < int(meeting.number) <= 64):
-            return HttpResponseRedirect(f'{settings.PROCEEDINGS_V1_BASE_URL.format(meeting=meeting)}')
-        else:
-            raise Http404("No such meeting")
+    if meeting is None:
+        raise Http404("No such full IETF meeting")
+    elif int(meeting.number) <= 64:
+        return HttpResponseRedirect(f'{settings.PROCEEDINGS_V1_BASE_URL.format(meeting=meeting)}')
+    else:
+        pass
 
     # Select the schedule to show
     if name is None: