Compare commits

...

8 Commits

Author SHA1 Message Date
Frappe PR Bot
0b50853985 chore(release): Bumped to Version 16.34.1
## [16.34.1](https://github.com/frappe/erpnext/compare/v16.34.0...v16.34.1) (2026-09-02)

### Bug Fixes

* filter cancelled BOMs in BOM Stock Analysis (backport [#58647](https://github.com/frappe/erpnext/issues/58647)) ([#58693](https://github.com/frappe/erpnext/issues/58693)) ([d6f9dde](d6f9dde3cb))
* improve message formatting and translation for validation issues ([#58425](https://github.com/frappe/erpnext/issues/58425)) ([a43de7c](a43de7ce95))
* include payment deductions in sales/purchase register ledger bal… (backport [#58437](https://github.com/frappe/erpnext/issues/58437)) ([#58680](https://github.com/frappe/erpnext/issues/58680)) ([4137401](41374019ba))
2026-09-02 12:24:04 +00:00
Sagar Vora
c19ddf187e Merge pull request #58707 from frappe/version-16-hotfix
chore: release v16
2026-09-02 17:51:22 +05:30
Sagar Vora
5f13a04633 Merge pull request #58704 from frappe/mergify/bp/version-16-hotfix/pr-58697
fix!: improve validation in financial report template (backport #58697)
2026-09-02 17:41:50 +05:30
Sagar Vora
7aad59b129 fix!: improve validation in financial report template 2026-09-02 17:31:36 +05:30
Sagar Vora
7c61dfe3e0 Merge pull request #58487 from frappe/mergify/bp/version-16-hotfix/pr-58425
fix: improve message formatting and translation for validation issues (backport #58425)
2026-09-02 17:24:03 +05:30
mergify[bot]
41374019ba fix: include payment deductions in sales/purchase register ledger bal… (backport #58437) (#58680) 2026-09-02 17:00:42 +05:30
mergify[bot]
d6f9dde3cb fix: filter cancelled BOMs in BOM Stock Analysis (backport #58647) (#58693) 2026-09-02 15:28:17 +05:30
Abdeali Chharchhodawala
a43de7ce95 fix: improve message formatting and translation for validation issues (#58425)
(cherry picked from commit 6842ebb186)
2026-08-27 10:57:38 +00:00
7 changed files with 208 additions and 67 deletions

View File

@@ -6,7 +6,7 @@ import frappe
from frappe.model.document import Document
from frappe.utils.user import is_website_user
__version__ = "16.34.0"
__version__ = "16.34.1"
def get_default_company(user=None):

View File

@@ -31,6 +31,7 @@ from erpnext.accounts.doctype.financial_report_template.financial_report_validat
AccountFilterValidator,
CalculationFormulaValidator,
DependencyValidator,
get_valid_api_method,
)
from erpnext.accounts.report.financial_statements import (
get_columns,
@@ -1164,10 +1165,12 @@ class RowProcessor:
def _process_api_row(self, row) -> RowData:
api_path = row.calculation_formula
# TODO
method = get_valid_api_method(api_path)
try:
values = frappe.call(api_path, filters=self.context.filters, periods=self.period_list, row=row)
# nosemgrep: frappe-semgrep-rules.rules.security.frappe-codeinjection-eval
values = frappe.call(method, filters=self.context.filters, periods=self.period_list, row=row)
if row.reverse_sign:
values = [-1 * v for v in values]

View File

@@ -163,7 +163,7 @@ function show_accounts_tree(template_rows, has_selection) {
fieldname: "company",
fieldtype: "Link",
options: "Company",
label: "Company",
label: __("Company"),
reqd: 1,
default: frappe.defaults.get_user_default("Company"),
onchange: () => {
@@ -176,7 +176,7 @@ function show_accounts_tree(template_rows, has_selection) {
fieldname: "view_type",
fieldtype: "Select",
options: ["Missing Accounts", "Filtered Accounts"],
label: "View",
label: __("View"),
default: has_selection ? "Filtered Accounts" : "Missing Accounts",
reqd: 1,
onchange: () => {
@@ -192,10 +192,10 @@ function show_accounts_tree(template_rows, has_selection) {
{
fieldname: "tip",
fieldtype: "HTML",
label: "Tip",
label: __("Tip"),
options: `
<div class="alert alert-success" role="alert">
Tip: Select report lines to view their accounts
${__("Tip: Select report lines to view their accounts")}
</div>
`,
depends_on: has_selection ? "eval: false" : "eval: true",
@@ -203,7 +203,7 @@ function show_accounts_tree(template_rows, has_selection) {
{
fieldname: "tree_area",
fieldtype: "HTML",
label: "Chart of Accounts",
label: __("Chart of Accounts"),
read_only: 1,
depends_on: "eval: doc.company",
},
@@ -288,14 +288,14 @@ function update_formula_label(frm, data_source) {
if (!field) return;
const labels = {
"Account Data": "Account Filter",
"Custom API": "API Method Path",
"Account Data": __("Account Filter"),
"Custom API": __("API Method Path"),
};
grid.update_docfield_property(
"calculation_formula",
"label",
labels[data_source] || "Calculation Formula"
labels[data_source] || __("Calculation Formula")
);
}
@@ -370,7 +370,7 @@ function update_formula_description(frm, data_source) {
description_html = `
<div ${container_style}>
<h5 ${title_style}>Custom API Setup</h5>
<p ${text_style}>Path to your custom method that returns financial data.</p>
<p ${text_style}>Path to your custom whitelisted method that returns financial data. It must permit GET requests.</p>
<h6 ${subtitle_style}>Format:</h6>
<ul ${list_style}>
@@ -380,7 +380,8 @@ function update_formula_description(frm, data_source) {
<h6 ${subtitle_style}>Method Signature:</h6>
<div ${code_style}>
<pre ${pre_style}>def get_custom_data(filters, periods, row): <br>&nbsp; # filters: dict — report filters (company, period, etc.) <br>&nbsp; # periods: list[dict] — period definitions <br>&nbsp; # row: dict — the current report row <br><br>&nbsp; return [1000.0, 1200.0, 1150.0] # one value per period</pre>
<!-- &#10; is used for line breaks since frappe.render replaces newlines with spaces -->
<pre ${pre_style} class="language-python">@frappe.whitelist(methods=["GET"])&#10;def get_custom_data(filters, periods, row):&#10; # filters: dict — report filters (company, period, etc.)&#10; # periods: list[dict] — period definitions&#10; # row: dict — the current report row&#10;&#10; return [1000.0, 1200.0, 1150.0] # one value per period</pre>
</div>
<h6 ${subtitle_style}>Return Format:</h6>

View File

@@ -10,18 +10,41 @@ from enum import Enum
from typing import Any, ClassVar
import frappe
from frappe import _
from frappe import _, is_whitelisted
from frappe.database.operator_map import OPERATOR_MAP
from frappe.database.query import SQLFunctionParser
def get_valid_api_method(api_path: str):
"""Resolve `api_path`, ensuring it is whitelisted and permits GET (i.e. read-only)."""
method = frappe.get_attr(api_path)
is_whitelisted(method)
if "GET" not in frappe.allowed_http_methods_for_whitelisted_func.get(method, ()):
frappe.throw(
_("Method {0} must permit GET requests").format(frappe.bold(api_path)),
frappe.PermissionError,
title=_("Method Not Allowed"),
)
return method
def get_formula_field_label(data_source: str) -> str:
# Must mirror the `labels` map in financial_report_template.js (update_formula_label),
labels = {
"Account Data": _("Account Filter"),
"Custom API": _("API Method Path"),
}
return labels.get(data_source, _("Calculation Formula"))
@dataclass
class ValidationIssue:
"""Represents a single validation issue"""
message: str
row_idx: int | None = None
field: str | None = None
details: dict[str, Any] = None
def __post_init__(self):
@@ -29,10 +52,9 @@ class ValidationIssue:
self.details = {}
def __str__(self) -> str:
prefix = f"Row {self.row_idx}: " if self.row_idx else ""
field_info = f"[{self.field}] " if self.field else ""
message = f"{prefix}{field_info}{self.message}"
return _(message)
if self.row_idx:
return _("Row {0}: {1}", context="Financial Report Template").format(self.row_idx, self.message)
return self.message
@dataclass
@@ -134,7 +156,9 @@ class TemplateStructureValidator(Validator):
if not re.match(r"^[A-Za-z][A-Za-z0-9_-]*$", ref_code):
result.add_error(
ValidationIssue(
message=f"Invalid line reference format: '{ref_code}'. Must start with letter and contain only letters, numbers, underscores, and hyphens",
message=_(
"Invalid line reference format: '{0}'. Must start with letter and contain only letters, numbers, underscores, and hyphens"
).format(ref_code),
row_idx=row.idx,
)
)
@@ -143,7 +167,7 @@ class TemplateStructureValidator(Validator):
if ref_code in used_codes:
result.add_error(
ValidationIssue(
message=f"Duplicate line reference: '{ref_code}'",
message=_("Duplicate line reference: '{0}'").format(ref_code),
row_idx=row.idx,
)
)
@@ -159,7 +183,7 @@ class TemplateStructureValidator(Validator):
if row.data_source == "Account Data" and not row.balance_type:
result.add_error(
ValidationIssue(
message="Balance Type is required for Account Data",
message=_("Balance Type is required for Account Data"),
row_idx=row.idx,
)
)
@@ -169,7 +193,9 @@ class TemplateStructureValidator(Validator):
if not row.calculation_formula:
result.add_error(
ValidationIssue(
message=f"Formula is required for {row.data_source}",
message=_("{0} is required for {1}").format(
get_formula_field_label(row.data_source), row.data_source
),
row_idx=row.idx,
)
)
@@ -226,7 +252,7 @@ class DependencyValidator(Validator):
cycle = [*path[cycle_start:], node]
result.add_error(
ValidationIssue(
message=f"Circular dependency detected: {''.join(cycle)}",
message=_("Circular dependency detected: {0}").format("".join(cycle)),
)
)
return
@@ -258,7 +284,7 @@ class DependencyValidator(Validator):
row_idx = self._get_row_idx(ref_code)
result.add_error(
ValidationIssue(
message=f"Line References undefined in Formula: {', '.join(undefined)}",
message=_("Line References undefined in Formula: {0}").format(", ".join(undefined)),
row_idx=row_idx,
)
)
@@ -288,9 +314,10 @@ class CalculationFormulaValidator(Validator):
if not row.calculation_formula:
result.add_error(
ValidationIssue(
message="Formula is required for Calculated Amount",
message=_("{0} is required for Calculated Amount").format(
get_formula_field_label(row.data_source)
),
row_idx=row.idx,
field="Formula",
)
)
return result
@@ -302,7 +329,7 @@ class CalculationFormulaValidator(Validator):
if not self._are_parentheses_balanced(formula):
result.add_error(
ValidationIssue(
message="Formula has unbalanced parentheses",
message=_("Formula has unbalanced parentheses"),
row_idx=row.idx,
)
)
@@ -314,7 +341,7 @@ class CalculationFormulaValidator(Validator):
if row.reference_code and row.reference_code in refs:
result.add_error(
ValidationIssue(
message=f"Formula references itself ('{row.reference_code}')",
message=_("Formula references itself ('{0}')").format(row.reference_code),
row_idx=row.idx,
)
)
@@ -324,7 +351,7 @@ class CalculationFormulaValidator(Validator):
if undefined:
result.add_error(
ValidationIssue(
message=f"Formula references undefined codes: {', '.join(undefined)}",
message=_("Formula references undefined codes: {0}").format(", ".join(undefined)),
row_idx=row.idx,
)
)
@@ -334,7 +361,7 @@ class CalculationFormulaValidator(Validator):
if eval_error:
result.add_error(
ValidationIssue(
message=f"Formula evaluation error: {eval_error}",
message=_("Formula evaluation error: {0}").format(eval_error),
row_idx=row.idx,
)
)
@@ -371,7 +398,7 @@ class CalculationFormulaValidator(Validator):
result = frappe.safe_eval(formula, eval_globals=None, eval_locals=context)
if not isinstance(result, (int, float)): # noqa: UP038
return f"Formula must return a numeric value, got {type(result).__name__}"
return _("Formula must return a numeric value, got {0}").format(type(result).__name__)
return None
except Exception as e:
@@ -394,9 +421,10 @@ class AccountFilterValidator(Validator):
if not row.calculation_formula:
result.add_error(
ValidationIssue(
message="Account filter is required for Account Data",
message=_("{0} is required for Account Data").format(
get_formula_field_label(row.data_source)
),
row_idx=row.idx,
field="Formula",
)
)
return result
@@ -412,18 +440,18 @@ class AccountFilterValidator(Validator):
if error:
result.add_error(
ValidationIssue(
message=error,
message=_("{0}: {1}").format(get_formula_field_label(row.data_source), error),
row_idx=row.idx,
field="Account Filter",
)
)
except json.JSONDecodeError as e:
result.add_error(
ValidationIssue(
message=f"Invalid JSON format: {e!s}",
message=_("{0}: Invalid JSON format: {1}").format(
get_formula_field_label(row.data_source), str(e)
),
row_idx=row.idx,
field="Account Filter",
)
)
@@ -438,36 +466,36 @@ class AccountFilterValidator(Validator):
# simple condition: [field, operator, value]
if isinstance(filter_config, list):
if len(filter_config) != 3:
return "Filter must be [field, operator, value]"
return _("Filter must be [field, operator, value]")
field, operator, value = filter_config
if not isinstance(field, str) or not isinstance(operator, str):
return "Field and operator must be strings"
return _("Field and operator must be strings")
display = (field if advanced_filtering else self.account_meta.get_label(field)) or field
if field not in account_fields:
return f"Field '{display}' is not a valid Account field"
return _("Field '{0}' is not a valid Account field").format(display)
if operator.casefold() not in OPERATOR_MAP:
return f"Invalid operator '{operator}'"
return _("Invalid operator '{0}'").format(operator)
if operator in ["in", "not in"] and not isinstance(value, list):
return f"Operator '{operator}' requires a list value"
return _("Operator '{0}' requires a list value").format(operator)
# logical condition: {"and": [condition1, condition2]}
elif isinstance(filter_config, dict):
if len(filter_config) != 1:
return "Logical condition must have exactly one operator"
return _("Logical condition must have exactly one operator")
op = next(iter(filter_config.keys())).lower()
if op not in ["and", "or"]:
return "Logical operators must be 'and' or 'or'"
return _("Logical operators must be 'and' or 'or'")
conditions = filter_config[next(iter(filter_config.keys()))]
if not isinstance(conditions, list) or len(conditions) < 1:
return "Logical conditions need at least 1 sub-condition"
return _("Logical conditions need at least 1 sub-condition")
# recursive
for condition in conditions:
@@ -475,7 +503,7 @@ class AccountFilterValidator(Validator):
if error:
return error
else:
return "Filter must be a list or dict"
return _("Filter must be a list or dict")
return None
@@ -511,34 +539,31 @@ class FormulaValidator(Validator):
if "." not in api_path:
result.add_error(
ValidationIssue(
message="Custom API path should be in format: app.module.method",
message=_("{0} should be in format: app.module.method").format(
get_formula_field_label(row.data_source)
),
row_idx=row.idx,
field="Formula",
)
)
return result
# Method exists?
try:
module_path, method_name = api_path.rsplit(".", 1)
module = frappe.get_module(module_path)
if not hasattr(module, method_name):
result.add_error(
ValidationIssue(
message=f"Method '{method_name}' not found in module '{module_path}' (might be environment-specific)",
row_idx=row.idx,
field="Formula",
)
)
get_valid_api_method(api_path)
except Exception as e:
result.add_error(
ValidationIssue(
message=f"Could not validate API path: {e!s}",
row_idx=row.idx,
field="Formula",
if isinstance(e, frappe.PermissionError | frappe.ValidationError):
# frappe.throw inside get_valid_api_method logs a message that would pop up in UI
frappe.clear_last_message()
if isinstance(e, frappe.PermissionError):
message = _("{0}: Method '{1}' must be whitelisted and permit GET requests").format(
get_formula_field_label(row.data_source), api_path
)
)
else:
message = _("Could not validate {0}: {1}").format(
get_formula_field_label(row.data_source), str(e)
)
result.add_error(ValidationIssue(message=message, row_idx=row.idx))
return result

View File

@@ -2,7 +2,12 @@
# For license information, please see license.txt
import frappe
from frappe.tests.utils import whitelist_for_tests
from erpnext.accounts.doctype.financial_report_template.financial_report_validation import (
FormulaValidator,
get_valid_api_method,
)
from erpnext.tests.utils import ERPNextTestSuite
@@ -72,3 +77,90 @@ class FinancialReportTemplateTestCase(ERPNextTestSuite):
{"doctype": "Financial Report Template", "template_name": template_name, "rows": rows_data}
)
return template
def not_whitelisted_method(**kwargs):
return [42.0]
@whitelist_for_tests(methods=["POST"])
def whitelisted_post_only_method(**kwargs):
return [42.0]
@whitelist_for_tests(methods=["GET"])
def whitelisted_get_method(**kwargs):
return [42.0]
class TestCustomAPIValidation(FinancialReportTemplateTestCase):
"""Custom API rows must point to whitelisted methods that permit GET"""
TEST_MODULE = "erpnext.accounts.doctype.financial_report_template.test_financial_report_template"
NOT_WHITELISTED = f"{TEST_MODULE}.not_whitelisted_method"
WHITELISTED_POST_ONLY = f"{TEST_MODULE}.whitelisted_post_only_method"
WHITELISTED_GET = f"{TEST_MODULE}.whitelisted_get_method"
def create_api_template(self, api_path):
template = self.create_test_template_with_rows(
[
{
"reference_code": "API001",
"display_name": "API Row",
"data_source": "Custom API",
"calculation_formula": api_path,
}
]
)
template.report_type = "Profit and Loss Statement"
return template
def test_get_valid_api_method(self):
self.assertRaises(frappe.PermissionError, get_valid_api_method, self.NOT_WHITELISTED)
self.assertRaises(frappe.PermissionError, get_valid_api_method, self.WHITELISTED_POST_ONLY)
self.assertEqual(get_valid_api_method(self.WHITELISTED_GET), frappe.get_attr(self.WHITELISTED_GET))
def test_save_rejects_invalid_api_methods(self):
for api_path in (self.NOT_WHITELISTED, self.WHITELISTED_POST_ONLY):
template = self.create_api_template(api_path)
self.assertRaises(frappe.ValidationError, template.insert)
def test_save_allows_get_whitelisted_method(self):
template = self.create_api_template(self.WHITELISTED_GET)
template.insert()
template.delete()
def test_engine_rejects_invalid_api_methods(self):
from erpnext.accounts.doctype.financial_report_template.financial_report_engine import (
ReportContext,
RowProcessor,
)
for api_path in (self.NOT_WHITELISTED, self.WHITELISTED_POST_ONLY):
template = self.create_api_template(api_path)
context = ReportContext(template=template, filters={}, period_list=[{"key": "p1"}])
processor = RowProcessor(context)
self.assertRaises(frappe.PermissionError, processor._process_api_row, template.rows[0])
def test_engine_calls_valid_api_method(self):
from erpnext.accounts.doctype.financial_report_template.financial_report_engine import (
ReportContext,
RowProcessor,
)
template = self.create_api_template(self.WHITELISTED_GET)
context = ReportContext(template=template, filters={}, period_list=[{"key": "p1"}])
processor = RowProcessor(context)
row_data = processor._process_api_row(template.rows[0])
self.assertEqual(row_data.values, [42.0])
def test_validation_keeps_message_log_clean(self):
validator = FormulaValidator(frappe._dict(rows=[]))
message_count = len(frappe.local.message_log)
# last path raises AppNotInstalledError, which also logs a message via frappe.throw
for api_path in (self.NOT_WHITELISTED, self.WHITELISTED_POST_ONLY, "missing_app.api.method"):
row = frappe._dict(data_source="Custom API", calculation_formula=api_path, idx=1)
result = validator.validate(row)
self.assertFalse(result.is_valid)
self.assertEqual(len(frappe.local.message_log), message_count)

View File

@@ -304,6 +304,9 @@ def get_payment_entries(filters, args):
pe.mode_of_payment,
pe.project,
pe.cost_center,
pe.payment_type,
pe.source_exchange_rate,
pe.target_exchange_rate,
)
.where(
(pe.docstatus == 1)
@@ -314,6 +317,22 @@ def get_payment_entries(filters, args):
)
query = apply_common_conditions(filters, query, doctype="Payment Entry", payments=True)
payment_entries = query.run(as_dict=True)
if payment_entries:
ded = frappe.qb.DocType("Payment Entry Deduction")
deduction_totals = frappe._dict(
frappe.qb.from_(ded)
.select(ded.parent, Sum(ded.amount))
.where(ded.parent.isin([d.name for d in payment_entries]) & (ded.is_exchange_gain_loss == 0))
.groupby(ded.parent)
.run()
)
for d in payment_entries:
exchange_rate = (
d.source_exchange_rate if d.payment_type == "Receive" else d.target_exchange_rate
) or 1
d.base_grand_total = flt(d.base_grand_total) + flt(deduction_totals.get(d.name)) / exchange_rate
return payment_entries

View File

@@ -9,6 +9,7 @@ frappe.query_reports["BOM Stock Analysis"] = {
fieldtype: "Link",
options: "BOM",
reqd: 1,
get_query: () => ({ filters: { docstatus: 1 } }),
},
{
fieldname: "warehouse",