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.

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

fix: dont update sle values from get_gl_entries #29304

Merged
merged 2 commits into from
Jan 17, 2022

Conversation

ankush
Copy link
Member

@ankush ankush commented Jan 16, 2022

Summary: get_gl_entries is adding average valuation rate when SLE has no stock value difference.

Steps to reproduce:

  1. Make sure FIFO is the valuation method. Make ~3 stock receipts; 1 with 0 valuation.
  2. Consume material; when 0 valued material is consumed instead the average rate is used to compute value difference.

Fix: Just assume what the stock ledger has done is right and don't try to modify it when making GL entries.

This was added in #7583; now there's no reason to assume that 0 rate is from a bug (and if there is, root cause should be fixed instead of applying average rate)

@codecov
Copy link

codecov bot commented Jan 16, 2022

Codecov Report

Merging #29304 (0272397) into develop (3438e1f) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

@@             Coverage Diff             @@
##           develop   #29304      +/-   ##
===========================================
- Coverage    57.93%   57.92%   -0.02%     
===========================================
  Files         1110     1110              
  Lines        68087    68079       -8     
===========================================
- Hits         39449    39435      -14     
- Misses       28638    28644       +6     
Impacted Files Coverage Δ
erpnext/controllers/stock_controller.py 90.70% <100.00%> (-0.21%) ⬇️
...eport/item_variant_details/item_variant_details.py 84.33% <0.00%> (-3.62%) ⬇️
erpnext/education/doctype/student/student.py 73.68% <0.00%> (-3.16%) ⬇️
erpnext/stock/report/stock_ledger/stock_ledger.py 75.60% <0.00%> (-2.44%) ⬇️
...ion/doctype/course_enrollment/course_enrollment.py 44.00% <0.00%> (-2.00%) ⬇️
...xt/stock/report/stock_analytics/stock_analytics.py 91.08% <0.00%> (-1.99%) ⬇️
...wise_balance_history/batch_wise_balance_history.py 91.37% <0.00%> (-1.73%) ⬇️
erpnext/stock/reorder_item.py 76.06% <0.00%> (-1.71%) ⬇️
.../report/stock_projected_qty/stock_projected_qty.py 88.15% <0.00%> (-1.32%) ⬇️
erpnext/stock/report/stock_ageing/stock_ageing.py 92.48% <0.00%> (-1.16%) ⬇️
... and 18 more

@ankush ankush marked this pull request as ready for review January 16, 2022 07:55
@ankush ankush added this to the v13.19 milestone Jan 16, 2022
@rohitwaghchaure rohitwaghchaure merged commit 9088984 into frappe:develop Jan 17, 2022
@rohitwaghchaure
Copy link
Collaborator

@Mergifyio backport version-13-hotfix

@mergify
Copy link
Contributor

mergify bot commented Jan 17, 2022

backport version-13-hotfix

✅ Backports have been created

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants