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: Add return against indexes for POS Invoice #32378

Merged

Conversation

deepeshgarg007
Copy link
Member

set_status is called twice while creating a POS Invoice, once on validate and once on submit. The set_status method in pos_invoice.py (similar to sales invoice) has an if block which check for return invoices to set the status as "Credit Note Issued"

The very same query in POS Invoice took around ~5 sec (depending on the no of existing POS Invoices) for a specific site. Post adding the index the POS invoice submission took 3 seconds instead of 13 seconds

telegram-cloud-photo-size-5-6165440435925923276-y

Also showed up in slow query logs
telegram-cloud-photo-size-5-6165440435925923281-y

@github-actions github-actions bot added the needs-tests This PR needs automated unit-tests. label Sep 27, 2022
@deepeshgarg007
Copy link
Member Author

@Mergifyio backport version-14-hotfix version-13-hotfix

@mergify
Copy link
Contributor

mergify bot commented Sep 27, 2022

backport version-14-hotfix version-13-hotfix

🟠 Waiting for conditions to match

  • merged [:pushpin: backport requirement]



def on_doctype_update():
frappe.db.add_index("POS Invoice", ["customer", "is_return", "return_against"])
Copy link
Member

@ankush ankush Sep 27, 2022

Choose a reason for hiding this comment

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

Just a simple return against index should be enough. It has high enough cardinality ~= unique return.

That can be managed from doctype directly 👀

@ankush ankush removed the needs-tests This PR needs automated unit-tests. label Sep 27, 2022
@codecov
Copy link

codecov bot commented Sep 27, 2022

Codecov Report

Merging #32378 (cbfe282) into develop (106ee1b) will increase coverage by 0.16%.
The diff coverage is 50.00%.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #32378      +/-   ##
===========================================
+ Coverage    63.94%   64.11%   +0.16%     
===========================================
  Files          817      817              
  Lines        58159    58171      +12     
===========================================
+ Hits         37190    37295     +105     
+ Misses       20969    20876      -93     
Impacted Files Coverage Δ
...rpnext/accounts/doctype/pos_invoice/pos_invoice.py 74.80% <50.00%> (-0.13%) ⬇️
erpnext/accounts/doctype/bank/bank.py 71.42% <0.00%> (-14.29%) ⬇️
...saction/incorrect_balance_qty_after_transaction.py 88.37% <0.00%> (-9.31%) ⬇️
...urity_shortfall/process_loan_security_shortfall.py 93.75% <0.00%> (-6.25%) ⬇️
erpnext/accounts/party.py 77.43% <0.00%> (-4.46%) ⬇️
...pnext/setup/doctype/sales_partner/sales_partner.py 65.21% <0.00%> (-4.35%) ⬇️
...e/asset_value_adjustment/asset_value_adjustment.py 87.95% <0.00%> (-3.62%) ⬇️
...ck/report/cogs_by_item_group/cogs_by_item_group.py 86.32% <0.00%> (-3.42%) ⬇️
...eorder_level/itemwise_recommended_reorder_level.py 90.74% <0.00%> (-1.86%) ⬇️
erpnext/stock/doctype/item_price/item_price.py 92.98% <0.00%> (-1.76%) ⬇️
... and 24 more

@deepeshgarg007
Copy link
Member Author

@Mergifyio backport version-14-hotfix version-13-hotfix

@mergify
Copy link
Contributor

mergify bot commented Sep 27, 2022

backport version-14-hotfix version-13-hotfix

✅ Backports have been created

@deepeshgarg007 deepeshgarg007 merged commit 4c8617e into frappe:develop Sep 27, 2022
deepeshgarg007 added a commit that referenced this pull request Sep 27, 2022
…-32378

fix: Add return against indexes for POS Invoice (backport #32378)
deepeshgarg007 added a commit that referenced this pull request Sep 28, 2022
…-32378

fix: Add return against indexes for POS Invoice (backport #32378)
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 12, 2024
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.

2 participants