Repository navigation
feat: add ingredient cost tracking and profitability reporting - #1332
Merged
Merged
Conversation
… resolution, and route payloads"}
Contributor
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configuration
📒 Files selected for processing: 44
✨ Finishing Touches
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 |
…ite (Shard 3/4)` and `Playwright E2E (Shard 1/4)` (which also cascaded to `End-to-End Playwright Tests` and `Required Checks Gate`): 1. **`Core Test Suite (Shard 3/4)` (`tests/expense-records.test.ts`)**: - **Cause**: Migration v117 (`add_ingredient_cost_tracking`) adds `unit_cost` to `supplies` and `unit_cost`, `total_cost`, and `unit_cost_after` to `supply_movements` (as well as `ingredient_cost` and `ingredient_cost_known` to `order_items`). In `tests/expense-records.test.ts`, `preservedSelections` projected prior columns only for `order_items` while using `SELECT *` for `orders`, `bills`, `supplies`, `supply_movements`, `cash_sessions`, and `cash_drawer_movements`, causing `SELECT * FROM supplies` after upgrading from v116 (`PRIOR_VERSION`) to v117 (`LATEST_VERSION`) to include `"unit_cost":null`. - **Fix**: Replaced `priorOrderItemColumns` and `SELECT *` with `selectPriorColumns(table)` (using the existing `columns(db, table)` helper at `PRIOR_VERSION`) across all preserved tables in `tests/expense-records.test.ts`. 2. **`Playwright E2E (Shard 1/4)` (`frontend/e2e/database-export-import.spec.ts`)**: - **Cause**: `Database import accepts contaminated payloads and rebuilds FTS search indexes` tested a same-version merge import using a hardcoded `schema_version: '116'`. After adding migration v117, `DatabaseSettingsTab` and `POST /api/db/import` treated `116 !== 117` as a cross-version schema mismatch requiring a Master PIN. - **Fix**: Updated `contaminatedPayload.schema_version` from `'116'` to `'117'` in `frontend/e2e/database-export-import.spec.ts`. 3. **Verification**: - `npm run test:expenses && npm run test:addon-inventory-db && npm run test:staff-approvers-directory` (passed) - `npm run test:release-regressions` (passed) - `cd frontend && CI=true npx playwright test --config=playwright.config.ts --project=chromium --retries=0 e2e/database-export-import.spec.ts` (2/2 passed) - `cd frontend && CI=true npx playwright test --config=playwright.config.ts --project=chromium --retries=0 --shard=1/4 --grep='@ci-tier1|@ci-tier2' --grep-invert='FINDING:|a paid order offers a refund, and issuing it settles against the recorded sale'` (22/22 passed) - `npm run build && npm run build:frontend` (passed) - `npm run lint && npm run lint:budget && npx oxfmt --check tests/expense-records.test.ts && (cd frontend && npx oxfmt --check e2e/database-export-import.spec.ts) && git diff --check` (passed)
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
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.
Intent
Implement ingredient cost and profitability reporting (#1283): track unit purchase costs on supplies with weighted average cost (WAC) on stock receives, derive menu item ingredient cost from recipes and freeze it onto order_items at sale time, and show revenue, ingredient cost, profit, and margin per product and for the period in sales reports and daily sales exports.
What Changed
unit_costtracking onsuppliesandunit_cost,total_cost, andunit_cost_afteronsupply_movements(migration 117), updating supply unit costs via weighted average cost (WAC) on pricedreceivemovements (or resetting when prior stock is<= 0or unpriced) and validatingsupplies[].unit_coston database imports.buildRecipeSnapshot()andscaleRecipeSnapshot()to compute recipe component and line ingredient costs from active recipes and variantrecipe_multipliervalues, freezingingredient_costandingredient_cost_knownontoorder_itemsacross order creation, item append, quantity-reduction edits, andvoid_adjustmentrefund reversals.known,partial,unknown) to/api/reports/financial-summary,/api/reports/topProducts, daily sales CSV/XLSX exports, and the Inventory and Dashboard UIs with translations across all supported locales.Risk Assessment
✅ Low: All Round 1 review findings were cleanly addressed, and the ingredient cost tracking, WAC calculation, frozen order-item cost snapshots, and profitability reporting/export changes are well-bounded and consistent with repository invariants.
Testing
Built the backend and static desktop frontend, ran 10 targeted backend/integration suites (including 115 assertions in tests/ingredient-cost-lifecycle.test.ts), and drove the real running product via Playwright on isolated ports 43101-43103 to capture live visual and export evidence across /inventory, /dashboard, /api/reports/topProducts, and /api/reports/daily-sales/export.
Evidence: Daily Sales Summary CSV Export with Period Profitability Metrics
metric,value business_date,2026-10-23 timezone,Asia/Bangkok business_day_start,00:00 currency,THB order_count,2 paid_bill_count,2 gross_collected,342.4 refunds_issued,0 net_collected,342.4 tax_total,22.4 discount_total,0 service_charge_total,0 packaging_charge_total,0 delivery_charge_total,0 known_cost_net_item_sales,220 ingredient_cost_total,36.4 gross_profit_total,183.6 gross_margin_percent,83.45 cost_coverage_status,partial payment_card,342.4Evidence: Daily Sales Items CSV Export with Per-Product Cost, Profit, Margin, and Cost Status
product_id,product_name,product_sku,quantity,gross_item_sales,item_discounts,net_item_sales,tax_amount,ingredient_cost,profit,margin_percent,cost_status e2e-product,E2E Coffee,,3,180,0,180,12.600000000000001,26.4,93.6,78,partial yfrl4i,Iced Matcha,,1,107,0,107,7,10,90,90,known uqyfeq,Bottled Sparkling Water,,1,40,0,40,2.8,,,,unknownEvidence: Live Top Products API Payload (Owner vs Manager Redacted Without reports.financial.view)
Evidence: Ingredient Cost Lifecycle Test Suite (115 assertions)
Source: Ingredient Cost Lifecycle Test Suite (115 assertions)
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 4 issues found → auto-fixed ✅
frontend/src/app/(dashboard)/inventory/page.tsx:577- In the Supplies table on the Inventory page (frontend/src/app/(dashboard)/inventory/page.tsx:574-583), the unit cost cell rendersfmt(Number(s.unit_cost)) / ${s.base_unit}usinguseFormatCurrency(), which rounds to the tenant currency's minor unit (0,2, or3decimal places). Becauses.unit_costis stored perbase_unit(g,ml, etc.) with up to 10 decimal places to preserve sub-minor-unit precision: (1) for a 2-decimal currency (e.g. USD) with a supply (base_unit = 'g') costing$2.00 / kg(s.unit_cost = 0.002), the primary bold label renders$0.00 / gabove the secondary($2.00 / kg)hint; and (2) for a 0-decimal currency (e.g. JPY) with a supply (base_unit = 'g') costing¥250 / kg(s.unit_cost = 0.25, matchingtests/ingredient-cost-lifecycle.test.ts:1873-1906),fmt(0.25)rounds to¥0 / gAND the secondary per-kg/per-L hint atfrontend/src/app/(dashboard)/inventory/page.tsx:578is gated onNumber(s.unit_cost) < 0.1(false for0.1 <= unit_cost < 0.5), so the cell displays only¥0 / g. Confirm whether sub-minor-unit base-unit costs should display the scaled per-kg/per-L rate whenevers.unit_costis below 1 minor unit of the active currency (or format sub-minor-unit base costs with significant decimals).main/services/supplies.ts:23-resolveDbCurrencyFractionDigits(db)(main/services/supplies.ts:23-32, called atmain/services/supplies.ts:280,518andmain/services/recipes.ts:146,365) introduces a second currency-lookup path that queries onlySELECT value FROM settings WHERE key = 'currency'and passesrow?.value ?? ''togetCurrencyFractionDigits, whereasgetTenantCurrency(db)(main/services/refund.ts:34-42, used bymain/routes/reports.ts:388,721andmain/services/daily-sales-export.ts:154-156) resolves the currency from bothsettings.currencyandsettings.countryviaresolveTenantCurrency. If a store hassettings.countryconfigured (for example'JP'or'KW') without an explicitsettings.currencyrow,resolveDbCurrencyFractionDigitsfalls back to 2 fraction digits while reports and daily sales exports use 0 (JPY) or 3 (KWD) fraction digits. Recommend simplifyingresolveDbCurrencyFractionDigits(db)to resolve the currency viagetTenantCurrency(db)first (falling back torow?.value ?? ''in itscatchblock for unconfigured unit-test databases).main/routes/supplies.ts:128- Simplification:POST /api/supplies(main/routes/supplies.ts:128-129),PUT /api/supplies/:id(main/routes/supplies.ts:201-202), andPOST /api/supplies/:id/movements(main/routes/supplies.ts:242-243) accept camelCase request-body fallbacks (body.unitCost,body.unitCostUnit,body.totalCost) alongside snake_case (body.unit_cost,body.unit_cost_unit,body.total_cost). Every other field on these endpoints (base_unit,current_stock,min_stock,movement_type, etc.) and all callers (frontend/src/app/(dashboard)/inventory/page.tsx:249-250,331-334andtests/ingredient-cost-lifecycle.test.ts) use snake_case only. Recommend removing the unrequired camelCase fallbacks and readingbody.unit_cost,body.unit_cost_unit, andbody.total_costdirectly.main/routes/orders.ts:5661- Simplification: InPATCH /api/orders/:orderId/items/:itemId/edit(main/routes/orders.ts:5661-5668), whenscaledRecipeSnapshotis null, an extra fallback branch checksNumber(oldItem.ingredient_cost_known) === 1 && typeof oldItem.ingredient_cost === 'number'and scalesoldItem.ingredient_cost * heldRatiodirectly. Under the stated design and invariant 9 (docs/reference/product-invariants.md:107-129),order_items.ingredient_costis derived exclusively fromrecipe_snapshotviaextractOrderItemCostFromSnapshot, which all other write paths (insertOrderItemsatmain/routes/orders.ts:983,POST /api/orders/:orderId/itemsatmain/routes/orders.ts:2000, and the sibling pending-decrease path atmain/routes/orders.ts:5772-5775) use unconditionally. Recommend removing the non-snapshot fallback branch atmain/routes/orders.ts:5661-5668and callingextractOrderItemCostFromSnapshot(scaledRecipeSnapshot)directly.🔧 Fix applied.
✅ Re-checked - no issues remain.
✅ **Test** - passed
✅ No issues found.
npm run buildnpm run build:frontendnpm run test:ingredient-cost (115/115 passed)npm run test:recipe-order-lifecycle (35/35 passed)npm run test:supplies-service (39/39 passed)npm run test:daily-sales-export (86/86 passed)npm run test:order-item-edit (147/147 passed)npm run test:refunds (30/30 passed)npm run test:refund-completed-orders (43/43 passed)npm run test:backup (71/71 passed)npm run test:database-tools-api (35/35 passed)npm run test:script-coverage (passed)Playwright live E2E verification against isolated FloCafe stack on ports 43101/43102/43103 (1 passed)✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.