Merge pull request #29519 from ankush/unique_bin
fix: bin uniqueness
diff --git a/erpnext/patches.txt b/erpnext/patches.txt
index b04eeb8..9e2ac0a 100644
--- a/erpnext/patches.txt
+++ b/erpnext/patches.txt
@@ -1,5 +1,6 @@
[pre_model_sync]
erpnext.patches.v12_0.update_is_cancelled_field
+erpnext.patches.v13_0.add_bin_unique_constraint
erpnext.patches.v11_0.rename_production_order_to_work_order
erpnext.patches.v11_0.refactor_naming_series
erpnext.patches.v11_0.refactor_autoname_naming
diff --git a/erpnext/patches/v13_0/add_bin_unique_constraint.py b/erpnext/patches/v13_0/add_bin_unique_constraint.py
new file mode 100644
index 0000000..57fbaae
--- /dev/null
+++ b/erpnext/patches/v13_0/add_bin_unique_constraint.py
@@ -0,0 +1,63 @@
+import frappe
+
+from erpnext.stock.stock_balance import (
+ get_balance_qty_from_sle,
+ get_indented_qty,
+ get_ordered_qty,
+ get_planned_qty,
+ get_reserved_qty,
+)
+from erpnext.stock.utils import get_bin
+
+
+def execute():
+ delete_broken_bins()
+ delete_and_patch_duplicate_bins()
+
+def delete_broken_bins():
+ # delete useless bins
+ frappe.db.sql("delete from `tabBin` where item_code is null or warehouse is null")
+
+def delete_and_patch_duplicate_bins():
+
+ duplicate_bins = frappe.db.sql("""
+ SELECT
+ item_code, warehouse, count(*) as bin_count
+ FROM
+ tabBin
+ GROUP BY
+ item_code, warehouse
+ HAVING
+ bin_count > 1
+ """, as_dict=1)
+
+ for duplicate_bin in duplicate_bins:
+ item_code = duplicate_bin.item_code
+ warehouse = duplicate_bin.warehouse
+ existing_bins = frappe.get_list("Bin",
+ filters={
+ "item_code": item_code,
+ "warehouse": warehouse
+ },
+ fields=["name"],
+ order_by="creation",)
+
+ # keep last one
+ existing_bins.pop()
+
+ for broken_bin in existing_bins:
+ frappe.delete_doc("Bin", broken_bin.name)
+
+ qty_dict = {
+ "reserved_qty": get_reserved_qty(item_code, warehouse),
+ "indented_qty": get_indented_qty(item_code, warehouse),
+ "ordered_qty": get_ordered_qty(item_code, warehouse),
+ "planned_qty": get_planned_qty(item_code, warehouse),
+ "actual_qty": get_balance_qty_from_sle(item_code, warehouse)
+ }
+
+ bin = get_bin(item_code, warehouse)
+ bin.update(qty_dict)
+ bin.update_reserved_qty_for_production()
+ bin.update_reserved_qty_for_sub_contracting()
+ bin.db_update()
diff --git a/erpnext/stock/doctype/bin/bin.json b/erpnext/stock/doctype/bin/bin.json
index 8e79f0e..56dc71c 100644
--- a/erpnext/stock/doctype/bin/bin.json
+++ b/erpnext/stock/doctype/bin/bin.json
@@ -33,6 +33,7 @@
"oldfieldtype": "Link",
"options": "Warehouse",
"read_only": 1,
+ "reqd": 1,
"search_index": 1
},
{
@@ -46,6 +47,7 @@
"oldfieldtype": "Link",
"options": "Item",
"read_only": 1,
+ "reqd": 1,
"search_index": 1
},
{
@@ -169,10 +171,11 @@
"idx": 1,
"in_create": 1,
"links": [],
- "modified": "2021-03-30 23:09:39.572776",
+ "modified": "2022-01-30 17:04:54.715288",
"modified_by": "Administrator",
"module": "Stock",
"name": "Bin",
+ "naming_rule": "Expression (old style)",
"owner": "Administrator",
"permissions": [
{
@@ -200,5 +203,6 @@
"quick_entry": 1,
"search_fields": "item_code,warehouse",
"sort_field": "modified",
- "sort_order": "ASC"
+ "sort_order": "ASC",
+ "states": []
}
\ No newline at end of file
diff --git a/erpnext/stock/doctype/bin/bin.py b/erpnext/stock/doctype/bin/bin.py
index 0ef7ce2..c34e9d0 100644
--- a/erpnext/stock/doctype/bin/bin.py
+++ b/erpnext/stock/doctype/bin/bin.py
@@ -123,7 +123,7 @@
self.db_set('projected_qty', self.projected_qty)
def on_doctype_update():
- frappe.db.add_index("Bin", ["item_code", "warehouse"])
+ frappe.db.add_unique("Bin", ["item_code", "warehouse"], constraint_name="unique_item_warehouse")
def update_stock(bin_name, args, allow_negative_stock=False, via_landed_cost_voucher=False):
diff --git a/erpnext/stock/doctype/bin/test_bin.py b/erpnext/stock/doctype/bin/test_bin.py
index 9c390d9..250126c 100644
--- a/erpnext/stock/doctype/bin/test_bin.py
+++ b/erpnext/stock/doctype/bin/test_bin.py
@@ -1,9 +1,36 @@
# Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors
# See license.txt
-import unittest
+import frappe
-# test_records = frappe.get_test_records('Bin')
+from erpnext.stock.doctype.item.test_item import make_item
+from erpnext.stock.utils import _create_bin
+from erpnext.tests.utils import ERPNextTestCase
-class TestBin(unittest.TestCase):
- pass
+
+class TestBin(ERPNextTestCase):
+
+
+ def test_concurrent_inserts(self):
+ """ Ensure no duplicates are possible in case of concurrent inserts"""
+ item_code = "_TestConcurrentBin"
+ make_item(item_code)
+ warehouse = "_Test Warehouse - _TC"
+
+ bin1 = frappe.get_doc(doctype="Bin", item_code=item_code, warehouse=warehouse)
+ bin1.insert()
+
+ bin2 = frappe.get_doc(doctype="Bin", item_code=item_code, warehouse=warehouse)
+ with self.assertRaises(frappe.UniqueValidationError):
+ bin2.insert()
+
+ # util method should handle it
+ bin = _create_bin(item_code, warehouse)
+ self.assertEqual(bin.item_code, item_code)
+
+ frappe.db.rollback()
+
+ def test_index_exists(self):
+ indexes = frappe.db.sql("show index from tabBin where Non_unique = 0", as_dict=1)
+ if not any(index.get("Key_name") == "unique_item_warehouse" for index in indexes):
+ self.fail(f"Expected unique index on item-warehouse")
diff --git a/erpnext/stock/doctype/stock_entry/stock_entry.py b/erpnext/stock/doctype/stock_entry/stock_entry.py
index 60154af..c51c9bc 100644
--- a/erpnext/stock/doctype/stock_entry/stock_entry.py
+++ b/erpnext/stock/doctype/stock_entry/stock_entry.py
@@ -1673,6 +1673,8 @@
for d in self.get("items"):
item_code = d.get('original_item') or d.get('item_code')
reserve_warehouse = item_wh.get(item_code)
+ if not (reserve_warehouse and item_code):
+ continue
stock_bin = get_bin(item_code, reserve_warehouse)
stock_bin.update_reserved_qty_for_sub_contracting()
diff --git a/erpnext/stock/utils.py b/erpnext/stock/utils.py
index f620c18..7c63c17 100644
--- a/erpnext/stock/utils.py
+++ b/erpnext/stock/utils.py
@@ -176,13 +176,7 @@
def get_bin(item_code, warehouse):
bin = frappe.db.get_value("Bin", {"item_code": item_code, "warehouse": warehouse})
if not bin:
- bin_obj = frappe.get_doc({
- "doctype": "Bin",
- "item_code": item_code,
- "warehouse": warehouse,
- })
- bin_obj.flags.ignore_permissions = 1
- bin_obj.insert()
+ bin_obj = _create_bin(item_code, warehouse)
else:
bin_obj = frappe.get_doc('Bin', bin, for_update=True)
bin_obj.flags.ignore_permissions = True
@@ -192,16 +186,24 @@
bin_record = frappe.db.get_value('Bin', {'item_code': item_code, 'warehouse': warehouse})
if not bin_record:
- bin_obj = frappe.get_doc({
- "doctype": "Bin",
- "item_code": item_code,
- "warehouse": warehouse,
- })
+ bin_obj = _create_bin(item_code, warehouse)
+ bin_record = bin_obj.name
+ return bin_record
+
+def _create_bin(item_code, warehouse):
+ """Create a bin and take care of concurrent inserts."""
+
+ bin_creation_savepoint = "create_bin"
+ try:
+ frappe.db.savepoint(bin_creation_savepoint)
+ bin_obj = frappe.get_doc(doctype="Bin", item_code=item_code, warehouse=warehouse)
bin_obj.flags.ignore_permissions = 1
bin_obj.insert()
- bin_record = bin_obj.name
+ except frappe.UniqueValidationError:
+ frappe.db.rollback(save_point=bin_creation_savepoint) # preserve transaction in postgres
+ bin_obj = frappe.get_last_doc("Bin", {"item_code": item_code, "warehouse": warehouse})
- return bin_record
+ return bin_obj
def update_bin(args, allow_negative_stock=False, via_landed_cost_voucher=False):
"""WARNING: This function is deprecated. Inline this function instead of using it."""