Skip to content

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

Closed
fw-bot wants to merge 2 commits intoodoo:13.0from
odoo-dev:13.0-11.0-stock-product_uom-unreserve-fah-H5yt-fw
Closed

[FW][FIX] product: Warning when setting a UOM more precise than Decimal A…#59718
fw-bot wants to merge 2 commits intoodoo:13.0from
odoo-dev:13.0-11.0-stock-product_uom-unreserve-fah-H5yt-fw

Conversation

@fw-bot
Copy link
Copy Markdown
Contributor

@fw-bot fw-bot commented Oct 12, 2020

…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

Forward-Port-Of: #58762
Forward-Port-Of: #54935

@robodoo robodoo added conflict There was an error while creating this forward-port PR forwardport This PR was created by @fw-bot seen 🙂 labels Oct 12, 2020
@fw-bot
Copy link
Copy Markdown
Contributor Author

fw-bot commented Oct 12, 2020

Ping @fah-odoo, @amoyaux
Cherrypicking 143610e of source #54935 failed

stderr:

error: could not apply 143610e0bcc... [FIX] product, decimal_precision, stock, mrp: Prevent quant reservation issue
hint: after resolving the conflicts, mark the corrected paths
hint: with 'git add <paths>' or 'git rm <paths>'
hint: and commit the result with 'git commit'

Either perform the forward-port manually (and push to this branch, proceeding as usual) or close this PR (maybe?).

In the former case, you may want to edit this PR message as well.

@C3POdoo C3POdoo added the OE the report is linked to a support ticket (opw-...) label Oct 12, 2020
@fah-mili fah-mili force-pushed the 13.0-11.0-stock-product_uom-unreserve-fah-H5yt-fw branch from bca8ad8 to ecff018 Compare October 16, 2020 14:48
@fah-mili
Copy link
Copy Markdown

Cannot reproduce the issues for _prepare_move_line_vals and _update_reserved_quantity (the tests in test_move2 always pass)
but can reproduce for _update_move_lines (the test in test_order fails before the change in _update_move_lines)

Not sure it can be merged straight away, to recheck next week.

@robodoo robodoo added ☑ ci/template CI 🤖 Robodoo has seen passing statuses and removed ☐ ci/template labels Oct 16, 2020
@fah-mili
Copy link
Copy Markdown

Hello @amoyaux
I still couldn't determine why for _prepare_move_line_vals and _update_reserved_quantity, the tests in test_move2 always pass (before the changes).

I'll be away for a bit, should we check it when I return? Or do you think these changes can still be done without breaking something elsewhere?
Thanks for your help!

@amoyaux
Copy link
Copy Markdown
Contributor

amoyaux commented Oct 29, 2020

At the move line creation. The value contains 0,1498 instead of 0.15. The use case still failing in reality. Probably need to check with framework team why the cache is not truncate to the digits (as in 12.0).

Comment on lines 2163 to 2164
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Never compare floats like that, use float_compare!

@rco-odoo
Copy link
Copy Markdown
Member

rco-odoo commented Nov 9, 2020

@fah-odoo @amoyaux I think that the difference between this version and the former ones comes from the ORM-pocalypse.
We modified the low-level method that reads from database so that there is no conversion between the value from the database and the one we put in cache. In 12.0:

odoo/odoo/models.py

Lines 2971 to 2972 in fe75ca9

values = [convert(value, target, validate=False) for value in values]
self.env.cache.update(fetched, field, values)

In 13.0:
self.env.cache.update(fetched, field, values)

The conversion used to "fix" the bad rounding from the database, and it is no longer the case.
Note that there are raw float comparisons in your tests (see comments above); if you use float_compare, things should work as expected.

@robodoo robodoo removed CI 🤖 Robodoo has seen passing statuses ☑ ci/runbot labels Nov 10, 2020
@robodoo robodoo added ☐ ci/runbot CI 🤖 Robodoo has seen passing statuses and removed ☑ legal/cla CI 🤖 Robodoo has seen passing statuses labels Nov 10, 2020
rco-odoo and others added 2 commits November 12, 2020 10:26
When modifying a field's decimal precision, the caches are cleared in
order to retrieve the new value from the database.  As the new value is
retrieved in pure SQL, one has to flush the updates before doing the
query!
…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 _update_move_lines, 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

X-original-commit: 9131852
@rco-odoo
Copy link
Copy Markdown
Member

@robodoo r+

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 conflict There was an error while creating this forward-port PR forwardport This PR was created by @fw-bot OE the report is linked to a support ticket (opw-...)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants