[16.0][FIX] product_pack: doing get_price more like price_compute method to avoid errors

[OCA-git-bot]

Hi @ernestotejeda, some modules you are maintaining are being modified, check this out!

[augusto-weiss]

@FrancoMaxime Hi Franco, I made this PR based on yours (#121) to fix problems getting price when the pricelist is based on Cost (standard_price). So, the most important change is the get_price method which I try to make seem more like the price_compute method.

To reproduce the error: Create a pricelist based on cost Create a product pack and add some components Add the pack to a new sale order with the pricelist based on cost The price unit is the sum of components list price and not the sum of cost price

How looks this fix?

[augusto-weiss]

@FrancoMaxime Sure!! thats why i made a new PR!! thanks a lot

[augusto-weiss]

Hi @FrancoMaxime i saw your PR was merged, good job and thanks for contributing!!

I resume this PR, could you review it ?

[augusto-weiss]

Thanks @FrancoMaxime i added a test case !

[augusto-weiss]

Hi @rousseldenis and @pedrobaeza , Could you too review this PR? Thanks a lot

[augusto-weiss]

Hi @pedrobaeza and @rousseldenis thanks for answer! I updated the changes.

The issue: problems getting price when the pricelist is based on Cost (standard_price).

To reproduce the error: Create a pricelist based on cost Create a product pack and add some components Add the pack to a new sale order with the pricelist based on cost The price unit is the sum of components list price and not the sum of cost price

[rousseldenis]

I now understand the problem you are mentioning, but what I don't get is why don't we just rely on Odoo itself for getting the proper sales price instead of redoing the pricelist engine.

@augusto-weiss I think @pedrobaeza is right as going up the stack make me thing we should use quite the same process done in sale :

https://github.com/odoo/odoo/blob/16.0/addons/sale/models/sale_order_line.py#L437

What do you think of ?

@FrancoMaxime

[augusto-weiss]

That's interesting!! But at the end of all, the method _get_display_price (defined in sale order line) call the method price_compute, which we are redefining in product_pack, and before this PR didn't consider the pricelist type (price_type in odoo: https://github.com/odoo/odoo/blob/16.0/addons/product/models/product_product.py#L609).

So, working at sale order line level, we need to redefine some workflow getting the pack price!

Right ? or I am confused ? @pedrobaeza @rousseldenis @FrancoMaxime

[pedrobaeza]

But IMO we should call _get_display_price for each component on the price_compute overwrite, which is the aim of this.

[FrancoMaxime]

Hi, @augusto-weiss @pedrobaeza and @rousseldenis I agree with you when you say the compute method for the price should approach the standard. But It will be hard for a product.pack.line to use a method inside a sale.order.line that needs information from a sale.order. But maybe i'm missing some information to see how to that without creating a sale.order with the new() method

[pedrobaeza]

Hi @FrancoMaxime, you can extract a bit of the code from sale to get it. Something like:

order = pack_line.order_id
order_date = order.date_order or fields.Date.today()
product = pack_line.product_id
qty = pack_line.product_uom_qty or 1.0
uom = pack_line.product_uom or product.uom_id
pricelist_rule = order.pricelist_id._get_product_rule(product, qty, uom=uom, date=order_date)
price = pricelist_rule._compute_price(product, qty, uom, order_date, currency=order.currency_id)

Although you can also instantiate a virtual sale.order.line (with new) and get the price.

[nicolascol]

Hi guys! I tested this PR and the behavior was as expected. Solves the problem about the price calculation when the pricelist it's based on cost

[OCA-git-bot]

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). 🤖

[bruno-zanotti]

Hey @pedrobaeza,

I've read the comments and I agree with you that redoing the pricelist engine might not be the best solution. However, I'm still not sure what the best approach is, maybe I’m missing something, but _get_display_price it only defined for sale.order.line, so to start we should move this to sale_product_pack.. but then what happen with other models that ends calling price_compute? for example here or the method _compute_price

I suggested trying with the method _get_contextual_price but we don't get any better solution..

[pedrobaeza]

Why my proposal in https://github.com/OCA/product-pack/pull/133#issuecomment-1598484645 is not valid?

[bruno-zanotti]

Because we don't have an order yet... and making a virtual sale.order.line wouldn't work because of what I said before. I think the method get_price is more generic, that's why I think we should use _get_contextual_price..

[pedrobaeza]

You should have already the date and the pricelist, so it's not involving to create a virtual SO line. Please check again.

[augusto-weiss]

Hi @pedrobaeza i implemented some of your recommendations! Please take a look First, I reverted the changes and then implemented the code but in sale_product_pack, the problem is that these changes don't work for e-commerce, maybe need to be implemented in website_product_pack also ?

[pedrobaeza]

Can you please put CI green?

[bruno-zanotti]

Hi @augusto-weiss, I made a small fix in a new commit to avoid computing the pack price when the pack is detailed by components, please review and squash if you think it's ok.

To replicate the bug just add a pack detailed per component to a sale order and check the prices

[github-actions[bot]]

There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days. If you want this PR to never become stale, please ask a PSC member to apply the "no stale" label.