From d2918486e995bbff091ead53a4acabc49376d989 Mon Sep 17 00:00:00 2001
From: Chris Hill-Scott <me@quis.cc>
Date: Mon, 13 Feb 2017 12:08:20 +0000
Subject: [PATCH] Add error summary to whitelist page

> If both sections of the page have errors and the page is submitted,
> focus moves to the mobile numbers section so screen reader users may
> not be aware of preceding errors - focus should move to a dedicated
> error summary at the top of the page.

This commit adds the dedicate error summary at the top of the page,
following the GOV.UK Elements style from:
http://govuk-elements.herokuapp.com/errors/
---
 app/templates/views/api/whitelist.html | 28 +++++++++++++++++++++---
 tests/app/main/views/test_api_keys.py  | 30 ++++++++++++++++++++++++++
 2 files changed, 55 insertions(+), 3 deletions(-)

diff --git a/app/templates/views/api/whitelist.html b/app/templates/views/api/whitelist.html
index 3e7c25b6e..441c1b00c 100644
--- a/app/templates/views/api/whitelist.html
+++ b/app/templates/views/api/whitelist.html
@@ -1,4 +1,5 @@
 {% extends "withnav_template.html" %}
+{% from "components/banner.html" import banner_wrapper %}
 {% from "components/table.html" import list_table, field, hidden_field_heading %}
 {% from "components/api-key.html" import api_key %}
 {% from "components/page-footer.html" import page_footer %}
@@ -10,9 +11,30 @@
 
 {% block maincolumn_content %}
 
-  <h1 class="heading-large">
-    Whitelist
-  </h1>
+  {% if form.email_addresses.errors or form.phone_numbers.errors %}
+    {% call banner_wrapper(type='dangerous') %}
+      <h1 class='banner-title'>
+        There was a problem with your whitelist
+      </h1>
+      <p>Fix these errors:</p>
+      <ul>
+        {% if form.email_addresses.errors %}
+          <li>
+            <a href="#{{ form.email_addresses.name }}">Enter valid email addresses</a>
+          </li>
+        {% endif %}
+        {% if form.phone_numbers.errors %}
+          <li>
+            <a href="#{{ form.phone_numbers.name }}">Enter valid phone numbers</a>
+          </li>
+        {% endif %}
+      </ul>
+    {% endcall %}
+  {% else %}
+    <h1 class="heading-large">
+      Whitelist
+    </h1>
+  {% endif %}
 
   <p>
     You and members of
diff --git a/tests/app/main/views/test_api_keys.py b/tests/app/main/views/test_api_keys.py
index 38c1100ee..a3c809e67 100644
--- a/tests/app/main/views/test_api_keys.py
+++ b/tests/app/main/views/test_api_keys.py
@@ -318,3 +318,33 @@ def test_should_update_whitelist(
     mock_update_whitelist.assert_called_once_with(service_id, {
         'email_addresses': ['test@example.com', 'test@example.com'],
         'phone_numbers': ['07900900000']})
+
+
+def test_should_validate_whitelist_items(
+    logged_in_client,
+    mock_login,
+    api_user_active,
+    mock_get_service,
+    mock_has_permissions,
+    mock_update_whitelist
+):
+
+    response = logged_in_client.post(
+        url_for('main.whitelist', service_id=str(uuid.uuid4())),
+        data=OrderedDict([
+            ('email_addresses-1', 'abc'),
+            ('phone_numbers-0', '123')
+        ])
+    )
+
+    page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
+    assert page.h1.string.strip() == 'There was a problem with your whitelist'
+    jump_links = page.select('.banner-dangerous a')
+
+    assert jump_links[0].string.strip() == 'Enter valid email addresses'
+    assert jump_links[0]['href'] == '#email_addresses'
+
+    assert jump_links[1].string.strip() == 'Enter valid phone numbers'
+    assert jump_links[1]['href'] == '#phone_numbers'
+
+    mock_update_whitelist.assert_not_called()