From f53ac4f15f934c7df16ac69871b2976dfc2469bf Mon Sep 17 00:00:00 2001 From: wbanks Date: Wed, 31 Jan 2024 14:39:33 -0400 Subject: [PATCH] Remove FF_EMAIL_DAILY_LIMIT feature flag --- app/celery/tasks.py | 5 ----- app/config.py | 4 ---- app/notifications/validators.py | 5 ----- app/service/rest.py | 4 +--- app/service/send_notification.py | 8 ++------ tests/app/celery/test_tasks.py | 15 +++++---------- tests/app/notifications/test_validators.py | 10 ++++------ tests/app/service/test_rest.py | 4 +--- .../app/service/test_send_one_off_notification.py | 6 ++---- 9 files changed, 15 insertions(+), 46 deletions(-) diff --git a/app/celery/tasks.py b/app/celery/tasks.py index 9dfc90ce54..df10c1db29 100644 --- a/app/celery/tasks.py +++ b/app/celery/tasks.py @@ -78,7 +78,6 @@ persist_notifications, send_notification_to_queue, ) -from app.notifications.validators import check_service_over_daily_message_limit from app.types import VerifiedNotification from app.utils import get_csv_max_rows, get_delivery_queue_for_template from app.v2.errors import ( @@ -300,8 +299,6 @@ def save_smss(self, service_id: Optional[str], signed_notifications: List[Signed current_app.logger.debug(f"Sending following sms notifications to AWS: {notification_id_queue.keys()}") for notification_obj in saved_notifications: try: - if not current_app.config["FF_EMAIL_DAILY_LIMIT"]: - check_service_over_daily_message_limit(notification_obj.key_type, service) queue = notification_id_queue.get(notification_obj.id) or get_delivery_queue_for_template(template) send_notification_to_queue( notification_obj, @@ -423,8 +420,6 @@ def try_to_send_notifications_to_queue(notification_id_queue, service, saved_not research_mode = service.research_mode # type: ignore for notification_obj in saved_notifications: try: - if not current_app.config["FF_EMAIL_DAILY_LIMIT"]: - check_service_over_daily_message_limit(notification_obj.key_type, service) queue = notification_id_queue.get(notification_obj.id) or get_delivery_queue_for_template(template) send_notification_to_queue( notification_obj, diff --git a/app/config.py b/app/config.py index 3dbecb98bc..d08a88b854 100644 --- a/app/config.py +++ b/app/config.py @@ -562,8 +562,6 @@ class Config(object): # Feature flag to enable custom retry policies such as lowering retry period for certain priority lanes. FF_CELERY_CUSTOM_TASK_PARAMS = env.bool("FF_CELERY_CUSTOM_TASK_PARAMS", True) FF_CLOUDWATCH_METRICS_ENABLED = env.bool("FF_CLOUDWATCH_METRICS_ENABLED", False) - # Feature flags for email_daily_limit - FF_EMAIL_DAILY_LIMIT = env.bool("FF_EMAIL_DAILY_LIMIT", False) FF_SALESFORCE_CONTACT = env.bool("FF_SALESFORCE_CONTACT", False) # SRE Tools auth keys @@ -671,8 +669,6 @@ class Test(Development): CRM_ORG_LIST_URL = "https://test-url.com" FAILED_LOGIN_LIMIT = 0 - FF_EMAIL_DAILY_LIMIT = False - class Production(Config): NOTIFY_EMAIL_DOMAIN = os.getenv("NOTIFY_EMAIL_DOMAIN", "notification.canada.ca") diff --git a/app/notifications/validators.py b/app/notifications/validators.py index 3aee7486d6..a144c22229 100644 --- a/app/notifications/validators.py +++ b/app/notifications/validators.py @@ -227,17 +227,12 @@ def increment_sms_daily_count_send_warnings_if_needed(service: Service, requeste def increment_email_daily_count_send_warnings_if_needed(service: Service, requested_email=0) -> None: - if not current_app.config["FF_EMAIL_DAILY_LIMIT"]: - return - increment_todays_email_count(service.id, requested_email) send_warning_email_limit_emails_if_needed(service) def check_rate_limiting(service: Service, api_key: ApiKey): check_service_over_api_rate_limit_and_update_rate(service, api_key) - if not current_app.config["FF_EMAIL_DAILY_LIMIT"]: - check_service_over_daily_message_limit(api_key.key_type, service) def warn_about_daily_message_limit(service: Service, messages_sent): diff --git a/app/service/rest.py b/app/service/rest.py index d3c790d841..8c198b0151 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -342,9 +342,7 @@ def update_service(service_id): def _warn_service_users_about_message_limit_changed(service_id, data): send_notification_to_service_users( service_id=service_id, - template_id=current_app.config["DAILY_EMAIL_LIMIT_UPDATED_TEMPLATE_ID"] - if current_app.config["FF_EMAIL_DAILY_LIMIT"] - else current_app.config["DAILY_LIMIT_UPDATED_TEMPLATE_ID"], + template_id=current_app.config["DAILY_EMAIL_LIMIT_UPDATED_TEMPLATE_ID"], personalisation={ "service_name": data["name"], "message_limit_en": "{:,}".format(data["message_limit"]), diff --git a/app/service/send_notification.py b/app/service/send_notification.py index 813ce2f138..77cd1313e3 100644 --- a/app/service/send_notification.py +++ b/app/service/send_notification.py @@ -67,13 +67,11 @@ def send_one_off_notification(service_id, post_data): _, template_with_content = validate_template(template.id, personalisation, service, template.template_type) - if not current_app.config["FF_EMAIL_DAILY_LIMIT"]: - check_service_over_daily_message_limit(KEY_TYPE_NORMAL, service) if template.template_type == SMS_TYPE: is_test_notification = simulated_recipient(post_data["to"], template.template_type) if not is_test_notification: check_sms_daily_limit(service, 1) - elif template.template_type == EMAIL_TYPE and current_app.config["FF_EMAIL_DAILY_LIMIT"]: + elif template.template_type == EMAIL_TYPE: check_email_daily_limit(service, 1) # 1 email validate_and_format_recipient( @@ -86,13 +84,11 @@ def send_one_off_notification(service_id, post_data): validate_created_by(service, post_data["created_by"]) - if not current_app.config["FF_EMAIL_DAILY_LIMIT"]: - pass # will remove this soon, don't bother refactoring if template.template_type == SMS_TYPE: is_test_notification = simulated_recipient(post_data["to"], template.template_type) if not is_test_notification: increment_sms_daily_count_send_warnings_if_needed(service, 1) - elif template.template_type == EMAIL_TYPE and current_app.config["FF_EMAIL_DAILY_LIMIT"]: + elif template.template_type == EMAIL_TYPE: increment_email_daily_count_send_warnings_if_needed(service, 1) # 1 email sender_id = post_data.get("sender_id", None) diff --git a/tests/app/celery/test_tasks.py b/tests/app/celery/test_tasks.py index 1b4f5b4e31..437d59e7ec 100644 --- a/tests/app/celery/test_tasks.py +++ b/tests/app/celery/test_tasks.py @@ -1328,12 +1328,11 @@ def test_save_sms_should_save_default_smm_sender_notification_reply_to_text_on(s def test_should_save_sms_template_to_and_persist_with_job_id(self, notify_api, sample_job, mocker): notification = _notification_json(sample_job.template, to="+1 650 253 2222", job_id=sample_job.id, row_number=2) mocker.patch("app.celery.provider_tasks.deliver_sms.apply_async") - mock_over_daily_limit = mocker.patch("app.celery.tasks.check_service_over_daily_message_limit") notification_id = uuid.uuid4() now = datetime.utcnow() - with set_config_values(notify_api, {"FF_EMAIL_DAILY_LIMIT": False}): - save_smss(sample_job.template.service_id, [signer_notification.sign(notification)], notification_id) + + save_smss(sample_job.template.service_id, [signer_notification.sign(notification)], notification_id) persisted_notification = Notification.query.one() assert persisted_notification.to == "+1 650 253 2222" assert persisted_notification.job_id == sample_job.id @@ -1350,7 +1349,6 @@ def test_should_save_sms_template_to_and_persist_with_job_id(self, notify_api, s provider_tasks.deliver_sms.apply_async.assert_called_once_with( [str(persisted_notification.id)], queue=QueueNames.SEND_SMS_MEDIUM ) - mock_over_daily_limit.assert_called_once_with("normal", sample_job.service) def test_save_sms_should_go_to_retry_queue_if_database_errors(self, sample_template, mocker): notification = _notification_json(sample_template, "+1 650 253 2222") @@ -1672,7 +1670,6 @@ def test_should_use_email_template_and_persist( self, notify_api, sample_email_template_with_placeholders, sample_api_key, mocker ): mocker.patch("app.celery.provider_tasks.deliver_email.apply_async") - mock_over_daily_limit = mocker.patch("app.celery.tasks.check_service_over_daily_message_limit") now = datetime(2016, 1, 1, 11, 9, 0) notification_id = uuid.uuid4() @@ -1686,10 +1683,9 @@ def test_should_use_email_template_and_persist( ) with freeze_time("2016-01-01 11:10:00.00000"): - with set_config_values(notify_api, {"FF_EMAIL_DAILY_LIMIT": False}): - save_emails( - sample_email_template_with_placeholders.service_id, [signer_notification.sign(notification)], notification_id - ) + save_emails( + sample_email_template_with_placeholders.service_id, [signer_notification.sign(notification)], notification_id + ) persisted_notification = Notification.query.one() assert persisted_notification.to == "my_email@my_email.com" @@ -1709,7 +1705,6 @@ def test_should_use_email_template_and_persist( provider_tasks.deliver_email.apply_async.assert_called_once_with( [str(persisted_notification.id)], queue=QueueNames.SEND_EMAIL_MEDIUM ) - mock_over_daily_limit.assert_called_once_with("normal", sample_email_template_with_placeholders.service) def test_save_email_should_use_template_version_from_job_not_latest(self, sample_email_template, mocker): notification = _notification_json(sample_email_template, "my_email@my_email.com") diff --git a/tests/app/notifications/test_validators.py b/tests/app/notifications/test_validators.py index 24175b431e..f49c4dc6e0 100644 --- a/tests/app/notifications/test_validators.py +++ b/tests/app/notifications/test_validators.py @@ -217,8 +217,7 @@ def test_check_service_message_limit_records_nearing_daily_limit( if limit_type == "sms": increment_sms_daily_count_send_warnings_if_needed(service) else: - with set_config(notify_api, "FF_EMAIL_DAILY_LIMIT", True): - increment_email_daily_count_send_warnings_if_needed(service) + increment_email_daily_count_send_warnings_if_needed(service) assert redis_get.call_args_list == [ call(count_key(limit_type, service.id)), @@ -290,8 +289,7 @@ def test_check_service_message_limit_in_cache_over_message_limit_fails( assert e.value.fields == [] with pytest.raises(TooManyEmailRequestsError) as e: - with set_config(notify_api, "FF_EMAIL_DAILY_LIMIT", True): - check_email_daily_limit(service) + check_email_daily_limit(service) assert e.value.status_code == 429 assert e.value.message == "Exceeded email daily sending limit of 4 messages" assert e.value.fields == [] @@ -350,8 +348,8 @@ def test_check_service_message_limit_skip_statsd_over_message_no_limit_fails_ema # When service = create_sample_service(notify_db, notify_db_session, restricted=True, limit=4, sms_limit=4) - with set_config(notify_api, "FF_EMAIL_DAILY_LIMIT", True): - check_email_daily_limit(service) + check_email_daily_limit(service) + # Then app_statsd.statsd_client.incr.assert_not_called() diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index d5f01b415d..e3e309cda5 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -2225,9 +2225,7 @@ def test_update_service_updating_daily_limit_sends_notification_to_users( if expected_call: send_notification_mock.assert_called_once_with( service_id=service.id, - template_id=current_app.config["DAILY_EMAIL_LIMIT_UPDATED_TEMPLATE_ID"] - if current_app.config["FF_EMAIL_DAILY_LIMIT"] - else current_app.config["DAILY_LIMIT_UPDATED_TEMPLATE_ID"], + template_id=current_app.config["DAILY_EMAIL_LIMIT_UPDATED_TEMPLATE_ID"], personalisation={ "service_name": service.name, "message_limit_en": "{:,}".format(new_limit), diff --git a/tests/app/service/test_send_one_off_notification.py b/tests/app/service/test_send_one_off_notification.py index 4b8de0890c..a0e120f1e4 100644 --- a/tests/app/service/test_send_one_off_notification.py +++ b/tests/app/service/test_send_one_off_notification.py @@ -31,7 +31,6 @@ create_template, create_user, ) -from tests.conftest import set_config_values @pytest.fixture @@ -314,9 +313,8 @@ def test_send_one_off_notification_raises_if_over_email_limit(notify_db_session, "created_by": str(service.created_by_id), } - with set_config_values(notify_api, {"FF_EMAIL_DAILY_LIMIT": True}): - with pytest.raises(LiveServiceTooManyEmailRequestsError): - send_one_off_notification(service.id, post_data) + with pytest.raises(LiveServiceTooManyEmailRequestsError): + send_one_off_notification(service.id, post_data) def test_send_one_off_notification_raises_if_over_sms_daily_limit(notify_db_session, mocker):