From 547343992a2938a683d64bf05b97f171fa17df60 Mon Sep 17 00:00:00 2001 From: Achmad Setyabudi Susilo Date: Sun, 28 Jun 2026 16:09:15 +0700 Subject: [PATCH] =?UTF-8?q?fix(#1):=20address=20PR=20review=20=E2=80=94=20?= =?UTF-8?q?split=20upsert=20into=20insert/update,=20add=20@Transaction,=20?= =?UTF-8?q?runBlocking=20seed,=20trailing=20newline?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per review on PR #8 (https://git.achmad.dev/admin/ledgerr/pulls/8): - Split @Upsert into @Insert(OnConflictStrategy.REPLACE) + @Update in all 3 DAOs. @Upsert returns -1 on the update path, so callers wanting the row ID would get a junk value. Interactors now call insert vs update based on id == 0. UpsertCategory returns category.id explicitly for the id != 0 branch. - Add @Transaction to the 3 @Relation queries (ExpenseDao.subscribeAll, ExpenseDao.subscribeByDateRange, RecurringExpenseDao.subscribeAll). This silences the KSP warnings the PR body mentioned and makes the intent explicit. - Switch MainApplication seeding from a fire-and-forget CoroutineScope to runBlocking(Dispatchers.IO). A fast first-tap on HomeScreen could otherwise call GetCategories.awaitDefault() before seeding completed and crash. - Add documenting comment on CategoryDao.getDefault() noting that the 'only one isDefault = 1' invariant is maintained at the interactor layer (partial unique index would be a v2 migration). - Add trailing newline to app/build.gradle.kts. - File follow-up issue #9 for flipping exportSchema to true before any v2 migration lands. --- app/build.gradle.kts | 2 +- .../ledgerr/data/local/dao/CategoryDao.kt | 21 +++++++++++++++---- .../ledgerr/data/local/dao/ExpenseDao.kt | 13 +++++++++--- .../data/local/dao/RecurringExpenseDao.kt | 13 +++++++++--- .../category/interactor/UpsertCategory.kt | 5 +++-- .../achmad/ledgerr/ui/base/MainApplication.kt | 5 ++--- 6 files changed, 43 insertions(+), 16 deletions(-) diff --git a/app/build.gradle.kts b/app/build.gradle.kts index c7dc2ea..ba755b0 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -90,4 +90,4 @@ dependencies { implementation(libs.vico.compose) implementation(libs.vico.compose.m3) implementation(libs.vico.core) -} \ No newline at end of file +} diff --git a/app/src/main/java/dev/achmad/ledgerr/data/local/dao/CategoryDao.kt b/app/src/main/java/dev/achmad/ledgerr/data/local/dao/CategoryDao.kt index 51f78c3..8a182c7 100644 --- a/app/src/main/java/dev/achmad/ledgerr/data/local/dao/CategoryDao.kt +++ b/app/src/main/java/dev/achmad/ledgerr/data/local/dao/CategoryDao.kt @@ -1,8 +1,10 @@ package dev.achmad.ledgerr.data.local.dao import androidx.room.Dao +import androidx.room.Insert +import androidx.room.OnConflictStrategy import androidx.room.Query -import androidx.room.Upsert +import androidx.room.Update import dev.achmad.ledgerr.data.local.entity.CategoryEntity import kotlinx.coroutines.flow.Flow @@ -18,16 +20,27 @@ interface CategoryDao { @Query("SELECT * FROM categories WHERE id = :id") suspend fun getById(id: Long): CategoryEntity? + /** + * Returns the single category marked as the system default. The "Uncategorized" + * category is seeded as the only default on first launch. There is no + * DB-level uniqueness constraint on `isDefault = 1` (a partial unique index + * would be a v2 migration); the invariant is maintained by `SeedDefaultCategories` + * and `UpsertCategory` at the interactor layer. If multiple defaults ever + * sneak in, this returns an arbitrary one. + */ @Query("SELECT * FROM categories WHERE isDefault = 1 LIMIT 1") suspend fun getDefault(): CategoryEntity? @Query("SELECT COUNT(*) FROM categories") suspend fun count(): Int - @Upsert - suspend fun upsert(category: CategoryEntity): Long + @Insert(onConflict = OnConflictStrategy.REPLACE) + suspend fun insert(category: CategoryEntity): Long - @Upsert + @Update + suspend fun update(category: CategoryEntity) + + @Insert(onConflict = OnConflictStrategy.REPLACE) suspend fun upsertAll(categories: List) @Query("DELETE FROM categories WHERE id = :id") diff --git a/app/src/main/java/dev/achmad/ledgerr/data/local/dao/ExpenseDao.kt b/app/src/main/java/dev/achmad/ledgerr/data/local/dao/ExpenseDao.kt index fe2d863..7757b06 100644 --- a/app/src/main/java/dev/achmad/ledgerr/data/local/dao/ExpenseDao.kt +++ b/app/src/main/java/dev/achmad/ledgerr/data/local/dao/ExpenseDao.kt @@ -3,9 +3,11 @@ package dev.achmad.ledgerr.data.local.dao import androidx.room.Dao import androidx.room.Embedded import androidx.room.Insert +import androidx.room.OnConflictStrategy import androidx.room.Query import androidx.room.Relation -import androidx.room.Upsert +import androidx.room.Transaction +import androidx.room.Update import dev.achmad.ledgerr.data.local.entity.CategoryEntity import dev.achmad.ledgerr.data.local.entity.ExpenseEntity import kotlinx.coroutines.flow.Flow @@ -22,9 +24,11 @@ data class ExpenseWithCategoryRow( @Dao interface ExpenseDao { + @Transaction @Query("SELECT * FROM expenses ORDER BY date DESC, createdAt DESC") fun subscribeAll(): Flow> + @Transaction @Query( "SELECT * FROM expenses " + "WHERE date BETWEEN :startDay AND :endDay " + @@ -50,8 +54,11 @@ interface ExpenseDao { ) suspend fun getByDateRange(startDay: Long, endDay: Long): List - @Upsert - suspend fun upsert(expense: ExpenseEntity): Long + @Insert(onConflict = OnConflictStrategy.REPLACE) + suspend fun insert(expense: ExpenseEntity): Long + + @Update + suspend fun update(expense: ExpenseEntity) @Insert suspend fun insertAll(expenses: List) diff --git a/app/src/main/java/dev/achmad/ledgerr/data/local/dao/RecurringExpenseDao.kt b/app/src/main/java/dev/achmad/ledgerr/data/local/dao/RecurringExpenseDao.kt index d82fb0a..421608f 100644 --- a/app/src/main/java/dev/achmad/ledgerr/data/local/dao/RecurringExpenseDao.kt +++ b/app/src/main/java/dev/achmad/ledgerr/data/local/dao/RecurringExpenseDao.kt @@ -2,9 +2,12 @@ package dev.achmad.ledgerr.data.local.dao import androidx.room.Dao import androidx.room.Embedded +import androidx.room.Insert +import androidx.room.OnConflictStrategy import androidx.room.Query import androidx.room.Relation -import androidx.room.Upsert +import androidx.room.Transaction +import androidx.room.Update import dev.achmad.ledgerr.data.local.entity.CategoryEntity import dev.achmad.ledgerr.data.local.entity.RecurringExpenseEntity import kotlinx.coroutines.flow.Flow @@ -21,6 +24,7 @@ data class RecurringExpenseWithCategoryRow( @Dao interface RecurringExpenseDao { + @Transaction @Query("SELECT * FROM recurring_expenses ORDER BY nextDueDate ASC") fun subscribeAll(): Flow> @@ -30,8 +34,11 @@ interface RecurringExpenseDao { @Query("SELECT * FROM recurring_expenses WHERE isActive = 1 AND nextDueDate <= :today") suspend fun getDue(today: Long): List - @Upsert - suspend fun upsert(recurring: RecurringExpenseEntity): Long + @Insert(onConflict = OnConflictStrategy.REPLACE) + suspend fun insert(recurring: RecurringExpenseEntity): Long + + @Update + suspend fun update(recurring: RecurringExpenseEntity) @Query("DELETE FROM recurring_expenses WHERE id = :id") suspend fun deleteById(id: Long) diff --git a/app/src/main/java/dev/achmad/ledgerr/domain/category/interactor/UpsertCategory.kt b/app/src/main/java/dev/achmad/ledgerr/domain/category/interactor/UpsertCategory.kt index a3191b3..82ff60f 100644 --- a/app/src/main/java/dev/achmad/ledgerr/domain/category/interactor/UpsertCategory.kt +++ b/app/src/main/java/dev/achmad/ledgerr/domain/category/interactor/UpsertCategory.kt @@ -9,7 +9,7 @@ class UpsertCategory( ) { suspend fun await(category: Category): Long { if (category.id == 0L) { - return dao.upsert(category.toEntity()) + return dao.insert(category.toEntity()) } val existing = dao.getById(category.id) val resolved = if (existing != null && category.isDefault && !existing.isDefault) { @@ -17,6 +17,7 @@ class UpsertCategory( } else { category } - return dao.upsert(resolved.toEntity()) + dao.update(resolved.toEntity()) + return category.id } } diff --git a/app/src/main/java/dev/achmad/ledgerr/ui/base/MainApplication.kt b/app/src/main/java/dev/achmad/ledgerr/ui/base/MainApplication.kt index bcfc67c..6adc97a 100644 --- a/app/src/main/java/dev/achmad/ledgerr/ui/base/MainApplication.kt +++ b/app/src/main/java/dev/achmad/ledgerr/ui/base/MainApplication.kt @@ -7,9 +7,8 @@ import dev.achmad.ledgerr.di.domainModule import dev.achmad.ledgerr.di.preferenceModule import dev.achmad.ledgerr.di.util.inject import dev.achmad.ledgerr.domain.category.interactor.SeedDefaultCategories -import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.launch +import kotlinx.coroutines.runBlocking import org.koin.android.ext.koin.androidContext import org.koin.android.ext.koin.androidLogger import org.koin.core.context.startKoin @@ -23,7 +22,7 @@ class MainApplication : Application() { androidContext(this@MainApplication) modules(coreModule, dataModule, domainModule, preferenceModule) } - CoroutineScope(Dispatchers.IO).launch { + runBlocking(Dispatchers.IO) { inject().await() } }