chore: update versions parity with kmp-project-template#2604
chore: update versions parity with kmp-project-template#2604niyajali merged 17 commits intoopenMF:developmentfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughCentralizes Room annotation/types into Changes
Sequence Diagram(s)sequenceDiagram
participant Platform as Platform (Android/Desktop/Native)
participant Module as Consumer Module
participant AppFactory as AppDatabaseFactory
participant Driver as BundledSQLiteDriver / SQLiteConnection
participant DB as MifosDatabase
Platform->>Module: request database instance (e.g., provide DI Module)
Module->>AppFactory: AppDatabaseFactory.createDatabase<MifosDatabase>(Constants.DATABASE_NAME)
AppFactory->>Driver: setDriver(BundledSQLiteDriver())
AppFactory->>AppFactory: setQueryCoroutineContext(ioContext)
AppFactory->>AppFactory: apply migrations / builder config
AppFactory->>Driver: build() -> returns SQLiteConnection-backed DB
AppFactory-->>Module: returns built MifosDatabase instance
Module-->>Platform: provides DB to consumers
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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
Outdated
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
Outdated
Show resolved
Hide resolved
…om/amanna13/android-client into chore/update-dependencies-versions
...src/commonMain/kotlin/com/mifos/room/entities/templates/loans/LoanRepaymentTemplateEntity.kt
Outdated
Show resolved
Hide resolved
core/database/src/desktopMain/kotlin/com/mifos/room/di/DatabaseModule.desktop.kt
Show resolved
Hide resolved
biplab1
left a comment
biplab1
left a comment
There was a problem hiding this comment.
Some changes and comments have been requested. Please review.
core-base/database/src/nonJsCommonMain/kotlin/template/core/base/database/Room.nonJsCommon.kt
Outdated
Show resolved
Hide resolved
core-base/database/src/nonJsCommonMain/kotlin/template/core/base/database/Room.nonJsCommon.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
core/database/src/commonMain/kotlin/com/mifos/room/entities/accounts/loans/LoanAccountEntity.kt (1)
28-44:⚠️ Potential issue | 🟠 MajorForeign key constraints cannot work with TypeConverted entity fields.
The
statusandloanTypefields store completeLoanStatusEntityandLoanTypeEntityobjects serialized as JSON strings (viaCustomTypeConverters), not integer IDs. Foreign key constraints require the child column to hold the same type as the parent's primary key (Int?). The constraint will fail to match a JSON string against an integerid, causing insert/update failures.To fix this, either:
- Remove the foreign key constraints (if denormalization is intentional)
- Change
statusandloanTypeto simpleInt?fields storing IDs, then use@Relationannotations to load the related entities separately- Use
@Embeddedannotations if these should be nested objects without foreign key enforcementThis also applies to lines 73-77.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/database/src/commonMain/kotlin/com/mifos/room/entities/accounts/loans/LoanAccountEntity.kt` around lines 28 - 44, LoanAccountEntity currently declares foreignKeys referencing LoanStatusEntity and LoanTypeEntity while the entity stores those fields as JSON-serialized objects via CustomTypeConverters (status and loanType), so the FK will never match the parent integer IDs; to fix, either remove the ForeignKey entries for LoanStatusEntity/LoanTypeEntity from the foreignKeys array (if you intend denormalized JSON), or change the LoanAccountEntity fields 'status' and 'loanType' to Int? (IDs) and remove the CustomTypeConverters for those fields, then load related LoanStatusEntity/LoanTypeEntity via `@Relation` in a separate data class, or alternatively convert those fields to `@Embedded` nested objects (and remove the foreign keys) if you want them stored as nested columns; update DAOs and any serialization code accordingly (also apply the same change to the other fields mentioned at lines 73-77).core/database/src/commonMain/kotlin/com/mifos/room/entities/group/GroupEntity.kt (1)
34-42:⚠️ Potential issue | 🔴 CriticalRemove or redesign the foreign key constraint on
groupDate.The field
groupDate: GroupDateEntity?is serialized to JSON (String) via TypeConverter, but the foreign key tries to referenceGroupDateEntity.groupId(Long). This type mismatch will cause runtime failures. Either:
- Remove the foreign key constraint entirely if referential integrity isn't needed, or
- Store a
groupId: Longfield directly inGroupEntityand reference that instead of the complex entity object.Additionally,
GroupEntitylacks a@TypeConvertersannotation on its@Entitydeclaration to enable the converters inGroupTypeConverters.This applies to lines 34-42 and 60-61.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/database/src/commonMain/kotlin/com/mifos/room/entities/group/GroupEntity.kt` around lines 34 - 42, The foreign key on GroupEntity referencing GroupDateEntity via the serialized field groupDate is invalid; either remove the ForeignKey block that references GroupDateEntity (delete the ForeignKey entry that uses parentColumns = ["groupId"] and childColumns = ["groupDate"]) or redesign GroupEntity to store a plain groupId: Long property and change the ForeignKey to reference that childColumns = ["groupId"]; additionally annotate the GroupEntity `@Entity` with `@TypeConverters`(GroupTypeConverters::class) so the groupDate: GroupDateEntity? property can be serialized/deserialized correctly if you keep it. Ensure you update the entity fields and foreign key metadata to match the actual column types (groupId Long vs groupDate String) and keep consistency between the property names and referenced columns.
🧹 Nitpick comments (4)
core/database/src/commonMain/kotlin/com/mifos/room/entities/group/GroupDateEntity.kt (2)
34-36:index = trueon@ColumnInfois redundant for a@PrimaryKeyfield.Primary keys are automatically indexed by Room/SQLite. Setting
index = trueexplicitly on a@PrimaryKeycolumn has no effect and adds noise. Again, likely a template-sync artifact, so this is a nit.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/database/src/commonMain/kotlin/com/mifos/room/entities/group/GroupDateEntity.kt` around lines 34 - 36, The `@ColumnInfo` on the GroupDateEntity primary key field groupId redundantly sets index = true; remove the unnecessary index attribute from the `@ColumnInfo` annotation for groupId (the primary key is automatically indexed by Room/SQLite) and keep the other attributes (name = INHERIT_FIELD_NAME, typeAffinity, collate, defaultValue) intact to eliminate the noisy template artifact.
25-32: Verbose explicit defaults on@Entity— acceptable for template sync.All parameters (
indices = [],inheritSuperIndices = false,primaryKeys = [],foreignKeys = [],ignoredColumns = []) are Room defaults. This is noisy but likely intentional to match the kmp-project-template pattern. Based on learnings, functional or behavioral fixes in synced template code are intentionally deferred to follow-up issues.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/database/src/commonMain/kotlin/com/mifos/room/entities/group/GroupDateEntity.kt` around lines 25 - 32, The `@Entity` annotation on GroupDateEntity (the `@Entity`(...) block) is needlessly explicit by specifying default parameters (indices, inheritSuperIndices, primaryKeys, foreignKeys, ignoredColumns); remove those redundant named parameters so the annotation just uses the defaults (e.g., `@Entity`(tableName = "GroupDate")) to reduce noise, unless the project template requires the explicit form—if the template does require it, leave as-is and add a comment noting it's intentionally verbose for template sync.core-base/database/src/commonMain/kotlin/template/core/base/database/Room.kt (1)
752-773:ColumnInfoTypeAffinitymixes column-affinity ints with unrelated string constants.
INHERIT_FIELD_NAMEandVALUE_UNSPECIFIEDare string constants that correspond toColumnInfo.INHERIT_FIELD_NAMEandColumnInfo.VALUE_UNSPECIFIEDin Room — they relate to column naming/defaults, not type affinity. Grouping them underColumnInfoTypeAffinityis misleading and will confuse consumers.Consider moving them to a separate object (e.g.,
ColumnInfoDefaults) or directly into a companion on theColumnInfoexpect class.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core-base/database/src/commonMain/kotlin/template/core/base/database/Room.kt` around lines 752 - 773, ColumnInfoTypeAffinity currently mixes integer affinity constants with unrelated string defaults (INHERIT_FIELD_NAME, VALUE_UNSPECIFIED); move these two string constants out of ColumnInfoTypeAffinity into a new container more appropriate for column defaults. Create a new object (e.g., ColumnInfoDefaults) or add them to the companion of the existing ColumnInfo expect class, copy/move INHERIT_FIELD_NAME and VALUE_UNSPECIFIED there, and update any usages to reference the new symbol (ColumnInfoDefaults.INHERIT_FIELD_NAME or ColumnInfo.Companion.INHERIT_FIELD_NAME) while leaving the integer affinity constants in ColumnInfoTypeAffinity unchanged. Ensure no behavior changes by keeping the constant values identical and update imports/usages across the module.core/database/src/commonMain/kotlin/com/mifos/room/entities/accounts/loans/LoanAccountEntity.kt (1)
46-49:indices = [],primaryKeys = [],ignoredColumns = []are redundant defaults.These are the default values for Room's
@Entityannotation. While they don't cause harm, they add visual clutter. This was likely auto-generated by the template sync, so just a minor note.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/database/src/commonMain/kotlin/com/mifos/room/entities/accounts/loans/LoanAccountEntity.kt` around lines 46 - 49, Remove the redundant default attributes from the Room `@Entity` annotation in LoanAccountEntity (the class annotated in LoanAccountEntity.kt): delete indices = [], primaryKeys = [], and ignoredColumns = [] from the annotation so it only declares the non-default parameters (leave other attributes like inheritSuperIndices if intentionally set). This cleans up visual clutter while preserving behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@core-base/database/src/commonMain/kotlin/template/core/base/database/Room.kt`:
- Around line 818-819: The CASCADE constant in Room.kt is incorrectly set to 4
(duplicating SET_DEFAULT) causing ForeignKeyAction.CASCADE to behave as
SET_DEFAULT; update the const val CASCADE from 4 to 5 so it matches
androidx.room.ForeignKey (NO_ACTION=1, RESTRICT=2, SET_NULL=3, SET_DEFAULT=4,
CASCADE=5) and ensure any uses of CASCADE/ForeignKeyAction.CASCADE in entity
definitions now map to the correct value.
---
Outside diff comments:
In
`@core/database/src/commonMain/kotlin/com/mifos/room/entities/accounts/loans/LoanAccountEntity.kt`:
- Around line 28-44: LoanAccountEntity currently declares foreignKeys
referencing LoanStatusEntity and LoanTypeEntity while the entity stores those
fields as JSON-serialized objects via CustomTypeConverters (status and
loanType), so the FK will never match the parent integer IDs; to fix, either
remove the ForeignKey entries for LoanStatusEntity/LoanTypeEntity from the
foreignKeys array (if you intend denormalized JSON), or change the
LoanAccountEntity fields 'status' and 'loanType' to Int? (IDs) and remove the
CustomTypeConverters for those fields, then load related
LoanStatusEntity/LoanTypeEntity via `@Relation` in a separate data class, or
alternatively convert those fields to `@Embedded` nested objects (and remove the
foreign keys) if you want them stored as nested columns; update DAOs and any
serialization code accordingly (also apply the same change to the other fields
mentioned at lines 73-77).
In
`@core/database/src/commonMain/kotlin/com/mifos/room/entities/group/GroupEntity.kt`:
- Around line 34-42: The foreign key on GroupEntity referencing GroupDateEntity
via the serialized field groupDate is invalid; either remove the ForeignKey
block that references GroupDateEntity (delete the ForeignKey entry that uses
parentColumns = ["groupId"] and childColumns = ["groupDate"]) or redesign
GroupEntity to store a plain groupId: Long property and change the ForeignKey to
reference that childColumns = ["groupId"]; additionally annotate the GroupEntity
`@Entity` with `@TypeConverters`(GroupTypeConverters::class) so the groupDate:
GroupDateEntity? property can be serialized/deserialized correctly if you keep
it. Ensure you update the entity fields and foreign key metadata to match the
actual column types (groupId Long vs groupDate String) and keep consistency
between the property names and referenced columns.
---
Nitpick comments:
In
`@core-base/database/src/commonMain/kotlin/template/core/base/database/Room.kt`:
- Around line 752-773: ColumnInfoTypeAffinity currently mixes integer affinity
constants with unrelated string defaults (INHERIT_FIELD_NAME,
VALUE_UNSPECIFIED); move these two string constants out of
ColumnInfoTypeAffinity into a new container more appropriate for column
defaults. Create a new object (e.g., ColumnInfoDefaults) or add them to the
companion of the existing ColumnInfo expect class, copy/move INHERIT_FIELD_NAME
and VALUE_UNSPECIFIED there, and update any usages to reference the new symbol
(ColumnInfoDefaults.INHERIT_FIELD_NAME or
ColumnInfo.Companion.INHERIT_FIELD_NAME) while leaving the integer affinity
constants in ColumnInfoTypeAffinity unchanged. Ensure no behavior changes by
keeping the constant values identical and update imports/usages across the
module.
In
`@core/database/src/commonMain/kotlin/com/mifos/room/entities/accounts/loans/LoanAccountEntity.kt`:
- Around line 46-49: Remove the redundant default attributes from the Room
`@Entity` annotation in LoanAccountEntity (the class annotated in
LoanAccountEntity.kt): delete indices = [], primaryKeys = [], and ignoredColumns
= [] from the annotation so it only declares the non-default parameters (leave
other attributes like inheritSuperIndices if intentionally set). This cleans up
visual clutter while preserving behavior.
In
`@core/database/src/commonMain/kotlin/com/mifos/room/entities/group/GroupDateEntity.kt`:
- Around line 34-36: The `@ColumnInfo` on the GroupDateEntity primary key field
groupId redundantly sets index = true; remove the unnecessary index attribute
from the `@ColumnInfo` annotation for groupId (the primary key is automatically
indexed by Room/SQLite) and keep the other attributes (name =
INHERIT_FIELD_NAME, typeAffinity, collate, defaultValue) intact to eliminate the
noisy template artifact.
- Around line 25-32: The `@Entity` annotation on GroupDateEntity (the `@Entity`(...)
block) is needlessly explicit by specifying default parameters (indices,
inheritSuperIndices, primaryKeys, foreignKeys, ignoredColumns); remove those
redundant named parameters so the annotation just uses the defaults (e.g.,
`@Entity`(tableName = "GroupDate")) to reduce noise, unless the project template
requires the explicit form—if the template does require it, leave as-is and add
a comment noting it's intentionally verbose for template sync.
core-base/database/src/commonMain/kotlin/template/core/base/database/Room.kt
Outdated
Show resolved
Hide resolved
|
@biplab1 I have fixed the requested changes. Please review this. Thanks ! |
…om/amanna13/android-client into chore/update-dependencies-versions
|
|
@biplab1 I have now fixed the bug related to the |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
core-base/database/src/commonMain/kotlin/template/core/base/database/Room.kt (1)
108-135:⚠️ Potential issue | 🟡 MinorKDoc example references
ForeignKey.CASCADE, which doesn't exist in this API.The
ForeignKeyannotation class defined here has no companion constants. The correct cross-platform reference isForeignKeyAction.CASCADE, notForeignKey.CASCADE.📝 Proposed doc fix
- * onDelete = ForeignKey.CASCADE + * onDelete = ForeignKeyAction.CASCADE🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core-base/database/src/commonMain/kotlin/template/core/base/database/Room.kt` around lines 108 - 135, The KDoc example for the expect annotation ForeignKey incorrectly references a nonexistent constant ForeignKey.CASCADE; update the documentation to reference the correct cross-platform enum/constant ForeignKeyAction.CASCADE and any other places in the comment block that refer to ForeignKey.CASCADE so the example compiles and matches the API (look for the expect annotation ForeignKey and replace ForeignKey.CASCADE with ForeignKeyAction.CASCADE in the example snippet).
🧹 Nitpick comments (1)
core-base/database/src/commonMain/kotlin/template/core/base/database/Room.kt (1)
752-773:INHERIT_FIELD_NAMEandVALUE_UNSPECIFIEDare semantically misplaced inColumnInfoTypeAffinity.These two String sentinel values relate to
ColumnInfo'snameanddefaultValueparameters respectively, not to type affinity. In AndroidX Room they are constants on theColumnInfoannotation class itself (ColumnInfo.NAME_INHERIT_FIELD_NAME,ColumnInfo.VALUE_UNSPECIFIED), not on the innerTypeAffinityclass. Grouping them here alongside the numeric type-affinity constants conflates two separate concerns and can confuse callers looking for the default-value or name sentinels.Consider placing them in a dedicated
ColumnInfoConstants(or similar) object, or at least in a separate section of the file to clearly separate them from the type-affinity integers.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core-base/database/src/commonMain/kotlin/template/core/base/database/Room.kt` around lines 752 - 773, The two string sentinels INHERIT_FIELD_NAME and VALUE_UNSPECIFIED are misplaced in the ColumnInfoTypeAffinity object (which should only contain numeric type-affinity constants); move these two constants out of ColumnInfoTypeAffinity and place them in a dedicated container (e.g., ColumnInfoConstants) or attach them to the ColumnInfo-related symbol so they read like ColumnInfo.NAME_INHERIT_FIELD_NAME and ColumnInfo.VALUE_UNSPECIFIED; update any references to INHERIT_FIELD_NAME and VALUE_UNSPECIFIED to point to the new container so type-affinity integers remain isolated in ColumnInfoTypeAffinity and the name/default-value sentinels live with ColumnInfo-related constants.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@core-base/database/src/commonMain/kotlin/template/core/base/database/Room.kt`:
- Around line 108-135: The KDoc example for the expect annotation ForeignKey
incorrectly references a nonexistent constant ForeignKey.CASCADE; update the
documentation to reference the correct cross-platform enum/constant
ForeignKeyAction.CASCADE and any other places in the comment block that refer to
ForeignKey.CASCADE so the example compiles and matches the API (look for the
expect annotation ForeignKey and replace ForeignKey.CASCADE with
ForeignKeyAction.CASCADE in the example snippet).
---
Duplicate comments:
In
`@core-base/database/src/commonMain/kotlin/template/core/base/database/Room.kt`:
- Around line 805-820: The ForeignKeyAction constants in object ForeignKeyAction
had an incorrect mapping for CASCADE and have been updated so they now exactly
match AndroidX Room (ensure SET_DEFAULT = 4 and CASCADE = 5); verify the
constants in ForeignKeyAction (NO_ACTION, RESTRICT, SET_NULL, SET_DEFAULT,
CASCADE) are set to 1, 2, 3, 4, 5 respectively and, if any mismatch remains,
correct those constant values to match Room's definitions.
---
Nitpick comments:
In
`@core-base/database/src/commonMain/kotlin/template/core/base/database/Room.kt`:
- Around line 752-773: The two string sentinels INHERIT_FIELD_NAME and
VALUE_UNSPECIFIED are misplaced in the ColumnInfoTypeAffinity object (which
should only contain numeric type-affinity constants); move these two constants
out of ColumnInfoTypeAffinity and place them in a dedicated container (e.g.,
ColumnInfoConstants) or attach them to the ColumnInfo-related symbol so they
read like ColumnInfo.NAME_INHERIT_FIELD_NAME and ColumnInfo.VALUE_UNSPECIFIED;
update any references to INHERIT_FIELD_NAME and VALUE_UNSPECIFIED to point to
the new container so type-affinity integers remain isolated in
ColumnInfoTypeAffinity and the name/default-value sentinels live with
ColumnInfo-related constants.
|
@amanna13 Just waiting for a confirmation from |
biplab1
left a comment
biplab1
left a comment
There was a problem hiding this comment.
Looks good to me. This can be merged.
|
@niyajali If everything looks good, please merge this. |
|
@amanna13 does the app is building/compiling fine on other platforms? |
|
@biplab1 In both PRs I'm seeing they're changing the codebase in the core-base modules, does this expected? |
|
Okay, let's merge this and that, and we'll see what happens |
3 participants
Fixes - Jira-#660
This PR includes -
kmp-templatekmp-templateScreenshots [updated]
Screen_recording_20260217_233443.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
Bug Fixes
Chores