${__( + "Set a Bank Account to see match counts." + )}
` + ); + return; + } + + let user_filters = []; + try { + user_filters = brr_get_user_filters_for_stats(frm); + } catch (e) { + user_filters = []; + } + + if (!user_filters.length) { + $el.html( + `${__( + "Add at least one filter to see match counts." + )}
` + ); + return; + } + + const request_id = (frm._brr_stats_request_id = + (frm._brr_stats_request_id || 0) + 1); + $el.html(`${__("Updating...")}
`); + + const filters_str = JSON.stringify(user_filters); + const method = + "banking.klarna_kosma_integration.doctype.bank_reconciliation_rule.bank_reconciliation_rule.get_bank_transaction_match_stats"; + + let data; + try { + const res = await frappe.call({ + method, + args: { + bank_account: frm.doc.bank_account, + filters: filters_str, + ...(frm.doc.name ? { bank_reconciliation_rule: frm.doc.name } : {}), + }, + }); + data = res.message; + } catch (e) { + if (request_id === frm._brr_stats_request_id) { + $el.html( + `${__( + "Could not load match counts." + )}
` + ); + } + return; + } + + if (request_id !== frm._brr_stats_request_id) { + return; + } + + const title = __("Submitted Bank Transactions matching this rule:"); + const counts = __( + "{0} in the last 30 days, {1} in the last 12 months (365 days).", + [String(data.last_30_days), String(data.last_12_months)] + ); + + $el.html( + `${frappe.utils.escape_html( + title + )} ${frappe.utils.escape_html(counts)}
` + ); +} + frappe.ui.form.on("Bank Reconciliation Rule", { - onload: function (frm) { - frm.trigger("render_bt_filters"); - }, refresh(frm) { frm.trigger("set_target_account_query"); + frm.remove_custom_button(__("Open Matches")); + const filters_json = + frm.doc.filters && frm.doc.filters !== "[]" ? frm.doc.filters : "[]"; + let has_filters = false; + try { + has_filters = JSON.parse(filters_json).length > 0; + } catch (e) { + has_filters = false; + } + if (frm.doc.bank_account && has_filters) { + frm.add_custom_button(__("Open Matches"), () => { + brr_open_bank_transaction_list(frm); + }); + } + + if (!brr_stats_host_is_live(frm)) { + frm.trigger("render_bt_filters"); + } else { + brr_fetch_and_show_match_stats(frm); + } }, bank_account(frm) { frm.trigger("set_target_account_query"); + frm._brr_stats_debounced?.(); }, async set_target_account_query(frm) { if (!frm.doc.bank_account) { @@ -58,21 +226,54 @@ frappe.ui.form.on("Bank Reconciliation Rule", { : []; frappe.model.with_doctype("Bank Transaction", () => { + if (!frm._brr_stats_debounced) { + frm._brr_stats_debounced = frappe.utils.debounce(() => { + brr_fetch_and_show_match_stats(frm); + }, 400); + } + const filter_group = new frappe.ui.FilterGroup({ parent: parent, doctype: "Bank Transaction", on_change: () => { frm.set_value("filters", JSON.stringify(filter_group.get_filters())); + frm._brr_stats_debounced?.(); }, }); - filter_group.add_filters_to_filter_group(filters); + frm._bank_transaction_filter_group = filter_group; + + const after_filters_ready = () => { + if (frm.doc.docstatus === 1) { + parent.find(".filter-action-buttons").remove(); + parent.find(".divider").remove(); + parent.find(".remove-filter").remove(); + parent.find(".form-control").prop("disabled", true); + } + + const $stats = $(''); + parent.append($stats); + frm._brr_match_stats_$el = $stats; + brr_fetch_and_show_match_stats(frm); + }; - if (frm.doc.docstatus === 1) { - parent.find(".filter-action-buttons").remove(); - parent.find(".divider").remove(); - parent.find(".remove-filter").remove(); - parent.find(".form-control").prop("disabled", true); + if (filters.length) { + filter_group.toggle_empty_filters(false); + frappe + .run_serially( + filters.map( + (f) => () => filter_group.add_filter(f[0], f[1], f[2], f[3]) + ) + ) + .then(() => { + filter_group.update_filters(); + after_filters_ready(); + }) + .catch(() => { + after_filters_ready(); + }); + } else { + after_filters_ready(); } }); }, diff --git a/banking/klarna_kosma_integration/doctype/bank_reconciliation_rule/bank_reconciliation_rule.py b/banking/klarna_kosma_integration/doctype/bank_reconciliation_rule/bank_reconciliation_rule.py index a2a3a6c7..2d3cc857 100644 --- a/banking/klarna_kosma_integration/doctype/bank_reconciliation_rule/bank_reconciliation_rule.py +++ b/banking/klarna_kosma_integration/doctype/bank_reconciliation_rule/bank_reconciliation_rule.py @@ -2,11 +2,13 @@ # For license information, please see license.txt import json +from typing import Any import frappe from frappe import _ from frappe.exceptions import ValidationError from frappe.model.document import Document +from frappe.utils import add_days, getdate, today from banking.exceptions import CurrencyMismatchError @@ -15,6 +17,77 @@ class NoFiltersError(ValidationError): pass +def _normalize_filter_rows(raw_filters: list) -> list[list[Any]]: + normalized: list[list[Any]] = [] + for row in raw_filters: + if not isinstance(row, list | tuple) or len(row) < 4: + frappe.throw(_("Invalid filter")) + doctype = row[0] + fieldname = row[1] + operator = row[2] + value = row[3] + normalized.append([doctype, fieldname, operator, value]) + return normalized + + +def _bank_transaction_stats_filters( + user_filters: list, + bank_account: str, + min_date: str, +) -> list[list[Any]]: + filters = _normalize_filter_rows(user_filters) + filters.append(["Bank Transaction", "bank_account", "=", bank_account]) + filters.append(["Bank Transaction", "docstatus", "=", 1]) + filters.append(["Bank Transaction", "date", ">=", min_date]) + return filters + + +@frappe.whitelist() +def get_bank_transaction_match_stats( + bank_account: str, + filters: str | None = None, + bank_reconciliation_rule: str | None = None, +) -> dict[str, Any]: + """Count submitted Bank Transactions matching the rule filters (scoped by bank account). + + Windows use calendar days: last 30 days and last 365 days (\"12 months\"), relative to *today*. + """ + if not bank_account: + frappe.throw(_("Bank Account is required")) + + frappe.has_permission("Bank Transaction", ptype="read", throw=True) + + if bank_reconciliation_rule: + rule = frappe.get_doc("Bank Reconciliation Rule", bank_reconciliation_rule) + if rule.bank_account != bank_account: + frappe.throw(_("Bank Account does not match this Bank Reconciliation Rule")) + + try: + user_filters = json.loads(filters or "[]") + except json.JSONDecodeError: + frappe.throw(_("Invalid filters")) + + if not user_filters: + frappe.throw(_("Please define at least one filter")) + + as_of = getdate(today()) + # Match UI wording: transactions with *date* on or after (today - 30) / (today - 365). + min_30 = add_days(as_of, -30) + min_365 = add_days(as_of, -365) + + return { + "last_30_days": frappe.db.count( + "Bank Transaction", + filters=_bank_transaction_stats_filters(user_filters, bank_account, min_30), + ), + "last_12_months": frappe.db.count( + "Bank Transaction", + filters=_bank_transaction_stats_filters(user_filters, bank_account, min_365), + ), + "as_of": as_of, + } + + class BankReconciliationRule(Document): # begin: auto-generated types # This code is auto-generated. Do not modify anything in this block. diff --git a/banking/klarna_kosma_integration/doctype/bank_reconciliation_rule/test_bank_reconciliation_rule.py b/banking/klarna_kosma_integration/doctype/bank_reconciliation_rule/test_bank_reconciliation_rule.py index 8d8652a5..cd65767b 100644 --- a/banking/klarna_kosma_integration/doctype/bank_reconciliation_rule/test_bank_reconciliation_rule.py +++ b/banking/klarna_kosma_integration/doctype/bank_reconciliation_rule/test_bank_reconciliation_rule.py @@ -1,36 +1,69 @@ # Copyright (c) 2025, ALYF GmbH and Contributors # See license.txt +import json +from unittest.mock import patch + import frappe from frappe.tests.utils import FrappeTestCase from banking.exceptions import CurrencyMismatchError from banking.klarna_kosma_integration.doctype.bank_reconciliation_rule.bank_reconciliation_rule import ( + BankReconciliationRule, NoFiltersError, + get_bank_transaction_match_stats, ) from banking.testing_utils import TEST_COMPANY, create_bank_account, create_currency_account +def _group_account_with_parent(company: str) -> str: + row = frappe.db.sql( + """ + select name from `tabAccount` + where company = %s and is_group = 1 and ifnull(parent_account, '') != '' + limit 1 + """, + (company,), + ) + if not row: + raise RuntimeError(f"No suitable group Account found for company {company!r}") + return row[0][0] + + +def _resolved_test_company() -> str: + if frappe.db.exists("Company", TEST_COMPANY): + return TEST_COMPANY + row = frappe.db.sql("select name from `tabCompany` order by creation asc limit 1") + if not row: + raise RuntimeError("No Company found for banking tests") + return row[0][0] + + class TestBankReconciliationRule(FrappeTestCase): @classmethod def setUpClass(cls): super().setUpClass() - parent_account = frappe.db.get_value("Account", {"is_group": 1, "company": TEST_COMPANY}) + cls.test_company = _resolved_test_company() + parent_account = _group_account_with_parent(cls.test_company) bank_account = create_currency_account("EUR", parent_account, "_Test_Account_EUR") cls.target_account = create_currency_account("USD", parent_account, "_Test_Account_USD") cls.ba = create_bank_account(bank_account.name) def test_validate_account_currencies(self): - brr_doc = frappe.new_doc("Bank Reconciliation Rule") - brr_doc.bank_account = self.ba.name - brr_doc.target_account = self.target_account.name + brr_doc = BankReconciliationRule( + { + "doctype": "Bank Reconciliation Rule", + "bank_account": self.ba.name, + "target_account": self.target_account.name, + } + ) with self.assertRaises(CurrencyMismatchError): brr_doc.validate_account_currencies() def test_validate_filters(self): - brr_doc = frappe.new_doc("Bank Reconciliation Rule") + brr_doc = BankReconciliationRule({"doctype": "Bank Reconciliation Rule"}) brr_doc.filters = None with self.assertRaises(NoFiltersError): brr_doc.validate_filters() @@ -38,3 +71,64 @@ def test_validate_filters(self): brr_doc.filters = "[]" with self.assertRaises(NoFiltersError): brr_doc.validate_filters() + + @patch( + "banking.klarna_kosma_integration.doctype.bank_reconciliation_rule.bank_reconciliation_rule.today", + return_value="2025-06-15", + ) + def test_get_bank_transaction_match_stats_windows(self, _mock_today): + desc = "STAT-COUNT-BRR-359" + filters = json.dumps([["Bank Transaction", "description", "=", desc, False]]) + + def _insert_submitted_bt(posting_date: str): + bt = frappe.new_doc("Bank Transaction") + bt.company = self.test_company + bt.bank_account = self.ba.name + bt.deposit = 1.0 + bt.date = posting_date + bt.description = desc + bt.insert(ignore_permissions=True, ignore_mandatory=True, ignore_links=True) + frappe.db.set_value("Bank Transaction", bt.name, "docstatus", 1, update_modified=False) + + try: + _insert_submitted_bt("2025-06-10") + _insert_submitted_bt("2025-05-01") + _insert_submitted_bt("2024-01-01") + + stats = get_bank_transaction_match_stats(self.ba.name, filters) + self.assertEqual(stats["last_30_days"], 1) + self.assertEqual(stats["last_12_months"], 2) + self.assertEqual(str(stats["as_of"]), "2025-06-15") + finally: + frappe.db.delete("Bank Transaction", {"description": desc}) + + def test_get_bank_transaction_match_stats_bank_account_mismatch(self): + parent_account = _group_account_with_parent(self.test_company) + eur_target = create_currency_account("EUR", parent_account, "_Test_BRR_EUR_Tgt_Mis") + other_bank_acc = create_currency_account("EUR", parent_account, "_Test_Account_EUR_BrrMis") + other_ba = frappe.new_doc("Bank Account") + other_ba.account_name = "_Test_B_Other_Mismatch" + other_ba.account = other_bank_acc.name + other_ba.bank = "_Test_Bank" + other_ba.is_company_account = 1 + other_ba.insert(ignore_permissions=True, ignore_links=True) + rule = frappe.new_doc("Bank Reconciliation Rule") + rule.bank_account = self.ba.name + rule.target_account = eur_target.name + rule.filters = json.dumps([["Bank Transaction", "description", "=", "X"]]) + rule.insert(ignore_permissions=True, ignore_mandatory=True) + rule.flags.ignore_permissions = True + rule.submit() + try: + with self.assertRaises(frappe.ValidationError): + get_bank_transaction_match_stats( + other_ba.name, + rule.filters, + bank_reconciliation_rule=rule.name, + ) + finally: + rule.cancel() + frappe.delete_doc("Bank Reconciliation Rule", rule.name, force=1, ignore_permissions=True) + frappe.delete_doc("Bank Account", other_ba.name, force=1, ignore_permissions=True) + frappe.delete_doc("Account", other_bank_acc.name, force=1, ignore_permissions=True) + frappe.delete_doc("Account", eur_target.name, force=1, ignore_permissions=True) From bc9cf384b7dedfe91ed4e7639d305213dbe280db Mon Sep 17 00:00:00 2001 From: 0xD0M1M0 <76812428+0xD0M1M0@users.noreply.github.com> Date: Fri, 15 May 2026 18:13:30 +0000 Subject: [PATCH 02/13] fix: security improvements --- .../bank_reconciliation_rule.js | 11 +++++------ .../bank_reconciliation_rule.py | 4 +++- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/banking/klarna_kosma_integration/doctype/bank_reconciliation_rule/bank_reconciliation_rule.js b/banking/klarna_kosma_integration/doctype/bank_reconciliation_rule/bank_reconciliation_rule.js index f0941096..a9708830 100644 --- a/banking/klarna_kosma_integration/doctype/bank_reconciliation_rule/bank_reconciliation_rule.js +++ b/banking/klarna_kosma_integration/doctype/bank_reconciliation_rule/bank_reconciliation_rule.js @@ -37,10 +37,7 @@ function brr_open_bank_transaction_list(frm) { frappe.throw(__("Please define at least one filter.")); } const merged = brr_merge_list_filters(frm, user_filters); - localStorage.setItem( - "route_options", - JSON.stringify(brr_filters_to_route_options(merged)) - ); + frappe.route_options = brr_filters_to_route_options(merged); frappe.set_route("List", "Bank Transaction"); } @@ -106,7 +103,9 @@ async function brr_fetch_and_show_match_stats(frm) { const request_id = (frm._brr_stats_request_id = (frm._brr_stats_request_id || 0) + 1); - $el.html(`${__("Updating...")}
`); + if (!$el.text().trim()) { + $el.html(`${__("Updating...")}
`); + } const filters_str = JSON.stringify(user_filters); const method = @@ -145,7 +144,7 @@ async function brr_fetch_and_show_match_stats(frm) { ); $el.html( - `${frappe.utils.escape_html(
+ ` ${frappe.utils.escape_html(
title
)} ${frappe.utils.escape_html(counts)} ${__(
- "Set a Bank Account to see match counts."
- )}
+ `;
+ })
+ .join("");
+
+ const html = `
+
+
+ ${label}
+ ${display}
+
+
+
+
+
+
+ ${table_rows}
+ ${frappe.utils.escape_html(__("Use"))}
+ ${frappe.utils.escape_html(__("Field"))}
+ ${frappe.utils.escape_html(__("Value"))}
+
${__( - "Add at least one filter to see match counts." - )}
` + "Add at least one filter to see match counts.", + )}`, ); return; } @@ -128,8 +128,8 @@ async function brr_fetch_and_show_match_stats(frm) { if (request_id === frm._brr_stats_request_id) { $el.html( `${__( - "Could not load match counts." - )}
` + "Could not load match counts.", + )}`, ); } return; @@ -142,13 +142,13 @@ async function brr_fetch_and_show_match_stats(frm) { const title = __("Submitted Bank Transactions matching this rule:"); const counts = __( "{0} in the last 30 days, {1} in the last 12 months (365 days).", - [String(data.last_30_days), String(data.last_12_months)] + [String(data.last_30_days), String(data.last_12_months)], ); $el.html( `${frappe.utils.escape_html( - title - )} ${frappe.utils.escape_html(counts)}
` + title, + )} ${frappe.utils.escape_html(counts)}`, ); } @@ -190,7 +190,7 @@ frappe.ui.form.on("Bank Reconciliation Rule", { } = await frappe.db.get_value( "Bank Account", frm.doc.bank_account, - "account" + "account", ); if (!account) { @@ -199,7 +199,7 @@ frappe.ui.form.on("Bank Reconciliation Rule", { frappe.bold(__("Bank Account")), frm.doc.bank_account, __("Company Account"), - ]) + ]), ); } @@ -221,10 +221,15 @@ frappe.ui.form.on("Bank Reconciliation Rule", { const parent = frm.fields_dict.filter_area.$wrapper; parent.empty(); - const filters = - frm.doc.filters && frm.doc.filters !== "[]" - ? JSON.parse(frm.doc.filters) - : []; + let filters = []; + try { + if (frm.doc.filters && frm.doc.filters !== "[]") { + const parsed = JSON.parse(frm.doc.filters); + filters = Array.isArray(parsed) ? parsed : []; + } + } catch (e) { + filters = []; + } frappe.model.with_doctype("Bank Transaction", () => { if (!frm._brr_stats_debounced) { @@ -263,8 +268,8 @@ frappe.ui.form.on("Bank Reconciliation Rule", { frappe .run_serially( filters.map( - (f) => () => filter_group.add_filter(f[0], f[1], f[2], f[3]) - ) + (f) => () => filter_group.add_filter(f[0], f[1], f[2], f[3]), + ), ) .then(() => { filter_group.update_filters(); diff --git a/banking/klarna_kosma_integration/doctype/bank_reconciliation_rule/bank_reconciliation_rule.py b/banking/klarna_kosma_integration/doctype/bank_reconciliation_rule/bank_reconciliation_rule.py index e9051c92..89967267 100644 --- a/banking/klarna_kosma_integration/doctype/bank_reconciliation_rule/bank_reconciliation_rule.py +++ b/banking/klarna_kosma_integration/doctype/bank_reconciliation_rule/bank_reconciliation_rule.py @@ -9,6 +9,7 @@ from frappe.exceptions import ValidationError from frappe.model.document import Document from frappe.utils import add_days, getdate, today +from frappe.utils.data import get_filter from banking.exceptions import CurrencyMismatchError @@ -25,10 +26,8 @@ def _normalize_filter_rows(raw_filters: list) -> list[list[Any]]: doctype = row[0] if doctype != "Bank Transaction": frappe.throw(_("Invalid filter: only Bank Transaction fields are supported")) - fieldname = row[1] - operator = row[2] - value = row[3] - normalized.append([doctype, fieldname, operator, value]) + fd = get_filter(doctype, row) + normalized.append([fd.doctype, fd.fieldname, fd.operator, fd.value]) return normalized @@ -122,5 +121,11 @@ def validate_account_currencies(self): ) def validate_filters(self): - if not self.filters or not json.loads(self.filters): + if not self.filters: + frappe.throw(_("Please define at least one filter!"), NoFiltersError) + try: + parsed = json.loads(self.filters) + except json.JSONDecodeError: + frappe.throw(_("Invalid filters"), NoFiltersError) + if not parsed: frappe.throw(_("Please define at least one filter!"), NoFiltersError) From d37592c617ae97d82fd1632ceddda4f7200525f9 Mon Sep 17 00:00:00 2001 From: 0xD0M1M0 <76812428+0xD0M1M0@users.noreply.github.com> Date: Fri, 15 May 2026 18:48:04 +0000 Subject: [PATCH 05/13] fix: linters --- .../bank_reconciliation_rule.js | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/banking/klarna_kosma_integration/doctype/bank_reconciliation_rule/bank_reconciliation_rule.js b/banking/klarna_kosma_integration/doctype/bank_reconciliation_rule/bank_reconciliation_rule.js index 79df5433..81b0bb66 100644 --- a/banking/klarna_kosma_integration/doctype/bank_reconciliation_rule/bank_reconciliation_rule.js +++ b/banking/klarna_kosma_integration/doctype/bank_reconciliation_rule/bank_reconciliation_rule.js @@ -79,8 +79,8 @@ async function brr_fetch_and_show_match_stats(frm) { if (!frm.doc.bank_account) { $el.html( `${__( - "Set a Bank Account to see match counts.", - )}
`, + "Set a Bank Account to see match counts." + )}` ); return; } @@ -95,8 +95,8 @@ async function brr_fetch_and_show_match_stats(frm) { if (!user_filters.length) { $el.html( `${__( - "Add at least one filter to see match counts.", - )}
`, + "Add at least one filter to see match counts." + )}` ); return; } @@ -128,8 +128,8 @@ async function brr_fetch_and_show_match_stats(frm) { if (request_id === frm._brr_stats_request_id) { $el.html( `${__( - "Could not load match counts.", - )}
`, + "Could not load match counts." + )}` ); } return; @@ -142,13 +142,13 @@ async function brr_fetch_and_show_match_stats(frm) { const title = __("Submitted Bank Transactions matching this rule:"); const counts = __( "{0} in the last 30 days, {1} in the last 12 months (365 days).", - [String(data.last_30_days), String(data.last_12_months)], + [String(data.last_30_days), String(data.last_12_months)] ); $el.html( `${frappe.utils.escape_html( - title, - )} ${frappe.utils.escape_html(counts)}
`, + title + )} ${frappe.utils.escape_html(counts)}` ); } @@ -190,7 +190,7 @@ frappe.ui.form.on("Bank Reconciliation Rule", { } = await frappe.db.get_value( "Bank Account", frm.doc.bank_account, - "account", + "account" ); if (!account) { @@ -199,7 +199,7 @@ frappe.ui.form.on("Bank Reconciliation Rule", { frappe.bold(__("Bank Account")), frm.doc.bank_account, __("Company Account"), - ]), + ]) ); } @@ -268,8 +268,8 @@ frappe.ui.form.on("Bank Reconciliation Rule", { frappe .run_serially( filters.map( - (f) => () => filter_group.add_filter(f[0], f[1], f[2], f[3]), - ), + (f) => () => filter_group.add_filter(f[0], f[1], f[2], f[3]) + ) ) .then(() => { filter_group.update_filters(); From 44184b942845a166419e75817d82e4446baef0da Mon Sep 17 00:00:00 2001 From: 0xD0M1M0 <76812428+0xD0M1M0@users.noreply.github.com> Date: Sun, 17 May 2026 20:10:12 +0000 Subject: [PATCH 06/13] feat: create separate class --- banking/custom/bank_transaction.js | 240 +++++++------ .../bank_reconciliation_rule.js | 329 ++++++++++-------- .../bank_reconciliation_rule.py | 10 +- .../test_bank_reconciliation_rule.py | 21 +- 4 files changed, 339 insertions(+), 261 deletions(-) diff --git a/banking/custom/bank_transaction.js b/banking/custom/bank_transaction.js index 8a9ee482..b6f796ce 100644 --- a/banking/custom/bank_transaction.js +++ b/banking/custom/bank_transaction.js @@ -1,96 +1,108 @@ // Copyright (c) 2025, ALYF GmbH and contributors // For license information, please see license.txt -const BT_BRR_CANDIDATE_FIELDS = [ - "deposit", - "withdrawal", - "currency", - "description", - "reference_number", - "included_fee", - "excluded_fee", - "subtransaction_id", - "transaction_type", - "party_type", - "party", - "bank_party_name", - "bank_party_account_number", - "bank_party_iban", -]; +class RuleCreationDialogManager { + static CANDIDATE_FIELDS = [ + "deposit", + "withdrawal", + "currency", + "description", + "reference_number", + "included_fee", + "excluded_fee", + "subtransaction_id", + "transaction_type", + "party_type", + "party", + "bank_party_name", + "bank_party_account_number", + "bank_party_iban", + ]; -const BT_BRR_NUMERIC_FIELDTYPES = ["Currency", "Float", "Int"]; - -function bt_brr_field_has_value(df, value) { - if (value === undefined || value === null) { - return false; - } - const ft = df.fieldtype; - if (BT_BRR_NUMERIC_FIELDTYPES.includes(ft)) { - return flt(value) !== 0; + constructor(frm) { + this.frm = frm; + this.menu_link = null; } - if (ft === "Link" || ft === "Dynamic Link") { - return Boolean(String(value).trim()); - } - return String(value).trim() !== ""; -} -function bt_brr_get_visible_field_rows(frm) { - const rows = []; - for (const fieldname of BT_BRR_CANDIDATE_FIELDS) { - const df = frappe.meta.get_docfield("Bank Transaction", fieldname); - if (!df) { - continue; + field_has_value(df, value) { + if (value === undefined || value === null) { + return false; } - const value = frm.doc[fieldname]; - if (!bt_brr_field_has_value(df, value)) { - continue; + const ft = df.fieldtype; + if (frappe.model.numeric_fieldtypes.includes(ft)) { + return flt(value) !== 0; } - rows.push({ fieldname, df, value }); + if (ft === "Link" || ft === "Dynamic Link") { + return Boolean(String(value).trim()); + } + return String(value).trim() !== ""; } - return rows; -} - -function bt_brr_format_cell(frm, df, value) { - const formatted = frappe.format(value, df, { only_value: true }, frm.doc); - return frappe.utils.escape_html(String(formatted ?? "")); -} -function bt_brr_open_create_rule_dialog(frm) { - if (!frm.doc.bank_account) { - frappe.throw(__("Set a Bank Account first.")); - } - if (!frappe.model.can_create("Bank Reconciliation Rule")) { - frappe.throw( - __("You do not have permission to create a Bank Reconciliation Rule.") - ); + get_visible_field_rows() { + const rows = []; + for (const fieldname of RuleCreationDialogManager.CANDIDATE_FIELDS) { + const df = frappe.meta.get_docfield("Bank Transaction", fieldname); + if (!df) { + continue; + } + const value = this.frm.doc[fieldname]; + if (!this.field_has_value(df, value)) { + continue; + } + rows.push({ fieldname, df, value }); + } + return rows; } - const field_rows = bt_brr_get_visible_field_rows(frm); - if (!field_rows.length) { - frappe.msgprint( - __("No filterable field values are set on this Bank Transaction.") + format_cell(df, value) { + const formatted = frappe.format( + value, + df, + { only_value: true }, + this.frm.doc ); - return; + return frappe.utils.escape_html(String(formatted ?? "")); } - const body_id = `bt-brr-picker-${frappe.utils.get_random(10)}`; - const table_rows = field_rows - .map((row) => { - const label = frappe.utils.escape_html(__(row.df.label) || row.fieldname); - const display = bt_brr_format_cell(frm, row.df, row.value); - return `${__( - "Set a Bank Account to see match counts." - )}
` - ); - return; + get_debounced_fetch() { + if (!this._debounced_fetch) { + this._debounced_fetch = frappe.utils.debounce( + () => this.fetch_and_show(), + 400 + ); + } + return this._debounced_fetch; } - let user_filters = []; - try { - user_filters = brr_get_user_filters_for_stats(frm); - } catch (e) { - user_filters = []; - } + async fetch_and_show() { + const $el = this.$stats_el; + if (!$el?.length || !this.host_is_live()) { + return; + } - if (!user_filters.length) { - $el.html( - `${__( - "Add at least one filter to see match counts." - )}
` - ); - return; - } + if (!this.frm.doc.bank_account) { + $el.html( + `${__( + "Set a Bank Account to see match counts." + )}
` + ); + return; + } - const request_id = (frm._brr_stats_request_id = - (frm._brr_stats_request_id || 0) + 1); - if (!$el.text().trim()) { - $el.html(`${__("Updating...")}
`); - } + let user_filters = []; + try { + user_filters = this.get_user_filters(); + } catch (e) { + user_filters = []; + } - const filters_str = JSON.stringify(user_filters); - const method = - "banking.klarna_kosma_integration.doctype.bank_reconciliation_rule.bank_reconciliation_rule.get_bank_transaction_match_stats"; - - let data; - try { - const res = await frappe.call({ - method, - args: { - bank_account: frm.doc.bank_account, - filters: filters_str, - ...(!frm.is_new() && frm.doc.name - ? { bank_reconciliation_rule: frm.doc.name } - : {}), - }, - }); - data = res.message; - } catch (e) { - if (request_id === frm._brr_stats_request_id) { + if (!user_filters.length) { $el.html( - `${__( - "Could not load match counts." + `
${__( + "Add at least one filter to see match counts." )}
` ); + return; + } + + const request_id = ++this.request_id; + if (!$el.text().trim()) { + $el.html(`${__("Updating...")}
`); + } + + const filters_str = JSON.stringify(user_filters); + const method = + "banking.klarna_kosma_integration.doctype.bank_reconciliation_rule.bank_reconciliation_rule.get_bank_transaction_match_stats"; + + let data; + try { + const res = await frappe.call({ + method, + args: { + bank_account: this.frm.doc.bank_account, + filters: filters_str, + ...(!this.frm.is_new() && this.frm.doc.name + ? { bank_reconciliation_rule: this.frm.doc.name } + : {}), + }, + }); + data = res.message; + } catch (e) { + if (request_id === this.request_id) { + $el.html( + `${__( + "Could not load match counts." + )}
` + ); + } + return; } - return; - } - if (request_id !== frm._brr_stats_request_id) { - return; + if (request_id !== this.request_id) { + return; + } + + const title = __("Submitted Bank Transactions matching this rule:"); + const counts = __( + "{0} in the last 30 days, {1} in the last 12 months (365 days).", + [String(data.last_30_days), String(data.last_12_months)] + ); + + $el.html( + `${frappe.utils.escape_html( + title + )} ${frappe.utils.escape_html(counts)}
` + ); } - const title = __("Submitted Bank Transactions matching this rule:"); - const counts = __( - "{0} in the last 30 days, {1} in the last 12 months (365 days).", - [String(data.last_30_days), String(data.last_12_months)] - ); - - $el.html( - `${frappe.utils.escape_html( - title - )} ${frappe.utils.escape_html(counts)}
` - ); + mount(parent) { + this.$stats_el = $(''); + parent.append(this.$stats_el); + this.fetch_and_show(); + } } frappe.ui.form.on("Bank Reconciliation Rule", { refresh(frm) { + if (!frm.stats_manager) { + frm.stats_manager = new BankReconciliationRuleStatsManager(frm); + } + const stats = frm.stats_manager; + frm.trigger("set_target_account_query"); frm.remove_custom_button(__("Open Matches")); const filters_json = @@ -166,19 +226,19 @@ frappe.ui.form.on("Bank Reconciliation Rule", { } if (frm.doc.bank_account && has_filters) { frm.add_custom_button(__("Open Matches"), () => { - brr_open_bank_transaction_list(frm); + stats.open_bank_transaction_list(); }); } - if (!brr_stats_host_is_live(frm)) { + if (stats.needs_filter_rerender()) { frm.trigger("render_bt_filters"); } else { - brr_fetch_and_show_match_stats(frm); + stats.fetch_and_show(); } }, bank_account(frm) { frm.trigger("set_target_account_query"); - frm._brr_stats_debounced?.(); + frm.stats_manager?.get_debounced_fetch()(); }, async set_target_account_query(frm) { if (!frm.doc.bank_account) { @@ -218,6 +278,9 @@ frappe.ui.form.on("Bank Reconciliation Rule", { })); }, render_bt_filters(frm) { + const stats = frm.stats_manager; + stats.teardown(); + const parent = frm.fields_dict.filter_area.$wrapper; parent.empty(); @@ -232,22 +295,16 @@ frappe.ui.form.on("Bank Reconciliation Rule", { } frappe.model.with_doctype("Bank Transaction", () => { - if (!frm._brr_stats_debounced) { - frm._brr_stats_debounced = frappe.utils.debounce(() => { - brr_fetch_and_show_match_stats(frm); - }, 400); - } - const filter_group = new frappe.ui.FilterGroup({ parent: parent, doctype: "Bank Transaction", on_change: () => { frm.set_value("filters", JSON.stringify(filter_group.get_filters())); - frm._brr_stats_debounced?.(); + stats.get_debounced_fetch()(); }, }); - frm._bank_transaction_filter_group = filter_group; + stats.filter_group = filter_group; const after_filters_ready = () => { if (frm.doc.docstatus === 1) { @@ -257,10 +314,8 @@ frappe.ui.form.on("Bank Reconciliation Rule", { parent.find(".form-control").prop("disabled", true); } - const $stats = $(''); - parent.append($stats); - frm._brr_match_stats_$el = $stats; - brr_fetch_and_show_match_stats(frm); + stats.rendered_for = frm.docname; + stats.mount(parent); }; if (filters.length) { diff --git a/banking/klarna_kosma_integration/doctype/bank_reconciliation_rule/bank_reconciliation_rule.py b/banking/klarna_kosma_integration/doctype/bank_reconciliation_rule/bank_reconciliation_rule.py index 89967267..46e8c886 100644 --- a/banking/klarna_kosma_integration/doctype/bank_reconciliation_rule/bank_reconciliation_rule.py +++ b/banking/klarna_kosma_integration/doctype/bank_reconciliation_rule/bank_reconciliation_rule.py @@ -37,9 +37,13 @@ def _bank_transaction_stats_filters( min_date: str, ) -> list[list[Any]]: filters = _normalize_filter_rows(user_filters) - filters.append(["Bank Transaction", "bank_account", "=", bank_account]) - filters.append(["Bank Transaction", "docstatus", "=", 1]) - filters.append(["Bank Transaction", "date", ">=", min_date]) + filters.extend( + [ + ["Bank Transaction", "bank_account", "=", bank_account], + ["Bank Transaction", "docstatus", "=", 1], + ["Bank Transaction", "date", ">=", min_date], + ] + ) return filters diff --git a/banking/klarna_kosma_integration/doctype/bank_reconciliation_rule/test_bank_reconciliation_rule.py b/banking/klarna_kosma_integration/doctype/bank_reconciliation_rule/test_bank_reconciliation_rule.py index cd65767b..4b85657e 100644 --- a/banking/klarna_kosma_integration/doctype/bank_reconciliation_rule/test_bank_reconciliation_rule.py +++ b/banking/klarna_kosma_integration/doctype/bank_reconciliation_rule/test_bank_reconciliation_rule.py @@ -17,26 +17,23 @@ def _group_account_with_parent(company: str) -> str: - row = frappe.db.sql( - """ - select name from `tabAccount` - where company = %s and is_group = 1 and ifnull(parent_account, '') != '' - limit 1 - """, - (company,), + name = frappe.db.get_value( + "Account", + {"company": company, "is_group": 1, "parent_account": ["!=", ""]}, + "name", ) - if not row: + if not name: raise RuntimeError(f"No suitable group Account found for company {company!r}") - return row[0][0] + return name def _resolved_test_company() -> str: if frappe.db.exists("Company", TEST_COMPANY): return TEST_COMPANY - row = frappe.db.sql("select name from `tabCompany` order by creation asc limit 1") - if not row: + name = frappe.db.get_value("Company", {}, "name", order_by="creation asc") + if not name: raise RuntimeError("No Company found for banking tests") - return row[0][0] + return name class TestBankReconciliationRule(FrappeTestCase): From d3853067e71c9de9d27a15921a2ad166acf89191 Mon Sep 17 00:00:00 2001 From: 0xD0M1M0 <76812428+0xD0M1M0@users.noreply.github.com> Date: Sun, 17 May 2026 20:32:39 +0000 Subject: [PATCH 07/13] fix: allow multiple filters --- .../bank_reconciliation_rule.js | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/banking/klarna_kosma_integration/doctype/bank_reconciliation_rule/bank_reconciliation_rule.js b/banking/klarna_kosma_integration/doctype/bank_reconciliation_rule/bank_reconciliation_rule.js index 740a5bfe..b3d3d57f 100644 --- a/banking/klarna_kosma_integration/doctype/bank_reconciliation_rule/bank_reconciliation_rule.js +++ b/banking/klarna_kosma_integration/doctype/bank_reconciliation_rule/bank_reconciliation_rule.js @@ -37,7 +37,7 @@ class BankReconciliationRuleStatsManager { } static filters_to_route_options(filters) { - const opts = {}; + const by_field = {}; for (const row of filters) { if (!Array.isArray(row) || row.length < 4) { continue; @@ -45,13 +45,17 @@ class BankReconciliationRuleStatsManager { const field = row[1]; const op = row[2]; const value = row[3]; - if (Object.prototype.hasOwnProperty.call(opts, field)) { - continue; - } - if (op === "=") { - opts[field] = value; + (by_field[field] ||= []).push([op, value]); + } + const opts = {}; + for (const [field, conditions] of Object.entries(by_field)) { + if (conditions.length === 1) { + const [op, value] = conditions[0]; + opts[field] = op === "=" ? value : [op, value]; } else { - opts[field] = [op, value]; + opts[field] = conditions.map(([op, value]) => + JSON.stringify([op, value]) + ); } } return opts; From 8db12ccc6bea094bb9f2790c7821e6edcb8d83aa Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Sat, 18 Jul 2026 00:19:27 +0200 Subject: [PATCH 08/13] refactor: revert pointless change to hooks --- banking/hooks.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/banking/hooks.py b/banking/hooks.py index 1b6d6bc9..6b0e105d 100644 --- a/banking/hooks.py +++ b/banking/hooks.py @@ -32,13 +32,13 @@ # include js in doctype views doctype_js = { "Bank": "custom/bank.js", - "Bank Transaction": "custom/bank_transaction.js", "Expense Claim": "custom/expense_claim.js", "Purchase Invoice": "custom/purchase_invoice.js", "Employee": "custom/employee.js", "Supplier": "custom/supplier.js", "Bank Reconciliation Tool": "custom/bank_reconciliation_tool.js", "Bank Account": "custom/bank_account.js", + "Bank Transaction": "custom/bank_transaction.js", } doctype_list_js = { "Expense Claim": "custom/expense_claim_list.js", From e4311d784c8b72302708c83c0f11f0078631f3cd Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Sat, 18 Jul 2026 00:25:36 +0200 Subject: [PATCH 09/13] refactor: move controllers into separate bundles --- banking/custom/bank_transaction.js | 179 +----------- .../bank_reconciliation_rule.js | 257 ++---------------- .../bank_reconciliation_rule_stats.bundle.js | 213 +++++++++++++++ .../public/js/rule_creation_dialog.bundle.js | 172 ++++++++++++ 4 files changed, 418 insertions(+), 403 deletions(-) create mode 100644 banking/public/js/bank_reconciliation_rule_stats.bundle.js create mode 100644 banking/public/js/rule_creation_dialog.bundle.js diff --git a/banking/custom/bank_transaction.js b/banking/custom/bank_transaction.js index 6dac901e..a4b4598f 100644 --- a/banking/custom/bank_transaction.js +++ b/banking/custom/bank_transaction.js @@ -1,174 +1,6 @@ // Copyright (c) 2026, ALYF GmbH and contributors // For license information, please see license.txt -class RuleCreationDialogManager { - static CANDIDATE_FIELDS = [ - "deposit", - "withdrawal", - "currency", - "description", - "reference_number", - "included_fee", - "excluded_fee", - "subtransaction_id", - "transaction_type", - "party_type", - "party", - "bank_party_name", - "bank_party_account_number", - "bank_party_iban", - ]; - - constructor(frm) { - this.frm = frm; - this.menu_link = null; - } - - field_has_value(df, value) { - if (value === undefined || value === null) { - return false; - } - const ft = df.fieldtype; - if (frappe.model.numeric_fieldtypes.includes(ft)) { - return flt(value) !== 0; - } - if (ft === "Link" || ft === "Dynamic Link") { - return Boolean(String(value).trim()); - } - return String(value).trim() !== ""; - } - - get_visible_field_rows() { - const rows = []; - for (const fieldname of RuleCreationDialogManager.CANDIDATE_FIELDS) { - const df = frappe.meta.get_docfield("Bank Transaction", fieldname); - if (!df) { - continue; - } - const value = this.frm.doc[fieldname]; - if (!this.field_has_value(df, value)) { - continue; - } - rows.push({ fieldname, df, value }); - } - return rows; - } - - format_cell(df, value) { - const formatted = frappe.format( - value, - df, - { only_value: true }, - this.frm.doc - ); - return frappe.utils.escape_html(String(formatted ?? "")); - } - - show() { - const frm = this.frm; - if (!frm.doc.bank_account) { - frappe.throw(__("Set a Bank Account first.")); - } - if (!frappe.model.can_create("Bank Reconciliation Rule")) { - frappe.throw( - __("You do not have permission to create a Bank Reconciliation Rule.") - ); - } - - const field_rows = this.get_visible_field_rows(); - if (!field_rows.length) { - frappe.msgprint( - __("No filterable field values are set on this Bank Transaction.") - ); - return; - } - - const body_id = `rule-creation-picker-${frappe.utils.get_random(10)}`; - const table_rows = field_rows - .map((row) => { - const label = frappe.utils.escape_html( - __(row.df.label) || row.fieldname - ); - const display = this.format_cell(row.df, row.value); - return `| ${frappe.utils.escape_html(__("Use"))} | -${frappe.utils.escape_html(__("Field"))} | -${frappe.utils.escape_html(__("Value"))} | -
|---|
${__( - "Set a Bank Account to see match counts." - )}
` - ); - return; - } - - let user_filters = []; - try { - user_filters = this.get_user_filters(); - } catch (e) { - user_filters = []; - } - - if (!user_filters.length) { - $el.html( - `${__( - "Add at least one filter to see match counts." - )}
` - ); - return; - } - - const request_id = ++this.request_id; - if (!$el.text().trim()) { - $el.html(`${__("Updating...")}
`); - } - - const filters_str = JSON.stringify(user_filters); - const method = - "banking.klarna_kosma_integration.doctype.bank_reconciliation_rule.bank_reconciliation_rule.get_bank_transaction_match_stats"; - - let data; - try { - const res = await frappe.call({ - method, - args: { - bank_account: this.frm.doc.bank_account, - filters: filters_str, - ...(!this.frm.is_new() && this.frm.doc.name - ? { bank_reconciliation_rule: this.frm.doc.name } - : {}), - }, - }); - data = res.message; - } catch (e) { - if (request_id === this.request_id) { - $el.html( - `${__( - "Could not load match counts." - )}
` - ); + if (frm.doc.bank_account && has_filters) { + frm.add_custom_button(__("Open Matches"), () => { + stats.open_bank_transaction_list(); + }); } - return; - } - - if (request_id !== this.request_id) { - return; - } - - const title = __("Submitted Bank Transactions matching this rule:"); - const counts = __( - "{0} in the last 30 days, {1} in the last 12 months (365 days).", - [String(data.last_30_days), String(data.last_12_months)] - ); - $el.html( - `${frappe.utils.escape_html( - title - )} ${frappe.utils.escape_html(counts)}
` - ); - } - - mount(parent) { - this.$stats_el = $(''); - parent.append(this.$stats_el); - this.fetch_and_show(); - } -} - -frappe.ui.form.on("Bank Reconciliation Rule", { - refresh(frm) { - if (!frm.stats_manager) { - frm.stats_manager = new BankReconciliationRuleStatsManager(frm); - } - const stats = frm.stats_manager; - - frm.trigger("set_target_account_query"); - frm.remove_custom_button(__("Open Matches")); - const filters_json = - frm.doc.filters && frm.doc.filters !== "[]" ? frm.doc.filters : "[]"; - let has_filters = false; - try { - has_filters = JSON.parse(filters_json).length > 0; - } catch (e) { - has_filters = false; - } - if (frm.doc.bank_account && has_filters) { - frm.add_custom_button(__("Open Matches"), () => { - stats.open_bank_transaction_list(); - }); - } - - if (stats.needs_filter_rerender()) { - frm.trigger("render_bt_filters"); - } else { - stats.fetch_and_show(); - } + if (stats.needs_filter_rerender()) { + frm.trigger("render_bt_filters"); + } else { + stats.fetch_and_show(); + } + }); }, bank_account(frm) { frm.trigger("set_target_account_query"); diff --git a/banking/public/js/bank_reconciliation_rule_stats.bundle.js b/banking/public/js/bank_reconciliation_rule_stats.bundle.js new file mode 100644 index 00000000..01c0af3b --- /dev/null +++ b/banking/public/js/bank_reconciliation_rule_stats.bundle.js @@ -0,0 +1,213 @@ +// Copyright (c) 2025, ALYF GmbH and contributors +// For license information, please see license.txt + +frappe.provide("banking.bank_reconciliation"); + +banking.bank_reconciliation.BankReconciliationRuleStatsManager = class BankReconciliationRuleStatsManager { + constructor(frm) { + this.frm = frm; + this.request_id = 0; + this.$stats_el = null; + this.filter_group = null; + this._debounced_fetch = null; + this.rendered_for = null; + } + + teardown() { + this.filter_group = null; + this.$stats_el = null; + this.rendered_for = null; + } + + needs_filter_rerender() { + if (!this.host_is_live() || !this.filter_group) { + return true; + } + if (this.rendered_for !== this.frm.docname) { + return true; + } + // Submitted rules disable filter controls; duplicate as draft must rebuild editable UI. + if ( + this.frm.doc.docstatus === 0 && + this.frm.fields_dict?.filter_area?.$wrapper?.find( + ".form-control:disabled" + ).length + ) { + return true; + } + return false; + } + + static filters_to_route_options(filters) { + const by_field = {}; + for (const row of filters) { + if (!Array.isArray(row) || row.length < 4) { + continue; + } + const field = row[1]; + const op = row[2]; + const value = row[3]; + (by_field[field] ||= []).push([op, value]); + } + const opts = {}; + for (const [field, conditions] of Object.entries(by_field)) { + if (conditions.length === 1) { + const [op, value] = conditions[0]; + opts[field] = op === "=" ? value : [op, value]; + } else { + opts[field] = conditions.map(([op, value]) => + JSON.stringify([op, value]) + ); + } + } + return opts; + } + + merge_list_filters(user_filters) { + const merged = user_filters.map((row) => row.slice(0, 4)); + merged.push([ + "Bank Transaction", + "bank_account", + "=", + this.frm.doc.bank_account, + ]); + return merged; + } + + open_bank_transaction_list() { + if (!this.frm.doc.bank_account) { + frappe.throw(__("Set a Bank Account first.")); + } + const user_filters = this.get_user_filters(); + if (!user_filters.length) { + frappe.throw(__("Please define at least one filter.")); + } + const merged = this.merge_list_filters(user_filters); + frappe.route_options = this.constructor.filters_to_route_options(merged); + frappe.set_route("List", "Bank Transaction"); + } + + host_is_live() { + const parent = this.frm.fields_dict?.filter_area?.$wrapper; + if (!parent?.length || !this.$stats_el?.length) { + return false; + } + return $.contains(parent[0], this.$stats_el[0]); + } + + get_user_filters() { + if (this.filter_group) { + try { + const from_ui = this.filter_group.get_filters() || []; + if (from_ui.length) { + return from_ui; + } + } catch (e) { + // fall through to saved filters + } + } + try { + const parsed = JSON.parse(this.frm.doc.filters || "[]"); + return Array.isArray(parsed) ? parsed : []; + } catch (e) { + return []; + } + } + + get_debounced_fetch() { + if (!this._debounced_fetch) { + this._debounced_fetch = frappe.utils.debounce( + () => this.fetch_and_show(), + 400 + ); + } + return this._debounced_fetch; + } + + async fetch_and_show() { + const $el = this.$stats_el; + if (!$el?.length || !this.host_is_live()) { + return; + } + + if (!this.frm.doc.bank_account) { + $el.html( + `${__( + "Set a Bank Account to see match counts." + )}
` + ); + return; + } + + let user_filters = []; + try { + user_filters = this.get_user_filters(); + } catch (e) { + user_filters = []; + } + + if (!user_filters.length) { + $el.html( + `${__( + "Add at least one filter to see match counts." + )}
` + ); + return; + } + + const request_id = ++this.request_id; + if (!$el.text().trim()) { + $el.html(`${__("Updating...")}
`); + } + + const filters_str = JSON.stringify(user_filters); + const method = + "banking.klarna_kosma_integration.doctype.bank_reconciliation_rule.bank_reconciliation_rule.get_bank_transaction_match_stats"; + + let data; + try { + const res = await frappe.call({ + method, + args: { + bank_account: this.frm.doc.bank_account, + filters: filters_str, + ...(!this.frm.is_new() && this.frm.doc.name + ? { bank_reconciliation_rule: this.frm.doc.name } + : {}), + }, + }); + data = res.message; + } catch (e) { + if (request_id === this.request_id) { + $el.html( + `${__( + "Could not load match counts." + )}
` + ); + } + return; + } + + if (request_id !== this.request_id) { + return; + } + + const title = __("Submitted Bank Transactions matching this rule:"); + const counts = __( + "{0} in the last 30 days, {1} in the last 12 months (365 days).", + [String(data.last_30_days), String(data.last_12_months)] + ); + + $el.html( + `${frappe.utils.escape_html( + title + )} ${frappe.utils.escape_html(counts)}
` + ); + } + + mount(parent) { + this.$stats_el = $(''); + parent.append(this.$stats_el); + this.fetch_and_show(); + } +}; diff --git a/banking/public/js/rule_creation_dialog.bundle.js b/banking/public/js/rule_creation_dialog.bundle.js new file mode 100644 index 00000000..2d2f6065 --- /dev/null +++ b/banking/public/js/rule_creation_dialog.bundle.js @@ -0,0 +1,172 @@ +// Copyright (c) 2026, ALYF GmbH and contributors +// For license information, please see license.txt + +frappe.provide("banking.bank_reconciliation"); + +banking.bank_reconciliation.RuleCreationDialogManager = class RuleCreationDialogManager { + static CANDIDATE_FIELDS = [ + "deposit", + "withdrawal", + "currency", + "description", + "reference_number", + "included_fee", + "excluded_fee", + "subtransaction_id", + "transaction_type", + "party_type", + "party", + "bank_party_name", + "bank_party_account_number", + "bank_party_iban", + ]; + + constructor(frm) { + this.frm = frm; + this.menu_link = null; + } + + field_has_value(df, value) { + if (value === undefined || value === null) { + return false; + } + const ft = df.fieldtype; + if (frappe.model.numeric_fieldtypes.includes(ft)) { + return flt(value) !== 0; + } + if (ft === "Link" || ft === "Dynamic Link") { + return Boolean(String(value).trim()); + } + return String(value).trim() !== ""; + } + + get_visible_field_rows() { + const rows = []; + for (const fieldname of this.constructor.CANDIDATE_FIELDS) { + const df = frappe.meta.get_docfield("Bank Transaction", fieldname); + if (!df) { + continue; + } + const value = this.frm.doc[fieldname]; + if (!this.field_has_value(df, value)) { + continue; + } + rows.push({ fieldname, df, value }); + } + return rows; + } + + format_cell(df, value) { + const formatted = frappe.format( + value, + df, + { only_value: true }, + this.frm.doc + ); + return frappe.utils.escape_html(String(formatted ?? "")); + } + + show() { + const frm = this.frm; + if (!frm.doc.bank_account) { + frappe.throw(__("Set a Bank Account first.")); + } + if (!frappe.model.can_create("Bank Reconciliation Rule")) { + frappe.throw( + __("You do not have permission to create a Bank Reconciliation Rule.") + ); + } + + const field_rows = this.get_visible_field_rows(); + if (!field_rows.length) { + frappe.msgprint( + __("No filterable field values are set on this Bank Transaction.") + ); + return; + } + + const body_id = `rule-creation-picker-${frappe.utils.get_random(10)}`; + const table_rows = field_rows + .map((row) => { + const label = frappe.utils.escape_html( + __(row.df.label) || row.fieldname + ); + const display = this.format_cell(row.df, row.value); + return `| ${frappe.utils.escape_html(__("Use"))} | +${frappe.utils.escape_html(__("Field"))} | +${frappe.utils.escape_html(__("Value"))} | +
|---|
|
+ const html = ` ${__( @@ -206,6 +189,7 @@ banking.bank_reconciliation.BankReconciliationRuleStatsManager = class BankRecon } mount(parent) { + this.rendered_readonly = this.frm.doc.docstatus !== 0; this.$stats_el = $(' '); parent.append(this.$stats_el); this.fetch_and_show(); From f2f5119d173e3361a665eb548fa59a9c129e6479 Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Sat, 18 Jul 2026 01:18:09 +0200 Subject: [PATCH 13/13] perf(bank reconciliation rule): use capped match counts like List View --- .../bank_reconciliation_rule.py | 30 ++++++++++++++----- .../bank_reconciliation_rule_stats.bundle.js | 10 ++++++- 2 files changed, 32 insertions(+), 8 deletions(-) diff --git a/banking/klarna_kosma_integration/doctype/bank_reconciliation_rule/bank_reconciliation_rule.py b/banking/klarna_kosma_integration/doctype/bank_reconciliation_rule/bank_reconciliation_rule.py index 46e8c886..37d5100f 100644 --- a/banking/klarna_kosma_integration/doctype/bank_reconciliation_rule/bank_reconciliation_rule.py +++ b/banking/klarna_kosma_integration/doctype/bank_reconciliation_rule/bank_reconciliation_rule.py @@ -7,12 +7,16 @@ import frappe from frappe import _ from frappe.exceptions import ValidationError +from frappe.model.db_query import DatabaseQuery from frappe.model.document import Document from frappe.utils import add_days, getdate, today from frappe.utils.data import get_filter from banking.exceptions import CurrencyMismatchError +# Same cap as List View (`count_upper_bound`): count at most N rows, show "N-1+" when hit. +COUNT_UPPER_BOUND = 1001 + class NoFiltersError(ValidationError): pass @@ -47,14 +51,27 @@ def _bank_transaction_stats_filters( return filters +def _approx_bank_transaction_count(filters: list) -> int: + """Fast capped count, same approach as List View (LIMIT then count rows).""" + return len( + DatabaseQuery("Bank Transaction").execute( + filters=filters, + limit=COUNT_UPPER_BOUND, + order_by=None, + pluck="name", + ) + ) + + @frappe.whitelist() def get_bank_transaction_match_stats( bank_account: str, filters: str | None = None, bank_reconciliation_rule: str | None = None, ) -> dict[str, Any]: - """Count submitted Bank Transactions matching the rule filters (scoped by bank account). + """Approximate count of submitted Bank Transactions matching the rule filters. + Uses a capped count (like List View) so large tables stay fast. Windows use calendar days: last 30 days and last 365 days (\"12 months\"), relative to *today*. """ if not bank_account: @@ -81,15 +98,14 @@ def get_bank_transaction_match_stats( min_365 = add_days(as_of, -365) return { - "last_30_days": frappe.db.count( - "Bank Transaction", - filters=_bank_transaction_stats_filters(user_filters, bank_account, min_30), + "last_30_days": _approx_bank_transaction_count( + _bank_transaction_stats_filters(user_filters, bank_account, min_30) ), - "last_12_months": frappe.db.count( - "Bank Transaction", - filters=_bank_transaction_stats_filters(user_filters, bank_account, min_365), + "last_12_months": _approx_bank_transaction_count( + _bank_transaction_stats_filters(user_filters, bank_account, min_365) ), "as_of": as_of, + "count_upper_bound": COUNT_UPPER_BOUND, } diff --git a/banking/public/js/bank_reconciliation_rule_stats.bundle.js b/banking/public/js/bank_reconciliation_rule_stats.bundle.js index 297b4708..28601767 100644 --- a/banking/public/js/bank_reconciliation_rule_stats.bundle.js +++ b/banking/public/js/bank_reconciliation_rule_stats.bundle.js @@ -175,10 +175,18 @@ banking.bank_reconciliation.BankReconciliationRuleStatsManager = class BankRecon return; } + const format_count = (n) => { + const bound = data.count_upper_bound; + if (bound && n === bound) { + return `${bound - 1}+`; + } + return String(n); + }; + const title = __("Submitted Bank Transactions matching this rule:"); const counts = __( "{0} in the last 30 days, {1} in the last 12 months (365 days).", - [String(data.last_30_days), String(data.last_12_months)] + [format_count(data.last_30_days), format_count(data.last_12_months)] ); $el.html( |