-
Notifications
You must be signed in to change notification settings - Fork 719
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: (feature:loan-account)
Migrate loan-account module to CMP
#2775
base: kmp-impl
Are you sure you want to change the base?
Conversation
import mifos_mobile.feature.loan_account.generated.resources.feature_account_matured | ||
import org.mifos.mobile.feature.loanaccount.model.CheckboxStatus | ||
|
||
object StatusUtil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Create a EnumClass instead like Status(val name: String) and create all type of status like
enum class Status(val name: String/StringResource){
ACTIVE(
name = Res.string.feature_account_active
)
}
and you can get those using Staus.entries, it will return as List no need to create overhead like above object/fun etc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
import org.mifos.mobile.feature.loanaccount.utils.FilterUtil | ||
import org.mifos.mobile.feature.loanaccount.utils.StatusUtil | ||
import org.mifos.mobile.feature.loanaccount.utils.getAccountsFilterLabels | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Optimize this codebase, move all filtering logic to separate UseCase and use ioDispatcher for thread safe, ask AI to enhance this codebase look messy to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
explained everything here:
#2779 (comment)
} | ||
|
||
@Composable | ||
private fun getAccountStatusDisplayAttributes(loanAccount: LoanAccount): Triple<Color, String, Color?> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
move this fun to AccountCard file since this only use for that Composable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
) | ||
|
||
AccountCard( | ||
accountNo = loanAccount.accountNo, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why don't we pass LoanAccount object in AccountCard directly and move all it's logic on that file like formatterBalance, etc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
import org.mifos.mobile.feature.loanaccount.utils.StatusUtil | ||
import org.mifos.mobile.feature.loanaccount.utils.getAccountsFilterLabels | ||
|
||
class LoanAccountViewmodel( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Like this, we could enhance/optimize more
// Domain - Use Cases
interface LoadLoanAccountsUseCase {
suspend operator fun invoke(clientId: String): Flow<Result<List<LoanAccount>>>
}
class LoadLoanAccountsUseCaseImpl(
private val accountsRepository: AccountsRepository
) : LoadLoanAccountsUseCase {
override suspend operator fun invoke(clientId: String): Flow<Result<List<LoanAccount>>> =
accountsRepository.loadAccounts(
clientId = clientId,
accountType = AccountType.LOAN.name
).map { response ->
Result.success(response.data?.loanAccounts.orEmpty())
}.catch { error ->
emit(Result.failure(error))
}
}
interface FilterLoanAccountsUseCase {
operator fun invoke(
accounts: List<LoanAccount>,
searchQuery: String?,
filterList: List<CheckboxStatus>
): List<LoanAccount>
}
class FilterLoanAccountsUseCaseImpl : FilterLoanAccountsUseCase {
override operator fun invoke(
accounts: List<LoanAccount>,
searchQuery: String?,
filterList: List<CheckboxStatus>
): List<LoanAccount> {
val searchTerm = searchQuery?.lowercase().orEmpty()
val activeFilters = filterList.filter { it.isChecked }
return if (searchTerm.isEmpty() && activeFilters.isEmpty()) {
accounts
} else {
accounts.filter { account ->
val matchesSearch = if (searchTerm.isEmpty()) true else
account.productName?.lowercase()?.contains(searchTerm) == true ||
account.accountNo?.lowercase()?.contains(searchTerm) == true
val matchesFilter = if (activeFilters.isEmpty()) true else
activeFilters.any { filter ->
account.matchesFilter(filter.status)
}
matchesSearch && matchesFilter
}
}
}
private fun LoanAccount.matchesFilter(status: String?): Boolean {
val criteria = getAccountsFilterLabels()
return when (status) {
criteria.inArrearsString -> inArrears == true
criteria.activeString -> this.status?.active == true
criteria.waitingForDisburseString -> this.status?.waitingForDisbursal == true
criteria.approvalPendingString -> this.status?.pendingApproval == true
criteria.overpaidString -> this.status?.overpaid == true
criteria.closedString -> this.status?.closed == true
criteria.withdrawnString -> this.status?.isLoanTypeWithdrawn() == true
else -> false
}
}
}
// UI State
sealed interface LoanAccountUiState {
data object Loading : LoanAccountUiState
data object Error : LoanAccountUiState
data class Success(
val accounts: List<LoanAccount>,
val filteredAccounts: List<LoanAccount>
) : LoanAccountUiState
}
// ViewModel
@HiltViewModel
class LoanAccountViewModel @Inject constructor(
private val loadLoanAccountsUseCase: LoadLoanAccountsUseCase,
private val filterLoanAccountsUseCase: FilterLoanAccountsUseCase,
networkMonitor: NetworkMonitor,
userPreferencesRepository: UserPreferencesRepository,
) : ViewModel() {
private val clientId = requireNotNull(userPreferencesRepository.clientId.value)
val isNetworkAvailable = networkMonitor.isOnline
.stateIn(
scope = viewModelScope,
started = SharingStarted.WhileSubscribed(5_000),
initialValue = false,
)
private val _uiState = MutableStateFlow<LoanAccountUiState>(LoanAccountUiState.Loading)
val uiState = _uiState.asStateFlow()
private val _searchQuery = MutableStateFlow("")
private val _filterList = MutableStateFlow(StatusUtil.getLoanAccountStatusList())
private val currentAccounts = MutableStateFlow<List<LoanAccount>>(emptyList())
init {
viewModelScope.launch {
// Combine search and filter changes to update UI
combine(
_searchQuery,
_filterList,
currentAccounts
) { query, filters, accounts ->
Triple(query, filters, accounts)
}.debounce(300) // Debounce user input
.collect { (query, filters, accounts) ->
val filteredAccounts = filterLoanAccountsUseCase(
accounts = accounts,
searchQuery = query,
filterList = filters
)
updateUiState(accounts, filteredAccounts)
}
}
}
fun loadAccounts() {
viewModelScope.launch {
_uiState.value = LoanAccountUiState.Loading
loadLoanAccountsUseCase(clientId)
.collect { result ->
result.fold(
onSuccess = { accounts ->
currentAccounts.value = accounts
updateUiState(accounts, accounts)
},
onFailure = {
_uiState.value = LoanAccountUiState.Error
}
)
}
}
}
fun updateSearchQuery(query: String) {
_searchQuery.value = query
}
fun updateFilters(filters: List<CheckboxStatus>) {
_filterList.value = filters
}
private fun updateUiState(
originalAccounts: List<LoanAccount>,
filteredAccounts: List<LoanAccount>
) {
_uiState.value = LoanAccountUiState.Success(
accounts = originalAccounts,
filteredAccounts = filteredAccounts
)
}
}
:feature:loan-account
Migrated to CMP
@niyajali Thanks for your valuable feedbacks! I will definitely take your suggestions on board and apply them. For now, I’m focusing on moving strings, methods, and screen-specific logic into their respective modules. Once everything is completed for each screen, I will revisit your suggestions and improve the code accordingly to ensure everything is clean and efficient. |
7ad6f93
to
df2e753
Compare
:feature:loan-account
Migrated to CMP(feature:loan-account)
Migrate loan-account module to CMP
Fixes - Jira-#183
Didn't create a Jira ticket, click here to create new.
Please Add Screenshots If there are any UI changes.
Please make sure these boxes are checked before submitting your pull request - thanks!
Run the static analysis check
./gradlew check
orci-prepush.sh
to make sure you didn't break anythingIf you have multiple commits please combine them into one commit by squashing them.