nsinnovations/erpnext
2
0
Fork 0
mirror of https://github.com/frappe/erpnext.git synced 2026-02-16 16:15:02 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: multiple Bank Reconciliation Tool issues (#46644)

Browse Source 
* fix: bank reconciliation tool issue

* refactor: separate Bank Transaction linking from other logic

* fix: delink old pe on update_after_submit in bank transaction

* fix: failing test case fixed

* fix: changes as per review

* refactor: rename `gles` to `gl_entries`

---------

Co-authored-by: Sagar Vora <sagar@resilient.tech>
This commit is contained in:
Vishakh Desai
2025-03-31 11:25:41 +05:30
committed by GitHub
parent 9aff191bb1
commit 646cf54679
4 changed files with 244 additions and 233 deletions
Download Patch File Download Diff File Expand all files Collapse all files

43
erpnext/accounts/doctype/bank_reconciliation_tool/bank_reconciliation_tool.py
View File

@@ -8,6 +8,7 @@ import frappe
from frappe import _
from frappe.model.document import Document
from frappe.query_builder.custom import ConstantColumn
from frappe.query_builder.functions import Sum
from frappe.utils import cint, flt
from erpnext import get_default_cost_center
@@ -517,16 +518,23 @@ def subtract_allocations(gl_account, vouchers):
voucher_allocated_amounts = get_total_allocated_amount(voucher_docs)
for voucher in vouchers:
rows = voucher_allocated_amounts.get((voucher.get("doctype"), voucher.get("name"))) or []
filtered_row = list(filter(lambda row: row.get("gl_account") == gl_account, rows))
if amount := None if not filtered_row else filtered_row[0]["total"]:
if amount := get_allocated_amount(voucher_allocated_amounts, voucher, gl_account):
voucher["paid_amount"] -= amount
copied.append(voucher)
return copied
def get_allocated_amount(voucher_allocated_amounts, voucher, gl_account):
if not (voucher_details := voucher_allocated_amounts.get((voucher.get("doctype"), voucher.get("name")))):
return
if not (row := voucher_details.get(gl_account)):
return
return row.get("total")
def check_matching(
bank_account,
company,
@@ -796,26 +804,20 @@ def get_je_matching_query(
je = frappe.qb.DocType("Journal Entry")
jea = frappe.qb.DocType("Journal Entry Account")
ref_condition = je.cheque_no == transaction.reference_number
ref_rank = frappe.qb.terms.Case().when(ref_condition, 1).else_(0)
amount_field = f"{cr_or_dr}_in_account_currency"
amount_equality = getattr(jea, amount_field) == transaction.unallocated_amount
amount_rank = frappe.qb.terms.Case().when(amount_equality, 1).else_(0)
filter_by_date = je.posting_date.between(from_date, to_date)
if cint(filter_by_reference_date):
filter_by_date = je.cheque_date.between(from_reference_date, to_reference_date)
query = (
subquery = (
frappe.qb.from_(jea)
.join(je)
.on(jea.parent == je.name)
.select(
(ref_rank + amount_rank + 1).as_("rank"),
Sum(getattr(jea, amount_field)).as_("paid_amount"),
ConstantColumn("Journal Entry").as_("doctype"),
je.name,
getattr(jea, amount_field).as_("paid_amount"),
je.cheque_no.as_("reference_no"),
je.cheque_date.as_("reference_date"),
je.pay_to_recd_from.as_("party"),
@@ -827,13 +829,26 @@ def get_je_matching_query(
.where(je.voucher_type != "Opening Entry")
.where(je.clearance_date.isnull())
.where(jea.account == common_filters.bank_account)
.where(amount_equality if exact_match else getattr(jea, amount_field) > 0.0)
.where(filter_by_date)
.groupby(je.name)
.orderby(je.cheque_date if cint(filter_by_reference_date) else je.posting_date)
)
if frappe.flags.auto_reconcile_vouchers is True:
query = query.where(ref_condition)
subquery = subquery.where(je.cheque_no == transaction.reference_number)
ref_rank = frappe.qb.terms.Case().when(subquery.reference_no == transaction.reference_number, 1).else_(0)
amount_equality = subquery.paid_amount == transaction.unallocated_amount
amount_rank = frappe.qb.terms.Case().when(amount_equality, 1).else_(0)
query = (
frappe.qb.from_(subquery)
.select(
"*",
(ref_rank + amount_rank + 1).as_("rank"),
)
.where(amount_equality if exact_match else subquery.paid_amount > 0.0)
)
return query

31
erpnext/accounts/doctype/bank_transaction/bank_transaction.js
View File

@@ -31,6 +31,12 @@ frappe.ui.form.on("Bank Transaction", {
},
};
});
frm.set_query("bank_account", function () {
return {
filters: { is_company_account: 1 },
};
});
},
get_payment_doctypes: function () {
@@ -39,31 +45,6 @@ frappe.ui.form.on("Bank Transaction", {
},
});
frappe.ui.form.on("Bank Transaction Payments", {
payment_entries_remove: function (frm, cdt, cdn) {
update_clearance_date(frm, cdt, cdn);
},
});
const update_clearance_date = (frm, cdt, cdn) => {
if (frm.doc.docstatus === 1) {
frappe
.xcall("erpnext.accounts.doctype.bank_transaction.bank_transaction.unclear_reference_payment", {
doctype: cdt,
docname: cdn,
bt_name: frm.doc.name,
})
.then((e) => {
if (e == "success") {
frappe.show_alert({
message: __("Document {0} successfully uncleared", [e]),
indicator: "green",
});
}
});
}
};
function set_bank_statement_filter(frm) {
frm.set_query("bank_statement", function () {
return {

402
erpnext/accounts/doctype/bank_transaction/bank_transaction.py
View File

@@ -5,7 +5,7 @@ import frappe
from frappe import _
from frappe.model.docstatus import DocStatus
from frappe.model.document import Document
from frappe.utils import flt
from frappe.utils import flt, getdate
class BankTransaction(Document):
@@ -84,16 +84,16 @@ class BankTransaction(Document):
if not self.payment_entries:
return
pe = []
references = set()
for row in self.payment_entries:
reference = (row.payment_document, row.payment_entry)
if reference in pe:
if reference in references:
frappe.throw(
_("{0} {1} is allocated twice in this Bank Transaction").format(
row.payment_document, row.payment_entry
)
)
pe.append(reference)
references.add(reference)
def update_allocated_amount(self):
allocated_amount = (
@@ -104,6 +104,19 @@ class BankTransaction(Document):
self.allocated_amount = flt(allocated_amount, self.precision("allocated_amount"))
self.unallocated_amount = flt(unallocated_amount, self.precision("unallocated_amount"))
def delink_old_payment_entries(self):
if self.flags.updating_linked_bank_transaction:
return
old_doc = self.get_doc_before_save()
payment_entry_names = set(pe.name for pe in self.payment_entries)
for old_pe in old_doc.payment_entries:
if old_pe.name in payment_entry_names:
continue
self.delink_payment_entry(old_pe)
def before_submit(self):
self.allocate_payment_entries()
self.set_status()
@@ -113,13 +126,14 @@ class BankTransaction(Document):
def before_update_after_submit(self):
self.validate_duplicate_references()
self.allocate_payment_entries()
self.update_allocated_amount()
self.delink_old_payment_entries()
self.allocate_payment_entries()
self.set_status()
def on_cancel(self):
for payment_entry in self.payment_entries:
self.clear_linked_payment_entry(payment_entry, for_cancel=True)
self.delink_payment_entry(payment_entry)
self.set_status()
@@ -152,43 +166,55 @@ class BankTransaction(Document):
- 0 > a: Error: already over-allocated
- clear means: set the latest transaction date as clearance date
"""
if self.flags.updating_linked_bank_transaction or not self.payment_entries:
return
remaining_amount = self.unallocated_amount
to_remove = []
payment_entry_docs = [(pe.payment_document, pe.payment_entry) for pe in self.payment_entries]
pe_bt_allocations = get_total_allocated_amount(payment_entry_docs)
gl_entries = get_related_bank_gl_entries(payment_entry_docs)
gl_bank_account = frappe.db.get_value("Bank Account", self.bank_account, "account")
for payment_entry in self.payment_entries:
if payment_entry.allocated_amount == 0.0:
unallocated_amount, should_clear, latest_transaction = get_clearance_details(
self,
payment_entry,
pe_bt_allocations.get((payment_entry.payment_document, payment_entry.payment_entry))
or [],
for payment_entry in list(self.payment_entries):
if payment_entry.allocated_amount != 0:
continue
allocable_amount, should_clear, clearance_date = get_clearance_details(
self,
payment_entry,
pe_bt_allocations.get((payment_entry.payment_document, payment_entry.payment_entry)) or {},
gl_entries.get((payment_entry.payment_document, payment_entry.payment_entry)) or {},
gl_bank_account,
)
if allocable_amount < 0:
frappe.throw(_("Voucher {0} is over-allocated by {1}").format(allocable_amount))
if remaining_amount <= 0:
self.remove(payment_entry)
continue
if allocable_amount == 0:
if should_clear:
self.clear_linked_payment_entry(payment_entry, clearance_date=clearance_date)
self.remove(payment_entry)
continue
should_clear = should_clear and allocable_amount <= remaining_amount
payment_entry.allocated_amount = min(allocable_amount, remaining_amount)
remaining_amount = flt(
remaining_amount - payment_entry.allocated_amount,
self.precision("unallocated_amount"),
)
if payment_entry.payment_document == "Bank Transaction":
self.update_linked_bank_transaction(
payment_entry.payment_entry, payment_entry.allocated_amount
)
elif should_clear:
self.clear_linked_payment_entry(payment_entry, clearance_date=clearance_date)
if 0.0 == unallocated_amount:
if should_clear:
latest_transaction.clear_linked_payment_entry(payment_entry)
to_remove.append(payment_entry)
elif remaining_amount <= 0.0:
to_remove.append(payment_entry)
elif 0.0 < unallocated_amount <= remaining_amount:
payment_entry.allocated_amount = unallocated_amount
remaining_amount -= unallocated_amount
if should_clear:
latest_transaction.clear_linked_payment_entry(payment_entry)
elif 0.0 < unallocated_amount:
payment_entry.allocated_amount = remaining_amount
remaining_amount = 0.0
elif 0.0 > unallocated_amount:
frappe.throw(_("Voucher {0} is over-allocated by {1}").format(unallocated_amount))
for payment_entry in to_remove:
self.remove(payment_entry)
self.update_allocated_amount()
@frappe.whitelist()
def remove_payment_entries(self):
@@ -199,14 +225,64 @@ class BankTransaction(Document):
def remove_payment_entry(self, payment_entry):
"Clear payment entry and clearance"
self.clear_linked_payment_entry(payment_entry, for_cancel=True)
self.delink_payment_entry(payment_entry)
self.remove(payment_entry)
def clear_linked_payment_entry(self, payment_entry, for_cancel=False):
clearance_date = None if for_cancel else self.date
set_voucher_clearance(
payment_entry.payment_document, payment_entry.payment_entry, clearance_date, self
)
def delink_payment_entry(self, payment_entry):
if payment_entry.payment_document == "Bank Transaction":
self.update_linked_bank_transaction(payment_entry.payment_entry, allocated_amount=None)
else:
self.clear_linked_payment_entry(payment_entry, clearance_date=None)
def clear_linked_payment_entry(self, payment_entry, clearance_date=None):
doctype = payment_entry.payment_document
docname = payment_entry.payment_entry
# might be a bank transaction
if doctype not in get_doctypes_for_bank_reconciliation():
return
if doctype == "Sales Invoice":
frappe.db.set_value(
"Sales Invoice Payment",
dict(parenttype=doctype, parent=docname),
"clearance_date",
clearance_date,
)
return
frappe.db.set_value(doctype, docname, "clearance_date", clearance_date)
def update_linked_bank_transaction(self, bank_transaction_name, allocated_amount=None):
"""For when a second bank transaction has fixed another, e.g. refund"""
bt = frappe.get_doc(self.doctype, bank_transaction_name)
if allocated_amount:
bt.append(
"payment_entries",
{
"payment_document": self.doctype,
"payment_entry": self.name,
"allocated_amount": allocated_amount,
},
)
else:
pe = next(
(
pe
for pe in bt.payment_entries
if pe.payment_document == self.doctype and pe.payment_entry == self.name
),
None,
)
if not pe:
return
bt.flags.updating_linked_bank_transaction = True
bt.remove(pe)
bt.save()
def auto_set_party(self):
from erpnext.accounts.doctype.bank_transaction.auto_match_party import AutoMatchParty
@@ -238,71 +314,107 @@ def get_doctypes_for_bank_reconciliation():
return frappe.get_hooks("bank_reconciliation_doctypes")
def get_clearance_details(transaction, payment_entry, bt_allocations):
def get_clearance_details(transaction, payment_entry, bt_allocations, gl_entries, gl_bank_account):
"""
There should only be one bank gle for a voucher.
Could be none for a Bank Transaction.
But if a JE, could affect two banks.
Should only clear the voucher if all bank gles are allocated.
There should only be one bank gl entry for a voucher, except for JE.
For JE, there can be multiple bank gl entries for the same account.
In this case, the allocable_amount will be the sum of amounts of all gl entries of the account.
There will be no gl entry for a Bank Transaction so return the unallocated amount.
Should only clear the voucher if all bank gl entries are allocated.
"""
gl_bank_account = frappe.db.get_value("Bank Account", transaction.bank_account, "account")
gles = get_related_bank_gl_entries(payment_entry.payment_document, payment_entry.payment_entry)
unallocated_amount = min(
transaction.unallocated_amount,
get_paid_amount(payment_entry, transaction.currency, gl_bank_account),
)
unmatched_gles = len(gles)
latest_transaction = transaction
for gle in gles:
if gle["gl_account"] == gl_bank_account:
if gle["amount"] <= 0.0:
frappe.throw(
_("Voucher {0} value is broken: {1}").format(payment_entry.payment_entry, gle["amount"])
transaction_date = getdate(transaction.date)
if payment_entry.payment_document == "Bank Transaction":
bt = frappe.db.get_value(
"Bank Transaction",
payment_entry.payment_entry,
("unallocated_amount", "bank_account"),
as_dict=True,
)
if bt.bank_account != gl_bank_account:
frappe.throw(
_("Bank Account {} in Bank Transaction {} is not matching with Bank Account {}").format(
bt.bank_account, payment_entry.payment_entry, gl_bank_account
)
)
unmatched_gles -= 1
unallocated_amount = gle["amount"]
for a in bt_allocations:
if a["gl_account"] == gle["gl_account"]:
unallocated_amount = gle["amount"] - a["total"]
if frappe.utils.getdate(transaction.date) < a["latest_date"]:
latest_transaction = frappe.get_doc("Bank Transaction", a["latest_name"])
else:
# Must be a Journal Entry affecting more than one bank
for a in bt_allocations:
if a["gl_account"] == gle["gl_account"] and a["total"] == gle["amount"]:
unmatched_gles -= 1
return abs(bt.unallocated_amount), True, transaction_date
return unallocated_amount, unmatched_gles == 0, latest_transaction
if gl_bank_account not in gl_entries:
frappe.throw(
_("{} {} is not affecting bank account {}").format(
payment_entry.payment_document, payment_entry.payment_entry, gl_bank_account
)
)
allocable_amount = gl_entries.pop(gl_bank_account) or 0
if allocable_amount <= 0.0:
frappe.throw(
_("Invalid amount in accounting entries of {} {} for Account {}: {}").format(
payment_entry.payment_document, payment_entry.payment_entry, gl_bank_account, allocable_amount
)
)
matching_bt_allocaion = bt_allocations.pop(gl_bank_account, {})
allocable_amount = flt(
allocable_amount - matching_bt_allocaion.get("total", 0), transaction.precision("unallocated_amount")
)
should_clear = all(
gl_entries[gle_account] == bt_allocations.get(gle_account, {}).get("total", 0)
for gle_account in gl_entries
)
bt_allocation_date = matching_bt_allocaion.get("latest_date", None)
clearance_date = transaction_date if not bt_allocation_date else max(transaction_date, bt_allocation_date)
return allocable_amount, should_clear, clearance_date
def get_related_bank_gl_entries(doctype, docname):
def get_related_bank_gl_entries(docs):
# nosemgrep: frappe-semgrep-rules.rules.frappe-using-db-sql
return frappe.db.sql(
if not docs:
return {}
result = frappe.db.sql(
"""
SELECT
ABS(gle.credit_in_account_currency - gle.debit_in_account_currency) AS amount,
gle.account AS gl_account
FROM
`tabGL Entry` gle
LEFT JOIN
`tabAccount` ac ON ac.name=gle.account
WHERE
ac.account_type = 'Bank'
AND gle.voucher_type = %(doctype)s
AND gle.voucher_no = %(docname)s
AND is_cancelled = 0
""",
dict(doctype=doctype, docname=docname),
SELECT
gle.voucher_type AS doctype,
gle.voucher_no AS docname,
gle.account AS gl_account,
SUM(ABS(gle.credit_in_account_currency - gle.debit_in_account_currency)) AS amount
FROM
`tabGL Entry` gle
LEFT JOIN
`tabAccount` ac ON ac.name = gle.account
WHERE
ac.account_type = 'Bank'
AND (gle.voucher_type, gle.voucher_no) IN %(docs)s
AND gle.is_cancelled = 0
GROUP BY
gle.voucher_type, gle.voucher_no, gle.account
""",
{"docs": docs},
as_dict=True,
)
entries = {}
for row in result:
key = (row["doctype"], row["docname"])
if key not in entries:
entries[key] = {}
entries[key][row["gl_account"]] = row["amount"]
return entries
def get_total_allocated_amount(docs):
"""
Gets the sum of allocations for a voucher on each bank GL account
along with the latest bank transaction name & date
along with the latest bank transaction date
NOTE: query may also include just saved vouchers/payments but with zero allocated_amount
"""
if not docs:
@@ -311,11 +423,10 @@ def get_total_allocated_amount(docs):
# nosemgrep: frappe-semgrep-rules.rules.frappe-using-db-sql
result = frappe.db.sql(
"""
SELECT total, latest_name, latest_date, gl_account, payment_document, payment_entry FROM (
SELECT total, latest_date, gl_account, payment_document, payment_entry FROM (
SELECT
ROW_NUMBER() OVER w AS rownum,
SUM(btp.allocated_amount) OVER(PARTITION BY ba.account, btp.payment_document, btp.payment_entry) AS total,
FIRST_VALUE(bt.name) OVER w AS latest_name,
FIRST_VALUE(bt.date) OVER w AS latest_date,
ba.account AS gl_account,
btp.payment_document,
@@ -338,104 +449,14 @@ def get_total_allocated_amount(docs):
payment_allocation_details = {}
for row in result:
# Why is this *sometimes* a byte string?
if isinstance(row["latest_name"], bytes):
row["latest_name"] = row["latest_name"].decode()
row["latest_date"] = frappe.utils.getdate(row["latest_date"])
payment_allocation_details.setdefault((row["payment_document"], row["payment_entry"]), []).append(row)
row["latest_date"] = getdate(row["latest_date"])
payment_allocation_details.setdefault((row["payment_document"], row["payment_entry"]), {})[
row["gl_account"]
] = row
return payment_allocation_details
def get_paid_amount(payment_entry, currency, gl_bank_account):
if payment_entry.payment_document in ["Payment Entry", "Sales Invoice", "Purchase Invoice"]:
paid_amount_field = "paid_amount"
if payment_entry.payment_document == "Payment Entry":
doc = frappe.get_doc("Payment Entry", payment_entry.payment_entry)
if doc.payment_type == "Receive":
paid_amount_field = (
"received_amount" if doc.paid_to_account_currency == currency else "base_received_amount"
)
elif doc.payment_type == "Pay":
paid_amount_field = (
"paid_amount" if doc.paid_from_account_currency == currency else "base_paid_amount"
)
return frappe.db.get_value(
payment_entry.payment_document, payment_entry.payment_entry, paid_amount_field
)
elif payment_entry.payment_document == "Journal Entry":
return abs(
frappe.db.get_value(
"Journal Entry Account",
{"parent": payment_entry.payment_entry, "account": gl_bank_account},
"sum(debit_in_account_currency-credit_in_account_currency)",
)
or 0
)
elif payment_entry.payment_document == "Expense Claim":
return frappe.db.get_value(
payment_entry.payment_document, payment_entry.payment_entry, "total_amount_reimbursed"
)
elif payment_entry.payment_document == "Loan Disbursement":
return frappe.db.get_value(
payment_entry.payment_document, payment_entry.payment_entry, "disbursed_amount"
)
elif payment_entry.payment_document == "Loan Repayment":
return frappe.db.get_value(payment_entry.payment_document, payment_entry.payment_entry, "amount_paid")
elif payment_entry.payment_document == "Bank Transaction":
dep, wth = frappe.db.get_value(
"Bank Transaction", payment_entry.payment_entry, ("deposit", "withdrawal")
)
return abs(flt(wth) - flt(dep))
else:
frappe.throw(
f"Please reconcile {payment_entry.payment_document}: {payment_entry.payment_entry} manually"
)
def set_voucher_clearance(doctype, docname, clearance_date, self):
if doctype in get_doctypes_for_bank_reconciliation():
if (
doctype == "Payment Entry"
and frappe.db.get_value("Payment Entry", docname, "payment_type") == "Internal Transfer"
and len(get_reconciled_bank_transactions(doctype, docname)) < 2
):
return
if doctype == "Sales Invoice":
frappe.db.set_value(
"Sales Invoice Payment",
dict(parenttype=doctype, parent=docname),
"clearance_date",
clearance_date,
)
return
frappe.db.set_value(doctype, docname, "clearance_date", clearance_date)
elif doctype == "Bank Transaction":
# For when a second bank transaction has fixed another, e.g. refund
bt = frappe.get_doc(doctype, docname)
if clearance_date:
vouchers = [{"payment_doctype": "Bank Transaction", "payment_name": self.name}]
bt.add_payment_entries(vouchers)
bt.save()
else:
for pe in bt.payment_entries:
if pe.payment_document == self.doctype and pe.payment_entry == self.name:
bt.remove(pe)
bt.save()
break
def get_reconciled_bank_transactions(doctype, docname):
return frappe.get_all(
"Bank Transaction Payments",
@@ -444,13 +465,6 @@ def get_reconciled_bank_transactions(doctype, docname):
)
@frappe.whitelist()
def unclear_reference_payment(doctype, docname, bt_name):
bt = frappe.get_doc("Bank Transaction", bt_name)
set_voucher_clearance(doctype, docname, None, bt)
return docname
def remove_from_bank_transaction(doctype, docname):
"""Remove a (cancelled) voucher from all Bank Transactions."""
for bt_name in get_reconciled_bank_transactions(doctype, docname):

1
erpnext/accounts/test/accounts_mixin.py
View File

@@ -91,6 +91,7 @@ class AccountsTestMixin:
"attribute_name": "bank",
"account_name": "HDFC",
"parent_account": "Bank Accounts - " + abbr,
"account_type": "Bank",
}
),
frappe._dict(