From 30e02f092d9422cf1f8a0489f70a5e5f60cd99e2 Mon Sep 17 00:00:00 2001 From: Frappe PR Bot Date: Wed, 8 Sep 2021 16:41:41 +0530 Subject: [PATCH] fix: General Ledger translation issues (#27298) (#27392) * fix: remove translations from GL report options Options need not be translated, their display label gets translated client side. * fix: make group by options translatable * ci: semgrep rule for translated options in report Co-authored-by: Ankush Menat (cherry picked from commit fa819f2fb0523276f64ed80c885088ac0596933d) --- .github/helper/semgrep_rules/report.yml | 13 +++++++++++ .../report/general_ledger/general_ledger.js | 23 ++++++++++++++++--- .../report/general_ledger/general_ledger.py | 20 ++++++++-------- 3 files changed, 43 insertions(+), 13 deletions(-) diff --git a/.github/helper/semgrep_rules/report.yml b/.github/helper/semgrep_rules/report.yml index 7f3dd011dc1..f2a9b167399 100644 --- a/.github/helper/semgrep_rules/report.yml +++ b/.github/helper/semgrep_rules/report.yml @@ -19,3 +19,16 @@ rules: languages: [python] severity: ERROR +- id: frappe-translated-values-in-business-logic + paths: + include: + - "**/report" + patterns: + - pattern-inside: | + {..., filters: [...], ...} + - pattern: | + {..., options: [..., __("..."), ...], ...} + message: | + Using translated values in options field will require you to translate the values while comparing in business logic. Instead of passing translated labels provide objects that contain both label and value. e.g. { label: __("Option value"), value: "Option value"} + languages: [javascript] + severity: ERROR diff --git a/erpnext/accounts/report/general_ledger/general_ledger.js b/erpnext/accounts/report/general_ledger/general_ledger.js index 095f5eda66a..b2968761c63 100644 --- a/erpnext/accounts/report/general_ledger/general_ledger.js +++ b/erpnext/accounts/report/general_ledger/general_ledger.js @@ -110,9 +110,26 @@ frappe.query_reports["General Ledger"] = { "fieldname":"group_by", "label": __("Group by"), "fieldtype": "Select", - "options": ["", __("Group by Voucher"), __("Group by Voucher (Consolidated)"), - __("Group by Account"), __("Group by Party")], - "default": __("Group by Voucher (Consolidated)") + "options": [ + "", + { + label: __("Group by Voucher"), + value: "Group by Voucher", + }, + { + label: __("Group by Voucher (Consolidated)"), + value: "Group by Voucher (Consolidated)", + }, + { + label: __("Group by Account"), + value: "Group by Account", + }, + { + label: __("Group by Party"), + value: "Group by Party", + }, + ], + "default": "Group by Voucher (Consolidated)" }, { "fieldname":"tax_id", diff --git a/erpnext/accounts/report/general_ledger/general_ledger.py b/erpnext/accounts/report/general_ledger/general_ledger.py index a0445187499..5bd6e583dbb 100644 --- a/erpnext/accounts/report/general_ledger/general_ledger.py +++ b/erpnext/accounts/report/general_ledger/general_ledger.py @@ -62,14 +62,14 @@ def validate_filters(filters, account_details): if not account_details.get(account): frappe.throw(_("Account {0} does not exists").format(account)) - if (filters.get("account") and filters.get("group_by") == _('Group by Account')): + if (filters.get("account") and filters.get("group_by") == 'Group by Account'): filters.account = frappe.parse_json(filters.get('account')) for account in filters.account: if account_details[account].is_group == 0: frappe.throw(_("Can not filter based on Child Account, if grouped by Account")) if (filters.get("voucher_no") - and filters.get("group_by") in [_('Group by Voucher')]): + and filters.get("group_by") in ['Group by Voucher']): frappe.throw(_("Can not filter based on Voucher No, if grouped by Voucher")) if filters.from_date > filters.to_date: @@ -153,7 +153,7 @@ def get_gl_entries(filters, accounting_dimensions): if filters.get("include_dimensions"): order_by_statement = "order by posting_date, creation" - if filters.get("group_by") == _("Group by Voucher"): + if filters.get("group_by") == "Group by Voucher": order_by_statement = "order by posting_date, voucher_type, voucher_no" if filters.get("include_default_book_entries"): @@ -312,13 +312,13 @@ def get_data_with_opening_closing(filters, account_details, accounting_dimension # Opening for filtered account data.append(totals.opening) - if filters.get("group_by") != _('Group by Voucher (Consolidated)'): + if filters.get("group_by") != 'Group by Voucher (Consolidated)': for acc, acc_dict in iteritems(gle_map): # acc if acc_dict.entries: # opening data.append({}) - if filters.get("group_by") != _("Group by Voucher"): + if filters.get("group_by") != "Group by Voucher": data.append(acc_dict.totals.opening) data += acc_dict.entries @@ -327,7 +327,7 @@ def get_data_with_opening_closing(filters, account_details, accounting_dimension data.append(acc_dict.totals.total) # closing - if filters.get("group_by") != _("Group by Voucher"): + if filters.get("group_by") != "Group by Voucher": data.append(acc_dict.totals.closing) data.append({}) else: @@ -357,9 +357,9 @@ def get_totals_dict(): ) def group_by_field(group_by): - if group_by == _('Group by Party'): + if group_by == 'Group by Party': return 'party' - elif group_by in [_('Group by Voucher (Consolidated)'), _('Group by Account')]: + elif group_by in ['Group by Voucher (Consolidated)', 'Group by Account']: return 'account' else: return 'voucher_no' @@ -423,9 +423,9 @@ def get_accountwise_gle(filters, accounting_dimensions, gl_entries, gle_map): elif gle.posting_date <= to_date: update_value_in_dict(gle_map[gle.get(group_by)].totals, 'total', gle) update_value_in_dict(totals, 'total', gle) - if filters.get("group_by") != _('Group by Voucher (Consolidated)'): + if filters.get("group_by") != 'Group by Voucher (Consolidated)': gle_map[gle.get(group_by)].entries.append(gle) - elif filters.get("group_by") == _('Group by Voucher (Consolidated)'): + elif filters.get("group_by") == 'Group by Voucher (Consolidated)': keylist = [gle.get("voucher_type"), gle.get("voucher_no"), gle.get("account")] for dim in accounting_dimensions: keylist.append(gle.get(dim))