fix: Render complete iCalendar event when show_location is False (#5394)

* test: More carefully test event syntax in ical file

* test: Test agenda.ics view with all meeting sessions

* fix: Render complete iCalendar event when show_location is False

* chore: Fix confusing comment

ietf/meeting/tests_views.py

@@ -219,6 +219,7 @@ def test_meeting_agenda(self):
         slot = TimeSlot.objects.get(sessionassignments__session=session,sessionassignments__schedule=meeting.schedule)
         slot.location.urlresource_set.create(name_id='meetecho_onsite', url='https://onsite.example.com')
         slot.location.urlresource_set.create(name_id='meetecho', url='https://meetecho.example.com')
+        meeting.timeslot_set.filter(type_id="break").update(show_location=False)
         #
         self.write_materials_files(meeting, session)
         #
@@ -348,9 +349,17 @@ def test_meeting_agenda(self):
         self.assertContains(r, session.materials.filter(type='slides').exclude(states__type__slug='slides',states__slug='deleted').first().uploaded_filename)
         self.assertNotContains(r, session.materials.filter(type='slides',states__type__slug='slides',states__slug='deleted').first().uploaded_filename)
 
-        # iCal
-        r = self.client.get(urlreverse("ietf.meeting.views.agenda_ical", kwargs=dict(num=meeting.number))
-                            + "?show=" + session.group.parent.acronym.upper())
+        # iCal, no session filtering
+        ical_url = urlreverse("ietf.meeting.views.agenda_ical", kwargs=dict(num=meeting.number))
+        r = self.client.get(ical_url)
+        with open('./ical-output.ics', 'w') as f:
+            f.write(r.content.decode())
+        assert_ical_response_is_valid(self, r)
+        self.assertContains(r, "BEGIN:VTIMEZONE")
+        self.assertContains(r, "END:VTIMEZONE")
+
+        # iCal, single group
+        r = self.client.get(ical_url + "?show=" + session.group.parent.acronym.upper())
         assert_ical_response_is_valid(self, r)
         self.assertContains(r, session.group.acronym)
         self.assertContains(r, session.group.name)

ietf/templates/meeting/agenda.ics

@@ -6,7 +6,7 @@ PRODID:-//IETF//datatracker.ietf.org ical agenda//EN
 UID:ietf-{{schedule.meeting.number}}-{{item.timeslot.pk}}-{{item.session.group.acronym}}
 SUMMARY:{% if item.session.name %}{{item.session.name|ics_esc}}{% else %}{{item.session.group_at_the_time.acronym|lower}} - {{item.session.group_at_the_time.name}}{%endif%}{% if item.session.agenda_note %} ({{item.session.agenda_note}}){% endif %}
 {% if item.timeslot.show_location %}LOCATION:{{item.timeslot.get_location}}
-STATUS:{{item.session.ical_status}}
+{% endif %}STATUS:{{item.session.ical_status}}
 CLASS:PUBLIC
 DTSTART{% ics_date_time item.timeslot.local_start_time schedule.meeting.time_zone %}
 DTEND{% ics_date_time item.timeslot.local_end_time schedule.meeting.time_zone %}
@@ -29,4 +29,4 @@ DESCRIPTION:{{item.timeslot.name|ics_esc}}\n{% if item.session.agenda_note %}
  \n{# link agenda for ietf meetings #}
  See in schedule: {% absurl 'agenda' num=schedule.meeting.number %}#row-{{ item.slug }}\n{% endif %}
 END:VEVENT
-{% endif %}{% endfor %}END:VCALENDAR{% endcache %}{% endtimezone %}{% endautoescape %}
+{% endfor %}END:VCALENDAR{% endcache %}{% endtimezone %}{% endautoescape %}

ietf/utils/test_utils.py

@@ -118,7 +118,7 @@ def assert_ical_response_is_valid(test_inst, response, expected_event_summaries=
                                   expected_event_uids=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. Checks that
+    Based on RFC5545, but not exhaustive by any means. Assumes a single iCalendar object. Checks that
     expected_event_summaries/_uids are found, but other events are allowed to be present. Specify the
     expected_event_count if you want to reject additional events. If any of these are None,
     the check for that property is skipped.
@@ -132,18 +132,43 @@ def assert_ical_response_is_valid(test_inst, response, expected_event_summaries=
     test_inst.assertContains(response, 'VERSION', count=1)
 
     # Validate event objects
+    event_count = 0
+    uids_found = set()
+    summaries_found = set()
+    got_begin = False
+    cur_event_props = set()
+    for line_num, line in enumerate(response.content.decode().split("\n")):
+        line = line.rstrip()
+        if line == 'BEGIN:VEVENT':
+            test_inst.assertFalse(got_begin, f"Nested BEGIN:VEVENT found on line {line_num + 1}")
+            got_begin = True
+        elif line == 'END:VEVENT':
+            test_inst.assertTrue(got_begin, f"Unexpected END:VEVENT on line {line_num + 1}")
+            test_inst.assertIn("uid", cur_event_props, f"Found END:VEVENT without UID on line {line_num + 1}")
+            got_begin = False
+            cur_event_props.clear()
+            event_count += 1
+        elif got_begin:
+            # properties in an event
+            if line.startswith("UID:"):
+                # mandatory, not more than once
+                test_inst.assertNotIn("uid", cur_event_props, f"Two UID properties in single event on line {line_num + 1}")
+                cur_event_props.add("uid")
+                uids_found.add(line.split(":", 1)[1])
+            elif line.startswith("SUMMARY:"):
+                # optional, not more than once
+                test_inst.assertNotIn("summary", cur_event_props, f"Two SUMMARY properties in single event on line {line_num + 1}")
+                cur_event_props.add("summary")
+                summaries_found.add(line.split(":", 1)[1])
+
     if expected_event_summaries is not None:
-        for summary in expected_event_summaries:
-            test_inst.assertContains(response, 'SUMMARY:' + summary)
+        test_inst.assertCountEqual(summaries_found, set(expected_event_summaries))
 
     if expected_event_uids is not None:
-        for uid in expected_event_uids:
-            test_inst.assertContains(response, 'UID:' + uid)
+        test_inst.assertCountEqual(uids_found, set(expected_event_uids))
 
     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)
+        test_inst.assertEqual(event_count, expected_event_count)
 
     # make sure no doubled colons after timestamp properties
     test_inst.assertNotContains(response, 'DTSTART::')