diff --git a/erpnext/accounts/report/sales_payment_summary/sales_payment_summary.py b/erpnext/accounts/report/sales_payment_summary/sales_payment_summary.py index f5571f756bc..b26914e248b 100644 --- a/erpnext/accounts/report/sales_payment_summary/sales_payment_summary.py +++ b/erpnext/accounts/report/sales_payment_summary/sales_payment_summary.py @@ -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 diff --git a/erpnext/accounts/report/sales_payment_summary/test_sales_payment_summary.py b/erpnext/accounts/report/sales_payment_summary/test_sales_payment_summary.py index d2ee6f47939..dc7dc446c11 100644 --- a/erpnext/accounts/report/sales_payment_summary/test_sales_payment_summary.py +++ b/erpnext/accounts/report/sales_payment_summary/test_sales_payment_summary.py @@ -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()