From 40c33b1281e8e405215cd1bf59b76c925ba24137 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Thu, 31 Mar 2022 17:16:21 -0300 Subject: [PATCH] fix: better handle corner cases for pre-IETF-64 agenda redirects (#3775) Fixes #3669 --- ietf/meeting/helpers.py | 5 ++-- ietf/meeting/tests_helpers.py | 19 +++++++++++++- ietf/meeting/tests_views.py | 48 +++++++++++++++++++++++++++++++++++ ietf/meeting/views.py | 18 ++++++------- 4 files changed, 78 insertions(+), 12 deletions(-) diff --git a/ietf/meeting/helpers.py b/ietf/meeting/helpers.py index 12bdabac5..4fd4579c9 100644 --- a/ietf/meeting/helpers.py +++ b/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 diff --git a/ietf/meeting/tests_helpers.py b/ietf/meeting/tests_helpers.py index a8158e462..3160f9388 100644 --- a/ietf/meeting/tests_helpers.py +++ b/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.group.models import Group 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 @@ class InterimTests(TestCase): 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') diff --git a/ietf/meeting/tests_views.py b/ietf/meeting/tests_views.py index 7eeb67145..4f7654d28 100644 --- a/ietf/meeting/tests_views.py +++ b/ietf/meeting/tests_views.py @@ -280,6 +280,54 @@ class MeetingTests(BaseMeetingTestCase): 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 diff --git a/ietf/meeting/views.py b/ietf/meeting/views.py index d1a15f4c0..8484fbc5c 100644 --- a/ietf/meeting/views.py +++ b/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: