From 3cc186cd4128cef9d05c89ac0de9f87058817cdd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Fri, 31 Aug 2012 11:01:20 +0200 Subject: [PATCH 1/5] [IMP] mail.message: check is now a check_access_rule override, allowing to remove explicit calls. Moved mail.notification inheritance in mail.message directly into mail.notification, because we are in the same module (no need to separate things too much when in the same module). bzr revid: tde@openerp.com-20120831090120-s56984dg6wpbs0li --- addons/mail/mail_followers.py | 7 +++ addons/mail/mail_message.py | 109 ++++++++++------------------------ 2 files changed, 39 insertions(+), 77 deletions(-) diff --git a/addons/mail/mail_followers.py b/addons/mail/mail_followers.py index 10d33536136..5143871745b 100644 --- a/addons/mail/mail_followers.py +++ b/addons/mail/mail_followers.py @@ -59,6 +59,8 @@ class mail_notification(osv.Model): 'partner_id': fields.many2one('res.partner', string='Contact', ondelete='cascade', required=True, select=1), 'read': fields.boolean('Read'), + 'message_id': fields.many2one('mail.message', string='Message', + ondelete='cascade', required=True, select=1), } _defaults = { @@ -72,6 +74,11 @@ class mail_notification(osv.Model): return super(mail_notification, self).create(cr, uid, vals, context=context) return False + def set_message_read(self, cr, uid, msg_id, context=None): + partner_id = self.pool.get('res.users').browse(cr, uid, uid, context=context).partner_id.id + notif_ids = self.search(cr, uid, [('partner_id', '=', partner_id), ('message_id', '=', msg_id)], context=context) + return self.write(cr, uid, notif_ids, {'read': True}, context=context) + def notify(self, cr, uid, partner_ids, msg_id, context=None): """ Send by email the notification depending on the user preferences """ context = context or {} diff --git a/addons/mail/mail_message.py b/addons/mail/mail_message.py index 413606b9ed2..d1f6dab6077 100644 --- a/addons/mail/mail_message.py +++ b/addons/mail/mail_message.py @@ -34,11 +34,8 @@ def decode(text): return ''.join([tools.ustr(x[0], x[1]) for x in text]) class mail_message(osv.Model): - """Model holding messages: system notification (replacing res.log - notifications), comments (for OpenChatter feature). This model also - provides facilities to parse new email messages. Type of messages are - differentiated using the 'type' column. """ - + """ Messages model: system notification (replacing res.log notifications), + comments (OpenChatter discussion) and incoming emails. """ _name = 'mail.message' _description = 'Message' _inherit = ['ir.needaction_mixin'] @@ -76,9 +73,9 @@ class mail_message(osv.Model): return res def _search_unread(self, cr, uid, obj, name, domain, context=None): - """ Search for messages unread by the current user. """ - read_value = not domain[0][2] - read_cond = '' if read_value else '!= true' + """ Search for messages unread by the current user. Condition is + inversed because we search unread message on a read column. """ + read_cond = '' if not domain[0][2] else '!= true' partner_id = self.pool.get('res.users').browse(cr, uid, uid, context=context).partner_id.id cr.execute(""" SELECT mail_message.id \ FROM mail_message \ @@ -87,10 +84,7 @@ class mail_message(osv.Model): WHERE mail_notification.partner_id = %%s AND \ mail_notification.read %s \ """ % read_cond, (partner_id,) ) - res = cr.fetchall() - message_ids = [r[0] for r in res] - return [('id', 'in', message_ids)] - + return [('id', 'in', [r[0] for r in cr.fetchall()])] def name_get(self, cr, uid, ids, context=None): # name_get may receive int id instead of an id list @@ -103,9 +97,8 @@ class mail_message(osv.Model): return res _columns = { - # should we keep a distinction between email and comment ? 'type': fields.selection([ - ('email', 'email'), + ('email', 'Email'), ('comment', 'Comment'), ('notification', 'System notification'), ], 'Type', @@ -146,7 +139,6 @@ class mail_message(osv.Model): 'author_id': _get_default_author } - #------------------------------------------------------ # Message loading for web interface #------------------------------------------------------ @@ -261,12 +253,11 @@ class mail_message(osv.Model): break # Flatten the result - if thread_level > 0: - result = self.message_read_tree_flatten(cr, uid, result, 0, thread_level, context=context) + # if thread_level > 0: + # result = self.message_read_tree_flatten(cr, uid, result, 0, thread_level, context=context) return result - #------------------------------------------------------ # Email api #------------------------------------------------------ @@ -276,52 +267,47 @@ class mail_message(osv.Model): if not cr.fetchone(): cr.execute("""CREATE INDEX mail_message_model_res_id_idx ON mail_message (model, res_id)""") - def check(self, cr, uid, ids, mode, context=None): - """ - You can read/write a message if: - - you received it (a notification exists) or - - you can read the related document (res_model, res_id) - If a message is not attached to a document, normal access rights on - the mail.message object apply. - """ - if not ids: - return + def check_access_rule(self, cr, uid, ids, operation, context=None): + """ mail.message access rule check + - message received (a notification exists) -> ok + - check rules of related document if exists + - fallback on normal mail.message check """ if isinstance(ids, (int, long)): ids = [ids] - partner_id = self.pool.get('res.users').browse(cr, uid, uid, context=context).partner_id.id - # check messages for which you have a notification + partner_id = self.pool.get('res.users').browse(cr, uid, uid, context=context).partner_id.id not_obj = self.pool.get('mail.notification') not_ids = not_obj.search(cr, uid, [ ('partner_id', '=', partner_id), ('message_id', 'in', ids), ], context=context) - for notification in not_obj.browse(cr, uid, not_ids, context=context): - if notification.message_id.id in ids: - pass - # FP Note: we should put this again !!! - #ids.remove(notification.message_id.id) + notified_ids = [notification.message_id.id for notification in not_obj.browse(cr, uid, not_ids, context=context) + if notification.message_id.id in ids] - # check messages according to related documents - res_ids = {} - cr.execute('SELECT DISTINCT model, res_id FROM mail_message WHERE id = ANY (%s)', (ids,)) - for rmod, rid in cr.fetchall(): + # check messages linked to an existing document + model_record_ids = {} + document_ids = [] + cr.execute('SELECT DISTINCT id, model, res_id FROM mail_message WHERE id = ANY (%s)', (ids,)) + for id, rmod, rid in cr.fetchall(): if not (rmod and rid): continue - res_ids.setdefault(rmod,set()).add(rid) + document_ids.append(id) + model_record_ids.setdefault(rmod,set()).add(rid) + for model, mids in model_record_ids.items(): + model_obj = self.pool.get(model) + mids = model_obj.exists(cr, uid, mids) + model_obj.check_access_rights(cr, uid, operation) + model_obj.check_access_rule(cr, uid, mids, operation, context=context) - ima_obj = self.pool.get('ir.model.access') - for model, mids in res_ids.items(): - mids = self.pool.get(model).exists(cr, uid, mids) - ima_obj.check(cr, uid, model, mode) - self.pool.get(model).check_access_rule(cr, uid, mids, mode, context=context) + # fall back on classic operation for other ids + other_ids = set(ids).difference(set(notified_ids), set(document_ids)) + super(mail_message, self).check_access_rule(cr, uid, other_ids, operation, context=None) def create(self, cr, uid, values, context=None): if not values.get('message_id') and values.get('res_id') and values.get('model'): values['message_id'] = tools.generate_tracking_message_id('%(model)s-%(res_id)s'% values) newid = super(mail_message, self).create(cr, uid, values, context) - self.check(cr, uid, [newid], mode='create', context=context) self.notify(cr, uid, newid, context=context) return newid @@ -344,40 +330,9 @@ class mail_message(osv.Model): partners_to_notify |= extra_notified self.pool.get('mail.notification').notify(cr, uid, list(partners_to_notify), newid, context=context) - def read(self, cr, uid, ids, fields_to_read=None, context=None, load='_classic_read'): - self.check(cr, uid, ids, 'read', context=context) - return super(mail_message, self).read(cr, uid, ids, fields_to_read, context, load) - def copy(self, cr, uid, id, default=None, context=None): """Overridden to avoid duplicating fields that are unique to each email""" if default is None: default = {} - self.check(cr, uid, [id], 'read', context=context) default.update(message_id=False, headers=False) return super(mail_message,self).copy(cr, uid, id, default=default, context=context) - - def write(self, cr, uid, ids, vals, context=None): - result = super(mail_message, self).write(cr, uid, ids, vals, context) - self.check(cr, uid, ids, 'write', context=context) - return result - - def unlink(self, cr, uid, ids, context=None): - self.check(cr, uid, ids, 'unlink', context=context) - return super(mail_message, self).unlink(cr, uid, ids, context) - - -class mail_notification(osv.Model): - """ mail_notification is a relational table modeling messages pushed to partners. - """ - _inherit = 'mail.notification' - _columns = { - 'message_id': fields.many2one('mail.message', string='Message', - ondelete='cascade', required=True, select=1), - } - - def set_message_read(self, cr, uid, msg_id, context=None): - partner_id = self.pool.get('res.users').browse(cr, uid, uid, context=context).partner_id.id - notif_ids = self.search(cr, uid, [('partner_id', '=', partner_id), ('message_id', '=', msg_id)], context=context) - return self.write(cr, uid, notif_ids, {'read': True}, context=context) - - From 21f17ba8a57faf038f6a1f7ba650aced726d4626 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Fri, 31 Aug 2012 11:31:39 +0200 Subject: [PATCH 2/5] [CLEAN] mail_mail: misc clean of comments and commented lines; also renamed message as mail in send, to have a more accurate variable name. bzr revid: tde@openerp.com-20120831093139-9o8ui5h912rx9i13 --- addons/mail/mail_mail.py | 88 +++++++++++++++++++--------------------- 1 file changed, 41 insertions(+), 47 deletions(-) diff --git a/addons/mail/mail_mail.py b/addons/mail/mail_mail.py index 7f3a8a880ea..36b71653556 100644 --- a/addons/mail/mail_mail.py +++ b/addons/mail/mail_mail.py @@ -31,11 +31,8 @@ from tools.translate import _ _logger = logging.getLogger(__name__) class mail_mail(osv.Model): - """ - Model holding RFC2822 email messages to send. This model also provides - facilities to queue and send new email messages. - """ - + """ Model holding RFC2822 email messages to send. This model also provides + facilities to queue and send new email messages. """ _name = 'mail.mail' _description = 'Outgoing Mails' _inherits = {'mail.message': 'mail_message_id'} @@ -64,9 +61,7 @@ class mail_mail(osv.Model): def _get_default_from(self, cr, uid, context=None): cur = self.pool.get('res.users').browse(cr, uid, uid, context=context) if not cur.alias_domain: - # TDE temp: impossible to install social module, because mail.group tries to send emails ... - # raise osv.except_osv(_('Invalid Action!'), _('Unable to send email, set an alias domain in your server settings.')) - pass + raise osv.except_osv(_('Invalid Action!'), _('Unable to send email, set an alias domain in your server settings.')) return cur.alias_name + '@' + cur.alias_domain _defaults = { @@ -127,69 +122,68 @@ class mail_mail(osv.Model): message.unlink() return True - def send(self, cr, uid, ids, auto_commit=False, context=None): - """Sends the selected emails immediately, ignoring their current - state (mails that have already been sent should not be passed - unless they should actually be re-sent). - Emails successfully delivered are marked as 'sent', and those - that fail to be deliver are marked as 'exception', and the - corresponding error message is output in the server logs. + def _send_get_mail_subject(self, cr, uid, mail, force=False, context=None): + """ if void and related document: ' posted on ' + :param mail: mail.mail browse_record """ + if force or (not mail.subject and mail.model and mail.res_id): + return '%s posted on %s' % (mail.author_id.name, mail.record_name) + return mail.subject - :param bool auto_commit: whether to force a commit of the message - status after sending each message (meant - only for processing by the scheduler), - should never be True during normal - transactions (default: False) - :return: True + def send(self, cr, uid, ids, auto_commit=False, context=None): + """ Sends the selected emails immediately, ignoring their current + state (mails that have already been sent should not be passed + unless they should actually be re-sent). + Emails successfully delivered are marked as 'sent', and those + that fail to be deliver are marked as 'exception', and the + corresponding error mail is output in the server logs. + + :param bool auto_commit: whether to force a commit of the mail status + after sending each mail (meant only for scheduler processing); + should never be True during normal transactions (default: False) + :return: True """ ir_mail_server = self.pool.get('ir.mail_server') - for message in self.browse(cr, uid, ids, context=context): + for mail in self.browse(cr, uid, ids, context=context): try: - body = message.body_html + body = mail.body_html + subject = self._send_get_mail_subject(cr, uid, mail, context=context) # handle attachments attachments = [] - for attach in message.attachment_ids: + for attach in mail.attachment_ids: attachments.append((attach.datas_fname, base64.b64decode(attach.datas))) - # no subject, res_id, model: ' posted on ' - if not message.subject and message.model and message.res_id: - subject = '%s posted on %s' % (message.author_id.name, message.record_name) - else: - subject = message.subject - # use only sanitized html and set its plaintexted version as alternative body_alternative = tools.html2plaintext(body) content_subtype_alternative = 'plain' - # build an RFC2822 email.message.Message object and send it - # without queuing + # build an RFC2822 email.message.Message object and send it without queuing msg = ir_mail_server.build_email( - email_from = message.email_from, - email_to = tools.email_split(message.email_to), + email_from = mail.email_from, + email_to = tools.email_split(mail.email_to), subject = subject, body = body, body_alternative = body_alternative, - email_cc = tools.email_split(message.email_cc), - reply_to = message.reply_to, + email_cc = tools.email_split(mail.email_cc), + reply_to = mail.reply_to, attachments = attachments, - message_id = message.message_id, - references = message.references, - object_id = message.res_id and ('%s-%s' % (message.res_id,message.model)), + message_id = mail.message_id, + references = mail.references, + object_id = mail.res_id and ('%s-%s' % (mail.res_id, mail.model)), subtype = 'html', subtype_alternative = content_subtype_alternative) res = ir_mail_server.send_email(cr, uid, msg, - mail_server_id=message.mail_server_id.id, context=context) + mail_server_id=mail.mail_server_id.id, context=context) if res: - message.write({'state':'sent', 'message_id': res}) + mail.write({'state':'sent', 'message_id': res}) else: - message.write({'state':'exception'}) - message.refresh() - if message.state == 'sent': - self._postprocess_sent_message(cr, uid, message, context=context) + mail.write({'state':'exception'}) + mail.refresh() + if mail.state == 'sent': + self._postprocess_sent_message(cr, uid, mail, context=context) except Exception: - _logger.exception('failed sending mail.mail %s', message.id) - message.write({'state':'exception'}) + _logger.exception('failed sending mail.mail %s', mail.id) + mail.write({'state':'exception'}) if auto_commit == True: cr.commit() From 86dd3bdc57b4498264900a498ac5f12e532a1109 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Fri, 31 Aug 2012 13:23:53 +0200 Subject: [PATCH 3/5] [FIX] mail_message: fixed and simplified unread messages query. Added an index on the read column of mail_notification. bzr revid: tde@openerp.com-20120831112353-egmeetrnecp2kflp --- addons/mail/mail_followers.py | 2 +- addons/mail/mail_message.py | 15 +++++++-------- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/addons/mail/mail_followers.py b/addons/mail/mail_followers.py index 5143871745b..9be82d077c6 100644 --- a/addons/mail/mail_followers.py +++ b/addons/mail/mail_followers.py @@ -58,7 +58,7 @@ class mail_notification(osv.Model): _columns = { 'partner_id': fields.many2one('res.partner', string='Contact', ondelete='cascade', required=True, select=1), - 'read': fields.boolean('Read'), + 'read': fields.boolean('Read', select=1), 'message_id': fields.many2one('mail.message', string='Message', ondelete='cascade', required=True, select=1), } diff --git a/addons/mail/mail_message.py b/addons/mail/mail_message.py index d1f6dab6077..d4dc66d75e0 100644 --- a/addons/mail/mail_message.py +++ b/addons/mail/mail_message.py @@ -75,15 +75,14 @@ class mail_message(osv.Model): def _search_unread(self, cr, uid, obj, name, domain, context=None): """ Search for messages unread by the current user. Condition is inversed because we search unread message on a read column. """ - read_cond = '' if not domain[0][2] else '!= true' + if domain[0][2]: + read_cond = '(read = false or read is null)' + else: + read_cond = 'read = true' partner_id = self.pool.get('res.users').browse(cr, uid, uid, context=context).partner_id.id - cr.execute(""" SELECT mail_message.id \ - FROM mail_message \ - JOIN mail_notification ON ( \ - mail_notification.message_id = mail_message.id ) \ - WHERE mail_notification.partner_id = %%s AND \ - mail_notification.read %s \ - """ % read_cond, (partner_id,) ) + cr.execute("SELECT message_id FROM mail_notification "\ + "WHERE partner_id = %%s AND %s" % read_cond, + (partner_id,)) return [('id', 'in', [r[0] for r in cr.fetchall()])] def name_get(self, cr, uid, ids, context=None): From 9e6cce28b80e64e7df2d8e350e02fa14566e0b6b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Fri, 31 Aug 2012 13:27:21 +0200 Subject: [PATCH 4/5] [FIX] mail: tests: fixed followers test, because using set() allows to override a test that was precisely about not having a same value more than once in follower_ids. bzr revid: tde@openerp.com-20120831112721-56qfnh9vk1xkbke0 --- addons/mail/tests/test_mail.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/addons/mail/tests/test_mail.py b/addons/mail/tests/test_mail.py index 12ae25e1815..fd4b116b364 100644 --- a/addons/mail/tests/test_mail.py +++ b/addons/mail/tests/test_mail.py @@ -203,17 +203,19 @@ class test_mail(common.TransactionCase): user_raoul_id = self.res_users.create(cr, uid, {'name': 'Raoul Grosbedon', 'login': 'raoul'}) user_raoul = self.res_users.browse(cr, uid, user_raoul_id) - # Subscribe Raoul twice (niak niak) through message_subscribe_users + # Subscribe Raoul three times (niak niak) through message_subscribe_users group_pigs.message_subscribe_users([user_raoul_id, user_raoul_id]) + group_pigs.message_subscribe_users([user_raoul_id]) group_pigs.refresh() - follower_ids = set([follower.id for follower in group_pigs.message_follower_ids]) - self.assertEqual(follower_ids, set([user_raoul.partner_id.id, user_admin.partner_id.id]), 'Admin and Raoul should be the only 2 Pigs group followers') + follower_ids = [follower.id for follower in group_pigs.message_follower_ids] + self.assertEqual(len(follower_ids), 2, 'There should be 2 Pigs fans') + self.assertEqual(set(follower_ids), set([user_raoul.partner_id.id, user_admin.partner_id.id]), 'Admin and Raoul should be the only 2 Pigs fans') # Unsubscribe Raoul twice through message_unsubscribe_users group_pigs.message_unsubscribe_users([user_raoul_id, user_raoul_id]) group_pigs.refresh() - follower_ids = set([follower.id for follower in group_pigs.message_follower_ids]) - self.assertEqual(follower_ids, set([user_admin.partner_id.id]), 'Admin must be the only Pigs fan') + follower_ids = [follower.id for follower in group_pigs.message_follower_ids] + self.assertEqual(follower_ids, [user_admin.partner_id.id], 'Admin must be the only Pigs fan') def test_20_message_post(self): """ Tests designed for message_post. """ From 8f59e1d906d623a4ac62c788e5d5792a69becb4e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Fri, 31 Aug 2012 13:52:02 +0200 Subject: [PATCH 5/5] [IMP] mail_notification: removed select on columns, added creation of a combined index in init. bzr revid: tde@openerp.com-20120831115202-q3fvj4tjoglaqw9c --- addons/mail/mail_followers.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/addons/mail/mail_followers.py b/addons/mail/mail_followers.py index 9be82d077c6..9646744fae8 100644 --- a/addons/mail/mail_followers.py +++ b/addons/mail/mail_followers.py @@ -57,16 +57,21 @@ class mail_notification(osv.Model): _columns = { 'partner_id': fields.many2one('res.partner', string='Contact', - ondelete='cascade', required=True, select=1), - 'read': fields.boolean('Read', select=1), + ondelete='cascade', required=True), + 'read': fields.boolean('Read'), 'message_id': fields.many2one('mail.message', string='Message', - ondelete='cascade', required=True, select=1), + ondelete='cascade', required=True), } _defaults = { 'read': False, } + def init(self, cr): + cr.execute('SELECT indexname FROM pg_indexes WHERE indexname = %s', ('mail_notification_partner_id_read_message_id',)) + if not cr.fetchone(): + cr.execute('CREATE INDEX mail_notification_partner_id_read_message_id ON mail_notification (partner_id, read, message_id)') + def create(self, cr, uid, vals, context=None): """ Override of create to check that we can not create a notification for a message the user can not read. """