From b1c6666d02c657227aaa5199008150d03080818d Mon Sep 17 00:00:00 2001 From: Mihir Kandoi Date: Thu, 18 Jun 2026 19:44:31 +0530 Subject: [PATCH 1/2] fix(selling): split multi-order invoice amount across its sales orders (payment terms status) payment_terms_status_for_sales_order grouped invoice rows by `sii.parent` and took `Max(sii.sales_order)`, so an invoice that bills several Sales Orders was credited in full to one arbitrary order and the rest were starved of that payment. get_so_with_invoices now returns one row per (invoice, sales_order) and splits the invoice's grand total across the orders in proportion to each order's net line amount on that invoice. A single-order invoice keeps the full grand total (ratio 1), so the common case is unchanged; the split is pure Python over deterministic SQL, so MariaDB and Postgres produce identical results (100% parity). Test (fails on the old code, passes on both engines): - test_invoice_billing_multiple_orders_splits_proportionally: one invoice billing two SOs 600/400 -> each order credited its share, summing to the grand total. Old Max(sales_order) collapsed the invoice onto one order. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../payment_terms_status_for_sales_order.py | 37 +++++++-- ...st_payment_terms_status_for_sales_order.py | 78 ++++++++++++++++++- 2 files changed, 108 insertions(+), 7 deletions(-) diff --git a/erpnext/selling/report/payment_terms_status_for_sales_order/payment_terms_status_for_sales_order.py b/erpnext/selling/report/payment_terms_status_for_sales_order/payment_terms_status_for_sales_order.py index 1f9c2ceb442..c1ad245c6ae 100644 --- a/erpnext/selling/report/payment_terms_status_for_sales_order/payment_terms_status_for_sales_order.py +++ b/erpnext/selling/report/payment_terms_status_for_sales_order/payment_terms_status_for_sales_order.py @@ -4,7 +4,8 @@ import frappe from frappe import _, qb, query_builder from frappe.query_builder import Criterion -from frappe.query_builder.functions import Max +from frappe.query_builder.functions import Max, Sum +from frappe.utils import flt from frappe.utils.dateutils import getdate @@ -230,20 +231,44 @@ def get_so_with_invoices(filters): .inner_join(soi) .on(soi.name == sii.so_detail) .select( - # grouped by the invoice (sii.parent); sales_order is arbitrary per invoice on MySQL and - # base_grand_total is constant per invoice -> Max() keeps the GROUP BY postgres-valid. - Max(sii.sales_order).as_("sales_order"), + # One row per (invoice, sales_order). An invoice can bill several Sales Orders; grouping + # by the invoice alone and taking Max(sales_order) credited the whole invoice to one + # arbitrary order and starved the rest. sales_order/invoice are GROUP BY keys and + # base_grand_total is constant per invoice; the grand total is split across the orders + # below in proportion to each order's net line amount on this invoice. + sii.sales_order.as_("sales_order"), sii.parent.as_("invoice"), - Max(si.base_grand_total).as_("invoice_amount"), + Sum(sii.base_net_amount).as_("order_net_amount"), + Max(si.base_grand_total).as_("invoice_grand_total"), ) .where((sii.sales_order.isin([x.name for x in sorders])) & (si.docstatus == 1)) - .groupby(sii.parent) + .groupby(sii.parent, sii.sales_order) ) invoices = query_inv.run(as_dict=True) + allocate_invoice_amount_across_orders(invoices) return sorders, invoices +def allocate_invoice_amount_across_orders(invoices): + """Split each invoice's grand total across the Sales Orders it bills, in proportion to each order's + net line amount on that invoice. A single-order invoice keeps the full grand total (ratio 1), so the + common case is unchanged; the arithmetic is identical on MariaDB and Postgres.""" + rows_by_invoice = {} + for row in invoices: + rows_by_invoice.setdefault(row.invoice, []).append(row) + + for rows in rows_by_invoice.values(): + total_net = sum(flt(r.order_net_amount) for r in rows) + grand_total = flt(rows[0].invoice_grand_total) + for r in rows: + if total_net: + r.invoice_amount = grand_total * flt(r.order_net_amount) / total_net + else: + # degenerate all-zero-net invoice: split evenly so both engines still agree + r.invoice_amount = grand_total / len(rows) + + def set_payment_terms_statuses(sales_orders, invoices, filters): """ compute status for payment terms with associated sales invoice using FIFO diff --git a/erpnext/selling/report/payment_terms_status_for_sales_order/test_payment_terms_status_for_sales_order.py b/erpnext/selling/report/payment_terms_status_for_sales_order/test_payment_terms_status_for_sales_order.py index ca7e338e936..d1d9cd0b7ed 100644 --- a/erpnext/selling/report/payment_terms_status_for_sales_order/test_payment_terms_status_for_sales_order.py +++ b/erpnext/selling/report/payment_terms_status_for_sales_order/test_payment_terms_status_for_sales_order.py @@ -1,7 +1,7 @@ import datetime import frappe -from frappe.utils import add_days, add_months, nowdate +from frappe.utils import add_days, add_months, flt, nowdate from erpnext.selling.doctype.sales_order.mapper import make_sales_invoice from erpnext.selling.doctype.sales_order.test_sales_order import make_sales_order @@ -399,3 +399,79 @@ class TestPaymentTermsStatusForSalesOrder(ERPNextTestSuite): # Only the first term should be pulled self.assertEqual(len(data), 1) self.assertEqual(data, expected_value) + + def test_invoice_billing_multiple_orders_splits_proportionally(self): + """An invoice that bills several Sales Orders must contribute to each, in proportion to each + order's net line amount. Grouping by the invoice alone and taking Max(sales_order) credited the + whole invoice to one arbitrary order and starved the rest. get_so_with_invoices now returns one + row per (invoice, sales_order) with the grand total split proportionally; the split is identical + on MariaDB and Postgres.""" + from erpnext.selling.report.payment_terms_status_for_sales_order.payment_terms_status_for_sales_order import ( + get_so_with_invoices, + ) + + self.create_payment_terms_template() + item = create_item(item_code="_Test PT Split Item", is_stock_item=0) + + def make_so(): + so = make_sales_order( + transaction_date="2021-06-15", + delivery_date=add_days("2021-06-15", 30), + item=item.item_code, + qty=10, + rate=100, + do_not_save=True, + ) + so.po_no = "" + so.taxes_and_charges = "" + so.taxes = "" + so.payment_terms_template = self.template.name + so.save() + so.submit() + return so + + # created in order so the OLD Max(sales_order) deterministically picks so_b and starves so_a + so_a = make_so() + so_b = make_so() + + # one invoice billing both orders, partially (so both stay in a billable status) + sinv = make_sales_invoice(so_a.name) + sinv.taxes_and_charges = "" + sinv.taxes = "" + sinv.items[0].qty = 6 # so_a: 6 * 100 = 600 net + so_b_item = so_b.items[0] + sinv.append( + "items", + { + "item_code": item.item_code, + "qty": 4, # so_b: 4 * 100 = 400 net + "rate": 100, + "sales_order": so_b.name, + "so_detail": so_b_item.name, + }, + ) + sinv.insert() + sinv.submit() + + filters = frappe._dict( + { + "company": "_Test Company", + "period_start_date": "2021-06-01", + "period_end_date": "2021-06-30", + "item": item.item_code, + } + ) + sorders, invoices = get_so_with_invoices(filters) + rows = {r.sales_order: r for r in invoices if r.invoice == sinv.name} + + # both orders are represented (the old Max(sales_order) collapsed the invoice onto one) + self.assertIn(so_a.name, rows) + self.assertIn(so_b.name, rows) + # grand total (1000, no tax) split 600 / 400 by net line amount, summing back to the grand total + self.assertAlmostEqual(rows[so_a.name].invoice_amount, 600.0, places=2) + self.assertAlmostEqual(rows[so_b.name].invoice_amount, 400.0, places=2) + self.assertAlmostEqual( + rows[so_a.name].invoice_amount + rows[so_b.name].invoice_amount, + flt(sinv.base_grand_total), + places=2, + ) From 3f6f3abf69c7ffcf4770e31f4b271b44d16706e4 Mon Sep 17 00:00:00 2001 From: Mihir Kandoi Date: Thu, 18 Jun 2026 20:48:36 +0530 Subject: [PATCH 2/2] fix(selling): make multi-order invoice split sum exactly to the grand total Address review (#56101): with 3+ orders the proportional shares could drift by a sub-cent and not sum back to the grand total, leaving the last payment term "Partly Paid". The last order (sorted, so MariaDB and Postgres agree) now absorbs the residual: grand_total - sum(prior shares). Extended the test to three orders. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../payment_terms_status_for_sales_order.py | 20 ++++++---- ...st_payment_terms_status_for_sales_order.py | 37 +++++++++---------- 2 files changed, 30 insertions(+), 27 deletions(-) diff --git a/erpnext/selling/report/payment_terms_status_for_sales_order/payment_terms_status_for_sales_order.py b/erpnext/selling/report/payment_terms_status_for_sales_order/payment_terms_status_for_sales_order.py index c1ad245c6ae..fee76bccc3b 100644 --- a/erpnext/selling/report/payment_terms_status_for_sales_order/payment_terms_status_for_sales_order.py +++ b/erpnext/selling/report/payment_terms_status_for_sales_order/payment_terms_status_for_sales_order.py @@ -251,22 +251,26 @@ def get_so_with_invoices(filters): def allocate_invoice_amount_across_orders(invoices): - """Split each invoice's grand total across the Sales Orders it bills, in proportion to each order's - net line amount on that invoice. A single-order invoice keeps the full grand total (ratio 1), so the - common case is unchanged; the arithmetic is identical on MariaDB and Postgres.""" + """Split each invoice's grand total across the Sales Orders it bills, proportional to each order's net + line amount. A single-order invoice keeps the full grand total (ratio 1). The last order (sorted, so + both engines agree) absorbs the rounding residual, so the shares always sum back to the grand total.""" rows_by_invoice = {} for row in invoices: rows_by_invoice.setdefault(row.invoice, []).append(row) for rows in rows_by_invoice.values(): + rows.sort(key=lambda r: r.sales_order) total_net = sum(flt(r.order_net_amount) for r in rows) grand_total = flt(rows[0].invoice_grand_total) - for r in rows: - if total_net: - r.invoice_amount = grand_total * flt(r.order_net_amount) / total_net - else: - # degenerate all-zero-net invoice: split evenly so both engines still agree + if not total_net: + for r in rows: r.invoice_amount = grand_total / len(rows) + continue + allocated = 0.0 + for r in rows[:-1]: + r.invoice_amount = grand_total * flt(r.order_net_amount) / total_net + allocated += r.invoice_amount + rows[-1].invoice_amount = grand_total - allocated def set_payment_terms_statuses(sales_orders, invoices, filters): diff --git a/erpnext/selling/report/payment_terms_status_for_sales_order/test_payment_terms_status_for_sales_order.py b/erpnext/selling/report/payment_terms_status_for_sales_order/test_payment_terms_status_for_sales_order.py index d1d9cd0b7ed..18a077581e3 100644 --- a/erpnext/selling/report/payment_terms_status_for_sales_order/test_payment_terms_status_for_sales_order.py +++ b/erpnext/selling/report/payment_terms_status_for_sales_order/test_payment_terms_status_for_sales_order.py @@ -430,26 +430,26 @@ class TestPaymentTermsStatusForSalesOrder(ERPNextTestSuite): so.submit() return so - # created in order so the OLD Max(sales_order) deterministically picks so_b and starves so_a so_a = make_so() so_b = make_so() + so_c = make_so() - # one invoice billing both orders, partially (so both stay in a billable status) + # one invoice billing all three orders, partially (so each stays in a billable status) sinv = make_sales_invoice(so_a.name) sinv.taxes_and_charges = "" sinv.taxes = "" - sinv.items[0].qty = 6 # so_a: 6 * 100 = 600 net - so_b_item = so_b.items[0] - sinv.append( - "items", - { - "item_code": item.item_code, - "qty": 4, # so_b: 4 * 100 = 400 net - "rate": 100, - "sales_order": so_b.name, - "so_detail": so_b_item.name, - }, - ) + sinv.items[0].qty = 6 # so_a: 600 net + for so, qty in ((so_b, 4), (so_c, 5)): # so_b: 400, so_c: 500 + sinv.append( + "items", + { + "item_code": item.item_code, + "qty": qty, + "rate": 100, + "sales_order": so.name, + "so_detail": so.items[0].name, + }, + ) sinv.insert() sinv.submit() @@ -464,14 +464,13 @@ class TestPaymentTermsStatusForSalesOrder(ERPNextTestSuite): sorders, invoices = get_so_with_invoices(filters) rows = {r.sales_order: r for r in invoices if r.invoice == sinv.name} - # both orders are represented (the old Max(sales_order) collapsed the invoice onto one) - self.assertIn(so_a.name, rows) - self.assertIn(so_b.name, rows) - # grand total (1000, no tax) split 600 / 400 by net line amount, summing back to the grand total + # each order gets its share (old Max(sales_order) collapsed the invoice onto one), and the shares + # always sum back to the grand total (the last order absorbs any rounding residual) self.assertAlmostEqual(rows[so_a.name].invoice_amount, 600.0, places=2) self.assertAlmostEqual(rows[so_b.name].invoice_amount, 400.0, places=2) + self.assertAlmostEqual(rows[so_c.name].invoice_amount, 500.0, places=2) self.assertAlmostEqual( - rows[so_a.name].invoice_amount + rows[so_b.name].invoice_amount, + sum(rows[so.name].invoice_amount for so in (so_a, so_b, so_c)), flt(sinv.base_grand_total), places=2, )