fix: accurate prefix parsing

Previous version of prefix parsing relied on partial reimplemntation of
naming series logic which was outdated and often incorrect.

frappe/core/doctype/document_naming_settings/document_naming_settings.py

@@ -7,7 +7,7 @@ import frappe
 from frappe import _
 from frappe.core.doctype.doctype.doctype import validate_series
 from frappe.model.document import Document
-from frappe.model.naming import make_autoname, parse_naming_series
+from frappe.model.naming import get_naming_series_prefix, make_autoname
 from frappe.permissions import get_doctypes_with_read
 from frappe.utils import cint
 
@@ -153,38 +153,35 @@ class DocumentNamingSettings(Document):
 			return frappe.get_meta(doctype or self.transaction_type).get_field("naming_series").options
 
 	@frappe.whitelist()
-	def get_current(self, arg=None):
+	def get_current(self):
 		"""get series current"""
 		if self.prefix:
-			prefix = self.parse_naming_series()
+			prefix = get_naming_series_prefix(self.prefix)
 			self.current_value = frappe.db.get_value("Series", prefix, "current", order_by="name")
 
-	def insert_series(self, series):
-		"""insert series if missing"""
-		if frappe.db.get_value("Series", series, "name", order_by="name") == None:
-			frappe.db.sql("insert into tabSeries (name, current) values (%s, 0)", (series))
-
 	@frappe.whitelist()
 	def update_series_start(self):
-		if self.prefix:
-			prefix = self.parse_naming_series()
-			self.insert_series(prefix)
-			frappe.db.sql(
-				"update `tabSeries` set current = %s where name = %s", (cint(self.current_value), prefix)
-			)
-			frappe.msgprint(_("Series Updated Successfully"))
-		else:
-			frappe.msgprint(_("Please select prefix first"))
+		if not self.prefix:
+			frappe.throw(_("Please select prefix first"))
 
-	def parse_naming_series(self):
-		parts = self.prefix.split(".")
+		series = frappe.qb.DocType("Series")
 
-		# Remove ### from the end of series
-		if parts[-1] == "#" * len(parts[-1]):
-			del parts[-1]
+		db_prefix = get_naming_series_prefix(self.prefix)
 
-		prefix = parse_naming_series(parts)
-		return prefix
+		if frappe.db.get_value("Series", db_prefix, "name", order_by="name") is None:
+			series.insert(db_prefix, 0).columns(series.name, series.current).run()
+
+		(
+			frappe.qb.update(series)
+			.set(series.current, cint(self.current_value))
+			.where(series.name == db_prefix)
+		).run()
+
+		frappe.msgprint(
+			_("Series counter for {} updated to {} successfully").format(self.prefix, self.current_value),
+			alert=True,
+			indicator="green",
+		)
 
 	@frappe.whitelist()
 	def preview_series(self) -> str:

frappe/model/naming.py

@@ -2,7 +2,7 @@
 # License: MIT. See LICENSE
 
 import re
-from typing import TYPE_CHECKING, Optional, Union
+from typing import TYPE_CHECKING, Callable, List, Optional, Union
 
 import frappe
 from frappe import _
@@ -11,6 +11,7 @@ from frappe.query_builder import DocType
 from frappe.utils import cint, cstr, now_datetime
 
 if TYPE_CHECKING:
+	from frappe.model.document import Document
 	from frappe.model.meta import Meta
 
 
@@ -189,10 +190,28 @@ def make_autoname(key="", doctype="", doc=""):
 	return n
 
 
-def parse_naming_series(parts, doctype="", doc=""):
-	n = ""
+def parse_naming_series(
+	parts: Union[List[str], str],
+	doctype=None,
+	doc: Optional["Document"] = None,
+	number_generator: Optional[Callable[[str, int], str]] = None,
+) -> str:
+
+	"""Parse the naming series and get next name.
+
+	args:
+	        parts: naming series parts (split by `.`)
+	        doc: document to use for series that have parts using fieldnames
+	        number_generator: Use different counter backend other than `tabSeries`. Primarily used for testing.
+	"""
+
+	name = ""
 	if isinstance(parts, str):
 		parts = parts.split(".")
+
+	if not number_generator:
+		number_generator = getseries
+
 	series_set = False
 	today = now_datetime()
 	for e in parts:
@@ -200,7 +219,7 @@ def parse_naming_series(parts, doctype="", doc=""):
 		if e.startswith("#"):
 			if not series_set:
 				digits = len(e)
-				part = getseries(n, digits)
+				part = number_generator(name, digits)
 				series_set = True
 		elif e == "YY":
 			part = today.strftime("%y")
@@ -225,9 +244,37 @@ def parse_naming_series(parts, doctype="", doc=""):
 			part = e
 
 		if isinstance(part, str):
-			n += part
+			name += part
 
-	return n
+	return name
+
+
+def get_naming_series_prefix(series: str) -> str:
+	"""Naming series stores prefix to maintain a counter in DB. This prefix can be used to update counter and/or other validations.
+
+	e.g. `SINV-.YY.-.####` has prefix of `SINV-22-` in database for year 2022.
+	"""
+
+	prefix = None
+
+	if "#" not in series:
+		series += ".#####"
+
+	def fake_counter_backend(partial_series, digits):
+		nonlocal prefix
+		prefix = partial_series
+		return "#" * digits
+
+	# This function evaluates all parts till we hit numerical parts and then
+	# sends prefix + digits to DB to find next number.
+	# Instead of reimplemnted the whole parsing logic in multiple places we can
+	# just ask this function to give us the prefix.
+	parse_naming_series(series, number_generator=fake_counter_backend)
+
+	if prefix is None:
+		frappe.throw(_("Invalid Naming Series"))
+
+	return prefix
 
 
 def determine_consecutive_week_number(datetime):

frappe/tests/test_naming.py

@@ -1,13 +1,12 @@
 # Copyright (c) 2018, Frappe Technologies Pvt. Ltd. and Contributors
 # License: MIT. See LICENSE
 
-import unittest
-
 import frappe
 from frappe.core.doctype.doctype.test_doctype import new_doctype
 from frappe.model.naming import (
 	append_number_if_name_exists,
 	determine_consecutive_week_number,
+	get_naming_series_prefix,
 	getseries,
 	revert_series_if_last,
 )
@@ -288,6 +287,21 @@ class TestNaming(FrappeTestCase):
 
 		dt.delete(ignore_permissions=True)
 
+	def test_naming_series_prefix(self):
+		today = now_datetime()
+		year = today.strftime("%y")
+		month = today.strftime("%m")
+
+		prefix_test_cases = {
+			"SINV-.YY.-.####": f"SINV-{year}-",
+			"SINV-.YY.-.MM.-.####": f"SINV-{year}-{month}-",
+			"SINV": "SINV",
+			"SINV-.": "SINV-",
+		}
+
+		for series, prefix in prefix_test_cases.items():
+			self.assertEqual(prefix, get_naming_series_prefix(series))
+
 
 def make_invalid_todo():
 	frappe.get_doc({"doctype": "ToDo", "description": "Test"}).insert(set_name="ToDo")