Skip to content

Conversation

@Kartikey15dem
Copy link
Contributor

@Kartikey15dem Kartikey15dem commented Jan 31, 2026

Fixes - Jira-#MIFOSAC-637

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.

Behavior Changes

Before After
overpaidBef.webm
overpaidAft.webm

Fix

Added explicit handling for status.overpaid == true to ensure the correct button text/status is shown.

Steps to Verify

  1. Open the client loan list in the Web App → Account 000000016 shows Overpaid.
  2. Open the same account in the Mobile App → Status now shows Overpaid.

Summary by CodeRabbit

  • New Features
    • Added support for displaying the "Overpaid" status in loan account summaries, providing clarity when a loan has been overpaid.

✏️ Tip: You can customize this high-level summary in your review settings.

Copilot AI review requested due to automatic review settings January 31, 2026 13:53
@coderabbitai
Copy link

coderabbitai bot commented Jan 31, 2026

📝 Walkthrough

Walkthrough

Added a new public string resource feature_loan_overpaid with value "Overpaid" and updated LoanAccountSummaryScreen to import and return that string when a loan's status has overpaid == true.

Changes

Cohort / File(s) Summary
Strings & UI
feature/loan/src/commonMain/composeResources/values/strings.xml, feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanAccountSummary/LoanAccountSummaryScreen.kt
Added feature_loan_overpaid = "Overpaid" to strings.xml. Imported the resource in LoanAccountSummaryScreen and added a branch in getButtonText to return feature_loan_overpaid when status.overpaid == true.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Suggested reviewers

  • revanthkumarJ
  • itsPronay

Poem

🐰 I hopped through strings with joy today,
"Overpaid" now sings where balances play,
A tiny change, a tidy cheer,
Screens and words now crisp and clear,
Hooray—more carrots for the dev buffet! 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: adding display of 'Overpaid' status for overpaid loan accounts, which directly matches the modifications to strings.xml and LoanAccountSummaryScreen.kt.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates loan status rendering in the Loan Account Summary so overpaid loan accounts display an Overpaid label (instead of being treated like closed), aligning Mobile App behavior with the Web App.

Changes:

  • Add explicit status.overpaid == true handling in getButtonText(...).
  • Add a new localized string resource for the overpaid label.

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 Adds an overpaid branch in button/status text selection for loan summary UI.
feature/loan/src/commonMain/composeResources/values/strings.xml Introduces a new string resource used to render the overpaid label.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanAccountSummary/LoanAccountSummaryScreen.kt (1)

698-718: ⚠️ Potential issue | 🟠 Major

Move overpaid check before closedObligationsMet to prevent masking.

The overpaid == true branch can be masked if a loan status has both closedObligationsMet == true and overpaid == true set simultaneously. The first branch would match and return "Make Repayment", preventing the "Loan Overpaid" text from ever displaying. Reorder the conditions to check overpaid before the closedObligationsMet check.

Suggested reorder
 `@Composable`
 fun getButtonText(status: LoanStatusEntity): String {
     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)
         }
     }
 }

fix(feature/loan): display Overpaid status for overpaid loan accounts
@Kartikey15dem Kartikey15dem force-pushed the fix/overpaid-loan-status branch from 3e78268 to f3f4d5e Compare January 31, 2026 14:05
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanAccountSummary/LoanAccountSummaryScreen.kt (1)

698-720: ⚠️ Potential issue | 🟠 Major

Move overpaid check before closedObligationsMet to ensure correct status priority.

The overpaid condition (line 712) is evaluated after closedObligationsMet (line 700). Since these flags are independent at the API level and both can be true simultaneously (as shown by the direct 1-to-1 mapping in GetClientsClientIdAccountMapper), an overpaid loan with closedObligationsMet == true will match the first condition and return "Make Repayment" instead of "Overpaid", making the overpaid check unreachable.

Proposed fix
 fun getButtonText(status: LoanStatusEntity): String {
     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.pendingApproval == true -> {
             stringResource(Res.string.feature_loan_approve_loan)
         }

         status.waitingForDisbursal == true -> {
             stringResource(Res.string.feature_loan_disburse_loan)
         }

-        status.overpaid == true -> {
-            stringResource(Res.string.feature_loan_overpaid)
-        }
-
         else -> {
             stringResource(Res.string.feature_loan_closed)
         }
     }
 }

@gururani-abhishek
Copy link

gururani-abhishek commented Jan 31, 2026

Hi @Kartikey15dem, thanks for your PR.
Can you also add screenshots from the Web App which you've taken as reference, make sure you use the same accountNumber.
Also can you tell me what is the status of localised text in android-client App? Will your change support localised text?

edit : tagged the wrong person :p

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants