From c898284c3b05a7287466ee139d41f516b73267ed Mon Sep 17 00:00:00 2001
From: Leo Hemsted <leo.hemsted@digital.cabinet-office.gov.uk>
Date: Mon, 15 Oct 2018 14:02:13 +0100
Subject: [PATCH] fix pdf download link trying to request a page count

It doesn't make sense to get a pdf for only one page - the template
preview app just returns a 400 if you try. So we shouldn't try!
---
 app/main/views/platform_admin.py  |  2 +-
 app/main/views/send.py            |  8 ++++++--
 tests/app/main/views/test_send.py | 30 ++++++++++++++++++++++++++++--
 3 files changed, 35 insertions(+), 5 deletions(-)

diff --git a/app/main/views/platform_admin.py b/app/main/views/platform_admin.py
index ce6bba892..d2eebb061 100644
--- a/app/main/views/platform_admin.py
+++ b/app/main/views/platform_admin.py
@@ -266,7 +266,7 @@ def platform_admin_letter_validation_preview():
             if response.status_code == 200:
                 pages, message, result = response.json()["pages"], response.json()["message"], response.json()["result"]
         except RequestException as error:
-            if error.response.status_code == 400:
+            if error.response and error.response.status_code == 400:
                 message = "Something was wrong with the file you tried to upload. Please upload a valid PDF file."
                 return render_template(
                     'views/platform-admin/letter-validation-preview.html',
diff --git a/app/main/views/send.py b/app/main/views/send.py
index 0ebbc0bbf..58fbcee67 100644
--- a/app/main/views/send.py
+++ b/app/main/views/send.py
@@ -656,13 +656,17 @@ def check_messages(service_id, template_id, upload_id, row_index=2):
 @login_required
 @user_has_permissions('send_messages')
 def check_messages_preview(service_id, template_id, upload_id, filetype, row_index=2):
-    if filetype not in ('pdf', 'png'):
+    if filetype == 'pdf':
+        page = None
+    elif filetype == 'png':
+        page = request.args.get('page', 1)
+    else:
         abort(404)
 
     template = _check_messages(
         service_id, template_id, upload_id, row_index, letters_as_pdf=True
     )['template']
-    return TemplatePreview.from_utils_template(template, filetype, page=request.args.get('page', 1))
+    return TemplatePreview.from_utils_template(template, filetype, page=page)
 
 
 @main.route("/services/<service_id>/start-job/<upload_id>", methods=['POST'])
diff --git a/tests/app/main/views/test_send.py b/tests/app/main/views/test_send.py
index 5010761b6..7f570d3cb 100644
--- a/tests/app/main/views/test_send.py
+++ b/tests/app/main/views/test_send.py
@@ -2060,19 +2060,43 @@ def test_can_start_letters_job(
     assert 'just_sent=yes' in response.location
 
 
-@pytest.mark.parametrize('filetype', ['pdf', 'png'])
-@pytest.mark.parametrize('extra_args, expected_values', [
+@pytest.mark.parametrize('filetype, extra_args, expected_values, expected_page', [
     (
+        'png',
         {},
         {'postcode': 'abc123', 'addressline1': '123 street'},
+        1
     ),
     (
+        'pdf',
+        {},
+        {'postcode': 'abc123', 'addressline1': '123 street'},
+        None
+    ),
+    (
+        'png',
         {'row_index': 2},
         {'postcode': 'abc123', 'addressline1': '123 street'},
+        1
     ),
     (
+        'png',
         {'row_index': 3},
         {'postcode': 'cba321', 'addressline1': '321 avenue'},
+        1
+    ),
+    (
+        'png',
+        {'row_index': 3, 'page': 2},
+        {'postcode': 'cba321', 'addressline1': '321 avenue'},
+        '2'
+    ),
+    (
+        # pdf expected page is always None
+        'pdf',
+        {'row_index': 3, 'page': 2},
+        {'postcode': 'cba321', 'addressline1': '321 avenue'},
+        None
     ),
 ])
 def test_should_show_preview_letter_message(
@@ -2087,6 +2111,7 @@ def test_should_show_preview_letter_message(
     mocker,
     extra_args,
     expected_values,
+    expected_page
 ):
     service_one['permissions'] = ['letter']
     mocker.patch('app.service_api_client.get_service', return_value={"data": service_one})
@@ -2135,6 +2160,7 @@ def test_should_show_preview_letter_message(
     assert type(mocked_preview.call_args[0][0]) == LetterPreviewTemplate
     assert mocked_preview.call_args[0][1] == filetype
     assert mocked_preview.call_args[0][0].values == expected_values
+    assert mocked_preview.call_args[1] == {'page': expected_page}
 
 
 def test_dont_show_preview_letter_templates_for_bad_filetype(