Skip to content

[18.0][MIG] product_attribute_value_dependent_mixin: Migration to 18.0#2250

Open
metaminux wants to merge 3 commits intoOCA:18.0from
akretion:18.0-mig-product_attribute_value_dependent_mixin
Open

[18.0][MIG] product_attribute_value_dependent_mixin: Migration to 18.0#2250
metaminux wants to merge 3 commits intoOCA:18.0from
akretion:18.0-mig-product_attribute_value_dependent_mixin

Conversation

@metaminux
Copy link
Copy Markdown

Migration of product_attribute_value_dependent_mixin module from 16.0 to 18.0.

Source PRs (not merged in 16.0):

cc @Kev-Roche @mourad-ehm for review

@metaminux
Copy link
Copy Markdown
Author

Hello @bealdav and @florian-dacosta

Can you review this migration PR ?

Copy link
Copy Markdown

@florian-dacosta florian-dacosta left a comment

Choose a reason for hiding this comment

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

Technical review, not test (not sure in which submodule it is used ?)

@metaminux metaminux force-pushed the 18.0-mig-product_attribute_value_dependent_mixin branch from 36439b5 to e80d3bf Compare April 15, 2026 20:23
@OCA-git-bot OCA-git-bot added mod:product_attribute_value_dependent_mixin Module product_attribute_value_dependent_mixin series:18.0 labels Apr 15, 2026
@metaminux metaminux force-pushed the 18.0-mig-product_attribute_value_dependent_mixin branch 2 times, most recently from 7c4ae3f to 3e29099 Compare April 15, 2026 22:14
@metaminux
Copy link
Copy Markdown
Author

Last commit fixes several bugs in the original implementation and significantly improves the documentation and test coverage.

Typo fix in model name

attribute.value.dependant.mixin (French spelling) has been renamed to attribute.value.dependent.mixin (correct English spelling). Since this is an AbstractModel with no database table, no migration script is needed — consumers just need to update their _inherit.

available_product_ids replaced by available_product_domain

The original available_product_ids Many2many compute field had several bugs:

  • self.product_tmpl_id was used instead of rec.product_tmpl_id inside the for rec in self loop, causing incorrect behavior on multi-record recordsets.
  • _origin was used unnecessarily and caused the domain [("product_tmpl_id", "=", False)] to be applied when product_tmpl_id was not set, matching all products without a template.
  • The missing if rec.product_tmpl_id guard meant the field was never properly assigned in the False case.
  • Performing a search() inside a compute is expensive and unnecessary since product_tmpl_id.product_variant_ids already holds the variants as a stored field.

The field has been replaced by available_product_domain (a Binary field following the same pattern already used for available_attribute_value_domain), and the domain attribute has been removed from product_id in the model — it belongs in the view, where the consuming module can reference available_product_domain explicitly. The @api.depends chain has also been simplified: "product_tmpl_id.product_variant_ids" alone is sufficient, as Odoo registers triggers for each level of a dotted dependency chain.

_compute_available_attribute_value_ids removed

This method was computing available_attribute_value_ids, a field that was never declared in the mixin. It was dead code and has been removed. The @api.depends for available_attribute_value_domain has also been simplified for the same reason as above, and the missing else branch (assigning [] when product_tmpl_id is False) has been added.

is_matching_product logic fixed

The original implementation had a structural flaw: the first check if self.product_tmpl_id != product.product_tmpl_id always returned False when product_tmpl_id was not set (since False != anything is always True), making it impossible to match on product_id or attribute_value_ids alone without a template.

The method has been rewritten so that each criterion is an independent filter applied only when set:

  • product_id is checked first as the most restrictive criterion — if set, it short-circuits all other checks.
  • product_tmpl_id is checked next — if set, the product must belong to that template.
  • attribute_value_ids is checked last — values within the same attribute are combined with OR, values across different attributes are combined with AND. Since an attribute value can exist on multiple templates, product_tmpl_id and attribute_value_ids should be used together to avoid false positives across templates.
  • If no criterion is set, the method returns True for any product (no constraint).

Documentation

The DESCRIPTION.md has been updated to document all fields and fully describe the matching logic, including the OR/AND semantics of attribute_value_ids. A new USAGE.md has been added explaining how to inherit the mixin and how to use the domain fields in a form view.

Tests

The single shallow test has been replaced by a comprehensive test suite covering all combinations of criteria. Following the pattern required by recent Odoo changes (see OCA/server-ux#1242), the FakeModelLoader is now loaded in setUp/tearDown instead of setUpClass/tearDownClass. The test class has been split into TestProductAttributeValueDependentMixinCommon (reusable by consuming modules) and TestProductAttributeValueDependentMixin (the actual test methods).

To get the full context of this [MIG] pull request, see also comments on previous unmerged PRs :

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

Labels

mod:product_attribute_value_dependent_mixin Module product_attribute_value_dependent_mixin series:18.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants