diff --git a/.gitignore b/.gitignore index d19df2e94..12f77b15c 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,5 @@ +queues.csv + # Byte-compiled / optimized / DLL files __pycache__/ *.py[cod] diff --git a/Makefile b/Makefile index 37d876d92..4142722ff 100644 --- a/Makefile +++ b/Makefile @@ -303,3 +303,8 @@ cf-rollback: ## Rollbacks the app to the previous release cf-push: $(if ${CF_APP},,$(error Must specify CF_APP)) cf push ${CF_APP} -f ${CF_MANIFEST_FILE} + +.PHONY: check-if-migrations-to-run +check-if-migrations-to-run: + @echo $(shell python3 scripts/check_if_new_migration.py) + diff --git a/app/celery/provider_tasks.py b/app/celery/provider_tasks.py index e00591f09..50d5a31b7 100644 --- a/app/celery/provider_tasks.py +++ b/app/celery/provider_tasks.py @@ -3,37 +3,14 @@ from notifications_utils.recipients import InvalidEmailError from sqlalchemy.orm.exc import NoResultFound from app import notify_celery +from app.config import QueueNames 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 -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) - 2: 300 seconds (5 minutes) - 3: 3600 seconds (60 minutes) - 4: 14400 seconds (4 hours) - :param retry (zero indexed): - :return length to retry in seconds, default 10 seconds - """ - - delays = { - 0: 10, - 1: 60, - 2: 300, - 3: 3600, - 4: 14400 - } - - return delays.get(retry, 10) - - -@notify_celery.task(bind=True, name="deliver_sms", max_retries=5, default_retry_delay=5) +@notify_celery.task(bind=True, name="deliver_sms", max_retries=48, default_retry_delay=300) @statsd(namespace="tasks") def deliver_sms(self, notification_id): try: @@ -46,7 +23,7 @@ def deliver_sms(self, notification_id): current_app.logger.exception( "SMS notification delivery for id: {} failed".format(notification_id) ) - self.retry(queue="retry", countdown=retry_iteration_to_delay(self.request.retries)) + self.retry(queue=QueueNames.RETRY) except self.MaxRetriesExceededError: current_app.logger.exception( "RETRY FAILED: task send_sms_to_provider failed for notification {}".format(notification_id), @@ -54,7 +31,7 @@ def deliver_sms(self, notification_id): update_notification_status_by_id(notification_id, 'technical-failure') -@notify_celery.task(bind=True, name="deliver_email", max_retries=5, default_retry_delay=5) +@notify_celery.task(bind=True, name="deliver_email", max_retries=48, default_retry_delay=300) @statsd(namespace="tasks") def deliver_email(self, notification_id): try: @@ -70,7 +47,7 @@ def deliver_email(self, notification_id): current_app.logger.exception( "RETRY: Email notification {} failed".format(notification_id) ) - self.retry(queue="retry", countdown=retry_iteration_to_delay(self.request.retries)) + self.retry(queue=QueueNames.RETRY) except self.MaxRetriesExceededError: current_app.logger.error( "RETRY FAILED: task send_email_to_provider failed for notification {}".format(notification_id) diff --git a/app/celery/scheduled_tasks.py b/app/celery/scheduled_tasks.py index fbea3bdb0..457877ed0 100644 --- a/app/celery/scheduled_tasks.py +++ b/app/celery/scheduled_tasks.py @@ -12,18 +12,21 @@ from app import performance_platform_client from app.dao.invited_user_dao import delete_invitations_created_more_than_two_days_ago from app.dao.jobs_dao import dao_set_scheduled_jobs_to_pending, dao_get_jobs_older_than_limited_by from app.dao.notifications_dao import ( - delete_notifications_created_more_than_a_week_ago, dao_timeout_notifications, - is_delivery_slow_for_provider -) + is_delivery_slow_for_provider, + delete_notifications_created_more_than_a_week_ago_by_type, + dao_get_scheduled_notifications, + set_scheduled_notification_to_processed) from app.dao.statistics_dao import dao_timeout_job_statistics from app.dao.provider_details_dao import ( get_current_provider, dao_toggle_sms_provider ) from app.dao.users_dao import delete_codes_older_created_more_than_a_day_ago +from app.notifications.process_notifications import send_notification_to_queue from app.statsd_decorators import statsd from app.celery.tasks import process_job +from app.config import QueueNames @notify_celery.task(name="remove_csv_files") @@ -40,13 +43,28 @@ def remove_csv_files(): def run_scheduled_jobs(): try: for job in dao_set_scheduled_jobs_to_pending(): - process_job.apply_async([str(job.id)], queue="process-job") + process_job.apply_async([str(job.id)], queue=QueueNames.JOBS) current_app.logger.info("Job ID {} added to process job queue".format(job.id)) - except SQLAlchemyError as e: + except SQLAlchemyError: current_app.logger.exception("Failed to run scheduled jobs") raise +@notify_celery.task(name='send-scheduled-notifications') +@statsd(namespace="tasks") +def send_scheduled_notifications(): + try: + scheduled_notifications = dao_get_scheduled_notifications() + for notification in scheduled_notifications: + send_notification_to_queue(notification, notification.service.research_mode) + set_scheduled_notification_to_processed(notification.id) + current_app.logger.info( + "Sent {} scheduled notifications to the provider queue".format(len(scheduled_notifications))) + except SQLAlchemyError: + current_app.logger.exception("Failed to send scheduled notifications") + raise + + @notify_celery.task(name="delete-verify-codes") @statsd(namespace="tasks") def delete_verify_codes(): @@ -61,42 +79,60 @@ def delete_verify_codes(): raise -@notify_celery.task(name="delete-successful-notifications") +@notify_celery.task(name="delete-sms-notifications") @statsd(namespace="tasks") -def delete_successful_notifications(): +def delete_sms_notifications_older_than_seven_days(): try: start = datetime.utcnow() - deleted = delete_notifications_created_more_than_a_week_ago('delivered') + deleted = delete_notifications_created_more_than_a_week_ago_by_type('sms') current_app.logger.info( - "Delete job started {} finished {} deleted {} successful notifications".format( + "Delete {} job started {} finished {} deleted {} sms notifications".format( + 'sms', start, datetime.utcnow(), deleted ) ) except SQLAlchemyError as e: - current_app.logger.exception("Failed to delete successful notifications") + current_app.logger.exception("Failed to delete sms notifications") raise -@notify_celery.task(name="delete-failed-notifications") +@notify_celery.task(name="delete-email-notifications") @statsd(namespace="tasks") -def delete_failed_notifications(): +def delete_email_notifications_older_than_seven_days(): try: start = datetime.utcnow() - deleted = delete_notifications_created_more_than_a_week_ago('failed') - deleted += delete_notifications_created_more_than_a_week_ago('technical-failure') - deleted += delete_notifications_created_more_than_a_week_ago('temporary-failure') - deleted += delete_notifications_created_more_than_a_week_ago('permanent-failure') + deleted = delete_notifications_created_more_than_a_week_ago_by_type('email') current_app.logger.info( - "Delete job started {} finished {} deleted {} failed notifications".format( + "Delete {} job started {} finished {} deleted {} email notifications".format( + 'email', start, datetime.utcnow(), deleted ) ) except SQLAlchemyError as e: - current_app.logger.exception("Failed to delete failed notifications") + current_app.logger.exception("Failed to delete sms notifications") + raise + + +@notify_celery.task(name="delete-letter-notifications") +@statsd(namespace="tasks") +def delete_letter_notifications_older_than_seven_days(): + try: + start = datetime.utcnow() + deleted = delete_notifications_created_more_than_a_week_ago_by_type('letter') + current_app.logger.info( + "Delete {} job started {} finished {} deleted {} letter notifications".format( + 'letter', + start, + datetime.utcnow(), + deleted + ) + ) + except SQLAlchemyError as e: + current_app.logger.exception("Failed to delete sms notifications") raise diff --git a/app/celery/statistics_tasks.py b/app/celery/statistics_tasks.py index a82a3791f..150fa6aac 100644 --- a/app/celery/statistics_tasks.py +++ b/app/celery/statistics_tasks.py @@ -3,7 +3,6 @@ from sqlalchemy.exc import SQLAlchemyError from app import notify_celery from flask import current_app -from app.models import JobStatistics from app.statsd_decorators import statsd from app.dao.statistics_dao import ( create_or_update_job_sending_statistics, @@ -11,16 +10,17 @@ from app.dao.statistics_dao import ( ) from app.dao.notifications_dao import get_notification_by_id from app.models import NOTIFICATION_STATUS_TYPES_COMPLETED +from app.config import QueueNames def create_initial_notification_statistic_tasks(notification): if notification.job_id and notification.status: - record_initial_job_statistics.apply_async((str(notification.id),), queue="statistics") + record_initial_job_statistics.apply_async((str(notification.id),), queue=QueueNames.STATISTICS) def create_outcome_notification_statistic_tasks(notification): if notification.job_id and notification.status in NOTIFICATION_STATUS_TYPES_COMPLETED: - record_outcome_job_statistics.apply_async((str(notification.id),), queue="statistics") + record_outcome_job_statistics.apply_async((str(notification.id),), queue=QueueNames.STATISTICS) @notify_celery.task(bind=True, name='record_initial_job_statistics', max_retries=20, default_retry_delay=10) @@ -35,7 +35,7 @@ def record_initial_job_statistics(self, notification_id): raise SQLAlchemyError("Failed to find notification with id {}".format(notification_id)) except SQLAlchemyError as e: current_app.logger.exception(e) - self.retry(queue="retry") + self.retry(queue=QueueNames.RETRY) except self.MaxRetriesExceededError: current_app.logger.error( "RETRY FAILED: task record_initial_job_statistics failed for notification {}".format( @@ -53,12 +53,12 @@ def record_outcome_job_statistics(self, notification_id): if notification: updated_count = update_job_stats_outcome_count(notification) if updated_count == 0: - self.retry(queue="retry") + self.retry(queue=QueueNames.RETRY) else: raise SQLAlchemyError("Failed to find notification with id {}".format(notification_id)) except SQLAlchemyError as e: current_app.logger.exception(e) - self.retry(queue="retry") + self.retry(queue=QueueNames.RETRY) except self.MaxRetriesExceededError: current_app.logger.error( "RETRY FAILED: task update_job_stats_outcome_count failed for notification {}".format( diff --git a/app/celery/tasks.py b/app/celery/tasks.py index cce6b05f0..bb9967ee4 100644 --- a/app/celery/tasks.py +++ b/app/celery/tasks.py @@ -16,6 +16,7 @@ from app import ( ) from app.aws import s3 from app.celery import provider_tasks +from app.config import QueueNames from app.dao.jobs_dao import ( dao_update_job, dao_get_job_by_id, @@ -80,7 +81,7 @@ def process_job(job_id): process_row(row_number, recipient, personalisation, template, job, service) if template.template_type == LETTER_TYPE: - build_dvla_file.apply_async([str(job.id)], queue='process-job') + build_dvla_file.apply_async([str(job.id)], queue=QueueNames.JOBS) # temporary logging current_app.logger.info("send job {} to build-dvla-file in the process-job queue".format(job_id)) else: @@ -112,12 +113,6 @@ def process_row(row_number, recipient, personalisation, template, job, service): LETTER_TYPE: persist_letter } - queues = { - SMS_TYPE: 'db-sms', - EMAIL_TYPE: 'db-email', - LETTER_TYPE: 'db-letter', - } - send_fn = send_fns[template_type] send_fn.apply_async( @@ -127,7 +122,7 @@ def process_row(row_number, recipient, personalisation, template, job, service): encrypted, datetime.utcnow().strftime(DATETIME_FORMAT) ), - queue=queues[template_type] if not service.research_mode else 'research-mode' + queue=QueueNames.DATABASE if not service.research_mode else QueueNames.RESEARCH_MODE ) @@ -165,23 +160,24 @@ def send_sms(self, return try: - saved_notification = persist_notification(template_id=notification['template'], - template_version=notification['template_version'], - recipient=notification['to'], - service=service, - personalisation=notification.get('personalisation'), - notification_type=SMS_TYPE, - api_key_id=api_key_id, - key_type=key_type, - created_at=created_at, - job_id=notification.get('job', None), - job_row_number=notification.get('row_number', None), - notification_id=notification_id - ) + saved_notification = persist_notification( + template_id=notification['template'], + template_version=notification['template_version'], + recipient=notification['to'], + service=service, + personalisation=notification.get('personalisation'), + notification_type=SMS_TYPE, + api_key_id=api_key_id, + key_type=key_type, + created_at=created_at, + job_id=notification.get('job', None), + job_row_number=notification.get('row_number', None), + notification_id=notification_id + ) provider_tasks.deliver_sms.apply_async( [str(saved_notification.id)], - queue='send-sms' if not service.research_mode else 'research-mode' + queue=QueueNames.SEND if not service.research_mode else QueueNames.RESEARCH_MODE ) current_app.logger.info( @@ -226,7 +222,7 @@ def send_email(self, provider_tasks.deliver_email.apply_async( [str(saved_notification.id)], - queue='send-email' if not service.research_mode else 'research-mode' + queue=QueueNames.SEND if not service.research_mode else QueueNames.RESEARCH_MODE ) current_app.logger.info("Email {} created at {}".format(saved_notification.id, created_at)) @@ -284,10 +280,9 @@ def build_dvla_file(self, job_id): file_location="{}-dvla-job.text".format(job_id) ) dao_update_job_status(job_id, JOB_STATUS_READY_TO_SEND) - notify_celery.send_task("aggregrate-dvla-files", ([str(job_id)], ), queue='aggregate-dvla-files') else: current_app.logger.info("All notifications for job {} are not persisted".format(job_id)) - self.retry(queue="retry", exc="All notifications for job {} are not persisted".format(job_id)) + self.retry(queue=QueueNames.RETRY, exc="All notifications for job {} are not persisted".format(job_id)) except Exception as e: current_app.logger.exception("build_dvla_file threw exception") raise e @@ -341,7 +336,7 @@ def handle_exception(task, notification, notification_id, exc): # send to the retry queue. current_app.logger.exception('Retry' + retry_msg) try: - task.retry(queue="retry", exc=exc) + task.retry(queue=QueueNames.RETRY, exc=exc) except task.MaxRetriesExceededError: current_app.logger.exception('Retry' + retry_msg) diff --git a/app/clients/performance_platform/performance_platform_client.py b/app/clients/performance_platform/performance_platform_client.py index 91e43a210..d8685f0f3 100644 --- a/app/clients/performance_platform/performance_platform_client.py +++ b/app/clients/performance_platform/performance_platform_client.py @@ -1,11 +1,11 @@ import base64 import json -from datetime import datetime, timedelta +from datetime import datetime import requests from flask import current_app -from app.utils import get_midnight_for_day_before, get_london_midnight_in_utc, get_utc_time_in_bst +from app.utils import get_midnight_for_day_before, get_london_midnight_in_utc, convert_utc_time_in_bst class PerformancePlatformClient: @@ -27,7 +27,7 @@ class PerformancePlatformClient: def send_performance_stats(self, date, channel, count, period): if self.active: payload = { - '_timestamp': get_utc_time_in_bst(date).isoformat(), + '_timestamp': convert_utc_time_in_bst(date).isoformat(), 'service': 'govuk-notify', 'channel': channel, 'count': count, diff --git a/app/commands.py b/app/commands.py index 197403eba..04b5186ab 100644 --- a/app/commands.py +++ b/app/commands.py @@ -2,7 +2,10 @@ import uuid from datetime import datetime from decimal import Decimal from flask.ext.script import Command, Manager, Option -from app.models import (PROVIDERS, Service, User) + + +from app import db +from app.models import (PROVIDERS, Service, User, NotificationHistory) from app.dao.services_dao import ( delete_service_and_all_associated_db_objects, dao_fetch_all_services_by_user @@ -60,3 +63,29 @@ class PurgeFunctionalTestDataCommand(Command): else: delete_user_verify_codes(usr) delete_model_user(usr) + + +class CustomDbScript(Command): + def run(self): + self.update_notification_international_flag() + + def update_notification_international_flag(self): + # 250,000 rows takes 30 seconds to update. + subq = "select id from notifications where international is null limit 250000" + update = "update notifications set international = False where id in ({})".format(subq) + result = db.session.execute(subq).fetchall() + while len(result) > 0: + db.session.execute(update) + print('commit 250000 updates at {}'.format(datetime.utcnow())) + db.session.commit() + result = db.session.execute(subq).fetchall() + + # Now update notification_history + subq_history = "select id from notification_history where international is null limit 250000" + update_history = "update notification_history set international = False where id in ({})".format(subq_history) + result_history = db.session.execute(subq_history).fetchall() + while len(result_history) > 0: + db.session.execute(update_history) + print('commit 250000 updates at {}'.format(datetime.utcnow())) + db.session.commit() + result_history = db.session.execute(subq_history).fetchall() diff --git a/app/config.py b/app/config.py index 4bdec85b6..f2cd39cdb 100644 --- a/app/config.py +++ b/app/config.py @@ -12,6 +12,34 @@ if os.environ.get('VCAP_SERVICES'): extract_cloudfoundry_config() +class QueueNames(object): + PERIODIC = 'periodic-tasks' + PRIORITY = 'priority-tasks' + DATABASE = 'database-tasks' + SEND = 'send-tasks' + RESEARCH_MODE = 'research-mode-tasks' + STATISTICS = 'statistics-tasks' + JOBS = 'job-tasks' + RETRY = 'retry-tasks' + NOTIFY = 'notify-internal-tasks' + PROCESS_FTP = 'process-ftp-tasks' + + @staticmethod + def all_queues(): + return [ + QueueNames.PRIORITY, + QueueNames.PERIODIC, + QueueNames.DATABASE, + QueueNames.SEND, + QueueNames.RESEARCH_MODE, + QueueNames.STATISTICS, + QueueNames.JOBS, + QueueNames.RETRY, + QueueNames.NOTIFY, + QueueNames.PROCESS_FTP + ] + + class Config(object): # URL of admin app ADMIN_BASE_URL = os.environ['ADMIN_BASE_URL'] @@ -95,7 +123,7 @@ class Config(object): BROKER_TRANSPORT_OPTIONS = { 'region': AWS_REGION, 'polling_interval': 1, # 1 second - 'visibility_timeout': 14410, # 4 hours 10 seconds. 10 seconds longer than max retry + 'visibility_timeout': 310, 'queue_name_prefix': NOTIFICATION_QUEUE_PREFIX } CELERY_ENABLE_UTC = True, @@ -107,59 +135,65 @@ class Config(object): 'run-scheduled-jobs': { 'task': 'run-scheduled-jobs', 'schedule': crontab(minute=1), - 'options': {'queue': 'periodic'} + 'options': {'queue': QueueNames.PERIODIC} }, + # 'send-scheduled-notifications': { + # 'task': 'send-scheduled-notifications', + # 'schedule': crontab(minute='*/15'), + # 'options': {'queue': 'periodic'} + # }, 'delete-verify-codes': { 'task': 'delete-verify-codes', 'schedule': timedelta(minutes=63), - 'options': {'queue': 'periodic'} + 'options': {'queue': QueueNames.PERIODIC} }, 'delete-invitations': { 'task': 'delete-invitations', 'schedule': timedelta(minutes=66), - 'options': {'queue': 'periodic'} + 'options': {'queue': QueueNames.PERIODIC} }, - 'delete-failed-notifications': { - 'task': 'delete-failed-notifications', + 'delete-sms-notifications': { + 'task': 'delete-sms-notifications', 'schedule': crontab(minute=0, hour=0), - 'options': {'queue': 'periodic'} + 'options': {'queue': QueueNames.PERIODIC} }, - 'delete-successful-notifications': { - 'task': 'delete-successful-notifications', - 'schedule': crontab(minute=0, hour=1), - 'options': {'queue': 'periodic'} + 'delete-email-notifications': { + 'task': 'delete-email-notifications', + 'schedule': crontab(minute=20, hour=0), + 'options': {'queue': QueueNames.PERIODIC} + }, + 'delete-letter-notifications': { + 'task': 'delete-letter-notifications', + 'schedule': crontab(minute=40, hour=0), + 'options': {'queue': QueueNames.PERIODIC} }, 'send-daily-performance-platform-stats': { 'task': 'send-daily-performance-platform-stats', 'schedule': crontab(minute=0, hour=2), - 'options': {'queue': 'periodic'} + 'options': {'queue': QueueNames.PERIODIC} }, 'switch-current-sms-provider-on-slow-delivery': { 'task': 'switch-current-sms-provider-on-slow-delivery', 'schedule': crontab(), # Every minute - 'options': {'queue': 'periodic'} + 'options': {'queue': QueueNames.PERIODIC} }, 'timeout-sending-notifications': { 'task': 'timeout-sending-notifications', 'schedule': crontab(minute=0, hour=3), - 'options': {'queue': 'periodic'} + 'options': {'queue': QueueNames.PERIODIC} }, 'remove_csv_files': { 'task': 'remove_csv_files', 'schedule': crontab(minute=0, hour=4), - 'options': {'queue': 'periodic'} + 'options': {'queue': QueueNames.PERIODIC} }, 'timeout-job-statistics': { 'task': 'timeout-job-statistics', 'schedule': crontab(minute=0, hour=5), - 'options': {'queue': 'periodic'} + 'options': {'queue': QueueNames.PERIODIC} } } - CELERY_QUEUES = [ - Queue('process-job', Exchange('default'), routing_key='process-job'), - Queue('retry', Exchange('default'), routing_key='retry'), - Queue('notify', Exchange('default'), routing_key='notify') - ] + CELERY_QUEUES = [] NOTIFICATIONS_ALERT = 5 # five mins FROM_NUMBER = 'development' @@ -210,17 +244,12 @@ class Development(Config): NOTIFY_ENVIRONMENT = 'development' NOTIFICATION_QUEUE_PREFIX = 'development' DEBUG = True - CELERY_QUEUES = Config.CELERY_QUEUES + [ - Queue('db-sms', Exchange('default'), routing_key='db-sms'), - Queue('priority', Exchange('default'), routing_key='priority'), - Queue('periodic', Exchange('default'), routing_key='periodic'), - Queue('db-email', Exchange('default'), routing_key='db-email'), - Queue('db-letter', Exchange('default'), routing_key='db-letter'), - Queue('send-sms', Exchange('default'), routing_key='send-sms'), - Queue('send-email', Exchange('default'), routing_key='send-email'), - Queue('research-mode', Exchange('default'), routing_key='research-mode'), - Queue('statistics', Exchange('default'), routing_key='statistics') - ] + + for queue in QueueNames.all_queues(): + Config.CELERY_QUEUES.append( + Queue(queue, Exchange('default'), routing_key=queue) + ) + API_HOST_NAME = "http://localhost:6011" API_RATE_LIMIT_ENABLED = True @@ -234,17 +263,11 @@ class Test(Config): STATSD_ENABLED = True STATSD_HOST = "localhost" STATSD_PORT = 1000 - CELERY_QUEUES = Config.CELERY_QUEUES + [ - Queue('periodic', Exchange('default'), routing_key='periodic'), - Queue('priority', Exchange('default'), routing_key='priority'), - Queue('db-sms', Exchange('default'), routing_key='db-sms'), - Queue('db-email', Exchange('default'), routing_key='db-email'), - Queue('db-letter', Exchange('default'), routing_key='db-letter'), - Queue('send-sms', Exchange('default'), routing_key='send-sms'), - Queue('send-email', Exchange('default'), routing_key='send-email'), - Queue('research-mode', Exchange('default'), routing_key='research-mode'), - Queue('statistics', Exchange('default'), routing_key='statistics') - ] + + for queue in QueueNames.all_queues(): + Config.CELERY_QUEUES.append( + Queue(queue, Exchange('default'), routing_key=queue) + ) API_RATE_LIMIT_ENABLED = True API_HOST_NAME = "http://localhost:6011" diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index da81c348b..f4ff2a24f 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -5,9 +5,17 @@ from datetime import ( date) from flask import current_app + +from notifications_utils.recipients import ( + validate_and_format_phone_number, + validate_and_format_email_address, + InvalidPhoneError, + InvalidEmailError, +) from werkzeug.datastructures import MultiDict from sqlalchemy import (desc, func, or_, and_, asc) from sqlalchemy.orm import joinedload +from notifications_utils.international_billing_rates import INTERNATIONAL_BILLING_RATES from app import db, create_uuid from app.dao import days_ago @@ -27,7 +35,7 @@ from app.models import ( NOTIFICATION_PERMANENT_FAILURE, KEY_TYPE_NORMAL, KEY_TYPE_TEST, LETTER_TYPE, - NOTIFICATION_SENT) + NOTIFICATION_SENT, ScheduledNotification) from app.dao.dao_utils import transactional from app.statsd_decorators import statsd @@ -163,6 +171,10 @@ def _decide_permanent_temporary_failure(current_status, status): return status +def country_records_delivery(phone_prefix): + return INTERNATIONAL_BILLING_RATES[phone_prefix]['attributes']['dlr'].lower() == 'yes' + + def _update_notification_status(notification, status): status = _decide_permanent_temporary_failure(current_status=notification.status, status=status) notification.status = status @@ -182,7 +194,10 @@ def update_notification_status_by_id(notification_id, status): Notification.status == NOTIFICATION_SENT )).first() - if not notification or notification.status == NOTIFICATION_SENT: + if not notification: + return None + + if notification.international and not country_records_delivery(notification.phone_prefix): return None return _update_notification_status( @@ -351,11 +366,11 @@ def _filter_query(query, filter_dict=None): @statsd(namespace="dao") -def delete_notifications_created_more_than_a_week_ago(status): +def delete_notifications_created_more_than_a_week_ago_by_type(notification_type): seven_days_ago = date.today() - timedelta(days=7) deleted = db.session.query(Notification).filter( func.date(Notification.created_at) < seven_days_ago, - Notification.status == status, + Notification.notification_type == notification_type, ).delete(synchronize_session='fetch') db.session.commit() return deleted @@ -459,7 +474,48 @@ def dao_update_notifications_sent_to_dvla(job_id, provider): @statsd(namespace="dao") -def dao_get_notifications_by_to_field(service_id, search_term): - return Notification.query.filter( +def dao_get_notifications_by_to_field(service_id, search_term, statuses=None): + try: + normalised = validate_and_format_phone_number(search_term) + except InvalidPhoneError: + try: + normalised = validate_and_format_email_address(search_term) + except InvalidEmailError: + normalised = search_term + + filters = [ Notification.service_id == service_id, - func.replace(func.lower(Notification.to), " ", "") == search_term.lower().replace(" ", "")).all() + Notification.normalised_to == normalised + ] + + if statuses: + filters.append(Notification.status.in_(statuses)) + + results = db.session.query(Notification).filter(*filters).all() + return results + + +@statsd(namespace="dao") +def dao_created_scheduled_notification(scheduled_notification): + db.session.add(scheduled_notification) + db.session.commit() + + +@statsd(namespace="dao") +def dao_get_scheduled_notifications(): + notifications = Notification.query.join( + ScheduledNotification + ).filter( + ScheduledNotification.scheduled_for < datetime.utcnow(), + ScheduledNotification.pending).all() + + return notifications + + +def set_scheduled_notification_to_processed(notification_id): + db.session.query(ScheduledNotification).filter( + ScheduledNotification.notification_id == notification_id + ).update( + {'pending': False} + ) + db.session.commit() diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index 0e0701560..ff06c356c 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -3,6 +3,7 @@ from datetime import date, datetime, timedelta from sqlalchemy import asc, func from sqlalchemy.orm import joinedload +from flask import current_app from app import db from app.dao.dao_utils import ( @@ -31,7 +32,9 @@ from app.models import ( TEMPLATE_TYPES, JobStatistics, SMS_TYPE, - EMAIL_TYPE + EMAIL_TYPE, + INTERNATIONAL_SMS_TYPE, + LETTER_TYPE ) from app.service.statistics import format_monthly_template_notification_stats from app.statsd_decorators import statsd @@ -129,6 +132,12 @@ def dao_fetch_service_by_id_and_user(service_id, user_id): @transactional @version_class(Service) def dao_create_service(service, user, service_id=None, service_permissions=[SMS_TYPE, EMAIL_TYPE]): + # the default property does not appear to work when there is a difference between the sqlalchemy schema and the + # db schema (ie: during a migration), so we have to set sms_sender manually here. After the GOVUK sms_sender + # migration is completed, this code should be able to be removed. + if not service.sms_sender: + service.sms_sender = current_app.config['FROM_NUMBER'] + from app.dao.permissions_dao import permission_dao service.users.append(user) permission_dao.add_default_service_permissions_for_user(user, service) @@ -136,10 +145,17 @@ def dao_create_service(service, user, service_id=None, service_permissions=[SMS_ service.active = True service.research_mode = False - for permission in service_permissions: - service_permission = ServicePermission(service_id=service.id, permission=permission) - db.session.add(service_permission) + def deprecate_process_service_permissions(): + for permission in service_permissions: + service_permission = ServicePermission(service_id=service.id, permission=permission) + service.permissions.append(service_permission) + if permission == INTERNATIONAL_SMS_TYPE: + service.can_send_international_sms = True + if permission == LETTER_TYPE: + service.can_send_letters = True + + deprecate_process_service_permissions() db.session.add(service) diff --git a/app/delivery/rest.py b/app/delivery/rest.py index 0bacb43bb..489a5fcda 100644 --- a/app/delivery/rest.py +++ b/app/delivery/rest.py @@ -1,5 +1,6 @@ from flask import Blueprint, jsonify +from app.config import QueueNames from app.delivery import send_to_providers from app.models import EMAIL_TYPE from app.celery import provider_tasks @@ -23,18 +24,16 @@ def send_notification_to_provider(notification_id): send_response( send_to_providers.send_email_to_provider, provider_tasks.deliver_email, - notification, - 'send-email') + notification) else: send_response( send_to_providers.send_sms_to_provider, provider_tasks.deliver_sms, - notification, - 'send-sms') + notification) return jsonify({}), 204 -def send_response(send_call, task_call, notification, queue): +def send_response(send_call, task_call, notification): try: send_call(notification) except Exception as e: @@ -43,4 +42,4 @@ def send_response(send_call, task_call, notification, queue): notification.id, notification.notification_type), e) - task_call.apply_async((str(notification.id)), queue=queue) + task_call.apply_async((str(notification.id)), queue=QueueNames.SEND) diff --git a/app/invite/rest.py b/app/invite/rest.py index 2361629d1..8105b171f 100644 --- a/app/invite/rest.py +++ b/app/invite/rest.py @@ -4,6 +4,7 @@ from flask import ( jsonify, current_app) +from app.config import QueueNames from app.dao.invited_user_dao import ( save_invited_user, get_invited_user, @@ -44,7 +45,7 @@ def create_invited_user(service_id): key_type=KEY_TYPE_NORMAL ) - send_notification_to_queue(saved_notification, False, queue="notify") + send_notification_to_queue(saved_notification, False, queue=QueueNames.NOTIFY) return jsonify(data=invited_user_schema.dump(invited_user).data), 201 diff --git a/app/job/rest.py b/app/job/rest.py index 8195ca87a..b4882945f 100644 --- a/app/job/rest.py +++ b/app/job/rest.py @@ -34,6 +34,8 @@ from app.models import JOB_STATUS_SCHEDULED, JOB_STATUS_PENDING, JOB_STATUS_CANC from app.utils import pagination_links +from app.config import QueueNames + job_blueprint = Blueprint('job', __name__, url_prefix='/service//job') from app.errors import ( @@ -143,7 +145,7 @@ def create_job(service_id): dao_create_job(job) if job.job_status == JOB_STATUS_PENDING: - process_job.apply_async([str(job.id)], queue="process-job") + process_job.apply_async([str(job.id)], queue=QueueNames.JOBS) job_json = job_schema.dump(job).data job_json['statistics'] = [] diff --git a/app/letters/send_letter_jobs.py b/app/letters/send_letter_jobs.py index 7030b9bc5..91c39615a 100644 --- a/app/letters/send_letter_jobs.py +++ b/app/letters/send_letter_jobs.py @@ -2,6 +2,7 @@ from flask import Blueprint, jsonify from flask import request from app import notify_celery +from app.config import QueueNames from app.dao.jobs_dao import dao_get_all_letter_jobs from app.schemas import job_schema from app.v2.errors import register_errors @@ -15,7 +16,7 @@ register_errors(letter_job) @letter_job.route('/send-letter-jobs', methods=['POST']) def send_letter_jobs(): job_ids = validate(request.get_json(), letter_job_ids) - notify_celery.send_task(name="send-files-to-dvla", args=(job_ids['job_ids'],), queue="process-ftp") + notify_celery.send_task(name="send-files-to-dvla", args=(job_ids['job_ids'],), queue=QueueNames.PROCESS_FTP) return jsonify(data={"response": "Task created to send files to DVLA"}), 201 diff --git a/app/models.py b/app/models.py index 79a873e95..6e258a98e 100644 --- a/app/models.py +++ b/app/models.py @@ -30,7 +30,7 @@ from app import ( ) from app.history_meta import Versioned -from app.utils import get_utc_time_in_bst +from app.utils import convert_utc_time_in_bst, convert_bst_to_utc SMS_TYPE = 'sms' EMAIL_TYPE = 'email' @@ -146,8 +146,10 @@ class DVLAOrganisation(db.Model): INTERNATIONAL_SMS_TYPE = 'international_sms' INBOUND_SMS_TYPE = 'inbound_sms' +SCHEDULE_NOTIFICATIONS = 'schedule_notifications' -SERVICE_PERMISSION_TYPES = [EMAIL_TYPE, SMS_TYPE, LETTER_TYPE, INTERNATIONAL_SMS_TYPE, INBOUND_SMS_TYPE] +SERVICE_PERMISSION_TYPES = [EMAIL_TYPE, SMS_TYPE, LETTER_TYPE, INTERNATIONAL_SMS_TYPE, INBOUND_SMS_TYPE, + SCHEDULE_NOTIFICATIONS] class ServicePermissionTypes(db.Model): @@ -215,6 +217,21 @@ class Service(db.Model, Versioned): self.can_send_letters = LETTER_TYPE in [p.permission for p in self.permissions] self.can_send_international_sms = INTERNATIONAL_SMS_TYPE in [p.permission for p in self.permissions] + @classmethod + def from_json(cls, data): + """ + Assumption: data has been validated appropriately. + + Returns a Service object based on the provided data. Deserialises created_by to created_by_id as marshmallow + would. + """ + # validate json with marshmallow + fields = data.copy() + + fields['created_by_id'] = fields.pop('created_by') + + return cls(**fields) + class ServicePermission(db.Model): __tablename__ = "service_permissions" @@ -226,7 +243,8 @@ class ServicePermission(db.Model): service = db.relationship("Service") created_at = db.Column(db.DateTime, default=datetime.datetime.utcnow, nullable=False) - service_permission_types = db.relationship(Service, backref=db.backref("permissions")) + service_permission_types = db.relationship( + Service, backref=db.backref("permissions", cascade="all, delete-orphan")) def __repr__(self): return '<{} has service permission: {}>'.format(self.service_id, self.permission) @@ -682,6 +700,7 @@ class Notification(db.Model): id = db.Column(UUID(as_uuid=True), primary_key=True, default=uuid.uuid4) to = db.Column(db.String, nullable=False) + normalised_to = db.Column(db.String, nullable=True) job_id = db.Column(UUID(as_uuid=True), db.ForeignKey('jobs.id'), index=True, unique=False) job = db.relationship('Job', backref=db.backref('notifications', lazy='dynamic')) job_row_number = db.Column(db.Integer, nullable=True) @@ -730,6 +749,8 @@ class Notification(db.Model): foreign(template_version) == remote(TemplateHistory.version) )) + scheduled_notification = db.relationship('ScheduledNotification', uselist=False) + client_reference = db.Column(db.String, index=True, nullable=True) international = db.Column(db.Boolean, nullable=False, default=False) @@ -851,7 +872,7 @@ class Notification(db.Model): }[self.template.template_type].get(self.status, self.status) def serialize_for_csv(self): - created_at_in_bst = get_utc_time_in_bst(self.created_at) + created_at_in_bst = convert_utc_time_in_bst(self.created_at) serialized = { "row_number": '' if self.job_row_number is None else self.job_row_number + 1, "recipient": self.to, @@ -890,7 +911,9 @@ class Notification(db.Model): "subject": self.subject, "created_at": self.created_at.strftime(DATETIME_FORMAT), "sent_at": self.sent_at.strftime(DATETIME_FORMAT) if self.sent_at else None, - "completed_at": self.completed_at() + "completed_at": self.completed_at(), + "scheduled_for": convert_bst_to_utc(self.scheduled_notification.scheduled_for + ).strftime(DATETIME_FORMAT) if self.scheduled_notification else None } return serialized @@ -956,6 +979,16 @@ class NotificationHistory(db.Model, HistoryModel): INVITED_USER_STATUS_TYPES = ['pending', 'accepted', 'cancelled'] +class ScheduledNotification(db.Model): + __tablename__ = 'scheduled_notifications' + + id = db.Column(UUID(as_uuid=True), primary_key=True, default=uuid.uuid4) + notification_id = db.Column(UUID(as_uuid=True), db.ForeignKey('notifications.id'), index=True, nullable=False) + notification = db.relationship('Notification', uselist=False) + scheduled_for = db.Column(db.DateTime, index=False, nullable=False) + pending = db.Column(db.Boolean, nullable=False, default=True) + + class InvitedUser(db.Model): __tablename__ = 'invited_users' diff --git a/app/notifications/notifications_letter_callback.py b/app/notifications/notifications_letter_callback.py index cfcaf28df..ac2de9e5c 100644 --- a/app/notifications/notifications_letter_callback.py +++ b/app/notifications/notifications_letter_callback.py @@ -13,7 +13,7 @@ from app.celery.tasks import update_letter_notifications_statuses from app.v2.errors import register_errors from app.notifications.utils import autoconfirm_subscription from app.schema_validation import validate - +from app.config import QueueNames letter_callback_blueprint = Blueprint('notifications_letter_callback', __name__) register_errors(letter_callback_blueprint) @@ -54,7 +54,7 @@ def process_letter_response(): filename = message['Records'][0]['s3']['object']['key'] current_app.logger.info('Received file from DVLA: {}'.format(filename)) current_app.logger.info('DVLA callback: Calling task to update letter notifications') - update_letter_notifications_statuses.apply_async([filename], queue='notify') + update_letter_notifications_statuses.apply_async([filename], queue=QueueNames.NOTIFY) return jsonify( result="success", message="DVLA callback succeeded" diff --git a/app/notifications/process_notifications.py b/app/notifications/process_notifications.py index 1d03efe8b..86291b46f 100644 --- a/app/notifications/process_notifications.py +++ b/app/notifications/process_notifications.py @@ -4,16 +4,21 @@ from flask import current_app from notifications_utils.recipients import ( get_international_phone_info, - validate_and_format_phone_number + validate_and_format_phone_number, + format_email_address ) from app import redis_store from app.celery import provider_tasks from notifications_utils.clients import redis -from app.dao.notifications_dao import dao_create_notification, dao_delete_notifications_and_history_by_id -from app.models import SMS_TYPE, Notification, KEY_TYPE_TEST, EMAIL_TYPE + +from app.config import QueueNames +from app.models import SMS_TYPE, Notification, KEY_TYPE_TEST, EMAIL_TYPE, ScheduledNotification +from app.dao.notifications_dao import (dao_create_notification, + dao_delete_notifications_and_history_by_id, + dao_created_scheduled_notification) from app.v2.errors import BadRequestError, SendNotificationToQueueError -from app.utils import get_template_instance, cache_key_for_service_template_counter +from app.utils import get_template_instance, cache_key_for_service_template_counter, convert_bst_to_utc def create_content_for_notification(template, personalisation): @@ -70,9 +75,12 @@ def persist_notification( if notification_type == SMS_TYPE: formatted_recipient = validate_and_format_phone_number(recipient, international=True) recipient_info = get_international_phone_info(formatted_recipient) + notification.normalised_to = formatted_recipient notification.international = recipient_info.international notification.phone_prefix = recipient_info.country_prefix notification.rate_multiplier = recipient_info.billable_units + elif notification_type == EMAIL_TYPE: + notification.normalised_to = format_email_address(notification.to) # if simulated create a Notification model to return but do not persist the Notification to the dB if not simulated: @@ -90,12 +98,9 @@ def persist_notification( def send_notification_to_queue(notification, research_mode, queue=None): if research_mode or notification.key_type == KEY_TYPE_TEST: - queue = 'research-mode' + queue = QueueNames.RESEARCH_MODE elif not queue: - if notification.notification_type == SMS_TYPE: - queue = 'send-sms' - if notification.notification_type == EMAIL_TYPE: - queue = 'send-email' + queue = QueueNames.SEND if notification.notification_type == SMS_TYPE: deliver_task = provider_tasks.deliver_sms @@ -123,3 +128,10 @@ def simulated_recipient(to_address, notification_type): return to_address in formatted_simulated_numbers else: return to_address in current_app.config['SIMULATED_EMAIL_ADDRESSES'] + + +def persist_scheduled_notification(notification_id, scheduled_for): + scheduled_datetime = convert_bst_to_utc(datetime.strptime(scheduled_for, "%Y-%m-%d %H:%M")) + scheduled_notification = ScheduledNotification(notification_id=notification_id, + scheduled_for=scheduled_datetime) + dao_created_scheduled_notification(scheduled_notification) diff --git a/app/notifications/receive_notifications.py b/app/notifications/receive_notifications.py index 08122fb41..923701cda 100644 --- a/app/notifications/receive_notifications.py +++ b/app/notifications/receive_notifications.py @@ -1,5 +1,5 @@ from flask import Blueprint -from flask import current_app +from flask import current_app, jsonify from flask import request from app.errors import register_errors @@ -15,3 +15,13 @@ def receive_mmg_sms(): current_app.logger.info("Recieve notification form data: {}".format(post_data)) return "RECEIVED" + + +@receive_notifications_blueprint.route('/notifications/sms/receive/firetext', methods=['POST']) +def receive_firetext_sms(): + post_data = request.form + current_app.logger.info("Received Firetext notification form data: {}".format(post_data)) + + return jsonify({ + "status": "ok" + }), 200 diff --git a/app/notifications/rest.py b/app/notifications/rest.py index faa086e32..bb136f8c8 100644 --- a/app/notifications/rest.py +++ b/app/notifications/rest.py @@ -6,6 +6,7 @@ from flask import ( ) from app import api_user, authenticated_service +from app.config import QueueNames from app.dao import ( templates_dao, notifications_dao @@ -134,7 +135,7 @@ def send_notification(notification_type): key_type=api_user.key_type, simulated=simulated) if not simulated: - queue_name = 'priority' if template.process_type == PRIORITY else None + queue_name = QueueNames.PRIORITY if template.process_type == PRIORITY else None send_notification_to_queue(notification=notification_model, research_mode=authenticated_service.research_mode, queue=queue_name) diff --git a/app/notifications/validators.py b/app/notifications/validators.py index a680f446e..5663a6046 100644 --- a/app/notifications/validators.py +++ b/app/notifications/validators.py @@ -6,7 +6,7 @@ from notifications_utils.recipients import ( ) from app.dao import services_dao -from app.models import KEY_TYPE_TEST, KEY_TYPE_TEAM, SMS_TYPE +from app.models import KEY_TYPE_TEST, KEY_TYPE_TEAM, SMS_TYPE, SCHEDULE_NOTIFICATIONS from app.service.utils import service_allowed_to_send_to from app.v2.errors import TooManyRequestsError, BadRequestError, RateLimitError from app import redis_store @@ -90,3 +90,9 @@ def check_sms_content_char_count(content_count): if content_count > char_count_limit: message = 'Content for template has a character count greater than the limit of {}'.format(char_count_limit) raise BadRequestError(message=message) + + +def service_can_schedule_notification(service, scheduled_for): + if scheduled_for: + if SCHEDULE_NOTIFICATIONS not in [p.permission for p in service.permissions]: + raise BadRequestError(message="Cannot schedule notifications (this feature is invite-only)") diff --git a/app/schema_validation/__init__.py b/app/schema_validation/__init__.py index 376315a8a..9202458eb 100644 --- a/app/schema_validation/__init__.py +++ b/app/schema_validation/__init__.py @@ -1,5 +1,7 @@ import json +from datetime import datetime, timedelta +from iso8601 import iso8601, ParseError from jsonschema import (Draft4Validator, ValidationError, FormatChecker) from notifications_utils.recipients import (validate_phone_number, validate_email_address, InvalidPhoneError, InvalidEmailError) @@ -20,6 +22,20 @@ def validate(json_to_validate, schema): validate_email_address(instance) return True + @format_checker.checks('datetime', raises=ValidationError) + def validate_schema_date_with_hour(instance): + if isinstance(instance, str): + try: + dt = iso8601.parse_date(instance).replace(tzinfo=None) + if dt < datetime.utcnow(): + raise ValidationError("datetime can not be in the past") + if dt > datetime.utcnow() + timedelta(hours=24): + raise ValidationError("datetime can only be 24 hours in the future") + except ParseError: + raise ValidationError("datetime format is invalid. It must be a valid ISO8601 date time format, " + "https://en.wikipedia.org/wiki/ISO_8601") + return True + validator = Draft4Validator(schema, format_checker=format_checker) errors = list(validator.iter_errors(json_to_validate)) if errors.__len__() > 0: diff --git a/app/schemas.py b/app/schemas.py index aa8980865..b8d3ee7af 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -25,7 +25,9 @@ from notifications_utils.recipients import ( from app import ma from app import models +from app.models import ServicePermission, INTERNATIONAL_SMS_TYPE, SMS_TYPE, LETTER_TYPE, EMAIL_TYPE from app.dao.permissions_dao import permission_dao +from app.dao.service_permissions_dao import dao_fetch_service_permissions from app.utils import get_template_instance @@ -178,18 +180,25 @@ class ServiceSchema(BaseSchema): organisation = field_for(models.Service, 'organisation') branding = field_for(models.Service, 'branding') dvla_organisation = field_for(models.Service, 'dvla_organisation') + permissions = fields.Method("service_permissions") + override_flag = False + + def service_permissions(self, service): + return [p.permission for p in service.permissions] class Meta: model = models.Service - exclude = ('updated_at', - 'created_at', - 'api_keys', - 'templates', - 'jobs', - 'old_id', - 'template_statistics', - 'service_provider_stats', - 'service_notification_stats') + exclude = ( + 'updated_at', + 'created_at', + 'api_keys', + 'templates', + 'jobs', + 'old_id', + 'template_statistics', + 'service_provider_stats', + 'service_notification_stats', + ) strict = True @validates('sms_sender') @@ -197,6 +206,52 @@ class ServiceSchema(BaseSchema): if value and not re.match(r'^[a-zA-Z0-9\s]+$', value): raise ValidationError('Only alphanumeric characters allowed') + @validates('permissions') + def validate_permissions(self, value): + permissions = [v.permission for v in value] + for p in permissions: + if p not in models.SERVICE_PERMISSION_TYPES: + raise ValidationError("Invalid Service Permission: '{}'".format(p)) + + if len(set(permissions)) != len(permissions): + duplicates = list(set([x for x in permissions if permissions.count(x) > 1])) + raise ValidationError('Duplicate Service Permission: {}'.format(duplicates)) + + @pre_load() + def format_for_data_model(self, in_data): + if isinstance(in_data, dict) and 'permissions' in in_data: + str_permissions = in_data['permissions'] + permissions = [] + for p in str_permissions: + permission = ServicePermission(service_id=in_data["id"], permission=p) + permissions.append(permission) + + def deprecate_override_flags(): + in_data['can_send_letters'] = LETTER_TYPE in str_permissions + in_data['can_send_international_sms'] = INTERNATIONAL_SMS_TYPE in str_permissions + + def deprecate_convert_flags_to_permissions(): + def convert_flags(flag, notify_type): + if flag and notify_type not in str_permissions: + permission = ServicePermission(service_id=in_data['id'], permission=notify_type) + permissions.append(permission) + elif flag is False and notify_type in str_permissions: + for p in permissions: + if p.permission == notify_type: + permissions.remove(p) + + convert_flags(in_data["can_send_international_sms"], INTERNATIONAL_SMS_TYPE) + convert_flags(in_data["can_send_letters"], LETTER_TYPE) + + if self.override_flag: + deprecate_override_flags() + else: + deprecate_convert_flags_to_permissions() + in_data['permissions'] = permissions + + def set_override_flag(self, flag): + self.override_flag = flag + class DetailedServiceSchema(BaseSchema): statistics = fields.Dict() diff --git a/app/service/rest.py b/app/service/rest.py index e8c585d1f..24fdcd513 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -5,12 +5,13 @@ from datetime import datetime from flask import ( jsonify, request, - current_app + current_app, + Blueprint ) from sqlalchemy.orm.exc import NoResultFound from app import redis_store -from app.dao import notification_usage_dao +from app.dao import notification_usage_dao, notifications_dao from app.dao.dao_utils import dao_rollback from app.dao.api_key_dao import ( save_model_api_key, @@ -42,11 +43,13 @@ from app.dao.service_whitelist_dao import ( dao_add_and_commit_whitelisted_contacts, dao_remove_service_whitelist ) -from app.dao import notifications_dao from app.dao.provider_statistics_dao import get_fragment_count from app.dao.users_dao import get_user_by_id from app.errors import ( - InvalidRequest, register_errors) + InvalidRequest, + register_errors +) +from app.models import Service from app.service import statistics from app.service.utils import get_whitelist_objects from app.service.sender import send_notification_to_service_users @@ -60,7 +63,6 @@ from app.schemas import ( detailed_service_schema ) from app.utils import pagination_links -from flask import Blueprint from notifications_utils.clients.redis import sms_billable_units_cache_key service_blueprint = Blueprint('service', __name__) @@ -97,12 +99,11 @@ def get_services(): def get_service_by_id(service_id): if request.args.get('detailed') == 'True': data = get_detailed_service(service_id, today_only=request.args.get('today_only') == 'True') - return jsonify(data=data) else: fetched = dao_fetch_service_by_id(service_id) data = service_schema.dump(fetched).data - return jsonify(data=data) + return jsonify(data=data) @service_blueprint.route('', methods=['POST']) @@ -112,9 +113,14 @@ def create_service(): errors = {'user_id': ['Missing data for required field.']} raise InvalidRequest(errors, status_code=400) - user = get_user_by_id(data['user_id']) - data.pop('user_id', None) - valid_service = service_schema.load(request.get_json()).data + # validate json with marshmallow + service_schema.load(request.get_json()) + + user = get_user_by_id(data.pop('user_id', None)) + + # unpack valid json into service object + valid_service = Service.from_json(data) + dao_create_service(valid_service, user) return jsonify(data=service_schema.dump(valid_service).data), 201 @@ -126,6 +132,7 @@ def update_service(service_id): service_going_live = fetched_service.restricted and not request.get_json().get('restricted', True) current_data = dict(service_schema.dump(fetched_service).data.items()) + service_schema.set_override_flag(request.get_json().get('permissions') is not None) current_data.update(request.get_json()) update_dict = service_schema.load(current_data).data dao_update_service(update_dict) @@ -262,8 +269,8 @@ def get_service_history(service_id): @service_blueprint.route('//notifications', methods=['GET']) def get_all_notifications_for_service(service_id): data = notifications_filter_schema.load(request.args).data - if data.get("to", None): - return search_for_notification_by_to_field(service_id, request.query_string.decode()) + if data.get('to'): + return search_for_notification_by_to_field(service_id, data['to'], statuses=data.get('status')) page = data['page'] if 'page' in data else 1 page_size = data['page_size'] if 'page_size' in data else current_app.config.get('PAGE_SIZE') limit_days = data.get('limit_days') @@ -293,11 +300,11 @@ def get_all_notifications_for_service(service_id): ), 200 -def search_for_notification_by_to_field(service_id, search_term): - search_term = search_term.replace('to=', '') - results = notifications_dao.dao_get_notifications_by_to_field(service_id, search_term) +def search_for_notification_by_to_field(service_id, search_term, statuses): + results = notifications_dao.dao_get_notifications_by_to_field(service_id, search_term, statuses) return jsonify( - notifications=notification_with_template_schema.dump(results, many=True).data), 200 + notifications=notification_with_template_schema.dump(results, many=True).data + ), 200 @service_blueprint.route('//notifications/monthly', methods=['GET']) diff --git a/app/service/sender.py b/app/service/sender.py index 3c6a6a03e..4919a93bf 100644 --- a/app/service/sender.py +++ b/app/service/sender.py @@ -1,5 +1,6 @@ from flask import current_app +from app.config import QueueNames from app.dao.services_dao import dao_fetch_service_by_id, dao_fetch_active_users_for_service from app.dao.templates_dao import dao_get_template_by_id from app.models import EMAIL_TYPE, KEY_TYPE_NORMAL @@ -24,7 +25,7 @@ def send_notification_to_service_users(service_id, template_id, personalisation= api_key_id=None, key_type=KEY_TYPE_NORMAL ) - send_notification_to_queue(notification, False, queue='notify') + send_notification_to_queue(notification, False, queue=QueueNames.NOTIFY) def _add_user_fields(user, personalisation, fields): diff --git a/app/user/rest.py b/app/user/rest.py index b40c3a31c..cc0c6e41b 100644 --- a/app/user/rest.py +++ b/app/user/rest.py @@ -4,6 +4,7 @@ from datetime import datetime from flask import (jsonify, request, Blueprint, current_app) +from app.config import QueueNames from app.dao.users_dao import ( get_user_by_id, save_model_user, @@ -182,7 +183,7 @@ def send_user_sms_code(user_id): # Assume that we never want to observe the Notify service's research mode # setting for this notification - we still need to be able to log into the # admin even if we're doing user research using this service: - send_notification_to_queue(saved_notification, False, queue='notify') + send_notification_to_queue(saved_notification, False, queue=QueueNames.NOTIFY) return jsonify({}), 204 @@ -212,7 +213,7 @@ def send_user_confirm_new_email(user_id): key_type=KEY_TYPE_NORMAL ) - send_notification_to_queue(saved_notification, False, queue='notify') + send_notification_to_queue(saved_notification, False, queue=QueueNames.NOTIFY) return jsonify({}), 204 @@ -239,7 +240,7 @@ def send_user_email_verification(user_id): key_type=KEY_TYPE_NORMAL ) - send_notification_to_queue(saved_notification, False, queue="notify") + send_notification_to_queue(saved_notification, False, queue=QueueNames.NOTIFY) return jsonify({}), 204 @@ -265,7 +266,7 @@ def send_already_registered_email(user_id): key_type=KEY_TYPE_NORMAL ) - send_notification_to_queue(saved_notification, False, queue="notify") + send_notification_to_queue(saved_notification, False, queue=QueueNames.NOTIFY) return jsonify({}), 204 @@ -327,7 +328,7 @@ def send_user_reset_password(): key_type=KEY_TYPE_NORMAL ) - send_notification_to_queue(saved_notification, False, queue="notify") + send_notification_to_queue(saved_notification, False, queue=QueueNames.NOTIFY) return jsonify({}), 204 diff --git a/app/utils.py b/app/utils.py index 34bb1ec3a..d93ea0613 100644 --- a/app/utils.py +++ b/app/utils.py @@ -51,10 +51,14 @@ def get_midnight_for_day_before(date): return get_london_midnight_in_utc(day_before) -def get_utc_time_in_bst(utc_dt): +def convert_utc_time_in_bst(utc_dt): return pytz.utc.localize(utc_dt).astimezone(local_timezone).replace(tzinfo=None) +def convert_bst_to_utc(date): + return local_timezone.localize(date).astimezone(pytz.UTC).replace(tzinfo=None) + + def get_london_month_from_utc_column(column): """ Where queries need to count notifications by month it needs to be diff --git a/app/v2/notifications/notification_schemas.py b/app/v2/notifications/notification_schemas.py index 240d41324..69372c4e1 100644 --- a/app/v2/notifications/notification_schemas.py +++ b/app/v2/notifications/notification_schemas.py @@ -1,7 +1,7 @@ from app.models import NOTIFICATION_STATUS_TYPES, TEMPLATE_TYPES from app.schema_validation.definitions import (uuid, personalisation) -# this may belong in a templates module + template = { "$schema": "http://json-schema.org/draft-04/schema#", "description": "template schema", @@ -39,7 +39,8 @@ get_notification_response = { "subject": {"type": ["string", "null"]}, "created_at": {"type": "string"}, "sent_at": {"type": ["string", "null"]}, - "completed_at": {"type": ["string", "null"]} + "completed_at": {"type": ["string", "null"]}, + "scheduled_for": {"type": ["string", "null"]} }, "required": [ # technically, all keys are required since we always have all of them @@ -111,7 +112,8 @@ post_sms_request = { "reference": {"type": "string"}, "phone_number": {"type": "string", "format": "phone_number"}, "template_id": uuid, - "personalisation": personalisation + "personalisation": personalisation, + "scheduled_for": {"type": ["string", "null"], "format": "datetime"} }, "required": ["phone_number", "template_id"] } @@ -138,7 +140,8 @@ post_sms_response = { "reference": {"type": ["string", "null"]}, "content": sms_content, "uri": {"type": "string", "format": "uri"}, - "template": template + "template": template, + "scheduled_for": {"type": ["string", "null"]} }, "required": ["id", "content", "uri", "template"] } @@ -153,7 +156,8 @@ post_email_request = { "reference": {"type": "string"}, "email_address": {"type": "string", "format": "email_address"}, "template_id": uuid, - "personalisation": personalisation + "personalisation": personalisation, + "scheduled_for": {"type": ["string", "null"], "format": "datetime"} }, "required": ["email_address", "template_id"] } @@ -181,13 +185,14 @@ post_email_response = { "reference": {"type": ["string", "null"]}, "content": email_content, "uri": {"type": "string", "format": "uri"}, - "template": template + "template": template, + "scheduled_for": {"type": ["string", "null"]} }, "required": ["id", "content", "uri", "template"] } -def create_post_sms_response_from_notification(notification, body, from_number, url_root, service_id): +def create_post_sms_response_from_notification(notification, body, from_number, url_root, service_id, scheduled_for): return {"id": notification.id, "reference": notification.client_reference, "content": {'body': body, @@ -195,11 +200,13 @@ def create_post_sms_response_from_notification(notification, body, from_number, "uri": "{}v2/notifications/{}".format(url_root, str(notification.id)), "template": __create_template_from_notification(notification=notification, url_root=url_root, - service_id=service_id) + service_id=service_id), + "scheduled_for": scheduled_for if scheduled_for else None } -def create_post_email_response_from_notification(notification, content, subject, email_from, url_root, service_id): +def create_post_email_response_from_notification(notification, content, subject, email_from, url_root, service_id, + scheduled_for): return { "id": notification.id, "reference": notification.client_reference, @@ -211,7 +218,8 @@ def create_post_email_response_from_notification(notification, content, subject, "uri": "{}v2/notifications/{}".format(url_root, str(notification.id)), "template": __create_template_from_notification(notification=notification, url_root=url_root, - service_id=service_id) + service_id=service_id), + "scheduled_for": scheduled_for if scheduled_for else None } diff --git a/app/v2/notifications/post_notifications.py b/app/v2/notifications/post_notifications.py index a5a3a1ec4..47a40a72e 100644 --- a/app/v2/notifications/post_notifications.py +++ b/app/v2/notifications/post_notifications.py @@ -2,19 +2,21 @@ from flask import request, jsonify, current_app from sqlalchemy.orm.exc import NoResultFound from app import api_user, authenticated_service -from app.dao import services_dao, templates_dao +from app.config import QueueNames +from app.dao import templates_dao from app.models import SMS_TYPE, EMAIL_TYPE, PRIORITY from app.notifications.process_notifications import ( create_content_for_notification, persist_notification, send_notification_to_queue, - simulated_recipient) + simulated_recipient, + persist_scheduled_notification) from app.notifications.validators import ( check_template_is_for_notification_type, check_template_is_active, check_sms_content_char_count, validate_and_format_recipient, - check_rate_limiting) + check_rate_limiting, service_can_schedule_notification) from app.schema_validation import validate from app.v2.errors import BadRequestError from app.v2.notifications import v2_notification_blueprint @@ -32,6 +34,9 @@ def post_notification(notification_type): else: form = validate(request.get_json(), post_sms_request) + scheduled_for = form.get("scheduled_for", None) + service_can_schedule_notification(authenticated_service, scheduled_for) + check_rate_limiting(authenticated_service, api_user) form_send_to = form['phone_number'] if notification_type == SMS_TYPE else form['email_address'] @@ -56,31 +61,35 @@ def post_notification(notification_type): client_reference=form.get('reference', None), simulated=simulated) - if not simulated: - queue_name = 'priority' if template.process_type == PRIORITY else None - send_notification_to_queue( - notification=notification, - research_mode=authenticated_service.research_mode, - queue=queue_name - ) - + if scheduled_for: + persist_scheduled_notification(notification.id, form["scheduled_for"]) else: - current_app.logger.info("POST simulated notification for id: {}".format(notification.id)) + if not simulated: + queue_name = QueueNames.PRIORITY if template.process_type == PRIORITY else None + send_notification_to_queue( + notification=notification, + research_mode=authenticated_service.research_mode, + queue=queue_name + ) + else: + current_app.logger.info("POST simulated notification for id: {}".format(notification.id)) + if notification_type == SMS_TYPE: sms_sender = authenticated_service.sms_sender or current_app.config.get('FROM_NUMBER') resp = create_post_sms_response_from_notification(notification=notification, body=str(template_with_content), from_number=sms_sender, url_root=request.url_root, - service_id=authenticated_service.id) + service_id=authenticated_service.id, + scheduled_for=scheduled_for) else: resp = create_post_email_response_from_notification(notification=notification, content=str(template_with_content), subject=template_with_content.subject, email_from=authenticated_service.email_from, url_root=request.url_root, - service_id=authenticated_service.id) - + service_id=authenticated_service.id, + scheduled_for=scheduled_for) return jsonify(resp), 201 diff --git a/application.py b/application.py index 0b940dc87..b2a59a6fd 100644 --- a/application.py +++ b/application.py @@ -15,6 +15,7 @@ migrate = Migrate(application, db) manager.add_command('db', MigrateCommand) manager.add_command('create_provider_rate', commands.CreateProviderRateCommand) manager.add_command('purge_functional_test_data', commands.PurgeFunctionalTestDataCommand) +manager.add_command('custom_db_script', commands.CustomDbScript) @manager.command diff --git a/celerybeat.pid b/celerybeat.pid deleted file mode 100644 index d2acb3a18..000000000 --- a/celerybeat.pid +++ /dev/null @@ -1 +0,0 @@ -9772 diff --git a/manifest-api-preview.yml b/manifest-api-preview.yml index 04b396388..eccae21b1 100644 --- a/manifest-api-preview.yml +++ b/manifest-api-preview.yml @@ -6,3 +6,6 @@ routes: - route: notify-api-preview.cloudapps.digital - route: api-paas.notify.works - route: api.notify.works + +instances: 1 +memory: 1G diff --git a/manifest-delivery-base.yml b/manifest-delivery-base.yml index 73457b3c9..7f73a4b54 100644 --- a/manifest-delivery-base.yml +++ b/manifest-delivery-base.yml @@ -23,33 +23,32 @@ applications: NOTIFY_APP_NAME: delivery-celery-beat - name: notify-delivery-worker-database - command: scripts/run_app_paas.sh celery -A aws_run_celery.notify_celery worker --loglevel=INFO --concurrency=11 -Q db-sms,db-email,db-letter + command: scripts/run_app_paas.sh celery -A aws_run_celery.notify_celery worker --loglevel=INFO --concurrency=11 -Q database-tasks env: NOTIFY_APP_NAME: delivery-worker-database - name: notify-delivery-worker-research - command: scripts/run_app_paas.sh celery -A aws_run_celery.notify_celery worker --loglevel=INFO --concurrency=5 -Q research-mode + command: scripts/run_app_paas.sh celery -A aws_run_celery.notify_celery worker --loglevel=INFO --concurrency=5 -Q research-mode-tasks env: NOTIFY_APP_NAME: delivery-worker-research - name: notify-delivery-worker-sender - command: scripts/run_app_paas.sh celery -A aws_run_celery.notify_celery worker --loglevel=INFO --concurrency=11 -Q send-sms,send-email + command: scripts/run_app_paas.sh celery -A aws_run_celery.notify_celery worker --loglevel=INFO --concurrency=11 -Q send-tasks env: NOTIFY_APP_NAME: delivery-worker-sender - name: notify-delivery-worker-periodic - command: scripts/run_app_paas.sh celery -A aws_run_celery.notify_celery worker --loglevel=INFO --concurrency=2 -Q periodic,statistics + command: scripts/run_app_paas.sh celery -A aws_run_celery.notify_celery worker --loglevel=INFO --concurrency=2 -Q periodic-tasks,statistics-tasks instances: 1 - memory: 2G env: NOTIFY_APP_NAME: delivery-worker-periodic - name: notify-delivery-worker-priority - command: scripts/run_app_paas.sh celery -A aws_run_celery.notify_celery worker --loglevel=INFO --concurrency=5 -Q priority + command: scripts/run_app_paas.sh celery -A aws_run_celery.notify_celery worker --loglevel=INFO --concurrency=5 -Q priority-tasks env: NOTIFY_APP_NAME: delivery-worker-priority - name: notify-delivery-worker - command: scripts/run_app_paas.sh celery -A aws_run_celery.notify_celery worker --loglevel=INFO --concurrency=11 + command: scripts/run_app_paas.sh celery -A aws_run_celery.notify_celery worker --loglevel=INFO --concurrency=11 -Q job-tasks,retry-tasks,notify-internal-tasks env: NOTIFY_APP_NAME: delivery-worker diff --git a/manifest-delivery-preview.yml b/manifest-delivery-preview.yml index d628e5fc9..492bc6c55 100644 --- a/manifest-delivery-preview.yml +++ b/manifest-delivery-preview.yml @@ -1,3 +1,4 @@ --- inherit: manifest-delivery-base.yml +memory: 1G diff --git a/migrations/versions/0086_add_norm_to_notification.py b/migrations/versions/0086_add_norm_to_notification.py new file mode 100644 index 000000000..346d5b6dc --- /dev/null +++ b/migrations/versions/0086_add_norm_to_notification.py @@ -0,0 +1,21 @@ +""" + +Revision ID: 0086_add_norm_to_notification +Revises: 0085_update_incoming_to_inbound +Create Date: 2017-05-23 10:37:00.404087 + +""" + +from alembic import op +import sqlalchemy as sa + +revision = '0086_add_norm_to_notification' +down_revision = '0085_update_incoming_to_inbound' + + +def upgrade(): + op.add_column('notifications', sa.Column('normalised_to', sa.String(), nullable=True)) + + +def downgrade(): + op.drop_column('notifications', 'normalised_to') diff --git a/migrations/versions/0087_scheduled_notifications.py b/migrations/versions/0087_scheduled_notifications.py new file mode 100644 index 000000000..9066e8ac1 --- /dev/null +++ b/migrations/versions/0087_scheduled_notifications.py @@ -0,0 +1,31 @@ +"""empty message + +Revision ID: 0087_scheduled_notifications +Revises: 0086_add_norm_to_notification +Create Date: 2017-05-15 12:50:20.041950 + +""" +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +revision = '0087_scheduled_notifications' +down_revision = '0086_add_norm_to_notification' + + +def upgrade(): + op.create_table('scheduled_notifications', + sa.Column('id', postgresql.UUID(as_uuid=True), nullable=False), + sa.Column('notification_id', postgresql.UUID(as_uuid=True), nullable=False), + sa.Column('scheduled_for', sa.DateTime(), nullable=False), + sa.Column('pending', sa.Boolean, nullable=False, default=True), + sa.ForeignKeyConstraint(['notification_id'], ['notifications.id'], ), + sa.PrimaryKeyConstraint('id') + ) + op.create_index(op.f('ix_scheduled_notifications_notification_id'), 'scheduled_notifications', ['notification_id'], + unique=False) + + +def downgrade(): + op.drop_index(op.f('ix_scheduled_notifications_notification_id'), table_name='scheduled_notifications') + op.drop_table('scheduled_notifications') diff --git a/migrations/versions/0088_add_schedule_serv_perm.py b/migrations/versions/0088_add_schedule_serv_perm.py new file mode 100644 index 000000000..0882c7c94 --- /dev/null +++ b/migrations/versions/0088_add_schedule_serv_perm.py @@ -0,0 +1,24 @@ +"""empty message + +Revision ID: 0088_add_schedule_serv_perm +Revises: 0087_scheduled_notifications +Create Date: 2017-05-26 14:53:18.581320 + +""" + +# revision identifiers, used by Alembic. +revision = '0088_add_schedule_serv_perm' +down_revision = '0087_scheduled_notifications' + +from alembic import op + + +def upgrade(): + op.get_bind() + op.execute("insert into service_permission_types values('schedule_notifications')") + + +def downgrade(): + op.get_bind() + op.execute("delete from service_permissions where permission = 'schedule_notifications'") + op.execute("delete from service_permission_types where name = 'schedule_notifications'") diff --git a/requirements.txt b/requirements.txt index 4cc9907bb..46b02daa3 100644 --- a/requirements.txt +++ b/requirements.txt @@ -20,6 +20,7 @@ jsonschema==2.5.1 gunicorn==19.6.0 docopt==0.6.2 six==1.10.0 +iso8601==0.1.11 # pin to minor version 3.1.x notifications-python-client>=3.1,<3.2 diff --git a/scripts/check_if_new_migration.py b/scripts/check_if_new_migration.py new file mode 100644 index 000000000..ba0519ea1 --- /dev/null +++ b/scripts/check_if_new_migration.py @@ -0,0 +1,34 @@ +import os +from os.path import dirname, abspath +import requests +import sys + + +def get_latest_db_migration_to_apply(): + project_dir = dirname(dirname(abspath(__file__))) # Get the main project directory + migrations_dir = '{}/migrations/versions/'.format(project_dir) + migration_files = [migration_file for migration_file in os.listdir(migrations_dir) if migration_file.endswith('py')] + latest_file = sorted(migration_files, reverse=True)[0].replace('.py', '') + return latest_file + + +def get_current_db_version(): + api_status_url = '{}/_status'.format(os.getenv('API_HOST_NAME')) + response = requests.get(api_status_url) + + if response.status_code != 200: + sys.exit('Could not make a request to the API: {}'.format()) + + current_db_version = response.json()['db_version'] + return current_db_version + + +def run(): + if get_current_db_version() == get_latest_db_migration_to_apply(): + print('no') + else: + print('yes') + + +if __name__ == "__main__": + run() diff --git a/scripts/delete_sqs_queues.py b/scripts/delete_sqs_queues.py new file mode 100755 index 000000000..b167ce392 --- /dev/null +++ b/scripts/delete_sqs_queues.py @@ -0,0 +1,119 @@ +""" + +Script to manage SQS queues. Can list or delete queues. + +Uses boto, so relies on correctly set up AWS access keys and tokens. + +In principle use this script to dump details of all queues in a gievn environment, and then +manipulate the resultant CSV file so that it contains the queues you want to delete. + +Very hands on. Starter for a more automagic process. + +Usage: + scripts/delete_sqs_queues.py + + options are: + - list: dumps queue details to local file queues.csv in current directory. + - delete: delete queues from local file queues.csv in current directory. + +Example: + scripts/delete_sqs_queues.py list delete +""" + +from docopt import docopt +import boto3 +import csv +from datetime import datetime + +FILE_NAME = "/tmp/queues.csv" + +client = boto3.client('sqs', region_name='eu-west-1') + + +def _formatted_date_from_timestamp(timestamp): + return datetime.fromtimestamp( + int(timestamp) + ).strftime('%Y-%m-%d %H:%M:%S') + + +def get_queues(): + response = client.list_queues() + queues = response['QueueUrls'] + return queues + + +def get_queue_attributes(queue_name): + response = client.get_queue_attributes( + QueueUrl=queue_name, + AttributeNames=[ + 'All' + ] + ) + queue_attributes = response['Attributes'] + queue_attributes.update({'QueueUrl': queue_name}) + return queue_attributes + + +def delete_queue(queue_url): + # Note that deleting a queue returns 200 OK if it doesn't exist + print("DELETEING {}".format(queue_url)) + response = client.delete_queue( + QueueUrl=queue_url + ) + if response['ResponseMetadata']['HTTPStatusCode'] == 200: + print('Deleted queue successfully {}'.format(response['ResponseMetadata'])) + else: + print('Error occured when attempting to delete queue') + pprint(response) + return response + + +def output_to_csv(queue_attributes): + with open(FILE_NAME, 'w') as csvfile: + fieldnames = [ + 'Queue Name', + 'Queue URL', + 'Number of Messages', + 'Number of Messages Delayed', + 'Number of Messages Not Visible', + 'Created' + ] + writer = csv.DictWriter(csvfile, fieldnames=fieldnames) + writer.writeheader() + for queue_attr in queue_attributes: + writer.writerow({ + 'Queue Name': queue_attr['QueueArn'], + 'Queue URL': queue_attr['QueueUrl'], + 'Number of Messages': queue_attr['ApproximateNumberOfMessages'], + 'Number of Messages Delayed': queue_attr['ApproximateNumberOfMessagesDelayed'], + 'Number of Messages Not Visible': queue_attr['ApproximateNumberOfMessagesNotVisible'], + 'Created': _formatted_date_from_timestamp(queue_attr['CreatedTimestamp']) + }) + + +def read_from_csv(): + queue_urls = [] + with open(FILE_NAME, 'r') as csvfile: + next(csvfile) + rows = csv.reader(csvfile, delimiter=',') + for row in rows: + queue_urls.append(row[1]) + return queue_urls + + +if __name__ == "__main__": + arguments = docopt(__doc__) + + if arguments[''] == 'list': + queues = get_queues() + queue_attributes = [] + for queue in queues: + queue_attributes.append(get_queue_attributes(queue)) + output_to_csv(queue_attributes) + elif arguments[''] == 'delete': + queues_to_delete = read_from_csv() + for queue in queues_to_delete: + delete_queue(queue) + else: + print("UNKNOWN COMMAND") + exit(1) diff --git a/server_commands.py b/server_commands.py index 6289da765..85bf13137 100644 --- a/server_commands.py +++ b/server_commands.py @@ -25,6 +25,7 @@ manager = Manager(application) migrate = Migrate(application, db) manager.add_command('db', MigrateCommand) manager.add_command('purge_functional_test_data', commands.PurgeFunctionalTestDataCommand) +manager.add_command('custom_db_script', commands.CustomDbScript) if __name__ == '__main__': manager.run() diff --git a/tests/app/celery/test_provider_tasks.py b/tests/app/celery/test_provider_tasks.py index d2f815d9d..b5dbc999a 100644 --- a/tests/app/celery/test_provider_tasks.py +++ b/tests/app/celery/test_provider_tasks.py @@ -11,34 +11,6 @@ def test_should_have_decorated_tasks_functions(): assert deliver_email.__wrapped__.__name__ == 'deliver_email' -def test_should_by_10_second_delay_as_default(): - assert provider_tasks.retry_iteration_to_delay() == 10 - - -def test_should_by_10_second_delay_on_unmapped_retry_iteration(): - assert provider_tasks.retry_iteration_to_delay(99) == 10 - - -def test_should_by_10_second_delay_on_retry_one(): - assert provider_tasks.retry_iteration_to_delay(0) == 10 - - -def test_should_by_1_minute_delay_on_retry_two(): - assert provider_tasks.retry_iteration_to_delay(1) == 60 - - -def test_should_by_5_minute_delay_on_retry_two(): - assert provider_tasks.retry_iteration_to_delay(2) == 300 - - -def test_should_by_60_minute_delay_on_retry_two(): - assert provider_tasks.retry_iteration_to_delay(3) == 3600 - - -def test_should_by_240_minute_delay_on_retry_two(): - assert provider_tasks.retry_iteration_to_delay(4) == 14400 - - def test_should_call_send_sms_to_provider_from_deliver_sms_task( notify_db, notify_db_session, @@ -61,7 +33,7 @@ def test_should_add_to_retry_queue_if_notification_not_found_in_deliver_sms_task 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) + app.celery.provider_tasks.deliver_sms.retry.assert_called_with(queue="retry-tasks") def test_should_call_send_email_to_provider_from_deliver_email_task( @@ -83,7 +55,7 @@ def test_should_add_to_retry_queue_if_notification_not_found_in_deliver_email_ta 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) + app.celery.provider_tasks.deliver_email.retry.assert_called_with(queue="retry-tasks") # DO THESE FOR THE 4 TYPES OF TASK @@ -94,7 +66,7 @@ def test_should_go_into_technical_error_if_exceeds_retries_on_deliver_sms_task(s deliver_sms(sample_notification.id) - provider_tasks.deliver_sms.retry.assert_called_with(queue='retry', countdown=10) + provider_tasks.deliver_sms.retry.assert_called_with(queue="retry-tasks") assert sample_notification.status == 'technical-failure' @@ -105,7 +77,7 @@ def test_should_go_into_technical_error_if_exceeds_retries_on_deliver_email_task deliver_email(sample_notification.id) - provider_tasks.deliver_email.retry.assert_called_with(queue='retry', countdown=10) + provider_tasks.deliver_email.retry.assert_called_with(queue="retry-tasks") assert sample_notification.status == 'technical-failure' diff --git a/tests/app/celery/test_scheduled_tasks.py b/tests/app/celery/test_scheduled_tasks.py index 50bf5f982..7a347e35c 100644 --- a/tests/app/celery/test_scheduled_tasks.py +++ b/tests/app/celery/test_scheduled_tasks.py @@ -5,13 +5,14 @@ from functools import partial from flask import current_app from freezegun import freeze_time -from app.celery.scheduled_tasks import s3, timeout_job_statistics +from app.celery.scheduled_tasks import s3, timeout_job_statistics, delete_sms_notifications_older_than_seven_days, \ + delete_letter_notifications_older_than_seven_days, delete_email_notifications_older_than_seven_days, \ + send_scheduled_notifications from app.celery import scheduled_tasks from app.celery.scheduled_tasks import ( delete_verify_codes, remove_csv_files, - delete_successful_notifications, - delete_failed_notifications, + delete_notifications_created_more_than_a_week_ago_by_type, delete_invitations, timeout_notifications, run_scheduled_jobs, @@ -20,6 +21,7 @@ from app.celery.scheduled_tasks import ( ) from app.clients.performance_platform.performance_platform_client import PerformancePlatformClient from app.dao.jobs_dao import dao_get_job_by_id +from app.dao.notifications_dao import dao_get_scheduled_notifications from app.dao.provider_details_dao import ( dao_update_provider_details, get_current_provider @@ -30,8 +32,7 @@ from tests.app.db import create_notification, create_service from tests.app.conftest import ( sample_job as create_sample_job, sample_notification_history as create_notification_history, - create_custom_template -) + create_custom_template) from tests.conftest import set_config_values from unittest.mock import call, patch, PropertyMock @@ -70,8 +71,7 @@ def prepare_current_provider(restore_provider_details): def test_should_have_decorated_tasks_functions(): assert delete_verify_codes.__wrapped__.__name__ == 'delete_verify_codes' - assert delete_successful_notifications.__wrapped__.__name__ == 'delete_successful_notifications' - assert delete_failed_notifications.__wrapped__.__name__ == 'delete_failed_notifications' + assert delete_notifications_created_more_than_a_week_ago_by_type.__wrapped__.__name__ == 'delete_notifications_created_more_than_a_week_ago_by_type' # noqa assert timeout_notifications.__wrapped__.__name__ == 'timeout_notifications' assert delete_invitations.__wrapped__.__name__ == 'delete_invitations' assert run_scheduled_jobs.__wrapped__.__name__ == 'run_scheduled_jobs' @@ -81,16 +81,22 @@ def test_should_have_decorated_tasks_functions(): 'switch_current_sms_provider_on_slow_delivery' -def test_should_call_delete_successful_notifications_more_than_week_in_task(notify_api, mocker): - mocked = mocker.patch('app.celery.scheduled_tasks.delete_notifications_created_more_than_a_week_ago') - delete_successful_notifications() - mocked.assert_called_once_with('delivered') +def test_should_call_delete_sms_notifications_more_than_week_in_task(notify_api, mocker): + mocked = mocker.patch('app.celery.scheduled_tasks.delete_notifications_created_more_than_a_week_ago_by_type') + delete_sms_notifications_older_than_seven_days() + mocked.assert_called_once_with('sms') -def test_should_call_delete_failed_notifications_more_than_week_in_task(notify_api, mocker): - mocker.patch('app.celery.scheduled_tasks.delete_notifications_created_more_than_a_week_ago') - delete_failed_notifications() - assert scheduled_tasks.delete_notifications_created_more_than_a_week_ago.call_count == 4 +def test_should_call_delete_email_notifications_more_than_week_in_task(notify_api, mocker): + mocked = mocker.patch('app.celery.scheduled_tasks.delete_notifications_created_more_than_a_week_ago_by_type') + delete_email_notifications_older_than_seven_days() + mocked.assert_called_once_with('email') + + +def test_should_call_delete_letter_notifications_more_than_week_in_task(notify_api, mocker): + mocked = mocker.patch('app.celery.scheduled_tasks.delete_notifications_created_more_than_a_week_ago_by_type') + delete_letter_notifications_older_than_seven_days() + mocked.assert_called_once_with('letter') def test_should_call_delete_codes_on_delete_verify_codes_task(notify_api, mocker): @@ -160,7 +166,7 @@ def test_should_update_scheduled_jobs_and_put_on_queue(notify_db, notify_db_sess updated_job = dao_get_job_by_id(job.id) assert updated_job.job_status == 'pending' - mocked.assert_called_with([str(job.id)], queue='process-job') + mocked.assert_called_with([str(job.id)], queue="job-tasks") def test_should_update_all_scheduled_jobs_and_put_on_queue(notify_db, notify_db_session, mocker): @@ -195,9 +201,9 @@ def test_should_update_all_scheduled_jobs_and_put_on_queue(notify_db, notify_db_ assert dao_get_job_by_id(job_2.id).job_status == 'pending' mocked.assert_has_calls([ - call([str(job_3.id)], queue='process-job'), - call([str(job_2.id)], queue='process-job'), - call([str(job_1.id)], queue='process-job') + call([str(job_3.id)], queue="job-tasks"), + call([str(job_2.id)], queue="job-tasks"), + call([str(job_1.id)], queue="job-tasks") ]) @@ -411,6 +417,24 @@ def test_switch_providers_on_slow_delivery_does_not_switch_based_on_older_notifi assert starting_provider.identifier == current_provider.identifier +@freeze_time("2017-05-01 14:00:00") +def test_should_send_all_scheduled_notifications_to_deliver_queue(sample_template, mocker): + mocked = mocker.patch('app.celery.provider_tasks.deliver_sms') + message_to_deliver = create_notification(template=sample_template, scheduled_for="2017-05-01 13:15") + create_notification(template=sample_template, scheduled_for="2017-05-01 10:15", status='delivered') + create_notification(template=sample_template) + create_notification(template=sample_template, scheduled_for="2017-05-01 14:15") + + scheduled_notifications = dao_get_scheduled_notifications() + assert len(scheduled_notifications) == 1 + + send_scheduled_notifications() + + mocked.apply_async.assert_called_once_with([str(message_to_deliver.id)], queue='send-tasks') + scheduled_notifications = dao_get_scheduled_notifications() + assert not scheduled_notifications + + def test_timeout_job_statistics_called_with_notification_timeout(notify_api, mocker): notify_api.config['SENDING_NOTIFICATIONS_TIMEOUT_PERIOD'] = 999 dao_mock = mocker.patch('app.celery.scheduled_tasks.dao_timeout_job_statistics') diff --git a/tests/app/celery/test_statistics_tasks.py b/tests/app/celery/test_statistics_tasks.py index 24aaba97d..40d20117d 100644 --- a/tests/app/celery/test_statistics_tasks.py +++ b/tests/app/celery/test_statistics_tasks.py @@ -17,7 +17,7 @@ def test_should_create_initial_job_task_if_notification_is_related_to_a_job( mock = mocker.patch("app.celery.statistics_tasks.record_initial_job_statistics.apply_async") notification = sample_notification(notify_db, notify_db_session, job=sample_job) create_initial_notification_statistic_tasks(notification) - mock.assert_called_once_with((str(notification.id), ), queue="statistics") + mock.assert_called_once_with((str(notification.id), ), queue="statistics-tasks") @pytest.mark.parametrize('status', [ @@ -29,7 +29,7 @@ def test_should_create_intial_job_task_if_notification_is_not_in_completed_state mock = mocker.patch("app.celery.statistics_tasks.record_initial_job_statistics.apply_async") notification = sample_notification(notify_db, notify_db_session, job=sample_job, status=status) create_initial_notification_statistic_tasks(notification) - mock.assert_called_once_with((str(notification.id), ), queue="statistics") + mock.assert_called_once_with((str(notification.id), ), queue="statistics-tasks") def test_should_not_create_initial_job_task_if_notification_is_not_related_to_a_job( @@ -47,7 +47,7 @@ def test_should_create_outcome_job_task_if_notification_is_related_to_a_job( mock = mocker.patch("app.celery.statistics_tasks.record_outcome_job_statistics.apply_async") notification = sample_notification(notify_db, notify_db_session, job=sample_job, status=NOTIFICATION_DELIVERED) create_outcome_notification_statistic_tasks(notification) - mock.assert_called_once_with((str(notification.id), ), queue="statistics") + mock.assert_called_once_with((str(notification.id), ), queue="statistics-tasks") @pytest.mark.parametrize('status', NOTIFICATION_STATUS_TYPES_COMPLETED) @@ -57,7 +57,7 @@ def test_should_create_outcome_job_task_if_notification_is_in_completed_state( mock = mocker.patch("app.celery.statistics_tasks.record_outcome_job_statistics.apply_async") notification = sample_notification(notify_db, notify_db_session, job=sample_job, status=status) create_outcome_notification_statistic_tasks(notification) - mock.assert_called_once_with((str(notification.id), ), queue='statistics') + mock.assert_called_once_with((str(notification.id), ), queue="statistics-tasks") @pytest.mark.parametrize('status', [ @@ -100,7 +100,7 @@ def test_should_retry_if_persisting_the_job_stats_has_a_sql_alchemy_exception( record_initial_job_statistics(str(sample_notification.id)) dao_mock.assert_called_once_with(sample_notification) - retry_mock.assert_called_with(queue="retry") + retry_mock.assert_called_with(queue="retry-tasks") def test_should_call_update_job_stats_dao_outcome_methods(notify_db, notify_db_session, sample_notification, mocker): @@ -123,7 +123,7 @@ def test_should_retry_if_persisting_the_job_outcome_stats_has_a_sql_alchemy_exce record_outcome_job_statistics(str(sample_notification.id)) dao_mock.assert_called_once_with(sample_notification) - retry_mock.assert_called_with(queue="retry") + retry_mock.assert_called_with(queue="retry-tasks") def test_should_retry_if_persisting_the_job_outcome_stats_updates_zero_rows( @@ -136,7 +136,7 @@ def test_should_retry_if_persisting_the_job_outcome_stats_updates_zero_rows( record_outcome_job_statistics(str(sample_notification.id)) dao_mock.assert_called_once_with(sample_notification) - retry_mock.assert_called_with(queue="retry") + retry_mock.assert_called_with(queue="retry-tasks") def test_should_retry_if_persisting_the_job_stats_creation_cant_find_notification_by_id( @@ -148,7 +148,7 @@ def test_should_retry_if_persisting_the_job_stats_creation_cant_find_notificatio record_initial_job_statistics(str(create_uuid())) dao_mock.assert_not_called() - retry_mock.assert_called_with(queue="retry") + retry_mock.assert_called_with(queue="retry-tasks") def test_should_retry_if_persisting_the_job_stats_outcome_cant_find_notification_by_id( @@ -161,4 +161,4 @@ def test_should_retry_if_persisting_the_job_stats_outcome_cant_find_notification record_outcome_job_statistics(str(create_uuid())) dao_mock.assert_not_called() - retry_mock.assert_called_with(queue="retry") + retry_mock.assert_called_with(queue="retry-tasks") diff --git a/tests/app/celery/test_tasks.py b/tests/app/celery/test_tasks.py index 27c9833a4..8cb5a13d4 100644 --- a/tests/app/celery/test_tasks.py +++ b/tests/app/celery/test_tasks.py @@ -108,7 +108,7 @@ def test_should_process_sms_job(sample_job, mocker): "uuid", "something_encrypted", "2016-01-01T11:09:00.061258Z"), - queue="db-sms" + queue="database-tasks" ) job = jobs_dao.dao_get_job_by_id(sample_job.id) assert job.job_status == 'finished' @@ -237,7 +237,7 @@ def test_should_process_email_job_if_exactly_on_send_limits(notify_db, "something_encrypted", "2016-01-01T11:09:00.061258Z" ), - queue="db-email" + queue="database-tasks" ) @@ -283,7 +283,7 @@ def test_should_process_email_job(email_job_with_placeholders, mocker): "something_encrypted", "2016-01-01T11:09:00.061258Z" ), - queue="db-email" + queue="database-tasks" ) job = jobs_dao.dao_get_job_by_id(email_job_with_placeholders.id) assert job.job_status == 'finished' @@ -324,7 +324,7 @@ def test_should_process_letter_job(sample_letter_job, mocker): assert process_row_mock.call_count == 1 assert sample_letter_job.job_status == 'in progress' - tasks.build_dvla_file.apply_async.assert_called_once_with([str(sample_letter_job.id)], queue="process-job") + tasks.build_dvla_file.apply_async.assert_called_once_with([str(sample_letter_job.id)], queue="job-tasks") def test_should_process_all_sms_job(sample_job_with_placeholdered_template, @@ -355,12 +355,12 @@ def test_should_process_all_sms_job(sample_job_with_placeholdered_template, @freeze_time('2001-01-01T12:00:00') @pytest.mark.parametrize('template_type, research_mode, expected_function, expected_queue', [ - (SMS_TYPE, False, 'send_sms', 'db-sms'), - (SMS_TYPE, True, 'send_sms', 'research-mode'), - (EMAIL_TYPE, False, 'send_email', 'db-email'), - (EMAIL_TYPE, True, 'send_email', 'research-mode'), - (LETTER_TYPE, False, 'persist_letter', 'db-letter'), - (LETTER_TYPE, True, 'persist_letter', 'research-mode'), + (SMS_TYPE, False, 'send_sms', 'database-tasks'), + (SMS_TYPE, True, 'send_sms', 'research-mode-tasks'), + (EMAIL_TYPE, False, 'send_email', 'database-tasks'), + (EMAIL_TYPE, True, 'send_email', 'research-mode-tasks'), + (LETTER_TYPE, False, 'persist_letter', 'database-tasks'), + (LETTER_TYPE, True, 'persist_letter', 'research-mode-tasks'), ]) def test_process_row_sends_letter_task(template_type, research_mode, expected_function, expected_queue, mocker): mocker.patch('app.celery.tasks.create_uuid', return_value='noti_uuid') @@ -420,7 +420,7 @@ def test_should_send_template_to_correct_sms_task_and_persist(sample_template_wi assert persisted_notification.notification_type == 'sms' mocked_deliver_sms.assert_called_once_with( [str(persisted_notification.id)], - queue="send-sms" + queue="send-tasks" ) @@ -446,7 +446,7 @@ def test_should_put_send_sms_task_in_research_mode_queue_if_research_mode_servic persisted_notification = Notification.query.one() provider_tasks.deliver_sms.apply_async.assert_called_once_with( [str(persisted_notification.id)], - queue="research-mode" + queue="research-mode-tasks" ) assert mocked_deliver_sms.called @@ -481,7 +481,7 @@ def test_should_send_sms_if_restricted_service_and_valid_number(notify_db, notif assert persisted_notification.notification_type == 'sms' provider_tasks.deliver_sms.apply_async.assert_called_once_with( [str(persisted_notification.id)], - queue="send-sms" + queue="send-tasks" ) @@ -507,7 +507,7 @@ def test_should_send_sms_if_restricted_service_and_non_team_number_with_test_key persisted_notification = Notification.query.one() mocked_deliver_sms.assert_called_once_with( [str(persisted_notification.id)], - queue="send-sms" + queue="send-tasks" ) @@ -535,7 +535,7 @@ def test_should_send_email_if_restricted_service_and_non_team_email_address_with persisted_notification = Notification.query.one() mocked_deliver_email.assert_called_once_with( [str(persisted_notification.id)], - queue="send-email" + queue="send-tasks" ) @@ -602,7 +602,7 @@ def test_should_put_send_email_task_in_research_mode_queue_if_research_mode_serv persisted_notification = Notification.query.one() provider_tasks.deliver_email.apply_async.assert_called_once_with( [str(persisted_notification.id)], - queue="research-mode" + queue="research-mode-tasks" ) @@ -639,7 +639,7 @@ def test_should_send_sms_template_to_and_persist_with_job_id(sample_job, sample_ provider_tasks.deliver_sms.apply_async.assert_called_once_with( [str(persisted_notification.id)], - queue="send-sms" + queue="send-tasks" ) @@ -736,7 +736,7 @@ def test_should_use_email_template_and_persist(sample_email_template_with_placeh assert persisted_notification.notification_type == 'email' provider_tasks.deliver_email.apply_async.assert_called_once_with( - [str(persisted_notification.id)], queue='send-email') + [str(persisted_notification.id)], queue='send-tasks') def test_send_email_should_use_template_version_from_job_not_latest(sample_email_template, mocker): @@ -767,7 +767,7 @@ def test_send_email_should_use_template_version_from_job_not_latest(sample_email assert not persisted_notification.sent_by assert persisted_notification.notification_type == 'email' provider_tasks.deliver_email.apply_async.assert_called_once_with([str(persisted_notification.id)], - queue='send-email') + queue='send-tasks') def test_should_use_email_template_subject_placeholders(sample_email_template_with_placeholders, mocker): @@ -793,7 +793,7 @@ def test_should_use_email_template_subject_placeholders(sample_email_template_wi assert not persisted_notification.reference assert persisted_notification.notification_type == 'email' provider_tasks.deliver_email.apply_async.assert_called_once_with( - [str(persisted_notification.id)], queue='send-email' + [str(persisted_notification.id)], queue='send-tasks' ) @@ -821,7 +821,7 @@ def test_should_use_email_template_and_persist_without_personalisation(sample_em assert not persisted_notification.reference assert persisted_notification.notification_type == 'email' provider_tasks.deliver_email.apply_async.assert_called_once_with([str(persisted_notification.id)], - queue='send-email') + queue='send-tasks') def test_send_sms_should_go_to_retry_queue_if_database_errors(sample_template, mocker): @@ -844,7 +844,7 @@ def test_send_sms_should_go_to_retry_queue_if_database_errors(sample_template, m now.strftime(DATETIME_FORMAT) ) assert provider_tasks.deliver_sms.apply_async.called is False - tasks.send_sms.retry.assert_called_with(exc=expected_exception, queue='retry') + tasks.send_sms.retry.assert_called_with(exc=expected_exception, queue="retry-tasks") assert Notification.query.count() == 0 @@ -869,7 +869,7 @@ def test_send_email_should_go_to_retry_queue_if_database_errors(sample_email_tem now.strftime(DATETIME_FORMAT) ) assert not provider_tasks.deliver_email.apply_async.called - tasks.send_email.retry.assert_called_with(exc=expected_exception, queue='retry') + tasks.send_email.retry.assert_called_with(exc=expected_exception, queue="retry-tasks") assert Notification.query.count() == 0 @@ -1002,7 +1002,6 @@ def test_build_dvla_file(sample_letter_template, mocker): file_location="{}-dvla-job.text".format(job.id) ) assert Job.query.get(job.id).job_status == 'ready to send' - mocked_send_task.assert_called_once_with("aggregrate-dvla-files", ([str(job.id)], ), queue='aggregate-dvla-files') def test_build_dvla_file_retries_if_all_notifications_are_not_created(sample_letter_template, mocker): @@ -1016,7 +1015,7 @@ def test_build_dvla_file_retries_if_all_notifications_are_not_created(sample_let build_dvla_file(job.id) mocked.assert_not_called() - tasks.build_dvla_file.retry.assert_called_with(queue='retry', + tasks.build_dvla_file.retry.assert_called_with(queue="retry-tasks", exc="All notifications for job {} are not persisted".format(job.id)) assert Job.query.get(job.id).job_status == 'in progress' mocked_send_task.assert_not_called() diff --git a/tests/app/conftest.py b/tests/app/conftest.py index c10c053cb..5b1d32144 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -24,7 +24,7 @@ from app.models import ( NotificationStatistics, ServiceWhitelist, KEY_TYPE_NORMAL, KEY_TYPE_TEST, KEY_TYPE_TEAM, - MOBILE_TYPE, EMAIL_TYPE, LETTER_TYPE, NOTIFICATION_STATUS_TYPES_COMPLETED) + MOBILE_TYPE, EMAIL_TYPE, SMS_TYPE, LETTER_TYPE, NOTIFICATION_STATUS_TYPES_COMPLETED, ScheduledNotification) from app.dao.users_dao import (create_user_code, create_secret_code) from app.dao.services_dao import (dao_create_service, dao_add_user_to_service) from app.dao.templates_dao import dao_create_template @@ -124,7 +124,8 @@ def sample_service( restricted=False, limit=1000, email_from=None, - can_send_international_sms=False + can_send_international_sms=False, + permissions=[SMS_TYPE, EMAIL_TYPE] ): if user is None: user = create_user() @@ -142,7 +143,7 @@ def sample_service( service = Service.query.filter_by(name=service_name).first() if not service: service = Service(**data) - dao_create_service(service, user) + dao_create_service(service, user, service_permissions=permissions) else: if user not in service.users: dao_add_user_to_service(service, user) @@ -445,7 +446,9 @@ def sample_notification( key_type=KEY_TYPE_NORMAL, sent_by=None, client_reference=None, - rate_multiplier=1.0 + rate_multiplier=1.0, + scheduled_for=None, + normalised_to=None ): if created_at is None: created_at = datetime.utcnow() @@ -483,12 +486,23 @@ def sample_notification( 'sent_by': sent_by, 'updated_at': created_at if status in NOTIFICATION_STATUS_TYPES_COMPLETED else None, 'client_reference': client_reference, - 'rate_multiplier': rate_multiplier + 'rate_multiplier': rate_multiplier, + 'normalised_to': normalised_to } if job_row_number is not None: data['job_row_number'] = job_row_number notification = Notification(**data) dao_create_notification(notification) + if scheduled_for: + scheduled_notification = ScheduledNotification(id=uuid.uuid4(), + notification_id=notification.id, + scheduled_for=datetime.strptime(scheduled_for, + "%Y-%m-%d %H:%M")) + if status != 'created': + scheduled_notification.pending = False + db.session.add(scheduled_notification) + db.session.commit() + return notification diff --git a/tests/app/dao/test_notification_dao.py b/tests/app/dao/test_notification_dao.py index 95dc3e09d..dc0f3fc34 100644 --- a/tests/app/dao/test_notification_dao.py +++ b/tests/app/dao/test_notification_dao.py @@ -12,12 +12,15 @@ from app.models import ( Job, NotificationStatistics, TemplateStatistics, + ScheduledNotification, NOTIFICATION_STATUS_TYPES, NOTIFICATION_STATUS_TYPES_FAILED, NOTIFICATION_SENT, + NOTIFICATION_DELIVERED, KEY_TYPE_NORMAL, KEY_TYPE_TEAM, - KEY_TYPE_TEST) + KEY_TYPE_TEST +) from app.dao.notifications_dao import ( dao_create_notification, @@ -26,7 +29,7 @@ from app.dao.notifications_dao import ( dao_get_potential_notification_statistics_for_day, dao_get_template_usage, dao_update_notification, - delete_notifications_created_more_than_a_week_ago, + delete_notifications_created_more_than_a_week_ago_by_type, get_notification_by_id, get_notification_for_job, get_notification_billable_unit_count_per_month, @@ -39,7 +42,9 @@ from app.dao.notifications_dao import ( dao_delete_notifications_and_history_by_id, dao_timeout_notifications, is_delivery_slow_for_provider, - dao_update_notifications_sent_to_dvla, dao_get_notifications_by_to_field) + dao_update_notifications_sent_to_dvla, + dao_get_notifications_by_to_field, + dao_created_scheduled_notification, dao_get_scheduled_notifications, set_scheduled_notification_to_processed) from app.dao.services_dao import dao_update_service from tests.app.db import create_notification @@ -49,8 +54,8 @@ from tests.app.conftest import ( sample_email_template, sample_service, sample_job, - sample_notification_history as create_notification_history -) + sample_notification_history as create_notification_history, + sample_letter_template) def test_should_have_decorated_notifications_dao_functions(): @@ -66,7 +71,7 @@ def test_should_have_decorated_notifications_dao_functions(): assert get_notification_with_personalisation.__wrapped__.__name__ == 'get_notification_with_personalisation' # noqa assert get_notifications_for_service.__wrapped__.__name__ == 'get_notifications_for_service' # noqa assert get_notification_by_id.__wrapped__.__name__ == 'get_notification_by_id' # noqa - assert delete_notifications_created_more_than_a_week_ago.__wrapped__.__name__ == 'delete_notifications_created_more_than_a_week_ago' # noqa + assert delete_notifications_created_more_than_a_week_ago_by_type.__wrapped__.__name__ == 'delete_notifications_created_more_than_a_week_ago_by_type' # noqa assert dao_delete_notifications_and_history_by_id.__wrapped__.__name__ == 'dao_delete_notifications_and_history_by_id' # noqa @@ -353,28 +358,45 @@ def test_should_update_status_by_id_if_created(notify_db, notify_db_session): assert updated.status == 'failed' -def test_should_not_update_status_by_reference_if_in_sent_status(notify_db, notify_db_session): - notification = sample_notification( - notify_db, - notify_db_session, +def test_should_not_update_status_by_reference_if_from_country_with_no_delivery_receipts(sample_template): + notification = create_notification( + sample_template, status=NOTIFICATION_SENT, reference='foo' ) - update_notification_status_by_reference('foo', 'failed') - assert Notification.query.get(notification.id).status == NOTIFICATION_SENT + res = update_notification_status_by_reference('foo', 'failed') + + assert res is None + assert notification.status == NOTIFICATION_SENT -def test_should_not_update_status_by_id_if_in_sent_status(notify_db, notify_db_session): - notification = sample_notification( - notify_db, - notify_db_session, - status=NOTIFICATION_SENT +def test_should_not_update_status_by_id_if_sent_to_country_with_no_delivery_receipts(sample_template): + notification = create_notification( + sample_template, + status=NOTIFICATION_SENT, + international=True, + phone_prefix='1' # americans only have carrier delivery receipts ) - update_notification_status_by_id(notification.id, 'failed') + res = update_notification_status_by_id(notification.id, 'delivered') - assert Notification.query.get(notification.id).status == NOTIFICATION_SENT + assert res is None + assert notification.status == NOTIFICATION_SENT + + +def test_should_not_update_status_by_id_if_sent_to_country_with_no_delivery_receipts(sample_template): + notification = create_notification( + sample_template, + status=NOTIFICATION_SENT, + international=True, + phone_prefix='7' # russians have full delivery receipts + ) + + res = update_notification_status_by_id(notification.id, 'delivered') + + assert res == notification + assert notification.status == NOTIFICATION_DELIVERED def test_should_not_update_status_by_reference_if_not_sending(notify_db, notify_db_session): @@ -713,13 +735,18 @@ def test_save_notification_with_no_job(sample_template, mmg_provider): assert notification_from_db.status == 'created' -def test_get_notification_by_id(sample_notification): +def test_get_notification_by_id(notify_db, notify_db_session, sample_template): + notification = sample_notification(notify_db=notify_db, notify_db_session=notify_db_session, + template=sample_template, + scheduled_for='2017-05-05 14:15', + status='created') notification_from_db = get_notification_with_personalisation( - sample_notification.service.id, - sample_notification.id, + sample_template.service.id, + notification.id, key_type=None ) - assert sample_notification == notification_from_db + assert notification == notification_from_db + assert notification_from_db.scheduled_notification.scheduled_for == datetime(2017, 5, 5, 14, 15) def test_get_notifications_by_reference(notify_db, notify_db_session, sample_service): @@ -868,63 +895,118 @@ def test_updating_notification_with_no_notification_status_updates_notification_ assert hist_from_db._status_fkey == 'failed' +@pytest.mark.parametrize('notification_type, expected_sms_count, expected_email_count, expected_letter_count', [ + ('sms', 8, 10, 10), ('email', 10, 8, 10), ('letter', 10, 10, 8) +]) @freeze_time("2016-01-10 12:00:00.000000") -def test_should_delete_notifications_after_seven_days(notify_db, notify_db_session): +def test_should_delete_notifications_by_type_after_seven_days( + notify_db, + notify_db_session, + sample_service, + notification_type, + expected_sms_count, + expected_email_count, + expected_letter_count +): assert len(Notification.query.all()) == 0 - # create one notification a day between 1st and 10th from 11:00 to 19:00 + email_template = sample_email_template(notify_db, notify_db_session, service=sample_service) + sms_template = sample_template(notify_db, notify_db_session, service=sample_service) + letter_template = sample_letter_template(sample_service) + + # create one notification a day between 1st and 10th from 11:00 to 19:00 of each type for i in range(1, 11): past_date = '2016-01-{0:02d} {0:02d}:00:00.000000'.format(i) with freeze_time(past_date): - sample_notification(notify_db, notify_db_session, created_at=datetime.utcnow(), status="failed") + sample_notification( + notify_db, + notify_db_session, + created_at=datetime.utcnow(), + status="failed", + service=sample_service, + template=email_template + ) + sample_notification( + notify_db, + notify_db_session, + created_at=datetime.utcnow(), + status="failed", + service=sample_service, + template=sms_template + ) + sample_notification( + notify_db, + notify_db_session, + created_at=datetime.utcnow(), + status="failed", + service=sample_service, + template=letter_template + ) all_notifications = Notification.query.all() - assert len(all_notifications) == 10 + assert len(all_notifications) == 30 # Records from before 3rd should be deleted - delete_notifications_created_more_than_a_week_ago('failed') - remaining_notifications = Notification.query.all() - assert len(remaining_notifications) == 8 - for notification in remaining_notifications: + delete_notifications_created_more_than_a_week_ago_by_type(notification_type) + remaining_sms_notifications = Notification.query.filter_by(notification_type='sms').all() + remaining_letter_notifications = Notification.query.filter_by(notification_type='letter').all() + remaining_email_notifications = Notification.query.filter_by(notification_type='email').all() + + assert len(remaining_sms_notifications) == expected_sms_count + assert len(remaining_email_notifications) == expected_email_count + assert len(remaining_letter_notifications) == expected_letter_count + + if notification_type == 'sms': + notifications_to_check = remaining_sms_notifications + if notification_type == 'email': + notifications_to_check = remaining_email_notifications + if notification_type == 'letter': + notifications_to_check = remaining_letter_notifications + + for notification in notifications_to_check: assert notification.created_at.date() >= date(2016, 1, 3) +@pytest.mark.parametrize('notification_type', ['sms', 'email', 'letter']) @freeze_time("2016-01-10 12:00:00.000000") -def test_should_not_delete_notification_history(notify_db, notify_db_session): +def test_should_not_delete_notification_history(notify_db, notify_db_session, sample_service, notification_type): with freeze_time('2016-01-01 12:00'): - notification = sample_notification(notify_db, notify_db_session, created_at=datetime.utcnow(), status="failed") - notification_id = notification.id + email_template = sample_email_template(notify_db, notify_db_session, service=sample_service) + sms_template = sample_template(notify_db, notify_db_session, service=sample_service) + letter_template = sample_letter_template(sample_service) - assert Notification.query.count() == 1 - assert NotificationHistory.query.count() == 1 + sample_notification( + notify_db, + notify_db_session, + created_at=datetime.utcnow(), + status="failed", + service=sample_service, + template=email_template + ) + sample_notification( + notify_db, + notify_db_session, + created_at=datetime.utcnow(), + status="failed", + service=sample_service, + template=sms_template + ) + sample_notification( + notify_db, + notify_db_session, + created_at=datetime.utcnow(), + status="failed", + service=sample_service, + template=letter_template + ) - delete_notifications_created_more_than_a_week_ago('failed') + assert Notification.query.count() == 3 + assert NotificationHistory.query.count() == 3 - assert Notification.query.count() == 0 - assert NotificationHistory.query.count() == 1 - assert NotificationHistory.query.one().id == notification_id + delete_notifications_created_more_than_a_week_ago_by_type(notification_type) - -def test_should_not_delete_failed_notifications_before_seven_days(notify_db, notify_db_session): - should_delete = datetime.utcnow() - timedelta(days=8) - do_not_delete = datetime.utcnow() - timedelta(days=7) - sample_notification(notify_db, notify_db_session, created_at=should_delete, status="failed", - to_field="should_delete") - sample_notification(notify_db, notify_db_session, created_at=do_not_delete, status="failed", - to_field="do_not_delete") - assert len(Notification.query.all()) == 2 - delete_notifications_created_more_than_a_week_ago('failed') - assert len(Notification.query.all()) == 1 - assert Notification.query.first().to == 'do_not_delete' - - -def test_should_delete_letter_notifications(sample_letter_template): - should_delete = datetime.utcnow() - timedelta(days=8) - - create_notification(sample_letter_template, created_at=should_delete) - - delete_notifications_created_more_than_a_week_ago('created') - assert len(Notification.query.all()) == 0 + assert Notification.query.count() == 2 + assert NotificationHistory.query.count() == 3 @freeze_time("2016-01-10") @@ -1664,30 +1746,160 @@ def test_dao_update_notifications_sent_to_dvla_does_update_history_if_test_key( def test_dao_get_notifications_by_to_field(sample_template): - notification1 = create_notification(template=sample_template, to_field='+447700900855') - notification2 = create_notification(template=sample_template, to_field='jack@gmail.com') - notification3 = create_notification(template=sample_template, to_field='jane@gmail.com') + notification1 = create_notification( + template=sample_template, to_field='+447700900855', normalised_to='447700900855' + ) + create_notification( + template=sample_template, to_field='jack@gmail.com', normalised_to='jack@gmail.com' + ) + create_notification( + template=sample_template, to_field='jane@gmail.com', normalised_to='jane@gmail.com' + ) results = dao_get_notifications_by_to_field(notification1.service_id, "+447700900855") + assert len(results) == 1 - assert results[0].id == notification1.id + assert notification1.id == results[0].id def test_dao_get_notifications_by_to_field_search_is_not_case_sensitive(sample_template): - notification1 = create_notification(template=sample_template, to_field='+447700900855') - notification2 = create_notification(template=sample_template, to_field='jack@gmail.com') - notification3 = create_notification(template=sample_template, to_field='jane@gmail.com') - results = dao_get_notifications_by_to_field(notification1.service_id, 'JACK@gmail.com') + notification = create_notification( + template=sample_template, to_field='jack@gmail.com', normalised_to='jack@gmail.com' + ) + results = dao_get_notifications_by_to_field(notification.service_id, 'JACK@gmail.com') + notification_ids = [notification.id for notification in results] + assert len(results) == 1 - assert results[0].id == notification2.id + assert notification.id in notification_ids + + +@pytest.mark.parametrize('to', [ + 'not@email', '123' +]) +def test_dao_get_notifications_by_to_field_accepts_invalid_phone_numbers_and_email_addresses( + sample_template, + to, +): + notification = create_notification( + template=sample_template, to_field='test@example.com', normalised_to='test@example.com' + ) + results = dao_get_notifications_by_to_field(notification.service_id, to) + assert len(results) == 0 def test_dao_get_notifications_by_to_field_search_ignores_spaces(sample_template): - notification1 = create_notification(template=sample_template, to_field='+447700900855') - notification2 = create_notification(template=sample_template, to_field='+44 77 00900 855') - notification3 = create_notification(template=sample_template, to_field=' +4477009 00 855 ') - notification4 = create_notification(template=sample_template, to_field='jack@gmail.com') + notification1 = create_notification( + template=sample_template, to_field='+447700900855', normalised_to='447700900855' + ) + notification2 = create_notification( + template=sample_template, to_field='+44 77 00900 855', normalised_to='447700900855' + ) + notification3 = create_notification( + template=sample_template, to_field=' +4477009 00 855 ', normalised_to='447700900855' + ) + create_notification( + template=sample_template, to_field='jaCK@gmail.com', normalised_to='jack@gmail.com' + ) + results = dao_get_notifications_by_to_field(notification1.service_id, '+447700900855') + notification_ids = [notification.id for notification in results] + assert len(results) == 3 - assert notification1.id in [r.id for r in results] - assert notification2.id in [r.id for r in results] - assert notification3.id in [r.id for r in results] + assert notification1.id in notification_ids + assert notification2.id in notification_ids + assert notification3.id in notification_ids + + +def test_dao_created_scheduled_notification(sample_notification): + + scheduled_notification = ScheduledNotification(notification_id=sample_notification.id, + scheduled_for=datetime.strptime("2017-01-05 14:15", + "%Y-%m-%d %H:%M")) + dao_created_scheduled_notification(scheduled_notification) + saved_notification = ScheduledNotification.query.all() + assert len(saved_notification) == 1 + assert saved_notification[0].notification_id == sample_notification.id + assert saved_notification[0].scheduled_for == datetime(2017, 1, 5, 14, 15) + + +def test_dao_get_scheduled_notifications(notify_db, notify_db_session, sample_template): + notification_1 = sample_notification(notify_db=notify_db, notify_db_session=notify_db_session, + template=sample_template, scheduled_for='2017-05-05 14:15', + status='created') + sample_notification(notify_db=notify_db, notify_db_session=notify_db_session, + template=sample_template, scheduled_for='2017-05-04 14:15', status='delivered') + sample_notification(notify_db=notify_db, notify_db_session=notify_db_session, + template=sample_template, status='created') + scheduled_notifications = dao_get_scheduled_notifications() + assert len(scheduled_notifications) == 1 + assert scheduled_notifications[0].id == notification_1.id + assert scheduled_notifications[0].scheduled_notification.pending + + +def test_set_scheduled_notification_to_processed(notify_db, notify_db_session, sample_template): + notification_1 = sample_notification(notify_db=notify_db, notify_db_session=notify_db_session, + template=sample_template, scheduled_for='2017-05-05 14:15', + status='created') + scheduled_notifications = dao_get_scheduled_notifications() + assert len(scheduled_notifications) == 1 + assert scheduled_notifications[0].id == notification_1.id + assert scheduled_notifications[0].scheduled_notification.pending + + set_scheduled_notification_to_processed(notification_1.id) + scheduled_notifications = dao_get_scheduled_notifications() + assert not scheduled_notifications + + +def test_dao_get_notifications_by_to_field_filters_status(sample_template): + notification = create_notification( + template=sample_template, to_field='+447700900855', + normalised_to='447700900855', status='delivered' + ) + create_notification( + template=sample_template, to_field='+447700900855', + normalised_to='447700900855', status='temporary-failure' + ) + + notifications = dao_get_notifications_by_to_field(notification.service_id, "+447700900855", statuses=['delivered']) + + assert len(notifications) == 1 + assert notification.id == notifications[0].id + + +def test_dao_get_notifications_by_to_field_filters_multiple_statuses(sample_template): + notification1 = create_notification( + template=sample_template, to_field='+447700900855', + normalised_to='447700900855', status='delivered' + ) + notification2 = create_notification( + template=sample_template, to_field='+447700900855', + normalised_to='447700900855', status='sending' + ) + + notifications = dao_get_notifications_by_to_field( + notification1.service_id, "+447700900855", statuses=['delivered', 'sending'] + ) + notification_ids = [notification.id for notification in notifications] + + assert len(notifications) == 2 + assert notification1.id in notification_ids + assert notification2.id in notification_ids + + +def test_dao_get_notifications_by_to_field_returns_all_if_no_status_filter(sample_template): + notification1 = create_notification( + template=sample_template, to_field='+447700900855', + normalised_to='447700900855', status='delivered' + ) + notification2 = create_notification( + template=sample_template, to_field='+447700900855', + normalised_to='447700900855', status='temporary-failure' + ) + + notifications = dao_get_notifications_by_to_field( + notification1.service_id, "+447700900855" + ) + notification_ids = [notification.id for notification in notifications] + + assert len(notifications) == 2 + assert notification1.id in notification_ids + assert notification2.id in notification_ids diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index ff7b3b54b..b7b3176dd 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -258,15 +258,15 @@ def test_create_service_returns_service_with_default_permissions(service_factory service = dao_fetch_service_by_id(service.id) assert len(service.permissions) == 2 - assert all(p.permission in [SMS_TYPE, EMAIL_TYPE] for p in service.permissions) + assert set([SMS_TYPE, EMAIL_TYPE]) == set(p.permission for p in service.permissions) # This test is only for backward compatibility and will be removed -# when the 'can_use' columns are dropped from the Service data model +# when the deprecated 'can_use' columns are not used in the Service data model @pytest.mark.parametrize("permission_to_add, can_send_letters, can_send_international_sms", [(LETTER_TYPE, True, False), (INTERNATIONAL_SMS_TYPE, False, True)]) -def test_create_service_by_id_adding_service_permission_returns_service_with_permissions_set( +def test_create_service_by_id_adding_service_permission_returns_service_with_flags_and_permissions_set( service_factory, permission_to_add, can_send_letters, can_send_international_sms): service = service_factory.get('testing', email_from='testing') @@ -275,18 +275,59 @@ def test_create_service_by_id_adding_service_permission_returns_service_with_per service = dao_fetch_service_by_id(service.id) assert len(service.permissions) == 3 - assert all(p.permission in [SMS_TYPE, EMAIL_TYPE, permission_to_add] for p in service.permissions) + assert set([SMS_TYPE, EMAIL_TYPE, permission_to_add]) == set(p.permission for p in service.permissions) assert service.can_send_letters == can_send_letters assert service.can_send_international_sms == can_send_international_sms -def test_remove_permission_from_service_by_id_returns_service_with_correct_permissions(service_factory): +# This test is only for backward compatibility and will be removed +# when the deprecated 'can_use' columns are not used in the Service data model +@pytest.mark.parametrize("permission_to_remove, can_send_letters, can_send_international_sms", + [(LETTER_TYPE, False, True), + (INTERNATIONAL_SMS_TYPE, True, False)]) +def test_create_service_by_id_removing_service_permission_returns_service_with_flags_and_permissions_set( + service_factory, permission_to_remove, can_send_letters, can_send_international_sms): service = service_factory.get('testing', email_from='testing') - dao_remove_service_permission(service_id=service.id, permission=SMS_TYPE) + + dao_add_service_permission(service_id=service.id, permission=LETTER_TYPE) + dao_add_service_permission(service_id=service.id, permission=INTERNATIONAL_SMS_TYPE) + service = dao_fetch_service_by_id(service.id) + service.set_permissions() + assert len(service.permissions) == 4 + assert service.can_send_letters + assert service.can_send_international_sms + + dao_remove_service_permission(service_id=service.id, permission=permission_to_remove) + service.set_permissions() service = dao_fetch_service_by_id(service.id) + expected_permissions = [SMS_TYPE, EMAIL_TYPE, LETTER_TYPE, INTERNATIONAL_SMS_TYPE] + expected_permissions.remove(permission_to_remove) + + assert len(service.permissions) == 3 + assert set(expected_permissions) == set(p.permission for p in service.permissions) + assert service.can_send_letters == can_send_letters + assert service.can_send_international_sms == can_send_international_sms + + +@pytest.mark.parametrize("permission_to_remove, permission_remaining", + [(SMS_TYPE, EMAIL_TYPE), + (EMAIL_TYPE, SMS_TYPE)]) +def test_remove_permission_from_service_by_id_returns_service_with_correct_permissions( + sample_service, permission_to_remove, permission_remaining): + dao_remove_service_permission(service_id=sample_service.id, permission=permission_to_remove) + + service = dao_fetch_service_by_id(sample_service.id) assert len(service.permissions) == 1 - assert service.permissions[0].permission == EMAIL_TYPE + assert service.permissions[0].permission == permission_remaining + + +def test_removing_all_permission_returns_service_with_no_permissions(sample_service): + dao_remove_service_permission(service_id=sample_service.id, permission=SMS_TYPE) + dao_remove_service_permission(service_id=sample_service.id, permission=EMAIL_TYPE) + + service = dao_fetch_service_by_id(sample_service.id) + assert len(service.permissions) == 0 def test_remove_service_does_not_remove_service_permission_types(sample_service): @@ -294,7 +335,7 @@ def test_remove_service_does_not_remove_service_permission_types(sample_service) services = dao_fetch_all_services() assert len(services) == 0 - assert set([p.name for p in ServicePermissionTypes.query.all()]) & set(SERVICE_PERMISSION_TYPES) + assert set(p.name for p in ServicePermissionTypes.query.all()) == set(SERVICE_PERMISSION_TYPES) def test_create_service_by_id_adding_and_removing_letter_returns_service_without_letter(service_factory): @@ -372,6 +413,42 @@ def test_update_service_creates_a_history_record_with_current_data(sample_user): assert Service.get_history_model().query.filter_by(name='updated_service_name').one().version == 2 +def test_update_service_permission_creates_a_history_record_with_current_data(sample_user): + assert Service.query.count() == 0 + assert Service.get_history_model().query.count() == 0 + service = Service(name="service_name", + email_from="email_from", + message_limit=1000, + restricted=False, + created_by=sample_user) + dao_create_service(service, sample_user) + + service.permissions.append(ServicePermission(service_id=service.id, permission='letter')) + dao_update_service(service) + + assert Service.query.count() == 1 + assert Service.get_history_model().query.count() == 2 + + service_from_db = Service.query.first() + + assert service_from_db.version == 2 + assert LETTER_TYPE in [p.permission for p in service_from_db.permissions] + + permission = [p for p in service.permissions if p.permission == 'sms'][0] + service.permissions.remove(permission) + dao_update_service(service) + + assert Service.query.count() == 1 + assert Service.get_history_model().query.count() == 3 + + service_from_db = Service.query.first() + assert service_from_db.version == 3 + assert SMS_TYPE not in [p.permission for p in service_from_db.permissions] + + assert len(Service.get_history_model().query.filter_by(name='service_name').all()) == 3 + assert Service.get_history_model().query.filter_by(name='service_name').all()[2].version == 3 + + def test_create_service_and_history_is_transactional(sample_user): assert Service.query.count() == 0 assert Service.get_history_model().query.count() == 0 diff --git a/tests/app/db.py b/tests/app/db.py index d418b1c3b..2f637cf1b 100644 --- a/tests/app/db.py +++ b/tests/app/db.py @@ -1,11 +1,22 @@ from datetime import datetime import uuid + from app.dao.jobs_dao import dao_create_job -from app.models import (Service, User, Template, Notification, EMAIL_TYPE, LETTER_TYPE, - SMS_TYPE, KEY_TYPE_NORMAL, Job, ServicePermission) +from app.models import ( + Service, + User, + Template, + Notification, + ScheduledNotification, + ServicePermission, + Job, + EMAIL_TYPE, + SMS_TYPE, + KEY_TYPE_NORMAL, +) from app.dao.users_dao import save_model_user -from app.dao.notifications_dao import dao_create_notification +from app.dao.notifications_dao import dao_create_notification, dao_created_scheduled_notification from app.dao.templates_dao import dao_create_template from app.dao.services_dao import dao_create_service from app.dao.service_permissions_dao import dao_add_service_permission @@ -80,7 +91,9 @@ def create_notification( client_reference=None, rate_multiplier=None, international=False, - phone_prefix=None + phone_prefix=None, + scheduled_for=None, + normalised_to=None ): if created_at is None: created_at = datetime.utcnow() @@ -114,10 +127,19 @@ def create_notification( 'job_row_number': job_row_number, 'rate_multiplier': rate_multiplier, 'international': international, - 'phone_prefix': phone_prefix + 'phone_prefix': phone_prefix, + 'normalised_to': normalised_to } notification = Notification(**data) dao_create_notification(notification) + if scheduled_for: + scheduled_notification = ScheduledNotification(id=uuid.uuid4(), + notification_id=notification.id, + scheduled_for=datetime.strptime(scheduled_for, + "%Y-%m-%d %H:%M")) + if status != 'created': + scheduled_notification.pending = False + dao_created_scheduled_notification(scheduled_notification) return notification diff --git a/tests/app/delivery/test_rest.py b/tests/app/delivery/test_rest.py index fc51fb508..984bf90e1 100644 --- a/tests/app/delivery/test_rest.py +++ b/tests/app/delivery/test_rest.py @@ -78,7 +78,7 @@ def test_should_call_deliver_sms_task_if_send_sms_to_provider_fails(notify_api, ) 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' + (str(sample_notification.id)), queue='send-tasks' ) assert response.status_code == 204 @@ -100,6 +100,6 @@ def test_should_call_deliver_email_task_if_send_email_to_provider_fails( ) 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' + (str(sample_email_notification.id)), queue='send-tasks' ) assert response.status_code == 204 diff --git a/tests/app/invite/test_invite_rest.py b/tests/app/invite/test_invite_rest.py index 182f4b05b..be1f46d82 100644 --- a/tests/app/invite/test_invite_rest.py +++ b/tests/app/invite/test_invite_rest.py @@ -33,7 +33,7 @@ def test_create_invited_user(client, sample_service, mocker, invitation_email_te assert json_resp['data']['id'] notification = Notification.query.first() - mocked.assert_called_once_with([(str(notification.id))], queue="notify") + mocked.assert_called_once_with([(str(notification.id))], queue="notify-internal-tasks") def test_create_invited_user_invalid_email(client, sample_service, mocker): diff --git a/tests/app/job/test_rest.py b/tests/app/job/test_rest.py index d583d6b31..6ebcb2e89 100644 --- a/tests/app/job/test_rest.py +++ b/tests/app/job/test_rest.py @@ -119,7 +119,7 @@ def test_create_unscheduled_job(notify_api, sample_template, mocker, fake_uuid): app.celery.tasks.process_job.apply_async.assert_called_once_with( ([str(fake_uuid)]), - queue="process-job" + queue="job-tasks" ) resp_json = json.loads(response.get_data(as_text=True)) diff --git a/tests/app/letters/test_send_letter_jobs.py b/tests/app/letters/test_send_letter_jobs.py index 7f1d88c85..1d32baf12 100644 --- a/tests/app/letters/test_send_letter_jobs.py +++ b/tests/app/letters/test_send_letter_jobs.py @@ -21,7 +21,7 @@ def test_send_letter_jobs(client, mocker): mock_celery.assert_called_once_with(name="send-files-to-dvla", args=(job_ids['job_ids'],), - queue="process-ftp") + queue="process-ftp-tasks") def test_send_letter_jobs_throws_validation_error(client, mocker): diff --git a/tests/app/notifications/rest/test_callbacks.py b/tests/app/notifications/rest/test_callbacks.py index 2c2ffd9cf..58bbc8b1f 100644 --- a/tests/app/notifications/rest/test_callbacks.py +++ b/tests/app/notifications/rest/test_callbacks.py @@ -68,7 +68,7 @@ def test_dvla_callback_calls_update_letter_notifications_task(client, mocker): assert response.status_code == 200 assert update_task.called - update_task.assert_called_with(['bar.txt'], queue='notify') + update_task.assert_called_with(['bar.txt'], queue='notify-internal-tasks') def test_dvla_callback_does_not_raise_error_parsing_json_for_plaintext_header(client, mocker): diff --git a/tests/app/notifications/rest/test_send_notification.py b/tests/app/notifications/rest/test_send_notification.py index ab527e06e..b491e3d7f 100644 --- a/tests/app/notifications/rest/test_send_notification.py +++ b/tests/app/notifications/rest/test_send_notification.py @@ -127,7 +127,7 @@ def test_send_notification_with_placeholders_replaced(notify_api, sample_email_t mocked.assert_called_once_with( [notification_id], - queue="send-email" + queue="send-tasks" ) assert response.status_code == 201 assert response_data['body'] == u'Hello Jo\nThis is an email from GOV.\u200BUK' @@ -338,7 +338,7 @@ def test_should_allow_valid_sms_notification(notify_api, sample_template, mocker response_data = json.loads(response.data)['data'] notification_id = response_data['notification']['id'] - mocked.assert_called_once_with([notification_id], queue='send-sms') + mocked.assert_called_once_with([notification_id], queue='send-tasks') assert response.status_code == 201 assert notification_id assert 'subject' not in response_data @@ -392,7 +392,7 @@ def test_should_allow_valid_email_notification(notify_api, sample_email_template notification_id = response_data['notification']['id'] app.celery.provider_tasks.deliver_email.apply_async.assert_called_once_with( [notification_id], - queue="send-email" + queue="send-tasks" ) assert response.status_code == 201 @@ -593,7 +593,7 @@ def test_should_send_email_if_team_api_key_and_a_service_user(notify_api, sample data=json.dumps(data), headers=[('Content-Type', 'application/json'), ('Authorization', 'Bearer {}'.format(auth_header))]) - app.celery.provider_tasks.deliver_email.apply_async.assert_called_once_with([fake_uuid], queue='send-email') + app.celery.provider_tasks.deliver_email.apply_async.assert_called_once_with([fake_uuid], queue='send-tasks') assert response.status_code == 201 @@ -626,7 +626,9 @@ def test_should_send_sms_to_anyone_with_test_key( data=json.dumps(data), headers=[('Content-Type', 'application/json'), ('Authorization', 'Bearer {}'.format(auth_header))] ) - app.celery.provider_tasks.deliver_sms.apply_async.assert_called_once_with([fake_uuid], queue='research-mode') + app.celery.provider_tasks.deliver_sms.apply_async.assert_called_once_with( + [fake_uuid], queue='research-mode-tasks' + ) assert response.status_code == 201 @@ -660,7 +662,9 @@ def test_should_send_email_to_anyone_with_test_key( headers=[('Content-Type', 'application/json'), ('Authorization', 'Bearer {}'.format(auth_header))] ) - app.celery.provider_tasks.deliver_email.apply_async.assert_called_once_with([fake_uuid], queue='research-mode') + app.celery.provider_tasks.deliver_email.apply_async.assert_called_once_with( + [fake_uuid], queue='research-mode-tasks' + ) assert response.status_code == 201 @@ -685,7 +689,7 @@ def test_should_send_sms_if_team_api_key_and_a_service_user(notify_api, sample_t data=json.dumps(data), headers=[('Content-Type', 'application/json'), ('Authorization', 'Bearer {}'.format(auth_header))]) - app.celery.provider_tasks.deliver_sms.apply_async.assert_called_once_with([fake_uuid], queue='send-sms') + app.celery.provider_tasks.deliver_sms.apply_async.assert_called_once_with([fake_uuid], queue='send-tasks') assert response.status_code == 201 @@ -718,7 +722,7 @@ def test_should_persist_notification(notify_api, sample_template, data=json.dumps(data), headers=[('Content-Type', 'application/json'), ('Authorization', 'Bearer {}'.format(auth_header))]) - mocked.assert_called_once_with([fake_uuid], queue='send-{}'.format(template_type)) + mocked.assert_called_once_with([fake_uuid], queue='send-tasks') assert response.status_code == 201 notification = notifications_dao.get_notification_by_id(fake_uuid) @@ -761,7 +765,7 @@ def test_should_delete_notification_and_return_error_if_sqs_fails( data=json.dumps(data), headers=[('Content-Type', 'application/json'), ('Authorization', 'Bearer {}'.format(auth_header))]) - mocked.assert_called_once_with([fake_uuid], queue='send-{}'.format(template_type)) + mocked.assert_called_once_with([fake_uuid], queue='send-tasks') assert response.status_code == 500 assert not notifications_dao.get_notification_by_id(fake_uuid) assert not NotificationHistory.query.get(fake_uuid) @@ -1046,7 +1050,7 @@ def test_send_notification_uses_priority_queue_when_template_is_marked_as_priori notification_id = response_data['notification']['id'] assert response.status_code == 201 - mocked.assert_called_once_with([notification_id], queue='priority') + mocked.assert_called_once_with([notification_id], queue='priority-tasks') @pytest.mark.parametrize( @@ -1114,7 +1118,7 @@ def test_should_allow_store_original_number_on_sms_notification(client, sample_t response_data = json.loads(response.data)['data'] notification_id = response_data['notification']['id'] - mocked.assert_called_once_with([notification_id], queue='send-sms') + mocked.assert_called_once_with([notification_id], queue='send-tasks') assert response.status_code == 201 assert notification_id notifications = Notification.query.all() diff --git a/tests/app/notifications/test_process_notification.py b/tests/app/notifications/test_process_notification.py index be2356c69..3220ef2ed 100644 --- a/tests/app/notifications/test_process_notification.py +++ b/tests/app/notifications/test_process_notification.py @@ -7,13 +7,14 @@ from sqlalchemy.exc import SQLAlchemyError from freezegun import freeze_time from collections import namedtuple -from app.models import Template, Notification, NotificationHistory +from app.models import Template, Notification, NotificationHistory, ScheduledNotification from app.notifications import SendNotificationToQueueError from app.notifications.process_notifications import ( create_content_for_notification, persist_notification, send_notification_to_queue, - simulated_recipient + simulated_recipient, + persist_scheduled_notification ) from notifications_utils.recipients import validate_and_format_phone_number, validate_and_format_email_address from app.utils import cache_key_for_service_template_counter @@ -207,17 +208,17 @@ def test_persist_notification_increments_cache_if_key_exists(sample_template, sa @pytest.mark.parametrize('research_mode, requested_queue, expected_queue, notification_type, key_type', - [(True, None, 'research-mode', 'sms', 'normal'), - (True, None, 'research-mode', 'email', 'normal'), - (True, None, 'research-mode', 'email', 'team'), - (False, None, 'send-sms', 'sms', 'normal'), - (False, None, 'send-email', 'email', 'normal'), - (False, None, 'send-sms', 'sms', 'team'), - (False, None, 'research-mode', 'sms', 'test'), - (True, 'notify', 'research-mode', 'email', 'normal'), - (False, 'notify', 'notify', 'sms', 'normal'), - (False, 'notify', 'notify', 'email', 'normal'), - (False, 'notify', 'research-mode', 'sms', 'test')]) + [(True, None, 'research-mode-tasks', 'sms', 'normal'), + (True, None, 'research-mode-tasks', 'email', 'normal'), + (True, None, 'research-mode-tasks', 'email', 'team'), + (False, None, 'send-tasks', 'sms', 'normal'), + (False, None, 'send-tasks', 'email', 'normal'), + (False, None, 'send-tasks', 'sms', 'team'), + (False, None, 'research-mode-tasks', 'sms', 'test'), + (True, 'notify-internal-tasks', 'research-mode-tasks', 'email', 'normal'), + (False, 'notify-internal-tasks', 'notify-internal-tasks', 'sms', 'normal'), + (False, 'notify-internal-tasks', 'notify-internal-tasks', 'email', 'normal'), + (False, 'notify-internal-tasks', 'research-mode-tasks', 'sms', 'test')]) def test_send_notification_to_queue(notify_db, notify_db_session, research_mode, requested_queue, expected_queue, notification_type, key_type, mocker): @@ -339,3 +340,75 @@ def test_persist_notification_with_international_info_does_not_store_for_email( assert persisted_notification.international is False assert persisted_notification.phone_prefix is None assert persisted_notification.rate_multiplier is None + + +def test_persist_scheduled_notification(sample_notification): + persist_scheduled_notification(sample_notification.id, '2017-05-12 14:15') + scheduled_notification = ScheduledNotification.query.all() + assert len(scheduled_notification) == 1 + assert scheduled_notification[0].notification_id == sample_notification.id + assert scheduled_notification[0].scheduled_for == datetime.datetime(2017, 5, 12, 13, 15) + + +@pytest.mark.parametrize('recipient, expected_recipient_normalised', [ + ('7900900123', '447900900123'), + ('+447900 900 123', '447900900123'), + (' 07700900222', '447700900222'), + ('07700900222', '447700900222'), + (' 73122345678', '73122345678'), + ('360623400400', '360623400400'), + ('-077-00900222-', '447700900222'), + ('(360623(400400)', '360623400400') + +]) +def test_persist_sms_notification_stores_normalised_number( + sample_job, + sample_api_key, + mocker, + recipient, + expected_recipient_normalised +): + persist_notification( + template_id=sample_job.template.id, + template_version=sample_job.template.version, + recipient=recipient, + service=sample_job.service, + personalisation=None, + notification_type='sms', + api_key_id=sample_api_key.id, + key_type=sample_api_key.key_type, + job_id=sample_job.id, + ) + persisted_notification = Notification.query.all()[0] + + assert persisted_notification.to == recipient + assert persisted_notification.normalised_to == expected_recipient_normalised + + +@pytest.mark.parametrize('recipient, expected_recipient_normalised', [ + ('FOO@bar.com', 'foo@bar.com'), + ('BAR@foo.com', 'bar@foo.com') + +]) +def test_persist_email_notification_stores_normalised_email( + sample_job, + sample_api_key, + mocker, + recipient, + expected_recipient_normalised +): + persist_notification( + template_id=sample_job.template.id, + template_version=sample_job.template.version, + recipient=recipient, + service=sample_job.service, + personalisation=None, + notification_type='email', + api_key_id=sample_api_key.id, + key_type=sample_api_key.key_type, + job_id=sample_job.id, + ) + persisted_notification = Notification.query.all()[0] + + assert persisted_notification.to == recipient + assert persisted_notification.normalised_to == expected_recipient_normalised diff --git a/tests/app/notifications/test_receive_notification.py b/tests/app/notifications/test_receive_notification.py index f325fe6f9..e82d3e638 100644 --- a/tests/app/notifications/test_receive_notification.py +++ b/tests/app/notifications/test_receive_notification.py @@ -16,3 +16,17 @@ def test_receive_notification_returns_received_to_mmg(client): assert response.status_code == 200 assert response.get_data(as_text=True) == 'RECEIVED' + + +def test_receive_notification_returns_received_to_firetext(client): + data = "source=07999999999&destination=07111111111&message=this is a message&time=2017-01-01 12:00:00" + + response = client.post( + path='/notifications/sms/receive/firetext', + data=data, + headers=[('Content-Type', 'application/x-www-form-urlencoded')]) + + assert response.status_code == 200 + result = json.loads(response.get_data(as_text=True)) + + assert result['status'] == 'ok' diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 691f0895c..a384c9136 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -10,7 +10,7 @@ from freezegun import freeze_time from app.dao.users_dao import save_model_user from app.dao.services_dao import dao_remove_user_from_service -from app.models import User, Organisation, DVLA_ORG_LAND_REGISTRY, Rate +from app.models import User, Organisation, DVLA_ORG_LAND_REGISTRY, Rate, ServicePermission from tests import create_authorization_header from tests.app.db import create_template from tests.app.conftest import ( @@ -20,28 +20,30 @@ from tests.app.conftest import ( sample_notification_history as create_notification_history, sample_notification_with_job ) -from app.models import KEY_TYPE_NORMAL, KEY_TYPE_TEAM, KEY_TYPE_TEST +from app.models import ( + Service, ServicePermission, + KEY_TYPE_NORMAL, KEY_TYPE_TEAM, KEY_TYPE_TEST, + EMAIL_TYPE, SMS_TYPE, LETTER_TYPE, INTERNATIONAL_SMS_TYPE, INBOUND_SMS_TYPE +) from tests.app.db import create_user -def test_get_service_list(notify_api, service_factory): - with notify_api.test_request_context(): - with notify_api.test_client() as client: - service_factory.get('one') - service_factory.get('two') - service_factory.get('three') - auth_header = create_authorization_header() - response = client.get( - '/service', - headers=[auth_header] - ) - assert response.status_code == 200 - json_resp = json.loads(response.get_data(as_text=True)) - assert len(json_resp['data']) == 3 - assert json_resp['data'][0]['name'] == 'one' - assert json_resp['data'][1]['name'] == 'two' - assert json_resp['data'][2]['name'] == 'three' +def test_get_service_list(client, service_factory): + service_factory.get('one') + service_factory.get('two') + service_factory.get('three') + auth_header = create_authorization_header() + response = client.get( + '/service', + headers=[auth_header] + ) + assert response.status_code == 200 + json_resp = json.loads(response.get_data(as_text=True)) + assert len(json_resp['data']) == 3 + assert json_resp['data'][0]['name'] == 'one' + assert json_resp['data'][1]['name'] == 'two' + assert json_resp['data'][2]['name'] == 'three' def test_get_service_list_with_only_active_flag(client, service_factory): @@ -117,17 +119,15 @@ def test_get_service_list_by_user_should_return_empty_list_if_no_services(client assert len(json_resp['data']) == 0 -def test_get_service_list_should_return_empty_list_if_no_services(notify_api, notify_db, notify_db_session): - with notify_api.test_request_context(): - with notify_api.test_client() as client: - auth_header = create_authorization_header() - response = client.get( - '/service', - headers=[auth_header] - ) - assert response.status_code == 200 - json_resp = json.loads(response.get_data(as_text=True)) - assert len(json_resp['data']) == 0 +def test_get_service_list_should_return_empty_list_if_no_services(client): + auth_header = create_authorization_header() + response = client.get( + '/service', + headers=[auth_header] + ) + assert response.status_code == 200 + json_resp = json.loads(response.get_data(as_text=True)) + assert len(json_resp['data']) == 0 def test_get_service_by_id(client, sample_service): @@ -147,6 +147,32 @@ def test_get_service_by_id(client, sample_service): assert json_resp['data']['sms_sender'] == current_app.config['FROM_NUMBER'] +def test_get_service_list_has_default_permissions(client, service_factory): + service_factory.get('one') + service_factory.get('two') + service_factory.get('three') + auth_header = create_authorization_header() + response = client.get( + '/service', + headers=[auth_header] + ) + assert response.status_code == 200 + json_resp = json.loads(response.get_data(as_text=True)) + assert len(json_resp['data']) == 3 + assert all([set(json['permissions']) == set([EMAIL_TYPE, SMS_TYPE]) for json in json_resp['data']]) + + +def test_get_service_by_id_has_default_service_permissions(client, sample_service): + auth_header = create_authorization_header() + resp = client.get( + '/service/{}'.format(sample_service.id), + headers=[auth_header] + ) + json_resp = json.loads(resp.get_data(as_text=True)) + + assert set(json_resp['data']['permissions']) == set([EMAIL_TYPE, SMS_TYPE]) + + def test_get_service_by_id_should_404_if_no_service(notify_api, notify_db): with notify_api.test_request_context(): with notify_api.test_client() as client: @@ -216,6 +242,10 @@ def test_create_service(client, sample_user): assert json_resp['data']['dvla_organisation'] == '001' assert json_resp['data']['sms_sender'] == current_app.config['FROM_NUMBER'] + service_db = Service.query.get(json_resp['data']['id']) + assert service_db.name == 'created service' + assert service_db.sms_sender == current_app.config['FROM_NUMBER'] + auth_header_fetch = create_authorization_header() resp = client.get( @@ -410,39 +440,194 @@ def test_update_service(client, notify_db, sample_service): assert result['data']['dvla_organisation'] == DVLA_ORG_LAND_REGISTRY -def test_update_service_flags(notify_api, sample_service): - with notify_api.test_request_context(): - with notify_api.test_client() as client: - auth_header = create_authorization_header() - resp = client.get( - '/service/{}'.format(sample_service.id), - headers=[auth_header] - ) - json_resp = json.loads(resp.get_data(as_text=True)) - assert resp.status_code == 200 - assert json_resp['data']['name'] == sample_service.name - assert json_resp['data']['research_mode'] is False - assert json_resp['data']['can_send_letters'] is False - assert json_resp['data']['can_send_international_sms'] is False +def test_update_service_flags(client, sample_service): + auth_header = create_authorization_header() + resp = client.get( + '/service/{}'.format(sample_service.id), + headers=[auth_header] + ) + json_resp = json.loads(resp.get_data(as_text=True)) + assert resp.status_code == 200 + assert json_resp['data']['name'] == sample_service.name + assert json_resp['data']['research_mode'] is False + assert json_resp['data']['can_send_letters'] is False + assert json_resp['data']['can_send_international_sms'] is False - data = { - 'research_mode': True, - 'can_send_letters': True, - 'can_send_international_sms': True, - } + data = { + 'research_mode': True, + 'can_send_letters': True, + 'can_send_international_sms': True, + } - auth_header = create_authorization_header() + auth_header = create_authorization_header() - resp = client.post( - '/service/{}'.format(sample_service.id), - data=json.dumps(data), - headers=[('Content-Type', 'application/json'), auth_header] - ) - result = json.loads(resp.get_data(as_text=True)) - assert resp.status_code == 200 - assert result['data']['research_mode'] is True - assert result['data']['can_send_letters'] is True - assert result['data']['can_send_international_sms'] is True + resp = client.post( + '/service/{}'.format(sample_service.id), + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), auth_header] + ) + result = json.loads(resp.get_data(as_text=True)) + assert resp.status_code == 200 + assert result['data']['research_mode'] is True + assert result['data']['can_send_letters'] is True + assert result['data']['can_send_international_sms'] is True + + +@pytest.fixture(scope='function') +def service_with_no_permissions(notify_db, notify_db_session): + return create_service(notify_db, notify_db_session, permissions=[]) + + +def test_update_service_flags_with_service_without_default_service_permissions(client, service_with_no_permissions): + auth_header = create_authorization_header() + data = { + 'can_send_letters': True, + 'can_send_international_sms': True, + } + + resp = client.post( + '/service/{}'.format(service_with_no_permissions.id), + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), auth_header] + ) + result = json.loads(resp.get_data(as_text=True)) + + assert resp.status_code == 200 + assert result['data']['can_send_letters'] is True + assert result['data']['can_send_international_sms'] is True + assert set(result['data']['permissions']) == set([LETTER_TYPE, INTERNATIONAL_SMS_TYPE]) + + +def test_update_service_flags_will_remove_service_permissions(client, notify_db, notify_db_session): + auth_header = create_authorization_header() + + service = create_service( + notify_db, notify_db_session, permissions=[SMS_TYPE, EMAIL_TYPE, INTERNATIONAL_SMS_TYPE]) + + assert service.can_send_international_sms is True + + data = { + 'can_send_international_sms': False + } + + resp = client.post( + '/service/{}'.format(service.id), + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), auth_header] + ) + result = json.loads(resp.get_data(as_text=True)) + + assert resp.status_code == 200 + assert result['data']['can_send_international_sms'] is False + + permissions = ServicePermission.query.filter_by(service_id=service.id).all() + assert set([p.permission for p in permissions]) == set([SMS_TYPE, EMAIL_TYPE]) + + +def test_update_permissions_will_override_permission_flags(client, service_with_no_permissions): + auth_header = create_authorization_header() + + data = { + 'permissions': [LETTER_TYPE, INTERNATIONAL_SMS_TYPE] + } + + resp = client.post( + '/service/{}'.format(service_with_no_permissions.id), + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), auth_header] + ) + result = json.loads(resp.get_data(as_text=True)) + + assert resp.status_code == 200 + assert result['data']['can_send_letters'] is True + assert result['data']['can_send_international_sms'] is True + assert set(result['data']['permissions']) == set([LETTER_TYPE, INTERNATIONAL_SMS_TYPE]) + + +def test_update_service_permissions_will_add_service_permissions(client, sample_service): + auth_header = create_authorization_header() + + data = { + 'permissions': [EMAIL_TYPE, SMS_TYPE, LETTER_TYPE] + } + + resp = client.post( + '/service/{}'.format(sample_service.id), + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), auth_header] + ) + result = json.loads(resp.get_data(as_text=True)) + + assert resp.status_code == 200 + assert set(result['data']['permissions']) == set([SMS_TYPE, EMAIL_TYPE, LETTER_TYPE]) + + +@pytest.mark.parametrize( + 'permission_to_add', + [ + (EMAIL_TYPE), + (SMS_TYPE), + (INTERNATIONAL_SMS_TYPE), + (LETTER_TYPE), + (INBOUND_SMS_TYPE), + ] +) +def test_add_service_permission_will_add_permission(client, service_with_no_permissions, permission_to_add): + auth_header = create_authorization_header() + + data = { + 'permissions': [permission_to_add] + } + + resp = client.post( + '/service/{}'.format(service_with_no_permissions.id), + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), auth_header] + ) + + permissions = ServicePermission.query.filter_by(service_id=service_with_no_permissions.id).all() + + assert resp.status_code == 200 + assert [p.permission for p in permissions] == [permission_to_add] + + +def test_update_permissions_with_an_invalid_permission_will_raise_error(client, sample_service): + auth_header = create_authorization_header() + invalid_permission = 'invalid_permission' + + data = { + 'permissions': [EMAIL_TYPE, SMS_TYPE, invalid_permission] + } + + resp = client.post( + '/service/{}'.format(sample_service.id), + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), auth_header] + ) + result = json.loads(resp.get_data(as_text=True)) + + assert resp.status_code == 400 + assert result['result'] == 'error' + assert "Invalid Service Permission: '{}'".format(invalid_permission) in result['message']['permissions'] + + +def test_update_permissions_with_duplicate_permissions_will_raise_error(client, sample_service): + auth_header = create_authorization_header() + + data = { + 'permissions': [EMAIL_TYPE, SMS_TYPE, LETTER_TYPE, LETTER_TYPE] + } + + resp = client.post( + '/service/{}'.format(sample_service.id), + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), auth_header] + ) + result = json.loads(resp.get_data(as_text=True)) + + assert resp.status_code == 400 + assert result['result'] == 'error' + assert "Duplicate Service Permission: ['{}']".format(LETTER_TYPE) in result['message']['permissions'] def test_update_service_research_mode_throws_validation_error(notify_api, sample_service): @@ -1609,48 +1794,59 @@ def test_get_monthly_billing_usage_returns_empty_list_if_no_notifications(client def test_search_for_notification_by_to_field(client, notify_db, notify_db_session): - notification1 = create_sample_notification(notify_db, notify_db_session, - to_field="+447700900855") - notification2 = create_sample_notification(notify_db, notify_db_session, to_field="jack@gmail.com") + create_notification = partial(create_sample_notification, notify_db, notify_db_session) + notification1 = create_notification(to_field='+447700900855', normalised_to='447700900855') + notification2 = create_notification(to_field='jack@gmail.com', normalised_to='jack@gmail.com') + + response = client.get( + '/service/{}/notifications?to={}'.format(notification1.service_id, 'jack@gmail.com'), + headers=[create_authorization_header()] + ) + notifications = json.loads(response.get_data(as_text=True))['notifications'] - response = client.get('/service/{}/notifications?to={}'.format(notification1.service_id, "jack@gmail.com"), - headers=[create_authorization_header()]) assert response.status_code == 200 - result = json.loads(response.get_data(as_text=True)) - assert len(result["notifications"]) == 1 - assert result["notifications"][0]["id"] == str(notification2.id) + assert len(notifications) == 1 + assert str(notification2.id) == notifications[0]['id'] def test_search_for_notification_by_to_field_return_empty_list_if_there_is_no_match( - client, notify_db, notify_db_session): - notification1 = create_sample_notification(notify_db, notify_db_session, - to_field="+447700900855") - notification2 = create_sample_notification(notify_db, notify_db_session, to_field="jack@gmail.com") + client, notify_db, notify_db_session +): + create_notification = partial(create_sample_notification, notify_db, notify_db_session) + notification1 = create_notification(to_field='+447700900855') + create_notification(to_field='jack@gmail.com') + + response = client.get( + '/service/{}/notifications?to={}'.format(notification1.service_id, '+447700900800'), + headers=[create_authorization_header()] + ) + notifications = json.loads(response.get_data(as_text=True))['notifications'] - response = client.get('/service/{}/notifications?to={}'.format(notification1.service_id, "+447700900800"), - headers=[create_authorization_header()]) assert response.status_code == 200 - assert len(json.loads(response.get_data(as_text=True))["notifications"]) == 0 + assert len(notifications) == 0 -def test_search_for_notification_by_to_field_return_multiple_matches( - client, notify_db, notify_db_session): - notification1 = create_sample_notification(notify_db, notify_db_session, - to_field="+447700900855") - notification2 = create_sample_notification(notify_db, notify_db_session, - to_field=" +44 77009 00855 ") - notification3 = create_sample_notification(notify_db, notify_db_session, - to_field="+44770 0900 855") - notification4 = create_sample_notification(notify_db, notify_db_session, to_field="jack@gmail.com") +def test_search_for_notification_by_to_field_return_multiple_matches(client, notify_db, notify_db_session): + create_notification = partial(create_sample_notification, notify_db, notify_db_session) + notification1 = create_notification(to_field='+447700900855', normalised_to='447700900855') + notification2 = create_notification(to_field=' +44 77009 00855 ', normalised_to='447700900855') + notification3 = create_notification(to_field='+44770 0900 855', normalised_to='447700900855') + notification4 = create_notification(to_field='jack@gmail.com', normalised_to='jack@gmail.com') + + response = client.get( + '/service/{}/notifications?to={}'.format(notification1.service_id, '+447700900855'), + headers=[create_authorization_header()] + ) + notifications = json.loads(response.get_data(as_text=True))['notifications'] + notification_ids = [notification['id'] for notification in notifications] - response = client.get('/service/{}/notifications?to={}'.format(notification1.service_id, "+447700900855"), - headers=[create_authorization_header()]) assert response.status_code == 200 - result = json.loads(response.get_data(as_text=True)) - assert len(result["notifications"]) == 3 - assert str(notification1.id) in [n["id"] for n in result["notifications"]] - assert str(notification2.id) in [n["id"] for n in result["notifications"]] - assert str(notification3.id) in [n["id"] for n in result["notifications"]] + assert len(notifications) == 3 + + assert str(notification1.id) in notification_ids + assert str(notification2.id) in notification_ids + assert str(notification3.id) in notification_ids + assert str(notification4.id) not in notification_ids def test_update_service_calls_send_notification_as_service_becomes_live(notify_db, notify_db_session, client, mocker): @@ -1808,3 +2004,70 @@ def test_get_yearly_billing_usage_count_returns_from_cache_if_present(client, sa mock_year.assert_not_called() mock_query.assert_not_called() redis_set_mock.assert_not_called() + + +def test_update_service_works_when_sms_sender_is_null(sample_service, client, mocker): + sample_service.sms_sender = None + data = {'name': 'new name'} + + resp = client.post( + 'service/{}'.format(sample_service.id), + data=json.dumps(data), + headers=[create_authorization_header()], + content_type='application/json' + ) + + assert resp.status_code == 200 + # make sure it wasn't changed to not-null under the hood + assert sample_service.sms_sender is None + + +def test_search_for_notification_by_to_field_filters_by_status(client, notify_db, notify_db_session): + create_notification = partial( + create_sample_notification, + notify_db, + notify_db_session, + to_field='+447700900855', + normalised_to='447700900855' + ) + notification1 = create_notification(status='delivered') + create_notification(status='sending') + + response = client.get( + '/service/{}/notifications?to={}&status={}'.format( + notification1.service_id, '+447700900855', 'delivered' + ), + headers=[create_authorization_header()] + ) + notifications = json.loads(response.get_data(as_text=True))['notifications'] + notification_ids = [notification['id'] for notification in notifications] + + assert response.status_code == 200 + assert len(notifications) == 1 + assert str(notification1.id) in notification_ids + + +def test_search_for_notification_by_to_field_filters_by_statuses(client, notify_db, notify_db_session): + create_notification = partial( + create_sample_notification, + notify_db, + notify_db_session, + to_field='+447700900855', + normalised_to='447700900855' + ) + notification1 = create_notification(status='delivered') + notification2 = create_notification(status='sending') + + response = client.get( + '/service/{}/notifications?to={}&status={}&status={}'.format( + notification1.service_id, '+447700900855', 'delivered', 'sending' + ), + headers=[create_authorization_header()] + ) + notifications = json.loads(response.get_data(as_text=True))['notifications'] + notification_ids = [notification['id'] for notification in notifications] + + assert response.status_code == 200 + assert len(notifications) == 2 + assert str(notification1.id) in notification_ids + assert str(notification2.id) in notification_ids diff --git a/tests/app/test_utils.py b/tests/app/test_utils.py index 5e45bdf7f..3b795549f 100644 --- a/tests/app/test_utils.py +++ b/tests/app/test_utils.py @@ -4,8 +4,8 @@ import pytest from app.utils import ( get_london_midnight_in_utc, get_midnight_for_day_before, - get_utc_time_in_bst -) + convert_utc_time_in_bst, + convert_bst_to_utc) @pytest.mark.parametrize('date, expected_date', [ @@ -32,7 +32,15 @@ def test_get_midnight_for_day_before_returns_expected_date(date, expected_date): (datetime(2017, 3, 28, 10, 0), datetime(2017, 3, 28, 11, 0)), (datetime(2017, 10, 28, 1, 0), datetime(2017, 10, 28, 2, 0)), (datetime(2017, 10, 29, 1, 0), datetime(2017, 10, 29, 1, 0)), + (datetime(2017, 5, 12, 14), datetime(2017, 5, 12, 15, 0)) ]) def test_get_utc_in_bst_returns_expected_date(date, expected_date): - ret_date = get_utc_time_in_bst(date) + ret_date = convert_utc_time_in_bst(date) assert ret_date == expected_date + + +def test_convert_bst_to_utc(): + bst = "2017-05-12 13:15" + bst_datetime = datetime.strptime(bst, "%Y-%m-%d %H:%M") + utc = convert_bst_to_utc(bst_datetime) + assert utc == datetime(2017, 5, 12, 12, 15) diff --git a/tests/app/user/test_rest.py b/tests/app/user/test_rest.py index ddbb3eaae..73f60f05b 100644 --- a/tests/app/user/test_rest.py +++ b/tests/app/user/test_rest.py @@ -374,7 +374,7 @@ def test_send_user_reset_password_should_send_reset_password_link(client, assert resp.status_code == 204 notification = Notification.query.first() - mocked.assert_called_once_with([str(notification.id)], queue="notify") + mocked.assert_called_once_with([str(notification.id)], queue="notify-internal-tasks") def test_send_user_reset_password_should_return_400_when_email_is_missing(client, mocker): @@ -436,7 +436,7 @@ def test_send_already_registered_email(client, sample_user, already_registered_t assert resp.status_code == 204 notification = Notification.query.first() - mocked.assert_called_once_with(([str(notification.id)]), queue="notify") + mocked.assert_called_once_with(([str(notification.id)]), queue="notify-internal-tasks") def test_send_already_registered_email_returns_400_when_data_is_missing(client, sample_user): @@ -464,7 +464,7 @@ def test_send_user_confirm_new_email_returns_204(client, sample_user, change_ema notification = Notification.query.first() mocked.assert_called_once_with( ([str(notification.id)]), - queue="notify") + queue="notify-internal-tasks") def test_send_user_confirm_new_email_returns_400_when_email_missing(client, sample_user, mocker): diff --git a/tests/app/user/test_rest_verify.py b/tests/app/user/test_rest_verify.py index 08f510bbd..84a88f6a0 100644 --- a/tests/app/user/test_rest_verify.py +++ b/tests/app/user/test_rest_verify.py @@ -218,7 +218,7 @@ def test_send_user_sms_code(client, app.celery.provider_tasks.deliver_sms.apply_async.assert_called_once_with( ([str(notification.id)]), - queue="notify" + queue="notify-internal-tasks" ) @@ -246,7 +246,7 @@ def test_send_user_code_for_sms_with_optional_to_field(client, assert notification.to == to_number app.celery.provider_tasks.deliver_sms.apply_async.assert_called_once_with( ([str(notification.id)]), - queue="notify" + queue="notify-internal-tasks" ) @@ -294,7 +294,7 @@ def test_send_user_email_verification(client, headers=[('Content-Type', 'application/json'), auth_header]) assert resp.status_code == 204 notification = Notification.query.first() - mocked.assert_called_once_with(([str(notification.id)]), queue="notify") + mocked.assert_called_once_with(([str(notification.id)]), queue="notify-internal-tasks") def test_send_email_verification_returns_404_for_bad_input_data(client, notify_db_session, mocker): diff --git a/tests/app/v2/notifications/test_get_notifications.py b/tests/app/v2/notifications/test_get_notifications.py index 6e3b72ed2..ff487275c 100644 --- a/tests/app/v2/notifications/test_get_notifications.py +++ b/tests/app/v2/notifications/test_get_notifications.py @@ -4,10 +4,10 @@ from flask import json from app import DATETIME_FORMAT from tests import create_authorization_header -from tests.app.conftest import ( - sample_notification as create_sample_notification, - sample_template as create_sample_template -) +from tests.app.db import ( + create_notification, + create_template, + create_service) @pytest.mark.parametrize('billable_units, provider', [ @@ -16,12 +16,15 @@ from tests.app.conftest import ( (1, None) ]) def test_get_notification_by_id_returns_200( - client, notify_db, notify_db_session, sample_provider_rate, billable_units, provider + client, billable_units, provider, sample_template ): - sample_notification = create_sample_notification( - notify_db, notify_db_session, billable_units=billable_units, sent_by=provider - ) + sample_notification = create_notification(template=sample_template, billable_units=billable_units, sent_by=provider, + scheduled_for="2017-05-12 15:15" + ) + another = create_notification(template=sample_template, billable_units=billable_units, sent_by=provider, + scheduled_for="2017-06-12 15:15" + ) auth_header = create_authorization_header(service_id=sample_notification.service_id) response = client.get( path='/v2/notifications/{}'.format(sample_notification.id), @@ -57,18 +60,19 @@ def test_get_notification_by_id_returns_200( 'body': sample_notification.template.content, "subject": None, 'sent_at': sample_notification.sent_at, - 'completed_at': sample_notification.completed_at() + 'completed_at': sample_notification.completed_at(), + 'scheduled_for': '2017-05-12T14:15:00.000000Z' } assert json_response == expected_response def test_get_notification_by_id_with_placeholders_returns_200( - client, notify_db, notify_db_session, sample_email_template_with_placeholders + client, sample_email_template_with_placeholders ): - sample_notification = create_sample_notification( - notify_db, notify_db_session, template=sample_email_template_with_placeholders, personalisation={"name": "Bob"} - ) + sample_notification = create_notification(template=sample_email_template_with_placeholders, + personalisation={"name": "Bob"} + ) auth_header = create_authorization_header(service_id=sample_notification.service_id) response = client.get( @@ -105,15 +109,16 @@ def test_get_notification_by_id_with_placeholders_returns_200( 'body': "Hello Bob\nThis is an email from GOV.\u200bUK", "subject": "Bob", 'sent_at': sample_notification.sent_at, - 'completed_at': sample_notification.completed_at() + 'completed_at': sample_notification.completed_at(), + 'scheduled_for': None } assert json_response == expected_response -def test_get_notification_by_reference_returns_200(client, notify_db, notify_db_session): - sample_notification_with_reference = create_sample_notification( - notify_db, notify_db_session, client_reference='some-client-reference') +def test_get_notification_by_reference_returns_200(client, sample_template): + sample_notification_with_reference = create_notification(template=sample_template, + client_reference='some-client-reference') auth_header = create_authorization_header(service_id=sample_notification_with_reference.service_id) response = client.get( @@ -130,6 +135,26 @@ def test_get_notification_by_reference_returns_200(client, notify_db, notify_db_ assert json_response['notifications'][0]['reference'] == "some-client-reference" +def test_get_notifications_returns_scheduled_for(client, sample_template): + sample_notification_with_reference = create_notification(template=sample_template, + client_reference='some-client-reference', + scheduled_for='2017-05-23 17:15') + + auth_header = create_authorization_header(service_id=sample_notification_with_reference.service_id) + response = client.get( + path='/v2/notifications?reference={}'.format(sample_notification_with_reference.client_reference), + headers=[('Content-Type', 'application/json'), auth_header]) + + assert response.status_code == 200 + assert response.headers['Content-type'] == 'application/json' + + json_response = json.loads(response.get_data(as_text=True)) + assert len(json_response['notifications']) == 1 + + assert json_response['notifications'][0]['id'] == str(sample_notification_with_reference.id) + assert json_response['notifications'][0]['scheduled_for'] == "2017-05-23T16:15:00.000000Z" + + def test_get_notification_by_reference_nonexistent_reference_returns_no_notifications(client, sample_service): auth_header = create_authorization_header(service_id=sample_service.id) response = client.get( @@ -182,8 +207,8 @@ def test_get_notification_by_id_invalid_id(client, sample_notification, id): } -def test_get_all_notifications_returns_200(client, notify_db, notify_db_session): - notifications = [create_sample_notification(notify_db, notify_db_session) for _ in range(2)] +def test_get_all_notifications_returns_200(client, sample_template): + notifications = [create_notification(template=sample_template) for _ in range(2)] notification = notifications[-1] auth_header = create_authorization_header(service_id=notification.service_id) @@ -208,6 +233,7 @@ def test_get_all_notifications_returns_200(client, notify_db, notify_db_session) } assert json_response['notifications'][0]['phone_number'] == "+447700900855" assert json_response['notifications'][0]['type'] == "sms" + assert not json_response['notifications'][0]['scheduled_for'] def test_get_all_notifications_no_notifications_if_no_notifications(client, sample_service): @@ -225,13 +251,13 @@ def test_get_all_notifications_no_notifications_if_no_notifications(client, samp assert len(json_response['notifications']) == 0 -def test_get_all_notifications_filter_by_template_type(client, notify_db, notify_db_session): - email_template = create_sample_template(notify_db, notify_db_session, template_type="email") - sms_template = create_sample_template(notify_db, notify_db_session, template_type="sms") +def test_get_all_notifications_filter_by_template_type(client): + service = create_service() + email_template = create_template(service=service, template_type="email") + sms_template = create_template(service=service, template_type="sms") - notification = create_sample_notification( - notify_db, notify_db_session, template=email_template, to_field="don.draper@scdp.biz") - create_sample_notification(notify_db, notify_db_session, template=sms_template) + notification = create_notification(template=email_template, to_field="don.draper@scdp.biz") + create_notification(template=sms_template) auth_header = create_authorization_header(service_id=notification.service_id) response = client.get( @@ -273,9 +299,9 @@ def test_get_all_notifications_filter_by_template_type_invalid_template_type(cli assert json_response['errors'][0]['message'] == "template_type orange is not one of [sms, email, letter]" -def test_get_all_notifications_filter_by_single_status(client, notify_db, notify_db_session): - notification = create_sample_notification(notify_db, notify_db_session, status="pending") - create_sample_notification(notify_db, notify_db_session) +def test_get_all_notifications_filter_by_single_status(client, sample_template): + notification = create_notification(template=sample_template, status="pending") + create_notification(template=sample_template) auth_header = create_authorization_header(service_id=notification.service_id) response = client.get( @@ -311,12 +337,12 @@ def test_get_all_notifications_filter_by_status_invalid_status(client, sample_no "delivered, pending, failed, technical-failure, temporary-failure, permanent-failure]" -def test_get_all_notifications_filter_by_multiple_statuses(client, notify_db, notify_db_session): +def test_get_all_notifications_filter_by_multiple_statuses(client, sample_template): notifications = [ - create_sample_notification(notify_db, notify_db_session, status=_status) + create_notification(template=sample_template, status=_status) for _status in ["created", "pending", "sending"] ] - failed_notification = create_sample_notification(notify_db, notify_db_session, status="permanent-failure") + failed_notification = create_notification(template=sample_template, status="permanent-failure") auth_header = create_authorization_header(service_id=notifications[0].service_id) response = client.get( @@ -338,10 +364,10 @@ def test_get_all_notifications_filter_by_multiple_statuses(client, notify_db, no assert failed_notification.id not in returned_notification_ids -def test_get_all_notifications_filter_by_failed_status(client, notify_db, notify_db_session): - created_notification = create_sample_notification(notify_db, notify_db_session, status="created") +def test_get_all_notifications_filter_by_failed_status(client, sample_template): + created_notification = create_notification(template=sample_template, status="created") failed_notifications = [ - create_sample_notification(notify_db, notify_db_session, status=_status) + create_notification(template=sample_template, status=_status) for _status in ["technical-failure", "temporary-failure", "permanent-failure"] ] @@ -365,9 +391,9 @@ def test_get_all_notifications_filter_by_failed_status(client, notify_db, notify assert created_notification.id not in returned_notification_ids -def test_get_all_notifications_filter_by_id(client, notify_db, notify_db_session): - older_notification = create_sample_notification(notify_db, notify_db_session) - newer_notification = create_sample_notification(notify_db, notify_db_session) +def test_get_all_notifications_filter_by_id(client, sample_template): + older_notification = create_notification(template=sample_template) + newer_notification = create_notification(template=sample_template) auth_header = create_authorization_header(service_id=newer_notification.service_id) response = client.get( @@ -398,8 +424,8 @@ def test_get_all_notifications_filter_by_id_invalid_id(client, sample_notificati assert json_response['errors'][0]['message'] == "older_than is not a valid UUID" -def test_get_all_notifications_filter_by_id_no_notifications_if_nonexistent_id(client, notify_db, notify_db_session): - notification = create_sample_notification(notify_db, notify_db_session) +def test_get_all_notifications_filter_by_id_no_notifications_if_nonexistent_id(client, sample_template): + notification = create_notification(template=sample_template) auth_header = create_authorization_header(service_id=notification.service_id) response = client.get( @@ -416,8 +442,8 @@ def test_get_all_notifications_filter_by_id_no_notifications_if_nonexistent_id(c assert len(json_response['notifications']) == 0 -def test_get_all_notifications_filter_by_id_no_notifications_if_last_notification(client, notify_db, notify_db_session): - notification = create_sample_notification(notify_db, notify_db_session) +def test_get_all_notifications_filter_by_id_no_notifications_if_last_notification(client, sample_template): + notification = create_notification(template=sample_template) auth_header = create_authorization_header(service_id=notification.service_id) response = client.get( @@ -433,23 +459,22 @@ def test_get_all_notifications_filter_by_id_no_notifications_if_last_notificatio assert len(json_response['notifications']) == 0 -def test_get_all_notifications_filter_multiple_query_parameters(client, notify_db, notify_db_session): - email_template = create_sample_template(notify_db, notify_db_session, template_type="email") - +def test_get_all_notifications_filter_multiple_query_parameters(client, sample_email_template): # this is the notification we are looking for - older_notification = create_sample_notification( - notify_db, notify_db_session, template=email_template, status="pending") + older_notification = create_notification( + template=sample_email_template, status="pending") # wrong status - create_sample_notification(notify_db, notify_db_session, template=email_template) + create_notification(template=sample_email_template) + wrong_template = create_template(sample_email_template.service, template_type='sms') # wrong template - create_sample_notification(notify_db, notify_db_session, status="pending") + create_notification(template=wrong_template, status="pending") # we only want notifications created before this one - newer_notification = create_sample_notification(notify_db, notify_db_session) + newer_notification = create_notification(template=sample_email_template) # this notification was created too recently - create_sample_notification(notify_db, notify_db_session, template=email_template, status="pending") + create_notification(template=sample_email_template, status="pending") auth_header = create_authorization_header(service_id=newer_notification.service_id) response = client.get( diff --git a/tests/app/v2/notifications/test_notification_schemas.py b/tests/app/v2/notifications/test_notification_schemas.py index 65a03b84d..0aafb38cc 100644 --- a/tests/app/v2/notifications/test_notification_schemas.py +++ b/tests/app/v2/notifications/test_notification_schemas.py @@ -2,6 +2,7 @@ import uuid import pytest from flask import json +from freezegun import freeze_time from jsonschema import ValidationError from app.v2.notifications.notification_schemas import ( @@ -246,7 +247,8 @@ def valid_email_response(): "id": str(uuid.uuid4()), "version": 1, "uri": "http://notify.api/v2/template/id" - } + }, + "scheduled_for": "" } @@ -262,7 +264,8 @@ def valid_email_response_with_optionals(): "id": str(uuid.uuid4()), "version": 1, "uri": "http://notify.api/v2/template/id" - } + }, + "schedule_for": "2017-05-12 13:00:00" } @@ -346,7 +349,72 @@ def test_get_notifications_response_with_email_and_phone_number(): "subject": "some subject", "created_at": "2016-01-01", "sent_at": "2016-01-01", - "completed_at": "2016-01-01" + "completed_at": "2016-01-01", + "schedule_for": "" } assert validate(response, get_notification_response) == response + + +@pytest.mark.parametrize("schema", + [post_email_request_schema, post_sms_request_schema]) +@freeze_time("2017-05-12 13:00:00") +def test_post_schema_valid_scheduled_for(schema): + j = {"template_id": str(uuid.uuid4()), + "email_address": "joe@gmail.com", + "scheduled_for": "2017-05-12 13:15"} + if schema == post_email_request_schema: + j.update({"email_address": "joe@gmail.com"}) + else: + j.update({"phone_number": "07515111111"}) + assert validate(j, schema) == j + + +@pytest.mark.parametrize("invalid_datetime", + ["13:00:00 2017-01-01", + "2017-31-12 13:00:00", + "01-01-2017T14:00:00.0000Z" + ]) +@pytest.mark.parametrize("schema", + [post_email_request_schema, post_sms_request_schema]) +def test_post_email_schema_invalid_scheduled_for(invalid_datetime, schema): + j = {"template_id": str(uuid.uuid4()), + "scheduled_for": invalid_datetime} + if schema == post_email_request_schema: + j.update({"email_address": "joe@gmail.com"}) + else: + j.update({"phone_number": "07515111111"}) + with pytest.raises(ValidationError) as e: + validate(j, schema) + error = json.loads(str(e.value)) + assert error['status_code'] == 400 + assert error['errors'] == [{'error': 'ValidationError', + 'message': "scheduled_for datetime format is invalid. " + "It must be a valid ISO8601 date time format, " + "https://en.wikipedia.org/wiki/ISO_8601"}] + + +@freeze_time("2017-05-12 13:00:00") +def test_scheduled_for_raises_validation_error_when_in_the_past(): + j = {"phone_number": "07515111111", + "template_id": str(uuid.uuid4()), + "scheduled_for": "2017-05-12 10:00"} + with pytest.raises(ValidationError) as e: + validate(j, post_sms_request_schema) + error = json.loads(str(e.value)) + assert error['status_code'] == 400 + assert error['errors'] == [{'error': 'ValidationError', + 'message': "scheduled_for datetime can not be in the past"}] + + +@freeze_time("2017-05-12 13:00:00") +def test_scheduled_for_raises_validation_error_when_more_than_24_hours_in_the_future(): + j = {"phone_number": "07515111111", + "template_id": str(uuid.uuid4()), + "scheduled_for": "2017-05-13 14:00"} + with pytest.raises(ValidationError) as e: + validate(j, post_sms_request_schema) + error = json.loads(str(e.value)) + assert error['status_code'] == 400 + assert error['errors'] == [{'error': 'ValidationError', + 'message': "scheduled_for datetime can only be 24 hours in the future"}] diff --git a/tests/app/v2/notifications/test_post_notifications.py b/tests/app/v2/notifications/test_post_notifications.py index 4e721f15f..61f2c4517 100644 --- a/tests/app/v2/notifications/test_post_notifications.py +++ b/tests/app/v2/notifications/test_post_notifications.py @@ -1,48 +1,51 @@ import uuid import pytest +from freezegun import freeze_time + +from app.models import Notification, ScheduledNotification, SCHEDULE_NOTIFICATIONS, EMAIL_TYPE, SMS_TYPE from flask import json, current_app from app.models import Notification from app.v2.errors import RateLimitError from tests import create_authorization_header from tests.app.conftest import sample_template as create_sample_template, sample_service +from tests.app.db import create_service, create_template @pytest.mark.parametrize("reference", [None, "reference_from_client"]) -def test_post_sms_notification_returns_201(notify_api, sample_template_with_placeholders, mocker, reference): - with notify_api.test_request_context(): - with notify_api.test_client() as client: - mocked = mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async') - data = { - 'phone_number': '+447700900855', - 'template_id': str(sample_template_with_placeholders.id), - 'personalisation': {' Name': 'Jo'} - } - if reference: - data.update({"reference": reference}) - auth_header = create_authorization_header(service_id=sample_template_with_placeholders.service_id) +def test_post_sms_notification_returns_201(client, sample_template_with_placeholders, mocker, reference): + mocked = mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async') + data = { + 'phone_number': '+447700900855', + 'template_id': str(sample_template_with_placeholders.id), + 'personalisation': {' Name': 'Jo'} + } + if reference: + data.update({"reference": reference}) + auth_header = create_authorization_header(service_id=sample_template_with_placeholders.service_id) - response = client.post( - path='/v2/notifications/sms', - data=json.dumps(data), - headers=[('Content-Type', 'application/json'), auth_header]) - assert response.status_code == 201 - resp_json = json.loads(response.get_data(as_text=True)) - notifications = Notification.query.all() - assert len(notifications) == 1 - notification_id = notifications[0].id - assert resp_json['id'] == str(notification_id) - assert resp_json['reference'] == reference - assert resp_json['content']['body'] == sample_template_with_placeholders.content.replace("(( Name))", "Jo") - assert resp_json['content']['from_number'] == current_app.config['FROM_NUMBER'] - assert 'v2/notifications/{}'.format(notification_id) in resp_json['uri'] - assert resp_json['template']['id'] == str(sample_template_with_placeholders.id) - assert resp_json['template']['version'] == sample_template_with_placeholders.version - assert 'services/{}/templates/{}'.format(sample_template_with_placeholders.service_id, - sample_template_with_placeholders.id) \ - in resp_json['template']['uri'] - assert mocked.called + response = client.post( + path='/v2/notifications/sms', + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), auth_header]) + assert response.status_code == 201 + resp_json = json.loads(response.get_data(as_text=True)) + notifications = Notification.query.all() + assert len(notifications) == 1 + notification_id = notifications[0].id + assert resp_json['id'] == str(notification_id) + assert resp_json['reference'] == reference + assert resp_json['content']['body'] == sample_template_with_placeholders.content.replace("(( Name))", "Jo") + assert resp_json['content']['from_number'] == current_app.config['FROM_NUMBER'] + assert 'v2/notifications/{}'.format(notification_id) in resp_json['uri'] + assert resp_json['template']['id'] == str(sample_template_with_placeholders.id) + assert resp_json['template']['version'] == sample_template_with_placeholders.version + assert 'services/{}/templates/{}'.format(sample_template_with_placeholders.service_id, + sample_template_with_placeholders.id) \ + in resp_json['template']['uri'] + assert not resp_json["scheduled_for"] + assert mocked.called @pytest.mark.parametrize("notification_type, key_send_to, send_to", @@ -150,6 +153,7 @@ def test_post_email_notification_returns_201(client, sample_email_template_with_ assert 'services/{}/templates/{}'.format(str(sample_email_template_with_placeholders.service_id), str(sample_email_template_with_placeholders.id)) \ in resp_json['template']['uri'] + assert not resp_json["scheduled_for"] assert mocked.called @@ -228,7 +232,7 @@ def test_send_notification_uses_priority_queue_when_template_is_marked_as_priori notification_id = json.loads(response.data)['id'] assert response.status_code == 201 - mocked.assert_called_once_with([notification_id], queue='priority') + mocked.assert_called_once_with([notification_id], queue='priority-tasks') @pytest.mark.parametrize( @@ -319,32 +323,78 @@ def test_post_sms_notification_returns_201_if_allowed_to_send_int_sms(notify_db, data=json.dumps(data), headers=[('Content-Type', 'application/json'), auth_header]) - print(json.loads(response.get_data(as_text=True))) assert response.status_code == 201 assert response.headers['Content-type'] == 'application/json' -def test_post_sms_should_persist_supplied_sms_number(notify_api, sample_template_with_placeholders, mocker): - with notify_api.test_request_context(): - with notify_api.test_client() as client: - mocked = mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async') - data = { - 'phone_number': '+(44) 77009-00855', - 'template_id': str(sample_template_with_placeholders.id), - 'personalisation': {' Name': 'Jo'} - } +def test_post_sms_should_persist_supplied_sms_number(client, sample_template_with_placeholders, mocker): + mocked = mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async') + data = { + 'phone_number': '+(44) 77009-00855', + 'template_id': str(sample_template_with_placeholders.id), + 'personalisation': {' Name': 'Jo'} + } - auth_header = create_authorization_header(service_id=sample_template_with_placeholders.service_id) + auth_header = create_authorization_header(service_id=sample_template_with_placeholders.service_id) - response = client.post( - path='/v2/notifications/sms', - data=json.dumps(data), - headers=[('Content-Type', 'application/json'), auth_header]) - assert response.status_code == 201 - resp_json = json.loads(response.get_data(as_text=True)) - notifications = Notification.query.all() - assert len(notifications) == 1 - notification_id = notifications[0].id - assert '+(44) 77009-00855' == notifications[0].to - assert resp_json['id'] == str(notification_id) - assert mocked.called + response = client.post( + path='/v2/notifications/sms', + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), auth_header]) + assert response.status_code == 201 + resp_json = json.loads(response.get_data(as_text=True)) + notifications = Notification.query.all() + assert len(notifications) == 1 + notification_id = notifications[0].id + assert '+(44) 77009-00855' == notifications[0].to + assert resp_json['id'] == str(notification_id) + assert mocked.called + + +@pytest.mark.parametrize("notification_type, key_send_to, send_to", + [("sms", "phone_number", "07700 900 855"), + ("email", "email_address", "sample@email.com")]) +@freeze_time("2017-05-14 14:00:00") +def test_post_notification_with_scheduled_for(client, notify_db, notify_db_session, + notification_type, key_send_to, send_to): + service = create_service(service_name=str(uuid.uuid4()), + service_permissions=[EMAIL_TYPE, SMS_TYPE, SCHEDULE_NOTIFICATIONS]) + template = create_template(service=service, template_type=notification_type) + data = { + key_send_to: send_to, + 'template_id': str(template.id) if notification_type == 'email' else str(template.id), + 'scheduled_for': '2017-05-14 14:15' + } + auth_header = create_authorization_header(service_id=service.id) + + response = client.post('/v2/notifications/{}'.format(notification_type), + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), auth_header]) + assert response.status_code == 201 + resp_json = json.loads(response.get_data(as_text=True)) + scheduled_notification = ScheduledNotification.query.filter_by(notification_id=resp_json["id"]).all() + assert len(scheduled_notification) == 1 + assert resp_json["id"] == str(scheduled_notification[0].notification_id) + assert resp_json["scheduled_for"] == '2017-05-14 14:15' + + +@pytest.mark.parametrize("notification_type, key_send_to, send_to", + [("sms", "phone_number", "07700 900 855"), + ("email", "email_address", "sample@email.com")]) +@freeze_time("2017-05-14 14:00:00") +def test_post_notification_raises_bad_request_if_service_not_invited_to_schedule( + client, sample_template, sample_email_template, notification_type, key_send_to, send_to): + data = { + key_send_to: send_to, + 'template_id': str(sample_email_template.id) if notification_type == 'email' else str(sample_template.id), + 'scheduled_for': '2017-05-14 14:15' + } + auth_header = create_authorization_header(service_id=sample_template.service_id) + + response = client.post('/v2/notifications/{}'.format(notification_type), + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), auth_header]) + assert response.status_code == 400 + error_json = json.loads(response.get_data(as_text=True)) + assert error_json['errors'] == [ + {"error": "BadRequestError", "message": 'Cannot schedule notifications (this feature is invite-only)'}]