Revert "Fix/use latest redis client (#2088)" (#2095)

This reverts commit c717ef4.

app/main/views/dashboard.py

@@ -15,12 +15,6 @@
 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 (
@@ -35,7 +29,6 @@
 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,
@@ -240,11 +233,10 @@ 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_TODAY, SMS_FAILED_TODAY, EMAIL_DELIVERED_TODAY, EMAIL_FAILED_TODAY]
+                field: daily.get(field, 0) for field in ["sms_delivered", "sms_failed", "email_delivered", "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]
+            annual["sms"] += daily_redis["sms_delivered"] + daily_redis["sms_failed"]
+            annual["email"] += daily_redis["email_delivered"] + daily_redis["email_failed"]
         elif mode == "db":
             annual["sms"] += daily["sms"]["requested"]
             annual["email"] += daily["email"]["requested"]
@@ -255,14 +247,13 @@ 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_TODAY, SMS_FAILED_TODAY, EMAIL_DELIVERED_TODAY, EMAIL_FAILED_TODAY]
+                field: daily.get(field, 0) for field in ["sms_delivered", "sms_failed", "email_delivered", "email_failed"]
             }
 
-            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]
+            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"]
         elif mode == "db":
             monthly[0]["sms_counts"]["failed"] += daily["sms"]["failed"]
             monthly[0]["sms_counts"]["requested"] += daily["sms"]["requested"]
@@ -398,8 +389,9 @@ 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 (either from redis or the api)
-    annual_data = notification_counts_client.get_total_notification_count(current_service)
+    # 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])
 
     return {
         "upcoming": render_template("views/dashboard/_upcoming.html", scheduled_jobs=scheduled_jobs),

app/notify_client/notification_counts_client.py

@@ -1,45 +1,51 @@
-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 notifications_utils.clients.redis import (
+    email_daily_count_cache_key,
+    sms_daily_count_cache_key,
 )
 
-from app import service_api_client
-from app.extensions import annual_limit_client
+from app import redis_client, service_api_client, template_statistics_client
 from app.models.service import Service
+from app.utils import get_current_financial_year
 
 
 class NotificationCounts:
-    def get_total_notification_count(self, service: Service):
-        """
-        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
-        """
+    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
 
-        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)
+        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:
-            # 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],
+            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
+
     def get_limit_stats(self, service: Service):
         """
         Get the limit stats for the current service, by notification type, including:
@@ -76,58 +82,69 @@ def get_limit_stats(self, service: Service):
                 }
         """
 
-        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)
+        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)
 
         limit_stats = {
             "email": {
                 "annual": {
                     "limit": service.email_annual_limit,
-                    "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]
-                    ),
+                    "sent": sent_thisyear["email"],
+                    "remaining": service.email_annual_limit - sent_thisyear["email"],
                 },
                 "daily": {
                     "limit": service.message_limit,
-                    "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]),
+                    "sent": sent_today["email"],
+                    "remaining": service.message_limit - sent_today["email"],
                 },
             },
             "sms": {
                 "annual": {
                     "limit": service.sms_annual_limit,
-                    "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]
-                    ),
+                    "sent": sent_thisyear["sms"],
+                    "remaining": service.sms_annual_limit - sent_thisyear["sms"],
                 },
                 "daily": {
                     "limit": service.sms_daily_limit,
-                    "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]),
+                    "sent": sent_today["sms"],
+                    "remaining": service.sms_daily_limit - sent_today["sms"],
                 },
             },
         }
 
         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,11 +59,6 @@ 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.2.0"}
+notifications-utils = { git = "https://github.com/cds-snc/notifier-utils.git", tag = "53.1.1"}
 pwnedpasswords = "2.0.0"
 pyexcel = "0.7.0"
 pyexcel-io = "0.6.6"

tests/app/main/views/test_accept_invite.py

@@ -163,17 +163,6 @@ 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)
 
@@ -515,17 +504,6 @@ 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,7 +200,6 @@ 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,
@@ -233,7 +232,6 @@ 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_,