mirror of
https://github.com/GSA/notifications-admin.git
synced 2026-06-07 06:50:56 -04:00
Merge pull request #2782 from GSA/2257-there-shouldnt-be-a-next-in-pagination-if-theres-less-than-50-items-in-a-table
fixing pagination and download links along with tests
This commit is contained in:
@@ -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/<uuid:service_id>")
|
||||
@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,13 @@ def handle_pagination(jobs, service_id, page):
|
||||
if page > 1
|
||||
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 jobs.get("links", {}).get("next")
|
||||
if has_next_link and total_items > 50 and page < total_pages
|
||||
else None
|
||||
)
|
||||
pagination = generate_pagination_pages(
|
||||
|
||||
@@ -253,7 +253,15 @@ 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)
|
||||
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
|
||||
)
|
||||
|
||||
@@ -126,20 +126,33 @@
|
||||
</div>
|
||||
{{show_pagination}}
|
||||
{% if current_user.has_permissions(ServicePermission.VIEW_ACTIVITY) %}
|
||||
<h2 class="line-height-sans-2 margin-bottom-0 margin-top-4">Download recent reports</h2>
|
||||
<p class="font-body-sm">
|
||||
<a href="{{ download_link_one_day }}" download="download" class="usa-link">Download all data last 24 hours (<abbr title="Comma separated values">CSV</abbr>)</a>
|
||||
</p>
|
||||
<p class="font-body-sm">
|
||||
<a href="{{ download_link_three_day }}" download="download" class="usa-link">Download all data last 3 days (<abbr title="Comma separated values">CSV</abbr>)</a>
|
||||
 
|
||||
</p>
|
||||
<p class="font-body-sm">
|
||||
<a href="{{ download_link_five_day }}" download="download" class="usa-link">Download all data last 5 days (<abbr title="Comma separated values">CSV</abbr>)</a>
|
||||
</p>
|
||||
<p class="font-body-sm">
|
||||
<a href="{{ download_link_seven_day }}" download="download" class="usa-link">Download all data last 7 days (<abbr title="Comma separated values">CSV</abbr>)</a>
|
||||
</p>
|
||||
{% if has_any_download_data %}
|
||||
<h2 class="line-height-sans-2 margin-bottom-0 margin-top-4">Download recent reports</h2>
|
||||
{% if has_1_day_data %}
|
||||
<p class="font-body-sm">
|
||||
<a href="{{ download_link_one_day }}" download="download" class="usa-link">Download all data last 24 hours (<abbr title="Comma separated values">CSV</abbr>)</a>
|
||||
</p>
|
||||
{% endif %}
|
||||
{% if has_3_day_data %}
|
||||
<p class="font-body-sm">
|
||||
<a href="{{ download_link_three_day }}" download="download" class="usa-link">Download all data last 3 days (<abbr title="Comma separated values">CSV</abbr>)</a>
|
||||
 
|
||||
</p>
|
||||
{% endif %}
|
||||
{% if has_5_day_data %}
|
||||
<p class="font-body-sm">
|
||||
<a href="{{ download_link_five_day }}" download="download" class="usa-link">Download all data last 5 days (<abbr title="Comma separated values">CSV</abbr>)</a>
|
||||
</p>
|
||||
{% endif %}
|
||||
{% if has_7_day_data %}
|
||||
<p class="font-body-sm">
|
||||
<a href="{{ download_link_seven_day }}" download="download" class="usa-link">Download all data last 7 days (<abbr title="Comma separated values">CSV</abbr>)</a>
|
||||
</p>
|
||||
{% endif %}
|
||||
{% else %}
|
||||
<h2 class="line-height-sans-2 margin-bottom-0 margin-top-4">Download recent reports</h2>
|
||||
<p class="font-body-sm">No recent activity to download. Download links will appear when jobs are available.</p>
|
||||
{% endif %}
|
||||
{% endif %}
|
||||
</div>
|
||||
{% endblock %}
|
||||
|
||||
@@ -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,
|
||||
@@ -284,6 +285,93 @@ 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
|
||||
}
|
||||
|
||||
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,
|
||||
@@ -465,12 +553,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": 150},
|
||||
)
|
||||
page = client_request.get(
|
||||
"main.view_notifications",
|
||||
service_id=service_one["id"],
|
||||
@@ -504,16 +597,76 @@ 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_next_button_on_last_page(
|
||||
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"],
|
||||
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,
|
||||
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=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"],
|
||||
|
||||
@@ -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")
|
||||
|
||||
Reference in New Issue
Block a user