From 2291c4faf476aa7ea74185fa67fc580a03a84006 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Tue, 25 Jul 2017 22:57:01 +0100 Subject: [PATCH 1/2] Make sure errors on one-off flow have red border MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When I split up the error messages on the check CSV page into multiple templates, I also reduced the repetition of wrapping `
`s and macro calls by moving them up outside the conditional blocks (see 8e947f315d9b864f79ffd8771b36cd0061eacc3d). Unfortunately I didn’t make the same adjustments for the one-off flow, which meant that errors on these pages lost their styling. This commit re-adds the styling for these error messages. --- app/templates/views/notifications/check.html | 30 ++++++++++++++------ tests/app/main/views/test_send.py | 4 +-- 2 files changed, 23 insertions(+), 11 deletions(-) diff --git a/app/templates/views/notifications/check.html b/app/templates/views/notifications/check.html index e9f65c00d..66a840659 100644 --- a/app/templates/views/notifications/check.html +++ b/app/templates/views/notifications/check.html @@ -1,5 +1,5 @@ {% extends "withnav_template.html" %} -{% from "components/radios.html" import radio_select %} +{% from "components/banner.html" import banner_wrapper %} {% from "components/message-count-label.html" import message_count_label %} {% block service_page_title %} @@ -8,17 +8,29 @@ {% block maincolumn_content %} {% if error == 'not-allowed-to-send-to' %} - {% with - count_of_recipients=1, - template_type_label='phone number' - %} - {% include "partials/check/not-allowed-to-send-to.html" %} - {% endwith %} +
+ {% call banner_wrapper(type='dangerous') %} + {% with + count_of_recipients=1, + template_type_label='phone number' + %} + {% include "partials/check/not-allowed-to-send-to.html" %} + {% endwith %} + {% endcall %} +
{% elif error == 'too-many-messages' %} - {% include "partials/check/too-many-messages.html" %} +
+ {% call banner_wrapper(type='dangerous') %} + {% include "partials/check/too-many-messages.html" %} + {% endcall %} +
{% elif error == 'message-too-long' %} {# the only row_errors we can get when sending one off messages is that the message is too long #} - {% include "partials/check/message-too-long.html" %} +
+ {% call banner_wrapper(type='dangerous') %} + {% include "partials/check/message-too-long.html" %} + {% endcall %} +
{% else %}

Preview of {{ template.name }} diff --git a/tests/app/main/views/test_send.py b/tests/app/main/views/test_send.py index 39d0e05a0..8a9499ee5 100644 --- a/tests/app/main/views/test_send.py +++ b/tests/app/main/views/test_send.py @@ -1937,6 +1937,6 @@ def test_send_notification_shows_error_if_400( _expected_status=200 ) - assert ' '.join(page.h1.text.split()) == expected_h1 - assert ' '.join(page.h1.parent.p.text.split()) == expected_err_details + assert normalize_spaces(page.select('.banner-dangerous h1')[0].text) == expected_h1 + assert normalize_spaces(page.select('.banner-dangerous p')[0].text) == expected_err_details assert not page.find('input[type=submit]') From a58d8816474b3509c372f49586b060a347ab32ed Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Tue, 25 Jul 2017 23:06:55 +0100 Subject: [PATCH 2/2] Make one-off error refer to correct message type MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This was hard coded to say ‘You can’t send to this phone number’ even when you were trying to send an email. --- app/templates/views/notifications/check.html | 4 ++- tests/app/main/views/test_send.py | 29 ++++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/app/templates/views/notifications/check.html b/app/templates/views/notifications/check.html index 66a840659..19c5d34e4 100644 --- a/app/templates/views/notifications/check.html +++ b/app/templates/views/notifications/check.html @@ -12,7 +12,9 @@ {% call banner_wrapper(type='dangerous') %} {% with count_of_recipients=1, - template_type_label='phone number' + template_type_label=( + 'phone number' if template.template_type == 'sms' else 'email address' + ) %} {% include "partials/check/not-allowed-to-send-to.html" %} {% endwith %} diff --git a/tests/app/main/views/test_send.py b/tests/app/main/views/test_send.py index 8a9499ee5..f025cae88 100644 --- a/tests/app/main/views/test_send.py +++ b/tests/app/main/views/test_send.py @@ -1940,3 +1940,32 @@ def test_send_notification_shows_error_if_400( assert normalize_spaces(page.select('.banner-dangerous h1')[0].text) == expected_h1 assert normalize_spaces(page.select('.banner-dangerous p')[0].text) == expected_err_details assert not page.find('input[type=submit]') + + +def test_send_notification_shows_email_error_in_trial_mode( + client_request, + fake_uuid, + mocker, + mock_get_service_email_template, +): + mocker.patch( + 'app.notification_api_client.send_notification', + side_effect=HTTPError(response=Mock(status_code=400), message=TRIAL_MODE_MSG) + ) + with client_request.session_transaction() as session: + session['recipient'] = 'test@example.com' + session['placeholders'] = {'date': 'foo', 'thing': 'bar'} + + page = client_request.post( + 'main.send_notification', + service_id=SERVICE_ONE_ID, + template_id=fake_uuid, + _expected_status=200, + ) + + assert normalize_spaces(page.select('.banner-dangerous h1')[0].text) == ( + 'You can’t send to this email address' + ) + assert normalize_spaces(page.select('.banner-dangerous p')[0].text) == ( + 'In trial mode you can only send to yourself and members of your team' + )