From acccdb38900f702d5c41c4063a25e193850554d8 Mon Sep 17 00:00:00 2001 From: tundebabzy Date: Thu, 23 Nov 2017 08:35:15 +0100 Subject: [PATCH] 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 dd024b1eb369eaeafab28165d523fb7a9f8ab8d3. * 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 --- .../user/manual/en/accounts/sales-invoice.md | 15 +++++ erpnext/selling/sales_common.js | 37 ++++++++++- erpnext/stock/doctype/batch/batch.py | 49 ++++++++++---- erpnext/stock/doctype/batch/test_batch.py | 66 +++++++++---------- erpnext/stock/get_item_details.py | 51 ++++++++++++-- 5 files changed, 167 insertions(+), 51 deletions(-) diff --git a/erpnext/docs/user/manual/en/accounts/sales-invoice.md b/erpnext/docs/user/manual/en/accounts/sales-invoice.md index f4c4143504..3ea13f09b5 100644 --- a/erpnext/docs/user/manual/en/accounts/sales-invoice.md +++ b/erpnext/docs/user/manual/en/accounts/sales-invoice.md @@ -56,6 +56,21 @@ bill this Invoice and the period for which the contract is valid. ERPNext will automatically create new Invoices and mail it to the Email Addresses you set. +#### Automatically Fetching Item Batch Numbers + +If you are selling an item from a [Batch](/docs/user/manual/en/stock/batch), +ERPNext will automatically fetch a batch number for you if "Update Stock" +is checked. The batch number will be fetched on a First Expiring First Out +(FEFO) basis. This is a variant of First In First Out (FIFO) that gives +highest priority to the soonest to expire Items. + +POS Invoice + +Note that if the first batch in the queue cannot satisfy the order on the invoice, +the next batch in the queue that can satisfy the order will be selected. If there is +no batch that can satisfy the order, ERPNext will cancel its attempt to automatically +fetch a suitable batch number. + #### POS Invoices Consider a scenario where retail transaction is carried out. For e.g: A retail shop. diff --git a/erpnext/selling/sales_common.js b/erpnext/selling/sales_common.js index 97b7b99d08..6a148e202a 100644 --- a/erpnext/selling/sales_common.js +++ b/erpnext/selling/sales_common.js @@ -335,7 +335,42 @@ erpnext.selling.SellingController = erpnext.TransactionController.extend({ } else { this.frm.set_value("company_address_display", ""); } - } + }, + + conversion_factor: function(doc, cdt, cdn, dont_fetch_price_list_rate) { + this._super(doc, cdt, cdn, dont_fetch_price_list_rate); + if(frappe.meta.get_docfield(cdt, "stock_qty", cdn)) { + this.set_batch_number(cdt, cdn); + } + }, + + qty: function(doc, cdt, cdn) { + this._super(doc, cdt, cdn); + this.set_batch_number(cdt, cdn); + }, + + /* Determine appropriate batch number and set it in the form. + * @param {string} cdt - Document Doctype + * @param {string} cdn - Document name + */ + set_batch_number: function(cdt, cdn) { + const doc = frappe.get_doc(cdt, cdn); + if(doc) { + this._set_batch_number(doc); + } + }, + + _set_batch_number: function(doc) { + return frappe.call({ + method: 'erpnext.stock.doctype.batch.batch.get_batch_no', + args: {'item_code': doc.item_code, 'warehouse': doc.warehouse, 'qty': flt(doc.qty) * flt(doc.conversion_factor)}, + callback: function(r) { + if(r.message) { + frappe.model.set_value(doc.doctype, doc.name, 'batch_no', r.message); + } + } + }); + }, }); frappe.ui.form.on(cur_frm.doctype,"project", function(frm) { diff --git a/erpnext/stock/doctype/batch/batch.py b/erpnext/stock/doctype/batch/batch.py index c58a98d7c8..8d72ac3e2c 100644 --- a/erpnext/stock/doctype/batch/batch.py +++ b/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 diff --git a/erpnext/stock/doctype/batch/test_batch.py b/erpnext/stock/doctype/batch/test_batch.py index eb3b48eb9e..a327b2ded9 100644 --- a/erpnext/stock/doctype/batch/test_batch.py +++ b/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''' diff --git a/erpnext/stock/get_item_details.py b/erpnext/stock/get_item_details.py index b9258b03f2..e5cd2fed23 100644 --- a/erpnext/stock/get_item_details.py +++ b/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