diff --git a/erpnext/change_log/current/serial_batch_identity.md b/erpnext/change_log/current/serial_batch_identity.md index 693d97a6b91..f401a28c037 100644 --- a/erpnext/change_log/current/serial_batch_identity.md +++ b/erpnext/change_log/current/serial_batch_identity.md @@ -6,6 +6,7 @@ Physical-number matching and uniqueness ignore letter case on both databases. St Existing document IDs remain unchanged. Migration fills missing physical numbers from those IDs and replaces the global unique indexes. Historical transactions, bundles, and stock valuations retain their references. Item merges fail when they would introduce duplicate numbers. +Before changing either DocType, migration checks for existing numbers that conflict under the new comparison rules. If it finds conflicts, it lists the item and document IDs for correction before retrying the upgrade. It does not merge stock records or change physical labels automatically. When a scanned number matches several items, select the item from the filtered Item field. diff --git a/erpnext/patches.txt b/erpnext/patches.txt index dc1f30710db..44e6bbf0815 100644 --- a/erpnext/patches.txt +++ b/erpnext/patches.txt @@ -262,6 +262,7 @@ erpnext.patches.v15_0.rename_subcontracting_fields erpnext.patches.v15_0.unset_incorrect_additional_discount_percentage erpnext.patches.v16_0.convert_commission_rate_to_percent erpnext.patches.v16_0.convert_hide_currency_symbol_to_check +erpnext.patches.separate_serial_batch_identity [post_model_sync] erpnext.patches.v15_0.rename_gross_purchase_amount_to_net_purchase_amount @@ -520,5 +521,3 @@ erpnext.patches.v16_0.add_transaction_roles_to_sms_settings erpnext.patches.v16_0.set_secondary_item_valuation_type erpnext.patches.v16_0.append_fieldname_to_pos_search_fields erpnext.patches.v16_0.set_supplier_quotation_order_status - -erpnext.patches.separate_serial_batch_identity diff --git a/erpnext/patches/separate_serial_batch_identity.py b/erpnext/patches/separate_serial_batch_identity.py index 31e826957a1..f02611370a2 100644 --- a/erpnext/patches/separate_serial_batch_identity.py +++ b/erpnext/patches/separate_serial_batch_identity.py @@ -4,6 +4,9 @@ from erpnext.stock.serial_batch_identity import SerialBatchIdentity def execute(): + for doctype in ("Serial No", "Batch"): + SerialBatchIdentity(doctype).validate_existing_numbers() + # Reload also drops the former single-field unique indexes on both database engines. for doctype in ("Serial No", "Batch"): frappe.reload_doc("stock", "doctype", frappe.scrub(doctype), force=True) diff --git a/erpnext/stock/serial_batch_identity.py b/erpnext/stock/serial_batch_identity.py index 4692f85c3c7..7c0cb93914b 100644 --- a/erpnext/stock/serial_batch_identity.py +++ b/erpnext/stock/serial_batch_identity.py @@ -3,7 +3,7 @@ import frappe from frappe import _ from frappe.model.naming import make_autoname -from frappe.query_builder.functions import Lower +from frappe.query_builder.functions import Coalesce, Count, Lower, NullIf from frappe.utils import cstr, now @@ -40,9 +40,29 @@ class SerialBatchIdentity: frappe.throw(_("{0} {1} does not exist for Item {2}").format(self.doctype, number, item_code)) ids[number] = name if missing: - ids.update(self.create_many(item_code, missing, defaults)) + ids.update(self.resolve_missing(item_code, missing, defaults)) return [ids[number] for number in numbers] + def resolve_missing(self, item_code, numbers, defaults): + savepoint = "serial_batch_resolve_" + frappe.generate_hash(length=10) + frappe.db.savepoint(savepoint) + try: + ids = self.create_many(item_code, numbers, defaults) + except Exception as error: + frappe.db.rollback(save_point=savepoint) + frappe.db.release_savepoint(savepoint) + if not isinstance(error, frappe.DuplicateEntryError): + raise + else: + frappe.db.release_savepoint(savepoint) + return ids + + # Retry aliases using the database's comparison rules, including MariaDB collation. + return { + number: self.exists(number, item_code) or self.create_many(item_code, [number], defaults)[number] + for number in numbers + } + def get_query(self, numbers, item_code=None, *, fields=None, filters=None, ignore_permissions=True): table = frappe.qb.DocType(self.doctype) filters = {**(filters or {}), **({self.item_field: item_code} if item_code else {})} @@ -66,7 +86,10 @@ class SerialBatchIdentity: def create_many(self, item_code, numbers, defaults=None): if self.doctype == "Batch": - return {number: self.create_batch(item_code, number, defaults) for number in numbers} + return { + number: self.exists(number, item_code) or self.create_batch(item_code, number, defaults) + for number in numbers + } # Inactive serials can be prepared before their first receipt assigns a company. item = frappe.get_cached_value( @@ -95,12 +118,9 @@ class SerialBatchIdentity: ) except Exception as error: if frappe.db.is_unique_key_violation(error) or frappe.db.is_primary_key_violation(error): - frappe.throw( - _("A serial number already exists for Item {0}. Refresh and try again.").format( - item_code - ), - frappe.DuplicateEntryError, - ) + raise frappe.DuplicateEntryError( + _("A serial number already exists for Item {0}. Refresh and try again.").format(item_code) + ) from error raise return ids @@ -142,6 +162,7 @@ class SerialBatchIdentity: ).run() def sync_constraint(self): + self.validate_existing_numbers() self.backfill_numbers() if frappe.db.db_type == "postgres": # The leading number expression also indexes scans without an item filter. @@ -157,6 +178,42 @@ class SerialBatchIdentity: else: frappe.db.add_unique(self.doctype, [self.item_field, self.number_field]) + def validate_existing_numbers(self): + table = frappe.qb.DocType(self.doctype) + # Use the future backfilled value without changing legacy records during the preflight. + number = ( + Coalesce(NullIf(table[self.number_field], ""), table.name) + if frappe.db.has_column(self.doctype, self.number_field) + else table.name + ) + key = self.number_key(number) + duplicates = ( + frappe.qb.from_(table) + .select(table[self.item_field], key) + .groupby(table[self.item_field], key) + .having(Count(table.name) > 1) + ).run() + if not duplicates: + return + + conflicts = [] + for item, value in duplicates: + names = ( + frappe.qb.from_(table) + .select(table.name) + .where((table[self.item_field] == item) & (key == value)) + .orderby(table.name) + ).run(pluck=True) + conflicts.append(_("Item {0}, number {1}: {2}").format(item, value, ", ".join(names))) + frappe.throw( + _( + "Resolve duplicate {0} physical numbers before upgrading. Document IDs and stock references have not been changed." + ).format(self.doctype) + + "\n" + + "\n".join(conflicts), + title=_("Duplicate Serial or Batch Numbers"), + ) + @frappe.whitelist(methods=["POST"]) def resolve_serial_batch_numbers( diff --git a/erpnext/stock/tests/test_serial_batch_identity_matching.py b/erpnext/stock/tests/test_serial_batch_identity_matching.py index 7b0830b570a..b155308af55 100644 --- a/erpnext/stock/tests/test_serial_batch_identity_matching.py +++ b/erpnext/stock/tests/test_serial_batch_identity_matching.py @@ -1,3 +1,5 @@ +from unittest.mock import patch + import frappe from erpnext.stock.doctype.item.test_item import make_item @@ -76,3 +78,53 @@ class TestSerialBatchIdentityMatching(ERPNextTestSuite): item.save() number = frappe.get_doc({"doctype": "Batch", "item": item.name}).insert().batch_id self.assertEqual(number, prefix + "00002") + + def test_create_case_variants_in_one_request(self): + for doctype in ("Serial No", "Batch"): + identity, item, existing = self.make_number(doctype, "Existing-Lot") + numbers = ["New-Lot", "NEW-LOT", "Second-Lot", "new-lot", "existing-lot"] + names = identity.resolve(item.name, numbers, create=True) + self.assertEqual(names[0], names[1]) + self.assertEqual(names[0], names[3]) + self.assertNotEqual(names[0], names[2]) + self.assertEqual(names[4], existing) + self.assertEqual(identity.resolve(item.name, numbers), names) + self.assertEqual(identity.labels(names)[names[0]], "New-Lot") + self.assertEqual(frappe.db.count(doctype, {identity.item_field: item.name}), 3) + + def test_create_aliases_uses_mariadb_collation(self): + if frappe.db.db_type != "mariadb": + self.skipTest("MariaDB's accent-insensitive collation") + for doctype in ("Serial No", "Batch"): + identity, item, _ = self.make_number(doctype, "Existing-Lot") + names = identity.resolve(item.name, ["Café-Lot", "Cafe-Lot"], create=True) + self.assertEqual(names[0], names[1]) + self.assertEqual(identity.labels(names)[names[0]], "Café-Lot") + + def test_migration_reports_case_conflicts_before_changing_records(self): + if frappe.db.db_type != "postgres": + self.skipTest("Legacy case-only duplicates are possible on PostgreSQL") + from erpnext.patches.separate_serial_batch_identity import execute + + for doctype, index in (("Serial No", "serial_no_number_item_ci"), ("Batch", "batch_number_item_ci")): + identity, item, name = self.make_number(doctype, "Legacy-Lot") + frappe.db.savepoint("legacy_case_conflict") + try: + # PostgreSQL DDL is transactional, so rollback restores the unique index. + frappe.db.sql(f'DROP INDEX "{index}"') + duplicate = frappe.get_doc(doctype, name) + duplicate.name = frappe.generate_hash() + duplicate.set(identity.number_field, "LEGACY-LOT") + duplicate.db_insert() + with patch("frappe.reload_doc") as reload_doc: + with self.assertRaises(frappe.ValidationError) as error: + execute() + reload_doc.assert_not_called() + for value in (item.name, name, duplicate.name): + self.assertIn(value, str(error.exception)) + self.assertEqual( + identity.labels([name, duplicate.name]), + {name: "Legacy-Lot", duplicate.name: "LEGACY-LOT"}, + ) + finally: + frappe.db.rollback(save_point="legacy_case_conflict")