From f07ff92a35c876e024703856cc2c0d31437b6dd5 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 1/9] 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 f795dfa83e..f67c59c254 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 61968cf627..9a0234a91f 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 @@ from erpnext.accounts.doctype.account.chart_of_accounts.chart_of_accounts import 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) From 648b2d72a547caa0703a334cd61111a7ba35e24b Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Mon, 20 Sep 2021 15:27:12 +0530 Subject: [PATCH 2/9] perf: extract loop invariant db calls --- erpnext/controllers/accounts_controller.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/erpnext/controllers/accounts_controller.py b/erpnext/controllers/accounts_controller.py index b90db054b5..6034cced8d 100644 --- a/erpnext/controllers/accounts_controller.py +++ b/erpnext/controllers/accounts_controller.py @@ -980,6 +980,9 @@ 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() + for item in self.get("items"): if item.get(item_ref_dn): ref_amt = flt(frappe.db.get_value(ref_dt + " Item", @@ -1009,9 +1012,7 @@ 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') - - if total_billed_amt - max_allowed_amt > 0.01 and role_allowed_to_over_bill not in frappe.get_roles(): + if total_billed_amt - max_allowed_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")): From ef66beec51dc98a12207b4c528fe0632413118b4 Mon Sep 17 00:00:00 2001 From: Saqib Date: Mon, 20 Sep 2021 15:33:29 +0530 Subject: [PATCH 3/9] fix(plaid): query to check if bank account exists --- .../doctype/plaid_settings/plaid_settings.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/erpnext/erpnext_integrations/doctype/plaid_settings/plaid_settings.py b/erpnext/erpnext_integrations/doctype/plaid_settings/plaid_settings.py index d2748c2faa..310afed481 100644 --- a/erpnext/erpnext_integrations/doctype/plaid_settings/plaid_settings.py +++ b/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", { - 'account_name': account["name"], - 'bank': bank["bank_name"] - }) + bank_account_name = "{} - {}".format(account["name"], bank["bank_name"]) + existing_bank_account = frappe.db.exists("Bank Account", bank_account_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): From 0ff7367f390682587346ebd54dbddf1e8eb5bb9e Mon Sep 17 00:00:00 2001 From: Subin Tom <36098155+nemesis189@users.noreply.github.com> Date: Mon, 20 Sep 2021 16:13:36 +0530 Subject: [PATCH 4/9] fix: Tax Breakup table headers fix (#27596) --- erpnext/regional/india/utils.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/erpnext/regional/india/utils.py b/erpnext/regional/india/utils.py index 903168d009..091cc8847c 100644 --- a/erpnext/regional/india/utils.py +++ b/erpnext/regional/india/utils.py @@ -112,10 +112,7 @@ 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 + return [_("Item"), _("Taxable Amount")] + tax_accounts def get_itemised_tax_breakup_data(doc, account_wise=False, hsn_wise=False): itemised_tax = get_itemised_tax(doc.taxes, with_tax_account=account_wise) From 43bf82b58beb0d057b10b0f98c398908acc8c00a Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Mon, 20 Sep 2021 16:31:20 +0530 Subject: [PATCH 5/9] fix: warn when overbilling checks are skipped. --- erpnext/controllers/accounts_controller.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/erpnext/controllers/accounts_controller.py b/erpnext/controllers/accounts_controller.py index 6034cced8d..22efcdf1cc 100644 --- a/erpnext/controllers/accounts_controller.py +++ b/erpnext/controllers/accounts_controller.py @@ -983,6 +983,8 @@ class AccountsController(TransactionBase): 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): ref_amt = flt(frappe.db.get_value(ref_dt + " Item", @@ -1012,12 +1014,19 @@ class AccountsController(TransactionBase): total_billed_amt = abs(total_billed_amt) max_allowed_amt = abs(max_allowed_amt) - if total_billed_amt - max_allowed_amt > 0.01 and role_allowed_to_over_bill not in user_roles: + overbill_amt = total_billed_amt - max_allowed_amt + total_overbilled_amt += overbill_amt + + 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(_("INFO: Overbilling of {} ignored because you have {} role.") + .format(total_overbilled_amt, role_allowed_to_over_bill)) + 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)) From 5e4fbba753f9a968a03324773cbff65c04d67f0c Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Mon, 20 Sep 2021 16:36:27 +0530 Subject: [PATCH 6/9] refactor: add guard clause in for loop Reduce overly indented code/improve readability. --- erpnext/controllers/accounts_controller.py | 69 +++++++++++----------- 1 file changed, 36 insertions(+), 33 deletions(-) diff --git a/erpnext/controllers/accounts_controller.py b/erpnext/controllers/accounts_controller.py index 22efcdf1cc..024a297870 100644 --- a/erpnext/controllers/accounts_controller.py +++ b/erpnext/controllers/accounts_controller.py @@ -986,46 +986,49 @@ class AccountsController(TransactionBase): total_overbilled_amt = 0.0 for item in self.get("items"): - if item.get(item_ref_dn): - 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") - .format(item.item_code, ref_dt)) - else: - already_billed = frappe.db.sql(""" - select sum(%s) - from `tab%s` - where %s=%s and docstatus=1 and parent != %s - """ % (based_on, self.doctype + " Item", item_ref_dn, '%s', '%s'), - (item.get(item_ref_dn), self.name))[0][0] + if not item.get(item_ref_dn): + continue - total_billed_amt = flt(flt(already_billed) + flt(item.get(based_on)), - self.precision(based_on, item)) + 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") + .format(item.item_code, ref_dt)) + continue - allowance, item_allowance, global_qty_allowance, global_amount_allowance = \ - get_allowance_for(item.item_code, item_allowance, global_qty_allowance, global_amount_allowance, "amount") + already_billed = frappe.db.sql(""" + select sum(%s) + from `tab%s` + where %s=%s and docstatus=1 and parent != %s + """ % (based_on, self.doctype + " Item", item_ref_dn, '%s', '%s'), + (item.get(item_ref_dn), self.name))[0][0] - max_allowed_amt = flt(ref_amt * (100 + allowance) / 100) + total_billed_amt = flt(flt(already_billed) + flt(item.get(based_on)), + self.precision(based_on, item)) - if total_billed_amt < 0 and max_allowed_amt < 0: - # while making debit note against purchase return entry(purchase receipt) getting overbill error - total_billed_amt = abs(total_billed_amt) - max_allowed_amt = abs(max_allowed_amt) + allowance, item_allowance, global_qty_allowance, global_amount_allowance = \ + get_allowance_for(item.item_code, item_allowance, global_qty_allowance, global_amount_allowance, "amount") - overbill_amt = total_billed_amt - max_allowed_amt - total_overbilled_amt += overbill_amt + max_allowed_amt = flt(ref_amt * (100 + allowance) / 100) - 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 total_billed_amt < 0 and max_allowed_amt < 0: + # while making debit note against purchase return entry(purchase receipt) getting overbill error + total_billed_amt = abs(total_billed_amt) + max_allowed_amt = abs(max_allowed_amt) - if role_allowed_to_over_bill in user_roles and total_overbilled_amt > 0.1: - frappe.msgprint(_("INFO: Overbilling of {} ignored because you have {} role.") - .format(total_overbilled_amt, role_allowed_to_over_bill)) + overbill_amt = total_billed_amt - max_allowed_amt + total_overbilled_amt += overbill_amt + + 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(_("INFO: Overbilling of {} ignored because you have {} role.") + .format(total_overbilled_amt, role_allowed_to_over_bill)) 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") From 21a955d20b83f6c55aafab294be0f1f4d4b89153 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Mon, 20 Sep 2021 16:58:23 +0530 Subject: [PATCH 7/9] fix(ux): better error message Co-authored-by: Saqib --- erpnext/controllers/accounts_controller.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/erpnext/controllers/accounts_controller.py b/erpnext/controllers/accounts_controller.py index 024a297870..5359089698 100644 --- a/erpnext/controllers/accounts_controller.py +++ b/erpnext/controllers/accounts_controller.py @@ -993,8 +993,8 @@ class AccountsController(TransactionBase): 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") - .format(item.item_code, ref_dt)) + _("System will not check overbilling since amount for Item {0} in {1} is zero") + .format(item.item_code, ref_dt), title=_("Warning"), indicator="orange") continue already_billed = frappe.db.sql(""" @@ -1002,7 +1002,7 @@ class AccountsController(TransactionBase): from `tab%s` where %s=%s and docstatus=1 and parent != %s """ % (based_on, self.doctype + " Item", item_ref_dn, '%s', '%s'), - (item.get(item_ref_dn), self.name))[0][0] + (item.get(item_ref_dn), self.name))[0][0] total_billed_amt = flt(flt(already_billed) + flt(item.get(based_on)), self.precision(based_on, item)) @@ -1027,8 +1027,8 @@ class AccountsController(TransactionBase): 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(_("INFO: Overbilling of {} ignored because you have {} role.") - .format(total_overbilled_amt, role_allowed_to_over_bill)) + 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") From 190c2d84da998344772fbba9675fada8f555b4fa Mon Sep 17 00:00:00 2001 From: Anuja Pawar <60467153+Anuja-pawar@users.noreply.github.com> Date: Mon, 20 Sep 2021 21:32:38 +0530 Subject: [PATCH 8/9] fix: reference row added in allocation table (#27602) --- .../payment_reconciliation_allocation.json | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/erpnext/accounts/doctype/payment_reconciliation_allocation/payment_reconciliation_allocation.json b/erpnext/accounts/doctype/payment_reconciliation_allocation/payment_reconciliation_allocation.json index 3653501432..b8c65eea84 100644 --- a/erpnext/accounts/doctype/payment_reconciliation_allocation/payment_reconciliation_allocation.json +++ b/erpnext/accounts/doctype/payment_reconciliation_allocation/payment_reconciliation_allocation.json @@ -7,6 +7,7 @@ "field_order": [ "reference_type", "reference_name", + "reference_row", "column_break_3", "invoice_type", "invoice_number", @@ -121,11 +122,17 @@ "label": "Amount", "options": "Currency", "read_only": 1 + }, + { + "fieldname": "reference_row", + "fieldtype": "Data", + "hidden": 1, + "label": "Reference Row" } ], "istable": 1, "links": [], - "modified": "2021-08-30 10:58:42.665107", + "modified": "2021-09-20 17:23:09.455803", "modified_by": "Administrator", "module": "Accounts", "name": "Payment Reconciliation Allocation", From 6d8e869a92285a9be890b58b1754cb373cfcd6fd Mon Sep 17 00:00:00 2001 From: "Abdullah A. Zaqout" <26047413+zaqoutabed@users.noreply.github.com> Date: Mon, 20 Sep 2021 19:05:37 +0300 Subject: [PATCH 9/9] fix(pos): check if item exists before fetching available qty (#27581) --- erpnext/selling/page/point_of_sale/pos_item_details.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/erpnext/selling/page/point_of_sale/pos_item_details.js b/erpnext/selling/page/point_of_sale/pos_item_details.js index d899c5c19b..ec861d7c53 100644 --- a/erpnext/selling/page/point_of_sale/pos_item_details.js +++ b/erpnext/selling/page/point_of_sale/pos_item_details.js @@ -236,7 +236,7 @@ erpnext.PointOfSale.ItemDetails = class { if (this.value) { me.events.form_updated(me.current_item, 'warehouse', this.value).then(() => { me.item_stock_map = me.events.get_item_stock_map(); - const available_qty = me.item_stock_map[me.item_row.item_code][this.value]; + const available_qty = me.item_stock_map[me.item_row.item_code] && me.item_stock_map[me.item_row.item_code][this.value]; if (available_qty === undefined) { me.events.get_available_stock(me.item_row.item_code, this.value).then(() => { // item stock map is updated now reset warehouse