Conversation
JasminSForgeFlow
force-pushed
the
19.0-mig-sale_mrp_bom
branch
from
05ec233 to
0d750a0
Compare
AaronHForgeFlow
left a comment
AaronHForgeFlow
left a comment
There was a problem hiding this comment.
Functional + code 👍
|
This PR has the |
|
/ocabot migration sale_mrp_bom |
| _inherit = "stock.rule" | ||
|
|
||
| @api.model | ||
| def run(self, procurements, raise_user_error=True): |
There was a problem hiding this comment.
@JasminSForgeFlow @LoisRForgeFlow
Should not all this code be avoided finding a way to replace the search in mrp module (in run() and _bom_find()) to then let Odoo mrp module do the procurement run job ?
alexey-pelykh
left a comment
alexey-pelykh
left a comment
There was a problem hiding this comment.
Thanks for the migration, @AaronHForgeFlow / @LoisRForgeFlow.
The procurement.group -> stock.rule refactor for the kit explosion logic looks correct for 19.0. A couple of observations:
License headers in new files: stock_rule.py and test_sale_mrp_bom_multi_line.py carry LGPL-3.0 headers (from 360ERP), while the module manifest declares AGPL-3 and all other files use AGPL-3.0. LGPL is compatible so it's not blocking, but for OCA consistency it might be worth aligning to AGPL-3.0 headers on those two files.
Translation template (sale_mrp_bom.pot) references model_procurement_group: Since 19.0 now inherits stock.rule instead of procurement.group, this ir.model reference in the pot (and the nl/nl_NL po files) looks stale from 18.0. It will likely self-correct on next pot regeneration from a running instance, but worth being aware of.
Otherwise the ORM usage, domain, constraint, view inheritance, security group, and tests all look solid. CI is green across the board.
| @@ -0,0 +1,95 @@ | |||
| # Copyright 2025 360ERP (<https://www.360erp.com>) | |||
| # License LGPL-3.0 or later (https://www.gnu.org/licenses/lgpl.html). | |||
There was a problem hiding this comment.
Nit: this file uses LGPL-3.0 while the module is licensed AGPL-3 per __manifest__.py. Consider aligning to AGPL-3.0 for OCA consistency.
There was a problem hiding this comment.
Updated, thanks
| @@ -0,0 +1,214 @@ | |||
| # Copyright 2025 360ERP (https://www.360erp.com) | |||
| # License LGPL-3.0 or later (https://www.gnu.org/licenses/lgpl). | |||
There was a problem hiding this comment.
Same license header remark -- LGPL-3.0 vs module-level AGPL-3.
There was a problem hiding this comment.
Updated, thanks
This commit fixes a bug where the module would not accept a BoM on a Sale Order Line if the product in question had multiple variants and the BoM did not specify a variant. Now, regardless of whether the product has multiple variants or not, if the variant is specified on the BoM, it must match the product on the Sale Order Line (like before), but if it's not specified, matching on the product template is enough.
Currently translated at 100.0% (6 of 6 strings) Translation: sale-workflow-18.0/sale-workflow-18.0-sale_mrp_bom Translate-URL: https://translation.odoo-community.org/projects/sale-workflow-18-0/sale-workflow-18-0-sale_mrp_bom/nl_NL/
JasminSForgeFlow
force-pushed
the
19.0-mig-sale_mrp_bom
branch
from
0d750a0 to
9e28061
Compare
|
Seems tests failing because other module |
BhaveshHeliconia
left a comment
BhaveshHeliconia
left a comment
There was a problem hiding this comment.
Functional review LGTM!
19 participants
Standard Migration
@ForgeFlow