diff --git a/ietf/meeting/test_data.py b/ietf/meeting/test_data.py index 37ad1b557..bef972ce8 100644 --- a/ietf/meeting/test_data.py +++ b/ietf/meeting/test_data.py @@ -219,7 +219,7 @@ def make_interim_test_data(): ad = Person.objects.get(user__username='ad') RoleFactory(group=area,person=ad,name_id='ad') mars = GroupFactory(acronym='mars',parent=area,name='Martian Special Interest Group') - ames = GroupFactory(acronym='ames',parent=area) + ames = GroupFactory(acronym='ames',parent=area,name='Asteroid Mining Equipment Standardization Group') RoleFactory(group=mars,person__user__username='marschairman',name_id='chair') RoleFactory(group=ames,person__user__username='ameschairman',name_id='chair') diff --git a/ietf/meeting/tests_views.py b/ietf/meeting/tests_views.py index 78a6e4436..8376c8292 100644 --- a/ietf/meeting/tests_views.py +++ b/ietf/meeting/tests_views.py @@ -22,6 +22,7 @@ from django.conf import settings from django.contrib.auth.models import User from django.test import Client, override_settings from django.db.models import F +from django.http import QueryDict import debug # pyflakes:ignore @@ -37,7 +38,7 @@ from ietf.meeting.models import Session, TimeSlot, Meeting, SchedTimeSessAssignm from ietf.meeting.test_data import make_meeting_test_data, make_interim_meeting, make_interim_test_data from ietf.meeting.utils import finalize, condition_slide_order from ietf.meeting.utils import add_event_info_to_session_qs -from ietf.meeting.views import session_draft_list +from ietf.meeting.views import session_draft_list, parse_agenda_filter_params from ietf.name.models import SessionStatusName, ImportantDateName, RoleName from ietf.utils.decorators import skip_coverage from ietf.utils.mail import outbox, empty_outbox, get_payload_text @@ -61,6 +62,31 @@ else: "location indicated in settings.py.") print(" "+skip_message) + +def assert_ical_response_is_valid(test_inst, response, expected_event_summaries=None, expected_event_count=None): + """Validate an HTTP response containing iCal data + + Based on RFC2445, but not exhaustive by any means. Assumes a single iCalendar object. + """ + test_inst.assertEqual(response.get('Content-Type'), "text/calendar") + + # Validate iCalendar object + test_inst.assertContains(response, 'BEGIN:VCALENDAR', count=1) + test_inst.assertContains(response, 'END:VCALENDAR', count=1) + test_inst.assertContains(response, 'PRODID:', count=1) + test_inst.assertContains(response, 'VERSION', count=1) + + # Validate event objects + if expected_event_summaries is not None: + for summary in expected_event_summaries: + test_inst.assertContains(response, 'SUMMARY:' + summary) + + if expected_event_count is not None: + test_inst.assertContains(response, 'BEGIN:VEVENT', count=expected_event_count) + test_inst.assertContains(response, 'END:VEVENT', count=expected_event_count) + test_inst.assertContains(response, 'UID', count=expected_event_count) + + class MeetingTests(TestCase): def setUp(self): self.materials_dir = self.tempdir('materials') @@ -580,28 +606,6 @@ class MeetingTests(TestCase): post_date = meeting.importantdate_set.get(name=idn).date self.assertEqual(pre_date, post_date+datetime.timedelta(days=1)) - def assert_ical_response_is_valid(self, response, expected_event_summaries=None, expected_event_count=None): - """Validate an HTTP response containing iCal data - - Based on RFC2445, but not exhaustive by any means. Assumes a single iCalendar object. - """ - self.assertEqual(response.get('Content-Type'), "text/calendar") - - # Validate iCalendar object - self.assertContains(response, 'BEGIN:VCALENDAR', count=1) - self.assertContains(response, 'END:VCALENDAR', count=1) - self.assertContains(response, 'PRODID:', count=1) - self.assertContains(response, 'VERSION', count=1) - - # Validate event objects - if expected_event_count is None: - expected_event_count = len(expected_event_summaries) - self.assertContains(response, 'BEGIN:VEVENT', count=expected_event_count) - self.assertContains(response, 'END:VEVENT', count=expected_event_count) - self.assertContains(response, 'UID', count=expected_event_count) - for summary in expected_event_summaries: - self.assertContains(response, 'SUMMARY:' + summary) - def test_group_ical(self): meeting = make_meeting_test_data() s1 = Session.objects.filter(meeting=meeting, group__acronym="mars").first() @@ -614,17 +618,18 @@ class MeetingTests(TestCase): # url = urlreverse('ietf.meeting.views.agenda_ical', kwargs={'num':meeting.number, 'acronym':s1.group.acronym, }) r = self.client.get(url) - self.assert_ical_response_is_valid(r, - expected_event_summaries=['mars - Martian Special Interest Group'], - expected_event_count=2) + assert_ical_response_is_valid(self, + r, + expected_event_summaries=['mars - Martian Special Interest Group'], + expected_event_count=2) self.assertContains(r, t1.time.strftime('%Y%m%dT%H%M%S')) self.assertContains(r, t2.time.strftime('%Y%m%dT%H%M%S')) # url = urlreverse('ietf.meeting.views.agenda_ical', kwargs={'num':meeting.number, 'session_id':s1.id, }) r = self.client.get(url) - self.assert_ical_response_is_valid(r, - expected_event_summaries=['mars - Martian Special Interest Group'], - expected_event_count=1) + assert_ical_response_is_valid(self, r, + expected_event_summaries=['mars - Martian Special Interest Group'], + expected_event_count=1) self.assertContains(r, t1.time.strftime('%Y%m%dT%H%M%S')) self.assertNotContains(r, t2.time.strftime('%Y%m%dT%H%M%S')) @@ -652,7 +657,36 @@ class MeetingTests(TestCase): for g in groups: if g.parent_id is not None: self.assertIn('%s?show=%s' % (ical_url, g.parent.acronym.lower()), content) - + + def test_parse_agenda_filter_params(self): + def _r(show=(), hide=(), showtypes=(), hidetypes=()): + """Helper to create expected result dict""" + return dict(show=set(show), hide=set(hide), showtypes=set(showtypes), hidetypes=set(hidetypes)) + + self.assertIsNone(parse_agenda_filter_params(QueryDict(''))) + + self.assertRaises(ValueError, parse_agenda_filter_params, QueryDict('unknown')) # unknown param + self.assertRaises(ValueError, parse_agenda_filter_params, QueryDict('unknown=x')) # unknown param + + # test valid combos (not exhaustive) + for qstr, expected in ( + ('show=', _r()), ('hide=', _r()), ('showtypes=', _r()), ('hidetypes=', _r()), + ('show=x', _r(show=['x'])), ('hide=x', _r(hide=['x'])), + ('showtypes=x', _r(showtypes=['x'])), ('hidetypes=x', _r(hidetypes=['x'])), + ('show=x,y,z', _r(show=['x','y','z'])), + ('hide=x,y,z', _r(hide=['x','y','z'])), + ('showtypes=x,y,z', _r(showtypes=['x','y','z'])), + ('hidetypes=x,y,z', _r(hidetypes=['x','y','z'])), + ('show=a&hide=a', _r(show=['a'], hide=['a'])), + ('show=a&hide=b', _r(show=['a'], hide=['b'])), + ('show=a&hide=b&showtypes=c&hidetypes=d', _r(show=['a'], hide=['b'], showtypes=['c'], hidetypes=['d'])), + ): + self.assertEqual( + parse_agenda_filter_params(QueryDict(qstr)), + expected, + 'Parsed "%s" incorrectly' % qstr, + ) + def test_ical_filter_invalid_syntaxes(self): meeting = make_meeting_test_data() url = urlreverse('ietf.meeting.views.agenda_ical', kwargs={'num':meeting.number}) @@ -667,7 +701,10 @@ class MeetingTests(TestCase): url = urlreverse('ietf.meeting.views.agenda_ical', kwargs={'num':meeting.number}) r = self.client.get(url + querystring) self.assertEqual(r.status_code, 200) - self.assert_ical_response_is_valid(r, expected_event_summaries=expected_session_summaries) + assert_ical_response_is_valid(self, + r, + expected_event_summaries=expected_session_summaries, + expected_event_count=len(expected_session_summaries)) def test_ical_filter_default(self): meeting = make_meeting_test_data() @@ -1799,37 +1836,179 @@ class InterimTests(TestCase): #self.assertIn('CANCELLED', q('[id*="'+id+'"]').text()) self.assertIn('CANCELLED', q('tr>td>a>span').text()) - def test_upcoming(self): - make_meeting_test_data(create_interims=True) + def do_upcoming_test(self, querystring=None, create_meeting=True): + if create_meeting: + make_meeting_test_data(create_interims=True) url = urlreverse("ietf.meeting.views.upcoming") + if querystring is not None: + url += '?' + querystring + today = datetime.date.today() add_event_info_to_session_qs(Session.objects.filter(meeting__type='interim', group__acronym='mars')).filter(current_status='apprw').first() - mars_interim = add_event_info_to_session_qs(Session.objects.filter(meeting__type='interim', meeting__date__gt=today, group__acronym='mars')).filter(current_status='sched').first().meeting - ames_interim = add_event_info_to_session_qs(Session.objects.filter(meeting__type='interim', meeting__date__gt=today, group__acronym='ames')).filter(current_status='canceled').first().meeting - r = self.client.get(url) - self.assertContains(r, mars_interim.number) - self.assertContains(r, ames_interim.number) + interims = dict( + mars=add_event_info_to_session_qs(Session.objects.filter(meeting__type='interim', meeting__date__gt=today, group__acronym='mars')).filter(current_status='sched').first().meeting, + ames=add_event_info_to_session_qs(Session.objects.filter(meeting__type='interim', meeting__date__gt=today, group__acronym='ames')).filter(current_status='canceled').first().meeting, + ) + self.check_interim_tabs(url) + return self.client.get(url), interims + + def test_upcoming(self): + r, interims = self.do_upcoming_test() + self.assertContains(r, interims['mars'].number) + self.assertContains(r, interims['ames'].number) self.assertContains(r, 'IETF 72') # cancelled session q = PyQuery(r.content) self.assertIn('CANCELLED', q('tr>td.text-right>span').text()) - self.check_interim_tabs(url) + + def test_upcoming_filter_show(self): + r, interims = self.do_upcoming_test('show=ames') + self.assertNotContains(r, interims['mars'].number) + self.assertContains(r, interims['ames'].number) + self.assertContains(r, 'IETF 72') + # cancelled session + q = PyQuery(r.content) + self.assertIn('CANCELLED', q('tr>td.text-right>span').text()) + + def test_upcoming_filter_show_area(self): + make_meeting_test_data(create_interims=True) + area = Group.objects.get(acronym='mars').parent + self.assertEqual(area, + Group.objects.get(acronym='ames').parent, + 'The mars and ames groups have different areas; this breaks this test') + r, interims = self.do_upcoming_test('show=%s' % area.acronym, create_meeting=False) + self.assertContains(r, interims['mars'].number) + self.assertContains(r, interims['ames'].number) + self.assertContains(r, 'IETF 72') + + def test_upcoming_filter_hide(self): + r, interims = self.do_upcoming_test('hide=mars') + self.assertNotContains(r, interims['mars'].number) + self.assertNotContains(r, interims['ames'].number) + self.assertContains(r, 'IETF 72') + + def test_upcoming_filter_show_and_hide(self): + r, interims = self.do_upcoming_test('show=mars,ames&hide=ames') + self.assertContains(r, interims['mars'].number) + self.assertNotContains(r, interims['ames'].number) + self.assertContains(r, 'IETF 72') + + def do_upcoming_ical_test(self, querystring=None): + make_meeting_test_data(create_interims=True) + + # Create a group with a plenary interim session for testing type filters + somegroup = GroupFactory(acronym='sg', name='Some Group') + sg_interim = make_interim_meeting(somegroup, datetime.date.today() + datetime.timedelta(days=20)) + sg_sess = sg_interim.session_set.first() + sg_slot = sg_sess.timeslotassignments.first().timeslot + sg_sess.type_id = 'plenary' + sg_slot.type_id = 'plenary' + sg_sess.save() + sg_slot.save() + + url = urlreverse("ietf.meeting.views.upcoming_ical") + if querystring is not None: + url += '?' + querystring + r = self.client.get(url) + self.assertEqual(r.status_code, 200) + return r def test_upcoming_ical(self): - make_meeting_test_data(create_interims=True) - url = urlreverse("ietf.meeting.views.upcoming_ical") - r = self.client.get(url) - self.assertEqual(r.status_code, 200) - self.assertEqual(r.get('Content-Type'), "text/calendar") - self.assertEqual(r.content.count(b'UID'), 8) - # check filtered output - url = url + '?filters=mars' - r = self.client.get(url) - self.assertEqual(r.status_code, 200) - self.assertEqual(r.get('Content-Type'), "text/calendar") - # print r.content - self.assertEqual(r.content.count(b'UID'), 2) + r = self.do_upcoming_ical_test() + print(r.content.decode()) + assert_ical_response_is_valid(self, r, + expected_event_summaries=[ + 'ames - Asteroid Mining Equipment Standardization Group', + 'mars - Martian Special Interest Group', + 'sg - Some Group', + ], + expected_event_count=9) + def test_upcoming_ical_filter_show(self): + r = self.do_upcoming_ical_test('show=mars,ames') + assert_ical_response_is_valid(self, r, + expected_event_summaries=[ + 'mars - Martian Special Interest Group', + 'ames - Asteroid Mining Equipment Standardization Group', + ]) + + def test_upcoming_ical_filter_hide(self): + r = self.do_upcoming_ical_test('hide=mars') + assert_ical_response_is_valid(self, r, expected_event_summaries=[]) + + def test_upcoming_ical_filter_show_and_hide(self): + r = self.do_upcoming_ical_test('show=mars,ames&hide=mars') + assert_ical_response_is_valid(self, r, + expected_event_summaries=[ + 'ames - Asteroid Mining Equipment Standardization Group', + ]) + + def test_upcoming_ical_filter_showtypes(self): + r = self.do_upcoming_ical_test('showtypes=regular') + assert_ical_response_is_valid(self, r, + expected_event_summaries=[ + 'mars - Martian Special Interest Group', + 'ames - Asteroid Mining Equipment Standardization Group', + ]) + + def test_upcoming_ical_filter_hidetypes(self): + r = self.do_upcoming_ical_test('hidetypes=regular') + assert_ical_response_is_valid(self, r, expected_event_summaries=[]) + + def test_upcoming_ical_filter_showtypes_and_hidetypes(self): + r = self.do_upcoming_ical_test('showtypes=plenary,regular&hidetypes=regular') + assert_ical_response_is_valid(self, r, + expected_event_summaries=[ + 'sg - Some Group', + ]) + + def test_upcoming_ical_filter_show_and_showtypes(self): + r = self.do_upcoming_ical_test('show=mars&showtypes=plenary') + assert_ical_response_is_valid(self, r, + expected_event_summaries=[ + 'mars - Martian Special Interest Group', + 'sg - Some Group', + ]) + + def test_upcoming_ical_filter_show_and_hidetypes(self): + r = self.do_upcoming_ical_test('show=mars,sg&showtypes=regular') + assert_ical_response_is_valid(self, r, + expected_event_summaries=[ + 'sg - Some Group', + ]) + + def test_upcoming_ical_filter_hide_and_showtypes(self): + r = self.do_upcoming_ical_test('hide=mars&showtypes=regular') + assert_ical_response_is_valid(self, r, + expected_event_summaries=[ + 'ames - Asteroid Mining Equipment Standardization Group', + ]) + + def test_upcoming_ical_filter_hide_and_hidetypes(self): + r = self.do_upcoming_ical_test('hide=mars&hidetypes=regular') + assert_ical_response_is_valid(self, r, expected_event_summaries=[]) + + def test_upcoming_ical_filter_show_hide_and_showtypes(self): + r = self.do_upcoming_ical_test('show=ames&hide=mars&showtypes=regular,plenary') + assert_ical_response_is_valid(self, r, + expected_event_summaries=[ + 'ames - Asteroid Mining Equipment Standardization Group', + 'sg - Some Group', + ]) + + def test_upcoming_ical_filter_show_hide_and_hidetypes(self): + r = self.do_upcoming_ical_test('show=ames,sg&hide=mars&hidetypes=regular') + assert_ical_response_is_valid(self, r, + expected_event_summaries=[ + 'sg - Some Group', + ]) + + def test_upcoming_ical_filter_all_params(self): + r = self.do_upcoming_ical_test('show=sg&hide=ames&showtypes=regular&hidetypes=plenary') + assert_ical_response_is_valid(self, r, + expected_event_summaries=[ + 'mars - Martian Special Interest Group', + ]) def test_upcoming_json(self): make_meeting_test_data(create_interims=True) diff --git a/ietf/meeting/views.py b/ietf/meeting/views.py index 625044f25..c81250cbc 100644 --- a/ietf/meeting/views.py +++ b/ietf/meeting/views.py @@ -1336,6 +1336,58 @@ def ical_session_status(session_with_current_status): else: return "CONFIRMED" +def parse_agenda_filter_params(querydict): + """Parse agenda filter parameters from a request""" + if len(querydict) == 0: + return None + + # Parse group filters from GET parameters. The keys in this dict define the + # allowed querystring parameters. + filt_params = {'show': set(), 'hide': set(), 'showtypes': set(), 'hidetypes': set()} + + for key, value in querydict.items(): + if key not in filt_params: + raise ValueError('Unrecognized parameter "%s"' % key) + if value is None: + return ValueError( + 'Parameter "%s" is not assigned a value (use "key=" for an empty value)' % key + ) + vals = unquote(value).lower().split(',') + filt_params[key] = set([v for v in vals if len(v) > 0]) # remove empty strings + + return filt_params + + +def should_include_assignment(filter_params, assignment): + """Decide whether to include an assignment + + When filtering by wg, uses historic_group if available as an attribute + on the session, otherwise falls back to using group. + """ + historic_group = getattr(assignment.session, 'historic_group', None) + if historic_group: + group_acronym = historic_group.acronym + parent = historic_group.historic_parent + parent_acronym = parent.acronym if parent else None + else: + group = assignment.session.group + group_acronym = group.acronym + if group.parent: + parent_acronym = group.parent.acronym + else: + parent_acronym = None + session_type = assignment.timeslot.type_id + + # Hide if wg or type hide lists apply + if (group_acronym in filter_params['hide']) or (session_type in filter_params['hidetypes']): + return False + + # Show if any of the show lists apply, including showing by parent group + return ((group_acronym in filter_params['show']) or + (parent_acronym in filter_params['show']) or + (session_type in filter_params['showtypes'])) + + def agenda_ical(request, num=None, name=None, acronym=None, session_id=None): """Agenda ical view @@ -1364,46 +1416,14 @@ def agenda_ical(request, num=None, name=None, acronym=None, session_id=None): assignments = schedule.assignments.exclude(timeslot__type__in=['lead','offagenda']) assignments = preprocess_assignments_for_agenda(assignments, meeting) - if len(request.GET) > 0: - # Parse group filters from GET parameters. The keys in this dict define the - # allowed querystring parameters. - filt_params = {'show': set(), 'hide': set(), 'showtypes': set(), 'hidetypes': set()} - - for key, value in request.GET.items(): - if key not in filt_params: - return HttpResponseBadRequest('Unrecognized parameter "%s"' % key) - if value is None: - return HttpResponseBadRequest( - 'Parameter "%s" is not assigned a value (use "key=" for an empty value)' % key - ) - filt_params[key] = set(unquote(value).lower().split(',')) + try: + filt_params = parse_agenda_filter_params(request.GET) + except ValueError as e: + return HttpResponseBadRequest(str(e)) - def _should_include_assignment(assignment): - """Decide whether to include an assignment - - Relies on filt_params from parent scope. - """ - historic_group = assignment.session.historic_group - if historic_group: - group_acronym = historic_group.acronym - parent = historic_group.historic_parent - parent_acronym = parent.acronym if parent else None - else: - group_acronym = None - parent_acronym = None - session_type = assignment.timeslot.type_id - - # Hide if wg or type hide lists apply - if (group_acronym in filt_params['hide']) or (session_type in filt_params['hidetypes']): - return False - - # Show if any of the show lists apply, including showing by parent group - return ((group_acronym in filt_params['show']) or - (parent_acronym in filt_params['show']) or - (session_type in filt_params['showtypes'])) - + if filt_params is not None: # Apply the filter - assignments = [a for a in assignments if _should_include_assignment(a)] + assignments = [a for a in assignments if should_include_assignment(filt_params, a)] if acronym: assignments = [ a for a in assignments if a.session.historic_group and a.session.historic_group.acronym == acronym ] @@ -2754,24 +2774,47 @@ def past(request): }) def upcoming(request): - '''List of upcoming meetings''' + """List of upcoming meetings + + Only querystring filters by wg name are supported. Always includes IETF meetings; + filters 'interim' type meetings by wg name as requested. The showtypes/hidetypes + filters are ignored.. + """ today = datetime.date.today() + filter_params = parse_agenda_filter_params(request.GET) # Get ietf meetings starting 7 days ago, and interim meetings starting today ietf_meetings = Meeting.objects.filter(type_id='ietf', date__gte=today-datetime.timedelta(days=7)) for m in ietf_meetings: m.end = m.date+datetime.timedelta(days=m.days) + entries = list(ietf_meetings) + interim_sessions = add_event_info_to_session_qs( Session.objects.filter( - meeting__type_id='interim', + meeting__type_id='interim', timeslotassignments__schedule=F('meeting__schedule'), timeslotassignments__timeslot__time__gte=today ) ).filter(current_status__in=('sched','canceled')) + if filter_params is not None: + group_shown = interim_sessions.filter( + group__acronym__in=filter_params['show'] + ) + parent_group_shown = interim_sessions.filter( + group__parent__acronym__in=filter_params['show'] + ) + # The '|' combines querysets with OR - qs.filter(x=1) | qs.filter(y=2) + # translates to a 'WHERE x=1 OR y=2' in the SQL. + interim_sessions = ( + group_shown | parent_group_shown + ).exclude( + # N.B., we only consider parent group (area) for show, not for hide. + # This is consistent with previous behavior but is worth revisiting. + group__acronym__in=filter_params['hide'] + ) + for session in interim_sessions: session.historic_group = session.group - - entries = list(ietf_meetings) entries.extend(list(interim_sessions)) entries.sort(key = lambda o: pytz.utc.localize(datetime.datetime.combine(o.date, datetime.datetime.min.time())) if isinstance(o,Meeting) else o.official_timeslotassignment().timeslot.utc_start_time()) @@ -2800,8 +2843,11 @@ def upcoming(request): def upcoming_ical(request): - '''Return Upcoming meetings in iCalendar file''' - filters = request.GET.getlist('filters') + """Return Upcoming meetings in iCalendar file + + Filters by wg name and session type. + """ + filter_params = parse_agenda_filter_params(request.GET) today = datetime.date.today() # get meetings starting 7 days ago -- we'll filter out sessions in the past further down @@ -2818,13 +2864,8 @@ def upcoming_ical(request): ).distinct()) # apply filters - if filters: - assignments = [a for a in assignments if - a.session.group and ( - a.session.group.acronym in filters or ( - a.session.group.parent and a.session.group.parent.acronym in filters - ) - ) ] + if filter_params is not None: + assignments = [a for a in assignments if should_include_assignment(filter_params, a)] # we already collected sessions with current_status, so reuse those sessions = {s.pk: s for m in meetings for s in m.sessions} diff --git a/ietf/templates/group/meetings.html b/ietf/templates/group/meetings.html index 8672c8d36..24a3603d6 100644 --- a/ietf/templates/group/meetings.html +++ b/ietf/templates/group/meetings.html @@ -34,7 +34,7 @@