pledra / odoo-product-configurator

Odoo modules enabling dynamic product configuration
https://www.pledra.com
GNU Affero General Public License v3.0
106 stars 140 forks source link

[12.0] product_configurator_mrp generates incorrect BOM #167

Open matt454357 opened 4 years ago

matt454357 commented 4 years ago

Impacted versions: 12.0

Steps to reproduce:

  1. Create configurable product
    • config test
  2. Create attributes and values
    • attr1
      • value1.1
      • value1.2
    • attr2
      • value2.1
      • value2.2
  3. Create Product Attribute Set in manufacturing
    • Configuration: config test
    • Configurations: attr: value1.1, attr: value2.1
  4. Create BOM for config test (setting the Configuration Set to "config test" for each line)
    • value1.1
    • value1.2
    • value2.1
    • value2.2
  5. Configure a product variant of config test
    • attr1: value1.1
    • attr2: value2.1
  6. Create a manufacturing order for product variant "config test (value1.1, value2.1)"

Current behavior:

The manufacturing order Consumed Materials includes all BOM lines.

Expected behavior:

The manufacturing order Consumed Materials should only include value1.1 and value2.1.

matt454357 commented 4 years ago

I am confused about the intended functionality of the v11/v12 product_configurator_mrp module. There is no demo data, and the following test doesn't actually test anything.

https://github.com/pledra/odoo-product-configurator/blob/f729d2e57831228464ed4be7df066d70d29928b6/product_configurator_mrp/tests/test_mrp.py#L69-L75

self.mrpBomLine is an empty recordset, so _skip_bom_line() will always return false.

In particular, I don't see the purpose of the mrp.bom.line.configuration.set and mrp.bom.line.configuration models.

Are we meant to manually create a record on mrp.bom.line.configuration for each product variant we configure? That would be redundant.

Perhaps there is a bit of code missing that would automatically populate these models when a product variant is created? In that case, it seems better to use the core Odoo functionality. That is to say, assign attbribute_value_ids to each mrp.bom.line, and let the standard _skip_bom_line() method do its thing.

Was there something wrong with the way we generated BOMs in v10? I mean, aside from the need for PR #109.

PCatinean commented 4 years ago

This is the approach required by our main paying customer in version 11. In order to not create thousands of boms one use the master bom on the product template just like in standard Odoo.

The "Apply on variants" field was not flexible enough so we added configuration sets. Attribute values on the same line are linked with "AND" then new lines are linked with "OR".

The ideal solution would have been to use the same configuration restriction mechanism as on the product templates but we did not get around to do it. Nevertheless this can generate raw material requirements without individual bom generation cluttering up the database

matt454357 commented 4 years ago

Thank you @PCatinean for the response.

Are you saying that my Steps to Reproduce have followed an incorrect procedure? If so, can you give me the proper steps?

I can see that attribute values on the same line are linked with "AND" then new lines are linked with "OR". But, I believe that the _skip_bom_line() method is still broken. And, the test is clearly broken.

The code is simple. Currently, we either get all BOM lines, or none. I don't see any reasonable way that the current method could filter individual lines:

def _skip_bom_line(self, product):
    ...
    product_value_ids = set(product.attribute_value_ids.ids)
    for config in self.config_set_id.configuration_ids:
        if len(set(config.value_ids.ids) - product_value_ids) == 0:
            return False
    return True

The only way I see this working, is the following

That would be way more data than either the standard Odoo system, or the v10 product_configurator_mrp system. And, it would all have to be manually entered.

matt454357 commented 4 years ago

I would be glad to contribute some code, including tests and demo data, but I need to understand the use-case.

Could you explain more about the "ideal solution"?

The ideal solution would have been to use the same configuration restriction mechanism as on the product templates...

It sounds like you are saying that mrp.bom.line.configuration records are used as a substitute for the configuration restrictions on the product template. And, in the current implementation, the restrictions on the product template must be duplicated in the mrp.bom.line.configuration model.

Configuration restrictions on the product template are used to guide a user's choices when creating the variant. I don't understand how they would be used when exploding the BOM for a variant that has already been created.

Do you mean that you would have used the attribute value data from the product template, rather than maintaining data in mrp.bom.line.configuration?

matt454357 commented 4 years ago

Hi @PCatinean,

Here is a set of steps that works with the current design. However, it requires creating Product Attribute Sets for the full expansion of all possible configurations, which would be impossible in real-life use cases. My full expansion includes over 18 trillion configurations.

  1. Create configurable product and component products
    • config test
    • comp1.1
    • comp1.2
    • comp2.1
    • comp2.2
  2. Create attributes and values
    • attr1
      • value1.1
      • value1.2
    • attr2
      • value2.1
      • value2.2
  3. Create Product Attribute Sets for the full expansion of all possible configurations
Set Configurations
config test 1.1/2.1 (attr1: value1.1) (attr2: value2.1)
config test 1.2/2.1 (attr1: value1.2) (attr2: value2.1)
config test 1.1/2.2 (attr1: value1.1) (attr2: value2.2)
config test 1.2/2.2 (attr1: value1.2) (attr2: value2.2)
  1. Create BOM for config test, with components duplicated for each possible configuration
Component Configuration Set
comp1.1 config test 1.1/2.1
comp1.1 config test 1.1/2.2
comp1.2 config test 1.2/2.1
comp1.2 config test 1.2/2.2
comp2.1 config test 1.1/2.1
comp2.1 config test 1.2/2.1
comp2.2 config test 1.1/2.2
comp2.2 config test 1.2/2.2
  1. Configure product variant "config test (value1.1, value2.1)"
    • attr1: value1.1
    • attr2: value2.1
  2. Create a manufacturing order for product variant "config test (value1.1, value2.1)"

Now we get the expected behavior: The manufacturing order Consumed Materials only includes value1.1 and value2.1

I can't possibly generate Product Attribute Sets for the full expansion. Just one of my product templates has over 1.8 million possible configurations: image