From c38b252289b46d6cc48667fe4b5fced7bbe3dd55 Mon Sep 17 00:00:00 2001 From: nishkagosalia Date: Thu, 19 Mar 2026 16:21:40 +0530 Subject: [PATCH 1/3] fix: Adding validation for operation time in BOM (cherry picked from commit 7f70e62c30d3939b975b40f956bbeccc81a16fa9) # Conflicts: # erpnext/manufacturing/doctype/job_card/test_job_card.py --- erpnext/manufacturing/doctype/bom/bom.py | 6 + erpnext/manufacturing/doctype/bom/test_bom.py | 9 + .../doctype/job_card/test_job_card.py | 193 ++++++++++++++++++ 3 files changed, 208 insertions(+) diff --git a/erpnext/manufacturing/doctype/bom/bom.py b/erpnext/manufacturing/doctype/bom/bom.py index ceb2e9d0ffd..efcbc5fa9b9 100644 --- a/erpnext/manufacturing/doctype/bom/bom.py +++ b/erpnext/manufacturing/doctype/bom/bom.py @@ -1204,6 +1204,12 @@ class BOM(WebsiteGenerator): "Row {0}: Workstation or Workstation Type is mandatory for an operation {1}" ).format(d.idx, d.operation) ) + if not d.time_in_mins or d.time_in_mins <= 0: + frappe.throw( + _("Row {0}: Operation time should be greater than 0 for operation {1}").format( + d.idx, d.operation + ) + ) def get_tree_representation(self) -> BOMTree: """Get a complete tree representation preserving order of child items.""" diff --git a/erpnext/manufacturing/doctype/bom/test_bom.py b/erpnext/manufacturing/doctype/bom/test_bom.py index 69b55e6d1d8..68a29d7da4e 100644 --- a/erpnext/manufacturing/doctype/bom/test_bom.py +++ b/erpnext/manufacturing/doctype/bom/test_bom.py @@ -133,6 +133,15 @@ class TestBOM(ERPNextTestSuite): self.assertAlmostEqual(bom.base_raw_material_cost, base_raw_material_cost) self.assertAlmostEqual(bom.base_total_cost, base_raw_material_cost + base_op_cost) + @timeout + def test_bom_no_operation_time_validation(self): + bom = frappe.copy_doc(self.globalTestRecords["BOM"][2]) + bom.docstatus = 0 + for op_row in bom.operations: + op_row.time_in_mins = 0 + + self.assertRaises(frappe.ValidationError, bom.save) + @timeout def test_bom_cost_with_batch_size(self): bom = frappe.copy_doc(self.globalTestRecords["BOM"][2]) diff --git a/erpnext/manufacturing/doctype/job_card/test_job_card.py b/erpnext/manufacturing/doctype/job_card/test_job_card.py index cb05acb9433..8d5c792f483 100644 --- a/erpnext/manufacturing/doctype/job_card/test_job_card.py +++ b/erpnext/manufacturing/doctype/job_card/test_job_card.py @@ -96,6 +96,7 @@ class TestJobCard(ERPNextTestSuite): "workstation": "_Test Workstation 1", "bom_no": cut_bom, "skip_material_transfer": 1, + "time_in_mins": 60, }, ) final_bom.append( @@ -105,6 +106,7 @@ class TestJobCard(ERPNextTestSuite): "workstation": "_Test Workstation 1", "bom_no": stitch_bom, "skip_material_transfer": 1, + "time_in_mins": 60, }, ) final_bom.append( @@ -115,6 +117,7 @@ class TestJobCard(ERPNextTestSuite): "is_final_finished_good": 1, "bom_no": final_bom.name, "skip_material_transfer": 1, + "time_in_mins": 60, }, ) final_bom.append("items", {"item_code": stitch_fg.name, "qty": 1, "operation_row_id": 3}) @@ -879,6 +882,196 @@ class TestJobCard(ERPNextTestSuite): s = frappe.get_doc(make_stock_entry_for_wo(wo_doc.name, "Manufacture", 6)) self.assertEqual(s.additional_costs[0].amount, 8) +<<<<<<< HEAD +======= + def test_co_by_product_for_sfg_flow(self): + from erpnext.manufacturing.doctype.operation.test_operation import make_operation + + frappe.db.set_value("UOM", "Nos", "must_be_whole_number", 0) + + def create_bom(raw_material, finished_good, scrap_item, submit=True): + bom = frappe.new_doc("BOM") + bom.company = "_Test Company" + bom.item = finished_good + bom.quantity = 1 + bom.append("items", {"item_code": raw_material, "qty": 1}) + bom.append( + "secondary_items", + { + "item_code": scrap_item, + "qty": 1, + "process_loss_per": 10, + "cost_allocation_per": 5, + "type": "Scrap", + }, + ) + if submit: + bom.insert() + bom.submit() + + return bom + + rm1 = create_item("RM 1") + scrap1 = create_item("Scrap 1") + sfg = create_item("SFG 1") + sfg_bom = create_bom(rm1.name, sfg.name, scrap1.name) + + rm2 = create_item("RM 2") + fg1 = create_item("FG 1") + scrap2 = create_item("Scrap 2") + scrap_extra = create_item("Scrap Extra") + fg_bom = create_bom(rm2.name, fg1.name, scrap2.name, submit=False) + fg_bom.with_operations = 1 + fg_bom.track_semi_finished_goods = 1 + + operation1 = { + "operation": "Test Operation A", + "workstation": "_Test Workstation A", + "finished_good": sfg.name, + "bom_no": sfg_bom.name, + "finished_good_qty": 1, + "sequence_id": 1, + "time_in_mins": 60, + } + operation2 = { + "operation": "Test Operation B", + "workstation": "_Test Workstation A", + "finished_good": fg1.name, + "bom_no": fg_bom.name, + "finished_good_qty": 1, + "is_final_finished_good": 1, + "sequence_id": 2, + "time_in_mins": 60, + } + + make_workstation(operation1) + make_operation(operation1) + make_operation(operation2) + + fg_bom.append("operations", operation1) + fg_bom.append("operations", operation2) + fg_bom.append("items", {"item_code": sfg.name, "qty": 1, "uom": "Nos", "operation_row_id": 2}) + fg_bom.insert() + fg_bom.save() + fg_bom.submit() + + work_order = make_wo_order_test_record( + item=fg1.name, + qty=10, + source_warehouse="Stores - _TC", + fg_warehouse="Finished Goods - _TC", + bom_no=fg_bom.name, + skip_transfer=1, + do_not_save=True, + ) + + work_order.operations[0].time_in_mins = 60 + work_order.operations[1].time_in_mins = 60 + work_order.save() + work_order.submit() + + job_card = frappe.get_doc( + "Job Card", + frappe.db.get_value( + "Job Card", {"work_order": work_order.name, "operation": "Test Operation A"}, "name" + ), + ) + job_card.append( + "time_logs", + { + "from_time": "2009-01-01 12:06:25", + "to_time": "2009-01-01 12:37:25", + "completed_qty": job_card.for_quantity, + }, + ) + job_card.append( + "secondary_items", {"item_code": scrap_extra.name, "stock_qty": 5, "type": "Co-Product"} + ) + job_card.submit() + + for row in sfg_bom.items: + make_stock_entry( + item_code=row.item_code, + target="Stores - _TC", + qty=10, + basic_rate=100, + ) + + manufacturing_entry = frappe.get_doc(job_card.make_stock_entry_for_semi_fg_item()) + manufacturing_entry.submit() + + self.assertEqual(manufacturing_entry.items[2].item_code, scrap1.name) + self.assertEqual(manufacturing_entry.items[2].qty, 9) + self.assertEqual(flt(manufacturing_entry.items[2].basic_rate, 3), 5.556) + self.assertEqual(manufacturing_entry.items[3].item_code, scrap_extra.name) + self.assertEqual(manufacturing_entry.items[3].type, "Co-Product") + self.assertEqual(manufacturing_entry.items[3].qty, 5) + self.assertEqual(manufacturing_entry.items[3].basic_rate, 0) + + job_card = frappe.get_doc( + "Job Card", + frappe.db.get_value( + "Job Card", {"work_order": work_order.name, "operation": "Test Operation B"}, "name" + ), + ) + job_card.append( + "time_logs", + { + "from_time": "2009-02-01 12:06:25", + "to_time": "2009-02-01 12:37:25", + "completed_qty": job_card.for_quantity, + }, + ) + job_card.submit() + + for row in fg_bom.items: + make_stock_entry( + item_code=row.item_code, + target="Stores - _TC", + qty=10, + basic_rate=100, + ) + + manufacturing_entry = frappe.get_doc(job_card.make_stock_entry_for_semi_fg_item()) + manufacturing_entry.submit() + + self.assertEqual(manufacturing_entry.items[2].item_code, scrap2.name) + self.assertEqual(manufacturing_entry.items[2].qty, 9) + self.assertEqual(flt(manufacturing_entry.items[2].basic_rate, 3), 5.556) + + def test_secondary_items_without_sfg(self): + for row in frappe.get_doc("BOM", self.work_order.bom_no).items: + make_stock_entry( + item_code=row.item_code, + target="_Test Warehouse - _TC", + qty=10, + basic_rate=100, + ) + + job_card = frappe.get_last_doc("Job Card", {"work_order": self.work_order.name}) + job_card.append("secondary_items", {"item_code": "_Test Item", "stock_qty": 2, "type": "Scrap"}) + job_card.append( + "time_logs", + { + "from_time": "2009-01-01 12:06:25", + "to_time": "2009-01-01 12:37:25", + "completed_qty": job_card.for_quantity, + }, + ) + job_card.save() + job_card.submit() + + from erpnext.manufacturing.doctype.work_order.work_order import ( + make_stock_entry as make_stock_entry_for_wo, + ) + + s = frappe.get_doc(make_stock_entry_for_wo(self.work_order.name, "Manufacture")) + s.submit() + + self.assertEqual(s.items[3].item_code, "_Test Item") + self.assertEqual(s.items[3].transfer_qty, 2) + +>>>>>>> 7f70e62c30 (fix: Adding validation for operation time in BOM) def create_bom_with_multiple_operations(): "Create a BOM with multiple operations and Material Transfer against Job Card" From f1c93d4b98181828e2825b2ef82d733e4f99ad25 Mon Sep 17 00:00:00 2001 From: nishkagosalia Date: Thu, 19 Mar 2026 18:35:07 +0530 Subject: [PATCH 2/3] fix: merge conflict --- .../doctype/job_card/test_job_card.py | 159 ------------------ 1 file changed, 159 deletions(-) diff --git a/erpnext/manufacturing/doctype/job_card/test_job_card.py b/erpnext/manufacturing/doctype/job_card/test_job_card.py index 8d5c792f483..5eca4c3a6ea 100644 --- a/erpnext/manufacturing/doctype/job_card/test_job_card.py +++ b/erpnext/manufacturing/doctype/job_card/test_job_card.py @@ -882,164 +882,6 @@ class TestJobCard(ERPNextTestSuite): s = frappe.get_doc(make_stock_entry_for_wo(wo_doc.name, "Manufacture", 6)) self.assertEqual(s.additional_costs[0].amount, 8) -<<<<<<< HEAD -======= - def test_co_by_product_for_sfg_flow(self): - from erpnext.manufacturing.doctype.operation.test_operation import make_operation - - frappe.db.set_value("UOM", "Nos", "must_be_whole_number", 0) - - def create_bom(raw_material, finished_good, scrap_item, submit=True): - bom = frappe.new_doc("BOM") - bom.company = "_Test Company" - bom.item = finished_good - bom.quantity = 1 - bom.append("items", {"item_code": raw_material, "qty": 1}) - bom.append( - "secondary_items", - { - "item_code": scrap_item, - "qty": 1, - "process_loss_per": 10, - "cost_allocation_per": 5, - "type": "Scrap", - }, - ) - if submit: - bom.insert() - bom.submit() - - return bom - - rm1 = create_item("RM 1") - scrap1 = create_item("Scrap 1") - sfg = create_item("SFG 1") - sfg_bom = create_bom(rm1.name, sfg.name, scrap1.name) - - rm2 = create_item("RM 2") - fg1 = create_item("FG 1") - scrap2 = create_item("Scrap 2") - scrap_extra = create_item("Scrap Extra") - fg_bom = create_bom(rm2.name, fg1.name, scrap2.name, submit=False) - fg_bom.with_operations = 1 - fg_bom.track_semi_finished_goods = 1 - - operation1 = { - "operation": "Test Operation A", - "workstation": "_Test Workstation A", - "finished_good": sfg.name, - "bom_no": sfg_bom.name, - "finished_good_qty": 1, - "sequence_id": 1, - "time_in_mins": 60, - } - operation2 = { - "operation": "Test Operation B", - "workstation": "_Test Workstation A", - "finished_good": fg1.name, - "bom_no": fg_bom.name, - "finished_good_qty": 1, - "is_final_finished_good": 1, - "sequence_id": 2, - "time_in_mins": 60, - } - - make_workstation(operation1) - make_operation(operation1) - make_operation(operation2) - - fg_bom.append("operations", operation1) - fg_bom.append("operations", operation2) - fg_bom.append("items", {"item_code": sfg.name, "qty": 1, "uom": "Nos", "operation_row_id": 2}) - fg_bom.insert() - fg_bom.save() - fg_bom.submit() - - work_order = make_wo_order_test_record( - item=fg1.name, - qty=10, - source_warehouse="Stores - _TC", - fg_warehouse="Finished Goods - _TC", - bom_no=fg_bom.name, - skip_transfer=1, - do_not_save=True, - ) - - work_order.operations[0].time_in_mins = 60 - work_order.operations[1].time_in_mins = 60 - work_order.save() - work_order.submit() - - job_card = frappe.get_doc( - "Job Card", - frappe.db.get_value( - "Job Card", {"work_order": work_order.name, "operation": "Test Operation A"}, "name" - ), - ) - job_card.append( - "time_logs", - { - "from_time": "2009-01-01 12:06:25", - "to_time": "2009-01-01 12:37:25", - "completed_qty": job_card.for_quantity, - }, - ) - job_card.append( - "secondary_items", {"item_code": scrap_extra.name, "stock_qty": 5, "type": "Co-Product"} - ) - job_card.submit() - - for row in sfg_bom.items: - make_stock_entry( - item_code=row.item_code, - target="Stores - _TC", - qty=10, - basic_rate=100, - ) - - manufacturing_entry = frappe.get_doc(job_card.make_stock_entry_for_semi_fg_item()) - manufacturing_entry.submit() - - self.assertEqual(manufacturing_entry.items[2].item_code, scrap1.name) - self.assertEqual(manufacturing_entry.items[2].qty, 9) - self.assertEqual(flt(manufacturing_entry.items[2].basic_rate, 3), 5.556) - self.assertEqual(manufacturing_entry.items[3].item_code, scrap_extra.name) - self.assertEqual(manufacturing_entry.items[3].type, "Co-Product") - self.assertEqual(manufacturing_entry.items[3].qty, 5) - self.assertEqual(manufacturing_entry.items[3].basic_rate, 0) - - job_card = frappe.get_doc( - "Job Card", - frappe.db.get_value( - "Job Card", {"work_order": work_order.name, "operation": "Test Operation B"}, "name" - ), - ) - job_card.append( - "time_logs", - { - "from_time": "2009-02-01 12:06:25", - "to_time": "2009-02-01 12:37:25", - "completed_qty": job_card.for_quantity, - }, - ) - job_card.submit() - - for row in fg_bom.items: - make_stock_entry( - item_code=row.item_code, - target="Stores - _TC", - qty=10, - basic_rate=100, - ) - - manufacturing_entry = frappe.get_doc(job_card.make_stock_entry_for_semi_fg_item()) - manufacturing_entry.submit() - - self.assertEqual(manufacturing_entry.items[2].item_code, scrap2.name) - self.assertEqual(manufacturing_entry.items[2].qty, 9) - self.assertEqual(flt(manufacturing_entry.items[2].basic_rate, 3), 5.556) - - def test_secondary_items_without_sfg(self): for row in frappe.get_doc("BOM", self.work_order.bom_no).items: make_stock_entry( item_code=row.item_code, @@ -1071,7 +913,6 @@ class TestJobCard(ERPNextTestSuite): self.assertEqual(s.items[3].item_code, "_Test Item") self.assertEqual(s.items[3].transfer_qty, 2) ->>>>>>> 7f70e62c30 (fix: Adding validation for operation time in BOM) def create_bom_with_multiple_operations(): "Create a BOM with multiple operations and Material Transfer against Job Card" From aac60b4c134b01224b606d19a85ef81602fc2b79 Mon Sep 17 00:00:00 2001 From: nishkagosalia Date: Thu, 19 Mar 2026 22:05:58 +0530 Subject: [PATCH 3/3] fix: test case --- .../doctype/job_card/test_job_card.py | 31 ------------------- 1 file changed, 31 deletions(-) diff --git a/erpnext/manufacturing/doctype/job_card/test_job_card.py b/erpnext/manufacturing/doctype/job_card/test_job_card.py index 5eca4c3a6ea..556d3911eb3 100644 --- a/erpnext/manufacturing/doctype/job_card/test_job_card.py +++ b/erpnext/manufacturing/doctype/job_card/test_job_card.py @@ -882,37 +882,6 @@ class TestJobCard(ERPNextTestSuite): s = frappe.get_doc(make_stock_entry_for_wo(wo_doc.name, "Manufacture", 6)) self.assertEqual(s.additional_costs[0].amount, 8) - for row in frappe.get_doc("BOM", self.work_order.bom_no).items: - make_stock_entry( - item_code=row.item_code, - target="_Test Warehouse - _TC", - qty=10, - basic_rate=100, - ) - - job_card = frappe.get_last_doc("Job Card", {"work_order": self.work_order.name}) - job_card.append("secondary_items", {"item_code": "_Test Item", "stock_qty": 2, "type": "Scrap"}) - job_card.append( - "time_logs", - { - "from_time": "2009-01-01 12:06:25", - "to_time": "2009-01-01 12:37:25", - "completed_qty": job_card.for_quantity, - }, - ) - job_card.save() - job_card.submit() - - from erpnext.manufacturing.doctype.work_order.work_order import ( - make_stock_entry as make_stock_entry_for_wo, - ) - - s = frappe.get_doc(make_stock_entry_for_wo(self.work_order.name, "Manufacture")) - s.submit() - - self.assertEqual(s.items[3].item_code, "_Test Item") - self.assertEqual(s.items[3].transfer_qty, 2) - def create_bom_with_multiple_operations(): "Create a BOM with multiple operations and Material Transfer against Job Card"