fix(asset): cannot create asset if cwip disabled and account not set

erpnext/accounts/doctype/purchase_invoice/purchase_invoice.py

@@ -636,7 +636,8 @@ def get_asset_gl_entry(self, gl_entries):
 									item.item_tax_amount / self.conversion_rate)
 						}, item=item))
 				else:
-					cwip_account = get_asset_account("capital_work_in_progress_account", company = self.company)
+					cwip_account = get_asset_account("capital_work_in_progress_account",
+						asset_category=item.asset_category,company=self.company)
 
 					cwip_account_currency = get_account_currency(cwip_account)
 					gl_entries.append(self.get_gl_dict({

erpnext/accounts/doctype/purchase_invoice/test_purchase_invoice.py

@@ -939,7 +939,8 @@ def make_purchase_invoice(**args):
 		"cost_center": args.cost_center or "_Test Cost Center - _TC",
 		"project": args.project,
 		"rejected_warehouse": args.rejected_warehouse or "",
-		"rejected_serial_no": args.rejected_serial_no or ""
+		"rejected_serial_no": args.rejected_serial_no or "",
+		"asset_location": args.location or ""
 	})
 
 	if args.get_taxes_and_charges:

erpnext/assets/doctype/asset/asset.py

@@ -470,50 +470,63 @@ def get_default_finance_book_idx(self):
 
 	def validate_make_gl_entry(self):
 		purchase_document = self.get_purchase_document()
-		asset_bought_with_invoice = purchase_document == self.purchase_invoice
-		fixed_asset_account, cwip_account = self.get_asset_accounts()
+		if not purchase_document:
+			return False
+
+		asset_bought_with_invoice = (purchase_document == self.purchase_invoice)
+		fixed_asset_account = self.get_fixed_asset_account()
+
 		cwip_enabled = is_cwip_accounting_enabled(self.asset_category)
-		# check if expense already has been booked in case of cwip was enabled after purchasing asset
-		expense_booked = False
-		cwip_booked = False
+		cwip_account = self.get_cwip_account(cwip_enabled=cwip_enabled)
 
+		query = """SELECT name FROM `tabGL Entry` WHERE voucher_no = %s and account = %s"""
 		if asset_bought_with_invoice:
-			expense_booked = frappe.db.sql("""SELECT name FROM `tabGL Entry` WHERE voucher_no = %s and account = %s""",
-				(purchase_document, fixed_asset_account), as_dict=1)
+			# with invoice purchase either expense or cwip has been booked
+			expense_booked = frappe.db.sql(query, (purchase_document, fixed_asset_account), as_dict=1)
+			if expense_booked:
+				# if expense is already booked from invoice then do not make gl entries regardless of cwip enabled/disabled
+				return False
+
+			cwip_booked = frappe.db.sql(query, (purchase_document, cwip_account), as_dict=1)
+			if cwip_booked:
+				# if cwip is booked from invoice then make gl entries regardless of cwip enabled/disabled
+				return True
 		else:
-			cwip_booked = frappe.db.sql("""SELECT name FROM `tabGL Entry` WHERE voucher_no = %s and account = %s""",
-				(purchase_document, cwip_account), as_dict=1)
+			# with receipt purchase either cwip has been booked or no entries have been made
+			if not cwip_account:
+				# if cwip account isn't available do not make gl entries
+				return False
 
-		if cwip_enabled and (expense_booked or not cwip_booked):
-			# if expense has already booked from invoice or cwip is booked from receipt
-			return False
-		elif not cwip_enabled and (not expense_booked or cwip_booked):
-			# if cwip is disabled but expense hasn't been booked yet
-			return True
-		elif cwip_enabled:
-			# default condition
-			return True
+			cwip_booked = frappe.db.sql(query, (purchase_document, cwip_account), as_dict=1)
+			# if cwip is not booked from receipt then do not make gl entries
+			# if cwip is booked from receipt then make gl entries
+			return cwip_booked
 
 	def get_purchase_document(self):
 		asset_bought_with_invoice = self.purchase_invoice and frappe.db.get_value('Purchase Invoice', self.purchase_invoice, 'update_stock')
 		purchase_document = self.purchase_invoice if asset_bought_with_invoice else self.purchase_receipt
 
 		return purchase_document
 
-	def get_asset_accounts(self):
-		fixed_asset_account = get_asset_category_account('fixed_asset_account', asset=self.name,
-					asset_category = self.asset_category, company = self.company)
+	def get_fixed_asset_account(self):
+		return get_asset_category_account('fixed_asset_account', None, self.name, None, self.asset_category, self.company)
+
+	def get_cwip_account(self, cwip_enabled=False):
+		cwip_account = None
+		try:
+			cwip_account = get_asset_account("capital_work_in_progress_account", self.name, self.asset_category, self.company)
+		except:
+			# if no cwip account found in category or company and "cwip is enabled" then raise else silently pass
+			if cwip_enabled:
+				raise
 
-		cwip_account = get_asset_account("capital_work_in_progress_account",
-			self.name, self.asset_category, self.company)
-
-		return fixed_asset_account, cwip_account
+		return cwip_account
 
 	def make_gl_entries(self):
 		gl_entries = []
 
 		purchase_document = self.get_purchase_document()
-		fixed_asset_account, cwip_account = self.get_asset_accounts()
+		fixed_asset_account, cwip_account = self.get_fixed_asset_account(), self.get_cwip_account()
 
 		if (purchase_document and self.purchase_receipt_amount and self.available_for_use_date <= nowdate()):
 

erpnext/assets/doctype/asset/test_asset.py

@@ -9,6 +9,7 @@
 from erpnext.assets.doctype.asset.depreciation import post_depreciation_entries, scrap_asset, restore_asset
 from erpnext.assets.doctype.asset.asset import make_sales_invoice
 from erpnext.stock.doctype.purchase_receipt.test_purchase_receipt import make_purchase_receipt
+from erpnext.accounts.doctype.purchase_invoice.test_purchase_invoice import make_purchase_invoice
 from erpnext.stock.doctype.purchase_receipt.purchase_receipt import make_purchase_invoice as make_invoice
 
 class TestAsset(unittest.TestCase):
@@ -561,88 +562,81 @@ def test_cwip_accounting(self):
 
 		self.assertEqual(gle, expected_gle)
 
-	def test_gle_with_cwip_toggling(self):
-		# TEST: purchase an asset with cwip enabled and then disable cwip and try submitting the asset
-		frappe.db.set_value("Asset Category", "Computers", "enable_cwip_accounting", 1)
-
+	def test_expense_head(self):
 		pr = make_purchase_receipt(item_code="Macbook Pro",
-			qty=1, rate=5000, do_not_submit=True, location="Test Location")
-		pr.set('taxes', [{
-			'category': 'Total',
-			'add_deduct_tax': 'Add',
-			'charge_type': 'On Net Total',
-			'account_head': '_Test Account Service Tax - _TC',
-			'description': '_Test Account Service Tax',
-			'cost_center': 'Main - _TC',
-			'rate': 5.0
-		}, {
-			'category': 'Valuation and Total',
-			'add_deduct_tax': 'Add',
-			'charge_type': 'On Net Total',
-			'account_head': '_Test Account Shipping Charges - _TC',
-			'description': '_Test Account Shipping Charges',
-			'cost_center': 'Main - _TC',
-			'rate': 5.0
-		}])
-		pr.submit()
-		expected_gle = (
-			("Asset Received But Not Billed - _TC", 0.0, 5250.0),
-			("CWIP Account - _TC", 5250.0, 0.0)
-		)
-		pr_gle = frappe.db.sql("""select account, debit, credit from `tabGL Entry`
-			where voucher_type='Purchase Receipt' and voucher_no = %s
-			order by account""", pr.name)
-		self.assertEqual(pr_gle, expected_gle)
+			qty=2, rate=200000.0, location="Test Location")
 
-		pi = make_invoice(pr.name)
-		pi.submit()
-		expected_gle = (
-			("_Test Account Service Tax - _TC", 250.0, 0.0),
-			("_Test Account Shipping Charges - _TC", 250.0, 0.0),
-			("Asset Received But Not Billed - _TC", 5250.0, 0.0),
-			("Creditors - _TC", 0.0, 5500.0),
-			("Expenses Included In Asset Valuation - _TC", 0.0, 250.0),
-		)
-		pi_gle = frappe.db.sql("""select account, debit, credit from `tabGL Entry`
-			where voucher_type='Purchase Invoice' and voucher_no = %s
-			order by account""", pi.name)
-		self.assertEqual(pi_gle, expected_gle)
+		doc = make_invoice(pr.name)
+
+		self.assertEquals('Asset Received But Not Billed - _TC', doc.items[0].expense_account)
+
+	def test_asset_cwip_toggling_cases(self):
+		cwip = frappe.db.get_value("Asset Category", "Computers", "enable_cwip_accounting")
+		name = frappe.db.get_value("Asset Category Account", filters={"parent": "Computers"}, fieldname=["name"])
+		cwip_acc = "CWIP Account - _TC"
+
+		frappe.db.set_value("Asset Category", "Computers", "enable_cwip_accounting", 0)
+		frappe.db.set_value("Asset Category Account", name, "capital_work_in_progress_account", "")
+		frappe.db.get_value("Company", "_Test Company", "capital_work_in_progress_account", "")
 
+		# case 0 -- PI with cwip disable, Asset with cwip disabled, No cwip account set
+		pi = make_purchase_invoice(item_code="Macbook Pro", qty=1, rate=200000.0, location="Test Location", update_stock=1)
+		asset = frappe.db.get_value('Asset', {'purchase_invoice': pi.name, 'docstatus': 0}, 'name')
+		asset_doc = frappe.get_doc('Asset', asset)
+		asset_doc.available_for_use_date = nowdate()
+		asset_doc.calculate_depreciation = 0
+		asset_doc.submit()
+		gle = frappe.db.sql("""select name from `tabGL Entry` where voucher_type='Asset' and voucher_no = %s""", asset_doc.name)
+		self.assertFalse(gle)
+
+		# case 1 -- PR with cwip disabled, Asset with cwip enabled
+		pr = make_purchase_receipt(item_code="Macbook Pro", qty=1, rate=200000.0, location="Test Location")
+		frappe.db.set_value("Asset Category", "Computers", "enable_cwip_accounting", 1)
+		frappe.db.set_value("Asset Category Account", name, "capital_work_in_progress_account", cwip_acc)
 		asset = frappe.db.get_value('Asset', {'purchase_receipt': pr.name, 'docstatus': 0}, 'name')
 		asset_doc = frappe.get_doc('Asset', asset)
-		month_end_date = get_last_day(nowdate())
-		asset_doc.available_for_use_date = nowdate() if nowdate() != month_end_date else add_days(nowdate(), -15)
-		self.assertEqual(asset_doc.gross_purchase_amount, 5250.0)
-		asset_doc.append("finance_books", {
-			"expected_value_after_useful_life": 200,
-			"depreciation_method": "Straight Line",
-			"total_number_of_depreciations": 3,
-			"frequency_of_depreciation": 10,
-			"depreciation_start_date": month_end_date
-		})
+		asset_doc.available_for_use_date = nowdate()
+		asset_doc.calculate_depreciation = 0
+		asset_doc.submit()
+		gle = frappe.db.sql("""select name from `tabGL Entry` where voucher_type='Asset' and voucher_no = %s""", asset_doc.name)
+		self.assertFalse(gle)
 
-		# disable cwip and try submitting
+		# case 2 -- PR with cwip enabled, Asset with cwip disabled
+		pr = make_purchase_receipt(item_code="Macbook Pro", qty=1, rate=200000.0, location="Test Location")
 		frappe.db.set_value("Asset Category", "Computers", "enable_cwip_accounting", 0)
+		asset = frappe.db.get_value('Asset', {'purchase_receipt': pr.name, 'docstatus': 0}, 'name')
+		asset_doc = frappe.get_doc('Asset', asset)
+		asset_doc.available_for_use_date = nowdate()
+		asset_doc.calculate_depreciation = 0
 		asset_doc.submit()
-		# asset should have gl entries even if cwip is disabled
-		expected_gle = (
-			("_Test Fixed Asset - _TC", 5250.0, 0.0),
-			("CWIP Account - _TC", 0.0, 5250.0)
-		)
-		gle = frappe.db.sql("""select account, debit, credit from `tabGL Entry`
-			where voucher_type='Asset' and voucher_no = %s
-			order by account""", asset_doc.name)
-		self.assertEqual(gle, expected_gle)
+		gle = frappe.db.sql("""select name from `tabGL Entry` where voucher_type='Asset' and voucher_no = %s""", asset_doc.name)
+		self.assertTrue(gle)
 
+		# case 3 -- PI with cwip disabled, Asset with cwip enabled
+		pi = make_purchase_invoice(item_code="Macbook Pro", qty=1, rate=200000.0, location="Test Location", update_stock=1)
 		frappe.db.set_value("Asset Category", "Computers", "enable_cwip_accounting", 1)
+		asset = frappe.db.get_value('Asset', {'purchase_invoice': pi.name, 'docstatus': 0}, 'name')
+		asset_doc = frappe.get_doc('Asset', asset)
+		asset_doc.available_for_use_date = nowdate()
+		asset_doc.calculate_depreciation = 0
+		asset_doc.submit()
+		gle = frappe.db.sql("""select name from `tabGL Entry` where voucher_type='Asset' and voucher_no = %s""", asset_doc.name)
+		self.assertFalse(gle)
 
-	def test_expense_head(self):
-		pr = make_purchase_receipt(item_code="Macbook Pro",
-			qty=2, rate=200000.0, location="Test Location")
-
-		doc = make_invoice(pr.name)
+		# case 4 -- PI with cwip enabled, Asset with cwip disabled
+		pi = make_purchase_invoice(item_code="Macbook Pro", qty=1, rate=200000.0, location="Test Location", update_stock=1)
+		frappe.db.set_value("Asset Category", "Computers", "enable_cwip_accounting", 0)
+		asset = frappe.db.get_value('Asset', {'purchase_invoice': pi.name, 'docstatus': 0}, 'name')
+		asset_doc = frappe.get_doc('Asset', asset)
+		asset_doc.available_for_use_date = nowdate()
+		asset_doc.calculate_depreciation = 0
+		asset_doc.submit()
+		gle = frappe.db.sql("""select name from `tabGL Entry` where voucher_type='Asset' and voucher_no = %s""", asset_doc.name)
+		self.assertTrue(gle)
 
-		self.assertEquals('Asset Received But Not Billed - _TC', doc.items[0].expense_account)
+		frappe.db.set_value("Asset Category", "Computers", "enable_cwip_accounting", cwip)
+		frappe.db.set_value("Asset Category Account", name, "capital_work_in_progress_account", cwip_acc)
+		frappe.db.get_value("Company", "_Test Company", "capital_work_in_progress_account", cwip_acc)
 
 def create_asset_data():
 	if not frappe.db.exists("Asset Category", "Computers"):

erpnext/assets/doctype/asset_category/asset_category.py

@@ -5,14 +5,15 @@
 from __future__ import unicode_literals
 import frappe
 from frappe import _
-from frappe.utils import cint
+from frappe.utils import cint, get_link_to_form
 from frappe.model.document import Document
 
 class AssetCategory(Document):
 	def validate(self):
 		self.validate_finance_books()
 		self.validate_account_types()
 		self.validate_account_currency()
+		self.valide_cwip_account()
 
 	def validate_finance_books(self):
 		for d in self.finance_books:
@@ -58,6 +59,21 @@ def validate_account_types(self):
 						frappe.throw(_("Row #{}: {} of {} should be {}. Please modify the account or select a different account.")
 							.format(d.idx, frappe.unscrub(key_to_match), frappe.bold(selected_account), frappe.bold(expected_key_type)),
 							title=_("Invalid Account"))
+
+	def valide_cwip_account(self):
+		if self.enable_cwip_accounting:
+			missing_cwip_accounts_for_company = []
+			for d in self.accounts:
+				if (not d.capital_work_in_progress_account and 
+					not frappe.db.get_value("Company", d.company_name, "capital_work_in_progress_account")):
+					missing_cwip_accounts_for_company.append(get_link_to_form("Company", d.company_name))
+
+			if missing_cwip_accounts_for_company:
+				msg = _("""To enable Capital Work in Progress Accounting, """)
+				msg += _("""you must select Capital Work in Progress Account in accounts table""")
+				msg += "<br><br>"
+				msg += _("You can also set default CWIP account in Company {}").format(", ".join(missing_cwip_accounts_for_company))
+				frappe.throw(msg, title=_("Missing Account"))
 
 
 @frappe.whitelist()

erpnext/assets/doctype/asset_category/test_asset_category.py

@@ -26,4 +26,22 @@ def test_mandatory_fields(self):
 			asset_category.insert()
 		except frappe.DuplicateEntryError:
 			pass
-
+
+	def test_cwip_accounting(self):
+		company_cwip_acc = frappe.db.get_value("Company", "_Test Company", "capital_work_in_progress_account")
+		frappe.db.set_value("Company", "_Test Company", "capital_work_in_progress_account", "")
+
+		asset_category = frappe.new_doc("Asset Category")
+		asset_category.asset_category_name = "Computers"
+		asset_category.enable_cwip_accounting = 1
+
+		asset_category.total_number_of_depreciations = 3
+		asset_category.frequency_of_depreciation = 3
+		asset_category.append("accounts", {
+			"company_name": "_Test Company",
+			"fixed_asset_account": "_Test Fixed Asset - _TC",
+			"accumulated_depreciation_account": "_Test Accumulated Depreciations - _TC",
+			"depreciation_expense_account": "_Test Depreciations - _TC"
+		})
+
+		self.assertRaises(frappe.ValidationError, asset_category.insert)