From f5281af43fd5ee6cdc8974d4cd74e1b499c76539 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Laur=C3=A9line=20Gu=C3=A9rin?= <zebuline@entrouvert.com>
Date: Tue, 2 Feb 2021 11:54:34 +0100
Subject: [PATCH 11/11] manager: use prefeched objects is available (#48924)

---
 chrono/agendas/models.py | 65 ++++++++++++++++++++++++-----
 chrono/manager/views.py  |  2 +-
 tests/test_agendas.py    | 77 ++++++++++++++++++----------------
 tests/test_manager.py    | 89 +++++++++++++++++++++++++++-------------
 4 files changed, 158 insertions(+), 75 deletions(-)

diff --git a/chrono/agendas/models.py b/chrono/agendas/models.py
index 99145be..2a256eb 100644
--- a/chrono/agendas/models.py
+++ b/chrono/agendas/models.py
@@ -544,7 +544,7 @@ class Agenda(models.Model):
         except (VariableDoesNotExist, TemplateSyntaxError):
             return
 
-    def prefetch_desks_and_exceptions(self):
+    def prefetch_desks_and_exceptions(self, with_sources=False):
         if self.kind == 'meetings':
             desks = self.desk_set.all()
         elif self.kind == 'virtual':
@@ -557,6 +557,8 @@ class Agenda(models.Model):
             raise ValueError('does not work with kind %r' % self.kind)
 
         self.prefetched_desks = desks.prefetch_related('timeperiod_set', 'unavailability_calendars')
+        if with_sources:
+            self.prefetched_desks = self.prefetched_desks.prefetch_related('timeperiodexceptionsource_set')
         unavailability_calendar_ids = UnavailabilityCalendar.objects.filter(
             desks__in=self.prefetched_desks
         ).values('pk')
@@ -575,43 +577,86 @@ class Agenda(models.Model):
         if self.kind != 'meetings':
             return False
 
-        desks = self.desk_set.all()
+        was_prefetched = False
+        if hasattr(self, 'prefetched_desks'):
+            desks = self.prefetched_desks
+            was_prefetched = True
+        else:
+            desks = self.desk_set.all()
         if len(desks) < 2:
             # no desk or just on, it's ok
             return True
 
         desk = desks[0]
 
+        def values_list(obj, qs_name, qs, fields, for_exception=False):
+            if not was_prefetched:
+                prefetched_qs = getattr(obj, qs).values_list(*fields)
+                if for_exception:
+                    prefetched_qs = prefetched_qs.filter(source__isnull=True)
+                return prefetched_qs
+            values = []
+            if for_exception:
+                prefetched_qs = obj.prefetched_exceptions
+            else:
+                prefetched_qs = obj._prefetched_objects_cache.get(qs_name)  # XXX django 1.11 compat
+                if prefetched_qs is None:
+                    prefetched_qs = obj._prefetched_objects_cache.get(qs)
+            for inst in prefetched_qs:
+                # queryset is prefetched, fake values_list
+                if for_exception and inst.source_id is not None:
+                    continue
+                values.append(tuple(getattr(inst, f) for f in fields))
+            return values
+
         period_fields = ['weekday', 'start_time', 'end_time']
         exception_fields = ['label', 'start_datetime', 'end_datetime']
         source_fields = ['ics_filename', 'ics_url', 'settings_slug', 'enabled']
-        desk_time_periods = set(desk.timeperiod_set.values_list(*period_fields))
+        desk_time_periods = set(values_list(desk, 'timeperiod', 'timeperiod_set', period_fields))
         desk_exceptions = set(
-            desk.timeperiodexception_set.filter(source__isnull=True).values_list(*exception_fields)
+            values_list(
+                desk, 'timeperiodexception', 'timeperiodexception_set', exception_fields, for_exception=True
+            )
+        )
+        desk_sources = set(
+            values_list(desk, 'timeperiodexceptionsource', 'timeperiodexceptionsource_set', source_fields)
+        )
+        desk_unavaibility_calendars = set(
+            values_list(desk, 'unavailability_calendars', 'unavailability_calendars', ['pk'])
         )
-        desk_sources = set(desk.timeperiodexceptionsource_set.values_list(*source_fields))
-        desk_unavaibility_calendars = set(desk.unavailability_calendars.values_list('pk', flat=True))
         for other_desk in desks[1:]:
             # compare time periods
-            other_desk_time_periods = set(other_desk.timeperiod_set.values_list(*period_fields))
+            other_desk_time_periods = set(
+                values_list(other_desk, 'timeperiod', 'timeperiod_set', period_fields)
+            )
             if desk_time_periods != other_desk_time_periods:
                 return False
 
             # compare exceptions
             other_desk_exceptions = set(
-                other_desk.timeperiodexception_set.filter(source__isnull=True).values_list(*exception_fields)
+                values_list(
+                    other_desk,
+                    'timeperiodexception',
+                    'timeperiodexception_set',
+                    exception_fields,
+                    for_exception=True,
+                )
             )
             if desk_exceptions != other_desk_exceptions:
                 return False
 
             # compare sources
-            other_desk_sources = set(other_desk.timeperiodexceptionsource_set.values_list(*source_fields))
+            other_desk_sources = set(
+                values_list(
+                    other_desk, 'timeperiodexceptionsource', 'timeperiodexceptionsource_set', source_fields
+                )
+            )
             if desk_sources != other_desk_sources:
                 return False
 
             # compare unavailability calendars
             other_desk_unavaibility_calendars = set(
-                other_desk.unavailability_calendars.values_list('pk', flat=True)
+                values_list(other_desk, 'unavailability_calendars', 'unavailability_calendars', ['pk'])
             )
             if desk_unavaibility_calendars != other_desk_unavaibility_calendars:
                 return False
diff --git a/chrono/manager/views.py b/chrono/manager/views.py
index 8bdf8ca..c54cb85 100644
--- a/chrono/manager/views.py
+++ b/chrono/manager/views.py
@@ -1360,7 +1360,7 @@ class AgendaSettings(ManagedAgendaMixin, DetailView):
 
     def get_object(self, *args, **kwargs):
         if self.agenda.kind == 'meetings':
-            self.agenda.prefetch_desks_and_exceptions()
+            self.agenda.prefetch_desks_and_exceptions(with_sources=True)
         return self.agenda
 
     def get_context_data(self, **kwargs):
diff --git a/tests/test_agendas.py b/tests/test_agendas.py
index 049d9c5..8c8804b 100644
--- a/tests/test_agendas.py
+++ b/tests/test_agendas.py
@@ -177,28 +177,35 @@ def test_category_duplicate_slugs():
     assert category.slug == 'foo-baz-2'
 
 
-def test_agenda_is_available_for_simple_management(settings):
+@pytest.mark.parametrize('with_prefetch', [True, False])
+def test_agenda_is_available_for_simple_management(settings, with_prefetch):
     settings.EXCEPTIONS_SOURCES = {
         'holidays': {'class': 'workalendar.europe.France', 'label': 'Holidays'},
     }
 
+    def check_is_available(result, use_prefetch=True):
+        agenda = Agenda.objects.get()
+        if with_prefetch and use_prefetch:
+            agenda.prefetch_desks_and_exceptions(with_sources=True)
+        assert agenda.is_available_for_simple_management() == result
+
     agenda = Agenda.objects.create(label='Agenda', kind='meetings')
     # no desks
-    assert agenda.is_available_for_simple_management() is True
+    check_is_available(True)
 
     # check kind
     agenda.kind = 'events'
     agenda.save()
-    assert agenda.is_available_for_simple_management() is False
+    check_is_available(False, use_prefetch=False)
     agenda.kind = 'virtual'
     agenda.save()
-    assert agenda.is_available_for_simple_management() is False
+    check_is_available(False, use_prefetch=False)
 
     # only one desk
     agenda.kind = 'meetings'
     agenda.save()
     desk = Desk.objects.create(label='Desk', agenda=agenda)
-    assert agenda.is_available_for_simple_management() is True
+    check_is_available(True)
 
     # create some related data for this desk
     time_period = TimePeriod.objects.create(
@@ -224,37 +231,37 @@ def test_agenda_is_available_for_simple_management(settings):
     unavailability_calendar2 = UnavailabilityCalendar.objects.create(label='Calendar 2')
 
     # still ok
-    assert agenda.is_available_for_simple_management() is True
+    check_is_available(True)
 
     # duplicate the desk twice
     desk2 = desk.duplicate()
     desk.duplicate()
 
     # still ok
-    assert agenda.is_available_for_simple_management() is True
+    check_is_available(True)
 
     # changes on time periods
     for _desk in [desk, desk2]:
         time_period2 = TimePeriod.objects.create(
             weekday=2, desk=_desk, start_time=datetime.time(10, 0), end_time=datetime.time(12, 0)
         )
-        assert agenda.is_available_for_simple_management() is False
+        check_is_available(False)
         time_period2.delete()
-        assert agenda.is_available_for_simple_management() is True
+        check_is_available(True)
     time_period.weekday = 2
     time_period.save()
-    assert agenda.is_available_for_simple_management() is False
+    check_is_available(False)
     time_period.weekday = 1
     time_period.start_time = datetime.time(10, 1)
     time_period.save()
-    assert agenda.is_available_for_simple_management() is False
+    check_is_available(False)
     time_period.start_time = datetime.time(10, 0)
     time_period.end_time = datetime.time(12, 1)
     time_period.save()
-    assert agenda.is_available_for_simple_management() is False
+    check_is_available(False)
     time_period.end_time = datetime.time(12, 0)
     time_period.save()
-    assert agenda.is_available_for_simple_management() is True
+    check_is_available(True)
 
     # changes on exceptions
     for _desk in [desk, desk2]:
@@ -264,23 +271,23 @@ def test_agenda_is_available_for_simple_management(settings):
             start_datetime=date_now + datetime.timedelta(days=3),
             end_datetime=date_now + datetime.timedelta(days=4),
         )
-        assert agenda.is_available_for_simple_management() is False
+        check_is_available(False)
         exception2.delete()
-        assert agenda.is_available_for_simple_management() is True
+        check_is_available(True)
     exception.label = 'Exception blah'
     exception.save()
-    assert agenda.is_available_for_simple_management() is False
+    check_is_available(False)
     exception.label = 'Exception'
     exception.start_datetime = date_now + datetime.timedelta(days=3)
     exception.save()
-    assert agenda.is_available_for_simple_management() is False
+    check_is_available(False)
     exception.start_datetime = date_now + datetime.timedelta(days=1)
     exception.end_datetime = date_now + datetime.timedelta(days=1)
     exception.save()
-    assert agenda.is_available_for_simple_management() is False
+    check_is_available(False)
     exception.end_datetime = date_now + datetime.timedelta(days=2)
     exception.save()
-    assert agenda.is_available_for_simple_management() is True
+    check_is_available(True)
     # exceptions from source are not checked
     exception3 = TimePeriodException.objects.create(
         label='Exception',
@@ -289,7 +296,7 @@ def test_agenda_is_available_for_simple_management(settings):
         end_datetime=date_now + datetime.timedelta(days=4),
         source=source2,
     )
-    assert agenda.is_available_for_simple_management() is True
+    check_is_available(True)
     exception3.delete()
 
     # changes on sources - from settings
@@ -297,34 +304,34 @@ def test_agenda_is_available_for_simple_management(settings):
         source = TimePeriodExceptionSource.objects.create(
             desk=_desk, settings_slug='holidays-bis', enabled=True
         )
-        assert agenda.is_available_for_simple_management() is False
+        check_is_available(False)
         source.delete()
-        assert agenda.is_available_for_simple_management() is True
+        check_is_available(True)
     source1.enabled = False
     source1.save()
-    assert agenda.is_available_for_simple_management() is False
+    check_is_available(False)
     source1.enabled = True
     source1.settings_slug = 'holidays-bis'
     source1.save()
-    assert agenda.is_available_for_simple_management() is False
+    check_is_available(False)
     source1.settings_slug = 'holidays'
     source1.save()
-    assert agenda.is_available_for_simple_management() is True
+    check_is_available(True)
 
     # changes on sources - from url
     for _desk in [desk, desk2]:
         source = TimePeriodExceptionSource.objects.create(
             desk=_desk, ics_url='http://example.com/sample-bis.ics'
         )
-        assert agenda.is_available_for_simple_management() is False
+        check_is_available(False)
         source.delete()
-        assert agenda.is_available_for_simple_management() is True
+        check_is_available(True)
     source2.ics_url = 'http://example.com/sample-bis.ics'
     source2.save()
-    assert agenda.is_available_for_simple_management() is False
+    check_is_available(False)
     source2.ics_url = 'http://example.com/sample.ics'
     source2.save()
-    assert agenda.is_available_for_simple_management() is True
+    check_is_available(True)
 
     # changes on sources - from file
     for _desk in [desk, desk2]:
@@ -333,23 +340,23 @@ def test_agenda_is_available_for_simple_management(settings):
             ics_filename='sample-bis.ics',
             ics_file=ContentFile(ICS_SAMPLE, name='sample-bis.ics'),
         )
-        assert agenda.is_available_for_simple_management() is False
+        check_is_available(False)
         source.delete()
-        assert agenda.is_available_for_simple_management() is True
+        check_is_available(True)
     source3.ics_filename = 'sample-bis.ics'
     source3.save()
-    assert agenda.is_available_for_simple_management() is False
+    check_is_available(False)
     source3.ics_filename = 'sample.ics'
     source3.save()
-    assert agenda.is_available_for_simple_management() is True
+    check_is_available(True)
     # ics_file content is not checked
 
     # changes on unavailability calendars
     for _desk in [desk, desk2]:
         unavailability_calendar2.desks.add(_desk)
-        assert agenda.is_available_for_simple_management() is False
+        check_is_available(False)
         unavailability_calendar2.desks.remove(_desk)
-        assert agenda.is_available_for_simple_management() is True
+        check_is_available(True)
 
 
 def test_event_slug():
diff --git a/tests/test_manager.py b/tests/test_manager.py
index 83c5394..100e968 100644
--- a/tests/test_manager.py
+++ b/tests/test_manager.py
@@ -1043,42 +1043,73 @@ def test_options_meetings_agenda_num_queries(app, admin_user, managers_group):
     )
     resource = Resource.objects.create(label='Resource')
     agenda.resources.add(resource)
+
     for i in range(0, 10):
         MeetingType.objects.create(agenda=agenda, label='MT %s' % i)
-        desk = Desk.objects.create(agenda=agenda, label='Desk %s' % i)
-        source = TimePeriodExceptionSource.objects.create(desk=desk, ics_url='https://example.com/test.ics')
-        calendar = UnavailabilityCalendar.objects.create(label='foo')
-        calendar.desks.add(desk)
-        for weekday in (0, 6):
-            TimePeriod.objects.create(
-                weekday=weekday, desk=desk, start_time=datetime.time(10, 0), end_time=datetime.time(12, 0)
-            )
-        # exception starts and ends in the past
-        TimePeriodException.objects.create(
-            desk=desk,
-            start_datetime=now() - datetime.timedelta(days=2),
-            end_datetime=now() - datetime.timedelta(days=1),
+
+    desk = Desk.objects.create(agenda=agenda, label='Desk')
+    source = TimePeriodExceptionSource.objects.create(desk=desk, ics_url='https://example.com/test.ics')
+    calendar = UnavailabilityCalendar.objects.create(label='foo')
+    calendar.desks.add(desk)
+    for weekday in (0, 6):
+        TimePeriod.objects.create(
+            weekday=weekday, desk=desk, start_time=datetime.time(10, 0), end_time=datetime.time(12, 0)
         )
-        if i % 2:
-            # exception starts in the past but ends in the futur
-            TimePeriodException.objects.create(
-                desk=desk,
-                source=source,
-                start_datetime=now() - datetime.timedelta(days=1),
-                end_datetime=now() + datetime.timedelta(days=1),
-            )
-        else:
-            # exception in more than 2 weeks
-            TimePeriodException.objects.create(
-                unavailability_calendar=calendar,
-                start_datetime=now() + datetime.timedelta(days=20),
-                end_datetime=now() + datetime.timedelta(days=21),
-            )
+
+    desk2 = desk.duplicate()
+    assert agenda.is_available_for_simple_management() is True
 
     app = login(app)
     with CaptureQueriesContext(connection) as ctx:
         app.get('/manage/agendas/%s/settings' % agenda.pk)
-        assert len(ctx.captured_queries) == 19
+        assert len(ctx.captured_queries) == 13
+
+    # check with different kind of exceptions:
+
+    # exception starts and ends in the past
+    exception = TimePeriodException.objects.create(
+        desk=desk,
+        start_datetime=now() - datetime.timedelta(days=2),
+        end_datetime=now() - datetime.timedelta(days=1),
+    )
+    desk2.delete()
+    desk2 = desk.duplicate()
+    assert agenda.is_available_for_simple_management() is True
+
+    with CaptureQueriesContext(connection) as ctx:
+        app.get('/manage/agendas/%s/settings' % agenda.pk)
+        assert len(ctx.captured_queries) == 13
+
+    # exception starts in the past but ends in the futur
+    exception.delete()
+    exception = TimePeriodException.objects.create(
+        desk=desk,
+        source=source,
+        start_datetime=now() - datetime.timedelta(days=1),
+        end_datetime=now() + datetime.timedelta(days=1),
+    )
+    desk2.delete()
+    desk2 = desk.duplicate()
+    assert agenda.is_available_for_simple_management() is True
+
+    with CaptureQueriesContext(connection) as ctx:
+        app.get('/manage/agendas/%s/settings' % agenda.pk)
+        assert len(ctx.captured_queries) == 13
+
+    # exception in more than 2 weeks
+    exception.delete()
+    exception = TimePeriodException.objects.create(
+        unavailability_calendar=calendar,
+        start_datetime=now() + datetime.timedelta(days=20),
+        end_datetime=now() + datetime.timedelta(days=21),
+    )
+    desk2.delete()
+    desk2 = desk.duplicate()
+    assert agenda.is_available_for_simple_management() is True
+
+    with CaptureQueriesContext(connection) as ctx:
+        app.get('/manage/agendas/%s/settings' % agenda.pk)
+        assert len(ctx.captured_queries) == 13
 
 
 @mock.patch('chrono.agendas.models.Agenda.is_available_for_simple_management')
-- 
2.29.2

