Merge pull request #58707 from frappe/version-16-hotfix

chore: release v16

erpnext/accounts/doctype/financial_report_template/financial_report_engine.py

@@ -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]

erpnext/accounts/doctype/financial_report_template/financial_report_template.js

@@ -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",
+		"Account Data": __("Account Filter"),
-		"Custom API": "API Method Path",
+		"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>

erpnext/accounts/doctype/financial_report_template/financial_report_validation.py

@@ -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 ""
+		if self.row_idx:
-		field_info = f"[{self.field}] " if self.field else ""
+			return _("Row {0}: {1}", context="Financial Report Template").format(self.row_idx, self.message)
-		message = f"{prefix}{field_info}{self.message}"
+		return self.message
-		return _(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)
+			get_valid_api_method(api_path)
-			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",
-					)
-				)
 		except Exception as e:
-			result.add_error(
+			if isinstance(e, frappe.PermissionError | frappe.ValidationError):
-				ValidationIssue(
+				# frappe.throw inside get_valid_api_method logs a message that would pop up in UI
-					message=f"Could not validate API path: {e!s}",
+				frappe.clear_last_message()
-					row_idx=row.idx,
+
-					field="Formula",
+			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
 

erpnext/accounts/doctype/financial_report_template/test_financial_report_template.py

@@ -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)

erpnext/accounts/report/utils.py

@@ -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
 
 

erpnext/manufacturing/report/bom_stock_analysis/bom_stock_analysis.js

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