Skip to content

Commit e3abcde

Browse files
authored
Track delivered & failed notification counts (#2338)
* Track delivered & failed notification counts - Increment daily delivered and failed notification counts for a service upon ses, sns, or pinpoint receipt received * Bump utils version * Update waffles version in WAF rule github action
1 parent 440935f commit e3abcde

10 files changed

+231
-8
lines changed

.github/workflows/test.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ jobs:
6767
run: |
6868
cp -f .env.example .env
6969
- name: Checks for new endpoints against AWS WAF rules
70-
uses: cds-snc/notification-utils/.github/actions/waffles@52.2.9
70+
uses: cds-snc/notification-utils/.github/actions/waffles@52.3.6
7171
with:
7272
app-loc: '/github/workspace'
7373
app-libs: '/github/workspace/env/site-packages'

app/__init__.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
from flask_migrate import Migrate
1313
from flask_redis import FlaskRedis
1414
from notifications_utils import logging, request_helper
15+
from notifications_utils.clients.redis.annual_limit import RedisAnnualLimit
1516
from notifications_utils.clients.redis.bounce_rate import RedisBounceRate
1617
from notifications_utils.clients.redis.redis_client import RedisClient
1718
from notifications_utils.clients.statsd.statsd_client import StatsdClient
@@ -60,6 +61,7 @@
6061
flask_redis_publish = FlaskRedis(config_prefix="REDIS_PUBLISH")
6162
redis_store = RedisClient()
6263
bounce_rate_client = RedisBounceRate(redis_store)
64+
annual_limit_client = RedisAnnualLimit(redis_store)
6365
metrics_logger = MetricsLogger()
6466
# TODO: Rework instantiation to decouple redis_store.redis_store and pass it in.\
6567
email_queue = RedisQueue("email")

app/aws/mocks.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,10 @@ def ses_soft_bounce_callback(reference, bounce_subtype=None):
7373
return _ses_bounce_callback(reference, "Transient", bounce_subtype)
7474

7575

76+
def ses_unknown_bounce_callback(reference, bounce_subtype=None):
77+
return _ses_bounce_callback(reference, "unknown-bounce", bounce_subtype)
78+
79+
7680
def ses_complaint_callback_malformed_message_id():
7781
return {
7882
"Signature": "bb",

app/celery/process_pinpoint_receipts_tasks.py

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
from notifications_utils.statsd_decorators import statsd
66
from sqlalchemy.orm.exc import NoResultFound
77

8-
from app import notify_celery, statsd_client
8+
from app import annual_limit_client, notify_celery, statsd_client
99
from app.config import QueueNames
1010
from app.dao import notifications_dao
1111
from app.models import (
@@ -43,6 +43,8 @@
4343
# }
4444

4545

46+
# TODO FF_ANNUAL_LIMIT removal: Temporarily ignore complexity
47+
# flake8: noqa: C901
4648
@notify_celery.task(bind=True, name="process-pinpoint-result", max_retries=5, default_retry_delay=300)
4749
@statsd(namespace="tasks")
4850
def process_pinpoint_results(self, response):
@@ -111,10 +113,22 @@ def process_pinpoint_results(self, response):
111113
f"Provider response: {provider_response}"
112114
)
113115
)
116+
# TODO FF_ANNUAL_LIMIT removal
117+
if current_app.config["FF_ANNUAL_LIMIT"]:
118+
annual_limit_client.increment_sms_failed(notification.service_id)
119+
current_app.logger.info(
120+
f"Incremented sms_delivered count in Redis. Service: {notification.service_id} Notification: {notification.id} Current counts: {annual_limit_client.get_all_notification_counts(notification.service_id)}"
121+
)
114122
else:
115123
current_app.logger.info(
116124
f"Pinpoint callback return status of {notification_status} for notification: {notification.id}"
117125
)
126+
# TODO FF_ANNUAL_LIMIT removal
127+
if current_app.config["FF_ANNUAL_LIMIT"]:
128+
annual_limit_client.increment_sms_delivered(notification.service_id)
129+
current_app.logger.info(
130+
f"Incremented sms_delivered count in Redis. Service: {notification.service_id} Notification: {notification.id} Current counts: {annual_limit_client.get_all_notification_counts(notification.service_id)}"
131+
)
118132

119133
statsd_client.incr(f"callback.pinpoint.{notification_status}")
120134

app/celery/process_ses_receipts_tasks.py

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
from notifications_utils.statsd_decorators import statsd
55
from sqlalchemy.orm.exc import NoResultFound
66

7-
from app import bounce_rate_client, notify_celery, statsd_client
7+
from app import annual_limit_client, bounce_rate_client, notify_celery, statsd_client
88
from app.config import QueueNames
99
from app.dao import notifications_dao
1010
from app.models import NOTIFICATION_DELIVERED, NOTIFICATION_PERMANENT_FAILURE
@@ -89,10 +89,20 @@ def process_ses_results(self, response): # noqa: C901
8989
notification.id, reference, aws_response_dict["message"]
9090
)
9191
)
92+
if current_app.config["FF_ANNUAL_LIMIT"]:
93+
annual_limit_client.increment_email_failed(notification.service_id)
94+
current_app.logger.info(
95+
f"Incremented email_failed count in Redis. Service: {notification.service_id} Notification: {notification.id} Current counts: {annual_limit_client.get_all_notification_counts(notification.service_id)}"
96+
)
9297
else:
9398
current_app.logger.info(
9499
"SES callback return status of {} for notification: {}".format(notification_status, notification.id)
95100
)
101+
if current_app.config["FF_ANNUAL_LIMIT"]:
102+
annual_limit_client.increment_email_delivered(notification.service_id)
103+
current_app.logger.info(
104+
f"Incremented email_delivered count in Redis. Service: {notification.service_id} Notification: {notification.id} current counts: {annual_limit_client.get_all_notification_counts(notification.service_id)}"
105+
)
96106

97107
statsd_client.incr("callback.ses.{}".format(notification_status))
98108

app/celery/process_sns_receipts_tasks.py

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
from notifications_utils.statsd_decorators import statsd
55
from sqlalchemy.orm.exc import NoResultFound
66

7-
from app import notify_celery, statsd_client
7+
from app import annual_limit_client, notify_celery, statsd_client
88
from app.config import QueueNames
99
from app.dao import notifications_dao
1010
from app.models import (
@@ -19,6 +19,8 @@
1919
from celery.exceptions import Retry
2020

2121

22+
# TODO: FF_ANNUAL_LIMIT removal: Temporarily ignore complexity
23+
# flake8: noqa: C901
2224
@notify_celery.task(bind=True, name="process-sns-result", max_retries=5, default_retry_delay=300)
2325
@statsd(namespace="tasks")
2426
def process_sns_results(self, response):
@@ -69,8 +71,20 @@ def process_sns_results(self, response):
6971
f"Provider response: {sns_message['delivery']['providerResponse']}"
7072
)
7173
)
74+
# TODO FF_ANNUAL_LIMIT removal
75+
if current_app.config["FF_ANNUAL_LIMIT"]:
76+
annual_limit_client.increment_sms_failed(notification.service_id)
77+
current_app.logger.info(
78+
f"Incremented sms_failed count in Redis. Service: {notification.service_id} Notification: {notification.id} Current counts: {annual_limit_client.get_all_notification_counts(notification.service_id)}"
79+
)
7280
else:
7381
current_app.logger.info(f"SNS callback return status of {notification_status} for notification: {notification.id}")
82+
# TODO FF_ANNUAL_LIMIT removal
83+
if current_app.config["FF_ANNUAL_LIMIT"]:
84+
annual_limit_client.increment_sms_delivered(notification.service_id)
85+
current_app.logger.info(
86+
f"Incremented sms_delivered count in Redis. Service: {notification.service_id} Notification: {notification.id} Current counts: {annual_limit_client.get_all_notification_counts(notification.service_id)}"
87+
)
7488

7589
statsd_client.incr(f"callback.sns.{notification_status}")
7690

pyproject.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ MarkupSafe = "2.1.5"
6666
# REVIEW: v2 is using sha512 instead of sha1 by default (in v1)
6767
itsdangerous = "2.2.0"
6868
notifications-utils = { git = "https://github.com/cds-snc/notifier-utils.git", tag = "52.3.6" }
69+
6970
# rsa = "4.9 # awscli 1.22.38 depends on rsa<4.8
7071
typing-extensions = "4.12.2"
7172
greenlet = "2.0.2"

tests/app/celery/test_process_pinpoint_receipts_tasks.py

Lines changed: 76 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
import pytest
44
from freezegun import freeze_time
55

6-
from app import statsd_client
6+
from app import annual_limit_client, statsd_client
77
from app.aws.mocks import (
88
pinpoint_delivered_callback,
99
pinpoint_delivered_callback_missing_sms_data,
@@ -28,6 +28,7 @@
2828
create_service_callback_api,
2929
save_notification,
3030
)
31+
from tests.conftest import set_config
3132

3233

3334
@pytest.mark.parametrize(
@@ -278,3 +279,77 @@ def test_process_pinpoint_results_calls_service_callback(sample_template, notify
278279
mock_send_status.assert_called_once_with(
279280
[str(notification.id), signed_data, notification.service_id], queue="service-callbacks"
280281
)
282+
283+
284+
@pytest.mark.parametrize(
285+
"provider_response",
286+
[
287+
"Blocked as spam by phone carrier",
288+
"Destination is on a blocked list",
289+
"Invalid phone number",
290+
"Message body is invalid",
291+
"Phone carrier has blocked this message",
292+
"Phone carrier is currently unreachable/unavailable",
293+
"Phone has blocked SMS",
294+
"Phone is on a blocked list",
295+
"Phone is currently unreachable/unavailable",
296+
"Phone number is opted out",
297+
"This delivery would exceed max price",
298+
"Unknown error attempting to reach phone",
299+
],
300+
)
301+
def test_process_pinpoint_results_should_increment_sms_failed_when_delivery_receipt_is_failure(
302+
sample_sms_template_with_html,
303+
notify_api,
304+
mocker,
305+
provider_response,
306+
):
307+
mocker.patch("app.annual_limit_client.increment_sms_delivered")
308+
mocker.patch("app.annual_limit_client.increment_sms_failed")
309+
310+
notification = save_notification(
311+
create_notification(
312+
sample_sms_template_with_html,
313+
reference="ref",
314+
sent_at=datetime.utcnow(),
315+
status=NOTIFICATION_SENT,
316+
sent_by="pinpoint",
317+
)
318+
)
319+
# TODO FF_ANNUAL_LIMIT removal
320+
with set_config(notify_api, "FF_ANNUAL_LIMIT", True):
321+
process_pinpoint_results(pinpoint_failed_callback(reference="ref", provider_response=provider_response))
322+
annual_limit_client.increment_sms_failed.assert_called_once_with(notification.service_id)
323+
annual_limit_client.increment_sms_delivered.assert_not_called()
324+
325+
326+
@pytest.mark.parametrize(
327+
"callback",
328+
[
329+
(pinpoint_delivered_callback),
330+
(pinpoint_shortcode_delivered_callback),
331+
],
332+
)
333+
def test_process_pinpoint_results_should_increment_sms_delivered_when_delivery_receipt_is_success(
334+
sample_sms_template_with_html,
335+
notify_api,
336+
mocker,
337+
callback,
338+
):
339+
mocker.patch("app.annual_limit_client.increment_sms_delivered")
340+
mocker.patch("app.annual_limit_client.increment_sms_failed")
341+
342+
notification = save_notification(
343+
create_notification(
344+
sample_sms_template_with_html,
345+
reference="ref",
346+
sent_at=datetime.utcnow(),
347+
status=NOTIFICATION_SENT,
348+
sent_by="pinpoint",
349+
)
350+
)
351+
# TODO FF_ANNUAL_LIMIT removal
352+
with set_config(notify_api, "FF_ANNUAL_LIMIT", True):
353+
process_pinpoint_results(callback(reference="ref"))
354+
annual_limit_client.increment_sms_delivered.assert_called_once_with(notification.service_id)
355+
annual_limit_client.increment_sms_failed.assert_not_called()

tests/app/celery/test_process_ses_receipts_tasks.py

Lines changed: 42 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@
44
import pytest
55
from freezegun import freeze_time
66

7-
from app import bounce_rate_client, signer_complaint, statsd_client
8-
from app.aws.mocks import ses_complaint_callback
7+
from app import annual_limit_client, bounce_rate_client, signer_complaint, statsd_client
8+
from app.aws.mocks import ses_complaint_callback, ses_unknown_bounce_callback
99
from app.celery.process_ses_receipts_tasks import process_ses_results
1010
from app.celery.research_mode_tasks import (
1111
ses_hard_bounce_callback,
@@ -26,6 +26,7 @@
2626
NOTIFICATION_SOFT_GENERAL,
2727
NOTIFICATION_SOFT_MAILBOXFULL,
2828
NOTIFICATION_SOFT_MESSAGETOOLARGE,
29+
NOTIFICATION_UNKNOWN_BOUNCE,
2930
Complaint,
3031
Notification,
3132
)
@@ -41,6 +42,7 @@
4142
create_service_callback_api,
4243
save_notification,
4344
)
45+
from tests.conftest import set_config
4446

4547

4648
def test_process_ses_results(sample_email_template):
@@ -436,3 +438,41 @@ def test_ses_callback_should_not_add_redis_keys_when_delivery_receipt_is_soft_bo
436438

437439
bounce_rate_client.set_sliding_hard_bounce.assert_not_called()
438440
bounce_rate_client.set_sliding_notifications.assert_not_called()
441+
442+
443+
class TestAnnualLimits:
444+
def test_ses_callback_should_increment_email_delivered_when_delivery_receipt_is_delivered(
445+
self, notify_api, sample_email_template, mocker
446+
):
447+
mocker.patch("app.annual_limit_client.increment_email_delivered")
448+
mocker.patch("app.annual_limit_client.increment_email_failed")
449+
450+
# TODO FF_ANNUAL_LIMIT removal
451+
with set_config(notify_api, "FF_ANNUAL_LIMIT", True):
452+
save_notification(create_notification(template=sample_email_template, reference="ref", status="sending"))
453+
454+
assert process_ses_results(ses_notification_callback(reference="ref"))
455+
annual_limit_client.increment_email_delivered.assert_called_once_with(sample_email_template.service_id)
456+
annual_limit_client.increment_email_failed.assert_not_called()
457+
458+
@pytest.mark.parametrize(
459+
"callback, bounce_type",
460+
[
461+
(ses_hard_bounce_callback, NOTIFICATION_HARD_BOUNCE),
462+
(ses_soft_bounce_callback, NOTIFICATION_SOFT_BOUNCE),
463+
(ses_unknown_bounce_callback, NOTIFICATION_UNKNOWN_BOUNCE),
464+
],
465+
)
466+
def test_ses_callback_should_increment_email_failed_when_delivery_receipt_is_failure(
467+
self, notify_api, sample_email_template, mocker, callback, bounce_type
468+
):
469+
mocker.patch("app.annual_limit_client.increment_email_failed")
470+
mocker.patch("app.annual_limit_client.increment_email_delivered")
471+
472+
# TODO FF_ANNUAL_LIMIT removal
473+
with set_config(notify_api, "FF_ANNUAL_LIMIT", True):
474+
save_notification(create_notification(template=sample_email_template, reference="ref", status="sending"))
475+
476+
assert process_ses_results(callback(reference="ref"))
477+
annual_limit_client.increment_email_failed.assert_called_once_with(sample_email_template.service_id)
478+
annual_limit_client.increment_email_delivered.assert_not_called()

0 commit comments

Comments
 (0)