[19.0][MIG] project_task_stock: Migration to 19.0

[bizzappdev]

No description provided.

[victoralmau]

Please, cherry-pick #1638 to commit history before migration commit

[alexey-pelykh]

Clean 19.0 migration, all CI green. LGTM

[alexey-pelykh]

This PR is currently in Draft status, so this is an early review with observations that may help as you prepare the module for formal review.

Overview

Migration of project_task_stock to 19.0. The module links project tasks to stock/inventory moves, allowing material consumption directly from tasks.

Key Findings

Bug: stock_scrap.py -- Wrong class name and non-existent field references

Class name mismatch: The class is named StockMove but inherits stock.scrap. It should be StockScrap:

class StockMove(models.Model):  # Should be StockScrap
    _inherit = "stock.scrap"

Non-existent field references in _onchange_task_id:

• self.task_id.move_raw_ids -- the project.task model in this module defines move_ids, not move_raw_ids (this was likely carried over from mrp manufacturing terminology)
• self.task_id.location_src_id -- the field is named location_id on project.task, not location_src_id

This will cause AttributeError at runtime when the onchange fires.

Bug: project_task.py action_done() -- picking_id may be empty

Line:

moves_to_do.picking_id.with_context(skip_sanity_check=True).button_validate()

Stock moves created from tasks (via default_get in stock_move.py) are not associated with a picking. So moves_to_do.picking_id will be an empty recordset, and button_validate() will effectively be a no-op. The moves will never actually be validated/done through this code path. Consider calling moves_to_do._action_done() directly instead.

Missing security/ir.model.access.csv

The manifest does not include a security/ir.model.access.csv file. While this module only extends existing models (no new _name), it does add new fields and relationships. If any new model were introduced in the future, access rules would be needed. For OCA standards, it is good practice to include this file even if empty, and it is worth verifying that no access rules are needed for the stock.reference many2many relation table (stock_reference_task_rel).

Code style: _compute_unreserve_visible

any_quantity_done = any(
    [
        m.quantity > 0
        for m in item.move_ids.filtered(lambda x: x.state == "done")
    ]
)

any([...]) creates an unnecessary intermediate list. Use a generator expression instead: any(m.quantity > 0 for m in ...).

Code style: Old-style super() call in unlink()

def unlink(self):
    return super(ProjectTask, self.sudo()).unlink()

For Odoo 19.0, the new-style super() call is preferred. Also, using self.sudo() inside unlink() means the entire deletion (including cascade deletes on related records) runs with elevated privileges, which is a security concern. Consider scoping the sudo() more narrowly to just the analytic line cleanup, similar to how action_cancel handles it.

i18n: Version headers not updated

The .pot file header says Project-Id-Version: Odoo Server 18.0 -- should be 19.0 for this migration. Various .po files reference even older versions (14.0, 16.0). These should be regenerated with oca-gen-addon-readme / exported fresh from 19.0.

Missing migration scripts

This is labeled as [MIG] but there is no migrations/ directory. If any users had this module installed on 18.0, they would need migration scripts to handle potential data/schema changes between versions. If this is a straightforward port with no schema changes, a brief note in the PR description confirming this would be helpful.

No PR description

The PR body is empty. OCA migration PRs typically include a brief description of what was done for the migration (API changes addressed, tests updated, etc.).

_compute_scrap_move_count uses formatted_read_group

Good use of the 19.0 formatted_read_group API -- this is the correct modern approach.

_prepare_reference_vals decorated with @api.model but uses self

@api.model
def _prepare_reference_vals(self):
    return {"name": f"Task-ID: {self.id}"}

@api.model means self is the model class, not a recordset. But self.id is used, which requires a specific record. This will return False or None for self.id. The decorator should be removed, or the method should receive the task ID as a parameter.

Summary

The module has several issues that should be addressed before marking the PR as ready for review, most critically the broken field references in stock_scrap.py and the ineffective action_done() method. The codebase shows solid structure and good test coverage (15+ test methods), which is encouraging.

Happy to do a full review with inline comments once the PR is marked as ready.

---

Review posted via CorporateHub OCA review campaign