json cell: add url, status and error in context (#17688)

combo/data/models.py

@@ -879,13 +879,22 @@ class JsonCellBase(CellBase):
                     raise_if_not_cached=not(context.get('synchronous')),
                     invalidate_cache=invalidate_cache,
                     )
-            if json_response.status_code == 200:
+            extra_context[data_key + '_url'] = url
+            extra_context[data_key + '_status'] = json_response.status_code
+            if json_response.status_code // 100 == 2:
+                if json_response.status_code != 204:  # 204 = No Content
+                    try:
+                        extra_context[data_key] = json.loads(json_response.content)
+                    except ValueError:
+                        extra_context[data_key + '_error'] = 'invalid_json'
+                        logger = logging.getLogger(__name__)
+                        logger.error('invalid json content (%s)', url)
+                        continue
+            elif json_response.headers.get('content-type') == 'application/json':
                 try:
-                    extra_context[data_key] = json.loads(json_response.content)
+                    extra_context[data_key + '_error'] = json.loads(json_response.content)
                 except ValueError:
-                    logger = logging.getLogger(__name__)
-                    logger.error('invalid json content (%s)', url)
-                    continue
+                    extra_context[data_key + '_error'] = 'invalid_json'
 
             # update context with data key so it can be used in future
             # templated URLs

tests/test_cells.py

@@ -158,11 +158,33 @@ def test_json_cell():
         requests_get.return_value = mock.Mock(content=json.dumps(data), status_code=200)
         context = cell.get_cell_extra_context({})
         assert context['json'] == data
+        assert context['json_url'] == ''
+        assert context['json_status'] == 200
+        assert 'json_error' not in context
 
-        cell.url = 'http://test2'
-        requests_get.return_value = mock.Mock(content=json.dumps(data), status_code=404)
+        requests_get.return_value = mock.Mock(status_code=204)  # 204 : No Content
         context = cell.get_cell_extra_context({})
         assert context['json'] is None
+        assert context['json_url'] == ''
+        assert context['json_status'] == 204
+        assert 'json_error' not in context
+
+        cell.url = 'http://test2'
+        requests_get.return_value = mock.Mock(content='not found', status_code=404,
+                                             headers={})
+        context = cell.get_cell_extra_context({})
+        assert context['json'] is None
+        assert context['json_url'] == 'http://test2'
+        assert context['json_status'] == 404
+        assert 'json_error' not in context
+
+        requests_get.return_value = mock.Mock(content=json.dumps(data), status_code=404,
+                                              headers={'content-type': 'application/json'})
+        context = cell.get_cell_extra_context({})
+        assert context['json'] is None
+        assert context['json_url'] == 'http://test2'
+        assert context['json_status'] == 404
+        assert context['json_error'] == data
 
     with pytest.raises(NothingInCacheException):
         cell.url = 'http://test3'
@@ -186,6 +208,12 @@ def test_json_cell():
         result = cell.render(Context({'synchronous': True}))
         assert result == ''
 
+        context = cell.get_cell_extra_context({})
+        assert context['json'] is None
+        assert context['json_url'] == 'http://test5'
+        assert context['json_status'] == 200
+        assert context['json_error'] == 'invalid_json'
+
     # URL is a template, using [variables]
     cell.cache_duration = 10
     data = {'data': [{'url': 'xxx', 'text': 'xxx'}]}
@@ -194,6 +222,8 @@ def test_json_cell():
         requests_get.return_value = mock.Mock(content=json.dumps(data), status_code=200)
         context = cell.get_cell_extra_context(Context({'foobar': 'barfoo'}))
         assert context['json'] == data
+        assert context['json_url'] == 'http://testuser?barfoo'
+        assert context['json_status'] == 200
         assert requests_get.call_args[0][0] == 'http://testuser?barfoo'
         assert requests_get.call_args[1]['cache_duration'] == 10
         assert requests_get.call_count == 1
@@ -209,6 +239,7 @@ def test_json_cell():
         requests_get.return_value = mock.Mock(content=json.dumps(data), status_code=200)
         context = cell.get_cell_extra_context(Context({'request': request}))
         assert context['json'] == data
+        assert context['json_url'] == 'http://testuser?email=foo%40example.net'
         assert requests_get.call_count == 1
         assert requests_get.call_args[0][0] == 'http://testuser?email=foo%40example.net'
         assert requests_get.call_args[1]['cache_duration'] == 10
@@ -257,6 +288,8 @@ def test_config_json_cell():
             requests_get.return_value = mock.Mock(content=json.dumps({'hello': 'world'}), status_code=200)
             context = cell.get_cell_extra_context(Context({'request': request}))
             assert context['json'] == {'hello': 'world'}
+            assert context['json_url'] == 'http://test/'
+            assert context['json_status'] == 200
             assert context['parameters'] == {'blah': 'plop'}
             assert context['blah'] == 'plop'
 
@@ -398,7 +431,8 @@ def test_config_json_cell_additional_url(app):
 
         with mock.patch('combo.utils.requests.get') as requests_get:
             data = {'data': 'toto'}
-            requests_get.return_value = mock.Mock(content=json.dumps(data), status_code=404)
+            requests_get.return_value = mock.Mock(content=json.dumps(data), status_code=404,
+                                                  headers={'content-type': 'application/json'})
             url = reverse('combo-public-ajax-page-cell',
                 kwargs={'page_pk': page.id, 'cell_reference': cell.get_reference()})
             resp = app.get(url)
@@ -406,3 +440,25 @@ def test_config_json_cell_additional_url(app):
             assert len(requests_get.mock_calls) == 2
             assert requests_get.mock_calls[0][1][0] == 'http://foo'
             assert requests_get.mock_calls[1][1][0] == 'http://bar'
+            context = cell.get_cell_extra_context({})
+            assert context['json'] == None
+            assert context['json_url'] == 'http://foo'
+            assert context['json_status'] == 404
+            assert context['json_error'] == data
+            assert context['plop'] == None
+            assert context['plop_url'] == 'http://bar'
+            assert context['plop_status'] == 404
+            assert context['plop_error'] == data
+
+        with mock.patch('combo.utils.requests.get') as requests_get:
+            data = {'data': 'toto'}
+            requests_get.return_value = mock.Mock(content=json.dumps(data), status_code=200)
+            context = cell.get_cell_extra_context({})
+            assert context['json'] == data
+            assert context['json_url'] == 'http://foo'
+            assert context['json_status'] == 200
+            assert 'json_error' not in context
+            assert context['plop'] == data
+            assert context['plop_url'] == 'http://bar'
+            assert context['plop_status'] == 200
+            assert 'plop_error' not in context