fix(stock): don't reserve qty on sales return.

[dj12djdjs]

Before PR

When a sales return is issued, the qty returned will be marked as "Reserved Qty".

After PR

When a sales return is issued, the qty returned won't reflect reserved qty.

References

https://discuss.erpnext.com/t/reserved-item-on-submit-of-sales-order-not-coming-back-to-stock-on-sales-return/51436
https://discuss.erpnext.com/t/after-return-the-product-appears-with-reserved-quantity-in-the-stock-summary/34829

[dj12djdjs]

Hoping I don't have to re-write that query using the new query builder 😅

[codecov]

Codecov Report

Merging #31017 (30f580e) into develop (3714e36) will decrease coverage by 0.27%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           develop   #31017      +/-   ##
===========================================
- Coverage    63.07%   62.79%   -0.28%     
===========================================
  Files          985      985              
  Lines        67272    67283      +11     
===========================================
- Hits         42431    42251     -180     
- Misses       24841    25032     +191     

Impacted Files	Coverage Δ
erpnext/stock/stock_balance.py	49.45% <ø> (ø)
...eport/item_variant_details/item_variant_details.py	31.32% <0.00%> (-53.02%)	⬇️
...unts/report/purchase_register/purchase_register.py	34.21% <0.00%> (-44.08%)	⬇️
...ext/accounts/report/balance_sheet/balance_sheet.py	36.36% <0.00%> (-21.82%)	⬇️
...e_purchase_register/item_wise_purchase_register.py	57.42% <0.00%> (-20.80%)	⬇️
...em_wise_sales_register/item_wise_sales_register.py	51.13% <0.00%> (-9.96%)	⬇️
...urity_shortfall/process_loan_security_shortfall.py	93.75% <0.00%> (-6.25%)	⬇️
erpnext/accounts/party.py	78.09% <0.00%> (-4.62%)	⬇️
erpnext/stock/doctype/warehouse/warehouse.py	76.92% <0.00%> (-4.20%)	⬇️
...e/asset_value_adjustment/asset_value_adjustment.py	86.04% <0.00%> (-3.49%)	⬇️
... and 26 more

[dj12djdjs]

This might need a patch, maybe calling get_reserved_qty and updating each row in tabBin ?

[ankush]

Hoping I don't have to re-write that query using the new query builder 😅

Not required for fixes. Required for new queries.

[ankush]

@dj12djdjs can you add a short test to avoid regression?

Useful utils:

• make_item() to make random item,
• make_sales_order
• make_delivery_note
• make_return_doc

[dj12djdjs]

@ankush I cannot get the test to pass. It works when I do it over the browser, but in the test environment the status of the documents is not even changing. I'm not sure how to proceed.

[dj12djdjs]

@ankush I cannot get the test to pass. It works when I do it over the browser, but in the test environment the status of the documents is not even changing. I'm not sure how to proceed.

Looks like this 4d0fa85 fixed it

[dj12djdjs]

I need to look into why the test is still failing. I was thinking make_item is generating a unique item based on a hash so there shouldn't be any other stock based on that item. I guess I was wrong.

[ankush]

@dj12djdjs will look into it today 👍

[dj12djdjs]

@ankush Looks good now

need to rethink this a bit.

[ankush]

@dj12djdjs this is kind of a breaking behaviour.

E.g. There are users for whom returned qty means they might have to send more to fulfill the order and hence reservation makes sense for them.

Others can close the sales order to remove reservations if further deliveries are not to be made.

[dj12djdjs]

@dj12djdjs this is kind of a breaking behaviour.

E.g. There are users for whom returned qty means they might have to send more to fulfill the order and hence reservation makes sense for them.

Others can close the sales order to remove reservations if further deliveries are not to be made.

Good point, maybe we can add something in the stock settings. Something like,

• Reserve Quantity on Sales Return (Default checked, so users can opt out of this)

LOL, I guess I'm gonna have to convert that query using the Query Builder if that's the case. Should be easier to maintain this condition if the query is dynamically built.

[stale]

This pull request has been automatically marked as inactive because it has not had recent activity. It will be closed within 3 days if no further activity occurs, but it only takes a comment to keep a contribution alive :) Also, even if it is closed, you can always reopen the PR when you're ready. Thank you for contributing.

[dj12djdjs]

bump

[dj12djdjs]

I have no idea how to open this again 🤣

[ankush]

Wait, you're unable to open closed PR? 👀

looks like you pushed a commit after PR got closed c9dce9c

still weird, only forced pushed branches don't reopen else it should be fine.

[dj12djdjs]

@ankush That's correct. I was thinking if I pushed something to the branch it might re-open. Sadly, it didn't.