Skip to content

Conversation

@vorasmit
Copy link
Collaborator

@vorasmit vorasmit commented Nov 27, 2025

Issue

  • Total debit and credit in opening balance in GL is not useful
  • Non-standard practice

Fixes

  • Always show net balance as opening

Before

image

After

image

TODOs for later

  • Optimize query to fetch opening balance
  • Smarter opening balance for Income / Expense Account (optional): use only current fiscal year entries to calculate opening balance

@github-actions github-actions bot added the needs-tests This PR needs automated unit-tests. label Nov 27, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 27, 2025

📝 Walkthrough

Walkthrough

The change modifies the update_value_in_dict helper function in the general ledger report to accept a new show_net_values parameter (default False). The net-value computation logic is updated to trigger when either this new parameter is true or when the existing show_net_values_in_party_account filter applies to Receivable/Payable accounts. All calls to this function for opening and closing totals are updated to pass True for the new parameter, ensuring net-value calculations apply to those entries when appropriate.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Focus on the conditional logic for net-value computation to ensure the revised conditions (new parameter OR existing filter with account type check) are correct and don't introduce regressions
  • Verify all calls to update_value_in_dict appropriately pass True for opening/closing scenarios
  • Validate that the default False value doesn't alter behavior for other code paths that don't explicitly pass the parameter

Suggested reviewers

  • ruthra-kumar
  • diptanilsaha

Pre-merge checks and finishing touches

❌ 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%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: switching to always show net balance as opening in the general ledger, which directly matches the code modifications and objectives.
Description check ✅ Passed The PR description clearly explains the issue (total debit/credit in opening balance is not useful), the fix (always show net balance), and includes before/after screenshots demonstrating the change.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 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

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

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cf449d8 and 2494ee4.

📒 Files selected for processing (4)
  • erpnext/accounts/doctype/process_statement_of_accounts/process_statement_of_accounts.json (1 hunks)
  • erpnext/accounts/doctype/process_statement_of_accounts/process_statement_of_accounts.py (0 hunks)
  • erpnext/accounts/report/general_ledger/general_ledger.js (0 hunks)
  • erpnext/accounts/report/general_ledger/general_ledger.py (2 hunks)
💤 Files with no reviewable changes (2)
  • erpnext/accounts/report/general_ledger/general_ledger.js
  • erpnext/accounts/doctype/process_statement_of_accounts/process_statement_of_accounts.py
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-09-26T08:52:41.228Z
Learnt from: diptanilsaha
Repo: frappe/erpnext PR: 49712
File: erpnext/accounts/report/financial_statements.py:531-539
Timestamp: 2025-09-26T08:52:41.228Z
Learning: The Account Closing Balance doctype includes debit_in_reporting_currency and credit_in_reporting_currency fields, making it compatible with reporting currency queries in financial statements.

Applied to files:

  • erpnext/accounts/report/general_ledger/general_ledger.py
📚 Learning: 2025-08-29T12:08:57.357Z
Learnt from: diptanilsaha
Repo: frappe/erpnext PR: 49373
File: erpnext/accounts/doctype/process_statement_of_accounts/process_statement_of_accounts_accounts_receivable.html:86-96
Timestamp: 2025-08-29T12:08:57.357Z
Learning: In the Process Statement of Accounts accounts receivable template, the summary table that appears when show_future_payments is enabled correctly includes an "Age" column header that displays aggregate age metrics alongside the aging bucket totals. This alignment is intentional and should not be removed.

Applied to files:

  • erpnext/accounts/doctype/process_statement_of_accounts/process_statement_of_accounts.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
  • GitHub Check: Python Unit Tests (4)
  • GitHub Check: Python Unit Tests (3)
  • GitHub Check: Python Unit Tests (2)
  • GitHub Check: Python Unit Tests (1)
  • GitHub Check: Patch Test
  • GitHub Check: Summary
🔇 Additional comments (2)
erpnext/accounts/doctype/process_statement_of_accounts/process_statement_of_accounts.json (1)

412-412: LGTM! DocType metadata updated correctly.

The timestamp reflects the removal of the show_net_values_in_party_account field, which aligns with the PR objective to always show net balance as opening balance instead of allowing selective display of totals.

erpnext/accounts/report/general_ledger/general_ledger.py (1)

511-526: Unable to verify codebase due to infrastructure limitations.

The repository cloning is failing in the sandbox environment, preventing verification of:

  • The actual code at lines 511-526 matching the provided snippet
  • References to show_net_values_in_party_account that may have been removed
  • Downstream code paths that might depend on gross debit/credit values
  • Related tests for general ledger opening/closing balances

The review comment's analysis of the net value calculation logic (debit - credit, sign-based field assignment, zeroing opposite field) is mathematically sound, but cannot be verified against the actual codebase state. Manual verification is recommended to confirm:

  1. No remaining references to the removed flag
  2. Tests pass with this breaking change
  3. Downstream reports/integrations don't break

@vorasmit vorasmit added backport version-15-hotfix backport version-16-beta Backport to the Version 16 Beta branch labels Nov 27, 2025
@ruthra-kumar ruthra-kumar self-assigned this Nov 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport version-15-hotfix backport version-16-beta Backport to the Version 16 Beta branch needs-tests This PR needs automated unit-tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants