Merge pull request #1708 from alphagov/callbacks-wording

Tweaks to callback pages

app/main/views/api_keys.py

@@ -157,7 +157,7 @@ def api_callbacks(service_id):
     if 'inbound_sms' not in current_service['permissions']:
         return redirect(url_for('.delivery_status_callback', service_id=service_id))
 
-    received_text_messages_callback, delivery_status_callback = get_apis()
+    delivery_status_callback, received_text_messages_callback = get_apis()
 
     return render_template(
         'views/api/callbacks.html',

app/templates/views/api/callbacks.html

@@ -18,14 +18,14 @@
 	    caption_visible=False
 	  ) %}
 	    {% call row() %}
-	      {{ text_field('Delivery status callback URL') }}
-	      {{ optional_text_field(received_text_messages_callback) }}
+	      {{ text_field('Callbacks for delivery receipts') }}
+	      {{ optional_text_field(delivery_status_callback) }}
 	      {{ edit_field('Change', url_for('.delivery_status_callback', service_id=current_service.id)) }}
 	    {% endcall %}
 
 	    {% call row() %}
-	      {{ text_field('Received text messages callback URL') }}
-	      {{ optional_text_field(delivery_status_callback) }}
+	      {{ text_field('Callbacks for received text messages') }}
+        {{ optional_text_field(received_text_messages_callback) }}
 	    	{{ edit_field('Change', url_for('.received_text_messages_callback', service_id=current_service.id)) }}
 	    {% endcall %}
 	  {% endcall %}

app/templates/views/api/callbacks/delivery-status-callback.html

@@ -3,13 +3,13 @@
 {% from "components/page-footer.html" import page_footer %}
 
 {% block service_page_title %}
-  Callback for delivery receipts
+  Callbacks for delivery receipts
 {% endblock %}
 
 {% block maincolumn_content %}
   <div class="grid-row">
     <div class="column-five-sixths">
-      <h1 class="heading-large">Callback for delivery receipts</h1>
+      <h1 class="heading-large">Callbacks for delivery receipts</h1>
       <p>
         When you send an email or text message, we can tell you if Notify was able to deliver it.
         Check the <a href="{{ url_for('.callbacks') }}"> callback documentation </a> for more information.

app/templates/views/api/callbacks/received-text-messages-callback.html

@@ -3,13 +3,13 @@
 {% from "components/page-footer.html" import page_footer %}
 
 {% block service_page_title %}
-  Callback for received text messages
+  Callbacks for received text messages
 {% endblock %}
 
 {% block maincolumn_content %}
+  <h1 class="heading-large">Callbacks for received text messages</h1>
   <div class="grid-row">
     <div class="column-five-sixths">
-      <h1 class="heading-large">Callback for received text messages</h1>
       <p>
         When you receive a text message in Notify, we can forward it to your system.
         Check the <a href="{{ url_for('.callbacks') }}"> callback documentation </a> for more information.

tests/app/main/views/test_api_keys.py

@@ -8,6 +8,7 @@ from unittest.mock import call
 
 from tests import validate_route_permission
 from tests.conftest import (
+    fake_uuid,
     mock_get_service,
     mock_get_live_service,
     mock_get_service_with_letters,
@@ -467,7 +468,7 @@ def test_callbacks_page_redirects_to_delivery_status_if_service_has_no_inbound_s
         _follow_redirects=True,
     )
 
-    assert normalize_spaces(page.select_one('h1').text) == "Callback for delivery receipts"
+    assert normalize_spaces(page.select_one('h1').text) == "Callbacks for delivery receipts"
 
 
 @pytest.mark.parametrize('has_inbound_sms, expected_link', [
@@ -639,21 +640,52 @@ def test_update_delivery_status_and_receive_text_message_callbacks_without_chang
         assert mock_update_service_callback_api.called is False
 
 
+@pytest.mark.parametrize('service_callback_api, delivery_url, expected_1st_table_row', [
+    (
+        None, {},
+        'Callbacks for delivery receipts Not set Change'
+    ),
+    (
+        fake_uuid(), {'url': 'https://delivery.receipts'},
+        'Callbacks for delivery receipts https://delivery.receipts Change'
+    ),
+])
+@pytest.mark.parametrize('inbound_api, inbound_url, expected_2nd_table_row', [
+    (
+        None, {},
+        'Callbacks for received text messages Not set Change'
+    ),
+    (
+        fake_uuid(), {'url': 'https://inbound.sms'},
+        'Callbacks for received text messages https://inbound.sms Change'
+    ),
+])
 def test_callbacks_page_works_when_no_apis_set(
-        client_request,
-        service_one,
-        mocker
+    client_request,
+    service_one,
+    mocker,
+    service_callback_api,
+    delivery_url,
+    expected_1st_table_row,
+    inbound_api,
+    inbound_url,
+    expected_2nd_table_row,
 ):
     service_one['permissions'] = ['inbound_sms']
-    mocker.patch('app.service_api_client.get_service_callback_api', side_effect={})
-    mocker.patch('app.service_api_client.get_service_inbound_api', side_effect={})
+    service_one['inbound_api'] = inbound_api
+    service_one['service_callback_api'] = service_callback_api
+
+    mocker.patch('app.service_api_client.get_service_callback_api', return_value=delivery_url)
+    mocker.patch('app.service_api_client.get_service_inbound_api', return_value=inbound_url)
 
     page = client_request.get('main.api_callbacks',
                               service_id=service_one['id'],
                               _follow_redirects=True)
-    expected_rows = ['Delivery status callback URL Not set Change',
-                     'Received text messages callback URL Not set Change']
-    rows = page.select('tr')
-    assert len(rows) == 3
+    expected_rows = [
+        expected_1st_table_row,
+        expected_2nd_table_row,
+    ]
+    rows = page.select('tbody tr')
+    assert len(rows) == 2
     for index, row in enumerate(expected_rows):
-        assert row == " ".join(rows[index + 1].text.split())
+        assert row == normalize_spaces(rows[index].text)