chore: update versions parity with kmp-project-template#2604
chore: update versions parity with kmp-project-template#2604amanna13 wants to merge 8 commits intoopenMF:developmentfrom
Conversation
📝 WalkthroughWalkthroughThis PR centralizes Room annotations into a Kotlin Multiplatform core (core-base/database), replaces platform-specific MifosDatabaseFactory with AppDatabaseFactory and its fluent builder across platforms, migrates imports across DAOs/entities/typeconverters, and performs broad dependency and build-tool version upgrades. Changes
Sequence Diagram(s)sequenceDiagram
participant App as App Code / DI
participant PlatformModule as PlatformSpecificModule
participant Factory as AppDatabaseFactory
participant Driver as BundledSQLiteDriver / Platform Driver
participant DB as MifosDatabase
App->>PlatformModule: request Database instance (Module)
PlatformModule->>Factory: createDatabase<DB>(Constants.DATABASE_NAME)
Factory->>Driver: setDriver(...)
Factory->>Factory: setQueryCoroutineContext(ioContext)
Factory->>Factory: build()
Factory-->>PlatformModule: return DB instance
PlatformModule-->>App: provide DB via DI
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
core-base/designsystem/src/commonMain/kotlin/template/core/base/designsystem/KptThemeExtensions.kt (1)
330-367:⚠️ Potential issue | 🟡 Minor
toKptColorScheme()omits the 12 fixed color tokens.While
toMaterial3ColorScheme()correctly maps all 12 fixed color properties (primaryFixed,primaryFixedDim,onPrimaryFixed,onPrimaryFixedVariant, and equivalents for secondary and tertiary), the reverse functionColorScheme.toKptColorScheme()omits them. This creates an asymmetric conversion where a round-tripKptColorScheme → ColorScheme → KptColorSchemesilently loses the fixed-color values, falling back toKptColorSchemeImpldefaults.Add the missing mappings to
toKptColorScheme():Proposed fix
fun ColorScheme.toKptColorScheme(): KptColorScheme = KptColorSchemeImpl( ... surfaceContainerLow = this.surfaceContainerLow, surfaceContainerLowest = this.surfaceContainerLowest, + primaryFixed = this.primaryFixed, + primaryFixedDim = this.primaryFixedDim, + onPrimaryFixed = this.onPrimaryFixed, + onPrimaryFixedVariant = this.onPrimaryFixedVariant, + secondaryFixed = this.secondaryFixed, + secondaryFixedDim = this.secondaryFixedDim, + onSecondaryFixed = this.onSecondaryFixed, + onSecondaryFixedVariant = this.onSecondaryFixedVariant, + tertiaryFixed = this.tertiaryFixed, + tertiaryFixedDim = this.tertiaryFixedDim, + onTertiaryFixed = this.onTertiaryFixed, + onTertiaryFixedVariant = this.onTertiaryFixedVariant, )core/database/src/commonMain/kotlin/com/mifos/room/entities/accounts/savings/SavingsAccountTransactionEntity.kt (1)
29-54:⚠️ Potential issue | 🟠 MajorForeign key definitions have inverted parent/child semantics and dangerous CASCADE deletes.
All three foreign keys declare
childColumns = ["id"](this entity's PK) matching parent tables' PKs, forcing every transaction'sidto match an existing type/currency/date recordid. This breaks insertion for any transaction whoseiddoesn't coincidentally match a type/currency/dateid.Additionally:
- CASCADE deletes are backwards:
onDelete = CASCADEwill delete transactions when a type, currency, or date record is deleted — the opposite of the intended relationship for reference/lookup data.- Missing FK columns: The entity defines nested objects (
transactionType,currency,savingsTransactionDate) but no corresponding FK columns (transactionTypeId,currencyId,dateId). These FKs are declaring constraints on the transaction's ownidrather than on actual foreign key columns.If the nested objects are being serialized (e.g., as JSON via TypeConverter), these FK constraints should be removed entirely. If the intent is to store relationships in the database, create dedicated FK columns (
transactionTypeId, etc.) on this entity and declare FKs on those columns instead, with CASCADE only on child tables that this transaction owns.
🤖 Fix all issues with AI agents
In
`@core/database/src/commonMain/kotlin/com/mifos/room/di/DatabaseModule.common.kt`:
- Around line 14-15: The expect val PlatformSpecificDatabaseModule currently
suppresses missing actuals while the appleMain (DatabaseModule.apple.kt), jsMain
(PlatformSpecificModule.kt) and wasmJsMain (PlatformSpecificModule.kt) files
contain TODOs that will crash at runtime; either replace those TODOs with real
platform-specific Koin Module implementations that initialize the DB (implement
the actual val PlatformSpecificDatabaseModule in each platform file), or remove
the expect suppression and gate module registration so the Koin graph never
loads PlatformSpecificDatabaseModule on unsupported targets (introduce and check
a compile-time/target-specific flag or function like isDatabaseSupported()
before including PlatformSpecificDatabaseModule in your DI setup). Ensure the
symbols to update are the actual declarations named
PlatformSpecificDatabaseModule and the platform files DatabaseModule.apple.kt,
PlatformSpecificModule.kt in jsMain and wasmJsMain.
In
`@core/database/src/commonMain/kotlin/com/mifos/room/entities/organisation/OfficeEntity.kt`:
- Around line 12-16: The import list in OfficeEntity.kt mixes packages: Entity,
ForeignKey, and PrimaryKey are imported from template.core.base.database while
ForeignKeyAction is still imported from com.mifos.room.utils; update the
OfficeEntity.kt import to use ForeignKeyAction from the new base package
(template.core.base.database.ForeignKeyAction) so all database annotations come
from the same package, or if the symbol wasn’t yet moved, add/move the
ForeignKeyAction definition into template.core.base.database and then update
references in OfficeEntity and any other classes that still import
com.mifos.room.utils.ForeignKeyAction to the new package.
In
`@core/database/src/commonMain/kotlin/com/mifos/room/typeconverters/GroupTypeConverters.kt`:
- Line 16: GroupTypeConverters.kt fails on JS/Wasm because it imports
template.core.base.database.TypeConverter but the base module has no actual for
JS/wasmJs; add actual declarations for the TypeConverter expect (from Room.kt)
into the base module's jsMain and wasmJsMain (or a shared jsWasmMain) providing
a typealias or stub compatible with Room for those targets, or alternatively
update GroupTypeConverters.kt to use the legacy stub package
(com.mifos.room.utils.TypeConverter) behind a platform-specific source set so
JS/Wasm compile paths don't import the android-specific annotation; locate the
expect in Room.kt and ensure corresponding actual typealiases are created for
JS/Wasm or adjust imports in GroupTypeConverters.kt to conditional platform
sources to avoid importing template.core.base.database.TypeConverter on JS/Wasm.
In `@feature/groups/build.gradle.kts`:
- Line 42: Remove the unused dependency
implementation(libs.androidx.compose.material.iconsExtended) from the module's
Gradle KTS so the large, unused material-icons-extended artifact is not pulled
in; after removing that line, run a Gradle sync/build and search for any
references to material.icons.extended (or symbols that would come exclusively
from that artifact) to confirm nothing else breaks, and ensure all icon usages
rely on the standard Material Icons (e.g., MifosIcons,
Icons.Default/Filled/Outlined/Rounded).
🧹 Nitpick comments (4)
core-base/designsystem/src/commonMain/kotlin/template/core/base/designsystem/theme/KptColorSchemeImpl.kt (1)
258-310:KptColorSchemeBuilderdoesn't expose the 12 new fixed color properties.The builder has no
varfields for the new tokens (primaryFixed,secondaryFixed,tertiaryFixed, etc.), so users of thekptTheme { colors { … } }DSL cannot customize them. They'll silently fall back to theKptColorSchemeImplconstructor defaults.This is fine to defer given the scope of this sync PR, but worth tracking so the builder stays in sync with the interface. Based on learnings, functional fixes in synced template code are intentionally deferred to follow-up issues.
Would you like me to open a follow-up issue to add the missing builder properties?
core/database/src/commonMain/kotlin/com/mifos/room/entities/PaymentTypeOptionEntity.kt (1)
36-51: Consider removing commented-out code.This block of dead code (
Comparableimplementation,toString) adds noise. If it's no longer needed, remove it; if it's planned for future use, track it in an issue instead.build-logic/convention/src/main/kotlin/org/mifos/KotlinMultiplatform.kt (1)
30-31: Clean up redundant opt-in flags for Kotlin 2.2.21.
kotlin.RequiresOptInhas been stable since Kotlin 1.7; opting in to it is unnecessary in Kotlin 2.2.21.kotlin.time.ExperimentalTimestill exists in the Kotlin standard library and remains a valid opt-in flag (though may be unnecessary depending on usage). The annotation was not removed; what changed in Kotlin 1.6.0 was that theDurationAPI itself became stable.Remove these flags if the code doesn't rely on experimental time APIs.
core-base/database/src/nonJsCommonMain/kotlin/template/core/base/database/Room.nonJsCommon.kt (1)
146-162: Minor inconsistency: FQN used forBuiltInTypeConvertersandAutoMigrationwhile other typealiases use imported short names.Lines 150 and 162 use fully qualified
androidx.room.BuiltInTypeConvertersandandroidx.room.AutoMigration, while all other typealiases reference their short imported names. Adding the missing imports would make the file consistent.Suggested diff for consistency
import androidx.room.ColumnInfo import androidx.room.Dao import androidx.room.Database import androidx.room.DatabaseView import androidx.room.Delete import androidx.room.Embedded import androidx.room.Entity import androidx.room.ForeignKey import androidx.room.Ignore import androidx.room.Index import androidx.room.Insert import androidx.room.Junction import androidx.room.PrimaryKey import androidx.room.Query import androidx.room.Relation import androidx.room.Transaction import androidx.room.TypeConverter import androidx.room.TypeConverters import androidx.room.Update import androidx.room.Upsert +import androidx.room.AutoMigration +import androidx.room.BuiltInTypeConvertersThen:
-actual typealias BuiltInTypeConverters = androidx.room.BuiltInTypeConverters +actual typealias BuiltInTypeConverters = BuiltInTypeConverters-actual typealias AutoMigration = androidx.room.AutoMigration +actual typealias AutoMigration = AutoMigration
core/database/src/commonMain/kotlin/com/mifos/room/di/DatabaseModule.common.kt
Show resolved
Hide resolved
core/database/src/commonMain/kotlin/com/mifos/room/entities/organisation/OfficeEntity.kt
Show resolved
Hide resolved
core/database/src/commonMain/kotlin/com/mifos/room/typeconverters/GroupTypeConverters.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@core/database/src/commonMain/kotlin/com/mifos/room/entities/client/ClientAddressEntity.kt`:
- Around line 12-17: The imports in ClientAddressEntity.kt are inconsistent with
the migrated annotation package: replace or align the constants usage so
annotations in template.core.base.database compile—either (A) update the code
that references UNDEFINED, UNSPECIFIED and VALUE_UNSPECIFIED to use the migrated
object-qualified names (e.g., ColumnInfoTypeAffinity.UNDEFINED,
CollationSequence.UNSPECIFIED) where relevant and remove the
com.mifos.room.utils imports, or (B) add plain exported constants to
template.core.base.database (define VALUE_UNSPECIFIED and re-export UNDEFINED
and UNSPECIFIED at package level) so the current unqualified imports work;
locate usages around the ColumnInfo annotation and PrimaryKey/Entity
declarations in ClientAddressEntity.kt and apply one consistent approach across
those annotations.
core/database/src/commonMain/kotlin/com/mifos/room/entities/client/ClientAddressEntity.kt
Show resolved
Hide resolved
|
There was a problem hiding this comment.
I’m not sure this file should be part of this PR. Even if it is required, the versioning changes don’t seem to make sense.
There was a problem hiding this comment.
I haven't touched this file part though. No idea how it get edited. Anyways I will revert this
| targetSdk=36 | ||
| compileSdk=36 | ||
|
|
||
| android.experimental.lint.version=8.8.2 |
There was a problem hiding this comment.
Removing this could break a workflow - Android build and publish.
2 participants
Fixes - Jira-#660
This PR includes -
kmp-templatekmp-templateScreenshots
Screen_recording_20260213_111754.mp4
Please make sure these boxes are checked before submitting your pull request - thanks!
Run the static analysis check
./gradlew checkorci-prepush.shto make sure you didn't break anythingIf you have multiple commits please combine them into one commit by squashing them.
Summary by CodeRabbit
New Features
Documentation
Chores