From bbb7384ea57cdf14c5a79840c605ba3145397410 Mon Sep 17 00:00:00 2001 From: Nabin Hait Date: Mon, 22 Jun 2026 12:48:14 +0530 Subject: [PATCH 1/3] test: add Subcontracting Order validation and process-loss coverage Covers previously untested Subcontracting Order paths: - a Subcontracting Order requires a subcontracting Purchase Order - service items must be non-stock items - a supplied item's reserve warehouse must differ from the supplier warehouse - the Subcontracting Receipt mapper applies BOM process-loss to the received qty --- .../test_subcontracting_order.py | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/erpnext/subcontracting/doctype/subcontracting_order/test_subcontracting_order.py b/erpnext/subcontracting/doctype/subcontracting_order/test_subcontracting_order.py index 4e60d37d356..478476e628d 100644 --- a/erpnext/subcontracting/doctype/subcontracting_order/test_subcontracting_order.py +++ b/erpnext/subcontracting/doctype/subcontracting_order/test_subcontracting_order.py @@ -112,6 +112,32 @@ 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() + 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() + 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_make_rm_stock_entry(self): sco = get_subcontracting_order() rm_items = get_rm_items(sco.supplied_items) From d68f7ea9d148c0d2c217d7c2ac129c066e708004 Mon Sep 17 00:00:00 2001 From: Nabin Hait Date: Mon, 22 Jun 2026 12:56:26 +0530 Subject: [PATCH 2/3] fix: match Subcontracting Order service cost by purchase order item calculate_service_costs paired the service_items and items child tables by list index, which breaks if the tables are not index-aligned (e.g. populate_items_table skips a service item with zero available qty), assigning the wrong service cost or raising IndexError. Match by purchase_order_item instead, and guard against division by zero qty. Adds a regression test asserting service costs follow purchase_order_item regardless of table ordering. --- .../subcontracting_order.py | 11 +++++-- .../test_subcontracting_order.py | 32 +++++++++++++++++++ 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/erpnext/subcontracting/doctype/subcontracting_order/subcontracting_order.py b/erpnext/subcontracting/doctype/subcontracting_order/subcontracting_order.py index 9b092ad5c0a..617791cda48 100644 --- a/erpnext/subcontracting/doctype/subcontracting_order/subcontracting_order.py +++ b/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"): diff --git a/erpnext/subcontracting/doctype/subcontracting_order/test_subcontracting_order.py b/erpnext/subcontracting/doctype/subcontracting_order/test_subcontracting_order.py index 478476e628d..dfae830a97b 100644 --- a/erpnext/subcontracting/doctype/subcontracting_order/test_subcontracting_order.py +++ b/erpnext/subcontracting/doctype/subcontracting_order/test_subcontracting_order.py @@ -138,6 +138,38 @@ class TestSubcontractingOrder(ERPNextTestSuite): 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) From 8cb94ebedb8dc8ad87f94909c39e312eba2511c6 Mon Sep 17 00:00:00 2001 From: Nabin Hait Date: Mon, 22 Jun 2026 14:25:34 +0530 Subject: [PATCH 3/3] test: avoid needless submit in SCO validation tests Use do_not_submit=1 for the service-item and reserve-warehouse validation tests; they only exercise in-memory validation methods, so submitting the Subcontracting Order is unnecessary. --- .../doctype/subcontracting_order/test_subcontracting_order.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/erpnext/subcontracting/doctype/subcontracting_order/test_subcontracting_order.py b/erpnext/subcontracting/doctype/subcontracting_order/test_subcontracting_order.py index dfae830a97b..0186549bb4d 100644 --- a/erpnext/subcontracting/doctype/subcontracting_order/test_subcontracting_order.py +++ b/erpnext/subcontracting/doctype/subcontracting_order/test_subcontracting_order.py @@ -118,12 +118,12 @@ class TestSubcontractingOrder(ERPNextTestSuite): self.assertRaises(frappe.ValidationError, sco.validate_purchase_order_for_subcontracting) def test_service_item_must_be_non_stock(self): - sco = get_subcontracting_order() + 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() + 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)