From db4fd7f5b688e757f05d67ce24033648e3eb151a Mon Sep 17 00:00:00 2001 From: tundebabzy Date: Fri, 26 Jan 2018 06:30:20 +0100 Subject: [PATCH] Improvements to batch auto naming (#12496) * refactor: add new function - batch_uses_naming_series use batch_uses_naming_series in autoname method * properly update naming series on delete: - add new functions - get_batch_prefix, get_batch_naming_series_key, get_batch_naming_series - refactor get_name_from_naming_series - add after_delete method * add documentation and rename some functions * PEP 8 compliance * test --- erpnext/stock/doctype/batch/batch.py | 110 +++++++++++++++++----- erpnext/stock/doctype/batch/test_batch.py | 38 +++++++- 2 files changed, 126 insertions(+), 22 deletions(-) diff --git a/erpnext/stock/doctype/batch/batch.py b/erpnext/stock/doctype/batch/batch.py index 42e2ceff68..03ce7b664c 100644 --- a/erpnext/stock/doctype/batch/batch.py +++ b/erpnext/stock/doctype/batch/batch.py @@ -5,24 +5,31 @@ from __future__ import unicode_literals import frappe from frappe import _ from frappe.model.document import Document -from frappe.model.naming import make_autoname +from frappe.model.naming import make_autoname, revert_series_if_last from frappe.utils import flt, cint -class UnableToSelectBatchError(frappe.ValidationError): pass +class UnableToSelectBatchError(frappe.ValidationError): + pass def get_name_from_naming_series(): - naming_series_prefix = frappe.db.get_single_value('Stock Settings', 'naming_series_prefix') - if not naming_series_prefix: - naming_series_prefix = 'BATCH-' - - name = make_autoname(naming_series_prefix + '.#####') + """ + Get a name generated for a Batch from the Batch's naming series. + :return: The string that was generated. + """ + naming_series_prefix = _get_batch_prefix() + key = _make_naming_series_key(naming_series_prefix) + name = make_autoname(key) return name def get_name_from_hash(): + """ + Get a name for a Batch by generating a unique hash. + :return: The hash that was generated. + """ temp = None while not temp: temp = frappe.generate_hash()[:7].upper() @@ -32,13 +39,66 @@ def get_name_from_hash(): return temp +def batch_uses_naming_series(): + """ + Verify if the Batch is to be named using a naming series + :return: bool + """ + use_naming_series = cint(frappe.db.get_single_value('Stock Settings', 'use_naming_series')) + return bool(use_naming_series) + + +def _get_batch_prefix(): + """ + Get the naming series prefix set in Stock Settings. + + It does not do any sanity checks so make sure to use it after checking if the Batch + is set to use naming series. + :return: The naming series. + """ + naming_series_prefix = frappe.db.get_single_value('Stock Settings', 'naming_series_prefix') + if not naming_series_prefix: + naming_series_prefix = 'BATCH-' + + return naming_series_prefix + + +def _make_naming_series_key(prefix): + """ + Make naming series key for a Batch. + + Naming series key is in the format [prefix].[#####] + :param prefix: Naming series prefix gotten from Stock Settings + :return: The derived key. If no prefix is given, an empty string is returned + """ + if not unicode(prefix): + return '' + else: + return prefix.upper() + '.#####' + + +def get_batch_naming_series(): + """ + Get naming series key for a Batch. + + Naming series key is in the format [prefix].[#####] + :return: The naming series or empty string if not available + """ + series = '' + if batch_uses_naming_series(): + prefix = _get_batch_prefix() + key = _make_naming_series_key(prefix) + series = key + + return series + + class Batch(Document): def autoname(self): """Generate random ID for batch if not specified""" if not self.batch_id: if frappe.db.get_value('Item', self.item, 'create_new_batch'): - use_naming_series = frappe.db.get_single_value('Stock Settings', 'use_naming_series') - if use_naming_series: + if batch_uses_naming_series(): self.batch_id = get_name_from_naming_series() else: self.batch_id = get_name_from_hash() @@ -50,13 +110,17 @@ class Batch(Document): def onload(self): self.image = frappe.db.get_value('Item', self.item, 'image') + def after_delete(self): + revert_series_if_last(get_batch_naming_series(), self.name) + def validate(self): self.item_has_batch_enabled() def item_has_batch_enabled(self): - if frappe.db.get_value("Item",self.item,"has_batch_no") == 0: + if frappe.db.get_value("Item", self.item, "has_batch_no") == 0: frappe.throw(_("The selected item cannot have Batch")) + @frappe.whitelist() def get_batch_qty(batch_no=None, warehouse=None, item_code=None): """Returns batch actual qty if warehouse is passed, @@ -89,16 +153,18 @@ def get_batch_qty(batch_no=None, warehouse=None, item_code=None): return out + @frappe.whitelist() def get_batches_by_oldest(item_code, warehouse): """Returns the oldest batch and qty for the given item_code and warehouse""" - batches = get_batch_qty(item_code = item_code, warehouse = warehouse) + batches = get_batch_qty(item_code=item_code, warehouse=warehouse) batches_dates = [[batch, frappe.get_value('Batch', batch.batch_no, 'expiry_date')] for batch in batches] batches_dates.sort(key=lambda tup: tup[1]) return batches_dates + @frappe.whitelist() -def split_batch(batch_no, item_code, warehouse, qty, new_batch_id = None): +def split_batch(batch_no, item_code, warehouse, qty, new_batch_id=None): """Split the batch into a new batch""" batch = frappe.get_doc(dict(doctype='Batch', item=item_code, batch_id=new_batch_id)).insert() stock_entry = frappe.get_doc(dict( @@ -106,16 +172,16 @@ def split_batch(batch_no, item_code, warehouse, qty, new_batch_id = None): purpose='Repack', items=[ dict( - item_code = item_code, - qty = float(qty or 0), - s_warehouse = warehouse, - batch_no = batch_no + item_code=item_code, + qty=float(qty or 0), + s_warehouse=warehouse, + batch_no=batch_no ), dict( - item_code = item_code, - qty = float(qty or 0), - t_warehouse = warehouse, - batch_no = batch.name + item_code=item_code, + qty=float(qty or 0), + t_warehouse=warehouse, + batch_no=batch.name ), ] )) @@ -124,7 +190,8 @@ def split_batch(batch_no, item_code, warehouse, qty, new_batch_id = None): return batch.name -def set_batch_nos(doc, warehouse_field, throw = False): + +def set_batch_nos(doc, warehouse_field, throw=False): """Automatically select `batch_no` for outgoing items in item table""" for d in doc.items: qty = d.get('stock_qty') or d.get('transfer_qty') or d.get('qty') or 0 @@ -138,6 +205,7 @@ 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)) + @frappe.whitelist() def get_batch_no(item_code, warehouse, qty=1, throw=False): """ diff --git a/erpnext/stock/doctype/batch/test_batch.py b/erpnext/stock/doctype/batch/test_batch.py index a327b2ded9..9538781d4d 100644 --- a/erpnext/stock/doctype/batch/test_batch.py +++ b/erpnext/stock/doctype/batch/test_batch.py @@ -7,6 +7,8 @@ from frappe.exceptions import ValidationError import unittest from erpnext.stock.doctype.batch.batch import get_batch_qty, UnableToSelectBatchError, get_batch_no +from frappe.utils import cint + class TestBatch(unittest.TestCase): @@ -21,7 +23,7 @@ class TestBatch(unittest.TestCase): def make_batch_item(cls, item_name): from erpnext.stock.doctype.item.test_item import make_item if not frappe.db.exists(item_name): - make_item(item_name, dict(has_batch_no = 1, create_new_batch = 1)) + return make_item(item_name, dict(has_batch_no = 1, create_new_batch = 1)) def test_purchase_receipt(self, batch_qty = 100): '''Test automated batch creation from Purchase Receipt''' @@ -192,3 +194,37 @@ class TestBatch(unittest.TestCase): ] )).insert() stock_entry.submit() + + def test_batch_name_with_naming_series(self): + stock_settings = frappe.get_single('Stock Settings') + use_naming_series = cint(stock_settings.use_naming_series) + + if not use_naming_series: + frappe.set_value('Stock Settings', 'Stock Settings', 'use_naming_series', 1) + + batch = self.make_new_batch('_Test Stock Item For Batch Test1') + batch_name = batch.name + + self.assertTrue(batch_name.startswith('BATCH-')) + + batch.delete() + batch = self.make_new_batch('_Test Stock Item For Batch Test2') + + self.assertEqual(batch_name, batch.name) + + # reset Stock Settings + if not use_naming_series: + frappe.set_value('Stock Settings', 'Stock Settings', 'use_naming_series', 0) + + def make_new_batch(self, item_name, batch_id=None, do_not_insert=0): + batch = frappe.new_doc('Batch') + item = self.make_batch_item(item_name) + batch.item = item.name + + if batch_id: + batch.batch_id = batch_id + + if not do_not_insert: + batch.insert() + + return batch