diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index bdffe264a4..037f12dbb5 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -69,7 +69,7 @@ jobs: run: | cp -f .env.example .env - name: Checks for new endpoints against AWS WAF rules - uses: cds-snc/notification-utils/.github/actions/waffles@52.3.7 + uses: cds-snc/notification-utils/.github/actions/waffles@52.3.6 with: app-loc: '/github/workspace' app-libs: '/github/workspace/env/site-packages' diff --git a/app/celery/process_pinpoint_receipts_tasks.py b/app/celery/process_pinpoint_receipts_tasks.py index 5edd796169..d1732c9b38 100644 --- a/app/celery/process_pinpoint_receipts_tasks.py +++ b/app/celery/process_pinpoint_receipts_tasks.py @@ -3,15 +3,11 @@ from flask import current_app, json from notifications_utils.statsd_decorators import statsd -from notifications_utils.timezones import convert_utc_to_local_timezone from sqlalchemy.orm.exc import NoResultFound from app import annual_limit_client, notify_celery, statsd_client from app.config import QueueNames from app.dao import notifications_dao -from app.dao.fact_notification_status_dao import ( - fetch_notification_status_for_service_for_day, -) from app.models import ( NOTIFICATION_DELIVERED, NOTIFICATION_PERMANENT_FAILURE, @@ -21,7 +17,6 @@ PINPOINT_PROVIDER, ) from app.notifications.callbacks import _check_and_queue_callback_task -from app.utils import prepare_notification_counts_for_seeding from celery.exceptions import Retry # Pinpoint receipts are of the form: @@ -111,22 +106,6 @@ def process_pinpoint_results(self, response): sms_origination_phone_number=origination_phone_number, ) - service_id = notification.service_id - # Flags if seeding has occurred. Since we seed after updating the notification status in the DB then the current notification - # is included in the fetch_notification_status_for_service_for_day call below, thus we don't need to increment the count. - notifications_to_seed = None - - if current_app.config["FF_ANNUAL_LIMIT"]: - if not annual_limit_client.was_seeded_today(service_id): - annual_limit_client.set_seeded_at(service_id) - notifications_to_seed = fetch_notification_status_for_service_for_day( - convert_utc_to_local_timezone(datetime.utcnow()), - service_id=service_id, - ) - annual_limit_client.seed_annual_limit_notifications( - service_id, prepare_notification_counts_for_seeding(notifications_to_seed) - ) - if notification_status != NOTIFICATION_DELIVERED: current_app.logger.info( ( @@ -136,34 +115,25 @@ def process_pinpoint_results(self, response): ) # TODO FF_ANNUAL_LIMIT removal if current_app.config["FF_ANNUAL_LIMIT"]: - # Only increment if we didn't just seed. - if notifications_to_seed is None: - annual_limit_client.increment_sms_failed(service_id) + annual_limit_client.increment_sms_failed(notification.service_id) current_app.logger.info( - f"Incremented sms_delivered count in Redis. Service: {service_id} Notification: {notification.id} Current counts: {annual_limit_client.get_all_notification_counts(service_id)}" + 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)}" ) else: current_app.logger.info( f"Pinpoint callback return status of {notification_status} for notification: {notification.id}" ) - # TODO FF_ANNUAL_LIMIT removal if current_app.config["FF_ANNUAL_LIMIT"]: - # Only increment if we didn't just seed. - if notifications_to_seed is None: - annual_limit_client.increment_sms_delivered(service_id) + annual_limit_client.increment_sms_delivered(notification.service_id) current_app.logger.info( - f"Incremented sms_delivered count in Redis. Service: {service_id} Notification: {notification.id} Current counts: {annual_limit_client.get_all_notification_counts(service_id)}" + 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)}" ) statsd_client.incr(f"callback.pinpoint.{notification_status}") if notification.sent_at: - statsd_client.timing_with_dates( - "callback.pinpoint.elapsed-time", - datetime.utcnow(), - notification.sent_at, - ) + statsd_client.timing_with_dates("callback.pinpoint.elapsed-time", datetime.utcnow(), notification.sent_at) _check_and_queue_callback_task(notification) diff --git a/app/celery/process_ses_receipts_tasks.py b/app/celery/process_ses_receipts_tasks.py index 5d2c29b133..86811bb720 100644 --- a/app/celery/process_ses_receipts_tasks.py +++ b/app/celery/process_ses_receipts_tasks.py @@ -2,15 +2,11 @@ from flask import current_app, json from notifications_utils.statsd_decorators import statsd -from notifications_utils.timezones import convert_utc_to_local_timezone from sqlalchemy.orm.exc import NoResultFound from app import annual_limit_client, bounce_rate_client, notify_celery, statsd_client from app.config import QueueNames from app.dao import notifications_dao -from app.dao.fact_notification_status_dao import ( - fetch_notification_status_for_service_for_day, -) from app.models import NOTIFICATION_DELIVERED, NOTIFICATION_PERMANENT_FAILURE from app.notifications.callbacks import _check_and_queue_callback_task from app.notifications.notifications_ses_callback import ( @@ -18,7 +14,6 @@ get_aws_responses, handle_complaint, ) -from app.utils import prepare_notification_counts_for_seeding from celery.exceptions import Retry @@ -88,22 +83,6 @@ def process_ses_results(self, response): # noqa: C901 bounce_response=aws_response_dict.get("bounce_response", None), ) - service_id = notification.service_id - # Flags if seeding has occurred. Since we seed after updating the notification status in the DB then the current notification - # is included in the fetch_notification_status_for_service_for_day call below, thus we don't need to increment the count. - notifications_to_seed = None - # Check if we have already seeded the annual limit counts for today - if current_app.config["FF_ANNUAL_LIMIT"]: - if not annual_limit_client.was_seeded_today(service_id): - annual_limit_client.set_seeded_at(service_id) - notifications_to_seed = fetch_notification_status_for_service_for_day( - convert_utc_to_local_timezone(datetime.utcnow()), - service_id=service_id, - ) - annual_limit_client.seed_annual_limit_notifications( - service_id, prepare_notification_counts_for_seeding(notifications_to_seed) - ) - if not aws_response_dict["success"]: current_app.logger.info( "SES delivery failed: notification id {} and reference {} has error found. Status {}".format( @@ -111,9 +90,7 @@ def process_ses_results(self, response): # noqa: C901 ) ) if current_app.config["FF_ANNUAL_LIMIT"]: - # Only increment if we didn't just seed. - if notifications_to_seed is None: - annual_limit_client.increment_email_failed(notification.service_id) + annual_limit_client.increment_email_failed(notification.service_id) current_app.logger.info( 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)}" ) @@ -122,9 +99,7 @@ def process_ses_results(self, response): # noqa: C901 "SES callback return status of {} for notification: {}".format(notification_status, notification.id) ) if current_app.config["FF_ANNUAL_LIMIT"]: - # Only increment if we didn't just seed. - if notifications_to_seed is None: - annual_limit_client.increment_email_delivered(notification.service_id) + annual_limit_client.increment_email_delivered(notification.service_id) current_app.logger.info( 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)}" ) diff --git a/app/celery/process_sns_receipts_tasks.py b/app/celery/process_sns_receipts_tasks.py index 326784f3e3..1ca9608a6a 100644 --- a/app/celery/process_sns_receipts_tasks.py +++ b/app/celery/process_sns_receipts_tasks.py @@ -2,15 +2,11 @@ from flask import current_app, json from notifications_utils.statsd_decorators import statsd -from notifications_utils.timezones import convert_utc_to_local_timezone from sqlalchemy.orm.exc import NoResultFound from app import annual_limit_client, notify_celery, statsd_client from app.config import QueueNames from app.dao import notifications_dao -from app.dao.fact_notification_status_dao import ( - fetch_notification_status_for_service_for_day, -) from app.models import ( NOTIFICATION_DELIVERED, NOTIFICATION_PERMANENT_FAILURE, @@ -20,7 +16,6 @@ SNS_PROVIDER, ) from app.notifications.callbacks import _check_and_queue_callback_task -from app.utils import prepare_notification_counts_for_seeding from celery.exceptions import Retry @@ -69,21 +64,6 @@ def process_sns_results(self, response): provider_response=provider_response, ) - service_id = notification.service_id - # Flags if seeding has occurred. Since we seed after updating the notification status in the DB then the current notification - # is included in the fetch_notification_status_for_service_for_day call below, thus we don't need to increment the count. - notifications_to_seed = None - if current_app.config["FF_ANNUAL_LIMIT"]: - if not annual_limit_client.was_seeded_today(service_id): - annual_limit_client.set_seeded_at(service_id) - notifications_to_seed = fetch_notification_status_for_service_for_day( - convert_utc_to_local_timezone(datetime.utcnow()), - service_id=service_id, - ) - annual_limit_client.seed_annual_limit_notifications( - service_id, prepare_notification_counts_for_seeding(notifications_to_seed) - ) - if notification_status != NOTIFICATION_DELIVERED: current_app.logger.info( ( @@ -93,9 +73,7 @@ def process_sns_results(self, response): ) # TODO FF_ANNUAL_LIMIT removal if current_app.config["FF_ANNUAL_LIMIT"]: - # Only increment if we didn't just seed. - if notifications_to_seed is None: - annual_limit_client.increment_sms_failed(notification.service_id) + annual_limit_client.increment_sms_failed(notification.service_id) current_app.logger.info( 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)}" ) @@ -103,9 +81,7 @@ def process_sns_results(self, response): current_app.logger.info(f"SNS callback return status of {notification_status} for notification: {notification.id}") # TODO FF_ANNUAL_LIMIT removal if current_app.config["FF_ANNUAL_LIMIT"]: - # Only increment if we didn't just seed. - if notifications_to_seed is None: - annual_limit_client.increment_sms_delivered(notification.service_id) + annual_limit_client.increment_sms_delivered(notification.service_id) current_app.logger.info( 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)}" ) diff --git a/app/celery/reporting_tasks.py b/app/celery/reporting_tasks.py index c74b1de247..8090d42bb7 100644 --- a/app/celery/reporting_tasks.py +++ b/app/celery/reporting_tasks.py @@ -5,7 +5,7 @@ from notifications_utils.statsd_decorators import statsd from notifications_utils.timezones import convert_utc_to_local_timezone -from app import annual_limit_client, notify_celery +from app import notify_celery from app.config import QueueNames from app.cronitor import cronitor from app.dao.annual_limits_data_dao import get_previous_quarter, insert_quarter_data @@ -113,10 +113,6 @@ def create_nightly_notification_status_for_day(process_day): len(transit_data), process_day, chunk ) ) - # TODO: FF_ANNUAL_LIMIT removal - if current_app.config["FF_ANNUAL_LIMIT"]: - annual_limit_client.reset_all_notification_counts(chunk) - except Exception as e: current_app.logger.error( "create-nightly-notification-status-for-day task failed for day: {}, for service_ids: {}. Error: {}".format( diff --git a/app/dao/fact_notification_status_dao.py b/app/dao/fact_notification_status_dao.py index 75e0ca1371..dc3ec5be7d 100644 --- a/app/dao/fact_notification_status_dao.py +++ b/app/dao/fact_notification_status_dao.py @@ -130,18 +130,6 @@ def update_fact_notification_status(data, process_day, service_ids=None): def fetch_notification_status_for_service_by_month(start_date, end_date, service_id): - filters = [ - FactNotificationStatus.service_id == service_id, - FactNotificationStatus.bst_date >= start_date.strftime("%Y-%m-%d"), - # This works only for timezones to the west of GMT - FactNotificationStatus.bst_date < end_date.strftime("%Y-%m-%d"), - FactNotificationStatus.key_type != KEY_TYPE_TEST, - ] - - # TODO FF_ANNUAL_LIMIT removal - if current_app.config["FF_ANNUAL_LIMIT"]: - filters.append(FactNotificationStatus.bst_date != datetime.utcnow().date().strftime("%Y-%m-%d")) - return ( db.session.query( func.date_trunc("month", FactNotificationStatus.bst_date).label("month"), @@ -149,7 +137,13 @@ def fetch_notification_status_for_service_by_month(start_date, end_date, service FactNotificationStatus.notification_status, func.sum(FactNotificationStatus.notification_count).label("count"), ) - .filter(*filters) + .filter( + FactNotificationStatus.service_id == service_id, + FactNotificationStatus.bst_date >= start_date.strftime("%Y-%m-%d"), + # This works only for timezones to the west of GMT + FactNotificationStatus.bst_date < end_date.strftime("%Y-%m-%d"), + FactNotificationStatus.key_type != KEY_TYPE_TEST, + ) .group_by( func.date_trunc("month", FactNotificationStatus.bst_date).label("month"), FactNotificationStatus.notification_type, diff --git a/app/service/rest.py b/app/service/rest.py index ed3d9b7752..84c2682820 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -404,10 +404,7 @@ def create_api_key(service_id=None): unsigned_api_key = get_unsigned_secret(valid_api_key.id) # prefix the API key so they keys can be easily identified for security scanning - keydata = { - "key": unsigned_api_key, - "key_name": current_app.config["API_KEY_PREFIX"] + valid_api_key.name, - } + keydata = {"key": unsigned_api_key, "key_name": current_app.config["API_KEY_PREFIX"] + valid_api_key.name} return jsonify(data=keydata), 201 @@ -634,8 +631,7 @@ def get_monthly_notification_stats(service_id): statistics.add_monthly_notification_status_stats(data, stats) now = datetime.utcnow() - # TODO FF_ANNUAL_LIMIT removal - if not current_app.config["FF_ANNUAL_LIMIT"] and end_date > now: + if end_date > now: todays_deltas = fetch_notification_status_for_service_for_day(convert_utc_to_local_timezone(now), service_id=service_id) statistics.add_monthly_notification_status_stats(data, todays_deltas) diff --git a/app/utils.py b/app/utils.py index 165991a71d..1836093753 100644 --- a/app/utils.py +++ b/app/utils.py @@ -15,16 +15,6 @@ local_timezone = pytz.timezone(os.getenv("TIMEZONE", "America/Toronto")) -DELIVERED_STATUSES = ["delivered", "sent", "returned-letter"] -FAILURE_STATUSES = [ - "failed", - "temporary-failure", - "permanent-failure", - "technical-failure", - "virus-scan-failed", - "validation-failed", -] - def pagination_links(pagination, endpoint, **kwargs): if "page" in kwargs: @@ -231,28 +221,3 @@ def get_limit_reset_time_et() -> dict[str, str]: limit_reset_time_et = {"12hr": next_midnight_utc_in_et.strftime("%-I%p"), "24hr": next_midnight_utc_in_et.strftime("%H")} return limit_reset_time_et - - -def prepare_notification_counts_for_seeding(notification_counts: list) -> dict: - """Utility method that transforms a list of notification counts into a dictionary, mapping notification counts by type and success/failure. - Used to seed notification counts in Redis for annual limits. - e.g. - ``` - [(datetime, 'email', 'sent', 1), - (datetime, 'sms', 'sent', 2)] - ``` - Becomes: - ``` - {'email_sent': 1, 'sms_sent': 2} - ``` - Args: - notification_counts (list): A list of tuples containing (date, notification_type, status, count) - - Returns: - dict: That acts as a mapping to build the notification counts in Redis - """ - return { - f"{notification_type}_{'delivered' if status in DELIVERED_STATUSES else 'failed'}": count - for _, notification_type, status, count in notification_counts - if status in DELIVERED_STATUSES or status in FAILURE_STATUSES - } diff --git a/poetry.lock b/poetry.lock index e1e1f9a2d4..b2ee531e85 100644 --- a/poetry.lock +++ b/poetry.lock @@ -2657,7 +2657,7 @@ requests = ">=2.0.0" [[package]] name = "notifications-utils" -version = "52.3.7" +version = "52.3.6" description = "Shared python code for Notification - Provides logging utils etc." optional = false python-versions = "~3.10.9" @@ -2693,8 +2693,8 @@ werkzeug = "3.0.4" [package.source] type = "git" url = "https://github.com/cds-snc/notifier-utils.git" -reference = "52.3.7" -resolved_reference = "42ded1e06cfac4209c0dd9d3131cf202b8e210bd" +reference = "52.3.6" +resolved_reference = "6a57a6b91cdfc18b823ad4476744515f89d4b7be" [[package]] name = "ordered-set" @@ -4623,4 +4623,4 @@ testing = ["coverage (>=5.0.3)", "zope.event", "zope.testing"] [metadata] lock-version = "2.0" python-versions = "~3.10.9" -content-hash = "9f820c94df5fda8d9619bba5e0294cc0a8108e68d7bf73646e5e099157f1fe4a" +content-hash = "d6297cc2dcb00a6b90e6a5914a1f5357cce172e853e12f9d221375f276c516d3" diff --git a/pyproject.toml b/pyproject.toml index 4bce92347b..fc9abadfaa 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -65,7 +65,7 @@ Werkzeug = "3.0.4" MarkupSafe = "2.1.5" # REVIEW: v2 is using sha512 instead of sha1 by default (in v1) itsdangerous = "2.2.0" -notifications-utils = { git = "https://github.com/cds-snc/notifier-utils.git", tag = "52.3.7" } +notifications-utils = { git = "https://github.com/cds-snc/notifier-utils.git", tag = "52.3.6" } # rsa = "4.9 # awscli 1.22.38 depends on rsa<4.8 typing-extensions = "4.12.2" diff --git a/tests/app/celery/test_process_pinpoint_receipts_tasks.py b/tests/app/celery/test_process_pinpoint_receipts_tasks.py index ec30fbe563..84c72c3615 100644 --- a/tests/app/celery/test_process_pinpoint_receipts_tasks.py +++ b/tests/app/celery/test_process_pinpoint_receipts_tasks.py @@ -281,120 +281,75 @@ def test_process_pinpoint_results_calls_service_callback(sample_template, notify ) -class TestAnnualLimits: - @pytest.mark.parametrize( - "provider_response", - [ - "Blocked as spam by phone carrier", - "Destination is on a blocked list", - "Invalid phone number", - "Message body is invalid", - "Phone carrier has blocked this message", - "Phone carrier is currently unreachable/unavailable", - "Phone has blocked SMS", - "Phone is on a blocked list", - "Phone is currently unreachable/unavailable", - "Phone number is opted out", - "This delivery would exceed max price", - "Unknown error attempting to reach phone", - ], - ) - def test_process_pinpoint_results_should_increment_sms_failed_when_delivery_receipt_is_failure( - self, - sample_sms_template_with_html, - notify_api, - mocker, - provider_response, - ): - mocker.patch("app.annual_limit_client.increment_sms_delivered") - mocker.patch("app.annual_limit_client.increment_sms_failed") - mocker.patch("app.annual_limit_client.was_seeded_today", return_value=True) - - notification = save_notification( - create_notification( - sample_sms_template_with_html, - reference="ref", - sent_at=datetime.utcnow(), - status=NOTIFICATION_SENT, - sent_by="pinpoint", - ) +@pytest.mark.parametrize( + "provider_response", + [ + "Blocked as spam by phone carrier", + "Destination is on a blocked list", + "Invalid phone number", + "Message body is invalid", + "Phone carrier has blocked this message", + "Phone carrier is currently unreachable/unavailable", + "Phone has blocked SMS", + "Phone is on a blocked list", + "Phone is currently unreachable/unavailable", + "Phone number is opted out", + "This delivery would exceed max price", + "Unknown error attempting to reach phone", + ], +) +def test_process_pinpoint_results_should_increment_sms_failed_when_delivery_receipt_is_failure( + sample_sms_template_with_html, + notify_api, + mocker, + provider_response, +): + mocker.patch("app.annual_limit_client.increment_sms_delivered") + mocker.patch("app.annual_limit_client.increment_sms_failed") + + notification = save_notification( + create_notification( + sample_sms_template_with_html, + reference="ref", + sent_at=datetime.utcnow(), + status=NOTIFICATION_SENT, + sent_by="pinpoint", ) - # TODO FF_ANNUAL_LIMIT removal - with set_config(notify_api, "FF_ANNUAL_LIMIT", True): - process_pinpoint_results(pinpoint_failed_callback(reference="ref", provider_response=provider_response)) - annual_limit_client.increment_sms_failed.assert_called_once_with(notification.service_id) - annual_limit_client.increment_sms_delivered.assert_not_called() - - @pytest.mark.parametrize( - "callback", - [ - (pinpoint_delivered_callback), - (pinpoint_shortcode_delivered_callback), - ], ) - def test_process_pinpoint_results_should_increment_sms_delivered_when_delivery_receipt_is_success( - self, - sample_sms_template_with_html, - notify_api, - mocker, - callback, - ): - mocker.patch("app.annual_limit_client.increment_sms_delivered") - mocker.patch("app.annual_limit_client.increment_sms_failed") - mocker.patch("app.annual_limit_client.was_seeded_today", return_value=True) - - notification = save_notification( - create_notification( - sample_sms_template_with_html, - reference="ref", - sent_at=datetime.utcnow(), - status=NOTIFICATION_SENT, - sent_by="pinpoint", - ) + # TODO FF_ANNUAL_LIMIT removal + with set_config(notify_api, "FF_ANNUAL_LIMIT", True): + process_pinpoint_results(pinpoint_failed_callback(reference="ref", provider_response=provider_response)) + annual_limit_client.increment_sms_failed.assert_called_once_with(notification.service_id) + annual_limit_client.increment_sms_delivered.assert_not_called() + + +@pytest.mark.parametrize( + "callback", + [ + (pinpoint_delivered_callback), + (pinpoint_shortcode_delivered_callback), + ], +) +def test_process_pinpoint_results_should_increment_sms_delivered_when_delivery_receipt_is_success( + sample_sms_template_with_html, + notify_api, + mocker, + callback, +): + mocker.patch("app.annual_limit_client.increment_sms_delivered") + mocker.patch("app.annual_limit_client.increment_sms_failed") + + notification = save_notification( + create_notification( + sample_sms_template_with_html, + reference="ref", + sent_at=datetime.utcnow(), + status=NOTIFICATION_SENT, + sent_by="pinpoint", ) - # TODO FF_ANNUAL_LIMIT removal - with set_config(notify_api, "FF_ANNUAL_LIMIT", True): - process_pinpoint_results(callback(reference="ref")) - annual_limit_client.increment_sms_delivered.assert_called_once_with(notification.service_id) - annual_limit_client.increment_sms_failed.assert_not_called() - - @pytest.mark.parametrize( - "callback, provider_response", - [ - (pinpoint_delivered_callback, None), - (pinpoint_failed_callback, "Blocked as spam by phone carrier"), - (pinpoint_failed_callback, "Phone carrier is currently unreachable/unavailable"), - (pinpoint_failed_callback, "Phone is currently unreachable/unavailable"), - (pinpoint_failed_callback, "This is not a real response"), - ], ) - def test_process_pinpoint_results_seeds_annual_limit_notifications_when_not_seeded_today_and_doesnt_increment_when_seeding( - self, - callback, - provider_response, - sample_sms_template_with_html, - notify_api, - mocker, - ): - mocker.patch("app.annual_limit_client.increment_sms_delivered") - mocker.patch("app.annual_limit_client.increment_sms_failed") - mocker.patch("app.annual_limit_client.was_seeded_today", return_value=False) - mocker.patch("app.annual_limit_client.set_seeded_at") - - notification = save_notification( - create_notification( - sample_sms_template_with_html, - reference="ref", - sent_at=datetime.utcnow(), - status=NOTIFICATION_SENT, - sent_by="pinpoint", - ) - ) - # TODO FF_ANNUAL_LIMIT removal - with set_config(notify_api, "FF_ANNUAL_LIMIT", True): - process_pinpoint_results( - callback(provider_response, reference="ref") if provider_response else callback(reference="ref") - ) - annual_limit_client.set_seeded_at.assert_called_once_with(notification.service_id) - annual_limit_client.increment_sms_delivered.assert_not_called() - annual_limit_client.increment_sms_failed.assert_not_called() + # TODO FF_ANNUAL_LIMIT removal + with set_config(notify_api, "FF_ANNUAL_LIMIT", True): + process_pinpoint_results(callback(reference="ref")) + annual_limit_client.increment_sms_delivered.assert_called_once_with(notification.service_id) + annual_limit_client.increment_sms_failed.assert_not_called() diff --git a/tests/app/celery/test_process_ses_receipts_tasks.py b/tests/app/celery/test_process_ses_receipts_tasks.py index 42e669282c..117a6a41e2 100644 --- a/tests/app/celery/test_process_ses_receipts_tasks.py +++ b/tests/app/celery/test_process_ses_receipts_tasks.py @@ -446,7 +446,6 @@ def test_ses_callback_should_increment_email_delivered_when_delivery_receipt_is_ ): mocker.patch("app.annual_limit_client.increment_email_delivered") mocker.patch("app.annual_limit_client.increment_email_failed") - mocker.patch("app.annual_limit_client.was_seeded_today", return_value=True) # TODO FF_ANNUAL_LIMIT removal with set_config(notify_api, "FF_ANNUAL_LIMIT", True): @@ -469,7 +468,6 @@ def test_ses_callback_should_increment_email_failed_when_delivery_receipt_is_fai ): mocker.patch("app.annual_limit_client.increment_email_failed") mocker.patch("app.annual_limit_client.increment_email_delivered") - mocker.patch("app.annual_limit_client.was_seeded_today", return_value=True) # TODO FF_ANNUAL_LIMIT removal with set_config(notify_api, "FF_ANNUAL_LIMIT", True): @@ -478,39 +476,3 @@ def test_ses_callback_should_increment_email_failed_when_delivery_receipt_is_fai assert process_ses_results(callback(reference="ref")) annual_limit_client.increment_email_failed.assert_called_once_with(sample_email_template.service_id) annual_limit_client.increment_email_delivered.assert_not_called() - - @pytest.mark.parametrize( - "callback", - [ - ses_notification_callback, - ses_hard_bounce_callback, - ses_soft_bounce_callback, - ], - ) - def test_process_ses_results_seeds_annual_limit_notifications_when_not_seeded_today_and_doesnt_increment_when_seeding( - self, - callback, - sample_email_template, - notify_api, - mocker, - ): - mocker.patch("app.annual_limit_client.increment_email_delivered") - mocker.patch("app.annual_limit_client.increment_email_failed") - mocker.patch("app.annual_limit_client.was_seeded_today", return_value=False) - mocker.patch("app.annual_limit_client.set_seeded_at") - - notification = save_notification( - create_notification( - sample_email_template, - reference="ref", - sent_at=datetime.utcnow(), - status="sending", - sent_by="ses", - ) - ) - # TODO FF_ANNUAL_LIMIT removal - with set_config(notify_api, "FF_ANNUAL_LIMIT", True): - process_ses_results(callback(reference="ref")) - annual_limit_client.set_seeded_at.assert_called_once_with(notification.service_id) - annual_limit_client.increment_email_delivered.assert_not_called() - annual_limit_client.increment_email_failed.assert_not_called() diff --git a/tests/app/celery/test_process_sns_receipts_tasks.py b/tests/app/celery/test_process_sns_receipts_tasks.py index 79cd534a3e..c443b5d0d3 100644 --- a/tests/app/celery/test_process_sns_receipts_tasks.py +++ b/tests/app/celery/test_process_sns_receipts_tasks.py @@ -189,111 +189,66 @@ def test_process_sns_results_calls_service_callback(sample_template, notify_db_s statsd_client.incr.assert_any_call("callback.sns.delivered") updated_notification = get_notification_by_id(notification.id) signed_data = create_delivery_status_callback_data(updated_notification, callback_api) - send_mock.assert_called_once_with( - [str(notification.id), signed_data, notification.service_id], - queue="service-callbacks", - ) + send_mock.assert_called_once_with([str(notification.id), signed_data, notification.service_id], queue="service-callbacks") + +def test_sns_callback_should_increment_sms_delivered_when_delivery_receipt_is_delivered( + sample_sms_template_with_html, notify_api, mocker +): + mocker.patch("app.annual_limit_client.increment_sms_delivered") + mocker.patch("app.annual_limit_client.increment_sms_failed") -class TestAnnualLimit: - def test_sns_callback_should_increment_sms_delivered_when_delivery_receipt_is_delivered( - self, sample_sms_template_with_html, notify_api, mocker - ): - mocker.patch("app.annual_limit_client.increment_sms_delivered") - mocker.patch("app.annual_limit_client.increment_sms_failed") - mocker.patch("app.annual_limit_client.was_seeded_today", return_value=True) - - notification = save_notification( - create_notification( - sample_sms_template_with_html, - reference="ref", - sent_at=datetime.utcnow(), - status=NOTIFICATION_SENT, - sent_by="sns", - ) + notification = save_notification( + create_notification( + sample_sms_template_with_html, + reference="ref", + sent_at=datetime.utcnow(), + status=NOTIFICATION_SENT, + sent_by="sns", ) - # TODO FF_ANNUAL_LIMIT removal - with set_config(notify_api, "FF_ANNUAL_LIMIT", True): - assert process_sns_results(sns_success_callback(reference="ref")) + ) + # TODO FF_ANNUAL_LIMIT removal + with set_config(notify_api, "FF_ANNUAL_LIMIT", True): + assert process_sns_results(sns_success_callback(reference="ref")) - annual_limit_client.increment_sms_delivered.assert_called_once_with(notification.service_id) - annual_limit_client.increment_sms_failed.assert_not_called() + annual_limit_client.increment_sms_delivered.assert_called_once_with(notification.service_id) + annual_limit_client.increment_sms_failed.assert_not_called() - @pytest.mark.parametrize( - "provider_response", - [ - "Blocked as spam by phone carrier", - "Destination is on a blocked list", - "Invalid phone number", - "Message body is invalid", - "Phone carrier has blocked this message", - "Phone carrier is currently unreachable/unavailable", - "Phone has blocked SMS", - "Phone is on a blocked list", - "Phone is currently unreachable/unavailable", - "Phone number is opted out", - "This delivery would exceed max price", - "Unknown error attempting to reach phone", - ], - ) - def test_sns_callback_should_increment_sms_failed_when_delivery_receipt_is_failure( - self, sample_sms_template_with_html, notify_api, mocker, provider_response - ): - mocker.patch("app.annual_limit_client.increment_sms_delivered") - mocker.patch("app.annual_limit_client.increment_sms_failed") - mocker.patch("app.annual_limit_client.was_seeded_today", return_value=True) - - notification = save_notification( - create_notification( - sample_sms_template_with_html, - reference="ref", - sent_at=datetime.utcnow(), - status=NOTIFICATION_SENT, - sent_by="sns", - ) - ) - # TODO FF_ANNUAL_LIMIT removal - with set_config(notify_api, "FF_ANNUAL_LIMIT", True): - assert process_sns_results(sns_failed_callback(reference="ref", provider_response=provider_response)) - annual_limit_client.increment_sms_failed.assert_called_once_with(notification.service_id) - annual_limit_client.increment_sms_delivered.assert_not_called() - - @pytest.mark.parametrize( - "callback, provider_response", - [ - (sns_success_callback, None), - (sns_failed_callback, "Blocked as spam by phone carrier"), - (sns_failed_callback, "Phone carrier is currently unreachable/unavailable"), - (sns_failed_callback, "Phone is currently unreachable/unavailable"), - (sns_failed_callback, "This is not a real response"), - ], - ) - def test_process_sns_results_seeds_annual_limit_notifications_when_not_seeded_today_and_doesnt_increment_when_seeding( - self, - callback, - provider_response, - sample_sms_template_with_html, - notify_api, - mocker, - ): - mocker.patch("app.annual_limit_client.increment_sms_delivered") - mocker.patch("app.annual_limit_client.increment_sms_failed") - mocker.patch("app.annual_limit_client.was_seeded_today", return_value=False) - mocker.patch("app.annual_limit_client.set_seeded_at") - - notification = save_notification( - create_notification( - sample_sms_template_with_html, - reference="ref", - sent_at=datetime.utcnow(), - status=NOTIFICATION_SENT, - sent_by="sns", - ) +@pytest.mark.parametrize( + "provider_response", + [ + "Blocked as spam by phone carrier", + "Destination is on a blocked list", + "Invalid phone number", + "Message body is invalid", + "Phone carrier has blocked this message", + "Phone carrier is currently unreachable/unavailable", + "Phone has blocked SMS", + "Phone is on a blocked list", + "Phone is currently unreachable/unavailable", + "Phone number is opted out", + "This delivery would exceed max price", + "Unknown error attempting to reach phone", + ], +) +def test_sns_callback_should_increment_sms_failed_when_delivery_receipt_is_failure( + sample_sms_template_with_html, notify_api, mocker, provider_response +): + mocker.patch("app.annual_limit_client.increment_sms_delivered") + mocker.patch("app.annual_limit_client.increment_sms_failed") + + notification = save_notification( + create_notification( + sample_sms_template_with_html, + reference="ref", + sent_at=datetime.utcnow(), + status=NOTIFICATION_SENT, + sent_by="sns", ) - # TODO FF_ANNUAL_LIMIT removal - with set_config(notify_api, "FF_ANNUAL_LIMIT", True): - process_sns_results(callback(provider_response, reference="ref") if provider_response else callback(reference="ref")) - annual_limit_client.set_seeded_at.assert_called_once_with(notification.service_id) - annual_limit_client.increment_sms_delivered.assert_not_called() - annual_limit_client.increment_sms_failed.assert_not_called() + ) + # TODO FF_ANNUAL_LIMIT removal + with set_config(notify_api, "FF_ANNUAL_LIMIT", True): + assert process_sns_results(sns_failed_callback(reference="ref", provider_response=provider_response)) + annual_limit_client.increment_sms_failed.assert_called_once_with(notification.service_id) + annual_limit_client.increment_sms_delivered.assert_not_called() diff --git a/tests/app/celery/test_reporting_tasks.py b/tests/app/celery/test_reporting_tasks.py index 6b987fcb9a..53ceb547ca 100644 --- a/tests/app/celery/test_reporting_tasks.py +++ b/tests/app/celery/test_reporting_tasks.py @@ -1,4 +1,3 @@ -import uuid from datetime import date, datetime, timedelta from decimal import Decimal @@ -31,10 +30,8 @@ create_rate, create_service, create_template, - create_user, save_notification, ) -from tests.conftest import set_config def mocker_get_rate( @@ -587,26 +584,6 @@ def test_create_nightly_notification_status_for_day_respects_local_timezone( assert noti_status[0].notification_status == "created" -@freeze_time("2019-04-01T5:30") -def test_create_nightly_notification_status_for_day_clears_failed_delivered_notification_counts( - sample_template, notify_api, mocker -): - mock_reset_counts = mocker.patch("app.annual_limit_client.reset_all_notification_counts") - for i in range(39): - user = create_user(email=f"test{i}@test.ca", mobile_number=f"{i}234567890") - service = create_service(service_id=uuid.uuid4(), service_name=f"service{i}", user=user, email_from=f"best.email{i}") - template_sms = create_template(service=service) - template_email = create_template(service=service, template_type="email") - save_notification(create_notification(template_sms, status="sent", created_at=datetime(2019, 4, 1, 5, 0))) - save_notification(create_notification(template_email, status="sent", created_at=datetime(2019, 4, 1, 5, 0))) - save_notification(create_notification(template_sms, status="failed", created_at=datetime(2019, 4, 1, 5, 0))) - save_notification(create_notification(template_email, status="delivered", created_at=datetime(2019, 4, 1, 5, 0))) - - with set_config(notify_api, "FF_ANNUAL_LIMIT", True): - create_nightly_notification_status_for_day("2019-04-01") - assert mock_reset_counts.call_count == 2 - - class TestInsertQuarterData: def test_insert_quarter_data(self, notify_db_session): service_1 = create_service(service_name="service_1") diff --git a/tests/app/service/test_statistics_rest.py b/tests/app/service/test_statistics_rest.py index a978979248..907d2fb5d2 100644 --- a/tests/app/service/test_statistics_rest.py +++ b/tests/app/service/test_statistics_rest.py @@ -20,7 +20,6 @@ create_template, save_notification, ) -from tests.conftest import set_config @freeze_time("2017-11-11 06:00") @@ -223,7 +222,7 @@ def test_get_monthly_notification_stats_returns_stats(admin_request, sample_serv @freeze_time("2016-06-05 00:00:00") # This test assumes the local timezone is EST -def test_get_monthly_notification_stats_combines_todays_data_and_historic_stats(admin_request, notify_api, sample_template): +def test_get_monthly_notification_stats_combines_todays_data_and_historic_stats(admin_request, sample_template): create_ft_notification_status(datetime(2016, 5, 1), template=sample_template, count=1) create_ft_notification_status( datetime(2016, 6, 1), @@ -238,27 +237,27 @@ def test_get_monthly_notification_stats_combines_todays_data_and_historic_stats( # this doesn't get returned in the stats because it is old - it should be in ft_notification_status by now save_notification(create_notification(sample_template, created_at=datetime(2016, 6, 4), status="sending")) - with set_config(notify_api, "FF_ANNUAL_LIMIT", True): - response = admin_request.get( - "service.get_monthly_notification_stats", - service_id=sample_template.service_id, - year=2016, - ) + response = admin_request.get( + "service.get_monthly_notification_stats", + service_id=sample_template.service_id, + year=2016, + ) - assert len(response["data"]) == 3 # apr, may, jun - assert response["data"]["2016-05"] == { - "sms": {"delivered": 1}, - "email": {}, - "letter": {}, - } - assert response["data"]["2016-06"] == { - "sms": { - # combines the stats from the historic ft_notification_status and the current notifications - "created": 2, - }, - "email": {}, - "letter": {}, - } + assert len(response["data"]) == 3 # apr, may, jun + assert response["data"]["2016-05"] == { + "sms": {"delivered": 1}, + "email": {}, + "letter": {}, + } + assert response["data"]["2016-06"] == { + "sms": { + # combines the stats from the historic ft_notification_status and the current notifications + "created": 3, + "delivered": 1, + }, + "email": {}, + "letter": {}, + } # This test assumes the local timezone is EST @@ -296,7 +295,7 @@ def test_get_monthly_notification_stats_checks_dates(admin_request, sample_servi assert response["data"]["2017-03"]["sms"] == {"delivered": 1} -def test_get_monthly_notification_stats_only_gets_for_one_service(admin_request, notify_api, notify_db_session): +def test_get_monthly_notification_stats_only_gets_for_one_service(admin_request, notify_db_session): services = [create_service(), create_service(service_name="2")] templates = [create_template(services[0]), create_template(services[1])] @@ -304,11 +303,10 @@ def test_get_monthly_notification_stats_only_gets_for_one_service(admin_request, create_ft_notification_status(datetime(2016, 6, 1), template=templates[0], notification_status="created") create_ft_notification_status(datetime(2016, 6, 1), template=templates[1], notification_status="delivered") - with set_config(notify_api, "FF_ANNUAL_LIMIT", True): - response = admin_request.get("service.get_monthly_notification_stats", service_id=services[0].id, year=2016) + response = admin_request.get("service.get_monthly_notification_stats", service_id=services[0].id, year=2016) - assert response["data"]["2016-06"] == { - "sms": {"created": 1}, - "email": {}, - "letter": {}, - } + assert response["data"]["2016-06"] == { + "sms": {"created": 1}, + "email": {}, + "letter": {}, + }