From 3232f944e08c50feb3c281c440baf51b5be28356 Mon Sep 17 00:00:00 2001 From: Saqib Date: Thu, 16 Sep 2021 17:31:37 +0530 Subject: [PATCH] fix: no validation on item defaults (#27393) * fix: no validation on item defaults * fix: cache value while validating * test: item default company relation * fix: reorder validations * refactor: add guard conditions on update_defaults * test: add default warehouse for item group * fix: validate item defaults for item groups Co-authored-by: Ankush Menat (cherry picked from commit 5eba1ccd51488a4b5d02382b94d659c71e73e36d) # Conflicts: # erpnext/setup/doctype/item_group/item_group.py # erpnext/stock/doctype/item/item.py # erpnext/stock/doctype/item/test_item.py --- .../setup/doctype/item_group/item_group.py | 15 +++++ erpnext/stock/doctype/item/item.py | 57 +++++++++++++++++++ erpnext/stock/doctype/item/test_item.py | 16 ++++++ 3 files changed, 88 insertions(+) diff --git a/erpnext/setup/doctype/item_group/item_group.py b/erpnext/setup/doctype/item_group/item_group.py index 36b64c55ddc..58204fc7cb3 100644 --- a/erpnext/setup/doctype/item_group/item_group.py +++ b/erpnext/setup/doctype/item_group/item_group.py @@ -37,6 +37,7 @@ class ItemGroup(NestedSet, WebsiteGenerator): self.make_route() self.validate_item_group_defaults() +<<<<<<< HEAD self.check_item_tax() ECommerceSettings.validate_field_filters(self.filter_fields, enable_field_filters=True) @@ -54,6 +55,8 @@ class ItemGroup(NestedSet, WebsiteGenerator): ) else: check_list.append((d.item_tax_template, d.tax_category)) +======= +>>>>>>> 5eba1ccd51 (fix: no validation on item defaults (#27393)) def on_update(self): NestedSet.on_update(self) @@ -122,6 +125,18 @@ class ItemGroup(NestedSet, WebsiteGenerator): def validate_item_group_defaults(self): from erpnext.stock.doctype.item.item import validate_item_default_company_links +<<<<<<< HEAD +======= + validate_item_default_company_links(self.item_group_defaults) + +@frappe.whitelist(allow_guest=True) +def get_product_list_for_group(product_group=None, start=0, limit=10, search=None): + if product_group: + item_group = frappe.get_cached_doc('Item Group', product_group) + if item_group.is_group: + # return child item groups if the type is of "Is Group" + return get_child_groups_for_list_in_html(item_group, start, limit, search) +>>>>>>> 5eba1ccd51 (fix: no validation on item defaults (#27393)) validate_item_default_company_links(self.item_group_defaults) diff --git a/erpnext/stock/doctype/item/item.py b/erpnext/stock/doctype/item/item.py index aa9b21ced17..d1573bdb01f 100644 --- a/erpnext/stock/doctype/item/item.py +++ b/erpnext/stock/doctype/item/item.py @@ -30,7 +30,14 @@ from erpnext.controllers.item_variant import ( make_variant_item_code, validate_item_variant_attributes, ) +<<<<<<< HEAD from erpnext.setup.doctype.item_group.item_group import invalidate_cache_for +======= +from erpnext.setup.doctype.item_group.item_group import ( + get_parent_item_groups, + invalidate_cache_for, +) +>>>>>>> 5eba1ccd51 (fix: no validation on item defaults (#27393)) from erpnext.stock.doctype.item_default.item_default import ItemDefault @@ -649,11 +656,16 @@ class Item(Document): validate_item_default_company_links(self.item_defaults) +<<<<<<< HEAD +======= + +>>>>>>> 5eba1ccd51 (fix: no validation on item defaults (#27393)) def update_defaults_from_item_group(self): """Get defaults from Item Group""" if self.item_defaults or not self.item_group: return +<<<<<<< HEAD item_defaults = frappe.db.get_values( "Item Default", {"parent": self.item_group}, @@ -684,10 +696,28 @@ class Item(Document): "income_account": item.income_account, }, ) +======= + item_defaults = frappe.db.get_values("Item Default", {"parent": self.item_group}, + ['company', 'default_warehouse','default_price_list','buying_cost_center','default_supplier', + 'expense_account','selling_cost_center','income_account'], as_dict = 1) + if item_defaults: + for item in item_defaults: + self.append('item_defaults', { + 'company': item.company, + 'default_warehouse': item.default_warehouse, + 'default_price_list': item.default_price_list, + 'buying_cost_center': item.buying_cost_center, + 'default_supplier': item.default_supplier, + 'expense_account': item.expense_account, + 'selling_cost_center': item.selling_cost_center, + 'income_account': item.income_account + }) +>>>>>>> 5eba1ccd51 (fix: no validation on item defaults (#27393)) else: defaults = frappe.defaults.get_defaults() or {} # To check default warehouse is belong to the default company +<<<<<<< HEAD if ( defaults.get("default_warehouse") and defaults.company @@ -699,6 +729,14 @@ class Item(Document): "item_defaults", {"company": defaults.get("company"), "default_warehouse": defaults.default_warehouse}, ) +======= + if defaults.get("default_warehouse") and defaults.company and frappe.db.exists("Warehouse", + {'name': defaults.default_warehouse, 'company': defaults.company}): + self.append("item_defaults", { + "company": defaults.get("company"), + "default_warehouse": defaults.default_warehouse + }) +>>>>>>> 5eba1ccd51 (fix: no validation on item defaults (#27393)) def update_variants(self): if self.flags.dont_update_variants or frappe.db.get_single_value( @@ -1274,6 +1312,7 @@ def set_item_tax_from_hsn_code(item): def validate_item_default_company_links(item_defaults: List[ItemDefault]) -> None: for item_default in item_defaults: for doctype, field in [ +<<<<<<< HEAD ["Warehouse", "default_warehouse"], ["Cost Center", "buying_cost_center"], ["Cost Center", "selling_cost_center"], @@ -1285,10 +1324,24 @@ def validate_item_default_company_links(item_defaults: List[ItemDefault]) -> Non if company and company != item_default.company: frappe.throw( _("Row #{}: {} {} doesn't belong to Company {}. Please select valid {}.").format( +======= + ['Warehouse', 'default_warehouse'], + ['Cost Center', 'buying_cost_center'], + ['Cost Center', 'selling_cost_center'], + ['Account', 'expense_account'], + ['Account', 'income_account'] + ]: + if item_default.get(field): + company = frappe.db.get_value(doctype, item_default.get(field), 'company', cache=True) + if company and company != item_default.company: + frappe.throw(_("Row #{}: {} {} doesn't belong to Company {}. Please select valid {}.") + .format( +>>>>>>> 5eba1ccd51 (fix: no validation on item defaults (#27393)) item_default.idx, doctype, frappe.bold(item_default.get(field)), frappe.bold(item_default.company), +<<<<<<< HEAD frappe.bold(frappe.unscrub(field)), ), title=_("Invalid Item Defaults"), @@ -1300,3 +1353,7 @@ def get_asset_naming_series(): from erpnext.assets.doctype.asset.asset import get_asset_naming_series return get_asset_naming_series() +======= + frappe.bold(frappe.unscrub(field)) + ), title=_("Invalid Item Defaults")) +>>>>>>> 5eba1ccd51 (fix: no validation on item defaults (#27393)) diff --git a/erpnext/stock/doctype/item/test_item.py b/erpnext/stock/doctype/item/test_item.py index 8f4bbfbf06b..6e34eaf76fb 100644 --- a/erpnext/stock/doctype/item/test_item.py +++ b/erpnext/stock/doctype/item/test_item.py @@ -315,6 +315,7 @@ class TestItem(FrappeTestCase): def test_item_default_validations(self): with self.assertRaises(frappe.ValidationError) as ve: +<<<<<<< HEAD make_item( "Bad Item defaults", { @@ -335,6 +336,21 @@ class TestItem(FrappeTestCase): "belong to company" in str(ve.exception).lower(), msg="Mismatching company entities in item defaults should not be allowed.", ) +======= + make_item("Bad Item defaults", { + "item_group": "_Test Item Group", + "item_defaults": [{ + "company": "_Test Company 1", + "default_warehouse": "_Test Warehouse - _TC", + "expense_account": "Stock In Hand - _TC", + "buying_cost_center": "_Test Cost Center - _TC", + "selling_cost_center": "_Test Cost Center - _TC", + }] + }) + + self.assertTrue("belong to company" in str(ve.exception).lower(), + msg="Mismatching company entities in item defaults should not be allowed.") +>>>>>>> 5eba1ccd51 (fix: no validation on item defaults (#27393)) def test_item_attribute_change_after_variant(self): frappe.delete_doc_if_exists("Item", "_Test Variant Item-L", force=1)