Skip to content

feat(core:domain): Migrate to KMP #2350

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

Draft
wants to merge 16 commits into
base: kmp-impl
Choose a base branch
from

Conversation

biplab1
Copy link
Contributor

@biplab1 biplab1 commented Mar 31, 2025

Fixes - Jira-#410

Didn't create a Jira ticket, click here to create new.

Please Add Screenshots If there are any UI changes.

Before After

Please make sure these boxes are checked before submitting your pull request - thanks!

  • Run the static analysis check ./gradlew check or ci-prepush.sh to make sure you didn't break anything

  • If you have multiple commits please combine them into one commit by squashing them.

@@ -8,37 +8,67 @@
* See https://github.com/openMF/android-client/blob/master/LICENSE.md
*/
plugins {
alias(libs.plugins.mifos.android.library)
alias(libs.plugins.mifos.kmp.library)
alias(libs.plugins.mifos.android.library.jacoco)
alias(libs.plugins.mifos.android.koin)
Copy link
Contributor

@itsPronay itsPronay Mar 31, 2025

Choose a reason for hiding this comment

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

this is for android. Remove this and include alias(libs.plugins.mifos.kmp.koin)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Implemented the suggestion. Thanks

Comment on lines +14 to +15
alias(libs.plugins.jetbrainsCompose)
alias(libs.plugins.compose.compiler)
Copy link
Contributor

Choose a reason for hiding this comment

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

why do we need this!?!

Copy link
Contributor Author

@biplab1 biplab1 Mar 31, 2025

Choose a reason for hiding this comment

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

Those are added for the following code in build.gradle of core:domain:

compose.resources {
    publicResClass = true
    generateResClass = always
    packageOfResClass = "core.domain.generated.resources"
}

itsPronay

This comment was marked as duplicate.

Comment on lines 26 to 34
): Flow<Resource<GenericResponse>> = callbackFlow {
): Flow<Resource<GenericResponse>> = flow {
try {
trySend(Resource.Loading())
emit(Resource.Loading())
val response = activateRepository.activateGroup(groupId, groupPayload)
trySend(Resource.Success(response))
emit(Resource.Success(response))
} catch (exception: Exception) {
send(Resource.Error(exception.message.toString()))
emit(Resource.Error(exception.message.toString()))
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

@biplab1 don't use try-catch inside flow. Take reference from LoginUseCase

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will look into it. Thanks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have implemented your suggestion.

Biplab Dutta added 4 commits April 1, 2025 11:37
# Conflicts:
#	core/domain/src/commonMain/kotlin/com/mifos/core/domain/useCases/DeleteClientAddressPinpointUseCase.kt
#	core/domain/src/main/java/com/mifos/core/domain/useCases/GetAllLoanUseCase.kt
try {
emit(Resource.Loading())
emit(Resource.Loading())
}.flatMapLatest {
repository.getCentersGroupAndMeeting(centerId)
Copy link
Contributor

@itsPronay itsPronay Apr 1, 2025

Choose a reason for hiding this comment

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

when return type is flow, i dont think we need useCases, use it directly in the viewmodel

Copy link
Contributor Author

@biplab1 biplab1 Apr 1, 2025

Choose a reason for hiding this comment

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

We are returning flow in every useCases. I don't think I understand your statement. Could you please elaborate?

Copy link
Contributor

Choose a reason for hiding this comment

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

@biplab1 since repository.getCentersGroupAndMeeting(centerId) already returns flow, we do not need to convert it to flow again.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I understand your point now. Thanks

Copy link
Contributor Author

@biplab1 biplab1 Apr 7, 2025

Choose a reason for hiding this comment

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

Here we are combining two flows into a single flow using zip

Biplab Dutta added 3 commits April 1, 2025 23:02
# Conflicts:
#	core/domain/src/androidMain/kotlin/com/mifos/core/domain/useCases/GroupsListPagingDataSource.android.kt
Comment on lines 25 to 40

operator fun invoke(): Flow<Resource<SavingProductsAndTemplate?>> =
flow {
try {
emit(Resource.Loading())

val savingProductsAndTemplate = coroutineScope {
val savingsAccount = async { repository.savingsAccounts() }
val template = async { repository.savingsAccountTemplate() }

SavingsProductsAndTemplate(
savingsAccount = savingsAccount.await(),
template = template.await(),
)
}
emit(Resource.Success(savingProductsAndTemplate))
Copy link
Contributor

Choose a reason for hiding this comment

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

use combine instead

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will look into it. Thanks

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have implemented your suggestion. Thanks again

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.

2 participants