From c59be439ce662786c88f91c67821386f8b0f6778 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Laur=C3=A9line=20Gu=C3=A9rin?= <zebuline@entrouvert.com>
Date: Thu, 20 Feb 2020 14:39:46 +0100
Subject: [PATCH 6/6] cells: data cells invalid report (#38009)

---
 combo/data/apps.py                            |   9 +
 combo/data/models.py                          | 155 +++++++++--
 .../combo/manager/link-list-cell-form.html    |  11 +-
 combo/manager/static/css/combo.manager.css    |   4 +-
 combo/manager/views.py                        |   7 +-
 combo/public/views.py                         |   2 +-
 tests/test_cells.py                           | 241 +++++++++++++++++-
 tests/test_manager.py                         |  20 +-
 8 files changed, 420 insertions(+), 29 deletions(-)

diff --git a/combo/data/apps.py b/combo/data/apps.py
index 673636d..61a4920 100644
--- a/combo/data/apps.py
+++ b/combo/data/apps.py
@@ -20,3 +20,12 @@ from django.apps import AppConfig
 class DataConfig(AppConfig):
     name = 'combo.data'
     verbose_name = 'data'
+
+    def hourly(self):
+        from combo.data.library import get_cell_classes
+        from combo.data.models import CellBase
+
+        cell_classes = [c for c in self.get_models() if c in get_cell_classes()]
+        for cell in CellBase.get_cells(cell_filter=lambda x: x in cell_classes, page__snapshot__isnull=True):
+            if hasattr(cell, 'check_validity'):
+                cell.check_validity()
diff --git a/combo/data/models.py b/combo/data/models.py
index 6ec7741..71eaf8b 100644
--- a/combo/data/models.py
+++ b/combo/data/models.py
@@ -768,7 +768,7 @@ class CellBase(six.with_metaclass(CellMeta, models.Model)):
         return validity_info
 
     def mark_as_valid(self, save=True):
-        validity_info = self.validity_info.all().first()
+        validity_info = self.get_validity_info()
         if validity_info is None:
             return
         if save:
@@ -782,6 +782,23 @@ class CellBase(six.with_metaclass(CellMeta, models.Model)):
             return self.prefetched_validity_info[0]
         return self.validity_info.all().first()
 
+    def set_validity_from_url(
+            self, resp,
+            not_found_code='url_not_found', invalid_code='url_invalid',
+            save=True):
+        if resp is None:
+            # can not retrieve data, don't report cell as invalid
+            self.mark_as_valid(save=save)
+        elif resp.status_code == 404:
+            self.mark_as_invalid(not_found_code, save=save)
+        elif 400 <= resp.status_code < 500:
+            # 4xx error, cell is invalid
+            self.mark_as_invalid(invalid_code, save=save)
+        else:
+            # 2xx or 3xx: cell is valid
+            # 5xx error: can not retrieve data, don't report cell as invalid
+            self.mark_as_valid(save=save)
+
     def get_invalid_reason(self):
         validity_info = self.get_validity_info()
         if validity_info is None:
@@ -1015,9 +1032,25 @@ class LinkCell(CellBase):
     edit_link_label = _('Edit link')
     add_as_link_code = 'link'
 
+    invalid_reason_codes = {
+        'data_url_not_defined': _('No link set'),
+        'data_url_not_found': _('URL seems to unexist'),
+        'data_url_invalid': _('URL seems to be invalid'),
+    }
+
     class Meta:
         verbose_name = _('Link')
 
+    def save(self, *args, **kwargs):
+        if 'update_fields' in kwargs:
+            # don't check validity
+            return super(LinkCell, self).save(*args, **kwargs)
+
+        result = super(LinkCell, self).save(*args, **kwargs)
+        # check validity
+        self.check_validity()
+        return result
+
     def get_additional_label(self):
         title = self.title
         if not title and self.link_page:
@@ -1026,21 +1059,29 @@ class LinkCell(CellBase):
             return None
         return utils.ellipsize(title)
 
+    def get_url(self, context=None):
+        context = context or {}
+        if self.link_page:
+            url = self.link_page.get_online_url()
+        else:
+            url = utils.get_templated_url(self.url, context=context)
+        if self.anchor:
+            url += '#' + self.anchor
+        return url
+
     def get_cell_extra_context(self, context):
         render_skeleton = context.get('render_skeleton')
         request = context.get('request')
         extra_context = super(LinkCell, self).get_cell_extra_context(context)
         if self.link_page:
-            extra_context['url'] = self.link_page.get_online_url()
             extra_context['title'] = self.title or self.link_page.title
         else:
-            extra_context['url'] = utils.get_templated_url(self.url, context=context)
             extra_context['title'] = self.title or self.url
-        if self.anchor:
-            extra_context['url'] += '#' + self.anchor
-        if render_skeleton and not urlparse.urlparse(extra_context['url']).netloc:
+        url = self.get_url(context)
+        if render_skeleton and not urlparse.urlparse(url).netloc:
             # create full URL when used in a skeleton
-            extra_context['url'] = request.build_absolute_uri(extra_context['url'])
+            url = request.build_absolute_uri(url)
+        extra_context['url'] = url
         return extra_context
 
     def get_default_form_class(self):
@@ -1058,6 +1099,23 @@ class LinkCell(CellBase):
             return [link_data]
         return []
 
+    def check_validity(self):
+        if self.link_page:
+            self.mark_as_valid()
+            return
+        if not self.url:
+            self.mark_as_invalid('data_url_not_defined')
+            return
+
+        resp = None
+        try:
+            resp = requests.get(self.get_url())
+            resp.raise_for_status()
+        except (requests.exceptions.RequestException):
+            pass
+
+        self.set_validity_from_url(resp, not_found_code='data_url_not_found', invalid_code='data_url_invalid')
+
 
 @register_cell_class
 class LinkListCell(CellBase):
@@ -1066,6 +1124,10 @@ class LinkListCell(CellBase):
     template_name = 'combo/link-list-cell.html'
     manager_form_template = 'combo/manager/link-list-cell-form.html'
 
+    invalid_reason_codes = {
+        'data_link_invalid': _('Invalid link'),
+    }
+
     class Meta:
         verbose_name = _('List of links')
 
@@ -1073,11 +1135,15 @@ class LinkListCell(CellBase):
     def link_placeholder(self):
         return '_linkslist:{}'.format(self.pk)
 
-    def get_items(self):
+    def get_items(self, prefetch_validity_info=False):
         return CellBase.get_cells(
             page=self.page,
             placeholder=self.link_placeholder,
-            cell_filter=lambda x: hasattr(x, 'add_as_link_label'))
+            cell_filter=lambda x: hasattr(x, 'add_as_link_label'),
+            prefetch_validity_info=prefetch_validity_info)
+
+    def get_items_with_prefetch(self):
+        return self.get_items(prefetch_validity_info=True)
 
     def get_additional_label(self):
         title = self.title
@@ -1129,17 +1195,32 @@ class LinkListCell(CellBase):
         for link in self.get_items():
             link.duplicate(page_target=new_cell.page, placeholder=new_cell.link_placeholder)
 
+    def check_validity(self):
+        for link in self.get_items(prefetch_validity_info=True):
+            validity_info = link.get_validity_info()
+            if validity_info is not None:
+                self.mark_as_invalid('data_link_invalid')
+                return
+        self.mark_as_valid()
+
 
 @register_cell_class
 class FeedCell(CellBase):
     title = models.CharField(_('Title'), max_length=150, blank=True)
     url = models.CharField(_('URL'), blank=True, max_length=200)
-    limit = models.PositiveSmallIntegerField(_('Maximum number of entries'),
-            null=True, blank=True)
+    limit = models.PositiveSmallIntegerField(
+        _('Maximum number of entries'),
+        null=True, blank=True)
 
     manager_form_factory_kwargs = {'field_classes': {'url': TemplatableURLField}}
     template_name = 'combo/feed-cell.html'
 
+    invalid_reason_codes = {
+        'data_url_not_defined': _('No URL set'),
+        'data_url_not_found': _('URL seems to unexist'),
+        'data_url_invalid': _('URL seems to be invalid'),
+    }
+
     class Meta:
         verbose_name = _('RSS/Atom Feed')
 
@@ -1148,12 +1229,19 @@ class FeedCell(CellBase):
 
     def get_cell_extra_context(self, context):
         extra_context = super(FeedCell, self).get_cell_extra_context(context)
+
+        if not self.url:
+            self.mark_as_invalid('data_url_not_defined')
+            return extra_context
+
         if context.get('placeholder_search_mode'):
             # don't call webservices when we're just looking for placeholders
             return extra_context
+
         cache_key = hashlib.md5(smart_bytes(self.url)).hexdigest()
         feed_content = cache.get(cache_key)
         if not feed_content:
+            feed_response = None
             try:
                 feed_response = requests.get(utils.get_templated_url(self.url))
                 feed_response.raise_for_status()
@@ -1163,6 +1251,7 @@ class FeedCell(CellBase):
                 if feed_response.status_code == 200:
                     feed_content = feed_response.content
                     cache.set(cache_key, feed_content, 600)
+            self.set_validity_from_url(feed_response, not_found_code='data_url_not_found', invalid_code='data_url_invalid')
         if feed_content:
             extra_context['feed'] = feedparser.parse(feed_content)
             if self.limit:
@@ -1240,6 +1329,11 @@ class JsonCellBase(CellBase):
 
     _json_content = None
 
+    invalid_reason_codes = {
+        'data_url_not_found': _('URL seems to unexist'),
+        'data_url_invalid': _('URL seems to be invalid'),
+    }
+
     class Meta:
         abstract = True
 
@@ -1291,17 +1385,18 @@ class JsonCellBase(CellBase):
             if not url:
                 continue
             try:
-                json_response = utils.requests.get(url,
-                        headers={'Accept': 'application/json'},
-                        remote_service='auto',
-                        cache_duration=data_url_dict.get('cache_duration', self.cache_duration),
-                        without_user=True,
-                        raise_if_not_cached=not(context.get('synchronous')),
-                        invalidate_cache=invalidate_cache,
-                        log_errors=log_errors,
-                        timeout=data_url_dict.get('timeout', self.timeout),
-                        django_request=context.get('request'),
-                        )
+                json_response = utils.requests.get(
+                    url,
+                    headers={'Accept': 'application/json'},
+                    remote_service='auto',
+                    cache_duration=data_url_dict.get('cache_duration', self.cache_duration),
+                    without_user=True,
+                    raise_if_not_cached=not(context.get('synchronous')),
+                    invalidate_cache=invalidate_cache,
+                    log_errors=log_errors,
+                    timeout=data_url_dict.get('timeout', self.timeout),
+                    django_request=context.get('request'),
+                )
             except requests.RequestException as e:
                 extra_context[data_key + '_status'] = -1
                 extra_context[data_key + '_error'] = force_text(e)
@@ -1335,6 +1430,22 @@ class JsonCellBase(CellBase):
             # templated URLs
             context[data_key] = extra_context[data_key]
 
+        if not self._meta.abstract:
+            returns = [extra_context.get(d['key'] + '_status') for d in data_urls]
+            returns = set([s for s in returns if s is not None])
+            if returns and 200 not in returns:  # not a single valid answer
+                if 404 in returns:
+                    self.mark_as_invalid('data_url_not_found')
+                elif any([400 <= r < 500 for r in returns]):
+                    # at least 4xx errors, report the cell as invalid
+                    self.mark_as_invalid('data_url_invalid')
+                else:
+                    # 2xx or 3xx: cell is valid
+                    # 5xx error: can not retrieve data, don't report cell as invalid
+                    self.mark_as_valid()
+            else:
+                self.mark_as_valid()
+
         # keep cache of first response as it may be used to find the
         # appropriate template.
         self._json_content = extra_context[self.first_data_key]
diff --git a/combo/data/templates/combo/manager/link-list-cell-form.html b/combo/data/templates/combo/manager/link-list-cell-form.html
index a0688ac..3b86c8a 100644
--- a/combo/data/templates/combo/manager/link-list-cell-form.html
+++ b/combo/data/templates/combo/manager/link-list-cell-form.html
@@ -3,7 +3,7 @@
 
 {% block cell-form %}
 {{ form.as_p }}
-{% with cell.get_items as links %}
+{% with cell.get_items_with_prefetch as links %}
 {% if links %}
 <p><label>{% trans "Links:" %}</label></p>
 <div>
@@ -11,7 +11,14 @@
      data-link-list-order-url="{% url 'combo-manager-link-list-order' page_pk=page.pk cell_reference=cell.get_reference %}">
    {% for link in links %}
    <li data-link-item-id="{{ link.pk }}"><span class="handle">⣿</span>
-   <span title="{{ link }}">{{ link|truncatechars:100 }}</span>
+   <span title="{{ link }}">
+     {{ link|truncatechars:100 }}
+     {% with link.get_invalid_reason as invalid_reason %}
+     {% if invalid_reason %}
+     <span class="invalid">/!\ {{ link.get_invalid_reason }}</span>
+     {% endif %}
+     {% endwith %}
+   </span>
        <a rel="popup" title="{% trans "Edit" %}" class="link-action-icon edit" href="{% url 'combo-manager-page-list-cell-edit-link' page_pk=page.id cell_reference=cell.get_reference link_cell_reference=link.get_reference %}">{% trans "Edit" %}</a>
        <a rel="popup" title="{% trans "Delete" %}" class="link-action-icon delete" href="{% url 'combo-manager-page-list-cell-delete-link' page_pk=page.id cell_reference=cell.get_reference link_cell_reference=link.get_reference %}">{% trans "Delete" %}</a>
    </li>
diff --git a/combo/manager/static/css/combo.manager.css b/combo/manager/static/css/combo.manager.css
index f45aefa..3f664b4 100644
--- a/combo/manager/static/css/combo.manager.css
+++ b/combo/manager/static/css/combo.manager.css
@@ -81,6 +81,7 @@ div.cell h3 span.extra-css-class {
 
 div.cell h3 span.additional-label,
 div.cell h3 span.invalid,
+ul.list-of-links span.invalid,
 div.cell h3 span.visibility-summary,
 div.page span.visibility-summary {
 	font-size: 80%;
@@ -112,7 +113,8 @@ div.cell h3 span.visibility-summary {
 	max-width: 30%;
 }
 
-div.cell h3 span.invalid {
+div.cell h3 span.invalid,
+ul.list-of-links span.invalid {
 	color: red;
 }
 
diff --git a/combo/manager/views.py b/combo/manager/views.py
index 014178f..0f2b324 100644
--- a/combo/manager/views.py
+++ b/combo/manager/views.py
@@ -97,6 +97,7 @@ def invalid_cell_report(request):
         select_related=['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('_')]
     context = {
         'object_list': invalid_cells,
     }
@@ -647,7 +648,9 @@ class PageListCellAddLinkView(CreateView):
         else:
             form.instance.order = 1
         PageSnapshot.take(self.cell.page, request=self.request, comment=_('changed cell "%s"') % self.cell)
-        return super(PageListCellAddLinkView, self).form_valid(form)
+        response = super(PageListCellAddLinkView, self).form_valid(form)
+        self.cell.check_validity()
+        return response
 
     def get_success_url(self):
         return '%s#cell-%s' % (
@@ -688,6 +691,7 @@ class PageListCellEditLinkView(UpdateView):
         else:
             response = super(PageListCellEditLinkView, self).form_valid(form)
         PageSnapshot.take(self.cell.page, request=self.request, comment=_('changed cell "%s"') % self.cell)
+        self.cell.check_validity()
         return response
 
     def get_success_url(self):
@@ -722,6 +726,7 @@ class PageListCellDeleteLinkView(DeleteView):
     def delete(self, request, *args, **kwargs):
         response = super(PageListCellDeleteLinkView, self).delete(request, *args, **kwargs)
         PageSnapshot.take(self.cell.page, request=self.request, comment=_('changed cell "%s"') % self.cell)
+        self.cell.check_validity()
         return response
 
     def get_success_url(self):
diff --git a/combo/public/views.py b/combo/public/views.py
index da387d8..443d48a 100644
--- a/combo/public/views.py
+++ b/combo/public/views.py
@@ -518,7 +518,7 @@ def publish_page(request, page, status=200, template_name=None):
         if redirect_url:
             return HttpResponseRedirect(redirect_url)
 
-    cells = CellBase.get_cells(page=page)
+    cells = CellBase.get_cells(page=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/tests/test_cells.py b/tests/test_cells.py
index ed74281..edbeacd 100644
--- a/tests/test_cells.py
+++ b/tests/test_cells.py
@@ -7,8 +7,9 @@ import requests
 
 from combo.data.models import (
     Page, CellBase, TextCell, LinkCell, MenuCell, JsonCellBase,
-    JsonCell, ConfigJsonCell, LinkListCell, ValidityInfo
+    JsonCell, ConfigJsonCell, LinkListCell, FeedCell, ValidityInfo
 )
+from django.apps import apps
 from django.conf import settings
 from django.db import connection
 from django.forms.widgets import Media
@@ -28,6 +29,15 @@ from .test_manager import login
 
 pytestmark = pytest.mark.django_db
 
+
+@pytest.fixture
+def context():
+    ctx = {'request': RequestFactory().get('/')}
+    ctx['request'].user = None
+    ctx['request'].session = {}
+    return ctx
+
+
 def mock_json_response(content, **kwargs):
     content = force_bytes(content)
     text = force_text(content)
@@ -136,6 +146,60 @@ def test_link_cell():
     assert cell.render(ctx).strip() == '<a href="http://example.net/#anchor">altertitle</a>'
 
 
+def test_link_cell_validity():
+    page = Page.objects.create(title='example page', slug='example-page')
+    cell = LinkCell.objects.create(
+        page=page,
+        title='Example Site',
+        order=0,
+    )
+
+    # no link defined
+    validity_info = ValidityInfo.objects.latest('pk')
+    assert validity_info.invalid_reason_code == 'data_url_not_defined'
+    assert validity_info.invalid_since is not None
+
+    # internal link - no check
+    cell.link_page = page
+    with mock.patch('combo.data.models.requests.get') as requests_get:
+        mock_json = mock.Mock(status_code=404)
+        requests_get.return_value = mock_json
+        cell.save()
+    assert requests_get.call_args_list == []
+    assert ValidityInfo.objects.exists() is False
+
+    # external link
+    cell.link_page = None
+    cell.url = 'http://example.net/'
+    with mock.patch('combo.data.models.requests.get') as requests_get:
+        mock_json = mock.Mock(status_code=404)
+        requests_get.return_value = mock_json
+        cell.save()
+    validity_info = ValidityInfo.objects.latest('pk')
+    assert validity_info.invalid_reason_code == 'data_url_not_found'
+    assert validity_info.invalid_since is not None
+
+    with mock.patch('combo.data.models.requests.get') as requests_get:
+        mock_json = mock.Mock(status_code=200)
+        requests_get.return_value = mock_json
+        cell.save()
+    assert ValidityInfo.objects.exists() is False
+
+    with mock.patch('combo.data.models.requests.get') as requests_get:
+        mock_json = mock.Mock(status_code=500)
+        requests_get.return_value = mock_json
+        cell.save()
+    assert ValidityInfo.objects.exists() is False
+
+    with mock.patch('combo.data.models.requests.get') as requests_get:
+        mock_json = mock.Mock(status_code=400)
+        requests_get.return_value = mock_json
+        cell.save()
+    validity_info = ValidityInfo.objects.latest('pk')
+    assert validity_info.invalid_reason_code == 'data_url_invalid'
+    assert validity_info.invalid_since is not None
+
+
 def test_link_list_cell():
     page = Page.objects.create(title='example page', slug='example-page')
 
@@ -167,6 +231,63 @@ def test_link_list_cell():
     assert '<ul><li><a href="http://example.net/#anchor">altertitle</a></li></ul>' in cell.render(ctx)
 
 
+def test_link_list_cell_validity():
+    page = Page.objects.create(title='example page', slug='example-page')
+
+    cell = LinkListCell.objects.create(order=0, page=page)
+    item = LinkCell.objects.create(page=page, placeholder=cell.link_placeholder, order=0)
+
+    item.mark_as_valid()
+    cell.check_validity()
+    assert ValidityInfo.objects.exists() is False
+
+    item.mark_as_invalid('foo_bar_reason')
+    cell.check_validity()
+    validity_info = ValidityInfo.objects.latest('pk')
+    assert validity_info.invalid_reason_code == 'data_link_invalid'
+    assert validity_info.invalid_since is not None
+
+
+def test_feed_cell_validity(context):
+    page = Page.objects.create(title='example page', slug='example-page')
+    cell = FeedCell.objects.create(page=page, placeholder='content', order=1)
+
+    cell.get_cell_extra_context(context)
+    validity_info = ValidityInfo.objects.latest('pk')
+    assert validity_info.invalid_reason_code == 'data_url_not_defined'
+    assert validity_info.invalid_since is not None
+
+    cell.url = 'http://example.net/'
+    cell.save()
+    with mock.patch('combo.data.models.requests.get') as requests_get:
+        mock_json = mock.Mock(status_code=404)
+        requests_get.return_value = mock_json
+        cell.get_cell_extra_context(context)
+    validity_info = ValidityInfo.objects.latest('pk')
+    assert validity_info.invalid_reason_code == 'data_url_not_found'
+    assert validity_info.invalid_since is not None
+
+    with mock.patch('combo.data.models.requests.get') as requests_get:
+        mock_json = mock.Mock(status_code=200, content='')
+        requests_get.return_value = mock_json
+        cell.get_cell_extra_context(context)
+    assert ValidityInfo.objects.exists() is False
+
+    with mock.patch('combo.data.models.requests.get') as requests_get:
+        mock_json = mock.Mock(status_code=500)
+        requests_get.return_value = mock_json
+        cell.get_cell_extra_context(context)
+    assert ValidityInfo.objects.exists() is False
+
+    with mock.patch('combo.data.models.requests.get') as requests_get:
+        mock_json = mock.Mock(status_code=400)
+        requests_get.return_value = mock_json
+        cell.get_cell_extra_context(context)
+    validity_info = ValidityInfo.objects.latest('pk')
+    assert validity_info.invalid_reason_code == 'data_url_invalid'
+    assert validity_info.invalid_since is not None
+
+
 def test_menu_cell():
     Page.objects.all().delete()
     parent = Page.objects.create(
@@ -388,6 +509,52 @@ def test_json_cell_with_varnames(app):
         assert '/var1=foo/' in resp.text
         assert '/var2=bar/' in resp.text
 
+
+def test_json_cell_validity(context):
+    page = Page.objects.create(title='example page', slug='example-page')
+    cell = JsonCell.objects.create(
+        page=page, placeholder='content', order=1,
+        varnames_str='var1, var2, ',
+        url='http://foo?varone=[var1]&vartwo=[var2]',
+        template_string='/var1={{var1}}/var2={{var2}}/')
+
+    with mock.patch('combo.data.models.requests.get') as requests_get:
+        cell.get_cell_extra_context(context)
+    assert requests_get.call_args_list == []  # invalid context
+    assert ValidityInfo.objects.exists() is False
+
+    context['var1'] = 'foo'
+    context['var2'] = 'bar'
+    context['synchronous'] = True  # to get fresh content
+    with mock.patch('combo.utils.requests.get') as requests_get:
+        mock_json = mock.Mock(status_code=404)
+        requests_get.side_effect = [mock_json]
+        cell.get_cell_extra_context(context)
+    validity_info = ValidityInfo.objects.latest('pk')
+    assert validity_info.invalid_reason_code == 'data_url_not_found'
+    assert validity_info.invalid_since is not None
+
+    with mock.patch('combo.utils.requests.get') as requests_get:
+        data = {'data': []}
+        requests_get.return_value = mock_json_response(content=json.dumps(data), status_code=200)
+        cell.get_cell_extra_context(context)
+    assert ValidityInfo.objects.exists() is False
+
+    with mock.patch('combo.utils.requests.get') as requests_get:
+        mock_json = mock.Mock(status_code=500)
+        requests_get.return_value = mock_json
+        cell.get_cell_extra_context(context)
+    assert ValidityInfo.objects.exists() is False
+
+    with mock.patch('combo.utils.requests.get') as requests_get:
+        mock_json = mock.Mock(status_code=400)
+        requests_get.return_value = mock_json
+        cell.get_cell_extra_context(context)
+    validity_info = ValidityInfo.objects.latest('pk')
+    assert validity_info.invalid_reason_code == 'data_url_invalid'
+    assert validity_info.invalid_since is not None
+
+
 def test_config_json_cell():
     page = Page(title='example page', slug='example-page')
     page.save()
@@ -508,6 +675,63 @@ def test_config_json_cell_with_param_in_url(app):
             assert requests_get.call_args[-1]['log_errors'] == False
             assert requests_get.call_args[-1]['timeout'] == 42
 
+
+def test_config_json_cell_validity(settings, context):
+    settings.JSON_CELL_TYPES = {
+        'test-config-json-cell': {
+            'name': 'Foobar',
+            'url': 'http://foo?varone=[var1]&vartwo=[var2]',
+            'varnames': ['var1', 'var2']
+        },
+    }
+    templates_settings = [settings.TEMPLATES[0].copy()]
+    templates_settings[0]['DIRS'] = ['%s/templates-1' % os.path.abspath(os.path.dirname(__file__))]
+    settings.TEMPLATES = templates_settings
+
+    page = Page.objects.create(title='example page', slug='example-page')
+    cell = ConfigJsonCell.objects.create(
+        page=page, placeholder='content', order=1,
+        key='test-config-json-cell',
+        parameters={'identifier': 'plop'})
+    assert cell.varnames == ['var1', 'var2']
+
+    with mock.patch('combo.data.models.requests.get') as requests_get:
+        cell.get_cell_extra_context(context)
+    assert requests_get.call_args_list == []  # invalid context
+    assert ValidityInfo.objects.exists() is False
+
+    context['var1'] = 'foo'
+    context['var2'] = 'bar'
+    context['synchronous'] = True  # to get fresh content
+    with mock.patch('combo.utils.requests.get') as requests_get:
+        mock_json = mock.Mock(status_code=404)
+        requests_get.side_effect = [mock_json]
+        cell.get_cell_extra_context(context)
+    validity_info = ValidityInfo.objects.latest('pk')
+    assert validity_info.invalid_reason_code == 'data_url_not_found'
+    assert validity_info.invalid_since is not None
+
+    with mock.patch('combo.utils.requests.get') as requests_get:
+        data = {'data': []}
+        requests_get.return_value = mock_json_response(content=json.dumps(data), status_code=200)
+        cell.get_cell_extra_context(context)
+    assert ValidityInfo.objects.exists() is False
+
+    with mock.patch('combo.utils.requests.get') as requests_get:
+        mock_json = mock.Mock(status_code=500)
+        requests_get.return_value = mock_json
+        cell.get_cell_extra_context(context)
+    assert ValidityInfo.objects.exists() is False
+
+    with mock.patch('combo.utils.requests.get') as requests_get:
+        mock_json = mock.Mock(status_code=400)
+        requests_get.return_value = mock_json
+        cell.get_cell_extra_context(context)
+    validity_info = ValidityInfo.objects.latest('pk')
+    assert validity_info.invalid_reason_code == 'data_url_invalid'
+    assert validity_info.invalid_since is not None
+
+
 def test_json_force_async():
     cell = JsonCellBase()
     cell.url = 'http://example.net/test-force-async'
@@ -807,3 +1031,18 @@ def test_cell_is_visible():
     validity_info.invalid_since = now() - datetime.timedelta(days=2)
     validity_info.save()
     assert cell.is_visible() is False
+
+
+def test_hourly():
+    appconfig = apps.get_app_config('data')
+    page = Page.objects.create(title='xxx', slug='test_current_forms_cell_render', template_name='standard')
+    cell_classes = [c for c in appconfig.get_models() if c in get_cell_classes()]
+    for klass in cell_classes:
+        klass.objects.create(page=page, placeholder='content', order=0)
+    for klass in cell_classes:
+        if klass in [LinkCell, LinkListCell]:
+            with mock.patch('combo.data.models.%s.check_validity' % klass.__name__) as check_validity:
+                appconfig.hourly()
+            assert check_validity.call_args_list == [mock.call()]
+        else:
+            assert hasattr(klass, 'check_validity') is False
diff --git a/tests/test_manager.py b/tests/test_manager.py
index 50c6d83..e894c96 100644
--- a/tests/test_manager.py
+++ b/tests/test_manager.py
@@ -182,6 +182,14 @@ def test_edit_page_cell_invalid_placeholder(app, admin_user):
     resp = app.get('/manage/pages/%s/' % page.pk)
     assert '<span class="invalid">/!\\ foo_bar_reason</span>' not in resp.text
 
+    cell2 = LinkListCell.objects.create(order=0, placeholder='content', page=page)
+    item = LinkCell.objects.create(page=page, placeholder=cell2.link_placeholder, order=0)
+    item.mark_as_invalid('foo_bar_reason')
+    cell2.check_validity()
+    resp = app.get('/manage/pages/%s/' % page.pk)
+    assert '<span class="invalid">/!\\ Invalid link</span>' in resp.text
+    assert '<span class="invalid">/!\\ foo_bar_reason</span>' in resp.text
+
 
 def test_edit_page_optional_placeholder(app, admin_user):
     Page.objects.all().delete()
@@ -367,7 +375,7 @@ def test_edit_page_num_queries(app, admin_user):
     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) == 29
+        assert len(ctx.captured_queries) == 31
 
 
 def test_delete_page(app, admin_user):
@@ -602,12 +610,22 @@ def test_invalid_cell_report(app, admin_user):
     assert '<a href="/manage/pages/{}/">{}</a>'.format(page.pk, page.title) in resp.text
     assert '<a href="/manage/pages/{}/#cell-{}">{}</a>'.format(page.pk, cell.get_reference(), cell.get_label()) in resp.text
 
+    # cells from snapshot are not reported
     snapshot = PageSnapshot.objects.create(page=page)
     page.snapshot = snapshot
     page.save()
     resp = app.get('/manage/cells/invalid-report/')
     assert resp.context['object_list'] == []
 
+    # cells used in LinkListCell are not reported
+    page.snapshot = None
+    page.save()
+    cell2 = LinkListCell.objects.create(order=0, placeholder='content', page=page)
+    item = LinkCell.objects.create(page=page, placeholder=cell2.link_placeholder, order=0)
+    item.mark_as_invalid('foo_bar_reason')
+    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

