Merge branch 'version-13-hotfix' into backport-fix-selling-settings

frappe · Sep 17, 2021 · 5451aed · 5451aed
2 parents 24c08f5 + 6ed4046
commit 5451aed
Show file tree

Hide file tree

Showing 6 changed files with 137 additions and 78 deletions.
diff --git a/erpnext/public/js/utils.js b/erpnext/public/js/utils.js
@@ -714,12 +714,15 @@ erpnext.utils.map_current_doc = function(opts) {
 			child_columns: opts.child_columns,
 			action: function(selections, args) {
 				let values = selections;
-				if(values.length === 0){
+				if (values.length === 0) {
 					frappe.msgprint(__("Please select {0}", [opts.source_doctype]))
 					return;
 				}
 				opts.source_name = values;
-				opts.args = args;
+				if (opts.allow_child_item_selection) {
+					// args contains filtered child docnames
+					opts.args = args;
+				}
 				d.dialog.hide();
 				_map();
 			},

diff --git a/erpnext/setup/doctype/item_group/item_group.py b/erpnext/setup/doctype/item_group/item_group.py
@@ -36,6 +36,7 @@ def validate(self):
 				self.parent_item_group = _('All Item Groups')
 
 		self.make_route()
+		self.validate_item_group_defaults()
 
 	def on_update(self):
 		NestedSet.on_update(self)
@@ -113,6 +114,10 @@ def get_context(self, context):
 	def delete_child_item_groups_key(self):
 		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)
+
 def get_child_groups_for_website(item_group_name, immediate=False):
 	"""Returns child item groups *excluding* passed group."""
 	item_group = frappe.get_cached_value("Item Group", item_group_name, ["lft", "rgt"], as_dict=1)

diff --git a/erpnext/setup/doctype/item_group/test_records.json b/erpnext/setup/doctype/item_group/test_records.json
@@ -1,73 +1,74 @@
 [
  {
-  "doctype": "Item Group", 
-  "is_group": 0, 
-  "item_group_name": "_Test Item Group", 
+  "doctype": "Item Group",
+  "is_group": 0,
+  "item_group_name": "_Test Item Group",
   "parent_item_group": "All Item Groups",
   "item_group_defaults": [{
 		"company": "_Test Company",
 		"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", 
-  "is_group": 0, 
-  "item_group_name": "_Test Item Group Desktops", 
+  "doctype": "Item Group",
+  "is_group": 0,
+  "item_group_name": "_Test Item Group Desktops",
   "parent_item_group": "All Item Groups"
- }, 
+ },
  {
-  "doctype": "Item Group", 
-  "is_group": 1, 
-  "item_group_name": "_Test Item Group A", 
+  "doctype": "Item Group",
+  "is_group": 1,
+  "item_group_name": "_Test Item Group A",
   "parent_item_group": "All Item Groups"
- }, 
+ },
  {
-  "doctype": "Item Group", 
-  "is_group": 1, 
-  "item_group_name": "_Test Item Group B", 
+  "doctype": "Item Group",
+  "is_group": 1,
+  "item_group_name": "_Test Item Group B",
   "parent_item_group": "All Item Groups"
- }, 
+ },
  {
-  "doctype": "Item Group", 
-  "is_group": 1, 
-  "item_group_name": "_Test Item Group B - 1", 
+  "doctype": "Item Group",
+  "is_group": 1,
+  "item_group_name": "_Test Item Group B - 1",
   "parent_item_group": "_Test Item Group B"
- }, 
+ },
  {
-  "doctype": "Item Group", 
-  "is_group": 1, 
-  "item_group_name": "_Test Item Group B - 2", 
+  "doctype": "Item Group",
+  "is_group": 1,
+  "item_group_name": "_Test Item Group B - 2",
   "parent_item_group": "_Test Item Group B"
- }, 
+ },
  {
-  "doctype": "Item Group", 
-  "is_group": 0, 
-  "item_group_name": "_Test Item Group B - 3", 
+  "doctype": "Item Group",
+  "is_group": 0,
+  "item_group_name": "_Test Item Group B - 3",
   "parent_item_group": "_Test Item Group B"
- }, 
+ },
  {
-  "doctype": "Item Group", 
-  "is_group": 1, 
-  "item_group_name": "_Test Item Group C", 
+  "doctype": "Item Group",
+  "is_group": 1,
+  "item_group_name": "_Test Item Group C",
   "parent_item_group": "All Item Groups"
- }, 
+ },
  {
-  "doctype": "Item Group", 
-  "is_group": 1, 
-  "item_group_name": "_Test Item Group C - 1", 
+  "doctype": "Item Group",
+  "is_group": 1,
+  "item_group_name": "_Test Item Group C - 1",
   "parent_item_group": "_Test Item Group C"
- }, 
+ },
  {
-  "doctype": "Item Group", 
-  "is_group": 1, 
-  "item_group_name": "_Test Item Group C - 2", 
+  "doctype": "Item Group",
+  "is_group": 1,
+  "item_group_name": "_Test Item Group C - 2",
   "parent_item_group": "_Test Item Group C"
- }, 
+ },
  {
-  "doctype": "Item Group", 
-  "is_group": 1, 
-  "item_group_name": "_Test Item Group D", 
+  "doctype": "Item Group",
+  "is_group": 1,
+  "item_group_name": "_Test Item Group D",
   "parent_item_group": "All Item Groups"
  },
  {
@@ -104,4 +105,4 @@
    }
   ]
  }
-]
+]
diff --git a/erpnext/stock/doctype/item/item.py b/erpnext/stock/doctype/item/item.py
@@ -3,6 +3,7 @@
 
 import copy
 import json
+from typing import List
 
 import frappe
 from frappe import _
@@ -29,6 +30,7 @@
 	validate_item_variant_attributes,
 )
 from erpnext.setup.doctype.item_group.item_group import invalidate_cache_for
+from erpnext.stock.doctype.item_default.item_default import ItemDefault
 
 
 class DuplicateReorderRows(frappe.ValidationError):
@@ -116,9 +118,9 @@ def validate(self):
 		self.validate_fixed_asset()
 		self.validate_retain_sample()
 		self.validate_uom_conversion_factor()
-		self.validate_item_defaults()
 		self.validate_customer_provided_part()
 		self.update_defaults_from_item_group()
+		self.validate_item_defaults()
 		self.validate_auto_reorder_enabled_in_stock_settings()
 		self.cant_change()
 		self.validate_item_tax_net_rate_range()
@@ -309,8 +311,12 @@ def check_for_active_boms(self):
 					_("Default BOM ({0}) must be active for this item or its template").format(bom_item))
 
 	def fill_customer_code(self):
-		""" Append all the customer codes and insert into "customer_code" field of item table """
-		self.customer_code = ','.join(d.ref_code for d in self.get("customer_items", []))
+		"""
+			Append all the customer codes and insert into "customer_code" field of item table.
+			Used to search Item by customer code.
+		"""
+		customer_codes = set(d.ref_code for d in self.get("customer_items", []))
+		self.customer_code = ','.join(customer_codes)
 
 	def check_item_tax(self):
 		"""Check whether Tax Rate is not entered twice for same Tax Type"""
@@ -526,35 +532,39 @@ def validate_item_defaults(self):
 		if len(companies) != len(self.item_defaults):
 			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):
 		"""Get defaults from Item Group"""
-		if self.item_group and not self.item_defaults:
-			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
-					})
-			else:
-				warehouse = ''
-				defaults = frappe.defaults.get_defaults() or {}
+		if self.item_defaults or not self.item_group:
+			return
 
-				# 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
-						})
+		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
+				})
+		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):
 		if self.flags.dont_update_variants or \
@@ -1024,3 +1034,25 @@ def update_variants(variants, template, publish_progress=True):
 @erpnext.allow_regional
 def set_item_tax_from_hsn_code(item):
 	pass
+
+
+def validate_item_default_company_links(item_defaults: List[ItemDefault]) -> None:
+	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"))
diff --git a/erpnext/stock/doctype/item/test_item.py b/erpnext/stock/doctype/item/test_item.py
@@ -232,6 +232,23 @@ def test_item_defaults(self):
 		for key, value in purchase_item_check.items():
 			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):
 		frappe.delete_doc_if_exists("Item", "_Test Variant Item-L", force=1)
 

diff --git a/erpnext/stock/doctype/material_request/material_request.py b/erpnext/stock/doctype/material_request/material_request.py
@@ -272,8 +272,9 @@ def update_status(name, status):
 	material_request.update_status(status)
 
 @frappe.whitelist()
-def make_purchase_order(source_name, target_doc=None, args={}):
-
+def make_purchase_order(source_name, target_doc=None, args=None):
+	if args is None:
+		args = {}
 	if isinstance(args, string_types):
 		args = json.loads(args)