From fe52e802ce260837ba7a3506f0aabe99f889bfee Mon Sep 17 00:00:00 2001 From: Khushi Rawat <142375893+khushi8112@users.noreply.github.com> Date: Tue, 21 Jan 2025 11:38:57 +0530 Subject: [PATCH] refactor: depreciation booking code --- erpnext/assets/doctype/asset/depreciation.py | 266 ++++++++---------- erpnext/assets/doctype/asset/test_asset.py | 46 ++- .../asset_depreciation_schedule.js | 2 +- .../asset_depreciation_schedule.py | 8 - .../depreciation_methods.py | 5 - .../doctype/asset_repair/asset_repair.py | 2 +- 6 files changed, 149 insertions(+), 180 deletions(-) diff --git a/erpnext/assets/doctype/asset/depreciation.py b/erpnext/assets/doctype/asset/depreciation.py index 868441d60c3..3ef116ffa0a 100644 --- a/erpnext/assets/doctype/asset/depreciation.py +++ b/erpnext/assets/doctype/asset/depreciation.py @@ -40,72 +40,43 @@ def post_depreciation_entries(date=None): ): return - if not date: - date = today() + date = date or today() + book_depreciation_entries(date) - failed_asset_names = [] - error_log_names = [] - depreciable_asset_depr_schedules_data = get_depreciable_asset_depr_schedules_data(date) - - credit_and_debit_accounts_for_asset_category_and_company = {} - depreciation_cost_center_and_depreciation_series_for_company = ( - get_depreciation_cost_center_and_depreciation_series_for_company() - ) +def book_depreciation_entries(date): + # Process depreciation entries for all depreciable assets + failed_assets, error_logs = [], [] + depreciable_assets_data = get_depreciable_assets_data(date) accounting_dimensions = get_checks_for_pl_and_bs_accounts() - for asset_depr_schedule_data in depreciable_asset_depr_schedules_data: - ( - asset_depr_schedule_name, - asset_name, - asset_category, - asset_company, - sch_start_idx, - sch_end_idx, - ) = asset_depr_schedule_data - - if ( - asset_category, - asset_company, - ) not in credit_and_debit_accounts_for_asset_category_and_company: - credit_and_debit_accounts_for_asset_category_and_company.update( - { - ( - asset_category, - asset_company, - ): get_credit_and_debit_accounts_for_asset_category_and_company( - asset_category, asset_company - ), - } - ) + for data in depreciable_assets_data: + (depr_schedule_name, asset_name, sch_start_idx, sch_end_idx) = data try: make_depreciation_entry( - asset_depr_schedule_name, + depr_schedule_name, date, sch_start_idx, sch_end_idx, - credit_and_debit_accounts_for_asset_category_and_company[(asset_category, asset_company)], - depreciation_cost_center_and_depreciation_series_for_company[asset_company], accounting_dimensions, ) frappe.db.commit() except Exception as e: frappe.db.rollback() - failed_asset_names.append(asset_name) + failed_assets.append(asset_name) error_log = frappe.log_error(e) - error_log_names.append(error_log.name) - - if failed_asset_names: - set_depr_entry_posting_status_for_failed_assets(failed_asset_names) - notify_depr_entry_posting_error(failed_asset_names, error_log_names) + error_logs.append(error_log.name) + if failed_assets: + set_depr_entry_posting_status_for_failed_assets(failed_assets) + notify_depr_entry_posting_error(failed_assets, error_logs) frappe.db.commit() -def get_depreciable_asset_depr_schedules_data(date): +def get_depreciable_assets_data(date): a = frappe.qb.DocType("Asset") ads = frappe.qb.DocType("Asset Depreciation Schedule") ds = frappe.qb.DocType("Depreciation Schedule") @@ -116,7 +87,7 @@ def get_depreciable_asset_depr_schedules_data(date): .on(ads.asset == a.name) .join(ds) .on(ads.name == ds.parent) - .select(ads.name, a.name, a.asset_category, a.company, Min(ds.idx) - 1, Max(ds.idx)) + .select(ads.name, a.name, Min(ds.idx) - 1, Max(ds.idx)) .where(a.calculate_depreciation == 1) .where(a.docstatus == 1) .where(ads.docstatus == 1) @@ -138,8 +109,8 @@ def get_depreciable_asset_depr_schedules_data(date): def make_depreciation_entry_on_disposal(asset_doc, disposal_date=None): for row in asset_doc.get("finance_books"): - asset_depr_schedule_name = get_asset_depr_schedule_name(asset_doc.name, "Active", row.finance_book) - make_depreciation_entry(asset_depr_schedule_name, disposal_date) + depr_schedule_name = get_asset_depr_schedule_name(asset_doc.name, "Active", row.finance_book) + make_depreciation_entry(depr_schedule_name, disposal_date) def get_acc_frozen_upto(): @@ -156,21 +127,26 @@ def get_acc_frozen_upto(): return -def get_credit_and_debit_accounts_for_asset_category_and_company(asset_category, company): - ( - _, - accumulated_depreciation_account, - depreciation_expense_account, - ) = get_depreciation_accounts(asset_category, company) +def get_credit_debit_accounts_for_asset(asset_category, company): + # Returns credit and debit accounts for the given asset category and company. + (_, accumulated_depr_account, depr_expense_account) = get_depreciation_accounts(asset_category, company) credit_account, debit_account = get_credit_and_debit_accounts( - accumulated_depreciation_account, depreciation_expense_account + accumulated_depr_account, depr_expense_account ) return (credit_account, debit_account) -def get_depreciation_cost_center_and_depreciation_series_for_company(): +def get_depreciation_cost_center_and_series(asset): + depreciation_cost_center, depreciation_series = frappe.get_cached_value( + "Company", asset.company, ["depreciation_cost_center", "series_for_depreciation_entry"] + ) + depreciation_cost_center = asset.cost_center or depreciation_cost_center + return depreciation_cost_center, depreciation_series + + +def get_depr_cost_center_and_series(): company_names = frappe.db.get_all("Company", pluck="name") res = {} @@ -179,91 +155,70 @@ def get_depreciation_cost_center_and_depreciation_series_for_company(): depreciation_cost_center, depreciation_series = frappe.get_cached_value( "Company", company_name, ["depreciation_cost_center", "series_for_depreciation_entry"] ) - res.update({company_name: (depreciation_cost_center, depreciation_series)}) + res.setdefault(company_name, (depreciation_cost_center, depreciation_series)) return res @frappe.whitelist() def make_depreciation_entry( - asset_depr_schedule_name, + depr_schedule_name, date=None, sch_start_idx=None, sch_end_idx=None, - credit_and_debit_accounts=None, - depreciation_cost_center_and_depreciation_series=None, accounting_dimensions=None, ): frappe.has_permission("Journal Entry", throw=True) + date = date or today() - if not date: - date = today() + depr_schedule_doc = frappe.get_doc("Asset Depreciation Schedule", depr_schedule_name) + asset = frappe.get_doc("Asset", depr_schedule_doc.asset) - asset_depr_schedule_doc = frappe.get_doc("Asset Depreciation Schedule", asset_depr_schedule_name) + credit_account, debit_account = get_credit_debit_accounts_for_asset(asset.asset_category, asset.company) + depr_cost_center, depr_series = get_depreciation_cost_center_and_series(asset) + accounting_dimensions = accounting_dimensions or get_checks_for_pl_and_bs_accounts() + depr_posting_error = None - asset = frappe.get_doc("Asset", asset_depr_schedule_doc.asset) - - if credit_and_debit_accounts: - credit_account, debit_account = credit_and_debit_accounts - else: - credit_account, debit_account = get_credit_and_debit_accounts_for_asset_category_and_company( - asset.asset_category, asset.company - ) - - if depreciation_cost_center_and_depreciation_series: - depreciation_cost_center, depreciation_series = depreciation_cost_center_and_depreciation_series - else: - depreciation_cost_center, depreciation_series = frappe.get_cached_value( - "Company", asset.company, ["depreciation_cost_center", "series_for_depreciation_entry"] - ) - - depreciation_cost_center = asset.cost_center or depreciation_cost_center - - if not accounting_dimensions: - accounting_dimensions = get_checks_for_pl_and_bs_accounts() - - depreciation_posting_error = None - - for d in asset_depr_schedule_doc.get("depreciation_schedule")[ - sch_start_idx or 0 : sch_end_idx or len(asset_depr_schedule_doc.get("depreciation_schedule")) + for d in depr_schedule_doc.get("depreciation_schedule")[ + (sch_start_idx or 0) : (sch_end_idx or len(depr_schedule_doc.get("depreciation_schedule"))) ]: try: _make_journal_entry_for_depreciation( - asset_depr_schedule_doc, + depr_schedule_doc, asset, date, d, sch_start_idx, sch_end_idx, - depreciation_cost_center, - depreciation_series, + depr_cost_center, + depr_series, credit_account, debit_account, accounting_dimensions, ) except Exception as e: - depreciation_posting_error = e + depr_posting_error = e asset.reload() asset.set_status() - if not depreciation_posting_error: + if not depr_posting_error: asset.db_set("depr_entry_posting_status", "Successful") - asset_depr_schedule_doc.reload() - return asset_depr_schedule_doc + depr_schedule_doc.reload() + return depr_schedule_doc - raise depreciation_posting_error + raise depr_posting_error def _make_journal_entry_for_depreciation( - asset_depr_schedule_doc, + depr_schedule_doc, asset, date, depr_schedule, sch_start_idx, sch_end_idx, - depreciation_cost_center, - depreciation_series, + depr_cost_center, + depr_series, credit_account, debit_account, accounting_dimensions, @@ -274,45 +229,11 @@ def _make_journal_entry_for_depreciation( return je = frappe.new_doc("Journal Entry") - je.voucher_type = "Depreciation Entry" - je.naming_series = depreciation_series - je.posting_date = depr_schedule.schedule_date - je.company = asset.company - je.finance_book = asset_depr_schedule_doc.finance_book - je.remark = f"Depreciation Entry against {asset.name} worth {depr_schedule.depreciation_amount}" + setup_journal_entry_metadata(je, depr_schedule_doc, depr_series, depr_schedule, asset) - credit_entry = { - "account": credit_account, - "credit_in_account_currency": depr_schedule.depreciation_amount, - "reference_type": "Asset", - "reference_name": asset.name, - "cost_center": depreciation_cost_center, - } - - debit_entry = { - "account": debit_account, - "debit_in_account_currency": depr_schedule.depreciation_amount, - "reference_type": "Asset", - "reference_name": asset.name, - "cost_center": depreciation_cost_center, - } - - for dimension in accounting_dimensions: - if asset.get(dimension["fieldname"]) or dimension.get("mandatory_for_bs"): - credit_entry.update( - { - dimension["fieldname"]: asset.get(dimension["fieldname"]) - or dimension.get("default_dimension") - } - ) - - if asset.get(dimension["fieldname"]) or dimension.get("mandatory_for_pl"): - debit_entry.update( - { - dimension["fieldname"]: asset.get(dimension["fieldname"]) - or dimension.get("default_dimension") - } - ) + credit_entry, debit_entry = get_credit_and_debit_entry( + credit_account, depr_schedule, asset, depr_cost_center, debit_account, accounting_dimensions + ) je.append("accounts", credit_entry) je.append("accounts", debit_entry) @@ -324,6 +245,47 @@ def _make_journal_entry_for_depreciation( je.submit() +def setup_journal_entry_metadata(je, depr_schedule_doc, depr_series, depr_schedule, asset): + je.voucher_type = "Depreciation Entry" + je.naming_series = depr_series + je.posting_date = depr_schedule.schedule_date + je.company = asset.company + je.finance_book = depr_schedule_doc.finance_book + je.remark = f"Depreciation Entry against {asset.name} worth {depr_schedule.depreciation_amount}" + + +def get_credit_and_debit_entry( + credit_account, depr_schedule, asset, depr_cost_center, debit_account, dimensions +): + credit_entry = { + "account": credit_account, + "credit_in_account_currency": depr_schedule.depreciation_amount, + "reference_type": "Asset", + "reference_name": asset.name, + "cost_center": depr_cost_center, + } + + debit_entry = { + "account": debit_account, + "debit_in_account_currency": depr_schedule.depreciation_amount, + "reference_type": "Asset", + "reference_name": asset.name, + "cost_center": depr_cost_center, + } + + for dimension in dimensions: + if asset.get(dimension["fieldname"]) or dimension.get("mandatory_for_bs"): + credit_entry[dimension["fieldname"]] = asset.get(dimension["fieldname"]) or dimension.get( + "default_dimension" + ) + + if asset.get(dimension["fieldname"]) or dimension.get("mandatory_for_pl"): + debit_entry[dimension["fieldname"]] = asset.get(dimension["fieldname"]) or dimension.get( + "default_dimension" + ) + return credit_entry, debit_entry + + def get_depreciation_accounts(asset_category, company): fixed_asset_account = accumulated_depreciation_account = depreciation_expense_account = None @@ -394,7 +356,24 @@ def notify_depr_entry_posting_error(failed_asset_names, error_log_names): asset_links = get_comma_separated_links(failed_asset_names, "Asset") error_log_links = get_comma_separated_links(error_log_names, "Error Log") - message = ( + message = get_message_for_depr_entry_posting_error(asset_links, error_log_links) + + frappe.sendmail(recipients=recipients, subject=subject, message=message) + + +def get_comma_separated_links(names, doctype): + links = [] + + for name in names: + links.append(get_link_to_form(doctype, name)) + + links = ", ".join(links) + + return links + + +def get_message_for_depr_entry_posting_error(asset_links, error_log_links): + return ( _("Hello,") + "

" + _("The following assets have failed to automatically post depreciation entries: {0}").format( @@ -410,19 +389,6 @@ def notify_depr_entry_posting_error(failed_asset_names, error_log_names): + _("Please share this email with your support team so that they can find and fix the issue.") ) - frappe.sendmail(recipients=recipients, subject=subject, message=message) - - -def get_comma_separated_links(names, doctype): - links = [] - - for name in names: - links.append(get_link_to_form(doctype, name)) - - links = ", ".join(links) - - return links - @frappe.whitelist() def scrap_asset(asset_name, scrap_date=None): diff --git a/erpnext/assets/doctype/asset/test_asset.py b/erpnext/assets/doctype/asset/test_asset.py index 7162e8f5a1e..3aaaf6e696f 100644 --- a/erpnext/assets/doctype/asset/test_asset.py +++ b/erpnext/assets/doctype/asset/test_asset.py @@ -30,14 +30,9 @@ from erpnext.assets.doctype.asset.depreciation import ( scrap_asset, ) from erpnext.assets.doctype.asset_depreciation_schedule.asset_depreciation_schedule import ( - _check_is_pro_rata, - _get_pro_rata_amt, get_asset_depr_schedule_doc, get_depr_schedule, ) -from erpnext.erpnext.assets.doctype.asset_depreciation_schedule.deppreciation_schedule_controller import ( - get_depreciation_amount, -) from erpnext.stock.doctype.purchase_receipt.purchase_receipt import ( make_purchase_invoice as make_invoice, ) @@ -260,7 +255,13 @@ class TestAsset(AssetSetup): asset.gross_purchase_amount - asset.finance_books[0].value_after_depreciation, asset.precision("gross_purchase_amount"), ) - pro_rata_amount, _, _ = _get_pro_rata_amt( + + second_asset_depr_schedule.depreciation_amount = 9000 + second_asset_depr_schedule.asset_doc = asset + second_asset_depr_schedule.get_finance_book_row() + second_asset_depr_schedule.fetch_asset_details() + + pro_rata_amount, _, _ = second_asset_depr_schedule._get_pro_rata_amt( asset.finance_books[0], 9000, add_days(get_last_day(add_months(purchase_date, 1)), 1), @@ -346,9 +347,12 @@ class TestAsset(AssetSetup): self.assertEqual(second_asset_depr_schedule.status, "Active") self.assertEqual(first_asset_depr_schedule.status, "Cancelled") - pro_rata_amount, _, _ = _get_pro_rata_amt( - asset.finance_books[0], - 9000, + second_asset_depr_schedule.depreciation_amount = 9000 + second_asset_depr_schedule.asset_doc = asset + second_asset_depr_schedule.get_finance_book_row() + second_asset_depr_schedule.fetch_asset_details() + + pro_rata_amount, _, _ = second_asset_depr_schedule._get_pro_rata_amt( add_days(get_last_day(add_months(purchase_date, 1)), 1), date, original_schedule_date=get_last_day(nowdate()), @@ -1029,7 +1033,7 @@ class TestDepreciationBasics(AssetSetup): asset_depr_schedule_doc = get_asset_depr_schedule_doc(asset.name, "Active") - depreciation_amount, prev_per_day_depr = get_depreciation_amount( + depreciation_amount = asset_depr_schedule_doc.get_depreciation_amount( asset_depr_schedule_doc, asset, 100000, 100000, asset.finance_books[0] ) self.assertEqual(depreciation_amount, 30000) @@ -1076,7 +1080,7 @@ class TestDepreciationBasics(AssetSetup): def test_check_is_pro_rata(self): """Tests if check_is_pro_rata() returns the right value(i.e. checks if has_pro_rata is accurate).""" - asset = create_asset(item_code="Macbook Pro", available_for_use_date="2019-12-31", do_not_save=1) + asset = create_asset(item_code="Macbook Pro", available_for_use_date="2019-12-31") asset.calculate_depreciation = 1 asset.append( @@ -1089,9 +1093,15 @@ class TestDepreciationBasics(AssetSetup): "depreciation_start_date": "2020-12-31", }, ) + asset.save() - has_pro_rata = _check_is_pro_rata(asset, asset.finance_books[0]) - self.assertFalse(has_pro_rata) + depr_schedule_doc = get_asset_depr_schedule_doc(asset.name, "Draft") + depr_schedule_doc.asset_doc = asset + depr_schedule_doc.get_finance_book_row() + depr_schedule_doc.fetch_asset_details() + depr_schedule_doc._check_is_pro_rata() + + self.assertFalse(depr_schedule_doc.has_pro_rata) asset.finance_books = [] asset.append( @@ -1104,9 +1114,15 @@ class TestDepreciationBasics(AssetSetup): "depreciation_start_date": "2020-07-01", }, ) + asset.save() - has_pro_rata = _check_is_pro_rata(asset, asset.finance_books[0]) - self.assertTrue(has_pro_rata) + depr_schedule_doc = get_asset_depr_schedule_doc(asset.name, "Draft") + depr_schedule_doc.asset_doc = asset + depr_schedule_doc.get_finance_book_row() + depr_schedule_doc.fetch_asset_details() + depr_schedule_doc._check_is_pro_rata() + + self.assertTrue(depr_schedule_doc.has_pro_rata) def test_expected_value_after_useful_life_greater_than_purchase_amount(self): """Tests if an error is raised when expected_value_after_useful_life(110,000) > gross_purchase_amount(100,000).""" diff --git a/erpnext/assets/doctype/asset_depreciation_schedule/asset_depreciation_schedule.js b/erpnext/assets/doctype/asset_depreciation_schedule/asset_depreciation_schedule.js index 3f7a2e7c7d8..f76f1d11720 100644 --- a/erpnext/assets/doctype/asset_depreciation_schedule/asset_depreciation_schedule.js +++ b/erpnext/assets/doctype/asset_depreciation_schedule/asset_depreciation_schedule.js @@ -31,7 +31,7 @@ frappe.ui.form.on("Depreciation Schedule", { frappe.call({ method: "erpnext.assets.doctype.asset.depreciation.make_depreciation_entry", args: { - asset_depr_schedule_name: frm.doc.name, + depr_schedule_name: frm.doc.name, date: row.schedule_date, }, debounce: 1000, diff --git a/erpnext/assets/doctype/asset_depreciation_schedule/asset_depreciation_schedule.py b/erpnext/assets/doctype/asset_depreciation_schedule/asset_depreciation_schedule.py index e0e60d7cdfd..1750a2a782f 100644 --- a/erpnext/assets/doctype/asset_depreciation_schedule/asset_depreciation_schedule.py +++ b/erpnext/assets/doctype/asset_depreciation_schedule/asset_depreciation_schedule.py @@ -4,18 +4,10 @@ import frappe from frappe import _ from frappe.utils import ( - add_days, - add_months, - add_years, - cint, - date_diff, flt, get_first_day, - get_last_day, get_link_to_form, getdate, - is_last_day_of_the_month, - month_diff, ) import erpnext diff --git a/erpnext/assets/doctype/asset_depreciation_schedule/depreciation_methods.py b/erpnext/assets/doctype/asset_depreciation_schedule/depreciation_methods.py index da7962db63e..961f7c70db1 100644 --- a/erpnext/assets/doctype/asset_depreciation_schedule/depreciation_methods.py +++ b/erpnext/assets/doctype/asset_depreciation_schedule/depreciation_methods.py @@ -1,13 +1,8 @@ import frappe from frappe.model.document import Document from frappe.utils import ( - add_days, - add_years, cint, - date_diff, flt, - month_diff, - nowdate, ) import erpnext diff --git a/erpnext/assets/doctype/asset_repair/asset_repair.py b/erpnext/assets/doctype/asset_repair/asset_repair.py index 27569eb6cce..8a4349233e5 100644 --- a/erpnext/assets/doctype/asset_repair/asset_repair.py +++ b/erpnext/assets/doctype/asset_repair/asset_repair.py @@ -4,7 +4,7 @@ import frappe from frappe import _ from frappe.query_builder import DocType -from frappe.utils import add_months, cint, flt, get_link_to_form, getdate, time_diff_in_hours +from frappe.utils import cint, flt, get_link_to_form, getdate, time_diff_in_hours import erpnext from erpnext.accounts.general_ledger import make_gl_entries