Default batch number selection (#11454)

* fetch batch_no in `get_basic_details`

* PEP8 changes and docstring

* only auto-fetch batch number for Sales documents:
otherwise, automatic batch creation will not work properly because all new
stock will be added to old batch

* new function to fetch batch number using FEFO:
FEFO - First Expiring First Out

* fetch batch number in `get_basic_details` using FEFO

* add new function - `set_batch_number`

* `set_batch_number` when `qty` is triggered

* `set_batch_number` when `uom` is triggered

* whitelist `get_batch_no_fefo`

* code clean up

* move `set_batch_number` to conversion_factor instead of uom

* rename `get_batch_no_fefo` to `get_batch_no`

* fix test case

* final cleanup

* Revert "fetch batch_no in `get_basic_details`"

This reverts commit dd024b1eb3.

* fix test case

* update Sales Invoice manual

* move changes away from `transaction.js`

* query should not fetch expired batches

* refactor `get_batch_no`:
add new function `get_batches`

* Update batch.py

erpnext/stock/doctype/batch/batch.py

@@ -5,7 +5,8 @@ from __future__ import unicode_literals
 import frappe
 from frappe import _
 from frappe.model.document import Document
-from frappe.utils import flt
+from frappe.utils import flt, cint
+
 
 class UnableToSelectBatchError(frappe.ValidationError): pass
 
@@ -53,16 +54,19 @@ def get_batch_qty(batch_no=None, warehouse=None, item_code=None):
 			from `tabStock Ledger Entry`
 			where warehouse=%s and batch_no=%s""",
 			(warehouse, batch_no))[0][0] or 0)
+
 	if batch_no and not warehouse:
 		out = frappe.db.sql('''select warehouse, sum(actual_qty) as qty
 			from `tabStock Ledger Entry`
 			where batch_no=%s
 			group by warehouse''', batch_no, as_dict=1)
+
 	if not batch_no and item_code and warehouse:
 		out = frappe.db.sql('''select batch_no, sum(actual_qty) as qty
 			from `tabStock Ledger Entry`
 			where item_code = %s and warehouse=%s
 			group by batch_no''', (item_code, warehouse), as_dict=1)
+
 	return out
 
 @frappe.whitelist()
@@ -114,21 +118,42 @@ def set_batch_nos(doc, warehouse_field, throw = False):
 				if flt(batch_qty) < flt(qty):
 					frappe.throw(_("Row #{0}: The batch {1} has only {2} qty. Please select another batch which has {3} qty available or split the row into multiple rows, to deliver/issue from multiple batches").format(d.idx, d.batch_no, batch_qty, d.qty))
 
-def get_batch_no(item_code, warehouse, qty, throw=False):
-	'''get the smallest batch with for the given item_code, warehouse and qty'''
+@frappe.whitelist()
+def get_batch_no(item_code, warehouse, qty=1, throw=False):
+	"""
+	Get batch number using First Expiring First Out method.
+	:param item_code: `item_code` of Item Document
+	:param warehouse: name of Warehouse to check
+	:param qty: quantity of Items
+	:return: String represent batch number of batch with sufficient quantity else an empty String
+	"""
 
 	batch_no = None
-	batches = get_batch_qty(item_code = item_code, warehouse = warehouse)
-	if batches:
-		batches = sorted(batches, lambda a, b: 1 if a.qty > b.qty else -1)
-		for b in batches:
-			if b.qty >= qty:
-				batch_no = b.batch_no
-				# found!
-				break
+	batches = get_batches(item_code, warehouse, qty, throw)
+
+	for batch in batches:
+		if cint(qty) <= cint(batch.qty):
+			batch_no = batch.batch_id
+			break
 
 	if not batch_no:
 		frappe.msgprint(_('Please select a Batch for Item {0}. Unable to find a single batch that fulfills this requirement').format(frappe.bold(item_code)))
-		if throw: raise UnableToSelectBatchError
+		if throw:
+			raise UnableToSelectBatchError
 
 	return batch_no
+
+
+def get_batches(item_code, warehouse, qty=1, throw=False):
+	batches = frappe.db.sql(
+		'select batch_id, sum(actual_qty) as qty from `tabBatch` join `tabStock Ledger Entry` '
+		'on `tabBatch`.batch_id = `tabStock Ledger Entry`.batch_no '
+		'where `tabStock Ledger Entry`.item_code = %s and  `tabStock Ledger Entry`.warehouse = %s '
+		'and `tabBatch`.expiry_date >= CURDATE() or `tabBatch`.expiry_date IS NULL '
+		'group by batch_id '
+		'order by `tabBatch`.expiry_date DESC, `tabBatch`.creation ASC',
+		(item_code, warehouse),
+		as_dict=True
+	)
+
+	return batches

erpnext/stock/doctype/batch/test_batch.py

@@ -6,7 +6,7 @@ import frappe
 from frappe.exceptions import ValidationError
 import unittest
 
-from erpnext.stock.doctype.batch.batch import get_batch_qty, UnableToSelectBatchError
+from erpnext.stock.doctype.batch.batch import get_batch_qty, UnableToSelectBatchError, get_batch_no
 
 class TestBatch(unittest.TestCase):
 
@@ -28,13 +28,13 @@ class TestBatch(unittest.TestCase):
 		self.make_batch_item('ITEM-BATCH-1')
 
 		receipt = frappe.get_doc(dict(
-			doctype = 'Purchase Receipt',
-			supplier = '_Test Supplier',
-			items = [
+			doctype='Purchase Receipt',
+			supplier='_Test Supplier',
+			items=[
 				dict(
-					item_code = 'ITEM-BATCH-1',
-					qty = batch_qty,
-					rate = 10
+					item_code='ITEM-BATCH-1',
+					qty=batch_qty,
+					rate=10
 				)
 			]
 		)).insert()
@@ -74,28 +74,28 @@ class TestBatch(unittest.TestCase):
 		'''Test automatic batch selection for outgoing items'''
 		batch_qty = 15
 		receipt = self.test_purchase_receipt(batch_qty)
+		item_code = 'ITEM-BATCH-1'
 
 		delivery_note = frappe.get_doc(dict(
-			doctype = 'Delivery Note',
-			customer = '_Test Customer',
-			company = receipt.company,
-			items = [
+			doctype='Delivery Note',
+			customer='_Test Customer',
+			company=receipt.company,
+			items=[
 				dict(
-					item_code = 'ITEM-BATCH-1',
-					qty = batch_qty,
-					rate = 10,
-					warehouse = receipt.items[0].warehouse
+					item_code=item_code,
+					qty=batch_qty,
+					rate=10,
+					warehouse=receipt.items[0].warehouse
 				)
 			]
 		)).insert()
 		delivery_note.submit()
 
-		# shipped with same batch
-		self.assertEquals(delivery_note.items[0].batch_no, receipt.items[0].batch_no)
-
-		# balance is 0
-		self.assertEquals(get_batch_qty(receipt.items[0].batch_no,
-			receipt.items[0].warehouse), 0)
+		# shipped from FEFO batch
+		self.assertEquals(
+			delivery_note.items[0].batch_no,
+			get_batch_no(item_code, receipt.items[0].warehouse, batch_qty)
+		)
 
 	def test_delivery_note_fail(self):
 		'''Test automatic batch selection for outgoing items'''
@@ -120,27 +120,27 @@ class TestBatch(unittest.TestCase):
 
 		batch_qty = 16
 		receipt = self.test_purchase_receipt(batch_qty)
+		item_code = 'ITEM-BATCH-1'
 
 		stock_entry = frappe.get_doc(dict(
-			doctype = 'Stock Entry',
-			purpose = 'Material Issue',
-			company = receipt.company,
-			items = [
+			doctype='Stock Entry',
+			purpose='Material Issue',
+			company=receipt.company,
+			items=[
 				dict(
-					item_code = 'ITEM-BATCH-1',
-					qty = batch_qty,
-					s_warehouse = receipt.items[0].warehouse,
+					item_code=item_code,
+					qty=batch_qty,
+					s_warehouse=receipt.items[0].warehouse,
 				)
 			]
 		)).insert()
 		stock_entry.submit()
 
 		# assert same batch is selected
-		self.assertEqual(stock_entry.items[0].batch_no, receipt.items[0].batch_no)
-
-		# balance is 0
-		self.assertEquals(get_batch_qty(receipt.items[0].batch_no,
-			receipt.items[0].warehouse), 0)
+		self.assertEqual(
+			stock_entry.items[0].batch_no,
+			get_batch_no(item_code, receipt.items[0].warehouse, batch_qty)
+		)
 
 	def test_batch_split(self):
 		'''Test batch splitting'''

erpnext/stock/get_item_details.py

@@ -11,6 +11,7 @@ from erpnext.setup.utils import get_exchange_rate
 from frappe.model.meta import get_field_precision
 from erpnext.stock.doctype.batch.batch import get_batch_no
 
+
 @frappe.whitelist()
 def get_item_details(args):
 	"""
@@ -84,7 +85,6 @@ def get_item_details(args):
 		if out.has_batch_no and not args.get("batch_no"):
 			out.batch_no = get_batch_no(out.item_code, out.warehouse, out.qty)
 
-
 	if args.transaction_date and item.lead_time_days:
 		out.schedule_date = out.lead_time_date = add_days(args.transaction_date,
 			item.lead_time_days)
@@ -113,6 +113,7 @@ def process_args(args):
 	set_transaction_type(args)
 	return args
 
+
 @frappe.whitelist()
 def get_item_code(barcode=None, serial_no=None):
 	if barcode:
@@ -126,6 +127,7 @@ def get_item_code(barcode=None, serial_no=None):
 
 	return item_code
 
+
 def validate_item_details(args, item):
 	if not args.company:
 		throw(_("Please specify Company"))
@@ -133,14 +135,52 @@ def validate_item_details(args, item):
 	from erpnext.stock.doctype.item.item import validate_end_of_life
 	validate_end_of_life(item.name, item.end_of_life, item.disabled)
 
-	if args.transaction_type=="selling" and cint(item.has_variants):
+	if args.transaction_type == "selling" and cint(item.has_variants):
 		throw(_("Item {0} is a template, please select one of its variants").format(item.name))
 
-	elif args.transaction_type=="buying" and args.doctype != "Material Request":
+	elif args.transaction_type == "buying" and args.doctype != "Material Request":
 		if args.get("is_subcontracted") == "Yes" and item.is_sub_contracted_item != 1:
 			throw(_("Item {0} must be a Sub-contracted Item").format(item.name))
 
+
 def get_basic_details(args, item):
+	"""
+	:param args: {
+			"item_code": "",
+			"warehouse": None,
+			"customer": "",
+			"conversion_rate": 1.0,
+			"selling_price_list": None,
+			"price_list_currency": None,
+			"plc_conversion_rate": 1.0,
+			"doctype": "",
+			"name": "",
+			"supplier": None,
+			"transaction_date": None,
+			"conversion_rate": 1.0,
+			"buying_price_list": None,
+			"is_subcontracted": "Yes" / "No",
+			"ignore_pricing_rule": 0/1
+			"project": "",
+			barcode: "",
+			serial_no: "",
+			warehouse: "",
+			currency: "",
+			update_stock: "",
+			price_list: "",
+			company: "",
+			order_type: "",
+			is_pos: "",
+			ignore_pricing_rule: "",
+			project: "",
+			qty: "",
+			stock_qty: "",
+			conversion_factor: ""
+		}
+	:param item: `item_code` of Item object
+	:return: frappe._dict
+	"""
+
 	if not item:
 		item = frappe.get_doc("Item", args.get("item_code"))
 
@@ -150,7 +190,7 @@ def get_basic_details(args, item):
 	from frappe.defaults import get_user_default_as_list
 	user_default_warehouse_list = get_user_default_as_list('Warehouse')
 	user_default_warehouse = user_default_warehouse_list[0] \
-		if len(user_default_warehouse_list)==1 else ""
+		if len(user_default_warehouse_list) == 1 else ""
 
 	warehouse = user_default_warehouse or item.default_warehouse or args.warehouse
 
@@ -207,7 +247,7 @@ def get_basic_details(args, item):
 		out.conversion_factor = 1.0
 	else:
 		out.conversion_factor = args.conversion_factor or \
-			get_conversion_factor(item.item_code, args.uom).get("conversion_factor")  or 1.0
+			get_conversion_factor(item.item_code, args.uom).get("conversion_factor") or 1.0
 
 	args.conversion_factor = out.conversion_factor
 	out.stock_qty = out.qty * out.conversion_factor
@@ -227,6 +267,7 @@ def get_basic_details(args, item):
 
 	return out
 
+
 def get_default_income_account(args, item):
 	return (item.income_account
 		or args.income_account