OPS-4144 new portfolio list page

[weimiao67]

What changed

This PR implements a new Portfolio List page with summary card, tale, filters and export button.

Issue

#4144

How to test

1. e2e tests pass
2. goto /portfolios
3. page should load correctly
4. filtering, sorting and exporting should work

Screenshots

Definition of Done Checklist

• OESA: Code refactored for clarity
• OESA: Dependency rules followed
• Automated unit tests updated and passed
• Automated integration tests updated and passed
• Automated quality tests updated and passed
• Automated load tests updated and passed
• Automated a11y tests updated and passed
• Automated security tests updated and passed
• 90%+ Code coverage achieved
• [-] Form validations updated

[Copilot]

Pull request overview

This PR implements a new Portfolio List page that replaces the previous card-based layout with a comprehensive table view featuring fiscal year selection, filtering capabilities, export functionality, and a summary visualization of budget distribution across portfolios.

Changes:

• Added new backend API endpoint for batch portfolio funding summary retrieval with filtering support (budget range, available percentage, portfolio IDs)
• Implemented frontend table view with sortable columns, tabs (All/My Portfolios), fiscal year selector, and filter system
• Created horizontal stacked bar chart component with interactive legend for portfolio budget visualization

Reviewed changes

Copilot reviewed 53 out of 55 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
backend/ops_api/ops/resources/portfolio_funding_summary.py	New batch API endpoint with filtering logic
backend/ops_api/ops/schemas/portfolio_funding_summary.py	Schema definitions for batch requests and responses
backend/ops_api/tests/ops/funding_summary/test_portfolio_funding_summary.py	Comprehensive API tests for new endpoint
frontend/src/pages/portfolios/list/PortfolioList.jsx	Complete page rewrite with table layout
frontend/src/pages/portfolios/list/PortfolioList.hooks.js	Custom hook for state management and data fetching
frontend/src/components/Portfolios/PortfolioTable/*	Table components with sorting functionality
frontend/src/components/Portfolios/PortfolioSummaryCards/*	Summary card with stacked bar visualization
frontend/src/components/UI/DataViz/HorizontalStackedBar/*	Reusable stacked bar chart component
frontend/src/pages/portfolios/list/PortfolioFilterButton/*	Filter modal with budget range slider and percentage filters
frontend/cypress/e2e/portfolioList.cy.js	Updated E2E tests for new functionality

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[fpigeonjr]

found a bug clearing the filters as I tested manually

Screen.Recording.2026-01-14.at.8.51.25.AM.mov

[fpigeonjr]

Code Review: PR #4812 - OPS-4144 New Portfolio List Page by Claude 🤖

Overview

This PR implements a comprehensive portfolio list page with advanced filtering, sorting, and export capabilities. The implementation includes:

• Backend: New /portfolio-funding-summary/ API endpoint with flexible query filtering
• Frontend: Full-featured portfolio list page with summary visualization, filtering UI, and Excel export
• Testing: Comprehensive unit tests and E2E coverage

PR Stats: 6,475 additions, 97 deletions across 76 files

---

🎯 What Works Well

Strong Architecture Patterns

✅ Clean separation of concerns with dedicated hooks (usePortfolioList) and helper modules
✅ Proper memoization using useMemo for expensive sorting/filtering operations
✅ Excellent test coverage including unit tests and comprehensive E2E tests
✅ Accessibility considerations with ARIA labels, keyboard navigation, and screen reader support
✅ Backend follows existing patterns (BaseListAPI, schema-based validation)
✅ Well-documented API with OpenAPI spec updates

Code Quality

• Good use of JSDoc types throughout
• Proper error boundaries and loading states
• Clean component composition and reusability
• Follows project conventions (Prettier, ESLint)

---

⚠️ Issues to Address Before Merge

1. Remove Temporary Test Code (Required)

Location: frontend/src/pages/portfolios/list/PortfolioList.helpers.js:30-53

// TODO: Remove this function before merge
export const doubleByDivision = (groupedPortfolios) => { ... }

This function is marked as "temporary for testing" but is exported and has test coverage. Should be removed along with its tests in PortfolioList.helpers.test.js.

---

2. Extract Hardcoded Constants (Recommended)

Locations: Multiple files

The default budget range [0, 100000000] is duplicated across:

• PortfolioList.hooks.js
• PortfolioFilterButton.jsx
• PortfolioFilterTags.jsx

Recommendation: Extract to a shared constant:
// frontend/src/constants.js
export const DEFAULT_PORTFOLIO_BUDGET_RANGE = [0, 100_000_000];

Same for PERCENTAGE_RANGE_LABELS in PortfolioList.constants.js - consider moving to global constants if used elsewhere.

---

3. Improve Export Error Handling (Recommended)

Location: frontend/src/pages/portfolios/list/PortfolioList.helpers.js:117-133

Current implementation:
} catch (error) {
console.error("Error exporting portfolio list:", error);
} finally {
setIsExporting(false);
}

Issue: Users won't know if export fails (silent failure)

Recommendation: Add user-facing error notification:
} catch (error) {
console.error("Error exporting portfolio list:", error);
// Add toast notification or error modal
showErrorToast("Failed to export portfolio data. Please try again.");
} finally {
setIsExporting(false);
}

---

4. Division Sort Order Consistency (Question)

Locations:

• frontend/src/pages/portfolios/list/PortfolioTable/PortfolioTable.helpers.js:1-11
• frontend/src/pages/portfolios/list/PortfolioSummaryCards/PortfolioSummaryCards.helpers.js:3

The table uses custom division order (DCFD→DFS→DEI→OD→Non-OPRE→OCDO) while summary cards use PORTFOLIO_ORDER constant.

Question: Is this intentional? If not, should they share the same constant?

---

5. Budget Range Edge Case (Minor)

Location: PortfolioList.hooks.js:60-62

if (min === max) {
return DEFAULT_BUDGET_RANGE;
}

Issue: If all portfolios have the same budget, falls back to default range. Could confuse users with slider showing $0-$100M when actual data is $5M-$5M.

Consideration: Could add small buffer (±10%) or display message about limited range.

---

6. Console Warning Spam (Minor)

Location: PortfolioList.hooks.js:73-77

if (!portfolio) {
console.warn(Portfolio with id ${summary.portfolio_id} not found in allPortfolios);
continue;
}

Issue: If API calls have different timing, could spam console during loading

Suggestion: Either remove (may be normal during loading) or convert to data consistency metric/log

---

🔍 Code Quality Observations

Backend (portfolio_funding_summary.py)

Good:

• Proper parameter extraction with _extract_first_or_default()
• Schema validation for percentage range values
• Good separation of concerns (helper functions)
• Follows existing BaseListAPI patterns

Consider:

• Add JSDoc/docstring explaining percentage range codes (over90, 75-90, etc.)
• Document the repeated parameter format for portfolio_ids and available_pct

---

Frontend Hook (PortfolioList.hooks.js)

Good:

• Clean state management
• Proper dependency arrays in useMemo
• Good use of refs for caching budget ranges
• Handles loading/error states

Consider:

• The hook is getting large (150+ lines). Could extract filter logic to separate hook if it grows further.

---

Frontend Components

PortfolioFilterButton.jsx:

• ✅ Good modal pattern with proper accessibility
• ✅ Clear separation between default range and applied filters
• ⚠️ Could benefit from PropTypes validation

PortfolioTable.jsx:

• ✅ Good sortable table implementation
• ✅ Proper keyboard navigation
• ✅ Clean row component separation

HorizontalStackedBar.jsx:

• ✅ Excellent accessibility (keyboard + mouse)
• ✅ Proper ARIA labels
• ✅ Good data visualization pattern

---

🧪 Testing

Coverage

• ✅ Backend endpoint tests exist
• ✅ Comprehensive frontend unit tests (hooks, helpers, components)
• ✅ Extensive E2E tests covering major workflows
• ✅ Accessibility tests included (axe-core)

E2E Test Coverage (portfolioList.cy.js)

• ✅ Page structure and loading
• ✅ Tab switching behavior
• ✅ Fiscal year selection and URL parameter passing
• ✅ Filter application and removal
• ✅ Table sorting
• ✅ Export button visibility
• ✅ Empty state handling

Potential Gaps

• ❓ No integration test for actual export file generation (data format validation)
• ❓ Could add test for concurrent fiscal year changes (race conditions)

---

📝 Documentation

OpenAPI Spec (backend/openapi.yml)

• ✅ Well-documented endpoint
• ✅ Clear parameter descriptions
• ✅ Example values provided

Code Comments

• ✅ Good JSDoc coverage
• ✅ Helpful inline comments explaining complex logic
• ⚠️ Some TODO comments should be resolved before merge

---

🚀 Performance Considerations

Good Practices

• ✅ Memoization of expensive calculations
• ✅ Efficient filtering logic
• ✅ Proper React re-render optimization

Potential Optimizations (Future)

• Consider virtualizing table if portfolio count exceeds 100+
• Backend could add pagination if dataset grows significantly
• Export could be moved to backend API endpoint for very large datasets

---

🔐 Security

• ✅ Uses existing authorization decorators
• ✅ Proper input validation via Marshmallow schemas
• ✅ No obvious SQL injection or XSS vulnerabilities
• ✅ CSRF protection via existing middleware

---

📋 Checklist Before Merge

Required

• Remove doubleByDivision() function and its tests
• Extract DEFAULT_BUDGET_RANGE to shared constant
• Improve export error handling with user notification

Recommended

• Clarify division sort order consistency between table and summary
• Add backend docstring for percentage range codes
• Review console.warn usage in usePortfolioList hook

Optional (Nice to Have)

• Add integration test for export data format
• Extract filter logic from usePortfolioList if it grows
• Add PropTypes to PortfolioFilterButton

---

🎉 Summary

This is a high-quality, well-tested implementation of a complex feature. The architecture is solid, test coverage is comprehensive, and the code follows project conventions. The issues identified are minor and primarily involve cleanup (temporary code) and polish (constants extraction, error handling).

Recommendation: Approve with minor changes. The required items are quick fixes that shouldn't require additional review rounds.

Great work on the thorough testing and accessibility considerations! 🚀

---

[github-actions]

🎉 This PR is included in version 1.257.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀