diff --git a/app/celery/tasks.py b/app/celery/tasks.py index 96d26fe7d..1948c0964 100644 --- a/app/celery/tasks.py +++ b/app/celery/tasks.py @@ -307,6 +307,7 @@ def save_letter( saved_notification = persist_notification( template_id=notification['template'], template_version=notification['template_version'], + template_postage=template.postage, recipient=recipient, service=service, personalisation=notification['personalisation'], diff --git a/app/notifications/process_letter_notifications.py b/app/notifications/process_letter_notifications.py index 984ad257e..06d1127bf 100644 --- a/app/notifications/process_letter_notifications.py +++ b/app/notifications/process_letter_notifications.py @@ -7,6 +7,7 @@ def create_letter_notification(letter_data, template, api_key, status, reply_to_ notification = persist_notification( template_id=template.id, template_version=template.version, + template_postage=template.postage, # 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=template.service, @@ -20,6 +21,7 @@ def create_letter_notification(letter_data, template, api_key, status, reply_to_ client_reference=letter_data.get('reference'), status=status, reply_to_text=reply_to_text, - billable_units=billable_units + billable_units=billable_units, + postage=letter_data.get('postage') ) return notification diff --git a/app/notifications/process_notifications.py b/app/notifications/process_notifications.py index 8fc2f15f6..f8850c3e9 100644 --- a/app/notifications/process_notifications.py +++ b/app/notifications/process_notifications.py @@ -32,8 +32,6 @@ from app.dao.notifications_dao import ( dao_created_scheduled_notification ) -from app.dao.templates_dao import dao_get_template_by_id - from app.v2.errors import BadRequestError from app.utils import ( cache_key_for_service_template_counter, @@ -75,7 +73,9 @@ def persist_notification( created_by_id=None, status=NOTIFICATION_CREATED, reply_to_text=None, - billable_units=None + billable_units=None, + postage=None, + template_postage=None ): notification_created_at = created_at or datetime.utcnow() if not notification_id: @@ -112,11 +112,13 @@ def persist_notification( elif notification_type == EMAIL_TYPE: notification.normalised_to = format_email_address(notification.to) elif notification_type == LETTER_TYPE: - template = dao_get_template_by_id(template_id, template_version) - if service.has_permission(CHOOSE_POSTAGE) and template.postage: - notification.postage = template.postage + if postage: + notification.postage = postage else: - notification.postage = service.postage + if service.has_permission(CHOOSE_POSTAGE) and template_postage: + notification.postage = template_postage + else: + notification.postage = service.postage # if simulated create a Notification model to return but do not persist the Notification to the dB if not simulated: diff --git a/app/notifications/rest.py b/app/notifications/rest.py index 04286688a..aa4be0ea9 100644 --- a/app/notifications/rest.py +++ b/app/notifications/rest.py @@ -124,6 +124,7 @@ def send_notification(notification_type): simulated = simulated_recipient(notification_form['to'], notification_type) notification_model = persist_notification(template_id=template.id, template_version=template.version, + template_postage=template.postage, recipient=request.get_json()['to'], service=authenticated_service, personalisation=notification_form.get('personalisation', None), diff --git a/app/schema_validation/__init__.py b/app/schema_validation/__init__.py index 382d9229c..98e67a50a 100644 --- a/app/schema_validation/__init__.py +++ b/app/schema_validation/__init__.py @@ -8,41 +8,54 @@ from notifications_utils.recipients import (validate_phone_number, validate_emai InvalidEmailError) +format_checker = FormatChecker() + + +@format_checker.checks("validate_uuid", raises=Exception) +def validate_uuid(instance): + if isinstance(instance, str): + UUID(instance) + return True + + +@format_checker.checks('phone_number', raises=InvalidPhoneError) +def validate_schema_phone_number(instance): + if isinstance(instance, str): + validate_phone_number(instance, international=True) + return True + + +@format_checker.checks('email_address', raises=InvalidEmailError) +def validate_schema_email_address(instance): + if isinstance(instance, str): + validate_email_address(instance) + return True + + +@format_checker.checks('postage', raises=ValidationError) +def validate_schema_postage(instance): + if isinstance(instance, str): + if instance not in ["first", "second"]: + raise ValidationError("invalid. It must be either first or second.") + return True + + +@format_checker.checks('datetime_within_next_day', raises=ValidationError) +def validate_schema_date_with_hour(instance): + if isinstance(instance, str): + try: + dt = iso8601.parse_date(instance).replace(tzinfo=None) + if dt < datetime.utcnow(): + raise ValidationError("datetime can not be in the past") + if dt > datetime.utcnow() + timedelta(hours=24): + raise ValidationError("datetime can only be 24 hours in the future") + except ParseError: + raise ValidationError("datetime format is invalid. It must be a valid ISO8601 date time format, " + "https://en.wikipedia.org/wiki/ISO_8601") + return True + + def validate(json_to_validate, schema): - format_checker = FormatChecker() - - @format_checker.checks("validate_uuid", raises=Exception) - def validate_uuid(instance): - if isinstance(instance, str): - UUID(instance) - return True - - @format_checker.checks('phone_number', raises=InvalidPhoneError) - def validate_schema_phone_number(instance): - if isinstance(instance, str): - validate_phone_number(instance, international=True) - return True - - @format_checker.checks('email_address', raises=InvalidEmailError) - def validate_schema_email_address(instance): - if isinstance(instance, str): - validate_email_address(instance) - return True - - @format_checker.checks('datetime_within_next_day', raises=ValidationError) - def validate_schema_date_with_hour(instance): - if isinstance(instance, str): - try: - dt = iso8601.parse_date(instance).replace(tzinfo=None) - if dt < datetime.utcnow(): - raise ValidationError("datetime can not be in the past") - if dt > datetime.utcnow() + timedelta(hours=24): - raise ValidationError("datetime can only be 24 hours in the future") - except ParseError: - raise ValidationError("datetime format is invalid. It must be a valid ISO8601 date time format, " - "https://en.wikipedia.org/wiki/ISO_8601") - return True - validator = Draft7Validator(schema, format_checker=format_checker) errors = list(validator.iter_errors(json_to_validate)) if errors.__len__() > 0: diff --git a/app/service/send_notification.py b/app/service/send_notification.py index a00d151a4..26307b8c3 100644 --- a/app/service/send_notification.py +++ b/app/service/send_notification.py @@ -77,6 +77,7 @@ def send_one_off_notification(service_id, post_data): notification = persist_notification( template_id=template.id, template_version=template.version, + template_postage=template.postage, recipient=post_data['to'], service=service, personalisation=personalisation, diff --git a/app/v2/notifications/notification_schemas.py b/app/v2/notifications/notification_schemas.py index 39c78d727..733eb8aef 100644 --- a/app/v2/notifications/notification_schemas.py +++ b/app/v2/notifications/notification_schemas.py @@ -239,7 +239,8 @@ post_precompiled_letter_request = { "title": "POST v2/notifications/letter", "properties": { "reference": {"type": "string"}, - "content": {"type": "string"} + "content": {"type": "string"}, + "postage": {"type": "string", "format": "postage"} }, "required": ["reference", "content"], "additionalProperties": False diff --git a/tests/app/notifications/test_process_notification.py b/tests/app/notifications/test_process_notification.py index 192644bde..9710146e0 100644 --- a/tests/app/notifications/test_process_notification.py +++ b/tests/app/notifications/test_process_notification.py @@ -504,6 +504,7 @@ def test_persist_letter_notification_finds_correct_postage( persist_notification( template_id=template.id, template_version=template.version, + template_postage=template.postage, recipient="Jane Doe, 10 Downing Street, London", service=service, personalisation=None, diff --git a/tests/app/service/test_send_one_off_notification.py b/tests/app/service/test_send_one_off_notification.py index 0c20611dc..b70459fc8 100644 --- a/tests/app/service/test_send_one_off_notification.py +++ b/tests/app/service/test_send_one_off_notification.py @@ -90,6 +90,7 @@ def test_send_one_off_notification_calls_persist_correctly_for_sms( persist_mock.assert_called_once_with( template_id=template.id, template_version=template.version, + template_postage=None, recipient=post_data['to'], service=template.service, personalisation={'name': 'foo'}, @@ -127,6 +128,7 @@ def test_send_one_off_notification_calls_persist_correctly_for_email( persist_mock.assert_called_once_with( template_id=template.id, template_version=template.version, + template_postage=None, recipient=post_data['to'], service=template.service, personalisation={'name': 'foo'}, @@ -153,6 +155,7 @@ def test_send_one_off_notification_calls_persist_correctly_for_letter( template = create_template( service=service, template_type=LETTER_TYPE, + postage='first', subject="Test subject", content="Hello (( Name))\nYour thing is due soon", ) @@ -174,6 +177,7 @@ def test_send_one_off_notification_calls_persist_correctly_for_letter( persist_mock.assert_called_once_with( template_id=template.id, template_version=template.version, + template_postage='first', recipient=post_data['to'], service=template.service, personalisation=post_data['personalisation'], diff --git a/tests/app/v2/notifications/test_post_letter_notifications.py b/tests/app/v2/notifications/test_post_letter_notifications.py index 734928088..db6cd4159 100644 --- a/tests/app/v2/notifications/test_post_letter_notifications.py +++ b/tests/app/v2/notifications/test_post_letter_notifications.py @@ -469,16 +469,27 @@ def test_post_precompiled_letter_with_invalid_base64(client, notify_user, mocker assert not Notification.query.first() -@pytest.mark.parametrize('postage', ['first', 'second']) -def test_post_precompiled_letter_notification_returns_201(client, notify_user, mocker, postage): +@pytest.mark.parametrize('service_postage, notification_postage, expected_postage', [ + ('second', 'second', 'second'), + ('second', 'first', 'first'), + ('second', None, 'second'), + ('first', 'first', 'first'), + ('first', 'second', 'second'), + ('first', None, 'first'), +]) +def test_post_precompiled_letter_notification_returns_201( + client, notify_user, mocker, service_postage, notification_postage, expected_postage +): sample_service = create_service(service_permissions=['letter', 'precompiled_letter']) - sample_service.postage = postage + sample_service.postage = service_postage s3mock = mocker.patch('app.v2.notifications.post_notifications.upload_letter_pdf') mocker.patch('app.celery.letters_pdf_tasks.notify_celery.send_task') data = { "reference": "letter-reference", "content": "bGV0dGVyLWNvbnRlbnQ=" } + if notification_postage: + data["postage"] = notification_postage auth_header = create_authorization_header(service_id=sample_service.id) response = client.post( path="v2/notifications/letter", @@ -493,10 +504,30 @@ def test_post_precompiled_letter_notification_returns_201(client, notify_user, m assert notification.billable_units == 0 assert notification.status == NOTIFICATION_PENDING_VIRUS_CHECK - assert notification.postage == postage + assert notification.postage == expected_postage notification_history = NotificationHistory.query.one() - assert notification_history.postage == postage + assert notification_history.postage == expected_postage resp_json = json.loads(response.get_data(as_text=True)) assert resp_json == {'id': str(notification.id), 'reference': 'letter-reference'} + + +def test_post_letter_notification_throws_error_for_invalid_postage(client, notify_user, mocker): + sample_service = create_service(service_permissions=['letter', 'precompiled_letter']) + data = { + "reference": "letter-reference", + "content": "bGV0dGVyLWNvbnRlbnQ=", + "postage": "space unicorn" + } + auth_header = create_authorization_header(service_id=sample_service.id) + response = client.post( + path="v2/notifications/letter", + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), auth_header]) + + assert response.status_code == 400, response.get_data(as_text=True) + resp_json = json.loads(response.get_data(as_text=True)) + assert resp_json['errors'][0]['message'] == "postage invalid. It must be either first or second." + + assert not Notification.query.first()