diff --git a/frappe/core/doctype/document_naming_settings/document_naming_settings.py b/frappe/core/doctype/document_naming_settings/document_naming_settings.py index 0402ea90ea..01a230b833 100644 --- a/frappe/core/doctype/document_naming_settings/document_naming_settings.py +++ b/frappe/core/doctype/document_naming_settings/document_naming_settings.py @@ -1,13 +1,14 @@ # Copyright (c) 2022, Frappe Technologies and contributors # For license information, please see license.txt +import re from typing import List 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 get_naming_series_prefix, make_autoname +from frappe.model.naming import NamingSeries, make_autoname from frappe.permissions import get_doctypes_with_read from frappe.utils import cint @@ -80,8 +81,8 @@ class DocumentNamingSettings(Document): options = self.get_options_list(options) # validate names - for i in options: - self.validate_series_name(i) + for series in options: + self.validate_series_name(series) if options and self.user_must_always_select: options = [""] + options @@ -125,13 +126,8 @@ class DocumentNamingSettings(Document): frappe.throw(_("Series {0} already used in {1}").format(series, existing_series[series])) validate_series(dt, series) - def validate_series_name(self, n): - import re - - if not re.match(r"^[\w\- \/.#{}]+$", n, re.UNICODE): - frappe.throw( - _('Special Characters except "-", "#", ".", "/", "{" and "}" not allowed in naming series') - ) + def validate_series_name(self, series): + NamingSeries(series).validate() @frappe.whitelist() def get_options(self, doctype=None): @@ -146,7 +142,7 @@ class DocumentNamingSettings(Document): def get_current(self): """get series current""" if self.prefix: - prefix = get_naming_series_prefix(self.prefix) + prefix = NamingSeries(self.prefix).get_prefix() self.current_value = frappe.db.get_value("Series", prefix, "current", order_by="name") @frappe.whitelist() @@ -156,7 +152,7 @@ class DocumentNamingSettings(Document): series = frappe.qb.DocType("Series") - db_prefix = get_naming_series_prefix(self.prefix) + db_prefix = NamingSeries(self.prefix).get_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() diff --git a/frappe/core/doctype/document_naming_settings/test_document_naming_settings.py b/frappe/core/doctype/document_naming_settings/test_document_naming_settings.py index f9e687e10d..85ecc3b5d9 100644 --- a/frappe/core/doctype/document_naming_settings/test_document_naming_settings.py +++ b/frappe/core/doctype/document_naming_settings/test_document_naming_settings.py @@ -3,6 +3,7 @@ import frappe from frappe.core.doctype.document_naming_settings.document_naming_settings import ( + NAMING_SERIES_PATTERN, DocumentNamingSettings, ) from frappe.model.naming import get_default_naming_series @@ -11,24 +12,24 @@ from frappe.tests.utils import FrappeTestCase class TestNamingSeries(FrappeTestCase): def setUp(self): - self.ns: DocumentNamingSettings = frappe.get_doc("Document Naming Settings") + self.dns: DocumentNamingSettings = frappe.get_doc("Document Naming Settings") def tearDown(self): frappe.db.rollback() def test_naming_preview(self): - self.ns.transaction_type = "Webhook" + self.dns.transaction_type = "Webhook" - self.ns.try_naming_series = "AXBZ.####" - serieses = self.ns.preview_series().split("\n") + self.dns.try_naming_series = "AXBZ.####" + serieses = self.dns.preview_series().split("\n") self.assertEqual(["AXBZ0001", "AXBZ0002", "AXBZ0003"], serieses) - self.ns.try_naming_series = "AXBZ-.{currency}.-" - serieses = self.ns.preview_series().split("\n") + self.dns.try_naming_series = "AXBZ-.{currency}.-" + serieses = self.dns.preview_series().split("\n") def test_get_transactions(self): - naming_info = self.ns.get_transactions_and_prefixes() + naming_info = self.dns.get_transactions_and_prefixes() self.assertIn("Webhook", naming_info["transactions"]) existing_naming_series = frappe.get_meta("Webhook").get_field("naming_series").options diff --git a/frappe/model/naming.py b/frappe/model/naming.py index 9ffb72b09c..50535f0a99 100644 --- a/frappe/model/naming.py +++ b/frappe/model/naming.py @@ -19,6 +19,67 @@ if TYPE_CHECKING: # whether `log_types` have autoincremented naming set for the site or not. autoincremented_site_status_map = {} +NAMING_SERIES_PATTERN = re.compile(r"^[\w\- \/.#{}]+$", re.UNICODE) + + +class InvalidNamingSeriesError(frappe.ValidationError): + pass + + +class NamingSeries: + __slots__ = ("series",) + + def __init__(self, series: str): + self.series = series + + # Add default number part if missing + if "#" not in self.series: + self.series += ".#####" + + def validate(self): + if "." not in self.series: + frappe.throw( + _("Invalid naming series {}: dot (.) missing").format(frappe.bold(self.series)), + exc=InvalidNamingSeriesError, + ) + + if not NAMING_SERIES_PATTERN.match(self.series): + frappe.throw( + _( + 'Special Characters except "-", "#", ".", "/", "{" and "}" not allowed in naming series', + ), + exc=InvalidNamingSeriesError, + ) + + def generate_next_name(self, doc: "Document") -> str: + self.validate() + parts = self.series.split(".") + return parse_naming_series(parts, doc) + + def get_prefix(self) -> str: + """Naming series stores prefix to maintain a counter in DB. This prefix can be used to update counter or validations. + + e.g. `SINV-.YY.-.####` has prefix of `SINV-22-` in database for year 2022. + """ + + prefix = None + + 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 reimplementing the whole parsing logic in multiple places we + # can just ask this function to give us the prefix. + parse_naming_series(self.series, number_generator=fake_counter_backend) + + if prefix is None: + frappe.throw(_("Invalid Naming Series")) + + return prefix + def set_new_name(doc): """ @@ -176,18 +237,8 @@ def make_autoname(key="", doctype="", doc=""): if key == "hash": return frappe.generate_hash(doctype, 10) - if "#" not in key: - key = key + ".#####" - elif "." not in key: - error_message = _("Invalid naming series (. missing)") - if doctype: - error_message = _("Invalid naming series (. missing) for {0}").format(doctype) - - frappe.throw(error_message) - - parts = key.split(".") - n = parse_naming_series(parts, doctype, doc) - return n + series = NamingSeries(key) + return series.generate_next_name(doc) def parse_naming_series( @@ -249,34 +300,6 @@ def parse_naming_series( 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): """Determines the consecutive calendar week""" m = datetime.month diff --git a/frappe/tests/test_naming.py b/frappe/tests/test_naming.py index cad91c11a3..a1e023461a 100644 --- a/frappe/tests/test_naming.py +++ b/frappe/tests/test_naming.py @@ -4,9 +4,10 @@ import frappe from frappe.core.doctype.doctype.test_doctype import new_doctype from frappe.model.naming import ( + InvalidNamingSeriesError, + NamingSeries, append_number_if_name_exists, determine_consecutive_week_number, - get_naming_series_prefix, getseries, revert_series_if_last, ) @@ -300,7 +301,23 @@ class TestNaming(FrappeTestCase): } for series, prefix in prefix_test_cases.items(): - self.assertEqual(prefix, get_naming_series_prefix(series)) + self.assertEqual(prefix, NamingSeries(series).get_prefix()) + + def test_naming_series_validation(self): + dns = frappe.get_doc("Document Naming Settings") + exisiting_series = dns.get_transactions_and_prefixes()["prefixes"] + valid = ["SINV-", "SI-.{field}.", "SI-#.###", ""] + exisiting_series + invalid = ["$INV-", r"WINDOWS\NAMING"] + + for series in valid: + if series.strip(): + try: + NamingSeries(series).validate() + except Exception as e: + self.fail(f"{series} should be valid\n{e}") + + for series in invalid: + self.assertRaises(InvalidNamingSeriesError, NamingSeries(series).validate) def make_invalid_todo():