[IMP] mail_activity_team: Improve default team ID retrieval and update field defaults

[CRogos]

Solves: #114

@StefanRijnhart could you have a look in this change?

What do you think about replacing team_user_id by user_id (removing team_user_id)?
I don't understand why this is needed.

@jgebel @lorenzomorandini could you review/test?

[lorenzomorandini]

Hi @CRogos , tested it and it does seem to fix yes (I tested it with the hr_holiday workflow).

[lorenzomorandini]

This probably means one thing: server side code that create activities will not have a team_id set by default, even if a matching team exists for that model and user. Is that wanted?

[CRogos]

I see what you mean. The current behavior is, that the default Team of the user triggering the activity is used, not the default team of the user assigned to the activity.
Hard to say what is the best/intended behavior. There is nothing mentioned in the description. All the other modules which might create activities are not aware of team_id, so leaving it empty would be the easiest solution.

On the other hand, we could also try to find the most suitable team for the assigned user, based on model-type and user, and only leave it empty if there is no match, and there is no explicit team set on the activity type.

When a special Activity Type is used, a Default Team can be set, and therefore the team_id would be set. (this could also lead to contraint errors, if the intended user_id is not member of the team.)

Personally I also see no need for the constraint, that the user_id must be member of the used team_id. The activity might be in the hands of a team, but there might be reasons that one activity is assigned to someone outside the team, without removing the team. (e.g. vacation replacement or a special customer should not be contacted by the Sales Team, but by the CEO itself [who is not member of the Sales Team])
Nevertheless removing the constraint, will only fix the symptom, but not the problem, that the wrong team is assigend.

I tend to the behavior, try to assign the team based of the assigned user/model and allow setting a team where the assigned user is not part of the team (and removing team_user_id).

[lorenzomorandini]

I agree. I don't think it really makes sense to set the team based on the user creating the activity, it should only be based on the assigned user. I also agree on removing the constraint, it can eventually be re-added by a custom module.

[StefanRijnhart]

Thanks! Slightly worried about the loss of functionality here, even though I have my own workaround for this which very much amounts to the current change. Would it make sense to update the team if a user is set that is not a member of the current team? That might be possible if we rewrite team_id to be a stored, readwrite computed field.

A small number of tests might help to determine the impact of the change. Would you be able to work this out a little further?

[CRogos]

@StefanRijnhart @lorenzomorandini I'll give it a try and keep you posted.

[CRogos]

@StefanRijnhart @lorenzomorandini Regarding the team_user_id I need to take a deeper look also in the tests. For now you could test the bugfix.