mirror of
https://github.com/GSA/notifications-api.git
synced 2026-02-02 09:26:08 -05:00
Don't error sending a letter that's sent already
Fixes this error (in Admin):
File "/home/vcap/app/app/notify_client/notification_api_client.py", line 74, in send_precompiled_letter
return self.post(url='/service/{}/send-pdf-letter'.format(service_id), data=data)
File "/home/vcap/app/app/notify_client/__init__.py", line 59, in post
return super().post(*args, **kwargs)
File "/home/vcap/deps/0/python/lib/python3.9/site-packages/notifications_python_client/base.py", line 48, in post
return self.request("POST", url, data=data)
File "/home/vcap/deps/0/python/lib/python3.9/site-packages/notifications_python_client/base.py", line 64, in request
response = self._perform_request(method, url, kwargs)
File "/home/vcap/deps/0/python/lib/python3.9/site-packages/notifications_python_client/base.py", line 118, in _perform_request
raise api_error
notifications_python_client.errors.HTTPError: 500 - Internal server error
Due to this error (in API):
File "/home/vcap/app/app/service/send_notification.py", line 178, in send_pdf_letter_notification
raise e
File "/home/vcap/app/app/service/send_notification.py", line 173, in send_pdf_letter_notification
letter = utils_s3download(current_app.config['TRANSIENT_UPLOADED_LETTERS'], file_location)
File "/home/vcap/deps/0/python/lib/python3.9/site-packages/notifications_utils/s3.py", line 53, in s3download
raise S3ObjectNotFound(error.response, error.operation_name)
notifications_utils.s3.S3ObjectNotFound: An error occurred (NoSuchKey) when calling the GetObject operation: The specified key does not exist.
I checked the DB to verify the letter does actually exist i.e. it
is an instance of the problem we're fixing here.
This commit is contained in:
@@ -7,7 +7,10 @@ from sqlalchemy.orm.exc import NoResultFound
|
||||
|
||||
from app import create_random_identifier
|
||||
from app.config import QueueNames
|
||||
from app.dao.notifications_dao import _update_notification_status
|
||||
from app.dao.notifications_dao import (
|
||||
_update_notification_status,
|
||||
get_notification_by_id,
|
||||
)
|
||||
from app.dao.service_email_reply_to_dao import dao_get_reply_to_by_id
|
||||
from app.dao.service_sms_sender_dao import dao_get_service_sms_senders_by_id
|
||||
from app.dao.services_dao import dao_fetch_service_by_id
|
||||
@@ -172,9 +175,14 @@ def send_pdf_letter_notification(service_id, post_data):
|
||||
try:
|
||||
letter = utils_s3download(current_app.config['TRANSIENT_UPLOADED_LETTERS'], file_location)
|
||||
except S3ObjectNotFound as e:
|
||||
current_app.logger.exception('Letter {}.pdf not in transient {} bucket'.format(
|
||||
current_app.logger.warning('Letter {}.pdf not in transient {} bucket'.format(
|
||||
post_data['file_id'], current_app.config['TRANSIENT_UPLOADED_LETTERS'])
|
||||
)
|
||||
|
||||
# notification already exists e.g. if the user clicked send in different tabs
|
||||
if get_notification_by_id(post_data['file_id']):
|
||||
return {'id': str(post_data['file_id'])}
|
||||
|
||||
raise e
|
||||
|
||||
# Getting the page count won't raise an error since admin has already checked the PDF is valid
|
||||
|
||||
@@ -75,14 +75,41 @@ def test_send_pdf_letter_notification_raises_error_when_pdf_is_not_in_transient_
|
||||
notify_user,
|
||||
):
|
||||
user = sample_service_full_permissions.users[0]
|
||||
post_data = {'filename': 'valid.pdf', 'created_by': user.id, 'file_id': fake_uuid, 'postage': 'first',
|
||||
'recipient_address': 'Bugs%20Bunny%0A123%20Main%20Street%0ALooney%20Town'}
|
||||
post_data = {
|
||||
'filename':
|
||||
'valid.pdf',
|
||||
'created_by': user.id,
|
||||
'file_id': fake_uuid,
|
||||
'postage': 'first',
|
||||
'recipient_address': 'Bugs%20Bunny%0A123%20Main%20Street%0ALooney%20Town'
|
||||
}
|
||||
mocker.patch('app.service.send_notification.utils_s3download', side_effect=S3ObjectNotFound({}, ''))
|
||||
|
||||
with pytest.raises(S3ObjectNotFound):
|
||||
send_pdf_letter_notification(sample_service_full_permissions.id, post_data)
|
||||
|
||||
|
||||
def test_send_pdf_letter_notification_does_nothing_if_notification_already_exists(
|
||||
mocker,
|
||||
sample_service_full_permissions,
|
||||
fake_uuid,
|
||||
notify_user,
|
||||
sample_notification,
|
||||
):
|
||||
user = sample_service_full_permissions.users[0]
|
||||
post_data = {
|
||||
'filename':
|
||||
'valid.pdf',
|
||||
'created_by': user.id,
|
||||
'file_id': str(sample_notification.id),
|
||||
'postage': 'first',
|
||||
'recipient_address': 'Bugs%20Bunny%0A123%20Main%20Street%0ALooney%20Town'
|
||||
}
|
||||
mocker.patch('app.service.send_notification.utils_s3download', side_effect=S3ObjectNotFound({}, ''))
|
||||
response = send_pdf_letter_notification(sample_service_full_permissions.id, post_data)
|
||||
assert response['id'] == str(sample_notification.id)
|
||||
|
||||
|
||||
@freeze_time("2019-08-02 11:00:00")
|
||||
def test_send_pdf_letter_notification_creates_notification_and_moves_letter(
|
||||
mocker,
|
||||
|
||||
Reference in New Issue
Block a user