feat(loan): Implement navigation from Loan Account Profile to sub-sections - #2631
Conversation
📝 WalkthroughWalkthroughThe pull request refactors loan account profile navigation by replacing a single generic Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested reviewers
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)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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.
🧹 Nitpick comments (1)
feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanAccountProfile/LoanAccountProfileScreen.kt (1)
106-116: FallbackloanId = -1may silently pass an invalid ID to downstream screens.If
state.loanAccountis null when this event fires, navigation will proceed withloanId = -1, which could cause unexpected behavior or errors in the destination screens. Consider either:
- Early-returning when
loanAccountis null (similar to line 96), or- Logging a warning when the fallback is used.
Also, the
else -> { }branch silently ignores any unhandledLoanAccountProfileActionItemvalues. If new action items are added in the future, this could hide bugs. Consider making thewhenexhaustive or adding a log statement.♻️ Suggested approach
is LoanAccountEvent.NavigateToDetail -> { - val loanId = state.loanAccount?.id ?: -1 + val loanId = state.loanAccount?.id ?: return@EventsEffect when (event.detailItem) { LoanAccountProfileActionItem.RepaymentSchedule -> navigateToRepaymentSchedule(loanId) LoanAccountProfileActionItem.Transactions -> navigateToTransactions(loanId) LoanAccountProfileActionItem.Charges -> navigateToCharges(loanId) LoanAccountProfileActionItem.Documents -> navigateToDocuments(loanId) - else -> { } + else -> Unit } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanAccountProfile/LoanAccountProfileScreen.kt` around lines 106 - 116, The NavigateToDetail handler uses a fallback loanId = -1 which may send an invalid ID to downstream screens; update the handler in LoanAccountEvent.NavigateToDetail to early-return if state.loanAccount is null (like the pattern on line ~96) or at minimum log a warning with context instead of using -1, and then only call navigateToRepaymentSchedule/navigateToTransactions/navigateToCharges/navigateToDocuments with a valid loanId; also remove the silent else -> { } and make the when on event.detailItem exhaustive or replace the else with a log/error branch so new LoanAccountProfileActionItem values are not ignored.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanAccountProfile/LoanAccountProfileScreen.kt`:
- Around line 106-116: The NavigateToDetail handler uses a fallback loanId = -1
which may send an invalid ID to downstream screens; update the handler in
LoanAccountEvent.NavigateToDetail to early-return if state.loanAccount is null
(like the pattern on line ~96) or at minimum log a warning with context instead
of using -1, and then only call
navigateToRepaymentSchedule/navigateToTransactions/navigateToCharges/navigateToDocuments
with a valid loanId; also remove the silent else -> { } and make the when on
event.detailItem exhaustive or replace the else with a log/error branch so new
LoanAccountProfileActionItem values are not ignored.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanAccountProfile/LoanAccountProfileNavigation.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanAccountProfile/LoanAccountProfileScreen.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/navigation/LoanNavigation.kt
2 participants
Fixes - Jira-#667
Description - This PR implements navigation from the new
LoanAccountProfileScreento Repayment Schedule, Transactions, Charges and Loan Documents sections. The navigation structure now follows the same pattern as the ClientProfile flow.MIFOSAC-667.mp4
Summary by CodeRabbit