From 9df24b39ceaf529447f10a0355d01b160b168e9b Mon Sep 17 00:00:00 2001 From: Imdad Ahad Date: Tue, 13 Sep 2016 17:00:28 +0100 Subject: [PATCH 1/4] Add capability for simulated numbers and email addresses --- app/notifications/rest.py | 56 +++++++++++++------ config.py | 7 +++ .../rest/test_send_notification.py | 51 ++++++++++++++++- 3 files changed, 94 insertions(+), 20 deletions(-) diff --git a/app/notifications/rest.py b/app/notifications/rest.py index 35e55f2ed..4647cfd8b 100644 --- a/app/notifications/rest.py +++ b/app/notifications/rest.py @@ -210,7 +210,6 @@ def send_notification(notification_type): template_id=notification['template'], service_id=service_id ) - errors = unarchived_template_schema.validate({'archived': template.archived}) if errors: raise InvalidRequest(errors, status_code=400) @@ -261,24 +260,29 @@ def send_notification(notification_type): status_code=400 ) - notification_id = create_uuid() - notification.update({"template_version": template.version}) - persist_notification( - service, - notification_id, - notification, - datetime.utcnow().strftime(DATETIME_FORMAT), - notification_type, - str(api_user.id), - api_user.key_type - ) - - return jsonify( - data=get_notification_return_data( + if not _simulated_recipient(notification['to'], notification_type): + notification_id = create_uuid() + notification.update({"template_version": template.version}) + persist_notification( + service, notification_id, notification, - template_object) - ), 201 + datetime.utcnow().strftime(DATETIME_FORMAT), + notification_type, + str(api_user.id), + api_user.key_type + ) + + return jsonify( + data=get_notification_return_data( + notification_id, + notification, + template_object) + ), 201 + else: + return jsonify( + data=_get_simulated_recipient_response_data(template_object) + ), 200 def get_notification_return_data(notification_id, notification, template): @@ -304,6 +308,24 @@ def get_notification_statistics_for_day(): return jsonify(data=data), 200 +def _simulated_recipient(to_address, notification_type): + return (to_address in current_app.config['SIMULATED_SMS_NUMBERS'] + if notification_type == SMS_TYPE + else to_address in current_app.config['SIMULATED_EMAIL_ADDRESSES']) + + +def _get_simulated_recipient_response_data(template_object): + response_data = { + 'body': template_object.replaced, + 'template_version': template_object._template['version'] + } + + if template_object._template['template_type'] == 'email': + response_data.update({'subject': template_object.replaced_subject}) + + return response_data + + def persist_notification( service, notification_id, diff --git a/config.py b/config.py index 6b2be3721..fed24ce27 100644 --- a/config.py +++ b/config.py @@ -142,6 +142,13 @@ class Config(object): SENDING_NOTIFICATIONS_TIMEOUT_PERIOD = 259200 + SIMULATED_EMAIL_ADDRESSES = ('simulate-delivered@notifications.service.gov.uk', + 'simulate-permanent-failure@notifications.service.gov.uk', + 'simulate-temporary-failure@notifications.service.gov.uk', + ) + + SIMULATED_SMS_NUMBERS = ('+447700900000', '+447700900111', '+447700900222') + ###################### # Config overrides ### diff --git a/tests/app/notifications/rest/test_send_notification.py b/tests/app/notifications/rest/test_send_notification.py index 9dd29e1b5..97cd8ec53 100644 --- a/tests/app/notifications/rest/test_send_notification.py +++ b/tests/app/notifications/rest/test_send_notification.py @@ -674,14 +674,11 @@ def test_should_not_return_html_in_body(notify_api, notify_db, notify_db_session def test_should_not_send_email_if_team_api_key_and_not_a_service_user(notify_api, sample_email_template, mocker): with notify_api.test_request_context(), notify_api.test_client() as client: mocker.patch('app.celery.provider_tasks.send_email_to_provider.apply_async') - data = { 'to': "not-someone-we-trust@email-address.com", 'template': str(sample_email_template.id), } - # import pdb - # pdb.set_trace() auth_header = create_authorization_header(service_id=sample_email_template.service_id, key_type=KEY_TYPE_TEAM) response = client.post( @@ -944,3 +941,51 @@ def test_should_delete_sms_notification_and_return_error_if_sqs_fails(notify_api assert response.status_code == 500 assert not notifications_dao.get_notification_by_id(fake_uuid) assert not NotificationHistory.query.get(fake_uuid) + + +@pytest.mark.parametrize('to_email', [ + 'simulate-delivered@notifications.service.gov.uk', + 'simulate-permanent-failure@notifications.service.gov.uk', + 'simulate-temporary-failure@notifications.service.gov.uk' +]) +def test_should_not_send_email_if_simulated_email_address(client, to_email, sample_email_template, mocker): + apply_async = mocker.patch('app.celery.provider_tasks.send_email_to_provider.apply_async') + + data = { + 'to': to_email, + 'template': sample_email_template.id + } + + auth_header = create_authorization_header(service_id=sample_email_template.service_id) + + response = client.post( + path='/notifications/email', + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), auth_header]) + + assert response.status_code == 200 + apply_async.assert_not_called() + + +@pytest.mark.parametrize('to_sms', [ + '07700 900000', + '07700 900111', + '07700 900222' +]) +def test_should_not_send_sms_if_simulated_sms_number(client, to_sms, sample_template, mocker): + apply_async = mocker.patch('app.celery.provider_tasks.send_sms_to_provider.apply_async') + + data = { + 'to': to_sms, + 'template': sample_template.id + } + + auth_header = create_authorization_header(service_id=sample_template.service_id) + + response = client.post( + path='/notifications/sms', + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), auth_header]) + + assert response.status_code == 200 + apply_async.assert_not_called() From d4b0f68a69874c30f51c686190200550c37a6c6f Mon Sep 17 00:00:00 2001 From: Imdad Ahad Date: Wed, 14 Sep 2016 10:25:09 +0100 Subject: [PATCH 2/4] Refactor to re-use existing contract-dependent method --- app/notifications/rest.py | 33 +++++-------------- .../rest/test_send_notification.py | 4 +-- 2 files changed, 11 insertions(+), 26 deletions(-) diff --git a/app/notifications/rest.py b/app/notifications/rest.py index 4647cfd8b..202ebc653 100644 --- a/app/notifications/rest.py +++ b/app/notifications/rest.py @@ -260,9 +260,10 @@ def send_notification(notification_type): status_code=400 ) + notification_id = create_uuid() + notification.update({"template_version": template.version}) + if not _simulated_recipient(notification['to'], notification_type): - notification_id = create_uuid() - notification.update({"template_version": template.version}) persist_notification( service, notification_id, @@ -273,16 +274,12 @@ def send_notification(notification_type): api_user.key_type ) - return jsonify( - data=get_notification_return_data( - notification_id, - notification, - template_object) - ), 201 - else: - return jsonify( - data=_get_simulated_recipient_response_data(template_object) - ), 200 + return jsonify( + data=get_notification_return_data( + notification_id, + notification, + template_object) + ), 201 def get_notification_return_data(notification_id, notification, template): @@ -314,18 +311,6 @@ def _simulated_recipient(to_address, notification_type): else to_address in current_app.config['SIMULATED_EMAIL_ADDRESSES']) -def _get_simulated_recipient_response_data(template_object): - response_data = { - 'body': template_object.replaced, - 'template_version': template_object._template['version'] - } - - if template_object._template['template_type'] == 'email': - response_data.update({'subject': template_object.replaced_subject}) - - return response_data - - def persist_notification( service, notification_id, diff --git a/tests/app/notifications/rest/test_send_notification.py b/tests/app/notifications/rest/test_send_notification.py index 97cd8ec53..0d8ad94c5 100644 --- a/tests/app/notifications/rest/test_send_notification.py +++ b/tests/app/notifications/rest/test_send_notification.py @@ -963,7 +963,7 @@ def test_should_not_send_email_if_simulated_email_address(client, to_email, samp data=json.dumps(data), headers=[('Content-Type', 'application/json'), auth_header]) - assert response.status_code == 200 + assert response.status_code == 201 apply_async.assert_not_called() @@ -987,5 +987,5 @@ def test_should_not_send_sms_if_simulated_sms_number(client, to_sms, sample_temp data=json.dumps(data), headers=[('Content-Type', 'application/json'), auth_header]) - assert response.status_code == 200 + assert response.status_code == 201 apply_async.assert_not_called() From af8cfbe78c065c8b9ff9e7afdb6b9074d39d7f5a Mon Sep 17 00:00:00 2001 From: Imdad Ahad Date: Wed, 14 Sep 2016 11:12:57 +0100 Subject: [PATCH 3/4] Add checks to ensure simulated notifications are not persisted to database --- .../rest/test_send_notification.py | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/tests/app/notifications/rest/test_send_notification.py b/tests/app/notifications/rest/test_send_notification.py index 0d8ad94c5..153b443b4 100644 --- a/tests/app/notifications/rest/test_send_notification.py +++ b/tests/app/notifications/rest/test_send_notification.py @@ -948,7 +948,13 @@ def test_should_delete_sms_notification_and_return_error_if_sqs_fails(notify_api 'simulate-permanent-failure@notifications.service.gov.uk', 'simulate-temporary-failure@notifications.service.gov.uk' ]) -def test_should_not_send_email_if_simulated_email_address(client, to_email, sample_email_template, mocker): +def test_should_not_persist_notification_or_send_email_if_simulated_email( + client, + to_email, + sample_email_template, + fake_uuid, + mocker): + mocker.patch('app.notifications.rest.create_uuid', return_value=fake_uuid) apply_async = mocker.patch('app.celery.provider_tasks.send_email_to_provider.apply_async') data = { @@ -964,6 +970,7 @@ def test_should_not_send_email_if_simulated_email_address(client, to_email, samp headers=[('Content-Type', 'application/json'), auth_header]) assert response.status_code == 201 + assert not notifications_dao.get_notification_by_id(fake_uuid) apply_async.assert_not_called() @@ -972,7 +979,13 @@ def test_should_not_send_email_if_simulated_email_address(client, to_email, samp '07700 900111', '07700 900222' ]) -def test_should_not_send_sms_if_simulated_sms_number(client, to_sms, sample_template, mocker): +def test_should_not_persist_notification_or_send_sms_if_simulated_number( + client, + to_sms, + sample_template, + fake_uuid, + mocker): + mocker.patch('app.notifications.rest.create_uuid', return_value=fake_uuid) apply_async = mocker.patch('app.celery.provider_tasks.send_sms_to_provider.apply_async') data = { @@ -988,4 +1001,5 @@ def test_should_not_send_sms_if_simulated_sms_number(client, to_sms, sample_temp headers=[('Content-Type', 'application/json'), auth_header]) assert response.status_code == 201 + assert not notifications_dao.get_notification_by_id(fake_uuid) apply_async.assert_not_called() From 98ef340bc60420913563f8c6b3cdddf9b45c6b1e Mon Sep 17 00:00:00 2001 From: Imdad Ahad Date: Wed, 14 Sep 2016 13:07:57 +0100 Subject: [PATCH 4/4] Refactor to use .count() method instead of mocking fake UUID --- tests/app/notifications/rest/test_send_notification.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/tests/app/notifications/rest/test_send_notification.py b/tests/app/notifications/rest/test_send_notification.py index 153b443b4..0b5e5eaf0 100644 --- a/tests/app/notifications/rest/test_send_notification.py +++ b/tests/app/notifications/rest/test_send_notification.py @@ -952,9 +952,7 @@ def test_should_not_persist_notification_or_send_email_if_simulated_email( client, to_email, sample_email_template, - fake_uuid, mocker): - mocker.patch('app.notifications.rest.create_uuid', return_value=fake_uuid) apply_async = mocker.patch('app.celery.provider_tasks.send_email_to_provider.apply_async') data = { @@ -970,8 +968,8 @@ def test_should_not_persist_notification_or_send_email_if_simulated_email( headers=[('Content-Type', 'application/json'), auth_header]) assert response.status_code == 201 - assert not notifications_dao.get_notification_by_id(fake_uuid) apply_async.assert_not_called() + assert Notification.query.count() == 0 @pytest.mark.parametrize('to_sms', [ @@ -983,9 +981,7 @@ def test_should_not_persist_notification_or_send_sms_if_simulated_number( client, to_sms, sample_template, - fake_uuid, mocker): - mocker.patch('app.notifications.rest.create_uuid', return_value=fake_uuid) apply_async = mocker.patch('app.celery.provider_tasks.send_sms_to_provider.apply_async') data = { @@ -1001,5 +997,5 @@ def test_should_not_persist_notification_or_send_sms_if_simulated_number( headers=[('Content-Type', 'application/json'), auth_header]) assert response.status_code == 201 - assert not notifications_dao.get_notification_by_id(fake_uuid) apply_async.assert_not_called() + assert Notification.query.count() == 0