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

chore: Fix a potential variable misuse bug #31372

Merged
merged 4 commits into from
Jun 16, 2022

Conversation

LostBenjamin
Copy link

Hi,

This pull request is a fix to a potential variable misuse bug at erpnext/accounts/doctype/pricing_rule/pricing_rule.py. Please check the changes.

It seems to me that the variable in the if condition should be d to check if d is None. If the variable should indeed be field, it would be better to put the condition in a separate if statement.

Best,
Jingxuan

@github-actions github-actions bot added the needs-tests This PR needs automated unit-tests. label Jun 15, 2022
@marination marination self-assigned this Jun 16, 2022
@marination marination removed the needs-tests This PR needs automated unit-tests. label Jun 16, 2022
@marination marination changed the title Fix a potential variable misuse bug chore: Fix a potential variable misuse bug Jun 16, 2022
marination
marination previously approved these changes Jun 16, 2022
@marination marination added the squash Meant to tell reviewers that this PR should be squashed into a single commit while merging. label Jun 16, 2022
@codecov
Copy link

codecov bot commented Jun 16, 2022

Codecov Report

Merging #31372 (ea1095f) into develop (86919d2) will increase coverage by 0.06%.
The diff coverage is 80.00%.

@@             Coverage Diff             @@
##           develop   #31372      +/-   ##
===========================================
+ Coverage    63.61%   63.67%   +0.06%     
===========================================
  Files          984      984              
  Lines        67556    67559       +3     
===========================================
+ Hits         42975    43018      +43     
+ Misses       24581    24541      -40     
Impacted Files Coverage Δ
...next/accounts/doctype/pricing_rule/pricing_rule.py 71.08% <80.00%> (-0.05%) ⬇️
.../report/delayed_item_report/delayed_item_report.py 60.78% <0.00%> (-35.30%) ⬇️
...urity_shortfall/process_loan_security_shortfall.py 93.75% <0.00%> (-6.25%) ⬇️
...eorder_level/itemwise_recommended_reorder_level.py 88.88% <0.00%> (-3.71%) ⬇️
erpnext/crm/doctype/prospect/prospect.py 56.71% <0.00%> (-1.50%) ⬇️
erpnext/stock/doctype/warehouse/warehouse.py 79.72% <0.00%> (-1.40%) ⬇️
...ctype/accounting_dimension/accounting_dimension.py 65.15% <0.00%> (-0.76%) ⬇️
erpnext/selling/doctype/quotation/quotation.py 54.10% <0.00%> (-0.49%) ⬇️
erpnext/payroll/doctype/salary_slip/salary_slip.py 86.89% <0.00%> (-0.25%) ⬇️
erpnext/selling/doctype/sales_order/sales_order.py 80.47% <0.00%> (ø)
... and 17 more

@marination marination merged commit b4a93da into frappe:develop Jun 16, 2022
mergify bot pushed a commit that referenced this pull request Jun 16, 2022
* Fix a potential variable misuse bug

* chore: Separate check (separate line) for empty table in Pricing Rule

* chore: Code readability & check for field in row (now row itself)

Co-authored-by: marination <maricadsouza221197@gmail.com>
(cherry picked from commit b4a93da)

# Conflicts:
#	erpnext/accounts/doctype/pricing_rule/pricing_rule.py
marination added a commit that referenced this pull request Jun 16, 2022
…-31372

chore: Fix a potential variable misuse bug (backport #31372)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport version-13-hotfix squash Meant to tell reviewers that this PR should be squashed into a single commit while merging.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants