nsinnovations/erpnext
2
0
Fork 0
mirror of https://github.com/frappe/erpnext.git synced 2026-05-12 17:51:20 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: no validation on item defaults (#27548)

Browse Source 
* 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 <ankush@iwebnotes.com>
(cherry picked from commit 5eba1ccd51)

# Conflicts:
#	erpnext/stock/doctype/item/item.py

* fix: resolve conflict and remove typehints for py2

Co-authored-by: Saqib <nextchamp.saqib@gmail.com>
Co-authored-by: Ankush Menat <ankush@iwebnotes.com>
This commit is contained in:
Frappe PR Bot
2021-09-16 19:55:43 +05:30
committed by GitHub
parent 6f6e390863
commit efddcbe42e
4 changed files with 121 additions and 72 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
erpnext/setup/doctype/item_group/item_group.py
View File

@@ -33,6 +33,7 @@ class ItemGroup(NestedSet, WebsiteGenerator):
self.parent_item_group = _('All Item Groups') self.parent_item_group = _('All Item Groups')
self.make_route() self.make_route()
self.validate_item_group_defaults()
def on_update(self): def on_update(self):
NestedSet.on_update(self) NestedSet.on_update(self)
@@ -88,6 +89,10 @@ class ItemGroup(NestedSet, WebsiteGenerator):
def delete_child_item_groups_key(self): def delete_child_item_groups_key(self):
frappe.cache().hdel("child_item_groups", self.name) frappe.cache().hdel("child_item_groups", self.name)
def validate_item_group_defaults(self):
from erpnext.stock.doctype.item.item import validate_item_default_company_links
validate_item_default_company_links(self.item_group_defaults)
@frappe.whitelist(allow_guest=True) @frappe.whitelist(allow_guest=True)
def get_product_list_for_group(product_group=None, start=0, limit=10, search=None): def get_product_list_for_group(product_group=None, start=0, limit=10, search=None):
if product_group: if product_group:

91
erpnext/setup/doctype/item_group/test_records.json
View File

@@ -1,73 +1,74 @@
[ [
{ {
"doctype": "Item Group", "doctype": "Item Group",
"is_group": 0, "is_group": 0,
"item_group_name": "_Test Item Group", "item_group_name": "_Test Item Group",
"parent_item_group": "All Item Groups", "parent_item_group": "All Item Groups",
"item_group_defaults": [{ "item_group_defaults": [{
"company": "_Test Company", "company": "_Test Company",
"buying_cost_center": "_Test Cost Center 2 - _TC", "buying_cost_center": "_Test Cost Center 2 - _TC",
"selling_cost_center": "_Test Cost Center 2 - _TC" "selling_cost_center": "_Test Cost Center 2 - _TC",
"default_warehouse": "_Test Warehouse - _TC"
}] }]
}, },
{ {
"doctype": "Item Group", "doctype": "Item Group",
"is_group": 0, "is_group": 0,
"item_group_name": "_Test Item Group Desktops", "item_group_name": "_Test Item Group Desktops",
"parent_item_group": "All Item Groups" "parent_item_group": "All Item Groups"
}, },
{ {
"doctype": "Item Group", "doctype": "Item Group",
"is_group": 1, "is_group": 1,
"item_group_name": "_Test Item Group A", "item_group_name": "_Test Item Group A",
"parent_item_group": "All Item Groups" "parent_item_group": "All Item Groups"
}, },
{ {
"doctype": "Item Group", "doctype": "Item Group",
"is_group": 1, "is_group": 1,
"item_group_name": "_Test Item Group B", "item_group_name": "_Test Item Group B",
"parent_item_group": "All Item Groups" "parent_item_group": "All Item Groups"
}, },
{ {
"doctype": "Item Group", "doctype": "Item Group",
"is_group": 1, "is_group": 1,
"item_group_name": "_Test Item Group B - 1", "item_group_name": "_Test Item Group B - 1",
"parent_item_group": "_Test Item Group B" "parent_item_group": "_Test Item Group B"
}, },
{ {
"doctype": "Item Group", "doctype": "Item Group",
"is_group": 1, "is_group": 1,
"item_group_name": "_Test Item Group B - 2", "item_group_name": "_Test Item Group B - 2",
"parent_item_group": "_Test Item Group B" "parent_item_group": "_Test Item Group B"
}, },
{ {
"doctype": "Item Group", "doctype": "Item Group",
"is_group": 0, "is_group": 0,
"item_group_name": "_Test Item Group B - 3", "item_group_name": "_Test Item Group B - 3",
"parent_item_group": "_Test Item Group B" "parent_item_group": "_Test Item Group B"
}, },
{ {
"doctype": "Item Group", "doctype": "Item Group",
"is_group": 1, "is_group": 1,
"item_group_name": "_Test Item Group C", "item_group_name": "_Test Item Group C",
"parent_item_group": "All Item Groups" "parent_item_group": "All Item Groups"
}, },
{ {
"doctype": "Item Group", "doctype": "Item Group",
"is_group": 1, "is_group": 1,
"item_group_name": "_Test Item Group C - 1", "item_group_name": "_Test Item Group C - 1",
"parent_item_group": "_Test Item Group C" "parent_item_group": "_Test Item Group C"
}, },
{ {
"doctype": "Item Group", "doctype": "Item Group",
"is_group": 1, "is_group": 1,
"item_group_name": "_Test Item Group C - 2", "item_group_name": "_Test Item Group C - 2",
"parent_item_group": "_Test Item Group C" "parent_item_group": "_Test Item Group C"
}, },
{ {
"doctype": "Item Group", "doctype": "Item Group",
"is_group": 1, "is_group": 1,
"item_group_name": "_Test Item Group D", "item_group_name": "_Test Item Group D",
"parent_item_group": "All Item Groups" "parent_item_group": "All Item Groups"
}, },
{ {
@@ -104,4 +105,4 @@
} }
] ]
} }
] ]

80
erpnext/stock/doctype/item/item.py
View File

@@ -6,6 +6,7 @@ from __future__ import unicode_literals
import itertools import itertools
import json import json
import erpnext import erpnext
import frappe import frappe
import copy import copy
from erpnext.controllers.item_variant import (ItemVariantExistsError, from erpnext.controllers.item_variant import (ItemVariantExistsError,
@@ -121,9 +122,9 @@ class Item(WebsiteGenerator):
self.validate_fixed_asset() self.validate_fixed_asset()
self.validate_retain_sample() self.validate_retain_sample()
self.validate_uom_conversion_factor() self.validate_uom_conversion_factor()
self.validate_item_defaults()
self.validate_customer_provided_part() self.validate_customer_provided_part()
self.update_defaults_from_item_group() self.update_defaults_from_item_group()
self.validate_item_defaults()
self.validate_auto_reorder_enabled_in_stock_settings() self.validate_auto_reorder_enabled_in_stock_settings()
self.cant_change() self.cant_change()
self.update_show_in_website() self.update_show_in_website()
@@ -758,35 +759,39 @@ class Item(WebsiteGenerator):
if len(companies) != len(self.item_defaults): if len(companies) != len(self.item_defaults):
frappe.throw(_("Cannot set multiple Item Defaults for a company.")) frappe.throw(_("Cannot set multiple Item Defaults for a company."))
validate_item_default_company_links(self.item_defaults)
def update_defaults_from_item_group(self): def update_defaults_from_item_group(self):
"""Get defaults from Item Group""" """Get defaults from Item Group"""
if self.item_group and not self.item_defaults: if self.item_defaults or not self.item_group:
item_defaults = frappe.db.get_values("Item Default", {"parent": self.item_group}, return
['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
})
else:
warehouse = ''
defaults = frappe.defaults.get_defaults() or {}
# To check default warehouse is belong to the default company item_defaults = frappe.db.get_values("Item Default", {"parent": self.item_group},
if defaults.get("default_warehouse") and defaults.company and frappe.db.exists("Warehouse", ['company', 'default_warehouse','default_price_list','buying_cost_center','default_supplier',
{'name': defaults.default_warehouse, 'company': defaults.company}): 'expense_account','selling_cost_center','income_account'], as_dict = 1)
self.append("item_defaults", { if item_defaults:
"company": defaults.get("company"), for item in item_defaults:
"default_warehouse": defaults.default_warehouse 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
})
else:
defaults = frappe.defaults.get_defaults() or {}
# To check default warehouse is belong to the default company
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
})
def update_variants(self): def update_variants(self):
if self.flags.dont_update_variants or \ if self.flags.dont_update_variants or \
@@ -1237,3 +1242,24 @@ def update_variants(variants, template, publish_progress=True):
def on_doctype_update(): def on_doctype_update():
# since route is a Text column, it needs a length for indexing # since route is a Text column, it needs a length for indexing
frappe.db.add_index("Item", ["route(500)"]) frappe.db.add_index("Item", ["route(500)"])
def validate_item_default_company_links(item_defaults):
for item_default in item_defaults:
for doctype, field in [
['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(
item_default.idx,
doctype,
frappe.bold(item_default.get(field)),
frappe.bold(item_default.company),
frappe.bold(frappe.unscrub(field))
), title=_("Invalid Item Defaults"))

17
erpnext/stock/doctype/item/test_item.py
View File

@@ -219,6 +219,23 @@ class TestItem(unittest.TestCase):
for key, value in iteritems(purchase_item_check): for key, value in iteritems(purchase_item_check):
self.assertEqual(value, purchase_item_details.get(key)) self.assertEqual(value, purchase_item_details.get(key))
def test_item_default_validations(self):
with self.assertRaises(frappe.ValidationError) as ve:
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.")
def test_item_attribute_change_after_variant(self): def test_item_attribute_change_after_variant(self):
frappe.delete_doc_if_exists("Item", "_Test Variant Item-L", force=1) frappe.delete_doc_if_exists("Item", "_Test Variant Item-L", force=1)