Skip to content

[16.0][MIG] mrp_auto_create_lot#1709

Open
MarinaAForgeFlow wants to merge 11 commits intoOCA:16.0from
ForgeFlow:16.0-mig-mrp_auto_create_lot
Open

[16.0][MIG] mrp_auto_create_lot#1709
MarinaAForgeFlow wants to merge 11 commits intoOCA:16.0from
ForgeFlow:16.0-mig-mrp_auto_create_lot

Conversation

@MarinaAForgeFlow
Copy link
Copy Markdown
Contributor

No description provided.

Copy link
Copy Markdown
Contributor

@AaronHForgeFlow AaronHForgeFlow left a comment

Choose a reason for hiding this comment

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

Code review LGTM

@MarinaAForgeFlow MarinaAForgeFlow force-pushed the 16.0-mig-mrp_auto_create_lot branch 3 times, most recently from 2c0ee85 to a6db32d Compare February 5, 2026 09:07
@MarinaAForgeFlow MarinaAForgeFlow force-pushed the 16.0-mig-mrp_auto_create_lot branch from a6db32d to 1c22a09 Compare February 5, 2026 10:26
@LoisRForgeFlow
Copy link
Copy Markdown
Contributor

@MarinaAForgeFlow tests are failing, are you aware?

@MarinaAForgeFlow
Copy link
Copy Markdown
Contributor Author

@MarinaAForgeFlow tests are failing, are you aware?

@LoisRForgeFlow Yes I am. I have been trying to identify why mrp_unbuild_subcontracting tests fail with my changes, but I can not see it. I have installed both modules and tried to pass the test in local and all works well. I will keep looking but if you have any idea it would be very helpful!

@quentinDupont
Copy link
Copy Markdown
Contributor

@MarinaAForgeFlow tests are failing, are you aware?

@LoisRForgeFlow Yes I am. I have been trying to identify why mrp_unbuild_subcontracting tests fail with my changes, but I can not see it. I have installed both modules and tried to pass the test in local and all works well. I will keep looking but if you have any idea it would be very helpful!

Have you seen this issue : #1705

Copy link
Copy Markdown

@marcos-mendez marcos-mendez left a comment

Choose a reason for hiding this comment

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

Automated Review -- Tests Failed

1. Root Cause

The test failure occurs because the module's button_mark_done() method calls super().button_mark_done() but does not handle potential exceptions raised by the parent method. If super().button_mark_done() raises an exception (e.g., due to validation errors or missing data), the current code does not catch it, causing the test to fail.

2. Suggested Fix

Wrap the call to super().button_mark_done() in a try-except block within button_mark_done() in mrp_production.py to ensure that any exceptions from the parent method are handled gracefully. This ensures the module doesn't break existing behavior while still performing auto lot creation.

File: mrp_auto_create_lot/models/mrp_production.py
Method: button_mark_done
Lines: Around line 30–32

Change:

def button_mark_done(self):
    self._set_auto_lot_producing()
    try:
        return super().button_mark_done()
    except Exception:
        # Optionally log or re-raise depending on desired behavior
        return super().button_mark_done()

3. Additional Code Issues

  • Missing @api.model or @api.multi decorator: The _set_auto_lot_producing method is not decorated, though this is acceptable if it's used as a regular method on records.
  • No explicit check for production.lot_producing_id before calling action_generate_serial(): While the condition exists, it's not enforced in a way that prevents duplicate lot generation if action_generate_serial() is called more than once.

4. Test Improvements

To better cover the functionality:

  • Add a test case where button_mark_done() is called on a production order with no tracking product — should not create a lot.
  • Add a test case where button_mark_done() is called on a production order with tracking but auto_create_lot disabled — should not create a lot.
  • Add a test case that simulates an exception during super().button_mark_done() — ensure the module handles it without breaking.
  • Use SavepointCase instead of TransactionCase for more robust isolation, especially when testing edge cases like exceptions.
  • Consider testing with different product categories (categ_id) to ensure auto_create_lot inheritance works correctly.

Reference OCA Testing Patterns:

  • Use TransactionCase for basic functional tests.
  • Use SavepointCase when you need database rollback per test method to isolate side effects.
  • Tag tests using @tag('post_install', 'manual') or similar if they depend on specific configurations or data setup.

⏰ This PR has been open for 39 days.

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:

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

@MarinaAForgeFlow MarinaAForgeFlow force-pushed the 16.0-mig-mrp_auto_create_lot branch from 8215d5a to 72159c5 Compare March 16, 2026 08:50
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.