fix(feature/loan): display Overpaid status for overpaid loan accounts#2592
fix(feature/loan): display Overpaid status for overpaid loan accounts#2592Kartikey15dem wants to merge 1 commit intoopenMF:developmentfrom
Conversation
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughAdded a new string resource Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 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)
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 |
There was a problem hiding this comment.
Pull request overview
This pull request fixes loan status display in the Mobile App to correctly show "Overpaid" for overpaid loan accounts, matching the Web App behavior. Previously, overpaid loans were incorrectly displayed as "Loan Closed".
Changes:
- Added explicit handling for
status.overpaid == truein thegetButtonText()function - Added new string resource "Loan Overpaid" for displaying the overpaid status
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanAccountSummary/LoanAccountSummaryScreen.kt | Added import for overpaid string resource and new condition in getButtonText() to return "Loan Overpaid" when loan status is overpaid |
| feature/loan/src/commonMain/composeResources/values/strings.xml | Added new string resource "feature_loan_overpaid" with value "Loan Overpaid" |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| status.overpaid == true -> { | ||
| stringResource(Res.string.feature_loan_overpaid) | ||
| } |
There was a problem hiding this comment.
The condition for checking overpaid status is placed after the closedObligationsMet check. This is a logic error because when a loan is overpaid, it typically also has closedObligationsMet set to true. As a result, the first condition (line 700) will match and return "Make Repayment" instead of checking for overpaid status. The overpaid condition needs to be checked BEFORE closedObligationsMet to ensure it takes precedence.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@feature/loan/src/commonMain/composeResources/values/strings.xml`:
- Line 111: The string resource feature_loan_overpaid currently reads "Loan
Overpaid" but should match the Web App label "Overpaid"; update the XML value
for the string resource named feature_loan_overpaid to "Overpaid" so mobile and
web labels are consistent (ensure no other occurrences override it).
🧹 Nitpick comments (1)
feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanAccountSummary/LoanAccountSummaryScreen.kt (1)
698-716: Move theoverpaidcheck beforeactiveandclosedObligationsMetto ensure it takes precedence.If a loan status can have both
activeandoverpaidflags true simultaneously, the current ordering will shadow the overpaid case and always show "Make Repayment". Although the codebase does not explicitly prevent this flag combination, defensive ordering is safer. Consider prioritizingoverpaidchecks first.✅ Safer ordering
return when { + status.overpaid == true -> { + stringResource(Res.string.feature_loan_overpaid) + } + status.active == true || status.closedObligationsMet == true -> { stringResource(Res.string.feature_loan_make_Repayment) } @@ - status.overpaid == true -> { - stringResource(Res.string.feature_loan_overpaid) - } - else -> { stringResource(Res.string.feature_loan_closed) } }
| <string name="feature_loan_closed">Loan Closed</string> | ||
| <string name="feature_loan_total_loan">Total Loan</string> | ||
|
|
||
| <string name="feature_loan_overpaid">Loan Overpaid</string> |
There was a problem hiding this comment.
Align the label with the expected “Overpaid” text.
PR objective says the mobile label should match the Web App’s “Overpaid”, but this string reads “Loan Overpaid”. That creates a visible mismatch.
✏️ Suggested update
- <string name="feature_loan_overpaid">Loan Overpaid</string>
+ <string name="feature_loan_overpaid">Overpaid</string>📝 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.
| <string name="feature_loan_overpaid">Loan Overpaid</string> | |
| <string name="feature_loan_overpaid">Overpaid</string> |
🤖 Prompt for AI Agents
In `@feature/loan/src/commonMain/composeResources/values/strings.xml` at line 111,
The string resource feature_loan_overpaid currently reads "Loan Overpaid" but
should match the Web App label "Overpaid"; update the XML value for the string
resource named feature_loan_overpaid to "Overpaid" so mobile and web labels are
consistent (ensure no other occurrences override it).
78262fb to
b96e68d
Compare
Fixes - Jira-#Issue_Number
Summary
Updated loan status handling to correctly display Overpaid for overpaid loan accounts in the Mobile App, aligning it with Web App behavior.
Issue
For overpaid loan accounts, the Web App correctly shows Overpaid, but the Mobile App was incorrectly displaying Loan Closed.
Fix
Added explicit handling for
status.overpaid == trueto ensure the correct button text/status is shown.Steps to Verify
000000016shows Overpaid.Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.