fix: address CodeRabbit review comments

sahilshivekar · sahilshivekar · commit 06875c6246bd · 2026-07-08T19:48:24.000+05:30
diff --git a/core/data/src/commonMain/kotlin/com/mifos/core/data/mappers/loan/GuarantorMapper.kt b/core/data/src/commonMain/kotlin/com/mifos/core/data/mappers/loan/GuarantorMapper.kt
@@ -9,13 +9,13 @@
  */
 package com.mifos.core.data.mappers.loan
 
+import com.mifos.core.model.objects.account.loan.guarantor.CreateGuarantor
 import com.mifos.core.model.objects.account.loan.guarantor.CreateGuarantorInput
-import com.mifos.core.model.objects.account.loan.guarantor.CreateGuarantorResponseDto
-import com.mifos.core.model.objects.account.loan.guarantor.CreatedGuarantor
 import com.mifos.core.model.objects.account.loan.guarantor.GuarantorAccountTemplate
 import com.mifos.core.model.objects.account.loan.guarantor.GuarantorRelationshipOption
 import com.mifos.core.model.objects.account.loan.guarantor.GuarantorTemplate
 import com.mifos.core.model.objects.account.loan.guarantor.GuarantorType
+import com.mifos.core.network.dto.loans.CreateGuarantorResponseDto
 import com.mifos.core.network.dto.loans.GuarantorAccountTemplateDto
 import com.mifos.core.network.dto.loans.GuarantorRelationshipOptionDto
 import com.mifos.core.network.dto.loans.GuarantorRequestDto
@@ -33,8 +33,8 @@ fun GuarantorRelationshipOptionDto.toDomain(): GuarantorRelationshipOption =
         name = name,
     )
 
-fun CreateGuarantorResponseDto.toDomain(): CreatedGuarantor =
-    CreatedGuarantor(
+fun CreateGuarantorResponseDto.toDomain(): CreateGuarantor =
+    CreateGuarantor(
         resourceId = resourceId,
         loanId = loanId,
         officeId = officeId,
diff --git a/core/data/src/commonMain/kotlin/com/mifos/core/data/repository/LoanCreateGuarantorRepository.kt b/core/data/src/commonMain/kotlin/com/mifos/core/data/repository/LoanCreateGuarantorRepository.kt
@@ -10,8 +10,8 @@
 package com.mifos.core.data.repository
 
 import com.mifos.core.common.utils.DataState
+import com.mifos.core.model.objects.account.loan.guarantor.CreateGuarantor
 import com.mifos.core.model.objects.account.loan.guarantor.CreateGuarantorInput
-import com.mifos.core.model.objects.account.loan.guarantor.CreatedGuarantor
 import com.mifos.core.model.objects.account.loan.guarantor.GuarantorAccountTemplate
 import com.mifos.core.model.objects.account.loan.guarantor.GuarantorTemplate
 
@@ -22,7 +22,7 @@ interface LoanCreateGuarantorRepository {
     suspend fun createGuarantor(
         loanId: Int,
         createGuarantorInput: CreateGuarantorInput,
-    ): DataState<CreatedGuarantor>
+    ): DataState<CreateGuarantor>
 
     suspend fun getGuarantorAccountTemplate(
         loanId: Int,
diff --git a/core/data/src/commonMain/kotlin/com/mifos/core/data/repositoryImp/LoanCreateGuarantorRepositoryImp.kt b/core/data/src/commonMain/kotlin/com/mifos/core/data/repositoryImp/LoanCreateGuarantorRepositoryImp.kt
@@ -15,8 +15,8 @@ import com.mifos.core.data.mappers.loan.toDto
 import com.mifos.core.data.repository.LoanCreateGuarantorRepository
 import com.mifos.core.data.util.NetworkMonitor
 import com.mifos.core.data.util.runAsDataState
+import com.mifos.core.model.objects.account.loan.guarantor.CreateGuarantor
 import com.mifos.core.model.objects.account.loan.guarantor.CreateGuarantorInput
-import com.mifos.core.model.objects.account.loan.guarantor.CreatedGuarantor
 import com.mifos.core.model.objects.account.loan.guarantor.GuarantorAccountTemplate
 import com.mifos.core.model.objects.account.loan.guarantor.GuarantorTemplate
 import com.mifos.core.network.datamanager.DataManagerLoan
@@ -40,7 +40,7 @@ class LoanCreateGuarantorRepositoryImp(
     override suspend fun createGuarantor(
         loanId: Int,
         createGuarantorInput: CreateGuarantorInput,
-    ): DataState<CreatedGuarantor> {
+    ): DataState<CreateGuarantor> {
         return runAsDataState(
             networkMonitor,
             ioDispatcher,
diff --git a/core/domain/src/commonMain/kotlin/com/mifos/core/domain/di/UseCaseModule.kt b/core/domain/src/commonMain/kotlin/com/mifos/core/domain/di/UseCaseModule.kt
@@ -90,6 +90,7 @@ import com.mifos.core.domain.useCases.ValidateServerPortUseCase
 import com.mifos.core.domain.useCases.ValidateServerProtocolUseCase
 import com.mifos.core.domain.useCases.ValidateServerTenantUseCase
 import com.mifos.core.domain.useCases.createGuarantor.CreateGuarantorUseCase
+import com.mifos.core.domain.useCases.createGuarantor.GetGuarantorAccountTemplateUseCase
 import com.mifos.core.domain.useCases.createGuarantor.GetGuarantorTemplateUseCase
 import com.mifos.core.domain.useCases.loanChargeOff.GetLoanChargeOffTemplateUseCase
 import com.mifos.core.domain.useCases.loanChargeOff.LoanChargeOffUseCase
@@ -107,6 +108,7 @@ val UseCaseModule = module {
     factoryOf(::ApproveSavingsApplicationUseCase)
     factoryOf(::CreateChargesUseCase)
     factoryOf(::CreateGuarantorUseCase)
+    factoryOf(::GetGuarantorAccountTemplateUseCase)
     factoryOf(::CreateClientIdentifierUseCase)
     factoryOf(::CalculateLoanScheduleUseCase)
     factoryOf(::CreateGroupLoansAccountUseCase)
diff --git a/core/domain/src/commonMain/kotlin/com/mifos/core/domain/useCases/createGuarantor/CreateGuarantorUseCase.kt b/core/domain/src/commonMain/kotlin/com/mifos/core/domain/useCases/createGuarantor/CreateGuarantorUseCase.kt
@@ -11,14 +11,14 @@ package com.mifos.core.domain.useCases.createGuarantor
 
 import com.mifos.core.common.utils.DataState
 import com.mifos.core.data.repository.LoanCreateGuarantorRepository
+import com.mifos.core.model.objects.account.loan.guarantor.CreateGuarantor
 import com.mifos.core.model.objects.account.loan.guarantor.CreateGuarantorInput
-import com.mifos.core.model.objects.account.loan.guarantor.CreatedGuarantor
 
 class CreateGuarantorUseCase(
     private val repository: LoanCreateGuarantorRepository,
 ) {
     suspend operator fun invoke(
         loanId: Int,
         createGuarantorInput: CreateGuarantorInput,
-    ): DataState<CreatedGuarantor> = repository.createGuarantor(loanId, createGuarantorInput)
+    ): DataState<CreateGuarantor> = repository.createGuarantor(loanId, createGuarantorInput)
 }
diff --git a/core/model/src/commonMain/kotlin/com/mifos/core/model/objects/account/loan/guarantor/CreateGuarantor.kt b/core/model/src/commonMain/kotlin/com/mifos/core/model/objects/account/loan/guarantor/CreateGuarantor.kt
@@ -9,7 +9,7 @@
  */
 package com.mifos.core.model.objects.account.loan.guarantor
 
-data class CreatedGuarantor(
+data class CreateGuarantor(
     val officeId: Long,
     val loanId: Long,
     val resourceId: Long,
diff --git a/core/model/src/commonMain/kotlin/com/mifos/core/model/objects/account/loan/guarantor/CreateGuarantorInput.kt b/core/model/src/commonMain/kotlin/com/mifos/core/model/objects/account/loan/guarantor/CreateGuarantorInput.kt
@@ -15,7 +15,7 @@ data class CreateGuarantorInput(
     val clientRelationshipTypeId: Long,
     val dateFormat: String = "dd-MM-yyyy",
     val entityId: Int? = null,
-    val guarantorTypeId: Long,
+    val guarantorTypeId: Long?,
     val locale: String = DateConstants.LOCALE,
     val firstname: String? = null,
     val lastname: String? = null,
diff --git a/core/network/src/commonMain/kotlin/com/mifos/core/network/datamanager/DataManagerLoan.kt b/core/network/src/commonMain/kotlin/com/mifos/core/network/datamanager/DataManagerLoan.kt
@@ -13,7 +13,6 @@ import com.mifos.core.common.utils.extractErrorMessage
 import com.mifos.core.datastore.UserPreferencesRepository
 import com.mifos.core.model.objects.account.loan.LoanDisbursement
 import com.mifos.core.model.objects.account.loan.RepaymentSchedule
-import com.mifos.core.model.objects.account.loan.guarantor.CreateGuarantorResponseDto
 import com.mifos.core.model.objects.account.loan.reschedules.LoanRescheduleApprovalRequest
 import com.mifos.core.model.objects.account.loan.reschedules.LoanRescheduleRejectionRequest
 import com.mifos.core.model.objects.account.loan.reschedules.LoanRescheduleRequest
@@ -24,6 +23,7 @@ import com.mifos.core.model.objects.account.loan.transfer.AccountTransferRespons
 import com.mifos.core.model.objects.account.loan.transfer.AccountTransferTemplate
 import com.mifos.core.network.BaseApiManager
 import com.mifos.core.network.GenericResponse
+import com.mifos.core.network.dto.loans.CreateGuarantorResponseDto
 import com.mifos.core.network.dto.loans.GuarantorAccountTemplateDto
 import com.mifos.core.network.dto.loans.GuarantorRequestDto
 import com.mifos.core.network.dto.loans.GuarantorTemplateDto
diff --git a/core/network/src/commonMain/kotlin/com/mifos/core/network/dto/loans/CreateGuarantorResponseDto.kt b/core/network/src/commonMain/kotlin/com/mifos/core/network/dto/loans/CreateGuarantorResponseDto.kt
@@ -7,7 +7,7 @@
  *
  * See https://github.com/openMF/mifos-x-field-officer-app/blob/master/LICENSE.md
  */
-package com.mifos.core.model.objects.account.loan.guarantor
+package com.mifos.core.network.dto.loans
 
 import kotlinx.serialization.Serializable
 
diff --git a/core/network/src/commonMain/kotlin/com/mifos/core/network/dto/loans/GuarantorRequestDto.kt b/core/network/src/commonMain/kotlin/com/mifos/core/network/dto/loans/GuarantorRequestDto.kt
@@ -16,7 +16,7 @@ data class GuarantorRequestDto(
     val clientRelationshipTypeId: Long,
     val dateFormat: String,
     val entityId: Int?,
-    val guarantorTypeId: Long,
+    val guarantorTypeId: Long?,
     val locale: String,
 
     val firstname: String?,
diff --git a/core/network/src/commonMain/kotlin/com/mifos/core/network/services/LoanService.kt b/core/network/src/commonMain/kotlin/com/mifos/core/network/services/LoanService.kt
@@ -11,7 +11,6 @@ package com.mifos.core.network.services
 
 import com.mifos.core.model.objects.account.loan.LoanApproval
 import com.mifos.core.model.objects.account.loan.LoanDisbursement
-import com.mifos.core.model.objects.account.loan.guarantor.CreateGuarantorResponseDto
 import com.mifos.core.model.objects.account.loan.reschedules.LoanRescheduleApprovalRequest
 import com.mifos.core.model.objects.account.loan.reschedules.LoanRescheduleRejectionRequest
 import com.mifos.core.model.objects.account.loan.reschedules.LoanRescheduleRequest
@@ -24,6 +23,7 @@ import com.mifos.core.model.objects.organisations.LoanProducts
 import com.mifos.core.model.objects.payloads.GroupLoanPayload
 import com.mifos.core.model.objects.template.loan.GroupLoanTemplate
 import com.mifos.core.network.GenericResponse
+import com.mifos.core.network.dto.loans.CreateGuarantorResponseDto
 import com.mifos.core.network.dto.loans.GuarantorAccountTemplateDto
 import com.mifos.core.network.dto.loans.GuarantorRequestDto
 import com.mifos.core.network.dto.loans.GuarantorTemplateDto
diff --git a/feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/createGuarantor/CreateGuarantorScreen.kt b/feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/createGuarantor/CreateGuarantorScreen.kt
@@ -80,11 +80,12 @@ internal fun CreateGuarantorScreenRoute(
     LaunchedEffect(Unit) {
         viewModel.eventFlow.collect { effect ->
             when (effect) {
-                is CreateGuarantorEffect.ShowMessage -> {
+                is CreateGuarantorEvent.ShowMessage -> {
                     val resolvedMessage = getString(effect.message)
                     snackbarHostState.showSnackbar(resolvedMessage)
                 }
-                CreateGuarantorEffect.NavigateBack -> navigateBack()
+
+                CreateGuarantorEvent.NavigateBack -> navigateBack()
             }
         }
     }
@@ -112,7 +113,6 @@ internal fun CreateGuarantorScreenRoute(
                     state = uiState,
                     modifier = Modifier.padding(paddingValues),
                     onAction = viewModel::trySendAction,
-                    navigateBack = navigateBack,
                 )
             }
         }
@@ -122,21 +122,12 @@ internal fun CreateGuarantorScreenRoute(
 @OptIn(ExperimentalMaterial3Api::class)
 @Composable
 private fun CreateGuarantorContent(
-    state: CreateGuarantorState,
     modifier: Modifier = Modifier,
+    state: CreateGuarantorState,
     onAction: (CreateGuarantorAction) -> Unit,
-    navigateBack: () -> Unit,
 ) {
     var showDatePicker by rememberSaveable { mutableStateOf(false) }
 
-    // Derive submit enablement logic natively in the UI
-    val isRelationshipSelected = state.selectedRelationshipIndex != -1
-    val canSubmit = isRelationshipSelected && if (state.existingClient) {
-        state.selectedClientId != null
-    } else {
-        state.firstName.isNotBlank() && state.lastName.isNotBlank()
-    }
-
     Box(
         modifier = Modifier.fillMaxSize(),
     ) {
@@ -207,7 +198,8 @@ private fun CreateGuarantorContent(
                 )
                 Spacer(Modifier.height(DesignToken.padding.medium))
                 MifosDatePickerTextField(
-                    value = state.dateOfBirthMillis?.let(DateHelper::getDateAsStringFromLong).orEmpty(),
+                    value = state.dateOfBirthMillis?.let(DateHelper::getDateAsStringFromLong)
+                        .orEmpty(),
                     label = stringResource(Res.string.feature_loan_create_guarantor_date_of_birth),
                     openDatePicker = { showDatePicker = true },
                 )
@@ -254,48 +246,69 @@ private fun CreateGuarantorContent(
             MifosTwoButtonRow(
                 firstBtnText = stringResource(Res.string.feature_loan_cancel),
                 secondBtnText = stringResource(Res.string.feature_loan_submit),
-                onFirstBtnClick = { if (!state.submitInProgress) navigateBack() },
+                onFirstBtnClick = { if (!state.submitInProgress) onAction(CreateGuarantorAction.NavigateBack) },
                 onSecondBtnClick = { onAction(CreateGuarantorAction.Submit) },
                 isButtonIconVisible = false,
-                isSecondButtonEnabled = canSubmit && !state.submitInProgress,
+                isSecondButtonEnabled = state.canSubmit && !state.submitInProgress,
                 modifier = Modifier.fillMaxWidth(),
             )
 
             Spacer(Modifier.height(DesignToken.padding.large))
         }
 
         if (showDatePicker) {
-            val datePickerState = rememberDatePickerState(
-                initialSelectedDateMillis = state.dateOfBirthMillis,
-                selectableDates = object : SelectableDates {
-                    override fun isSelectableDate(utcTimeMillis: Long): Boolean {
-                        return utcTimeMillis <= Clock.System.now().toEpochMilliseconds()
-                    }
+            CreateGuarantorDatePickerDialog(
+                initialDateMillis = state.dateOfBirthMillis,
+                onConfirm = { dateMillis ->
+                    onAction(CreateGuarantorAction.UpdateDateOfBirth(dateMillis))
                 },
+                onDismiss = { showDatePicker = false },
             )
-            DatePickerDialog(
-                onDismissRequest = { showDatePicker = false },
-                confirmButton = {
-                    TextButton(
-                        onClick = {
-                            onAction(CreateGuarantorAction.UpdateDateOfBirth(datePickerState.selectedDateMillis))
-                            showDatePicker = false
-                        },
-                    ) {
-                        Text(stringResource(Res.string.feature_loan_select_date))
-                    }
-                },
-                dismissButton = {
-                    TextButton(onClick = { showDatePicker = false }) {
-                        Text(stringResource(Res.string.feature_loan_cancel))
-                    }
-                },
-            ) {
-                DatePicker(state = datePickerState)
-            }
         }
         if (state.submitInProgress || state.fetchingGuarantorAccountTemplate) {
             MifosProgressIndicatorOverlay()
         }
     }
 }
+
+@OptIn(ExperimentalMaterial3Api::class)
+@Composable
+private fun CreateGuarantorDatePickerDialog(
+    initialDateMillis: Long?,
+    onConfirm: (Long?) -> Unit,
+    onDismiss: () -> Unit,
+) {
+    val datePickerState = rememberDatePickerState(
+        initialSelectedDateMillis = initialDateMillis,
+        selectableDates = object : SelectableDates {
+            override fun isSelectableDate(utcTimeMillis: Long): Boolean {
+                return utcTimeMillis <= Clock.System.now().toEpochMilliseconds()
+            }
+        },
+    )
+
+    DatePickerDialog(
+        onDismissRequest = onDismiss,
+        confirmButton = {
+            TextButton(
+                onClick = {
+                    onConfirm(datePickerState.selectedDateMillis)
+                    onDismiss()
+                },
+            ) {
+                Text(stringResource(Res.string.feature_loan_select_date))
+            }
+        },
+        dismissButton = {
+            TextButton(
+                onClick = onDismiss,
+            ) {
+                Text(stringResource(Res.string.feature_loan_cancel))
+            }
+        },
+    ) {
+        DatePicker(
+            state = datePickerState,
+        )
+    }
+}
diff --git a/feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/createGuarantor/CreateGuarantorViewModel.kt b/feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/createGuarantor/CreateGuarantorViewModel.kt

Original file line number	Diff line number	Diff line change
`@@ -7,7 +7,7 @@`
`7`	`7`	`*`
`8`	`8`	`* See https://github.com/openMF/mifos-x-field-officer-app/blob/master/LICENSE.md`
`9`	`9`	`*/`
`10`		`-package com.mifos.core.model.objects.account.loan.guarantor`
	`10`	`+package com.mifos.core.network.dto.loans`
`11`	`11`
`12`	`12`	`import kotlinx.serialization.Serializable`
`13`	`13`