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

[17.0][MIG] mrp_bom_tracking: Migration to 17.0 #1226

Merged
merged 31 commits into from
Mar 26, 2024

Conversation

AndreuOForgeFlow
Copy link
Contributor

Standard migration

@ForgeFlow

MiquelRForgeFlow and others added 30 commits March 7, 2024 16:48
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: manufacture-15.0/manufacture-15.0-mrp_bom_tracking
Translate-URL: https://translation.odoo-community.org/projects/manufacture-15-0/manufacture-15-0-mrp_bom_tracking/
Currently translated at 100.0% (19 of 19 strings)

Translation: manufacture-15.0/manufacture-15.0-mrp_bom_tracking
Translate-URL: https://translation.odoo-community.org/projects/manufacture-15-0/manufacture-15-0-mrp_bom_tracking/it/
Currently translated at 100.0% (19 of 19 strings)

Translation: manufacture-15.0/manufacture-15.0-mrp_bom_tracking
Translate-URL: https://translation.odoo-community.org/projects/manufacture-15-0/manufacture-15-0-mrp_bom_tracking/it/
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: manufacture-16.0/manufacture-16.0-mrp_bom_tracking
Translate-URL: https://translation.odoo-community.org/projects/manufacture-16-0/manufacture-16-0-mrp_bom_tracking/
Currently translated at 100.0% (19 of 19 strings)

Translation: manufacture-16.0/manufacture-16.0-mrp_bom_tracking
Translate-URL: https://translation.odoo-community.org/projects/manufacture-16-0/manufacture-16-0-mrp_bom_tracking/it/
Currently translated at 100.0% (19 of 19 strings)

Translation: manufacture-16.0/manufacture-16.0-mrp_bom_tracking
Translate-URL: https://translation.odoo-community.org/projects/manufacture-16-0/manufacture-16-0-mrp_bom_tracking/es/
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: manufacture-16.0/manufacture-16.0-mrp_bom_tracking
Translate-URL: https://translation.odoo-community.org/projects/manufacture-16-0/manufacture-16-0-mrp_bom_tracking/
Copy link

@GuillemCForgeFlow GuillemCForgeFlow left a comment

Choose a reason for hiding this comment

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

general improvements and feedback 🙂

Comment on lines 58 to 74
if "product_id" in values:
for bom in self.mapped("bom_id"):
# Bind bom variable for the current iteration to the lambda
lines = self.filtered(lambda x, bom=bom: x.bom_id == bom)
product_id = values.get("product_id")
if product_id:
product_id = self.env["product.product"].browse(product_id)
product_id = product_id or lines.product_id
if lines:
message_body = template_env._render_template(
"mrp_bom_tracking.track_bom_template_2",
values={"lines": lines, "product_id": product_id},
)
bom.message_post(
body=message_body, subtype_id=self.env.ref("mail.mt_note").id
)

Choose a reason for hiding this comment

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

Suggested change
if "product_id" in values:
for bom in self.mapped("bom_id"):
# Bind bom variable for the current iteration to the lambda
lines = self.filtered(lambda x, bom=bom: x.bom_id == bom)
product_id = values.get("product_id")
if product_id:
product_id = self.env["product.product"].browse(product_id)
product_id = product_id or lines.product_id
if lines:
message_body = template_env._render_template(
"mrp_bom_tracking.track_bom_template_2",
values={"lines": lines, "product_id": product_id},
)
bom.message_post(
body=message_body, subtype_id=self.env.ref("mail.mt_note").id
)
if "product_id" in values:
for bom_line in self:
bom = bom_line.bom_id
product_id = values.get("product_id")
product = self.env["product.product"].browse(product_id)
if product.exists():
message_body = template_env._render_template(
"mrp_bom_tracking.track_bom_template_2",
values={"lines": bom_line, "product_id": product},
)
bom.message_post(
body=message_body, subtype_id=self.env.ref("mail.mt_note").id
)

product_id check was already done at the beginning of the indentation, no need to do it again. Also, refactor to iterate over self, which contains the BoM lines, as before the change, it was already doing one message posting per line change. I believe this refactoring simplifies the understanding of the logic. What do you think?
I believe this is more of a general improvement, nothing related to the migration itself.

mrp_bom_tracking/models/mrp_bom.py Outdated Show resolved Hide resolved
mrp_bom_tracking/models/mrp_bom.py Show resolved Hide resolved
mrp_bom_tracking/views/bom_template.xml Show resolved Hide resolved
Copy link

@GuillemCForgeFlow GuillemCForgeFlow left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you for the improvements 👍🏽

@LoisRForgeFlow
Copy link
Contributor

/ocabot migration mrp_bom_tracking

@OCA-git-bot OCA-git-bot added this to the 17.0 milestone Mar 25, 2024
@OCA-git-bot OCA-git-bot mentioned this pull request Mar 25, 2024
22 tasks
Copy link
Contributor

@LoisRForgeFlow LoisRForgeFlow left a comment

Choose a reason for hiding this comment

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

Tested and working good, I want to suggest one more improvement: Can you add tracking to the fields in the Miscelaneous tab too?

image

Thanks!

@AndreuOForgeFlow AndreuOForgeFlow force-pushed the 17.0-mig-mrp_bom_tracking branch from 71dce9e to eeb307e Compare March 26, 2024 09:52
@AndreuOForgeFlow
Copy link
Contributor Author

Tested and working good, I want to suggest one more improvement: Can you add tracking to the fields in the Miscelaneous tab too?

image

Thanks!

Done @LoisRForgeFlow!

Copy link
Contributor

@LoisRForgeFlow LoisRForgeFlow left a comment

Choose a reason for hiding this comment

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

Thanks!

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

On my way to merge this fine PR!
Prepared branch 17.0-ocabot-merge-pr-1226-by-LoisRForgeFlow-bump-nobump, awaiting test results.

@OCA-git-bot
Copy link
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

@OCA-git-bot OCA-git-bot merged commit 7931593 into OCA:17.0 Mar 26, 2024
7 checks passed
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at 817ade7. Thanks a lot for contributing to OCA. ❤️

@AndreuOForgeFlow AndreuOForgeFlow deleted the 17.0-mig-mrp_bom_tracking branch March 26, 2024 10:36
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.