From 390771854366d759964b2f881ce46106c2f2fbb9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Laur=C3=A9line=20Gu=C3=A9rin?= <zebuline@entrouvert.com>
Date: Tue, 10 Mar 2020 09:52:41 +0100
Subject: [PATCH 1/4] assets: add generic assets for cells (#40223)

---
 combo/apps/assets/templatetags/assets.py      | 17 ++++-
 combo/apps/assets/views.py                    |  6 +-
 combo/data/models.py                          | 63 +++++++++++++++++--
 combo/manager/views.py                        |  2 +-
 .../combo/asset_picture_fragment.html         |  7 +++
 combo/public/templates/combo/feed-cell.html   |  1 +
 combo/public/templates/combo/json-cell.html   |  1 +
 .../templates/combo/json-list-cell.html       |  1 +
 combo/public/templates/combo/link-cell.html   |  1 +
 .../templates/combo/link-list-cell.html       |  1 +
 combo/public/templates/combo/text-cell.html   |  1 +
 combo/public/views.py                         |  6 +-
 combo/settings.py                             | 28 +++++++++
 tests/test_cells.py                           | 62 ++++++++++++++++++
 tests/test_manager.py                         |  3 +-
 tests/test_public_templatetags.py             | 29 ++++++++-
 16 files changed, 218 insertions(+), 11 deletions(-)
 create mode 100644 combo/public/templates/combo/asset_picture_fragment.html

diff --git a/combo/apps/assets/templatetags/assets.py b/combo/apps/assets/templatetags/assets.py
index 5d7d569..232d976 100644
--- a/combo/apps/assets/templatetags/assets.py
+++ b/combo/apps/assets/templatetags/assets.py
@@ -62,6 +62,7 @@ def asset_url(*args, **kwargs):
 
     return get_thumbnail(asset, geometry_string, **kwargs).url
 
+
 @register.simple_tag
 def asset_css_url(*args, **kwargs):
     url = asset_url(*args, **kwargs)
@@ -72,7 +73,21 @@ def asset_css_url(*args, **kwargs):
 
 
 @register.simple_tag
-def get_asset(key):
+def get_asset(*args, **kwargs):
+    key = None
+    if 'cell' in kwargs and 'type' in kwargs:
+        try:
+            if not kwargs['cell'].can_have_assets():
+                return None
+            key = kwargs['cell'].get_asset_slot_key(kwargs['type'])
+        except AttributeError:
+            return None
+    elif len(args) == 1:
+        key = args[0]
+
+    if not key:
+        return None
+
     try:
         return Asset.objects.get(key=key)
     except Asset.DoesNotExist:
diff --git a/combo/apps/assets/views.py b/combo/apps/assets/views.py
index 8b875dd..f9005b5 100644
--- a/combo/apps/assets/views.py
+++ b/combo/apps/assets/views.py
@@ -103,8 +103,10 @@ class SlotAsset(object):
         assets = dict([(x.key, x) for x in Asset.objects.all()])
         uniq_slots = {}
         uniq_slots.update(settings.COMBO_ASSET_SLOTS)
-        for cell in CellBase.get_cells(
-                cell_filter=lambda x: bool(x.get_asset_slots)):
+        cells = CellBase.get_cells(select_related={
+            '__all__': ['page'],
+            'data_linkcell': ['link_page']})
+        for cell in cells:
             uniq_slots.update(cell.get_asset_slots())
         for key, value in uniq_slots.items():
             yield cls(key,
diff --git a/combo/data/models.py b/combo/data/models.py
index 6f53f35..8a2dea6 100644
--- a/combo/data/models.py
+++ b/combo/data/models.py
@@ -577,9 +577,6 @@ class CellBase(six.with_metaclass(CellMeta, models.Model)):
     # get_badge(self, context); set to None so cell types can be skipped easily
     get_badge = None
 
-    # get_asset_slots(self); set to None so cell types can be skipped easily
-    get_asset_slots = None
-
     # message displayed when the cell is loaded asynchronously
     loading_message = _('Loading...')
 
@@ -623,6 +620,50 @@ class CellBase(six.with_metaclass(CellMeta, models.Model)):
     def css_class_names(self):
         return ' '.join([self.class_name, self.legacy_class_name, self.extra_css_class])
 
+    def can_have_assets(self):
+        return self.get_slug_for_asset() and self.get_asset_slot_templates()
+
+    def get_slug_for_asset(self):
+        return self.slug
+
+    def get_label_for_asset(self):
+        return _(u'%(cell_label)s on page %(page_name)s (%(page_slug)s)') % {
+            'cell_label': str(self),
+            'page_name': str(self.page),
+            'page_slug': self.page.slug,
+        }
+
+    def get_asset_slot_key(self, key):
+        return 'cell:%s:%s:%s' % (
+            self.get_cell_type_str(),
+            key,
+            self.get_slug_for_asset())
+
+    def get_asset_slot_templates(self):
+        return settings.COMBO_CELL_ASSET_SLOTS.get(self.get_cell_type_str()) or {}
+
+    def get_asset_slots(self):
+        if not self.can_have_assets():
+            return {}
+
+        slot_templates = self.get_asset_slot_templates()
+        slots = {}
+        for slot_template_key, slot_template_data in slot_templates.items():
+            suffix = ''
+            if slot_template_data.get('suffix'):
+                suffix = ' (%s)' % slot_template_data['suffix']
+            slot_key = self.get_asset_slot_key(slot_template_key)
+            label = u'%(prefix)s — %(label)s%(suffix)s' % {
+                'prefix': slot_template_data['prefix'],
+                'label': self.get_label_for_asset(),
+                'suffix': suffix
+            }
+            slots[slot_key] = {
+                'label': label
+            }
+            slots[slot_key].update(slot_template_data)
+        return slots
+
     @classmethod
     def get_cell_classes(cls, class_filter=lambda x: True):
         for klass in get_cell_classes():
@@ -673,7 +714,9 @@ class CellBase(six.with_metaclass(CellMeta, models.Model)):
                 continue
             cells_queryset = klass.objects.filter(**kwargs)
             if select_related:
-                cells_queryset = cells_queryset.select_related(*select_related)
+                cells_queryset = cells_queryset.select_related(
+                    *select_related.get('__all__', []),
+                    *select_related.get(klass.get_cell_type_str(), []))
             cells.extend(cells_queryset)
         if prefetch_validity_info:
             validity_info_list = list(ValidityInfo.objects.select_related('content_type'))
@@ -1059,6 +1102,18 @@ class LinkCell(CellBase):
             return None
         return utils.ellipsize(title)
 
+    def get_slug_for_asset(self):
+        if self.placeholder and self.placeholder.startswith('_'):
+            return
+        if self.link_page:
+            return self.link_page.slug
+        return self.slug
+
+    def get_label_for_asset(self):
+        if self.link_page:
+            return str(self)
+        return super().get_label_for_asset()
+
     def get_url(self, context=None):
         context = context or {}
         if self.link_page:
diff --git a/combo/manager/views.py b/combo/manager/views.py
index b4e2687..00898f1 100644
--- a/combo/manager/views.py
+++ b/combo/manager/views.py
@@ -95,7 +95,7 @@ site_import = SiteImportView.as_view()
 
 def invalid_cell_report(request):
     invalid_cells = CellBase.get_cells(
-        select_related=['page'],
+        select_related={'__all__': ['page']},
         page__snapshot__isnull=True,
         validity_info__invalid_since__isnull=False)
     invalid_cells = [c for c in invalid_cells if c.placeholder and not c.placeholder.startswith('_')]
diff --git a/combo/public/templates/combo/asset_picture_fragment.html b/combo/public/templates/combo/asset_picture_fragment.html
new file mode 100644
index 0000000..95930bd
--- /dev/null
+++ b/combo/public/templates/combo/asset_picture_fragment.html
@@ -0,0 +1,7 @@
+{% load assets %}
+{% get_asset cell=cell type='picture' as asset %}
+{% if asset %}
+  <picture>
+  <img src="{% asset_url asset size="660x360" crop="center" upscale=False %}" alt="">
+  </picture>
+{% endif %}
diff --git a/combo/public/templates/combo/feed-cell.html b/combo/public/templates/combo/feed-cell.html
index 2a1724b..1400718 100644
--- a/combo/public/templates/combo/feed-cell.html
+++ b/combo/public/templates/combo/feed-cell.html
@@ -3,6 +3,7 @@
 {% if cell.title %}
 <h2>{{ cell.title }}</h2>
 {% endif %}
+{% include "combo/asset_picture_fragment.html" %}
 <div class="feed-content">
 {% for entry in feed.entries %}
 {% if entry.link %}
diff --git a/combo/public/templates/combo/json-cell.html b/combo/public/templates/combo/json-cell.html
index f72574f..c1082a4 100644
--- a/combo/public/templates/combo/json-cell.html
+++ b/combo/public/templates/combo/json-cell.html
@@ -1,5 +1,6 @@
 {% block cell-content %}
 {% if title %}<h2>{{title}}</h2>{% endif %}
+{% include "combo/asset_picture_fragment.html" %}
 <!--
 {{json}}
 -->
diff --git a/combo/public/templates/combo/json-list-cell.html b/combo/public/templates/combo/json-list-cell.html
index a2890ff..b24ae53 100644
--- a/combo/public/templates/combo/json-list-cell.html
+++ b/combo/public/templates/combo/json-list-cell.html
@@ -1,6 +1,7 @@
 {% block cell-content %}
 <div class="links-list">
 {% if title %}<h2>{{title}}</h2>{% endif %}
+{% include "combo/asset_picture_fragment.html" %}
 {% for row in json.data %}
 <ul>
 <li><a href="{{row.url}}">{{row.text}}</a></li>
diff --git a/combo/public/templates/combo/link-cell.html b/combo/public/templates/combo/link-cell.html
index 2969438..f8c4fa1 100644
--- a/combo/public/templates/combo/link-cell.html
+++ b/combo/public/templates/combo/link-cell.html
@@ -1,3 +1,4 @@
 {% block cell-content %}
+{% include "combo/asset_picture_fragment.html" %}
 <a href="{{url}}">{{title}}</a>
 {% endblock %}
diff --git a/combo/public/templates/combo/link-list-cell.html b/combo/public/templates/combo/link-list-cell.html
index 706d730..38dd02c 100644
--- a/combo/public/templates/combo/link-list-cell.html
+++ b/combo/public/templates/combo/link-list-cell.html
@@ -2,6 +2,7 @@
 {% spaceless %}
 <div class="links-list">
   {% if title %}<h2>{{title}}</h2>{% endif %}
+  {% include "combo/asset_picture_fragment.html" %}
   <ul>
   {% for link in links %}
     <li><a href="{{ link.url }}">{{ link.title }}</a></li>
diff --git a/combo/public/templates/combo/text-cell.html b/combo/public/templates/combo/text-cell.html
index 9a97ea5..846995a 100644
--- a/combo/public/templates/combo/text-cell.html
+++ b/combo/public/templates/combo/text-cell.html
@@ -1,3 +1,4 @@
 {% block cell-content %}
+{% include "combo/asset_picture_fragment.html" %}
 {{text}}
 {% endblock %}
diff --git a/combo/public/views.py b/combo/public/views.py
index 443d48a..4a0c0c9 100644
--- a/combo/public/views.py
+++ b/combo/public/views.py
@@ -499,6 +499,7 @@ def page(request):
 
     return publish_page(request, page)
 
+
 def publish_page(request, page, status=200, template_name=None):
     pages = page.get_parents_and_self()
 
@@ -518,7 +519,10 @@ def publish_page(request, page, status=200, template_name=None):
         if redirect_url:
             return HttpResponseRedirect(redirect_url)
 
-    cells = CellBase.get_cells(page=page, prefetch_validity_info=True)
+    cells = CellBase.get_cells(
+        page=page,
+        select_related={'data_linkcell': ['link_page']},
+        prefetch_validity_info=True)
     extend_with_parent_cells(cells, hierarchy=pages)
     cells = [x for x in cells if x.is_visible(user=request.user)]
     mark_duplicated_slugs(cells)
diff --git a/combo/settings.py b/combo/settings.py
index 8b472a9..e998799 100644
--- a/combo/settings.py
+++ b/combo/settings.py
@@ -324,6 +324,34 @@ WCS_CATEGORY_ASSET_SLOTS = {
 
 WCS_FORM_ASSET_SLOTS = {}
 
+COMBO_CELL_ASSET_SLOTS = {
+    'data_feedcell': {
+        'picture': {
+            'prefix': _('Picture'),
+        },
+    },
+    'data_jsoncell': {
+        'picture': {
+            'prefix': _('Picture'),
+        },
+    },
+    'data_linkcell': {
+        'picture': {
+            'prefix': _('Picture'),
+        },
+    },
+    'data_linklistcell': {
+        'picture': {
+            'prefix': _('Picture'),
+        },
+    },
+    'data_textcell': {
+        'picture': {
+            'prefix': _('Picture'),
+        },
+    },
+}
+
 # known services
 KNOWN_SERVICES = {}
 
diff --git a/tests/test_cells.py b/tests/test_cells.py
index edbeacd..ca4241f 100644
--- a/tests/test_cells.py
+++ b/tests/test_cells.py
@@ -1046,3 +1046,65 @@ def test_hourly():
             assert check_validity.call_args_list == [mock.call()]
         else:
             assert hasattr(klass, 'check_validity') is False
+
+
+def test_cell_assets(settings, app, admin_user):
+    page = Page.objects.create(title='xxx', slug='test_cell_assets', template_name='standard')
+    text_cell = TextCell.objects.create(page=page, order=0, slug='foo')
+    list_cell = LinkListCell.objects.create(page=page, order=2, slug='bar')
+    item = LinkCell.objects.create(
+        page=page,
+        placeholder=list_cell.link_placeholder,
+        title='Example Site',
+        link_page=page,
+        order=1,
+    )
+
+    app = login(app)
+    settings.COMBO_CELL_ASSET_SLOTS = {}
+    resp = app.get('/manage/assets/')
+    assert 'have any asset yet.' in resp.text
+
+    # only text cells are defined for assets
+    settings.COMBO_CELL_ASSET_SLOTS = {'data_textcell': {'picture': {'prefix': 'Picture'}}}
+    resp = app.get('/manage/assets/')
+    assert u'Picture — %s' % text_cell.get_label_for_asset() in resp.text
+    assert u'Picture — %s' % list_cell.get_label_for_asset() not in resp.text
+    assert u'Picture — %s' % item.get_label_for_asset() not in resp.text
+
+    # text and list link cells are defined for assets
+    settings.COMBO_CELL_ASSET_SLOTS = {
+        'data_textcell': {'picture': {'prefix': 'Picture'}},
+        'data_linklistcell': {'picture': {'prefix': 'Picture'}},
+        'data_linkcell': {'picture': {'prefix': 'Picture', 'suffix': 'test'}},
+    }
+    resp = app.get('/manage/assets/')
+    assert u'Picture — %s' % text_cell.get_label_for_asset() in resp.text
+    assert u'Picture — %s' % list_cell.get_label_for_asset() in resp.text
+    # but items are excluded
+    assert item.get_slug_for_asset() is None  # slug for asset is always None for items
+    assert u'Picture — %s' % item.get_label_for_asset() not in resp.text
+
+    # test slugs
+    link_cell = LinkCell.objects.create(
+        page=page,
+        url='http://example.net/',
+        order=1,
+    )
+    resp = app.get('/manage/assets/')
+    assert link_cell.get_slug_for_asset() == ''
+    assert u'Picture — %s' % link_cell.get_label_for_asset() not in resp.text
+
+    link_cell.slug = 'foo'
+    link_cell.save()
+    resp = app.get('/manage/assets/')
+    assert link_cell.get_slug_for_asset() == 'foo'
+    assert u'Picture — %s (test)' % link_cell.get_label_for_asset() in resp.text
+
+    link_cell.slug = ''
+    link_cell.url = ''
+    link_cell.link_page = page
+    link_cell.save()
+    resp = app.get('/manage/assets/')
+    assert link_cell.get_slug_for_asset() == 'test_cell_assets'
+    assert u'Picture — %s (test)' % link_cell.get_label_for_asset() in resp.text
diff --git a/tests/test_manager.py b/tests/test_manager.py
index 0d0f8ec..0a76ba0 100644
--- a/tests/test_manager.py
+++ b/tests/test_manager.py
@@ -352,7 +352,8 @@ def test_edit_page_navigation(app, admin_user):
     assert '<li class="nav-right"' not in resp.text
 
 
-def test_edit_page_num_queries(app, admin_user):
+def test_edit_page_num_queries(settings, app, admin_user):
+    settings.COMBO_CELL_ASSET_SLOTS = {}
     page = Page.objects.create(title='One', slug='one', parent=None, template_name='standard')
     page2 = Page.objects.create(title='Two', slug='two', parent=page, template_name='standard')
     MenuCell.objects.create(page=page, order=0)
diff --git a/tests/test_public_templatetags.py b/tests/test_public_templatetags.py
index 5312fb4..27ccf59 100644
--- a/tests/test_public_templatetags.py
+++ b/tests/test_public_templatetags.py
@@ -13,7 +13,7 @@ from django.test.client import RequestFactory
 from django.contrib.auth.models import User, Group, AnonymousUser
 from django.utils.six import StringIO
 
-from combo.data.models import Page
+from combo.data.models import Page, TextCell
 from combo.apps.assets.models import Asset
 
 pytestmark = pytest.mark.django_db
@@ -141,6 +141,7 @@ def test_get_group():
                  '{% for c in country_list|get_group:"USA" %}{{c.name}},{% endfor %}')
     assert t.render(context) == 'New York,Chicago,'
 
+
 def test_asset_template_tags():
     for path in ('uploads', 'assets'):
         if os.path.exists(default_storage.path(path)):
@@ -194,6 +195,32 @@ def test_asset_template_tags():
         t = Template('''{% load assets %}{% asset_url page.picture "collectivity:banner" size="200x200" %}''')
         assert t.render(Context({'page': page})) == '/media/page-pictures/test2.svg'
 
+    cell = TextCell()
+    with override_settings(COMBO_CELL_ASSET_SLOTS={'data_textcell': {'picture': {'prefix': 'Picture'}}}):
+        # no slug
+        t = Template('''{% load assets %}{% get_asset cell=cell type='picture' as banner %}{% if banner %}BANNER{% endif %}''')
+        assert t.render(Context({'cell': cell})) == ''
+
+        # no asset
+        cell.slug = 'foobar'
+        t = Template('''{% load assets %}{% get_asset cell=cell type='picture' as banner %}{% if banner %}BANNER{% endif %}''')
+        assert t.render(Context({'cell': cell})) == ''
+
+        # ok
+        Asset.objects.create(key=cell.get_asset_slot_key('picture'), asset=File(StringIO('test'), 'test.png'))
+        t = Template('''{% load assets %}{% get_asset cell=cell type='picture' as banner %}{% if banner %}BANNER{% endif %}''')
+        assert t.render(Context({'cell': cell})) == 'BANNER'
+
+        # no context: AttributeError
+        t = Template('''{% load assets %}{% get_asset cell=cell type='picture' as banner %}{% if banner %}BANNER{% endif %}''')
+        assert t.render(Context()) == ''
+
+    with override_settings(COMBO_CELL_ASSET_SLOTS={}):
+        # cell type not defined in COMBO_CELL_ASSET_SLOTS
+        t = Template('''{% load assets %}{% get_asset cell=cell type='picture' as banner %}{% if banner %}BANNER{% endif %}''')
+        assert t.render(Context({'cell': cell})) == ''
+
+
 def test_startswith():
     t = Template('{% if foo|startswith:"bar" %}ok{% endif %}')
     context = Context({'foo': None})
-- 
2.20.1

