Implement expense interactors (#2) #11

2026-06-28T09:33:47Z

admin commented

2026-06-28 09:33:47 +00:00

Summary

Implement 5 expense interactors: GetExpenses (4 methods), UpsertExpense, InsertExpenses, DeleteExpense, GetExpenseSummary (ReassignExpenseCategory was already implemented on main)
Add ExpenseWithCategoryRow.toModel() mapper for the joined DAO row (used by both subscribe* methods and awaitOne)
Register the 5 new interactors as factory { } in DomainModule
InsertExpenses.awaitAll enforces id == 0 via require before delegating to the plain @Insert
Fix docs/03-function-todos.md doc drift (UpsertExpense routes through separate insert / update, not @Upsert)
Note in GetExpenseSummary explaining the orphan-category drop behavior

Test plan

./gradlew assembleDebug succeeds
GetExpenses.awaitOne returns null when either the expense or its category row is missing
InsertExpenses.awaitAll throws IllegalArgumentException on any non-zero id
GetExpenses.awaitAll filters on note / category.name case-insensitively and on amount as a string match
GetExpenseSummary.await sums amounts and groups by category, sorted DESC
All 6 interactor factories resolve through Koin (DomainModule)

Closes #2

## Summary - Implement 5 expense interactors: `GetExpenses` (4 methods), `UpsertExpense`, `InsertExpenses`, `DeleteExpense`, `GetExpenseSummary` (`ReassignExpenseCategory` was already implemented on `main`) - Add `ExpenseWithCategoryRow.toModel()` mapper for the joined DAO row (used by both `subscribe*` methods and `awaitOne`) - Register the 5 new interactors as `factory { }` in `DomainModule` - `InsertExpenses.awaitAll` enforces `id == 0` via `require` before delegating to the plain `@Insert` - Fix `docs/03-function-todos.md` doc drift (`UpsertExpense` routes through separate `insert` / `update`, not `@Upsert`) - Note in `GetExpenseSummary` explaining the orphan-category drop behavior ## Test plan - [x] `./gradlew assembleDebug` succeeds - [x] `GetExpenses.awaitOne` returns `null` when either the expense or its category row is missing - [x] `InsertExpenses.awaitAll` throws `IllegalArgumentException` on any non-zero `id` - [x] `GetExpenses.awaitAll` filters on `note` / `category.name` case-insensitively and on `amount` as a string match - [x] `GetExpenseSummary.await` sums amounts and groups by category, sorted DESC - [x] All 6 interactor factories resolve through Koin (`DomainModule`) Closes #2

admin added 1 commit 2026-06-28 09:33:47 +00:00

feat(#2 ): implement expense interactors 46f882b3c3

admin reviewed 2026-06-28 09:41:39 +00:00

admin left a comment

PR looks good and is faithful to the spec. A few findings, in priority order:

Must address:

GetExpenses.awaitOne doesn't use the new ExpenseWithCategoryRow.toModel() helper that this PR adds to ExpenseMapper.kt — see inline comment.
InsertExpenses.awaitAll doesn't enforce the "all id == 0" invariant from docs/03-function-todos.md:33 — see inline comment.
Test plan is thin for the size of the change. Acceptance is "6 interactors implemented + assembleDebug succeeds" — a smoke test that calls each method (or unit tests if the project has any) would be reassuring.

Nits (non-blocking):

GetExpenses.kt search filter uses amountStr.contains(query, ignoreCase = true) — ignoreCase is meaningless on numeric strings; drop it.
docs/03-function-todos.md:26 says UpsertExpense.await "Uses Room @Upsert", but the implementation routes through separate dao.insert / dao.update (and the DAO has no @Upsert method). Doc drift, not a code bug.
GetExpenseSummary silently drops expenses with deleted categories via mapNotNull, which is asymmetric with the @Relation non-nullable path used by subscribeAll / subscribeByDateRange. Worth a one-line comment, or returning byCategory without the dropped amounts.

Verdict: approve after the two must-fix items.

PR looks good and is faithful to the spec. A few findings, in priority order: **Must address:** - `GetExpenses.awaitOne` doesn't use the new `ExpenseWithCategoryRow.toModel()` helper that this PR adds to `ExpenseMapper.kt` — see inline comment. - `InsertExpenses.awaitAll` doesn't enforce the "all `id == 0`" invariant from `docs/03-function-todos.md:33` — see inline comment. - Test plan is thin for the size of the change. Acceptance is "6 interactors implemented + `assembleDebug` succeeds" — a smoke test that calls each method (or unit tests if the project has any) would be reassuring. **Nits (non-blocking):** - `GetExpenses.kt` search filter uses `amountStr.contains(query, ignoreCase = true)` — `ignoreCase` is meaningless on numeric strings; drop it. - `docs/03-function-todos.md:26` says `UpsertExpense.await` "Uses Room `@Upsert`", but the implementation routes through separate `dao.insert` / `dao.update` (and the DAO has no `@Upsert` method). Doc drift, not a code bug. - `GetExpenseSummary` silently drops expenses with deleted categories via `mapNotNull`, which is asymmetric with the `@Relation` non-nullable path used by `subscribeAll` / `subscribeByDateRange`. Worth a one-line comment, or returning `byCategory` without the dropped amounts. **Verdict:** approve after the two must-fix items.

app/src/main/java/dev/achmad/ledgerr/domain/expense/interactor/GetExpenses.kt Outdated

						
				@@ -0,0 +25,4 @@

				        val expense = dao.getById(id) ?: return null

				        val category = categoryDao.getById(expense.categoryId) ?: return null

				        return ExpenseWithCategory(

				            expense = expense.toModel(),

admin commented

2026-06-28 09:41:39 +00:00

This manually inlines what the new ExpenseWithCategoryRow.toModel() mapper in ExpenseMapper.kt already does. The helper is added in this same PR and used by subscribeAll (line 16) and subscribeByDateRange (line 22), but not here. Pick one for consistency:

construct ExpenseWithCategoryRow(expense, category).toModel() here, or
remove the new mapper and inline .map { it.expense.toModel() ... } in the two subscribers.

This manually inlines what the new `ExpenseWithCategoryRow.toModel()` mapper in `ExpenseMapper.kt` already does. The helper is added in this same PR and used by `subscribeAll` (line 16) and `subscribeByDateRange` (line 22), but not here. Pick one for consistency: - construct `ExpenseWithCategoryRow(expense, category).toModel()` here, or - remove the new mapper and inline `.map { it.expense.toModel() ... }` in the two subscribers.

app/src/main/java/dev/achmad/ledgerr/domain/expense/interactor/InsertExpenses.kt Outdated

						
				@@ -0,0 +9,4 @@

				) {

				    suspend fun awaitAll(expenses: List<Expense>) {

				        if (expenses.isEmpty()) return

				        dao.insertAll(expenses.map { it.toEntity() })

admin commented

2026-06-28 09:41:39 +00:00

docs/03-function-todos.md:33 says "expense.id must be 0 for all items", but this isn't enforced. A non-zero id would hit the plain @Insert on dao.insertAll (no OnConflictStrategy.REPLACE) and throw SQLiteConstraintException at runtime. Either:

require(expenses.all { it.id == 0L }) (fail fast with a clear message), or
.map { it.copy(id = 0).toEntity() } (silently normalize — less safe, but matches the bulk-import use case where ids are caller-provided).

`docs/03-function-todos.md:33` says "expense.id must be 0 for all items", but this isn't enforced. A non-zero id would hit the plain `@Insert` on `dao.insertAll` (no `OnConflictStrategy.REPLACE`) and throw `SQLiteConstraintException` at runtime. Either: - `require(expenses.all { it.id == 0L })` (fail fast with a clear message), or - `.map { it.copy(id = 0).toEntity() }` (silently normalize — less safe, but matches the bulk-import use case where ids are caller-provided).

admin added 1 commit 2026-06-28 09:52:50 +00:00

fix(#2 ): address PR review — use row mapper, require id==0, drop ignoreCase on amount, doc drift, orphan-category note 63bfe2a6b5

admin added 1 commit 2026-06-28 09:58:29 +00:00

Merge branch 'main' of https://git.achmad.dev/admin/ledgerr into feat/2-implement-expense-interactors ca2992fceb

# Conflicts:
#	app/src/main/java/dev/achmad/ledgerr/di/DomainModule.kt

admin merged commit 1bb6747610 into main

2026-06-28 09:58:45 +00:00

admin deleted branch feat/2-implement-expense-interactors

2026-06-28 15:02:16 +00:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: admin/ledgerr#11