feat : Migrate Datatable to cmp #2382

itsPronay · 2025-05-23T16:29:11Z

Fixes - https://mifosforge.jira.com/browse/MIFOSAC-419

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.

...ure/data-table/src/commonMain/kotlin/com/mifos/feature/dataTable/dataTableList/FormWidget.kt

.../data-table/src/commonMain/kotlin/com/mifos/feature/dataTable/dataTableList/FormWidgetDto.kt

niyajali · 2025-06-10T17:45:18Z

@itsPronay resolve merge conflicts

itsPronay · 2025-06-10T17:46:56Z

@niyajali done

niyajali · 2025-06-10T17:47:55Z

@HekmatullahAmin @kapmaurya @Aditya3815 Despite repeated requests, why haven't you reviewed the PRs?

niyajali · 2025-06-10T17:48:38Z

@itsPronay ask one more team member to approve the PR

HekmatullahAmin · 2025-06-10T18:05:56Z

@itsPronay i will review it

HekmatullahAmin · 2025-06-10T18:11:38Z

feature/data-table/build.gradle.kts

-    alias(libs.plugins.mifos.android.library.compose)
-    alias(libs.plugins.mifos.android.library.jacoco)
+    alias(libs.plugins.mifos.cmp.feature)
+    alias(libs.plugins.mifos.kmp.koin)


mifos.kmp.koin is already added as plugin in CMPFeatureConventionPlugin and KMPLibraryConventionPlugin

HekmatullahAmin · 2025-06-10T18:14:40Z

...data-table/src/commonMain/kotlin/com/mifos/feature/dataTable/dataTable/DataTableViewModel.kt

-        } else {
-            _dataTableUiState.value = DataTableUiState.ShowDataTables(response)
+    fun loadDataTable(tableName: String?) {
+        viewModelScope.launch {


Both the refresh and loadDataTable independently call viewModelScope.launch, leading to Redundant coroutine launches.

make the loadDataTable suspend and remove viewModelScope.launch

HekmatullahAmin · 2025-06-10T18:16:52Z

...table/src/commonMain/kotlin/com/mifos/feature/dataTable/dataTableData/DataTableDataScreen.kt

-                        ).show()
-                        onBackPressed()
+                        MifosAlertDialog(
+                            dialogTitle = "Success",


This is a completed action you should use for it snackbar.
When to use which:

Use an AlertDialog only if:

You want the user to acknowledge something critical.

There's a decision to be made (Yes/No, Confirm/Cancel).

It’s a destructive or risky operation before it's confirmed.

Use Snackbar if:

Users don’t need to confirm a successful deletion just be informed.

A Snackbar allows the user to continue using the app without interruption.

Android Material guidelines encourage using snackbars for lightweight feedback.

HekmatullahAmin · 2025-06-10T18:21:36Z

...le/src/commonMain/kotlin/com/mifos/feature/dataTable/dataTableList/DataTableListViewModel.kt

-                    _dataTableListUiState.value =
-                        DataTableListUiState.ShowMessage(R.string.feature_data_table_loan_creation_success)
+        viewModelScope.launch {
+            repository.createLoansAccount(loansPayload)


Could please look into these types of function these doesn't need to be flowable. They are one shot api calls like creating, deleting, editing .... these need to be taken care of

saw your response below, fixing it accordingly

HekmatullahAmin · 2025-06-10T18:22:14Z

...le/src/commonMain/kotlin/com/mifos/feature/dataTable/dataTableList/DataTableListViewModel.kt

-                override fun onNext(loans: Loan?) {
-                    _dataTableListUiState.value =
-                        DataTableListUiState.ShowMessage(R.string.feature_data_table_loan_creation_success)
+        viewModelScope.launch {


HekmatullahAmin · 2025-06-10T18:30:28Z

...commonMain/kotlin/com/mifos/feature/dataTable/dataTableRowDialog/DataTableRowDialogScreen.kt

-                            Toast.LENGTH_SHORT,
-                        ).show()
-                        onSuccess()
+                        MifosAlertDialog(


same here use Snackbar instead of AlertDialog

HekmatullahAmin · 2025-06-10T18:31:49Z

...monMain/kotlin/com/mifos/feature/dataTable/dataTableRowDialog/DataTableRowDialogViewModel.kt

+        payload: Map<String, String>,
+    ) {
+        viewModelScope.launch {
+            addDataTableEntryUseCase.invoke(table, entityId, payload)


addDataTableEntry() is a single network call it doesn't emit multiple values or benefit from reactive chaining.

Using Flow adds boilerplate: mapping to DataState, managing emissions, and collecting with lifecycle scope.

For one-shot results, suspend functions with try/catch are simpler, faster, and easier to test.

Below is just an example change it accordingly and handle others like this:

class AddDataTableEntryUseCase( private val repository: DataTableRowDialogRepository, ) { suspend operator fun invoke( table: String, entityId: Int, payload: Map<String, String>, ): DataState<GenericResponse> { return try { val response = repository.addDataTableEntry(table, entityId, payload) DataState.Success(response) } catch (e: Exception) { DataState.Error(e) } } }

fun addDataTableEntry(table: String, entityId: Int, payload: Map<String, String>) { _dataTableRowDialogUiState.value = DataTableRowDialogUiState.Loading viewModelScope.launch(Dispatchers.IO) { val result = addDataTableEntryUseCase(table, entityId, payload) _dataTableRowDialogUiState.value = when (result) { is DataState.Success -> DataTableRowDialogUiState.DataTableEntrySuccessfully is DataState.Error -> DataTableRowDialogUiState.Error( R.string.feature_data_table_failed_to_add_data_table ) else -> DataTableRowDialogUiState.Initial } } }

HekmatullahAmin · 2025-06-10T18:40:40Z

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

@@ -35,6 +36,9 @@ interface DataTableService {
        @Path("entityId") entityId: Int,
    ): JsonArray

+    @GET("datatables")
+    fun getDatatables(@Query("apptable") apptable: String? = null): Flow<List<GetDataTablesResponse>>


This is already in core -> network -> apis -> DataTablesApi.kt is there any reason you created here otherwise delete it

@HekmatullahAmin I am getting ResponseConverter error when i use baseApiManager instead of mBaseApiManager & that's why I used mBaseApiManager instead of baseApiManager

Could not figure out exactly why this is happening. Any idea!?!

@itsPronay I think adding a FlowConverterFactory in FineractClient.kt's ktofitBuilder (like we did it in NetworkModule.kt) might resolve the ResponseConverter error for now:

val ktorfitBuilder = Ktorfit.Builder() .httpClient(ktorClient) .baseUrl(baseURL) .converterFactories(FlowConverterFactory()) .build()

@itsPronay if you see that error it is completely obvious because the converter class is not added when doing DI.
@biplab1 brother i thought in previous PR's you took care of it and add it why it is missing or you forgot.

HekmatullahAmin · 2025-06-10T18:54:40Z

...data-table/src/commonMain/kotlin/com/mifos/feature/dataTable/dataTable/DataTableViewModel.kt

+                            if (result.isEmpty()) {
+                                _dataTableUiState.value = DataTableUiState.ShowEmptyDataTables
+                            } else {
+                                _dataTableUiState.value = DataTableUiState.ShowDataTables(result)


sometimes when you see a part of code is duplicated have a look if there is a way to avaoid duplication of code:

is DataState.Success -> { val result = dataState.data _dataTableUiState.value = if (result.isEmpty()) { DataTableUiState.ShowEmptyDataTables } else { DataTableUiState.ShowDataTables(result) } }

HekmatullahAmin · 2025-06-10T19:03:32Z

...le/src/commonMain/kotlin/com/mifos/feature/dataTable/dataTableData/DataTableDataViewModel.kt

        }

    fun deleteDataTableEntry(table: String, entity: Int, rowId: Int) =
-        viewModelScope.launch(Dispatchers.IO) {
+        viewModelScope.launch {


Why deleting a table should be flowable this is wrong properly handle it in DeleteDataTableEntryUseCase

also have a look at GetDataTableInfoUseCase and fix it accordingly

right now, there are few issues with the domain module as @biplab1 mentioned, there is a separate jira ticket as well for that. we will be working on it after this migration and fixing those issues in domain

@itsPronay i just looked at the repository in so many use cases although the api call is suspend the change it to flow and then call it as a one time api call these need to be take care of. If there is a seperate ticket no issue.

biplab1 · 2025-06-11T18:11:06Z

...-table/src/commonMain/kotlin/com/mifos/feature/dataTable/dataTableList/FormSpinnerCompose.kt

+//    fun getIdOfSelectedItem(key: String): Int {
+//        return spinnerValueIdMap[key]!!
+//    }
+// }


I noticed this block of code is commented out — could you share why it's been left that way? Was it for debugging, future use, or is it safe to remove?

kept it for future reference since there are so many todos in datatable module. once we take care of todos these will be deleted

HekmatullahAmin · 2025-06-11T21:26:53Z

@itsPronay when you are done with the rest of changes that need to be made let me know i will approve.

itsPronay · 2025-06-12T16:22:05Z

@itsPronay when you are done with the rest of changes that need to be made let me know i will approve.

@HekmatullahAmin done, just left out the domain-related changes since we will be working on that in a separate ticket and fixing those accordingly in all viewmodel

HekmatullahAmin · 2025-06-12T16:36:24Z

@itsPronay when you are done with the rest of changes that need to be made let me know i will approve.

@HekmatullahAmin done, just left out the domain-related changes since we will be working on that in a separate ticket and fixing those accordingly in all viewmodel

@itsPronay great, just do one thing for those you commented that needed to be done later like from XML to compose please add a TODO so in our studio we can see what things are in the TODO section. It will be easy to chase.

itsPronay · 2025-06-12T17:06:12Z

@HekmatullahAmin There are no todos from my side since I have already migrated the XML to Compose. The existing todos in the DataTable module were already present before.

also I noticed that the old XML code is still there along with those todos, so I kept the old code as well, as we might need it while implementing them

HekmatullahAmin · 2025-06-12T17:15:10Z

...commonMain/kotlin/com/mifos/feature/dataTable/dataTableRowDialog/DataTableRowDialogScreen.kt

+        onCreate = {},
+    )
+}
+
 // private fun createForm(table: DataTable?) {


@itsPronay by adding TODO I mean for example this code is commented at above add why and when it will be taken care of.
for ex: this is commented because of X and we will fix it after doing Y

oh.okok. I'll add it. thanks for the clarification

@itsPronay when i say TODO you should type it TODO in capital letter and then it will be higlighted as yellow otherwise it will be the same as the rest of comments.
But no issue next time, have it in your mind.

HekmatullahAmin · 2025-06-13T07:37:22Z

@niyajali looks good to me

feature/dataTable Migration to CMP

37b9ed4

itsPronay marked this pull request as draft May 23, 2025 16:29

itsPronay added 6 commits May 27, 2025 23:15

Merge branch 'kmp-impl' into datatable-cmp

edcb6a6

datatable

0e673fa

Merge branch 'kmp-impl' into datatable-cmp

97e501b

updated

ad5ab9e

updated

ea270df

updated

5dae2ce

itsPronay marked this pull request as ready for review June 10, 2025 15:02

revanthkumarJ suggested changes Jun 10, 2025

View reviewed changes

...ure/data-table/src/commonMain/kotlin/com/mifos/feature/dataTable/dataTableList/FormWidget.kt Show resolved Hide resolved

.../data-table/src/commonMain/kotlin/com/mifos/feature/dataTable/dataTableList/FormWidgetDto.kt Outdated Show resolved Hide resolved

revanthkumarJ approved these changes Jun 10, 2025

View reviewed changes

Merge branch 'kmp-impl' into datatable-cmp

ff9610b

HekmatullahAmin suggested changes Jun 10, 2025

View reviewed changes

itsPronay added 3 commits June 11, 2025 21:51

Merge branch 'kmp-impl' into datatable-cmp

d87b7b2

made suggested changes

a4f95ed

update

674aad8

biplab1 reviewed Jun 11, 2025

View reviewed changes

biplab1 approved these changes Jun 11, 2025

View reviewed changes

itsPronay changed the title ~~Draft : Migrate Datatable to cmp~~ draft : Migrate Datatable to cmp Jun 12, 2025

itsPronay changed the title ~~draft : Migrate Datatable to cmp~~ feat : Migrate Datatable to cmp Jun 12, 2025

revanthkumarJ approved these changes Jun 12, 2025

View reviewed changes

revanthkumarJ and others added 2 commits June 12, 2025 18:23

Merge branch 'kmp-impl' into datatable-cmp

b0d82dd

update

bad1362

HekmatullahAmin reviewed Jun 12, 2025

View reviewed changes

added todo for future reference

6aa56d6

HekmatullahAmin approved these changes Jun 13, 2025

View reviewed changes

niyajali merged commit db36288 into openMF:kmp-impl Jun 13, 2025
2 checks passed

biplab1 mentioned this pull request Jun 13, 2025

[DMP 2025]: Mifos Pay v4.0 openMF/mobile-wallet#1852

Open

Nagarjuna0033 mentioned this pull request Jun 20, 2025

[DMP 2025]: Mifos Mobile - End User Application v8.0 openMF/mifos-mobile#2817

Open

feat : Migrate Datatable to cmp #2382

feat : Migrate Datatable to cmp #2382

Uh oh!

Conversation

itsPronay commented May 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

niyajali commented Jun 10, 2025

Uh oh!

itsPronay commented Jun 10, 2025

Uh oh!

niyajali commented Jun 10, 2025

Uh oh!

niyajali commented Jun 10, 2025

Uh oh!

HekmatullahAmin commented Jun 10, 2025

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!

This comment was marked as outdated.

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!

itsPronay Jun 11, 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!

biplab1 Jun 11, 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!

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!

HekmatullahAmin commented Jun 11, 2025

Uh oh!

itsPronay commented Jun 12, 2025

Uh oh!

HekmatullahAmin commented Jun 12, 2025

Uh oh!

itsPronay commented Jun 12, 2025

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!

HekmatullahAmin commented Jun 13, 2025

Uh oh!

Uh oh!

Uh oh!

itsPronay commented May 23, 2025 •

edited

Loading

itsPronay Jun 11, 2025 •

edited

Loading

biplab1 Jun 11, 2025 •

edited

Loading