Skip to content

Commit

Permalink
fix: dont update RM items table if not required (frappe#31408)
Browse files Browse the repository at this point in the history
Currently on PO update RM item table is auto computed again and again,
if there was any transfer/consumption against that then it will be lost.

This change:
1. Disables updating RM table if no change in qty of FG was made. Since
   RM table can't possibly be different with same FG qty.
2. Blocks update completely if qty is changed and RM items are already
   transferred.
  • Loading branch information
ankush committed Jun 27, 2022
1 parent 20dac08 commit dd11f26
Show file tree
Hide file tree
Showing 2 changed files with 73 additions and 4 deletions.
37 changes: 37 additions & 0 deletions erpnext/buying/doctype/purchase_order/test_purchase_order.py
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,43 @@ def test_update_remove_child_linked_to_mr(self):
# ordered qty decreases as ordered qty is 0 (deleted row)
self.assertEqual(get_ordered_qty(), existing_ordered_qty - 10) # 0

def test_supplied_items_validations_on_po_update_after_submit(self):
po = create_purchase_order(item_code="_Test FG Item", is_subcontracted=1, qty=5, rate=100)
item = po.items[0]

original_supplied_items = {po.name: po.required_qty for po in po.supplied_items}

# Just update rate
trans_item = [
{
"item_code": "_Test FG Item",
"rate": 20,
"qty": 5,
"conversion_factor": 1.0,
"docname": item.name,
}
]
update_child_qty_rate("Purchase Order", json.dumps(trans_item), po.name)
po.reload()

new_supplied_items = {po.name: po.required_qty for po in po.supplied_items}
self.assertEqual(set(original_supplied_items.keys()), set(new_supplied_items.keys()))

# Update qty to 2x
trans_item[0]["qty"] *= 2
update_child_qty_rate("Purchase Order", json.dumps(trans_item), po.name)
po.reload()

new_supplied_items = {po.name: po.required_qty for po in po.supplied_items}
self.assertEqual(2 * sum(original_supplied_items.values()), sum(new_supplied_items.values()))

# Set transfer qty and attempt to update qty, shouldn't be allowed
po.supplied_items[0].supplied_qty = 2
po.supplied_items[0].db_update()
trans_item[0]["qty"] *= 2
with self.assertRaises(frappe.ValidationError):
update_child_qty_rate("Purchase Order", json.dumps(trans_item), po.name)

def test_update_child(self):
mr = make_material_request(qty=10)
po = make_purchase_order(mr.name)
Expand Down
40 changes: 36 additions & 4 deletions erpnext/controllers/accounts_controller.py
Original file line number Diff line number Diff line change
Expand Up @@ -2440,7 +2440,7 @@ def update_bin_on_delete(row, doctype):
update_bin_qty(row.item_code, row.warehouse, qty_dict)


def validate_and_delete_children(parent, data):
def validate_and_delete_children(parent, data) -> bool:
deleted_children = []
updated_item_names = [d.get("docname") for d in data]
for item in parent.items:
Expand All @@ -2459,6 +2459,8 @@ def validate_and_delete_children(parent, data):
for d in deleted_children:
update_bin_on_delete(d, parent.doctype)

return bool(deleted_children)


@frappe.whitelist()
def update_child_qty_rate(parent_doctype, trans_items, parent_doctype_name, child_docname="items"):
Expand Down Expand Up @@ -2522,13 +2524,38 @@ def validate_quantity(child_item, new_data):
):
frappe.throw(_("Cannot set quantity less than received quantity"))

def should_update_supplied_items(doc) -> bool:
"""Subcontracted PO can allow following changes *after submit*:
1. Change rate of subcontracting - regardless of other changes.
2. Change qty and/or add new items and/or remove items
Exception: Transfer/Consumption is already made, qty change not allowed.
"""

supplied_items_processed = any(
item.supplied_qty or item.consumed_qty or item.returned_qty for item in doc.supplied_items
)

update_supplied_items = (
any_qty_changed or items_added_or_removed or any_conversion_factor_changed
)
if update_supplied_items and supplied_items_processed:
frappe.throw(_("Item qty can not be updated as raw materials are already processed."))

return update_supplied_items

data = json.loads(trans_items)

any_qty_changed = False # updated to true if any item's qty changes
items_added_or_removed = False # updated to true if any new item is added or removed
any_conversion_factor_changed = False

sales_doctypes = ["Sales Order", "Sales Invoice", "Delivery Note", "Quotation"]
parent = frappe.get_doc(parent_doctype, parent_doctype_name)

check_doc_permissions(parent, "write")
validate_and_delete_children(parent, data)
_removed_items = validate_and_delete_children(parent, data)
items_added_or_removed |= _removed_items

for d in data:
new_child_flag = False
Expand All @@ -2539,6 +2566,7 @@ def validate_quantity(child_item, new_data):

if not d.get("docname"):
new_child_flag = True
items_added_or_removed = True
check_doc_permissions(parent, "create")
child_item = get_new_child_item(d)
else:
Expand All @@ -2561,6 +2589,7 @@ def validate_quantity(child_item, new_data):
qty_unchanged = prev_qty == new_qty
uom_unchanged = prev_uom == new_uom
conversion_factor_unchanged = prev_con_fac == new_con_fac
any_conversion_factor_changed |= not conversion_factor_unchanged
date_unchanged = (
prev_date == getdate(new_date) if prev_date and new_date else False
) # in case of delivery note etc
Expand All @@ -2574,6 +2603,8 @@ def validate_quantity(child_item, new_data):
continue

validate_quantity(child_item, d)
if flt(child_item.get("qty")) != flt(d.get("qty")):
any_qty_changed = True

child_item.qty = flt(d.get("qty"))
rate_precision = child_item.precision("rate") or 2
Expand Down Expand Up @@ -2679,8 +2710,9 @@ def validate_quantity(child_item, new_data):
parent.update_ordered_and_reserved_qty()
parent.update_receiving_percentage()
if parent.is_subcontracted:
parent.update_reserved_qty_for_subcontract()
parent.create_raw_materials_supplied("supplied_items")
if should_update_supplied_items(parent):
parent.update_reserved_qty_for_subcontract()
parent.create_raw_materials_supplied("supplied_items")
parent.save()
else: # Sales Order
parent.validate_warehouse()
Expand Down

0 comments on commit dd11f26

Please sign in to comment.