cells: exclude cells with inactive placeholders from invalid report

(#40252)

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:

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()

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,
     }

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

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)