fix(accounts): take POS summary warehouse and cost centre from one item line (#57723)

* fix(accounts): take POS summary warehouse and cost centre from one item line

Both describe an item line, not the invoice, and an invoice can carry several.
They were aggregated independently per invoice, so the report could show a
warehouse from one line beside a cost centre from another -- a pair that was
never posted.

The warehouse then becomes an outer grouping key, so the pick is not merely a
label: it decides how rows are partitioned across owner/date and therefore what
each row totals. Max() over text is a sort, and MariaDB folds case while
PostgreSQL orders by byte value, so the two engines can partition differently.

Take both off one real line instead, and the mode of payment off one real
payment line for the same reason. Sales Invoice Item is hash-named and Sales
Invoice Payment declares no autoname rule, so frappe hash-names it too -- which
keeps Min(name) free of the collation divergence that sorting text has.

* test(accounts): cover POS summary warehouse/cost-centre coherence

The existing tests post a single item line, so they cannot see this. Adds an
invoice with two lines whose warehouse and cost centre are deliberately
crossed: the higher warehouse sits on the line with the lower cost centre, so
an independently aggregated pair belongs to neither line.

* fix(accounts): pick the POS summary representative by idx, not by hash

Min(name) selected whichever child row happened to have the lowest hash, which
is arbitrary and turns on something unrelated to the data. Min(idx) selects the
first line the user actually entered: an integer, so the pick is free of
collation, and it is meaningful rather than incidental.

The join moves to (parent, idx), which is unique per parent.
This commit is contained in:
Mihir Kandoi
2026-08-03 12:12:28 +05:30
committed by GitHub
parent bf869c3426
commit d74add35d4
2 changed files with 84 additions and 15 deletions

View File

@@ -3,7 +3,7 @@
import frappe
from frappe import _
from frappe.query_builder.functions import Coalesce, Max, Sum
from frappe.query_builder.functions import Coalesce, Max, Min, Sum
from frappe.utils import cstr
@@ -128,25 +128,47 @@ def get_pos_invoice_data(filters):
sip = frappe.qb.DocType("Sales Invoice Payment")
si = frappe.qb.DocType("Sales Invoice")
# t1: one row per invoice with the summed item base_total. warehouse/cost_center are line-level and
# not grouped, so they are arbitrary per invoice -- Max() makes that pick deterministic and valid on
# Postgres (item_code was selected but never consumed downstream, so it is dropped).
t1 = (
# t1: one row per invoice with the summed item base_total. warehouse and cost_center describe an
# item line, not the invoice, and an invoice may carry several. warehouse then becomes an outer
# grouping key below, so which line wins decides how rows are partitioned and what each row totals
# -- not merely which label is shown. Max() over text is a sort, and MariaDB (case-folding) and
# PostgreSQL (byte order) resolve it differently, so take both off one real line instead.
# The representative is the first line the user entered: Min(idx) is an integer, so the pick is
# free of collation and is meaningful, rather than turning on an unrelated hash-named row.
grouped_items = (
frappe.qb.from_(sii)
.select(
sii.parent,
Sum(sii.amount).as_("base_total"),
Max(sii.warehouse).as_("warehouse"),
Max(sii.cost_center).as_("cost_center"),
)
.select(sii.parent, Sum(sii.amount).as_("base_total"), Min(sii.idx).as_("representative_idx"))
.groupby(sii.parent)
).as_("grouped_items")
representative_item = frappe.qb.DocType("Sales Invoice Item").as_("representative_item")
t1 = (
frappe.qb.from_(grouped_items)
.inner_join(representative_item)
.on(
(representative_item.parent == grouped_items.parent)
& (representative_item.idx == grouped_items.representative_idx)
)
.select(
grouped_items.parent,
grouped_items.base_total,
representative_item.warehouse,
representative_item.cost_center,
)
)
# t3: mode_of_payment per invoice (arbitrary across an invoice's payment lines -> Max() to be valid)
# t3: mode_of_payment per invoice, from one real payment line for the same reason
grouped_payments = (
frappe.qb.from_(sip).select(sip.parent, Min(sip.idx).as_("representative_idx")).groupby(sip.parent)
).as_("grouped_payments")
representative_payment = frappe.qb.DocType("Sales Invoice Payment").as_("representative_payment")
t3 = (
frappe.qb.from_(sip)
.select(sip.parent, Max(sip.mode_of_payment).as_("mode_of_payment"))
.groupby(sip.parent)
frappe.qb.from_(grouped_payments)
.inner_join(representative_payment)
.on(
(representative_payment.parent == grouped_payments.parent)
& (representative_payment.idx == grouped_payments.representative_idx)
)
.select(grouped_payments.parent, representative_payment.mode_of_payment.as_("mode_of_payment"))
)
# a: invoice-level aggregates. Grouped by the primary key (si.name), so the other plain si columns

View File

@@ -54,6 +54,53 @@ class TestSalesPaymentSummary(ERPNextTestSuite):
self.assertIn("Credit Card", next(iter(mop.values())))
self.assertNotIn("Cash", next(iter(mop.values())))
def test_pos_invoice_warehouse_and_cost_center_come_from_one_item(self):
"""The reported warehouse and cost centre must belong to the same item line.
They describe a line, not the invoice, and an invoice can carry several. Aggregating each
on its own can report a warehouse from one line beside a cost centre from another -- a pair
that was never posted. The warehouse is also an outer grouping key, so the pick decides how
rows are partitioned and what each one totals, not just what is displayed.
"""
from erpnext.stock.doctype.item.test_item import make_item
from erpnext.stock.doctype.warehouse.test_warehouse import create_warehouse
low_warehouse = create_warehouse("_Test POS Summary AAA")
high_warehouse = create_warehouse("_Test POS Summary ZZZ")
second_item = make_item("_Test POS Summary Second Item", {"is_stock_item": 0}).name
si = create_sales_invoice_record()
si.is_pos = 1
# cross the two picks: the higher warehouse is on the line with the lower cost centre, so an
# independently aggregated pair cannot belong to either line
si.items[0].warehouse = high_warehouse
si.items[0].cost_center = "Main - _TC"
si.append(
"items",
{
"item_code": second_item,
"qty": 1,
"rate": 5000,
"income_account": "Sales - _TC",
"expense_account": "Cost of Goods Sold - _TC",
"warehouse": low_warehouse,
"cost_center": "Sub - _TC",
},
)
si.append("payments", {"mode_of_payment": "Cash", "account": "_Test Cash - _TC", "amount": 15000})
si.insert()
si.submit()
posted = {(row.warehouse, row.cost_center) for row in si.items}
self.assertGreater(len(posted), 1, "fixture must post more than one distinct pair")
rows = get_pos_invoice_data(get_filters())
reported = [r for r in rows if r.get("warehouse") in {w for w, _ in posted}]
self.assertTrue(reported)
for row in reported:
self.assertIn((row["warehouse"], row["cost_center"]), posted)
def test_get_mode_of_payments_details(self):
filters = get_filters()