Merge pull request #81 from alphagov/celery-send-sms-code

Use Celery to send sms code

app/celery/tasks.py

@@ -30,3 +30,13 @@ def send_sms(service_id, notification_id, encrypted_notification):
 
     except SQLAlchemyError as e:
         current_app.logger.debug(e)
+
+
+@notify_celery.task(name='send-sms-code')
+def send_sms_code(encrypted_verification):
+    verification_message = encryption.decrypt(encrypted_verification)
+
+    try:
+        firetext_client.send_sms(verification_message['to'], verification_message['secret_code'])
+    except FiretextClientException as e:
+        current_app.logger.debug(e)

app/user/rest.py

@@ -2,6 +2,8 @@ from datetime import datetime
 from flask import (jsonify, request, abort, Blueprint, current_app)
 from sqlalchemy.exc import DataError
 from sqlalchemy.orm.exc import NoResultFound
+
+from app import encryption
 from app.dao.services_dao import get_model_services
 from app.aws_sqs import add_notification_to_queue
 from app.dao.users_dao import (
@@ -18,7 +20,7 @@ from app.schemas import (
     user_schema, users_schema, service_schema, services_schema,
     request_verify_code_schema, user_schema_load_json)
 from app import api_user
-
+from app.celery.tasks import send_sms_code
 
 user = Blueprint('user', __name__)
 
@@ -138,16 +140,16 @@ def send_user_code(user_id):
     create_user_code(user, secret_code, verify_code.get('code_type'))
     if verify_code.get('code_type') == 'sms':
         mobile = user.mobile_number if verify_code.get('to', None) is None else verify_code.get('to')
-        notification = {'to': mobile, 'content': secret_code}
+        verification_message = {'to': mobile, 'secret_code': secret_code}
-        add_notification_to_queue(api_user['client'], 'admin', 'sms', notification)
+        send_sms_code.apply_async([encryption.encrypt(verification_message)], queue='sms_code')
     elif verify_code.get('code_type') == 'email':
         email = user.email_address if verify_code.get('to', None) is None else verify_code.get('to')
-        notification = {
+        verification_message = {
             'to_address': email,
             'from_address': current_app.config['VERIFY_CODE_FROM_EMAIL_ADDRESS'],
             'subject': 'Verification code',
             'body': secret_code}
-        add_notification_to_queue(api_user['client'], 'admin', 'email', notification)
+        add_notification_to_queue(api_user['client'], 'admin', 'email', verification_message)
     else:
         abort(500)
     return jsonify({}), 204

config.py

@@ -25,7 +25,7 @@ class Config(object):
         'region': 'eu-west-1',
         'polling_interval': 1,  # 1 second
         'visibility_timeout': 60,  # 60 seconds
-        'queue_name_prefix': os.environ['NOTIFICATION_QUEUE_PREFIX']
+        'queue_name_prefix': os.environ['NOTIFICATION_QUEUE_PREFIX']+'-'
     }
     CELERY_ENABLE_UTC = True,
     CELERY_TIMEZONE = 'Europe/London'

scripts/run_celery.sh

@@ -3,4 +3,4 @@
 set -e
 
 source environment.sh
-celery -A run_celery.notify_celery worker --loglevel=INFO --logfile=/var/log/notify/application.log --concurrency=4 -Q sms
+celery -A run_celery.notify_celery worker --loglevel=INFO --logfile=/var/log/notify/application.log --concurrency=4 -Q sms, sms-code

tests/app/celery/test_tasks.py

@@ -1,5 +1,8 @@
 import uuid
 import pytest
+from app.celery.tasks import (send_sms, send_sms_code)
+from app import twilio_client, encryption
+from app.clients.sms.twilio import TwilioClientException
 from app.celery.tasks import send_sms
 from app import firetext_client
 from app.clients.sms.firetext import FiretextClientException
@@ -74,3 +77,24 @@ def test_should_not_send_sms_if_db_peristance_failed(sample_template, mocker):
     with pytest.raises(NoResultFound) as e:
         notifications_dao.get_notification(sample_template.service_id, notification_id)
     assert 'No row was found for one' in str(e.value)
+
+
+def test_should_send_sms_code(mocker):
+    notification = {'to': '+441234123123',
+                    'secret_code': '12345'}
+
+    encrypted_notification = encryption.encrypt(notification)
+
+    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'])
+
+
+def test_should_throw_firetext_client_exception(mocker):
+    notification = {'to': '+441234123123',
+                    'secret_code': '12345'}
+
+    encrypted_notification = encryption.encrypt(notification)
+    mocker.patch('app.firetext_client.send_sms', side_effect=FiretextClientException)
+    send_sms_code(encrypted_notification)
+    firetext_client.send_sms.assert_called_once_with(notification['to'], notification['secret_code'])

tests/app/conftest.py

@@ -202,3 +202,8 @@ def sample_notification(notify_db,
     notification = Notification(**data)
     save_notification(notification)
     return notification
+
+
+@pytest.fixture(scope='function')
+def mock_celery_send_sms_code(mocker):
+    return mocker.patch('app.celery.tasks.send_sms_code.apply_async')

tests/app/user/test_rest_verify.py

@@ -4,7 +4,9 @@ from datetime import (datetime, timedelta)
 from flask import url_for
 
 from app.models import (VerifyCode)
-from app import db
+
+import app.celery.tasks
+from app import db, encryption
 from tests import create_authorization_header
 
 
@@ -247,13 +249,10 @@ def test_user_verify_password_missing_password(notify_api,
             assert 'Required field missing data' in json_resp['message']['password']
 
 
-@moto.mock_sqs
 def test_send_user_code_for_sms(notify_api,
-                                notify_db,
-                                notify_db_session,
                                 sample_sms_code,
-                                sqs_client_conn,
+                                mock_secret_code,
-                                mock_secret_code):
+                                mock_celery_send_sms_code):
     """
    Tests POST endpoint '/<user_id>/code' successful sms
    """
@@ -270,20 +269,20 @@ def test_send_user_code_for_sms(notify_api,
                 headers=[('Content-Type', 'application/json'), auth_header])
 
             assert resp.status_code == 204
+            encrpyted = encryption.encrypt({'to': sample_sms_code.user.mobile_number, 'secret_code': '11111'})
+            app.celery.tasks.send_sms_code.apply_async.assert_called_once_with([encrpyted], queue='sms_code')
 
 
-@moto.mock_sqs
 def test_send_user_code_for_sms_with_optional_to_field(notify_api,
-                                                       notify_db,
-                                                       notify_db_session,
                                                        sample_sms_code,
-                                                       sqs_client_conn,
+                                                       mock_secret_code,
-                                                       mock_secret_code):
+                                                       mock_celery_send_sms_code):
     """
    Tests POST endpoint '/<user_id>/code' successful sms with optional to field
    """
     with notify_api.test_request_context():
         with notify_api.test_client() as client:
+
             data = json.dumps({'code_type': 'sms', 'to': '+441119876757'})
             auth_header = create_authorization_header(
                 path=url_for('user.send_user_code', user_id=sample_sms_code.user.id),
@@ -295,6 +294,8 @@ def test_send_user_code_for_sms_with_optional_to_field(notify_api,
                 headers=[('Content-Type', 'application/json'), auth_header])
 
             assert resp.status_code == 204
+            encrpyted = encryption.encrypt({'to': '+441119876757', 'secret_code': '11111'})
+            app.celery.tasks.send_sms_code.apply_async.assert_called_once_with([encrpyted], queue='sms_code')
 
 
 @moto.mock_sqs