fix: firebase upload failure in KMP demo app workflow#3112
Open
amanna13 wants to merge 1 commit intoopenMF:developmentfrom
Open
fix: firebase upload failure in KMP demo app workflow#3112amanna13 wants to merge 1 commit intoopenMF:developmentfrom
amanna13 wants to merge 1 commit intoopenMF:developmentfrom
Conversation
Contributor
📝 WalkthroughWalkthroughThe pull request refactors database abstractions from platform-specific implementations into a shared core-base module using Kotlin's expect/actual pattern. Room annotations are consolidated into expect declarations with nonJsCommon typealiases, platform factories are consolidated, and all database-dependent modules update imports to reference the centralized abstraction layer. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
core-base/analytics/src/commonMain/kotlin/template/core/base/analytics/PerformanceTracker.kt (1)
28-50:⚠️ Potential issue | 🟠 MajorFix
currentTimebeing constant (breaks all timing).
private val currentTime = Clock.System.now().toEpochMilliseconds()is initialized once, so all timers, durations, and lifecycle timestamps use the same value. This yields 0/negative durations and duplicate timer IDs. Replace with a function/property that reads the clock each time.🔧 Proposed fix
-private val currentTime = Clock.System.now().toEpochMilliseconds() +private fun currentTimeMillis(): Long = Clock.System.now().toEpochMilliseconds()- val timerId = "${operationName}_$currentTime" - activeTimers[timerId] = currentTime + val now = currentTimeMillis() + val timerId = "${operationName}_$now" + activeTimers[timerId] = now- val duration = currentTime - startTime + val duration = currentTimeMillis() - startTime- appStartTime = currentTime + appStartTime = currentTimeMillis()- val launchDuration = currentTime - startTime + val launchDuration = currentTimeMillis() - startTime- backgroundTime = currentTime + backgroundTime = currentTimeMillis()- val currentTime = currentTime + val currentTime = currentTimeMillis()Also applies to: 188-240, 265-265
core/database/build.gradle.kts (1)
1-19:⚠️ Potential issue | 🟡 MinorDuplicate copyright headers.
The file contains two consecutive copyright blocks (2026 on Lines 1–9, 2024 on Lines 11–19). Remove one of them.
Proposed fix
/* * Copyright 2026 Mifos Initiative * * This Source Code Form is subject to the terms of the Mozilla Public * 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 https://github.com/openMF/mobile-mobile/blob/master/LICENSE.md */ - -/* - * Copyright 2024 Mifos Initiative - * - * This Source Code Form is subject to the terms of the Mozilla Public - * 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 https://github.com/openMF/mobile-mobile/blob/master/LICENSE.md - */ plugins {
🤖 Fix all issues with AI agents
In `@build-logic/convention/src/main/kotlin/org/mifos/mobile/KotlinAndroid.kt`:
- Around line 71-73: Remove the incorrect opt-in flag added via
freeCompilerArgs.add("-opt-in=kotlin.RequiresOptIn") in KotlinAndroid.kt (the
meta-annotation has no effect as an opt-in) and delete that line; also review
the freeCompilerArgs.add("-opt-in=kotlin.time.ExperimentalTime") entry and
remove it if your codebase no longer uses legacy `@ExperimentalTime` annotations
or if Kotlin 2.1.20/1.9+ makes those APIs stable—search for freeCompilerArgs.add
invocations in the KotlinAndroid.kt setup and update those specific lines
accordingly.
In `@cmp-shared/cmp_shared.podspec`:
- Line 53: The podspec currently uses a Windows-style path in the spec.resources
string ('build\compose\cocoapods\compose-resources'); change that to a
POSIX-style path with forward slashes (e.g.,
'build/compose/cocoapods/compose-resources') so CocoaPods on macOS can correctly
discover resources; update the spec.resources assignment in cmp_shared.podspec
(the spec.resources = ... line) to use the forward-slash path.
In `@core-base/database/README.md`:
- Around line 613-615: The README's license reference incorrectly points to the
template repo URL
"https://github.com/openMF/kmp-project-template/blob/main/LICENSE"; update that
link in core-base/database/README.md to point to this project's LICENSE (replace
the kmp-project-template URL with the correct mifos-mobile repository license
URL, e.g. "https://github.com/openMF/mifos-mobile/blob/main/LICENSE") so the
license link reflects this repository.
- Line 534: The README incorrectly references Room version and API level: update
any mention of "androidx.room.runtime (2.8.4+)" to the actual configured version
"2.7.2" (and note that 2.8.x would require API 23 if you want to call that out),
and change the minimum API statement "API 16+" to the correct minimum for 2.7.2
which is "API 21"; search the README for other Room version/API mentions and
align them with gradle/libs.versions.toml (or annotate that newer 2.8.x requires
API 23) so documentation and actual dependency config match.
In
`@core-base/database/src/commonMain/kotlin/template/core/base/database/Room.kt`:
- Around line 670-676: The expect annotation class Ignore currently lists only
AnnotationTarget.FIELD and AnnotationTarget.CONSTRUCTOR which prevents using
`@Ignore` on getters and functions; update the target list for the expect
annotation class Ignore to also include AnnotationTarget.FUNCTION and
AnnotationTarget.PROPERTY_GETTER so it matches androidx.room.Ignore (i.e.,
modify the AnnotationTarget(...) block in the expect annotation class Ignore
declaration).
In
`@core/database/src/commonMain/kotlin/org/mifos/mobile/core/database/dao/ChargeDao.kt`:
- Around line 13-17: The import of OnConflictStrategy in ChargeDao.kt should be
changed to use the centralized definition in the base module to match the other
Room abstractions; update the import for OnConflictStrategy to come from
template.core.base.database (same module as Dao, Insert, Query) so that
ChargeDao references template.core.base.database.OnConflictStrategy instead of
org.mifos.mobile.core.database.OnConflictStrategy.
🧹 Nitpick comments (1)
core-base/database/src/commonMain/kotlin/template/core/base/database/Room.kt (1)
10-10: File-level suppression is redundant with per-declaration suppressions.Every
expectdeclaration in this file already carries its own@Suppress("NO_ACTUAL_FOR_EXPECT"). The file-level@file:Suppress("KotlinNoActualForExpect")suppresses a slightly different warning key (KotlinNoActualForExpectvsNO_ACTUAL_FOR_EXPECT). If the intent is to cover both warning variants, this is fine, but worth a brief inline comment explaining why both suppression keys are needed.
build-logic/convention/src/main/kotlin/org/mifos/mobile/KotlinAndroid.kt
Show resolved
Hide resolved
core-base/database/src/commonMain/kotlin/template/core/base/database/Room.kt
Show resolved
Hide resolved
core/database/src/commonMain/kotlin/org/mifos/mobile/core/database/dao/ChargeDao.kt
Show resolved
Hide resolved
amanna13
force-pushed
the
fix/firebase-upload-failure
branch
from
576c501 to
4abf61a
Compare
Contributor
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/database/build.gradle.kts (1)
1-19:⚠️ Potential issue | 🟡 MinorDuplicate copyright header block.
The file contains two consecutive copyright headers (lines 1–9 and 11–19). Remove one.
Proposed fix
-/* - * Copyright 2026 Mifos Initiative - * - * This Source Code Form is subject to the terms of the Mozilla Public - * 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 https://github.com/openMF/mobile-mobile/blob/master/LICENSE.md - */ - /* - * Copyright 2024 Mifos Initiative + * Copyright 2026 Mifos Initiative * * This Source Code Form is subject to the terms of the Mozilla Public * License, v. 2.0. If a copy of the MPL was not distributed with this
🧹 Nitpick comments (1)
core-base/database/src/commonMain/kotlin/template/core/base/database/Room.kt (1)
567-570:BuiltInTypeConvertersexpect has no parameters — intentional limitation in common code.The actual
androidx.room.BuiltInTypeConvertershasenumsanduuidparameters. The zero-arg expect means common code can only use defaults. If fine-grained control is needed in shared code later, parameters will need to be added here.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes - Jira-#534
Screenshots
by running ./gradlew assembleDemoRelease
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
Release Notes
New Features
Documentation
Refactor