diff --git a/app/notifications/process_notifications.py b/app/notifications/process_notifications.py index 99665ce78..7acc19d27 100644 --- a/app/notifications/process_notifications.py +++ b/app/notifications/process_notifications.py @@ -23,8 +23,7 @@ from app.models import ( LETTER_TYPE, NOTIFICATION_CREATED, Notification, - ScheduledNotification, - CHOOSE_POSTAGE + ScheduledNotification ) from app.dao.notifications_dao import ( dao_create_notification, @@ -108,13 +107,7 @@ def persist_notification( elif notification_type == EMAIL_TYPE: notification.normalised_to = format_email_address(notification.to) elif notification_type == LETTER_TYPE: - if postage: - notification.postage = postage - else: - if service.has_permission(CHOOSE_POSTAGE) and template_postage: - notification.postage = template_postage - else: - notification.postage = service.postage + notification.postage = postage or template_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/v2/notifications/post_notifications.py b/app/v2/notifications/post_notifications.py index 511155a7e..c15ac4ba9 100644 --- a/app/v2/notifications/post_notifications.py +++ b/app/v2/notifications/post_notifications.py @@ -28,6 +28,7 @@ from app.models import ( NOTIFICATION_SENDING, NOTIFICATION_DELIVERED, NOTIFICATION_PENDING_VIRUS_CHECK, + SECOND_CLASS ) from app.notifications.process_letter_notifications import ( create_letter_notification @@ -355,6 +356,7 @@ def get_precompiled_letter_template(service_id): hidden=True, subject='Pre-compiled PDF', content='', + postage=SECOND_CLASS ) dao_create_template(template) diff --git a/tests/app/celery/test_tasks.py b/tests/app/celery/test_tasks.py index 13c6f9b4f..7a43a4ded 100644 --- a/tests/app/celery/test_tasks.py +++ b/tests/app/celery/test_tasks.py @@ -40,7 +40,8 @@ from app.models import ( JOB_STATUS_ERROR, JOB_STATUS_IN_PROGRESS, LETTER_TYPE, - SMS_TYPE + SMS_TYPE, + PRECOMPILED_LETTER ) from tests.app import load_example_csv @@ -50,6 +51,8 @@ from tests.app.conftest import ( sample_job as create_sample_job, sample_email_template as create_sample_email_template, sample_notification as create_sample_notification, + sample_letter_template, + sample_letter_job ) from tests.app.db import ( create_inbound_sms, @@ -1011,22 +1014,22 @@ def test_save_letter_saves_letter_to_database(mocker, notify_db_session): @pytest.mark.parametrize('postage', ['first', 'second']) -def test_save_letter_saves_letter_to_database_with_correct_postage(mocker, sample_letter_job, postage): - sample_letter_job.service.postage = postage +def test_save_letter_saves_letter_to_database_with_correct_postage(mocker, notify_db_session, postage): + service = create_service(service_permissions=[LETTER_TYPE, PRECOMPILED_LETTER]) + template = sample_letter_template(service, postage=postage) + letter_job = sample_letter_job(template) mocker.patch('app.celery.tasks.letters_pdf_tasks.create_letters_pdf.apply_async') - notification_json = _notification_json( - template=sample_letter_job.template, + template=letter_job.template, to='Foo', personalisation={'addressline1': 'Foo', 'addressline2': 'Bar', 'postcode': 'Flob'}, - job_id=sample_letter_job.id, + job_id=letter_job.id, row_number=1 ) notification_id = uuid.uuid4() - save_letter( - sample_letter_job.service_id, + letter_job.service_id, notification_id, encryption.encrypt(notification_json), ) diff --git a/tests/app/conftest.py b/tests/app/conftest.py index 5b6aa2234..b3c2627a8 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -253,6 +253,8 @@ def sample_template( data.update({ 'subject': subject_line }) + if template_type == 'letter': + data['postage'] = 'second' template = Template(**data) dao_create_template(template) @@ -317,8 +319,8 @@ def sample_template_without_email_permission(notify_db, notify_db_session): @pytest.fixture -def sample_letter_template(sample_service_full_permissions): - return create_template(sample_service_full_permissions, template_type=LETTER_TYPE) +def sample_letter_template(sample_service_full_permissions, postage="second"): + return create_template(sample_service_full_permissions, template_type=LETTER_TYPE, postage=postage) @pytest.fixture diff --git a/tests/app/db.py b/tests/app/db.py index a58d1fbcb..7932ffb87 100644 --- a/tests/app/db.py +++ b/tests/app/db.py @@ -42,6 +42,7 @@ from app.models import ( User, EMAIL_TYPE, SMS_TYPE, + LETTER_TYPE, KEY_TYPE_NORMAL, AnnualBilling, InvitedOrganisationUser, @@ -159,9 +160,10 @@ def create_template( 'created_by': service.created_by, 'reply_to': reply_to, 'hidden': hidden, - 'folder': folder, - 'postage': postage, + 'folder': folder } + if template_type == LETTER_TYPE: + data["postage"] = postage or "second" if template_type != SMS_TYPE: data['subject'] = subject template = Template(**data) diff --git a/tests/app/notifications/test_process_notification.py b/tests/app/notifications/test_process_notification.py index 61d177176..349cb82a6 100644 --- a/tests/app/notifications/test_process_notification.py +++ b/tests/app/notifications/test_process_notification.py @@ -12,8 +12,7 @@ from app.models import ( NotificationHistory, ScheduledNotification, Template, - LETTER_TYPE, - CHOOSE_POSTAGE + LETTER_TYPE ) from app.notifications.process_notifications import ( create_content_for_notification, @@ -466,22 +465,23 @@ def test_persist_email_notification_stores_normalised_email( @pytest.mark.parametrize( - "service_permissions, template_postage, expected_postage", + "postage_argument, template_postage, expected_postage", [ - ([LETTER_TYPE], "first", "second"), - ([LETTER_TYPE, CHOOSE_POSTAGE], "first", "first"), - ([LETTER_TYPE, CHOOSE_POSTAGE], None, "second"), + ("second", "first", "second"), + ("first", "first", "first"), + ("first", "second", "first"), + (None, "second", "second") ] ) def test_persist_letter_notification_finds_correct_postage( mocker, notify_db, notify_db_session, - service_permissions, + postage_argument, template_postage, expected_postage ): - service = create_service(service_permissions=service_permissions, postage="second") + service = create_service(service_permissions=[LETTER_TYPE]) api_key = create_api_key(notify_db, notify_db_session, service=service) template = create_template(service, template_type=LETTER_TYPE, postage=template_postage) mocker.patch('app.dao.templates_dao.dao_get_template_by_id', return_value=template) @@ -495,6 +495,7 @@ def test_persist_letter_notification_finds_correct_postage( notification_type=LETTER_TYPE, api_key_id=api_key.id, key_type=api_key.key_type, + postage=postage_argument ) persisted_notification = Notification.query.all()[0] @@ -502,18 +503,22 @@ def test_persist_letter_notification_finds_correct_postage( def test_persist_notification_with_billable_units_stores_correct_info( - sample_template, + mocker ): + service = create_service(service_permissions=[LETTER_TYPE]) + template = create_template(service, template_type=LETTER_TYPE) + mocker.patch('app.dao.templates_dao.dao_get_template_by_id', return_value=template) persist_notification( - template_id=sample_template.id, - template_version=sample_template.version, + template_id=template.id, + template_version=template.version, recipient="123 Main Street", - service=sample_template.service, + service=template.service, personalisation=None, - notification_type='letter', + notification_type=template.template_type, api_key_id=None, key_type="normal", - billable_units=3 + billable_units=3, + template_postage=template.postage ) persisted_notification = Notification.query.all()[0] diff --git a/tests/app/template/test_rest.py b/tests/app/template/test_rest.py index b84d989c1..017921c1a 100644 --- a/tests/app/template/test_rest.py +++ b/tests/app/template/test_rest.py @@ -18,8 +18,7 @@ from app.models import ( LETTER_TYPE, SMS_TYPE, Template, - TemplateHistory, - CHOOSE_POSTAGE + TemplateHistory ) from app.dao.templates_dao import dao_get_template_by_id, dao_redact_template @@ -44,7 +43,7 @@ from tests.conftest import set_config_values def test_should_create_a_new_template_for_a_service( client, sample_user, template_type, subject ): - service = create_service(service_permissions=[template_type, CHOOSE_POSTAGE]) + service = create_service(service_permissions=[template_type]) data = { 'name': 'my template', 'template_type': template_type, @@ -339,7 +338,7 @@ def test_must_have_a_subject_on_an_email_or_letter_template(client, sample_user, def test_update_should_update_a_template(client, sample_user): - service = create_service(service_permissions=[LETTER_TYPE, CHOOSE_POSTAGE]) + service = create_service(service_permissions=[LETTER_TYPE]) template = create_template(service, template_type="letter", postage="second") data = { 'content': 'my template has new content, swell!', diff --git a/tests/app/v2/notifications/test_post_letter_notifications.py b/tests/app/v2/notifications/test_post_letter_notifications.py index 22232aa73..6a0b8ba21 100644 --- a/tests/app/v2/notifications/test_post_letter_notifications.py +++ b/tests/app/v2/notifications/test_post_letter_notifications.py @@ -19,8 +19,7 @@ from app.models import ( NOTIFICATION_SENDING, NOTIFICATION_DELIVERED, NOTIFICATION_PENDING_VIRUS_CHECK, - SMS_TYPE, - CHOOSE_POSTAGE + SMS_TYPE ) from app.schema_validation import validate from app.v2.errors import RateLimitError @@ -96,20 +95,11 @@ def test_post_letter_notification_returns_201(client, sample_letter_template, mo mock.assert_called_once_with([str(notification.id)], queue=QueueNames.CREATE_LETTERS_PDF) -@pytest.mark.parametrize('service_permissions, service_postage, template_postage, expected_postage', [ - ([LETTER_TYPE], "second", "first", "second"), - ([LETTER_TYPE], "first", "second", "first"), - ([LETTER_TYPE], "first", None, "first"), - ([LETTER_TYPE, CHOOSE_POSTAGE], "second", "first", "first"), - ([LETTER_TYPE, CHOOSE_POSTAGE], "second", None, "second"), - ([LETTER_TYPE, CHOOSE_POSTAGE], "second", "second", "second"), - ([LETTER_TYPE, CHOOSE_POSTAGE], "first", "second", "second"), -]) def test_post_letter_notification_sets_postage( - client, notify_db_session, mocker, service_permissions, service_postage, template_postage, expected_postage + client, notify_db_session, mocker ): - service = create_service(service_permissions=service_permissions, postage=service_postage) - template = create_template(service, template_type="letter", postage=template_postage) + service = create_service(service_permissions=[LETTER_TYPE]) + template = create_template(service, template_type="letter", postage="first") mocker.patch('app.celery.tasks.letters_pdf_tasks.create_letters_pdf.apply_async') data = { 'template_id': str(template.id), @@ -126,7 +116,7 @@ def test_post_letter_notification_sets_postage( assert validate(resp_json, post_letter_response) == resp_json notification = Notification.query.one() - assert notification.postage == expected_postage + assert notification.postage == "first" @pytest.mark.parametrize('env', [ @@ -469,19 +459,15 @@ def test_post_precompiled_letter_with_invalid_base64(client, notify_user, mocker assert not Notification.query.first() -@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'), +@pytest.mark.parametrize('notification_postage, expected_postage', [ + ('second', 'second'), + ('first', 'first'), + (None, 'second') ]) def test_post_precompiled_letter_notification_returns_201( - client, notify_user, mocker, service_postage, notification_postage, expected_postage + client, notify_user, mocker, notification_postage, expected_postage ): sample_service = create_service(service_permissions=['letter', 'precompiled_letter']) - 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 = { diff --git a/tests/app/v2/template/test_get_template.py b/tests/app/v2/template/test_get_template.py index 900621db7..dadf82983 100644 --- a/tests/app/v2/template/test_get_template.py +++ b/tests/app/v2/template/test_get_template.py @@ -10,13 +10,15 @@ from tests.app.db import create_template valid_version_params = [None, 1] -@pytest.mark.parametrize("tmp_type, expected_name, expected_subject", [ - (SMS_TYPE, 'sms Template Name', None), - (EMAIL_TYPE, 'email Template Name', 'Template subject'), - (LETTER_TYPE, 'letter Template Name', 'Template subject') +@pytest.mark.parametrize("tmp_type, expected_name, expected_subject,postage", [ + (SMS_TYPE, 'sms Template Name', None, None), + (EMAIL_TYPE, 'email Template Name', 'Template subject', None), + (LETTER_TYPE, 'letter Template Name', 'Template subject', "second") ]) @pytest.mark.parametrize("version", valid_version_params) -def test_get_template_by_id_returns_200(client, sample_service, tmp_type, expected_name, expected_subject, version): +def test_get_template_by_id_returns_200( + client, sample_service, tmp_type, expected_name, expected_subject, version, postage +): template = create_template(sample_service, template_type=tmp_type) auth_header = create_authorization_header(service_id=sample_service.id) @@ -41,7 +43,7 @@ def test_get_template_by_id_returns_200(client, sample_service, tmp_type, expect "subject": expected_subject, 'name': expected_name, 'personalisation': {}, - 'postage': None, + 'postage': postage, } assert json_response == expected_response