fix(accounts): match returns to source invoice items (backport #58250) (#58264)

Co-authored-by: Mihir Kandoi <kandoimihir@gmail.com>
2026-08-19 09:28:46 +00:00 · 2026-08-18 09:08:31 +00:00
parent a62a949b2a
commit 9b0db26c25
2 changed files with 393 additions and 32 deletions
--- 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
--- 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