Merge pull request #28228 from marination/se-required-items-from-jc

fix: Pull only Items that are in Job Card in a Stock Entry against Job Card

erpnext/manufacturing/doctype/job_card/job_card.py

@@ -627,17 +627,22 @@ def make_material_request(source_name, target_doc=None):
 
 @frappe.whitelist()
 def make_stock_entry(source_name, target_doc=None):
-	def update_item(obj, target, source_parent):
+	def update_item(source, target, source_parent):
 		target.t_warehouse = source_parent.wip_warehouse
+
 		if not target.conversion_factor:
 			target.conversion_factor = 1
 
+		pending_rm_qty = flt(source.required_qty) - flt(source.transferred_qty)
+		if pending_rm_qty > 0:
+			target.qty = pending_rm_qty
+
 	def set_missing_values(source, target):
 		target.purpose = "Material Transfer for Manufacture"
 		target.from_bom = 1
 
 		# avoid negative 'For Quantity'
-		pending_fg_qty = source.get('for_quantity', 0) - source.get('transferred_qty', 0)
+		pending_fg_qty = flt(source.get('for_quantity', 0)) - flt(source.get('transferred_qty', 0))
 		target.fg_completed_qty = pending_fg_qty if pending_fg_qty > 0 else 0
 
 		target.set_transfer_qty()

erpnext/manufacturing/doctype/job_card/test_job_card.py

@@ -15,11 +15,22 @@ from erpnext.stock.doctype.stock_entry.stock_entry_utils import make_stock_entry
 
 
 class TestJobCard(unittest.TestCase):
-
 	def setUp(self):
+		make_bom_for_jc_tests()
+
 		transfer_material_against, source_warehouse = None, None
-		tests_that_transfer_against_jc = ("test_job_card_multiple_materials_transfer",
-			"test_job_card_excess_material_transfer")
+
+		tests_that_skip_setup = (
+			"test_job_card_material_transfer_correctness",
+		)
+		tests_that_transfer_against_jc = (
+			"test_job_card_multiple_materials_transfer",
+			"test_job_card_excess_material_transfer",
+			"test_job_card_partial_material_transfer"
+		)
+
+		if self._testMethodName in tests_that_skip_setup:
+			return
 
 		if self._testMethodName in tests_that_transfer_against_jc:
 			transfer_material_against = "Job Card"
@@ -190,4 +201,132 @@ class TestJobCard(unittest.TestCase):
 		job_card.submit()
 
 		# JC is Completed with excess transfer
-		self.assertEqual(job_card.status, "Completed")
+		self.assertEqual(job_card.status, "Completed")
+
+	def test_job_card_partial_material_transfer(self):
+		"Test partial material transfer against Job Card"
+
+		make_stock_entry(item_code="_Test Item", target="Stores - _TC",
+			qty=25, basic_rate=100)
+		make_stock_entry(item_code="_Test Item Home Desktop Manufactured",
+			target="Stores - _TC", qty=15, basic_rate=100)
+
+		job_card_name = frappe.db.get_value("Job Card", {'work_order': self.work_order.name})
+		job_card = frappe.get_doc("Job Card", job_card_name)
+
+		# partially transfer
+		transfer_entry = make_stock_entry_from_jc(job_card_name)
+		transfer_entry.fg_completed_qty = 1
+		transfer_entry.get_items()
+		transfer_entry.insert()
+		transfer_entry.submit()
+
+		job_card.reload()
+		self.assertEqual(job_card.transferred_qty, 1)
+		self.assertEqual(transfer_entry.items[0].qty, 5)
+		self.assertEqual(transfer_entry.items[1].qty, 3)
+
+		# transfer remaining
+		transfer_entry_2 = make_stock_entry_from_jc(job_card_name)
+
+		self.assertEqual(transfer_entry_2.fg_completed_qty, 1)
+		self.assertEqual(transfer_entry_2.items[0].qty, 5)
+		self.assertEqual(transfer_entry_2.items[1].qty, 3)
+
+		transfer_entry_2.insert()
+		transfer_entry_2.submit()
+
+		job_card.reload()
+		self.assertEqual(job_card.transferred_qty, 2)
+
+	def test_job_card_material_transfer_correctness(self):
+		"""
+			1. Test if only current Job Card Items are pulled in a Stock Entry against a Job Card
+			2. Test impact of changing 'For Qty' in such a Stock Entry
+		"""
+		create_bom_with_multiple_operations()
+		work_order = make_wo_with_transfer_against_jc()
+
+		job_card_name = frappe.db.get_value(
+			"Job Card",
+			{"work_order": work_order.name,"operation": "Test Operation A"}
+		)
+		job_card = frappe.get_doc("Job Card", job_card_name)
+
+		self.assertEqual(len(job_card.items), 1)
+		self.assertEqual(job_card.items[0].item_code, "_Test Item")
+
+		# check if right items are mapped in transfer entry
+		transfer_entry = make_stock_entry_from_jc(job_card_name)
+		transfer_entry.insert()
+
+		self.assertEqual(len(transfer_entry.items), 1)
+		self.assertEqual(transfer_entry.items[0].item_code, "_Test Item")
+		self.assertEqual(transfer_entry.items[0].qty, 4)
+
+		# change 'For Qty' and check impact on items table
+		# no.of items should be the same with qty change
+		transfer_entry.fg_completed_qty = 2
+		transfer_entry.get_items()
+
+		self.assertEqual(len(transfer_entry.items), 1)
+		self.assertEqual(transfer_entry.items[0].item_code, "_Test Item")
+		self.assertEqual(transfer_entry.items[0].qty, 2)
+
+		# rollback via tearDown method
+
+def create_bom_with_multiple_operations():
+	"Create a BOM with multiple operations and Material Transfer against Job Card"
+	from erpnext.manufacturing.doctype.operation.test_operation import make_operation
+
+	test_record = frappe.get_test_records("BOM")[2]
+	bom_doc = frappe.get_doc(test_record)
+
+	row = {
+		"operation": "Test Operation A",
+		"workstation": "_Test Workstation A",
+		"hour_rate_rent": 300,
+		"time_in_mins": 60
+	}
+	make_workstation(row)
+	make_operation(row)
+
+	bom_doc.append("operations", {
+		"operation": "Test Operation A",
+		"description": "Test Operation A",
+		"workstation": "_Test Workstation A",
+		"hour_rate": 300,
+		"time_in_mins": 60,
+		"operating_cost": 100
+	})
+
+	bom_doc.transfer_material_against = "Job Card"
+	bom_doc.save()
+	bom_doc.submit()
+
+	return bom_doc
+
+def make_wo_with_transfer_against_jc():
+	"Create a WO with multiple operations and Material Transfer against Job Card"
+
+	work_order = make_wo_order_test_record(
+		item="_Test FG Item 2",
+		qty=4,
+		transfer_material_against="Job Card",
+		source_warehouse="Stores - _TC",
+		do_not_submit=True
+	)
+	work_order.required_items[0].operation = "Test Operation A"
+	work_order.required_items[1].operation = "_Test Operation 1"
+	work_order.submit()
+
+	return work_order
+
+def make_bom_for_jc_tests():
+	test_records = frappe.get_test_records('BOM')
+	bom = frappe.copy_doc(test_records[2])
+	bom.set_rate_of_sub_assembly_item_based_on_bom = 0
+	bom.rm_cost_as_per = "Valuation Rate"
+	bom.items[0].uom = "_Test UOM 1"
+	bom.items[0].conversion_factor = 5
+	bom.insert()

erpnext/manufacturing/doctype/operation/test_operation.py

@@ -17,15 +17,13 @@ def make_operation(*args, **kwargs):
 
 	args = frappe._dict(args)
 
-	try:
+	if not frappe.db.exists("Operation", args.operation):
 		doc = frappe.get_doc({
 			"doctype": "Operation",
 			"name": args.operation,
 			"workstation": args.workstation
 		})
-
 		doc.insert()
-
 		return doc
-	except frappe.DuplicateEntryError:
-		return frappe.get_doc("Operation", args.operation)
+
+	return frappe.get_doc("Operation", args.operation)

erpnext/manufacturing/doctype/workstation/test_workstation.py

@@ -89,7 +89,7 @@ def make_workstation(*args, **kwargs):
 	args = frappe._dict(args)
 
 	workstation_name = args.workstation_name or args.workstation
-	try:
+	if not frappe.db.exists("Workstation", workstation_name):
 		doc = frappe.get_doc({
 			"doctype": "Workstation",
 			"workstation_name": workstation_name
@@ -99,5 +99,5 @@ def make_workstation(*args, **kwargs):
 		doc.insert()
 
 		return doc
-	except frappe.DuplicateEntryError:
-		return frappe.get_doc("Workstation", workstation_name)
+
+	return frappe.get_doc("Workstation", workstation_name)

erpnext/manufacturing/report/production_plan_summary/production_plan_summary.py

@@ -28,8 +28,15 @@ def get_production_plan_item_details(filters, data, order_details):
 
 	production_plan_doc = frappe.get_cached_doc("Production Plan", filters.get("production_plan"))
 	for row in production_plan_doc.po_items:
-		work_order = frappe.get_cached_value("Work Order", {"production_plan_item": row.name,
-			"bom_no": row.bom_no, "production_item": row.item_code}, "name")
+		work_order = frappe.get_value(
+			"Work Order",
+			{
+				"production_plan_item": row.name,
+				"bom_no": row.bom_no,
+				"production_item": row.item_code
+			},
+			"name"
+		)
 
 		if row.item_code not in itemwise_indent:
 			itemwise_indent.setdefault(row.item_code, {})
@@ -40,10 +47,10 @@ def get_production_plan_item_details(filters, data, order_details):
 			"item_name": frappe.get_cached_value("Item", row.item_code, "item_name"),
 			"qty": row.planned_qty,
 			"document_type": "Work Order",
-			"document_name": work_order,
+			"document_name": work_order or "",
 			"bom_level": frappe.get_cached_value("BOM", row.bom_no, "bom_level"),
-			"produced_qty": order_details.get((work_order, row.item_code)).get("produced_qty"),
-			"pending_qty": flt(row.planned_qty) - flt(order_details.get((work_order, row.item_code)).get("produced_qty"))
+			"produced_qty": order_details.get((work_order, row.item_code), {}).get("produced_qty", 0),
+			"pending_qty": flt(row.planned_qty) - flt(order_details.get((work_order, row.item_code), {}).get("produced_qty", 0))
 		})
 
 		get_production_plan_sub_assembly_item_details(filters, row, production_plan_doc, data, order_details)
@@ -54,11 +61,23 @@ def get_production_plan_sub_assembly_item_details(filters, row, production_plan_
 			subcontracted_item = (item.type_of_manufacturing == 'Subcontract')
 
 			if subcontracted_item:
-				docname = frappe.get_cached_value("Purchase Order Item",
-					{"production_plan_sub_assembly_item": item.name, "docstatus": ("<", 2)}, "parent")
+				docname = frappe.get_value(
+					"Purchase Order Item",
+					{
+						"production_plan_sub_assembly_item": item.name,
+						"docstatus": ("<", 2)
+					},
+					"parent"
+				)
 			else:
-				docname = frappe.get_cached_value("Work Order",
-					{"production_plan_sub_assembly_item": item.name, "docstatus": ("<", 2)}, "name")
+				docname = frappe.get_value(
+					"Work Order",
+					{
+						"production_plan_sub_assembly_item": item.name,
+						"docstatus": ("<", 2)
+					},
+					"name"
+				)
 
 			data.append({
 				"indent": 1,
@@ -66,10 +85,10 @@ def get_production_plan_sub_assembly_item_details(filters, row, production_plan_
 				"item_name": item.item_name,
 				"qty": item.qty,
 				"document_type": "Work Order" if not subcontracted_item else "Purchase Order",
-				"document_name": docname,
+				"document_name": docname or "",
 				"bom_level": item.bom_level,
-				"produced_qty": order_details.get((docname, item.production_item)).get("produced_qty"),
-				"pending_qty": flt(item.qty) - flt(order_details.get((docname, item.production_item)).get("produced_qty"))
+				"produced_qty": order_details.get((docname, item.production_item), {}).get("produced_qty", 0),
+				"pending_qty": flt(item.qty) - flt(order_details.get((docname, item.production_item), {}).get("produced_qty", 0))
 			})
 
 def get_work_order_details(filters, order_details):

erpnext/stock/doctype/stock_entry/stock_entry.py

@@ -1462,52 +1462,94 @@ class StockEntry(StockController):
 		return item_dict
 
 	def get_pro_order_required_items(self, backflush_based_on=None):
-		item_dict = frappe._dict()
-		pro_order = frappe.get_doc("Work Order", self.work_order)
-		if not frappe.db.get_value("Warehouse", pro_order.wip_warehouse, "is_group"):
-			wip_warehouse = pro_order.wip_warehouse
+		"""
+			Gets Work Order Required Items only if Stock Entry purpose is **Material Transferred for Manufacture**.
+		"""
+		item_dict, job_card_items = frappe._dict(), []
+		work_order = frappe.get_doc("Work Order", self.work_order)
+
+		consider_job_card = work_order.transfer_material_against == "Job Card" and self.get("job_card")
+		if consider_job_card:
+			job_card_items = self.get_job_card_item_codes(self.get("job_card"))
+
+		if not frappe.db.get_value("Warehouse", work_order.wip_warehouse, "is_group"):
+			wip_warehouse = work_order.wip_warehouse
 		else:
 			wip_warehouse = None
 
-		for d in pro_order.get("required_items"):
-			if ( ((flt(d.required_qty) > flt(d.transferred_qty)) or
-				(backflush_based_on == "Material Transferred for Manufacture")) and
-				(d.include_item_in_manufacturing or self.purpose != "Material Transfer for Manufacture")):
+		for d in work_order.get("required_items"):
+			if consider_job_card and (d.item_code not in job_card_items):
+				continue
+
+			transfer_pending = flt(d.required_qty) > flt(d.transferred_qty)
+			can_transfer = transfer_pending or (backflush_based_on == "Material Transferred for Manufacture")
+
+			if not can_transfer:
+				continue
+
+			if d.include_item_in_manufacturing:
 				item_row = d.as_dict()
+				item_row["idx"] = len(item_dict) + 1
+
+				if consider_job_card:
+					job_card_item = frappe.db.get_value(
+						"Job Card Item",
+						{
+							"item_code": d.item_code,
+							"parent": self.get("job_card")
+						}
+					)
+					item_row["job_card_item"] = job_card_item or None
+
 				if d.source_warehouse and not frappe.db.get_value("Warehouse", d.source_warehouse, "is_group"):
 					item_row["from_warehouse"] = d.source_warehouse
 
 				item_row["to_warehouse"] = wip_warehouse
 				if item_row["allow_alternative_item"]:
-					item_row["allow_alternative_item"] = pro_order.allow_alternative_item
+					item_row["allow_alternative_item"] = work_order.allow_alternative_item
 
 				item_dict.setdefault(d.item_code, item_row)
 
 		return item_dict
 
+	def get_job_card_item_codes(self, job_card=None):
+		if not job_card:
+			return []
+
+		job_card_items = frappe.get_all(
+			"Job Card Item",
+			filters={
+				"parent": job_card
+			},
+			fields=["item_code"],
+			distinct=True
+		)
+		return [d.item_code for d in job_card_items]
+
 	def add_to_stock_entry_detail(self, item_dict, bom_no=None):
 		for d in item_dict:
-			stock_uom = item_dict[d].get("stock_uom") or frappe.db.get_value("Item", d, "stock_uom")
+			item_row = item_dict[d]
+			stock_uom = item_row.get("stock_uom") or frappe.db.get_value("Item", d, "stock_uom")
 
 			se_child = self.append('items')
-			se_child.s_warehouse = item_dict[d].get("from_warehouse")
-			se_child.t_warehouse = item_dict[d].get("to_warehouse")
-			se_child.item_code = item_dict[d].get('item_code') or cstr(d)
-			se_child.uom = item_dict[d]["uom"] if item_dict[d].get("uom") else stock_uom
+			se_child.s_warehouse = item_row.get("from_warehouse")
+			se_child.t_warehouse = item_row.get("to_warehouse")
+			se_child.item_code = item_row.get('item_code') or cstr(d)
+			se_child.uom = item_row["uom"] if item_row.get("uom") else stock_uom
 			se_child.stock_uom = stock_uom
-			se_child.qty = flt(item_dict[d]["qty"], se_child.precision("qty"))
-			se_child.allow_alternative_item = item_dict[d].get("allow_alternative_item", 0)
-			se_child.subcontracted_item = item_dict[d].get("main_item_code")
-			se_child.cost_center = (item_dict[d].get("cost_center") or
-				get_default_cost_center(item_dict[d], company = self.company))
-			se_child.is_finished_item = item_dict[d].get("is_finished_item", 0)
-			se_child.is_scrap_item = item_dict[d].get("is_scrap_item", 0)
-			se_child.is_process_loss = item_dict[d].get("is_process_loss", 0)
+			se_child.qty = flt(item_row["qty"], se_child.precision("qty"))
+			se_child.allow_alternative_item = item_row.get("allow_alternative_item", 0)
+			se_child.subcontracted_item = item_row.get("main_item_code")
+			se_child.cost_center = (item_row.get("cost_center") or
+				get_default_cost_center(item_row, company = self.company))
+			se_child.is_finished_item = item_row.get("is_finished_item", 0)
+			se_child.is_scrap_item = item_row.get("is_scrap_item", 0)
+			se_child.is_process_loss = item_row.get("is_process_loss", 0)
 
 			for field in ["idx", "po_detail", "original_item", "expense_account",
 				"description", "item_name", "serial_no", "batch_no", "allow_zero_valuation_rate"]:
-				if item_dict[d].get(field):
-					se_child.set(field, item_dict[d].get(field))
+				if item_row.get(field):
+					se_child.set(field, item_row.get(field))
 
 			if se_child.s_warehouse==None:
 				se_child.s_warehouse = self.from_warehouse
@@ -1515,12 +1557,11 @@ class StockEntry(StockController):
 				se_child.t_warehouse = self.to_warehouse
 
 			# in stock uom
-			se_child.conversion_factor = flt(item_dict[d].get("conversion_factor")) or 1
-			se_child.transfer_qty = flt(item_dict[d]["qty"]*se_child.conversion_factor, se_child.precision("qty"))
-
+			se_child.conversion_factor = flt(item_row.get("conversion_factor")) or 1
+			se_child.transfer_qty = flt(item_row["qty"]*se_child.conversion_factor, se_child.precision("qty"))
 
-			# to be assigned for finished item
-			se_child.bom_no = bom_no
+			se_child.bom_no = bom_no # to be assigned for finished item
+			se_child.job_card_item = item_row.get("job_card_item") if self.get("job_card") else None
 
 	def validate_with_material_request(self):
 		for item in self.get("items"):