fix: handle groups_at_the_time cache misses (#4853)

* fix: handle groups_at_the_time cache misses

* refactor: add comments, share meeting-start code between calls

* test: test Meeting.group_at_the_time()

ietf/meeting/models.py

@@ -39,7 +39,7 @@ from ietf.name.models import (
 )
 from ietf.person.models import Person
 from ietf.utils.decorators import memoize
-from ietf.utils.history import find_history_replacements_active_at
+from ietf.utils.history import find_history_replacements_active_at, find_history_active_at
 from ietf.utils.storage import NoLocationMigrationFileSystemStorage
 from ietf.utils.text import xslugify
 from ietf.utils.timezone import datetime_from_date, date_today
@@ -409,18 +409,36 @@ class Meeting(models.Model):
     def uses_notes(self):
         return self.date>=datetime.date(2020,7,6)
 
-    def groups_at_the_time(self):
+    def meeting_start(self):
+        """Meeting-local midnight at the start of the meeting date"""
+        return self.tz().localize(datetime.datetime.combine(self.date, datetime.time()))
+
+    def _groups_at_the_time(self):
+        """Get dict mapping Group PK to appropriate Group or GroupHistory at meeting time
+
+        Known issue: only looks up Groups and their *current* parents when called. If a Group's
+        parent was different at meeting time, that parent will not be in the cache. Use
+        group_at_the_time() to look up values - that will fill in missing groups for you.
+        """
         if not hasattr(self,'cached_groups_at_the_time'):
             all_group_pks = set(self.session_set.values_list('group__pk', flat=True))
             all_group_pks.update(self.session_set.values_list('group__parent__pk', flat=True))
             all_group_pks.discard(None)
-            # meeting_time is meeting-local midnight at the start of the meeting date
-            meeting_start = self.tz().localize(
-                datetime.datetime.combine(self.date, datetime.time())
+            self.cached_groups_at_the_time = find_history_replacements_active_at(
+                Group.objects.filter(pk__in=all_group_pks),
+                self.meeting_start(),
             )
-            self.cached_groups_at_the_time = find_history_replacements_active_at(Group.objects.filter(pk__in=all_group_pks), meeting_start)
         return self.cached_groups_at_the_time
 
+    def group_at_the_time(self, group):
+        # MUST call self._groups_at_the_time() before assuming cached_groups_at_the_time exists
+        gatt = self._groups_at_the_time()
+        if group.pk in gatt:
+            return gatt[group.pk]
+        # Cache miss - look up the missing historical group and add it to the cache.
+        new_item = find_history_active_at(group, self.meeting_start()) or group  # fall back to original if no history
+        self.cached_groups_at_the_time[group.pk] = new_item
+        return new_item
 
     class Meta:
         ordering = ["-date", "-id"]
@@ -1301,11 +1319,12 @@ class Session(models.Model):
         return urljoin(settings.IETF_NOTES_URL, self.notes_id())
 
     def group_at_the_time(self):
-        return self.meeting.groups_at_the_time()[self.group.pk]
+        return self.meeting.group_at_the_time(self.group)
 
     def group_parent_at_the_time(self):
         if self.group_at_the_time().parent:
-            return self.meeting.groups_at_the_time()[self.group_at_the_time().parent.pk]
+            return self.meeting.group_at_the_time(self.group_at_the_time().parent)
+
 
 class SchedulingEvent(models.Model):
     session = ForeignKey(Session)

ietf/meeting/tests_models.py

@@ -5,9 +5,11 @@ import datetime
 
 from mock import patch
 
+from ietf.group.factories import GroupFactory, GroupHistoryFactory
 from ietf.meeting.factories import MeetingFactory, SessionFactory, AttendedFactory
 from ietf.stats.factories import MeetingRegistrationFactory
 from ietf.utils.test_utils import TestCase
+from ietf.utils.timezone import date_today, datetime_today
 
 
 class MeetingTests(TestCase):
@@ -104,6 +106,14 @@ class MeetingTests(TestCase):
             vtz = meeting.vtimezone()
         self.assertIsNone(vtz)
 
+    def test_group_at_the_time(self):
+        m = MeetingFactory(type_id='ietf', date=date_today() - datetime.timedelta(days=10))
+        cached_groups = GroupFactory.create_batch(2)
+        m.cached_groups_at_the_time = {g.pk: g for g in cached_groups}  # fake the cache
+        uncached_group_hist = GroupHistoryFactory(time=datetime_today() - datetime.timedelta(days=30))
+        self.assertEqual(m.group_at_the_time(uncached_group_hist.group), uncached_group_hist)
+        self.assertIn(uncached_group_hist.group.pk, m.cached_groups_at_the_time)
+
 
 class SessionTests(TestCase):
     def test_chat_archive_url_with_jabber(self):