mirror of
https://github.com/frappe/erpnext.git
synced 2026-08-16 07:58:38 +00:00
fix(buying): make Procurement Tracker rows coherent PO lines
get_po_entries aggregated every non-key column with Max() over (PO, material_request_item), which could stitch values from different PO lines into a row that never existed (one line's item_code with another's qty and amount). Select one representative line per group instead: a subquery picks Min(child.name) per group under the same filters and the outer query reads all columns bare from that line. Row count is unchanged.
This commit is contained in:
@@ -4,7 +4,7 @@
|
||||
|
||||
import frappe
|
||||
from frappe import _
|
||||
from frappe.query_builder.functions import Max
|
||||
from frappe.query_builder.functions import Min
|
||||
from frappe.utils import flt
|
||||
|
||||
|
||||
@@ -279,39 +279,44 @@ def get_po_entries(filters):
|
||||
parent = frappe.qb.DocType("Purchase Order")
|
||||
child = frappe.qb.DocType("Purchase Order Item")
|
||||
|
||||
query = (
|
||||
# one coherent representative line per (PO, material_request_item): per-column Max() over the
|
||||
# old GROUP BY could stitch values from different PO lines into a row that never existed
|
||||
representative_lines = (
|
||||
frappe.qb.from_(parent)
|
||||
.from_(child)
|
||||
.select(
|
||||
Max(child.name).as_("name"),
|
||||
Max(child.parent).as_("parent"),
|
||||
Max(child.cost_center).as_("cost_center"),
|
||||
Max(child.project).as_("project"),
|
||||
Max(child.warehouse).as_("warehouse"),
|
||||
Max(child.material_request).as_("material_request"),
|
||||
child.material_request_item,
|
||||
Max(child.item_code).as_("item_code"),
|
||||
Max(child.stock_uom).as_("stock_uom"),
|
||||
Max(child.qty).as_("qty"),
|
||||
Max(child.amount).as_("amount"),
|
||||
Max(child.base_amount).as_("base_amount"),
|
||||
Max(child.schedule_date).as_("schedule_date"),
|
||||
Max(parent.transaction_date).as_("transaction_date"),
|
||||
Max(parent.supplier).as_("supplier"),
|
||||
Max(parent.status).as_("status"),
|
||||
Max(parent.owner).as_("owner"),
|
||||
)
|
||||
.select(Min(child.name))
|
||||
.where(
|
||||
(parent.docstatus == 1)
|
||||
& (parent.name == child.parent)
|
||||
& (parent.status.notin(("Closed", "Completed", "Cancelled")))
|
||||
)
|
||||
# Group only by the PO and material_request_item (the pre-effort key) and aggregate the rest
|
||||
# with Max(): postgres requires every non-grouped column to be aggregated, and this keeps one
|
||||
# row per (PO, material_request_item) — matching the prior MariaDB row count. Adding the PO
|
||||
# Item PK to the GROUP BY would split a multi-line PO into one row per line.
|
||||
.groupby(parent.name, child.material_request_item)
|
||||
.groupby(child.parent, child.material_request_item)
|
||||
)
|
||||
representative_lines = apply_filters_on_query(filters, parent, child, representative_lines)
|
||||
|
||||
query = (
|
||||
frappe.qb.from_(parent)
|
||||
.from_(child)
|
||||
.select(
|
||||
child.name,
|
||||
child.parent,
|
||||
child.cost_center,
|
||||
child.project,
|
||||
child.warehouse,
|
||||
child.material_request,
|
||||
child.material_request_item,
|
||||
child.item_code,
|
||||
child.stock_uom,
|
||||
child.qty,
|
||||
child.amount,
|
||||
child.base_amount,
|
||||
child.schedule_date,
|
||||
parent.transaction_date,
|
||||
parent.supplier,
|
||||
parent.status,
|
||||
parent.owner,
|
||||
)
|
||||
.where((parent.name == child.parent) & (child.name.isin(representative_lines)))
|
||||
)
|
||||
query = apply_filters_on_query(filters, parent, child, query)
|
||||
|
||||
return query.run(as_dict=True)
|
||||
|
||||
@@ -2,16 +2,15 @@
|
||||
# For license information, please see license.txt
|
||||
|
||||
|
||||
from frappe.utils import add_days, nowdate
|
||||
from frappe.utils import add_days, flt, nowdate
|
||||
|
||||
from erpnext.tests.utils import ERPNextTestSuite
|
||||
|
||||
|
||||
class TestProcurementTracker(ERPNextTestSuite):
|
||||
def test_report_executes_and_lists_po(self):
|
||||
# get_po_entries groups by (Purchase Order, material_request_item) and Max()-aggregates the
|
||||
# other child columns; this exercises that GROUP BY so the report stays valid on Postgres
|
||||
# (which rejects selecting non-grouped columns).
|
||||
# get_po_entries returns one representative line per (Purchase Order, material_request_item);
|
||||
# this exercises that query so the report stays valid on Postgres.
|
||||
from erpnext.buying.doctype.purchase_order.test_purchase_order import create_purchase_order
|
||||
from erpnext.buying.report.procurement_tracker.procurement_tracker import execute
|
||||
|
||||
@@ -22,11 +21,10 @@ class TestProcurementTracker(ERPNextTestSuite):
|
||||
self.assertTrue(columns)
|
||||
self.assertIn(po.name, {row.get("purchase_order") for row in data})
|
||||
|
||||
def test_multi_line_po_stays_one_row(self):
|
||||
# A PO can carry several lines that share the same (blank) material_request_item. get_po_entries
|
||||
# groups by (Purchase Order, material_request_item) and Max()-aggregates the rest, so such a PO
|
||||
# yields ONE row — matching the pre-effort MariaDB output. Adding the Purchase Order Item PK to
|
||||
# the GROUP BY (the regression) splits it into one row per line, changing the MariaDB row count.
|
||||
def test_multi_line_po_stays_one_coherent_row(self):
|
||||
# Lines sharing the same (blank) material_request_item collapse to ONE row, matching the
|
||||
# pre-effort MariaDB row count — and that row must be a real PO line, not a per-column
|
||||
# Max() chimera mixing one line's item_code with another line's qty/amount.
|
||||
from erpnext.buying.doctype.purchase_order.test_purchase_order import create_purchase_order
|
||||
from erpnext.buying.report.procurement_tracker.procurement_tracker import execute
|
||||
from erpnext.stock.doctype.item.test_item import make_item
|
||||
@@ -50,3 +48,10 @@ class TestProcurementTracker(ERPNextTestSuite):
|
||||
|
||||
po_rows = [row for row in data if row.get("purchase_order") == po.name]
|
||||
self.assertEqual(len(po_rows), 1)
|
||||
|
||||
real_lines = {(d.item_code, flt(d.qty), flt(d.amount)) for d in po.items}
|
||||
row = po_rows[0]
|
||||
self.assertIn(
|
||||
(row.get("item_code"), flt(row.get("quantity")), flt(row.get("purchase_order_amt"))),
|
||||
real_lines,
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user