Merge pull request #56294 from frappe/chore/subcontracting-test-coverage

test: Subcontracting coverage; fix service-cost mismatch by PO item

erpnext/subcontracting/doctype/subcontracting_order/subcontracting_order.py

@@ -182,8 +182,15 @@ class SubcontractingOrder(SubcontractingController):
 		self.calculate_items_qty_and_amount()
 
 	def calculate_service_costs(self):
-		for idx, item in enumerate(self.get("service_items")):
-			self.items[idx].service_cost_per_qty = item.amount / self.items[idx].qty
+		# Match by purchase_order_item rather than list position: the service_items and items
+		# tables are not guaranteed to stay index-aligned (e.g. a skipped zero-qty service item).
+		service_amount_by_po_item = {
+			service_item.purchase_order_item: service_item.amount
+			for service_item in self.get("service_items")
+		}
+		for item in self.items:
+			service_amount = flt(service_amount_by_po_item.get(item.purchase_order_item))
+			item.service_cost_per_qty = service_amount / item.qty if item.qty else 0
 
 	def calculate_supplied_items_qty_and_amount(self):
 		for item in self.get("items"):

erpnext/subcontracting/doctype/subcontracting_order/test_subcontracting_order.py

@@ -112,6 +112,64 @@ class TestSubcontractingOrder(ERPNextTestSuite):
 		sco.load_from_db()
 		self.assertEqual(sco.status, "Partially Received")
 
+	def test_sco_requires_a_subcontracting_purchase_order(self):
+		sco = get_subcontracting_order(do_not_save=1)
+		sco.purchase_order = None
+		self.assertRaises(frappe.ValidationError, sco.validate_purchase_order_for_subcontracting)
+
+	def test_service_item_must_be_non_stock(self):
+		sco = get_subcontracting_order(do_not_submit=1)
+		sco.service_items[0].item_code = "_Test Item"  # a stock item
+		self.assertRaises(frappe.ValidationError, sco.validate_service_items)
+
+	def test_reserve_warehouse_must_differ_from_supplier_warehouse(self):
+		sco = get_subcontracting_order(do_not_submit=1)
+		sco.supplied_items[0].reserve_warehouse = sco.supplier_warehouse
+		self.assertRaises(frappe.ValidationError, sco.validate_supplied_items)
+
+	def test_subcontracting_receipt_applies_bom_process_loss(self):
+		sco = get_subcontracting_order()
+		frappe.db.set_value("BOM", sco.items[0].bom, "process_loss_percentage", 10)
+
+		scr = make_subcontracting_receipt(sco.name)
+
+		# 10% of the ordered 10 qty is lost in processing
+		self.assertEqual(scr.items[0].received_qty, 10)
+		self.assertEqual(scr.items[0].process_loss_qty, 1)
+		self.assertEqual(scr.items[0].qty, 9)
+
+	def test_service_cost_is_matched_by_purchase_order_item(self):
+		service_items = [
+			{
+				"warehouse": "_Test Warehouse - _TC",
+				"item_code": "Subcontracted Service Item 7",
+				"qty": 10,
+				"rate": 100,
+				"fg_item": "Subcontracted Item SA7",
+				"fg_item_qty": 10,
+			},
+			{
+				"warehouse": "_Test Warehouse - _TC",
+				"item_code": "Subcontracted Service Item 1",
+				"qty": 10,
+				"rate": 200,
+				"fg_item": "Subcontracted Item SA1",
+				"fg_item_qty": 10,
+			},
+		]
+		sco = get_subcontracting_order(service_items=service_items)
+		expected = {item.purchase_order_item: item.service_cost_per_qty for item in sco.items}
+
+		# The two finished goods have distinct service costs, so a position-based pairing would swap them
+		self.assertEqual(len(set(expected.values())), 2)
+
+		# Service costs must follow purchase_order_item, not list position
+		sco.service_items.reverse()
+		sco.calculate_service_costs()
+
+		for item in sco.items:
+			self.assertEqual(item.service_cost_per_qty, expected[item.purchase_order_item])
+
 	def test_make_rm_stock_entry(self):
 		sco = get_subcontracting_order()
 		rm_items = get_rm_items(sco.supplied_items)