cells: don't check validity for json cells with multiple variables in url (#43604)

combo/data/models.py

@@ -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 data_url['url'].count('{{') > 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:

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,32 +207,28 @@ 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/'}):
-        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
+    settings.TEMPLATE_VARS = {'var1': 'foo'}
+    cell.url = 'http://foo?varone={{var1}}'
+    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 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
+    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 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/'}):
-        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 == []
-        assert ValidityInfo.objects.exists() is False
+    settings.TEMPLATE_VARS = {'var1': 'foo', 'var2': 'bar'}
+    cell.url = 'http://foo?varone={{var1}}&vartwo={{var2}}'
+    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 ValidityInfo.objects.exists() is False
 
 
 def test_link_list_cell():
@@ -562,17 +558,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 +596,32 @@ 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'
+    cell.url = 'http://foo?varone={{var1}}'
+    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'
+    cell.url = 'http://foo?varone={{var1}}&vartwo={{var2}}'
+    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 +768,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 +799,32 @@ 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'
+    settings.JSON_CELL_TYPES['test-config-json-cell']['varnames'] = ['var1']
+    settings.JSON_CELL_TYPES['test-config-json-cell']['url'] = 'http://foo?varone={{var1}}'
+    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']
+    settings.JSON_CELL_TYPES['test-config-json-cell']['url'] = 'http://foo?varone={{var1}}&vartwo={{var2}}'
+    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()