From a03732472c68e8d890057fd4da68c7098b889aef Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Tue, 20 Dec 2016 11:55:26 +0000 Subject: [PATCH] Refactor send_user_reset_password to persist and send message to the notify queue. The reason for doing this is to ensure the tasks performed for the Notify users are not queued behind a large job, a way to ensure priority for messages. 5th task for story: https://www.pivotaltracker.com/story/show/135839709 --- app/user/rest.py | 25 ++++--- tests/app/user/test_rest.py | 113 ++++++++++++----------------- tests/app/user/test_rest_verify.py | 8 +- 3 files changed, 65 insertions(+), 81 deletions(-) diff --git a/app/user/rest.py b/app/user/rest.py index 5ee8558be..77e757d23 100644 --- a/app/user/rest.py +++ b/app/user/rest.py @@ -288,19 +288,22 @@ def send_user_reset_password(): user_to_send_to = get_user_by_email(email['email']) template = dao_get_template_by_id(current_app.config['PASSWORD_RESET_TEMPLATE_ID']) - message = { - 'template': str(template.id), - 'template_version': template.version, - 'to': user_to_send_to.email_address, - 'personalisation': { + + saved_notification = persist_notification( + template_id=template.id, + template_version=template.version, + recipient=email['email'], + service_id=current_app.config['NOTIFY_SERVICE_ID'], + personalisation={ 'user_name': user_to_send_to.name, 'url': _create_reset_password_url(user_to_send_to.email_address) - } - } - send_email.apply_async([current_app.config['NOTIFY_SERVICE_ID'], - str(uuid.uuid4()), - encryption.encrypt(message), - datetime.utcnow().strftime(DATETIME_FORMAT)], queue='notify') + }, + notification_type=EMAIL_TYPE, + api_key_id=None, + key_type=KEY_TYPE_NORMAL + ) + + send_notification_to_queue(saved_notification, False, queue="notify") return jsonify({}), 204 diff --git a/tests/app/user/test_rest.py b/tests/app/user/test_rest.py index 269139c6d..b9580827d 100644 --- a/tests/app/user/test_rest.py +++ b/tests/app/user/test_rest.py @@ -414,86 +414,69 @@ def test_set_user_permissions_remove_old(notify_api, @freeze_time("2016-01-01 11:09:00.061258") -def test_send_user_reset_password_should_send_reset_password_link(notify_api, +def test_send_user_reset_password_should_send_reset_password_link(client, sample_user, mocker, password_reset_email_template): - with notify_api.test_request_context(): - with notify_api.test_client() as client: - mocker.patch('notifications_utils.url_safe_token.generate_token', return_value='the-token') - mocker.patch('uuid.uuid4', return_value='some_uuid') # for the notification id - mocker.patch('app.celery.tasks.send_email.apply_async') - data = json.dumps({'email': sample_user.email_address}) - auth_header = create_authorization_header() - resp = client.post( - url_for('user.send_user_reset_password'), - data=data, - headers=[('Content-Type', 'application/json'), auth_header]) + mocked = mocker.patch('app.celery.provider_tasks.deliver_email.apply_async') + data = json.dumps({'email': sample_user.email_address}) + auth_header = create_authorization_header() + resp = client.post( + url_for('user.send_user_reset_password'), + data=data, + headers=[('Content-Type', 'application/json'), auth_header]) - message = { - 'template': str(password_reset_email_template.id), - 'template_version': password_reset_email_template.version, - 'to': sample_user.email_address, - 'personalisation': { - 'user_name': sample_user.name, - 'url': current_app.config['ADMIN_BASE_URL'] + '/new-password/' + 'the-token' - } - } - assert resp.status_code == 204 - app.celery.tasks.send_email.apply_async.assert_called_once_with( - [str(current_app.config['NOTIFY_SERVICE_ID']), - 'some_uuid', - app.encryption.encrypt(message), - "2016-01-01T11:09:00.061258Z"], - queue="notify") + assert resp.status_code == 204 + notification = Notification.query.first() + mocked.assert_called_once_with([str(notification.id)], queue="notify") -def test_send_user_reset_password_should_return_400_when_email_is_missing(notify_api): - with notify_api.test_request_context(): - with notify_api.test_client() as client: - data = json.dumps({}) - auth_header = create_authorization_header() +def test_send_user_reset_password_should_return_400_when_email_is_missing(client, mocker): + mocked = mocker.patch('app.celery.provider_tasks.deliver_email.apply_async') + data = json.dumps({}) + auth_header = create_authorization_header() - resp = client.post( - url_for('user.send_user_reset_password'), - data=data, - headers=[('Content-Type', 'application/json'), auth_header]) + resp = client.post( + url_for('user.send_user_reset_password'), + data=data, + headers=[('Content-Type', 'application/json'), auth_header]) - assert resp.status_code == 400 - assert json.loads(resp.get_data(as_text=True))['message'] == {'email': ['Missing data for required field.']} + assert resp.status_code == 400 + assert json.loads(resp.get_data(as_text=True))['message'] == {'email': ['Missing data for required field.']} + assert mocked.call_count == 0 -def test_send_user_reset_password_should_return_400_when_user_doesnot_exist(notify_api, +def test_send_user_reset_password_should_return_400_when_user_doesnot_exist(client, mocker): - with notify_api.test_request_context(): - with notify_api.test_client() as client: - bad_email_address = 'bad@email.gov.uk' - data = json.dumps({'email': bad_email_address}) - auth_header = create_authorization_header() + mocked = mocker.patch('app.celery.provider_tasks.deliver_email.apply_async') + bad_email_address = 'bad@email.gov.uk' + data = json.dumps({'email': bad_email_address}) + auth_header = create_authorization_header() - resp = client.post( - url_for('user.send_user_reset_password'), - data=data, - headers=[('Content-Type', 'application/json'), auth_header]) + resp = client.post( + url_for('user.send_user_reset_password'), + data=data, + headers=[('Content-Type', 'application/json'), auth_header]) - assert resp.status_code == 404 - assert json.loads(resp.get_data(as_text=True))['message'] == 'No result found' + assert resp.status_code == 404 + assert json.loads(resp.get_data(as_text=True))['message'] == 'No result found' + assert mocked.call_count == 0 -def test_send_user_reset_password_should_return_400_when_data_is_not_email_address(notify_api, mocker): - with notify_api.test_request_context(): - with notify_api.test_client() as client: - bad_email_address = 'bad.email.gov.uk' - data = json.dumps({'email': bad_email_address}) - auth_header = create_authorization_header() +def test_send_user_reset_password_should_return_400_when_data_is_not_email_address(client, mocker): + mocked = mocker.patch('app.celery.provider_tasks.deliver_email.apply_async') + bad_email_address = 'bad.email.gov.uk' + data = json.dumps({'email': bad_email_address}) + auth_header = create_authorization_header() - resp = client.post( - url_for('user.send_user_reset_password'), - data=data, - headers=[('Content-Type', 'application/json'), auth_header]) + resp = client.post( + url_for('user.send_user_reset_password'), + data=data, + headers=[('Content-Type', 'application/json'), auth_header]) - assert resp.status_code == 400 - assert json.loads(resp.get_data(as_text=True))['message'] == {'email': ['Not a valid email address.']} + assert resp.status_code == 400 + assert json.loads(resp.get_data(as_text=True))['message'] == {'email': ['Not a valid email address.']} + assert mocked.call_count == 0 def test_send_already_registered_email(client, sample_user, already_registered_template, mocker): @@ -508,9 +491,7 @@ def test_send_already_registered_email(client, sample_user, already_registered_t assert resp.status_code == 204 notification = Notification.query.first() - mocked.assert_called_once_with( - ([str(notification.id)]), - queue="notify") + mocked.assert_called_once_with(([str(notification.id)]), queue="notify") def test_send_already_registered_email_returns_400_when_data_is_missing(client, sample_user): diff --git a/tests/app/user/test_rest_verify.py b/tests/app/user/test_rest_verify.py index f389312f8..6f84d5154 100644 --- a/tests/app/user/test_rest_verify.py +++ b/tests/app/user/test_rest_verify.py @@ -324,15 +324,14 @@ def test_send_user_email_verification(client, headers=[('Content-Type', 'application/json'), auth_header]) assert resp.status_code == 204 notification = Notification.query.first() - mocked.assert_called_once_with( - ([str(notification.id)]), - queue="notify") + mocked.assert_called_once_with(([str(notification.id)]), queue="notify") -def test_send_email_verification_returns_404_for_bad_input_data(client, notify_db, notify_db_session): +def test_send_email_verification_returns_404_for_bad_input_data(client, notify_db, notify_db_session, mocker): """ Tests POST endpoint /user//sms-code return 404 for bad input data """ + mocked = mocker.patch('app.celery.provider_tasks.deliver_email.apply_async') data = json.dumps({}) import uuid uuid_ = uuid.uuid4() @@ -343,3 +342,4 @@ def test_send_email_verification_returns_404_for_bad_input_data(client, notify_d headers=[('Content-Type', 'application/json'), auth_header]) assert resp.status_code == 404 assert json.loads(resp.get_data(as_text=True))['message'] == 'No result found' + assert mocked.call_count == 0