diff --git a/.ds.baseline b/.ds.baseline index ad8d6eb1e..19c85f3c6 100644 --- a/.ds.baseline +++ b/.ds.baseline @@ -239,7 +239,7 @@ "filename": "tests/app/dao/test_services_dao.py", "hashed_secret": "5baa61e4c9b93f3f0682250b6cf8331b7ee68fd8", "is_verified": false, - "line_number": 289, + "line_number": 290, "is_secret": false } ], @@ -384,5 +384,5 @@ } ] }, - "generated_at": "2025-01-28T20:24:49Z" + "generated_at": "2025-02-06T14:48:51Z" } diff --git a/Makefile b/Makefile index 741ceae5b..7d899c80a 100644 --- a/Makefile +++ b/Makefile @@ -122,7 +122,10 @@ static-scan: clean: rm -rf node_modules cache target venv .coverage build tests/.cache ${CF_MANIFEST_PATH} - +.PHONY: test-single +test-single: export NEW_RELIC_ENVIRONMENT=test +test-single: ## Run a single test file + poetry run pytest $(TEST_FILE) ## DEPLOYMENT # .PHONY: cf-deploy-failwhale diff --git a/app/dao/date_util.py b/app/dao/date_util.py index d93986b45..383ccfa97 100644 --- a/app/dao/date_util.py +++ b/app/dao/date_util.py @@ -1,6 +1,8 @@ import calendar from datetime import date, datetime, time, timedelta +import pytz + from app.utils import utc_now @@ -93,3 +95,24 @@ def generate_date_range(start_date, end_date=None, days=0): current_date += timedelta(days=1) else: return "An end_date or number of days must be specified" + + +def build_local_and_utc_date_range( + start_date_str: str, + days: int = 7, + timezone: str = "UTC" +): + """ + Convert date to local range based on timezone + """ + + user_timezone = pytz.timezone(timezone) + local_end_date = datetime.strptime(start_date_str, "%Y-%m-%d").replace(tzinfo=user_timezone) + # Subtract (days - 1) so the entire final day is included in the range + local_start_date = local_end_date - timedelta(days=days) + + # Convert to UTC for database queries + utc_start_date = local_start_date.astimezone(pytz.utc).replace(hour=0, minute=0, second=0) + utc_end_date = local_end_date.astimezone(pytz.utc).replace(hour=23, minute=59, second=59) + + return (local_start_date, utc_start_date, utc_end_date) diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index 47be682ce..6ff1f1e76 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -1,6 +1,7 @@ import uuid -from datetime import timedelta +from datetime import datetime, timedelta +import pytz from flask import current_app from sqlalchemy import Float, cast, delete, select from sqlalchemy.orm import joinedload @@ -502,7 +503,7 @@ def dao_fetch_stats_for_service_from_days(service_id, start_date, end_date): select( NotificationAllTimeView.notification_type, NotificationAllTimeView.status, - func.date_trunc("day", NotificationAllTimeView.created_at).label("day"), + NotificationAllTimeView.created_at.label("timestamp"), func.count(NotificationAllTimeView.id).label("count"), ) .where( @@ -514,7 +515,7 @@ def dao_fetch_stats_for_service_from_days(service_id, start_date, end_date): .group_by( NotificationAllTimeView.notification_type, NotificationAllTimeView.status, - func.date_trunc("day", NotificationAllTimeView.created_at), + NotificationAllTimeView.created_at, ) ) @@ -563,7 +564,7 @@ def dao_fetch_stats_for_service_from_days_for_user( select( NotificationAllTimeView.notification_type, NotificationAllTimeView.status, - func.date_trunc("day", NotificationAllTimeView.created_at).label("day"), + func.date_trunc("day", NotificationAllTimeView.created_at).label("timestamp"), func.count(NotificationAllTimeView.id).label("count"), ) .where( @@ -799,31 +800,49 @@ def fetch_notification_stats_for_service_by_month_by_user( def get_specific_days_stats( - data, start_date, days=None, end_date=None, total_notifications=None + data, + start_date, + days=None, + end_date=None, + timezone="UTC", + total_notifications=None ): + user_timezone = pytz.timezone(timezone or "UTC") + if days is not None and end_date is not None: raise ValueError("Only set days OR set end_date, not both.") elif days is not None: - gen_range = generate_date_range(start_date, days=days) + date_range = list(generate_date_range(start_date, days=days)) elif end_date is not None: - gen_range = generate_date_range(start_date, end_date) + date_range = list(generate_date_range(start_date, end_date=end_date)) else: - raise ValueError("Either days or end_date must be set.") + raise ValueError("Either 'days' or 'end_date' must be set.") - grouped_data = {date: [] for date in gen_range} | { - day: [row for row in data if row.day == day] - for day in {item.day for item in data} - } + # Ensure date_range is in correct format + grouped_data = {day.strftime("%Y-%m-%d"): [] for day in date_range} - stats = { - day.strftime("%Y-%m-%d"): statistics.format_statistics( - rows, - total_notifications=( - total_notifications.get(day, 0) - if total_notifications is not None - else None - ), + for item in data: + if not isinstance(item.timestamp, datetime): + continue + + local_datetime = item.timestamp.astimezone(user_timezone) + local_date_str = local_datetime.strftime("%Y-%m-%d") + + if local_date_str in grouped_data: + grouped_data[local_date_str].append(item) + + # Build final stats, optionally including total_notifications + stats = {} + for day in date_range: + formatted_day = day.strftime("%Y-%m-%d") + day_data = grouped_data.get(formatted_day, []) # Avoid KeyError + total_for_day = None + if total_notifications and formatted_day in total_notifications: + total_for_day = total_notifications[formatted_day] + + stats[formatted_day] = statistics.format_statistics( + day_data, + total_notifications=total_for_day ) - for day, rows in grouped_data.items() - } + return stats diff --git a/app/service/rest.py b/app/service/rest.py index 657555348..cf3b772dc 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -1,5 +1,5 @@ import itertools -from datetime import datetime, timedelta +from datetime import datetime from flask import Blueprint, current_app, jsonify, request from sqlalchemy import select @@ -19,7 +19,11 @@ save_model_api_key, ) from app.dao.dao_utils import dao_rollback, transaction -from app.dao.date_util import get_calendar_year, get_month_start_and_end_date_in_utc +from app.dao.date_util import ( + build_local_and_utc_date_range, + get_calendar_year, + get_month_start_and_end_date_in_utc, +) from app.dao.fact_notification_status_dao import ( fetch_monthly_template_usage_for_service, fetch_notification_status_for_service_by_month, @@ -221,66 +225,63 @@ def get_service_notification_statistics(service_id): @service_blueprint.route("//statistics//") def get_service_notification_statistics_by_day(service_id, start, days): + user_timezone = request.args.get("timezone", "UTC") + return jsonify( - data=get_service_statistics_for_specific_days(service_id, start, int(days)) + data=get_service_statistics_for_specific_days(service_id, start, int(days), user_timezone) ) -def get_service_statistics_for_specific_days(service_id, start, days=1): - # start and end dates needs to be reversed because - # the end date is today and the start is x days in the past - # a day needs to be substracted to allow for today - end_date = datetime.strptime(start, "%Y-%m-%d") - start_date = end_date - timedelta(days=days - 1) +def get_service_statistics_for_specific_days(service_id, start, days=7, timezone="UTC"): + ( + local_start_date, + utc_start_date, + utc_end_date + ) = build_local_and_utc_date_range(start, days, timezone) total_notifications, results = dao_fetch_stats_for_service_from_days( service_id, - start_date, - end_date, + utc_start_date, + utc_end_date, ) stats = get_specific_days_stats( - results, - start_date, + data=results, + start_date=local_start_date, days=days, total_notifications=total_notifications, + timezone=timezone ) - return stats @service_blueprint.route( "//statistics/user///" ) -def get_service_notification_statistics_by_day_by_user( - service_id, user_id, start, days -): +def get_service_notification_statistics_by_day_by_user(service_id, user_id, start, days): + user_timezone = request.args.get('timezone', 'UTC') return jsonify( data=get_service_statistics_for_specific_days_by_user( - service_id, user_id, start, int(days) + service_id, user_id, start, int(days), timezone=user_timezone ) ) def get_service_statistics_for_specific_days_by_user( - service_id, user_id, start, days=1 + service_id, user_id, start, days=1, timezone="UTC" ): - # start and end dates needs to be reversed because - # the end date is today and the start is x days in the past - # a day needs to be substracted to allow for today - end_date = datetime.strptime(start, "%Y-%m-%d") - start_date = end_date - timedelta(days=days - 1) + ( + local_start_date, + utc_start_date, + utc_end_date + ) = build_local_and_utc_date_range(start_date_str=start, days=days, timezone=timezone) total_notifications, results = dao_fetch_stats_for_service_from_days_for_user( - service_id, start_date, end_date, user_id + service_id, utc_start_date, utc_end_date, user_id ) - stats = get_specific_days_stats( - results, - start_date, - days=days, - total_notifications=total_notifications, - ) + stats = get_specific_days_stats(results, local_start_date, days=days, total_notifications=total_notifications) + return stats diff --git a/tests/app/dao/test_date_utils.py b/tests/app/dao/test_date_utils.py index d4581104d..051cf4af7 100644 --- a/tests/app/dao/test_date_utils.py +++ b/tests/app/dao/test_date_utils.py @@ -1,8 +1,10 @@ -from datetime import date, datetime +from datetime import date, datetime, timedelta import pytest +import pytz from app.dao.date_util import ( + build_local_and_utc_date_range, get_calendar_year, get_calendar_year_for_datetime, get_month_start_and_end_date_in_utc, @@ -75,3 +77,32 @@ def test_get_month_start_and_end_date_in_utc(month, year, expected_start, expect ) def test_get_calendar_year_for_datetime(dt, fy): assert get_calendar_year_for_datetime(dt) == fy + + +def test_build_local_and_utc_date_range(): + local_start, utc_start, utc_end = build_local_and_utc_date_range( + "2025-02-04", 7, "America/New_York" + ) + assert local_start.tzinfo + assert utc_start.tzinfo + assert utc_end.tzinfo + assert utc_end > utc_start + + +def test_build_local_and_utc_7_days_ny(): + local_start, utc_start, utc_end = build_local_and_utc_date_range( + "2025-02-10", 7, "America/New_York" + ) + diff = local_start + timedelta(days=7) + assert diff == datetime(2025, 2, 10, tzinfo=pytz.timezone("America/New_York")) + assert utc_start < utc_end + + +def test_build_local_and_utc_1_day_utc(): + local_start, utc_start, utc_end = build_local_and_utc_date_range( + "2025-02-10", 1, "UTC" + ) + # this should be one day before + assert local_start.isoformat() == "2025-02-09T00:00:00+00:00" + assert utc_start.hour == 0 + assert utc_end.hour == 23 diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index d319ffcaf..26bddef1f 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -1,9 +1,10 @@ import uuid +from collections import namedtuple from datetime import datetime, timedelta from unittest import mock -from unittest.mock import Mock import pytest +import pytz import sqlalchemy from freezegun import freeze_time from sqlalchemy import func, select @@ -1627,7 +1628,7 @@ def test_get_live_services_with_organization(sample_organization): ] -_this_date = utc_now() - timedelta(days=4) +_this_date = datetime(2025, 2, 2, 13, 45, 22, 222955, tzinfo=pytz.utc) @pytest.mark.parametrize( @@ -1644,7 +1645,7 @@ def test_get_live_services_with_organization(sample_organization): {"day": _this_date + timedelta(days=4), "something": "blue"}, ], _this_date, - 4, + 5, None, { _this_date.date().strftime("%Y-%m-%d"): { @@ -1812,23 +1813,30 @@ def test_get_specific_days(data, start_date, days, end_date, expected, is_error) with pytest.raises(ValueError): get_specific_days_stats(data, start_date, days, end_date) else: - new_data = [] - for line in data: - new_line = Mock() - new_line.day = line["day"] - new_line.notification_type = NotificationType.SMS - new_line.count = 1 - new_line.something = line["something"] - new_data.append(new_line) + Notification = namedtuple("Notification", ["timestamp", "notification_type", "count", "something", "status"]) + + new_data = [ + Notification( + timestamp=line["day"], + notification_type=NotificationType.SMS, + count=1, + something=line["something"], + status=None + ) + for line in data + ] total_notifications = None - date_key = _this_date.date().strftime("%Y-%m-%d") + # Convert start_date to UTC to ensure consistency + start_date = start_date.astimezone(pytz.utc) + + date_key = start_date.date().strftime("%Y-%m-%d") if expected and date_key in expected: sms_stats = expected[date_key].get(TemplateType.SMS, {}) requested = sms_stats.get(StatisticsType.REQUESTED, 0) if requested > 0: - total_notifications = {_this_date: requested} + total_notifications = {date_key: requested} results = get_specific_days_stats( new_data, @@ -1837,4 +1845,5 @@ def test_get_specific_days(data, start_date, days, end_date, expected, is_error) end_date, total_notifications=total_notifications, ) + assert results == expected diff --git a/tests/app/dao/test_services_get_specific_days.py b/tests/app/dao/test_services_get_specific_days.py new file mode 100644 index 000000000..083781736 --- /dev/null +++ b/tests/app/dao/test_services_get_specific_days.py @@ -0,0 +1,187 @@ +from datetime import datetime +from unittest.mock import Mock + +import pytest +import pytz + +from app.dao.services_dao import get_specific_days_stats +from app.enums import StatisticsType +from app.models import TemplateType + + +def generate_expected_output(requested_days, requested_sms_days): + output = {} + for day in requested_days: + output[day] = { + TemplateType.SMS: { + StatisticsType.REQUESTED: 1 if day in requested_sms_days else 0, + StatisticsType.DELIVERED: 0, + StatisticsType.FAILURE: 0, + StatisticsType.PENDING: 0, + }, + TemplateType.EMAIL: { + StatisticsType.REQUESTED: 0, + StatisticsType.DELIVERED: 0, + StatisticsType.FAILURE: 0, + StatisticsType.PENDING: 0, + }, + } + return output + + +def create_mock_notification(notification_type, status, timestamp, count=1): + return Mock( + notification_type=notification_type, + status=status, + timestamp=timestamp, + count=count, + ) + + +test_cases = [ + ( + [create_mock_notification( + TemplateType.SMS, + StatisticsType.REQUESTED, + datetime(2025, 1, 29, 1, 20, 18, tzinfo=pytz.utc), + )], + datetime(2025, 1, 28, tzinfo=pytz.utc), + 2, + "America/New_York", + generate_expected_output(["2025-01-28", "2025-01-29"], ["2025-01-28"]), + ), + ( + [create_mock_notification( + TemplateType.SMS, + StatisticsType.REQUESTED, + datetime(2025, 1, 30, 4, 30, 0, tzinfo=pytz.utc), + )], + datetime(2025, 1, 29, tzinfo=pytz.utc), + 2, + "America/New_York", + generate_expected_output(["2025-01-29", "2025-01-30"], ["2025-01-29"]), + ), + ( + [create_mock_notification( + TemplateType.SMS, + StatisticsType.REQUESTED, + datetime(2025, 1, 29, 10, 15, 0, tzinfo=pytz.utc), + )], + datetime(2025, 1, 28, tzinfo=pytz.utc), + 2, + "UTC", + generate_expected_output(["2025-01-28", "2025-01-29"], ["2025-01-29"]), + ), + ( + [create_mock_notification( + TemplateType.SMS, + StatisticsType.REQUESTED, + datetime(2025, 1, 29, 3, 0, 0, tzinfo=pytz.utc), + )], + datetime(2025, 1, 28, tzinfo=pytz.utc), + 2, + "America/Chicago", + generate_expected_output(["2025-01-28", "2025-01-29"], ["2025-01-28"]), + ), + ( + [create_mock_notification( + TemplateType.SMS, + StatisticsType.REQUESTED, + datetime(2025, 1, 29, 5, 0, 0, tzinfo=pytz.utc), + )], + datetime(2025, 1, 28, tzinfo=pytz.utc), + 2, + "America/Denver", + generate_expected_output(["2025-01-28", "2025-01-29"], ["2025-01-28"]), + ), + ( + [create_mock_notification( + TemplateType.SMS, + StatisticsType.REQUESTED, + datetime(2025, 1, 29, 7, 30, 0, tzinfo=pytz.utc), + )], + datetime(2025, 1, 28, tzinfo=pytz.utc), + 2, + "America/Los_Angeles", + generate_expected_output(["2025-01-28", "2025-01-29"], ["2025-01-28"]), + ), + ( + [create_mock_notification( + TemplateType.SMS, + StatisticsType.REQUESTED, + datetime(2025, 1, 29, 10, 15, 0, tzinfo=pytz.utc), + )], + datetime(2025, 1, 28, tzinfo=pytz.utc), + 2, + None, + generate_expected_output(["2025-01-28", "2025-01-29"], ["2025-01-29"]), + ), + ( + [create_mock_notification( + TemplateType.SMS, + StatisticsType.REQUESTED, + datetime(2024, 3, 10, 6, 30, 0, tzinfo=pytz.utc), + )], + datetime(2024, 3, 9, tzinfo=pytz.utc), + 2, + "America/New_York", + generate_expected_output(["2024-03-09", "2024-03-10"], ["2024-03-10"]), + ), + ( + [create_mock_notification( + TemplateType.SMS, + StatisticsType.REQUESTED, + datetime(2024, 11, 3, 5, 30, 0, tzinfo=pytz.utc), + )], + datetime(2024, 11, 2, tzinfo=pytz.utc), + 2, + "America/New_York", + generate_expected_output(["2024-11-02", "2024-11-03"], ["2024-11-03"]), + ), + ( + [], + datetime(2025, 1, 29, tzinfo=pytz.utc), + 2, + "UTC", + generate_expected_output(["2025-01-29", "2025-01-30"], []), + ), + ( + [create_mock_notification( + TemplateType.SMS, + StatisticsType.REQUESTED, + datetime(2025, 1, 10, 0, 0, 0, tzinfo=pytz.utc), + )], + datetime(2025, 1, 9, tzinfo=pytz.utc), + 2, + "America/New_York", + generate_expected_output(["2025-01-09", "2025-01-10"], ["2025-01-09"]), + ), + ( + [create_mock_notification( + TemplateType.SMS, + StatisticsType.REQUESTED, + datetime(2025, 1, 15, 12, 0, 0, tzinfo=pytz.utc), + )], + datetime(2025, 1, 1, tzinfo=pytz.utc), + 30, + "America/New_York", + generate_expected_output( + [f"2025-01-{str(day).zfill(2)}" for day in range(1, 31)], + ["2025-01-15"], + ), + ), +] + + +@pytest.mark.parametrize( + "mocked_notifications, start_date, days, timezone, expected_output", + test_cases, +) +def test_get_specific_days(mocked_notifications, start_date, days, timezone, expected_output): + results = get_specific_days_stats( + mocked_notifications, + start_date, + days, + timezone=timezone, + ) + assert results == expected_output