From 4c677eafe958a448074b3efc859334c9a088be2c Mon Sep 17 00:00:00 2001 From: marination Date: Tue, 25 Jan 2022 23:52:52 +0530 Subject: [PATCH] fix: Linter and minor code refactor - Create an indexed map of stale packed items table to avoid loops to check if packed item row exists - Reset packed items if row deletion takes place - Renamed functions to self-explain them - Split long function - Reduce function calls inside function (makes it harder to follow through) --- erpnext/public/js/controllers/buying.js | 2 +- .../stock/doctype/packed_item/packed_item.py | 191 ++++++++++-------- 2 files changed, 103 insertions(+), 90 deletions(-) diff --git a/erpnext/public/js/controllers/buying.js b/erpnext/public/js/controllers/buying.js index d696ef55ae..54e5daa6bd 100644 --- a/erpnext/public/js/controllers/buying.js +++ b/erpnext/public/js/controllers/buying.js @@ -441,7 +441,7 @@ erpnext.buying.get_items_from_product_bundle = function(frm) { type: "GET", method: "erpnext.stock.doctype.packed_item.packed_item.get_items_from_product_bundle", args: { - args: { + row: { item_code: args.product_bundle, quantity: args.quantity, parenttype: frm.doc.doctype, diff --git a/erpnext/stock/doctype/packed_item/packed_item.py b/erpnext/stock/doctype/packed_item/packed_item.py index 3cdc134a1c..9c9dfcb455 100644 --- a/erpnext/stock/doctype/packed_item/packed_item.py +++ b/erpnext/stock/doctype/packed_item/packed_item.py @@ -8,7 +8,7 @@ import json import frappe from frappe.model.document import Document -from frappe.utils import cstr, flt +from frappe.utils import flt from erpnext.stock.get_item_details import get_item_details @@ -16,23 +16,26 @@ from erpnext.stock.get_item_details import get_item_details class PackedItem(Document): pass + def make_packing_list(doc): """make packing list for Product Bundle item""" - if doc.get("_action") and doc._action == "update_after_submit": - return + if doc.get("_action") and doc._action == "update_after_submit": return + + parent_items, reset = [], False + stale_packed_items_table = get_indexed_packed_items_table(doc) if not doc.is_new(): - reset_packing_list_if_deleted_items_exist(doc) + reset = reset_packing_list_if_deleted_items_exist(doc) - parent_items = [] for item in doc.get("items"): if frappe.db.exists("Product Bundle", {"new_item_code": item.item_code}): for bundle_item in get_product_bundle_items(item.item_code): - update_packing_list_item( - doc=doc, packing_item_code=bundle_item.item_code, - qty=flt(bundle_item.qty) * flt(item.stock_qty), - main_item_row=item, description=bundle_item.description + pi_row = add_packed_item_row( + doc=doc, packing_item=bundle_item, + main_item_row=item, packed_items_table=stale_packed_items_table, + reset=reset ) + update_packed_item_details(bundle_item, pi_row, item, doc) if [item.item_code, item.name] not in parent_items: parent_items.append([item.item_code, item.name]) @@ -40,15 +43,31 @@ def make_packing_list(doc): if frappe.db.get_single_value("Selling Settings", "editable_bundle_item_rates"): update_product_bundle_price(doc, parent_items) +def get_indexed_packed_items_table(doc): + """ + Create dict from stale packed items table like: + {(Parent Item 1, Bundle Item 1, ae4b5678): {...}, (key): {value}} + """ + indexed_table = {} + for packed_item in doc.get("packed_items"): + key = (packed_item.parent_item, packed_item.item_code, packed_item.parent_detail_docname) + indexed_table[key] = packed_item + + return indexed_table + def reset_packing_list_if_deleted_items_exist(doc): doc_before_save = doc.get_doc_before_save() - if doc_before_save: - items_are_deleted = len(doc_before_save.get("items")) != len(doc.get("items")) - else: - items_are_deleted = True + reset_table = False - if items_are_deleted: + if doc_before_save: + # reset table if items were deleted + reset_table = len(doc_before_save.get("items")) > len(doc.get("items")) + else: + reset_table = True # reset if via Update Items (cannot determine action) + + if reset_table: doc.set("packed_items", []) + return reset_table def get_product_bundle_items(item_code): product_bundle = frappe.qb.DocType("Product Bundle") @@ -70,52 +89,30 @@ def get_product_bundle_items(item_code): ) return query.run(as_dict=True) -def update_packing_list_item(doc, packing_item_code, qty, main_item_row, description): - old_packed_items_map = None +def add_packed_item_row(doc, packing_item, main_item_row, packed_items_table, reset): + """Add and return packed item row. + doc: Transaction document + packing_item (dict): Packed Item details + main_item_row (dict): Items table row corresponding to packed item + packed_items_table (dict): Packed Items table before save (indexed) + reset (bool): State if table is reset or preserved as is + """ + exists, pi_row = False, {} - if doc.amended_from: - old_packed_items_map = get_old_packed_item_details(doc.packed_items) - - item = get_packing_item_details(packing_item_code, doc.company) - - # check if exists - exists = 0 - for d in doc.get("packed_items"): - if (d.parent_item == main_item_row.item_code and - d.item_code == packing_item_code and - d.parent_detail_docname == main_item_row.name - ): - pi, exists = d, 1 - break + # check if row already exists in packed items table + key = (main_item_row.item_code, packing_item.item_code, main_item_row.name) + if packed_items_table.get(key): + pi_row, exists = packed_items_table.get(key), True if not exists: - pi = doc.append('packed_items', {}) + pi_row = doc.append('packed_items', {}) + elif reset: # add row if row exists but table is reset + pi_row.idx, pi_row.name = None, None + pi_row = doc.append('packed_items', pi_row) - pi.parent_item = main_item_row.item_code - pi.item_code = packing_item_code - pi.item_name = item.item_name - pi.parent_detail_docname = main_item_row.name - pi.uom = item.stock_uom - pi.qty = flt(qty) - pi.conversion_factor = main_item_row.conversion_factor - if description and not pi.description: - pi.description = description - if not pi.warehouse and not doc.amended_from: - pi.warehouse = (main_item_row.warehouse if ((doc.get('is_pos') or item.is_stock_item \ - or not item.default_warehouse) and main_item_row.warehouse) else item.default_warehouse) - if not pi.batch_no and not doc.amended_from: - pi.batch_no = cstr(main_item_row.get("batch_no")) - if not pi.target_warehouse: - pi.target_warehouse = main_item_row.get("target_warehouse") - bin = get_packed_item_bin_qty(packing_item_code, pi.warehouse) - pi.actual_qty = flt(bin.get("actual_qty")) - pi.projected_qty = flt(bin.get("projected_qty")) - if old_packed_items_map and old_packed_items_map.get((packing_item_code, main_item_row.item_code)): - pi.batch_no = old_packed_items_map.get((packing_item_code, main_item_row.item_code))[0].batch_no - pi.serial_no = old_packed_items_map.get((packing_item_code, main_item_row.item_code))[0].serial_no - pi.warehouse = old_packed_items_map.get((packing_item_code, main_item_row.item_code))[0].warehouse + return pi_row -def get_packing_item_details(item_code, company): +def get_packed_item_details(item_code, company): item = frappe.qb.DocType("Item") item_default = frappe.qb.DocType("Item Default") query = ( @@ -134,6 +131,44 @@ def get_packing_item_details(item_code, company): ) return query.run(as_dict=True)[0] +def update_packed_item_details(packing_item, pi_row, main_item_row, doc): + "Update additional packed item row details." + item = get_packed_item_details(packing_item.item_code, doc.company) + + prev_doc_packed_items_map = None + if doc.amended_from: + prev_doc_packed_items_map = get_cancelled_doc_packed_item_details(doc.packed_items) + + pi_row.parent_item = main_item_row.item_code + pi_row.parent_detail_docname = main_item_row.name + pi_row.item_code = packing_item.item_code + pi_row.item_name = item.item_name + pi_row.uom = item.stock_uom + pi_row.qty = flt(packing_item.qty) * flt(main_item_row.stock_qty) + pi_row.conversion_factor = main_item_row.conversion_factor + + if not pi_row.description: + pi_row.description = packing_item.get("description") + + if not pi_row.warehouse and not doc.amended_from: + pi_row.warehouse = (main_item_row.warehouse if ((doc.get('is_pos') or item.is_stock_item \ + or not item.default_warehouse) and main_item_row.warehouse) else item.default_warehouse) + + # TODO batch_no, actual_batch_qty, incoming_rate + + if not pi_row.target_warehouse: + pi_row.target_warehouse = main_item_row.get("target_warehouse") + + bin = get_packed_item_bin_qty(packing_item.item_code, pi_row.warehouse) + pi_row.actual_qty = flt(bin.get("actual_qty")) + pi_row.projected_qty = flt(bin.get("projected_qty")) + + if prev_doc_packed_items_map and prev_doc_packed_items_map.get((packing_item.item_code, main_item_row.item_code)): + prev_doc_row = prev_doc_packed_items_map.get((packing_item.item_code, main_item_row.item_code)) + pi_row.batch_no = prev_doc_row[0].batch_no + pi_row.serial_no = prev_doc_row[0].serial_no + pi_row.warehouse = prev_doc_row[0].warehouse + def get_packed_item_bin_qty(item, warehouse): bin_data = frappe.db.get_values( "Bin", @@ -144,37 +179,14 @@ def get_packed_item_bin_qty(item, warehouse): return bin_data[0] if bin_data else {} -def get_old_packed_item_details(old_packed_items): - old_packed_items_map = {} +def get_cancelled_doc_packed_item_details(old_packed_items): + prev_doc_packed_items_map = {} for items in old_packed_items: - old_packed_items_map.setdefault((items.item_code ,items.parent_item), []).append(items.as_dict()) - return old_packed_items_map - -def add_item_to_packing_list(doc, packed_item): - doc.append("packed_items", { - 'parent_item': packed_item.parent_item, - 'item_code': packed_item.item_code, - 'item_name': packed_item.item_name, - 'uom': packed_item.uom, - 'qty': packed_item.qty, - 'rate': packed_item.rate, - 'conversion_factor': packed_item.conversion_factor, - 'description': packed_item.description, - 'warehouse': packed_item.warehouse, - 'batch_no': packed_item.batch_no, - 'actual_batch_qty': packed_item.actual_batch_qty, - 'serial_no': packed_item.serial_no, - 'target_warehouse': packed_item.target_warehouse, - 'actual_qty': packed_item.actual_qty, - 'projected_qty': packed_item.projected_qty, - 'incoming_rate': packed_item.incoming_rate, - 'prevdoc_doctype': packed_item.prevdoc_doctype, - 'parent_detail_docname': packed_item.parent_detail_docname - }) + prev_doc_packed_items_map.setdefault((items.item_code ,items.parent_item), []).append(items.as_dict()) + return prev_doc_packed_items_map def update_product_bundle_price(doc, parent_items): """Updates the prices of Product Bundles based on the rates of the Items in the bundle.""" - if not doc.get('items'): return @@ -204,17 +216,18 @@ def update_parent_item_price(doc, parent_item_code, bundle_price): parent_item_doc.amount = bundle_price parent_item_doc.rate = bundle_price/(parent_item_doc.qty or 1) -@frappe.whitelist() -def get_items_from_product_bundle(args): - args, items = json.loads(args), [] - bundled_items = get_product_bundle_items(args["item_code"]) +@frappe.whitelist() +def get_items_from_product_bundle(row): + row, items = json.loads(row), [] + + bundled_items = get_product_bundle_items(row["item_code"]) for item in bundled_items: - args.update({ + row.update({ "item_code": item.item_code, - "qty": flt(args["quantity"]) * flt(item.qty) + "qty": flt(row["quantity"]) * flt(item.qty) }) - items.append(get_item_details(args)) + items.append(get_item_details(row)) return items