Merge branch 'v12-pre-release' into version-12

erpnext/__init__.py

@@ -5,7 +5,7 @@ import frappe
 from erpnext.hooks import regional_overrides
 from frappe.utils import getdate
 
-__version__ = '12.24.0'
+__version__ = '12.25.0'
 
 def get_default_company(user=None):
 	'''Get default company for user'''

erpnext/accounts/doctype/pricing_rule/pricing_rule.py

@@ -330,13 +330,17 @@ def get_pricing_rule_details(args, pricing_rule):
 def apply_price_discount_rule(pricing_rule, item_details, args):
 	item_details.pricing_rule_for = pricing_rule.rate_or_discount
 
-	if ((pricing_rule.margin_type == 'Amount' and pricing_rule.currency == args.currency)
-			or (pricing_rule.margin_type == 'Percentage')):
-		item_details.margin_type = pricing_rule.margin_type
-		item_details.margin_rate_or_amount = pricing_rule.margin_rate_or_amount
-	else:
-		item_details.margin_type = None
-		item_details.margin_rate_or_amount = 0.0
+	for apply_on in ['Percentage', 'Amount']:
+		if pricing_rule.margin_type != apply_on:
+			continue
+
+		field = 'margin_rate_or_amount'
+		if field not in item_details:
+			item_details.setdefault(field, 0)
+			item_details.setdefault('margin_type', apply_on)
+
+		item_details[field] += (pricing_rule.get(field, 0)
+			if pricing_rule else args.get(field, 0))
 
 	if pricing_rule.rate_or_discount == 'Rate':
 		pricing_rule_rate = 0.0

erpnext/accounts/doctype/sales_invoice/sales_invoice.js

@@ -825,45 +825,43 @@ frappe.ui.form.on('Sales Invoice', {
 			method: "erpnext.accounts.doctype.sales_invoice.sales_invoice.create_invoice_discounting",
 			frm: frm
 		});
-	}
-})
+	},
 
-frappe.ui.form.on('Sales Invoice Timesheet', {
+	calculate_timesheet_totals: function(frm) {
+		frm.set_value("total_billing_amount",
+			frm.doc.timesheets.reduce((a, b) => a + (b["billing_amount"] || 0.0), 0.0));
+		frm.set_value("total_billing_hours",
+			frm.doc.timesheets.reduce((a, b) => a + (b["billing_hours"] || 0.0), 0.0));
+	}
+});
+
+frappe.ui.form.on("Sales Invoice Timesheet", {
 	time_sheet: function(frm, cdt, cdn){
 		var d = locals[cdt][cdn];
 		if(d.time_sheet) {
 			frappe.call({
 				method: "erpnext.projects.doctype.timesheet.timesheet.get_timesheet_data",
 				args: {
-					'name': d.time_sheet,
-					'project': frm.doc.project || null
+					"name": d.time_sheet,
+					"project": frm.doc.project || null
 				},
-				callback: function(r, rt) {
+				callback: function(r) {
 					if(r.message){
-						data = r.message;
-						frappe.model.set_value(cdt, cdn, "billing_hours", data.billing_hours);
-						frappe.model.set_value(cdt, cdn, "billing_amount", data.billing_amount);
-						frappe.model.set_value(cdt, cdn, "timesheet_detail", data.timesheet_detail);
-						calculate_total_billing_amount(frm)
+						frappe.model.set_value(cdt, cdn, "billing_hours", r.message.billing_hours);
+						frappe.model.set_value(cdt, cdn, "billing_amount", r.message.billing_amount);
+						frappe.model.set_value(cdt, cdn, "timesheet_detail", r.message.timesheet_detail);
+						frm.trigger("calculate_timesheet_totals");
 					}
 				}
-			})
+			});
 		}
+	},
+
+	timesheets_remove: function(frm, cdt, cdn) {
+		frm.trigger("calculate_timesheet_totals");
 	}
-})
+});
 
-var calculate_total_billing_amount =  function(frm) {
-	var doc = frm.doc;
-
-	doc.total_billing_amount = 0.0
-	if(doc.timesheets) {
-		$.each(doc.timesheets, function(index, data){
-			doc.total_billing_amount += data.billing_amount
-		})
-	}
-
-	refresh_field('total_billing_amount')
-}
 
 var select_loyalty_program = function(frm, loyalty_programs) {
 	var dialog = new frappe.ui.Dialog({

erpnext/accounts/doctype/sales_invoice/sales_invoice.json

@@ -69,6 +69,7 @@
   "time_sheet_list",
   "timesheets",
   "total_billing_amount",
+  "total_billing_hours",
   "section_break_30",
   "total_qty",
   "base_total",
@@ -1564,12 +1565,20 @@
   {
    "fieldname": "dimension_col_break",
    "fieldtype": "Column Break"
+  },
+  {
+   "default": "0",
+   "fieldname": "total_billing_hours",
+   "fieldtype": "Float",
+   "label": "Total Billing Hours",
+   "print_hide": 1,
+   "read_only": 1
   }
  ],
  "icon": "fa fa-file-text",
  "idx": 181,
  "is_submittable": 1,
- "modified": "2020-07-01 12:41:29.484813",
+ "modified": "2021-07-26 14:01:34.605644",
  "modified_by": "Administrator",
  "module": "Accounts",
  "name": "Sales Invoice",

erpnext/accounts/doctype/sales_invoice/sales_invoice.py

@@ -683,12 +683,11 @@ class SalesInvoice(SellingController):
 			self.calculate_billing_amount_for_timesheet()
 
 	def calculate_billing_amount_for_timesheet(self):
-		total_billing_amount = 0.0
-		for data in self.timesheets:
-			if data.billing_amount:
-				total_billing_amount += data.billing_amount
+		def timesheet_sum(field):
+			return sum((ts.get(field) or 0.0) for ts in self.timesheets)
 
-		self.total_billing_amount = total_billing_amount
+		self.total_billing_amount = timesheet_sum("billing_amount")
+		self.total_billing_hours = timesheet_sum("billing_hours")
 
 	def get_warehouse(self):
 		user_pos_profile = frappe.db.sql("""select name, warehouse from `tabPOS Profile`

erpnext/accounts/doctype/subscription/subscription.py

@@ -288,6 +288,7 @@ class Subscription(Document):
 		invoice.to_date = self.current_invoice_end
 
 		invoice.flags.ignore_mandatory = True
+		invoice.set_missing_values()
 		invoice.save()
 		
 		if self.submit_invoice:

erpnext/accounts/report/gross_profit/gross_profit.json

@@ -1,16 +1,20 @@
 {
- "add_total_row": 1,
+ "add_total_row": 0,
+ "columns": [],
  "creation": "2013-02-25 17:03:34",
+ "disable_prepared_report": 0,
  "disabled": 0,
  "docstatus": 0,
  "doctype": "Report",
+ "filters": [],
  "idx": 3,
  "is_standard": "Yes",
- "modified": "2020-08-13 11:26:39.112352",
+ "modified": "2021-08-19 18:57:07.468202",
  "modified_by": "Administrator",
  "module": "Accounts",
  "name": "Gross Profit",
  "owner": "Administrator",
+ "prepared_report": 0,
  "ref_doctype": "Sales Invoice",
  "report_name": "Gross Profit",
  "report_type": "Script Report",

erpnext/accounts/report/gross_profit/gross_profit.py

@@ -41,12 +41,14 @@ def execute(filters=None):
 
 	columns = get_columns(group_wise_columns, filters)
 
-	for src in gross_profit_data.grouped_data:
+	for idx, src in enumerate(gross_profit_data.grouped_data):
 		row = []
 		for col in group_wise_columns.get(scrub(filters.group_by)):
 			row.append(src.get(col))
 
 		row.append(filters.currency)
+		if idx == len(gross_profit_data.grouped_data)-1:
+			row[0] = frappe.bold("Total")
 		data.append(row)
 
 	return columns, data
@@ -154,6 +156,15 @@ class GrossProfitGenerator(object):
 
 	def get_average_rate_based_on_group_by(self):
 		# sum buying / selling totals for group
+		self.totals = frappe._dict(
+			qty=0,
+			base_amount=0,
+			buying_amount=0,
+			gross_profit=0,
+			gross_profit_percent=0,
+			base_rate=0,
+			buying_rate=0
+		)
 		for key in list(self.grouped):
 			if self.filters.get("group_by") != "Invoice":
 				for i, row in enumerate(self.grouped[key]):
@@ -165,6 +176,7 @@ class GrossProfitGenerator(object):
 						new_row.base_amount += flt(row.base_amount, self.currency_precision)
 				new_row = self.set_average_rate(new_row)
 				self.grouped_data.append(new_row)
+				self.add_to_totals(new_row)
 			else:
 				for i, row in enumerate(self.grouped[key]):
 					if row.parent in self.returned_invoices \
@@ -177,15 +189,25 @@ class GrossProfitGenerator(object):
 					if row.qty or row.base_amount:
 						row = self.set_average_rate(row)
 						self.grouped_data.append(row)
+					self.add_to_totals(row)
+		self.set_average_gross_profit(self.totals)
+		self.grouped_data.append(self.totals)
 
 	def set_average_rate(self, new_row):
+		self.set_average_gross_profit(new_row)
+		new_row.buying_rate = flt(new_row.buying_amount / new_row.qty, self.float_precision) if new_row.qty else 0
+		new_row.base_rate = flt(new_row.base_amount / new_row.qty, self.float_precision) if new_row.qty else 0
+		return new_row
+
+	def set_average_gross_profit(self, new_row):
 		new_row.gross_profit = flt(new_row.base_amount - new_row.buying_amount, self.currency_precision)
 		new_row.gross_profit_percent = flt(((new_row.gross_profit / new_row.base_amount) * 100.0), self.currency_precision) \
 			if new_row.base_amount else 0
-		new_row.buying_rate = flt(new_row.buying_amount / new_row.qty, self.float_precision) if new_row.qty else 0
-		new_row.base_rate = flt(new_row.base_amount / new_row.qty, self.float_precision) if new_row.qty else 0
 
-		return new_row
+	def add_to_totals(self, new_row):
+		for key in self.totals:
+			if new_row.get(key):
+				self.totals[key] += new_row[key]
 
 	def get_returned_invoice_items(self):
 		returned_invoices = frappe.db.sql("""

erpnext/change_log/v12/v12_25_0.md

@@ -0,0 +1,10 @@
+## Version 12.25.0 Release Notes
+
+### Fixes & Enhancements
+- Multiple price rules margin. ([#24844](https://github.com/frappe/erpnext/pull/24844))
+- Document naming rule not working for subscription invoices ([#27394](https://github.com/frappe/erpnext/pull/27394))
+- Prematurely referenced variable in buying controller for subcontracting ([#27333](https://github.com/frappe/erpnext/pull/27333))
+- Calculation of gross profit percentage in Gross Profit Report ([#26713](https://github.com/frappe/erpnext/pull/27045))
+- Price list rate not fetched for return sales invoice fixed ([#26593](https://github.com/frappe/erpnext/pull/26593))
+- Set production plan to completed even on over production ([#27027](https://github.com/frappe/erpnext/pull/27027))
+- Add `total_billing_hours` to Sales Invoice ([#26652](https://github.com/frappe/erpnext/pull/26652))

erpnext/controllers/accounts_controller.py

@@ -1290,6 +1290,11 @@ def update_child_qty_rate(parent_doctype, trans_items, parent_doctype_name, chil
 
 	for d in data:
 		new_child_flag = False
+
+		if not d.get("item_code"):
+			# ignore empty rows
+			continue
+
 		if not d.get("docname"):
 			new_child_flag = True
 			check_doc_permissions(parent, 'create')

erpnext/controllers/buying_controller.py

@@ -983,11 +983,11 @@ def get_non_stock_items(purchase_order, fg_item_code):
 def set_serial_nos(raw_material, consumed_serial_nos, qty):
 	consumed_serial_nos_list = []
 
-	if isinstance(consumed_serial_nos, list):
+	if consumed_serial_nos and isinstance(consumed_serial_nos, list):
 		for row in consumed_serial_nos:
 			consumed_serial_nos_list.extend(get_serial_nos(row))
-	else:
-		consumed_serial_nos_list = get_serial_nos(row)
+	elif consumed_serial_nos:
+		consumed_serial_nos_list = get_serial_nos(consumed_serial_nos)
 
 	serial_nos = set(get_serial_nos(raw_material.serial_nos)) - set(consumed_serial_nos_list)
 

erpnext/maintenance/doctype/maintenance_visit/maintenance_visit.js

@@ -49,13 +49,17 @@ erpnext.maintenance.MaintenanceVisit = frappe.ui.form.Controller.extend({
 
 		if (this.frm.doc.docstatus===0) {
 			this.frm.add_custom_button(__('Maintenance Schedule'),
-				function() {
+				function () {
+					if (!me.frm.doc.customer) {
+						frappe.msgprint(__('Please select Customer first'));
+						return;
+					}
 					erpnext.utils.map_current_doc({
 						method: "erpnext.maintenance.doctype.maintenance_schedule.maintenance_schedule.make_maintenance_visit",
 						source_doctype: "Maintenance Schedule",
 						target: me.frm,
 						setters: {
-							customer: me.frm.doc.customer || undefined,
+							customer: me.frm.doc.customer,
 						},
 						get_query_filters: {
 							docstatus: 1,
@@ -80,13 +84,17 @@ erpnext.maintenance.MaintenanceVisit = frappe.ui.form.Controller.extend({
 					})
 				}, __("Get items from"));
 			this.frm.add_custom_button(__('Sales Order'),
-				function() {
+				function () {
+					if (!me.frm.doc.customer) {
+						frappe.msgprint(__('Please select Customer first'));
+						return;
+					}
 					erpnext.utils.map_current_doc({
 						method: "erpnext.selling.doctype.sales_order.sales_order.make_maintenance_visit",
 						source_doctype: "Sales Order",
 						target: me.frm,
 						setters: {
-							customer: me.frm.doc.customer || undefined,
+							customer: me.frm.doc.customer,
 						},
 						get_query_filters: {
 							docstatus: 1,
@@ -99,4 +107,4 @@ erpnext.maintenance.MaintenanceVisit = frappe.ui.form.Controller.extend({
 	},
 });
 
-$.extend(cur_frm.cscript, new erpnext.maintenance.MaintenanceVisit({frm: cur_frm}));
+$.extend(cur_frm.cscript, new erpnext.maintenance.MaintenanceVisit({frm: cur_frm}));

erpnext/manufacturing/doctype/bom/bom.py

@@ -414,25 +414,29 @@ class BOM(WebsiteGenerator):
 				frappe.throw(_("Quantity required for Item {0} in row {1}").format(m.item_code, m.idx))
 			check_list.append(m)
 
-	def check_recursion(self, bom_list=[]):
+	def check_recursion(self, bom_list=None):
 		""" Check whether recursion occurs in any bom"""
+		def _throw_error(bom_name):
+			frappe.throw(_("BOM recursion: {0} cannot be parent or child of {0}").format(bom_name))
+
 		bom_list = self.traverse_tree()
-		bom_nos = frappe.get_all('BOM Item', fields=["bom_no"],
-			filters={'parent': ('in', bom_list), 'parenttype': 'BOM'})
+		child_items = frappe.get_all('BOM Item', fields=["bom_no", "item_code"],
+			filters={'parent': ('in', bom_list), 'parenttype': 'BOM'}) or []
 
-		raise_exception = False
-		if bom_nos and self.name in [d.bom_no for d in bom_nos]:
-			raise_exception = True
+		child_bom = {d.bom_no for d in child_items}
+		child_items_codes = {d.item_code for d in child_items}
 
-		if not raise_exception:
-			bom_nos = frappe.get_all('BOM Item', fields=["parent"],
-				filters={'bom_no': self.name, 'parenttype': 'BOM'})
+		if self.name in child_bom:
+			_throw_error(self.name)
 
-			if self.name in [d.parent for d in bom_nos]:
-				raise_exception = True
+		if self.item in child_items_codes:
+			_throw_error(self.item)
 
-		if raise_exception:
-			frappe.throw(_("BOM recursion: {0} cannot be parent or child of {1}").format(self.name, self.name))
+		bom_nos = frappe.get_all('BOM Item', fields=["parent"],
+			filters={'bom_no': self.name, 'parenttype': 'BOM'}) or []
+
+		if self.name in {d.parent for d in bom_nos}:
+			_throw_error(self.name)
 
 	def update_cost_and_exploded_items(self, bom_list=[]):
 		bom_list = self.traverse_tree(bom_list)

erpnext/manufacturing/doctype/production_plan/production_plan.py

@@ -228,7 +228,7 @@ class ProductionPlan(Document):
 
 		if self.total_produced_qty > 0:
 			self.status = "In Process"
-			if self.total_produced_qty == self.total_planned_qty:
+			if self.total_produced_qty >= self.total_planned_qty:
 				self.status = "Completed"
 
 		if self.status != 'Completed':

erpnext/manufacturing/doctype/production_plan/test_production_plan.py

@@ -198,6 +198,7 @@ class TestProductionPlan(unittest.TestCase):
 		pln.cancel()
 		frappe.delete_doc("Production Plan", pln.name)
 
+
 def create_production_plan(**args):
 	args = frappe._dict(args)
 

erpnext/public/js/utils.js

@@ -550,7 +550,7 @@ erpnext.utils.update_child_items = function(opts) {
 			},
 		],
 		primary_action: function() {
-			const trans_items = this.get_values()["trans_items"];
+			const trans_items = this.get_values()["trans_items"].filter((item) => !!item.item_code);
 			frappe.call({
 				method: 'erpnext.controllers.accounts_controller.update_child_qty_rate',
 				freeze: true,

erpnext/selling/doctype/sales_order/sales_order.py

@@ -714,8 +714,7 @@ def make_maintenance_schedule(source_name, target_doc=None):
 				"doctype": "Maintenance Schedule Item",
 				"field_map": {
 					"parent": "sales_order"
-				},
-				"add_if_empty": True
+				}
 			}
 		}, target_doc)
 
@@ -741,8 +740,7 @@ def make_maintenance_visit(source_name, target_doc=None):
 				"field_map": {
 					"parent": "prevdoc_docname",
 					"parenttype": "prevdoc_doctype"
-				},
-				"add_if_empty": True
+				}
 			}
 		}, target_doc)
 

erpnext/setup/doctype/item_group/item_group.py

@@ -33,6 +33,7 @@ class ItemGroup(NestedSet, WebsiteGenerator):
 				self.parent_item_group = _('All Item Groups')
 
 		self.make_route()
+		self.validate_item_group_defaults()
 
 	def on_update(self):
 		NestedSet.on_update(self)
@@ -88,6 +89,10 @@ class ItemGroup(NestedSet, WebsiteGenerator):
 	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)
+
 @frappe.whitelist(allow_guest=True)
 def get_product_list_for_group(product_group=None, start=0, limit=10, search=None):
 	if product_group:

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 @@
    }
   ]
  }
-]
+]

erpnext/stock/doctype/item/item.js

@@ -85,7 +85,7 @@ frappe.ui.form.on("Item", {
 		}
 		if (frm.doc.variant_of) {
 			frm.set_intro(__('This Item is a Variant of {0} (Template).',
-				[`<a href="/app/item/${frm.doc.variant_of}" onclick="location.reload()">${frm.doc.variant_of}</a>`]), true);
+				[`<a href="#Form/Item/${frm.doc.variant_of}" target="_blank">${frm.doc.variant_of}</a>`]), true);
 		}
 
 		if (frappe.defaults.get_default("item_naming_by")!="Naming Series" || frm.doc.variant_of) {

erpnext/stock/doctype/item/item.py

@@ -6,6 +6,7 @@ from __future__ import unicode_literals
 import itertools
 import json
 import erpnext
+
 import frappe
 import copy
 from erpnext.controllers.item_variant import (ItemVariantExistsError,
@@ -121,9 +122,9 @@ class Item(WebsiteGenerator):
 		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.update_show_in_website()
@@ -758,35 +759,39 @@ class Item(WebsiteGenerator):
 		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 \
@@ -1237,3 +1242,24 @@ def update_variants(variants, template, publish_progress=True):
 def on_doctype_update():
 	# since route is a Text column, it needs a length for indexing
 	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"))

erpnext/stock/doctype/item/test_item.py

@@ -219,6 +219,23 @@ class TestItem(unittest.TestCase):
 		for key, value in iteritems(purchase_item_check):
 			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)
 

erpnext/stock/get_item_details.py

@@ -72,9 +72,7 @@ def get_item_details(args, doc=None, for_validate=False, overwrite_warehouse=Tru
 
 	update_party_blanket_order(args, out)
 
-	if not doc or cint(doc.get('is_return')) == 0:
-		# get price list rate only if the invoice is not a credit or debit note
-		get_price_list_rate(args, item, out)
+	get_price_list_rate(args, item, out)
 
 	if args.customer and cint(args.is_pos):
 		out.update(get_pos_profile_item_details(args.company, args, update_data=True))

erpnext/stock/report/stock_ageing/stock_ageing.py

@@ -175,9 +175,7 @@ def get_fifo_queue(filters, sle=None):
 					fifo_queue.append([d.actual_qty, d.posting_date])
 		else:
 			if serial_no_list:
-				for serial_no in fifo_queue:
-					if serial_no[0] in serial_no_list:
-						fifo_queue.remove(serial_no)
+				fifo_queue[:] = [serial_no for serial_no in fifo_queue if serial_no[0] not in serial_no_list]
 			else:
 				qty_to_pop = abs(d.actual_qty)
 				while qty_to_pop: