darkhelm/notifications-admin
1
0
Fork 0
mirror of https://github.com/GSA/notifications-admin.git synced 2026-07-14 02:09:44 -04:00
Code Issues Packages Projects Releases Wiki Activity

Fix order of callbacks

Browse Source 
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.
This commit is contained in:
Chris Hill-Scott
2017-12-13 11:42:41 +00:00
parent f02ec30979
commit 3f01da05c7
3 changed files with 43 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
app/main/views/api_keys.py
View File

@@ -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',

4
app/templates/views/api/callbacks.html
View File

@@ -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 %}

50
tests/app/main/views/test_api_keys.py
View File

@@ -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)