From 62e5361b0c77b07b33edb17c072bcbd88b57a840 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Wed, 24 Aug 2016 15:00:09 +0100 Subject: [PATCH 01/15] cleaned up environment file * sorted list in README and environment_test.sh * removed some unused vars * cleaned up some names to be more accurate in the readme * removed twilio as a dependency --- README.md | 26 +++++++++++++++----------- config.py | 1 - environment_test.sh | 5 ++--- requirements.txt | 1 - 4 files changed, 17 insertions(+), 16 deletions(-) diff --git a/README.md b/README.md index 8d31a6be3..018c372de 100644 --- a/README.md +++ b/README.md @@ -16,7 +16,7 @@ Get and update notification status. mkvirtualenv -p /usr/local/bin/python3 notifications-api ``` -Creating the environment.sh file. Replace [unique-to-environment] with your something unique to the environment. The local development environments are using the AWS on preview. +Creating the environment.sh file. Replace [unique-to-environment] with your something unique to the environment. Your AWS credentials should be set up for notify-tools (the development/CI AWS account). Create a local environment.sh file containing the following: @@ -24,30 +24,34 @@ Create a local environment.sh file containing the following: echo " export NOTIFY_ENVIRONMENT='development' export ADMIN_BASE_URL='http://localhost:6012' -export ADMIN_CLIENT_SECRET='dev-notify-secret-key' export ADMIN_CLIENT_USER_NAME='dev-notify-admin' +export ADMIN_CLIENT_SECRET='dev-notify-secret-key' +export API_HOST_NAME='http://localhost:6011' + export AWS_REGION='eu-west-1' +export AWS_ACCESS_KEY_ID=[MY ACCESS KEY] +export AWS_SECRET_ACCESS_KEY=[MY SECRET] + export DANGEROUS_SALT='dev-notify-salt' export FIRETEXT_API_KEY=[contact team member for api key] -export INVITATION_EMAIL_FROM='invites@notifications.service.gov.uk' +export FROM_NUMBER='40605' +export INVITATION_EMAIL_FROM='invites' export INVITATION_EXPIRATION_DAYS=2 -export NOTIFY_EMAIL_DOMAIN='notify.works' -export NOTIFY_JOB_QUEUE='[unique-to-environment]-notify-jobs-queue' # NOTE unique prefix -export NOTIFICATION_QUEUE_PREFIX='[unique-to-environment]-notification_development' # NOTE unique prefix +export MMG_API_KEY=mmg=secret-key +export MMG_URL="https://api.mmg.co.uk/json/api.php" +export NOTIFICATION_QUEUE_PREFIX='[unique-to-environment]' # +export NOTIFY_EMAIL_DOMAIN='notify.tools' export SECRET_KEY='dev-notify-secret-key' export SQLALCHEMY_DATABASE_URI='postgresql://localhost/notification_api' -export VERIFY_CODE_FROM_EMAIL_ADDRESS='no-reply@notify.works' -export MMG_API_KEY=mmg=secret-key export STATSD_ENABLED=True export STATSD_HOST="localhost" export STATSD_PORT=1000 export STATSD_PREFIX="stats-prefix" -export FROM_NUMBER='from_number' +export VERIFY_CODE_FROM_EMAIL_ADDRESS='no-reply@notify.tools' "> environment.sh ``` -NOTE: the DELIVERY_CLIENT_USER_NAME, DELIVERY_CLIENT_SECRET, NOTIFY_JOB_QUEUE and NOTIFICATION_QUEUE_PREFIX must be the same as the ones in the [notifications-delivery](https://github.com/alphagov/notifications-delivery) app. -The SECRET_KEY and DANGEROUS_SALT are the same in [notifications-delivery](https://github.com/alphagov/notifications-delivery) and [notifications-admin](https://github.com/alphagov/notifications-admin) app. +NOTE: The SECRET_KEY and DANGEROUS_SALT should match those in the [notifications-admin](https://github.com/alphagov/notifications-admin) app. NOTE: Also note the unique prefix for the queue names. This prevents clashing with others queues in shared amazon environment and using a prefix enables filtering by queue name in the SQS interface. diff --git a/config.py b/config.py index d8067dc53..6e569bb23 100644 --- a/config.py +++ b/config.py @@ -15,7 +15,6 @@ class Config(object): INVITATION_EMAIL_FROM = os.environ['INVITATION_EMAIL_FROM'] NOTIFY_APP_NAME = 'api' NOTIFY_LOG_PATH = '/var/log/notify/application.log' - NOTIFY_JOB_QUEUE = os.environ['NOTIFY_JOB_QUEUE'] # Notification Queue names are a combination of a prefix plus a name NOTIFICATION_QUEUE_PREFIX = os.environ['NOTIFICATION_QUEUE_PREFIX'] SECRET_KEY = os.environ['SECRET_KEY'] diff --git a/environment_test.sh b/environment_test.sh index d4665632b..c265171d5 100644 --- a/environment_test.sh +++ b/environment_test.sh @@ -7,11 +7,10 @@ export AWS_REGION='eu-west-1' export DANGEROUS_SALT='dangerous-salt' export INVITATION_EMAIL_FROM='invites' export INVITATION_EXPIRATION_DAYS=2 -export NOTIFY_JOB_QUEUE='notify-jobs-queue-test' -export NOTIFICATION_QUEUE_PREFIX='notification_development-test' +export NOTIFICATION_QUEUE_PREFIX='test-env-not-used' export SECRET_KEY='secret-key' export SQLALCHEMY_DATABASE_URI=${TEST_DATABASE:='postgresql://localhost/test_notification_api'} -export VERIFY_CODE_FROM_EMAIL_ADDRESS='no-reply@notify.works' +export VERIFY_CODE_FROM_EMAIL_ADDRESS='no-reply@notify.tools' export FIRETEXT_API_KEY="Firetext" export NOTIFY_EMAIL_DOMAIN="test.notify.com" export MMG_API_KEY='mmg-secret-key' diff --git a/requirements.txt b/requirements.txt index 904bd6e3e..f3fa950eb 100644 --- a/requirements.txt +++ b/requirements.txt @@ -16,7 +16,6 @@ credstash==1.8.0 boto3==1.2.3 boto==2.39.0 celery==3.1.20 -twilio==4.6.0 monotonic==0.3 statsd==3.2.1 From 5eaec8c2351084dd5b54b37049aab257d3744a28 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Tue, 30 Aug 2016 10:47:27 +0100 Subject: [PATCH 02/15] Allow test key to send irrespective of trial mode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When you use a simulate API key it should behave like a live service, except for actually sending the messages. There should be no limits even if the service is in trial mode. This commit removes the restriction on sending messages to peole outside your team when you’re in trial mode. --- app/notifications/rest.py | 16 +++++--- .../rest/test_send_notification.py | 37 ++++++++++++++++++- 2 files changed, 46 insertions(+), 7 deletions(-) diff --git a/app/notifications/rest.py b/app/notifications/rest.py index f9fa85e2f..354cdbecb 100644 --- a/app/notifications/rest.py +++ b/app/notifications/rest.py @@ -15,7 +15,7 @@ from notifications_utils.renderers import PassThrough from app.clients.email.aws_ses import get_aws_responses from app import api_user, encryption, create_uuid, DATETIME_FORMAT, DATE_FORMAT, statsd_client from app.dao.services_dao import dao_fetch_todays_stats_for_service -from app.models import KEY_TYPE_TEAM +from app.models import KEY_TYPE_TEAM, KEY_TYPE_TEST from app.dao import ( templates_dao, services_dao, @@ -259,12 +259,16 @@ def send_notification(notification_type): errors = {'content': [message]} raise InvalidRequest(errors, status_code=400) - if (service.restricted or api_user.key_type == KEY_TYPE_TEAM) and not allowed_to_send_to( - notification['to'], - itertools.chain.from_iterable( - [user.mobile_number, user.email_address] for user in service.users + if all(( + api_user.key_type != KEY_TYPE_TEST, + service.restricted or api_user.key_type == KEY_TYPE_TEAM, + not allowed_to_send_to( + notification['to'], + itertools.chain.from_iterable( + [user.mobile_number, user.email_address] for user in service.users + ) ) - ): + )): if (api_user.key_type == KEY_TYPE_TEAM): message = 'Can’t send to this recipient using a team-only API key' else: diff --git a/tests/app/notifications/rest/test_send_notification.py b/tests/app/notifications/rest/test_send_notification.py index 47626ee85..96f1980c9 100644 --- a/tests/app/notifications/rest/test_send_notification.py +++ b/tests/app/notifications/rest/test_send_notification.py @@ -10,7 +10,7 @@ from notifications_python_client.authentication import create_jwt_token import app from app import encryption -from app.models import ApiKey, KEY_TYPE_TEAM +from app.models import ApiKey, KEY_TYPE_TEAM, KEY_TYPE_TEST from app.dao.templates_dao import dao_get_all_templates_for_service, dao_update_template from app.dao.services_dao import dao_update_service from app.dao.api_key_dao import save_model_api_key @@ -764,6 +764,41 @@ def test_should_send_email_if_team_api_key_and_a_service_user(notify_api, sample assert response.status_code == 201 +def test_should_send_email_to_anyone_with_test_key(notify_api, sample_email_template, mocker): + with notify_api.test_request_context(), notify_api.test_client() as client: + mocker.patch('app.celery.tasks.send_email.apply_async') + + data = { + 'to': 'anyone123@example.com', + 'template': sample_email_template.id + } + sample_email_template.service.restricted = True + api_key = ApiKey( + service=sample_email_template.service, + name='test_key', + created_by=sample_email_template.created_by, + key_type=KEY_TYPE_TEST + ) + save_model_api_key(api_key) + auth_header = create_jwt_token(secret=api_key.unsigned_secret, client_id=str(api_key.service_id)) + + response = client.post( + path='/notifications/email', + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), ('Authorization', 'Bearer {}'.format(auth_header))] + ) + + app.celery.tasks.send_email.apply_async.assert_called_once_with( + ANY, + kwargs={ + 'api_key_id': str(api_key.id), + 'key_type': api_key.key_type + }, + queue='email' + ) + assert response.status_code == 201 + + def test_should_send_sms_if_team_api_key_and_a_service_user(notify_api, sample_template, mocker): with notify_api.test_request_context(), notify_api.test_client() as client: mocker.patch('app.celery.tasks.send_sms.apply_async') From 7769923ddadbe4fa3a59f03cbed737365d6f9f83 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Tue, 30 Aug 2016 11:00:22 +0100 Subject: [PATCH 03/15] Allow test key to send irrespective of daily limit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When you use a simulate API key it should behave like a live service, except for actually sending the messages. There should be no limits even if the service is in trial mode. This commit removes the restriction on sending messages when you’ve sent up to your daily limit. --- app/notifications/rest.py | 5 ++++- tests/app/notifications/rest/test_send_notification.py | 9 +++++++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/app/notifications/rest.py b/app/notifications/rest.py index 354cdbecb..5352f1063 100644 --- a/app/notifications/rest.py +++ b/app/notifications/rest.py @@ -215,7 +215,10 @@ def send_notification(notification_type): service_stats = sum(row.count for row in dao_fetch_todays_stats_for_service(service.id)) - if service_stats >= service.message_limit: + if all(( + api_user.key_type != KEY_TYPE_TEST, + service_stats >= service.message_limit + )): error = 'Exceeded send limits ({}) for today'.format(service.message_limit) raise InvalidRequest(error, status_code=429) diff --git a/tests/app/notifications/rest/test_send_notification.py b/tests/app/notifications/rest/test_send_notification.py index 96f1980c9..0a1951352 100644 --- a/tests/app/notifications/rest/test_send_notification.py +++ b/tests/app/notifications/rest/test_send_notification.py @@ -764,7 +764,11 @@ def test_should_send_email_if_team_api_key_and_a_service_user(notify_api, sample assert response.status_code == 201 -def test_should_send_email_to_anyone_with_test_key(notify_api, sample_email_template, mocker): +@pytest.mark.parametrize('restricted', [True, False]) +@pytest.mark.parametrize('limit', [0, 1]) +def test_should_send_email_to_anyone_with_test_key( + notify_api, sample_email_template, mocker, restricted, limit +): with notify_api.test_request_context(), notify_api.test_client() as client: mocker.patch('app.celery.tasks.send_email.apply_async') @@ -772,7 +776,8 @@ def test_should_send_email_to_anyone_with_test_key(notify_api, sample_email_temp 'to': 'anyone123@example.com', 'template': sample_email_template.id } - sample_email_template.service.restricted = True + sample_email_template.service.restricted = restricted + sample_email_template.service.message_limit = limit api_key = ApiKey( service=sample_email_template.service, name='test_key', From e640e048e5b89f6437a005b02088f953dbd8e6d0 Mon Sep 17 00:00:00 2001 From: Martyn Inglis Date: Tue, 30 Aug 2016 17:27:01 +0100 Subject: [PATCH 04/15] Pushed the notifications from the CSV in the DB queue --- app/celery/tasks.py | 4 ++-- tests/app/celery/test_tasks.py | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/app/celery/tasks.py b/app/celery/tasks.py index 0ad308e49..a03df1878 100644 --- a/app/celery/tasks.py +++ b/app/celery/tasks.py @@ -84,7 +84,7 @@ def process_job(job_id): create_uuid(), encrypted, datetime.utcnow().strftime(DATETIME_FORMAT)), - queue='bulk-sms' + queue='db-sms' ) if template.template_type == EMAIL_TYPE: @@ -93,7 +93,7 @@ def process_job(job_id): create_uuid(), encrypted, datetime.utcnow().strftime(DATETIME_FORMAT)), - queue='bulk-email') + queue='db-email') finished = datetime.utcnow() job.status = 'finished' diff --git a/tests/app/celery/test_tasks.py b/tests/app/celery/test_tasks.py index d1c4ba18e..798d04b35 100644 --- a/tests/app/celery/test_tasks.py +++ b/tests/app/celery/test_tasks.py @@ -82,7 +82,7 @@ def test_should_process_sms_job(sample_job, mocker, mock_celery_remove_job): "uuid", "something_encrypted", "2016-01-01T11:09:00.061258"), - queue="bulk-sms" + queue="db-sms" ) job = jobs_dao.dao_get_job_by_id(sample_job.id) assert job.status == 'finished' @@ -201,7 +201,7 @@ def test_should_process_email_job_if_exactly_on_send_limits(notify_db, "something_encrypted", "2016-01-01T11:09:00.061258" ), - queue="bulk-email" + queue="db-email" ) mock_celery_remove_job.assert_called_once_with((str(job.id),), queue="remove-job") @@ -245,7 +245,7 @@ def test_should_process_email_job(sample_email_job, mocker, mock_celery_remove_j "something_encrypted", "2016-01-01T11:09:00.061258" ), - queue="bulk-email" + queue="db-email" ) job = jobs_dao.dao_get_job_by_id(sample_email_job.id) assert job.status == 'finished' From 316e4b9ea62c0e8ba0be1a4899392857438fc38c Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 31 Aug 2016 10:39:25 +0100 Subject: [PATCH 05/15] Remove outdated comment Added in January[1] and (for better or worse) it got done. 1. 49e98c21e74e4bdcb8ad63223a46170d05f9182d) --- app/schemas.py | 7 ------- 1 file changed, 7 deletions(-) diff --git a/app/schemas.py b/app/schemas.py index a053ab57c..673983114 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -64,13 +64,6 @@ def _validate_datetime_not_in_past(dte, msg="Date cannot be in the past"): raise ValidationError(msg) -# TODO I think marshmallow provides a better integration and error handling. -# Would be better to replace functionality in dao with the marshmallow supported -# functionality. -# http://marshmallow.readthedocs.org/en/latest/api_reference.html -# http://marshmallow.readthedocs.org/en/latest/extending.html - - class BaseSchema(ma.ModelSchema): def __init__(self, load_json=False, *args, **kwargs): From c108c493e6d4570315d92a9259e715232625d04d Mon Sep 17 00:00:00 2001 From: Martyn Inglis Date: Wed, 31 Aug 2016 11:40:31 +0100 Subject: [PATCH 06/15] Rejigged the queue config. - the two new queues that handle delivery of notifications (db-[type] / send-[type]) Now no longer are processed by the default workers - These workers now to the admin type queues for notify (CSV/Validation codes) etc. - Two new workers are deployed to the AWS environments, one focused on db- tasks and one for send tasks - These will pick up those queues explicitly. --- config.py | 40 ++++++++++++++++++++++++---------------- 1 file changed, 24 insertions(+), 16 deletions(-) diff --git a/config.py b/config.py index 6541b8f65..ee4ac076a 100644 --- a/config.py +++ b/config.py @@ -84,11 +84,7 @@ class Config(object): CELERY_QUEUES = [ Queue('periodic', Exchange('default'), routing_key='periodic'), Queue('sms', Exchange('default'), routing_key='sms'), - Queue('db-sms', Exchange('default'), routing_key='db-sms'), - Queue('send-sms', Exchange('default'), routing_key='send-sms'), Queue('email', Exchange('default'), routing_key='email'), - Queue('db-email', Exchange('default'), routing_key='db-email'), - Queue('send-email', Exchange('default'), routing_key='send-email'), Queue('sms-code', Exchange('default'), routing_key='sms-code'), Queue('email-code', Exchange('default'), routing_key='email-code'), Queue('email-reset-password', Exchange('default'), routing_key='email-reset-password'), @@ -123,6 +119,25 @@ class Development(Config): NOTIFY_ENVIRONMENT = 'development' CSV_UPLOAD_BUCKET_NAME = 'development-notifications-csv-upload' DEBUG = True + SQLALCHEMY_ECHO = False + CELERY_QUEUES = Config.CELERY_QUEUES + [ + Queue('db-sms', Exchange('default'), routing_key='db-sms'), + Queue('send-sms', Exchange('default'), routing_key='send-sms'), + Queue('db-email', Exchange('default'), routing_key='db-email'), + Queue('send-email', Exchange('default'), routing_key='send-email') + ] + + +class Test(Config): + NOTIFY_ENVIRONMENT = 'test' + CSV_UPLOAD_BUCKET_NAME = 'test-notifications-csv-upload' + STATSD_PREFIX = "test" + CELERY_QUEUES = Config.CELERY_QUEUES + [ + Queue('db-sms', Exchange('default'), routing_key='db-sms'), + Queue('send-sms', Exchange('default'), routing_key='send-sms'), + Queue('db-email', Exchange('default'), routing_key='db-email'), + Queue('send-email', Exchange('default'), routing_key='send-email') + ] class Preview(Config): @@ -131,12 +146,6 @@ class Preview(Config): STATSD_PREFIX = "preview" -class Test(Development): - NOTIFY_ENVIRONMENT = 'test' - CSV_UPLOAD_BUCKET_NAME = 'test-notifications-csv-upload' - STATSD_PREFIX = "test" - - class Staging(Config): NOTIFY_ENVIRONMENT = 'staging' CSV_UPLOAD_BUCKET_NAME = 'staging-notify-csv-upload' @@ -147,15 +156,14 @@ class Staging(Config): class Live(Config): NOTIFY_ENVIRONMENT = 'live' CSV_UPLOAD_BUCKET_NAME = 'live-notifications-csv-upload' - STATSD_ENABLED = True STATSD_PREFIX = os.getenv('STATSD_PREFIX') STATSD_ENABLED = True configs = { - 'development': Development, - 'test': Test, - 'live': Live, - 'staging': Staging, - 'preview': Preview + 'development': Development(), + 'test': Test(), + 'live': Live(), + 'staging': Staging(), + 'preview': Preview() } From ce006bd92d18000c3c30efa883ffc601abaf5edb Mon Sep 17 00:00:00 2001 From: Martyn Inglis Date: Wed, 31 Aug 2016 11:40:47 +0100 Subject: [PATCH 07/15] Start/Stop the new worker scripts on the code deploy actions. --- scripts/aws_start_app.sh | 12 ++++++++++++ scripts/aws_stop_app.sh | 22 ++++++++++++++++++++-- 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/scripts/aws_start_app.sh b/scripts/aws_start_app.sh index 7be657a03..47f7103ba 100755 --- a/scripts/aws_start_app.sh +++ b/scripts/aws_start_app.sh @@ -12,6 +12,18 @@ then sudo service notifications-api-celery-worker start fi +if [ -e "/etc/init/notifications-api-celery-worker-sender.conf" ] +then + echo "Starting celery worker" + sudo service notifications-api-celery-worker-sender start +fi + +if [ -e "/etc/init/notifications-api-celery-worker-db.conf" ] +then + echo "Starting celery worker" + sudo service notifications-api-celery-worker-db start +fi + if [ -e "/etc/init/notifications-api-celery-beat.conf" ] then echo "Starting celery beat" diff --git a/scripts/aws_stop_app.sh b/scripts/aws_stop_app.sh index 2f5660ae1..3f9735d4b 100755 --- a/scripts/aws_stop_app.sh +++ b/scripts/aws_stop_app.sh @@ -19,7 +19,7 @@ fi if [ -e "/etc/init/notifications-api-celery-beat.conf" ]; then echo "stopping notifications-api-celery-beat" if sudo service notifications-api-celery-beat stop; then - echo "notifications-api stopped" + echo "notifications-api beat stopped" else error_exit "Could not stop notifications-celery-beat" fi @@ -28,8 +28,26 @@ fi if [ -e "/etc/init/notifications-api-celery-worker.conf" ]; then echo "stopping notifications-api-celery-worker" if sudo service notifications-api-celery-worker stop; then - echo "notifications-api stopped" + echo "notifications-api worker stopped" else error_exit "Could not stop notifications-celery-worker" fi fi + +if [ -e "/etc/init/notifications-api-celery-worker-sender.conf" ]; then + echo "stopping notifications-api-celery-worker-sender" + if sudo service notifications-api-celery-worker-sender stop; then + echo "notifications-api sender worker stopped" + else + error_exit "Could not stop notifications-celery-worker-sender" + fi +fi + +if [ -e "/etc/init/notifications-api-celery-worker-db.conf" ]; then + echo "stopping notifications-api-celery-worker-db" + if sudo service notifications-api-celery-worker-db stop; then + echo "notifications-api db worker stopped" + else + error_exit "Could not stop notifications-celery-worker-db" + fi +fi From 06cb440fd289826bbc2b98ef3b78a41887e8fd25 Mon Sep 17 00:00:00 2001 From: Martyn Inglis Date: Wed, 31 Aug 2016 11:42:05 +0100 Subject: [PATCH 08/15] Not a function call. --- config.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/config.py b/config.py index ee4ac076a..fe44862d7 100644 --- a/config.py +++ b/config.py @@ -161,9 +161,9 @@ class Live(Config): configs = { - 'development': Development(), - 'test': Test(), - 'live': Live(), - 'staging': Staging(), - 'preview': Preview() + 'development': Development, + 'test': Test, + 'live': Live, + 'staging': Staging, + 'preview': Preview } From 6858641a3c262b02808e438f76433bab1e68881f Mon Sep 17 00:00:00 2001 From: Martyn Inglis Date: Wed, 31 Aug 2016 12:09:12 +0100 Subject: [PATCH 09/15] Debug mode on for tests. --- config.py | 1 + 1 file changed, 1 insertion(+) diff --git a/config.py b/config.py index fe44862d7..d234e8b63 100644 --- a/config.py +++ b/config.py @@ -130,6 +130,7 @@ class Development(Config): class Test(Config): NOTIFY_ENVIRONMENT = 'test' + DEBUG = True CSV_UPLOAD_BUCKET_NAME = 'test-notifications-csv-upload' STATSD_PREFIX = "test" CELERY_QUEUES = Config.CELERY_QUEUES + [ From 57267a36f29ba96eeaa423d4eeedfff387ece1f9 Mon Sep 17 00:00:00 2001 From: Martyn Inglis Date: Wed, 31 Aug 2016 12:12:09 +0100 Subject: [PATCH 10/15] Return empty stats list for job creation --- app/job/rest.py | 5 ++++- tests/app/job/test_rest.py | 2 ++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/app/job/rest.py b/app/job/rest.py index 235700918..8d490c333 100644 --- a/app/job/rest.py +++ b/app/job/rest.py @@ -129,4 +129,7 @@ def create_job(service_id): if job.job_status == JOB_STATUS_PENDING: process_job.apply_async([str(job.id)], queue="process-job") - return jsonify(data=job_schema.dump(job).data), 201 + job_json = job_schema.dump(job).data + job_json['statistics'] = [] + + return jsonify(data=job_json), 201 diff --git a/tests/app/job/test_rest.py b/tests/app/job/test_rest.py index df901cd1d..488a75419 100644 --- a/tests/app/job/test_rest.py +++ b/tests/app/job/test_rest.py @@ -140,6 +140,7 @@ def test_create_unscheduled_job(notify_api, sample_template, mocker, fake_uuid): assert resp_json['data']['id'] == fake_uuid assert resp_json['data']['status'] == 'pending' + assert resp_json['data']['statistics'] == [] assert not resp_json['data']['scheduled_for'] assert resp_json['data']['job_status'] == 'pending' assert resp_json['data']['template'] == str(sample_template.id) @@ -176,6 +177,7 @@ def test_create_scheduled_job(notify_api, sample_template, mocker, fake_uuid): resp_json = json.loads(response.get_data(as_text=True)) assert resp_json['data']['id'] == fake_uuid + assert resp_json['data']['statistics'] == [] assert resp_json['data']['status'] == 'pending' assert resp_json['data']['scheduled_for'] == datetime(2016, 1, 2, 11, 59, 0, tzinfo=pytz.UTC).isoformat() From e98a740dbf719b376e528441f789086f939d3623 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 31 Aug 2016 13:46:00 +0100 Subject: [PATCH 11/15] Fix test to expect correct db queue Sending emails was moved to a different queue in: https://github.com/alphagov/notifications-api/pull/647 At the time that https://github.com/alphagov/notifications-api/pull/645 was written, the previous queues were still being used. So when #645 got merged it was testing for the old queues, which broke the tests. --- tests/app/notifications/rest/test_send_notification.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/app/notifications/rest/test_send_notification.py b/tests/app/notifications/rest/test_send_notification.py index 450020e9c..b0492e8bb 100644 --- a/tests/app/notifications/rest/test_send_notification.py +++ b/tests/app/notifications/rest/test_send_notification.py @@ -799,7 +799,7 @@ def test_should_send_email_to_anyone_with_test_key( 'api_key_id': str(api_key.id), 'key_type': api_key.key_type }, - queue='email' + queue='db-email' ) assert response.status_code == 201 From 5ac4e630d814cb87f055396af15f26251c62e74a Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Wed, 31 Aug 2016 14:49:26 +0100 Subject: [PATCH 12/15] remove some legacy code/tests relating to old invite/verification code --- README.md | 1 - app/notifications/rest.py | 21 ---------- config.py | 1 - environment_test.sh | 1 - .../app/notifications/rest/test_callbacks.py | 41 ------------------- 5 files changed, 65 deletions(-) diff --git a/README.md b/README.md index 018c372de..ca31534b3 100644 --- a/README.md +++ b/README.md @@ -47,7 +47,6 @@ export STATSD_ENABLED=True export STATSD_HOST="localhost" export STATSD_PORT=1000 export STATSD_PREFIX="stats-prefix" -export VERIFY_CODE_FROM_EMAIL_ADDRESS='no-reply@notify.tools' "> environment.sh ``` diff --git a/app/notifications/rest.py b/app/notifications/rest.py index 8e4af63b2..1249926a2 100644 --- a/app/notifications/rest.py +++ b/app/notifications/rest.py @@ -77,15 +77,6 @@ def process_ses_response(): notification_statistics_status = aws_response_dict['notification_statistics_status'] try: - source = ses_message['mail']['source'] - if is_not_a_notification(source): - current_app.logger.info( - "SES callback for notify success:. source {} status {}".format(source, notification_status) - ) - return jsonify( - result="success", message="SES callback succeeded" - ), 200 - reference = ses_message['mail']['messageId'] if not notifications_dao.update_notification_status_by_reference( reference, @@ -118,18 +109,6 @@ def process_ses_response(): raise InvalidRequest(error, status_code=400) -def is_not_a_notification(source): - invite_email = "{}@{}".format( - current_app.config['INVITATION_EMAIL_FROM'], - current_app.config['NOTIFY_EMAIL_DOMAIN'] - ) - if current_app.config['VERIFY_CODE_FROM_EMAIL_ADDRESS'] == source: - return True - if invite_email == source: - return True - return False - - @notifications.route('/notifications/sms/mmg', methods=['POST']) def process_mmg_response(): client_name = 'MMG' diff --git a/config.py b/config.py index 6e569bb23..33f0540cd 100644 --- a/config.py +++ b/config.py @@ -22,7 +22,6 @@ class Config(object): SQLALCHEMY_DATABASE_URI = os.environ['SQLALCHEMY_DATABASE_URI'] SQLALCHEMY_RECORD_QUERIES = True SQLALCHEMY_TRACK_MODIFICATIONS = True - VERIFY_CODE_FROM_EMAIL_ADDRESS = os.environ['VERIFY_CODE_FROM_EMAIL_ADDRESS'] NOTIFY_EMAIL_DOMAIN = os.environ['NOTIFY_EMAIL_DOMAIN'] PAGE_SIZE = 50 SMS_CHAR_COUNT_LIMIT = 495 diff --git a/environment_test.sh b/environment_test.sh index c265171d5..d0c536072 100644 --- a/environment_test.sh +++ b/environment_test.sh @@ -10,7 +10,6 @@ export INVITATION_EXPIRATION_DAYS=2 export NOTIFICATION_QUEUE_PREFIX='test-env-not-used' export SECRET_KEY='secret-key' export SQLALCHEMY_DATABASE_URI=${TEST_DATABASE:='postgresql://localhost/test_notification_api'} -export VERIFY_CODE_FROM_EMAIL_ADDRESS='no-reply@notify.tools' export FIRETEXT_API_KEY="Firetext" export NOTIFY_EMAIL_DOMAIN="test.notify.com" export MMG_API_KEY='mmg-secret-key' diff --git a/tests/app/notifications/rest/test_callbacks.py b/tests/app/notifications/rest/test_callbacks.py index b6fc84757..8a7f7d572 100644 --- a/tests/app/notifications/rest/test_callbacks.py +++ b/tests/app/notifications/rest/test_callbacks.py @@ -634,39 +634,6 @@ def test_ses_callback_should_set_status_to_permanent_failure(notify_api, assert stats.emails_failed == 1 -def test_should_handle_invite_email_callbacks(notify_api, notify_db, notify_db_session): - with notify_api.test_request_context(): - with notify_api.test_client() as client: - notify_api.config['INVITATION_EMAIL_FROM'] = 'test-invite' - notify_api.config['NOTIFY_EMAIL_DOMAIN'] = 'test-domain.com' - - response = client.post( - path='/notifications/email/ses', - data=ses_invite_callback(), - headers=[('Content-Type', 'text/plain; charset=UTF-8')] - ) - json_resp = json.loads(response.get_data(as_text=True)) - assert response.status_code == 200 - assert json_resp['result'] == 'success' - assert json_resp['message'] == 'SES callback succeeded' - - -def test_should_handle_validation_code_callbacks(notify_api, notify_db, notify_db_session): - with notify_api.test_request_context(): - with notify_api.test_client() as client: - notify_api.config['VERIFY_CODE_FROM_EMAIL_ADDRESS'] = 'valid-code@test.com' - - response = client.post( - path='/notifications/email/ses', - data=ses_validation_code_callback(), - headers=[('Content-Type', 'text/plain; charset=UTF-8')] - ) - json_resp = json.loads(response.get_data(as_text=True)) - assert response.status_code == 200 - assert json_resp['result'] == 'success' - assert json_resp['message'] == 'SES callback succeeded' - - def test_process_mmg_response_records_statsd(notify_api, sample_notification, mocker): with notify_api.test_client() as client: mocker.patch('app.statsd_client.incr') @@ -698,14 +665,6 @@ def test_firetext_callback_should_record_statsd(notify_api, notify_db, notify_db app.statsd_client.incr.assert_any_call("callback.firetext.delivered") -def ses_validation_code_callback(): - return b'{\n "Type" : "Notification",\n "MessageId" : "ref",\n "TopicArn" : "arn:aws:sns:eu-west-1:123456789012:testing",\n "Message" : "{\\"notificationType\\":\\"Delivery\\",\\"mail\\":{\\"timestamp\\":\\"2016-03-14T12:35:25.909Z\\",\\"source\\":\\"valid-code@test.com\\",\\"sourceArn\\":\\"arn:aws:ses:eu-west-1:123456789012:identity/testing-notify\\",\\"sendingAccountId\\":\\"123456789012\\",\\"messageId\\":\\"ref\\",\\"destination\\":[\\"testing@digital.cabinet-office.gov.uk\\"]},\\"delivery\\":{\\"timestamp\\":\\"2016-03-14T12:35:26.567Z\\",\\"processingTimeMillis\\":658,\\"recipients\\":[\\"testing@digital.cabinet-office.gov.u\\"],\\"smtpResponse\\":\\"250 2.0.0 OK 1457958926 uo5si26480932wjc.221 - gsmtp\\",\\"reportingMTA\\":\\"a6-238.smtp-out.eu-west-1.amazonses.com\\"}}",\n "Timestamp" : "2016-03-14T12:35:26.665Z",\n "SignatureVersion" : "1",\n "Signature" : "X8d7eTAOZ6wlnrdVVPYanrAlsX0SMPfOzhoTEBnQqYkrNWTqQY91C0f3bxtPdUhUtOowyPAOkTQ4KnZuzphfhVb2p1MyVYMxNKcBFB05/qaCX99+92fjw4x9LeUOwyGwMv5F0Vkfi5qZCcEw69uVrhYLVSTFTrzi/yCtru+yFULMQ6UhbY09GwiP6hjxZMVr8aROQy5lLHglqQzOuSZ4KeD85JjifHdKzlx8jjQ+uj+FLzHXPMAPmPU1JK9kpoHZ1oPshAFgPDpphJe+HwcJ8ezmk+3AEUr3wWli3xF+49y8Z2anASSVp6YI2YP95UT8Rlh3qT3T+V9V8rbSVislxA==",\n "SigningCertURL" : "https://sns.eu-west-1.amazonaws.com/SimpleNotificationService-bb750dd426d95ee9390147a5624348ee.pem",\n "UnsubscribeURL" : "https://sns.eu-west-1.amazonaws.com/?Action=Unsubscribe&SubscriptionArn=arn:aws:sns:eu-west-1:302763885840:preview-emails:d6aad3ef-83d6-4cf3-a470-54e2e75916da"\n}' # noqa - - -def ses_invite_callback(): - return b'{\n "Type" : "Notification",\n "MessageId" : "ref",\n "TopicArn" : "arn:aws:sns:eu-west-1:123456789012:testing",\n "Message" : "{\\"notificationType\\":\\"Delivery\\",\\"mail\\":{\\"timestamp\\":\\"2016-03-14T12:35:25.909Z\\",\\"source\\":\\"test-invite@test-domain.com\\",\\"sourceArn\\":\\"arn:aws:ses:eu-west-1:123456789012:identity/testing-notify\\",\\"sendingAccountId\\":\\"123456789012\\",\\"messageId\\":\\"ref\\",\\"destination\\":[\\"testing@digital.cabinet-office.gov.uk\\"]},\\"delivery\\":{\\"timestamp\\":\\"2016-03-14T12:35:26.567Z\\",\\"processingTimeMillis\\":658,\\"recipients\\":[\\"testing@digital.cabinet-office.gov.u\\"],\\"smtpResponse\\":\\"250 2.0.0 OK 1457958926 uo5si26480932wjc.221 - gsmtp\\",\\"reportingMTA\\":\\"a6-238.smtp-out.eu-west-1.amazonses.com\\"}}",\n "Timestamp" : "2016-03-14T12:35:26.665Z",\n "SignatureVersion" : "1",\n "Signature" : "X8d7eTAOZ6wlnrdVVPYanrAlsX0SMPfOzhoTEBnQqYkrNWTqQY91C0f3bxtPdUhUtOowyPAOkTQ4KnZuzphfhVb2p1MyVYMxNKcBFB05/qaCX99+92fjw4x9LeUOwyGwMv5F0Vkfi5qZCcEw69uVrhYLVSTFTrzi/yCtru+yFULMQ6UhbY09GwiP6hjxZMVr8aROQy5lLHglqQzOuSZ4KeD85JjifHdKzlx8jjQ+uj+FLzHXPMAPmPU1JK9kpoHZ1oPshAFgPDpphJe+HwcJ8ezmk+3AEUr3wWli3xF+49y8Z2anASSVp6YI2YP95UT8Rlh3qT3T+V9V8rbSVislxA==",\n "SigningCertURL" : "https://sns.eu-west-1.amazonaws.com/SimpleNotificationService-bb750dd426d95ee9390147a5624348ee.pem",\n "UnsubscribeURL" : "https://sns.eu-west-1.amazonaws.com/?Action=Unsubscribe&SubscriptionArn=arn:aws:sns:eu-west-1:302763885840:preview-emails:d6aad3ef-83d6-4cf3-a470-54e2e75916da"\n}' # noqa - - def ses_notification_callback(ref='ref'): return str.encode( '{\n "Type" : "Notification",\n "MessageId" : "%(ref)s",\n "TopicArn" : "arn:aws:sns:eu-west-1:123456789012:testing",\n "Message" : "{\\"notificationType\\":\\"Delivery\\",\\"mail\\":{\\"timestamp\\":\\"2016-03-14T12:35:25.909Z\\",\\"source\\":\\"test@test-domain.com\\",\\"sourceArn\\":\\"arn:aws:ses:eu-west-1:123456789012:identity/testing-notify\\",\\"sendingAccountId\\":\\"123456789012\\",\\"messageId\\":\\"%(ref)s\\",\\"destination\\":[\\"testing@digital.cabinet-office.gov.uk\\"]},\\"delivery\\":{\\"timestamp\\":\\"2016-03-14T12:35:26.567Z\\",\\"processingTimeMillis\\":658,\\"recipients\\":[\\"testing@digital.cabinet-office.gov.uk\\"],\\"smtpResponse\\":\\"250 2.0.0 OK 1457958926 uo5si26480932wjc.221 - gsmtp\\",\\"reportingMTA\\":\\"a6-238.smtp-out.eu-west-1.amazonses.com\\"}}",\n "Timestamp" : "2016-03-14T12:35:26.665Z",\n "SignatureVersion" : "1",\n "Signature" : "X8d7eTAOZ6wlnrdVVPYanrAlsX0SMPfOzhoTEBnQqYkrNWTqQY91C0f3bxtPdUhUtOowyPAOkTQ4KnZuzphfhVb2p1MyVYMxNKcBFB05/qaCX99+92fjw4x9LeUOwyGwMv5F0Vkfi5qZCcEw69uVrhYLVSTFTrzi/yCtru+yFULMQ6UhbY09GwiP6hjxZMVr8aROQy5lLHglqQzOuSZ4KeD85JjifHdKzlx8jjQ+uj+FLzHXPMAPmPU1JK9kpoHZ1oPshAFgPDpphJe+HwcJ8ezmk+3AEUr3wWli3xF+49y8Z2anASSVp6YI2YP95UT8Rlh3qT3T+V9V8rbSVislxA==",\n "SigningCertURL" : "https://sns.eu-west-1.amazonaws.com/SimpleNotificationService-bb750dd426d95ee9390147a5624348ee.pem",\n "UnsubscribeURL" : "https://sns.eu-west-1.amazonaws.com/?Action=Unsubscribe&SubscriptionArn=arn:aws:sns:eu-west-1:302763885840:preview-emails:d6aad3ef-83d6-4cf3-a470-54e2e75916da"\n}' % {'ref': ref} # noqa From 00cac0f88b98fa7834bbd0bfb3faabcb9ecdd4f2 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Wed, 31 Aug 2016 15:02:27 +0100 Subject: [PATCH 13/15] add junit xml reporting to pytest also add file location to gitignore preemptively --- .gitignore | 2 +- scripts/run_tests.sh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.gitignore b/.gitignore index 8be1355da..3350c3437 100644 --- a/.gitignore +++ b/.gitignore @@ -42,8 +42,8 @@ htmlcov/ .coverage .coverage.* .cache -nosetests.xml coverage.xml +test_results.xml *,cover # Translations diff --git a/scripts/run_tests.sh b/scripts/run_tests.sh index 0e185da2f..e6c1bb886 100755 --- a/scripts/run_tests.sh +++ b/scripts/run_tests.sh @@ -30,5 +30,5 @@ fi pep8 . display_result $? 1 "Code style check" -py.test --cov=app --cov-report=term-missing tests/ +py.test --cov=app --cov-report=term-missing tests/ --junitxml=test_results.xml display_result $? 2 "Unit tests" From ab5ff64dd0c7458a8bd8ca529d3ea45b9948165b Mon Sep 17 00:00:00 2001 From: Martyn Inglis Date: Wed, 31 Aug 2016 16:24:18 +0100 Subject: [PATCH 14/15] Indexes to improves stats for service query --- migrations/versions/0050_index_for_stats.py | 31 +++++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 migrations/versions/0050_index_for_stats.py diff --git a/migrations/versions/0050_index_for_stats.py b/migrations/versions/0050_index_for_stats.py new file mode 100644 index 000000000..ceab66aee --- /dev/null +++ b/migrations/versions/0050_index_for_stats.py @@ -0,0 +1,31 @@ +"""empty message + +Revision ID: 0050_index_for_stats +Revises: 0048_job_scheduled_time +Create Date: 2016-08-24 13:21:51.744526 + +""" + +# revision identifiers, used by Alembic. +revision = '0050_index_for_stats' +down_revision = '0048_job_scheduled_time' + +from alembic import op +import sqlalchemy as sa + + +def upgrade(): + op.create_index( + 'ix_notifications_service_id_created_at', + 'notifications', + ['service_id', sa.text("date(created_at)")] + ) + op.create_index( + 'ix_notification_history_service_id_created_at', + 'notification_history', + ['service_id', sa.text("date(created_at)")] + ) + +def downgrade(): + op.drop_index(op.f('ix_notifications_service_id_created_at'), table_name='notifications') + op.drop_index(op.f('ix_notification_history_service_id_created_at'), table_name='notification_history') From 3d41fa963450825094277e3dc001159af4f390e9 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Thu, 1 Sep 2016 10:23:37 +0100 Subject: [PATCH 15/15] Update mmg status code 2 to be a permanent failure. --- app/clients/sms/mmg.py | 4 ++-- tests/app/notifications/rest/test_callbacks.py | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/app/clients/sms/mmg.py b/app/clients/sms/mmg.py index 0615c7a1c..066c1f259 100644 --- a/app/clients/sms/mmg.py +++ b/app/clients/sms/mmg.py @@ -6,10 +6,10 @@ from app.clients.sms import (SmsClient, SmsClientException) mmg_response_map = { '2': { - "message": ' Temporary failure', + "message": ' Permanent failure', "notification_statistics_status": STATISTICS_FAILURE, "success": False, - "notification_status": 'temporary-failure' + "notification_status": 'permanent-failure' }, '3': { "message": 'Delivered', diff --git a/tests/app/notifications/rest/test_callbacks.py b/tests/app/notifications/rest/test_callbacks.py index cc4fa4fbd..7364145d2 100644 --- a/tests/app/notifications/rest/test_callbacks.py +++ b/tests/app/notifications/rest/test_callbacks.py @@ -297,8 +297,8 @@ def test_process_mmg_response_status_5_updates_notification_with_permanently_fai assert get_notification_by_id(sample_notification.id).status == 'permanent-failure' -def test_process_mmg_response_status_2_updates_notification_with_temporary_failed(notify_api, - sample_notification): +def test_process_mmg_response_status_2_updates_notification_with_permanently_failed(notify_api, + sample_notification): with notify_api.test_client() as client: data = json.dumps({"reference": "mmg_reference", "CID": str(sample_notification.id), @@ -312,7 +312,7 @@ def test_process_mmg_response_status_2_updates_notification_with_temporary_faile json_data = json.loads(response.data) assert json_data['result'] == 'success' assert json_data['message'] == 'MMG callback succeeded. reference {} updated'.format(sample_notification.id) - assert get_notification_by_id(sample_notification.id).status == 'temporary-failure' + assert get_notification_by_id(sample_notification.id).status == 'permanent-failure' def test_process_mmg_response_status_4_updates_notification_with_temporary_failed(notify_api,