From 0b62d1e75510e60475d60c0f9805f03cc42f122f Mon Sep 17 00:00:00 2001
From: Chris Hill-Scott <me@quis.cc>
Date: Thu, 7 Jan 2016 16:24:43 +0000
Subject: [PATCH] Add confirm loop
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

For pages where
- we want you to be sure that you want to do what you’re about to do
- we want to be sure it’s you trying to do the thing

This adds a page that asks the user to confirm their password.
---
 .../stylesheets/components/browse-list.scss   |  2 +-
 .../stylesheets/components/submit-form.scss   |  3 +-
 app/main/views/service_settings.py            | 42 +++++++++++++-
 app/templates/components/list.html            | 11 ----
 app/templates/components/textbox.html         |  4 +-
 app/templates/views/service-settings.html     | 56 +++++++++----------
 .../views/service-settings/confirm.html       | 27 +++++++++
 .../views/service-settings/delete.html        | 37 +++++++++---
 .../views/service-settings/name.html          | 22 +++++---
 .../service-settings/request-to-go-live.html  | 37 ++++++++----
 .../views/service-settings/status.html        | 18 +++---
 tests/app/main/views/test_service_settings.py | 50 +++++++++++++++--
 12 files changed, 227 insertions(+), 82 deletions(-)
 delete mode 100644 app/templates/components/list.html
 create mode 100644 app/templates/views/service-settings/confirm.html

diff --git a/app/assets/stylesheets/components/browse-list.scss b/app/assets/stylesheets/components/browse-list.scss
index 1393aa352..abb95ecc0 100644
--- a/app/assets/stylesheets/components/browse-list.scss
+++ b/app/assets/stylesheets/components/browse-list.scss
@@ -20,7 +20,7 @@
   }
 
   &-hint {
-    @include core-16;
+    @include core-19;
     margin-top: 5px;
   }
 
diff --git a/app/assets/stylesheets/components/submit-form.scss b/app/assets/stylesheets/components/submit-form.scss
index 7a6681e46..8b98d2905 100644
--- a/app/assets/stylesheets/components/submit-form.scss
+++ b/app/assets/stylesheets/components/submit-form.scss
@@ -12,7 +12,8 @@
   .button {}
 
   .button-destructive {
-    @include button($error-colour)
+    @include button($error-colour);
+    padding: 0.52632em 0.78947em 0.26316em 0.78947em;
   }
 
 }
diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py
index 0bdd9ba71..768a08a42 100644
--- a/app/main/views/service_settings.py
+++ b/app/main/views/service_settings.py
@@ -1,4 +1,4 @@
-from flask import render_template, redirect, request, url_for
+from flask import render_template, redirect, request, url_for, abort
 from flask_login import login_required
 
 from app.main import main
@@ -25,6 +25,18 @@ def name():
             'views/service-settings/name.html',
             service=service
         )
+    elif request.method == 'POST':
+        return redirect(url_for('.confirm_name_change'))
+
+
+@main.route("/service-settings/name/confirm", methods=['GET', 'POST'])
+def confirm_name_change():
+    if request.method == 'GET':
+        return render_template(
+            'views/service-settings/confirm.html',
+            heading='Rename service',
+            submit_button_text='Confirm'
+        )
     elif request.method == 'POST':
         return redirect(url_for('.service_settings'))
 
@@ -47,6 +59,19 @@ def status():
             'views/service-settings/status.html',
             service=service
         )
+    elif request.method == 'POST':
+        return redirect(url_for('.confirm_status_change'))
+
+
+@main.route("/service-settings/status/confirm", methods=['GET', 'POST'])
+def confirm_status_change():
+    if request.method == 'GET':
+        return render_template(
+            'views/service-settings/confirm.html',
+            heading='Turn off outgoing messages',
+            submit_button_text='Confirm',
+            destructive=True
+        )
     elif request.method == 'POST':
         return redirect(url_for('.service_settings'))
 
@@ -59,4 +84,17 @@ def delete():
             service=service
         )
     elif request.method == 'POST':
-        return redirect(url_for('.index'))
+        return redirect(url_for('.confirm_delete'))
+
+
+@main.route("/service-settings/delete/confirm", methods=['GET', 'POST'])
+def confirm_delete():
+    if request.method == 'GET':
+        return render_template(
+            'views/service-settings/confirm.html',
+            heading='Delete service',
+            submit_button_text='Confirm',
+            destructive=True
+        )
+    elif request.method == 'POST':
+        return redirect(url_for('.dashboard'))
diff --git a/app/templates/components/list.html b/app/templates/components/list.html
deleted file mode 100644
index 71025a8ed..000000000
--- a/app/templates/components/list.html
+++ /dev/null
@@ -1,11 +0,0 @@
-{% macro list(items) %}
-  {% if items %}
-    <ul class="list list-bullet">
-      {% for item in items %}
-        <li>
-          {{ item }}
-        </li>
-      {% endfor %}
-    </ul>
-  {% endif %}
-{% endmacro %}
diff --git a/app/templates/components/textbox.html b/app/templates/components/textbox.html
index a32a76c4b..7a74f0985 100644
--- a/app/templates/components/textbox.html
+++ b/app/templates/components/textbox.html
@@ -1,8 +1,8 @@
-{% macro textbox(name, label, value='', small=True, highlight_tags=False) %}
+{% macro textbox(name, label, value='', small=True, highlight_tags=False, password=False) %}
   <div class="form-group">
     <label class="form-label" for="{{ name }}">{{ label }}</label>
     {% if small %}
-      <input class="form-control" id="{{ name }}" name="{{ name }}" type="text" value="{{ value }}">
+      <input class="form-control" id="{{ name }}" name="{{ name }}" type="{{ 'password' if password else 'text' }}" value="{{ value }}">
     {% else %}
       <textarea
         class="form-control {% if highlight_tags %}textbox-highlight-textbox{% endif %}"
diff --git a/app/templates/views/service-settings.html b/app/templates/views/service-settings.html
index a9e0bce19..4768b0f55 100644
--- a/app/templates/views/service-settings.html
+++ b/app/templates/views/service-settings.html
@@ -2,38 +2,38 @@
 {% from "components/browse-list.html" import browse_list %}
 
 {% block page_title %}
-GOV.UK Notify | Service settings
+  GOV.UK Notify | Service settings
 {% endblock %}
 
 {% block maincolumn_content %}
 
-  <h1 class="heading-xlarge">Service settings</h1>
+    <h1 class="heading-xlarge">Service settings</h1>
 
-  {{ browse_list([
-    {
-      'title': 'Rename service',
-      'link': url_for('.name'),
-      'hint': 'Your service is named “{}”'.format(service.name)
-    },
-    {
-      'title': 'Make service live',
-      'link': url_for('.request_to_go_live'),
-      'hint': 'A live service can send messages to anyone',
-    } if not service.live else {
-    },
-    {
-      'title': 'Stop sending messages',
-      'link': url_for('.status')
-    } if service.active else {
-      'title': 'Unsuspend service',
-      'link': url_for('.status')
-    },
-    {
-      'title': 'Delete everything',
-      'link': url_for('.delete'),
-      'hint': '',
-      'destructive': True
-    },
-  ]) }}
+    {{ browse_list([
+      {
+        'title': 'Rename service',
+        'link': url_for('.name'),
+        'hint': 'Your service is called ‘{}’'.format(service.name)
+      },
+      {
+        'title': 'Request to make service live',
+        'link': url_for('.request_to_go_live'),
+        'hint': 'A live service can send messages to any phone number or email address',
+      } if not service.live else {
+      },
+      {
+        'title': 'Turn off outgoing messages',
+        'link': url_for('.status'),
+        'destructive': True
+      } if service.active else {
+        'title': 'Desuspend service',
+        'link': url_for('.status')
+      },
+      {
+        'title': 'Delete service',
+        'link': url_for('.delete'),
+        'destructive': True
+      },
+    ]) }}
 
 {% endblock %}
diff --git a/app/templates/views/service-settings/confirm.html b/app/templates/views/service-settings/confirm.html
new file mode 100644
index 000000000..5216f3085
--- /dev/null
+++ b/app/templates/views/service-settings/confirm.html
@@ -0,0 +1,27 @@
+{% extends "withnav_template.html" %}
+{% from "components/textbox.html" import textbox %}
+{% from "components/submit-form.html" import submit_form %}
+
+{% block page_title %}
+GOV.UK Notify | Service settings
+{% endblock %}
+
+{% block maincolumn_content %}
+
+  <h1 class="heading-xlarge">{{ heading }}</h1>
+
+  <div class="grid-row">
+    <div class="column-three-quarters">
+
+    <form method="post">
+      {{ textbox('new_name', 'Enter your password', password=True) }}
+      {{ submit_form(
+        submit_button_text,
+        destructive=destructive,
+        back_link=url_for('.service_settings')
+      ) }}
+    </form>
+    </div>
+  </div>
+
+{% endblock %}
diff --git a/app/templates/views/service-settings/delete.html b/app/templates/views/service-settings/delete.html
index 2449f95bd..1c025ec3a 100644
--- a/app/templates/views/service-settings/delete.html
+++ b/app/templates/views/service-settings/delete.html
@@ -7,16 +7,37 @@
 
 {% block maincolumn_content %}
 
-  <h1 class="heading-xlarge">Delete this service</h1>
+  <div class="grid-row">
+    <div class="column-three-quarters">
+      <h1 class="heading-xlarge">Delete service</h1>
 
-  <p>
-    All will be lost.
-  </p>
+      <p>
+        This can’t be undone. You will lose:
+      </p>
 
-  <form method="post">
-    {{ submit_form('Delete', destructive=True) }}
-  </form>
+      <ul class="list list-bullet">
+        <li>
+          any data you’ve uploaded messages
+        </li>
+        <li>
+          any templates you’ve created
+        </li>
+        <li>
+          the history of
+        </li>
+        <li>
+          API keys
+        </li>
+      </ul>
 
-  <p><a href="{{ url_for('.service_settings') }}">Back to service settings</a></p>
+      <form method="post">
+        {{ submit_form(
+          'Yes, delete ‘{}’'.format(service.name),
+          destructive=True,
+          back_link=url_for('.service_settings')
+        ) }}
+      </form>
 
+    </div>
+  </div>
 {% endblock %}
diff --git a/app/templates/views/service-settings/name.html b/app/templates/views/service-settings/name.html
index c12fb377b..ab411f23c 100644
--- a/app/templates/views/service-settings/name.html
+++ b/app/templates/views/service-settings/name.html
@@ -8,15 +8,23 @@ GOV.UK Notify | Service settings
 
 {% block maincolumn_content %}
 
-    <h1 class="heading-xlarge">Rename your service</h1>
+  <div class="grid-row">
+    <div class="column-three-quarters">
 
-    <form method="post">
-      {{ textbox('new_name', 'New name', value=service.name) }}
-      {{ submit_form('Save') }}
-    </form>
-
-    <p><a href="{{ url_for('.service_settings') }}">Back to service settings</a></p>
+      <h1 class="heading-xlarge">Rename service</h1>
 
+      <p>
+        Users will see this name where?
+      </p>
 
+      <form method="post">
+        {{ textbox('new_name', 'New name', value=service.name) }}
+        {{ submit_form(
+          'Save',
+          back_link=url_for('.service_settings')
+        ) }}
+      </form>
+    </div>
+  </div>
 
 {% endblock %}
diff --git a/app/templates/views/service-settings/request-to-go-live.html b/app/templates/views/service-settings/request-to-go-live.html
index 110f15fb6..5d9878712 100644
--- a/app/templates/views/service-settings/request-to-go-live.html
+++ b/app/templates/views/service-settings/request-to-go-live.html
@@ -1,6 +1,5 @@
 {% extends "withnav_template.html" %}
 {% from "components/submit-form.html" import submit_form %}
-{% from "components/list.html" import list %}
 
 {% block page_title %}
 GOV.UK Notify | Service settings
@@ -8,26 +7,42 @@ GOV.UK Notify | Service settings
 
 {% block maincolumn_content %}
 
-  <h1 class="heading-xlarge">Make your service live</h1>
+  <h1 class="heading-xlarge">Request to make service live</h1>
 
   <div class="grid-row">
-    <div class="column-two-thirds">
+    <div class="column-three-quarters">
 
       <p>
-        Before you can send messages to anyone, you need to:
+        A live service can send messages to any phone number or email address.
       </p>
 
-      {{ list([
-        "Have spoken to someone",
-        "Have permission from someone else",
-        "etc."
-      ]) }}
+      <p>
+        First you need to:
+      </p>
+
+      <ul class="list list-bullet">
+        <li>
+          have spoken to someone
+        </li>
+        <li>
+          have permission from someone else
+        </li>
+        <li>
+          etc.
+        </li>
+      </ul>
+
+      <p>
+        It takes a few working days for your request to be approved.
+      </p>
 
       <form method="post">
-        {{ submit_form("Request to go live") }}
+        {{ submit_form(
+          'Request to make service live',
+          back_link=url_for('.service_settings')
+        ) }}
       </form>
 
-      <p><a href="{{ url_for('.service_settings') }}">Back to service settings</a></p>
     </div>
   </div>
 
diff --git a/app/templates/views/service-settings/status.html b/app/templates/views/service-settings/status.html
index b7c210a98..f88e46830 100644
--- a/app/templates/views/service-settings/status.html
+++ b/app/templates/views/service-settings/status.html
@@ -7,24 +7,28 @@ GOV.UK Notify | Service settings
 
 {% block maincolumn_content %}
 
+  <h1 class="heading-xlarge">Turn off outgoing messages</h1>
+
   <div class="grid-row">
-    <div class="column-two-thirds">
-      <h1 class="heading-xlarge">Suspend your service</h1>
+    <div class="column-three-quarters">
 
       <p>
-        Suspending a service means it won’t send messages. Any messages that are
-        already queued will still be sent.
+        You’ll still be able to send messages to yourself by uploading a CSV
+        file.
       </p>
 
       <p>
-        You can undo this at any time.
+        You can start sending messages again when you’re ready.
       </p>
 
       <form method="post">
-        {{ submit_form('Suspend') }}
+        {{ submit_form(
+          'Turn off outgoing messages',
+          destructive=True,
+          back_link=url_for('.service_settings')
+        ) }}
       </form>
 
-      <p><a href="{{ url_for('.service_settings') }}">Back to service settings</a></p>
     </div>
   </div>
 
diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py
index 115e7121d..3ae1f284b 100644
--- a/tests/app/main/views/test_service_settings.py
+++ b/tests/app/main/views/test_service_settings.py
@@ -9,7 +9,7 @@ def test_should_show_service_name(notifications_admin):
     response = notifications_admin.test_client().get('/service-settings/name')
 
     assert response.status_code == 200
-    assert 'Service name' in response.get_data(as_text=True)
+    assert 'Rename service' in response.get_data(as_text=True)
 
 
 def test_should_redirect_after_change_service_name(notifications_admin):
@@ -19,11 +19,25 @@ def test_should_redirect_after_change_service_name(notifications_admin):
     assert 'http://localhost/service-settings' == response.location
 
 
+def test_should_show_service_name_confirmation(notifications_admin):
+    response = notifications_admin.test_client().get('/service-settings/name/confirm')
+
+    assert response.status_code == 200
+    assert 'Rename service' in response.get_data(as_text=True)
+
+
+def test_should_redirect_after_service_name_confirmation(notifications_admin):
+    response = notifications_admin.test_client().post('/service-settings/name/confirm')
+
+    assert response.status_code == 302
+    assert 'http://localhost/service-settings' == response.location
+
+
 def test_should_show_request_to_go_live(notifications_admin):
     response = notifications_admin.test_client().get('/service-settings/request-to-go-live')
 
     assert response.status_code == 200
-    assert 'Request to go live' in response.get_data(as_text=True)
+    assert 'Request to make service live' in response.get_data(as_text=True)
 
 
 def test_should_redirect_after_request_to_go_live(notifications_admin):
@@ -37,12 +51,26 @@ def test_should_show_status_page(notifications_admin):
     response = notifications_admin.test_client().get('/service-settings/status')
 
     assert response.status_code == 200
-    assert 'Suspend your service' in response.get_data(as_text=True)
+    assert 'Turn off outgoing messages' in response.get_data(as_text=True)
 
 
 def test_should_show_redirect_after_status_change(notifications_admin):
     response = notifications_admin.test_client().post('/service-settings/status')
 
+    assert response.status_code == 302
+    assert 'http://localhost/service-settings/status/confirm' == response.location
+
+
+def test_should_show_status_confirmation(notifications_admin):
+    response = notifications_admin.test_client().get('/service-settings/status/confirm')
+
+    assert response.status_code == 200
+    assert 'Turn off outgoing messages' in response.get_data(as_text=True)
+
+
+def test_should_redirect_after_status_confirmation(notifications_admin):
+    response = notifications_admin.test_client().post('/service-settings/status/confirm')
+
     assert response.status_code == 302
     assert 'http://localhost/service-settings' == response.location
 
@@ -58,4 +86,18 @@ def test_should_show_redirect_after_deleting_service(notifications_admin):
     response = notifications_admin.test_client().post('/service-settings/delete')
 
     assert response.status_code == 302
-    assert 'http://localhost/' == response.location
+    assert 'http://localhost/service-settings/delete/confirm' == response.location
+
+
+def test_should_show_delete_confirmation(notifications_admin):
+    response = notifications_admin.test_client().get('/service-settings/delete/confirm')
+
+    assert response.status_code == 200
+    assert 'Delete service' in response.get_data(as_text=True)
+
+
+def test_should_redirect_delete_confirmation(notifications_admin):
+    response = notifications_admin.test_client().post('/service-settings/delete/confirm')
+
+    assert response.status_code == 302
+    assert 'http://localhost/dashboard' == response.location