Implement budget management backend and dashboard UI#240
Implement budget management backend and dashboard UI#240gmarav05 wants to merge 2 commits intoOpenLake:mainfrom
Conversation
|
@gmarav05 is attempting to deploy a commit to the openlake's projects Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughA comprehensive budget management system is introduced, including a backend controller with transaction handling ( Changes
Sequence Diagram(s)sequenceDiagram
participant User as User (Browser)
participant UI as BudgetManagement
participant Hook as useBudgetManagement
participant API as Backend API
participant Auth as Auth Middleware
participant Controller as budgetController
participant DB as Database
User->>UI: Select unit & enter amount
UI->>Hook: submitTransaction(endpoint, amount, ...)
Hook->>API: POST /api/budget/{allocate|spend|refund}<br/>(unitId, amount, eventId, description)
API->>Auth: Check authentication & role
Auth-->>API: ✓ Authorized
API->>Controller: allocateBudget/recordExpense/recordRefund
Controller->>DB: Start Mongoose session
Controller->>DB: Load OrganizationalUnit
Controller->>DB: Validate access (unit, event)
Controller->>DB: Update unit.budget_info<br/>(validate remaining balance)
alt eventId provided
Controller->>DB: Load Event (validate belongs to unit)
Controller->>DB: Update event.budget<br/>(allocated/spent adjustments)
end
Controller->>DB: Create BudgetTransaction record<br/>(capture unit, amount, type, creator)
Controller->>DB: Commit transaction
DB-->>Controller: Transaction created + budget overview
Controller-->>API: HTTP 200 + response
API-->>Hook: Success response
Hook->>API: GET /api/budget/overview & transactions
API-->>Hook: Updated budget data
Hook->>UI: Update state (budget, transactions)
UI-->>User: Display updated budget & history
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes The changes span heterogeneous components (schema, controller with transaction logic, routes with authorization, React hook with async state management, and UI components) across backend and frontend. Review requires verification of transaction validation logic, access control enforcement, Mongoose session handling, state management patterns, API integration, and data flow consistency. Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
backend/controllers/budgetController.js (2)
78-92: Consider caching coordinator unit lookup to avoid duplicate queries.For
CLUB_COORDINATORrole, bothgetAccessibleUnitFilter(line 80) andassertUnitAccess(line 114) perform the same database query to find the coordinator's unit by email. When both functions are called in sequence (e.g., ingetBudgetTransactionswithout a specificunitId), this results in redundant database calls.♻️ Possible optimization
Consider caching the coordinator's unit ID on the user object during authentication middleware, or passing the already-fetched unit through the call chain to avoid the duplicate lookup.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/controllers/budgetController.js` around lines 78 - 92, The coordinator unit lookup in getAccessibleUnitFilter and assertUnitAccess (for role === ROLES.CLUB_COORDINATOR using normalizeEmail and OrganizationalUnit.findOne(...).select("_id")) is duplicated; modify the flow to fetch the coordinator's unit once and reuse it by either attaching the resolved unitId/_id to the user object in the authentication middleware (e.g., user.coordUnitId or user.organizationalUnit) or by changing the signatures of getAccessibleUnitFilter and assertUnitAccess to accept an optional coordUnitId parameter so callers (like getBudgetTransactions) can pass the already-fetched unit instead of triggering a second OrganizationalUnit.findOne call.
390-399: Ensure appropriate indexes exist on BudgetTransaction collection.The query sorts by
created_atand filters byunit_id(and optionallyevent_id,type). For efficient pagination at scale, consider adding a compound index:// In your migration or schema definition BudgetTransactionSchema.index({ unit_id: 1, created_at: -1 });This is especially relevant when the
$infilter (line 371) contains multiple unit IDs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/controllers/budgetController.js` around lines 390 - 399, The query using BudgetTransaction.find(...) sorts by created_at and filters by unit_id (and optionally event_id, type), so add a compound index on the BudgetTransaction schema/migration to support the filter+sort pattern — e.g., create an index like BudgetTransactionSchema.index({ unit_id: 1, created_at: -1 }) (and extend to include event_id or type if those filters are commonly used) to ensure efficient pagination for BudgetTransaction.find, BudgetTransaction.countDocuments and the path that builds the $in filter for unit_id.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/models/schema.js`:
- Around line 648-654: Add the missing import for the UUID generator used by the
schema: require or import the v4 function as uuidv4 and ensure it's defined
before budgetTransactionSchema so the default () => `BGT_${uuidv4()}` can call
it; e.g. add const { v4: uuidv4 } = require('uuid') (or equivalent ES import) at
the top of the file and remove any duplicate/conflicting definitions if present.
In `@backend/routes/budget.js`:
- Line 27: The route uses an undefined constant ROLE_GROUPS.REVIEW_ROLES in the
authorizeRole middleware which causes all requests to be denied; fix it by
replacing ROLE_GROUPS.REVIEW_ROLES with the actual existing key from the
ROLE_GROUPS enum/object (or add REVIEW_ROLES to ROLE_GROUPS if that was
intended), e.g., change the middleware argument in the /allocate route to the
correct symbol (authorizeRole(ROLE_GROUPS.<EXISTING_REVIEW_GROUP>)) or update
the ROLE_GROUPS definition to include REVIEW_ROLES so authorizeRole receives a
valid value.
---
Nitpick comments:
In `@backend/controllers/budgetController.js`:
- Around line 78-92: The coordinator unit lookup in getAccessibleUnitFilter and
assertUnitAccess (for role === ROLES.CLUB_COORDINATOR using normalizeEmail and
OrganizationalUnit.findOne(...).select("_id")) is duplicated; modify the flow to
fetch the coordinator's unit once and reuse it by either attaching the resolved
unitId/_id to the user object in the authentication middleware (e.g.,
user.coordUnitId or user.organizationalUnit) or by changing the signatures of
getAccessibleUnitFilter and assertUnitAccess to accept an optional coordUnitId
parameter so callers (like getBudgetTransactions) can pass the already-fetched
unit instead of triggering a second OrganizationalUnit.findOne call.
- Around line 390-399: The query using BudgetTransaction.find(...) sorts by
created_at and filters by unit_id (and optionally event_id, type), so add a
compound index on the BudgetTransaction schema/migration to support the
filter+sort pattern — e.g., create an index like BudgetTransactionSchema.index({
unit_id: 1, created_at: -1 }) (and extend to include event_id or type if those
filters are commonly used) to ensure efficient pagination for
BudgetTransaction.find, BudgetTransaction.countDocuments and the path that
builds the $in filter for unit_id.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1dcca6d2-d31f-432b-906d-ed759f5e00de
📒 Files selected for processing (10)
backend/controllers/budgetController.jsbackend/index.jsbackend/models/schema.jsbackend/routes/budget.jsbackend/seed.jsfrontend/src/Components/Budget/BudgetManagement.jsxfrontend/src/config/dashboardComponents.jsfrontend/src/config/navbarConfig.jsfrontend/src/hooks/useBudgetManagement.jsfrontend/src/pages/budgetPage.jsx
|
@gmarav05, Currently, eventId is set up as a free-text input (input type="text"). Because the backend strictly expects a valid MongoDB ObjectId, users typing an event name (like "TechFest 2024") or making a typo will immediately get a 400 Invalid event ID format error. |
|
@gmarav05Also fix this: Currently, if a budget transaction fails, two banners appear at the same time (a red error banner from the hook, and a blue status banner from the component.
|
|
@gmarav05 also pls fix/update the below things:
|
2 participants
name: Pull Request
about: Propose and submit changes to the project for review
title: PR: Implement Budget Management backend APIs and dashboard UI
labels: ""
assignees: harshitap1305, sakshi1755
Related Issue
Changes Introduced
Why This Change?
Screenshots / Video (if applicable)
Testing
Documentation Updates
Checklist
Deployment Notes
💬 Additional Notes
Summary by CodeRabbit
Release Notes
New Features