mirror of
https://github.com/frappe/erpnext.git
synced 2026-08-13 06:31:48 +00:00
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
(cherry picked from commit 7bffd84482)
# Conflicts:
# erpnext/manufacturing/doctype/job_card/job_card.py
# erpnext/manufacturing/doctype/job_card/test_job_card.py
# erpnext/public/js/shop_floor/shop_floor.js
This commit is contained in:
@@ -250,6 +250,7 @@ frappe.ui.form.on("Job Card", {
|
|||||||
change() {
|
change() {
|
||||||
const dialog = frm.job_completion_dialog;
|
const dialog = frm.job_completion_dialog;
|
||||||
dialog.set_value("completed_qty", dialog.get_value("for_quantity"));
|
dialog.set_value("completed_qty", dialog.get_value("for_quantity"));
|
||||||
|
dialog.set_value("pending_qty", 0);
|
||||||
dialog.set_value("process_loss_qty", 0);
|
dialog.set_value("process_loss_qty", 0);
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
@@ -261,8 +262,21 @@ frappe.ui.form.on("Job Card", {
|
|||||||
default: pending_qty,
|
default: pending_qty,
|
||||||
change() {
|
change() {
|
||||||
const dialog = frm.job_completion_dialog;
|
const dialog = frm.job_completion_dialog;
|
||||||
const remaining = dialog.get_value("for_quantity") - dialog.get_value("completed_qty");
|
const remaining =
|
||||||
if (remaining > 0 && remaining != dialog.get_value("pending_qty")) {
|
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);
|
dialog.set_value("pending_qty", remaining);
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
@@ -278,7 +292,18 @@ frappe.ui.form.on("Job Card", {
|
|||||||
dialog.get_value("for_quantity") -
|
dialog.get_value("for_quantity") -
|
||||||
dialog.get_value("completed_qty") -
|
dialog.get_value("completed_qty") -
|
||||||
dialog.get_value("pending_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);
|
dialog.set_value("process_loss_qty", process_loss_qty);
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
@@ -293,7 +318,18 @@ frappe.ui.form.on("Job Card", {
|
|||||||
dialog.get_value("for_quantity") -
|
dialog.get_value("for_quantity") -
|
||||||
dialog.get_value("completed_qty") -
|
dialog.get_value("completed_qty") -
|
||||||
dialog.get_value("process_loss_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);
|
dialog.set_value("pending_qty", remaining);
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
|
|||||||
@@ -895,12 +895,13 @@ class JobCard(Document):
|
|||||||
)
|
)
|
||||||
|
|
||||||
precision = self.precision("total_completed_qty")
|
precision = self.precision("total_completed_qty")
|
||||||
total_completed_qty = flt(
|
accounted_qty = flt(
|
||||||
flt(self.total_completed_qty, precision)
|
flt(self.total_completed_qty, precision)
|
||||||
+ flt(self.process_loss_qty, precision)
|
+ flt(self.process_loss_qty, precision)
|
||||||
+ flt(self.pending_qty, precision)
|
+ flt(self.pending_qty, precision)
|
||||||
)
|
)
|
||||||
|
|
||||||
|
<<<<<<< HEAD
|
||||||
if self.for_quantity and flt(total_completed_qty, precision) != flt(self.for_quantity, precision):
|
if self.for_quantity and flt(total_completed_qty, precision) != flt(self.for_quantity, precision):
|
||||||
total_completed_qty_label = bold(_("Total Completed Qty"))
|
total_completed_qty_label = bold(_("Total Completed Qty"))
|
||||||
qty_to_manufacture = bold(_("Qty to Manufacture"))
|
qty_to_manufacture = bold(_("Qty to Manufacture"))
|
||||||
@@ -911,6 +912,17 @@ class JobCard(Document):
|
|||||||
bold(flt(total_completed_qty, precision)),
|
bold(flt(total_completed_qty, precision)),
|
||||||
qty_to_manufacture,
|
qty_to_manufacture,
|
||||||
bold(self.for_quantity),
|
bold(self.for_quantity),
|
||||||
|
=======
|
||||||
|
if self.for_quantity and flt(accounted_qty, precision) != flt(self.for_quantity, precision):
|
||||||
|
frappe.throw(
|
||||||
|
_(
|
||||||
|
"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)),
|
||||||
|
>>>>>>> 7bffd84482 (fix(job_card): reject a completion split that cannot add up (#57687))
|
||||||
)
|
)
|
||||||
)
|
)
|
||||||
|
|
||||||
@@ -1515,6 +1527,7 @@ class JobCard(Document):
|
|||||||
kwargs = frappe._dict(kwargs)
|
kwargs = frappe._dict(kwargs)
|
||||||
|
|
||||||
self.validate_complete_job_card_qty(kwargs)
|
self.validate_complete_job_card_qty(kwargs)
|
||||||
|
self.set_for_quantity(kwargs)
|
||||||
|
|
||||||
def validate_docstatus(self):
|
def validate_docstatus(self):
|
||||||
if self.docstatus == 2:
|
if self.docstatus == 2:
|
||||||
@@ -1533,9 +1546,36 @@ class JobCard(Document):
|
|||||||
if flt(kwargs.pending_qty) and flt(kwargs.pending_qty) > self.for_quantity:
|
if flt(kwargs.pending_qty) and flt(kwargs.pending_qty) > self.for_quantity:
|
||||||
frappe.throw(_("Pending quantity cannot be greater than the for quantity."))
|
frappe.throw(_("Pending quantity cannot be greater than the for quantity."))
|
||||||
|
|
||||||
|
<<<<<<< HEAD
|
||||||
self.pending_qty = flt(kwargs.pending_qty)
|
self.pending_qty = flt(kwargs.pending_qty)
|
||||||
self.process_loss_qty = flt(kwargs.process_loss_qty)
|
self.process_loss_qty = flt(kwargs.process_loss_qty)
|
||||||
|
|
||||||
|
=======
|
||||||
|
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):
|
||||||
|
>>>>>>> 7bffd84482 (fix(job_card): reject a completion split that cannot add up (#57687))
|
||||||
if kwargs.end_time:
|
if kwargs.end_time:
|
||||||
self.add_time_logs(
|
self.add_time_logs(
|
||||||
to_time=kwargs.end_time,
|
to_time=kwargs.end_time,
|
||||||
|
|||||||
@@ -1879,3 +1879,94 @@ def create_semi_fg_bom(semi_fg_item, raw_item, inspection_required):
|
|||||||
bom.append("items", {"item_code": raw_item, "qty": 1})
|
bom.append("items", {"item_code": raw_item, "qty": 1})
|
||||||
bom.submit()
|
bom.submit()
|
||||||
return bom.name
|
return bom.name
|
||||||
|
<<<<<<< HEAD
|
||||||
|
=======
|
||||||
|
|
||||||
|
|
||||||
|
class TestJobCardLogic(ERPNextTestSuite):
|
||||||
|
"""Field-level validations and pure quantity/capacity helpers, exercised on the
|
||||||
|
document directly so they don't need a Work Order / BOM (the integration suite does)."""
|
||||||
|
|
||||||
|
def test_processing_a_submitted_or_cancelled_card_is_blocked(self):
|
||||||
|
submitted = frappe.new_doc("Job Card")
|
||||||
|
submitted.docstatus = 1
|
||||||
|
self.assertRaises(frappe.ValidationError, submitted.validate_docstatus)
|
||||||
|
|
||||||
|
cancelled = frappe.new_doc("Job Card")
|
||||||
|
cancelled.docstatus = 2
|
||||||
|
self.assertRaises(frappe.ValidationError, cancelled.validate_docstatus)
|
||||||
|
|
||||||
|
def test_complete_job_card_qty_guards(self):
|
||||||
|
jc = frappe.new_doc("Job Card")
|
||||||
|
jc.for_quantity = 5
|
||||||
|
jc.validate_complete_job_card_qty(frappe._dict(pending_qty=3)) # within range -> passes
|
||||||
|
self.assertRaises(
|
||||||
|
frappe.ValidationError, jc.validate_complete_job_card_qty, frappe._dict(pending_qty=-1)
|
||||||
|
)
|
||||||
|
self.assertRaises(
|
||||||
|
frappe.ValidationError, jc.validate_complete_job_card_qty, frappe._dict(process_loss_qty=-1)
|
||||||
|
)
|
||||||
|
self.assertRaises(
|
||||||
|
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
|
||||||
|
jc.total_completed_qty = 6
|
||||||
|
jc.process_loss_qty = 0
|
||||||
|
jc.pending_qty = 0
|
||||||
|
# 6 + 0 + 0 != 10 -> throws
|
||||||
|
self.assertRaises(frappe.ValidationError, jc.validate_completed_qty_matches_for_quantity)
|
||||||
|
# completed + loss + pending == for_quantity -> passes
|
||||||
|
jc.pending_qty = 4
|
||||||
|
jc.validate_completed_qty_matches_for_quantity()
|
||||||
|
|
||||||
|
def test_set_process_loss(self):
|
||||||
|
jc = frappe.new_doc("Job Card")
|
||||||
|
jc.for_quantity = 10
|
||||||
|
jc.total_completed_qty = 6
|
||||||
|
jc.pending_qty = 1
|
||||||
|
jc.set_process_loss()
|
||||||
|
self.assertEqual(jc.process_loss_qty, 3) # 10 - 6 - 1
|
||||||
|
|
||||||
|
# no loss when nothing completed yet
|
||||||
|
nothing_done = frappe.new_doc("Job Card")
|
||||||
|
nothing_done.for_quantity = 10
|
||||||
|
nothing_done.total_completed_qty = 0
|
||||||
|
nothing_done.set_process_loss()
|
||||||
|
self.assertEqual(nothing_done.process_loss_qty, 0)
|
||||||
|
|
||||||
|
def test_capacity_overlap_detection(self):
|
||||||
|
jc = frappe.new_doc("Job Card")
|
||||||
|
sequential = [
|
||||||
|
{"from_time": "2026-01-01 10:00:00", "to_time": "2026-01-01 11:00:00"},
|
||||||
|
{"from_time": "2026-01-01 11:00:00", "to_time": "2026-01-01 12:00:00"},
|
||||||
|
]
|
||||||
|
overlapping = [
|
||||||
|
{"from_time": "2026-01-01 10:00:00", "to_time": "2026-01-01 11:00:00"},
|
||||||
|
{"from_time": "2026-01-01 10:30:00", "to_time": "2026-01-01 11:30:00"},
|
||||||
|
]
|
||||||
|
# sequential logs share one capacity slot; overlapping logs need two
|
||||||
|
self.assertEqual(len(jc.get_alloted_capacity(sequential)), 1)
|
||||||
|
self.assertEqual(len(jc.get_alloted_capacity(overlapping)), 2)
|
||||||
|
# capacity 1 overlaps with any log; capacity 2 only when both slots are taken
|
||||||
|
self.assertTrue(jc.has_overlap(1, sequential))
|
||||||
|
self.assertFalse(jc.has_overlap(2, sequential))
|
||||||
|
self.assertTrue(jc.has_overlap(2, overlapping))
|
||||||
|
>>>>>>> 7bffd84482 (fix(job_card): reject a completion split that cannot add up (#57687))
|
||||||
|
|||||||
1747
erpnext/public/js/shop_floor/shop_floor.js
Normal file
1747
erpnext/public/js/shop_floor/shop_floor.js
Normal file
File diff suppressed because it is too large
Load Diff
Reference in New Issue
Block a user