search

OCA / product-pack

Odoo modules related to product packs
GNU Affero General Public License v3.0
48 stars 127 forks source link

[REF] x_product_pack: pricing refactor #173

Closed bruno-zanotti closed 3 weeks ago

bruno-zanotti commented 5 months ago

Pricing refactor

This PR aims to perform a thorough refactor of the product_pack, sale_product_pack and website_sale_product_pack modules. The primary goal is to enhance the pricing logic for product packs, ensuring consistency and improved functionality.

Key improvements / fixes:

  1. Refactor Pack Price Computation: Removed the inheritance from price_compute in favor of a clearer and more comprehensible _get_product_price method within the Pricelist class

  2. 'get_price' -> '_get_pack_line_price': The get_price method on pack lines has been improved and renamed to _get_pack_line_price. It is now capable of returning the price of the pack line for a specified pricelist.

  3. List Price Calculation: Enhanced the _compute_product_lst_price method to return the list price of a pack. It now leverages a new method analogous to price_compute, named _pack_line_price_compute, which can determine the prices of pack components, including nested packs.

  4. Resolves an issue related to the action "Update Prices" on Sale Orders. Now when update the pricelist just take into account the components of packs "Detailed - Detailed per component" and ignore the rest. More info about how to replicate the bug in the commit message.

  5. website Pack prices: show the correct price on the website.

OCA-git-bot commented 5 months ago

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

bruno-zanotti commented 5 months ago

WIP: This PR is based on https://github.com/OCA/product-pack/pull/174 so we need to merge it first

bruno-zanotti commented 1 month ago

Hello @pedrobaeza @FrancoMaxime @augusto-weiss @rousseldenis @ernestotejeda

This PR is mainly a migration to v17 of https://github.com/OCA/product-pack/pull/149. In Adhoc, we are already using it in our v17 clientes and everything seems to work fine. It is ready for review, but not for merge since depends on the migration of website_sale_product_pack https://github.com/OCA/product-pack/pull/174 (I included in the firs commit so you can test it on runboat). It is an important fix / change so let's try to merge it ASAP, I tried to explain the change in the description and commits messages but if you have any doubt please let me know.

bruno-zanotti commented 1 month ago

Thanks for the PR, Great work!

@FrancoMaxime thanks for the review, I already made the changes!

augusto-weiss commented 1 month ago

LGTM!! Waiting on this to migrate product pack to 18!! @bruno-zanotti

OCA-git-bot commented 1 month ago

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

pedrobaeza commented 3 weeks ago

/ocabot merge major

OCA-git-bot commented 3 weeks ago

What a great day to merge this nice PR. Let's do it! Prepared branch 17.0-ocabot-merge-pr-173-by-pedrobaeza-bump-major, awaiting test results.

OCA-git-bot commented 3 weeks ago

Congratulations, your PR was merged at a9782aa9563561c3eac719b917cad4957c60f14f. Thanks a lot for contributing to OCA. ❤️

FrancoMaxime commented 3 weeks ago

Thanks for the PR, Great work!

@FrancoMaxime thanks for the review, I already made the changes!

With a little delay, thanks for the changes!