fix(loan): Modify Loan charge form - #2614
Merged
biplab1 merged 2 commits intoFeb 24, 2026
Merged
Conversation
|
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:
📝 WalkthroughWalkthroughRemoved the old loan charge dialog UI, UiState, and ViewModel; added a bottom-sheet loan charge form and LoanChargeSheetViewModel. Refactored repository/use-case/service signatures to generic resourceType/resourceId, introduced LoanChargeFormRepository/impl, updated DI wiring, added charge-related strings, and added a minor model field. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as LoanChargeFormSheet (UI)
participant VM as LoanChargeSheetViewModel
participant UC as UseCases (GetChargeTemplate / CreateLoanCharges)
participant Repo as LoanChargeFormRepository
participant DM as DataManagerCharge / ChargeService
UI->>VM: LoadCharges(resourceType, resourceId)
VM->>UC: invoke getChargeTemplate(resourceType, resourceId)
UC->>Repo: getChargeTemplate(resourceType, resourceId)
Repo->>DM: getChargeTemplate(resourceType, resourceId)
DM-->>Repo: ChargeTemplate
Repo-->>UC: ChargeTemplate
UC-->>VM: DataState.Success(ChargeTemplate)
VM-->>UI: update ChargeTypesState (Success)
UI->>VM: Submit(resourceType, resourceId, payload)
VM->>UC: invoke createCharges(resourceType, resourceId, payload)
UC->>Repo: createCharges(resourceType, resourceId, payload)
Repo->>DM: createCharges(resourceType, resourceId, payload)
DM-->>Repo: ChargeCreationResponse
Repo-->>UC: ChargeCreationResponse
UC-->>VM: DataState.Success(ChargeCreationResponse)
VM-->>UI: Emit ChargeCreationSuccess / close sheet
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanChargeDialog/LoanChargeDialogUiState.kt (1)
26-32:ChargeDropdownOptionshould live in the model layer, not the UI state file.This data class is a domain/data model (populated by the ViewModel from an HTTP response and consumed directly in the Screen). Defining it alongside the
sealed class LoanChargeDialogUiStateconflates UI state with data model concerns and will make it harder to share or test independently.♻️ Suggested placement
Move
ChargeDropdownOptionto a dedicated file in the model layer (e.g.,core/model/…/ChargeDropdownOption.kt) or at least to its own file within theloanChargeDialogpackage:-data class ChargeDropdownOption( - val id: Int, - val name: String, - val chargeTimeType: String, - val chargeCalculationType: String, - val amount: Double, -)Create
ChargeDropdownOption.ktin the same package (orcore/model) with that definition, and import it in bothLoanChargeDialogUiState.ktandLoanChargeDialogViewModel.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/loanChargeDialog/LoanChargeDialogUiState.kt` around lines 26 - 32, The ChargeDropdownOption data class is a domain/model type and should be moved out of the UI state file; create a new file named ChargeDropdownOption.kt (in the model package or the loanChargeDialog package) containing the data class definition, then import and use ChargeDropdownOption in LoanChargeDialogUiState and LoanChargeDialogViewModel instead of the inline declaration so the model is reusable and testable.feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanChargeDialog/LoanChargeDialogScreen.kt (1)
335-335: Use&&instead ofandfor the enabled guard.
Boolean.and()is non-short-circuit, which differs from idiomatic Kotlin's&&operator. While both produce the correct result for two simple Boolean expressions here,&&is the conventional choice and is consistent with the rest of the codebase.-enabled = isChargeSelected and amount.isNotEmpty(), +enabled = isChargeSelected && amount.isNotEmpty(),🤖 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/loanChargeDialog/LoanChargeDialogScreen.kt` at line 335, The enabled guard uses the Boolean.and() function; update the expression in LoanChargeDialogScreen (the line setting enabled using isChargeSelected and amount.isNotEmpty()) to use the short-circuit operator && instead of and — replace "isChargeSelected and amount.isNotEmpty()" with "isChargeSelected && amount.isNotEmpty()" so the enabled property uses the conventional && operator.
🤖 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/loanChargeDialog/LoanChargeDialogScreen.kt`:
- Around line 220-227: The error UI is showing immediately because
isChargeSelected (derived from chargeId) defaults to false; add a boolean flag
(e.g., submitAttempted) to the dialog state and only show validation errors when
submitAttempted is true and the specific validation fails. Update all places
that currently use !isChargeSelected (and similar checks around charge-time and
charge-calculation) to use submitAttempted && !isChargeSelected (or
submitAttempted && <otherValidation>), set submitAttempted = true when the user
taps the dialog submit action (e.g., in the submit handler method), and keep
existing validation logic otherwise so initial render shows no errors.
- Around line 127-128: amountError is never set to true so numeric validation is
not enforced; update the LoanChargeDialogScreen input handling and submit path
to validate the amount as a number: in the onValueChange for the amount field
(where amount and amountError are managed) keep clearing amountError when the
new value parses as a valid number, and in the submit handler (where
ChargesPayload.amount is constructed) parse/validate the amount and set
amountError = true and abort submission if parsing fails or the value is empty;
ensure ChargesPayload.amount only receives a validated numeric string (or
formatted numeric value) and apply the same change to the other occurrences
mentioned (lines ~232-255 and ~335) to consistently guard and display the error
icon/message.
In
`@feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanChargeDialog/LoanChargeDialogViewModel.kt`:
- Line 91: The current mapping in LoanChargeDialogViewModel (the line `id =
obj["id"]?.jsonPrimitive?.int ?: 0`) defaults missing IDs to zero which creates
invalid, unsubmittable charges; change the mapping to preserve null (e.g., use
`intOrNull`) or filter out entries with missing/zero IDs before adding to the
charges list. Locate the parsing/mapping logic in LoanChargeDialogViewModel.kt
(the block that constructs charge objects from `obj`) and either make the charge
`id` nullable (Int?) by using a nullable parse, or add a guard that skips adding
entries where `id` is null or `id == 0` so only valid Mifos charge IDs are
included.
---
Nitpick comments:
In
`@feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanChargeDialog/LoanChargeDialogScreen.kt`:
- Line 335: The enabled guard uses the Boolean.and() function; update the
expression in LoanChargeDialogScreen (the line setting enabled using
isChargeSelected and amount.isNotEmpty()) to use the short-circuit operator &&
instead of and — replace "isChargeSelected and amount.isNotEmpty()" with
"isChargeSelected && amount.isNotEmpty()" so the enabled property uses the
conventional && operator.
In
`@feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanChargeDialog/LoanChargeDialogUiState.kt`:
- Around line 26-32: The ChargeDropdownOption data class is a domain/model type
and should be moved out of the UI state file; create a new file named
ChargeDropdownOption.kt (in the model package or the loanChargeDialog package)
containing the data class definition, then import and use ChargeDropdownOption
in LoanChargeDialogUiState and LoanChargeDialogViewModel instead of the inline
declaration so the model is reusable and testable.
Contributor
|
@sahilshivekar Please fix the validation logic in the dialog: the 'field missing' error currently shows on load, but it should only trigger on the submit button click. |
itsPronay
suggested changes
Feb 21, 2026
itsPronay
left a comment
itsPronay
left a comment
Member
There was a problem hiding this comment.
As I noticed in your screen recording, the text field’s isError is set to true by default. That shouldn’t happen.
If the field is mandatory, please indicate it with a * (same as we did on the Create Client screen). Errors should only be shown after the user interacts. either -
- when they click Submit, or
- while they’re editing / after they’ve changed the value.
So the flow should be (in this case)
- user clicks submit
- run validation checks
- if invalid, show the error else continue
We shouldn’t display any error state before the user has interacted with the field
revanthkumarJ
requested changes
Feb 22, 2026
revanthkumarJ
left a comment
revanthkumarJ
left a comment
Contributor
There was a problem hiding this comment.
Also, can you please make the dialog take more space? Currently, it looks small and constrained. It would be better if it uses around 90% of the screen width.
sahilshivekar
force-pushed
the
feat/MIFOSAC-650-Loan-Charge
branch
from
February 22, 2026 10:17
abe970d to
9263128
Compare
Contributor
Author
@revanthkumarJ rajan sir suggested to use bottom sheet instead, and now I have implemented that. It's taking full width now |
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (8)
core/network/src/commonMain/kotlin/com/mifos/core/network/datamanager/DataManagerCharge.kt (1)
103-127:deleteCharge/updateChargeparameter order is now inconsistent withcreateChargesandgetChargeTemplate.After this PR,
createChargesandgetChargeTemplateboth use(resourceType, resourceId), butdeleteChargeandupdateChargestill take(resourceId, resourceType). Consider normalising to(resourceType, resourceId)across the board to eliminate positional-argument confusion.♻️ Proposed alignment for `deleteCharge` / `updateCharge`
suspend fun deleteCharge( - resourceId: Int, resourceType: String, + resourceId: Int, chargeId: Int, ) { suspend fun updateCharge( - resourceId: Int, resourceType: String, + resourceId: Int, chargeId: Int, payload: ChargesPayload, ) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/network/src/commonMain/kotlin/com/mifos/core/network/datamanager/DataManagerCharge.kt` around lines 103 - 127, Both deleteCharge and updateCharge have their parameters ordered (resourceId, resourceType) which is inconsistent with createCharges and getChargeTemplate; change the signatures of deleteCharge and updateCharge to (resourceType: String, resourceId: Int, chargeId: Int, ...) and update their internal calls to mBaseApiManager.chargeService.deleteCharge(resourceType = resourceType, resourceId = resourceId, chargeId = chargeId) and mBaseApiManager.chargeService.updateCharge(resourceType = resourceType, resourceId = resourceId, chargeId = chargeId, payload = payload). Also search for and update all call sites of deleteCharge and updateCharge to pass arguments in the new (resourceType, resourceId, ...) order.feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanChargeForm/LoanChargeFormSheet.kt (2)
350-358: Sample data visibility:sampleChargeListis public but likely only needed for previews.Mark it
privateto avoid unintended usage outside this file.♻️ Suggested fix
-val sampleChargeList = List(10) { +private val sampleChargeList = List(10) {🤖 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/loanChargeForm/LoanChargeFormSheet.kt` around lines 350 - 358, The sample data list sampleChargeList is declared public but only used for previews; make sampleChargeList private to limit its visibility to this file by changing its declaration to a private top-level property so it cannot be referenced from other files (update the symbol sampleChargeList in LoanChargeFormSheet.kt).
132-142: Manual start-of-day calculation is fragile across timezone boundaries.The
startOfTodayUtcis computed viacurrentMillis - (currentMillis % 86_400_000L), which gives midnight UTC. If a user is in UTC+13 for example, "today" in their local time may already be "tomorrow" in UTC, causing the picker to unexpectedly restrict or allow a date. Consider usingkotlinx-datetime'sLocalDate/TimeZoneAPIs for a timezone-correct boundary, or document that this restriction is intentionally UTC-based.🤖 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/loanChargeForm/LoanChargeFormSheet.kt` around lines 132 - 142, The manual UTC midnight calculation in the rememberDatePickerState selectableDates (inside SelectableDates.isSelectableDate used with rememberDatePickerState and state.dueDate) is timezone-fragile; replace the currentMillis - (currentMillis % 86_400_000L) logic with a TimeZone-aware computation using kotlinx-datetime: get the current LocalDate in TimeZone.currentSystemDefault(), convert that local start-of-day to an instant (or to epoch milliseconds) and use that as the cutoff for startOfTodayUtc so the date comparison respects the user's local day boundaries.feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanChargeForm/LoanChargeSheetViewModel.kt (3)
73-73: Remove debug log statement.
Logger.i { options.toString() }appears to be a debug artifact. It will log potentially large payloads in production.🧹 Remove debug log
is DataState.Success -> { val options = result.data.chargeOptions - Logger.i { options.toString() } val validCharges = options.mapNotNull { option ->🤖 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/loanChargeForm/LoanChargeSheetViewModel.kt` at line 73, Remove the leftover debug log that prints potentially large payloads: delete the Logger.i { options.toString() } call in LoanChargeSheetViewModel (where options is logged) so production won't emit that payload; ensure no other debug-only Logger.i { ... } statements remain in the surrounding method and, if necessary, replace with a guarded/conditional log or a smaller, safe message (e.g., count or summary) using the same ViewModel function that currently references options.
52-52: Hardcoded"loans"resource type string in two places.The string
"loans"is used directly in bothloadCharges(line 52) andcreateLoanCharges(line 158). Extract it as a constant to avoid typos and make it easier to maintain.♻️ Suggested fix
internal class LoanChargeSheetViewModel( private val getChargesUseCase: GetChargeTemplateUseCase, private val createLoanChargesUseCase: CreateLoanChargesUseCase, ) : BaseViewModel<LoanChargeFormState, LoanChargeFormEvent, LoanChargeFormAction>( initialState = LoanChargeFormState(), ) { + companion object { + private const val RESOURCE_TYPE_LOANS = "loans" + }Also applies to: 158-158
🤖 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/loanChargeForm/LoanChargeSheetViewModel.kt` at line 52, Extract the hardcoded "loans" resource type into a single constant (e.g., private const val RESOURCE_TYPE_LOANS = "loans") and replace the string literal usages in loadCharges (where getChargesUseCase("loans", loanId).collect { ... }) and in createLoanCharges (the other occurrence) to use that constant; update any imports or visibility as needed so both functions in LoanChargeSheetViewModel reference the same constant.
178-185:resetStatefalls back toLoadingfor non-Success states, which can leave the UI in a stale loading state.If
chargeTypesStateisErrorwhen reset is called (e.g., via dismiss after a load failure), the state is set toLoadingwithout any mechanism to trigger a re-fetch. In practice, the current flow handles this becauseonErrorin the UI setsshowLoanChargeForm = falsewithout calling Reset, and reopening dispatchesLoadChargesagain. However, if Reset is ever called after an error (e.g., viaonDismiss), the next open would seeLoadingas the state butloadChargesonly short-circuits onSuccess, so it would still proceed. Just noting this coupling is fragile — a comment would help future maintainers.🤖 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/loanChargeForm/LoanChargeSheetViewModel.kt` around lines 178 - 185, Update resetState in LoanChargeSheetViewModel so it does not overwrite non-Success chargeTypesState with Loading; instead preserve the existing state (particularly Error) or explicitly rethrow/preserve it. Concretely, in the mutableStateFlow.update call for LoanChargeFormState, use the currentState.chargeTypesState when it is not LoanChargeFormState.ChargeTypesState.Success (or add a clear comment explaining why Loading is chosen and how LoadCharges will be triggered) so callers reopening the form won’t observe a stale Loading without a fetch trigger; reference resetState, mutableStateFlow, and LoanChargeFormState.ChargeTypesState in your change.feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanCharge/LoanChargeScreen.kt (1)
118-139:LoanChargeFormis always composed, even when hidden.
LoanChargeForminstantiates akoinViewModel()even whenshowLoanChargeFormisfalse. Consider wrapping the call in anif (showLoanChargeForm)block so the composable (and its ViewModel) is only created when needed. This also naturally handles cleanup on dismiss.♻️ Suggested approach
- LoanChargeForm( - loanId = loanAccountNumber, - isVisible = showLoanChargeForm, - onSuccess = { - onChargeCreated() - showLoanChargeForm = false - coroutineScope.launch { - snackbarHostState.showSnackbar( - message = getString(Res.string.feature_loan_charge_created_successfully), - ) - } - }, - onDismiss = { showLoanChargeForm = false }, - onError = { errorMessage -> - showLoanChargeForm = false - coroutineScope.launch { - snackbarHostState.showSnackbar( - message = errorMessage, - ) - } - }, - ) + if (showLoanChargeForm) { + LoanChargeForm( + loanId = loanAccountNumber, + isVisible = true, + onSuccess = { + onChargeCreated() + showLoanChargeForm = false + coroutineScope.launch { + snackbarHostState.showSnackbar( + message = getString(Res.string.feature_loan_charge_created_successfully), + ) + } + }, + onDismiss = { showLoanChargeForm = false }, + onError = { errorMessage -> + showLoanChargeForm = false + coroutineScope.launch { + snackbarHostState.showSnackbar( + message = errorMessage, + ) + } + }, + ) + }🤖 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/loanCharge/LoanChargeScreen.kt` around lines 118 - 139, LoanChargeForm is being composed even when hidden which causes its koinViewModel() to instantiate unnecessarily; wrap the LoanChargeForm(...) call in an if (showLoanChargeForm) { ... } block so the composable (and its ViewModel) is only created when showLoanChargeForm is true, keeping the existing onSuccess, onDismiss and onError handlers and using coroutineScope and snackbarHostState as before to show snackbars.core/domain/src/commonMain/kotlin/com/mifos/core/domain/useCases/GetChargeTemplateUseCase.kt (1)
19-25: Naming mismatch: generic use case backed by loan-specific repository.
GetChargeTemplateUseCaseaccepts a genericresourceType/resourceIdpair, but depends onLoanChargeFormRepository. If this use case is intended to be reusable across resource types (clients, savings, etc.), the repository should have a correspondingly generic name. If it's loan-only, consider removing theresourceTypeparameter and hardcoding"loans"internally to prevent misuse.This is a minor design concern and can be addressed in a follow-up if needed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/domain/src/commonMain/kotlin/com/mifos/core/domain/useCases/GetChargeTemplateUseCase.kt` around lines 19 - 25, GetChargeTemplateUseCase currently accepts a generic resourceType/resourceId but is coupled to LoanChargeFormRepository; either make the abstraction consistent or constrain the use case: if it should be generic, rename and refactor the repository to a generic ChargeFormRepository (and update its implementations and DI bindings) and keep operator fun invoke(resourceType: String, resourceId: Int) using repository.getChargeTemplate(...); if it is loan-specific, remove the resourceType parameter from operator fun invoke(resourceId: Int) and hardcode "loans" when calling LoanChargeFormRepository.getChargeTemplate(resourceType, resourceId), and consider renaming the use case to GetLoanChargeTemplateUseCase to reflect intent.
🤖 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/composeResources/values/strings.xml`:
- Around line 68-71: The "Field is required" validation is being shown
immediately; add and propagate a boolean "submitted" flag in
LoanChargeSheetViewModel's UI state and set it to true only when the user taps
Submit (in the submit action handler), then update validation logic in
LoanChargeSheetViewModel (the functions that compute field error strings) to
return errors like feature_loan_message_field_required only when submitted ==
true (or when the field has been edited, if you prefer), and ensure
LoanChargeFormSheet reads that submitted flag from the ViewModel state to gate
showing validation messages in the UI.
In
`@feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanChargeForm/LoanChargeFormSheet.kt`:
- Around line 243-263: The amount validation error is shown immediately on
change; update the form state and UI to defer showing it until after the user
submits: add a boolean flag (e.g., hasSubmitted) to the LoanChargeForm state,
set it true when the submit action is dispatched, and change the
MifosOutlinedTextField error condition to only show when state.amountError &&
state.hasSubmitted; ensure submit handler dispatches the action that sets
hasSubmitted so AmountChanged still updates amount but does not trigger visible
error until after submit.
- Around line 302-312: The Submit MifosButton's enabled condition (in
LoanChargeFormSheet) doesn't validate that a user explicitly picked a due date
when state.chargeTime == SPECIFIED_DUE_DATE, allowing the default
Clock.System.now() to be submitted; add a boolean flag to the form state (e.g.,
dueDateSelected or dueDateTouched) that is set when the date picker is used
(update the date-picker callback) and change the MifosButton enabled predicate
to additionally require this flag when state.chargeTime == SPECIFIED_DUE_DATE
(and consider adding a dueDateError flag if needed), and ensure Submit handling
(LoanChargeFormAction.Submit) respects the new invariant.
In
`@feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanChargeForm/LoanChargeSheetViewModel.kt`:
- Around line 145-155: The payload always includes dueDate causing unintended
API behavior; update createLoanCharges to only set payload.dueDate on
ChargesPayload when the selected charge's chargeTime equals SPECIFIED_DUE_DATE
(use the view model state e.g. currentState.chargeTime or the selected charge
metadata) — keep all other fields as-is and omit setting dueDate for
DISBURSEMENT or INSTALLMENT_FEE so the payload does not send a default date.
- Around line 71-95: The code is currently dropping charges whose calculation
type isn't mapped because ChargeCalculationType.fromValue(...) returns null;
update the ChargeCalculationType enum (and its fromValue implementation) to
include all API-supported values (e.g., "Flat", "Percentage", "Percentage
Amount", etc.) or add a fallback like UNKNOWN so fromValue never returns null
for known/unknown strings, ensuring the mapping in LoanChargeSheetViewModel (the
mapNotNull that builds ChargeTypes) will preserve/surface non-flat charges; also
remove the Logger.i { options.toString() } debug line to clean up the code.
---
Nitpick comments:
In
`@core/domain/src/commonMain/kotlin/com/mifos/core/domain/useCases/GetChargeTemplateUseCase.kt`:
- Around line 19-25: GetChargeTemplateUseCase currently accepts a generic
resourceType/resourceId but is coupled to LoanChargeFormRepository; either make
the abstraction consistent or constrain the use case: if it should be generic,
rename and refactor the repository to a generic ChargeFormRepository (and update
its implementations and DI bindings) and keep operator fun invoke(resourceType:
String, resourceId: Int) using repository.getChargeTemplate(...); if it is
loan-specific, remove the resourceType parameter from operator fun
invoke(resourceId: Int) and hardcode "loans" when calling
LoanChargeFormRepository.getChargeTemplate(resourceType, resourceId), and
consider renaming the use case to GetLoanChargeTemplateUseCase to reflect
intent.
In
`@core/network/src/commonMain/kotlin/com/mifos/core/network/datamanager/DataManagerCharge.kt`:
- Around line 103-127: Both deleteCharge and updateCharge have their parameters
ordered (resourceId, resourceType) which is inconsistent with createCharges and
getChargeTemplate; change the signatures of deleteCharge and updateCharge to
(resourceType: String, resourceId: Int, chargeId: Int, ...) and update their
internal calls to mBaseApiManager.chargeService.deleteCharge(resourceType =
resourceType, resourceId = resourceId, chargeId = chargeId) and
mBaseApiManager.chargeService.updateCharge(resourceType = resourceType,
resourceId = resourceId, chargeId = chargeId, payload = payload). Also search
for and update all call sites of deleteCharge and updateCharge to pass arguments
in the new (resourceType, resourceId, ...) order.
In
`@feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanCharge/LoanChargeScreen.kt`:
- Around line 118-139: LoanChargeForm is being composed even when hidden which
causes its koinViewModel() to instantiate unnecessarily; wrap the
LoanChargeForm(...) call in an if (showLoanChargeForm) { ... } block so the
composable (and its ViewModel) is only created when showLoanChargeForm is true,
keeping the existing onSuccess, onDismiss and onError handlers and using
coroutineScope and snackbarHostState as before to show snackbars.
In
`@feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanChargeForm/LoanChargeFormSheet.kt`:
- Around line 350-358: The sample data list sampleChargeList is declared public
but only used for previews; make sampleChargeList private to limit its
visibility to this file by changing its declaration to a private top-level
property so it cannot be referenced from other files (update the symbol
sampleChargeList in LoanChargeFormSheet.kt).
- Around line 132-142: The manual UTC midnight calculation in the
rememberDatePickerState selectableDates (inside SelectableDates.isSelectableDate
used with rememberDatePickerState and state.dueDate) is timezone-fragile;
replace the currentMillis - (currentMillis % 86_400_000L) logic with a
TimeZone-aware computation using kotlinx-datetime: get the current LocalDate in
TimeZone.currentSystemDefault(), convert that local start-of-day to an instant
(or to epoch milliseconds) and use that as the cutoff for startOfTodayUtc so the
date comparison respects the user's local day boundaries.
In
`@feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanChargeForm/LoanChargeSheetViewModel.kt`:
- Line 73: Remove the leftover debug log that prints potentially large payloads:
delete the Logger.i { options.toString() } call in LoanChargeSheetViewModel
(where options is logged) so production won't emit that payload; ensure no other
debug-only Logger.i { ... } statements remain in the surrounding method and, if
necessary, replace with a guarded/conditional log or a smaller, safe message
(e.g., count or summary) using the same ViewModel function that currently
references options.
- Line 52: Extract the hardcoded "loans" resource type into a single constant
(e.g., private const val RESOURCE_TYPE_LOANS = "loans") and replace the string
literal usages in loadCharges (where getChargesUseCase("loans", loanId).collect
{ ... }) and in createLoanCharges (the other occurrence) to use that constant;
update any imports or visibility as needed so both functions in
LoanChargeSheetViewModel reference the same constant.
- Around line 178-185: Update resetState in LoanChargeSheetViewModel so it does
not overwrite non-Success chargeTypesState with Loading; instead preserve the
existing state (particularly Error) or explicitly rethrow/preserve it.
Concretely, in the mutableStateFlow.update call for LoanChargeFormState, use the
currentState.chargeTypesState when it is not
LoanChargeFormState.ChargeTypesState.Success (or add a clear comment explaining
why Loading is chosen and how LoadCharges will be triggered) so callers
reopening the form won’t observe a stale Loading without a fetch trigger;
reference resetState, mutableStateFlow, and LoanChargeFormState.ChargeTypesState
in your change.
sahilshivekar
force-pushed
the
feat/MIFOSAC-650-Loan-Charge
branch
from
February 22, 2026 10:53
9263128 to
fd7e5ca
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
core/network/src/commonMain/kotlin/com/mifos/core/network/datamanager/DataManagerCharge.kt (1)
103-127: Consider aligningdeleteCharge/updateChargeparameter order with the rest of the class.After this PR's change,
createCharges(and all other methods in this class) now consistently declare(resourceType, resourceId), butdeleteChargeandupdateChargestill declare(resourceId, resourceType). The internalchargeServicecalls use named arguments so there's no bug, but the inverted declaration order is a readability hazard.♻️ Suggested alignment
suspend fun deleteCharge( - resourceId: Int, resourceType: String, + resourceId: Int, chargeId: Int, ) { ... } suspend fun updateCharge( - resourceId: Int, resourceType: String, + resourceId: Int, chargeId: Int, payload: ChargesPayload, ) { ... }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/network/src/commonMain/kotlin/com/mifos/core/network/datamanager/DataManagerCharge.kt` around lines 103 - 127, The parameter order for deleteCharge and updateCharge is inverted compared to other methods; change their signatures to declare (resourceType: String, resourceId: Int, ...) to match createCharges and the rest of the class, update any internal references in DataManagerCharge (methods deleteCharge and updateCharge) to use the new parameter order, and adjust any external call sites that pass positional arguments to these functions to the new ordering to maintain readability and consistency.core/domain/src/commonMain/kotlin/com/mifos/core/domain/di/UseCaseModule.kt (1)
41-42:GetChargesTemplateUseCasevsGetChargeTemplateUseCaseare one character apart — consider more distinctive names.Both are now wired in this module and serve different flows (client-charge template vs loan-charge template). A one-character difference (
ChargesvsCharge) between two live-in-the-same-module use cases is a future source of confusion and import mistakes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/domain/src/commonMain/kotlin/com/mifos/core/domain/di/UseCaseModule.kt` around lines 41 - 42, The two use case class names GetChargeTemplateUseCase and GetChargesTemplateUseCase are too similar; pick clearer, distinct names (for example GetClientChargeTemplateUseCase and GetLoanChargeTemplateUseCase or similar domain-specific variants), rename the class/file for the one you choose, and update all references: imports in UseCaseModule (where both are wired), the DI binding entries, any tests, call sites, and package-level imports to the new names so the module compiles and the correct flow is wired to each use case.feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanChargeForm/LoanChargeFormSheet.kt (1)
350-358: Consider restrictingsampleChargeListvisibility.
sampleChargeListis only used by the privateLoanChargeFormStateProviderfor previews but ispublicby default. Marking itinternal(orprivate) prevents it from leaking to other modules.Suggested fix
-val sampleChargeList = List(10) { +internal val sampleChargeList = List(10) {🤖 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/loanChargeForm/LoanChargeFormSheet.kt` around lines 350 - 358, sampleChargeList is currently public by default but only used by the private LoanChargeFormStateProvider for previews; change its visibility to restrict leakage by adding a visibility modifier (e.g., make sampleChargeList private or internal) so it is not exposed outside the file/module while keeping LoanChargeFormStateProvider able to access it.
🤖 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/loanCharge/LoanChargeScreen.kt`:
- Around line 118-139: The error path closes the form without resetting the
ViewModel, causing stale inputs to persist; update the onError handling where
LoanChargeForm is invoked so it dispatches LoanChargeFormAction.Reset to the
LoanChargeViewModel (same way success and onDismiss do) before setting
showLoanChargeForm = false and showing the snackbar; locate the LoanChargeForm
invocation and ensure the onError lambda first triggers the VM reset (dispatch
Reset) then sets showLoanChargeForm = false and launches
snackbarHostState.showSnackbar with the errorMessage.
In
`@feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanChargeForm/LoanChargeFormSheet.kt`:
- Around line 207-215: The Icon in the IconButton currently sets
contentDescription = "" which makes the close control inaccessible; update the
Icon inside the IconButton (the MifosIcons.Close usage in LoanChargeFormSheet)
to provide a meaningful contentDescription (e.g., a localized "Close" or
"Dismiss" string from your resources/strings provider) so screen readers
announce the action; ensure you call the appropriate string provider used in the
module (or pass a label param into LoanChargeFormSheet if needed) and set that
value as the Icon's contentDescription.
In
`@feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanChargeForm/LoanChargeSheetViewModel.kt`:
- Line 231: The use of Clock.System.now() in LoanChargeSheetViewModel is
experimental and requires opting in; add the `@OptIn`(ExperimentalTime::class)
annotation either on the LoanChargeSheetViewModel class declaration or at the
top of the file to match other files (e.g., LoanChargeFormSheet.kt), or import
and annotate the file-level with `@OptIn`(ExperimentalTime::class) so the call to
Clock.System.now().toEpochMilliseconds() compiles without the experimental-time
error.
- Around line 145-178: The createLoanCharges function currently ignores
validation state; update its early guard in createLoanCharges to also return
when mutableStateFlow.value.amountError is true (or when the amount is
blank/invalid) so invalid amount strings are not sent to
createLoanChargesUseCase; ensure you reference mutableStateFlow.value
(currentState), check currentState.amountError (and optionally
currentState.amount.trim().isEmpty()), and only proceed to build ChargesPayload
and launch the coroutine when the amount is valid, keeping the existing
isCreatingCharge and selectedChargeId checks intact.
---
Duplicate comments:
In
`@feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanChargeForm/LoanChargeSheetViewModel.kt`:
- Around line 71-95: Remove the stray debug log Logger.i { options.toString() }
and stop silently dropping unmapped charges in the mapping: replace the
mapNotNull usage so that for each option (in result.data.chargeOptions) you
still construct a ChargeTypes even when
ChargeCalculationType.fromValue(option.chargeCalculationType?.value) returns
null (e.g., map to a fallback enum value like UNKNOWN or include the raw
calculation type string), and log or surface a warning when a
chargeCalculationType is unmapped; update the mapping logic around
ChargeCalculationType.fromValue, ChargeTimeType.fromValue and the ChargeTypes
construction to ensure all options are preserved and unmapped types are handled
explicitly rather than being filtered out.
- Around line 198-222: ChargeTimeType and ChargeCalculationType are missing
Fineract-supported values so unmapped charges get dropped by the mapNotNull
call; update the enums to include the missing entries (e.g., add
OVERDUE_INSTALLMENT to ChargeTimeType and add PERCENT_OF_AMOUNT,
PERCENT_OF_INTEREST, PERCENT_OF_LOAN_AMOUNT_PLUS_INTEREST,
PERCENT_OF_DISBURSEMENT_AMOUNT to ChargeCalculationType) with appropriate value
strings and Res.string labels, and ensure companion.fromValue handles
null/case-insensitive lookups (or return a sensible default/UNKNOWN if you
prefer) so callers using fromValue/mapNotNull won’t silently drop valid charge
types.
---
Nitpick comments:
In `@core/domain/src/commonMain/kotlin/com/mifos/core/domain/di/UseCaseModule.kt`:
- Around line 41-42: The two use case class names GetChargeTemplateUseCase and
GetChargesTemplateUseCase are too similar; pick clearer, distinct names (for
example GetClientChargeTemplateUseCase and GetLoanChargeTemplateUseCase or
similar domain-specific variants), rename the class/file for the one you choose,
and update all references: imports in UseCaseModule (where both are wired), the
DI binding entries, any tests, call sites, and package-level imports to the new
names so the module compiles and the correct flow is wired to each use case.
In
`@core/network/src/commonMain/kotlin/com/mifos/core/network/datamanager/DataManagerCharge.kt`:
- Around line 103-127: The parameter order for deleteCharge and updateCharge is
inverted compared to other methods; change their signatures to declare
(resourceType: String, resourceId: Int, ...) to match createCharges and the rest
of the class, update any internal references in DataManagerCharge (methods
deleteCharge and updateCharge) to use the new parameter order, and adjust any
external call sites that pass positional arguments to these functions to the new
ordering to maintain readability and consistency.
In
`@feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanChargeForm/LoanChargeFormSheet.kt`:
- Around line 350-358: sampleChargeList is currently public by default but only
used by the private LoanChargeFormStateProvider for previews; change its
visibility to restrict leakage by adding a visibility modifier (e.g., make
sampleChargeList private or internal) so it is not exposed outside the
file/module while keeping LoanChargeFormStateProvider able to access it.
sahilshivekar
force-pushed
the
feat/MIFOSAC-650-Loan-Charge
branch
from
February 22, 2026 11:59
fd7e5ca to
62f2a5c
Compare
There was a problem hiding this comment.
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/composeResources/values/strings.xml (1)
255-255:⚠️ Potential issue | 🟡 MinorPre-existing malformed XML on this line.
There's a stray
">after the closing</string>tag. While not introduced by this PR, it will cause XML parsing errors if not already handled by the build system.<string name="principle_paid_off">Principle Paid Off</string>">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/loan/src/commonMain/composeResources/values/strings.xml` at line 255, The XML entry for the string resource named "principle_paid_off" is malformed due to an extra trailing '">'; locate the <string name="principle_paid_off">Principle Paid Off</string> element and remove the stray characters so the tag is well-formed (i.e., end with </string> only), then validate the XML to ensure no other stray characters remain.
🧹 Nitpick comments (3)
feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanChargeForm/LoanChargeFormSheet.kt (2)
91-108: UseUnitas theLaunchedEffectkey for a stable event flow.
viewModel.eventFlowis a stable reference (doesn't change across recompositions). Using it as the key is functionally fine but misleading — it implies the effect should restart when the flow reference changes.Unitis the idiomatic key for "launch once and collect for the lifetime of the composition."Suggested change
- LaunchedEffect(viewModel.eventFlow) { + LaunchedEffect(Unit) { viewModel.eventFlow.collect { event ->🤖 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/loanChargeForm/LoanChargeFormSheet.kt` around lines 91 - 108, The LaunchedEffect is keyed on viewModel.eventFlow which is a stable reference and misleading; change the key to Unit so the effect runs once for the composition lifetime: update the LaunchedEffect(viewModel.eventFlow) { ... } invocation in LoanChargeFormSheet to LaunchedEffect(Unit) { viewModel.eventFlow.collect { ... } } so it no longer suggests restart semantics tied to the flow reference while still collecting events from viewModel.eventFlow (referencing LaunchedEffect and viewModel.eventFlow and handling events like LoanChargeFormEvent.ChargeCreationSuccess/ChargeCreationFailed/FailedToLoadChargeTypes).
350-358:sampleChargeListis unintentionally public.This preview/sample data is declared at package-level with no visibility modifier, making it public API. Mark it
internalorprivateto avoid leaking preview data into the module's public surface.Suggested change
-val sampleChargeList = List(10) { +internal val sampleChargeList = List(10) {🤖 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/loanChargeForm/LoanChargeFormSheet.kt` around lines 350 - 358, The sampleChargeList variable is declared at package-level and is currently public; change its visibility to internal or private to avoid exposing preview data—update the declaration of sampleChargeList in LoanChargeFormSheet.kt (where ChargeTypes is used) to use an explicit visibility modifier (e.g., internal val sampleChargeList = ... or private val sampleChargeList = ...), and if it’s referenced from other files adjust those references or move the variable into the appropriate scope or companion object to keep it non-public.feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanChargeForm/LoanChargeSheetViewModel.kt (1)
131-137: Empty string is treated as an amount error.
"".toDoubleOrNull()returnsnull, so clearing the field immediately setsamountError = true. Since the submit button is independently disabled whenamount.isNotEmpty()is false (Line 307 of LoanChargeFormSheet.kt), this doesn't cause functional harm, but it means the error icon/message appears as soon as the user clears the field rather than only for genuinely invalid input like "abc".Consider distinguishing empty from invalid:
Suggested change
private fun updateAmount(amount: String) { mutableStateFlow.update { it.copy( amount = amount, - amountError = amount.toDoubleOrNull() == null, + amountError = amount.isNotEmpty() && amount.toDoubleOrNull() == null, ) } }🤖 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/loanChargeForm/LoanChargeSheetViewModel.kt` around lines 131 - 137, The updateAmount function sets amountError when amount.toDoubleOrNull() == null which marks an empty string as an error; change the logic in updateAmount (which updates mutableStateFlow and amountError) to only mark an error for non-empty invalid input, e.g. set amountError = amount.isNotEmpty() && amount.toDoubleOrNull() == null (optionally trim the input first) so clearing the field does not immediately show an error.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@feature/loan/src/commonMain/composeResources/values/strings.xml`:
- Line 255: The XML entry for the string resource named "principle_paid_off" is
malformed due to an extra trailing '">'; locate the <string
name="principle_paid_off">Principle Paid Off</string> element and remove the
stray characters so the tag is well-formed (i.e., end with </string> only), then
validate the XML to ensure no other stray characters remain.
---
Duplicate comments:
In
`@feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanChargeForm/LoanChargeSheetViewModel.kt`:
- Around line 211-221: The ChargeCalculationType enum currently only defines
FLAT so fromValue will return null for Fineract's percentage-based types and
those charges are dropped; add enum entries for all expected calculation types
(e.g., PERCENT_OF_AMOUNT, PERCENT_OF_INTEREST,
PERCENT_OF_LOAN_AMOUNT_PLUS_INTEREST, PERCENT_OF_DISBURSEMENT_AMOUNT) with the
exact value strings returned by the API and appropriate labelRes constants, and
keep using the companion fromValue (entries.find { it.value.equals(value, true)
}) to map API values; also update ChargeTimeType to include the missing
OverdueInstallment (chargeTimeType.overdueInstallment) entry so
overdue-installment charges are recognized.
---
Nitpick comments:
In
`@feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanChargeForm/LoanChargeFormSheet.kt`:
- Around line 91-108: The LaunchedEffect is keyed on viewModel.eventFlow which
is a stable reference and misleading; change the key to Unit so the effect runs
once for the composition lifetime: update the
LaunchedEffect(viewModel.eventFlow) { ... } invocation in LoanChargeFormSheet to
LaunchedEffect(Unit) { viewModel.eventFlow.collect { ... } } so it no longer
suggests restart semantics tied to the flow reference while still collecting
events from viewModel.eventFlow (referencing LaunchedEffect and
viewModel.eventFlow and handling events like
LoanChargeFormEvent.ChargeCreationSuccess/ChargeCreationFailed/FailedToLoadChargeTypes).
- Around line 350-358: The sampleChargeList variable is declared at
package-level and is currently public; change its visibility to internal or
private to avoid exposing preview data—update the declaration of
sampleChargeList in LoanChargeFormSheet.kt (where ChargeTypes is used) to use an
explicit visibility modifier (e.g., internal val sampleChargeList = ... or
private val sampleChargeList = ...), and if it’s referenced from other files
adjust those references or move the variable into the appropriate scope or
companion object to keep it non-public.
In
`@feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanChargeForm/LoanChargeSheetViewModel.kt`:
- Around line 131-137: The updateAmount function sets amountError when
amount.toDoubleOrNull() == null which marks an empty string as an error; change
the logic in updateAmount (which updates mutableStateFlow and amountError) to
only mark an error for non-empty invalid input, e.g. set amountError =
amount.isNotEmpty() && amount.toDoubleOrNull() == null (optionally trim the
input first) so clearing the field does not immediately show an error.
therajanmaurya
enabled auto-merge (rebase)
February 23, 2026 10:26
therajanmaurya
disabled auto-merge
February 23, 2026 10:26
therajanmaurya
enabled auto-merge (rebase)
February 23, 2026 10:26
therajanmaurya
disabled auto-merge
February 23, 2026 10:26
therajanmaurya
enabled auto-merge (squash)
February 23, 2026 10:27
auto-merge was automatically disabled
February 23, 2026 10:43
Head branch was pushed to by a user without write access
There was a problem hiding this comment.
🧹 Nitpick comments (1)
core/domain/src/commonMain/kotlin/com/mifos/core/domain/di/UseCaseModule.kt (1)
42-43: Near-identical namesGetChargeTemplateUseCase/GetChargesTemplateUseCaserisk confusion.The two use cases differ only by a single
s. Consider distinguishing them more clearly — e.g.GetLoanChargeTemplateUseCasefor the new one — to avoid accidental misuse at call sites.Also applies to: 122-123
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/domain/src/commonMain/kotlin/com/mifos/core/domain/di/UseCaseModule.kt` around lines 42 - 43, The two almost-identical use case names GetChargeTemplateUseCase and GetChargesTemplateUseCase are confusing; rename the one that represents the new/loan-specific behavior (e.g., change GetChargesTemplateUseCase to GetLoanChargeTemplateUseCase) and update all references and DI bindings in UseCaseModule.kt (imports and provider/registration entries) as well as any classes that construct or inject it (constructor params, factory methods); ensure the old name is removed or aliased and run the build to catch remaining references.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@core/domain/src/commonMain/kotlin/com/mifos/core/domain/di/UseCaseModule.kt`:
- Around line 42-43: The two almost-identical use case names
GetChargeTemplateUseCase and GetChargesTemplateUseCase are confusing; rename the
one that represents the new/loan-specific behavior (e.g., change
GetChargesTemplateUseCase to GetLoanChargeTemplateUseCase) and update all
references and DI bindings in UseCaseModule.kt (imports and
provider/registration entries) as well as any classes that construct or inject
it (constructor params, factory methods); ensure the old name is removed or
aliased and run the build to catch remaining references.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
core/domain/src/commonMain/kotlin/com/mifos/core/domain/di/UseCaseModule.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanChargeDialog/LoanChargeDialogScreen.kt
💤 Files with no reviewable changes (1)
- feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanChargeDialog/LoanChargeDialogScreen.kt
itsPronay
reviewed
Feb 23, 2026
sahilshivekar
force-pushed
the
feat/MIFOSAC-650-Loan-Charge
branch
2 times, most recently
from
February 23, 2026 16:16
54b4d7f to
3423e39
Compare
itsPronay
approved these changes
Feb 23, 2026
sahilshivekar
force-pushed
the
feat/MIFOSAC-650-Loan-Charge
branch
from
February 23, 2026 16:34
3423e39 to
38d8317
Compare
sahilshivekar
force-pushed
the
feat/MIFOSAC-650-Loan-Charge
branch
from
February 23, 2026 18:35
38d8317 to
a42470e
Compare
|
biplab1
approved these changes
Feb 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes - Jira-MIFOSAC-650
mifosac-650-old.mp4
mifosac-650-bottom-sheet.mp4
Summary by CodeRabbit
New Features
Improvements