diff --git a/app/celery/tasks.py b/app/celery/tasks.py index f3244edb5..bece59f7c 100644 --- a/app/celery/tasks.py +++ b/app/celery/tasks.py @@ -148,9 +148,9 @@ def process_row(row_number, recipient, personalisation, template, job, service): }) send_fns = { - SMS_TYPE: send_sms, - EMAIL_TYPE: send_email, - LETTER_TYPE: persist_letter + SMS_TYPE: save_sms, + EMAIL_TYPE: save_email, + LETTER_TYPE: save_letter } send_fn = send_fns[template_type] @@ -160,7 +160,6 @@ def process_row(row_number, recipient, personalisation, template, job, service): str(service.id), create_uuid(), encrypted, - datetime.utcnow().strftime(DATETIME_FORMAT) ), queue=QueueNames.DATABASE if not service.research_mode else QueueNames.RESEARCH_MODE ) @@ -181,13 +180,12 @@ def __sending_limits_for_job_exceeded(service, job, job_id): return False -@notify_celery.task(bind=True, name="send-sms", max_retries=5, default_retry_delay=300) +@notify_celery.task(bind=True, name="save-sms", max_retries=5, default_retry_delay=300) @statsd(namespace="tasks") -def send_sms(self, +def save_sms(self, service_id, notification_id, encrypted_notification, - created_at, api_key_id=None, key_type=KEY_TYPE_NORMAL): notification = encryption.decrypt(encrypted_notification) @@ -221,20 +219,22 @@ def send_sms(self, ) current_app.logger.info( - "SMS {} created at {} for job {}".format(saved_notification.id, created_at, notification.get('job', None)) + "SMS {} created at {} for job {}".format( + saved_notification.id, + saved_notification.created_at, + notification.get('job', None)) ) except SQLAlchemyError as e: handle_exception(self, notification, notification_id, e) -@notify_celery.task(bind=True, name="send-email", max_retries=5, default_retry_delay=300) +@notify_celery.task(bind=True, name="save-email", max_retries=5, default_retry_delay=300) @statsd(namespace="tasks") -def send_email(self, +def save_email(self, service_id, notification_id, encrypted_notification, - created_at, api_key_id=None, key_type=KEY_TYPE_NORMAL): notification = encryption.decrypt(encrypted_notification) @@ -265,19 +265,18 @@ def send_email(self, queue=QueueNames.SEND_EMAIL if not service.research_mode else QueueNames.RESEARCH_MODE ) - current_app.logger.info("Email {} created at {}".format(saved_notification.id, created_at)) + current_app.logger.info("Email {} created at {}".format(saved_notification.id, saved_notification.created_at)) except SQLAlchemyError as e: handle_exception(self, notification, notification_id, e) -@notify_celery.task(bind=True, name="persist-letter", max_retries=5, default_retry_delay=300) +@notify_celery.task(bind=True, name="save-letter", max_retries=5, default_retry_delay=300) @statsd(namespace="tasks") -def persist_letter( +def save_letter( self, service_id, notification_id, encrypted_notification, - created_at ): notification = encryption.decrypt(encrypted_notification) @@ -302,7 +301,7 @@ def persist_letter( reference=create_random_identifier() ) - current_app.logger.info("Letter {} created at {}".format(saved_notification.id, created_at)) + current_app.logger.info("Letter {} created at {}".format(saved_notification.id, saved_notification.created_at)) except SQLAlchemyError as e: handle_exception(self, notification, notification_id, e) diff --git a/app/dao/service_sms_sender_dao.py b/app/dao/service_sms_sender_dao.py index 18760cd29..69397f441 100644 --- a/app/dao/service_sms_sender_dao.py +++ b/app/dao/service_sms_sender_dao.py @@ -1,3 +1,5 @@ +from sqlalchemy import desc + from app import db from app.dao.dao_utils import transactional from app.models import ServiceSmsSender @@ -32,3 +34,78 @@ def insert_service_sms_sender(service, sms_sender): is_default=True ) db.session.add(new_sms_sender) + + +def dao_get_service_sms_senders_by_id(service_id, service_sms_sender_id): + return ServiceSmsSender.query.filter_by( + id=service_sms_sender_id, + service_id=service_id + ).one() + + +def dao_get_sms_senders_by_service_id(service_id): + return ServiceSmsSender.query.filter_by(service_id=service_id).order_by(desc(ServiceSmsSender.is_default)).all() + + +@transactional +def dao_add_sms_sender_for_service(service_id, sms_sender, is_default, inbound_number_id=None): + old_default = _get_existing_default(service_id=service_id) + if is_default: + _reset_old_default_to_false(old_default) + else: + _raise_when_no_default(old_default) + + new_sms_sender = ServiceSmsSender( + service_id=service_id, + sms_sender=sms_sender, + is_default=is_default, + inbound_number_id=inbound_number_id + ) + + db.session.add(new_sms_sender) + return new_sms_sender + + +@transactional +def dao_update_service_sms_sender(service_id, service_sms_sender_id, is_default, sms_sender=None): + old_default = _get_existing_default(service_id) + if is_default: + _reset_old_default_to_false(old_default) + else: + if old_default.id == service_sms_sender_id: + raise Exception("You must have at least one SMS sender as the default") + + sms_sender_to_update = ServiceSmsSender.query.get(service_sms_sender_id) + sms_sender_to_update.is_default = is_default + if not sms_sender_to_update.inbound_number_id and sms_sender: + sms_sender_to_update.sms_sender = sms_sender + db.session.add(sms_sender_to_update) + return sms_sender_to_update + + +def _get_existing_default(service_id): + sms_senders = dao_get_sms_senders_by_service_id(service_id=service_id) + if sms_senders: + old_default = [x for x in sms_senders if x.is_default] + if len(old_default) == 1: + return old_default[0] + else: + raise Exception( + "There should only be one default sms sender for each service. Service {} has {}".format( + service_id, + len(old_default) + ) + ) + return None + + +def _reset_old_default_to_false(old_default): + if old_default: + old_default.is_default = False + db.session.add(old_default) + + +def _raise_when_no_default(old_default): + # check that the update is not updating the only default to false + if not old_default: + raise Exception("You must have at least one SMS sender as the default.", 400) diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index 5ae25c0e6..e6e677adc 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -35,11 +35,19 @@ from app.models import ( JobStatistics, SMS_TYPE, EMAIL_TYPE, - ServiceSmsSender) + INTERNATIONAL_SMS_TYPE, + ServiceSmsSender, +) from app.service.statistics import format_monthly_template_notification_stats from app.statsd_decorators import statsd from app.utils import get_london_month_from_utc_column, get_london_midnight_in_utc +DEFAULT_SERVICE_PERMISSIONS = [ + SMS_TYPE, + EMAIL_TYPE, + INTERNATIONAL_SMS_TYPE, +] + def dao_fetch_all_services(only_active=False): query = Service.query.order_by( @@ -146,13 +154,16 @@ def dao_fetch_service_by_id_and_user(service_id, user_id): @transactional @version_class(Service) -def dao_create_service(service, user, service_id=None, service_permissions=[SMS_TYPE, EMAIL_TYPE]): +def dao_create_service(service, user, service_id=None, service_permissions=None): # the default property does not appear to work when there is a difference between the sqlalchemy schema and the # db schema (ie: during a migration), so we have to set sms_sender manually here. After the GOVUK sms_sender # migration is completed, this code should be able to be removed. if not service.sms_sender: service.sms_sender = current_app.config['FROM_NUMBER'] + if service_permissions is None: + service_permissions = DEFAULT_SERVICE_PERMISSIONS + from app.dao.permissions_dao import permission_dao service.users.append(user) permission_dao.add_default_service_permissions_for_user(user, service) @@ -412,6 +423,35 @@ def fetch_stats_by_date_range_for_all_services(start_date, end_date, include_fro return query.all() +@statsd(namespace='dao') +def fetch_aggregate_stats_by_date_range_for_all_services(start_date, end_date, include_from_test_key=True): + start_date = get_london_midnight_in_utc(start_date) + end_date = get_london_midnight_in_utc(end_date + timedelta(days=1)) + table = NotificationHistory + + if start_date >= datetime.utcnow() - timedelta(days=7): + table = Notification + + query = db.session.query( + table.notification_type, + table.status, + func.count(table.id).label('count') + ).filter( + table.created_at >= start_date, + table.created_at < end_date + ).group_by( + table.notification_type, + table.status + ).order_by( + table.notification_type + ) + + if not include_from_test_key: + query = query.filter(table.key_type != KEY_TYPE_TEST) + + return query.all() + + @transactional @version_class(Service) @version_class(ApiKey) diff --git a/app/models.py b/app/models.py index 19f89dafe..31cbd0a3d 100644 --- a/app/models.py +++ b/app/models.py @@ -321,7 +321,7 @@ class ServiceSmsSender(db.Model): id = db.Column(UUID(as_uuid=True), primary_key=True, default=uuid.uuid4) sms_sender = db.Column(db.String(11), nullable=False) - service_id = db.Column(UUID(as_uuid=True), db.ForeignKey('services.id'), unique=True, index=True, nullable=False) + service_id = db.Column(UUID(as_uuid=True), db.ForeignKey('services.id'), index=True, nullable=False) service = db.relationship(Service, backref=db.backref("service_sms_senders", uselist=True)) is_default = db.Column(db.Boolean, nullable=False, default=True) inbound_number_id = db.Column(UUID(as_uuid=True), db.ForeignKey('inbound_numbers.id'), @@ -330,6 +330,17 @@ class ServiceSmsSender(db.Model): created_at = db.Column(db.DateTime, default=datetime.datetime.utcnow, nullable=False) updated_at = db.Column(db.DateTime, nullable=True, onupdate=datetime.datetime.utcnow) + def serialize(self): + return { + "id": str(self.id), + "sms_sender": self.sms_sender, + "service_id": str(self.service_id), + "is_default": self.is_default, + "inbound_number_id": str(self.inbound_number_id) if self.inbound_number_id else None, + "created_at": self.created_at.strftime(DATETIME_FORMAT), + "updated_at": self.updated_at.strftime(DATETIME_FORMAT) if self.updated_at else None, + } + class ServicePermission(db.Model): __tablename__ = "service_permissions" diff --git a/app/service/rest.py b/app/service/rest.py index f36046d7c..eab409bc1 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -17,12 +17,19 @@ from app.dao.api_key_dao import ( get_model_api_keys, get_unsigned_secret, expire_api_key) +from app.dao.inbound_numbers_dao import dao_allocate_number_for_service from app.dao.service_inbound_api_dao import ( save_service_inbound_api, reset_service_inbound_api, get_service_inbound_api ) -from app.dao.service_sms_sender_dao import insert_or_update_service_sms_sender +from app.dao.service_sms_sender_dao import ( + insert_or_update_service_sms_sender, + dao_add_sms_sender_for_service, + dao_update_service_sms_sender, + dao_get_service_sms_senders_by_id, + dao_get_sms_senders_by_service_id +) from app.dao.services_dao import ( dao_fetch_service_by_id, dao_fetch_all_services, @@ -39,8 +46,8 @@ from app.dao.services_dao import ( dao_suspend_service, dao_resume_service, dao_fetch_monthly_historical_stats_for_service, - dao_fetch_monthly_historical_stats_by_template_for_service -) + dao_fetch_monthly_historical_stats_by_template_for_service, + fetch_aggregate_stats_by_date_range_for_all_services) from app.dao.service_whitelist_dao import ( dao_fetch_service_whitelist, dao_add_and_commit_whitelisted_contacts, @@ -70,11 +77,14 @@ from app.errors import ( from app.models import Service, ServiceInboundApi, AnnualBilling from app.schema_validation import validate from app.service import statistics -from app.service.service_inbound_api_schema import service_inbound_api, update_service_inbound_api_schema +from app.service.service_inbound_api_schema import ( + service_inbound_api, + update_service_inbound_api_schema +) from app.service.service_senders_schema import ( add_service_email_reply_to_request, add_service_letter_contact_block_request, -) + add_service_sms_sender_request) from app.service.utils import get_whitelist_objects from app.service.sender import send_notification_to_service_users from app.service.send_notification import send_one_off_notification @@ -95,6 +105,25 @@ service_blueprint = Blueprint('service', __name__) register_errors(service_blueprint) +@service_blueprint.route('/platform-stats', methods=['GET']) +def get_platform_stats(): + include_from_test_key = request.args.get('include_from_test_key', 'True') != 'False' + + # If start and end date are not set, we are expecting today's stats. + today = str(datetime.utcnow().date()) + + start_date = datetime.strptime(request.args.get('start_date', today), '%Y-%m-%d').date() + end_date = datetime.strptime(request.args.get('end_date', today), '%Y-%m-%d').date() + data = fetch_aggregate_stats_by_date_range_for_all_services(start_date=start_date, + end_date=end_date, + include_from_test_key=include_from_test_key + ) + stats = statistics.format_statistics(data) + + result = jsonify(stats) + return result + + @service_blueprint.route('', methods=['GET']) def get_services(): only_active = request.args.get('only_active') == 'True' @@ -621,6 +650,55 @@ def update_service_letter_contact(service_id, letter_contact_id): return jsonify(data=new_reply_to.serialize()), 200 +@service_blueprint.route('//sms-sender', methods=['POST']) +def add_service_sms_sender(service_id): + dao_fetch_service_by_id(service_id) + form = validate(request.get_json(), add_service_sms_sender_request) + inbound_number_id = form.get('inbound_number_id', None) + sms_sender = form.get('sms_sender') + if inbound_number_id: + updated_number = dao_allocate_number_for_service(service_id=service_id, inbound_number_id=inbound_number_id) + # the sms_sender in the form is the inbound_number_id from client, use number from table. + sms_sender = updated_number.number + new_sms_sender = dao_add_sms_sender_for_service(service_id=service_id, + sms_sender=sms_sender, + is_default=form['is_default'], + inbound_number_id=inbound_number_id + ) + return jsonify(new_sms_sender.serialize()), 201 + + +@service_blueprint.route('//sms-sender/', methods=['POST']) +def update_service_sms_sender(service_id, sms_sender_id): + form = validate(request.get_json(), add_service_sms_sender_request) + + sms_sender_to_update = dao_get_service_sms_senders_by_id(service_id=service_id, + service_sms_sender_id=sms_sender_id) + if sms_sender_to_update.inbound_number_id and form['sms_sender'] != sms_sender_to_update.sms_sender: + raise InvalidRequest("You can not change the inbound number for service {}".format(service_id), + status_code=400) + + new_sms_sender = dao_update_service_sms_sender(service_id=service_id, + service_sms_sender_id=sms_sender_id, + is_default=form['is_default'], + sms_sender=form['sms_sender'] + ) + return jsonify(new_sms_sender.serialize()), 200 + + +@service_blueprint.route('//sms-sender/', methods=['GET']) +def get_service_sms_sender_by_id(service_id, sms_sender_id): + sms_sender = dao_get_service_sms_senders_by_id(service_id=service_id, + service_sms_sender_id=sms_sender_id) + return jsonify(sms_sender.serialize()), 200 + + +@service_blueprint.route('//sms-sender', methods=['GET']) +def get_service_sms_senders_for_service(service_id): + sms_senders = dao_get_sms_senders_by_service_id(service_id=service_id) + return jsonify([sms_sender.serialize() for sms_sender in sms_senders]), 200 + + @service_blueprint.route('/unique', methods=["GET"]) def is_service_name_unique(): name, email_from = check_request_args(request) diff --git a/app/service/service_senders_schema.py b/app/service/service_senders_schema.py index e0a07b0a4..9d168cb8d 100644 --- a/app/service/service_senders_schema.py +++ b/app/service/service_senders_schema.py @@ -1,3 +1,5 @@ +from app.schema_validation.definitions import uuid + add_service_email_reply_to_request = { "$schema": "http://json-schema.org/draft-04/schema#", "description": "POST service email reply to address", @@ -22,3 +24,17 @@ add_service_letter_contact_block_request = { }, "required": ["contact_block", "is_default"] } + + +add_service_sms_sender_request = { + "$schema": "http://json-schema.org/draft-04/schema#", + "description": "POST add service SMS sender", + "type": "object", + "title": "Add new SMS sender for service", + "properties": { + "sms_sender": {"type": "string"}, + "is_default": {"type": "boolean"}, + "inbound_number_id": uuid + }, + "required": ["sms_sender", "is_default"] +} diff --git a/manifest-api-preview.yml b/manifest-api-preview.yml index 1eb88ef10..1e4174cb1 100644 --- a/manifest-api-preview.yml +++ b/manifest-api-preview.yml @@ -2,6 +2,17 @@ inherit: manifest-api-base.yml +services: + - notify-aws + - notify-config + - notify-db + - mmg + - firetext + - hosted-graphite + - redis + - performance-platform + - logit-ssl-syslog-drain + routes: - route: notify-api-preview.cloudapps.digital - route: api.notify.works diff --git a/manifest-api-staging.yml b/manifest-api-staging.yml index ec6602930..67044f918 100644 --- a/manifest-api-staging.yml +++ b/manifest-api-staging.yml @@ -2,6 +2,17 @@ inherit: manifest-api-base.yml +services: + - notify-aws + - notify-config + - notify-db + - mmg + - firetext + - hosted-graphite + - redis + - performance-platform + - logit-ssl-syslog-drain + routes: - route: notify-api-staging.cloudapps.digital - route: api.staging-notify.works diff --git a/manifest-delivery-preview.yml b/manifest-delivery-preview.yml index 492bc6c55..ec8203cac 100644 --- a/manifest-delivery-preview.yml +++ b/manifest-delivery-preview.yml @@ -1,4 +1,16 @@ --- inherit: manifest-delivery-base.yml + +services: + - notify-aws + - notify-config + - notify-db + - mmg + - firetext + - hosted-graphite + - redis + - performance-platform + - logit-ssl-syslog-drain + memory: 1G diff --git a/manifest-delivery-staging.yml b/manifest-delivery-staging.yml index 53c8d2f12..5c7c1dea7 100644 --- a/manifest-delivery-staging.yml +++ b/manifest-delivery-staging.yml @@ -2,5 +2,16 @@ inherit: manifest-delivery-base.yml +services: + - notify-aws + - notify-config + - notify-db + - mmg + - firetext + - hosted-graphite + - redis + - performance-platform + - logit-ssl-syslog-drain + instances: 2 memory: 1G diff --git a/migrations/versions/0127_remove_unique_constraint.py b/migrations/versions/0127_remove_unique_constraint.py new file mode 100644 index 000000000..2db71b9ea --- /dev/null +++ b/migrations/versions/0127_remove_unique_constraint.py @@ -0,0 +1,24 @@ +""" + +Revision ID: 0127_remove_unique_constraint +Revises: 0126_add_annual_billing +Create Date: 2017-10-17 16:47:37.826333 + +""" +from alembic import op +import sqlalchemy as sa + +revision = '0127_remove_unique_constraint' +down_revision = '0126_add_annual_billing' + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.drop_index('ix_service_sms_senders_service_id', table_name='service_sms_senders') + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.create_index('ix_service_sms_senders_service_id', 'service_sms_senders', ['service_id'], unique=True) + # ### end Alembic commands ### diff --git a/tests/app/celery/test_tasks.py b/tests/app/celery/test_tasks.py index 6841f69e7..e018a4f32 100644 --- a/tests/app/celery/test_tasks.py +++ b/tests/app/celery/test_tasks.py @@ -19,9 +19,9 @@ from app.celery.tasks import ( create_dvla_file_contents_for_job, process_job, process_row, - send_sms, - send_email, - persist_letter, + save_sms, + save_email, + save_letter, process_incomplete_job, process_incomplete_jobs, get_template_class, @@ -86,8 +86,9 @@ def _notification_json(template, to, personalisation=None, job_id=None, row_numb def test_should_have_decorated_tasks_functions(): assert process_job.__wrapped__.__name__ == 'process_job' - assert send_sms.__wrapped__.__name__ == 'send_sms' - assert send_email.__wrapped__.__name__ == 'send_email' + assert save_sms.__wrapped__.__name__ == 'save_sms' + assert save_email.__wrapped__.__name__ == 'save_email' + assert save_letter.__wrapped__.__name__ == 'save_letter' @pytest.fixture @@ -98,10 +99,9 @@ def email_job_with_placeholders(notify_db, notify_db_session, sample_email_templ # -------------- process_job tests -------------- # -@freeze_time("2016-01-01 11:09:00.061258") def test_should_process_sms_job(sample_job, mocker): mocker.patch('app.celery.tasks.s3.get_job_from_s3', return_value=load_example_csv('sms')) - mocker.patch('app.celery.tasks.send_sms.apply_async') + mocker.patch('app.celery.tasks.save_sms.apply_async') mocker.patch('app.encryption.encrypt', return_value="something_encrypted") mocker.patch('app.celery.tasks.build_dvla_file') mocker.patch('app.celery.tasks.create_uuid', return_value="uuid") @@ -117,11 +117,10 @@ def test_should_process_sms_job(sample_job, mocker): assert encryption.encrypt.call_args[0][0]['template_version'] == sample_job.template.version assert encryption.encrypt.call_args[0][0]['personalisation'] == {'phonenumber': '+441234123123'} assert encryption.encrypt.call_args[0][0]['row_number'] == 0 - tasks.send_sms.apply_async.assert_called_once_with( + tasks.save_sms.apply_async.assert_called_once_with( (str(sample_job.service_id), "uuid", - "something_encrypted", - "2016-01-01T11:09:00.061258Z"), + "something_encrypted"), queue="database-tasks" ) job = jobs_dao.dao_get_job_by_id(sample_job.id) @@ -223,7 +222,6 @@ def test_should_not_process_job_if_already_pending(notify_db, notify_db_session, tasks.build_dvla_file.assert_not_called() -@freeze_time("2016-01-01 11:09:00.061258") def test_should_process_email_job_if_exactly_on_send_limits(notify_db, notify_db_session, mocker): @@ -232,7 +230,7 @@ def test_should_process_email_job_if_exactly_on_send_limits(notify_db, job = create_sample_job(notify_db, notify_db_session, service=service, template=template, notification_count=10) mocker.patch('app.celery.tasks.s3.get_job_from_s3', return_value=load_example_csv('multiple_email')) - mocker.patch('app.celery.tasks.send_email.apply_async') + mocker.patch('app.celery.tasks.save_email.apply_async') mocker.patch('app.encryption.encrypt', return_value="something_encrypted") mocker.patch('app.celery.tasks.create_uuid', return_value="uuid") @@ -244,20 +242,19 @@ def test_should_process_email_job_if_exactly_on_send_limits(notify_db, ) job = jobs_dao.dao_get_job_by_id(job.id) assert job.job_status == 'finished' - tasks.send_email.apply_async.assert_called_with( + tasks.save_email.apply_async.assert_called_with( ( str(job.service_id), "uuid", "something_encrypted", - "2016-01-01T11:09:00.061258Z" ), queue="database-tasks" ) -def test_should_not_create_send_task_for_empty_file(sample_job, mocker): +def test_should_not_create_save_task_for_empty_file(sample_job, mocker): mocker.patch('app.celery.tasks.s3.get_job_from_s3', return_value=load_example_csv('empty')) - mocker.patch('app.celery.tasks.send_sms.apply_async') + mocker.patch('app.celery.tasks.save_sms.apply_async') process_job(sample_job.id) @@ -267,16 +264,15 @@ def test_should_not_create_send_task_for_empty_file(sample_job, mocker): ) job = jobs_dao.dao_get_job_by_id(sample_job.id) assert job.job_status == 'finished' - assert tasks.send_sms.apply_async.called is False + assert tasks.save_sms.apply_async.called is False -@freeze_time("2016-01-01 11:09:00.061258") def test_should_process_email_job(email_job_with_placeholders, mocker): email_csv = """email_address,name test@test.com,foo """ mocker.patch('app.celery.tasks.s3.get_job_from_s3', return_value=email_csv) - mocker.patch('app.celery.tasks.send_email.apply_async') + mocker.patch('app.celery.tasks.save_email.apply_async') mocker.patch('app.encryption.encrypt', return_value="something_encrypted") mocker.patch('app.celery.tasks.create_uuid', return_value="uuid") @@ -290,12 +286,11 @@ def test_should_process_email_job(email_job_with_placeholders, mocker): assert encryption.encrypt.call_args[0][0]['template'] == str(email_job_with_placeholders.template.id) assert encryption.encrypt.call_args[0][0]['template_version'] == email_job_with_placeholders.template.version assert encryption.encrypt.call_args[0][0]['personalisation'] == {'emailaddress': 'test@test.com', 'name': 'foo'} - tasks.send_email.apply_async.assert_called_once_with( + tasks.save_email.apply_async.assert_called_once_with( ( str(email_job_with_placeholders.service_id), "uuid", "something_encrypted", - "2016-01-01T11:09:00.061258Z" ), queue="database-tasks" ) @@ -309,7 +304,6 @@ def test_should_process_letter_job(sample_letter_job, mocker): A1,A2,A3,A4,A_POST,Alice """ s3_mock = mocker.patch('app.celery.tasks.s3.get_job_from_s3', return_value=csv) - mocker.patch('app.celery.tasks.send_email.apply_async') process_row_mock = mocker.patch('app.celery.tasks.process_row') mocker.patch('app.celery.tasks.create_uuid', return_value="uuid") mocker.patch('app.celery.tasks.build_dvla_file') @@ -344,7 +338,7 @@ def test_should_process_letter_job(sample_letter_job, mocker): def test_should_process_all_sms_job(sample_job_with_placeholdered_template, mocker): mocker.patch('app.celery.tasks.s3.get_job_from_s3', return_value=load_example_csv('multiple_sms')) - mocker.patch('app.celery.tasks.send_sms.apply_async') + mocker.patch('app.celery.tasks.save_sms.apply_async') mocker.patch('app.encryption.encrypt', return_value="something_encrypted") mocker.patch('app.celery.tasks.create_uuid', return_value="uuid") @@ -359,7 +353,7 @@ def test_should_process_all_sms_job(sample_job_with_placeholdered_template, assert encryption.encrypt.call_args[0][0][ 'template_version'] == sample_job_with_placeholdered_template.template.version # noqa assert encryption.encrypt.call_args[0][0]['personalisation'] == {'phonenumber': '+441234123120', 'name': 'chris'} - assert tasks.send_sms.apply_async.call_count == 10 + assert tasks.save_sms.apply_async.call_count == 10 job = jobs_dao.dao_get_job_by_id(sample_job_with_placeholdered_template.id) assert job.job_status == 'finished' @@ -367,14 +361,13 @@ def test_should_process_all_sms_job(sample_job_with_placeholdered_template, # -------------- process_row tests -------------- # -@freeze_time('2001-01-01T12:00:00') @pytest.mark.parametrize('template_type, research_mode, expected_function, expected_queue', [ - (SMS_TYPE, False, 'send_sms', 'database-tasks'), - (SMS_TYPE, True, 'send_sms', 'research-mode-tasks'), - (EMAIL_TYPE, False, 'send_email', 'database-tasks'), - (EMAIL_TYPE, True, 'send_email', 'research-mode-tasks'), - (LETTER_TYPE, False, 'persist_letter', 'database-tasks'), - (LETTER_TYPE, True, 'persist_letter', 'research-mode-tasks'), + (SMS_TYPE, False, 'save_sms', 'database-tasks'), + (SMS_TYPE, True, 'save_sms', 'research-mode-tasks'), + (EMAIL_TYPE, False, 'save_email', 'database-tasks'), + (EMAIL_TYPE, True, 'save_email', 'research-mode-tasks'), + (LETTER_TYPE, False, 'save_letter', 'database-tasks'), + (LETTER_TYPE, True, 'save_letter', 'research-mode-tasks'), ]) def test_process_row_sends_letter_task(template_type, research_mode, expected_function, expected_queue, mocker): mocker.patch('app.celery.tasks.create_uuid', return_value='noti_uuid') @@ -400,11 +393,10 @@ def test_process_row_sends_letter_task(template_type, research_mode, expected_fu 'noti_uuid', # encrypted data encrypt_mock.return_value, - '2001-01-01T12:00:00.000000Z' ), queue=expected_queue ) -# -------- send_sms and send_email tests -------- # +# -------- save_sms and save_email tests -------- # def test_should_send_template_to_correct_sms_task_and_persist(sample_template_with_placeholders, mocker): @@ -413,11 +405,10 @@ def test_should_send_template_to_correct_sms_task_and_persist(sample_template_wi mocked_deliver_sms = mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async') - send_sms( + save_sms( sample_template_with_placeholders.service_id, uuid.uuid4(), encryption.encrypt(notification), - datetime.utcnow().strftime(DATETIME_FORMAT) ) persisted_notification = Notification.query.one() @@ -438,7 +429,7 @@ def test_should_send_template_to_correct_sms_task_and_persist(sample_template_wi ) -def test_should_put_send_sms_task_in_research_mode_queue_if_research_mode_service(notify_db, notify_db_session, mocker): +def test_should_put_save_sms_task_in_research_mode_queue_if_research_mode_service(notify_db, notify_db_session, mocker): service = create_sample_service(notify_db, notify_db_session) service.research_mode = True services_dao.dao_update_service(service) @@ -451,11 +442,10 @@ def test_should_put_send_sms_task_in_research_mode_queue_if_research_mode_servic notification_id = uuid.uuid4() - send_sms( + save_sms( template.service_id, notification_id, encryption.encrypt(notification), - datetime.utcnow().strftime(DATETIME_FORMAT) ) persisted_notification = Notification.query.one() provider_tasks.deliver_sms.apply_async.assert_called_once_with( @@ -465,7 +455,7 @@ def test_should_put_send_sms_task_in_research_mode_queue_if_research_mode_servic assert mocked_deliver_sms.called -def test_should_send_sms_if_restricted_service_and_valid_number(notify_db, notify_db_session, mocker): +def test_should_save_sms_if_restricted_service_and_valid_number(notify_db, notify_db_session, mocker): user = create_user(mobile_number="07700 900890") service = create_sample_service(notify_db, notify_db_session, user=user, restricted=True) template = create_sample_template(notify_db, notify_db_session, service=service) @@ -475,11 +465,10 @@ def test_should_send_sms_if_restricted_service_and_valid_number(notify_db, notif notification_id = uuid.uuid4() encrypt_notification = encryption.encrypt(notification) - send_sms( + save_sms( service.id, notification_id, encrypt_notification, - datetime.utcnow().strftime(DATETIME_FORMAT) ) persisted_notification = Notification.query.one() @@ -499,7 +488,7 @@ def test_should_send_sms_if_restricted_service_and_valid_number(notify_db, notif ) -def test_should_send_sms_if_restricted_service_and_non_team_number_with_test_key(notify_db, +def test_should_save_sms_if_restricted_service_and_non_team_number_with_test_key(notify_db, notify_db_session, mocker): user = create_user(mobile_number="07700 900205") @@ -510,11 +499,10 @@ def test_should_send_sms_if_restricted_service_and_non_team_number_with_test_key mocked_deliver_sms = mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async') notification_id = uuid.uuid4() - send_sms( + save_sms( service.id, notification_id, encryption.encrypt(notification), - datetime.utcnow().strftime(DATETIME_FORMAT), key_type=KEY_TYPE_TEST ) @@ -525,7 +513,7 @@ def test_should_send_sms_if_restricted_service_and_non_team_number_with_test_key ) -def test_should_send_email_if_restricted_service_and_non_team_email_address_with_test_key(notify_db, +def test_should_save_email_if_restricted_service_and_non_team_email_address_with_test_key(notify_db, notify_db_session, mocker): user = create_user() @@ -538,11 +526,10 @@ def test_should_send_email_if_restricted_service_and_non_team_email_address_with mocked_deliver_email = mocker.patch('app.celery.provider_tasks.deliver_email.apply_async') notification_id = uuid.uuid4() - send_email( + save_email( service.id, notification_id, encryption.encrypt(notification), - datetime.utcnow().strftime(DATETIME_FORMAT), key_type=KEY_TYPE_TEST ) @@ -553,7 +540,7 @@ def test_should_send_email_if_restricted_service_and_non_team_email_address_with ) -def test_should_not_send_sms_if_restricted_service_and_invalid_number(notify_db, notify_db_session, mocker): +def test_should_not_save_sms_if_restricted_service_and_invalid_number(notify_db, notify_db_session, mocker): user = create_user(mobile_number="07700 900205") service = create_sample_service(notify_db, notify_db_session, user=user, restricted=True) template = create_sample_template(notify_db, notify_db_session, service=service) @@ -562,17 +549,16 @@ def test_should_not_send_sms_if_restricted_service_and_invalid_number(notify_db, mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async') notification_id = uuid.uuid4() - send_sms( + save_sms( service.id, notification_id, encryption.encrypt(notification), - datetime.utcnow().strftime(DATETIME_FORMAT) ) assert provider_tasks.deliver_sms.apply_async.called is False assert Notification.query.count() == 0 -def test_should_not_send_email_if_restricted_service_and_invalid_email_address(notify_db, notify_db_session, mocker): +def test_should_not_save_email_if_restricted_service_and_invalid_email_address(notify_db, notify_db_session, mocker): user = create_user() service = create_sample_service(notify_db, notify_db_session, user=user, restricted=True) template = create_sample_template( @@ -581,17 +567,16 @@ def test_should_not_send_email_if_restricted_service_and_invalid_email_address(n notification = _notification_json(template, to="test@example.com") notification_id = uuid.uuid4() - send_email( + save_email( service.id, notification_id, encryption.encrypt(notification), - datetime.utcnow().strftime(DATETIME_FORMAT) ) assert Notification.query.count() == 0 -def test_should_put_send_email_task_in_research_mode_queue_if_research_mode_service( +def test_should_put_save_email_task_in_research_mode_queue_if_research_mode_service( notify_db, notify_db_session, mocker ): service = create_sample_service(notify_db, notify_db_session) @@ -606,11 +591,10 @@ def test_should_put_send_email_task_in_research_mode_queue_if_research_mode_serv notification_id = uuid.uuid4() - send_email( + save_email( template.service_id, notification_id, encryption.encrypt(notification), - datetime.utcnow().strftime(DATETIME_FORMAT) ) persisted_notification = Notification.query.one() @@ -631,7 +615,7 @@ def test_should_not_build_dvla_file_in_research_mode_for_letter_job( """ mocker.patch('app.celery.tasks.s3.get_job_from_s3', return_value=csv) mocker.patch('app.celery.tasks.update_job_to_sent_to_dvla.apply_async') - mocker.patch('app.celery.tasks.persist_letter.apply_async') + mocker.patch('app.celery.tasks.save_letter.apply_async') mocker.patch('app.celery.tasks.create_uuid', return_value=fake_uuid) mocker.patch('app.celery.tasks.encryption.encrypt', return_value=test_encrypted_data) mock_dvla_file_task = mocker.patch('app.celery.tasks.build_dvla_file.apply_async') @@ -641,7 +625,6 @@ def test_should_not_build_dvla_file_in_research_mode_for_letter_job( assert not mock_dvla_file_task.called -@freeze_time("2017-08-29 17:30:00") def test_should_update_job_to_sent_to_dvla_in_research_mode_for_letter_job( mocker, sample_letter_job, fake_uuid ): @@ -653,7 +636,7 @@ def test_should_update_job_to_sent_to_dvla_in_research_mode_for_letter_job( """ mocker.patch('app.celery.tasks.s3.get_job_from_s3', return_value=csv) mock_update_job_task = mocker.patch('app.celery.tasks.update_job_to_sent_to_dvla.apply_async') - mocker.patch('app.celery.tasks.persist_letter.apply_async') + mocker.patch('app.celery.tasks.save_letter.apply_async') mocker.patch('app.celery.tasks.create_uuid', return_value=fake_uuid) mocker.patch('app.celery.tasks.encryption.encrypt', return_value=test_encrypted_data) mock_dvla_file_task = mocker.patch('app.celery.tasks.build_dvla_file.apply_async') @@ -662,12 +645,11 @@ def test_should_update_job_to_sent_to_dvla_in_research_mode_for_letter_job( job = jobs_dao.dao_get_job_by_id(sample_letter_job.id) - persist_letter.apply_async.assert_called_once_with( + save_letter.apply_async.assert_called_once_with( ( str(sample_letter_job.service_id), fake_uuid, test_encrypted_data, - datetime.utcnow().strftime(DATETIME_FORMAT) ), queue=QueueNames.RESEARCH_MODE ) @@ -676,7 +658,7 @@ def test_should_update_job_to_sent_to_dvla_in_research_mode_for_letter_job( [str(job.id)], queue=QueueNames.RESEARCH_MODE) -def test_should_send_sms_template_to_and_persist_with_job_id(sample_job, sample_api_key, mocker): +def test_should_save_sms_template_to_and_persist_with_job_id(sample_job, sample_api_key, mocker): notification = _notification_json( sample_job.template, to="+447234123123", @@ -686,11 +668,10 @@ def test_should_send_sms_template_to_and_persist_with_job_id(sample_job, sample_ notification_id = uuid.uuid4() now = datetime.utcnow() - send_sms( + save_sms( sample_job.service.id, notification_id, encryption.encrypt(notification), - now.strftime(DATETIME_FORMAT), api_key_id=str(sample_api_key.id), key_type=KEY_TYPE_NORMAL ) @@ -713,7 +694,7 @@ def test_should_send_sms_template_to_and_persist_with_job_id(sample_job, sample_ ) -def test_should_not_send_email_if_team_key_and_recipient_not_in_team(sample_email_template_with_placeholders, +def test_should_not_save_email_if_team_key_and_recipient_not_in_team(sample_email_template_with_placeholders, sample_team_api_key, mocker): notification = _notification_json( @@ -727,24 +708,20 @@ def test_should_not_send_email_if_team_key_and_recipient_not_in_team(sample_emai team_members = [user.email_address for user in sample_email_template_with_placeholders.service.users] assert "my_email@my_email.com" not in team_members - with freeze_time("2016-01-01 11:09:00.00000"): - now = datetime.utcnow() + save_email( + sample_email_template_with_placeholders.service_id, + notification_id, + encryption.encrypt(notification), + api_key_id=str(sample_team_api_key.id), + key_type=KEY_TYPE_TEAM + ) - send_email( - sample_email_template_with_placeholders.service_id, - notification_id, - encryption.encrypt(notification), - now.strftime(DATETIME_FORMAT), - api_key_id=str(sample_team_api_key.id), - key_type=KEY_TYPE_TEAM - ) - - assert Notification.query.count() == 0 + assert Notification.query.count() == 0 apply_async.not_called() -def test_should_not_send_sms_if_team_key_and_recipient_not_in_team(notify_db, notify_db_session, mocker): +def test_should_not_save_sms_if_team_key_and_recipient_not_in_team(notify_db, notify_db_session, mocker): assert Notification.query.count() == 0 user = create_user(mobile_number="07700 900205") service = create_sample_service(notify_db, notify_db_session, user=user, restricted=True) @@ -757,11 +734,10 @@ def test_should_not_send_sms_if_team_key_and_recipient_not_in_team(notify_db, no mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async') notification_id = uuid.uuid4() - send_sms( + save_sms( service.id, notification_id, encryption.encrypt(notification), - datetime.utcnow().strftime(DATETIME_FORMAT) ) assert provider_tasks.deliver_sms.apply_async.called is False assert Notification.query.count() == 0 @@ -781,11 +757,10 @@ def test_should_use_email_template_and_persist(sample_email_template_with_placeh row_number=1) with freeze_time("2016-01-01 11:10:00.00000"): - send_email( + save_email( sample_email_template_with_placeholders.service_id, notification_id, encryption.encrypt(notification), - now.strftime(DATETIME_FORMAT), api_key_id=str(sample_api_key.id), key_type=sample_api_key.key_type ) @@ -809,7 +784,7 @@ def test_should_use_email_template_and_persist(sample_email_template_with_placeh [str(persisted_notification.id)], queue='send-email-tasks') -def test_send_email_should_use_template_version_from_job_not_latest(sample_email_template, mocker): +def test_save_email_should_use_template_version_from_job_not_latest(sample_email_template, mocker): notification = _notification_json(sample_email_template, 'my_email@my_email.com') version_on_notification = sample_email_template.version # Change the template @@ -820,11 +795,10 @@ def test_send_email_should_use_template_version_from_job_not_latest(sample_email t = dao_get_template_by_id(sample_email_template.id) assert t.version > version_on_notification now = datetime.utcnow() - send_email( + save_email( sample_email_template.service_id, uuid.uuid4(), encryption.encrypt(notification), - now.strftime(DATETIME_FORMAT) ) persisted_notification = Notification.query.one() @@ -847,11 +821,10 @@ def test_should_use_email_template_subject_placeholders(sample_email_template_wi notification_id = uuid.uuid4() now = datetime.utcnow() - send_email( + save_email( sample_email_template_with_placeholders.service_id, notification_id, encryption.encrypt(notification), - now.strftime(DATETIME_FORMAT) ) persisted_notification = Notification.query.one() assert persisted_notification.to == 'my_email@my_email.com' @@ -874,11 +847,10 @@ def test_should_use_email_template_and_persist_without_personalisation(sample_em notification_id = uuid.uuid4() now = datetime.utcnow() - send_email( + save_email( sample_email_template.service_id, notification_id, encryption.encrypt(notification), - now.strftime(DATETIME_FORMAT) ) persisted_notification = Notification.query.one() assert persisted_notification.to == 'my_email@my_email.com' @@ -894,95 +866,87 @@ def test_should_use_email_template_and_persist_without_personalisation(sample_em queue='send-email-tasks') -def test_send_sms_should_go_to_retry_queue_if_database_errors(sample_template, mocker): +def test_save_sms_should_go_to_retry_queue_if_database_errors(sample_template, mocker): notification = _notification_json(sample_template, "+447234123123") expected_exception = SQLAlchemyError() mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async') - mocker.patch('app.celery.tasks.send_sms.retry', side_effect=Retry) + mocker.patch('app.celery.tasks.save_sms.retry', side_effect=Retry) mocker.patch('app.notifications.process_notifications.dao_create_notification', side_effect=expected_exception) - now = datetime.utcnow() notification_id = uuid.uuid4() with pytest.raises(Retry): - send_sms( + save_sms( sample_template.service_id, notification_id, encryption.encrypt(notification), - now.strftime(DATETIME_FORMAT) ) assert provider_tasks.deliver_sms.apply_async.called is False - tasks.send_sms.retry.assert_called_with(exc=expected_exception, queue="retry-tasks") + tasks.save_sms.retry.assert_called_with(exc=expected_exception, queue="retry-tasks") assert Notification.query.count() == 0 -def test_send_email_should_go_to_retry_queue_if_database_errors(sample_email_template, mocker): +def test_save_email_should_go_to_retry_queue_if_database_errors(sample_email_template, mocker): notification = _notification_json(sample_email_template, "test@example.gov.uk") expected_exception = SQLAlchemyError() mocker.patch('app.celery.provider_tasks.deliver_email.apply_async') - mocker.patch('app.celery.tasks.send_email.retry', side_effect=Retry) + mocker.patch('app.celery.tasks.save_email.retry', side_effect=Retry) mocker.patch('app.notifications.process_notifications.dao_create_notification', side_effect=expected_exception) - now = datetime.utcnow() notification_id = uuid.uuid4() with pytest.raises(Retry): - send_email( + save_email( sample_email_template.service_id, notification_id, encryption.encrypt(notification), - now.strftime(DATETIME_FORMAT) ) assert not provider_tasks.deliver_email.apply_async.called - tasks.send_email.retry.assert_called_with(exc=expected_exception, queue="retry-tasks") + tasks.save_email.retry.assert_called_with(exc=expected_exception, queue="retry-tasks") assert Notification.query.count() == 0 -def test_send_email_does_not_send_duplicate_and_does_not_put_in_retry_queue(sample_notification, mocker): +def test_save_email_does_not_send_duplicate_and_does_not_put_in_retry_queue(sample_notification, mocker): json = _notification_json(sample_notification.template, sample_notification.to, job_id=uuid.uuid4(), row_number=1) deliver_email = mocker.patch('app.celery.provider_tasks.deliver_email.apply_async') - retry = mocker.patch('app.celery.tasks.send_email.retry', side_effect=Exception()) - now = datetime.utcnow() + retry = mocker.patch('app.celery.tasks.save_email.retry', side_effect=Exception()) notification_id = sample_notification.id - send_email( + save_email( sample_notification.service_id, notification_id, encryption.encrypt(json), - now.strftime(DATETIME_FORMAT) ) assert Notification.query.count() == 1 assert not deliver_email.called assert not retry.called -def test_send_sms_does_not_send_duplicate_and_does_not_put_in_retry_queue(sample_notification, mocker): +def test_save_sms_does_not_send_duplicate_and_does_not_put_in_retry_queue(sample_notification, mocker): json = _notification_json(sample_notification.template, sample_notification.to, job_id=uuid.uuid4(), row_number=1) deliver_sms = mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async') - retry = mocker.patch('app.celery.tasks.send_sms.retry', side_effect=Exception()) - now = datetime.utcnow() + retry = mocker.patch('app.celery.tasks.save_sms.retry', side_effect=Exception()) notification_id = sample_notification.id - send_sms( + save_sms( sample_notification.service_id, notification_id, encryption.encrypt(json), - now.strftime(DATETIME_FORMAT) ) assert Notification.query.count() == 1 assert not deliver_sms.called assert not retry.called -def test_persist_letter_saves_letter_to_database(sample_letter_job, mocker): +def test_save_letter_saves_letter_to_database(sample_letter_job, mocker): mocker.patch('app.celery.tasks.create_random_identifier', return_value="this-is-random-in-real-life") @@ -1005,11 +969,10 @@ def test_persist_letter_saves_letter_to_database(sample_letter_job, mocker): notification_id = uuid.uuid4() created_at = datetime.utcnow() - persist_letter( + save_letter( sample_letter_job.service_id, notification_id, encryption.encrypt(notification_json), - created_at ) notification_db = Notification.query.one() @@ -1229,7 +1192,7 @@ def test_check_job_status_task_does_not_raise_error(sample_template): def test_process_incomplete_job_sms(mocker, sample_template): mocker.patch('app.celery.tasks.s3.get_job_from_s3', return_value=load_example_csv('multiple_sms')) - send_sms = mocker.patch('app.celery.tasks.send_sms.apply_async') + save_sms = mocker.patch('app.celery.tasks.save_sms.apply_async') job = create_job(template=sample_template, notification_count=10, created_at=datetime.utcnow() - timedelta(hours=2), @@ -1248,13 +1211,13 @@ def test_process_incomplete_job_sms(mocker, sample_template): assert completed_job.job_status == JOB_STATUS_FINISHED - assert send_sms.call_count == 8 # There are 10 in the file and we've added two already + assert save_sms.call_count == 8 # There are 10 in the file and we've added two already def test_process_incomplete_job_with_notifications_all_sent(mocker, sample_template): mocker.patch('app.celery.tasks.s3.get_job_from_s3', return_value=load_example_csv('multiple_sms')) - mock_send_sms = mocker.patch('app.celery.tasks.send_sms.apply_async') + mock_save_sms = mocker.patch('app.celery.tasks.save_sms.apply_async') job = create_job(template=sample_template, notification_count=10, created_at=datetime.utcnow() - timedelta(hours=2), @@ -1281,13 +1244,13 @@ def test_process_incomplete_job_with_notifications_all_sent(mocker, sample_templ assert completed_job.job_status == JOB_STATUS_FINISHED - assert mock_send_sms.call_count == 0 # There are 10 in the file and we've added 10 it should not have been called + assert mock_save_sms.call_count == 0 # There are 10 in the file and we've added 10 it should not have been called def test_process_incomplete_jobs_sms(mocker, sample_template): mocker.patch('app.celery.tasks.s3.get_job_from_s3', return_value=load_example_csv('multiple_sms')) - mock_send_sms = mocker.patch('app.celery.tasks.send_sms.apply_async') + mock_save_sms = mocker.patch('app.celery.tasks.save_sms.apply_async') job = create_job(template=sample_template, notification_count=10, created_at=datetime.utcnow() - timedelta(hours=2), @@ -1324,12 +1287,12 @@ def test_process_incomplete_jobs_sms(mocker, sample_template): assert completed_job2.job_status == JOB_STATUS_FINISHED - assert mock_send_sms.call_count == 12 # There are 20 in total over 2 jobs we've added 8 already + assert mock_save_sms.call_count == 12 # There are 20 in total over 2 jobs we've added 8 already def test_process_incomplete_jobs_no_notifications_added(mocker, sample_template): mocker.patch('app.celery.tasks.s3.get_job_from_s3', return_value=load_example_csv('multiple_sms')) - mock_send_sms = mocker.patch('app.celery.tasks.send_sms.apply_async') + mock_save_sms = mocker.patch('app.celery.tasks.save_sms.apply_async') job = create_job(template=sample_template, notification_count=10, created_at=datetime.utcnow() - timedelta(hours=2), @@ -1345,35 +1308,35 @@ def test_process_incomplete_jobs_no_notifications_added(mocker, sample_template) assert completed_job.job_status == JOB_STATUS_FINISHED - assert mock_send_sms.call_count == 10 # There are 10 in the csv file + assert mock_save_sms.call_count == 10 # There are 10 in the csv file def test_process_incomplete_jobs(mocker): mocker.patch('app.celery.tasks.s3.get_job_from_s3', return_value=load_example_csv('multiple_sms')) - mock_send_sms = mocker.patch('app.celery.tasks.send_sms.apply_async') + mock_save_sms = mocker.patch('app.celery.tasks.save_sms.apply_async') jobs = [] process_incomplete_jobs(jobs) - assert mock_send_sms.call_count == 0 # There are no jobs to process so it will not have been called + assert mock_save_sms.call_count == 0 # There are no jobs to process so it will not have been called def test_process_incomplete_job_no_job_in_database(mocker, fake_uuid): mocker.patch('app.celery.tasks.s3.get_job_from_s3', return_value=load_example_csv('multiple_sms')) - mock_send_sms = mocker.patch('app.celery.tasks.send_sms.apply_async') + mock_save_sms = mocker.patch('app.celery.tasks.save_sms.apply_async') with pytest.raises(expected_exception=Exception) as e: process_incomplete_job(fake_uuid) - assert mock_send_sms.call_count == 0 # There is no job in the db it will not have been called + assert mock_save_sms.call_count == 0 # There is no job in the db it will not have been called def test_process_incomplete_job_email(mocker, sample_email_template): mocker.patch('app.celery.tasks.s3.get_job_from_s3', return_value=load_example_csv('multiple_email')) - mock_email_sender = mocker.patch('app.celery.tasks.send_email.apply_async') + mock_email_saver = mocker.patch('app.celery.tasks.save_email.apply_async') job = create_job(template=sample_email_template, notification_count=10, created_at=datetime.utcnow() - timedelta(hours=2), @@ -1392,13 +1355,13 @@ def test_process_incomplete_job_email(mocker, sample_email_template): assert completed_job.job_status == JOB_STATUS_FINISHED - assert mock_email_sender.call_count == 8 # There are 10 in the file and we've added two already + assert mock_email_saver.call_count == 8 # There are 10 in the file and we've added two already def test_process_incomplete_job_letter(mocker, sample_letter_template): mocker.patch('app.celery.tasks.s3.get_job_from_s3', return_value=load_example_csv('multiple_letter')) - mock_letter_sender = mocker.patch('app.celery.tasks.persist_letter.apply_async') + mock_letter_saver = mocker.patch('app.celery.tasks.save_letter.apply_async') job = create_job(template=sample_letter_template, notification_count=10, created_at=datetime.utcnow() - timedelta(hours=2), @@ -1417,4 +1380,4 @@ def test_process_incomplete_job_letter(mocker, sample_letter_template): assert completed_job.job_status == JOB_STATUS_FINISHED - assert mock_letter_sender.call_count == 8 + assert mock_letter_saver.call_count == 8 diff --git a/tests/app/conftest.py b/tests/app/conftest.py index d904c8ecf..7f3bd3008 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -137,7 +137,7 @@ def sample_service( restricted=False, limit=1000, email_from=None, - permissions=[SMS_TYPE, EMAIL_TYPE], + permissions=None, research_mode=None, free_sms_fragment_limit=250000 ): @@ -166,7 +166,7 @@ def sample_service( if user not in service.users: dao_add_user_to_service(service, user) - if INBOUND_SMS_TYPE in permissions: + if permissions and INBOUND_SMS_TYPE in permissions: create_inbound_number('12345', service_id=service.id) return service diff --git a/tests/app/dao/test_service_sms_sender_dao.py b/tests/app/dao/test_service_sms_sender_dao.py index 5c04629c6..3dcec1469 100644 --- a/tests/app/dao/test_service_sms_sender_dao.py +++ b/tests/app/dao/test_service_sms_sender_dao.py @@ -1,4 +1,12 @@ -from app.dao.service_sms_sender_dao import insert_or_update_service_sms_sender +import uuid + +import pytest +from sqlalchemy.exc import SQLAlchemyError + +from app.dao.service_sms_sender_dao import ( + insert_or_update_service_sms_sender, + dao_add_sms_sender_for_service, + dao_update_service_sms_sender, dao_get_service_sms_senders_by_id, dao_get_sms_senders_by_service_id) from app.models import ServiceSmsSender from tests.app.db import create_service @@ -27,3 +35,113 @@ def test_create_service_inserts_new_service_sms_sender(notify_db_session): assert len(service_sms_senders) == 1 assert service_sms_senders[0].sms_sender == 'new_sms' assert service_sms_senders[0].is_default + + +def test_dao_get_service_sms_senders_id(notify_db_session): + service = create_service(sms_sender='first_sms') + second_sender = dao_add_sms_sender_for_service(service_id=service.id, + sms_sender='second', + is_default=False, + inbound_number_id=None) + result = dao_get_service_sms_senders_by_id(service_id=service.id, + service_sms_sender_id=second_sender.id) + assert result.sms_sender == "second" + assert not result.is_default + + +def test_dao_get_service_sms_senders_id_raise_exception_when_not_found(notify_db_session): + service = create_service() + with pytest.raises(expected_exception=SQLAlchemyError): + dao_get_service_sms_senders_by_id(service_id=service.id, + service_sms_sender_id=uuid.uuid4()) + + +def test_dao_get_sms_senders_by_service_id(notify_db_session): + service = create_service(sms_sender='first_sms') + second_sender = dao_add_sms_sender_for_service(service_id=service.id, + sms_sender='second', + is_default=False, + inbound_number_id=None) + results = dao_get_sms_senders_by_service_id(service_id=service.id) + assert len(results) == 2 + for x in results: + if x.is_default: + assert x.sms_sender == 'first_sms' + else: + assert x == second_sender + + +def test_dao_add_sms_sender_for_service(notify_db_session): + service = create_service(sms_sender="first_sms") + new_sms_sender = dao_add_sms_sender_for_service(service_id=service.id, + sms_sender='new_sms', + is_default=False, + inbound_number_id=None) + + service_sms_senders = ServiceSmsSender.query.order_by(ServiceSmsSender.created_at).all() + assert len(service_sms_senders) == 2 + assert service_sms_senders[0].sms_sender == 'first_sms' + assert service_sms_senders[0].is_default + assert service_sms_senders[1] == new_sms_sender + + +def test_dao_add_sms_sender_for_service_switches_default(notify_db_session): + service = create_service(sms_sender="first_sms") + new_sms_sender = dao_add_sms_sender_for_service(service_id=service.id, + sms_sender='new_sms', + is_default=True, + inbound_number_id=None) + + service_sms_senders = ServiceSmsSender.query.order_by(ServiceSmsSender.created_at).all() + assert len(service_sms_senders) == 2 + assert service_sms_senders[0].sms_sender == 'first_sms' + assert not service_sms_senders[0].is_default + assert service_sms_senders[1] == new_sms_sender + + +def test_dao_update_service_sms_sender(notify_db_session): + service = create_service(sms_sender='first_sms') + service_sms_senders = ServiceSmsSender.query.filter_by(service_id=service.id).all() + assert len(service_sms_senders) == 1 + sms_sender_to_update = service_sms_senders[0] + + dao_update_service_sms_sender(service_id=service.id, + service_sms_sender_id=sms_sender_to_update.id, + is_default=True, + sms_sender="updated") + sms_senders = ServiceSmsSender.query.filter_by(service_id=service.id).all() + assert len(sms_senders) == 1 + assert sms_senders[0].is_default + assert sms_senders[0].sms_sender == 'updated' + assert not sms_senders[0].inbound_number_id + + +def test_dao_update_service_sms_sender_switches_default(notify_db_session): + service = create_service(sms_sender='first_sms') + sms_sender = dao_add_sms_sender_for_service(service_id=service.id, + sms_sender='new_sms', + is_default=False, + inbound_number_id=None) + dao_update_service_sms_sender(service_id=service.id, + service_sms_sender_id=sms_sender.id, + is_default=True, + sms_sender="updated") + sms_senders = ServiceSmsSender.query.filter_by(service_id=service.id).order_by(ServiceSmsSender.created_at).all() + assert len(sms_senders) == 2 + assert sms_senders[0].sms_sender == 'first_sms' + assert not sms_senders[0].is_default + assert sms_senders[1].sms_sender == 'updated' + assert sms_senders[1].is_default + + +def test_dao_update_service_sms_sender_raises_exception_when_no_default_after_update(notify_db_session): + service = create_service(sms_sender='first_sms') + sms_sender = dao_add_sms_sender_for_service(service_id=service.id, + sms_sender='new_sms', + is_default=True, + inbound_number_id=None) + with pytest.raises(expected_exception=Exception) as e: + dao_update_service_sms_sender(service_id=service.id, + service_sms_sender_id=sms_sender.id, + is_default=False, + sms_sender="updated") diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index 1d01c4a3e..a1538eebf 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -57,6 +57,7 @@ from app.models import ( KEY_TYPE_TEST, EMAIL_TYPE, SMS_TYPE, + INTERNATIONAL_SMS_TYPE, LETTER_TYPE, SERVICE_PERMISSION_TYPES ) @@ -260,25 +261,30 @@ def test_create_service_returns_service_with_default_permissions(service_factory service = service_factory.get('testing', email_from='testing') service = dao_fetch_service_by_id(service.id) - assert len(service.permissions) == 2 - assert set([SMS_TYPE, EMAIL_TYPE]) == set(p.permission for p in service.permissions) + _assert_service_permissions(service.permissions, ( + SMS_TYPE, EMAIL_TYPE, INTERNATIONAL_SMS_TYPE, + )) -@pytest.mark.parametrize("permission_to_remove, permission_remaining", - [(SMS_TYPE, EMAIL_TYPE), - (EMAIL_TYPE, SMS_TYPE)]) +@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): + 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 == permission_remaining + _assert_service_permissions(service.permissions, ( + permission_remaining, INTERNATIONAL_SMS_TYPE, + )) 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) + dao_remove_service_permission(service_id=sample_service.id, permission=INTERNATIONAL_SMS_TYPE) service = dao_fetch_service_by_id(sample_service.id) assert len(service.permissions) == 0 @@ -298,14 +304,16 @@ def test_create_service_by_id_adding_and_removing_letter_returns_service_without dao_add_service_permission(service_id=service.id, permission=LETTER_TYPE) service = dao_fetch_service_by_id(service.id) - assert len(service.permissions) == 3 - assert set([SMS_TYPE, EMAIL_TYPE, LETTER_TYPE]) == set([p.permission for p in service.permissions]) + _assert_service_permissions(service.permissions, ( + SMS_TYPE, EMAIL_TYPE, LETTER_TYPE, INTERNATIONAL_SMS_TYPE, + )) dao_remove_service_permission(service_id=service.id, permission=LETTER_TYPE) service = dao_fetch_service_by_id(service.id) - assert len(service.permissions) == 2 - assert set([SMS_TYPE, EMAIL_TYPE]) == set([p.permission for p in service.permissions]) + _assert_service_permissions(service.permissions, ( + SMS_TYPE, EMAIL_TYPE, INTERNATIONAL_SMS_TYPE, + )) def test_create_service_creates_a_history_record_with_current_data(sample_user): @@ -382,7 +390,10 @@ def test_update_service_permission_creates_a_history_record_with_current_data(sa 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] + + _assert_service_permissions(service.permissions, ( + SMS_TYPE, EMAIL_TYPE, INTERNATIONAL_SMS_TYPE, LETTER_TYPE, + )) permission = [p for p in service.permissions if p.permission == 'sms'][0] service.permissions.remove(permission) @@ -393,7 +404,9 @@ def test_update_service_permission_creates_a_history_record_with_current_data(sa 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_service_permissions(service.permissions, ( + EMAIL_TYPE, INTERNATIONAL_SMS_TYPE, LETTER_TYPE, + )) 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 @@ -431,7 +444,7 @@ def test_delete_service_and_associated_objects(notify_db, sample_permission, sample_provider_statistics): # Default service permissions of Email and SMS - assert ServicePermission.query.count() == 2 + assert ServicePermission.query.count() == 3 delete_service_and_all_associated_db_objects(sample_service) assert NotificationStatistics.query.count() == 0 @@ -949,3 +962,9 @@ def test_dao_allocating_inbound_number_shows_on_service(notify_db_session, sampl dao_set_inbound_number_to_service(service.id, inbound_numbers[0]) assert service.inbound_number.number == inbound_numbers[0].number + + +def _assert_service_permissions(service_permissions, expected): + + assert len(service_permissions) == len(expected) + assert set(expected) == set(p.permission for p in service_permissions) diff --git a/tests/app/db.py b/tests/app/db.py index 518f0cbdd..0cdf7cfee 100644 --- a/tests/app/db.py +++ b/tests/app/db.py @@ -63,12 +63,13 @@ def create_service( research_mode=False, active=True, do_create_inbound_number=True, + email_from=None ): service = Service( name=service_name, message_limit=1000, restricted=restricted, - email_from=service_name.lower().replace(' ', '.'), + email_from=email_from if email_from else service_name.lower().replace(' ', '.'), created_by=user or create_user(email='{}@digital.cabinet-office.gov.uk'.format(uuid.uuid4())), sms_sender=sms_sender, ) @@ -355,12 +356,14 @@ def create_reply_to_email( def create_service_sms_sender( service, sms_sender, - is_default=True + is_default=True, + inbound_number_id=None ): data = { 'service_id': service.id, 'sms_sender': sms_sender, 'is_default': is_default, + 'inbound_number_id': inbound_number_id } service_sms_sender = ServiceSmsSender(**data) diff --git a/tests/app/notifications/test_validators.py b/tests/app/notifications/test_validators.py index 387da140e..fecfc4da5 100644 --- a/tests/app/notifications/test_validators.py +++ b/tests/app/notifications/test_validators.py @@ -310,9 +310,14 @@ def test_should_not_rate_limit_if_limiting_is_disabled( @pytest.mark.parametrize('key_type', ['test', 'normal']) -def test_rejects_api_calls_with_international_numbers_if_service_does_not_allow_int_sms(sample_service, key_type): +def test_rejects_api_calls_with_international_numbers_if_service_does_not_allow_int_sms( + key_type, + notify_db, + notify_db_session, +): + service = create_service(notify_db, notify_db_session, permissions=[SMS_TYPE]) with pytest.raises(BadRequestError) as e: - validate_and_format_recipient('20-12-1234-1234', key_type, sample_service, SMS_TYPE) + validate_and_format_recipient('20-12-1234-1234', key_type, service, SMS_TYPE) assert e.value.status_code == 400 assert e.value.message == 'Cannot send to international mobile numbers' assert e.value.fields == [] diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 21046702d..3e1483d1a 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -12,25 +12,29 @@ from app.dao.services_dao import dao_remove_user_from_service from app.dao.templates_dao import dao_redact_template from app.dao.users_dao import save_model_user from app.models import ( - User, Organisation, Service, ServicePermission, Notification, ServiceEmailReplyTo, ServiceLetterContact, + User, Organisation, Service, ServicePermission, Notification, + ServiceEmailReplyTo, ServiceLetterContact, + ServiceSmsSender, InboundNumber, DVLA_ORG_LAND_REGISTRY, KEY_TYPE_NORMAL, KEY_TYPE_TEAM, KEY_TYPE_TEST, - EMAIL_TYPE, SMS_TYPE, LETTER_TYPE, INTERNATIONAL_SMS_TYPE, INBOUND_SMS_TYPE, - ServiceSmsSender) + EMAIL_TYPE, SMS_TYPE, LETTER_TYPE, INTERNATIONAL_SMS_TYPE, INBOUND_SMS_TYPE +) from tests import create_authorization_header from tests.app.conftest import ( - sample_service as create_service, sample_user_service_permission as create_user_service_permission, sample_notification as create_sample_notification, sample_notification_history as create_notification_history, sample_notification_with_job ) from tests.app.db import ( + create_service, create_template, create_service_inbound_api, create_notification, create_reply_to_email, create_letter_contact, + create_inbound_number, + create_service_sms_sender ) from tests.app.db import create_user @@ -203,7 +207,14 @@ def test_get_service_list_has_default_permissions(client, service_factory): 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']]) + assert all( + set( + json['permissions'] + ) == set([ + EMAIL_TYPE, SMS_TYPE, INTERNATIONAL_SMS_TYPE, + ]) + for json in json_resp['data'] + ) def test_get_service_by_id_has_default_service_permissions(client, sample_service): @@ -214,7 +225,11 @@ def test_get_service_by_id_has_default_service_permissions(client, sample_servic ) json_resp = json.loads(resp.get_data(as_text=True)) - assert set(json_resp['data']['permissions']) == set([EMAIL_TYPE, SMS_TYPE]) + assert set( + json_resp['data']['permissions'] + ) == set([ + EMAIL_TYPE, SMS_TYPE, INTERNATIONAL_SMS_TYPE, + ]) def test_get_service_by_id_should_404_if_no_service(notify_api, notify_db): @@ -563,7 +578,7 @@ def test_update_service_flags(client, sample_service): @pytest.fixture(scope='function') def service_with_no_permissions(notify_db, notify_db_session): - return create_service(notify_db, notify_db_session, permissions=[]) + return create_service(service_permissions=[]) def test_update_service_flags_with_service_without_default_service_permissions(client, service_with_no_permissions): @@ -586,8 +601,7 @@ def test_update_service_flags_with_service_without_default_service_permissions(c 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]) + service = create_service(service_permissions=[SMS_TYPE, EMAIL_TYPE, INTERNATIONAL_SMS_TYPE]) assert INTERNATIONAL_SMS_TYPE in [p.permission for p in service.permissions] @@ -781,8 +795,6 @@ def test_should_not_update_service_with_duplicate_name(notify_api, with notify_api.test_client() as client: service_name = "another name" service = create_service( - notify_db, - notify_db_session, service_name=service_name, user=sample_user, email_from='another.name') @@ -814,8 +826,6 @@ def test_should_not_update_service_with_duplicate_email_from(notify_api, email_from = "duplicate.name" service_name = "duplicate name" service = create_service( - notify_db, - notify_db_session, service_name=service_name, user=sample_user, email_from=email_from) @@ -1338,8 +1348,8 @@ def test_set_reply_to_email_for_service(notify_api, sample_service): def test_get_all_notifications_for_service_in_order(notify_api, notify_db, notify_db_session): with notify_api.test_request_context(), notify_api.test_client() as client: - service_1 = create_service(notify_db, notify_db_session, service_name="1", email_from='1') - service_2 = create_service(notify_db, notify_db_session, service_name="2", email_from='2') + service_1 = create_service(service_name="1", email_from='1') + service_2 = create_service(service_name="2", email_from='2') create_sample_notification(notify_db, notify_db_session, service=service_2) @@ -1362,7 +1372,7 @@ def test_get_all_notifications_for_service_in_order(notify_api, notify_db, notif def test_get_notification_for_service_without_uuid(client, notify_db, notify_db_session): - service_1 = create_service(notify_db, notify_db_session, service_name="1", email_from='1') + service_1 = create_service(service_name="1", email_from='1') response = client.get( path='/service/{}/notifications/{}'.format(service_1.id, 'foo'), headers=[create_authorization_header()] @@ -1372,8 +1382,8 @@ def test_get_notification_for_service_without_uuid(client, notify_db, notify_db_ def test_get_notification_for_service(client, notify_db, notify_db_session): - service_1 = create_service(notify_db, notify_db_session, service_name="1", email_from='1') - service_2 = create_service(notify_db, notify_db_session, service_name="2", email_from='2') + service_1 = create_service(service_name="1", email_from='1') + service_2 = create_service(service_name="2", email_from='2') service_1_notifications = [ create_sample_notification(notify_db, notify_db_session, service=service_1), @@ -1669,8 +1679,8 @@ def test_get_services_with_detailed_flag_defaults_to_today(client, mocker): def test_get_detailed_services_groups_by_service(notify_db, notify_db_session): from app.service.rest import get_detailed_services - service_1 = create_service(notify_db, notify_db_session, service_name="1", email_from='1') - service_2 = create_service(notify_db, notify_db_session, service_name="2", email_from='2') + service_1 = create_service(service_name="1", email_from='1') + service_2 = create_service(service_name="2", email_from='2') create_sample_notification(notify_db, notify_db_session, service=service_1, status='created') create_sample_notification(notify_db, notify_db_session, service=service_2, status='created') @@ -1698,8 +1708,8 @@ def test_get_detailed_services_groups_by_service(notify_db, notify_db_session): def test_get_detailed_services_includes_services_with_no_notifications(notify_db, notify_db_session): from app.service.rest import get_detailed_services - service_1 = create_service(notify_db, notify_db_session, service_name="1", email_from='1') - service_2 = create_service(notify_db, notify_db_session, service_name="2", email_from='2') + service_1 = create_service(service_name="1", email_from='1') + service_2 = create_service(service_name="2", email_from='2') create_sample_notification(notify_db, notify_db_session, service=service_1) @@ -1892,11 +1902,7 @@ def test_search_for_notification_by_to_field_return_multiple_matches(client, not def test_update_service_calls_send_notification_as_service_becomes_live(notify_db, notify_db_session, client, mocker): send_notification_mock = mocker.patch('app.service.rest.send_notification_to_service_users') - restricted_service = create_service( - notify_db, - notify_db_session, - restricted=True - ) + restricted_service = create_service(restricted=True) data = { "restricted": False @@ -2213,8 +2219,7 @@ def test_is_service_name_unique_returns_200_if_unique(client): ("something unique", "something.unique") ]) def test_is_service_name_unique_returns_200_and_false(client, notify_db, notify_db_session, name, email_from): - create_service(notify_db=notify_db, notify_db_session=notify_db_session, - service_name='something unique', email_from='something.unique') + create_service(service_name='something unique', email_from='something.unique') response = client.get('/service/unique?name={}&email_from={}'.format(name, email_from), headers=[create_authorization_header()]) assert response.status_code == 200 @@ -2255,7 +2260,7 @@ def test_get_email_reply_to_addresses_when_there_are_no_reply_to_email_addresses def test_get_email_reply_to_addresses_with_one_email_address(client, notify_db, notify_db_session): - service = create_service(notify_db=notify_db, notify_db_session=notify_db_session) + service = create_service() reply_to = create_reply_to_email(service, 'test@mail.com') response = client.get('/service/{}/email-reply-to'.format(service.id), @@ -2271,7 +2276,7 @@ def test_get_email_reply_to_addresses_with_one_email_address(client, notify_db, def test_get_email_reply_to_addresses_with_multiple_email_addresses(client, notify_db, notify_db_session): - service = create_service(notify_db=notify_db, notify_db_session=notify_db_session) + service = create_service() reply_to_a = create_reply_to_email(service, 'test_a@mail.com') reply_to_b = create_reply_to_email(service, 'test_b@mail.com', False) @@ -2389,7 +2394,7 @@ def test_update_service_reply_to_email_address_404s_when_invalid_service_id(clie def test_get_email_reply_to_address(client, notify_db, notify_db_session): - service = create_service(notify_db=notify_db, notify_db_session=notify_db_session) + service = create_service() reply_to = create_reply_to_email(service, 'test_a@mail.com') response = client.get('/service/{}/email-reply-to/{}'.format(service.id, reply_to.id), @@ -2426,7 +2431,7 @@ def test_get_letter_contacts_when_there_are_no_letter_contacts(client, sample_se def test_get_letter_contacts_with_one_letter_contact(client, notify_db, notify_db_session): - service = create_service(notify_db=notify_db, notify_db_session=notify_db_session) + service = create_service() letter_contact = create_letter_contact(service, 'Aberdeen, AB23 1XH') response = client.get('/service/{}/letter-contact'.format(service.id), @@ -2442,7 +2447,7 @@ def test_get_letter_contacts_with_one_letter_contact(client, notify_db, notify_d def test_get_letter_contacts_with_multiple_letter_contacts(client, notify_db, notify_db_session): - service = create_service(notify_db=notify_db, notify_db_session=notify_db_session) + service = create_service() letter_contact_a = create_letter_contact(service, 'Aberdeen, AB23 1XH') letter_contact_b = create_letter_contact(service, 'London, E1 8QS', False) @@ -2469,7 +2474,7 @@ def test_get_letter_contacts_with_multiple_letter_contacts(client, notify_db, no def test_get_letter_contact_by_id(client, notify_db, notify_db_session): - service = create_service(notify_db=notify_db, notify_db_session=notify_db_session) + service = create_service() letter_contact = create_letter_contact(service, 'London, E1 8QS') response = client.get('/service/{}/letter-contact/{}'.format(service.id, letter_contact.id), @@ -2480,7 +2485,7 @@ def test_get_letter_contact_by_id(client, notify_db, notify_db_session): def test_get_letter_contact_return_404_when_invalid_contact_id(client, notify_db, notify_db_session): - service = create_service(notify_db=notify_db, notify_db_session=notify_db_session) + service = create_service() response = client.get('/service/{}/letter-contact/{}'.format(service.id, '93d59f88-4aa1-453c-9900-f61e2fc8a2de'), headers=[('Content-Type', 'application/json'), create_authorization_header()]) @@ -2577,3 +2582,250 @@ def test_update_service_letter_contact_returns_404_when_invalid_service_id(clien result = json.loads(response.get_data(as_text=True)) assert result['result'] == 'error' assert result['message'] == 'No result found' + + +def test_add_service_sms_sender_can_add_multiple_senders(client, notify_db_session): + service = create_service() + data = { + "sms_sender": 'second', + "is_default": False, + } + response = client.post('/service/{}/sms-sender'.format(service.id), + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), create_authorization_header()] + ) + assert response.status_code == 201 + resp_json = json.loads(response.get_data(as_text=True)) + assert resp_json['sms_sender'] == 'second' + assert not resp_json['is_default'] + senders = ServiceSmsSender.query.all() + assert len(senders) == 2 + + +def test_add_service_sms_sender_when_it_is_an_inbound_number(client, notify_db_session): + service = create_service() + inbound_number = create_inbound_number(number='12345') + data = { + "sms_sender": str(inbound_number.id), + "is_default": False, + "inbound_number_id": str(inbound_number.id) + } + response = client.post('/service/{}/sms-sender'.format(service.id), + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), create_authorization_header()] + ) + assert response.status_code == 201 + updated_number = InboundNumber.query.get(inbound_number.id) + assert updated_number.service_id == service.id + resp_json = json.loads(response.get_data(as_text=True)) + assert resp_json['sms_sender'] == inbound_number.number + assert resp_json['inbound_number_id'] == str(inbound_number.id) + assert not resp_json['is_default'] + + +def test_add_service_sms_sender_switches_default(client, notify_db_session): + service = create_service(sms_sender='first') + data = { + "sms_sender": 'second', + "is_default": True, + } + response = client.post('/service/{}/sms-sender'.format(service.id), + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), create_authorization_header()] + ) + assert response.status_code == 201 + resp_json = json.loads(response.get_data(as_text=True)) + assert resp_json['sms_sender'] == 'second' + assert not resp_json['inbound_number_id'] + assert resp_json['is_default'] + sms_senders = ServiceSmsSender.query.filter_by(sms_sender='first').first() + assert not sms_senders.is_default + + +def test_add_service_sms_sender_return_404_when_service_does_not_exist(client): + data = { + "sms_sender": '12345', + "is_default": False + } + response = client.post('/service/{}/sms-sender'.format(uuid.uuid4()), + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), create_authorization_header()] + ) + assert response.status_code == 404 + result = json.loads(response.get_data(as_text=True)) + assert result['result'] == 'error' + assert result['message'] == 'No result found' + + +def test_update_service_sms_sender(client, notify_db_session): + service = create_service() + service_sms_sender = create_service_sms_sender(service=service, sms_sender='1235', is_default=False) + data = { + "sms_sender": 'second', + "is_default": False, + } + response = client.post('/service/{}/sms-sender/{}'.format(service.id, service_sms_sender.id), + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), create_authorization_header()] + ) + assert response.status_code == 200 + resp_json = json.loads(response.get_data(as_text=True)) + assert resp_json['sms_sender'] == 'second' + assert not resp_json['inbound_number_id'] + assert not resp_json['is_default'] + + +def test_update_service_sms_sender_switches_default(client, notify_db_session): + service = create_service(sms_sender='first') + service_sms_sender = create_service_sms_sender(service=service, sms_sender='1235', is_default=False) + data = { + "sms_sender": 'second', + "is_default": True, + } + response = client.post('/service/{}/sms-sender/{}'.format(service.id, service_sms_sender.id), + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), create_authorization_header()] + ) + assert response.status_code == 200 + resp_json = json.loads(response.get_data(as_text=True)) + assert resp_json['sms_sender'] == 'second' + assert not resp_json['inbound_number_id'] + assert resp_json['is_default'] + sms_senders = ServiceSmsSender.query.filter_by(sms_sender='first').first() + assert not sms_senders.is_default + + +def test_update_service_sms_sender_does_not_allow_sender_update_for_inbound_number(client, notify_db_session): + service = create_service() + inbound_number = create_inbound_number('12345', service_id=service.id) + service_sms_sender = create_service_sms_sender(service=service, + sms_sender='1235', + is_default=False, + inbound_number_id=inbound_number.id) + data = { + "sms_sender": 'second', + "is_default": True, + "inbound_number_id": str(inbound_number.id) + } + response = client.post('/service/{}/sms-sender/{}'.format(service.id, service_sms_sender.id), + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), create_authorization_header()] + ) + assert response.status_code == 400 + + +def test_update_service_sms_sender_return_404_when_service_does_not_exist(client): + data = { + "sms_sender": '12345', + "is_default": False + } + response = client.post('/service/{}/sms-sender/{}'.format(uuid.uuid4(), uuid.uuid4()), + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), create_authorization_header()] + ) + assert response.status_code == 404 + result = json.loads(response.get_data(as_text=True)) + assert result['result'] == 'error' + assert result['message'] == 'No result found' + + +def test_get_service_sms_sender_by_id(client, notify_db_session): + service_sms_sender = create_service_sms_sender(service=create_service(), + sms_sender='1235', + is_default=False) + response = client.get('/service/{}/sms-sender/{}'.format(service_sms_sender.service_id, service_sms_sender.id), + headers=[('Content-Type', 'application/json'), create_authorization_header()] + ) + assert response.status_code == 200 + assert json.loads(response.get_data(as_text=True)) == service_sms_sender.serialize() + + +def test_get_service_sms_sender_by_id_returns_404_when_service_does_not_exist(client, notify_db_session): + service_sms_sender = create_service_sms_sender(service=create_service(), + sms_sender='1235', + is_default=False) + response = client.get('/service/{}/sms-sender/{}'.format(uuid.uuid4(), service_sms_sender.id), + headers=[('Content-Type', 'application/json'), create_authorization_header()] + ) + assert response.status_code == 404 + + +def test_get_service_sms_sender_by_id_returns_404_when_sms_sender_does_not_exist(client, notify_db_session): + service_sms_sender = create_service_sms_sender(service=create_service(), + sms_sender='1235', + is_default=False) + response = client.get('/service/{}/sms-sender/{}'.format(service_sms_sender.service_id, uuid.uuid4()), + headers=[('Content-Type', 'application/json'), create_authorization_header()] + ) + assert response.status_code == 404 + + +def test_get_service_sms_senders_for_service(client, notify_db_session): + service_sms_sender = create_service_sms_sender(service=create_service(sms_sender='first'), + sms_sender='second', + is_default=False) + response = client.get('/service/{}/sms-sender'.format(service_sms_sender.service_id), + headers=[('Content-Type', 'application/json'), create_authorization_header()] + ) + assert response.status_code == 200 + json_resp = json.loads(response.get_data(as_text=True)) + assert len(json_resp) == 2 + assert json_resp[0]['is_default'] + assert json_resp[0]['sms_sender'] == 'first' + assert not json_resp[1]['is_default'] + assert json_resp[1]['sms_sender'] == 'second' + + +def test_get_service_sms_senders_for_service_returns_empty_list_when_service_does_not_exist(client): + response = client.get('/service/{}/sms-sender'.format(uuid.uuid4()), + headers=[('Content-Type', 'application/json'), create_authorization_header()] + ) + assert response.status_code == 200 + assert json.loads(response.get_data(as_text=True)) == [] + + +def test_get_platform_stats(client, notify_db_session): + service_1 = create_service(service_name='Service 1') + service_2 = create_service(service_name='Service 2') + sms_template = create_template(service=service_1) + email_template = create_template(service=service_2, template_type=EMAIL_TYPE) + letter_template = create_template(service=service_2, template_type=LETTER_TYPE) + create_notification(template=sms_template, status='sending') + create_notification(template=sms_template, status='delivered') + create_notification(template=sms_template, status='delivered') + create_notification(template=sms_template, status='delivered') + create_notification(template=email_template, status='temporary-failure') + create_notification(template=email_template, status='delivered') + create_notification(template=letter_template, status='sending') + create_notification(template=letter_template, status='sending') + + response = client.get('/service/platform-stats', + headers=[('Content-Type', 'application/json'), create_authorization_header()] + ) + assert response.status_code == 200 + json_resp = json.loads(response.get_data(as_text=True)) + assert json_resp['email'] == {'delivered': 1, 'requested': 2, 'failed': 1} + assert json_resp['letter'] == {'delivered': 0, 'requested': 2, 'failed': 0} + assert json_resp['sms'] == {'delivered': 3, 'requested': 4, 'failed': 0} + + +def test_get_platform_stats_creates_zero_stats(client, notify_db_session): + service_1 = create_service(service_name='Service 1') + service_2 = create_service(service_name='Service 2') + sms_template = create_template(service=service_1) + email_template = create_template(service=service_2, template_type=EMAIL_TYPE) + create_notification(template=sms_template, status='sending') + create_notification(template=sms_template, status='delivered') + create_notification(template=sms_template, status='delivered') + create_notification(template=sms_template, status='delivered') + create_notification(template=email_template, status='temporary-failure') + create_notification(template=email_template, status='delivered') + + response = client.get('/service/platform-stats', + headers=[('Content-Type', 'application/json'), create_authorization_header()] + ) + assert response.status_code == 200 + json_resp = json.loads(response.get_data(as_text=True)) + assert json_resp['email'] == {'failed': 1, 'requested': 2, 'delivered': 1} + assert json_resp['letter'] == {'failed': 0, 'requested': 0, 'delivered': 0} + assert json_resp['sms'] == {'failed': 0, 'requested': 4, 'delivered': 3} diff --git a/tests/app/v2/notifications/test_post_notifications.py b/tests/app/v2/notifications/test_post_notifications.py index 390ccac08..94fea27e5 100644 --- a/tests/app/v2/notifications/test_post_notifications.py +++ b/tests/app/v2/notifications/test_post_notifications.py @@ -328,17 +328,26 @@ def test_returns_a_429_limit_exceeded_if_rate_limit_exceeded( assert not deliver_mock.called -def test_post_sms_notification_returns_400_if_not_allowed_to_send_int_sms(client, sample_service, sample_template): +def test_post_sms_notification_returns_400_if_not_allowed_to_send_int_sms( + client, + notify_db, + notify_db_session, +): + + service = sample_service(notify_db, notify_db_session, permissions=[SMS_TYPE]) + template = create_sample_template(notify_db, notify_db_session, service=service) + data = { 'phone_number': '20-12-1234-1234', - 'template_id': sample_template.id + 'template_id': template.id } - auth_header = create_authorization_header(service_id=sample_service.id) + auth_header = create_authorization_header(service_id=service.id) response = client.post( path='/v2/notifications/sms', data=json.dumps(data), - headers=[('Content-Type', 'application/json'), auth_header]) + headers=[('Content-Type', 'application/json'), auth_header] + ) assert response.status_code == 400 assert response.headers['Content-type'] == 'application/json' @@ -378,18 +387,20 @@ def test_post_sms_notification_returns_400_if_not_allowed_to_send_notification( ] -def test_post_sms_notification_returns_201_if_allowed_to_send_int_sms(notify_db, notify_db_session, client, mocker): - - service = sample_service(notify_db, notify_db_session, permissions=[SMS_TYPE, INTERNATIONAL_SMS_TYPE]) - template = create_sample_template(notify_db, notify_db_session, service=service) +def test_post_sms_notification_returns_201_if_allowed_to_send_int_sms( + sample_service, + sample_template, + client, + mocker, +): mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async') data = { 'phone_number': '20-12-1234-1234', - 'template_id': template.id + 'template_id': sample_template.id } - auth_header = create_authorization_header(service_id=service.id) + auth_header = create_authorization_header(service_id=sample_service.id) response = client.post( path='/v2/notifications/sms',