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: Stock Ageing Transfer Bucket logic for Repack Entry with split batch rows #29816

Merged
merged 8 commits into from
Feb 18, 2022

Conversation

marination
Copy link
Collaborator

@marination marination commented Feb 15, 2022

Issue:

Consider Repack Entry with Same Item-Warehouse:

Item 1 > -50 > Batch 1
Item 1 > -50 > Batch 2
Item 1 > +100 > Batch 3
TLDR: Above ledger caused a balance difference of 50 in stock ageing

  • Repack entries with split consumption rows (mostly due to batch) would generate inaccurate time bucket balances in FIFO queue for Stock Ageing
  • For such an entry technically there's no material coming in. Just existing material repurposed.
  • This is why we maintain transferred_item_details in FIFOSlots.
  • We maintain qty that was consumed from the FIFO Queue via repack in transferred_item_details. Which is then re-added to the FIFO Queue and adjusted in transferred_item_details
  • After consuming 50, 50 qty from the FIFO Queue, transferred_item_details looks like : [[50, date], [50, date]]
  • Now for the 100 that is incoming we need to put it back in the FIFO Queue
  • The older logic simply popped the first slot from transferred_item_details, thus making it : [[50, date]] and only adding 50 to the FIFO Queue
  • The remaining 50, was left in transferred_item_details, which caused a difference of 50 in the generated FIFO Queue.

Fix:

  • Consume transferred_item_details just like outgoing stock is consumed in the FIFO Queue (ref: __compute_outgoing_stock())
  • So each bucket in transferred_item_details : [[50, date], [50, date]] is consumed given there's equivalent incoming stock
  • If incoming stock is more than transfer bucket qty, excess incoming stock will be added as new slot in the FIFO Queue

To Test:

Consider Ledger below (note voucher nos):
Screenshot 2022-02-15 at 7 44 27 PM

  • Before:
    Screenshot 2022-02-15 at 7 48 39 PM

  • After:
    Screenshot 2022-02-15 at 7 47 02 PM

More Cases:

  • Repack where Item 1 is consumed and Item 2 is produced. Check Stock Ageing for Item 2.
  • Repack (negative stock) while having 0 stock, so outgoing SLE causes negative stock, and incoming makes stock 0 again. For same Item-Warehouse
  • Repack: Overconsume. 26 consumed, 1 produced (more consumed, less produced)
  • Repack: Overconsume with split rows (multiple rows), and produce less in one row
  • Repack: Overproduce. Consumed 28, Produced 30
  • Repack: Overproduce with split rows (multiple rows), and produce more in one row

You can check the test file and simulate similar ledgers if you want to

@marination marination changed the title Repack entry stock ageing fix: Stock Ageing Transfer Bucket logic for Repack Entry with split batch rows Feb 15, 2022
@marination
Copy link
Collaborator Author

Screenshot 2022-02-15 at 7 54 25 PM

@marination marination marked this pull request as ready for review February 15, 2022 15:07
@marination marination marked this pull request as draft February 16, 2022 08:10
@marination marination added this to the v13.21 milestone Feb 18, 2022
@marination marination marked this pull request as ready for review February 18, 2022 13:33
@marination
Copy link
Collaborator Author

Unrelated test_item_search failing

@marination marination merged commit 18c6cc9 into frappe:develop Feb 18, 2022
@marination
Copy link
Collaborator Author

@Mergifyio backport version-13-hotfix

@marination
Copy link
Collaborator Author

@Mergifyio backport version-13-pre-release

@mergify
Copy link
Contributor

mergify bot commented Feb 18, 2022

backport version-13-hotfix

✅ Backports have been created

@mergify
Copy link
Contributor

mergify bot commented Feb 18, 2022

backport version-13-pre-release

✅ Backports have been created

marination added a commit that referenced this pull request Feb 18, 2022
…-29816

fix: Stock Ageing Transfer Bucket logic for Repack Entry with split batch rows (backport #29816)
marination added a commit that referenced this pull request Feb 18, 2022
…se/pr-29816

fix: Stock Ageing Transfer Bucket logic for Repack Entry with split batch rows (backport #29816)
Copy link
Collaborator

@gavindsouza gavindsouza left a comment

Choose a reason for hiding this comment

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

Great job on the tests + docs btw :trollface:

@@ -12,6 +12,7 @@
from erpnext.stock.doctype.serial_no.serial_no import get_serial_nos

Filters = frappe._dict
precision = cint(frappe.db.get_single_value("System Settings", "float_precision"))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Breaks multi-tenancy!

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 15, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants