From 39125a78e0e6e86282d32f1cb2801c12c86f0245 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Tue, 14 Dec 2021 17:45:08 +0530 Subject: [PATCH] fix!: dont allow renaming warehouse primary key (backport #28712) * fix!: dont allow renaming warehouse primary key (cherry picked from commit 72dbc3d6b8e10fc6ee9f7cd8132da90fe9cdb3bb) * fix: remove autocommit from item rename (cherry picked from commit 5caf411be3ffcb5638cf2d3a3cedca233ec9f317) Co-authored-by: Ankush Menat --- erpnext/stock/doctype/item/item.py | 2 - .../stock/doctype/warehouse/test_warehouse.py | 60 +------------------ .../stock/doctype/warehouse/warehouse.json | 3 +- erpnext/stock/doctype/warehouse/warehouse.py | 54 +---------------- 4 files changed, 4 insertions(+), 115 deletions(-) diff --git a/erpnext/stock/doctype/item/item.py b/erpnext/stock/doctype/item/item.py index 917854a3ee0..0ee30d86e9a 100644 --- a/erpnext/stock/doctype/item/item.py +++ b/erpnext/stock/doctype/item/item.py @@ -690,7 +690,6 @@ class Item(WebsiteGenerator): def recalculate_bin_qty(self, new_name): from erpnext.stock.stock_balance import repost_stock - frappe.db.auto_commit_on_many_writes = 1 existing_allow_negative_stock = frappe.db.get_value("Stock Settings", None, "allow_negative_stock") frappe.db.set_value("Stock Settings", None, "allow_negative_stock", 1) @@ -704,7 +703,6 @@ class Item(WebsiteGenerator): repost_stock(new_name, warehouse) frappe.db.set_value("Stock Settings", None, "allow_negative_stock", existing_allow_negative_stock) - frappe.db.auto_commit_on_many_writes = 0 def copy_specification_from_item_group(self): self.set("website_specifications", []) diff --git a/erpnext/stock/doctype/warehouse/test_warehouse.py b/erpnext/stock/doctype/warehouse/test_warehouse.py index 121222d7bc3..4ba87ac5fb8 100644 --- a/erpnext/stock/doctype/warehouse/test_warehouse.py +++ b/erpnext/stock/doctype/warehouse/test_warehouse.py @@ -32,64 +32,6 @@ class TestWarehouse(unittest.TestCase): self.assertEqual(p_warehouse.name, child_warehouse.parent_warehouse) self.assertEqual(child_warehouse.is_group, 0) - def test_warehouse_renaming(self): - set_perpetual_inventory(1) - create_warehouse("Test Warehouse for Renaming 1") - account = get_inventory_account("_Test Company", "Test Warehouse for Renaming 1 - _TC") - self.assertTrue(frappe.db.get_value("Warehouse", filters={"account": account})) - - # Rename with abbr - if frappe.db.exists("Warehouse", "Test Warehouse for Renaming 2 - _TC"): - frappe.delete_doc("Warehouse", "Test Warehouse for Renaming 2 - _TC") - rename_doc("Warehouse", "Test Warehouse for Renaming 1 - _TC", "Test Warehouse for Renaming 2 - _TC") - - self.assertTrue(frappe.db.get_value("Warehouse", - filters={"account": "Test Warehouse for Renaming 1 - _TC"})) - - # Rename without abbr - if frappe.db.exists("Warehouse", "Test Warehouse for Renaming 3 - _TC"): - frappe.delete_doc("Warehouse", "Test Warehouse for Renaming 3 - _TC") - - rename_doc("Warehouse", "Test Warehouse for Renaming 2 - _TC", "Test Warehouse for Renaming 3") - - self.assertTrue(frappe.db.get_value("Warehouse", - filters={"account": "Test Warehouse for Renaming 1 - _TC"})) - - # Another rename with multiple dashes - if frappe.db.exists("Warehouse", "Test - Warehouse - Company - _TC"): - frappe.delete_doc("Warehouse", "Test - Warehouse - Company - _TC") - rename_doc("Warehouse", "Test Warehouse for Renaming 3 - _TC", "Test - Warehouse - Company") - - def test_warehouse_merging(self): - set_perpetual_inventory(1) - - create_warehouse("Test Warehouse for Merging 1") - create_warehouse("Test Warehouse for Merging 2") - - make_stock_entry(item_code="_Test Item", target="Test Warehouse for Merging 1 - _TC", - qty=1, rate=100) - make_stock_entry(item_code="_Test Item", target="Test Warehouse for Merging 2 - _TC", - qty=1, rate=100) - - existing_bin_qty = ( - cint(frappe.db.get_value("Bin", - {"item_code": "_Test Item", "warehouse": "Test Warehouse for Merging 1 - _TC"}, "actual_qty")) - + cint(frappe.db.get_value("Bin", - {"item_code": "_Test Item", "warehouse": "Test Warehouse for Merging 2 - _TC"}, "actual_qty")) - ) - - rename_doc("Warehouse", "Test Warehouse for Merging 1 - _TC", - "Test Warehouse for Merging 2 - _TC", merge=True) - - self.assertFalse(frappe.db.exists("Warehouse", "Test Warehouse for Merging 1 - _TC")) - - bin_qty = frappe.db.get_value("Bin", - {"item_code": "_Test Item", "warehouse": "Test Warehouse for Merging 2 - _TC"}, "actual_qty") - - self.assertEqual(bin_qty, existing_bin_qty) - - self.assertTrue(frappe.db.get_value("Warehouse", - filters={"account": "Test Warehouse for Merging 2 - _TC"})) def create_warehouse(warehouse_name, properties=None, company=None): if not company: @@ -145,4 +87,4 @@ def get_group_stock_account(company, company_abbr=None): if not company_abbr: company_abbr = frappe.get_cached_value("Company", company, 'abbr') group_stock_account = "Current Assets - " + company_abbr - return group_stock_account \ No newline at end of file + return group_stock_account diff --git a/erpnext/stock/doctype/warehouse/warehouse.json b/erpnext/stock/doctype/warehouse/warehouse.json index 6e40bc53d07..f66bf334cfb 100644 --- a/erpnext/stock/doctype/warehouse/warehouse.json +++ b/erpnext/stock/doctype/warehouse/warehouse.json @@ -1,6 +1,5 @@ { "allow_import": 1, - "allow_rename": 1, "creation": "2013-03-07 18:50:32", "description": "A logical Warehouse against which stock entries are made.", "doctype": "DocType", @@ -235,7 +234,7 @@ "idx": 1, "is_tree": 1, "links": [], - "modified": "2020-08-03 18:41:52.442502", + "modified": "2021-12-03 04:40:06.414630", "modified_by": "Administrator", "module": "Stock", "name": "Warehouse", diff --git a/erpnext/stock/doctype/warehouse/warehouse.py b/erpnext/stock/doctype/warehouse/warehouse.py index 5a55635d334..f953083d62e 100644 --- a/erpnext/stock/doctype/warehouse/warehouse.py +++ b/erpnext/stock/doctype/warehouse/warehouse.py @@ -7,6 +7,7 @@ from frappe.utils import cint, flt from frappe import throw, _ from collections import defaultdict from frappe.utils.nestedset import NestedSet + from erpnext.stock import get_warehouse_account from frappe.contacts.address_and_contact import load_address_and_contact @@ -63,57 +64,6 @@ class Warehouse(NestedSet): return frappe.db.sql("""select name from `tabWarehouse` where parent_warehouse = %s limit 1""", self.name) - def before_rename(self, old_name, new_name, merge=False): - super(Warehouse, self).before_rename(old_name, new_name, merge) - - # Add company abbr if not provided - new_warehouse = erpnext.encode_company_abbr(new_name, self.company) - - if merge: - if not frappe.db.exists("Warehouse", new_warehouse): - frappe.throw(_("Warehouse {0} does not exist").format(new_warehouse)) - - if self.company != frappe.db.get_value("Warehouse", new_warehouse, "company"): - frappe.throw(_("Both Warehouse must belong to same Company")) - - return new_warehouse - - def after_rename(self, old_name, new_name, merge=False): - super(Warehouse, self).after_rename(old_name, new_name, merge) - - new_warehouse_name = self.get_new_warehouse_name_without_abbr(new_name) - self.db_set("warehouse_name", new_warehouse_name) - - if merge: - self.recalculate_bin_qty(new_name) - - def get_new_warehouse_name_without_abbr(self, name): - company_abbr = frappe.get_cached_value('Company', self.company, "abbr") - parts = name.rsplit(" - ", 1) - - if parts[-1].lower() == company_abbr.lower(): - name = parts[0] - - return name - - def recalculate_bin_qty(self, new_name): - from erpnext.stock.stock_balance import repost_stock - frappe.db.auto_commit_on_many_writes = 1 - existing_allow_negative_stock = frappe.db.get_value("Stock Settings", None, "allow_negative_stock") - frappe.db.set_value("Stock Settings", None, "allow_negative_stock", 1) - - repost_stock_for_items = frappe.db.sql_list("""select distinct item_code - from tabBin where warehouse=%s""", new_name) - - # Delete all existing bins to avoid duplicate bins for the same item and warehouse - frappe.db.sql("delete from `tabBin` where warehouse=%s", new_name) - - for item_code in repost_stock_for_items: - repost_stock(item_code, new_name) - - frappe.db.set_value("Stock Settings", None, "allow_negative_stock", existing_allow_negative_stock) - frappe.db.auto_commit_on_many_writes = 0 - def convert_to_group_or_ledger(self): if self.is_group: self.convert_to_ledger() @@ -232,4 +182,4 @@ def get_warehouses_based_on_account(account, company=None): frappe.throw(_("Warehouse not found against the account {0}") .format(account)) - return warehouses \ No newline at end of file + return warehouses