Fix Room implementation and Koin DatabaseModule integration#127
Fix Room implementation and Koin DatabaseModule integration#127niyajali merged 1 commit intoopenMF:devfrom
Conversation
📝 WalkthroughWalkthroughThe PR expands Room annotation signatures across the multiplatform database layer by adding new parameters to annotations (Query, Insert, PrimaryKey, ForeignKey, ColumnInfo, Embedded, Relation, Junction), introduces platform-specific annotation implementations for JS and WasmJS targets, removes TypeConverters configuration from the core database, and eliminates NO_ACTUAL_FOR_EXPECT suppressions while adding OptionalExpectation metadata. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ 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 |
3923d0f to
a1fa422
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
core/database/src/wasmJsMain/kotlin/org/mifos/core/database/di/DatabaseModule.wasmJs.kt (1)
14-16: Same missing explicit type annotation as the JS counterpart.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/database/src/wasmJsMain/kotlin/org/mifos/core/database/di/DatabaseModule.wasmJs.kt` around lines 14 - 16, The actual declaration platformModule in DatabaseModule.wasmJs.kt lacks the explicit type annotation present in the JS counterpart; update the declaration for platformModule to include the explicit Koin Module type (e.g., ": Module" or the fully qualified type) so it matches the expected signature used elsewhere (ensure any necessary import for Module is added) and keep the body as-is (module { /* EMPTY — no DB on WasmJs */ }).
🧹 Nitpick comments (1)
core/database/src/jsMain/kotlin/org/mifos/core/database/di/DatabaseModule.js.kt (1)
14-16: Add explicit type annotation toactual val platformModulefor consistency.The
expectdeclaration in commonMain explicitly specifiesModuleas the type. Most platform implementations (android, desktop, native) include this type annotation, but jsMain is missing it. Adding it improves clarity and maintains consistency across all platform targets.♻️ Proposed refactor
-actual val platformModule = module { +actual val platformModule: Module = module {Ensure the import is present:
import org.koin.core.module.Module🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/database/src/jsMain/kotlin/org/mifos/core/database/di/DatabaseModule.js.kt` around lines 14 - 16, The JS implementation of the expected declaration is missing an explicit type: change the declaration of actual val platformModule to include the Module type (i.e., actual val platformModule: Module = module { /* EMPTY — no DB on Js */ }) and ensure the file imports org.koin.core.module.Module so the type resolves; update the declaration for the symbol platformModule in DatabaseModule.js.kt accordingly.
🤖 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/database/src/jsMain/kotlin/org/mifos/core/database/di/DatabaseModule.js.kt`:
- Line 8: Fix the duplicated word in the top-of-file license comment in
DatabaseModule.js.kt: replace the "See See" occurrence with a single "See" in
the license header comment (the comment near the file header/license block) to
correct the typo while preserving the rest of the license text and formatting.
In
`@core/database/src/wasmJsMain/kotlin/org/mifos/core/database/di/DatabaseModule.wasmJs.kt`:
- Line 8: The license header in DatabaseModule.wasmJs.kt contains a duplicated
word "See See"; update the top-of-file comment to read "See" (replace "See See"
with "See") so the license line matches the JS counterpart and corrects the
typo.
---
Duplicate comments:
In
`@core/database/src/wasmJsMain/kotlin/org/mifos/core/database/di/DatabaseModule.wasmJs.kt`:
- Around line 14-16: The actual declaration platformModule in
DatabaseModule.wasmJs.kt lacks the explicit type annotation present in the JS
counterpart; update the declaration for platformModule to include the explicit
Koin Module type (e.g., ": Module" or the fully qualified type) so it matches
the expected signature used elsewhere (ensure any necessary import for Module is
added) and keep the body as-is (module { /* EMPTY — no DB on WasmJs */ }).
---
Nitpick comments:
In
`@core/database/src/jsMain/kotlin/org/mifos/core/database/di/DatabaseModule.js.kt`:
- Around line 14-16: The JS implementation of the expected declaration is
missing an explicit type: change the declaration of actual val platformModule to
include the Module type (i.e., actual val platformModule: Module = module { /*
EMPTY — no DB on Js */ }) and ensure the file imports
org.koin.core.module.Module so the type resolves; update the declaration for the
symbol platformModule in DatabaseModule.js.kt accordingly.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
core-base/database/src/commonMain/kotlin/template/core/base/database/Room.ktcore-base/database/src/jsMain/kotlin/template/core/base/database/Room.js.ktcore-base/database/src/wasmJsMain/kotlin/template/core/base/database/Room.wasmJs.ktcore/database/src/commonMain/kotlin/org/mifos/core/database/AppDatabase.ktcore/database/src/commonMain/kotlin/org/mifos/core/database/di/DatabaseModule.ktcore/database/src/jsMain/kotlin/org/mifos/core/database/di/DatabaseModule.js.ktcore/database/src/wasmJsMain/kotlin/org/mifos/core/database/di/DatabaseModule.wasmJs.kt
💤 Files with no reviewable changes (2)
- core/database/src/commonMain/kotlin/org/mifos/core/database/AppDatabase.kt
- core/database/src/commonMain/kotlin/org/mifos/core/database/di/DatabaseModule.kt
| * License, v. 2.0. If a copy of the MPL was not distributed with this | ||
| * file, You can obtain one at https://mozilla.org/MPL/2.0/. | ||
| * | ||
| * See See https://github.com/openMF/kmp-project-template/blob/main/LICENSE |
There was a problem hiding this comment.
Duplicate word in license comment.
See See should be See.
✏️ Proposed fix
- * See See https://github.com/openMF/kmp-project-template/blob/main/LICENSE
+ * See https://github.com/openMF/kmp-project-template/blob/main/LICENSE📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| * See See https://github.com/openMF/kmp-project-template/blob/main/LICENSE | |
| * See https://github.com/openMF/kmp-project-template/blob/main/LICENSE |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@core/database/src/jsMain/kotlin/org/mifos/core/database/di/DatabaseModule.js.kt`
at line 8, Fix the duplicated word in the top-of-file license comment in
DatabaseModule.js.kt: replace the "See See" occurrence with a single "See" in
the license header comment (the comment near the file header/license block) to
correct the typo while preserving the rest of the license text and formatting.
| * License, v. 2.0. If a copy of the MPL was not distributed with this | ||
| * file, You can obtain one at https://mozilla.org/MPL/2.0/. | ||
| * | ||
| * See See https://github.com/openMF/kmp-project-template/blob/main/LICENSE |
There was a problem hiding this comment.
Same duplicate-word typo in license comment as in the JS counterpart.
See See → See.
✏️ Proposed fix
- * See See https://github.com/openMF/kmp-project-template/blob/main/LICENSE
+ * See https://github.com/openMF/kmp-project-template/blob/main/LICENSE📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| * See See https://github.com/openMF/kmp-project-template/blob/main/LICENSE | |
| * See https://github.com/openMF/kmp-project-template/blob/main/LICENSE |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@core/database/src/wasmJsMain/kotlin/org/mifos/core/database/di/DatabaseModule.wasmJs.kt`
at line 8, The license header in DatabaseModule.wasmJs.kt contains a duplicated
word "See See"; update the top-of-file comment to read "See" (replace "See See"
with "See") so the license line matches the JS counterpart and corrects the
typo.
|
@niyajali please review and merge it |
Fixes - Jira-#KMPPT-92
I added these files because declaration annotated with
@OptionalExpectationcan only be used inside an annotation entry.Marks an
@OptionalExpectationannotation class that it isn't required to have actual counterparts in all platforms.core-base/database/src/jsMain/kotlin/template/core/base/database/Room.js.kt
core-base/database/src/wasmJsMain/kotlin/template/core/base/database/Room.wasmJs.kt
Summary by CodeRabbit
New Features
Refactor