diff --git a/app/main/forms.py b/app/main/forms.py index 9340c3ba4..52fd8b2c3 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -1726,8 +1726,12 @@ class TemplateFolderForm(StripWhitespaceForm): def __init__(self, all_service_users=None, *args, **kwargs): super().__init__(*args, **kwargs) if all_service_users is not None: - regular_users = [user for user in all_service_users if not user.platform_admin] - platform_admins = [user for user in all_service_users if user.platform_admin] + regular_users = [ + user for user in all_service_users if not user.platform_admin + ] + platform_admins = [ + user for user in all_service_users if user.platform_admin + ] self.users_with_permission.all_service_users = regular_users self.users_with_permission.choices = [ diff --git a/app/main/views/activity.py b/app/main/views/activity.py index 4ecf30bae..3e92c0f64 100644 --- a/app/main/views/activity.py +++ b/app/main/views/activity.py @@ -34,14 +34,16 @@ def get_report_info(service_id, report_name, s3_config): # check_s3_file_exists already called obj.load(), so metadata should be populated size_bytes = obj.content_length - if size_bytes < 1024: - size_str = f"{size_bytes} B" - elif size_bytes < 1024 * 1024: - size_str = f"{size_bytes / 1024:.1f} KB" - else: - size_str = f"{size_bytes / (1024 * 1024):.1f} MB" + # Only show as available if file has any content (not empty) + if size_bytes > 0: + if size_bytes < 1024: + size_str = f"{size_bytes} B" + elif size_bytes < 1024 * 1024: + size_str = f"{size_bytes / 1024:.1f} KB" + else: + size_str = f"{size_bytes / (1024 * 1024):.1f} MB" - return {"available": True, "size": size_str} + return {"available": True, "size": size_str} except Exception: # nosec B110 pass diff --git a/app/main/views/notifications.py b/app/main/views/notifications.py index e28271a5b..bdae1e33c 100644 --- a/app/main/views/notifications.py +++ b/app/main/views/notifications.py @@ -174,7 +174,11 @@ def download_notifications_csv(service_id): ) s3_file_content = s3download(service_id, s3_report_id) return Response( - stream_with_context(convert_s3_csv_timestamps(s3_file_content)), + stream_with_context( + convert_s3_csv_timestamps( + s3_file_content, user_timezone=user_tz_name + ) + ), mimetype="text/csv", headers={ "Content-Disposition": 'inline; filename="{} - {} - {} report.csv"'.format( @@ -185,7 +189,6 @@ def download_notifications_csv(service_id): }, ) except S3ObjectNotFound: - # Edge case: File was deleted between page load and download attempt current_app.logger.warning( f"File {s3_report_id} was expected but not found for service {service_id}. " "It may have been deleted after page load." diff --git a/app/main/views/templates.py b/app/main/views/templates.py index 9d697361e..c310063a5 100644 --- a/app/main/views/templates.py +++ b/app/main/views/templates.py @@ -183,7 +183,7 @@ def process_folder_management_form(form, current_folder_id): current_service.id, name=form.get_folder_name(), parent_id=current_folder_id, - created_by_id=str(current_user.id) + created_by_id=str(current_user.id), ) if form.is_move_op: diff --git a/app/notify_client/template_folder_api_client.py b/app/notify_client/template_folder_api_client.py index 3257e5729..cb070e388 100644 --- a/app/notify_client/template_folder_api_client.py +++ b/app/notify_client/template_folder_api_client.py @@ -4,7 +4,9 @@ from app.notify_client import NotifyAdminAPIClient, cache class TemplateFolderAPIClient(NotifyAdminAPIClient): @cache.delete("service-{service_id}-template-folders") - def create_template_folder(self, service_id, name, parent_id=None, created_by_id=None): + def create_template_folder( + self, service_id, name, parent_id=None, created_by_id=None + ): data = {"name": name, "parent_id": parent_id} if created_by_id: data["created_by_id"] = created_by_id diff --git a/app/templates/views/activity/all-activity.html b/app/templates/views/activity/all-activity.html index 94b271bec..3e2608035 100644 --- a/app/templates/views/activity/all-activity.html +++ b/app/templates/views/activity/all-activity.html @@ -203,7 +203,7 @@ {% endif %} diff --git a/app/utils/csv.py b/app/utils/csv.py index 519188473..33c4874f7 100644 --- a/app/utils/csv.py +++ b/app/utils/csv.py @@ -179,36 +179,66 @@ def generate_notifications_csv(**kwargs): raise Exception("Should never reach here") -def convert_report_date_to_preferred_timezone(db_date_str_in_utc): +def convert_report_date_to_preferred_timezone(db_date_str_in_utc, target_timezone=None): """ Report dates in the db are in UTC. We need to convert them to the user's default timezone, which defaults to "US/Eastern" + Format: "2025-05-14 07:22:18 AM America/Los_Angeles" + + Args: + db_date_str_in_utc: UTC datetime string in format "YYYY-MM-DD HH:MM:SS" + target_timezone: Optional timezone name to use (e.g. "US/Eastern", "America/Los_Angeles") + If not provided, will use current user's preference or default """ - date_arr = db_date_str_in_utc.split(" ") - db_date_str_in_utc = f"{date_arr[0]}T{date_arr[1]}+00:00" - utc_date_obj = datetime.datetime.fromisoformat(db_date_str_in_utc) + try: + date_arr = db_date_str_in_utc.split(" ") + db_date_str_in_utc = f"{date_arr[0]}T{date_arr[1]}+00:00" + utc_date_obj = datetime.datetime.fromisoformat(db_date_str_in_utc) - utc_date_obj = utc_date_obj.replace(tzinfo=ZoneInfo("UTC")) - preferred_timezone = get_user_preferred_timezone_obj() - preferred_date_obj = utc_date_obj.astimezone(preferred_timezone) - preferred_tz_created_at = preferred_date_obj.strftime("%Y-%m-%d %H:%M:%S") + utc_date_obj = utc_date_obj.replace(tzinfo=ZoneInfo("UTC")) - return preferred_tz_created_at + if not target_timezone: + target_timezone = get_user_preferred_timezone() + + preferred_timezone = ZoneInfo(target_timezone) + preferred_date_obj = utc_date_obj.astimezone(preferred_timezone) + + formatted_date = preferred_date_obj.strftime("%Y-%m-%d %I:%M:%S %p") + preferred_tz_created_at = f"{formatted_date} {target_timezone}" + return preferred_tz_created_at + + except Exception as e: + try: + current_app.logger.error( + f"Error converting timezone for {db_date_str_in_utc}: {e}" + ) + except RuntimeError: + pass + return f"{db_date_str_in_utc} UTC" _timezone_cache = {} def get_user_preferred_timezone(): - if current_user and hasattr(current_user, "preferred_timezone"): - tz = current_user.preferred_timezone - # Validate timezone using ZoneInfo - it will raise if invalid - try: - ZoneInfo(tz) - return tz - except ZoneInfoNotFoundError: - # Invalid timezone, fall back to default - pass + try: + if ( + current_user + and current_user.is_authenticated + and hasattr(current_user, "preferred_timezone") + ): + tz = current_user.preferred_timezone + if tz: + try: + ZoneInfo(tz) + return tz + except ZoneInfoNotFoundError: + pass + except (AttributeError, RuntimeError): + # AttributeError: current_user might not have expected attributes + # RuntimeError: working outside of request context + pass + return "US/Eastern" diff --git a/app/utils/s3_csv.py b/app/utils/s3_csv.py index 79620f9ad..6498cac18 100644 --- a/app/utils/s3_csv.py +++ b/app/utils/s3_csv.py @@ -1,13 +1,26 @@ import csv import io -from app.utils.csv import convert_report_date_to_preferred_timezone +from app.utils.csv import ( + convert_report_date_to_preferred_timezone, + get_user_preferred_timezone, +) -def convert_s3_csv_timestamps(csv_content): +def convert_s3_csv_timestamps(csv_content, user_timezone=None): + """ + Convert UTC timestamps in CSV to user's preferred timezone. + + Args: + csv_content: The CSV content as string or bytes + user_timezone: Optional pre-captured timezone (to avoid context issues in streaming) + """ if isinstance(csv_content, bytes): csv_content = csv_content.decode("utf-8") + if user_timezone is None: + user_timezone = get_user_preferred_timezone() + reader = csv.reader(io.StringIO(csv_content)) time_column_index = None @@ -39,7 +52,7 @@ def convert_s3_csv_timestamps(csv_content): 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], target_timezone=user_timezone ) 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 4634ece3c..fa8cc1bfa 100644 --- a/tests/app/main/views/test_download_notifications_csv_s3.py +++ b/tests/app/main/views/test_download_notifications_csv_s3.py @@ -107,5 +107,9 @@ def test_s3_csv_gets_timezone_converted( _test_page_title=False, ) - mock_convert.assert_called_once_with(b"csv,data") + # Now it passes the user_timezone parameter + assert mock_convert.call_count == 1 + call_args = mock_convert.call_args + assert call_args[0][0] == b"csv,data" + assert "user_timezone" in call_args[1] assert response.status_code == 200 diff --git a/tests/app/main/views/test_template_folders.py b/tests/app/main/views/test_template_folders.py index a3d9e60ad..9e03a86e6 100644 --- a/tests/app/main/views/test_template_folders.py +++ b/tests/app/main/views/test_template_folders.py @@ -1500,7 +1500,10 @@ def test_new_folder_is_created_if_only_new_folder_is_filled_out( assert mock_move_to_template_folder.called is False mock_create_template_folder.assert_called_once_with( - SERVICE_ONE_ID, name="new folder", parent_id=None, created_by_id="6ce466d0-fd6a-11e5-82f5-e0accb9d11a6" + SERVICE_ONE_ID, + name="new folder", + parent_id=None, + created_by_id="6ce466d0-fd6a-11e5-82f5-e0accb9d11a6", ) @@ -1538,7 +1541,10 @@ def test_should_be_able_to_move_to_new_folder( ) mock_create_template_folder.assert_called_once_with( - SERVICE_ONE_ID, name="new folder", parent_id=None, created_by_id="6ce466d0-fd6a-11e5-82f5-e0accb9d11a6" + SERVICE_ONE_ID, + name="new folder", + parent_id=None, + created_by_id="6ce466d0-fd6a-11e5-82f5-e0accb9d11a6", ) mock_move_to_template_folder.assert_called_once_with( service_id=SERVICE_ONE_ID, diff --git a/tests/app/utils/test_csv.py b/tests/app/utils/test_csv.py index cb1e1e474..4f039e5cb 100644 --- a/tests/app/utils/test_csv.py +++ b/tests/app/utils/test_csv.py @@ -90,14 +90,14 @@ def get_notifications_csv_mock( None, [ "Phone Number,Template,Sent by,Batch File,Carrier Response,Status,Time,Carrier\n", - "8005555555,foo,,,Did not like it,Delivered,1943-04-19 08:00:00,AT&T Mobility\r\n", + "8005555555,foo,,,Did not like it,Delivered,1943-04-19 08:00:00 AM US/Eastern,AT&T Mobility\r\n", ], ), ( "Anne Example", [ "Phone Number,Template,Sent by,Batch File,Carrier Response,Status,Time,Carrier\n", - "8005555555,foo,Anne Example,,Did not like it,Delivered,1943-04-19 08:00:00,AT&T Mobility\r\n", # noqa + "8005555555,foo,Anne Example,,Did not like it,Delivered,1943-04-19 08:00:00 AM US/Eastern,AT&T Mobility\r\n", # noqa ], ), ], @@ -145,7 +145,7 @@ def test_generate_notifications_csv_without_job( "bar.csv", "Did not like it", "Delivered", - "1943-04-19 08:00:00", + "1943-04-19 08:00:00 AM US/Eastern", "AT&T Mobility", ], ), @@ -174,7 +174,7 @@ def test_generate_notifications_csv_without_job( "bar.csv", "Did not like it", "Delivered", - "1943-04-19 08:00:00", + "1943-04-19 08:00:00 AM US/Eastern", "AT&T Mobility", "🐜", "🐝", @@ -206,7 +206,7 @@ def test_generate_notifications_csv_without_job( "bar.csv", "Did not like it", "Delivered", - "1943-04-19 08:00:00", + "1943-04-19 08:00:00 AM US/Eastern", "AT&T Mobility", "🐜,🐜", "🐝,🐝", @@ -383,6 +383,47 @@ def test_get_errors_for_csv( def test_convert_report_date_to_preferred_timezone(): + """Test that timezone conversion includes AM/PM and timezone name.""" original = "2023-11-16 05:00:00" altered = convert_report_date_to_preferred_timezone(original) - assert altered == "2023-11-16 00:00:00" + assert altered == "2023-11-16 12:00:00 AM US/Eastern" + + original = "2023-11-16 17:30:00" + altered = convert_report_date_to_preferred_timezone(original) + assert altered == "2023-11-16 12:30:00 PM US/Eastern" + + original = "2023-11-16 17:00:00" + altered = convert_report_date_to_preferred_timezone(original) + assert altered == "2023-11-16 12:00:00 PM US/Eastern" + + original = "2023-11-16 05:00:00" + altered = convert_report_date_to_preferred_timezone(original) + assert altered == "2023-11-16 12:00:00 AM US/Eastern" + + +def test_convert_report_date_with_custom_timezone(mocker): + """Test timezone conversion with a user who has a custom timezone.""" + mocker.patch( + "app.utils.csv.get_user_preferred_timezone", return_value="America/Los_Angeles" + ) + + original = "2023-11-16 15:22:18" + altered = convert_report_date_to_preferred_timezone(original) + assert altered == "2023-11-16 07:22:18 AM America/Los_Angeles" + + +def test_convert_report_date_with_explicit_timezone(): + """Test timezone conversion with explicitly provided timezone.""" + original = "2023-11-16 15:22:18" + altered = convert_report_date_to_preferred_timezone( + original, target_timezone="America/Los_Angeles" + ) + assert altered == "2023-11-16 07:22:18 AM America/Los_Angeles" + + altered = convert_report_date_to_preferred_timezone( + original, target_timezone="US/Eastern" + ) + assert altered == "2023-11-16 10:22:18 AM US/Eastern" + + altered = convert_report_date_to_preferred_timezone(original, target_timezone="UTC") + assert altered == "2023-11-16 03:22:18 PM UTC" diff --git a/tests/app/utils/test_s3_csv.py b/tests/app/utils/test_s3_csv.py index 257ed953a..a4b1af993 100644 --- a/tests/app/utils/test_s3_csv.py +++ b/tests/app/utils/test_s3_csv.py @@ -16,7 +16,7 @@ def test_convert_s3_csv_timestamps_with_real_format(): "app.utils.s3_csv.convert_report_date_to_preferred_timezone" ) as mock_convert: - def mock_conversion(timestamp): + def mock_conversion(timestamp, target_timezone=None): # Just return the timestamp as-is for testing return timestamp @@ -30,8 +30,12 @@ def test_convert_s3_csv_timestamps_with_real_format(): 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") + mock_convert.assert_any_call( + "2024-03-15 17:19:00", target_timezone="US/Eastern" + ) + mock_convert.assert_any_call( + "2024-03-15 20:30:00", target_timezone="US/Eastern" + ) assert "2024-03-15 17:19:00" in full_result assert "2024-03-15 20:30:00" in full_result @@ -92,7 +96,7 @@ 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" + mock_convert.side_effect = lambda x, target_timezone=None: f"{x} Converted" result = list(convert_s3_csv_timestamps(csv_content)) full_result = "".join(result) @@ -106,11 +110,11 @@ 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.is_authenticated = True mock_user.preferred_timezone = "US/Eastern" result = convert_report_date_to_preferred_timezone("2024-01-15 20:30:00") - # Should be in 24-hour format without AM/PM or timezone - assert "15:30:00" in result - assert "PM" not in result - assert "US/Eastern" not in result + # Should now be in 12-hour format with AM/PM and timezone + assert "03:30:00 PM" in result + assert "US/Eastern" in result