From f1a9761ea1d139f949c2224cbde2cbb76fb5d75e Mon Sep 17 00:00:00 2001
From: Chris Hill-Scott <me@quis.cc>
Date: Mon, 29 Oct 2018 12:11:47 +0000
Subject: [PATCH] Show letters that fail validation as cancelled
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

At the moment we show precompiled letters that have failed the
validation as having been sent. This is confusing.

We should communicate it as having been cancelled (rather than failed),
with the implication being that Notify has come along and cancelled the
letter before printing it. I think this is conceptually what makes the
most sense.

From the user’s point of view any letters that show up as cancelled
probably need to be fixed and resent, so it makes sense to group them
with the same name.
---
 app/templates/components/table.html              |  2 +-
 .../views/notifications/notification.html        |  5 +++++
 tests/app/main/views/test_activity.py            |  1 +
 tests/app/main/views/test_notifications.py       | 16 ++++++++++++++--
 4 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/app/templates/components/table.html b/app/templates/components/table.html
index 2c43ecbe2..c24f1ba6e 100644
--- a/app/templates/components/table.html
+++ b/app/templates/components/table.html
@@ -162,7 +162,7 @@
           notification.template.template_type
         ) }}
       {% endif %}
-      {% if notification.notification_type == "letter" and notification.status == 'permanent-failure' %}
+      {% if notification.notification_type == "letter" and notification.status in ['permanent-failure', 'validation-failed'] %}
         Cancelled
       {% endif %}
       {% if notification.status|format_notification_status_as_url(notification.notification_type) %}
diff --git a/app/templates/views/notifications/notification.html b/app/templates/views/notifications/notification.html
index 2c2e9788a..15fbc194a 100644
--- a/app/templates/views/notifications/notification.html
+++ b/app/templates/views/notifications/notification.html
@@ -39,6 +39,11 @@
         <p class="notification-status-cancelled">
           Cancelled {{ updated_at|format_datetime_short }}
         </p>
+      {% elif notification_status == 'validation-failed' %}
+        <p class="notification-status-cancelled">
+          Cancelled {{ updated_at|format_datetime_short }}
+          (letter has content outside the printable area)
+        </p>
       {% else %}
         <p>
           Postage: {{ postage }} class
diff --git a/tests/app/main/views/test_activity.py b/tests/app/main/views/test_activity.py
index e9727f171..f1e9623f0 100644
--- a/tests/app/main/views/test_activity.py
+++ b/tests/app/main/views/test_activity.py
@@ -547,6 +547,7 @@ def test_big_numbers_and_search_dont_show_for_letters(
         ('sms', 'delivered', 'Delivered 27 September at 5:31pm', True),
         ('letter', 'delivered', '27 September at 5:30pm', True),
         ('letter', 'permanent-failure', 'Cancelled 27 September at 5:31pm', False),
+        ('letter', 'validation-failed', 'Cancelled 27 September at 5:30pm', False),
     ]
 )
 def test_sending_status_hint_does_not_include_status_for_letters(
diff --git a/tests/app/main/views/test_notifications.py b/tests/app/main/views/test_notifications.py
index 82fc55e22..c89d0bae8 100644
--- a/tests/app/main/views/test_notifications.py
+++ b/tests/app/main/views/test_notifications.py
@@ -174,18 +174,30 @@ def test_notification_page_shows_page_for_letter_notification(
         )
 
 
+@pytest.mark.parametrize('notification_status, expected_message', (
+    (
+        'permanent-failure',
+        'Cancelled 1 January at 1:02am',
+    ),
+    (
+        'validation-failed',
+        'Cancelled 1 January at 1:02am (letter has content outside the printable area)',
+    ),
+))
 @freeze_time("2016-01-01 01:01")
 def test_notification_page_shows_cancelled_letter(
     client_request,
     mocker,
     fake_uuid,
+    notification_status,
+    expected_message,
 ):
 
     mock_get_notification(
         mocker,
         fake_uuid,
         template_type='letter',
-        notification_status='permanent-failure',
+        notification_status=notification_status,
     )
     mocker.patch(
         'app.main.views.notifications.get_page_count_for_letter',
@@ -202,7 +214,7 @@ def test_notification_page_shows_cancelled_letter(
         'sample template sent by Test User on 1 January at 1:01am'
     )
     assert normalize_spaces(page.select('main p')[1].text) == (
-        'Cancelled 1 January at 1:02am'
+        expected_message
     )
     assert not page.select('p.notification-status')