merge from main

.ds.baseline

@@ -209,7 +209,7 @@
         "filename": "tests/app/aws/test_s3.py",
         "hashed_secret": "67a74306b06d0c01624fe0d0249a570f4d093747",
         "is_verified": false,
-        "line_number": 40,
+        "line_number": 42,
         "is_secret": false
       }
     ],
@@ -384,5 +384,5 @@
       }
     ]
   },
-  "generated_at": "2025-02-10T16:57:15Z"
+  "generated_at": "2025-02-27T21:09:45Z"
 }

Makefile

@@ -152,4 +152,4 @@ clean:
 .PHONY: test-single
 test-single: export NEW_RELIC_ENVIRONMENT=test
 test-single: ## Run a single test file
-	poetry run pytest $(TEST_FILE)
+	poetry run pytest -s $(TEST_FILE)

app/clients/cloudwatch/aws_cloudwatch.py

@@ -107,6 +107,12 @@ class AwsCloudwatchClient(Client):
         provider_response = self._aws_value_or_default(
             event, "delivery", "providerResponse"
         )
+        message_cost = self._aws_value_or_default(event, "delivery", "priceInUSD")
+        if message_cost is None or message_cost == "":
+            message_cost = 0.0
+        else:
+            message_cost = float(message_cost)
+
         my_timestamp = self._aws_value_or_default(event, "notification", "timestamp")
         return {
             "notification.messageId": event["notification"]["messageId"],
@@ -114,6 +120,7 @@ class AwsCloudwatchClient(Client):
             "delivery.phoneCarrier": phone_carrier,
             "delivery.providerResponse": provider_response,
             "@timestamp": my_timestamp,
+            "delivery.priceInUSD": message_cost,
         }
 
     # Here is an example of how to get the events with log insights

app/dao/notifications_dao.py

@@ -507,7 +507,7 @@ def insert_notification_history_delete_notifications(
          SELECT id, job_id, job_row_number, service_id, template_id, template_version, api_key_id,
              key_type, notification_type, created_at, sent_at, sent_by, updated_at, reference, billable_units,
              client_reference, international, phone_prefix, rate_multiplier, notification_status,
-              created_by_id, document_download_count
+              created_by_id, document_download_count, message_cost
           FROM notifications
         WHERE service_id = :service_id
           AND notification_type = :notification_type
@@ -842,7 +842,6 @@ def dao_update_delivery_receipts(receipts, delivered):
         new_receipts.append(r)
 
     receipts = new_receipts
-
     id_to_carrier = {
         r["notification.messageId"]: r["delivery.phoneCarrier"] for r in receipts
     }
@@ -851,9 +850,13 @@ def dao_update_delivery_receipts(receipts, delivered):
     }
     id_to_timestamp = {r["notification.messageId"]: r["@timestamp"] for r in receipts}
 
+    id_to_message_cost = {
+        r["notification.messageId"]: r["delivery.priceInUSD"] for r in receipts
+    }
     status_to_update_with = NotificationStatus.DELIVERED
     if not delivered:
         status_to_update_with = NotificationStatus.FAILED
+
     stmt = (
         update(Notification)
         .where(Notification.message_id.in_(id_to_carrier.keys()))
@@ -877,6 +880,12 @@ def dao_update_delivery_receipts(receipts, delivered):
                     for key, value in id_to_provider_response.items()
                 ]
             ),
+            message_cost=case(
+                *[
+                    (Notification.message_id == key, value)
+                    for key, value in id_to_message_cost.items()
+                ]
+            ),
         )
     )
     db.session.execute(stmt)
@@ -908,7 +917,6 @@ def dao_close_out_delivery_receipts():
 
 
 def dao_batch_insert_notifications(batch):
-
     db.session.bulk_save_objects(batch)
     db.session.commit()
     current_app.logger.info(f"Batch inserted notifications: {len(batch)}")

app/delivery/send_to_providers.py

@@ -118,7 +118,13 @@ def send_sms_to_provider(notification):
                     "international": notification.international,
                 }
                 db.session.close()  # no commit needed as no changes to objects have been made above
-
+                real_sender_number = notification.reply_to_text
+                # interleave spaces to bypass PII scrubbing since sender number is not PII
+                arr = list(real_sender_number)
+                real_sender_number = " ".join(arr)
+                current_app.logger.info(
+                    f"#notify-debug-api-1701 real sender number going to AWS is {real_sender_number}"
+                )
                 message_id = provider.send_sms(**send_sms_kwargs)
 
                 update_notification_message_id(notification.id, message_id)

app/models.py

@@ -1508,6 +1508,7 @@ class Notification(db.Model):
     created_at = db.Column(db.DateTime, index=True, unique=False, nullable=False)
     sent_at = db.Column(db.DateTime, index=False, unique=False, nullable=True)
     sent_by = db.Column(db.String, nullable=True)
+    message_cost = db.Column(db.Float, nullable=True, default=0.0)
     updated_at = db.Column(
         db.DateTime,
         index=False,
@@ -1708,7 +1709,7 @@ class Notification(db.Model):
                         value = (obj.created_at.strftime("%Y-%m-%d %H:%M:%S"),)
                 elif column.name in ["sent_at", "completed_at"]:
                     value = None
-                elif column.name.endswith("_id"):
+                elif column.name.endswith("_id") or column.name == "id":
                     value = getattr(obj, column.name)
                     value = str(value)
                 else:
@@ -1813,6 +1814,7 @@ class NotificationHistory(db.Model, HistoryModel):
     created_at = db.Column(db.DateTime, unique=False, nullable=False)
     sent_at = db.Column(db.DateTime, index=False, unique=False, nullable=True)
     sent_by = db.Column(db.String, nullable=True)
+    message_cost = db.Column(db.Float, nullable=True, default=0.0)
     updated_at = db.Column(
         db.DateTime,
         index=False,

migrations/versions/0415_add_message_cost.py

@@ -0,0 +1,23 @@
+"""
+
+Revision ID: 0415_add_message_cost
+Revises: 0414_change_total_message_limit
+Create Date: 2025-02-28 11:35:22.873930
+
+"""
+
+import sqlalchemy as sa
+from alembic import op
+
+down_revision = "0414_change_total_message_limit"
+revision = "0415_add_message_cost"
+
+
+def upgrade():
+    op.add_column("notifications", sa.Column("message_cost", sa.Float))
+    op.add_column("notification_history", sa.Column("message_cost", sa.Float))
+
+
+def downgrade():
+    op.drop_column("notifications", "message_cost")
+    op.add_column("notification_history", sa.Column("message_cost", sa.Float))

pyproject.toml

@@ -32,10 +32,10 @@ flask-marshmallow = "==1.2.1"
 flask-migrate = "==4.0.7"
 flask-redis = "==0.4.0"
 flask-sqlalchemy = "==3.1.1"
-gunicorn = {version = "==22.0.0", extras = ["eventlet"]}
+gunicorn = {version = "==23.0.0", extras = ["eventlet"]}
 iso8601 = "==2.1.0"
 jsonschema = {version = "==4.23.0", extras = ["format"]}
-lxml = "==5.2.2"
+lxml = "==5.3.1"
 marshmallow = "==3.22.0"
 marshmallow-sqlalchemy = "==1.0.0"
 newrelic = "*"
@@ -44,7 +44,7 @@ oscrypto = { git = "https://github.com/wbond/oscrypto.git", rev = "1547f53" }
 packaging = "==24.1"
 poetry-dotenv-plugin = "==0.2.0"
 psycopg2-binary = "==2.9.9"
-pyjwt = "==2.8.0"
+pyjwt = "==2.10.1"
 python-dotenv = "==1.0.1"
 sqlalchemy = "==2.0.31"
 werkzeug = "^3.0.6"
@@ -52,7 +52,7 @@ faker = "^26.0.0"
 async-timeout = "^4.0.3"
 bleach = "^6.1.0"
 geojson = "^3.2.0"
-numpy = "^1.26.4"
+numpy = "^2.2.3"
 ordered-set = "^4.1.0"
 phonenumbers = "^8.13.42"
 python-json-logger = "^2.0.7"
@@ -73,7 +73,7 @@ six = "^1.16.0"
 urllib3 = "^2.2.2"
 webencodings = "^0.5.1"
 itsdangerous = "^2.2.0"
-jinja2 = "^3.1.5"
+jinja2 = "^3.1.6"
 redis = "^5.0.8"
 requests = "^2.32.3"
 

tests/app/aws/test_s3.py

@@ -22,8 +22,10 @@ from app.aws.s3 import (
     get_s3_object,
     get_s3_resource,
     list_s3_objects,
+    purge_bucket,
     read_s3_file,
     remove_csv_object,
+    remove_job_from_s3,
     remove_s3_object,
 )
 from app.clients import AWS_CLIENT_CONFIG
@@ -563,3 +565,71 @@ def test_get_s3_object_client_error(mocker):
     mock_logger.exception.assert_called_once_with(
         f"Can't retrieve S3 Object from {file_location}"
     )
+
+
+def test_purge_bucket(mocker):
+    mock_s3_resource = MagicMock()
+    mock_bucket = MagicMock()
+    mock_s3_resource.Bucket.return_value = mock_bucket
+    mocker.patch("app.aws.s3.get_s3_resource", return_value=mock_s3_resource)
+
+    purge_bucket("my-bucket", "access-key", "secret-key", "region")
+
+    # Assert that the bucket's objects.all().delete() method was called
+    mock_bucket.objects.all.return_value.delete.assert_called_once()
+
+
+def test_remove_job_from_s3(mocker):
+    mock_get_job_location = mocker.patch("app.aws.s3.get_job_location")
+    mock_remove_s3_object = mocker.patch("app.aws.s3.remove_s3_object")
+
+    mock_get_job_location.return_value = (
+        "test-bucket",
+        "test.csv",
+        "fake-stuff",
+    )
+
+    remove_job_from_s3("service-id-123", "job-id-456")
+
+    mock_get_job_location.assert_called_once_with("service-id-123", "job-id-456")
+    mock_remove_s3_object.assert_called_once_with(
+        "test-bucket",
+        "test.csv",
+        "fake-stuff",
+    )
+
+
+def test_get_s3_files_handles_exception(mocker):
+    mock_current_app = mocker.patch("app.aws.s3.current_app")
+    mock_current_app.config = {
+        "CSV_UPLOAD_BUCKET": {"bucket": "test-bucket"},
+        "job_cache": {},
+    }
+
+    mock_list_s3_objects = mocker.patch("app.aws.s3.list_s3_objects")
+    mock_list_s3_objects.return_value = ["file1.csv", "file2.csv"]
+
+    mock_get_s3_resource = mocker.patch("app.aws.s3.get_s3_resource")
+
+    # Make the first call succeed, second call should fail.
+    mock_read_s3_file = mocker.patch(
+        "app.aws.s3.read_s3_file", side_effect=[None, Exception("exception here")]
+    )
+
+    mock_thread_pool_executor = mocker.patch("app.aws.s3.ThreadPoolExecutor")
+    mock_executor = mock_thread_pool_executor.return_value.__enter__.return_value
+
+    def mock_map(func, iterable):
+        for item in iterable:
+            func(item)
+
+    mock_executor.map.side_effect = mock_map
+    get_s3_files()
+
+    calls = [
+        mocker.call("test-bucket", "file1.csv", mock_get_s3_resource.return_value),
+        mocker.call("test-bucket", "file2.csv", mock_get_s3_resource.return_value),
+    ]
+    mock_read_s3_file.assert_has_calls(calls, any_order=True)
+
+    mock_current_app.logger.exception.assert_called_with("Connection pool issue")

tests/app/celery/test_scheduled_tasks.py

@@ -583,7 +583,7 @@ def test_batch_insert_with_expired_notifications(mocker):
     rs.llen.assert_called_once_with("message_queue")
     rs.rpush.assert_called_once()
     requeued_notification = json.loads(rs.rpush.call_args[0][1])
-    assert requeued_notification["id"] == 1
+    assert requeued_notification["id"] == '1'
 
 
 def test_batch_insert_with_malformed_notifications(mocker):

tests/app/clients/test_aws_cloudwatch.py

@@ -31,7 +31,7 @@ def side_effect(filterPattern, logGroupName, startTime, endTime):
                 {
                     "logStreamName": "89db9712-c6d1-49f9-be7c-4caa7ed9efb1",
                     "message": '{"delivery":{"destination":"+1661","phoneCarrier":"ATT Mobility", '
-                    '"providerResponse":"Invalid phone number"}}',
+                    '"providerResponse":"Invalid phone number", "priceInUSD": "0.00881"}}',
                     "eventId": "37535432778099870001723210579798865345508698025292922880",
                 }
             ]
@@ -44,7 +44,7 @@ def side_effect(filterPattern, logGroupName, startTime, endTime):
                     "logStreamName": "89db9712-c6d1-49f9-be7c-4caa7ed9efb1",
                     "timestamp": 1683147017911,
                     "message": '{"delivery":{"destination":"+1661","phoneCarrier":"ATT Mobility",'
-                    '"providerResponse":"Phone accepted msg"}}',
+                    '"providerResponse":"Phone accepted msg", "priceInUSD": "0.00881"}}',
                     "ingestionTime": 1683147018026,
                     "eventId": "37535432778099870001723210579798865345508698025292922880",
                 }
@@ -131,6 +131,7 @@ def test_event_to_db_format_with_missing_fields():
         "status": "UNKNOWN",
         "delivery.phoneCarrier": "",
         "delivery.providerResponse": "",
+        "delivery.priceInUSD": 0.0,
         "@timestamp": "",
     }
 
@@ -140,7 +141,11 @@ def test_event_to_db_format_with_string_input():
         {
             "notification": {"messageId": "67890", "timestamp": "2024-01-01T14:00:00Z"},
             "status": "FAILED",
-            "delivery": {"phoneCarrier": "Verizon", "providerResponse": "Error"},
+            "delivery": {
+                "phoneCarrier": "Verizon",
+                "providerResponse": "Error",
+                "priceInUSD": "0.00881",
+            },
         }
     )
     result = aws_cloudwatch_client.event_to_db_format(event)
@@ -149,5 +154,6 @@ def test_event_to_db_format_with_string_input():
         "status": "FAILED",
         "delivery.phoneCarrier": "Verizon",
         "delivery.providerResponse": "Error",
+        "delivery.priceInUSD": 0.00881,
         "@timestamp": "2024-01-01T14:00:00Z",
     }

tests/app/dao/notification_dao/test_notification_dao.py

@@ -2019,8 +2019,8 @@ def test_notifications_not_yet_sent_return_no_rows(sample_service, notification_
 def test_update_delivery_receipts(mocker):
     mock_session = mocker.patch("app.dao.notifications_dao.db.session")
     receipts = [
-        '{"notification.messageId": "msg1", "delivery.phoneCarrier": "carrier1", "delivery.providerResponse": "resp1", "@timestamp": "2024-01-01T12:00:00"}',  # noqa
-        '{"notification.messageId": "msg2", "delivery.phoneCarrier": "carrier2", "delivery.providerResponse": "resp2", "@timestamp": "2024-01-01T13:00:00"}',  # noqa
+        '{"notification.messageId": "msg1", "delivery.phoneCarrier": "carrier1", "delivery.providerResponse": "resp1", "@timestamp": "2024-01-01T12:00:00", "delivery.priceInUSD": "0.00881"}',  # noqa
+        '{"notification.messageId": "msg2", "delivery.phoneCarrier": "carrier2", "delivery.providerResponse": "resp2", "@timestamp": "2024-01-01T13:00:00", "delivery.priceInUSD": "0.00881"}',  # noqa
     ]
     delivered = True
     mock_update = MagicMock()
@@ -2134,3 +2134,57 @@ def test_sanitize_successful_notification_by_id():
                 "sent_at": ANY,
             },
         )
+
+
+def test_dao_get_notifications_by_recipient_or_reference_covers_sms_search_by_reference(
+    notify_db_session,
+):
+    """
+    This test:
+      1. Creates a service and an SMS template.
+      2. Creates a notification with a specific client_reference and status=FAILED.
+      3. Calls dao_get_notifications_by_recipient_or_reference with notification_type=SMS,
+         statuses=[FAILED], and a search term = client_reference.
+      4. Confirms the function returns exactly one notification matching that reference.
+    """
+
+    service = create_service(service_name="Test Service")
+    template = create_template(service=service, template_type=NotificationType.SMS)
+
+    # Instead of matching phone logic, we'll match on client_reference
+    data = {
+        "id": uuid.uuid4(),
+        "to": "1",
+        "normalised_to": "1",  # phone is irrelevant here
+        "service_id": service.id,
+        "service": service,
+        "template_id": template.id,
+        "template_version": template.version,
+        "status": NotificationStatus.FAILED,
+        "created_at": utc_now(),
+        "billable_units": 1,
+        "notification_type": template.template_type,
+        "key_type": KeyType.NORMAL,
+        "client_reference": "some-ref",  # <--- We'll search for this
+    }
+    notification = Notification(**data)
+    dao_create_notification(notification)
+
+    # We'll search by this reference instead of a phone number
+    search_term = "some-ref"
+
+    results_page = dao_get_notifications_by_recipient_or_reference(
+        service_id=service.id,
+        search_term=search_term,
+        notification_type=NotificationType.SMS,
+        statuses=[NotificationStatus.FAILED],
+        page=1,
+        page_size=50,
+    )
+
+    # Now we should find exactly one match
+    assert len(results_page.items) == 1, "Should find exactly one matching notification"
+    found = results_page.items[0]
+    assert found.id == notification.id
+    assert found.status == NotificationStatus.FAILED
+    assert found.client_reference == "some-ref"

tests/app/dao/test_fact_notification_status_dao.py

@@ -36,7 +36,6 @@ def test_fetch_notification_status_for_service_by_month(notify_db_session):
     service_2 = create_service(service_name="service_2")
 
     create_template(service=service_1)
-    create_template(service=service_1, template_type=TemplateType.EMAIL)
     # not the service being tested
     create_template(service=service_2)
 
@@ -50,11 +49,7 @@ def test_fetch_notification_status_for_service_by_month(notify_db_session):
     create_notification(
         service_1.templates[0], created_at=datetime(2018, 1, 1, 1, 1, 0)
     )
-    create_notification(
-        service_1.templates[1],
-        created_at=datetime(2018, 1, 1, 1, 1, 0),
-        status=NotificationStatus.DELIVERED,
-    )
+
     create_notification(
         service_1.templates[0],
         created_at=datetime(2018, 2, 1, 1, 1, 0),
@@ -79,30 +74,25 @@ def test_fetch_notification_status_for_service_by_month(notify_db_session):
         fetch_notification_status_for_service_by_month(
             date(2018, 1, 1), date(2018, 2, 28), service_1.id
         ),
-        key=lambda x: (x.month, x.notification_type, x.notification_status),
+        key=lambda x: (x.month, x.notification_status),
     )
 
-    assert len(results) == 4
+    assert len(results) == 3
 
     assert results[0].month.date() == date(2018, 1, 1)
-    assert results[0].notification_type == NotificationType.EMAIL
-    assert results[0].notification_status == NotificationStatus.DELIVERED
+    assert results[0].notification_type == NotificationType.SMS
+    assert results[0].notification_status == NotificationStatus.CREATED
     assert results[0].count == 1
 
     assert results[1].month.date() == date(2018, 1, 1)
     assert results[1].notification_type == NotificationType.SMS
-    assert results[1].notification_status == NotificationStatus.CREATED
-    assert results[1].count == 1
+    assert results[1].notification_status == NotificationStatus.DELIVERED
+    assert results[1].count == 14
 
-    assert results[2].month.date() == date(2018, 1, 1)
+    assert results[2].month.date() == date(2018, 2, 1)
     assert results[2].notification_type == NotificationType.SMS
     assert results[2].notification_status == NotificationStatus.DELIVERED
-    assert results[2].count == 14
-
-    assert results[3].month.date() == date(2018, 2, 1)
-    assert results[3].notification_type == NotificationType.SMS
-    assert results[3].notification_status == NotificationStatus.DELIVERED
-    assert results[3].count == 1
+    assert results[2].count == 1
 
 
 def test_fetch_notification_status_for_service_for_day(notify_db_session):

tests/app/organization/test_rest.py

@@ -831,7 +831,9 @@ def test_get_organization_users_returns_users_for_organization(
     )
 
     assert len(response["data"]) == 2
-    assert response["data"][0]["id"] == str(first.id)
+    response_ids = [response["data"][0]["id"], response["data"][1]["id"]]
+    assert str(first.id) in response_ids
+    assert str(second.id) in response_ids
 
 
 @freeze_time("2019-12-24 13:30")

tests/app/test_commands.py

@@ -15,6 +15,7 @@ from app.commands import (
     dump_sms_senders,
     dump_user_info,
     fix_billable_units,
+    generate_salt,
     insert_inbound_numbers_from_file,
     populate_annual_billing_with_defaults,
     populate_annual_billing_with_the_previous_years_allowance,
@@ -661,3 +662,10 @@ def test_dump_user_info(notify_api, mocker):
 
     mock_get_user_by_email.assert_called_once_with("john@example.com")
     mock_open_file.assert_called_once_with("user_download.json", "wb")
+
+
+def test_generate_salt(notify_api):
+    runner = notify_api.test_cli_runner()
+    result = runner.invoke(generate_salt)
+    assert result.exit_code == 0
+    assert len(result.output.strip()) == 32