diff --git a/app/main/views/dashboard.py b/app/main/views/dashboard.py index de84979790..41a6cd2513 100644 --- a/app/main/views/dashboard.py +++ b/app/main/views/dashboard.py @@ -15,6 +15,12 @@ from flask_babel import _ from flask_babel import lazy_gettext as _l from flask_login import current_user +from notifications_utils.clients.redis.annual_limit import ( + EMAIL_DELIVERED_TODAY, + EMAIL_FAILED_TODAY, + SMS_DELIVERED_TODAY, + SMS_FAILED_TODAY, +) from werkzeug.utils import redirect from app import ( @@ -29,6 +35,7 @@ from app.models.enum.bounce_rate_status import BounceRateStatus from app.models.enum.notification_statuses import NotificationStatuses from app.models.enum.template_types import TemplateType +from app.notify_client.notification_counts_client import notification_counts_client from app.statistics_utils import add_rate_to_job, get_formatted_percentage from app.utils import ( DELIVERED_STATUSES, @@ -233,10 +240,11 @@ def combine_daily_to_annual(daily, annual, mode): if mode == "redis": # the redis client omits properties if there are no counts yet, so account for this here\ daily_redis = { - field: daily.get(field, 0) for field in ["sms_delivered", "sms_failed", "email_delivered", "email_failed"] + field: daily.get(field, 0) + for field in [SMS_DELIVERED_TODAY, SMS_FAILED_TODAY, EMAIL_DELIVERED_TODAY, EMAIL_FAILED_TODAY] } - annual["sms"] += daily_redis["sms_delivered"] + daily_redis["sms_failed"] - annual["email"] += daily_redis["email_delivered"] + daily_redis["email_failed"] + annual["sms"] += daily_redis[SMS_DELIVERED_TODAY] + daily_redis[SMS_FAILED_TODAY] + annual["email"] += daily_redis[EMAIL_DELIVERED_TODAY] + daily_redis[EMAIL_FAILED_TODAY] elif mode == "db": annual["sms"] += daily["sms"]["requested"] annual["email"] += daily["email"]["requested"] @@ -247,13 +255,14 @@ def combine_daily_to_monthly(daily, monthly, mode): if mode == "redis": # the redis client omits properties if there are no counts yet, so account for this here\ daily_redis = { - field: daily.get(field, 0) for field in ["sms_delivered", "sms_failed", "email_delivered", "email_failed"] + field: daily.get(field, 0) + for field in [SMS_DELIVERED_TODAY, SMS_FAILED_TODAY, EMAIL_DELIVERED_TODAY, EMAIL_FAILED_TODAY] } - monthly[0]["sms_counts"]["failed"] += daily_redis["sms_failed"] - monthly[0]["sms_counts"]["requested"] += daily_redis["sms_failed"] + daily_redis["sms_delivered"] - monthly[0]["email_counts"]["failed"] += daily_redis["email_failed"] - monthly[0]["email_counts"]["requested"] += daily_redis["email_failed"] + daily_redis["email_delivered"] + monthly[0]["sms_counts"]["failed"] += daily_redis[SMS_FAILED_TODAY] + monthly[0]["sms_counts"]["requested"] += daily_redis[SMS_FAILED_TODAY] + daily_redis[SMS_DELIVERED_TODAY] + monthly[0]["email_counts"]["failed"] += daily_redis[EMAIL_FAILED_TODAY] + monthly[0]["email_counts"]["requested"] += daily_redis[EMAIL_FAILED_TODAY] + daily_redis[EMAIL_DELIVERED_TODAY] elif mode == "db": monthly[0]["sms_counts"]["failed"] += daily["sms"]["failed"] monthly[0]["sms_counts"]["requested"] += daily["sms"]["requested"] @@ -389,9 +398,8 @@ def aggregate_by_type(data, daily_data): dashboard_totals_weekly = (get_dashboard_totals(stats_weekly),) bounce_rate_data = get_bounce_rate_data_from_redis(service_id) - # get annual data from fact table (all data this year except today) - annual_data = service_api_client.get_monthly_notification_stats(service_id, get_current_financial_year()) - annual_data = aggregate_by_type(annual_data, dashboard_totals_daily[0]) + # get annual data (either from redis or the api) + annual_data = notification_counts_client.get_total_notification_count(current_service) return { "upcoming": render_template("views/dashboard/_upcoming.html", scheduled_jobs=scheduled_jobs), diff --git a/app/notify_client/notification_counts_client.py b/app/notify_client/notification_counts_client.py index 001a980b7a..c69c557311 100644 --- a/app/notify_client/notification_counts_client.py +++ b/app/notify_client/notification_counts_client.py @@ -1,50 +1,44 @@ -from notifications_utils.clients.redis import ( - email_daily_count_cache_key, - sms_daily_count_cache_key, +from notifications_utils.clients.redis.annual_limit import ( + EMAIL_DELIVERED_TODAY, + EMAIL_FAILED_TODAY, + SMS_DELIVERED_TODAY, + SMS_FAILED_TODAY, + TOTAL_EMAIL_FISCAL_YEAR_TO_YESTERDAY, + TOTAL_SMS_FISCAL_YEAR_TO_YESTERDAY, ) -from app import redis_client, service_api_client, template_statistics_client +from app import service_api_client +from app.extensions import annual_limit_client from app.models.service import Service -from app.utils import get_current_financial_year 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_sms = int(todays_sms) if todays_sms is not None else None - - todays_email = redis_client.get(email_daily_count_cache_key(service_id)) - todays_email = int(todays_email) if todays_email is not None else None - - 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): + def get_total_notification_count(self, service: Service): """ - Get total number of notifications by type for the current service for the current year - - Return value: - { - 'sms': int, - 'email': int - } - + Get the total number of notifications sent by a service + Args: + service_id (str): The ID of the service + Returns: + int: The total number of notifications sent by the service """ - 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 + annual_limit_data = {} + if annual_limit_client.was_seeded_today(service.id): + # get data from redis + annual_limit_data = annual_limit_client.get_all_notification_counts(service.id) + else: + # get data from db + annual_limit_data = service_api_client.get_year_to_date_service_statistics(service.id) + + # transform data so dashboard can use it + return { + "sms": annual_limit_data[TOTAL_SMS_FISCAL_YEAR_TO_YESTERDAY] + + annual_limit_data[SMS_FAILED_TODAY] + + annual_limit_data[SMS_DELIVERED_TODAY], + "email": annual_limit_data[TOTAL_EMAIL_FISCAL_YEAR_TO_YESTERDAY] + + annual_limit_data[EMAIL_FAILED_TODAY] + + annual_limit_data[EMAIL_DELIVERED_TODAY], + } def get_limit_stats(self, service: Service): """ @@ -82,34 +76,53 @@ def get_limit_stats(self, service: Service): } """ - current_financial_year = get_current_financial_year() - sent_today = self.get_all_notification_counts_for_today(service.id) - # We are interested in getting data for the financial year, not the calendar year - sent_thisyear = self.get_all_notification_counts_for_year(service.id, current_financial_year) + annual_limit_data = {} + if annual_limit_client.was_seeded_today(service.id): + # get data from redis + annual_limit_data = annual_limit_client.get_all_notification_counts(service.id) + else: + # get data from the api + annual_limit_data = service_api_client.get_year_to_date_service_statistics(service.id) limit_stats = { "email": { "annual": { "limit": service.email_annual_limit, - "sent": sent_thisyear["email"], - "remaining": service.email_annual_limit - sent_thisyear["email"], + "sent": annual_limit_data[TOTAL_EMAIL_FISCAL_YEAR_TO_YESTERDAY] + + annual_limit_data[EMAIL_FAILED_TODAY] + + annual_limit_data[EMAIL_DELIVERED_TODAY], + "remaining": service.email_annual_limit + - ( + annual_limit_data[TOTAL_EMAIL_FISCAL_YEAR_TO_YESTERDAY] + + annual_limit_data[EMAIL_FAILED_TODAY] + + annual_limit_data[EMAIL_DELIVERED_TODAY] + ), }, "daily": { "limit": service.message_limit, - "sent": sent_today["email"], - "remaining": service.message_limit - sent_today["email"], + "sent": annual_limit_data[EMAIL_FAILED_TODAY] + annual_limit_data[EMAIL_DELIVERED_TODAY], + "remaining": service.message_limit + - (annual_limit_data[EMAIL_DELIVERED_TODAY] + annual_limit_data[EMAIL_FAILED_TODAY]), }, }, "sms": { "annual": { "limit": service.sms_annual_limit, - "sent": sent_thisyear["sms"], - "remaining": service.sms_annual_limit - sent_thisyear["sms"], + "sent": annual_limit_data[TOTAL_SMS_FISCAL_YEAR_TO_YESTERDAY] + + annual_limit_data[SMS_FAILED_TODAY] + + annual_limit_data[SMS_DELIVERED_TODAY], + "remaining": service.sms_annual_limit + - ( + annual_limit_data[TOTAL_SMS_FISCAL_YEAR_TO_YESTERDAY] + + annual_limit_data[SMS_FAILED_TODAY] + + annual_limit_data[SMS_DELIVERED_TODAY] + ), }, "daily": { "limit": service.sms_daily_limit, - "sent": sent_today["sms"], - "remaining": service.sms_daily_limit - sent_today["sms"], + "sent": annual_limit_data[SMS_FAILED_TODAY] + annual_limit_data[SMS_DELIVERED_TODAY], + "remaining": service.sms_daily_limit + - (annual_limit_data[SMS_DELIVERED_TODAY] + annual_limit_data[SMS_FAILED_TODAY]), }, }, } @@ -117,34 +130,4 @@ def get_limit_stats(self, service: Service): return limit_stats -# 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/service_api_client.py b/app/notify_client/service_api_client.py index 6eeb7bf181..4feba52ecf 100644 --- a/app/notify_client/service_api_client.py +++ b/app/notify_client/service_api_client.py @@ -59,6 +59,11 @@ def get_service_statistics(self, service_id, today_only, limit_days=None): params={"today_only": today_only, "limit_days": limit_days}, )["data"] + def get_year_to_date_service_statistics(self, service_id): + return self.get( + "/service/{0}/annual-limit-stats".format(service_id), + ) + def get_services(self, params_dict=None): """ Retrieve a list of services. diff --git a/poetry.lock b/poetry.lock index 1cc480eec4..4405abc9ae 100644 --- a/poetry.lock +++ b/poetry.lock @@ -1565,7 +1565,7 @@ requests = ">=2.0.0" [[package]] name = "notifications-utils" -version = "53.1.1" +version = "53.2.0" description = "Shared python code for Notification - Provides logging utils etc." optional = false python-versions = "~3.12.7" @@ -1601,8 +1601,8 @@ werkzeug = "3.0.4" [package.source] type = "git" url = "https://github.com/cds-snc/notifier-utils.git" -reference = "53.1.1" -resolved_reference = "a41ad8ff73042c27236f6500ca4590db2225b86d" +reference = "53.2.0" +resolved_reference = "4f16210a5f0346351cb0a13ad31cc5c796e97ec6" [[package]] name = "openpyxl" @@ -2769,4 +2769,4 @@ testing = ["coverage (>=5.0.3)", "zope.event", "zope.testing"] [metadata] lock-version = "2.0" python-versions = "~3.12.7" -content-hash = "c0e348914122f836acd1d71ecbc9a792d0b1234095a1e3069d664db17f9b1503" +content-hash = "c5c99693d0e04c4c0c660a0b3995a27737f67d71ed477dbc41569443a127e22a" diff --git a/pyproject.toml b/pyproject.toml index 87b1e86677..0ae5e43767 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -41,7 +41,7 @@ gunicorn = "22.0.0" itsdangerous = "2.2.0" # pyup: <1.0.0 newrelic = "10.3.0" notifications-python-client = "6.4.1" -notifications-utils = { git = "https://github.com/cds-snc/notifier-utils.git", tag = "53.1.1"} +notifications-utils = { git = "https://github.com/cds-snc/notifier-utils.git", tag = "53.2.0"} pwnedpasswords = "2.0.0" pyexcel = "0.7.0" pyexcel-io = "0.6.6" diff --git a/tests/app/main/views/test_accept_invite.py b/tests/app/main/views/test_accept_invite.py index 03befcb941..0238f2ae94 100644 --- a/tests/app/main/views/test_accept_invite.py +++ b/tests/app/main/views/test_accept_invite.py @@ -163,6 +163,17 @@ def test_accepting_invite_removes_invite_from_session( mocker.patch("app.invite_api_client.check_token", return_value=sample_invite) mocker.patch("app.billing_api_client.get_billable_units", return_value="") + mocker.patch( + "app.notify_client.notification_counts_client.service_api_client.get_year_to_date_service_statistics", + return_value={ + "sms_delivered_today": 5, + "email_delivered_today": 10, + "sms_failed_today": 5, + "email_failed_today": 10, + "total_sms_fiscal_year_to_yesterday": 90, + "total_email_fiscal_year_to_yesterday": 180, + }, + ) client_request.login(user) @@ -504,6 +515,17 @@ def test_new_invited_user_verifies_and_added_to_service( mocker, ): # visit accept token page + mocker.patch( + "app.notify_client.notification_counts_client.service_api_client.get_year_to_date_service_statistics", + return_value={ + "sms_delivered_today": 5, + "email_delivered_today": 10, + "sms_failed_today": 5, + "email_failed_today": 10, + "total_sms_fiscal_year_to_yesterday": 90, + "total_email_fiscal_year_to_yesterday": 180, + }, + ) response = client.get(url_for("main.accept_invite", token="thisisnotarealtoken")) assert response.status_code == 302 assert response.location == url_for("main.register_from_invite") diff --git a/tests/app/main/views/test_bounce_rate.py b/tests/app/main/views/test_bounce_rate.py index 3465b9ae68..83022b28f9 100644 --- a/tests/app/main/views/test_bounce_rate.py +++ b/tests/app/main/views/test_bounce_rate.py @@ -200,6 +200,7 @@ def test_bounce_rate_widget_displays_correct_status_and_totals_v15( mock_get_service_templates, mock_get_template_statistics, mock_get_service_statistics, + mock_annual_limit_client_stats, mock_get_jobs, service_one, bounce_rate, @@ -232,6 +233,7 @@ def test_bounce_rate_widget_doesnt_change_when_under_threshold_v15( mock_get_service_templates, mock_get_template_statistics, mock_get_service_statistics, + mock_annual_limit_client_stats, mock_get_jobs, service_one, app_, diff --git a/tests/app/main/views/test_dashboard.py b/tests/app/main/views/test_dashboard.py index 3be02e81d6..38a3aaf445 100644 --- a/tests/app/main/views/test_dashboard.py +++ b/tests/app/main/views/test_dashboard.py @@ -6,6 +6,12 @@ from bs4 import BeautifulSoup from flask import url_for from freezegun import freeze_time +from notifications_utils.clients.redis.annual_limit import ( + EMAIL_DELIVERED_TODAY, + EMAIL_FAILED_TODAY, + SMS_DELIVERED_TODAY, + SMS_FAILED_TODAY, +) from app.main.views.dashboard import ( aggregate_notifications_stats, @@ -141,6 +147,7 @@ def test_task_shortcuts_are_visible_based_on_permissions( mock_get_jobs, mock_get_template_statistics, mock_get_service_statistics, + mock_annual_limit_client_stats, permissions: list, text_in_page: list, text_not_in_page: list, @@ -175,6 +182,7 @@ def test_survey_widget_presence( mock_get_jobs, mock_get_template_statistics, mock_get_service_statistics, + mock_annual_limit_client_stats, mocker, admin_url, is_widget_present, @@ -199,6 +207,7 @@ def test_sending_link_has_query_param( mock_get_jobs, mock_get_template_statistics, mock_get_service_statistics, + mock_annual_limit_client_stats, ): active_user_with_permissions["permissions"][SERVICE_ONE_ID] = ["view_activity", "send_messages"] client_request.login(active_user_with_permissions) @@ -217,6 +226,7 @@ def test_no_sending_link_if_no_templates( mock_get_template_statistics, mock_get_service_statistics, mock_get_jobs, + mock_annual_limit_client_stats, ): page = client_request.get("main.service_dashboard", service_id=SERVICE_ONE_ID) @@ -232,6 +242,7 @@ def test_should_show_recent_templates_on_dashboard( mock_get_service_statistics, mock_get_usage, mock_get_inbound_sms_summary, + mock_annual_limit_client_stats, app_, ): mock_template_stats = mocker.patch( @@ -386,6 +397,7 @@ def test_should_show_upcoming_jobs_on_dashboard( mock_get_jobs, mock_get_usage, mock_get_inbound_sms_summary, + mock_annual_limit_client_stats, ): page = client_request.get( "main.service_dashboard", @@ -414,6 +426,7 @@ def test_daily_usage_section_shown( mock_get_template_statistics, mock_get_service_statistics, mock_get_jobs, + mock_annual_limit_client_stats, service_one, app_, ): @@ -458,6 +471,7 @@ def test_correct_font_size_for_big_numbers( mock_get_template_statistics, mock_get_service_statistics, mock_get_jobs, + mock_annual_limit_client_stats, service_one, permissions, totals, @@ -567,6 +581,7 @@ def test_dashboard_single_and_plural_v15( mock_get_template_statistics, mock_get_service_statistics, mock_get_jobs, + mock_annual_limit_client_stats, service_one, permissions, totals, @@ -597,6 +612,7 @@ def test_should_show_recent_jobs_on_dashboard( mock_get_jobs, mock_get_usage, mock_get_inbound_sms_summary, + mock_annual_limit_client_stats, ): page = client_request.get( "main.service_dashboard", @@ -967,6 +983,7 @@ def test_menu_all_services_for_platform_admin_user( mock_get_usage, mock_get_inbound_sms_summary, mock_get_free_sms_fragment_limit, + mock_annual_limit_client_stats, ): with app_.test_request_context(): resp = _test_dashboard_menu(mocker, app_, platform_admin_user, service_one, []) @@ -1006,6 +1023,7 @@ def test_route_for_service_permissions( mock_get_service_statistics, mock_get_usage, mock_get_inbound_sms_summary, + mock_annual_limit_client_stats, ): with app_.test_request_context(): validate_route_permission( @@ -1050,6 +1068,7 @@ def test_service_dashboard_updates_gets_dashboard_totals( mock_get_jobs, mock_get_usage, mock_get_inbound_sms_summary, + mock_annual_limit_client_stats, ): mocker.patch( "app.main.views.dashboard.get_dashboard_totals", @@ -1372,6 +1391,7 @@ def test_dashboard_daily_limits( mock_get_service_templates, mock_get_template_statistics, mock_get_service_statistics, + mock_annual_limit_client_stats, mock_get_jobs, service_one, totals, @@ -1416,6 +1436,7 @@ def test_daily_usage_uses_muted_component( mock_get_jobs, mock_get_service_statistics, mock_get_usage, + mock_annual_limit_client_stats, app_, ): with set_config(app_, "FF_ANNUAL_LIMIT", True): # REMOVE LINE WHEN FF REMOVED @@ -1439,6 +1460,7 @@ def test_annual_usage_uses_muted_component( mock_get_jobs, mock_get_service_statistics, mock_get_usage, + mock_annual_limit_client_stats, app_, ): with set_config(app_, "FF_ANNUAL_LIMIT", True): # REMOVE LINE WHEN FF REMOVED @@ -1459,7 +1481,7 @@ def test_annual_usage_uses_muted_component( "redis_daily_data, monthly_data, expected_data", [ ( - {"sms_delivered": 100, "email_delivered": 50, "sms_failed": 1000, "email_failed": 500}, + {SMS_DELIVERED_TODAY: 100, EMAIL_DELIVERED_TODAY: 50, SMS_FAILED_TODAY: 1000, EMAIL_FAILED_TODAY: 500}, { "data": { "2024-04": {"sms": {}, "email": {}, "letter": {}}, @@ -1483,7 +1505,7 @@ def test_annual_usage_uses_muted_component( {"email": 990, "letter": 0, "sms": 1420}, ), ( - {"sms_delivered": 6, "email_delivered": 6, "sms_failed": 6, "email_failed": 6}, + {SMS_DELIVERED_TODAY: 6, EMAIL_DELIVERED_TODAY: 6, SMS_FAILED_TODAY: 6, EMAIL_FAILED_TODAY: 6}, { "data": { "2024-10": { @@ -1582,7 +1604,7 @@ def test_usage_report_aggregates_calculated_properly_without_redis( # mock annual_limit_client.get_all_notification_counts mocker.patch( "app.main.views.dashboard.annual_limit_client.get_all_notification_counts", - return_value={"sms_delivered": 0, "email_delivered": 0, "sms_failed": 0, "email_failed": 0}, + return_value={SMS_DELIVERED_TODAY: 0, EMAIL_DELIVERED_TODAY: 0, SMS_FAILED_TODAY: 0, EMAIL_FAILED_TODAY: 0}, ) mocker.patch( diff --git a/tests/app/main/views/test_menu.py b/tests/app/main/views/test_menu.py index e1ba562572..d48952a20d 100644 --- a/tests/app/main/views/test_menu.py +++ b/tests/app/main/views/test_menu.py @@ -32,6 +32,7 @@ def test_selected_menus( mock_get_all_letter_branding, mock_get_inbound_number_for_service, mock_get_service_data_retention, + mock_annual_limit_client_stats, single_reply_to_email_address, single_letter_contact_block, mock_get_service_organisation, diff --git a/tests/app/main/views/test_send.py b/tests/app/main/views/test_send.py index d9d704cf7c..027aee6c9c 100644 --- a/tests/app/main/views/test_send.py +++ b/tests/app/main/views/test_send.py @@ -386,6 +386,7 @@ def test_upload_csv_file_with_errors_shows_check_page_with_errors( mock_get_template_statistics, mock_get_job_doesnt_exist, mock_get_jobs, + mock_annual_limit_client_stats, fake_uuid, ): mocker.patch( @@ -501,6 +502,7 @@ def test_upload_csv_file_with_missing_columns_shows_error( mock_get_template_statistics, mock_get_job_doesnt_exist, mock_get_jobs, + mock_annual_limit_client_stats, service_one, fake_uuid, file_contents, @@ -561,6 +563,7 @@ def test_upload_csv_variables_too_long_shows_banner_and_inline_cell_errors( mock_get_template_statistics, mock_get_job_doesnt_exist, mock_get_jobs, + mock_annual_limit_client_stats, service_one, fake_uuid, file_contents, @@ -658,6 +661,7 @@ def test_upload_valid_csv_shows_preview_and_table( mock_get_job_doesnt_exist, mock_get_jobs, mock_s3_set_metadata, + mock_annual_limit_client_stats, fake_uuid, extra_args, expected_recipient, @@ -807,6 +811,7 @@ def test_file_name_truncated_to_fit_in_s3_metadata( mock_get_job_doesnt_exist, mock_get_jobs, mock_s3_set_metadata, + mock_annual_limit_client_stats, fake_uuid, ): with client_request.session_transaction() as session: @@ -852,6 +857,7 @@ def test_check_messages_replaces_invalid_characters_in_file_name( mock_get_job_doesnt_exist, mock_get_jobs, mock_s3_set_metadata, + mock_annual_limit_client_stats, fake_uuid, ): with client_request.session_transaction() as session: @@ -895,6 +901,7 @@ def test_show_all_columns_if_there_are_duplicate_recipient_columns( mock_get_template_statistics, mock_get_job_doesnt_exist, mock_get_jobs, + mock_annual_limit_client_stats, fake_uuid, ): with client_request.session_transaction() as session: @@ -942,6 +949,7 @@ def test_404_for_previewing_a_row_out_of_range( mock_get_job_doesnt_exist, mock_get_jobs, mock_s3_set_metadata, + mock_annual_limit_client_stats, fake_uuid, row_index, expected_status, @@ -1232,6 +1240,7 @@ def test_send_one_off_or_test_shows_placeholders_in_correct_order( fake_uuid, mock_has_no_jobs, mock_get_service_template_with_multiple_placeholders, + mock_annual_limit_client_stats, endpoint, step_index, prefilled, @@ -2082,6 +2091,7 @@ def test_upload_csvfile_with_valid_phone_shows_all_numbers( mock_get_job_doesnt_exist, mock_get_jobs, mock_s3_set_metadata, + mock_annual_limit_client_stats, service_one, fake_uuid, mock_s3_upload, @@ -2138,6 +2148,7 @@ def test_upload_csvfile_with_international_validates( mock_get_template_statistics, mock_get_job_doesnt_exist, mock_get_jobs, + mock_annual_limit_client_stats, fake_uuid, international_sms_permission, should_allow_international, @@ -2176,6 +2187,7 @@ def test_test_message_can_only_be_sent_now( mock_get_job_doesnt_exist, mock_get_jobs, mock_s3_set_metadata, + mock_annual_limit_client_stats, fake_uuid, ): content = client_request.get( @@ -2485,6 +2497,7 @@ def test_check_messages_back_link( mock_get_jobs, mock_s3_download, mock_s3_set_metadata, + mock_annual_limit_client_stats, fake_uuid, mocker, template_type, @@ -2681,6 +2694,7 @@ def test_check_messages_shows_trial_mode_error( mock_get_template_statistics, mock_get_job_doesnt_exist, mock_get_jobs, + mock_annual_limit_client_stats, fake_uuid, mocker, ): @@ -2729,6 +2743,7 @@ def test_warns_if_file_sent_already( mock_get_template_statistics, mock_get_job_doesnt_exist, mock_get_jobs, + mock_annual_limit_client_stats, fake_uuid, mocker, uploaded_file_name, @@ -2762,6 +2777,7 @@ def test_check_messages_adds_sender_id_in_session_to_metadata( mock_get_job_doesnt_exist, mock_get_jobs, mock_s3_set_metadata, + mock_annual_limit_client_stats, fake_uuid, ): mocker.patch("app.main.views.send.s3download", return_value=("phone number,\n+16502532222")) @@ -2801,6 +2817,7 @@ def test_check_messages_shows_over_max_row_error( mock_get_job_doesnt_exist, mock_get_jobs, mock_s3_download, + mock_annual_limit_client_stats, fake_uuid, mocker, ): @@ -3153,6 +3170,7 @@ def test_reply_to_is_previewed_if_chosen( mock_get_template_statistics, mock_get_job_doesnt_exist, mock_get_jobs, + mock_annual_limit_client_stats, get_default_reply_to_email_address, fake_uuid, endpoint, @@ -3241,6 +3259,7 @@ def test_sms_sender_is_previewed( mock_get_template_statistics, mock_get_job_doesnt_exist, mock_get_jobs, + mock_annual_limit_client_stats, get_default_sms_sender, fake_uuid, endpoint, diff --git a/tests/app/main/views/test_sign_out.py b/tests/app/main/views/test_sign_out.py index 736bef8c06..cff1d4aca9 100644 --- a/tests/app/main/views/test_sign_out.py +++ b/tests/app/main/views/test_sign_out.py @@ -23,6 +23,7 @@ def test_sign_out_user( mock_get_service_statistics, mock_get_usage, mock_get_inbound_sms_summary, + mock_annual_limit_client_stats, ): with client_request.session_transaction() as session: assert session.get("user_id") is not None diff --git a/tests/app/notify_client/test_notification_counts_client.py b/tests/app/notify_client/test_notification_counts_client.py index da938daa4c..bc4a3dff63 100644 --- a/tests/app/notify_client/test_notification_counts_client.py +++ b/tests/app/notify_client/test_notification_counts_client.py @@ -1,116 +1,29 @@ -from unittest.mock import Mock, patch - -import pytest +from unittest.mock import Mock from app.notify_client.notification_counts_client import NotificationCounts -from app.utils import get_current_financial_year - - -@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 - - -@pytest.fixture -def mock_get_all_notification_counts_for_today(): - with patch("app.notify_client.notification_counts_client.get_all_notification_counts_for_today") 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 - ([25, 25], {"sms": 25, "email": 25}), # 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 == expected_result - - if None in redis_side_effect: - 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 - - def test_get_limit_stats(self, mocker): + def test_get_limit_stats_redis_on(self, mocker): # Setup mock_service = Mock(id="service-1", email_annual_limit=1000, sms_annual_limit=500, message_limit=100, sms_daily_limit=50) mock_notification_client = NotificationCounts() - # Mock the dependency methods - - mocker.patch.object( - mock_notification_client, "get_all_notification_counts_for_today", return_value={"email": 20, "sms": 10} - ) - mocker.patch.object( - mock_notification_client, "get_all_notification_counts_for_year", return_value={"email": 200, "sms": 100} + # Mock was_seeded_today to return True so we use Redis + mocker.patch("app.notify_client.notification_counts_client.annual_limit_client.was_seeded_today", return_value=True) + + # Mock get_all_notification_counts + mocker.patch( + "app.notify_client.notification_counts_client.annual_limit_client.get_all_notification_counts", + return_value={ + "sms_delivered_today": 5, + "email_delivered_today": 10, + "sms_failed_today": 5, + "email_failed_today": 10, + "total_sms_fiscal_year_to_yesterday": 90, + "total_email_fiscal_year_to_yesterday": 180, + }, ) # Execute @@ -144,62 +57,55 @@ def test_get_limit_stats(self, mocker): }, } - @pytest.mark.parametrize( - "today_counts,year_counts,expected_remaining", - [ - ( - {"email": 0, "sms": 0}, - {"email": 0, "sms": 0}, - {"email": {"annual": 1000, "daily": 100}, "sms": {"annual": 500, "daily": 50}}, - ), - ( - {"email": 100, "sms": 50}, - {"email": 1000, "sms": 500}, - {"email": {"annual": 0, "daily": 0}, "sms": {"annual": 0, "daily": 0}}, - ), - ( - {"email": 50, "sms": 25}, - {"email": 500, "sms": 250}, - {"email": {"annual": 500, "daily": 50}, "sms": {"annual": 250, "daily": 25}}, - ), - ], - ) - def test_get_limit_stats_remaining_calculations(self, mocker, today_counts, year_counts, expected_remaining): + def test_get_limit_stats_redis_off(self, mocker): # Setup mock_service = Mock(id="service-1", email_annual_limit=1000, sms_annual_limit=500, message_limit=100, sms_daily_limit=50) mock_notification_client = NotificationCounts() - mocker.patch.object(mock_notification_client, "get_all_notification_counts_for_today", return_value=today_counts) - mocker.patch.object(mock_notification_client, "get_all_notification_counts_for_year", return_value=year_counts) - - # Execute - result = mock_notification_client.get_limit_stats(mock_service) - - # Assert remaining counts - assert result["email"]["annual"]["remaining"] == expected_remaining["email"]["annual"] - assert result["email"]["daily"]["remaining"] == expected_remaining["email"]["daily"] - assert result["sms"]["annual"]["remaining"] == expected_remaining["sms"]["annual"] - assert result["sms"]["daily"]["remaining"] == expected_remaining["sms"]["daily"] - - def test_get_limit_stats_dependencies_called(self, mocker): - # Setup - mock_service = Mock(id="service-1", email_annual_limit=1000, sms_annual_limit=500, message_limit=100, sms_daily_limit=50) - mock_notification_client = NotificationCounts() - - mock_today = mocker.patch.object( - mock_notification_client, "get_all_notification_counts_for_today", return_value={"email": 0, "sms": 0} - ) - mock_year = mocker.patch.object( - mock_notification_client, "get_all_notification_counts_for_year", return_value={"email": 0, "sms": 0} + # Mock was_seeded_today to return True so we use Redis + mocker.patch("app.notify_client.notification_counts_client.annual_limit_client.was_seeded_today", return_value=False) + + # Mock get_all_notification_counts + mocker.patch( + "app.notify_client.notification_counts_client.service_api_client.get_year_to_date_service_statistics", + return_value={ + "sms_delivered_today": 5, + "email_delivered_today": 10, + "sms_failed_today": 5, + "email_failed_today": 10, + "total_sms_fiscal_year_to_yesterday": 90, + "total_email_fiscal_year_to_yesterday": 180, + }, ) # Execute - mock_notification_client.get_limit_stats(mock_service) + result = mock_notification_client.get_limit_stats(mock_service) - # Assert dependencies called - mock_today.assert_called_once_with(mock_service.id) - mock_year.assert_called_once_with( - mock_service.id, - get_current_financial_year(), - ) + # Assert + assert result == { + "email": { + "annual": { + "limit": 1000, + "sent": 200, + "remaining": 800, + }, + "daily": { + "limit": 100, + "sent": 20, + "remaining": 80, + }, + }, + "sms": { + "annual": { + "limit": 500, + "sent": 100, + "remaining": 400, + }, + "daily": { + "limit": 50, + "sent": 10, + "remaining": 40, + }, + }, + } diff --git a/tests/conftest.py b/tests/conftest.py index a491717587..5588af6fd7 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -2970,6 +2970,50 @@ def _get(): return mocker.patch("app.template_category_api_client.get_all_template_categories", side_effect=_get) +@pytest.fixture(scope="function") +def mock_annual_limit_client_stats( + mocker, + sms_delivered_today=None, + email_delivered_today=None, + sms_failed_today=None, + email_failed_today=None, + total_sms_fiscal_year_to_yesterday=None, + total_email_fiscal_year_to_yesterday=None, +): + def _get_stats( + sms_delivered_today, + email_delivered_today, + sms_failed_today, + email_failed_today, + total_sms_fiscal_year_to_yesterday, + total_email_fiscal_year_to_yesterday, + ): + return { + "sms_delivered_today": 5 if sms_delivered_today is None else sms_delivered_today, + "email_delivered_today": 10 if email_delivered_today is None else email_delivered_today, + "sms_failed_today": 5 if sms_failed_today is None else sms_failed_today, + "email_failed_today": 10 if email_failed_today is None else email_failed_today, + "total_sms_fiscal_year_to_yesterday": 90 + if total_sms_fiscal_year_to_yesterday is None + else total_sms_fiscal_year_to_yesterday, + "total_email_fiscal_year_to_yesterday": 180 + if total_email_fiscal_year_to_yesterday is None + else total_email_fiscal_year_to_yesterday, + } + + mocker.patch( + "app.notify_client.notification_counts_client.service_api_client.get_year_to_date_service_statistics", + return_value=_get_stats( + sms_delivered_today, + email_delivered_today, + sms_failed_today, + email_failed_today, + total_sms_fiscal_year_to_yesterday, + total_email_fiscal_year_to_yesterday, + ), + ) + + @pytest.fixture(scope="function") def mock_get_template_statistics(mocker, service_one, fake_uuid): template = template_json(