fix: item selection writes to the child table that fired it (#59068)

erpnext/controllers/tests/test_reactivity.py

@@ -104,3 +104,96 @@ class TestReactivity(ERPNextTestSuite):
 		self.assertEqual(sales_invoice.items[0].uom, "Kg")
 		self.assertEqual(sales_invoice.items[0].conversion_factor, 1)
 		self.assertEqual(sales_invoice.items[0].stock_qty, sales_invoice.items[0].qty)
+
+	def add_optional_items_table(self):
+		from frappe.custom.doctype.custom_field.custom_field import create_custom_fields
+
+		create_custom_fields(
+			{
+				"Sales Order": [
+					{
+						"fieldname": "optional_items",
+						"label": "Optional Items",
+						"fieldtype": "Table",
+						"options": "Sales Order Item",
+						"insert_after": "items",
+					}
+				]
+			}
+		)
+		self.addCleanup(frappe.clear_cache, doctype="Sales Order")
+		self.addCleanup(frappe.delete_doc, "Custom Field", "Sales Order-optional_items")
+
+	def make_sales_order_with_optional_items(self, item_code, optional_item_codes):
+		from erpnext.selling.doctype.sales_order.test_sales_order import make_sales_order
+
+		self.add_optional_items_table()
+		sales_order = make_sales_order(item_code=item_code, uom="Kg", rate=500, do_not_save=True)
+		for optional_item_code in optional_item_codes:
+			sales_order.append("optional_items", {"item_code": optional_item_code, "qty": 1})
+
+		return sales_order
+
+	def test_item_selection_updates_the_row_in_its_own_child_table(self):
+		from erpnext.stock.doctype.item.test_item import make_item
+
+		item = make_item(properties={"is_stock_item": 0, "stock_uom": "Kg"})
+		optional_item = make_item(properties={"is_stock_item": 0, "stock_uom": "Nos"})
+		sales_order = self.make_sales_order_with_optional_items(item.name, [item.name, optional_item.name])
+
+		standard_row = sales_order.items[0]
+		row_state = (standard_row.item_code, standard_row.uom, standard_row.rate)
+		edited_row = sales_order.optional_items[1]
+
+		sales_order.process_item_selection(
+			edited_row.idx, reset_item_details=True, parentfield="optional_items"
+		)
+
+		self.assertEqual(edited_row.item_name, optional_item.item_name)
+		self.assertEqual(edited_row.uom, "Nos")
+		self.assertEqual((standard_row.item_code, standard_row.uom, standard_row.rate), row_state)
+
+	def test_item_selection_ignores_a_row_that_is_gone(self):
+		from erpnext.stock.doctype.item.test_item import make_item
+
+		item = make_item(properties={"is_stock_item": 0, "stock_uom": "Kg"})
+		sales_order = self.make_sales_order_with_optional_items(item.name, [])
+
+		sales_order.process_item_selection(len(sales_order.items) + 1)
+
+		self.assertEqual(len(sales_order.items), 1)
+
+	def test_item_selection_rejects_a_field_that_is_not_a_child_table(self):
+		from erpnext.stock.doctype.item.test_item import make_item
+
+		item = make_item(properties={"is_stock_item": 0, "stock_uom": "Kg"})
+		sales_order = self.make_sales_order_with_optional_items(item.name, [])
+
+		self.assertRaises(
+			frappe.ValidationError, sales_order.process_item_selection, 1, parentfield="company"
+		)
+
+	def test_free_item_is_added_to_the_table_that_earned_it(self):
+		from erpnext.accounts.doctype.pricing_rule.test_pricing_rule import make_pricing_rule
+		from erpnext.stock.doctype.item.test_item import make_item
+
+		item = make_item(properties={"is_stock_item": 0, "stock_uom": "Kg"})
+		optional_item = make_item(properties={"is_stock_item": 0, "stock_uom": "Kg"})
+		free_item = make_item(properties={"is_stock_item": 0, "stock_uom": "Kg"})
+		make_pricing_rule(
+			title=f"_Test Free Item Rule {optional_item.name}",
+			selling=1,
+			item_code=optional_item.name,
+			price_or_product_discount="Product",
+			free_item=free_item.name,
+			free_qty=1,
+		)
+		sales_order = self.make_sales_order_with_optional_items(item.name, [optional_item.name])
+
+		sales_order.process_item_selection(sales_order.optional_items[0].idx, parentfield="optional_items")
+
+		self.assertEqual([row.item_code for row in sales_order.items], [item.name])
+		self.assertEqual(
+			[row.item_code for row in sales_order.optional_items],
+			[optional_item.name, free_item.name],
+		)

erpnext/public/js/controllers/transaction.js

@@ -785,6 +785,7 @@ erpnext.TransactionController = class TransactionController extends erpnext.taxe
 				args: {
 					item_idx: item.idx,
 					reset_item_details: true,
+					parentfield: item.parentfield,
 				},
 				callback: function (r) {
 					if (!r.exc) {

erpnext/utilities/transaction_base.py

@@ -350,11 +350,13 @@ class TransactionBase(StatusUpdater):
 		)
 
 	@frappe.whitelist()
-	def process_item_selection(self, item_idx: int, reset_item_details: bool = False):
+	def process_item_selection(
+		self, item_idx: int, reset_item_details: bool = False, parentfield: str = "items"
+	):
 		# Server side 'item' doc. Update this to reflect in UI
-		item_obj = self.get("items", {"idx": item_idx})[0]
+		item_obj = self.get_selected_item_row(parentfield, item_idx)
 
-		if not item_obj.item_code:
+		if not item_obj or not item_obj.item_code:
 			return
 
 		if cint(reset_item_details):
@@ -384,6 +386,13 @@ class TransactionBase(StatusUpdater):
 		self.conversion_factor(item_obj, item_details)
 		self.calculate_taxes_and_totals()
 
+	def get_selected_item_row(self, parentfield: str, item_idx: int):
+		if not self.get_table_field_doctype(parentfield):
+			frappe.throw(_("{0} is not a child table of {1}").format(parentfield, self.doctype))
+
+		rows = self.get(parentfield, {"idx": item_idx})
+		return rows[0] if rows else None
+
 	def set_fetched_values(self, item_obj: object, item_details: dict) -> None:
 		for k, v in item_details.items():
 			if hasattr(item_obj, k):
@@ -457,31 +466,35 @@ class TransactionBase(StatusUpdater):
 			)
 
 	def copy_from_first_row(self, row, fields):
-		if self.items and row:
+		sibling_rows = self.get(row.parentfield) if row else None
+		if sibling_rows:
 			fields.extend([x.get("fieldname") for x in get_dimensions(True)[0]])
-			first_row = self.items[0]
+			first_row = sibling_rows[0]
 			[setattr(row, k, first_row.get(k)) for k in fields if hasattr(first_row, k)]
 
 	def add_free_item(self, item_obj: object, item_details: dict) -> None:
 		free_items = item_details.get("free_item_data")
-		if free_items and len(free_items):
-			existing_free_items = [x for x in self.items if x.is_free_item]
-			for free_item in free_items:
-				_matches = [
-					x
-					for x in existing_free_items
-					if x.item_code == free_item.get("item_code")
-					and x.pricing_rules == free_item.get("pricing_rules")
-				]
-				if _matches:
-					row_to_modify = _matches[0]
-				else:
-					row_to_modify = self.append("items")
+		if not free_items:
+			return
 
-				for k, _v in free_item.items():
-					setattr(row_to_modify, k, free_item.get(k))
+		parentfield = item_obj.parentfield
+		existing_free_items = [x for x in self.get(parentfield) if x.is_free_item]
+		for free_item in free_items:
+			_matches = [
+				x
+				for x in existing_free_items
+				if x.item_code == free_item.get("item_code")
+				and x.pricing_rules == free_item.get("pricing_rules")
+			]
+			if _matches:
+				row_to_modify = _matches[0]
+			else:
+				row_to_modify = self.append(parentfield)
 
-				self.copy_from_first_row(row_to_modify, ["expense_account", "income_account"])
+			for k, _v in free_item.items():
+				setattr(row_to_modify, k, free_item.get(k))
+
+			self.copy_from_first_row(row_to_modify, ["expense_account", "income_account"])
 
 	def conversion_factor(self, item_obj: object, item_details: dict) -> None:
 		if frappe.get_meta(item_obj.doctype).has_field("stock_qty"):