From c620f5f1f85bf0ba8b2286c5c78577d9fc1c456e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Laur=C3=A9line=20Gu=C3=A9rin?= <zebuline@entrouvert.com>
Date: Fri, 27 Mar 2020 15:18:21 +0100
Subject: [PATCH 3/7] cells: exclude cells with inactive placeholders from
 invalid report (#40252)

---
 combo/apps/assets/templatetags/assets.py |  8 ++++++--
 combo/data/models.py                     | 14 ++++++++++++++
 combo/manager/views.py                   | 10 +++++++++-
 combo/public/templatetags/combo.py       |  4 ++++
 tests/test_manager.py                    | 19 +++++++++++++++++++
 5 files changed, 52 insertions(+), 3 deletions(-)

diff --git a/combo/apps/assets/templatetags/assets.py b/combo/apps/assets/templatetags/assets.py
index 232d976..52496b0 100644
--- a/combo/apps/assets/templatetags/assets.py
+++ b/combo/apps/assets/templatetags/assets.py
@@ -72,8 +72,12 @@ def asset_css_url(*args, **kwargs):
         return 'none'
 
 
-@register.simple_tag
-def get_asset(*args, **kwargs):
+@register.simple_tag(takes_context=True)
+def get_asset(context, *args, **kwargs):
+    if context.get('traverse_cells'):
+        # assets are not required when we are just searching for page placeholders
+        return None
+
     key = None
     if 'cell' in kwargs and 'type' in kwargs:
         try:
diff --git a/combo/data/models.py b/combo/data/models.py
index 23ce71b..154e304 100644
--- a/combo/data/models.py
+++ b/combo/data/models.py
@@ -859,6 +859,20 @@ class CellBase(six.with_metaclass(CellMeta, models.Model)):
         return self.invalid_reason_codes.get(
             validity_info.invalid_reason_code, validity_info.invalid_reason_code)
 
+    def is_placeholder_active(self):
+        if not self.placeholder:
+            return False
+        if self.placeholder.startswith('_'):
+            return True
+
+        request = RequestFactory().get('/')
+        if not hasattr(self.page, '_placeholders'):
+            self.page._placeholders = self.page.get_placeholders(request, traverse_cells=True)
+        for placeholder in self.page._placeholders:
+            if placeholder.key == self.placeholder:
+                return True
+        return False
+
     def is_visible(self, user=None, check_validity_info=True):
         if check_validity_info:
             validity_info = self.get_validity_info()
diff --git a/combo/manager/views.py b/combo/manager/views.py
index 4d1a752..7d9fb3a 100644
--- a/combo/manager/views.py
+++ b/combo/manager/views.py
@@ -95,12 +95,20 @@ site_import = SiteImportView.as_view()
 
 def invalid_cell_report(request):
     invalid_cells = CellBase.get_cells(
-        select_related={'__all__': ['page']},
         page__snapshot__isnull=True,
         validity_info__invalid_since__isnull=False,
         load_contenttypes=True)
+    # manual prefetch of cell pages (for ordering and placeholders)
+    all_pages_by_pk = {p.pk: p for p in Page.objects.filter(snapshot__isnull=True)}
+    for cell in invalid_cells:
+        cell.page = all_pages_by_pk.get(cell.page_id)
+    # exclude some cells on placeholder naming
     invalid_cells = [c for c in invalid_cells if c.placeholder and not c.placeholder.startswith('_')]
+    # exclude cells with an inactive placeholder
+    invalid_cells = [c for c in invalid_cells if c.is_placeholder_active()]
+    # sort cells
     invalid_cells.sort(key=attrgetter('page.order', 'page.pk', 'order'))
+
     context = {
         'object_list': invalid_cells,
     }
diff --git a/combo/public/templatetags/combo.py b/combo/public/templatetags/combo.py
index f392cbe..ba17005 100644
--- a/combo/public/templatetags/combo.py
+++ b/combo/public/templatetags/combo.py
@@ -70,9 +70,13 @@ def placeholder(context, placeholder_name, **options):
         # ajax call
         page_cells = context.get('page_cells')
     elif 'page' in context and hasattr(context['page'], 'prefetched_cells'):
+        # sometimes cells are prefetched; use them
         page_cells = context['page'].prefetched_cells
     elif not context.get('render_skeleton'):
         page_cells = context['page'].get_cells() if 'page' in context else []
+        if 'page' in context:
+            # store cells for later use
+            context['page'].prefetched_cells = page_cells
     context['cells'] = [
         x for x in page_cells if
         x.placeholder == placeholder_name and
diff --git a/tests/test_manager.py b/tests/test_manager.py
index a09ae27..20ea921 100644
--- a/tests/test_manager.py
+++ b/tests/test_manager.py
@@ -13,6 +13,7 @@ from django.contrib.auth.models import User, Group
 from django.db import connection
 from django.template import TemplateSyntaxError
 from django.test import override_settings
+from django.test.client import RequestFactory
 from django.test.utils import CaptureQueriesContext
 from django.utils.http import urlencode
 from django.utils.six import BytesIO
@@ -627,6 +628,24 @@ def test_invalid_cell_report(app, admin_user):
     resp = app.get('/manage/cells/invalid-report/')
     assert resp.context['object_list'] == [cell]
 
+    # cells without placeholder are not reported
+    cell.placeholder = ''
+    cell.save()
+    resp = app.get('/manage/cells/invalid-report/')
+    assert resp.context['object_list'] == []
+
+    # cells with unknown placeholder are not reported
+    request = RequestFactory().get('/')
+    assert [p.key for p in page.get_placeholders(request, traverse_cells=True)] == ['content', 'footer']
+    cell.placeholder = 'foobar'
+    cell.save()
+    resp = app.get('/manage/cells/invalid-report/')
+    assert resp.context['object_list'] == []
+    cell.placeholder = 'footer'
+    cell.save()
+    resp = app.get('/manage/cells/invalid-report/')
+    assert resp.context['object_list'] == [cell]
+
 
 def test_duplicate_page(app, admin_user):
     page = Page.objects.create(title='One', slug='one', template_name='standard', exclude_from_navigation=False)
-- 
2.20.1

