[17.0][IMP] maintenance_location: Add inverse relation and equipment count to locations#546
Conversation
cgarcia-solvos
force-pushed
the
17.0-imp-maintenance_location-equipments
branch
from
29846e8 to
8b73ad8
Compare
cgarcia-solvos
changed the title
cgarcia-solvos
force-pushed
the
17.0-imp-maintenance_location-equipments
branch
2 times, most recently
from
de6dfa4 to
80ce1fb
Compare
|
@luisDIXMIT @christian-ramos-tecnativa could you review? Thanks!! |
|
@cgarcia-solvos one one suggestion, not blocking. Since equipments as 
|
| @api.depends("equipment_ids") | ||
| def _compute_equipment_count(self): | ||
| res = self.env["maintenance.equipment"].read_group( | ||
| domain=[("location_id", "in", self.ids)], |
There was a problem hiding this comment.
We should count equipments in sublocations too
| domain=[("location_id", "in", self.ids)], | |
| domain=[("location_id", "child_of", self.ids)], |
Also may be good idea adding a child_equipment_ids or similar as a readonly computable, or adding a sublocations field with the count in a tree view. Otherwise the user will see that there are 5 equipments in the location but just 3 equipments are displayed
cgarcia-solvos
force-pushed
the
17.0-imp-maintenance_location-equipments
branch
2 times, most recently
from
84d521c to
dc75d7e
Compare
luisDIXMIT
left a comment
luisDIXMIT
left a comment
There was a problem hiding this comment.
Tested on runboat and LGTM!
luisDIXMIT
left a comment
luisDIXMIT
left a comment
There was a problem hiding this comment.
Sorry, I skipped a controversial problem. With this development, an equipment can be assigned to several locations, and this is not correct.
|
Sorry, ignore the last comment. I realized that it can be chosen and is automatically moved to the new location. LGTM! |
cgarcia-solvos
force-pushed
the
17.0-imp-maintenance_location-equipments
branch
2 times, most recently
from
85b43dc to
b4d03ab
Compare
cgarcia-solvos
force-pushed
the
17.0-imp-maintenance_location-equipments
branch
3 times, most recently
from
598066b to
5e1f68d
Compare
|
@dalonsod I've updated the form view to follow your suggestion. Does it look better this way? 
|
|
This PR has the |
|
@pedrobaeza Is this ready to be merged? Thanks! |
pedrobaeza
left a comment
pedrobaeza
left a comment
There was a problem hiding this comment.
Please don't add extra empty lines in between.
cgarcia-solvos
force-pushed
the
17.0-imp-maintenance_location-equipments
branch
from
5e1f68d to
83e40a4
Compare
OCA-git-bot
added
mod:maintenance_location
|
@pedrobaeza Fixed, thanks! |
|
Hey, thanks for contributing! Proceeding to merge this for you. |
|
Congratulations, your PR was merged at a0d7e0a. Thanks a lot for contributing to OCA. ❤️ |
6 participants
Currently, the maintenance_location addon allows assigning a specific location to a maintenance.equipment. However, there is no quick way to check this relationship from the location's perspective.
This introduces the inverse relationship in the maintenance.location model.
maintenance.location model:
Added equipment_ids (One2many) to establish the inverse relation with maintenance.equipment.
Added equipment_count (Computed Integer) to count the number of equipments in a given location.Views:
Views:
Tree view: Added the equipment_count field so users can see the number of devices in each location.
Form view: Added the equipment_ids field (using the many2many_tags widget) to display the specific equipments assigned to the location.