Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[17.0] [IMP] fieldservice[_*]: populate person_id and warehouse_id from the related territory (if set) #1281

Open
wants to merge 5 commits into
base: 17.0
Choose a base branch
from

Conversation

ivantodorovich
Copy link

In order to achieve it in a clean and performant way, some refactors have been made to convert onchanges to computed (stored and writable) fields.

@OCA-git-bot
Copy link
Contributor

Hi @brian10048, @max3903, @wolfhall, @smangukiya,
some modules you are maintaining are being modified, check this out!

@ivantodorovich ivantodorovich force-pushed the 17.0-fieldservice-computed-ref branch from 2dda2db to 539076d Compare December 3, 2024 15:36
@ivantodorovich ivantodorovich force-pushed the 17.0-fieldservice-computed-ref branch from 539076d to bc6f807 Compare December 3, 2024 16:00
@ivantodorovich ivantodorovich force-pushed the 17.0-fieldservice-computed-ref branch from bc6f807 to 4467947 Compare December 3, 2024 16:16
@max3903 max3903 added this to the 17.0 milestone Dec 4, 2024
@max3903 max3903 self-assigned this Dec 4, 2024
Copy link
Contributor

@yankinmax yankinmax left a comment

Choose a reason for hiding this comment

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

Thanks for the improvements

@ivantodorovich
Copy link
Author

@OCA/field-service-maintainers ok to merge?

@max3903
Copy link
Member

max3903 commented Jan 21, 2025

@ivantodorovich Yes

)
equipment_ids = fields.Many2many("fsm.equipment")

@api.depends("location_id")
def _compute_person_id(self):
"""Compute the person from the location's territory"""
Copy link
Contributor

Choose a reason for hiding this comment

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

If I recall correctly, you may not use territory and assing person directly

Copy link
Author

@ivantodorovich ivantodorovich Jan 21, 2025

Choose a reason for hiding this comment

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

The territory comes from the location (related to location_id.territory_id)
So the goal here is to populate the order's assigned user from the related territory's partner (if set)

Not sure if it clarifies something for you, I'm not sure I understood what you mean

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes but leaving person_id blank is a feature.

The use of territory_id on location is optionnal.

I have business case where the vendor create the FSM Order and set a location (and leave person_id with no value), then (few days after) some manager assign a person to the fsm order.

Copy link
Author

Choose a reason for hiding this comment

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

That case still works, you just don't set any territory on the location, or no Assigned user on the territory.
Isn't it enough for your use case?

Copy link
Contributor

Choose a reason for hiding this comment

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

If you need a "default_person_id" on territory or fsm_location, why not just create one ?
It will be more explicit about what it does.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants