From aa347802656a92695c3f3b94c292d7f6a6209709 Mon Sep 17 00:00:00 2001 From: GangaManoj Date: Wed, 17 Nov 2021 04:57:40 +0530 Subject: [PATCH 1/8] fix: Filter Depreciation Expense Account by root type --- erpnext/assets/doctype/asset_category/asset_category.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/erpnext/assets/doctype/asset_category/asset_category.js b/erpnext/assets/doctype/asset_category/asset_category.js index 51ce157a81..c702687072 100644 --- a/erpnext/assets/doctype/asset_category/asset_category.js +++ b/erpnext/assets/doctype/asset_category/asset_category.js @@ -33,7 +33,7 @@ frappe.ui.form.on('Asset Category', { var d = locals[cdt][cdn]; return { "filters": { - "root_type": "Expense", + "root_type": ["in", ["Expense", "Income"]], "is_group": 0, "company": d.company_name } From 8fc31e3ceabcda85ba15087a07f7c8ca4c1d57a7 Mon Sep 17 00:00:00 2001 From: GangaManoj Date: Wed, 17 Nov 2021 04:58:13 +0530 Subject: [PATCH 2/8] fix: Make Depreciation Entry posting more flexible --- erpnext/assets/doctype/asset/depreciation.py | 30 ++++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) diff --git a/erpnext/assets/doctype/asset/depreciation.py b/erpnext/assets/doctype/asset/depreciation.py index ca10b1db19..6591654f94 100644 --- a/erpnext/assets/doctype/asset/depreciation.py +++ b/erpnext/assets/doctype/asset/depreciation.py @@ -57,8 +57,10 @@ def make_depreciation_entry(asset_name, date=None): je.finance_book = d.finance_book je.remark = "Depreciation Entry against {0} worth {1}".format(asset_name, d.depreciation_amount) + credit_account, debit_account = get_credit_and_debit_accounts(accumulated_depreciation_account, depreciation_expense_account) + credit_entry = { - "account": accumulated_depreciation_account, + "account": credit_account, "credit_in_account_currency": d.depreciation_amount, "reference_type": "Asset", "reference_name": asset.name, @@ -66,7 +68,7 @@ def make_depreciation_entry(asset_name, date=None): } debit_entry = { - "account": depreciation_expense_account, + "account": debit_account, "debit_in_account_currency": d.depreciation_amount, "reference_type": "Asset", "reference_name": asset.name, @@ -132,6 +134,30 @@ def get_depreciation_accounts(asset): return fixed_asset_account, accumulated_depreciation_account, depreciation_expense_account +def get_credit_and_debit_accounts(accumulated_depreciation_account, depreciation_expense_account): + if is_income_or_expense_account(depreciation_expense_account) == "Expense": + credit_account = accumulated_depreciation_account + debit_account = depreciation_expense_account + else: + credit_account = depreciation_expense_account + debit_account = accumulated_depreciation_account + + return credit_account, debit_account + +def is_income_or_expense_account(account): + from frappe.utils.nestedset import get_ancestors_of + + ancestors = get_ancestors_of("Account", account) + if ancestors: + root_account = ancestors[-1].split(' - ')[0] + + if root_account == "Expenses": + return "Expense" + elif root_account == "Income": + return "Income" + + frappe.throw(_("Depreciation Expense Account should be an Income or Expense Account.")) + @frappe.whitelist() def scrap_asset(asset_name): asset = frappe.get_doc("Asset", asset_name) From 62fbbe8915fcb3b03c823b5e5aae7c2e400c5002 Mon Sep 17 00:00:00 2001 From: GangaManoj Date: Wed, 17 Nov 2021 04:59:59 +0530 Subject: [PATCH 3/8] fix: Only raise an error if Depreciation Expense Account is neither an Income nor an Expense Account --- .../doctype/asset_category/asset_category.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/erpnext/assets/doctype/asset_category/asset_category.py b/erpnext/assets/doctype/asset_category/asset_category.py index e2f3ca318f..bd573bf479 100644 --- a/erpnext/assets/doctype/asset_category/asset_category.py +++ b/erpnext/assets/doctype/asset_category/asset_category.py @@ -42,10 +42,10 @@ class AssetCategory(Document): def validate_account_types(self): account_type_map = { - 'fixed_asset_account': { 'account_type': 'Fixed Asset' }, - 'accumulated_depreciation_account': { 'account_type': 'Accumulated Depreciation' }, - 'depreciation_expense_account': { 'root_type': 'Expense' }, - 'capital_work_in_progress_account': { 'account_type': 'Capital Work in Progress' } + 'fixed_asset_account': {'account_type': ['Fixed Asset']}, + 'accumulated_depreciation_account': {'account_type': ['Accumulated Depreciation']}, + 'depreciation_expense_account': {'root_type': ['Expense', 'Income']}, + 'capital_work_in_progress_account': {'account_type': ['Capital Work in Progress']} } for d in self.accounts: for fieldname in account_type_map.keys(): @@ -53,11 +53,11 @@ class AssetCategory(Document): selected_account = d.get(fieldname) key_to_match = next(iter(account_type_map.get(fieldname))) # acount_type or root_type selected_key_type = frappe.db.get_value('Account', selected_account, key_to_match) - expected_key_type = account_type_map[fieldname][key_to_match] + expected_key_types = account_type_map[fieldname][key_to_match] - if selected_key_type != expected_key_type: + if selected_key_type not in expected_key_types: 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)), + .format(d.idx, frappe.unscrub(key_to_match), frappe.bold(selected_account), frappe.bold(expected_key_types)), title=_("Invalid Account")) def valide_cwip_account(self): From cb93cc972d18ba7f52482901eb849bd418fb9fc5 Mon Sep 17 00:00:00 2001 From: GangaManoj Date: Wed, 17 Nov 2021 05:22:43 +0530 Subject: [PATCH 4/8] fix: Check all ancestors and not just the root node --- erpnext/assets/doctype/asset/depreciation.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/erpnext/assets/doctype/asset/depreciation.py b/erpnext/assets/doctype/asset/depreciation.py index 6591654f94..0c96b519ed 100644 --- a/erpnext/assets/doctype/asset/depreciation.py +++ b/erpnext/assets/doctype/asset/depreciation.py @@ -147,13 +147,11 @@ def get_credit_and_debit_accounts(accumulated_depreciation_account, depreciation def is_income_or_expense_account(account): from frappe.utils.nestedset import get_ancestors_of - ancestors = get_ancestors_of("Account", account) + ancestors = [ancestor.split(' - ')[0] for ancestor in get_ancestors_of("Account", account)] if ancestors: - root_account = ancestors[-1].split(' - ')[0] - - if root_account == "Expenses": + if "Expenses" in ancestors: return "Expense" - elif root_account == "Income": + elif "Income" in ancestors: return "Income" frappe.throw(_("Depreciation Expense Account should be an Income or Expense Account.")) From d406775d1ab9b5b5c3a73da4aa00e656cc239047 Mon Sep 17 00:00:00 2001 From: GangaManoj Date: Tue, 23 Nov 2021 20:21:24 +0530 Subject: [PATCH 5/8] fix: Check root_type of Depreciation Expense Account --- erpnext/assets/doctype/asset/depreciation.py | 20 ++++++-------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/erpnext/assets/doctype/asset/depreciation.py b/erpnext/assets/doctype/asset/depreciation.py index 0c96b519ed..874fb630f8 100644 --- a/erpnext/assets/doctype/asset/depreciation.py +++ b/erpnext/assets/doctype/asset/depreciation.py @@ -135,27 +135,19 @@ def get_depreciation_accounts(asset): return fixed_asset_account, accumulated_depreciation_account, depreciation_expense_account def get_credit_and_debit_accounts(accumulated_depreciation_account, depreciation_expense_account): - if is_income_or_expense_account(depreciation_expense_account) == "Expense": + root_type = frappe.get_value("Account", depreciation_expense_account, "root_type") + + if root_type == "Expense": credit_account = accumulated_depreciation_account debit_account = depreciation_expense_account - else: + elif root_type == "Income": credit_account = depreciation_expense_account debit_account = accumulated_depreciation_account + else: + frappe.throw(_("Depreciation Expense Account should be an Income or Expense Account.")) return credit_account, debit_account -def is_income_or_expense_account(account): - from frappe.utils.nestedset import get_ancestors_of - - ancestors = [ancestor.split(' - ')[0] for ancestor in get_ancestors_of("Account", account)] - if ancestors: - if "Expenses" in ancestors: - return "Expense" - elif "Income" in ancestors: - return "Income" - - frappe.throw(_("Depreciation Expense Account should be an Income or Expense Account.")) - @frappe.whitelist() def scrap_asset(asset_name): asset = frappe.get_doc("Asset", asset_name) From a36713cd2d353826d4c0e6330eb993ab972c1313 Mon Sep 17 00:00:00 2001 From: GangaManoj Date: Tue, 23 Nov 2021 21:03:03 +0530 Subject: [PATCH 6/8] fix: Test Depreciation Entry posting when Depreciation Expense Account is an Expense Account --- erpnext/assets/doctype/asset/test_asset.py | 28 ++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/erpnext/assets/doctype/asset/test_asset.py b/erpnext/assets/doctype/asset/test_asset.py index d1d4527ec7..cc7eab0ffd 100644 --- a/erpnext/assets/doctype/asset/test_asset.py +++ b/erpnext/assets/doctype/asset/test_asset.py @@ -869,6 +869,34 @@ class TestDepreciationBasics(AssetSetup): self.assertFalse(asset.schedules[1].journal_entry) self.assertFalse(asset.schedules[2].journal_entry) + def test_depr_entry_posting_when_depr_expense_account_is_an_expense_account(self): + """Tests if the Depreciation Expense Account gets debited and the Accumulated Depreciation Account gets credited when the former's an Expense Account.""" + + asset = create_asset( + item_code = "Macbook Pro", + calculate_depreciation = 1, + available_for_use_date = "2019-12-31", + depreciation_start_date = "2020-12-31", + frequency_of_depreciation = 12, + total_number_of_depreciations = 3, + expected_value_after_useful_life = 10000, + submit = 1 + ) + + post_depreciation_entries(date="2021-06-01") + asset.load_from_db() + + je = frappe.get_doc("Journal Entry", asset.schedules[0].journal_entry) + accounting_entries = [{"account": entry.account, "debit": entry.debit, "credit": entry.credit} for entry in je.accounts] + + for entry in accounting_entries: + if entry["account"] == "_Test Depreciations - _TC": + self.assertTrue(entry["debit"]) + self.assertFalse(entry["credit"]) + else: + self.assertTrue(entry["credit"]) + self.assertFalse(entry["debit"]) + def test_clear_depreciation_schedule(self): """Tests if clear_depreciation_schedule() works as expected.""" From 60d82d913c2a09dcafc1f917e79fbe1a6be444bc Mon Sep 17 00:00:00 2001 From: GangaManoj Date: Tue, 23 Nov 2021 23:03:48 +0530 Subject: [PATCH 7/8] fix: Test Depreciation Entry posting when Depreciation Expense Account is an Income Account --- erpnext/assets/doctype/asset/test_asset.py | 36 ++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/erpnext/assets/doctype/asset/test_asset.py b/erpnext/assets/doctype/asset/test_asset.py index cc7eab0ffd..b0549b1027 100644 --- a/erpnext/assets/doctype/asset/test_asset.py +++ b/erpnext/assets/doctype/asset/test_asset.py @@ -897,6 +897,42 @@ class TestDepreciationBasics(AssetSetup): self.assertTrue(entry["credit"]) self.assertFalse(entry["debit"]) + def test_depr_entry_posting_when_depr_expense_account_is_an_income_account(self): + """Tests if the Depreciation Expense Account gets credited and the Accumulated Depreciation Account gets debited when the former's an Income Account.""" + + depr_expense_account = frappe.get_doc("Account", "_Test Depreciations - _TC") + depr_expense_account.root_type = "Income" + depr_expense_account.parent_account = "Income - _TC" + + asset = create_asset( + item_code = "Macbook Pro", + calculate_depreciation = 1, + available_for_use_date = "2019-12-31", + depreciation_start_date = "2020-12-31", + frequency_of_depreciation = 12, + total_number_of_depreciations = 3, + expected_value_after_useful_life = 10000, + submit = 1 + ) + + post_depreciation_entries(date="2021-06-01") + asset.load_from_db() + + je = frappe.get_doc("Journal Entry", asset.schedules[0].journal_entry) + accounting_entries = [{"account": entry.account, "debit": entry.debit, "credit": entry.credit} for entry in je.accounts] + + for entry in accounting_entries: + if entry["account"] == "_Test Depreciations - _TC": + self.assertTrue(entry["credit"]) + self.assertFalse(entry["debit"]) + else: + self.assertTrue(entry["debit"]) + self.assertFalse(entry["credit"]) + + # resetting + depr_expense_account.root_type = "Expense" + depr_expense_account.parent_account = "Expenses - _TC" + def test_clear_depreciation_schedule(self): """Tests if clear_depreciation_schedule() works as expected.""" From 56df646067af5c127ff133f54ee31e507fc8cd5e Mon Sep 17 00:00:00 2001 From: Saqib Ansari Date: Mon, 6 Dec 2021 11:19:58 +0530 Subject: [PATCH 8/8] fix(test): test_depr_entry_posting_with_income_account --- erpnext/assets/doctype/asset/test_asset.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/erpnext/assets/doctype/asset/test_asset.py b/erpnext/assets/doctype/asset/test_asset.py index b0549b1027..9b5dcc4fe4 100644 --- a/erpnext/assets/doctype/asset/test_asset.py +++ b/erpnext/assets/doctype/asset/test_asset.py @@ -903,6 +903,7 @@ class TestDepreciationBasics(AssetSetup): depr_expense_account = frappe.get_doc("Account", "_Test Depreciations - _TC") depr_expense_account.root_type = "Income" depr_expense_account.parent_account = "Income - _TC" + depr_expense_account.save() asset = create_asset( item_code = "Macbook Pro", @@ -932,6 +933,7 @@ class TestDepreciationBasics(AssetSetup): # resetting depr_expense_account.root_type = "Expense" depr_expense_account.parent_account = "Expenses - _TC" + depr_expense_account.save() def test_clear_depreciation_schedule(self): """Tests if clear_depreciation_schedule() works as expected."""