From ee2f8d8ebce94dc3588e46413232ce661ffeb3c8 Mon Sep 17 00:00:00 2001 From: Smit Vora Date: Tue, 3 Feb 2026 15:21:38 +0530 Subject: [PATCH 1/6] fix: correctly calculate running balances for financial report --- .../financial_report_engine.py | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/erpnext/accounts/doctype/financial_report_template/financial_report_engine.py b/erpnext/accounts/doctype/financial_report_template/financial_report_engine.py index 4de556b7e46..ce8ed4adb07 100644 --- a/erpnext/accounts/doctype/financial_report_template/financial_report_engine.py +++ b/erpnext/accounts/doctype/financial_report_template/financial_report_engine.py @@ -636,12 +636,15 @@ class FinancialQueryBuilder: return self._execute_with_permissions(query, "GL Entry") def _calculate_running_balances(self, balances_data: dict, gl_data: list[dict]) -> dict: - for row in gl_data: - account = row["account"] + gl_dict = {row["account"]: row for row in gl_data} + accounts = set(balances_data.keys()) | set(gl_dict.keys()) + + for account in accounts: if account not in balances_data: balances_data[account] = AccountData(account=account, **self._get_account_meta(account)) account_data: AccountData = balances_data[account] + gl_movement = gl_dict.get(account, {}) if account_data.has_periods(): first_period = account_data.get_period(self.periods[0]["key"]) @@ -651,20 +654,13 @@ class FinancialQueryBuilder: for period in self.periods: period_key = period["key"] - movement = row.get(period_key, 0.0) + movement = gl_movement.get(period_key, 0.0) closing_balance = current_balance + movement account_data.add_period(PeriodValue(period_key, current_balance, closing_balance, movement)) current_balance = closing_balance - # Accounts with no movements - for account_data in balances_data.values(): - for period in self.periods: - period_key = period["key"] - if period_key not in account_data.period_values: - account_data.add_period(PeriodValue(period_key, 0.0, 0.0, 0.0)) - def _handle_balance_accumulation(self, balances_data): for account_data in balances_data.values(): account_data: AccountData From b41c1858a39f98c5576bbaedfdf0354d4588ff48 Mon Sep 17 00:00:00 2001 From: Smit Vora Date: Tue, 3 Feb 2026 16:03:14 +0530 Subject: [PATCH 2/6] fix: Period Closing Voucher doesn't exist for GL Entry --- .../financial_report_engine.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/erpnext/accounts/doctype/financial_report_template/financial_report_engine.py b/erpnext/accounts/doctype/financial_report_template/financial_report_engine.py index ce8ed4adb07..216a9034be4 100644 --- a/erpnext/accounts/doctype/financial_report_template/financial_report_engine.py +++ b/erpnext/accounts/doctype/financial_report_template/financial_report_engine.py @@ -541,7 +541,7 @@ class FinancialQueryBuilder: .where(acb_table.period_closing_voucher == closing_voucher) ) - query = self._apply_standard_filters(query, acb_table) + query = self._apply_standard_filters(query, acb_table, "Account Closing Balance") results = self._execute_with_permissions(query, "Account Closing Balance") for row in results: @@ -679,12 +679,12 @@ class FinancialQueryBuilder: else: account_data.unaccumulate_values() - def _apply_standard_filters(self, query, table): + def _apply_standard_filters(self, query, table, doctype: str = "GL Entry"): if self.filters.get("ignore_closing_entries"): - if hasattr(table, "is_period_closing_voucher_entry"): - query = query.where(table.is_period_closing_voucher_entry == 0) - else: + if doctype == "GL Entry": query = query.where(table.voucher_type != "Period Closing Voucher") + else: + query = query.where(table.is_period_closing_voucher_entry == 0) if self.filters.get("project"): projects = self.filters.get("project") From 61d8308e81cba62774726a13babe31fda84bcbd9 Mon Sep 17 00:00:00 2001 From: Smit Vora Date: Tue, 3 Feb 2026 16:18:27 +0530 Subject: [PATCH 3/6] test: add tests for query builder --- .../test_financial_report_engine.py | 278 +++++++++++++++++- 1 file changed, 277 insertions(+), 1 deletion(-) diff --git a/erpnext/accounts/doctype/financial_report_template/test_financial_report_engine.py b/erpnext/accounts/doctype/financial_report_template/test_financial_report_engine.py index c0b0816df70..7bc6cdd4c79 100644 --- a/erpnext/accounts/doctype/financial_report_template/test_financial_report_engine.py +++ b/erpnext/accounts/doctype/financial_report_template/test_financial_report_engine.py @@ -7,12 +7,14 @@ from frappe.utils import flt from erpnext.accounts.doctype.financial_report_template.financial_report_engine import ( DependencyResolver, FilterExpressionParser, + FinancialQueryBuilder, FormulaCalculator, ) from erpnext.accounts.doctype.financial_report_template.test_financial_report_template import ( FinancialReportTemplateTestCase, ) -from erpnext.accounts.utils import get_currency_precision +from erpnext.accounts.doctype.journal_entry.test_journal_entry import make_journal_entry +from erpnext.accounts.utils import get_currency_precision, get_fiscal_year # On IntegrationTestCase, the doctype test records and all # link-field test record dependencies are recursively loaded @@ -1668,3 +1670,277 @@ class TestFilterExpressionParser(FinancialReportTemplateTestCase): mock_row_invalid = self._create_mock_report_row(invalid_formula) condition = parser.build_condition(mock_row_invalid, account_table) self.assertIsNone(condition) + + +class TestFinancialQueryBuilder(FinancialReportTemplateTestCase): + def test_fetch_balances_with_journal_entries(self): + company = "_Test Company" + cash_account = "_Test Cash - _TC" + bank_account = "_Test Bank - _TC" + + # Create journal entries in different periods + # October: Transfer 1000 from Bank to Cash + jv_oct = make_journal_entry( + account1=cash_account, + account2=bank_account, + amount=1000, + posting_date="2024-10-15", + company=company, + submit=True, + ) + + # November: Transfer 500 from Bank to Cash + jv_nov = make_journal_entry( + account1=cash_account, + account2=bank_account, + amount=500, + posting_date="2024-11-20", + company=company, + submit=True, + ) + + # December: No transactions (test zero movement period) + + try: + # Set up filters and periods for Q4 2024 + filters = { + "company": company, + "from_fiscal_year": "2024", + "to_fiscal_year": "2024", + "period_start_date": "2024-10-01", + "period_end_date": "2024-12-31", + "filter_based_on": "Date Range", + "periodicity": "Monthly", + } + + periods = [ + {"key": "2024_oct", "from_date": "2024-10-01", "to_date": "2024-10-31"}, + {"key": "2024_nov", "from_date": "2024-11-01", "to_date": "2024-11-30"}, + {"key": "2024_dec", "from_date": "2024-12-01", "to_date": "2024-12-31"}, + ] + + query_builder = FinancialQueryBuilder(filters, periods) + + # Create account objects as expected by fetch_account_balances + accounts = [ + frappe._dict({"name": cash_account, "account_name": "Cash", "account_number": "1001"}), + frappe._dict({"name": bank_account, "account_name": "Bank", "account_number": "1002"}), + ] + + # Fetch balances using the full workflow + balances_data = query_builder.fetch_account_balances(accounts) + + # Verify Cash account balances + cash_data = balances_data.get(cash_account) + self.assertIsNotNone(cash_data, "Cash account should exist in results") + + # October: movement = +1000 (debit) + oct_cash = cash_data.get_period("2024_oct") + self.assertIsNotNone(oct_cash, "October period should exist for cash") + self.assertEqual(oct_cash.movement, 1000.0, "October cash movement should be 1000") + + # November: movement = +500 + nov_cash = cash_data.get_period("2024_nov") + self.assertIsNotNone(nov_cash, "November period should exist for cash") + self.assertEqual(nov_cash.movement, 500.0, "November cash movement should be 500") + self.assertEqual( + nov_cash.opening, oct_cash.closing, "November opening should equal October closing" + ) + + # December: movement = 0 (no transactions) + dec_cash = cash_data.get_period("2024_dec") + self.assertIsNotNone(dec_cash, "December period should exist for cash") + self.assertEqual(dec_cash.movement, 0.0, "December cash movement should be 0") + self.assertEqual( + dec_cash.closing, + nov_cash.closing, + "December closing should equal November closing when no movement", + ) + + # Verify Bank account balances (opposite direction) + bank_data = balances_data.get(bank_account) + self.assertIsNotNone(bank_data, "Bank account should exist in results") + + oct_bank = bank_data.get_period("2024_oct") + self.assertEqual(oct_bank.movement, -1000.0, "October bank movement should be -1000") + + nov_bank = bank_data.get_period("2024_nov") + self.assertEqual(nov_bank.movement, -500.0, "November bank movement should be -500") + + finally: + # Clean up: cancel journal entries + jv_nov.cancel() + jv_oct.cancel() + + def test_opening_balance_from_previous_period_closing(self): + company = "_Test Company" + cash_account = "_Test Cash - _TC" + sales_account = "Sales - _TC" + posting_date_2023 = "2023-06-15" + + # Create journal entry in prior period (2023) + # Cash Dr 5000, Sales Cr 5000 + jv_2023 = make_journal_entry( + account1=cash_account, + account2=sales_account, + amount=5000, + posting_date=posting_date_2023, + company=company, + submit=True, + ) + + pcv = None + jv_2024 = None + + try: + # Create Period Closing Voucher for 2023 + # This will create Account Closing Balance entries + closing_account = frappe.db.get_value( + "Account", + { + "company": company, + "root_type": "Liability", + "is_group": 0, + "account_type": ["not in", ["Payable", "Receivable"]], + }, + "name", + ) + + fy_2023 = get_fiscal_year(posting_date_2023, company=company) + + frappe.db.set_single_value("Accounts Settings", "use_legacy_controller_for_pcv", 1) + + pcv = frappe.get_doc( + { + "doctype": "Period Closing Voucher", + "transaction_date": "2023-12-31", + "period_start_date": fy_2023[1], + "period_end_date": fy_2023[2], + "company": company, + "fiscal_year": fy_2023[0], + "cost_center": "_Test Cost Center - _TC", + "closing_account_head": closing_account, + "remarks": "Test Period Closing", + } + ) + pcv.insert() + pcv.submit() + pcv.reload() + + # Now create a small transaction in 2024 to ensure the account appears + jv_2024 = make_journal_entry( + account1=cash_account, + account2=sales_account, + amount=100, + posting_date="2024-01-15", + company=company, + submit=True, + ) + + # Set up filters for Q1 2024 (after the period closing) + filters = { + "company": company, + "from_fiscal_year": "2024", + "to_fiscal_year": "2024", + "period_start_date": "2024-01-01", + "period_end_date": "2024-03-31", + "filter_based_on": "Date Range", + "periodicity": "Monthly", + "ignore_closing_entries": True, # Don't include PCV entries in movements + } + + periods = [ + {"key": "2024_jan", "from_date": "2024-01-01", "to_date": "2024-01-31"}, + {"key": "2024_feb", "from_date": "2024-02-01", "to_date": "2024-02-29"}, + {"key": "2024_mar", "from_date": "2024-03-01", "to_date": "2024-03-31"}, + ] + + query_builder = FinancialQueryBuilder(filters, periods) + + accounts = [ + frappe._dict({"name": cash_account, "account_name": "Cash", "account_number": "1001"}), + ] + + balances_data = query_builder.fetch_account_balances(accounts) + + # Verify Cash account has opening balance from 2023 transactions + cash_data = balances_data.get(cash_account) + self.assertIsNotNone(cash_data, "Cash account should exist in results") + + jan_cash = cash_data.get_period("2024_jan") + self.assertIsNotNone(jan_cash, "January period should exist") + + # Opening balance should be from prior period + # Cash had 5000 debit in 2023, so opening in 2024 should be >= 5000 + # (may be higher if there were other test transactions) + self.assertEqual( + jan_cash.opening, + 5000.0, + "January opening should include balance from 2023 (5000 or more)", + ) + + # Verify running balance logic + # Movement in January is 100 (from jv_2024) + self.assertEqual(jan_cash.movement, 100.0, "January movement should be 100") + self.assertEqual( + jan_cash.closing, jan_cash.opening + jan_cash.movement, "Closing = Opening + Movement" + ) + + # February and March should have no movement but carry the balance + feb_cash = cash_data.get_period("2024_feb") + self.assertEqual(feb_cash.opening, jan_cash.closing, "Feb opening = Jan closing") + self.assertEqual(feb_cash.movement, 0.0, "February should have no movement") + self.assertEqual(feb_cash.closing, feb_cash.opening, "Feb closing = opening when no movement") + + mar_cash = cash_data.get_period("2024_mar") + self.assertEqual(mar_cash.opening, feb_cash.closing, "Mar opening = Feb closing") + self.assertEqual(mar_cash.movement, 0.0, "March should have no movement") + self.assertEqual(mar_cash.closing, mar_cash.opening, "Mar closing = opening when no movement") + + # Set up filters for Q2 2024 + filters_q2 = { + "company": company, + "from_fiscal_year": "2024", + "to_fiscal_year": "2024", + "period_start_date": "2024-04-01", + "period_end_date": "2024-06-30", + "filter_based_on": "Date Range", + "periodicity": "Monthly", + "ignore_closing_entries": True, + } + + periods_q2 = [ + {"key": "2024_apr", "from_date": "2024-04-01", "to_date": "2024-04-30"}, + {"key": "2024_may", "from_date": "2024-05-01", "to_date": "2024-05-31"}, + {"key": "2024_jun", "from_date": "2024-06-01", "to_date": "2024-06-30"}, + ] + + query_builder_q2 = FinancialQueryBuilder(filters_q2, periods_q2) + + balances_data_q2 = query_builder_q2.fetch_account_balances(accounts) + + # Verify Cash account in Q2 + cash_data_q2 = balances_data_q2.get(cash_account) + self.assertIsNotNone(cash_data_q2, "Cash account should exist in Q2 results") + + apr_cash = cash_data_q2.get_period("2024_apr") + self.assertIsNotNone(apr_cash, "April period should exist") + + # Opening balance in April should equal closing in March + self.assertEqual( + apr_cash.opening, + mar_cash.closing, + "April opening should equal March closing balance", + ) + + finally: + # Clean up + if jv_2024: + jv_2024.cancel() + + if pcv: + pcv.reload() + if pcv.docstatus == 1: + pcv.cancel() + + jv_2023.cancel() From f45a5a63a7dd8dc27f3162faba35456743960241 Mon Sep 17 00:00:00 2001 From: Smit Vora Date: Tue, 3 Feb 2026 16:30:05 +0530 Subject: [PATCH 4/6] test: revert original pcv setting --- .../test_financial_report_engine.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/erpnext/accounts/doctype/financial_report_template/test_financial_report_engine.py b/erpnext/accounts/doctype/financial_report_template/test_financial_report_engine.py index 7bc6cdd4c79..ddc6924bd6d 100644 --- a/erpnext/accounts/doctype/financial_report_template/test_financial_report_engine.py +++ b/erpnext/accounts/doctype/financial_report_template/test_financial_report_engine.py @@ -1791,6 +1791,9 @@ class TestFinancialQueryBuilder(FinancialReportTemplateTestCase): pcv = None jv_2024 = None + original_pcv_setting = frappe.db.get_single_value( + "Accounts Settings", "use_legacy_controller_for_pcv" + ) try: # Create Period Closing Voucher for 2023 @@ -1935,6 +1938,10 @@ class TestFinancialQueryBuilder(FinancialReportTemplateTestCase): finally: # Clean up + frappe.db.set_single_value( + "Accounts Settings", "use_legacy_controller_for_pcv", original_pcv_setting or 0 + ) + if jv_2024: jv_2024.cancel() From a29710dc07c2cc782abbedbc3691ec195e4b1e28 Mon Sep 17 00:00:00 2001 From: Smit Vora Date: Tue, 3 Feb 2026 16:32:26 +0530 Subject: [PATCH 5/6] test: correct error message --- .../financial_report_template/test_financial_report_engine.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/erpnext/accounts/doctype/financial_report_template/test_financial_report_engine.py b/erpnext/accounts/doctype/financial_report_template/test_financial_report_engine.py index ddc6924bd6d..b496f441e2f 100644 --- a/erpnext/accounts/doctype/financial_report_template/test_financial_report_engine.py +++ b/erpnext/accounts/doctype/financial_report_template/test_financial_report_engine.py @@ -1879,7 +1879,7 @@ class TestFinancialQueryBuilder(FinancialReportTemplateTestCase): self.assertEqual( jan_cash.opening, 5000.0, - "January opening should include balance from 2023 (5000 or more)", + "January opening should equal to balance from 2023 (5000)", ) # Verify running balance logic From 12f8bb2937e16c1063559b74e40b73f616f1a450 Mon Sep 17 00:00:00 2001 From: Smit Vora Date: Tue, 3 Feb 2026 18:11:34 +0530 Subject: [PATCH 6/6] test: further tests for query builder --- .../test_financial_report_engine.py | 76 +++++++++++++++++++ 1 file changed, 76 insertions(+) diff --git a/erpnext/accounts/doctype/financial_report_template/test_financial_report_engine.py b/erpnext/accounts/doctype/financial_report_template/test_financial_report_engine.py index b496f441e2f..e65e3f185a0 100644 --- a/erpnext/accounts/doctype/financial_report_template/test_financial_report_engine.py +++ b/erpnext/accounts/doctype/financial_report_template/test_financial_report_engine.py @@ -1936,6 +1936,8 @@ class TestFinancialQueryBuilder(FinancialReportTemplateTestCase): "April opening should equal March closing balance", ) + self.assertEqual(apr_cash.closing, apr_cash.opening, "April closing = opening when no movement") + finally: # Clean up frappe.db.set_single_value( @@ -1951,3 +1953,77 @@ class TestFinancialQueryBuilder(FinancialReportTemplateTestCase): pcv.cancel() jv_2023.cancel() + + def test_account_with_gl_entries_but_no_prior_closing_balance(self): + company = "_Test Company" + cash_account = "_Test Cash - _TC" + bank_account = "_Test Bank - _TC" + + # Create journal entries WITHOUT any prior Period Closing Voucher + # This ensures the account exists in gl_dict but NOT in balances_data + jv = make_journal_entry( + account1=cash_account, + account2=bank_account, + amount=2500, + posting_date="2024-07-15", + company=company, + submit=True, + ) + + try: + # Set up filters - use a period with no prior PCV + filters = { + "company": company, + "from_fiscal_year": "2024", + "to_fiscal_year": "2024", + "period_start_date": "2024-07-01", + "period_end_date": "2024-09-30", + "filter_based_on": "Date Range", + "periodicity": "Monthly", + } + + periods = [ + {"key": "2024_jul", "from_date": "2024-07-01", "to_date": "2024-07-31"}, + {"key": "2024_aug", "from_date": "2024-08-01", "to_date": "2024-08-31"}, + {"key": "2024_sep", "from_date": "2024-09-01", "to_date": "2024-09-30"}, + ] + + query_builder = FinancialQueryBuilder(filters, periods) + + # Use accounts that have GL entries but may not have Account Closing Balance + accounts = [ + frappe._dict({"name": cash_account, "account_name": "Cash", "account_number": "1001"}), + frappe._dict({"name": bank_account, "account_name": "Bank", "account_number": "1002"}), + ] + + balances_data = query_builder.fetch_account_balances(accounts) + + # Verify accounts are present in results even without prior closing balance + cash_data = balances_data.get(cash_account) + self.assertIsNotNone(cash_data, "Cash account should exist in results") + + bank_data = balances_data.get(bank_account) + self.assertIsNotNone(bank_data, "Bank account should exist in results") + + # Verify July has the movement from journal entry + jul_cash = cash_data.get_period("2024_jul") + self.assertIsNotNone(jul_cash, "July period should exist for cash") + self.assertEqual(jul_cash.movement, 2500.0, "July cash movement should be 2500") + + jul_bank = bank_data.get_period("2024_jul") + self.assertIsNotNone(jul_bank, "July period should exist for bank") + self.assertEqual(jul_bank.movement, -2500.0, "July bank movement should be -2500") + + # Verify subsequent periods exist with zero movement + aug_cash = cash_data.get_period("2024_aug") + self.assertIsNotNone(aug_cash, "August period should exist for cash") + self.assertEqual(aug_cash.movement, 0.0, "August cash movement should be 0") + self.assertEqual(aug_cash.opening, jul_cash.closing, "August opening = July closing") + + sep_cash = cash_data.get_period("2024_sep") + self.assertIsNotNone(sep_cash, "September period should exist for cash") + self.assertEqual(sep_cash.movement, 0.0, "September cash movement should be 0") + self.assertEqual(sep_cash.opening, aug_cash.closing, "September opening = August closing") + + finally: + jv.cancel()