Implemented Export Dialog Box in Transaction Screen#2606
Implemented Export Dialog Box in Transaction Screen#2606gurnoorpannu wants to merge 23 commits intoopenMF:developmentfrom
Conversation
# Conflicts: # feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanAccountSummary/LoanAccountSummaryScreen.kt
…mmary-screen' into refactor/improve-loan-account-summary-screen
📝 WalkthroughWalkthroughThis PR introduces a state/action-driven architecture for loan account summary, replacing the callback-heavy UI state pattern. It adds export transaction functionality with date range selection, removes the sealed Changes
Sequence DiagramsequenceDiagram
participant User
participant LoanTransactionsScreen
participant ExportTransactionsDialog
participant DatePicker
participant ViewModel
User->>LoanTransactionsScreen: Clicks Export Icon
LoanTransactionsScreen->>ExportTransactionsDialog: Show Dialog
User->>ExportTransactionsDialog: Taps From Date Field
ExportTransactionsDialog->>DatePicker: Open From Date Picker
User->>DatePicker: Select Date
DatePicker->>ExportTransactionsDialog: Confirm (fromDate: Long)
User->>ExportTransactionsDialog: Taps To Date Field
ExportTransactionsDialog->>DatePicker: Open To Date Picker
User->>DatePicker: Select Date
DatePicker->>ExportTransactionsDialog: Confirm (toDate: Long)
User->>ExportTransactionsDialog: Clicks Generate Report
alt Valid Date Range
ExportTransactionsDialog->>ViewModel: onGenerateReport(fromDate, toDate)
ViewModel->>ViewModel: Process Export
else Invalid Date Range
ExportTransactionsDialog->>ExportTransactionsDialog: Show Error Message
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 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)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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: 2
🤖 Fix all issues with AI agents
In
`@feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanAccountSummary/LoanAccountSummaryScreen.kt`:
- Around line 640-659: The composable function name LoadAccountSummaryDialog is
misspelled and should be LoanAccountSummaryDialog; rename the function
declaration (LoadAccountSummaryDialog -> LoanAccountSummaryDialog) and update
every call site (e.g., the route composable that references it) to use the
corrected symbol so references match, then rebuild to ensure imports and usages
(function name, any lambda parameters) are updated accordingly.
- Around line 272-277: The statusDescription uses hardcoded English strings in
LoanAccountSummaryScreen (val statusDescription) which breaks i18n; replace
those literals with stringResource lookups (e.g.,
stringResource(R.string.loan_status_active),
stringResource(R.string.loan_status_pending_approval),
stringResource(R.string.loan_status_waiting_for_disbursal),
stringResource(R.string.loan_status_closed)), add the corresponding string
resources in XML/strings files for translations, and ensure the composable
imports androidx.compose.ui.res.stringResource so the semantics
contentDescription uses localized text derived from
loanWithAssociations.status.active / pendingApproval / waitingForDisbursal
checks.
🧹 Nitpick comments (7)
feature/loan/src/commonMain/composeResources/values/strings.xml (1)
297-298: Duplicate string values:feature_loan_exportandfeature_loan_export_transactionsare both "Export".These two resources resolve to the same text. If they serve different semantic purposes (e.g., icon content description vs. dialog title), consider giving them distinct values to aid translators and accessibility. Otherwise, consolidate into a single resource.
feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanTransaction/ExportTransactionsDialog.kt (3)
200-209: Dead code: the!isValidDateRangeguard insideonClickis unreachable.The button is already
enabled = isValidDateRange(Line 209), soonClickcan never fire when the range is invalid. The early-return block on lines 202-204 is dead code.♻️ Simplify the onClick handler
MifosButton( onClick = { - if (!isValidDateRange) { - showInvalidDateRangeError = true - return@MifosButton - } - showInvalidDateRangeError = false onGenerateReport(fromDate!!, toDate!!) }, enabled = isValidDateRange, modifier = Modifier.weight(1f), ) {
225-233:formatDateFromMillisis marked@Composablebut uses no Compose APIs.This unnecessarily restricts where the function can be called. Remove the
@Composableannotation.♻️ Remove unnecessary `@Composable` annotation
`@OptIn`(ExperimentalTime::class) -@Composable private fun formatDateFromMillis(millis: Long?): String {
72-78:showInvalidDateRangeErrorusesrememberwhile sibling state variables userememberSaveable.This is inconsistent—on configuration change,
showInvalidDateRangeErrorresets tofalsewhile the date selections survive. This is likely acceptable since the error is transient, but worth noting the inconsistency. If intentional, no change needed.feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanAccountSummary/LoanAccountSummaryViewModel.kt (2)
124-131: Error details fromDataState.Errorare discarded.
dataStatelikely contains exception/message information, but only a generic "unknown error" string is shown. Consider using the actual error message when available for better user feedback and debugging.♻️ Use the actual error message with fallback
is DataState.Error -> { - val errorMessage = getString(Res.string.feature_loan_unknown_error_occured) + val errorMessage = dataState.exception.message + ?: getString(Res.string.feature_loan_unknown_error_occured) mutableStateFlow.update { it.copy( dialogState = LoanAccountSummaryState.DialogState.Error(errorMessage), ) } }
288-297:getPrimaryAction()silently defaults unknown/null statuses toCLOSED.If a new loan status is introduced or all status flags are false/null, the function returns
CLOSED, which disables the action button. This is a safe default, but consider logging a warning in theelsebranch to aid debugging.feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanAccountSummary/LoanAccountSummaryScreen.kt (1)
220-231: Unused lambda parameter inletblock.
state.loanWithAssociations?.let { loanWithAssociations -> ... }introducesloanWithAssociationsbut the content composable (LoanAccountSummaryContent) receivesstatedirectly—so theletparameter is only used as a null-guard. Consider using?.let { ... }without naming the parameter, or just use anifcheck.♻️ Simplify the null check
- state.loanWithAssociations?.let { loanWithAssociations -> - LoanAccountSummaryContent( - state = state, - onAction = onAction, - ) - } + if (state.loanWithAssociations != null) { + LoanAccountSummaryContent( + state = state, + onAction = onAction, + ) + }
| val statusDescription = when { | ||
| loanWithAssociations.status.active == true -> "Active" | ||
| loanWithAssociations.status.pendingApproval == true -> "Pending Approval" | ||
| loanWithAssociations.status.waitingForDisbursal == true -> "Waiting for Disbursal" | ||
| else -> "Closed" | ||
| } |
There was a problem hiding this comment.
Hardcoded English strings in status description break i18n/l10n.
The status descriptions ("Active", "Pending Approval", "Waiting for Disbursal", "Closed") are used in a semantics content description but are not backed by string resources. This will not be localized for non-English users, degrading accessibility.
♻️ Use string resources for status descriptions
Consider defining string resources for these status labels and referencing them via stringResource(...), similar to other UI text in this file.
🤖 Prompt for AI Agents
In
`@feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanAccountSummary/LoanAccountSummaryScreen.kt`
around lines 272 - 277, The statusDescription uses hardcoded English strings in
LoanAccountSummaryScreen (val statusDescription) which breaks i18n; replace
those literals with stringResource lookups (e.g.,
stringResource(R.string.loan_status_active),
stringResource(R.string.loan_status_pending_approval),
stringResource(R.string.loan_status_waiting_for_disbursal),
stringResource(R.string.loan_status_closed)), add the corresponding string
resources in XML/strings files for translations, and ensure the composable
imports androidx.compose.ui.res.stringResource so the semantics
contentDescription uses localized text derived from
loanWithAssociations.status.active / pendingApproval / waitingForDisbursal
checks.
| @Composable | ||
| fun getButtonText(status: LoanStatusEntity): String { | ||
| return when { | ||
| status.active == true || status.closedObligationsMet == true -> { | ||
| stringResource(Res.string.feature_loan_make_Repayment) | ||
| } | ||
|
|
||
| status.pendingApproval == true -> { | ||
| stringResource(Res.string.feature_loan_approve_loan) | ||
| } | ||
|
|
||
| status.waitingForDisbursal == true -> { | ||
| stringResource(Res.string.feature_loan_disburse_loan) | ||
| private fun LoadAccountSummaryDialog( | ||
| dialogState: LoanAccountSummaryState.DialogState?, | ||
| onAction: (LoanAccountSummaryAction) -> Unit, | ||
| ) { | ||
| when (dialogState) { | ||
| is LoanAccountSummaryState.DialogState.Error -> { | ||
| MifosSweetError( | ||
| message = dialogState.message, | ||
| onclick = { onAction(LoanAccountSummaryAction.OnRetry) }, | ||
| ) | ||
| } | ||
|
|
||
| status.overpaid == true -> { | ||
| stringResource(Res.string.feature_loan_overpaid) | ||
| LoanAccountSummaryState.DialogState.Loading -> { | ||
| MifosProgressIndicator() | ||
| } | ||
|
|
||
| else -> { | ||
| stringResource(Res.string.feature_loan_closed) | ||
| } | ||
| null -> Unit | ||
| } | ||
| } |
There was a problem hiding this comment.
LoadAccountSummaryDialog has a typo in the function name.
Should be LoanAccountSummaryDialog (missing the "n" in "Loan"). This is referenced both here (Line 641) and in the route composable (Line 181).
🤖 Prompt for AI Agents
In
`@feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanAccountSummary/LoanAccountSummaryScreen.kt`
around lines 640 - 659, The composable function name LoadAccountSummaryDialog is
misspelled and should be LoanAccountSummaryDialog; rename the function
declaration (LoadAccountSummaryDialog -> LoanAccountSummaryDialog) and update
every call site (e.g., the route composable that references it) to use the
corrected symbol so references match, then rebuild to ensure imports and usages
(function name, any lambda parameters) are updated accordingly.
2 participants
Fixes - Jira-#657
Before:-
After:-
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
Release Notes