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] purchase_stock: Bill currency different than PO #162827

Closed

Conversation

HANNICHE-Walid
Copy link
Member

@HANNICHE-Walid HANNICHE-Walid commented Apr 22, 2024

Steps to reproduce:

The Company Currency is the Dollar
Create a product > Set FIFO and Manual valuation
Set BIlling policy as Ordered Quantities
Create Purchase order in Euro
Create Vendor bill in USD (10 $)
Now Receive the quantities
Check the valuation
Wrong value (15.92 $)
the value should be 10$ (taken from the bill)

Bug:
In the case of BIlling policy on Ordered Quantities and PO in a foreign
currency we assume the bill will be in same currency as the PO

Fix:
currently unit price is first computed in PO currency and then converted
in the end to company currency added conversion from bill to PO
opw-3805454

also fixed a rounding issue opw-3773413

alternative fix:
compute everything in company currency (#155937)

@robodoo
Copy link
Contributor

robodoo commented Apr 22, 2024

@C3POdoo C3POdoo requested a review from a team April 22, 2024 12:12
@C3POdoo C3POdoo added the OE the report is linked to a support ticket (opw-...) label Apr 22, 2024
@HANNICHE-Walid HANNICHE-Walid force-pushed the 16.0-opw-3805454-bill_cuurency-waha branch from 5170c9f to 517cba4 Compare April 22, 2024 13:41
Copy link
Contributor

@Sehnde Sehnde left a comment

Choose a reason for hiding this comment

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

Hello,
Maybe I missed something but when I create a bill, it is in the currency of the PO (€10/unit). Then if I switch the currency to $, it simply changes the sign without doing any conversion ($10/unit)
Then if I receive the product and look at the valuation, it will be in the main currency ($) and properly converted ($15.29/unit)

This is different from the steps you explained, so is there still a need for this PR?

@HANNICHE-Walid
Copy link
Member Author

HANNICHE-Walid commented May 7, 2024

and properly converted ($15.29/unit)

it shouldn't have been converted that's the point
on the bill its value is 10$ if the bill is already posted when we receive the product that's the value we should use for its stock valuation
(we are converting 10$ to 15.29$)

@HANNICHE-Walid HANNICHE-Walid requested a review from Sehnde May 7, 2024 10:40
@HANNICHE-Walid
Copy link
Member Author

hi @Sehnde you can refer to the test to check the incorrect behaviour

@Sehnde
Copy link
Contributor

Sehnde commented May 8, 2024

@HANNICHE-Walid ok I see what you mean. Would you please update your commit and PR message to explain that? This is not clear.

Now for your fix, it will immediately create a layer at the bill value in the company currency. So for the following case:

  • PO -> 1 unit of product A (purchase_method == 'purchase'), €10/unit
  • create bill -> 1 unit of product A, $10/unit, validate
  • receive product

the SVL will be at $10 instead of $15.29 in a single layer.

Comparatively, if purchase_method == 'receive', it will create 2 layers: one at $15.29 and one at -$5.29
Now here's my idea: we could keep the same result on both purchase_method: 1 layer at converted value and 1 correction layer. To do this, we could call _generate_price_difference_vals after the converted layer is created.
For that, you'd need to add an inherited method _action_done in purchase_stock/models/stock_move.py that would look like this:

    def _action_done(self, cancel_backorder=False):
        res = super()._action_done(cancel_backorder=cancel_backorder)
        if res.product_id.purchase_method == 'purchase' and res.purchase_line_id.invoice_lines:
            amls = res.purchase_line_id.invoice_lines
            svls = res.stock_valuation_layer_ids
            correction_svl_vals, _dummy = res.purchase_line_id.invoice_lines._generate_price_difference_vals(res.stock_valuation_layer_ids)
        return res

This is a very rough draft that does not care for multiple invoice lines or layers, but it works as a proof of concept.

This is just an idea, do you have an opinion @amoyaux ?

@amoyaux
Copy link
Contributor

amoyaux commented May 8, 2024

Why would you need a correction layer? When you receive the product you take the bill value and that's all

@HANNICHE-Walid
Copy link
Member Author

R+ ?

@Sehnde
Copy link
Contributor

Sehnde commented May 14, 2024

Why would you need a correction layer? When you receive the product you take the bill value and that's all

I wanted to keep the same behavior as when receiving quantities then invoicing a different total price. Not a good idea evidently.

@HANNICHE-Walid some remarks from me:

  • please update the commit message to make it clear that we don't want the valuation to convert the PO value.

In valuation taking the price from the bill, and currency from PO.

This line made me think that the valuation layer was in the PO currency (€) with the bill value (10) so I was expecting to see €10 in valuation instead of $15.29

  • change invoice_value into something that is distinct enough from invoiced_value. There's only 1 letter difference and it is not obvious at first glance. I'd suggest invoice_line_value
  • if the bill already is in the company currency, could you skip the 2 conversions? In your test, the price unit returned is 99.999999 instead of 100. It is rounded later but we could avoid that and gain time by simply taking the invoice value if it's already in the company currency.

Steps to reproduce:
> The Company Currency is the Dollar
> Create a product > Set FIFO and Manual valuation
> Set BIlling policy as Ordered Quantities
> Create Purchase order in Euro
> Create Vendor bill in USD (10 $)
> Now Receive the quantities
> Check the valuation
> Wrong value (15.92 $)
the value should be 10$ (taken from the bill)

Bug:
In the case of BIlling policy on Ordered Quantities and PO in a foreign
currency we assume the bill will be in same currency as the PO

Fix:
currently unit price is first computed in PO currency and then converted
in the end to company currency added conversion from bill to PO
opw-3805454

also fixed a rounding issue opw-3773413

alternative fix:
compute everything in company currency (odoo#155937)
@HANNICHE-Walid HANNICHE-Walid force-pushed the 16.0-opw-3805454-bill_cuurency-waha branch from 517cba4 to 26db1c6 Compare May 14, 2024 13:30
@HANNICHE-Walid
Copy link
Member Author

  • if the bill already is in the company currency, could you skip the 2 conversions? In your test, the price unit returned is 99.999999 instead of 100. It is rounded later but we could avoid that and gain time by simply taking the invoice value if it's already in the company currency.

this was also done to account for the case of the bill being in another different currency aswell (although an unlikely scenario)
right now we are doing the computations in the PO currency and finally converting everything back to the company currency. I also suggested doing all the computations In the company currency as an alternative.

@HANNICHE-Walid
Copy link
Member Author

hi @Sehnde I have updated the PR

@HANNICHE-Walid
Copy link
Member Author

@amoyaux R+ please

@nle-odoo
Copy link
Contributor

robodoo delegate+

@HANNICHE-Walid
Copy link
Member Author

@robodoo r+

robodoo pushed a commit that referenced this pull request May 22, 2024
Steps to reproduce:
> The Company Currency is the Dollar
> Create a product > Set FIFO and Manual valuation
> Set BIlling policy as Ordered Quantities
> Create Purchase order in Euro
> Create Vendor bill in USD (10 $)
> Now Receive the quantities
> Check the valuation
> Wrong value (15.92 $)
the value should be 10$ (taken from the bill)

Bug:
In the case of BIlling policy on Ordered Quantities and PO in a foreign
currency we assume the bill will be in same currency as the PO

Fix:
currently unit price is first computed in PO currency and then converted
in the end to company currency added conversion from bill to PO
opw-3805454

also fixed a rounding issue opw-3773413

alternative fix:
compute everything in company currency (#155937)

closes #162827

Signed-off-by: Walid Hanniche (waha) <waha@odoo.com>
@robodoo robodoo closed this May 22, 2024
@fw-bot
Copy link
Contributor

fw-bot commented May 26, 2024

1 similar comment
@fw-bot
Copy link
Contributor

fw-bot commented May 27, 2024

@fw-bot fw-bot deleted the 16.0-opw-3805454-bill_cuurency-waha branch June 5, 2024 09:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OE the report is linked to a support ticket (opw-...)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants