feat(pagination): add pagination to the entries page since the list was getting too huge

config/settings/base.py

@@ -264,6 +264,10 @@
     },
 }
 
+# Pagination
+# -------------------------------------------------------------------------------
+PAGINATION_SIZE = 100
+
 # django-rest-framework
 # -------------------------------------------------------------------------------
 # django-rest-framework - https://www.django-rest-framework.org/api-guide/settings/
@@ -277,6 +281,8 @@
     "DEFAULT_PERMISSION_CLASSES": ("rest_framework.permissions.IsAuthenticated",),
     "TEST_REQUEST_DEFAULT_FORMAT": "json",
     "DEFAULT_SCHEMA_CLASS": "drf_spectacular.openapi.AutoSchema",
+    "DEFAULT_PAGINATION_CLASS": "rest_framework.pagination.PageNumberPagination",
+    "PAGE_SIZE": PAGINATION_SIZE,
 }
 
 # django-cors-headers - https://github.com/adamchainz/django-cors-headers#setup

docs/decisions.md

@@ -82,3 +82,9 @@ If you want two or more instances of SCRAM to share data between themselves we h
 Honestly, step 3 is kind of gross and we realize this. We are probably looking at a task runner or something to handle
 this moving forward, but we needed to get this fixed in the meantime. Status can be tracked with
 [Github Issue 125](https://github.com/esnet-security/SCRAM/issues/125) 
+
+#### Entries Page
+We intentionally chose to only list the active entries. Our thinking is that the home page shows the most recent additions.
+Then, if you went to the entries page, it would be overwhelmingly huge to show all the historical entries including the 
+ones that timed out/were deactivated. If you wanted to know about a specific entry even if it were not currently active
+(to see the history of it say), you would likely be using the search anyway.

pyproject.toml

@@ -97,6 +97,7 @@ max-complexity = 7 # our current code adheres to this without too much effort
     "PLR6301", # could be a static method
     "S101", # use of assert
     "S106", # hardcoded password
+    "PLR2004" # magic value used in comparison
 ]
 "test.py" = [
     "S105", # hardcoded password as argument

scram/route_manager/tests/acceptance/steps/common.py

@@ -185,7 +185,7 @@ def update_object(context, model, value_from, value_to):
 def count_objects(context, model, num):
     """Count the number of objects of an arbitrary model."""
     objs = context.test.client.get(reverse(f"api:v1:{model.lower()}-list"))
-    context.test.assertEqual(len(objs.json()), num)
+    context.test.assertEqual(len(objs.json()["results"]), num)
 
 
 model_to_field_mapping = {"entry": "route"}
@@ -197,7 +197,7 @@ def check_object(context, value, model):
     objs = context.test.client.get(reverse(f"api:v1:{model.lower()}-list"))
 
     found = False
-    for obj in objs.json():
+    for obj in objs.json()["results"]:
         # For some models, we need to look at a different field.
         model = model_to_field_mapping.get(model.lower(), model.lower())
         if obj[model].lower() == value.lower():

scram/route_manager/tests/acceptance/steps/ip.py

@@ -13,7 +13,7 @@ def check_route(context, route, model):
     ip_target = ipaddress.ip_address(route)
 
     ip_found = False
-    for obj in objs.json():
+    for obj in objs.json()["results"]:
         net = ipaddress.ip_network(obj["route"])
         if ip_target in net:
             ip_found = True

scram/route_manager/tests/test_autocreate_admin.py

@@ -17,13 +17,13 @@ def test_autocreate_admin(settings):
     settings.AUTOCREATE_ADMIN = True
     client = Client()
     response = client.get(reverse("route_manager:home"))
-    assert response.status_code == 200  # noqa: PLR2004
+    assert response.status_code == 200
     assert User.objects.count() == 1
     user = User.objects.get(username="admin")
     assert user.is_superuser
     assert user.email == "admin@example.com"
     messages = list(get_messages(response.wsgi_request))
-    assert len(messages) == 2  # noqa: PLR2004
+    assert len(messages) == 2
     assert messages[0].level == LEVEL_SUCCESS
     assert messages[1].level == LEVEL_INFO
 
@@ -34,7 +34,7 @@ def test_autocreate_admin_disabled(settings):
     settings.AUTOCREATE_ADMIN = False
     client = Client()
     response = client.get(reverse("route_manager:home"))
-    assert response.status_code == 200  # noqa: PLR2004
+    assert response.status_code == 200
     assert User.objects.count() == 0
 
 
@@ -45,6 +45,6 @@ def test_autocreate_admin_existing_user(settings):
     User.objects.create_user("testuser", "test@example.com", "password")
     client = Client()
     response = client.get(reverse("route_manager:home"))
-    assert response.status_code == 200  # noqa: PLR2004
+    assert response.status_code == 200
     assert User.objects.count() == 1
     assert not User.objects.filter(username="admin").exists()

scram/route_manager/tests/test_pagination.py

@@ -0,0 +1,139 @@
+"""Define simple tests for pagination."""
+
+import pytest
+from django.conf import settings
+from django.contrib.auth import get_user_model
+from django.test import TestCase, override_settings
+from django.urls import reverse
+from faker import Faker
+from faker.providers import internet
+
+from scram.route_manager.models import ActionType, Entry, Route
+
+
+@pytest.mark.django_db
+class TestEntriesListView(TestCase):
+    """Test to make sure our pagination and related scaffolding work."""
+
+    TEST_PAGINATION_SIZE = 5
+
+    def setUp(self):
+        """Set up the test environment."""
+        self.fake = Faker()
+        self.fake.add_provider(internet)
+        get_user_model().objects.create_user(username="testuser", password="testpass123")
+
+        self.atype1 = ActionType.objects.create(name="Type1", available=True)
+        self.atype2 = ActionType.objects.create(name="Type2", available=True)
+        self.atype3 = ActionType.objects.create(name="Type3", available=False)
+
+        # Create enough entries to test pagination
+        created_routes = Route.objects.bulk_create([
+            Route(route=self.fake.unique.ipv4_public()) for x in range(self.TEST_PAGINATION_SIZE + 3)
+        ])
+        entries_type1 = Entry.objects.bulk_create([
+            Entry(route=route, actiontype=self.atype1, is_active=True) for route in created_routes
+        ])
+
+        # Create a second type of entries to test filtering per actiontype
+        created_routes = Route.objects.bulk_create([Route(route=self.fake.unique.ipv4_public()) for x in range(3)])
+        entries_type2 = Entry.objects.bulk_create([
+            Entry(route=route, actiontype=self.atype2, is_active=True) for route in created_routes
+        ])
+
+        # Create inactive entries to test filtering by available actiontypes
+        created_routes = Route.objects.bulk_create([Route(route=self.fake.unique.ipv4_public()) for x in range(3)])
+        Entry.objects.bulk_create([
+            Entry(route=route, actiontype=self.atype1, is_active=False) for route in created_routes
+        ])
+
+        # Create entries for an invalid actiontype to test that
+        created_routes = Route.objects.bulk_create([Route(route=self.fake.unique.ipv4_public()) for x in range(3)])
+        Entry.objects.bulk_create([
+            Entry(route=route, actiontype=self.atype3, is_active=False) for route in created_routes
+        ])
+
+        self.entries = {
+            "type1": entries_type1,
+            "type2": entries_type2,
+        }
+
+    def test_context(self):
+        """Test that the context structure is correctly filled out."""
+        self.client.login(username="testuser", password="testpass123")
+
+        url = reverse("route_manager:entry-list")
+        response = self.client.get(url)
+
+        assert response.status_code == 200
+        assert "entries" in response.context
+        entries_context = response.context["entries"]
+
+        assert self.atype1 in entries_context
+        assert self.atype2 in entries_context
+        assert self.atype3 not in entries_context
+
+    def test_filtering_entries_by_action_type(self):
+        """Test that our paginated output has entries for all available actiontypes in our paginated output."""
+        self.client.login(username="testuser", password="testpass123")
+
+        url = reverse("route_manager:entry-list")
+        response = self.client.get(url)
+
+        entries_context = response.context["entries"]
+
+        assert entries_context[self.atype1]["total"] == len(self.entries["type1"])
+        assert entries_context[self.atype2]["total"] == len(self.entries["type2"])
+
+    @override_settings(PAGINATION_SIZE=5)
+    def test_pagination(self):
+        """Test pagination when there's multiple action types."""
+        self.client.login(username="testuser", password="testpass123")
+
+        url = reverse("route_manager:entry-list")
+
+        response = self.client.get(url)
+        entries_context = response.context["entries"]
+
+        # First page should have PAGINATION_SIZE entries for actiontype with more entries than pagination size
+        assert len(entries_context[self.atype1]["objs"]) == settings.PAGINATION_SIZE
+        assert entries_context[self.atype1]["page_param"] == "page_type1"
+        assert str(entries_context[self.atype1]["page_number"]) == "1"
+
+        # First page should include all entries for actiontype with less entries than pagination size
+        assert len(entries_context[self.atype2]["objs"]) == len(self.entries["type2"])
+
+        # Second page should have the rest of the entries for actiontype with more entries than pagination size
+        page2_response = self.client.get(f"{url}?page_type1=2")
+        page2_context = page2_response.context["entries"]
+
+        assert str(page2_context[self.atype1]["page_number"]) == "2"
+        assert len(page2_context[self.atype1]["objs"]) == 3
+
+    @override_settings(PAGINATION_SIZE=TEST_PAGINATION_SIZE)
+    def test_invalid_page_handling(self):
+        """Test handling of invalid page numbers."""
+        self.client.login(username="testuser", password="testpass123")
+
+        url = reverse("route_manager:entry-list")
+        response = self.client.get(f"{url}?page_type1=999")
+
+        entries_context = response.context["entries"]
+
+        # Should default to page 1
+        assert entries_context[self.atype1]["objs"].number == 1
+
+    def test_multiple_page_parameters(self):
+        """Test that we can have separate pages when we have more than one actiontype."""
+        self.client.login(username="testuser", password="testpass123")
+
+        url = reverse("route_manager:entry-list")
+        response = self.client.get(f"{url}?page_type1=2&page_type2=1")
+
+        entries_context = response.context["entries"]
+
+        # Each type should have its own page number
+        assert str(entries_context[self.atype1]["page_number"]) == "2"
+        assert str(entries_context[self.atype2]["page_number"]) == "1"
+        assert "page_type1" in entries_context[self.atype1]["current_page_params"]
+        assert "page_type2" in entries_context[self.atype1]["current_page_params"]

scram/route_manager/tests/test_swagger.py

@@ -9,22 +9,22 @@ def test_swagger_api(client):
     """Test that the Swagger API endpoint returns a successful response."""
     url = reverse("swagger-ui")
     response = client.get(url)
-    assert response.status_code == 200  # noqa: PLR2004
+    assert response.status_code == 200
 
 
 @pytest.mark.django_db
 def test_redoc_api(client):
     """Test that the Redoc API endpoint returns a successful response."""
     url = reverse("redoc")
     response = client.get(url)
-    assert response.status_code == 200  # noqa: PLR2004
+    assert response.status_code == 200
 
 
 @pytest.mark.django_db
 def test_schema_api(client):
     """Test that the Schema API endpoint returns a successful response."""
     url = reverse("schema")
     response = client.get(url)
-    assert response.status_code == 200  # noqa: PLR2004
+    assert response.status_code == 200
     expected_strings = [b"/entries/", b"/actiontypes/", b"/ignore_entries/", b"/users/"]
     assert all(string in response.content for string in expected_strings)

scram/route_manager/views.py

@@ -13,6 +13,7 @@
 from django.contrib.auth import authenticate, login
 from django.contrib.auth.decorators import permission_required
 from django.contrib.auth.mixins import PermissionRequiredMixin
+from django.core.paginator import EmptyPage, PageNotAnInteger, Paginator
 from django.db import transaction
 from django.http import HttpResponse
 from django.shortcuts import redirect, render
@@ -191,15 +192,43 @@ class EntryListView(ListView):
 
     model = Entry
     template_name = "route_manager/entry_list.html"
+    context_object_name = "object_list"
+    paginate_by = settings.PAGINATION_SIZE
 
-    @staticmethod
-    def get_context_data(**kwargs):
-        """Group entries by action type."""
-        context = {"entries": {}}
-        for at in ActionType.objects.all():
-            queryset = Entry.objects.filter(actiontype=at).order_by("-pk")
-            context["entries"][at] = {
-                "objs": queryset,
+    def get_context_data(self, **kwargs):
+        """Add action type grouping to context with separate paginators."""
+        context = super().get_context_data(**kwargs)
+
+        current_page_params = {}
+        for key, value in self.request.GET.items():
+            if key.startswith("page_"):
+                current_page_params[key] = value
+
+        entries_by_type = {}
+
+        # Get all available action types
+        for at in ActionType.objects.filter(available=True):
+            queryset = Entry.objects.filter(actiontype=at, is_active=True).order_by("-pk")
+
+            # Create a paginator for this action type
+            paginator = Paginator(queryset, settings.PAGINATION_SIZE)
+
+            # Get page number from request with a unique parameter name per type
+            page_param = f"page_{at.name.lower()}"
+            page_number = self.request.GET.get(page_param, 1)
+
+            try:
+                page_obj = paginator.page(page_number)
+            except (PageNotAnInteger, EmptyPage):
+                page_obj = paginator.page(1)
+
+            entries_by_type[at] = {
                 "total": queryset.count(),
+                "objs": page_obj,
+                "page_param": page_param,
+                "page_number": page_number,
+                "current_page_params": current_page_params.copy(),
             }
+
+        context["entries"] = entries_by_type
         return context