fix(asset): cannot create asset if cwip disabled and account not set (#23580)
* Never add an asset GL entry if CWIP is not enabled * fix: asset purchase with purchase invoice * chore: allow enable cwip accounting only if cwip account is set * fix: cannot create asset if cwip disabled and account not set Co-authored-by: Saqib Ansari <nextchamp.saqib@gmail.com>
This commit is contained in:
Glen Whitney
GitHub
parent
15d46040ff
commit
6ec3dc5453
@ -711,7 +711,8 @@ class PurchaseInvoice(BuyingController):
|
||||
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({
|
||||
|
||||
@ -1002,7 +1002,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:
|
||||
|
||||
@ -466,50 +466,63 @@ class Asset(AccountsController):
|
||||
|
||||
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()
|
||||
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
|
||||
|
||||
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)
|
||||
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)
|
||||
|
||||
if cwip_enabled and (expense_booked or not cwip_booked):
|
||||
# if expense has already booked from invoice or cwip is booked from receipt
|
||||
if not purchase_document:
|
||||
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
|
||||
|
||||
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)
|
||||
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:
|
||||
# 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:
|
||||
# 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
|
||||
|
||||
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_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
|
||||
|
||||
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)
|
||||
|
||||
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()):
|
||||
|
||||
|
||||
@ -9,6 +9,7 @@ from frappe.utils import cstr, nowdate, getdate, flt, get_last_day, add_days, ad
|
||||
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):
|
||||
@ -558,81 +559,6 @@ class TestAsset(unittest.TestCase):
|
||||
|
||||
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)
|
||||
|
||||
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)
|
||||
|
||||
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)
|
||||
|
||||
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
|
||||
})
|
||||
|
||||
# disable cwip and try submitting
|
||||
frappe.db.set_value("Asset Category", "Computers", "enable_cwip_accounting", 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)
|
||||
|
||||
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=2, rate=200000.0, location="Test Location")
|
||||
@ -640,6 +566,74 @@ class TestAsset(unittest.TestCase):
|
||||
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)
|
||||
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 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()
|
||||
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)
|
||||
|
||||
# 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)
|
||||
|
||||
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"):
|
||||
|
||||
@ -5,7 +5,7 @@
|
||||
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):
|
||||
@ -13,6 +13,7 @@ class AssetCategory(Document):
|
||||
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 @@ class AssetCategory(Document):
|
||||
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()
|
||||
|
||||
@ -26,4 +26,22 @@ class TestAssetCategory(unittest.TestCase):
|
||||
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)
|
||||
Loading…
Reference in New Issue
Block a user