fix: do not map the same row twice in "Get Items From" (backport #58617) (#58618)

Co-authored-by: Raffael Meyer <14891507+barredterra@users.noreply.github.com>

erpnext/accounts/doctype/purchase_invoice/purchase_invoice.py

@@ -40,6 +40,7 @@ from erpnext.assets.doctype.asset_category.asset_category import get_asset_categ
 from erpnext.buying.utils import check_on_hold_or_closed_status
 from erpnext.controllers.accounts_controller import merge_taxes, validate_account_head
 from erpnext.controllers.buying_controller import BuyingController
+from erpnext.controllers.mapper import get_qty_already_mapped
 from erpnext.stock import get_warehouse_account_map
 from erpnext.stock.doctype.purchase_receipt.purchase_receipt import (
 	get_item_account_wise_additional_cost,
@@ -2201,6 +2202,11 @@ def make_purchase_receipt(source_name, target_doc=None, args=None):
 	if isinstance(args, str):
 		args = json.loads(args)
 
+	mapped_qty_by_item = get_qty_already_mapped(target_doc, "purchase_invoice_item")
+
+	def received_and_mapped_qty(obj):
+		return flt(obj.received_qty) + flt(mapped_qty_by_item.get(obj.name, 0))
+
 	def post_parent_process(source_parent, target_parent):
 		remove_items_with_zero_qty(target_parent)
 		set_missing_values(source_parent, target_parent)
@@ -2215,13 +2221,13 @@ def make_purchase_receipt(source_name, target_doc=None, args=None):
 		target_parent.run_method("calculate_taxes_and_totals")
 
 	def update_item(obj, target, source_parent):
-		target.qty = flt(obj.qty) - flt(obj.received_qty)
+		pending_qty = flt(obj.qty) - received_and_mapped_qty(obj)
-		target.received_qty = flt(obj.qty) - flt(obj.received_qty)
+
-		target.stock_qty = (flt(obj.qty) - flt(obj.received_qty)) * flt(obj.conversion_factor)
+		target.qty = pending_qty
-		target.amount = (flt(obj.qty) - flt(obj.received_qty)) * flt(obj.rate)
+		target.received_qty = pending_qty
-		target.base_amount = (
+		target.stock_qty = pending_qty * flt(obj.conversion_factor)
-			(flt(obj.qty) - flt(obj.received_qty)) * flt(obj.rate) * flt(source_parent.conversion_rate)
+		target.amount = pending_qty * flt(obj.rate)
-		)
+		target.base_amount = pending_qty * flt(obj.rate) * flt(source_parent.conversion_rate)
 
 	def select_item(d):
 		filtered_items = args.get("filtered_children", [])
@@ -2251,7 +2257,8 @@ def make_purchase_receipt(source_name, target_doc=None, args=None):
 					"wip_composite_asset": "wip_composite_asset",
 				},
 				"postprocess": update_item,
-				"condition": lambda doc: abs(doc.received_qty) < abs(doc.qty) and select_item(doc),
+				"condition": lambda doc: abs(received_and_mapped_qty(doc)) < abs(doc.qty)
+				and select_item(doc),
 			},
 			"Purchase Taxes and Charges": {
 				"doctype": "Purchase Taxes and Charges",

erpnext/buying/doctype/purchase_order/purchase_order.py

@@ -21,6 +21,7 @@ from erpnext.accounts.doctype.tax_withholding_category.tax_withholding_category
 from erpnext.accounts.party import get_party_account, get_party_account_currency
 from erpnext.buying.utils import check_on_hold_or_closed_status, validate_for_items
 from erpnext.controllers.buying_controller import BuyingController
+from erpnext.controllers.mapper import get_qty_already_mapped
 from erpnext.manufacturing.doctype.blanket_order.blanket_order import (
 	validate_against_blanket_order,
 )
@@ -737,13 +738,16 @@ def make_purchase_receipt(source_name, target_doc=None, args=None):
 	def is_unit_price_row(source):
 		return has_unit_price_items and source.qty == 0
 
+	mapped_qty_by_item = get_qty_already_mapped(target_doc, "purchase_order_item")
+
 	def update_item(obj, target, source_parent):
-		target.qty = flt(obj.qty) if is_unit_price_row(obj) else flt(obj.qty) - flt(obj.received_qty)
+		received_qty = flt(obj.received_qty) + flt(mapped_qty_by_item.get(obj.name, 0))
-		target.stock_qty = (flt(obj.qty) - flt(obj.received_qty)) * flt(obj.conversion_factor)
+		pending_qty = flt(obj.qty) - received_qty
-		target.amount = (flt(obj.qty) - flt(obj.received_qty)) * flt(obj.rate)
+
-		target.base_amount = (
+		target.qty = flt(obj.qty) if is_unit_price_row(obj) else pending_qty
-			(flt(obj.qty) - flt(obj.received_qty)) * flt(obj.rate) * flt(source_parent.conversion_rate)
+		target.stock_qty = pending_qty * flt(obj.conversion_factor)
-		)
+		target.amount = pending_qty * flt(obj.rate)
+		target.base_amount = pending_qty * flt(obj.rate) * flt(source_parent.conversion_rate)
 
 	def select_item(d):
 		filtered_items = args.get("filtered_children", [])
@@ -775,7 +779,9 @@ def make_purchase_receipt(source_name, target_doc=None, args=None):
 				},
 				"postprocess": update_item,
 				"condition": lambda doc: (
-					True if is_unit_price_row(doc) else abs(doc.received_qty) < abs(doc.qty)
+					doc.name not in mapped_qty_by_item
+					if is_unit_price_row(doc)
+					else abs(doc.received_qty) + abs(mapped_qty_by_item.get(doc.name, 0)) < abs(doc.qty)
 				)
 				and doc.delivered_by_supplier != 1
 				and select_item(doc),
@@ -837,9 +843,13 @@ def get_mapped_purchase_invoice(source_name, target_doc=None, ignore_permissions
 		)
 		return query.run(pluck="qty")[0] or 0
 
+	mapped_qty_by_item = get_qty_already_mapped(target_doc, "po_detail")
+
+	def get_billed_and_mapped_qty(po_item_name):
+		return flt(get_billed_qty(po_item_name)) + flt(mapped_qty_by_item.get(po_item_name, 0))
+
 	def update_item(obj, target, source_parent):
-		billed_qty = flt(get_billed_qty(obj.name))
+		target.qty = flt(obj.qty) - get_billed_and_mapped_qty(obj.name)
-		target.qty = flt(obj.qty) - billed_qty
 
 		item = get_item_defaults(target.item_code, source_parent.company)
 		item_group = get_item_group_defaults(target.item_code, source_parent.company)
@@ -882,6 +892,7 @@ def get_mapped_purchase_invoice(source_name, target_doc=None, ignore_permissions
 				or abs(doc.billed_amt) < abs(doc.amount)
 				or doc.qty > flt(get_billed_qty(doc.name))
 			)
+			and (doc.name not in mapped_qty_by_item or doc.qty > get_billed_and_mapped_qty(doc.name))
 			and select_item(doc),
 		},
 		"Purchase Taxes and Charges": {"doctype": "Purchase Taxes and Charges", "reset_value": True},

erpnext/buying/doctype/supplier_quotation/supplier_quotation.py

@@ -11,6 +11,7 @@ from frappe.utils import flt, getdate, nowdate
 
 from erpnext.buying.utils import validate_for_items
 from erpnext.controllers.buying_controller import BuyingController
+from erpnext.controllers.mapper import get_qty_already_mapped
 
 form_grid_templates = {"items": "templates/form_grid/item_grid.html"}
 
@@ -243,6 +244,8 @@ def make_purchase_order(source_name, target_doc=None, args=None):
 	if isinstance(args, str):
 		args = json.loads(args)
 
+	mapped_items = get_qty_already_mapped(target_doc, "supplier_quotation_item")
+
 	def set_missing_values(source, target):
 		target.run_method("set_missing_values")
 		target.run_method("get_schedule_dates")
@@ -277,7 +280,8 @@ def make_purchase_order(source_name, target_doc=None, args=None):
 					["sales_order", "sales_order"],
 				],
 				"postprocess": update_item,
-				"condition": select_item,
+				# no qty tracking between the two, so dedupe on the row reference alone
+				"condition": lambda d: d.name not in mapped_items and select_item(d),
 			},
 			"Purchase Taxes and Charges": {
 				"doctype": "Purchase Taxes and Charges",

erpnext/controllers/mapper.py

@@ -0,0 +1,25 @@
+# Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors
+# License: GNU General Public License v3. See license.txt
+
+import frappe
+from frappe.utils import flt
+
+
+def get_qty_already_mapped(target_doc, ref_field: str, qty_field: str = "qty") -> frappe._dict:
+	"""Return a map: {source row name: qty} of rows already mapped into the target document.
+
+	"Get Items From" passes the in-progress (unsaved) document back as `target_doc`. Its rows
+	are invisible to the pending-qty queries in the mappers, which only count submitted
+	documents -- so without this, selecting the same source document twice maps every row
+	again. Rows are keyed by `ref_field` (dn_detail, so_detail, ...), and a row is present in
+	the map even when its qty is 0, so mappers without qty tracking can dedupe on presence.
+	"""
+	if isinstance(target_doc, str):
+		target_doc = frappe.parse_json(target_doc)
+
+	qty_map = frappe._dict()
+	for row in (target_doc and target_doc.get("items")) or []:
+		if ref := row.get(ref_field):
+			qty_map[ref] = qty_map.get(ref, 0) + flt(row.get(qty_field))
+
+	return qty_map

erpnext/controllers/tests/test_mapper.py

@@ -28,6 +28,96 @@ class TestMapper(unittest.TestCase):
 		src_items = item_list_1 + item_list_2 + item_list_3
 		self.assertEqual(set(d for d in src_items), set(d.item_code for d in updated_so.items))
 
+	def test_get_items_from_is_idempotent(self):
+		"""Selecting the same source document twice must not duplicate rows in the target.
+
+		"Get Items From" hands the in-progress document back to the mapper as `target_doc`.
+		Its rows are unsaved, so the mappers' pending-qty queries (submitted documents only)
+		cannot see them -- every mapper has to discount them explicitly.
+		"""
+		for label, make_source, method in self.idempotency_cases():
+			with self.subTest(label):
+				source = make_source()
+				target = frappe.get_attr(method)(source.name)
+				mapped_rows = len(target.items)
+				self.assertTrue(mapped_rows, f"{label}: nothing was mapped")
+
+				target = frappe.get_attr(method)(source.name, target)
+				self.assertEqual(len(target.items), mapped_rows, f"{label}: rows were duplicated")
+
+	def idempotency_cases(self):
+		"""(label, source factory, mapper method) for every "Get Items From" button.
+
+		Quotation -> Sales Invoice is absent: Sales Invoice Item keeps no reference to the
+		Quotation row, so there is nothing to deduplicate on.
+		"""
+		from erpnext.accounts.doctype.purchase_invoice.test_purchase_invoice import make_purchase_invoice
+		from erpnext.buying.doctype.purchase_order.test_purchase_order import create_purchase_order
+		from erpnext.buying.doctype.supplier_quotation.test_supplier_quotation import (
+			test_records as supplier_quotation_records,
+		)
+		from erpnext.selling.doctype.quotation.test_quotation import make_quotation
+		from erpnext.selling.doctype.sales_order.test_sales_order import make_sales_order
+		from erpnext.stock.doctype.delivery_note.test_delivery_note import create_delivery_note
+		from erpnext.stock.doctype.material_request.test_material_request import make_material_request
+		from erpnext.stock.doctype.purchase_receipt.test_purchase_receipt import make_purchase_receipt
+
+		def make_supplier_quotation():
+			return frappe.copy_doc(supplier_quotation_records[0]).submit()
+
+		return [
+			(
+				"Quotation -> Sales Order",
+				lambda: make_quotation(),
+				"erpnext.selling.doctype.quotation.quotation.make_sales_order",
+			),
+			(
+				"Sales Order -> Sales Invoice",
+				lambda: make_sales_order(),
+				"erpnext.selling.doctype.sales_order.sales_order.make_sales_invoice",
+			),
+			(
+				"Sales Order -> Delivery Note",
+				lambda: make_sales_order(),
+				"erpnext.selling.doctype.sales_order.sales_order.make_delivery_note",
+			),
+			(
+				"Delivery Note -> Sales Invoice",
+				lambda: create_delivery_note(),
+				"erpnext.stock.doctype.delivery_note.delivery_note.make_sales_invoice",
+			),
+			(
+				"Material Request -> Purchase Order",
+				lambda: make_material_request(),
+				"erpnext.stock.doctype.material_request.material_request.make_purchase_order",
+			),
+			(
+				"Supplier Quotation -> Purchase Order",
+				make_supplier_quotation,
+				"erpnext.buying.doctype.supplier_quotation.supplier_quotation.make_purchase_order",
+			),
+			(
+				"Purchase Order -> Purchase Receipt",
+				lambda: create_purchase_order(),
+				"erpnext.buying.doctype.purchase_order.purchase_order.make_purchase_receipt",
+			),
+			(
+				"Purchase Order -> Purchase Invoice",
+				lambda: create_purchase_order(),
+				"erpnext.buying.doctype.purchase_order.purchase_order.make_purchase_invoice",
+			),
+			(
+				"Purchase Receipt -> Purchase Invoice",
+				lambda: make_purchase_receipt(),
+				"erpnext.stock.doctype.purchase_receipt.purchase_receipt.make_purchase_invoice",
+			),
+			(
+				"Purchase Invoice -> Purchase Receipt",
+				lambda: make_purchase_invoice(),
+				"erpnext.accounts.doctype.purchase_invoice.purchase_invoice.make_purchase_receipt",
+			),
+		]
+
 	def make_quotation(self, item_list, customer):
 		qtn = frappe.get_doc(
 			{

erpnext/selling/doctype/quotation/quotation.py

@@ -9,6 +9,7 @@ from frappe import _
 from frappe.model.mapper import get_mapped_doc
 from frappe.utils import cint, flt, getdate, nowdate
 
+from erpnext.controllers.mapper import get_qty_already_mapped
 from erpnext.controllers.selling_controller import SellingController
 
 form_grid_templates = {"items": "templates/form_grid/item_grid.html"}
@@ -391,6 +392,9 @@ def _make_sales_order(source_name, target_doc=None, ignore_permissions=False, ar
 
 	customer = _make_customer(source_name, ignore_permissions)
 	ordered_items = get_ordered_items(source_name)
+	mapped_items = get_qty_already_mapped(target_doc, "quotation_item", "stock_qty")
+	for name, stock_qty in mapped_items.items():
+		ordered_items[name] = flt(ordered_items.get(name)) + stock_qty
 
 	selected_rows = [x.get("name") for x in frappe.flags.get("args", {}).get("selected_items", [])]
 
@@ -444,7 +448,10 @@ def _make_sales_order(source_name, target_doc=None, ignore_permissions=False, ar
 		2. If selections: Is Alternative Item/Has Alternative Item: Map if selected and adequate qty
 		3. If no selections: Simple row: Map if adequate qty
 		"""
-		if not ((item.stock_qty > ordered_items.get(item.name, 0.0)) or is_unit_price_row(item)):
+		if not (
+			(item.stock_qty > ordered_items.get(item.name, 0.0))
+			or (is_unit_price_row(item) and item.name not in mapped_items)
+		):
 			return False
 
 		if not selected_rows:

erpnext/selling/doctype/sales_order/sales_order.py

@@ -22,6 +22,7 @@ from erpnext.accounts.doctype.sales_invoice.sales_invoice import (
 	validate_inter_company_party,
 )
 from erpnext.accounts.party import CROSS_PARTY_FIELD_NO_MAP, get_party_account
+from erpnext.controllers.mapper import get_qty_already_mapped
 from erpnext.controllers.selling_controller import SellingController
 from erpnext.manufacturing.doctype.blanket_order.blanket_order import (
 	validate_against_blanket_order,
@@ -977,6 +978,8 @@ def make_delivery_note(source_name, target_doc=None, kwargs=None):
 	if kwargs.for_reserved_stock:
 		sre_details = get_sre_reserved_qty_details_for_voucher("Sales Order", source_name)
 
+	mapped_qty_by_item = get_qty_already_mapped(target_doc, "so_detail")
+
 	mapper = {
 		"Sales Order": {"doctype": "Delivery Note", "validation": {"docstatus": ["=", 1]}},
 		"Sales Taxes and Charges": {"doctype": "Sales Taxes and Charges", "reset_value": True},
@@ -1031,15 +1034,17 @@ def make_delivery_note(source_name, target_doc=None, kwargs=None):
 				return False
 
 		return (
-			(abs(doc.delivered_qty) < abs(doc.qty)) or is_unit_price_row(doc)
+			(abs(doc.delivered_qty) + abs(mapped_qty_by_item.get(doc.name, 0)) < abs(doc.qty))
+			or (is_unit_price_row(doc) and doc.name not in mapped_qty_by_item)
 		) and doc.delivered_by_supplier != 1
 
+	def remaining_qty(source):
+		return flt(source.qty) - flt(source.delivered_qty) - flt(mapped_qty_by_item.get(source.name, 0))
+
 	def update_item(source, target, source_parent):
-		target.base_amount = (flt(source.qty) - flt(source.delivered_qty)) * flt(source.base_rate)
+		target.base_amount = remaining_qty(source) * flt(source.base_rate)
-		target.amount = (flt(source.qty) - flt(source.delivered_qty)) * flt(source.rate)
+		target.amount = remaining_qty(source) * flt(source.rate)
-		target.qty = (
+		target.qty = flt(source.qty) if is_unit_price_row(source) else remaining_qty(source)
-			flt(source.qty) if is_unit_price_row(source) else flt(source.qty) - flt(source.delivered_qty)
-		)
 
 		item = get_item_defaults(target.item_code, source_parent.company)
 		item_group = get_item_group_defaults(target.item_code, source_parent.company)
@@ -1137,6 +1142,7 @@ def make_sales_invoice(
 	has_unit_price_items = frappe.db.get_value("Sales Order", source_name, "has_unit_price_items")
 	billed_qty_by_item = None
 	pending_qty_by_item = {}
+	mapped_qty_by_item = get_qty_already_mapped(target_doc, "so_detail")
 
 	def is_unit_price_row(source):
 		return has_unit_price_items and source.qty == 0
@@ -1165,6 +1171,7 @@ def make_sales_invoice(
 			if source.qty and source.billed_amt:
 				billable_qty -= get_billed_qty_by_item().get(source.name, 0)
 
+			billable_qty -= mapped_qty_by_item.get(source.name, 0)
 			pending_qty_by_item[source.name] = max(flt(billable_qty, source.precision("qty")), 0)
 
 		return pending_qty_by_item[source.name]
@@ -1244,7 +1251,7 @@ def make_sales_invoice(
 				"postprocess": update_item,
 				"condition": lambda doc: select_item(doc)
 				and (
-					True
+					doc.name not in mapped_qty_by_item
 					if is_unit_price_row(doc)
 					else (
 						doc.qty

erpnext/stock/doctype/delivery_note/delivery_note.py

@@ -18,6 +18,7 @@ from frappe.utils import cint, flt
 
 from erpnext.accounts.party import CROSS_PARTY_FIELD_NO_MAP, get_due_date
 from erpnext.controllers.accounts_controller import get_taxes_and_charges, merge_taxes
+from erpnext.controllers.mapper import get_qty_already_mapped
 from erpnext.controllers.selling_controller import SellingController
 from erpnext.stock.stock_ledger import validate_reserved_stock
 
@@ -936,6 +937,8 @@ def make_sales_invoice(
 	to_make_invoice_qty_map = {}
 	returned_qty_map = get_returned_qty_map(source_name)
 	invoiced_qty_map = get_invoiced_qty_map(source_name)
+	for ref, qty in get_qty_already_mapped(target_doc, "dn_detail").items():
+		invoiced_qty_map[ref] = invoiced_qty_map.get(ref, 0) + qty
 
 	def set_missing_values(source, target):
 		target.run_method("set_missing_values")
@@ -1008,7 +1011,7 @@ def make_sales_invoice(
 				"postprocess": update_item,
 				"filter": lambda d: get_pending_qty(d) <= 0
 				if not doc.get("is_return")
-				else get_pending_qty(d) > 0,
+				else get_pending_qty(d) >= 0,
 				"condition": select_item,
 			},
 			"Sales Taxes and Charges": {

erpnext/stock/doctype/delivery_note/test_delivery_note.py

@@ -1091,6 +1091,25 @@ class TestDeliveryNote(FrappeTestCase):
 		self.assertEqual(dn2.per_billed, 100)
 		self.assertEqual(dn2.status, "Completed")
 
+	def test_mapping_same_dn_twice_is_idempotent(self):
+		# "Get Items From > Delivery Note" passes the in-progress invoice back as target_doc.
+		# Selecting the same DN again must not append a second row for the same dn_detail.
+		dn = create_delivery_note(qty=5)
+
+		si = make_sales_invoice(dn.name)
+		self.assertEqual(len(si.items), 1)
+		self.assertEqual(si.items[0].qty, 5)
+
+		si = make_sales_invoice(dn.name, target_doc=si)
+		self.assertEqual(len(si.items), 1)
+		self.assertEqual(si.items[0].qty, 5)
+
+		# a partly reduced draft row still tops up to the delivered qty
+		si.items[0].qty = 2
+		si = make_sales_invoice(dn.name, target_doc=si)
+		self.assertEqual(len(si.items), 2)
+		self.assertEqual([d.qty for d in si.items], [2, 3])
+
 	@change_settings("Accounts Settings", {"delete_linked_ledger_entries": True})
 	def test_sales_invoice_qty_after_return(self):
 		from erpnext.stock.doctype.delivery_note.delivery_note import make_sales_return

erpnext/stock/doctype/material_request/material_request.py

@@ -17,6 +17,7 @@ from frappe.utils import cint, cstr, flt, get_link_to_form, getdate, new_line_se
 
 from erpnext.buying.utils import check_on_hold_or_closed_status, validate_for_items
 from erpnext.controllers.buying_controller import BuyingController
+from erpnext.controllers.mapper import get_qty_already_mapped
 from erpnext.manufacturing.doctype.work_order.work_order import get_item_details
 from erpnext.stock.doctype.item.item import get_item_defaults
 from erpnext.stock.get_item_details import get_price_list_rate_for
@@ -499,6 +500,8 @@ def make_purchase_order(source_name, target_doc=None, args=None):
 	if isinstance(args, str):
 		args = json.loads(args)
 
+	mapped_qty_by_item = get_qty_already_mapped(target_doc, "material_request_item", "stock_qty")
+
 	def postprocess(source, target_doc):
 		if frappe.flags.args and frappe.flags.args.default_supplier:
 			# items only for given default supplier
@@ -515,7 +518,7 @@ def make_purchase_order(source_name, target_doc=None, args=None):
 		filtered_items = args.get("filtered_children", [])
 		child_filter = d.name in filtered_items if filtered_items else True
 
-		qty = d.ordered_qty or d.received_qty
+		qty = (d.ordered_qty or d.received_qty) + flt(mapped_qty_by_item.get(d.name, 0))
 
 		return qty < d.stock_qty and child_filter
 

erpnext/stock/doctype/purchase_receipt/purchase_receipt.py

@@ -18,6 +18,7 @@ from erpnext.assets.doctype.asset.asset import get_asset_account, is_cwip_accoun
 from erpnext.buying.utils import check_on_hold_or_closed_status
 from erpnext.controllers.accounts_controller import merge_taxes
 from erpnext.controllers.buying_controller import BuyingController
+from erpnext.controllers.mapper import get_qty_already_mapped
 from erpnext.stock.doctype.delivery_note.delivery_note import make_inter_company_transaction
 from erpnext.stock.serial_batch_bundle import (
 	SerialBatchCreation,
@@ -1350,6 +1351,8 @@ def make_purchase_invoice(source_name, target_doc=None, args=None):
 	doc = frappe.get_doc("Purchase Receipt", source_name)
 	returned_qty_map = get_returned_qty_map(source_name)
 	invoiced_qty_map = get_invoiced_qty_map(source_name)
+	for ref, qty in get_qty_already_mapped(target_doc, "pr_detail").items():
+		invoiced_qty_map[ref] = invoiced_qty_map.get(ref, 0) + qty
 
 	def set_missing_values(source, target):
 		if len(target.get("items")) == 0:
@@ -1434,7 +1437,7 @@ def make_purchase_invoice(source_name, target_doc=None, args=None):
 				},
 				"postprocess": update_item,
 				"filter": lambda d: (
-					get_pending_qty(d)[0] <= 0 if not doc.get("is_return") else get_pending_qty(d)[0] > 0
+					get_pending_qty(d)[0] <= 0 if not doc.get("is_return") else get_pending_qty(d)[0] >= 0
 				),
 				"condition": select_item,
 			},