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): don't reserve qty on sales return. #31271

Merged
merged 25 commits into from
Apr 4, 2023

Conversation

dj12djdjs
Copy link
Collaborator

bringing #31017 back to life

@codecov
Copy link

codecov bot commented Jun 7, 2022

Codecov Report

Merging #31271 (edb0666) into develop (ddb17a8) will increase coverage by 0.00%.
The diff coverage is 87.50%.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop   #31271   +/-   ##
========================================
  Coverage    63.79%   63.80%           
========================================
  Files          810      810           
  Lines        59533    59539    +6     
========================================
+ Hits         37981    37986    +5     
- Misses       21552    21553    +1     
Impacted Files Coverage Δ
erpnext/stock/doctype/batch/batch.py 82.92% <75.00%> (-0.41%) ⬇️
erpnext/stock/report/stock_ledger/stock_ledger.py 75.59% <100.00%> (+0.44%) ⬆️
erpnext/stock/stock_balance.py 50.00% <100.00%> (+0.54%) ⬆️

@dj12djdjs
Copy link
Collaborator Author

@ankush Just wanted to checkup, have you thought of any way to implement this without disturbing existing workflows ?

@ankush
Copy link
Member

ankush commented Jun 17, 2022

@dj12djdjs we can add an option in selling settings.

The default behavior should be consistent with current behavior.

# Conflicts:
#	erpnext/patches.txt
#	erpnext/public/js/utils/barcode_scanner.js
#	erpnext/regional/report/gstr_1/gstr_1.py
#	erpnext/stock/doctype/delivery_note/test_delivery_note.py
@dj12djdjs
Copy link
Collaborator Author

@ankush Ready for review

@dj12djdjs
Copy link
Collaborator Author

@ankush Sorry about that, I just realized there was more conflicts 😅

@dj12djdjs
Copy link
Collaborator Author

@ankush Is there any issues with this ?

@stale
Copy link

stale bot commented Aug 7, 2022

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.

@stale stale bot added the inactive label Aug 7, 2022
@stale stale bot closed this Aug 10, 2022
@dj12djdjs dj12djdjs reopened this Feb 9, 2023
@stale stale bot removed inactive labels Feb 9, 2023
# Conflicts:
#	erpnext/stock/doctype/delivery_note/test_delivery_note.py
Copy link
Contributor

@s-aga-r s-aga-r left a comment

Choose a reason for hiding this comment

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

@dj12djdjs please resolve conflicts.

@dj12djdjs dj12djdjs requested a review from s-aga-r February 14, 2023 15:50
Copy link
Collaborator

@barredterra barredterra left a comment

Choose a reason for hiding this comment

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

Changes to erpnext/selling/doctype/sales_order/sales_order.json seem to be unrelated; please revert these.

@stale
Copy link

stale bot commented Mar 18, 2023

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.

@stale stale bot added the inactive label Mar 18, 2023
@@ -94,51 +98,85 @@ def get_balance_qty_from_sle(item_code, warehouse):


def get_reserved_qty(item_code, warehouse):
Copy link
Collaborator

Choose a reason for hiding this comment

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

This function looks very complex and difficult to read, could you split the code into multiple methods?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@rohitwaghchaure Honestly, I don't even fully understand the query. I just reverted it back to the previous query and used f string to inject the condition.

= and others added 3 commits March 24, 2023 23:50
@rohitwaghchaure rohitwaghchaure merged commit 12325cb into frappe:develop Apr 4, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 4, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants