From d24850f103ac484247dd0cc0cfc1365c7eda52b8 Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Tue, 17 Feb 2026 22:29:58 +0100 Subject: [PATCH 01/14] feat(Bank Transaction): auto-book included bank fees Add bank fee account configuration and create fee Journal Entries on bank transaction submission so included fees are recognized immediately and reconciled correctly. Co-authored-by: Cursor --- banking/custom/bank_account.js | 33 ++++ banking/custom_fields.py | 10 ++ banking/hooks.py | 4 + banking/overrides/bank_account.py | 16 ++ banking/overrides/bank_transaction.py | 150 ++++++++++++++++++ banking/overrides/test_bank_account.py | 42 +++++ .../test_bank_transaction_included_fees.py | 137 ++++++++++++++++ banking/patches.txt | 2 +- 8 files changed, 393 insertions(+), 1 deletion(-) create mode 100644 banking/custom/bank_account.js create mode 100644 banking/overrides/test_bank_account.py create mode 100644 banking/overrides/test_bank_transaction_included_fees.py diff --git a/banking/custom/bank_account.js b/banking/custom/bank_account.js new file mode 100644 index 00000000..c6792658 --- /dev/null +++ b/banking/custom/bank_account.js @@ -0,0 +1,33 @@ +// Copyright (c) 2025, ALYF GmbH and contributors +// For license information, please see license.txt + +frappe.ui.form.on("Bank Account", { + refresh(frm) { + frm.trigger("set_fee_account_query"); + }, + + account(frm) { + frm.trigger("set_fee_account_query"); + }, + + async set_fee_account_query(frm) { + if (!frm.doc.account) { + return; + } + + const { + message: { account_currency: currency, company: company }, + } = await frappe.db.get_value("Account", frm.doc.account, [ + "account_currency", + "company", + ]); + + frm.set_query("bank_fee_account", () => ({ + filters: { + root_type: "Expense", + account_currency: currency, + company: company, + }, + })); + }, +}); diff --git a/banking/custom_fields.py b/banking/custom_fields.py index 841b5daa..e3c8015a 100644 --- a/banking/custom_fields.py +++ b/banking/custom_fields.py @@ -87,4 +87,14 @@ def get_custom_fields(): insert_after="transaction_id", ), ], + "Bank Account": [ + dict( + fieldname="bank_fee_account", + fieldtype="Link", + label=_("Bank Fee Account"), + options="Account", + depends_on="eval:doc.is_company_account && doc.account", + insert_after="account_subtype", + ), + ], } diff --git a/banking/hooks.py b/banking/hooks.py index 72d299c1..02b46532 100644 --- a/banking/hooks.py +++ b/banking/hooks.py @@ -34,6 +34,7 @@ "Employee": "custom/employee.js", "Supplier": "custom/supplier.js", "Bank Reconciliation Tool": "custom/bank_reconciliation_tool.js", + "Bank Account": "custom/bank_account.js", } doctype_list_js = {"Purchase Invoice": "custom/purchase_invoice_list.js"} # doctype_tree_js = {"doctype" : "public/js/doctype_tree.js"} @@ -108,9 +109,12 @@ doc_events = { "Bank Transaction": { "on_update_after_submit": "banking.overrides.bank_transaction.on_update_after_submit", + "before_submit": "banking.overrides.bank_transaction.before_submit", + "on_cancel": "banking.overrides.bank_transaction.on_cancel", }, "Bank Account": { "before_validate": "banking.overrides.bank_account.before_validate", + "validate": "banking.overrides.bank_account.validate", }, "Employee": { "validate": "banking.custom.employee.validate", diff --git a/banking/overrides/bank_account.py b/banking/overrides/bank_account.py index 6b856435..c65a93dd 100644 --- a/banking/overrides/bank_account.py +++ b/banking/overrides/bank_account.py @@ -1,4 +1,20 @@ +import frappe +from frappe import _ + + def before_validate(doc, method): """Remove spaces from IBAN""" if doc.iban: doc.iban = doc.iban.replace(" ", "") + + +def validate(doc, method): + validate_account_currencies(doc) + + +def validate_account_currencies(doc): + if doc.account and doc.bank_fee_account: + bank_account_currency = frappe.db.get_value("Account", doc.account, "account_currency") + bank_fee_currency = frappe.db.get_value("Account", doc.bank_fee_account, "account_currency") + if bank_account_currency != bank_fee_currency: + frappe.throw(_("Company Account and Bank Fee Account must be in the same currency!")) diff --git a/banking/overrides/bank_transaction.py b/banking/overrides/bank_transaction.py index 38fc3544..ed174f68 100644 --- a/banking/overrides/bank_transaction.py +++ b/banking/overrides/bank_transaction.py @@ -3,6 +3,7 @@ from frappe import _ from frappe.core.utils import find from frappe.utils import flt, getdate +from frappe.utils.data import get_link_to_form class CustomBankTransaction(BankTransaction): @@ -72,3 +73,152 @@ def on_update_after_submit(doc, event): ), title=_("Over Allocation"), ) + + +def before_submit(doc: "CustomBankTransaction", method): + date = doc.date or frappe.utils.nowdate() + + if not doc.bank_account: + frappe.throw( + _("The field {0} is required. Please verify the input data.").format( + _(doc.meta.get_label("bank_account")) + ) + ) + + if doc.deposit == 0 and doc.withdrawal == 0: + return + + for fieldname in ["deposit", "withdrawal", "included_fee"]: + value = doc.get(fieldname) + if value is None: + continue + + if value < 0: + frappe.throw( + _("The field {0} is negative. Please verify the input data.").format( + _(doc.meta.get_label(fieldname)) + ) + ) + + cost_center = frappe.get_cached_value("Company", doc.company, "cost_center") + account = frappe.get_cached_value("Bank Account", doc.bank_account, "account") + debit, credit = (doc.deposit, 0) if doc.deposit else (0, doc.withdrawal) + + create_je_bank_fees(doc, cost_center, date, account, debit, credit) + + +def on_cancel(doc, method): + # Cancel the journal entries created by this Bank Transaction. + auto_created_journal_entries = frappe.get_all( + "Journal Entry", + filters={"cheque_no": doc.name}, + pluck="name", + ) + + for journal_entry in auto_created_journal_entries: + try: + je_doc = frappe.get_doc("Journal Entry", journal_entry) + if je_doc.docstatus == 1: + je_doc.cancel() + except Exception as e: + frappe.msgprint( + _("Failed to cancel {0}: {1}").format(get_link_to_form("Journal Entry", journal_entry), e) + ) + + +def create_je_bank_fees(doc, cost_center, date, account, debit, credit): + # Create a journal entry for included bank fees. + included_fee = doc.included_fee + + if included_fee is None or included_fee <= 0: + return + + bank_fee_account = frappe.db.get_value("Bank Account", doc.bank_account, "bank_fee_account") + if not bank_fee_account: + frappe.throw( + _("Please specify a Bank Fee Account for {0}.").format( + get_link_to_form("Bank Account", doc.bank_account) + ) + ) + + je_fee_name = create_automatic_journal_entry( + company=doc.company, + bank_account=doc.bank_account, + bank_transaction=doc.name, + cost_center=cost_center, + date=date, + account=account, + target_account=bank_fee_account, + debit=0, + credit=included_fee, + ) + + if credit > 0: + # Only adjust withdrawals, because deposits never include the fee in the bank amount. + credit_no_fee = credit - included_fee + doc.allocated_amount = included_fee + doc.unallocated_amount = debit + (credit_no_fee or 0) + allocated_amount = included_fee + else: + allocated_amount = 0 + + # For deposits, this entry remains unallocated so deposit reconciliation still works correctly. + doc.append( + "payment_entries", + { + "payment_document": "Journal Entry", + "payment_entry": je_fee_name, + "allocated_amount": allocated_amount, + }, + ) + + if doc.unallocated_amount == 0: + doc.status = "Reconciled" + + +def create_automatic_journal_entry( + company: str, + bank_account: str, + bank_transaction: str, + cost_center: str, + date: str, + account: str, + target_account: str, + debit: float = 0, + credit: float = 0, +): + journal_entry = frappe.new_doc("Journal Entry") + journal_entry.voucher_type = "Journal Entry" + journal_entry.posting_date = date + journal_entry.company = company + journal_entry.user_remark = _("Auto-created from Bank Transaction {0}").format(bank_transaction) + journal_entry.cheque_no = bank_transaction + journal_entry.cheque_date = date + journal_entry.multi_currency = 1 + + journal_entry.append( + "accounts", + { + "account": account, + "bank_account": bank_account, + "debit_in_account_currency": debit, + "credit_in_account_currency": credit, + "cost_center": cost_center, + }, + ) + + journal_entry.append( + "accounts", + { + "account": target_account, + "bank_account": "", + "debit_in_account_currency": credit, + "credit_in_account_currency": debit, + "cost_center": cost_center, + }, + ) + + journal_entry.submit() + frappe.db.set_value("Journal Entry", journal_entry.name, "clearance_date", frappe.utils.today()) + + return journal_entry.name diff --git a/banking/overrides/test_bank_account.py b/banking/overrides/test_bank_account.py new file mode 100644 index 00000000..393a963e --- /dev/null +++ b/banking/overrides/test_bank_account.py @@ -0,0 +1,42 @@ +# Copyright (c) 2025, ALYF GmbH and Contributors +# See license.txt + +import frappe +from frappe.tests.utils import FrappeTestCase + +TEST_COMPANY = "Bolt Trades" + + +def create_currency_account(currency: str, parent_account: str, account_name: str): + acc = frappe.new_doc("Account") + acc.account_name = account_name + acc.account_currency = currency + acc.parent_account = parent_account + acc.insert(ignore_permissions=True, ignore_mandatory=True, ignore_links=True) + return acc + + +def create_bank_account(account: str, bank_fee_account: str | None = None): + ba = frappe.new_doc("Bank Account") + ba.account_name = "_Test_B_Account" + ba.account = account + ba.bank = "_Test_Bank" + ba.is_company_account = 1 + if bank_fee_account: + ba.bank_fee_account = bank_fee_account + ba.insert(ignore_permissions=True, ignore_links=True) + return ba + + +class TestBankAccount(FrappeTestCase): + @classmethod + def setUpClass(cls): + super().setUpClass() + + parent_account = frappe.db.get_value("Account", {"is_group": 1, "company": TEST_COMPANY}) + cls.account_eur = create_currency_account("EUR", parent_account, "_Test_Account_EUR") + cls.account_usd = create_currency_account("USD", parent_account, "_Test_Account_USD") + + def test_validate_account_currencies(self): + with self.assertRaises(frappe.ValidationError): + create_bank_account(self.account_eur.name, bank_fee_account=self.account_usd.name) diff --git a/banking/overrides/test_bank_transaction_included_fees.py b/banking/overrides/test_bank_transaction_included_fees.py new file mode 100644 index 00000000..fb2c4952 --- /dev/null +++ b/banking/overrides/test_bank_transaction_included_fees.py @@ -0,0 +1,137 @@ +# Copyright (c) 2025, ALYF GmbH and Contributors +# See license.txt + +from unittest.mock import patch + +import frappe +from frappe.tests.utils import FrappeTestCase + +TEST_COMPANY = "Bolt Trades" + + +def create_currency_account(currency: str, parent_account: str, account_name: str): + acc = frappe.new_doc("Account") + acc.account_name = account_name + acc.account_currency = currency + acc.parent_account = parent_account + acc.insert(ignore_permissions=True, ignore_mandatory=True, ignore_links=True) + return acc + + +def create_bank_account(account: str, bank_fee_account: str | None = None): + ba = frappe.new_doc("Bank Account") + ba.account_name = "_Test_B_Account" + ba.account = account + ba.bank = "_Test_Bank" + ba.is_company_account = 1 + if bank_fee_account: + ba.bank_fee_account = bank_fee_account + ba.insert(ignore_permissions=True, ignore_links=True) + return ba + + +def create_bank_transaction(insert=True, **values): + doc = frappe.new_doc("Bank Transaction") + doc.company = TEST_COMPANY + doc.update(values) + if insert: + doc.insert(ignore_permissions=True, ignore_mandatory=True, ignore_links=True) + return doc + + +class TestIncludedBankFees(FrappeTestCase): + @classmethod + def setUpClass(cls): + super().setUpClass() + + parent_account = frappe.db.get_value("Account", {"is_group": 1, "company": TEST_COMPANY}) + cls.account_main = create_currency_account("EUR", parent_account, "_Test_Account_EUR") + cls.account_fee = create_currency_account("EUR", parent_account, "_Test_Account_EUR_Fee") + cls.bank_account = create_bank_account(cls.account_main.name, bank_fee_account=cls.account_fee.name) + + @patch("banking.overrides.bank_transaction.create_je_bank_fees") + def test_before_submit(self, mock_create_bank_fees): + from banking.overrides.bank_transaction import before_submit + + bt = create_bank_transaction(insert=False) + + with self.assertRaises(frappe.ValidationError): + before_submit(bt, None) + + bt.bank_account = self.bank_account.name + bt.deposit = -1.0 + with self.assertRaises(frappe.ValidationError): + before_submit(bt, None) + + bt.deposit = 0.0 + bt.withdrawal = -1.0 + with self.assertRaises(frappe.ValidationError): + before_submit(bt, None) + + bt.withdrawal = 1.0 + before_submit(bt, None) + mock_create_bank_fees.assert_called_once() + + @patch("banking.overrides.bank_transaction.create_automatic_journal_entry") + def test_create_je_bank_fees_withdrawal(self, mock_create_je): + from banking.overrides.bank_transaction import create_je_bank_fees + + mock_create_je.return_value = "JE-TEST-0001" + date = "2025-01-01" + + bt = create_bank_transaction(withdrawal=5.0, included_fee=1.0, bank_account=self.bank_account.name) + cost_center = frappe.get_cached_value("Company", bt.company, "cost_center") + + create_je_bank_fees(bt, cost_center, date, self.account_main.name, 0, bt.withdrawal) + + mock_create_je.assert_called_once_with( + company=bt.company, + bank_account=bt.bank_account, + bank_transaction=bt.name, + cost_center=cost_center, + date=date, + account=self.account_main.name, + target_account=self.account_fee.name, + debit=0, + credit=1.0, + ) + self.assertEqual(bt.payment_entries[0].payment_entry, "JE-TEST-0001") + self.assertEqual(bt.payment_entries[0].allocated_amount, 1.0) + self.assertEqual(bt.allocated_amount, 1.0) + self.assertEqual(bt.unallocated_amount, 4.0) + self.assertEqual(bt.status, "Pending") + + bt.withdrawal = 1.0 + create_je_bank_fees(bt, cost_center, date, self.account_main.name, 0, bt.withdrawal) + self.assertEqual(bt.allocated_amount, 1.0) + self.assertEqual(bt.unallocated_amount, 0.0) + self.assertEqual(bt.status, "Reconciled") + + @patch("banking.overrides.bank_transaction.create_automatic_journal_entry") + def test_create_je_bank_fees_deposit(self, mock_create_je): + from banking.overrides.bank_transaction import create_je_bank_fees + + mock_create_je.return_value = "JE-TEST-0001" + date = "2025-01-01" + + bt = create_bank_transaction(deposit=5.0, included_fee=1.0, bank_account=self.bank_account.name) + cost_center = frappe.get_cached_value("Company", bt.company, "cost_center") + + create_je_bank_fees(bt, cost_center, date, self.account_main.name, bt.deposit, 0) + + mock_create_je.assert_called_once_with( + company=bt.company, + bank_account=bt.bank_account, + bank_transaction=bt.name, + cost_center=cost_center, + date=date, + account=self.account_main.name, + target_account=self.account_fee.name, + debit=0, + credit=1.0, + ) + self.assertEqual(bt.payment_entries[0].payment_entry, "JE-TEST-0001") + self.assertEqual(bt.payment_entries[0].allocated_amount, 0.0) + self.assertEqual(bt.allocated_amount, 0.0) + self.assertEqual(bt.unallocated_amount, 5.0) + self.assertEqual(bt.status, "Pending") diff --git a/banking/patches.txt b/banking/patches.txt index 15a2faee..bf0a6a01 100644 --- a/banking/patches.txt +++ b/banking/patches.txt @@ -1,5 +1,5 @@ [pre_model_sync] -banking.patches.recreate_custom_fields # 2025-10-23 +banking.patches.recreate_custom_fields # 2025-12-16 banking.patches.remove_spaces_from_iban [post_model_sync] From 0c2230f88645f3212fcf403b0ccec97b318b5dfb Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Tue, 17 Feb 2026 23:50:01 +0100 Subject: [PATCH 02/14] fix(Bank Transaction): make automatic fee booking optional Guard included-fee Journal Entry creation behind a Banking Settings flag so existing sites without bank fee account setup continue to work until the feature is explicitly enabled. Co-authored-by: Cursor --- .../banking_settings/banking_settings.json | 8 ++++++++ .../banking_settings/banking_settings.py | 1 + banking/overrides/bank_transaction.py | 3 ++- .../test_bank_transaction_included_fees.py | 18 ++++++++++++++++++ 4 files changed, 29 insertions(+), 1 deletion(-) diff --git a/banking/klarna_kosma_integration/doctype/banking_settings/banking_settings.json b/banking/klarna_kosma_integration/doctype/banking_settings/banking_settings.json index 102ab36a..74289b9a 100644 --- a/banking/klarna_kosma_integration/doctype/banking_settings/banking_settings.json +++ b/banking/klarna_kosma_integration/doctype/banking_settings/banking_settings.json @@ -19,6 +19,7 @@ "fintech_licensee_name", "fintech_license_key", "bank_reconciliation_tab", + "enable_automatic_journal_entries_for_bank_fees", "advanced_section", "reference_fields", "voucher_matching_defaults" @@ -115,6 +116,13 @@ "fieldtype": "Table MultiSelect", "label": "Voucher Matching Defaults", "options": "Voucher Matching Default" + }, + { + "default": "0", + "description": "When a Bank Transaction with Included Fee is submitted, we'll automatically create a Journal Entry for the fee amount. Please specify the Bank Fee Account in your Bank Account.", + "fieldname": "enable_automatic_journal_entries_for_bank_fees", + "fieldtype": "Check", + "label": "Enable Automatic Journal Entries for Bank Fees" } ], "grid_page_length": 50, diff --git a/banking/klarna_kosma_integration/doctype/banking_settings/banking_settings.py b/banking/klarna_kosma_integration/doctype/banking_settings/banking_settings.py index 2ed7c120..6ae16fbc 100644 --- a/banking/klarna_kosma_integration/doctype/banking_settings/banking_settings.py +++ b/banking/klarna_kosma_integration/doctype/banking_settings/banking_settings.py @@ -33,6 +33,7 @@ class BankingSettings(Document): admin_endpoint: DF.Data | None api_token: DF.Password | None customer_id: DF.Data | None + enable_automatic_journal_entries_for_bank_fees: DF.Check enable_ebics: DF.Check enabled: DF.Check fintech_license_key: DF.Password | None diff --git a/banking/overrides/bank_transaction.py b/banking/overrides/bank_transaction.py index ed174f68..098fe884 100644 --- a/banking/overrides/bank_transaction.py +++ b/banking/overrides/bank_transaction.py @@ -104,7 +104,8 @@ def before_submit(doc: "CustomBankTransaction", method): account = frappe.get_cached_value("Bank Account", doc.bank_account, "account") debit, credit = (doc.deposit, 0) if doc.deposit else (0, doc.withdrawal) - create_je_bank_fees(doc, cost_center, date, account, debit, credit) + if flt(frappe.db.get_single_value("Banking Settings", "enable_automatic_journal_entries_for_bank_fees")): + create_je_bank_fees(doc, cost_center, date, account, debit, credit) def on_cancel(doc, method): diff --git a/banking/overrides/test_bank_transaction_included_fees.py b/banking/overrides/test_bank_transaction_included_fees.py index fb2c4952..c898011b 100644 --- a/banking/overrides/test_bank_transaction_included_fees.py +++ b/banking/overrides/test_bank_transaction_included_fees.py @@ -69,9 +69,27 @@ def test_before_submit(self, mock_create_bank_fees): before_submit(bt, None) bt.withdrawal = 1.0 + frappe.db.set_single_value("Banking Settings", "enable_automatic_journal_entries_for_bank_fees", 1) before_submit(bt, None) mock_create_bank_fees.assert_called_once() + @patch("banking.overrides.bank_transaction.create_je_bank_fees") + def test_before_submit_with_disabled_automatic_fee_entries(self, mock_create_bank_fees): + from banking.overrides.bank_transaction import before_submit + + frappe.db.set_single_value("Banking Settings", "enable_automatic_journal_entries_for_bank_fees", 0) + bt = create_bank_transaction( + insert=False, + bank_account=self.bank_account.name, + withdrawal=10.0, + included_fee=1.0, + date="2025-01-01", + ) + + before_submit(bt, None) + + mock_create_bank_fees.assert_not_called() + @patch("banking.overrides.bank_transaction.create_automatic_journal_entry") def test_create_je_bank_fees_withdrawal(self, mock_create_je): from banking.overrides.bank_transaction import create_je_bank_fees From b2354c145011394e753712cb04eae7723a215248 Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Tue, 17 Feb 2026 23:56:27 +0100 Subject: [PATCH 03/14] chore(Banking Settings): bump modified timestamp for model sync Update the DocType JSON modified timestamp so the controller change is picked up reliably during model sync. Co-authored-by: Cursor --- .../doctype/banking_settings/banking_settings.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/banking/klarna_kosma_integration/doctype/banking_settings/banking_settings.json b/banking/klarna_kosma_integration/doctype/banking_settings/banking_settings.json index 74289b9a..e118310c 100644 --- a/banking/klarna_kosma_integration/doctype/banking_settings/banking_settings.json +++ b/banking/klarna_kosma_integration/doctype/banking_settings/banking_settings.json @@ -129,7 +129,7 @@ "index_web_pages_for_search": 1, "issingle": 1, "links": [], - "modified": "2025-10-07 18:44:07.181312", + "modified": "2026-02-17 23:56:13.629502", "modified_by": "Administrator", "module": "Klarna Kosma Integration", "name": "Banking Settings", From 6b1c2e23ecd83f570edc6895933c4f268a549fae Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Wed, 18 Feb 2026 00:09:50 +0100 Subject: [PATCH 04/14] fix(Bank Transaction): validate withdrawal against included fee Reject withdrawal transactions where included fee exceeds withdrawal amount, while still allowing fee handling when deposit is not set. Co-authored-by: Cursor --- banking/overrides/bank_transaction.py | 7 ++++ .../test_bank_transaction_included_fees.py | 35 +++++++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/banking/overrides/bank_transaction.py b/banking/overrides/bank_transaction.py index 1b27db69..8a031148 100644 --- a/banking/overrides/bank_transaction.py +++ b/banking/overrides/bank_transaction.py @@ -133,6 +133,13 @@ def before_submit(doc: "CustomBankTransaction", method): ) ) + if flt(doc.withdrawal) and flt(doc.included_fee) > flt(doc.withdrawal): + frappe.throw( + _("The field {0} cannot be greater than {1}. Please verify the input data.").format( + _(doc.meta.get_label("included_fee")), _(doc.meta.get_label("withdrawal")) + ) + ) + cost_center = frappe.get_cached_value("Company", doc.company, "cost_center") account = frappe.get_cached_value("Bank Account", doc.bank_account, "account") debit, credit = (doc.deposit, 0) if doc.deposit else (0, doc.withdrawal) diff --git a/banking/overrides/test_bank_transaction_included_fees.py b/banking/overrides/test_bank_transaction_included_fees.py index c898011b..17d0ccc9 100644 --- a/banking/overrides/test_bank_transaction_included_fees.py +++ b/banking/overrides/test_bank_transaction_included_fees.py @@ -90,6 +90,41 @@ def test_before_submit_with_disabled_automatic_fee_entries(self, mock_create_ban mock_create_bank_fees.assert_not_called() + @patch("banking.overrides.bank_transaction.create_je_bank_fees") + def test_before_submit_rejects_fee_larger_than_withdrawal(self, mock_create_bank_fees): + from banking.overrides.bank_transaction import before_submit + + frappe.db.set_single_value("Banking Settings", "enable_automatic_journal_entries_for_bank_fees", 1) + bt = create_bank_transaction( + insert=False, + bank_account=self.bank_account.name, + withdrawal=1.0, + included_fee=2.0, + date="2025-01-01", + ) + + with self.assertRaises(frappe.ValidationError): + before_submit(bt, None) + + mock_create_bank_fees.assert_not_called() + + @patch("banking.overrides.bank_transaction.create_je_bank_fees") + def test_before_submit_allows_missing_deposit_for_withdrawal_with_fee(self, mock_create_bank_fees): + from banking.overrides.bank_transaction import before_submit + + frappe.db.set_single_value("Banking Settings", "enable_automatic_journal_entries_for_bank_fees", 1) + bt = create_bank_transaction( + insert=False, + bank_account=self.bank_account.name, + withdrawal=2.0, + included_fee=1.0, + date="2025-01-01", + ) + + before_submit(bt, None) + + mock_create_bank_fees.assert_called_once() + @patch("banking.overrides.bank_transaction.create_automatic_journal_entry") def test_create_je_bank_fees_withdrawal(self, mock_create_je): from banking.overrides.bank_transaction import create_je_bank_fees From 6c40e7c85a62fc23e4f1faaef55708b79ab3b598 Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Sat, 21 Mar 2026 15:25:37 +0100 Subject: [PATCH 05/14] chore: bump min required erp version we depend on https://github.com/frappe/erpnext/pull/52760 --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index eb2f6fd9..497d2772 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -19,7 +19,7 @@ build-backend = "flit_core.buildapi" [tool.bench.frappe-dependencies] frappe = ">=15.60.0,<16.0.0" -erpnext = ">=15.57.0,<16.0.0" +erpnext = ">=15.99.0,<16.0.0" [tool.ruff] line-length = 110 From 2f3cc247d0ccba6f0b6568982b05da0f88cf416d Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Sat, 21 Mar 2026 16:29:59 +0100 Subject: [PATCH 06/14] fix: Cancel only fee JEs created by this feature --- banking/hooks.py | 1 - banking/overrides/bank_transaction.py | 24 ++++-------------------- 2 files changed, 4 insertions(+), 21 deletions(-) diff --git a/banking/hooks.py b/banking/hooks.py index 02b46532..150c2dec 100644 --- a/banking/hooks.py +++ b/banking/hooks.py @@ -110,7 +110,6 @@ "Bank Transaction": { "on_update_after_submit": "banking.overrides.bank_transaction.on_update_after_submit", "before_submit": "banking.overrides.bank_transaction.before_submit", - "on_cancel": "banking.overrides.bank_transaction.on_cancel", }, "Bank Account": { "before_validate": "banking.overrides.bank_account.before_validate", diff --git a/banking/overrides/bank_transaction.py b/banking/overrides/bank_transaction.py index 8a031148..6d8d10c1 100644 --- a/banking/overrides/bank_transaction.py +++ b/banking/overrides/bank_transaction.py @@ -148,25 +148,6 @@ def before_submit(doc: "CustomBankTransaction", method): create_je_bank_fees(doc, cost_center, date, account, debit, credit) -def on_cancel(doc, method): - # Cancel the journal entries created by this Bank Transaction. - auto_created_journal_entries = frappe.get_all( - "Journal Entry", - filters={"cheque_no": doc.name}, - pluck="name", - ) - - for journal_entry in auto_created_journal_entries: - try: - je_doc = frappe.get_doc("Journal Entry", journal_entry) - if je_doc.docstatus == 1: - je_doc.cancel() - except Exception as e: - frappe.msgprint( - _("Failed to cancel {0}: {1}").format(get_link_to_form("Journal Entry", journal_entry), e) - ) - - def create_je_bank_fees(doc, cost_center, date, account, debit, credit): # Create a journal entry for included bank fees. included_fee = doc.included_fee @@ -229,10 +210,11 @@ def create_automatic_journal_entry( credit: float = 0, ): journal_entry = frappe.new_doc("Journal Entry") - journal_entry.voucher_type = "Journal Entry" + journal_entry.voucher_type = "Bank Entry" journal_entry.posting_date = date journal_entry.company = company journal_entry.user_remark = _("Auto-created from Bank Transaction {0}").format(bank_transaction) + journal_entry.is_system_generated = 1 journal_entry.cheque_no = bank_transaction journal_entry.cheque_date = date journal_entry.multi_currency = 1 @@ -245,6 +227,8 @@ def create_automatic_journal_entry( "debit_in_account_currency": debit, "credit_in_account_currency": credit, "cost_center": cost_center, + "reference_type": "Bank Transaction", + "reference_name": bank_transaction, }, ) From 2d697cadf70609c482b79dc4cfbf2a5dc848fb78 Mon Sep 17 00:00:00 2001 From: Raffael Meyer <14891507+barredterra@users.noreply.github.com> Date: Sat, 21 Mar 2026 16:34:27 +0100 Subject: [PATCH 07/14] fix: use bank date as clearance_date Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> --- banking/overrides/bank_transaction.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/banking/overrides/bank_transaction.py b/banking/overrides/bank_transaction.py index 6d8d10c1..b3de687b 100644 --- a/banking/overrides/bank_transaction.py +++ b/banking/overrides/bank_transaction.py @@ -244,6 +244,6 @@ def create_automatic_journal_entry( ) journal_entry.submit() - frappe.db.set_value("Journal Entry", journal_entry.name, "clearance_date", frappe.utils.today()) + frappe.db.set_value("Journal Entry", journal_entry.name, "clearance_date", date) return journal_entry.name From ba43fb528a13739581ef06f07c94f534e2e058df Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Sat, 21 Mar 2026 16:56:06 +0100 Subject: [PATCH 08/14] test: cover disabled automatic bank fee entries Ensure a Bank Transaction can be submitted without a bank fee account when automatic bank fee journal entries are disabled, and assert that no linked fee Journal Entry is created. --- .../test_bank_transaction_included_fees.py | 43 ++++++++++++++++++- 1 file changed, 41 insertions(+), 2 deletions(-) diff --git a/banking/overrides/test_bank_transaction_included_fees.py b/banking/overrides/test_bank_transaction_included_fees.py index 17d0ccc9..00f3affb 100644 --- a/banking/overrides/test_bank_transaction_included_fees.py +++ b/banking/overrides/test_bank_transaction_included_fees.py @@ -18,9 +18,11 @@ def create_currency_account(currency: str, parent_account: str, account_name: st return acc -def create_bank_account(account: str, bank_fee_account: str | None = None): +def create_bank_account( + account: str, bank_fee_account: str | None = None, account_name: str = "_Test_B_Account" +): ba = frappe.new_doc("Bank Account") - ba.account_name = "_Test_B_Account" + ba.account_name = account_name ba.account = account ba.bank = "_Test_Bank" ba.is_company_account = 1 @@ -46,8 +48,14 @@ def setUpClass(cls): parent_account = frappe.db.get_value("Account", {"is_group": 1, "company": TEST_COMPANY}) cls.account_main = create_currency_account("EUR", parent_account, "_Test_Account_EUR") + cls.account_main_without_fee = create_currency_account( + "EUR", parent_account, "_Test_Account_EUR_No_Fee" + ) cls.account_fee = create_currency_account("EUR", parent_account, "_Test_Account_EUR_Fee") cls.bank_account = create_bank_account(cls.account_main.name, bank_fee_account=cls.account_fee.name) + cls.bank_account_without_fee = create_bank_account( + cls.account_main_without_fee.name, account_name="_Test_B_Account_No_Fee" + ) @patch("banking.overrides.bank_transaction.create_je_bank_fees") def test_before_submit(self, mock_create_bank_fees): @@ -90,6 +98,37 @@ def test_before_submit_with_disabled_automatic_fee_entries(self, mock_create_ban mock_create_bank_fees.assert_not_called() + def test_submit_without_fee_account_when_automatic_fee_entries_are_disabled(self): + """Ensure a Bank Transaction can be submitted without a bank fee account + when automatic bank fee journal entries are disabled, and assert that + no linked fee Journal Entry is created.""" + frappe.db.set_single_value("Banking Settings", "enable_automatic_journal_entries_for_bank_fees", 0) + + bt = create_bank_transaction( + bank_account=self.bank_account_without_fee.name, + withdrawal=10.0, + included_fee=1.0, + date="2025-01-01", + currency="EUR", + description="Bank fee should not create a Journal Entry when disabled", + ) + + bt.submit() + bt.reload() + + self.assertEqual(bt.docstatus, 1) + self.assertEqual(bt.status, "Unreconciled") + self.assertFalse(bt.payment_entries) + self.assertFalse( + frappe.db.exists( + "Journal Entry Account", + { + "reference_type": "Bank Transaction", + "reference_name": bt.name, + }, + ) + ) + @patch("banking.overrides.bank_transaction.create_je_bank_fees") def test_before_submit_rejects_fee_larger_than_withdrawal(self, mock_create_bank_fees): from banking.overrides.bank_transaction import before_submit From 8606e758586f4420e9a0aaa11ed6c9d8ecd0e7fb Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Sat, 21 Mar 2026 17:19:29 +0100 Subject: [PATCH 09/14] fix(banking): enforce fee account setup for automatic bank fee entries Prevent automatic bank fee journal entries from being enabled until every company Bank Account has a bank fee account, and reject company bank accounts without one while the feature is enabled. --- .../banking_settings/banking_settings.py | 19 +++++++ .../banking_settings/test_banking_settings.py | 56 +++++++++++++++++++ banking/overrides/bank_account.py | 27 +++++++++ banking/overrides/test_bank_account.py | 42 +++++++++++++- 4 files changed, 141 insertions(+), 3 deletions(-) diff --git a/banking/klarna_kosma_integration/doctype/banking_settings/banking_settings.py b/banking/klarna_kosma_integration/doctype/banking_settings/banking_settings.py index 6ae16fbc..07542eb1 100644 --- a/banking/klarna_kosma_integration/doctype/banking_settings/banking_settings.py +++ b/banking/klarna_kosma_integration/doctype/banking_settings/banking_settings.py @@ -12,6 +12,7 @@ from semantic_version import Version from banking.klarna_kosma_integration.admin import Admin +from banking.overrides.bank_account import get_company_bank_accounts_without_fee_account class BankingSettings(Document): @@ -42,9 +43,27 @@ class BankingSettings(Document): voucher_matching_defaults: DF.TableMultiSelect[VoucherMatchingDefault] # end: auto-generated types + def validate(self): + self.validate_automatic_fee_entry_configuration() + def before_validate(self): self.update_fintech_license() + def validate_automatic_fee_entry_configuration(self): + if not self.enable_automatic_journal_entries_for_bank_fees: + return + + if not self.has_value_changed("enable_automatic_journal_entries_for_bank_fees"): + return + + missing_fee_accounts = get_company_bank_accounts_without_fee_account() + if missing_fee_accounts: + frappe.throw( + _( + "Automatic journal entries for bank fees can only be enabled when all company Bank Accounts have a Bank Fee Account." + ) + ) + def update_fintech_license(self): if not self.enabled: return self.reset_fintech_license() diff --git a/banking/klarna_kosma_integration/doctype/banking_settings/test_banking_settings.py b/banking/klarna_kosma_integration/doctype/banking_settings/test_banking_settings.py index 5de127f8..6edb3603 100644 --- a/banking/klarna_kosma_integration/doctype/banking_settings/test_banking_settings.py +++ b/banking/klarna_kosma_integration/doctype/banking_settings/test_banking_settings.py @@ -12,13 +12,69 @@ successful_request_exists, ) +TEST_COMPANY = "Bolt Trades" + + +def create_currency_account(currency: str, parent_account: str, account_name: str): + acc = frappe.new_doc("Account") + acc.account_name = account_name + acc.account_currency = currency + acc.parent_account = parent_account + acc.insert(ignore_permissions=True, ignore_mandatory=True, ignore_links=True) + return acc + + +def create_bank_account(account: str, account_name: str): + ba = frappe.new_doc("Bank Account") + ba.account_name = account_name + ba.account = account + ba.bank = "_Test_Bank" + ba.is_company_account = 1 + ba.insert(ignore_permissions=True, ignore_links=True) + return ba + class TestBankingSettings(FrappeTestCase): + @classmethod + def setUpClass(cls): + super().setUpClass() + frappe.db.set_single_value("Banking Settings", "enable_automatic_journal_entries_for_bank_fees", 0) + + parent_account = frappe.db.get_value("Account", {"is_group": 1, "company": TEST_COMPANY}) + cls.account_without_fee = create_currency_account( + "EUR", parent_account, "_Test_Banking_Settings_Account_No_Fee" + ) + cls.bank_account_without_fee = create_bank_account( + cls.account_without_fee.name, "_Test_Banking_Settings_Bank_Account_No_Fee" + ) + + def setUp(self): + super().setUp() + frappe.db.set_single_value("Banking Settings", "enable_automatic_journal_entries_for_bank_fees", 0) + + def tearDown(self): + frappe.db.set_single_value("Banking Settings", "enable_automatic_journal_entries_for_bank_fees", 0) + super().tearDown() + def test_successful_request_exists(self): with create_successful_request("C53", date(2025, 1, 1), date(2025, 1, 1)) as request: self.assertTrue(successful_request_exists(None, request.order_type, date(2025, 1, 1))) self.assertFalse(successful_request_exists(None, request.order_type, date(2025, 1, 2))) + def test_cannot_enable_automatic_fee_entries_without_fee_accounts_for_all_company_bank_accounts( + self, + ): + settings = frappe.get_single("Banking Settings") + settings.enable_automatic_journal_entries_for_bank_fees = 1 + + with self.assertRaises(frappe.ValidationError): + settings.save() + + self.assertEqual( + frappe.db.get_single_value("Banking Settings", "enable_automatic_journal_entries_for_bank_fees"), + 0, + ) + @contextmanager def create_successful_request(order_type: str, start_date: date, end_date: date): diff --git a/banking/overrides/bank_account.py b/banking/overrides/bank_account.py index c65a93dd..1c6e1efe 100644 --- a/banking/overrides/bank_account.py +++ b/banking/overrides/bank_account.py @@ -1,5 +1,6 @@ import frappe from frappe import _ +from frappe.utils import cint def before_validate(doc, method): @@ -10,6 +11,7 @@ def before_validate(doc, method): def validate(doc, method): validate_account_currencies(doc) + validate_required_bank_fee_account(doc) def validate_account_currencies(doc): @@ -18,3 +20,28 @@ def validate_account_currencies(doc): bank_fee_currency = frappe.db.get_value("Account", doc.bank_fee_account, "account_currency") if bank_account_currency != bank_fee_currency: frappe.throw(_("Company Account and Bank Fee Account must be in the same currency!")) + + +def validate_required_bank_fee_account(doc): + if doc.bank_fee_account or not cint(doc.is_company_account) or not automatic_fee_entries_enabled(): + return + + frappe.throw( + _( + "Bank Fee Account is mandatory for company Bank Accounts while automatic journal entries for bank fees are enabled in Banking Settings." + ) + ) + + +def automatic_fee_entries_enabled() -> bool: + return bool( + cint(frappe.db.get_single_value("Banking Settings", "enable_automatic_journal_entries_for_bank_fees")) + ) + + +def get_company_bank_accounts_without_fee_account() -> list[str]: + return frappe.get_all( + "Bank Account", + filters={"is_company_account": 1, "bank_fee_account": ["is", "not set"]}, + pluck="name", + ) diff --git a/banking/overrides/test_bank_account.py b/banking/overrides/test_bank_account.py index 393a963e..1c0f9fd2 100644 --- a/banking/overrides/test_bank_account.py +++ b/banking/overrides/test_bank_account.py @@ -16,9 +16,11 @@ def create_currency_account(currency: str, parent_account: str, account_name: st return acc -def create_bank_account(account: str, bank_fee_account: str | None = None): +def create_bank_account( + account: str, bank_fee_account: str | None = None, account_name: str = "_Test_B_Account" +): ba = frappe.new_doc("Bank Account") - ba.account_name = "_Test_B_Account" + ba.account_name = account_name ba.account = account ba.bank = "_Test_Bank" ba.is_company_account = 1 @@ -32,11 +34,45 @@ class TestBankAccount(FrappeTestCase): @classmethod def setUpClass(cls): super().setUpClass() + frappe.db.set_single_value("Banking Settings", "enable_automatic_journal_entries_for_bank_fees", 0) parent_account = frappe.db.get_value("Account", {"is_group": 1, "company": TEST_COMPANY}) cls.account_eur = create_currency_account("EUR", parent_account, "_Test_Account_EUR") cls.account_usd = create_currency_account("USD", parent_account, "_Test_Account_USD") + cls.account_fee = create_currency_account("EUR", parent_account, "_Test_Account_EUR_Fee") + cls.bank_account_with_fee = create_bank_account( + cls.account_eur.name, + bank_fee_account=cls.account_fee.name, + account_name="_Test_B_Account_With_Fee", + ) - def test_validate_account_currencies(self): + def setUp(self): + super().setUp() + frappe.db.set_single_value("Banking Settings", "enable_automatic_journal_entries_for_bank_fees", 0) + + def tearDown(self): + frappe.db.set_single_value("Banking Settings", "enable_automatic_journal_entries_for_bank_fees", 0) + super().tearDown() + + def test_currency_mismatch_rejected(self): + """Bank and fee accounts must use the same currency.""" with self.assertRaises(frappe.ValidationError): create_bank_account(self.account_eur.name, bank_fee_account=self.account_usd.name) + + def test_requires_fee_account_when_enabled(self): + """Company bank accounts cannot be created or saved without a fee account.""" + frappe.db.set_single_value("Banking Settings", "enable_automatic_journal_entries_for_bank_fees", 1) + + with self.assertRaises(frappe.ValidationError): + create_bank_account(self.account_eur.name, account_name="_Test_B_Account_Without_Fee") + + bank_account = frappe.get_doc("Bank Account", self.bank_account_with_fee.name) + bank_account.bank_fee_account = None + + with self.assertRaises(frappe.ValidationError): + bank_account.save() + + self.assertEqual( + frappe.db.get_value("Bank Account", self.bank_account_with_fee.name, "bank_fee_account"), + self.account_fee.name, + ) From 0b77596357f1b88d59ef6170cfb65292e3b25368 Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Sat, 21 Mar 2026 17:49:00 +0100 Subject: [PATCH 10/14] test: submitting a withdrawal with an included fee must create and reconcile the fee JE --- .../test_bank_transaction_included_fees.py | 79 +++++++++++-------- 1 file changed, 44 insertions(+), 35 deletions(-) diff --git a/banking/overrides/test_bank_transaction_included_fees.py b/banking/overrides/test_bank_transaction_included_fees.py index 00f3affb..b2727dd4 100644 --- a/banking/overrides/test_bank_transaction_included_fees.py +++ b/banking/overrides/test_bank_transaction_included_fees.py @@ -129,6 +129,50 @@ def test_submit_without_fee_account_when_automatic_fee_entries_are_disabled(self ) ) + def test_submit_creates_and_reconciles_fee_journal_entry_for_withdrawal(self): + """Submitting a withdrawal with an included fee must create and reconcile the fee JE.""" + frappe.db.set_single_value("Banking Settings", "enable_automatic_journal_entries_for_bank_fees", 1) + + bt = create_bank_transaction( + bank_account=self.bank_account.name, + withdrawal=5.0, + included_fee=1.0, + date=frappe.utils.nowdate(), + currency="EUR", + description="Withdrawal with included bank fee", + ) + + bt.submit() + bt.reload() + + self.assertEqual(bt.docstatus, 1) + self.assertEqual(len(bt.payment_entries), 1) + self.assertEqual(bt.payment_entries[0].payment_document, "Journal Entry") + self.assertEqual(bt.payment_entries[0].allocated_amount, 1.0) + self.assertEqual(bt.allocated_amount, 1.0) + self.assertEqual(bt.unallocated_amount, 4.0) + + je = frappe.get_doc("Journal Entry", bt.payment_entries[0].payment_entry) + self.assertEqual(je.docstatus, 1) + self.assertEqual(je.voucher_type, "Bank Entry") + self.assertTrue(je.is_system_generated) + self.assertEqual(je.cheque_no, bt.name) + self.assertEqual(str(frappe.db.get_value("Journal Entry", je.name, "clearance_date")), str(bt.date)) + self.assertTrue( + frappe.db.exists( + "Journal Entry Account", + { + "parent": je.name, + "reference_type": "Bank Transaction", + "reference_name": bt.name, + }, + ) + ) + + accounts = {row.account: row for row in je.accounts} + self.assertEqual(accounts[self.account_main.name].credit_in_account_currency, 1.0) + self.assertEqual(accounts[self.account_fee.name].debit_in_account_currency, 1.0) + @patch("banking.overrides.bank_transaction.create_je_bank_fees") def test_before_submit_rejects_fee_larger_than_withdrawal(self, mock_create_bank_fees): from banking.overrides.bank_transaction import before_submit @@ -164,41 +208,6 @@ def test_before_submit_allows_missing_deposit_for_withdrawal_with_fee(self, mock mock_create_bank_fees.assert_called_once() - @patch("banking.overrides.bank_transaction.create_automatic_journal_entry") - def test_create_je_bank_fees_withdrawal(self, mock_create_je): - from banking.overrides.bank_transaction import create_je_bank_fees - - mock_create_je.return_value = "JE-TEST-0001" - date = "2025-01-01" - - bt = create_bank_transaction(withdrawal=5.0, included_fee=1.0, bank_account=self.bank_account.name) - cost_center = frappe.get_cached_value("Company", bt.company, "cost_center") - - create_je_bank_fees(bt, cost_center, date, self.account_main.name, 0, bt.withdrawal) - - mock_create_je.assert_called_once_with( - company=bt.company, - bank_account=bt.bank_account, - bank_transaction=bt.name, - cost_center=cost_center, - date=date, - account=self.account_main.name, - target_account=self.account_fee.name, - debit=0, - credit=1.0, - ) - self.assertEqual(bt.payment_entries[0].payment_entry, "JE-TEST-0001") - self.assertEqual(bt.payment_entries[0].allocated_amount, 1.0) - self.assertEqual(bt.allocated_amount, 1.0) - self.assertEqual(bt.unallocated_amount, 4.0) - self.assertEqual(bt.status, "Pending") - - bt.withdrawal = 1.0 - create_je_bank_fees(bt, cost_center, date, self.account_main.name, 0, bt.withdrawal) - self.assertEqual(bt.allocated_amount, 1.0) - self.assertEqual(bt.unallocated_amount, 0.0) - self.assertEqual(bt.status, "Reconciled") - @patch("banking.overrides.bank_transaction.create_automatic_journal_entry") def test_create_je_bank_fees_deposit(self, mock_create_je): from banking.overrides.bank_transaction import create_je_bank_fees From b3f1265e0fede2c12f147485a24112cd35758c3f Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Sun, 22 Mar 2026 15:08:28 +0100 Subject: [PATCH 11/14] fix: keep deposit fee journal entries out of bank reconciliation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Create fee Journal Entries for deposit Bank Transactions with included fees, but do not add them to payment entries because the fee is already deducted before the bank credits the deposit. Semantically, a row in Bank Transaction would say "this JE is allocatable against the bank amount later", which is exactly what we don’t want for a deposit fee. Add regression coverage for included-fee deposit and withdrawal flows. --- .../banking_settings/test_banking_settings.py | 22 ++-- banking/overrides/bank_transaction.py | 23 ++-- banking/overrides/test_bank_account.py | 22 ++-- .../test_bank_transaction_included_fees.py | 107 ++++++++++++------ 4 files changed, 110 insertions(+), 64 deletions(-) diff --git a/banking/klarna_kosma_integration/doctype/banking_settings/test_banking_settings.py b/banking/klarna_kosma_integration/doctype/banking_settings/test_banking_settings.py index 6edb3603..9cbf89a2 100644 --- a/banking/klarna_kosma_integration/doctype/banking_settings/test_banking_settings.py +++ b/banking/klarna_kosma_integration/doctype/banking_settings/test_banking_settings.py @@ -6,6 +6,7 @@ from datetime import date import frappe +from erpnext.accounts.doctype.account.test_account import create_account from frappe.tests.utils import FrappeTestCase from banking.klarna_kosma_integration.doctype.banking_settings.banking_settings import ( @@ -15,13 +16,19 @@ TEST_COMPANY = "Bolt Trades" +def get_bank_parent_account(company: str) -> str: + return frappe.db.get_value("Account", {"account_type": "Bank", "is_group": 1, "company": company}) + + def create_currency_account(currency: str, parent_account: str, account_name: str): - acc = frappe.new_doc("Account") - acc.account_name = account_name - acc.account_currency = currency - acc.parent_account = parent_account - acc.insert(ignore_permissions=True, ignore_mandatory=True, ignore_links=True) - return acc + account = create_account( + account_name=account_name, + account_type="Bank", + parent_account=parent_account, + company=TEST_COMPANY, + account_currency=currency, + ) + return frappe.get_doc("Account", account) def create_bank_account(account: str, account_name: str): @@ -29,6 +36,7 @@ def create_bank_account(account: str, account_name: str): ba.account_name = account_name ba.account = account ba.bank = "_Test_Bank" + ba.company = TEST_COMPANY ba.is_company_account = 1 ba.insert(ignore_permissions=True, ignore_links=True) return ba @@ -40,7 +48,7 @@ def setUpClass(cls): super().setUpClass() frappe.db.set_single_value("Banking Settings", "enable_automatic_journal_entries_for_bank_fees", 0) - parent_account = frappe.db.get_value("Account", {"is_group": 1, "company": TEST_COMPANY}) + parent_account = get_bank_parent_account(TEST_COMPANY) cls.account_without_fee = create_currency_account( "EUR", parent_account, "_Test_Banking_Settings_Account_No_Fee" ) diff --git a/banking/overrides/bank_transaction.py b/banking/overrides/bank_transaction.py index b3de687b..29e759d6 100644 --- a/banking/overrides/bank_transaction.py +++ b/banking/overrides/bank_transaction.py @@ -180,19 +180,16 @@ def create_je_bank_fees(doc, cost_center, date, account, debit, credit): credit_no_fee = credit - included_fee doc.allocated_amount = included_fee doc.unallocated_amount = debit + (credit_no_fee or 0) - allocated_amount = included_fee - else: - allocated_amount = 0 - - # For deposits, this entry remains unallocated so deposit reconciliation still works correctly. - doc.append( - "payment_entries", - { - "payment_document": "Journal Entry", - "payment_entry": je_fee_name, - "allocated_amount": allocated_amount, - }, - ) + doc.append( + "payment_entries", + { + "payment_document": "Journal Entry", + "payment_entry": je_fee_name, + "allocated_amount": included_fee, + }, + ) + # Deposit fees are linked via the Journal Entry reference only and must stay + # out of the reconciliation table. if doc.unallocated_amount == 0: doc.status = "Reconciled" diff --git a/banking/overrides/test_bank_account.py b/banking/overrides/test_bank_account.py index 1c0f9fd2..afafb5a4 100644 --- a/banking/overrides/test_bank_account.py +++ b/banking/overrides/test_bank_account.py @@ -2,18 +2,25 @@ # See license.txt import frappe +from erpnext.accounts.doctype.account.test_account import create_account from frappe.tests.utils import FrappeTestCase TEST_COMPANY = "Bolt Trades" +def get_bank_parent_account(company: str) -> str: + return frappe.db.get_value("Account", {"account_type": "Bank", "is_group": 1, "company": company}) + + def create_currency_account(currency: str, parent_account: str, account_name: str): - acc = frappe.new_doc("Account") - acc.account_name = account_name - acc.account_currency = currency - acc.parent_account = parent_account - acc.insert(ignore_permissions=True, ignore_mandatory=True, ignore_links=True) - return acc + account = create_account( + account_name=account_name, + account_type="Bank", + parent_account=parent_account, + company=TEST_COMPANY, + account_currency=currency, + ) + return frappe.get_doc("Account", account) def create_bank_account( @@ -23,6 +30,7 @@ def create_bank_account( ba.account_name = account_name ba.account = account ba.bank = "_Test_Bank" + ba.company = TEST_COMPANY ba.is_company_account = 1 if bank_fee_account: ba.bank_fee_account = bank_fee_account @@ -36,7 +44,7 @@ def setUpClass(cls): super().setUpClass() frappe.db.set_single_value("Banking Settings", "enable_automatic_journal_entries_for_bank_fees", 0) - parent_account = frappe.db.get_value("Account", {"is_group": 1, "company": TEST_COMPANY}) + parent_account = get_bank_parent_account(TEST_COMPANY) cls.account_eur = create_currency_account("EUR", parent_account, "_Test_Account_EUR") cls.account_usd = create_currency_account("USD", parent_account, "_Test_Account_USD") cls.account_fee = create_currency_account("EUR", parent_account, "_Test_Account_EUR_Fee") diff --git a/banking/overrides/test_bank_transaction_included_fees.py b/banking/overrides/test_bank_transaction_included_fees.py index b2727dd4..e7b6849a 100644 --- a/banking/overrides/test_bank_transaction_included_fees.py +++ b/banking/overrides/test_bank_transaction_included_fees.py @@ -4,18 +4,25 @@ from unittest.mock import patch import frappe +from erpnext.accounts.doctype.account.test_account import create_account from frappe.tests.utils import FrappeTestCase TEST_COMPANY = "Bolt Trades" +def get_bank_parent_account(company: str) -> str: + return frappe.db.get_value("Account", {"account_type": "Bank", "is_group": 1, "company": company}) + + def create_currency_account(currency: str, parent_account: str, account_name: str): - acc = frappe.new_doc("Account") - acc.account_name = account_name - acc.account_currency = currency - acc.parent_account = parent_account - acc.insert(ignore_permissions=True, ignore_mandatory=True, ignore_links=True) - return acc + account = create_account( + account_name=account_name, + account_type="Bank", + parent_account=parent_account, + company=TEST_COMPANY, + account_currency=currency, + ) + return frappe.get_doc("Account", account) def create_bank_account( @@ -25,6 +32,7 @@ def create_bank_account( ba.account_name = account_name ba.account = account ba.bank = "_Test_Bank" + ba.company = TEST_COMPANY ba.is_company_account = 1 if bank_fee_account: ba.bank_fee_account = bank_fee_account @@ -46,7 +54,7 @@ class TestIncludedBankFees(FrappeTestCase): def setUpClass(cls): super().setUpClass() - parent_account = frappe.db.get_value("Account", {"is_group": 1, "company": TEST_COMPANY}) + parent_account = get_bank_parent_account(TEST_COMPANY) cls.account_main = create_currency_account("EUR", parent_account, "_Test_Account_EUR") cls.account_main_without_fee = create_currency_account( "EUR", parent_account, "_Test_Account_EUR_No_Fee" @@ -54,7 +62,8 @@ def setUpClass(cls): cls.account_fee = create_currency_account("EUR", parent_account, "_Test_Account_EUR_Fee") cls.bank_account = create_bank_account(cls.account_main.name, bank_fee_account=cls.account_fee.name) cls.bank_account_without_fee = create_bank_account( - cls.account_main_without_fee.name, account_name="_Test_B_Account_No_Fee" + cls.account_main_without_fee.name, + account_name="_Test_B_Account_No_Fee", ) @patch("banking.overrides.bank_transaction.create_je_bank_fees") @@ -173,6 +182,59 @@ def test_submit_creates_and_reconciles_fee_journal_entry_for_withdrawal(self): self.assertEqual(accounts[self.account_main.name].credit_in_account_currency, 1.0) self.assertEqual(accounts[self.account_fee.name].debit_in_account_currency, 1.0) + def test_submit_creates_fee_journal_entry_without_reconciliation_for_deposit(self): + """Submitting a deposit with an included fee must keep the fee JE out of reconciliation.""" + frappe.db.set_single_value("Banking Settings", "enable_automatic_journal_entries_for_bank_fees", 1) + + bt = create_bank_transaction( + bank_account=self.bank_account.name, + deposit=5.0, + included_fee=1.0, + date=frappe.utils.nowdate(), + currency="EUR", + description="Deposit with included bank fee", + ) + + bt.submit() + bt.reload() + + self.assertEqual(bt.docstatus, 1) + self.assertEqual(bt.status, "Unreconciled") + self.assertFalse(bt.payment_entries) + self.assertEqual(bt.allocated_amount, 0.0) + self.assertEqual(bt.unallocated_amount, 5.0) + + journal_entries = frappe.get_all( + "Journal Entry Account", + filters={ + "reference_type": "Bank Transaction", + "reference_name": bt.name, + }, + pluck="parent", + ) + self.assertEqual(len(journal_entries), 1) + + je = frappe.get_doc("Journal Entry", journal_entries[0]) + self.assertEqual(je.docstatus, 1) + self.assertEqual(je.voucher_type, "Bank Entry") + self.assertTrue(je.is_system_generated) + self.assertEqual(je.cheque_no, bt.name) + self.assertEqual(str(frappe.db.get_value("Journal Entry", je.name, "clearance_date")), str(bt.date)) + self.assertTrue( + frappe.db.exists( + "Journal Entry Account", + { + "parent": je.name, + "reference_type": "Bank Transaction", + "reference_name": bt.name, + }, + ) + ) + + accounts = {row.account: row for row in je.accounts} + self.assertEqual(accounts[self.account_main.name].credit_in_account_currency, 1.0) + self.assertEqual(accounts[self.account_fee.name].debit_in_account_currency, 1.0) + @patch("banking.overrides.bank_transaction.create_je_bank_fees") def test_before_submit_rejects_fee_larger_than_withdrawal(self, mock_create_bank_fees): from banking.overrides.bank_transaction import before_submit @@ -207,32 +269,3 @@ def test_before_submit_allows_missing_deposit_for_withdrawal_with_fee(self, mock before_submit(bt, None) mock_create_bank_fees.assert_called_once() - - @patch("banking.overrides.bank_transaction.create_automatic_journal_entry") - def test_create_je_bank_fees_deposit(self, mock_create_je): - from banking.overrides.bank_transaction import create_je_bank_fees - - mock_create_je.return_value = "JE-TEST-0001" - date = "2025-01-01" - - bt = create_bank_transaction(deposit=5.0, included_fee=1.0, bank_account=self.bank_account.name) - cost_center = frappe.get_cached_value("Company", bt.company, "cost_center") - - create_je_bank_fees(bt, cost_center, date, self.account_main.name, bt.deposit, 0) - - mock_create_je.assert_called_once_with( - company=bt.company, - bank_account=bt.bank_account, - bank_transaction=bt.name, - cost_center=cost_center, - date=date, - account=self.account_main.name, - target_account=self.account_fee.name, - debit=0, - credit=1.0, - ) - self.assertEqual(bt.payment_entries[0].payment_entry, "JE-TEST-0001") - self.assertEqual(bt.payment_entries[0].allocated_amount, 0.0) - self.assertEqual(bt.allocated_amount, 0.0) - self.assertEqual(bt.unallocated_amount, 5.0) - self.assertEqual(bt.status, "Pending") From 2d2235be91597bde7e45984e4c48b0850723d97f Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Sun, 22 Mar 2026 16:01:11 +0100 Subject: [PATCH 12/14] fix: distinguish fee and custom journal entries in bank reconciliation Keep cheque_no-linked custom journal entries hidden from matching while excluding standard fee journal entries only for deposit transactions. This lets unreconciled withdrawal fee journal entries be offered again. --- .../bank_reconciliation_tool_beta.py | 29 ++- .../test_bank_reconciliation_tool_beta.py | 165 ++++++++++++++++++ .../test_bank_transaction_included_fees.py | 4 +- 3 files changed, 192 insertions(+), 6 deletions(-) diff --git a/banking/klarna_kosma_integration/doctype/bank_reconciliation_tool_beta/bank_reconciliation_tool_beta.py b/banking/klarna_kosma_integration/doctype/bank_reconciliation_tool_beta/bank_reconciliation_tool_beta.py index 96889dfa..0fb9f9c3 100644 --- a/banking/klarna_kosma_integration/doctype/bank_reconciliation_tool_beta/bank_reconciliation_tool_beta.py +++ b/banking/klarna_kosma_integration/doctype/bank_reconciliation_tool_beta/bank_reconciliation_tool_beta.py @@ -16,6 +16,7 @@ from frappe.query_builder.functions import Cast, Coalesce, Sum from frappe.utils import cint, flt, sbool from pypika import Order +from pypika.terms import ExistsCriterion from banking.klarna_kosma_integration.doctype.bank_reconciliation_tool_beta.utils import ( amount_rank_condition, @@ -981,10 +982,30 @@ def get_je_matching_query( ) if bank_transaction_name: - # This filter ensures that Journal Entries that have been created - # automatically for the Bank Transaction (e.g. to Cash In Transit) via - # other apps are not offered as matches. - subquery = subquery.where(je.cheque_no != bank_transaction_name) + je_reference = frappe.qb.DocType("Journal Entry Account") + has_bank_transaction_reference = ExistsCriterion( + frappe.qb.from_(je_reference) + .select(je_reference.name) + .where( + (je_reference.parent == je.name) + & (je_reference.reference_type == "Bank Transaction") + & (je_reference.reference_name == bank_transaction_name) + ) + ) + is_auto_fee_journal_entry = (je.is_system_generated == 1) & has_bank_transaction_reference + # Journal Entries that other apps create for the Bank Transaction + # (e.g. to Cash In Transit) are often linked via cheque_no and should + # not be offered as matches. Standard withdrawal fee JEs are excluded + # from this bucket because they should reappear after unreconciliation. + is_cheque_linked_custom_journal_entry = ( + je.cheque_no == bank_transaction_name + ) & ~is_auto_fee_journal_entry + subquery = subquery.where(~is_cheque_linked_custom_journal_entry) + + if common_filters.payment_type == "Receive": + # Standard deposit fee JEs from the built-in bank-fee logic must stay + # hidden even if they are not identified by cheque_no. + subquery = subquery.where(~is_auto_fee_journal_entry) if frappe.flags.auto_reconcile_vouchers: subquery = subquery.where(je.cheque_no == common_filters.reference_no) diff --git a/banking/klarna_kosma_integration/doctype/bank_reconciliation_tool_beta/test_bank_reconciliation_tool_beta.py b/banking/klarna_kosma_integration/doctype/bank_reconciliation_tool_beta/test_bank_reconciliation_tool_beta.py index 4477485d..a3034e59 100644 --- a/banking/klarna_kosma_integration/doctype/bank_reconciliation_tool_beta/test_bank_reconciliation_tool_beta.py +++ b/banking/klarna_kosma_integration/doctype/bank_reconciliation_tool_beta/test_bank_reconciliation_tool_beta.py @@ -683,6 +683,171 @@ def test_split_jv_match_against_transaction(self): self.assertEqual(first_match["name"], journal_entry.name) self.assertEqual(first_match["paid_amount"], 200.0) + def test_auto_generated_fee_journal_entry_is_excluded_for_deposit_matches(self): + """Deposit fee JEs must stay hidden even if they are otherwise query-eligible.""" + frappe.db.set_single_value("Banking Settings", "enable_automatic_journal_entries_for_bank_fees", 1) + + bank = create_bank("Citi Bank Fee Filter", swift_number="CITIUS35") + gl_account = create_bank_gl_account("_Test Fee Filter Bank - _TC") + fee_account = create_bank_gl_account("_Test Fee Filter Offset - _TC") + bank_account = frappe.get_doc( + { + "doctype": "Bank Account", + "account_name": "Fee Filter Account", + "bank": bank.name, + "account": gl_account, + "bank_fee_account": fee_account, + "company": "_Test Company", + "is_company_account": 1, + } + ).insert() + + bt = frappe.get_doc( + { + "doctype": "Bank Transaction", + "company": "_Test Company", + "description": "Deposit with auto-generated fee entry", + "date": getdate(), + "deposit": 5.0, + "included_fee": 1.0, + "currency": "INR", + "bank_account": bank_account.name, + "reference_number": "FEE-FILTER-001", + } + ).insert() + bt.submit() + bt.reload() + + journal_entries = frappe.get_all( + "Journal Entry Account", + filters={ + "reference_type": "Bank Transaction", + "reference_name": bt.name, + }, + pluck="parent", + ) + self.assertEqual(len(journal_entries), 1) + + fee_journal_entry = frappe.get_doc("Journal Entry", journal_entries[0]) + self.assertTrue(fee_journal_entry.is_system_generated) + self.assertEqual(fee_journal_entry.cheque_no, bt.name) + + # Make the JE query-eligible and prove deposit exclusion does not depend on cheque_no. + frappe.db.set_value("Journal Entry", fee_journal_entry.name, "clearance_date", None) + frappe.db.set_value("Journal Entry", fee_journal_entry.name, "cheque_no", "UNRELATED-CHEQUE-NO") + + matched_vouchers = get_linked_payments( + bank_transaction_name=bt.name, + document_types=["journal_entry"], + from_date=add_days(getdate(), -1), + to_date=add_days(getdate(), 1), + ) + matched_names = [voucher["name"] for voucher in matched_vouchers] + + self.assertEqual(matched_names, []) + self.assertNotIn(fee_journal_entry.name, matched_names) + + def test_auto_generated_fee_journal_entry_is_offered_again_after_unreconcile(self): + """Withdrawal fee JEs must be offered again once unreconciled from the BT.""" + frappe.db.set_single_value("Banking Settings", "enable_automatic_journal_entries_for_bank_fees", 1) + + bank = create_bank("Citi Bank Fee Reoffer", swift_number="CITIUS36") + gl_account = create_bank_gl_account("_Test Fee Reoffer Bank - _TC") + fee_account = create_bank_gl_account("_Test Fee Reoffer Offset - _TC") + bank_account = frappe.get_doc( + { + "doctype": "Bank Account", + "account_name": "Fee Reoffer Account", + "bank": bank.name, + "account": gl_account, + "bank_fee_account": fee_account, + "company": "_Test Company", + "is_company_account": 1, + } + ).insert() + + bt = frappe.get_doc( + { + "doctype": "Bank Transaction", + "company": "_Test Company", + "description": "Withdrawal with auto-generated fee entry", + "date": getdate(), + "withdrawal": 5.0, + "included_fee": 1.0, + "currency": "INR", + "bank_account": bank_account.name, + "reference_number": "FEE-FILTER-002", + } + ).insert() + bt.submit() + bt.reload() + + self.assertEqual(len(bt.payment_entries), 1) + fee_journal_entry = frappe.get_doc("Journal Entry", bt.payment_entries[0].payment_entry) + self.assertEqual(str(fee_journal_entry.clearance_date), str(bt.date)) + + bt.remove_payment_entries() + bt.reload() + fee_journal_entry.reload() + + self.assertEqual(bt.status, "Unreconciled") + self.assertFalse(bt.payment_entries) + self.assertIsNone(fee_journal_entry.clearance_date) + + matched_vouchers = get_linked_payments( + bank_transaction_name=bt.name, + document_types=["journal_entry"], + from_date=add_days(getdate(), -1), + to_date=add_days(getdate(), 1), + ) + matched_names = [voucher["name"] for voucher in matched_vouchers] + + self.assertIn(fee_journal_entry.name, matched_names) + self.assertEqual(matched_vouchers[0]["paid_amount"], 1.0) + + def test_cheque_number_linked_journal_entry_is_excluded_from_matches(self): + """Cheque-number-linked custom JEs must stay hidden.""" + bt = create_bank_transaction( + date=getdate(), + withdrawal=200, + bank_account=self.bank_account, + reference_no="CUSTOM-JE-001", + ) + journal_entry = create_journal_entry_bts( + bank_transaction_name=bt.name, + party_type="Customer", + party=self.customer, + posting_date=bt.date, + reference_number=bt.name, + reference_date=bt.date, + entry_type="Bank Entry", + second_account=frappe.db.get_value("Company", bt.company, "default_receivable_account"), + allow_edit=True, + ) + journal_entry.submit() + + self.assertFalse(journal_entry.is_system_generated) + self.assertFalse( + frappe.db.exists( + "Journal Entry Account", + { + "parent": journal_entry.name, + "reference_type": "Bank Transaction", + "reference_name": bt.name, + }, + ) + ) + + matched_vouchers = get_linked_payments( + bank_transaction_name=bt.name, + document_types=["journal_entry"], + from_date=add_days(getdate(), -1), + to_date=add_days(getdate(), 1), + ) + matched_names = [voucher["name"] for voucher in matched_vouchers] + + self.assertNotIn(journal_entry.name, matched_names) + def test_usd_purchase_invoice_paid_in_usd(self): """Reconcile a USD Purchase Invoice via a USD bank account. diff --git a/banking/overrides/test_bank_transaction_included_fees.py b/banking/overrides/test_bank_transaction_included_fees.py index e7b6849a..f1d44807 100644 --- a/banking/overrides/test_bank_transaction_included_fees.py +++ b/banking/overrides/test_bank_transaction_included_fees.py @@ -182,8 +182,8 @@ def test_submit_creates_and_reconciles_fee_journal_entry_for_withdrawal(self): self.assertEqual(accounts[self.account_main.name].credit_in_account_currency, 1.0) self.assertEqual(accounts[self.account_fee.name].debit_in_account_currency, 1.0) - def test_submit_creates_fee_journal_entry_without_reconciliation_for_deposit(self): - """Submitting a deposit with an included fee must keep the fee JE out of reconciliation.""" + def test_submit_creates_fee_journal_entry_for_deposit(self): + """Submitting a deposit with an included fee must create a cleared fee JE without allocation.""" frappe.db.set_single_value("Banking Settings", "enable_automatic_journal_entries_for_bank_fees", 1) bt = create_bank_transaction( From e55a7c1b7d2943408c4a4d62be9f165fee8ffae9 Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Sun, 22 Mar 2026 16:26:45 +0100 Subject: [PATCH 13/14] fix: preserve existing allocations for included bank fees Recompute `allocated_amount` and `unallocated_amount` from `payment_entries` after appending the fee journal entry, so prior allocations are not lost. Add a regression test covering fee entry creation when an allocation already exists. --- banking/overrides/bank_transaction.py | 7 ++-- .../test_bank_transaction_included_fees.py | 42 +++++++++++++++++++ 2 files changed, 46 insertions(+), 3 deletions(-) diff --git a/banking/overrides/bank_transaction.py b/banking/overrides/bank_transaction.py index 29e759d6..d0cb31d1 100644 --- a/banking/overrides/bank_transaction.py +++ b/banking/overrides/bank_transaction.py @@ -177,9 +177,6 @@ def create_je_bank_fees(doc, cost_center, date, account, debit, credit): if credit > 0: # Only adjust withdrawals, because deposits never include the fee in the bank amount. - credit_no_fee = credit - included_fee - doc.allocated_amount = included_fee - doc.unallocated_amount = debit + (credit_no_fee or 0) doc.append( "payment_entries", { @@ -188,6 +185,10 @@ def create_je_bank_fees(doc, cost_center, date, account, debit, credit): "allocated_amount": included_fee, }, ) + # Recompute from the rows so existing allocations are preserved if this helper + # is reused outside the current submit flow. + doc.allocated_amount = sum(flt(entry.allocated_amount) for entry in doc.payment_entries) + doc.unallocated_amount = abs(flt(doc.withdrawal) - flt(doc.deposit)) - doc.allocated_amount # Deposit fees are linked via the Journal Entry reference only and must stay # out of the reconciliation table. diff --git a/banking/overrides/test_bank_transaction_included_fees.py b/banking/overrides/test_bank_transaction_included_fees.py index f1d44807..2285ecc5 100644 --- a/banking/overrides/test_bank_transaction_included_fees.py +++ b/banking/overrides/test_bank_transaction_included_fees.py @@ -182,6 +182,48 @@ def test_submit_creates_and_reconciles_fee_journal_entry_for_withdrawal(self): self.assertEqual(accounts[self.account_main.name].credit_in_account_currency, 1.0) self.assertEqual(accounts[self.account_fee.name].debit_in_account_currency, 1.0) + @patch( + "banking.overrides.bank_transaction.create_automatic_journal_entry", + return_value="ACC-JV-TEST-FEE", + ) + def test_create_je_bank_fees_preserves_existing_allocations(self, mock_create_automatic_journal_entry): + from banking.overrides.bank_transaction import create_je_bank_fees + + bt = create_bank_transaction( + insert=False, + bank_account=self.bank_account.name, + withdrawal=10.0, + included_fee=1.0, + date="2025-01-01", + ) + bt.append( + "payment_entries", + { + "payment_document": "Payment Entry", + "payment_entry": "ACC-PE-EXISTING", + "allocated_amount": 3.0, + }, + ) + bt.allocated_amount = 0.0 + bt.unallocated_amount = 10.0 + + create_je_bank_fees( + bt, + cost_center=frappe.get_cached_value("Company", TEST_COMPANY, "cost_center"), + date="2025-01-01", + account=self.account_main.name, + debit=0.0, + credit=10.0, + ) + + mock_create_automatic_journal_entry.assert_called_once() + self.assertEqual(len(bt.payment_entries), 2) + self.assertEqual(bt.payment_entries[-1].payment_document, "Journal Entry") + self.assertEqual(bt.payment_entries[-1].payment_entry, "ACC-JV-TEST-FEE") + self.assertEqual(bt.payment_entries[-1].allocated_amount, 1.0) + self.assertEqual(bt.allocated_amount, 4.0) + self.assertEqual(bt.unallocated_amount, 6.0) + def test_submit_creates_fee_journal_entry_for_deposit(self): """Submitting a deposit with an included fee must create a cleared fee JE without allocation.""" frappe.db.set_single_value("Banking Settings", "enable_automatic_journal_entries_for_bank_fees", 1) From fd3ac94f4f6162258c1f1430f1956f10815b48b5 Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Sun, 22 Mar 2026 16:42:03 +0100 Subject: [PATCH 14/14] refactor: centralize common testing utils --- .../banking_settings/test_banking_settings.py | 43 +++------- banking/overrides/test_bank_account.py | 48 +++-------- .../test_bank_transaction_included_fees.py | 80 +++++------------- .../test_bank_transaction_positive_values.py | 29 ++++--- banking/testing_utils.py | 82 +++++++++++++++++++ 5 files changed, 142 insertions(+), 140 deletions(-) create mode 100644 banking/testing_utils.py diff --git a/banking/klarna_kosma_integration/doctype/banking_settings/test_banking_settings.py b/banking/klarna_kosma_integration/doctype/banking_settings/test_banking_settings.py index 9cbf89a2..8cf0430b 100644 --- a/banking/klarna_kosma_integration/doctype/banking_settings/test_banking_settings.py +++ b/banking/klarna_kosma_integration/doctype/banking_settings/test_banking_settings.py @@ -6,49 +6,26 @@ from datetime import date import frappe -from erpnext.accounts.doctype.account.test_account import create_account from frappe.tests.utils import FrappeTestCase from banking.klarna_kosma_integration.doctype.banking_settings.banking_settings import ( successful_request_exists, ) - -TEST_COMPANY = "Bolt Trades" - - -def get_bank_parent_account(company: str) -> str: - return frappe.db.get_value("Account", {"account_type": "Bank", "is_group": 1, "company": company}) - - -def create_currency_account(currency: str, parent_account: str, account_name: str): - account = create_account( - account_name=account_name, - account_type="Bank", - parent_account=parent_account, - company=TEST_COMPANY, - account_currency=currency, - ) - return frappe.get_doc("Account", account) - - -def create_bank_account(account: str, account_name: str): - ba = frappe.new_doc("Bank Account") - ba.account_name = account_name - ba.account = account - ba.bank = "_Test_Bank" - ba.company = TEST_COMPANY - ba.is_company_account = 1 - ba.insert(ignore_permissions=True, ignore_links=True) - return ba +from banking.testing_utils import ( + create_bank_account, + create_currency_account, + get_bank_parent_account, + set_automatic_bank_fee_entries, +) class TestBankingSettings(FrappeTestCase): @classmethod def setUpClass(cls): super().setUpClass() - frappe.db.set_single_value("Banking Settings", "enable_automatic_journal_entries_for_bank_fees", 0) + set_automatic_bank_fee_entries(False) - parent_account = get_bank_parent_account(TEST_COMPANY) + parent_account = get_bank_parent_account() cls.account_without_fee = create_currency_account( "EUR", parent_account, "_Test_Banking_Settings_Account_No_Fee" ) @@ -58,10 +35,10 @@ def setUpClass(cls): def setUp(self): super().setUp() - frappe.db.set_single_value("Banking Settings", "enable_automatic_journal_entries_for_bank_fees", 0) + set_automatic_bank_fee_entries(False) def tearDown(self): - frappe.db.set_single_value("Banking Settings", "enable_automatic_journal_entries_for_bank_fees", 0) + set_automatic_bank_fee_entries(False) super().tearDown() def test_successful_request_exists(self): diff --git a/banking/overrides/test_bank_account.py b/banking/overrides/test_bank_account.py index afafb5a4..24fed7c7 100644 --- a/banking/overrides/test_bank_account.py +++ b/banking/overrides/test_bank_account.py @@ -2,49 +2,23 @@ # See license.txt import frappe -from erpnext.accounts.doctype.account.test_account import create_account from frappe.tests.utils import FrappeTestCase -TEST_COMPANY = "Bolt Trades" - - -def get_bank_parent_account(company: str) -> str: - return frappe.db.get_value("Account", {"account_type": "Bank", "is_group": 1, "company": company}) - - -def create_currency_account(currency: str, parent_account: str, account_name: str): - account = create_account( - account_name=account_name, - account_type="Bank", - parent_account=parent_account, - company=TEST_COMPANY, - account_currency=currency, - ) - return frappe.get_doc("Account", account) - - -def create_bank_account( - account: str, bank_fee_account: str | None = None, account_name: str = "_Test_B_Account" -): - ba = frappe.new_doc("Bank Account") - ba.account_name = account_name - ba.account = account - ba.bank = "_Test_Bank" - ba.company = TEST_COMPANY - ba.is_company_account = 1 - if bank_fee_account: - ba.bank_fee_account = bank_fee_account - ba.insert(ignore_permissions=True, ignore_links=True) - return ba +from banking.testing_utils import ( + create_bank_account, + create_currency_account, + get_bank_parent_account, + set_automatic_bank_fee_entries, +) class TestBankAccount(FrappeTestCase): @classmethod def setUpClass(cls): super().setUpClass() - frappe.db.set_single_value("Banking Settings", "enable_automatic_journal_entries_for_bank_fees", 0) + set_automatic_bank_fee_entries(False) - parent_account = get_bank_parent_account(TEST_COMPANY) + parent_account = get_bank_parent_account() cls.account_eur = create_currency_account("EUR", parent_account, "_Test_Account_EUR") cls.account_usd = create_currency_account("USD", parent_account, "_Test_Account_USD") cls.account_fee = create_currency_account("EUR", parent_account, "_Test_Account_EUR_Fee") @@ -56,10 +30,10 @@ def setUpClass(cls): def setUp(self): super().setUp() - frappe.db.set_single_value("Banking Settings", "enable_automatic_journal_entries_for_bank_fees", 0) + set_automatic_bank_fee_entries(False) def tearDown(self): - frappe.db.set_single_value("Banking Settings", "enable_automatic_journal_entries_for_bank_fees", 0) + set_automatic_bank_fee_entries(False) super().tearDown() def test_currency_mismatch_rejected(self): @@ -69,7 +43,7 @@ def test_currency_mismatch_rejected(self): def test_requires_fee_account_when_enabled(self): """Company bank accounts cannot be created or saved without a fee account.""" - frappe.db.set_single_value("Banking Settings", "enable_automatic_journal_entries_for_bank_fees", 1) + set_automatic_bank_fee_entries(True) with self.assertRaises(frappe.ValidationError): create_bank_account(self.account_eur.name, account_name="_Test_B_Account_Without_Fee") diff --git a/banking/overrides/test_bank_transaction_included_fees.py b/banking/overrides/test_bank_transaction_included_fees.py index 2285ecc5..3d7ff6c7 100644 --- a/banking/overrides/test_bank_transaction_included_fees.py +++ b/banking/overrides/test_bank_transaction_included_fees.py @@ -4,49 +4,17 @@ from unittest.mock import patch import frappe -from erpnext.accounts.doctype.account.test_account import create_account from frappe.tests.utils import FrappeTestCase -TEST_COMPANY = "Bolt Trades" - - -def get_bank_parent_account(company: str) -> str: - return frappe.db.get_value("Account", {"account_type": "Bank", "is_group": 1, "company": company}) - - -def create_currency_account(currency: str, parent_account: str, account_name: str): - account = create_account( - account_name=account_name, - account_type="Bank", - parent_account=parent_account, - company=TEST_COMPANY, - account_currency=currency, - ) - return frappe.get_doc("Account", account) - - -def create_bank_account( - account: str, bank_fee_account: str | None = None, account_name: str = "_Test_B_Account" -): - ba = frappe.new_doc("Bank Account") - ba.account_name = account_name - ba.account = account - ba.bank = "_Test_Bank" - ba.company = TEST_COMPANY - ba.is_company_account = 1 - if bank_fee_account: - ba.bank_fee_account = bank_fee_account - ba.insert(ignore_permissions=True, ignore_links=True) - return ba - - -def create_bank_transaction(insert=True, **values): - doc = frappe.new_doc("Bank Transaction") - doc.company = TEST_COMPANY - doc.update(values) - if insert: - doc.insert(ignore_permissions=True, ignore_mandatory=True, ignore_links=True) - return doc +from banking.testing_utils import ( + TEST_COMPANY, + create_bank_account, + create_bank_transaction, + create_currency_account, + get_bank_parent_account, + make_bank_transaction, + set_automatic_bank_fee_entries, +) class TestIncludedBankFees(FrappeTestCase): @@ -54,7 +22,7 @@ class TestIncludedBankFees(FrappeTestCase): def setUpClass(cls): super().setUpClass() - parent_account = get_bank_parent_account(TEST_COMPANY) + parent_account = get_bank_parent_account() cls.account_main = create_currency_account("EUR", parent_account, "_Test_Account_EUR") cls.account_main_without_fee = create_currency_account( "EUR", parent_account, "_Test_Account_EUR_No_Fee" @@ -70,7 +38,7 @@ def setUpClass(cls): def test_before_submit(self, mock_create_bank_fees): from banking.overrides.bank_transaction import before_submit - bt = create_bank_transaction(insert=False) + bt = make_bank_transaction() with self.assertRaises(frappe.ValidationError): before_submit(bt, None) @@ -86,7 +54,7 @@ def test_before_submit(self, mock_create_bank_fees): before_submit(bt, None) bt.withdrawal = 1.0 - frappe.db.set_single_value("Banking Settings", "enable_automatic_journal_entries_for_bank_fees", 1) + set_automatic_bank_fee_entries(True) before_submit(bt, None) mock_create_bank_fees.assert_called_once() @@ -94,9 +62,8 @@ def test_before_submit(self, mock_create_bank_fees): def test_before_submit_with_disabled_automatic_fee_entries(self, mock_create_bank_fees): from banking.overrides.bank_transaction import before_submit - frappe.db.set_single_value("Banking Settings", "enable_automatic_journal_entries_for_bank_fees", 0) - bt = create_bank_transaction( - insert=False, + set_automatic_bank_fee_entries(False) + bt = make_bank_transaction( bank_account=self.bank_account.name, withdrawal=10.0, included_fee=1.0, @@ -111,7 +78,7 @@ def test_submit_without_fee_account_when_automatic_fee_entries_are_disabled(self """Ensure a Bank Transaction can be submitted without a bank fee account when automatic bank fee journal entries are disabled, and assert that no linked fee Journal Entry is created.""" - frappe.db.set_single_value("Banking Settings", "enable_automatic_journal_entries_for_bank_fees", 0) + set_automatic_bank_fee_entries(False) bt = create_bank_transaction( bank_account=self.bank_account_without_fee.name, @@ -140,7 +107,7 @@ def test_submit_without_fee_account_when_automatic_fee_entries_are_disabled(self def test_submit_creates_and_reconciles_fee_journal_entry_for_withdrawal(self): """Submitting a withdrawal with an included fee must create and reconcile the fee JE.""" - frappe.db.set_single_value("Banking Settings", "enable_automatic_journal_entries_for_bank_fees", 1) + set_automatic_bank_fee_entries(True) bt = create_bank_transaction( bank_account=self.bank_account.name, @@ -189,8 +156,7 @@ def test_submit_creates_and_reconciles_fee_journal_entry_for_withdrawal(self): def test_create_je_bank_fees_preserves_existing_allocations(self, mock_create_automatic_journal_entry): from banking.overrides.bank_transaction import create_je_bank_fees - bt = create_bank_transaction( - insert=False, + bt = make_bank_transaction( bank_account=self.bank_account.name, withdrawal=10.0, included_fee=1.0, @@ -226,7 +192,7 @@ def test_create_je_bank_fees_preserves_existing_allocations(self, mock_create_au def test_submit_creates_fee_journal_entry_for_deposit(self): """Submitting a deposit with an included fee must create a cleared fee JE without allocation.""" - frappe.db.set_single_value("Banking Settings", "enable_automatic_journal_entries_for_bank_fees", 1) + set_automatic_bank_fee_entries(True) bt = create_bank_transaction( bank_account=self.bank_account.name, @@ -281,9 +247,8 @@ def test_submit_creates_fee_journal_entry_for_deposit(self): def test_before_submit_rejects_fee_larger_than_withdrawal(self, mock_create_bank_fees): from banking.overrides.bank_transaction import before_submit - frappe.db.set_single_value("Banking Settings", "enable_automatic_journal_entries_for_bank_fees", 1) - bt = create_bank_transaction( - insert=False, + set_automatic_bank_fee_entries(True) + bt = make_bank_transaction( bank_account=self.bank_account.name, withdrawal=1.0, included_fee=2.0, @@ -299,9 +264,8 @@ def test_before_submit_rejects_fee_larger_than_withdrawal(self, mock_create_bank def test_before_submit_allows_missing_deposit_for_withdrawal_with_fee(self, mock_create_bank_fees): from banking.overrides.bank_transaction import before_submit - frappe.db.set_single_value("Banking Settings", "enable_automatic_journal_entries_for_bank_fees", 1) - bt = create_bank_transaction( - insert=False, + set_automatic_bank_fee_entries(True) + bt = make_bank_transaction( bank_account=self.bank_account.name, withdrawal=2.0, included_fee=1.0, diff --git a/banking/overrides/test_bank_transaction_positive_values.py b/banking/overrides/test_bank_transaction_positive_values.py index 5f118d30..4cb39caa 100644 --- a/banking/overrides/test_bank_transaction_positive_values.py +++ b/banking/overrides/test_bank_transaction_positive_values.py @@ -1,19 +1,16 @@ # Copyright (c) 2025, ALYF GmbH and Contributors # See license.txt -import frappe from frappe.tests.utils import FrappeTestCase +from banking.testing_utils import make_bank_transaction -class TestEnforcePositiveValues(FrappeTestCase): - def make_doc_and_run_before_validate(self, **kwargs): - doc = frappe.new_doc("Bank Transaction") - doc.update(kwargs) - doc.run_method("before_validate") - return doc +class TestEnforcePositiveValues(FrappeTestCase): def test_deposit_values_are_normalized(self): - doc = self.make_doc_and_run_before_validate( + doc = make_bank_transaction( + company=None, + run_before_validate=True, deposit=-2.0, withdrawal=0.0, included_fee=-1.0, @@ -27,7 +24,9 @@ def test_deposit_values_are_normalized(self): self.assertEqual(doc.unallocated_amount, 1.0) def test_withdrawal_values_are_normalized(self): - doc = self.make_doc_and_run_before_validate( + doc = make_bank_transaction( + company=None, + run_before_validate=True, deposit=0.0, withdrawal=-1.0, included_fee=-1.0, @@ -41,7 +40,9 @@ def test_withdrawal_values_are_normalized(self): self.assertEqual(doc.unallocated_amount, 2.0) def test_negative_withdrawal_with_positive_excluded_fee(self): - doc = self.make_doc_and_run_before_validate( + doc = make_bank_transaction( + company=None, + run_before_validate=True, deposit=0.0, withdrawal=-10.0, included_fee=0.0, @@ -55,7 +56,9 @@ def test_negative_withdrawal_with_positive_excluded_fee(self): self.assertEqual(doc.unallocated_amount, 11.0) def test_negative_deposit_with_positive_excluded_fee(self): - doc = self.make_doc_and_run_before_validate( + doc = make_bank_transaction( + company=None, + run_before_validate=True, deposit=-10.0, withdrawal=0.0, included_fee=0.0, @@ -69,7 +72,9 @@ def test_negative_deposit_with_positive_excluded_fee(self): self.assertEqual(doc.unallocated_amount, 9.0) def test_none_values_are_left_untouched(self): - doc = self.make_doc_and_run_before_validate( + doc = make_bank_transaction( + company=None, + run_before_validate=True, deposit=None, withdrawal=None, included_fee=None, diff --git a/banking/testing_utils.py b/banking/testing_utils.py new file mode 100644 index 00000000..45e5a03b --- /dev/null +++ b/banking/testing_utils.py @@ -0,0 +1,82 @@ +# Copyright (c) 2025, ALYF GmbH and Contributors +# See license.txt + +import frappe +from erpnext.accounts.doctype.account.test_account import create_account + +TEST_COMPANY = "Bolt Trades" + + +def set_automatic_bank_fee_entries(enabled: bool) -> None: + frappe.db.set_single_value( + "Banking Settings", + "enable_automatic_journal_entries_for_bank_fees", + int(enabled), + ) + + +def get_bank_parent_account(company: str = TEST_COMPANY) -> str: + return frappe.db.get_value("Account", {"account_type": "Bank", "is_group": 1, "company": company}) + + +def create_currency_account( + currency: str, parent_account: str, account_name: str, company: str = TEST_COMPANY +): + account = create_account( + account_name=account_name, + account_type="Bank", + parent_account=parent_account, + company=company, + account_currency=currency, + ) + return frappe.get_doc("Account", account) + + +def create_bank_account( + account: str, + account_name: str = "_Test_B_Account", + *, + bank_fee_account: str | None = None, + company: str = TEST_COMPANY, + bank: str = "_Test_Bank", +): + bank_account = frappe.new_doc("Bank Account") + bank_account.account_name = account_name + bank_account.account = account + bank_account.bank = bank + bank_account.company = company + bank_account.is_company_account = 1 + if bank_fee_account: + bank_account.bank_fee_account = bank_fee_account + bank_account.insert(ignore_permissions=True, ignore_links=True) + return bank_account + + +def make_bank_transaction( + *, + company: str | None = TEST_COMPANY, + run_before_validate: bool = False, + **values, +): + doc = frappe.new_doc("Bank Transaction") + if company is not None: + doc.company = company + doc.update(values) + if run_before_validate: + doc.run_method("before_validate") + return doc + + +def create_bank_transaction( + *, + company: str = TEST_COMPANY, + run_before_validate: bool = False, + **values, +): + doc = make_bank_transaction( + company=company, + run_before_validate=run_before_validate, + **values, + ) + doc.insert(ignore_permissions=True, ignore_mandatory=True, ignore_links=True) + return doc