mirror of
https://github.com/frappe/erpnext.git
synced 2026-08-21 18:29:57 +00:00
fix(accounts): Journal Entry future payments mis-allocated to one invoice in AR
get_future_payments_from_journal_entry has no GROUP BY: the Sum() makes it an implicit single-group aggregate, so every future-dated JE payment company-wide collapses into ONE row keyed by an arbitrary (invoice, party). The Accounts Receivable report then allocates the entire future sum against that one invoice and shows zero future payment for every other invoice. This predates the postgres work (MariaDB returned an arbitrary single row); the qb conversion only made the arbitrary pick deterministic via Max(). Add an explicit GROUP BY (je.name, jea.reference_name, jea.party, jea.party_type, je.posting_date, je.cheque_no) and drop the Max() wrappers, so each (JE, invoice, party) is its own future-payment row -- matching the Payment Entry path and the (invoice_no, party) keying the report's allocator already expects. Identical on MariaDB and PostgreSQL. Ships a JE-path future-payment test (one future JE paying two invoices -> each invoice keeps its own future amount; fails on the old code with 0 != 50). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -7,7 +7,7 @@ from collections import OrderedDict
|
||||
import frappe
|
||||
from frappe import _, qb, query_builder, scrub
|
||||
from frappe.query_builder import Criterion
|
||||
from frappe.query_builder.functions import Date, Max, Substring, Sum
|
||||
from frappe.query_builder.functions import Date, Substring, Sum
|
||||
from frappe.utils import cint, cstr, flt, getdate, nowdate
|
||||
|
||||
from erpnext.accounts.doctype.accounting_dimension.accounting_dimension import (
|
||||
@@ -691,13 +691,11 @@ class ReceivablePayableReport:
|
||||
.inner_join(jea)
|
||||
.on(jea.parent == je.name)
|
||||
.select(
|
||||
# Sum() below makes this an implicit aggregate (no GROUP BY); the non-aggregated columns
|
||||
# are arbitrary per the single group on MySQL -> Max() keeps it valid on postgres.
|
||||
Max(jea.reference_name).as_("invoice_no"),
|
||||
Max(jea.party).as_("party"),
|
||||
Max(jea.party_type).as_("party_type"),
|
||||
Max(je.posting_date).as_("future_date"),
|
||||
Max(je.cheque_no).as_("future_ref"),
|
||||
jea.reference_name.as_("invoice_no"),
|
||||
jea.party,
|
||||
jea.party_type,
|
||||
je.posting_date.as_("future_date"),
|
||||
je.cheque_no.as_("future_ref"),
|
||||
)
|
||||
.where(
|
||||
(je.docstatus < 2)
|
||||
@@ -727,6 +725,14 @@ class ReceivablePayableReport:
|
||||
future_amount.as_("future_amount"),
|
||||
future_amount_in_base_currency.as_("future_amount_in_base_currency"),
|
||||
)
|
||||
# One row per (future-payment JE, invoice, party): group by the JE name (primary key, so the
|
||||
# JE-level posting_date/cheque_no are deterministic) plus the per-reference dimensions, summing
|
||||
# amounts across JE Account rows that hit the same invoice. Without this GROUP BY the implicit
|
||||
# single-group aggregate collapsed every future JE payment into one row keyed by an arbitrary
|
||||
# invoice, mis-allocating the whole sum.
|
||||
query = query.groupby(
|
||||
je.name, jea.reference_name, jea.party, jea.party_type, je.posting_date, je.cheque_no
|
||||
)
|
||||
# use the aggregate expression in HAVING; postgres can't reference a SELECT alias there
|
||||
query = query.having(future_amount > 0)
|
||||
return query.run(as_dict=True)
|
||||
|
||||
@@ -699,6 +699,61 @@ class TestAccountsReceivable(ERPNextTestSuite, AccountsTestMixin):
|
||||
[row.invoiced, row.paid, row.outstanding, row.remaining_balance, row.future_amount],
|
||||
)
|
||||
|
||||
def test_future_payments_from_journal_entry(self):
|
||||
# A single future-dated Journal Entry paying two different invoices must surface as one
|
||||
# future-payment row PER invoice, not collapse the whole sum onto one arbitrary invoice
|
||||
# (regression: the implicit single-group aggregate filed all future JE payments under one key).
|
||||
si_a = self.create_sales_invoice(no_payment_schedule=True)
|
||||
si_b = self.create_sales_invoice(no_payment_schedule=True)
|
||||
|
||||
je = frappe.get_doc(
|
||||
{
|
||||
"doctype": "Journal Entry",
|
||||
"voucher_type": "Journal Entry",
|
||||
"company": self.company,
|
||||
"posting_date": add_days(today(), 1),
|
||||
"accounts": [
|
||||
{
|
||||
"account": self.debit_to,
|
||||
"party_type": "Customer",
|
||||
"party": self.customer,
|
||||
"reference_type": "Sales Invoice",
|
||||
"reference_name": si_a.name,
|
||||
"credit_in_account_currency": 50,
|
||||
"credit": 50,
|
||||
},
|
||||
{
|
||||
"account": self.debit_to,
|
||||
"party_type": "Customer",
|
||||
"party": self.customer,
|
||||
"reference_type": "Sales Invoice",
|
||||
"reference_name": si_b.name,
|
||||
"credit_in_account_currency": 50,
|
||||
"credit": 50,
|
||||
},
|
||||
{"account": self.cash, "debit_in_account_currency": 100, "debit": 100},
|
||||
],
|
||||
}
|
||||
)
|
||||
je.insert().submit()
|
||||
|
||||
filters = {
|
||||
"company": self.company,
|
||||
"report_date": today(),
|
||||
"range": "30, 60, 90, 120",
|
||||
"show_future_payments": True,
|
||||
}
|
||||
report = execute(filters)[1]
|
||||
rows_a = [row for row in report if row.voucher_no == si_a.name]
|
||||
rows_b = [row for row in report if row.voucher_no == si_b.name]
|
||||
|
||||
# exactly one report row per invoice, each keeping its own future payment; the bug collapsed
|
||||
# both into a single row and allocated the whole 100 to one arbitrary invoice
|
||||
self.assertEqual(len(rows_a), 1)
|
||||
self.assertEqual(len(rows_b), 1)
|
||||
self.assertEqual(rows_a[0].future_amount, 50.0)
|
||||
self.assertEqual(rows_b[0].future_amount, 50.0)
|
||||
|
||||
def test_sales_person(self):
|
||||
sales_person = frappe.get_doc(
|
||||
{"doctype": "Sales Person", "sales_person_name": "John Clark", "enabled": True}
|
||||
|
||||
Reference in New Issue
Block a user