feat(savingsAccount): preview functionality #2521
Conversation
…eating a new savings account
…vings application, navigation to details screen.
…vings application, navigation to details screen.
| verticalAlignment = Alignment.CenterVertically, | ||
| ) { | ||
| Row( | ||
| modifier = Modifier.fillMaxWidth(), |
There was a problem hiding this comment.
Drop the nested Row it’s redundant.
| navController = navController, | ||
| onNavigateBack = onBackPressed, | ||
| onFinish = onBackPressed, | ||
| onFinish = onFinish, |
There was a problem hiding this comment.
what does onFinish achieve here? if it is the same as onBackPressed then use onBackPressed
There was a problem hiding this comment.
onFinish is not same as onBackPressed.
onFinish = navController::navigateToClientProfileGeneralRouteOnStatus,
where
fun NavController.navigateToClientProfileGeneralRouteOnStatus(id: Int) {
this.navigate(
ClientProfileGeneralRoute(id = id),
) {
popBackStack(0, true)
navigate(ClientListScreenRoute)
navigate(ClientProfileRoute(id))
popUpTo(ClientProfileGeneralRoute(id = id)) { inclusive = true }
launchSingleTop = true
}
}
There was a problem hiding this comment.
Basically we need to navigate to client profile general route after applying for the savings account. That is what onFinish Achieves
| ) | ||
|
|
||
| Column(modifier = Modifier.fillMaxSize()) { | ||
| LazyColumn(modifier = modifier.weight(1f)) { |
There was a problem hiding this comment.
Avoid using LazyColumn iniside Column.
There was a problem hiding this comment.
According to CodeRabbit:
128-193: The LazyColumn inside Column pattern is acceptable here.
While generally LazyColumn inside Column should be avoided, this implementation correctly bounds the LazyColumn's height with weight(1f), allowing proper scrolling while keeping the button row fixed at the bottom. This is an acceptable pattern for this use case.
| enforceMinRequiredBalance = state.isCheckedMinimumBalance | ||
| minRequiredOpeningBalance = state.minimumOpeningBalance | ||
| minRequiredBalance = state.monthlyMinimumBalance | ||
| lockinPeriodFrequency = state.frequency.let { if (it.isNotBlank()) it.toInt() else null } |
There was a problem hiding this comment.
use state.frequency.toIntOrNull() instead of creating long if else statement
| savingsPayload.apply { | ||
| locale = "en" | ||
| dateFormat = "dd-MM-yyyy" | ||
| productId = state.savingsProductSelected + 1 |
There was a problem hiding this comment.
why id's are created like this added by 1?
There was a problem hiding this comment.
Because the productId 0 does not exist and the ids start from 1. However in the app, they are 0 indexed.
| interestCompoundingPeriodType = | ||
| state.savingsProductTemplate?.interestCompoundingPeriodTypeOptions?.get(state.interestCompPeriodIndex)?.id | ||
| interestCalculationType = | ||
| state.savingsProductTemplate?.interestCompoundingPeriodTypeOptions?.get(state.interestCalcIndex)?.id |
There was a problem hiding this comment.
this one also reading from interestCompoundingPeriodTypeOptions but shouldn't it read from itself sth like interestCalculationTypeOptions?
| interestCalculationDaysInYearType = | ||
| state.savingsProductTemplate?.interestCalculationDaysInYearTypeOptions?.get(state.daysInYearIndex)?.id | ||
| interestPostingPeriodType = | ||
| state.savingsProductTemplate?.interestCompoundingPeriodTypeOptions?.get(state.interestPostingPeriodIndex)?.id |
There was a problem hiding this comment.
same here shouldn't read from itself sth like state.savingsProductTemplate?.interestPostingPeriodTypeOptions or i am missing smething?
| stringResource(Res.string.feature_savings_product_name) + " : " to state.savingProductOptions[state.savingsProductSelected].name, | ||
| stringResource(Res.string.feature_savings_field_officer) + " : " to state.fieldOfficerOptions[state.fieldOfficerIndex].displayName, | ||
| stringResource(Res.string.feature_savings_submission_date) + " : " to state.submissionDate, | ||
| stringResource(Res.string.feature_savings_external_id) + " : " to state.externalId, |
There was a problem hiding this comment.
Don’t hardcode : in strings put formatting in your UI component (or use string resources with placeholders), not in the keys. If MifosGeneralCard requires the colon, let it add it; if not possible today, at least keep labels clean and add the colon once when rendering.
| )?.value ?: "" | ||
| ) | ||
| ) | ||
| }, |
There was a problem hiding this comment.
these types of giving names is long and confusing checking if else inside it come when cleaner qpproach see if sth like this is possible:
lockInType = state.savingsProductTemplate?.lockinPeriodFrequencyTypeOptions
?.getOrNull(state.freqTypeIndex)?.value.orEmpty()
| is DataState.Success -> { | ||
| // Not used in New UI implementation, however was causing build issue so commented for now, can be updated in further code refactoring. | ||
| // _savingAccountUiState.value = | ||
| // SavingAccountUiState.ShowSavingsAccountCreatedSuccessfully( | ||
| // dataState.data, | ||
| // ) | ||
| } |
There was a problem hiding this comment.
This piece of code even though commented out for future refactoring shouldn't amount to any build issue. Please find an alternative to the current fix.
| @@ -1 +1 @@ | |||
| 2025.9.2-beta.0.5 No newline at end of file | |||
| 2025.10.4-beta.0.6 No newline at end of file | |||
There was a problem hiding this comment.
Should this be a part of this PR?
| state.savingsProductTemplate?.interestCompoundingPeriodTypeOptions?.get(state.interestCompPeriodIndex)?.id | ||
| interestCalculationType = | ||
| state.savingsProductTemplate?.interestCompoundingPeriodTypeOptions?.get(state.interestCalcIndex)?.id |
There was a problem hiding this comment.
Using .get(index) directly risks crashes for invalid indices.
Suggested fix: Replace with getOrNull(index)
| interestCalculationDaysInYearType = | ||
| state.savingsProductTemplate?.interestCalculationDaysInYearTypeOptions?.get(state.daysInYearIndex)?.id | ||
| interestPostingPeriodType = | ||
| state.savingsProductTemplate?.interestCompoundingPeriodTypeOptions?.get(state.interestPostingPeriodIndex)?.id |
| savingsPayload.apply { | ||
| locale = "en" | ||
| dateFormat = "dd-MM-yyyy" | ||
| productId = state.savingsProductSelected + 1 |
There was a problem hiding this comment.
Try this:
productId = state.savingProductOptions.getOrNull(state.savingsProductSelected)?.id
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
WalkthroughThis PR adds a complete savings submission flow: model payload extensions, network response handling, use-case return-type tightening, new UI components and screens, submission/status dialog handling, and navigation updates wiring savings completion to client profile routes. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant PreviewPage
participant SavingsScreen
participant SavingsVM
participant CreateUseCase
participant NetworkLayer
User->>PreviewPage: Click Submit
PreviewPage->>SavingsScreen: onAction(SubmitSavingsApplication)
SavingsScreen->>SavingsVM: onAction(SubmitSavingsApplication)
SavingsVM->>SavingsVM: createSavingsPayload()
SavingsVM->>CreateUseCase: invoke(payload)
CreateUseCase->>NetworkLayer: POST /savings (HttpResponse)
alt HTTP Success
NetworkLayer-->>CreateUseCase: HttpResponse (200) + body
CreateUseCase-->>SavingsVM: DataState.Success(Savings)
SavingsVM->>SavingsScreen: ScreenState.ShowStatusDialog(SUCCESS)
User->>SavingsScreen: Confirm -> Finish
SavingsScreen->>SavingsVM: onAction(Finish)
SavingsVM-->>SavingsScreen: onFinish(clientId)
else HTTP Error
NetworkLayer-->>CreateUseCase: HttpResponse (error) + body
CreateUseCase-->>SavingsVM: DataState.Error(MifosError/message)
SavingsVM->>SavingsScreen: ScreenState.ShowStatusDialog(FAILED)
User->>SavingsScreen: Retry/Cancel
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
core/domain/src/commonMain/kotlin/com/mifos/core/domain/useCases/CreateSavingsAccountUseCase.kt (1)
15-27: Consider returning domain objects instead of HttpResponse from the domain layer.Returning
HttpResponsefrom the domain layer couples the domain to HTTP protocol details and violates clean architecture principles. The domain layer should expose domain entities (e.g., aSavingsAccountResultor similar) while HTTP concerns remain in data/network layers.Consider creating a domain model to represent the result and mapping HttpResponse to that model in the repository layer.
♻️ Duplicate comments (8)
core/ui/src/commonMain/kotlin/com/mifos/core/ui/components/MifosGeneralCard.kt (1)
61-80: Remove the redundant outer Row.The outer
RowwithSpaceBetweenarrangement is unnecessary since the innerRowalready fills the width and uses weightedBoxcomponents to distribute content. This creates unnecessary nesting without adding value.Apply this diff to simplify the structure:
@Composable fun MifosGeneralRowItem( keyContent: @Composable () -> Unit, valueContent: @Composable () -> Unit, ) { Row( modifier = Modifier.fillMaxWidth(), - horizontalArrangement = Arrangement.SpaceBetween, verticalAlignment = Alignment.CenterVertically, ) { - Row( - modifier = Modifier.fillMaxWidth(), - verticalAlignment = Alignment.CenterVertically, - ) { - Box( - modifier = Modifier.weight(1f), - contentAlignment = Alignment.CenterStart, - ) { keyContent() } - - Box( - modifier = Modifier.weight(1f), - contentAlignment = Alignment.CenterEnd, - ) { valueContent() } - } + Box( + modifier = Modifier.weight(1f), + contentAlignment = Alignment.CenterStart, + ) { keyContent() } + + Box( + modifier = Modifier.weight(1f), + contentAlignment = Alignment.CenterEnd, + ) { valueContent() } } }version.txt (1)
1-1: Version bump in a feature PR — confirm scope.Should the version change ride this PR, or be handled in a release bump PR after merge? Align with your release process.
feature/savings/src/commonMain/kotlin/com/mifos/feature/savings/savingsAccountv2/pages/PreviewPage.kt (2)
61-126: Remove hardcoded colons from string concatenation.Multiple lines hardcode " : " when building the preview maps (e.g., lines 62-65, 69, 74, 75, etc.). This approach has localization and maintainability issues. Consider:
- Moving the colon to the UI component (MifosGeneralCard) if it doesn't already add it
- Using string resources with placeholders instead of concatenation
- Keeping label strings clean without formatting characters
111-122: Simplify the lock-in period expression.The conditional logic for the lock-in period is complex and could be more readable using safe navigation.
Consider this approach:
- stringResource(Res.string.step_terms_lock_in_period) + " : " to - if (state.freqTypeIndex == -1) { - "" - } else { - ( - state.frequency + " " + ( - state.savingsProductTemplate?.lockinPeriodFrequencyTypeOptions?.get( - state.freqTypeIndex, - )?.value ?: "" - ) - ) - }, + stringResource(Res.string.step_terms_lock_in_period) + " : " to + state.savingsProductTemplate?.lockinPeriodFrequencyTypeOptions + ?.getOrNull(state.freqTypeIndex)?.value + ?.let { "${state.frequency} $it" }.orEmpty(),feature/savings/src/commonMain/kotlin/com/mifos/feature/savings/savingsAccountv2/SavingsAccountViewModel.kt (4)
134-134: Use the actual product ID instead of index + 1.While the author noted that IDs start from 1, hardcoding
index + 1is fragile and assumes sequential IDs. Retrieve the actual ID from the product object.Apply this fix:
- productId = state.savingsProductSelected + 1 + productId = state.savingProductOptions.getOrNull(state.savingsProductSelected)?.id
143-143: Replace toInt() with toIntOrNull() to avoid exceptions.Using
toInt()can throwNumberFormatExceptionif the string is invalid. UsetoIntOrNull()for safe parsing.Apply this fix:
- lockinPeriodFrequency = state.frequency.let { if (it.isNotBlank()) it.toInt() else null } + lockinPeriodFrequency = state.frequency.toIntOrNull()
152-154: Wrong options collection used for interestCalculationType.Line 154 uses
interestCompoundingPeriodTypeOptionsinstead ofinterestCalculationTypeOptions. This will assign the wrong ID.Apply this fix:
interestCompoundingPeriodType = - state.savingsProductTemplate?.interestCompoundingPeriodTypeOptions?.get(state.interestCompPeriodIndex)?.id + state.savingsProductTemplate?.interestCompoundingPeriodTypeOptions?.getOrNull(state.interestCompPeriodIndex)?.id interestCalculationType = - state.savingsProductTemplate?.interestCompoundingPeriodTypeOptions?.get(state.interestCalcIndex)?.id + state.savingsProductTemplate?.interestCalculationTypeOptions?.getOrNull(state.interestCalcIndex)?.id
155-158: Wrong options collection used for interestPostingPeriodType.Line 158 uses
interestCompoundingPeriodTypeOptionsinstead ofinterestPostingPeriodTypeOptions. This will assign the wrong ID. Additionally, usegetOrNull()to avoid crashes.Apply this fix:
interestCalculationDaysInYearType = - state.savingsProductTemplate?.interestCalculationDaysInYearTypeOptions?.get(state.daysInYearIndex)?.id + state.savingsProductTemplate?.interestCalculationDaysInYearTypeOptions?.getOrNull(state.daysInYearIndex)?.id interestPostingPeriodType = - state.savingsProductTemplate?.interestCompoundingPeriodTypeOptions?.get(state.interestPostingPeriodIndex)?.id + state.savingsProductTemplate?.interestPostingPeriodTypeOptions?.getOrNull(state.interestPostingPeriodIndex)?.id
🧹 Nitpick comments (5)
core/ui/src/commonMain/kotlin/com/mifos/core/ui/components/MifosGeneralCard.kt (2)
49-49: Consider using a more semantic border color.
secondaryContaineris typically used for filled container backgrounds rather than borders. Consider usingMaterialTheme.colorScheme.outlineoroutlineVariantfor better semantic alignment with Material Design guidelines.
92-98: Remove trailing spacer to avoid extra bottom padding.The
forEachloop adds aSpacerafter each item, including the last one. This creates unwanted extra padding at the bottom of the card.Apply this diff to fix the trailing spacer:
contentMap.forEach { map -> MifosGeneralRowItem( keyContent = { Text(text = map.key, style = MifosTypography.labelMediumEmphasized) }, valueContent = { Text(text = map.value, style = MifosTypography.labelMediumEmphasized) }, ) - Spacer(modifier = Modifier.height(DesignToken.padding.small)) } + + contentMap.entries.forEachIndexed { index, map -> + MifosGeneralRowItem( + keyContent = { Text(text = map.key, style = MifosTypography.labelMediumEmphasized) }, + valueContent = { Text(text = map.value, style = MifosTypography.labelMediumEmphasized) }, + ) + if (index < contentMap.size - 1) { + Spacer(modifier = Modifier.height(DesignToken.padding.small)) + } + }feature/savings/src/commonMain/kotlin/com/mifos/feature/savings/navigation/SavingsNavigation.kt (1)
28-34: Document onFinish contract.Add brief KDoc: “onFinish receives clientId to navigate to ClientProfileGeneral.” This prevents passing savingsAccountId by mistake.
core/data/src/commonMain/kotlin/com/mifos/core/data/repository/SavingsAccountRepository.kt (1)
38-39: Repository contract should be domain-centric, not transport-centric.Consider returning Flow<DataState> (with resourceId/status) instead of HttpResponse. This keeps Ktor out of domain/UI and simplifies testing.
feature/savings/src/commonMain/kotlin/com/mifos/feature/savings/savingsAccountv2/SavingsAccountRoute.kt (1)
22-34: Rename id parameter to clientId for clarity; verify route guards against invalid -1 values.The id parameter is confirmed to be clientId (passed at SavingsAccountScreen.kt:86), not savingsAccountId. However, the generic name obscures this. Rename the parameter to
clientIdthroughout the signature chain.Additionally, SavingsAccountRoute defaults clientId to -1. Verify that invalid clientId=-1 values cannot reach onFinish, or add defensive checks in callers.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
core/data/src/commonMain/kotlin/com/mifos/core/data/repository/SavingsAccountRepository.kt(2 hunks)core/data/src/commonMain/kotlin/com/mifos/core/data/repositoryImp/SavingsAccountRepositoryImp.kt(2 hunks)core/domain/src/commonMain/kotlin/com/mifos/core/domain/useCases/CreateSavingsAccountUseCase.kt(2 hunks)core/model/src/commonMain/kotlin/com/mifos/core/model/objects/payloads/SavingsPayload.kt(1 hunks)core/network/src/commonMain/kotlin/com/mifos/core/network/datamanager/DataManagerSavings.kt(2 hunks)core/network/src/commonMain/kotlin/com/mifos/core/network/services/SavingsAccountService.kt(2 hunks)core/ui/src/commonMain/kotlin/com/mifos/core/ui/components/MifosGeneralCard.kt(1 hunks)feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientGeneral/ClientProfileGeneralNavigation.kt(2 hunks)feature/client/src/commonMain/kotlin/com/mifos/feature/client/navigation/ClientNavigation.kt(2 hunks)feature/savings/src/commonMain/composeResources/values/feature_savings_strings.xml(1 hunks)feature/savings/src/commonMain/kotlin/com/mifos/feature/savings/navigation/SavingsNavigation.kt(2 hunks)feature/savings/src/commonMain/kotlin/com/mifos/feature/savings/savingsAccount/SavingAccountViewModel.kt(1 hunks)feature/savings/src/commonMain/kotlin/com/mifos/feature/savings/savingsAccountv2/SavingsAccountRoute.kt(1 hunks)feature/savings/src/commonMain/kotlin/com/mifos/feature/savings/savingsAccountv2/SavingsAccountScreen.kt(6 hunks)feature/savings/src/commonMain/kotlin/com/mifos/feature/savings/savingsAccountv2/SavingsAccountViewModel.kt(7 hunks)feature/savings/src/commonMain/kotlin/com/mifos/feature/savings/savingsAccountv2/pages/PreviewPage.kt(1 hunks)version.txt(1 hunks)
🔇 Additional comments (18)
core/ui/src/commonMain/kotlin/com/mifos/core/ui/components/MifosGeneralCard.kt (1)
103-117: LGTM!The preview composable provides good sample data for design-time visualization and is correctly structured.
core/network/src/commonMain/kotlin/com/mifos/core/network/services/SavingsAccountService.kt (2)
27-27: Transport type import noted.Acknowledged shift to HttpResponse.
108-110: Return a typed response (GenericResponse) instead of HttpResponse for consistency and schema safety.The codebase establishes a clear pattern where similar create/action operations use
GenericResponse. Other savings operations (e.g.,ShowSavingAccountApprovedSuccessfully,ShowSavingAccountActivatedSuccessfully) wrapGenericResponsein UI states. Similarly, note creation and loan operations returnGenericResponsedirectly. The currentFlow<HttpResponse>return type is inconsistent and exposes untyped HTTP details to downstream consumers who expect a schema-aware response with fields likeresourceId.Change the return type from
Flow<HttpResponse>toFlow<GenericResponse>in:
SavingsAccountService.createSavingsAccount()- Update
DataManagerSavings.createSavingsAccount()accordingly- Update repository and use case signatures to propagate the typed response
feature/savings/src/commonMain/kotlin/com/mifos/feature/savings/navigation/SavingsNavigation.kt (1)
86-90: Forwarding is correct; no changes needed.The code at lines 86-90 properly forwards
onFinish: (id: Int) -> Unitfrom thesavingsDestinationfunction parameter tosavingsAccountDestination, which expects the same signature. Type matching is maintained across all call sites.feature/client/src/commonMain/kotlin/com/mifos/feature/client/navigation/ClientNavigation.kt (1)
338-344: LGTM!The navigation wiring correctly connects the savings flow completion to the client profile general route via the
onFinishcallback.core/model/src/commonMain/kotlin/com/mifos/core/model/objects/payloads/SavingsPayload.kt (1)
35-39: LGTM!The new fields appropriately extend the payload schema to support the preview functionality. The nullable types provide backward compatibility.
feature/savings/src/commonMain/kotlin/com/mifos/feature/savings/savingsAccountv2/SavingsAccountScreen.kt (4)
77-77: LGTM!The signature change enables the caller to receive the client ID for proper navigation flow.
83-88: LGTM!Correctly passes the client ID when the finish event is triggered, enabling proper navigation.
130-134: LGTM!The PreviewPage invocation now follows the same state-driven pattern as the other pages, ensuring consistency.
170-181: LGTM!The status dialog implementation correctly handles success and failure states with appropriate actions and messaging.
feature/savings/src/commonMain/composeResources/values/feature_savings_strings.xml (1)
142-149: LGTM!The new string resources appropriately support the status dialog feature with clear, user-friendly messages.
feature/savings/src/commonMain/kotlin/com/mifos/feature/savings/savingsAccountv2/pages/PreviewPage.kt (2)
56-60: LGTM!The new signature follows the state-driven architecture pattern consistently used across the savings account pages.
128-193: The LazyColumn inside Column pattern is acceptable here.While generally LazyColumn inside Column should be avoided, this implementation correctly bounds the LazyColumn's height with
weight(1f), allowing proper scrolling while keeping the button row fixed at the bottom. This is an acceptable pattern for this use case.feature/savings/src/commonMain/kotlin/com/mifos/feature/savings/savingsAccountv2/SavingsAccountViewModel.kt (5)
43-49: LGTM!The addition of
createSavingsAccountUseCaseto the constructor enables the submission functionality.
66-73: LGTM!The Finish action handler correctly resets the screen state and triggers navigation.
163-219: Submission flow implementation is correct.The method properly handles loading, success, error, and offline states with appropriate status dialogs. The use of
extractErrorMessageto determine success vs. error response is working correctly.
661-661: LGTM!The
ShowStatusDialogscreen state appropriately supports the status dialog feature with status and message parameters.
685-685: LGTM!The
SubmitSavingsApplicationaction appropriately triggers the submission workflow.
.../data/src/commonMain/kotlin/com/mifos/core/data/repositoryImp/SavingsAccountRepositoryImp.kt
Outdated
Show resolved
Hide resolved
core/network/src/commonMain/kotlin/com/mifos/core/network/datamanager/DataManagerSavings.kt
Outdated
Show resolved
Hide resolved
...c/commonMain/kotlin/com/mifos/feature/client/clientGeneral/ClientProfileGeneralNavigation.kt
Show resolved
Hide resolved
| is DataState.Success -> { | ||
| // Not used in New UI implementation, however was causing build issue so commented for now, can be updated in further code refactoring. | ||
| // _savingAccountUiState.value = | ||
| // SavingAccountUiState.ShowSavingsAccountCreatedSuccessfully( | ||
| // dataState.data, | ||
| // ) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Critical: Silent success handling breaks user feedback and ignores past review.
The success case is now empty, providing no feedback to the user after account creation. This is a critical UX issue - users will have no confirmation that their savings account was created successfully.
Additionally, the past review comment by biplab1 on these exact lines specifically requested "an alternative to the current fix" rather than commenting out the code, but this concern remains unaddressed.
Issues:
- No UI state update on success
HttpResponsedata (likely containing the created account ID) is not extracted or used- Violates the principle of providing user feedback for significant actions
Required action:
Extract relevant data from dataState.data (the HttpResponse) and update the UI state appropriately. If the new UI flow uses a different mechanism (e.g., ShowStatusDialog as suggested in the AI summary), the ViewModel should still:
- Extract the account ID from the response
- Update state to reflect successful creation
- Provide the necessary data for the UI to display confirmation
Please verify how success is handled in the new UI flow. Run the following script to understand the success handling mechanism:
🏁 Script executed:
#!/bin/bash
# Description: Find how success state is handled in the new UI and what data is needed from HttpResponse
# Search for ShowStatusDialog usage in savings account screens
echo "=== ShowStatusDialog usage ==="
rg -n "ShowStatusDialog" --type=kotlin -g "*savings*" -C3
# Search for SavingAccountUiState sealed class definition
echo -e "\n=== SavingAccountUiState definitions ==="
ast-grep --pattern 'sealed class SavingAccountUiState {
$$$
}'
# Search for how createSavingsAccount is called in UI
echo -e "\n=== createSavingsAccount invocations ==="
rg -n "createSavingsAccount" --type=kotlin -g "*SavingsAccountScreen*" -C5
# Check HttpResponse structure to see what data is available
echo -e "\n=== HttpResponse class definition ==="
fd -e kt -x grep -l "class HttpResponse" | xargs catLength of output: 10250
🏁 Script executed:
#!/bin/bash
# Description: Verify how success state is handled and observe uiState in UI
# Search for uiState collection in SavingsAccountScreen
echo "=== uiState observation in SavingsAccountScreen ==="
rg -n "uiState" feature/savings/src/commonMain/kotlin/com/mifos/feature/savings/savingsAccount/SavingsAccountScreen.kt -C5
# Check the full ViewModel to see createSavingsAccount method signature and return type
echo -e "\n=== createSavingsAccount method in ViewModel ==="
rg -n "fun createSavingsAccount" feature/savings/src/commonMain/kotlin/com/mifos/feature/savings/savingsAccount/SavingAccountViewModel.kt -A15
# Check if ShowSavingsAccountCreatedSuccessfully is handled anywhere in UI
echo -e "\n=== ShowSavingsAccountCreatedSuccessfully handling ==="
rg -n "ShowSavingsAccountCreatedSuccessfully" --type=kotlin -g "*savings*" -C3
# Search for where uiState is being collected as state flow
echo -e "\n=== _savingAccountUiState and uiState in ViewModel ==="
rg -n "savingAccountUiState\|_savingAccountUiState" feature/savings/src/commonMain/kotlin/com/mifos/feature/savings/savingsAccount/SavingAccountViewModel.kt -C2Length of output: 4423
🏁 Script executed:
#!/bin/bash
# Description: Verify how success state is handled and observe uiState in UI
# Search for uiState collection in SavingsAccountScreen
echo "=== uiState observation in SavingsAccountScreen ==="
rg -n "uiState" feature/savings/src/commonMain/kotlin/com/mifos/feature/savings/savingsAccount/SavingsAccountScreen.kt -C5
# Check the full ViewModel to see createSavingsAccount method signature and return type
echo -e "\n=== createSavingsAccount method in ViewModel ==="
rg -n "fun createSavingsAccount" feature/savings/src/commonMain/kotlin/com/mifos/feature/savings/savingsAccount/SavingAccountViewModel.kt -A15
# Check if ShowSavingsAccountCreatedSuccessfully is handled anywhere in UI
echo -e "\n=== ShowSavingsAccountCreatedSuccessfully handling ==="
rg -n "ShowSavingsAccountCreatedSuccessfully" --type=kotlin -g "*savings*" -C3
# Search for where uiState is being collected as state flow
echo -e "\n=== _savingAccountUiState and uiState in ViewModel ==="
rg -n "savingAccountUiState\|_savingAccountUiState" feature/savings/src/commonMain/kotlin/com/mifos/feature/savings/savingsAccount/SavingAccountViewModel.kt -C2Length of output: 4423
Critical: Success state not handled in UI - causes silent success with no user feedback.
The ShowSavingsAccountCreatedSuccessfully state is defined in SavingAccountUiState but is completely missing from the UI's when statement (lines 173-209 in SavingsAccountScreen.kt). Additionally, the ViewModel's success case (lines 131-137 in SavingAccountViewModel.kt) remains empty with commented-out code.
This creates a critical issue where:
- Success responses are ignored by both ViewModel and UI
- Users receive no confirmation that their account was created
- The sealed class
whenstatement is non-exhaustive (missing the success case) - The past review feedback from biplab1 requesting an alternative to commenting out code remains unaddressed
Required fixes:
- Implement success handling in ViewModel: extract account data from the response and emit
ShowSavingsAccountCreatedSuccessfullystate - Add
is SavingAccountUiState.ShowSavingsAccountCreatedSuccessfullycase to the UI's when statement - Provide user feedback (e.g., success message, navigation back, or status dialog)
🤖 Prompt for AI Agents
In
feature/savings/src/commonMain/kotlin/com/mifos/feature/savings/savingsAccount/SavingAccountViewModel.kt
around lines 131-137, the DataState.Success branch is commented out leaving no
success emission; restore and implement it to extract the created account data
from dataState.data and set _savingAccountUiState.value =
SavingAccountUiState.ShowSavingsAccountCreatedSuccessfully(dataState.data) (or
map to the expected DTO) instead of commenting; and in
feature/savings/src/commonMain/kotlin/com/mifos/feature/savings/savingsAccount/SavingsAccountScreen.kt
around lines 173-209 add a new when branch for is
SavingAccountUiState.ShowSavingsAccountCreatedSuccessfully that surfaces user
feedback (e.g., show a success Snackbar/Toast or navigate back with a result) so
the UI no longer silently ignores successful creations and the sealed when
becomes exhaustive.
… string resources for clarity
…gs object, enhance error handling
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
core/network/src/commonMain/kotlin/com/mifos/core/network/datamanager/DataManagerSavings.kt (1)
292-317: Critical: Response body read twice and HttpResponse never closed (leak + potential crash).
- bodyAsText() is invoked in extractErrorMessage (Line 294) and again on success (Line 314). Ktor responses are single-consume; the second read can return empty or fail.
- HttpResponse isn’t closed, risking connection pool exhaustion.
- Also confirm the success DTO: Fineract commonly returns GenericResponse with resourceId for create endpoints; decoding to Savings may be incorrect.
Fix: read body once, branch on status, decode accordingly, and always close the response.
Apply this diff to the function body:
- fun createSavingsAccount(savingsPayload: SavingsPayload?): Flow<Savings> { - suspend fun extractErrorMessage(response: HttpResponse): String { - val responseText = response.bodyAsText() - var result = "" - try { - if (!response.status.isSuccess()) { - val json = Json { ignoreUnknownKeys = true } - val errorResponse = json.decodeFromString<MifosError>(responseText) - result = errorResponse.errors.firstOrNull()?.defaultUserMessage - ?: errorResponse.defaultUserMessage - ?: "Message Not Found" - } - } catch (e: Exception) { - result = "Failed to parse error response" - } - return result - } - return mBaseApiManager.savingsService.createSavingsAccount(savingsPayload).map { response -> - val errorMessage = extractErrorMessage(response) - if (errorMessage.isNotEmpty()) { - throw Exception(errorMessage) - } - val responseText = response.bodyAsText() - val json = Json { ignoreUnknownKeys = true } - json.decodeFromString<Savings>(responseText) - } - } + fun createSavingsAccount(savingsPayload: SavingsPayload?): Flow<Savings> { + return mBaseApiManager.savingsService.createSavingsAccount(savingsPayload).map { response -> + try { + val text = response.bodyAsText() + if (!response.status.isSuccess()) { + val json = Json { ignoreUnknownKeys = true } + val parsed = runCatching { json.decodeFromString<MifosError>(text) }.getOrNull() + val msg = parsed?.errors?.firstOrNull()?.defaultUserMessage + ?: parsed?.defaultUserMessage + ?: "HTTP ${'$'}{response.status.value} ${'$'}{response.status.description}" + throw IllegalStateException(msg) + } + val json = Json { ignoreUnknownKeys = true } + json.decodeFromString<Savings>(text) + } finally { + response.close() + } + } + }If the endpoint returns GenericResponse with a resourceId, switch the return type and success decode accordingly; see verification below. Based on learnings.
🧹 Nitpick comments (4)
feature/savings/src/commonMain/kotlin/com/mifos/feature/savings/savingsAccountv2/pages/PreviewPage.kt (1)
170-181: i18n: avoid manual string concatenation for the charges summary.Compose a single localized string with placeholders instead of "size + ' ' + label + ' ' + label". Define a resource like “%d active charges” and use it here.
Example (once the new string resource exists):
- text = state.addedCharges.size.toString() + " " + stringResource(Res.string.step_charges_active) + " " + stringResource( - Res.string.step_charges, - ), + text = stringResource(Res.string.feature_savings_charges_active_count, state.addedCharges.size),core/ui/src/commonMain/kotlin/com/mifos/core/ui/components/MifosGeneralCard.kt (2)
86-93: Improve long-value handling (truncate gracefully).When values are long, layout may overflow. Add maxLines/ellipsis to value (and optionally key).
Apply this diff:
- keyContent = { Text(text = map.key + separator, style = MifosTypography.labelMediumEmphasized) }, - valueContent = { Text(text = map.value, style = MifosTypography.labelMediumEmphasized) }, + keyContent = { + Text( + text = map.key + separator, + style = MifosTypography.labelMediumEmphasized, + maxLines = 1 + ) + }, + valueContent = { + Text( + text = map.value, + style = MifosTypography.labelMediumEmphasized, + maxLines = 1 + ) + },
80-83: Consider localizing the separator.Expose separator from strings (e.g., “: ”) for full i18n control, or accept null to omit it and let callers format.
feature/savings/src/commonMain/kotlin/com/mifos/feature/savings/savingsAccountv2/SavingsAccountViewModel.kt (1)
163-206: Submission UX polish: ensure loading flag is always reset.Loading is cleared on Success/Error but not set before collecting; set isOverLayLoadingActive = true before collect to avoid a short flash.
- createSavingsAccountUseCase(savingsPayload).collect { result -> + createSavingsAccountUseCase(savingsPayload).collect { result -> when (result) { is DataState.Loading -> { mutableStateFlow.update { it.copy( isOverLayLoadingActive = true, ) } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
core/domain/src/commonMain/kotlin/com/mifos/core/domain/useCases/CreateSavingsAccountUseCase.kt(1 hunks)core/network/src/commonMain/kotlin/com/mifos/core/network/datamanager/DataManagerSavings.kt(3 hunks)core/ui/src/commonMain/kotlin/com/mifos/core/ui/components/MifosGeneralCard.kt(1 hunks)feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientGeneral/ClientProfileGeneralNavigation.kt(2 hunks)feature/savings/src/commonMain/composeResources/values/feature_savings_strings.xml(4 hunks)feature/savings/src/commonMain/kotlin/com/mifos/feature/savings/savingsAccount/SavingAccountViewModel.kt(2 hunks)feature/savings/src/commonMain/kotlin/com/mifos/feature/savings/savingsAccountv2/SavingsAccountViewModel.kt(7 hunks)feature/savings/src/commonMain/kotlin/com/mifos/feature/savings/savingsAccountv2/pages/PreviewPage.kt(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- feature/savings/src/commonMain/kotlin/com/mifos/feature/savings/savingsAccount/SavingAccountViewModel.kt
- core/domain/src/commonMain/kotlin/com/mifos/core/domain/useCases/CreateSavingsAccountUseCase.kt
- feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientGeneral/ClientProfileGeneralNavigation.kt
🧰 Additional context used
🧬 Code graph analysis (1)
feature/savings/src/commonMain/kotlin/com/mifos/feature/savings/savingsAccountv2/pages/PreviewPage.kt (3)
core/ui/src/commonMain/kotlin/com/mifos/core/ui/components/MifosGeneralCard.kt (1)
MifosGeneralCard(76-97)core/ui/src/commonMain/kotlin/com/mifos/core/ui/components/MifosRowWithTextAndButton.kt (1)
MifosRowWithTextAndButton(35-95)core/ui/src/commonMain/kotlin/com/mifos/core/ui/components/MifosTwoButtonRow.kt (1)
MifosTwoButtonRow(31-91)
🔇 Additional comments (5)
feature/savings/src/commonMain/composeResources/values/feature_savings_strings.xml (2)
16-16: Required field indicator asterisks added consistently.The asterisks appended to "Product Name," "Currency," and "Field Officer" appropriately mark these as required form fields, following standard UI conventions. This aligns with the new preview workflow for account creation.
Also applies to: 31-31, 70-70
142-149: All newly added strings are actively used in the codebase.Verification confirms all seven strings have corresponding imports and are referenced in the following files:
feature_savings_yesandfeature_savings_no: Used in PreviewPage.kt for withdrawal fee and overdraft confirmation displaysfeature_savings_continue,feature_savings_success,feature_savings_failed: Used in SavingsAccountScreen.kt for submission result flowfeature_savings_new_savings_account_submitted_successandfeature_savings_new_savings_account_submitted_failed: Used in SavingsAccountViewModel.kt for success/error messagingNo unused strings were found.
feature/savings/src/commonMain/kotlin/com/mifos/feature/savings/savingsAccountv2/SavingsAccountViewModel.kt (2)
671-673: No action needed — implementation correctly separates concerns.The code already addresses the reviewer's concern:
SubmitSavingsApplicationis dispatched from the Preview step (PreviewPage.kt:190)Finishis reserved for post-success navigation only (dispatched at SavingsAccountScreen.kt:174 afterResultStatus.SUCCESS)The action separation is clear and prevents accidental bypass of submission logic.
127-161: The code is correctly typed—no serialization issues exist.State properties
minimumOpeningBalanceandmonthlyMinimumBalanceare alreadyString(line 608, 612 in SavingsAccountViewModel), andSavingsPayloadfieldsminRequiredOpeningBalanceandminRequiredBalanceexpectString?. The direct assignment at lines 139–140 is type-safe. Unlikecharges.amount(which isDoubleand requires.toString()), these balance fields are already stored as strings. Validation logic in TermsPage confirms values are numeric strings via.toDoubleOrNull()conversion.Likely an incorrect or invalid review comment.
core/network/src/commonMain/kotlin/com/mifos/core/network/datamanager/DataManagerSavings.kt (1)
292-317: Verify API response type before refactoring to GenericResponse.The review comment correctly identifies that
createSavingsAccountdoesn't follow the established codebase pattern. Other POST/create operations (e.g.,activateSavings,approveSavingsApplication) returnFlow<GenericResponse>, while this method attempts to parse the full response as aSavingsentity.The GenericResponse pattern with
responseFields: Map<String, JsonElement>is consistent across DataManager operations for POST requests. However, the refactor is only advisable if confirmed that the API returns a GenericResponse (typically containingresourceId) rather than the full Savings object.Action: Check the Fineract API documentation or test the
/savingsaccountsPOST endpoint directly to confirm whether it returns a GenericResponse withresourceIdor the complete Savings entity before proceeding with the suggested refactor.
...avings/src/commonMain/kotlin/com/mifos/feature/savings/savingsAccountv2/pages/PreviewPage.kt
Outdated
Show resolved
Hide resolved
| private fun createSavingsPayload(): SavingsPayload { | ||
| val savingsPayload = SavingsPayload() | ||
| savingsPayload.apply { | ||
| locale = "en" | ||
| dateFormat = "dd-MM-yyyy" | ||
| productId = state.savingProductOptions.getOrNull(state.savingsProductSelected)?.id | ||
| clientId = state.clientId | ||
| fieldOfficerId = state.fieldOfficerOptions[state.fieldOfficerIndex].id | ||
| submittedOnDate = state.submissionDate | ||
| externalId = state.externalId | ||
| allowOverdraft = state.isCheckedOverdraftAllowed | ||
| enforceMinRequiredBalance = state.isCheckedMinimumBalance | ||
| minRequiredOpeningBalance = state.minimumOpeningBalance | ||
| minRequiredBalance = state.monthlyMinimumBalance | ||
| lockinPeriodFrequency = state.frequency.toIntOrNull() | ||
| lockinPeriodFrequencyType = state.freqTypeIndex.let { | ||
| if (it != -1 && state.frequency.toIntOrNull() != null) it else null | ||
| } | ||
| charges = state.addedCharges.map { charges -> | ||
| ChargesPayload( | ||
| chargeId = charges.id, | ||
| amount = charges.amount.toString(), | ||
| ) | ||
| } | ||
| interestCompoundingPeriodType = | ||
| state.savingsProductTemplate?.interestCompoundingPeriodTypeOptions?.getOrNull(state.interestCompPeriodIndex)?.id | ||
| interestCalculationType = | ||
| state.savingsProductTemplate?.interestCalculationTypeOptions?.getOrNull(state.interestCalcIndex)?.id | ||
| interestCalculationDaysInYearType = | ||
| state.savingsProductTemplate?.interestCalculationDaysInYearTypeOptions?.getOrNull(state.daysInYearIndex)?.id | ||
| interestPostingPeriodType = | ||
| state.savingsProductTemplate?.interestPostingPeriodTypeOptions?.getOrNull(state.interestPostingPeriodIndex)?.id | ||
| } | ||
| return savingsPayload | ||
| } |
There was a problem hiding this comment.
Correct payload fields: avoid unsafe indexing and send option.id, not index.
- fieldOfficerId directly indexes; -1 will crash.
- lockinPeriodFrequencyType should be the selected option’s id, not the raw index.
Apply this diff:
- fieldOfficerId = state.fieldOfficerOptions[state.fieldOfficerIndex].id
+ fieldOfficerId = state.fieldOfficerOptions.getOrNull(state.fieldOfficerIndex)?.id
...
- lockinPeriodFrequencyType = state.freqTypeIndex.let {
- if (it != -1 && state.frequency.toIntOrNull() != null) it else null
- }
+ lockinPeriodFrequencyType =
+ state.savingsProductTemplate?.lockinPeriodFrequencyTypeOptions
+ ?.getOrNull(state.freqTypeIndex)?.id
+ ?.takeIf { state.frequency.toIntOrNull() != null }Also consider guarding submittedOnDate/format consistency with backend expectations.
🤖 Prompt for AI Agents
In
feature/savings/src/commonMain/kotlin/com/mifos/feature/savings/savingsAccountv2/SavingsAccountViewModel.kt
around lines 127-161, avoid unsafe indexing and send option.id (not raw
indexes): replace direct fieldOfficerOptions[state.fieldOfficerIndex].id with a
safe lookup like fieldOfficerOptions.getOrNull(state.fieldOfficerIndex)?.id (or
null if out of range) to prevent crashes when index is -1; change
lockinPeriodFrequencyType to use the selected option’s id (e.g.,
lockinPeriodFrequencyOptions.getOrNull(state.lockinIndex)?.id) instead of
passing the integer index; similarly ensure any other places using indices (like
freqTypeIndex) return the option.id via getOrNull and only when the numeric
frequency is present; finally guard submittedOnDate for null/format consistency
with backend (convert or null-check before assignment).
…unt, improve string resource usage and null safety
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
core/network/src/commonMain/kotlin/com/mifos/core/network/datamanager/DataManagerSavings.kt (1)
294-294: Consider extracting the Json instance to avoid repeated instantiation.The
Jsoninstance withignoreUnknownKeys = trueis created on every function call. For better performance, consider extracting it to a companion object or module-level property if this configuration is reused across the codebase.Example:
private val json = Json { ignoreUnknownKeys = true }Then reference it directly:
- val json = Json { ignoreUnknownKeys = true } + // Use the module-level json instancefeature/savings/src/commonMain/kotlin/com/mifos/feature/savings/savingsAccountv2/SavingsAccountViewModel.kt (1)
64-71: Reconsider setting screenState to Success on Finish.Setting
screenStatetoSuccessbefore emitting theFinishevent seems misleading. TheSuccessstate typically indicates a successful operation completion, but here it's used for navigation finish. Consider either:
- Preserving the current
screenState, or- Introducing a dedicated state like
ReadyToFinishif you need to signal this transition
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
core/network/src/commonMain/kotlin/com/mifos/core/network/datamanager/DataManagerSavings.kt(3 hunks)feature/savings/src/commonMain/composeResources/values/feature_savings_strings.xml(4 hunks)feature/savings/src/commonMain/kotlin/com/mifos/feature/savings/savingsAccountv2/SavingsAccountViewModel.kt(7 hunks)feature/savings/src/commonMain/kotlin/com/mifos/feature/savings/savingsAccountv2/pages/PreviewPage.kt(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- feature/savings/src/commonMain/composeResources/values/feature_savings_strings.xml
🧰 Additional context used
🧬 Code graph analysis (1)
feature/savings/src/commonMain/kotlin/com/mifos/feature/savings/savingsAccountv2/pages/PreviewPage.kt (3)
core/ui/src/commonMain/kotlin/com/mifos/core/ui/components/MifosGeneralCard.kt (1)
MifosGeneralCard(76-97)core/ui/src/commonMain/kotlin/com/mifos/core/ui/components/MifosRowWithTextAndButton.kt (1)
MifosRowWithTextAndButton(35-95)core/ui/src/commonMain/kotlin/com/mifos/core/ui/components/MifosTwoButtonRow.kt (1)
MifosTwoButtonRow(31-91)
🔇 Additional comments (10)
core/network/src/commonMain/kotlin/com/mifos/core/network/datamanager/DataManagerSavings.kt (3)
15-15: LGTM! Imports support the error handling implementation.The new imports are all properly utilized in the
createSavingsAccountfunction to consume the response body, check status, parse errors, and deserialize the result.Also applies to: 26-27, 33-33
291-310: Good implementation that addresses past review feedback.The function now properly consumes the HTTP response body, validates the status, handles errors with descriptive messages, and returns a typed
Savingsresult. This resolves the resource leak and error handling concerns raised in previous reviews.
298-301: Error extraction logic is correct and fully supported.The
MifosErrorstructure perfectly matches the error handling pattern in lines 298-301. The data class defines:
errors: List<Errors>where each item hasdefaultUserMessage: String- Top-level
defaultUserMessage: String?fieldAll three fallback options in the error extraction chain are valid and will execute as intended. No changes needed.
feature/savings/src/commonMain/kotlin/com/mifos/feature/savings/savingsAccountv2/pages/PreviewPage.kt (3)
57-69: LGTM! Safe access pattern correctly implemented.The function signature is now state-driven, and the preview details map construction uses
getOrNull()with null-safe fallbacks throughout. This addresses the past review concerns about unsafe indexing.
71-130: Excellent defensive programming for optional fields.The terms details map uses safe access throughout, and the lock-in period logic (lines 120-126) properly handles the case where either the frequency type is not selected or the frequency value is invalid, returning an empty string rather than crashing or displaying invalid data.
132-198: Well-structured layout with proper action handling.The UI layout correctly uses
LazyColumnwithweight(1f)inside aColumnto allow scrolling while keeping the button row fixed at the bottom. The actions are properly wired through theonActioncallback, and the submit button's enabled state is correctly driven bystate.isTermsNextEnabled.feature/savings/src/commonMain/kotlin/com/mifos/feature/savings/savingsAccountv2/SavingsAccountViewModel.kt (4)
119-125: LGTM! Clean delegation to submission flow.The action handling for
SubmitSavingsApplicationis clear and delegates appropriately to the payload creation and submission logic.
127-161: Excellent payload construction with comprehensive safe access.The payload builder correctly addresses all past review concerns:
- Safe index lookups with
getOrNull()throughout (lines 132, 134, 142-144, 151-158)lockinPeriodFrequencyTypecorrectly uses the option'sidrather than the raw index (lines 142-144)- Conditional inclusion via
takeIfensures the lock-in type is only set when frequency is valid- Charges properly mapped with correct field extraction
163-206: Robust submission flow with proper state management.The submission handling is well-implemented:
- Network status check before attempting submission
- Proper loading state management with overlay indicator
- Success and error cases display status dialogs with localized messages
- Network offline scenario handled separately with
NetworkErrorstate- The
getString()suspend calls (lines 181, 191) are correctly used within the coroutine context
648-648: LGTM! State and action extensions support the new submission workflow.The additions are well-defined:
ShowStatusDialogstate properly encapsulates operation result displaySubmitSavingsApplicationaction clearly represents user intent- Type declarations are clean and consistent with the existing pattern
Also applies to: 672-672, 722-723
3 participants
Fixes - Jira-#539
Apply for New Savings Account : Preview Functionality Implementation
Screen Recording:
PreviewSavings.mp4
Please make sure these boxes are checked before submitting your pull request - thanks!
Run the static analysis check
./gradlew checkorci-prepush.shto make sure you didn't break anythingIf you have multiple commits please combine them into one commit by squashing them.
Summary by CodeRabbit
New Features
Bug Fixes
Chores