diff --git a/app/celery/tasks.py b/app/celery/tasks.py index 9650eb487..91997b2fd 100644 --- a/app/celery/tasks.py +++ b/app/celery/tasks.py @@ -58,7 +58,7 @@ from app.models import ( from app.notifications.process_notifications import persist_notification from app.serialised_models import SerialisedService, SerialisedTemplate from app.service.utils import service_allowed_to_send_to -from app.utils import DATETIME_FORMAT +from app.utils import DATETIME_FORMAT, get_reference_from_personalisation @notify_celery.task(name="process-job") @@ -383,6 +383,7 @@ def save_letter( job_row_number=notification['row_number'], notification_id=notification_id, reference=create_random_identifier(), + client_reference=get_reference_from_personalisation(notification['personalisation']), reply_to_text=template.reply_to_text, status=status ) diff --git a/app/notifications/process_notifications.py b/app/notifications/process_notifications.py index 4918da1e5..ed4334d29 100644 --- a/app/notifications/process_notifications.py +++ b/app/notifications/process_notifications.py @@ -121,7 +121,7 @@ def persist_notification( created_at=notification_created_at, job_id=job_id, job_row_number=job_row_number, - client_reference=client_reference or _get_reference_from_personalisation(personalisation), + client_reference=client_reference, reference=reference, created_by_id=created_by_id, status=status, @@ -159,12 +159,6 @@ def persist_notification( return notification -def _get_reference_from_personalisation(personalisation): - if personalisation: - return personalisation.get("reference") - return None - - def send_notification_to_queue_detached( key_type, notification_type, notification_id, research_mode, queue=None ): diff --git a/app/notifications/rest.py b/app/notifications/rest.py index 66ca9973d..15da4eb34 100644 --- a/app/notifications/rest.py +++ b/app/notifications/rest.py @@ -108,6 +108,7 @@ def send_notification(notification_type): if notification_type == SMS_TYPE: check_if_service_can_send_to_number(authenticated_service, notification_form['to']) + # Do not persist or send notification to the queue if it is a simulated recipient simulated = simulated_recipient(notification_form['to'], notification_type) notification_model = persist_notification(template_id=template.id, @@ -120,7 +121,7 @@ def send_notification(notification_type): api_key_id=api_user.id, key_type=api_user.key_type, simulated=simulated, - reply_to_text=template.reply_to_text + reply_to_text=template.reply_to_text, ) if not simulated: queue_name = QueueNames.PRIORITY if template.process_type == PRIORITY else None diff --git a/app/service/send_notification.py b/app/service/send_notification.py index f6d7e5e5b..357aa8db0 100644 --- a/app/service/send_notification.py +++ b/app/service/send_notification.py @@ -81,12 +81,16 @@ def send_one_off_notification(service_id, post_data): allow_guest_list_recipients=False, ) postage = None + client_reference = None if template.template_type == LETTER_TYPE: # Validate address and set postage to europe|rest-of-world if international letter, # otherwise persist_notification with use template postage postage = validate_address(service, personalisation) if not postage: postage = template.postage + from app.utils import get_reference_from_personalisation + client_reference = get_reference_from_personalisation(personalisation) + validate_created_by(service, post_data['created_by']) sender_id = post_data.get('sender_id', None) @@ -108,7 +112,8 @@ def send_one_off_notification(service_id, post_data): created_by_id=post_data['created_by'], reply_to_text=reply_to, reference=create_one_off_reference(template.template_type), - postage=postage + postage=postage, + client_reference=client_reference ) queue_name = QueueNames.PRIORITY if template.process_type == PRIORITY else None diff --git a/app/utils.py b/app/utils.py index 6ba170824..d2cf06181 100644 --- a/app/utils.py +++ b/app/utils.py @@ -160,3 +160,9 @@ def get_uuid_string_or_none(val): def format_sequential_number(sequential_number): return format(sequential_number, "x").zfill(8) + + +def get_reference_from_personalisation(personalisation): + if personalisation: + return personalisation.get("reference") + return None diff --git a/tests/app/celery/test_tasks.py b/tests/app/celery/test_tasks.py index 4caab5e60..ba0443bad 100644 --- a/tests/app/celery/test_tasks.py +++ b/tests/app/celery/test_tasks.py @@ -1070,6 +1070,38 @@ def test_save_letter_saves_letter_to_database_with_correct_postage( assert notification_db.international == expected_international +@pytest.mark.parametrize('reference_paceholder,', [None, 'ref2']) +def test_save_letter_saves_letter_to_database_with_correct_client_reference( + mocker, notify_db_session, reference_paceholder +): + service = create_service(service_permissions=[LETTER_TYPE]) + template = create_template(service=service, template_type=LETTER_TYPE) + letter_job = create_job(template=template) + + personalisation = {'addressline1': 'Foo', 'addressline2': 'Bar', 'postcode': 'SW1A 1AA'} + if reference_paceholder: + personalisation['reference'] = reference_paceholder + + mocker.patch('app.celery.tasks.letters_pdf_tasks.get_pdf_for_templated_letter.apply_async') + notification_json = _notification_json( + template=letter_job.template, + to='Foo', + personalisation=personalisation, + job_id=letter_job.id, + row_number=1 + ) + notification_id = uuid.uuid4() + save_letter( + letter_job.service_id, + notification_id, + encryption.encrypt(notification_json), + ) + + notification_db = Notification.query.one() + assert notification_db.id == notification_id + assert notification_db.client_reference == reference_paceholder + + def test_save_letter_saves_letter_to_database_with_formatted_postcode(mocker, notify_db_session): service = create_service(service_permissions=[LETTER_TYPE]) template = create_template(service=service, template_type=LETTER_TYPE) diff --git a/tests/app/notifications/rest/test_send_notification.py b/tests/app/notifications/rest/test_send_notification.py index f13398d9e..dff771abc 100644 --- a/tests/app/notifications/rest/test_send_notification.py +++ b/tests/app/notifications/rest/test_send_notification.py @@ -1249,3 +1249,27 @@ def test_send_notification_should_send_international_letters( notification = Notification.query.get(notification_id['id']) assert notification.postage == expected_postage assert notification.international == expected_international + + +@pytest.mark.parametrize('reference_paceholder,', [None, 'ref2']) +def test_send_notification_should_set_client_reference_from_placeholder( + sample_letter_template, mocker, reference_paceholder +): + deliver_mock = mocker.patch('app.celery.tasks.letters_pdf_tasks.get_pdf_for_templated_letter.apply_async') + data = { + 'template_id': sample_letter_template.id, + 'personalisation': { + 'address_line_1': 'Jane', + 'address_line_2': 'Moss Lane', + 'address_line_3': 'SW1A 1AA', + }, + 'to': 'Jane', + 'created_by': sample_letter_template.service.created_by_id + } + if reference_paceholder: + data['personalisation']['reference'] = reference_paceholder + + notification_id = send_one_off_notification(sample_letter_template.service_id, data) + assert deliver_mock.called + notification = Notification.query.get(notification_id['id']) + assert notification.client_reference == reference_paceholder diff --git a/tests/app/notifications/test_process_notification.py b/tests/app/notifications/test_process_notification.py index 3321f74ba..7b22282fa 100644 --- a/tests/app/notifications/test_process_notification.py +++ b/tests/app/notifications/test_process_notification.py @@ -96,42 +96,6 @@ def test_persist_notification_creates_and_save_to_db(sample_template, sample_api assert notification_from_db.reply_to_text == sample_template.service.get_default_sms_sender() -@pytest.mark.parametrize('client_reference,personalisation,expected_client_reference', [ - ('clientref1', {}, 'clientref1'), - ('clientref1', {'reference': 'ref2'}, 'clientref1'), - (None, {'reference': 'ref2'}, 'ref2'), - (None, {'foo': 'bar'}, None), -]) -def test_persist_notification_sets_client_reference_correctly( - sample_letter_template, sample_api_key, client_reference, personalisation, expected_client_reference -): - - assert Notification.query.count() == 0 - assert NotificationHistory.query.count() == 0 - notification = persist_notification( - template_id=sample_letter_template.id, - template_version=sample_letter_template.version, - recipient='3 Somerset House', - service=sample_letter_template.service, - personalisation=personalisation, - notification_type='letter', - api_key_id=sample_api_key.id, - key_type=sample_api_key.key_type, - reference="ref", - client_reference=client_reference, - ) - - assert Notification.query.get(notification.id) is not None - - notification_from_db = Notification.query.one() - - assert notification_from_db.id == notification.id - assert notification_from_db.template_id == notification.template_id - assert notification_from_db.notification_type == notification.notification_type - assert notification_from_db.reference == notification.reference - assert notification_from_db.client_reference == expected_client_reference - - def test_persist_notification_throws_exception_when_missing_template(sample_api_key): assert Notification.query.count() == 0 assert NotificationHistory.query.count() == 0 diff --git a/tests/app/service/test_send_one_off_notification.py b/tests/app/service/test_send_one_off_notification.py index 1fe29292f..857f8d700 100644 --- a/tests/app/service/test_send_one_off_notification.py +++ b/tests/app/service/test_send_one_off_notification.py @@ -100,7 +100,8 @@ def test_send_one_off_notification_calls_persist_correctly_for_sms( created_by_id=str(service.created_by_id), reply_to_text='testing', reference=None, - postage=None + postage=None, + client_reference=None ) @@ -161,7 +162,8 @@ def test_send_one_off_notification_calls_persist_correctly_for_email( created_by_id=str(service.created_by_id), reply_to_text=None, reference=None, - postage=None + postage=None, + client_reference=None ) @@ -210,7 +212,8 @@ def test_send_one_off_notification_calls_persist_correctly_for_letter( created_by_id=str(service.created_by_id), reply_to_text=None, reference='this-is-random-in-real-life', - postage='first' + postage='first', + client_reference=None )