Merge pull request #1828 from alphagov/refactor-post-letters

refactor post letters
This commit is contained in:
Rebecca Law
2018-04-09 16:13:54 +01:00
committed by GitHub
4 changed files with 95 additions and 134 deletions

View File

@@ -64,16 +64,16 @@ def get_reference_from_filename(filename):
return filename_parts[1]
def upload_letter_pdf(notification, pdf_data):
def upload_letter_pdf(notification, pdf_data, precompiled=False):
current_app.logger.info("PDF Letter {} reference {} created at {}, {} bytes".format(
notification.id, notification.reference, notification.created_at, len(pdf_data)))
upload_file_name = get_letter_pdf_filename(
notification.reference,
notification.service.crown,
is_scan_letter=notification.template.is_precompiled_letter)
is_scan_letter=precompiled)
if notification.template.is_precompiled_letter:
if precompiled:
bucket_name = current_app.config['LETTERS_SCAN_BUCKET_NAME']
else:
bucket_name = current_app.config['LETTERS_PDF_BUCKET_NAME']

View File

@@ -220,23 +220,17 @@ def process_letter_notification(*, letter_data, api_key, template, reply_to_text
if not api_key.service.research_mode and api_key.service.restricted and api_key.key_type != KEY_TYPE_TEST:
raise BadRequestError(message='Cannot send letters when service is in trial mode', status_code=403)
should_send = not (api_key.service.research_mode or api_key.key_type == KEY_TYPE_TEST)
if precompiled:
return process_precompiled_letter_notifications(letter_data=letter_data,
api_key=api_key,
template=template,
reply_to_text=reply_to_text)
should_send = not (api_key.key_type == KEY_TYPE_TEST)
# if we don't want to actually send the letter, then start it off in SENDING so we don't pick it up
status = NOTIFICATION_CREATED if should_send else NOTIFICATION_SENDING
if precompiled:
try:
if should_send or (precompiled and api_key.key_type == KEY_TYPE_TEST):
status = NOTIFICATION_PENDING_VIRUS_CHECK
letter_content = base64.b64decode(letter_data['content'])
pages = pdf_page_count(io.BytesIO(letter_content))
except ValueError:
raise BadRequestError(message='Cannot decode letter content (invalid base64 encoding)', status_code=400)
except PdfReadError:
current_app.logger.exception(msg='Invalid PDF received')
raise BadRequestError(message='Letter content is not a valid PDF', status_code=400)
notification = create_letter_notification(letter_data=letter_data,
template=template,
api_key=api_key,
@@ -244,45 +238,52 @@ def process_letter_notification(*, letter_data, api_key, template, reply_to_text
reply_to_text=reply_to_text)
if should_send:
if precompiled:
filename = upload_letter_pdf(notification, letter_content)
pages_per_sheet = 2
notification.billable_units = math.ceil(pages / pages_per_sheet)
dao_update_notification(notification)
current_app.logger.info(
'Calling task scan-file for {}'.format(filename)
)
# call task to add the filename to anti virus queue
notify_celery.send_task(
name=TaskNames.SCAN_FILE,
kwargs={'filename': filename},
queue=QueueNames.ANTIVIRUS,
)
else:
create_letters_pdf.apply_async(
[str(notification.id)],
queue=QueueNames.CREATE_LETTERS_PDF
)
elif (api_key.service.research_mode and
current_app.config['NOTIFY_ENVIRONMENT'] in ['preview', 'development']):
create_letters_pdf.apply_async(
[str(notification.id)],
queue=QueueNames.CREATE_LETTERS_PDF
)
elif current_app.config['NOTIFY_ENVIRONMENT'] in ['preview', 'development']:
create_fake_letter_response_file.apply_async(
(notification.reference,),
queue=QueueNames.RESEARCH_MODE
)
else:
if precompiled and api_key.key_type == KEY_TYPE_TEST:
filename = upload_letter_pdf(notification, letter_content)
update_notification_status_by_reference(notification.reference, NOTIFICATION_DELIVERED)
# call task to add the filename to anti virus queue
notify_celery.send_task(
name=TaskNames.SCAN_FILE,
kwargs={'filename': filename},
queue=QueueNames.ANTIVIRUS,
)
else:
update_notification_status_by_reference(notification.reference, NOTIFICATION_DELIVERED)
return notification
def process_precompiled_letter_notifications(*, letter_data, api_key, template, reply_to_text):
try:
status = NOTIFICATION_PENDING_VIRUS_CHECK
letter_content = base64.b64decode(letter_data['content'])
pages = pdf_page_count(io.BytesIO(letter_content))
except ValueError:
raise BadRequestError(message='Cannot decode letter content (invalid base64 encoding)', status_code=400)
except PdfReadError:
current_app.logger.exception(msg='Invalid PDF received')
raise BadRequestError(message='Letter content is not a valid PDF', status_code=400)
notification = create_letter_notification(letter_data=letter_data,
template=template,
api_key=api_key,
status=status,
reply_to_text=reply_to_text)
filename = upload_letter_pdf(notification, letter_content, precompiled=True)
pages_per_sheet = 2
notification.billable_units = math.ceil(pages / pages_per_sheet)
dao_update_notification(notification)
current_app.logger.info('Calling task scan-file for {}'.format(filename))
# call task to add the filename to anti virus queue
notify_celery.send_task(
name=TaskNames.SCAN_FILE,
kwargs={'filename': filename},
queue=QueueNames.ANTIVIRUS,
)
return notification

View File

@@ -128,7 +128,7 @@ def test_upload_letter_pdf_to_correct_bucket(
is_scan_letter=is_precompiled_letter
)
upload_letter_pdf(sample_letter_notification, b'\x00\x01')
upload_letter_pdf(sample_letter_notification, b'\x00\x01', precompiled=is_precompiled_letter)
mock_s3.assert_called_once_with(
bucket_name=current_app.config[bucket_config_name],

View File

@@ -96,12 +96,11 @@ def test_post_letter_notification_returns_201(client, sample_letter_template, mo
@pytest.mark.parametrize('env', [
'development',
'preview',
'staging',
'live',
])
def test_post_letter_notification_calls_create_fake_response_in_research_and_test_key_correct_env(
def test_post_letter_notification_with_test_key_set_status_to_delivered(
notify_api, client, sample_letter_template, mocker, env):
sample_letter_template.service.research_mode = True
data = {
'template_id': str(sample_letter_template.id),
@@ -126,83 +125,44 @@ def test_post_letter_notification_calls_create_fake_response_in_research_and_tes
notification = Notification.query.one()
assert not fake_create_letter_task.called
assert not fake_create_dvla_response_task.called
assert notification.status == NOTIFICATION_DELIVERED
@pytest.mark.parametrize('env', [
'development',
'preview',
])
def test_post_letter_notification_with_test_key_sets_status_to_sending_and_sends_fake_response_file(
notify_api, client, sample_letter_template, mocker, env):
data = {
'template_id': str(sample_letter_template.id),
'personalisation': {
'address_line_1': 'Her Royal Highness Queen Elizabeth II',
'address_line_2': 'Buckingham Palace',
'address_line_3': 'London',
'postcode': 'SW1 1AA',
'name': 'Lizzie'
},
'reference': 'foo'
}
fake_create_letter_task = mocker.patch('app.celery.letters_pdf_tasks.create_letters_pdf.apply_async')
fake_create_dvla_response_task = mocker.patch(
'app.celery.research_mode_tasks.create_fake_letter_response_file.apply_async')
with set_config_values(notify_api, {
'NOTIFY_ENVIRONMENT': env
}):
letter_request(client, data, service_id=sample_letter_template.service_id, key_type=KEY_TYPE_TEST)
notification = Notification.query.one()
assert not fake_create_letter_task.called
assert fake_create_dvla_response_task.called
assert notification.status == NOTIFICATION_SENDING
assert not fake_create_letter_task.called
fake_create_dvla_response_task.assert_called_once_with((notification.reference,), queue=QueueNames.RESEARCH_MODE)
@pytest.mark.parametrize('env', [
'staging',
'live',
])
def test_post_letter_notification_sets_status_delivered_in_research_and_test_key_incorrect_env(
notify_api, client, sample_letter_template, mocker, env):
sample_letter_template.service.research_mode = True
data = {
'template_id': str(sample_letter_template.id),
'personalisation': {
'address_line_1': 'Her Royal Highness Queen Elizabeth II',
'address_line_2': 'Buckingham Palace',
'address_line_3': 'London',
'postcode': 'SW1 1AA',
'name': 'Lizzie'
},
'reference': 'foo'
}
fake_create_letter_task = mocker.patch('app.celery.letters_pdf_tasks.create_letters_pdf.apply_async')
fake_create_dvla_response_task = mocker.patch(
'app.celery.research_mode_tasks.create_fake_letter_response_file.apply_async')
with set_config_values(notify_api, {
'NOTIFY_ENVIRONMENT': env
}):
letter_request(client, data, service_id=sample_letter_template.service_id, key_type=KEY_TYPE_TEST)
notification = Notification.query.one()
assert not fake_create_letter_task.called
assert not fake_create_dvla_response_task.called
assert notification.status == NOTIFICATION_DELIVERED
@pytest.mark.parametrize('env', [
'development',
'preview',
'staging',
'live',
])
def test_post_letter_notification_sets_status_to_delivered_using_test_key_and_not_research_all_env(
notify_api, client, sample_letter_template, mocker, env):
sample_letter_template.service.research_mode = False
data = {
'template_id': str(sample_letter_template.id),
'personalisation': {
'address_line_1': 'Her Royal Highness Queen Elizabeth II',
'address_line_2': 'Buckingham Palace',
'address_line_3': 'London',
'postcode': 'SW1 1AA',
'name': 'Lizzie'
},
'reference': 'foo'
}
fake_create_letter_task = mocker.patch('app.celery.letters_pdf_tasks.create_letters_pdf.apply_async')
fake_create_dvla_response_task = mocker.patch(
'app.celery.research_mode_tasks.create_fake_letter_response_file.apply_async')
with set_config_values(notify_api, {
'NOTIFY_ENVIRONMENT': env
}):
letter_request(client, data, service_id=sample_letter_template.service_id, key_type=KEY_TYPE_TEST)
notification = Notification.query.one()
assert not fake_create_letter_task.called
assert not fake_create_dvla_response_task.called
assert notification.status == NOTIFICATION_DELIVERED
def test_post_letter_notification_returns_400_and_missing_template(
@@ -234,11 +194,11 @@ def test_post_letter_notification_returns_400_for_empty_personalisation(
assert error_json['status_code'] == 400
assert all([e['error'] == 'ValidationError' for e in error_json['errors']])
assert set([e['message'] for e in error_json['errors']]) == set([
assert set([e['message'] for e in error_json['errors']]) == {
'personalisation address_line_1 is required',
'personalisation address_line_2 is required',
'personalisation postcode is required'
])
}
def test_notification_returns_400_for_missing_template_field(
@@ -415,7 +375,7 @@ def test_post_letter_notification_is_delivered_and_has_pdf_uploaded_to_test_lett
notification = Notification.query.one()
assert notification.status == NOTIFICATION_PENDING_VIRUS_CHECK
s3mock.assert_called_once_with(ANY, b'letter-content')
s3mock.assert_called_once_with(ANY, b'letter-content', precompiled=True)
mock_celery.assert_called_once_with(
name=TaskNames.SCAN_FILE,
kwargs={'filename': 'test.pdf'},
@@ -498,7 +458,7 @@ def test_post_precompiled_letter_notification_returns_201(client, notify_user, m
assert response.status_code == 201, response.get_data(as_text=True)
s3mock.assert_called_once_with(ANY, b'letter-content')
s3mock.assert_called_once_with(ANY, b'letter-content', precompiled=True)
notification = Notification.query.first()