fix(roles): display status in Roles list using safe type extension#56
fix(roles): display status in Roles list using safe type extension#56khushisahu476 wants to merge 6 commits intoopenMF:devfrom
Conversation
|
Hey @khushisahu476 why did you deleted the package-lock json file? |
📝 WalkthroughWalkthroughThis pull request replaces hardcoded placeholder values with proper type definitions and dynamic data rendering. It updates type imports in CreateClosure, introduces local type extensions in ManageDataTables and RolesAndPermissions to support optional fields, and modifies display logic to render dynamic values instead of static placeholders. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/pages/accounting/closing-entries/CreateClosure.tsx`:
- Line 46: Replace the unsafe cast in the setOffices call by guarding the API
payload and providing a safe fallback: when calling setOffices(use the
response.data from the API call and instead of casting to OfficeData[]
unconditionally, coerce/validate it with a runtime fallback such as using
response.data ?? [] or response.data || [] so setOffices always receives an
array), so that subsequent usage like the .map() in CreateClosure does not throw
when response.data is null/undefined; update the call site where
setOffices(response.data as OfficeData[]) is used to pass a guaranteed array.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/pages/accounting/closing-entries/CreateClosure.tsxsrc/pages/system/manage-data-tables/ManageDataTables.tsxsrc/pages/system/roles-and-permissions/RolesAndPermissions.tsx
| try { | ||
| const response = await officesApi.retrieveOffices() | ||
| setOffices(response.data || []) | ||
| setOffices(response.data as OfficeData[]) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for OfficesApi and retrieveOffices method definition
rg -n -C3 'class\s+OfficesApi|retrieveOffices\s*\(' --type ts --type tsxRepository: openMF/mifos-x-web-app-react
Length of output: 99
🏁 Script executed:
#!/bin/bash
# Search for OfficeData type definition
rg -n -C3 'export\s+(type|interface)\s+OfficeData\b' --type ts --type tsxRepository: openMF/mifos-x-web-app-react
Length of output: 99
🏁 Script executed:
#!/bin/bash
# Check how setOffices is used in CreateClosure.tsx
rg -n -C5 'setOffices\s*\(' src/pages/accounting/closing-entries/CreateClosure.tsxRepository: openMF/mifos-x-web-app-react
Length of output: 421
🏁 Script executed:
#!/bin/bash
# Get the full context around line 46 in CreateClosure.tsx
cat -n src/pages/accounting/closing-entries/CreateClosure.tsx | sed -n '35,60p'Repository: openMF/mifos-x-web-app-react
Length of output: 949
🏁 Script executed:
#!/bin/bash
# Search for OfficesApi class definition
rg -n -C3 'class\s+OfficesApi|retrieveOffices\s*\('Repository: openMF/mifos-x-web-app-react
Length of output: 13626
🏁 Script executed:
#!/bin/bash
# Search for OfficeData type/interface definition
rg -n -C3 'export\s+(type|interface)\s+OfficeData'Repository: openMF/mifos-x-web-app-react
Length of output: 54
🏁 Script executed:
#!/bin/bash
# Search for officesApi initialization in CreateClosure.tsx
rg -n -C2 'officesApi' src/pages/accounting/closing-entries/CreateClosure.tsx | head -20Repository: openMF/mifos-x-web-app-react
Length of output: 422
🏁 Script executed:
#!/bin/bash
# Check imports at the top of CreateClosure.tsx
head -30 src/pages/accounting/closing-entries/CreateClosure.tsxRepository: openMF/mifos-x-web-app-react
Length of output: 1049
🏁 Script executed:
#!/bin/bash
# Find OfficeData interface in fineract-api
find . -path "*/fineract-api*" -name "*.ts" -o -name "*.tsx" | head -20Repository: openMF/mifos-x-web-app-react
Length of output: 814
🏁 Script executed:
#!/bin/bash
# Search for OfficeData interface definition
rg -n 'interface\s+OfficeData|type\s+OfficeData|export.*OfficeData'Repository: openMF/mifos-x-web-app-react
Length of output: 148
🏁 Script executed:
#!/bin/bash
# Search for OfficesApi class
rg -n 'class OfficesApi' -A 10Repository: openMF/mifos-x-web-app-react
Length of output: 54
🏁 Script executed:
#!/bin/bash
# Check if fineract-api is a local directory or path alias
find . -type d -name "fineract-api" 2>/dev/null | grep -v node_modulesRepository: openMF/mifos-x-web-app-react
Length of output: 54
🏁 Script executed:
#!/bin/bash
# Check tsconfig to understand `@/fineract-api` path alias
cat tsconfig.json | grep -A 5 "paths"Repository: openMF/mifos-x-web-app-react
Length of output: 137
🏁 Script executed:
#!/bin/bash
# Search for fineract-api in package.json
cat package.json | grep -i fineractRepository: openMF/mifos-x-web-app-react
Length of output: 226
🏁 Script executed:
#!/bin/bash
# List files in the generated fineract-api directory
ls -la src/fineract-api/ | head -30Repository: openMF/mifos-x-web-app-react
Length of output: 136
🏁 Script executed:
#!/bin/bash
# Search for OfficeData and OfficesApi in the generated API code
rg -n 'interface OfficeData|type OfficeData|class OfficesApi' src/fineract-api/ -A 5 | head -50Repository: openMF/mifos-x-web-app-react
Length of output: 129
🏁 Script executed:
#!/bin/bash
# Check the index file to understand exports
cat src/fineract-api/index.ts 2>/dev/null | grep -i office | head -20Repository: openMF/mifos-x-web-app-react
Length of output: 54
🏁 Script executed:
#!/bin/bash
# Check if fineract-api is in node_modules
ls -la node_modules/@fineract* 2>/dev/null | head -20Repository: openMF/mifos-x-web-app-react
Length of output: 54
🏁 Script executed:
#!/bin/bash
# Look for any API type definitions or models in src
find src -type f -name "*.ts" -o -name "*.tsx" | xargs rg -l "OfficeData|OfficesApi" | grep -v node_modulesRepository: openMF/mifos-x-web-app-react
Length of output: 1328
🏁 Script executed:
#!/bin/bash
# Get a broader context around the setOffices calls to understand patterns
rg -B 2 -A 1 'setOffices\(.*\.data' src/pages --type ts --type tsx 2>/dev/null | head -80Repository: openMF/mifos-x-web-app-react
Length of output: 54
🏁 Script executed:
#!/bin/bash
# Check how offices is used after setOffices in CreateClosure.tsx
rg -n 'offices\.' src/pages/accounting/closing-entries/CreateClosure.tsx -A 2Repository: openMF/mifos-x-web-app-react
Length of output: 212
🏁 Script executed:
#!/bin/bash
# Get the complete usage of offices state in CreateClosure.tsx
cat src/pages/accounting/closing-entries/CreateClosure.tsx | grep -n "offices" | head -20Repository: openMF/mifos-x-web-app-react
Length of output: 393
🏁 Script executed:
#!/bin/bash
# Check how many other files use the same unsafe pattern vs. the safe pattern
echo "=== Files using '|| []' or '?? []' pattern ==="
rg -c 'setOffices\(.*\|\|.*\[\]|setOffices\(.*\?\?.*\[\]' src/pages
echo ""
echo "=== Files using unconditional cast 'as OfficeData[]' pattern ==="
rg -c 'setOffices\(.*as OfficeData' src/pages
echo ""
echo "=== Specific instances with unconditional casts ==="
rg -n 'setOffices.*as (Office|any)' src/pagesRepository: openMF/mifos-x-web-app-react
Length of output: 1016
Use runtime guard instead of unconditional cast on API payload.
setOffices(response.data as OfficeData[]) suppresses type safety without validation. If the API returns undefined or null, the subsequent .map() call at line 117 will fail at runtime. The codebase consistently uses safe fallbacks (|| [] or ?? []) across 12+ similar patterns; align with this convention.
Suggested fix
- setOffices(response.data as OfficeData[])
+ setOffices(response.data || [])📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| setOffices(response.data as OfficeData[]) | |
| setOffices(response.data || []) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/accounting/closing-entries/CreateClosure.tsx` at line 46, Replace
the unsafe cast in the setOffices call by guarding the API payload and providing
a safe fallback: when calling setOffices(use the response.data from the API call
and instead of casting to OfficeData[] unconditionally, coerce/validate it with
a runtime fallback such as using response.data ?? [] or response.data || [] so
setOffices always receives an array), so that subsequent usage like the .map()
in CreateClosure does not throw when response.data is null/undefined; update the
call site where setOffices(response.data as OfficeData[]) is used to pass a
guaranteed array.
2 participants
Description
The
/system/roles-and-permissionsendpoint returns the fieldstatus,but it is missing from the OpenAPI-generated interface.
This change safely extends the
GetRolesResponsetype locally in theRoles list page and displays the value without modifying generated SDK code.
Related issues and discussion
Addresses the item in ISSUES.md:
Admin System → /system/roles-and-permissions
GetRolesResponse missing status
Screenshots, if any
Checklist
Please make sure these boxes are checked before submitting your pull request - thanks!
If you have multiple commits please combine them into one commit by squashing them.
Read and understood the contribution guidelines at
CONTRIBUTING.md.Summary by CodeRabbit
Release Notes