From 7c9b56f0732e6e824de129090fa69d5cdd525ded Mon Sep 17 00:00:00 2001 From: Deepesh Garg <42651287+deepeshgarg007@users.noreply.github.com> Date: Mon, 20 Sep 2021 11:12:39 +0530 Subject: [PATCH] fix: Improvements in COA Importer (#27584) --- .../chart_of_accounts_importer.js | 56 +++++++------------ .../chart_of_accounts_importer.py | 27 +++++---- 2 files changed, 33 insertions(+), 50 deletions(-) diff --git a/erpnext/accounts/doctype/chart_of_accounts_importer/chart_of_accounts_importer.js b/erpnext/accounts/doctype/chart_of_accounts_importer/chart_of_accounts_importer.js index f795dfa83e6d..f67c59c25490 100644 --- a/erpnext/accounts/doctype/chart_of_accounts_importer/chart_of_accounts_importer.js +++ b/erpnext/accounts/doctype/chart_of_accounts_importer/chart_of_accounts_importer.js @@ -79,7 +79,6 @@ frappe.ui.form.on('Chart of Accounts Importer', { $(frm.fields_dict['chart_tree'].wrapper).empty(); // empty wrapper on removing file } else { generate_tree_preview(frm); - validate_csv_data(frm); } }, @@ -104,23 +103,6 @@ frappe.ui.form.on('Chart of Accounts Importer', { } }); -var validate_csv_data = function(frm) { - frappe.call({ - method: "erpnext.accounts.doctype.chart_of_accounts_importer.chart_of_accounts_importer.validate_accounts", - args: {file_name: frm.doc.import_file}, - callback: function(r) { - if(r.message && r.message[0]===true) { - frm.page["show_import_button"] = true; - frm.page["total_accounts"] = r.message[1]; - frm.trigger("refresh"); - } else { - frm.page.set_indicator(__('Resolve error and upload again.'), 'orange'); - frappe.throw(__(r.message)); - } - } - }); -}; - var create_import_button = function(frm) { frm.page.set_primary_action(__("Import"), function () { frappe.call({ @@ -151,23 +133,25 @@ var create_reset_button = function(frm) { }; var generate_tree_preview = function(frm) { - let parent = __('All Accounts'); - $(frm.fields_dict['chart_tree'].wrapper).empty(); // empty wrapper to load new data + if (frm.doc.import_file) { + let parent = __('All Accounts'); + $(frm.fields_dict['chart_tree'].wrapper).empty(); // empty wrapper to load new data - // generate tree structure based on the csv data - new frappe.ui.Tree({ - parent: $(frm.fields_dict['chart_tree'].wrapper), - label: parent, - expandable: true, - method: 'erpnext.accounts.doctype.chart_of_accounts_importer.chart_of_accounts_importer.get_coa', - args: { - file_name: frm.doc.import_file, - parent: parent, - doctype: 'Chart of Accounts Importer', - file_type: frm.doc.file_type - }, - onclick: function(node) { - parent = node.value; - } - }); + // generate tree structure based on the csv data + new frappe.ui.Tree({ + parent: $(frm.fields_dict['chart_tree'].wrapper), + label: parent, + expandable: true, + method: 'erpnext.accounts.doctype.chart_of_accounts_importer.chart_of_accounts_importer.get_coa', + args: { + file_name: frm.doc.import_file, + parent: parent, + doctype: 'Chart of Accounts Importer', + file_type: frm.doc.file_type + }, + onclick: function(node) { + parent = node.value; + } + }); + } }; diff --git a/erpnext/accounts/doctype/chart_of_accounts_importer/chart_of_accounts_importer.py b/erpnext/accounts/doctype/chart_of_accounts_importer/chart_of_accounts_importer.py index 61968cf627d7..9a0234a91f97 100644 --- a/erpnext/accounts/doctype/chart_of_accounts_importer/chart_of_accounts_importer.py +++ b/erpnext/accounts/doctype/chart_of_accounts_importer/chart_of_accounts_importer.py @@ -25,8 +25,16 @@ class ChartofAccountsImporter(Document): - def validate(self): - validate_accounts(self.import_file) + pass + +def validate_columns(data): + if not data: + frappe.throw(_('No data found. Seems like you uploaded a blank file')) + + no_of_columns = max([len(d) for d in data]) + + if no_of_columns > 7: + frappe.throw(_('More columns found than expected. Please compare the uploaded file with standard template')) @frappe.whitelist() def validate_company(company): @@ -131,6 +139,8 @@ def get_coa(doctype, parent, is_root=False, file_name=None): else: data = generate_data_from_excel(file_doc, extension) + validate_columns(data) + validate_accounts(data) forest = build_forest(data) accounts = build_tree_from_json("", chart_data=forest) # returns alist of dict in a tree render-able form @@ -322,9 +332,6 @@ def validate_accounts(file_name): def validate_root(accounts): roots = [accounts[d] for d in accounts if not accounts[d].get('parent_account')] - if len(roots) < 4: - frappe.throw(_("Number of root accounts cannot be less than 4")) - error_messages = [] for account in roots: @@ -364,20 +371,12 @@ def get_mandatory_account_types(): def validate_account_types(accounts): account_types_for_ledger = ["Cost of Goods Sold", "Depreciation", "Fixed Asset", "Payable", "Receivable", "Stock Adjustment"] - account_types = [accounts[d]["account_type"] for d in accounts if not accounts[d]['is_group'] == 1] + account_types = [accounts[d]["account_type"] for d in accounts if not cint(accounts[d]['is_group']) == 1] missing = list(set(account_types_for_ledger) - set(account_types)) if missing: frappe.throw(_("Please identify/create Account (Ledger) for type - {0}").format(' , '.join(missing))) - account_types_for_group = ["Bank", "Cash", "Stock"] - # fix logic bug - account_groups = [accounts[d]["account_type"] for d in accounts if accounts[d]['is_group'] == 1] - - missing = list(set(account_types_for_group) - set(account_groups)) - if missing: - frappe.throw(_("Please identify/create Account (Group) for type - {0}").format(' , '.join(missing))) - def unset_existing_data(company): linked = frappe.db.sql('''select fieldname from tabDocField where fieldtype="Link" and options="Account" and parent="Company"''', as_dict=True)