From 7bffd844828475d60562161d7e91640a13501d7c Mon Sep 17 00:00:00 2001 From: Mihir Kandoi Date: Sat, 1 Aug 2026 18:34:30 +0530 Subject: [PATCH] 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 --- .../doctype/job_card/job_card.js | 44 +++++++++++++++++-- .../doctype/job_card/job_card.py | 41 +++++++++++++---- .../doctype/job_card/test_job_card.py | 15 +++++++ erpnext/public/js/shop_floor/shop_floor.js | 42 ++++++++++++++++-- 4 files changed, 126 insertions(+), 16 deletions(-) diff --git a/erpnext/manufacturing/doctype/job_card/job_card.js b/erpnext/manufacturing/doctype/job_card/job_card.js index 74ec603da34..dea30133658 100644 --- a/erpnext/manufacturing/doctype/job_card/job_card.js +++ b/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); } }, diff --git a/erpnext/manufacturing/doctype/job_card/job_card.py b/erpnext/manufacturing/doctype/job_card/job_card.py index d792a7f6ff5..3f17822ebe5 100644 --- a/erpnext/manufacturing/doctype/job_card/job_card.py +++ b/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( diff --git a/erpnext/manufacturing/doctype/job_card/test_job_card.py b/erpnext/manufacturing/doctype/job_card/test_job_card.py index d38f55ff653..8762a6a3d13 100644 --- a/erpnext/manufacturing/doctype/job_card/test_job_card.py +++ b/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 diff --git a/erpnext/public/js/shop_floor/shop_floor.js b/erpnext/public/js/shop_floor/shop_floor.js index dc14f6d33da..6e57b77ed7a 100644 --- a/erpnext/public/js/shop_floor/shop_floor.js +++ b/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); } },