diff --git a/app/aws/s3.py b/app/aws/s3.py index 927eb0ad1..a64be797c 100644 --- a/app/aws/s3.py +++ b/app/aws/s3.py @@ -5,7 +5,10 @@ from flask import current_app import pytz from boto3 import client, resource +from notifications_utils.s3 import s3upload as utils_s3upload + FILE_LOCATION_STRUCTURE = 'service-{}-notify/{}.csv' +LETTERS_PDF_FILE_LOCATION_STRUCTURE = '{folder}/NOTIFY.{reference}.{duplex}.{letter_class}.{colour}.{crown}.{date}.pdf' def get_s3_file(bucket_name, file_location): @@ -76,3 +79,23 @@ def remove_transformed_dvla_file(job_id): file_location = '{}-dvla-job.text'.format(job_id) obj = get_s3_object(bucket_name, file_location) return obj.delete() + + +def upload_letters_pdf(reference, crown, filedata): + now = datetime.utcnow() + upload_file_name = LETTERS_PDF_FILE_LOCATION_STRUCTURE.format( + folder=now.date().isoformat(), + reference=reference, + duplex="D", + letter_class="2", + colour="C", + crown="C" if crown else "N", + date=now.strftime('%Y%m%d%H%M%S') + ).upper() + + utils_s3upload( + filedata=filedata, + region=current_app.config['AWS_REGION'], + bucket_name=current_app.config['LETTERS_PDF_BUCKET_NAME'], + file_location=upload_file_name + ) diff --git a/app/celery/callback_tasks.py b/app/celery/process_ses_receipts_tasks.py similarity index 100% rename from app/celery/callback_tasks.py rename to app/celery/process_ses_receipts_tasks.py diff --git a/app/celery/research_mode_tasks.py b/app/celery/research_mode_tasks.py index b37062826..509ee1681 100644 --- a/app/celery/research_mode_tasks.py +++ b/app/celery/research_mode_tasks.py @@ -5,7 +5,7 @@ from requests import request, RequestException, HTTPError from app.models import SMS_TYPE from app.config import QueueNames -from app.celery.callback_tasks import process_ses_results +from app.celery.process_ses_receipts_tasks import process_ses_results temp_fail = "7700900003" perm_fail = "7700900002" diff --git a/app/celery/service_callback_tasks.py b/app/celery/service_callback_tasks.py new file mode 100644 index 000000000..3bf033a80 --- /dev/null +++ b/app/celery/service_callback_tasks.py @@ -0,0 +1,71 @@ +import json +from app import ( + DATETIME_FORMAT, + notify_celery, +) +from app.dao.notifications_dao import ( + get_notification_by_id, +) + +from app.statsd_decorators import statsd +from app.dao.service_callback_api_dao import get_service_callback_api_for_service +from requests import ( + HTTPError, + request, + RequestException +) +from flask import current_app +from app.config import QueueNames + + +@notify_celery.task(bind=True, name="send-delivery-status", max_retries=5, default_retry_delay=300) +@statsd(namespace="tasks") +def send_delivery_status_to_service(self, notification_id): + # TODO: do we need to do rate limit this? + notification = get_notification_by_id(notification_id) + service_callback_api = get_service_callback_api_for_service(service_id=notification.service_id) + if not service_callback_api: + # No delivery receipt API info set + return + + data = { + "id": str(notification_id), + "reference": str(notification.client_reference), + "to": notification.to, + "status": notification.status, + "created_at": notification.created_at.strftime(DATETIME_FORMAT), # the time GOV.UK email sent the request + "updated_at": notification.updated_at.strftime(DATETIME_FORMAT), # the last time the status was updated + "sent_at": notification.sent_at.strftime(DATETIME_FORMAT), # the time the email was sent + "notification_type": notification.notification_type + } + + try: + response = request( + method="POST", + url=service_callback_api.url, + data=json.dumps(data), + headers={ + 'Content-Type': 'application/json', + 'Authorization': 'Bearer {}'.format(service_callback_api.bearer_token) + }, + timeout=60 + ) + current_app.logger.info('send_delivery_status_to_service sending {} to {}, response {}'.format( + notification_id, + service_callback_api.url, + response.status_code + )) + response.raise_for_status() + except RequestException as e: + current_app.logger.warning( + "send_delivery_status_to_service request failed for service_id: {} and url: {}. exc: {}".format( + notification_id, + service_callback_api.url, + e + ) + ) + if not isinstance(e, HTTPError) or e.response.status_code >= 500: + try: + self.retry(queue=QueueNames.RETRY) + except self.MaxRetriesExceededError: + current_app.logger.exception('Retry: send_delivery_status_to_service has retried the max num of times') diff --git a/app/celery/tasks.py b/app/celery/tasks.py index 250f786eb..53d714213 100644 --- a/app/celery/tasks.py +++ b/app/celery/tasks.py @@ -29,6 +29,7 @@ from app import ( ) from app.aws import s3 from app.celery import provider_tasks +from app.celery.service_callback_tasks import send_delivery_status_to_service from app.config import QueueNames from app.dao.inbound_sms_dao import dao_get_inbound_sms_by_id from app.dao.jobs_dao import ( @@ -42,12 +43,14 @@ from app.dao.notifications_dao import ( get_notification_by_id, dao_update_notifications_for_job_to_sent_to_dvla, dao_update_notifications_by_reference, - dao_get_last_notification_added_for_job_id + dao_get_last_notification_added_for_job_id, + dao_get_notifications_by_references ) from app.dao.provider_details_dao import get_current_provider from app.dao.service_inbound_api_dao import get_service_inbound_api_for_service from app.dao.services_dao import dao_fetch_service_by_id, fetch_todays_total_message_count from app.dao.templates_dao import dao_get_template_by_id +from app.dao.service_callback_api_dao import get_service_callback_api_for_service from app.models import ( DVLA_RESPONSE_STATUS_SENT, EMAIL_TYPE, @@ -391,6 +394,12 @@ def update_letter_notifications_to_error(self, notification_references): ) current_app.logger.info("Updated {} letter notifications to technical-failure".format(updated_count)) + notifications = dao_get_notifications_by_references(references=notification_references) + # queue callback task only if the service_callback_api exists + service_callback_api = get_service_callback_api_for_service(service_id=notifications[0].service_id) + if service_callback_api: + for notification in notifications: + send_delivery_status_to_service.apply_async([str(notification.id)], queue=QueueNames.NOTIFY) def create_dvla_file_contents_for_job(job_id): @@ -455,7 +464,7 @@ def update_letter_notifications_statuses(self, filename): for update in notification_updates: status = NOTIFICATION_DELIVERED if update.status == DVLA_RESPONSE_STATUS_SENT \ else NOTIFICATION_TECHNICAL_FAILURE - notification = dao_update_notifications_by_reference( + updated_count = dao_update_notifications_by_reference( references=[update.reference], update_dict={"status": status, "billable_units": update.page_count, @@ -463,7 +472,7 @@ def update_letter_notifications_statuses(self, filename): } ) - if not notification: + if not updated_count: msg = "Update letter notification file {filename} failed: notification either not found " \ "or already updated from delivered. Status {status} for notification reference {reference}".format( filename=filename, status=status, reference=update.reference) @@ -472,6 +481,12 @@ def update_letter_notifications_statuses(self, filename): current_app.logger.info( 'DVLA file: {filename}, notification updated to {status}: {reference}'.format( filename=filename, status=status, reference=str(update.reference))) + notifications = dao_get_notifications_by_references(references=[update.reference]) + # queue callback task only if the service_callback_api exists + service_callback_api = get_service_callback_api_for_service(service_id=notifications[0].service_id) + if service_callback_api: + for notification in notifications: + send_delivery_status_to_service.apply_async([str(notification.id)], queue=QueueNames.NOTIFY) def process_updates_from_file(response_file): diff --git a/app/config.py b/app/config.py index 3199fbe93..d553041e3 100644 --- a/app/config.py +++ b/app/config.py @@ -314,6 +314,7 @@ class Development(Config): SQLALCHEMY_ECHO = False NOTIFY_EMAIL_DOMAIN = 'notify.tools' CSV_UPLOAD_BUCKET_NAME = 'development-notifications-csv-upload' + LETTERS_PDF_BUCKET_NAME = 'development-letters-pdf' DVLA_RESPONSE_BUCKET_NAME = 'notify.tools-ftp' NOTIFY_ENVIRONMENT = 'development' NOTIFICATION_QUEUE_PREFIX = 'development' @@ -335,6 +336,7 @@ class Test(Config): DEBUG = True TESTING = True CSV_UPLOAD_BUCKET_NAME = 'test-notifications-csv-upload' + LETTERS_PDF_BUCKET_NAME = 'test-letters-pdf' DVLA_RESPONSE_BUCKET_NAME = 'test.notify.com-ftp' STATSD_ENABLED = True STATSD_HOST = "localhost" @@ -373,6 +375,7 @@ class Preview(Config): NOTIFY_EMAIL_DOMAIN = 'notify.works' NOTIFY_ENVIRONMENT = 'preview' CSV_UPLOAD_BUCKET_NAME = 'preview-notifications-csv-upload' + LETTERS_PDF_BUCKET_NAME = 'preview-letters-pdf' DVLA_RESPONSE_BUCKET_NAME = 'notify.works-ftp' FROM_NUMBER = 'preview' API_RATE_LIMIT_ENABLED = True @@ -383,23 +386,25 @@ class Staging(Config): NOTIFY_EMAIL_DOMAIN = 'staging-notify.works' NOTIFY_ENVIRONMENT = 'staging' CSV_UPLOAD_BUCKET_NAME = 'staging-notify-csv-upload' + LETTERS_PDF_BUCKET_NAME = 'staging-letters-pdf' DVLA_RESPONSE_BUCKET_NAME = 'staging-notify.works-ftp' STATSD_ENABLED = True FROM_NUMBER = 'stage' API_RATE_LIMIT_ENABLED = True CHECK_PROXY_HEADER = True + REDIS_ENABLED = True API_KEY_LIMITS = { KEY_TYPE_TEAM: { - "limit": 21000, + "limit": 24000, "interval": 60 }, KEY_TYPE_NORMAL: { - "limit": 21000, + "limit": 24000, "interval": 60 }, KEY_TYPE_TEST: { - "limit": 21000, + "limit": 24000, "interval": 60 } } @@ -409,6 +414,7 @@ class Live(Config): NOTIFY_EMAIL_DOMAIN = 'notifications.service.gov.uk' NOTIFY_ENVIRONMENT = 'live' CSV_UPLOAD_BUCKET_NAME = 'live-notifications-csv-upload' + LETTERS_PDF_BUCKET_NAME = 'live-letters-pdf' DVLA_RESPONSE_BUCKET_NAME = 'notifications.service.gov.uk-ftp' STATSD_ENABLED = True FROM_NUMBER = 'GOVUK' @@ -416,7 +422,7 @@ class Live(Config): FUNCTIONAL_TEST_PROVIDER_SMS_TEMPLATE_ID = 'ba9e1789-a804-40b8-871f-cc60d4c1286f' PERFORMANCE_PLATFORM_ENABLED = True API_RATE_LIMIT_ENABLED = True - CHECK_PROXY_HEADER = False + CHECK_PROXY_HEADER = True class CloudFoundryConfig(Config): @@ -428,6 +434,7 @@ class Sandbox(CloudFoundryConfig): NOTIFY_EMAIL_DOMAIN = 'notify.works' NOTIFY_ENVIRONMENT = 'sandbox' CSV_UPLOAD_BUCKET_NAME = 'cf-sandbox-notifications-csv-upload' + LETTERS_PDF_BUCKET_NAME = 'cf-sandbox-letters-pdf' DVLA_RESPONSE_BUCKET_NAME = 'notify.works-ftp' FROM_NUMBER = 'sandbox' REDIS_ENABLED = False diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index 2eb713699..1f7b33167 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -459,6 +459,13 @@ def dao_get_notifications_by_to_field(service_id, search_term, statuses=None): return results +@statsd(namespace="dao") +def dao_get_notifications_by_references(references): + return Notification.query.filter( + Notification.reference.in_(references) + ).all() + + @statsd(namespace="dao") def dao_created_scheduled_notification(scheduled_notification): db.session.add(scheduled_notification) diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index 8753b2dc4..0b5f3e245 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -172,6 +172,7 @@ def dao_create_service(service, user, service_id=None, service_permissions=None) service.id = service_id or uuid.uuid4() # must be set now so version history model can use same id service.active = True service.research_mode = False + service.crown = service.organisation_type == 'central' for permission in service_permissions: service_permission = ServicePermission(service_id=service.id, permission=permission) diff --git a/app/models.py b/app/models.py index fb3a2610d..fdd00fa9a 100644 --- a/app/models.py +++ b/app/models.py @@ -178,6 +178,7 @@ INTERNATIONAL_SMS_TYPE = 'international_sms' INBOUND_SMS_TYPE = 'inbound_sms' SCHEDULE_NOTIFICATIONS = 'schedule_notifications' EMAIL_AUTH = 'email_auth' +LETTERS_AS_PDF = 'letters_as_pdf' SERVICE_PERMISSION_TYPES = [ EMAIL_TYPE, @@ -187,6 +188,7 @@ SERVICE_PERMISSION_TYPES = [ INBOUND_SMS_TYPE, SCHEDULE_NOTIFICATIONS, EMAIL_AUTH, + LETTERS_AS_PDF, ] @@ -247,6 +249,7 @@ class Service(db.Model, Versioned): db.String(255), nullable=True, ) + crown = db.Column(db.Boolean, index=False, nullable=False, default=True) association_proxy('permissions', 'service_permission_types') diff --git a/app/notifications/notifications_ses_callback.py b/app/notifications/notifications_ses_callback.py index b35704304..28fa96f2e 100644 --- a/app/notifications/notifications_ses_callback.py +++ b/app/notifications/notifications_ses_callback.py @@ -10,8 +10,11 @@ from app.clients.email.aws_ses import get_aws_responses from app.dao import ( notifications_dao ) +from app.dao.service_callback_api_dao import get_service_callback_api_for_service from app.celery.statistics_tasks import create_outcome_notification_statistic_tasks from app.notifications.process_client_response import validate_callback_data +from app.celery.service_callback_tasks import send_delivery_status_to_service +from app.config import QueueNames def process_ses_response(ses_request): @@ -75,7 +78,7 @@ def process_ses_response(ses_request): ) create_outcome_notification_statistic_tasks(notification) - + _check_and_queue_callback_task(notification.id, notification.service_id) return except KeyError: @@ -90,3 +93,10 @@ def process_ses_response(ses_request): def remove_emails_from_bounce(bounce_dict): for recip in bounce_dict['bouncedRecipients']: recip.pop('emailAddress') + + +def _check_and_queue_callback_task(notification_id, service_id): + # queue callback task only if the service_callback_api exists + service_callback_api = get_service_callback_api_for_service(service_id=service_id) + if service_callback_api: + send_delivery_status_to_service.apply_async([str(notification_id)], queue=QueueNames.NOTIFY) diff --git a/app/notifications/process_client_response.py b/app/notifications/process_client_response.py index 534408ce6..ed825766a 100644 --- a/app/notifications/process_client_response.py +++ b/app/notifications/process_client_response.py @@ -8,6 +8,9 @@ from app.dao import notifications_dao from app.clients.sms.firetext import get_firetext_responses from app.clients.sms.mmg import get_mmg_responses from app.celery.statistics_tasks import create_outcome_notification_statistic_tasks +from app.celery.service_callback_tasks import send_delivery_status_to_service +from app.config import QueueNames +from app.dao.service_callback_api_dao import get_service_callback_api_for_service sms_response_mapper = { @@ -82,6 +85,11 @@ def process_sms_client_response(status, reference, client_name): ) create_outcome_notification_statistic_tasks(notification) + # queue callback task only if the service_callback_api exists + service_callback_api = get_service_callback_api_for_service(service_id=notification.service_id) + + if service_callback_api: + send_delivery_status_to_service.apply_async([str(notification.id)], queue=QueueNames.NOTIFY) success = "{} callback succeeded. reference {} updated".format(client_name, reference) return success, errors diff --git a/app/notifications/process_notifications.py b/app/notifications/process_notifications.py index df46b0f02..1277d9921 100644 --- a/app/notifications/process_notifications.py +++ b/app/notifications/process_notifications.py @@ -13,6 +13,7 @@ from notifications_utils.recipients import ( from app import redis_store from app.celery import provider_tasks from app.config import QueueNames + from app.models import ( EMAIL_TYPE, KEY_TYPE_TEST, @@ -26,6 +27,8 @@ from app.dao.notifications_dao import ( dao_delete_notifications_and_history_by_id, dao_created_scheduled_notification ) + +from app.statsd_decorators import statsd from app.v2.errors import BadRequestError from app.utils import get_template_instance, cache_key_for_service_template_counter, convert_bst_to_utc @@ -43,6 +46,7 @@ def check_placeholders(template_object): raise BadRequestError(fields=[{'template': message}], message=message) +@statsd(namespace="performance-testing") def persist_notification( *, template_id, @@ -112,6 +116,7 @@ def persist_notification( return notification +@statsd(namespace="performance-testing") def send_notification_to_queue(notification, research_mode, queue=None): if research_mode or notification.key_type == KEY_TYPE_TEST: queue = QueueNames.RESEARCH_MODE diff --git a/app/notifications/validators.py b/app/notifications/validators.py index 344f05ebe..176241f1e 100644 --- a/app/notifications/validators.py +++ b/app/notifications/validators.py @@ -14,6 +14,7 @@ from app.models import ( KEY_TYPE_TEST, KEY_TYPE_TEAM, SCHEDULE_NOTIFICATIONS ) from app.service.utils import service_allowed_to_send_to +from app.statsd_decorators import statsd from app.v2.errors import TooManyRequestsError, BadRequestError, RateLimitError from app import redis_store from app.notifications.process_notifications import create_content_for_notification @@ -22,7 +23,7 @@ from app.dao.service_email_reply_to_dao import dao_get_reply_to_by_id def check_service_over_api_rate_limit(service, api_key): - if current_app.config['API_RATE_LIMIT_ENABLED']: + if current_app.config['API_RATE_LIMIT_ENABLED'] and current_app.config['REDIS_ENABLED']: cache_key = rate_limit_cache_key(service.id, api_key.key_type) rate_limit = current_app.config['API_KEY_LIMITS'][api_key.key_type]['limit'] interval = current_app.config['API_KEY_LIMITS'][api_key.key_type]['interval'] @@ -32,7 +33,7 @@ def check_service_over_api_rate_limit(service, api_key): def check_service_over_daily_message_limit(key_type, service): - if key_type != KEY_TYPE_TEST: + if key_type != KEY_TYPE_TEST and current_app.config['REDIS_ENABLED']: cache_key = daily_limit_cache_key(service.id) service_stats = redis_store.get(cache_key) if not service_stats: @@ -46,6 +47,7 @@ def check_service_over_daily_message_limit(key_type, service): raise TooManyRequestsError(service.message_limit) +@statsd(namespace="performance-testing") def check_rate_limiting(service, api_key): check_service_over_api_rate_limit(service, api_key) check_service_over_daily_message_limit(api_key.key_type, service) @@ -80,12 +82,14 @@ def service_has_permission(notify_type, permissions): return notify_type in [p.permission for p in permissions] +@statsd(namespace="performance-testing") def check_service_has_permission(notify_type, permissions): if not service_has_permission(notify_type, permissions): raise BadRequestError(message="Cannot send {}".format( get_public_notify_type_text(notify_type, plural=True))) +@statsd(namespace="performance-testing") def check_service_can_schedule_notification(permissions, scheduled_for): if scheduled_for: if not service_has_permission(SCHEDULE_NOTIFICATIONS, permissions): @@ -117,6 +121,7 @@ def check_sms_content_char_count(content_count): raise BadRequestError(message=message) +@statsd(namespace="performance-testing") def validate_template(template_id, personalisation, service, notification_type): try: template = templates_dao.dao_get_template_by_id_and_service_id( diff --git a/app/service/rest.py b/app/service/rest.py index efb7044a3..c4d0e2bb9 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -178,7 +178,11 @@ def update_service(service_id): service_going_live = fetched_service.restricted and not req_json.get('restricted', True) current_data = dict(service_schema.dump(fetched_service).data.items()) current_data.update(request.get_json()) + update_dict = service_schema.load(current_data).data + org_type = req_json.get('organisation_type', None) + if org_type: + update_dict.crown = org_type == 'central' dao_update_service(update_dict) # bridging code between frontend is deployed and data has not been migrated yet. Can only update current year diff --git a/app/v2/notifications/create_response.py b/app/v2/notifications/create_response.py index 7eecfb1b9..a2e44001c 100644 --- a/app/v2/notifications/create_response.py +++ b/app/v2/notifications/create_response.py @@ -1,3 +1,5 @@ +from app.statsd_decorators import statsd + def create_post_sms_response_from_notification(notification, content, from_number, url_root, scheduled_for): noti = __create_notification_response(notification, url_root, scheduled_for) @@ -8,6 +10,7 @@ def create_post_sms_response_from_notification(notification, content, from_numbe return noti +@statsd(namespace="performance-testing") def create_post_email_response_from_notification(notification, content, subject, email_from, url_root, scheduled_for): noti = __create_notification_response(notification, url_root, scheduled_for) noti['content'] = { diff --git a/app/v2/notifications/post_notifications.py b/app/v2/notifications/post_notifications.py index be35f30b4..08847f2cb 100644 --- a/app/v2/notifications/post_notifications.py +++ b/app/v2/notifications/post_notifications.py @@ -34,6 +34,7 @@ from app.notifications.validators import ( check_service_sms_sender_id ) from app.schema_validation import validate +from app.statsd_decorators import statsd from app.v2.errors import BadRequestError from app.v2.notifications import v2_notification_blueprint from app.v2.notifications.notification_schemas import ( @@ -49,6 +50,7 @@ from app.v2.notifications.create_response import ( @v2_notification_blueprint.route('/', methods=['POST']) +@statsd(namespace="performance-testing") def post_notification(notification_type): if notification_type == EMAIL_TYPE: form = validate(request.get_json(), post_email_request) @@ -118,6 +120,7 @@ def post_notification(notification_type): return jsonify(resp), 201 +@statsd(namespace="performance-testing") def process_sms_or_email_notification(*, form, notification_type, api_key, template, service, reply_to_text=None): form_send_to = form['email_address'] if notification_type == EMAIL_TYPE else form['phone_number'] @@ -187,6 +190,7 @@ def process_letter_notification(*, letter_data, api_key, template): return notification +@statsd(namespace="performance-testing") def get_reply_to_text(notification_type, form): reply_to = None if notification_type == EMAIL_TYPE: diff --git a/manifest-api-staging.yml b/manifest-api-staging.yml index 05b1d420d..868d2853b 100644 --- a/manifest-api-staging.yml +++ b/manifest-api-staging.yml @@ -1,7 +1,6 @@ --- inherit: manifest-api-base.yml -command: scripts/run_app_paas.sh gunicorn -c /home/vcap/app/gunicorn_config.py --error-logfile /home/vcap/logs/gunicorn_error.log -w 6 -b 0.0.0.0:$PORT application services: - notify-aws - notify-config diff --git a/migrations/versions/0148_add_letters_as_pdf_svc_perm.py b/migrations/versions/0148_add_letters_as_pdf_svc_perm.py new file mode 100644 index 000000000..94f56a2af --- /dev/null +++ b/migrations/versions/0148_add_letters_as_pdf_svc_perm.py @@ -0,0 +1,24 @@ +"""empty message + +Revision ID: 0148_add_letters_as_pdf_svc_perm +Revises: 0147_drop_mapping_tables +Create Date: 2017-12-01 13:33:18.581320 + +""" + +# revision identifiers, used by Alembic. +revision = '0148_add_letters_as_pdf_svc_perm' +down_revision = '0147_drop_mapping_tables' + +from alembic import op + + +def upgrade(): + op.get_bind() + op.execute("insert into service_permission_types values('letters_as_pdf')") + + +def downgrade(): + op.get_bind() + op.execute("delete from service_permissions where permission = 'letters_as_pdf'") + op.execute("delete from service_permission_types where name = 'letters_as_pdf'") diff --git a/migrations/versions/0149_add_crown_to_services.py b/migrations/versions/0149_add_crown_to_services.py new file mode 100644 index 000000000..bfbbf976c --- /dev/null +++ b/migrations/versions/0149_add_crown_to_services.py @@ -0,0 +1,50 @@ +""" + +Revision ID: 0149_add_crown_column_to_services +Revises: 0148_add_letters_as_pdf_svc_perm +Create Date: 2017-12-04 12:13:35.268712 + +""" +from alembic import op +import sqlalchemy as sa + + +revision = '0149_add_crown_to_services' +down_revision = '0148_add_letters_as_pdf_svc_perm' + + +def upgrade(): + op.add_column('services', sa.Column('crown', sa.Boolean(), nullable=True)) + op.execute(""" + update services set crown = True + where organisation_type = 'central' + """) + op.execute(""" + update services set crown = True + where organisation_type is null + """) + op.execute(""" + update services set crown = False + where crown is null + """) + op.alter_column('services', 'crown', nullable=False) + + op.add_column('services_history', sa.Column('crown', sa.Boolean(), nullable=True)) + op.execute(""" + update services_history set crown = True + where organisation_type = 'central' + """) + op.execute(""" + update services_history set crown = True + where organisation_type is null + """) + op.execute(""" + update services_history set crown = False + where crown is null + """) + op.alter_column('services_history', 'crown', nullable=False) + + +def downgrade(): + op.drop_column('services', 'crown') + op.drop_column('services_history', 'crown') diff --git a/requirements.txt b/requirements.txt index a14b97cee..9bf77ea99 100644 --- a/requirements.txt +++ b/requirements.txt @@ -23,7 +23,7 @@ statsd==3.2.1 notifications-python-client==4.6.0 # PaaS -awscli==1.14.1 +awscli==1.14.2 awscli-cwlogs>=1.4,<1.5 git+https://github.com/alphagov/notifications-utils.git@23.2.1#egg=notifications-utils==23.2.1 diff --git a/scripts/run_app_paas.sh b/scripts/run_app_paas.sh index 2813f851f..78610c1bd 100755 --- a/scripts/run_app_paas.sh +++ b/scripts/run_app_paas.sh @@ -16,6 +16,10 @@ function check_params { } function configure_aws_logs { + # create files so that aws logs agent doesn't complain + touch /home/vcap/logs/gunicorn_error.log + touch /home/vcap/logs/app.log.json + aws configure set plugins.cwlogs cwlogs export AWS_ACCESS_KEY_ID=$(echo ${VCAP_SERVICES} | jq -r '.["user-provided"][]|select(.name=="notify-aws")|.credentials.aws_access_key_id') diff --git a/tests/app/aws/test_s3.py b/tests/app/aws/test_s3.py index d6e4c1b75..e610560db 100644 --- a/tests/app/aws/test_s3.py +++ b/tests/app/aws/test_s3.py @@ -1,5 +1,6 @@ from unittest.mock import call from datetime import datetime, timedelta +import pytest from flask import current_app @@ -9,7 +10,8 @@ from app.aws.s3 import ( get_s3_bucket_objects, get_s3_file, filter_s3_bucket_objects_within_date_range, - remove_transformed_dvla_file + remove_transformed_dvla_file, + upload_letters_pdf ) from tests.app.conftest import datetime_in_past @@ -139,3 +141,21 @@ def test_get_s3_bucket_objects_does_not_return_outside_of_date_range(notify_api, filtered_items = filter_s3_bucket_objects_within_date_range(s3_objects_stub) assert len(filtered_items) == 0 + + +@pytest.mark.parametrize('crown_flag,expected_crown_text', [ + (True, 'C'), + (False, 'N'), +]) +@freeze_time("2017-12-04 15:00:00") +def test_upload_letters_pdf_calls_utils_s3upload_with_correct_args( + notify_api, mocker, crown_flag, expected_crown_text): + s3_upload_mock = mocker.patch('app.aws.s3.utils_s3upload') + upload_letters_pdf(reference='foo', crown=crown_flag, filedata='some_data') + + s3_upload_mock.assert_called_with( + filedata='some_data', + region='eu-west-1', + bucket_name='test-letters-pdf', + file_location='2017-12-04/NOTIFY.FOO.D.2.C.{}.20171204150000.PDF'.format(expected_crown_text) + ) diff --git a/tests/app/celery/test_ftp_update_tasks.py b/tests/app/celery/test_ftp_update_tasks.py index d9b1b22b9..2c6bc4393 100644 --- a/tests/app/celery/test_ftp_update_tasks.py +++ b/tests/app/celery/test_ftp_update_tasks.py @@ -21,8 +21,9 @@ from app.celery.tasks import ( update_letter_notifications_to_sent_to_dvla ) -from tests.app.db import create_notification +from tests.app.db import create_notification, create_service_callback_api from tests.conftest import set_config +from unittest.mock import call def test_update_job_to_sent_to_dvla(sample_letter_template, sample_letter_job): @@ -97,11 +98,15 @@ def test_update_letter_notifications_statuses_persisted(notify_api, mocker, samp billable_units=0) failed_letter = create_notification(sample_letter_template, reference='ref-bar', status=NOTIFICATION_SENDING, billable_units=0) - + create_service_callback_api(service=sample_letter_template.service, url="https://original_url.com") valid_file = '{}|Sent|1|Unsorted\n{}|Failed|2|Sorted'.format( sent_letter.reference, failed_letter.reference) mocker.patch('app.celery.tasks.s3.get_s3_file', return_value=valid_file) + send_mock = mocker.patch( + 'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async' + ) + update_letter_notifications_statuses(filename='foo.txt') assert sent_letter.status == NOTIFICATION_DELIVERED @@ -111,6 +116,25 @@ def test_update_letter_notifications_statuses_persisted(notify_api, mocker, samp assert failed_letter.billable_units == 2 assert failed_letter.updated_at + calls = [call([str(failed_letter.id)], queue="notify-internal-tasks"), + call([str(sent_letter.id)], queue="notify-internal-tasks")] + send_mock.assert_has_calls(calls, any_order=True) + + +def test_update_letter_notifications_does_not_call_send_callback_if_no_db_entry(notify_api, mocker, + sample_letter_template): + sent_letter = create_notification(sample_letter_template, reference='ref-foo', status=NOTIFICATION_SENDING, + billable_units=0) + valid_file = '{}|Sent|1|Unsorted\n'.format(sent_letter.reference) + mocker.patch('app.celery.tasks.s3.get_s3_file', return_value=valid_file) + + send_mock = mocker.patch( + 'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async' + ) + + update_letter_notifications_statuses(filename='foo.txt') + send_mock.assert_not_called() + def test_update_letter_notifications_to_sent_to_dvla_updates_based_on_notification_references( client, @@ -132,11 +156,15 @@ def test_update_letter_notifications_to_sent_to_dvla_updates_based_on_notificati def test_update_letter_notifications_to_error_updates_based_on_notification_references( client, - sample_letter_template + sample_letter_template, + mocker ): + send_mock = mocker.patch( + 'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async' + ) first = create_notification(sample_letter_template, reference='first ref') second = create_notification(sample_letter_template, reference='second ref') - + create_service_callback_api(service=sample_letter_template.service, url="https://original_url.com") dt = datetime.utcnow() with freeze_time(dt): update_letter_notifications_to_error([first.reference]) @@ -146,3 +174,4 @@ def test_update_letter_notifications_to_error_updates_based_on_notification_refe assert first.sent_at is None assert first.updated_at == dt assert second.status == NOTIFICATION_CREATED + assert send_mock.called diff --git a/tests/app/celery/test_callback_tasks.py b/tests/app/celery/test_process_ses_receipts_tasks.py similarity index 91% rename from tests/app/celery/test_callback_tasks.py rename to tests/app/celery/test_process_ses_receipts_tasks.py index 4b5ce4547..ddca32e0e 100644 --- a/tests/app/celery/test_callback_tasks.py +++ b/tests/app/celery/test_process_ses_receipts_tasks.py @@ -1,7 +1,7 @@ import json from datetime import datetime -from app.celery.callback_tasks import process_ses_results +from app.celery.process_ses_receipts_tasks import process_ses_results from tests.app.db import create_notification @@ -18,7 +18,7 @@ def test_process_ses_results(sample_email_template): def test_process_ses_results_does_not_retry_if_errors(notify_db, mocker): - mocked = mocker.patch('app.celery.callback_tasks.process_ses_results.retry') + mocked = mocker.patch('app.celery.process_ses_receipts_tasks.process_ses_results.retry') response = json.loads(ses_notification_callback()) process_ses_results(response=response) assert mocked.call_count == 0 @@ -26,7 +26,7 @@ def test_process_ses_results_does_not_retry_if_errors(notify_db, mocker): def test_process_ses_results_retry_called(notify_db, mocker): mocker.patch("app.dao.notifications_dao.update_notification_status_by_reference", side_effect=Exception("EXPECTED")) - mocked = mocker.patch('app.celery.callback_tasks.process_ses_results.retry') + mocked = mocker.patch('app.celery.process_ses_receipts_tasks.process_ses_results.retry') response = json.loads(ses_notification_callback()) process_ses_results(response=response) assert mocked.call_count != 0 diff --git a/tests/app/celery/test_service_callback_tasks.py b/tests/app/celery/test_service_callback_tasks.py new file mode 100644 index 000000000..f09333d2d --- /dev/null +++ b/tests/app/celery/test_service_callback_tasks.py @@ -0,0 +1,185 @@ +import json +from datetime import datetime + +import pytest +import requests_mock + +from requests import RequestException + +from app import (DATETIME_FORMAT) + +from tests.app.conftest import ( + sample_service as create_sample_service, + sample_template as create_sample_template, +) +from tests.app.db import ( + create_notification, + create_user, + create_service_callback_api +) +from app.celery.service_callback_tasks import send_delivery_status_to_service + + +@pytest.mark.parametrize("notification_type", + ["email", "letter", "sms"]) +def test_send_delivery_status_to_service_post_https_request_to_service(notify_db, + notify_db_session, + notification_type): + user = create_user() + service = create_sample_service(notify_db, notify_db_session, user=user, restricted=True) + + callback_api = create_service_callback_api(service=service, url="https://some.service.gov.uk/", + bearer_token="something_unique") + template = create_sample_template( + notify_db, notify_db_session, service=service, template_type=notification_type, subject_line='Hello' + ) + + datestr = datetime(2017, 6, 20) + + notification = create_notification(template=template, + created_at=datestr, + updated_at=datestr, + sent_at=datestr, + status='sent' + ) + + with requests_mock.Mocker() as request_mock: + request_mock.post(callback_api.url, + json={}, + status_code=200) + send_delivery_status_to_service(notification.id) + + mock_data = { + "id": str(notification.id), + "reference": str(notification.client_reference), + "to": notification.to, + "status": notification.status, + "created_at": datestr.strftime(DATETIME_FORMAT), # the time GOV.UK email sent the request + "updated_at": datestr.strftime(DATETIME_FORMAT), # the last time the status was updated + "sent_at": datestr.strftime(DATETIME_FORMAT), # the time the email was sent + "notification_type": notification_type + } + + assert request_mock.call_count == 1 + assert request_mock.request_history[0].url == callback_api.url + assert request_mock.request_history[0].method == 'POST' + assert request_mock.request_history[0].text == json.dumps(mock_data) + assert request_mock.request_history[0].headers["Content-type"] == "application/json" + assert request_mock.request_history[0].headers["Authorization"] == "Bearer {}".format(callback_api.bearer_token) + + +@pytest.mark.parametrize("notification_type", + ["email", "letter", "sms"]) +def test_send_delivery_status_to_service_does_not_sent_request_when_service_callback_api_does_not_exist( + notify_db, notify_db_session, mocker, notification_type): + service = create_sample_service(notify_db, notify_db_session, restricted=True) + + template = create_sample_template( + notify_db, notify_db_session, service=service, template_type=notification_type, subject_line='Hello' + ) + datestr = datetime(2017, 6, 20) + + notification = create_notification(template=template, + created_at=datestr, + updated_at=datestr, + sent_at=datestr, + status='sent' + ) + mocked = mocker.patch("requests.request") + send_delivery_status_to_service(notification.id) + + mocked.call_count == 0 + + +@pytest.mark.parametrize("notification_type", + ["email", "letter", "sms"]) +def test_send_delivery_status_to_service_retries_if_request_returns_500(notify_db, + notify_db_session, + mocker, + notification_type): + user = create_user() + service = create_sample_service(notify_db, notify_db_session, user=user, restricted=True) + + template = create_sample_template( + notify_db, notify_db_session, service=service, template_type=notification_type, subject_line='Hello' + ) + callback_api = create_service_callback_api(service=service, url="https://some.service.gov.uk/", + bearer_token="something_unique") + datestr = datetime(2017, 6, 20) + notification = create_notification(template=template, + created_at=datestr, + updated_at=datestr, + sent_at=datestr, + status='sent' + ) + mocked = mocker.patch('app.celery.service_callback_tasks.send_delivery_status_to_service.retry') + with requests_mock.Mocker() as request_mock: + request_mock.post(callback_api.url, + json={}, + status_code=500) + send_delivery_status_to_service(notification.id) + + assert mocked.call_count == 1 + assert mocked.call_args[1]['queue'] == 'retry-tasks' + + +@pytest.mark.parametrize("notification_type", + ["email", "letter", "sms"]) +def test_send_delivery_status_to_service_retries_if_request_throws_unknown(notify_db, + notify_db_session, + mocker, + notification_type): + user = create_user() + service = create_sample_service(notify_db, notify_db_session, user=user, restricted=True) + + template = create_sample_template( + notify_db, notify_db_session, service=service, template_type=notification_type, subject_line='Hello' + ) + create_service_callback_api(service=service, url="https://some.service.gov.uk/", + bearer_token="something_unique") + datestr = datetime(2017, 6, 20) + notification = create_notification(template=template, + created_at=datestr, + updated_at=datestr, + sent_at=datestr, + status='sent' + ) + + mocked = mocker.patch('app.celery.service_callback_tasks.send_delivery_status_to_service.retry') + mocker.patch("app.celery.tasks.request", side_effect=RequestException()) + + send_delivery_status_to_service(notification.id) + + assert mocked.call_count == 1 + assert mocked.call_args[1]['queue'] == 'retry-tasks' + + +@pytest.mark.parametrize("notification_type", + ["email", "letter", "sms"]) +def test_send_delivery_status_to_service_does_not_retries_if_request_returns_404(notify_db, + notify_db_session, + mocker, + notification_type): + user = create_user() + service = create_sample_service(notify_db, notify_db_session, user=user, restricted=True) + + template = create_sample_template( + notify_db, notify_db_session, service=service, template_type=notification_type, subject_line='Hello' + ) + callback_api = create_service_callback_api(service=service, url="https://some.service.gov.uk/", + bearer_token="something_unique") + datestr = datetime(2017, 6, 20) + notification = create_notification(template=template, + created_at=datestr, + updated_at=datestr, + sent_at=datestr, + status='sent' + ) + mocked = mocker.patch('app.celery.service_callback_tasks.send_delivery_status_to_service.retry') + with requests_mock.Mocker() as request_mock: + request_mock.post(callback_api.url, + json={}, + status_code=404) + send_delivery_status_to_service(notification.id) + + mocked.call_count == 0 diff --git a/tests/app/celery/test_tasks.py b/tests/app/celery/test_tasks.py index 6d48acf56..e33956a56 100644 --- a/tests/app/celery/test_tasks.py +++ b/tests/app/celery/test_tasks.py @@ -29,7 +29,8 @@ from app.celery.tasks import ( process_incomplete_jobs, get_template_class, s3, - send_inbound_sms_to_service) + send_inbound_sms_to_service, +) from app.config import QueueNames from app.dao import jobs_dao, services_dao from app.models import ( @@ -64,7 +65,7 @@ from tests.app.db import ( create_template, create_user, create_reply_to_email, - create_service_with_defined_sms_sender + create_service_with_defined_sms_sender, ) diff --git a/tests/app/dao/notification_dao/test_notification_dao.py b/tests/app/dao/notification_dao/test_notification_dao.py index c11d03268..0bb9b5322 100644 --- a/tests/app/dao/notification_dao/test_notification_dao.py +++ b/tests/app/dao/notification_dao/test_notification_dao.py @@ -29,7 +29,8 @@ from app.dao.notifications_dao import ( is_delivery_slow_for_provider, set_scheduled_notification_to_processed, update_notification_status_by_id, - update_notification_status_by_reference + update_notification_status_by_reference, + dao_get_notifications_by_references ) from app.dao.services_dao import dao_update_service from app.models import ( @@ -1989,3 +1990,14 @@ def test_dao_update_notifications_by_reference_returns_zero_when_no_notification "billable_units": 2} ) assert updated_count == 0 + + +def test_dao_get_notifications_by_reference(sample_template): + create_notification(template=sample_template, reference='noref') + notification_1 = create_notification(template=sample_template, reference='ref') + notification_2 = create_notification(template=sample_template, reference='ref') + + notifications = dao_get_notifications_by_references(['ref']) + assert len(notifications) == 2 + assert notifications[0].id in [notification_1.id, notification_2.id] + assert notifications[1].id in [notification_1.id, notification_2.id] diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index d601bd59d..2caf092fe 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -87,6 +87,7 @@ def test_create_service(sample_user): email_from="email_from", message_limit=1000, restricted=False, + organisation_type='central', created_by=sample_user) dao_create_service(service, sample_user) assert Service.query.count() == 1 @@ -96,10 +97,14 @@ def test_create_service(sample_user): assert service_db.id == service.id assert service_db.branding == BRANDING_GOVUK assert service_db.dvla_organisation_id == DVLA_ORG_HM_GOVERNMENT + assert service_db.email_from == 'email_from' assert service_db.research_mode is False assert service_db.prefix_sms is True assert service.active is True assert sample_user in service_db.users + assert service_db.free_sms_fragment_limit == 250000 + assert service_db.organisation_type == 'central' + assert service_db.crown is True def test_cannot_create_two_services_with_same_name(sample_user): diff --git a/tests/app/notifications/rest/test_callbacks.py b/tests/app/notifications/rest/test_callbacks.py index 73d33c7a8..0e224871b 100644 --- a/tests/app/notifications/rest/test_callbacks.py +++ b/tests/app/notifications/rest/test_callbacks.py @@ -10,6 +10,7 @@ from app.dao.notifications_dao import ( get_notification_by_id ) from tests.app.conftest import sample_notification as create_sample_notification +from tests.app.db import create_service_callback_api def firetext_post(client, data): @@ -18,7 +19,7 @@ def firetext_post(client, data): data=data, headers=[ ('Content-Type', 'application/x-www-form-urlencoded'), - ('X-Forwarded-For', '203.0.113.195, 70.41.3.18, 150.172.238.178') + ('X-Forwarded-For', '203.0.113.195, 70.41.3.18, 150.172.238.178') # fake IPs ]) @@ -28,7 +29,7 @@ def mmg_post(client, data): data=data, headers=[ ('Content-Type', 'application/json'), - ('X-Forwarded-For', '203.0.113.195, 70.41.3.18, 150.172.238.178') + ('X-Forwarded-For', '203.0.113.195, 70.41.3.18, 150.172.238.178') # fake IPs ]) @@ -178,7 +179,9 @@ def test_firetext_callback_should_update_notification_status( notify_db, notify_db_session, client, sample_email_template, mocker ): mocker.patch('app.statsd_client.incr') - + send_mock = mocker.patch( + 'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async' + ) notification = create_sample_notification( notify_db, notify_db_session, @@ -202,13 +205,16 @@ def test_firetext_callback_should_update_notification_status( updated = get_notification_by_id(notification.id) assert updated.status == 'delivered' assert get_notification_by_id(notification.id).status == 'delivered' + assert send_mock.called_once_with([notification.id], queue="notify-internal-tasks") def test_firetext_callback_should_update_notification_status_failed( notify_db, notify_db_session, client, sample_template, mocker ): mocker.patch('app.statsd_client.incr') - + mocker.patch( + 'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async' + ) notification = create_sample_notification( notify_db, notify_db_session, @@ -235,6 +241,9 @@ def test_firetext_callback_should_update_notification_status_failed( def test_firetext_callback_should_update_notification_status_pending(client, notify_db, notify_db_session, mocker): mocker.patch('app.statsd_client.incr') + mocker.patch( + 'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async' + ) notification = create_sample_notification( notify_db, notify_db_session, status='sending', sent_at=datetime.utcnow() ) @@ -265,8 +274,11 @@ def test_process_mmg_response_return_200_when_cid_is_send_sms_code(client): def test_process_mmg_response_returns_200_when_cid_is_valid_notification_id( - notify_db, notify_db_session, client + notify_db, notify_db_session, client, mocker ): + mocker.patch( + 'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async' + ) notification = create_sample_notification( notify_db, notify_db_session, status='sending', sent_at=datetime.utcnow() ) @@ -286,8 +298,11 @@ def test_process_mmg_response_returns_200_when_cid_is_valid_notification_id( def test_process_mmg_response_status_5_updates_notification_with_permanently_failed( - notify_db, notify_db_session, client + notify_db, notify_db_session, client, mocker ): + mocker.patch( + 'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async' + ) notification = create_sample_notification( notify_db, notify_db_session, status='sending', sent_at=datetime.utcnow() ) @@ -306,8 +321,11 @@ def test_process_mmg_response_status_5_updates_notification_with_permanently_fai def test_process_mmg_response_status_2_updates_notification_with_permanently_failed( - notify_db, notify_db_session, client + notify_db, notify_db_session, client, mocker ): + mocker.patch( + 'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async' + ) notification = create_sample_notification( notify_db, notify_db_session, status='sending', sent_at=datetime.utcnow() ) @@ -325,8 +343,11 @@ def test_process_mmg_response_status_2_updates_notification_with_permanently_fai def test_process_mmg_response_status_4_updates_notification_with_temporary_failed( - notify_db, notify_db_session, client + notify_db, notify_db_session, client, mocker ): + mocker.patch( + 'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async' + ) notification = create_sample_notification( notify_db, notify_db_session, status='sending', sent_at=datetime.utcnow() ) @@ -345,8 +366,11 @@ def test_process_mmg_response_status_4_updates_notification_with_temporary_faile def test_process_mmg_response_unknown_status_updates_notification_with_failed( - notify_db, notify_db_session, client + notify_db, notify_db_session, client, mocker ): + send_mock = mocker.patch( + 'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async' + ) notification = create_sample_notification( notify_db, notify_db_session, status='sending', sent_at=datetime.utcnow() ) @@ -354,13 +378,14 @@ def test_process_mmg_response_unknown_status_updates_notification_with_failed( "CID": str(notification.id), "MSISDN": "447777349060", "status": 10}) - + create_service_callback_api(service=notification.service, url="https://original_url.com") response = mmg_post(client, data) assert response.status_code == 200 json_data = json.loads(response.data) assert json_data['result'] == 'success' assert json_data['message'] == 'MMG callback succeeded. reference {} updated'.format(notification.id) assert get_notification_by_id(notification.id).status == 'failed' + assert send_mock.called def test_process_mmg_response_returns_400_for_malformed_data(client): @@ -392,6 +417,9 @@ def test_process_mmg_response_records_statsd(notify_db, notify_db_session, clien mocker.patch('app.statsd_client.incr') mocker.patch('app.statsd_client.timing_with_dates') + mocker.patch( + 'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async' + ) notification = create_sample_notification( notify_db, notify_db_session, status='sending', sent_at=datetime.utcnow() ) @@ -415,6 +443,9 @@ def test_firetext_callback_should_record_statsd(client, notify_db, notify_db_ses mocker.patch('app.statsd_client.incr') mocker.patch('app.statsd_client.timing_with_dates') + mocker.patch( + 'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async' + ) notification = create_sample_notification( notify_db, notify_db_session, status='sending', sent_at=datetime.utcnow() ) diff --git a/tests/app/notifications/rest/test_send_notification.py b/tests/app/notifications/rest/test_send_notification.py index 4b57c4128..480455e75 100644 --- a/tests/app/notifications/rest/test_send_notification.py +++ b/tests/app/notifications/rest/test_send_notification.py @@ -16,7 +16,10 @@ from app.models import ( 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 -from app.v2.errors import RateLimitError +from app.errors import InvalidRequest +from app.models import Template +from app.v2.errors import RateLimitError, TooManyRequestsError + from tests import create_authorization_header from tests.app.conftest import ( sample_notification as create_sample_notification, @@ -28,9 +31,6 @@ from tests.app.conftest import ( sample_service, sample_template_without_sms_permission, sample_template_without_email_permission) - -from app.models import Template -from app.errors import InvalidRequest from tests.app.db import create_service, create_reply_to_email @@ -414,6 +414,10 @@ def test_should_block_api_call_if_over_day_limit_for_live_service( mocker): with notify_api.test_request_context(): with notify_api.test_client() as client: + mocker.patch( + 'app.notifications.validators.check_service_over_daily_message_limit', + side_effect=TooManyRequestsError(1) + ) mocker.patch('app.celery.provider_tasks.deliver_email.apply_async') service = create_sample_service(notify_db, notify_db_session, limit=1, restricted=False) @@ -446,6 +450,10 @@ def test_should_block_api_call_if_over_day_limit_for_restricted_service( with notify_api.test_request_context(): with notify_api.test_client() as client: mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async') + mocker.patch( + 'app.notifications.validators.check_service_over_daily_message_limit', + side_effect=TooManyRequestsError(1) + ) service = create_sample_service(notify_db, notify_db_session, limit=1, restricted=True) email_template = create_sample_email_template(notify_db, notify_db_session, service=service) diff --git a/tests/app/notifications/test_notifications_ses_callback.py b/tests/app/notifications/test_notifications_ses_callback.py index 330466234..49ce90829 100644 --- a/tests/app/notifications/test_notifications_ses_callback.py +++ b/tests/app/notifications/test_notifications_ses_callback.py @@ -10,6 +10,7 @@ from app.notifications.notifications_ses_callback import process_ses_response, r from app.celery.research_mode_tasks import ses_hard_bounce_callback, ses_soft_bounce_callback, ses_notification_callback from tests.app.conftest import sample_notification as create_sample_notification +from tests.app.db import create_service_callback_api def test_ses_callback_should_update_notification_status( @@ -24,7 +25,42 @@ def test_ses_callback_should_update_notification_status( stats_mock = mocker.patch( 'app.notifications.notifications_ses_callback.create_outcome_notification_statistic_tasks' ) + send_mock = mocker.patch( + 'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async' + ) + notification = create_sample_notification( + notify_db, + notify_db_session, + template=sample_email_template, + reference='ref', + status='sending', + sent_at=datetime.utcnow() + ) + create_service_callback_api(service=sample_email_template.service, url="https://original_url.com") + assert get_notification_by_id(notification.id).status == 'sending' + errors = process_ses_response(ses_notification_callback(reference='ref')) + assert errors is None + assert get_notification_by_id(notification.id).status == 'delivered' + statsd_client.timing_with_dates.assert_any_call( + "callback.ses.elapsed-time", datetime.utcnow(), notification.sent_at + ) + statsd_client.incr.assert_any_call("callback.ses.delivered") + stats_mock.assert_called_once_with(notification) + send_mock.assert_called_once_with([str(notification.id)], queue="notify-internal-tasks") + + +def test_ses_callback_does_not_call_send_delivery_status_if_no_db_entry( + client, + notify_db, + notify_db_session, + sample_email_template, + mocker): + with freeze_time('2001-01-01T12:00:00'): + + send_mock = mocker.patch( + 'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async' + ) notification = create_sample_notification( notify_db, notify_db_session, @@ -39,11 +75,8 @@ def test_ses_callback_should_update_notification_status( errors = process_ses_response(ses_notification_callback(reference='ref')) assert errors is None assert get_notification_by_id(notification.id).status == 'delivered' - statsd_client.timing_with_dates.assert_any_call( - "callback.ses.elapsed-time", datetime.utcnow(), notification.sent_at - ) - statsd_client.incr.assert_any_call("callback.ses.delivered") - stats_mock.assert_called_once_with(notification) + + send_mock.assert_not_called() def test_ses_callback_should_update_multiple_notification_status_sent( @@ -56,7 +89,9 @@ def test_ses_callback_should_update_multiple_notification_status_sent( stats_mock = mocker.patch( 'app.notifications.notifications_ses_callback.create_outcome_notification_statistic_tasks' ) - + send_mock = mocker.patch( + 'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async' + ) notification1 = create_sample_notification( notify_db, notify_db_session, @@ -80,7 +115,7 @@ def test_ses_callback_should_update_multiple_notification_status_sent( reference='ref3', sent_at=datetime.utcnow(), status='sending') - + create_service_callback_api(service=sample_email_template.service, url="https://original_url.com") assert process_ses_response(ses_notification_callback(reference='ref1')) is None assert process_ses_response(ses_notification_callback(reference='ref2')) is None assert process_ses_response(ses_notification_callback(reference='ref3')) is None @@ -90,6 +125,7 @@ def test_ses_callback_should_update_multiple_notification_status_sent( call(notification2), call(notification3) ]) + assert send_mock.called def test_ses_callback_should_set_status_to_temporary_failure(client, @@ -101,7 +137,9 @@ def test_ses_callback_should_set_status_to_temporary_failure(client, stats_mock = mocker.patch( 'app.notifications.notifications_ses_callback.create_outcome_notification_statistic_tasks' ) - + send_mock = mocker.patch( + 'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async' + ) notification = create_sample_notification( notify_db, notify_db_session, @@ -110,9 +148,11 @@ def test_ses_callback_should_set_status_to_temporary_failure(client, status='sending', sent_at=datetime.utcnow() ) + create_service_callback_api(service=notification.service, url="https://original_url.com") assert get_notification_by_id(notification.id).status == 'sending' assert process_ses_response(ses_soft_bounce_callback(reference='ref')) is None assert get_notification_by_id(notification.id).status == 'temporary-failure' + assert send_mock.called stats_mock.assert_called_once_with(notification) @@ -146,7 +186,9 @@ def test_ses_callback_should_set_status_to_permanent_failure(client, stats_mock = mocker.patch( 'app.notifications.notifications_ses_callback.create_outcome_notification_statistic_tasks' ) - + send_mock = mocker.patch( + 'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async' + ) notification = create_sample_notification( notify_db, notify_db_session, @@ -155,10 +197,12 @@ def test_ses_callback_should_set_status_to_permanent_failure(client, status='sending', sent_at=datetime.utcnow() ) + create_service_callback_api(service=sample_email_template.service, url="https://original_url.com") assert get_notification_by_id(notification.id).status == 'sending' assert process_ses_response(ses_hard_bounce_callback(reference='ref')) is None assert get_notification_by_id(notification.id).status == 'permanent-failure' + assert send_mock.called stats_mock.assert_called_once_with(notification) diff --git a/tests/app/notifications/test_process_client_response.py b/tests/app/notifications/test_process_client_response.py index a4e3d2413..40e2a608c 100644 --- a/tests/app/notifications/test_process_client_response.py +++ b/tests/app/notifications/test_process_client_response.py @@ -4,6 +4,7 @@ from app.notifications.process_client_response import ( validate_callback_data, process_sms_client_response ) +from tests.app.db import create_service_callback_api def test_validate_callback_data_returns_none_when_valid(): @@ -51,15 +52,32 @@ def test_outcome_statistics_called_for_successful_callback(sample_notification, 'app.notifications.process_client_response.notifications_dao.update_notification_status_by_id', return_value=sample_notification ) - + send_mock = mocker.patch( + 'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async' + ) + create_service_callback_api(service=sample_notification.service, url="https://original_url.com") reference = str(uuid.uuid4()) success, error = process_sms_client_response(status='3', reference=reference, client_name='MMG') assert success == "MMG callback succeeded. reference {} updated".format(str(reference)) assert error is None + send_mock.assert_called_once_with([str(sample_notification.id)], queue="notify-internal-tasks") stats_mock.assert_called_once_with(sample_notification) +def test_sms_resonse_does_not_call_send_callback_if_no_db_entry(sample_notification, mocker): + mocker.patch( + 'app.notifications.process_client_response.notifications_dao.update_notification_status_by_id', + return_value=sample_notification + ) + send_mock = mocker.patch( + 'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async' + ) + reference = str(uuid.uuid4()) + process_sms_client_response(status='3', reference=reference, client_name='MMG') + send_mock.assert_not_called() + + def test_process_sms_response_return_success_for_send_sms_code_reference(mocker): stats_mock = mocker.patch('app.notifications.process_client_response.create_outcome_notification_statistic_tasks') diff --git a/tests/app/notifications/test_validators.py b/tests/app/notifications/test_validators.py index cb841e431..1c9fbb579 100644 --- a/tests/app/notifications/test_validators.py +++ b/tests/app/notifications/test_validators.py @@ -1,6 +1,7 @@ import pytest from freezegun import freeze_time from flask import current_app + import app from app.models import INTERNATIONAL_SMS_TYPE, SMS_TYPE, EMAIL_TYPE from app.notifications.validators import ( @@ -18,6 +19,8 @@ from app.v2.errors import ( BadRequestError, TooManyRequestsError, RateLimitError) + +from tests.conftest import set_config from tests.app.conftest import ( sample_notification as create_notification, sample_service as create_service, @@ -26,6 +29,13 @@ from tests.app.conftest import ( from tests.app.db import create_reply_to_email, create_service_sms_sender +# all of these tests should have redis enabled (except where we specifically disable it) +@pytest.fixture(scope='module', autouse=True) +def enable_redis(notify_api): + with set_config(notify_api, 'REDIS_ENABLED', True): + yield + + @pytest.mark.parametrize('key_type', ['test', 'team', 'normal']) def test_check_service_message_limit_in_cache_with_unrestricted_service_is_allowed( key_type, @@ -78,6 +88,15 @@ def test_should_set_cache_value_as_value_from_database_if_cache_not_set( ) +def test_should_not_access_database_if_redis_disabled(notify_api, sample_service, mocker): + with set_config(notify_api, 'REDIS_ENABLED', False): + db_mock = mocker.patch('app.notifications.validators.services_dao') + + check_service_over_daily_message_limit('normal', sample_service) + + assert db_mock.method_calls == [] + + @pytest.mark.parametrize('key_type', ['team', 'normal']) def test_check_service_message_limit_over_message_limit_fails(key_type, notify_db, notify_db_session, mocker): with freeze_time("2016-01-01 12:00:00.000000"): diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 73679001f..cc8f13eaa 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -536,6 +536,26 @@ def test_update_service_flags(client, sample_service): assert set(result['data']['permissions']) == set([LETTER_TYPE, INTERNATIONAL_SMS_TYPE]) +@pytest.mark.parametrize("org_type, expected", + [("central", True), + ('local', False), + ("nhs", False)]) +def test_update_service_sets_crown(client, sample_service, org_type, expected): + data = { + 'organisation_type': org_type, + } + auth_header = create_authorization_header() + + resp = client.post( + '/service/{}'.format(sample_service.id), + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), auth_header] + ) + result = json.loads(resp.get_data(as_text=True)) + assert resp.status_code == 200 + assert result['data']['crown'] is expected + + @pytest.fixture(scope='function') def service_with_no_permissions(notify_db, notify_db_session): return create_service(service_permissions=[]) @@ -2163,18 +2183,17 @@ def test_search_for_notification_by_to_field_returns_content( assert notifications[0]['template']['content'] == 'Hello (( Name))\nYour thing is due soon' -def test_send_one_off_notification(admin_request, mocker): - service = create_service() - template = create_template(service=service) +def test_send_one_off_notification(sample_service, admin_request, mocker): + template = create_template(service=sample_service) mocker.patch('app.service.send_notification.send_notification_to_queue') response = admin_request.post( 'service.create_one_off_notification', - service_id=service.id, + service_id=sample_service.id, _data={ 'template_id': str(template.id), 'to': '07700900001', - 'created_by': str(service.created_by_id) + 'created_by': str(sample_service.created_by_id) }, _expected_status=201 ) diff --git a/tests/app/service/test_send_one_off_notification.py b/tests/app/service/test_send_one_off_notification.py index 14c49aa56..6a9a681ff 100644 --- a/tests/app/service/test_send_one_off_notification.py +++ b/tests/app/service/test_send_one_off_notification.py @@ -151,9 +151,13 @@ def test_send_one_off_notification_raises_if_cant_send_to_recipient(notify_db_se assert 'service is in trial mode' in e.value.message -def test_send_one_off_notification_raises_if_over_limit(notify_db_session): +def test_send_one_off_notification_raises_if_over_limit(notify_db_session, mocker): service = create_service(message_limit=0) template = create_template(service=service) + mocker.patch( + 'app.service.send_notification.check_service_over_daily_message_limit', + side_effect=TooManyRequestsError(1) + ) post_data = { 'template_id': str(template.id),