From 463be2a46ba105c247e83b32cab243fcaeadbc81 Mon Sep 17 00:00:00 2001 From: Beverly Nguyen Date: Wed, 6 Aug 2025 13:24:28 -0700 Subject: [PATCH 1/6] add quick filters for easy data consumption without scrolling through many pages --- app/assets/javascripts/scrollPosition.js | 13 ++++++++ app/main/views/activity.py | 17 ++++++++++- .../views/activity/all-activity.html | 30 +++++++++++++++++++ gulpfile.js | 1 + 4 files changed, 60 insertions(+), 1 deletion(-) create mode 100644 app/assets/javascripts/scrollPosition.js diff --git a/app/assets/javascripts/scrollPosition.js b/app/assets/javascripts/scrollPosition.js new file mode 100644 index 000000000..520e96992 --- /dev/null +++ b/app/assets/javascripts/scrollPosition.js @@ -0,0 +1,13 @@ +document.querySelectorAll('.usa-button-group a, .usa-pagination a').forEach(function(button) { + button.addEventListener('click', function() { + sessionStorage.setItem('scrollPosition', window.pageYOffset); + }); +}); + +document.addEventListener('DOMContentLoaded', function() { + var scrollPosition = sessionStorage.getItem('scrollPosition'); + if (scrollPosition !== null) { + window.scrollTo(0, parseInt(scrollPosition)); + sessionStorage.removeItem('scrollPosition'); + } +}); diff --git a/app/main/views/activity.py b/app/main/views/activity.py index 1d6d12d98..0a0b4f4c0 100644 --- a/app/main/views/activity.py +++ b/app/main/views/activity.py @@ -44,7 +44,22 @@ def get_download_availability(service_id): def all_jobs_activity(service_id): service_data_retention_days = 7 page = get_page_from_request() - jobs = job_api_client.get_page_of_jobs(service_id, page=page) + + filter_type = request.args.get('filter') + + limit_days = None + if filter_type == '24hours': + limit_days = 1 + elif filter_type == '3days': + limit_days = 3 + elif filter_type == '7days': + limit_days = 7 + + if limit_days: + jobs = job_api_client.get_page_of_jobs(service_id, page=page, limit_days=limit_days) + else: + jobs = job_api_client.get_page_of_jobs(service_id, page=page) + all_jobs_dict = generate_job_dict(jobs) prev_page, next_page, pagination = handle_pagination(jobs, service_id, page) message_type = ("sms",) diff --git a/app/templates/views/activity/all-activity.html b/app/templates/views/activity/all-activity.html index 94209a38c..675b7c478 100644 --- a/app/templates/views/activity/all-activity.html +++ b/app/templates/views/activity/all-activity.html @@ -58,6 +58,36 @@

All activity

All activity

Sent jobs

+
+
+ +
+
diff --git a/gulpfile.js b/gulpfile.js index 087039a13..78b76640f 100644 --- a/gulpfile.js +++ b/gulpfile.js @@ -81,6 +81,7 @@ const javascripts = () => { paths.src + 'javascripts/sidenav.js', paths.src + 'javascripts/validation.js', paths.src + 'javascripts/socketio.js', + paths.src + 'javascripts/scrollPosition.js', ]) .pipe(plugins.prettyerror()) .pipe( From 511b0b8b9ebd2fed2be4572460e2c0ebe361ddc4 Mon Sep 17 00:00:00 2001 From: Beverly Nguyen Date: Wed, 6 Aug 2025 13:36:28 -0700 Subject: [PATCH 2/6] add in test and fix isort --- app/main/views/activity.py | 12 ++++---- gunicorn_config.py | 1 + notifications_python_client/base.py | 1 - tests/app/main/views/test_jobs_activity.py | 33 ++++++++++++++++++++++ 4 files changed, 41 insertions(+), 6 deletions(-) diff --git a/app/main/views/activity.py b/app/main/views/activity.py index 0a0b4f4c0..d1bddf351 100644 --- a/app/main/views/activity.py +++ b/app/main/views/activity.py @@ -45,18 +45,20 @@ def all_jobs_activity(service_id): service_data_retention_days = 7 page = get_page_from_request() - filter_type = request.args.get('filter') + filter_type = request.args.get("filter") limit_days = None - if filter_type == '24hours': + if filter_type == "24hours": limit_days = 1 - elif filter_type == '3days': + elif filter_type == "3days": limit_days = 3 - elif filter_type == '7days': + elif filter_type == "7days": limit_days = 7 if limit_days: - jobs = job_api_client.get_page_of_jobs(service_id, page=page, limit_days=limit_days) + jobs = job_api_client.get_page_of_jobs( + service_id, page=page, limit_days=limit_days + ) else: jobs = job_api_client.get_page_of_jobs(service_id, page=page) diff --git a/gunicorn_config.py b/gunicorn_config.py index 65494fd14..345abc4f3 100644 --- a/gunicorn_config.py +++ b/gunicorn_config.py @@ -2,6 +2,7 @@ import multiprocessing import os import sys import traceback + import gunicorn # Let gunicorn figure out the right number of workers diff --git a/notifications_python_client/base.py b/notifications_python_client/base.py index 9c010118a..5b333d97e 100644 --- a/notifications_python_client/base.py +++ b/notifications_python_client/base.py @@ -1,7 +1,6 @@ import json import logging import urllib.parse - from os import getenv import requests diff --git a/tests/app/main/views/test_jobs_activity.py b/tests/app/main/views/test_jobs_activity.py index c2a6bcd3e..a965faeaa 100644 --- a/tests/app/main/views/test_jobs_activity.py +++ b/tests/app/main/views/test_jobs_activity.py @@ -1,3 +1,4 @@ +import pytest from bs4 import BeautifulSoup from app.utils.pagination import get_page_from_request @@ -211,3 +212,35 @@ def test_all_activity_pagination(client_request, mocker): assert ( pagination_texts == expected_pagination_texts ), 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), +]) +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( + "app.job_api_client.get_page_of_jobs", return_value=MOCK_JOBS + ) + mocker.patch("app.job_api_client.get_immediate_jobs", return_value=[]) + + 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 + ) + + assert response.status_code == 200 + assert "All activity" in response.text + + if expected_limit_days: + mock_get_page_of_jobs.assert_any_call( + SERVICE_ONE_ID, page=current_page, limit_days=expected_limit_days + ) + else: + mock_get_page_of_jobs.assert_any_call(SERVICE_ONE_ID, page=current_page) From d774ab89d1a421b242664b2f49c90c57561a2733 Mon Sep 17 00:00:00 2001 From: Beverly Nguyen Date: Thu, 7 Aug 2025 10:06:46 -0700 Subject: [PATCH 3/6] fix syntax --- tests/app/main/views/test_jobs_activity.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/app/main/views/test_jobs_activity.py b/tests/app/main/views/test_jobs_activity.py index a965faeaa..305d9fdeb 100644 --- a/tests/app/main/views/test_jobs_activity.py +++ b/tests/app/main/views/test_jobs_activity.py @@ -214,7 +214,7 @@ 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(("filter_type", "expected_limit_days"), [ ("24hours", 1), ("3days", 3), ("7days", 7), From 0a25d75146bdd0ed3c0fe45805dc5c54db488b9d Mon Sep 17 00:00:00 2001 From: Beverly Nguyen Date: Thu, 7 Aug 2025 18:31:53 -0700 Subject: [PATCH 4/6] fix filtering to use processing_started time --- app/main/views/activity.py | 11 ++++++++--- app/notify_client/job_api_client.py | 7 +++++-- app/templates/views/activity/all-activity.html | 2 +- 3 files changed, 14 insertions(+), 6 deletions(-) diff --git a/app/main/views/activity.py b/app/main/views/activity.py index 43f51dfa7..0021f4a84 100644 --- a/app/main/views/activity.py +++ b/app/main/views/activity.py @@ -57,7 +57,7 @@ def all_jobs_activity(service_id): if limit_days: jobs = job_api_client.get_page_of_jobs( - service_id, page=page, limit_days=limit_days + service_id, page=page, limit_days=limit_days, use_processing_time=True ) else: jobs = job_api_client.get_page_of_jobs(service_id, page=page) @@ -109,8 +109,13 @@ def all_jobs_activity(service_id): def handle_pagination(jobs, service_id, page): if page is None: abort(404, "Invalid page argument ({}).".format(request.args.get("page"))) + + url_args = {} + if request.args.get("filter"): + url_args["filter"] = request.args.get("filter") + prev_page = ( - generate_previous_dict("main.all_jobs_activity", service_id, page) + generate_previous_dict("main.all_jobs_activity", service_id, page, url_args) if page > 1 else None ) @@ -119,7 +124,7 @@ def handle_pagination(jobs, service_id, page): 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) + generate_next_dict("main.all_jobs_activity", service_id, page, url_args) if has_next_link and total_items > 50 and page < total_pages else None ) diff --git a/app/notify_client/job_api_client.py b/app/notify_client/job_api_client.py index 0e04cfc74..9de84f0d4 100644 --- a/app/notify_client/job_api_client.py +++ b/app/notify_client/job_api_client.py @@ -32,12 +32,14 @@ class JobApiClient(NotifyAdminAPIClient): return job - def get_jobs(self, service_id, *, limit_days=None, statuses=None, page=1): + 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 if statuses is not None: params["statuses"] = ",".join(statuses) + if use_processing_time: + params["use_processing_time"] = "true" job = self.get(url=f"/service/{service_id}/job", params=params) return job @@ -61,12 +63,13 @@ class JobApiClient(NotifyAdminAPIClient): if job["job_status"] != JobStatus.CANCELLED ) - def get_page_of_jobs(self, service_id, *, page, statuses=None, limit_days=None): + 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, page=page, limit_days=limit_days, + use_processing_time=use_processing_time, ) def get_immediate_jobs(self, service_id): diff --git a/app/templates/views/activity/all-activity.html b/app/templates/views/activity/all-activity.html index 2a3daac97..edbaf5f04 100644 --- a/app/templates/views/activity/all-activity.html +++ b/app/templates/views/activity/all-activity.html @@ -30,7 +30,7 @@ {% else %}
  • - + {{ page }}
    • From 2853a49f0cf18c5de407bb0ccaccf412807cc581 Mon Sep 17 00:00:00 2001 From: Beverly Nguyen Date: Thu, 7 Aug 2025 18:43:04 -0700 Subject: [PATCH 5/6] clean up --- app/main/views/activity.py | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/app/main/views/activity.py b/app/main/views/activity.py index 0021f4a84..89097db74 100644 --- a/app/main/views/activity.py +++ b/app/main/views/activity.py @@ -39,12 +39,7 @@ def get_download_availability(service_id): } -@main.route("/activity/services/") -@user_has_permissions(ServicePermission.VIEW_ACTIVITY) -def all_jobs_activity(service_id): - service_data_retention_days = 7 - page = get_page_from_request() - +def get_filtered_jobs(service_id, page): filter_type = request.args.get("filter") limit_days = None @@ -56,11 +51,20 @@ def all_jobs_activity(service_id): limit_days = 7 if limit_days: - jobs = job_api_client.get_page_of_jobs( + return job_api_client.get_page_of_jobs( service_id, page=page, limit_days=limit_days, use_processing_time=True ) else: - jobs = job_api_client.get_page_of_jobs(service_id, page=page) + return job_api_client.get_page_of_jobs(service_id, page=page) + + +@main.route("/activity/services/") +@user_has_permissions(ServicePermission.VIEW_ACTIVITY) +def all_jobs_activity(service_id): + service_data_retention_days = 7 + page = get_page_from_request() + + jobs = get_filtered_jobs(service_id, page) all_jobs_dict = generate_job_dict(jobs) prev_page, next_page, pagination = handle_pagination(jobs, service_id, page) From dbfeb32eb2d11a354374cefb4d85b29b63b2f6a3 Mon Sep 17 00:00:00 2001 From: Beverly Nguyen Date: Thu, 7 Aug 2025 18:49:09 -0700 Subject: [PATCH 6/6] 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)
    Table showing all sent jobs for this service