diff --git a/app/dao/jobs_dao.py b/app/dao/jobs_dao.py index ec7bbe2a6..e6a80832e 100644 --- a/app/dao/jobs_dao.py +++ b/app/dao/jobs_dao.py @@ -1,3 +1,4 @@ +import uuid from datetime import datetime, timedelta from flask import current_app @@ -10,6 +11,7 @@ from app.models import ( JOB_STATUS_SCHEDULED, JOB_STATUS_PENDING, LETTER_TYPE ) +from app.variables import LETTER_TEST_API_FILENAME from app.statsd_decorators import statsd @@ -108,6 +110,8 @@ def dao_get_future_scheduled_job_by_id_and_service_id(job_id, service_id): def dao_create_job(job): + if not job.id: + job.id = uuid.uuid4() job_stats = JobStatistics( job_id=job.id, updated_at=datetime.utcnow() @@ -139,9 +143,18 @@ def dao_get_jobs_older_than_limited_by(job_types, older_than=7, limit_days=2): def dao_get_all_letter_jobs(): - return db.session.query(Job).join(Job.template).filter( - Template.template_type == LETTER_TYPE - ).order_by(desc(Job.created_at)).all() + return db.session.query( + Job + ).join( + Job.template + ).filter( + Template.template_type == LETTER_TYPE, + # test letter jobs (or from research mode services) are created with a different filename, + # exclude them so we don't see them on the send to CSV + Job.original_file_name != LETTER_TEST_API_FILENAME + ).order_by( + desc(Job.created_at) + ).all() @statsd(namespace="dao") diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index 432224653..231ceba59 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -338,7 +338,8 @@ def get_notifications_for_service( filters.append(Notification.created_at < older_than_created_at) if not include_jobs or (key_type and key_type != KEY_TYPE_NORMAL): - filters.append(Notification.job_id.is_(None)) + # we can't say "job_id == None" here, because letters sent via the API still have a job_id :( + filters.append(Notification.api_key_id != None) # noqa if key_type is not None: filters.append(Notification.key_type == key_type) diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index 310bd8f32..20099c009 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -207,14 +207,14 @@ def delete_service_and_all_associated_db_objects(service): _delete_commit(ProviderStatistics.query.filter_by(service=service)) _delete_commit(InvitedUser.query.filter_by(service=service)) _delete_commit(Permission.query.filter_by(service=service)) - _delete_commit(ApiKey.query.filter_by(service=service)) - _delete_commit(ApiKey.get_history_model().query.filter_by(service_id=service.id)) _delete_commit(NotificationHistory.query.filter_by(service=service)) _delete_commit(Notification.query.filter_by(service=service)) _delete_commit(Job.query.filter_by(service=service)) _delete_commit(Template.query.filter_by(service=service)) _delete_commit(TemplateHistory.query.filter_by(service_id=service.id)) _delete_commit(ServicePermission.query.filter_by(service_id=service.id)) + _delete_commit(ApiKey.query.filter_by(service=service)) + _delete_commit(ApiKey.get_history_model().query.filter_by(service_id=service.id)) verify_codes = VerifyCode.query.join(User).filter(User.id.in_([x.id for x in service.users])) list(map(db.session.delete, verify_codes)) diff --git a/app/models.py b/app/models.py index 5ab742b4f..1592557b6 100644 --- a/app/models.py +++ b/app/models.py @@ -619,7 +619,7 @@ class JobStatus(db.Model): class Job(db.Model): __tablename__ = 'jobs' - id = db.Column(UUID(as_uuid=True), primary_key=True) + id = db.Column(UUID(as_uuid=True), primary_key=True, default=uuid.uuid4) original_file_name = db.Column(db.String, nullable=False) service_id = db.Column(UUID(as_uuid=True), db.ForeignKey('services.id'), index=True, unique=False, nullable=False) service = db.relationship('Service', backref=db.backref('jobs', lazy='dynamic')) @@ -654,7 +654,7 @@ class Job(db.Model): unique=False, nullable=True) created_by = db.relationship('User') - created_by_id = db.Column(UUID(as_uuid=True), db.ForeignKey('users.id'), index=True, nullable=False) + created_by_id = db.Column(UUID(as_uuid=True), db.ForeignKey('users.id'), index=True, nullable=True) scheduled_for = db.Column( db.DateTime, index=True, @@ -891,7 +891,7 @@ class Notification(db.Model): @property def subject(self): from app.utils import get_template_instance - if self.notification_type == EMAIL_TYPE: + if self.notification_type != SMS_TYPE: template_object = get_template_instance(self.template.__dict__, self.personalisation) return template_object.subject @@ -971,10 +971,24 @@ class Notification(db.Model): "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(), - "scheduled_for": convert_bst_to_utc(self.scheduled_notification.scheduled_for - ).strftime(DATETIME_FORMAT) if self.scheduled_notification else None + "scheduled_for": ( + convert_bst_to_utc( + self.scheduled_notification.scheduled_for + ).strftime(DATETIME_FORMAT) + if self.scheduled_notification + else None + ) } + if self.notification_type == LETTER_TYPE: + serialized['line_1'] = self.personalisation['address_line_1'] + serialized['line_2'] = self.personalisation.get('address_line_2') + serialized['line_3'] = self.personalisation.get('address_line_3') + serialized['line_4'] = self.personalisation.get('address_line_4') + serialized['line_5'] = self.personalisation.get('address_line_5') + serialized['line_6'] = self.personalisation.get('address_line_6') + serialized['postcode'] = self.personalisation['postcode'] + return serialized diff --git a/app/notifications/process_letter_notifications.py b/app/notifications/process_letter_notifications.py new file mode 100644 index 000000000..332f6ed32 --- /dev/null +++ b/app/notifications/process_letter_notifications.py @@ -0,0 +1,45 @@ +from app import create_random_identifier +from app.models import LETTER_TYPE, JOB_STATUS_READY_TO_SEND, Job +from app.dao.jobs_dao import dao_create_job +from app.notifications.process_notifications import persist_notification +from app.v2.errors import InvalidRequest +from app.variables import LETTER_API_FILENAME + + +def create_letter_api_job(template): + service = template.service + if not service.active: + raise InvalidRequest('Service {} is inactive'.format(service.id), 403) + if template.archived: + raise InvalidRequest('Template {} is deleted'.format(template.id), 400) + + job = Job( + original_file_name=LETTER_API_FILENAME, + service=service, + template=template, + template_version=template.version, + notification_count=1, + job_status=JOB_STATUS_READY_TO_SEND, + created_by=None + ) + dao_create_job(job) + return job + + +def create_letter_notification(letter_data, job, api_key): + notification = persist_notification( + template_id=job.template.id, + template_version=job.template.version, + # we only accept addresses_with_underscores from the API (from CSV we also accept dashes, spaces etc) + recipient=letter_data['personalisation']['address_line_1'], + service=job.service, + personalisation=letter_data['personalisation'], + notification_type=LETTER_TYPE, + api_key_id=api_key.id, + key_type=api_key.key_type, + job_id=job.id, + job_row_number=0, + reference=create_random_identifier(), + client_reference=letter_data.get('reference') + ) + return notification diff --git a/app/notifications/process_notifications.py b/app/notifications/process_notifications.py index 19430c386..e7e8779f2 100644 --- a/app/notifications/process_notifications.py +++ b/app/notifications/process_notifications.py @@ -55,7 +55,7 @@ def persist_notification( created_by_id=None ): notification_created_at = created_at or datetime.utcnow() - if not notification_id and simulated: + if not notification_id: notification_id = uuid.uuid4() notification = Notification( id=notification_id, diff --git a/app/schema_validation/definitions.py b/app/schema_validation/definitions.py index e779c5794..0bf163999 100644 --- a/app/schema_validation/definitions.py +++ b/app/schema_validation/definitions.py @@ -14,7 +14,6 @@ uuid = { personalisation = { "type": "object", - "validationMessage": "should contain key value pairs", "code": "1001", # yet to be implemented "link": "link to our error documentation not yet implemented" } diff --git a/app/utils.py b/app/utils.py index ea30a6df2..3cb69294e 100644 --- a/app/utils.py +++ b/app/utils.py @@ -30,7 +30,7 @@ def url_with_token(data, url, config): def get_template_instance(template, values): from app.models import SMS_TYPE, EMAIL_TYPE, LETTER_TYPE return { - SMS_TYPE: SMSMessageTemplate, EMAIL_TYPE: PlainTextEmailTemplate, LETTER_TYPE: LetterPreviewTemplate + SMS_TYPE: SMSMessageTemplate, EMAIL_TYPE: PlainTextEmailTemplate, LETTER_TYPE: PlainTextEmailTemplate }[template['template_type']](template, values) diff --git a/app/v2/errors.py b/app/v2/errors.py index d38477474..0fa6bddac 100644 --- a/app/v2/errors.py +++ b/app/v2/errors.py @@ -29,10 +29,10 @@ class RateLimitError(InvalidRequest): class BadRequestError(InvalidRequest): - status_code = 400 message = "An error occurred" - def __init__(self, fields=[], message=None): + def __init__(self, fields=[], message=None, status_code=400): + self.status_code = status_code self.fields = fields self.message = message if message else self.message diff --git a/app/v2/notifications/notification_schemas.py b/app/v2/notifications/notification_schemas.py index 7f61f0ee0..fefbf5634 100644 --- a/app/v2/notifications/notification_schemas.py +++ b/app/v2/notifications/notification_schemas.py @@ -228,7 +228,8 @@ post_letter_response = { "content": letter_content, "uri": {"type": "string", "format": "uri"}, "template": template, - "scheduled_for": {"type": ["string", "null"]} + # letters cannot be scheduled + "scheduled_for": {"type": "null"} }, "required": ["id", "content", "uri", "template"] } diff --git a/app/v2/notifications/post_notifications.py b/app/v2/notifications/post_notifications.py index f39f54345..d18bb3b4d 100644 --- a/app/v2/notifications/post_notifications.py +++ b/app/v2/notifications/post_notifications.py @@ -4,12 +4,18 @@ from flask import request, jsonify, current_app, abort from app import api_user, authenticated_service from app.config import QueueNames -from app.models import SMS_TYPE, EMAIL_TYPE, LETTER_TYPE, PRIORITY +from app.dao.jobs_dao import dao_update_job +from app.models import SMS_TYPE, EMAIL_TYPE, LETTER_TYPE, PRIORITY, KEY_TYPE_TEST, KEY_TYPE_TEAM +from app.celery.tasks import build_dvla_file, update_job_to_sent_to_dvla from app.notifications.process_notifications import ( persist_notification, send_notification_to_queue, simulated_recipient, persist_scheduled_notification) +from app.notifications.process_letter_notifications import ( + create_letter_api_job, + create_letter_notification +) from app.notifications.validators import ( validate_and_format_recipient, check_rate_limiting, @@ -18,6 +24,7 @@ from app.notifications.validators import ( validate_template ) from app.schema_validation import validate +from app.v2.errors import BadRequestError from app.v2.notifications import v2_notification_blueprint from app.v2.notifications.notification_schemas import ( post_sms_request, @@ -29,6 +36,7 @@ from app.v2.notifications.create_response import ( create_post_email_response_from_notification, create_post_letter_response_from_notification ) +from app.variables import LETTER_TEST_API_FILENAME @v2_notification_blueprint.route('/', methods=['POST']) @@ -58,10 +66,9 @@ def post_notification(notification_type): if notification_type == LETTER_TYPE: notification = process_letter_notification( - form=form, + letter_data=form, api_key=api_user, template=template, - service=authenticated_service, ) else: notification = process_sms_or_email_notification( @@ -140,10 +147,25 @@ def process_sms_or_email_notification(*, form, notification_type, api_key, templ return notification -def process_letter_notification(*, form, api_key, template, service): - # create job +def process_letter_notification(*, letter_data, api_key, template): + if api_key.key_type == KEY_TYPE_TEAM: + raise BadRequestError(message='Cannot send letters with a team api key', status_code=403) - # create notification + job = create_letter_api_job(template) + notification = create_letter_notification(letter_data, job, api_key) - # trigger build_dvla_file task - raise NotImplementedError + if api_key.service.research_mode or api_key.key_type == KEY_TYPE_TEST: + + # distinguish real API jobs from test jobs by giving the test jobs a different filename + job.original_file_name = LETTER_TEST_API_FILENAME + dao_update_job(job) + + update_job_to_sent_to_dvla.apply_async([str(job.id)], queue=QueueNames.RESEARCH_MODE) + else: + build_dvla_file.apply_async([str(job.id)], queue=QueueNames.JOBS) + + current_app.logger.info("send job {} for api notification {} to build-dvla-file in the process-job queue".format( + job.id, + notification.id + )) + return notification diff --git a/app/variables.py b/app/variables.py new file mode 100644 index 000000000..513e30b96 --- /dev/null +++ b/app/variables.py @@ -0,0 +1,3 @@ +# all jobs for letters created via the api must have this filename +LETTER_API_FILENAME = 'letter submitted via api' +LETTER_TEST_API_FILENAME = 'test letter submitted via api' diff --git a/migrations/versions/0113_job_created_by_nullable.py b/migrations/versions/0113_job_created_by_nullable.py new file mode 100644 index 000000000..c6a391523 --- /dev/null +++ b/migrations/versions/0113_job_created_by_nullable.py @@ -0,0 +1,24 @@ +"""empty message + +Revision ID: 0113_job_created_by_nullable +Revises: 0112_add_start_end_dates +Create Date: 2017-07-27 11:12:34.938086 + +""" + +# revision identifiers, used by Alembic. +revision = '0113_job_created_by_nullable' +down_revision = '0112_add_start_end_dates' + +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +def upgrade(): + op.alter_column('jobs', 'created_by_id', nullable=True) + + +def downgrade(): + # This will error if there are any jobs with no created_by - we'll have to decide how to handle those as and when + # we downgrade + op.alter_column('jobs', 'created_by_id', nullable=False) diff --git a/tests/app/conftest.py b/tests/app/conftest.py index 4fb05d422..30a9d4210 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -39,7 +39,7 @@ from app.dao.invited_user_dao import save_invited_user from app.dao.provider_rates_dao import create_provider_rates from app.clients.sms.firetext import FiretextClient from tests import create_authorization_header -from tests.app.db import create_user, create_template, create_notification +from tests.app.db import create_user, create_template, create_notification, create_api_key @pytest.yield_fixture @@ -255,8 +255,8 @@ def sample_template_without_email_permission(notify_db, notify_db_session): @pytest.fixture -def sample_letter_template(sample_service): - return create_template(sample_service, template_type=LETTER_TYPE) +def sample_letter_template(sample_service_full_permissions): + return create_template(sample_service_full_permissions, template_type=LETTER_TYPE) @pytest.fixture(scope='function') @@ -397,17 +397,18 @@ def sample_email_job(notify_db, @pytest.fixture -def sample_letter_job(sample_service, sample_letter_template): +def sample_letter_job(sample_letter_template): + service = sample_letter_template.service data = { 'id': uuid.uuid4(), - 'service_id': sample_service.id, - 'service': sample_service, + 'service_id': service.id, + 'service': service, 'template_id': sample_letter_template.id, 'template_version': sample_letter_template.version, 'original_file_name': 'some.csv', 'notification_count': 1, 'created_at': datetime.utcnow(), - 'created_by': sample_service.created_by, + 'created_by': service.created_by, } job = Job(**data) dao_create_job(job) @@ -429,7 +430,7 @@ def sample_notification_with_job( sent_at=None, billable_units=1, personalisation=None, - api_key_id=None, + api_key=None, key_type=KEY_TYPE_NORMAL ): if job is None: @@ -448,7 +449,7 @@ def sample_notification_with_job( sent_at=sent_at, billable_units=billable_units, personalisation=personalisation, - api_key_id=api_key_id, + api_key=api_key, key_type=key_type ) @@ -468,7 +469,7 @@ def sample_notification( sent_at=None, billable_units=1, personalisation=None, - api_key_id=None, + api_key=None, key_type=KEY_TYPE_NORMAL, sent_by=None, client_reference=None, @@ -483,6 +484,12 @@ def sample_notification( if template is None: template = sample_template(notify_db, notify_db_session, service=service) + if job is None and api_key is None: + # we didn't specify in test - lets create it + api_key = ApiKey.query.filter(ApiKey.service == template.service, ApiKey.key_type == key_type).first() + if not api_key: + api_key = create_api_key(template.service, key_type=key_type) + notification_id = uuid.uuid4() if to_field: @@ -507,8 +514,9 @@ def sample_notification( 'billable_units': billable_units, 'personalisation': personalisation, 'notification_type': template.template_type, - 'api_key_id': api_key_id, - 'key_type': key_type, + 'api_key': api_key, + 'api_key_id': api_key and api_key.id, + 'key_type': api_key.key_type if api_key else key_type, 'sent_by': sent_by, 'updated_at': created_at if status in NOTIFICATION_STATUS_TYPES_COMPLETED else None, 'client_reference': client_reference, @@ -549,11 +557,12 @@ def sample_letter_notification(sample_letter_template): @pytest.fixture(scope='function') def sample_notification_with_api_key(notify_db, notify_db_session): notification = sample_notification(notify_db, notify_db_session) - notification.api_key_id = sample_api_key( + notification.api_key = sample_api_key( notify_db, notify_db_session, name='Test key' - ).id + ) + notification.api_key_id = notification.api_key.id return notification @@ -1026,7 +1035,7 @@ def admin_request(client): headers=[('Content-Type', 'application/json'), create_authorization_header()] ) json_resp = json.loads(resp.get_data(as_text=True)) - assert resp.status_code == _expected_status + assert resp.status_code == _expected_status, json_resp return json_resp @staticmethod @@ -1036,7 +1045,7 @@ def admin_request(client): headers=[create_authorization_header()] ) json_resp = json.loads(resp.get_data(as_text=True)) - assert resp.status_code == _expected_status + assert resp.status_code == _expected_status, json_resp return json_resp return AdminRequest diff --git a/tests/app/dao/test_notification_dao.py b/tests/app/dao/test_notification_dao.py index 6e50c8718..58680f39d 100644 --- a/tests/app/dao/test_notification_dao.py +++ b/tests/app/dao/test_notification_dao.py @@ -46,7 +46,7 @@ from app.dao.notifications_dao import ( 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 +from tests.app.db import create_notification, create_api_key from tests.app.conftest import ( sample_notification, sample_template, @@ -117,14 +117,14 @@ def test_template_usage_should_ignore_test_keys( notify_db_session, created_at=two_minutes_ago, template=sms, - api_key_id=sample_team_api_key.id, + api_key=sample_team_api_key, key_type=KEY_TYPE_TEAM) sample_notification( notify_db, notify_db_session, created_at=one_minute_ago, template=sms, - api_key_id=sample_test_api_key.id, + api_key=sample_test_api_key, key_type=KEY_TYPE_TEST) results = dao_get_last_template_usage(sms.id) @@ -169,11 +169,11 @@ def test_template_history_should_ignore_test_keys( sms = sample_template(notify_db, notify_db_session) sample_notification( - notify_db, notify_db_session, template=sms, api_key_id=sample_api_key.id, key_type=KEY_TYPE_NORMAL) + notify_db, notify_db_session, template=sms, api_key=sample_api_key, key_type=KEY_TYPE_NORMAL) sample_notification( - notify_db, notify_db_session, template=sms, api_key_id=sample_team_api_key.id, key_type=KEY_TYPE_TEAM) + notify_db, notify_db_session, template=sms, api_key=sample_team_api_key, key_type=KEY_TYPE_TEAM) sample_notification( - notify_db, notify_db_session, template=sms, api_key_id=sample_test_api_key.id, key_type=KEY_TYPE_TEST) + notify_db, notify_db_session, template=sms, api_key=sample_test_api_key, key_type=KEY_TYPE_TEST) sample_notification( notify_db, notify_db_session, template=sms) @@ -836,13 +836,16 @@ def test_get_notification_by_id(notify_db, notify_db_session, sample_template): 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): +def test_get_notifications_by_reference(sample_template): client_reference = 'some-client-ref' assert len(Notification.query.all()) == 0 - sample_notification(notify_db, notify_db_session, client_reference=client_reference) - sample_notification(notify_db, notify_db_session, client_reference=client_reference) - sample_notification(notify_db, notify_db_session, client_reference='other-ref') - all_notifications = get_notifications_for_service(sample_service.id, client_reference=client_reference).items + create_notification(sample_template, client_reference=client_reference) + create_notification(sample_template, client_reference=client_reference) + create_notification(sample_template, client_reference='other-ref') + all_notifications = get_notifications_for_service( + sample_template.service_id, + client_reference=client_reference + ).items assert len(all_notifications) == 2 @@ -1066,22 +1069,22 @@ def test_should_not_delete_notification_history(notify_db, notify_db_session, sa @freeze_time("2016-01-10") -def test_should_limit_notifications_return_by_day_limit_plus_one(notify_db, notify_db_session, sample_service): +def test_should_limit_notifications_return_by_day_limit_plus_one(sample_template): assert len(Notification.query.all()) == 0 # create one notification a day between 1st and 9th for i in range(1, 11): past_date = '2016-01-{0:02d}'.format(i) with freeze_time(past_date): - sample_notification(notify_db, notify_db_session, created_at=datetime.utcnow(), status="failed") + create_notification(sample_template, created_at=datetime.utcnow(), status="failed") all_notifications = Notification.query.all() assert len(all_notifications) == 10 - all_notifications = get_notifications_for_service(sample_service.id, limit_days=10).items + all_notifications = get_notifications_for_service(sample_template.service_id, limit_days=10).items assert len(all_notifications) == 10 - all_notifications = get_notifications_for_service(sample_service.id, limit_days=1).items + all_notifications = get_notifications_for_service(sample_template.service_id, limit_days=1).items assert len(all_notifications) == 2 @@ -1302,42 +1305,20 @@ def test_dao_timeout_notifications_doesnt_affect_letters(sample_letter_template) assert updated == 0 -def test_should_return_notifications_excluding_jobs_by_default(notify_db, notify_db_session, sample_service): - assert len(Notification.query.all()) == 0 +def test_should_return_notifications_excluding_jobs_by_default(sample_template, sample_job, sample_api_key): + with_job = create_notification(sample_template, job=sample_job) + without_job = create_notification(sample_template, api_key=sample_api_key) - job = sample_job(notify_db, notify_db_session) - with_job = sample_notification( - notify_db, notify_db_session, created_at=datetime.utcnow(), status="delivered", job=job - ) - without_job = sample_notification( - notify_db, notify_db_session, created_at=datetime.utcnow(), status="delivered" - ) + include_jobs = get_notifications_for_service(sample_template.service_id, include_jobs=True).items + assert len(include_jobs) == 2 - all_notifications = Notification.query.all() - assert len(all_notifications) == 2 + exclude_jobs_by_default = get_notifications_for_service(sample_template.service_id).items + assert len(exclude_jobs_by_default) == 1 + assert exclude_jobs_by_default[0].id == without_job.id - all_notifications = get_notifications_for_service(sample_service.id).items - assert len(all_notifications) == 1 - assert all_notifications[0].id == without_job.id - - -def test_should_return_notifications_including_jobs(notify_db, notify_db_session, sample_service): - assert len(Notification.query.all()) == 0 - - job = sample_job(notify_db, notify_db_session) - with_job = sample_notification( - notify_db, notify_db_session, created_at=datetime.utcnow(), status="delivered", job=job - ) - - all_notifications = Notification.query.all() - assert len(all_notifications) == 1 - - all_notifications = get_notifications_for_service(sample_service.id).items - assert len(all_notifications) == 0 - - all_notifications = get_notifications_for_service(sample_service.id, limit_days=1, include_jobs=True).items - assert len(all_notifications) == 1 - assert all_notifications[0].id == with_job.id + exclude_jobs_manually = get_notifications_for_service(sample_template.service_id, include_jobs=False).items + assert len(exclude_jobs_manually) == 1 + assert exclude_jobs_manually[0].id == without_job.id def test_get_notifications_created_by_api_or_csv_are_returned_correctly_excluding_test_key_notifications( @@ -1353,15 +1334,15 @@ def test_get_notifications_created_by_api_or_csv_are_returned_correctly_excludin notify_db, notify_db_session, created_at=datetime.utcnow(), job=sample_job ) sample_notification( - notify_db, notify_db_session, created_at=datetime.utcnow(), api_key_id=sample_api_key.id, + notify_db, notify_db_session, created_at=datetime.utcnow(), api_key=sample_api_key, key_type=sample_api_key.key_type ) sample_notification( - notify_db, notify_db_session, created_at=datetime.utcnow(), api_key_id=sample_team_api_key.id, + notify_db, notify_db_session, created_at=datetime.utcnow(), api_key=sample_team_api_key, key_type=sample_team_api_key.key_type ) sample_notification( - notify_db, notify_db_session, created_at=datetime.utcnow(), api_key_id=sample_test_api_key.id, + notify_db, notify_db_session, created_at=datetime.utcnow(), api_key=sample_test_api_key, key_type=sample_test_api_key.key_type ) @@ -1394,15 +1375,15 @@ def test_get_notifications_with_a_live_api_key_type( notify_db, notify_db_session, created_at=datetime.utcnow(), job=sample_job ) sample_notification( - notify_db, notify_db_session, created_at=datetime.utcnow(), api_key_id=sample_api_key.id, + notify_db, notify_db_session, created_at=datetime.utcnow(), api_key=sample_api_key, key_type=sample_api_key.key_type ) sample_notification( - notify_db, notify_db_session, created_at=datetime.utcnow(), api_key_id=sample_team_api_key.id, + notify_db, notify_db_session, created_at=datetime.utcnow(), api_key=sample_team_api_key, key_type=sample_team_api_key.key_type ) sample_notification( - notify_db, notify_db_session, created_at=datetime.utcnow(), api_key_id=sample_test_api_key.id, + notify_db, notify_db_session, created_at=datetime.utcnow(), api_key=sample_test_api_key, key_type=sample_test_api_key.key_type ) @@ -1432,15 +1413,15 @@ def test_get_notifications_with_a_test_api_key_type( notify_db, notify_db_session, created_at=datetime.utcnow(), job=sample_job ) sample_notification( - notify_db, notify_db_session, created_at=datetime.utcnow(), api_key_id=sample_api_key.id, + notify_db, notify_db_session, created_at=datetime.utcnow(), api_key=sample_api_key, key_type=sample_api_key.key_type ) sample_notification( - notify_db, notify_db_session, created_at=datetime.utcnow(), api_key_id=sample_team_api_key.id, + notify_db, notify_db_session, created_at=datetime.utcnow(), api_key=sample_team_api_key, key_type=sample_team_api_key.key_type ) sample_notification( - notify_db, notify_db_session, created_at=datetime.utcnow(), api_key_id=sample_test_api_key.id, + notify_db, notify_db_session, created_at=datetime.utcnow(), api_key=sample_test_api_key, key_type=sample_test_api_key.key_type ) @@ -1467,15 +1448,15 @@ def test_get_notifications_with_a_team_api_key_type( notify_db, notify_db_session, created_at=datetime.utcnow(), job=sample_job ) sample_notification( - notify_db, notify_db_session, created_at=datetime.utcnow(), api_key_id=sample_api_key.id, + notify_db, notify_db_session, created_at=datetime.utcnow(), api_key=sample_api_key, key_type=sample_api_key.key_type ) sample_notification( - notify_db, notify_db_session, created_at=datetime.utcnow(), api_key_id=sample_team_api_key.id, + notify_db, notify_db_session, created_at=datetime.utcnow(), api_key=sample_team_api_key, key_type=sample_team_api_key.key_type ) sample_notification( - notify_db, notify_db_session, created_at=datetime.utcnow(), api_key_id=sample_test_api_key.id, + notify_db, notify_db_session, created_at=datetime.utcnow(), api_key=sample_test_api_key, key_type=sample_test_api_key.key_type ) @@ -1503,15 +1484,15 @@ def test_should_exclude_test_key_notifications_by_default( ) sample_notification( - notify_db, notify_db_session, created_at=datetime.utcnow(), api_key_id=sample_api_key.id, + notify_db, notify_db_session, created_at=datetime.utcnow(), api_key=sample_api_key, key_type=sample_api_key.key_type ) sample_notification( - notify_db, notify_db_session, created_at=datetime.utcnow(), api_key_id=sample_team_api_key.id, + notify_db, notify_db_session, created_at=datetime.utcnow(), api_key=sample_team_api_key, key_type=sample_team_api_key.key_type ) sample_notification( - notify_db, notify_db_session, created_at=datetime.utcnow(), api_key_id=sample_test_api_key.id, + notify_db, notify_db_session, created_at=datetime.utcnow(), api_key=sample_test_api_key, key_type=sample_test_api_key.key_type ) @@ -1784,13 +1765,15 @@ def test_dao_update_notifications_sent_to_dvla(notify_db, notify_db_session, sam assert history.updated_at -def test_dao_update_notifications_sent_to_dvla_does_update_history_if_test_key( - notify_db, notify_db_session, sample_letter_template, sample_api_key): - job = sample_job(notify_db=notify_db, notify_db_session=notify_db_session, template=sample_letter_template) +def test_dao_update_notifications_sent_to_dvla_does_update_history_if_test_key(sample_letter_job): + api_key = create_api_key(sample_letter_job.service, key_type=KEY_TYPE_TEST) notification = create_notification( - template=sample_letter_template, job=job, api_key_id=sample_api_key.id, key_type='test') + sample_letter_job.template, + job=sample_letter_job, + api_key=api_key + ) - updated_count = dao_update_notifications_sent_to_dvla(job_id=job.id, provider='some provider') + updated_count = dao_update_notifications_sent_to_dvla(job_id=sample_letter_job.id, provider='some provider') assert updated_count == 1 updated_notification = Notification.query.get(notification.id) @@ -1798,7 +1781,7 @@ def test_dao_update_notifications_sent_to_dvla_does_update_history_if_test_key( assert updated_notification.sent_by == 'some provider' assert updated_notification.sent_at assert updated_notification.updated_at - assert not NotificationHistory.query.get(notification.id) + assert NotificationHistory.query.count() == 0 def test_dao_get_notifications_by_to_field(sample_template): diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index 38ef273cd..ee4f4f912 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -552,11 +552,11 @@ def test_fetch_stats_counts_should_ignore_team_key( sample_team_api_key ): # two created email, one failed email, and one created sms - create_notification(notify_db, notify_db_session, api_key_id=sample_api_key.id, key_type=sample_api_key.key_type) + create_notification(notify_db, notify_db_session, api_key=sample_api_key, key_type=sample_api_key.key_type) create_notification( - notify_db, notify_db_session, api_key_id=sample_test_api_key.id, key_type=sample_test_api_key.key_type) + notify_db, notify_db_session, api_key=sample_test_api_key, key_type=sample_test_api_key.key_type) create_notification( - notify_db, notify_db_session, api_key_id=sample_team_api_key.id, key_type=sample_team_api_key.key_type) + notify_db, notify_db_session, api_key=sample_team_api_key, key_type=sample_team_api_key.key_type) create_notification( notify_db, notify_db_session) @@ -757,24 +757,17 @@ def test_dao_suspend_service_marks_service_as_inactive_and_expires_api_keys(samp ("8", "4", "2")]) # a date range that starts more than 7 days ago def test_fetch_stats_by_date_range_for_all_services_returns_test_notifications(notify_db, notify_db_session, - sample_api_key, start_delta, end_delta, expected): - result_one = create_notification(notify_db, notify_db_session, created_at=datetime.now(), - api_key_id=sample_api_key.id, key_type='test') - create_notification(notify_db, notify_db_session, created_at=datetime.now() - timedelta(days=2), - api_key_id=sample_api_key.id, key_type='test') - create_notification(notify_db, notify_db_session, created_at=datetime.now() - timedelta(days=3), - api_key_id=sample_api_key.id, key_type='test') - create_notification(notify_db, notify_db_session, created_at=datetime.now() - timedelta(days=4), - api_key_id=sample_api_key.id, key_type='normal') - create_notification(notify_db, notify_db_session, created_at=datetime.now() - timedelta(days=4), - api_key_id=sample_api_key.id, key_type='test') - create_notification(notify_db, notify_db_session, created_at=datetime.now() - timedelta(days=8), - api_key_id=sample_api_key.id, key_type='test') - create_notification(notify_db, notify_db_session, created_at=datetime.now() - timedelta(days=8), - api_key_id=sample_api_key.id, key_type='normal') + create_noti = functools.partial(create_notification, notify_db, notify_db_session) + result_one = create_noti(created_at=datetime.now(), key_type='test') + create_noti(created_at=datetime.now() - timedelta(days=2), key_type='test') + create_noti(created_at=datetime.now() - timedelta(days=3), key_type='test') + create_noti(created_at=datetime.now() - timedelta(days=4), key_type='normal') + create_noti(created_at=datetime.now() - timedelta(days=4), key_type='test') + create_noti(created_at=datetime.now() - timedelta(days=8), key_type='test') + create_noti(created_at=datetime.now() - timedelta(days=8), key_type='normal') start_date = (datetime.utcnow() - timedelta(days=int(start_delta))).date() end_date = (datetime.utcnow() - timedelta(days=int(end_delta))).date() diff --git a/tests/app/db.py b/tests/app/db.py index ace7b7c1c..71bf29bcb 100644 --- a/tests/app/db.py +++ b/tests/app/db.py @@ -5,6 +5,7 @@ from app import db from app.dao.jobs_dao import dao_create_job from app.dao.service_inbound_api_dao import save_service_inbound_api from app.models import ( + ApiKey, Service, User, Template, @@ -50,7 +51,9 @@ def create_service( service_id=None, restricted=False, service_permissions=[EMAIL_TYPE, SMS_TYPE], - sms_sender='testing' + sms_sender='testing', + research_mode=False, + active=True, ): service = Service( name=service_name, @@ -58,9 +61,13 @@ def create_service( restricted=restricted, email_from=service_name.lower().replace(' ', '.'), created_by=user or create_user(), - sms_sender=sms_sender + sms_sender=sms_sender, ) dao_create_service(service, service.created_by, service_id, service_permissions=service_permissions) + + service.active = active + service.research_mode = research_mode + return service @@ -97,7 +104,7 @@ def create_notification( updated_at=None, billable_units=1, personalisation=None, - api_key_id=None, + api_key=None, key_type=KEY_TYPE_NORMAL, sent_by=None, client_reference=None, @@ -114,14 +121,20 @@ def create_notification( sent_at = sent_at or datetime.utcnow() updated_at = updated_at or datetime.utcnow() + if job is None and api_key is None: + # we didn't specify in test - lets create it + api_key = ApiKey.query.filter(ApiKey.service == template.service, ApiKey.key_type == key_type).first() + if not api_key: + api_key = create_api_key(template.service, key_type=key_type) + data = { 'id': uuid.uuid4(), 'to': to_field, - 'job_id': job.id if job else None, + 'job_id': job and job.id, 'job': job, 'service_id': template.service.id, 'service': template.service, - 'template_id': template.id if template else None, + 'template_id': template and template.id, 'template': template, 'template_version': template.version, 'status': status, @@ -131,8 +144,9 @@ def create_notification( 'billable_units': billable_units, 'personalisation': personalisation, 'notification_type': template.template_type, - 'api_key_id': api_key_id, - 'key_type': key_type, + 'api_key': api_key, + 'api_key_id': api_key and api_key.id, + 'key_type': api_key.key_type if api_key else key_type, 'sent_by': sent_by, 'updated_at': updated_at, 'client_reference': client_reference, @@ -247,3 +261,18 @@ def create_rate(start_date, value, notification_type): db.session.add(rate) db.session.commit() return rate + + +def create_api_key(service, key_type=KEY_TYPE_NORMAL): + id_ = uuid.uuid4() + api_key = ApiKey( + service=service, + name='{} api key {}'.format(key_type, id_), + created_by=service.created_by, + key_type=key_type, + id=id_, + secret=uuid.uuid4() + ) + db.session.add(api_key) + db.session.commit() + return api_key diff --git a/tests/app/job/test_rest.py b/tests/app/job/test_rest.py index ca25e0769..7ce45b51e 100644 --- a/tests/app/job/test_rest.py +++ b/tests/app/job/test_rest.py @@ -276,7 +276,6 @@ def test_create_job_returns_400_if_missing_data(notify_api, sample_template, moc assert resp_json['result'] == 'error' assert 'Missing data for required field.' in resp_json['message']['original_file_name'] assert 'Missing data for required field.' in resp_json['message']['notification_count'] - assert 'Missing data for required field.' in resp_json['message']['id'] def test_create_job_returns_404_if_template_does_not_exist(notify_api, sample_service, mocker): diff --git a/tests/app/letters/test_send_letter_jobs.py b/tests/app/letters/test_send_letter_jobs.py index 1d32baf12..b0d34b987 100644 --- a/tests/app/letters/test_send_letter_jobs.py +++ b/tests/app/letters/test_send_letter_jobs.py @@ -2,6 +2,8 @@ import uuid from flask import json +from app.variables import LETTER_TEST_API_FILENAME + from tests import create_authorization_header @@ -41,7 +43,7 @@ def test_send_letter_jobs_throws_validation_error(client, mocker): assert not mock_celery.called -def test_send_letter_jobs_throws_validation_error(client, sample_letter_job): +def test_get_letter_jobs_excludes_non_letter_jobs(client, sample_letter_job, sample_job): auth_header = create_authorization_header() response = client.get( path='/letter-jobs', @@ -53,3 +55,11 @@ def test_send_letter_jobs_throws_validation_error(client, sample_letter_job): assert json_resp['data'][0]['id'] == str(sample_letter_job.id) assert json_resp['data'][0]['service_name']['name'] == sample_letter_job.service.name assert json_resp['data'][0]['job_status'] == 'pending' + + +def test_get_letter_jobs_excludes_test_jobs(admin_request, sample_letter_job): + sample_letter_job.original_file_name = LETTER_TEST_API_FILENAME + + json_resp = admin_request.get('letter-job.get_letter_jobs') + + assert len(json_resp['data']) == 0 diff --git a/tests/app/notifications/rest/test_send_notification.py b/tests/app/notifications/rest/test_send_notification.py index 3822395cd..73eab744a 100644 --- a/tests/app/notifications/rest/test_send_notification.py +++ b/tests/app/notifications/rest/test_send_notification.py @@ -572,128 +572,119 @@ def test_should_not_send_sms_if_team_api_key_and_not_a_service_user(notify_api, ] == json_resp['message']['to'] -def test_should_send_email_if_team_api_key_and_a_service_user(notify_api, sample_email_template, fake_uuid, mocker): - with notify_api.test_request_context(), notify_api.test_client() as client: - mocker.patch('app.celery.provider_tasks.deliver_email.apply_async') - mocker.patch('app.dao.notifications_dao.create_uuid', return_value=fake_uuid) +def test_should_send_email_if_team_api_key_and_a_service_user(client, sample_email_template, fake_uuid, mocker): + mocker.patch('app.celery.provider_tasks.deliver_email.apply_async') + mocker.patch('app.notifications.process_notifications.uuid.uuid4', return_value=fake_uuid) - data = { - 'to': sample_email_template.service.created_by.email_address, - 'template': sample_email_template.id - } - api_key = ApiKey(service=sample_email_template.service, - name='team_key', - created_by=sample_email_template.created_by, - key_type=KEY_TYPE_TEAM) - save_model_api_key(api_key) - auth_header = create_jwt_token(secret=api_key.secret, client_id=str(api_key.service_id)) + data = { + 'to': sample_email_template.service.created_by.email_address, + 'template': sample_email_template.id + } + auth_header = create_authorization_header(service_id=sample_email_template.service_id, key_type=KEY_TYPE_TEAM) - response = client.post( - path='/notifications/email', - data=json.dumps(data), - headers=[('Content-Type', 'application/json'), ('Authorization', 'Bearer {}'.format(auth_header))]) + response = client.post( + path='/notifications/email', + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), auth_header]) - app.celery.provider_tasks.deliver_email.apply_async.assert_called_once_with( - [fake_uuid], - queue='send-email-tasks' - ) - assert response.status_code == 201 + app.celery.provider_tasks.deliver_email.apply_async.assert_called_once_with( + [fake_uuid], + queue='send-email-tasks' + ) + assert response.status_code == 201 @pytest.mark.parametrize('restricted', [True, False]) @pytest.mark.parametrize('limit', [0, 1]) def test_should_send_sms_to_anyone_with_test_key( - notify_api, sample_template, mocker, restricted, limit, fake_uuid + client, sample_template, mocker, restricted, limit, fake_uuid ): - with notify_api.test_request_context(), notify_api.test_client() as client: - mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async') - mocker.patch('app.dao.notifications_dao.create_uuid', return_value=fake_uuid) + mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async') + mocker.patch('app.notifications.process_notifications.uuid.uuid4', return_value=fake_uuid) - data = { - 'to': '07811111111', - 'template': sample_template.id - } - sample_template.service.restricted = restricted - sample_template.service.message_limit = limit - api_key = ApiKey( - service=sample_template.service, - name='test_key', - created_by=sample_template.created_by, - key_type=KEY_TYPE_TEST - ) - save_model_api_key(api_key) - auth_header = create_jwt_token(secret=api_key.secret, client_id=str(api_key.service_id)) + data = { + 'to': '07811111111', + 'template': sample_template.id + } + sample_template.service.restricted = restricted + sample_template.service.message_limit = limit + api_key = ApiKey( + service=sample_template.service, + name='test_key', + created_by=sample_template.created_by, + key_type=KEY_TYPE_TEST + ) + save_model_api_key(api_key) + auth_header = create_jwt_token(secret=api_key.secret, client_id=str(api_key.service_id)) - response = client.post( - path='/notifications/sms', - data=json.dumps(data), - headers=[('Content-Type', 'application/json'), ('Authorization', 'Bearer {}'.format(auth_header))] - ) - app.celery.provider_tasks.deliver_sms.apply_async.assert_called_once_with( - [fake_uuid], queue='research-mode-tasks' - ) - assert response.status_code == 201 + response = client.post( + path='/notifications/sms', + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), ('Authorization', 'Bearer {}'.format(auth_header))] + ) + app.celery.provider_tasks.deliver_sms.apply_async.assert_called_once_with( + [fake_uuid], queue='research-mode-tasks' + ) + assert response.status_code == 201 @pytest.mark.parametrize('restricted', [True, False]) @pytest.mark.parametrize('limit', [0, 1]) def test_should_send_email_to_anyone_with_test_key( - notify_api, sample_email_template, mocker, restricted, limit, fake_uuid + client, sample_email_template, mocker, restricted, limit, fake_uuid ): - with notify_api.test_request_context(), notify_api.test_client() as client: - mocker.patch('app.celery.provider_tasks.deliver_email.apply_async') - mocker.patch('app.dao.notifications_dao.create_uuid', return_value=fake_uuid) + mocker.patch('app.celery.provider_tasks.deliver_email.apply_async') + mocker.patch('app.notifications.process_notifications.uuid.uuid4', return_value=fake_uuid) - data = { - 'to': 'anyone123@example.com', - 'template': sample_email_template.id - } - sample_email_template.service.restricted = restricted - sample_email_template.service.message_limit = limit - api_key = ApiKey( - service=sample_email_template.service, - name='test_key', - created_by=sample_email_template.created_by, - key_type=KEY_TYPE_TEST - ) - save_model_api_key(api_key) - auth_header = create_jwt_token(secret=api_key.secret, client_id=str(api_key.service_id)) + data = { + 'to': 'anyone123@example.com', + 'template': sample_email_template.id + } + sample_email_template.service.restricted = restricted + sample_email_template.service.message_limit = limit + api_key = ApiKey( + service=sample_email_template.service, + name='test_key', + created_by=sample_email_template.created_by, + key_type=KEY_TYPE_TEST + ) + save_model_api_key(api_key) + auth_header = create_jwt_token(secret=api_key.secret, client_id=str(api_key.service_id)) - response = client.post( - path='/notifications/email', - data=json.dumps(data), - headers=[('Content-Type', 'application/json'), ('Authorization', 'Bearer {}'.format(auth_header))] - ) + response = client.post( + path='/notifications/email', + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), ('Authorization', 'Bearer {}'.format(auth_header))] + ) - app.celery.provider_tasks.deliver_email.apply_async.assert_called_once_with( - [fake_uuid], queue='research-mode-tasks' - ) - assert response.status_code == 201 + app.celery.provider_tasks.deliver_email.apply_async.assert_called_once_with( + [fake_uuid], queue='research-mode-tasks' + ) + assert response.status_code == 201 -def test_should_send_sms_if_team_api_key_and_a_service_user(notify_api, sample_template, fake_uuid, mocker): - with notify_api.test_request_context(), notify_api.test_client() as client: - mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async') - mocker.patch('app.dao.notifications_dao.create_uuid', return_value=fake_uuid) +def test_should_send_sms_if_team_api_key_and_a_service_user(client, sample_template, fake_uuid, mocker): + mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async') + mocker.patch('app.notifications.process_notifications.uuid.uuid4', return_value=fake_uuid) - data = { - 'to': sample_template.service.created_by.mobile_number, - 'template': sample_template.id - } - api_key = ApiKey(service=sample_template.service, - name='team_key', - created_by=sample_template.created_by, - key_type=KEY_TYPE_TEAM) - save_model_api_key(api_key) - auth_header = create_jwt_token(secret=api_key.secret, client_id=str(api_key.service_id)) + data = { + 'to': sample_template.service.created_by.mobile_number, + 'template': sample_template.id + } + api_key = ApiKey(service=sample_template.service, + name='team_key', + created_by=sample_template.created_by, + key_type=KEY_TYPE_TEAM) + save_model_api_key(api_key) + auth_header = create_jwt_token(secret=api_key.secret, client_id=str(api_key.service_id)) - response = client.post( - path='/notifications/sms', - data=json.dumps(data), - headers=[('Content-Type', 'application/json'), ('Authorization', 'Bearer {}'.format(auth_header))]) + response = client.post( + path='/notifications/sms', + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), ('Authorization', 'Bearer {}'.format(auth_header))]) - app.celery.provider_tasks.deliver_sms.apply_async.assert_called_once_with([fake_uuid], queue='send-sms-tasks') - assert response.status_code == 201 + app.celery.provider_tasks.deliver_sms.apply_async.assert_called_once_with([fake_uuid], queue='send-sms-tasks') + assert response.status_code == 201 @pytest.mark.parametrize('template_type,queue_name', [ @@ -710,7 +701,8 @@ def test_should_persist_notification( queue_name ): mocked = mocker.patch('app.celery.provider_tasks.deliver_{}.apply_async'.format(template_type)) - mocker.patch('app.dao.notifications_dao.create_uuid', return_value=fake_uuid) + mocker.patch('app.notifications.process_notifications.uuid.uuid4', return_value=fake_uuid) + template = sample_template if template_type == SMS_TYPE else sample_email_template to = sample_template.service.created_by.mobile_number if template_type == SMS_TYPE \ else sample_email_template.service.created_by.email_address @@ -757,7 +749,8 @@ def test_should_delete_notification_and_return_error_if_sqs_fails( 'app.celery.provider_tasks.deliver_{}.apply_async'.format(template_type), side_effect=Exception("failed to talk to SQS") ) - mocker.patch('app.dao.notifications_dao.create_uuid', return_value=fake_uuid) + mocker.patch('app.notifications.process_notifications.uuid.uuid4', return_value=fake_uuid) + template = sample_template if template_type == SMS_TYPE else sample_email_template to = sample_template.service.created_by.mobile_number if template_type == SMS_TYPE \ else sample_email_template.service.created_by.email_address diff --git a/tests/app/notifications/test_process_letter_notifications.py b/tests/app/notifications/test_process_letter_notifications.py new file mode 100644 index 000000000..8d964b706 --- /dev/null +++ b/tests/app/notifications/test_process_letter_notifications.py @@ -0,0 +1,84 @@ +import pytest + +from app.dao.services_dao import dao_archive_service +from app.models import Job +from app.models import JOB_STATUS_READY_TO_SEND +from app.models import LETTER_TYPE +from app.models import Notification +from app.notifications.process_letter_notifications import create_letter_api_job +from app.notifications.process_letter_notifications import create_letter_notification +from app.v2.errors import InvalidRequest +from app.variables import LETTER_API_FILENAME + +from tests.app.db import create_service +from tests.app.db import create_template + + +def test_create_job_rejects_inactive_service(notify_db_session): + service = create_service() + template = create_template(service, template_type=LETTER_TYPE) + dao_archive_service(service.id) + + with pytest.raises(InvalidRequest) as exc_info: + create_letter_api_job(template) + + assert exc_info.value.message == 'Service {} is inactive'.format(service.id) + + +def test_create_job_rejects_archived_template(sample_letter_template): + sample_letter_template.archived = True + + with pytest.raises(InvalidRequest) as exc_info: + create_letter_api_job(sample_letter_template) + + assert exc_info.value.message == 'Template {} is deleted'.format(sample_letter_template.id) + + +def test_create_job_creates_job(sample_letter_template): + job = create_letter_api_job(sample_letter_template) + + assert job == Job.query.one() + assert job.original_file_name == LETTER_API_FILENAME + assert job.service == sample_letter_template.service + assert job.template_id == sample_letter_template.id + assert job.template_version == sample_letter_template.version + assert job.notification_count == 1 + assert job.job_status == JOB_STATUS_READY_TO_SEND + assert job.created_by is None + + +def test_create_letter_notification_creates_notification(sample_letter_job, sample_api_key): + data = { + 'personalisation': { + 'address_line_1': 'The Queen', + 'address_line_2': 'Buckingham Palace', + 'postcode': 'SW1 1AA', + } + } + + notification = create_letter_notification(data, sample_letter_job, sample_api_key) + + assert notification == Notification.query.one() + assert notification.job == sample_letter_job + assert notification.template == sample_letter_job.template + assert notification.api_key == sample_api_key + assert notification.notification_type == LETTER_TYPE + assert notification.key_type == sample_api_key.key_type + assert notification.job_row_number == 0 + assert notification.reference is not None + assert notification.client_reference is None + + +def test_create_letter_notification_sets_reference(sample_letter_job, sample_api_key): + data = { + 'personalisation': { + 'address_line_1': 'The Queen', + 'address_line_2': 'Buckingham Palace', + 'postcode': 'SW1 1AA', + }, + 'reference': 'foo' + } + + notification = create_letter_notification(data, sample_letter_job, sample_api_key) + + assert notification.client_reference == 'foo' diff --git a/tests/app/notifications/test_rest.py b/tests/app/notifications/test_rest.py index 811fc1290..6a0b53925 100644 --- a/tests/app/notifications/test_rest.py +++ b/tests/app/notifications/test_rest.py @@ -9,51 +9,53 @@ from app.dao.notifications_dao import dao_update_notification from app.dao.api_key_dao import save_model_api_key from app.dao.templates_dao import dao_update_template from app.models import ApiKey, KEY_TYPE_NORMAL, KEY_TYPE_TEAM, KEY_TYPE_TEST + from tests import create_authorization_header from tests.app.conftest import sample_notification as create_sample_notification +from tests.app.db import create_notification, create_api_key @pytest.mark.parametrize('type', ('email', 'sms')) def test_get_notification_by_id(client, sample_notification, sample_email_notification, type): - if type == 'email': - notification_to_get = sample_email_notification - if type == 'sms': - notification_to_get = sample_notification + if type == 'email': + notification_to_get = sample_email_notification + if type == 'sms': + notification_to_get = sample_notification - auth_header = create_authorization_header(service_id=notification_to_get.service_id) - response = client.get( - '/notifications/{}'.format(notification_to_get.id), - headers=[auth_header]) + auth_header = create_authorization_header(service_id=notification_to_get.service_id) + response = client.get( + '/notifications/{}'.format(notification_to_get.id), + headers=[auth_header]) - assert response.status_code == 200 - notification = json.loads(response.get_data(as_text=True))['data']['notification'] - assert notification['status'] == 'created' - assert notification['template'] == { - 'id': str(notification_to_get.template.id), - 'name': notification_to_get.template.name, - 'template_type': notification_to_get.template.template_type, - 'version': 1 - } - assert notification['to'] == notification_to_get.to - assert notification['service'] == str(notification_to_get.service_id) - assert notification['body'] == notification_to_get.template.content - assert notification.get('subject', None) == notification_to_get.subject + assert response.status_code == 200 + notification = json.loads(response.get_data(as_text=True))['data']['notification'] + assert notification['status'] == 'created' + assert notification['template'] == { + 'id': str(notification_to_get.template.id), + 'name': notification_to_get.template.name, + 'template_type': notification_to_get.template.template_type, + 'version': 1 + } + assert notification['to'] == notification_to_get.to + assert notification['service'] == str(notification_to_get.service_id) + assert notification['body'] == notification_to_get.template.content + assert notification.get('subject', None) == notification_to_get.subject @pytest.mark.parametrize("id", ["1234-badly-formatted-id-7890", "0"]) @pytest.mark.parametrize('type', ('email', 'sms')) def test_get_notification_by_invalid_id(client, sample_notification, sample_email_notification, id, type): - if type == 'email': - notification_to_get = sample_email_notification - if type == 'sms': - notification_to_get = sample_notification - auth_header = create_authorization_header(service_id=notification_to_get.service_id) + if type == 'email': + notification_to_get = sample_email_notification + if type == 'sms': + notification_to_get = sample_notification + auth_header = create_authorization_header(service_id=notification_to_get.service_id) - response = client.get( - '/notifications/{}'.format(id), - headers=[auth_header]) + response = client.get( + '/notifications/{}'.format(id), + headers=[auth_header]) - assert response.status_code == 405 + assert response.status_code == 405 def test_get_notifications_empty_result(client, sample_api_key): @@ -156,7 +158,7 @@ def test_normal_api_key_returns_notifications_created_from_jobs_and_from_api( api_notification = create_sample_notification( notify_db, notify_db_session, - api_key_id=sample_api_key.id + api_key=sample_api_key ) api_notification.job = None @@ -200,19 +202,19 @@ def test_get_all_notifications_only_returns_notifications_of_matching_type( normal_notification = create_sample_notification( notify_db, notify_db_session, - api_key_id=normal_api_key.id, + api_key=normal_api_key, key_type=KEY_TYPE_NORMAL ) team_notification = create_sample_notification( notify_db, notify_db_session, - api_key_id=team_api_key.id, + api_key=team_api_key, key_type=KEY_TYPE_TEAM ) test_notification = create_sample_notification( notify_db, notify_db_session, - api_key_id=test_api_key.id, + api_key=test_api_key, key_type=KEY_TYPE_TEST ) @@ -236,54 +238,18 @@ def test_get_all_notifications_only_returns_notifications_of_matching_type( @pytest.mark.parametrize('key_type', [KEY_TYPE_NORMAL, KEY_TYPE_TEAM, KEY_TYPE_TEST]) def test_no_api_keys_return_job_notifications_by_default( client, - notify_db, - notify_db_session, - sample_service, + sample_template, sample_job, key_type ): - team_api_key = ApiKey(service=sample_service, - name='team_api_key', - created_by=sample_service.created_by, - key_type=KEY_TYPE_TEAM) - save_model_api_key(team_api_key) + team_api_key = create_api_key(sample_template.service, KEY_TYPE_TEAM) + normal_api_key = create_api_key(sample_template.service, KEY_TYPE_NORMAL) + test_api_key = create_api_key(sample_template.service, KEY_TYPE_TEST) - normal_api_key = ApiKey(service=sample_service, - name='normal_api_key', - created_by=sample_service.created_by, - key_type=KEY_TYPE_NORMAL) - save_model_api_key(normal_api_key) - - test_api_key = ApiKey(service=sample_service, - name='test_api_key', - created_by=sample_service.created_by, - key_type=KEY_TYPE_TEST) - save_model_api_key(test_api_key) - - job_notification = create_sample_notification( - notify_db, - notify_db_session, - api_key_id=normal_api_key.id, - job=sample_job - ) - normal_notification = create_sample_notification( - notify_db, - notify_db_session, - api_key_id=normal_api_key.id, - key_type=KEY_TYPE_NORMAL - ) - team_notification = create_sample_notification( - notify_db, - notify_db_session, - api_key_id=team_api_key.id, - key_type=KEY_TYPE_TEAM - ) - test_notification = create_sample_notification( - notify_db, - notify_db_session, - api_key_id=test_api_key.id, - key_type=KEY_TYPE_TEST - ) + job_notification = create_notification(sample_template, job=sample_job) + normal_notification = create_notification(sample_template, api_key=normal_api_key) + team_notification = create_notification(sample_template, api_key=team_api_key) + test_notification = create_notification(sample_template, api_key=test_api_key) notification_objs = { KEY_TYPE_NORMAL: normal_notification, @@ -336,25 +302,24 @@ def test_only_normal_api_keys_can_return_job_notifications( job_notification = create_sample_notification( notify_db, notify_db_session, - api_key_id=normal_api_key.id, job=sample_job ) normal_notification = create_sample_notification( notify_db, notify_db_session, - api_key_id=normal_api_key.id, + api_key=normal_api_key, key_type=KEY_TYPE_NORMAL ) team_notification = create_sample_notification( notify_db, notify_db_session, - api_key_id=team_api_key.id, + api_key=team_api_key, key_type=KEY_TYPE_TEAM ) test_notification = create_sample_notification( notify_db, notify_db_session, - api_key_id=test_api_key.id, + api_key=test_api_key, key_type=KEY_TYPE_TEST ) @@ -505,20 +470,10 @@ def test_filter_by_template_type(client, notify_db, notify_db_session, sample_te def test_filter_by_multiple_template_types(client, - notify_db, - notify_db_session, sample_template, sample_email_template): - notification_1 = create_sample_notification( - notify_db, - notify_db_session, - service=sample_email_template.service, - template=sample_template) - notification_2 = create_sample_notification( - notify_db, - notify_db_session, - service=sample_email_template.service, - template=sample_email_template) + notification_1 = create_notification(sample_template) + notification_2 = create_notification(sample_email_template) auth_header = create_authorization_header(service_id=sample_email_template.service_id) @@ -529,23 +484,12 @@ def test_filter_by_multiple_template_types(client, assert response.status_code == 200 notifications = json.loads(response.get_data(as_text=True)) assert len(notifications['notifications']) == 2 - set(['sms', 'email']) == set( - [x['template']['template_type'] for x in notifications['notifications']]) + assert {'sms', 'email'} == set(x['template']['template_type'] for x in notifications['notifications']) -def test_filter_by_status(client, notify_db, notify_db_session, sample_email_template): - notification_1 = create_sample_notification( - notify_db, - notify_db_session, - service=sample_email_template.service, - template=sample_email_template, - status="delivered") - - notification_2 = create_sample_notification( - notify_db, - notify_db_session, - service=sample_email_template.service, - template=sample_email_template) +def test_filter_by_status(client, sample_email_template): + notification_1 = create_notification(sample_email_template, status="delivered") + notification_2 = create_notification(sample_email_template) auth_header = create_authorization_header(service_id=sample_email_template.service_id) @@ -559,58 +503,27 @@ def test_filter_by_status(client, notify_db, notify_db_session, sample_email_tem assert response.status_code == 200 -def test_filter_by_multiple_statuss(client, - notify_db, - notify_db_session, - sample_email_template): - notification_1 = create_sample_notification( - notify_db, - notify_db_session, - service=sample_email_template.service, - template=sample_email_template, - status="delivered") - - notification_2 = create_sample_notification( - notify_db, - notify_db_session, - service=sample_email_template.service, - template=sample_email_template, - status='sending') +def test_filter_by_multiple_statuses(client, sample_email_template): + notification_1 = create_notification(sample_email_template, status="delivered") + notification_2 = create_notification(sample_email_template, status='sending') auth_header = create_authorization_header(service_id=sample_email_template.service_id) response = client.get( '/notifications?status=delivered&status=sending', - headers=[auth_header]) + headers=[auth_header] + ) assert response.status_code == 200 notifications = json.loads(response.get_data(as_text=True)) assert len(notifications['notifications']) == 2 - set(['delivered', 'sending']) == set( - [x['status'] for x in notifications['notifications']]) + assert {'delivered', 'sending'} == set(x['status'] for x in notifications['notifications']) -def test_filter_by_status_and_template_type(client, - notify_db, - notify_db_session, - sample_template, - sample_email_template): - notification_1 = create_sample_notification( - notify_db, - notify_db_session, - service=sample_email_template.service, - template=sample_template) - notification_2 = create_sample_notification( - notify_db, - notify_db_session, - service=sample_email_template.service, - template=sample_email_template) - notification_3 = create_sample_notification( - notify_db, - notify_db_session, - service=sample_email_template.service, - template=sample_email_template, - status="delivered") +def test_filter_by_status_and_template_type(client, sample_template, sample_email_template): + notification_1 = create_notification(sample_template) + notification_2 = create_notification(sample_email_template) + notification_3 = create_notification(sample_email_template, status="delivered") auth_header = create_authorization_header(service_id=sample_email_template.service_id) @@ -625,15 +538,9 @@ def test_filter_by_status_and_template_type(client, assert notifications['notifications'][0]['status'] == 'delivered' -def test_get_notification_by_id_returns_merged_template_content(notify_db, - notify_db_session, - client, - sample_template_with_placeholders): +def test_get_notification_by_id_returns_merged_template_content(client, sample_template_with_placeholders): - sample_notification = create_sample_notification(notify_db, - notify_db_session, - template=sample_template_with_placeholders, - personalisation={"name": "world"}) + sample_notification = create_notification(sample_template_with_placeholders, personalisation={"name": "world"}) auth_header = create_authorization_header(service_id=sample_notification.service_id) @@ -649,15 +556,13 @@ def test_get_notification_by_id_returns_merged_template_content(notify_db, def test_get_notification_by_id_returns_merged_template_content_for_email( - notify_db, - notify_db_session, client, sample_email_template_with_placeholders ): - sample_notification = create_sample_notification(notify_db, - notify_db_session, - template=sample_email_template_with_placeholders, - personalisation={"name": "world"}) + sample_notification = create_notification( + sample_email_template_with_placeholders, + personalisation={"name": "world"} + ) auth_header = create_authorization_header(service_id=sample_notification.service_id) response = client.get( diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 7b0478916..9df09cfa1 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -1362,24 +1362,20 @@ def test_get_all_notifications_for_service_including_ones_made_by_jobs( def test_get_only_api_created_notifications_for_service( - client, - notify_db, - notify_db_session, - sample_service + admin_request, + sample_job, + sample_template ): - with_job = sample_notification_with_job(notify_db, notify_db_session, service=sample_service) - without_job = create_sample_notification(notify_db, notify_db_session, service=sample_service) + with_job = create_notification(sample_template, job=sample_job) + without_job = create_notification(sample_template) - auth_header = create_authorization_header() - - response = client.get( - path='/service/{}/notifications?include_jobs=false'.format(sample_service.id), - headers=[auth_header]) - - resp = json.loads(response.get_data(as_text=True)) + resp = admin_request.get( + 'service.get_all_notifications_for_service', + service_id=sample_template.service_id, + include_jobs=False + ) assert len(resp['notifications']) == 1 assert resp['notifications'][0]['id'] == str(without_job.id) - assert response.status_code == 200 def test_set_sms_sender_for_service(client, sample_service): diff --git a/tests/app/test_model.py b/tests/app/test_model.py index d662c6670..9cefb66df 100644 --- a/tests/app/test_model.py +++ b/tests/app/test_model.py @@ -6,6 +6,7 @@ from app import encryption from app.models import ( ServiceWhitelist, Notification, + SMS_TYPE, MOBILE_TYPE, EMAIL_TYPE, NOTIFICATION_CREATED, @@ -18,6 +19,7 @@ from tests.app.conftest import ( sample_template as create_sample_template, sample_notification_with_job as create_sample_notification_with_job ) +from tests.app.db import create_notification @pytest.mark.parametrize('mobile_number', [ @@ -148,21 +150,56 @@ def test_notification_for_csv_returns_bst_correctly(notify_db, notify_db_session assert serialized['created_at'] == 'Monday 27 March 2017 at 00:01' -def test_notification_personalisation_getter_returns_empty_dict_from_None(sample_notification): - sample_notification._personalisation = None - assert sample_notification.personalisation == {} +def test_notification_personalisation_getter_returns_empty_dict_from_None(): + noti = Notification() + noti._personalisation = None + assert noti.personalisation == {} -def test_notification_personalisation_getter_always_returns_empty_dict(sample_notification): - sample_notification._personalisation = encryption.encrypt({}) - assert sample_notification.personalisation == {} +def test_notification_personalisation_getter_always_returns_empty_dict(): + noti = Notification() + noti._personalisation = encryption.encrypt({}) + assert noti.personalisation == {} @pytest.mark.parametrize('input_value', [ None, {} ]) -def test_notification_personalisation_setter_always_sets_empty_dict(sample_notification, input_value): - sample_notification.personalisation = input_value +def test_notification_personalisation_setter_always_sets_empty_dict(input_value): + noti = Notification() + noti.personalisation = input_value - assert sample_notification._personalisation == encryption.encrypt({}) + assert noti._personalisation == encryption.encrypt({}) + + +def test_notification_subject_is_none_for_sms(): + assert Notification(notification_type=SMS_TYPE).subject is None + + +def test_notification_subject_fills_in_placeholders_for_email(sample_email_template_with_placeholders): + noti = create_notification(sample_email_template_with_placeholders, personalisation={'name': 'hello'}) + assert noti.subject == 'hello' + + +def test_notification_subject_fills_in_placeholders_for_letter(sample_letter_template): + sample_letter_template.subject = '((name))' + noti = create_notification(sample_letter_template, personalisation={'name': 'hello'}) + assert noti.subject == 'hello' + + +def test_letter_notification_serializes_with_address(client, sample_letter_notification): + sample_letter_notification.personalisation = { + 'address_line_1': 'foo', + 'address_line_3': 'bar', + 'address_line_5': None, + 'postcode': 'SW1 1AA' + } + res = sample_letter_notification.serialize() + assert res['line_1'] == 'foo' + assert res['line_2'] is None + assert res['line_3'] == 'bar' + assert res['line_4'] is None + assert res['line_5'] is None + assert res['line_6'] is None + assert res['postcode'] == 'SW1 1AA' diff --git a/tests/app/v2/notifications/test_notification_schemas.py b/tests/app/v2/notifications/test_notification_schemas.py index 0aafb38cc..83ad8c5f7 100644 --- a/tests/app/v2/notifications/test_notification_schemas.py +++ b/tests/app/v2/notifications/test_notification_schemas.py @@ -127,7 +127,7 @@ def test_post_sms_schema_with_personalisation_that_is_not_a_dict(): error = json.loads(str(e.value)) assert len(error.get('errors')) == 1 assert error['errors'] == [{'error': 'ValidationError', - 'message': "personalisation should contain key value pairs"}] + 'message': "personalisation not_a_dict is not of type object"}] assert error.get('status_code') == 400 assert len(error.keys()) == 2 diff --git a/tests/app/v2/notifications/test_post_letter_notifications.py b/tests/app/v2/notifications/test_post_letter_notifications.py index d65c6c5d8..4751defd0 100644 --- a/tests/app/v2/notifications/test_post_letter_notifications.py +++ b/tests/app/v2/notifications/test_post_letter_notifications.py @@ -1,24 +1,34 @@ - import uuid -from flask import url_for, json +from flask import json +from flask import url_for import pytest -from app.models import Job, Notification, SMS_TYPE, EMAIL_TYPE, LETTER_TYPE +from app.models import EMAIL_TYPE +from app.models import Job +from app.models import KEY_TYPE_NORMAL +from app.models import KEY_TYPE_TEAM +from app.models import KEY_TYPE_TEST +from app.models import LETTER_TYPE +from app.models import Notification +from app.models import SMS_TYPE from app.v2.errors import RateLimitError +from app.variables import LETTER_TEST_API_FILENAME +from app.variables import LETTER_API_FILENAME from tests import create_authorization_header -from tests.app.db import create_service, create_template +from tests.app.db import create_service +from tests.app.db import create_template -pytestmark = pytest.mark.skip('Leters not currently implemented') - - -def letter_request(client, data, service_id, _expected_status=201): +def letter_request(client, data, service_id, key_type=KEY_TYPE_NORMAL, _expected_status=201): resp = client.post( url_for('v2_notifications.post_notification', notification_type='letter'), data=json.dumps(data), - headers=[('Content-Type', 'application/json'), create_authorization_header(service_id=service_id)] + headers=[ + ('Content-Type', 'application/json'), + create_authorization_header(service_id=service_id, key_type=key_type) + ] ) json_resp = json.loads(resp.get_data(as_text=True)) assert resp.status_code == _expected_status, json_resp @@ -45,7 +55,8 @@ def test_post_letter_notification_returns_201(client, sample_letter_template, mo resp_json = letter_request(client, data, service_id=sample_letter_template.service_id) job = Job.query.one() - notification = Notification.query.all() + assert job.original_file_name == LETTER_API_FILENAME + notification = Notification.query.one() notification_id = notification.id assert resp_json['id'] == str(notification_id) assert resp_json['reference'] == reference @@ -62,51 +73,33 @@ def test_post_letter_notification_returns_201(client, sample_letter_template, mo ) assert not resp_json['scheduled_for'] - mocked.assert_called_once_with((str(job.id), ), queue='job-tasks') + mocked.assert_called_once_with([str(job.id)], queue='job-tasks') def test_post_letter_notification_returns_400_and_missing_template( client, - sample_service + sample_service_full_permissions ): data = { 'template_id': str(uuid.uuid4()), 'personalisation': {'address_line_1': '', 'postcode': ''} } - error_json = letter_request(client, data, service_id=sample_service.id, _expected_status=400) + error_json = letter_request(client, data, service_id=sample_service_full_permissions.id, _expected_status=400) assert error_json['status_code'] == 400 assert error_json['errors'] == [{'error': 'BadRequestError', 'message': 'Template not found'}] -def test_post_notification_returns_403_and_well_formed_auth_error( - client, - sample_letter_template -): - data = { - 'template_id': str(sample_letter_template.id), - 'personalisation': {'address_line_1': '', 'postcode': ''} - } - - error_json = letter_request(client, data, service_id=sample_letter_template.service_id, _expected_status=401) - - assert error_json['status_code'] == 401 - assert error_json['errors'] == [{ - 'error': 'AuthError', - 'message': 'Unauthorized, authentication token must be provided' - }] - - def test_notification_returns_400_for_schema_problems( client, - sample_service + sample_service_full_permissions ): data = { 'personalisation': {'address_line_1': '', 'postcode': ''} } - error_json = letter_request(client, data, service_id=sample_service.id, _expected_status=400) + error_json = letter_request(client, data, service_id=sample_service_full_permissions.id, _expected_status=400) assert error_json['status_code'] == 400 assert error_json['errors'] == [{ @@ -115,6 +108,27 @@ def test_notification_returns_400_for_schema_problems( }] +def test_notification_returns_400_if_address_doesnt_have_underscores( + client, + sample_letter_template +): + data = { + 'template_id': str(sample_letter_template.id), + 'personalisation': { + 'address line 1': 'Her Royal Highness Queen Elizabeth II', + 'postcode': 'SW1 1AA', + } + } + + error_json = letter_request(client, data, service_id=sample_letter_template.service_id, _expected_status=400) + + assert error_json['status_code'] == 400 + assert error_json['errors'] == [{ + 'error': 'ValidationError', + 'message': 'personalisation address_line_1 is a required property' + }] + + def test_returns_a_429_limit_exceeded_if_rate_limit_exceeded( client, sample_letter_template, @@ -160,7 +174,55 @@ def test_post_letter_notification_returns_403_if_not_allowed_to_send_notificatio } error_json = letter_request(client, data, service_id=service.id, _expected_status=400) - assert error_json['status_code'] == 403 + assert error_json['status_code'] == 400 assert error_json['errors'] == [ {'error': 'BadRequestError', 'message': 'Cannot send letters'} ] + + +@pytest.mark.parametrize('research_mode, key_type', [ + (True, KEY_TYPE_NORMAL), + (False, KEY_TYPE_TEST) +]) +def test_post_letter_notification_doesnt_queue_task( + client, + notify_db_session, + mocker, + research_mode, + key_type +): + real_task = mocker.patch('app.celery.tasks.build_dvla_file.apply_async') + fake_task = mocker.patch('app.celery.tasks.update_job_to_sent_to_dvla.apply_async') + + service = create_service(research_mode=research_mode, service_permissions=[LETTER_TYPE]) + template = create_template(service, template_type=LETTER_TYPE) + + data = { + 'template_id': str(template.id), + 'personalisation': {'address_line_1': 'Foo', 'postcode': 'Bar'} + } + + letter_request(client, data, service_id=service.id, key_type=key_type) + + job = Job.query.one() + assert job.original_file_name == LETTER_TEST_API_FILENAME + assert not real_task.called + fake_task.assert_called_once_with([str(job.id)], queue='research-mode-tasks') + + +def test_post_letter_notification_doesnt_accept_team_key(client, sample_letter_template): + data = { + 'template_id': str(sample_letter_template.id), + 'personalisation': {'address_line_1': 'Foo', 'postcode': 'Bar'} + } + + error_json = letter_request( + client, + data, + sample_letter_template.service_id, + key_type=KEY_TYPE_TEAM, + _expected_status=403 + ) + + assert error_json['status_code'] == 403 + assert error_json['errors'] == [{'error': 'BadRequestError', 'message': 'Cannot send letters with a team api key'}] diff --git a/tests/app/v2/notifications/test_post_notifications.py b/tests/app/v2/notifications/test_post_notifications.py index 71dc87fce..c108faacf 100644 --- a/tests/app/v2/notifications/test_post_notifications.py +++ b/tests/app/v2/notifications/test_post_notifications.py @@ -80,10 +80,12 @@ def test_post_notification_returns_400_and_missing_template(client, sample_servi "message": 'Template not found'}] -@pytest.mark.parametrize("notification_type, key_send_to, send_to", - [("sms", "phone_number", "+447700900855"), - ("email", "email_address", "sample@email.com")]) -def test_post_notification_returns_403_and_well_formed_auth_error(client, sample_template, +@pytest.mark.parametrize("notification_type, key_send_to, send_to", [ + ("sms", "phone_number", "+447700900855"), + ("email", "email_address", "sample@email.com"), + ("letter", "personalisation", {"address_line_1": "The queen", "postcode": "SW1 1AA"}) +]) +def test_post_notification_returns_401_and_well_formed_auth_error(client, sample_template, notification_type, key_send_to, send_to): data = { key_send_to: send_to, diff --git a/tests/app/v2/template/test_template_schemas.py b/tests/app/v2/template/test_template_schemas.py index 4517ad7e8..1e5a91e11 100644 --- a/tests/app/v2/template/test_template_schemas.py +++ b/tests/app/v2/template/test_template_schemas.py @@ -51,11 +51,18 @@ valid_json_post_args = { } invalid_json_post_args = [ - ({"id": "invalid_uuid", "personalisation": {"key": "value"}}, ["id is not a valid UUID"]), - ({"id": str(uuid.uuid4()), "personalisation": "invalid_personalisation"}, - ["personalisation should contain key value pairs"]), - ({"personalisation": "invalid_personalisation"}, - ["id is a required property", "personalisation should contain key value pairs"]) + ( + {"id": "invalid_uuid", "personalisation": {"key": "value"}}, + ["id is not a valid UUID"] + ), + ( + {"id": str(uuid.uuid4()), "personalisation": ['a', 'b']}, + ["personalisation [a, b] is not of type object"] + ), + ( + {"personalisation": "invalid_personalisation"}, + ["id is a required property", "personalisation invalid_personalisation is not of type object"] + ) ] valid_json_post_response = {