From d3887e5c6c77509f721bb9e55b64a86f07303d6c Mon Sep 17 00:00:00 2001 From: Andrew Date: Tue, 3 Dec 2024 11:24:53 -0400 Subject: [PATCH 1/3] feat: validate annual limit (#2002) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat: validate annual limit * fix: modify wrapper to work with older redis keys * test: add tests! * chore: `utcnow` is deprecated, use `now` instead * feat(annual limits): show error if user is over annual limits * fix: move daily limit message into the appropriate template * feat: add a client to get daily/yearly sent stats using caching where possible * add translations * chore: formatting * fix send to work without FF too * feat: fix sending; write tests; 🙏 * fix: only check for `send_exceeds_annual_limit` when FF is on * fix(tests): only run the new tests with the FF on (they cant pass with it off!) * chore: remove unused imports * fix: move secondary message outside banner * fix: undo ruff change made by accident --------- Co-authored-by: William B <7444334+whabanks@users.noreply.github.com> --- app/__init__.py | 2 +- app/main/views/send.py | 40 +++- .../notification_counts_client.py | 75 ++++++ .../template_statistics_api_client.py | 46 ++++ .../check/too-many-email-messages.html | 22 +- .../check/too-many-sms-message-parts.html | 2 +- app/templates/views/check/column-errors.html | 19 +- app/translations/csv/fr.csv | 6 +- gunicorn_config.py | 1 + tests/app/main/views/test_send.py | 223 +++++++++++++++++- .../test_notification_counts_client.py | 89 +++++++ tests/conftest.py | 28 ++- 12 files changed, 526 insertions(+), 27 deletions(-) create mode 100644 app/notify_client/notification_counts_client.py create mode 100644 tests/app/notify_client/test_notification_counts_client.py diff --git a/app/__init__.py b/app/__init__.py index 0025ac021d..6e42f80bc9 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -226,7 +226,7 @@ def get_locale(): application.jinja_env.globals["show_tou_prompt"] = show_tou_prompt application.jinja_env.globals["parse_ua"] = parse application.jinja_env.globals["events_key"] = EVENTS_KEY - application.jinja_env.globals["now"] = datetime.utcnow + application.jinja_env.globals["now"] = datetime.now # Initialize Salesforce Account list if application.config["FF_SALESFORCE_CONTACT"]: diff --git a/app/main/views/send.py b/app/main/views/send.py index 2a5720e38b..e96c3f4f1d 100644 --- a/app/main/views/send.py +++ b/app/main/views/send.py @@ -1,5 +1,6 @@ import itertools import json +from datetime import datetime, timezone from string import ascii_uppercase from zipfile import BadZipFile @@ -53,6 +54,7 @@ ) from app.main.views.dashboard import aggregate_notifications_stats from app.models.user import Users +from app.notify_client.notification_counts_client import notification_counts_client from app.s3_client.s3_csv_client import ( copy_bulk_send_file_to_uploads, list_bulk_send_uploads, @@ -649,8 +651,8 @@ def _check_messages(service_id, template_id, upload_id, preview_row, letters_as_ sms_fragments_sent_today = daily_sms_fragment_count(service_id) emails_sent_today = daily_email_count(service_id) - remaining_sms_message_fragments = current_service.sms_daily_limit - sms_fragments_sent_today - remaining_email_messages = current_service.message_limit - emails_sent_today + remaining_sms_message_fragments_today = current_service.sms_daily_limit - sms_fragments_sent_today + remaining_email_messages_today = current_service.message_limit - emails_sent_today contents = s3download(service_id, upload_id) @@ -659,7 +661,7 @@ def _check_messages(service_id, template_id, upload_id, preview_row, letters_as_ email_reply_to = None sms_sender = None recipients_remaining_messages = ( - remaining_email_messages if db_template["template_type"] == "email" else remaining_sms_message_fragments + remaining_email_messages_today if db_template["template_type"] == "email" else remaining_sms_message_fragments_today ) if db_template["template_type"] == "email": @@ -743,8 +745,8 @@ def _check_messages(service_id, template_id, upload_id, preview_row, letters_as_ original_file_name=request.args.get("original_file_name", ""), upload_id=upload_id, form=CsvUploadForm(), - remaining_messages=remaining_email_messages, - remaining_sms_message_fragments=remaining_sms_message_fragments, + remaining_messages=remaining_email_messages_today, + remaining_sms_message_fragments=remaining_sms_message_fragments_today, sms_parts_to_send=sms_parts_to_send, is_sms_parts_estimated=is_sms_parts_estimated, choose_time_form=choose_time_form, @@ -783,7 +785,29 @@ def check_messages(service_id, template_id, upload_id, row_index=2): data["original_file_name"] = SanitiseASCII.encode(data.get("original_file_name", "")) data["sms_parts_requested"] = data["stats_daily"]["sms"]["requested"] data["sms_parts_remaining"] = current_service.sms_daily_limit - daily_sms_fragment_count(service_id) - data["send_exceeds_daily_limit"] = data["recipients"].sms_fragment_count > data["sms_parts_remaining"] + + if current_app.config["FF_ANNUAL_LIMIT"]: + # Override the remaining messages counts with the remaining annual counts, if the latter are lower + stats_ytd = notification_counts_client.get_all_notification_counts_for_year(service_id, datetime.now(timezone.utc).year) + remaining_sms_this_year = current_service.sms_annual_limit - stats_ytd["sms"] + remaining_email_this_year = current_service.email_annual_limit - stats_ytd["email"] + + # Show annual limit validation over the daily one (even if both are true) + data["send_exceeds_annual_limit"] = False + data["send_exceeds_daily_limit"] = False + if data["template"].template_type == "email": + if remaining_email_this_year < data["count_of_recipients"]: + data["recipients_remaining_messages"] = remaining_email_this_year + data["send_exceeds_annual_limit"] = True + else: + if remaining_sms_this_year < data["count_of_recipients"]: + data["recipients_remaining_messages"] = remaining_sms_this_year + data["send_exceeds_annual_limit"] = True + else: + data["send_exceeds_daily_limit"] = data["recipients"].sms_fragment_count > data["sms_parts_remaining"] + + else: + data["send_exceeds_daily_limit"] = data["recipients"].sms_fragment_count > data["sms_parts_remaining"] if ( data["recipients"].too_many_rows @@ -804,6 +828,10 @@ def check_messages(service_id, template_id, upload_id, row_index=2): if data["send_exceeds_daily_limit"]: return render_template("views/check/column-errors.html", **data) + if current_app.config["FF_ANNUAL_LIMIT"]: + if data["send_exceeds_annual_limit"]: + return render_template("views/check/column-errors.html", **data) + metadata_kwargs = { "notification_count": data["count_of_recipients"], "template_id": str(template_id), diff --git a/app/notify_client/notification_counts_client.py b/app/notify_client/notification_counts_client.py new file mode 100644 index 0000000000..ae6e1a4930 --- /dev/null +++ b/app/notify_client/notification_counts_client.py @@ -0,0 +1,75 @@ +from notifications_utils.clients.redis import ( + email_daily_count_cache_key, + sms_daily_count_cache_key, +) + +from app import redis_client, service_api_client, template_statistics_client + + +class NotificationCounts: + def get_all_notification_counts_for_today(self, service_id): + # try to get today's stats from redis + todays_sms = redis_client.get(sms_daily_count_cache_key(service_id)) + todays_email = redis_client.get(email_daily_count_cache_key(service_id)) + + if todays_sms is not None and todays_email is not None: + return {"sms": todays_sms, "email": todays_email} + # fallback to the API if the stats are not in redis + else: + stats = template_statistics_client.get_template_statistics_for_service(service_id, limit_days=1) + transformed_stats = _aggregate_notifications_stats(stats) + + return transformed_stats + + def get_all_notification_counts_for_year(self, service_id, year): + """ + Get total number of notifications by type for the current service for the current year + + Return value: + { + 'sms': int, + 'email': int + } + + """ + stats_today = self.get_all_notification_counts_for_today(service_id) + stats_this_year = service_api_client.get_monthly_notification_stats(service_id, year)["data"] + stats_this_year = _aggregate_stats_from_service_api(stats_this_year) + # aggregate stats_today and stats_this_year + for template_type in ["sms", "email"]: + stats_this_year[template_type] += stats_today[template_type] + + return stats_this_year + + +# TODO: consolidate this function and other functions that transform the results of template_statistics_client calls +def _aggregate_notifications_stats(template_statistics): + template_statistics = _filter_out_cancelled_stats(template_statistics) + notifications = {"sms": 0, "email": 0} + for stat in template_statistics: + notifications[stat["template_type"]] += stat["count"] + + return notifications + + +def _filter_out_cancelled_stats(template_statistics): + return [s for s in template_statistics if s["status"] != "cancelled"] + + +def _aggregate_stats_from_service_api(stats): + """Aggregate monthly notification stats excluding cancelled""" + total_stats = {"sms": {}, "email": {}} + + for month_data in stats.values(): + for msg_type in ["sms", "email"]: + if msg_type in month_data: + for status, count in month_data[msg_type].items(): + if status != "cancelled": + if status not in total_stats[msg_type]: + total_stats[msg_type][status] = 0 + total_stats[msg_type][status] += count + + return {msg_type: sum(counts.values()) for msg_type, counts in total_stats.items()} + + +notification_counts_client = NotificationCounts() diff --git a/app/notify_client/template_statistics_api_client.py b/app/notify_client/template_statistics_api_client.py index 132f8938fa..fb6acbc138 100644 --- a/app/notify_client/template_statistics_api_client.py +++ b/app/notify_client/template_statistics_api_client.py @@ -1,4 +1,7 @@ +from itertools import groupby + from app.notify_client import NotifyAdminAPIClient +from app.utils import DELIVERED_STATUSES, FAILURE_STATUSES class TemplateStatisticsApiClient(NotifyAdminAPIClient): @@ -17,3 +20,46 @@ def get_template_statistics_for_template(self, service_id, template_id): template_statistics_client = TemplateStatisticsApiClient() + + +class TemplateStatistics: + def __init__(self, stats): + self.stats = stats + + def as_aggregates(self): + template_statistics = self._filter_out_cancelled_stats(self.stats) + notifications = { + template_type: {status: 0 for status in ("requested", "delivered", "failed")} for template_type in ["sms", "email"] + } + for stat in template_statistics: + notifications[stat["template_type"]]["requested"] += stat["count"] + if stat["status"] in DELIVERED_STATUSES: + notifications[stat["template_type"]]["delivered"] += stat["count"] + elif stat["status"] in FAILURE_STATUSES: + notifications[stat["template_type"]]["failed"] += stat["count"] + + return notifications + + def as_template_usage(self, sort_key="count"): + template_statistics = self._filter_out_cancelled_stats(self.stats) + templates = [] + for k, v in groupby( + sorted(template_statistics, key=lambda x: x["template_id"]), + key=lambda x: x["template_id"], + ): + template_stats = list(v) + + templates.append( + { + "template_id": k, + "template_name": template_stats[0]["template_name"], + "template_type": template_stats[0]["template_type"], + "is_precompiled_letter": template_stats[0]["is_precompiled_letter"], + "count": sum(s["count"] for s in template_stats), + } + ) + + return sorted(templates, key=lambda x: x[sort_key], reverse=True) + + def _filter_out_cancelled_stats(self, template_statistics): + return [s for s in template_statistics if s["status"] != "cancelled"] diff --git a/app/templates/partials/check/too-many-email-messages.html b/app/templates/partials/check/too-many-email-messages.html index 987846c472..4ead4e464b 100644 --- a/app/templates/partials/check/too-many-email-messages.html +++ b/app/templates/partials/check/too-many-email-messages.html @@ -1,9 +1,29 @@ +{% from "components/banner.html" import banner_wrapper %} {% from "components/links.html" import content_link %}

{%- if current_service.trial_mode %} {{ _("Your service is in trial mode. To send more messages, request to go live").format(url_for('main.request_to_go_live', service_id=current_service.id)) }} {% else %} - {{ _("To request a daily limit above {} emails, {}").format(current_service.message_limit, content_link(_("contact us"), url_for('main.contact'), is_external_link=true)) }} + {% if send_exceeds_annual_limit %} + +

{{ _('{} can only send {} more email messages until annual limit resets'.format(current_service.name, recipients_remaining_messages)) }}

+

+ {{ _('To send some of these messages now, edit the spreadsheet to {} recipients maximum. '.format(recipients_remaining_messages)) }} + {{ _('To send to recipients you removed, wait until April 1, {} or contact them some other way.'.format(now().year)) }} +

+ + {% elif send_exceeds_daily_limit or recipients.more_rows_than_can_send %} + {% call banner_wrapper(type='dangerous') %} + {{ _("To request a daily limit above {} emails, {}").format(current_service.message_limit, content_link(_("contact us"), url_for('main.contact'), is_external_link=true)) }} + {% endcall %} + +

{{ _('You cannot send all these email messages today') }}

+

+ {{ _("You can try sending these messages after {} Eastern Time. Check {}.").format(time_to_reset[current_lang], + content_link(_("your current local time"), _('https://nrc.canada.ca/en/web-clock/'), is_external_link=true))}} +

+ {% endif %} + {%- endif -%}

\ No newline at end of file diff --git a/app/templates/partials/check/too-many-sms-message-parts.html b/app/templates/partials/check/too-many-sms-message-parts.html index 1000fe1ac5..8ceee20762 100644 --- a/app/templates/partials/check/too-many-sms-message-parts.html +++ b/app/templates/partials/check/too-many-sms-message-parts.html @@ -1,6 +1,6 @@ {% from "components/links.html" import content_link %} -

+

{%- if current_service.trial_mode %} {{ _("Your service is in trial mode. To send more messages, request to go live").format(url_for('main.request_to_go_live', service_id=current_service.id)) }} {% else %} diff --git a/app/templates/views/check/column-errors.html b/app/templates/views/check/column-errors.html index 32f2ae9e8e..852d30f0bd 100644 --- a/app/templates/views/check/column-errors.html +++ b/app/templates/views/check/column-errors.html @@ -10,7 +10,9 @@ {% set prefix_txt = _('a column called') %} {% set prefix_plural_txt = _('columns called') %} -{% if send_exceeds_daily_limit and (sms_parts_remaining <= 0) %} +{% if send_exceeds_annual_limit %} + {% set page_title = _('These messages exceed the annual limit') %} +{% elif send_exceeds_daily_limit and (sms_parts_remaining <= 0) %} {% set page_title = _('These messages exceed your daily limit') %} {% elif send_exceeds_daily_limit or recipients.more_rows_than_can_send %} {% set page_title = _('These messages exceed your daily limit') %} @@ -168,20 +170,9 @@

{{ _('You cannot send all these text messages today') {% call banner_wrapper(type='dangerous') %} {% include "partials/check/too-many-email-messages.html" %} {% endcall %} - {% elif recipients.more_rows_than_can_send %} - {% call banner_wrapper(type='dangerous') %} - {% include "partials/check/too-many-email-messages.html" %} - {% endcall %} -

{{ _('You cannot send all these email messages today') }}

-

- {{ _("You can try sending these messages after {} Eastern Time. Check {}.").format(time_to_reset[current_lang], - content_link(_("your current local time"), _('https://nrc.canada.ca/en/web-clock/'), is_external_link=true))}} -

- - + {% elif recipients.more_rows_than_can_send or send_exceeds_annual_limit %} + {% include "partials/check/too-many-email-messages.html" %} {% endif %} - - {% if not send_exceeds_daily_limit %} diff --git a/app/translations/csv/fr.csv b/app/translations/csv/fr.csv index 57aead4f21..fe2c41b635 100644 --- a/app/translations/csv/fr.csv +++ b/app/translations/csv/fr.csv @@ -2027,4 +2027,8 @@ "Annual usage","Utilisation annuelle" "resets at 7pm Eastern Time","Réinitialisation à 19 h, heure de l’Est" "Visit usage report","Consulter le rapport d’utilisation" -"Month by month totals","Totaux mensuels" \ No newline at end of file +"Month by month totals","Totaux mensuels" +"{} can only send {} more email messages until annual limit resets","FR: {} can only send {} more email messages until annual limit resets" +"To send some of these messages now, edit the spreadsheet to {} recipients maximum. ","FR: To send some of these messages now, edit the spreadsheet to {} recipients maximum. " +"To send to recipients you removed, wait until April 1, {} or contact them some other way.","FR: To send to recipients you removed, wait until April 1, {} or contact them some other way" +"These messages exceed the annual limit","FR: These messages exceed the annual limit" \ No newline at end of file diff --git a/gunicorn_config.py b/gunicorn_config.py index 63325ca386..cf6e5e7112 100644 --- a/gunicorn_config.py +++ b/gunicorn_config.py @@ -51,6 +51,7 @@ # Start timer for total running time start_time = time.time() + def on_starting(server): server.log.info("Starting Notifications Admin") diff --git a/tests/app/main/views/test_send.py b/tests/app/main/views/test_send.py index 78df4b10cc..c1350bdb46 100644 --- a/tests/app/main/views/test_send.py +++ b/tests/app/main/views/test_send.py @@ -6,6 +6,7 @@ from io import BytesIO from itertools import repeat from os import path +from unittest.mock import patch from uuid import uuid4 from zipfile import BadZipFile @@ -41,6 +42,7 @@ mock_get_service_letter_template, mock_get_service_template, normalize_spaces, + set_config, ) template_types = ["email", "sms"] @@ -2543,6 +2545,7 @@ def test_check_messages_shows_too_many_sms_messages_errors( mock_get_jobs, mock_s3_download, mock_s3_set_metadata, + mock_notification_counts_client, fake_uuid, num_requested, expected_msg, @@ -2560,6 +2563,10 @@ def test_check_messages_shows_too_many_sms_messages_errors( }, ) + mock_notification_counts_client.get_all_notification_counts_for_year.return_value = { + "sms": 0, + "email": 0, + } with client_request.session_transaction() as session: session["file_uploads"] = { fake_uuid: { @@ -2584,6 +2591,30 @@ def test_check_messages_shows_too_many_sms_messages_errors( assert details == expected_msg +@pytest.fixture +def mock_notification_counts_client(): + with patch("app.main.views.send.notification_counts_client") as mock: + yield mock + + +@pytest.fixture +def mock_daily_sms_fragment_count(): + with patch("app.main.views.send.daily_sms_fragment_count") as mock: + yield mock + + +@pytest.fixture +def mock_daily_email_count(): + with patch("app.main.views.send.daily_email_count") as mock: + yield mock + + +@pytest.fixture +def mock_get_service_template_annual_limits(): + with patch("app.service_api_client.get_service_template") as mock: + yield mock + + @pytest.mark.parametrize( "num_requested,expected_msg", [ @@ -2601,6 +2632,7 @@ def test_check_messages_shows_too_many_email_messages_errors( mock_get_template_statistics, mock_get_job_doesnt_exist, mock_get_jobs, + mock_notification_counts_client, fake_uuid, num_requested, expected_msg, @@ -2617,7 +2649,10 @@ def test_check_messages_shows_too_many_email_messages_errors( "email": {"requested": num_requested, "delivered": 0, "failed": 0}, }, ) - + mock_notification_counts_client.get_all_notification_counts_for_year.return_value = { + "sms": 0, + "email": 0, + } with client_request.session_transaction() as session: session["file_uploads"] = { fake_uuid: { @@ -3401,3 +3436,189 @@ class Object(object): multiple_choise_options = [x.text.strip() for x in options] assert multiple_choise_options == expected_filenames + + +class TestAnnualLimitsSend: + @pytest.mark.parametrize( + "num_being_sent, num_sent_today, num_sent_this_year, expect_to_see_annual_limit_msg, expect_to_see_daily_limit_msg", + [ + # annual limit for mock_get_live_service is 10,000email/10,000sms + # daily limit for mock_get_live_service is 1,000email/1,000sms + # 1000 have already been sent today, trying to send 100 more [over both limits] + (100, 1000, 10000, True, False), + # No sent yet today or this year, trying to send 1001 [over both limits] + (10001, 0, 0, True, False), + # 600 have already been sent this year, trying to send 500 more [over annual limit but not daily] + (500, 0, 9600, True, False), + # No sent yet today or this year, trying to send 1001 [over daily limit but not annual] + (1001, 0, 0, False, True), + # No sent yet today or this year, trying to send 100 [over neither limit] + (100, 0, 0, False, False), + ], + ids=[ + "email_over_both_limits", + "email_over_both_limits2", + "email_over_annual_but_not_daily", + "email_over_daily_but_not_annual", + "email_over_neither", + ], + ) + def test_email_send_fails_approrpiately_when_over_limits( + self, + mocker, + client_request, + mock_get_live_service, # set email_annual_limit and sms_annual_limit to 1000 + mock_get_users_by_service, + mock_get_service_email_template_without_placeholders, + mock_get_template_statistics, + mock_get_job_doesnt_exist, + mock_get_jobs, + mock_s3_set_metadata, + mock_notification_counts_client, + mock_daily_sms_fragment_count, + mock_daily_email_count, + fake_uuid, + num_being_sent, + num_sent_today, + num_sent_this_year, + expect_to_see_annual_limit_msg, + expect_to_see_daily_limit_msg, + app_, + ): + with set_config(app_, "FF_ANNUAL_LIMIT", True): + mocker.patch( + "app.main.views.send.s3download", + return_value=",\n".join( + ["email address"] + ([mock_get_users_by_service(None)[0]["email_address"]] * num_being_sent) + ), + ) + + # mock that `num_sent_this_year` have already been sent this year + mock_notification_counts_client.get_all_notification_counts_for_year.return_value = { + "sms": 1000, # not used in test but needs a value + "email": num_sent_this_year, + } + + # mock that we've already sent `emails_sent_today` emails today + mock_daily_email_count.return_value = num_sent_today + mock_daily_sms_fragment_count.return_value = 900 # not used in test but needs a value + + with client_request.session_transaction() as session: + session["file_uploads"] = { + fake_uuid: { + "template_id": fake_uuid, + "notification_count": 1, + "valid": True, + } + } + + page = client_request.get( + "main.check_messages", + service_id=SERVICE_ONE_ID, + template_id=fake_uuid, + upload_id=fake_uuid, + original_file_name="valid.csv", + _test_page_title=False, + ) + + if expect_to_see_annual_limit_msg: + assert page.find(attrs={"data-testid": "exceeds-annual"}) is not None + else: + assert page.find(attrs={"data-testid": "exceeds-annual"}) is None + + if expect_to_see_daily_limit_msg: + assert page.find(attrs={"data-testid": "exceeds-daily"}) is not None + else: + assert page.find(attrs={"data-testid": "exceeds-daily"}) is None + + @pytest.mark.parametrize( + "num_being_sent, num_sent_today, num_sent_this_year, expect_to_see_annual_limit_msg, expect_to_see_daily_limit_msg", + [ + # annual limit for mock_get_live_service is 10,000email/10,000sms + # daily limit for mock_get_live_service is 1,000email/1,000sms + # 1000 have already been sent today, trying to send 100 more [over both limits] + (100, 1000, 10000, True, False), + # No sent yet today or this year, trying to send 1001 [over both limits] + (10001, 0, 0, True, False), + # 600 have already been sent this year, trying to send 500 more [over annual limit but not daily] + (500, 0, 9600, True, False), + # No sent yet today or this year, trying to send 1001 [over daily limit but not annual] + (1001, 0, 0, False, True), + # No sent yet today or this year, trying to send 100 [over neither limit] + (100, 0, 0, False, False), + ], + ids=[ + "sms_over_both_limits", + "sms_over_both_limits2", + "sms_over_annual_but_not_daily", + "sms_over_daily_but_not_annual", + "sms_over_neither", + ], + ) + def test_sms_send_fails_approrpiately_when_over_limits( + self, + mocker, + client_request, + mock_get_live_service, # set email_annual_limit and sms_annual_limit to 1000 + mock_get_users_by_service, + mock_get_service_sms_template_without_placeholders, + mock_get_template_statistics, + mock_get_job_doesnt_exist, + mock_get_jobs, + mock_s3_set_metadata, + mock_notification_counts_client, + mock_daily_sms_fragment_count, + mock_daily_email_count, + fake_uuid, + num_being_sent, + num_sent_today, + num_sent_this_year, + expect_to_see_annual_limit_msg, + expect_to_see_daily_limit_msg, + app_, + ): + with set_config(app_, "FF_ANNUAL_LIMIT", True): # REMOVE LINE WHEN FF REMOVED + mocker.patch( + "app.main.views.send.s3download", + return_value=",\n".join( + ["phone number"] + ([mock_get_users_by_service(None)[0]["mobile_number"]] * num_being_sent) + ), + ) + + # mock that `num_sent_this_year` have already been sent this year + mock_notification_counts_client.get_all_notification_counts_for_year.return_value = { + "sms": num_sent_this_year, + "email": 1000, # not used in test but needs a value + } + + # mock that we've already sent `emails_sent_today` emails today + mock_daily_email_count.return_value = 900 # not used in test but needs a value + mock_daily_sms_fragment_count.return_value = num_sent_today + + with client_request.session_transaction() as session: + session["file_uploads"] = { + fake_uuid: { + "template_id": fake_uuid, + "notification_count": 1, + "valid": True, + } + } + + page = client_request.get( + "main.check_messages", + service_id=SERVICE_ONE_ID, + template_id=fake_uuid, + upload_id=fake_uuid, + original_file_name="valid.csv", + _test_page_title=False, + ) + + if expect_to_see_annual_limit_msg: + assert page.find(attrs={"data-testid": "exceeds-annual"}) is not None + else: + assert page.find(attrs={"data-testid": "exceeds-annual"}) is None + + if expect_to_see_daily_limit_msg: + assert page.find(attrs={"data-testid": "exceeds-daily"}) is not None + else: + assert page.find(attrs={"data-testid": "exceeds-daily"}) is None diff --git a/tests/app/notify_client/test_notification_counts_client.py b/tests/app/notify_client/test_notification_counts_client.py new file mode 100644 index 0000000000..fa7183aea8 --- /dev/null +++ b/tests/app/notify_client/test_notification_counts_client.py @@ -0,0 +1,89 @@ +from unittest.mock import patch + +import pytest + +from app.notify_client.notification_counts_client import NotificationCounts + + +@pytest.fixture +def mock_redis(): + with patch("app.notify_client.notification_counts_client.redis_client") as mock: + yield mock + + +@pytest.fixture +def mock_template_stats(): + with patch("app.notify_client.notification_counts_client.template_statistics_client") as mock: + yield mock + + +@pytest.fixture +def mock_service_api(): + with patch("app.notify_client.notification_counts_client.service_api_client") as mock: + yield mock + + +class TestNotificationCounts: + def test_get_all_notification_counts_for_today_redis_has_data(self, mock_redis): + # Setup + mock_redis.get.side_effect = [5, 10] # sms, email + wrapper = NotificationCounts() + + # Execute + result = wrapper.get_all_notification_counts_for_today("service-123") + + # Assert + assert result == {"sms": 5, "email": 10} + assert mock_redis.get.call_count == 2 + + @pytest.mark.parametrize( + "redis_side_effect, expected_result", + [ + ([None, None], {"sms": 10, "email": 10}), + ([None, 10], {"sms": 10, "email": 10}), # Falls back to API if either is None + ([10, None], {"sms": 10, "email": 10}), # Falls back to API if either is None + ], + ) + def test_get_all_notification_counts_for_today_redis_missing_data( + self, mock_redis, mock_template_stats, redis_side_effect, expected_result + ): + # Setup + mock_redis.get.side_effect = redis_side_effect + mock_template_stats.get_template_statistics_for_service.return_value = [ + {"template_id": "a1", "template_type": "sms", "count": 3, "status": "delivered"}, + {"template_id": "a2", "template_type": "email", "count": 7, "status": "temporary-failure"}, + {"template_id": "a3", "template_type": "email", "count": 3, "status": "delivered"}, + {"template_id": "a4", "template_type": "sms", "count": 7, "status": "delivered"}, + ] + + wrapper = NotificationCounts() + + # Execute + result = wrapper.get_all_notification_counts_for_today("service-123") + + # Assert + assert result == {"sms": 10, "email": 10} + mock_template_stats.get_template_statistics_for_service.assert_called_once() + + def test_get_all_notification_counts_for_year(self, mock_service_api): + # Setup + mock_service_api.get_monthly_notification_stats.return_value = { + "data": { + "2024-01": { + "sms": {"sent": 1, "temporary-failure:": 22}, + "email": {"delivered": 1, "permanent-failure": 1, "sending": 12, "technical-failure": 1}, + }, + "2024-02": {"sms": {"sent": 1}, "email": {"delivered": 1}}, + } + } + wrapper = NotificationCounts() + + with patch.object(wrapper, "get_all_notification_counts_for_today") as mock_today: + mock_today.return_value = {"sms": 5, "email": 5} + + # Execute + result = wrapper.get_all_notification_counts_for_year("service-123", 2024) + + # Assert + assert result["sms"] == 29 # 1 + 22 + 1 + 5 + assert result["email"] == 21 # 1 + 1 + 12 + 1 + 1 + 5 diff --git a/tests/conftest.py b/tests/conftest.py index 94ff0ca5be..21ed1f782b 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -550,7 +550,14 @@ def fake_uuid(): @pytest.fixture(scope="function") def mock_get_service(mocker, api_user_active): def _get(service_id): - service = service_json(service_id, users=[api_user_active["id"]], message_limit=50, sms_daily_limit=20) + service = service_json( + service_id, + users=[api_user_active["id"]], + message_limit=50, + sms_daily_limit=20, + email_annual_limit=1000, + sms_annual_limit=1000, + ) return {"data": service} return mocker.patch("app.service_api_client.get_service", side_effect=_get) @@ -675,7 +682,9 @@ def mock_service_email_from_is_unique(mocker): @pytest.fixture(scope="function") def mock_get_live_service(mocker, api_user_active): def _get(service_id): - service = service_json(service_id, users=[api_user_active["id"]], restricted=False) + service = service_json( + service_id, users=[api_user_active["id"]], restricted=False, sms_annual_limit=10000, email_annual_limit=10000 + ) return {"data": service} return mocker.patch("app.service_api_client.get_service", side_effect=_get) @@ -971,6 +980,21 @@ def _get(service_id, template_id, version=None): return mocker.patch("app.service_api_client.get_service_template", side_effect=_get) +@pytest.fixture(scope="function") +def mock_get_service_sms_template_without_placeholders(mocker): + def _get(service_id, template_id, version=None): + template = template_json( + service_id, + template_id, + "Two week reminder", + "sms", + "Yo.", + ) + return {"data": template} + + return mocker.patch("app.service_api_client.get_service_template", side_effect=_get) + + @pytest.fixture(scope="function") def mock_get_service_letter_template(mocker, content=None, subject=None, postage="second"): def _get(service_id, template_id, version=None, postage=postage): From 52dc3405a40224b49b4bd7fe159f639a9bc5c198 Mon Sep 17 00:00:00 2001 From: Ben Larabie Date: Tue, 3 Dec 2024 11:09:09 -0500 Subject: [PATCH 2/3] Admin workflow fix (#2006) * Admin workflow to use the correct secret for OP * commenting out sbom for now --- .github/workflows/docker.yaml | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/.github/workflows/docker.yaml b/.github/workflows/docker.yaml index 6c1871f36d..a6f8ceae91 100644 --- a/.github/workflows/docker.yaml +++ b/.github/workflows/docker.yaml @@ -8,7 +8,7 @@ env: AWS_REGION: ca-central-1 DOCKER_ORG: public.ecr.aws/v6b8u5o6 DOCKER_SLUG: public.ecr.aws/v6b8u5o6/notify-admin - OP_SERVICE_ACCOUNT_TOKEN: ${{ secrets.OP_SERVICE_ACCOUNT_TOKEN }} + OP_SERVICE_ACCOUNT_TOKEN: ${{ secrets.OP_SERVICE_ACCOUNT_TOKEN_STAGING }} permissions: id-token: write # This is required for requesting the OIDC JWT @@ -108,15 +108,15 @@ jobs: env: TOKEN: ${{ steps.notify-pr-bot.outputs.token }} - - name: Docker generate SBOM - uses: cds-snc/security-tools/.github/actions/generate-sbom@34794baf2af592913bb5b51d8df4f8d0acc49b6f # v3.2.0 - env: - TRIVY_DB_REPOSITORY: ${{ vars.TRIVY_DB_REPOSITORY }} - with: - docker_image: "${{ env.DOCKER_SLUG }}:latest" - dockerfile_path: "ci/Dockerfile" - sbom_name: "notification-admin" - token: "${{ secrets.GITHUB_TOKEN }}" + #- name: Docker generate SBOM + # uses: cds-snc/security-tools/.github/actions/generate-sbom@34794baf2af592913bb5b51d8df4f8d0acc49b6f # v3.2.0 + # env: + # TRIVY_DB_REPOSITORY: ${{ vars.TRIVY_DB_REPOSITORY }} + # with: + # docker_image: "${{ env.DOCKER_SLUG }}:latest" + # dockerfile_path: "ci/Dockerfile" + # sbom_name: "notification-admin" + # token: "${{ secrets.GITHUB_TOKEN }}" - name: Notify Slack channel if this job failed if: ${{ failure() }} From 677f52f07eb528291da116a689fe9f3579535f9c Mon Sep 17 00:00:00 2001 From: Ben Larabie Date: Tue, 3 Dec 2024 11:41:42 -0500 Subject: [PATCH 3/3] adding docker sbom back (#2007) --- .github/workflows/docker.yaml | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/.github/workflows/docker.yaml b/.github/workflows/docker.yaml index a6f8ceae91..71358535af 100644 --- a/.github/workflows/docker.yaml +++ b/.github/workflows/docker.yaml @@ -108,15 +108,15 @@ jobs: env: TOKEN: ${{ steps.notify-pr-bot.outputs.token }} - #- name: Docker generate SBOM - # uses: cds-snc/security-tools/.github/actions/generate-sbom@34794baf2af592913bb5b51d8df4f8d0acc49b6f # v3.2.0 - # env: - # TRIVY_DB_REPOSITORY: ${{ vars.TRIVY_DB_REPOSITORY }} - # with: - # docker_image: "${{ env.DOCKER_SLUG }}:latest" - # dockerfile_path: "ci/Dockerfile" - # sbom_name: "notification-admin" - # token: "${{ secrets.GITHUB_TOKEN }}" + - name: Docker generate SBOM + uses: cds-snc/security-tools/.github/actions/generate-sbom@34794baf2af592913bb5b51d8df4f8d0acc49b6f # v3.2.0 + env: + TRIVY_DB_REPOSITORY: ${{ vars.TRIVY_DB_REPOSITORY }} + with: + docker_image: "${{ env.DOCKER_SLUG }}:latest" + dockerfile_path: "ci/Dockerfile" + sbom_name: "notification-admin" + token: "${{ secrets.GITHUB_TOKEN }}" - name: Notify Slack channel if this job failed if: ${{ failure() }}