[16.0][FIX] mrp_bom_version: fix compute_kit_quantities#1668
[16.0][FIX] mrp_bom_version: fix compute_kit_quantities#1668
Conversation
|
This PR has the |
marcos-mendez
left a comment
There was a problem hiding this comment.
Automated Review -- Tests Failed
1. Root Cause
The test failure occurs because the stock.move model's _compute_kit_quantities method is overridden in a way that assumes self.bom_line_id.bom_id exists and is comparable with kit_bom, but during the test execution, the bom_line_id may not be set or the BOM comparison fails due to missing or mismatched version logic.
2. Suggested Fix
In mrp_bom_version/models/stock_move.py, line 7, the condition if len(self.bom_line_id.bom_id) == 1 and self.bom_line_id.bom_id != kit_bom: can raise an error if self.bom_line_id is not set or bom_id is not accessible. Add a safety check before accessing bom_id:
if self.bom_line_id and self.bom_line_id.bom_id and len(self.bom_line_id.bom_id) == 1 and self.bom_line_id.bom_id != kit_bom:Also, ensure that kit_bom is not None before comparison, or handle cases where kit_bom is not initialized properly.
3. Additional Code Issues
- The method
_compute_kit_quantitiesinstock_move.pydoes not validate thatself.bom_line_idexists before accessing its attributes. - The logic assumes
self.bom_line_id.bom_id.versionexists and is comparable, which might fail if the BOM does not have a version field or is not properly initialized.
4. Test Improvements
Add a test case in test_mrp_bom_version.py that explicitly tests scenarios where:
bom_line_idis not set.bom_line_id.bom_idis not set or invalid.kit_bomis None.- Version comparisons between BOMs fail due to missing fields.
Use TransactionCase or SavepointCase with tagged tests to cover edge cases and ensure robustness. For example:
@tagged('post_install', 'standard')
def test_compute_kit_quantities_with_invalid_bom_line(self):
...This will ensure that the method handles edge cases gracefully without crashing.
⏰ PR Aging Alert
This PR by @Kev-Roche has been open for 111 days (3 months).
💤 Last activity was 33 days ago.
Every ignored PR is a contributor who might not come back. Review time matters. (OCA Aging Report)
Reciprocal Review Request
Hi everyone! I found some test failures on this PR and left detailed feedback above. I am happy to discuss or help debug. In the meantime, if any of you get a chance, I would appreciate a look at my open PR(s):
My open PRs across OCA:
- server-tools#3554 [MIG] datetime_formatter: Migration to 18.0
- server-tools#3548 [18.0][MIG] base_kanban_stage: Migration to 18.0
- hr-attendance#262 [16.0][ADD] Hr_attendance_idsecure: iDSecure (ControliD) attendance integration
- stock-logistics-workflow#2276 [16.0][ADD] stock_move_line_devaluation
- stock-logistics-workflow#2275 [16.0][ADD] Stock move line analytic account
- stock-logistics-workflow#2268 [16.0][ADD] stock_move_line_picking_partner
- purchase-workflow#2694 [16.0][IMP]Purchase workflow added to review state & exception fix
Reviewing each other's work helps the whole community move forward. Thank you!
Environment via OCA Neural Reviewer: Minikube + K8s Job + oca-ci/py3.10-odoo16.0 | Odoo 16.0
Automated review by OCA Neural Reviewer + qwen3-coder:30b
In case of a purchase or sale reconfirmation, the delivery quantity is wrong if the bom version changed on the product.
This PR fixes it :