mirror of
https://github.com/frappe/erpnext.git
synced 2026-09-09 15:29:30 +00:00
fix(stock): resolve number aliases and detect migration conflicts
This commit is contained in:
@@ -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.
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -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")
|
||||
|
||||
Reference in New Issue
Block a user