Skip to content
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: Migrate :core:datastore module to KMP #2291

Open
wants to merge 14 commits into
base: kmp-impl
Choose a base branch
from

Conversation

kapmaurya
Copy link
Contributor

Fixes - Jira-#Issue_Number

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.

@kapmaurya kapmaurya marked this pull request as draft January 21, 2025 17:56
@Nagarjuna0033
Copy link

Nagarjuna0033 commented Jan 21, 2025

@kapmaurya dataState wrapper should be in common, for now let it be after merging common you can remove from this module and use it from common and also mifosDispatcher will be present in common module

@niyajali niyajali changed the title Datastore migrate kmpl feat: Migrate :core:datastore module to KMP Jan 22, 2025

//expect annotation class IgnoredOnParcel()

expect interface Parceler<P> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

remove Parcelize class implementation from this module you no need in this module, it should be in common module.

Choose a reason for hiding this comment

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

@kapmaurya have you used that class anywhere in this module.

started = SharingStarted.Eagerly,
)

override val defaultAccountId: StateFlow<Long?>
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think you needed account id in this project update this data source according to android-client project take a look at old PreferenceManger class and implement those thing only and remove remaining functions

Choose a reason for hiding this comment

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

@kapmaurya clientId and defaultAccountId those are there in mifos-mobile , just use the arguments which are there in preferenceManager before conversion only. I think only user is there in it


private const val USER_INFO_KEY = "userInfo"
private const val CLIENT_INFO_KEY = "clientInfo"

Copy link
Collaborator

Choose a reason for hiding this comment

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

Update this class accordingly for this project

@@ -7,7 +7,7 @@ androidDesugarJdkLibs = "2.1.4"
androidIconifyMaterial = "2.2.2"
androidJob = "1.2.6"
androidMapsUtils = "0.4.2"
androidGradlePlugin = "8.8.0"
androidGradlePlugin = "8.7.2"
Copy link
Collaborator

Choose a reason for hiding this comment

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

why you're downgrading the version, use AGP 8.8.0 or later

@niyajali
Copy link
Collaborator

@kapmaurya ask Revnath for help, connect with him and fix above issues

@revanthkumarJ
Copy link

@kapmaurya and regarding DataState if you want to merge this PR first then replace DataState with default kotlin ones like Response.success

otherwise as common module is also your task complete it first merge it and then use that DataState here

@revanthkumarJ
Copy link

@kapmaurya you can refer revanthkumarJ@bc03d5b

I think there is no need to use Parcelize, it is not used in mobile-wallet and mifos mobile dataStore

Similarly search what are the viewmodels and others using PreferenceManager if needed add other data models

@therajanmaurya therajanmaurya marked this pull request as ready for review January 30, 2025 16:42
Comment on lines 25 to 38
dependencies {
api(projects.core.model)
api(projects.core.common)

api(libs.converter.gson)

// fineract sdk dependencies
api(libs.mifos.android.sdk.arch)

// sdk client
api(libs.fineract.client)
kotlin {
sourceSets {
commonMain.dependencies {
implementation(libs.multiplatform.settings)
implementation(libs.multiplatform.settings.serialization)
implementation(libs.multiplatform.settings.coroutines)
implementation(libs.kotlinx.coroutines.core)
implementation(libs.kotlinx.serialization.core)
// implementation(projects.core.common)
}
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

why it's inside the dependencies block?

//import com.russhwolf.settings.serialization.decodeValueOrNull
//import com.russhwolf.settings.serialization.encodeValue
//import kotlinx.coroutines.CoroutineDispatcher
//

Choose a reason for hiding this comment

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

if these commented code not required remove it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

okk

// }
//
// fun setPermissionDeniedStatus(permissionDeniedStatus: String, status: Boolean) {
// preference.edit().putBoolean(permissionDeniedStatus, status).apply()

Choose a reason for hiding this comment

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

remove this file as you converted this in commonMain

Copy link
Contributor Author

Choose a reason for hiding this comment

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

okk

alias(libs.plugins.mifos.android.library.jacoco)
alias(libs.plugins.mifos.android.hilt)
alias(libs.plugins.mifos.kmp.library)
//id(libs.plugins.kotlin.parcelize.get().pluginId)

Choose a reason for hiding this comment

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

remove //id(libs.plugins.kotlin.parcelize.get().pluginId)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

okk

"identityHash": "52a9a8b64b78975c74e557a9c0411c25",
"entities": [
{
"tableName": "ColumnValue",

Choose a reason for hiding this comment

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

I think this file is not needed during dataStoreMigration

…into Datastore-migrate-kmpl

# Conflicts:
#	core/datastore/src/commonMain/kotlin/org.mifos.core.datastore/UserPreferencesDataSource.kt
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.

6 participants