fix(job_card): reject a completion split that cannot add up (#57687)

* fix(job_card): reject a completion split that cannot add up

The completion dialogs silently dropped a recalculation whose result went
negative, so entering a pending qty larger than what is left of the qty to
manufacture kept the contradiction (3 to manufacture, 3 completed, 2 pending)
and the job card only failed much later, on submission.

Keep the split consistent while it is entered: reset the pending qty when the
qty to manufacture changes, and refuse a completed, pending or process loss qty
that leaves the others negative. complete_job_card validates the same rule, so
the shop floor and the API cannot store a split that will never submit.

Also name the three parts in the submission error instead of calling their sum
the Total Completed Qty, which read as a contradiction of the field itself.

* test(job_card): cover the completion qty split guard

erpnext/manufacturing/doctype/job_card/job_card.js

@@ -249,6 +249,7 @@ frappe.ui.form.on("Job Card", {
 				change() {
 					const dialog = frm.job_completion_dialog;
 					dialog.set_value("completed_qty", dialog.get_value("for_quantity"));
+					dialog.set_value("pending_qty", 0);
 					dialog.set_value("process_loss_qty", 0);
 				},
 			},
@@ -260,8 +261,21 @@ frappe.ui.form.on("Job Card", {
 				default: pending_qty,
 				change() {
 					const dialog = frm.job_completion_dialog;
-					const remaining = dialog.get_value("for_quantity") - dialog.get_value("completed_qty");
-					if (remaining > 0 && remaining != dialog.get_value("pending_qty")) {
+					const remaining =
+						dialog.get_value("for_quantity") -
+						dialog.get_value("completed_qty") -
+						dialog.get_value("process_loss_qty");
+
+					if (remaining < 0) {
+						const max_completed_qty =
+							flt(dialog.get_value("for_quantity")) - flt(dialog.get_value("process_loss_qty"));
+						dialog.set_value("completed_qty", max_completed_qty);
+						frappe.throw(
+							__("Completed Quantity cannot be greater than {0}", [max_completed_qty])
+						);
+					}
+
+					if (remaining != dialog.get_value("pending_qty")) {
 						dialog.set_value("pending_qty", remaining);
 					}
 				},
@@ -277,7 +291,18 @@ frappe.ui.form.on("Job Card", {
 						dialog.get_value("for_quantity") -
 						dialog.get_value("completed_qty") -
 						dialog.get_value("pending_qty");
-					if (process_loss_qty >= 0 && process_loss_qty != dialog.get_value("process_loss_qty")) {
+
+					if (process_loss_qty < 0) {
+						dialog.set_value("pending_qty", 0);
+						frappe.throw(
+							__("Pending Quantity cannot be greater than {0}", [
+								flt(dialog.get_value("for_quantity")) -
+									flt(dialog.get_value("completed_qty")),
+							])
+						);
+					}
+
+					if (process_loss_qty != dialog.get_value("process_loss_qty")) {
 						dialog.set_value("process_loss_qty", process_loss_qty);
 					}
 				},
@@ -292,7 +317,18 @@ frappe.ui.form.on("Job Card", {
 						dialog.get_value("for_quantity") -
 						dialog.get_value("completed_qty") -
 						dialog.get_value("process_loss_qty");
-					if (remaining >= 0 && remaining != dialog.get_value("pending_qty")) {
+
+					if (remaining < 0) {
+						dialog.set_value("process_loss_qty", 0);
+						frappe.throw(
+							__("Process Loss Quantity cannot be greater than {0}", [
+								flt(dialog.get_value("for_quantity")) -
+									flt(dialog.get_value("completed_qty")),
+							])
+						);
+					}
+
+					if (remaining != dialog.get_value("pending_qty")) {
 						dialog.set_value("pending_qty", remaining);
 					}
 				},

erpnext/manufacturing/doctype/job_card/job_card.py

@@ -944,19 +944,21 @@ class JobCard(Document):
 			return
 
 		precision = self.precision("total_completed_qty")
-		total_completed_qty = flt(
+		accounted_qty = flt(
 			flt(self.total_completed_qty, precision)
 			+ flt(self.process_loss_qty, precision)
 			+ flt(self.pending_qty, precision)
 		)
 
-		if self.for_quantity and flt(total_completed_qty, precision) != flt(self.for_quantity, precision):
+		if self.for_quantity and flt(accounted_qty, precision) != flt(self.for_quantity, precision):
 			frappe.throw(
-				_("The {0} ({1}) must be equal to {2} ({3})").format(
-					bold(_("Total Completed Qty")),
-					bold(flt(total_completed_qty, precision)),
-					bold(_("Qty to Manufacture")),
-					bold(self.for_quantity),
+				_(
+					"Total Completed Qty ({0}), Process Loss Qty ({1}) and Pending Qty ({2}) must add up to the Qty to Manufacture ({3})."
+				).format(
+					bold(flt(self.total_completed_qty, precision)),
+					bold(flt(self.process_loss_qty, precision)),
+					bold(flt(self.pending_qty, precision)),
+					bold(flt(self.for_quantity, precision)),
 				)
 			)
 
@@ -1663,8 +1665,8 @@ class JobCard(Document):
 		if isinstance(kwargs, dict):
 			kwargs = frappe._dict(kwargs)
 
-		self.set_for_quantity(kwargs)
 		self.validate_complete_job_card_qty(kwargs)
+		self.set_for_quantity(kwargs)
 
 		self.pending_qty = flt(kwargs.pending_qty)
 		self.process_loss_qty = flt(kwargs.process_loss_qty)
@@ -1699,6 +1701,29 @@ class JobCard(Document):
 		if flt(kwargs.pending_qty) and flt(kwargs.pending_qty) > self.for_quantity:
 			frappe.throw(_("Pending quantity cannot be greater than the for quantity."))
 
+		self.validate_completion_qty_split(kwargs)
+
+	def validate_completion_qty_split(self, kwargs):
+		if not flt(kwargs.for_quantity):
+			return
+
+		precision = self.precision("total_completed_qty")
+		accounted_qty = flt(kwargs.qty) + flt(kwargs.pending_qty) + flt(kwargs.process_loss_qty)
+
+		if flt(accounted_qty, precision) == flt(kwargs.for_quantity, precision):
+			return
+
+		frappe.throw(
+			_(
+				"Completed Quantity ({0}), Pending Quantity ({1}) and Process Loss Quantity ({2}) must add up to the Qty to Manufacture ({3})."
+			).format(
+				bold(flt(kwargs.qty, precision)),
+				bold(flt(kwargs.pending_qty, precision)),
+				bold(flt(kwargs.process_loss_qty, precision)),
+				bold(flt(kwargs.for_quantity, precision)),
+			)
+		)
+
 	def add_completion_time_logs(self, kwargs):
 		if kwargs.end_time:
 			self.add_time_logs(

erpnext/manufacturing/doctype/job_card/test_job_card.py

@@ -2228,6 +2228,21 @@ class TestJobCardLogic(ERPNextTestSuite):
 			frappe.ValidationError, jc.validate_complete_job_card_qty, frappe._dict(pending_qty=10)
 		)
 
+	def test_completion_qty_split_must_add_up(self):
+		jc = frappe.new_doc("Job Card")
+		jc.for_quantity = 5
+
+		# 3 completed + 2 pending + 0 lost == 5 to manufacture -> passes
+		jc.validate_complete_job_card_qty(
+			frappe._dict(for_quantity=5, qty=3, pending_qty=2, process_loss_qty=0)
+		)
+
+		self.assertRaises(
+			frappe.ValidationError,
+			jc.validate_complete_job_card_qty,
+			frappe._dict(for_quantity=3, qty=3, pending_qty=2, process_loss_qty=0),
+		)
+
 	def test_completed_qty_must_reconcile_with_for_quantity(self):
 		jc = frappe.new_doc("Job Card")
 		jc.for_quantity = 10

erpnext/public/js/shop_floor/shop_floor.js

@@ -796,6 +796,7 @@ class ShopFloor {
 				change() {
 					const d = me.session_dialog;
 					d.set_value("completed_qty", d.get_value("for_quantity"));
+					d.set_value("pending_qty", 0);
 					d.set_value("process_loss_qty", 0);
 				},
 			},
@@ -807,8 +808,21 @@ class ShopFloor {
 				default: pending,
 				change() {
 					const d = me.session_dialog;
-					const remaining = flt(d.get_value("for_quantity")) - flt(d.get_value("completed_qty"));
-					if (remaining > 0 && remaining !== flt(d.get_value("pending_qty"))) {
+					const remaining =
+						flt(d.get_value("for_quantity")) -
+						flt(d.get_value("completed_qty")) -
+						flt(d.get_value("process_loss_qty"));
+
+					if (remaining < 0) {
+						const max_completed_qty =
+							flt(d.get_value("for_quantity")) - flt(d.get_value("process_loss_qty"));
+						d.set_value("completed_qty", max_completed_qty);
+						frappe.throw(
+							__("Completed Quantity cannot be greater than {0}", [max_completed_qty])
+						);
+					}
+
+					if (remaining !== flt(d.get_value("pending_qty"))) {
 						d.set_value("pending_qty", remaining);
 					}
 				},
@@ -824,7 +838,17 @@ class ShopFloor {
 						flt(d.get_value("for_quantity")) -
 						flt(d.get_value("completed_qty")) -
 						flt(d.get_value("pending_qty"));
-					if (pl >= 0 && pl !== flt(d.get_value("process_loss_qty"))) {
+
+					if (pl < 0) {
+						d.set_value("pending_qty", 0);
+						frappe.throw(
+							__("Pending Quantity cannot be greater than {0}", [
+								flt(d.get_value("for_quantity")) - flt(d.get_value("completed_qty")),
+							])
+						);
+					}
+
+					if (pl !== flt(d.get_value("process_loss_qty"))) {
 						d.set_value("process_loss_qty", pl);
 					}
 				},
@@ -840,7 +864,17 @@ class ShopFloor {
 						flt(d.get_value("for_quantity")) -
 						flt(d.get_value("completed_qty")) -
 						flt(d.get_value("process_loss_qty"));
-					if (remaining >= 0 && remaining !== flt(d.get_value("pending_qty"))) {
+
+					if (remaining < 0) {
+						d.set_value("process_loss_qty", 0);
+						frappe.throw(
+							__("Process Loss Quantity cannot be greater than {0}", [
+								flt(d.get_value("for_quantity")) - flt(d.get_value("completed_qty")),
+							])
+						);
+					}
+
+					if (remaining !== flt(d.get_value("pending_qty"))) {
 						d.set_value("pending_qty", remaining);
 					}
 				},