From 7ce92de9049f3c360f2880672ba2eb91e7a4b3a4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Laur=C3=A9line=20Gu=C3=A9rin?= <zebuline@entrouvert.com>
Date: Thu, 4 Jun 2020 14:55:20 +0200
Subject: [PATCH] cells: don't check validity for json cells with multiple
 variables in url (#43604)

---
 combo/data/models.py |   9 +++-
 tests/test_cells.py  | 112 ++++++++++++++++++++++++++++++++++++-------
 2 files changed, 103 insertions(+), 18 deletions(-)

diff --git a/combo/data/models.py b/combo/data/models.py
index 4e4c0ddc..16268321 100644
--- a/combo/data/models.py
+++ b/combo/data/models.py
@@ -1228,7 +1228,7 @@ class LinkCell(CellBase):
             self.mark_as_invalid('data_url_not_defined')
             return
 
-        if self.url.count('{{') > 1:
+        if sum(map(lambda a: self.url.count(a), ['{{', '['])) > 1:
             self.mark_as_valid()
             return
 
@@ -1565,7 +1565,12 @@ class JsonCellBase(CellBase):
             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 = []
+            for data_url in data_urls:
+                if sum(map(lambda a: data_url['url'].count(a), ['{{', '['])) > 1:
+                    # ignore returns of url with more than one variable
+                    continue
+                returns.append(extra_context.get(data_url['key'] + '_status'))
             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:
diff --git a/tests/test_cells.py b/tests/test_cells.py
index d78d94fa..1e03304b 100644
--- a/tests/test_cells.py
+++ b/tests/test_cells.py
@@ -146,7 +146,7 @@ def test_link_cell():
     assert cell.render(ctx).strip() == '<a href="http://example.net/#anchor">altertitle</a>'
 
 
-def test_link_cell_validity():
+def test_link_cell_validity(settings):
     page = Page.objects.create(title='example page', slug='example-page')
     cell = LinkCell.objects.create(
         page=page,
@@ -207,31 +207,38 @@ def test_link_cell_validity():
     # external link with a single variable
     cell.link_page = None
     cell.anchor = ''
-    cell.url = '{{test_url}}'
-    with override_settings(TEMPLATE_VARS={'test_url': 'http://www.example.net/'}):
+    settings.TEMPLATE_VARS = {'var1': 'foo'}
+    test_urls = [
+        'http://foo?varone=[var1]',
+        'http://foo?varone={{var1}}',
+    ]
+    for test_url in test_urls:
+        cell.url = test_url
         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 requests_get.call_args_list[0][0][0] == 'http://www.example.net/'
         assert ValidityInfo.objects.exists() is False
 
-    with override_settings(TEMPLATE_VARS={'test_url': 'http://www.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()
-        assert requests_get.call_args_list[0][0][0] == 'http://www.example.net/'
         assert ValidityInfo.objects.exists() is True
 
     # external link with two variables
-    cell.url = '{{test_url}}/path/{{other}}'
-    with override_settings(TEMPLATE_VARS={'test_url': 'http://www.example.net/'}):
+    settings.TEMPLATE_VARS = {'var1': 'foo', 'var2': 'bar'}
+    test_urls = [
+        'http://foo?varone=[var1]&vartwo=[var2]',
+        'http://foo?varone={{var1}}&vartwo={{var2}}',
+        'http://foo?varone=[var1]&vartwo={{var2}}',
+    ]
+    for test_url in test_urls:
+        cell.url = test_url
         with mock.patch('combo.data.models.requests.get') as requests_get:
-            mock_json = mock.Mock(status_code=200)
+            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
 
 
@@ -562,17 +569,16 @@ def test_json_cell_validity(context):
     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}}/')
+        url='http://foo?varone=[var1]&vartwo=[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
+    cell.varnames_str = ''
+    cell.url = 'http://foo'
     with mock.patch('combo.utils.requests.get') as requests_get:
         mock_json = mock.Mock(status_code=404)
         requests_get.side_effect = [mock_json]
@@ -601,6 +607,43 @@ def test_json_cell_validity(context):
     assert validity_info.invalid_reason_code == 'data_url_invalid'
     assert validity_info.invalid_since is not None
 
+    # url with a single variable
+    context['var1'] = 'foo'
+    cell.varnames_str = 'var1'
+    test_urls = [
+        'http://foo?varone=[var1]',
+        'http://foo?varone={{var1}}',
+    ]
+    for test_url in test_urls:
+        cell.url = test_url
+        with mock.patch('combo.utils.requests.get') as requests_get:
+            mock_json = mock.Mock(status_code=200)
+            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=404)
+            requests_get.side_effect = [mock_json]
+            cell.get_cell_extra_context(context)
+        assert ValidityInfo.objects.exists() is True
+
+    # url with two variables
+    context['var2'] = 'bar'
+    cell.varnames_str = 'var1, var2'
+    test_urls = [
+        'http://foo?varone=[var1]&vartwo=[var2]',
+        'http://foo?varone={{var1}}&vartwo={{var2}}',
+        'http://foo?varone=[var1]&vartwo={{var2}}',
+    ]
+    for test_url in test_urls:
+        cell.url = test_url
+        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)
+        assert ValidityInfo.objects.exists() is False
+
 
 def test_config_json_cell():
     page = Page(title='example page', slug='example-page')
@@ -747,9 +790,9 @@ def test_config_json_cell_validity(settings, 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
+    settings.JSON_CELL_TYPES['test-config-json-cell']['url'] = 'http://foo'
+    settings.JSON_CELL_TYPES['test-config-json-cell']['varnames'] = []
     with mock.patch('combo.utils.requests.get') as requests_get:
         mock_json = mock.Mock(status_code=404)
         requests_get.side_effect = [mock_json]
@@ -778,6 +821,43 @@ def test_config_json_cell_validity(settings, context):
     assert validity_info.invalid_reason_code == 'data_url_invalid'
     assert validity_info.invalid_since is not None
 
+    # url with a single variable
+    context['var1'] = 'foo'
+    test_urls = [
+        'http://foo?varone=[var1]',
+        'http://foo?varone={{var1}}',
+    ]
+    settings.JSON_CELL_TYPES['test-config-json-cell']['varnames'] = ['var1']
+    for test_url in test_urls:
+        settings.JSON_CELL_TYPES['test-config-json-cell']['url'] = test_url
+        with mock.patch('combo.utils.requests.get') as requests_get:
+            mock_json = mock.Mock(status_code=200)
+            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=404)
+            requests_get.side_effect = [mock_json]
+            cell.get_cell_extra_context(context)
+        assert ValidityInfo.objects.exists() is True
+
+    # url with two variables
+    context['var2'] = 'bar'
+    settings.JSON_CELL_TYPES['test-config-json-cell']['varnames'] = ['var1', 'var2']
+    test_urls = [
+        'http://foo?varone=[var1]&vartwo=[var2]',
+        'http://foo?varone={{var1}}&vartwo={{var2}}',
+        'http://foo?varone=[var1]&vartwo={{var2}}',
+    ]
+    for test_url in test_urls:
+        settings.JSON_CELL_TYPES['test-config-json-cell']['url'] = test_url
+        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)
+        assert ValidityInfo.objects.exists() is False
+
 
 def test_json_force_async():
     cell = JsonCellBase()
-- 
2.20.1

