From 6d72aa377f98f07f28312074b453a97e482998e8 Mon Sep 17 00:00:00 2001 From: marination Date: Thu, 24 Feb 2022 15:20:18 +0530 Subject: [PATCH 1/5] feat: Combine Sub-assembly items in Production Plan - Combine subassembly rows if same Item, Warehouse, Inhouse/Outhouse Manu.g, BOM No. --- erpnext/manufacturing/doctype/bom/bom.py | 2 +- .../production_plan/production_plan.js | 9 +++- .../production_plan/production_plan.json | 11 ++++- .../production_plan/production_plan.py | 49 ++++++++++++++++--- 4 files changed, 61 insertions(+), 10 deletions(-) diff --git a/erpnext/manufacturing/doctype/bom/bom.py b/erpnext/manufacturing/doctype/bom/bom.py index d640f3fda7..37d2b9ff97 100644 --- a/erpnext/manufacturing/doctype/bom/bom.py +++ b/erpnext/manufacturing/doctype/bom/bom.py @@ -918,7 +918,7 @@ def validate_bom_no(item, bom_no): frappe.throw(_("BOM {0} does not belong to Item {1}").format(bom_no, item)) @frappe.whitelist() -def get_children(doctype, parent=None, is_root=False, **filters): +def get_children(parent=None, is_root=False, **filters): if not parent or parent=="BOM": frappe.msgprint(_('Please select a BOM')) return diff --git a/erpnext/manufacturing/doctype/production_plan/production_plan.js b/erpnext/manufacturing/doctype/production_plan/production_plan.js index 0babf875e7..c78ff51a7c 100644 --- a/erpnext/manufacturing/doctype/production_plan/production_plan.js +++ b/erpnext/manufacturing/doctype/production_plan/production_plan.js @@ -232,7 +232,7 @@ frappe.ui.form.on('Production Plan', { }); }, combine_items: function (frm) { - frm.clear_table('prod_plan_references'); + frm.clear_table("prod_plan_references"); frappe.call({ method: "get_items", @@ -247,6 +247,13 @@ frappe.ui.form.on('Production Plan', { }); }, + combine_sub_items: (frm) => { + if (frm.doc.sub_assembly_items.length > 0) { + frm.clear_table("sub_assembly_items"); + frm.trigger("get_sub_assembly_items"); + } + }, + get_sub_assembly_items: function(frm) { frm.dirty(); diff --git a/erpnext/manufacturing/doctype/production_plan/production_plan.json b/erpnext/manufacturing/doctype/production_plan/production_plan.json index 56cf2b4f08..3bfb764ba5 100644 --- a/erpnext/manufacturing/doctype/production_plan/production_plan.json +++ b/erpnext/manufacturing/doctype/production_plan/production_plan.json @@ -36,6 +36,7 @@ "prod_plan_references", "section_break_24", "get_sub_assembly_items", + "combine_sub_items", "sub_assembly_items", "material_request_planning", "include_non_stock_items", @@ -340,7 +341,6 @@ { "fieldname": "prod_plan_references", "fieldtype": "Table", - "hidden": 1, "label": "Production Plan Item Reference", "options": "Production Plan Item Reference" }, @@ -370,16 +370,23 @@ "fieldname": "to_delivery_date", "fieldtype": "Date", "label": "To Delivery Date" + }, + { + "default": "0", + "fieldname": "combine_sub_items", + "fieldtype": "Check", + "label": "Consolidate Sub Assembly Items" } ], "icon": "fa fa-calendar", "index_web_pages_for_search": 1, "is_submittable": 1, "links": [], - "modified": "2021-09-06 18:35:59.642232", + "modified": "2022-02-23 17:16:10.629378", "modified_by": "Administrator", "module": "Manufacturing", "name": "Production Plan", + "naming_rule": "By \"Naming Series\" field", "owner": "Administrator", "permissions": [ { diff --git a/erpnext/manufacturing/doctype/production_plan/production_plan.py b/erpnext/manufacturing/doctype/production_plan/production_plan.py index 80003dab78..48cd753d75 100644 --- a/erpnext/manufacturing/doctype/production_plan/production_plan.py +++ b/erpnext/manufacturing/doctype/production_plan/production_plan.py @@ -21,7 +21,8 @@ from frappe.utils import ( ) from frappe.utils.csvutils import build_csv_response -from erpnext.manufacturing.doctype.bom.bom import get_children, validate_bom_no +from erpnext.manufacturing.doctype.bom.bom import get_children as get_bom_children +from erpnext.manufacturing.doctype.bom.bom import validate_bom_no from erpnext.manufacturing.doctype.work_order.work_order import get_item_details from erpnext.setup.doctype.item_group.item_group import get_item_group_defaults @@ -570,17 +571,28 @@ class ProductionPlan(Document): @frappe.whitelist() def get_sub_assembly_items(self, manufacturing_type=None): + "Fetch sub assembly items and optionally combine them." self.sub_assembly_items = [] + sub_assembly_items_store = [] # temporary store to process all subassembly items + for row in self.po_items: bom_data = [] get_sub_assembly_items(row.bom_no, bom_data, row.planned_qty) self.set_sub_assembly_items_based_on_level(row, bom_data, manufacturing_type) + sub_assembly_items_store.extend(bom_data) - self.sub_assembly_items.sort(key= lambda d: d.bom_level, reverse=True) - for idx, row in enumerate(self.sub_assembly_items, start=1): - row.idx = idx + if self.combine_sub_items: + # Combine subassembly items + sub_assembly_items_store = self.combine_subassembly_items(sub_assembly_items_store) + + sub_assembly_items_store.sort(key= lambda d: d.bom_level, reverse=True) # sort by bom level + + for idx, row in enumerate(sub_assembly_items_store): + row.idx = idx + 1 + self.append("sub_assembly_items", row) def set_sub_assembly_items_based_on_level(self, row, bom_data, manufacturing_type=None): + "Modify bom_data, set additional details." for data in bom_data: data.qty = data.stock_qty data.production_plan_item = row.name @@ -589,7 +601,32 @@ class ProductionPlan(Document): data.type_of_manufacturing = manufacturing_type or ("Subcontract" if data.is_sub_contracted_item else "In House") - self.append("sub_assembly_items", data) + def combine_subassembly_items(self, sub_assembly_items_store): + "Aggregate if same: Item, Warehouse, Inhouse/Outhouse Manu.g, BOM No." + key_wise_data = {} + for row in sub_assembly_items_store: + key = ( + row.get("production_item"), row.get("fg_warehouse"), + row.get("bom_no"), row.get("type_of_manufacturing") + ) + if key not in key_wise_data: + # intialise (item, wh, bom no, man.g type) wise dict + key_wise_data[key] = row + continue + + existing_row = key_wise_data[key] + if existing_row: + # if row with same (item, wh, bom no, man.g type) key, merge + existing_row.qty += flt(row.qty) + existing_row.stock_qty += flt(row.stock_qty) + existing_row.bom_level = max(existing_row.bom_level, row.bom_level) + continue + else: + # add row with key + key_wise_data[key] = row + + sub_assembly_items_store = [key_wise_data[key] for key in key_wise_data] # unpack into single level list + return sub_assembly_items_store def all_items_completed(self): all_items_produced = all(flt(d.planned_qty) - flt(d.produced_qty) < 0.000001 @@ -1031,7 +1068,7 @@ def get_item_data(item_code): } def get_sub_assembly_items(bom_no, bom_data, to_produce_qty, indent=0): - data = get_children('BOM', parent = bom_no) + data = get_bom_children(parent=bom_no) for d in data: if d.expandable: parent_item_code = frappe.get_cached_value("BOM", bom_no, "item") From 292fe370dbb1d4c090c78f34bd70ddf95774fe5d Mon Sep 17 00:00:00 2001 From: marination Date: Mon, 28 Feb 2022 16:35:20 +0530 Subject: [PATCH 2/5] test: combine sub assembly items and make Production Plan tests atomic --- .../production_plan/test_production_plan.py | 45 ++++++++++++++++++- 1 file changed, 44 insertions(+), 1 deletion(-) diff --git a/erpnext/manufacturing/doctype/production_plan/test_production_plan.py b/erpnext/manufacturing/doctype/production_plan/test_production_plan.py index 2359815813..8e34e1bca3 100644 --- a/erpnext/manufacturing/doctype/production_plan/test_production_plan.py +++ b/erpnext/manufacturing/doctype/production_plan/test_production_plan.py @@ -20,7 +20,7 @@ from erpnext.tests.utils import ERPNextTestCase class TestProductionPlan(ERPNextTestCase): - def setUp(self): + def setUp(self) -> None: for item in ['Test Production Item 1', 'Subassembly Item 1', 'Raw Material Item 1', 'Raw Material Item 2']: create_item(item, valuation_rate=100) @@ -38,6 +38,9 @@ class TestProductionPlan(ERPNextTestCase): if not frappe.db.get_value('BOM', {'item': item}): make_bom(item = item, raw_materials = raw_materials) + def tearDown(self) -> None: + frappe.db.rollback() + def test_production_plan_mr_creation(self): "Test if MRs are created for unavailable raw materials." pln = create_production_plan(item_code='Test Production Item 1') @@ -258,6 +261,46 @@ class TestProductionPlan(ERPNextTestCase): pln.reload() pln.cancel() + def test_production_plan_combine_subassembly(self): + """ + Test combining Sub assembly items belonging to the same BOM in Prod Plan. + 1) Red-Car -> Wheel (sub assembly) > BOM-WHEEL-001 + 2) Green-Car -> Wheel (sub assembly) > BOM-WHEEL-001 + """ + from erpnext.manufacturing.doctype.bom.test_bom import create_nested_bom + + bom_tree_1 = { + "Red-Car": {"Wheel": {"Rubber": {}}} + } + bom_tree_2 = { + "Green-Car": {"Wheel": {"Rubber": {}}} + } + + parent_bom_1 = create_nested_bom(bom_tree_1, prefix="") + parent_bom_2 = create_nested_bom(bom_tree_2, prefix="") + + # make sure both boms use same subassembly bom + subassembly_bom = parent_bom_1.items[0].bom_no + frappe.db.set_value("BOM Item", parent_bom_2.items[0].name, "bom_no", subassembly_bom) + + plan = create_production_plan(item_code="Red-Car", use_multi_level_bom=1, do_not_save=True) + plan.append("po_items", { # Add Green-Car to Prod Plan + 'use_multi_level_bom': 1, + 'item_code': "Green-Car", + 'bom_no': frappe.db.get_value('Item', "Green-Car", 'default_bom'), + 'planned_qty': 1, + 'planned_start_date': now_datetime() + }) + plan.get_sub_assembly_items() + self.assertTrue(len(plan.sub_assembly_items), 2) + + plan.combine_sub_items = 1 + plan.get_sub_assembly_items() + + self.assertTrue(len(plan.sub_assembly_items), 1) # check if sub-assembly items merged + self.assertEqual(plan.sub_assembly_items[0].qty, 2.0) + self.assertEqual(plan.sub_assembly_items[0].stock_qty, 2.0) + def test_pp_to_mr_customer_provided(self): " Test Material Request from Production Plan for Customer Provided Item." create_item('CUST-0987', is_customer_provided_item = 1, customer = '_Test Customer', is_purchase_item = 0) From 25fa6344a6c3dc3af7ecea250d30a421f4016afc Mon Sep 17 00:00:00 2001 From: marination Date: Tue, 1 Mar 2022 12:24:45 +0530 Subject: [PATCH 3/5] test: Adjust test as per teardown - make tests use multi level bom in WO, as BOM at setup is used - earlier, an intermediate BOM made in a test would override the BOM at setup --- .../doctype/production_plan/test_production_plan.py | 4 +++- erpnext/manufacturing/doctype/work_order/test_work_order.py | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/erpnext/manufacturing/doctype/production_plan/test_production_plan.py b/erpnext/manufacturing/doctype/production_plan/test_production_plan.py index 85ecec2f69..ca8cc7a47e 100644 --- a/erpnext/manufacturing/doctype/production_plan/test_production_plan.py +++ b/erpnext/manufacturing/doctype/production_plan/test_production_plan.py @@ -154,7 +154,7 @@ class TestProductionPlan(FrappeTestCase): use_multi_level_bom=0, ignore_existing_ordered_qty=0 ) - self.assertTrue(len(pln.mr_items), 0) + self.assertTrue(len(pln.mr_items)) sr1.cancel() sr2.cancel() @@ -575,6 +575,7 @@ class TestProductionPlan(FrappeTestCase): wip_warehouse='Work In Progress - _TC', fg_warehouse='Finished Goods - _TC', skip_transfer=1, + use_multi_level_bom=1, do_not_submit=True ) wo.production_plan = pln.name @@ -619,6 +620,7 @@ class TestProductionPlan(FrappeTestCase): wip_warehouse='Work In Progress - _TC', fg_warehouse='Finished Goods - _TC', skip_transfer=1, + use_multi_level_bom=1, do_not_submit=True ) wo.production_plan = pln.name diff --git a/erpnext/manufacturing/doctype/work_order/test_work_order.py b/erpnext/manufacturing/doctype/work_order/test_work_order.py index 549ec7b4a6..bc07d22e83 100644 --- a/erpnext/manufacturing/doctype/work_order/test_work_order.py +++ b/erpnext/manufacturing/doctype/work_order/test_work_order.py @@ -1040,7 +1040,7 @@ def make_wo_order_test_record(**args): wo_order.scrap_warehouse = args.fg_warehouse or "_Test Scrap Warehouse - _TC" wo_order.company = args.company or "_Test Company" wo_order.stock_uom = args.stock_uom or "_Test UOM" - wo_order.use_multi_level_bom=0 + wo_order.use_multi_level_bom= args.use_multi_level_bom or 0 wo_order.skip_transfer=args.skip_transfer or 0 wo_order.get_items_and_operations_from_bom() wo_order.sales_order = args.sales_order or None From ffbb2c831152686b9ec0b230b3c72b94edce0c6a Mon Sep 17 00:00:00 2001 From: marination Date: Tue, 1 Mar 2022 12:57:09 +0530 Subject: [PATCH 4/5] fix: Assert False for test that is anticipating an empty table and remove second arg to assertTrue --- .../doctype/production_plan/test_production_plan.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/erpnext/manufacturing/doctype/production_plan/test_production_plan.py b/erpnext/manufacturing/doctype/production_plan/test_production_plan.py index ca8cc7a47e..59cc17a5ee 100644 --- a/erpnext/manufacturing/doctype/production_plan/test_production_plan.py +++ b/erpnext/manufacturing/doctype/production_plan/test_production_plan.py @@ -113,7 +113,7 @@ class TestProductionPlan(FrappeTestCase): item_code='Test Production Item 1', ignore_existing_ordered_qty=1 ) - self.assertTrue(len(pln.mr_items), 1) + self.assertTrue(len(pln.mr_items)) self.assertTrue(flt(pln.mr_items[0].quantity), 1.0) sr1.cancel() @@ -154,7 +154,7 @@ class TestProductionPlan(FrappeTestCase): use_multi_level_bom=0, ignore_existing_ordered_qty=0 ) - self.assertTrue(len(pln.mr_items)) + self.assertFalse(len(pln.mr_items)) sr1.cancel() sr2.cancel() From 1d1d586d4dd356cf84edaed8a51f934194219d11 Mon Sep 17 00:00:00 2001 From: marination Date: Tue, 1 Mar 2022 15:42:34 +0530 Subject: [PATCH 5/5] test: Included sub assembly non-merging in test case --- .../doctype/production_plan/test_production_plan.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/erpnext/manufacturing/doctype/production_plan/test_production_plan.py b/erpnext/manufacturing/doctype/production_plan/test_production_plan.py index 59cc17a5ee..eeab788d5c 100644 --- a/erpnext/manufacturing/doctype/production_plan/test_production_plan.py +++ b/erpnext/manufacturing/doctype/production_plan/test_production_plan.py @@ -301,6 +301,11 @@ class TestProductionPlan(FrappeTestCase): self.assertEqual(plan.sub_assembly_items[0].qty, 2.0) self.assertEqual(plan.sub_assembly_items[0].stock_qty, 2.0) + # change warehouse in one row, sub-assemblies should not merge + plan.po_items[0].warehouse = "Finished Goods - _TC" + plan.get_sub_assembly_items() + self.assertTrue(len(plan.sub_assembly_items), 2) + def test_pp_to_mr_customer_provided(self): " Test Material Request from Production Plan for Customer Provided Item." create_item('CUST-0987', is_customer_provided_item = 1, customer = '_Test Customer', is_purchase_item = 0)