From 0f53f630d9137587e808c199735b6a663f0b1b80 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Laur=C3=A9line=20Gu=C3=A9rin?= <zebuline@entrouvert.com>
Date: Tue, 11 Feb 2020 21:26:12 +0100
Subject: [PATCH 2/2] misc: reduce querysets on page edition page (#39761)

---
 combo/apps/lingo/models.py                    | 24 ++++++++++---------
 combo/data/models.py                          |  3 ++-
 combo/manager/templatetags/cells.py           |  2 +-
 combo/manager/views.py                        |  3 ++-
 combo/public/templates/combo/menu.html        |  2 +-
 combo/public/templates/combo/placeholder.html |  2 +-
 combo/public/templatetags/combo.py            |  2 ++
 tests/test_manager.py                         | 13 ++++++----
 8 files changed, 30 insertions(+), 21 deletions(-)

diff --git a/combo/apps/lingo/models.py b/combo/apps/lingo/models.py
index e0f766c..49240aa 100644
--- a/combo/apps/lingo/models.py
+++ b/combo/apps/lingo/models.py
@@ -148,7 +148,7 @@ class Regie(models.Model):
             if self.pk:
                 qs = qs.exclude(pk=self.pk)
             qs.update(is_default=False)
-        elif self.__class__.objects.filter(is_default=True).count() == 0:
+        elif not self.__class__.objects.filter(is_default=True).exists():
             self.is_default = True
         super(Regie, self).save(*args, **kwargs)
 
@@ -538,12 +538,12 @@ class LingoBasketCell(CellBase):
 
     @classmethod
     def is_enabled(cls):
-        return Regie.objects.count() > 0
+        return Regie.objects.exists()
 
     def is_relevant(self, context):
         if not (getattr(context['request'], 'user', None) and context['request'].user.is_authenticated):
             return False
-        return BasketItem.get_items_to_be_paid(context['request'].user).count() > 0
+        return BasketItem.get_items_to_be_paid(context['request'].user).exists()
 
     def get_badge(self, context):
         if not (getattr(context['request'], 'user', None) and context['request'].user.is_authenticated):
@@ -581,15 +581,16 @@ class LingoRecentTransactionsCell(CellBase):
 
     @classmethod
     def is_enabled(cls):
-        return Regie.objects.count() > 0
+        return Regie.objects.exists()
 
     def is_relevant(self, context):
         if not (getattr(context['request'], 'user', None) and context['request'].user.is_authenticated):
             return False
-        transactions = Transaction.objects.filter(
+        return (
+            Transaction.objects.filter(
                 user=context['request'].user,
                 start_date__gte=timezone.now()-datetime.timedelta(days=7))
-        return len(transactions) > 0
+            .exists())
 
     def render(self, context):
         recent_transactions_template = template.loader.get_template(
@@ -610,12 +611,12 @@ class LingoBasketLinkCell(CellBase):
 
     @classmethod
     def is_enabled(cls):
-        return Regie.objects.count() > 0
+        return Regie.objects.exists()
 
     def is_relevant(self, context):
         if not (getattr(context['request'], 'user', None) and context['request'].user.is_authenticated):
             return False
-        return BasketItem.get_items_to_be_paid(context['request'].user).count() > 0
+        return BasketItem.get_items_to_be_paid(context['request'].user).exists()
 
     def render(self, context):
         if not (getattr(context['request'], 'user', None) and context['request'].user.is_authenticated):
@@ -648,7 +649,7 @@ class Items(CellBase):
 
     @classmethod
     def is_enabled(cls):
-        return Regie.objects.exclude(webservice_url='').count() > 0
+        return Regie.objects.exclude(webservice_url='').exists()
 
     def is_relevant(self, context):
         return (getattr(context['request'], 'user', None) and context['request'].user.is_authenticated)
@@ -656,9 +657,10 @@ class Items(CellBase):
     def get_default_form_class(self):
         fields = ['title', 'text']
         widgets = {}
-        if Regie.objects.exclude(webservice_url='').count() > 1:
+        regie_qs = Regie.objects.exclude(webservice_url='')
+        if len(regie_qs) > 1:
             regies = [('', _('All'))]
-            regies.extend([(r.slug, r.label) for r in Regie.objects.exclude(webservice_url='')])
+            regies.extend([(r.slug, r.label) for r in regie_qs])
             widgets['regie'] = Select(choices=regies)
             fields.insert(0, 'regie')
         return model_forms.modelform_factory(self.__class__, fields=fields, widgets=widgets)
diff --git a/combo/data/models.py b/combo/data/models.py
index 429b1cd..e0c5a91 100644
--- a/combo/data/models.py
+++ b/combo/data/models.py
@@ -316,7 +316,8 @@ class Page(models.Model):
             parenting[page.parent_id].append(page)
         def fill_list(object_sublist, level=0, parent=None):
             for page in object_sublist:
-                if page.parent == parent:
+                parent_id = parent.pk if parent else None
+                if page.parent_id == parent_id:
                     page.level = level
                     reordered.append(page)
                     if page.id in parenting:
diff --git a/combo/manager/templatetags/cells.py b/combo/manager/templatetags/cells.py
index 3f95a6d..59812d4 100644
--- a/combo/manager/templatetags/cells.py
+++ b/combo/manager/templatetags/cells.py
@@ -22,7 +22,7 @@ register = template.Library()
 @register.simple_tag(takes_context=True)
 def cell_form(context, cell):
     context['url'] = reverse('combo-manager-page-edit-cell', kwargs={
-        'page_pk': cell.page.id, 'cell_reference': cell.get_reference()})
+        'page_pk': cell.page_id, 'cell_reference': cell.get_reference()})
     form_class = cell.get_default_form_class()
     if form_class:
         context['form'] = form_class(instance=cell, prefix='c%s' % cell.get_reference())
diff --git a/combo/manager/views.py b/combo/manager/views.py
index 28963bd..98a6c6b 100644
--- a/combo/manager/views.py
+++ b/combo/manager/views.py
@@ -261,7 +261,8 @@ class PageView(DetailView):
         context['cell_type_groups'] = list(cell_type_groups.items())
         context['cell_type_groups'].sort(key=lambda x: x[0])
 
-        cells = CellBase.get_cells(page_id=self.object.id)
+        cells = CellBase.get_cells(page=self.object)
+        self.object.prefeteched_cells = cells
         template = self.object.template_name
         placeholders = []
         optional_placeholders = []
diff --git a/combo/public/templates/combo/menu.html b/combo/public/templates/combo/menu.html
index 8a8b241..213db5f 100644
--- a/combo/public/templates/combo/menu.html
+++ b/combo/public/templates/combo/menu.html
@@ -2,7 +2,7 @@
 <ul>
 {% spaceless %}
 {% for menuitem in menuitems %}
-<li data-menu-page-id="{{ menuitem.page.id }}" class="menu-{{ menuitem.page.slug }} {% if menuitem.selected %}selected{% endif %}"><a
+<li data-menu-page-id="{{ menuitem.page_id }}" class="menu-{{ menuitem.page.slug }} {% if menuitem.selected %}selected{% endif %}"><a
     href="{% page_absolute_url menuitem.page %}"><span>{{ menuitem.page.title }}</span></a>
   {% if depth > 1 %}
     {% show_menu current_page=menuitem.page level=-1 depth=depth reduce_depth=True ignore_visibility=ignore_visibility %}
diff --git a/combo/public/templates/combo/placeholder.html b/combo/public/templates/combo/placeholder.html
index 4ec2369..8f86b73 100644
--- a/combo/public/templates/combo/placeholder.html
+++ b/combo/public/templates/combo/placeholder.html
@@ -6,7 +6,7 @@
 {% for cell in cells %}
 <div class="cell {{ cell.css_class_names }} {% if cell.slug %}{{cell.slug}}{% endif %} {% if cell|shown_because_admin:request %}shown-because-admin{% endif %}"
      {% if cell.slug and cell.use_slug_as_id %}id="{{ cell.slug }}"{% endif %}
-     data-ajax-cell-url="{{ site_base }}{% url 'combo-public-ajax-page-cell' page_pk=cell.page.id cell_reference=cell.get_reference %}"
+     data-ajax-cell-url="{{ site_base }}{% url 'combo-public-ajax-page-cell' page_pk=cell.page_id cell_reference=cell.get_reference %}"
      data-ajax-cell-loading-message="{{ cell.loading_message }}"
      data-ajax-cell-error-message="{% trans "Loading error" %}"
      {% if cell.ajax_refresh %}data-ajax-cell-refresh="{{ cell.ajax_refresh }}"{% endif %}
diff --git a/combo/public/templatetags/combo.py b/combo/public/templatetags/combo.py
index cf491d6..683a3d6 100644
--- a/combo/public/templatetags/combo.py
+++ b/combo/public/templatetags/combo.py
@@ -64,6 +64,8 @@ def placeholder(context, placeholder_name, **options):
         # page cells are not precomputed when rendering a single cell in an
         # ajax call
         page_cells = context.get('page_cells')
+    elif 'page' in context and hasattr(context['page'], 'prefeteched_cells'):
+        page_cells = context['page'].prefeteched_cells
     elif not context.get('render_skeleton'):
         page_cells = context['page'].get_cells() if 'page' in context else []
     context['cells'] = [x for x in page_cells if
diff --git a/tests/test_manager.py b/tests/test_manager.py
index c645166..982a829 100644
--- a/tests/test_manager.py
+++ b/tests/test_manager.py
@@ -334,9 +334,10 @@ def test_edit_page_num_queries(app, admin_user):
         )
 
     app = login(app)
+    app.get('/manage/pages/%s/' % page.pk)  # load once to populate caches
     with CaptureQueriesContext(connection) as ctx:
         app.get('/manage/pages/%s/' % page.pk)
-        assert len(ctx.captured_queries) == 89
+        assert len(ctx.captured_queries) == 29
 
 
 def test_delete_page(app, admin_user):
@@ -471,12 +472,13 @@ def test_export_page(app, admin_user):
     assert resp.headers['content-type'] == 'application/json'
     assert resp.json['pages'][0].get('fields').get('slug') == 'one'
 
+
 def test_export_page_order():
     Page.objects.all().delete()
-    page1 = Page(title='One', slug='one', template_name='standard')
-    page2 = Page(title='Two', slug='two', parent=page1, template_name='standard')
-    page3 = Page(title='Three', slug='three', parent=page2, template_name='standard')
-    page4 = Page(title='Four', slug='four', parent=page1, template_name='standard')
+    page1 = Page.objects.create(title='One', slug='one', template_name='standard')
+    page2 = Page.objects.create(title='Two', slug='two', parent=page1, template_name='standard')
+    page3 = Page.objects.create(title='Three', slug='three', parent=page2, template_name='standard')
+    page4 = Page.objects.create(title='Four', slug='four', parent=page1, template_name='standard')
     random_list = [page3, page4, page1, page2]
     ordered_list = Page.get_as_reordered_flat_hierarchy(random_list)
     assert ordered_list[0] == page1
@@ -484,6 +486,7 @@ def test_export_page_order():
     assert ordered_list[2] == page2
     assert ordered_list[3] == page3
 
+
 def test_site_export_import(app, admin_user):
     Page.objects.all().delete()
     page1 = Page(title='One', slug='one', template_name='standard')
-- 
2.20.1

