Skip to content

Commit

Permalink
Remove FF_EMAIL_DAILY_LIMIT feature flag
Browse files Browse the repository at this point in the history
  • Loading branch information
whabanks committed Jan 31, 2024
1 parent 3d27ba5 commit f53ac4f
Show file tree
Hide file tree
Showing 9 changed files with 15 additions and 46 deletions.
5 changes: 0 additions & 5 deletions app/celery/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
4 changes: 0 additions & 4 deletions app/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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")
Expand Down
5 changes: 0 additions & 5 deletions app/notifications/validators.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
4 changes: 1 addition & 3 deletions app/service/rest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"]),
Expand Down
8 changes: 2 additions & 6 deletions app/service/send_notification.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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)
Expand Down
15 changes: 5 additions & 10 deletions tests/app/celery/test_tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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")
Expand Down Expand Up @@ -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()
Expand All @@ -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"
Expand All @@ -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")
Expand Down
10 changes: 4 additions & 6 deletions tests/app/notifications/test_validators.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)),
Expand Down Expand Up @@ -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 == []
Expand Down Expand Up @@ -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()

Expand Down
4 changes: 1 addition & 3 deletions tests/app/service/test_rest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
6 changes: 2 additions & 4 deletions tests/app/service/test_send_one_off_notification.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
create_template,
create_user,
)
from tests.conftest import set_config_values


@pytest.fixture
Expand Down Expand Up @@ -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):
Expand Down

0 comments on commit f53ac4f

Please sign in to comment.