From 0673f558c1a9e5e25d31d5c48298269e0e6c3afa Mon Sep 17 00:00:00 2001 From: marination Date: Wed, 31 Mar 2021 01:38:22 +0530 Subject: [PATCH] fix: Cleaned up and fixed validation and bin updation on deletion - Created separate smaller functions for validation and bin updation of deleted row - Made sure previous doc (linked doc) was also updated after deletion of row - Tests to check bin updation on add/update/delete - Made reserved qty for subcontrating read only in bin --- .../doctype/purchase_order/purchase_order.py | 1 + .../purchase_order/test_purchase_order.py | 69 +++++++++++++++-- erpnext/controllers/accounts_controller.py | 76 ++++++++++++------- .../doctype/sales_order/sales_order.py | 2 +- .../doctype/sales_order/test_sales_order.py | 26 +++++++ erpnext/stock/doctype/bin/bin.json | 8 +- 6 files changed, 144 insertions(+), 38 deletions(-) diff --git a/erpnext/buying/doctype/purchase_order/purchase_order.py b/erpnext/buying/doctype/purchase_order/purchase_order.py index d32e98e8d9..29a8d59cb0 100644 --- a/erpnext/buying/doctype/purchase_order/purchase_order.py +++ b/erpnext/buying/doctype/purchase_order/purchase_order.py @@ -252,6 +252,7 @@ class PurchaseOrder(BuyingController): self.update_prevdoc_status() # Must be called after updating ordered qty in Material Request + # bin uses Material Request Items to recalculate & update self.update_requested_qty() self.update_ordered_qty() diff --git a/erpnext/buying/doctype/purchase_order/test_purchase_order.py b/erpnext/buying/doctype/purchase_order/test_purchase_order.py index 604c88682f..3c4f908ee4 100644 --- a/erpnext/buying/doctype/purchase_order/test_purchase_order.py +++ b/erpnext/buying/doctype/purchase_order/test_purchase_order.py @@ -90,6 +90,50 @@ class TestPurchaseOrder(unittest.TestCase): frappe.db.set_value('Item', '_Test Item', 'over_billing_allowance', 0) frappe.db.set_value("Accounts Settings", None, "over_billing_allowance", 0) + def test_update_remove_child_linked_to_mr(self): + """Test impact on linked PO and MR on deleting/updating row.""" + mr = make_material_request(qty=10) + po = make_purchase_order(mr.name) + po.supplier = "_Test Supplier" + po.save() + po.submit() + + first_item_of_po = po.get("items")[0] + existing_ordered_qty = get_ordered_qty() # 10 + existing_requested_qty = get_requested_qty() # 0 + + # decrease ordered qty by 3 (10 -> 7) and add item + trans_item = json.dumps([ + { + 'item_code': first_item_of_po.item_code, + 'rate': first_item_of_po.rate, + 'qty': 7, + 'docname': first_item_of_po.name + }, + {'item_code' : '_Test Item 2', 'rate' : 200, 'qty' : 2} + ]) + update_child_qty_rate('Purchase Order', trans_item, po.name) + mr.reload() + + # requested qty increases as ordered qty decreases + self.assertEqual(get_requested_qty(), existing_requested_qty + 3) # 3 + self.assertEqual(mr.items[0].ordered_qty, 7) + + self.assertEqual(get_ordered_qty(), existing_ordered_qty - 3) # 7 + + # delete first item linked to Material Request + trans_item = json.dumps([ + {'item_code' : '_Test Item 2', 'rate' : 200, 'qty' : 2} + ]) + update_child_qty_rate('Purchase Order', trans_item, po.name) + mr.reload() + + # requested qty increases as ordered qty is 0 (deleted row) + self.assertEqual(get_requested_qty(), existing_requested_qty + 10) # 10 + self.assertEqual(mr.items[0].ordered_qty, 0) + + # ordered qty decreases as ordered qty is 0 (deleted row) + self.assertEqual(get_ordered_qty(), existing_ordered_qty - 10) # 0 def test_update_child(self): mr = make_material_request(qty=10) @@ -120,7 +164,6 @@ class TestPurchaseOrder(unittest.TestCase): self.assertEqual(po.get("items")[0].amount, 1400) self.assertEqual(get_ordered_qty(), existing_ordered_qty + 3) - def test_update_child_adding_new_item(self): po = create_purchase_order(do_not_save=1) po.items[0].qty = 4 @@ -129,6 +172,7 @@ class TestPurchaseOrder(unittest.TestCase): pr = make_pr_against_po(po.name, 2) po.load_from_db() + existing_ordered_qty = get_ordered_qty() first_item_of_po = po.get("items")[0] trans_item = json.dumps([ @@ -145,7 +189,8 @@ class TestPurchaseOrder(unittest.TestCase): po.reload() self.assertEquals(len(po.get('items')), 2) self.assertEqual(po.status, 'To Receive and Bill') - + # ordered qty should increase on row addition + self.assertEqual(get_ordered_qty(), existing_ordered_qty + 7) def test_update_child_removing_item(self): po = create_purchase_order(do_not_save=1) @@ -156,6 +201,7 @@ class TestPurchaseOrder(unittest.TestCase): po.reload() first_item_of_po = po.get("items")[0] + existing_ordered_qty = get_ordered_qty() # add an item trans_item = json.dumps([ { @@ -168,6 +214,10 @@ class TestPurchaseOrder(unittest.TestCase): update_child_qty_rate('Purchase Order', trans_item, po.name) po.reload() + + # ordered qty should increase on row addition + self.assertEqual(get_ordered_qty(), existing_ordered_qty + 7) + # check if can remove received item trans_item = json.dumps([{'item_code' : '_Test Item', 'rate' : 200, 'qty' : 7, 'docname': po.get("items")[1].name}]) self.assertRaises(frappe.ValidationError, update_child_qty_rate, 'Purchase Order', trans_item, po.name) @@ -187,6 +237,9 @@ class TestPurchaseOrder(unittest.TestCase): self.assertEquals(len(po.get('items')), 1) self.assertEqual(po.status, 'To Receive and Bill') + # ordered qty should decrease (back to initial) on row deletion + self.assertEqual(get_ordered_qty(), existing_ordered_qty) + def test_update_child_perm(self): po = create_purchase_order(item_code= "_Test Item", qty=4) @@ -230,11 +283,13 @@ class TestPurchaseOrder(unittest.TestCase): new_item_with_tax = frappe.get_doc("Item", "Test Item with Tax") - new_item_with_tax.append("taxes", { - "item_tax_template": "Test Update Items Template - _TC", - "valid_from": nowdate() - }) - new_item_with_tax.save() + if not frappe.db.exists("Item Tax", + {"item_tax_template": "Test Update Items Template - _TC", "parent": "Test Item with Tax"}): + new_item_with_tax.append("taxes", { + "item_tax_template": "Test Update Items Template - _TC", + "valid_from": nowdate() + }) + new_item_with_tax.save() tax_template = "_Test Account Excise Duty @ 10 - _TC" item = "_Test Item Home Desktop 100" diff --git a/erpnext/controllers/accounts_controller.py b/erpnext/controllers/accounts_controller.py index 477ad6a79f..97242a2b69 100644 --- a/erpnext/controllers/accounts_controller.py +++ b/erpnext/controllers/accounts_controller.py @@ -1317,26 +1317,63 @@ def set_order_defaults(parent_doctype, parent_doctype_name, child_doctype, child p_doc = frappe.get_doc(parent_doctype, parent_doctype_name) child_item = frappe.new_doc(child_doctype, p_doc, child_docname) item = frappe.get_doc("Item", trans_item.get('item_code')) + for field in ("item_code", "item_name", "description", "item_group"): - child_item.update({field: item.get(field)}) + child_item.update({field: item.get(field)}) + date_fieldname = "delivery_date" if child_doctype == "Sales Order Item" else "schedule_date" child_item.update({date_fieldname: trans_item.get(date_fieldname) or p_doc.get(date_fieldname)}) + child_item.stock_uom = item.stock_uom child_item.uom = trans_item.get("uom") or item.stock_uom child_item.warehouse = get_item_warehouse(item, p_doc, overwrite_warehouse=True) conversion_factor = flt(get_conversion_factor(item.item_code, child_item.uom).get("conversion_factor")) child_item.conversion_factor = flt(trans_item.get('conversion_factor')) or conversion_factor + if child_doctype == "Purchase Order Item": - child_item.base_rate = 1 # Initiallize value will update in parent validation - child_item.base_amount = 1 # Initiallize value will update in parent validation + # Initialized value will update in parent validation + child_item.base_rate = 1 + child_item.base_amount = 1 if child_doctype == "Sales Order Item": child_item.warehouse = get_item_warehouse(item, p_doc, overwrite_warehouse=True) if not child_item.warehouse: frappe.throw(_("Cannot find {} for item {}. Please set the same in Item Master or Stock Settings.") .format(frappe.bold("default warehouse"), frappe.bold(item.item_code))) + set_child_tax_template_and_map(item, child_item, p_doc) add_taxes_from_tax_template(child_item, p_doc) return child_item +def validate_child_on_delete(row, parent): + """Check if partially transacted item (row) is being deleted.""" + if parent.doctype == "Sales Order": + if flt(row.delivered_qty): + frappe.throw(_("Row #{0}: Cannot delete item {1} which has already been delivered").format(row.idx, row.item_code)) + if flt(row.work_order_qty): + frappe.throw(_("Row #{0}: Cannot delete item {1} which has work order assigned to it.").format(row.idx, row.item_code)) + if flt(row.ordered_qty): + frappe.throw(_("Row #{0}: Cannot delete item {1} which is assigned to customer's purchase order.").format(row.idx, row.item_code)) + + if parent.doctype == "Purchase Order" and flt(row.received_qty): + frappe.throw(_("Row #{0}: Cannot delete item {1} which has already been received").format(row.idx, row.item_code)) + + if flt(row.billed_amt): + frappe.throw(_("Row #{0}: Cannot delete item {1} which has already been billed.").format(row.idx, row.item_code)) + +def update_bin_on_delete(row, doctype): + """Update bin for deleted item (row).""" + from erpnext.stock.stock_balance import update_bin_qty, get_reserved_qty, get_ordered_qty, get_indented_qty + qty_dict = {} + + if doctype == "Sales Order": + qty_dict["reserved_qty"] = get_reserved_qty(row.item_code, row.warehouse) + else: + if row.material_request_item: + qty_dict["indented_qty"] = get_indented_qty(row.item_code, row.warehouse) + + qty_dict["ordered_qty"] = get_ordered_qty(row.item_code, row.warehouse) + + update_bin_qty(row.item_code, row.warehouse, qty_dict) + def validate_and_delete_children(parent, data): deleted_children = [] updated_item_names = [d.get("docname") for d in data] @@ -1345,33 +1382,16 @@ def validate_and_delete_children(parent, data): deleted_children.append(item) for d in deleted_children: - if parent.doctype == "Sales Order": - if flt(d.delivered_qty): - frappe.throw(_("Row #{0}: Cannot delete item {1} which has already been delivered").format(d.idx, d.item_code)) - if flt(d.work_order_qty): - frappe.throw(_("Row #{0}: Cannot delete item {1} which has work order assigned to it.").format(d.idx, d.item_code)) - if flt(d.ordered_qty): - frappe.throw(_("Row #{0}: Cannot delete item {1} which is assigned to customer's purchase order.").format(d.idx, d.item_code)) - - if parent.doctype == "Purchase Order" and flt(d.received_qty): - frappe.throw(_("Row #{0}: Cannot delete item {1} which has already been received").format(d.idx, d.item_code)) - - if flt(d.billed_amt): - frappe.throw(_("Row #{0}: Cannot delete item {1} which has already been billed.").format(d.idx, d.item_code)) - + validate_child_on_delete(d, parent) d.cancel() d.delete() - - from erpnext.stock.stock_balance import update_bin_qty, get_reserved_qty, get_ordered_qty - # updating both will be time consuming, update it based on the doctype. reserved qty if sales order, otherwise ordered qty - if parent.doctype == "Sales Order": - update_bin_qty(d.item_code, d.warehouse, { - "reserved_qty": get_reserved_qty(d.item_code, d.warehouse) - }) - else: - update_bin_qty(d.item_code, d.warehouse, { - "ordered_qty": get_ordered_qty(d.item_code, d.warehouse) - }) + + # need to update ordered qty in Material Request first + # bin uses Material Request Items to recalculate & update + parent.update_prevdoc_status() + + for d in deleted_children: + update_bin_on_delete(d, parent.doctype) @frappe.whitelist() def update_child_qty_rate(parent_doctype, trans_items, parent_doctype_name, child_docname="items"): diff --git a/erpnext/selling/doctype/sales_order/sales_order.py b/erpnext/selling/doctype/sales_order/sales_order.py index e56129170c..fd9ddd8463 100755 --- a/erpnext/selling/doctype/sales_order/sales_order.py +++ b/erpnext/selling/doctype/sales_order/sales_order.py @@ -150,7 +150,7 @@ class SalesOrder(SellingController): if enq: frappe.db.sql("update `tabOpportunity` set status = %s where name=%s",(flag,enq[0][0])) - def update_prevdoc_status(self, flag): + def update_prevdoc_status(self, flag=None): for quotation in list(set([d.prevdoc_docname for d in self.get("items")])): if quotation: doc = frappe.get_doc("Quotation", quotation) diff --git a/erpnext/selling/doctype/sales_order/test_sales_order.py b/erpnext/selling/doctype/sales_order/test_sales_order.py index 0fdfb1b889..b06e775bd1 100644 --- a/erpnext/selling/doctype/sales_order/test_sales_order.py +++ b/erpnext/selling/doctype/sales_order/test_sales_order.py @@ -341,6 +341,9 @@ class TestSalesOrder(unittest.TestCase): prev_total = so.get("base_total") prev_total_in_words = so.get("base_in_words") + # get reserved qty before update items + reserved_qty_for_second_item = get_reserved_qty("_Test Item 2") + first_item_of_so = so.get("items")[0] trans_item = json.dumps([ {'item_code' : first_item_of_so.item_code, 'rate' : first_item_of_so.rate, \ @@ -354,6 +357,10 @@ class TestSalesOrder(unittest.TestCase): self.assertEqual(so.get("items")[-1].rate, 200) self.assertEqual(so.get("items")[-1].qty, 7) self.assertEqual(so.get("items")[-1].amount, 1400) + + # reserved qty should increase after adding row + self.assertEqual(get_reserved_qty('_Test Item 2'), reserved_qty_for_second_item + 7) + self.assertEqual(so.status, 'To Deliver and Bill') updated_total = so.get("base_total") @@ -373,6 +380,9 @@ class TestSalesOrder(unittest.TestCase): create_dn_against_so(so.name, 2) make_sales_invoice(so.name) + # get reserved qty before update items + reserved_qty_for_second_item = get_reserved_qty("_Test Item 2") + # add an item so as to try removing items trans_item = json.dumps([ {"item_code": '_Test Item', "qty": 5, "rate":1000, "docname": so.get("items")[0].name}, @@ -382,6 +392,9 @@ class TestSalesOrder(unittest.TestCase): so.reload() self.assertEqual(len(so.get("items")), 2) + # reserved qty should increase after adding row + self.assertEqual(get_reserved_qty('_Test Item 2'), reserved_qty_for_second_item + 2) + # check if delivered items can be removed trans_item = json.dumps([{ "item_code": '_Test Item 2', @@ -402,6 +415,10 @@ class TestSalesOrder(unittest.TestCase): so.reload() self.assertEqual(len(so.get("items")), 1) + + # reserved qty should decrease (back to initial) after deleting row + self.assertEqual(get_reserved_qty('_Test Item 2'), reserved_qty_for_second_item) + self.assertEqual(so.status, 'To Deliver and Bill') @@ -503,12 +520,18 @@ class TestSalesOrder(unittest.TestCase): so = make_sales_order(item_code = "_Test Item", warehouse=None) + # get reserved qty of packed item + existing_reserved_qty = get_reserved_qty("_Packed Item") + added_item = json.dumps([{"item_code" : "_Product Bundle Item", "rate" : 200, 'qty' : 2}]) update_child_qty_rate('Sales Order', added_item, so.name) so.reload() self.assertEqual(so.packed_items[0].qty, 4) + # reserved qty in packed item should increase after adding bundle item + self.assertEqual(get_reserved_qty("_Packed Item"), existing_reserved_qty + 4) + # test uom and conversion factor change update_uom_conv_factor = json.dumps([{ 'item_code': so.get("items")[0].item_code, @@ -523,6 +546,9 @@ class TestSalesOrder(unittest.TestCase): so.reload() self.assertEqual(so.packed_items[0].qty, 8) + # reserved qty in packed item should increase after changing bundle item uom + self.assertEqual(get_reserved_qty("_Packed Item"), existing_reserved_qty + 8) + def test_update_child_with_tax_template(self): """ Test Action: Create a SO with one item having its tax account head already in the SO. diff --git a/erpnext/stock/doctype/bin/bin.json b/erpnext/stock/doctype/bin/bin.json index 04d624ec0b..8e79f0e555 100644 --- a/erpnext/stock/doctype/bin/bin.json +++ b/erpnext/stock/doctype/bin/bin.json @@ -1,4 +1,5 @@ { + "actions": [], "autoname": "MAT-BIN-.YYYY.-.#####", "creation": "2013-01-10 16:34:25", "doctype": "DocType", @@ -112,7 +113,8 @@ { "fieldname": "reserved_qty_for_sub_contract", "fieldtype": "Float", - "label": "Reserved Qty for sub contract" + "label": "Reserved Qty for sub contract", + "read_only": 1 }, { "fieldname": "ma_rate", @@ -166,7 +168,8 @@ "hide_toolbar": 1, "idx": 1, "in_create": 1, - "modified": "2019-11-18 18:34:59.456882", + "links": [], + "modified": "2021-03-30 23:09:39.572776", "modified_by": "Administrator", "module": "Stock", "name": "Bin", @@ -196,5 +199,6 @@ ], "quick_entry": 1, "search_fields": "item_code,warehouse", + "sort_field": "modified", "sort_order": "ASC" } \ No newline at end of file