From d71681a99e3ff94675db42e9915f92849ca04be8 Mon Sep 17 00:00:00 2001 From: Mihir Kandoi Date: Thu, 23 Jul 2026 12:37:29 +0530 Subject: [PATCH 1/5] refactor: drop unused operation_row_number field from Job Card --- .../doctype/job_card/job_card.js | 29 +------------------ .../doctype/job_card/job_card.json | 8 +---- .../doctype/job_card/job_card.py | 23 --------------- .../doctype/job_card/test_job_card.py | 14 --------- 4 files changed, 2 insertions(+), 72 deletions(-) diff --git a/erpnext/manufacturing/doctype/job_card/job_card.js b/erpnext/manufacturing/doctype/job_card/job_card.js index b6538d4d4d3..bea0bc914c6 100644 --- a/erpnext/manufacturing/doctype/job_card/job_card.js +++ b/erpnext/manufacturing/doctype/job_card/job_card.js @@ -172,8 +172,6 @@ frappe.ui.form.on("Job Card", { }, })); - frm.trigger("toggle_operation_number"); - const is_timer_running = frm.events.setup_job_action_buttons(frm, has_items); if (!is_timer_running) { @@ -498,8 +496,6 @@ frappe.ui.form.on("Job Card", { }, operation(frm) { - frm.trigger("toggle_operation_number"); - if (frm.doc.operation && frm.doc.work_order) { frappe.call({ method: "erpnext.manufacturing.doctype.job_card.job_card.get_operation_details", @@ -508,37 +504,14 @@ frappe.ui.form.on("Job Card", { operation: frm.doc.operation, }, callback(r) { - if (!r.message) return; - - if (r.message.length == 1) { + if (r.message && r.message.length == 1) { frm.set_value("operation_id", r.message[0].name); - } else { - const args = r.message.map((row) => ({ label: row.idx, value: row.name })); - const description = __("Operation {0} added multiple times in the work order {1}", [ - frm.doc.operation, - frm.doc.work_order, - ]); - frm.set_df_property("operation_row_number", "options", args); - frm.set_df_property("operation_row_number", "description", description); } - - frm.trigger("toggle_operation_number"); }, }); } }, - operation_row_number(frm) { - if (frm.doc.operation_row_number) { - frm.set_value("operation_id", frm.doc.operation_row_number); - } - }, - - toggle_operation_number(frm) { - frm.toggle_display("operation_row_number", !frm.doc.operation_id && frm.doc.operation); - frm.toggle_reqd("operation_row_number", !frm.doc.operation_id && frm.doc.operation); - }, - make_time_log(frm, args) { frm.events.update_sub_operation(frm, args); diff --git a/erpnext/manufacturing/doctype/job_card/job_card.json b/erpnext/manufacturing/doctype/job_card/job_card.json index 554e6f2395a..6da36cf6014 100644 --- a/erpnext/manufacturing/doctype/job_card/job_card.json +++ b/erpnext/manufacturing/doctype/job_card/job_card.json @@ -92,7 +92,6 @@ "operation_row_id", "amended_from", "column_break_xhzg", - "operation_row_number", "operation_id", "sequence_id", "section_break_jcmx", @@ -324,11 +323,6 @@ "label": "Item Name", "read_only": 1 }, - { - "fieldname": "operation_row_number", - "fieldtype": "Select", - "label": "Operation Row Number" - }, { "fieldname": "sequence_id", "fieldtype": "Int", @@ -695,7 +689,7 @@ "grid_page_length": 50, "is_submittable": 1, "links": [], - "modified": "2026-06-20 17:39:42.293242", + "modified": "2026-07-23 12:00:00.000000", "modified_by": "Administrator", "module": "Manufacturing", "name": "Job Card", diff --git a/erpnext/manufacturing/doctype/job_card/job_card.py b/erpnext/manufacturing/doctype/job_card/job_card.py index c796ff7877b..d69d8346f1d 100644 --- a/erpnext/manufacturing/doctype/job_card/job_card.py +++ b/erpnext/manufacturing/doctype/job_card/job_card.py @@ -45,10 +45,6 @@ class OverlapError(frappe.ValidationError): pass -class OperationMismatchError(frappe.ValidationError): - pass - - class OperationSequenceError(frappe.ValidationError): pass @@ -106,7 +102,6 @@ class JobCard(Document): operation: DF.Link operation_id: DF.Data | None operation_row_id: DF.Int - operation_row_number: DF.Literal[None] pending_qty: DF.Float posting_date: DF.Date | None process_loss_qty: DF.Float @@ -168,7 +163,6 @@ class JobCard(Document): self.validate_time_logs() self.validate_on_hold() self.set_status() - self.validate_operation_id() self.validate_sequence_id() self.set_sub_operations() self.update_sub_operation_status() @@ -1344,23 +1338,6 @@ class JobCard(Document): if not self.wip_warehouse: self.wip_warehouse = frappe.get_cached_value("Company", self.company, "default_wip_warehouse") - def validate_operation_id(self): - if ( - self.get("operation_id") - and self.get("operation_row_number") - and self.operation - and self.work_order - and frappe.get_cached_value("Work Order Operation", self.operation_row_number, "name") - != self.operation_id - ): - work_order = bold(get_link_to_form("Work Order", self.work_order)) - frappe.throw( - _("Operation {0} does not belong to the work order {1}").format( - bold(self.operation), work_order - ), - OperationMismatchError, - ) - @frappe.whitelist() def pause_job(self, **kwargs): frappe.has_permission("Job Card", "write", doc=self, throw=True) diff --git a/erpnext/manufacturing/doctype/job_card/test_job_card.py b/erpnext/manufacturing/doctype/job_card/test_job_card.py index 36c653abf7b..9acbe89c34e 100644 --- a/erpnext/manufacturing/doctype/job_card/test_job_card.py +++ b/erpnext/manufacturing/doctype/job_card/test_job_card.py @@ -10,7 +10,6 @@ from frappe.utils.data import add_to_date, now, today from erpnext.manufacturing.doctype.job_card.job_card import ( JobCardOverTransferError, - OperationMismatchError, OverlapError, ) from erpnext.manufacturing.doctype.job_card.mapper import ( @@ -149,19 +148,6 @@ class TestJobCard(ERPNextTestSuite): ) self.assertRaises(frappe.ValidationError, job_card_doc.submit) - def test_job_card_operations(self): - job_cards = frappe.get_all( - "Job Card", filters={"work_order": self.work_order.name}, fields=["operation_id", "name"] - ) - - if job_cards: - job_card = job_cards[0] - frappe.db.set_value("Job Card", job_card.name, "operation_row_number", job_card.operation_id) - - doc = frappe.get_doc("Job Card", job_card.name) - doc.operation_id = "Test Data" - self.assertRaises(OperationMismatchError, doc.save) - def test_job_card_with_different_work_station(self): job_cards = frappe.get_all( "Job Card", From 8cd36f60ab8dda4b854b38e7701d89924ede902f Mon Sep 17 00:00:00 2001 From: Mihir Kandoi Date: Thu, 23 Jul 2026 12:52:27 +0530 Subject: [PATCH 2/5] fix: prompt for operation row when operation repeats in work order --- .../doctype/job_card/job_card.js | 20 ++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/erpnext/manufacturing/doctype/job_card/job_card.js b/erpnext/manufacturing/doctype/job_card/job_card.js index bea0bc914c6..9a0194618ed 100644 --- a/erpnext/manufacturing/doctype/job_card/job_card.js +++ b/erpnext/manufacturing/doctype/job_card/job_card.js @@ -504,8 +504,26 @@ frappe.ui.form.on("Job Card", { operation: frm.doc.operation, }, callback(r) { - if (r.message && r.message.length == 1) { + if (!r.message || !r.message.length) return; + + if (r.message.length == 1) { frm.set_value("operation_id", r.message[0].name); + } else { + frappe.prompt( + { + fieldname: "operation_row", + fieldtype: "Select", + label: __("Operation Row"), + options: r.message.map((row) => ({ label: row.idx, value: row.name })), + reqd: 1, + description: __( + "Operation {0} is added multiple times in the work order {1}", + [frm.doc.operation, frm.doc.work_order] + ), + }, + (values) => frm.set_value("operation_id", values.operation_row), + __("Select Operation Row") + ); } }, }); From 50de87c9ea8ee0a44eb53d9e036cc0b0d71c8c79 Mon Sep 17 00:00:00 2001 From: Mihir Kandoi Date: Thu, 23 Jul 2026 13:00:10 +0530 Subject: [PATCH 3/5] fix: clear stale operation_id and guard async row selection --- .../doctype/job_card/job_card.js | 70 +++++++++++-------- 1 file changed, 39 insertions(+), 31 deletions(-) diff --git a/erpnext/manufacturing/doctype/job_card/job_card.js b/erpnext/manufacturing/doctype/job_card/job_card.js index 9a0194618ed..378fbb5c69e 100644 --- a/erpnext/manufacturing/doctype/job_card/job_card.js +++ b/erpnext/manufacturing/doctype/job_card/job_card.js @@ -496,38 +496,46 @@ frappe.ui.form.on("Job Card", { }, operation(frm) { - if (frm.doc.operation && frm.doc.work_order) { - frappe.call({ - method: "erpnext.manufacturing.doctype.job_card.job_card.get_operation_details", - args: { - work_order: frm.doc.work_order, - operation: frm.doc.operation, - }, - callback(r) { - if (!r.message || !r.message.length) return; - - if (r.message.length == 1) { - frm.set_value("operation_id", r.message[0].name); - } else { - frappe.prompt( - { - fieldname: "operation_row", - fieldtype: "Select", - label: __("Operation Row"), - options: r.message.map((row) => ({ label: row.idx, value: row.name })), - reqd: 1, - description: __( - "Operation {0} is added multiple times in the work order {1}", - [frm.doc.operation, frm.doc.work_order] - ), - }, - (values) => frm.set_value("operation_id", values.operation_row), - __("Select Operation Row") - ); - } - }, - }); + if (frm.doc.operation_id) { + frm.set_value("operation_id", ""); } + + if (!frm.doc.operation || !frm.doc.work_order) return; + + const { operation, work_order } = frm.doc; + const is_current = () => frm.doc.operation === operation && frm.doc.work_order === work_order; + + frappe.call({ + method: "erpnext.manufacturing.doctype.job_card.job_card.get_operation_details", + args: { work_order, operation }, + callback(r) { + if (!is_current() || !r.message || !r.message.length) return; + + if (r.message.length == 1) { + frm.set_value("operation_id", r.message[0].name); + } else { + frappe.prompt( + { + fieldname: "operation_row", + fieldtype: "Select", + label: __("Operation Row"), + options: r.message.map((row) => ({ label: row.idx, value: row.name })), + reqd: 1, + description: __("Operation {0} is added multiple times in the work order {1}", [ + operation, + work_order, + ]), + }, + (values) => { + if (is_current()) { + frm.set_value("operation_id", values.operation_row); + } + }, + __("Select Operation Row") + ); + } + }, + }); }, make_time_log(frm, args) { From 553336fa6fdb22176433c9ff22f4ba53f92babc6 Mon Sep 17 00:00:00 2001 From: Mihir Kandoi Date: Thu, 23 Jul 2026 13:07:51 +0530 Subject: [PATCH 4/5] fix: resolve or require operation_id server-side --- .../doctype/job_card/job_card.py | 20 ++++++++++++++ .../doctype/job_card/test_job_card.py | 27 +++++++++++++++++++ 2 files changed, 47 insertions(+) diff --git a/erpnext/manufacturing/doctype/job_card/job_card.py b/erpnext/manufacturing/doctype/job_card/job_card.py index d69d8346f1d..4437a0d89e7 100644 --- a/erpnext/manufacturing/doctype/job_card/job_card.py +++ b/erpnext/manufacturing/doctype/job_card/job_card.py @@ -163,6 +163,7 @@ class JobCard(Document): self.validate_time_logs() self.validate_on_hold() self.set_status() + self.set_operation_id() self.validate_sequence_id() self.set_sub_operations() self.update_sub_operation_status() @@ -1338,6 +1339,25 @@ class JobCard(Document): if not self.wip_warehouse: self.wip_warehouse = frappe.get_cached_value("Company", self.company, "default_wip_warehouse") + def set_operation_id(self): + if self.operation_id or not (self.work_order and self.operation): + return + + operation_rows = frappe.get_all( + "Work Order Operation", + filters={"parent": self.work_order, "operation": self.operation}, + pluck="name", + ) + + if len(operation_rows) == 1: + self.operation_id = operation_rows[0] + elif operation_rows and self.docstatus == 0: + frappe.throw( + _( + "Operation {0} is added multiple times in the work order {1}. Please select the operation row." + ).format(bold(self.operation), get_link_to_form("Work Order", self.work_order)) + ) + @frappe.whitelist() def pause_job(self, **kwargs): frappe.has_permission("Job Card", "write", doc=self, throw=True) diff --git a/erpnext/manufacturing/doctype/job_card/test_job_card.py b/erpnext/manufacturing/doctype/job_card/test_job_card.py index 9acbe89c34e..32f84c9943a 100644 --- a/erpnext/manufacturing/doctype/job_card/test_job_card.py +++ b/erpnext/manufacturing/doctype/job_card/test_job_card.py @@ -148,6 +148,33 @@ class TestJobCard(ERPNextTestSuite): ) self.assertRaises(frappe.ValidationError, job_card_doc.submit) + def test_set_operation_id(self): + work_order = make_wo_order_test_record(item="_Test FG Item 2", qty=2, do_not_submit=1) + operation_row = work_order.operations[0] + + job_card = frappe.new_doc("Job Card") + job_card.work_order = work_order.name + job_card.operation = operation_row.operation + job_card.set_operation_id() + self.assertEqual(job_card.operation_id, operation_row.name) + + work_order.append( + "operations", + { + "operation": operation_row.operation, + "workstation": operation_row.workstation, + "time_in_mins": operation_row.time_in_mins, + "hour_rate": operation_row.hour_rate, + "sequence_id": work_order.operations[-1].sequence_id, + }, + ) + work_order.save() + + job_card = frappe.new_doc("Job Card") + job_card.work_order = work_order.name + job_card.operation = operation_row.operation + self.assertRaises(frappe.ValidationError, job_card.set_operation_id) + def test_job_card_with_different_work_station(self): job_cards = frappe.get_all( "Job Card", From 9c8d9ac42b02ffdbfc20db151cca7803a1e36adb Mon Sep 17 00:00:00 2001 From: Mihir Kandoi Date: Thu, 23 Jul 2026 13:23:15 +0530 Subject: [PATCH 5/5] fix: validate supplied operation_id belongs to work order operation --- erpnext/manufacturing/doctype/job_card/job_card.py | 14 ++++++++++++-- .../doctype/job_card/test_job_card.py | 7 +++++++ 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/erpnext/manufacturing/doctype/job_card/job_card.py b/erpnext/manufacturing/doctype/job_card/job_card.py index 4437a0d89e7..084782ab12d 100644 --- a/erpnext/manufacturing/doctype/job_card/job_card.py +++ b/erpnext/manufacturing/doctype/job_card/job_card.py @@ -1340,7 +1340,10 @@ class JobCard(Document): self.wip_warehouse = frappe.get_cached_value("Company", self.company, "default_wip_warehouse") def set_operation_id(self): - if self.operation_id or not (self.work_order and self.operation): + if not (self.work_order and self.operation): + return + + if self.operation_id and self.docstatus != 0: return operation_rows = frappe.get_all( @@ -1349,7 +1352,14 @@ class JobCard(Document): pluck="name", ) - if len(operation_rows) == 1: + if self.operation_id: + if operation_rows and self.operation_id not in operation_rows: + frappe.throw( + _("Operation {0} does not belong to the work order {1}").format( + bold(self.operation), get_link_to_form("Work Order", self.work_order) + ) + ) + elif len(operation_rows) == 1: self.operation_id = operation_rows[0] elif operation_rows and self.docstatus == 0: frappe.throw( diff --git a/erpnext/manufacturing/doctype/job_card/test_job_card.py b/erpnext/manufacturing/doctype/job_card/test_job_card.py index 32f84c9943a..26260627c46 100644 --- a/erpnext/manufacturing/doctype/job_card/test_job_card.py +++ b/erpnext/manufacturing/doctype/job_card/test_job_card.py @@ -175,6 +175,13 @@ class TestJobCard(ERPNextTestSuite): job_card.operation = operation_row.operation self.assertRaises(frappe.ValidationError, job_card.set_operation_id) + job_card.operation_id = "bogus-row" + self.assertRaises(frappe.ValidationError, job_card.set_operation_id) + + job_card.operation_id = work_order.operations[-1].name + job_card.set_operation_id() + self.assertEqual(job_card.operation_id, work_order.operations[-1].name) + def test_job_card_with_different_work_station(self): job_cards = frappe.get_all( "Job Card",