[17.0][FIX] repair_type: Remove duplicated destination location for added parts #68
Conversation
|
Fix for module repair_type 17.0 cc https://github.com/APSL 163691 @miquelalzanillas @lbarry-apsl @mpascuall @peluko00 @javierobcn @BernatObrador please review |
ppyczko
force-pushed
the
17.0-fix-repair_type-remove-duplicate-field
branch
2 times, most recently
from
a390e69 to
0497317
Compare
ppyczko
changed the title
ppyczko
force-pushed
the
17.0-fix-repair_type-remove-duplicate-field
branch
2 times, most recently
from
ec4eeee to
6d7804b
Compare
|
Hi @LoisRForgeFlow, when you have the time, could you please take a look at this? I’d love to hear your input. Thank you in advance! |
repair_type/models/repair.py
Outdated
| class RepairOrder(models.Model): | ||
| _inherit = "repair.order" | ||
|
|
||
| product_location_dest_id = fields.Many2one( |
There was a problem hiding this comment.
I agree overall with the change 👍
However, being OCA we need to be as modular as possible. Since this fields doesn't do anything without a future repair_picking_after_done extension, I think we would need to move that to a glue module: something like repair_type_picking_after_done where the field is added and affects the picking after done behavior.
Does this sound reasonable?
There was a problem hiding this comment.
Thank you for your feedback @LoisRForgeFlow, your suggestion makes a lot of sense.
I’ve applied the suggested changes by removing the fields related to the repaired product's location out of repair_type and will move them into a separate module as suggested. Let me know if there’s anything else to adjust!
ppyczko
force-pushed
the
17.0-fix-repair_type-remove-duplicate-field
branch
from
6d7804b to
4cd121b
Compare
…arts and add product destination location
ppyczko
force-pushed
the
17.0-fix-repair_type-remove-duplicate-field
branch
from
4cd121b to
c719218
Compare
ppyczko
changed the title
LoisRForgeFlow
changed the title
Thank you again, @LoisRForgeFlow! Can you merge it, or should I ping someone? |
|
This PR has the |
|
All good, let's merge! /ocabot merge minor |
|
This PR looks fantastic, let's merge it! |
|
Congratulations, your PR was merged at 202f1e4. Thanks a lot for contributing to OCA. ❤️ |
The default 'Locations' for a Repair operation in Odoo core are as follows:
After some tests, I observed that the field 'Default Destination Location' is misleading, as it doesn't correspond to the destination location of the repaired product. Instead, it corresponds to the destination location of the added parts.
This can be seen in the following clip:
simplescreenrecorder-2024-11-20_12.38.10.mp4
As we can see, the 'Pedal Bin' product is used as an added part to the repair order. The stock move that is being created transfers the product to 'Virtual Locations/Production', which corresponds to the 'Default Destination Location' field on the related stock picking type. On the other hand, the 'Customizable Desk' product is being sent back to 'WH/Stock', which corresponds to the 'Default Source Location' field.
In contrast, the 'Locations' for a Repair operation that are defined in this repair_type module are the following:
Basically, the new field 'Default Add Destination Location' overrides the functionality of the 'Default Destination Location' field.
Therefore, the changes in this PR intend to remove this field. Additionally, the tests have been refactored to be more reusable.