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][IMP] shopinvader_product: main variant: add hooks #1524

Conversation

marielejeune
Copy link
Contributor

@marielejeune marielejeune commented Mar 27, 2024

Forward-port of #1529

@simahawk
Copy link
Contributor

@marielejeune can you please review this #1529 and fwd port here instead?

@marielejeune marielejeune force-pushed the 16.0-shopinvader_product_extract_method-mle branch from 2561e0b to 73ad7bb Compare April 24, 2024 12:58
@marielejeune marielejeune force-pushed the 16.0-shopinvader_product_extract_method-mle branch 4 times, most recently from 70e1be3 to 5e3cbd7 Compare April 24, 2024 13:47
@marielejeune marielejeune changed the title [16.0][IMP] shopinvader_product: extract inner function to allow extending it [16.0][IMP] shopinvader_product: main variant: add hooks Apr 24, 2024
shopinvader_product/models/product_product.py Outdated Show resolved Hide resolved
# NOTE: if the order is changed by adding `asc/desc` this can be broken
# but it's very unlikely that the default order for product.product
# will be changed.
order_by = [x.strip() for x in self.env["product.product"]._order.split(",")]
Copy link
Contributor

Choose a reason for hiding this comment

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

order_by used to be the same as the one used to compute fields_to_read. I'm a bit lost, but shouldn't we keep this consistency ? @simahawk ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You would like to extract the compute of order_by from _get_main_product_read_fields into a new hook to re-use it here into _get_main_product_sorted_variants right?

Copy link
Contributor

Choose a reason for hiding this comment

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

If necessary, yes.

@marielejeune marielejeune force-pushed the 16.0-shopinvader_product_extract_method-mle branch from 5e3cbd7 to aca445c Compare May 2, 2024 10:13
@marielejeune
Copy link
Contributor Author

It seems that there is a problem with Codecov but I cannot rerun the job.

@sebastienbeau sebastienbeau added this to the 16.0 milestone Jun 3, 2024
@lmignon
Copy link
Collaborator

lmignon commented Jun 4, 2024

/ocabot rebase

@lmignon
Copy link
Collaborator

lmignon commented Jun 4, 2024

/ocabot merge patch

@shopinvader-git-bot
Copy link

On my way to merge this fine PR!
Prepared branch 16.0-ocabot-merge-pr-1524-by-lmignon-bump-patch, awaiting test results.

@shopinvader-git-bot shopinvader-git-bot merged commit b585e99 into shopinvader:16.0 Jun 4, 2024
2 of 3 checks passed
@shopinvader-git-bot
Copy link

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

@lmignon lmignon deleted the 16.0-shopinvader_product_extract_method-mle branch June 4, 2024 10:34
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.

None yet

7 participants