fix: Improvements in COA Importer (frappe#27584)

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;
+			}
+		});
+	}
 };

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)