Skip to content

Add product revision management for BOMs and lot/serial numbers #188

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

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

Conversation

WangTKurata
Copy link
Collaborator

Introduce functions for managing product revisions in BOMs and lot/serial numbers, including filtering, copying, and assigning revisions. Enhance traceability by integrating revision information into stock moves and purchase orders.

@WangTKurata
Copy link
Collaborator Author

I will fix it so that the pre-commit will pass. It's been a while since I last worked on it, so I've forgotten a lot of things. Please wait a bit for the review. Sorry.

@WangTKurata WangTKurata force-pushed the wng-add-revision-functions branch 4 times, most recently from fec0042 to f1b3592 Compare May 8, 2025 02:32
@WangTKurata
Copy link
Collaborator Author

pre-commit is passed now. Please review this PR.

@WangTKurata
Copy link
Collaborator Author

There seems to be an issue with how product.product and product.template are being handled. It looks like changes may be necessary.

@WangTKurata
Copy link
Collaborator Author

Checked functionality in my development environment. Seems all functions are working as expected.

@WangTKurata
Copy link
Collaborator Author

If we are going to use this module, we need to modify the stock_picking_auto_create_lot side to implement a process that, if prod_revision exists, carries over the Revision information from the source that triggered the Lot creation.

@WangTKurata WangTKurata force-pushed the wng-add-revision-functions branch from 318f925 to be17243 Compare May 19, 2025 06:11
feat: add stock quant tree lot revision module to display lot revision numbers

feat: add tests for product_revision

Add comprehensive tests for product revision functionality across various modules

- Implemented unit tests for product lot revisions in `product_lot_revision` module.
- Added tests for automatic revision setting, variant-specific revisions, and revision assignment via wizard.
- Created tests for purchase order line revisions in `purchase_order_product_revision` module, ensuring correct revision assignment and filtering.
- Developed stock move revision tests in `stock_move_revision` module, validating revision fields and variant-specific revisions.
- Established stock picking product revision tests in `stock_picking_product_revision` module, focusing on move line revisions and lot revision copying.
- Introduced stock quant tree lot revision tests in `stock_quant_tree_lot_revision` module, verifying lot revision numbers and variant-specific revisions.
@WangTKurata WangTKurata force-pushed the wng-add-revision-functions branch from 8cd6016 to 9965289 Compare May 22, 2025 06:14
Copy link
Contributor

@AungKoKoLin1997 AungKoKoLin1997 left a comment

Choose a reason for hiding this comment

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

Partial Review.
Please let me know if I need to follow up on myself about the points.

Comment on lines +12 to +18
"data": [
"views/product_revision_views.xml",
"views/product_product_views.xml",
"views/product_kanban_views.xml",
"security/ir.model.access.csv",
"demo/demo.xml",
],
Copy link
Contributor

Choose a reason for hiding this comment

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

Please reorder by alphabetically.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we don't remove this file. If you want to show how the record was, using screenshot in readme is better.

Copy link
Contributor

Choose a reason for hiding this comment

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

If there is no technical requirement, please order by alphabetically.

Copy link
Contributor

Choose a reason for hiding this comment

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

Better to have copyright and license in each .py file.

product.product_tmpl_id.current_revision_id
)

@api.depends("revision_ids", "product_tmpl_id.revision_ids")
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
@api.depends("revision_ids", "product_tmpl_id.revision_ids")
@api.depends("revision_ids", "product_tmpl_id.revision_count ")

else:
product.revision_count = len(product.revision_ids)

def action_view_revisions(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

action method should come after all compute methods.

def action_view_revisions(self):
"""Open the revisions view for this product variant"""
self.ensure_one()

Copy link
Contributor

Choose a reason for hiding this comment

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

Please don't add the new line inside the methods to align with OCA rule.

Suggested change

"search_default_inactive": 1,
},
}
else:
Copy link
Contributor

Choose a reason for hiding this comment

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

To reduce the ladder, we can remove this else since there is a return in above if .

Comment on lines +86 to +110
if self.is_default_variant() and self.product_tmpl_id.revision_ids:
tree_view_id = self.env.ref(
"product_revision.view_product_revision_tree"
).id
form_view_id = self.env.ref(
"product_revision.view_product_revision_form"
).id

return {
"name": _("Revisions"),
"type": "ir.actions.act_window",
"view_mode": "tree,form",
"res_model": "product.revision",
"views": [(tree_view_id, "tree"), (form_view_id, "form")],
"domain": [("product_tmpl_id", "=", self.product_tmpl_id.id)],
"context": {
"default_product_tmpl_id": self.product_tmpl_id.id,
"search_default_active": 1,
"search_default_inactive": 1,
},
}
else:
# If no variant-specific revisions and not a default variant,
# delegate to the template
return self.product_tmpl_id.action_view_revisions()
Copy link
Contributor

Choose a reason for hiding this comment

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

To reduce ladder. Seems like these two conditions are doing the same behavior after checking action_view_revisions() from product.template. I think we can simply use self.product_tmpl_id.action_view_revisions() instead of using condition. Am I missing?

Suggested change
if self.is_default_variant() and self.product_tmpl_id.revision_ids:
tree_view_id = self.env.ref(
"product_revision.view_product_revision_tree"
).id
form_view_id = self.env.ref(
"product_revision.view_product_revision_form"
).id
return {
"name": _("Revisions"),
"type": "ir.actions.act_window",
"view_mode": "tree,form",
"res_model": "product.revision",
"views": [(tree_view_id, "tree"), (form_view_id, "form")],
"domain": [("product_tmpl_id", "=", self.product_tmpl_id.id)],
"context": {
"default_product_tmpl_id": self.product_tmpl_id.id,
"search_default_active": 1,
"search_default_inactive": 1,
},
}
else:
# If no variant-specific revisions and not a default variant,
# delegate to the template
return self.product_tmpl_id.action_view_revisions()
if not self.is_default_variant() or not self.product_tmpl_id.revision_ids:
return self.product_tmpl_id.action_view_revisions()
tree_view_id = self.env.ref(
"product_revision.view_product_revision_tree"
).id
form_view_id = self.env.ref(
"product_revision.view_product_revision_form"
).id
return {
"name": _("Revisions"),
"type": "ir.actions.act_window",
"view_mode": "tree,form",
"res_model": "product.revision",
"views": [(tree_view_id, "tree"), (form_view_id, "form")],
"domain": [("product_tmpl_id", "=", self.product_tmpl_id.id)],
"context": {
"default_product_tmpl_id": self.product_tmpl_id.id,
"search_default_active": 1,
"search_default_inactive": 1,
},
}

Co-authored-by: Aung Ko Ko Lin (Quartile) <[email protected]>
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.

2 participants