From dbfeb32eb2d11a354374cefb4d85b29b63b2f6a3 Mon Sep 17 00:00:00 2001 From: Beverly Nguyen Date: Thu, 7 Aug 2025 18:49:09 -0700 Subject: [PATCH] clean up --- app/main/views/activity.py | 49 ++++++++-------------- app/notify_client/job_api_client.py | 20 ++++++++- tests/app/main/views/test_jobs_activity.py | 30 +++++++------ 3 files changed, 53 insertions(+), 46 deletions(-) diff --git a/app/main/views/activity.py b/app/main/views/activity.py index 89097db74..4e2f9fd3e 100644 --- a/app/main/views/activity.py +++ b/app/main/views/activity.py @@ -14,9 +14,6 @@ from app.utils.user import user_has_permissions def get_download_availability(service_id): - """ - Check if there are jobs available for each download time period. - """ jobs_1_day = job_api_client.get_page_of_jobs(service_id, page=1, limit_days=1) jobs_3_days = job_api_client.get_page_of_jobs(service_id, page=1, limit_days=3) jobs_5_days = job_api_client.get_page_of_jobs(service_id, page=1, limit_days=5) @@ -39,6 +36,21 @@ def get_download_availability(service_id): } +def get_download_links(message_type): + time_periods = ["one_day", "three_day", "five_day", "seven_day"] + links = {} + + for period in time_periods: + links[f"download_link_{period}"] = url_for( + ".download_notifications_csv", + service_id=current_service.id, + message_type=message_type, + status=request.args.get("status"), + number_of_days=period, + ) + return links + + def get_filtered_jobs(service_id, page): filter_type = request.args.get("filter") @@ -70,6 +82,8 @@ def all_jobs_activity(service_id): prev_page, next_page, pagination = handle_pagination(jobs, service_id, page) message_type = ("sms",) download_availability = get_download_availability(service_id) + download_links = get_download_links(message_type) + return render_template( "views/activity/all-activity.html", all_jobs_dict=all_jobs_dict, @@ -79,34 +93,7 @@ def all_jobs_activity(service_id): pagination=pagination, total_jobs=jobs.get("total", 0), **download_availability, - download_link_one_day=url_for( - ".download_notifications_csv", - service_id=current_service.id, - message_type=message_type, - status=request.args.get("status"), - number_of_days="one_day", - ), - download_link_three_day=url_for( - ".download_notifications_csv", - service_id=current_service.id, - message_type=message_type, - status=request.args.get("status"), - number_of_days="three_day", - ), - download_link_five_day=url_for( - ".download_notifications_csv", - service_id=current_service.id, - message_type=message_type, - status=request.args.get("status"), - number_of_days="five_day", - ), - download_link_seven_day=url_for( - ".download_notifications_csv", - service_id=current_service.id, - message_type=message_type, - status=request.args.get("status"), - number_of_days="seven_day", - ), + **download_links, ) diff --git a/app/notify_client/job_api_client.py b/app/notify_client/job_api_client.py index 9de84f0d4..c9a02a1f2 100644 --- a/app/notify_client/job_api_client.py +++ b/app/notify_client/job_api_client.py @@ -32,7 +32,15 @@ class JobApiClient(NotifyAdminAPIClient): return job - def get_jobs(self, service_id, *, limit_days=None, statuses=None, page=1, use_processing_time=False): + def get_jobs( + self, + service_id, + *, + limit_days=None, + statuses=None, + page=1, + use_processing_time=False, + ): params = {"page": page} if limit_days is not None: params["limit_days"] = limit_days @@ -63,7 +71,15 @@ class JobApiClient(NotifyAdminAPIClient): if job["job_status"] != JobStatus.CANCELLED ) - def get_page_of_jobs(self, service_id, *, page, statuses=None, limit_days=None, use_processing_time=False): + def get_page_of_jobs( + self, + service_id, + *, + page, + statuses=None, + limit_days=None, + use_processing_time=False, + ): return self.get_jobs( service_id, statuses=statuses or self.NON_SCHEDULED_JOB_STATUSES, diff --git a/tests/app/main/views/test_jobs_activity.py b/tests/app/main/views/test_jobs_activity.py index 53d50d56f..601660003 100644 --- a/tests/app/main/views/test_jobs_activity.py +++ b/tests/app/main/views/test_jobs_activity.py @@ -114,8 +114,12 @@ def test_all_activity( assert report_cell == "N/A", f"Expected report 'N/A', but got '{report_cell}'" status_cell = cells[5].get_text(strip=True) - assert "1 delivered" in status_cell, f"Expected status to contain '1 delivered', but got '{status_cell}'" - assert "5 failed" in status_cell, f"Expected status to contain '5 failed', but got '{status_cell}'" + assert ( + "1 delivered" in status_cell + ), f"Expected status to contain '1 delivered', but got '{status_cell}'" + assert ( + "5 failed" in status_cell + ), f"Expected status to contain '5 failed', but got '{status_cell}'" def test_all_activity_no_jobs(client_request, mocker): @@ -209,12 +213,15 @@ def test_all_activity_pagination(client_request, mocker): ), f"Expected pagination controls {expected_pagination_texts}, but got {pagination_texts}" -@pytest.mark.parametrize(("filter_type", "expected_limit_days"), [ - ("24hours", 1), - ("3days", 3), - ("7days", 7), - (None, None), -]) +@pytest.mark.parametrize( + ("filter_type", "expected_limit_days"), + [ + ("24hours", 1), + ("3days", 3), + ("7days", 7), + (None, None), + ], +) def test_all_activity_filters(client_request, mocker, filter_type, expected_limit_days): current_page = get_page_from_request() mock_get_page_of_jobs = mocker.patch( @@ -224,10 +231,7 @@ def test_all_activity_filters(client_request, mocker, filter_type, expected_limi kwargs = {"filter": filter_type} if filter_type else {} response = client_request.get_response( - "main.all_jobs_activity", - service_id=SERVICE_ONE_ID, - page=current_page, - **kwargs + "main.all_jobs_activity", service_id=SERVICE_ONE_ID, page=current_page, **kwargs ) assert response.status_code == 200 @@ -235,7 +239,7 @@ 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 + 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)