mirror of
https://github.com/GSA/notifications-api.git
synced 2025-12-23 00:41:35 -05:00
Remove unneeded check that notification is in created state
We instead rely on the fact that only files being passed into this function we already know are in the created state
This commit is contained in:
@@ -25,7 +25,6 @@ from app.dao.notifications_dao import (
|
||||
update_notification_status_by_id,
|
||||
dao_update_notification,
|
||||
dao_get_notification_by_reference,
|
||||
dao_get_notifications_by_references,
|
||||
dao_update_notifications_by_reference,
|
||||
dao_get_letters_to_be_printed,
|
||||
)
|
||||
@@ -180,10 +179,6 @@ def get_key_and_size_of_letters_to_be_sent_to_print(print_run_date):
|
||||
postage=letter.postage
|
||||
)
|
||||
|
||||
current_app.logger.info(
|
||||
f'Found notification {letter.id} to send to DVLA to print: {letter_file_name}'
|
||||
)
|
||||
|
||||
letter_head = s3.head_s3_object(current_app.config['LETTERS_PDF_BUCKET_NAME'], letter_file_name)
|
||||
letter_pdfs.append({"Key": letter_file_name, "Size": letter_head['ContentLength']})
|
||||
|
||||
@@ -199,7 +194,7 @@ def group_letters(letter_pdfs):
|
||||
running_filesize = 0
|
||||
list_of_files = []
|
||||
for letter in letter_pdfs:
|
||||
if letter['Key'].lower().endswith('.pdf') and letter_in_created_state(letter['Key']):
|
||||
if letter['Key'].lower().endswith('.pdf'):
|
||||
if (
|
||||
running_filesize + letter['Size'] > current_app.config['MAX_LETTER_PDF_ZIP_FILESIZE'] or
|
||||
len(list_of_files) >= current_app.config['MAX_LETTER_PDF_COUNT_PER_ZIP']
|
||||
@@ -215,22 +210,6 @@ def group_letters(letter_pdfs):
|
||||
yield list_of_files
|
||||
|
||||
|
||||
def letter_in_created_state(filename):
|
||||
# filename looks like '2018-01-13/NOTIFY.ABCDEF1234567890.D.2.C.C.20180113120000.PDF'
|
||||
subfolder = filename.split('/')[0]
|
||||
ref = get_reference_from_filename(filename)
|
||||
notifications = dao_get_notifications_by_references([ref])
|
||||
if notifications:
|
||||
if notifications[0].status == NOTIFICATION_CREATED:
|
||||
return True
|
||||
current_app.logger.info('Collating letters for {} but notification with reference {} already in {}'.format(
|
||||
subfolder,
|
||||
ref,
|
||||
notifications[0].status
|
||||
))
|
||||
return False
|
||||
|
||||
|
||||
@notify_celery.task(bind=True, name='process-virus-scan-passed', max_retries=15, default_retry_delay=300)
|
||||
def process_virus_scan_passed(self, filename):
|
||||
reference = get_reference_from_filename(filename)
|
||||
|
||||
@@ -22,7 +22,6 @@ from app.celery.letters_pdf_tasks import (
|
||||
collate_letter_pdfs_for_day,
|
||||
get_key_and_size_of_letters_to_be_sent_to_print,
|
||||
group_letters,
|
||||
letter_in_created_state,
|
||||
process_sanitised_letter,
|
||||
process_virus_scan_passed,
|
||||
process_virus_scan_failed,
|
||||
@@ -41,7 +40,6 @@ from app.models import (
|
||||
NOTIFICATION_CREATED,
|
||||
NOTIFICATION_DELIVERED,
|
||||
NOTIFICATION_PENDING_VIRUS_CHECK,
|
||||
NOTIFICATION_SENDING,
|
||||
NOTIFICATION_TECHNICAL_FAILURE,
|
||||
NOTIFICATION_VALIDATION_FAILED,
|
||||
NOTIFICATION_VIRUS_SCAN_FAILED,
|
||||
@@ -423,8 +421,7 @@ def test_collate_letter_pdfs_for_day_when_run_after_midnight(notify_api, sample_
|
||||
)
|
||||
|
||||
|
||||
def test_group_letters_splits_on_file_size(notify_api, mocker):
|
||||
mocker.patch('app.celery.letters_pdf_tasks.letter_in_created_state', return_value=True)
|
||||
def test_group_letters_splits_on_file_size(notify_api):
|
||||
letters = [
|
||||
# ends under max but next one is too big
|
||||
{'Key': 'A.pdf', 'Size': 1}, {'Key': 'B.pdf', 'Size': 2},
|
||||
@@ -450,8 +447,7 @@ def test_group_letters_splits_on_file_size(notify_api, mocker):
|
||||
assert next(x, None) is None
|
||||
|
||||
|
||||
def test_group_letters_splits_on_file_count(notify_api, mocker):
|
||||
mocker.patch('app.celery.letters_pdf_tasks.letter_in_created_state', return_value=True)
|
||||
def test_group_letters_splits_on_file_count(notify_api):
|
||||
letters = [
|
||||
{'Key': 'A.pdf', 'Size': 1},
|
||||
{'Key': 'B.pdf', 'Size': 2},
|
||||
@@ -474,8 +470,7 @@ def test_group_letters_splits_on_file_count(notify_api, mocker):
|
||||
assert next(x, None) is None
|
||||
|
||||
|
||||
def test_group_letters_splits_on_file_size_and_file_count(notify_api, mocker):
|
||||
mocker.patch('app.celery.letters_pdf_tasks.letter_in_created_state', return_value=True)
|
||||
def test_group_letters_splits_on_file_size_and_file_count(notify_api):
|
||||
letters = [
|
||||
# ends under max file size but next file is too big
|
||||
{'Key': 'A.pdf', 'Size': 1},
|
||||
@@ -510,43 +505,14 @@ def test_group_letters_splits_on_file_size_and_file_count(notify_api, mocker):
|
||||
|
||||
|
||||
def test_group_letters_ignores_non_pdfs(notify_api, mocker):
|
||||
mocker.patch('app.celery.letters_pdf_tasks.letter_in_created_state', return_value=True)
|
||||
letters = [{'Key': 'A.zip'}]
|
||||
assert list(group_letters(letters)) == []
|
||||
|
||||
|
||||
def test_group_letters_ignores_notifications_already_sent(notify_api, mocker):
|
||||
mock = mocker.patch('app.celery.letters_pdf_tasks.letter_in_created_state', return_value=False)
|
||||
letters = [{'Key': 'A.pdf'}]
|
||||
assert list(group_letters(letters)) == []
|
||||
mock.assert_called_once_with('A.pdf')
|
||||
|
||||
|
||||
def test_group_letters_with_no_letters(notify_api, mocker):
|
||||
mocker.patch('app.celery.letters_pdf_tasks.letter_in_created_state', return_value=True)
|
||||
assert list(group_letters([])) == []
|
||||
|
||||
|
||||
def test_letter_in_created_state(sample_notification):
|
||||
sample_notification.reference = 'ABCDEF1234567890'
|
||||
filename = '2018-01-13/NOTIFY.ABCDEF1234567890.D.2.C.C.20180113120000.PDF'
|
||||
|
||||
assert letter_in_created_state(filename) is True
|
||||
|
||||
|
||||
def test_letter_in_created_state_fails_if_notification_not_in_created(sample_notification):
|
||||
sample_notification.reference = 'ABCDEF1234567890'
|
||||
sample_notification.status = NOTIFICATION_SENDING
|
||||
filename = '2018-01-13/NOTIFY.ABCDEF1234567890.D.2.C.C.20180113120000.PDF'
|
||||
assert letter_in_created_state(filename) is False
|
||||
|
||||
|
||||
def test_letter_in_created_state_fails_if_notification_doesnt_exist(sample_notification):
|
||||
sample_notification.reference = 'QWERTY1234567890'
|
||||
filename = '2018-01-13/NOTIFY.ABCDEF1234567890.D.2.C.C.20180113120000.PDF'
|
||||
assert letter_in_created_state(filename) is False
|
||||
|
||||
|
||||
@freeze_time('2018-01-01 18:00')
|
||||
@mock_s3
|
||||
@pytest.mark.parametrize('key_type,noti_status,bucket_config_name,destination_folder', [
|
||||
|
||||
Reference in New Issue
Block a user