fix(banking): include paid purchase invoices in reports and bank clearance

[nikkothari22]

Purchase Invoices which have a direct payment (Is Paid is checked) were not included in banking reports - Bank Clearance, Bank Clearance Tool and Bank Reconciliation Statement. This has now been fixed.

Also noticed that these reports were using raw SQL so migrated all queries to the new query builder.

Closes #51552

no-docs

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Replaces raw SQL with Frappé Query Builder across bank clearance and reconciliation modules. get_payment_entries_for_bank_clearance now builds Journal Entry, Journal Entry Account, Payment Entry, Purchase Invoice, and optional POS/Sales Invoice queries using QB constructs, Case/Coalesce expressions, and conditional currency handling; results are combined into unified entry lists with preserved ordering and filters for account, date range, docstatus, opening, and reconciliation status. bank_clearance_summary and bank_reconciliation_statement were migrated to QB and a new get_purchase_invoices(filters) function was added. Public signatures unchanged.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main change: adding paid purchase invoices to banking reports while migrating to the query builder.
Description check	✅ Passed	The description clearly explains the bug fix (including paid purchase invoices in reports) and mentions the migration to query builder, directly addressing the PR's purpose.
Linked Issues check	✅ Passed	The code changes successfully address issue #51552 by adding logic to retrieve and include paid Purchase Invoices in bank clearance and reconciliation reports.
Out of Scope Changes check	✅ Passed	All changes are focused on including paid purchase invoices in banking reports and migrating SQL to query builder, with no unrelated modifications detected.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 5

🤖 Fix all issues with AI agents

In `@erpnext/accounts/doctype/bank_clearance/bank_clearance.py`:
- Around line 237-268: The debit expression currently always adds
pe.total_taxes_and_charges, causing currency mismatches for "Receive" payments;
update the debit branch (the Case() producing "debit" that uses
pe.received_amount + pe.total_taxes_and_charges) to mirror the credit-side
currency-aware logic: when pe.payment_type == "Receive" and
company.default_currency == pe.paid_to_account_currency use
pe.base_total_taxes_and_charges, otherwise use pe.total_taxes_and_charges,
keeping pe.received_amount added in either case (refer to the existing credit
Case() and the fields pe.received_amount, pe.total_taxes_and_charges,
pe.base_total_taxes_and_charges, pe.payment_type, pe.paid_to_account_currency,
and the "debit" alias to locate the code).

In `@erpnext/accounts/report/bank_clearance_summary/bank_clearance_summary.py`:
- Around line 75-99: The journal_entries query in bank_clearance_summary.py is
using base currency fields (jea.debit - jea.credit) which yields wrong amounts
for non-company account currencies; update the select expression in the
journal_entries query (the block building
frappe.qb.from_(jeaa).inner_join(je)...select(...)) to compute amounts using
account-currency fields jea.debit_in_account_currency -
jea.credit_in_account_currency (matching bank_reconciliation_statement.py and
bank_clearance.py) and keep the same alias used elsewhere for the amount column
so downstream code (e.g., any consumers of journal_entries) continues to work.
- Around line 128-148: The Purchase Invoice query built against DocType pi
(variable pi, result purchase_invoices) should explicitly filter for paid
invoices: add a condition (pi.is_paid == 1) to the .where(...) clause alongside
the existing (pi.cash_bank_account == filters.account) and date filters so only
paid PIs with a cash_bank_account are returned; update the .where in the
function that constructs purchase_invoices to include this is_paid check (use
the pi.is_paid symbol) to match the doctype business logic.

In
`@erpnext/accounts/report/bank_reconciliation_statement/bank_reconciliation_statement.py`:
- Around line 303-320: The where clause in the pe_amount query has an
operator-precedence bug: & binds tighter than | so the date/company filters only
apply to the paid_to branch; update the filter grouping in the pe_amount query
(the .where(...) using pe.docstatus, pe.paid_from, pe.paid_to, pe.posting_date,
pe.clearance_date, pe.company) to explicitly parenthesize the OR condition so
that both (pe.paid_from == filters.account) and (pe.paid_to == filters.account)
are evaluated together before applying the
posting_date/clearance_date/company/docstatus filters (e.g. group the
paid_from|paid_to with parentheses or rewrite as docstatus==1 &
(paid_from==account | paid_to==account) & posting_date...).
- Around line 236-265: In get_pos_entries, the payment_document is incorrectly
set to the child DocType "Sales Invoice Payment" and payment_entry uses
si_payment.name, which breaks the Dynamic Link; change the ConstantColumn value
to "Sales Invoice" for payment_document and make payment_entry use si.name (the
parent Sales Invoice) so the link points to the Sales Invoice record; update the
select clause in get_pos_entries accordingly (reference:
ConstantColumn(...).as_("payment_document"),
si_payment.name.as_("payment_entry") -> si.name.as_("payment_entry")).

[codecov]

Codecov Report

❌ Patch coverage is 43.93939% with 37 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.38%. Comparing base (3732348) to head (78ac3f5).
⚠️ Report is 8 commits behind head on develop.

Files with missing lines	Patch %	Lines
...liation_statement/bank_reconciliation_statement.py	16.66%	25 Missing ⚠️
...t/bank_clearance_summary/bank_clearance_summary.py	0.00%	12 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop   #52675      +/-   ##
===========================================
- Coverage    79.39%   79.38%   -0.01%     
===========================================
  Files         1169     1169              
  Lines       123573   123607      +34     
===========================================
+ Hits         98111    98131      +20     
- Misses       25462    25476      +14     

Files with missing lines	Coverage Δ
.../accounts/doctype/bank_clearance/bank_clearance.py	79.46% <100.00%> (+2.69%)	⬆️
...t/bank_clearance_summary/bank_clearance_summary.py	0.00% <0.00%> (ø)
...liation_statement/bank_reconciliation_statement.py	24.67% <16.66%> (-0.33%)	⬇️

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[coderabbitai]

Actionable comments posted: 5

🤖 Fix all issues with AI agents

In `@erpnext/accounts/doctype/bank_clearance/bank_clearance.py`:
- Around line 291-304: The select for paid_purchase_invoices_query is missing
the cheque/reference field; add pi.bill_no mapped to the expected output column
(e.g. pi.bill_no.as_("cheque_number")) to the .select(...) list so Purchase
Invoice rows include the cheque_number like the other reports; update the select
in paid_purchase_invoices_query where pi and acc are referenced to include this
new column.

In `@erpnext/accounts/report/bank_clearance_summary/bank_clearance_summary.py`:
- Around line 111-116: The Case() expression handling credit vs debit is
asymmetric: the when branch uses ((pe.paid_amount * -1) -
pe.total_taxes_and_charges) but the else_ branch returns pe.received_amount
alone; update the else_ branch to include taxes (e.g., pe.received_amount +
pe.total_taxes_and_charges or the appropriate currency-aware variant) so
incoming payments reflect taxes consistently—modify the Case().when/else_
expression referencing pe.paid_from, pe.paid_amount, pe.total_taxes_and_charges,
and pe.received_amount accordingly.

In
`@erpnext/accounts/report/bank_reconciliation_statement/bank_reconciliation_statement.py`:
- Around line 323-338: The Purchase Invoice aggregation (pi, pi_amount) for
"amounts not reflected" is missing the paid filter; update the where clause of
the frappe.qb.from_(pi).select(...).where(...) used to compute pi_amount to
include (pi.is_paid == 1) alongside the existing predicates so only paid
Purchase Invoices are summed (match other uses like
get_purchase_invoices/bank_clearance references).
- Around line 304-321: The companion query that computes pe_amount uses
pe.paid_amount and pe.received_amount but should mirror get_payment_entries' use
of pe.paid_amount_after_tax and pe.received_amount_after_tax; update the
Sum(Case()...) expression in the pe_amount query to reference
pe.paid_amount_after_tax and pe.received_amount_after_tax (keeping the same
Case/when logic and filters) so the "amounts not reflected" calculation matches
the payment entry rows used by get_payment_entries.
- Around line 180-181: The amounts-not-reflected query in
get_amounts_not_reflected_in_system_for_bank_reconciliation_statement is using
paid_amount and received_amount (untaxed) while the payment entry queries use
paid_amount_after_tax and received_amount_after_tax; update that function so the
SELECT/Cases use paid_amount_after_tax and received_amount_after_tax (and any
aliases referencing those columns) to ensure both queries use the same
tax-inclusive bases for debit/credit calculations (search for
get_amounts_not_reflected_in_system_for_bank_reconciliation_statement and
replace paid_amount/received_amount usages with their _after_tax counterparts).

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

erpnext/accounts/doctype/bank_clearance/bank_clearance.py (1)

187-224: Non-aggregated columns in GROUP BY query may return indeterminate values.

against_account (line 202) and account_currency (line 204) are selected without aggregation but are not part of the GROUP BY clause (line 221). MariaDB will silently pick an arbitrary row value, which is usually fine in practice (a JE rarely has duplicate lines on the same account), but it's technically non-deterministic and would fail under MySQL's ONLY_FULL_GROUP_BY mode.

Consider wrapping them with an aggregate or adding them to the group-by:

Proposed fix (add to groupby)

-	journal_entry_query.groupby(journal_entry_account.account, journal_entry.name)
+	journal_entry_query.groupby(journal_entry_account.account, journal_entry.name, journal_entry_account.against_account, journal_entry_account.account_currency)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@erpnext/accounts/doctype/bank_clearance/bank_clearance.py` around lines 187 -
224, The GROUP BY on journal_entry_query currently groups by
journal_entry_account.account and journal_entry.name but selects non-aggregated
columns journal_entry_account.against_account and
journal_entry_account.account_currency (and others like posting_date/cheque
fields), which is non-deterministic; fix it by adding
journal_entry_account.against_account and journal_entry_account.account_currency
to the groupby call (i.e., extend the groupby arguments on journal_entry_query)
or alternatively wrap those selects with an aggregate/ANY_VALUE, updating the
journal_entry_query.groupby(...) invocation accordingly.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@erpnext/accounts/report/bank_clearance_summary/bank_clearance_summary.py`:
- Around line 91-96: The journal entry query in bank_clearance_summary is
missing the is_opening filter, so add a predicate to the existing .where clause
to exclude opening entries (e.g., include (jea.is_opening == "No") alongside
(jea.account == filters.account), (je.docstatus == 1), (je.posting_date >=
filters.from_date), and (je.posting_date <= filters.to_date)) to match the
behavior in bank_clearance.py and bank_reconciliation_statement.py; modify the
query that builds on the jea alias to include this filter.

---

Duplicate comments:
In `@erpnext/accounts/report/bank_clearance_summary/bank_clearance_summary.py`:
- Around line 111-116: The debit branch returned by the Case().else_ currently
omits bank charges (pe.total_taxes_and_charges), causing incoming payments to be
understated; update the expression so the else branch includes taxes (replace
.else_(pe.received_amount) with .else_(pe.received_amount +
pe.total_taxes_and_charges)) so both branches account for
pe.total_taxes_and_charges alongside pe.paid_amount/pe.received_amount when
computing the value.

---

Nitpick comments:
In `@erpnext/accounts/doctype/bank_clearance/bank_clearance.py`:
- Around line 187-224: The GROUP BY on journal_entry_query currently groups by
journal_entry_account.account and journal_entry.name but selects non-aggregated
columns journal_entry_account.against_account and
journal_entry_account.account_currency (and others like posting_date/cheque
fields), which is non-deterministic; fix it by adding
journal_entry_account.against_account and journal_entry_account.account_currency
to the groupby call (i.e., extend the groupby arguments on journal_entry_query)
or alternatively wrap those selects with an aggregate/ANY_VALUE, updating the
journal_entry_query.groupby(...) invocation accordingly.

[nikkothari22]

@ruthra-kumar Good to go?