Approve button in recurring account Issue #2580
Conversation
📝 WalkthroughWalkthroughThis PR adds a complete approval workflow for recurring deposit accounts across the application stack. It includes a date formatting utility update, new data model classes, repository/network layer methods, use cases, view models, UI screens, navigation routes, and string resources to enable users to approve recurring deposit accounts with approval date and optional notes. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User / UI
participant Screen as RecurringDepositAccountApprovalScreen
participant ViewModel as RecurringDepositAccountApprovalViewModel
participant UseCase as ApproveRecurringDepositUseCase
participant Repo as RecurringAccountRepository
participant Service as RecurringAccountService
participant API as Remote API
User->>Screen: Selects date & note, taps Save
Screen->>ViewModel: approveRecurringDepositApplication(approval)
ViewModel->>ViewModel: Sets ShowProgressbar state
ViewModel->>UseCase: invoke(accountId, approval)
UseCase->>Repo: approveRecurringDepositAccount(accountId, approval)
Repo->>Service: approveRecurringDepositAccount(accountId, approval)
Service->>API: POST recurringdepositaccounts/{accountId}?command=approve
API-->>Service: HttpResponse
Service-->>Repo: HttpResponse
Repo->>Repo: Parse response to GenericResponse
Repo-->>UseCase: DataState.Success(GenericResponse)
UseCase-->>ViewModel: DataState.Success(GenericResponse)
ViewModel->>ViewModel: Sets ShowRecurringDepositAccountApprovedSuccessfully state
ViewModel-->>Screen: State updated
Screen->>Screen: Shows success dialog
Screen->>User: Navigates back on dismiss
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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: 8
🤖 Fix all issues with AI agents
In `@core/common/src/commonMain/kotlin/com/mifos/core/common/utils/DateHelper.kt`:
- Around line 44-50: The apiDateFormat constant currently uses day() which
produces non-padded days; replace that call with dayOfMonth(Padding.ZERO) in the
apiDateFormat formatter (the LocalDateTime.Format block) so dates are rendered
as "dd MMMM yyyy" (e.g., "05 January 2026") to match the API and the other date
format constants referenced by RecurringDepositAccountApprovalScreen.kt:223.
In
`@core/network/src/androidMain/kotlin/com/mifos/core/network/KtorHttpClient.android.kt`:
- Around line 31-41: The file currently installs ContentNegotiation twice which
causes DuplicatePluginException; remove the redundant
install(ContentNegotiation) block and keep a single installation that preserves
the desired Json settings (ensure ignoreUnknownKeys = true, encodeDefaults =
true, isLenient = true, prettyPrint = true, explicitNulls = false) so the final
install(ContentNegotiation) { json(Json { ... }) } contains all required
options; verify only one call to install(ContentNegotiation) exists and run a
quick build to confirm HttpClient initializes without the duplicate plugin
error.
In
`@feature/client/src/commonMain/kotlin/com/mifos/feature/client/navigation/ClientNavigation.kt`:
- Around line 254-265: clientRecurringDepositAccountDestination is passing
onViewAccount as an empty lambda and onApproveAccount forwards accountNumber to
navigateToRecurringDepositAccountApproval which expects accountId; wire the
onViewAccount callback to navigate to the appropriate account view (e.g., call
navController.navigateToRecurringDepositAccount with the same identifier) and
make identifier names consistent: either rename accountNumber to accountId in
the onApproveAccount lambda or adapt the
navigateToRecurringDepositAccountApproval invocation to accept accountNumber,
ensuring RecurringDepositAccountScreen's OnViewAccount event and
navigateToRecurringDepositAccountApproval use the same identifier name/type.
In
`@feature/client/src/commonMain/kotlin/com/mifos/feature/client/recurringDepositAccount/RecurringDepositAccountScreen.kt`:
- Line 178: Fix the typo in the TODO comment inside
RecurringDepositAccountScreen (RecurringDepositAccountScreen.kt): replace
"clientimplement" with either "client implement" or simply "implement" (e.g.,
"todo implement search bar functionality") so the TODO reads clearly and
consistently with other comments.
- Around line 76-78: The LaunchedEffect keyed on
navController.currentBackStackEntry causes a duplicate fetch because the
ViewModel already loads data in its init via
checkNetworkAndRecurringDepositAccounts(); remove or change this LaunchedEffect
to avoid the immediate re-fetch: either delete the LaunchedEffect that calls
viewModel.trySendAction(RecurringDepositAccountAction.Refresh) or replace it
with a targeted mechanism that only triggers on return (e.g., observe
NavBackStackEntry.lifecycle events or a navigation result/SharedViewModel flag)
so that refresh runs only when coming back from another screen rather than on
initial composition.
In
`@feature/recurringDeposit/src/commonMain/kotlin/com/mifos/feature/recurringDeposit/recurringAccountApproval/RecurringDepositAccountApprovalScreen.kt`:
- Around line 113-121: Replace the hardcoded dialogTitle "OK" in the
RecurringDepositAccountApprovalUiState.ShowRecurringDepositAccountApprovedSuccessfully
branch with a localized string resource; update the MifosAlertDialog call to use
stringResource(Res.string.xxx) (e.g., Res.string.ok or a feature-specific
resource) for dialogTitle so the title is internationalized while keeping
onConfirmation calling navigateBack and other parameters unchanged.
- Line 235: The file defines a local no-op function rememberDatePickerState that
shadows the imported API and causes datePickerState to be Unit; remove this
stubbed function (the rememberDatePickerState declaration) so the code uses the
correct imported rememberDatePickerState and datePickerState will have the
proper type to access selectedDateMillis, or if you intentionally need a local
helper, rename it and match the real API signature to return the proper state
instead of Unit.
- Around line 142-147: The date picker currently uses
SelectableDates.isSelectableDate to allow only dates >= Clock.System.now(),
blocking today and past dates; update the logic in the
datePickerState/selectableDates block (SelectableDates.isSelectableDate) to
allow today and earlier by inverting the comparison (use utcTimeMillis <=
Clock.System.now().toEpochMilliseconds()) or, better, compare dates by LocalDate
(convert utcTimeMillis and now to LocalDate and allow selectable when
candidateDate <= today) so same-day approvals are permitted; adjust the
implementation in RecurringDepositAccountApprovalScreen where datePickerState is
created.
🧹 Nitpick comments (5)
feature/client/src/commonMain/kotlin/com/mifos/feature/client/recurringDepositAccount/RecurringDepositAccountScreen.kt (1)
232-250: Empty string fallback for nullaccountNomay cause silent failures.Passing an empty string when
accountNois null (lines 237, 244) could lead to confusing behavior or failed API calls downstream. Consider either:
- Filtering out items with null
accountNofrom displaying actions, or- Handling the null case explicitly (e.g., showing an error dialog)
Suggested approach: guard against null before dispatching action
onActionClicked = { actions -> + val accountNo = recurringDeposit.accountNo + if (accountNo.isNullOrEmpty()) return@MifosActionsSavingsListingComponent when (actions) { is Actions.ViewAccount -> { onAction( RecurringDepositAccountAction.ViewAccount( - recurringDeposit.accountNo ?: "", + accountNo, ) ) } is Actions.ApproveAccount -> { onAction( RecurringDepositAccountAction.ApproveAccount( - recurringDeposit.accountNo ?: "", + accountNo, ) ) } else -> Unit } }feature/recurringDeposit/src/commonMain/kotlin/com/mifos/feature/recurringDeposit/di/RecurringDepositModule.kt (1)
22-33: Consider usingviewModelOffor consistency.The existing
RecurringAccountViewModelusesviewModelOf(::RecurringAccountViewModel)(line 20), while the newRecurringDepositAccountApprovalViewModeluses explicit parameter injection. IfSavedStateHandleandApproveRecurringDepositUseCaseare both available via DI, you could simplify:♻️ Suggested simplification
- viewModel { - RecurringDepositAccountApprovalViewModel( - savedStateHandle = get(), - approveRecurringDepositUseCase = get() - ) - } + viewModelOf(::RecurringDepositAccountApprovalViewModel) - factory { - ApproveRecurringDepositUseCase( - repository = get() - ) - } + factory(::ApproveRecurringDepositUseCase)core/network/src/commonMain/kotlin/com/mifos/core/network/services/RecurringAccountService.kt (1)
37-41: UseAPIEndPoint.CREATE_RECURRING_DEPOSIT_ACCOUNTSfor consistency.Lines 26 and 31 use
APIEndPoint.CREATE_RECURRING_DEPOSIT_ACCOUNTS, but this endpoint uses a hardcoded string. For maintainability and consistency with the rest of the interface, apply the suggested refactor:♻️ Suggested refactor
- `@POST`("recurringdepositaccounts/{accountId}?command=approve") + `@POST`(APIEndPoint.CREATE_RECURRING_DEPOSIT_ACCOUNTS + "/{accountId}?command=approve") fun approveRecurringDepositAccount( `@Path`("accountId") accountId: String, `@Body` approval: RecurringDepositApproval, ): Flow<HttpResponse>core/model/src/commonMain/kotlin/com/mifos/core/model/objects/template/recurring/approval/RecurringDepositApproval.kt (1)
15-22: Prefer immutable properties for request payloads.
These fields look write-once; switching tovalavoids accidental mutation.♻️ Suggested change
-data class RecurringDepositApproval( - var locale: String = "en", - var dateFormat: String = "dd MMMM yyyy", - var approvedOnDate: String? = null, - var note: String? = null, -) +data class RecurringDepositApproval( + val locale: String = "en", + val dateFormat: String = "dd MMMM yyyy", + val approvedOnDate: String? = null, + val note: String? = null, +)core/network/src/commonMain/kotlin/com/mifos/core/network/datamanager/DataManagerRecurringAccount.kt (1)
56-71: Consider extracting the shared response-mapping logic.
This duplicates the error/success parsing increateRecurringDepositAccount; a helper keeps it consistent and easier to maintain.
| private val apiDateFormat = LocalDateTime.Format { | ||
| day() | ||
| char(' ') | ||
| monthName(MonthNames.ENGLISH_FULL) | ||
| char(' ') | ||
| year() | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n core/common/src/commonMain/kotlin/com/mifos/core/common/utils/DateHelper.kt | head -60Repository: openMF/android-client
Length of output: 2245
🏁 Script executed:
# Search for approval-related date formatting requirements or documentation
rg -n "getDateAsStringForApproval|approval.*date|date.*approval" --type=kt -B3 -A3Repository: openMF/android-client
Length of output: 91
🏁 Script executed:
# Check for other date formatting examples in the file to understand the pattern
rg -n "LocalDateTime.Format|day\(\)|dayOfMonth" --type=kt core/common/src/commonMain/kotlin/com/mifos/core/common/utils/DateHelper.ktRepository: openMF/android-client
Length of output: 91
🏁 Script executed:
# Check for API contracts or tests that might show expected format
fd -e kt -e java -e txt | xargs rg -l "getDateAsStringForApproval" 2>/dev/null | head -10Repository: openMF/android-client
Length of output: 288
🏁 Script executed:
# Find the getDateAsStringForApproval function implementation
rg -n "getDateAsStringForApproval" core/common/src/commonMain/kotlin/com/mifos/core/common/utils/DateHelper.kt -A5Repository: openMF/android-client
Length of output: 362
🏁 Script executed:
# Check usage in RecurringDepositAccountApprovalScreen
cat feature/recurringDeposit/src/commonMain/kotlin/com/mifos/feature/recurringDeposit/recurringAccountApproval/RecurringDepositAccountApprovalScreen.kt | grep -n "getDateAsStringForApproval" -B3 -A3Repository: openMF/android-client
Length of output: 413
🏁 Script executed:
# Check if there are any tests or documentation about expected date format
fd -type f \( -name "*Test*" -o -name "*test*" \) | xargs rg -l "apiDateFormat\|getDateAsStringForApproval" 2>/dev/null | head -5Repository: openMF/android-client
Length of output: 235
🏁 Script executed:
# Look for any pattern that might show API requirements (search for "dd" pattern in date formats)
rg -n "dd.*yyyy|yyyy.*dd" core/common/src/commonMain/kotlin/com/mifos/core/common/utils/DateHelper.ktRepository: openMF/android-client
Length of output: 1053
Use dayOfMonth(Padding.ZERO) to match the API's expected date format.
The API expects dates in the format "dd MMMM yyyy" (zero-padded days) as shown in RecurringDepositAccountApprovalScreen.kt:223. The current implementation using day() produces non-padded days (e.g., "5 January 2026" instead of "05 January 2026"), which will cause a format mismatch with the API.
This is inconsistent with all other date format constants in the file (FULL_MONTH = "dd MM yyyy", SHORT_MONTH = "dd-MM-yyyy"), which all use zero-padded days. Apply the suggested fix:
Required fix
private val apiDateFormat = LocalDateTime.Format {
- day()
+ dayOfMonth(Padding.ZERO)
char(' ')
monthName(MonthNames.ENGLISH_FULL)
char(' ')
year()
}🤖 Prompt for AI Agents
In `@core/common/src/commonMain/kotlin/com/mifos/core/common/utils/DateHelper.kt`
around lines 44 - 50, The apiDateFormat constant currently uses day() which
produces non-padded days; replace that call with dayOfMonth(Padding.ZERO) in the
apiDateFormat formatter (the LocalDateTime.Format block) so dates are rendered
as "dd MMMM yyyy" (e.g., "05 January 2026") to match the API and the other date
format constants referenced by RecurringDepositAccountApprovalScreen.kt:223.
| install(ContentNegotiation) { | ||
| json( | ||
| Json { | ||
| ignoreUnknownKeys = true | ||
| encodeDefaults = true | ||
| isLenient = true | ||
| prettyPrint = true | ||
| explicitNulls = false | ||
| }, | ||
| ) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd -t f "KtorHttpClient.android.kt" | head -5Repository: openMF/android-client
Length of output: 243
🏁 Script executed:
cat -n core/network/src/androidMain/kotlin/com/mifos/core/network/KtorHttpClient.android.kt | head -80Repository: openMF/android-client
Length of output: 2500
🌐 Web query:
Ktor HttpClient ContentNegotiation install twice duplicate plugin exception
💡 Result:
Cause
- The exception means ContentNegotiation was installed more than once with the same plugin key (e.g., installed twice on the same HttpClient or both globally and again in a test/server scope). Ktor throws DuplicatePluginException when a plugin is installed twice. [1][2]
How to fix
- Ensure you install ContentNegotiation only once per client (or per application scope). Remove the duplicate install (common places: shared client factory, tests that call module twice, or installing on both a parent route and its child incorrectly). [1][3]
How to check/guard in code (client)
- Use pluginOrNull to see if the plugin is already installed before installing:
- if (client.pluginOrNull(ContentNegotiation) == null) { /* install */ }
- or when creating clients, centralize installation in one factory so callers reuse the same configured client. [2]
Example (HttpClient factory)
- val client = HttpClient(CIO) {
install(ContentNegotiation) { json() }
}
Testing tip
- If tests start your app twice (EngineMain vs application { module(...) } or you call module twice), remove the duplicate startup or use embeddedServer only in one place to avoid reinstalling server plugins. StackOverflow examples show this pattern as a common cause. [3]
Sources
- Ktor Client ContentNegotiation docs. [1]
- Ktor API: pluginOrNull for HttpClient. [2]
- StackOverflow discussion about DuplicatePluginException when modules/tests start things twice. [3]
Consolidate duplicate ContentNegotiation installations (causes DuplicatePluginException).
This file installs ContentNegotiation twice (lines 31–41 and lines 53–62). Ktor throws DuplicatePluginException when the same plugin is installed more than once, preventing HttpClient initialization. Keep one install(ContentNegotiation) block with your desired JSON configuration. Note: the first block includes encodeDefaults = true, which the second lacks—ensure you retain the options you need.
🤖 Prompt for AI Agents
In
`@core/network/src/androidMain/kotlin/com/mifos/core/network/KtorHttpClient.android.kt`
around lines 31 - 41, The file currently installs ContentNegotiation twice which
causes DuplicatePluginException; remove the redundant
install(ContentNegotiation) block and keep a single installation that preserves
the desired Json settings (ensure ignoreUnknownKeys = true, encodeDefaults =
true, isLenient = true, prettyPrint = true, explicitNulls = false) so the final
install(ContentNegotiation) { json(Json { ... }) } contains all required
options; verify only one call to install(ContentNegotiation) exists and run a
quick build to confirm HttpClient initializes without the duplicate plugin
error.
| clientRecurringDepositAccountDestination( | ||
| navController = navController, | ||
| navigateBack = navController::popBackStack, | ||
| {}, | ||
| {}, | ||
| onApproveAccount = { accountNumber -> | ||
| navController.navigateToRecurringDepositAccountApproval(accountNumber) | ||
| }, | ||
| onViewAccount = {}, | ||
| ) | ||
|
|
||
| recurringDepositAccountApprovalDestination( | ||
| navigateBack = navController::popBackStack, | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for clientRecurringDepositAccountDestination function definition
rg -n "fun.*clientRecurringDepositAccountDestination" --type=kt -A 10Repository: openMF/android-client
Length of output: 186
🏁 Script executed:
#!/bin/bash
# Search for recurringDepositAccountApprovalDestination and how it uses parameters
rg -n "fun.*recurringDepositAccountApprovalDestination" --type=kt -A 10Repository: openMF/android-client
Length of output: 186
🏁 Script executed:
#!/bin/bash
# Search for how onApproveAccount callback is defined in the recurring deposit screen
rg -n "onApproveAccount" --type=kt -B 3 -A 3Repository: openMF/android-client
Length of output: 91
🏁 Script executed:
#!/bin/bash
# Search for how onViewAccount callback is defined/used
rg -n "onViewAccount" --type=kt -B 3 -A 3Repository: openMF/android-client
Length of output: 91
🏁 Script executed:
#!/bin/bash
# Check the navigateToRecurringDepositAccountApproval extension function
rg -n "navigateToRecurringDepositAccountApproval" --type=kt -A 5Repository: openMF/android-client
Length of output: 91
🏁 Script executed:
#!/bin/bash
# Search for clientRecurringDepositAccountDestination function definition
rg -n "fun.*clientRecurringDepositAccountDestination" -A 10Repository: openMF/android-client
Length of output: 1805
🏁 Script executed:
#!/bin/bash
# Search for recurringDepositAccountApprovalDestination definition
rg -n "fun.*recurringDepositAccountApprovalDestination" -A 10Repository: openMF/android-client
Length of output: 2090
🏁 Script executed:
#!/bin/bash
# Search for onApproveAccount usage patterns
rg -n "onApproveAccount" -B 2 -A 2Repository: openMF/android-client
Length of output: 7632
🏁 Script executed:
#!/bin/bash
# Search for onViewAccount usage patterns
rg -n "onViewAccount" -B 2 -A 2Repository: openMF/android-client
Length of output: 7593
🏁 Script executed:
#!/bin/bash
# Search for navigateToRecurringDepositAccountApproval extension
rg -n "navigateToRecurringDepositAccountApproval" -A 5Repository: openMF/android-client
Length of output: 2888
Wire the onViewAccount callback and verify the account identifier naming.
The onViewAccount callback is actively used by RecurringDepositAccountScreen (triggered on OnViewAccount event), but it's passed as an empty lambda in the navigation setup. Additionally, the onApproveAccount callback passes accountNumber to navigateToRecurringDepositAccountApproval(), which expects a parameter named accountId—verify these represent the same identifier and use consistent naming throughout.
🤖 Prompt for AI Agents
In
`@feature/client/src/commonMain/kotlin/com/mifos/feature/client/navigation/ClientNavigation.kt`
around lines 254 - 265, clientRecurringDepositAccountDestination is passing
onViewAccount as an empty lambda and onApproveAccount forwards accountNumber to
navigateToRecurringDepositAccountApproval which expects accountId; wire the
onViewAccount callback to navigate to the appropriate account view (e.g., call
navController.navigateToRecurringDepositAccount with the same identifier) and
make identifier names consistent: either rename accountNumber to accountId in
the onApproveAccount lambda or adapt the
navigateToRecurringDepositAccountApproval invocation to accept accountNumber,
ensuring RecurringDepositAccountScreen's OnViewAccount event and
navigateToRecurringDepositAccountApproval use the same identifier name/type.
| LaunchedEffect(navController.currentBackStackEntry) { | ||
| viewModel.trySendAction(RecurringDepositAccountAction.Refresh) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "RecurringDepositAccountViewModel.kt" | head -5Repository: openMF/android-client
Length of output: 188
🏁 Script executed:
rg -n "class RecurringDepositAccountViewModel" --type=kt -A 20Repository: openMF/android-client
Length of output: 91
🏁 Script executed:
rg -n "init\s*\{" --type=kt | grep -i "RecurringDepositAccount"Repository: openMF/android-client
Length of output: 91
🏁 Script executed:
cat ./feature/client/src/commonMain/kotlin/com/mifos/feature/client/recurringDepositAccount/RecurringDepositAccountViewModel.ktRepository: openMF/android-client
Length of output: 7379
Remove redundant data fetch on initial composition.
The ViewModel loads data in its init block via checkNetworkAndRecurringDepositAccounts(). Using navController.currentBackStackEntry as the LaunchedEffect key will trigger another fetch immediately after composition, causing duplicate API calls on initial load.
Use a more targeted approach if the intent is only to refresh when returning from another screen—for example, track navigation result or use NavBackStackEntry.lifecycle events instead of currentBackStackEntry reference changes.
🤖 Prompt for AI Agents
In
`@feature/client/src/commonMain/kotlin/com/mifos/feature/client/recurringDepositAccount/RecurringDepositAccountScreen.kt`
around lines 76 - 78, The LaunchedEffect keyed on
navController.currentBackStackEntry causes a duplicate fetch because the
ViewModel already loads data in its init via
checkNetworkAndRecurringDepositAccounts(); remove or change this LaunchedEffect
to avoid the immediate re-fetch: either delete the LaunchedEffect that calls
viewModel.trySendAction(RecurringDepositAccountAction.Refresh) or replace it
with a targeted mechanism that only triggers on return (e.g., observe
NavBackStackEntry.lifecycle events or a navigation result/SharedViewModel flag)
so that refresh runs only when coming back from another screen rather than on
initial composition.
| ) | ||
|
|
||
| // todo implement search bar functionality | ||
| // todo clientimplement search bar functionality |
There was a problem hiding this comment.
Typo in TODO comment.
"clientimplement" should be "client implement" or possibly just "implement".
Proposed fix
- // todo clientimplement search bar functionality
+ // TODO: implement search bar functionality📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // todo clientimplement search bar functionality | |
| // TODO: implement search bar functionality |
🤖 Prompt for AI Agents
In
`@feature/client/src/commonMain/kotlin/com/mifos/feature/client/recurringDepositAccount/RecurringDepositAccountScreen.kt`
at line 178, Fix the typo in the TODO comment inside
RecurringDepositAccountScreen (RecurringDepositAccountScreen.kt): replace
"clientimplement" with either "client implement" or simply "implement" (e.g.,
"todo implement search bar functionality") so the TODO reads clearly and
consistently with other comments.
| is RecurringDepositAccountApprovalUiState.ShowRecurringDepositAccountApprovedSuccessfully -> | ||
| MifosAlertDialog( | ||
| dialogTitle = "OK", | ||
| dialogText = stringResource(Res.string.feature_recurring_deposit_account), | ||
| dismissText = null, | ||
| onConfirmation = { | ||
| navigateBack.invoke() | ||
| }, | ||
| onDismissRequest = {}, |
There was a problem hiding this comment.
Localize the dialog title instead of hardcoding.
"OK" should come from a string resource for i18n.
🤖 Prompt for AI Agents
In
`@feature/recurringDeposit/src/commonMain/kotlin/com/mifos/feature/recurringDeposit/recurringAccountApproval/RecurringDepositAccountApprovalScreen.kt`
around lines 113 - 121, Replace the hardcoded dialogTitle "OK" in the
RecurringDepositAccountApprovalUiState.ShowRecurringDepositAccountApprovedSuccessfully
branch with a localized string resource; update the MifosAlertDialog call to use
stringResource(Res.string.xxx) (e.g., Res.string.ok or a feature-specific
resource) for dialogTitle so the title is internationalized while keeping
onConfirmation calling navigateBack and other parameters unchanged.
| val datePickerState = rememberDatePickerState( | ||
| initialSelectedDateMillis = approvalDate, | ||
| selectableDates = object : SelectableDates { | ||
| override fun isSelectableDate(utcTimeMillis: Long): Boolean { | ||
| return utcTimeMillis >= Clock.System.now().toEpochMilliseconds() | ||
| } |
There was a problem hiding this comment.
Approval date picker currently only allows future dates.
The >= now check blocks today/past approvals. If approvals should be today or earlier, flip the comparison (or compare by date).
🐛 Suggested fix
- return utcTimeMillis >= Clock.System.now().toEpochMilliseconds()
+ return utcTimeMillis <= Clock.System.now().toEpochMilliseconds()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| val datePickerState = rememberDatePickerState( | |
| initialSelectedDateMillis = approvalDate, | |
| selectableDates = object : SelectableDates { | |
| override fun isSelectableDate(utcTimeMillis: Long): Boolean { | |
| return utcTimeMillis >= Clock.System.now().toEpochMilliseconds() | |
| } | |
| val datePickerState = rememberDatePickerState( | |
| initialSelectedDateMillis = approvalDate, | |
| selectableDates = object : SelectableDates { | |
| override fun isSelectableDate(utcTimeMillis: Long): Boolean { | |
| return utcTimeMillis <= Clock.System.now().toEpochMilliseconds() | |
| } |
🤖 Prompt for AI Agents
In
`@feature/recurringDeposit/src/commonMain/kotlin/com/mifos/feature/recurringDeposit/recurringAccountApproval/RecurringDepositAccountApprovalScreen.kt`
around lines 142 - 147, The date picker currently uses
SelectableDates.isSelectableDate to allow only dates >= Clock.System.now(),
blocking today and past dates; update the logic in the
datePickerState/selectableDates block (SelectableDates.isSelectableDate) to
allow today and earlier by inverting the comparison (use utcTimeMillis <=
Clock.System.now().toEpochMilliseconds()) or, better, compare dates by LocalDate
(convert utcTimeMillis and now to LocalDate and allow selectable when
candidateDate <= today) so same-day approvals are permitted; adjust the
implementation in RecurringDepositAccountApprovalScreen where datePickerState is
created.
| } | ||
| } | ||
|
|
||
| fun rememberDatePickerState(initialSelectedDateMillis: Long, selectableDates: Any) {} |
There was a problem hiding this comment.
Remove the stubbed rememberDatePickerState.
The local no-op function shadows the imported API and makes datePickerState type Unit, which will not compile when accessing selectedDateMillis.
🐛 Suggested fix
-fun rememberDatePickerState(initialSelectedDateMillis: Long, selectableDates: Any) {}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fun rememberDatePickerState(initialSelectedDateMillis: Long, selectableDates: Any) {} |
🤖 Prompt for AI Agents
In
`@feature/recurringDeposit/src/commonMain/kotlin/com/mifos/feature/recurringDeposit/recurringAccountApproval/RecurringDepositAccountApprovalScreen.kt`
at line 235, The file defines a local no-op function rememberDatePickerState
that shadows the imported API and causes datePickerState to be Unit; remove this
stubbed function (the rememberDatePickerState declaration) so the code uses the
correct imported rememberDatePickerState and datePickerState will have the
proper type to access selectedDateMillis, or if you intentionally need a local
helper, rename it and match the real API signature to return the proper state
instead of Unit.
|
@ufuomaisaac Please address the @coderabbitai reviews which are relevant to this PR. Make sure the UI guidelines are being followed as well. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 11400
Critical Issues (Must Fix) 🔴
Important Fixes 🟡
Code Quality Improvements (Nice to Have) 💡
I'm happy to help clarify any of these items or provide more specific guidance on how to fix them. Just let me know! 🤖 |
Alright, will work on that. Thank you! |
|
Tip For best results, initiate chat on the files or code changes.
|
1 similar comment
|
Tip For best results, initiate chat on the files or code changes.
|
2 participants
Fixes - Jira-#Issue_Number
Didn't create a Jira ticket, click here to create new.
Please Add Screenshots If there are any UI changes.
trim.DD8A9707-D371-437B-9E54-E1375B38FC0D.MOV
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.