From 703b48b157431c74f8d8f4b091c28f0b74534c8d Mon Sep 17 00:00:00 2001
From: Rebecca Law <rebecca.law@digital.cabinet-office.gov.uk>
Date: Thu, 15 Jun 2017 16:20:07 +0100
Subject: [PATCH 01/11] [WIP] Page and form to persist the inbound api data for
 a service.

---
 app/main/forms.py                             | 11 ++++++
 app/main/views/service_settings.py            | 24 ++++++++++++-
 app/notify_client/service_api_client.py       |  8 +++++
 app/templates/views/service-settings.html     |  7 ++++
 .../service-settings/set-inbound-api.html     | 35 +++++++++++++++++++
 5 files changed, 84 insertions(+), 1 deletion(-)
 create mode 100644 app/templates/views/service-settings/set-inbound-api.html

diff --git a/app/main/forms.py b/app/main/forms.py
index f8db6e577..e233b37a5 100644
--- a/app/main/forms.py
+++ b/app/main/forms.py
@@ -651,6 +651,17 @@ class PlaceholderForm(Form):
     pass
 
 
+class ServiceInboundApiForm(Form):
+    url = StringField("Inbound sms url",
+                      validators=[DataRequired(message='Can’t be empty'),
+                                  Regexp(regex="^https.*",
+                                         message='Must be a valid https url')]
+                      )
+    bearer_token = StringField("Bearer token",
+                               validators=[DataRequired(message='Can’t be empty'),
+                                           Length(min=5, message='Must be at least 10 characters')])
+
+
 def get_placeholder_form_instance(
     placeholder_name,
     dict_to_populate_from,
diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py
index d1667bfa7..7f3762a8c 100644
--- a/app/main/views/service_settings.py
+++ b/app/main/views/service_settings.py
@@ -30,7 +30,7 @@ from app.main.forms import (
     ServiceLetterContactBlock,
     ServiceBrandingOrg,
     LetterBranding,
-)
+    ServiceInboundApiForm)
 from app import user_api_client, current_service, organisations_client
 
 
@@ -38,6 +38,7 @@ 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):
+    print(current_service)
     letter_branding_organisations = organisations_client.get_letter_organisations()
     if current_service['organisation']:
         organisation = organisations_client.get_organisation(current_service['organisation'])['organisation']
@@ -410,3 +411,24 @@ def get_branding_as_dict(organisations):
             'colour': organisation['colour']
         } for organisation in organisations
     }
+
+
+@main.route("/services/<service_id>/service-settings/set-inbound-api", methods=['GET', 'POST'])
+@login_required
+@user_has_permissions('manage_settings', admin_override=True)
+def service_set_inbound_api(service_id):
+    form = ServiceInboundApiForm()
+
+    if form.validate_on_submit():
+        service_api_client.update_service_inbound_api(
+            service_id,
+            url=form.url.data,
+            bearer_token=form.bearer_token.data,
+            user_id=current_user.id
+        )
+        return redirect(url_for('.service_settings', service_id=service_id))
+
+    return render_template(
+        'views/service-settings/set-inbound-api.html',
+        form=form,
+    )
diff --git a/app/notify_client/service_api_client.py b/app/notify_client/service_api_client.py
index bbc4fdf81..6c30c3e8a 100644
--- a/app/notify_client/service_api_client.py
+++ b/app/notify_client/service_api_client.py
@@ -266,6 +266,14 @@ class ServiceAPIClient(NotifyAdminAPIClient):
             '/service/{}/inbound-sms/summary'.format(service_id)
         )
 
+    def update_service_inbound_api(self, service_id, url, bearer_token, user_id):
+        data = {
+            "url":url,
+            "bearer_token": bearer_token,
+            "updated_by_id": user_id
+        }
+        return self.post("/service/{}/inbound-api".format(service_id), data)
+
 
 class ServicesBrowsableItem(BrowsableItem):
     @property
diff --git a/app/templates/views/service-settings.html b/app/templates/views/service-settings.html
index 60fcf1f3e..0eb557a0e 100644
--- a/app/templates/views/service-settings.html
+++ b/app/templates/views/service-settings.html
@@ -72,6 +72,13 @@
             {{ edit_field('Change', url_for('.service_set_letter_contact_block', service_id=current_service.id)) }}
           {% endcall %}
         {% endif %}
+
+        {% call row() %}
+          {{ text_field('Inbound API') }}
+             {{ boolean_field(current_service.can_send_letters) }}
+            {{ edit_field('Change', url_for('.service_set_inbound_api', service_id=current_service.id)) }}
+          {% endcall %}
+
       {% endcall %}
     </div>
 
diff --git a/app/templates/views/service-settings/set-inbound-api.html b/app/templates/views/service-settings/set-inbound-api.html
new file mode 100644
index 000000000..e858e20a2
--- /dev/null
+++ b/app/templates/views/service-settings/set-inbound-api.html
@@ -0,0 +1,35 @@
+{% extends "withnav_template.html" %}
+{% from "components/textbox.html" import textbox %}
+{% from "components/page-footer.html" import page_footer %}
+
+{% block service_page_title %}
+  Inbound api
+{% endblock %}
+
+{% block maincolumn_content %}
+
+  <h1 class="heading-large">Inbound API</h1>
+  <p>
+    This is the https url that the inbound SMS messages will be posted to
+      and the bearer token used in the authorisation header of the request.
+  </p>
+
+  <form method="post">
+    {{ textbox(
+      form.url,
+      width='1-4',
+      hint='Valid https url'
+    ) }}
+    {{ textbox(
+      form.bearer_token,
+      width='1-4',
+      hint='At least 10 characters'
+    ) }}
+      {{ page_footer(
+      'Save',
+      back_link=url_for('.service_settings', service_id=current_service.id),
+      back_link_text='Back to settings'
+  ) }}
+  </form>
+
+{% endblock %}
\ No newline at end of file

From f40448f80e5378ca9ebbc360daf02d1f70058443 Mon Sep 17 00:00:00 2001
From: Rebecca Law <rebecca.law@digital.cabinet-office.gov.uk>
Date: Mon, 19 Jun 2017 11:35:25 +0100
Subject: [PATCH 02/11] Remove print

---
 app/main/views/service_settings.py | 1 -
 1 file changed, 1 deletion(-)

diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py
index 7f3762a8c..c14ddfb10 100644
--- a/app/main/views/service_settings.py
+++ b/app/main/views/service_settings.py
@@ -38,7 +38,6 @@ 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):
-    print(current_service)
     letter_branding_organisations = organisations_client.get_letter_organisations()
     if current_service['organisation']:
         organisation = organisations_client.get_organisation(current_service['organisation'])['organisation']

From 5b54dd53a2e4fe79f6bab68da019aedb3ce523d2 Mon Sep 17 00:00:00 2001
From: Ken Tsang <ken.tsang@digital.cabinet-office.gov.uk>
Date: Tue, 20 Jun 2017 13:03:28 +0100
Subject: [PATCH 03/11] Change bearer input to password

---
 app/main/forms.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/app/main/forms.py b/app/main/forms.py
index e233b37a5..78827b418 100644
--- a/app/main/forms.py
+++ b/app/main/forms.py
@@ -657,9 +657,9 @@ class ServiceInboundApiForm(Form):
                                   Regexp(regex="^https.*",
                                          message='Must be a valid https url')]
                       )
-    bearer_token = StringField("Bearer token",
-                               validators=[DataRequired(message='Can’t be empty'),
-                                           Length(min=5, message='Must be at least 10 characters')])
+    bearer_token = PasswordField("Bearer token",
+                                 validators=[DataRequired(message='Can’t be empty'),
+                                             Length(min=10, message='Must be at least 10 characters')])
 
 
 def get_placeholder_form_instance(

From d9286b036f53c8b2ac5446c20d67be491cf293ed Mon Sep 17 00:00:00 2001
From: Ken Tsang <ken.tsang@digital.cabinet-office.gov.uk>
Date: Tue, 20 Jun 2017 13:06:46 +0100
Subject: [PATCH 04/11] Add get and update service inbound api

---
 app/notify_client/service_api_client.py | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/app/notify_client/service_api_client.py b/app/notify_client/service_api_client.py
index 6c30c3e8a..70ee891ac 100644
--- a/app/notify_client/service_api_client.py
+++ b/app/notify_client/service_api_client.py
@@ -266,13 +266,21 @@ class ServiceAPIClient(NotifyAdminAPIClient):
             '/service/{}/inbound-sms/summary'.format(service_id)
         )
 
-    def update_service_inbound_api(self, service_id, url, bearer_token, user_id):
+    def update_service_inbound_api(self, service_id, url, bearer_token, user_id, inbound_api_id):
         data = {
-            "url":url,
+            "url": url,
             "bearer_token": bearer_token,
             "updated_by_id": user_id
         }
-        return self.post("/service/{}/inbound-api".format(service_id), data)
+        update_inbound_api_path = '/{}'.format(inbound_api_id) if inbound_api_id else ''
+        return self.post("/service/{}/inbound-api{}".format(service_id, update_inbound_api_path), data)
+
+    def get_service_inbound_api(self, service_id, inbound_sms_api_id):
+        return self.get(
+            "/service/{}/inbound-api/{}".format(
+                service_id, inbound_sms_api_id
+            )
+        )['data']
 
 
 class ServicesBrowsableItem(BrowsableItem):

From 5d5bb69f0eade5cb0bbc1f03c7442d2a2b5b9d46 Mon Sep 17 00:00:00 2001
From: Ken Tsang <ken.tsang@digital.cabinet-office.gov.uk>
Date: Tue, 20 Jun 2017 13:07:50 +0100
Subject: [PATCH 05/11] Refactored set inbound api page

---
 .../service-settings/set-inbound-api.html     | 49 ++++++++++---------
 1 file changed, 26 insertions(+), 23 deletions(-)

diff --git a/app/templates/views/service-settings/set-inbound-api.html b/app/templates/views/service-settings/set-inbound-api.html
index e858e20a2..0609ff126 100644
--- a/app/templates/views/service-settings/set-inbound-api.html
+++ b/app/templates/views/service-settings/set-inbound-api.html
@@ -7,29 +7,32 @@
 {% endblock %}
 
 {% block maincolumn_content %}
+  <div class="grid-row">
+    <div class="column-five-sixths">
+      <h1 class="heading-large">API endpoint for received text messages</h1>
+      <p>
+        This is the https url that the inbound SMS messages will be posted to
+          and the bearer token used in the authorisation header of the request.
+      </p>
 
-  <h1 class="heading-large">Inbound API</h1>
-  <p>
-    This is the https url that the inbound SMS messages will be posted to
-      and the bearer token used in the authorisation header of the request.
-  </p>
-
-  <form method="post">
-    {{ textbox(
-      form.url,
-      width='1-4',
-      hint='Valid https url'
-    ) }}
-    {{ textbox(
-      form.bearer_token,
-      width='1-4',
-      hint='At least 10 characters'
-    ) }}
-      {{ page_footer(
-      'Save',
-      back_link=url_for('.service_settings', service_id=current_service.id),
-      back_link_text='Back to settings'
-  ) }}
-  </form>
+      <form method="post">
+        {{ textbox(
+          form.url,
+          width='2-3',
+          hint='Valid https url'
+        ) }}
+        {{ textbox(
+          form.bearer_token,
+          width='1-4',
+          hint='At least 10 characters'
+        ) }}
+          {{ page_footer(
+          'Save',
+          back_link=url_for('.service_settings', service_id=current_service.id),
+          back_link_text='Back to settings'
+      ) }}
+      </form>
+    </div>
+  </div>
 
 {% endblock %}
\ No newline at end of file

From 827c6ccc99a3908bae9e9744b2f591f25b8f93f6 Mon Sep 17 00:00:00 2001
From: Ken Tsang <ken.tsang@digital.cabinet-office.gov.uk>
Date: Tue, 20 Jun 2017 13:24:01 +0100
Subject: [PATCH 06/11] Refactor inbound api html and processing

---
 app/main/views/service_settings.py        | 55 ++++++++++++++++++-----
 app/templates/views/service-settings.html | 17 ++++---
 2 files changed, 54 insertions(+), 18 deletions(-)

diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py
index c14ddfb10..37034f93f 100644
--- a/app/main/views/service_settings.py
+++ b/app/main/views/service_settings.py
@@ -1,3 +1,5 @@
+from urllib.parse import urlparse
+
 import requests
 from flask import (
     render_template,
@@ -34,6 +36,14 @@ from app.main.forms import (
 from app import user_api_client, current_service, organisations_client
 
 
+def get_inbound_api():
+    if current_service['inbound_api']:
+        return service_api_client.get_service_inbound_api(
+            current_service['id'],
+            current_service.get('inbound_api')[0]
+        )
+
+
 @main.route("/services/<service_id>/service-settings")
 @login_required
 @user_has_permissions('manage_settings', admin_override=True)
@@ -43,6 +53,15 @@ def service_settings(service_id):
         organisation = organisations_client.get_organisation(current_service['organisation'])['organisation']
     else:
         organisation = None
+
+    inbound_api = get_inbound_api()
+    if inbound_api:
+        parsed_url = urlparse(inbound_api.get('url')) if inbound_api else ''
+        inbound_api_url = '{uri.scheme}://{uri.netloc}{elide_token}'.format(
+            uri=parsed_url, elide_token='...' if parsed_url.path else '')
+    else:
+        inbound_api_url = ''
+
     return render_template(
         'views/service-settings.html',
         organisation=organisation,
@@ -50,6 +69,7 @@ def service_settings(service_id):
             current_service.get('dvla_organisation', '001')
         ),
         can_receive_inbound=('inbound_sms' in current_service['permissions']),
+        inbound_api_url=inbound_api_url,
         letter_contact_block=Field(current_service['letter_contact_block'], html='escape')
     )
 
@@ -268,19 +288,25 @@ def service_set_reply_to_email(service_id):
 @user_has_permissions('manage_settings', admin_override=True)
 def service_set_sms_sender(service_id):
     form = ServiceSmsSender()
+
+    def update_service(permissions, sms_sender):
+        service_api_client.update_service_with_properties(
+            current_service['id'],
+            {'permissions': permissions, 'sms_sender': sms_sender}
+        )
+
+    set_inbound_sms = request.args.get('set_inbound_sms')
+    if set_inbound_sms == 'True':
+        if 'inbound_sms' in current_service['permissions']:
+            current_service['permissions'].remove('inbound_sms')
+            update_service(current_service['permissions'], current_service['sms_sender'])
+            return redirect(url_for('.service_settings', service_id=service_id))
+
     if form.validate_on_submit():
-        set_inbound_sms = request.args.get('set_inbound_sms', False)
         if set_inbound_sms == 'True':
             permissions = current_service['permissions']
-            if 'inbound_sms' in permissions:
-                permissions.remove('inbound_sms')
-            else:
-                permissions.append('inbound_sms')
-            service_api_client.update_service_with_properties(
-                current_service['id'],
-                {'permissions': permissions,
-                 'sms_sender': form.sms_sender.data or None}
-            )
+            permissions.append('inbound_sms')
+            update_service(permissions, form.sms_sender.data)
         else:
             service_api_client.update_service(
                 current_service['id'],
@@ -416,14 +442,19 @@ def get_branding_as_dict(organisations):
 @login_required
 @user_has_permissions('manage_settings', admin_override=True)
 def service_set_inbound_api(service_id):
-    form = ServiceInboundApiForm()
+    if 'inbound_sms' not in current_service['permissions']:
+        abort(403)
+
+    inbound_api = get_inbound_api()
+    form = ServiceInboundApiForm(url=inbound_api.get('url') if inbound_api else '')
 
     if form.validate_on_submit():
         service_api_client.update_service_inbound_api(
             service_id,
             url=form.url.data,
             bearer_token=form.bearer_token.data,
-            user_id=current_user.id
+            user_id=current_user.id,
+            inbound_api_id=inbound_api.get('id') if inbound_api else ''
         )
         return redirect(url_for('.service_settings', service_id=service_id))
 
diff --git a/app/templates/views/service-settings.html b/app/templates/views/service-settings.html
index 0eb557a0e..762126ee0 100644
--- a/app/templates/views/service-settings.html
+++ b/app/templates/views/service-settings.html
@@ -57,6 +57,17 @@
           {{ edit_field('Change', url_for('.service_set_inbound_sms', service_id=current_service.id)) }}
         {% endcall %}
 
+        {% if 'inbound_sms' in current_service.permissions %}
+          {% call row() %}
+            {{ text_field('API endpoint for received text messages') }}
+            {{ text_field(
+              'None' if not inbound_api_url else inbound_api_url,
+              status='' if inbound_api_url else 'default'
+            ) }}
+            {{ edit_field('Change', url_for('.service_set_inbound_api', service_id=current_service.id)) }}
+          {% endcall %}
+        {% endif %}
+
         {% call row() %}
           {{ text_field('Letters') }}
           {{ boolean_field(current_service.can_send_letters) }}
@@ -73,12 +84,6 @@
           {% endcall %}
         {% endif %}
 
-        {% call row() %}
-          {{ text_field('Inbound API') }}
-             {{ boolean_field(current_service.can_send_letters) }}
-            {{ edit_field('Change', url_for('.service_set_inbound_api', service_id=current_service.id)) }}
-          {% endcall %}
-
       {% endcall %}
     </div>
 

From d3d0c0c0b60addd8e879b1d8efaa0c6cc02c84c3 Mon Sep 17 00:00:00 2001
From: Ken Tsang <ken.tsang@digital.cabinet-office.gov.uk>
Date: Tue, 20 Jun 2017 13:29:20 +0100
Subject: [PATCH 07/11] Add create_service_inbound_api

---
 app/main/views/service_settings.py      | 22 +++++++++++++++-------
 app/notify_client/service_api_client.py | 11 +++++++++--
 2 files changed, 24 insertions(+), 9 deletions(-)

diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py
index 37034f93f..ff763e7ec 100644
--- a/app/main/views/service_settings.py
+++ b/app/main/views/service_settings.py
@@ -449,13 +449,21 @@ def service_set_inbound_api(service_id):
     form = ServiceInboundApiForm(url=inbound_api.get('url') if inbound_api else '')
 
     if form.validate_on_submit():
-        service_api_client.update_service_inbound_api(
-            service_id,
-            url=form.url.data,
-            bearer_token=form.bearer_token.data,
-            user_id=current_user.id,
-            inbound_api_id=inbound_api.get('id') if inbound_api else ''
-        )
+        if inbound_api:
+            service_api_client.update_service_inbound_api(
+                service_id,
+                url=form.url.data,
+                bearer_token=form.bearer_token.data,
+                user_id=current_user.id,
+                inbound_api_id=inbound_api.get('id') if inbound_api else ''
+            )
+        else:
+            service_api_client.create_service_inbound_api(
+                service_id,
+                url=form.url.data,
+                bearer_token=form.bearer_token.data,
+                user_id=current_user.id
+            )
         return redirect(url_for('.service_settings', service_id=service_id))
 
     return render_template(
diff --git a/app/notify_client/service_api_client.py b/app/notify_client/service_api_client.py
index 70ee891ac..0fa530999 100644
--- a/app/notify_client/service_api_client.py
+++ b/app/notify_client/service_api_client.py
@@ -266,14 +266,21 @@ class ServiceAPIClient(NotifyAdminAPIClient):
             '/service/{}/inbound-sms/summary'.format(service_id)
         )
 
+    def create_service_inbound_api(self, service_id, url, bearer_token, user_id):
+        data = {
+            "url": url,
+            "bearer_token": bearer_token,
+            "updated_by_id": user_id
+        }
+        return self.post("/service/{}/inbound-api".format(service_id), data)
+
     def update_service_inbound_api(self, service_id, url, bearer_token, user_id, inbound_api_id):
         data = {
             "url": url,
             "bearer_token": bearer_token,
             "updated_by_id": user_id
         }
-        update_inbound_api_path = '/{}'.format(inbound_api_id) if inbound_api_id else ''
-        return self.post("/service/{}/inbound-api{}".format(service_id, update_inbound_api_path), data)
+        return self.post("/service/{}/inbound-api/{}".format(service_id, inbound_api_id), data)
 
     def get_service_inbound_api(self, service_id, inbound_sms_api_id):
         return self.get(

From b2d07f1bb3fba8eb82a8f9f057f09b8141d44587 Mon Sep 17 00:00:00 2001
From: Ken Tsang <ken.tsang@digital.cabinet-office.gov.uk>
Date: Tue, 20 Jun 2017 17:51:58 +0100
Subject: [PATCH 08/11] Refactor test to use flag

---
 app/templates/views/service-settings.html | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/templates/views/service-settings.html b/app/templates/views/service-settings.html
index 762126ee0..52f053cfa 100644
--- a/app/templates/views/service-settings.html
+++ b/app/templates/views/service-settings.html
@@ -57,7 +57,7 @@
           {{ edit_field('Change', url_for('.service_set_inbound_sms', service_id=current_service.id)) }}
         {% endcall %}
 
-        {% if 'inbound_sms' in current_service.permissions %}
+        {% if can_receive_inbound %}
           {% call row() %}
             {{ text_field('API endpoint for received text messages') }}
             {{ text_field(

From 3b47ff28f02fff3acb5abc3731ad9f427c6827ba Mon Sep 17 00:00:00 2001
From: Ken Tsang <ken.tsang@digital.cabinet-office.gov.uk>
Date: Wed, 21 Jun 2017 12:15:53 +0100
Subject: [PATCH 09/11] Refactored tests for inbound api

---
 app/main/views/service_settings.py            |  27 ++--
 tests/__init__.py                             |   4 +
 tests/app/main/views/test_service_settings.py | 119 ++++++++++++++++--
 3 files changed, 125 insertions(+), 25 deletions(-)

diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py
index ff763e7ec..654955047 100644
--- a/app/main/views/service_settings.py
+++ b/app/main/views/service_settings.py
@@ -289,24 +289,19 @@ def service_set_reply_to_email(service_id):
 def service_set_sms_sender(service_id):
     form = ServiceSmsSender()
 
-    def update_service(permissions, sms_sender):
-        service_api_client.update_service_with_properties(
-            current_service['id'],
-            {'permissions': permissions, 'sms_sender': sms_sender}
-        )
-
-    set_inbound_sms = request.args.get('set_inbound_sms')
-    if set_inbound_sms == 'True':
-        if 'inbound_sms' in current_service['permissions']:
-            current_service['permissions'].remove('inbound_sms')
-            update_service(current_service['permissions'], current_service['sms_sender'])
-            return redirect(url_for('.service_settings', service_id=service_id))
-
     if form.validate_on_submit():
+        set_inbound_sms = request.args.get('set_inbound_sms', False)
         if set_inbound_sms == 'True':
             permissions = current_service['permissions']
-            permissions.append('inbound_sms')
-            update_service(permissions, form.sms_sender.data)
+            if 'inbound_sms' in permissions:
+                permissions.remove('inbound_sms')
+            else:
+                permissions.append('inbound_sms')
+            service_api_client.update_service_with_properties(
+                current_service['id'],
+                {'permissions': permissions,
+                 'sms_sender': form.sms_sender.data or None}
+            )
         else:
             service_api_client.update_service(
                 current_service['id'],
@@ -455,7 +450,7 @@ def service_set_inbound_api(service_id):
                 url=form.url.data,
                 bearer_token=form.bearer_token.data,
                 user_id=current_user.id,
-                inbound_api_id=inbound_api.get('id') if inbound_api else ''
+                inbound_api_id=inbound_api.get('id')
             )
         else:
             service_api_client.create_service_inbound_api(
diff --git a/tests/__init__.py b/tests/__init__.py
index 8560ead2d..498fc1132 100644
--- a/tests/__init__.py
+++ b/tests/__init__.py
@@ -56,11 +56,14 @@ def service_json(
     created_at=None,
     letter_contact_block=None,
     permissions=None,
+    inbound_api=None,
 ):
     if users is None:
         users = []
     if permissions is None:
         permissions = []
+    if inbound_api is None:
+        inbound_api = []
     return {
         'id': id_,
         'name': name,
@@ -80,6 +83,7 @@ def service_json(
         'letter_contact_block': letter_contact_block,
         'dvla_organisation': '001',
         'permissions': permissions,
+        'inbound_api': inbound_api,
     }
 
 
diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py
index 4228d4f8e..a211a1205 100644
--- a/tests/app/main/views/test_service_settings.py
+++ b/tests/app/main/views/test_service_settings.py
@@ -60,6 +60,25 @@ def test_should_show_overview(
     app.service_api_client.get_service.assert_called_with(service_one['id'])
 
 
+@pytest.mark.parametrize('permissions, expected_rows', [
+    (['email', 'sms', 'inbound_sms'], [
+        'Service name service one Change',
+        'Email reply to address test@example.com Change',
+        'Text message sender elevenchars',
+        'International text messages On Change',
+        'Receive text messages On Change',
+        'API endpoint for received text messages None Change',
+        'Letters Off Change',
+    ]),
+    (['email', 'sms'], [
+        'Service name service one Change',
+        'Email reply to address test@example.com Change',
+        'Text message sender elevenchars Change',
+        'International text messages On Change',
+        'Receive text messages Off Change',
+        'Letters Off Change',
+    ]),
+])
 def test_should_show_overview_for_service_with_more_things_set(
     client,
     active_user_with_permissions,
@@ -67,22 +86,17 @@ def test_should_show_overview_for_service_with_more_things_set(
     service_with_reply_to_addresses,
     mock_get_organisation,
     mock_get_letter_organisations,
+    permissions,
+    expected_rows
 ):
     client.login(active_user_with_permissions, mocker, service_with_reply_to_addresses)
-    service_with_reply_to_addresses['permissions'] = ['inbound_sms']
+    service_with_reply_to_addresses['permissions'] = permissions
     service_with_reply_to_addresses['can_send_international_sms'] = True
     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',
-        'International text messages On Change',
-        'Receive text messages On Change',
-        'Letters Off Change',
-    ]):
+    for index, row in enumerate(expected_rows):
         assert row == " ".join(page.find_all('tr')[index + 1].text.split())
 
 
@@ -736,6 +750,34 @@ def test_set_text_message_sender_validation(
     assert not mock_update_service.called
 
 
+@pytest.mark.parametrize('url, bearer_token, expected_errors', [
+    ("", "", "Can’t be empty Can’t be empty"),
+    ("http://not_https.com", "1234567890", "Must be a valid https url"),
+    ("https://test.com", "123456789", "Must be at least 10 characters"),
+])
+def test_set_inbound_api_validation(
+    logged_in_client,
+    mock_update_service,
+    service_one,
+    mock_get_letter_organisations,
+    url,
+    bearer_token,
+    expected_errors,
+):
+    service_one['permissions'] = ['inbound_sms']
+    response = logged_in_client.post(url_for(
+        'main.service_set_inbound_api',
+        service_id=service_one['id']),
+        data={"url": url, "bearer_token": bearer_token}
+    )
+    page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
+    error_msgs = ' '.join(msg.text.strip() for msg in page.select(".error-message"))
+
+    assert response.status_code == 200
+    assert error_msgs == expected_errors
+    assert not mock_update_service.called
+
+
 def test_if_sms_sender_set_then_form_populated(
     logged_in_client,
     service_one,
@@ -1008,6 +1050,65 @@ def test_switch_service_disable_international_sms(
     assert mocked_fn.call_args == call(service_one['id'], {"can_send_international_sms": False})
 
 
+def test_set_new_inbound_api_and_valid_bearer_token_calls_create_inbound_api_endpoint(
+    logged_in_platform_admin_client,
+    service_one,
+    mocker,
+):
+    service_one['permissions'] = ['inbound_sms']
+    service_one['inbound_api'] = []
+
+    mocked_post_fn = mocker.patch('app.service_api_client.post', return_value=service_one)
+
+    inbound_api_data = {'url': "https://test.url.com/", 'bearer_token': '1234567890'}
+    response = logged_in_platform_admin_client.post(
+        url_for(
+            'main.service_set_inbound_api',
+            service_id=service_one['id']
+        ),
+        data=inbound_api_data
+    )
+    assert response.status_code == 302
+    assert response.location == url_for('main.service_settings', service_id=service_one['id'], _external=True)
+    assert mocked_post_fn.called
+
+    inbound_api_data['updated_by_id'] = service_one['users'][0]
+    assert mocked_post_fn.call_args == call("/service/{}/inbound-api".format(service_one['id']), inbound_api_data)
+
+
+def test_update_inbound_api_and_valid_bearer_token_calls_update_inbound_api_endpoint(
+    logged_in_platform_admin_client,
+    service_one,
+    mocker,
+    fake_uuid
+):
+    service_one['permissions'] = ['inbound_sms']
+    service_one['inbound_api'] = [fake_uuid]
+
+    mocked_get_fn = mocker.patch(
+        'app.service_api_client.get',
+        return_value={'data': {'id': fake_uuid, 'url': "https://test.url.com/"}})
+    mocked_post_fn = mocker.patch('app.service_api_client.post', return_value=service_one)
+
+    inbound_api_data = {'url': "https://test.url.com/", 'bearer_token': '1234567890', 'inbound_api_id': fake_uuid}
+    response = logged_in_platform_admin_client.post(
+        url_for(
+            'main.service_set_inbound_api',
+            service_id=service_one['id']
+        ),
+        data=inbound_api_data
+    )
+    assert response.status_code == 302
+    assert response.location == url_for('main.service_settings', service_id=service_one['id'], _external=True)
+    assert mocked_post_fn.called
+
+    del inbound_api_data['inbound_api_id']
+    inbound_api_data['updated_by_id'] = service_one['users'][0]
+
+    assert mocked_post_fn.call_args == call(
+        "/service/{}/inbound-api/{}".format(service_one['id'], fake_uuid), inbound_api_data)
+
+
 def test_archive_service_after_confirm(
     logged_in_platform_admin_client,
     service_one,

From ba8ee3e60a135914b1cf71256fa68f4b6532a17c Mon Sep 17 00:00:00 2001
From: Ken Tsang <ken.tsang@digital.cabinet-office.gov.uk>
Date: Wed, 21 Jun 2017 13:43:52 +0100
Subject: [PATCH 10/11] Add test for elided inbound api urls

---
 tests/app/main/views/test_service_settings.py | 36 +++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py
index a211a1205..d0cc9ad7b 100644
--- a/tests/app/main/views/test_service_settings.py
+++ b/tests/app/main/views/test_service_settings.py
@@ -100,6 +100,42 @@ def test_should_show_overview_for_service_with_more_things_set(
         assert row == " ".join(page.find_all('tr')[index + 1].text.split())
 
 
+@pytest.mark.parametrize('url, elided_url', [
+    ('https://test.url.com/inbound', 'https://test.url.com...'),
+    ('https://test.url.com/', 'https://test.url.com...'),
+    ('https://test.url.com', 'https://test.url.com'),
+])
+def test_service_settings_show_elided_api_url_if_needed(
+    logged_in_platform_admin_client,
+    service_one,
+    mock_get_letter_organisations,
+    mocker,
+    fake_uuid,
+    url,
+    elided_url
+):
+    service_one['permissions'] = ['inbound_sms']
+    service_one['inbound_api'] = [fake_uuid]
+
+    mocked_get_fn = mocker.patch(
+        'app.service_api_client.get',
+        return_value={'data': {'id': fake_uuid, 'url': url}})
+
+    response = logged_in_platform_admin_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')
+
+    non_empty_trs = [tr.find_all('td') for tr in page.find_all('tr') if tr.find_all('td')]
+    api_url = [api_setting[1].text.strip() for api_setting in non_empty_trs
+               if api_setting[0].text.strip() == 'API endpoint for received text messages'][0]
+    assert api_url == elided_url
+
+
 def test_if_cant_send_letters_then_cant_see_letter_contact_block(
     logged_in_client,
     service_one,

From c5e1de9b330763196b8b90629ae593d4e6041307 Mon Sep 17 00:00:00 2001
From: Ken Tsang <ken.tsang@digital.cabinet-office.gov.uk>
Date: Wed, 21 Jun 2017 17:34:22 +0100
Subject: [PATCH 11/11] Enable update of url / bearer_token inbound api

---
 app/main/forms.py                             | 11 +++-
 app/main/views/service_settings.py            | 23 +++++---
 app/notify_client/service_api_client.py       |  3 +-
 tests/app/main/views/test_service_settings.py | 59 +++++++++++++++++--
 4 files changed, 78 insertions(+), 18 deletions(-)

diff --git a/app/main/forms.py b/app/main/forms.py
index 78827b418..d9afad2f1 100644
--- a/app/main/forms.py
+++ b/app/main/forms.py
@@ -9,6 +9,7 @@ from notifications_utils.recipients import (
 )
 from notifications_utils.columns import Columns
 from wtforms import (
+    widgets,
     validators,
     StringField,
     PasswordField,
@@ -651,15 +652,19 @@ class PlaceholderForm(Form):
     pass
 
 
+class PasswordFieldShowHasContent(StringField):
+    widget = widgets.PasswordInput(hide_value=False)
+
+
 class ServiceInboundApiForm(Form):
     url = StringField("Inbound sms url",
                       validators=[DataRequired(message='Can’t be empty'),
                                   Regexp(regex="^https.*",
                                          message='Must be a valid https url')]
                       )
-    bearer_token = PasswordField("Bearer token",
-                                 validators=[DataRequired(message='Can’t be empty'),
-                                             Length(min=10, message='Must be at least 10 characters')])
+    bearer_token = PasswordFieldShowHasContent("Bearer token",
+                                               validators=[DataRequired(message='Can’t be empty'),
+                                                           Length(min=10, message='Must be at least 10 characters')])
 
 
 def get_placeholder_form_instance(
diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py
index 654955047..85e02c12c 100644
--- a/app/main/views/service_settings.py
+++ b/app/main/views/service_settings.py
@@ -36,6 +36,9 @@ from app.main.forms import (
 from app import user_api_client, current_service, organisations_client
 
 
+dummy_bearer_token = 'bearer_token_set'
+
+
 def get_inbound_api():
     if current_service['inbound_api']:
         return service_api_client.get_service_inbound_api(
@@ -441,17 +444,21 @@ def service_set_inbound_api(service_id):
         abort(403)
 
     inbound_api = get_inbound_api()
-    form = ServiceInboundApiForm(url=inbound_api.get('url') if inbound_api else '')
+    form = ServiceInboundApiForm(
+        url=inbound_api.get('url') if inbound_api else '',
+        bearer_token=dummy_bearer_token if inbound_api else ''
+    )
 
     if form.validate_on_submit():
         if inbound_api:
-            service_api_client.update_service_inbound_api(
-                service_id,
-                url=form.url.data,
-                bearer_token=form.bearer_token.data,
-                user_id=current_user.id,
-                inbound_api_id=inbound_api.get('id')
-            )
+            if inbound_api.get('url') != form.url.data or form.bearer_token.data != dummy_bearer_token:
+                service_api_client.update_service_inbound_api(
+                    service_id,
+                    url=form.url.data,
+                    bearer_token=form.bearer_token.data if form.bearer_token.data != dummy_bearer_token else '',
+                    user_id=current_user.id,
+                    inbound_api_id=inbound_api.get('id')
+                )
         else:
             service_api_client.create_service_inbound_api(
                 service_id,
diff --git a/app/notify_client/service_api_client.py b/app/notify_client/service_api_client.py
index 0fa530999..73b324924 100644
--- a/app/notify_client/service_api_client.py
+++ b/app/notify_client/service_api_client.py
@@ -277,9 +277,10 @@ class ServiceAPIClient(NotifyAdminAPIClient):
     def update_service_inbound_api(self, service_id, url, bearer_token, user_id, inbound_api_id):
         data = {
             "url": url,
-            "bearer_token": bearer_token,
             "updated_by_id": user_id
         }
+        if bearer_token:
+            data['bearer_token'] = bearer_token
         return self.post("/service/{}/inbound-api/{}".format(service_id, inbound_api_id), data)
 
     def get_service_inbound_api(self, service_id, inbound_sms_api_id):
diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py
index d0cc9ad7b..ad8f96888 100644
--- a/tests/app/main/views/test_service_settings.py
+++ b/tests/app/main/views/test_service_settings.py
@@ -7,6 +7,7 @@ from bs4 import BeautifulSoup
 from werkzeug.exceptions import InternalServerError
 
 import app
+from app.main.views.service_settings import dummy_bearer_token
 from app.utils import email_safe
 from tests import validate_route_permission, service_json
 from tests.app.test_utils import normalize_spaces
@@ -1112,21 +1113,39 @@ def test_set_new_inbound_api_and_valid_bearer_token_calls_create_inbound_api_end
     assert mocked_post_fn.call_args == call("/service/{}/inbound-api".format(service_one['id']), inbound_api_data)
 
 
+@pytest.mark.parametrize(
+    'inbound_api_data', [
+        {'url': "https://test.url.com/inbound", 'bearer_token': dummy_bearer_token},
+        {'url': "https://test.url.com/inbound", 'bearer_token': '1234567890'},
+        {'url': "https://test.url.com/", 'bearer_token': 'new_1234567890'},
+    ]
+)
 def test_update_inbound_api_and_valid_bearer_token_calls_update_inbound_api_endpoint(
     logged_in_platform_admin_client,
     service_one,
     mocker,
-    fake_uuid
+    fake_uuid,
+    inbound_api_data
 ):
     service_one['permissions'] = ['inbound_sms']
     service_one['inbound_api'] = [fake_uuid]
 
-    mocked_get_fn = mocker.patch(
-        'app.service_api_client.get',
-        return_value={'data': {'id': fake_uuid, 'url': "https://test.url.com/"}})
+    initial_api_data = {'data': {'id': fake_uuid, 'url': "https://test.url.com/"}}
+
+    mocked_get_fn = mocker.patch('app.service_api_client.get', return_value=initial_api_data)
     mocked_post_fn = mocker.patch('app.service_api_client.post', return_value=service_one)
 
-    inbound_api_data = {'url': "https://test.url.com/", 'bearer_token': '1234567890', 'inbound_api_id': fake_uuid}
+    response = logged_in_platform_admin_client.get(
+        url_for(
+            'main.service_set_inbound_api',
+            service_id=service_one['id']
+        )
+    )
+    page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
+
+    assert page.find('input', {'id': 'url'}).get('value') == initial_api_data['data']['url']
+    assert page.find('input', {'id': 'bearer_token'}).get('value') == dummy_bearer_token
+
     response = logged_in_platform_admin_client.post(
         url_for(
             'main.service_set_inbound_api',
@@ -1138,13 +1157,41 @@ def test_update_inbound_api_and_valid_bearer_token_calls_update_inbound_api_endp
     assert response.location == url_for('main.service_settings', service_id=service_one['id'], _external=True)
     assert mocked_post_fn.called
 
-    del inbound_api_data['inbound_api_id']
+    if inbound_api_data['bearer_token'] == dummy_bearer_token:
+        del inbound_api_data['bearer_token']
     inbound_api_data['updated_by_id'] = service_one['users'][0]
 
     assert mocked_post_fn.call_args == call(
         "/service/{}/inbound-api/{}".format(service_one['id'], fake_uuid), inbound_api_data)
 
 
+def test_save_inbound_api_without_changes_does_not_update_inbound_api(
+    logged_in_platform_admin_client,
+    service_one,
+    mocker,
+    fake_uuid
+):
+    service_one['permissions'] = ['inbound_sms']
+    service_one['inbound_api'] = [fake_uuid]
+
+    initial_api_data = {'data': {'id': fake_uuid, 'url': "https://test.url.com/"}}
+    inbound_api_data = {'url': initial_api_data['data']['url'], 'bearer_token': dummy_bearer_token}
+
+    mocked_get_fn = mocker.patch('app.service_api_client.get', return_value=initial_api_data)
+    mocked_post_fn = mocker.patch('app.service_api_client.post', return_value=service_one)
+
+    response = logged_in_platform_admin_client.post(
+        url_for(
+            'main.service_set_inbound_api',
+            service_id=service_one['id']
+        ),
+        data=inbound_api_data
+    )
+    assert response.status_code == 302
+    assert response.location == url_for('main.service_settings', service_id=service_one['id'], _external=True)
+    assert mocked_post_fn.called is False
+
+
 def test_archive_service_after_confirm(
     logged_in_platform_admin_client,
     service_one,