clean up

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_links,
-            ".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",
-        ),
     )
 
 

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,

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 (
-    assert "5 failed" in status_cell, f"Expected status to contain '5 failed', but got '{status_cell}'"
+        "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"), [
+@pytest.mark.parametrize(
-    ("24hours", 1),
+    ("filter_type", "expected_limit_days"),
-    ("3days", 3),
+    [
-    ("7days", 7),
+        ("24hours", 1),
-    (None, None),
+        ("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",
+        "main.all_jobs_activity", service_id=SERVICE_ONE_ID, page=current_page, **kwargs
-        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)