Fix/use latest redis client (#2088)

* update(notification_counts_client): use new annual limits redis client + api fallback

* fix(dashboard): use notification_counts_client for annual stats

* feat(service_api_client): add new route for annual stats

* chore: bump utils temporarily until merged

* chore: fix poetry lock

* chore: fix initialization

* update(notification_counts_client): use new annual limits redis client + api fallback

* fix(dashboard): use notification_counts_client for annual stats

* feat(service_api_client): add new route for annual stats

* chore: bump utils temporarily until merged

* chore: fix poetry lock

* chore: fix initialization

* fix test

* annual limit client

* test add

* fix

* fix

* fix

* fix

* chore: update cypress-io/github-action to 5.8.4 to try to eliminate intermittent errors

* Update app/notify_client/service_api_client.py

* chore: fix comment

* chore: use keys instead of strings

* chore: bump utils

* fix(dashboard): update aggregate function to use new redis client keys

* chore: update tests to use new v2 keys

---------

Co-authored-by: Jumana Bahrainwala <[email protected]>

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),

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,69 +76,58 @@ 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]),
                 },
             },
         }
 
         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()

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.

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"

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")

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_,