From a8558d9b4e5a8e9c6e768a5247626f8ddbf6d327 Mon Sep 17 00:00:00 2001 From: Andrew Leith Date: Wed, 4 Dec 2024 14:26:03 +0000 Subject: [PATCH] Revert "feat: validate annual limit (#2002)" This reverts commit d3887e5c6c77509f721bb9e55b64a86f07303d6c. --- 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, 27 insertions(+), 526 deletions(-) delete mode 100644 app/notify_client/notification_counts_client.py delete mode 100644 tests/app/notify_client/test_notification_counts_client.py diff --git a/app/__init__.py b/app/__init__.py index 6e42f80bc9..0025ac021d 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.now + application.jinja_env.globals["now"] = datetime.utcnow # 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 e96c3f4f1d..2a5720e38b 100644 --- a/app/main/views/send.py +++ b/app/main/views/send.py @@ -1,6 +1,5 @@ import itertools import json -from datetime import datetime, timezone from string import ascii_uppercase from zipfile import BadZipFile @@ -54,7 +53,6 @@ ) 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, @@ -651,8 +649,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_today = current_service.sms_daily_limit - sms_fragments_sent_today - remaining_email_messages_today = current_service.message_limit - emails_sent_today + remaining_sms_message_fragments = current_service.sms_daily_limit - sms_fragments_sent_today + remaining_email_messages = current_service.message_limit - emails_sent_today contents = s3download(service_id, upload_id) @@ -661,7 +659,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_today if db_template["template_type"] == "email" else remaining_sms_message_fragments_today + remaining_email_messages if db_template["template_type"] == "email" else remaining_sms_message_fragments ) if db_template["template_type"] == "email": @@ -745,8 +743,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_today, - remaining_sms_message_fragments=remaining_sms_message_fragments_today, + remaining_messages=remaining_email_messages, + remaining_sms_message_fragments=remaining_sms_message_fragments, sms_parts_to_send=sms_parts_to_send, is_sms_parts_estimated=is_sms_parts_estimated, choose_time_form=choose_time_form, @@ -785,29 +783,7 @@ 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) - - 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"] + data["send_exceeds_daily_limit"] = data["recipients"].sms_fragment_count > data["sms_parts_remaining"] if ( data["recipients"].too_many_rows @@ -828,10 +804,6 @@ 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 deleted file mode 100644 index ae6e1a4930..0000000000 --- a/app/notify_client/notification_counts_client.py +++ /dev/null @@ -1,75 +0,0 @@ -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 fb6acbc138..132f8938fa 100644 --- a/app/notify_client/template_statistics_api_client.py +++ b/app/notify_client/template_statistics_api_client.py @@ -1,7 +1,4 @@ -from itertools import groupby - from app.notify_client import NotifyAdminAPIClient -from app.utils import DELIVERED_STATUSES, FAILURE_STATUSES class TemplateStatisticsApiClient(NotifyAdminAPIClient): @@ -20,46 +17,3 @@ 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 4ead4e464b..987846c472 100644 --- a/app/templates/partials/check/too-many-email-messages.html +++ b/app/templates/partials/check/too-many-email-messages.html @@ -1,29 +1,9 @@ -{% 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 %} - {% 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 %} - + {{ _("To request a daily limit above {} emails, {}").format(current_service.message_limit, content_link(_("contact us"), url_for('main.contact'), is_external_link=true)) }} {%- 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 8ceee20762..1000fe1ac5 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 852d30f0bd..32f2ae9e8e 100644 --- a/app/templates/views/check/column-errors.html +++ b/app/templates/views/check/column-errors.html @@ -10,9 +10,7 @@ {% set prefix_txt = _('a column called') %} {% set prefix_plural_txt = _('columns called') %} -{% if send_exceeds_annual_limit %} - {% set page_title = _('These messages exceed the annual limit') %} -{% elif send_exceeds_daily_limit and (sms_parts_remaining <= 0) %} +{% if 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') %} @@ -170,9 +168,20 @@

{{ _('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 or send_exceeds_annual_limit %} - {% include "partials/check/too-many-email-messages.html" %} + {% 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))}} +

+ + {% endif %} + + {% if not send_exceeds_daily_limit %} diff --git a/app/translations/csv/fr.csv b/app/translations/csv/fr.csv index fe2c41b635..57aead4f21 100644 --- a/app/translations/csv/fr.csv +++ b/app/translations/csv/fr.csv @@ -2027,8 +2027,4 @@ "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" -"{} 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 +"Month by month totals","Totaux mensuels" \ No newline at end of file diff --git a/gunicorn_config.py b/gunicorn_config.py index cf6e5e7112..63325ca386 100644 --- a/gunicorn_config.py +++ b/gunicorn_config.py @@ -51,7 +51,6 @@ # 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 c1350bdb46..78df4b10cc 100644 --- a/tests/app/main/views/test_send.py +++ b/tests/app/main/views/test_send.py @@ -6,7 +6,6 @@ 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 @@ -42,7 +41,6 @@ mock_get_service_letter_template, mock_get_service_template, normalize_spaces, - set_config, ) template_types = ["email", "sms"] @@ -2545,7 +2543,6 @@ 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, @@ -2563,10 +2560,6 @@ 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: { @@ -2591,30 +2584,6 @@ 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", [ @@ -2632,7 +2601,6 @@ 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, @@ -2649,10 +2617,7 @@ 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: { @@ -3436,189 +3401,3 @@ 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 deleted file mode 100644 index fa7183aea8..0000000000 --- a/tests/app/notify_client/test_notification_counts_client.py +++ /dev/null @@ -1,89 +0,0 @@ -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 21ed1f782b..94ff0ca5be 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -550,14 +550,7 @@ 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, - email_annual_limit=1000, - sms_annual_limit=1000, - ) + service = service_json(service_id, users=[api_user_active["id"]], message_limit=50, sms_daily_limit=20) return {"data": service} return mocker.patch("app.service_api_client.get_service", side_effect=_get) @@ -682,9 +675,7 @@ 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, sms_annual_limit=10000, email_annual_limit=10000 - ) + service = service_json(service_id, users=[api_user_active["id"]], restricted=False) return {"data": service} return mocker.patch("app.service_api_client.get_service", side_effect=_get) @@ -980,21 +971,6 @@ 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):