[18.0][IMP] mail_activity_team: Optional notify for team members#148
Open
[18.0][IMP] mail_activity_team: Optional notify for team members#148
Conversation
CRogos
force-pushed
the
18.0-mail_activity_team-notify
branch
from
bcc5796 to
13ac7b7
Compare
CRogos
changed the title
CRogos
force-pushed
the
18.0-mail_activity_team-notify
branch
from
13ac7b7 to
0dfdcc9
Compare
Contributor
Author
|
@StefanRijnhart @lorenzomorandini could you have a look? |
|
@CRogos it seemes to me that the assigned user received a double notification because of the You could exclude |
CRogos
force-pushed
the
18.0-mail_activity_team-notify
branch
from
0dfdcc9 to
babda9a
Compare
|
LGTM |
Contributor
Author
|
@lorenzomorandini you need to post the Review here: |
Member
StefanRijnhart
left a comment
StefanRijnhart
left a comment
There was a problem hiding this comment.
A couple of things that may not be handled correctly:
- Team members may not be notified if the activity is assigned to the current user, as action_notify is not called (https://github.com/odoo/odoo/blob/42ea101/addons/mail/models/mail_activity.py#L282)
- action_notify may not be called with sudo even if there are members with unreadable partners (cf. https://github.com/odoo/odoo/blob/42ea101/addons/mail/models/mail_activity.py#L283-L287)
- if the user of the activity is updated, then we don't want all the team members to receive a notification (or even the newly assigned user itself, if they are a team member) (see https://github.com/odoo/odoo/blob/42ea101/addons/mail/models/mail_activity.py#L325-L328). Now, if a new team is assigned, we may want to notify the members of the team, but this is currently not triggered.
Contributor
|
This PR has the |
Contributor
Author
I think the idea behind this is that, if you create an activity for yourself, you are aware of this activity and take action. I think there are arguments for both behaviors (notify/not notify) the team. Leave it as is? Make it configurable? What do you think?
I don't understand why sudo() is not called always? I'll check if it sufficient to call sudo() when accessing partner_id
I think I implement action_notify_team which gets called by action_notify, but can also be called in this case. Thanks for your input. I'm working on an update. |
Member
Ideally, notifications would be send for all users (team + activity user) excluding the creating user.
Agreed!
👍 You could consider circumventing the regular action_notify if a team with the send-team-notifications option is assigned, then duplicating the create and write logic from the original model to trigger calls to this new |
CRogos
force-pushed
the
18.0-mail_activity_team-notify
branch
5 times, most recently
from
3149783 to
7779f36
Compare
|
@CRogos is this ready or are there things not implemented yet? |
lorenzomorandini
suggested changes
Mar 12, 2026
|
|
||
| # when creating activities for other: send a notification to assigned user; | ||
| if not self.env.context.get("mail_activity_quick_update"): | ||
| activities.action_notify_team() |
There was a problem hiding this comment.
This should be removed since you are already calling action_notify_team inside action_notify, I am getting double notifications.
Contributor
Author
There was a problem hiding this comment.
As mentioned above action_notify is not called, when user_id is the current user. Therefore it needs to be added.
There was a problem hiding this comment.
If I as User A assign an activity to user B that is in a team with user C and D, 6 notifications are sent (2 for B, 2 for C and 2 for D).
Contributor
Author
There was a problem hiding this comment.
I am still struggling with the duplicate notifications. I've added a test for this case, which is currently failing. The problem is that the write method is called during the create process, which triggers the action_notify from the mail code... I have to postpone this to next week, but feel free to send an PR.
There was a problem hiding this comment.
I am currently using a custom module I made that contains the changes from both of your PRs. I'll leave you my current mail_activity.py that contains both but should fix the duplication mail problem (your test passes).
# Copyright 2026 MyNet
# License AGPL-3.0 or later (https://www.gnu.org/licenses/agpl).
from odoo import SUPERUSER_ID, api, fields, models
from odoo.exceptions import ValidationError
from odoo.tools.misc import get_lang
class MailActivity(models.Model):
_inherit = 'mail.activity'
@api.model
def _get_default_team_id(self, user_id=None, model_id=None):
if not user_id:
user_id = self.env.uid
if not model_id:
res_model = self.env.context.get('default_res_model')
model = (
self.sudo().env['ir.model'].search([('model', '=', res_model)], limit=1)
)
model_id = model.id if model else None
domain = [('member_ids', 'in', [user_id])]
if model_id:
domain.extend(
[
'|',
('res_model_ids', '=', False),
('res_model_ids', 'in', [model_id]),
]
)
results = self.env['mail.activity.team'].search(domain)
if model_id:
result_with_model = results.filtered(
lambda team: model_id in team.res_model_ids.ids
)
if result_with_model:
return result_with_model[0]
return results[0] if results else self.env['mail.activity.team']
user_id = fields.Many2one(string='User', required=False, default=False)
team_user_id = fields.Many2one(
string='Team user', related='user_id', readonly=False
)
team_id = fields.Many2one(
comodel_name='mail.activity.team', default=False, index=True
)
@api.model_create_multi
def create(self, vals_list):
for vals in vals_list:
# Ensure explicit team assignee is already reflected on user_id in the
# initial create payload. This avoids intermediate notification flows
# with empty assignee that may trigger duplicates.
if vals.get('team_user_id') and not vals.get('user_id'):
vals['user_id'] = vals['team_user_id']
if vals.get('team_id'):
if 'user_id' in vals and not vals.get('team_user_id'):
del vals['user_id']
elif (
'team_id' not in vals
and 'user_id' in vals
and 'team_user_id' not in vals
):
team_id = self._get_default_team_id(
vals['user_id'], vals.get('res_model_id')
)
if team_id:
vals['team_id'] = team_id.id
if vals.get('user_id'):
# Keep explicit assignee when team is inferred automatically.
vals['team_user_id'] = vals['user_id']
activities = super().create(vals_list)
if self.env.context.get('mail_activity_quick_update'):
return activities
# Core create() triggers action_notify() only for activities assigned to
# users different from the current one. Notify team members here only for
# activities not covered by that path to avoid duplicate notifications.
activities_without_core_notify = activities.filtered(
lambda activity: activity.user_id == self.env.user
)
activities_without_core_notify.action_notify_team()
return activities
def write(self, values):
team_notify_activities = self.env['mail.activity']
core_notified_activities = self.env['mail.activity']
if not self.env.context.get('mail_activity_quick_update', False):
if 'team_id' in values:
new_team_id = values.get('team_id') or False
team_notify_activities |= self.filtered(
lambda activity, new_team_id=new_team_id: activity.team_id.id
!= new_team_id
)
if 'user_id' in values:
new_user_id = values.get('user_id') or False
user_changed_activities = self.filtered(
lambda activity, new_user_id=new_user_id: activity.user_id.id
!= new_user_id
)
team_notify_activities |= user_changed_activities
# Core write() calls action_notify() for user changes except when
# assigning to the current user; avoid re-sending team notifications.
if values.get('user_id') != self.env.uid:
core_notified_activities = user_changed_activities
res = super().write(values)
if not self.env.context.get('mail_activity_quick_update', False):
(team_notify_activities - core_notified_activities).action_notify_team()
return res
@api.onchange('user_id')
def _onchange_user_id(self):
if not self.user_id or (
self.team_id and self.user_id in self.team_id.member_ids
):
return
self.team_id = self._get_default_team_id(
self.user_id.id, self.sudo().res_model_id.id
)
@api.constrains('team_id', 'user_id')
def _check_team_and_user(self):
for activity in self:
if (
activity.user_id.id != SUPERUSER_ID
and activity.team_id
and activity.user_id
and activity.user_id
not in activity.team_id.with_context(active_test=False).member_ids
):
raise ValidationError(
self.env._(
'The assigned user %(user_name)s is '
'not member of the team %(team_name)s.',
user_name=activity.user_id.name,
team_name=activity.team_id.name,
)
)
def action_notify_team(self):
classified = self._classify_by_model()
for model, activity_data in classified.items():
records_sudo = self.env[model].sudo().browse(activity_data['record_ids'])
activity_data['record_ids'] = records_sudo.exists().ids
for activity in self:
if activity.res_id not in classified[activity.res_model]['record_ids']:
continue
if not (activity.team_id and activity.team_id.notify_members):
continue
model_description = (
activity.env['ir.model']._get(activity.res_model).display_name
)
record = activity.env[activity.res_model].browse(activity.res_id)
members = activity.team_id.member_ids.filtered(
lambda member, assigned_user_id=activity.user_id: self.env.uid
not in member.user_ids.ids
and (
not assigned_user_id
or (assigned_user_id and assigned_user_id not in member.user_ids)
)
)
for member in members:
activity_ctx = (
activity.with_context(lang=member.lang) if member.lang else activity
)
body = activity_ctx.env['ir.qweb']._render(
'mail.message_activity_assigned',
{
'activity': activity_ctx,
'model_description': model_description,
'is_html_empty': lambda value: not value
or value == '<p><br></p>',
},
minimal_qcontext=True,
)
record.message_notify(
partner_ids=member.sudo().partner_id.ids,
body=body,
record_name=activity_ctx.res_name,
model_description=model_description,
email_layout_xmlid='mail.mail_notification_layout',
subject=self.env._(
'%(activity_name)s: %(summary)s (Team Activity)',
activity_name=activity_ctx.res_name,
summary=activity_ctx.summary
or activity_ctx.activity_type_id.name,
),
subtitles=[
self.env._('Activity: %s', activity_ctx.activity_type_id.name),
self.env._('Team: %s', activity_ctx.team_id.name),
self.env._(
'Deadline: %s',
(
activity_ctx.date_deadline.strftime(
get_lang(activity_ctx.env).date_format
)
if hasattr(activity_ctx.date_deadline, 'strftime')
else str(activity_ctx.date_deadline)
),
),
],
)
def action_notify(self):
result = super().action_notify()
self.action_notify_team()
return result| # notify new responsibles | ||
| if "team_id" in values: | ||
| if not self.env.context.get("mail_activity_quick_update", False): | ||
| new_team_activities.action_notify_team() |
There was a problem hiding this comment.
This should be removed since you are already calling action_notify_team inside action_notify, I am getting double notifications.
…rove team user assignment
CRogos
force-pushed
the
18.0-mail_activity_team-notify
branch
from
7779f36 to
1900666
Compare
Contributor
Author
I found a bug where some code from another PR is in the wizard file. I've removed this. |
…ate notifications for assigned users and teams
lorenzomorandini
suggested changes
Mar 13, 2026
|
|
||
| # when creating activities for other: send a notification to assigned user; | ||
| if not self.env.context.get("mail_activity_quick_update"): | ||
| activities.action_notify_team() |
There was a problem hiding this comment.
I am currently using a custom module I made that contains the changes from both of your PRs. I'll leave you my current mail_activity.py that contains both but should fix the duplication mail problem (your test passes).
# Copyright 2026 MyNet
# License AGPL-3.0 or later (https://www.gnu.org/licenses/agpl).
from odoo import SUPERUSER_ID, api, fields, models
from odoo.exceptions import ValidationError
from odoo.tools.misc import get_lang
class MailActivity(models.Model):
_inherit = 'mail.activity'
@api.model
def _get_default_team_id(self, user_id=None, model_id=None):
if not user_id:
user_id = self.env.uid
if not model_id:
res_model = self.env.context.get('default_res_model')
model = (
self.sudo().env['ir.model'].search([('model', '=', res_model)], limit=1)
)
model_id = model.id if model else None
domain = [('member_ids', 'in', [user_id])]
if model_id:
domain.extend(
[
'|',
('res_model_ids', '=', False),
('res_model_ids', 'in', [model_id]),
]
)
results = self.env['mail.activity.team'].search(domain)
if model_id:
result_with_model = results.filtered(
lambda team: model_id in team.res_model_ids.ids
)
if result_with_model:
return result_with_model[0]
return results[0] if results else self.env['mail.activity.team']
user_id = fields.Many2one(string='User', required=False, default=False)
team_user_id = fields.Many2one(
string='Team user', related='user_id', readonly=False
)
team_id = fields.Many2one(
comodel_name='mail.activity.team', default=False, index=True
)
@api.model_create_multi
def create(self, vals_list):
for vals in vals_list:
# Ensure explicit team assignee is already reflected on user_id in the
# initial create payload. This avoids intermediate notification flows
# with empty assignee that may trigger duplicates.
if vals.get('team_user_id') and not vals.get('user_id'):
vals['user_id'] = vals['team_user_id']
if vals.get('team_id'):
if 'user_id' in vals and not vals.get('team_user_id'):
del vals['user_id']
elif (
'team_id' not in vals
and 'user_id' in vals
and 'team_user_id' not in vals
):
team_id = self._get_default_team_id(
vals['user_id'], vals.get('res_model_id')
)
if team_id:
vals['team_id'] = team_id.id
if vals.get('user_id'):
# Keep explicit assignee when team is inferred automatically.
vals['team_user_id'] = vals['user_id']
activities = super().create(vals_list)
if self.env.context.get('mail_activity_quick_update'):
return activities
# Core create() triggers action_notify() only for activities assigned to
# users different from the current one. Notify team members here only for
# activities not covered by that path to avoid duplicate notifications.
activities_without_core_notify = activities.filtered(
lambda activity: activity.user_id == self.env.user
)
activities_without_core_notify.action_notify_team()
return activities
def write(self, values):
team_notify_activities = self.env['mail.activity']
core_notified_activities = self.env['mail.activity']
if not self.env.context.get('mail_activity_quick_update', False):
if 'team_id' in values:
new_team_id = values.get('team_id') or False
team_notify_activities |= self.filtered(
lambda activity, new_team_id=new_team_id: activity.team_id.id
!= new_team_id
)
if 'user_id' in values:
new_user_id = values.get('user_id') or False
user_changed_activities = self.filtered(
lambda activity, new_user_id=new_user_id: activity.user_id.id
!= new_user_id
)
team_notify_activities |= user_changed_activities
# Core write() calls action_notify() for user changes except when
# assigning to the current user; avoid re-sending team notifications.
if values.get('user_id') != self.env.uid:
core_notified_activities = user_changed_activities
res = super().write(values)
if not self.env.context.get('mail_activity_quick_update', False):
(team_notify_activities - core_notified_activities).action_notify_team()
return res
@api.onchange('user_id')
def _onchange_user_id(self):
if not self.user_id or (
self.team_id and self.user_id in self.team_id.member_ids
):
return
self.team_id = self._get_default_team_id(
self.user_id.id, self.sudo().res_model_id.id
)
@api.constrains('team_id', 'user_id')
def _check_team_and_user(self):
for activity in self:
if (
activity.user_id.id != SUPERUSER_ID
and activity.team_id
and activity.user_id
and activity.user_id
not in activity.team_id.with_context(active_test=False).member_ids
):
raise ValidationError(
self.env._(
'The assigned user %(user_name)s is '
'not member of the team %(team_name)s.',
user_name=activity.user_id.name,
team_name=activity.team_id.name,
)
)
def action_notify_team(self):
classified = self._classify_by_model()
for model, activity_data in classified.items():
records_sudo = self.env[model].sudo().browse(activity_data['record_ids'])
activity_data['record_ids'] = records_sudo.exists().ids
for activity in self:
if activity.res_id not in classified[activity.res_model]['record_ids']:
continue
if not (activity.team_id and activity.team_id.notify_members):
continue
model_description = (
activity.env['ir.model']._get(activity.res_model).display_name
)
record = activity.env[activity.res_model].browse(activity.res_id)
members = activity.team_id.member_ids.filtered(
lambda member, assigned_user_id=activity.user_id: self.env.uid
not in member.user_ids.ids
and (
not assigned_user_id
or (assigned_user_id and assigned_user_id not in member.user_ids)
)
)
for member in members:
activity_ctx = (
activity.with_context(lang=member.lang) if member.lang else activity
)
body = activity_ctx.env['ir.qweb']._render(
'mail.message_activity_assigned',
{
'activity': activity_ctx,
'model_description': model_description,
'is_html_empty': lambda value: not value
or value == '<p><br></p>',
},
minimal_qcontext=True,
)
record.message_notify(
partner_ids=member.sudo().partner_id.ids,
body=body,
record_name=activity_ctx.res_name,
model_description=model_description,
email_layout_xmlid='mail.mail_notification_layout',
subject=self.env._(
'%(activity_name)s: %(summary)s (Team Activity)',
activity_name=activity_ctx.res_name,
summary=activity_ctx.summary
or activity_ctx.activity_type_id.name,
),
subtitles=[
self.env._('Activity: %s', activity_ctx.activity_type_id.name),
self.env._('Team: %s', activity_ctx.team_id.name),
self.env._(
'Deadline: %s',
(
activity_ctx.date_deadline.strftime(
get_lang(activity_ctx.env).date_format
)
if hasattr(activity_ctx.date_deadline, 'strftime')
else str(activity_ctx.date_deadline)
),
),
],
)
def action_notify(self):
result = super().action_notify()
self.action_notify_team()
return result
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add the option to send notifications to the team members.
Bugfix: If a team is selected, no notification was send to the user_id because it is replaced by team_user_id.
(Why do we have team_user_id anyway? Shouldn't be user_id sufficent?)