Merge branch 'main' into 2041-implement-datadog-apm-for-celery

.talismanrc

@@ -1,6 +1,8 @@
 fileignoreconfig:
 - filename: README.md
   checksum: b2cbbb8508af49abccae8b35b317f7ce09215f3508430ed31440add78f450e5a
+- filename: tests/README.md
+  checksum: 4de2edc5ca36f266118231611036c7ccc6d9328bcceefafd8ca5ca92021f8aed
 - filename: app/callback/webhook_callback_strategy.py
   checksum: 47846ab651c27512d3ac7864c08cb25d647f63bb84321953f907551fd9d2e85f
 - filename: app/celery/contact_information_tasks.py

app/celery/provider_tasks.py

@@ -8,6 +8,7 @@
 from app.celery.exceptions import NonRetryableException, AutoRetryException
 from app.celery.service_callback_tasks import check_and_queue_callback_task
 from app.clients.email.aws_ses import AwsSesClientThrottlingSendRateException
+from app.clients.sms import OPT_OUT_MESSAGE
 from app.config import QueueNames
 from app.constants import NOTIFICATION_TECHNICAL_FAILURE
 from app.dao import notifications_dao
@@ -77,11 +78,17 @@ def deliver_sms(
         raise NotificationTechnicalFailureException from e
     except NonRetryableException as e:
         # Likely an opted out from pinpoint
+
+        if 'opted out' in str(e).lower():
+            status_reason = OPT_OUT_MESSAGE
+        else:
+            status_reason = 'ERROR: NonRetryableException - permanent failure, not retrying'
+
         log_and_update_permanent_failure(
             notification.id,
             'deliver_sms',
             e,
-            'ERROR: NonRetryableException - permanent failure, not retrying',
+            status_reason,
         )
         # Expected chain termination
         self.request.chain = None

tests/README.md

@@ -1,27 +1,39 @@
-# Running a Subset of Tests Using Containers
+# Running Unit Tests With Containers
 
-You can run specific test directories, files, or individual tests rather than the entire suite as follows.  These instructions assume that you have previously run the docker-compose command to start the local ecosystem and, therefore, that all migrations have been applied to the database and the default container network, ci_default, exists.
+These instructions assume that you have previously run the docker-compose command to start the local ecosystem and, therefore, that all migrations have been applied to the database.  This also ensures that the default container network, ci_default, exists.
 
 ## Setting Environment Variables
 
-The docker-compose command used to run the full test suite sets environment variables.  Step 3 below references the file env_vars for the same purpose.
+The docker-compose command used to run the full test suite sets environment variables.  When running a subset of tests, you must ensure that you set the variables.
+
+## Running all checks and unit tests
+
+Build and test the "ci_test" Docker image by running this command:
+
+```bash
+docker compose -f ci/docker-compose-test.yml up
+```
+
+**Rebuild ci_test whenever Dockerfile or poetry.lock changes.**
+
+## Running a subset of tests
+
+There are two options for running ad hoc tests.  Step 0 for either option is to stop all running containers other than the database.
 
-## Setup
-There are two options for running ad hoc tests.
 ### Option 1
-1. Stop all running containers associated with Notification-api.
-2. Start the Postgres (ci_db_1) container, and any other containers required by the functionality under test: `docker start ci-db-1`.  All migrations should already be applied.
-3. Start a test container shell by running `docker run --rm -it -v "$(pwd):/app" --env-file ci/.local.env --name ci-test --network ci_default test-notification-api bash`.
-4. In the test container shell, run `pytest -h` to see the syntax for running tests.  Without flags, you can run `pytest [file or directory]...`.
+
+1. Start the Postgres (ci_db_1) container, if necessary: `docker start ci-db-1`.  All migrations should already be applied.
+2. Start a test container shell: `docker run --rm -it -v "$(pwd):/app" --env-file ci/.local.env --name ci-test --network ci_default test-notification-api bash`.  Note the use of the environment file.
+3. In the test container shell, run `pytest -h` to see the syntax for running tests.  Without flags, you can run `pytest [file or directory]...`.
 
 ### Option 2
-1. Stop all running containers
+
 2. Edit `scripts/run_tests.sh` by commenting any `pytest` execution and adding `tail -f` to the end of the file
 3. Run `docker compose -f ci/docker-compose-test.yml up`
 4. In a separate window run `docker exec -it ci-test-1 bash`
 5. Execute any command, such as `pytest --durations 10 tests/app/celery`
 
-## Running Individual Tests
+### Example
 
 This is an example of running a specific test in a test file from *within a test container shell*:
 
@@ -33,15 +45,7 @@ $ pytest tests/lambda_functions/va_profile/test_va_profile_integration.py::test_
 
 Running Bash commands on a container with read-write access, such as ci_test, will result in the creation of .bash_history in your notification_api/ directory.  That file is in .gitignore.
 
-## Additional Options
-
-Build and test the "ci_test" Docker image by running this command:
-
-```bash
-docker compose -f ci/docker-compose-test.yml up
-```
-
-**Rebuild ci_test whenever Dockerfile or poetry.lock changes.**
+## Additional examples and information
 
 For a more interactive testing experience, edit `scripts/run_tests.sh` so that it does not execute any pytest command, and place `tail -f` on the final line e.g.
 ```bash

tests/app/celery/test_provider_tasks.py

@@ -4,8 +4,15 @@
 from app.celery.exceptions import NonRetryableException, AutoRetryException
 from app.celery.provider_tasks import deliver_sms, deliver_email, deliver_sms_with_rate_limiting
 from app.clients.email.aws_ses import AwsSesClientThrottlingSendRateException
+from app.clients.sms import OPT_OUT_MESSAGE
 from app.config import QueueNames
-from app.constants import EMAIL_TYPE, NOTIFICATION_PERMANENT_FAILURE, NOTIFICATION_TECHNICAL_FAILURE, SMS_TYPE
+from app.constants import (
+    EMAIL_TYPE,
+    NOTIFICATION_CREATED,
+    NOTIFICATION_PERMANENT_FAILURE,
+    NOTIFICATION_TECHNICAL_FAILURE,
+    SMS_TYPE,
+)
 from app.exceptions import (
     NotificationTechnicalFailureException,
     InvalidProviderException,
@@ -440,3 +447,41 @@ def test_deliver_sms_with_rate_limiting_max_retries_exceeded(
     assert notification.status == NOTIFICATION_TECHNICAL_FAILURE
     assert notification.status_reason == RETRIES_EXCEEDED
     mocked_check_and_queue_callback_task.assert_called_once()
+
+
+def test_deliver_sms_opt_out(
+    notify_db_session,
+    mocker,
+    sample_service,
+    sample_sms_sender,
+    sample_template,
+    sample_notification,
+):
+    """
+    An SMS notification sent to a recipient who has opted out of receiving notifications
+    from the given SMS sender should result in permanent failure with a relevant status reason.
+    """
+
+    service = sample_service()
+    sms_sender = sample_sms_sender(service_id=service.id, sms_sender='17045555555')
+    template = sample_template(service=service)
+    notification = sample_notification(
+        template=template,
+        status=NOTIFICATION_CREATED,
+        sms_sender_id=sms_sender.id,
+    )
+    assert notification.notification_type == SMS_TYPE
+    assert notification.status == NOTIFICATION_CREATED
+    assert notification.sms_sender_id == sms_sender.id
+    assert notification.status_reason is None
+
+    mock_send_sms_to_provider = mocker.patch(
+        'app.delivery.send_to_providers.send_sms_to_provider',
+        side_effect=NonRetryableException('Destination phone number opted out'),
+    )
+    deliver_sms(notification.id, sms_sender_id=notification.sms_sender_id)
+    mock_send_sms_to_provider.assert_called_once()
+
+    notify_db_session.session.refresh(notification)
+    assert notification.status == NOTIFICATION_PERMANENT_FAILURE
+    assert notification.status_reason == OPT_OUT_MESSAGE