From 0252a1792aea1e0fd85cc717f2dbaede3882cd2b Mon Sep 17 00:00:00 2001
From: Chris Hill-Scott <me@quis.cc>
Date: Mon, 22 Aug 2016 10:44:45 +0100
Subject: [PATCH 01/15] Split platform admin settings into new section

Makes it clearer what a normal user can change and what a platform
admin can change.
---
 app/templates/views/service-settings.html | 47 +++++++++++++----------
 1 file changed, 27 insertions(+), 20 deletions(-)

diff --git a/app/templates/views/service-settings.html b/app/templates/views/service-settings.html
index ccb2804d7..936de2548 100644
--- a/app/templates/views/service-settings.html
+++ b/app/templates/views/service-settings.html
@@ -18,10 +18,6 @@
         'title': 'Set email reply to address',
         'link': url_for('.service_set_reply_to_email', service_id=current_service.id)
       },
-      {
-        'title': 'Set email branding',
-        'link': url_for('.service_set_branding_and_org', service_id=current_service.id)
-      } if current_user.has_permissions([], admin_override=True) else {},
       {
         'title': 'Set text message sender name',
         'link': url_for('.service_set_sms_sender', service_id=current_service.id)
@@ -39,23 +35,34 @@
       } if not current_service.active else {
         'title': 'Reactivate API keys',
         'link': url_for('.service_status_change', service_id=current_service.id)
-      },
-      {
-        'title': 'Make service live',
-        'link': url_for('.service_switch_live', service_id=current_service.id)
-      } if current_service.restricted and current_user.has_permissions([], admin_override=True) else {
-        'title': 'Revert service to trial',
-        'link': url_for('.service_switch_live', service_id=current_service.id)
-      } if not current_service.restricted and current_user.has_permissions([], admin_override=True) else {
-      },
-      {
-        'title': 'Put service into research mode',
-        'link': url_for('.service_switch_research_mode', service_id=current_service.id)
-      } if not current_service.research_mode and current_user.has_permissions([], admin_override=True) else {
-        'title': 'Take service out of research mode',
-        'link': url_for('.service_switch_research_mode', service_id=current_service.id)
-      } if current_service.research_mode and current_user.has_permissions([], admin_override=True) else {
       }
     ]) }}
 
+    {% if current_user.has_permissions([], admin_override=True) %}
+
+      <h2 class="heading-large">Platform admin settings</h2>
+
+      {{ browse_list([
+        {
+          'title': 'Set email branding',
+          'link': url_for('.service_set_branding_and_org', service_id=current_service.id)
+        },
+        {
+          'title': 'Make service live',
+          'link': url_for('.service_switch_live', service_id=current_service.id)
+        } if current_service.restricted else {
+          'title': 'Revert service to trial',
+          'link': url_for('.service_switch_live', service_id=current_service.id)
+        },
+        {
+          'title': 'Put service into research mode',
+          'link': url_for('.service_switch_research_mode', service_id=current_service.id)
+        } if not current_service.research_mode else {
+          'title': 'Take service out of research mode',
+          'link': url_for('.service_switch_research_mode', service_id=current_service.id)
+        }
+      ]) }}
+
+    {% endif %}
+
 {% endblock %}

From 1691c1a821ee42106ecc92017df07a787d1f0b5e Mon Sep 17 00:00:00 2001
From: Chris Hill-Scott <me@quis.cc>
Date: Mon, 22 Aug 2016 10:45:39 +0100
Subject: [PATCH 02/15] Make page <title> match navigation and <h1>

---
 app/templates/views/service-settings.html     | 2 +-
 tests/app/main/views/test_service_settings.py | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/templates/views/service-settings.html b/app/templates/views/service-settings.html
index 936de2548..404ec396f 100644
--- a/app/templates/views/service-settings.html
+++ b/app/templates/views/service-settings.html
@@ -2,7 +2,7 @@
 {% from "components/browse-list.html" import browse_list %}
 
 {% block page_title %}
-  Service settings – GOV.UK Notify
+  Settings – GOV.UK Notify
 {% endblock %}
 
 {% block maincolumn_content %}
diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py
index 77817e033..969f5c454 100644
--- a/tests/app/main/views/test_service_settings.py
+++ b/tests/app/main/views/test_service_settings.py
@@ -20,7 +20,7 @@ def test_should_show_overview(app_,
                 'main.service_settings', service_id=service_one['id']))
         assert response.status_code == 200
         resp_data = response.get_data(as_text=True)
-        assert 'Service settings' in resp_data
+        assert 'Settings' in resp_data
         app.service_api_client.get_service.assert_called_with(service_one['id'])
 
 

From 3719cdd115f7dd523cb8e5f64ecaf1492cb795c0 Mon Sep 17 00:00:00 2001
From: Chris Hill-Scott <me@quis.cc>
Date: Mon, 22 Aug 2016 11:43:35 +0100
Subject: [PATCH 03/15] Make settings page a table not a browse list
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

There’s no way of seeing what the current settings are for a service
without individually going to the pages where you can change them.

This commit replaces the list of settings with a table. The table plays
back the current value for each setting.

This is a pattern that has worked well on various services[1] as well
as on our own user profile page.

1. https://designpatterns.hackpad.com/Check-your-answers-page-2DSpTH9J0wU
---
 app/templates/views/service-settings.html     | 66 +++++++++++--------
 tests/app/main/views/test_service_settings.py | 54 +++++++++++----
 tests/conftest.py                             | 11 ++++
 3 files changed, 90 insertions(+), 41 deletions(-)

diff --git a/app/templates/views/service-settings.html b/app/templates/views/service-settings.html
index 404ec396f..ae09f5e8e 100644
--- a/app/templates/views/service-settings.html
+++ b/app/templates/views/service-settings.html
@@ -1,5 +1,6 @@
 {% extends "withnav_template.html" %}
 {% from "components/browse-list.html" import browse_list %}
+{% from "components/table.html" import mapping_table, row, text_field, link_field %}
 
 {% block page_title %}
   Settings – GOV.UK Notify
@@ -9,34 +10,43 @@
 
     <h1 class="heading-large">Settings</h1>
 
-    {{ browse_list([
-      {
-        'title': 'Change your service name',
-        'link': url_for('.service_name_change', service_id=current_service.id)
-      },
-      {
-        'title': 'Set email reply to address',
-        'link': url_for('.service_set_reply_to_email', service_id=current_service.id)
-      },
-      {
-        'title': 'Set text message sender name',
-        'link': url_for('.service_set_sms_sender', service_id=current_service.id)
-      },
-      {
-        'title': 'Request to go live and turn off trial mode',
-        'link': url_for('.service_request_to_go_live', service_id=current_service.id),
-        'hint': 'A live service can send notifications to any phone number or email address',
-      } if current_service.restricted else {
-      },
-      {
-        'title': 'Temporarily suspend API keys',
-        'link': url_for('.service_status_change', service_id=current_service.id),
-        'destructive': True
-      } if not current_service.active else {
-        'title': 'Reactivate API keys',
-        'link': url_for('.service_status_change', service_id=current_service.id)
-      }
-    ]) }}
+    {% call mapping_table(
+      caption='Settings',
+      field_headings=['Label', 'Value', 'Action'],
+      field_headings_visible=False,
+      caption_visible=False
+    ) %}
+      {% call row() %}
+        {{ text_field('Service name' )}}
+        {{ text_field(current_service.name) }}
+        {{ link_field('Change', url_for('.service_name_change', service_id=current_service.id)) }}
+      {% endcall %}
+      {% call row() %}
+        {{ text_field('Email reply to address')}}
+        {{ text_field(current_service.reply_to_email_address or '{}@notifications.service.gov.uk'.format(current_service.email_from)) }}
+        {{ link_field('Change', url_for('.service_set_reply_to_email', service_id=current_service.id)) }}
+      {% endcall %}
+      {% call row() %}
+        {{ text_field('Text message sender')}}
+        {{ text_field(current_service.sms_sender or '40604') }}
+        {{ link_field('Change', url_for('.service_set_sms_sender', service_id=current_service.id)) }}
+      {% endcall %}
+      {% call row() %}
+        {{ text_field('Mode')}}
+        {% if current_service.restricted %}
+          {{ text_field('Trial') }}
+          {{ link_field('Go live', url_for('.service_request_to_go_live', service_id=current_service.id)) }}
+        {% else %}
+          {{ text_field('Live') }}
+          {{ link_field() }}
+        {% endif %}
+      {% endcall %}
+      {% call row() %}
+        {{ text_field('Active')}}
+        {{ text_field(current_service.active) }}
+        {{ link_field('Suspend', url_for('.service_status_change', service_id=current_service.id)) }}
+      {% endcall %}
+    {% endcall %}
 
     {% if current_user.has_permissions([], admin_override=True) %}
 
diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py
index 969f5c454..919a75c9d 100644
--- a/tests/app/main/views/test_service_settings.py
+++ b/tests/app/main/views/test_service_settings.py
@@ -9,19 +9,47 @@ from unittest.mock import ANY, Mock
 from werkzeug.exceptions import InternalServerError
 
 
-def test_should_show_overview(app_,
-                              active_user_with_permissions,
-                              mocker,
-                              service_one):
-    with app_.test_request_context():
-        with app_.test_client() as client:
-            client.login(active_user_with_permissions, mocker, service_one)
-            response = client.get(url_for(
-                'main.service_settings', service_id=service_one['id']))
-        assert response.status_code == 200
-        resp_data = response.get_data(as_text=True)
-        assert 'Settings' in resp_data
-        app.service_api_client.get_service.assert_called_with(service_one['id'])
+def test_should_show_overview(
+    app_,
+    active_user_with_permissions,
+    mocker,
+    service_one
+):
+    with app_.test_request_context(), app_.test_client() as client:
+        client.login(active_user_with_permissions, mocker, service_one)
+        response = client.get(url_for(
+            'main.service_settings', service_id=service_one['id']
+        ))
+    assert response.status_code == 200
+    page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
+    assert page.find('h1').text == 'Settings'
+    for index, row in enumerate([
+        'Service name service one Change',
+        'Email reply to address None Change',
+        'Text message sender 40604 Change'
+    ]):
+        assert row == " ".join(page.find_all('tr')[index + 1].text.split())
+    app.service_api_client.get_service.assert_called_with(service_one['id'])
+
+
+def test_should_show_overview_for_service_with_more_things_set(
+    app_,
+    active_user_with_permissions,
+    mocker,
+    service_with_reply_to_addresses
+):
+    with app_.test_request_context(), app_.test_client() as client:
+        client.login(active_user_with_permissions, mocker, service_with_reply_to_addresses)
+        response = client.get(url_for(
+            'main.service_settings', service_id=service_with_reply_to_addresses['id']
+        ))
+    page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
+    for index, row in enumerate([
+        'Service name service one Change',
+        'Email reply to address test@example.com Change',
+        'Text message sender elevenchars Change'
+    ]):
+        assert row == " ".join(page.find_all('tr')[index + 1].text.split())
 
 
 def test_should_show_service_name(app_,
diff --git a/tests/conftest.py b/tests/conftest.py
index 12b9e95b5..68b43b34a 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -43,6 +43,17 @@ def service_one(api_user_active):
     return service_json(SERVICE_ONE_ID, 'service one', [api_user_active.id])
 
 
+@pytest.fixture(scope='function')
+def service_with_reply_to_addresses(api_user_active):
+    return service_json(
+        SERVICE_ONE_ID,
+        'service one',
+        [api_user_active.id],
+        reply_to_email_address='test@example.com',
+        sms_sender='elevenchars',
+    )
+
+
 @pytest.fixture(scope='function')
 def mock_send_sms(request, mocker):
     return mocker.patch("app.service_api_client.send_sms")

From f84e8e2519cfcac2a220d5694c21756be2ab2814 Mon Sep 17 00:00:00 2001
From: Chris Hill-Scott <me@quis.cc>
Date: Tue, 23 Aug 2016 15:29:46 +0100
Subject: [PATCH 04/15] Remove flash messages when changing settings
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Since we’re playing back the values in the table there’s no need to
give confirmation of the new value in a flash message.
---
 app/main/views/service_settings.py            |  7 -----
 tests/app/main/views/test_service_settings.py | 28 -------------------
 2 files changed, 35 deletions(-)

diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py
index a49af5924..746c4de3f 100644
--- a/app/main/views/service_settings.py
+++ b/app/main/views/service_settings.py
@@ -233,12 +233,10 @@ def service_set_reply_to_email(service_id):
     if request.method == 'GET':
         form.email_address.data = current_service.get('reply_to_email_address')
     if form.validate_on_submit():
-        message = 'Reply to email set to {}'.format(form.email_address.data)
         service_api_client.update_service(
             current_service['id'],
             reply_to_email_address=form.email_address.data
         )
-        flash(message, 'default_with_tick')
         return redirect(url_for('.service_settings', service_id=service_id))
     return render_template(
         'views/service-settings/set-reply-to-email.html',
@@ -253,15 +251,10 @@ def service_set_sms_sender(service_id):
     if request.method == 'GET':
         form.sms_sender.data = current_service.get('sms_sender')
     if form.validate_on_submit():
-        if form.sms_sender.data:
-            message = 'Text message sender set to {}'.format(form.sms_sender.data)
-        else:
-            message = 'Text message sender removed'
         service_api_client.update_service(
             current_service['id'],
             sms_sender=form.sms_sender.data or None
         )
-        flash(message, 'default_with_tick')
         return redirect(url_for('.service_settings', service_id=service_id))
     return render_template(
         'views/service-settings/set-sms-sender.html',
diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py
index 919a75c9d..ee81e2d24 100644
--- a/tests/app/main/views/test_service_settings.py
+++ b/tests/app/main/views/test_service_settings.py
@@ -765,34 +765,6 @@ def test_if_sms_sender_set_then_form_populated(app_,
         assert page.find(id='sms_sender')['value'] == 'elevenchars'
 
 
-@pytest.mark.parametrize("sender, expected_flash_message", [
-    ("elevenchars", 'Text message sender set to elevenchars'),
-    ('', 'Text message sender removed')
-])
-def test_set_text_message_sender_flash_messages(
-        app_,
-        active_user_with_permissions,
-        mocker,
-        mock_update_service,
-        service_one,
-        sender,
-        expected_flash_message):
-    with app_.test_request_context():
-        with app_.test_client() as client:
-            client.login(active_user_with_permissions, mocker, service_one)
-            data = {"sms_sender": sender}
-            response = client.post(url_for('main.service_set_sms_sender', service_id=service_one['id']),
-                                   data=data,
-                                   follow_redirects=True)
-
-            assert response.status_code == 200
-
-            page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
-            element = page.find('div', {"class": "banner-default-with-tick"})
-
-            assert element.text.strip() == expected_flash_message
-
-
 def test_should_show_branding(
     mocker, app_, platform_admin_user, service_one, mock_get_organisations
 ):

From fc93d9ff525aed8d00157f4b53b108548658d11f Mon Sep 17 00:00:00 2001
From: Chris Hill-Scott <me@quis.cc>
Date: Mon, 22 Aug 2016 13:49:41 +0100
Subject: [PATCH 05/15] =?UTF-8?q?Add=20an=20=E2=80=98edit=E2=80=99=20field?=
 =?UTF-8?q?=20type?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Like the link field, but right aligns so that it sits nicely in the
last column of a table.
---
 app/templates/components/table.html       |  6 ++++++
 app/templates/views/service-settings.html | 16 ++++++++--------
 2 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/app/templates/components/table.html b/app/templates/components/table.html
index 95ba0264f..ba4266d0b 100644
--- a/app/templates/components/table.html
+++ b/app/templates/components/table.html
@@ -85,6 +85,12 @@
   {% endcall %}
 {%- endmacro %}
 
+{% macro edit_field(text, link) -%}
+  {% call field(align='right') %}
+    <a href="{{ link }}">{{ text }}</a>
+  {% endcall %}
+{%- endmacro %}
+
 {% macro boolean_field(yes) -%}
   {% call field(status='yes' if yes else 'no') %}
     {{ "Yes" if yes else "No" }}
diff --git a/app/templates/views/service-settings.html b/app/templates/views/service-settings.html
index ae09f5e8e..b66d838e4 100644
--- a/app/templates/views/service-settings.html
+++ b/app/templates/views/service-settings.html
@@ -1,6 +1,6 @@
 {% extends "withnav_template.html" %}
 {% from "components/browse-list.html" import browse_list %}
-{% from "components/table.html" import mapping_table, row, text_field, link_field %}
+{% from "components/table.html" import mapping_table, row, text_field, edit_field %}
 
 {% block page_title %}
   Settings – GOV.UK Notify
@@ -19,32 +19,32 @@
       {% call row() %}
         {{ text_field('Service name' )}}
         {{ text_field(current_service.name) }}
-        {{ link_field('Change', url_for('.service_name_change', service_id=current_service.id)) }}
+        {{ edit_field('Change', url_for('.service_name_change', service_id=current_service.id)) }}
       {% endcall %}
       {% call row() %}
         {{ text_field('Email reply to address')}}
-        {{ text_field(current_service.reply_to_email_address or '{}@notifications.service.gov.uk'.format(current_service.email_from)) }}
-        {{ link_field('Change', url_for('.service_set_reply_to_email', service_id=current_service.id)) }}
+        {{ text_field(current_service.reply_to_email_address) }}
+        {{ edit_field('Change', url_for('.service_set_reply_to_email', service_id=current_service.id)) }}
       {% endcall %}
       {% call row() %}
         {{ text_field('Text message sender')}}
         {{ text_field(current_service.sms_sender or '40604') }}
-        {{ link_field('Change', url_for('.service_set_sms_sender', service_id=current_service.id)) }}
+        {{ edit_field('Change', url_for('.service_set_sms_sender', service_id=current_service.id)) }}
       {% endcall %}
       {% call row() %}
         {{ text_field('Mode')}}
         {% if current_service.restricted %}
           {{ text_field('Trial') }}
-          {{ link_field('Go live', url_for('.service_request_to_go_live', service_id=current_service.id)) }}
+          {{ edit_field('Go live', url_for('.service_request_to_go_live', service_id=current_service.id)) }}
         {% else %}
           {{ text_field('Live') }}
-          {{ link_field() }}
+          {{ edit_field() }}
         {% endif %}
       {% endcall %}
       {% call row() %}
         {{ text_field('Active')}}
         {{ text_field(current_service.active) }}
-        {{ link_field('Suspend', url_for('.service_status_change', service_id=current_service.id)) }}
+        {{ edit_field('Suspend', url_for('.service_status_change', service_id=current_service.id)) }}
       {% endcall %}
     {% endcall %}
 

From 036f2a88803f7dd78cf72a7c643ca5c98e8454c2 Mon Sep 17 00:00:00 2001
From: Chris Hill-Scott <me@quis.cc>
Date: Mon, 22 Aug 2016 14:05:42 +0100
Subject: [PATCH 06/15] Allow `text_field`s to have an optional status

Status is used for making the text greyed-out or red, etc.
---
 app/templates/components/table.html       | 4 ++--
 app/templates/views/service-settings.html | 5 ++++-
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/app/templates/components/table.html b/app/templates/components/table.html
index ba4266d0b..bf8893592 100644
--- a/app/templates/components/table.html
+++ b/app/templates/components/table.html
@@ -73,8 +73,8 @@
   </td>
 {% endmacro %}
 
-{% macro text_field(text) -%}
-  {% call field() %}
+{% macro text_field(text, status='') -%}
+  {% call field(status=status) %}
     {{ text }}
   {% endcall %}
 {%- endmacro %}
diff --git a/app/templates/views/service-settings.html b/app/templates/views/service-settings.html
index b66d838e4..14c5d28fd 100644
--- a/app/templates/views/service-settings.html
+++ b/app/templates/views/service-settings.html
@@ -23,7 +23,10 @@
       {% endcall %}
       {% call row() %}
         {{ text_field('Email reply to address')}}
-        {{ text_field(current_service.reply_to_email_address) }}
+        {{ text_field(
+          current_service.reply_to_email_address,
+          status='' if current_service.reply_to_email_address else 'default'
+        ) }}
         {{ edit_field('Change', url_for('.service_set_reply_to_email', service_id=current_service.id)) }}
       {% endcall %}
       {% call row() %}

From 05a493f1e867e77a1cfad755654824a2373fa16c Mon Sep 17 00:00:00 2001
From: Chris Hill-Scott <me@quis.cc>
Date: Mon, 22 Aug 2016 14:39:57 +0100
Subject: [PATCH 07/15] =?UTF-8?q?Remove=20=E2=80=98suspend=20API=20keys?=
 =?UTF-8?q?=E2=80=99?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This was an early reckon feature. There were a few of problems with
it:
- it worked on the service, not just on the API keys as described
- it was back to front, ‘suspending’ a service set `active` to `True`,
  reactivating it set `active` to `False`
- no part of the API actually respected the `active` flag on a service

The same intent can be acheived by either:
- revoking an API key
- having a platform admin put your service back into trial mode

So this commit removes the link and the code behind it.
---
 app/main/views/service_settings.py            | 35 -------
 app/templates/views/service-settings.html     | 85 +++++++++--------
 .../views/service-settings/status.html        | 38 --------
 tests/app/main/views/test_service_settings.py | 95 -------------------
 4 files changed, 46 insertions(+), 207 deletions(-)
 delete mode 100644 app/templates/views/service-settings/status.html

diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py
index 746c4de3f..301aafd61 100644
--- a/app/main/views/service_settings.py
+++ b/app/main/views/service_settings.py
@@ -157,41 +157,6 @@ def service_switch_research_mode(service_id):
     return redirect(url_for('.service_settings', service_id=service_id))
 
 
-@main.route("/services/<service_id>/service-settings/status", methods=['GET', 'POST'])
-@login_required
-@user_has_permissions('manage_settings', admin_override=True)
-def service_status_change(service_id):
-    if request.method == 'GET':
-        return render_template(
-            'views/service-settings/status.html'
-        )
-    elif request.method == 'POST':
-        return redirect(url_for('.service_status_change_confirm', service_id=service_id))
-
-
-@main.route("/services/<service_id>/service-settings/status/confirm", methods=['GET', 'POST'])
-@login_required
-@user_has_permissions('manage_settings', admin_override=True)
-def service_status_change_confirm(service_id):
-    # Validate password for form
-    def _check_password(pwd):
-        return user_api_client.verify_password(current_user.id, pwd)
-
-    form = ConfirmPasswordForm(_check_password)
-
-    if form.validate_on_submit():
-        service_api_client.update_service(
-            current_service['id'],
-            active=True
-        )
-        return redirect(url_for('.service_settings', service_id=service_id))
-    return render_template(
-        'views/service-settings/confirm.html',
-        heading='Turn off all outgoing notifications',
-        destructive=True,
-        form=form)
-
-
 @main.route("/services/<service_id>/service-settings/delete", methods=['GET', 'POST'])
 @login_required
 @user_has_permissions('manage_settings', admin_override=True)
diff --git a/app/templates/views/service-settings.html b/app/templates/views/service-settings.html
index 14c5d28fd..07b0b243e 100644
--- a/app/templates/views/service-settings.html
+++ b/app/templates/views/service-settings.html
@@ -1,6 +1,6 @@
 {% extends "withnav_template.html" %}
 {% from "components/browse-list.html" import browse_list %}
-{% from "components/table.html" import mapping_table, row, text_field, edit_field %}
+{% from "components/table.html" import mapping_table, row, text_field, edit_field, field %}
 
 {% block page_title %}
   Settings – GOV.UK Notify
@@ -10,46 +10,53 @@
 
     <h1 class="heading-large">Settings</h1>
 
-    {% call mapping_table(
-      caption='Settings',
-      field_headings=['Label', 'Value', 'Action'],
-      field_headings_visible=False,
-      caption_visible=False
-    ) %}
-      {% call row() %}
-        {{ text_field('Service name' )}}
-        {{ text_field(current_service.name) }}
-        {{ edit_field('Change', url_for('.service_name_change', service_id=current_service.id)) }}
+    <div class="dashboard-table">
+
+      {% call mapping_table(
+        caption='Settings',
+        field_headings=['Label', 'Value', 'Action'],
+        field_headings_visible=False,
+        caption_visible=False
+      ) %}
+        {% call row() %}
+          {{ text_field('Service name' )}}
+          {{ text_field(current_service.name) }}
+          {{ edit_field('Change', url_for('.service_name_change', service_id=current_service.id)) }}
+        {% endcall %}
+        {% call row() %}
+          {{ text_field('Email reply to address')}}
+          {{ text_field(
+            current_service.reply_to_email_address,
+            status='' if current_service.reply_to_email_address else 'default'
+          ) }}
+          {{ edit_field('Change', url_for('.service_set_reply_to_email', service_id=current_service.id)) }}
+        {% endcall %}
+        {% call row() %}
+          {{ text_field('Text message sender')}}
+          {{ text_field(current_service.sms_sender or '40604') }}
+          {{ edit_field('Change', url_for('.service_set_sms_sender', service_id=current_service.id)) }}
+        {% endcall %}
       {% endcall %}
-      {% call row() %}
-        {{ text_field('Email reply to address')}}
-        {{ text_field(
-          current_service.reply_to_email_address,
-          status='' if current_service.reply_to_email_address else 'default'
-        ) }}
-        {{ edit_field('Change', url_for('.service_set_reply_to_email', service_id=current_service.id)) }}
+
+      {% call mapping_table(
+        caption='Restrictions',
+        field_headings=['Label', 'Value', 'Action'],
+        field_headings_visible=False,
+        caption_visible=True
+      ) %}
+        {% call row() %}
+          {% if current_service.restricted %}
+            {{ text_field('Trial mode') }}
+            {{ text_field('On') }}
+            {{ edit_field('Request to go live', url_for('.service_request_to_go_live', service_id=current_service.id)) }}
+          {% else %}
+            {{ text_field('Trial mode') }}
+            {{ text_field('Off')}}
+            {{ edit_field() }}
+          {% endif %}
+        {% endcall %}
       {% endcall %}
-      {% call row() %}
-        {{ text_field('Text message sender')}}
-        {{ text_field(current_service.sms_sender or '40604') }}
-        {{ edit_field('Change', url_for('.service_set_sms_sender', service_id=current_service.id)) }}
-      {% endcall %}
-      {% call row() %}
-        {{ text_field('Mode')}}
-        {% if current_service.restricted %}
-          {{ text_field('Trial') }}
-          {{ edit_field('Go live', url_for('.service_request_to_go_live', service_id=current_service.id)) }}
-        {% else %}
-          {{ text_field('Live') }}
-          {{ edit_field() }}
-        {% endif %}
-      {% endcall %}
-      {% call row() %}
-        {{ text_field('Active')}}
-        {{ text_field(current_service.active) }}
-        {{ edit_field('Suspend', url_for('.service_status_change', service_id=current_service.id)) }}
-      {% endcall %}
-    {% endcall %}
+    </div>
 
     {% if current_user.has_permissions([], admin_override=True) %}
 
diff --git a/app/templates/views/service-settings/status.html b/app/templates/views/service-settings/status.html
deleted file mode 100644
index d2648a5d7..000000000
--- a/app/templates/views/service-settings/status.html
+++ /dev/null
@@ -1,38 +0,0 @@
-{% extends "withnav_template.html" %}
-{% from "components/page-footer.html" import page_footer %}
-
-{% block page_title %}
-  Temporrily suspend API keys – GOV.UK Notify
-{% endblock %}
-
-{% block maincolumn_content %}
-
-  <h1 class="heading-large">Temporarily suspend API keys</h1>
-
-  <div class="grid-row">
-    <div class="column-three-quarters">
-
-      <p>
-        You’ll still be able to send notifications to yourself by uploading a
-        CSV file.
-      </p>
-
-      <p>
-        You can start sending notifications again when you’re ready.
-      </p>
-
-      <form method="post">
-        {{ page_footer(
-          'Suspend API keys',
-          destructive=True,
-          back_link=url_for('.service_settings', service_id=current_service.id),
-          back_link_text='Back to settings'
-        ) }}
-      </form>
-
-    </div>
-  </div>
-
-
-
-{% endblock %}
diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py
index ee81e2d24..6f9585f62 100644
--- a/tests/app/main/views/test_service_settings.py
+++ b/tests/app/main/views/test_service_settings.py
@@ -321,95 +321,6 @@ def test_log_error_on_request_to_go_live(
             )
 
 
-def test_should_show_status_page(app_,
-                                 api_user_active,
-                                 mock_get_service,
-                                 mock_get_user,
-                                 mock_get_user_by_email,
-                                 mock_login,
-                                 mock_has_permissions,
-                                 fake_uuid):
-    with app_.test_request_context():
-        with app_.test_client() as client:
-            client.login(api_user_active)
-            service_id = fake_uuid
-            response = client.get(url_for(
-                'main.service_status_change', service_id=service_id))
-
-        assert response.status_code == 200
-        resp_data = response.get_data(as_text=True)
-        assert 'Suspend API keys' in resp_data
-        assert mock_get_service.called
-
-
-def test_should_show_redirect_after_status_change(app_,
-                                                  api_user_active,
-                                                  mock_get_service,
-                                                  mock_get_user,
-                                                  mock_get_user_by_email,
-                                                  mock_login,
-                                                  mock_has_permissions,
-                                                  fake_uuid):
-    with app_.test_request_context():
-        with app_.test_client() as client:
-            client.login(api_user_active)
-            service_id = fake_uuid
-            response = client.post(url_for(
-                'main.service_status_change', service_id=service_id))
-
-        assert response.status_code == 302
-        redirect_url = url_for(
-            'main.service_status_change_confirm', service_id=service_id, _external=True)
-        assert redirect_url == response.location
-        assert mock_get_service.called
-
-
-def test_should_show_status_confirmation(app_,
-                                         api_user_active,
-                                         mock_get_service,
-                                         mock_get_user,
-                                         mock_get_user_by_email,
-                                         mock_login,
-                                         mock_has_permissions,
-                                         fake_uuid):
-    with app_.test_request_context():
-        with app_.test_client() as client:
-            client.login(api_user_active)
-            service_id = fake_uuid
-            response = client.get(url_for(
-                'main.service_status_change_confirm', service_id=service_id))
-
-        assert response.status_code == 200
-        resp_data = response.get_data(as_text=True)
-        assert 'Turn off all outgoing notifications' in resp_data
-        assert mock_get_service.called
-
-
-def test_should_redirect_after_status_confirmation(app_,
-                                                   api_user_active,
-                                                   mock_get_service,
-                                                   mock_update_service,
-                                                   mock_get_user,
-                                                   mock_get_user_by_email,
-                                                   mock_login,
-                                                   mock_verify_password,
-                                                   mock_has_permissions,
-                                                   fake_uuid):
-    with app_.test_request_context():
-        with app_.test_client() as client:
-            client.login(api_user_active)
-            service_id = fake_uuid
-            response = client.post(url_for(
-                'main.service_status_change_confirm', service_id=service_id))
-
-        assert response.status_code == 302
-        settings_url = url_for(
-            'main.service_settings', service_id=service_id, _external=True)
-        assert settings_url == response.location
-        assert mock_get_service.called
-        assert mock_update_service.called
-
-
 def test_should_show_delete_page(app_,
                                  api_user_active,
                                  mock_login,
@@ -502,8 +413,6 @@ def test_route_permissions(mocker, app_, api_user_active, service_one):
         'main.service_name_change',
         'main.service_name_change_confirm',
         'main.service_request_to_go_live',
-        'main.service_status_change',
-        'main.service_status_change_confirm',
         'main.service_delete',
         'main.service_delete_confirm']
     with app_.test_request_context():
@@ -527,8 +436,6 @@ def test_route_invalid_permissions(mocker, app_, api_user_active, service_one):
         'main.service_request_to_go_live',
         'main.service_switch_live',
         'main.service_switch_research_mode',
-        'main.service_status_change',
-        'main.service_status_change_confirm',
         'main.service_delete',
         'main.service_delete_confirm']
     with app_.test_request_context():
@@ -550,8 +457,6 @@ def test_route_for_platform_admin(mocker, app_, platform_admin_user, service_one
         'main.service_name_change',
         'main.service_name_change_confirm',
         'main.service_request_to_go_live',
-        'main.service_status_change',
-        'main.service_status_change_confirm',
         'main.service_delete',
         'main.service_delete_confirm'
     ]

From 3b3245adbd9b837b437930e08d840825c6751539 Mon Sep 17 00:00:00 2001
From: Chris Hill-Scott <me@quis.cc>
Date: Mon, 22 Aug 2016 15:39:50 +0100
Subject: [PATCH 08/15] Explain trial mode on the settings page
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The previous text on this page around trial mode was a bit of a
mouthful. Also it only really made sense if you already knew what trial
mode was.

This commit tries to make it really explicit:
- that you’re in trial mode
- what it means to be in trial mode (copied from the trial mode page)
- where you go to not be in trial mode
---
 app/assets/stylesheets/_grids.scss        |  4 +++
 app/templates/views/service-settings.html | 36 ++++++++++-------------
 2 files changed, 19 insertions(+), 21 deletions(-)

diff --git a/app/assets/stylesheets/_grids.scss b/app/assets/stylesheets/_grids.scss
index 183a85630..4f571ca13 100644
--- a/app/assets/stylesheets/_grids.scss
+++ b/app/assets/stylesheets/_grids.scss
@@ -35,6 +35,10 @@
   margin-bottom: $gutter-half;
 }
 
+.bottom-gutter-2 {
+  margin-bottom: $gutter * 2;
+}
+
 .align-with-heading {
   display: block;
   text-align: center;
diff --git a/app/templates/views/service-settings.html b/app/templates/views/service-settings.html
index 07b0b243e..f064030fb 100644
--- a/app/templates/views/service-settings.html
+++ b/app/templates/views/service-settings.html
@@ -10,7 +10,7 @@
 
     <h1 class="heading-large">Settings</h1>
 
-    <div class="dashboard-table">
+    <div class="dashboard-table bottom-gutter-2">
 
       {% call mapping_table(
         caption='Settings',
@@ -37,28 +37,22 @@
           {{ edit_field('Change', url_for('.service_set_sms_sender', service_id=current_service.id)) }}
         {% endcall %}
       {% endcall %}
-
-      {% call mapping_table(
-        caption='Restrictions',
-        field_headings=['Label', 'Value', 'Action'],
-        field_headings_visible=False,
-        caption_visible=True
-      ) %}
-        {% call row() %}
-          {% if current_service.restricted %}
-            {{ text_field('Trial mode') }}
-            {{ text_field('On') }}
-            {{ edit_field('Request to go live', url_for('.service_request_to_go_live', service_id=current_service.id)) }}
-          {% else %}
-            {{ text_field('Trial mode') }}
-            {{ text_field('Off')}}
-            {{ edit_field() }}
-          {% endif %}
-        {% endcall %}
-      {% endcall %}
     </div>
 
-    {% if current_user.has_permissions([], admin_override=True) %}
+    <h2 class="heading-medium">Your service is in trial mode</h2>
+
+    <ul class='list list-bullet'>
+      <li>you can only send messages to yourself</li>
+      <li>you can add people to your team, then you can send messages to them too</li>
+      <li>you can only send 50 messages per day</li>
+    </ul>
+
+    <p>
+      To remove these restrictions
+      <a href="{{ url_for('.service_request_to_go_live', service_id=current_service.id) }}">request to go live</a>.
+    </p>
+
+    {% if false and current_user.has_permissions([], admin_override=True) %}
 
       <h2 class="heading-large">Platform admin settings</h2>
 

From 8eb2dadaa66fde3d3c185c40dfa635a479d3c527 Mon Sep 17 00:00:00 2001
From: Chris Hill-Scott <me@quis.cc>
Date: Mon, 22 Aug 2016 20:22:47 +0100
Subject: [PATCH 09/15] Add spacing, balance type size

---
 app/templates/views/service-settings.html | 28 +++++++++++++----------
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/app/templates/views/service-settings.html b/app/templates/views/service-settings.html
index f064030fb..55f0b9abc 100644
--- a/app/templates/views/service-settings.html
+++ b/app/templates/views/service-settings.html
@@ -10,7 +10,7 @@
 
     <h1 class="heading-large">Settings</h1>
 
-    <div class="dashboard-table bottom-gutter-2">
+    <div class="bottom-gutter-2">
 
       {% call mapping_table(
         caption='Settings',
@@ -39,22 +39,26 @@
       {% endcall %}
     </div>
 
-    <h2 class="heading-medium">Your service is in trial mode</h2>
+    <div class="bottom-gutter-2">
 
-    <ul class='list list-bullet'>
-      <li>you can only send messages to yourself</li>
-      <li>you can add people to your team, then you can send messages to them too</li>
-      <li>you can only send 50 messages per day</li>
-    </ul>
+      <h2 class="heading-medium">Your service is in trial mode</h2>
 
-    <p>
-      To remove these restrictions
-      <a href="{{ url_for('.service_request_to_go_live', service_id=current_service.id) }}">request to go live</a>.
-    </p>
+      <ul class='list list-bullet'>
+        <li>you can only send messages to yourself</li>
+        <li>you can add people to your team, then you can send messages to them too</li>
+        <li>you can only send 50 messages per day</li>
+      </ul>
+
+      <p>
+        To remove these restrictions
+        <a href="{{ url_for('.service_request_to_go_live', service_id=current_service.id) }}">request to go live</a>.
+      </p>
+
+    </div>
 
     {% if false and current_user.has_permissions([], admin_override=True) %}
 
-      <h2 class="heading-large">Platform admin settings</h2>
+      <h2 class="heading-medium">Platform admin settings</h2>
 
       {{ browse_list([
         {

From 709882065fec18b52a8982eb4b629684db48c44d Mon Sep 17 00:00:00 2001
From: Chris Hill-Scott <me@quis.cc>
Date: Tue, 23 Aug 2016 14:16:07 +0100
Subject: [PATCH 10/15] Add some explaining to the email reply to page

---
 app/main/forms.py                             |  2 +-
 app/templates/views/service-settings.html     |  4 ++--
 .../service-settings/set-reply-to-email.html  | 21 +++++++++++++++++--
 3 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/app/main/forms.py b/app/main/forms.py
index 820d47c9c..e09f9cc23 100644
--- a/app/main/forms.py
+++ b/app/main/forms.py
@@ -327,7 +327,7 @@ class ProviderForm(Form):
 
 
 class ServiceReplyToEmailFrom(Form):
-    email_address = email_address()
+    email_address = email_address(label='Email reply to address')
 
 
 class ServiceSmsSender(Form):
diff --git a/app/templates/views/service-settings.html b/app/templates/views/service-settings.html
index 55f0b9abc..b9ad9bff1 100644
--- a/app/templates/views/service-settings.html
+++ b/app/templates/views/service-settings.html
@@ -56,13 +56,13 @@
 
     </div>
 
-    {% if false and current_user.has_permissions([], admin_override=True) %}
+    {% if current_user.has_permissions([], admin_override=True) %}
 
       <h2 class="heading-medium">Platform admin settings</h2>
 
       {{ browse_list([
         {
-          'title': 'Set email branding',
+          'title': 'Email branding',
           'link': url_for('.service_set_branding_and_org', service_id=current_service.id)
         },
         {
diff --git a/app/templates/views/service-settings/set-reply-to-email.html b/app/templates/views/service-settings/set-reply-to-email.html
index fcd77c303..a7a40774b 100644
--- a/app/templates/views/service-settings/set-reply-to-email.html
+++ b/app/templates/views/service-settings/set-reply-to-email.html
@@ -3,14 +3,31 @@
 {% from "components/page-footer.html" import page_footer %}
 
 {% block page_title %}
-  Request to go live – GOV.UK Notify
+  Email reply to address – GOV.UK Notify
 {% endblock %}
 
 {% block maincolumn_content %}
 
   <div class="grid-row">
     <div class="column-three-quarters">
-      <h1 class="heading-large">Set email reply to address</h1>
+      <h1 class="heading-large">
+        Email reply to address
+      </h1>
+      <p>
+        Your emails will be sent from
+        {{ current_service.email_from }}@notifications.service.gov.uk.
+        This is so they have the best chance of being delivered.
+        This email address can’t receive replies.
+      </p>
+      <p>
+        Set up a separate email address to receive replies from
+        your users, then enter it here.
+      </p>
+      {% if current_service.restricted %}
+        <p>
+          Your service can’t go live until you’ve done this.
+        </p>
+      {% endif %}
       <form method="post">
         {{ textbox(
           form.email_address,

From c5c693ef0b77b2f6f0b6cbcb6b6f6241450a2101 Mon Sep 17 00:00:00 2001
From: Chris Hill-Scott <me@quis.cc>
Date: Mon, 22 Aug 2016 16:10:57 +0100
Subject: [PATCH 11/15] Add some explaining to the SMS sender page

---
 app/main/forms.py                             | 10 +++++++---
 .../service-settings/set-sms-sender.html      | 19 +++++++++++++++++--
 tests/app/main/test_validators.py             |  4 ++--
 3 files changed, 26 insertions(+), 7 deletions(-)

diff --git a/app/main/forms.py b/app/main/forms.py
index e09f9cc23..343deb062 100644
--- a/app/main/forms.py
+++ b/app/main/forms.py
@@ -331,13 +331,17 @@ class ServiceReplyToEmailFrom(Form):
 
 
 class ServiceSmsSender(Form):
-    sms_sender = StringField('', validators=[Length(max=11,
-                             message="Text message sender can't be longer than 11 characters")])
+    sms_sender = StringField(
+        'Text message sender',
+        validators=[
+            Length(max=11, message="Enter fewer than 11 characters")
+        ]
+    )
 
     def validate_sms_sender(form, field):
         import re
         if field.data and not re.match('^[a-zA-Z0-9\s]+$', field.data):
-            raise ValidationError('Text message sender can only contain alpha-numeric characters')
+            raise ValidationError('Use letters and numbers only')
 
 
 class ServiceBrandingOrg(Form):
diff --git a/app/templates/views/service-settings/set-sms-sender.html b/app/templates/views/service-settings/set-sms-sender.html
index e9c39a374..fdd9b6416 100644
--- a/app/templates/views/service-settings/set-sms-sender.html
+++ b/app/templates/views/service-settings/set-sms-sender.html
@@ -10,12 +10,27 @@
 
   <div class="grid-row">
     <div class="column-three-quarters">
-      <h1 class="heading-large">Set text message sender name</h1>
+      <h1 class="heading-large">Text message sender</h1>
+      <p>
+        This appears instead of a phone number when a user receives a
+        text message from your service.
+      <p>
+        If you leave this blank:
+      </p>
+      <ul class="list list-bullet">
+        <li>
+          your messages will be sent from 40604 (a shortcode that’s
+          reserved for government use)
+        </li>
+        <li>
+          each message will begin with ‘{{ current_service.name }}:’
+        </li>
+      </ul>
       <form method="post">
         {{ textbox(
           form.sms_sender,
           width='1-1',
-          safe_error_message=True
+          hint='Up to 11 characters, letters, numbers and spaces only'
         ) }}
         {{ page_footer(
           'Save',
diff --git a/tests/app/main/test_validators.py b/tests/app/main/test_validators.py
index 5ea1300ca..c1ffa12d7 100644
--- a/tests/app/main/test_validators.py
+++ b/tests/app/main/test_validators.py
@@ -137,8 +137,8 @@ def test_sms_sender_form_validation(app_, mock_get_user_by_email):
 
         form.sms_sender.data = 'morethanelevenchars'
         form.validate()
-        assert "Text message sender can't be longer than 11 characters" == form.errors['sms_sender'][0]
+        assert "Enter fewer than 11 characters" == form.errors['sms_sender'][0]
 
         form.sms_sender.data = '###########'
         form.validate()
-        assert 'Text message sender can only contain alpha-numeric characters' == form.errors['sms_sender'][0]
+        assert 'Use letters and numbers only' == form.errors['sms_sender'][0]

From e234ed97417def22b3c7fd3c1d3356f0a97da302 Mon Sep 17 00:00:00 2001
From: Chris Hill-Scott <me@quis.cc>
Date: Tue, 23 Aug 2016 15:47:43 +0100
Subject: [PATCH 12/15] Make live and research mode switches into buttons
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

They change state, so they should be buttons (even if they’re still
links).
---
 app/templates/views/service-settings.html | 27 +++++++++++------------
 1 file changed, 13 insertions(+), 14 deletions(-)

diff --git a/app/templates/views/service-settings.html b/app/templates/views/service-settings.html
index b9ad9bff1..9f4afcfab 100644
--- a/app/templates/views/service-settings.html
+++ b/app/templates/views/service-settings.html
@@ -64,23 +64,22 @@
         {
           'title': 'Email branding',
           'link': url_for('.service_set_branding_and_org', service_id=current_service.id)
-        },
-        {
-          'title': 'Make service live',
-          'link': url_for('.service_switch_live', service_id=current_service.id)
-        } if current_service.restricted else {
-          'title': 'Revert service to trial',
-          'link': url_for('.service_switch_live', service_id=current_service.id)
-        },
-        {
-          'title': 'Put service into research mode',
-          'link': url_for('.service_switch_research_mode', service_id=current_service.id)
-        } if not current_service.research_mode else {
-          'title': 'Take service out of research mode',
-          'link': url_for('.service_switch_research_mode', service_id=current_service.id)
         }
       ]) }}
 
+      <ul>
+        <li class="bottom-gutter">
+          <a href="{{ url_for('.service_switch_live', service_id=current_service.id) }}" class="button">
+            {{ 'Make service live' if current_service.restricted else 'Revert service to trial mode' }}
+          </a>
+        </li>
+        <li>
+          <a href="{{ url_for('.service_switch_research_mode', service_id=current_service.id) }}" class="button">
+            {{ 'Take service out of research mode' if current_service.research_mode else 'Put into research mode' }}
+          </a>
+        </li>
+      </ul>
+
     {% endif %}
 
 {% endblock %}

From 58b2a8a825f090aefabddd7670e2f8748d0fab7a Mon Sep 17 00:00:00 2001
From: Chris Hill-Scott <me@quis.cc>
Date: Wed, 24 Aug 2016 09:34:14 +0100
Subject: [PATCH 13/15] Make email branding settings a table

So that it matches the other settings on this page
---
 app/main/views/service_settings.py            |   9 +-
 app/notify_client/organisations_client.py     |   3 +
 app/templates/views/service-settings.html     |  26 ++-
 tests/app/main/views/test_service_settings.py | 150 ++++++++++--------
 tests/conftest.py                             |  17 ++
 5 files changed, 133 insertions(+), 72 deletions(-)

diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py
index 301aafd61..3f90d2bdf 100644
--- a/app/main/views/service_settings.py
+++ b/app/main/views/service_settings.py
@@ -34,7 +34,14 @@ from app import user_api_client, current_service, organisations_client
 @login_required
 @user_has_permissions('manage_settings', admin_override=True)
 def service_settings(service_id):
-    return render_template('views/service-settings.html')
+    if current_service['organisation']:
+        organisation = organisations_client.get_organisation(current_service['organisation'])['organisation']
+    else:
+        organisation = None
+    return render_template(
+        'views/service-settings.html',
+        organisation=organisation
+    )
 
 
 @main.route("/services/<service_id>/service-settings/name", methods=['GET', 'POST'])
diff --git a/app/notify_client/organisations_client.py b/app/notify_client/organisations_client.py
index 3d39decf7..21a4ae36e 100644
--- a/app/notify_client/organisations_client.py
+++ b/app/notify_client/organisations_client.py
@@ -15,5 +15,8 @@ class OrganisationsClient(BaseAPIClient):
         self.client_id = app.config['ADMIN_CLIENT_USER_NAME']
         self.secret = app.config['ADMIN_CLIENT_SECRET']
 
+    def get_organisation(self, id):
+        return self.get(url='/organisation/{}'.format(id))
+
     def get_organisations(self):
         return self.get(url='/organisation')['organisations']
diff --git a/app/templates/views/service-settings.html b/app/templates/views/service-settings.html
index 9f4afcfab..053e950f5 100644
--- a/app/templates/views/service-settings.html
+++ b/app/templates/views/service-settings.html
@@ -60,12 +60,26 @@
 
       <h2 class="heading-medium">Platform admin settings</h2>
 
-      {{ browse_list([
-        {
-          'title': 'Email branding',
-          'link': url_for('.service_set_branding_and_org', service_id=current_service.id)
-        }
-      ]) }}
+      {% call mapping_table(
+        caption='Settings',
+        field_headings=['Label', 'Value', 'Action'],
+        field_headings_visible=False,
+        caption_visible=False
+      ) %}
+        {% call row() %}
+          {{ text_field('Email branding' )}}
+          {% call field() %}
+            {% if current_service.branding == 'govuk' %}
+              GOV.UK
+            {% elif current_service.branding == 'both' %}
+              GOV.UK and {{ organisation.name if organisation else None }}
+            {% elif current_service.branding == 'org' %}
+              Only {{ organisation.name if organisation else None }}
+            {% endif %}
+          {% endcall %}
+          {{ edit_field('Change', url_for('.service_set_branding_and_org', service_id=current_service.id)) }}
+        {% endcall %}
+      {% endcall %}
 
       <ul>
         <li class="bottom-gutter">
diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py
index 6f9585f62..08080b599 100644
--- a/tests/app/main/views/test_service_settings.py
+++ b/tests/app/main/views/test_service_settings.py
@@ -13,7 +13,8 @@ def test_should_show_overview(
     app_,
     active_user_with_permissions,
     mocker,
-    service_one
+    service_one,
+    mock_get_organisation
 ):
     with app_.test_request_context(), app_.test_client() as client:
         client.login(active_user_with_permissions, mocker, service_one)
@@ -36,7 +37,8 @@ def test_should_show_overview_for_service_with_more_things_set(
     app_,
     active_user_with_permissions,
     mocker,
-    service_with_reply_to_addresses
+    service_with_reply_to_addresses,
+    mock_get_organisation
 ):
     with app_.test_request_context(), app_.test_client() as client:
         client.login(active_user_with_permissions, mocker, service_with_reply_to_addresses)
@@ -91,14 +93,17 @@ def test_should_redirect_after_change_service_name(app_,
         assert mock_get_services.called
 
 
-def test_switch_service_to_live(app_,
-                                service_one,
-                                mock_login,
-                                mock_get_user,
-                                active_user_with_permissions,
-                                mock_get_service,
-                                mock_update_service,
-                                mock_has_permissions):
+def test_switch_service_to_live(
+    app_,
+    service_one,
+    mock_login,
+    mock_get_user,
+    active_user_with_permissions,
+    mock_get_service,
+    mock_update_service,
+    mock_has_permissions,
+    mock_get_organisation
+):
     with app_.test_request_context():
         with app_.test_client() as client:
             client.login(active_user_with_permissions)
@@ -115,14 +120,17 @@ def test_switch_service_to_live(app_,
         )
 
 
-def test_switch_service_to_restricted(app_,
-                                      service_one,
-                                      mock_login,
-                                      mock_get_user,
-                                      active_user_with_permissions,
-                                      mock_get_live_service,
-                                      mock_update_service,
-                                      mock_has_permissions):
+def test_switch_service_to_restricted(
+    app_,
+    service_one,
+    mock_login,
+    mock_get_user,
+    active_user_with_permissions,
+    mock_get_live_service,
+    mock_update_service,
+    mock_has_permissions,
+    mock_get_organisation
+):
     with app_.test_request_context():
         with app_.test_client() as client:
             client.login(active_user_with_permissions)
@@ -176,12 +184,15 @@ def test_should_show_service_name_confirmation(app_,
         app.service_api_client.get_service.assert_called_with(service_one['id'])
 
 
-def test_should_redirect_after_service_name_confirmation(app_,
-                                                         active_user_with_permissions,
-                                                         service_one,
-                                                         mocker,
-                                                         mock_update_service,
-                                                         mock_verify_password):
+def test_should_redirect_after_service_name_confirmation(
+    app_,
+    active_user_with_permissions,
+    service_one,
+    mocker,
+    mock_update_service,
+    mock_verify_password,
+    mock_get_organisation
+):
     with app_.test_request_context():
         with app_.test_client() as client:
             client.login(active_user_with_permissions, mocker, service_one)
@@ -250,12 +261,13 @@ def test_should_show_request_to_go_live(app_,
 
 
 def test_should_redirect_after_request_to_go_live(
-        app_,
-        api_user_active,
-        mock_get_user,
-        mock_get_service,
-        mock_has_permissions,
-        mocker
+    app_,
+    api_user_active,
+    mock_get_user,
+    mock_get_service,
+    mock_has_permissions,
+    mock_get_organisation,
+    mocker
 ):
     mock_post = mocker.patch(
         'app.main.views.feedback.requests.post',
@@ -290,12 +302,12 @@ def test_should_redirect_after_request_to_go_live(
 
 
 def test_log_error_on_request_to_go_live(
-        app_,
-        api_user_active,
-        mock_get_user,
-        mock_get_service,
-        mock_has_permissions,
-        mocker
+    app_,
+    api_user_active,
+    mock_get_user,
+    mock_get_service,
+    mock_has_permissions,
+    mocker
 ):
     mock_post = mocker.patch(
         'app.main.views.service_settings.requests.post',
@@ -407,7 +419,7 @@ def test_should_redirect_delete_confirmation(app_,
         assert mock_delete_service.called
 
 
-def test_route_permissions(mocker, app_, api_user_active, service_one):
+def test_route_permissions(mocker, app_, api_user_active, service_one, mock_get_organisation):
     routes = [
         'main.service_settings',
         'main.service_name_change',
@@ -428,7 +440,7 @@ def test_route_permissions(mocker, app_, api_user_active, service_one):
                 service_one)
 
 
-def test_route_invalid_permissions(mocker, app_, api_user_active, service_one):
+def test_route_invalid_permissions(mocker, app_, api_user_active, service_one, mock_get_organisation):
     routes = [
         'main.service_settings',
         'main.service_name_change',
@@ -451,7 +463,7 @@ def test_route_invalid_permissions(mocker, app_, api_user_active, service_one):
                 service_one)
 
 
-def test_route_for_platform_admin(mocker, app_, platform_admin_user, service_one):
+def test_route_for_platform_admin(mocker, app_, platform_admin_user, service_one, mock_get_organisation):
     routes = [
         'main.service_settings',
         'main.service_name_change',
@@ -490,11 +502,13 @@ def test_route_for_platform_admin_update_service(mocker, app_, platform_admin_us
 
 
 def test_set_reply_to_email_address(
-        app_,
-        active_user_with_permissions,
-        mocker,
-        mock_update_service,
-        service_one):
+    app_,
+    active_user_with_permissions,
+    mocker,
+    mock_update_service,
+    service_one,
+    mock_get_organisation
+):
     with app_.test_request_context():
         with app_.test_client() as client:
             client.login(active_user_with_permissions, mocker, service_one)
@@ -583,14 +597,16 @@ def test_switch_service_from_research_mode_to_normal(
 
 
 def test_shows_research_mode_indicator(
-        app_,
-        service_one,
-        mock_login,
-        mock_get_user,
-        active_user_with_permissions,
-        mock_get_service,
-        mock_has_permissions,
-        mocker):
+    app_,
+    service_one,
+    mock_login,
+    mock_get_user,
+    active_user_with_permissions,
+    mock_get_service,
+    mock_has_permissions,
+    mock_get_organisation,
+    mocker
+):
     with app_.test_request_context():
         with app_.test_client() as client:
             service = service_json(
@@ -615,14 +631,16 @@ def test_shows_research_mode_indicator(
 
 
 def test_does_not_show_research_mode_indicator(
-        app_,
-        service_one,
-        mock_login,
-        mock_get_user,
-        active_user_with_permissions,
-        mock_get_service,
-        mock_has_permissions,
-        mocker):
+    app_,
+    service_one,
+    mock_login,
+    mock_get_user,
+    active_user_with_permissions,
+    mock_get_service,
+    mock_has_permissions,
+    mock_get_organisation,
+    mocker
+):
     with app_.test_request_context():
         with app_.test_client() as client:
             client.login(active_user_with_permissions)
@@ -635,11 +653,13 @@ def test_does_not_show_research_mode_indicator(
 
 
 def test_set_text_message_sender(
-        app_,
-        active_user_with_permissions,
-        mocker,
-        mock_update_service,
-        service_one):
+    app_,
+    active_user_with_permissions,
+    mocker,
+    mock_update_service,
+    service_one,
+    mock_get_organisation
+):
     with app_.test_request_context():
         with app_.test_client() as client:
             client.login(active_user_with_permissions, mocker, service_one)
diff --git a/tests/conftest.py b/tests/conftest.py
index 68b43b34a..8d3190e64 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -1144,3 +1144,20 @@ def mock_get_organisations(mocker):
     return mocker.patch(
         'app.organisations_client.get_organisations', side_effect=_get_organisations
     )
+
+
+@pytest.fixture(scope='function')
+def mock_get_organisation(mocker):
+    def _get_organisation(id):
+        return {
+            'organisation': {
+                'logo': 'example.png',
+                'name': 'Organisation name',
+                'id': 'organisation-id',
+                'colour': '#f00'
+            }
+        }
+
+    return mocker.patch(
+        'app.organisations_client.get_organisation', side_effect=_get_organisation
+    )

From 09d4ff44bbdb7cf057a51b1ac57162da32fca32a Mon Sep 17 00:00:00 2001
From: Chris Hill-Scott <me@quis.cc>
Date: Wed, 24 Aug 2016 14:44:15 +0100
Subject: [PATCH 14/15] Make back button text consistent

---
 app/templates/views/service-settings/set-branding-and-org.html | 3 ++-
 app/templates/views/service-settings/set-reply-to-email.html   | 3 ++-
 app/templates/views/service-settings/set-sms-sender.html       | 3 ++-
 3 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/app/templates/views/service-settings/set-branding-and-org.html b/app/templates/views/service-settings/set-branding-and-org.html
index b49b17a52..34c78da8f 100644
--- a/app/templates/views/service-settings/set-branding-and-org.html
+++ b/app/templates/views/service-settings/set-branding-and-org.html
@@ -16,7 +16,8 @@
         {{ branding_radios(form.organisation, branding_dict=branding_dict) }}
         {{ page_footer(
           'Save',
-          back_link=url_for('.service_settings', service_id=current_service.id)
+          back_link=url_for('.service_settings', service_id=current_service.id),
+          back_link_text='Back to settings'
         ) }}
       </form>
     </div>
diff --git a/app/templates/views/service-settings/set-reply-to-email.html b/app/templates/views/service-settings/set-reply-to-email.html
index a7a40774b..b6d881847 100644
--- a/app/templates/views/service-settings/set-reply-to-email.html
+++ b/app/templates/views/service-settings/set-reply-to-email.html
@@ -36,7 +36,8 @@
         ) }}
         {{ page_footer(
           'Save',
-          back_link=url_for('.service_settings', service_id=current_service.id)
+          back_link=url_for('.service_settings', service_id=current_service.id),
+          back_link_text='Back to settings'
       ) }}
       </form>
     </div>
diff --git a/app/templates/views/service-settings/set-sms-sender.html b/app/templates/views/service-settings/set-sms-sender.html
index fdd9b6416..c6f19daab 100644
--- a/app/templates/views/service-settings/set-sms-sender.html
+++ b/app/templates/views/service-settings/set-sms-sender.html
@@ -34,7 +34,8 @@
         ) }}
         {{ page_footer(
           'Save',
-          back_link=url_for('.service_settings', service_id=current_service.id)
+          back_link=url_for('.service_settings', service_id=current_service.id),
+          back_link_text='Back to settings'
       ) }}
       </form>
     </div>

From 82b5cc6fc6d181c17768b5abeef0b8b8dd058610 Mon Sep 17 00:00:00 2001
From: Chris Hill-Scott <me@quis.cc>
Date: Wed, 24 Aug 2016 15:31:49 +0100
Subject: [PATCH 15/15] Remove 3/4 width column on settings pages

The mesaure on these pages was too short, making them awkward to read.

Also varies the size of text boxes to make them appropriate to the
expected size of content that they will contain.
---
 .../views/service-settings/name.html          | 34 +++++-----
 .../service-settings/set-reply-to-email.html  | 64 +++++++++----------
 .../service-settings/set-sms-sender.html      | 60 ++++++++---------
 3 files changed, 73 insertions(+), 85 deletions(-)

diff --git a/app/templates/views/service-settings/name.html b/app/templates/views/service-settings/name.html
index 5420b3936..395bf36d9 100644
--- a/app/templates/views/service-settings/name.html
+++ b/app/templates/views/service-settings/name.html
@@ -10,26 +10,22 @@
 
   <h1 class="heading-large">Change your service name</h1>
 
-  <div class="grid-row">
-    <div class="column-three-quarters">
 
-      <div class="form-group">
-        <p>Users will see your service name:</p>
-        <ul class="list-bullet">
-          <li>at the start of every text message, eg ‘Vehicle tax: we received your payment, thank you’</li>
-          <li>as your email sender name</li>
-        </ul>
-      </div>
-
-      <form method="post">
-        {{ textbox(form.name) }}
-        {{ page_footer(
-          'Save',
-          back_link=url_for('.service_settings', service_id=current_service.id),
-          back_link_text='Back to settings'
-        ) }}
-      </form>
-    </div>
+  <div class="form-group">
+    <p>Users will see your service name:</p>
+    <ul class="list-bullet">
+      <li>at the start of every text message, eg ‘Vehicle tax: we received your payment, thank you’</li>
+      <li>as your email sender name</li>
+    </ul>
   </div>
 
+  <form method="post">
+    {{ textbox(form.name) }}
+    {{ page_footer(
+      'Save',
+      back_link=url_for('.service_settings', service_id=current_service.id),
+      back_link_text='Back to settings'
+    ) }}
+  </form>
+
 {% endblock %}
diff --git a/app/templates/views/service-settings/set-reply-to-email.html b/app/templates/views/service-settings/set-reply-to-email.html
index b6d881847..1de3cb77d 100644
--- a/app/templates/views/service-settings/set-reply-to-email.html
+++ b/app/templates/views/service-settings/set-reply-to-email.html
@@ -8,39 +8,35 @@
 
 {% block maincolumn_content %}
 
-  <div class="grid-row">
-    <div class="column-three-quarters">
-      <h1 class="heading-large">
-        Email reply to address
-      </h1>
-      <p>
-        Your emails will be sent from
-        {{ current_service.email_from }}@notifications.service.gov.uk.
-        This is so they have the best chance of being delivered.
-        This email address can’t receive replies.
-      </p>
-      <p>
-        Set up a separate email address to receive replies from
-        your users, then enter it here.
-      </p>
-      {% if current_service.restricted %}
-        <p>
-          Your service can’t go live until you’ve done this.
-        </p>
-      {% endif %}
-      <form method="post">
-        {{ textbox(
-          form.email_address,
-          width='1-1',
-          safe_error_message=True
-        ) }}
-        {{ page_footer(
-          'Save',
-          back_link=url_for('.service_settings', service_id=current_service.id),
-          back_link_text='Back to settings'
-      ) }}
-      </form>
-    </div>
-  </div>
+  <h1 class="heading-large">
+    Email reply to address
+  </h1>
+  <p>
+    Your emails will be sent from
+    {{ current_service.email_from }}@notifications.service.gov.uk.
+    This is so they have the best chance of being delivered.
+    This email address can’t receive replies.
+  </p>
+  <p>
+    Set up a separate email address to receive replies from
+    your users, then enter it here.
+  </p>
+  {% if current_service.restricted %}
+    <p>
+      Your service can’t go live until you’ve done this.
+    </p>
+  {% endif %}
+  <form method="post">
+    {{ textbox(
+      form.email_address,
+      width='2-3',
+      safe_error_message=True
+    ) }}
+    {{ page_footer(
+      'Save',
+      back_link=url_for('.service_settings', service_id=current_service.id),
+      back_link_text='Back to settings'
+  ) }}
+  </form>
 
 {% endblock %}
diff --git a/app/templates/views/service-settings/set-sms-sender.html b/app/templates/views/service-settings/set-sms-sender.html
index c6f19daab..3bc58eeb3 100644
--- a/app/templates/views/service-settings/set-sms-sender.html
+++ b/app/templates/views/service-settings/set-sms-sender.html
@@ -8,37 +8,33 @@
 
 {% block maincolumn_content %}
 
-  <div class="grid-row">
-    <div class="column-three-quarters">
-      <h1 class="heading-large">Text message sender</h1>
-      <p>
-        This appears instead of a phone number when a user receives a
-        text message from your service.
-      <p>
-        If you leave this blank:
-      </p>
-      <ul class="list list-bullet">
-        <li>
-          your messages will be sent from 40604 (a shortcode that’s
-          reserved for government use)
-        </li>
-        <li>
-          each message will begin with ‘{{ current_service.name }}:’
-        </li>
-      </ul>
-      <form method="post">
-        {{ textbox(
-          form.sms_sender,
-          width='1-1',
-          hint='Up to 11 characters, letters, numbers and spaces only'
-        ) }}
-        {{ page_footer(
-          'Save',
-          back_link=url_for('.service_settings', service_id=current_service.id),
-          back_link_text='Back to settings'
-      ) }}
-      </form>
-    </div>
-  </div>
+  <h1 class="heading-large">Text message sender</h1>
+  <p>
+    This appears instead of a phone number when a user receives a
+    text message from your service.
+  <p>
+    If you leave this blank:
+  </p>
+  <ul class="list list-bullet">
+    <li>
+      your messages will be sent from 40604 (a shortcode that’s
+      reserved for government use)
+    </li>
+    <li>
+      each message will begin with ‘{{ current_service.name }}:’
+    </li>
+  </ul>
+  <form method="post">
+    {{ textbox(
+      form.sms_sender,
+      width='1-4',
+      hint='Up to 11 characters, letters, numbers and spaces only'
+    ) }}
+    {{ page_footer(
+      'Save',
+      back_link=url_for('.service_settings', service_id=current_service.id),
+      back_link_text='Back to settings'
+  ) }}
+  </form>
 
 {% endblock %}