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

[16.0] [MIG] website_sale_product_pack: Migration to 16.0 #135

Conversation

alvaro-domatix
Copy link

No description provided.

chienandalu and others added 13 commits June 6, 2023 13:14
Compatibility module between sale_product_pack and website_sale

TT30385
Done for e-commerce compatibility purposes, althoug it's more performant
indeed.
Create and select a specific pricelist for avoiding problems in
integrated environments where the default pricelist currency has been
changed.
When the cart is confirmed, a price recalculation is triggered for every
order line. This is wrong for detailed totalized packs, which lines
should be at 0.

TT38186
@alvaro-domatix alvaro-domatix changed the title 16.0 mig website sale product pack [16.0] [MIG] website sale product pack: Migration to 16.0 Jun 7, 2023
@alvaro-domatix alvaro-domatix force-pushed the 16.0-mig-website_sale_product_pack branch 6 times, most recently from 7e9d549 to 13d14d0 Compare June 7, 2023 16:00
@alvaro-domatix alvaro-domatix force-pushed the 16.0-mig-website_sale_product_pack branch 5 times, most recently from 07cd90a to f4afb50 Compare July 4, 2023 11:11
@alvaro-domatix
Copy link
Author

Hi, @ernestotejeda @pedrobaeza !

I would appreciate if you could review this and #134.

@pedrobaeza pedrobaeza changed the title [16.0] [MIG] website sale product pack: Migration to 16.0 [16.0] [MIG] website_sale_product_pack: Migration to 16.0 Jul 4, 2023
@pedrobaeza
Copy link
Member

/ocabot migration website_sale_product_pack

I'm not the most suitable for reviewing this. @chienandalu if you may?

@OCA-git-bot OCA-git-bot added this to the 16.0 milestone Jul 4, 2023
@OCA-git-bot OCA-git-bot mentioned this pull request Jul 4, 2023
5 tasks
@liebana
Copy link
Sponsor

liebana commented Jul 12, 2023

Functional review, LGTM.

As an small improvement (maybe not here), if Pack Display Type is "Detailed" once we add the pack to the cart I think the number of items should be the sum of the child lines instead of the number of packs.

image

@augusto-weiss
Copy link

Hi @alvaro-domatix i'm testing this and looks good! But, I notice a problem with packs detailed per components. On website the price is computed in a wrong way. Do you have any idea about that ?
I am working on that but didn't find a good fix yet!
The problem is that somewhere the context "whole_price_pack" is lost

@augusto-weiss
Copy link

Hi @alvaro-domatix, @pedrobaeza and @chienandalu
I added this pr #144 to fix the problem I mentioned before, could you review it?
Thanks a lot!!

Copy link
Member

@chienandalu chienandalu left a comment

Choose a reason for hiding this comment

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

Thanks @alvaro-domatix :D

Some comments:

@@ -4,10 +4,15 @@
"name": "Website Sale Product Pack",
"category": "E-Commerce",
"summary": "Compatibility module of product pack with e-commerce",
"version": "13.0.1.0.2",
"version": "16.0.1.0.3",
Copy link
Member

Choose a reason for hiding this comment

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

You just need to bump the major version and reset the minor digits :)

Suggested change
"version": "16.0.1.0.3",
"version": "16.0.1.0.0",

Comment on lines +52 to +56
# Neccessary for the website_sale_product_pack module because the price in /shop
# is calculated by the product.template.price_compute method
def price_compute(
self, price_type, uom=False, currency=False, company=False, date=False
):
Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't this override belong to the base module? Isn't there a common method there we can use?

Copy link

github-actions bot commented Mar 3, 2024

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.

@github-actions github-actions bot added the stale PR/Issue without recent activity, it'll be soon closed automatically. label Mar 3, 2024
Comment on lines +9 to +29
def _cart_update(
self, product_id=None, line_id=None, add_qty=0, set_qty=0, **kwargs
):
"""We need to keep the discount defined on the components when checking out.
Also when a line comes from a totalized pack, we should flag it to avoid
changing it's price in a cart step."""
line = self.env["sale.order.line"].browse(line_id)
if line and line.pack_parent_line_id:
pack = line.pack_parent_line_id.product_id
detailed_totalized_pack = (
pack.pack_type == "detailed"
and pack.pack_component_price in {"totalized", "ignored"}
)
return super(
SaleOrder,
self.with_context(
pack_discount=line.discount,
detailed_totalized_pack=detailed_totalized_pack,
),
)._cart_update(product_id, line_id, add_qty, set_qty, **kwargs)
return super()._cart_update(product_id, line_id, add_qty, set_qty, **kwargs)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
def _cart_update(
self, product_id=None, line_id=None, add_qty=0, set_qty=0, **kwargs
):
"""We need to keep the discount defined on the components when checking out.
Also when a line comes from a totalized pack, we should flag it to avoid
changing it's price in a cart step."""
line = self.env["sale.order.line"].browse(line_id)
if line and line.pack_parent_line_id:
pack = line.pack_parent_line_id.product_id
detailed_totalized_pack = (
pack.pack_type == "detailed"
and pack.pack_component_price in {"totalized", "ignored"}
)
return super(
SaleOrder,
self.with_context(
pack_discount=line.discount,
detailed_totalized_pack=detailed_totalized_pack,
),
)._cart_update(product_id, line_id, add_qty, set_qty, **kwargs)
return super()._cart_update(product_id, line_id, add_qty, set_qty, **kwargs)
def _cart_update(
self, *args, **kwargs
):
"""We need to keep the discount defined on the components when checking out.
Also when a line comes from a totalized pack, we should flag it to avoid
changing it's price in a cart step."""
line_id = kwargs.get("line_id")
if line_id:
line = self.env["sale.order.line"].browse(line_id)
if line and line.pack_parent_line_id:
pack = line.pack_parent_line_id.product_id
detailed_totalized_pack = (
pack.pack_type == "detailed"
and pack.pack_component_price in {"totalized", "ignored"}
)
return super(
SaleOrder,
self.with_context(
pack_discount=line.discount,
detailed_totalized_pack=detailed_totalized_pack,
),
)._cart_update(*args, **kwargs)
return super()._cart_update(*args, **kwargs)

@alvaro-domatix Odoo has recently made these changes and now I have an error caused by this module and that I have been able to solve in this way. Can you apply the changes? Or if you think it should be done another way...

  File "/odoo/odoo/addons/website_sale_delivery/models/sale_order.py", line 76, in _cart_update
    return super()._cart_update(*args, **kwargs)
  File "/oca/product-pack/website_sale_product_pack/models/sale_order.py", line 29, in _cart_update
    return super()._cart_update(product_id, line_id, add_qty, set_qty, **kwargs)
  File "/odoo/odoo/addons/website_sale_loyalty/models/sale_order.py", line 160, in _cart_update
    product_id, set_qty = kwargs['product_id'], kwargs.get('set_qty')
KeyError: 'product_id'

Copy link
Member

Choose a reason for hiding this comment

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

Choose a reason for hiding this comment

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

@pedrobaeza
Copy link
Member

Superseded by #165

@pedrobaeza pedrobaeza closed this Apr 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved stale PR/Issue without recent activity, it'll be soon closed automatically.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants