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: can't cancel reconciliation that created new sr no #29393

Merged
merged 4 commits into from
Jan 21, 2022

Conversation

ankush
Copy link
Member

@ankush ankush commented Jan 21, 2022

closes: #25390

Steps to reproduce:

  1. Create a new serialized item (WITHOUT batches)
  2. Create a stock reco and add serial nos for it via stock reco.
  3. New serial nos will get created
  4. Cancel the said stock reco. You'll get error that sr is undefined.

Root cause: stock reco was setting "0" as current_serial_no instead it should have been "" (empty string) or None.

Fix: set current_serial_no to empty string when none are found
patch: update stock reco item row's serial no to NULL where qty is 0.

Minor unrelated perf: N+1 query for unlinking inactive serial nos instead of N+2. (will become just 2 after frappe/frappe#15560 goes through)

serial_no field should have empty string if it doesn't have any serial
nos
When current serial nos are non-existing there shouldn't be any value in
current_serial_no field.
@codecov
Copy link

codecov bot commented Jan 21, 2022

Codecov Report

Merging #29393 (a6472d6) into develop (f5ed0c8) will increase coverage by 0.05%.
The diff coverage is 100.00%.

@@             Coverage Diff             @@
##           develop   #29393      +/-   ##
===========================================
+ Coverage    57.87%   57.92%   +0.05%     
===========================================
  Files         1091     1091              
  Lines        67894    67893       -1     
===========================================
+ Hits         39292    39329      +37     
+ Misses       28602    28564      -38     
Impacted Files Coverage Δ
erpnext/stock/utils.py 67.57% <ø> (ø)
erpnext/controllers/stock_controller.py 90.65% <100.00%> (-0.05%) ⬇️
...ext/accounts/report/balance_sheet/balance_sheet.py 36.36% <0.00%> (-21.82%) ⬇️
...payroll/doctype/income_tax_slab/income_tax_slab.py 83.33% <0.00%> (-16.67%) ⬇️
...ctype/woocommerce_settings/woocommerce_settings.py 80.76% <0.00%> (-3.85%) ⬇️
...cial_statement/consolidated_financial_statement.py 88.81% <0.00%> (-1.60%) ⬇️
...ctype/accounting_dimension/accounting_dimension.py 64.34% <0.00%> (-1.56%) ⬇️
...e/period_closing_voucher/period_closing_voucher.py 88.23% <0.00%> (-1.48%) ⬇️
...rpnext/stock/report/stock_balance/stock_balance.py 79.16% <0.00%> (-0.60%) ⬇️
erpnext/stock/stock_ledger.py 85.42% <0.00%> (-0.56%) ⬇️
... and 16 more

@ankush ankush merged commit 4fa93e2 into frappe:develop Jan 21, 2022
@ankush ankush deleted the undefined_sr branch January 21, 2022 09:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Stock Reconciliation - "sr" referenced before assignment
1 participant