mirror of
https://github.com/frappe/erpnext.git
synced 2026-08-26 21:35:19 +00:00
feat(accounts): add Bank Charges account for Payment Entry deductions (#57840)
* feat(accounts): add Bank Charges account for Payment Entry deductions Add an optional Bank Charges Account field on Company. When a Payment Entry has a difference between the paid and received amount (e.g. a same-currency Internal Transfer where the bank deducted a fee), that amount now books to the Bank Charges account in the deductions table instead of always going to the Exchange Gain/Loss account. Left blank, behavior is unchanged. Mirrors the resolution on both the server (set_exchange_gain_loss) and client (set_exchange_gain_loss_deduction) so the deduction row is pre-filled consistently before and after save. A user's manual account edit on an existing deduction row is preserved across recalculation, same as before this change. * fix(accounts): only route Payment Entry difference to Bank Charges for same-currency transfers Cross-currency Payment Entries were also matching the unconditional bank_charges_account precedence, misrouting a genuine exchange gain/loss into the Bank Charges account. Only prefer Bank Charges Account when paid_from and paid_to share a currency; cross-currency differences continue to book to Exchange Gain/Loss Account. * test(payment_entry): assert against actual exchange gain/loss account, not a hardcoded name CI failed: _Test Company's exchange_gain_loss_account is auto-provisioned as "Exchange Gain/Loss - _TC" by the standard chart of accounts, not the "_Test Exchange Gain/Loss - _TC" account used only by a sibling test. * fix(accounts): auto-set Bank Charges Account from chart of accounts default The standard chart of accounts already ships a "Bank Charges" ledger account, but set_default_accounts() never picked it up into the Company's bank_charges_account field, unlike its write_off_account and exchange_gain_loss_account siblings. New and existing companies now get it auto-populated the same way. --------- Co-authored-by: test <test@test.com>
This commit is contained in:
@@ -727,6 +727,7 @@ def get_company_default_account_fields():
|
||||
"stock_delivered_but_not_billed": "Stock Delivered But Not Billed Account",
|
||||
"stock_adjustment_account": "Stock Adjustment Account",
|
||||
"write_off_account": "Write Off Account",
|
||||
"bank_charges_account": "Bank Charges Account",
|
||||
"default_discount_account": "Default Payment Discount Account",
|
||||
"unrealized_profit_loss_account": "Unrealized Profit / Loss Account",
|
||||
"exchange_gain_loss_account": "Exchange Gain / Loss Account",
|
||||
|
||||
@@ -1292,7 +1292,10 @@ frappe.ui.form.on("Payment Entry", {
|
||||
|
||||
if (!row) {
|
||||
const company_defaults = frappe.get_doc(":Company", frm.doc.company);
|
||||
const is_single_currency =
|
||||
frm.doc.paid_from_account_currency === frm.doc.paid_to_account_currency;
|
||||
const account =
|
||||
(is_single_currency && company_defaults?.bank_charges_account) ||
|
||||
company_defaults?.[account_fieldname] ||
|
||||
(await prompt_for_missing_account(frm, account_fieldname));
|
||||
|
||||
@@ -1847,7 +1850,7 @@ frappe.ui.form.on("Payment Entry Deduction", {
|
||||
before_deductions_remove: function (doc, cdt, cdn) {
|
||||
const row = frappe.get_doc(cdt, cdn);
|
||||
if (row.is_exchange_gain_loss && row.amount) {
|
||||
frappe.throw(__("Cannot delete Exchange Gain/Loss row"));
|
||||
frappe.throw(__("Cannot delete a system-generated deduction row"));
|
||||
}
|
||||
},
|
||||
|
||||
|
||||
@@ -1135,10 +1135,18 @@ class PaymentEntry(AccountsController):
|
||||
|
||||
if not exchange_gain_loss_row:
|
||||
values = frappe.get_cached_value(
|
||||
"Company", self.company, ("exchange_gain_loss_account", "cost_center"), as_dict=True
|
||||
"Company",
|
||||
self.company,
|
||||
("bank_charges_account", "exchange_gain_loss_account", "cost_center"),
|
||||
as_dict=True,
|
||||
)
|
||||
is_single_currency = self.paid_from_account_currency == self.paid_to_account_currency
|
||||
account = (
|
||||
is_single_currency and values.bank_charges_account
|
||||
) or values.exchange_gain_loss_account
|
||||
|
||||
for fieldname, value in values.items():
|
||||
missing_fields = {"exchange_gain_loss_account": account, "cost_center": values.cost_center}
|
||||
for fieldname, value in missing_fields.items():
|
||||
if value:
|
||||
continue
|
||||
|
||||
@@ -1155,7 +1163,7 @@ class PaymentEntry(AccountsController):
|
||||
exchange_gain_loss_row = self.append(
|
||||
"deductions",
|
||||
{
|
||||
"account": values.exchange_gain_loss_account,
|
||||
"account": account,
|
||||
"cost_center": values.cost_center,
|
||||
"is_exchange_gain_loss": 1,
|
||||
},
|
||||
|
||||
@@ -782,6 +782,94 @@ class TestPaymentEntry(ERPNextTestSuite):
|
||||
|
||||
self.validate_gl_entries(pe.name, expected_gle)
|
||||
|
||||
def test_bank_charges_deduction(self):
|
||||
bank_charges_account = create_account(
|
||||
parent_account="Indirect Expenses - _TC",
|
||||
account_name="_Test Bank Charges",
|
||||
company="_Test Company",
|
||||
)
|
||||
frappe.db.set_value("Company", "_Test Company", "bank_charges_account", bank_charges_account)
|
||||
self.addCleanup(frappe.db.set_value, "Company", "_Test Company", "bank_charges_account", "")
|
||||
|
||||
pe = frappe.new_doc("Payment Entry")
|
||||
pe.payment_type = "Internal Transfer"
|
||||
pe.company = "_Test Company"
|
||||
pe.paid_from = "_Test Bank - _TC"
|
||||
pe.paid_to = "_Test Cash - _TC"
|
||||
pe.paid_amount = 1000
|
||||
pe.received_amount = 990
|
||||
pe.reference_no = "4"
|
||||
pe.reference_date = nowdate()
|
||||
|
||||
pe.setup_party_account_field()
|
||||
pe.set_missing_values()
|
||||
pe.set_exchange_rate()
|
||||
pe.set_amounts()
|
||||
|
||||
self.assertEqual(pe.deductions[0].account, bank_charges_account)
|
||||
self.assertEqual(pe.deductions[0].amount, 10)
|
||||
pe.deductions[0].cost_center = "_Test Cost Center - _TC"
|
||||
|
||||
pe.insert()
|
||||
pe.submit()
|
||||
|
||||
expected_gle = dict(
|
||||
(d[0], d)
|
||||
for d in [
|
||||
["_Test Bank - _TC", 0, 1000, None],
|
||||
["_Test Cash - _TC", 990, 0, None],
|
||||
[bank_charges_account, 10, 0, None],
|
||||
]
|
||||
)
|
||||
|
||||
self.validate_gl_entries(pe.name, expected_gle)
|
||||
|
||||
def test_cross_currency_transfer_ignores_bank_charges_account(self):
|
||||
exchange_gain_loss_account = frappe.db.get_value(
|
||||
"Company", "_Test Company", "exchange_gain_loss_account"
|
||||
)
|
||||
bank_charges_account = create_account(
|
||||
parent_account="Indirect Expenses - _TC",
|
||||
account_name="_Test Bank Charges",
|
||||
company="_Test Company",
|
||||
)
|
||||
frappe.db.set_value("Company", "_Test Company", "bank_charges_account", bank_charges_account)
|
||||
self.addCleanup(frappe.db.set_value, "Company", "_Test Company", "bank_charges_account", "")
|
||||
|
||||
pe = frappe.new_doc("Payment Entry")
|
||||
pe.payment_type = "Internal Transfer"
|
||||
pe.company = "_Test Company"
|
||||
pe.paid_from = "_Test Bank USD - _TC"
|
||||
pe.paid_to = "_Test Bank - _TC"
|
||||
pe.paid_amount = 100
|
||||
pe.source_exchange_rate = 50
|
||||
pe.received_amount = 4500
|
||||
pe.reference_no = "5"
|
||||
pe.reference_date = nowdate()
|
||||
|
||||
pe.setup_party_account_field()
|
||||
pe.set_missing_values()
|
||||
pe.set_exchange_rate()
|
||||
pe.set_amounts()
|
||||
|
||||
self.assertEqual(pe.deductions[0].account, exchange_gain_loss_account)
|
||||
self.assertEqual(pe.deductions[0].amount, 500)
|
||||
pe.deductions[0].cost_center = "_Test Cost Center - _TC"
|
||||
|
||||
pe.insert()
|
||||
pe.submit()
|
||||
|
||||
expected_gle = dict(
|
||||
(d[0], d)
|
||||
for d in [
|
||||
["_Test Bank USD - _TC", 0, 5000, None],
|
||||
["_Test Bank - _TC", 4500, 0, None],
|
||||
[exchange_gain_loss_account, 500.0, 0, None],
|
||||
]
|
||||
)
|
||||
|
||||
self.validate_gl_entries(pe.name, expected_gle)
|
||||
|
||||
def test_payment_against_negative_sales_invoice(self):
|
||||
si1 = create_sales_invoice()
|
||||
|
||||
|
||||
@@ -53,7 +53,7 @@
|
||||
"depends_on": "eval:doc.is_exchange_gain_loss",
|
||||
"fieldname": "is_exchange_gain_loss",
|
||||
"fieldtype": "Check",
|
||||
"label": "Is Exchange Gain / Loss?",
|
||||
"label": "System Generated",
|
||||
"read_only": 1
|
||||
}
|
||||
],
|
||||
|
||||
@@ -303,6 +303,7 @@ erpnext.company.setup_queries = function (frm) {
|
||||
["round_off_account", { root_type: ["in", ["Expense", "Income"]] }],
|
||||
["round_off_for_opening", { root_type: "Liability", account_type: "Round Off for Opening" }],
|
||||
["write_off_account", { root_type: "Expense" }],
|
||||
["bank_charges_account", { root_type: "Expense" }],
|
||||
["default_deferred_expense_account", {}],
|
||||
["default_deferred_revenue_account", {}],
|
||||
["default_discount_account", {}],
|
||||
|
||||
@@ -54,6 +54,7 @@
|
||||
"default_receivable_account",
|
||||
"default_payable_account",
|
||||
"write_off_account",
|
||||
"bank_charges_account",
|
||||
"unrealized_profit_loss_account",
|
||||
"column_break0",
|
||||
"allow_account_creation_against_child_company",
|
||||
@@ -390,6 +391,15 @@
|
||||
"no_copy": 1,
|
||||
"options": "Account"
|
||||
},
|
||||
{
|
||||
"depends_on": "eval:!doc.__islocal",
|
||||
"fieldname": "bank_charges_account",
|
||||
"fieldtype": "Link",
|
||||
"ignore_user_permissions": 1,
|
||||
"label": "Bank Charges Account",
|
||||
"no_copy": 1,
|
||||
"options": "Account"
|
||||
},
|
||||
{
|
||||
"depends_on": "eval:!doc.__islocal",
|
||||
"fieldname": "exchange_gain_loss_account",
|
||||
|
||||
@@ -49,6 +49,7 @@ class Company(NestedSet):
|
||||
asset_received_but_not_billed: DF.Link | None
|
||||
auto_err_frequency: DF.Literal["Daily", "Weekly", "Monthly"]
|
||||
auto_exchange_rate_revaluation: DF.Check
|
||||
bank_charges_account: DF.Link | None
|
||||
book_advance_payments_in_separate_party_account: DF.Check
|
||||
capital_work_in_progress_account: DF.Link | None
|
||||
chart_of_accounts: DF.Literal[None]
|
||||
@@ -368,6 +369,7 @@ class Company(NestedSet):
|
||||
["Stock Delivered But Not Billed Account", "stock_delivered_but_not_billed"],
|
||||
["Stock Adjustment Account", "stock_adjustment_account"],
|
||||
["Write Off Account", "write_off_account"],
|
||||
["Bank Charges Account", "bank_charges_account"],
|
||||
["Default Payment Discount Account", "default_discount_account"],
|
||||
["Unrealized Profit / Loss Account", "unrealized_profit_loss_account"],
|
||||
["Exchange Gain / Loss Account", "exchange_gain_loss_account"],
|
||||
@@ -789,6 +791,13 @@ class Company(NestedSet):
|
||||
|
||||
self.db_set("write_off_account", write_off_acct)
|
||||
|
||||
if not self.bank_charges_account:
|
||||
bank_charges_acct = frappe.db.get_value(
|
||||
"Account", {"account_name": _("Bank Charges"), "company": self.name, "is_group": 0}
|
||||
)
|
||||
|
||||
self.db_set("bank_charges_account", bank_charges_acct)
|
||||
|
||||
if not self.exchange_gain_loss_account:
|
||||
exchange_gain_loss_acct = frappe.db.get_value(
|
||||
"Account", {"account_name": _("Exchange Gain/Loss"), "company": self.name, "is_group": 0}
|
||||
|
||||
@@ -53,6 +53,7 @@ def boot_session(bootinfo):
|
||||
"enable_perpetual_inventory",
|
||||
"country",
|
||||
"exchange_gain_loss_account",
|
||||
"bank_charges_account",
|
||||
],
|
||||
limit_page_length=0, # intentionally unbounded: all companies are needed for boot
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user