Collapse entry ledger to a scalar count column #50

Closed
opened 2026-08-15 13:35:07 +02:00 by myrmidex · 2 comments
Owner

The core data-model change. The counter's value stops being SUM(quantity) over a ledger and becomes a plain integer.

Decision

Scalar wins. Per-increment history is dropped permanently and deliberately — it was never displayed, and keeping both a count and an audit log means two sources of truth that can drift.

Scope

Schema: add trackers.count (unsigned integer, default 0); drop the entries table.

PHP:

  • Delete app/Models/Transactions/Entry.php — including totalQuantity(), totalCost(), averageCostPerUnit()
  • Delete app/Http/Controllers/Transactions/EntryController.php — including the quantity × unit_price ≈ total_cost validation rule
  • Tracker::entries(); User::hasEntries()
  • entries.* route group → replaced by:
    • POST /increment$tracker->increment('count')
    • PATCH /count — set an absolute value, validated integer|min:0

Frontend:

  • components/Transactions/AddEntryForm.tsx — the date + quantity + price form is replaced by a plain number dialog
  • dashboard.tsx: totalSharescount; drop handlePurchaseSuccess

Data migration

There is real data in entries on the dev DB. The migration must seed trackers.count from SUM(quantity) before dropping the table, rounded to an integer — the counter is now whole numbers only.

Acceptance criteria

  • Existing dev data lands on the right number after migrating
  • POST /increment bumps the count by exactly 1
  • PATCH /count sets an absolute value; rejects negatives and non-integers
  • No entries references remain
  • Lint + PHPStan clean
The core data-model change. The counter's value stops being `SUM(quantity)` over a ledger and becomes a plain integer. ## Decision **Scalar wins.** Per-increment history is dropped permanently and deliberately — it was never displayed, and keeping both a `count` and an audit log means two sources of truth that can drift. ## Scope **Schema:** add `trackers.count` (unsigned integer, default 0); drop the `entries` table. **PHP:** - Delete `app/Models/Transactions/Entry.php` — including `totalQuantity()`, `totalCost()`, `averageCostPerUnit()` - Delete `app/Http/Controllers/Transactions/EntryController.php` — including the `quantity × unit_price ≈ total_cost` validation rule - `Tracker::entries()`; `User::hasEntries()` - `entries.*` route group → replaced by: - `POST /increment` — `$tracker->increment('count')` - `PATCH /count` — set an absolute value, validated `integer|min:0` **Frontend:** - `components/Transactions/AddEntryForm.tsx` — the date + quantity + price form is replaced by a plain number dialog - `dashboard.tsx`: `totalShares` → `count`; drop `handlePurchaseSuccess` ## Data migration There is real data in `entries` on the dev DB. The migration must seed `trackers.count` from `SUM(quantity)` **before** dropping the table, rounded to an integer — the counter is now whole numbers only. ## Acceptance criteria - [ ] Existing dev data lands on the right number after migrating - [ ] `POST /increment` bumps the count by exactly 1 - [ ] `PATCH /count` sets an absolute value; rejects negatives and non-integers - [ ] No `entries` references remain - [ ] Lint + PHPStan clean
myrmidex added this to the (deleted) milestone 2026-08-15 13:35:07 +02:00
myrmidex added the
enhancement
label 2026-08-15 13:35:07 +02:00
myrmidex self-assigned this 2026-08-15 13:35:08 +02:00
myrmidex modified the milestone from (deleted) to v0.4.0 2026-08-15 13:42:21 +02:00
Author
Owner

Known bug this ticket must fix

Found by code-reviewer during #51 (8ce8a20). Deliberately left unfixed there because this ticket deletes the code path.

OnboardingFlow.tsx posts the starting value to entries.store with raw fetch(). But EntryController::store returns Inertia-style redirects — back()->withErrors() on validation failure, back()->with('success') on success — because it was written for useForm().post() (see the old AddEntryForm.tsx).

fetch() follows redirects by default, so a validation failure 302s to /dashboard, gets 200 text/html, and response.ok is true. The error branch never fires: the user is told setup succeeded while no entry was saved. The "Could not save the starting value" message is currently unreachable via status code.

Not currently exploitable in normal use — the input is type="number" min="0" step="1" and guarded client-side by quantity > 0 — but the error path is dead code.

Note: the reviewer also flagged a timezone risk (todayISO() is UTC vs Laravel today()). Checked — config/app.php:68 sets 'timezone' => 'UTC', so both sides agree and this does not apply unless the app timezone changes.

Resolution here: POST /increment and PATCH /count replace entries.store entirely. Make them return JSON with real status codes, not redirects, so the client's error handling works.

Add to acceptance criteria:

  • POST /increment and PATCH /count return JSON with appropriate status codes (not back() redirects)
  • A rejected value surfaces an error in the UI rather than reporting success
## Known bug this ticket must fix Found by `code-reviewer` during #51 (`8ce8a20`). Deliberately left unfixed there because this ticket deletes the code path. `OnboardingFlow.tsx` posts the starting value to `entries.store` with raw `fetch()`. But `EntryController::store` returns Inertia-style redirects — `back()->withErrors()` on validation failure, `back()->with('success')` on success — because it was written for `useForm().post()` (see the old `AddEntryForm.tsx`). `fetch()` follows redirects by default, so a validation failure 302s to `/dashboard`, gets `200 text/html`, and `response.ok` is `true`. The error branch never fires: **the user is told setup succeeded while no entry was saved.** The "Could not save the starting value" message is currently unreachable via status code. Not currently exploitable in normal use — the input is `type="number" min="0" step="1"` and guarded client-side by `quantity > 0` — but the error path is dead code. Note: the reviewer also flagged a timezone risk (`todayISO()` is UTC vs Laravel `today()`). Checked — `config/app.php:68` sets `'timezone' => 'UTC'`, so both sides agree and this does not apply unless the app timezone changes. **Resolution here:** `POST /increment` and `PATCH /count` replace `entries.store` entirely. Make them return JSON with real status codes, not redirects, so the client's error handling works. Add to acceptance criteria: - [ ] `POST /increment` and `PATCH /count` return JSON with appropriate status codes (not `back()` redirects) - [ ] A rejected value surfaces an error in the UI rather than reporting success
Author
Owner

Done — d482c6d

The counter is now trackers.count, a scalar. SUM(entries.quantity) and the ledger are gone.

Schema: 2026_08_15_000002_add_count_to_trackers_drop_entries.php — adds count (unsigned int, default 0), backfills each tracker from SUM(quantity) rounded and clamped at 0, drops entries. down() reverses the schema (lossy on per-entry history by design, noted in a comment).

PHP: new CounterController with POST /increment and PATCH /count; deleted Entry, EntryController, Tracker::entries(), User::hasEntries().

Frontend: clicking the LED increments with an optimistic update, rolled back on failure and guarded against overlapping requests. New SetCountForm behind [SET VALUE]. Deleted AddEntryForm and InlineForm. dashboard.tsx now makes one fetch instead of two. todayISO() removed, which also retires the UTC/timezone risk flagged in the #51 review.

The redirect bug is fixed

Both endpoints return JSON with real status codes. The bug logged on this ticket — entries.store returning back(), so fetch() followed the redirect and reported success on validation failure — is gone with the endpoint. #54 adds a regression test asserting content-type: application/json on both routes.

From review

  • max:4294967295 added to PATCH /count, so an oversized value returns 422 rather than a DB exception surfaced as a 500
  • In-flight guard on the LED click, released in finally
  • Declined removing $tracker->refresh(): the response is what the client trusts as truth, and re-reading from the database is a stronger contract than trusting an in-memory value. One query on a single-user app is not worth weakening that.

Backfill verification

Originally shipped unverified — the migration had already run via a silent container_tinker call (#58), and the dev volume wipe had likely destroyed the original entries rows.

#54 closes that gap. CountBackfillMigrationTest exercises the conversion against real ledger rows: summing, fractional rounding, negative clamping, empty-tracker default, table drop, and the down() round-trip. Mutation-checked — replacing max(0, (int) round($total)) with (int) floor($total) fails the rounding test and surfaces SQLSTATE[22003] Out of range value for column 'count', i.e. without the clamp a negative total would crash the migration rather than store a wrong number.

Gates

Pint PASS (50 files) · PHPUnit OK (25 tests, 62 assertions) · ESLint exit 0 · build PASS

"PHPStan clean" not checked — not installed yet (#55).

## Done — `d482c6d` The counter is now `trackers.count`, a scalar. `SUM(entries.quantity)` and the ledger are gone. **Schema:** `2026_08_15_000002_add_count_to_trackers_drop_entries.php` — adds `count` (unsigned int, default 0), backfills each tracker from `SUM(quantity)` rounded and clamped at 0, drops `entries`. `down()` reverses the schema (lossy on per-entry history by design, noted in a comment). **PHP:** new `CounterController` with `POST /increment` and `PATCH /count`; deleted `Entry`, `EntryController`, `Tracker::entries()`, `User::hasEntries()`. **Frontend:** clicking the LED increments with an optimistic update, rolled back on failure and guarded against overlapping requests. New `SetCountForm` behind `[SET VALUE]`. Deleted `AddEntryForm` and `InlineForm`. `dashboard.tsx` now makes one fetch instead of two. `todayISO()` removed, which also retires the UTC/timezone risk flagged in the #51 review. ### The redirect bug is fixed Both endpoints return JSON with real status codes. The bug logged on this ticket — `entries.store` returning `back()`, so `fetch()` followed the redirect and reported success on validation failure — is gone with the endpoint. #54 adds a regression test asserting `content-type: application/json` on both routes. ### From review - `max:4294967295` added to `PATCH /count`, so an oversized value returns 422 rather than a DB exception surfaced as a 500 - In-flight guard on the LED click, released in `finally` - Declined removing `$tracker->refresh()`: the response is what the client trusts as truth, and re-reading from the database is a stronger contract than trusting an in-memory value. One query on a single-user app is not worth weakening that. ### Backfill verification Originally shipped **unverified** — the migration had already run via a silent `container_tinker` call (#58), and the dev volume wipe had likely destroyed the original `entries` rows. **#54 closes that gap.** `CountBackfillMigrationTest` exercises the conversion against real ledger rows: summing, fractional rounding, negative clamping, empty-tracker default, table drop, and the `down()` round-trip. Mutation-checked — replacing `max(0, (int) round($total))` with `(int) floor($total)` fails the rounding test *and* surfaces `SQLSTATE[22003] Out of range value for column 'count'`, i.e. without the clamp a negative total would crash the migration rather than store a wrong number. ### Gates Pint PASS (50 files) · PHPUnit OK (25 tests, 62 assertions) · ESLint exit 0 · build PASS "PHPStan clean" not checked — not installed yet (#55).
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: lvl0/incr#50
No description provided.