diff --git a/app/notifications/rest.py b/app/notifications/rest.py index bddd13532..3a74d9341 100644 --- a/app/notifications/rest.py +++ b/app/notifications/rest.py @@ -23,33 +23,17 @@ def get_notifications(notification_id): return jsonify(notify_alpha_client.fetch_notification_by_id(notification_id)), 200 -def _add_notification_to_queue(template_id, service, msg_type, to): - q = boto3.resource('sqs', region_name=current_app.config['AWS_REGION']).create_queue( - QueueName=str(service.queue_name)) - import uuid - notification = json.dumps({'message_id': str(uuid.uuid4()), - 'service_id': service.id, - 'to': to, - 'message_type': msg_type, - 'template_id': template_id}) - serializer = URLSafeSerializer(current_app.config.get('SECRET_KEY')) - encrypted = serializer.dumps(notification, current_app.config.get('DANGEROUS_SALT')) - q.send_message(MessageBody=encrypted, - MessageAttributes={'type': {'StringValue': msg_type, 'DataType': 'String'}, - 'service_id': {'StringValue': str(service.id), 'DataType': 'String'}, - 'template_id': {'StringValue': str(template_id), 'DataType': 'String'}}) - - @notifications.route('/sms', methods=['POST']) def create_sms_notification(): notification = request.get_json()['notification'] errors = {} + to, to_errors = validate_to(notification) + if to_errors['to']: + errors.update(to_errors) + # TODO: should create a different endpoint for the admin client to send verify codes. if api_user['client'] == current_app.config.get('ADMIN_CLIENT_USER_NAME'): - to, to_errors = validate_to_for_admin_client(notification) content, content_errors = validate_content_for_admin_client(notification) - if to_errors['to']: - errors.update(to_errors) if content_errors['content']: errors.update(content_errors) if errors: @@ -58,16 +42,17 @@ def create_sms_notification(): return jsonify(notify_alpha_client.send_sms(mobile_number=to, message=content)), 200 else: - to, to_errors = validate_to(notification, api_user['client']) + to, restricted_errors = validate_to_for_service(notification, api_user['client']) + if restricted_errors['restricted']: + errors.update(restricted_errors) + template, template_errors = validate_template(notification, api_user['client']) - if to_errors['to']: - errors.update(to_errors) if template_errors['template']: errors.update(template_errors) if errors: return jsonify(result="error", message=errors), 400 - # add notification to the queue + # add notification to the queue service = services_dao.get_model_services(api_user['client'], _raise=False) _add_notification_to_queue(template.id, service, 'sms', to) return jsonify(notify_alpha_client.send_sms(mobile_number=to, message=template.content)), 200 @@ -92,28 +77,21 @@ def create_email_notification(): notification['subject'])) -def validate_to(json_body, service_id): - errors = {"to": []} - mob = json_body.get('to', None) - if not mob: - errors['to'].append('Required data missing') - else: - if not mobile_regex.match(mob): - errors['to'].append('invalid phone number, must be of format +441234123123') - - service = services_dao.get_model_services(service_id=service_id) - if service.restricted: - valid = False - for usr in service.users: - if mob == usr.mobile_number: - valid = True - break - if not valid: - errors['to'].append('Invalid phone number for restricted service') +def validate_to_for_service(mob, service_id): + errors = {"restricted": []} + service = services_dao.get_model_services(service_id=service_id) + if service.restricted: + valid = False + for usr in service.users: + if mob == usr.mobile_number: + valid = True + break + if not valid: + errors['restricted'].append('Invalid phone number for restricted service') return mob, errors -def validate_to_for_admin_client(json_body): +def validate_to(json_body): errors = {"to": []} mob = json_body.get('to', None) if not mob: @@ -154,3 +132,20 @@ def validate_required_and_something(json_body, field): if field not in json_body and json_body[field]: errors.append('Required data for field.') return {field: errors} if errors else None + + +def _add_notification_to_queue(template_id, service, msg_type, to): + q = boto3.resource('sqs', region_name=current_app.config['AWS_REGION']).create_queue( + QueueName=str(service.queue_name)) + import uuid + notification = json.dumps({'message_id': str(uuid.uuid4()), + 'service_id': service.id, + 'to': to, + 'message_type': msg_type, + 'template_id': template_id}) + serializer = URLSafeSerializer(current_app.config.get('SECRET_KEY')) + encrypted = serializer.dumps(notification, current_app.config.get('DANGEROUS_SALT')) + q.send_message(MessageBody=encrypted, + MessageAttributes={'type': {'StringValue': msg_type, 'DataType': 'String'}, + 'service_id': {'StringValue': str(service.id), 'DataType': 'String'}, + 'template_id': {'StringValue': str(template_id), 'DataType': 'String'}}) diff --git a/tests/app/notifications/test_rest.py b/tests/app/notifications/test_rest.py index c8c13d5db..e940a8e6e 100644 --- a/tests/app/notifications/test_rest.py +++ b/tests/app/notifications/test_rest.py @@ -8,7 +8,7 @@ from app.models import Service def test_get_notifications( - notify_api, notify_db, notify_db_session, sample_service, sample_admin_service_id, mocker): + notify_api, notify_db, notify_db_session, sample_api_key, mocker): """ Tests GET endpoint '/' to retrieve entire service list. """ @@ -27,7 +27,7 @@ def test_get_notifications( ) auth_header = create_authorization_header( - service_id=sample_admin_service_id, + service_id=sample_api_key.service_id, path=url_for('notifications.get_notifications', notification_id=123), method='GET') @@ -44,7 +44,7 @@ def test_get_notifications( def test_get_notifications_empty_result( - notify_api, notify_db, notify_db_session, sample_service, sample_admin_service_id, mocker): + notify_api, notify_db, notify_db_session, sample_api_key, mocker): """ Tests GET endpoint '/' to retrieve entire service list. """ @@ -59,7 +59,7 @@ def test_get_notifications_empty_result( ) auth_header = create_authorization_header( - service_id=sample_admin_service_id, + service_id=sample_api_key.service_id, path=url_for('notifications.get_notifications', notification_id=123), method='GET') @@ -73,15 +73,13 @@ def test_get_notifications_empty_result( notify_alpha_client.fetch_notification_by_id.assert_called_with("123") -@moto.mock_sqs -def test_should_reject_if_no_phone_numbers( - notify_api, notify_db, notify_db_session, sample_service, sample_admin_service_id, mocker): +def test_create_sms_should_reject_if_no_phone_numbers( + notify_api, notify_db, notify_db_session, sample_api_key, mocker): """ Tests GET endpoint '/' to retrieve entire service list. """ with notify_api.test_request_context(): with notify_api.test_client() as client: - set_up_mock_queue() mocker.patch( 'app.notify_alpha_client.send_sms', return_value='success' @@ -92,7 +90,7 @@ def test_should_reject_if_no_phone_numbers( } } auth_header = create_authorization_header( - service_id=sample_admin_service_id, + service_id=sample_api_key.service_id, request_body=json.dumps(data), path=url_for('notifications.create_sms_notification'), method='POST') @@ -109,15 +107,13 @@ def test_should_reject_if_no_phone_numbers( assert not notify_alpha_client.send_sms.called -@moto.mock_sqs def test_should_reject_bad_phone_numbers( - notify_api, notify_db, notify_db_session, sample_service, sample_admin_service_id, mocker): + notify_api, notify_db, notify_db_session, mocker): """ Tests GET endpoint '/' to retrieve entire service list. """ with notify_api.test_request_context(): with notify_api.test_client() as client: - set_up_mock_queue() mocker.patch( 'app.notify_alpha_client.send_sms', return_value='success' @@ -129,7 +125,6 @@ def test_should_reject_bad_phone_numbers( } } auth_header = create_authorization_header( - service_id=sample_admin_service_id, request_body=json.dumps(data), path=url_for('notifications.create_sms_notification'), method='POST') @@ -146,15 +141,13 @@ def test_should_reject_bad_phone_numbers( assert not notify_alpha_client.send_sms.called -@moto.mock_sqs -def test_should_reject_missing_template( - notify_api, notify_db, notify_db_session, sample_service, sample_admin_service_id, mocker): +def test_should_reject_missing_content( + notify_api, notify_db, notify_db_session, mocker): """ Tests GET endpoint '/' to retrieve entire service list. """ with notify_api.test_request_context(): with notify_api.test_client() as client: - set_up_mock_queue() mocker.patch( 'app.notify_alpha_client.send_sms', return_value='success' @@ -165,7 +158,6 @@ def test_should_reject_missing_template( } } auth_header = create_authorization_header( - service_id=sample_admin_service_id, request_body=json.dumps(data), path=url_for('notifications.create_sms_notification'), method='POST') @@ -178,7 +170,7 @@ def test_should_reject_missing_template( json_resp = json.loads(response.get_data(as_text=True)) assert response.status_code == 400 assert json_resp['result'] == 'error' - assert 'Required data missing' in json_resp['message']['template'] + assert 'Required content' in json_resp['message']['content'] assert not notify_alpha_client.send_sms.called @@ -186,9 +178,6 @@ def test_should_reject_missing_template( def test_send_template_content(notify_api, notify_db, notify_db_session, - sample_api_key, - sample_template, - sample_user, mocker): """ Test POST endpoint '/sms' with service notification. @@ -196,6 +185,8 @@ def test_send_template_content(notify_api, with notify_api.test_request_context(): with notify_api.test_client() as client: set_up_mock_queue() + mobile = '+447719087678' + msg = 'Message content' mocker.patch( 'app.notify_alpha_client.send_sms', return_value={ @@ -203,21 +194,20 @@ def test_send_template_content(notify_api, "createdAt": "2015-11-03T09:37:27.414363Z", "id": 100, "jobId": 65, - "message": sample_template.content, + "message": msg, "method": "sms", "status": "created", - "to": sample_user.mobile_number + "to": mobile } } ) data = { 'notification': { - 'to': sample_user.mobile_number, - 'template': sample_template.id + 'to': mobile, + 'template': msg } } auth_header = create_authorization_header( - service_id=sample_template.service.id, request_body=json.dumps(data), path=url_for('notifications.create_sms_notification'), method='POST') @@ -231,11 +221,10 @@ def test_send_template_content(notify_api, assert response.status_code == 200 assert json_resp['notification']['id'] == 100 notify_alpha_client.send_sms.assert_called_with( - mobile_number=sample_user.mobile_number, - message=sample_template.content) + mobile_number=mobile, + message=msg) -@moto.mock_sqs def test_send_notification_restrict_mobile(notify_api, notify_db, notify_db_session, @@ -249,7 +238,7 @@ def test_send_notification_restrict_mobile(notify_api, """ with notify_api.test_request_context(): with notify_api.test_client() as client: - set_up_mock_queue() + Service.query.filter_by( id=sample_template.service.id).update({'restricted': True}) invalid_mob = '+449999999999' @@ -277,12 +266,12 @@ def test_send_notification_restrict_mobile(notify_api, json_resp = json.loads(response.get_data(as_text=True)) assert response.status_code == 400 - assert 'Invalid phone number for restricted service' in json_resp['message']['to'] + assert 'Invalid phone number for restricted service' in json_resp['message']['restricted'] + assert not notify_alpha_client.send_sms.called @moto.mock_sqs -def test_should_allow_valid_message( - notify_api, notify_db, notify_db_session, sample_service, mocker): +def test_should_allow_valid_message(notify_api, notify_db, notify_db_session, mocker): """ Tests POST endpoint '/sms' with notifications-admin notification. """ @@ -296,7 +285,7 @@ def test_should_allow_valid_message( "createdAt": "2015-11-03T09:37:27.414363Z", "id": 100, "jobId": 65, - "message": "This is the message", + "message": "valid", "method": "sms", "status": "created", "to": "+449999999999" @@ -378,13 +367,6 @@ def test_send_email_valid_data(notify_api, to_address, message, from_address, subject) -@moto.mock_sqs -def test_add_notification_to_queue(notify_api, notify_db, notify_db_session, sample_service): - set_up_mock_queue() - from app.notifications.rest import _add_notification_to_queue - _add_notification_to_queue('some message', sample_service, 'sms', '+447515349060') - - def set_up_mock_queue(): # set up mock queue boto3.setup_default_session(region_name='eu-west-1')