diff --git a/app/celery/tasks.py b/app/celery/tasks.py index 9a29ec3d2..1808cd7f0 100644 --- a/app/celery/tasks.py +++ b/app/celery/tasks.py @@ -295,7 +295,9 @@ def send_email(service_id, notification_id, subject, from_address, encrypted_not def send_sms_code(encrypted_verification): verification_message = encryption.decrypt(encrypted_verification) try: - firetext_client.send_sms(verification_message['to'], verification_message['secret_code']) + firetext_client.send_sms( + verification_message['to'], verification_message['secret_code'], 'send-sms-code' + ) except FiretextClientException as e: current_app.logger.exception(e) diff --git a/app/clients/sms/firetext.py b/app/clients/sms/firetext.py index 9b3793e2c..5b41c2811 100644 --- a/app/clients/sms/firetext.py +++ b/app/clients/sms/firetext.py @@ -51,20 +51,16 @@ class FiretextClient(SmsClient): def get_name(self): return self.name - def send_sms(self, to, content, notification_id=None): + def send_sms(self, to, content, reference): data = { "apiKey": self.api_key, "from": self.from_number, "to": to.replace('+', ''), - "message": content + "message": content, + "reference": reference } - if notification_id: - data.update({ - "reference": notification_id - }) - start_time = monotonic() try: response = request( diff --git a/app/notifications/rest.py b/app/notifications/rest.py index ffe9f5085..67f7a35bc 100644 --- a/app/notifications/rest.py +++ b/app/notifications/rest.py @@ -45,19 +45,22 @@ def process_firetext_response(): current_app.logger.info( "Firetext callback with no reference" ) - return jsonify(result="success", message="Firetext callback succeeded"), 200 + return jsonify(result="error", message="Firetext callback failed: reference missing"), 400 - notification_id = request.form['reference'] + reference = request.form['reference'] status = request.form['status'] + if reference == 'send-sms-code': + return jsonify(result="success", message="Firetext callback succeeded: send-sms-code"), 200 + try: - uuid.UUID(notification_id, version=4) + uuid.UUID(reference, version=4) except ValueError: current_app.logger.info( - "Firetext callback with invalid reference {}".format(notification_id) + "Firetext callback with invalid reference {}".format(reference) ) return jsonify( - result="error", message="Firetext callback with invalid reference {}".format(notification_id) + result="error", message="Firetext callback with invalid reference {}".format(reference) ), 400 notification_status = firetext_response_status.get(status, None) @@ -67,15 +70,15 @@ def process_firetext_response(): ) return jsonify(result="error", message="Firetext callback failed: status {} not found.".format(status)), 400 - notification = notifications_dao.get_notification_by_id(notification_id) + notification = notifications_dao.get_notification_by_id(reference) if not notification: current_app.logger.info( - "Firetext callback failed: notification {} not found. Status {}".format(notification_id, status) + "Firetext callback failed: notification {} not found. Status {}".format(reference, status) ) return jsonify( result="error", message="Firetext callback failed: notification {} not found. Status {}".format( - notification_id, + reference, notification_status['firetext_message'] ) ), 404 @@ -83,14 +86,14 @@ def process_firetext_response(): if not notification_status['success']: current_app.logger.info( "Firetext delivery failed: notification {} has error found. Status {}".format( - notification_id, + reference, firetext_response_status[status]['firetext_message'] ) ) notification.status = notification_status['notify_status'] notifications_dao.dao_update_notification(notification) return jsonify( - result="success", message="Firetext callback succeeded. reference {} updated".format(notification_id) + result="success", message="Firetext callback succeeded. reference {} updated".format(reference) ), 200 diff --git a/tests/app/celery/test_tasks.py b/tests/app/celery/test_tasks.py index a185eadf2..f596eb034 100644 --- a/tests/app/celery/test_tasks.py +++ b/tests/app/celery/test_tasks.py @@ -619,7 +619,7 @@ def test_should_send_sms_code(mocker): mocker.patch('app.firetext_client.send_sms') send_sms_code(encrypted_notification) - firetext_client.send_sms.assert_called_once_with(notification['to'], notification['secret_code']) + firetext_client.send_sms.assert_called_once_with(notification['to'], notification['secret_code'], 'send-sms-code') def test_should_throw_firetext_client_exception(mocker): @@ -629,7 +629,7 @@ def test_should_throw_firetext_client_exception(mocker): encrypted_notification = encryption.encrypt(notification) mocker.patch('app.firetext_client.send_sms', side_effect=FiretextClientException(firetext_error())) send_sms_code(encrypted_notification) - firetext_client.send_sms.assert_called_once_with(notification['to'], notification['secret_code']) + firetext_client.send_sms.assert_called_once_with(notification['to'], notification['secret_code'], 'send-sms-code') def test_should_send_email_code(mocker): diff --git a/tests/app/notifications/test_rest.py b/tests/app/notifications/test_rest.py index 88916ab89..91940cf04 100644 --- a/tests/app/notifications/test_rest.py +++ b/tests/app/notifications/test_rest.py @@ -866,13 +866,13 @@ def test_firetext_callback_should_not_need_auth(notify_api): with notify_api.test_client() as client: response = client.post( path='/notifications/sms/firetext', - data='mobile=441234123123&status=0&reference=&time=2016-03-10 14:17:00', + data='mobile=441234123123&status=0&reference=send-sms-code&time=2016-03-10 14:17:00', headers=[('Content-Type', 'application/x-www-form-urlencoded')]) assert response.status_code == 200 -def test_firetext_callback_should_return_200_if_empty_reference(notify_api): +def test_firetext_callback_should_return_400_if_empty_reference(notify_api): with notify_api.test_request_context(): with notify_api.test_client() as client: response = client.post( @@ -881,12 +881,12 @@ def test_firetext_callback_should_return_200_if_empty_reference(notify_api): headers=[('Content-Type', 'application/x-www-form-urlencoded')]) json_resp = json.loads(response.get_data(as_text=True)) - assert response.status_code == 200 - assert json_resp['result'] == 'success' - assert json_resp['message'] == 'Firetext callback succeeded' + assert response.status_code == 400 + assert json_resp['result'] == 'error' + assert json_resp['message'] == 'Firetext callback failed: reference missing' -def test_firetext_callback_should_return_200_if_no_reference(notify_api): +def test_firetext_callback_should_return_400_if_no_reference(notify_api): with notify_api.test_request_context(): with notify_api.test_client() as client: response = client.post( @@ -894,10 +894,24 @@ def test_firetext_callback_should_return_200_if_no_reference(notify_api): data='mobile=441234123123&status=0&time=2016-03-10 14:17:00', headers=[('Content-Type', 'application/x-www-form-urlencoded')]) + json_resp = json.loads(response.get_data(as_text=True)) + assert response.status_code == 400 + assert json_resp['result'] == 'error' + assert json_resp['message'] == 'Firetext callback failed: reference missing' + + +def test_firetext_callback_should_return_200_if_send_sms_reference(notify_api): + with notify_api.test_request_context(): + with notify_api.test_client() as client: + response = client.post( + path='/notifications/sms/firetext', + data='mobile=441234123123&status=0&time=2016-03-10 14:17:00&reference=send-sms-code', + headers=[('Content-Type', 'application/x-www-form-urlencoded')]) + json_resp = json.loads(response.get_data(as_text=True)) assert response.status_code == 200 assert json_resp['result'] == 'success' - assert json_resp['message'] == 'Firetext callback succeeded' + assert json_resp['message'] == 'Firetext callback succeeded: send-sms-code' def test_firetext_callback_should_return_400_if_no_status(notify_api):