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

[MIG] base_product_merge: Migration to 18.0 #2250

Draft
wants to merge 12 commits into
base: 18.0
Choose a base branch
from

Conversation

PieterPaulussen
Copy link

No description provided.

@rousseldenis
Copy link
Contributor

/ocabot migration base_product_merge

@OCA-git-bot OCA-git-bot added this to the 18.0 milestone Jan 30, 2025
@OCA-git-bot OCA-git-bot mentioned this pull request Jan 30, 2025
40 tasks
@@ -0,0 +1 @@

Copy link
Contributor

Choose a reason for hiding this comment

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

Remove if void

@@ -0,0 +1 @@
We can merge duplicates products into single product
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe This module allows to merge products that are considered as duplicates into a single product. is better

@@ -0,0 +1,3 @@
Select products or templates then click on Merge products in Action
menu. Select Destination product in which need to merge all other
Copy link
Contributor

Choose a reason for hiding this comment

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

It is better to use a list here to enhance user reading.

if ptype == "product.product":
products = self.env[active_model].browse(active_ids)
rec.update({"product_ids": [(6, 0, products.ids)]})
else:
Copy link
Contributor

Choose a reason for hiding this comment

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

It should test also product.template and fail in case of other model.

rec.update({"product_tmpl_ids": [(6, 0, product_templates.ids)]})
return rec

dst_product_id = fields.Many2one("product.product", string="Destination product")
Copy link
Contributor

Choose a reason for hiding this comment

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

Please put field definitions on top

def merge_products(self, model, products_to_merge, dst_product):
try:
if not products_to_merge:
raise UserError(_("You cannot merge product to it self."))
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
raise UserError(_("You cannot merge product to it self."))
raise UserError(_("You cannot merge product into itself."))

method=self.merge_method,
)
except Exception as e:
_logger.warning(e)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm wondering why not displaying the exact cause of error

@PieterPaulussen
Copy link
Author

@rousseldenis This PR should currently be kept as a draft. I migrated it to check its functionality, but there's some missing functionality in this module that should be discussed/mitigated or added (which you can find here.

@PieterPaulussen PieterPaulussen marked this pull request as draft January 30, 2025 10:51
Copy link
Contributor

@jbaudoux jbaudoux left a comment

Choose a reason for hiding this comment

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

Not a stock module

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

Successfully merging this pull request may close these issues.

8 participants