refactor (core:network ) Network converted to KMP by revanthkumarJ · Pull Request #2346 · openMF/android-client

revanthkumarJ · 2025-03-28T10:04:37Z

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.

# Conflicts: # core/network/build.gradle.kts

core/network/build.gradle.kts

revanthkumarJ · 2025-04-16T17:45:29Z

@biplab1 review it once if things are ok then we can merge this and complete the data and domain next

core/network/src/commonMain/kotlin/com/mifos/core/network/BaseApiManager.kt

biplab1

These are some minor changes that needs to be addressed.

Also, add @OptIn(ExperimentalCoroutinesApi::class) to functions using flatMapLatest in DataManagers

biplab1 · 2025-04-17T03:22:18Z

core/database/src/commonMain/kotlin/com/mifos/room/entities/client/ChargesEntity.kt

                if (dueDateList.size > 2) {
-                    return String.format(pattern, dueDateList[0], dueDateList[1], dueDateList[2])
+                    return ""
+//                    return String.format(pattern, dueDateList[0], dueDateList[1], dueDateList[2])


Can you tell me the reason for commenting out this code?

by mistake uncommented it

biplab1 · 2025-04-17T07:43:16Z

core/network/build.gradle.kts

+    id("kotlinx-serialization")
+    id("com.google.devtools.ksp")


Please use version catalog instead:

alias(libs.plugins.kotlin.serialization) alias(libs.plugins.ksp)

biplab1 · 2025-04-17T08:07:51Z

.../network/src/commonMain/kotlin/com/mifos/core/network/datamanager/DataManagerCheckerInbox.kt

@@ -47,15 +47,15 @@ class DataManagerCheckerInbox(
        return mBaseApiManager.checkerInboxApi.getRescheduleLoansTaskList()


"getRechdeduleLoansTaskList" should be "getRescheduleLoansTaskList"

I didn't understood . It's already getRescheduleLoansTaskList

biplab1 · 2025-04-17T08:13:29Z

core/data/src/main/java/com/mifos/core/data/repositoryImp/CheckerInboxTasksRepositoryImp.kt

        resourceId: Int?,
    ): Flow<List<CheckerTask>> {
-        return flow { emit(dataManagerCheckerInbox.getCheckerTaskList()) }
+        return dataManagerCheckerInbox.getCheckerTaskList()


So due to Github limitation, I cannot comment on the lines that are outside this PR, so I am adding it here. In line no. 27 of this file,
"getRechdeduleLoansTaskList" should be "getRescheduleLoansTaskList"

biplab1 · 2025-04-17T08:16:50Z

core/network/src/commonMain/kotlin/com/mifos/core/network/datamanager/DataManagerClient.kt

-                            )
-                            clientsTemplate
-                        }
+                    flow {


clientTemplate should be marked with @OptIn(ExperimentalCoroutinesApi::class)

@OptIn(ExperimentalCoroutinesApi::class) val clientTemplate: Flow<ClientsTemplateEntity>

biplab1 · 2025-04-17T09:25:51Z

core/network/src/commonMain/kotlin/com/mifos/core/network/model/GetCentersResponse.kt

+@Serializable
+data class GetCentersResponse(
+
+    val pageItems: kotlin.collections.Set<GetCentersPageItems>? = null,


kotlin.collections is redundant here and can be removed.

Similarly, other files in :core:network:model makes use of it, please check and update.

biplab1 · 2025-04-17T09:32:35Z

core/network/src/commonMain/kotlin/com/mifos/core/network/model/ResultsetColumnHeaderData.kt

+
+    val columnName: kotlin.String? = null,
+
+    val columnType: ResultsetColumnHeaderData.ColumnType? = null,


Here ResultsetColumnHeaderData is redundant. It is better to remove. There are two occurrences in this file.

biplab1 · 2025-04-17T09:42:56Z

core/network/src/commonMain/kotlin/com/mifos/core/network/model/ScorecardPayload.kt

Please use an alternative to import java.util.Date which is compatible with KMP. Try using
import kotlinx.datetime.Instant and Instant in place of Date data type.

biplab1 · 2025-04-17T09:46:23Z

core/network/src/commonMain/kotlin/com/mifos/core/network/services/CheckerInboxService.kt


    @GET(APIEndPoint.MAKER_CHECKER + "/searchtemplate?fields=entityNames,actionNames")
-    fun getCheckerInboxSearchTempalate(): Observable<CheckerInboxSearchTemplate>
+    fun getCheckerInboxSearchTempalate(): Flow<CheckerInboxSearchTemplate>


getCheckerInboxSearchTempalate should be getCheckerInboxSearchTemplate. Please make appropriate changes in places where it is used.

biplab1 · 2025-04-17T09:49:50Z

core/network/src/commonMain/kotlin/com/mifos/core/network/services/CollectionSheetService.kt

Line 40: saveindividualCollectionSheet should be saveIndividualCollectionSheet. Also, make changes where it is used.

biplab1 · 2025-04-17T10:05:11Z

@revanthkumarJ Just a quick note for future reference — it's best to leave the review comments unresolved even after you've fixed them. That way, maintainers and reviewers can go through and verify the changes before marking them resolved.

HekmatullahAmin · 2025-04-17T11:19:03Z

@revanthkumarJ I have seen below in many places I give an example change all of them accordingly.

In some Places you changed the data types from nullable to non nullable be careful with it.
Also in some places you have asserted not null !! be careful with it
when you change the api call to return stream then change it in all places it gets called.

Api

interface CenterService {
    @GET(APIEndPoint.CENTERS)
    fun getAllCentersInOffice(
        @Query("officeId") officeId: Int,
        @QueryMap additionalParams: Map<String, String>,
    ): Flow<List<CenterEntity>>
}

DataManager

class DataManager @Inject constructor(private val mBaseApiManager: BaseApiManager) {
    fun getCentersInOffice(id: Int, params: Map<String, String>): Flow<List<CenterEntity>> {
        return mBaseApiManager.centerApi.getAllCentersInOffice(id, params)
    }
}

Repository

class GenerateCollectionSheetRepositoryImp @Inject constructor(
    private val dataManager: DataManager,
) : GenerateCollectionSheetRepository {

    override fun getCentersInOffice(id: Int, params: Map<String, String>): Flow<List<CenterEntity>> {
        return dataManager.getCentersInOffice(id, params)
    }
}

UseCase (it is just an example fix it accordingly)

class GetCentersInOfficeUseCase @Inject constructor(
    private val repository: GenerateCollectionSheetRepository,
) {
    operator fun invoke(id: Int, params: Map<String, String>): Flow<Resource<List<CenterEntity>>> =
        repository.getCentersInOffice(id, params)
            .map { Resource.Success(it) }
            .catch { emit(Resource.Error(it.message ?: "Unknown error")) }
            .onStart { emit(Resource.Loading()) }
}

HekmatullahAmin · 2025-04-17T07:30:09Z

core/data/src/main/java/com/mifos/core/data/repository/GenerateCollectionSheetRepository.kt

 interface GenerateCollectionSheetRepository {

-    suspend fun getCentersInOffice(id: Int, params: Map<String, String>): List<CenterEntity>
+    suspend fun getCentersInOffice(id: Int, params: Map<String, String>): Flow<List<CenterEntity>>


why some functions like these are at the same time suspend and flowable

ok i will resolve them . actually they belongs to data module at first i tried to resolve the things to make application run but later Rajan sir told that it will be done during data module migration

@revanthkumarJ ok then keep your scope to your own and change it to where it belongs to your module. If it is sth like that can you revert it back so someone who works in data module should get any conflicts. and for them it might be confusing like ok it's suspend and at the same time stream and they keep it

HekmatullahAmin · 2025-04-17T08:00:22Z

core/network/src/commonMain/kotlin/com/mifos/core/network/services/GroupService.kt


    @GET(APIEndPoint.GROUPS)
-    suspend fun getAllGroupsInOffice(
+    fun getAllGroupsInOffice(


I see the API call previously was suspend not a stream. so keep it suspend. and you manually wrap it in flow {} at repository layer or wherever it is called.

Or in the above i provided an example if you change it to stream how to do it. In chain some places you added both suspent and return type flow, in some places you removed suspend.

in nagarjuna pr niyaj brother mentioned that if any service returns list make it flow and remove suspend i followed that .

yes cause of that in my second message i mentioned that if you remove the suspend and add flow what you should do in other files that calls it

yah after the domain and data are merged we have to change in all feature modules where they are called.
In mifos-mobile we have done similarly

What niyaj mentioned in mifos-mobile network module conversion is

Few things to change I'm commenting on one place

if the function return as List or Observable then convert into Flow<List> or Flow
If the function return as a Flow then that shouldn't be suspend fun, so remove suspend modifier from function.

HekmatullahAmin · 2025-04-17T08:26:26Z

core/data/src/main/java/com/mifos/core/data/repositoryImp/CreateNewClientRepositoryImp.kt

-    override suspend fun uploadClientImage(id: Int, file: MultipartBody.Part?) {
-        dataManagerClient.uploadClientImage(id, file)
+    override suspend fun uploadClientImage(id: Int, file: PartData?) {
+//        dataManagerClient.uploadClientImage(id, file)


why you commented this one?

uncommneted it was a mistake

HekmatullahAmin · 2025-04-17T08:33:38Z

core/data/src/main/java/com/mifos/core/data/repositoryImp/CreateNewGroupRepositoryImp.kt


    override fun offices(): Flow<List<OfficeEntity>> {
-        return dataManagerOffices.offices()
+        return dataManagerOffices.offices


previously it was calling offices() from DataManagerOffices, which you changed it to fetchOffices. But now you are calling offices not fetchOffices. what is the reason?

resolved, used fetchoffices()

HekmatullahAmin · 2025-04-17T10:14:42Z

...ta/src/main/java/com/mifos/core/data/repositoryImp/SavingsAccountTransactionRepositoryImp.kt

    ): Flow<SavingsAccountTransactionTemplateEntity?> {
        return dataManagerSavings.getSavingsAccountTransactionTemplate(
-            type,
+            type!!,


Don't force it like this in here that it is not null. If it is always not null then change the type type to String wherever it is called.

HekmatullahAmin · 2025-04-17T10:27:01Z

core/network/src/commonMain/kotlin/com/mifos/core/network/datamanager/DataManagerNote.kt

     * This Method Request the REST API of Note and In response give the List of Notes
     */
-    fun getNotes(entityType: String?, entityId: Int): List<Note> {
+    fun getNotes(entityType: String, entityId: Int): List<Note> {


you changed the entityType type from String? to String. are you sure that when this function is called they entityType is not null, and also the actual api getNotes(...) only accept String cause of that you changed?

The reason i done that is when i runned the application it showed the error like path parameter in ktorfit should not be null

HekmatullahAmin · 2025-04-17T10:36:05Z

core/network/src/commonMain/kotlin/com/mifos/core/network/di/NetworkModule.kt

-            .apply {
-                logger(DebugLogger())
-            }.build()
+    single { (context: Any) ->


This DI injection looks so messy. I haven't work on network, but ask @niyajali to review this file for you.

HekmatullahAmin · 2025-04-17T10:40:19Z

core/network/src/commonMain/kotlin/com/mifos/core/network/mappers/groups/GroupMapper.kt

-                code = entity.status?.code,
-                value = entity.status?.description,
+                code = entity.status.code,
+                value = entity.status.description,


Does GetGroupsStatus code and description is not nullable that you removed safe call?

I don't think so so make it safe call

HekmatullahAmin · 2025-04-17T10:49:11Z

core/network/src/commonMain/kotlin/com/mifos/core/network/services/ClientService.kt

-    @get:GET(APIEndPoint.CLIENTS + "/template")
-    val clientTemplate: Flow<ClientsTemplateEntity>
+    @GET(APIEndPoint.CLIENTS + "/template")
+    suspend fun getClientTemplate(): ClientsTemplateEntity


why you are making it suspend when it is stream

it is not stream the below function to it is Flow it is not suspend function

HekmatullahAmin · 2025-04-17T10:55:08Z

core/data/src/main/java/com/mifos/core/data/repository/CheckerInboxRepository.kt

        entityName: String? = null,
        resourceId: Int? = null,
-    ): List<CheckerTask>
+    ): Flow<List<CheckerTask>>


when it is suspend why adding flow, maybe in the UseCase that it is called it will call this suspend function and then emit it accordigly

revanthkumarJ · 2025-04-17T13:25:28Z

@biplab1 @HekmatullahAmin i have done all the suggestions you guys mentioned , see it once if i have missed anything let me know before standup i will do it

biplab1 · 2025-04-17T14:03:34Z

core/data/src/main/java/com/mifos/core/data/repository/ClientDetailsRepository.kt

 interface ClientDetailsRepository {

-    suspend fun uploadClientImage(id: Int, file: MultipartBody.Part?)
+    suspend fun uploadClientImage(id: Int, file: PartData?)


Are these two equivalent— okhttp3.MultipartBody and io.ktor.http.content.PartData? They must be having different APIs and hence different implementations.

yes they have different implementations , while passing the the things in domain we have to take care of it

I have taken reference from mifos-wallet there MultipartBody.Part? is replaced with PartData

From mobile-wallet network module conversion PR

Thanks for the information

* initial setup * updated * Updated * Changed Imports in Services * Updated * Updated * Added ktorHttpClient * Added ktorHttpClient * Updated * Updated * Updated * updated mappers * updated mappers * Updated * Updated * Updated * Updated * Updated * Updated ImageLoader.Utils * changed observable to flow in data * Updated * Updated * Updated * Updated based on suggestions * Updated * Updated * Updated

revanthkumarJ added 2 commits March 28, 2025 15:28

initial setup

dfb2149

updated

e5c4011

revanthkumarJ changed the title ~~initial setup~~ refactor (core:network ) Network converted to KMP Mar 28, 2025

revanthkumarJ added 3 commits April 4, 2025 23:35

Merge branch 'kmp-impl' into network_migration

3f52d21

# Conflicts: # core/network/build.gradle.kts

Updated

5c0a21f

Changed Imports in Services

d09eba7

biplab1 reviewed Apr 7, 2025

View reviewed changes

core/network/build.gradle.kts Outdated Show resolved Hide resolved

revanthkumarJ added 18 commits April 8, 2025 15:32

Updated

ece3cac

Merge branch 'kmp-impl' into network_migration

a48fb8b

Updated

373ee29

Added ktorHttpClient

320da65

Added ktorHttpClient

653775a

Updated

8597f02

Updated

6453719

Updated

35d4ef2

updated mappers

c07b346

updated mappers

ad82e7a

Updated

e0e8063

Updated

cc7658b

Updated

e42a35c

Updated

f12e640

Updated

0dc4323

Updated ImageLoader.Utils

204ab71

changed observable to flow in data

b16bd65

Updated

73e2c11

revanthkumarJ marked this pull request as ready for review April 14, 2025 16:44

Updated

1d24ae5

biplab1 reviewed Apr 16, 2025

View reviewed changes

core/network/src/commonMain/kotlin/com/mifos/core/network/BaseApiManager.kt Outdated Show resolved Hide resolved

Updated

e82ee18

biplab1 requested changes Apr 17, 2025

View reviewed changes

HekmatullahAmin reviewed Apr 17, 2025

View reviewed changes

revanthkumarJ added 2 commits April 17, 2025 18:30

Updated based on suggestions

779171b

Updated

0803f61

Updated

f372589

biplab1 reviewed Apr 17, 2025

View reviewed changes

Updated

f62ce85

therajanmaurya merged commit befb189 into openMF:kmp-impl Apr 18, 2025
5 checks passed

		@@ -47,15 +47,15 @@ class DataManagerCheckerInbox(
		return mBaseApiManager.checkerInboxApi.getRescheduleLoansTaskList()


		val columnName: kotlin.String? = null,

		val columnType: ResultsetColumnHeaderData.ColumnType? = null,

Conversation

revanthkumarJ commented Mar 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

revanthkumarJ commented Apr 16, 2025

Uh oh!

Uh oh!

biplab1 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

biplab1 commented Apr 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

HekmatullahAmin commented Apr 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

In some Places you changed the data types from nullable to non nullable be careful with it.

Also in some places you have asserted not null !! be careful with it

when you change the api call to return stream then change it in all places it gets called.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

revanthkumarJ Apr 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

revanthkumarJ commented Mar 28, 2025 •

edited

Loading

biplab1 commented Apr 17, 2025 •

edited

Loading

HekmatullahAmin commented Apr 17, 2025 •

edited

Loading

Also in some places you have asserted not null `!!` be careful with it

revanthkumarJ Apr 17, 2025 •

edited

Loading