From 7bcad019538b8d85717a4bc48bccd54ba90ed1f2 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Sun, 20 Mar 2022 19:25:10 +0530 Subject: [PATCH] fix(UX): misc serial no selector + warehouse.py refactor (backport #30309) (#30310) * fix: set current qty as default qty for stock entry (cherry picked from commit f4c213379e03f09c59dac1c16efbafe00ae37e3f) * fix: filter serial nos by selected batch number (cherry picked from commit 5ec27c90552ab5c95ef734bb620a0e1bdd5256ee) * fix: skip already selected serials in sr selector (cherry picked from commit 0a533d6ccc7356ef8b7c10e1c581cf630f7c3121) * fix: sort serial nos before sending (cherry picked from commit 4f8bb91eae3ac6d3a3837a21be7f635a4044ccd0) * test: auto serial fetching (cherry picked from commit b9eec331e3b884a2da5ee6995cca1a7135b7c3b0) * refactor: batch no filter handling (cherry picked from commit a585dff6fd05c885c6b4c35892f48d1fdf644487) # Conflicts: # erpnext/stock/doctype/serial_no/serial_no.py * refactor: Use QB for serial fetching query (cherry picked from commit 4b695915f458a7acb5257d1208d91f23998727eb) # Conflicts: # erpnext/stock/doctype/serial_no/serial_no.py * refactor(warehouse): raw query to ORM (cherry picked from commit 953afda01bc74f930d844d426d00f9f8cadea897) * test: warehouse conversion and treeview test (cherry picked from commit 684d9d66d1bf98a010516f125507712a3d59b2c8) * perf: Single query to delete bins instead of `N` (cherry picked from commit 48595742338a5ca2cd429164a643ae70a8c63caf) * chore: resolve conflicts Co-authored-by: Ankush Menat --- .../js/utils/serial_no_batch_selector.js | 32 ++++-- erpnext/stock/doctype/serial_no/serial_no.py | 101 ++++++++++-------- .../stock/doctype/serial_no/test_serial_no.py | 58 +++++++++- .../stock/doctype/warehouse/test_warehouse.py | 31 +++++- erpnext/stock/doctype/warehouse/warehouse.py | 30 +++--- 5 files changed, 178 insertions(+), 74 deletions(-) diff --git a/erpnext/public/js/utils/serial_no_batch_selector.js b/erpnext/public/js/utils/serial_no_batch_selector.js index 16e3fa0abd1..26e559f672b 100644 --- a/erpnext/public/js/utils/serial_no_batch_selector.js +++ b/erpnext/public/js/utils/serial_no_batch_selector.js @@ -75,7 +75,7 @@ erpnext.SerialNoBatchSelector = Class.extend({ fieldtype:'Float', read_only: me.has_batch && !me.has_serial_no, label: __(me.has_batch && !me.has_serial_no ? 'Selected Qty' : 'Qty'), - default: flt(me.item.stock_qty), + default: flt(me.item.stock_qty) || flt(me.item.transfer_qty), }, ...get_pending_qty_fields(me), { @@ -94,14 +94,16 @@ erpnext.SerialNoBatchSelector = Class.extend({ description: __('Fetch Serial Numbers based on FIFO'), click: () => { let qty = this.dialog.fields_dict.qty.get_value(); + let already_selected_serial_nos = get_selected_serial_nos(me); let numbers = frappe.call({ method: "erpnext.stock.doctype.serial_no.serial_no.auto_fetch_serial_number", args: { qty: qty, item_code: me.item_code, warehouse: typeof me.warehouse_details.name == "string" ? me.warehouse_details.name : '', - batch_no: me.item.batch_no || null, - posting_date: me.frm.doc.posting_date || me.frm.doc.transaction_date + batch_nos: me.item.batch_no || null, + posting_date: me.frm.doc.posting_date || me.frm.doc.transaction_date, + exclude_sr_nos: already_selected_serial_nos } }); @@ -575,15 +577,29 @@ function get_pending_qty_fields(me) { return pending_qty_fields; } -function calc_total_selected_qty(me) { +// get all items with same item code except row for which selector is open. +function get_rows_with_same_item_code(me) { const { frm: { doc: { items }}, item: { name, item_code }} = me; - const totalSelectedQty = items - .filter( item => ( item.name !== name ) && ( item.item_code === item_code ) ) - .map( item => flt(item.qty) ) - .reduce( (i, j) => i + j, 0); + return items.filter(item => (item.name !== name) && (item.item_code === item_code)) +} + +function calc_total_selected_qty(me) { + const totalSelectedQty = get_rows_with_same_item_code(me) + .map(item => flt(item.qty)) + .reduce((i, j) => i + j, 0); return totalSelectedQty; } +function get_selected_serial_nos(me) { + const selected_serial_nos = get_rows_with_same_item_code(me) + .map(item => item.serial_no) + .filter(serial => serial) + .map(sr_no_string => sr_no_string.split('\n')) + .reduce((acc, arr) => acc.concat(arr), []) + .filter(serial => serial); + return selected_serial_nos; +}; + function check_can_calculate_pending_qty(me) { const { frm: { doc }, item } = me; const docChecks = doc.bom_no diff --git a/erpnext/stock/doctype/serial_no/serial_no.py b/erpnext/stock/doctype/serial_no/serial_no.py index 350d5fec50f..f97ebb97140 100644 --- a/erpnext/stock/doctype/serial_no/serial_no.py +++ b/erpnext/stock/doctype/serial_no/serial_no.py @@ -7,9 +7,17 @@ import json import frappe from frappe import ValidationError, _ from frappe.model.naming import make_autoname -from frappe.utils import add_days, cint, cstr, flt, get_link_to_form, getdate, nowdate -from six import string_types -from six.moves import map +from frappe.query_builder.functions import Coalesce +from frappe.utils import ( + add_days, + cint, + cstr, + flt, + get_link_to_form, + getdate, + nowdate, + safe_json_loads, +) from erpnext.controllers.stock_controller import StockController from erpnext.stock.get_item_details import get_reserved_qty_for_so @@ -583,30 +591,37 @@ def get_delivery_note_serial_no(item_code, qty, delivery_note): return serial_nos @frappe.whitelist() -def auto_fetch_serial_number(qty, item_code, warehouse, posting_date=None, batch_nos=None, for_doctype=None): - filters = { "item_code": item_code, "warehouse": warehouse } +def auto_fetch_serial_number(qty, item_code, warehouse, + posting_date=None, batch_nos=None, for_doctype=None, exclude_sr_nos=None): + + filters = frappe._dict({"item_code": item_code, "warehouse": warehouse}) + + if exclude_sr_nos is None: + exclude_sr_nos = [] + else: + exclude_sr_nos = get_serial_nos(clean_serial_no_string("\n".join(exclude_sr_nos))) if batch_nos: - try: - filters["batch_no"] = json.loads(batch_nos) if (type(json.loads(batch_nos)) == list) else [json.loads(batch_nos)] - except Exception: - filters["batch_no"] = [batch_nos] + batch_nos = safe_json_loads(batch_nos) + if isinstance(batch_nos, list): + filters.batch_no = batch_nos + elif isinstance(batch_nos, str): + filters.batch_no = [batch_nos] if posting_date: - filters["expiry_date"] = posting_date + filters.expiry_date = posting_date serial_numbers = [] if for_doctype == 'POS Invoice': - reserved_sr_nos = get_pos_reserved_serial_nos(filters) - serial_numbers = fetch_serial_numbers(filters, qty, do_not_include=reserved_sr_nos) - else: - serial_numbers = fetch_serial_numbers(filters, qty) + exclude_sr_nos.extend(get_pos_reserved_serial_nos(filters)) - return [d.get('name') for d in serial_numbers] + serial_numbers = fetch_serial_numbers(filters, qty, do_not_include=exclude_sr_nos) + + return sorted([d.get('name') for d in serial_numbers]) @frappe.whitelist() def get_pos_reserved_serial_nos(filters): - if isinstance(filters, string_types): + if isinstance(filters, str): filters = json.loads(filters) pos_transacted_sr_nos = frappe.db.sql("""select item.serial_no as serial_no @@ -629,37 +644,37 @@ def get_pos_reserved_serial_nos(filters): def fetch_serial_numbers(filters, qty, do_not_include=None): if do_not_include is None: do_not_include = [] - batch_join_selection = "" - batch_no_condition = "" + batch_nos = filters.get("batch_no") expiry_date = filters.get("expiry_date") + serial_no = frappe.qb.DocType("Serial No") + + query = ( + frappe.qb + .from_(serial_no) + .select(serial_no.name) + .where( + (serial_no.item_code == filters["item_code"]) + & (serial_no.warehouse == filters["warehouse"]) + & (Coalesce(serial_no.sales_invoice, "") == "") + & (Coalesce(serial_no.delivery_document_no, "") == "") + ) + .orderby(serial_no.creation) + .limit(qty or 1) + ) + + if do_not_include: + query = query.where(serial_no.name.notin(do_not_include)) + if batch_nos: - batch_no_condition = """and sr.batch_no in ({}) """.format(', '.join("'%s'" % d for d in batch_nos)) + query = query.where(serial_no.batch_no.isin(batch_nos)) if expiry_date: - batch_join_selection = "LEFT JOIN `tabBatch` batch on sr.batch_no = batch.name " - expiry_date_cond = "AND ifnull(batch.expiry_date, '2500-12-31') >= %(expiry_date)s " - batch_no_condition += expiry_date_cond - - excluded_sr_nos = ", ".join(["" + frappe.db.escape(sr) + "" for sr in do_not_include]) or "''" - serial_numbers = frappe.db.sql(""" - SELECT sr.name FROM `tabSerial No` sr {batch_join_selection} - WHERE - sr.name not in ({excluded_sr_nos}) AND - sr.item_code = %(item_code)s AND - sr.warehouse = %(warehouse)s AND - ifnull(sr.sales_invoice,'') = '' AND - ifnull(sr.delivery_document_no, '') = '' - {batch_no_condition} - ORDER BY - sr.creation - LIMIT - {qty} - """.format( - excluded_sr_nos=excluded_sr_nos, - qty=qty or 1, - batch_join_selection=batch_join_selection, - batch_no_condition=batch_no_condition - ), filters, as_dict=1) + batch = frappe.qb.DocType("Batch") + query = (query + .left_join(batch).on(serial_no.batch_no == batch.name) + .where(Coalesce(batch.expiry_date, "4000-12-31") >= expiry_date) + ) + serial_numbers = query.run(as_dict=True) return serial_numbers diff --git a/erpnext/stock/doctype/serial_no/test_serial_no.py b/erpnext/stock/doctype/serial_no/test_serial_no.py index 057a7d4c01f..cca63078402 100644 --- a/erpnext/stock/doctype/serial_no/test_serial_no.py +++ b/erpnext/stock/doctype/serial_no/test_serial_no.py @@ -6,10 +6,12 @@ import frappe +from frappe.tests.utils import FrappeTestCase from erpnext.stock.doctype.delivery_note.test_delivery_note import create_delivery_note from erpnext.stock.doctype.item.test_item import make_item from erpnext.stock.doctype.purchase_receipt.test_purchase_receipt import make_purchase_receipt +from erpnext.stock.doctype.serial_no.serial_no import * from erpnext.stock.doctype.serial_no.serial_no import get_serial_nos from erpnext.stock.doctype.stock_entry.stock_entry_utils import make_stock_entry from erpnext.stock.doctype.stock_entry.test_stock_entry import make_serialized_item @@ -18,9 +20,6 @@ from erpnext.stock.doctype.warehouse.test_warehouse import create_warehouse test_dependencies = ["Item"] test_records = frappe.get_test_records('Serial No') -from frappe.tests.utils import FrappeTestCase - -from erpnext.stock.doctype.serial_no.serial_no import * class TestSerialNo(FrappeTestCase): @@ -242,3 +241,56 @@ class TestSerialNo(FrappeTestCase): ) self.assertEqual(value_diff, -113) + def test_auto_fetch(self): + item_code = make_item(properties={ + "has_serial_no": 1, + "has_batch_no": 1, + "create_new_batch": 1, + "serial_no_series": "TEST.#######" + }).name + warehouse = "_Test Warehouse - _TC" + + in1 = make_stock_entry(item_code=item_code, to_warehouse=warehouse, qty=5) + in2 = make_stock_entry(item_code=item_code, to_warehouse=warehouse, qty=5) + + in1.reload() + in2.reload() + + batch1 = in1.items[0].batch_no + batch2 = in2.items[0].batch_no + + batch_wise_serials = { + batch1 : get_serial_nos(in1.items[0].serial_no), + batch2: get_serial_nos(in2.items[0].serial_no) + } + + # Test FIFO + first_fetch = auto_fetch_serial_number(5, item_code, warehouse) + self.assertEqual(first_fetch, batch_wise_serials[batch1]) + + # partial FIFO + partial_fetch = auto_fetch_serial_number(2, item_code, warehouse) + self.assertTrue(set(partial_fetch).issubset(set(first_fetch)), + msg=f"{partial_fetch} should be subset of {first_fetch}") + + # exclusion + remaining = auto_fetch_serial_number(3, item_code, warehouse, exclude_sr_nos=partial_fetch) + self.assertEqual(sorted(remaining + partial_fetch), first_fetch) + + # batchwise + for batch, expected_serials in batch_wise_serials.items(): + fetched_sr = auto_fetch_serial_number(5, item_code, warehouse, batch_nos=batch) + self.assertEqual(fetched_sr, sorted(expected_serials)) + + # non existing warehouse + self.assertEqual(auto_fetch_serial_number(10, item_code, warehouse="Nonexisting"), []) + + # multi batch + all_serials = [sr for sr_list in batch_wise_serials.values() for sr in sr_list] + fetched_serials = auto_fetch_serial_number(10, item_code, warehouse, batch_nos=list(batch_wise_serials.keys())) + self.assertEqual(sorted(all_serials), fetched_serials) + + # expiry date + frappe.db.set_value("Batch", batch1, "expiry_date", "1980-01-01") + non_expired_serials = auto_fetch_serial_number(5, item_code, warehouse, posting_date="2021-01-01", batch_nos=batch1) + self.assertEqual(non_expired_serials, []) diff --git a/erpnext/stock/doctype/warehouse/test_warehouse.py b/erpnext/stock/doctype/warehouse/test_warehouse.py index cdb771935b0..08d7c993521 100644 --- a/erpnext/stock/doctype/warehouse/test_warehouse.py +++ b/erpnext/stock/doctype/warehouse/test_warehouse.py @@ -4,12 +4,12 @@ import frappe from frappe.test_runner import make_test_records from frappe.tests.utils import FrappeTestCase -from frappe.utils import cint import erpnext -from erpnext.accounts.doctype.account.test_account import create_account, get_inventory_account +from erpnext.accounts.doctype.account.test_account import create_account from erpnext.stock.doctype.item.test_item import create_item from erpnext.stock.doctype.stock_entry.stock_entry_utils import make_stock_entry +from erpnext.stock.doctype.warehouse.warehouse import convert_to_group_or_ledger, get_children test_records = frappe.get_test_records('Warehouse') @@ -65,6 +65,33 @@ class TestWarehouse(FrappeTestCase): f"{item} linked to {item_default.default_warehouse} in {warehouse_ids}." ) + def test_group_non_group_conversion(self): + + warehouse = frappe.get_doc("Warehouse", create_warehouse("TestGroupConversion")) + + convert_to_group_or_ledger(warehouse.name) + warehouse.reload() + self.assertEqual(warehouse.is_group, 1) + + child = create_warehouse("GroupWHChild", {"parent_warehouse": warehouse.name}) + # chid exists + self.assertRaises(frappe.ValidationError, convert_to_group_or_ledger, warehouse.name) + frappe.delete_doc("Warehouse", child) + + convert_to_group_or_ledger(warehouse.name) + warehouse.reload() + self.assertEqual(warehouse.is_group, 0) + + make_stock_entry(item_code="_Test Item", target=warehouse.name, qty=1) + # SLE exists + self.assertRaises(frappe.ValidationError, convert_to_group_or_ledger, warehouse.name) + + def test_get_children(self): + company = "_Test Company" + + children = get_children("Warehouse", parent=company, company=company, is_root=True) + self.assertTrue(any(wh['value'] == "_Test Warehouse - _TC" for wh in children)) + def create_warehouse(warehouse_name, properties=None, company=None): if not company: diff --git a/erpnext/stock/doctype/warehouse/warehouse.py b/erpnext/stock/doctype/warehouse/warehouse.py index 9cfad86f142..4c7f41dcb5e 100644 --- a/erpnext/stock/doctype/warehouse/warehouse.py +++ b/erpnext/stock/doctype/warehouse/warehouse.py @@ -41,14 +41,11 @@ class Warehouse(NestedSet): def on_trash(self): # delete bin - bins = frappe.db.sql("select * from `tabBin` where warehouse = %s", - self.name, as_dict=1) + bins = frappe.get_all("Bin", fields="*", filters={"warehouse": self.name}) for d in bins: if d['actual_qty'] or d['reserved_qty'] or d['ordered_qty'] or \ d['indented_qty'] or d['projected_qty'] or d['planned_qty']: throw(_("Warehouse {0} can not be deleted as quantity exists for Item {1}").format(self.name, d['item_code'])) - else: - frappe.db.sql("delete from `tabBin` where name = %s", d['name']) if self.check_if_sle_exists(): throw(_("Warehouse can not be deleted as stock ledger entry exists for this warehouse.")) @@ -56,16 +53,15 @@ class Warehouse(NestedSet): if self.check_if_child_exists(): throw(_("Child warehouse exists for this warehouse. You can not delete this warehouse.")) + frappe.db.delete("Bin", filters={"warehouse": self.name}) self.update_nsm_model() self.unlink_from_items() def check_if_sle_exists(self): - return frappe.db.sql("""select name from `tabStock Ledger Entry` - where warehouse = %s limit 1""", self.name) + return frappe.db.exists("Stock Ledger Entry", {"warehouse": self.name}) def check_if_child_exists(self): - return frappe.db.sql("""select name from `tabWarehouse` - where parent_warehouse = %s limit 1""", self.name) + return frappe.db.exists("Warehouse", {"parent_warehouse": self.name}) def convert_to_group_or_ledger(self): if self.is_group: @@ -92,10 +88,7 @@ class Warehouse(NestedSet): return 1 def unlink_from_items(self): - frappe.db.sql(""" - update `tabItem Default` - set default_warehouse=NULL - where default_warehouse=%s""", self.name) + frappe.db.set_value("Item Default", {"default_warehouse": self.name}, "default_warehouse", None) @frappe.whitelist() def get_children(doctype, parent=None, company=None, is_root=False): @@ -164,15 +157,16 @@ def add_node(): frappe.get_doc(args).insert() @frappe.whitelist() -def convert_to_group_or_ledger(): - args = frappe.form_dict - return frappe.get_doc("Warehouse", args.docname).convert_to_group_or_ledger() +def convert_to_group_or_ledger(docname=None): + if not docname: + docname = frappe.form_dict.docname + return frappe.get_doc("Warehouse", docname).convert_to_group_or_ledger() def get_child_warehouses(warehouse): - lft, rgt = frappe.get_cached_value("Warehouse", warehouse, ["lft", "rgt"]) + from frappe.utils.nestedset import get_descendants_of - return frappe.db.sql_list("""select name from `tabWarehouse` - where lft >= %s and rgt <= %s""", (lft, rgt)) + children = get_descendants_of("Warehouse", warehouse, ignore_permissions=True, order_by="lft") + return children + [warehouse] # append self for backward compatibility def get_warehouses_based_on_account(account, company=None): warehouses = []