From 0b61019a267dceef7c3e32eaafc4d1a1c7bb75e6 Mon Sep 17 00:00:00 2001 From: Kyle MacMillan <16893311+k-macmillan@users.noreply.github.com> Date: Fri, 27 Dec 2024 12:21:59 -0500 Subject: [PATCH] #2205 - Comp and Pen Batches (#2206) --- app/celery/process_comp_and_pen.py | 119 ++++++++ app/celery/scheduled_tasks.py | 69 ----- app/config.py | 7 +- app/feature_flags.py | 1 - .../comp_and_pen/scheduled_message_helpers.py | 205 ------------- app/notifications/send_notifications.py | 9 +- cd/application-deployment/dev/dev.env | 1 - cd/application-deployment/perf/perf.env | 1 - .../vaec-celery-beat-task-definition.json | 4 - cd/application-deployment/prod/prod.env | 1 - cd/application-deployment/staging/staging.env | 1 - tests/app/celery/test_process_comp_and_pen.py | 78 +++++ tests/app/celery/test_scheduled_tasks.py | 155 +--------- .../test_scheduled_message_helpers.py | 287 ------------------ .../notifications/test_send_notifications.py | 5 - 15 files changed, 203 insertions(+), 740 deletions(-) create mode 100644 app/celery/process_comp_and_pen.py delete mode 100644 app/integrations/comp_and_pen/scheduled_message_helpers.py create mode 100644 tests/app/celery/test_process_comp_and_pen.py delete mode 100644 tests/app/integrations/comp_and_pen/test_scheduled_message_helpers.py diff --git a/app/celery/process_comp_and_pen.py b/app/celery/process_comp_and_pen.py new file mode 100644 index 0000000000..895c4f563f --- /dev/null +++ b/app/celery/process_comp_and_pen.py @@ -0,0 +1,119 @@ +from dataclasses import dataclass +from uuid import uuid4 + +from flask import current_app +from sqlalchemy.orm.exc import NoResultFound + +from notifications_utils.statsd_decorators import statsd + +from app import notify_celery +from app.dao.service_sms_sender_dao import dao_get_service_sms_sender_by_id +from app.models import ( + Service, + Template, +) +from app.notifications.send_notifications import lookup_notification_sms_setup_data, send_notification_bypass_route +from app.va.identifier import IdentifierType + + +@dataclass +class DynamoRecord: + participant_id: str + payment_amount: str + vaprofile_id: str + + +@notify_celery.task(name='comp-and-pen-batch-process') +@statsd(namespace='tasks') +def comp_and_pen_batch_process(records: list[dict[str, str]]) -> None: + """Process batches of Comp and Pen notification requests. + + Args: + records (list[dict[str, str]]): The incoming records + """ + current_app.logger.debug(f'comp_and_pen_batch_process records: {records}') + + # Grab all the necessary data + try: + service, template, sms_sender_id = lookup_notification_sms_setup_data( + current_app.config['COMP_AND_PEN_SERVICE_ID'], + current_app.config['COMP_AND_PEN_TEMPLATE_ID'], + current_app.config['COMP_AND_PEN_SMS_SENDER_ID'], + ) + reply_to_text = dao_get_service_sms_sender_by_id(service.id, sms_sender_id).sms_sender + except (AttributeError, NoResultFound, ValueError): + current_app.logger.exception('Unable to send comp and pen notifications due to improper configuration') + raise + + _send_comp_and_pen_sms( + service, + template, + sms_sender_id, + reply_to_text, + [DynamoRecord(**item) for item in records], + current_app.config['COMP_AND_PEN_PERF_TO_NUMBER'], + ) + + +def _send_comp_and_pen_sms( + service: Service, + template: Template, + sms_sender_id: str, + reply_to_text: str, + comp_and_pen_messages: list[DynamoRecord], + perf_to_number: str, +) -> None: + """ + Sends scheduled SMS notifications to recipients based on the provided parameters. + + Args: + :param service (Service): The service used to send the SMS notifications. + :param template (Template): The template used for the SMS notifications. + :param sms_sender_id (str): The ID of the SMS sender. + :param reply_to_text (str): The text a Veteran can reply to. + :param comp_and_pen_messages (list[DynamoRecord]): A list of DynamoRecord from the dynamodb table containing + the details needed to send the messages. + :param perf_to_number (str): The recipient's phone number. + + Raises: + Exception: If there is an error while sending the SMS notification. + """ + + for item in comp_and_pen_messages: + current_app.logger.debug('sending - record from dynamodb: %s', item.participant_id) + + # Use perf_to_number as the recipient if available. Otherwise, use vaprofile_id as recipient_item. + recipient_item = ( + None + if perf_to_number is not None + else { + 'id_type': IdentifierType.VA_PROFILE_ID.value, + 'id_value': item.vaprofile_id, + } + ) + + try: + # call generic method to send messages + send_notification_bypass_route( + service=service, + template=template, + reply_to_text=reply_to_text, + personalisation={'amount': item.payment_amount}, + sms_sender_id=sms_sender_id, + recipient=perf_to_number, + recipient_item=recipient_item, + notification_id=uuid4(), + ) + except Exception: + current_app.logger.exception( + 'Error attempting to send Comp and Pen notification with ' + 'send_comp_and_pen_sms | record from dynamodb: %s', + item.participant_id, + ) + else: + if perf_to_number is not None: + current_app.logger.info( + 'Notification sent using Perf simulated number %s instead of vaprofile_id', perf_to_number + ) + + current_app.logger.info('Notification sent to queue for record from dynamodb: %s', item.participant_id) diff --git a/app/celery/scheduled_tasks.py b/app/celery/scheduled_tasks.py index 1331036fe3..b06e0bf959 100644 --- a/app/celery/scheduled_tasks.py +++ b/app/celery/scheduled_tasks.py @@ -1,7 +1,5 @@ from datetime import datetime, timedelta -from time import monotonic -from botocore.exceptions import ClientError from flask import current_app from notifications_utils.statsd_decorators import statsd from sqlalchemy import and_, select @@ -28,11 +26,8 @@ dao_old_letters_with_created_status, ) from app.dao.users_dao import delete_codes_older_created_more_than_a_day_ago -from app.feature_flags import is_feature_enabled, FeatureFlag -from app.integrations.comp_and_pen.scheduled_message_helpers import CompPenMsgHelper from app.models import Job from app.notifications.process_notifications import send_notification_to_queue -from app.notifications.send_notifications import lookup_notification_sms_setup_data from app.v2.errors import JobIncompleteError @@ -196,67 +191,3 @@ def check_templated_letter_state(): message=msg, ticket_type=zendesk_client.TYPE_INCIDENT, ) - - -@notify_celery.task(name='send-scheduled-comp-and-pen-sms') -@statsd(namespace='tasks') -def send_scheduled_comp_and_pen_sms() -> None: - start_time = monotonic() - # this is the agreed upon message per 1 minute limit - messages_per_min = 90 - - # get config info - dynamodb_table_name = current_app.config['COMP_AND_PEN_DYNAMODB_TABLE_NAME'] - service_id = current_app.config['COMP_AND_PEN_SERVICE_ID'] - template_id = current_app.config['COMP_AND_PEN_TEMPLATE_ID'] - sms_sender_id = current_app.config['COMP_AND_PEN_SMS_SENDER_ID'] - # Perf uses the AWS simulated delivered number - perf_to_number = current_app.config['COMP_AND_PEN_PERF_TO_NUMBER'] - - comp_pen_helper = CompPenMsgHelper(dynamodb_table_name=dynamodb_table_name) - - current_app.logger.debug('send_scheduled_comp_and_pen_sms connecting to dynamodb...') - try: - comp_pen_helper._connect_to_dynamodb() - except ClientError as e: - current_app.logger.critical( - 'Unable to connect to dynamodb table with name %s - exception: %s', dynamodb_table_name, e - ) - raise - - current_app.logger.debug('... connected to dynamodb in send_scheduled_comp_and_pen_sms') - current_app.logger.info('dynamo connection took: %s seconds', monotonic() - start_time) - - # get messages to send - try: - comp_and_pen_messages: list = comp_pen_helper.get_dynamodb_comp_pen_messages(messages_per_min) - except Exception as e: - current_app.logger.critical( - 'Exception trying to scan dynamodb table for send_scheduled_comp_and_pen_sms exception_type: %s - ' - 'exception_message: %s', - type(e), - e, - ) - raise - - current_app.logger.debug('send_scheduled_comp_and_pen_sms list of items from dynamodb: %s', comp_and_pen_messages) - - # only continue if there are messages to update and send - if comp_and_pen_messages: - comp_pen_helper.remove_dynamo_item_is_processed(comp_and_pen_messages) - - if is_feature_enabled(FeatureFlag.COMP_AND_PEN_MESSAGES_ENABLED): - # get the data necessary to send the notifications - service, template, sms_sender_id = lookup_notification_sms_setup_data( - service_id, template_id, sms_sender_id - ) - - comp_pen_helper.send_comp_and_pen_sms( - service=service, - template=template, - sms_sender_id=sms_sender_id, - comp_and_pen_messages=comp_and_pen_messages, - perf_to_number=perf_to_number, - ) - else: - current_app.logger.info('Comp and Pen Notifications not sent to queue (feature flag disabled)') diff --git a/app/config.py b/app/config.py index 6e3f8e8930..84273ff622 100644 --- a/app/config.py +++ b/app/config.py @@ -226,6 +226,7 @@ class Config(object): 'task_serializer': 'json', 'imports': ( 'app.celery.tasks', + 'app.celery.process_comp_and_pen', 'app.celery.scheduled_tasks', 'app.celery.reporting_tasks', 'app.celery.nightly_tasks', @@ -316,12 +317,6 @@ class Config(object): 'schedule': crontab(hour=4, minute=0), 'options': {'queue': QueueNames.PERIODIC}, }, - 'send-scheduled-comp-and-pen-sms': { - 'task': 'send-scheduled-comp-and-pen-sms', - # Every 1 minute past every hour from 13 through 21 on every day-of-month from 22 through end-of-month - 'schedule': crontab(hour='13-21', day_of_month='23-31', minute='*/1'), - 'options': {'queue': QueueNames.PERIODIC}, - }, 'update-twilio-status': { 'task': 'update-twilio-status', 'schedule': crontab(hour='*', minute='*/5'), diff --git a/app/feature_flags.py b/app/feature_flags.py index 156c43c0c0..c8a48ff0d9 100644 --- a/app/feature_flags.py +++ b/app/feature_flags.py @@ -20,7 +20,6 @@ class FeatureFlag(Enum): PLATFORM_STATS_ENABLED = 'PLATFORM_STATS_ENABLED' VA_SSO_ENABLED = 'VA_SSO_ENABLED' V3_ENABLED = 'V3_ENABLED' - COMP_AND_PEN_MESSAGES_ENABLED = 'COMP_AND_PEN_MESSAGES_ENABLED' VA_PROFILE_SMS_STATUS_ENABLED = 'VA_PROFILE_SMS_STATUS_ENABLED' diff --git a/app/integrations/comp_and_pen/scheduled_message_helpers.py b/app/integrations/comp_and_pen/scheduled_message_helpers.py deleted file mode 100644 index 113ad78d81..0000000000 --- a/app/integrations/comp_and_pen/scheduled_message_helpers.py +++ /dev/null @@ -1,205 +0,0 @@ -from time import monotonic -from uuid import uuid4 - -import boto3 -from boto3.dynamodb.conditions import Attr -from flask import current_app -from sqlalchemy.orm.exc import NoResultFound - -from app.constants import SMS_TYPE -from app.dao.service_sms_sender_dao import dao_get_service_sms_sender_by_id -from app.models import ( - Service, - Template, -) -from app.notifications.send_notifications import send_notification_bypass_route -from app.va.identifier import IdentifierType - - -class CompPenMsgHelper: - dynamodb_table = None - - def __init__(self, dynamodb_table_name: str) -> None: - """ - This class is a collection of helper methods to facilitate the delivery of schedule Comp and Pen notifications. - - :param dynamodb_table_name (str): the name of the dynamodb table for the db operations, required - """ - self.dynamodb_table_name = dynamodb_table_name - - def _connect_to_dynamodb(self, dynamodb_table_name: str = None) -> None: - """Establishes a connection to the dynamodb table with the given name. - - :param dynamodb_table_name (str): the name of the dynamodb table to establish a connection with - - Raises: - ClientError: if it has trouble connectingto the dynamodb - """ - start_time = monotonic() - if dynamodb_table_name is None: - dynamodb_table_name = self.dynamodb_table_name - - # connect to dynamodb table - dynamodb_resource = boto3.resource('dynamodb') - self.dynamodb_table = dynamodb_resource.Table(dynamodb_table_name) - current_app.logger.info('dynamodb connection took: %s seconds', monotonic() - start_time) - - def get_dynamodb_comp_pen_messages( - self, - message_limit: int, - ) -> list: - """ - Helper function to get the Comp and Pen data from our dynamodb cache table. - - Items should be returned if all of these attribute conditions are met: - 1) item exists on the `is-processed-index` - 2) paymentAmount exists - - :param message_limit: the number of rows to search at a time and the max number of items that should be returned - :return: a list of entries from the table that have not been processed yet - - https://boto3.amazonaws.com/v1/documentation/api/latest/guide/dynamodb.html#querying-and-scanning - """ - - start_time = monotonic() - if self.dynamodb_table is None: - self._connect_to_dynamodb() - - is_processed_index = 'is-processed-index' - - filters = Attr('paymentAmount').exists() - - results = self.dynamodb_table.scan(FilterExpression=filters, Limit=message_limit, IndexName=is_processed_index) - items: list = results.get('Items') - - if items is None: - items = [] - current_app.logger.critical( - 'Error in get_dynamodb_comp_pen_messages trying to read "Items" from dynamodb table scan result. ' - 'Returned results does not include "Items" - results: %s', - results, - ) - - # Keep getting items from the table until we have the number we want to send, or run out of items - while 'LastEvaluatedKey' in results and len(items) < message_limit: - results = self.dynamodb_table.scan( - FilterExpression=filters, - Limit=message_limit, - IndexName=is_processed_index, - ExclusiveStartKey=results['LastEvaluatedKey'], - ) - - items.extend(results['Items']) - current_app.logger.info('get_dynamodb_comp_pen_messages took: %s seconds', monotonic() - start_time) - return items[:message_limit] - - def remove_dynamo_item_is_processed(self, comp_and_pen_messages: list) -> None: - """ - Remove the 'is_processed' key from each item in the provided list and update the entries in the DynamoDB table. - - :param comp_and_pen_messages (list): A list of dictionaries, where each dictionary contains the data for an item - to be updated in the DynamoDB table. Each dictionary should at least contain 'participant_id' and - 'payment_id' keys, as well as the 'is_processed' key to be removed. - - Raises: - Exception: If an error occurs during the update of any item in the DynamoDB table, the exception is logged - with critical severity, and then re-raised. - """ - start_time = monotonic() - if self.dynamodb_table is None: - self._connect_to_dynamodb() - - # send messages and update entries in dynamodb table - with self.dynamodb_table.batch_writer() as batch: - for item in comp_and_pen_messages: - participant_id = item.get('participant_id') - - item.pop('is_processed', None) - - # update dynamodb entries - try: - batch.put_item(Item=item) - current_app.logger.debug( - 'updated record from dynamodb ("is_processed" should no longer exist): %s', item - ) - except Exception: - current_app.logger.exception('Failed to update the record from dynamodb: %s', participant_id) - - current_app.logger.info( - 'Comp and Pen - Successfully updated dynamodb entries - removed "is_processed" field in %s seconds', - monotonic() - start_time, - ) - - def send_comp_and_pen_sms( - self, - service: Service, - template: Template, - sms_sender_id: str, - comp_and_pen_messages: list[dict], - perf_to_number: str, - ) -> None: - """ - Sends scheduled SMS notifications to recipients based on the provided parameters. - - Args: - :param service (Service): The service used to send the SMS notifications. - :param template (Template): The template used for the SMS notifications. - :param sms_sender_id (str): The ID of the SMS sender. - :param comp_and_pen_messages (list[dict]): A list of dictionaries from the dynamodb table containing the - details needed to send the messages. - :param perf_to_number (str): The recipient's phone number. - - Raises: - Exception: If there is an error while sending the SMS notification. - """ - try: - reply_to_text = dao_get_service_sms_sender_by_id(service.id, sms_sender_id).sms_sender - except (NoResultFound, AttributeError): - current_app.logger.exception('Unable to send comp and pen notifications due to improper sms_sender') - raise - - for item in comp_and_pen_messages: - vaprofile_id = str(item.get('vaprofile_id')) - participant_id = item.get('participant_id') - # Format payment amount as str with appropriate commas - payment_amount = f'{item.get("paymentAmount", 0):0,.2f}' - - current_app.logger.debug('sending - record from dynamodb: %s', participant_id) - - # Use perf_to_number as the recipient if available. Otherwise, use vaprofile_id as recipient_item. - recipient = perf_to_number - recipient_item = ( - None - if perf_to_number is not None - else { - 'id_type': IdentifierType.VA_PROFILE_ID.value, - 'id_value': vaprofile_id, - } - ) - - try: - # call generic method to send messages - send_notification_bypass_route( - service=service, - template=template, - notification_type=SMS_TYPE, - reply_to_text=reply_to_text, - personalisation={'amount': payment_amount}, - sms_sender_id=sms_sender_id, - recipient=recipient, - recipient_item=recipient_item, - notification_id=uuid4(), - ) - except Exception: - current_app.logger.exception( - 'Error attempting to send Comp and Pen notification with ' - 'send_comp_and_pen_sms | record from dynamodb: %s', - participant_id, - ) - else: - if perf_to_number is not None: - current_app.logger.info( - 'Notification sent using Perf simulated number %s instead of vaprofile_id', perf_to_number - ) - - current_app.logger.info('Notification sent to queue for record from dynamodb: %s', participant_id) diff --git a/app/notifications/send_notifications.py b/app/notifications/send_notifications.py index b9099084df..3c39072132 100644 --- a/app/notifications/send_notifications.py +++ b/app/notifications/send_notifications.py @@ -54,7 +54,6 @@ def lookup_notification_sms_setup_data( def send_notification_bypass_route( service: Service, template: Template, - notification_type: str, reply_to_text: str | None, recipient: str = None, personalisation: dict = None, @@ -105,7 +104,7 @@ def send_notification_bypass_route( ) # Use the service's default sms_sender if applicable - if notification_type == SMS_TYPE and sms_sender_id is None: + if template.template_type == SMS_TYPE and sms_sender_id is None: sms_sender_id = service.get_default_sms_sender_id() start_time = monotonic() @@ -115,7 +114,7 @@ def send_notification_bypass_route( recipient=recipient, service_id=service.id, personalisation=personalisation, - notification_type=notification_type, + notification_type=template.template_type, api_key_id=None, key_type=api_key_type, recipient_identifier=recipient_item, @@ -129,7 +128,7 @@ def send_notification_bypass_route( current_app.logger.info( 'sending %s notification with send_notification_bypass_route via ' 'send_to_queue_for_recipient_info_based_on_recipient_identifier, notification id %s', - notification_type, + template.template_type, notification.id, ) @@ -146,7 +145,7 @@ def send_notification_bypass_route( current_app.logger.info( 'sending %s notification with send_notification_bypass_route via send_notification_to_queue, ' 'notification id %s', - notification_type, + template.template_type, notification.id, ) diff --git a/cd/application-deployment/dev/dev.env b/cd/application-deployment/dev/dev.env index 8a676b80bf..7a815307c2 100644 --- a/cd/application-deployment/dev/dev.env +++ b/cd/application-deployment/dev/dev.env @@ -8,7 +8,6 @@ AWS_SES_EMAIL_FROM_USER=dev-do-not-reply CHECK_GITHUB_SCOPE_ENABLED=True CHECK_TEMPLATE_NAME_EXISTS_ENABLED=True COMP_AND_PEN_DYNAMODB_NAME=dev-bip-payment-notification-table -COMP_AND_PEN_MESSAGES_ENABLED=True DD_ENV=dev DD_PROFILING_ENABLED=True DD_PROFILING_ENABLE_CODE_PROVENANCE=True diff --git a/cd/application-deployment/perf/perf.env b/cd/application-deployment/perf/perf.env index f3212769b6..840a9b3795 100644 --- a/cd/application-deployment/perf/perf.env +++ b/cd/application-deployment/perf/perf.env @@ -6,7 +6,6 @@ AWS_SES_EMAIL_FROM_USER=perf-do-not-reply CHECK_GITHUB_SCOPE_ENABLED=False CHECK_TEMPLATE_NAME_EXISTS_ENABLED=False COMP_AND_PEN_DYNAMODB_NAME=perf-bip-payment-notification-table -COMP_AND_PEN_MESSAGES_ENABLED=True COMP_AND_PEN_PERF_TO_NUMBER=+14254147755 DD_ENV=perf DD_PROFILING_ENABLED=True diff --git a/cd/application-deployment/perf/vaec-celery-beat-task-definition.json b/cd/application-deployment/perf/vaec-celery-beat-task-definition.json index 72a8f7440f..1e385e31b6 100644 --- a/cd/application-deployment/perf/vaec-celery-beat-task-definition.json +++ b/cd/application-deployment/perf/vaec-celery-beat-task-definition.json @@ -163,10 +163,6 @@ "name": "PLATFORM_STATS_ENABLED", "value": "False" }, - { - "name": "COMP_AND_PEN_MESSAGES_ENABLED", - "value": "True" - }, { "name": "COMP_AND_PEN_DYNAMODB_NAME", "value": "perf-bip-payment-notification-table" diff --git a/cd/application-deployment/prod/prod.env b/cd/application-deployment/prod/prod.env index 2d01e8f024..666eed4cdb 100644 --- a/cd/application-deployment/prod/prod.env +++ b/cd/application-deployment/prod/prod.env @@ -6,7 +6,6 @@ AWS_SES_EMAIL_FROM_USER=do-not-reply CHECK_GITHUB_SCOPE_ENABLED=False CHECK_TEMPLATE_NAME_EXISTS_ENABLED=False COMP_AND_PEN_DYNAMODB_NAME=prod-bip-payment-notification-table -COMP_AND_PEN_MESSAGES_ENABLED=True DD_ENV=prod DD_PROFILING_ENABLED=True DD_PROFILING_ENABLE_CODE_PROVENANCE=True diff --git a/cd/application-deployment/staging/staging.env b/cd/application-deployment/staging/staging.env index e816336ad7..2e4fe05ef6 100644 --- a/cd/application-deployment/staging/staging.env +++ b/cd/application-deployment/staging/staging.env @@ -8,7 +8,6 @@ AWS_SES_EMAIL_FROM_USER=staging-do-not-reply CHECK_GITHUB_SCOPE_ENABLED=False CHECK_TEMPLATE_NAME_EXISTS_ENABLED=False COMP_AND_PEN_DYNAMODB_NAME=staging-bip-payment-notification-table -COMP_AND_PEN_MESSAGES_ENABLED=True DD_ENV=staging DD_PROFILING_ENABLED=True DD_PROFILING_ENABLE_CODE_PROVENANCE=True diff --git a/tests/app/celery/test_process_comp_and_pen.py b/tests/app/celery/test_process_comp_and_pen.py new file mode 100644 index 0000000000..f3bf14c45f --- /dev/null +++ b/tests/app/celery/test_process_comp_and_pen.py @@ -0,0 +1,78 @@ +from unittest.mock import patch + +import pytest +from sqlalchemy.orm.exc import NoResultFound + +from app.celery.process_comp_and_pen import comp_and_pen_batch_process +from app.exceptions import NotificationTechnicalFailureException + + +def test_comp_and_pen_batch_process_happy_path(mocker, sample_template) -> None: + template = sample_template() + mocker.patch( + 'app.celery.process_comp_and_pen.lookup_notification_sms_setup_data', + return_value=(template.service, template, str(template.service.get_default_sms_sender_id())), + ) + mock_send = mocker.patch( + 'app.notifications.send_notifications.send_to_queue_for_recipient_info_based_on_recipient_identifier' + ) + + records = [ + {'participant_id': '55', 'payment_amount': '55.56', 'vaprofile_id': '57'}, + {'participant_id': '42', 'payment_amount': '42.42', 'vaprofile_id': '43627'}, + ] + comp_and_pen_batch_process(records) + + # comp_and_pen_batch_process can fail without raising an exception, so test it called send_to_queue_for_recipient... + mock_send.call_count == len(records) + + +def test_comp_and_pen_batch_process_perf_number_happy_path(mocker, sample_template) -> None: + template = sample_template() + mocker.patch( + 'app.celery.process_comp_and_pen.lookup_notification_sms_setup_data', + return_value=(template.service, template, str(template.service.get_default_sms_sender_id())), + ) + mock_send = mocker.patch('app.notifications.send_notifications.send_notification_to_queue') + + records = [ + {'participant_id': '55', 'payment_amount': '55.56', 'vaprofile_id': '57'}, + {'participant_id': '42', 'payment_amount': '42.42', 'vaprofile_id': '43627'}, + ] + with patch.dict( + 'app.celery.process_comp_and_pen.current_app.config', {'COMP_AND_PEN_PERF_TO_NUMBER': '8888675309'} + ): + comp_and_pen_batch_process(records) + + # comp_and_pen_batch_process can fail without raising an exception, so test it called send_notification_to_queue + mock_send.call_count == len(records) + + +@pytest.mark.parametrize('exception_tested', [AttributeError, NoResultFound, ValueError]) +def test_comp_and_pen_batch_process_exception(mocker, exception_tested) -> None: + mocker.patch( + 'app.celery.process_comp_and_pen.lookup_notification_sms_setup_data', + side_effect=exception_tested, + ) + + with pytest.raises(exception_tested): + comp_and_pen_batch_process({}) + + +def test_comp_and_pen_batch_process_bypass_exception(mocker, sample_template) -> None: + template = sample_template() + mocker.patch( + 'app.celery.process_comp_and_pen.lookup_notification_sms_setup_data', + return_value=(template.service, template, str(template.service.get_default_sms_sender_id())), + ) + mocker.patch( + 'app.celery.process_comp_and_pen.send_notification_bypass_route', + side_effect=NotificationTechnicalFailureException, + ) + mock_logger = mocker.patch('app.celery.process_comp_and_pen.current_app.logger') + + comp_and_pen_batch_process([{'participant_id': '55', 'payment_amount': '55.56', 'vaprofile_id': '57'}]) + + # comp_and_pen_batch_process can fail without raising an exception + mock_logger.exception.assert_called_once() + mock_logger.info.assert_not_called() diff --git a/tests/app/celery/test_scheduled_tasks.py b/tests/app/celery/test_scheduled_tasks.py index 3fcd240704..302f3d5f8a 100644 --- a/tests/app/celery/test_scheduled_tasks.py +++ b/tests/app/celery/test_scheduled_tasks.py @@ -1,14 +1,12 @@ from datetime import datetime, timedelta from decimal import getcontext -from unittest.mock import ANY, call, patch -from uuid import UUID +from unittest.mock import call import pytest from freezegun import freeze_time from app.celery import scheduled_tasks from app.celery.scheduled_tasks import ( - send_scheduled_comp_and_pen_sms, check_job_status, delete_invitations, delete_verify_codes, @@ -391,154 +389,3 @@ def test_check_precompiled_letter_state(mocker, sample_template, sample_notifica mock_create_ticket.assert_called_with( message=message, subject='[test] Letters still pending virus check', ticket_type='incident' ) - - -######################################################################################### -# Comp and Pen scheduled message tests -######################################################################################### - - -def test_ut_send_scheduled_comp_and_pen_sms_does_not_call_send_notification(mocker, dynamodb_mock): - mocker.patch('app.celery.scheduled_tasks.is_feature_enabled', return_value=True) - - # Mocks necessary for dynamodb - mocker.patch('boto3.resource') - mocker.patch('boto3.resource.Table', return_value=dynamodb_mock) - - # Mocking an empty list being returned from the dynamodb table - mocker.patch('app.celery.scheduled_tasks.CompPenMsgHelper.get_dynamodb_comp_pen_messages', return_value=[]) - - mock_update_dynamo_item = mocker.patch( - 'app.celery.scheduled_tasks.CompPenMsgHelper.remove_dynamo_item_is_processed' - ) - mock_lookup_notification_data = mocker.patch('app.celery.scheduled_tasks.lookup_notification_sms_setup_data') - mock_send_notification = mocker.patch('app.celery.scheduled_tasks.CompPenMsgHelper.send_comp_and_pen_sms') - - send_scheduled_comp_and_pen_sms() - - mock_update_dynamo_item.assert_not_called() - mock_lookup_notification_data.assert_not_called() - mock_send_notification.assert_not_called() - - -def test_ut_send_scheduled_comp_and_pen_sms_calls_send_comp_and_pen_sms( - mocker, dynamodb_mock, sample_service, sample_template -): - # Set up test data - dynamo_data = [ - { - 'participant_id': '123', - 'vaprofile_id': '123', - 'payment_id': '123', - 'paymentAmount': 123, - 'is_processed': False, - }, - ] - - # Mock the comp and pen perf number for sending with recipient - test_recipient = '+11234567890' - mocker.patch.dict( - 'app.celery.scheduled_tasks.current_app.config', - { - 'COMP_AND_PEN_SMS_SENDER_ID': '', - 'COMP_AND_PEN_PERF_TO_NUMBER': test_recipient, - }, - ) - - mocker.patch('app.celery.scheduled_tasks.is_feature_enabled', return_value=True) - - # Mocks necessary for dynamodb - mocker.patch('boto3.resource') - mocker.patch('boto3.resource.Table', return_value=dynamodb_mock) - - # Mock the various functions called - mock_get_dynamodb_messages = mocker.patch( - 'app.celery.scheduled_tasks.CompPenMsgHelper.get_dynamodb_comp_pen_messages', return_value=dynamo_data - ) - service = sample_service() - template = sample_template() - sms_sender_id = service.get_default_sms_sender_id() - - mocker.patch( - 'app.celery.scheduled_tasks.lookup_notification_sms_setup_data', - return_value=( - service, - template, - sms_sender_id, - ), - ) - - mock_send_notification = mocker.patch('app.celery.scheduled_tasks.CompPenMsgHelper.send_comp_and_pen_sms') - - send_scheduled_comp_and_pen_sms() - - # Assert the functions are being called that should be - mock_get_dynamodb_messages.assert_called_once() - - # Assert the expected information is passed to send_comp_and_pen_sms - mock_send_notification.assert_called_once_with( - service=service, - template=template, - sms_sender_id=sms_sender_id, - comp_and_pen_messages=dynamo_data, - perf_to_number=test_recipient, - ) - - -@pytest.mark.parametrize('sms_sender_id', ('This is not a UUID.', 'd1abbb27-9c72-4de4-8463-dbdf24d3fdd6')) -def test_it_send_scheduled_comp_and_pen_sms_sms_sender_id_selection( - mocker, sample_service, sample_template, sms_sender_id, dynamodb_mock -): - """ - When the configuration variable COMP_AND_PEN_SMS_SENDER_ID is a valid UUID, the task send_scheduled_comp_and_pen_sms - should send notifications using the ServiceSmsSender associated with that UUID. Otherwise, the task should use the - default SMS sender associated with the service referenced by the configuration variable COMP_AND_PEN_SERVICE_ID. - """ - - mocker.patch('boto3.resource') - mocker.patch('boto3.resource.Table', return_value=dynamodb_mock) - - items = [ - { - 'participant_id': '123', - 'vaprofile_id': '123', - 'payment_id': '123', - 'paymentAmount': 123, - 'is_processed': False, - }, - ] - mocker.patch('app.celery.scheduled_tasks.CompPenMsgHelper.get_dynamodb_comp_pen_messages', return_value=items) - mock_send_notification = mocker.patch('app.celery.scheduled_tasks.CompPenMsgHelper.send_comp_and_pen_sms') - mocker.patch('app.celery.scheduled_tasks.CompPenMsgHelper.remove_dynamo_item_is_processed') - - service = sample_service() - template = sample_template() - - mocker.patch.dict( - 'app.celery.scheduled_tasks.current_app.config', - { - 'COMP_AND_PEN_SERVICE_ID': service.id, - 'COMP_AND_PEN_TEMPLATE_ID': template.id, - 'COMP_AND_PEN_SMS_SENDER_ID': sms_sender_id, - 'COMP_AND_PEN_PERF_TO_NUMBER': '+15552225566', - }, - ) - - with patch.dict('os.environ', {'COMP_AND_PEN_MESSAGES_ENABLED': 'True'}): - send_scheduled_comp_and_pen_sms() - - try: - # If this line doesn't raise ValueError, sms_sender_id is a valid UUID. - expected_sms_sender_id = UUID(sms_sender_id) - except ValueError: - expected_sms_sender_id = service.get_default_sms_sender_id() - - mock_send_notification.assert_called_once_with( - service=ANY, - template=ANY, - sms_sender_id=str(expected_sms_sender_id), - comp_and_pen_messages=items, - perf_to_number='+15552225566', - ) - assert mock_send_notification.call_args.kwargs['service'].id == service.id - assert mock_send_notification.call_args.kwargs['template'].id == template.id diff --git a/tests/app/integrations/comp_and_pen/test_scheduled_message_helpers.py b/tests/app/integrations/comp_and_pen/test_scheduled_message_helpers.py deleted file mode 100644 index 38784356f8..0000000000 --- a/tests/app/integrations/comp_and_pen/test_scheduled_message_helpers.py +++ /dev/null @@ -1,287 +0,0 @@ -from decimal import Decimal -from uuid import uuid4 - -import pytest - -from app.constants import SMS_TYPE -from app.integrations.comp_and_pen.scheduled_message_helpers import CompPenMsgHelper -from app.models import Service -from app.va.identifier import IdentifierType - - -@pytest.fixture -def msg_helper(mocker, dynamodb_mock) -> CompPenMsgHelper: - # Mocks necessary for dynamodb - mocker.patch('boto3.resource') - helper = CompPenMsgHelper('test') - mocker.patch.object(helper, 'dynamodb_table', dynamodb_mock) - return helper - - -def test_ut_get_dynamodb_comp_pen_messages_with_empty_table(msg_helper): - # Invoke the function with the mocked table and application - messages = msg_helper.get_dynamodb_comp_pen_messages(message_limit=1) - - assert messages == [], 'Expected no messages from an empty table' - - -def test_get_dynamodb_comp_pen_messages_filters(msg_helper, sample_dynamodb_insert): - """ - Items should not be returned if any of the following conditions are met: - 1) is_processed is absent (not on is-processed-index) - 2) paymentAmount is absent (required by downstream Celery task) - """ - # Insert mock data into the DynamoDB table. - items_to_insert = [ - # The first 3 items are valid. - {'participant_id': 1, 'is_processed': 'F', 'payment_id': 1, 'paymentAmount': Decimal(1.00), 'vaprofile_id': 1}, - {'participant_id': 2, 'is_processed': 'F', 'payment_id': 2, 'paymentAmount': Decimal(2.50), 'vaprofile_id': 2}, - {'participant_id': 3, 'is_processed': 'F', 'payment_id': 3, 'paymentAmount': Decimal('3.9'), 'vaprofile_id': 3}, - # Already processed - {'participant_id': 4, 'payment_id': 4, 'paymentAmount': Decimal(0), 'vaprofile_id': 4}, - # Missing paymentAmount - {'participant_id': 5, 'is_processed': 'F', 'payment_id': 5, 'vaprofile_id': 5}, - ] - sample_dynamodb_insert(items_to_insert) - - # Invoke the function with the mocked table and application - messages = msg_helper.get_dynamodb_comp_pen_messages(message_limit=7) - - for msg in messages: - assert ( - str(msg['participant_id']) in '123' - ), f"The message with ID {msg['participant_id']} should have been filtered out." - assert len(messages) == 3 - - -def test_it_get_dynamodb_comp_pen_messages_with_multiple_scans(msg_helper, sample_dynamodb_insert): - """ - Items should be searched based on the is-processed-index and anything missing a paymentAmount should be filtered out. - - This is also testing the pagination of the scan operation in which a bug previously existed. - """ - items_to_insert = ( - # items with is_processed = 'F' - { - 'participant_id': x, - 'is_processed': 'F', - 'payment_id': x, - 'paymentAmount': Decimal(x * 2.50), - 'vaprofile_id': x * 10, - } - if x % 2 == 0 - # items with is_processed removed (not in index) - else { - 'participant_id': x, - 'payment_id': x, - 'paymentAmount': Decimal(x * 2.50), - 'vaprofile_id': x * 10, - } - for x in range(0, 250) - ) - - # Insert mock data into the DynamoDB table. - sample_dynamodb_insert(items_to_insert) - - # Invoke the function with the mocked table and application - messages = msg_helper.get_dynamodb_comp_pen_messages(message_limit=100) - - assert len(messages) == 100 - - # ensure we only have messages that have not been processed - for m in messages: - assert m['is_processed'] == 'F' - assert m['paymentAmount'] is not None - - -def test_it_update_dynamo_item_is_processed_updates_properly( - notify_api, mocker, msg_helper, dynamodb_mock, sample_dynamodb_insert -): - """Ensure that the 'is_processed' key is removed from the items in the list and the DynamoDB table is updated.""" - - items_to_insert = [ - {'participant_id': 1, 'is_processed': 'F', 'payment_id': 1, 'paymentAmount': Decimal(1.00)}, - {'participant_id': 2, 'is_processed': 'F', 'payment_id': 2, 'paymentAmount': Decimal(2.50)}, - {'participant_id': 3, 'payment_id': 1, 'paymentAmount': Decimal(0.00)}, - {'participant_id': 4, 'is_processed': 'F', 'payment_id': 2, 'paymentAmount': Decimal(4.50)}, - {'participant_id': 5, 'is_processed': 'F', 'payment_id': 1, 'paymentAmount': Decimal(5.50)}, - ] - - # Insert mock data into the DynamoDB table. - sample_dynamodb_insert(items_to_insert) - - mock_logger = mocker.patch('app.integrations.comp_and_pen.scheduled_message_helpers.current_app.logger') - - # why is this giving this error? "RuntimeError: Working outside of application context." - msg_helper.remove_dynamo_item_is_processed(items_to_insert) - - assert mock_logger.debug.call_count == 5 - - response = dynamodb_mock.scan() - - # Ensure we get all 5 records back and they are set with is_processed removed - assert response['Count'] == 5 - for item in response['Items']: - assert 'is_processed' not in item - - -def test_ut_send_scheduled_comp_and_pen_sms_calls_send_notification_with_recipient_item( - mocker, msg_helper, dynamodb_mock, sample_service, sample_template -): - # Set up test data - dynamo_data = [ - { - 'participant_id': '123', - 'vaprofile_id': '123', - 'payment_id': '123', - 'paymentAmount': 123.05, # Named by Kafka - 'is_processed': False, - }, - ] - - recipient_item = {'id_type': IdentifierType.VA_PROFILE_ID.value, 'id_value': '123'} - - mocker.patch('app.celery.scheduled_tasks.is_feature_enabled', return_value=True) - notification_id = uuid4() - mocker.patch('app.integrations.comp_and_pen.scheduled_message_helpers.uuid4', return_value=notification_id) - - service: Service = sample_service() - template = sample_template() - sms_sender_id = str(service.get_default_sms_sender_id()) - sender_number = service.get_default_sms_sender() - - mock_send_notification = mocker.patch( - 'app.integrations.comp_and_pen.scheduled_message_helpers.send_notification_bypass_route' - ) - - msg_helper.send_comp_and_pen_sms( - service=service, - template=template, - sms_sender_id=sms_sender_id, - comp_and_pen_messages=dynamo_data, - perf_to_number=None, - ) - - # Assert the expected information is passed to "send_notification_bypass_route" - mock_send_notification.assert_called_once_with( - service=service, - template=template, - notification_type=SMS_TYPE, - reply_to_text=sender_number, - personalisation={'amount': '123.05'}, - sms_sender_id=sms_sender_id, - recipient=None, - recipient_item=recipient_item, - notification_id=notification_id, - ) - - -@pytest.mark.parametrize( - 'amount, formatted_amount', - [ - (1123.05, '1,123.05'), - (1000.00, '1,000.00'), - (10000.00, '10,000.00'), - (1234567.89, '1,234,567.89'), - (50.5, '50.50'), - (0.5, '0.50'), - (0.0, '0.00'), - ], -) -def test_ut_send_scheduled_comp_and_pen_sms_formatted_amount_correctly( - mocker, msg_helper, dynamodb_mock, sample_service, sample_template, amount, formatted_amount -): - dynamo_data = [ - { - 'participant_id': '123', - 'vaprofile_id': '123', - 'payment_id': '123', - 'paymentAmount': amount, - 'is_processed': False, - }, - ] - - recipient_item = {'id_type': IdentifierType.VA_PROFILE_ID.value, 'id_value': '123'} - notification_id = uuid4() - mocker.patch('app.integrations.comp_and_pen.scheduled_message_helpers.uuid4', return_value=notification_id) - - mocker.patch('app.celery.scheduled_tasks.is_feature_enabled', return_value=True) - - service: Service = sample_service() - template = sample_template() - sms_sender_id = str(service.get_default_sms_sender_id()) - - mock_send_notification = mocker.patch( - 'app.integrations.comp_and_pen.scheduled_message_helpers.send_notification_bypass_route' - ) - - msg_helper.send_comp_and_pen_sms( - service=service, - template=template, - sms_sender_id=sms_sender_id, - comp_and_pen_messages=dynamo_data, - perf_to_number=None, - ) - - mock_send_notification.assert_called_once_with( - service=service, - template=template, - notification_type=SMS_TYPE, - reply_to_text=service.get_default_sms_sender(), - personalisation={'amount': formatted_amount}, - sms_sender_id=sms_sender_id, - recipient=None, - recipient_item=recipient_item, - notification_id=notification_id, - ) - - -def test_ut_send_scheduled_comp_and_pen_sms_payment_amount_key_does_not_exist( - mocker, - msg_helper, - dynamodb_mock, - sample_service, - sample_template, -): - dynamo_data = [ - { - 'participant_id': '123', - 'vaprofile_id': '123', - 'payment_id': '123', - 'is_processed': False, - }, - ] - - recipient_item = {'id_type': IdentifierType.VA_PROFILE_ID.value, 'id_value': '123'} - - mocker.patch('app.celery.scheduled_tasks.is_feature_enabled', return_value=True) - notification_id = uuid4() - mocker.patch('app.integrations.comp_and_pen.scheduled_message_helpers.uuid4', return_value=notification_id) - - service = sample_service() - template = sample_template() - sms_sender_id = str(service.get_default_sms_sender_id()) - - mock_send_notification = mocker.patch( - 'app.integrations.comp_and_pen.scheduled_message_helpers.send_notification_bypass_route' - ) - - msg_helper.send_comp_and_pen_sms( - service=service, - template=template, - sms_sender_id=sms_sender_id, - comp_and_pen_messages=dynamo_data, - perf_to_number=None, - ) - - mock_send_notification.assert_called_once_with( - service=service, - template=template, - notification_type=SMS_TYPE, - reply_to_text=service.get_default_sms_sender(), - personalisation={'amount': '0.00'}, - sms_sender_id=sms_sender_id, - recipient=None, - recipient_item=recipient_item, - notification_id=notification_id, - ) diff --git a/tests/app/notifications/test_send_notifications.py b/tests/app/notifications/test_send_notifications.py index ac3f6db578..3d51c07078 100644 --- a/tests/app/notifications/test_send_notifications.py +++ b/tests/app/notifications/test_send_notifications.py @@ -78,7 +78,6 @@ def test_send_notification_bypass_route_no_recipient( send_notification_bypass_route( service, template, - SMS_TYPE, reply_to_text=service.get_default_sms_sender(), recipient=None, recipient_item=None, @@ -109,7 +108,6 @@ def test_send_notification_bypass_route_sms_with_recipient_and_default_sms_sende send_notification_bypass_route( service=service, template=template, - notification_type=SMS_TYPE, reply_to_text=sender_number, recipient='+11234567890', ) @@ -162,7 +160,6 @@ def test_send_notification_bypass_route_sms_with_recipient_item( send_notification_bypass_route( service=service, template=template, - notification_type=SMS_TYPE, reply_to_text=sender_number, recipient_item=recipient_item, sms_sender_id='test_sms_sender', @@ -216,7 +213,6 @@ def test_send_notification_bypass_route_email_with_recipient( service=service, template=template, reply_to_text=send_number, - notification_type=EMAIL_TYPE, recipient='test123@email.com', ) @@ -265,7 +261,6 @@ def test_send_notification_bypass_route_email_with_recipient_item( send_notification_bypass_route( service=service, template=template, - notification_type=EMAIL_TYPE, reply_to_text=reply_to, recipient_item=recipient_item, )