From a64364e1adb1bf5b28f2e047b4fb55c7d8e9abbf Mon Sep 17 00:00:00 2001
From: Mike Cantelon <mcantelon@gmail.com>
Date: Wed, 11 Oct 2023 00:32:44 -0700
Subject: [PATCH] Use server-side paging in aips page. (#209) (#215)

To avoid slow page responses when there are a large amount of AIPs use
server-side paging to limit the number of AIP HTML sent to the browser.
---
 AIPscan/Reporter/helpers.py            | 11 ++++++
 AIPscan/Reporter/request_params.py     |  1 +
 AIPscan/Reporter/templates/aips.html   | 53 ++++++++++++++++++--------
 AIPscan/Reporter/tests/test_helpers.py | 36 +++++++++++++++++
 AIPscan/Reporter/tests/test_views.py   | 26 +++++++++++++
 AIPscan/Reporter/views.py              | 42 +++++++++++++++++---
 6 files changed, 147 insertions(+), 22 deletions(-)

diff --git a/AIPscan/Reporter/helpers.py b/AIPscan/Reporter/helpers.py
index 17c38048..8de8ea3a 100644
--- a/AIPscan/Reporter/helpers.py
+++ b/AIPscan/Reporter/helpers.py
@@ -145,3 +145,14 @@ def get_premis_xml_lines(file_object):
         premis_xml_lines = file_object.premis_object.split("\n")
 
     return premis_xml_lines
+
+
+def calculate_paging_window(pagination):
+    first_item = ((pagination.page - 1) * pagination.per_page) + 1
+    last_item = pagination.page * pagination.per_page
+
+    return first_item, min(last_item, pagination.total)
+
+
+def remove_dict_none_values(values):
+    return {index: "" if value is None else value for (index, value) in values.items()}
diff --git a/AIPscan/Reporter/request_params.py b/AIPscan/Reporter/request_params.py
index 732f048c..a2027b1b 100644
--- a/AIPscan/Reporter/request_params.py
+++ b/AIPscan/Reporter/request_params.py
@@ -11,6 +11,7 @@
 PUID = "puid"
 STORAGE_LOCATION_ID = "storage_location"
 STORAGE_SERVICE_ID = "amss_id"
+PAGE = "page"
 
 START_DATE = "start_date"
 END_DATE = "end_date"
diff --git a/AIPscan/Reporter/templates/aips.html b/AIPscan/Reporter/templates/aips.html
index c456f527..2f6e8c67 100644
--- a/AIPscan/Reporter/templates/aips.html
+++ b/AIPscan/Reporter/templates/aips.html
@@ -2,7 +2,7 @@
 
 {% block content %}
 
-<div class="alert alert-header"><strong>AIPs ({{ aips|length }})</strong></div>
+<div class="alert alert-header"><strong>AIPs ({{ pager.items|length }})</strong></div>
 
 {% if storage_services %}
 
@@ -42,9 +42,9 @@
     </div>
   </div>
 
-  {% if aips %}
+  {% if pager.items %}
 
-    <table id="aiptable" class="table table-striped table-bordered">
+    <table id="aipstable" class="table table-striped table-bordered">
       <thead>
       <tr>
       <th><strong>Transfer name</strong></th>
@@ -54,19 +54,17 @@
       <th><strong>Preservation copies</strong></th>
       <th><strong>Action</strong></th>
       </tr>
-      </thead>
-      {% if aips %}
-        {% for aip in aips %}
-          <tr>
-            <td>{{ aip.transfer_name }}</td>
-            <td>{{ aip.uuid }}</td>
-            <td>{{ aip.create_date }}</td>
-            <td>{{ aip.original_file_count }}</td>
-            <td>{{ aip.preservation_file_count }}</td>
-            <td><a href='{{ url_for("reporter.view_aip", aip_id = aip.id) }}'><button type="button" class="btn btn-info">View</button></a></td>
-          </tr>
-        {% endfor %}
-      {% endif %}
+
+      {% for aip in pager.items %}
+        <tr>
+          <td>{{ aip.transfer_name }}</td>
+          <td>{{ aip.uuid }}</td>
+          <td>{{ aip.create_date }}</td>
+          <td>{{ aip.original_file_count }}</td>
+          <td>{{ aip.preservation_file_count }}</td>
+          <td><a href='{{ url_for("reporter.view_aip", aip_id = aip.id) }}'><button type="button" class="btn btn-info">View</button></a></td>
+        </tr>
+      {% endfor %}
     </table>
 
   {% else %}
@@ -85,6 +83,29 @@
 
 {% endif %}
 
+{% if pager.total %}
+
+  <div class="dataTables_wrapper">
+    <div class="row">
+      <div class="col-sm-12 col-md-5">
+        <div class="dataTables_info" id="aiptable_info" role="status" aria-live="polite">Showing {{ first_item }} to {{ last_item }} of {{ pager.total }} entries</div>
+      </div>
+      <div class="col-sm-12 col-md-7">
+        <div class="dataTables_paginate paging_full_numbers" id="aiptable_paginate">
+          <ul class="pagination">
+            <li class="paginate_button page-item first {% if pager.page == 1 %}disabled{% endif %}"><a href="{{ url_for('reporter.view_aips', page=1, **state_query_params) }}" class="page-link">First</a></li>
+            <li class="paginate_button page-item previous {% if pager.prev_num is none %}disabled{% endif %}"><a href="{{ url_for('reporter.view_aips', page=pager.prev_num, **state_query_params) }}" class="page-link">Previous</a></li>
+            <li class="paginate_button page-item active"><a href="#" class="page-link">{{ pager.page }}</a></li>
+            <li class="paginate_button page-item next {% if pager.next_num is none %}disabled{% endif %}"><a href="{{ url_for('reporter.view_aips', page=pager.next_num, **state_query_params) }}" class="page-link">Next</a></li>
+            <li class="paginate_button page-item last {% if pager.page == pager.pages %}disabled{% endif %}"><a href="{{ url_for('reporter.view_aips', page=pager.pages, **state_query_params) }}" class="page-link">Last</a></li>
+          </ul>
+        </div>
+      </div>
+    </div>
+  </div>
+
+{% endif %}
+
 <script>
 $(document).ready(function(){
   var storageServiceId = $('#ss').val();
diff --git a/AIPscan/Reporter/tests/test_helpers.py b/AIPscan/Reporter/tests/test_helpers.py
index e7700c1b..3b7bfb6d 100644
--- a/AIPscan/Reporter/tests/test_helpers.py
+++ b/AIPscan/Reporter/tests/test_helpers.py
@@ -129,3 +129,39 @@ def test_get_premis_xml_lines():
     lines = helpers.get_premis_xml_lines(file_)
 
     assert len(lines) == 2
+
+
+@pytest.mark.parametrize(
+    "paging",
+    [
+        # Test paging window at start of results
+        {"per_page": 5, "total": 17, "page": 1, "first_item": 1, "last_item": 5},
+        # Test paging window on an arbitrary page of results
+        {"per_page": 5, "total": 17, "page": 3, "first_item": 11, "last_item": 15},
+        # Test paging window with last item being set to the total of items
+        {"per_page": 5, "total": 17, "page": 4, "first_item": 16, "last_item": 17},
+        # Test paging window at start of results with incomplete page
+        {"per_page": 7, "total": 4, "page": 1, "first_item": 1, "last_item": 4},
+    ],
+)
+def test_calculate_paging_window(paging):
+    class MockPagination(object):
+        pass
+
+    pagination = MockPagination()
+    pagination.page = paging["page"]
+    pagination.per_page = paging["per_page"]
+    pagination.total = paging["total"]
+
+    first_item, last_item = helpers.calculate_paging_window(pagination)
+
+    assert first_item == paging["first_item"]
+    assert last_item == paging["last_item"]
+
+
+def test_remove_dict_none_values():
+    test_values = {"foo": "bar", "nada": None}
+
+    querystring = helpers.remove_dict_none_values(test_values)
+
+    assert querystring == {"foo": "bar", "nada": ""}
diff --git a/AIPscan/Reporter/tests/test_views.py b/AIPscan/Reporter/tests/test_views.py
index e600bf0c..f765c69f 100644
--- a/AIPscan/Reporter/tests/test_views.py
+++ b/AIPscan/Reporter/tests/test_views.py
@@ -1,6 +1,10 @@
+import pytest
 from flask import current_app
 from werkzeug.datastructures import Headers
 
+from AIPscan import test_helpers
+from AIPscan.Reporter import views
+
 
 def test_download_mets(app_with_populated_files, mocker):
     # Mock storage server API request response
@@ -32,3 +36,25 @@ class MockResponse(object):
 
         assert response.data == return_response.content
         assert response.status_code == return_response.status_code
+
+
+@pytest.mark.parametrize("page,pager_page", [(1, 1), ("bad", 1)])
+def test_get_aip_pager(app_instance, mocker, page, pager_page):
+    paginate_mock = mocker.Mock()
+    filter_by_mock = mocker.Mock()
+    filter_by_mock.paginate.return_value = paginate_mock
+
+    query_mock = mocker.Mock()
+    query_mock.filter_by.return_value = filter_by_mock
+
+    storage_service = test_helpers.create_test_storage_service()
+    storage_location = test_helpers.create_test_storage_location(
+        storage_service_id=storage_service.id
+    )
+    pager = views.get_aip_pager(page, 2, storage_service, storage_location)
+
+    assert pager.page == pager_page
+    assert pager.pages == 0
+    assert pager.per_page == 2
+    assert pager.prev_num is None
+    assert pager.next_num is None
diff --git a/AIPscan/Reporter/views.py b/AIPscan/Reporter/views.py
index 66b4273d..cedf1003 100644
--- a/AIPscan/Reporter/views.py
+++ b/AIPscan/Reporter/views.py
@@ -39,7 +39,11 @@
     request_params,
     sort_puids,
 )
-from AIPscan.Reporter.helpers import get_premis_xml_lines
+from AIPscan.Reporter.helpers import (
+    calculate_paging_window,
+    get_premis_xml_lines,
+    remove_dict_none_values,
+)
 
 
 def _get_storage_service(storage_service_id):
@@ -73,6 +77,24 @@ def storage_locations_with_aips(storage_locations):
     return [loc for loc in storage_locations if loc.aips]
 
 
+def get_aip_pager(page, per_page, storage_service, storage_location):
+    try:
+        page = int(page)
+    except ValueError:
+        page = 1
+
+    pager = AIP.query.filter_by(storage_service_id=storage_service.id).paginate(
+        page=page, per_page=per_page, error_out=False
+    )
+
+    if storage_location:
+        pager = pager.query.filter_by(storage_location_id=storage_location.id).paginate(
+            page=page, per_page=per_page, error_out=False
+        )
+
+    return pager
+
+
 @reporter.route("/aips/", methods=["GET"])
 def view_aips():
     """Overview of AIPs in given Storage Service and Location."""
@@ -85,10 +107,15 @@ def view_aips():
     except Exception as e:
         print(e)
 
-    aips = AIP.query.filter_by(storage_service_id=storage_service.id)
-    if storage_location:
-        aips = aips.filter_by(storage_location_id=storage_location_id)
-    aips = aips.all()
+    page = request.args.get(request_params.PAGE, default="1")
+    pager = get_aip_pager(page, 10, storage_service, storage_location)
+
+    first_item, last_item = calculate_paging_window(pager)
+
+    state_query_params = {
+        request_params.STORAGE_SERVICE_ID: storage_service_id,
+        request_params.STORAGE_LOCATION_ID: storage_location_id,
+    }
 
     return render_template(
         "aips.html",
@@ -98,7 +125,10 @@ def view_aips():
             storage_service.storage_locations
         ),
         storage_location=storage_location,
-        aips=aips,
+        pager=pager,
+        first_item=first_item,
+        last_item=last_item,
+        state_query_params=remove_dict_none_values(state_query_params),
     )