Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validate annual limits on message send #2375

Merged
merged 15 commits into from
Dec 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading