[17.0][MIG] repair_type: Migration to 17.0

[miquelalzanillas]

Standard migration with one exception:

• 'repair.line' model is missing in v17 and repair lines has been moved to 'stock.move'

cc https://github.com/APSL 155813 @peluko00 @mpascuall please review

• [Issue] Migration to version 17.0 #36

[peluko00]

Hi @BernatPForgeFlow,

Do you know how to test migration script in v17 in that case?

Thanks in advance

[peluko00]

Hello @BernatPForgeFlow @LoisRForgeFlow! In collaboration with @miquelalzanillas we carried out the migration including the data migration script.
Can you review when you can? Thank you very much in advance

[BernatPForgeFlow]

Hi @peluko00 @miquelalzanillas! Thanks for the improvements! I add some comments for things that I miss.

I am missing the 'Default Add Source Location' and 'Default Remove Destination Location' in the operation types. We use these locations to record which components have been used to repair products and are under warranty.

Also, can you create the new operation type fields with the same definition as the core ones? https://github.com/odoo/odoo/blob/17.0/addons/repair/models/stock_picking.py#L25 In that way, we pre-compute a value when creating it and we can edit it later. For the 'Default ... Source Location' fields I would maybe use this compute: https://github.com/odoo/odoo/blob/17.0/addons/stock/models/stock_picking.py#L252

Then, for the migration script, if you want to add fields like 'sequence_id' or 'sequence_prefix' that are not from this module, I would use a column_exists to make sure those columns exist: https://github.com/OCA/partner-contact/blob/14.0/partner_stage/migrations/14.0.2.0.0/pre-migrate.py#L8
If you want to migrate all repair_type_* modules in this single migration script, I would add a comment in the readme saying that the migration of that modules is added to this one.
As a final comment, in the migration script, since you will probably need to add many ifs to check if some columns exist and there won't probably be many records to migrate, I would perhaps create the new records via python.

[peluko00]

Hi @peluko00 @miquelalzanillas! Thanks for the improvements! I add some comments for things that I miss.

I am missing the 'Default Add Source Location' and 'Default Remove Destination Location' in the operation types. We use these locations to record which components have been used to repair products and are under warranty.

Also, can you create the new operation type fields with the same definition as the core ones? https://github.com/odoo/odoo/blob/17.0/addons/repair/models/stock_picking.py#L25 In that way, we pre-compute a value when creating it and we can edit it later. For the 'Default ... Source Location' fields I would maybe use this compute: https://github.com/odoo/odoo/blob/17.0/addons/stock/models/stock_picking.py#L252

Then, for the migration script, if you want to add fields like 'sequence_id' or 'sequence_prefix' that are not from this module, I would use a column_exists to make sure those columns exist: https://github.com/OCA/partner-contact/blob/14.0/partner_stage/migrations/14.0.2.0.0/pre-migrate.py#L8 If you want to migrate all repair_type_* modules in this single migration script, I would add a comment in the readme saying that the migration of that modules is added to this one. As a final comment, in the migration script, since you will probably need to add many ifs to check if some columns exist and there won't probably be many records to migrate, I would perhaps create the new records via python.

Hi @BernatPForgeFlow, thanks for the annotations.

The 'Default Remove Destination Location' field is defined in the core.

[BernatPForgeFlow]

I add the last comments. To set the default locations in the Operation Type I would use the following structure. To do that, we can inherit from core compute methods and add some logic afterwards. I add as individual comments how I would replace each of the methods.

Apart from that, in the '_get_repair_locations()' I would use the default_add_location_dest_id that we are defining in the operation type.

[BernatPForgeFlow]

@api.depends("code")
def _compute_default_location_src_id(self):
    res = super()._compute_default_location_src_id()
    for picking_type in self:
        stock_location = picking_type.warehouse_id.lot_stock_id
        if picking_type.code == "repair_operation":
            picking_type.default_add_location_src_id = stock_location.id
    return res

[BernatPForgeFlow]

@api.depends("code")
def _compute_default_location_dest_id(self):
    res = super()._compute_default_location_dest_id()
    for picking_type in self:
        if picking_type.code == "repair_operation":
            picking_type.default_add_location_dest_id = picking_type.default_location_dest_id.id
            picking_type.default_remove_location_src_id = picking_type.default_location_dest_id.id
            picking_type.default_recycle_location_src_id = picking_type.default_location_dest_id.id
    return res

[BernatPForgeFlow]

self.repair_id.picking_type_id.default_add_location_dest_id

[BernatPForgeFlow]

@peluko00 Here I was meaning to change the res[1] by self.repair_id.picking_type_id.default_add_location_dest_id.
First one should keep as self.repair_id.picking_type_id.default_add_location_src_id

[peluko00]

I add the last comments. To set the default locations in the Operation Type I would use the following structure. To do that, we can inherit from core compute methods and add some logic afterwards. I add as individual comments how I would replace each of the methods. Apart from that, in the '_get_repair_locations()' I would use the default_add_location_dest_id that we are defining in the operation type.

Thanks for your comments but i don't undestand exactly what is the logical purpose now for that changes.
I commit new changes for that, can you give me new feedback about it.
Thanks!

[BernatPForgeFlow]

Thanks for your comments but i don't undestand exactly what is the logical purpose now for that changes. I commit new changes for that, can you give me new feedback about it. Thanks!

Thank you very much! The purpose of these changes is to default to a logical stock move if someone wants to set up a new "Repair" operation type. This way, even if they want to change locations, they will have a schema to base on their changes. And then also that when the repair orders are made, that we actually take the locations that we have configured in the operation type.
https://github.com/user-attachments/assets/d786a2e5-cfa8-42dc-ad1d-2e3a9364cfe6

Then, with the last change that I requested some minutes ago I will approve this PR 👍 .

[peluko00]

Thanks for your comments but i don't undestand exactly what is the logical purpose now for that changes. I commit new changes for that, can you give me new feedback about it. Thanks!

Thank you very much! The purpose of these changes is to default to a logical stock move if someone wants to set up a new "Repair" operation type. This way, even if they want to change locations, they will have a schema to base on their changes. And then also that when the repair orders are made, that we actually take the locations that we have configured in the operation type. https://github.com/user-attachments/assets/d786a2e5-cfa8-42dc-ad1d-2e3a9364cfe6

Then, with the last change that I requested some minutes ago I will approve this PR 👍 .

Just done! Can you make the final review? Thanks in advance

[BernatPForgeFlow]

LGTM! Functional and code review! 👍

[LoisRForgeFlow]

Thanks

[peluko00]

Is ready for merge @LoisRForgeFlow @BernatPForgeFlow ?

[LoisRForgeFlow]

/ocabot merge nobump

[OCA-git-bot]

What a great day to merge this nice PR. Let's do it!
Prepared branch 17.0-ocabot-merge-pr-43-by-LoisRForgeFlow-bump-nobump, awaiting test results.

[OCA-git-bot]

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

[OCA-git-bot]

Congratulations, your PR was merged at 1d0a56a. Thanks a lot for contributing to OCA. ❤️