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.

Sign up for GitHub

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

Jump to bottom

[11.0][FIX] mrp_bom_current_stock: UoM compute quantity error #53

Merged
merged 1 commit into from May 27, 2019
Merged

[11.0][FIX] mrp_bom_current_stock: UoM compute quantity error #53

merged 1 commit into from May 27, 2019

Conversation

HviorForgeFlow
Copy link
Member

CC @jbeficent

chienandalu
chienandalu approved these changes Mar 7, 2019
View reviewed changes
Copy link
Member

@chienandalu chienandalu left a comment

Choose a reason for hiding this comment

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

Obvious 😄 👍

AaronHForgeFlow
AaronHForgeFlow approved these changes Mar 7, 2019
View reviewed changes
Copy link

@AaronHForgeFlow AaronHForgeFlow left a comment

Choose a reason for hiding this comment

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

👍 LGTM

@OCA-git-bot
Copy link
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

LoisRForgeFlow
LoisRForgeFlow requested changes Mar 7, 2019
View reviewed changes
Copy link
Contributor

@LoisRForgeFlow LoisRForgeFlow left a comment

Choose a reason for hiding this comment

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

if this is true, then do not call the convert method at all.

Could you add some tests?

LoisRForgeFlow
LoisRForgeFlow requested changes Mar 25, 2019
View reviewed changes
Copy link
Contributor

@LoisRForgeFlow LoisRForgeFlow left a comment

Choose a reason for hiding this comment

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

You should update the qty available computation, now it is incorrect. I was reproducing your test BoMs locally and if found this:

image

image

So, what's available is 10 units (not 120 dozens), I guess the conversion is reversed (10 [units] * 12 [units/dozens] instead of 10 [units] / 12 [units/dozens]).

@HviorForgeFlow HviorForgeFlow force-pushed the 11.0-fix-mrp_bom_current_stock branch 3 times, most recently from 5ee6c67 to 0943da1 Compare March 26, 2019 13:05
@HviorForgeFlow
Copy link
Member Author

HviorForgeFlow commented Mar 28, 2019 •
edited

@lreficent it seems ready

@HviorForgeFlow
Copy link
Member Author

ping @lreficent 😄

@LoisRForgeFlow
Copy link
Contributor

Problem still persists 😓

@HviorForgeFlow HviorForgeFlow force-pushed the 11.0-fix-mrp_bom_current_stock branch 2 times, most recently from 32f74bb to 70965ac Compare May 21, 2019 11:45
@HviorForgeFlow HviorForgeFlow changed the title [11.0][FIX] UoM compute quantity error [11.0][FIX] mrp_bom_current_stock: UoM compute quantity error May 21, 2019
@HviorForgeFlow HviorForgeFlow force-pushed the 11.0-fix-mrp_bom_current_stock branch 2 times, most recently from 8f4b053 to ee51032 Compare May 21, 2019 12:29
@HviorForgeFlow HviorForgeFlow force-pushed the 11.0-fix-mrp_bom_current_stock branch from ee51032 to 98b2251 Compare May 27, 2019 08:25
LoisRForgeFlow
LoisRForgeFlow approved these changes May 27, 2019
View reviewed changes
Copy link
Contributor

@LoisRForgeFlow LoisRForgeFlow left a comment

Choose a reason for hiding this comment

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

Looks good now 👍

@OCA-git-bot
Copy link
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

@LoisRForgeFlow LoisRForgeFlow merged commit 348c821 into OCA:11.0 May 27, 2019
@LoisRForgeFlow LoisRForgeFlow deleted the 11.0-fix-mrp_bom_current_stock branch May 27, 2019 10:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chienandalu chienandalu chienandalu approved these changes

@AaronHForgeFlow AaronHForgeFlow AaronHForgeFlow approved these changes

@LoisRForgeFlow LoisRForgeFlow LoisRForgeFlow approved these changes

Assignees
No one assigned
Labels
approved ready to merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@HviorForgeFlow @OCA-git-bot @LoisRForgeFlow @chienandalu @AaronHForgeFlow