10.0 enhanced copy

[richard-willdooit]

Copy configurable function added to product.product when a new copy is needed with the same attributes - ensures that validation is reperformed, as well as ensuring new custom values are created and not just referencing the old ones.

Copy on SO line ensures that a configured product is copied.

[richard-willdooit]

@PCatinean - This should be self explanatory.

[codecov-io]

Codecov Report

Merging #53 into 10.0 will increase coverage by 0.14%.
The diff coverage is 70.58%.

@@           Coverage Diff            @@
##           10.0      #53      +/-   ##
========================================
+ Coverage    59%   59.14%   +0.14%     
========================================
  Files        14       14              
  Lines      1344     1356      +12     
========================================
+ Hits        793      802       +9     
- Misses      551      554       +3

Impacted Files	Coverage Δ
product_configurator/models/product.py	75.86% <100%> (+1.25%)	⬆️
product_configurator/models/sale.py	58.33% <37.5%> (-41.67%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bb4fdb6...2a962bd. Read the comment docs.

[PCatinean]

@richard-willdooit very nice contribution, also passing all tests and coverage also increased. Super!

My only question for this is regarding the copy function itself.

In theory there should never be two variants with the exact same set of attribute ids (and custom values).

This is to prevent duplicates and have only one result when doing the search for an existing configuration just before creation in both configuration interfaces.

Since this doesn't generate an error I think I made the mistake of not adding a constraint.

Does your scenario fit in duplicate products with the same configuration without a problem?

[richard-willdooit]

@PCatinean

In theory there should never be two variants with the exact same set of attribute ids (and custom values).

This is to prevent duplicates and have only one result when doing the search for an existing configuration just before creation in both configuration interfaces.

Yes, interesting you say that, because there is no such search in the backend wizard. It always creates a new variant, and I thought is was a design feature. Certainly, for our implementation, this is not undesired behaviour, as once the variant is created, we can do things with that particular instance.

The scary thing, though, is that if Order 1 and Order 2 are configured identically, and therefore point to the same product id as you said the code should do, there is a glaring issue. If you go back to the first order, and change the configuration, your code currently modifies the product id directly, rather than unlinking and pointing to a new one. This would change the definition of the product for Order 1 AND Order 2. This is why I thought the behaviour was deliberate. If you do want the code to only allow a varaint to exist once in the product.product file, then you would need to re-factor that code.

I had a quick look through the Purchase Order proposed pull request (#42 by @elemire ), and notice that he has introduced an option to not create a new variant if it already existed, ("reuse_variant" on a template by template basis) and maybe this is a good solution. (However, it still has the problem that modifying a configuration on a line if that functionality were introduced on PO, would redefine the product rather than unlinking and linking to the new one).