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][FIX] product_variant_configurator: Solve recursion error. #331

Conversation

JordiBForgeFlow
Copy link
Member

This fix solves a recursion problem caused when the groupby refers to a field of a many2one model, and that model also has the same field name. In this example, the stock.valuation.layer.tree view contains a groupby product_id, and the product.product model already has a product_id field, relating to the same model. Without this code we would reach a recursion error.

Fixes #330

cc @JordiMForgeFlow @MarinaAForgeFlow @LauraCForgeFlow @pedrobaeza

This fix solves a recursion problem caused when the groupby refers to a
field of a many2one model, and that model also has the same field name.
In this example, the stock.valuation.layer.tree view contains a groupby
product_id, and the product.product model already has a product_id field,
relating to the same model. Without this code we would reach a recursion error
Copy link

@JordiMForgeFlow JordiMForgeFlow left a comment

Choose a reason for hiding this comment

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

Makes sense 👍🏼

Copy link

@MarinaAForgeFlow MarinaAForgeFlow left a comment

Choose a reason for hiding this comment

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

LGTM

@pedrobaeza pedrobaeza added this to the 16.0 milestone Jan 11, 2024
Copy link
Member

@pedrobaeza pedrobaeza left a comment

Choose a reason for hiding this comment

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

Great hack for avoiding to do other massive changes, and well documented for not having future doubts about its usage. The test coverage is limited though, so this can fail again in the future (or future versions), but it will require a tour test for recreating the condition, so let's continue as is.

/ocabot merge patch

@OCA-git-bot
Copy link
Contributor

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

@OCA-git-bot OCA-git-bot merged commit 2eb076a into OCA:16.0 Jan 11, 2024
5 of 7 checks passed
@OCA-git-bot
Copy link
Contributor

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

@JordiBForgeFlow
Copy link
Member Author

@pedrobaeza thanks for your review.

@LoisRForgeFlow LoisRForgeFlow deleted the 16.0-fix-product_variant_configurator-postprocess-tag-groupby branch January 16, 2024 12:17
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