diff --git a/app/clients/cloudwatch/aws_cloudwatch.py b/app/clients/cloudwatch/aws_cloudwatch.py index eb8343810..9c1d1e61c 100644 --- a/app/clients/cloudwatch/aws_cloudwatch.py +++ b/app/clients/cloudwatch/aws_cloudwatch.py @@ -1,7 +1,6 @@ import json import os import re -import time from datetime import datetime, timedelta from boto3 import client @@ -51,25 +50,30 @@ class AwsCloudwatchClient(Client): def _get_log(self, my_filter, log_group_name, sent_at): # Check all cloudwatch logs from the time the notification was sent (currently 5 minutes previously) until now - now = round(time.time() * 1000) + now = datetime.utcnow() beginning = sent_at next_token = None all_log_events = [] + current_app.logger.info(f"START TIME {beginning} END TIME {now}") + # There has been a change somewhere and the time range we were previously using has become too + # narrow or wrong in some way, so events can't be found. For the time being, adjust by adding + # a buffer on each side of 12 hours. + TWELVE_HOURS = 12 * 60 * 60 * 1000 while True: if next_token: response = self._client.filter_log_events( logGroupName=log_group_name, filterPattern=my_filter, nextToken=next_token, - startTime=int(beginning.timestamp() * 1000), - endTime=now, + startTime=int(beginning.timestamp() * 1000) - TWELVE_HOURS, + endTime=int(now.timestamp() * 1000) + TWELVE_HOURS, ) else: response = self._client.filter_log_events( logGroupName=log_group_name, filterPattern=my_filter, - startTime=int(beginning.timestamp() * 1000), - endTime=now, + startTime=int(beginning.timestamp() * 1000) - TWELVE_HOURS, + endTime=int(now.timestamp() * 1000) + TWELVE_HOURS, ) log_events = response.get("events", []) all_log_events.extend(log_events) @@ -87,6 +91,7 @@ class AwsCloudwatchClient(Client): return account_number def check_sms(self, message_id, notification_id, created_at): + current_app.logger.info(f"CREATED AT = {created_at}") region = cloud_config.sns_region # TODO this clumsy approach to getting the account number will be fixed as part of notify-api #258 account_number = self._extract_account_number(cloud_config.ses_domain_arn) @@ -128,7 +133,7 @@ class AwsCloudwatchClient(Client): if time_now > (created_at + timedelta(hours=3)): # see app/models.py Notification. This message corresponds to "permanent-failure", # but we are copy/pasting here to avoid circular imports. - return "failure", "Unable to find carrier response.", "unknown" + return "failure", "Unable to find carrier response." raise NotificationTechnicalFailureException( f"No event found for message_id {message_id} notification_id {notification_id}" ) diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index 6c59aaec2..61fba4f6f 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -304,12 +304,17 @@ def _filter_query(query, filter_dict=None): return query -def sanitize_successful_notification_by_id(notification_id, provider_response): +def sanitize_successful_notification_by_id(notification_id, carrier, provider_response): update_query = """ - update notifications set provider_response=:response, notification_status='delivered', "to"='1', normalised_to='1' + update notifications set provider_response=:response, carrier=:carrier, + notification_status='delivered', "to"='1', normalised_to='1' where id=:notification_id """ - input_params = {"notification_id": notification_id, "response": provider_response} + input_params = { + "notification_id": notification_id, + "carrier": carrier, + "response": provider_response, + } db.session.execute(update_query, input_params) db.session.commit() diff --git a/app/models.py b/app/models.py index 1cba15c6b..c46aa6bd9 100644 --- a/app/models.py +++ b/app/models.py @@ -554,8 +554,8 @@ class Service(db.Model, Versioned): def get_default_sms_sender(self): default_sms_sender = [x for x in self.service_sms_senders if x.is_default] - return "sns" - # return default_sms_sender[0].sms_sender + # return "sns" + return default_sms_sender[0].sms_sender def get_default_reply_to_email_address(self): default_reply_to = [x for x in self.reply_to_email_addresses if x.is_default] diff --git a/tests/app/clients/test_aws_cloudwatch.py b/tests/app/clients/test_aws_cloudwatch.py index 6662d2edb..5eeffb979 100644 --- a/tests/app/clients/test_aws_cloudwatch.py +++ b/tests/app/clients/test_aws_cloudwatch.py @@ -30,7 +30,8 @@ def side_effect(filterPattern, logGroupName, startTime, endTime): "events": [ { "logStreamName": "89db9712-c6d1-49f9-be7c-4caa7ed9efb1", - "message": '{"delivery":{"destination":"+1661","providerResponse":"Invalid phone number"}}', + "message": '{"delivery":{"destination":"+1661","phoneCarrier":"ATT Mobility", ' + '"providerResponse":"Invalid phone number"}}', "eventId": "37535432778099870001723210579798865345508698025292922880", } ] @@ -42,7 +43,8 @@ def side_effect(filterPattern, logGroupName, startTime, endTime): { "logStreamName": "89db9712-c6d1-49f9-be7c-4caa7ed9efb1", "timestamp": 1683147017911, - "message": '{"delivery":{"destination":"+1661","providerResponse":"Phone accepted msg"}}', + "message": '{"delivery":{"destination":"+1661","phoneCarrier":"ATT Mobility",' + '"providerResponse":"Phone accepted msg"}}', "ingestionTime": 1683147018026, "eventId": "37535432778099870001723210579798865345508698025292922880", } diff --git a/tests/app/dao/notification_dao/test_notification_dao.py b/tests/app/dao/notification_dao/test_notification_dao.py index babddda1b..a0d19ecd1 100644 --- a/tests/app/dao/notification_dao/test_notification_dao.py +++ b/tests/app/dao/notification_dao/test_notification_dao.py @@ -111,7 +111,7 @@ def test_should_be_able_to_sanitize_successful_notification( with freeze_time("2000-01-02 12:00:00"): sanitize_successful_notification_by_id( - notification.id, provider_response="Don't know what happened" + notification.id, carrier="ATT", provider_response="Don't know what happened" ) assert Notification.query.get(notification.id).status == "delivered" assert Notification.query.get(notification.id).normalised_to == "1" diff --git a/tests/app/v2/notifications/test_get_notifications.py b/tests/app/v2/notifications/test_get_notifications.py index a618933e3..be03595b9 100644 --- a/tests/app/v2/notifications/test_get_notifications.py +++ b/tests/app/v2/notifications/test_get_notifications.py @@ -67,6 +67,7 @@ def test_get_notification_by_id_returns_200( "completed_at": sample_notification.completed_at(), "scheduled_for": None, "provider_response": None, + "carrier": None, } assert json_response == expected_response @@ -122,6 +123,7 @@ def test_get_notification_by_id_with_placeholders_returns_200( "completed_at": sample_notification.completed_at(), "scheduled_for": None, "provider_response": None, + "carrier": None } assert json_response == expected_response