diff --git a/app/config.py b/app/config.py index 3f6b077f5..5a17d006f 100644 --- a/app/config.py +++ b/app/config.py @@ -332,7 +332,6 @@ class Development(Config): SQLALCHEMY_ECHO = False CSV_UPLOAD_BUCKET_NAME = 'development-notifications-csv-upload' - LETTERS_PDF_BUCKET_NAME = 'development-letters-pdf' TEST_LETTERS_BUCKET_NAME = 'development-test-letters' DVLA_RESPONSE_BUCKET_NAME = 'notify.tools-ftp' LETTERS_PDF_BUCKET_NAME = 'development-letters-pdf' @@ -375,7 +374,6 @@ class Test(Development): TESTING = True CSV_UPLOAD_BUCKET_NAME = 'test-notifications-csv-upload' - LETTERS_PDF_BUCKET_NAME = 'test-letters-pdf' TEST_LETTERS_BUCKET_NAME = 'test-test-letters' DVLA_RESPONSE_BUCKET_NAME = 'test.notify.com-ftp' LETTERS_PDF_BUCKET_NAME = 'test-letters-pdf' @@ -404,7 +402,6 @@ class Preview(Config): NOTIFY_EMAIL_DOMAIN = 'notify.works' NOTIFY_ENVIRONMENT = 'preview' CSV_UPLOAD_BUCKET_NAME = 'preview-notifications-csv-upload' - LETTERS_PDF_BUCKET_NAME = 'preview-letters-pdf' TEST_LETTERS_BUCKET_NAME = 'preview-test-letters' DVLA_RESPONSE_BUCKET_NAME = 'notify.works-ftp' LETTERS_PDF_BUCKET_NAME = 'preview-letters-pdf' @@ -419,7 +416,6 @@ class Staging(Config): NOTIFY_EMAIL_DOMAIN = 'staging-notify.works' NOTIFY_ENVIRONMENT = 'staging' CSV_UPLOAD_BUCKET_NAME = 'staging-notify-csv-upload' - LETTERS_PDF_BUCKET_NAME = 'staging-letters-pdf' TEST_LETTERS_BUCKET_NAME = 'staging-test-letters' DVLA_RESPONSE_BUCKET_NAME = 'staging-notify.works-ftp' LETTERS_PDF_BUCKET_NAME = 'staging-letters-pdf' @@ -436,7 +432,6 @@ class Live(Config): NOTIFY_EMAIL_DOMAIN = 'notifications.service.gov.uk' NOTIFY_ENVIRONMENT = 'live' CSV_UPLOAD_BUCKET_NAME = 'live-notifications-csv-upload' - LETTERS_PDF_BUCKET_NAME = 'production-letters-pdf' TEST_LETTERS_BUCKET_NAME = 'production-test-letters' DVLA_RESPONSE_BUCKET_NAME = 'notifications.service.gov.uk-ftp' LETTERS_PDF_BUCKET_NAME = 'production-letters-pdf' diff --git a/app/letters/utils.py b/app/letters/utils.py index 57395fc87..fc84b50ac 100644 --- a/app/letters/utils.py +++ b/app/letters/utils.py @@ -6,7 +6,7 @@ from flask import current_app from notifications_utils.s3 import s3upload -from app.models import KEY_TYPE_TEST, SECOND_CLASS, RESOLVE_POSTAGE_FOR_FILE_NAME +from app.models import KEY_TYPE_TEST, SECOND_CLASS, RESOLVE_POSTAGE_FOR_FILE_NAME, NOTIFICATION_VALIDATION_FAILED from app.utils import convert_utc_to_bst @@ -48,14 +48,23 @@ def get_letter_pdf_filename(reference, crown, is_scan_letter=False, postage=SECO return upload_file_name -def get_bucket_prefix_for_notification(notification, is_test_letter=False): +def get_bucket_name_and_prefix_for_notification(notification): + is_test_letter = notification.key_type == KEY_TYPE_TEST and notification.template.is_precompiled_letter + folder = '' + if notification.status == NOTIFICATION_VALIDATION_FAILED: + bucket_name = current_app.config['INVALID_PDF_BUCKET_NAME'] + elif is_test_letter: + bucket_name = current_app.config['TEST_LETTERS_BUCKET_NAME'] + else: + bucket_name = current_app.config['LETTERS_PDF_BUCKET_NAME'] + folder = '{}/'.format(notification.created_at.date()) + upload_file_name = PRECOMPILED_BUCKET_PREFIX.format( - folder='' if is_test_letter else - '{}/'.format(notification.created_at.date()), + folder=folder, reference=notification.reference ).upper() - return upload_file_name + return bucket_name, upload_file_name def get_reference_from_filename(filename): @@ -122,18 +131,12 @@ def get_file_names_from_error_bucket(): def get_letter_pdf(notification): - is_test_letter = notification.key_type == KEY_TYPE_TEST and notification.template.is_precompiled_letter - if is_test_letter: - bucket_name = current_app.config['TEST_LETTERS_BUCKET_NAME'] - else: - bucket_name = current_app.config['LETTERS_PDF_BUCKET_NAME'] + bucket_name, prefix = get_bucket_name_and_prefix_for_notification(notification) s3 = boto3.resource('s3') bucket = s3.Bucket(bucket_name) - item = next(x for x in bucket.objects.filter( - Prefix=get_bucket_prefix_for_notification(notification, is_test_letter) - )) + item = next(x for x in bucket.objects.filter(Prefix=prefix)) obj = s3.Object( bucket_name=bucket_name, diff --git a/tests/app/conftest.py b/tests/app/conftest.py index a1d71d2dd..04d77a60e 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -189,24 +189,22 @@ def sample_service( return service -@pytest.fixture(scope='function') -def sample_service_full_permissions(notify_db, notify_db_session): - return sample_service( - notify_db, - notify_db_session, - # ensure name doesn't clash with regular sample service +@pytest.fixture(scope='function', name='sample_service_full_permissions') +def _sample_service_full_permissions(notify_db_session): + service = create_service( service_name="sample service full permissions", - permissions=set(SERVICE_PERMISSION_TYPES) + service_permissions=set(SERVICE_PERMISSION_TYPES) ) - - -@pytest.fixture(scope='function') -def sample_service_custom_letter_contact_block(notify_db, notify_db_session): - service = sample_service(notify_db, notify_db_session) - create_letter_contact(service, contact_block='((contact block))') + create_inbound_number('12345', service_id=service.id) return service +@pytest.fixture(scope='function', name='sample_service_custom_letter_contact_block') +def _sample_service_custom_letter_contact_block(sample_service): + create_letter_contact(sample_service, contact_block='((contact block))') + return sample_service + + @pytest.fixture(scope='function') def sample_template( notify_db, @@ -226,7 +224,9 @@ def sample_template( if user is None: user = create_user() if service is None: - service = sample_service(notify_db, notify_db_session, permissions=permissions) + service = Service.query.filter_by(name='Sample service').first() + if not service: + service = create_service(service_permissions=permissions) if created_by is None: created_by = create_user() diff --git a/tests/app/letters/test_letter_utils.py b/tests/app/letters/test_letter_utils.py index e7068dc9f..f2077b786 100644 --- a/tests/app/letters/test_letter_utils.py +++ b/tests/app/letters/test_letter_utils.py @@ -7,33 +7,39 @@ from freezegun import freeze_time from moto import mock_s3 from app.letters.utils import ( - get_bucket_prefix_for_notification, + get_bucket_name_and_prefix_for_notification, get_letter_pdf_filename, get_letter_pdf, upload_letter_pdf, ScanErrorType, move_failed_pdf, get_folder_name ) -from app.models import KEY_TYPE_NORMAL, KEY_TYPE_TEST, PRECOMPILED_TEMPLATE_NAME +from app.models import KEY_TYPE_NORMAL, KEY_TYPE_TEST, PRECOMPILED_TEMPLATE_NAME, NOTIFICATION_VALIDATION_FAILED from tests.app.db import create_notification FROZEN_DATE_TIME = "2018-03-14 17:00:00" -@pytest.fixture() -def sample_precompiled_letter_notification_using_test_key(sample_letter_notification): +@pytest.fixture(name='sample_precompiled_letter_notification') +def _sample_precompiled_letter_notification(sample_letter_notification): sample_letter_notification.template.hidden = True sample_letter_notification.template.name = PRECOMPILED_TEMPLATE_NAME - sample_letter_notification.key_type = KEY_TYPE_TEST sample_letter_notification.reference = 'foo' with freeze_time(FROZEN_DATE_TIME): sample_letter_notification.created_at = datetime.utcnow() return sample_letter_notification -def test_get_bucket_prefix_for_notification_valid_notification(sample_notification): +@pytest.fixture(name='sample_precompiled_letter_notification_using_test_key') +def _sample_precompiled_letter_notification_using_test_key(sample_precompiled_letter_notification): + sample_precompiled_letter_notification.key_type = KEY_TYPE_TEST + return sample_precompiled_letter_notification - bucket_prefix = get_bucket_prefix_for_notification(sample_notification) +def test_get_bucket_name_and_prefix_for_notification_valid_notification(sample_notification): + + bucket, bucket_prefix = get_bucket_name_and_prefix_for_notification(sample_notification) + + assert bucket == current_app.config['LETTERS_PDF_BUCKET_NAME'] assert bucket_prefix == '{folder}/NOTIFY.{reference}'.format( folder=sample_notification.created_at.date(), reference=sample_notification.reference @@ -41,19 +47,44 @@ def test_get_bucket_prefix_for_notification_valid_notification(sample_notificati @freeze_time(FROZEN_DATE_TIME) -def test_get_bucket_prefix_for_notification_precompiled_letter_using_test_key( +def test_get_bucket_name_and_prefix_for_notification_precompiled_letter_using_test_key( sample_precompiled_letter_notification_using_test_key ): - bucket_prefix = get_bucket_prefix_for_notification( - sample_precompiled_letter_notification_using_test_key, is_test_letter=True) + bucket, bucket_prefix = get_bucket_name_and_prefix_for_notification( + sample_precompiled_letter_notification_using_test_key) + assert bucket == current_app.config['TEST_LETTERS_BUCKET_NAME'] assert bucket_prefix == 'NOTIFY.{}'.format( sample_precompiled_letter_notification_using_test_key.reference).upper() -def test_get_bucket_prefix_for_notification_invalid_notification(): +@freeze_time(FROZEN_DATE_TIME) +def test_get_bucket_name_and_prefix_for_failed_validation(sample_precompiled_letter_notification): + sample_precompiled_letter_notification.status = NOTIFICATION_VALIDATION_FAILED + bucket, bucket_prefix = get_bucket_name_and_prefix_for_notification(sample_precompiled_letter_notification) + + assert bucket == current_app.config['INVALID_PDF_BUCKET_NAME'] + assert bucket_prefix == 'NOTIFY.{}'.format( + sample_precompiled_letter_notification.reference).upper() + + +@freeze_time(FROZEN_DATE_TIME) +def test_get_bucket_name_and_prefix_for_test_noti_with_failed_validation( + sample_precompiled_letter_notification_using_test_key +): + sample_precompiled_letter_notification_using_test_key.status = NOTIFICATION_VALIDATION_FAILED + bucket, bucket_prefix = get_bucket_name_and_prefix_for_notification( + sample_precompiled_letter_notification_using_test_key + ) + + assert bucket == current_app.config['INVALID_PDF_BUCKET_NAME'] + assert bucket_prefix == 'NOTIFY.{}'.format( + sample_precompiled_letter_notification_using_test_key.reference).upper() + + +def test_get_bucket_name_and_prefix_for_notification_invalid_notification(): with pytest.raises(AttributeError): - get_bucket_prefix_for_notification(None) + get_bucket_name_and_prefix_for_notification(None) @pytest.mark.parametrize('crown_flag,expected_crown_text', [