Feat/Complete Sales Summaries #5

Merged
notid merged 5 commits from feat/complete-sales-summaries into master 2026-05-16 00:16:44 -07:00
Owner

Summary

Completes the automatic sales summary pipeline end-to-end: the sales-summaries-v2 job now calculates aggregate totals, preserves manual adjustments, and automatically posts balanced journal entries to the ledger.

What Changed

New Datomic transaction function (upsert-sales-summary-ledger)

  • Transforms detailed sales-summary-items into aggregated journal-entry lines grouped by account and ledger side
  • Handles the full upsert: posts a new journal entry for summaries with mapped accounts, or retracts the orphaned entry if items no longer qualify

Enhanced sales-summaries-v2 job

  • Calculates and stores 13 aggregate total attributes (card/cash/food-app/gift-card payments, refunds, fees, discounts, tax, tip, returns, unknown, net)
  • Preserves manual items (manual? true) during recalculation — only auto-calculated items are replaced

Ledger reconciliation

  • reconcile-ledger now queries for sales summaries missing journal entries and repairs them via :upsert-sales-summary-ledger, alongside existing invoice and transaction repairs

Schema

  • Added 13 total-* attributes on sales-summary (all db.type/double, no history)
  • Registered the new transaction function in tx.clj and datomic.clj

Admin UI cleanup

  • Resolved "clientize" and HTMX client-id TODOs in the sales summaries admin page
  • new-summary-item now correctly passes client-id via hx-vals
  • Removed stale TODO comments and placeholder code

Files Changed (8)

File Purpose
iol_ion/.../upsert_sales_summary_ledger.clj New Datomic tx function
iol_ion/.../tx.clj Register new tx function
resources/schema.edn 13 new total-* attributes
src/.../datomic.clj Load new tx namespace
src/.../jobs/sales_summaries.clj Aggregate totals + manual item preservation
src/.../ledger.clj Sales summary repair in reconcile-ledger
src/.../ssr/admin/sales_summaries.clj UI TODO cleanup
docs/plans/...plan.md Implementation plan document
## Summary Completes the automatic sales summary pipeline end-to-end: the `sales-summaries-v2` job now calculates aggregate totals, preserves manual adjustments, and automatically posts balanced journal entries to the ledger. ## What Changed **New Datomic transaction function** (`upsert-sales-summary-ledger`) - Transforms detailed `sales-summary-item`s into aggregated `journal-entry` lines grouped by account and ledger side - Handles the full upsert: posts a new journal entry for summaries with mapped accounts, or retracts the orphaned entry if items no longer qualify **Enhanced `sales-summaries-v2` job** - Calculates and stores 13 aggregate total attributes (card/cash/food-app/gift-card payments, refunds, fees, discounts, tax, tip, returns, unknown, net) - Preserves manual items (`manual? true`) during recalculation — only auto-calculated items are replaced **Ledger reconciliation** - `reconcile-ledger` now queries for sales summaries missing journal entries and repairs them via `:upsert-sales-summary-ledger`, alongside existing invoice and transaction repairs **Schema** - Added 13 `total-*` attributes on `sales-summary` (all `db.type/double`, no history) - Registered the new transaction function in `tx.clj` and `datomic.clj` **Admin UI cleanup** - Resolved "clientize" and HTMX `client-id` TODOs in the sales summaries admin page - `new-summary-item` now correctly passes `client-id` via `hx-vals` - Removed stale TODO comments and placeholder code ## Files Changed (8) | File | Purpose | |------|---------| | `iol_ion/.../upsert_sales_summary_ledger.clj` | New Datomic tx function | | `iol_ion/.../tx.clj` | Register new tx function | | `resources/schema.edn` | 13 new `total-*` attributes | | `src/.../datomic.clj` | Load new tx namespace | | `src/.../jobs/sales_summaries.clj` | Aggregate totals + manual item preservation | | `src/.../ledger.clj` | Sales summary repair in `reconcile-ledger` | | `src/.../ssr/admin/sales_summaries.clj` | UI TODO cleanup | | `docs/plans/...plan.md` | Implementation plan document |
Author
Owner

Code Review

Scope: feat/complete-sales-summariesmaster (1 commit, 8 files, +529/-105)
Review team: correctness, testing, maintainability, project-standards, agent-native, learnings-researcher, data-migrations, adversarial


P0 -- Critical

# File Issue Confidence
1 upsert_sales_summary_ledger.clj:76 Invalid :db/retractEntity syntax — passes vector instead of entity ID. Crashes whenever summary has no mapped accounts. 100%
2 upsert_sales_summary_ledger.clj:66 nil client-id crashes entire reconciliation. Hard-deleted client with orphaned summaries aborts ALL pending ledger repairs. 75%

P1 -- High

# File Issue Confidence
3 sales_summaries.clj:279 Duplicate summary->journal-entry — dead code identical to tx function version. Will diverge. 100%
4 upsert_sales_summary_ledger.clj:51 total-amount only sums debits, ignoring credits. Downstream balance invariant expects amount == debits == credits. 75%
5 sales_summaries.clj:337 calc-aggregate-totals :else clause silently absorbs all unmatched categories into total-net. Zero visibility. 100%
6 ledger.clj:58 reconcile-ledger can't retract stale journal entries when ALL accounts are cleared — query requires at least one mapped account. 100%
7 resources/schema.edn:1973 Missing schema for :sales-summary/total-gift-card-refunds. SSR reads it, job never computes it. Always $0.00. 100%
8 resources/schema.edn:1985 Missing schema for per-type fee attributes (total-card-fees, total-food-app-fees, total-gift-card-fees). 100%
9 resources/schema.edn:2005 Naming mismatch: schema has total-unknown-payments, SSR reads total-unknown-processor-payments. Always $0.00. 100%

P2 -- Moderate

# File Issue Confidence
10 upsert_sales_summary_ledger.clj:5-25 Dead code: -random-tempid, extant-read, calc-client+account+location+date, unused Date import 100%
11 upsert_sales_summary_ledger.clj:75 current-date called with original db instead of :db-after — potentially stale transaction timestamp 50%
12 sales_summaries.clj:397 Ad-hoc schema transaction in comment block for total-unknown-processor-payments — not in schema.edn 100%
13 sales_summaries.clj:350 Manual items bypass account mapping — can silently disappear from ledger 75%

P3 -- Low

# File Issue Confidence
14 tx.clj:40, datomic.clj:8 Cosmetic indentation changes mixed with functional changes 100%
15 upsert_sales_summary_ledger.clj:73 Redundant client/ledger-last-change update (already set by upsert-ledger) 50%

Requirements Completeness

Requirement Status Notes
R1. Calculate and store aggregate totals Met But missing gift-card-refunds, per-type fees
R2. Preserve manual adjustments Met Manual items filtered and concatenated back
R3. Aggregate into balanced journal-entry lines Partial total-amount only sums debits; no balance assertion
R4. Automate ledger posting Met reconcile-ledger + :upsert-sales-summary-ledger tx fn
R5. Resolve UI TODOs Met Clientize, HTMX client-id, stale TODOs cleaned up

Residual Risks & Testing Gaps

  • Zero test coverage for upsert-sales-summary-ledger, calc-aggregate-totals, and reconcile-ledger sales summary path
  • Duplicate summary->journal-entry will diverge over time
  • Floating-point accumulation in financial aggregation without rounding
  • dc/with simulation may not faithfully reproduce upsert-entity behavior

Verdict: Not ready

Two P0 blockers must be fixed before merge: the :db/retractEntity crash (#1) and the nil client-id reconciliation abort (#2). Three P1 schema mismatches (#7-9) cause silent $0.00 display for gift card refunds, per-type fees, and unknown payments.

# Code Review **Scope:** `feat/complete-sales-summaries` → `master` (1 commit, 8 files, +529/-105) **Review team:** correctness, testing, maintainability, project-standards, agent-native, learnings-researcher, data-migrations, adversarial --- ### P0 -- Critical | # | File | Issue | Confidence | |---|------|-------|------------| | 1 | `upsert_sales_summary_ledger.clj:76` | Invalid `:db/retractEntity` syntax — passes vector instead of entity ID. Crashes whenever summary has no mapped accounts. | 100% | | 2 | `upsert_sales_summary_ledger.clj:66` | `nil` client-id crashes entire reconciliation. Hard-deleted client with orphaned summaries aborts ALL pending ledger repairs. | 75% | ### P1 -- High | # | File | Issue | Confidence | |---|------|-------|------------| | 3 | `sales_summaries.clj:279` | Duplicate `summary->journal-entry` — dead code identical to tx function version. Will diverge. | 100% | | 4 | `upsert_sales_summary_ledger.clj:51` | `total-amount` only sums debits, ignoring credits. Downstream balance invariant expects amount == debits == credits. | 75% | | 5 | `sales_summaries.clj:337` | `calc-aggregate-totals` `:else` clause silently absorbs all unmatched categories into `total-net`. Zero visibility. | 100% | | 6 | `ledger.clj:58` | `reconcile-ledger` can't retract stale journal entries when ALL accounts are cleared — query requires at least one mapped account. | 100% | | 7 | `resources/schema.edn:1973` | Missing schema for `:sales-summary/total-gift-card-refunds`. SSR reads it, job never computes it. Always $0.00. | 100% | | 8 | `resources/schema.edn:1985` | Missing schema for per-type fee attributes (`total-card-fees`, `total-food-app-fees`, `total-gift-card-fees`). | 100% | | 9 | `resources/schema.edn:2005` | Naming mismatch: schema has `total-unknown-payments`, SSR reads `total-unknown-processor-payments`. Always $0.00. | 100% | ### P2 -- Moderate | # | File | Issue | Confidence | |---|------|-------|------------| | 10 | `upsert_sales_summary_ledger.clj:5-25` | Dead code: `-random-tempid`, `extant-read`, `calc-client+account+location+date`, unused `Date` import | 100% | | 11 | `upsert_sales_summary_ledger.clj:75` | `current-date` called with original `db` instead of `:db-after` — potentially stale transaction timestamp | 50% | | 12 | `sales_summaries.clj:397` | Ad-hoc schema transaction in comment block for `total-unknown-processor-payments` — not in schema.edn | 100% | | 13 | `sales_summaries.clj:350` | Manual items bypass account mapping — can silently disappear from ledger | 75% | ### P3 -- Low | # | File | Issue | Confidence | |---|------|-------|------------| | 14 | `tx.clj:40`, `datomic.clj:8` | Cosmetic indentation changes mixed with functional changes | 100% | | 15 | `upsert_sales_summary_ledger.clj:73` | Redundant `client/ledger-last-change` update (already set by `upsert-ledger`) | 50% | --- ## Requirements Completeness | Requirement | Status | Notes | |-------------|--------|-------| | R1. Calculate and store aggregate totals | Met | But missing gift-card-refunds, per-type fees | | R2. Preserve manual adjustments | Met | Manual items filtered and concatenated back | | R3. Aggregate into balanced journal-entry lines | Partial | `total-amount` only sums debits; no balance assertion | | R4. Automate ledger posting | Met | `reconcile-ledger` + `:upsert-sales-summary-ledger` tx fn | | R5. Resolve UI TODOs | Met | Clientize, HTMX `client-id`, stale TODOs cleaned up | --- ## Residual Risks & Testing Gaps - **Zero test coverage** for `upsert-sales-summary-ledger`, `calc-aggregate-totals`, and `reconcile-ledger` sales summary path - Duplicate `summary->journal-entry` will diverge over time - Floating-point accumulation in financial aggregation without rounding - `dc/with` simulation may not faithfully reproduce `upsert-entity` behavior --- **Verdict: Not ready** Two P0 blockers must be fixed before merge: the `:db/retractEntity` crash (#1) and the nil client-id reconciliation abort (#2). Three P1 schema mismatches (#7-9) cause silent $0.00 display for gift card refunds, per-type fees, and unknown payments.
notid added 4 commits 2026-05-15 23:23:16 -07:00
Move journal entry calculation and creation from the reconcile-ledger
background job into the upsert-sales-summary tx function. Now any save
of a sales summary (job recalculation, admin edit wizard, or manual
touch) automatically creates the journal entry if balanced with all
accounts mapped, or retracts it if conditions no longer hold. Eliminates
the need for a separate upsert-sales-summary-ledger call and the
reconcile ledger pass for sales summaries.
The 13 sales-summary/total-* attributes were computed and stored but never
read — the only consumer (get-debits) was commented out. Active display code
computes totals on-the-fly from the items list instead.
notid force-pushed feat/complete-sales-summaries from 5a49ad777c to 5a39a0c762 2026-05-15 23:23:16 -07:00 Compare
notid added 1 commit 2026-05-16 00:14:32 -07:00
- Move SSR handler from auto-ap.ssr.admin.sales-summaries to auto-ap.ssr.pos.sales-summaries
- Move route namespace from auto-ap.routes.admin.sales-summaries to auto-ap.routes.pos.sales-summaries
- Update nav to use main-aside-nav with POS breadcrumbs
- Use pos.common date-range-field* filter component
- Remove wrap-admin/wrap-client-redirect-unauthenticated from middleware
- Add Summaries to Sales sidebar menu
notid merged commit cc31d8849b into master 2026-05-16 00:16:44 -07:00
Sign in to join this conversation.
No Reviewers
No Label
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: notid/integreat#5