fix(manufacturing): prevent excess process loss from inflating plan quantities (#58847)

erpnext/manufacturing/doctype/job_card/job_card.py

@@ -1080,6 +1080,10 @@ class JobCard(Document):
 		frappe.db.set_value("Work Order Operation", self.operation_id, "completed_qty", completed_qty)
 		if self.finished_good and work_order.production_item == self.finished_good:
 			work_order.db_set("produced_qty", sum(flt(row.manufactured_qty) for row in job_cards))
+			if work_order.production_plan:
+				ProductionPlanWorkOrderQuantities(work_order.production_plan).validate_work_order(
+					work_order, process_loss_qty=work_order.process_loss_qty
+				)
 			work_order.db_set("status", work_order.get_status())
 
 	def update_corrective_in_work_order(self, wo):

erpnext/manufacturing/doctype/production_plan/services/work_order_quantities.py

@@ -29,18 +29,17 @@ class ProductionPlanWorkOrderQuantities:
 			frappe.db.get_single_value("Manufacturing Settings", "overproduction_percentage_for_work_order")
 		)
 		precision = work_order.precision("qty")
-		maximum_qty = flt(
+		maximum_qty = flt(flt(row.planned_qty) * (1 + allowance / 100) - committed, precision)
-			flt(row.planned_qty) * (1 + allowance / 100) - committed + flt(process_loss_qty), precision
+		committed_qty = flt(self._get_committed_qty(work_order, process_loss_qty), precision)
-		)
+		if committed_qty > maximum_qty:
-		if flt(work_order.qty, precision) > maximum_qty:
 			frappe.throw(
 				_(
-					"Row {0} in {1} {2}: Work Order quantity {3} exceeds the remaining allowed quantity {4}."
+					"Row {0} in {1} {2}: Work Order quantity after process loss {3} exceeds the remaining allowed quantity {4}."
 				).format(
 					row.idx,
 					_(row.doctype),
 					get_link_to_form("Production Plan", self.production_plan),
-					flt(work_order.qty, precision),
+					committed_qty,
 					max(0, maximum_qty),
 				),
 				OverProductionError,
@@ -111,7 +110,13 @@ class ProductionPlanWorkOrderQuantities:
 		# Read rows instead of an aggregate so MariaDB uses a current locking read on submit.
 		work_orders = frappe.qb.get_query(
 			"Work Order",
-			fields=["production_plan_item", "production_plan_sub_assembly_item", "qty", "process_loss_qty"],
+			fields=[
+				"production_plan_item",
+				"production_plan_sub_assembly_item",
+				"qty",
+				"produced_qty",
+				"process_loss_qty",
+			],
 			filters=filters,
 			for_update=for_update,
 			order_by="name",
@@ -127,5 +132,11 @@ class ProductionPlanWorkOrderQuantities:
 				else "production_plan_item"
 			)
 			if work_order.get(field):
-				quantities[field][work_order[field]] += flt(work_order.qty) - flt(work_order.process_loss_qty)
+				quantities[field][work_order[field]] += self._get_committed_qty(
+					work_order, work_order.process_loss_qty
+				)
 		return quantities
+
+	def _get_committed_qty(self, work_order, process_loss_qty):
+		# Excess loss in existing records must not erase finished goods already produced.
+		return max(0, flt(work_order.produced_qty), flt(work_order.qty) - flt(process_loss_qty))

erpnext/manufacturing/doctype/production_plan/test_work_order_quantities.py

@@ -7,6 +7,9 @@ from unittest.mock import patch
 import frappe
 
 from erpnext.manufacturing.doctype.operation.test_operation import make_operation
+from erpnext.manufacturing.doctype.production_plan.services.work_order_quantities import (
+	ProductionPlanWorkOrderQuantities,
+)
 from erpnext.manufacturing.doctype.production_plan.test_production_plan import (
 	create_production_plan,
 	make_bom,
@@ -14,6 +17,7 @@ from erpnext.manufacturing.doctype.production_plan.test_production_plan import (
 from erpnext.manufacturing.doctype.work_order.mapper import make_stock_entry as make_se_from_wo
 from erpnext.manufacturing.doctype.work_order.work_order import (
 	OverProductionError,
+	StockOverProductionError,
 	close_work_order,
 	stop_unstop,
 )
@@ -87,6 +91,101 @@ class TestProductionPlanWorkOrderQuantities(ERPNextTestSuite):
 				first.reload().cancel()
 				self.assert_pending_qty(plan, field, 100)
 
+	def test_cumulative_manufacture_loss_exceeds_work_order(self):
+		plan = self.make_plan()
+		for field in (None, *REFERENCE_FIELDS):
+			with self.subTest(field=field):
+				first = self.create_work_order(plan, field or "production_plan_item")
+				if field is None:
+					first.production_plan = None
+					first.production_plan_item = None
+				first.qty = 100
+				first.submit()
+				self.manufacture_with_loss(first, loss_qty=99)
+				second_entry = self.manufacture_with_loss(first, loss_qty=99, submit=False)
+				self.assert_manufacture_rejected(second_entry, StockOverProductionError)
+				self.assertEqual(first.reload().produced_qty, 1)
+				self.assertEqual(first.process_loss_qty, 99)
+				if field:
+					self.assert_pending_qty(plan, field, 99)
+					replacement = self.create_work_order(plan, field)
+					replacement.submit()
+					self.assert_pending_qty(plan, field, 0)
+
+	def test_cumulative_manufacture_loss_respects_allowance(self):
+		frappe.db.set_single_value("Manufacturing Settings", "overproduction_percentage_for_work_order", 10)
+		plan = self.make_plan()
+		for field in REFERENCE_FIELDS:
+			with self.subTest(field=field):
+				first = self.create_work_order(plan, field)
+				first.submit()
+				self.manufacture_with_loss(first, qty=50)
+				self.manufacture_with_loss(first, qty=50)
+				last_entry = self.manufacture_with_loss(first, qty=10)
+				self.assertEqual(first.reload().produced_qty, 99)
+				self.assertEqual(first.process_loss_qty, 11)
+				self.assert_pending_qty(plan, field, 1)
+				excess = self.manufacture_with_loss(first, qty=1, submit=False)
+				self.assert_manufacture_rejected(excess, StockOverProductionError)
+				excess.delete()
+				last_entry.cancel()
+				self.assertEqual(first.reload().produced_qty, 90)
+				self.assertEqual(first.process_loss_qty, 10)
+				self.manufacture_with_loss(first, qty=10)
+
+	@ERPNextTestSuite.change_settings("System Settings", {"float_precision": 6})
+	def test_cumulative_fractional_manufacture_loss(self):
+		plan = self.make_plan(qty=0.3)
+		for field in REFERENCE_FIELDS:
+			with self.subTest(field=field):
+				first = self.create_work_order(plan, field)
+				first.submit()
+				self.manufacture_with_loss(first, qty=0.1, loss_qty=0.025)
+				self.manufacture_with_loss(first, qty=0.2, loss_qty=0.05)
+				self.assertAlmostEqual(first.reload().produced_qty, 0.225)
+				self.assertAlmostEqual(first.process_loss_qty, 0.075)
+				excess = self.manufacture_with_loss(first, qty=0.001, submit=False)
+				self.assert_manufacture_rejected(excess, StockOverProductionError)
+
+	def test_existing_excess_loss_preserves_produced_quantity(self):
+		for field in REFERENCE_FIELDS:
+			for loss_qty, produced_qty in ((198, 2), (100, 2), (20, 90), (198, 0)):
+				with self.subTest(field=field, loss_qty=loss_qty, produced_qty=produced_qty):
+					plan = self.make_plan()
+					first = self.create_work_order(plan, field)
+					first.submit()
+					# Reproduce records saved before cumulative manufacture validation existed.
+					first.db_set({"process_loss_qty": loss_qty, "produced_qty": produced_qty})
+					pending_qty = 100 - produced_qty
+					self.assert_pending_qty(plan, field, pending_qty)
+					replacement = self.create_work_order(plan, field)
+					self.assertEqual(replacement.qty, pending_qty)
+					self.assert_overproduction(replacement, pending_qty + 1)
+					replacement.qty = pending_qty
+					replacement.submit()
+					self.assert_pending_qty(plan, field, 0)
+					quantities = ProductionPlanWorkOrderQuantities(plan.name)
+					quantities.validate_work_order(first, process_loss_qty=loss_qty)
+					with self.assertRaises(OverProductionError):
+						quantities.validate_work_order(first, process_loss_qty=0)
+
+	def test_more_production_cannot_consume_replacement_allowance(self):
+		frappe.db.set_single_value("Manufacturing Settings", "overproduction_percentage_for_work_order", 10)
+		plan = self.make_plan()
+		for field in REFERENCE_FIELDS:
+			with self.subTest(field=field):
+				first = self.create_work_order(plan, field)
+				first.submit()
+				self.manufacture_with_loss(first, loss_qty=99)
+				replacement = self.create_work_order(plan, field)
+				replacement.qty = 109
+				replacement.submit()
+				# This fits the first Work Order's allowance, but exceeds the plan's 110 units.
+				excess = self.manufacture_with_loss(first, qty=10, loss_qty=9, submit=False)
+				self.assert_manufacture_rejected(excess, OverProductionError)
+				self.assertEqual(first.reload().produced_qty, 1)
+				self.assertEqual(first.process_loss_qty, 99)
+
 	def test_loss_reversal_with_draft_replacement(self):
 		plan = self.make_plan()
 		for field in REFERENCE_FIELDS:
@@ -337,15 +436,33 @@ class TestProductionPlanWorkOrderQuantities(ERPNextTestSuite):
 		copy.insert()
 		return copy
 
-	def manufacture_with_loss(self, work_order, qty=None):
+	def manufacture_with_loss(self, work_order, qty=None, *, loss_qty=None, submit=True):
 		for item in work_order.required_items:
 			make_stock_entry(
 				item_code=item.item_code, target=self.warehouse, qty=item.required_qty, basic_rate=10
 			)
 		entry = frappe.get_doc(make_se_from_wo(work_order.name, "Manufacture", qty or work_order.qty))
-		entry.submit()
+		if loss_qty is not None:
+			entry.process_loss_qty = loss_qty
+			entry.process_loss_percentage = loss_qty / entry.fg_completed_qty * 100
+			for item in entry.items:
+				if item.is_finished_item:
+					item.qty = entry.fg_completed_qty - loss_qty
+		if submit:
+			entry.submit()
+		else:
+			entry.save()
 		return entry
 
+	def assert_manufacture_rejected(self, entry, exception):
+		frappe.db.savepoint("excess_manufacture")
+		try:
+			with self.assertRaises(exception):
+				entry.submit()
+		finally:
+			frappe.db.rollback(save_point="excess_manufacture")
+		self.assertEqual(entry.reload().docstatus, 0)
+
 	def assert_loss_reversal_blocked(self, document):
 		work_order = frappe.get_doc("Work Order", document.work_order)
 		frappe.db.savepoint("loss_reversal")

erpnext/manufacturing/doctype/work_order/services/status.py

@@ -216,11 +216,15 @@ class StatusService:
 
 		qty = self.get_transferred_or_manufactured_qty(purpose, fieldname)
 		completed_qty = self.doc.qty + (self._qty_allowance(purpose) / 100 * self.doc.qty)
-		if qty > completed_qty:
+		qty_to_validate = qty + flt(self.doc.process_loss_qty) if purpose == "Manufacture" else qty
+		precision = self.doc.precision(fieldname)
+		if flt(qty_to_validate, precision) > flt(completed_qty, precision):
 			frappe.throw(
 				_("{0} ({1}) cannot be greater than planned quantity ({2}) in Work Order {3}").format(
-					self.doc.meta.get_translated_label(fieldname),
+					_("Manufactured Qty (including Process Loss)")
-					qty,
+					if purpose == "Manufacture"
+					else self.doc.meta.get_translated_label(fieldname),
+					flt(qty_to_validate, precision),
 					completed_qty,
 					self.doc.name,
 				),
@@ -228,6 +232,10 @@ class StatusService:
 			)
 
 		self.doc.db_set(fieldname, qty)
+		if purpose == "Manufacture" and self.doc.production_plan:
+			ProductionPlanWorkOrderQuantities(self.doc.production_plan).validate_work_order(
+				self.doc, process_loss_qty=self.doc.process_loss_qty
+			)
 		self._update_produced_qty_in_so()
 
 	def _skip_transfer_purpose(self, purpose):