diff --git a/app/dao/jobs_dao.py b/app/dao/jobs_dao.py index ec7bbe2a6..81c00633c 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 @@ -108,6 +109,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() diff --git a/app/notifications/process_letter_notifications.py b/app/notifications/process_letter_notifications.py index 358df8771..0cfb35442 100644 --- a/app/notifications/process_letter_notifications.py +++ b/app/notifications/process_letter_notifications.py @@ -1,15 +1,16 @@ from app import create_random_identifier -from app.models import LETTER_TYPE, JOB_STATUS_READY_TO_SEND +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 def create_letter_api_job(template): service = template.service if not service.active: - raise InvalidRequest('Create job is not allowed: service is inactive', 403) + raise InvalidRequest('Service {} is inactive'.format(service.id), 403) if template.archived: - raise InvalidRequest('Create job is not allowed: template is deleted', 400) - + raise InvalidRequest('Template {} is deleted'.format(template.id), 400) job = Job( original_file_name='letter submitted via api', @@ -21,13 +22,15 @@ def create_letter_api_job(template): 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, - recipient=letter_data['personalisation']['address line 1'], # or addressline1 or address_line_1? + # 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, 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/migrations/versions/0113_job_created_by_nullable.py b/migrations/versions/0113_job_created_by_nullable.py index 8596dac7b..c6a391523 100644 --- a/migrations/versions/0113_job_created_by_nullable.py +++ b/migrations/versions/0113_job_created_by_nullable.py @@ -15,10 +15,10 @@ import sqlalchemy as sa from sqlalchemy.dialects import postgresql def upgrade(): - op.alter_column('job', 'created_by', nullable=True) + 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('job', 'created_by', nullable=False) + op.alter_column('jobs', 'created_by_id', nullable=False) 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..4a65a487e --- /dev/null +++ b/tests/app/notifications/test_process_letter_notifications.py @@ -0,0 +1,83 @@ +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 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 submitted via api' + 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/v2/notifications/test_post_letter_notifications.py b/tests/app/v2/notifications/test_post_letter_notifications.py index e794520f2..66a9004c7 100644 --- a/tests/app/v2/notifications/test_post_letter_notifications.py +++ b/tests/app/v2/notifications/test_post_letter_notifications.py @@ -1,13 +1,20 @@ 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 LETTER_TYPE +from app.models import Notification +from app.models import SMS_TYPE from app.v2.errors import RateLimitError +from app.v2.notifications.post_notifications import process_letter_notification 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 def letter_request(client, data, service_id, _expected_status=201): @@ -41,7 +48,7 @@ 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() + notification = Notification.query.one() notification_id = notification.id assert resp_json['id'] == str(notification_id) assert resp_json['reference'] == reference @@ -58,51 +65,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'] == [{ @@ -111,6 +100,7 @@ def test_notification_returns_400_for_schema_problems( }] + def test_returns_a_429_limit_exceeded_if_rate_limit_exceeded( client, sample_letter_template, 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,