Merge pull request #1747 from alphagov/add_precompiled_letters

Fixed bug only where only data being passed to TemplatePreview when JSON is required

app/template/rest.py

@@ -8,7 +8,6 @@ from flask import (
     request)
 from requests import post as requests_post
 
-
 from app.dao.notifications_dao import get_notification_by_id
 from app.dao.templates_dao import (
     dao_update_template,
@@ -33,7 +32,6 @@ from app.utils import get_template_instance, get_public_notify_type_text
 
 template_blueprint = Blueprint('template', __name__, url_prefix='/service/<uuid:service_id>/template')
 
-
 register_errors(template_blueprint)
 
 
@@ -189,7 +187,6 @@ def redact_template(template, data):
 
 @template_blueprint.route('/preview/<uuid:notification_id>/<file_type>', methods=['GET'])
 def preview_letter_template_by_notification_id(service_id, notification_id, file_type):
-
     if file_type not in ('pdf', 'png'):
         raise InvalidRequest({'content': ["file_type must be pdf or png"]}, status_code=400)
 
@@ -206,20 +203,20 @@ def preview_letter_template_by_notification_id(service_id, notification_id, file
             pdf_file = get_letter_pdf(notification)
 
         except botocore.exceptions.ClientError:
-            current_app.logger.info
+            current_app.logger.exception(
+                'Error getting letter file from S3 notification id {}'.format(notification_id))
             raise InvalidRequest('Error getting letter file from S3 notification id {}'.format(notification_id),
                                  status_code=500)
 
         content = base64.b64encode(pdf_file).decode('utf-8')
 
         if file_type == 'png':
-
             url = '{}/precompiled-preview.png{}'.format(
                 current_app.config['TEMPLATE_PREVIEW_API_HOST'],
                 '?page={}'.format(page) if page else ''
             )
 
-            content = _get_png_preview(url, content, notification.id)
+            content = _get_png_preview(url, content, notification.id, json=False)
 
     else:
 
@@ -245,21 +242,38 @@ def preview_letter_template_by_notification_id(service_id, notification_id, file
             '?page={}'.format(page) if page else ''
         )
 
-        content = _get_png_preview(url, data, notification.id)
+        content = _get_png_preview(url, data, notification.id, json=True)
 
     return jsonify({"content": content})
 
 
-def _get_png_preview(url, data, notification_id):
+def _get_png_preview(url, data, notification_id, json=True):
-    resp = requests_post(
+    if json:
-        url,
+        resp = requests_post(
-        data=data,
+            url,
-        headers={'Authorization': 'Token {}'.format(current_app.config['TEMPLATE_PREVIEW_API_KEY'])}
+            json=data,
-    )
+            headers={'Authorization': 'Token {}'.format(current_app.config['TEMPLATE_PREVIEW_API_KEY'])}
+        )
+    else:
+        resp = requests_post(
+            url,
+            data=data,
+            headers={'Authorization': 'Token {}'.format(current_app.config['TEMPLATE_PREVIEW_API_KEY'])}
+        )
 
     if resp.status_code != 200:
+        current_app.logger.exception(
+            'Error generating preview letter for {} \nStatus code: {}\n{}'.format(
+                notification_id,
+                resp.status_code,
+                resp.content
+            ))
         raise InvalidRequest(
-            'Error generating preview for {}'.format(notification_id), status_code=500
+            'Error generating preview letter for {}\nStatus code: {}\n{}'.format(
+                notification_id,
+                resp.status_code,
+                resp.content
+            ), status_code=500
         )
 
     return base64.b64encode(resp.content).decode('utf-8')

tests/app/template/test_rest.py

@@ -828,7 +828,7 @@ def test_preview_letter_template_by_id_valid_file_type(
         with requests_mock.Mocker() as request_mock:
             content = b'\x00\x01'
 
-            request_mock.post(
+            mock_post = request_mock.post(
                 'http://localhost/notifications-template-preview/preview.pdf',
                 content=content,
                 headers={'X-pdf-page-count': '1'},
@@ -842,6 +842,7 @@ def test_preview_letter_template_by_id_valid_file_type(
                 file_type='pdf'
             )
 
+            assert mock_post.last_request.json()
             assert base64.b64decode(resp['content']) == content
 
 
@@ -859,7 +860,7 @@ def test_preview_letter_template_by_id_template_preview_500(
         with requests_mock.Mocker() as request_mock:
             content = b'\x00\x01'
 
-            request_mock.post(
+            mock_post = request_mock.post(
                 'http://localhost/notifications-template-preview/preview.pdf',
                 content=content,
                 headers={'X-pdf-page-count': '1'},
@@ -874,7 +875,9 @@ def test_preview_letter_template_by_id_template_preview_500(
                 _expected_status=500
             )
 
-            assert resp['message'] == 'Error generating preview for {}'.format(sample_letter_notification.id)
+            assert mock_post.last_request.json()
+            assert 'Status code: 404' in resp['message']
+            assert 'Error generating preview letter for {}'.format(sample_letter_notification.id) in resp['message']
 
 
 def test_preview_letter_template_precompiled_pdf_file_type(
@@ -978,7 +981,7 @@ def test_preview_letter_template_precompiled_png_file_type(
 
             mock_get_letter_pdf = mocker.patch('app.template.rest.get_letter_pdf', return_value=pdf_content)
 
-            request_mock.post(
+            mock_post = request_mock.post(
                 'http://localhost/notifications-template-preview/precompiled-preview.png',
                 content=png_content,
                 headers={'X-pdf-page-count': '1'},
@@ -992,6 +995,8 @@ def test_preview_letter_template_precompiled_png_file_type(
                 file_type='png'
             )
 
+            with pytest.raises(ValueError):
+                mock_post.last_request.json()
             assert mock_get_letter_pdf.called_once_with(notification)
             assert base64.b64decode(resp['content']) == png_content
 
@@ -1023,7 +1028,7 @@ def test_preview_letter_template_precompiled_png_template_preview_500_error(
 
             mocker.patch('app.template.rest.get_letter_pdf', return_value=pdf_content)
 
-            request_mock.post(
+            mock_post = request_mock.post(
                 'http://localhost/notifications-template-preview/precompiled-preview.png',
                 content=png_content,
                 headers={'X-pdf-page-count': '1'},
@@ -1039,6 +1044,9 @@ def test_preview_letter_template_precompiled_png_template_preview_500_error(
 
             )
 
+            with pytest.raises(ValueError):
+                mock_post.last_request.json()
+
 
 def test_preview_letter_template_precompiled_png_template_preview_400_error(
         notify_api,
@@ -1067,7 +1075,7 @@ def test_preview_letter_template_precompiled_png_template_preview_400_error(
 
             mocker.patch('app.template.rest.get_letter_pdf', return_value=pdf_content)
 
-            request_mock.post(
+            mock_post = request_mock.post(
                 'http://localhost/notifications-template-preview/precompiled-preview.png',
                 content=png_content,
                 headers={'X-pdf-page-count': '1'},
@@ -1080,5 +1088,7 @@ def test_preview_letter_template_precompiled_png_template_preview_400_error(
                 notification_id=notification.id,
                 file_type='png',
                 _expected_status=500
-
             )
+
+            with pytest.raises(ValueError):
+                mock_post.last_request.json()