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

Fix post_bulk annual limit validation #2384

Merged
merged 4 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
38 changes: 29 additions & 9 deletions app/celery/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,9 @@
from app.dao.service_email_reply_to_dao import dao_get_reply_to_by_id
from app.dao.service_inbound_api_dao import get_service_inbound_api_for_service
from app.dao.service_sms_sender_dao import dao_get_service_sms_senders_by_id
from app.dao.services_dao import (
dao_fetch_service_by_id,
fetch_todays_total_message_count,
)
from app.dao.services_dao import dao_fetch_service_by_id
from app.dao.templates_dao import dao_get_template_by_id
from app.email_limit_utils import fetch_todays_email_count
from app.encryption import SignedNotification
from app.exceptions import DVLAException
from app.models import (
Expand All @@ -81,6 +79,7 @@
persist_notifications,
send_notification_to_queue,
)
from app.sms_fragment_utils import fetch_todays_requested_sms_count
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 @@ -205,15 +204,36 @@ def process_rows(rows: List, template: Template, job: Job, service: Service):


def __sending_limits_for_job_exceeded(service, job: Job, job_id):
total_sent = fetch_todays_total_message_count(service.id)
error_message = None

if job.template.template_type == SMS_TYPE:
total_post_send = fetch_todays_requested_sms_count(service.id) + job.notification_count

if total_post_send > service.sms_annual_limit:
error_message = (
f"Job {job_id} size {job.notification_count} error. SMS annual limit {service.sms_annual_limit} exceeded."
)
elif total_post_send > service.sms_daily_limit:
error_message = (
f"Job {job_id} size {job.notification_count} error. SMS daily limit {service.sms_daily_limit} exceeded."
)
else:
total_post_send = fetch_todays_email_count(service.id) + job.notification_count

if total_sent + job.notification_count > service.message_limit:
if total_post_send > service.email_annual_limit:
error_message = (
f"Job {job_id} size {job.notification_count} error. Email annual limit {service.sms_annual_limit} exceeded."
)
elif total_post_send > service.message_limit:
error_message = (
f"Job {job_id} size {job.notification_count} error. SMS daily limit {service.sms_daily_limit} exceeded."
)

if error_message:
job.job_status = JOB_STATUS_SENDING_LIMITS_EXCEEDED
job.processing_finished = datetime.utcnow()
dao_update_job(job)
current_app.logger.info(
"Job {} size {} error. Sending limits {} exceeded".format(job_id, job.notification_count, service.message_limit)
)
current_app.logger.info(error_message)
return True
return False

Expand Down
4 changes: 2 additions & 2 deletions app/v2/notifications/post_notifications.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@
from app.config import QueueNames, TaskNames
from app.dao.jobs_dao import dao_create_job
from app.dao.notifications_dao import update_notification_status_by_reference
from app.dao.services_dao import fetch_todays_total_message_count
from app.dao.templates_dao import get_precompiled_letter_template
from app.email_limit_utils import fetch_todays_email_count
from app.encryption import NotificationDictToSign
from app.letters.utils import upload_letter_pdf
from app.models import (
Expand Down Expand Up @@ -189,7 +189,7 @@ def post_bulk():

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

Expand Down
63 changes: 60 additions & 3 deletions tests/app/celery/test_tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -547,7 +547,7 @@ def test_should_process_sms_job_with_sender_id(self, sample_template, mocker, fa

@freeze_time("2016-01-01 11:09:00.061258")
def test_should_not_process_sms_job_if_would_exceed_send_limits(self, notify_db_session, mocker):
service = create_service(message_limit=9)
service = create_service(sms_daily_limit=9)
template = create_template(service=service)
job = create_job(template=template, notification_count=10, original_file_name="multiple_sms.csv")
mocker.patch(
Expand All @@ -564,7 +564,7 @@ def test_should_not_process_sms_job_if_would_exceed_send_limits(self, notify_db_
assert tasks.process_rows.called is False

def test_should_not_process_sms_job_if_would_exceed_send_limits_inc_today(self, notify_db_session, mocker):
service = create_service(message_limit=1)
service = create_service(sms_daily_limit=1)
template = create_template(service=service)
job = create_job(template=template)

Expand All @@ -580,9 +580,27 @@ def test_should_not_process_sms_job_if_would_exceed_send_limits_inc_today(self,
assert s3.get_job_from_s3.called is False
assert tasks.process_rows.called is False

@pytest.mark.parametrize("template_type", ["sms", "email"])
def test_should_not_process_job_if_would_exceed_annual_limit(self, notify_db_session, template_type, mocker):
service = create_service(email_annual_limit=1, sms_annual_limit=1)
template = create_template(service=service, template_type=template_type)
job = create_job(template=template)

save_notification(create_notification(template=template, job=job))

mocker.patch("app.celery.tasks.s3.get_job_from_s3")
mocker.patch("app.celery.tasks.process_rows")

process_job(job.id)

job = jobs_dao.dao_get_job_by_id(job.id)
assert job.job_status == "sending limits exceeded"
assert s3.get_job_from_s3.called is False
assert tasks.process_rows.called is False

@pytest.mark.parametrize("template_type", ["sms", "email"])
def test_should_not_process_email_job_if_would_exceed_send_limits_inc_today(self, notify_db_session, template_type, mocker):
service = create_service(message_limit=1)
service = create_service(message_limit=1, sms_daily_limit=1)
template = create_template(service=service, template_type=template_type)
job = create_job(template=template)

Expand Down Expand Up @@ -647,6 +665,45 @@ def test_should_process_email_job_if_exactly_on_send_limits(self, notify_db_sess
queue="-normal-database-tasks",
)

@pytest.mark.parametrize("template_type, save_task", [("sms", save_smss), ("email", save_emails)])
def test_should_process_job_if_exactly_on_send_limits(self, notify_db_session, template_type, save_task, mocker):
service = create_service(message_limit=10)
template = create_template(service=service, template_type=template_type)
job = create_job(template=template, notification_count=10)

mocker.patch(
"app.celery.tasks.s3.get_job_from_s3",
return_value=load_example_csv("multiple_email"),
)
save_task_mock = mocker.patch(f"app.celery.tasks.{save_task.__name__}.apply_async")
mocker.patch("app.signer_notification.sign", return_value="something_encrypted")
mocker.patch("app.celery.tasks.create_uuid", return_value="uuid")

process_job(job.id)

s3.get_job_from_s3.assert_called_once_with(str(job.service.id), str(job.id))
job = jobs_dao.dao_get_job_by_id(job.id)
assert job.job_status == "in progress"
save_task_mock.assert_called_with(
(
str(job.service_id),
[
"something_encrypted",
"something_encrypted",
"something_encrypted",
"something_encrypted",
"something_encrypted",
"something_encrypted",
"something_encrypted",
"something_encrypted",
"something_encrypted",
"something_encrypted",
],
None,
),
queue="-normal-database-tasks",
)

def test_should_process_smss_job(self, notify_db_session, mocker):
service = create_service(message_limit=20)
template = create_template(service=service)
Expand Down
6 changes: 2 additions & 4 deletions tests/app/v2/notifications/test_post_notifications.py
Original file line number Diff line number Diff line change
Expand Up @@ -2359,9 +2359,7 @@ def test_post_bulk_flags_recipient_not_in_safelist_with_restricted_service(self,
def test_post_bulk_flags_not_enough_remaining_messages(self, client, notify_api, notify_db, notify_db_session, mocker):
service = create_service(message_limit=10)
template = create_sample_template(notify_db, notify_db_session, service=service, template_type="email")
messages_count_mock = mocker.patch(
"app.v2.notifications.post_notifications.fetch_todays_total_message_count", return_value=9
)
messages_count_mock = mocker.patch("app.v2.notifications.post_notifications.fetch_todays_email_count", return_value=9)
data = {
"name": "job_name",
"template_id": template.id,
Expand Down Expand Up @@ -2389,7 +2387,7 @@ def test_post_bulk_flags_not_enough_remaining_messages(self, client, notify_api,
def test_post_bulk_flags_not_enough_remaining_sms_messages(self, notify_api, client, notify_db, notify_db_session, mocker):
service = create_service(sms_daily_limit=10, message_limit=100)
template = create_sample_template(notify_db, notify_db_session, service=service, template_type="sms")
mocker.patch("app.v2.notifications.post_notifications.fetch_todays_total_message_count", return_value=9)
mocker.patch("app.v2.notifications.post_notifications.fetch_todays_email_count", return_value=9)
messages_count_mock = mocker.patch(
"app.v2.notifications.post_notifications.fetch_todays_requested_sms_count", return_value=9
)
Expand Down
Loading