fix: adjust outstanding amount calculation in purchase and sales registers#54597
Draft
PKSowmiya05 wants to merge 2 commits intofrappe:developfrom
Draft
fix: adjust outstanding amount calculation in purchase and sales registers#54597PKSowmiya05 wants to merge 2 commits intofrappe:developfrom
PKSowmiya05 wants to merge 2 commits intofrappe:developfrom
Conversation
PKSowmiya05
changed the title
PKSowmiya05
force-pushed
the
pr-sr-report-oustanding-amount
branch
from
ce382f2 to
0aa3679
Compare
PKSowmiya05
force-pushed
the
pr-sr-report-oustanding-amount
branch
from
0aa3679 to
e2f117a
Compare
Contributor
📝 WalkthroughWalkthroughThe purchase and sales register reports were updated to enhance the calculation and precision handling of outstanding amounts for invoices. Both changes introduce precision derivation based on field metadata from the respective doctype definitions, defaulting to 2 decimal places. The reports now fetch the Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
erpnext/accounts/report/sales_register/sales_register.py (1)
144-150: Move invariant precision lookup outside the invoice loop.
get_field_precision(...)is invariant for the report run; computing it per row adds avoidable overhead on large datasets.♻️ Proposed refactor
- data = [] + outstanding_precision = ( + get_field_precision( + frappe.get_meta("Sales Invoice").get_field("outstanding_amount"), + currency=company_currency, + ) + or 2 + ) + data = [] for inv in invoice_list: @@ - outstanding_precision = ( - get_field_precision( - frappe.get_meta("Sales Invoice").get_field("outstanding_amount"), - currency=company_currency, - ) - or 2 - ) row.update(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@erpnext/accounts/report/sales_register/sales_register.py` around lines 144 - 150, The call to get_field_precision(frappe.get_meta("Sales Invoice").get_field("outstanding_amount"), currency=company_currency) is invariant and should be moved outside the per-invoice loop: compute outstanding_precision once (falling back to 2) before iterating over invoices and then use that variable inside the loop; locate the current assignment to outstanding_precision and the invoice-processing loop (e.g., where invoices are processed/rows built) and remove the per-row call so the loop only references the precomputed outstanding_precision.erpnext/accounts/report/purchase_register/purchase_register.py (1)
130-136: Compute outstanding precision once per execution, not per invoice row.The precision value is constant for the report context and can be initialized before the loop for better scalability.
♻️ Proposed refactor
- data = [] + outstanding_precision = ( + get_field_precision( + frappe.get_meta("Purchase Invoice").get_field("outstanding_amount"), + currency=company_currency, + ) + or 2 + ) + data = [] for inv in invoice_list: @@ - outstanding_precision = ( - get_field_precision( - frappe.get_meta("Purchase Invoice").get_field("outstanding_amount"), - currency=company_currency, - ) - or 2 - ) row.update(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@erpnext/accounts/report/purchase_register/purchase_register.py` around lines 130 - 136, The outstanding precision (outstanding_precision) is being recomputed inside the invoice loop; compute it once before iterating rows by calling get_field_precision(frappe.get_meta("Purchase Invoice").get_field("outstanding_amount"), currency=company_currency) (falling back to 2) and reuse that single value inside the loop where outstanding_amount is formatted; move that expression out of the per-row logic in the report generation function/method that iterates invoices so the loop uses the precomputed outstanding_precision.
🤖 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/purchase_register/purchase_register.py`:
- Around line 146-154: Add a regression test that verifies outstanding_amount
conversion and rounding in purchase_register logic: create foreign-currency
Purchase Invoices with a non-1 conversion_rate and known outstanding_amount,
call the function that builds rows (the code path that executes row.update with
"debit", "credit", "outstanding_amount" in purchase_register.py) and assert that
outstanding_amount equals flt(inv.outstanding_amount * (inv.conversion_rate or
1), outstanding_precision) and that debit/credit are set as expected; name the
test e.g. test_purchase_register_outstanding_conversion and include fixtures for
an invoice in another currency, a specific conversion_rate, and expected
precision values to lock in rounding behavior.
In `@erpnext/accounts/report/sales_register/sales_register.py`:
- Around line 160-168: Add regression tests that exercise the Sales Register
report's outstanding_amount conversion and rounding: create one foreign-currency
invoice with conversion_rate != 1 and one same-currency invoice with
conversion_rate = 1, set known outstanding_amount and outstanding_precision, run
the Sales Register report (the code path that produces row.update({...
"outstanding_amount": flt((inv.outstanding_amount * (inv.conversion_rate or 1)),
outstanding_precision) })) and assert the returned/displayed outstanding_amount
equals the expected (inv.outstanding_amount * conversion_rate) rounded to
outstanding_precision; include assertions for both invoices and an explicit
precision check to ensure the flt and rounding behavior is correct.
---
Nitpick comments:
In `@erpnext/accounts/report/purchase_register/purchase_register.py`:
- Around line 130-136: The outstanding precision (outstanding_precision) is
being recomputed inside the invoice loop; compute it once before iterating rows
by calling get_field_precision(frappe.get_meta("Purchase
Invoice").get_field("outstanding_amount"), currency=company_currency) (falling
back to 2) and reuse that single value inside the loop where outstanding_amount
is formatted; move that expression out of the per-row logic in the report
generation function/method that iterates invoices so the loop uses the
precomputed outstanding_precision.
In `@erpnext/accounts/report/sales_register/sales_register.py`:
- Around line 144-150: The call to get_field_precision(frappe.get_meta("Sales
Invoice").get_field("outstanding_amount"), currency=company_currency) is
invariant and should be moved outside the per-invoice loop: compute
outstanding_precision once (falling back to 2) before iterating over invoices
and then use that variable inside the loop; locate the current assignment to
outstanding_precision and the invoice-processing loop (e.g., where invoices are
processed/rows built) and remove the per-row call so the loop only references
the precomputed outstanding_precision.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 4f743d36-49f3-4b32-ae90-41e9c80ecf72
📒 Files selected for processing (2)
erpnext/accounts/report/purchase_register/purchase_register.pyerpnext/accounts/report/sales_register/sales_register.py
Comment on lines
+146
to
+154
| row.update( | ||
| { | ||
| "debit": inv.base_grand_total, | ||
| "credit": 0.0, | ||
| "outstanding_amount": flt( | ||
| (inv.outstanding_amount * (inv.conversion_rate or 1)), outstanding_precision | ||
| ), | ||
| } | ||
| ) |
Contributor
There was a problem hiding this comment.
Please add regression tests for Purchase Register outstanding conversion.
Line 150-Line 152 changes monetary behavior (conversion + rounding). Add coverage for foreign-currency invoices and precision expectations to prevent regressions.
I can help draft concrete fixtures/assertions if you want.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@erpnext/accounts/report/purchase_register/purchase_register.py` around lines
146 - 154, Add a regression test that verifies outstanding_amount conversion and
rounding in purchase_register logic: create foreign-currency Purchase Invoices
with a non-1 conversion_rate and known outstanding_amount, call the function
that builds rows (the code path that executes row.update with "debit", "credit",
"outstanding_amount" in purchase_register.py) and assert that outstanding_amount
equals flt(inv.outstanding_amount * (inv.conversion_rate or 1),
outstanding_precision) and that debit/credit are set as expected; name the test
e.g. test_purchase_register_outstanding_conversion and include fixtures for an
invoice in another currency, a specific conversion_rate, and expected precision
values to lock in rounding behavior.
Comment on lines
+160
to
+168
| row.update( | ||
| { | ||
| "debit": inv.base_grand_total, | ||
| "credit": 0.0, | ||
| "outstanding_amount": flt( | ||
| (inv.outstanding_amount * (inv.conversion_rate or 1)), outstanding_precision | ||
| ), | ||
| } | ||
| ) |
Contributor
There was a problem hiding this comment.
Add regression tests for multi-currency outstanding conversion in Sales Register.
This changes financial output semantics (outstanding_amount now converted and rounded). Please add report-level coverage for at least: foreign-currency invoice, same-currency invoice, and precision assertion on the displayed outstanding value.
If useful, I can draft a focused test matrix and expected values for these scenarios.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@erpnext/accounts/report/sales_register/sales_register.py` around lines 160 -
168, Add regression tests that exercise the Sales Register report's
outstanding_amount conversion and rounding: create one foreign-currency invoice
with conversion_rate != 1 and one same-currency invoice with conversion_rate =
1, set known outstanding_amount and outstanding_precision, run the Sales
Register report (the code path that produces row.update({...
"outstanding_amount": flt((inv.outstanding_amount * (inv.conversion_rate or 1)),
outstanding_precision) })) and assert the returned/displayed outstanding_amount
equals the expected (inv.outstanding_amount * conversion_rate) rounded to
outstanding_precision; include assertions for both invoices and an explicit
precision check to ensure the flt and rounding behavior is correct.
fa6bbf0 to
9a28922
Compare
PKSowmiya05
force-pushed
the
pr-sr-report-oustanding-amount
branch
from
9a28922 to
9a4f58d
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #54597 +/- ##
===========================================
+ Coverage 79.65% 79.76% +0.11%
===========================================
Files 1160 1160
Lines 126213 126476 +263
===========================================
+ Hits 100532 100882 +350
+ Misses 25681 25594 -87
🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue:
While generating the Sales Register and Purchase Register reports, the Outstanding Amount is currently displayed in the transaction currency
Ref: 66243
Reference Report:
Accounts Receivable
Before:
After:
Backport needed for v15, v16