From 3f01da05c75a8361e40a989f14853cb4c72776b7 Mon Sep 17 00:00:00 2001
From: Chris Hill-Scott <me@quis.cc>
Date: Wed, 13 Dec 2017 11:42:41 +0000
Subject: [PATCH] Fix order of callbacks

Delivery comes before inbound. The order of the URLs was jumbled in two
places:
- in the view function
- in the Jinja template

So as the user saw it the URLs were in the right order, because the
double jumbling cancelled itself out. But it made the code _really_
confusing to read.
---
 app/main/views/api_keys.py             |  2 +-
 app/templates/views/api/callbacks.html |  4 +--
 tests/app/main/views/test_api_keys.py  | 50 ++++++++++++++++++++------
 3 files changed, 43 insertions(+), 13 deletions(-)

diff --git a/app/main/views/api_keys.py b/app/main/views/api_keys.py
index 4a86986a0..d8e6225f7 100644
--- a/app/main/views/api_keys.py
+++ b/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',
diff --git a/app/templates/views/api/callbacks.html b/app/templates/views/api/callbacks.html
index 6d502f67f..54a86bafe 100644
--- a/app/templates/views/api/callbacks.html
+++ b/app/templates/views/api/callbacks.html
@@ -19,13 +19,13 @@
 	  ) %}
 	    {% call row() %}
 	      {{ text_field('Callbacks for delivery receipts') }}
-	      {{ optional_text_field(received_text_messages_callback) }}
+	      {{ optional_text_field(delivery_status_callback) }}
 	      {{ edit_field('Change', url_for('.delivery_status_callback', service_id=current_service.id)) }}
 	    {% endcall %}
 
 	    {% call row() %}
 	      {{ text_field('Callbacks for received text messages') }}
-	      {{ optional_text_field(delivery_status_callback) }}
+        {{ optional_text_field(received_text_messages_callback) }}
 	    	{{ edit_field('Change', url_for('.received_text_messages_callback', service_id=current_service.id)) }}
 	    {% endcall %}
 	  {% endcall %}
diff --git a/tests/app/main/views/test_api_keys.py b/tests/app/main/views/test_api_keys.py
index 096426043..b5e11f251 100644
--- a/tests/app/main/views/test_api_keys.py
+++ b/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,
@@ -639,23 +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 = [
-        'Callbacks for delivery receipts Not set Change',
-        'Callbacks for received text messages Not set Change',
+        expected_1st_table_row,
+        expected_2nd_table_row,
     ]
-    rows = page.select('tr')
-    assert len(rows) == 3
+    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)