From 9b0db26c2552c62f33eca85a0d76383f7925adcc Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Tue, 18 Aug 2026 09:08:31 +0000 Subject: [PATCH] fix(accounts): match returns to source invoice items (backport #58250) (#58264) Co-authored-by: Mihir Kandoi --- .../report/gross_profit/gross_profit.py | 124 ++++++-- .../report/gross_profit/test_gross_profit.py | 301 +++++++++++++++++- 2 files changed, 393 insertions(+), 32 deletions(-) diff --git a/erpnext/accounts/report/gross_profit/gross_profit.py b/erpnext/accounts/report/gross_profit/gross_profit.py index 838e086ddd8..d73faead0f7 100644 --- a/erpnext/accounts/report/gross_profit/gross_profit.py +++ b/erpnext/accounts/report/gross_profit/gross_profit.py @@ -519,6 +519,7 @@ class GrossProfitGenerator: self.load_non_stock_items() self.get_returned_invoice_items() + self.allocate_legacy_return_items() self.process() def process(self): @@ -535,6 +536,8 @@ class GrossProfitGenerator: base_amount = 0 for row in reversed(self.si_list): + sales_invoice_item = row.item_row + if self.filters.get("group_by") == "Monthly": row.monthly = formatdate(row.posting_date, "MMM YYYY") @@ -597,7 +600,7 @@ class GrossProfitGenerator: row.buying_rate, row.base_rate = 0.0, 0.0 if self.is_not_invoice_row(row): - self.update_return_invoices(row) + self.update_return_invoices(row, sales_invoice_item) if grouped_by_invoice and row.indent == 1.0: buying_amount += row.buying_amount @@ -624,30 +627,32 @@ class GrossProfitGenerator: if self.grouped: self.get_average_rate_based_on_group_by() - def update_return_invoices(self, row): - if row.parent in self.returned_invoices and row.item_code in self.returned_invoices[row.parent]: - returned_item_rows = self.returned_invoices[row.parent][row.item_code] - for returned_item_row in returned_item_rows: - # returned_items 'qty' should be stateful - if returned_item_row.qty != 0: - if row.qty >= abs(returned_item_row.qty): - row.qty += returned_item_row.qty - row.base_amount += flt(returned_item_row.base_amount, self.currency_precision) - returned_item_row.qty = 0 - returned_item_row.base_amount = 0 + def update_return_invoices(self, row, sales_invoice_item): + returned_item_rows = self.returned_invoices.get(row.parent, {}).get(sales_invoice_item) + if not returned_item_rows: + return - else: - row.qty = 0 - row.base_amount = 0 - returned_item_row.qty += row.qty - returned_item_row.base_amount += row.base_amount + for returned_item_row in returned_item_rows: + # returned_items 'qty' should be stateful + if returned_item_row.qty != 0: + if row.qty >= abs(returned_item_row.qty): + row.qty += returned_item_row.qty + row.base_amount += flt(returned_item_row.base_amount, self.currency_precision) + returned_item_row.qty = 0 + returned_item_row.base_amount = 0 - if row.delivered_by_supplier: - buying_amount = self.get_drop_ship_buying_amount(row) - if buying_amount is not None: - row.buying_amount = flt(buying_amount, self.currency_precision) - else: - row.buying_amount = flt(flt(row.qty) * flt(row.buying_rate), self.currency_precision) + else: + returned_item_row.qty += row.qty + returned_item_row.base_amount += row.base_amount + row.qty = 0 + row.base_amount = 0 + + if row.delivered_by_supplier: + buying_amount = self.get_drop_ship_buying_amount(row) + if buying_amount is not None: + row.buying_amount = flt(buying_amount, self.currency_precision) + else: + row.buying_amount = flt(flt(row.qty) * flt(row.buying_rate), self.currency_precision) def get_average_rate_based_on_group_by(self): for key in list(self.grouped): @@ -728,7 +733,8 @@ class GrossProfitGenerator: returned_invoices = frappe.db.sql( """ select - si.name, si_item.item_code, si_item.stock_qty as qty, si_item.base_net_amount as base_amount, si.return_against + si.name, si_item.item_code, si_item.sales_invoice_item, si_item.stock_qty as qty, + si_item.base_net_amount as base_amount, si.return_against from `tabSales Invoice` si, `tabSales Invoice Item` si_item where @@ -742,17 +748,73 @@ class GrossProfitGenerator: ) self.returned_invoices = frappe._dict() + self.legacy_returned_invoices = frappe._dict() for inv in returned_invoices: - self.returned_invoices.setdefault(inv.return_against, frappe._dict()).setdefault( - inv.item_code, [] + invoice_returns = ( + self.returned_invoices if inv.sales_invoice_item else self.legacy_returned_invoices + ) + invoice_returns.setdefault(inv.return_against, frappe._dict()).setdefault( + inv.sales_invoice_item or inv.item_code, [] ).append(inv) - def skip_row(self, row): - if self.filters.get("group_by") != "Invoice": - if not row.get(scrub(self.filters.get("group_by", ""))): - return True + def allocate_legacy_return_items(self): + source_invoice_items = {} + for row in reversed(self.si_list): + if row.is_return or not row.parent or self.skip_row(row): + continue - return False + source_invoice_items.setdefault((row.parent, row.item_code), {}).setdefault(row.item_row, row.qty) + + for invoice, legacy_invoice_items in self.legacy_returned_invoices.items(): + returned_invoice_items = self.returned_invoices.setdefault(invoice, frappe._dict()) + for item_code, legacy_item_rows in legacy_invoice_items.items(): + targets = self.get_legacy_return_targets( + source_invoice_items.get((invoice, item_code), {}), returned_invoice_items + ) + for legacy_item_row in legacy_item_rows: + self.allocate_legacy_return_item(legacy_item_row, targets, returned_invoice_items) + + def get_legacy_return_targets(self, source_invoice_items, returned_invoice_items): + targets = [] + for item_row, qty in source_invoice_items.items(): + linked_return_qty = sum( + flt(returned_item.qty) for returned_item in returned_invoice_items.get(item_row, []) + ) + if available_qty := max(flt(qty) + linked_return_qty, 0): + targets.append(frappe._dict(item_row=item_row, available_qty=available_qty)) + + targets.sort(key=lambda target: bool(returned_invoice_items.get(target.item_row))) + return targets + + def allocate_legacy_return_item(self, legacy_item_row, targets, returned_invoice_items): + remaining_qty = abs(flt(legacy_item_row.qty)) + remaining_base_amount = flt(legacy_item_row.base_amount) + if not remaining_qty: + return + + qty_sign = -1 if legacy_item_row.qty < 0 else 1 + for target in targets: + if not target.available_qty: + continue + + allocated_qty = min(target.available_qty, remaining_qty) + allocated_item_row = frappe._dict(legacy_item_row.copy()) + allocated_item_row.qty = qty_sign * allocated_qty + allocated_item_row.base_amount = remaining_base_amount * allocated_qty / remaining_qty + returned_invoice_items.setdefault(target.item_row, []).append(allocated_item_row) + + target.available_qty -= allocated_qty + remaining_qty -= allocated_qty + remaining_base_amount -= allocated_item_row.base_amount + if not remaining_qty: + break + + def skip_row(self, row): + group_by = self.filters.get("group_by") + if group_by in {"Invoice", "Monthly"}: + return False + + return not row.get(scrub(group_by)) def get_buying_amount_from_product_bundle(self, row, product_bundle): buying_amount = 0.0 diff --git a/erpnext/accounts/report/gross_profit/test_gross_profit.py b/erpnext/accounts/report/gross_profit/test_gross_profit.py index 0c7054d7c76..43f03513c71 100644 --- a/erpnext/accounts/report/gross_profit/test_gross_profit.py +++ b/erpnext/accounts/report/gross_profit/test_gross_profit.py @@ -4,7 +4,7 @@ from frappe.utils import add_days, flt, get_first_day, get_last_day, nowdate from erpnext.accounts.doctype.sales_invoice.sales_invoice import make_delivery_note, make_sales_return from erpnext.accounts.doctype.sales_invoice.test_sales_invoice import create_sales_invoice -from erpnext.accounts.report.gross_profit.gross_profit import execute +from erpnext.accounts.report.gross_profit.gross_profit import GrossProfitGenerator, execute from erpnext.stock.doctype.delivery_note.delivery_note import make_sales_invoice from erpnext.stock.doctype.delivery_note.test_delivery_note import create_delivery_note from erpnext.stock.doctype.item.test_item import create_item @@ -726,6 +726,305 @@ class TestGrossProfit(ERPNextTestSuite): self.assertEqual(first_invoice_row.buying_amount, 160) self.assertEqual(first_invoice_row.gross_profit, 40) + def test_drop_ship_return_matches_sales_invoice_item(self): + from erpnext.buying.doctype.purchase_order.purchase_order import make_purchase_invoice + from erpnext.selling.doctype.sales_order.sales_order import make_purchase_order, make_sales_invoice + from erpnext.selling.doctype.sales_order.test_sales_order import make_sales_order + from erpnext.stock.doctype.item.test_item import make_item + + item = make_item( + "_Test Drop Ship Consolidated Return Item", + properties={"is_stock_item": 1, "delivered_by_supplier": 1}, + ) + sales_orders = [] + for qty, selling_rate, buying_rate in [(4, 100, 50), (6, 200, 80)]: + sales_order = make_sales_order(item=item.name, qty=qty, rate=selling_rate, do_not_submit=True) + sales_order.items[0].delivered_by_supplier = 1 + sales_order.items[0].supplier = "_Test Supplier" + sales_order.submit() + sales_orders.append(sales_order) + + purchase_order = make_purchase_order(sales_order.name, selected_items=[sales_order.items[0]])[0] + purchase_order.items[0].rate = buying_rate + purchase_order.supplier = "_Test Supplier" + purchase_order.submit() + make_purchase_invoice(purchase_order.name).submit() + + sales_invoice = make_sales_invoice(sales_orders[0].name) + sales_invoice = make_sales_invoice(sales_orders[1].name, target_doc=sales_invoice).submit() + sales_return = make_sales_return(sales_invoice.name) + sales_return.set("items", [sales_return.items[0]]) + sales_return.items[0].qty = -1 + sales_return.submit() + + filters = frappe._dict( + company=sales_invoice.company, + from_date=sales_invoice.posting_date, + to_date=sales_invoice.posting_date, + group_by="Invoice", + ) + _, data = execute(filters=filters) + invoice_rows = [row for row in data if row.parent_invoice == sales_invoice.name and row.indent == 1] + invoice_rows.sort(key=lambda row: row["avg._selling_rate"]) + self.assertEqual([row.qty for row in invoice_rows], [3, 6]) + self.assertEqual([row.buying_amount for row in invoice_rows], [150, 480]) + + def test_return_matches_sales_invoice_item_for_delivery_note(self): + make_stock_entry( + company=self.company, + item_code=self.item, + target=self.warehouse, + qty=4, + basic_rate=50, + ) + delivery_note = self.create_delivery_note(qty=4, rate=100) + sales_invoice = make_sales_invoice(delivery_note.name).submit() + sales_return = make_sales_return(sales_invoice.name) + sales_return.items[0].qty = -1 + sales_return.submit() + + filters = frappe._dict( + company=sales_invoice.company, + from_date=sales_invoice.posting_date, + to_date=sales_invoice.posting_date, + group_by="Invoice", + ) + _, data = execute(filters=filters) + invoice_row = next( + row for row in data if row.parent_invoice == sales_invoice.name and row.indent == 1 + ) + self.assertEqual(invoice_row.qty, 3) + self.assertEqual(invoice_row.selling_amount, 300) + + def test_return_combines_linked_and_legacy_item_buckets(self): + sales_invoice = self.create_sales_invoice(qty=4, rate=100) + linked_return = make_sales_return(sales_invoice.name) + linked_return.items[0].qty = -1 + linked_return.submit() + + legacy_return = make_sales_return(sales_invoice.name) + legacy_return.items[0].qty = -1 + legacy_return.submit() + frappe.db.set_value("Sales Invoice Item", legacy_return.items[0].name, "sales_invoice_item", None) + + filters = frappe._dict( + company=sales_invoice.company, + from_date=sales_invoice.posting_date, + to_date=sales_invoice.posting_date, + group_by="Invoice", + ) + _, data = execute(filters=filters) + invoice_row = next( + row for row in data if row.parent_invoice == sales_invoice.name and row.indent == 1 + ) + self.assertEqual(invoice_row.qty, 2) + self.assertEqual(invoice_row.selling_amount, 200) + + @ERPNextTestSuite.change_settings("Selling Settings", {"allow_multiple_items": True}) + def test_legacy_return_prefers_item_without_linked_return(self): + sales_invoice = self.create_sales_invoice(qty=2, rate=100, do_not_submit=True) + second_item = frappe.copy_doc(sales_invoice.items[0], ignore_no_copy=False) + second_item.rate = 200 + sales_invoice.append("items", second_item) + sales_invoice.submit() + + linked_return = make_sales_return(sales_invoice.name) + linked_return.set("items", [linked_return.items[0]]) + linked_return.items[0].qty = -1 + linked_return.submit() + + legacy_return = make_sales_return(sales_invoice.name) + legacy_return.set("items", [legacy_return.items[1]]) + legacy_return.items[0].qty = -1 + legacy_return.submit() + frappe.db.set_value("Sales Invoice Item", legacy_return.items[0].name, "sales_invoice_item", None) + + filters = frappe._dict( + company=sales_invoice.company, + from_date=sales_invoice.posting_date, + to_date=sales_invoice.posting_date, + group_by="Invoice", + ) + _, data = execute(filters=filters) + invoice_rows = [row for row in data if row.parent_invoice == sales_invoice.name and row.indent == 1] + invoice_rows.sort(key=lambda row: row["avg._selling_rate"]) + self.assertEqual([row.qty for row in invoice_rows], [1, 1]) + self.assertEqual([row.selling_amount for row in invoice_rows], [100, 200]) + + def test_legacy_return_remainder_spills_into_linked_item(self): + invoice = "SINV-TEST-RETURN-ALLOCATION" + linked_item = "SINV-ITEM-LINKED" + unlinked_item = "SINV-ITEM-LEGACY" + generator = GrossProfitGenerator.__new__(GrossProfitGenerator) + generator.currency_precision = 3 + generator.filters = frappe._dict(group_by="Invoice") + generator.returned_invoices = frappe._dict( + {invoice: frappe._dict({linked_item: [frappe._dict(qty=-1, base_amount=-100)]})} + ) + generator.legacy_returned_invoices = frappe._dict( + {invoice: frappe._dict({self.item: [frappe._dict(qty=-2, base_amount=-200)]})} + ) + linked_row = frappe._dict( + parent=invoice, + item_code=self.item, + item_row=linked_item, + is_return=False, + qty=3, + base_amount=300, + buying_rate=50, + delivered_by_supplier=False, + ) + unlinked_row = frappe._dict( + parent=invoice, + item_code=self.item, + item_row=unlinked_item, + is_return=False, + qty=1, + base_amount=100, + buying_rate=50, + delivered_by_supplier=False, + ) + + generator.si_list = [unlinked_row, linked_row] + generator.allocate_legacy_return_items() + generator.update_return_invoices(linked_row, linked_item) + generator.update_return_invoices(unlinked_row, unlinked_item) + + self.assertEqual((linked_row.qty, linked_row.base_amount), (1, 100)) + self.assertEqual((unlinked_row.qty, unlinked_row.base_amount), (0, 0)) + + def test_legacy_return_ignores_skipped_group_rows(self): + invoice = "SINV-TEST-SKIPPED-RETURN-ALLOCATION" + visible_item = "SINV-ITEM-WITH-PROJECT" + skipped_item = "SINV-ITEM-WITHOUT-PROJECT" + generator = GrossProfitGenerator.__new__(GrossProfitGenerator) + generator.currency_precision = 3 + generator.filters = frappe._dict(group_by="Project") + generator.returned_invoices = frappe._dict( + {invoice: frappe._dict({visible_item: [frappe._dict(qty=-1, base_amount=-100)]})} + ) + generator.legacy_returned_invoices = frappe._dict( + {invoice: frappe._dict({self.item: [frappe._dict(qty=-1, base_amount=-100)]})} + ) + visible_row = frappe._dict( + parent=invoice, + item_code=self.item, + item_row=visible_item, + is_return=False, + project="_Test Project", + qty=2, + base_amount=200, + buying_rate=50, + delivered_by_supplier=False, + ) + skipped_row = frappe._dict( + parent=invoice, + item_code=self.item, + item_row=skipped_item, + is_return=False, + project=None, + qty=1, + ) + + generator.si_list = [visible_row, skipped_row] + generator.allocate_legacy_return_items() + generator.update_return_invoices(visible_row, visible_item) + + self.assertNotIn(skipped_item, generator.returned_invoices[invoice]) + self.assertEqual((visible_row.qty, visible_row.base_amount), (0, 0)) + + def test_monthly_group_allocates_legacy_return(self): + invoice = "SINV-TEST-MONTHLY-RETURN-ALLOCATION" + item_row = "SINV-ITEM-MONTHLY-RETURN" + generator = GrossProfitGenerator.__new__(GrossProfitGenerator) + generator.currency_precision = 3 + generator.filters = frappe._dict(group_by="Monthly") + generator.returned_invoices = frappe._dict() + generator.legacy_returned_invoices = frappe._dict( + {invoice: frappe._dict({self.item: [frappe._dict(qty=-1, base_amount=-100)]})} + ) + invoice_row = frappe._dict( + parent=invoice, + item_code=self.item, + item_row=item_row, + is_return=False, + posting_date=nowdate(), + qty=1, + base_amount=100, + buying_rate=50, + delivered_by_supplier=False, + ) + + generator.si_list = [invoice_row] + generator.allocate_legacy_return_items() + generator.update_return_invoices(invoice_row, item_row) + + self.assertEqual((invoice_row.qty, invoice_row.base_amount), (0, 0)) + + def test_return_remainder_stays_available_for_next_row(self): + invoice = "SINV-TEST-RETURN-REMAINDER" + item_row = "SINV-ITEM-RETURN-REMAINDER" + returned_item = frappe._dict(qty=-2, base_amount=-200) + generator = GrossProfitGenerator.__new__(GrossProfitGenerator) + generator.currency_precision = 3 + generator.returned_invoices = frappe._dict({invoice: frappe._dict({item_row: [returned_item]})}) + first_row = frappe._dict( + parent=invoice, + item_code=self.item, + qty=1, + base_amount=100, + buying_rate=50, + delivered_by_supplier=False, + ) + second_row = first_row.copy() + + generator.update_return_invoices(first_row, item_row) + self.assertEqual((returned_item.qty, returned_item.base_amount), (-1, -100)) + + generator.update_return_invoices(second_row, item_row) + self.assertEqual((returned_item.qty, returned_item.base_amount), (0, 0)) + self.assertEqual((first_row.qty, second_row.qty), (0, 0)) + + @ERPNextTestSuite.change_settings("Selling Settings", {"allow_multiple_items": True}) + def test_return_keeps_buying_amount_of_unreturned_row(self): + unreturned_item = create_item( + "_Test Gross Profit Unreturned Item", warehouse=self.warehouse, company=self.company + ) + make_stock_entry( + company=self.company, + item_code=unreturned_item.name, + target=self.warehouse, + qty=40000, + basic_rate=33.33333, + ) + sales_invoice = self.create_sales_invoice(qty=1, rate=100, do_not_submit=True) + second_item = frappe.copy_doc(sales_invoice.items[0], ignore_no_copy=False) + second_item.item_code = unreturned_item.name + second_item.item_name = unreturned_item.name + second_item.qty = 30000 + sales_invoice.append("items", second_item) + sales_invoice.submit() + + sales_return = make_sales_return(sales_invoice.name) + sales_return.set("items", [sales_return.items[0]]) + sales_return.items[0].qty = -1 + sales_return.submit() + + filters = frappe._dict( + company=sales_invoice.company, + from_date=sales_invoice.posting_date, + to_date=sales_invoice.posting_date, + group_by="Invoice", + ) + _, data = execute(filters=filters) + invoice_row = next( + row + for row in data + if row.parent_invoice == sales_invoice.name and row.item_code == unreturned_item.name + ) + self.assertEqual(invoice_row.qty, 30000) + self.assertEqual(invoice_row.buying_amount, 999999.9) + def create_drop_ship_order(self, qty=10, selling_rate=100, buying_rate=80): from erpnext.buying.doctype.purchase_order.purchase_order import make_purchase_invoice from erpnext.selling.doctype.sales_order.sales_order import make_purchase_order