diff --git a/app/celery/scheduled_tasks.py b/app/celery/scheduled_tasks.py index 460280424..457877ed0 100644 --- a/app/celery/scheduled_tasks.py +++ b/app/celery/scheduled_tasks.py @@ -14,13 +14,16 @@ from app.dao.jobs_dao import dao_set_scheduled_jobs_to_pending, dao_get_jobs_old from app.dao.notifications_dao import ( dao_timeout_notifications, is_delivery_slow_for_provider, - delete_notifications_created_more_than_a_week_ago_by_type) + 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 @@ -42,11 +45,26 @@ def run_scheduled_jobs(): for job in dao_set_scheduled_jobs_to_pending(): 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(): diff --git a/app/celery/tasks.py b/app/celery/tasks.py index 8fc6954f8..bb9967ee4 100644 --- a/app/celery/tasks.py +++ b/app/celery/tasks.py @@ -160,19 +160,20 @@ 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)], 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/config.py b/app/config.py index 76917b79c..60d182d91 100644 --- a/app/config.py +++ b/app/config.py @@ -154,6 +154,11 @@ class Config(object): 'schedule': crontab(minute=1), '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), diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index 6d99d1163..7ebf9170c 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -5,9 +5,16 @@ 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 +) 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 +34,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 +170,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 +193,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( @@ -459,7 +473,45 @@ 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: + normalised = validate_and_format_email_address(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..572e36e63 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -31,7 +31,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 @@ -136,10 +138,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/models.py b/app/models.py index 8b3b8e053..8c1e22e79 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' @@ -215,6 +215,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 +241,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) @@ -679,6 +695,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) @@ -727,6 +744,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) @@ -848,7 +867,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, @@ -887,7 +906,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 @@ -953,6 +974,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/process_notifications.py b/app/notifications/process_notifications.py index d59477143..86291b46f 100644 --- a/app/notifications/process_notifications.py +++ b/app/notifications/process_notifications.py @@ -4,7 +4,8 @@ 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 @@ -12,10 +13,12 @@ from app.celery import provider_tasks from notifications_utils.clients import redis from app.config import QueueNames -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.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): @@ -72,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: @@ -122,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/validators.py b/app/notifications/validators.py index a680f446e..1193d4015 100644 --- a/app/notifications/validators.py +++ b/app/notifications/validators.py @@ -90,3 +90,8 @@ 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): + # TODO: implement once the service permission works. + raise BadRequestError(message="Your service must be invited to schedule notifications via the API.") 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 180740063..dfca05c89 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -5,11 +5,12 @@ from datetime import datetime from flask import ( jsonify, request, - current_app + current_app, + Blueprint ) from sqlalchemy.orm.exc import NoResultFound -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, @@ -39,11 +40,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 @@ -57,7 +60,6 @@ from app.schemas import ( detailed_service_schema ) from app.utils import pagination_links -from flask import Blueprint service_blueprint = Blueprint('service', __name__) @@ -93,12 +95,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']) @@ -108,9 +109,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 @@ -122,6 +128,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) @@ -258,8 +265,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') @@ -289,11 +296,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/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..6aa4dcb77 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", "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"} }, "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", "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"} }, "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 cf9269318..0b54d98af 100644 --- a/app/v2/notifications/post_notifications.py +++ b/app/v2/notifications/post_notifications.py @@ -3,19 +3,20 @@ from sqlalchemy.orm.exc import NoResultFound from app import api_user, authenticated_service from app.config import QueueNames -from app.dao import services_dao, templates_dao +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 @@ -33,6 +34,10 @@ def post_notification(notification_type): else: form = validate(request.get_json(), post_sms_request) + scheduled_for = form.get("scheduled_for", None) + if scheduled_for: + if not service_can_schedule_notification(authenticated_service): + return check_rate_limiting(authenticated_service, api_user) form_send_to = form['phone_number'] if notification_type == SMS_TYPE else form['email_address'] @@ -57,31 +62,35 @@ def post_notification(notification_type): client_reference=form.get('reference', None), simulated=simulated) - 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 - ) - + 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/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/requirements.txt b/requirements.txt index fe25533c1..3551937fa 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/delete_sqs_queues.py b/scripts/delete_sqs_queues.py new file mode 100644 index 000000000..bdbf6ff65 --- /dev/null +++ b/scripts/delete_sqs_queues.py @@ -0,0 +1,84 @@ +import boto3 +import csv +from datetime import datetime +from pprint import pprint +import os + +client = boto3.client('sqs', region_name=os.getenv('AWS_REGION')) + + +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'] + return queue_attributes + + +def delete_queue(queue_name): + response = client.delete_queue( + QueueUrl=queue_name + ) + if response['ResponseMetadata']['HTTPStatusCode'] == 200: + print('Deleted queue successfully') + else: + print('Error occured when attempting to delete queue') + pprint(response) + return response + + +def output_to_csv(queue_attributes): + csv_name = 'queues.csv' + with open(csv_name, 'w') as csvfile: + fieldnames = [ + 'Queue Name', + '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: + queue_url = client.get_queue_url( + QueueName=queue_attr['QueueArn'] + )['QueueUrl'] + writer.writerow({ + 'Queue Name': queue_attr['QueueArn'], + 'Queue URL': queue_url, + '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']) + }) + return csv_name + + +def read_from_csv(csv_name): + queue_urls = [] + with open(csv_name, 'r') as csvfile: + next(csvfile) + rows = csv.reader(csvfile, delimiter=',') + for row in rows: + queue_urls.append(row[1]) + return queue_urls + + +queues = get_queues() +for queue in queues: + delete_queue(queue) diff --git a/tests/app/celery/test_scheduled_tasks.py b/tests/app/celery/test_scheduled_tasks.py index 5566ebd70..7a347e35c 100644 --- a/tests/app/celery/test_scheduled_tasks.py +++ b/tests/app/celery/test_scheduled_tasks.py @@ -6,7 +6,8 @@ from functools import partial from flask import current_app from freezegun import freeze_time 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 + 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, @@ -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 @@ -416,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/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 1cede31bb..a6b69cd24 100644 --- a/tests/app/dao/test_notification_dao.py +++ b/tests/app/dao/test_notification_dao.py @@ -12,9 +12,11 @@ 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 @@ -40,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 @@ -354,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): @@ -714,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): @@ -1720,30 +1746,146 @@ 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 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/notifications/test_process_notification.py b/tests/app/notifications/test_process_notification.py index 8d707c7dc..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 @@ -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/service/test_rest.py b/tests/app/service/test_rest.py index 126e9b948..46adbba1b 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): @@ -1618,48 +1803,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): @@ -1731,3 +1927,70 @@ def test_update_service_does_not_call_send_notification_when_restricted_not_chan assert resp.status_code == 200 assert not send_notification_mock.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/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 0bd1156ba..8d112030a 100644 --- a/tests/app/v2/notifications/test_post_notifications.py +++ b/tests/app/v2/notifications/test_post_notifications.py @@ -1,6 +1,9 @@ import uuid import pytest +from freezegun import freeze_time + +from app.models import Notification, ScheduledNotification from flask import json, current_app from app.models import Notification @@ -10,39 +13,38 @@ from tests.app.conftest import sample_template as create_sample_template, sample @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 +152,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 @@ -319,32 +322,76 @@ 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.skip("Once the service can be invited to schedule notifications we can add this test.") +@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, 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 == 201 + resp_json = json.loads(response.get_data(as_text=True)) + scheduled_notification = ScheduledNotification.query.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_with_scheduled_for_raises_bad_request(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": 'Your service must be invited to schedule notifications via the API.'}]