Merge pull request #2164 from alphagov/fix-bucket-names

make sure validation_failed notifications still return pdf
This commit is contained in:
Leo Hemsted
2018-10-17 17:04:25 +01:00
committed by GitHub
4 changed files with 73 additions and 44 deletions

View File

@@ -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'

View File

@@ -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,

View File

@@ -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()

View File

@@ -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', [