Merge pull request #26654 from marination/pi-expense-account

fix: Set Expense account from warehouse only if warehouse exists (Inter company invoice)

erpnext/accounts/doctype/purchase_invoice/purchase_invoice.py

@@ -27,6 +27,8 @@ from erpnext.accounts.doctype.tax_withholding_category.tax_withholding_category
 from erpnext.accounts.deferred_revenue import validate_service_stop_date
 from erpnext.stock.doctype.purchase_receipt.purchase_receipt import get_item_account_wise_additional_cost
 
+class WarehouseMissingError(frappe.ValidationError): pass
+
 form_grid_templates = {
 	"items": "templates/form_grid/item_grid.html"
 }
@@ -207,8 +209,8 @@ class PurchaseInvoice(BuyingController):
 		if self.update_stock and for_validate:
 			for d in self.get('items'):
 				if not d.warehouse:
-					frappe.throw(_("Warehouse required at Row No {0}, please set default warehouse for the item {1} for the company {2}").
-						format(d.idx, d.item_code, self.company))
+					frappe.throw(_("Row No {0}: Warehouse is required. Please set a Default Warehouse for Item {1} and Company {2}").
+						format(d.idx, d.item_code, self.company), exc=WarehouseMissingError)
 
 		super(PurchaseInvoice, self).validate_warehouse()
 
@@ -246,7 +248,7 @@ class PurchaseInvoice(BuyingController):
 				and (not item.po_detail or
 					not frappe.db.get_value("Purchase Order Item", item.po_detail, "delivered_by_supplier")):
 
-				if self.update_stock and (not item.from_warehouse):
+				if self.update_stock and item.warehouse and (not item.from_warehouse):
 					if for_validate and item.expense_account and item.expense_account != warehouse_account[item.warehouse]["account"]:
 						msg = _("Row {0}: Expense Head changed to {1} because account {2} is not linked to warehouse {3} or it is not the default inventory account").format(
 							item.idx, frappe.bold(warehouse_account[item.warehouse]["account"]), frappe.bold(item.expense_account), frappe.bold(item.warehouse))
@@ -657,7 +659,7 @@ class PurchaseInvoice(BuyingController):
 								)
 								gl_entries.append(
 									self.get_gl_dict({
-										"account": self.get_company_default("exchange_gain_loss_account"),		
+										"account": self.get_company_default("exchange_gain_loss_account"),
 										"against": self.supplier,
 										"credit": discrepancy_caused_by_exchange_rate_difference,
 										"cost_center": item.cost_center,
@@ -1193,7 +1195,7 @@ def get_purchase_document_details(doc):
 			purchase_receipts_or_invoices.append(item.get(doc_reference))
 		if item.get(items_reference):
 			items.append(item.get(items_reference))
-	
+
 	exchange_rate_map = frappe._dict(frappe.get_all(parent_doctype, filters={'name': ('in',
 		purchase_receipts_or_invoices)}, fields=['name', 'conversion_rate'], as_list=1))
 

erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py

@@ -2,13 +2,14 @@
 # License: GNU General Public License v3. See license.txt
 from __future__ import unicode_literals
 
-import frappe
+import frappe, erpnext
 
 import unittest, copy, time
 from frappe.utils import nowdate, flt, getdate, cint, add_days, add_months
 from frappe.model.dynamic_links import get_dynamic_link_map
 from erpnext.stock.doctype.stock_entry.test_stock_entry import make_stock_entry, get_qty_after_transaction
 from erpnext.accounts.doctype.purchase_invoice.test_purchase_invoice import unlink_payment_on_cancel_of_invoice
+from erpnext.accounts.doctype.purchase_invoice.purchase_invoice import WarehouseMissingError
 from erpnext.accounts.doctype.pos_profile.test_pos_profile import make_pos_profile
 from erpnext.assets.doctype.asset.test_asset import create_asset, create_asset_data
 from erpnext.exceptions import InvalidAccountCurrency, InvalidCurrency
@@ -1073,7 +1074,7 @@ class TestSalesInvoice(unittest.TestCase):
 	def test_gle_made_when_asset_is_returned(self):
 		create_asset_data()
 		asset = create_asset(item_code="Macbook Pro")
-	
+
 		si = create_sales_invoice(item_code="Macbook Pro", asset=asset.name, qty=1, rate=90000)
 		return_si = create_sales_invoice(is_return=1, return_against=si.name, item_code="Macbook Pro", asset=asset.name, qty=-1, rate=90000)
 
@@ -1081,7 +1082,7 @@ class TestSalesInvoice(unittest.TestCase):
 
 		# Asset value is 100,000 but it was sold for 90,000, so there should be a loss of 10,000
 		loss_for_si = frappe.get_all(
-			"GL Entry", 
+			"GL Entry",
 			filters = {
 				"voucher_no": si.name,
 				"account": disposal_account
@@ -1090,7 +1091,7 @@ class TestSalesInvoice(unittest.TestCase):
 		)[0]
 
 		loss_for_return_si = frappe.get_all(
-			"GL Entry", 
+			"GL Entry",
 			filters = {
 				"voucher_no": return_si.name,
 				"account": disposal_account
@@ -1836,6 +1837,89 @@ class TestSalesInvoice(unittest.TestCase):
 		self.assertEqual(target_doc.company, "_Test Company 1")
 		self.assertEqual(target_doc.supplier, "_Test Internal Supplier")
 
+	def test_inter_company_transaction_without_default_warehouse(self):
+		"Check mapping (expense account) of inter company SI to PI in absence of default warehouse."
+		# setup
+		old_negative_stock = frappe.db.get_single_value("Stock Settings", "allow_negative_stock")
+		frappe.db.set_value("Stock Settings", None, "allow_negative_stock", 1)
+
+		old_perpetual_inventory = erpnext.is_perpetual_inventory_enabled('_Test Company 1')
+		frappe.local.enable_perpetual_inventory['_Test Company 1'] = 1
+
+		frappe.db.set_value("Company", '_Test Company 1', "stock_received_but_not_billed", "Stock Received But Not Billed - _TC1")
+		frappe.db.set_value("Company", '_Test Company 1', "expenses_included_in_valuation", "Expenses Included In Valuation - _TC1")
+
+
+		if not frappe.db.exists("Customer", "_Test Internal Customer"):
+			customer = frappe.get_doc({
+				"customer_group": "_Test Customer Group",
+				"customer_name": "_Test Internal Customer",
+				"customer_type": "Individual",
+				"doctype": "Customer",
+				"territory": "_Test Territory",
+				"is_internal_customer": 1,
+				"represents_company": "_Test Company 1"
+			})
+
+			customer.append("companies", {
+				"company": "Wind Power LLC"
+			})
+
+			customer.insert()
+
+		if not frappe.db.exists("Supplier", "_Test Internal Supplier"):
+			supplier = frappe.get_doc({
+				"supplier_group": "_Test Supplier Group",
+				"supplier_name": "_Test Internal Supplier",
+				"doctype": "Supplier",
+				"is_internal_supplier": 1,
+				"represents_company": "Wind Power LLC"
+			})
+
+			supplier.append("companies", {
+				"company": "_Test Company 1"
+			})
+
+			supplier.insert()
+
+		# begin test
+		si = create_sales_invoice(
+			company = "Wind Power LLC",
+			customer = "_Test Internal Customer",
+			debit_to = "Debtors - WP",
+			warehouse = "Stores - WP",
+			income_account = "Sales - WP",
+			expense_account = "Cost of Goods Sold - WP",
+			cost_center = "Main - WP",
+			currency = "USD",
+			update_stock = 1,
+			do_not_save = 1
+		)
+		si.selling_price_list = "_Test Price List Rest of the World"
+		si.submit()
+
+		target_doc = make_inter_company_transaction("Sales Invoice", si.name)
+
+		# in absence of warehouse Stock Received But Not Billed is set as expense account while mapping
+		# mapping is not obstructed
+		self.assertIsNone(target_doc.items[0].warehouse)
+		self.assertEqual(target_doc.items[0].expense_account, "Stock Received But Not Billed - _TC1")
+
+		target_doc.items[0].update({"cost_center": "Main - _TC1"})
+
+		# missing warehouse is validated on save, after mapping
+		self.assertRaises(WarehouseMissingError, target_doc.save)
+
+		target_doc.items[0].update({"warehouse": "Stores - _TC1"})
+		target_doc.save()
+
+		# after warehouse is set, linked account or default inventory account is set
+		self.assertEqual(target_doc.items[0].expense_account, 'Stock In Hand - _TC1')
+
+		# tear down
+		frappe.local.enable_perpetual_inventory['_Test Company 1'] = old_perpetual_inventory
+		frappe.db.set_value("Stock Settings", None, "allow_negative_stock", old_negative_stock)
+
 	def test_internal_transfer_gl_entry(self):
 		## Create internal transfer account
 		account = create_account(account_name="Unrealized Profit",