Merge branch 'main' into feat/scaffold-ui-test-user-on-demand

.devcontainer/devcontainer.json

@@ -13,10 +13,14 @@
 				"[python]": {
 					"editor.formatOnSave": true
 				},
+				"python.formatting.blackPath": "/usr/local/bin/black",
+				"python.linting.enabled": true,
+				"python.linting.pylintEnabled": true,
+				"python.linting.pylintPath": "/usr/local/bin/pylint",
 				"python.pythonPath": "/usr/local/bin/python"
 			},
 			"extensions": [
-				"tamasfe.even-better-toml",
+				"bungcip.better-toml",
 				"donjayamanne.python-extension-pack",
 				"eamodio.gitlens",
 				"GitHub.copilot",
@@ -37,8 +41,7 @@
 				"visualstudioexptteam.vscodeintellicode",
 				"wenfangdu.jump",
 				"wholroyd.jinja",
-				"yzhang.markdown-all-in-one",
-				"charliermarsh.ruff"
+				"yzhang.markdown-all-in-one"
 			]
 		}
 	},
@@ -58,4 +61,5 @@
 	},
 	"postCreateCommand": "notify-dev-entrypoint.sh",
 	"remoteUser": "vscode",
+
 }

.devcontainer/scripts/notify-dev-entrypoint.sh

@@ -35,7 +35,7 @@ cd /workspace
 echo -e "fpath+=/.zfunc" >> ~/.zshrc
 echo -e "autoload -Uz compinit && compinit"
 
-pip install poetry==${POETRY_VERSION} poetry-plugin-sort
+pip install poetry==${POETRY_VERSION}
 export PATH=$PATH:/home/vscode/.local/bin/
 which poetry
 poetry --version

.github/workflows/backstage-catalog-helper.yml

@@ -25,9 +25,10 @@ jobs:
           app_id: ${{ secrets.SRE_BOT_RW_APP_ID }}
           private_key: ${{ secrets.SRE_BOT_RW_PRIVATE_KEY }}
       - name: Create pull request
-        uses: peter-evans/create-pull-request@v3
+        uses: peter-evans/create-pull-request@6cd32fd93684475c31847837f87bb135d40a2b79 # v7.0.3
         with:
           token: ${{ steps.generate_token.outputs.token}}
+          sign-commits: true
           commit-message: 'Add catalog-info.yaml'
           branch: 'backstage/catalog-info'
           title: 'Add catalog-info.yaml'

.github/workflows/test.yaml

@@ -34,7 +34,7 @@ jobs:
     - name: Install poetry
       env:
         POETRY_VERSION: "1.7.1"
-      run: pip install poetry==${POETRY_VERSION} poetry-plugin-sort && poetry --version
+      run: pip install poetry==${POETRY_VERSION} && poetry --version
     - name: Check poetry.lock aligns with pyproject.toml
       run: poetry check --lock
     - name: Install requirements

.gitignore

@@ -91,5 +91,3 @@ jinja_templates/
 cypress.env.json
 node_modules/
 tests_cypress/cypress/videos/
-
-.ruff_cache/

Makefile

@@ -37,9 +37,9 @@ clean:
 
 .PHONY: format
 format:
-	ruff check --select I --fix .
-	ruff check
-	ruff format .
+	poetry run isort .
+	poetry run black --config pyproject.toml .
+	poetry run flake8 .
 	poetry run mypy .
 
 .PHONY: smoke-test

app/celery/reporting_tasks.py

@@ -1,4 +1,5 @@
-from datetime import datetime, timedelta
+from datetime import datetime, timedelta, timezone
+from itertools import islice
 
 from flask import current_app
 from notifications_utils.statsd_decorators import statsd
@@ -12,6 +13,7 @@
     fetch_notification_status_for_day,
     update_fact_notification_status,
 )
+from app.models import Service
 
 
 @notify_celery.task(name="create-nightly-billing")
@@ -72,19 +74,38 @@ def create_nightly_notification_status(day_start=None):
 @notify_celery.task(name="create-nightly-notification-status-for-day")
 @statsd(namespace="tasks")
 def create_nightly_notification_status_for_day(process_day):
-    process_day = datetime.strptime(process_day, "%Y-%m-%d").date()
-
-    start = datetime.utcnow()
-    transit_data = fetch_notification_status_for_day(process_day=process_day)
-    end = datetime.utcnow()
-    current_app.logger.info(
-        "create-nightly-notification-status-for-day {} fetched in {} seconds".format(process_day, (end - start).seconds)
-    )
+    """
+    This function gets all the service ids and fetches the notification status for the given day.
+    It does it in chunks of 20 service ids at a time.
 
-    update_fact_notification_status(transit_data, process_day)
+    Args:
+        process_day (_type_): datetime object
+    """
+    process_day = datetime.strptime(process_day, "%Y-%m-%d").date()
+    service_ids = [x.id for x in Service.query.all()]
+    chunk_size = 20
+    iter_service_ids = iter(service_ids)
+
+    while True:
+        chunk = list(islice(iter_service_ids, chunk_size))
+
+        if not chunk:
+            current_app.logger.info(
+                "create-nightly-notification-status-for-day job completed for process_day {} on {}".format(
+                    process_day, datetime.now(timezone.utc).date()
+                )
+            )
+            break
+        start = datetime.now(timezone.utc)
+        transit_data = fetch_notification_status_for_day(process_day=process_day, service_ids=chunk)
+        end = datetime.now(timezone.utc)
+        current_app.logger.info(
+            "create-nightly-notification-status-for-day {} fetched in {} seconds".format(process_day, (end - start).seconds)
+        )
+        update_fact_notification_status(transit_data, process_day, service_ids=chunk)
 
-    current_app.logger.info(
-        "create-nightly-notification-status-for-day task complete: {} rows updated for day: {}".format(
-            len(transit_data), process_day
+        current_app.logger.info(
+            "create-nightly-notification-status-for-day task complete: {} rows updated for day: {}, for service_ids: {}".format(
+                len(transit_data), process_day, chunk
+            )
         )
-    )

app/celery/scheduled_tasks.py

@@ -215,9 +215,9 @@ def replay_created_notifications():
 
         if len(notifications_to_resend) > 0:
             current_app.logger.info(
-                "Sending {} {} notifications " "to the delivery queue because the notification " "status was created.".format(
-                    len(notifications_to_resend), notification_type
-                )
+                "Sending {} {} notifications "
+                "to the delivery queue because the notification "
+                "status was created.".format(len(notifications_to_resend), notification_type)
             )
 
         for n in notifications_to_resend:

app/celery/tasks.py

@@ -613,7 +613,9 @@ def send_inbound_sms_to_service(self, inbound_sms_id, service_id):
             except self.MaxRetriesExceededError:
                 current_app.logger.error(
                     """Retry: send_inbound_sms_to_service has retried the max number of
-                     times for service: {} and  inbound_sms {}""".format(service_id, inbound_sms_id)
+                     times for service: {} and  inbound_sms {}""".format(
+                        service_id, inbound_sms_id
+                    )
                 )
 
 

app/config.py

@@ -499,6 +499,7 @@ class Config(object):
     }
     CELERY_QUEUES: List[Any] = []
     CELERY_DELIVER_SMS_RATE_LIMIT = os.getenv("CELERY_DELIVER_SMS_RATE_LIMIT", "1/s")
+    AWS_SEND_SMS_BOTO_CALL_LATENCY = os.getenv("AWS_SEND_SMS_BOTO_CALL_LATENCY", 0.06)  # average delay in production
 
     CONTACT_FORM_EMAIL_ADDRESS = os.getenv("CONTACT_FORM_EMAIL_ADDRESS", "[email protected]")
 

app/dao/date_util.py

@@ -1,5 +1,5 @@
 import os
-from datetime import date, datetime, time, timedelta
+from datetime import date, datetime, time, timedelta, timezone
 
 import pytz
 from notifications_utils.strftime_codes import no_pad_month
@@ -104,3 +104,10 @@ def utc_midnight_n_days_ago(number_of_days):
     Returns utc midnight a number of days ago.
     """
     return get_midnight(datetime.utcnow() - timedelta(days=number_of_days))
+
+
+def get_query_date_based_on_retention_period(retention_period):
+    """
+    Computes a date to be used when querying for notifications based on retention period
+    """
+    return datetime.combine(datetime.now(timezone.utc) - timedelta(days=retention_period), time.max)

app/dao/fact_notification_status_dao.py

@@ -1,4 +1,4 @@
-from datetime import datetime, time, timedelta
+from datetime import datetime, time, timedelta, timezone
 
 from flask import current_app
 from sqlalchemy import Date, case, func
@@ -7,7 +7,7 @@
 from sqlalchemy.types import DateTime, Integer
 
 from app import db
-from app.dao.date_util import tz_aware_midnight_n_days_ago, utc_midnight_n_days_ago
+from app.dao.date_util import get_query_date_based_on_retention_period
 from app.models import (
     EMAIL_TYPE,
     KEY_TYPE_NORMAL,
@@ -37,15 +37,15 @@
 )
 
 
-def fetch_notification_status_for_day(process_day, service_id=None):
+def fetch_notification_status_for_day(process_day, service_ids=None):
     start_date = datetime.combine(process_day, time.min)
     end_date = datetime.combine(process_day + timedelta(days=1), time.min)
     # use notification_history if process day is older than 7 days
     # this is useful if we need to rebuild the ft_billing table for a date older than 7 days ago.
     current_app.logger.info("Fetch ft_notification_status for {} to {}".format(start_date, end_date))
 
     all_data_for_process_day = []
-    service_ids = [x.id for x in Service.query.all()]
+    service_ids = service_ids if service_ids else [x.id for x in Service.query.all()]
     # for each service
     # for each notification type
     # query notifications for day
@@ -104,9 +104,14 @@ def query_for_fact_status_data(table, start_date, end_date, notification_type, s
     return query.all()
 
 
-def update_fact_notification_status(data, process_day):
+def update_fact_notification_status(data, process_day, service_ids=None):
     table = FactNotificationStatus.__table__
-    FactNotificationStatus.query.filter(FactNotificationStatus.bst_date == process_day).delete()
+    if service_ids:
+        FactNotificationStatus.query.filter(
+            FactNotificationStatus.bst_date == process_day, FactNotificationStatus.service_id.in_(service_ids)
+        ).delete()
+    else:
+        FactNotificationStatus.query.filter(FactNotificationStatus.bst_date == process_day).delete()
 
     for row in data:
         stmt = insert(table).values(
@@ -241,32 +246,41 @@ def fetch_notification_status_for_service_for_day(bst_day, service_id):
     )
 
 
-def fetch_notification_status_for_service_for_today_and_7_previous_days(service_id, by_template=False, limit_days=7):
-    if limit_days == 1:
-        ft_start_date = utc_midnight_n_days_ago(limit_days - 1)
-        # For daily stats, service limits reset at 12:00am UTC each night, so we need to fetch the data from 12:00 UTC to now
-        start = utc_midnight_n_days_ago(0)
-        end = datetime.utcnow()
-    else:
-        ft_start_date = utc_midnight_n_days_ago(limit_days)
-
-        # The nightly task that populates ft_notification_status counts collects notifications from
-        # 5AM the day before to 5AM of the current day. So we need to match that timeframe when
-        # we fetch notifications for the current day.
-        start = (tz_aware_midnight_n_days_ago(1) + timedelta(hours=5)).replace(minute=0, second=0, microsecond=0)
-        end = (tz_aware_midnight_n_days_ago(0) + timedelta(hours=5)).replace(minute=0, second=0, microsecond=0)
+def _stats_for_days_facts(service_id, start_time, by_template=False, notification_type=None):
+    """
+    We want to take the data from the fact_notification_status table for bst_data>=start_date
 
-    stats_for_7_days = db.session.query(
+    Returns:
+        Aggregate data in a certain format for total notifications
+    """
+    stats_from_facts = db.session.query(
         FactNotificationStatus.notification_type.label("notification_type"),
         FactNotificationStatus.notification_status.label("status"),
         *([FactNotificationStatus.template_id.label("template_id")] if by_template else []),
         *([FactNotificationStatus.notification_count.label("count")]),
     ).filter(
         FactNotificationStatus.service_id == service_id,
-        FactNotificationStatus.bst_date >= ft_start_date,
+        FactNotificationStatus.bst_date >= start_time,
         FactNotificationStatus.key_type != KEY_TYPE_TEST,
     )
 
+    if notification_type:
+        stats_from_facts = stats_from_facts.filter(FactNotificationStatus.notification_type == notification_type)
+
+    return stats_from_facts
+
+
+def _timing_notification_table(service_id):
+    max_date_from_facts = (
+        FactNotificationStatus.query.with_entities(func.max(FactNotificationStatus.bst_date))
+        .filter(FactNotificationStatus.service_id == service_id)
+        .scalar()
+    )
+    date_to_use = max_date_from_facts + timedelta(days=1) if max_date_from_facts else datetime.now(timezone.utc)
+    return datetime.combine(date_to_use, time.min)
+
+
+def _stats_for_today(service_id, start_time, by_template=False, notification_type=None):
     stats_for_today = (
         db.session.query(
             Notification.notification_type.cast(db.Text),
@@ -275,8 +289,7 @@ def fetch_notification_status_for_service_for_today_and_7_previous_days(service_
             *([func.count().label("count")]),
         )
         .filter(
-            Notification.created_at >= start,
-            Notification.created_at <= end,
+            Notification.created_at >= start_time,
             Notification.service_id == service_id,
             Notification.key_type != KEY_TYPE_TEST,
         )
@@ -286,8 +299,41 @@ def fetch_notification_status_for_service_for_today_and_7_previous_days(service_
             Notification.status,
         )
     )
+    if notification_type:
+        stats_for_today = stats_for_today.filter(Notification.notification_type == notification_type)
+
+    return stats_for_today
+
+
+def fetch_notification_status_for_service_for_today_and_7_previous_days(
+    service_id, by_template=False, limit_days=7, notification_type=None
+):
+    """
+    We want to take the data from the fact_notification_status table and the notifications table and combine them
+    We will take the data from notifications ONLY for today and the fact_notification_status for the last 6 days.
+    In total we will have 7 days worth of data.
+
+    As the data in facts is populated by a job, instead of
+    keeping track of the job - we will find the max date in the facts table and then use that date to get the
+    data from the notifications table.
+
+    Args:
+        service_id (uuid): service_id
+        by_template (bool, optional): aggregate by template Defaults to False.
+        limit_days (int, optional): Number of days we want to get data for - it can depend on sensitive services.
+            Defaults to 7.
+        notification_type (str, optional): notification type. Defaults to None which means all notification types.
+
+    Returns:
+        Aggregate data in a certain format for total notifications
+    """
+    facts_notification_start_time = get_query_date_based_on_retention_period(limit_days)
+    stats_from_facts = _stats_for_days_facts(service_id, facts_notification_start_time, by_template, notification_type)
+
+    start_time_notify_table = _timing_notification_table(service_id)
+    stats_for_today = _stats_for_today(service_id, start_time_notify_table, by_template, notification_type)
 
-    all_stats_table = stats_for_7_days.union_all(stats_for_today).subquery()
+    all_stats_table = stats_from_facts.union_all(stats_for_today).subquery()
 
     query = db.session.query(
         *(

app/dao/jobs_dao.py

@@ -12,6 +12,7 @@
 
 from app import db
 from app.dao.dao_utils import transactional
+from app.dao.date_util import get_query_date_based_on_retention_period
 from app.dao.templates_dao import dao_get_template_by_id
 from app.models import (
     JOB_STATUS_CANCELLED,
@@ -28,7 +29,6 @@
     ServiceDataRetention,
     Template,
 )
-from app.utils import midnight_n_days_ago
 
 
 @statsd(namespace="dao")
@@ -58,7 +58,8 @@ def dao_get_jobs_by_service_id(service_id, limit_days=None, page=1, page_size=50
         Job.original_file_name != current_app.config["ONE_OFF_MESSAGE_FILENAME"],
     ]
     if limit_days is not None:
-        query_filter.append(Job.created_at >= midnight_n_days_ago(limit_days))
+        query_filter.append(Job.created_at > get_query_date_based_on_retention_period(limit_days))
+
     if statuses is not None and statuses != [""]:
         query_filter.append(Job.job_status.in_(statuses))
     return (