nsinnovations/erpnext
2
0
Fork 0
mirror of https://github.com/frappe/erpnext.git synced 2026-09-13 17:20:36 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(banking): UI cleanup and better statement parsing (#58817)

Browse Source 
* fix(banking): reset scroll on searching accounts

* fix(banking): show only past dates in date filter

* fix(banking): clean up line heights and remove beta badge

* fix(banking): show accurate count of import progress
fix(banking): show latest 20 imports instead of 10

* fix(banking): layout sizing needs to be preserved on page change

* fix(banking): cleaner bank balance UI

* fix(banking): correctly parse Cr/Dr values in statement importer

* Update banking/src/components/features/BankReconciliation/BankBalance.tsx

Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>

---------

Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
This commit is contained in:
Nikhil Kothari
2026-09-07 18:40:02 +05:30
committed by GitHub
parent e85e300f8f
commit ebe5decb96
22 changed files with 1162 additions and 437 deletions
Download Patch File Download Diff File Expand all files Collapse all files

170
erpnext/accounts/doctype/bank_statement_import_log/bank_statement_import_log.py
View File

@@ -375,8 +375,7 @@ class BankStatementImportLog(Document):
table["column_mapping"] = guess_column_mapping_by_content(table["rows"])
final_transactions, table["date_format"], table["amount_format"] = build_table_transactions(table)
# Tables with no detectable transactions (ads, summaries, headers) start excluded.
table["included"] = bool(final_transactions)
table["included"] = should_include_table(table, final_transactions)
self.pdf_tables = json.dumps(tables)
return tables
@@ -542,6 +541,8 @@ class BankStatementImportLog(Document):
"bank-rec-statement-import-progress",
{
"progress": round(progress / total_transactions * 100),
"current": progress,
"total": total_transactions,
},
doctype="Bank Statement Import Log",
docname=self.name,
@@ -551,6 +552,7 @@ class BankStatementImportLog(Document):
"bank-rec-statement-import-progress",
{
"progress": 100,
"current": total_transactions,
"total": total_transactions,
},
doctype="Bank Statement Import Log",
@@ -821,6 +823,15 @@ def compute_final_transactions(transaction_rows: list, date_format: str, amount_
"""Pure version of the final-transaction builder (date normalized, amount split)."""
final_transactions = []
# Which marker does this statement actually write? A statement that only ever says "Cr"
# is marking the credits as its exceptions, so an unmarked row is a withdrawal; one that
# only ever says "Dr" means the opposite. With both markers present an unmarked row is
# genuinely undetermined, so it stays a withdrawal.
unmarked_is_deposit = False
if amount_format == 'Amount column has "CR"/"DR" values':
markers = {get_amount_cr_dr_marker(row.get("amount")) for row in transaction_rows}
unmarked_is_deposit = markers - {None} == {"dr"}
def parse_amount(transaction_row: dict):
if amount_format == "Separate columns for withdrawal and deposit":
return get_float_amount(transaction_row.get("withdrawal")), get_float_amount(
@@ -829,44 +840,43 @@ def compute_final_transactions(transaction_rows: list, date_format: str, amount_
if amount_format == 'Amount column has "CR"/"DR" values':
amount = transaction_row.get("amount")
marker = get_amount_cr_dr_marker(amount)
# The marker carries the direction, so the amount's own sign is ignored.
signed_amount = get_float_amount(amount) or 0
# If the amount column has CR/DR in it - we should remove any signs (negative or positive) from the amount
float_amount = abs(get_float_amount(amount) or 0)
if "cr" in amount.lower():
return 0, float_amount
else:
return float_amount, 0
if marker:
return (0, abs(signed_amount)) if marker == "cr" else (abs(signed_amount), 0)
# An unmarked row takes the opposite direction to the marker this statement
# uses. A negative amount reverses that again (a refund).
is_deposit = unmarked_is_deposit
if signed_amount < 0:
is_deposit = not is_deposit
return (0, abs(signed_amount)) if is_deposit else (abs(signed_amount), 0)
# `or 0` below: get_float_amount returns None for an unparseable cell, and a blank
# transaction-type cell comes through as None. Both used to raise.
if amount_format == "Amount column has positive/negative values":
amount = get_float_amount(transaction_row.get("amount", "0"))
amount = get_float_amount(transaction_row.get("amount", "0")) or 0
if amount > 0:
return 0, abs(amount)
else:
return abs(amount), 0
transaction_type = str(transaction_row.get("debit_credit") or "").strip().lower()
amount = abs(get_float_amount(transaction_row.get("amount", "0")) or 0)
if amount_format == 'Transaction type column has "CR"/"DR" values':
transaction_type = transaction_row.get("debit_credit")
amount = get_float_amount(transaction_row.get("amount", "0"))
if "cr" in transaction_type.lower():
return 0, abs(amount)
else:
return abs(amount), 0
# "credit" contains "cr". "debit" does not contain "dr", so it correctly falls
# through to the withdrawal side.
return (0, amount) if "cr" in transaction_type else (amount, 0)
if amount_format == 'Transaction type column has "C"/"D" values':
transaction_type = transaction_row.get("debit_credit")
amount = get_float_amount(transaction_row.get("amount", "0"))
if transaction_type.lower().strip() == "c":
return 0, abs(amount)
else:
return abs(amount), 0
return (0, amount) if transaction_type == "c" else (amount, 0)
if amount_format == 'Transaction type column has "Deposit"/"Withdrawal" values':
transaction_type = transaction_row.get("debit_credit")
amount = get_float_amount(transaction_row.get("amount", "0"))
if "deposit" in transaction_type.lower():
return 0, abs(amount)
else:
return abs(amount), 0
return (0, amount) if "deposit" in transaction_type else (amount, 0)
return 0, 0
@@ -910,6 +920,26 @@ def build_table_transactions(table: dict):
return final_transactions, date_format, amount_format
def should_include_table(table: dict, final_transactions: list) -> bool:
"""
Whether a freshly extracted PDF table should START as included - only the default state
of the checkbox, which the user can change afterwards.
It must have yielded transactions, and it must have a Description column mapped. A
transaction table always carries a narration; the summary boxes printed around it -
payment due, credit limit, reward points - are dates and figures only. Otherwise the
HDFC credit-card "Payment Due Date / Total Dues / Minimum Amount Due" box parses as one
transaction and imports a phantom row.
A description is NOT needed to import (it is not mandatory on Bank Transaction), so a
bank that omits narration still works - its table just starts unticked.
"""
if not final_transactions:
return False
return any(column.get("maps_to") == "Description" for column in table.get("column_mapping", []))
def _clean_cell(cell) -> str:
"""Normalize a pdfplumber cell: None -> '', collapse wrapped newlines, strip."""
if cell is None:
@@ -1055,6 +1085,43 @@ def get_float_amount(amount):
return amount
# A "CR"/"DR" marker on the amount itself, at either end: "2,378.00Cr", "Cr 100",
# "INR 50.90 Cr.", "DR 1,234.50".
# `(?![a-zA-Z])` rather than `\b` on the leading form: there is no word boundary between
# the "r" of "Cr100" and the digit, but there IS one inside "CREDIT" and "DRAFT".
AMOUNT_CR_DR_PATTERN = re.compile(r"^\s*(cr|dr)(?![a-zA-Z])\.?|(?:^|[\s\d.)])(cr|dr)\b\.?\s*$", re.IGNORECASE)
def get_amount_cr_dr_marker(amount) -> str | None:
"""
Return "cr" or "dr" if the amount cell carries a direction marker of its own, else None.
What is left after removing the marker has to look like an amount - it must hold a digit
and at most a short currency token - so that text which merely starts or ends with the
letters is not read as a marker. That guard is what separates "Cr 100" from a
description that bled into the amount column, like "Dr Smith Clinic 500".
"""
if not isinstance(amount, str):
return None
match = AMOUNT_CR_DR_PATTERN.search(amount)
if not match:
return None
# Only the marker itself is removed - the surrounding character the pattern needed to
# anchor on (a digit, say) stays part of the remainder.
group = 1 if match.group(1) else 2
start, end = match.span(group)
remainder = amount[:start] + amount[end:]
if not any(char.isdigit() for char in remainder):
return None
if sum(char.isalpha() for char in remainder) > 3:
return None
return match.group(group).lower()
def get_file_properties(transactions: list):
"""
From the transaction rows, try to figure out the following:
@@ -1075,6 +1142,8 @@ def get_file_properties(transactions: list):
'Transaction type column has "C"/"D" values': 0,
}
amount_column_has_cr_dr = False
for transaction in transactions:
date_format = transaction.get("date_format")
@@ -1092,33 +1161,40 @@ def get_file_properties(transactions: list):
if not amount:
continue
if isinstance(amount, str) and ("cr" in amount.lower() or "dr" in amount.lower()):
debit_credit = str(transaction.get("debit_credit") or "").strip().lower()
# One vote per row, most specific signal first. Order matters: "withdrawal" contains
# "dr", so it must be matched before the loose cr/dr check or a Deposit/Withdrawal
# column reads as CR/DR. "debit" needs listing because, unlike "credit", it does not
# contain "dr". The final else means every row votes, even an unrecognised type.
if get_amount_cr_dr_marker(amount):
amount_column_has_cr_dr = True
amount_format_frequency['Amount column has "CR"/"DR" values'] += 1
# Check if there's a debit_credit column containing "cr"/"dr"
if transaction.get("debit_credit", None):
if (
"cr" in transaction.get("debit_credit", "").lower()
or "dr" in transaction.get("debit_credit", "").lower()
):
amount_format_frequency['Transaction type column has "CR"/"DR" values'] += 1
elif (
"deposit" in transaction.get("debit_credit", "").lower()
or "withdrawal" in transaction.get("debit_credit", "").lower()
):
amount_format_frequency['Transaction type column has "Deposit"/"Withdrawal" values'] += 1
elif (transaction.get("debit_credit", "").lower().strip() == "c") or (
transaction.get("debit_credit", "").lower().strip() == "d"
):
amount_format_frequency['Transaction type column has "C"/"D" values'] += 1
# Else assume that the amount is expressed as positive/negative value
elif "deposit" in debit_credit or "withdrawal" in debit_credit:
amount_format_frequency['Transaction type column has "Deposit"/"Withdrawal" values'] += 1
elif debit_credit in ("c", "d"):
amount_format_frequency['Transaction type column has "C"/"D" values'] += 1
elif any(token in debit_credit for token in ("cr", "dr", "debit")):
amount_format_frequency['Transaction type column has "CR"/"DR" values'] += 1
else:
# Nothing said which direction this is, so assume the amount carries the sign.
amount_format_frequency["Amount column has positive/negative values"] += 1
most_common_date_format = max(date_format_frequency, key=date_format_frequency.get)
most_common_amount_format = max(amount_format_frequency, key=amount_format_frequency.get)
# With no votes at all (no rows, or every amount blank) max() would return whichever key
# happens to be first in the dict. Say what we mean instead.
if not amount_format_frequency[most_common_amount_format]:
most_common_amount_format = "Amount column has positive/negative values"
# A CR/DR amount column is proved by a single marker, not by a majority: both formats
# describe the same column, and an unmarked row is only the default direction, not
# evidence against the notation. Statements mark just the exceptions - one HDFC
# credit-card page has 18 rows and a single "50.90Cr".
if amount_column_has_cr_dr and most_common_amount_format == "Amount column has positive/negative values":
most_common_amount_format = 'Amount column has "CR"/"DR" values'
return most_common_date_format, most_common_amount_format

251
erpnext/accounts/doctype/bank_statement_import_log/test_bank_statement_import_log.py
View File

@@ -11,12 +11,14 @@ from erpnext.accounts.doctype.bank_statement_import_log.bank_statement_import_lo
detect_column_mapping,
detect_header_row,
extract_pdf_tables,
get_amount_cr_dr_marker,
get_float_amount,
get_statement_details,
guess_column_mapping_by_content,
reextract_pdf_table,
set_header_index,
set_pdf_table_header,
should_include_table,
update_column_mapping,
update_pdf_tables,
)
@@ -124,6 +126,184 @@ class TestBankStatementImportLog(ERPNextTestSuite, AccountsTestMixin):
self.assertIsNone(get_float_amount("ABCD"))
self.assertIsNone(get_float_amount("****"))
# ------------------------------------------------------------------ #
# Amount format detection
# ------------------------------------------------------------------ #
def test_amount_cr_dr_marker(self):
"""The marker is read at either end of the cell, but only next to the amount."""
for amount in ("2,378.00Cr", "50.90 CR", "INR 50.90 Cr.", "1000cr", "5cr", "(100) Cr"):
self.assertEqual(get_amount_cr_dr_marker(amount), "cr", amount)
for amount in ("2,378.00Dr", "50.90 DR", "1000dr", "-100 Dr"):
self.assertEqual(get_amount_cr_dr_marker(amount), "dr", amount)
# Some banks put the marker in front of the digits instead.
for amount in ("Cr 100", "Cr100", "CR INR 100", "cr 0.00"):
self.assertEqual(get_amount_cr_dr_marker(amount), "cr", amount)
for amount in ("Dr 100", "Dr100", "Dr. 1,234.50"):
self.assertEqual(get_amount_cr_dr_marker(amount), "dr", amount)
for amount in ("100.00", "-2,000.00", "INR 25,236.00", "", None, 100.0):
self.assertIsNone(get_amount_cr_dr_marker(amount), amount)
# Text that merely starts or ends with the letters must not be read as a marker, or
# a description that bled into the amount column would reclassify the statement.
for amount in (
"CREDIT CARD PAYMENT 500",
"DRAFT 100",
"Dr Smith Clinic 500",
"DR AMBEDKAR ROAD BRANCH 500",
"500 CRC",
"Cheque Dr",
"Cr",
):
self.assertIsNone(get_amount_cr_dr_marker(amount), amount)
def test_sparsely_marked_cr_dr_amount_column(self):
"""One marker is enough to prove a CR/DR amount column - it is not a majority vote.
A real HDFC credit-card page carries 18 rows and a single "50.90Cr": the unmarked
rows are ordinary purchases, and only the exceptions are marked. A frequency vote
therefore picked "positive/negative" 17-1 and imported that lone credit as a debit.
"""
doc = self._create_bank_statement_import_log(
[
["Date", "Transaction Description", "Amount (in Rs.)"],
["21/07/2026", "ITC MAURYA NEW DELHI", "2,495.00"],
["22/07/2026", "ZOMATO LIMITED Gurugram", "1,288.68"],
["23/07/2026", "SWIGGY Bangalore", "532.00"],
["26/07/2026", "SWIGGY Bangalore", "1,043.00"],
["27/07/2026", "PETRO SURCHARGE WAIVER", "50.90Cr"],
]
)
self.assertEqual(doc.detected_amount_format, 'Amount column has "CR"/"DR" values')
# Only "Cr" appears, so it is the marked exception and unmarked rows are debits.
self.assertEqual(doc.total_credits, 50.90)
self.assertEqual(doc.total_credit_transactions, 1)
self.assertEqual(doc.total_debits, 5358.68)
self.assertEqual(doc.total_debit_transactions, 4)
def test_dr_only_statement_treats_unmarked_rows_as_deposits(self):
"""The mirror image of a Cr-only statement: only withdrawals are marked.
The unmarked default cannot be hardcoded to the debit, because which side gets
marked varies by bank. It is derived from the markers the statement actually uses -
here only "Dr" appears, so "Dr" is the exception and everything unmarked is a
deposit.
"""
doc = self._create_bank_statement_import_log(
[
["Date", "Narration", "Amount"],
["01/04/2026", "ATM WITHDRAWAL", "2,000.00Dr"],
["03/04/2026", "SALARY", "20,000.00"],
["05/04/2026", "INTEREST", "150.00"],
]
)
self.assertEqual(doc.detected_amount_format, 'Amount column has "CR"/"DR" values')
self.assertEqual(doc.total_debits, 2000.0)
self.assertEqual(doc.total_debit_transactions, 1)
self.assertEqual(doc.total_credits, 20150.0)
self.assertEqual(doc.total_credit_transactions, 2)
def test_leading_cr_dr_markers(self):
"""Some banks print the marker in front of the amount."""
doc = self._create_bank_statement_import_log(
[
["Date", "Narration", "Amount"],
["01/04/2026", "ATM WITHDRAWAL", "Dr 2,000.00"],
["03/04/2026", "SALARY", "Cr 20,000.00"],
]
)
self.assertEqual(doc.detected_amount_format, 'Amount column has "CR"/"DR" values')
self.assertEqual(doc.total_debits, 2000.0)
self.assertEqual(doc.total_credits, 20000.0)
def test_partially_marked_cr_dr_amount_column(self):
"""A CR/DR amount column stays CR/DR even when some rows carry no marker.
Every unmarked row used to also vote for "positive/negative", so an ordinary
statement with a few unmarked rows was detected as positive/negative and a
"2000.00Dr" was then imported as a deposit.
"""
doc = self._create_bank_statement_import_log(
[
["Date", "Narration", "Amount", "Balance"],
["01/04/2026", "OPENING FEE", "100.00", "9,900.00"],
["03/04/2026", "SALARY", "20000.00Cr", "29,900.00"],
["05/04/2026", "ATM WDL", "2000.00Dr", "27,900.00"],
]
)
self.assertEqual(doc.detected_amount_format, 'Amount column has "CR"/"DR" values')
# Both markers appear, so an unmarked row is undetermined and stays a debit.
self.assertEqual(doc.total_debits, 2100.0)
self.assertEqual(doc.total_debit_transactions, 2)
self.assertEqual(doc.total_credits, 20000.0)
self.assertEqual(doc.total_credit_transactions, 1)
def test_deposit_withdrawal_type_column(self):
"""The word Withdrawal contains "dr", so a loose CR/DR check claims this column first.
It then reads "Deposit" (which has no "cr" in it) as a withdrawal, flipping the
direction of every credit in the statement.
"""
doc = self._create_bank_statement_import_log(
[
["Date", "Narration", "Transaction Type", "Amount"],
["01/04/2026", "ATM WDL", "Withdrawal", "2,000.00"],
["03/04/2026", "SALARY", "Deposit", "20,000.00"],
["05/04/2026", "ATM WDL", "Withdrawal", "500.00"],
]
)
self.assertEqual(
doc.detected_amount_format, 'Transaction type column has "Deposit"/"Withdrawal" values'
)
self.assertEqual(doc.total_debits, 2500.0)
self.assertEqual(doc.total_debit_transactions, 2)
self.assertEqual(doc.total_credits, 20000.0)
self.assertEqual(doc.total_credit_transactions, 1)
def test_unrecognised_type_column_falls_back_to_signed_amount(self):
"""An unrecognised transaction type must not stop the amount being read.
No tally was incremented for these rows, so max() returned the first key -
"Separate columns for withdrawal and deposit" - and, with no such columns in the
file, every amount came through as None.
"""
doc = self._create_bank_statement_import_log(
[
["Date", "Narration", "Transaction Type", "Amount"],
["01/04/2026", "ATM WDL", "NEFT", "-2,000.00"],
["03/04/2026", "SALARY", "IMPS", "20,000.00"],
]
)
self.assertEqual(doc.detected_amount_format, "Amount column has positive/negative values")
self.assertEqual(doc.total_debits, 2000.0)
self.assertEqual(doc.total_credits, 20000.0)
def test_blank_transaction_type_cell(self):
"""A blank type cell used to raise - `None.lower()` - instead of parsing the row."""
doc = self._create_bank_statement_import_log(
[
["Date", "Narration", "Transaction Type", "Amount"],
["01/04/2026", "ATM WDL", "Dr", "2,000.00"],
["03/04/2026", "SALARY", "Cr", "20,000.00"],
["05/04/2026", "UNKNOWN", None, "500.00"],
]
)
self.assertEqual(doc.detected_amount_format, 'Transaction type column has "CR"/"DR" values')
# The unmarked row has no direction of its own, so it counts as a withdrawal.
self.assertEqual(doc.total_debits, 2500.0)
self.assertEqual(doc.total_credits, 20000.0)
# ------------------------------------------------------------------ #
# PDF statement import
# ------------------------------------------------------------------ #
@@ -159,7 +339,8 @@ class TestBankStatementImportLog(ERPNextTestSuite, AccountsTestMixin):
else:
table["header_index"] = None
table["column_mapping"] = guess_column_mapping_by_content(table["rows"])
table["included"] = True
final_transactions, _df, _af = build_table_transactions(table)
table["included"] = should_include_table(table, final_transactions)
return table
def test_pdf_multi_page_kept_separate_and_unioned(self):
@@ -197,6 +378,74 @@ class TestBankStatementImportLog(ERPNextTestSuite, AccountsTestMixin):
final, _df, _af = build_table_transactions(ad_table)
self.assertEqual(final, [])
def test_pdf_summary_box_not_auto_included(self):
"""A summary box that happens to parse as one transaction must not start included.
The "Payment Due Date / Total Dues / Minimum Amount Due" block on an HDFC
credit-card statement has a date column and a figures column, so it yields a single
transaction - the due date and the minimum amount - and used to import as a phantom
row. What it does not have, and a real transaction table always does, is a narration.
"""
summary_box = {
"header_index": 1,
"rows": [
["Statement Date:17/08/2025", "Card No: 4341 55XX XXXX 2754", ""],
["Payment Due Date", "Total Dues", "Minimum Amount Due"],
["06/09/2025", "73,200.00", "3,660.00"],
["Credit Limit", "Available Credit Limit", "Available Cash Limit"],
["", "32,800", ""],
],
"column_mapping": [
{"index": 0, "header_text": "Payment Due Date", "variable": "a", "maps_to": "Date"},
{"index": 1, "header_text": "Total Dues", "variable": "b", "maps_to": "Do not import"},
{"index": 2, "header_text": "Minimum Amount Due", "variable": "c", "maps_to": "Amount"},
],
}
final, _df, _af = build_table_transactions(summary_box)
# It really does parse as a transaction - that is why the previous check missed it.
self.assertEqual(len(final), 1)
self.assertFalse(should_include_table(summary_box, final))
# The transaction table beside it, which does carry a narration, still starts included.
transactions = self._auto_map(
{
"rows": [
["Date", "Transaction Description", "Amount (in Rs.)"],
["21/07/2025", "ITC MAURYA NEW DELHI", "2,495.00"],
["27/07/2025", "PETRO SURCHARGE WAIVER", "50.90Cr"],
]
}
)
self.assertTrue(transactions["included"])
def test_pdf_table_without_description_still_importable(self):
"""No narration column means "starts unticked", NOT "cannot be imported".
`description` is not mandatory on Bank Transaction, so a bank that omits narration
must still import once the user ticks the table.
"""
table = {
"header_index": 0,
"rows": [
["Date", "Amount", "Balance"],
["01/04/2025", "500.00", "9,500.00"],
["03/04/2025", "20000.00", "29,500.00"],
],
"column_mapping": [
{"index": 0, "header_text": "Date", "variable": "a", "maps_to": "Date"},
{"index": 1, "header_text": "Amount", "variable": "b", "maps_to": "Amount"},
{"index": 2, "header_text": "Balance", "variable": "c", "maps_to": "Balance"},
],
}
final, _df, _af = build_table_transactions(table)
self.assertFalse(should_include_table(table, final))
# The transactions themselves are intact and importable.
self.assertEqual(len(final), 2)
self.assertEqual([t["date"] for t in final], ["2025-04-01", "2025-04-03"])
def test_headerless_content_mapping(self):
"""Without a header row, columns are guessed from their contents."""
rows = [