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

[IMP] website_sale_product_assortment #975

Open
wants to merge 1 commit into
base: 16.0
Choose a base branch
from

Conversation

Pablocce
Copy link

@Pablocce Pablocce commented Oct 15, 2024

Description:

When more than one assortment is applied to the customer on e-commerce, the functionality doesn't work correctly, and the configurations are not applied properly, for example, when combining "no restriction" with "no show" or "no purchase."

This PR is essential for the functionality of this module because the products are not shown or cannot be bought when they should.

Cases covered with this improvement:

  1. Assortment A with product 1 and configuration "Avoid to show non available products" and assortment B with product 2 and configuration "Avoid to show non available products". Before, both products were displayed with the label "Not available". With this PR both products are displayed and they are allowed to be purchased.
  2. Assorment A with product 1 and configuration "Avoid to show non available products" and assortment B with product 2 and configuration "No restrictions". Before, only the products from de assortment A were displayed, restricting the products from de B assortment. With this PR, the restrictions are not applied to the assortments with the configuration "No restrictions" when simultaneous assortments are applied to the same client or website.

Solution:
To resolve this problem, we have made the following modifications:

  1. The method that returns the products to display is adjusted so that, in the absence of restrictive stock, it correctly returns the values.
  2. Additionally, the method that returns the products and their restrictions is adjusted to make the assortments compatible.

@OCA-git-bot
Copy link
Contributor

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

@Pablocce Pablocce force-pushed the 16.0-imp-website_sale_product_assortment branch 5 times, most recently from ef4cf6f to f58223c Compare October 20, 2024 21:47
@Pablocce Pablocce marked this pull request as ready for review October 28, 2024 08:52
@Pablocce Pablocce force-pushed the 16.0-imp-website_sale_product_assortment branch from 3fb2022 to a249a20 Compare October 28, 2024 08:55
@pedrobaeza pedrobaeza added this to the 16.0 milestone Nov 4, 2024
no_restriction_assortments = any(
assortment.website_availability == "no_show" for assortment in assortments
)

Copy link
Member

Choose a reason for hiding this comment

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

Please remove extra empty lines inside a method.

Copy link
Author

Choose a reason for hiding this comment

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

Done! Thanks for the correction

Copy link
Contributor

@CarlosRoca13 CarlosRoca13 left a comment

Choose a reason for hiding this comment

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

Thank you so much! please remove all the extra empty lines

assortment_restriction = False
allowed_product_ids = set()

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change


if not no_restriction_assortments:
return allowed_product_ids, assortment_restriction

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change

"|",
("website_ids", "=", website.id),
("website_ids", "=", False),
]
)
)
assortment_dict = {}
partner_assortments = self.env["ir.filters"].sudo()

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change

if assortment.website_availability != "no_restriction":
partner_assortments |= assortment
allowed_product_ids.update(assortment.all_product_ids.ids)

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change

@CarlosRoca13
Copy link
Contributor

And if you can, add a test to check that is working OK on future migrations 😄

@Pablocce Pablocce force-pushed the 16.0-imp-website_sale_product_assortment branch 2 times, most recently from 2abb8e5 to 60bc3f6 Compare November 8, 2024 15:01
Copy link
Contributor

@CarlosRoca13 CarlosRoca13 left a comment

Choose a reason for hiding this comment

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

Thanks! 😄

@CarlosRoca13
Copy link
Contributor

Ups! the whitespaces still there...

@Pablocce Pablocce force-pushed the 16.0-imp-website_sale_product_assortment branch from 60bc3f6 to a60663d Compare November 8, 2024 15:26
… products

* Methods that calculate product availability based on e-commerce assortments are being corrected to make them compatible.
@Pablocce Pablocce force-pushed the 16.0-imp-website_sale_product_assortment branch from a60663d to efbb7a1 Compare November 8, 2024 15:26
Copy link
Contributor

@CarlosRoca13 CarlosRoca13 left a comment

Choose a reason for hiding this comment

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

👍

@Pablocce
Copy link
Author

Pablocce commented Nov 8, 2024

Ups! the whitespaces still there...

Done! I was doing the rebase, thanks!

@OCA-git-bot
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants