Merge remote-tracking branch 'origin/OPS-5073/procurement-dashboard-initial-layout' into OPS-5077/procurement-dashboard-details

Author: Santi-3rd

.github/workflows/security_semgrep.yml

@@ -9,7 +9,7 @@ jobs:
     name: Semgrep Analyze
     runs-on: ubuntu-latest
     container:
-      image: returntocorp/semgrep@sha256:7810f1d7884974ab6dda7bef8f4a2c8e165ea2142fd8260515d380e4f1407263 # 1.47.0
+      image: returntocorp/semgrep@sha256:326e5f41cc972bb423b764a14febbb62bbad29ee1c01820805d077dd868fea48 # 1.47.0
     steps:
       - name: Checkout
         uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6

backend/openapi.yml

@@ -3564,9 +3564,23 @@ paths:
                   agreement_names:
                     type: array
                     items:
-                      type: string
-                    description: List of agreement names and nicknames associated with projects (sorted alphabetically, no duplicates)
-                    example: ["Agreement 001", "Contract Alpha", "Grant Beta"]
+                      type: object
+                      required:
+                        - id
+                        - name
+                      properties:
+                        id:
+                          type: integer
+                        name:
+                          type: string
+                    description: List of agreement names and nicknames associated with projects (sorted alphabetically by name, no duplicates)
+                    example:
+                      - id: 1
+                        name: "Agreement 001"
+                      - id: 2
+                        name: "Contract Alpha"
+                      - id: 3
+                        name: "Grant Beta"
         "401":
           description: Unauthorized
         "500":

backend/ops_api/ops/schemas/projects.py

@@ -208,7 +208,7 @@ class ProjectListFilterOptionResponseSchema(Schema):
     portfolios = fields.List(fields.Dict(keys=fields.String(), values=fields.Raw()), required=True)
     project_titles = fields.List(fields.Dict(keys=fields.String(), values=fields.Raw()), required=True)
     project_types = fields.List(fields.String(), required=True)
-    agreement_names = fields.List(fields.String(), required=True)
+    agreement_names = fields.List(fields.Nested(AgreementNameListItem), required=True)
 
 
 class ProjectFundingRequestSchema(Schema):

backend/ops_api/ops/services/projects.py

@@ -20,7 +20,6 @@
     User,
 )
 from ops_api.ops.services.ops_service import OpsService, ResourceNotFoundError, ValidationError
-from ops_api.ops.utils.query_helpers import QueryHelper
 
 
 @dataclass
@@ -227,12 +226,10 @@ def _get_research_projects_query(filters: ProjectFilters):
         Returns:
             SQLAlchemy select statement
         """
+        # Base query with all necessary eager loading
         stmt = (
             select(ResearchProject)
             .distinct(ResearchProject.id)
-            .join(Agreement, isouter=True)
-            .join(BudgetLineItem, isouter=True)
-            .join(CAN, isouter=True)
             .options(
                 selectinload(ResearchProject.agreements).selectinload(Agreement.services_components),
                 selectinload(ResearchProject.agreements)
@@ -244,35 +241,63 @@ def _get_research_projects_query(filters: ProjectFilters):
             )
         )
 
-        query_helper = QueryHelper(stmt)
+        # For filtering, we use EXISTS subqueries to ensure each filter can match different
+        # related records. This prevents the issue where a single joined row must satisfy
+        # multiple conditions across different tables (e.g., one agreement for fiscal year
+        # and a different agreement for agreement name).
+        where_clauses = []
 
-        # Apply portfolio filter (OR logic - match any portfolio)
+        # Apply portfolio filter using EXISTS subquery
         if filters.portfolio_id:
-            query_helper.add_column_in_list(CAN.portfolio_id, filters.portfolio_id)
+            portfolio_subquery = (
+                select(1)
+                .select_from(Agreement)
+                .join(BudgetLineItem)
+                .join(CAN)
+                .where(Agreement.project_id == ResearchProject.id)
+                .where(CAN.portfolio_id.in_(filters.portfolio_id))
+                .exists()
+            )
+            where_clauses.append(portfolio_subquery)
 
-        # Apply fiscal year filter (OR logic - match any fiscal year)
+        # Apply fiscal year filter using EXISTS subquery
         if filters.fiscal_year:
-            if len(filters.fiscal_year) == 1:
-                fiscal_year = filters.fiscal_year[0]
-                query_helper.add_column_equals(BudgetLineItem.fiscal_year, fiscal_year)
-            else:
-                # Multiple fiscal years - use IN clause
-                query_helper.add_column_in_list(BudgetLineItem.fiscal_year, filters.fiscal_year)
+            fy_subquery = (
+                select(1)
+                .select_from(Agreement)
+                .join(BudgetLineItem)
+                .where(Agreement.project_id == ResearchProject.id)
+                .where(BudgetLineItem.fiscal_year.in_(filters.fiscal_year))
+                .exists()
+            )
+            where_clauses.append(fy_subquery)
 
         # Apply project search filter on project title (OR logic, exact match on title/short title)
         if filters.project_search:
-            query_helper.where_clauses.append(
-                or_(Project.title.in_(filters.project_search), Project.short_title.in_(filters.project_search))
+            where_clauses.append(
+                or_(
+                    ResearchProject.title.in_(filters.project_search),
+                    ResearchProject.short_title.in_(filters.project_search),
+                )
             )
 
-        # Apply agreement search filter on agreement name and nick_name (exact match - OR logic)
-        # Projects are returned if any agreement has name OR nick_name matching any search term
+        # Apply agreement search filter using EXISTS subquery
         if filters.agreement_search:
-            query_helper.where_clauses.append(
-                or_(Agreement.name.in_(filters.agreement_search), Agreement.nick_name.in_(filters.agreement_search))
+            agreement_subquery = (
+                select(1)
+                .select_from(Agreement)
+                .where(Agreement.project_id == ResearchProject.id)
+                .where(
+                    or_(Agreement.name.in_(filters.agreement_search), Agreement.nick_name.in_(filters.agreement_search))
+                )
+                .exists()
             )
+            where_clauses.append(agreement_subquery)
+
+        # Apply all where clauses
+        if where_clauses:
+            stmt = stmt.where(*where_clauses)
 
-        stmt = query_helper.get_stmt()
         logger.debug(f"SQL: {stmt}")
 
         return stmt
@@ -287,55 +312,78 @@ def _get_administrative_and_support_projects_query(filters: ProjectFilters):
         Returns:
             SQLAlchemy select statement
         """
+        # Base query with all necessary eager loading
         stmt = (
             select(AdministrativeAndSupportProject)
             .distinct(AdministrativeAndSupportProject.id)
-            .join(Agreement, isouter=True)
-            .join(BudgetLineItem, isouter=True)
-            .join(CAN, isouter=True)
             .options(
-                selectinload(ResearchProject.agreements).selectinload(Agreement.services_components),
-                selectinload(ResearchProject.agreements)
+                selectinload(AdministrativeAndSupportProject.agreements).selectinload(Agreement.services_components),
+                selectinload(AdministrativeAndSupportProject.agreements)
                 .selectinload(Agreement.budget_line_items)
                 .selectinload(BudgetLineItem.can),
-                selectinload(ResearchProject.agreements).selectinload(Agreement.special_topics),
-                selectinload(ResearchProject.agreements).selectinload(Agreement.research_methodologies),
-                selectinload(ResearchProject.agreements).selectinload(Agreement.team_members),
+                selectinload(AdministrativeAndSupportProject.agreements).selectinload(Agreement.special_topics),
+                selectinload(AdministrativeAndSupportProject.agreements).selectinload(Agreement.research_methodologies),
+                selectinload(AdministrativeAndSupportProject.agreements).selectinload(Agreement.team_members),
             )
         )
 
-        query_helper = QueryHelper(stmt)
+        # For filtering, we use EXISTS subqueries to ensure each filter can match different
+        # related records. This prevents the issue where a single joined row must satisfy
+        # multiple conditions across different tables (e.g., one agreement for fiscal year
+        # and a different agreement for agreement name).
+        where_clauses = []
 
-        # Apply portfolio filter (OR logic - match any portfolio)
+        # Apply portfolio filter using EXISTS subquery
         if filters.portfolio_id:
-            query_helper.add_column_in_list(CAN.portfolio_id, filters.portfolio_id)
+            portfolio_subquery = (
+                select(1)
+                .select_from(Agreement)
+                .join(BudgetLineItem)
+                .join(CAN)
+                .where(Agreement.project_id == AdministrativeAndSupportProject.id)
+                .where(CAN.portfolio_id.in_(filters.portfolio_id))
+                .exists()
+            )
+            where_clauses.append(portfolio_subquery)
 
-        # Apply fiscal year filter (OR logic - match any fiscal year)
+        # Apply fiscal year filter using EXISTS subquery
         if filters.fiscal_year:
-            if len(filters.fiscal_year) == 1:
-                fiscal_year = filters.fiscal_year[0]
-                query_helper.add_column_equals(BudgetLineItem.fiscal_year, fiscal_year)
-            else:
-                # Multiple fiscal years - use IN clause
-                query_helper.add_column_in_list(BudgetLineItem.fiscal_year, filters.fiscal_year)
+            fy_subquery = (
+                select(1)
+                .select_from(Agreement)
+                .join(BudgetLineItem)
+                .where(Agreement.project_id == AdministrativeAndSupportProject.id)
+                .where(BudgetLineItem.fiscal_year.in_(filters.fiscal_year))
+                .exists()
+            )
+            where_clauses.append(fy_subquery)
 
-        # Apply project search filter on project title (AND logic - must match all search terms)
+        # Apply project search filter on project title (OR logic, exact match on title/short title)
         if filters.project_search:
-            query_helper.where_clauses.append(
+            where_clauses.append(
                 or_(
-                    Project.title.in_(filters.project_search),
-                    Project.short_title.in_(filters.project_search),
+                    AdministrativeAndSupportProject.title.in_(filters.project_search),
+                    AdministrativeAndSupportProject.short_title.in_(filters.project_search),
                 )
             )
 
-        # Apply agreement search filter on agreement name and nick_name (exact match - OR logic)
-        # Projects are returned if any agreement has name OR nick_name matching any search term
+        # Apply agreement search filter using EXISTS subquery
         if filters.agreement_search:
-            query_helper.where_clauses.append(
-                or_(Agreement.name.in_(filters.agreement_search), Agreement.nick_name.in_(filters.agreement_search))
+            agreement_subquery = (
+                select(1)
+                .select_from(Agreement)
+                .where(Agreement.project_id == AdministrativeAndSupportProject.id)
+                .where(
+                    or_(Agreement.name.in_(filters.agreement_search), Agreement.nick_name.in_(filters.agreement_search))
+                )
+                .exists()
             )
+            where_clauses.append(agreement_subquery)
+
+        # Apply all where clauses
+        if where_clauses:
+            stmt = stmt.where(*where_clauses)
 
-        stmt = query_helper.get_stmt()
         logger.debug(f"SQL: {stmt}")
 
         return stmt
@@ -626,24 +674,27 @@ def get_filter_options(self) -> dict[str, Any]:
         )
         project_types = sorted([pt.name for pt in self.db_session.scalars(project_types_query).all()])
 
-        # Step 6: Agreement names and nick_names - Query both and create a sorted list
+        # Step 6: Agreement names and nick_names - Query both and create a sorted list of dicts
         agreement_names_query = (
             select(Agreement.id, Agreement.name, Agreement.nick_name)
             .join(Project, Agreement.project_id == Project.id)
             .where(Project.id.in_(project_ids_subquery))
             .where(Agreement.name.isnot(None))
         )
 
-        # Collect all names and nick_names into a single sorted list
-        # Don't need ids because the match query matches directly on name and nick_name.
-        agreement_values = set()  # Use set to avoid duplicates
-        for _, a_name, a_nick_name in self.db_session.execute(agreement_names_query).all():
-            if a_name:
-                agreement_values.add(a_name)
-            if a_nick_name:
-                agreement_values.add(a_nick_name)
-
-        agreement_names = sorted(list(agreement_values))
+        # Collect all names and nick_names into a list of dicts with ids
+        # Use a dict keyed by name to avoid duplicates while preserving id
+        agreement_values_dict = {}  # Key: name, Value: id
+        for a_id, a_name, a_nick_name in self.db_session.execute(agreement_names_query).all():
+            if a_name and a_name not in agreement_values_dict:
+                agreement_values_dict[a_name] = a_id
+            if a_nick_name and a_nick_name not in agreement_values_dict:
+                agreement_values_dict[a_nick_name] = a_id
+
+        # Convert to list of dicts and sort by name
+        agreement_names = sorted(
+            [{"id": a_id, "name": name} for name, a_id in agreement_values_dict.items()], key=lambda x: x["name"]
+        )
 
         # Build response
         filters = {

backend/ops_api/tests/ops/project/test_project.py

@@ -346,6 +346,35 @@ def test_project_type_filter_single_type(auth_client, loaded_db):
     assert len(response_research.json["data"]) + len(response_admin.json["data"]) == len(response_all.json["data"])
 
 
+def test_agreement_and_fiscal_year_filter(auth_client, loaded_db):
+    """Test filtering by agreement and fiscal year at the same time"""
+    response_research = auth_client.get(
+        url_for(
+            "api.projects-group",
+            agreement_search=["AA #1: Fathers and Continuous Learning (FCL)"],
+            fiscal_year=[2044],
+            limit=50,
+        )
+    )
+
+    assert response_research.status_code == 200
+    projects = [p for p in response_research.json["data"]]
+    assert len(projects) == 1
+
+    response_2 = auth_client.get(
+        url_for(
+            "api.projects-group",
+            agreement_search=["AA #1: Fathers and Continuous Learning (FCL)"],
+            fiscal_year=[2045],
+            limit=50,
+        )
+    )
+
+    assert response_2.status_code == 200
+    projects = [p for p in response_2.json["data"]]
+    assert len(projects) == 0
+
+
 def test_projects_get_all_includes_summary(auth_client, loaded_db):
     """Test that GET /projects/ includes summary with total_projects, projects_by_type, and amounts_by_type."""
     response = auth_client.get(url_for("api.projects-group"))
@@ -1716,12 +1745,18 @@ def test_filter_options_agreement_types_sorted(self, auth_client, app_ctx):
         assert project_types == sorted(project_types)
 
     def test_filter_options_agreement_names_sorted_by_name(self, auth_client, app_ctx):
-        """Agreement names should be sorted alphabetically by name."""
+        """Agreement names should be sorted alphabetically by name and returned as list of dicts with id and name."""
         response = auth_client.get(url_for("api.projects-filters"))
         assert response.status_code == 200
 
         agreement_names = response.json["agreement_names"]
-        names = [a for a in agreement_names]
+        assert len(agreement_names) > 0
+        # Verify structure: each item should have id and name
+        for agreement in agreement_names:
+            assert "id" in agreement
+            assert "name" in agreement
+        # Verify sorted by name
+        names = [a["name"] for a in agreement_names]
         assert names == sorted(names)