From de7c119e666d108564b3889d1c08e5bf2e4da082 Mon Sep 17 00:00:00 2001 From: Xavier-Do Date: Fri, 12 Apr 2019 13:21:18 +0000 Subject: [PATCH] [IMP] mail: reduce message_post query count and clean code Purpose of this commit is to improve performance of post and notify through various code improvements. Containing * making of _notify_customize_recipients a mail.thread method; * _message_auto_subscribe_followers optimized; * renaming and optimisation of _notify_partners; * message_post_attachements optization; * remove unused mail_post_autofollow_partner_ids context key; * minimize message create queries; * only check record related access right for thread messages not in pending moderation; * get attachements from values instead of db; * storing lang in ctx from the beginning; * don't track on write on attachments; Related to task 1943901 Linked to PR #32404 --- addons/im_livechat/models/mail_channel.py | 4 +- addons/mail/models/mail_channel.py | 22 +- addons/mail/models/mail_message.py | 127 ++++-- addons/mail/models/mail_thread.py | 381 ++++++++++-------- addons/mail/models/res_partner.py | 11 - addons/mail/wizard/mail_compose_message.py | 2 +- addons/test_mail/tests/test_discuss.py | 7 - .../test_mail/tests/test_message_compose.py | 1 + addons/test_mail/tests/test_performance.py | 49 ++- addons/website_blog/models/website_blog.py | 4 +- addons/website_forum/models/forum.py | 9 +- odoo/addons/base/models/ir_attachment.py | 23 +- 12 files changed, 363 insertions(+), 277 deletions(-) diff --git a/addons/im_livechat/models/mail_channel.py b/addons/im_livechat/models/mail_channel.py index cb3f57f689c..bb9a07a8158 100644 --- a/addons/im_livechat/models/mail_channel.py +++ b/addons/im_livechat/models/mail_channel.py @@ -48,7 +48,7 @@ class MailChannel(models.Model): record.is_chat = True @api.multi - def _channel_message_notifications(self, message): + def _channel_message_notifications(self, message, message_format=False): """ When a anonymous user create a mail.channel, the operator is not notify (to avoid massive polling when clicking on livechat button). So when the anonymous person is sending its FIRST message, the channel header should be added to the notification, since the user cannot be listining to the channel. @@ -56,7 +56,7 @@ class MailChannel(models.Model): livechat_channels = self.filtered(lambda x: x.channel_type == 'livechat') other_channels = self.filtered(lambda x: x.channel_type != 'livechat') notifications = super(MailChannel, livechat_channels)._channel_message_notifications(message.with_context(im_livechat_use_username=True)) + \ - super(MailChannel, other_channels)._channel_message_notifications(message) + super(MailChannel, other_channels)._channel_message_notifications(message, message_format) for channel in self: # add uuid for private livechat channels to allow anonymous to listen if channel.channel_type == 'livechat' and channel.public == 'private': diff --git a/addons/mail/models/mail_channel.py b/addons/mail/models/mail_channel.py index e2163810dab..70e2ba63ea4 100644 --- a/addons/mail/models/mail_channel.py +++ b/addons/mail/models/mail_channel.py @@ -506,20 +506,6 @@ class Channel(models.Model): notifications.append([(self._cr.dbname, 'res.partner', partner.id), channel_info]) return notifications - @api.multi - def _notify(self, message): - """ Broadcast the given message on the current channels. - Send the message on the Bus Channel (uuid for public mail.channel, and partner private bus channel (the tuple)). - A partner will receive only on message on its bus channel, even if this message belongs to multiple mail channel. Then 'channel_ids' field - of the received message indicates on wich mail channel the message should be displayed. - :param : mail.message to broadcast - """ - if not self: - return - message.ensure_one() - notifications = self._channel_message_notifications(message) - self.env['bus.bus'].sendmany(notifications) - def _notify_thread(self, message, msg_vals=False, model_description=False, mail_auto_delete=True): # When posting a message on a mail channel, manage moderation and postpone notify users if not msg_vals or msg_vals.get('moderation_status') != 'pending_moderation': @@ -533,18 +519,18 @@ class Channel(models.Model): message._notify_pending_by_chat() @api.multi - def _channel_message_notifications(self, message): + def _channel_message_notifications(self, message, message_format=False): """ Generate the bus notifications for the given message :param message : the mail.message to sent :returns list of bus notifications (tuple (bus_channe, message_content)) """ - message_values = message.message_format()[0] + message_format = message_format or message.message_format()[0] notifications = [] for channel in self: - notifications.append([(self._cr.dbname, 'mail.channel', channel.id), dict(message_values)]) + notifications.append([(self._cr.dbname, 'mail.channel', channel.id), dict(message_format)]) # add uuid to allow anonymous to listen if channel.public == 'public': - notifications.append([channel.uuid, dict(message_values)]) + notifications.append([channel.uuid, dict(message_format)]) return notifications @api.model diff --git a/addons/mail/models/mail_message.py b/addons/mail/models/mail_message.py index 95fc750066b..e1fc850d433 100644 --- a/addons/mail/models/mail_message.py +++ b/addons/mail/models/mail_message.py @@ -393,8 +393,8 @@ class Message(models.Model): customer_email_data.append((partner_tree[notification.res_partner_id.id][0], partner_tree[notification.res_partner_id.id][1], notification.email_status)) has_access_to_model = message.model and self.env[message.model].check_access_rights('read', raise_exception=False) - if message.attachment_ids: - main_attachment = has_access_to_model and message.res_id and self.env[message.model].search([('id', '=', message.res_id)]) and getattr(self.env[message.model].browse(message.res_id), 'message_main_attachment_id') + if message.attachment_ids and message.res_id and issubclass(self.pool[message.model], self.pool['mail.thread']) and has_access_to_model: + main_attachment = self.env[message.model].browse(message.res_id).message_main_attachment_id attachment_ids = [] for attachment in message.attachment_ids: if attachment.id in attachments_tree: @@ -860,6 +860,55 @@ class Message(models.Model): author_ids = [mid for mid, message in message_values.items() if not self.is_thread_message(message)] + # Moderator condition: allow to WRITE, UNLINK if moderator of a pending message + moderator_ids = [] + if operation in ['write', 'unlink']: + moderator_ids = [mid for mid, message in message_values.items() if message.get('moderator_id')] + messages_to_check = self.ids + messages_to_check = set(messages_to_check).difference(set(author_ids), set(moderator_ids)) + if not messages_to_check: + return + + # Recipients condition, for read and write (partner_ids) + # keep on top, usefull for systray notifications + notified_ids = [] + model_record_ids = _generate_model_record_ids(message_values, messages_to_check) + if operation in ['read', 'write']: + notified_ids = [mid for mid, message in message_values.items() if message.get('notified')] + + messages_to_check = set(messages_to_check).difference(set(notified_ids)) + if not messages_to_check: + return + + # CRUD: Access rights related to the document + document_related_ids = [] + document_related_candidate_ids = [mid for mid, message in message_values.items() + if (message.get('model') and message.get('res_id') and + message.get('message_type') != 'user_notification' and + (message.get('moderation_status') != 'pending_moderation' or operation not in ['write', 'unlink']))] + model_record_ids = _generate_model_record_ids(message_values, document_related_candidate_ids) + for model, doc_ids in model_record_ids.items(): + DocumentModel = self.env[model] + if hasattr(DocumentModel, 'get_mail_message_access'): + check_operation = DocumentModel.get_mail_message_access(doc_ids, operation) ## why not giving model here? + else: + check_operation = self.env['mail.thread'].get_mail_message_access(doc_ids, operation, model_name=model) + records = DocumentModel.browse(doc_ids) + records.check_access_rights(check_operation) + mids = records.browse(doc_ids)._filter_access_rules(check_operation) + document_related_ids += [ + mid for mid, message in message_values.items() + if (message.get('model') == model and + message.get('res_id') in mids.ids and + message.get('message_type') != 'user_notification' and + (message.get('moderation_status') != 'pending_moderation' or + operation not in ['write', 'unlink']))] + + messages_to_check = messages_to_check.difference(set(document_related_ids)) + + if not messages_to_check: + return + # Parent condition, for create (check for received notifications for the created message parent) notified_ids = [] if operation == 'create': @@ -880,17 +929,12 @@ class Message(models.Model): notified_ids += [mid for mid, message in message_values.items() if message.get('parent_id') in not_parent_ids] - # Moderator condition: allow to WRITE, UNLINK if moderator of a pending message - moderator_ids = [] - if operation in ['write', 'unlink']: - moderator_ids = [mid for mid, message in message_values.items() if message.get('moderator_id')] + messages_to_check = messages_to_check.difference(set(notified_ids)) + if not messages_to_check: + return - # Recipients condition, for read and write (partner_ids) and create (message_follower_ids) - other_ids = set(self.ids).difference(set(author_ids), set(notified_ids), set(moderator_ids)) - model_record_ids = _generate_model_record_ids(message_values, other_ids) - if operation in ['read', 'write']: - notified_ids = [mid for mid, message in message_values.items() if message.get('notified')] - elif operation == 'create': + # Recipients condition for create (message_follower_ids) + if operation == 'create': for doc_model, doc_ids in model_record_ids.items(): followers = self.env['mail.followers'].sudo().search([ ('res_model', '=', doc_model), @@ -904,28 +948,11 @@ class Message(models.Model): message.get('message_type') != 'user_notification' ] - # CRUD: Access rights related to the document - other_ids = other_ids.difference(set(notified_ids)) - model_record_ids = _generate_model_record_ids(message_values, other_ids) - document_related_ids = [] - for model, doc_ids in model_record_ids.items(): - DocumentModel = self.env[model] - mids = DocumentModel.browse(doc_ids).exists() - if hasattr(DocumentModel, 'check_mail_message_access'): - DocumentModel.check_mail_message_access(mids.ids, operation) # ?? mids ? - else: - self.env['mail.thread'].check_mail_message_access(mids.ids, operation, model_name=model) - document_related_ids += [ - mid for mid, message in message_values.items() - if (message.get('model') == model and - message.get('res_id') in mids.ids and - message.get('message_type') != 'user_notification' and - (message.get('moderation_status') != 'pending_moderation' or - operation not in ['write', 'unlink']))] + messages_to_check = messages_to_check.difference(set(notified_ids)) + if not messages_to_check: + return - # Calculate remaining ids: if not void, raise an error - other_ids = other_ids.difference(set(document_related_ids)) - if not (other_ids and self.browse(other_ids).exists()): + if not self.browse(messages_to_check).exists(): return raise AccessError( _('The requested operation cannot be completed due to security restrictions. Please contact your system administrator.\n\n(Document type: %s, Operation: %s)') % @@ -966,17 +993,19 @@ class Message(models.Model): return message_id @api.multi - def _invalidate_documents(self): + def _invalidate_documents(self, model=None, res_id=None): """ Invalidate the cache of the documents followed by ``self``. """ for record in self: - if record.is_thread_message() and 'message_ids' in self.env[record.model]: - self.env[record.model].invalidate_cache(fnames=[ + model = model or record.model + res_id = res_id or record.res_id + if 'message_ids' in self.env[model]: + self.env[model].invalidate_cache(fnames=[ 'message_ids', 'message_unread', 'message_unread_counter', 'message_needaction', 'message_needaction_counter', - ], ids=[record.res_id]) + ], ids=[res_id]) @api.model def create(self, values): @@ -994,8 +1023,7 @@ class Message(models.Model): values['record_name'] = self._get_record_name(values) if 'attachment_ids' not in values: - values.setdefault('attachment_ids', []) - + values['attachment_ids'] = [] # extract base64 images if 'body' in values: Attachments = self.env['ir.attachment'] @@ -1021,8 +1049,21 @@ class Message(models.Model): tracking_values_cmd = values.pop('tracking_value_ids', False) message = super(Message, self).create(values) + # check attachement access if values.get('attachment_ids'): - message.attachment_ids.check(mode='read') + attachment_ids = [] + if all(isinstance(command, int) or command[0] in (4, 6) for command in values.get('attachment_ids')): + attachment_ids = [] + for command in values.get('attachment_ids'): + if isinstance(command, int): + attachment_ids += [command] + elif command[0] == 6: + attachment_ids += command[2] + else: # command[0] == 4: + attachment_ids += [command[1]] + else: + attachment_ids = message.mapped('attachment_ids').ids # fallback on read if any unknow command + self.env['ir.attachment'].browse(attachment_ids).check(mode='read') if tracking_values_cmd: vals_lst = [dict(cmd[2], mail_message_id=message.id) for cmd in tracking_values_cmd if len(cmd) == 3 and cmd[0] == 0] @@ -1033,7 +1074,7 @@ class Message(models.Model): message.sudo().write({'tracking_value_ids': tracking_values_cmd}) if message.is_thread_message(values): - message._invalidate_documents() + message._invalidate_documents(values.get('model'), values.get('res_id')) return message @@ -1067,7 +1108,9 @@ class Message(models.Model): self.mapped('attachment_ids').filtered( lambda attach: attach.res_model == self._name and (attach.res_id in self.ids or attach.res_id == 0) ).unlink() - self._invalidate_documents() + for elem in self: + if elem.is_thread_message(): + elem._invalidate_documents() return super(Message, self).unlink() # -------------------------------------------------- diff --git a/addons/mail/models/mail_thread.py b/addons/mail/models/mail_thread.py index 7f29e0d13c4..17d6941377b 100644 --- a/addons/mail/models/mail_thread.py +++ b/addons/mail/models/mail_thread.py @@ -488,19 +488,14 @@ class MailThread(models.AbstractModel): return True @api.model - def check_mail_message_access(self, res_ids, operation, model_name=None): + def get_mail_message_access(self, res_ids, operation, model_name=None): """ mail.message check permission rules for related document. This method is meant to be inherited in order to implement addons-specific behavior. A common behavior would be to allow creating messages when having read access rule on the document, for portal document such as issues. """ - if model_name: - DocModel = self.env[model_name] - else: - DocModel = self - if hasattr(DocModel, '_mail_post_access'): - create_allow = DocModel._mail_post_access - else: - create_allow = 'write' + + DocModel = self.env[model_name] if model_name else self + create_allow = getattr(DocModel, '_mail_post_access', 'write') if operation in ['write', 'unlink']: check_operation = 'write' @@ -510,9 +505,7 @@ class MailThread(models.AbstractModel): check_operation = 'write' else: check_operation = operation - - DocModel.check_access_rights(check_operation) - DocModel.browse(res_ids).check_access_rule(check_operation) + return check_operation @api.multi def message_change_thread(self, new_thread): @@ -1582,97 +1575,137 @@ class MailThread(models.AbstractModel): # Post / Send message API # ------------------------------------------------------ - def _message_post_process_attachments(self, attachments, attachment_ids, message_data): + def _message_post_process_attachments(self, attachments, attachment_ids, message_values): """ Preprocess attachments for mail_thread.message_post() or mail_mail.create(). - :param list attachments: list of attachment tuples in the form ``(name,content)``, + :param list attachments: list of attachment tuples in the form ``(name,content)``, #todo xdo update that where content is NOT base64 encoded :param list attachment_ids: a list of attachment ids, not in tomany command form :param dict message_data: model: the model of the attachments parent record, res_id: the id of the attachments parent record """ - IrAttachment = self.env['ir.attachment'] + return_values = {} + body = message_values.get('body') + model = message_values['model'] + res_id = message_values['res_id'] + m2m_attachment_ids = [] - cid_mapping = {} - fname_mapping = {} if attachment_ids: - filtered_attachment_ids = self.env['ir.attachment'].sudo().search([ - ('res_model', '=', 'mail.compose.message'), - ('create_uid', '=', self._uid), - ('id', 'in', attachment_ids)]) + # taking advantage of cache looks better in this case, to check + filtered_attachment_ids = self.env['ir.attachment'].sudo().browse(attachment_ids).filtered( + lambda a: a.res_model == 'mail.compose.message' and a.create_uid.id == self._uid) if filtered_attachment_ids: - filtered_attachment_ids.write({'res_model': message_data['model'], 'res_id': message_data['res_id']}) + filtered_attachment_ids.write({'res_model': model, 'res_id': res_id}) m2m_attachment_ids += [(4, id) for id in attachment_ids] # Handle attachments parameter, that is a dictionary of attachments - for attachment in attachments: - cid = False - if len(attachment) == 2: - name, content = attachment - elif len(attachment) == 3: - name, content, info = attachment - cid = info and info.get('cid') - else: - continue - if isinstance(content, str): - content = content.encode('utf-8') - elif content is None: - continue - data_attach = { - 'name': name, - 'datas': base64.b64encode(content), - 'type': 'binary', - 'datas_fname': name, - 'description': name, - 'res_model': message_data['model'], - 'res_id': message_data['res_id'], - } - new_attachment = IrAttachment.create(data_attach) - m2m_attachment_ids.append((4, new_attachment.id)) - if cid: - cid_mapping[cid] = new_attachment - fname_mapping[name] = new_attachment - if cid_mapping and message_data.get('body'): - root = lxml.html.fromstring(tools.ustr(message_data['body'])) - postprocessed = False - for node in root.iter('img'): - if node.get('src', '').startswith('cid:'): - cid = node.get('src').split('cid:')[1] - attachment = cid_mapping.get(cid) - if not attachment: - attachment = fname_mapping.get(node.get('data-filename'), '') - if attachment: - attachment.generate_access_token() - node.set('src', '/web/image/%s?access_token=%s' % (attachment.id, attachment.access_token)) + if attachments: # generate + cids_in_body = set() + names_in_body = set() + cid_list = [] + name_list = [] + + if body: + root = lxml.html.fromstring(tools.ustr(body)) + # first list all attachments that will be needed in body + for node in root.iter('img'): + if node.get('src', '').startswith('cid:'): + cids_in_body.add(node.get('src').split('cid:')[1]) + elif node.get('data-filename'): + names_in_body.add(node.get('data-filename')) + attachement_values_list = [] + + # generate values + for attachment in attachments: + cid = False + if len(attachment) == 2: + name, content = attachment + elif len(attachment) == 3: + name, content, info = attachment + cid = info and info.get('cid') + else: + continue + if isinstance(content, str): + content = content.encode('utf-8') + elif content is None: + continue + attachement_values= { + 'name': name, + 'datas': base64.b64encode(content), + 'type': 'binary', + 'datas_fname': name, + 'description': name, + 'res_model': model, + 'res_id': res_id, + } + if body and (cid and cid in cids_in_body or name in names_in_body): + attachement_values['access_token'] = self.env['ir.attachment']._generate_access_token() + attachement_values_list.append(attachement_values) + # keep cid and name list synced with attachement_values_list length to match ids latter + cid_list.append(cid) + name_list.append(name) + new_attachments = self.env['ir.attachment'].create(attachement_values_list) + cid_mapping = {} + name_mapping = {} + for counter, new_attachment in enumerate(new_attachments): + cid = cid_list[counter] + if 'access_token' in attachement_values_list[counter]: + if cid: + cid_mapping[cid] = (new_attachment.id, attachement_values_list[counter]['access_token']) + name = name_list[counter] + name_mapping[name] = (new_attachment.id, attachement_values_list[counter]['access_token']) + m2m_attachment_ids.append((4, new_attachment.id)) + + # note: right know we are only taking attachments and ignoring attachment_ids. + if (cid_mapping or name_mapping) and body: + postprocessed = False + for node in root.iter('img'): + attachment_data = False + if node.get('src', '').startswith('cid:'): + cid = node.get('src').split('cid:')[1] + attachment_data = cid_mapping.get(cid) + if not attachment_data and node.get('data-filename'): + attachment_data = name_mapping.get(node.get('data-filename'), False) + if attachment_data: + node.set('src', '/web/image/%s?access_token=%s' % attachment_data) postprocessed = True - if postprocessed: - body = lxml.html.tostring(root, pretty_print=False, encoding='UTF-8') - message_data['body'] = body - - return m2m_attachment_ids + if postprocessed: + return_values['body'] = lxml.html.tostring(root, pretty_print=False, encoding='UTF-8') + return_values['attachment_ids'] = m2m_attachment_ids + return return_values @api.multi @api.returns('mail.message', lambda value: value.id) - def message_post(self, body='', subject=None, subtype_id=False, - message_type='notification', subtype=None, author_id=None, partner_ids=None, channel_ids=None, - parent_id=False, attachments=None, attachment_ids=None, model=False, - add_sign=True, model_description=False, - mail_auto_delete=True, **kwargs): + def message_post(self, + body='', subject=None, message_type='notification', + email_from=False, author_id=None, parent_id=False, + subtype_id=False, subtype=None, partner_ids=None, channel_ids=None, + attachments=None, attachment_ids=None, + add_sign=True, model_description=False, mail_auto_delete=True, record_name=False, + **kwargs): """ Post a new message in an existing thread, returning the new mail.message ID. - :param int thread_id: thread ID to post into, or list with one ID; - if False/0, mail.message model will also be set as False :param str body: body of the message, usually raw HTML that will be sanitized - :param str type: see mail_message.message_type field + :param str subject: subject of the message + :param str message_type: see mail_message.message_type field. Can be anything but + user_notification, reserved for message_notify :param int parent_id: handle reply to a previous message by adding the parent partners to the message in case of private discussion - :param tuple(str,str) attachments or list id: list of attachment tuples in the form - ``(name,content)``, where content is NOT base64 encoded + :param int subtype_id: subtype_id of the message, mainly use fore + followers mechanism + :param int subtype: xmlid that will be used to compute subtype_id + if subtype_id is not given. + :param list(int) partner_ids: partner_ids to notify + :param list(int) channel_ids: channel_ids to notify + :param list(tuple(str,str), tuple(str,str, dict) or int) attachments : list of attachment tuples in the form + ``(name,content)`` or ``(name,content, info)``, where content is NOT base64 encoded + :param list id attachment_ids: list of existing attachement to link to this message + -Should only be setted by chatter + -Attachement object attached to mail.compose.message(0) will be attached + to the related document. Extra keyword arguments will be used as default column values for the - new mail.message record. Special cases: - - attachment_ids: supposed not attached to any document; attach them - to the related document. Should only be set by Chatter. + new mail.message record. :return int: ID of newly created mail.message """ self.ensure_one() # should always be posted on a record, use message_notify if no record @@ -1683,16 +1716,21 @@ class MailThread(models.AbstractModel): if 'model' in kwargs or 'res_id' in kwargs: raise ValueError("message_post doesn't support model and res_id parameters anymore. Please call message_post on record") - # Find the message's author, because we need it for private discussion - if author_id is None: # keep False values - author_id = self.env['mail.message']._get_default_author().id # self.env.user.partner_id + self = self.with_lang() # add lang to context imediatly since it will be usefull in various flows latter. + + record_name = record_name or self.display_name partner_ids = set(partner_ids or []) channel_ids = set(channel_ids or []) - for pc_id in partner_ids | channel_ids: - if not isinstance(pc_id, int): - raise ValueError('message_post partner_ids and channel_ids must be integer list, not commands') + if any(not isinstance(pc_id, int) for pc_id in partner_ids | channel_ids): + raise ValueError('message_post partner_ids and channel_ids must be integer list, not commands') + + # Find the message's author, because we need it for private discussion + if author_id is None: # keep False values + author_id = self.env.user.partner_id.id + if not email_from: + email_from = self.env['res.partner'].browse(author_id).sudo().email_formatted if author_id else self.env['mail.message']._get_default_from() if not subtype_id: subtype = subtype or 'mt_note' @@ -1702,33 +1740,30 @@ class MailThread(models.AbstractModel): # automatically subscribe recipients if asked to if self._context.get('mail_post_autofollow') and partner_ids: - partner_to_subscribe = partner_ids - if self._context.get('mail_post_autofollow_partner_ids'): - partner_to_subscribe = [p for p in partner_ids if p in self._context.get('mail_post_autofollow_partner_ids')] - self.message_subscribe(list(partner_to_subscribe)) + self.message_subscribe(list(partner_ids)) - # _mail_flat_thread: automatically set free messages to the first posted message - MailMessage = self.env['mail.message'] + MailMessage_sudo = self.env['mail.message'].sudo() if self._mail_flat_thread and not parent_id: - messages = MailMessage.search(['&', ('res_id', '=', self.ids[0]), ('model', '=', self._name), ('message_type', '!=', 'user_notification')], order="id ASC", limit=1) - parent_id = messages.ids and messages.ids[0] or False - # we want to set a parent: force to set the parent_id to the oldest ancestor, to avoid having more than 1 level of thread - elif parent_id: - messages = MailMessage.sudo().search([('id', '=', parent_id), ('parent_id', '!=', False)], limit=1) + parent_message = MailMessage_sudo.search([('res_id', '=', self.id), ('model', '=', self._name), ('message_type', '!=', 'user_notification')], order="id ASC", limit=1) + # parent_message searched in sudo for performance, only used for id. + # Note that with sudo we will match message with internal subtypes. + parent_id = parent_message.id if parent_message else False + elif parent_id: + old_parent_id = parent_id + parent_message = MailMessage_sudo.search([('id', '=', parent_id), ('parent_id', '!=', False)], limit=1) # avoid loops when finding ancestors processed_list = [] - if messages: - message = messages[0] - while (message.parent_id and message.parent_id.id not in processed_list): - processed_list.append(message.parent_id.id) - message = message.parent_id - parent_id = message.id - - values = kwargs + if parent_message: + new_parent_id = parent_message.parent_id and parent_message.parent_id.id + while (new_parent_id and new_parent_id not in processed_list): + processed_list.append(new_parent_id) + parent_message = parent_message.parent_id + parent_id = parent_message.id + values = dict(kwargs) values.update({ 'author_id': author_id, 'model': self._name, - 'res_id': self._name and self.id or False, + 'res_id': self.id, 'body': body, 'subject': subject or False, 'message_type': message_type, @@ -1737,13 +1772,16 @@ class MailThread(models.AbstractModel): 'partner_ids': partner_ids, 'channel_ids': channel_ids, 'add_sign': add_sign, + 'record_name': record_name, + 'email_from': email_from, }) + attachments = attachments or [] + attachment_ids = attachment_ids or [] + attachement_values = self._message_post_process_attachments(attachments, attachment_ids, values) + values.update(attachement_values) # attachement_ids, [body] - # 3. Attachments - # - HACK TDE FIXME: Chatter: attachments linked to the document (not done JS-side), load the message + new_message= self._message_create(values) - values['attachment_ids'] = self._message_post_process_attachments(attachments or [], attachment_ids or [], values) - new_message = self._message_create(values) # Set main attachment field if necessary self._message_set_main_attachment_id(values['attachment_ids']) @@ -1761,7 +1799,7 @@ class MailThread(models.AbstractModel): prioritary_attachments = all_attachments.filtered(lambda x: x.mimetype.endswith('pdf')) \ or all_attachments.filtered(lambda x: x.mimetype.startswith('image')) \ or all_attachments - self.sudo().write({'message_main_attachment_id': prioritary_attachments[0].id}) + self.sudo().with_context(tracking_disable=True).write({'message_main_attachment_id': prioritary_attachments[0].id}) def _message_post_after_hook(self, message, msg_vals): """ Hook to add custom behavior after having posted the message. Both @@ -1833,7 +1871,6 @@ class MailThread(models.AbstractModel): displayed on a document. It pushes notifications on inbox or by email depending on the user configuration, like other notifications. """ - # should we handle channel_ids here? if self: self.ensure_one() @@ -1844,7 +1881,7 @@ class MailThread(models.AbstractModel): if not author.email: raise exceptions.UserError(_("Unable to notify message, please configure the sender's email address.")) - email_from = formataddr((author.name, author.email)) + email_from = author.email_formatted partner_ids = partner_ids or set() if parent_id: # looks like no test case are going throug this condition. Linked to private discussion. This may be removed soon. @@ -1928,7 +1965,6 @@ class MailThread(models.AbstractModel): create_values['channel_ids'] = [(4, cid) for cid in create_values.get('channel_ids', [])] return self.env['mail.message'].create(create_values) - # ------------------------------------------------------ # Notification API # ------------------------------------------------------ @@ -1970,20 +2006,28 @@ class MailThread(models.AbstractModel): # (could also be interresting for, we could add partners with r['notif'] = 'ocn_client' and r['needaction']=False) # then overide a notify_recipients (as it was before) to effectively send ocn notifications. # enveloppe will contain more needaction, those for the member of a email channel. - if message_values and self and hasattr(self, '_notify_customize_recipients'): - message_values.update(self._notify_customize_recipients(message, msg_vals, rdata)) + if message_values and self: + message_values.update(self._notify_customize_recipients(message, msg_vals)) if message_values: message.write(message_values) inbox_pids = [r['id'] for r in rdata['partners'] if r['notif'] == 'inbox'] partner_email_rdata = [r for r in rdata['partners'] if r['notif'] == 'email'] + channel_ids = [r['id'] for r in rdata['channels']] + + notifications = [] + if inbox_pids or channel_ids: + message_values = False + if inbox_pids: + message_values = message.message_format()[0] + for partner in self.env['res.partner'].browse(inbox_pids): + notifications.append([(self._cr.dbname, 'ir.needaction', partner), dict(message_values)]) + if channel_ids: + notifications += self.env['mail.channel'].sudo().browse(channel_ids)._channel_message_notifications(message, message_values) if partner_email_rdata: self._notify_record_by_email(message, partner_email_rdata, msg_vals=msg_vals, model_description=model_description, mail_auto_delete=mail_auto_delete) - if inbox_pids: - self.env['res.partner'].browse(inbox_pids)._notify_by_chat(message) - if rdata['channels']: - # send a notification on bus to update channel - self.env['mail.channel'].sudo().browse([r['id'] for r in rdata['channels']])._notify(message) + if notifications: + self.env['bus.bus'].sudo().sendmany(notifications) return True def _notify_record_by_email(self, message, partners_data, msg_vals=False, model_description=False, mail_auto_delete=True, send_after_commit=False): @@ -1998,7 +2042,7 @@ class MailThread(models.AbstractModel): :param mail_auto_delete: delete notification emails once sent; """ model = msg_vals.get('model') if msg_vals else message.model - model_name = model_description or (self.with_lang().env['ir.model']._get(model).display_name if model else False) + model_name = model_description or (self.with_lang().env['ir.model']._get(model).display_name if model else False) # one query for display name recipients_groups_data = self._notify_classify_recipients(partners_data, model_name) if not recipients_groups_data: @@ -2006,21 +2050,25 @@ class MailThread(models.AbstractModel): force_send = self.env.context.get('mail_notify_force_send', True) - base_template_ctx = self._notify_prepare_template_context(message, msg_vals, model_description=model_description) + template_values = self._notify_prepare_template_context(message, msg_vals, model_description=model_description) # 10 queries - template_xmlid = message.email_layout_xmlid if message.email_layout_xmlid else 'mail.message_notification_email' + email_layout_xmlid = msg_vals.get('email_layout_xmlid') if msg_vals else message.email_layout_xmlid + template_xmlid = email_layout_xmlid if email_layout_xmlid else 'mail.message_notification_email' try: - base_template = self.env.ref(template_xmlid, raise_if_not_found=True).with_context(lang=base_template_ctx['lang']) + base_template = self.env.ref(template_xmlid, raise_if_not_found=True).with_context(lang=template_values['lang']) # 1 query except ValueError: _logger.warning('QWeb template %s not found when sending notification emails. Sending without layouting.' % (template_xmlid)) base_template = False + + mail_subject = message.subject or (message.record_name and 'Re: %s' % message.record_name) # in cache, no queries # prepare notification mail values base_mail_values = { 'mail_message_id': message.id, - 'mail_server_id': message.mail_server_id.id, + 'mail_server_id': message.mail_server_id.id, # 2 query, check acces + read, may be useless, Falsy, when will it be used? 'auto_delete': mail_auto_delete, - 'references': message.parent_id.message_id if message.parent_id else False + 'references': message.parent_id.message_id if message.parent_id else False, + 'subject': mail_subject, } headers = self._notify_email_headers() if headers: @@ -2028,24 +2076,24 @@ class MailThread(models.AbstractModel): Mail = self.env['mail.mail'].sudo() emails = self.env['mail.mail'].sudo() - email_pids = set() # loop on groups (customer, portal, user, ... + model specific like group_sale_salesman) recipients_max = 50 for recipients_group_data in recipients_groups_data: # generate notification email content - template_ctx = {**base_template_ctx, **recipients_group_data} + recipients_ids = recipients_group_data.pop('recipients') + render_values = {**template_values, **recipients_group_data} # {company, is_discussion, lang, message, model_description, record, record_name, signature, subtype, tracking_values, website_url} # {actions, button_access, has_button_access, recipients} + if base_template: - mail_body = base_template.render(template_ctx, engine='ir.qweb', minimal_qcontext=True) + mail_body = base_template.render(render_values, engine='ir.qweb', minimal_qcontext=True) else: mail_body = message.body mail_body = self._replace_local_links(mail_body) - mail_subject = message.subject or (message.record_name and 'Re: %s' % message.record_name) # send email - for email_chunk in split_every(recipients_max, recipients_group_data['recipients']): - recipient_values = self._notify_email_recipient_values(email_chunk) + for recipients_ids_chunk in split_every(recipients_max, recipients_ids): + recipient_values = self._notify_email_recipient_values(recipients_ids_chunk) email_to = recipient_values['email_to'] recipient_ids = recipient_values['recipient_ids'] @@ -2062,7 +2110,11 @@ class MailThread(models.AbstractModel): if email and recipient_ids: notifications = self.env['mail.notification'].sudo().search([ ('mail_message_id', '=', email.mail_message_id.id), - ('res_partner_id', 'in', list(recipient_ids)) + ('res_partner_id', 'in', list(recipient_ids)) # not sure to check. + # TODO XDO what if recipient_ids are empty because of _notify_email_recipient_values + # should we use recipients_ids_chunk? + # should we unlink recipients_ids_chunk - recipient_ids ? + # should we avoid to create needation? by calling _notify_email_recipient_values at the same place _notify_customize_recipients does? (but no chubnk at this step) ]) notifications.write({ 'is_email': True, @@ -2071,7 +2123,6 @@ class MailThread(models.AbstractModel): 'email_status': 'ready', }) emails |= email - email_pids.update(recipient_ids) # NOTE: # 1. for more than 50 followers, use the queue system @@ -2080,13 +2131,12 @@ class MailThread(models.AbstractModel): # using the command-line. test_mode = getattr(threading.currentThread(), 'testing', False) if force_send and len(emails) < recipients_max and (not self.pool._init or test_mode): - email_ids = emails.ids - dbname = self.env.cr.dbname - _context = self._context - # unless asked specifically, send emails after the transaction to # avoid side effects due to emails being sent while the transaction fails if not test_mode and send_after_commit: + email_ids = emails.ids + dbname = self.env.cr.dbname + _context = self._context def send_notifications(): db_registry = registry(dbname) with api.Environment.manage(), db_registry.cursor() as cr: @@ -2103,13 +2153,22 @@ class MailThread(models.AbstractModel): # compute send user and its related signature signature = '' user = self.env.user - if message.author_id and message.author_id.user_ids: - user = message.author_id.user_ids[0] - if message.add_sign: + author = message.env['res.partner'].browse(msg_vals.get('author_id')) if msg_vals else message.author_id + model = msg_vals.get('model') if msg_vals else message.model + add_sign = msg_vals.get('add_sign') if msg_vals else message.add_sign + subtype_id = msg_vals.get('subtype_id') if msg_vals else message.subtype_id.id + message_id = message.id + record_name = msg_vals.get('record_name') if msg_vals else message.record_name + author_user = user if user.partner_id == author else author.user_ids[0] if author and author.user_ids else False + # trying to use user (self.env.user) instead of browing user_ids if he is the author will give a sudo user, + # improving access performances and cache usage. + if author_user: + user = author_user + if add_sign: signature = user.signature else: - if message.add_sign: - signature = "

--
%s

" % message.author_id.name + if add_sign: + signature = "

--
%s

" % author.name company = self.company_id.sudo() if self and 'company_id' in self else user.company_id if company.website: @@ -2125,11 +2184,11 @@ class MailThread(models.AbstractModel): if template and template.lang: lang = template._render_template(template.lang, self.env.context['default_model'], self.env.context['default_res_id']) - if not model_description and message.model: - model_description = self.env['ir.model'].with_context(lang=lang)._get(message.model).display_name + if not model_description and model: + model_description = self.env['ir.model'].with_context(lang=lang)._get(model).display_name tracking = [] - if msg_vals.get('tracking_value_ids') if msg_vals else bool(self): # could be tracking + if msg_vals.get('tracking_value_ids', True) if msg_vals else bool(self): # could be tracking for tracking_value in self.env['mail.tracking.value'].sudo().search([('mail_message_id', '=', message.id)]): groups = tracking_value.field_groups if not groups or self.user_has_groups(groups): @@ -2137,7 +2196,7 @@ class MailThread(models.AbstractModel): tracking_value.get_old_display_value()[0], tracking_value.get_new_display_value()[0])) - is_discussion = message.subtype_id.id == self.env['ir.model.data'].xmlid_to_res_id('mail.mt_comment') + is_discussion = subtype_id == self.env['ir.model.data'].xmlid_to_res_id('mail.mt_comment') return { 'message': message, @@ -2146,7 +2205,7 @@ class MailThread(models.AbstractModel): 'company': company, 'model_description': model_description, 'record': self, - 'record_name': message.record_name, + 'record_name': record_name, 'tracking_values': tracking, 'is_discussion': is_discussion, 'subtype': message.subtype_id, @@ -2452,6 +2511,9 @@ class MailThread(models.AbstractModel): 'email_to': False, 'recipient_ids': recipient_ids, } + @api.multi + def _notify_customize_recipients(self, message, msg_vals): + return {} # ------------------------------------------------------ # Followers API @@ -2558,19 +2620,16 @@ class MailThread(models.AbstractModel): :param default_subtype_ids: coming from ``_get_auto_subscription_subtypes`` """ fnames = [] - for name, field in self._fields.items(): - if name == 'user_id' and updated_values.get(name) and (getattr(field, 'track_visibility', False) or getattr(field, 'tracking', False)): - if field.comodel_name == 'res.users': - fnames.append(name) - - new_subscriptions = [] - user_ids = [updated_values[fname] for fname in fnames if updated_values[fname]] - if user_ids: - new_pids = self.env['res.partner'].sudo().search([('user_ids', 'in', user_ids), ('active', '=', True)]).ids - for new_pid in new_pids: - new_subscriptions.append((new_pid, default_subtype_ids, 'mail.message_user_assigned' if new_pid != self.env.user.partner_id.id else False)) - - return new_subscriptions + field = self._fields.get('user_id') + user_id = updated_values.get('user_id') + if field and user_id and field.comodel_name == 'res.users' and (getattr(field, 'track_visibility', False) or getattr(field, 'tracking', False)): + user = self.env['res.users'].sudo().browse(user_id) + try: # avoid to make an exists, lets be optimistic and try to read it. + if user.active: + return [(user.partner_id.id, default_subtype_ids, 'mail.message_user_assigned' if user != self.env.user else False)] + except: + pass + return [] @api.multi def _message_auto_subscribe_notify(self, partner_ids, template): diff --git a/addons/mail/models/res_partner.py b/addons/mail/models/res_partner.py index 4cd62ac35c3..e2abaf7d75b 100644 --- a/addons/mail/models/res_partner.py +++ b/addons/mail/models/res_partner.py @@ -37,17 +37,6 @@ class Partner(models.Model): 'email_cc': False} for r in self} - @api.multi - def _notify_by_chat(self, message): - """ Broadcast the message to all the partner since """ - if not self: - return - message_values = message.message_format()[0] - notifications = [] - for partner in self: - notifications.append([(self._cr.dbname, 'ir.needaction', partner.id), dict(message_values)]) - self.env['bus.bus'].sendmany(notifications) - @api.model def get_needaction_count(self): """ compute the number of needaction of the current user """ diff --git a/addons/mail/wizard/mail_compose_message.py b/addons/mail/wizard/mail_compose_message.py index 2db6a3d8324..1d54406b086 100644 --- a/addons/mail/wizard/mail_compose_message.py +++ b/addons/mail/wizard/mail_compose_message.py @@ -367,7 +367,7 @@ class MailComposer(models.TransientModel): mail_values.pop('attachments', []), attachment_ids, {'model': 'mail.message', 'res_id': 0} - ) + )['attachment_ids'] # Filter out the blacklisted records by setting the mail state to cancel -> Used for Mass Mailing stats if res_id in blacklisted_rec_ids: mail_values['state'] = 'cancel' diff --git a/addons/test_mail/tests/test_discuss.py b/addons/test_mail/tests/test_discuss.py index a14b10ffdd6..3cb885b715c 100644 --- a/addons/test_mail/tests/test_discuss.py +++ b/addons/test_mail/tests/test_discuss.py @@ -28,13 +28,6 @@ class TestChatterTweaks(BaseFunctionalTest, TestRecipients): self.assertEqual(self.test_record.message_follower_ids.mapped('partner_id'), original.mapped('partner_id') | self.partner_1 | self.partner_2) self.assertEqual(self.test_record.message_follower_ids.mapped('channel_id'), original.mapped('channel_id')) - def test_post_subscribe_recipients_partial(self): - original = self.test_record.message_follower_ids - self.test_record.sudo(self.user_employee).with_context({'mail_create_nosubscribe': True, 'mail_post_autofollow': True, 'mail_post_autofollow_partner_ids': [self.partner_2.id]}).message_post( - body='Test Body', message_type='comment', subtype='mt_comment', partner_ids=[self.partner_1.id, self.partner_2.id]) - self.assertEqual(self.test_record.message_follower_ids.mapped('partner_id'), original.mapped('partner_id') | self.partner_2) - self.assertEqual(self.test_record.message_follower_ids.mapped('channel_id'), original.mapped('channel_id')) - def test_chatter_mail_create_nolog(self): """ Test disable of automatic chatter message at create """ rec = self.env['mail.test.simple'].sudo(self.user_employee).with_context({'mail_create_nolog': True}).create({'name': 'Test'}) diff --git a/addons/test_mail/tests/test_message_compose.py b/addons/test_mail/tests/test_message_compose.py index 5b40c56e7cf..32a9a44e3ee 100644 --- a/addons/test_mail/tests/test_message_compose.py +++ b/addons/test_mail/tests/test_message_compose.py @@ -218,6 +218,7 @@ class TestMessagePost(BaseFunctionalTest, MockEmails, TestRecipients): self.assertEqual(new_notification.email_from, formataddr((self.env.user.name, self.env.user.email))) self.assertEqual(new_notification.needaction_partner_ids, self.partner_1 | self.user_employee.partner_id) self.assertNotIn(new_notification, self.test_record.message_ids) + # todo xdo add test message_notify on thread with followers and stuff class TestComposer(BaseFunctionalTest, MockEmails, TestRecipients): diff --git a/addons/test_mail/tests/test_performance.py b/addons/test_mail/tests/test_performance.py index 01bc729d321..cb2300a6df3 100644 --- a/addons/test_mail/tests/test_performance.py +++ b/addons/test_mail/tests/test_performance.py @@ -83,7 +83,7 @@ class TestMailPerformance(TransactionCase): 'partner_id': self.env.ref('base.res_partner_12').id, }) - with self.assertQueryCount(__system__=5, demo=5): # test_mail only: 5 - 5 + with self.assertQueryCount(__system__=4, demo=4): # test_mail only: 4 - 4 record.track = 'X' @users('__system__', 'demo') @@ -99,13 +99,13 @@ class TestMailPerformance(TransactionCase): @warmup def test_create_mail_with_tracking(self): """ Create records inheriting from 'mail.thread' (with field tracking). """ - with self.assertQueryCount(__system__=10, demo=10): # test_mail only: 10 - 10 + with self.assertQueryCount(__system__=9, demo=9): # test_mail only: 9 - 9 self.env['test_performance.mail'].create({'name': 'X'}) @users('__system__', 'emp') @warmup def test_create_mail_simple(self): - with self.assertQueryCount(__system__=8, emp=8): # test_mail only: 8 - 8 + with self.assertQueryCount(__system__=7, emp=7): # test_mail only: 7 - 7 self.env['mail.test.simple'].create({'name': 'Test'}) @users('__system__', 'emp') @@ -161,7 +161,7 @@ class TestAdvMailPerformance(TransactionCase): def test_adv_activity(self): model = self.env['mail.test.activity'] - with self.assertQueryCount(__system__=9, emp=8): # test_mail only: 9 - 8 + with self.assertQueryCount(__system__=8, emp=7): # test_mail only: 8 - 7 model.create({'name': 'Test'}) @users('__system__', 'emp') @@ -173,7 +173,7 @@ class TestAdvMailPerformance(TransactionCase): 'default_res_model': 'mail.test.activity', }) - with self.assertQueryCount(__system__=10, emp=17): # com runbot: 10 - 16 // test_mail only: 10 - 15 + with self.assertQueryCount(__system__=10, emp=13): # com runbot: 10 - 13 // test_mail only: 10 - 13 activity = MailActivity.create({ 'summary': 'Test Activity', 'res_id': record.id, @@ -183,7 +183,7 @@ class TestAdvMailPerformance(TransactionCase): #voip module read activity_type during create leading to one less query in enterprise on action_feedback category = activity.activity_type_id.category - with self.assertQueryCount(__system__=24, emp=41): # com runbot: 24 - 41 // test_mail only: 24 - 39 + with self.assertQueryCount(__system__=24, emp=31): # com runbot: 24 - 31 // test_mail only: 24 - 31 activity.action_feedback(feedback='Zizisse Done !') @users('__system__', 'emp') @@ -192,7 +192,7 @@ class TestAdvMailPerformance(TransactionCase): def test_adv_activity_mixin(self): record = self.env['mail.test.activity'].create({'name': 'Test'}) - with self.assertQueryCount(__system__=10, emp=17): # com runbot: 10 - 16 // test_mail only: 10 - 15 + with self.assertQueryCount(__system__=10, emp=14): # com runbot: 10 - 14 // test_mail only: 10 - 14 activity = record.action_start('Test Start') #read activity_type to normalize cache between enterprise and community #voip module read activity_type during create leading to one less query in enterprise on action_close @@ -200,7 +200,7 @@ class TestAdvMailPerformance(TransactionCase): record.write({'name': 'Dupe write'}) - with self.assertQueryCount(__system__=26, emp=42): # com runbot: 26 - 42 // test_mail only: 26 - 40 + with self.assertQueryCount(__system__=26, emp=31): # com runbot: 26 - 31 // test_mail only: 26 - 31 record.action_close('Dupe feedback') self.assertEqual(record.activity_ids, self.env['mail.activity']) @@ -211,7 +211,7 @@ class TestAdvMailPerformance(TransactionCase): def test_message_assignation_email(self): self.user_test.write({'notification_type': 'email'}) record = self.env['mail.test.track'].create({'name': 'Test'}) - with self.assertQueryCount(__system__=61, emp=76): # com runbot: 61 - 76 // test_mail only: 61 - 71 + with self.assertQueryCount(__system__=56, emp=59): # com runbot: 56 - 59 // test_mail only: 56 - 59 record.write({ 'user_id': self.user_test.id, }) @@ -220,7 +220,7 @@ class TestAdvMailPerformance(TransactionCase): @warmup def test_message_assignation_inbox(self): record = self.env['mail.test.track'].create({'name': 'Test'}) - with self.assertQueryCount(__system__=37, emp=44): # test_mail only: 37 - 44 + with self.assertQueryCount(__system__=34, emp=40): # test_mail only: 34 - 40 record.write({ 'user_id': self.user_test.id, }) @@ -240,7 +240,7 @@ class TestAdvMailPerformance(TransactionCase): def test_message_log_with_post(self): record = self.env['mail.test.simple'].create({'name': 'Test'}) - with self.assertQueryCount(__system__=7, emp=13): # test_mail only: 7 - 13 + with self.assertQueryCount(__system__=6, emp=7): # test_mail only: 6 - 7 record.message_post( body='

Test message_post as log

', subtype='mail.mt_note', @@ -251,7 +251,7 @@ class TestAdvMailPerformance(TransactionCase): def test_message_post_no_notification(self): record = self.env['mail.test.simple'].create({'name': 'Test'}) - with self.assertQueryCount(__system__=7, emp=13): # test_mail only: 7 - 13 + with self.assertQueryCount(__system__=6, emp=7): # test_mail only: 6 - 7 record.message_post( body='

Test Post Performances basic

', partner_ids=[], @@ -264,7 +264,7 @@ class TestAdvMailPerformance(TransactionCase): def test_message_post_one_email_notification(self): record = self.env['mail.test.simple'].create({'name': 'Test'}) - with self.assertQueryCount(__system__=52, emp=72): # com runbot: 49 - 69 // test_mail only: 52 - 69 + with self.assertQueryCount(__system__=47, emp=51): # com runbot: 47 - 51 // test_mail only: 47 - 51 record.message_post( body='

Test Post Performances with an email ping

', partner_ids=self.customer.ids, @@ -276,7 +276,7 @@ class TestAdvMailPerformance(TransactionCase): def test_message_post_one_inbox_notification(self): record = self.env['mail.test.simple'].create({'name': 'Test'}) - with self.assertQueryCount(__system__=32, emp=44): # com runbot 31 - 43 // test_mail only: 32 - 44 + with self.assertQueryCount(__system__=30, emp=36): # com runbot 30 - 36 // test_mail only: 30 - 36 record.message_post( body='

Test Post Performances with an inbox ping

', partner_ids=self.user_test.partner_id.ids, @@ -386,8 +386,7 @@ class TestHeavyMailPerformance(TransactionCase): 'recipient_ids': [(4, pid) for pid in self.partners.ids], }) mail_ids = mail.ids - - with self.assertQueryCount(__system__=14, emp=21): # test_mail only: 14 - 21 + with self.assertQueryCount(__system__=16, emp=22): # test_mail only: 16 - 22 self.env['mail.mail'].browse(mail_ids).send() self.assertEqual(mail.body_html, '

Test

') @@ -400,7 +399,7 @@ class TestHeavyMailPerformance(TransactionCase): self.umbrella.message_subscribe(self.user_portal.partner_id.ids) record = self.umbrella.sudo(self.env.user) - with self.assertQueryCount(__system__=85, emp=108): # com runbot: 82 - 105 // test_mail only: 85 - 105 + with self.assertQueryCount(__system__=78, emp=82): # com runbot: 78 - 82 // test_mail only: 78 - 82 record.message_post( body='

Test Post Performances

', message_type='comment', @@ -417,7 +416,7 @@ class TestHeavyMailPerformance(TransactionCase): record = self.umbrella.sudo(self.env.user) template_id = self.env.ref('test_mail.mail_test_tpl').id - with self.assertQueryCount(__system__=102, emp=126): # com runbot: 99 - 123 // test_mail only: 102 - 123 + with self.assertQueryCount(__system__=95, emp=101): # com runbot: 95 - 101 // test_mail only: 95 - 101 record.message_post_with_template(template_id, message_type='comment', composition_mode='comment') self.assertEqual(record.message_ids[0].body, '

Adding stuff on %s

' % record.name) @@ -486,7 +485,7 @@ class TestHeavyMailPerformance(TransactionCase): 'user_id': self.env.uid, }) self.assertEqual(rec.message_partner_ids, self.partners | self.env.user.partner_id) - with self.assertQueryCount(__system__=61, emp=75): # com runbot: 60 - 75 // test_mail only: 60 - 72 + with self.assertQueryCount(__system__=55, emp=58): # com runbot: 55 - 58 // test_mail only: 55 - 58 rec.write({'user_id': self.user_portal.id}) self.assertEqual(rec.message_partner_ids, self.partners | self.env.user.partner_id | self.user_portal.partner_id) # write tracking message @@ -506,7 +505,7 @@ class TestHeavyMailPerformance(TransactionCase): customer_id = self.customer.id user_id = self.user_portal.id - with self.assertQueryCount(__system__=155, emp=172): # com runbot: 155 - 172 // test_mail only: 154 - 168 + with self.assertQueryCount(__system__=147, emp=155): # com runbot: 147 - 155 // test_mail only: 147 - 155 rec = self.env['mail.test.full'].create({ 'name': 'Test', 'umbrella_id': umbrella_id, @@ -533,7 +532,7 @@ class TestHeavyMailPerformance(TransactionCase): }) self.assertEqual(rec.message_partner_ids, self.user_portal.partner_id | self.env.user.partner_id) self.assertEqual(len(rec.message_ids), 1) - with self.assertQueryCount(__system__=99, emp=122): # com runbot: 99 - 122 // test_mail only: 99 - 119 + with self.assertQueryCount(__system__=96, emp=101): # com runbot: 96 - 101 // test_mail only: 96 - 101 rec.write({ 'name': 'Test2', 'umbrella_id': self.umbrella.id, @@ -569,7 +568,7 @@ class TestHeavyMailPerformance(TransactionCase): }) self.assertEqual(rec.message_partner_ids, self.user_portal.partner_id | self.env.user.partner_id) - with self.assertQueryCount(__system__=105, emp=127): # test_mail only: 104 - 123 + with self.assertQueryCount(__system__=101, emp=106): # test_mail only: 101 - 106 rec.write({ 'name': 'Test2', 'umbrella_id': umbrella_id, @@ -601,7 +600,7 @@ class TestHeavyMailPerformance(TransactionCase): }) self.assertEqual(rec.message_partner_ids, self.partners | self.env.user.partner_id | self.user_portal.partner_id) - with self.assertQueryCount(__system__=52, emp=66): # test_mail only: 51 - 66 + with self.assertQueryCount(__system__=50, emp=62): # test_mail only: 50 - 62 rec.write({ 'name': 'Test2', 'customer_id': customer_id, @@ -620,7 +619,7 @@ class TestHeavyMailPerformance(TransactionCase): self.assertEqual(len(rec.message_ids), 3) -@tagged('mail_performance_post') +@tagged('mail_performance') class TestMailPerformancePost(TransactionCase): def setUp(self): @@ -753,7 +752,7 @@ class TestMailPerformancePost(TransactionCase): ] self.attachements = self.env['ir.attachment'].sudo(self.env.user).create(self.vals) #-> 163-> 165 query attachement_ids = self.attachements.ids - with self.assertQueryCount(emp=248): # test_mail only: 210 + with self.assertQueryCount(emp=163): # com runbot 156 // test_mail only: 133 self.cr.sql_log = self.warm and self.cr.sql_log_count record.with_context({}).message_post( body='

Test body

', diff --git a/addons/website_blog/models/website_blog.py b/addons/website_blog/models/website_blog.py index 12676691977..6ddb0cb06a6 100644 --- a/addons/website_blog/models/website_blog.py +++ b/addons/website_blog/models/website_blog.py @@ -259,14 +259,14 @@ class BlogPost(models.Model): return groups @api.multi - def _notify_customize_recipients(self, message, msg_vals, recipients_vals): + def _notify_customize_recipients(self, message, msg_vals): """ Override to avoid keeping all notified recipients of a comment. We avoid tracking needaction on post comments. Only emails should be sufficient. """ msg_type = msg_vals.get('message_type') or message.message_type if msg_type == 'comment': return {'needaction_partner_ids': []} - return {} + return super(BlogPost, self)._notify_customize_recipients(message, msg_vals) def _default_website_meta(self): res = super(BlogPost, self)._default_website_meta() diff --git a/addons/website_forum/models/forum.py b/addons/website_forum/models/forum.py index b2673a24351..4ec595516a5 100644 --- a/addons/website_forum/models/forum.py +++ b/addons/website_forum/models/forum.py @@ -422,12 +422,13 @@ class Post(models.Model): return post @api.model - def check_mail_message_access(self, res_ids, operation, model_name=None): + def get_mail_message_access(self, res_ids, operation, model_name=None): + # XDO FIXME: to be correctly fixed with new get_mail_message_access and filter access rule if operation in ('write', 'unlink') and (not model_name or model_name == 'forum.post'): # Make sure only author or moderator can edit/delete messages if any(not post.can_edit for post in self.browse(res_ids)): raise KarmaError('Not enough karma to edit a post.') - return super(Post, self).check_mail_message_access(res_ids, operation, model_name=model_name) + return super(Post, self).get_mail_message_access(res_ids, operation, model_name=model_name) @api.multi def write(self, vals): @@ -805,14 +806,14 @@ class Post(models.Model): return super(Post, self).message_post(message_type=message_type, **kwargs) @api.multi - def _notify_customize_recipients(self, message, msg_vals, recipients_vals): + def _notify_customize_recipients(self, message, msg_vals): """ Override to avoid keeping all notified recipients of a comment. We avoid tracking needaction on post comments. Only emails should be sufficient. """ msg_type = msg_vals.get('message_type') or message.message_type if msg_type == 'comment': return {'needaction_partner_ids': [], 'partner_ids': []} - return {} + return super(Post, self)._notify_customize_recipients(message, msg_vals) class PostReason(models.Model): diff --git a/odoo/addons/base/models/ir_attachment.py b/odoo/addons/base/models/ir_attachment.py index a3176dccef8..ce98ee7c296 100644 --- a/odoo/addons/base/models/ir_attachment.py +++ b/odoo/addons/base/models/ir_attachment.py @@ -11,7 +11,7 @@ from collections import defaultdict import uuid from odoo import api, fields, models, tools, SUPERUSER_ID, _ -from odoo.exceptions import AccessError, ValidationError +from odoo.exceptions import AccessError, ValidationError, MissingError from odoo.tools import config, human_size, ustr, html_escape from odoo.tools.mimetypes import guess_mimetype @@ -313,6 +313,8 @@ class IrAttachment(models.Model): def _check_serving_attachments(self): # restrict writing on attachments that could be served by the # ir.http's dispatch exception handling + # XDO note: this should be done in check(write), constraints for access rights? + # XDO note: if read on sudo, read twice, one for constraints, one for _inverse_datas as user if self.env.user._is_admin(): return if self.type == 'binary' and self.url: @@ -326,13 +328,15 @@ class IrAttachment(models.Model): In the 'document' module, it is overriden to relax this hard rule, since more complex ones apply there. """ + if self.env.user._is_superuser(): + return True # collect the records to check (by model) model_ids = defaultdict(set) # {model_name: set(ids)} require_employee = False if self: self._cr.execute('SELECT res_model, res_id, create_uid, public, res_field FROM ir_attachment WHERE id IN %s', [tuple(self.ids)]) for res_model, res_id, create_uid, public, res_field in self._cr.fetchall(): - if self.env.user.id != SUPERUSER_ID and not self.env.user._is_system() and res_field: + if not self.env.user._is_system() and res_field: raise AccessError(_("Sorry, you are not allowed to access this document.")) if public and mode == 'read': continue @@ -488,12 +492,20 @@ class IrAttachment(models.Model): @api.model_create_multi def create(self, vals_list): + record_tuple_set = set() for values in vals_list: # remove computed field depending of datas for field in ('file_size', 'checksum'): values.pop(field, False) values = self._check_contents(values) - self.browse().check('write', values=values) + # 'check()' only uses res_model and res_id from values, and make an exists. + # We can group the values by model, res_id to make only one query when + # creating multiple attachments on a single record. + record_tuple = (values.get('res_model'), values.get('res_id')) + record_tuple_set.add(record_tuple) + for record_tuple in record_tuple_set: + (res_model, res_id) = record_tuple + self.check('write', values={'res_model':res_model, 'res_id':res_id}) return super(IrAttachment, self).create(vals_list) @api.multi @@ -504,10 +516,13 @@ class IrAttachment(models.Model): def generate_access_token(self): if self.access_token: return self.access_token - access_token = str(uuid.uuid4()) + access_token = self._generate_access_token() self.write({'access_token': access_token}) return access_token + def _generate_access_token(self): + return str(uuid.uuid4()) + @api.model def action_get(self): return self.env['ir.actions.act_window'].for_xml_id('base', 'action_attachment')