fix: Usability fixes for chart of Accounts Importer (#18285)

* fix: Usablity fixes for chart of Accounts Importer

* fix: Provide Row no with error messages

erpnext/accounts/doctype/chart_of_accounts_importer/chart_of_accounts_importer.js

@@ -78,7 +78,6 @@ var validate_csv_data = function(frm) {
 
 var create_import_button = function(frm) {
 	frm.page.set_primary_action(__("Start Import"), function () {
-		setup_progress_bar(frm);
 		frappe.call({
 			method: "erpnext.accounts.doctype.chart_of_accounts_importer.chart_of_accounts_importer.import_coa",
 			args: {
@@ -86,11 +85,11 @@ var create_import_button = function(frm) {
 				company: frm.doc.company
 			},
 			freeze: true,
+			freeze_message: __("Creating Accounts..."),
 			callback: function(r) {
 				if(!r.exc) {
 					clearInterval(frm.page["interval"]);
 					frm.page.set_indicator(__('Import Successfull'), 'blue');
-					frappe.hide_progress();
 					create_reset_button(frm);
 				}
 			}
@@ -126,13 +125,3 @@ var generate_tree_preview = function(frm) {
 		}
 	});
 };
-
-var setup_progress_bar = function(frm) {
-	frm.page["seconds_elapsed"] = 0;
-	frm.page["execution_time"] = (frm.page["total_accounts"] > 100) ? 100 : frm.page["total_accounts"];
-
-	frm.page["interval"] = setInterval(function()  {
-		frm.page["seconds_elapsed"] += 1;
-		frappe.show_progress(__('Creating Accounts'), frm.page["seconds_elapsed"], frm.page["execution_time"]);
-	}, 250);
-};

erpnext/accounts/doctype/chart_of_accounts_importer/chart_of_accounts_importer.py

@@ -33,6 +33,9 @@ def import_coa(file_name, company):
 
 def generate_data_from_csv(file_name, as_dict=False):
 	''' read csv file and return the generated nested tree '''
+	if not file_name.endswith('.csv'):
+		frappe.throw("Only CSV files can be used to for importing data. Please check the file format you are trying to upload")
+
 	file_doc = frappe.get_doc('File', {"file_url": file_name})
 	file_path = file_doc.get_full_path()
 
@@ -96,15 +99,27 @@ def build_forest(data):
 				return [child] + return_parent(data, parent_account)
 
 	charts_map, paths = {}, []
+
+	line_no = 3
+	error_messages = []
+
 	for i in data:
 		account_name, _, account_number, is_group, account_type, root_type = i
+
+		if not account_name:
+			error_messages.append("Row {0}: Please enter Account Name".format(line_no))
+
 		charts_map[account_name] = {}
-		if is_group: charts_map[account_name]["is_group"] = is_group
+		if is_group == 1: charts_map[account_name]["is_group"] = is_group
 		if account_type: charts_map[account_name]["account_type"] = account_type
 		if root_type: charts_map[account_name]["root_type"] = root_type
 		if account_number: charts_map[account_name]["account_number"] = account_number
 		path = return_parent(data, account_name)[::-1]
 		paths.append(path) # List of path is created
+		line_no += 1
+
+	if error_messages:
+		frappe.throw("<br>".join(error_messages))
 
 	out = {}
 	for path in paths:
@@ -150,22 +165,27 @@ def validate_root(accounts):
 	if len(roots) < 4:
 		return _("Number of root accounts cannot be less than 4")
 
+	error_messages = []
+
 	for account in roots:
-		if not account.get("root_type"):
+		if not account.get("root_type") and account.get("account_name"):
-			return _("Please enter Root Type for - {0}").format(account.get("account_name"))
+			error_messages.append("Please enter Root Type for account- {0}".format(account.get("account_name")))
-		elif account.get("root_type") not in ("Asset", "Liability", "Expense", "Income", "Equity"):
+		elif account.get("root_type") not in ("Asset", "Liability", "Expense", "Income", "Equity") and account.get("account_name"):
-			return _('Root Type for "{0}" must be one of the Asset, Liability, Income, Expense and Equity').format(account.get("account_name"))
+			error_messages.append("Root Type for {0} must be one of the Asset, Liability, Income, Expense and Equity".format(account.get("account_name")))
+
+	if error_messages:
+		return "<br>".join(error_messages)
 
 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']]
+	account_types = [accounts[d]["account_type"] for d in accounts if not accounts[d]['is_group'] == 1]
 
 	missing = list(set(account_types_for_ledger) - set(account_types))
 	if missing:
 		return _("Please identify/create Account (Ledger) for type - {0}").format(' , '.join(missing))
 
 	account_types_for_group = ["Bank", "Cash", "Stock"]
-	account_groups = [accounts[d]["account_type"] for d in accounts if accounts[d]['is_group']]
+	account_groups = [accounts[d]["account_type"] for d in accounts if accounts[d]['is_group'] not in ('', 1)]
 
 	missing = list(set(account_types_for_group) - set(account_groups))
 	if missing: