Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: precision loss when transferring #30834

Merged
merged 1 commit into from
May 16, 2022
Merged

fix: precision loss when transferring #30834

merged 1 commit into from
May 16, 2022

Conversation

ankush
Copy link
Member

@ankush ankush commented Apr 28, 2022 •
edited
Loading

  • Stock Entry transfer relies on "incoming_rate" to determine added value in target warehouse during transfers.
  • Incoming rate was getting rounded off while setting. This means if rate was more precise than currency precision then remaining value wont be added to target warehouse.
  • This results in very small rounding adjustment which in turn results in GLE for stock adjustment account.

This change disables rounding of incoming rate to preserve incoming value = outgoing value invariant.

Example (same as test case):

  1. Material bought at 156,526 for 20 tonne where stock UOM is KG. So per kg rate is 156.526 (but currency precision is 2 so this would get rounded off to 156.53 when transferring)
  2. This rounding off results in small rounding adjustment. 1000 * 156.526 - 1000 * 156.53 = 4.0.

This only happens when purchase UOM is higher than stock UOM and rate doesn't fit in currency precision.

@codecov
Copy link

codecov bot commented Apr 28, 2022 •
edited
Loading

Codecov Report

Merging #30834 (e1987e2) into develop (861d4b8) will decrease coverage by 1.63%.
The diff coverage is 100.00%.

❗ Current head e1987e2 differs from pull request most recent head b1c90e9. Consider uploading reports for the commit b1c90e9 to get more accurate results

@@             Coverage Diff             @@
##           develop   #30834      +/-   ##
===========================================
- Coverage    63.09%   61.46%   -1.64%     
===========================================
  Files          984     1079      +95     
  Lines        67168    69700    +2532     
===========================================
+ Hits         42379    42838     +459     
- Misses       24789    26862    +2073
Impacted Files Coverage Δ
erpnext/stock/doctype/stock_entry/stock_entry.py 81.11% <100.00%> (ø)
...iliation_invoice/payment_reconciliation_invoice.py 0.00% <0.00%> (-100.00%) ⬇️
...iliation_payment/payment_reconciliation_payment.py 0.00% <0.00%> (-100.00%) ⬇️
...on_allocation/payment_reconciliation_allocation.py 0.00% <0.00%> (-100.00%) ⬇️
...e/payment_reconciliation/payment_reconciliation.py 16.56% <0.00%> (-68.23%) ⬇️
...eport/item_variant_details/item_variant_details.py 31.32% <0.00%> (-53.02%) ⬇️
erpnext/utilities/activation.py 14.28% <0.00%> (-48.58%) ⬇️
...unts/report/purchase_register/purchase_register.py 34.21% <0.00%> (-44.08%) ⬇️
...tch_item_expiry_status/batch_item_expiry_status.py 67.92% <0.00%> (-24.53%) ⬇️
...wise_balance_history/batch_wise_balance_history.py 67.79% <0.00%> (-22.04%) ⬇️
... and 172 more

@ankush ankush force-pushed the transfer_precision branch from e1987e2 to b1c90e9 Compare May 12, 2022 05:40
@ankush ankush marked this pull request as ready for review May 12, 2022 05:40
@marination
Copy link
Collaborator

Tested locally works as expected

@marination marination merged commit 06c036f into frappe:develop May 16, 2022
@mergify mergify bot mentioned this pull request May 16, 2022
Merged
ankush added a commit that referenced this pull request May 16, 2022
@mergify @ankush @marination
fix: precision loss when transferring (backport #30834) (#31032)
fc80a50 
* fix: stock transfer value when precision differs

(cherry picked from commit b1c90e9)

# Conflicts:
#	erpnext/stock/doctype/stock_ledger_entry/test_stock_ledger_entry.py

* fix: Merge conflicts

* chore: Remove unused `flt` (sider)

Co-authored-by: Ankush Menat <ankush@frappe.io>
Co-authored-by: Marica <maricadsouza221197@gmail.com>
frappe-pr-bot pushed a commit that referenced this pull request May 17, 2022
@frappe-bot
chore(release): Bumped to Version 13.30.0
d9f7647 
# [13.30.0](v13.29.2...v13.30.0) (2022-05-17)

### Bug Fixes

* `set_missing_values` in SE and re-use the same on all SE mappings ([fe52c1f](fe52c1f))
* Add validation for SEZ and Export invoices without payment of taxes ([cb8453d](cb8453d))
* allow to use formatting for the field to_discuss in opportunity ([e126d4e](e126d4e))
* Block 0 Qty via Update Items to be consistent with form validation ([5647875](5647875))
* Calculate totals even though pricing rule is not applied on mapped doc ([678a01d](678a01d))
* **charts:** Pass fieldtype for chart data in selling reports ([917e7c3](917e7c3))
* conflicts ([87fd933](87fd933))
* conflicts ([fb62bbf](fb62bbf))
* disable pricing rules for internal transfers (backport [#31034](#31034)) ([#31036](#31036)) ([d5eb9fb](d5eb9fb))
* discount ledger entry in case of multicurrency invoice ([#31047](#31047)) ([c3417e4](c3417e4))
* dont fail repost for recoverable errors (backport [#30979](#30979)) ([#31023](#31023)) ([a019cb6](a019cb6))
* **Employee Advance:** Return/Deduction from Salary button visibility (backport [#31011](#31011)) ([#31012](#31012)) ([5b1d85e](5b1d85e))
* Failing accounting dimension patch ([b14a7b8](b14a7b8))
* german translations for Employee ([b9bda04](b9bda04))
* gl entry validation for miniscule loan penalty ([e958ef2](e958ef2))
* hide template items from sales/purchase order ([8b99f43](8b99f43))
* IN time not captured in Attendance through Employee Checkin (backport [#31029](#31029)) ([#31031](#31031)) ([477bbcc](477bbcc))
* Item rate reset on changing posting date ([#30990](#30990)) ([8ef649f](8ef649f))
* Just add one rate in GST HSN Code ([ed76687](ed76687))
* Merge Conflicts ([3abf264](3abf264))
* Multiple fixes in GSTR-1 report ([f2cbb70](f2cbb70))
* **patch:** avoid checking for return field if it doesnt exits (backport [#30995](#30995)) ([#30997](#30997)) ([a94b5c0](a94b5c0))
* per_billed for return DN (backport [#30868](#30868)) ([#30971](#30971)) ([97ea1f5](97ea1f5))
* precision loss when transferring  (backport [#30834](#30834)) ([#31032](#31032)) ([fc80a50](fc80a50))
* precision of total penalty paid ([ad21853](ad21853))
* precision of total penalty paid ([5c45737](5c45737))
* prevent bypassing forced valuation rate (backport [#30987](#30987)) ([#31020](#31020)) ([706c19d](706c19d))
* pro rata calculation for monthly depreciation ([#30989](#30989)) ([408d952](408d952))
* remove item attribute limit from variant selector (backport [#31026](#31026)) ([#31028](#31028)) ([1f016e9](1f016e9))
* Set actual qty and basic rate in SE on warehouse triggers (`get_warehouse_details`) ([30b0aee](30b0aee))
* stock analytics report shows incorrect data there's no stock movement in a period (backport [#30945](#30945)) ([#30980](#30980)) ([295ffb3](295ffb3))
* translation for status filter ([e5f8231](e5f8231))
* **translations:** Update ru translations ([#30992](#30992)) ([f797005](f797005))
* TypeError in add_indicator_for_multicompany (backport [#31042](#31042)) ([#31048](#31048)) ([e24bb1d](e24bb1d))
* unlink Attendance from Employee Checkins on cancellation (backport [#31045](#31045)) ([#31049](#31049)) ([e03fe97](e03fe97))
* UOM in HSN-wise summary of outward supply ([cd7d5cd](cd7d5cd))
* user can select disabled accounts in taxes table ([047c879](047c879))
* validate disabled accounts before posting ledger entries ([515e49b](515e49b))
* validate on hold purchase invoices in payment entry ([9fbd170](9fbd170))

### Features

* add Employee Status filter in leave balance reports ([716b525](716b525))
* add Link to Opportunity ([#30614](#30614)) ([bc23bc7](bc23bc7))
* request_for_quotation ([db4e264](db4e264))
* request_for_quotation - refactor ([b6a3e69](b6a3e69))
* select multiple values for accounting dimension (backport [#31015](#31015)) ([#31041](#31041)) ([9c21eb5](9c21eb5))
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@marination marination marination approved these changes

@rohitwaghchaure rohitwaghchaure Awaiting requested review from rohitwaghchaure rohitwaghchaure is a code owner

Assignees
No one assigned
Labels
backport version-13-hotfix
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ankush @marination