Skip to content

fix: centralize terminal-item order total recalculation #757

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
khaira777 merged 4 commits into from
Sep 15, 2026
+145 −112
Merged

fix: centralize terminal-item order total recalculation #757

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
cb17b01
refactor: deduplicate order total recalculation
khaira777 Sep 15, 2026
64de643
no-mistakes(review): Route discounts through shared terminal-item fil…
khaira777 Sep 15, 2026
7690b2f
no-mistakes(document): Documented terminal-item recalculation; lint a…
khaira777 Sep 15, 2026
1570461
test: preserve order tax regression after void
khaira777 Sep 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions docs/API.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -552,6 +552,9 @@ Update item status (KDS workflow).

## Order Discounts

Both discount endpoints recalculate order totals from non-terminal order items;
cancelled, voided, `void_adjustment`, and refunded items are excluded.

### PATCH `/api/orders/:id/discount`
Apply order-level discount.

Expand Down
47 changes: 4 additions & 43 deletions main/routes/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ import {
invertTaxBreakdown,
invertTaxSnapshot,
} from '../services/tax';
import { calculateOrderTotals } from '../services/orders';
import { cloudSync } from '../services/cloud-sync';
import { parsePhoneE164, stripPhoneDigits } from '../lib/phone';
import QRCode from 'qrcode';
Expand Down Expand Up @@ -407,28 +408,8 @@ export function registerRoutes(app: Express): void {
}
}

// Recalculate order totals excluding cancelled, voided, and void_adjustment items
const activeItems = db.prepare("SELECT * FROM order_items WHERE order_id = ? AND status NOT IN ('cancelled', 'voided', 'void_adjustment', 'refunded')")
.all(orderId) as any[];
let subtotal = 0;
let totalTax = 0;
let exclusiveTax = 0;
const allTaxBreakdowns: any[] = [];
const allTaxSnapshots: (string | null)[] = [];
for (const i of activeItems) {
subtotal += i.subtotal || 0;
totalTax += i.tax_amount || 0;
if (i.tax_type !== 'inclusive') {
exclusiveTax += i.tax_amount || 0;
}
if (i.tax_breakdown) {
try {
const breakdown = JSON.parse(i.tax_breakdown);
if (Array.isArray(breakdown)) allTaxBreakdowns.push(breakdown);
} catch { }
}
allTaxSnapshots.push(i.tax_snapshot || null);
}
// Recalculate order totals excluding terminal items.
const { activeItems, subtotal, totalTax, exclusiveTax, allTaxBreakdowns, allTaxSnapshots } = calculateOrderTotals(db, orderId);
// BUG #13 FIX: Preserve order-level discount (scale percentage proportionally)
const currency = getTenantCurrency();
const decimals = getCurrencyFractionDigits(currency);
Expand Down Expand Up @@ -602,27 +583,7 @@ export function registerRoutes(app: Express): void {
.run(now(), itemId);

// Recalculate order totals
const activeItems = db.prepare("SELECT * FROM order_items WHERE order_id = ? AND status NOT IN ('cancelled', 'voided', 'void_adjustment', 'refunded')")
.all(orderId) as any[];
let subtotal = 0;
let totalTax = 0;
let exclusiveTax = 0;
const allTaxBreakdowns: any[] = [];
const allTaxSnapshots: (string | null)[] = [];
for (const i of activeItems) {
subtotal += i.subtotal || 0;
totalTax += i.tax_amount || 0;
if (i.tax_type !== 'inclusive') {
exclusiveTax += i.tax_amount || 0;
}
if (i.tax_breakdown) {
try {
const breakdown = JSON.parse(i.tax_breakdown);
if (Array.isArray(breakdown)) allTaxBreakdowns.push(breakdown);
} catch { }
}
allTaxSnapshots.push(i.tax_snapshot || null);
}
const { subtotal, totalTax, exclusiveTax, allTaxBreakdowns, allTaxSnapshots } = calculateOrderTotals(db, orderId);
// BUG #13 FIX: Preserve order-level discount (scale percentage proportionally)
const currency = getTenantCurrency();
const decimals = getCurrencyFractionDigits(currency);
Expand Down
83 changes: 23 additions & 60 deletions main/routes/orders.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
normalizeChargeAmount,
} from '../services/tax';
import { applyPayableRounding } from '../services/tax-engine';
import { calculateOrderTotals } from '../services/orders';
import { notifyKdsUpdate, notifyOrderUpdated } from '../services/kds';
import { cloudSync } from '../services/cloud-sync';
import { validateOrderNotes, validateItemNotes, validateProductQuantity } from './orders-validation';
Expand Down Expand Up @@ -775,27 +776,14 @@ router.post('/:id/items', orderWriteRateLimit, requireRole(...ROLE_ACCESS.sales)
}
}

// BUG #3 FIX: Filter out cancelled items from total recalculation
const activeItems = db.prepare("SELECT * FROM order_items WHERE order_id = ? AND status != 'cancelled'").all(req.params.id) as any[];
let subtotal = 0;
let totalTax = 0;
let exclusiveTax = 0;
const allTaxBreakdowns: any[] = [];
const allTaxSnapshots: (string | null)[] = [];
for (const item of activeItems) {
subtotal += item.subtotal;
totalTax += item.tax_amount;
if (item.tax_type !== 'inclusive') {
exclusiveTax += item.tax_amount;
}
if (item.tax_breakdown) {
try {
const breakdown = JSON.parse(item.tax_breakdown);
if (Array.isArray(breakdown)) allTaxBreakdowns.push(breakdown);
} catch { }
}
allTaxSnapshots.push(item.tax_snapshot || null);
}
// BUG #3 FIX: Filter out terminal items from total recalculation.
const {
subtotal,
totalTax,
exclusiveTax,
allTaxBreakdowns,
allTaxSnapshots,
} = calculateOrderTotals(db, req.params.id as string);

// BUG #12 FIX: Preserve order-level discount (scale percentage proportionally)
const currency = getTenantCurrency();
Expand Down Expand Up @@ -1264,24 +1252,12 @@ router.patch('/:id/discount', orderWriteRateLimit, requireRole(...ROLE_ACCESS.ow
}

// Recalculate tax from item-level data to avoid compounding on repeated discount edits.
const activeItems = db.prepare("SELECT * FROM order_items WHERE order_id = ? AND status != 'cancelled'").all(req.params.id) as any[];
let freshTax = 0;
let exclusiveTax = 0;
const allTaxBreakdowns: any[] = [];
const allTaxSnapshots: (string | null)[] = [];
for (const item of activeItems) {
freshTax += item.tax_amount || 0;
if (item.tax_type !== 'inclusive') {
exclusiveTax += item.tax_amount || 0;
}
if (item.tax_breakdown) {
try {
const breakdown = JSON.parse(item.tax_breakdown);
if (Array.isArray(breakdown)) allTaxBreakdowns.push(breakdown);
} catch { }
}
allTaxSnapshots.push(item.tax_snapshot || null);
}
const {
totalTax: freshTax,
exclusiveTax,
allTaxBreakdowns,
allTaxSnapshots,
} = calculateOrderTotals(db, req.params.id as string);
let newTaxAmount = freshTax;
let newExclusiveTax = exclusiveTax;
let taxRatio = 1;
Expand Down Expand Up @@ -1490,27 +1466,14 @@ router.patch('/:id/items/:itemId/discount', orderWriteRateLimit, requireRole(...
newTaxSnapshotJson, taxResult.tax_type, newTotal, now(), req.params.itemId,
);

// Update order totals excluding cancelled, voided, or refunded items.
const allItems = db.prepare("SELECT * FROM order_items WHERE order_id = ? AND status NOT IN ('cancelled', 'voided', 'void_adjustment', 'refunded')").all(req.params.id) as any[];
let orderSubtotal = 0;
let orderTax = 0;
let exclusiveOrderTax = 0;
const allTaxBreakdowns: any[] = [];
const allTaxSnapshots: (string | null)[] = [];
for (const i of allItems) {
orderSubtotal += i.subtotal;
orderTax += i.tax_amount;
if (i.tax_type !== 'inclusive') {
exclusiveOrderTax += i.tax_amount;
}
if (i.tax_breakdown) {
try {
const breakdown = JSON.parse(i.tax_breakdown);
if (Array.isArray(breakdown)) allTaxBreakdowns.push(breakdown);
} catch { }
}
allTaxSnapshots.push(i.tax_snapshot || null);
}
// Update order totals excluding terminal items.
const {
subtotal: orderSubtotal,
totalTax: orderTax,
exclusiveTax: exclusiveOrderTax,
allTaxBreakdowns,
allTaxSnapshots,
} = calculateOrderTotals(db, req.params.id as string);

// Recalculate order-level discount proportionally on new subtotal
const existingDiscountAmount = order.discount_amount || 0;
Expand Down
49 changes: 49 additions & 0 deletions main/services/orders.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
import { getDatabase } from '../db';
import type { TaxBreakdown } from './tax';
import { TERMINAL_ITEM_STATUSES } from './refund';

type Database = ReturnType<typeof getDatabase>;
type OrderItemRow = {
subtotal: number | null;
tax_amount: number | null;
tax_type: string | null;
tax_breakdown: string | null;
tax_snapshot: string | null;
};

export interface OrderTotals {
subtotal: number;
totalTax: number;
exclusiveTax: number;
allTaxBreakdowns: TaxBreakdown[][];
allTaxSnapshots: (string | null)[];
activeItems: OrderItemRow[];
}

export function calculateOrderTotals(db: Database, orderId: string | number): OrderTotals {
const statusPlaceholders = TERMINAL_ITEM_STATUSES.map(() => '?').join(', ');
const activeItems = db.prepare(`SELECT * FROM order_items WHERE order_id = ? AND status NOT IN (${statusPlaceholders})`)
.all(orderId, ...TERMINAL_ITEM_STATUSES) as OrderItemRow[];
let subtotal = 0;
let totalTax = 0;
let exclusiveTax = 0;
const allTaxBreakdowns: TaxBreakdown[][] = [];
const allTaxSnapshots: (string | null)[] = [];

for (const item of activeItems) {
subtotal += item.subtotal || 0;
totalTax += item.tax_amount || 0;
if (item.tax_type !== 'inclusive') {
exclusiveTax += item.tax_amount || 0;
}
if (item.tax_breakdown) {
try {
const breakdown = JSON.parse(item.tax_breakdown);
if (Array.isArray(breakdown)) allTaxBreakdowns.push(breakdown);
} catch { }
}
allTaxSnapshots.push(item.tax_snapshot || null);
}

return { subtotal, totalTax, exclusiveTax, allTaxBreakdowns, allTaxSnapshots, activeItems };
}
75 changes: 66 additions & 9 deletions tests/integration-tax.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ Module._load = function (request: string, parent: unknown, isMain: boolean) {

const {
initTestDb, createApp, startServer,
seedOwnerUser, seedCategory, seedProduct,
seedOwnerUser, seedManagerUser, seedCategory, seedProduct,
installAndActivateTestTaxPack,
api, assert, assertEqual,
getResults, closeDatabase, getDatabase, now,
Expand Down Expand Up @@ -55,6 +55,7 @@ async function main() {

// Seed data
const { authHeader } = seedOwnerUser(db);
const { authHeader: managerAuth } = seedManagerUser(db);
seedCategory(db, 'cat-tax', 'Tax Test Menu');
seedProduct(db, 'prod-tax-1', 'cat-tax', 'Premium Coffee', 1000, {
tax_category_id: 'standard',
Expand Down Expand Up @@ -198,8 +199,64 @@ async function main() {
const afterOrder = (await api(baseUrl, `/api/orders/${mixedOrderId}`, { headers: authHeader })).data.order;
assertEqual(afterOrder.subtotal, 450, "order subtotal excludes the cancelled item — didn't silently un-cancel it");

// ── Step 7: bill discount edits must use item tax, not prior bill tax ──
console.log('\n7. Edit a bill discount — tax must not compound on the prior edit');
// -- Step 7: voided items must stay excluded from order-discount tax recompute --
console.log('\n7. Void one taxable item, then discount the order - void data must stay excluded');
const voidOrderRes = await api(baseUrl, '/api/orders', {
method: 'POST',
body: {
type: 'takeaway',
items: [
{ product_id: 'prod-tax-1', quantity: 1 },
{ product_id: 'prod-tax-2', quantity: 1 },
],
},
headers: authHeader,
});
assertEqual(voidOrderRes.status, 201, 'void regression order created');
const voidOrderId = voidOrderRes.data.order.id;
const voidedItem = voidOrderRes.data.order.items.find((item: any) => item.product_id === 'prod-tax-1');

const prepareVoidItemRes = await api(baseUrl, `/api/order-items/${voidedItem.id}/status`, {
method: 'PATCH',
body: { status: 'preparing' },
headers: authHeader,
});
assertEqual(prepareVoidItemRes.status, 200, 'taxable item moved to preparing before void');

const voidItemRes = await api(baseUrl, `/api/orders/${voidOrderId}/items/${voidedItem.id}/cancel`, {
method: 'PATCH',
body: { override_pin: '1234' },
headers: managerAuth,
});
assertEqual(voidItemRes.status, 200, 'taxable item voided with manager PIN');

const discountAfterVoidRes = await api(baseUrl, `/api/orders/${voidOrderId}/discount`, {
method: 'PATCH',
body: { discount_type: 'percentage', discount_value: 10 },
headers: authHeader,
});
assertEqual(discountAfterVoidRes.status, 200, 'order discount applied after item void');
assertEqual(discountAfterVoidRes.data.order.subtotal, 500, 'subtotal excludes the voided taxable item');
assertEqual(discountAfterVoidRes.data.order.discount_amount, 50, 'discount uses only the active taxable item');
assertEqual(discountAfterVoidRes.data.order.tax_amount, 22.5, 'tax is 5% of the discounted active subtotal');
assertEqual(discountAfterVoidRes.data.order.total, 472.5, 'total includes only the discounted active item and its tax');

const postVoidBreakdown = discountAfterVoidRes.data.order.tax_breakdown;
const postVoidBreakdownGroups = Array.isArray(postVoidBreakdown?.[0]) ? postVoidBreakdown : [postVoidBreakdown];
assertEqual(postVoidBreakdownGroups.length, 1, 'tax breakdown contains only the active item');
const postVoidBreakdownEntries = postVoidBreakdownGroups.flat();
assertEqual(
Math.round(postVoidBreakdownEntries.reduce((sum: number, part: any) => sum + part.amount, 0) * 100) / 100,
22.5,
'tax breakdown reconciles to the active item tax',
);
const postVoidSnapshot = typeof discountAfterVoidRes.data.order.tax_snapshot === 'string'
? JSON.parse(discountAfterVoidRes.data.order.tax_snapshot)
: discountAfterVoidRes.data.order.tax_snapshot;
assertEqual(postVoidSnapshot.length, 1, 'tax snapshot contains only the active item');

// ── Step 8: bill discount edits must use item tax, not prior bill tax ──
console.log('\n8. Edit a bill discount — tax must not compound on the prior edit');
const mixedBillRes = await api(baseUrl, '/api/bills/generate', {
method: 'POST',
body: { order_id: mixedOrderId },
Expand Down Expand Up @@ -234,8 +291,8 @@ async function main() {
'bill discount refreshes component amounts to the final tax',
);

// ── Step 8: engine-resolved inclusive behavior survives persistence ──
console.log('\n8. Inclusive categorized product — tax stays inside the displayed price');
// ── Step 9: engine-resolved inclusive behavior survives persistence ──
console.log('\n9. Inclusive categorized product — tax stays inside the displayed price');
seedProduct(db, 'prod-tax-inclusive', 'cat-tax', 'Inclusive Meal', 105);
db.prepare(
`UPDATE products SET tax_category_id = 'standard', tax_behavior = 'inclusive'
Expand Down Expand Up @@ -270,8 +327,8 @@ async function main() {
// (0.01 for the bundled IN pack) rather than being force-rounded to a whole rupee.
assertEqual(inclusiveDiscountRes.data.bill.total, 94.5, 'inclusive tax is not added again after discount, and total is not force-rounded to a whole unit');

// ── Step 9: category writes validate and allow explicit no-tax fallback ──
console.log('\n9. Product/add-on tax category writes are validated and reversible');
// ── Step 10: category writes validate and allow explicit no-tax fallback ──
console.log('\n10. Product/add-on tax category writes are validated and reversible');
const invalidCategoryRes = await api(baseUrl, '/api/products/prod-tax-2', {
method: 'PUT',
body: { tax_category_id: 'does-not-exist' },
Expand Down Expand Up @@ -390,8 +447,8 @@ async function main() {
assertEqual(noTaxCheckoutRes.data.order.tax_breakdown.length, 0, 'product without a tax category has no order tax breakdown');
assert(!noTaxCheckoutRes.data.order.tax_snapshot, 'product without a tax category has no order tax snapshot');

// ── Step 10: payable preview, bill settlement, and payment stay reconciled ──
console.log('\n10. Tax preview and bill settlement use the same active-pack payable rounding');
// ── Step 11: payable preview, bill settlement, and payment stay reconciled ──
console.log('\n11. Tax preview and bill settlement use the same active-pack payable rounding');
db.prepare("UPDATE settings SET value = 'TH' WHERE key = 'country'").run();
seedProduct(db, 'prod-tax-th-preview', 'cat-tax', 'Thai Preview Coffee', 60, {
tax_category_id: 'standard',
Expand Down
Loading