darkhelm/notifications-api
1
0
Fork 0
mirror of https://github.com/GSA/notifications-api.git synced 2026-09-09 06:43:12 -04:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #802 from alphagov/return-200-for-duplicate-callback

Browse Source 
Return success of the callback is a duplicate or for an id that does not exist
This commit is contained in:
Rebecca Law
2017-01-27 14:02:56 +00:00
committed by GitHub
parent 568d93e67f 27b6aa736c
commit fba3332488
3 changed files with 580 additions and 634 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
app/notifications/process_client_response.py
View File

@@ -58,11 +58,11 @@ def process_sms_client_response(status, reference, client_name):
# record stats # record stats
notification = notifications_dao.update_notification_status_by_id(reference, notification_status) notification = notifications_dao.update_notification_status_by_id(reference, notification_status)
if not notification: if not notification:
status_error = "{} callback failed: notification {} either not found or already updated " \ current_app.logger.warning("{} callback failed: notification {} either not found or already updated "
"from sending. Status {}".format(client_name, "from sending. Status {}".format(client_name,
reference, reference,
notification_status_message) notification_status_message))
return success, status_error return success, errors
if not notification_success: if not notification_success:
current_app.logger.info( current_app.logger.info(

3
app/notifications/rest.py
View File

@@ -149,10 +149,7 @@ def process_firetext_response():
if errors: if errors:
raise InvalidRequest(errors, status_code=400) raise InvalidRequest(errors, status_code=400)
response_code = request.form.get('code')
status = request.form.get('status') status = request.form.get('status')
current_app.logger.info('Firetext status: {}, extended error code: {}'.format(status, response_code))
success, errors = process_sms_client_response(status=status, success, errors = process_sms_client_response(status=status,
reference=request.form.get('reference'), reference=request.form.get('reference'),
client_name=client_name) client_name=client_name)

141
tests/app/notifications/rest/test_callbacks.py
View File

@@ -1,9 +1,9 @@
import uuid import uuid
from datetime import datetime from datetime import datetime
from flask import json from flask import json
from freezegun import freeze_time from freezegun import freeze_time
from unittest.mock import call
import app.celery.tasks import app.celery.tasks
from app.dao.notifications_dao import ( from app.dao.notifications_dao import (
@@ -13,9 +13,7 @@ from app.models import NotificationStatistics
from tests.app.conftest import sample_notification as create_sample_notification from tests.app.conftest import sample_notification as create_sample_notification
def test_firetext_callback_should_not_need_auth(notify_api, mocker): def test_firetext_callback_should_not_need_auth(client, mocker):
with notify_api.test_request_context():
with notify_api.test_client() as client:
mocker.patch('app.statsd_client.incr') mocker.patch('app.statsd_client.incr')
response = client.post( response = client.post(
path='/notifications/sms/firetext', path='/notifications/sms/firetext',
@@ -25,9 +23,7 @@ def test_firetext_callback_should_not_need_auth(notify_api, mocker):
assert response.status_code == 200 assert response.status_code == 200
def test_firetext_callback_should_return_400_if_empty_reference(notify_api, mocker): def test_firetext_callback_should_return_400_if_empty_reference(client, mocker):
with notify_api.test_request_context():
with notify_api.test_client() as client:
mocker.patch('app.statsd_client.incr') mocker.patch('app.statsd_client.incr')
response = client.post( response = client.post(
path='/notifications/sms/firetext', path='/notifications/sms/firetext',
@@ -40,9 +36,7 @@ def test_firetext_callback_should_return_400_if_empty_reference(notify_api, mock
assert json_resp['message'] == ['Firetext callback failed: reference missing'] assert json_resp['message'] == ['Firetext callback failed: reference missing']
def test_firetext_callback_should_return_400_if_no_reference(notify_api, mocker): def test_firetext_callback_should_return_400_if_no_reference(client, mocker):
with notify_api.test_request_context():
with notify_api.test_client() as client:
mocker.patch('app.statsd_client.incr') mocker.patch('app.statsd_client.incr')
response = client.post( response = client.post(
path='/notifications/sms/firetext', path='/notifications/sms/firetext',
@@ -55,9 +49,7 @@ def test_firetext_callback_should_return_400_if_no_reference(notify_api, mocker)
assert json_resp['message'] == ['Firetext callback failed: reference missing'] assert json_resp['message'] == ['Firetext callback failed: reference missing']
def test_firetext_callback_should_return_200_if_send_sms_reference(notify_api, mocker): def test_firetext_callback_should_return_200_if_send_sms_reference(client, mocker):
with notify_api.test_request_context():
with notify_api.test_client() as client:
mocker.patch('app.statsd_client.incr') mocker.patch('app.statsd_client.incr')
response = client.post( response = client.post(
path='/notifications/sms/firetext', path='/notifications/sms/firetext',
@@ -70,9 +62,7 @@ def test_firetext_callback_should_return_200_if_send_sms_reference(notify_api, m
assert json_resp['message'] == 'Firetext callback succeeded: send-sms-code' assert json_resp['message'] == 'Firetext callback succeeded: send-sms-code'
def test_firetext_callback_should_return_400_if_no_status(notify_api, mocker): def test_firetext_callback_should_return_400_if_no_status(client, mocker):
with notify_api.test_request_context():
with notify_api.test_client() as client:
mocker.patch('app.statsd_client.incr') mocker.patch('app.statsd_client.incr')
response = client.post( response = client.post(
path='/notifications/sms/firetext', path='/notifications/sms/firetext',
@@ -85,9 +75,7 @@ def test_firetext_callback_should_return_400_if_no_status(notify_api, mocker):
assert json_resp['message'] == ['Firetext callback failed: status missing'] assert json_resp['message'] == ['Firetext callback failed: status missing']
def test_firetext_callback_should_return_400_if_unknown_status(notify_api, mocker): def test_firetext_callback_should_return_400_if_unknown_status(client, mocker):
with notify_api.test_request_context():
with notify_api.test_client() as client:
mocker.patch('app.statsd_client.incr') mocker.patch('app.statsd_client.incr')
response = client.post( response = client.post(
path='/notifications/sms/firetext', path='/notifications/sms/firetext',
@@ -100,9 +88,7 @@ def test_firetext_callback_should_return_400_if_unknown_status(notify_api, mocke
assert json_resp['message'] == 'Firetext callback failed: status 99 not found.' assert json_resp['message'] == 'Firetext callback failed: status 99 not found.'
def test_firetext_callback_should_return_400_if_invalid_guid_notification_id(notify_api, mocker): def test_firetext_callback_returns_200_when_notification_id_not_found_or_already_updated(client, mocker):
with notify_api.test_request_context():
with notify_api.test_client() as client:
mocker.patch('app.statsd_client.incr') mocker.patch('app.statsd_client.incr')
response = client.post( response = client.post(
path='/notifications/sms/firetext', path='/notifications/sms/firetext',
@@ -115,14 +101,12 @@ def test_firetext_callback_should_return_400_if_invalid_guid_notification_id(not
assert json_resp['message'] == 'Firetext callback with invalid reference 1234' assert json_resp['message'] == 'Firetext callback with invalid reference 1234'
def test_firetext_callback_should_return_404_if_cannot_find_notification_id( def test_callback_should_return_200_if_cannot_find_notification_id(
notify_db, notify_db,
notify_db_session, notify_db_session,
notify_api, client,
mocker mocker
): ):
with notify_api.test_request_context():
with notify_api.test_client() as client:
mocker.patch('app.statsd_client.incr') mocker.patch('app.statsd_client.incr')
missing_notification_id = uuid.uuid4() missing_notification_id = uuid.uuid4()
response = client.post( response = client.post(
@@ -133,20 +117,13 @@ def test_firetext_callback_should_return_404_if_cannot_find_notification_id(
headers=[('Content-Type', 'application/x-www-form-urlencoded')]) headers=[('Content-Type', 'application/x-www-form-urlencoded')])
json_resp = json.loads(response.get_data(as_text=True)) json_resp = json.loads(response.get_data(as_text=True))
assert response.status_code == 400 assert response.status_code == 200
assert json_resp['result'] == 'error' assert json_resp['result'] == 'success'
assert json_resp['message'] == 'Firetext callback failed: notification {} either not found ' \
'or already updated from sending. Status {}'.format(
missing_notification_id,
'Delivered'
)
def test_firetext_callback_should_update_notification_status( def test_firetext_callback_should_update_notification_status(
notify_db, notify_db_session, notify_api, sample_email_template, mocker notify_db, notify_db_session, client, sample_email_template, mocker
): ):
with notify_api.test_request_context():
with notify_api.test_client() as client:
mocker.patch('app.statsd_client.incr') mocker.patch('app.statsd_client.incr')
notification = create_sample_notification( notification = create_sample_notification(
@@ -179,10 +156,8 @@ def test_firetext_callback_should_update_notification_status(
def test_firetext_callback_should_update_notification_status_failed( def test_firetext_callback_should_update_notification_status_failed(
notify_db, notify_db_session, notify_api, sample_template, mocker notify_db, notify_db_session, client, sample_template, mocker
): ):
with notify_api.test_request_context():
with notify_api.test_client() as client:
mocker.patch('app.statsd_client.incr') mocker.patch('app.statsd_client.incr')
notification = create_sample_notification( notification = create_sample_notification(
@@ -212,9 +187,7 @@ def test_firetext_callback_should_update_notification_status_failed(
assert get_notification_by_id(notification.id).status == 'permanent-failure' assert get_notification_by_id(notification.id).status == 'permanent-failure'
def test_firetext_callback_should_update_notification_status_pending(notify_api, notify_db, notify_db_session, mocker): def test_firetext_callback_should_update_notification_status_pending(client, notify_db, notify_db_session, mocker):
with notify_api.test_request_context():
with notify_api.test_client() as client:
mocker.patch('app.statsd_client.incr') mocker.patch('app.statsd_client.incr')
notification = create_sample_notification( notification = create_sample_notification(
notify_db, notify_db_session, status='sending', sent_at=datetime.utcnow() notify_db, notify_db_session, status='sending', sent_at=datetime.utcnow()
@@ -239,13 +212,11 @@ def test_firetext_callback_should_update_notification_status_pending(notify_api,
def test_firetext_callback_should_update_multiple_notification_status_sent( def test_firetext_callback_should_update_multiple_notification_status_sent(
notify_api, client,
notify_db, notify_db,
notify_db_session, notify_db_session,
mocker mocker
): ):
with notify_api.test_request_context():
with notify_api.test_client() as client:
mocker.patch('app.statsd_client.incr') mocker.patch('app.statsd_client.incr')
notification1 = create_sample_notification( notification1 = create_sample_notification(
notify_db, notify_db_session, status='sending', sent_at=datetime.utcnow() notify_db, notify_db_session, status='sending', sent_at=datetime.utcnow()
@@ -279,12 +250,10 @@ def test_firetext_callback_should_update_multiple_notification_status_sent(
headers=[('Content-Type', 'application/x-www-form-urlencoded')]) headers=[('Content-Type', 'application/x-www-form-urlencoded')])
def test_process_mmg_response_return_200_when_cid_is_send_sms_code(notify_api): def test_process_mmg_response_return_200_when_cid_is_send_sms_code(client):
with notify_api.test_request_context():
data = '{"reference": "10100164", "CID": "send-sms-code", "MSISDN": "447775349060", "status": "3", \ data = '{"reference": "10100164", "CID": "send-sms-code", "MSISDN": "447775349060", "status": "3", \
"deliverytime": "2016-04-05 16:01:07"}' "deliverytime": "2016-04-05 16:01:07"}'
with notify_api.test_client() as client:
response = client.post(path='notifications/sms/mmg', response = client.post(path='notifications/sms/mmg',
data=data, data=data,
headers=[('Content-Type', 'application/json')]) headers=[('Content-Type', 'application/json')])
@@ -295,9 +264,8 @@ def test_process_mmg_response_return_200_when_cid_is_send_sms_code(notify_api):
def test_process_mmg_response_returns_200_when_cid_is_valid_notification_id( def test_process_mmg_response_returns_200_when_cid_is_valid_notification_id(
notify_db, notify_db_session, notify_api notify_db, notify_db_session, client
): ):
with notify_api.test_client() as client:
notification = create_sample_notification( notification = create_sample_notification(
notify_db, notify_db_session, status='sending', sent_at=datetime.utcnow() notify_db, notify_db_session, status='sending', sent_at=datetime.utcnow()
) )
@@ -318,9 +286,8 @@ def test_process_mmg_response_returns_200_when_cid_is_valid_notification_id(
def test_process_mmg_response_status_5_updates_notification_with_permanently_failed( def test_process_mmg_response_status_5_updates_notification_with_permanently_failed(
notify_db, notify_db_session, notify_api notify_db, notify_db_session, client
): ):
with notify_api.test_client() as client:
notification = create_sample_notification( notification = create_sample_notification(
notify_db, notify_db_session, status='sending', sent_at=datetime.utcnow() notify_db, notify_db_session, status='sending', sent_at=datetime.utcnow()
) )
@@ -341,9 +308,8 @@ def test_process_mmg_response_status_5_updates_notification_with_permanently_fai
def test_process_mmg_response_status_2_updates_notification_with_permanently_failed( def test_process_mmg_response_status_2_updates_notification_with_permanently_failed(
notify_db, notify_db_session, notify_api notify_db, notify_db_session, client
): ):
with notify_api.test_client() as client:
notification = create_sample_notification( notification = create_sample_notification(
notify_db, notify_db_session, status='sending', sent_at=datetime.utcnow() notify_db, notify_db_session, status='sending', sent_at=datetime.utcnow()
) )
@@ -363,9 +329,8 @@ def test_process_mmg_response_status_2_updates_notification_with_permanently_fai
def test_process_mmg_response_status_4_updates_notification_with_temporary_failed( def test_process_mmg_response_status_4_updates_notification_with_temporary_failed(
notify_db, notify_db_session, notify_api notify_db, notify_db_session, client
): ):
with notify_api.test_client() as client:
notification = create_sample_notification( notification = create_sample_notification(
notify_db, notify_db_session, status='sending', sent_at=datetime.utcnow() notify_db, notify_db_session, status='sending', sent_at=datetime.utcnow()
) )
@@ -386,10 +351,8 @@ def test_process_mmg_response_status_4_updates_notification_with_temporary_faile
def test_process_mmg_response_unknown_status_updates_notification_with_failed( def test_process_mmg_response_unknown_status_updates_notification_with_failed(
notify_db, notify_db_session, notify_api notify_db, notify_db_session, client
): ):
with notify_api.test_client() as client:
notification = create_sample_notification( notification = create_sample_notification(
notify_db, notify_db_session, status='sending', sent_at=datetime.utcnow() notify_db, notify_db_session, status='sending', sent_at=datetime.utcnow()
) )
@@ -408,8 +371,7 @@ def test_process_mmg_response_unknown_status_updates_notification_with_failed(
assert get_notification_by_id(notification.id).status == 'failed' assert get_notification_by_id(notification.id).status == 'failed'
def test_process_mmg_response_returns_400_for_malformed_data(notify_api): def test_process_mmg_response_returns_400_for_malformed_data(client):
with notify_api.test_client() as client:
data = json.dumps({"reference": "mmg_reference", data = json.dumps({"reference": "mmg_reference",
"monkey": 'random thing', "monkey": 'random thing',
"MSISDN": "447777349060", "MSISDN": "447777349060",
@@ -427,9 +389,17 @@ def test_process_mmg_response_returns_400_for_malformed_data(notify_api):
assert "{} callback failed: {} missing".format('MMG', 'CID') in json_data['message'] assert "{} callback failed: {} missing".format('MMG', 'CID') in json_data['message']
def test_ses_callback_should_not_need_auth(notify_api): def test_mmg_callback_returns_200_when_notification_id_not_found_or_already_updated(client):
with notify_api.test_request_context(): data = '{"reference": "10100164", "CID": "send-sms-code", "MSISDN": "447775349060", "status": "3", \
with notify_api.test_client() as client: "deliverytime": "2016-04-05 16:01:07"}'
response = client.post(path='notifications/sms/mmg',
data=data,
headers=[('Content-Type', 'application/json')])
assert response.status_code == 200
def test_ses_callback_should_not_need_auth(client):
response = client.post( response = client.post(
path='/notifications/email/ses', path='/notifications/email/ses',
data=ses_notification_callback(), data=ses_notification_callback(),
@@ -438,9 +408,7 @@ def test_ses_callback_should_not_need_auth(notify_api):
assert response.status_code == 404 assert response.status_code == 404
def test_ses_callback_should_fail_if_invalid_json(notify_api): def test_ses_callback_should_fail_if_invalid_json(client):
with notify_api.test_request_context():
with notify_api.test_client() as client:
response = client.post( response = client.post(
path='/notifications/email/ses', path='/notifications/email/ses',
data="nonsense", data="nonsense",
@@ -452,9 +420,7 @@ def test_ses_callback_should_fail_if_invalid_json(notify_api):
assert json_resp['message'] == 'SES callback failed: invalid json' assert json_resp['message'] == 'SES callback failed: invalid json'
def test_ses_callback_should_fail_if_invalid_notification_type(notify_api): def test_ses_callback_should_fail_if_invalid_notification_type(client):
with notify_api.test_request_context():
with notify_api.test_client() as client:
response = client.post( response = client.post(
path='/notifications/email/ses', path='/notifications/email/ses',
data=ses_invalid_notification_type_callback(), data=ses_invalid_notification_type_callback(),
@@ -466,9 +432,7 @@ def test_ses_callback_should_fail_if_invalid_notification_type(notify_api):
assert json_resp['message'] == 'SES callback failed: status Unknown not found' assert json_resp['message'] == 'SES callback failed: status Unknown not found'
def test_ses_callback_should_fail_if_missing_message_id(notify_api): def test_ses_callback_should_fail_if_missing_message_id(client):
with notify_api.test_request_context():
with notify_api.test_client() as client:
response = client.post( response = client.post(
path='/notifications/email/ses', path='/notifications/email/ses',
data=ses_missing_notification_id_callback(), data=ses_missing_notification_id_callback(),
@@ -480,9 +444,7 @@ def test_ses_callback_should_fail_if_missing_message_id(notify_api):
assert json_resp['message'] == 'SES callback failed: messageId missing' assert json_resp['message'] == 'SES callback failed: messageId missing'
def test_ses_callback_should_fail_if_notification_cannot_be_found(notify_db, notify_db_session, notify_api): def test_ses_callback_should_fail_if_notification_cannot_be_found(notify_db, notify_db_session, client):
with notify_api.test_request_context():
with notify_api.test_client() as client:
response = client.post( response = client.post(
path='/notifications/email/ses', path='/notifications/email/ses',
data=ses_invalid_notification_id_callback(), data=ses_invalid_notification_id_callback(),
@@ -495,13 +457,11 @@ def test_ses_callback_should_fail_if_notification_cannot_be_found(notify_db, not
def test_ses_callback_should_update_notification_status( def test_ses_callback_should_update_notification_status(
notify_api, client,
notify_db, notify_db,
notify_db_session, notify_db_session,
sample_email_template, sample_email_template,
mocker): mocker):
with notify_api.test_request_context():
with notify_api.test_client() as client:
with freeze_time('2001-01-01T12:00:00'): with freeze_time('2001-01-01T12:00:00'):
mocker.patch('app.statsd_client.incr') mocker.patch('app.statsd_client.incr')
mocker.patch('app.statsd_client.timing_with_dates') mocker.patch('app.statsd_client.timing_with_dates')
@@ -533,13 +493,11 @@ def test_ses_callback_should_update_notification_status(
def test_ses_callback_should_update_multiple_notification_status_sent( def test_ses_callback_should_update_multiple_notification_status_sent(
notify_api, client,
notify_db, notify_db,
notify_db_session, notify_db_session,
sample_email_template, sample_email_template,
mocker): mocker):
with notify_api.test_request_context():
with notify_api.test_client() as client:
notification1 = create_sample_notification( notification1 = create_sample_notification(
notify_db, notify_db,
notify_db_session, notify_db_session,
@@ -585,12 +543,10 @@ def test_ses_callback_should_update_multiple_notification_status_sent(
assert resp3.status_code == 200 assert resp3.status_code == 200
def test_ses_callback_should_set_status_to_temporary_failure(notify_api, def test_ses_callback_should_set_status_to_temporary_failure(client,
notify_db, notify_db,
notify_db_session, notify_db_session,
sample_email_template): sample_email_template):
with notify_api.test_request_context():
with notify_api.test_client() as client:
notification = create_sample_notification( notification = create_sample_notification(
notify_db, notify_db,
notify_db_session, notify_db_session,
@@ -613,12 +569,10 @@ def test_ses_callback_should_set_status_to_temporary_failure(notify_api,
assert get_notification_by_id(notification.id).status == 'temporary-failure' assert get_notification_by_id(notification.id).status == 'temporary-failure'
def test_ses_callback_should_not_set_status_once_status_is_delivered(notify_api, def test_ses_callback_should_not_set_status_once_status_is_delivered(client,
notify_db, notify_db,
notify_db_session, notify_db_session,
sample_email_template): sample_email_template):
with notify_api.test_request_context():
with notify_api.test_client() as client:
notification = create_sample_notification( notification = create_sample_notification(
notify_db, notify_db,
notify_db_session, notify_db_session,
@@ -642,12 +596,10 @@ def test_ses_callback_should_not_set_status_once_status_is_delivered(notify_api,
assert get_notification_by_id(notification.id).status == 'delivered' assert get_notification_by_id(notification.id).status == 'delivered'
def test_ses_callback_should_set_status_to_permanent_failure(notify_api, def test_ses_callback_should_set_status_to_permanent_failure(client,
notify_db, notify_db,
notify_db_session, notify_db_session,
sample_email_template): sample_email_template):
with notify_api.test_request_context():
with notify_api.test_client() as client:
notification = create_sample_notification( notification = create_sample_notification(
notify_db, notify_db,
notify_db_session, notify_db_session,
@@ -671,8 +623,7 @@ def test_ses_callback_should_set_status_to_permanent_failure(notify_api,
assert get_notification_by_id(notification.id).status == 'permanent-failure' assert get_notification_by_id(notification.id).status == 'permanent-failure'
def test_process_mmg_response_records_statsd(notify_db, notify_db_session, notify_api, mocker): def test_process_mmg_response_records_statsd(notify_db, notify_db_session, client, mocker):
with notify_api.test_client() as client:
with freeze_time('2001-01-01T12:00:00'): with freeze_time('2001-01-01T12:00:00'):
mocker.patch('app.statsd_client.incr') mocker.patch('app.statsd_client.incr')
@@ -697,9 +648,7 @@ def test_process_mmg_response_records_statsd(notify_db, notify_db_session, notif
) )
def test_firetext_callback_should_record_statsd(notify_api, notify_db, notify_db_session, mocker): def test_firetext_callback_should_record_statsd(client, notify_db, notify_db_session, mocker):
with notify_api.test_request_context():
with notify_api.test_client() as client:
with freeze_time('2001-01-01T12:00:00'): with freeze_time('2001-01-01T12:00:00'):
mocker.patch('app.statsd_client.incr') mocker.patch('app.statsd_client.incr')