Skip to content

[FIX] product: Warning when setting a UOM more precise than Decimal A…#54935

Closed
fah-mili wants to merge 1 commit intoodoo:11.0from
odoo-dev:11.0-stock-product_uom-unreserve-fah
Closed

[FIX] product: Warning when setting a UOM more precise than Decimal A…#54935
fah-mili wants to merge 1 commit intoodoo:11.0from
odoo-dev:11.0-stock-product_uom-unreserve-fah

Conversation

@fah-mili
Copy link
Copy Markdown

…ccuracy

One can set a Unit of Measure precision rounding more
precise than the overall Decimal Accuracy. This can trigger the error
'It is not possible to unreserve more products', as there will be
inconsistencies on reserved quantities due to decimal roundings.
We add a warning, so that the users who make the change are aware of the
impact of modifying their UOM precision.

opw 2171541
opw 2221227
opw 2233937
opw 2198775
and many more

Description of the issue/feature this PR addresses:

Current behavior before PR:

Desired behavior after PR is merged:

--
I confirm I have signed the CLA and read the PR guidelines at www.odoo.com/submit-pr

@robodoo robodoo added CI 🤖 Robodoo has seen passing statuses seen 🙂 labels Jul 27, 2020
@C3POdoo C3POdoo added the OE the report is linked to a support ticket (opw-...) label Jul 27, 2020
@rim-odoo rim-odoo self-requested a review July 27, 2020 07:01
@fah-mili fah-mili force-pushed the 11.0-stock-product_uom-unreserve-fah branch from 9fe7443 to ed2165b Compare July 27, 2020 09:42
@robodoo robodoo added CI 🤖 Robodoo has seen passing statuses and removed CI 🤖 Robodoo has seen passing statuses labels Jul 27, 2020
@fah-mili fah-mili force-pushed the 11.0-stock-product_uom-unreserve-fah branch from ed2165b to d125ef3 Compare July 27, 2020 12:38
@robodoo robodoo added CI 🤖 Robodoo has seen passing statuses and removed CI 🤖 Robodoo has seen passing statuses labels Jul 27, 2020
@amoyaux
Copy link
Copy Markdown
Contributor

amoyaux commented Aug 3, 2020

Globally, I'm not sure that a warning is enough. Maybe we should block the flow at https://github.com/odoo/odoo/blob/11.0/addons/stock/models/stock_move.py#L886 and ensure that the quantity in the move uom is similar to the quantity reserved on the quant

@rim-odoo
Copy link
Copy Markdown
Contributor

rim-odoo commented Aug 4, 2020

Globally, I'm not sure that a warning is enough.

Indeed, it's a first attempt to contain the damages.

Maybe we should block the flow at https://github.com/odoo/odoo/blob/11.0/addons/stock/models/stock_move.py#L886 and ensure that the quantity in the move uom is similar to the quantity reserved on the quant

Ok if it has no huge perf impact

@fah-mili fah-mili force-pushed the 11.0-stock-product_uom-unreserve-fah branch from d125ef3 to 217218c Compare August 27, 2020 11:51
@robodoo robodoo added ☐ ci/runbot CI 🤖 Robodoo has seen passing statuses and removed CI 🤖 Robodoo has seen passing statuses ☐ legal/cla labels Aug 27, 2020
@antonylesuisse
Copy link
Copy Markdown
Contributor

I dont think it's the proper fix, we'll discuss. Also does the warning allow you to proceed if you really want to proceed ?

@fah-mili
Copy link
Copy Markdown
Author

fah-mili commented Aug 28, 2020

Ok, thanks for checking it out.
Yes it does let you proceed if you really want. Which is why @amoyaux suggested to prevent the issue occurring in the first place by ensuring that both the move and the quant have the same reserved quantity (which could differ because of rounding).
(I updated the code yesterday with these changes, but still need to do and write tests)

@fah-mili fah-mili force-pushed the 11.0-stock-product_uom-unreserve-fah branch from 217218c to c245049 Compare September 14, 2020 13:57
@robodoo robodoo removed CI 🤖 Robodoo has seen passing statuses ☑ ci/runbot labels Sep 14, 2020
@robodoo robodoo added CI 🤖 Robodoo has seen passing statuses ☑ ci/runbot labels Sep 21, 2020
@fah-mili fah-mili requested a review from amoyaux September 21, 2020 10:22
@amoyaux
Copy link
Copy Markdown
Contributor

amoyaux commented Sep 21, 2020

robodoo r+

robodoo pushed a commit that referenced this pull request Sep 21, 2020
…on issue

One can set a Unit of Measure precision rounding more
precise than the overall Decimal Accuracy. This can trigger the error
'It is not possible to unreserve more products', as there will be
inconsistencies on reserved quantities due to decimal roundings.
We add 2 warnings on Units of Measure and Decimal Accuracy.
Therefore, users are warned when their UOM/decimal changes can
cause inconsistencies in quants reservation.

Also, we want to prevent the issue at the root by ensuring that both the move
and the quant have the same reserved quantity.
The contrary can happen e.g. when a product is defined in Liters (rounding .001)
but used in an operation in ml (rounding .01, so e.g. 187.5ml).
The conversions between the 2 UOM can cause the differences of reservations.

In _prepare_move_line_vals we make sure uom_quantity_back_to_product_uom
is not more precise than the overall Decimal Accuracy. That way, if
the reserved quantity changes between UOM conversions, the move line created
is in the quant/product UOM, and we do not have move line's reserved quantity >
quantity in stock.
We do the same for _update_reserved_quantity (stock.move), when updating
a move line.

In _generate_consumed_move_line, because of UOM conversions, new_quantity_done
can differ from ml.product_uom_qty, which triggers the creation of an extra
move line, leading to reserved quantity > quantity in stock. To prevent
that, we compare the reserved quantities in the product UOM.

opw 2206902 (Example 1)
opw 2171541
opw 2221227
opw 2233937
opw 2198775
and many more

closes #54935

Signed-off-by: Arnold Moyaux <amoyaux@users.noreply.github.com>
@robodoo robodoo closed this Sep 21, 2020
@robodoo robodoo temporarily deployed to merge September 21, 2020 12:19 Inactive
@fw-bot
Copy link
Copy Markdown
Contributor

fw-bot commented Sep 25, 2020

This pull request has forward-port PRs awaiting action (not merged or closed): #58173

4 similar comments
@fw-bot
Copy link
Copy Markdown
Contributor

fw-bot commented Sep 26, 2020

This pull request has forward-port PRs awaiting action (not merged or closed): #58173

@fw-bot
Copy link
Copy Markdown
Contributor

fw-bot commented Sep 27, 2020

This pull request has forward-port PRs awaiting action (not merged or closed): #58173

@fw-bot
Copy link
Copy Markdown
Contributor

fw-bot commented Sep 28, 2020

This pull request has forward-port PRs awaiting action (not merged or closed): #58173

@fw-bot
Copy link
Copy Markdown
Contributor

fw-bot commented Sep 29, 2020

This pull request has forward-port PRs awaiting action (not merged or closed): #58173

@fw-bot
Copy link
Copy Markdown
Contributor

fw-bot commented Sep 30, 2020

This pull request has forward-port PRs awaiting action (not merged or closed): #58762

2 similar comments
@fw-bot
Copy link
Copy Markdown
Contributor

fw-bot commented Oct 1, 2020

This pull request has forward-port PRs awaiting action (not merged or closed): #58762

@fw-bot
Copy link
Copy Markdown
Contributor

fw-bot commented Oct 3, 2020

This pull request has forward-port PRs awaiting action (not merged or closed): #58762

@fw-bot fw-bot deleted the 11.0-stock-product_uom-unreserve-fah branch October 5, 2020 12:47
@fw-bot
Copy link
Copy Markdown
Contributor

fw-bot commented Oct 11, 2020

This pull request has forward-port PRs awaiting action (not merged or closed): #58762

@fw-bot
Copy link
Copy Markdown
Contributor

fw-bot commented Oct 27, 2020

This pull request has forward-port PRs awaiting action (not merged or closed): #59718

niboo-jva pushed a commit to Niboo/odoo that referenced this pull request Dec 10, 2020
…on issue

One can set a Unit of Measure precision rounding more
precise than the overall Decimal Accuracy. This can trigger the error
'It is not possible to unreserve more products', as there will be
inconsistencies on reserved quantities due to decimal roundings.
We add 2 warnings on Units of Measure and Decimal Accuracy.
Therefore, users are warned when their UOM/decimal changes can
cause inconsistencies in quants reservation.

Also, we want to prevent the issue at the root by ensuring that both the move
and the quant have the same reserved quantity.
The contrary can happen e.g. when a product is defined in Liters (rounding .001)
but used in an operation in ml (rounding .01, so e.g. 187.5ml).
The conversions between the 2 UOM can cause the differences of reservations.

In _prepare_move_line_vals we make sure uom_quantity_back_to_product_uom
is not more precise than the overall Decimal Accuracy. That way, if
the reserved quantity changes between UOM conversions, the move line created
is in the quant/product UOM, and we do not have move line's reserved quantity >
quantity in stock.
We do the same for _update_reserved_quantity (stock.move), when updating
a move line.

In _generate_consumed_move_line, because of UOM conversions, new_quantity_done
can differ from ml.product_uom_qty, which triggers the creation of an extra
move line, leading to reserved quantity > quantity in stock. To prevent
that, we compare the reserved quantities in the product UOM.

opw 2206902 (Example 1)
opw 2171541
opw 2221227
opw 2233937
opw 2198775
and many more

closes odoo#54935

Signed-off-by: Arnold Moyaux <amoyaux@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI 🤖 Robodoo has seen passing statuses OE the report is linked to a support ticket (opw-...)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants