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
diff --git a/erpnext/stock/doctype/batch/batch.py b/erpnext/stock/doctype/batch/batch.py
index 42e2cef..03ce7b6 100644
--- a/erpnext/stock/doctype/batch/batch.py
+++ b/erpnext/stock/doctype/batch/batch.py
@@ -5,24 +5,31 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 a327b2d..9538781 100644
--- a/erpnext/stock/doctype/batch/test_batch.py
+++ b/erpnext/stock/doctype/batch/test_batch.py
@@ -7,6 +7,8 @@
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 @@
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 @@
]
)).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