From 9578ead7f9d6ac25094108e341a9f7b9c09561d0 Mon Sep 17 00:00:00 2001 From: Saqib Ansari Date: Sat, 28 Mar 2020 18:44:53 +0530 Subject: [PATCH 1/7] fix: no server side validations for accounts in asset category --- .../doctype/asset_category/asset_category.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/erpnext/assets/doctype/asset_category/asset_category.py b/erpnext/assets/doctype/asset_category/asset_category.py index fc08841be99..855c7094115 100644 --- a/erpnext/assets/doctype/asset_category/asset_category.py +++ b/erpnext/assets/doctype/asset_category/asset_category.py @@ -11,12 +11,29 @@ from frappe.model.document import Document class AssetCategory(Document): def validate(self): self.validate_finance_books() + self.validate_accounts() def validate_finance_books(self): for d in self.finance_books: for field in ("Total Number of Depreciations", "Frequency of Depreciation"): if cint(d.get(frappe.scrub(field)))<1: frappe.throw(_("Row {0}: {1} must be greater than 0").format(d.idx, field), frappe.MandatoryError) + + def validate_accounts(self): + account_type_map = { + 'fixed_asset_account': { 'account_type': 'Fixed Asset' }, + 'accumulated_depreciation_account': { 'account_type': 'Accumulated Depreciation' }, + 'depreciation_expense_account': { 'account_type': 'Expense' }, + 'capital_work_in_progress_account': { 'account_type': 'Capital Work in Progress' } + } + for d in self.accounts: + for account in account_type_map.keys(): + if d.get(account): + account_type = frappe.db.get_value('Account', d.get(account), 'account_type') + if account_type != account_type_map[account]['account_type']: + frappe.throw(_("Row {}: {} should be a {} account".format(d.idx, frappe.bold(frappe.unscrub(account)), + frappe.bold(account_type_map[account]['account_type']))), title=_("Invalid Account")) + @frappe.whitelist() def get_asset_category_account(fieldname, item=None, asset=None, account=None, asset_category = None, company = None): From 086e5c4dac235ef99775488fbd0d497ff23b54f2 Mon Sep 17 00:00:00 2001 From: Saqib Ansari Date: Sat, 28 Mar 2020 18:44:53 +0530 Subject: [PATCH 2/7] fix: no server side validations for accounts in asset category --- erpnext/assets/doctype/asset_category/asset_category.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/erpnext/assets/doctype/asset_category/asset_category.py b/erpnext/assets/doctype/asset_category/asset_category.py index 855c7094115..e9ef0c7d9ee 100644 --- a/erpnext/assets/doctype/asset_category/asset_category.py +++ b/erpnext/assets/doctype/asset_category/asset_category.py @@ -31,7 +31,7 @@ class AssetCategory(Document): if d.get(account): account_type = frappe.db.get_value('Account', d.get(account), 'account_type') if account_type != account_type_map[account]['account_type']: - frappe.throw(_("Row {}: {} should be a {} account".format(d.idx, frappe.bold(frappe.unscrub(account)), + frappe.throw(_("Row {}: Account Type of {} should be {} account".format(d.idx, frappe.bold(frappe.unscrub(account)), frappe.bold(account_type_map[account]['account_type']))), title=_("Invalid Account")) From 04201028d15063901faf952c8b03dc125006efc7 Mon Sep 17 00:00:00 2001 From: Saqib Ansari Date: Thu, 2 Apr 2020 22:17:41 +0530 Subject: [PATCH 3/7] fix: tests --- erpnext/accounts/doctype/account/test_account.py | 5 +++-- .../doctype/asset_category/asset_category.py | 16 ++++++++++------ .../purchase_receipt/test_purchase_receipt.py | 4 ++-- 3 files changed, 15 insertions(+), 10 deletions(-) diff --git a/erpnext/accounts/doctype/account/test_account.py b/erpnext/accounts/doctype/account/test_account.py index dc23b2b2d05..9894b9309aa 100644 --- a/erpnext/accounts/doctype/account/test_account.py +++ b/erpnext/accounts/doctype/account/test_account.py @@ -69,6 +69,7 @@ class TestAccount(unittest.TestCase): acc.account_name = "Accumulated Depreciation" acc.parent_account = "Fixed Assets - _TC" acc.company = "_Test Company" + acc.account_type = "Accumulated Depreciation" acc.insert() doc = frappe.get_doc("Account", "Securities and Deposits - _TC") @@ -149,8 +150,8 @@ def _make_test_records(verbose): # fixed asset depreciation ["_Test Fixed Asset", "Current Assets", 0, "Fixed Asset", None], - ["_Test Accumulated Depreciations", "Current Assets", 0, None, None], - ["_Test Depreciations", "Expenses", 0, None, None], + ["_Test Accumulated Depreciations", "Current Assets", 0, "Accumulated Depreciation", None], + ["_Test Depreciations", "Expenses", 0, "Expense", None], ["_Test Gain/Loss on Asset Disposal", "Expenses", 0, None, None], # Receivable / Payable Account diff --git a/erpnext/assets/doctype/asset_category/asset_category.py b/erpnext/assets/doctype/asset_category/asset_category.py index e9ef0c7d9ee..85e5d98d163 100644 --- a/erpnext/assets/doctype/asset_category/asset_category.py +++ b/erpnext/assets/doctype/asset_category/asset_category.py @@ -27,12 +27,16 @@ class AssetCategory(Document): 'capital_work_in_progress_account': { 'account_type': 'Capital Work in Progress' } } for d in self.accounts: - for account in account_type_map.keys(): - if d.get(account): - account_type = frappe.db.get_value('Account', d.get(account), 'account_type') - if account_type != account_type_map[account]['account_type']: - frappe.throw(_("Row {}: Account Type of {} should be {} account".format(d.idx, frappe.bold(frappe.unscrub(account)), - frappe.bold(account_type_map[account]['account_type']))), title=_("Invalid Account")) + for fieldname in account_type_map.keys(): + if d.get(fieldname): + selected_account = d.get(fieldname) + selected_account_type = frappe.db.get_value('Account', selected_account, 'account_type') + expected_account_type = account_type_map[fieldname]['account_type'] + + if selected_account_type != expected_account_type: + frappe.throw(_("Row #{}: Account Type of {} should be {}. Please modify the account type or select a different account." + .format(d.idx, frappe.bold(selected_account), frappe.bold(expected_account_type))), + title=_("Invalid Account")) @frappe.whitelist() diff --git a/erpnext/stock/doctype/purchase_receipt/test_purchase_receipt.py b/erpnext/stock/doctype/purchase_receipt/test_purchase_receipt.py index a51b25bf36d..09271cebfa8 100644 --- a/erpnext/stock/doctype/purchase_receipt/test_purchase_receipt.py +++ b/erpnext/stock/doctype/purchase_receipt/test_purchase_receipt.py @@ -355,8 +355,8 @@ class TestPurchaseReceipt(unittest.TestCase): 'accounts': [{ 'company_name': '_Test Company', 'fixed_asset_account': '_Test Fixed Asset - _TC', - 'accumulated_depreciation_account': 'Depreciation - _TC', - 'depreciation_expense_account': 'Depreciation - _TC' + 'accumulated_depreciation_account': '_Test Accumulated Depreciations - _TC', + 'depreciation_expense_account': '_Test Depreciation - _TC' }] }).insert() From de9c73c5cd89b8e071388a3b299aa81787472d4e Mon Sep 17 00:00:00 2001 From: Saqib Ansari Date: Thu, 2 Apr 2020 22:17:41 +0530 Subject: [PATCH 4/7] fix: tests --- .../assets/doctype/asset_category/asset_category.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/erpnext/assets/doctype/asset_category/asset_category.py b/erpnext/assets/doctype/asset_category/asset_category.py index 85e5d98d163..770b1ee9a41 100644 --- a/erpnext/assets/doctype/asset_category/asset_category.py +++ b/erpnext/assets/doctype/asset_category/asset_category.py @@ -23,19 +23,20 @@ class AssetCategory(Document): account_type_map = { 'fixed_asset_account': { 'account_type': 'Fixed Asset' }, 'accumulated_depreciation_account': { 'account_type': 'Accumulated Depreciation' }, - 'depreciation_expense_account': { 'account_type': 'Expense' }, + 'depreciation_expense_account': { 'root_type': 'Expense' }, 'capital_work_in_progress_account': { 'account_type': 'Capital Work in Progress' } } for d in self.accounts: for fieldname in account_type_map.keys(): if d.get(fieldname): selected_account = d.get(fieldname) - selected_account_type = frappe.db.get_value('Account', selected_account, 'account_type') - expected_account_type = account_type_map[fieldname]['account_type'] + key_to_match = account_type_map[fieldname].keys()[0] + selected_key_type = frappe.db.get_value('Account', selected_account, key_to_match) + expected_key_type = account_type_map[fieldname][key_to_match] - if selected_account_type != expected_account_type: - frappe.throw(_("Row #{}: Account Type of {} should be {}. Please modify the account type or select a different account." - .format(d.idx, frappe.bold(selected_account), frappe.bold(expected_account_type))), + if selected_key_type != expected_key_type: + 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")) From 39b8e150bf78c961503b19fe07d7610086a090ce Mon Sep 17 00:00:00 2001 From: Saqib Ansari Date: Thu, 9 Apr 2020 12:41:24 +0530 Subject: [PATCH 5/7] fix: travis --- erpnext/accounts/doctype/account/test_account.py | 2 +- erpnext/assets/doctype/asset_category/asset_category.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/erpnext/accounts/doctype/account/test_account.py b/erpnext/accounts/doctype/account/test_account.py index 9894b9309aa..89bb0184af8 100644 --- a/erpnext/accounts/doctype/account/test_account.py +++ b/erpnext/accounts/doctype/account/test_account.py @@ -151,7 +151,7 @@ def _make_test_records(verbose): # fixed asset depreciation ["_Test Fixed Asset", "Current Assets", 0, "Fixed Asset", None], ["_Test Accumulated Depreciations", "Current Assets", 0, "Accumulated Depreciation", None], - ["_Test Depreciations", "Expenses", 0, "Expense", None], + ["_Test Depreciations", "Expenses", 0, None, None], ["_Test Gain/Loss on Asset Disposal", "Expenses", 0, None, None], # Receivable / Payable Account diff --git a/erpnext/assets/doctype/asset_category/asset_category.py b/erpnext/assets/doctype/asset_category/asset_category.py index 770b1ee9a41..d4ea2504944 100644 --- a/erpnext/assets/doctype/asset_category/asset_category.py +++ b/erpnext/assets/doctype/asset_category/asset_category.py @@ -30,7 +30,7 @@ class AssetCategory(Document): for fieldname in account_type_map.keys(): if d.get(fieldname): selected_account = d.get(fieldname) - key_to_match = account_type_map[fieldname].keys()[0] + key_to_match = account_type_map.get(fieldname).keys()[0] # 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] From ae8a0940f526bc06ade2858a5abc6c2915820689 Mon Sep 17 00:00:00 2001 From: Saqib Ansari Date: Sat, 11 Apr 2020 18:10:59 +0530 Subject: [PATCH 6/7] fix: cannot iterate over dict_keys --- erpnext/assets/doctype/asset_category/asset_category.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/erpnext/assets/doctype/asset_category/asset_category.py b/erpnext/assets/doctype/asset_category/asset_category.py index d4ea2504944..2178b7d50f3 100644 --- a/erpnext/assets/doctype/asset_category/asset_category.py +++ b/erpnext/assets/doctype/asset_category/asset_category.py @@ -30,7 +30,7 @@ class AssetCategory(Document): for fieldname in account_type_map.keys(): if d.get(fieldname): selected_account = d.get(fieldname) - key_to_match = account_type_map.get(fieldname).keys()[0] # acount_type or root_type + 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] From f390b8062cbef1ee03ebc7aee1b7b7c533fdf3b8 Mon Sep 17 00:00:00 2001 From: Saqib Ansari Date: Thu, 16 Apr 2020 16:16:09 +0530 Subject: [PATCH 7/7] fix: incorrect transalation --- erpnext/assets/doctype/asset_category/asset_category.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/erpnext/assets/doctype/asset_category/asset_category.py b/erpnext/assets/doctype/asset_category/asset_category.py index 2178b7d50f3..9bf4df423c2 100644 --- a/erpnext/assets/doctype/asset_category/asset_category.py +++ b/erpnext/assets/doctype/asset_category/asset_category.py @@ -35,8 +35,8 @@ class AssetCategory(Document): expected_key_type = account_type_map[fieldname][key_to_match] if selected_key_type != expected_key_type: - 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))), + 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"))