[15.0][ADD] product_variant_template_reassign: New module

[chienandalu]

cc @Tecnativa TT51495

ready for review @carlosdauden @pedrobaeza

[pedrobaeza]

With method="sql", the possible default_code in the unique variant is lost, as well as other data. One possibility is to specify the merge options.

[chienandalu]

Ok, done. Although with products with operations like done stock moves normally isn't possible

[pedrobaeza]

Well, I was not talking about changing the merge method, but adding the extra parameters to indicate the field merge options.

[chienandalu]

Understood. I added a hook method to configure it and added a test case. Do know of other possible issues of that kind?

[pedrobaeza]

The image and list price.

[chienandalu]

I should try for image. As for pricelist_id, its related to the template... what should we do there?

[pedrobaeza]

Don't do anything on the list price then.

[chienandalu]

I understood pricelist_id which is related indeed. list_price should be kept and it's done so now. A special post-process hook has been added to deal with field specs that don't work with merge_records

[pedrobaeza]

The image and list price.

[pedrobaeza]

You have forgotten this again.

[chienandalu]

sorry... 😵‍💫️

[pedrobaeza]

This again...

[chienandalu]

@carlosdauden can you review

[pedrobaeza]

Why not using read instead?

[lmignon]

I was thinking of doing the same kind of addon in V16 and I found your PR.

There is, however, one element in the approach that questions me. Why does attaching the variant to another template involve creating a new variant and then merging the existing variant into this new variant? Naively I thought I'd do this first step by simply changing the parent_id in DB and managing the attributes between the variant and the template in the same process. As for template management, my idea was also to update all the FKs to the original template with the value of the target template and thereafter remove-it.

I haven't yet really analyzed all the impacts and functional elements linked to the template and there are undoubtedly many challenges to be met. For example, supplier information should be linked to the variants when the reference is updated from the old template to the new .

[chienandalu]

Hi, @lmignon

Why does attaching the variant to another template involve creating a new variant and then merging the existing variant into this new variant? Naively I thought I'd do this first step by simply changing the parent_id in DB and managing the attributes between the variant and the template in the same process. As for template management, my idea was also to update all the FKs to the original template with the value of the target template and thereafter remove-it.

The main goal of the approach is using the power openupgradelib.merge_records to avoid dealing with the major part of all that stuff. It isn't exempt of pitfalls as we want to preserve the origin variant data and merge_records doesn't have the rules to deal with it sometimes... Maybe we should improve the tool to simplify this module...

I haven't yet really analyzed all the impacts and functional elements linked to the template and there are undoubtedly many challenges to be met. For example, supplier information should be linked to the variants when the reference is updated from the old template to the new.

Yes, in cases like that one we could be missing some of the bussiness logic and we should deal with it.

[lmignon]

Thank you for your explanation @chienandalu

My main concern with this approach is the impact on the database. With the creation and merge of the old variant to a new one, the process will imply the update of a too large number of records into too many tables. This will lead for sure to concurrent update errors and deadlock on systems daily managing thousands of stock pickings, sale orders etc, making this functionality unusable (you should also take into account all the stock.move.line, sale.order.line, stock.move and sock.move.line existing into your db where the variant is referenced). In my case, it's the biggest challenge. That's why I'm thinking to an approach with the less impact on the database. If we don't create a new variant, we drastically reduce the impact and will speed up the process.... The merge of the template is also possible but do we really need-it. All we need is to be able to use the logic of swapping the original template id with the new one into all the column where a FK exists.

[pedrobaeza]

There are a lot of submodels and some tricky fields like product_combination which should be properly set for the variant, so that's why we merge the old one into the existing variant, for not having to care too much on each version about internals, and also to avoid non trivial issues "swapping" the real variant. The downside is what you commented, but they are done in SQL, and at the end, this is for correcting an exceptional situation, so it should be done knowing what implies.

[lmignon]

Thank you @pedrobaeza. My use case is to move from an installation where the variant were not used at all to a full support of the variants. This process will involve a lot of reassignment of variants to others template. This process must be possible on a running instance as part of the day to day work. That's why I will go for the less intrusive approach. An sql update on the table stock_move, stock_move_line, account_move_line, .. is not possible without locking the system. For example, we've 9 986 501 records into the account_move_line table. I'm conscious that the change on the product_combination field will be tricky but if the functionality always ends with a concurrent update error or blocks the normal flow of operations in the company it will be unusable and i will stay with my challenge. Your approach seems pragmatic by reusing very useful/powerfull code of the openupgrade lib. Nevertheless I've the feeling that the indent of openupgradelib is not to be used on a running instance but as part of a migration/update/installation process. That doesn't mean that some part of the code can't be used into normal methods but we have to keep in mind that the need to work properly in an environment with many parallel tasks has not been taken into account in the development of this library. (Maybe I'm wrong...)