From 08810dbcce3c96855c2c5bfe266ba9d7ea61fc42 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sun, 30 Jan 2022 16:25:42 +0530 Subject: [PATCH 1/5] fix: add unique constraint on bin at db level --- erpnext/stock/doctype/bin/bin.json | 8 ++++-- erpnext/stock/doctype/bin/bin.py | 2 +- erpnext/stock/doctype/bin/test_bin.py | 35 ++++++++++++++++++++++++--- erpnext/stock/utils.py | 30 ++++++++++++----------- 4 files changed, 54 insertions(+), 21 deletions(-) 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 0ef7ce29230..c34e9d05cef 100644 --- a/erpnext/stock/doctype/bin/bin.py +++ b/erpnext/stock/doctype/bin/bin.py @@ -123,7 +123,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/utils.py b/erpnext/stock/utils.py index f620c183ebb..7c63c17ad03 100644 --- a/erpnext/stock/utils.py +++ b/erpnext/stock/utils.py @@ -176,13 +176,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 @@ -192,16 +186,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.""" From a1e7771cdd7713c64ab525ba7955a139712c727e Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sun, 30 Jan 2022 18:06:23 +0530 Subject: [PATCH 2/5] fix: ignore None item/wh whie updating reservation --- erpnext/stock/doctype/stock_entry/stock_entry.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/erpnext/stock/doctype/stock_entry/stock_entry.py b/erpnext/stock/doctype/stock_entry/stock_entry.py index 60154afa15c..c51c9bc5f47 100644 --- a/erpnext/stock/doctype/stock_entry/stock_entry.py +++ b/erpnext/stock/doctype/stock_entry/stock_entry.py @@ -1673,6 +1673,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() From 7ff3ca25e5b8916d9ebb375f7574dc1a5ebdd4ff Mon Sep 17 00:00:00 2001 From: Sagar Sharma Date: Sun, 30 Jan 2022 18:10:43 +0530 Subject: [PATCH 3/5] fix(patch): patch duplicate bins --- erpnext/patches.txt | 1 + .../v13_0/add_bin_unique_constraint.py | 45 +++++++++++++++++++ 2 files changed, 46 insertions(+) create mode 100644 erpnext/patches/v13_0/add_bin_unique_constraint.py diff --git a/erpnext/patches.txt b/erpnext/patches.txt index ad5062fc0ef..1593ff09796 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 00000000000..29ae631be85 --- /dev/null +++ b/erpnext/patches/v13_0/add_bin_unique_constraint.py @@ -0,0 +1,45 @@ +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, + update_bin_qty, +) + + +def execute(): + + duplicate_rows = frappe.db.sql(""" + SELECT + item_code, warehouse + FROM + tabBin + GROUP BY + item_code, warehouse + HAVING + COUNT(*) > 1 + """, as_dict=1) + + for row in duplicate_rows: + bins = frappe.get_list("Bin", + filters={"item_code": row.item_code, + "warehouse": row.warehouse}, + fields=["name"], + order_by="creation", + ) + + for x in range(len(bins) - 1): + frappe.delete_doc("Bin", bins[x].name) + + qty_dict = { + "reserved_qty": get_reserved_qty(row.item_code, row.warehouse), + "indented_qty": get_indented_qty(row.item_code, row.warehouse), + "ordered_qty": get_ordered_qty(row.item_code, row.warehouse), + "planned_qty": get_planned_qty(row.item_code, row.warehouse), + "actual_qty": get_balance_qty_from_sle(row.item_code, row.warehouse) + } + + update_bin_qty(row.item_code, row.warehouse, qty_dict) From c2ecc7a2d1da839423fd768821b1f77ddcf7f53d Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sun, 30 Jan 2022 18:16:14 +0530 Subject: [PATCH 4/5] refactor: patch for fixing broken bins fix(patch): delete fully broken bins if bin doesn't have item_code or warehouse then it's not recoverable. --- .../v13_0/add_bin_unique_constraint.py | 48 ++++++++++++------- 1 file changed, 30 insertions(+), 18 deletions(-) diff --git a/erpnext/patches/v13_0/add_bin_unique_constraint.py b/erpnext/patches/v13_0/add_bin_unique_constraint.py index 29ae631be85..979fcf5a4f8 100644 --- a/erpnext/patches/v13_0/add_bin_unique_constraint.py +++ b/erpnext/patches/v13_0/add_bin_unique_constraint.py @@ -11,35 +11,47 @@ from erpnext.stock.stock_balance import ( def execute(): + delete_broken_bins() + delete_and_patch_duplicate_bins() - duplicate_rows = frappe.db.sql(""" +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 + item_code, warehouse, count(*) as bin_count FROM tabBin GROUP BY item_code, warehouse HAVING - COUNT(*) > 1 + bin_count > 1 """, as_dict=1) - for row in duplicate_rows: - bins = frappe.get_list("Bin", - filters={"item_code": row.item_code, - "warehouse": row.warehouse}, - fields=["name"], - order_by="creation", - ) + for duplicate_bin in duplicate_bins: + existing_bins = frappe.get_list("Bin", + filters={ + "item_code": duplicate_bin.item_code, + "warehouse": duplicate_bin.warehouse + }, + fields=["name"], + order_by="creation",) - for x in range(len(bins) - 1): - frappe.delete_doc("Bin", bins[x].name) + # 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(row.item_code, row.warehouse), - "indented_qty": get_indented_qty(row.item_code, row.warehouse), - "ordered_qty": get_ordered_qty(row.item_code, row.warehouse), - "planned_qty": get_planned_qty(row.item_code, row.warehouse), - "actual_qty": get_balance_qty_from_sle(row.item_code, row.warehouse) + "reserved_qty": get_reserved_qty(duplicate_bin.item_code, duplicate_bin.warehouse), + "indented_qty": get_indented_qty(duplicate_bin.item_code, duplicate_bin.warehouse), + "ordered_qty": get_ordered_qty(duplicate_bin.item_code, duplicate_bin.warehouse), + "planned_qty": get_planned_qty(duplicate_bin.item_code, duplicate_bin.warehouse), + "actual_qty": get_balance_qty_from_sle(duplicate_bin.item_code, duplicate_bin.warehouse) } - update_bin_qty(row.item_code, row.warehouse, qty_dict) + update_bin_qty(duplicate_bin.item_code, duplicate_bin.warehouse, qty_dict) From 0a1533446495f27445afbcb9d58c30d4d31e7b20 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Mon, 31 Jan 2022 14:10:42 +0530 Subject: [PATCH 5/5] fix: update reserved qty for production/ s/c --- .../v13_0/add_bin_unique_constraint.py | 24 ++++++++++++------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/erpnext/patches/v13_0/add_bin_unique_constraint.py b/erpnext/patches/v13_0/add_bin_unique_constraint.py index 979fcf5a4f8..57fbaae9d8d 100644 --- a/erpnext/patches/v13_0/add_bin_unique_constraint.py +++ b/erpnext/patches/v13_0/add_bin_unique_constraint.py @@ -6,8 +6,8 @@ from erpnext.stock.stock_balance import ( get_ordered_qty, get_planned_qty, get_reserved_qty, - update_bin_qty, ) +from erpnext.stock.utils import get_bin def execute(): @@ -32,10 +32,12 @@ def delete_and_patch_duplicate_bins(): """, 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": duplicate_bin.item_code, - "warehouse": duplicate_bin.warehouse + "item_code": item_code, + "warehouse": warehouse }, fields=["name"], order_by="creation",) @@ -47,11 +49,15 @@ def delete_and_patch_duplicate_bins(): frappe.delete_doc("Bin", broken_bin.name) qty_dict = { - "reserved_qty": get_reserved_qty(duplicate_bin.item_code, duplicate_bin.warehouse), - "indented_qty": get_indented_qty(duplicate_bin.item_code, duplicate_bin.warehouse), - "ordered_qty": get_ordered_qty(duplicate_bin.item_code, duplicate_bin.warehouse), - "planned_qty": get_planned_qty(duplicate_bin.item_code, duplicate_bin.warehouse), - "actual_qty": get_balance_qty_from_sle(duplicate_bin.item_code, duplicate_bin.warehouse) + "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) } - update_bin_qty(duplicate_bin.item_code, duplicate_bin.warehouse, qty_dict) + 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()