diff --git a/erpnext/patches.txt b/erpnext/patches.txt index 704b6696db1..b5ee1a4ce8c 100644 --- a/erpnext/patches.txt +++ b/erpnext/patches.txt @@ -1,4 +1,5 @@ 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 00000000000..57fbaae9d8d --- /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 8e79f0e5552..56dc71c57e1 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 11ff359b483..1d874cd06fb 100644 --- a/erpnext/stock/doctype/bin/bin.py +++ b/erpnext/stock/doctype/bin/bin.py @@ -96,7 +96,7 @@ class Bin(Document): 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 9c390d94b4e..250126c6b98 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 ab106e036f1..a5bf2397411 100644 --- a/erpnext/stock/doctype/stock_entry/stock_entry.py +++ b/erpnext/stock/doctype/stock_entry/stock_entry.py @@ -1674,6 +1674,8 @@ class StockEntry(StockController): 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 4d77367a051..b8bdf39301e 100644 --- a/erpnext/stock/utils.py +++ b/erpnext/stock/utils.py @@ -177,13 +177,7 @@ def get_latest_stock_balance(): 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 @@ -193,16 +187,24 @@ def get_or_make_bin(item_code: str , warehouse: str) -> str: 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."""