From e828f7885355e4f73fb032c764a9d978c9c8417f Mon Sep 17 00:00:00 2001 From: Beverly Nguyen Date: Mon, 28 Jul 2025 17:04:53 -0700 Subject: [PATCH 1/5] fixing pagination and download links along with tests --- app/main/views/activity.py | 32 +++++++++++++- app/main/views/jobs.py | 7 ++- .../views/activity/all-activity.html | 41 ++++++++++++------ tests/app/main/views/test_activity.py | 43 +++++++++++++++++-- 4 files changed, 104 insertions(+), 19 deletions(-) diff --git a/app/main/views/activity.py b/app/main/views/activity.py index c4cd2d583..9dd60b35b 100644 --- a/app/main/views/activity.py +++ b/app/main/views/activity.py @@ -13,6 +13,32 @@ from app.utils.pagination import ( 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) + jobs_7_days = job_api_client.get_immediate_jobs(service_id) + + has_1_day_data = len(generate_job_dict(jobs_1_day)) > 0 + has_3_day_data = len(generate_job_dict(jobs_3_days)) > 0 + has_5_day_data = len(generate_job_dict(jobs_5_days)) > 0 + has_7_day_data = len(jobs_7_days) > 0 + + return { + "has_1_day_data": has_1_day_data, + "has_3_day_data": has_3_day_data, + "has_5_day_data": has_5_day_data, + "has_7_day_data": has_7_day_data, + "has_any_download_data": has_1_day_data + or has_3_day_data + or has_5_day_data + or has_7_day_data, + } + + @main.route("/activity/services/") @user_has_permissions(ServicePermission.VIEW_ACTIVITY) def all_jobs_activity(service_id): @@ -22,6 +48,7 @@ def all_jobs_activity(service_id): all_jobs_dict = generate_job_dict(jobs) prev_page, next_page, pagination = handle_pagination(jobs, service_id, page) message_type = ("sms",) + download_availability = get_download_availability(service_id) return render_template( "views/activity/all-activity.html", all_jobs_dict=all_jobs_dict, @@ -29,6 +56,7 @@ def all_jobs_activity(service_id): next_page=next_page, prev_page=prev_page, pagination=pagination, + **download_availability, download_link_one_day=url_for( ".download_notifications_csv", service_id=current_service.id, @@ -68,9 +96,11 @@ def handle_pagination(jobs, service_id, page): if page > 1 else None ) + total_items = jobs.get("total", 0) + has_next_link = jobs.get("links", {}).get("next") is not None next_page = ( generate_next_dict("main.all_jobs_activity", service_id, page) - if jobs.get("links", {}).get("next") + if has_next_link and total_items > 50 else None ) pagination = generate_pagination_pages( diff --git a/app/main/views/jobs.py b/app/main/views/jobs.py index 596743af7..938a0849d 100644 --- a/app/main/views/jobs.py +++ b/app/main/views/jobs.py @@ -253,7 +253,12 @@ def get_notifications(service_id, message_type, status_override=None): # noqa ) next_page = None - if "links" in notifications and notifications["links"].get("next", None): + total_items = notifications.get("total", 0) + if ( + "links" in notifications + and notifications["links"].get("next", None) + and total_items > 50 + ): next_page = generate_next_dict( "main.view_notifications", service_id, page, url_args ) diff --git a/app/templates/views/activity/all-activity.html b/app/templates/views/activity/all-activity.html index d722414b3..94209a38c 100644 --- a/app/templates/views/activity/all-activity.html +++ b/app/templates/views/activity/all-activity.html @@ -126,20 +126,33 @@ {{show_pagination}} {% if current_user.has_permissions(ServicePermission.VIEW_ACTIVITY) %} -

Download recent reports

-

- Download all data last 24 hours (CSV) -

-

- Download all data last 3 days (CSV) -   -

-

- Download all data last 5 days (CSV) -

-

- Download all data last 7 days (CSV) -

+ {% if has_any_download_data %} +

Download recent reports

+ {% if has_1_day_data %} +

+ Download all data last 24 hours (CSV) +

+ {% endif %} + {% if has_3_day_data %} +

+ Download all data last 3 days (CSV) +   +

+ {% endif %} + {% if has_5_day_data %} +

+ Download all data last 5 days (CSV) +

+ {% endif %} + {% if has_7_day_data %} +

+ Download all data last 7 days (CSV) +

+ {% endif %} + {% else %} +

Download recent reports

+

No recent activity to download. Download links will appear when jobs are available.

+ {% endif %} {% endif %} {% endblock %} diff --git a/tests/app/main/views/test_activity.py b/tests/app/main/views/test_activity.py index 753931ce5..09e2ab4fd 100644 --- a/tests/app/main/views/test_activity.py +++ b/tests/app/main/views/test_activity.py @@ -9,6 +9,7 @@ from freezegun import freeze_time from app.main.views.jobs import get_status_filters, get_time_left from app.models.service import Service +from tests import notification_json from tests.conftest import ( SERVICE_ONE_ID, create_active_caseworking_user, @@ -465,12 +466,17 @@ def test_should_show_notifications_for_a_service_with_next_previous( client_request, service_one, active_user_with_permissions, - mock_get_notifications_with_previous_next, mock_get_service_statistics, mock_get_service_data_retention, mock_get_no_api_keys, mocker, ): + mocker.patch( + "app.notification_api_client.get_notifications_for_service", + return_value=notification_json( + service_one["id"], rows=50, with_links=True + ) | {"total": 100}, + ) page = client_request.get( "main.view_notifications", service_id=service_one["id"], @@ -504,16 +510,47 @@ def test_should_show_notifications_for_a_service_with_next_previous( assert "page 1" in prev_page_link.text.strip() -def test_doesnt_show_pagination_with_search_term( +def test_doesnt_show_pagination_when_50_or_fewer_items( client_request, service_one, active_user_with_permissions, - mock_get_notifications_with_previous_next, mock_get_service_statistics, mock_get_service_data_retention, mock_get_no_api_keys, mocker, ): + mocker.patch( + "app.notification_api_client.get_notifications_for_service", + return_value=notification_json( + service_one["id"], rows=50, with_links=False + ), + ) + page = client_request.get( + "main.view_notifications", + service_id=service_one["id"], + message_type="sms", + ) + + assert not page.find("a", {"rel": "next"}) + assert not page.find("a", {"rel": "previous"}) + assert not page.select_one(".table-show-more-link") + + +def test_doesnt_show_pagination_with_search_term( + client_request, + service_one, + active_user_with_permissions, + mock_get_service_statistics, + mock_get_service_data_retention, + mock_get_no_api_keys, + mocker, +): + mocker.patch( + "app.notification_api_client.get_notifications_for_service", + return_value=notification_json( + service_one["id"], rows=50, with_links=True + ) | {"total": 100}, + ) page = client_request.post( "main.view_notifications", service_id=service_one["id"], From ba787f8cd166a9abf858a2310fcf96d4d74219ae Mon Sep 17 00:00:00 2001 From: Beverly Nguyen Date: Mon, 28 Jul 2025 17:46:00 -0700 Subject: [PATCH 2/5] add test --- tests/app/main/views/test_activity.py | 88 +++++++++++++++++++++++++++ 1 file changed, 88 insertions(+) diff --git a/tests/app/main/views/test_activity.py b/tests/app/main/views/test_activity.py index 09e2ab4fd..f2431bf17 100644 --- a/tests/app/main/views/test_activity.py +++ b/tests/app/main/views/test_activity.py @@ -285,6 +285,94 @@ def test_link_to_download_notifications( ) +def test_download_links_show_when_data_available( + client_request, + service_one, + active_user_with_permissions, + mocker, +): + + mock_jobs_with_data = { + "data": [{"id": "job1", "created_at": "2020-01-01T00:00:00.000000+00:00"}], + "total": 1, + "page_size": 50 + } + mock_jobs_empty = {"data": [], "total": 0, "page_size": 50} + + mocker.patch("app.job_api_client.get_page_of_jobs", return_value=mock_jobs_with_data) + mocker.patch("app.job_api_client.get_immediate_jobs", return_value=[{"id": "job1"}]) + + page = client_request.get( + "main.all_jobs_activity", + service_id=service_one["id"], + ) + + assert "Download recent reports" in page.text + assert "Download all data last 24 hours" in page.text + assert "Download all data last 3 days" in page.text + assert "Download all data last 5 days" in page.text + assert "Download all data last 7 days" in page.text + assert "No recent activity to download" not in page.text + + +def test_download_links_partial_data_available( + client_request, + service_one, + active_user_with_permissions, + mocker, +): + mock_jobs_with_data = { + "data": [{"id": "job1", "created_at": "2020-01-01T00:00:00.000000+00:00"}], + "total": 1, + "page_size": 50 + } + mock_jobs_empty = {"data": [], "total": 0, "page_size": 50} + + def mock_get_page_of_jobs(service_id, page=1, limit_days=None): + if limit_days in [1, 5]: + return mock_jobs_with_data + return mock_jobs_empty + + mocker.patch("app.job_api_client.get_page_of_jobs", side_effect=mock_get_page_of_jobs) + mocker.patch("app.job_api_client.get_immediate_jobs", return_value=[]) + + page = client_request.get( + "main.all_jobs_activity", + service_id=service_one["id"], + ) + + assert "Download recent reports" in page.text + assert "Download all data last 24 hours" in page.text + assert "Download all data last 3 days" not in page.text + assert "Download all data last 5 days" in page.text + assert "Download all data last 7 days" not in page.text + assert "No recent activity to download" not in page.text + + +def test_download_links_no_data_available( + client_request, + service_one, + active_user_with_permissions, + mocker, +): + mock_jobs_empty = {"data": [], "total": 0, "page_size": 50} + + mocker.patch("app.job_api_client.get_page_of_jobs", return_value=mock_jobs_empty) + mocker.patch("app.job_api_client.get_immediate_jobs", return_value=[]) + + page = client_request.get( + "main.all_jobs_activity", + service_id=service_one["id"], + ) + + assert "Download recent reports" in page.text + assert "Download all data last 24 hours" not in page.text + assert "Download all data last 3 days" not in page.text + assert "Download all data last 5 days" not in page.text + assert "Download all data last 7 days" not in page.text + assert "No recent activity to download. Download links will appear when jobs are available." in page.text + + def test_download_not_available_to_users_without_dashboard( client_request, active_caseworking_user, From eac2733a33122c7f426b399635f3b322c9d56041 Mon Sep 17 00:00:00 2001 From: Beverly Nguyen Date: Tue, 29 Jul 2025 09:57:59 -0700 Subject: [PATCH 3/5] flake8 --- tests/app/main/views/test_activity.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/app/main/views/test_activity.py b/tests/app/main/views/test_activity.py index f2431bf17..9fd5118db 100644 --- a/tests/app/main/views/test_activity.py +++ b/tests/app/main/views/test_activity.py @@ -297,7 +297,6 @@ def test_download_links_show_when_data_available( "total": 1, "page_size": 50 } - mock_jobs_empty = {"data": [], "total": 0, "page_size": 50} mocker.patch("app.job_api_client.get_page_of_jobs", return_value=mock_jobs_with_data) mocker.patch("app.job_api_client.get_immediate_jobs", return_value=[{"id": "job1"}]) From d65be41da5804a5e4ea76ee87e0788b64104e71d Mon Sep 17 00:00:00 2001 From: Beverly Nguyen Date: Tue, 29 Jul 2025 10:17:35 -0700 Subject: [PATCH 4/5] fix test get_immediate_jobs for download availability check --- tests/app/main/views/test_jobs_activity.py | 26 ++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/tests/app/main/views/test_jobs_activity.py b/tests/app/main/views/test_jobs_activity.py index 125b8e46a..ff1e7080d 100644 --- a/tests/app/main/views/test_jobs_activity.py +++ b/tests/app/main/views/test_jobs_activity.py @@ -50,6 +50,9 @@ def test_all_activity( mock_get_page_of_jobs = mocker.patch( "app.job_api_client.get_page_of_jobs", return_value=MOCK_JOBS ) + mocker.patch( + "app.job_api_client.get_immediate_jobs", return_value=[] + ) response = client_request.get_response( "main.all_jobs_activity", @@ -60,7 +63,11 @@ def test_all_activity( assert response.data is not None, "Response data is None" assert "All activity" in response.text - mock_get_page_of_jobs.assert_called_with(SERVICE_ONE_ID, page=current_page) + + assert any( + call[0][0] == SERVICE_ONE_ID and call[1].get('page') == current_page + for call in mock_get_page_of_jobs.call_args_list + ) page = BeautifulSoup(response.data, "html.parser") table = page.find("table") assert table is not None, "Table not found in the response" @@ -115,7 +122,6 @@ def test_all_activity( failed_cell = cells[6].get_text(strip=True) assert failed_cell == "5", f"Expected failed count '5', but got '{failed_cell}'" - mock_get_page_of_jobs.assert_called_with(SERVICE_ONE_ID, page=current_page) def test_all_activity_no_jobs(client_request, mocker): @@ -133,6 +139,9 @@ def test_all_activity_no_jobs(client_request, mocker): "total": 0, }, ) + mocker.patch( + "app.job_api_client.get_immediate_jobs", return_value=[] + ) response = client_request.get_response( "main.all_jobs_activity", service_id=SERVICE_ONE_ID, @@ -152,7 +161,10 @@ def test_all_activity_no_jobs(client_request, mocker): assert ( expected_message == actual_message ), f"Expected message '{expected_message}', but got '{actual_message}'" - mock_get_page_of_jobs.assert_called_with(SERVICE_ONE_ID, page=current_page) + assert any( + call[0][0] == SERVICE_ONE_ID and call[1].get('page') == current_page + for call in mock_get_page_of_jobs.call_args_list + ) def test_all_activity_pagination(client_request, mocker): @@ -182,13 +194,19 @@ def test_all_activity_pagination(client_request, mocker): "total": 100, }, ) + mocker.patch( + "app.job_api_client.get_immediate_jobs", return_value=[] + ) response = client_request.get_response( "main.all_jobs_activity", service_id=SERVICE_ONE_ID, page=current_page, ) - mock_get_page_of_jobs.assert_called_with(SERVICE_ONE_ID, page=current_page) + assert any( + call[0][0] == SERVICE_ONE_ID and call[1].get('page') == current_page + for call in mock_get_page_of_jobs.call_args_list + ) page = BeautifulSoup(response.data, "html.parser") pagination_controls = page.find_all("li", class_="usa-pagination__item") From e5885f674b35e1c03630ec4e4fdfabe5d1146699 Mon Sep 17 00:00:00 2001 From: Beverly Nguyen Date: Tue, 29 Jul 2025 10:25:43 -0700 Subject: [PATCH 5/5] Fixed pagination logic to prevent infinite next button --- app/main/views/activity.py | 4 +++- app/main/views/jobs.py | 3 +++ tests/app/main/views/test_activity.py | 31 ++++++++++++++++++++++++++- 3 files changed, 36 insertions(+), 2 deletions(-) diff --git a/app/main/views/activity.py b/app/main/views/activity.py index 9dd60b35b..1d6d12d98 100644 --- a/app/main/views/activity.py +++ b/app/main/views/activity.py @@ -97,10 +97,12 @@ def handle_pagination(jobs, service_id, page): else None ) total_items = jobs.get("total", 0) + page_size = jobs.get("page_size", 50) + total_pages = (total_items + page_size - 1) // page_size has_next_link = jobs.get("links", {}).get("next") is not None next_page = ( generate_next_dict("main.all_jobs_activity", service_id, page) - if has_next_link and total_items > 50 + if has_next_link and total_items > 50 and page < total_pages else None ) pagination = generate_pagination_pages( diff --git a/app/main/views/jobs.py b/app/main/views/jobs.py index 938a0849d..afe0504bf 100644 --- a/app/main/views/jobs.py +++ b/app/main/views/jobs.py @@ -254,10 +254,13 @@ def get_notifications(service_id, message_type, status_override=None): # noqa next_page = None total_items = notifications.get("total", 0) + page_size = notifications.get("page_size", 50) + total_pages = (total_items + page_size - 1) // page_size if ( "links" in notifications and notifications["links"].get("next", None) and total_items > 50 + and page < total_pages ): next_page = generate_next_dict( "main.view_notifications", service_id, page, url_args diff --git a/tests/app/main/views/test_activity.py b/tests/app/main/views/test_activity.py index 9fd5118db..b9dc3d94a 100644 --- a/tests/app/main/views/test_activity.py +++ b/tests/app/main/views/test_activity.py @@ -562,7 +562,7 @@ def test_should_show_notifications_for_a_service_with_next_previous( "app.notification_api_client.get_notifications_for_service", return_value=notification_json( service_one["id"], rows=50, with_links=True - ) | {"total": 100}, + ) | {"total": 150}, ) page = client_request.get( "main.view_notifications", @@ -597,6 +597,35 @@ def test_should_show_notifications_for_a_service_with_next_previous( assert "page 1" in prev_page_link.text.strip() +def test_doesnt_show_next_button_on_last_page( + client_request, + service_one, + active_user_with_permissions, + mock_get_service_statistics, + mock_get_service_data_retention, + mock_get_no_api_keys, + mocker, +): + mocker.patch( + "app.notification_api_client.get_notifications_for_service", + return_value=notification_json( + service_one["id"], rows=50, with_links=True + ) | {"total": 100}, + ) + page = client_request.get( + "main.view_notifications", + service_id=service_one["id"], + message_type="sms", + page=2, + ) + + next_page_link = page.find("a", {"rel": "next"}) + prev_page_link = page.find("a", {"rel": "previous"}) + + assert next_page_link is None + assert prev_page_link is not None + + def test_doesnt_show_pagination_when_50_or_fewer_items( client_request, service_one,