From e25c6cd3b9c9e8f29a64bb6a0ba3d5f951d58f01 Mon Sep 17 00:00:00 2001 From: Richard Chapman Date: Thu, 8 Mar 2018 17:10:34 +0000 Subject: [PATCH 1/2] Updated so that a precompiled pdf page count can be passed to the rendering template. Currently it uses the template from the API to calculate this which for a precompiled template is always 1. Gets the PDF and then uses the utils method to get the page count. * Added logic for precompiled letters * Added test to test the new path * Updated existing tests now the path has changed --- app/main/views/notifications.py | 11 +++++- tests/app/main/views/test_notifications.py | 43 ++++++++++++++++++++++ 2 files changed, 53 insertions(+), 1 deletion(-) diff --git a/app/main/views/notifications.py b/app/main/views/notifications.py index b5805f913..3ba0bb706 100644 --- a/app/main/views/notifications.py +++ b/app/main/views/notifications.py @@ -1,5 +1,6 @@ # -*- coding: utf-8 -*- import base64 +import io import os from datetime import datetime @@ -14,6 +15,7 @@ from flask import ( ) from flask_login import login_required from notifications_python_client.errors import APIError +from notifications_utils.pdf import pdf_page_count from app import ( current_service, @@ -44,6 +46,13 @@ def view_notification(service_id, notification_id): notification = notification_api_client.get_notification(service_id, str(notification_id)) notification['template'].update({'reply_to_text': notification['reply_to_text']}) + file_contents = view_letter_notification_as_preview(service_id, notification_id, "pdf") + + if notification['template']['is_precompiled_letter']: + page_count = pdf_page_count(io.BytesIO(file_contents)) + else: + page_count = get_page_count_for_letter(notification['template']) + template = get_template( notification['template'], current_service, @@ -53,7 +62,7 @@ def view_notification(service_id, notification_id): notification_id=notification_id, filetype='png', ), - page_count=get_page_count_for_letter(notification['template']), + page_count=page_count, show_recipient=True, redact_missing_personalisation=True, ) diff --git a/tests/app/main/views/test_notifications.py b/tests/app/main/views/test_notifications.py index 7e3f50e2b..a4034f4f6 100644 --- a/tests/app/main/views/test_notifications.py +++ b/tests/app/main/views/test_notifications.py @@ -320,8 +320,19 @@ def test_notification_page_has_expected_template_link_for_letter( has_template_link ): + mocker.patch( + 'app.main.views.notifications.view_letter_notification_as_preview', + return_value=b'foo' + ) + + mocker.patch( + 'app.main.views.notifications.pdf_page_count', + return_value=1 + ) + mock_get_notification( mocker, fake_uuid, template_type='letter', is_precompiled_letter=is_precompiled_letter) + mocker.patch( 'app.main.views.notifications.get_page_count_for_letter', return_value=1 @@ -339,3 +350,35 @@ def test_notification_page_has_expected_template_link_for_letter( assert link else: assert link is None + + +def test_should_show_image_of_precompiled_letter_notification( + logged_in_client, + fake_uuid, + mocker, +): + + mock_get_notification(mocker, fake_uuid, template_type='letter', is_precompiled_letter=True) + + mock_pdf_page_count = mocker.patch( + 'app.main.views.notifications.pdf_page_count', + return_value=1 + ) + + mocker.patch( + 'app.notify_client.notification_api_client.NotificationApiClient.get', + return_value={ + 'content': base64.b64encode(b'foo').decode('utf-8') + } + ) + + response = logged_in_client.get(url_for( + 'main.view_letter_notification_as_preview', + service_id=SERVICE_ONE_ID, + notification_id=fake_uuid, + filetype="png" + )) + + assert response.status_code == 200 + assert response.get_data(as_text=True) == 'foo' + assert mock_pdf_page_count.called_once() From 86a474ce8b297f03715b2d34a9f36d29072794c7 Mon Sep 17 00:00:00 2001 From: Richard Chapman Date: Fri, 9 Mar 2018 13:28:16 +0000 Subject: [PATCH 2/2] Moved the call to retrieve the pdf into the is_precompiled_letter block as it did not need to be called for standard letters. Changed the tests to use the mock from get_notification_letter_preview instead of a generic NotificationApiClient.get. This will hopefully protect any subsequent changes or calls from not being tested in future. --- app/main/views/notifications.py | 3 +-- tests/app/main/views/test_notifications.py | 6 +++--- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/app/main/views/notifications.py b/app/main/views/notifications.py index 3ba0bb706..ca27f94f0 100644 --- a/app/main/views/notifications.py +++ b/app/main/views/notifications.py @@ -46,9 +46,8 @@ def view_notification(service_id, notification_id): notification = notification_api_client.get_notification(service_id, str(notification_id)) notification['template'].update({'reply_to_text': notification['reply_to_text']}) - file_contents = view_letter_notification_as_preview(service_id, notification_id, "pdf") - if notification['template']['is_precompiled_letter']: + file_contents = view_letter_notification_as_preview(service_id, notification_id, "pdf") page_count = pdf_page_count(io.BytesIO(file_contents)) else: page_count = get_page_count_for_letter(notification['template']) diff --git a/tests/app/main/views/test_notifications.py b/tests/app/main/views/test_notifications.py index a4034f4f6..0adbe6908 100644 --- a/tests/app/main/views/test_notifications.py +++ b/tests/app/main/views/test_notifications.py @@ -166,7 +166,7 @@ def test_should_show_image_of_letter_notification( mock_get_notification(mocker, fake_uuid, template_type='letter') mocker.patch( - 'app.notify_client.notification_api_client.NotificationApiClient.get', + 'app.main.views.notifications.notification_api_client.get_notification_letter_preview', return_value={ 'content': base64.b64encode(b'foo').decode('utf-8') } @@ -192,7 +192,7 @@ def test_should_show_preview_error_image_letter_notification_on_preview_error( mock_get_notification(mocker, fake_uuid, template_type='letter') mocker.patch( - 'app.notify_client.notification_api_client.NotificationApiClient.get', + 'app.main.views.notifications.notification_api_client.get_notification_letter_preview', side_effect=APIError ) @@ -366,7 +366,7 @@ def test_should_show_image_of_precompiled_letter_notification( ) mocker.patch( - 'app.notify_client.notification_api_client.NotificationApiClient.get', + 'app.main.views.notifications.notification_api_client.get_notification_letter_preview', return_value={ 'content': base64.b64encode(b'foo').decode('utf-8') }