feat: add pagination to change requests list with envelope response

backend/openapi.yml

@@ -4747,9 +4747,26 @@ paths:
           content:
             application/json:
               schema:
-                type: array
-                items:
-                  $ref: "#/components/schemas/GenericChangeRequest"
+                type: object
+                properties:
+                  data:
+                    type: array
+                    items:
+                      $ref: "#/components/schemas/GenericChangeRequest"
+                  count:
+                    type: integer
+                    description: Total number of change requests after permission filtering, before pagination
+                  limit:
+                    type: integer
+                    description: Maximum number of change requests returned
+                  offset:
+                    type: integer
+                    description: Number of change requests skipped
+                required:
+                  - data
+                  - count
+                  - limit
+                  - offset
         "400":
           description: Bad Request
         "401":

backend/ops_api/ops/resources/change_requests.py

@@ -52,13 +52,17 @@ def get(self) -> Response:
         }
 
         service = ChangeRequestService(current_app.db_session)
-        change_requests, _ = service.get_list(data=filters)
-
-        if change_requests:
-            response = make_response_with_headers(self._response_schema_collection.dump(change_requests))
-        else:
-            response = make_response_with_headers([], 200)
-        return response
+        change_requests, metadata = service.get_list(data=filters)
+
+        data = self._response_schema_collection.dump(change_requests) if change_requests else []
+        return make_response_with_headers(
+            {
+                "data": data,
+                "count": metadata["count"],
+                "limit": metadata["limit"],
+                "offset": metadata["offset"],
+            }
+        )
 
 
 class ChangeRequestItemAPI(BaseItemAPI):

backend/ops_api/ops/services/change_requests.py

@@ -117,13 +117,14 @@ def get_list(self, data: dict | None = None) -> tuple[list[ChangeRequest], dict
         reviewer_user_id = data.get("reviewer_user_id")
         if not reviewer_user_id:
             raise ValidationError({"reviewer_user_id": "This field is required."})
-        results = find_in_review_requests_by_user(
+        # Get in review users, provide default limit of 10 and offset of 0
+        page, total_count = find_in_review_requests_by_user(
             int(reviewer_user_id),
-            data.get("limit"),
-            data.get("offset"),
+            data.get("limit", 10),
+            data.get("offset", 0),
         )
 
-        return results, None
+        return page, {"count": total_count, "limit": data.get("limit"), "offset": data.get("offset")}
 
     # --- Review & Authorization Logic ---
 

backend/ops_api/ops/utils/change_requests_helpers.py

@@ -1,19 +1,22 @@
 from typing import Type
 
 from flask import current_app
-from sqlalchemy import or_, select
+from sqlalchemy import and_, func, or_, select
 from sqlalchemy.orm import with_polymorphic
 
 from models import (
+    CAN,
+    Agreement,
     AgreementChangeRequest,
+    BudgetLineItem,
     BudgetLineItemChangeRequest,
     BudgetLineItemStatus,
     ChangeRequest,
     ChangeRequestStatus,
     ChangeRequestType,
     Division,
+    Portfolio,
 )
-from ops_api.ops.utils.agreements_helpers import get_division_directors_for_agreement
 from ops_api.ops.utils.budget_line_items_helpers import (
     convert_BLI_status_name_to_pretty_string,
 )
@@ -69,30 +72,51 @@ def get_division_ids_user_can_review_for(user_id: int) -> set[int]:
     return {row[0] for row in current_app.db_session.execute(stmt).all()}
 
 
+def _get_reviewable_agreement_ids_subquery(user_id: int):
+    """Subquery returning agreement IDs where the user is a division director or deputy via BLI→CAN→Portfolio→Division."""
+    return (
+        select(Agreement.id)
+        .distinct()
+        .join(Agreement.budget_line_items)
+        .join(BudgetLineItem.can)
+        .join(CAN.portfolio)
+        .join(Portfolio.division)
+        .where(
+            or_(
+                Division.division_director_id == user_id,
+                Division.deputy_division_director_id == user_id,
+            )
+        )
+        .scalar_subquery()
+    )
+
+
 def find_in_review_requests_by_user(user_id: int, limit: int = 10, offset: int = 0):
-    # Prepare polymorphic loading to access subtype fields
     cr_poly = with_polymorphic(ChangeRequest, [AgreementChangeRequest, BudgetLineItemChangeRequest])
 
-    # Get explicit divisions
     reviewable_division_ids = get_division_ids_user_can_review_for(user_id)
+    reviewable_agreement_ids_subq = _get_reviewable_agreement_ids_subquery(user_id)
 
-    # Query all in-review change requests
-    stmt = select(cr_poly).where(cr_poly.status == ChangeRequestStatus.IN_REVIEW).limit(limit).offset(offset)
-
-    results = current_app.db_session.execute(stmt).scalars().all()
-    filtered_results = []
+    bli_filter = and_(
+        ChangeRequest.change_request_type == ChangeRequestType.BUDGET_LINE_ITEM_CHANGE_REQUEST,
+        BudgetLineItemChangeRequest.managing_division_id.in_(reviewable_division_ids),
+    )
+    agr_filter = and_(
+        ChangeRequest.change_request_type == ChangeRequestType.AGREEMENT_CHANGE_REQUEST,
+        AgreementChangeRequest.agreement_id.in_(reviewable_agreement_ids_subq),
+    )
 
-    # Filter results based on user's review permissions
-    for cr in results:
-        if isinstance(cr, BudgetLineItemChangeRequest):
-            if cr.managing_division_id in reviewable_division_ids:
-                filtered_results.append(cr)
-        elif isinstance(cr, AgreementChangeRequest):
-            division_directors, deputies = get_division_directors_for_agreement(cr.agreement)
-            if user_id in division_directors or user_id in deputies:
-                filtered_results.append(cr)
+    stmt = (
+        select(cr_poly)
+        .where(ChangeRequest.status == ChangeRequestStatus.IN_REVIEW)
+        .where(or_(bli_filter, agr_filter))
+        .order_by(ChangeRequest.id.desc())
+    )
 
-    return filtered_results
+    count_stmt = select(func.count()).select_from(stmt.subquery())
+    total_count = current_app.db_session.execute(count_stmt).scalar()
+    results = current_app.db_session.execute(stmt.limit(limit).offset(offset)).scalars().all()
+    return results, total_count
 
 
 def build_review_outcome_title_and_message(

backend/ops_api/tests/ops/agreement/test_agreement_change_requests.py

@@ -238,8 +238,8 @@ def test_update_procurement_shop_creates_change_request_e2e(
     )
 
     assert request.status_code == 200
-    assert len(request.json) == 1
-    assert matching_crs[0].id == request.json[0]["id"]
+    assert len(request.json["data"]) == 1
+    assert matching_crs[0].id == request.json["data"][0]["id"]
 
     # approve the change request
     response = division_director_auth_client.patch(

backend/ops_api/tests/ops/services/test_change_requests.py

@@ -94,10 +94,10 @@ def test_delete_not_found(service, mock_db_session):
 
 def test_get_list_uses_find_in_review_requests_by_user(service):
     with patch("ops_api.ops.services.change_requests.find_in_review_requests_by_user") as mock_find:
-        mock_find.return_value = [Mock()]
-        results, pagination = service.get_list({"reviewer_user_id": 1})
+        mock_find.return_value = [Mock()], 1
+        results, pagination = service.get_list({"reviewer_user_id": 1, "limit": 10, "offset": 0})
 
         assert isinstance(results, list)
         assert len(results) == 1
-        assert pagination is None
-        mock_find.assert_called_once_with(1, None, None)
+        assert pagination == {"count": 1, "limit": 10, "offset": 0}
+        mock_find.assert_called_once_with(1, 10, 0)

backend/ops_api/tests/ops/utils/test_change_requests_helpers.py

@@ -4,6 +4,7 @@
 
 from models import (
     AgreementChangeRequest,
+    BudgetLineItem,
     BudgetLineItemChangeRequest,
     BudgetLineItemStatus,
     ChangeRequest,
@@ -64,6 +65,45 @@ def test_build_approve_url_raises_on_unrecognized():
         cr_helpers.build_approve_url(cr, agreement_id=1, fe_url="http://localhost")
 
 
+def test_find_in_review_requests_by_user_with_pagination(loaded_db, app_ctx):
+    dave_director = loaded_db.get(User, 522)  # Dave Director — division 4
+    director_derrek = loaded_db.get(User, 525)  # Director Derrek
+
+    # Assign Derrek as director of division 6
+    division_6: Division = loaded_db.get(Division, 6)
+    division_6.division_director_id = director_derrek.id
+    loaded_db.flush()
+
+    bli = loaded_db.get(BudgetLineItem, 15000)
+
+    def make_bli_cr(division_id: int) -> BudgetLineItemChangeRequest:
+        cr = BudgetLineItemChangeRequest()
+        cr.status = ChangeRequestStatus.IN_REVIEW
+        cr.budget_line_item_id = bli.id
+        cr.agreement_id = bli.agreement_id
+        cr.created_by = dave_director.id
+        cr.managing_division_id = division_id
+        cr.requested_change_data = {"amount": 100}
+        loaded_db.add(cr)
+        return cr
+
+    cr1 = make_bli_cr(4)
+    cr2 = make_bli_cr(4)
+    cr3 = make_bli_cr(4)
+    cr4 = make_bli_cr(6)
+    loaded_db.flush()
+
+    # Derrek can only review division 6; with a page size of 3 his lone CR still appears
+    results, total = cr_helpers.find_in_review_requests_by_user(user_id=director_derrek.id, limit=3, offset=0)
+    result_ids = [r.id for r in results]
+    assert cr1.id not in result_ids
+    assert cr2.id not in result_ids
+    assert cr3.id not in result_ids
+    assert cr4.id in result_ids
+    assert len(results) == 1
+    assert total == 1
+
+
 def test_get_division_ids_user_can_review_for(loaded_db, app_ctx):
     director = User(
         first_name="Jane",
@@ -108,28 +148,23 @@ def test_get_division_ids_user_can_review_for(loaded_db, app_ctx):
 
 @patch("ops_api.ops.utils.change_requests_helpers.current_app", new_callable=Mock)
 @patch("ops_api.ops.utils.change_requests_helpers.get_division_ids_user_can_review_for")
-@patch("ops_api.ops.utils.change_requests_helpers.get_division_directors_for_agreement")
-def test_find_in_review_requests_by_user(mock_get_division_directors, mock_get_division_ids, mock_app, app_ctx):
-    # Mocks 3 change requests
+def test_find_in_review_requests_by_user(mock_get_division_ids, mock_app, app_ctx):
     cr1 = Mock(spec=BudgetLineItemChangeRequest, managing_division_id=1)
-    cr2 = Mock(spec=AgreementChangeRequest, agreement="AGREEMENT")
-    cr3 = Mock(spec=BudgetLineItemChangeRequest, managing_division_id=999)  # Should be filtered out
+    cr2 = Mock(spec=AgreementChangeRequest, agreement_id=42)
 
-    # Mock the return values for the database session to return the 3 change requests
-    mock_app.db_session.execute.return_value.scalars.return_value.all.return_value = [
-        cr1,
-        cr2,
-        cr3,
-    ]
+    # execute is called twice: first for COUNT, then for the paginated results
+    count_result = Mock()
+    count_result.scalar.return_value = 2
+    rows_result = Mock()
+    rows_result.scalars.return_value.all.return_value = [cr1, cr2]
+    mock_app.db_session.execute.side_effect = [count_result, rows_result]
 
-    # Mocks the helper functions
     mock_get_division_ids.return_value = {1}
-    mock_get_division_directors.return_value = ([123], [])
 
-    result = cr_helpers.find_in_review_requests_by_user(user_id=123)
+    result, total = cr_helpers.find_in_review_requests_by_user(user_id=123)
     assert cr1 in result
     assert cr2 in result
-    assert cr3 not in result  # cr3 should be filtered out due to managing_division_id not being in {1}
+    assert total == 2
 
 
 @pytest.mark.parametrize(

backend/ops_api/tests/ops/workflows/test_change_requests.py

@@ -239,7 +239,7 @@ def test_change_request_list(auth_client, app, test_user, test_admin_user, test_
     # verify no change request in list to review for this user
     response = auth_client.get(url_for("api.change-requests-list"), query_string={"userId": test_admin_user.id})
     assert response.status_code == 200
-    assert len(response.json) == 0
+    assert len(response.json["data"]) == 0
 
     # create a change request
     change_request1 = BudgetLineItemChangeRequest()
@@ -264,8 +264,9 @@ def test_change_request_list(auth_client, app, test_user, test_admin_user, test_
     # verify there is one change request in the list to review for this user
     response = auth_client.get(url_for("api.change-requests-list"), query_string={"userId": test_admin_user.id})
     assert response.status_code == 200
-    assert len(response.json) == 1
-    cr1 = response.json[0]
+    assert response.json["count"] == 1
+    assert len(response.json["data"]) == 1
+    cr1 = response.json["data"][0]
     assert "has_budget_change" in cr1
     assert not cr1["has_status_change"]
     assert "has_status_change" in cr1
@@ -287,7 +288,8 @@ def test_change_request_list(auth_client, app, test_user, test_admin_user, test_
     # verify there is two change requests in the list to review for this user
     response = auth_client.get(url_for("api.change-requests-list"), query_string={"userId": test_admin_user.id})
     assert response.status_code == 200
-    assert len(response.json) == 2
+    assert response.json["count"] == 2
+    assert len(response.json["data"]) == 2
 
     # review (approve/reject) the change requests
     change_request1.status = ChangeRequestStatus.APPROVED
@@ -300,7 +302,7 @@ def test_change_request_list(auth_client, app, test_user, test_admin_user, test_
     response = auth_client.get(url_for("api.change-requests-list"), query_string={"userId": test_admin_user.id})
 
     assert response.status_code == 200
-    assert len(response.json) == 0
+    assert len(response.json["data"]) == 0
 
     # cleanup
     division1.division_director_id = 522

frontend/cypress.config.ci.js

@@ -55,7 +55,8 @@ export default defineConfig({
             "cypress/e2e/procurementTracker.cy.js",
             "cypress/e2e/procurementTrackerReadOnly.cy.js",
             "cypress/e2e/reportingPage.cy.js",
-            "cypress/e2e/procurementDashboard.cy.js"
+            "cypress/e2e/procurementDashboard.cy.js",
+            "cypress/e2e/changeRequestsPagination.cy.js"
         ],
         // Adding custom task logging, for better a11y output
         // ref: https://docs.cypress.io/api/commands/task#Usage

frontend/cypress.config.js

@@ -50,7 +50,8 @@ export default defineConfig({
             "cypress/e2e/procurementTracker.cy.js",
             "cypress/e2e/procurementTrackerReadOnly.cy.js",
             "cypress/e2e/reportingPage.cy.js",
-            "cypress/e2e/procurementDashboard.cy.js"
+            "cypress/e2e/procurementDashboard.cy.js",
+            "cypress/e2e/changeRequestsPagination.cy.js"
         ],
         // Adding custom task logging, for better a11y output
         // ref: https://docs.cypress.io/api/commands/task#Usage

frontend/cypress/e2e/agreementsPagination.cy.js

@@ -1,5 +1,5 @@
 /// <reference types="cypress" />
-import {terminalLog, testLogin} from "./utils";
+import { terminalLog, testLogin } from "./utils";
 
 describe("Agreements List - Pagination", () => {
     beforeEach(() => {
@@ -66,10 +66,12 @@ describe("Agreements List - Pagination", () => {
         it("should hide Next button on last page", () => {
             // Navigate to last page (need to find what the last page is)
             // Get all page number buttons
-            cy.get("button[aria-label^='Page']").last().then(($lastButton) => {
-                // Click the last page number
-                $lastButton.click();
-            });
+            cy.get("button[aria-label^='Page']")
+                .last()
+                .then(($lastButton) => {
+                    // Click the last page number
+                    $lastButton.click();
+                });
 
             // Next button should be hidden on last page
             cy.get("button[aria-label='Next page']").should("not.be.visible");