diff --git a/app/__init__.py b/app/__init__.py index 17c81d24d..3c06ac77c 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -71,7 +71,7 @@ def create_app(): def init_app(app): @app.before_request def required_authentication(): - if request.path != url_for('status.show_status'): + if request.path not in [url_for('status.show_status'), url_for('notifications.process_firetext_response')]: from app.authentication import auth error = auth.requires_auth() if error: diff --git a/app/celery/tasks.py b/app/celery/tasks.py index 8f609ed3f..9a29ec3d2 100644 --- a/app/celery/tasks.py +++ b/app/celery/tasks.py @@ -208,6 +208,9 @@ def send_sms(service_id, notification_id, encrypted_notification, created_at): notification_id=notification_id ) except FiretextClientException as e: + current_app.logger.error( + "SMS notification {} failed".format(notification_id) + ) current_app.logger.exception(e) notification_db_object.status = 'failed' dao_update_notification(notification_db_object) diff --git a/app/clients/sms/firetext.py b/app/clients/sms/firetext.py index d10f0f875..9b3793e2c 100644 --- a/app/clients/sms/firetext.py +++ b/app/clients/sms/firetext.py @@ -9,6 +9,24 @@ from requests import request, RequestException, HTTPError logger = logging.getLogger(__name__) +firetext_response_status = { + '0': { + "firetext_message": 'delivered', + "success": True, + "notify_status": 'delivered' + }, + '1': { + "firetext_message": 'declined', + "success": False, + "notify_status": 'failed' + }, + '2': { + "firetext_message": 'Undelivered (Pending with Network)', + "success": False, + "notify_status": 'sent' + } +} + class FiretextClientException(SmsClientException): def __init__(self, response): diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index 95c3fb54d..0c5109233 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -86,6 +86,10 @@ def get_notification(service_id, notification_id): return Notification.query.filter_by(service_id=service_id, id=notification_id).one() +def get_notification_by_id(notification_id): + return Notification.query.filter_by(id=notification_id).first() + + def get_notifications_for_service(service_id, page=1): query = Notification.query.filter_by(service_id=service_id).order_by(desc(Notification.created_at)).paginate( page=page, diff --git a/app/models.py b/app/models.py index d4784d489..f5dc5fb86 100644 --- a/app/models.py +++ b/app/models.py @@ -231,7 +231,7 @@ class VerifyCode(db.Model): return check_hash(cde, self._code) -NOTIFICATION_STATUS_TYPES = ['sent', 'failed'] +NOTIFICATION_STATUS_TYPES = ['sent', 'delivered', 'failed'] class Notification(db.Model): diff --git a/app/notifications/rest.py b/app/notifications/rest.py index 1fc35fe8e..ffe9f5085 100644 --- a/app/notifications/rest.py +++ b/app/notifications/rest.py @@ -1,4 +1,5 @@ from datetime import datetime +import uuid from flask import ( Blueprint, @@ -9,7 +10,7 @@ from flask import ( ) from utils.template import Template - +from app.clients.sms.firetext import firetext_response_status from app import api_user, encryption, create_uuid, DATETIME_FORMAT, DATE_FORMAT from app.authentication.auth import require_admin from app.dao import ( @@ -32,6 +33,67 @@ from app.errors import register_errors register_errors(notifications) +@notifications.route('/notifications/sms/firetext', methods=['POST']) +def process_firetext_response(): + if 'status' not in request.form: + current_app.logger.info( + "Firetext callback failed: status missing" + ) + return jsonify(result="error", message="Firetext callback failed: status missing"), 400 + + if len(request.form.get('reference', '')) <= 0: + current_app.logger.info( + "Firetext callback with no reference" + ) + return jsonify(result="success", message="Firetext callback succeeded"), 200 + + notification_id = request.form['reference'] + status = request.form['status'] + + try: + uuid.UUID(notification_id, version=4) + except ValueError: + current_app.logger.info( + "Firetext callback with invalid reference {}".format(notification_id) + ) + return jsonify( + result="error", message="Firetext callback with invalid reference {}".format(notification_id) + ), 400 + + notification_status = firetext_response_status.get(status, None) + if not notification_status: + current_app.logger.info( + "Firetext callback failed: status {} not found.".format(status) + ) + return jsonify(result="error", message="Firetext callback failed: status {} not found.".format(status)), 400 + + notification = notifications_dao.get_notification_by_id(notification_id) + if not notification: + current_app.logger.info( + "Firetext callback failed: notification {} not found. Status {}".format(notification_id, status) + ) + return jsonify( + result="error", + message="Firetext callback failed: notification {} not found. Status {}".format( + notification_id, + notification_status['firetext_message'] + ) + ), 404 + + if not notification_status['success']: + current_app.logger.info( + "Firetext delivery failed: notification {} has error found. Status {}".format( + notification_id, + 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) + ), 200 + + @notifications.route('/notifications/', methods=['GET']) def get_notifications(notification_id): try: diff --git a/migrations/versions/0039_more_notification_states.py b/migrations/versions/0039_more_notification_states.py new file mode 100644 index 000000000..495c1ea21 --- /dev/null +++ b/migrations/versions/0039_more_notification_states.py @@ -0,0 +1,31 @@ +"""empty message + +Revision ID: 0039_more_notification_states +Revises: 0038_reduce_limits +Create Date: 2016-03-08 11:16:25.659463 + +""" + +# revision identifiers, used by Alembic. +revision = '0039_more_notification_states' +down_revision = '0038_reduce_limits' + +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + + +def upgrade(): + op.drop_column('notifications', 'status') + op.execute('DROP TYPE notification_status_types') + notification_status_types = sa.Enum('sent', 'delivered', 'failed', name='notification_status_types') + notification_status_types.create(op.get_bind()) + op.add_column('notifications', sa.Column('status', notification_status_types, nullable=True)) + op.get_bind() + op.execute("update notifications set status='delivered'") + op.alter_column('notifications', 'status', nullable=False) + + +def downgrade(): + op.drop_column('notifications', 'status') + op.execute('DROP TYPE notification_status_types') diff --git a/tests/app/celery/test_tasks.py b/tests/app/celery/test_tasks.py index 65abc977e..a185eadf2 100644 --- a/tests/app/celery/test_tasks.py +++ b/tests/app/celery/test_tasks.py @@ -35,6 +35,10 @@ from tests.app.conftest import ( ) +def firetext_error(): + return {'code': 0, 'description': 'error'} + + def test_should_call_delete_successful_notifications_in_task(notify_api, mocker): mocker.patch('app.celery.tasks.delete_successful_notifications_created_more_than_a_day_ago') delete_successful_notifications() @@ -494,7 +498,7 @@ def test_should_persist_notification_as_failed_if_sms_client_fails(sample_templa "to": "+441234123123" } mocker.patch('app.encryption.decrypt', return_value=notification) - mocker.patch('app.firetext_client.send_sms', side_effect=FiretextClientException()) + mocker.patch('app.firetext_client.send_sms', side_effect=FiretextClientException(firetext_error())) mocker.patch('app.firetext_client.get_name', return_value="firetext") now = datetime.utcnow() @@ -623,7 +627,7 @@ def test_should_throw_firetext_client_exception(mocker): 'secret_code': '12345'} encrypted_notification = encryption.encrypt(notification) - mocker.patch('app.firetext_client.send_sms', side_effect=FiretextClientException) + 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']) diff --git a/tests/app/notifications/test_rest.py b/tests/app/notifications/test_rest.py index cdc7c1957..88916ab89 100644 --- a/tests/app/notifications/test_rest.py +++ b/tests/app/notifications/test_rest.py @@ -7,6 +7,7 @@ from flask import json from app.models import Service from app.dao.templates_dao import dao_get_all_templates_for_service from app.dao.services_dao import dao_update_service +from app.dao.notifications_dao import get_notification_by_id from freezegun import freeze_time @@ -858,3 +859,174 @@ def test_should_allow_api_call_if_under_day_limit_regardless_of_type(notify_db, headers=[('Content-Type', 'application/json'), auth_header]) assert response.status_code == 201 + + +def test_firetext_callback_should_not_need_auth(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&reference=&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): + 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&reference=&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 == 200 + assert json_resp['result'] == 'success' + assert json_resp['message'] == 'Firetext callback succeeded' + + +def test_firetext_callback_should_return_200_if_no_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', + 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' + + +def test_firetext_callback_should_return_400_if_no_status(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&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: status missing' + + +def test_firetext_callback_should_return_400_if_unknown_status(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=99&time=2016-03-10 14:17:00&reference={}'.format(uuid.uuid4()), + 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: status 99 not found.' + + +def test_firetext_callback_should_return_400_if_invalid_guid_notification_id(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=1234', + 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 with invalid reference 1234' + + +def test_firetext_callback_should_return_404_if_cannot_find_notification_id(notify_db, notify_db_session, notify_api): + with notify_api.test_request_context(): + with notify_api.test_client() as client: + missing_notification_id = uuid.uuid4() + response = client.post( + path='/notifications/sms/firetext', + data='mobile=441234123123&status=0&time=2016-03-10 14:17:00&reference={}'.format( + missing_notification_id + ), + headers=[('Content-Type', 'application/x-www-form-urlencoded')]) + + json_resp = json.loads(response.get_data(as_text=True)) + assert response.status_code == 404 + assert json_resp['result'] == 'error' + assert json_resp['message'] == 'Firetext callback failed: notification {} not found. Status {}'.format( + missing_notification_id, + 'delivered' + ) + + +def test_firetext_callback_should_update_notification_status(notify_api, sample_notification): + with notify_api.test_request_context(): + with notify_api.test_client() as client: + original = get_notification_by_id(sample_notification.id) + assert original.status == 'sent' + + response = client.post( + path='/notifications/sms/firetext', + data='mobile=441234123123&status=0&time=2016-03-10 14:17:00&reference={}'.format( + sample_notification.id + ), + 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. reference {} updated'.format( + sample_notification.id + ) + updated = get_notification_by_id(sample_notification.id) + assert updated.status == 'delivered' + + +def test_firetext_callback_should_update_notification_status_failed(notify_api, sample_notification): + with notify_api.test_request_context(): + with notify_api.test_client() as client: + original = get_notification_by_id(sample_notification.id) + assert original.status == 'sent' + + response = client.post( + path='/notifications/sms/firetext', + data='mobile=441234123123&status=1&time=2016-03-10 14:17:00&reference={}'.format( + sample_notification.id + ), + 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. reference {} updated'.format( + sample_notification.id + ) + updated = get_notification_by_id(sample_notification.id) + assert updated.status == 'failed' + + +def test_firetext_callback_should_update_notification_status_sent(notify_api, notify_db, notify_db_session): + with notify_api.test_request_context(): + with notify_api.test_client() as client: + notification = sample_notification(notify_db, notify_db_session, status='delivered') + original = get_notification_by_id(notification.id) + assert original.status == 'delivered' + + response = client.post( + path='/notifications/sms/firetext', + data='mobile=441234123123&status=2&time=2016-03-10 14:17:00&reference={}'.format( + notification.id + ), + 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. reference {} updated'.format( + notification.id + ) + updated = get_notification_by_id(notification.id) + assert updated.status == 'sent'