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: batch negative qty validation #28782

Merged
merged 5 commits into from
Dec 8, 2021
Merged

Conversation

ankush
Copy link
Member

@ankush ankush commented Dec 8, 2021

continuation from #27945

Change:

  • Validate that any backdated transaction can never cause an individual Batch's ledger to go negative at any time in the future.

The simplest way to reproduce:

  1. Create a batched item. Create one batch at T0, a second batch at T0+2 of equal quantities.
  2. Consume the second batch at T0+1.

Observed: The Batch didn't exist at this time. The transaction gets submitted without any warning/error.

Expected: Transaction can't be submitted.

More use cases:

  1. Try to consume more from a batch that produced qty in chronological order (no backdated entries) This was caught before this PR too.
  2. Add any transaction that makes the intermediate quantity of batch go negative but overall stays positive.
  3. Use multiple batches like in the example for reproducing above.
  4. ANY weird combination you can think of, since this validation constructs a batch's ledger in memory it should catch any and all possible instances of negative stock for a batch.

TODO:

@ankush ankush added this to the v13.17 milestone Dec 8, 2021
@ankush ankush added the needs-tests This PR needs automated unit-tests. label Dec 8, 2021
18alantom and others added 4 commits December 8, 2021 13:06
batch's ledger is only maintained in form of `actual_qty` on batch's
SLEs. To validate if batch has any negative qty in future, cumulative
total of `actual_qty` is required to ensure it never goes negative.
This check was only checking total sum, which is problamatic when making
backdated entries that can cause intermediate values to go negative
while overall values stay positive.
@ankush ankush marked this pull request as ready for review December 8, 2021 07:37
@ankush ankush removed the needs-tests This PR needs automated unit-tests. label Dec 8, 2021
@codecov
Copy link

codecov bot commented Dec 8, 2021

Codecov Report

Merging #28782 (5977c4d) into develop (9a644f6) will increase coverage by 8.47%.
The diff coverage is 100.00%.

@@             Coverage Diff             @@
##           develop   #28782      +/-   ##
===========================================
+ Coverage    46.80%   55.27%   +8.47%     
===========================================
  Files         1122     1122              
  Lines        66777    66782       +5     
===========================================
+ Hits         31255    36915    +5660     
+ Misses       35522    29867    -5655     
Impacted Files Coverage Δ
...k/doctype/stock_ledger_entry/stock_ledger_entry.py 85.18% <100.00%> (+13.25%) ⬆️
erpnext/stock/stock_ledger.py 86.43% <100.00%> (+7.35%) ⬆️
.../hr/doctype/upload_attendance/upload_attendance.py 44.69% <0.00%> (-1.52%) ⬇️
...stock/doctype/purchase_receipt/purchase_receipt.py 90.29% <0.00%> (+0.26%) ⬆️
erpnext/accounts/doctype/account/account.py 71.76% <0.00%> (+0.39%) ⬆️
erpnext/hr/utils.py 74.29% <0.00%> (+0.40%) ⬆️
.../report/accounts_receivable/accounts_receivable.py 68.80% <0.00%> (+0.42%) ⬆️
erpnext/controllers/buying_controller.py 86.84% <0.00%> (+0.47%) ⬆️
erpnext/controllers/item_variant.py 70.76% <0.00%> (+0.51%) ⬆️
...xt/buying/doctype/purchase_order/purchase_order.py 78.30% <0.00%> (+0.56%) ⬆️
... and 181 more

@ankush ankush merged commit 96a019e into frappe:develop Dec 8, 2021
@ankush ankush deleted the batch_neg_check branch December 8, 2021 10:22
@ankush ankush added this to In progress in ERPNext Roadmap via automation Dec 8, 2021
@ankush ankush moved this from In progress to Done in ERPNext Roadmap Dec 8, 2021
@ankush
Copy link
Member Author

ankush commented Dec 8, 2021

@Mergifyio backport version-12-hotfix

@mergify
Copy link
Contributor

mergify bot commented Dec 8, 2021

backport version-12-hotfix

✅ Backports have been created

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants