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: pricing rule item code UOM apply & conversions #32566

Merged
merged 10 commits into from Oct 18, 2022

Conversation

maharshivpatel
Copy link
Contributor

@maharshivpatel maharshivpatel commented Oct 11, 2022

There were multiple issues with pricing rule's apply_on_field == "item_code" ( UOM : blank, other_than_stock_uom etc ). I have added the required checks and proper conversions.

Before:

Before.Pricing.Rule.UOM.mov

After:

After.Pricing.Rule.UOM.mov

there were multiple issues with pricing rule's apply_on_field ==  "item_code" ( UOM : blank, other_than_stock_uom etc ). I have added required checks and proper conversions.
@github-actions github-actions bot added the needs-tests This PR needs automated unit-tests. label Oct 11, 2022
@deepeshgarg007 deepeshgarg007 marked this pull request as ready for review October 12, 2022 08:51
@maharshivpatel maharshivpatel changed the title fix: pricing rule item_code UOM apply & conversions fix: pricing rule item code UOM apply & conversions Oct 12, 2022
Copy link
Member

@deepeshgarg007 deepeshgarg007 left a comment

Choose a reason for hiding this comment

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

Also need a test case for this, please add that too

erpnext/accounts/doctype/pricing_rule/utils.py Outdated Show resolved Hide resolved
maharshivpatel and others added 2 commits October 12, 2022 23:58
Instead of handling multiple cases / conversion i have simplified the rule application.

if pricing rule uom is blank apply on all UOMs and multiple by conversion_factor.

if pricing rule uom is specified apply rule only if sales_uom matches the pricing rule uom.

also added test for both cases.
@maharshivpatel maharshivpatel added the squash Meant to tell reviewers that this PR should be squashed into a single commit while merging. label Oct 12, 2022
remove unused import
maharshivpatel and others added 3 commits October 14, 2022 11:44
pricing rule when called again on_save used get_cached_doc for performance reasons and didn't have uom parameter so looped in child table and specified matching uom.
@codecov
Copy link

codecov bot commented Oct 17, 2022

Codecov Report

Merging #32566 (dfd8ac5) into develop (f08c42e) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop   #32566   +/-   ##
========================================
  Coverage    63.71%   63.71%           
========================================
  Files          816      816           
  Lines        58300    58310   +10     
========================================
+ Hits         37144    37155   +11     
+ Misses       21156    21155    -1     
Impacted Files Coverage Δ
...next/accounts/doctype/pricing_rule/pricing_rule.py 72.69% <100.00%> (+0.73%) ⬆️
erpnext/accounts/doctype/pricing_rule/utils.py 74.05% <100.00%> (+0.38%) ⬆️

@maharshivpatel maharshivpatel removed dont-merge needs-tests This PR needs automated unit-tests. labels Oct 17, 2022
@deepeshgarg007
Copy link
Member

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

@mergify
Copy link
Contributor

mergify bot commented Oct 18, 2022

backport version-13-hotfix version-14-hotfix

✅ Backports have been created

deepeshgarg007 pushed a commit that referenced this pull request Oct 18, 2022
…#32637)

fix: pricing rule for non stock UOM and conversions

* fix: pricing rule for non stock UOM and conversions

(cherry picked from commit 96b4211)

Co-authored-by: Maharshi Patel <39730881+maharshivpatel@users.noreply.github.com>
deepeshgarg007 added a commit that referenced this pull request Oct 18, 2022
…#32636)

* fix: pricing rule for non stock UOM and conversions

* fix: pricing rule for non stock UOM and conversions

(cherry picked from commit 96b4211)

# Conflicts:
#	erpnext/accounts/doctype/pricing_rule/pricing_rule.py

* chore: resolve conflicts

Co-authored-by: Maharshi Patel <39730881+maharshivpatel@users.noreply.github.com>
Co-authored-by: Deepesh Garg <deepeshgarg6@gmail.com>
frappe-pr-bot pushed a commit that referenced this pull request Oct 18, 2022
## [14.3.1](v14.3.0...v14.3.1) (2022-10-18)

### Bug Fixes

* `Brand Defaults` filters ([cb0c4b5](cb0c4b5))
* add project settings to projects workspace (backport [#32568](#32568)) ([#32600](#32600)) ([af4dafd](af4dafd))
* delete old ple's on item value repost ([62cabdf](62cabdf))
* don't try to update youtube data if disabled in settings (backport [#32588](#32588)) ([#32589](#32589)) ([ed85683](ed85683))
* group warehouse filter not working for Batchwise Balance history report ([faedd85](faedd85))
* Ignore linked purchase invoice on cancel ([cc938fb](cc938fb))
* linter ([831f60f](831f60f))
* Party account for multi-order invoices ([eaea846](eaea846))
* pricing rule item code UOM apply & conversions (backport [#32566](#32566)) ([#32637](#32637)) ([ffd82f3](ffd82f3))
* Renamed Dashboard tab label to Connections ([dac5989](dac5989))
* Renamed Notes section to Comments ([9e94dd9](9e94dd9))
* type-cast while saving an item (backport [#32549](#32549)) ([#32578](#32578)) ([829a0ff](829a0ff))
frappe-pr-bot pushed a commit that referenced this pull request Oct 19, 2022
## [13.40.2](v13.40.1...v13.40.2) (2022-10-19)

### Bug Fixes

* `Brand Defaults` filters ([b1c70af](b1c70af))
* don't try to update youtube data if disabled in settings (backport [#32588](#32588)) ([#32590](#32590)) ([7e7122b](7e7122b))
* Ignore linked purchase invoice on cancel ([eec770a](eec770a))
* Party account for multi-order invoices ([0ae2a4f](0ae2a4f))
* pricing rule item code UOM apply & conversions (backport [#32566](#32566)) ([#32636](#32636)) ([b8d97b8](b8d97b8))
@sjain17
Copy link

sjain17 commented Oct 26, 2022

Thank you for fixing this bug; however, it is still not considering the max qty limitations.

maharshivpatel added a commit to maharshivpatel/erpnext that referenced this pull request Nov 9, 2022
Pricing rule's apply_on_field == "item_group" didn't check for UOM. I have added the required checks.

Same as frappe#32566
frappe-pr-bot pushed a commit that referenced this pull request Nov 15, 2022
# [13.42.0](v13.41.1...v13.42.0) (2022-11-15)

### Bug Fixes

* add Document Date in E-Invoice print format ([a16347f](a16347f))
* ambiguous 'cost_center' on payment reconciliation ([e9e5ded](e9e5ded))
* check type for reference name ([ad648f3](ad648f3))
* don't set WIP Warehouse if  is checked in WO ([4a17711](4a17711))
* Label for applicable dimension table ([cbc8b2d](cbc8b2d))
* Pricing rule item group consider UOM ([1ed0b6e](1ed0b6e)), closes [#32566](#32566)
* query condition change ([88ce59a](88ce59a))
* repayment schedule regeneration ([a19031c](a19031c))
* set `WIP Warehouse` in Job Card ([f09e427](f09e427))
* set stock UOM in args to ensure item price is fetched ([bd2242b](bd2242b))
* wrong totals in hsn summary report ([79d508f](79d508f))

### Features

* page break in SoA pdf ([e93ac3c](e93ac3c))
developmentforpeople pushed a commit to Ayuda-Efectiva/erpnext that referenced this pull request Jan 26, 2023
…32566) (frappe#32636)

* fix: pricing rule for non stock UOM and conversions

* fix: pricing rule for non stock UOM and conversions

(cherry picked from commit 96b4211)

# Conflicts:
#	erpnext/accounts/doctype/pricing_rule/pricing_rule.py

* chore: resolve conflicts

Co-authored-by: Maharshi Patel <39730881+maharshivpatel@users.noreply.github.com>
Co-authored-by: Deepesh Garg <deepeshgarg6@gmail.com>
developmentforpeople pushed a commit to Ayuda-Efectiva/erpnext that referenced this pull request Jan 26, 2023
Pricing rule's apply_on_field == "item_group" didn't check for UOM. I have added the required checks.

Same as frappe#32566
developmentforpeople pushed a commit to Ayuda-Efectiva/erpnext that referenced this pull request Jan 26, 2023
## [13.40.2](frappe/erpnext@v13.40.1...v13.40.2) (2022-10-19)

### Bug Fixes

* `Brand Defaults` filters ([b1c70af](frappe@b1c70af))
* don't try to update youtube data if disabled in settings (backport [frappe#32588](frappe#32588)) ([frappe#32590](frappe#32590)) ([7e7122b](frappe@7e7122b))
* Ignore linked purchase invoice on cancel ([eec770a](frappe@eec770a))
* Party account for multi-order invoices ([0ae2a4f](frappe@0ae2a4f))
* pricing rule item code UOM apply & conversions (backport [frappe#32566](frappe#32566)) ([frappe#32636](frappe#32636)) ([b8d97b8](frappe@b8d97b8))
developmentforpeople pushed a commit to Ayuda-Efectiva/erpnext that referenced this pull request Jan 26, 2023
# [13.42.0](frappe/erpnext@v13.41.1...v13.42.0) (2022-11-15)

### Bug Fixes

* add Document Date in E-Invoice print format ([a16347f](frappe@a16347f))
* ambiguous 'cost_center' on payment reconciliation ([e9e5ded](frappe@e9e5ded))
* check type for reference name ([ad648f3](frappe@ad648f3))
* don't set WIP Warehouse if  is checked in WO ([4a17711](frappe@4a17711))
* Label for applicable dimension table ([cbc8b2d](frappe@cbc8b2d))
* Pricing rule item group consider UOM ([1ed0b6e](frappe@1ed0b6e)), closes [frappe#32566](frappe#32566)
* query condition change ([88ce59a](frappe@88ce59a))
* repayment schedule regeneration ([a19031c](frappe@a19031c))
* set `WIP Warehouse` in Job Card ([f09e427](frappe@f09e427))
* set stock UOM in args to ensure item price is fetched ([bd2242b](frappe@bd2242b))
* wrong totals in hsn summary report ([79d508f](frappe@79d508f))

### Features

* page break in SoA pdf ([e93ac3c](frappe@e93ac3c))
@barredterra barredterra mentioned this pull request Mar 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

None yet

3 participants