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: Generate Warehouse wise FIFO Queue always and later aggregate if required #29788

Merged
merged 3 commits into from
Feb 14, 2022

Conversation

marination
Copy link
Collaborator

@marination marination commented Feb 14, 2022

Issue:

Consider the following ledger with different warehouses:

No. Document Date Qty Warehouse
1 Stock Reco 1st Feb 1200 WH 1
2 Stock Reco 2nd Feb 500 WH 2
3 Delivery Note 3rd Feb -10 WH 2

TLDR: Total Balance (across warehouses) should be 1200 + 490 (500-10) = 1690 but instead is 500 - 10 = 490

  • Our current calculation, while show_warehouse_wise_stock is disabled, ignores warehouse completely and maintains only item wise data
  • In the case above, when reco is done for WH 2, FIFOSlots does not know/care about which warehouse this reco is for.
  • So it resets the entire previous balance, even if part of the balance belonged to WH 1

Screenshot 2022-02-14 at 8 36 58 PM

Fix:

  • Maintain Queue and balances warehouse wise and later aggregate if show_warehouse_wise_stock is disabled
  • Due to warehouse wise data, sequential stock recos can rightfully reset the entire balance qty of one warehouse only

Screenshot 2022-02-14 at 8 35 37 PM

To test:

  • Test above scenario with and without Show Warehouse Wise Stock enabled in Report Stock Ageing
  • Test Report Stock Balance for the same with Show Ageing Data enabled
  • Test Report Warehouse Wise Item Balance Age and Value

- Back to back stock recos cause incorrect qty calculation across warehouses
- Hard to differentiate how much of the qty is reset by the reco
- Maintain Queue and balances warehouse wise and later aggregate for accurate values
@github-actions github-actions bot added the needs-tests This PR needs automated unit-tests. label Feb 14, 2022
@marination marination marked this pull request as ready for review February 14, 2022 15:13
@marination marination removed the needs-tests This PR needs automated unit-tests. label Feb 14, 2022
@codecov
Copy link

codecov bot commented Feb 14, 2022

Codecov Report

Merging #29788 (cd728fc) into develop (19a6c21) will increase coverage by 7.72%.
The diff coverage is 100.00%.

@@             Coverage Diff             @@
##           develop   #29788      +/-   ##
===========================================
+ Coverage    51.82%   59.54%   +7.72%     
===========================================
  Files         1108     1108              
  Lines        68893    68908      +15     
===========================================
+ Hits         35703    41033    +5330     
+ Misses       33190    27875    -5315     
Impacted Files Coverage Δ
erpnext/stock/report/stock_ageing/stock_ageing.py 94.14% <100.00%> (+1.08%) ⬆️
erpnext/portal/utils.py 23.18% <0.00%> (-5.80%) ⬇️
...e/asset_value_adjustment/asset_value_adjustment.py 86.04% <0.00%> (-3.49%) ⬇️
...e/period_closing_voucher/period_closing_voucher.py 88.23% <0.00%> (-1.48%) ⬇️
erpnext/support/doctype/issue/issue.py 64.56% <0.00%> (-0.98%) ⬇️
...ctype/accounting_dimension/accounting_dimension.py 64.34% <0.00%> (-0.78%) ⬇️
erpnext/selling/doctype/sales_order/sales_order.py 77.12% <0.00%> (-0.38%) ⬇️
erpnext/projects/doctype/project/project.py 54.54% <0.00%> (-0.35%) ⬇️
...rpnext/stock/report/stock_balance/stock_balance.py 79.16% <0.00%> (ø)
...stock/doctype/purchase_receipt/purchase_receipt.py 91.34% <0.00%> (+0.25%) ⬆️
... and 145 more

@marination
Copy link
Collaborator Author

Patch test breaking due to framework

@marination marination merged commit 728545d into frappe:develop Feb 14, 2022
@marination
Copy link
Collaborator Author

@Mergifyio backport version-13-hotfix

@mergify
Copy link
Contributor

mergify bot commented Feb 14, 2022

backport version-13-hotfix

✅ Backports have been created

marination added a commit that referenced this pull request Feb 14, 2022
…-29788

fix: Generate Warehouse wise FIFO Queue always and later aggregate if required (backport #29788)
@marination marination added this to the v13.21 milestone Feb 14, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 8, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant