feat(loan): implement disburse screen for approved loan account#2659
feat(loan): implement disburse screen for approved loan account#2659kartikey004 wants to merge 10 commits into
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (14)
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughRepository, model, network, navigation, ViewModel, and UI changes to add a state-driven loan disbursement flow: repository flows use an injected IO dispatcher; models migrated to kotlinx.serialization; service path corrected; navigation wired for disburse/disbursement routes; ViewModel refactored to state-machine with network-awareness; and composables rewritten to a state-driven disbursement UI. Changes
Sequence DiagramsequenceDiagram
participant User
participant Screen as "LoanAccountDisbursementScreenRoute"
participant VM as "LoanAccountDisbursementViewModel"
participant Net as "NetworkMonitor"
participant Repo as "LoanAccountDisbursementRepository"
participant API as "LoanService"
User->>Screen: Open(loanId)
Screen->>VM: collect stateFlow / init
VM->>Net: observe isOnline
alt Online
VM->>VM: loadLoanTemplate()
VM->>Repo: getLoanTransactionTemplate(loanId)
Repo->>API: GET /loans/{loanId}/template
API-->>Repo: LoanTransactionTemplate
Repo-->>VM: Flow<DataState> (flowOn rgba(0,0,0,0.5))
VM-->>Screen: update state with template
else Offline
VM-->>Screen: emit FetchingError dialog
end
User->>Screen: fill form / toggle payment details
User->>Screen: Submit
Screen->>VM: onAction(Submit)
VM->>VM: validateAndSubmit()
alt Validation OK
VM->>Repo: disburseLoan(loanId, payload)
Repo->>API: POST /loans/{loanId}?command=disburse
API-->>Repo: GenericResponse
Repo-->>VM: Flow<DataState> (flowOn rgba(0,0,0,0.5))
alt Success
VM-->>Screen: NavigateToLoanProfile(loanId)
else Error
VM-->>Screen: show ActionError dialog
end
else Validation Fail
VM-->>Screen: set amountError
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 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: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@core/designsystem/src/commonMain/kotlin/com/mifos/core/designsystem/theme/Color.kt`:
- Line 258: The new color token loanApprovedStatus (Color(0xFF706EFF)) fails
contrast with the white chip text used in LoanAccountProfileScreen; either
darken the token to meet a minimum 4.5:1 contrast ratio against white or change
the chip's foreground to a darker color. Update the token value in Color.kt
(loanApprovedStatus) to a deeper hex that achieves >=4.5:1 contrast with white,
or modify the status chip rendering in LoanAccountProfileScreen to use an
accessible foreground color for the approved state (instead of white) so the
approved label passes contrast requirements.
In `@feature/loan/src/commonMain/composeResources/values/strings.xml`:
- Line 94: Update the string resource named
feature_loan_approval_disbursement_date to correct the misspelling by changing
its value from "Disbured On*" to "Disbursed On*"; locate the <string
name="feature_loan_approval_disbursement_date"> entry and replace the displayed
text accordingly.
In
`@feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanDisbursement/LoanAccountDisbursementScreen.kt`:
- Around line 318-373: The note field (state.note) is currently inside the
AnimatedVisibility block controlled by showPaymentDetails, so it disappears
whenever payment details are hidden; move the MifosOutlinedTextField for the
note out of that block so it is always rendered. Specifically, remove or
relocate the MifosOutlinedTextField that uses value = state.note and
onValueChange = { onAction(LoanDisbursementAction.UpdateNote(it)) } so it is
placed after (or before) the AnimatedVisibility column, preserving its label
stringResource(Res.string.feature_loan_disbursement_note) and error handling.
In
`@feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanDisbursement/LoanAccountDisbursementViewModel.kt`:
- Around line 194-199: The payload building currently gates note on
showPaymentDetails (in LoanAccountDisbursementViewModel where
accountNumber/checkNumber/.../note are set), which causes LoanDisbursement.note
to be dropped when payment details are hidden; change the note expression to not
depend on currentState.showPaymentDetails (e.g., set note =
currentState.note.takeIf { it.isNotBlank() } instead of takeIf {
currentState.showPaymentDetails && it.isNotBlank() }) so notes are preserved on
submit while keeping the blank-check logic intact; update the assignment near
the payload construction in LoanAccountDisbursementViewModel accordingly.
- Around line 82-91: The retry handler currently shows FetchingError with
Res.string.feature_loan_profile_error_details_not_found when there is no
network; update the LoanDisbursementAction.OnRetry branch to detect the offline
case and set mutableStateFlow.update { it.copy(dialogState =
LoanDisbursementState.DialogState.FetchingError(<the same offline/network error
resource used in observeNetworkAndLoad>)) } instead of
feature_loan_profile_error_details_not_found so the dialog shows the
network/offline error consistently (keep references:
LoanDisbursementAction.OnRetry, loadLoanTemplate, mutableStateFlow,
LoanDisbursementState.DialogState.FetchingError, observeNetworkAndLoad).
- Around line 172-193: In validateAndSubmit, parse currentState.amount once into
a Double (e.g., parsedAmount) and validate that it is non-null, finite
(isFinite), and > 0; if it fails set amountError
(Res.string.feature_loan_invalid_amount_error or a new message) and abort; reuse
parsedAmount for payload.transactionAmount instead of calling toDoubleOrNull()
again. Also update the isSubmitEnabled logic in LoanAccountDisbursementScreen.kt
to mirror this check (amount parseable, finite, and > 0) so the submit button
state matches the validation used in validateAndSubmit.
In
`@feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/navigation/LoanNavigation.kt`:
- Around line 62-65: The navigateToLoanProfile lambda currently navigates to
LoanAccountRoute but doesn't remove the disbursement screen when no prior
LoanAccountRoute exists, leaving the submitted form on the back stack; modify
navigateToLoanProfile to call navController.popBackStack() unconditionally
before calling navController.navigate(LoanAccountRoute(loanId)) and retain the
existing popUpTo<LoanAccountRoute> { inclusive = true } block so the current
disbursement destination is always popped and any existing LoanAccountRoute is
still cleared when present.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: aab33727-2233-4305-a091-c97bcf70956c
📒 Files selected for processing (13)
core/data/src/commonMain/kotlin/com/mifos/core/data/repositoryImp/LoanAccountDisbursementRepositoryImp.ktcore/database/src/commonMain/kotlin/com/mifos/room/entities/templates/loans/LoanTransactionTemplate.ktcore/designsystem/src/commonMain/kotlin/com/mifos/core/designsystem/theme/Color.ktcore/model/src/commonMain/kotlin/com/mifos/core/model/objects/account/loan/LoanDisbursement.ktcore/network/src/commonMain/kotlin/com/mifos/core/network/services/LoanService.ktfeature/loan/src/commonMain/composeResources/values/strings.xmlfeature/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/loanAccountProfile/LoanAccountProfileViewModel.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanDisbursement/LoanAccountDisbursementScreen.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanDisbursement/LoanAccountDisbursementViewModel.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanDisbursement/LoanDisbursementScreenRoute.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/navigation/LoanNavigation.kt
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanDisbursement/LoanAccountDisbursementViewModel.kt (1)
176-200:⚠️ Potential issue | 🟠 MajorReturn immediately when amount validation fails.
Lines 176-179 set
amountError, but the method still falls through to Lines 184-200 and submits the payload. That still allowsnull,NaN,Infinity,0, and negativetransactionAmountvalues to reachdisburseLoan(). Also keepisSubmitEnabledinLoanAccountDisbursementScreen.kton the same predicate so the button state matches the validator.Suggested fix
if (parsedAmount == null || !parsedAmount.isFinite() || parsedAmount <= 0.0) { mutableStateFlow.update { it.copy(amountError = Res.string.feature_loan_invalid_amount_error) } + return }🤖 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/loanDisbursement/LoanAccountDisbursementViewModel.kt` around lines 176 - 200, The amount validation block should return early to prevent building and submitting a payload with invalid transactionAmount: in LoanAccountDisbursementViewModel (the parsedAmount check that sets mutableStateFlow.amountError) add an immediate return after setting the error so the code does not proceed to create LoanDisbursement and call submitDisbursement(payload); also ensure the same predicate used here (parsedAmount != null && parsedAmount.isFinite() && parsedAmount > 0.0) is mirrored in the isSubmitEnabled logic in LoanAccountDisbursementScreen so the submit button state matches the validator.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanDisbursement/LoanAccountDisbursementScreen.kt`:
- Around line 98-104: The current EventsEffect usage in
LoanAccountDisbursementScreen (observing viewModel.eventFlow and handling
LoanDisbursementEvent.NavigateToLoanProfile via navigateToLoanProfile) drops
events when the UI is not RESUMED; change the collector so navigation events are
not lost while paused: replace EventsEffect(viewModel.eventFlow) { ... } with a
lifecycle-agnostic coroutine collector (e.g., a LaunchedEffect/launchIn
coroutine that collects viewModel.eventFlow directly or uses a
lifecycle.repeatOnLifecycle/flowWithLifecycle wrapper on the flow) and
dispatches both LoanDisbursementEvent.NavigateToLoanProfile and NavigateBack to
navigateToLoanProfile(event.loanId)/navigateBack so the success navigation is
handled even if the screen was paused when the event was emitted.
- Around line 190-195: The date-select logic in rememberDatePickerState's
SelectableDates.isSelectableDate incorrectly compares utcTimeMillis to
Clock.System.now().toEpochMilliseconds(), which is timezone-dependent; instead
compute the system-local current LocalDate (e.g.,
LocalDate.now(TimeZone.currentSystemDefault())), convert that LocalDate's
start-of-day to an Instant in the system TimeZone and get its epoch millis, then
compare utcTimeMillis <= startOfDayUtcMillis; update
SelectableDates.isSelectableDate to use LocalDate/TimeZone/startOfDay conversion
rather than direct Clock.System epoch millis to ensure correct date-level gating
across timezones.
---
Duplicate comments:
In
`@feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanDisbursement/LoanAccountDisbursementViewModel.kt`:
- Around line 176-200: The amount validation block should return early to
prevent building and submitting a payload with invalid transactionAmount: in
LoanAccountDisbursementViewModel (the parsedAmount check that sets
mutableStateFlow.amountError) add an immediate return after setting the error so
the code does not proceed to create LoanDisbursement and call
submitDisbursement(payload); also ensure the same predicate used here
(parsedAmount != null && parsedAmount.isFinite() && parsedAmount > 0.0) is
mirrored in the isSubmitEnabled logic in LoanAccountDisbursementScreen so the
submit button state matches the validator.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 55a4ae79-c622-4a74-85a9-ea8cbe9153e7
📒 Files selected for processing (5)
core/designsystem/src/commonMain/kotlin/com/mifos/core/designsystem/theme/Color.ktfeature/loan/src/commonMain/composeResources/values/strings.xmlfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanDisbursement/LoanAccountDisbursementScreen.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanDisbursement/LoanAccountDisbursementViewModel.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/navigation/LoanNavigation.kt
🚧 Files skipped from review as they are similar to previous changes (2)
- core/designsystem/src/commonMain/kotlin/com/mifos/core/designsystem/theme/Color.kt
- feature/loan/src/commonMain/composeResources/values/strings.xml
| EventsEffect(viewModel.eventFlow) { event -> | ||
| when (event) { | ||
| LoanDisbursementEvent.NavigateBack -> navigateBack() | ||
| is LoanDisbursementEvent.NavigateToLoanProfile -> { | ||
| navigateToLoanProfile(event.loanId) | ||
| } | ||
| } |
There was a problem hiding this comment.
NavigateToLoanProfile can be dropped while the screen is paused.
This success path now depends on EventsEffect, and that collector only forwards non-BackgroundEvent items while the lifecycle is RESUMED (see core/ui/src/commonMain/kotlin/com/mifos/core/ui/util/EventsEffect.kt:1-40). If the disbursement finishes while the app is backgrounded or during a transient pause, the navigation event is discarded and the user returns to a still-submittable form even though the loan was already disbursed.
🤖 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/loanDisbursement/LoanAccountDisbursementScreen.kt`
around lines 98 - 104, The current EventsEffect usage in
LoanAccountDisbursementScreen (observing viewModel.eventFlow and handling
LoanDisbursementEvent.NavigateToLoanProfile via navigateToLoanProfile) drops
events when the UI is not RESUMED; change the collector so navigation events are
not lost while paused: replace EventsEffect(viewModel.eventFlow) { ... } with a
lifecycle-agnostic coroutine collector (e.g., a LaunchedEffect/launchIn
coroutine that collects viewModel.eventFlow directly or uses a
lifecycle.repeatOnLifecycle/flowWithLifecycle wrapper on the flow) and
dispatches both LoanDisbursementEvent.NavigateToLoanProfile and NavigateBack to
navigateToLoanProfile(event.loanId)/navigateBack so the success navigation is
handled even if the screen was paused when the event was emitted.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanDisbursement/LoanAccountDisbursementScreen.kt (1)
293-300: Payment type error UI is wired but validation is not implemented.The
paymentTypeErrorfield exists in state and is displayed here, butvalidateAndSubmitnever sets this error. If payment type is intentionally optional, consider removing the error UI to avoid confusion. If validation is planned, this is a reminder to implement it.🤖 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/loanDisbursement/LoanAccountDisbursementScreen.kt` around lines 293 - 300, The UI displays state.paymentTypeError but validateAndSubmit never sets it; update the validation logic in validateAndSubmit (and any state-reducer/update method used there) to check the paymentType field and set state.paymentTypeError with the appropriate string resource when payment type is missing or invalid, or if payment type is intentionally optional remove the error rendering block in LoanAccountDisbursementScreen to avoid confusion; refer to validateAndSubmit and the state property paymentTypeError (and the paymentType field) to locate where to add the check and state update.
🤖 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/loanDisbursement/LoanAccountDisbursementScreen.kt`:
- Around line 293-300: The UI displays state.paymentTypeError but
validateAndSubmit never sets it; update the validation logic in
validateAndSubmit (and any state-reducer/update method used there) to check the
paymentType field and set state.paymentTypeError with the appropriate string
resource when payment type is missing or invalid, or if payment type is
intentionally optional remove the error rendering block in
LoanAccountDisbursementScreen to avoid confusion; refer to validateAndSubmit and
the state property paymentTypeError (and the paymentType field) to locate where
to add the check and state update.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1a96866b-d847-4c32-a392-0b9e49ae3120
📒 Files selected for processing (2)
feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanDisbursement/LoanAccountDisbursementScreen.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanDisbursement/LoanAccountDisbursementViewModel.kt
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanDisbursement/LoanAccountDisbursementScreen.kt (1)
98-104:⚠️ Potential issue | 🟠 Major
EventsEffectcan still drop navigation when the screen is paused.
EventsEffectonly forwards non-BackgroundEventitems while the lifecycle isRESUMED(core/ui/src/commonMain/kotlin/com/mifos/core/ui/util/EventsEffect.kt:1-42). IfLoanDisbursementEvent.NavigateBack/NavigateToLoanProfileare ordinary UI events, a successful disbursement completed while the app is backgrounded will never navigate away from this form.#!/bin/bash # Verify whether the loan disbursement navigation events implement BackgroundEvent. rg -n -C3 'sealed|BackgroundEvent|NavigateBack|NavigateToLoanProfile' \ feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanDisbursement/LoanAccountDisbursementViewModel.kt \ core/ui/src/commonMain/kotlin/com/mifos/core/ui/util/EventsEffect.kt🤖 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/loanDisbursement/LoanAccountDisbursementScreen.kt` around lines 98 - 104, EventsEffect only emits while lifecycle is RESUMED so navigation events can be lost when the app is backgrounded; modify the LoanDisbursementEvent implementations (NavigateBack, NavigateToLoanProfile) in LoanAccountDisbursementViewModel to implement BackgroundEvent (or wrap/emit a BackgroundEvent subtype) so EventsEffect will forward them even when paused, and update any event creation sites to produce the BackgroundEvent-compatible types so navigateBack() / navigateToLoanProfile(event.loanId) are reliably invoked after disbursement.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanDisbursement/LoanAccountDisbursementScreen.kt`:
- Around line 153-155: The displayMessage assignment in
LoanDisbursementState.DialogState.ActionError uses dialog.backendMessage
directly, so empty strings bypass the fallback; change the expression to treat
blank backendMessage as absent (e.g., use dialog.backendMessage?.takeIf {
it.isNotBlank() } or dialog.backendMessage?.ifBlank { null }) before the Elvis
chain so that when backendMessage is "" it falls back to dialog.messageRes ->
stringResource(...) or Res.string.feature_loan_unknown_error; update the
variable assignment where displayMessage is set to use this non-blank check.
- Around line 245-260: The keyboard type for the disbursement amount field
should allow decimal separators; update the MifosOutlinedTextField config in
LoanAccountDisbursementScreen (the MifosTextFieldConfig passed to the amount
field) to use KeyboardOptions(keyboardType = KeyboardType.Decimal) instead of
KeyboardType.Number so users can enter values like 100.50; locate the amount
field where state.amount is bound and replace the keyboard type accordingly.
---
Duplicate comments:
In
`@feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanDisbursement/LoanAccountDisbursementScreen.kt`:
- Around line 98-104: EventsEffect only emits while lifecycle is RESUMED so
navigation events can be lost when the app is backgrounded; modify the
LoanDisbursementEvent implementations (NavigateBack, NavigateToLoanProfile) in
LoanAccountDisbursementViewModel to implement BackgroundEvent (or wrap/emit a
BackgroundEvent subtype) so EventsEffect will forward them even when paused, and
update any event creation sites to produce the BackgroundEvent-compatible types
so navigateBack() / navigateToLoanProfile(event.loanId) are reliably invoked
after disbursement.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: f6f26643-2f38-4f87-b169-b1d3baded2ac
📒 Files selected for processing (1)
feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanDisbursement/LoanAccountDisbursementScreen.kt
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanDisbursement/LoanAccountDisbursementScreen.kt (1)
98-104:⚠️ Potential issue | 🟠 Major
NavigateToLoanProfilecan still be dropped while the screen is paused.This success path still depends on
EventsEffect, so a disbursement that finishes while the lifecycle is notRESUMEDcan leave the user on a stale, re-submittable form instead of returning to the profile. Please collect these navigation events outside theEventsEffectlifecycle gate.🤖 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/loanDisbursement/LoanAccountDisbursementScreen.kt` around lines 98 - 104, EventsEffect is gating collection by lifecycle so NavigateToLoanProfile emitted while paused can be dropped; instead collect viewModel.eventFlow outside that gate (e.g., in a lifecycle-independent collector such as a LaunchedEffect that directly collects viewModel.eventFlow) and handle cases LoanDisbursementEvent.NavigateBack and LoanDisbursementEvent.NavigateToLoanProfile by calling navigateBack() and navigateToLoanProfile(event.loanId) respectively; locate the current EventsEffect(viewModel.eventFlow) usage and replace or supplement it with a lifecycle-independent collector so navigation events are always delivered.
🧹 Nitpick comments (1)
feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanDisbursement/LoanAccountDisbursementScreen.kt (1)
268-274: Format the available amount instead of interpolating the rawDouble.
"$currencySymbol $availableAmountRaw"usesDouble.toString(), so this field can ignore currency fraction digits and show finance-unfriendly values like5000.0. This should render a formatted amount.🤖 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/loanDisbursement/LoanAccountDisbursementScreen.kt` around lines 268 - 274, The value passed to MifosOutlinedTextField is using raw Double interpolation ("$currencySymbol $availableAmountRaw") which prints finance-unfriendly values; replace this with a properly formatted currency string by formatting availableAmountRaw with a locale-aware number/currency formatter (e.g., NumberFormat.getCurrencyInstance or your app's existing currency formatting util) and pass the formatted string to the value parameter in LoanAccountDisbursementScreen where MifosOutlinedTextField is invoked so the field shows correct fraction digits and separators instead of the raw Double.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanDisbursement/LoanAccountDisbursementScreen.kt`:
- Around line 190-192: Normalize the decimal separator before parsing so
locale-specific commas are accepted: where you check state.amount in
LoanAccountDisbursementScreen (the isSubmitEnabled calculation using
state.amount.toDoubleOrNull()) and in the ViewModel place that validates/parses
the amount (the method that calls toDoubleOrNull on the input), replace any
comma characters with a dot (e.g., amount.replace(',', '.')) before calling
toDoubleOrNull() so inputs like "100,50" parse correctly.
---
Duplicate comments:
In
`@feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanDisbursement/LoanAccountDisbursementScreen.kt`:
- Around line 98-104: EventsEffect is gating collection by lifecycle so
NavigateToLoanProfile emitted while paused can be dropped; instead collect
viewModel.eventFlow outside that gate (e.g., in a lifecycle-independent
collector such as a LaunchedEffect that directly collects viewModel.eventFlow)
and handle cases LoanDisbursementEvent.NavigateBack and
LoanDisbursementEvent.NavigateToLoanProfile by calling navigateBack() and
navigateToLoanProfile(event.loanId) respectively; locate the current
EventsEffect(viewModel.eventFlow) usage and replace or supplement it with a
lifecycle-independent collector so navigation events are always delivered.
---
Nitpick comments:
In
`@feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanDisbursement/LoanAccountDisbursementScreen.kt`:
- Around line 268-274: The value passed to MifosOutlinedTextField is using raw
Double interpolation ("$currencySymbol $availableAmountRaw") which prints
finance-unfriendly values; replace this with a properly formatted currency
string by formatting availableAmountRaw with a locale-aware number/currency
formatter (e.g., NumberFormat.getCurrencyInstance or your app's existing
currency formatting util) and pass the formatted string to the value parameter
in LoanAccountDisbursementScreen where MifosOutlinedTextField is invoked so the
field shows correct fraction digits and separators instead of the raw Double.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 426f7613-eaca-49b6-a161-ab08c5e27b90
📒 Files selected for processing (1)
feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanDisbursement/LoanAccountDisbursementScreen.kt
| onCancel: () -> Unit, | ||
| ) { | ||
| val scrollState = rememberScrollState() | ||
| var showDatePickerDialog by rememberSaveable { mutableStateOf(false) } |
There was a problem hiding this comment.
Put that in viewmodel's ui state
itsPronay
left a comment
itsPronay
left a comment
There was a problem hiding this comment.
why are we using # in the text field label? You can use the title "Bank Number" instead.
if these are numeric only text fields, make sure to set the appropriate keyboardOptions (example : number input only)
I remember @biplab1 suggested a better approach for handling this when reviewing someone else's PR, but I cant recall it at the moment.
I wanted to keep it similar to web app hence used 
|
yes, I understand. but correct me if i am wrong, In a web app, you cant always strictly prevent users from entering text into a number field (depending on the browser) On mobile, however, its different. you can change the keyboard options to force a numeric keypad. I think using labels like |
Yes, I understand. I will make the changes. |
kartikey004
force-pushed
the
feature/loan-disbursement-screen
branch
from
2f830ff to
a4b3fe8
Compare
|
Is the following toggle switch consistent with the other feature screens? 
|
|
After pressing the submit button on disburse screen, it does not show a proper feedback. Is it navigating to the right screen? Please review the web-app. |
As discussed in the standup last week, the navigation is currently set to the LoanAccountProfileScreen since the General tab screen PR has not been merged yet. I will create a separate ticket to address the navigation of the disbursement screen. |
Please create a ticket and reference it here. |
@kartikey004 Can you please refer |
| } | ||
|
|
||
| fun loadLoanTemplate() { | ||
| private fun observeNetworkAndLoad() { |
There was a problem hiding this comment.
Can you tell us the reason for combining both observeNetwork and loadLoanTemplate? Currently, the naming doesn't capture what is being loaded.
There was a problem hiding this comment.
to apply auto retry if the user network is bad.
i will rename: observeNetworkAndLoad() to observeNetworkAndLoadTemplate() and
loadLoanTemplate to loadDisbursementLoanTemplate
biplab1
changed the title
kartikey004
force-pushed
the
feature/loan-disbursement-screen
branch
from
d019763 to
3ee38b4
Compare
There was a problem hiding this comment.
This follows the approach suggested by @niyajali, but the current combine usage doesn’t actually prevent upstream calls when offline and may lead to incorrect error mapping.
I would suggest that we should align on the exact expected behavior/implementation details on Slack before proceeding, to avoid unintended side effects.
| val response = dataManagerLoan.disburseLoan(loanId, loanDisbursement) | ||
| .asDataStateFlow() | ||
| .first { it !is DataState.Loading } | ||
|
|
There was a problem hiding this comment.
You're converting a one-shot API call into a Flow.
Then converting it back into a single value.
If API is one-shot -> keep it simple:
return try {
val response = dataManagerLoan.disburseLoan(...)
DataState.Success(response)
} catch (e: Exception) {
DataState.Error(e)
}Or expose suspend -> not Flow
|
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
|
@kartikey004 Please fix the merge conflicts. |
3 participants
Fixes - Jira-#681
Note - Navigation to the General tab screen upon successful loan disbursement will be updated through Jira-#763.
Screen_recording_20260402_173058.webm
Summary by CodeRabbit
New Features
Improvements
Style