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 f4c213379e)

* fix: filter serial nos by selected batch number

(cherry picked from commit 5ec27c9055)

* fix: skip already selected serials in sr selector

(cherry picked from commit 0a533d6ccc)

* fix: sort serial nos before sending

(cherry picked from commit 4f8bb91eae)

* test: auto serial fetching

(cherry picked from commit b9eec331e3)

* refactor: batch no filter handling

(cherry picked from commit a585dff6fd)

# Conflicts:
#	erpnext/stock/doctype/serial_no/serial_no.py

* refactor: Use QB for serial fetching query

(cherry picked from commit 4b695915f4)

# Conflicts:
#	erpnext/stock/doctype/serial_no/serial_no.py

* refactor(warehouse): raw query to ORM

(cherry picked from commit 953afda01b)

* test: warehouse conversion and treeview test

(cherry picked from commit 684d9d66d1)

* perf: Single query to delete bins instead of `N`

(cherry picked from commit 4859574233)

* chore: resolve conflicts

Co-authored-by: Ankush Menat <ankush@frappe.io>

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

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

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, [])

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:

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 = []