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 8625b274ace..faabebc1cbb 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 @@ -46,9 +46,7 @@ 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 != 8: + if any(len(row) != 8 for row in data): frappe.throw( _( "Columns are not according to template. Please compare the uploaded file with standard template" @@ -99,13 +97,13 @@ def import_coa(file_name, company): data = generate_data_from_excel(file_doc, extension) validate_columns(data) + forest = build_forest(data) validate_company(company) unset_existing_data(company) frappe.local.flags.ignore_root_company_validation = True - forest = build_forest(data) create_charts(company, custom_chart=forest, from_coa_importer=True) # trigger on_update for company to reset default accounts @@ -135,25 +133,8 @@ def get_file(file_name): def generate_data_from_csv(file_doc, as_dict=False): """read csv file and return the generated nested tree""" - file_path = file_doc.get_full_path() - - data = [] - with open(file_path) as in_file: - csv_reader = list(csv.reader(in_file)) - headers = csv_reader[0] - del csv_reader[0] # delete top row and headers row - - for row in csv_reader: - if as_dict: - data.append({frappe.scrub(header): row[index] for index, header in enumerate(headers)}) - else: - if not row[1] and len(row) > 1: - row[1] = row[0] - row[3] = row[2] - data.append(row) - - # convert csv data - return data + with open(file_doc.get_full_path()) as in_file: + return generate_data_from_rows(list(csv.reader(in_file)), as_dict) def generate_data_from_excel(file_doc, extension, as_dict=False): @@ -164,13 +145,19 @@ def generate_data_from_excel(file_doc, extension, as_dict=False): elif extension == "xls": rows = read_xls_file_from_attached_file(content) - data = [] - headers = rows[0] - del rows[0] + return generate_data_from_rows(rows, as_dict) - for row in rows: + +def generate_data_from_rows(rows, as_dict=False): + rows = [row for row in rows if any(cstr(value).strip() for value in row)] + validate_columns(rows[1:]) + validate_columns(rows[:1]) + + headers = rows[0] + data = [] + for row in rows[1:]: if as_dict: - data.append({frappe.scrub(header): row[index] for index, header in enumerate(headers)}) + data.append({frappe.scrub(cstr(header)): row[index] for index, header in enumerate(headers)}) else: if not row[1]: row[1] = row[0] @@ -195,9 +182,9 @@ def get_coa(doctype, parent, is_root=False, file_name=None, for_validate=0): validate_columns(data) validate_accounts(file_doc, extension) + forest = build_forest(data) if not for_validate: - forest = build_forest(data) accounts = build_tree_from_json( "", chart_data=forest, from_coa_importer=True ) # returns a list of dict in a tree render-able form @@ -225,6 +212,7 @@ def build_forest(data): } } """ + validate_columns(data) # set the value of nested dictionary def set_nested(d, path, value): @@ -232,31 +220,25 @@ def build_forest(data): return d # returns the path of any node in list format - def return_parent(data, child): - from frappe import _ - - for row in data: - account_name, parent_account, account_number, parent_account_number = row[0:4] - if account_number: - account_number = cstr(account_number).strip() - account_name = f"{account_number} - {account_name}" - if parent_account_number: - parent_account_number = cstr(parent_account_number).strip() - parent_account = f"{parent_account_number} - {parent_account}" - - if parent_account == account_name == child: - return [parent_account] - elif account_name == child: - parent_account_list = return_parent(data, parent_account) - if not parent_account_list and parent_account: - frappe.throw( - _("The parent account {0} does not exists in the uploaded template").format( - frappe.bold(parent_account) - ) + def return_parent(parent_accounts, child): + path = [] + while child: + if child in path: + frappe.throw(_("Circular reference found for account {0}").format(frappe.bold(child))) + path.append(child) + parent_account = parent_accounts[child] + if parent_account == child: + break + if parent_account and parent_account not in parent_accounts: + frappe.throw( + _("The parent account {0} does not exist in the uploaded template").format( + frappe.bold(parent_account) ) - return [child, *parent_account_list] + ) + child = parent_account + return path - charts_map, paths = {}, [] + charts_map, parent_accounts = {}, {} line_no = 2 error_messages = [] @@ -274,12 +256,15 @@ def build_forest(data): ) = i if not account_name: - error_messages.append(f"Row {line_no}: Please enter Account Name") + error_messages.append(_("Row {0}: Please enter Account Name").format(line_no)) name = account_name if account_number: account_number = cstr(account_number).strip() account_name = f"{account_number} - {account_name}" + if parent_account_number: + parent_account = f"{cstr(parent_account_number).strip()} - {parent_account}" + parent_accounts[account_name] = parent_account charts_map[account_name] = {} charts_map[account_name]["account_name"] = name @@ -293,15 +278,14 @@ def build_forest(data): charts_map[account_name]["root_type"] = root_type if account_currency: charts_map[account_name]["account_currency"] = account_currency - path = return_parent(data, account_name)[::-1] - paths.append(path) # List of path is created line_no += 1 if error_messages: frappe.throw("
".join(error_messages)) out = {} - for path in paths: + for account_name in charts_map: + path = return_parent(parent_accounts, account_name)[::-1] for n, account_name in enumerate(path): set_nested( out, path[: n + 1], charts_map[account_name] @@ -361,14 +345,14 @@ def get_template(template_type, company): if template_type == "Blank Template": for root_type in get_root_types(): - writer.writerow(["", "", "", "", 1, "", root_type]) + writer.writerow(["", "", "", "", 1, "", root_type, ""]) for account in get_mandatory_group_accounts(): - writer.writerow(["", "", "", "", 1, account, "Asset"]) + writer.writerow(["", "", "", "", 1, account, "Asset", ""]) for account_type in get_mandatory_account_types(): writer.writerow( - ["", "", "", "", 0, account_type.get("account_type"), account_type.get("root_type")] + ["", "", "", "", 0, account_type.get("account_type"), account_type.get("root_type"), ""] ) else: writer = get_sample_template(writer, company) @@ -395,6 +379,8 @@ def validate_accounts(file_doc, extension): accounts_dict = {} for account in accounts: + if not account.get("account_name"): + frappe.throw(_("Please enter Account Name for every row in the uploaded template")) accounts_dict.setdefault(account["account_name"], account) if "parent_account" not in account: msg = _(