From e07ce6efe0afde1bdbade6cbed9f53ac0dd236f0 Mon Sep 17 00:00:00 2001 From: marination Date: Tue, 17 May 2022 17:39:45 +0530 Subject: [PATCH 1/2] fix: Job Card excess transfer behaviour - Block excess transfer of items if not allowed in settings - Behaviour made consistent with js behaviour (button disappears if not pending and not allowed in settings) - Test for same case --- .../doctype/job_card/job_card.py | 51 +++++++++++++++---- .../doctype/job_card/test_job_card.py | 33 +++++++++++- .../stock/doctype/stock_entry/stock_entry.py | 2 +- 3 files changed, 73 insertions(+), 13 deletions(-) diff --git a/erpnext/manufacturing/doctype/job_card/job_card.py b/erpnext/manufacturing/doctype/job_card/job_card.py index a98fc948683d..776f2d0b4163 100644 --- a/erpnext/manufacturing/doctype/job_card/job_card.py +++ b/erpnext/manufacturing/doctype/job_card/job_card.py @@ -42,6 +42,10 @@ class JobCardCancelError(frappe.ValidationError): pass +class JobCardOverTransferError(frappe.ValidationError): + pass + + class JobCard(Document): def onload(self): excess_transfer = frappe.db.get_single_value( @@ -522,23 +526,50 @@ def get_current_operation_data(self): }, ) - def set_transferred_qty_in_job_card(self, ste_doc): + def set_transferred_qty_in_job_card_item(self, ste_doc): + from frappe.query_builder.functions import Sum + + def _validate_over_transfer(row, transferred_qty): + "Block over transfer of items if not allowed in settings." + allow_excess = frappe.db.get_single_value("Manufacturing Settings", "job_card_excess_transfer") + required_qty = frappe.db.get_value("Job Card Item", row.job_card_item, "required_qty") + is_excess = flt(transferred_qty) > flt(required_qty) + + if is_excess and not allow_excess: + frappe.throw( + _( + "Row #{0}: Cannot transfer more than Required Qty {1} for Item {2} against Job Card {3}" + ).format( + row.idx, frappe.bold(required_qty), frappe.bold(row.item_code), ste_doc.job_card + ), + title=_("Excess Transfer"), + exc=JobCardOverTransferError, + ) + for row in ste_doc.items: if not row.job_card_item: continue - qty = frappe.db.sql( - """ SELECT SUM(qty) from `tabStock Entry Detail` sed, `tabStock Entry` se - WHERE sed.job_card_item = %s and se.docstatus = 1 and sed.parent = se.name and - se.purpose = 'Material Transfer for Manufacture' - """, - (row.job_card_item), - )[0][0] + sed = frappe.qb.DocType("Stock Entry Detail") + se = frappe.qb.DocType("Stock Entry") + transferred_qty = ( + frappe.qb.from_(sed) + .join(se) + .on(sed.parent == se.name) + .select(Sum(sed.qty)) + .where( + (sed.job_card_item == row.job_card_item) + & (se.docstatus == 1) + & (se.purpose == "Material Transfer for Manufacture") + ) + ).run()[0][0] + + _validate_over_transfer(row, transferred_qty) - frappe.db.set_value("Job Card Item", row.job_card_item, "transferred_qty", flt(qty)) + frappe.db.set_value("Job Card Item", row.job_card_item, "transferred_qty", flt(transferred_qty)) def set_transferred_qty(self, update_status=False): - "Set total FG Qty for which RM was transferred." + "Set total FG Qty in Job Card for which RM was transferred." if not self.items: self.transferred_qty = self.for_quantity if self.docstatus == 1 else 0 diff --git a/erpnext/manufacturing/doctype/job_card/test_job_card.py b/erpnext/manufacturing/doctype/job_card/test_job_card.py index 4647ddf05f7c..d21e542f9088 100644 --- a/erpnext/manufacturing/doctype/job_card/test_job_card.py +++ b/erpnext/manufacturing/doctype/job_card/test_job_card.py @@ -2,10 +2,14 @@ # See license.txt import frappe -from frappe.tests.utils import FrappeTestCase +from frappe.tests.utils import FrappeTestCase, change_settings from frappe.utils import random_string -from erpnext.manufacturing.doctype.job_card.job_card import OperationMismatchError, OverlapError +from erpnext.manufacturing.doctype.job_card.job_card import ( + JobCardOverTransferError, + OperationMismatchError, + OverlapError, +) from erpnext.manufacturing.doctype.job_card.job_card import ( make_stock_entry as make_stock_entry_from_jc, ) @@ -25,6 +29,7 @@ def setUp(self): "test_job_card_multiple_materials_transfer", "test_job_card_excess_material_transfer", "test_job_card_partial_material_transfer", + "test_job_card_excess_material_transfer_block", ) if self._testMethodName in tests_that_skip_setup: @@ -165,6 +170,7 @@ def test_job_card_multiple_materials_transfer(self): # transfer was made for 2 fg qty in first transfer Stock Entry self.assertEqual(transfer_entry_2.fg_completed_qty, 0) + @change_settings("Manufacturing Settings", {"job_card_excess_transfer": 1}) def test_job_card_excess_material_transfer(self): "Test transferring more than required RM against Job Card." make_stock_entry(item_code="_Test Item", target="Stores - _TC", qty=25, basic_rate=100) @@ -208,6 +214,29 @@ def test_job_card_excess_material_transfer(self): # JC is Completed with excess transfer self.assertEqual(job_card.status, "Completed") + @change_settings("Manufacturing Settings", {"job_card_excess_transfer": 0}) + def test_job_card_excess_material_transfer_block(self): + make_stock_entry(item_code="_Test Item", target="Stores - _TC", qty=25, basic_rate=100) + make_stock_entry( + item_code="_Test Item Home Desktop Manufactured", target="Stores - _TC", qty=15, basic_rate=100 + ) + + job_card_name = frappe.db.get_value("Job Card", {"work_order": self.work_order.name}) + + # fully transfer both RMs + transfer_entry_1 = make_stock_entry_from_jc(job_card_name) + transfer_entry_1.insert() + transfer_entry_1.submit() + + # transfer extra qty of both RM due to previously damaged RM + transfer_entry_2 = make_stock_entry_from_jc(job_card_name) + # deliberately change 'For Quantity' + transfer_entry_2.fg_completed_qty = 1 + transfer_entry_2.items[0].qty = 5 + transfer_entry_2.items[1].qty = 3 + transfer_entry_2.insert() + self.assertRaises(JobCardOverTransferError, transfer_entry_2.submit) + def test_job_card_partial_material_transfer(self): "Test partial material transfer against Job Card" diff --git a/erpnext/stock/doctype/stock_entry/stock_entry.py b/erpnext/stock/doctype/stock_entry/stock_entry.py index 890ac476a79a..26e866034ee6 100644 --- a/erpnext/stock/doctype/stock_entry/stock_entry.py +++ b/erpnext/stock/doctype/stock_entry/stock_entry.py @@ -1141,7 +1141,7 @@ def _validate_work_order(pro_doc): if self.job_card: job_doc = frappe.get_doc("Job Card", self.job_card) job_doc.set_transferred_qty(update_status=True) - job_doc.set_transferred_qty_in_job_card(self) + job_doc.set_transferred_qty_in_job_card_item(self) if self.work_order: pro_doc = frappe.get_doc("Work Order", self.work_order) From 9f6e10663b77489ba1f98ede96e30c23682c111a Mon Sep 17 00:00:00 2001 From: marination Date: Mon, 23 May 2022 11:05:55 +0530 Subject: [PATCH 2/2] chore: Run `_validate_over_transfer` only if excess transfer is blocked in settings --- erpnext/manufacturing/doctype/job_card/job_card.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/erpnext/manufacturing/doctype/job_card/job_card.py b/erpnext/manufacturing/doctype/job_card/job_card.py index d16281a5ba73..0a9fd8a09960 100644 --- a/erpnext/manufacturing/doctype/job_card/job_card.py +++ b/erpnext/manufacturing/doctype/job_card/job_card.py @@ -531,11 +531,9 @@ def set_transferred_qty_in_job_card_item(self, ste_doc): def _validate_over_transfer(row, transferred_qty): "Block over transfer of items if not allowed in settings." - allow_excess = frappe.db.get_single_value("Manufacturing Settings", "job_card_excess_transfer") required_qty = frappe.db.get_value("Job Card Item", row.job_card_item, "required_qty") is_excess = flt(transferred_qty) > flt(required_qty) - - if is_excess and not allow_excess: + if is_excess: frappe.throw( _( "Row #{0}: Cannot transfer more than Required Qty {1} for Item {2} against Job Card {3}" @@ -564,7 +562,9 @@ def _validate_over_transfer(row, transferred_qty): ) ).run()[0][0] - _validate_over_transfer(row, transferred_qty) + allow_excess = frappe.db.get_single_value("Manufacturing Settings", "job_card_excess_transfer") + if not allow_excess: + _validate_over_transfer(row, transferred_qty) frappe.db.set_value("Job Card Item", row.job_card_item, "transferred_qty", flt(transferred_qty))