Skip to content

Conversation

@shashank-0-0
Copy link
Contributor

@shashank-0-0 shashank-0-0 commented Nov 17, 2025

This pr completes this #2534 .

Screen_recording_20251117_225525.mp4

@coderabbitai
Copy link

coderabbitai bot commented Nov 17, 2025

Walkthrough

This PR introduces comprehensive Fixed Deposit account creation functionality across data, network, and feature layers. It establishes a repository pattern for data access, adds network services and models for API integration, and implements a complete feature-layer MVVM flow with state management and UI components for user-driven account creation.

Changes

Cohort / File(s) Summary
Repository & Data Layer
core/data/src/commonMain/kotlin/com/mifos/core/data/repository/FixedDepositRepository.kt, core/data/src/commonMain/kotlin/com/mifos/core/data/repositoryImp/FixedDepositRepositoryImpl.kt, core/data/src/commonMain/kotlin/com/mifos/core/data/di/RepositoryModule.kt
Introduces FixedDepositRepository interface with getFixedDepositTemplate method, FixedDepositRepositoryImpl implementation delegating to DataManagerFixedDeposit, and DI binding via singleOf.
Network Data Manager & DI
core/network/src/commonMain/kotlin/com/mifos/core/network/datamanager/DataManagerFixedDeposit.kt, core/network/src/commonMain/kotlin/com/mifos/core/network/di/DataMangerModule.kt
Adds DataManagerFixedDeposit class providing getFixedDepositTemplate by delegating to FixedDepositService, with Koin singleton registration.
Network Service & API Manager
core/network/src/commonMain/kotlin/com/mifos/core/network/services/FixedDepositService.kt, core/network/src/commonMain/kotlin/com/mifos/core/network/BaseApiManager.kt
Introduces FixedDepositService interface with fixedDepositProductTemplate GET endpoint, and adds fixedDepositService property to BaseApiManager.
Network Data Models
core/network/src/commonMain/kotlin/com/mifos/core/network/model/FixedDepositTemplate.kt, core/network/src/commonMain/kotlin/com/mifos/core/network/model/FixedDepositProductOption.kt
Defines @Serializable data classes FixedDepositTemplate and FixedDepositProductOption for API response mapping.
API Endpoint Configuration
core/database/src/commonMain/kotlin/com/mifos/room/basemodel/APIEndPoint.kt, core/database/src/commonMain/kotlin/com/mifos/room/entities/accounts/savings/SavingAccountDepositTypeEntity.kt
Adds FIXED_DEPOSIT constant to APIEndPoint; updates ServerTypes.FIXED enum to reference FIXED_DEPOSIT instead of SAVINGS_ACCOUNTS.
Feature Navigation & Integration
feature/client/src/commonMain/kotlin/com/mifos/feature/client/navigation/ClientNavigation.kt, feature/client/src/commonMain/kotlin/com/mifos/feature/client/newFixedDepositAccount/createFixedDepositAccountRoute.kt, feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientApplyNewApplications/ClientApplyNewApplicationRoute.kt, feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientApplyNewApplications/ClientApplyNewApplicationsScreen.kt
Replaces old FixedDepositRoute with createFixedDepositAccountRoute(clientId), updates navigation functions to pass clientId parameter, and wires navigation from ClientApplyNewApplications to CreateFixedDepositAccountScreen.
Feature ViewModel & DI
feature/client/src/commonMain/kotlin/com/mifos/feature/client/newFixedDepositAccount/createFixedDepositAccountViewmodel.kt, feature/client/src/commonMain/kotlin/com/mifos/feature/client/di/ClientModule.kt
Introduces CreateFixedDepositAccountViewmodel with complete state management (NewFixedDepositAccountState, FixedDepositAccountDetailsState, actions, and events); registers viewmodel in Koin module; removes old NewFixedDepositAccountViewmodel.
Feature UI Components & Resources
feature/client/src/commonMain/kotlin/com/mifos/feature/client/newFixedDepositAccount/createFixedDepositAccountScreen.kt, feature/client/src/commonMain/kotlin/com/mifos/feature/client/newFixedDepositAccount/pages/DetailsPage.kt, feature/client/src/commonMain/composeResources/values/strings.xml
Refactors FixedDepositAccountScreen to CreateFixedDepositAccountScreen with Koin-based DI and NavController integration; replaces DetailsPage with state-driven form UI including date picker, product selection, field officer selection, and validation; adds new string resources.

Sequence Diagram(s)

sequenceDiagram
    actor User
    participant Screen as CreateFixedDepositAccountScreen
    participant ViewModel as CreateFixedDepositAccountViewmodel
    participant Repo as FixedDepositRepository
    participant DataMgr as DataManagerFixedDeposit
    participant Service as FixedDepositService
    participant API as Backend API

    User->>Screen: Navigate with clientId
    Screen->>ViewModel: Load (SavedStateHandle)
    ViewModel->>ViewModel: Extract clientId from route
    ViewModel->>Repo: getFixedDepositTemplate(clientId, productId)
    Repo->>DataMgr: getFixedDepositTemplate(clientId, productId)
    DataMgr->>Service: fixedDepositProductTemplate(clientId, productId)
    Service->>API: GET /fixeddepositaccounts/template
    API-->>Service: FixedDepositTemplate (productOptions, fieldOfficerOptions)
    Service-->>DataMgr: Flow<FixedDepositTemplate>
    DataMgr-->>Repo: Flow<FixedDepositTemplate>
    Repo-->>ViewModel: Flow<DataState<FixedDepositTemplate>>
    
    rect rgb(200, 240, 255)
    Note over ViewModel: Load state → Success with template
    end
    
    ViewModel->>ViewModel: Update state.screenState = Success
    ViewModel->>ViewModel: Update template in state
    Screen->>Screen: Render with template data (products, officers)
    User->>Screen: Select product, officer, set details
    Screen->>ViewModel: Dispatch action (OnProductNameChange, OnFieldOfficerChange, etc.)
    ViewModel->>ViewModel: Update fixedDepositAccountDetail state
    User->>Screen: Click Next on DetailsPage
    Screen->>ViewModel: Dispatch OnNextPress
    ViewModel->>ViewModel: Validate isDetailsNextEnabled, emit event
    Screen->>Screen: Advance to next step
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20–30 minutes

  • Multiple interconnected layers: Changes span repository, data manager, network service, data models, and feature layers, each requiring understanding of the pattern and integration.
  • New ViewModel with complex state management: CreateFixedDepositAccountViewmodel introduces nested sealed interfaces (ScreenState), computed properties, and multi-step form state that warrant careful review of action handling and state transitions.
  • UI composition refactor: CreateFixedDepositAccountScreen and DetailsPage undergo significant structural changes with new DI, error/loading states, and form validation logic.
  • Navigation parameter changes: Parameter propagation of clientId across multiple layers requires tracing to ensure consistency.

Areas requiring extra attention:

  • CreateFixedDepositAccountViewmodel action handling logic and state transitions, especially for step navigation and template loading error scenarios
  • CreateFixedDepositAccountScreen state-driven rendering with ScreenState (Loading/Error/Success) and conditional composition
  • Navigation route extraction and parameter passing consistency (clientId propagation through SavedStateHandle)
  • Impact of ServerTypes.FIXED endpoint change on existing savings account flows
  • DetailsPage form validation and error state handling (fieldOfficerError, externalIdError)

Possibly related PRs

  • Implement New Share Account – Details Step #2530: Introduces parallel account-type (ShareAccount) support with an identical pattern—repository/data manager/service layers, ViewModel/navigation/screens, and DI bindings—making it a strong structural parallel to this Fixed Deposit implementation.

Suggested reviewers

  • Arinyadav1
  • sam-arth07
  • biplab1

Poem

🐰 A rabbit's ode to Fixed Deposits!

Hop through templates, gather those rates,
Step by step, the form navigates,
Repository layers, services align,
From client to product, a flow so fine,
Fixed Deposits now bloom—no more delays! 🌱

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Title check ❓ Inconclusive The title 'Mifo 557' is vague and does not convey meaningful information about the changeset. While it references an issue number, it lacks context about the actual changes being made. Replace the title with a descriptive summary of the main change, such as 'Add Fixed Deposit Account creation flow' or 'Implement FixedDepositRepository and CreateFixedDepositAccountViewmodel'.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Tip

📝 Customizable high-level summaries are now available in beta!

You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.

  • Provide your own instructions using the high_level_summary_instructions setting.
  • Format the summary however you like (bullet lists, tables, contributor stats, etc.).
  • Use high_level_summary_in_walkthrough to move the summary from the description to the walkthrough section.

Example:

"Create a concise high-level summary as a bullet-point list. Then include a Markdown table showing lines added and removed by each contributing author."

Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (16)
feature/client/src/commonMain/composeResources/values/strings.xml (1)

540-544: Align resource key naming with established convention.

The new string resource keys follow the established snake_case lowercase naming pattern, except for line 544: Field_officer uses mixed case. For consistency with the rest of the file (e.g., feature_client_add_savings_account, client_profile_title), rename the resource key to field_officer.

The string value "Field Officer" is correctly capitalized for display purposes.

Apply this change:

-    <string name="Field_officer">Field Officer</string>
+    <string name="field_officer">Field Officer</string>
feature/client/src/commonMain/kotlin/com/mifos/feature/client/di/ClientModule.kt (1)

43-43: Inconsistent casing in ViewModel class name.

The class is named CreateFixedDepositAccountViewmodel (lowercase 'm'), while most other ViewModels in this file use ViewModel (uppercase 'M'). Consider renaming to CreateFixedDepositAccountViewModel for consistency with the codebase convention.

-import com.mifos.feature.client.newFixedDepositAccount.CreateFixedDepositAccountViewmodel
+import com.mifos.feature.client.newFixedDepositAccount.CreateFixedDepositAccountViewModel
-    viewModelOf(::CreateFixedDepositAccountViewmodel)
+    viewModelOf(::CreateFixedDepositAccountViewModel)

Also applies to: 87-87

core/database/src/commonMain/kotlin/com/mifos/room/basemodel/APIEndPoint.kt (2)

44-45: Remove extra blank line.

There's an unnecessary blank line on line 44 before the new constant declaration.

-
 const val FIXED_DEPOSIT ="fixeddepositaccounts"

45-45: Missing space after equals sign.

There should be a space after the = for consistency with other constant declarations in this file.

-    const val FIXED_DEPOSIT ="fixeddepositaccounts"
+    const val FIXED_DEPOSIT = "fixeddepositaccounts"
core/data/src/commonMain/kotlin/com/mifos/core/data/repository/FixedDepositRepository.kt (1)

13-14: Remove extra blank lines.

There are unnecessary blank lines at the end of the file.

     ): Flow<DataState<FixedDepositTemplate>>
-
-
 }
core/network/src/commonMain/kotlin/com/mifos/core/network/datamanager/DataManagerFixedDeposit.kt (2)

7-7: Fix formatting: remove space before parenthesis and add space before brace.

The class declaration has inconsistent spacing. Per Kotlin conventions, there should be no space before the constructor parenthesis and a space before the opening brace.

Apply this diff:

-class DataManagerFixedDeposit (private val baseApiManager: BaseApiManager){
+class DataManagerFixedDeposit(private val baseApiManager: BaseApiManager) {

9-10: Fix spacing in method signature and call.

Missing spaces after colons and commas per Kotlin conventions.

Apply this diff:

-    fun getFixedDepositTemplate(clientId:Int,productId: Int?): Flow<FixedDepositTemplate> =
-        baseApiManager.fixedDepositService.fixedDepositProductTemplate(clientId,productId)
+    fun getFixedDepositTemplate(clientId: Int, productId: Int?): Flow<FixedDepositTemplate> =
+        baseApiManager.fixedDepositService.fixedDepositProductTemplate(clientId, productId)
core/network/src/commonMain/kotlin/com/mifos/core/network/services/FixedDepositService.kt (1)

9-13: Consider removing excessive blank lines.

The file contains multiple consecutive blank lines (5 and 4 respectively) which are unnecessary. Kotlin style typically uses at most 1-2 blank lines between top-level declarations.

Also applies to: 22-26

core/network/src/commonMain/kotlin/com/mifos/core/network/model/FixedDepositTemplate.kt (1)

4-4: Remove unused import.

The IgnoredOnParcel import is not used in this file. There's no @Parcelize annotation or usage of the import.

Apply this diff:

 import com.mifos.core.model.objects.account.saving.FieldOfficerOptions
-import com.mifos.core.model.utils.IgnoredOnParcel
 import kotlinx.serialization.SerialName
feature/client/src/commonMain/kotlin/com/mifos/feature/client/newFixedDepositAccount/createFixedDepositAccountRoute.kt (2)

19-21: Follow Kotlin naming conventions for class names.

The data class name createFixedDepositAccountRoute uses camelCase, but Kotlin convention dictates PascalCase for class names. Consider renaming to CreateFixedDepositAccountRoute.

Apply this diff:

-data class createFixedDepositAccountRoute(
+data class CreateFixedDepositAccountRoute(
     val clientId: Int,
 )

And update all references throughout the codebase accordingly.


34-38: Consider adding space around equals sign in named argument.

Line 36 has clientId=clientId without spaces around the equals sign. Kotlin convention typically uses spaces: clientId = clientId.

Apply this diff:

     this.navigate(
-        createFixedDepositAccountRoute(clientId=clientId)
+        createFixedDepositAccountRoute(clientId = clientId)
     )
core/data/src/commonMain/kotlin/com/mifos/core/data/repositoryImp/FixedDepositRepositoryImpl.kt (1)

11-15: Repository delegation is correct; consider minor idiomatic cleanup

The implementation cleanly delegates to DataManagerFixedDeposit and exposes a Flow<DataState<FixedDepositTemplate>>, which matches the repository contract.

If you want to make it a bit more idiomatic, you could use an expression body and normalize spacing:

-class FixedDepositRepositoryImpl (private val dataManagerFixedDeposit: DataManagerFixedDeposit): FixedDepositRepository{
-
-    override fun getFixedDepositTemplate(clientId: Int,productId: Int?): Flow<DataState<FixedDepositTemplate>> {
-        return dataManagerFixedDeposit.getFixedDepositTemplate(clientId,productId).asDataStateFlow()
-    }
-}
+class FixedDepositRepositoryImpl(
+    private val dataManagerFixedDeposit: DataManagerFixedDeposit,
+) : FixedDepositRepository {
+
+    override fun getFixedDepositTemplate(
+        clientId: Int,
+        productId: Int?,
+    ): Flow<DataState<FixedDepositTemplate>> =
+        dataManagerFixedDeposit.getFixedDepositTemplate(clientId, productId).asDataStateFlow()
+}
feature/client/src/commonMain/kotlin/com/mifos/feature/client/newFixedDepositAccount/createFixedDepositAccountScreen.kt (1)

72-101: Use scaffold content padding to avoid overlapping UI

Inside MifosScaffold’s content lambda, paddingValues is currently ignored:

MifosScaffold(
    ...
) { paddingValues ->
    Column {
        MifosBreadcrumbNavBar(navController)
        when (newFixedDepositAccountState.screenState) { ... }
    }
}

To avoid content rendering under the app bar or system insets (and to keep this consistent with other screens), consider applying the padding to the Column:

-    ) { paddingValues ->
-        Column {
+    ) { paddingValues ->
+        Column(modifier = Modifier.padding(paddingValues)) {
             MifosBreadcrumbNavBar(navController)
             when (newFixedDepositAccountState.screenState) {
                 is NewFixedDepositAccountState.ScreenState.Error -> { ... }
                 is NewFixedDepositAccountState.ScreenState.Loading -> { ... }
                 is NewFixedDepositAccountState.ScreenState.Success -> {
                     MifosStepper(
                         steps = steps,
                         currentIndex = newFixedDepositAccountState.currentStep,
                         onStepChange = { newIndex ->
                             onAction(NewFixedDepositAccountAction.OnStepChange(newIndex))
                         },
                         modifier = Modifier.fillMaxWidth(),
                     )
                 }
             }
         }
     }

Also applies to: 108-137

feature/client/src/commonMain/kotlin/com/mifos/feature/client/newFixedDepositAccount/createFixedDepositAccountViewmodel.kt (3)

68-93: Make template load updates atomic and guard product index access

Two small robustness improvements here:

  1. Unify state updates in the Success branch

You currently set the screen state and then update the template in two separate update calls:

is DataState.Success -> {
    setSuccessState()
    mutableStateFlow.update {
        it.copy(
            template = state.data,
        )
    }
}

To avoid extra recompositions and ensure atomic updates, consider doing this in a single update:

-                is DataState.Success -> {
-                    setSuccessState()
-                    mutableStateFlow.update {
-                        it.copy(
-                            template = state.data,
-                        )
-                    }
-                }
+                is DataState.Success -> {
+                    mutableStateFlow.update {
+                        it.copy(
+                            template = state.data,
+                            screenState = NewFixedDepositAccountState.ScreenState.Success,
+                        )
+                    }
+                }

(You can then drop setSuccessState() or keep it only where it’s actually shared.)

  1. Safer lookup of productId

productId is derived via:

productId = state.template.productOptions
    ?.get(state.fixedDepositAccountDetail.productSelected)
    ?.id

If productOptions is non‑null while productSelected is still -1 or out of range, this will throw. Using getOrNull (and treating “no product selected” as null) makes this more defensive:

-        fixedDepositRepository.getFixedDepositTemplate(
-            clientId = state.clientId,
-            productId = state.template.productOptions
-                ?.get(state.fixedDepositAccountDetail.productSelected)?.id,
-        )
+        fixedDepositRepository.getFixedDepositTemplate(
+            clientId = state.clientId,
+            productId = state.template.productOptions
+                ?.getOrNull(state.fixedDepositAccountDetail.productSelected)
+                ?.id,
+        )

181-192: Clarify totalSteps semantics to match the UI step list

moveToNextStep uses totalSteps as an upper bound:

private fun moveToNextStep() {
    val current = state.currentStep
    if (current < state.totalSteps) {
        mutableStateFlow.update {
            it.copy(
                currentStep = current + 1,
            )
        }
    } else {
        sendEvent(NewFixedDepositAccountEvent.Finish)
    }
}

and NewFixedDepositAccountState sets:

val totalSteps: Int = 4,

While this works with the current 5‑item steps list in the screen (indices 0–4), totalSteps is effectively “last step index” rather than “number of steps”. That’s easy to misinterpret and forget to adjust when the UI steps change.

Consider one of the following to make this more self‑documenting:

  • Rename the property to something like lastStepIndex, or
  • Store the actual count (e.g., steps.size) and change the condition to current < totalSteps - 1.

Either approach will make future step changes less error‑prone.

Also applies to: 197-205


213-229: Revisit isDetailsNextEnabled when no field officer is required

isDetailsNextEnabled is currently:

val isDetailsNextEnabled = submissionDate.isNotEmpty() && fieldOfficerIndex != -1

In DetailsPage, the field officer UI is only shown if state.template.fieldOfficerOptions is not empty, but the “Next” button is always bound to isDetailsNextEnabled. For templates where fieldOfficerOptions is empty, fieldOfficerIndex will remain -1 and the Next button will stay disabled with no way for the user to proceed.

It may be safer to enable Next whenever there are no field officer options, e.g.:

data class FixedDepositAccountDetailsState @OptIn(ExperimentalTime::class) constructor(
    ...
-    val fieldOfficerOptions: List<FieldOfficerOption>? = null,
-) {
-    val isDetailsNextEnabled = submissionDate.isNotEmpty() && fieldOfficerIndex != -1
+    val fieldOfficerOptions: List<FieldOfficerOption>? = null,
+) {
+    val isDetailsNextEnabled =
+        submissionDate.isNotEmpty() &&
+            (fieldOfficerOptions.isNullOrEmpty() || fieldOfficerIndex != -1)
}

You’d then want to set fieldOfficerOptions from the loaded template, or compute this logic directly in DetailsPage using state.template.fieldOfficerOptions.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 87acaa5 and 8b99dfb.

📒 Files selected for processing (21)
  • core/data/src/commonMain/kotlin/com/mifos/core/data/di/RepositoryModule.kt (3 hunks)
  • core/data/src/commonMain/kotlin/com/mifos/core/data/repository/FixedDepositRepository.kt (1 hunks)
  • core/data/src/commonMain/kotlin/com/mifos/core/data/repositoryImp/FixedDepositRepositoryImpl.kt (1 hunks)
  • core/database/src/commonMain/kotlin/com/mifos/room/basemodel/APIEndPoint.kt (1 hunks)
  • core/database/src/commonMain/kotlin/com/mifos/room/entities/accounts/savings/SavingAccountDepositTypeEntity.kt (1 hunks)
  • core/network/src/commonMain/kotlin/com/mifos/core/network/BaseApiManager.kt (3 hunks)
  • core/network/src/commonMain/kotlin/com/mifos/core/network/datamanager/DataManagerFixedDeposit.kt (1 hunks)
  • core/network/src/commonMain/kotlin/com/mifos/core/network/di/DataMangerModule.kt (2 hunks)
  • core/network/src/commonMain/kotlin/com/mifos/core/network/model/FixedDepositProductOption.kt (1 hunks)
  • core/network/src/commonMain/kotlin/com/mifos/core/network/model/FixedDepositTemplate.kt (1 hunks)
  • core/network/src/commonMain/kotlin/com/mifos/core/network/services/FixedDepositService.kt (1 hunks)
  • feature/client/src/commonMain/composeResources/values/strings.xml (1 hunks)
  • feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientApplyNewApplications/ClientApplyNewApplicationRoute.kt (1 hunks)
  • feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientApplyNewApplications/ClientApplyNewApplicationsScreen.kt (2 hunks)
  • feature/client/src/commonMain/kotlin/com/mifos/feature/client/di/ClientModule.kt (2 hunks)
  • feature/client/src/commonMain/kotlin/com/mifos/feature/client/navigation/ClientNavigation.kt (3 hunks)
  • feature/client/src/commonMain/kotlin/com/mifos/feature/client/newFixedDepositAccount/FixedDepositAccountViewmodel.kt (0 hunks)
  • feature/client/src/commonMain/kotlin/com/mifos/feature/client/newFixedDepositAccount/createFixedDepositAccountRoute.kt (1 hunks)
  • feature/client/src/commonMain/kotlin/com/mifos/feature/client/newFixedDepositAccount/createFixedDepositAccountScreen.kt (4 hunks)
  • feature/client/src/commonMain/kotlin/com/mifos/feature/client/newFixedDepositAccount/createFixedDepositAccountViewmodel.kt (1 hunks)
  • feature/client/src/commonMain/kotlin/com/mifos/feature/client/newFixedDepositAccount/pages/DetailsPage.kt (1 hunks)
💤 Files with no reviewable changes (1)
  • feature/client/src/commonMain/kotlin/com/mifos/feature/client/newFixedDepositAccount/FixedDepositAccountViewmodel.kt
🧰 Additional context used
🧬 Code graph analysis (4)
feature/client/src/commonMain/kotlin/com/mifos/feature/client/newFixedDepositAccount/createFixedDepositAccountRoute.kt (1)
feature/client/src/commonMain/kotlin/com/mifos/feature/client/newFixedDepositAccount/createFixedDepositAccountScreen.kt (1)
  • CreateFixedDepositAccountScreen (42-63)
feature/client/src/commonMain/kotlin/com/mifos/feature/client/newFixedDepositAccount/pages/DetailsPage.kt (3)
core/designsystem/src/commonMain/kotlin/com/mifos/core/designsystem/component/MifosTextFieldDropdown.kt (1)
  • MifosTextFieldDropdown (39-112)
core/designsystem/src/commonMain/kotlin/com/mifos/core/designsystem/component/MifosOutlinedTextField.kt (1)
  • MifosDatePickerTextField (363-413)
core/ui/src/commonMain/kotlin/com/mifos/core/ui/components/MifosTwoButtonRow.kt (1)
  • MifosTwoButtonRow (31-91)
feature/client/src/commonMain/kotlin/com/mifos/feature/client/navigation/ClientNavigation.kt (1)
feature/client/src/commonMain/kotlin/com/mifos/feature/client/newFixedDepositAccount/createFixedDepositAccountRoute.kt (1)
  • createFixedDepositAccountDestination (25-33)
feature/client/src/commonMain/kotlin/com/mifos/feature/client/newFixedDepositAccount/createFixedDepositAccountScreen.kt (5)
feature/client/src/commonMain/kotlin/com/mifos/feature/client/newFixedDepositAccount/pages/DetailsPage.kt (1)
  • DetailsPage (56-176)
core/ui/src/commonMain/kotlin/com/mifos/core/ui/components/MifosBreadCrumb.kt (1)
  • MifosBreadcrumbNavBar (36-107)
core/ui/src/commonMain/kotlin/com/mifos/core/ui/components/MifosErrorComponent.kt (1)
  • MifosErrorComponent (41-60)
core/ui/src/commonMain/kotlin/com/mifos/core/ui/components/MifosProgressIndicator.kt (1)
  • MifosProgressIndicator (41-68)
core/ui/src/commonMain/kotlin/com/mifos/core/ui/components/MifosStepper.kt (1)
  • MifosStepper (47-139)
🔇 Additional comments (19)
feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientApplyNewApplications/ClientApplyNewApplicationRoute.kt (1)

29-29: LGTM! Navigation callback signature now consistent.

The change from onNavigateApplyFixedAccount: () -> Unit to onNavigateApplyFixedAccount: (Int) -> Unit aligns the Fixed Account navigation with the pattern used by other account types (Loan, Savings, Share, and Recurring), all of which accept an Int parameter for clientId.

feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientApplyNewApplications/ClientApplyNewApplicationsScreen.kt (2)

66-66: LGTM! Parameter signature updated correctly.

The function signature now matches the updated route definition and is consistent with other account type navigation callbacks.


77-77: LGTM! Call site updated to pass clientId.

The navigation now correctly passes state.clientId when navigating to the Fixed Account flow, consistent with the other account type navigation calls.

core/network/src/commonMain/kotlin/com/mifos/core/network/BaseApiManager.kt (1)

47-47: LGTM! Fixed Deposit service registered correctly.

The fixedDepositService is properly initialized using the Ktorfit factory method and follows the established pattern for service registration in this class. The placement between savingsService and recurringSavingsAccountService is logical.

Also applies to: 63-63, 81-81

core/data/src/commonMain/kotlin/com/mifos/core/data/repository/FixedDepositRepository.kt (1)

7-11: LGTM! Clean repository interface design.

The interface follows the repository pattern correctly with a Flow-based async API wrapped in DataState for state management. The nullable productId parameter allows optional product filtering.

core/network/src/commonMain/kotlin/com/mifos/core/network/di/DataMangerModule.kt (1)

24-24: LGTM! Data manager registered correctly.

The DataManagerFixedDeposit is properly registered as a singleton with dependency injection, following the established pattern for other data managers in this module.

Also applies to: 59-59

core/database/src/commonMain/kotlin/com/mifos/room/entities/accounts/savings/SavingAccountDepositTypeEntity.kt (1)

47-47: The code change is correctly implemented, but backend API support and test coverage require manual verification.

The endpoint mapping is semantically correct: FIXED(200, "depositAccountType.fixedDeposit", APIEndPoint.FIXED_DEPOSIT) properly routes to "fixeddepositaccounts" instead of "savingsaccounts". All consuming code (FixedDepositAccountViewModel, sync dialogs across client/group/center features) will automatically use the new endpoint through the dynamic property.

However, no test files were found in the codebase for fixed deposit operations, and backend API support for the fixeddepositaccounts endpoint cannot be verified from the repository alone. The change is a breaking change if the backend doesn't support this endpoint.

core/data/src/commonMain/kotlin/com/mifos/core/data/di/RepositoryModule.kt (1)

32-32: LGTM!

The DI binding for FixedDepositRepository is correctly implemented and follows the established pattern in this module. The imports are alphabetically ordered and the binding is placed logically.

Also applies to: 95-95, 228-228

core/network/src/commonMain/kotlin/com/mifos/core/network/model/FixedDepositProductOption.kt (1)

7-17: LGTM!

The data class is properly structured with appropriate kotlinx.serialization annotations. Nullable properties with default null values are suitable for optional API response fields.

core/network/src/commonMain/kotlin/com/mifos/core/network/services/FixedDepositService.kt (1)

14-21: LGTM!

The service interface is correctly defined with appropriate Ktorfit annotations. The productId parameter is properly nullable for optional filtering.

core/network/src/commonMain/kotlin/com/mifos/core/network/model/FixedDepositTemplate.kt (1)

10-24: LGTM!

The data class structure is well-defined with appropriate serialization annotations and nullable properties for API response handling.

feature/client/src/commonMain/kotlin/com/mifos/feature/client/newFixedDepositAccount/pages/DetailsPage.kt (4)

56-72: LGTM!

The date picker state is properly configured with a reasonable constraint allowing dates from yesterday onwards, which handles timezone edge cases appropriately.


73-103: LGTM!

The date picker dialog is properly implemented with correct state management and date formatting using DateHelper.


166-174: LGTM!

The button row is properly implemented with state-driven enable/disable logic for the Next button.


127-165: Code snippet in review comment does not match actual codebase—externalId fields are named n and nError.

The review comment shows state.fixedDepositAccountDetail.externalId and externalIdError, but the actual code uses state.fixedDepositAccountDetail.n and nError. This naming inconsistency should be corrected.

Regarding the conditional rendering: The logic is intentional. Form completion requires both a non-empty submission date AND a selected field officer (isDetailsNextEnabled = submissionDate.isNotEmpty() && fieldOfficerIndex != -1). This same pattern is consistently applied in the Recurring Deposit flow, indicating that disabling the details section when no field officers exist is by design.

Likely an incorrect or invalid review comment.

feature/client/src/commonMain/kotlin/com/mifos/feature/client/navigation/ClientNavigation.kt (2)

79-81: LGTM!

The import updates align with the refactored type-safe navigation pattern for Fixed Deposit account creation.


327-327: Navigation callback is correctly wired and clientId is passed.

The verification confirms the implementation is correct:

  • onNavigateApplyFixedAccount: (Int) -> Unit callback signature matches the method reference navController::navigateToCreateFixedDepositRoute(clientId: Int)
  • At the invocation site in ClientApplyNewApplicationsScreen.kt line 77, state.clientId is passed to the callback
  • The clientId flows correctly through the entire navigation chain
feature/client/src/commonMain/kotlin/com/mifos/feature/client/newFixedDepositAccount/createFixedDepositAccountRoute.kt (1)

25-33: LGTM!

The destination function properly wires the screen with navigation callbacks. The empty onFinish placeholder is acceptable for the current implementation.

feature/client/src/commonMain/kotlin/com/mifos/feature/client/newFixedDepositAccount/createFixedDepositAccountScreen.kt (1)

42-63: Screen wiring with ViewModel and events looks consistent

CreateFixedDepositAccountScreen correctly collects stateFlow, reacts to NewFixedDepositAccountEvent, and passes state and onAction down into the scaffold. The NavController parameter and koinViewModel() default are wired in a straightforward way and match the rest of the flow.

@@ -0,0 +1,14 @@
package com.mifos.core.data.repository
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Missing copyright header.

This file is missing the standard Mifos Initiative copyright and license header that is present in all other files in this PR. Please add the MPL 2.0 license header for consistency.

Add the copyright header at the beginning of the file:

+/*
+ * Copyright 2025 Mifos Initiative
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at https://mozilla.org/MPL/2.0/.
+ *
+ * See https://github.com/openMF/android-client/blob/master/LICENSE.md
+ */
 package com.mifos.core.data.repository
🤖 Prompt for AI Agents
In
core/data/src/commonMain/kotlin/com/mifos/core/data/repository/FixedDepositRepository.kt
around lines 1 to 1, the file is missing the project-standard MPL 2.0 copyright
and license header; add the same multi-line header block used across the repo
(Mifos Initiative copyright, SPDX identifier and MPL 2.0 text) at the very top
of the file before the package statement so the file matches the other source
files' licensing header.

@therajanmaurya therajanmaurya merged commit f22b0d4 into openMF:development Nov 17, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants