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..150c2dec 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,11 @@ doc_events = { "Bank Transaction": { "on_update_after_submit": "banking.overrides.bank_transaction.on_update_after_submit", + "before_submit": "banking.overrides.bank_transaction.before_submit", }, "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/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/klarna_kosma_integration/doctype/banking_settings/banking_settings.json b/banking/klarna_kosma_integration/doctype/banking_settings/banking_settings.json index 102ab36a..e118310c 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,13 +116,20 @@ "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, "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", 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..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): @@ -33,6 +34,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 @@ -41,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..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 @@ -11,14 +11,55 @@ from banking.klarna_kosma_integration.doctype.banking_settings.banking_settings import ( successful_request_exists, ) +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() + set_automatic_bank_fee_entries(False) + + parent_account = get_bank_parent_account() + 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() + set_automatic_bank_fee_entries(False) + + def tearDown(self): + set_automatic_bank_fee_entries(False) + 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 6b856435..1c6e1efe 100644 --- a/banking/overrides/bank_account.py +++ b/banking/overrides/bank_account.py @@ -1,4 +1,47 @@ +import frappe +from frappe import _ +from frappe.utils import cint + + 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) + validate_required_bank_fee_account(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!")) + + +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/bank_transaction.py b/banking/overrides/bank_transaction.py index 0aa765f7..d0cb31d1 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): @@ -105,3 +106,142 @@ 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)) + ) + ) + + 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) + + 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 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. + doc.append( + "payment_entries", + { + "payment_document": "Journal Entry", + "payment_entry": je_fee_name, + "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. + + 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 = "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 + + journal_entry.append( + "accounts", + { + "account": account, + "bank_account": bank_account, + "debit_in_account_currency": debit, + "credit_in_account_currency": credit, + "cost_center": cost_center, + "reference_type": "Bank Transaction", + "reference_name": bank_transaction, + }, + ) + + 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", date) + + 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..24fed7c7 --- /dev/null +++ b/banking/overrides/test_bank_account.py @@ -0,0 +1,60 @@ +# Copyright (c) 2025, ALYF GmbH and Contributors +# See license.txt + +import frappe +from frappe.tests.utils import FrappeTestCase + +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() + set_automatic_bank_fee_entries(False) + + 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") + 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 setUp(self): + super().setUp() + set_automatic_bank_fee_entries(False) + + def tearDown(self): + set_automatic_bank_fee_entries(False) + 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.""" + 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") + + 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, + ) 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..3d7ff6c7 --- /dev/null +++ b/banking/overrides/test_bank_transaction_included_fees.py @@ -0,0 +1,277 @@ +# Copyright (c) 2025, ALYF GmbH and Contributors +# See license.txt + +from unittest.mock import patch + +import frappe +from frappe.tests.utils import FrappeTestCase + +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): + @classmethod + def setUpClass(cls): + super().setUpClass() + + 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" + ) + 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): + from banking.overrides.bank_transaction import before_submit + + bt = make_bank_transaction() + + 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 + set_automatic_bank_fee_entries(True) + 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 + + set_automatic_bank_fee_entries(False) + bt = make_bank_transaction( + 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() + + 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.""" + set_automatic_bank_fee_entries(False) + + 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, + }, + ) + ) + + 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.""" + set_automatic_bank_fee_entries(True) + + 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_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 = make_bank_transaction( + 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.""" + set_automatic_bank_fee_entries(True) + + 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 + + set_automatic_bank_fee_entries(True) + bt = make_bank_transaction( + 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 + + set_automatic_bank_fee_entries(True) + bt = make_bank_transaction( + 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() 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/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] 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 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