From 518466dcd2ac117afa5d5b417a8ec77759259adf Mon Sep 17 00:00:00 2001 From: alexjanousekGSA Date: Thu, 14 Aug 2025 09:52:49 -0400 Subject: [PATCH] Cleanup --- app/main/views/notifications.py | 5 +- app/s3_client/__init__.py | 6 +++ app/utils/s3_csv.py | 9 ++-- .../test_download_notifications_csv_s3.py | 1 - tests/app/main/views/test_jobs_activity.py | 5 +- tests/app/utils/test_s3_csv.py | 52 ++++++++++++++----- 6 files changed, 56 insertions(+), 22 deletions(-) diff --git a/app/main/views/notifications.py b/app/main/views/notifications.py index f3b46c948..f1ff1437b 100644 --- a/app/main/views/notifications.py +++ b/app/main/views/notifications.py @@ -1,10 +1,9 @@ # -*- coding: utf-8 -*- from datetime import datetime -import pytz +import pytz from flask import ( Response, - current_app, flash, jsonify, render_template, @@ -17,7 +16,6 @@ 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, @@ -27,6 +25,7 @@ from app.utils import ( set_status_filters, ) from app.utils.csv import generate_notifications_csv, get_user_preferred_timezone +from app.utils.s3_csv import convert_s3_csv_timestamps from app.utils.templates import get_template from app.utils.user import user_has_permissions from notifications_utils.s3 import S3ObjectNotFound diff --git a/app/s3_client/__init__.py b/app/s3_client/__init__.py index fda938ebe..8a82fff23 100644 --- a/app/s3_client/__init__.py +++ b/app/s3_client/__init__.py @@ -5,6 +5,8 @@ from boto3 import Session from botocore.config import Config from flask import current_app +from notifications_utils.s3 import S3ObjectNotFound + AWS_CLIENT_CONFIG = Config( # This config is required to enable S3 to connect to FIPS-enabled # endpoints. See https://aws.amazon.com/compliance/fips/ for more @@ -55,6 +57,8 @@ def get_s3_metadata(obj): current_app.logger.error( f"Unable to download s3 file {obj.bucket_name}/{obj.key}" ) + if client_error.response["Error"]["Code"] == "NoSuchKey": + raise S3ObjectNotFound(client_error.response, client_error.operation_name) raise client_error @@ -76,5 +80,7 @@ def get_s3_contents(obj): current_app.logger.error( f"Unable to download s3 file {obj.bucket_name}/{obj.key}" ) + if client_error.response["Error"]["Code"] == "NoSuchKey": + raise S3ObjectNotFound(client_error.response, client_error.operation_name) raise client_error return contents diff --git a/app/utils/s3_csv.py b/app/utils/s3_csv.py index 7d3dfb4a3..79620f9ad 100644 --- a/app/utils/s3_csv.py +++ b/app/utils/s3_csv.py @@ -1,11 +1,12 @@ 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') + csv_content = csv_content.decode("utf-8") reader = csv.reader(io.StringIO(csv_content)) @@ -13,7 +14,7 @@ def convert_s3_csv_timestamps(csv_content): try: header = next(reader) for i, col in enumerate(header): - if col.strip().lower() == 'time': + if col.strip().lower() == "time": time_column_index = i break @@ -37,7 +38,9 @@ def convert_s3_csv_timestamps(csv_content): 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]) + row[time_column_index] = convert_report_date_to_preferred_timezone( + row[time_column_index] + ) except Exception: # nosec B110 pass diff --git a/tests/app/main/views/test_download_notifications_csv_s3.py b/tests/app/main/views/test_download_notifications_csv_s3.py index 34d6a8adf..6c8b40b37 100644 --- a/tests/app/main/views/test_download_notifications_csv_s3.py +++ b/tests/app/main/views/test_download_notifications_csv_s3.py @@ -1,4 +1,3 @@ -import pytest from unittest.mock import patch from app.main.views.notifications import ( diff --git a/tests/app/main/views/test_jobs_activity.py b/tests/app/main/views/test_jobs_activity.py index 601660003..2d5a55f2e 100644 --- a/tests/app/main/views/test_jobs_activity.py +++ b/tests/app/main/views/test_jobs_activity.py @@ -239,7 +239,10 @@ def test_all_activity_filters(client_request, mocker, filter_type, expected_limi if expected_limit_days: mock_get_page_of_jobs.assert_any_call( - SERVICE_ONE_ID, page=current_page, limit_days=expected_limit_days, use_processing_time=True + SERVICE_ONE_ID, + page=current_page, + limit_days=expected_limit_days, + use_processing_time=True, ) else: mock_get_page_of_jobs.assert_any_call(SERVICE_ONE_ID, page=current_page) diff --git a/tests/app/utils/test_s3_csv.py b/tests/app/utils/test_s3_csv.py index 7bb6ad365..3e4afcbee 100644 --- a/tests/app/utils/test_s3_csv.py +++ b/tests/app/utils/test_s3_csv.py @@ -1,21 +1,33 @@ 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""" + s3_csv_content = ( + "Phone Number,Template,Sent By,Carrier,Status,Time,Batch File,Carrier Response\n" + "14254147167,Example text message template,Backstop Test User,,Failed," + "2024-03-15 17:19:00,one-off-f0b91c0f.csv,Phone has blocked SMS\n" + "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: - 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) + full_result = "".join(result) - assert "Phone Number,Template,Sent By,Carrier,Status,Time,Batch File,Carrier Response" in result[0] + 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") @@ -36,9 +48,14 @@ def test_convert_s3_csv_handles_headers_only(): 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" + csv_bytes = ( + b"Phone Number,Template,Sent by,Batch File,Carrier Response,Status,Time,Carrier\n" + b"+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: + 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)) @@ -51,11 +68,16 @@ def test_convert_s3_csv_handles_malformed_dates(): +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"] + 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) + full_result = "".join(result) assert "INVALID_DATE" in full_result assert "2024-01-15 04:45:00 PM US/Eastern" in full_result @@ -66,11 +88,13 @@ def test_finds_time_column_dynamically(): 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: + 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) + full_result = "".join(result) assert mock_convert.call_count == 2 assert "2024-01-15 20:30:00 Converted" in full_result @@ -80,7 +104,7 @@ Another Template,+12025555678,2024-01-15 21:45:00,delivered""" 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: + 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")