From 90abf9c90eaee7ec74564c5b47a792bf06b57f37 Mon Sep 17 00:00:00 2001 From: alexjanousekGSA Date: Wed, 13 Aug 2025 20:37:41 -0400 Subject: [PATCH] Refactored reports to use pregenerated docs instead --- app/main/views/notifications.py | 60 ++++++++- app/utils/s3_csv.py | 47 +++++++ .../test_download_notifications_csv_s3.py | 119 ++++++++++++++++++ tests/app/utils/test_s3_csv.py | 89 +++++++++++++ 4 files changed, 313 insertions(+), 2 deletions(-) create mode 100644 app/utils/s3_csv.py create mode 100644 tests/app/main/views/test_download_notifications_csv_s3.py create mode 100644 tests/app/utils/test_s3_csv.py diff --git a/app/main/views/notifications.py b/app/main/views/notifications.py index 59390741d..f3b46c948 100644 --- a/app/main/views/notifications.py +++ b/app/main/views/notifications.py @@ -1,8 +1,10 @@ # -*- coding: utf-8 -*- from datetime import datetime +import pytz from flask import ( Response, + current_app, flash, jsonify, render_template, @@ -15,6 +17,8 @@ from app import current_service, job_api_client, notification_api_client from app.enums import ServicePermission from app.main import main from app.notify_client.api_key_api_client import KEY_TYPE_TEST +from app.utils.s3_csv import convert_s3_csv_timestamps +from app.s3_client.s3_csv_client import s3download from app.utils import ( DELIVERED_STATUSES, FAILURE_STATUSES, @@ -25,6 +29,7 @@ from app.utils import ( from app.utils.csv import generate_notifications_csv, get_user_preferred_timezone from app.utils.templates import get_template from app.utils.user import user_has_permissions +from notifications_utils.s3 import S3ObjectNotFound @main.route("/services//notification/") @@ -135,6 +140,28 @@ def get_all_personalisation_from_notification(notification): return notification["personalisation"] +PERIOD_TO_S3_FILENAME = { + "one_day": "1-day-report", + "three_day": "3-day-report", + "five_day": "5-day-report", + "seven_day": "7-day-report", +} + + +def generate_empty_report_csv(): + headers = [ + "Phone Number", + "Template", + "Sent by", + "Batch File", + "Carrier Response", + "Status", + "Time", + "Carrier", + ] + yield ",".join(headers) + "\n" + + @main.route("/services//download-notifications.csv") @user_has_permissions(ServicePermission.VIEW_ACTIVITY) def download_notifications_csv(service_id): @@ -144,14 +171,43 @@ def download_notifications_csv(service_id): service_data_retention_days = current_service.get_days_of_retention( filter_args.get("message_type")[0], number_of_days ) - file_time = datetime.now().strftime("%Y-%m-%d %I:%M:%S %p") + user_tz = pytz.timezone(get_user_preferred_timezone()) + file_time = datetime.now(user_tz).strftime("%Y-%m-%d %I:%M:%S %p") file_time = f"{file_time} {get_user_preferred_timezone()}" + job_id = request.args.get("job_id") + if not job_id and number_of_days in PERIOD_TO_S3_FILENAME: + try: + s3_report_id = PERIOD_TO_S3_FILENAME[number_of_days] + s3_file_content = s3download(service_id, s3_report_id) + return Response( + stream_with_context(convert_s3_csv_timestamps(s3_file_content)), + mimetype="text/csv", + headers={ + "Content-Disposition": 'inline; filename="{} - {} - {} report.csv"'.format( + file_time, + filter_args["message_type"][0], + current_service.name, + ) + }, + ) + except S3ObjectNotFound: + return Response( + stream_with_context(generate_empty_report_csv()), + mimetype="text/csv", + headers={ + "Content-Disposition": 'inline; filename="{} - {} - {} report.csv"'.format( + file_time, + filter_args["message_type"][0], + current_service.name, + ) + }, + ) return Response( stream_with_context( generate_notifications_csv( service_id=service_id, - job_id=None, + job_id=job_id, status=filter_args.get("status"), page=request.args.get("page", 1), page_size=10000, diff --git a/app/utils/s3_csv.py b/app/utils/s3_csv.py new file mode 100644 index 000000000..353e1c963 --- /dev/null +++ b/app/utils/s3_csv.py @@ -0,0 +1,47 @@ +import csv +import io +from app.utils.csv import convert_report_date_to_preferred_timezone + + +def convert_s3_csv_timestamps(csv_content): + if isinstance(csv_content, bytes): + csv_content = csv_content.decode('utf-8') + + reader = csv.reader(io.StringIO(csv_content)) + + time_column_index = None + try: + header = next(reader) + for i, col in enumerate(header): + if col.strip().lower() == 'time': + time_column_index = i + break + + output = io.StringIO() + writer = csv.writer(output) + writer.writerow(header) + yield output.getvalue() + output.truncate(0) + output.seek(0) + except StopIteration: + return + + if time_column_index is None: + for row in reader: + writer.writerow(row) + yield output.getvalue() + output.truncate(0) + output.seek(0) + return + + for row in reader: + if len(row) > time_column_index and row[time_column_index]: + try: + row[time_column_index] = convert_report_date_to_preferred_timezone(row[time_column_index]) + except Exception: + pass + + writer.writerow(row) + yield output.getvalue() + output.truncate(0) + output.seek(0) diff --git a/tests/app/main/views/test_download_notifications_csv_s3.py b/tests/app/main/views/test_download_notifications_csv_s3.py new file mode 100644 index 000000000..34d6a8adf --- /dev/null +++ b/tests/app/main/views/test_download_notifications_csv_s3.py @@ -0,0 +1,119 @@ +import pytest +from unittest.mock import patch + +from app.main.views.notifications import ( + PERIOD_TO_S3_FILENAME, + generate_empty_report_csv, +) +from notifications_utils.s3 import S3ObjectNotFound +from tests.conftest import SERVICE_ONE_ID + + +def test_period_to_s3_filename_mapping(): + assert PERIOD_TO_S3_FILENAME["one_day"] == "1-day-report" + assert PERIOD_TO_S3_FILENAME["three_day"] == "3-day-report" + assert PERIOD_TO_S3_FILENAME["five_day"] == "5-day-report" + assert PERIOD_TO_S3_FILENAME["seven_day"] == "7-day-report" + + +def test_empty_csv_has_correct_headers(): + result = list(generate_empty_report_csv()) + assert len(result) == 1 + assert "Phone Number,Template,Sent by,Batch File" in result[0] + + +@patch("app.main.views.notifications.s3download") +@patch("app.main.views.notifications.generate_notifications_csv") +def test_job_based_reports_dont_use_s3( + mock_generate_csv, + mock_s3download, + client_request, + service_one, + mock_get_service_data_retention, +): + mock_generate_csv.return_value = iter(["test,data\n"]) + + response = client_request.get_response( + "main.download_notifications_csv", + service_id=SERVICE_ONE_ID, + number_of_days="one_day", + message_type="sms", + job_id="test-job-123", + _test_page_title=False, + ) + + mock_s3download.assert_not_called() + mock_generate_csv.assert_called_once() + assert response.status_code == 200 + + +@patch("app.main.views.notifications.s3download") +@patch("app.main.views.notifications.generate_notifications_csv") +def test_general_reports_use_s3( + mock_generate_csv, + mock_s3download, + client_request, + service_one, + mock_get_service_data_retention, +): + mock_s3download.return_value = b"s3,csv,content\n" + + response = client_request.get_response( + "main.download_notifications_csv", + service_id=SERVICE_ONE_ID, + number_of_days="three_day", + message_type="sms", + _test_page_title=False, + ) + + mock_s3download.assert_called_once_with(SERVICE_ONE_ID, "3-day-report") + mock_generate_csv.assert_not_called() + assert response.status_code == 200 + + +@patch("app.main.views.notifications.s3download") +def test_missing_s3_file_returns_headers_only( + mock_s3download, + client_request, + service_one, + mock_get_service_data_retention, +): + mock_s3download.side_effect = S3ObjectNotFound( + {"Error": {"Code": "NoSuchKey"}}, "GetObject" + ) + + response = client_request.get_response( + "main.download_notifications_csv", + service_id=SERVICE_ONE_ID, + number_of_days="five_day", + message_type="sms", + _test_page_title=False, + ) + + assert response.status_code == 200 + assert b"Phone Number,Template,Sent by" in response.data + assert response.data.count(b"\n") == 1 + + +@patch("app.main.views.notifications.convert_s3_csv_timestamps") +@patch("app.main.views.notifications.s3download") +def test_s3_csv_gets_timezone_converted( + mock_s3download, + mock_convert, + client_request, + service_one, + mock_get_service_data_retention, +): + mock_s3download.return_value = b"csv,data" + mock_convert.return_value = iter(["converted,csv,data\n"]) + + response = client_request.get_response( + "main.download_notifications_csv", + service_id=SERVICE_ONE_ID, + number_of_days="three_day", + message_type="sms", + _test_page_title=False, + ) + + mock_convert.assert_called_once_with(b"csv,data") + assert response.status_code == 200 diff --git a/tests/app/utils/test_s3_csv.py b/tests/app/utils/test_s3_csv.py new file mode 100644 index 000000000..7bb6ad365 --- /dev/null +++ b/tests/app/utils/test_s3_csv.py @@ -0,0 +1,89 @@ +from unittest.mock import patch +from app.utils.s3_csv import convert_s3_csv_timestamps + + +def test_convert_s3_csv_timestamps_with_real_format(): + s3_csv_content = """Phone Number,Template,Sent By,Carrier,Status,Time,Batch File,Carrier Response +14254147167,Example text message template,Backstop Test User,,Failed,2024-03-15 17:19:00,one-off-f0b91c0f.csv,Phone has blocked SMS +14254147755,Example text message template,Admin User,,Delivered,2024-03-15 20:30:00,batch1.csv,Success""" + + with patch('app.utils.s3_csv.convert_report_date_to_preferred_timezone') as mock_convert: + def mock_conversion(timestamp): + return f"{timestamp} US/Eastern" + mock_convert.side_effect = mock_conversion + + result = list(convert_s3_csv_timestamps(s3_csv_content)) + full_result = ''.join(result) + + assert "Phone Number,Template,Sent By,Carrier,Status,Time,Batch File,Carrier Response" in result[0] + assert mock_convert.call_count == 2 + mock_convert.assert_any_call("2024-03-15 17:19:00") + mock_convert.assert_any_call("2024-03-15 20:30:00") + assert "2024-03-15 17:19:00 US/Eastern" in full_result + assert "2024-03-15 20:30:00 US/Eastern" in full_result + + +def test_convert_s3_csv_handles_empty_csv(): + result = list(convert_s3_csv_timestamps("")) + assert result == [] + + +def test_convert_s3_csv_handles_headers_only(): + csv_content = "Phone Number,Template,Sent by,Batch File,Carrier Response,Status,Time,Carrier\n" + result = list(convert_s3_csv_timestamps(csv_content)) + assert len(result) == 1 + assert "Phone Number,Template" in result[0] + + +def test_convert_s3_csv_handles_bytes(): + csv_bytes = b"Phone Number,Template,Sent by,Batch File,Carrier Response,Status,Time,Carrier\n+12025551234,Test,John,,Success,delivered,2024-01-15 20:30:00,Verizon" + + with patch('app.utils.s3_csv.convert_report_date_to_preferred_timezone') as mock_convert: + mock_convert.return_value = "2024-01-15 03:30:00 PM US/Eastern" + + result = list(convert_s3_csv_timestamps(csv_bytes)) + assert len(result) == 2 + assert mock_convert.called + + +def test_convert_s3_csv_handles_malformed_dates(): + csv_content = """Phone Number,Template,Sent by,Batch File,Carrier Response,Status,Time,Carrier ++12025551234,Test,John,,Success,delivered,INVALID_DATE,Verizon ++12025555678,Test,Jane,,Success,delivered,2024-01-15 21:45:00,AT&T""" + + with patch('app.utils.s3_csv.convert_report_date_to_preferred_timezone') as mock_convert: + mock_convert.side_effect = [Exception("Invalid date"), "2024-01-15 04:45:00 PM US/Eastern"] + + result = list(convert_s3_csv_timestamps(csv_content)) + full_result = ''.join(result) + + assert "INVALID_DATE" in full_result + assert "2024-01-15 04:45:00 PM US/Eastern" in full_result + + +def test_finds_time_column_dynamically(): + csv_content = """Template,Phone Number,Time,Status +Test Template,+12025551234,2024-01-15 20:30:00,delivered +Another Template,+12025555678,2024-01-15 21:45:00,delivered""" + + with patch('app.utils.s3_csv.convert_report_date_to_preferred_timezone') as mock_convert: + mock_convert.side_effect = lambda x: f"{x} Converted" + + result = list(convert_s3_csv_timestamps(csv_content)) + full_result = ''.join(result) + + assert mock_convert.call_count == 2 + assert "2024-01-15 20:30:00 Converted" in full_result + assert "2024-01-15 21:45:00 Converted" in full_result + + +def test_actual_timezone_conversion(): + from app.utils.csv import convert_report_date_to_preferred_timezone + + with patch('app.utils.csv.current_user') as mock_user: + mock_user.preferred_timezone = "US/Eastern" + + result = convert_report_date_to_preferred_timezone("2024-01-15 20:30:00") + + assert "03:30:00 PM" in result or "15:30:00 PM" in result + assert "US/Eastern" in result