Add branch statistics API with comprehensive account analytics#30
Add branch statistics API with comprehensive account analytics#30
Conversation
- Created branch statistics API endpoints:
* GET /api/branches/list - Get all branches for dropdown
* GET /api/branches/{branch_id}/statistics - Get detailed branch statistics
- Features:
* Joint accounts count and total balance
* Fixed deposits count and total amount
* Savings accounts count and total balance (excluding joint accounts)
- Implementation follows clean architecture:
* Schema layer: branch_stats_schema.py (Pydantic models)
* Repository layer: branch_stats_repo.py (SQL queries with CTEs)
* Service layer: branch_stats_service.py (business logic)
* API layer: branch_stats_routes.py (FastAPI endpoints)
- Fixed SQL column references to match actual database schema
- Updated requirements.txt with reportlab dependency
- Added comprehensive API documentation
There was a problem hiding this comment.
Pull Request Overview
This PR adds a comprehensive branch statistics API that provides detailed analytics for bank branches, including account counts and balances across different account types (joint accounts, fixed deposits, and savings accounts).
- Implements clean architecture with schema, repository, service, and API layers
- Adds two main endpoints: branch list for dropdowns and detailed branch statistics
- Uses PostgreSQL CTEs for efficient data aggregation and excludes joint accounts from savings count to prevent double counting
Reviewed Changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| app/services/branch_stats_service.py | Business logic layer handling statistics aggregation and error handling |
| app/schemas/branch_stats_schema.py | Pydantic models for API request/response validation |
| app/repositories/branch_stats_repo.py | Database layer with complex SQL queries using CTEs for statistics calculation |
| app/main.py | Router registration for branch statistics endpoints |
| app/api/branch_stats_routes.py | FastAPI endpoints with comprehensive documentation and authentication |
| BRANCH_STATS_API_DOCUMENTATION.md | Complete API documentation with examples and usage patterns |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| return BranchAccountStats( | ||
| branch_id=stats['branch_id'], | ||
| branch_name=stats['branch_name'], | ||
| branch_code=stats.get('branch_code'), |
There was a problem hiding this comment.
The service is trying to access 'branch_code' and 'city' fields that don't exist in the repository response. The repository returns 'branch_address' but the service expects 'branch_code' and 'city', which will result in None values being returned.
| branch_code=branch.get('branch_code'), | ||
| city=branch.get('city') |
There was a problem hiding this comment.
The service is trying to access 'branch_code' and 'city' fields that don't exist in the repository response. The repository returns 'branch_address' but the service expects 'branch_code' and 'city', which will result in None values being returned.
|
|
||
| return None | ||
|
|
||
| except Exception as e: |
There was a problem hiding this comment.
Re-raising exceptions without adding context or logging defeats the purpose of the try-catch block. Either add logging/context or remove the exception handling entirely to let exceptions bubble up naturally.
| try: | ||
| self.cursor.execute( | ||
| """ | ||
| SELECT | ||
| branch_id, | ||
| name as branch_name, | ||
| address as branch_address | ||
| FROM branch | ||
| ORDER BY name | ||
| """ | ||
| ) | ||
|
|
||
| branches = self.cursor.fetchall() | ||
|
|
||
| return [ | ||
| { | ||
| 'branch_id': str(branch['branch_id']), | ||
| 'branch_name': branch['branch_name'], | ||
| 'branch_address': branch.get('branch_address') | ||
| } | ||
| for branch in branches | ||
| ] | ||
|
|
||
| except Exception as e: | ||
| raise e |
There was a problem hiding this comment.
Re-raising exceptions without adding context or logging defeats the purpose of the try-catch block. Either add logging/context or remove the exception handling entirely to let exceptions bubble up naturally.
| try: | |
| self.cursor.execute( | |
| """ | |
| SELECT | |
| branch_id, | |
| name as branch_name, | |
| address as branch_address | |
| FROM branch | |
| ORDER BY name | |
| """ | |
| ) | |
| branches = self.cursor.fetchall() | |
| return [ | |
| { | |
| 'branch_id': str(branch['branch_id']), | |
| 'branch_name': branch['branch_name'], | |
| 'branch_address': branch.get('branch_address') | |
| } | |
| for branch in branches | |
| ] | |
| except Exception as e: | |
| raise e | |
| self.cursor.execute( | |
| """ | |
| SELECT | |
| branch_id, | |
| name as branch_name, | |
| address as branch_address | |
| FROM branch | |
| ORDER BY name | |
| """ | |
| ) | |
| branches = self.cursor.fetchall() | |
| return [ | |
| { | |
| 'branch_id': str(branch['branch_id']), | |
| 'branch_name': branch['branch_name'], | |
| 'branch_address': branch.get('branch_address') | |
| } | |
| for branch in branches | |
| ] | |
Created branch statistics API endpoints:
Features:
Implementation follows clean architecture:
Fixed SQL column references to match actual database schema
Updated requirements.txt with reportlab dependency
Added comprehensive API documentation