Merge branch 'add-error-summary'

app/assets/javascripts/main.js

@@ -5,3 +5,5 @@ $(() => GOVUK.modules.start());
 $(() => new GOVUK.SelectionButtons('.block-label input, .sms-message-option input'));
 
 $(() => $('.error-message').eq(0).parent('label').next('input').trigger('focus'));
+
+$(() => $('.banner-dangerous').eq(0).trigger('focus'));

app/assets/stylesheets/components/banner.scss

@@ -54,6 +54,10 @@
   margin: 15px 0;
   text-align: left;
 
+  &:focus {
+    outline: 3px solid $yellow;
+  }
+
   .button {
     @include button($error-colour);
     margin-top: 10px;

app/templates/components/banner.html

@@ -1,5 +1,11 @@
 {% macro banner(body, type=None, with_tick=False, delete_button=None, subhead=None) %}
-  <div class='banner{% if type %}-{{ type }}{% endif %}{% if with_tick %}-with-tick{% endif %}'>
+  <div
+    class='banner{% if type %}-{{ type }}{% endif %}{% if with_tick %}-with-tick{% endif %}'
+    {% if type == 'dangerous' %}
+    role='group'
+    tabindex='-1'
+    {% endif %}
+  >
     {% if subhead -%}
         {{ subhead }}&ensp;
     {%- endif -%}

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

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()