diff --git a/README.md b/README.md index 5c6143b4d..dc29d8fb1 100644 --- a/README.md +++ b/README.md @@ -1,4 +1,3 @@ -![](https://travis-ci.org/alphagov/notifications-api.svg) [![Requirements Status](https://requires.io/github/alphagov/notifications-api/requirements.svg?branch=master)](https://requires.io/github/alphagov/notifications-api/requirements/?branch=master) # notifications-api diff --git a/app/__init__.py b/app/__init__.py index cf47632f9..c602003fb 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -70,6 +70,7 @@ def create_app(app_name=None): from app.provider_details.rest import provider_details as provider_details_blueprint from app.spec.rest import spec as spec_blueprint from app.organisation.rest import organisation_blueprint + from app.delivery.rest import delivery_blueprint application.register_blueprint(service_blueprint, url_prefix='/service') application.register_blueprint(user_blueprint, url_prefix='/user') @@ -78,6 +79,7 @@ def create_app(app_name=None): application.register_blueprint(notifications_blueprint) application.register_blueprint(job_blueprint) application.register_blueprint(invite_blueprint) + application.register_blueprint(delivery_blueprint) application.register_blueprint(accept_invite, url_prefix='/invite') application.register_blueprint(template_statistics_blueprint) diff --git a/app/authentication/auth.py b/app/authentication/auth.py index 5e8cb2d4e..09ccadb66 100644 --- a/app/authentication/auth.py +++ b/app/authentication/auth.py @@ -1,4 +1,5 @@ -from flask import request, jsonify, _request_ctx_stack, current_app +from flask import request, _request_ctx_stack, current_app +from sqlalchemy.exc import DataError from sqlalchemy.orm.exc import NoResultFound from notifications_python_client.authentication import decode_jwt_token, get_token_issuer @@ -37,8 +38,10 @@ def requires_auth(): if client == current_app.config.get('ADMIN_CLIENT_USER_NAME'): return handle_admin_key(auth_token, current_app.config.get('ADMIN_CLIENT_SECRET')) - api_keys = get_model_api_keys(client) - + try: + api_keys = get_model_api_keys(client) + except DataError: + raise AuthError("Invalid token: service id is not the right data type", 403) for api_key in api_keys: try: get_decode_errors(auth_token, api_key.unsigned_secret) @@ -59,19 +62,19 @@ def requires_auth(): if not api_keys: raise AuthError("Invalid token: service has no API keys", 403) else: - raise AuthError("Invalid token: signature", 403) + raise AuthError("Invalid token: signature, api token is not valid", 403) def handle_admin_key(auth_token, secret): try: get_decode_errors(auth_token, secret) return - except TokenDecodeError as e: + except TokenDecodeError: raise AuthError("Invalid token: signature", 403) def get_decode_errors(auth_token, unsigned_secret): try: decode_jwt_token(auth_token, unsigned_secret) - except TokenExpiredError as e: + except TokenExpiredError: raise AuthError("Invalid token: expired", 403) diff --git a/app/celery/provider_tasks.py b/app/celery/provider_tasks.py index 2c07eb251..c94120e79 100644 --- a/app/celery/provider_tasks.py +++ b/app/celery/provider_tasks.py @@ -1,31 +1,17 @@ -from datetime import datetime - from flask import current_app -from notifications_utils.recipients import ( - validate_and_format_phone_number -) -from notifications_utils.template import Template, get_sms_fragment_count -from notifications_utils.renderers import HTMLEmail, PlainTextEmail, SMSMessage -from app import notify_celery, statsd_client, clients, create_uuid -from app.clients.email import EmailClientException -from app.clients.sms import SmsClientException -from app.dao.notifications_dao import ( - get_notification_by_id, - dao_update_notification, - update_notification_status_by_id -) -from app.dao.provider_details_dao import get_provider_details_by_notification_type -from app.dao.services_dao import dao_fetch_service_by_id -from app.celery.research_mode_tasks import send_sms_response, send_email_response -from app.dao.templates_dao import dao_get_template_by_id - -from app.models import SMS_TYPE, EMAIL_TYPE, KEY_TYPE_TEST, BRANDING_ORG +from app import notify_celery +from app.dao import notifications_dao +from app.dao.notifications_dao import update_notification_status_by_id from app.statsd_decorators import statsd +from app.delivery import send_to_providers +from sqlalchemy.orm.exc import NoResultFound + def retry_iteration_to_delay(retry=0): """ + :param retry times we have performed a retry Given current retry calculate some delay before retrying 0: 10 seconds 1: 60 seconds (1 minutes) @@ -47,153 +33,93 @@ def retry_iteration_to_delay(retry=0): return delays.get(retry, 10) +@notify_celery.task(bind=True, name="deliver_sms", max_retries=5, default_retry_delay=5) +@statsd(namespace="tasks") +def deliver_sms(self, notification_id): + try: + notification = notifications_dao.get_notification_by_id(notification_id) + if not notification: + raise NoResultFound() + send_to_providers.send_sms_to_provider(notification) + except Exception as e: + try: + current_app.logger.error( + "RETRY: SMS notification {} failed".format(notification_id) + ) + current_app.logger.exception(e) + self.retry(queue="retry", countdown=retry_iteration_to_delay(self.request.retries)) + except self.MaxRetriesExceededError: + current_app.logger.error( + "RETRY FAILED: task send_sms_to_provider failed for notification {}".format(notification_id), + e + ) + update_notification_status_by_id(notification_id, 'technical-failure') + + +@notify_celery.task(bind=True, name="deliver_email", max_retries=5, default_retry_delay=5) +@statsd(namespace="tasks") +def deliver_email(self, notification_id): + try: + notification = notifications_dao.get_notification_by_id(notification_id) + if not notification: + raise NoResultFound() + send_to_providers.send_email_to_provider(notification) + except Exception as e: + try: + current_app.logger.error( + "RETRY: Email notification {} failed".format(notification_id) + ) + current_app.logger.exception(e) + self.retry(queue="retry", countdown=retry_iteration_to_delay(self.request.retries)) + except self.MaxRetriesExceededError: + current_app.logger.error( + "RETRY FAILED: task send_email_to_provider failed for notification {}".format(notification_id), + e + ) + update_notification_status_by_id(notification_id, 'technical-failure') + + @notify_celery.task(bind=True, name="send-sms-to-provider", max_retries=5, default_retry_delay=5) @statsd(namespace="tasks") def send_sms_to_provider(self, service_id, notification_id): - service = dao_fetch_service_by_id(service_id) - provider = provider_to_use(SMS_TYPE, notification_id) - notification = get_notification_by_id(notification_id) - if notification.status == 'created': - template_model = dao_get_template_by_id(notification.template_id, notification.template_version) - template = Template( - template_model.__dict__, - values={} if not notification.personalisation else notification.personalisation, - renderer=SMSMessage(prefix=service.name, sender=service.sms_sender) - ) + try: + notification = notifications_dao.get_notification_by_id(notification_id) + if not notification: + raise NoResultFound() + send_to_providers.send_sms_to_provider(notification) + except Exception as e: try: - if service.research_mode or notification.key_type == KEY_TYPE_TEST: - send_sms_response.apply_async( - (provider.get_name(), str(notification_id), notification.to), queue='research-mode' - ) - notification.billable_units = 0 - else: - provider.send_sms( - to=validate_and_format_phone_number(notification.to), - content=template.replaced, - reference=str(notification_id), - sender=service.sms_sender - ) - notification.billable_units = get_sms_fragment_count(template.replaced_content_count) - - notification.sent_at = datetime.utcnow() - notification.sent_by = provider.get_name() - notification.status = 'sending' - dao_update_notification(notification) - except SmsClientException as e: - try: - current_app.logger.error( - "RETRY: SMS notification {} failed".format(notification_id) - ) - current_app.logger.exception(e) - self.retry(queue="retry", countdown=retry_iteration_to_delay(self.request.retries)) - except self.MaxRetriesExceededError: - current_app.logger.error( - "RETRY FAILED: task send_sms_to_provider failed for notification {}".format(notification.id), - e - ) - update_notification_status_by_id(notification.id, 'technical-failure') - - current_app.logger.info( - "SMS {} sent to provider at {}".format(notification_id, notification.sent_at) - ) - delta_milliseconds = (datetime.utcnow() - notification.created_at).total_seconds() * 1000 - statsd_client.timing("sms.total-time", delta_milliseconds) - - -def provider_to_use(notification_type, notification_id): - active_providers_in_order = [ - provider for provider in get_provider_details_by_notification_type(notification_type) if provider.active - ] - - if not active_providers_in_order: - current_app.logger.error( - "{} {} failed as no active providers".format(notification_type, notification_id) - ) - raise Exception("No active {} providers".format(notification_type)) - - return clients.get_client_by_name_and_type(active_providers_in_order[0].identifier, notification_type) + current_app.logger.error( + "RETRY: SMS notification {} failed".format(notification_id) + ) + current_app.logger.exception(e) + self.retry(queue="retry", countdown=retry_iteration_to_delay(self.request.retries)) + except self.MaxRetriesExceededError: + current_app.logger.error( + "RETRY FAILED: task send_sms_to_provider failed for notification {}".format(notification_id), + e + ) + update_notification_status_by_id(notification_id, 'technical-failure') @notify_celery.task(bind=True, name="send-email-to-provider", max_retries=5, default_retry_delay=5) @statsd(namespace="tasks") def send_email_to_provider(self, service_id, notification_id): - service = dao_fetch_service_by_id(service_id) - provider = provider_to_use(EMAIL_TYPE, notification_id) - notification = get_notification_by_id(notification_id) - if notification.status == 'created': + try: + notification = notifications_dao.get_notification_by_id(notification_id) + if not notification: + raise NoResultFound() + send_to_providers.send_email_to_provider(notification) + except Exception as e: try: - template_dict = dao_get_template_by_id(notification.template_id, notification.template_version).__dict__ - - html_email = Template( - template_dict, - values=notification.personalisation, - renderer=get_html_email_renderer(service) + current_app.logger.error( + "RETRY: Email notification {} failed".format(notification_id) ) - - plain_text_email = Template( - template_dict, - values=notification.personalisation, - renderer=PlainTextEmail() + current_app.logger.exception(e) + self.retry(queue="retry", countdown=retry_iteration_to_delay(self.request.retries)) + except self.MaxRetriesExceededError: + current_app.logger.error( + "RETRY FAILED: task send_email_to_provider failed for notification {}".format(notification_id), + e ) - - if service.research_mode or notification.key_type == KEY_TYPE_TEST: - reference = str(create_uuid()) - send_email_response.apply_async( - (provider.get_name(), reference, notification.to), queue='research-mode' - ) - notification.billable_units = 0 - else: - from_address = '"{}" <{}@{}>'.format(service.name, service.email_from, - current_app.config['NOTIFY_EMAIL_DOMAIN']) - reference = provider.send_email( - from_address, - notification.to, - plain_text_email.replaced_subject, - body=plain_text_email.replaced, - html_body=html_email.replaced, - reply_to_address=service.reply_to_email_address, - ) - - notification.reference = reference - notification.sent_at = datetime.utcnow() - notification.sent_by = provider.get_name(), - notification.status = 'sending' - dao_update_notification(notification) - except EmailClientException as e: - try: - current_app.logger.error( - "RETRY: Email notification {} failed".format(notification_id) - ) - current_app.logger.exception(e) - self.retry(queue="retry", countdown=retry_iteration_to_delay(self.request.retries)) - except self.MaxRetriesExceededError: - current_app.logger.error( - "RETRY FAILED: task send_email_to_provider failed for notification {}".format(notification.id), - e - ) - update_notification_status_by_id(notification.id, 'technical-failure') - - current_app.logger.info( - "Email {} sent to provider at {}".format(notification_id, notification.sent_at) - ) - delta_milliseconds = (datetime.utcnow() - notification.created_at).total_seconds() * 1000 - statsd_client.timing("email.total-time", delta_milliseconds) - - -def get_html_email_renderer(service): - govuk_banner = service.branding != BRANDING_ORG - if service.organisation: - logo = '{}{}{}'.format( - current_app.config['ADMIN_BASE_URL'], - current_app.config['BRANDING_PATH'], - service.organisation.logo - ) - branding = { - 'brand_colour': service.organisation.colour, - 'brand_logo': logo, - 'brand_name': service.organisation.name, - } - else: - branding = {} - - return HTMLEmail(govuk_banner=govuk_banner, **branding) + update_notification_status_by_id(notification_id, 'technical-failure') diff --git a/app/clients/sms/__init__.py b/app/clients/sms/__init__.py index 90d63ed0d..e6accdb8a 100644 --- a/app/clients/sms/__init__.py +++ b/app/clients/sms/__init__.py @@ -1,11 +1,16 @@ from app.clients import (Client, ClientException) -class SmsClientException(ClientException): +class SmsClientResponseException(ClientException): ''' - Base Exception for SmsClients + Base Exception for SmsClientsResponses ''' - pass + + def __init__(self, message): + self.message = message + + def __str__(self): + return "Message {}".format(self.message) class SmsClient(Client): diff --git a/app/clients/sms/firetext.py b/app/clients/sms/firetext.py index e9760a4e6..9617b88bf 100644 --- a/app/clients/sms/firetext.py +++ b/app/clients/sms/firetext.py @@ -1,12 +1,10 @@ +import json import logging from monotonic import monotonic -from requests import request, RequestException, HTTPError +from requests import request, RequestException -from app.clients.sms import ( - SmsClient, - SmsClientException -) +from app.clients.sms import (SmsClient, SmsClientResponseException) from app.clients import STATISTICS_DELIVERED, STATISTICS_FAILURE logger = logging.getLogger(__name__) @@ -42,13 +40,14 @@ def get_firetext_responses(status): return firetext_responses[status] -class FiretextClientException(SmsClientException): - def __init__(self, response): - self.code = response['code'] - self.description = response['description'] +class FiretextClientResponseException(SmsClientResponseException): + def __init__(self, response, exception): + self.status_code = response.status_code + self.text = response.text + self.exception = exception def __str__(self): - return "Code {} description {}".format(self.code, self.description) + return "Code {} text {} exception {}".format(self.status_code, self.text, str(self.exception)) class FiretextClient(SmsClient): @@ -62,11 +61,27 @@ class FiretextClient(SmsClient): self.api_key = current_app.config.get('FIRETEXT_API_KEY') self.from_number = current_app.config.get('FROM_NUMBER') self.name = 'firetext' + self.url = "https://www.firetext.co.uk/api/sendsms/json" self.statsd_client = statsd_client def get_name(self): return self.name + def record_outcome(self, success, response): + log_message = "API {} request {} on {} response status_code {}".format( + "POST", + "succeeded" if success else "failed", + self.url, + response.status_code + ) + + if success: + self.current_app.logger.info(log_message) + self.statsd_client.incr("clients.firetext.success") + else: + self.statsd_client.incr("clients.firetext.error") + self.current_app.logger.error(log_message) + def send_sms(self, to, content, reference, sender=None): data = { @@ -81,36 +96,23 @@ class FiretextClient(SmsClient): try: response = request( "POST", - "https://www.firetext.co.uk/api/sendsms/json", + self.url, data=data ) - firetext_response = response.json() - if firetext_response['code'] != 0: - raise FiretextClientException(firetext_response) response.raise_for_status() - self.current_app.logger.info( - "API {} request on {} succeeded with {} '{}'".format( - "POST", - "https://www.firetext.co.uk/api/sendsms", - response.status_code, - firetext_response.items() - ) - ) + try: + json.loads(response.text) + if response.json()['code'] != 0: + raise ValueError() + except (ValueError, AttributeError) as e: + self.record_outcome(False, response) + raise FiretextClientResponseException(response=response, exception=e) + self.record_outcome(True, response) except RequestException as e: - api_error = HTTPError.create(e) - logger.error( - "API {} request on {} failed with {} '{}'".format( - "POST", - "https://www.firetext.co.uk/api/sendsms", - api_error.status_code, - api_error.message - ) - ) - self.statsd_client.incr("clients.firetext.error") - raise api_error + self.record_outcome(False, e.response) + raise FiretextClientResponseException(response=e.response, exception=e) finally: elapsed_time = monotonic() - start_time self.current_app.logger.info("Firetext request finished in {}".format(elapsed_time)) - self.statsd_client.incr("clients.firetext.success") self.statsd_client.timing("clients.firetext.request-time", elapsed_time) return response diff --git a/app/clients/sms/loadtesting.py b/app/clients/sms/loadtesting.py index a3e500ba8..429a6d314 100644 --- a/app/clients/sms/loadtesting.py +++ b/app/clients/sms/loadtesting.py @@ -1,4 +1,7 @@ import logging + +from flask import current_app + from app.clients.sms.firetext import ( FiretextClient ) @@ -13,7 +16,9 @@ class LoadtestingClient(FiretextClient): def init_app(self, config, statsd_client, *args, **kwargs): super(FiretextClient, self).__init__(*args, **kwargs) + self.current_app = current_app self.api_key = config.config.get('LOADTESTING_API_KEY') - self.from_number = config.config.get('LOADTESTING_NUMBER') + self.from_number = config.config.get('FROM_NUMBER') self.name = 'loadtesting' + self.url = "https://www.firetext.co.uk/api/sendsms/json" self.statsd_client = statsd_client diff --git a/app/clients/sms/mmg.py b/app/clients/sms/mmg.py index 066c1f259..b036d637c 100644 --- a/app/clients/sms/mmg.py +++ b/app/clients/sms/mmg.py @@ -1,8 +1,9 @@ import json from monotonic import monotonic -from requests import (request, RequestException, HTTPError) +from requests import (request, RequestException) + from app.clients import (STATISTICS_DELIVERED, STATISTICS_FAILURE) -from app.clients.sms import (SmsClient, SmsClientException) +from app.clients.sms import (SmsClient, SmsClientResponseException) mmg_response_map = { '2': { @@ -42,13 +43,14 @@ def get_mmg_responses(status): return mmg_response_map.get(status, mmg_response_map.get('default')) -class MMGClientException(SmsClientException): - def __init__(self, error_response): - self.code = error_response['Error'] - self.description = error_response['Description'] +class MMGClientResponseException(SmsClientResponseException): + def __init__(self, response, exception): + self.status_code = response.status_code + self.text = response.text + self.exception = exception def __str__(self): - return "Code {} description {}".format(self.code, self.description) + return "Code {} text {} exception {}".format(self.status_code, self.text, str(self.exception)) class MMGClient(SmsClient): @@ -65,6 +67,21 @@ class MMGClient(SmsClient): self.statsd_client = statsd_client self.mmg_url = current_app.config.get('MMG_URL') + def record_outcome(self, success, response): + log_message = "API {} request {} on {} response status_code {}".format( + "POST", + "succeeded" if success else "failed", + self.mmg_url, + response.status_code + ) + + if success: + self.current_app.logger.info(log_message) + self.statsd_client.incr("clients.mmg.success") + else: + self.statsd_client.incr("clients.mmg.error") + self.current_app.logger.error(log_message) + def get_name(self): return self.name @@ -79,38 +96,30 @@ class MMGClient(SmsClient): } start_time = monotonic() - try: - response = request("POST", self.mmg_url, - data=json.dumps(data), - headers={'Content-Type': 'application/json', - 'Authorization': 'Basic {}'.format(self.api_key)}) - if response.status_code != 200: - raise MMGClientException(response.json()) + response = request( + "POST", + self.mmg_url, + data=json.dumps(data), + headers={ + 'Content-Type': 'application/json', + 'Authorization': 'Basic {}'.format(self.api_key) + } + ) + response.raise_for_status() - self.current_app.logger.info( - "API {} request on {} succeeded with {} '{}'".format( - "POST", - self.mmg_url, - response.status_code, - response.json().items() - ) - ) + try: + json.loads(response.text) + except (ValueError, AttributeError) as e: + self.record_outcome(False, response) + raise MMGClientResponseException(response=response, exception=e) + self.record_outcome(True, response) except RequestException as e: - api_error = HTTPError.create(e) - self.current_app.logger.error( - "API {} request on {} failed with {} '{}'".format( - "POST", - self.mmg_url, - api_error.status_code, - api_error.message - ) - ) - self.statsd_client.incr("clients.mmg.error") - raise api_error + self.record_outcome(False, e.response) + raise MMGClientResponseException(response=e.response, exception=e) finally: elapsed_time = monotonic() - start_time self.statsd_client.timing("clients.mmg.request-time", elapsed_time) - self.statsd_client.incr("clients.mmg.success") self.current_app.logger.info("MMG request finished in {}".format(elapsed_time)) + return response diff --git a/app/dao/jobs_dao.py b/app/dao/jobs_dao.py index b4cfbde4d..41ba00940 100644 --- a/app/dao/jobs_dao.py +++ b/app/dao/jobs_dao.py @@ -1,10 +1,11 @@ -from datetime import date, timedelta, datetime -from sqlalchemy import desc, asc, cast, Date as sql_date +from datetime import datetime + +from sqlalchemy import func, desc, asc, cast, Date as sql_date + from app import db from app.dao import days_ago from app.models import Job, NotificationHistory, JOB_STATUS_SCHEDULED from app.statsd_decorators import statsd -from sqlalchemy import func, asc @statsd(namespace="dao") @@ -26,11 +27,18 @@ def dao_get_job_by_service_id_and_job_id(service_id, job_id): return Job.query.filter_by(service_id=service_id, id=job_id).one() -def dao_get_jobs_by_service_id(service_id, limit_days=None): +def dao_get_jobs_by_service_id(service_id, limit_days=None, page=1, page_size=50, statuses=None): query_filter = [Job.service_id == service_id] if limit_days is not None: query_filter.append(cast(Job.created_at, sql_date) >= days_ago(limit_days)) - return Job.query.filter(*query_filter).order_by(desc(Job.created_at)).all() + if statuses is not None and statuses != ['']: + query_filter.append( + Job.job_status.in_(statuses) + ) + return Job.query \ + .filter(*query_filter) \ + .order_by(desc(Job.created_at)) \ + .paginate(page=page, per_page=page_size) def dao_get_job_by_id(job_id): diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index fa65e8a72..c289f60ed 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -228,14 +228,17 @@ def get_notifications(filter_dict=None): @statsd(namespace="dao") -def get_notifications_for_service(service_id, - filter_dict=None, - page=1, - page_size=None, - limit_days=None, - key_type=None, - personalisation=False, - include_jobs=False): +def get_notifications_for_service( + service_id, + filter_dict=None, + page=1, + page_size=None, + limit_days=None, + key_type=None, + personalisation=False, + include_jobs=False, + include_from_test_key=False +): if page_size is None: page_size = current_app.config['PAGE_SIZE'] @@ -250,7 +253,7 @@ def get_notifications_for_service(service_id, if key_type is not None: filters.append(Notification.key_type == key_type) - else: + elif not include_from_test_key: filters.append(Notification.key_type != KEY_TYPE_TEST) query = Notification.query.filter(*filters) diff --git a/app/delivery/__init__.py b/app/delivery/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/app/delivery/rest.py b/app/delivery/rest.py new file mode 100644 index 000000000..0bacb43bb --- /dev/null +++ b/app/delivery/rest.py @@ -0,0 +1,46 @@ +from flask import Blueprint, jsonify + +from app.delivery import send_to_providers +from app.models import EMAIL_TYPE +from app.celery import provider_tasks +from app.dao import notifications_dao +from flask import current_app + +delivery_blueprint = Blueprint('delivery', __name__) + +from app.errors import register_errors + +register_errors(delivery_blueprint) + + +@delivery_blueprint.route('/deliver/notification/', methods=['POST']) +def send_notification_to_provider(notification_id): + notification = notifications_dao.get_notification_by_id(notification_id) + if not notification: + return jsonify({"result": "error", "message": "No result found"}), 404 + + if notification.notification_type == EMAIL_TYPE: + send_response( + send_to_providers.send_email_to_provider, + provider_tasks.deliver_email, + notification, + 'send-email') + else: + send_response( + send_to_providers.send_sms_to_provider, + provider_tasks.deliver_sms, + notification, + 'send-sms') + return jsonify({}), 204 + + +def send_response(send_call, task_call, notification, queue): + try: + send_call(notification) + except Exception as e: + current_app.logger.exception( + "Failed to send notification, retrying in celery. ID {} type {}".format( + notification.id, + notification.notification_type), + e) + task_call.apply_async((str(notification.id)), queue=queue) diff --git a/app/delivery/send_to_providers.py b/app/delivery/send_to_providers.py new file mode 100644 index 000000000..8c7c021a8 --- /dev/null +++ b/app/delivery/send_to_providers.py @@ -0,0 +1,135 @@ +from datetime import datetime + +from flask import current_app +from notifications_utils.recipients import ( + validate_and_format_phone_number +) +from notifications_utils.template import Template, get_sms_fragment_count +from notifications_utils.renderers import HTMLEmail, PlainTextEmail, SMSMessage + +from app import clients, statsd_client, create_uuid +from app.dao.notifications_dao import dao_update_notification +from app.dao.provider_details_dao import get_provider_details_by_notification_type +from app.dao.services_dao import dao_fetch_service_by_id +from app.celery.research_mode_tasks import send_sms_response, send_email_response +from app.dao.templates_dao import dao_get_template_by_id + +from app.models import SMS_TYPE, KEY_TYPE_TEST, BRANDING_ORG, EMAIL_TYPE + + +def send_sms_to_provider(notification): + service = dao_fetch_service_by_id(notification.service_id) + provider = provider_to_use(SMS_TYPE, notification.id) + if notification.status == 'created': + template_model = dao_get_template_by_id(notification.template_id, notification.template_version) + template = Template( + template_model.__dict__, + values={} if not notification.personalisation else notification.personalisation, + renderer=SMSMessage(prefix=service.name, sender=service.sms_sender) + ) + if service.research_mode or notification.key_type == KEY_TYPE_TEST: + send_sms_response.apply_async( + (provider.get_name(), str(notification.id), notification.to), queue='research-mode' + ) + notification.billable_units = 0 + else: + provider.send_sms( + to=validate_and_format_phone_number(notification.to), + content=template.replaced, + reference=str(notification.id), + sender=service.sms_sender + ) + notification.billable_units = get_sms_fragment_count(template.replaced_content_count) + + notification.sent_at = datetime.utcnow() + notification.sent_by = provider.get_name() + notification.status = 'sending' + dao_update_notification(notification) + + current_app.logger.info( + "SMS {} sent to provider at {}".format(notification.id, notification.sent_at) + ) + delta_milliseconds = (datetime.utcnow() - notification.created_at).total_seconds() * 1000 + statsd_client.timing("sms.total-time", delta_milliseconds) + + +def send_email_to_provider(notification): + service = dao_fetch_service_by_id(notification.service_id) + provider = provider_to_use(EMAIL_TYPE, notification.id) + if notification.status == 'created': + template_dict = dao_get_template_by_id(notification.template_id, notification.template_version).__dict__ + + html_email = Template( + template_dict, + values=notification.personalisation, + renderer=get_html_email_renderer(service) + ) + + plain_text_email = Template( + template_dict, + values=notification.personalisation, + renderer=PlainTextEmail() + ) + + if service.research_mode or notification.key_type == KEY_TYPE_TEST: + reference = str(create_uuid()) + send_email_response.apply_async( + (provider.get_name(), reference, notification.to), queue='research-mode' + ) + notification.billable_units = 0 + else: + from_address = '"{}" <{}@{}>'.format(service.name, service.email_from, + current_app.config['NOTIFY_EMAIL_DOMAIN']) + reference = provider.send_email( + from_address, + notification.to, + plain_text_email.replaced_subject, + body=plain_text_email.replaced, + html_body=html_email.replaced, + reply_to_address=service.reply_to_email_address, + ) + + notification.reference = reference + notification.sent_at = datetime.utcnow() + notification.sent_by = provider.get_name(), + notification.status = 'sending' + dao_update_notification(notification) + + current_app.logger.info( + "Email {} sent to provider at {}".format(notification.id, notification.sent_at) + ) + delta_milliseconds = (datetime.utcnow() - notification.created_at).total_seconds() * 1000 + statsd_client.timing("email.total-time", delta_milliseconds) + + +def provider_to_use(notification_type, notification_id): + active_providers_in_order = [ + provider for provider in get_provider_details_by_notification_type(notification_type) if provider.active + ] + + if not active_providers_in_order: + current_app.logger.error( + "{} {} failed as no active providers".format(notification_type, notification_id) + ) + raise Exception("No active {} providers".format(notification_type)) + + return clients.get_client_by_name_and_type(active_providers_in_order[0].identifier, notification_type) + + +def get_html_email_renderer(service): + govuk_banner = service.branding != BRANDING_ORG + if service.organisation: + logo = '{}{}{}'.format( + current_app.config['ADMIN_BASE_URL'], + current_app.config['BRANDING_PATH'], + service.organisation.logo + ) + branding = { + 'brand_colour': service.organisation.colour, + 'brand_logo': logo, + 'brand_name': service.organisation.name, + } + else: + branding = {} + + return HTMLEmail(govuk_banner=govuk_banner, **branding) diff --git a/app/job/rest.py b/app/job/rest.py index 9f9ef0047..3a8a60219 100644 --- a/app/job/rest.py +++ b/app/job/rest.py @@ -96,20 +96,16 @@ def get_jobs_by_service(service_id): if request.args.get('limit_days'): try: limit_days = int(request.args['limit_days']) - except ValueError as e: + except ValueError: errors = {'limit_days': ['{} is not an integer'.format(request.args['limit_days'])]} raise InvalidRequest(errors, status_code=400) else: limit_days = None - jobs = dao_get_jobs_by_service_id(service_id, limit_days) - data = job_schema.dump(jobs, many=True).data + statuses = [x.strip() for x in request.args.get('statuses', '').split(',')] - for job_data in data: - statistics = dao_get_notification_outcomes_for_job(service_id, job_data['id']) - job_data['statistics'] = [{'status': statistic[1], 'count': statistic[0]} for statistic in statistics] - - return jsonify(data=data) + page = int(request.args.get('page', 1)) + return jsonify(**get_paginated_jobs(service_id, limit_days, statuses, page)) @job.route('', methods=['POST']) @@ -144,3 +140,28 @@ def create_job(service_id): job_json['statistics'] = [] return jsonify(data=job_json), 201 + + +def get_paginated_jobs(service_id, limit_days, statuses, page): + pagination = dao_get_jobs_by_service_id( + service_id, + limit_days=limit_days, + page=page, + page_size=current_app.config['PAGE_SIZE'], + statuses=statuses + ) + data = job_schema.dump(pagination.items, many=True).data + for job_data in data: + statistics = dao_get_notification_outcomes_for_job(service_id, job_data['id']) + job_data['statistics'] = [{'status': statistic[1], 'count': statistic[0]} for statistic in statistics] + + return { + 'data': data, + 'page_size': pagination.per_page, + 'total': pagination.total, + 'links': pagination_links( + pagination, + '.get_jobs_by_service', + service_id=service_id + ) + } diff --git a/app/models.py b/app/models.py index b39de532d..2ed7cd771 100644 --- a/app/models.py +++ b/app/models.py @@ -352,6 +352,14 @@ JOB_STATUS_FINISHED = 'finished' JOB_STATUS_SENDING_LIMITS_EXCEEDED = 'sending limits exceeded' JOB_STATUS_SCHEDULED = 'scheduled' JOB_STATUS_CANCELLED = 'cancelled' +JOB_STATUS_TYPES = [ + JOB_STATUS_PENDING, + JOB_STATUS_IN_PROGRESS, + JOB_STATUS_FINISHED, + JOB_STATUS_SENDING_LIMITS_EXCEEDED, + JOB_STATUS_SCHEDULED, + JOB_STATUS_CANCELLED +] class JobStatus(db.Model): diff --git a/app/notifications/rest.py b/app/notifications/rest.py index 269da9bef..48172aedc 100644 --- a/app/notifications/rest.py +++ b/app/notifications/rest.py @@ -205,6 +205,8 @@ def send_notification(notification_type): notification, errors = ( sms_template_notification_schema if notification_type == SMS_TYPE else email_notification_schema ).load(request.get_json()) + if errors: + raise InvalidRequest(errors, status_code=400) if all((api_user.key_type != KEY_TYPE_TEST, service.restricted)): service_stats = sum(row.count for row in dao_fetch_todays_stats_for_service(service.id)) @@ -212,13 +214,16 @@ def send_notification(notification_type): error = 'Exceeded send limits ({}) for today'.format(service.message_limit) raise InvalidRequest(error, status_code=429) - if errors: - raise InvalidRequest(errors, status_code=400) - template = templates_dao.dao_get_template_by_id_and_service_id( template_id=notification['template'], service_id=service_id ) + + if notification_type != template.template_type: + raise InvalidRequest("{0} template is not suitable for {1} notification".format(template.template_type, + notification_type), + status_code=400) + errors = unarchived_template_schema.validate({'archived': template.archived}) if errors: raise InvalidRequest(errors, status_code=400) diff --git a/app/schemas.py b/app/schemas.py index ccabfc9d6..5d50e9443 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -315,6 +315,16 @@ class NotificationWithTemplateSchema(BaseSchema): ) job = fields.Nested(JobSchema, only=["id", "original_file_name"], dump_only=True) personalisation = fields.Dict(required=False) + key_type = field_for(models.Notification, 'key_type', required=True) + key_name = fields.String() + + @pre_dump + def add_api_key_name(self, in_data): + if in_data.api_key: + in_data.key_name = in_data.api_key.name + else: + in_data.key_name = None + return in_data class NotificationWithPersonalisationSchema(NotificationWithTemplateSchema): @@ -422,6 +432,7 @@ class NotificationsFilterSchema(ma.Schema): page_size = fields.Int(required=False) limit_days = fields.Int(required=False) include_jobs = fields.Boolean(required=False) + include_from_test_key = fields.Boolean(required=False) @pre_load def handle_multidict(self, in_data): diff --git a/app/service/rest.py b/app/service/rest.py index 966af79f1..1da828067 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -224,6 +224,7 @@ def get_all_notifications_for_service(service_id): page_size = data['page_size'] if 'page_size' in data else current_app.config.get('PAGE_SIZE') limit_days = data.get('limit_days') include_jobs = data.get('include_jobs', True) + include_from_test_key = data.get('include_from_test_key', False) pagination = notifications_dao.get_notifications_for_service( service_id, @@ -231,7 +232,9 @@ def get_all_notifications_for_service(service_id): page=page, page_size=page_size, limit_days=limit_days, - include_jobs=include_jobs) + include_jobs=include_jobs, + include_from_test_key=include_from_test_key + ) kwargs = request.args.to_dict() kwargs['service_id'] = service_id return jsonify( diff --git a/app/utils.py b/app/utils.py index fb65fef48..f67643290 100644 --- a/app/utils.py +++ b/app/utils.py @@ -4,7 +4,7 @@ from flask import url_for def pagination_links(pagination, endpoint, **kwargs): if 'page' in kwargs: kwargs.pop('page', None) - links = dict() + links = {} if pagination.has_prev: links['prev'] = url_for(endpoint, page=pagination.prev_num, **kwargs) if pagination.has_next: diff --git a/tests/app/authentication/test_authentication.py b/tests/app/authentication/test_authentication.py index 057760585..3b9ff1009 100644 --- a/tests/app/authentication/test_authentication.py +++ b/tests/app/authentication/test_authentication.py @@ -53,7 +53,7 @@ def test_should_not_allow_invalid_secret(notify_api, sample_api_key): ) assert response.status_code == 403 data = json.loads(response.get_data()) - assert data['message'] == {"token": ['Invalid token: signature']} + assert data['message'] == {"token": ['Invalid token: signature, api token is not valid']} def test_should_allow_valid_token(notify_api, sample_api_key): @@ -67,6 +67,20 @@ def test_should_allow_valid_token(notify_api, sample_api_key): assert response.status_code == 200 +def test_should_not_allow_service_id_that_is_not_the_wrong_data_type(notify_api, sample_api_key): + with notify_api.test_request_context(): + with notify_api.test_client() as client: + token = create_jwt_token(secret=get_unsigned_secrets(sample_api_key.service_id)[0], + client_id=str('not-a-valid-id')) + response = client.get( + '/service', + headers={'Authorization': "Bearer {}".format(token)} + ) + assert response.status_code == 403 + data = json.loads(response.get_data()) + assert data['message'] == {"token": ['Invalid token: service id is not the right data type']} + + def test_should_allow_valid_token_for_request_with_path_params(notify_api, sample_api_key): with notify_api.test_request_context(): with notify_api.test_client() as client: @@ -95,8 +109,6 @@ def test_should_allow_valid_token_when_service_has_multiple_keys(notify_api, sam def test_authentication_passes_admin_client_token(notify_api, - notify_db, - notify_db_session, sample_api_key): with notify_api.test_request_context(): with notify_api.test_client() as client: @@ -173,8 +185,7 @@ def test_authentication_returns_token_expired_when_service_uses_expired_key_and_ def test_authentication_returns_error_when_admin_client_has_no_secrets(notify_api, - notify_db, - notify_db_session): + sample_service): with notify_api.test_request_context(): with notify_api.test_client() as client: api_secret = notify_api.config.get('ADMIN_CLIENT_SECRET') @@ -194,17 +205,14 @@ def test_authentication_returns_error_when_admin_client_has_no_secrets(notify_ap def test_authentication_returns_error_when_service_doesnt_exit( notify_api, - notify_db, - notify_db_session, - sample_service, - fake_uuid + sample_api_key ): with notify_api.test_request_context(), notify_api.test_client() as client: # get service ID and secret the wrong way around token = create_jwt_token( - secret=str(sample_service.id), - client_id=fake_uuid - ) + secret=str(sample_api_key.service_id), + client_id=str(sample_api_key.id)) + response = client.get( '/service', headers={'Authorization': 'Bearer {}'.format(token)} @@ -215,8 +223,6 @@ def test_authentication_returns_error_when_service_doesnt_exit( def test_authentication_returns_error_when_service_has_no_secrets(notify_api, - notify_db, - notify_db_session, sample_service, fake_uuid): with notify_api.test_request_context(): @@ -248,8 +254,6 @@ def test_should_attach_the_current_api_key_to_current_app(notify_api, sample_ser def test_should_return_403_when_token_is_expired(notify_api, - notify_db, - notify_db_session, sample_api_key): with notify_api.test_request_context(): with notify_api.test_client() as client: diff --git a/tests/app/celery/test_provider_tasks.py b/tests/app/celery/test_provider_tasks.py index b6ae54637..a2af75d36 100644 --- a/tests/app/celery/test_provider_tasks.py +++ b/tests/app/celery/test_provider_tasks.py @@ -1,28 +1,10 @@ -import uuid -from datetime import datetime - -import pytest from celery.exceptions import MaxRetriesExceededError -from unittest.mock import ANY -from notifications_utils.recipients import validate_phone_number, format_phone_number - -import app -from app import mmg_client from app.celery import provider_tasks -from app.celery.provider_tasks import send_sms_to_provider, send_email_to_provider -from app.celery.research_mode_tasks import send_sms_response, send_email_response +from app.celery.provider_tasks import send_sms_to_provider, send_email_to_provider, deliver_sms, deliver_email from app.clients.email import EmailClientException -from app.clients.sms import SmsClientException -from app.dao import notifications_dao, provider_details_dao -from app.models import ( - Notification, - Organisation, - KEY_TYPE_NORMAL, - KEY_TYPE_TEST, - BRANDING_ORG, - BRANDING_BOTH, - KEY_TYPE_TEAM) +from app.models import Notification from tests.app.conftest import sample_notification +import app def test_should_have_decorated_tasks_functions(): @@ -58,250 +40,119 @@ def test_should_by_240_minute_delay_on_retry_two(): assert provider_tasks.retry_iteration_to_delay(4) == 14400 -def test_should_return_highest_priority_active_provider(notify_db, notify_db_session): - providers = provider_details_dao.get_provider_details_by_notification_type('sms') - - first = providers[0] - second = providers[1] - - assert provider_tasks.provider_to_use('sms', '1234').name == first.identifier - - first.priority = 20 - second.priority = 10 - - provider_details_dao.dao_update_provider_details(first) - provider_details_dao.dao_update_provider_details(second) - - assert provider_tasks.provider_to_use('sms', '1234').name == second.identifier - - first.priority = 10 - first.active = False - second.priority = 20 - - provider_details_dao.dao_update_provider_details(first) - provider_details_dao.dao_update_provider_details(second) - - assert provider_tasks.provider_to_use('sms', '1234').name == second.identifier - - first.active = True - provider_details_dao.dao_update_provider_details(first) - - assert provider_tasks.provider_to_use('sms', '1234').name == first.identifier - - -def test_should_send_personalised_template_to_correct_sms_provider_and_persist( - notify_db, - notify_db_session, - sample_template_with_placeholders, - mocker -): - db_notification = sample_notification(notify_db, notify_db_session, template=sample_template_with_placeholders, - to_field="+447234123123", personalisation={"name": "Jo"}, - status='created') - - mocker.patch('app.mmg_client.send_sms') - mocker.patch('app.mmg_client.get_name', return_value="mmg") - - send_sms_to_provider( - db_notification.service_id, - db_notification.id - ) - - mmg_client.send_sms.assert_called_once_with( - to=format_phone_number(validate_phone_number("+447234123123")), - content="Sample service: Hello Jo\nYour thing is due soon", - reference=str(db_notification.id), - sender=None - ) - notification = Notification.query.filter_by(id=db_notification.id).one() - - assert notification.status == 'sending' - assert notification.sent_at <= datetime.utcnow() - assert notification.sent_by == 'mmg' - assert notification.billable_units == 1 - assert notification.personalisation == {"name": "Jo"} - - -def test_should_send_personalised_template_to_correct_email_provider_and_persist( - notify_db, - notify_db_session, - sample_email_template_with_placeholders, - mocker -): - db_notification = sample_notification( - notify_db=notify_db, notify_db_session=notify_db_session, - template=sample_email_template_with_placeholders, - to_field="jo.smith@example.com", - personalisation={'name': 'Jo'} - ) - - mocker.patch('app.aws_ses_client.send_email', return_value='reference') - mocker.patch('app.aws_ses_client.get_name', return_value="ses") - - send_email_to_provider( - db_notification.service_id, - db_notification.id - ) - - app.aws_ses_client.send_email.assert_called_once_with( - '"Sample service" ', - 'jo.smith@example.com', - 'Jo', - body='Hello Jo\nThis is an email from GOV.\u200bUK', - html_body=ANY, - reply_to_address=None - ) - assert ' version_on_notification - - send_sms_to_provider( - db_notification.service_id, - db_notification.id - ) - - mmg_client.send_sms.assert_called_once_with( - to=format_phone_number(validate_phone_number("+447234123123")), - content="Sample service: This is a template:\nwith a newline", - reference=str(db_notification.id), - sender=None - ) - - persisted_notification = notifications_dao.get_notification_by_id(db_notification.id) - assert persisted_notification.to == db_notification.to - assert persisted_notification.template_id == sample_template.id - assert persisted_notification.template_version == version_on_notification - assert persisted_notification.template_version != sample_template.version - assert persisted_notification.status == 'sending' - assert not persisted_notification.personalisation + deliver_sms(sample_notification.id) + app.delivery.send_to_providers.send_sms_to_provider.assert_called_with(sample_notification) -@pytest.mark.parametrize('research_mode,key_type', [ - (True, KEY_TYPE_NORMAL), - (False, KEY_TYPE_TEST) -]) -def test_should_call_send_sms_response_task_if_research_mode(notify_db, sample_service, sample_notification, mocker, - research_mode, key_type): - mocker.patch('app.mmg_client.send_sms') - mocker.patch('app.mmg_client.get_name', return_value="mmg") - mocker.patch('app.celery.research_mode_tasks.send_sms_response.apply_async') +def test_should_add_to_retry_queue_if_notification_not_found_in_deliver_sms_task( + notify_db, + notify_db_session, + mocker): + mocker.patch('app.delivery.send_to_providers.send_sms_to_provider') + mocker.patch('app.celery.provider_tasks.deliver_sms.retry') - if research_mode: - sample_service.research_mode = True - notify_db.session.add(sample_service) - notify_db.session.commit() + notification_id = app.create_uuid() - sample_notification.key_type = key_type - - send_sms_to_provider( - sample_notification.service_id, - sample_notification.id - ) - assert not mmg_client.send_sms.called - send_sms_response.apply_async.assert_called_once_with( - ('mmg', str(sample_notification.id), sample_notification.to), queue='research-mode' - ) - - persisted_notification = notifications_dao.get_notification_by_id(sample_notification.id) - assert persisted_notification.to == sample_notification.to - assert persisted_notification.template_id == sample_notification.template_id - assert persisted_notification.status == 'sending' - assert persisted_notification.sent_at <= datetime.utcnow() - assert persisted_notification.sent_by == 'mmg' - assert not persisted_notification.personalisation + deliver_sms(notification_id) + app.delivery.send_to_providers.send_sms_to_provider.assert_not_called() + app.celery.provider_tasks.deliver_sms.retry.assert_called_with(queue="retry", countdown=10) -@pytest.mark.parametrize('research_mode,key_type, billable_units', [ - (True, KEY_TYPE_NORMAL, 0), - (False, KEY_TYPE_NORMAL, 1), - (False, KEY_TYPE_TEST, 0), - (True, KEY_TYPE_TEST, 0), - (True, KEY_TYPE_TEAM, 0), - (False, KEY_TYPE_TEAM, 1) -]) -def test_should_update_billable_units_according_to_research_mode_and_key_type(notify_db, - sample_service, - sample_notification, - mocker, - research_mode, - key_type, - billable_units): +def test_should_call_send_sms_to_provider_from_send_sms_to_provider_task( + notify_db, + notify_db_session, + sample_notification, + mocker): + mocker.patch('app.delivery.send_to_providers.send_sms_to_provider') - assert Notification.query.count() == 1 - - mocker.patch('app.mmg_client.send_sms') - mocker.patch('app.mmg_client.get_name', return_value="mmg") - mocker.patch('app.celery.research_mode_tasks.send_sms_response.apply_async') - if research_mode: - sample_service.research_mode = True - notify_db.session.add(sample_service) - notify_db.session.commit() - sample_notification.key_type = key_type - - send_sms_to_provider( - sample_notification.service_id, - sample_notification.id - ) - - assert Notification.query.get(sample_notification.id).billable_units == billable_units, \ - "Research mode: {0}, key type: {1}, billable_units: {2}".format(research_mode, key_type, billable_units) + send_sms_to_provider(sample_notification.service_id, sample_notification.id) + app.delivery.send_to_providers.send_sms_to_provider.assert_called_with(sample_notification) -def test_should_not_send_to_provider_when_status_is_not_created(notify_db, notify_db_session, - sample_service, - mocker): - notification = sample_notification(notify_db=notify_db, notify_db_session=notify_db_session, - service=sample_service, - status='sending') - mocker.patch('app.mmg_client.send_sms') - mocker.patch('app.mmg_client.get_name', return_value="mmg") - mocker.patch('app.celery.research_mode_tasks.send_sms_response.apply_async') +def test_should_add_to_retry_queue_if_notification_not_found_in_send_sms_to_provider_task( + notify_db, + notify_db_session, + mocker): + mocker.patch('app.delivery.send_to_providers.send_sms_to_provider') + mocker.patch('app.celery.provider_tasks.send_sms_to_provider.retry') - send_sms_to_provider( - notification.service_id, - notification.id - ) + notification_id = app.create_uuid() + service_id = app.create_uuid() - app.mmg_client.send_sms.assert_not_called() - app.celery.research_mode_tasks.send_sms_response.apply_async.assert_not_called() + send_sms_to_provider(service_id, notification_id) + app.delivery.send_to_providers.send_sms_to_provider.assert_not_called() + app.celery.provider_tasks.send_sms_to_provider.retry.assert_called_with(queue="retry", countdown=10) -def test_should_go_into_technical_error_if_exceeds_retries( +def test_should_call_send_email_to_provider_from_deliver_email_task( + notify_db, + notify_db_session, + sample_notification, + mocker): + mocker.patch('app.delivery.send_to_providers.send_email_to_provider') + + deliver_email(sample_notification.id) + app.delivery.send_to_providers.send_email_to_provider.assert_called_with(sample_notification) + + +def test_should_add_to_retry_queue_if_notification_not_found_in_deliver_email_task( + notify_db, + notify_db_session, + mocker): + mocker.patch('app.delivery.send_to_providers.send_email_to_provider') + mocker.patch('app.celery.provider_tasks.deliver_email.retry') + + notification_id = app.create_uuid() + + deliver_email(notification_id) + app.delivery.send_to_providers.send_email_to_provider.assert_not_called() + app.celery.provider_tasks.deliver_email.retry.assert_called_with(queue="retry", countdown=10) + + +def test_should_call_send_email_to_provider_from_email_task( + notify_db, + notify_db_session, + sample_notification, + mocker): + mocker.patch('app.delivery.send_to_providers.send_email_to_provider') + + send_email_to_provider(sample_notification.service_id, sample_notification.id) + app.delivery.send_to_providers.send_email_to_provider.assert_called_with(sample_notification) + + +def test_should_add_to_retry_queue_if_notification_not_found_in_send_email_to_provider_task( + notify_db, + notify_db_session, + mocker): + mocker.patch('app.delivery.send_to_providers.send_email_to_provider') + mocker.patch('app.celery.provider_tasks.send_email_to_provider.retry') + + notification_id = app.create_uuid() + service_id = app.create_uuid() + + send_email_to_provider(service_id, notification_id) + app.delivery.send_to_providers.send_email_to_provider.assert_not_called() + app.celery.provider_tasks.send_email_to_provider.retry.assert_called_with(queue="retry", countdown=10) + + +# DO THESE FOR THE 4 TYPES OF TASK + +def test_should_go_into_technical_error_if_exceeds_retries_on_send_sms_to_provider_task( notify_db, notify_db_session, sample_service, mocker): - notification = sample_notification(notify_db=notify_db, notify_db_session=notify_db_session, service=sample_service, status='created') - mocker.patch('app.mmg_client.send_sms', side_effect=SmsClientException("EXPECTED")) + mocker.patch('app.delivery.send_to_providers.send_sms_to_provider', side_effect=Exception("EXPECTED")) mocker.patch('app.celery.provider_tasks.send_sms_to_provider.retry', side_effect=MaxRetriesExceededError()) send_sms_to_provider( @@ -315,92 +166,33 @@ def test_should_go_into_technical_error_if_exceeds_retries( assert db_notification.status == 'technical-failure' -def test_should_send_sms_sender_from_service_if_present( +def test_should_go_into_technical_error_if_exceeds_retries_on_deliver_sms_task( notify_db, notify_db_session, sample_service, - sample_template, mocker): - db_notification = sample_notification(notify_db, notify_db_session, template=sample_template, - to_field="+447234123123", - status='created') - - sample_service.sms_sender = 'elevenchars' - notify_db.session.add(sample_service) - notify_db.session.commit() - - mocker.patch('app.mmg_client.send_sms') - mocker.patch('app.mmg_client.get_name', return_value="mmg") - - send_sms_to_provider( - db_notification.service_id, - db_notification.id - ) - - mmg_client.send_sms.assert_called_once_with( - to=format_phone_number(validate_phone_number("+447234123123")), - content="This is a template:\nwith a newline", - reference=str(db_notification.id), - sender=sample_service.sms_sender - ) - - -@pytest.mark.parametrize('research_mode,key_type', [ - (True, KEY_TYPE_NORMAL), - (False, KEY_TYPE_TEST) -]) -def test_send_email_to_provider_should_call_research_mode_task_response_task_if_research_mode( - notify_db, - notify_db_session, - sample_service, - sample_email_template, - ses_provider, - mocker, - research_mode, - key_type): notification = sample_notification(notify_db=notify_db, notify_db_session=notify_db_session, - template=sample_email_template, - to_field="john@smith.com", - key_type=key_type - ) + service=sample_service, status='created') - reference = uuid.uuid4() - mocker.patch('app.uuid.uuid4', return_value=reference) - mocker.patch('app.aws_ses_client.send_email') - mocker.patch('app.aws_ses_client.get_name', return_value="ses") - mocker.patch('app.celery.research_mode_tasks.send_email_response.apply_async') + mocker.patch('app.delivery.send_to_providers.send_sms_to_provider', side_effect=Exception("EXPECTED")) + mocker.patch('app.celery.provider_tasks.deliver_sms.retry', side_effect=MaxRetriesExceededError()) - if research_mode: - sample_service.research_mode = True - notify_db.session.add(sample_service) - notify_db.session.commit() - send_email_to_provider( - sample_service.id, + deliver_sms( notification.id ) - assert not app.aws_ses_client.send_email.called - send_email_response.apply_async.assert_called_once_with( - ('ses', str(reference), 'john@smith.com'), queue="research-mode" - ) - persisted_notification = Notification.query.filter_by(id=notification.id).one() - assert persisted_notification.to == 'john@smith.com' - assert persisted_notification.template_id == sample_email_template.id - assert persisted_notification.status == 'sending' - assert persisted_notification.sent_at <= datetime.utcnow() - assert persisted_notification.created_at <= datetime.utcnow() - assert persisted_notification.sent_by == 'ses' - assert persisted_notification.reference == str(reference) - assert persisted_notification.billable_units == 0 + provider_tasks.deliver_sms.retry.assert_called_with(queue='retry', countdown=10) + + db_notification = Notification.query.filter_by(id=notification.id).one() + assert db_notification.status == 'technical-failure' -def test_send_email_to_provider_should_go_into_technical_error_if_exceeds_retries( +def test_send_email_to_provider_should_go_into_technical_error_if_exceeds_retries_on_send_email_to_provider_task( notify_db, notify_db_session, sample_service, sample_email_template, mocker): - notification = sample_notification(notify_db=notify_db, notify_db_session=notify_db_session, service=sample_service, status='created', template=sample_email_template) @@ -418,86 +210,22 @@ def test_send_email_to_provider_should_go_into_technical_error_if_exceeds_retrie assert db_notification.status == 'technical-failure' -def test_send_email_to_provider_should_not_send_to_provider_when_status_is_not_created(notify_db, notify_db_session, - sample_service, - sample_email_template, - mocker): +def test_should_go_into_technical_error_if_exceeds_retries_on_deliver_email_task( + notify_db, + notify_db_session, + sample_service, + mocker): notification = sample_notification(notify_db=notify_db, notify_db_session=notify_db_session, - template=sample_email_template, - service=sample_service, - status='sending') - mocker.patch('app.aws_ses_client.send_email') - mocker.patch('app.aws_ses_client.get_name', return_value="ses") - mocker.patch('app.celery.research_mode_tasks.send_email_response.apply_async') + service=sample_service, status='created') - send_sms_to_provider( - notification.service_id, + mocker.patch('app.delivery.send_to_providers.send_email_to_provider', side_effect=Exception("EXPECTED")) + mocker.patch('app.celery.provider_tasks.deliver_email.retry', side_effect=MaxRetriesExceededError()) + + deliver_email( notification.id ) - app.aws_ses_client.send_email.assert_not_called() - app.celery.research_mode_tasks.send_email_response.apply_async.assert_not_called() + provider_tasks.deliver_email.retry.assert_called_with(queue='retry', countdown=10) - -def test_send_email_should_use_service_reply_to_email( - notify_db, notify_db_session, - sample_service, - sample_email_template, - mocker): - mocker.patch('app.aws_ses_client.send_email', return_value='reference') - mocker.patch('app.aws_ses_client.get_name', return_value="ses") - - db_notification = sample_notification(notify_db, notify_db_session, template=sample_email_template) - sample_service.reply_to_email_address = 'foo@bar.com' - - send_email_to_provider( - db_notification.service_id, - db_notification.id, - ) - - app.aws_ses_client.send_email.assert_called_once_with( - ANY, - ANY, - ANY, - body=ANY, - html_body=ANY, - reply_to_address=sample_service.reply_to_email_address - ) - - -def test_get_html_email_renderer_should_return_for_normal_service(sample_service): - renderer = provider_tasks.get_html_email_renderer(sample_service) - assert renderer.govuk_banner - assert renderer.brand_colour is None - assert renderer.brand_logo is None - assert renderer.brand_name is None - - -@pytest.mark.parametrize('branding_type, govuk_banner', [ - (BRANDING_ORG, False), - (BRANDING_BOTH, True) -]) -def test_get_html_email_renderer_with_branding_details(branding_type, govuk_banner, notify_db, sample_service): - sample_service.branding = branding_type - org = Organisation(colour='#000000', logo='justice-league.png', name='Justice League') - sample_service.organisation = org - notify_db.session.add_all([sample_service, org]) - notify_db.session.commit() - - renderer = provider_tasks.get_html_email_renderer(sample_service) - - assert renderer.govuk_banner == govuk_banner - assert renderer.brand_colour == '000000' - assert renderer.brand_name == 'Justice League' - - -def test_get_html_email_renderer_prepends_logo_path(notify_db, sample_service): - sample_service.branding = BRANDING_ORG - org = Organisation(colour='#000000', logo='justice-league.png', name='Justice League') - sample_service.organisation = org - notify_db.session.add_all([sample_service, org]) - notify_db.session.commit() - - renderer = provider_tasks.get_html_email_renderer(sample_service) - - assert renderer.brand_logo == 'http://localhost:6012/static/images/email-template/crests/justice-league.png' + db_notification = Notification.query.filter_by(id=notification.id).one() + assert db_notification.status == 'technical-failure' diff --git a/tests/app/clients/test_firetext.py b/tests/app/clients/test_firetext.py index ea8630913..80737e23d 100644 --- a/tests/app/clients/test_firetext.py +++ b/tests/app/clients/test_firetext.py @@ -1,9 +1,10 @@ +from requests import HTTPError from urllib.parse import parse_qs import pytest import requests_mock -from app.clients.sms.firetext import (get_firetext_responses, FiretextClientException) +from app.clients.sms.firetext import get_firetext_responses, SmsClientResponseException def test_should_return_correct_details_for_delivery(): @@ -88,12 +89,26 @@ def test_send_sms_raises_if_firetext_rejects(mocker, mock_firetext_client): 'responseData': '' } - with pytest.raises(FiretextClientException) as exc, requests_mock.Mocker() as request_mock: + with pytest.raises(SmsClientResponseException) as exc, requests_mock.Mocker() as request_mock: request_mock.post('https://www.firetext.co.uk/api/sendsms/json', json=response_dict, status_code=200) mock_firetext_client.send_sms(to, content, reference) - assert exc.value.code == 1 - assert exc.value.description == 'Some kind of error' + assert exc.value.status_code == 200 + assert '"description": "Some kind of error"' in exc.value.text + assert '"code": 1' in exc.value.text + + +def test_send_sms_raises_if_firetext_rejects(mocker, mock_firetext_client): + to = content = reference = 'foo' + response_dict = {"something": "gone bad"} + + with pytest.raises(SmsClientResponseException) as exc, requests_mock.Mocker() as request_mock: + request_mock.post('https://www.firetext.co.uk/api/sendsms/json', json=response_dict, status_code=400) + mock_firetext_client.send_sms(to, content, reference) + + assert exc.value.status_code == 400 + assert exc.value.text == '{"something": "gone bad"}' + assert type(exc.value.exception) == HTTPError def test_send_sms_override_configured_shortcode_with_sender(mocker, mock_firetext_client): diff --git a/tests/app/clients/test_mmg.py b/tests/app/clients/test_mmg.py index 649764d89..d87f2f914 100644 --- a/tests/app/clients/test_mmg.py +++ b/tests/app/clients/test_mmg.py @@ -1,8 +1,13 @@ +import json + import pytest import requests_mock -from app import mmg_client +from requests import HTTPError -from app.clients.sms.mmg import (get_mmg_responses, MMGClientException) +from app import mmg_client +from app.clients.sms import SmsClientResponseException + +from app.clients.sms.mmg import get_mmg_responses def test_should_return_correct_details_for_delivery(): @@ -72,12 +77,14 @@ def test_send_sms_raises_if_mmg_rejects(notify_api, mocker): 'Description': 'Some kind of error' } - with pytest.raises(MMGClientException) as exc, requests_mock.Mocker() as request_mock: + with pytest.raises(SmsClientResponseException) as exc, requests_mock.Mocker() as request_mock: request_mock.post('https://api.mmg.co.uk/json/api.php', json=response_dict, status_code=400) mmg_client.send_sms(to, content, reference) - assert exc.value.code == 206 - assert exc.value.description == 'Some kind of error' + assert exc.value.status_code == 400 + assert '"Error": 206' in exc.value.text + assert '"Description": "Some kind of error"' in exc.value.text + assert type(exc.value.exception) == HTTPError def test_send_sms_override_configured_shortcode_with_sender(notify_api, mocker): @@ -93,3 +100,16 @@ def test_send_sms_override_configured_shortcode_with_sender(notify_api, mocker): request_args = request_mock.request_history[0].json() assert request_args['sender'] == 'fromservice' + + +def test_send_sms_raises_if_mmg_fails_to_return_json(notify_api, mocker): + to = content = reference = 'foo' + response_dict = 'NOT AT ALL VALID JSON {"key" : "value"}}' + + with pytest.raises(SmsClientResponseException) as exc, requests_mock.Mocker() as request_mock: + request_mock.post('https://api.mmg.co.uk/json/api.php', text=response_dict, status_code=200) + mmg_client.send_sms(to, content, reference) + + assert 'app.clients.sms.mmg.MMGClientResponseException: Code 200 text NOT AT ALL VALID JSON {"key" : "value"}} exception Expecting value: line 1 column 1 (char 0)' in str(exc) # noqa + assert exc.value.status_code == 200 + assert exc.value.text == 'NOT AT ALL VALID JSON {"key" : "value"}}' diff --git a/tests/app/conftest.py b/tests/app/conftest.py index 217ea4df2..c3b8e1093 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -236,10 +236,11 @@ def sample_email_template_with_placeholders(notify_db, notify_db_session): def sample_api_key(notify_db, notify_db_session, service=None, - key_type=KEY_TYPE_NORMAL): + key_type=KEY_TYPE_NORMAL, + name=None): if service is None: service = sample_service(notify_db, notify_db_session) - data = {'service': service, 'name': uuid.uuid4(), 'created_by': service.created_by, 'key_type': key_type} + data = {'service': service, 'name': name or uuid.uuid4(), 'created_by': service.created_by, 'key_type': key_type} api_key = ApiKey(**data) save_model_api_key(api_key) return api_key @@ -261,7 +262,7 @@ def sample_job(notify_db, service=None, template=None, notification_count=1, - created_at=datetime.utcnow(), + created_at=None, job_status='pending', scheduled_for=None): if service is None: @@ -277,7 +278,7 @@ def sample_job(notify_db, 'template_version': template.version, 'original_file_name': 'some.csv', 'notification_count': notification_count, - 'created_at': created_at, + 'created_at': created_at or datetime.utcnow(), 'created_by': service.created_by, 'job_status': job_status, 'scheduled_for': scheduled_for @@ -445,6 +446,17 @@ def sample_notification(notify_db, return notification +@pytest.fixture(scope='function') +def sample_notification_with_api_key(notify_db, notify_db_session): + notification = sample_notification(notify_db, notify_db_session) + notification.api_key_id = sample_api_key( + notify_db, + notify_db_session, + name='Test key' + ).id + return notification + + @pytest.fixture(scope='function') def sample_email_notification(notify_db, notify_db_session): created_at = datetime.utcnow() diff --git a/tests/app/dao/test_jobs_dao.py b/tests/app/dao/test_jobs_dao.py index ee116c8ca..f9a77192f 100644 --- a/tests/app/dao/test_jobs_dao.py +++ b/tests/app/dao/test_jobs_dao.py @@ -1,5 +1,6 @@ -from datetime import (datetime, timedelta) +from datetime import datetime, timedelta import uuid + from freezegun import freeze_time from app.dao.jobs_dao import ( @@ -10,10 +11,15 @@ from app.dao.jobs_dao import ( dao_get_scheduled_jobs, dao_get_future_scheduled_job_by_id_and_service_id, dao_get_notification_outcomes_for_job, - dao_get_jobs_older_than) - + dao_get_jobs_older_than +) from app.models import Job -from tests.app.conftest import sample_notification, sample_job, sample_service + +from tests.app.conftest import sample_notification as create_notification +from tests.app.conftest import sample_job as create_job +from tests.app.conftest import sample_service as create_service +from tests.app.conftest import sample_template as create_template +from tests.app.conftest import sample_user as create_user def test_should_have_decorated_notifications_dao_functions(): @@ -26,14 +32,14 @@ def test_should_get_all_statuses_for_notifications_associated_with_job( sample_service, sample_job): - sample_notification(notify_db, notify_db_session, service=sample_service, job=sample_job, status='created') - sample_notification(notify_db, notify_db_session, service=sample_service, job=sample_job, status='sending') - sample_notification(notify_db, notify_db_session, service=sample_service, job=sample_job, status='delivered') - sample_notification(notify_db, notify_db_session, service=sample_service, job=sample_job, status='pending') - sample_notification(notify_db, notify_db_session, service=sample_service, job=sample_job, status='failed') - sample_notification(notify_db, notify_db_session, service=sample_service, job=sample_job, status='technical-failure') # noqa - sample_notification(notify_db, notify_db_session, service=sample_service, job=sample_job, status='temporary-failure') # noqa - sample_notification(notify_db, notify_db_session, service=sample_service, job=sample_job, status='permanent-failure') # noqa + create_notification(notify_db, notify_db_session, service=sample_service, job=sample_job, status='created') + create_notification(notify_db, notify_db_session, service=sample_service, job=sample_job, status='sending') + create_notification(notify_db, notify_db_session, service=sample_service, job=sample_job, status='delivered') + create_notification(notify_db, notify_db_session, service=sample_service, job=sample_job, status='pending') + create_notification(notify_db, notify_db_session, service=sample_service, job=sample_job, status='failed') + create_notification(notify_db, notify_db_session, service=sample_service, job=sample_job, status='technical-failure') # noqa + create_notification(notify_db, notify_db_session, service=sample_service, job=sample_job, status='temporary-failure') # noqa + create_notification(notify_db, notify_db_session, service=sample_service, job=sample_job, status='permanent-failure') # noqa results = dao_get_notification_outcomes_for_job(sample_service.id, sample_job.id) assert [(row.count, row.status) for row in results] == [ @@ -53,14 +59,14 @@ def test_should_count_of_statuses_for_notifications_associated_with_job( notify_db_session, sample_service, sample_job): - sample_notification(notify_db, notify_db_session, service=sample_service, job=sample_job, status='created') - sample_notification(notify_db, notify_db_session, service=sample_service, job=sample_job, status='created') - sample_notification(notify_db, notify_db_session, service=sample_service, job=sample_job, status='sending') - sample_notification(notify_db, notify_db_session, service=sample_service, job=sample_job, status='sending') - sample_notification(notify_db, notify_db_session, service=sample_service, job=sample_job, status='sending') - sample_notification(notify_db, notify_db_session, service=sample_service, job=sample_job, status='sending') - sample_notification(notify_db, notify_db_session, service=sample_service, job=sample_job, status='delivered') - sample_notification(notify_db, notify_db_session, service=sample_service, job=sample_job, status='delivered') + create_notification(notify_db, notify_db_session, service=sample_service, job=sample_job, status='created') + create_notification(notify_db, notify_db_session, service=sample_service, job=sample_job, status='created') + create_notification(notify_db, notify_db_session, service=sample_service, job=sample_job, status='sending') + create_notification(notify_db, notify_db_session, service=sample_service, job=sample_job, status='sending') + create_notification(notify_db, notify_db_session, service=sample_service, job=sample_job, status='sending') + create_notification(notify_db, notify_db_session, service=sample_service, job=sample_job, status='sending') + create_notification(notify_db, notify_db_session, service=sample_service, job=sample_job, status='delivered') + create_notification(notify_db, notify_db_session, service=sample_service, job=sample_job, status='delivered') results = dao_get_notification_outcomes_for_job(sample_service.id, sample_job.id) assert [(row.count, row.status) for row in results] == [ @@ -75,11 +81,11 @@ def test_should_return_zero_length_array_if_no_notifications_for_job(sample_serv def test_should_return_notifications_only_for_this_job(notify_db, notify_db_session, sample_service): - job_1 = sample_job(notify_db, notify_db_session, service=sample_service) - job_2 = sample_job(notify_db, notify_db_session, service=sample_service) + job_1 = create_job(notify_db, notify_db_session, service=sample_service) + job_2 = create_job(notify_db, notify_db_session, service=sample_service) - sample_notification(notify_db, notify_db_session, service=sample_service, job=job_1, status='created') - sample_notification(notify_db, notify_db_session, service=sample_service, job=job_2, status='created') + create_notification(notify_db, notify_db_session, service=sample_service, job=job_1, status='created') + create_notification(notify_db, notify_db_session, service=sample_service, job=job_2, status='created') results = dao_get_notification_outcomes_for_job(sample_service.id, job_1.id) assert [(row.count, row.status) for row in results] == [ @@ -88,14 +94,14 @@ def test_should_return_notifications_only_for_this_job(notify_db, notify_db_sess def test_should_return_notifications_only_for_this_service(notify_db, notify_db_session): - service_1 = sample_service(notify_db, notify_db_session, service_name="one", email_from="one") - service_2 = sample_service(notify_db, notify_db_session, service_name="two", email_from="two") + service_1 = create_service(notify_db, notify_db_session, service_name="one", email_from="one") + service_2 = create_service(notify_db, notify_db_session, service_name="two", email_from="two") - job_1 = sample_job(notify_db, notify_db_session, service=service_1) - job_2 = sample_job(notify_db, notify_db_session, service=service_2) + job_1 = create_job(notify_db, notify_db_session, service=service_1) + job_2 = create_job(notify_db, notify_db_session, service=service_2) - sample_notification(notify_db, notify_db_session, service=service_1, job=job_1, status='created') - sample_notification(notify_db, notify_db_session, service=service_2, job=job_2, status='created') + create_notification(notify_db, notify_db_session, service=service_1, job=job_1, status='created') + create_notification(notify_db, notify_db_session, service=service_2, job=job_2, status='created') assert len(dao_get_notification_outcomes_for_job(service_1.id, job_2.id)) == 0 @@ -130,11 +136,6 @@ def test_get_job_by_id(sample_job): def test_get_jobs_for_service(notify_db, notify_db_session, sample_template): - from tests.app.conftest import sample_job as create_job - from tests.app.conftest import sample_service as create_service - from tests.app.conftest import sample_template as create_template - from tests.app.conftest import sample_user as create_user - one_job = create_job(notify_db, notify_db_session, sample_template.service, sample_template) other_user = create_user(notify_db, notify_db_session, email="test@digital.cabinet-office.gov.uk") @@ -143,8 +144,8 @@ def test_get_jobs_for_service(notify_db, notify_db_session, sample_template): other_template = create_template(notify_db, notify_db_session, service=other_service) other_job = create_job(notify_db, notify_db_session, service=other_service, template=other_template) - one_job_from_db = dao_get_jobs_by_service_id(one_job.service_id) - other_job_from_db = dao_get_jobs_by_service_id(other_job.service_id) + one_job_from_db = dao_get_jobs_by_service_id(one_job.service_id).items + other_job_from_db = dao_get_jobs_by_service_id(other_job.service_id).items assert len(one_job_from_db) == 1 assert one_job == one_job_from_db[0] @@ -156,27 +157,23 @@ def test_get_jobs_for_service(notify_db, notify_db_session, sample_template): def test_get_jobs_for_service_with_limit_days_param(notify_db, notify_db_session, sample_template): - from tests.app.conftest import sample_job as create_job - one_job = create_job(notify_db, notify_db_session, sample_template.service, sample_template) old_job = create_job(notify_db, notify_db_session, sample_template.service, sample_template, created_at=datetime.now() - timedelta(days=8)) - jobs = dao_get_jobs_by_service_id(one_job.service_id) + jobs = dao_get_jobs_by_service_id(one_job.service_id).items assert len(jobs) == 2 assert one_job in jobs assert old_job in jobs - jobs_limit_days = dao_get_jobs_by_service_id(one_job.service_id, limit_days=7) + jobs_limit_days = dao_get_jobs_by_service_id(one_job.service_id, limit_days=7).items assert len(jobs_limit_days) == 1 assert one_job in jobs_limit_days assert old_job not in jobs_limit_days def test_get_jobs_for_service_with_limit_days_edge_case(notify_db, notify_db_session, sample_template): - from tests.app.conftest import sample_job as create_job - one_job = create_job(notify_db, notify_db_session, sample_template.service, sample_template) job_two = create_job(notify_db, notify_db_session, sample_template.service, sample_template, created_at=(datetime.now() - timedelta(days=7)).date()) @@ -187,7 +184,7 @@ def test_get_jobs_for_service_with_limit_days_edge_case(notify_db, notify_db_ses job_eight_days_old = create_job(notify_db, notify_db_session, sample_template.service, sample_template, created_at=datetime.now() - timedelta(days=8)) - jobs_limit_days = dao_get_jobs_by_service_id(one_job.service_id, limit_days=7) + jobs_limit_days = dao_get_jobs_by_service_id(one_job.service_id, limit_days=7).items assert len(jobs_limit_days) == 3 assert one_job in jobs_limit_days assert job_two in jobs_limit_days @@ -196,8 +193,6 @@ def test_get_jobs_for_service_with_limit_days_edge_case(notify_db, notify_db_ses def test_get_jobs_for_service_in_created_at_order(notify_db, notify_db_session, sample_template): - from tests.app.conftest import sample_job as create_job - job_1 = create_job( notify_db, notify_db_session, sample_template.service, sample_template, created_at=datetime.utcnow()) job_2 = create_job( @@ -207,7 +202,7 @@ def test_get_jobs_for_service_in_created_at_order(notify_db, notify_db_session, job_4 = create_job( notify_db, notify_db_session, sample_template.service, sample_template, created_at=datetime.utcnow()) - jobs = dao_get_jobs_by_service_id(sample_template.service.id) + jobs = dao_get_jobs_by_service_id(sample_template.service.id).items assert len(jobs) == 4 assert jobs[0].id == job_4.id @@ -231,8 +226,8 @@ def test_update_job(sample_job): def test_get_scheduled_jobs_gets_all_jobs_in_scheduled_state_scheduled_before_now(notify_db, notify_db_session): one_minute_ago = datetime.utcnow() - timedelta(minutes=1) one_hour_ago = datetime.utcnow() - timedelta(minutes=60) - job_new = sample_job(notify_db, notify_db_session, scheduled_for=one_minute_ago, job_status='scheduled') - job_old = sample_job(notify_db, notify_db_session, scheduled_for=one_hour_ago, job_status='scheduled') + job_new = create_job(notify_db, notify_db_session, scheduled_for=one_minute_ago, job_status='scheduled') + job_old = create_job(notify_db, notify_db_session, scheduled_for=one_hour_ago, job_status='scheduled') jobs = dao_get_scheduled_jobs() assert len(jobs) == 2 assert jobs[0].id == job_old.id @@ -241,8 +236,8 @@ def test_get_scheduled_jobs_gets_all_jobs_in_scheduled_state_scheduled_before_no def test_get_scheduled_jobs_gets_ignores_jobs_not_scheduled(notify_db, notify_db_session): one_minute_ago = datetime.utcnow() - timedelta(minutes=1) - sample_job(notify_db, notify_db_session) - job_scheduled = sample_job(notify_db, notify_db_session, scheduled_for=one_minute_ago, job_status='scheduled') + create_job(notify_db, notify_db_session) + job_scheduled = create_job(notify_db, notify_db_session, scheduled_for=one_minute_ago, job_status='scheduled') jobs = dao_get_scheduled_jobs() assert len(jobs) == 1 assert jobs[0].id == job_scheduled.id @@ -263,11 +258,32 @@ def test_should_get_jobs_older_than_seven_days(notify_db, notify_db_session): midnight = datetime(2016, 10, 10, 0, 0, 0, 0) one_millisecond_past_midnight = datetime(2016, 10, 10, 0, 0, 0, 1) - job_1 = sample_job(notify_db, notify_db_session, created_at=one_millisecond_before_midnight) - sample_job(notify_db, notify_db_session, created_at=midnight) - sample_job(notify_db, notify_db_session, created_at=one_millisecond_past_midnight) + job_1 = create_job(notify_db, notify_db_session, created_at=one_millisecond_before_midnight) + create_job(notify_db, notify_db_session, created_at=midnight) + create_job(notify_db, notify_db_session, created_at=one_millisecond_past_midnight) with freeze_time('2016-10-17T00:00:00'): jobs = dao_get_jobs_older_than(7) assert len(jobs) == 1 assert jobs[0].id == job_1.id + + +def test_get_jobs_for_service_is_paginated(notify_db, notify_db_session, sample_service, sample_template): + with freeze_time('2015-01-01T00:00:00') as the_time: + for _ in range(10): + the_time.tick(timedelta(hours=1)) + create_job(notify_db, notify_db_session, sample_service, sample_template) + + res = dao_get_jobs_by_service_id(sample_service.id, page=1, page_size=2) + + assert res.per_page == 2 + assert res.total == 10 + assert len(res.items) == 2 + assert res.items[0].created_at == datetime(2015, 1, 1, 10) + assert res.items[1].created_at == datetime(2015, 1, 1, 9) + + res = dao_get_jobs_by_service_id(sample_service.id, page=2, page_size=2) + + assert len(res.items) == 2 + assert res.items[0].created_at == datetime(2015, 1, 1, 8) + assert res.items[1].created_at == datetime(2015, 1, 1, 7) diff --git a/tests/app/dao/test_notification_dao.py b/tests/app/dao/test_notification_dao.py index 39c91912c..97885f50a 100644 --- a/tests/app/dao/test_notification_dao.py +++ b/tests/app/dao/test_notification_dao.py @@ -1000,11 +1000,15 @@ def test_get_notifications_created_by_api_or_csv_are_returned_correctly_excludin all_notifications = Notification.query.all() assert len(all_notifications) == 4 - # returns all API derived notifications + # returns all real API derived notifications all_notifications = get_notifications_for_service(sample_service.id).items assert len(all_notifications) == 2 - # all notifications including jobs + # returns all API derived notifications, including those created with test key + all_notifications = get_notifications_for_service(sample_service.id, include_from_test_key=True).items + assert len(all_notifications) == 3 + + # all real notifications including jobs all_notifications = get_notifications_for_service(sample_service.id, limit_days=1, include_jobs=True).items assert len(all_notifications) == 3 diff --git a/tests/app/delivery/__init__.py b/tests/app/delivery/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/app/delivery/test_rest.py b/tests/app/delivery/test_rest.py new file mode 100644 index 000000000..fc51fb508 --- /dev/null +++ b/tests/app/delivery/test_rest.py @@ -0,0 +1,105 @@ +from flask import json + +import app +from tests import create_authorization_header + + +def test_should_reject_if_not_authenticated(notify_api): + with notify_api.test_request_context(): + with notify_api.test_client() as client: + response = client.post('/deliver/notification/{}'.format(app.create_uuid())) + assert response.status_code == 401 + + +def test_should_reject_if_invalid_uuid(notify_api): + with notify_api.test_request_context(): + with notify_api.test_client() as client: + auth = create_authorization_header() + response = client.post( + '/deliver/notification/{}', + headers=[auth] + ) + body = json.loads(response.get_data(as_text=True)) + assert response.status_code == 404 + assert body['message'] == 'The requested URL was not found on the server. If you entered the URL manually please check your spelling and try again.' # noqa + assert body['result'] == 'error' + + +def test_should_reject_if_notification_id_cannot_be_found(notify_api): + with notify_api.test_request_context(): + with notify_api.test_client() as client: + auth = create_authorization_header() + response = client.post( + '/deliver/notification/{}'.format(app.create_uuid()), + headers=[auth] + ) + body = json.loads(response.get_data(as_text=True)) + assert response.status_code == 404 + assert body['message'] == 'No result found' + assert body['result'] == 'error' + + +def test_should_call_send_sms_to_provider_as_primary(notify_api, sample_notification, mocker): + mocker.patch('app.delivery.send_to_providers.send_sms_to_provider') + with notify_api.test_request_context(): + with notify_api.test_client() as client: + auth = create_authorization_header() + response = client.post( + '/deliver/notification/{}'.format(sample_notification.id), + headers=[auth] + ) + app.delivery.send_to_providers.send_sms_to_provider.assert_called_with(sample_notification) + assert response.status_code == 204 + + +def test_should_call_send_email_to_provider_as_primary(notify_api, sample_email_notification, mocker): + mocker.patch('app.delivery.send_to_providers.send_email_to_provider') + with notify_api.test_request_context(): + with notify_api.test_client() as client: + auth = create_authorization_header() + response = client.post( + '/deliver/notification/{}'.format(sample_email_notification.id), + headers=[auth] + ) + app.delivery.send_to_providers.send_email_to_provider.assert_called_with(sample_email_notification) + assert response.status_code == 204 + + +def test_should_call_deliver_sms_task_if_send_sms_to_provider_fails(notify_api, sample_notification, mocker): + mocker.patch('app.delivery.send_to_providers.send_sms_to_provider', side_effect=Exception()) + mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async') + + with notify_api.test_request_context(): + with notify_api.test_client() as client: + auth = create_authorization_header() + response = client.post( + '/deliver/notification/{}'.format(sample_notification.id), + headers=[auth] + ) + app.delivery.send_to_providers.send_sms_to_provider.assert_called_with(sample_notification) + app.celery.provider_tasks.deliver_sms.apply_async.assert_called_with( + (str(sample_notification.id)), queue='send-sms' + ) + assert response.status_code == 204 + + +def test_should_call_deliver_email_task_if_send_email_to_provider_fails( + notify_api, + sample_email_notification, + mocker +): + mocker.patch('app.delivery.send_to_providers.send_email_to_provider', side_effect=Exception()) + mocker.patch('app.celery.provider_tasks.deliver_email.apply_async') + + with notify_api.test_request_context(): + with notify_api.test_client() as client: + auth = create_authorization_header() + response = client.post( + '/deliver/notification/{}'.format(sample_email_notification.id), + headers=[auth] + ) + app.delivery.send_to_providers.send_email_to_provider.assert_called_with(sample_email_notification) + app.celery.provider_tasks.deliver_email.apply_async.assert_called_with( + (str(sample_email_notification.id)), queue='send-email' + ) + assert response.status_code == 204 diff --git a/tests/app/delivery/test_send_to_providers.py b/tests/app/delivery/test_send_to_providers.py new file mode 100644 index 000000000..17b16e063 --- /dev/null +++ b/tests/app/delivery/test_send_to_providers.py @@ -0,0 +1,445 @@ +import uuid +from datetime import datetime + +import pytest +from unittest.mock import ANY + +import app +from app import mmg_client +from app.dao import (provider_details_dao, notifications_dao) +from app.delivery import send_to_providers +from app.models import Notification, KEY_TYPE_NORMAL, KEY_TYPE_TEST, BRANDING_ORG, BRANDING_BOTH, Organisation, \ + KEY_TYPE_TEAM +from tests.app.conftest import sample_notification + +from notifications_utils.recipients import validate_phone_number, format_phone_number + + +def test_should_return_highest_priority_active_provider(notify_db, notify_db_session): + providers = provider_details_dao.get_provider_details_by_notification_type('sms') + + first = providers[0] + second = providers[1] + + assert send_to_providers.provider_to_use('sms', '1234').name == first.identifier + + first.priority = 20 + second.priority = 10 + + provider_details_dao.dao_update_provider_details(first) + provider_details_dao.dao_update_provider_details(second) + + assert send_to_providers.provider_to_use('sms', '1234').name == second.identifier + + first.priority = 10 + first.active = False + second.priority = 20 + + provider_details_dao.dao_update_provider_details(first) + provider_details_dao.dao_update_provider_details(second) + + assert send_to_providers.provider_to_use('sms', '1234').name == second.identifier + + first.active = True + provider_details_dao.dao_update_provider_details(first) + + assert send_to_providers.provider_to_use('sms', '1234').name == first.identifier + + +def test_should_send_personalised_template_to_correct_sms_provider_and_persist( + notify_db, + notify_db_session, + sample_template_with_placeholders, + mocker +): + db_notification = sample_notification(notify_db, notify_db_session, template=sample_template_with_placeholders, + to_field="+447234123123", personalisation={"name": "Jo"}, + status='created') + + mocker.patch('app.mmg_client.send_sms') + mocker.patch('app.mmg_client.get_name', return_value="mmg") + + send_to_providers.send_sms_to_provider( + db_notification + ) + + mmg_client.send_sms.assert_called_once_with( + to=format_phone_number(validate_phone_number("+447234123123")), + content="Sample service: Hello Jo\nYour thing is due soon", + reference=str(db_notification.id), + sender=None + ) + notification = Notification.query.filter_by(id=db_notification.id).one() + + assert notification.status == 'sending' + assert notification.sent_at <= datetime.utcnow() + assert notification.sent_by == 'mmg' + assert notification.billable_units == 1 + assert notification.personalisation == {"name": "Jo"} + + +def test_should_send_personalised_template_to_correct_email_provider_and_persist( + notify_db, + notify_db_session, + sample_email_template_with_placeholders, + mocker +): + db_notification = sample_notification( + notify_db=notify_db, notify_db_session=notify_db_session, + template=sample_email_template_with_placeholders, + to_field="jo.smith@example.com", + personalisation={'name': 'Jo'} + ) + + mocker.patch('app.aws_ses_client.send_email', return_value='reference') + mocker.patch('app.aws_ses_client.get_name', return_value="ses") + + send_to_providers.send_email_to_provider( + db_notification + ) + + app.aws_ses_client.send_email.assert_called_once_with( + '"Sample service" ', + 'jo.smith@example.com', + 'Jo', + body='Hello Jo\nThis is an email from GOV.\u200bUK', + html_body=ANY, + reply_to_address=None + ) + assert ' version_on_notification + + send_to_providers.send_sms_to_provider( + db_notification + ) + + mmg_client.send_sms.assert_called_once_with( + to=format_phone_number(validate_phone_number("+447234123123")), + content="Sample service: This is a template:\nwith a newline", + reference=str(db_notification.id), + sender=None + ) + + persisted_notification = notifications_dao.get_notification_by_id(db_notification.id) + assert persisted_notification.to == db_notification.to + assert persisted_notification.template_id == sample_template.id + assert persisted_notification.template_version == version_on_notification + assert persisted_notification.template_version != sample_template.version + assert persisted_notification.status == 'sending' + assert not persisted_notification.personalisation + + +@pytest.mark.parametrize('research_mode,key_type', [ + (True, KEY_TYPE_NORMAL), + (False, KEY_TYPE_TEST) +]) +def test_should_call_send_sms_response_task_if_research_mode(notify_db, sample_service, sample_notification, mocker, + research_mode, key_type): + mocker.patch('app.mmg_client.send_sms') + mocker.patch('app.mmg_client.get_name', return_value="mmg") + mocker.patch('app.celery.research_mode_tasks.send_sms_response.apply_async') + + if research_mode: + sample_service.research_mode = True + notify_db.session.add(sample_service) + notify_db.session.commit() + + sample_notification.key_type = key_type + + send_to_providers.send_sms_to_provider( + sample_notification + ) + assert not mmg_client.send_sms.called + send_to_providers.send_sms_response.apply_async.assert_called_once_with( + ('mmg', str(sample_notification.id), sample_notification.to), queue='research-mode' + ) + + persisted_notification = notifications_dao.get_notification_by_id(sample_notification.id) + assert persisted_notification.to == sample_notification.to + assert persisted_notification.template_id == sample_notification.template_id + assert persisted_notification.status == 'sending' + assert persisted_notification.sent_at <= datetime.utcnow() + assert persisted_notification.sent_by == 'mmg' + assert not persisted_notification.personalisation + + +@pytest.mark.parametrize('research_mode,key_type', [ + (True, KEY_TYPE_NORMAL), + (False, KEY_TYPE_TEST) +]) +def test_should_set_billable_units_to_zero_in_research_mode_or_test_key( + notify_db, sample_service, sample_notification, mocker, research_mode, key_type): + + mocker.patch('app.mmg_client.send_sms') + mocker.patch('app.mmg_client.get_name', return_value="mmg") + mocker.patch('app.celery.research_mode_tasks.send_sms_response.apply_async') + if research_mode: + sample_service.research_mode = True + notify_db.session.add(sample_service) + notify_db.session.commit() + sample_notification.key_type = key_type + + send_to_providers.send_sms_to_provider( + sample_notification + ) + + assert notifications_dao.get_notification_by_id(sample_notification.id).billable_units == 0 + + +def test_should_not_send_to_provider_when_status_is_not_created(notify_db, notify_db_session, + sample_service, + mocker): + notification = sample_notification(notify_db=notify_db, notify_db_session=notify_db_session, + service=sample_service, + status='sending') + mocker.patch('app.mmg_client.send_sms') + mocker.patch('app.mmg_client.get_name', return_value="mmg") + mocker.patch('app.celery.research_mode_tasks.send_sms_response.apply_async') + + send_to_providers.send_sms_to_provider( + notification + ) + + app.mmg_client.send_sms.assert_not_called() + app.celery.research_mode_tasks.send_sms_response.apply_async.assert_not_called() + + +def test_should_send_sms_sender_from_service_if_present( + notify_db, + notify_db_session, + sample_service, + sample_template, + mocker): + db_notification = sample_notification(notify_db, notify_db_session, template=sample_template, + to_field="+447234123123", + status='created') + + sample_service.sms_sender = 'elevenchars' + notify_db.session.add(sample_service) + notify_db.session.commit() + + mocker.patch('app.mmg_client.send_sms') + mocker.patch('app.mmg_client.get_name', return_value="mmg") + + send_to_providers.send_sms_to_provider( + db_notification + ) + + mmg_client.send_sms.assert_called_once_with( + to=format_phone_number(validate_phone_number("+447234123123")), + content="This is a template:\nwith a newline", + reference=str(db_notification.id), + sender=sample_service.sms_sender + ) + + +@pytest.mark.parametrize('research_mode,key_type', [ + (True, KEY_TYPE_NORMAL), + (False, KEY_TYPE_TEST) +]) +def test_send_email_to_provider_should_call_research_mode_task_response_task_if_research_mode( + notify_db, + notify_db_session, + sample_service, + sample_email_template, + ses_provider, + mocker, + research_mode, + key_type): + notification = sample_notification(notify_db=notify_db, notify_db_session=notify_db_session, + template=sample_email_template, + to_field="john@smith.com", + key_type=key_type + ) + + reference = uuid.uuid4() + mocker.patch('app.uuid.uuid4', return_value=reference) + mocker.patch('app.aws_ses_client.send_email') + mocker.patch('app.aws_ses_client.get_name', return_value="ses") + mocker.patch('app.celery.research_mode_tasks.send_email_response.apply_async') + + if research_mode: + sample_service.research_mode = True + notify_db.session.add(sample_service) + notify_db.session.commit() + + send_to_providers.send_email_to_provider( + notification + ) + assert not app.aws_ses_client.send_email.called + send_to_providers.send_email_response.apply_async.assert_called_once_with( + ('ses', str(reference), 'john@smith.com'), queue="research-mode" + ) + persisted_notification = Notification.query.filter_by(id=notification.id).one() + + assert persisted_notification.to == 'john@smith.com' + assert persisted_notification.template_id == sample_email_template.id + assert persisted_notification.status == 'sending' + assert persisted_notification.sent_at <= datetime.utcnow() + assert persisted_notification.created_at <= datetime.utcnow() + assert persisted_notification.sent_by == 'ses' + assert persisted_notification.reference == str(reference) + assert persisted_notification.billable_units == 0 + + +def test_send_email_to_provider_should_not_send_to_provider_when_status_is_not_created(notify_db, notify_db_session, + sample_service, + sample_email_template, + mocker): + notification = sample_notification(notify_db=notify_db, notify_db_session=notify_db_session, + template=sample_email_template, + service=sample_service, + status='sending') + mocker.patch('app.aws_ses_client.send_email') + mocker.patch('app.aws_ses_client.get_name', return_value="ses") + mocker.patch('app.celery.research_mode_tasks.send_email_response.apply_async') + + send_to_providers.send_sms_to_provider( + notification + ) + + app.aws_ses_client.send_email.assert_not_called() + app.celery.research_mode_tasks.send_email_response.apply_async.assert_not_called() + + +def test_send_email_should_use_service_reply_to_email( + notify_db, notify_db_session, + sample_service, + sample_email_template, + mocker): + mocker.patch('app.aws_ses_client.send_email', return_value='reference') + mocker.patch('app.aws_ses_client.get_name', return_value="ses") + + db_notification = sample_notification(notify_db, notify_db_session, template=sample_email_template) + sample_service.reply_to_email_address = 'foo@bar.com' + + send_to_providers.send_email_to_provider( + db_notification, + ) + + app.aws_ses_client.send_email.assert_called_once_with( + ANY, + ANY, + ANY, + body=ANY, + html_body=ANY, + reply_to_address=sample_service.reply_to_email_address + ) + + +def test_get_html_email_renderer_should_return_for_normal_service(sample_service): + renderer = send_to_providers.get_html_email_renderer(sample_service) + assert renderer.govuk_banner + assert renderer.brand_colour is None + assert renderer.brand_logo is None + assert renderer.brand_name is None + + +@pytest.mark.parametrize('branding_type, govuk_banner', [ + (BRANDING_ORG, False), + (BRANDING_BOTH, True) +]) +def test_get_html_email_renderer_with_branding_details(branding_type, govuk_banner, notify_db, sample_service): + sample_service.branding = branding_type + org = Organisation(colour='#000000', logo='justice-league.png', name='Justice League') + sample_service.organisation = org + notify_db.session.add_all([sample_service, org]) + notify_db.session.commit() + + renderer = send_to_providers.get_html_email_renderer(sample_service) + + assert renderer.govuk_banner == govuk_banner + assert renderer.brand_colour == '000000' + assert renderer.brand_name == 'Justice League' + + +def test_get_html_email_renderer_prepends_logo_path(notify_db, sample_service): + sample_service.branding = BRANDING_ORG + org = Organisation(colour='#000000', logo='justice-league.png', name='Justice League') + sample_service.organisation = org + notify_db.session.add_all([sample_service, org]) + notify_db.session.commit() + + renderer = send_to_providers.get_html_email_renderer(sample_service) + + assert renderer.brand_logo == 'http://localhost:6012/static/images/email-template/crests/justice-league.png' + + +def test_should_not_set_billable_units_if_research_mode(notify_db, sample_service, sample_notification, mocker): + mocker.patch('app.mmg_client.send_sms') + mocker.patch('app.mmg_client.get_name', return_value="mmg") + mocker.patch('app.celery.research_mode_tasks.send_sms_response.apply_async') + + sample_service.research_mode = True + notify_db.session.add(sample_service) + notify_db.session.commit() + + send_to_providers.send_sms_to_provider( + sample_notification + ) + + persisted_notification = notifications_dao.get_notification_by_id(sample_notification.id) + assert persisted_notification.billable_units == 0 + + +@pytest.mark.parametrize('research_mode,key_type, billable_units', [ + (True, KEY_TYPE_NORMAL, 0), + (False, KEY_TYPE_NORMAL, 1), + (False, KEY_TYPE_TEST, 0), + (True, KEY_TYPE_TEST, 0), + (True, KEY_TYPE_TEAM, 0), + (False, KEY_TYPE_TEAM, 1) +]) +def test_should_update_billable_units_according_to_research_mode_and_key_type(notify_db, + sample_service, + sample_notification, + mocker, + research_mode, + key_type, + billable_units): + + assert Notification.query.count() == 1 + + mocker.patch('app.mmg_client.send_sms') + mocker.patch('app.mmg_client.get_name', return_value="mmg") + mocker.patch('app.celery.research_mode_tasks.send_sms_response.apply_async') + if research_mode: + sample_service.research_mode = True + notify_db.session.add(sample_service) + notify_db.session.commit() + + sample_notification.key_type = key_type + + send_to_providers.send_sms_to_provider( + sample_notification + ) + + assert Notification.query.get(sample_notification.id).billable_units == billable_units, \ + "Research mode: {0}, key type: {1}, billable_units: {2}".format(research_mode, key_type, billable_units) diff --git a/tests/app/job/test_rest.py b/tests/app/job/test_rest.py index cef1fe793..20d6a4248 100644 --- a/tests/app/job/test_rest.py +++ b/tests/app/job/test_rest.py @@ -1,59 +1,20 @@ import json import uuid from datetime import datetime, timedelta -from freezegun import freeze_time +from freezegun import freeze_time import pytest import pytz import app.celery.tasks from tests import create_authorization_header +from tests.conftest import set_config from tests.app.conftest import ( sample_job as create_job, - sample_notification as create_sample_notification, sample_notification, sample_job) + sample_notification as create_notification +) from app.dao.templates_dao import dao_update_template -from app.models import NOTIFICATION_STATUS_TYPES - - -def test_get_jobs(notify_api, notify_db, notify_db_session, sample_template): - _setup_jobs(notify_db, notify_db_session, sample_template) - - service_id = sample_template.service.id - - with notify_api.test_request_context(): - with notify_api.test_client() as client: - path = '/service/{}/job'.format(service_id) - auth_header = create_authorization_header(service_id=service_id) - response = client.get(path, headers=[auth_header]) - assert response.status_code == 200 - resp_json = json.loads(response.get_data(as_text=True)) - assert len(resp_json['data']) == 5 - - -def test_get_jobs_with_limit_days(notify_api, notify_db, notify_db_session, sample_template): - create_job( - notify_db, - notify_db_session, - service=sample_template.service, - template=sample_template, - ) - create_job( - notify_db, - notify_db_session, - service=sample_template.service, - template=sample_template, - created_at=datetime.now() - timedelta(days=7)) - - service_id = sample_template.service.id - - with notify_api.test_request_context(): - with notify_api.test_client() as client: - path = '/service/{}/job'.format(service_id) - auth_header = create_authorization_header(service_id=service_id) - response = client.get(path, headers=[auth_header], query_string={'limit_days': 5}) - assert response.status_code == 200 - resp_json = json.loads(response.get_data(as_text=True)) - assert len(resp_json['data']) == 1 +from app.models import NOTIFICATION_STATUS_TYPES, JOB_STATUS_TYPES, JOB_STATUS_PENDING def test_get_job_with_invalid_service_id_returns404(notify_api, sample_api_key, sample_service): @@ -391,7 +352,7 @@ def test_get_all_notifications_for_job_in_order_of_job_number(notify_api, main_job = create_job(notify_db, notify_db_session, service=sample_service) another_job = create_job(notify_db, notify_db_session, service=sample_service) - notification_1 = create_sample_notification( + notification_1 = create_notification( notify_db, notify_db_session, job=main_job, @@ -399,7 +360,7 @@ def test_get_all_notifications_for_job_in_order_of_job_number(notify_api, created_at=datetime.utcnow(), job_row_number=1 ) - notification_2 = create_sample_notification( + notification_2 = create_notification( notify_db, notify_db_session, job=main_job, @@ -407,7 +368,7 @@ def test_get_all_notifications_for_job_in_order_of_job_number(notify_api, created_at=datetime.utcnow(), job_row_number=2 ) - notification_3 = create_sample_notification( + notification_3 = create_notification( notify_db, notify_db_session, job=main_job, @@ -415,7 +376,7 @@ def test_get_all_notifications_for_job_in_order_of_job_number(notify_api, created_at=datetime.utcnow(), job_row_number=3 ) - create_sample_notification(notify_db, notify_db_session, job=another_job) + create_notification(notify_db, notify_db_session, job=another_job) auth_header = create_authorization_header() @@ -454,7 +415,7 @@ def test_get_all_notifications_for_job_filtered_by_status( with notify_api.test_request_context(), notify_api.test_client() as client: job = create_job(notify_db, notify_db_session, service=sample_service) - create_sample_notification( + create_notification( notify_db, notify_db_session, job=job, @@ -492,14 +453,14 @@ def test_get_job_by_id_should_return_statistics(notify_db, notify_db_session, no job_id = str(sample_job.id) service_id = sample_job.service.id - sample_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='created') - sample_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='sending') - sample_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='delivered') - sample_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='pending') - sample_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='failed') - sample_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='technical-failure') # noqa - sample_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='temporary-failure') # noqa - sample_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='permanent-failure') # noqa + create_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='created') + create_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='sending') + create_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='delivered') + create_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='pending') + create_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='failed') + create_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='technical-failure') # noqa + create_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='temporary-failure') # noqa + create_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='permanent-failure') # noqa with notify_api.test_request_context(): with notify_api.test_client() as client: @@ -524,16 +485,16 @@ def test_get_job_by_id_should_return_summed_statistics(notify_db, notify_db_sess job_id = str(sample_job.id) service_id = sample_job.service.id - sample_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='created') - sample_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='created') - sample_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='created') - sample_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='sending') - sample_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='failed') - sample_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='failed') - sample_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='failed') - sample_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='technical-failure') # noqa - sample_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='temporary-failure') # noqa - sample_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='temporary-failure') # noqa + create_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='created') + create_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='created') + create_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='created') + create_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='sending') + create_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='failed') + create_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='failed') + create_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='failed') + create_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='technical-failure') # noqa + create_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='temporary-failure') # noqa + create_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='temporary-failure') # noqa with notify_api.test_request_context(): with notify_api.test_client() as client: @@ -551,22 +512,63 @@ def test_get_job_by_id_should_return_summed_statistics(notify_db, notify_db_sess assert resp_json['data']['created_by']['name'] == 'Test User' -def test_get_jobs_for_service_should_return_statistics(notify_db, notify_db_session, notify_api, sample_service): - now = datetime.utcnow() - earlier = datetime.utcnow() - timedelta(days=1) - job_1 = sample_job(notify_db, notify_db_session, service=sample_service, created_at=earlier) - job_2 = sample_job(notify_db, notify_db_session, service=sample_service, created_at=now) +def test_get_jobs(notify_api, notify_db, notify_db_session, sample_template): + _setup_jobs(notify_db, notify_db_session, sample_template) - sample_notification(notify_db, notify_db_session, service=sample_service, job=job_1, status='created') - sample_notification(notify_db, notify_db_session, service=sample_service, job=job_1, status='created') - sample_notification(notify_db, notify_db_session, service=sample_service, job=job_1, status='created') - sample_notification(notify_db, notify_db_session, service=sample_service, job=job_2, status='sending') - sample_notification(notify_db, notify_db_session, service=sample_service, job=job_2, status='sending') - sample_notification(notify_db, notify_db_session, service=sample_service, job=job_2, status='sending') + service_id = sample_template.service.id with notify_api.test_request_context(): with notify_api.test_client() as client: - path = '/service/{}/job'.format(str(sample_service.id)) + path = '/service/{}/job'.format(service_id) + auth_header = create_authorization_header(service_id=service_id) + response = client.get(path, headers=[auth_header]) + assert response.status_code == 200 + resp_json = json.loads(response.get_data(as_text=True)) + assert len(resp_json['data']) == 5 + + +def test_get_jobs_with_limit_days(notify_api, notify_db, notify_db_session, sample_template): + create_job( + notify_db, + notify_db_session, + service=sample_template.service, + template=sample_template, + ) + create_job( + notify_db, + notify_db_session, + service=sample_template.service, + template=sample_template, + created_at=datetime.now() - timedelta(days=7)) + + service_id = sample_template.service.id + + with notify_api.test_request_context(): + with notify_api.test_client() as client: + path = '/service/{}/job'.format(service_id) + auth_header = create_authorization_header(service_id=service_id) + response = client.get(path, headers=[auth_header], query_string={'limit_days': 5}) + assert response.status_code == 200 + resp_json = json.loads(response.get_data(as_text=True)) + assert len(resp_json['data']) == 1 + + +def test_get_jobs_should_return_statistics(notify_db, notify_db_session, notify_api, sample_service): + now = datetime.utcnow() + earlier = datetime.utcnow() - timedelta(days=1) + job_1 = create_job(notify_db, notify_db_session, service=sample_service, created_at=earlier) + job_2 = create_job(notify_db, notify_db_session, service=sample_service, created_at=now) + + create_notification(notify_db, notify_db_session, service=sample_service, job=job_1, status='created') + create_notification(notify_db, notify_db_session, service=sample_service, job=job_1, status='created') + create_notification(notify_db, notify_db_session, service=sample_service, job=job_1, status='created') + create_notification(notify_db, notify_db_session, service=sample_service, job=job_2, status='sending') + create_notification(notify_db, notify_db_session, service=sample_service, job=job_2, status='sending') + create_notification(notify_db, notify_db_session, service=sample_service, job=job_2, status='sending') + + with notify_api.test_request_context(): + with notify_api.test_client() as client: + path = '/service/{}/job'.format(sample_service.id) auth_header = create_authorization_header(service_id=str(sample_service.id)) response = client.get(path, headers=[auth_header]) assert response.status_code == 200 @@ -578,7 +580,7 @@ def test_get_jobs_for_service_should_return_statistics(notify_db, notify_db_sess assert {'status': 'created', 'count': 3} in resp_json['data'][1]['statistics'] -def test_get_jobs_for_service_should_return_no_stats_if_no_rows_in_notifications( +def test_get_jobs_should_return_no_stats_if_no_rows_in_notifications( notify_db, notify_db_session, notify_api, @@ -586,12 +588,12 @@ def test_get_jobs_for_service_should_return_no_stats_if_no_rows_in_notifications now = datetime.utcnow() earlier = datetime.utcnow() - timedelta(days=1) - job_1 = sample_job(notify_db, notify_db_session, service=sample_service, created_at=earlier) - job_2 = sample_job(notify_db, notify_db_session, service=sample_service, created_at=now) + job_1 = create_job(notify_db, notify_db_session, service=sample_service, created_at=earlier) + job_2 = create_job(notify_db, notify_db_session, service=sample_service, created_at=now) with notify_api.test_request_context(): with notify_api.test_client() as client: - path = '/service/{}/job'.format(str(sample_service.id)) + path = '/service/{}/job'.format(sample_service.id) auth_header = create_authorization_header(service_id=str(sample_service.id)) response = client.get(path, headers=[auth_header]) assert response.status_code == 200 @@ -601,3 +603,96 @@ def test_get_jobs_for_service_should_return_no_stats_if_no_rows_in_notifications assert resp_json['data'][0]['statistics'] == [] assert resp_json['data'][1]['id'] == str(job_1.id) assert resp_json['data'][1]['statistics'] == [] + + +def test_get_jobs_should_paginate( + notify_db, + notify_db_session, + client, + sample_template +): + create_10_jobs(notify_db, notify_db_session, sample_template.service, sample_template) + + path = '/service/{}/job'.format(sample_template.service_id) + auth_header = create_authorization_header(service_id=str(sample_template.service_id)) + + with set_config(client.application, 'PAGE_SIZE', 2): + response = client.get(path, headers=[auth_header]) + + assert response.status_code == 200 + resp_json = json.loads(response.get_data(as_text=True)) + assert len(resp_json['data']) == 2 + assert resp_json['data'][0]['created_at'] == '2015-01-01T10:00:00+00:00' + assert resp_json['data'][1]['created_at'] == '2015-01-01T09:00:00+00:00' + assert resp_json['page_size'] == 2 + assert resp_json['total'] == 10 + assert 'links' in resp_json + assert set(resp_json['links'].keys()) == {'next', 'last'} + + +def test_get_jobs_accepts_page_parameter( + notify_db, + notify_db_session, + client, + sample_template +): + create_10_jobs(notify_db, notify_db_session, sample_template.service, sample_template) + + path = '/service/{}/job'.format(sample_template.service_id) + auth_header = create_authorization_header(service_id=str(sample_template.service_id)) + + with set_config(client.application, 'PAGE_SIZE', 2): + response = client.get(path, headers=[auth_header], query_string={'page': 2}) + + assert response.status_code == 200 + resp_json = json.loads(response.get_data(as_text=True)) + assert len(resp_json['data']) == 2 + assert resp_json['data'][0]['created_at'] == '2015-01-01T08:00:00+00:00' + assert resp_json['data'][1]['created_at'] == '2015-01-01T07:00:00+00:00' + assert resp_json['page_size'] == 2 + assert resp_json['total'] == 10 + assert 'links' in resp_json + assert set(resp_json['links'].keys()) == {'prev', 'next', 'last'} + + +@pytest.mark.parametrize('statuses_filter, expected_statuses', [ + ('', JOB_STATUS_TYPES), + ('pending', [JOB_STATUS_PENDING]), + ('pending, in progress, finished, sending limits exceeded, scheduled, cancelled', JOB_STATUS_TYPES), + # bad statuses are accepted, just return no data + ('foo', []) +]) +def test_get_jobs_can_filter_on_statuses( + notify_db, + notify_db_session, + client, + sample_service, + statuses_filter, + expected_statuses +): + create_job(notify_db, notify_db_session, job_status='pending') + create_job(notify_db, notify_db_session, job_status='in progress') + create_job(notify_db, notify_db_session, job_status='finished') + create_job(notify_db, notify_db_session, job_status='sending limits exceeded') + create_job(notify_db, notify_db_session, job_status='scheduled') + create_job(notify_db, notify_db_session, job_status='cancelled') + + path = '/service/{}/job'.format(sample_service.id) + response = client.get( + path, + headers=[create_authorization_header()], + query_string={'statuses': statuses_filter} + ) + + assert response.status_code == 200 + resp_json = json.loads(response.get_data(as_text=True)) + from pprint import pprint + pprint(resp_json) + assert {x['job_status'] for x in resp_json['data']} == set(expected_statuses) + + +def create_10_jobs(db, session, service, template): + with freeze_time('2015-01-01T00:00:00') as the_time: + for _ in range(10): + the_time.tick(timedelta(hours=1)) + create_job(db, session, service, template) diff --git a/tests/app/notifications/rest/test_send_notification.py b/tests/app/notifications/rest/test_send_notification.py index 56fcfad33..4525fa9ed 100644 --- a/tests/app/notifications/rest/test_send_notification.py +++ b/tests/app/notifications/rest/test_send_notification.py @@ -1,16 +1,13 @@ -import uuid from datetime import datetime import random import string import pytest -from unittest.mock import ANY from flask import (json, current_app) from freezegun import freeze_time from notifications_python_client.authentication import create_jwt_token import app -from app import encryption from app.dao import notifications_dao from app.models import ApiKey, KEY_TYPE_TEAM, KEY_TYPE_TEST, Notification, NotificationHistory from app.dao.templates_dao import dao_get_all_templates_for_service, dao_update_template @@ -1235,3 +1232,33 @@ def test_should_send_email_to_whitelist_recipient_in_trial_mode_with_live_key(cl assert json_resp['data']['body'] == email_template.content assert json_resp['data']['template_version'] == email_template.version apply_async.called + + +@pytest.mark.parametrize( + 'notification_type, template_type, to', [ + ('email', 'sms', 'notify@digital.cabinet-office.gov.uk'), + ('sms', 'email', '+447700900986') + ]) +def test_should_error_if_notification_type_does_not_match_template_type( + client, + notify_db, + notify_db_session, + template_type, + notification_type, + to +): + template = create_sample_template(notify_db, notify_db_session, template_type=template_type) + data = { + 'to': to, + 'template': template.id + } + auth_header = create_authorization_header(service_id=template.service_id) + response = client.post("/notifications/{}".format(notification_type), + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), auth_header]) + + assert response.status_code == 400 + json_resp = json.loads(response.get_data(as_text=True)) + assert json_resp['result'] == 'error' + assert '{0} template is not suitable for {1} notification'.format(template_type, notification_type) \ + in json_resp['message'] diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index ac8c58e3f..179d9383a 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -16,6 +16,7 @@ from tests.app.conftest import ( sample_user as create_sample_user, sample_notification as create_sample_notification, sample_notification_with_job) +from app.models import KEY_TYPE_TEST def test_get_service_list(notify_api, service_factory): @@ -1037,26 +1038,41 @@ def test_get_all_notifications_for_service_in_order(notify_api, notify_db, notif assert response.status_code == 200 +@pytest.mark.parametrize( + 'include_from_test_key, expected_count_of_notifications', + [ + (False, 2), + (True, 3) + ] +) def test_get_all_notifications_for_service_including_ones_made_by_jobs( - notify_api, - notify_db, - notify_db_session, - sample_service): - with notify_api.test_request_context(), notify_api.test_client() as client: - with_job = sample_notification_with_job(notify_db, notify_db_session, service=sample_service) - without_job = create_sample_notification(notify_db, notify_db_session, service=sample_service) + client, + notify_db, + notify_db_session, + sample_service, + include_from_test_key, + expected_count_of_notifications +): + with_job = sample_notification_with_job(notify_db, notify_db_session, service=sample_service) + without_job = create_sample_notification(notify_db, notify_db_session, service=sample_service) + from_test_api_key = create_sample_notification( + notify_db, notify_db_session, service=sample_service, key_type=KEY_TYPE_TEST + ) - auth_header = create_authorization_header() + auth_header = create_authorization_header() - response = client.get( - path='/service/{}/notifications'.format(sample_service.id), - headers=[auth_header]) + response = client.get( + path='/service/{}/notifications?include_from_test_key={}'.format( + sample_service.id, include_from_test_key + ), + headers=[auth_header] + ) - resp = json.loads(response.get_data(as_text=True)) - assert len(resp['notifications']) == 2 - assert resp['notifications'][0]['to'] == with_job.to - assert resp['notifications'][1]['to'] == without_job.to - assert response.status_code == 200 + resp = json.loads(response.get_data(as_text=True)) + assert len(resp['notifications']) == expected_count_of_notifications + assert resp['notifications'][0]['to'] == with_job.to + assert resp['notifications'][1]['to'] == without_job.to + assert response.status_code == 200 def test_get_only_api_created_notifications_for_service( diff --git a/tests/app/test_schemas.py b/tests/app/test_schemas.py index e7d037311..4ba1c98b5 100644 --- a/tests/app/test_schemas.py +++ b/tests/app/test_schemas.py @@ -8,3 +8,17 @@ def test_job_schema_doesnt_return_notifications(sample_notification_with_job): assert not errors assert 'notifications' not in data + + +def test_notification_schema_ignores_absent_api_key(sample_notification_with_job): + from app.schemas import notification_with_template_schema + + data = notification_with_template_schema.dump(sample_notification_with_job).data + assert data['key_name'] is None + + +def test_notification_schema_adds_api_key_name(sample_notification_with_api_key): + from app.schemas import notification_with_template_schema + + data = notification_with_template_schema.dump(sample_notification_with_api_key).data + assert data['key_name'] == 'Test key' diff --git a/tests/conftest.py b/tests/conftest.py index 3da2dead3..11f570de5 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,3 +1,4 @@ +from contextlib import contextmanager import os import boto3 @@ -83,3 +84,11 @@ def pytest_generate_tests(metafunc): argnames, testdata = idparametrize.args ids, argvalues = zip(*sorted(testdata.items())) metafunc.parametrize(argnames, argvalues, ids=ids) + + +@contextmanager +def set_config(app, name, value): + old_val = app.config.get(name) + app.config[name] = value + yield + app.config[name] = old_val