Skip to content

Commit

Permalink
Validate annual limits on message send (#2375)
Browse files Browse the repository at this point in the history
* Add backend validators for annual limits

* Fix CodeQL issues

* formatting

* Implement annual limit validation

* Undo accidental changes to devcontainer.json

* Revert accidental change to launch.json

* Ensure annual limits are checked before daily limits

* Add annual limit checks to one-off code path

- Added missing feature flag checks

* missing a singular placeholder

* Point utils to pre-3.12 branch

- Fix tests

---------

Co-authored-by: Jumana B <[email protected]>
Co-authored-by: Jumana Bahrainwala <[email protected]>
  • Loading branch information
3 people authored Dec 9, 2024
1 parent f21f01f commit c566564
Show file tree
Hide file tree
Showing 11 changed files with 409 additions and 57 deletions.
4 changes: 4 additions & 0 deletions app/job/rest.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,9 @@
)
from app.notifications.process_notifications import simulated_recipient
from app.notifications.validators import (
check_email_annual_limit,
check_email_daily_limit,
check_sms_annual_limit,
check_sms_daily_limit,
increment_email_daily_count_send_warnings_if_needed,
increment_sms_daily_count_send_warnings_if_needed,
Expand Down Expand Up @@ -183,6 +185,7 @@ def create_job(service_id):
is_test_notification = len(recipient_csv) == numberOfSimulated

if not is_test_notification:
check_sms_annual_limit(service, len(recipient_csv))
check_sms_daily_limit(service, len(recipient_csv))
increment_sms_daily_count_send_warnings_if_needed(service, len(recipient_csv))

Expand All @@ -195,6 +198,7 @@ def create_job(service_id):
)
notification_count = len(recipient_csv)

check_email_annual_limit(service, notification_count)
check_email_daily_limit(service, notification_count)

scheduled_for = datetime.fromisoformat(data.get("scheduled_for")) if data.get("scheduled_for") else None
Expand Down
5 changes: 5 additions & 0 deletions app/notifications/rest.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,10 @@
simulated_recipient,
)
from app.notifications.validators import (
check_email_annual_limit,
check_email_daily_limit,
check_rate_limiting,
check_sms_annual_limit,
check_template_is_active,
check_template_is_for_notification_type,
service_has_permission,
Expand Down Expand Up @@ -113,6 +115,7 @@ def send_notification(notification_type: NotificationType):

simulated = simulated_recipient(notification_form["to"], notification_type)
if not simulated != api_user.key_type == KEY_TYPE_TEST and notification_type == EMAIL_TYPE:
check_email_annual_limit(authenticated_service, 1)
check_email_daily_limit(authenticated_service, 1)

check_template_is_for_notification_type(notification_type, template.template_type)
Expand All @@ -129,6 +132,8 @@ def send_notification(notification_type: NotificationType):

if notification_type == SMS_TYPE:
_service_can_send_internationally(authenticated_service, notification_form["to"])
if not simulated and api_user.key_type != KEY_TYPE_TEST:
check_sms_annual_limit(authenticated_service, 1)
# Do not persist or send notification to the queue if it is a simulated recipient

notification_model = persist_notification(
Expand Down
3 changes: 2 additions & 1 deletion app/notifications/validators.py
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ def check_email_annual_limit(service: Service, requested_emails=0):
return

current_app.logger.info(
f"Service {service.id} is exceeding their annual email limit [total sent this fiscal: {int(emails_sent_today + emails_sent_this_fiscal)} limit: {service.email_annual_limit}, attempted send: {requested_emails}"
f"{'Trial service' if service.restricted else 'Service'} {service.id} is exceeding their annual email limit [total sent this fiscal: {int(emails_sent_today + emails_sent_this_fiscal)} limit: {service.email_annual_limit}, attempted send: {requested_emails}"
)
if service.restricted:
raise TrialServiceRequestExceedsEmailAnnualLimitError(service.email_annual_limit)
Expand Down Expand Up @@ -549,6 +549,7 @@ def send_annual_limit_updated_email(service: Service, notification_type: Notific
service_id=service.id,
template_id=current_app.config["ANNUAL_LIMIT_UPDATED_TEMPLATE_ID"],
personalisation={
"service_name": service.name,
"message_type_en": notification_type,
"message_type_fr": "Courriel" if notification_type == EMAIL_TYPE else "SMS",
"message_limit_en": service.email_annual_limit if notification_type == EMAIL_TYPE else service.sms_annual_limit,
Expand Down
4 changes: 4 additions & 0 deletions app/service/send_notification.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,11 @@
simulated_recipient,
)
from app.notifications.validators import (
check_email_annual_limit,
check_email_daily_limit,
check_service_has_permission,
check_service_over_daily_message_limit,
check_sms_annual_limit,
check_sms_daily_limit,
increment_email_daily_count_send_warnings_if_needed,
increment_sms_daily_count_send_warnings_if_needed,
Expand Down Expand Up @@ -70,8 +72,10 @@ def send_one_off_notification(service_id, post_data):
if template.template_type == SMS_TYPE:
is_test_notification = simulated_recipient(post_data["to"], template.template_type)
if not is_test_notification:
check_sms_annual_limit(service, 1)
check_sms_daily_limit(service, 1)
elif template.template_type == EMAIL_TYPE:
check_email_annual_limit(service, 1)
check_email_daily_limit(service, 1) # 1 email

validate_and_format_recipient(
Expand Down
86 changes: 66 additions & 20 deletions app/v2/notifications/post_notifications.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@
Notification,
NotificationType,
Service,
TemplateType,
)
from app.notifications.process_letter_notifications import create_letter_notification
from app.notifications.process_notifications import (
Expand All @@ -74,12 +73,14 @@
transform_notification,
)
from app.notifications.validators import (
check_email_annual_limit,
check_email_daily_limit,
check_rate_limiting,
check_service_can_schedule_notification,
check_service_email_reply_to_id,
check_service_has_permission,
check_service_sms_sender_id,
check_sms_annual_limit,
check_sms_daily_limit,
increment_email_daily_count_send_warnings_if_needed,
increment_sms_daily_count_send_warnings_if_needed,
Expand Down Expand Up @@ -183,10 +184,14 @@ def post_bulk():

if template.template_type == SMS_TYPE:
fragments_sent = fetch_todays_requested_sms_count(authenticated_service.id)
remaining_messages = authenticated_service.sms_daily_limit - fragments_sent
remaining_daily_messages = authenticated_service.sms_daily_limit - fragments_sent
remaining_annual_messages = authenticated_service.sms_annual_limit - fragments_sent

else:
current_app.logger.info(f"[post_notifications.post_bulk()] Checking bounce rate for service: {authenticated_service.id}")
remaining_messages = authenticated_service.message_limit - fetch_todays_total_message_count(authenticated_service.id)
emails_sent = fetch_todays_total_message_count(authenticated_service.id)
remaining_daily_messages = authenticated_service.message_limit - emails_sent
remaining_annual_messages = authenticated_service.email_annual_limit - emails_sent

form["validated_sender_id"] = validate_sender_id(template, form.get("reply_to_id"))

Expand All @@ -199,19 +204,33 @@ def post_bulk():
else:
file_data = form["csv"]

recipient_csv = RecipientCSV(
file_data,
template_type=template.template_type,
placeholders=template._as_utils_template().placeholders,
max_rows=max_rows,
safelist=safelisted_members(authenticated_service, api_user.key_type),
remaining_messages=remaining_messages,
template=Template(template.__dict__),
)
if current_app.config["FF_ANNUAL_LIMIT"]:
recipient_csv = RecipientCSV(
file_data,
template_type=template.template_type,
placeholders=template._as_utils_template().placeholders,
max_rows=max_rows,
safelist=safelisted_members(authenticated_service, api_user.key_type),
remaining_messages=remaining_daily_messages,
remaining_daily_messages=remaining_daily_messages,
remaining_annual_messages=remaining_annual_messages,
template=Template(template.__dict__),
)
else: # TODO FF_ANNUAL_LIMIT REMOVAL - Remove this block
recipient_csv = RecipientCSV(
file_data,
template_type=template.template_type,
placeholders=template._as_utils_template().placeholders,
max_rows=max_rows,
safelist=safelisted_members(authenticated_service, api_user.key_type),
remaining_messages=remaining_daily_messages,
template=Template(template.__dict__),
)
except csv.Error as e:
raise BadRequestError(message=f"Error converting to CSV: {str(e)}", status_code=400)

check_for_csv_errors(recipient_csv, max_rows, remaining_messages)
check_for_csv_errors(recipient_csv, max_rows, remaining_daily_messages, remaining_annual_messages)
notification_count_requested = len(list(recipient_csv.get_rows()))

for row in recipient_csv.get_rows():
try:
Expand All @@ -223,11 +242,12 @@ def post_bulk():
raise BadRequestError(message=message)

if template.template_type == EMAIL_TYPE and api_user.key_type != KEY_TYPE_TEST:
check_email_daily_limit(authenticated_service, len(list(recipient_csv.get_rows())))
check_email_annual_limit(authenticated_service, notification_count_requested)
check_email_daily_limit(authenticated_service, notification_count_requested)
scheduled_for = datetime.fromisoformat(form.get("scheduled_for")) if form.get("scheduled_for") else None

if scheduled_for is None or not scheduled_for.date() > datetime.today().date():
increment_email_daily_count_send_warnings_if_needed(authenticated_service, len(list(recipient_csv.get_rows())))
increment_email_daily_count_send_warnings_if_needed(authenticated_service, notification_count_requested)

if template.template_type == SMS_TYPE:
# set sender_id if missing
Expand All @@ -240,15 +260,16 @@ def post_bulk():
numberOfSimulated = sum(
simulated_recipient(i["phone_number"].data, template.template_type) for i in list(recipient_csv.get_rows())
)
mixedRecipients = numberOfSimulated > 0 and numberOfSimulated != len(list(recipient_csv.get_rows()))
mixedRecipients = numberOfSimulated > 0 and numberOfSimulated != notification_count_requested

# if its a live or a team key, and they have specified testing and NON-testing recipients, raise an error
if api_user.key_type != KEY_TYPE_TEST and mixedRecipients:
raise BadRequestError(message="Bulk sending to testing and non-testing numbers is not supported", status_code=400)

is_test_notification = api_user.key_type == KEY_TYPE_TEST or len(list(recipient_csv.get_rows())) == numberOfSimulated
is_test_notification = api_user.key_type == KEY_TYPE_TEST or notification_count_requested == numberOfSimulated

if not is_test_notification:
check_sms_annual_limit(authenticated_service, len(recipient_csv))
check_sms_daily_limit(authenticated_service, len(recipient_csv))
increment_sms_daily_count_send_warnings_if_needed(authenticated_service, len(recipient_csv))

Expand Down Expand Up @@ -302,11 +323,13 @@ def post_notification(notification_type: NotificationType):
)

if template.template_type == EMAIL_TYPE and api_user.key_type != KEY_TYPE_TEST:
check_email_annual_limit(authenticated_service, 1)
check_email_daily_limit(authenticated_service, 1) # 1 email

if template.template_type == SMS_TYPE:
is_test_notification = api_user.key_type == KEY_TYPE_TEST or simulated_recipient(form["phone_number"], notification_type)
if not is_test_notification:
check_sms_annual_limit(authenticated_service, 1)
check_sms_daily_limit(authenticated_service, 1)

current_app.logger.info(f"Trying to send notification for Template ID: {template.id}")
Expand Down Expand Up @@ -664,7 +687,7 @@ def strip_keys_from_personalisation_if_send_attach(personalisation):
return {k: v for (k, v) in personalisation.items() if not (type(v) is dict and v.get("sending_method") == "attach")}


def check_for_csv_errors(recipient_csv, max_rows, remaining_messages):
def check_for_csv_errors(recipient_csv, max_rows, remaining_daily_messages, remaining_annual_messages):
nb_rows = len(recipient_csv)

if recipient_csv.has_errors:
Expand All @@ -678,15 +701,38 @@ def check_for_csv_errors(recipient_csv, max_rows, remaining_messages):
message=f"Duplicate column headers: {', '.join(sorted(recipient_csv.duplicate_recipient_column_headers))}",
status_code=400,
)
## TODO: FF_ANNUAL_LIMIT - remove this if block in favour of more_rows_than_can_send_today found below
if recipient_csv.more_rows_than_can_send:
if recipient_csv.template_type == SMS_TYPE:
raise BadRequestError(
message=f"You only have {remaining_messages} remaining sms messages before you reach your daily limit. You've tried to send {len(recipient_csv)} sms messages.",
message=f"You only have {remaining_daily_messages} remaining sms messages before you reach your daily limit. You've tried to send {len(recipient_csv)} sms messages.",
status_code=400,
)
else:
raise BadRequestError(
message=f"You only have {remaining_daily_messages} remaining messages before you reach your daily limit. You've tried to send {nb_rows} messages.",
status_code=400,
)
if recipient_csv.more_rows_than_can_send_this_year:
if recipient_csv.template_type == SMS_TYPE:
raise BadRequestError(
message=f"You only have {remaining_annual_messages} remaining sms messages before you reach your annual limit. You've tried to send {len(recipient_csv)} sms messages.",
status_code=400,
)
else:
raise BadRequestError(
message=f"You only have {remaining_annual_messages} remaining messages before you reach your annual limit. You've tried to send {nb_rows} messages.",
status_code=400,
)
if recipient_csv.more_rows_than_can_send_today:
if recipient_csv.template_type == SMS_TYPE:
raise BadRequestError(
message=f"You only have {remaining_daily_messages} remaining sms messages before you reach your daily limit. You've tried to send {len(recipient_csv)} sms messages.",
status_code=400,
)
else:
raise BadRequestError(
message=f"You only have {remaining_messages} remaining messages before you reach your daily limit. You've tried to send {nb_rows} messages.",
message=f"You only have {remaining_daily_messages} remaining messages before you reach your daily limit. You've tried to send {nb_rows} messages.",
status_code=400,
)

Expand Down
8 changes: 4 additions & 4 deletions poetry.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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.4.0" }
notifications-utils = { git = "https://github.com/cds-snc/notifier-utils.git", branch = "task/pre-3.12-recipient-csv-annual-limit-validation"}

# rsa = "4.9 # awscli 1.22.38 depends on rsa<4.8
typing-extensions = "4.12.2"
Expand Down
6 changes: 4 additions & 2 deletions tests/app/db.py
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,8 @@ def create_service(
prefix_sms=True,
message_limit=1000,
sms_daily_limit=1000,
annual_email_limit=10000000,
annual_sms_limit=25000,
email_annual_limit=10000000,
sms_annual_limit=25000,
organisation_type="central",
check_if_service_exists=False,
go_live_user=None,
Expand All @@ -136,6 +136,8 @@ def create_service(
organisation_type=organisation_type,
go_live_user=go_live_user,
go_live_at=go_live_at,
email_annual_limit=email_annual_limit,
sms_annual_limit=sms_annual_limit,
crown=crown,
sensitive_service=sensitive_service,
)
Expand Down
Loading

0 comments on commit c566564

Please sign in to comment.