Merge branch 'develop' into unlink-po-on-cancelling-so

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,6 +133,7 @@ var create_reset_button = function(frm) {
 };
 
 var generate_tree_preview = function(frm) {
+	if (frm.doc.import_file) {
 		let parent = __('All Accounts');
 		$(frm.fields_dict['chart_tree'].wrapper).empty(); // empty wrapper to load new data
 
@@ -170,4 +153,5 @@ var generate_tree_preview = function(frm) {
 				parent = node.value;
 			}
 		});
+	}
 };

erpnext/accounts/doctype/chart_of_accounts_importer/chart_of_accounts_importer.py

@@ -25,8 +25,16 @@ from erpnext.accounts.doctype.account.chart_of_accounts.chart_of_accounts import
 
 
 class ChartofAccountsImporter(Document):
-	def validate(self):
+	pass
-		validate_accounts(self.import_file)
+
+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)

erpnext/controllers/accounts_controller.py

@@ -1027,15 +1027,23 @@ class AccountsController(TransactionBase):
 		item_allowance = {}
 		global_qty_allowance, global_amount_allowance = None, None
 
+		role_allowed_to_over_bill = frappe.db.get_single_value('Accounts Settings', 'role_allowed_to_over_bill')
+		user_roles = frappe.get_roles()
+
+		total_overbilled_amt = 0.0
+
 		for item in self.get("items"):
-			if item.get(item_ref_dn):
+			if not item.get(item_ref_dn):
+				continue
+
 			ref_amt = flt(frappe.db.get_value(ref_dt + " Item",
 				item.get(item_ref_dn), based_on), self.precision(based_on, item))
 			if not ref_amt:
 				frappe.msgprint(
-						_("Warning: System will not check overbilling since amount for Item {0} in {1} is zero")
+					_("System will not check overbilling since amount for Item {0} in {1} is zero")
-							.format(item.item_code, ref_dt))
+						.format(item.item_code, ref_dt), title=_("Warning"), indicator="orange")
-				else:
+				continue
+
 			already_billed = frappe.db.sql("""
 				select sum(%s)
 				from `tab%s`
@@ -1056,14 +1064,19 @@ class AccountsController(TransactionBase):
 				total_billed_amt = abs(total_billed_amt)
 				max_allowed_amt = abs(max_allowed_amt)
 
-					role_allowed_to_over_bill = frappe.db.get_single_value('Accounts Settings', 'role_allowed_to_over_bill')
+			overbill_amt = total_billed_amt - max_allowed_amt
+			total_overbilled_amt += overbill_amt
 
-					if total_billed_amt - max_allowed_amt > 0.01 and role_allowed_to_over_bill not in frappe.get_roles():
+			if overbill_amt > 0.01 and role_allowed_to_over_bill not in user_roles:
 				if self.doctype != "Purchase Invoice":
 					self.throw_overbill_exception(item, max_allowed_amt)
 				elif not cint(frappe.db.get_single_value("Buying Settings", "bill_for_rejected_quantity_in_purchase_invoice")):
 					self.throw_overbill_exception(item, max_allowed_amt)
 
+		if role_allowed_to_over_bill in user_roles and total_overbilled_amt > 0.1:
+			frappe.msgprint(_("Overbilling of {} ignored because you have {} role.")
+					.format(total_overbilled_amt, role_allowed_to_over_bill), title=_("Warning"), indicator="orange")
+
 	def throw_overbill_exception(self, item, max_allowed_amt):
 		frappe.throw(_("Cannot overbill for Item {0} in row {1} more than {2}. To allow over-billing, please set allowance in Accounts Settings")
 			.format(item.item_code, item.idx, max_allowed_amt))

erpnext/erpnext_integrations/doctype/plaid_settings/plaid_settings.py

@@ -85,10 +85,8 @@ def add_bank_accounts(response, bank, company):
 		if not acc_subtype:
 			add_account_subtype(account["subtype"])
 
-		existing_bank_account = frappe.db.exists("Bank Account", {
+		bank_account_name = "{} - {}".format(account["name"], bank["bank_name"])
-			'account_name': account["name"],
+		existing_bank_account = frappe.db.exists("Bank Account", bank_account_name)
-			'bank': bank["bank_name"]
-		})
 
 		if not existing_bank_account:
 			try:
@@ -197,6 +195,7 @@ def get_transactions(bank, bank_account=None, start_date=None, end_date=None):
 
 	plaid = PlaidConnector(access_token)
 
+	transactions = []
 	try:
 		transactions = plaid.get_transactions(start_date=start_date, end_date=end_date, account_id=account_id)
 	except ItemError as e:
@@ -205,7 +204,7 @@ def get_transactions(bank, bank_account=None, start_date=None, end_date=None):
 			msg += _("Please refresh or reset the Plaid linking of the Bank {}.").format(bank) + " "
 			frappe.log_error(msg, title=_("Plaid Link Refresh Required"))
 
-	return transactions or []
+	return transactions
 
 
 def new_bank_transaction(transaction):

erpnext/regional/india/utils.py

@@ -112,9 +112,6 @@ def validate_gstin_check_digit(gstin, label='GSTIN'):
 		frappe.throw(_("""Invalid {0}! The check digit validation has failed. Please ensure you've typed the {0} correctly.""").format(label))
 
 def get_itemised_tax_breakup_header(item_doctype, tax_accounts):
-	if frappe.get_meta(item_doctype).has_field('gst_hsn_code'):
-		return [_("HSN/SAC"), _("Taxable Amount")] + tax_accounts
-	else:
 	return [_("Item"), _("Taxable Amount")] + tax_accounts
 
 def get_itemised_tax_breakup_data(doc, account_wise=False, hsn_wise=False):