[15.0][FIX] purchase_order_secondary_unit: Purchase secondary uom by default non store in variant #2875
Conversation
sergio-teruel
force-pushed
the
15.0-FIX-purchase_order_secondary_unit-non-default-sec-uom-variant
branch
from
9609dfd to
e08e853
Compare
carlos-lopez-tecnativa
left a comment
carlos-lopez-tecnativa
left a comment
There was a problem hiding this comment.
Code review
| # Set default purchase secondary uom from template to variants | ||
| # Step 1: For templates with >1 variants, nullify purchase_secondary_uom_id | ||
| sql = """ | ||
| WITH multi AS ( | ||
| SELECT product_tmpl_id AS id | ||
| FROM product_product | ||
| GROUP BY product_tmpl_id | ||
| HAVING COUNT(*) > 1 | ||
| ) | ||
| UPDATE product_template pt | ||
| SET purchase_secondary_uom_id = NULL | ||
| FROM multi | ||
| WHERE pt.id = multi.id | ||
| AND pt.purchase_secondary_uom_id IS NOT NULL; | ||
| """ |
There was a problem hiding this comment.
In the compute, you consider that a template can have more than one variant, but if all variants have the same purchase_secondary_uom_id, it should be set on the template. Why didn’t you consider this case in the migration script?
| sql = """ | ||
| WITH single AS ( | ||
| SELECT product_tmpl_id AS id | ||
| FROM product_product | ||
| GROUP BY product_tmpl_id | ||
| HAVING COUNT(*) = 1 | ||
| ) | ||
| UPDATE product_product pp | ||
| SET purchase_secondary_uom_id = pt.purchase_secondary_uom_id | ||
| FROM product_template pt | ||
| JOIN single s ON s.id = pt.id | ||
| WHERE pp.product_tmpl_id = pt.id | ||
| AND pt.purchase_secondary_uom_id IS NOT NULL | ||
| -- avoid writing unchanged rows | ||
| AND COALESCE(pp.purchase_secondary_uom_id, 0) <> pt.purchase_secondary_uom_id; | ||
| """ |
There was a problem hiding this comment.
Once you consider the case mentioned above and later set null on templates with more than a single variant, I think the remaining cases don’t require a complex query, because the remaining records have a single variant. So, the UPDATE can be done directly without the WITH clause.
…t non store in variant
sergio-teruel
force-pushed
the
15.0-FIX-purchase_order_secondary_unit-non-default-sec-uom-variant
branch
from
e08e853 to
d63f98f
Compare
|
@carlos-lopez-tecnativa changes done!! Finally I have used orm |
carlos-lopez-tecnativa
left a comment
carlos-lopez-tecnativa
left a comment
There was a problem hiding this comment.
LGTM, thanks for the improvement. Please forward-port to the upper versions.
| help="In order to set a value, please first add at least one record" | ||
| " in 'Secondary Unit of Measure'", | ||
| domain="[('product_tmpl_id', '=', id), ('product_id', '=', False)]", | ||
| store=True, |
There was a problem hiding this comment.
Why being stored if it has inverse?
5 participants
@Tecnativa TT59046
ping @carlosdauden @carlos-lopez-tecnativa