Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[16.0][ADD] product_merge #1830

Open
wants to merge 2 commits into
base: 16.0
Choose a base branch
from
Open

Conversation

sbejaoui
Copy link
Contributor

@sbejaoui sbejaoui commented Jan 2, 2025

This module allows users to efficiently merge multiple product templates into one. This merge process ensures that attributes and variants from all selected products are consolidated into the primary product template, without creating any new variants. This approach is particularly important for maintaining data integrity and avoiding unnecessary database load.

By not creating new variants during the merge, the module helps to prevent heavy updates on existing tables, making it ideal for large-scale databases.

This module allows users to efficiently merge multiple product templates into one.
This merge process ensures that attributes and variants from all selected products
are consolidated into the primary product template, without creating any new variants.
This approach is particularly important for maintaining data integrity and
avoiding unnecessary database load.

By not creating new variants during the merge, the module helps to prevent
heavy updates on existing tables, making it ideal for large-scale databases.
This module extends the functionality of the product_merge module to ensure
that product sale-related information is appropriately updated during the
merging process.
@sbejaoui sbejaoui force-pushed the 16.0-product_merge-sbj branch from eed5099 to b3efaff Compare January 7, 2025 23:26
Comment on lines +12 to +25
pricelist_items = self.env["product.pricelist.item"].search(
[
("applied_on", "=", "1_product"),
("product_tmpl_id", "=", product_variant.product_tmpl_id.id),
]
)
pricelist_items.write(
{
"applied_on": "0_product_variant",
"product_id": product_variant.id,
"product_tmpl_id": self.product_tmpl_id.id,
}
)
return super()._move_variant_to_template(product_variant, attribute_values)
Copy link
Member

Choose a reason for hiding this comment

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

This could go in the main module, as that model is in the product module. Or is there anything else?

self._update_supplier_info(product_variant)
self._move_variant_to_template(product_variant, line.attribute_value_ids)

other_templates.write({"active": False})
Copy link
Contributor

Choose a reason for hiding this comment

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

@sbejaoui IMO something is missing. You should use a generic method to update into the database all the columns where the other_templates.ids are referenced.... The merge_records from the openupgrade lib could be a good candidate 🤔

@StefanRijnhart
Copy link
Member

@StefanRijnhart
Copy link
Member

StefanRijnhart commented Jan 19, 2025

This module emits error logs during the tests. Can you mute the logger when testing the exception?

2025-01-07T23:29:23.6985267Z 2025-01-07 23:29:23,697 500 �[1;32m�[1;49mINFO�[0m odoo odoo.addons.product_merge.tests.test_product_merge: Starting TestProductMerge.test_action_merge_products_same_attribute_value ... 
2025-01-07T23:29:23.7717242Z 2025-01-07 23:29:23,770 500 �[1;31m�[1;49mERROR�[0m odoo odoo.sql_db: bad query: UPDATE "product_product" SET "combination_indices" = '20', "write_date" = '2025-01-07T23:29:23.397594'::timestamp, "write_uid" = 1 WHERE id IN (78)
2025-01-07T23:29:23.7718580Z ERROR: duplicate key value violates unique constraint "product_product_combination_unique"
2025-01-07T23:29:23.7719177Z DETAIL:  Key (product_tmpl_id, combination_indices)=(63, 20) already exists.
2025-01-07T23:29:23.7719532Z  

Copy link
Member

@StefanRijnhart StefanRijnhart left a comment

Choose a reason for hiding this comment

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

See comments above.

@lmignon
Copy link
Contributor

lmignon commented Jan 22, 2025

Possible duplicate of https://github.com/OCA/stock-logistics-warehouse/tree/16.0/base_product_merge?

The goal is not the same. This PR propose to merge 2 product templates into one product with 2 variants.

@StefanRijnhart
Copy link
Member

StefanRijnhart commented Jan 22, 2025

From what I can see, base_product_merge will not merge product variants if there are product attributes at play, in which case the result is also one product with the combined set of variants. Can you be more precise about the difference in outcome?

@lmignon
Copy link
Contributor

lmignon commented Jan 22, 2025

From what I can see, base_product_merge will not merge product variants if there are product attributes at play, in which case the result is also one product with the combined set of variants. Can you be more precise about the difference in outcome?

I didn't know that the base_product_merge module existed so I didn't really test it.
A typical use case is, for example, when you have two TShirt products in DB, one for TShirt M and one for TShirt L, which are not variants of each other. The wizard will allow you to group the products together under the same template while completing the attribute values possible at template level. I don't get the impression from reading the code of the base_product_merge module that this is the use case supported.
Moreover we avoid modifying the variant identifiers. We're trying to minimize the impact on the DB as much as possible by keeping the current variant identifiers and only replacing the values of the template that has been orphaned by its variant. The merge must be possible on a running instance with a lot of sales and logistics activities.

@StefanRijnhart
Copy link
Member

Sounds like there is a strong case to try and improve base_product_merge for this scenario, also given that it already uses OpenUpgradelib's merge_records.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants