From 55adc9239f99fd38db6a181428f13b8c7105e773 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Wed, 6 Jul 2016 16:42:49 +0100 Subject: [PATCH 1/4] Fix update script --- migrations/versions/0039_fix_notifications.py | 43 ++++++++++++------- 1 file changed, 27 insertions(+), 16 deletions(-) diff --git a/migrations/versions/0039_fix_notifications.py b/migrations/versions/0039_fix_notifications.py index 14c509c49..f8f9b6f6c 100644 --- a/migrations/versions/0039_fix_notifications.py +++ b/migrations/versions/0039_fix_notifications.py @@ -17,26 +17,37 @@ import sqlalchemy as sa def upgrade(): op.execute('update notifications set notification_type = (select cast(cast(template_type as text) as notification_type) from templates where templates.id= notifications.template_id)') conn = op.get_bind() - del_sql = "select n.id, results.* from (select 'failed' as stat_type, count(status) as count, notification_type, date(created_at) as day, service_id " \ - "from notifications where status in ('temporary-failure', 'permanent-failure', 'technical-failure') group by service_id, notification_type, date(created_at) " \ - "union select 'delivered' as stat_type, count(status) , notification_type, date(created_at) as day, service_id " \ - "from notifications where status in ('delivered') group by service_id, notification_type, date(created_at)) as results, " \ - "notification_statistics n " \ - "where n.day = results.day and n.service_id = results.service_id order by results.day;" + reset_counts = "update notification_statistics set emails_requested = 0, emails_delivered = 0, emails_failed=0," \ + "sms_requested = 0, sms_delivered = 0, sms_failed=0 where day > '2016-06-30'" + op.execute(reset_counts) + all_notifications = "select * from notifications where date(created_at) > '2016-06-30' order by created_at;" - results = conn.execute(del_sql) + results = conn.execute(all_notifications) res = results.fetchall() for x in res: - if x.stat_type == 'delivered' and x.notification_type == 'email': - op.execute("update notification_statistics set emails_delivered = {} where id = '{}'".format(x.count, x.id)) - if x.stat_type == 'failed' and x.notification_type == 'email': - op.execute("update notification_statistics set emails_failed = {} where id = '{}'".format(x.count, x.id)) - if x.stat_type == 'delivered' and x.notification_type == 'sms': - op.execute("update notification_statistics set sms_delivered = {} where id = '{}'".format(x.count, x.id)) - if x.stat_type == 'failed' and x.notification_type == 'sms': - op.execute("update notification_statistics set sms_failed = {} where id = '{}'".format(x.count, x.id)) - + print(' in loop {} {}'.format(x.notification_type, x.created_at)) + created = x.created_at.strftime("%Y-%m-%d") + if x.notification_type == 'email' and x.status == 'delivered': + sql = "update notification_statistics set emails_requested = emails_requested + 1, " \ + "emails_delivered = emails_delivered + 1 where day = date('{}') and service_id = '{}'".format(created, x.service_id) + if x.notification_type == 'sms' and x.status == 'delivered': + sql = "update notification_statistics set sms_requested = sms_requested + 1, " \ + "sms_delivered = sms_delivered + 1 where day = date('{}') and service_id = '{}'".format(created, x.service_id) + if x.notification_type == 'email' and x.status in ['technical-failure', 'temporary-failure', 'permanent-failure']: + sql = "update notification_statistics set emails_requested = emails_requested + 1, " \ + "emails_failed = emails_failed + 1 where day = date('{}') and service_id = '{}'".format(created, x.service_id) + if x.notification_type == 'sms' and x.status in ['technical-failure', 'temporary-failure', 'permanent-failure']: + sql = "update notification_statistics set sms_requested = sms_requested + 1, " \ + "sms_failed = sms_failed + 1 where day = date('{}') and service_id = '{}'".format(created, x.service_id) + if x.notification_type == 'email' and x.status in ['created', 'sending', 'pending']: + sql = "update notification_statistics set emails_requested = emails_requested + 1 " \ + " where day = date('{}') and service_id = '{}'".format(created, x.service_id) + if x.notification_type == 'sms' and x.status in ['created', 'sending', 'pending']: + sql = "update notification_statistics set sms_requested = sms_requested + 1 " \ + " where day = date('{}') and service_id = '{}'".format(created, x.service_id) + print(sql) + conn.execute(sql) def downgrade(): ### commands auto generated by Alembic - please adjust! ### From 36ecdca04c8f57a2229bf31f848b184ded255ecb Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Thu, 7 Jul 2016 17:23:07 +0100 Subject: [PATCH 2/4] Add new email template for the GOV.UK Notify service, to send an email to users that register with the same email address. Add a new endpoint to send the email. --- app/user/rest.py | 25 +++++++++ config.py | 1 + migrations/versions/0041_email_template_.py | 56 +++++++++++++++++++++ tests/app/conftest.py | 35 +++++++++++++ tests/app/user/test_rest.py | 32 ++++++++++++ 5 files changed, 149 insertions(+) create mode 100644 migrations/versions/0041_email_template_.py diff --git a/app/user/rest.py b/app/user/rest.py index 167e4fcc3..4202ae06d 100644 --- a/app/user/rest.py +++ b/app/user/rest.py @@ -174,6 +174,31 @@ def send_user_email_verification(user_id): return jsonify({}), 204 +@user.route('//email-already-registered', methods=['POST']) +def send_already_registered_email(user_id): + template = dao_get_template_by_id(current_app.config['ALREADY_REGISTERED_EMAIL_TEMPLATE_ID']) + to, errors = request_verify_code_schema.load(request.get_json()) + + message = { + 'template': str(template.id), + 'template_version': template.version, + 'to': to['to'], + 'personalisation': { + 'signin_url': current_app.config['ADMIN_BASE_URL'] + '/sign-in', + 'forgot_password_url': current_app.config['ADMIN_BASE_URL'] + '/forgot-password', + 'feedback_url': current_app.config['ADMIN_BASE_URL'] + '/feedback' + } + } + send_email.apply_async(( + current_app.config['NOTIFY_SERVICE_ID'], + str(uuid.uuid4()), + encryption.encrypt(message), + datetime.utcnow().strftime(DATETIME_FORMAT) + ), queue='email-already-registered') + + return jsonify({}), 204 + + @user.route('/', methods=['GET']) @user.route('', methods=['GET']) def get_user(user_id=None): diff --git a/config.py b/config.py index 34b737435..33adefa84 100644 --- a/config.py +++ b/config.py @@ -33,6 +33,7 @@ class Config(object): SMS_CODE_TEMPLATE_ID = '36fb0730-6259-4da1-8a80-c8de22ad4246' EMAIL_VERIFY_CODE_TEMPLATE_ID = 'ece42649-22a8-4d06-b87f-d52d5d3f0a27' PASSWORD_RESET_TEMPLATE_ID = '474e9242-823b-4f99-813d-ed392e7f1201' + ALREADY_REGISTERED_EMAIL_TEMPLATE_ID = '0880fbb1-a0c6-46f0-9a8e-36c986381ceb' BROKER_URL = 'sqs://' BROKER_TRANSPORT_OPTIONS = { diff --git a/migrations/versions/0041_email_template_.py b/migrations/versions/0041_email_template_.py new file mode 100644 index 000000000..6e9218444 --- /dev/null +++ b/migrations/versions/0041_email_template_.py @@ -0,0 +1,56 @@ +"""empty message + +Revision ID: 0041_email_template +Revises: 0040_adjust_mmg_provider_rate +Create Date: 2016-07-07 16:02:06.241769 + +""" + +# revision identifiers, used by Alembic. +from datetime import datetime + +revision = '0041_email_template' +down_revision = '0040_adjust_mmg_provider_rate' + +from alembic import op + +user_id = '6af522d0-2915-4e52-83a3-3690455a5fe6' +service_id = 'd6aa2c68-a2d9-4437-ab19-3ae8eb202553' + + +def upgrade(): + template_history_insert = """INSERT INTO templates_history (id, name, template_type, created_at, + content, archived, service_id, + subject, created_by_id, version) + VALUES ('{}', '{}', '{}', '{}', '{}', False, '{}', '{}', '{}', 1) + """ + template_insert = """INSERT INTO templates (id, name, template_type, created_at, + content, archived, service_id, subject, created_by_id, version) + VALUES ('{}', '{}', '{}', '{}', '{}', False, '{}', '{}', '{}', 1) + """ + content = """You already have a GOV.UK Notify account with this email address. + +Sign in here: ((signin_url)) + +If you’ve forgotten your password, you can reset it here: ((forgot_password_url)) + + +If you didn’t try to register for a GOV.UK Notify account recently, please let us know here: ((feedback_url))""" + + op.get_bind() + op.execute(template_history_insert.format('0880fbb1-a0c6-46f0-9a8e-36c986381ceb', + 'Your GOV.UK Notify account', 'email', + datetime.utcnow(), content, service_id, + 'Your GOV.UK Notify account', user_id)) + op.execute( + template_insert.format('0880fbb1-a0c6-46f0-9a8e-36c986381ceb', 'Your GOV.UK Notify account', 'email', + datetime.utcnow(), content, service_id, + 'Your GOV.UK Notify account', user_id)) + + +def downgrade(): + op.execute("delete from notifications where template_id = '0880fbb1-a0c6-46f0-9a8e-36c986381ceb'") + op.execute("delete from jobs where template_id = '0880fbb1-a0c6-46f0-9a8e-36c986381ceb'") + op.execute("delete from template_statistics where template_id = '0880fbb1-a0c6-46f0-9a8e-36c986381ceb'") + op.execute("delete from templates_history where id = '0880fbb1-a0c6-46f0-9a8e-36c986381ceb'") + op.execute("delete from templates where id = '0880fbb1-a0c6-46f0-9a8e-36c986381ceb'") diff --git a/tests/app/conftest.py b/tests/app/conftest.py index 610b5e63c..cbb2707f4 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -699,3 +699,38 @@ def password_reset_email_template(notify_db, template = Template(**data) db.session.add(template) return template + + +@pytest.fixture(scope='function') +def already_registered_template(notify_db, + notify_db_session): + user = sample_user(notify_db, notify_db_session) + service = Service.query.get(current_app.config['NOTIFY_SERVICE_ID']) + if not service: + data = { + 'id': current_app.config['NOTIFY_SERVICE_ID'], + 'name': 'Notify Service', + 'message_limit': 1000, + 'active': True, + 'restricted': False, + 'email_from': 'notify.service', + 'created_by': user + } + service = Service(**data) + db.session.add(service) + + template = Template.query.get(current_app.config['ALREADY_REGISTERED_EMAIL_TEMPLATE_ID']) + if not template: + data = { + 'id': current_app.config['ALREADY_REGISTERED_EMAIL_TEMPLATE_ID'], + 'name': 'ALREADY_REGISTERED_EMAIL_TEMPLATE_ID', + 'template_type': 'email', + 'content': """Sign in here: ((signin_url)) If you’ve forgotten your password, + you can reset it here: ((forgot_password_url)) feedback:((feedback_url))""", + 'service': service, + 'created_by': user, + 'archived': False + } + template = Template(**data) + db.session.add(template) + return template diff --git a/tests/app/user/test_rest.py b/tests/app/user/test_rest.py index 0b979711c..b6ad36f29 100644 --- a/tests/app/user/test_rest.py +++ b/tests/app/user/test_rest.py @@ -455,3 +455,35 @@ def test_send_user_reset_password_should_return_400_when_data_is_not_email_addre assert resp.status_code == 400 assert json.loads(resp.get_data(as_text=True))['message'] == {'email': ['Not a valid email address']} + + +@freeze_time("2016-01-01 11:09:00.061258") +def test_send_already_registered_email(notify_api, sample_user, already_registered_template, mocker): + with notify_api.test_request_context(): + with notify_api.test_client() as client: + data = json.dumps({'to': sample_user.email_address}) + auth_header = create_authorization_header() + mocker.patch('app.celery.tasks.send_email.apply_async') + mocker.patch('uuid.uuid4', return_value='some_uuid') # for the notification id + + resp = client.post( + url_for('user.send_already_registered_email', user_id=str(sample_user.id)), + data=data, + headers=[('Content-Type', 'application/json'), auth_header]) + assert resp.status_code == 204 + message = { + 'template': str(already_registered_template.id), + 'template_version': already_registered_template.version, + 'to': sample_user.email_address, + 'personalisation': { + 'signin_url': current_app.config['ADMIN_BASE_URL'] + '/sign-in', + 'forgot_password_url': current_app.config['ADMIN_BASE_URL'] + '/forgot-password', + 'feedback_url': current_app.config['ADMIN_BASE_URL'] + '/feedback' + } + } + 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.061258"), + queue="email-already-registered") From f4976539e4e52303f09d411cf6def99d99e7daf4 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Fri, 8 Jul 2016 10:57:20 +0100 Subject: [PATCH 3/4] Make email a required field for the email_data_schema. --- app/schemas.py | 2 +- app/user/rest.py | 7 +++---- tests/app/user/test_rest.py | 31 ++++++++++++++++++++++++++++++- 3 files changed, 34 insertions(+), 6 deletions(-) diff --git a/app/schemas.py b/app/schemas.py index 0cb9010e3..eacf41333 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -319,7 +319,7 @@ class EmailDataSchema(ma.Schema): class Meta: strict = True - email = fields.Str(required=False) + email = fields.Str(required=True) @validates('email') def validate_email(self, value): diff --git a/app/user/rest.py b/app/user/rest.py index 4202ae06d..f0cc8a84a 100644 --- a/app/user/rest.py +++ b/app/user/rest.py @@ -23,8 +23,7 @@ from app.schemas import ( user_schema, request_verify_code_schema, user_schema_load_json, - permission_schema -) + permission_schema) from app.celery.tasks import ( send_sms, @@ -176,13 +175,13 @@ def send_user_email_verification(user_id): @user.route('//email-already-registered', methods=['POST']) def send_already_registered_email(user_id): + to, errors = email_data_request_schema.load(request.get_json()) template = dao_get_template_by_id(current_app.config['ALREADY_REGISTERED_EMAIL_TEMPLATE_ID']) - to, errors = request_verify_code_schema.load(request.get_json()) message = { 'template': str(template.id), 'template_version': template.version, - 'to': to['to'], + 'to': to['email'], 'personalisation': { 'signin_url': current_app.config['ADMIN_BASE_URL'] + '/sign-in', 'forgot_password_url': current_app.config['ADMIN_BASE_URL'] + '/forgot-password', diff --git a/tests/app/user/test_rest.py b/tests/app/user/test_rest.py index b6ad36f29..9e3d726fe 100644 --- a/tests/app/user/test_rest.py +++ b/tests/app/user/test_rest.py @@ -424,6 +424,21 @@ def test_send_user_reset_password_should_send_reset_password_link(notify_api, queue="email-reset-password") +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() + + 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.']} + + def test_send_user_reset_password_should_return_400_when_user_doesnot_exist(notify_api, mocker): with notify_api.test_request_context(): @@ -461,7 +476,7 @@ def test_send_user_reset_password_should_return_400_when_data_is_not_email_addre def test_send_already_registered_email(notify_api, sample_user, already_registered_template, mocker): with notify_api.test_request_context(): with notify_api.test_client() as client: - data = json.dumps({'to': sample_user.email_address}) + data = json.dumps({'email': sample_user.email_address}) auth_header = create_authorization_header() mocker.patch('app.celery.tasks.send_email.apply_async') mocker.patch('uuid.uuid4', return_value='some_uuid') # for the notification id @@ -487,3 +502,17 @@ def test_send_already_registered_email(notify_api, sample_user, already_register app.encryption.encrypt(message), "2016-01-01T11:09:00.061258"), queue="email-already-registered") + + +def test_send_already_registered_email_returns_400_when_data_is_missing(notify_api, sample_user): + with notify_api.test_request_context(): + with notify_api.test_client() as client: + data = json.dumps({}) + auth_header = create_authorization_header() + + resp = client.post( + url_for('user.send_already_registered_email', user_id=str(sample_user.id)), + 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.']} From 7927901938ef88b138eeccad0c845cc3584222b4 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Tue, 12 Jul 2016 10:45:47 +0100 Subject: [PATCH 4/4] Fix indentation --- tests/app/user/test_rest.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/app/user/test_rest.py b/tests/app/user/test_rest.py index 9e3d726fe..7d2e61ff5 100644 --- a/tests/app/user/test_rest.py +++ b/tests/app/user/test_rest.py @@ -430,10 +430,10 @@ def test_send_user_reset_password_should_return_400_when_email_is_missing(notify 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.']}