From 488a5d5af5418ed5c223af5b61aea0b3100508c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Wed, 22 May 2019 15:10:54 +0000 Subject: [PATCH 1/8] [IMP] mail: prepare new SMS model by updating mail.notification This commit prepares SMS refactoring by updating some mail models. Purpose is to be able to store SMS notification information inside existing mail flow and models. Main changes * mail.notification: define a notification_type field to store the medium used to notify people. In mail two ways exist: inbox and email. It will ease introduction of SMS notification mechanism. This field replaces the is_email boolean field; * mail.notification: make res_partner_id field not required. This means notifications could be linked to something else than a partner. An SMS for example. For Inbox and email notifications partner is still required and a constraint is added accordingly; * mail.thread: let notification methods handle the creation and update of their notification instead of creating them in _notify_thread and tweaking them in sub notification methods; * mail.thread: clearly move inbox-style notification in its own method like notify by email; Some lighter code changes * propagate message_type to notification recipient computation. It will allow for example to be more precise when computing a notification type depending on the message_type. For example, send a notification by SMS when the message_type is SMS; * ease inheritance of ``_notify_thread`` by returning computed recipients data. It will allow to work on it without having to re-compute it; * propagate kwargs from message_post and message_notify to notify methods. This allow to avoid depending on context and set explicit parameters. Drawback is that message_post and notify must separate kwargs used to create a message and those that are propagated to notification methods; * update various notification check to ensure they work on email notifications, notably the resend and cancel wizards; One side effect of this commit is that notifications are created by the relevant notification method. Previously all notifications were created by writing on needaction_partner_ids fields then updated according to the notification process. Notably there could be too much notification created when sending emails due to _notify_customize_recipients not being correctly synchronized with notification. This issue is now solved as only really sent emails create notifications. Migration tips * notification_type: notification.is_email and 'email' else 'inbox'; * remove is_email; Related to task 1922163 Linked to PR #33510 --- addons/mail/models/mail_channel.py | 9 +- addons/mail/models/mail_followers.py | 3 +- addons/mail/models/mail_mail.py | 18 +- addons/mail/models/mail_message.py | 88 +++++---- addons/mail/models/mail_notification.py | 21 ++- addons/mail/models/mail_thread.py | 208 ++++++++++++--------- addons/mail/views/mail_message_views.xml | 4 +- addons/mail/wizard/invite.py | 14 +- addons/mail/wizard/mail_resend_cancel.py | 8 +- addons/mail/wizard/mail_resend_message.py | 32 ++-- addons/test_mail/tests/common.py | 4 +- addons/test_mail/tests/test_mail_race.py | 23 +-- addons/test_mail/tests/test_mail_resend.py | 2 +- addons/test_mail/tests/test_performance.py | 26 +-- addons/website_blog/models/website_blog.py | 9 +- addons/website_forum/models/forum.py | 9 +- 16 files changed, 265 insertions(+), 213 deletions(-) diff --git a/addons/mail/models/mail_channel.py b/addons/mail/models/mail_channel.py index 7912a856ca4..cbc548825d9 100644 --- a/addons/mail/models/mail_channel.py +++ b/addons/mail/models/mail_channel.py @@ -503,15 +503,10 @@ class Channel(models.Model): notifications.append([(self._cr.dbname, 'res.partner', partner.id), channel_info]) return notifications - def _notify_thread(self, message, msg_vals=False, model_description=False, mail_auto_delete=True): + def _notify_thread(self, message, msg_vals=False, **kwargs): # 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': - super(Channel, self)._notify_thread( - message, - msg_vals=msg_vals, - model_description=model_description, - mail_auto_delete=mail_auto_delete, - ) + super(Channel, self)._notify_thread(message, msg_vals=msg_vals, **kwargs) else: message._notify_pending_by_chat() diff --git a/addons/mail/models/mail_followers.py b/addons/mail/models/mail_followers.py index 1b5c94972c2..417b83e59f5 100644 --- a/addons/mail/models/mail_followers.py +++ b/addons/mail/models/mail_followers.py @@ -81,7 +81,7 @@ class Followers(models.Model): # Private tools methods to fetch followers data # -------------------------------------------------- - def _get_recipient_data(self, records, subtype_id, pids=None, cids=None): + def _get_recipient_data(self, records, message_type, subtype_id, pids=None, cids=None): """ Private method allowing to fetch recipients data based on a subtype. Purpose of this method is to fetch all data necessary to notify recipients in a single query. It fetches data from @@ -92,6 +92,7 @@ class Followers(models.Model): * channels if cids is given; :param records: fetch data from followers of records that follow subtype_id; + :param message_type: mail.message.message_type in order to allow custom behavior depending on it (SMS for example); :param subtype_id: mail.message.subtype to check against followers; :param pids: additional set of partner IDs from which to fetch recipient data; :param cids: additional set of channel IDs from which to fetch recipient data; diff --git a/addons/mail/models/mail_mail.py b/addons/mail/models/mail_mail.py index f97d0a52cde..f9069021307 100644 --- a/addons/mail/models/mail_mail.py +++ b/addons/mail/models/mail_mail.py @@ -154,24 +154,24 @@ class MailMail(models.Model): notif_mails_ids = [mail.id for mail in self if mail.notification] if notif_mails_ids: notifications = self.env['mail.notification'].search([ - ('is_email', '=', True), + ('notification_type', '=', 'email'), ('mail_id', 'in', notif_mails_ids), - ('email_status', 'not in', ('sent', 'canceled')) + ('notification_status', 'not in', ('sent', 'canceled')) ]) if notifications: - #find all notification linked to a failure + # find all notification linked to a failure failed = self.env['mail.notification'] if failure_type: failed = notifications.filtered(lambda notif: notif.res_partner_id not in success_pids) failed.sudo().write({ - 'email_status': 'exception', + 'notification_status': 'exception', 'failure_type': failure_type, 'failure_reason': failure_reason, }) messages = notifications.mapped('mail_message_id').filtered(lambda m: m.is_thread_message()) - messages._notify_failure_update() # notify user that we have a failure + messages._notify_mail_failure_update() # notify user that we have a failure (notifications - failed).sudo().write({ - 'email_status': 'sent', + 'notification_status': 'sent', 'failure_type': '', 'failure_reason': '', }) @@ -335,14 +335,14 @@ class MailMail(models.Model): # update in case an email bounces while sending all emails related to current # mail record. notifs = self.env['mail.notification'].search([ - ('is_email', '=', True), + ('notification_type', '=', 'email'), ('mail_id', 'in', mail.ids), - ('email_status', 'not in', ('sent', 'canceled')) + ('notification_status', 'not in', ('sent', 'canceled')) ]) if notifs: notif_msg = _('Error without exception. Probably due do concurrent access update of notification records. Please see with an administrator.') notifs.sudo().write({ - 'email_status': 'exception', + 'notification_status': 'exception', 'failure_type': 'UNKNOWN', 'failure_reason': notif_msg, }) diff --git a/addons/mail/models/mail_message.py b/addons/mail/models/mail_message.py index e90249e9533..45b71613a85 100644 --- a/addons/mail/models/mail_message.py +++ b/addons/mail/models/mail_message.py @@ -5,6 +5,7 @@ import logging import re from binascii import Error as binascii_error +from collections import defaultdict from operator import itemgetter from email.utils import formataddr from openerp.http import request @@ -148,15 +149,15 @@ class Message(models.Model): def _compute_has_error(self): error_from_notification = self.env['mail.notification'].sudo().search([ ('mail_message_id', 'in', self.ids), - ('email_status', 'in', ('bounce', 'exception'))]).mapped('mail_message_id') + ('notification_status', 'in', ('bounce', 'exception'))]).mapped('mail_message_id') for message in self: message.has_error = message in error_from_notification @api.multi def _search_has_error(self, operator, operand): if operator == '=' and operand: - return [('notification_ids.email_status', 'in', ('bounce', 'exception'))] - return ['!', ('notification_ids.email_status', 'in', ('bounce', 'exception'))] # this wont work and will be equivalent to "not in" beacause of orm restrictions. Dont use "has_error = False" + return [('notification_ids.notification_status', 'in', ('bounce', 'exception'))] + return ['!', ('notification_ids.notification_status', 'in', ('bounce', 'exception'))] # this wont work and will be equivalent to "not in" beacause of orm restrictions. Dont use "has_error = False" @api.depends('starred_partner_ids') def _get_starred(self): @@ -373,25 +374,22 @@ class Message(models.Model): partner_ids = [] if message.subtype_id: partner_ids = [partner_tree[partner.id] for partner in message.partner_ids - if partner.id in partner_tree] + if partner.id in partner_tree] else: partner_ids = [partner_tree[partner.id] for partner in message.partner_ids - if partner.id in partner_tree] + if partner.id in partner_tree] # we read customer_email_status before filtering inactive user because we don't want to miss a red enveloppe customer_email_status = ( - (all(n.email_status == 'sent' for n in message.notification_ids) and 'sent') or - (any(n.email_status == 'exception' for n in message.notification_ids) and 'exception') or - (any(n.email_status == 'bounce' for n in message.notification_ids) and 'bounce') or + (all(n.notification_status == 'sent' for n in message.notification_ids if n.notification_type == 'email') and 'sent') or + (any(n.notification_status == 'exception' for n in message.notification_ids if n.notification_type == 'email') and 'exception') or + (any(n.notification_status == 'bounce' for n in message.notification_ids if n.notification_type == 'email') and 'bounce') or 'ready' ) customer_email_data = [] - def filter_notification(notif): - return ( - (notif.email_status in ('bounce', 'exception', 'canceled') or notif.res_partner_id.partner_share) and - notif.res_partner_id.active - ) - for notification in message.notification_ids.filtered(filter_notification): - customer_email_data.append((partner_tree[notification.res_partner_id.id][0], partner_tree[notification.res_partner_id.id][1], notification.email_status)) + for notification in message.notification_ids.filtered( + lambda n: n.notification_type == 'email' and n.res_partner_id.active and + (n.notification_status in ('bounce', 'exception', 'canceled') or n.res_partner_id.partner_share)): + customer_email_data.append((partner_tree[notification.res_partner_id.id][0], partner_tree[notification.res_partner_id.id][1], notification.notification_status)) has_access_to_model = message.model and self.env[message.model].check_access_rights('read', raise_exception=False) if message.attachment_ids and message.res_id and issubclass(self.pool[message.model], self.pool['mail.thread']) and has_access_to_model: @@ -512,7 +510,7 @@ class Message(models.Model): # fetch notification status notif_dict = {} - notifs = self.env['mail.notification'].sudo().search([('mail_message_id', 'in', list(mid for mid in message_tree)), ('is_read', '=', False)]) + notifs = self.env['mail.notification'].sudo().search([('mail_message_id', 'in', list(mid for mid in message_tree)), ('res_partner_id', '!=', False), ('is_read', '=', False)]) for notif in notifs: mid = notif.mail_message_id.id if not notif_dict.get(mid): @@ -540,13 +538,45 @@ class Message(models.Model): 'moderation_status', ] + def _get_mail_failure_dict(self): + return { + 'message_id': self.id, + 'record_name': self.record_name, + 'model_name': self.env['ir.model']._get(self.model).display_name, + 'uuid': self.message_id, + 'res_id': self.res_id, + 'model': self.model, + 'last_message_date': self.date, + 'module_icon': '/mail/static/src/img/smiley/mailfailure.jpg', + } + @api.multi def _format_mail_failures(self): - """ - A shorter message to notify a failure update - """ + """ A shorter message to notify a failure update """ failures_infos = [] + + # prepare notifications computation in batch + all_notifications = self.env['mail.notification'].sudo().search([ + ('mail_message_id', 'in', self.ids) + ]) + msgid_to_notif = defaultdict(lambda: self.env['mail.notification'].sudo()) + for notif in all_notifications: + msgid_to_notif[notif.mail_message_id.id] += notif + # for each channel, build the information header and include the logged partner information + for message in self: + notifications = msgid_to_notif[message.id] + if not any(notification.notification_type == 'email' for notification in notifications): + continue + info = dict(message._get_mail_failure_dict(), + failure_type='mail', + notifications=dict((notif.res_partner_id.id, (notif.notification_status, notif.res_partner_id.name)) for notif in notifications)) + failures_infos.append(info) + return failures_infos + + @api.multi + def _notify_mail_failure_update(self): + messages = self.env['mail.message'] for message in self: # Check if user has access to the record before displaying a notification about it. # In case the user switches from one company to another, it might happen that he doesn't @@ -558,24 +588,10 @@ class Message(models.Model): record.check_access_rule('read') except AccessError: continue - info = { - 'message_id': message.id, - 'record_name': message.record_name, - 'model_name': self.env['ir.model']._get(message.model).display_name, - 'uuid': message.message_id, - 'res_id': message.res_id, - 'model': message.model, - 'last_message_date': message.date, - 'module_icon': '/mail/static/src/img/smiley/mailfailure.jpg', - 'notifications': dict((notif.res_partner_id.id, (notif.email_status, notif.res_partner_id.name)) for notif in message.notification_ids.sudo()) - } - failures_infos.append(info) - return failures_infos + else: + messages |= message - @api.multi - def _notify_failure_update(self): - authors = {} - for author, author_messages in groupby(self, itemgetter('author_id')): + for author, author_messages in groupby(messages, itemgetter('author_id')): self.env['bus.bus'].sendone( (self._cr.dbname, 'res.partner', author.id), {'type': 'mail_failure', 'elements': self.env['mail.message'].concat(*author_messages)._format_mail_failures()} diff --git a/addons/mail/models/mail_notification.py b/addons/mail/models/mail_notification.py index 3397342a818..5cf42a2dc72 100644 --- a/addons/mail/models/mail_notification.py +++ b/addons/mail/models/mail_notification.py @@ -14,10 +14,12 @@ class Notification(models.Model): mail_message_id = fields.Many2one( 'mail.message', 'Message', index=True, ondelete='cascade', required=True) res_partner_id = fields.Many2one( - 'res.partner', 'Needaction Recipient', index=True, ondelete='cascade', required=True) + 'res.partner', 'Needaction Recipient', index=True, ondelete='cascade', required=False) is_read = fields.Boolean('Is Read', index=True) - is_email = fields.Boolean('Sent by Email', index=True) - email_status = fields.Selection([ + notification_type = fields.Selection([ + ('inbox', 'Inbox'), ('email', 'Email')], string='Notification Type', + default='inbox', index=True, required=True) + notification_status = fields.Selection([ ('ready', 'Ready to Send'), ('sent', 'Sent'), ('bounce', 'Bounced'), @@ -37,10 +39,17 @@ class Notification(models.Model): ], string='Failure type') failure_reason = fields.Text('Failure reason', copy=False) + _sql_constraints = [ + # email notification;: partner is required + ('notification_partner_required', + "CHECK(notification_type in ('email', 'inbox') AND res_partner_id IS NOT NULL)", + 'Customer is required for inbox / email notification'), + ] + def init(self): - self._cr.execute('SELECT indexname FROM pg_indexes WHERE indexname = %s', ('mail_notification_res_partner_id_is_read_email_status_mail_message_id',)) + self._cr.execute('SELECT indexname FROM pg_indexes WHERE indexname = %s', ('mail_notification_res_partner_id_is_read_notification_status_mail_message_id',)) if not self._cr.fetchone(): - self._cr.execute('CREATE INDEX mail_notification_res_partner_id_is_read_email_status_mail_message_id ON mail_message_res_partner_needaction_rel (res_partner_id, is_read, email_status, mail_message_id)') + self._cr.execute('CREATE INDEX mail_notification_res_partner_id_is_read_notification_status_mail_message_id ON mail_message_res_partner_needaction_rel (res_partner_id, is_read, notification_status, mail_message_id)') @api.multi def format_failure_reason(self): @@ -49,5 +58,3 @@ class Notification(models.Model): return dict(type(self).failure_type.selection).get(self.failure_type, _('No Error')) else: return _("Unknown error") + ": %s" % (self.failure_reason or '') - - diff --git a/addons/mail/models/mail_thread.py b/addons/mail/models/mail_thread.py index c1feb9ba820..432c0657b42 100644 --- a/addons/mail/models/mail_thread.py +++ b/addons/mail/models/mail_thread.py @@ -226,7 +226,7 @@ class MailThread(models.AbstractModel): if self.ids: self._cr.execute(""" SELECT msg.res_id, COUNT(msg.res_id) FROM mail_message msg RIGHT JOIN mail_message_res_partner_needaction_rel rel - ON rel.mail_message_id = msg.id AND rel.email_status in ('exception','bounce') + ON rel.mail_message_id = msg.id AND rel.notification_status in ('exception','bounce') WHERE msg.author_id = %s AND msg.model = %s AND msg.res_id in %s AND msg.message_type != 'user_notification' GROUP BY msg.res_id""", (self.env.user.partner_id.id, self._name, tuple(self.ids),)) @@ -915,7 +915,7 @@ class MailThread(models.AbstractModel): ('mail_message_id', '=', mail_message.id), ('res_partner_id', 'in', partners.ids)]) notifications.write({ - 'email_status': 'bounce' + 'notification_status': 'bounce' }) if bounced_model in self.env and hasattr(self.env[bounced_model], '_message_receive_bounce') and bounced_thread_id: @@ -1679,7 +1679,7 @@ class MailThread(models.AbstractModel): 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, + add_sign=True, record_name=False, **kwargs): """ Post a new message in an existing thread, returning the new mail.message ID. @@ -1707,11 +1707,14 @@ class MailThread(models.AbstractModel): :return int: ID of newly created mail.message """ self.ensure_one() # should always be posted on a record, use message_notify if no record + # split message additional values from notify additional values + msg_kwargs = dict((key, val) for key, val in kwargs.items() if key in self.env['mail.message']._fields) + notif_kwargs = dict((key, val) for key, val in kwargs.items() if key not in msg_kwargs) if self._name == 'mail.thread' or not self.id or message_type == 'user_notification': raise ValueError('message_post should only be call to post message on record. Use message_notify instead') - if 'model' in kwargs or 'res_id' in kwargs: + if 'model' in msg_kwargs or 'res_id' in msg_kwargs: raise ValueError("message_post doesn't support model and res_id parameters anymore. Please call message_post on record") self = self.with_lang() # add lang to context imediatly since it will be usefull in various flows latter. @@ -1746,7 +1749,7 @@ class MailThread(models.AbstractModel): # 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: + 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 @@ -1757,7 +1760,8 @@ class MailThread(models.AbstractModel): processed_list.append(new_parent_id) parent_message = parent_message.parent_id parent_id = parent_message.id - values = dict(kwargs) + + values = dict(msg_kwargs) values.update({ 'author_id': author_id, 'model': self._name, @@ -1776,19 +1780,19 @@ class MailThread(models.AbstractModel): 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] + values.update(attachement_values) # attachement_ids, [body] - new_message= self._message_create(values) + new_message = self._message_create(values) # Set main attachment field if necessary self._message_set_main_attachment_id(values['attachment_ids']) if values['author_id'] and values['message_type'] != 'notification' and not self._context.get('mail_create_nosubscribe'): - #if self.env['res.partner'].browse(values['author_id']).active: # we dont want to add odoobot/inactive as a follower + # if self.env['res.partner'].browse(values['author_id']).active: # we dont want to add odoobot/inactive as a follower self._message_subscribe([values['author_id']]) self._message_post_after_hook(new_message, values) - self._notify_thread(new_message, values, model_description=model_description, mail_auto_delete=mail_auto_delete) + self._notify_thread(new_message, values, **notif_kwargs) return new_message def _message_set_main_attachment_id(self, attachment_ids): # todo move this out of mail.thread @@ -1863,14 +1867,15 @@ class MailThread(models.AbstractModel): return composer.send_mail() def message_notify(self, partner_ids=False, parent_id=False, model=False, res_id=False, - author_id=False, body='', subject=False, model_description=False, - mail_auto_delete=True, **kwargs): + author_id=False, body='', subject=False, **kwargs): """ Shortcut allowing to notify partners of messages that shouldn't be displayed on a document. It pushes notifications on inbox or by email depending on the user configuration, like other notifications. """ - if self: self.ensure_one() + # split message additional values from notify additional values + msg_kwargs = dict((key, val) for key, val in kwargs.items() if key in self.env['mail.message']._fields) + notif_kwargs = dict((key, val) for key, val in kwargs.items() if key not in msg_kwargs) if author_id: author = self.env['res.partner'].sudo().browse(author_id) @@ -1914,9 +1919,9 @@ class MailThread(models.AbstractModel): 'reply_to': MailThread._notify_get_reply_to(default=email_from, records=None)[False], 'message_id': tools.generate_tracking_message_id('message-notify'), } - values.update(kwargs) + values.update(msg_kwargs) new_message = MailThread._message_create(values) - MailThread._notify_thread(new_message, values, model_description=model_description, mail_auto_delete=mail_auto_delete) + MailThread._notify_thread(new_message, values, **notif_kwargs) return new_message def _message_log(self, body='', author_id=None, subject=False, message_type='notification', **kwargs): @@ -1972,24 +1977,27 @@ class MailThread(models.AbstractModel): # ------------------------------------------------------ @api.multi - def _notify_thread(self, message, msg_vals=False, model_description=False, mail_auto_delete=True): + def _notify_thread(self, message, msg_vals=False, **kwargs): """ Main notification method. This method basically does two things + * call ``_notify_compute_recipients`` that computes recipients to notify based on message record or message creation values if given (to optimize performance if we already have data computed); - * performs the notification process; - Can be overridden to intercept and postpone notification mecanism (mail.channel moderation) - :param message: posted message; - :param msg_vals: dictionary of values used to create the message. If given - it is used instead of accessing ``self`` to lesen query count in some - simple cases where no notification is actually required; - :param force_send: tells whether to send notification emails within the - current transaction or to use the email queue; - :param model_description: optional data used in notification process (see - notification templates); - :param mail_auto_delete: delete notification emails once sent; - """ + * performs the notification process by calling the various notification + methods implemented; + This method cnn be overridden to intercept and postpone notification + mechanism like mail.channel moderation. + + :param message: mail.message record to notify; + :param msg_vals: dictionary of values used to create the message. If given + it is used instead of accessing ``self`` to lessen query count in some + simple cases where no notification is actually required; + + Kwargs allow to pass various parameters that are given to sub notification + methods. See those methods for more details about the additional parameters. + Parameters used for email-style notifications + """ msg_vals = msg_vals if msg_vals else {} rdata = self._notify_compute_recipients(message, msg_vals) if not rdata: @@ -1998,59 +2006,78 @@ class MailThread(models.AbstractModel): message_values = {} if rdata['channels']: message_values['channel_ids'] = [(6, 0, [r['id'] for r in rdata['channels']])] - if rdata['partners']: - message_values['needaction_partner_ids'] = [(6, 0, [r['id'] for r in rdata['partners'] if r['type'] != 'channel_email'])] - # change of behavior to check: since email_cids partner are added in _notify_compute_recipients, - # they will be added to needaction_partner_ids to. - # we may want to filter them (example with channel_email, a cleaner solution may be great) - # -> instead of using _notify_customize_recipients, we could add a flag on rdata - # (would work for needactions, not if we want to erase partner_ids, ids) - # (could also be interesting for, we could add partners with r['notif'] = 'ocn_client' and r['needaction']=False) - # then override a notify_recipients (as it was before) to effectively send ocn notifications. - # envelope will contain more needaction, those for the member of a email channel. - 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']] + self._notify_record_by_inbox(message, rdata, msg_vals=msg_vals, **kwargs) + self._notify_record_by_email(message, rdata, msg_vals=msg_vals, **kwargs) - notifications = [] + return rdata + + def _notify_record_by_inbox(self, message, recipients_data, msg_vals=False, **kwargs): + """ Notification method: inbox. Do two main things + + * create an inbox notification for users; + * create channel / message link (channel_ids field of mail.message); + * send bus notifications; + + TDE/XDO TODO: flag rdata directly, with for example r['notif'] = 'ocn_client' and r['needaction']=False + and correctly override notify_recipients + """ + channel_ids = [r['id'] for r in recipients_data['channels']] + if channel_ids: + message.write({'channel_ids': [(6, 0, channel_ids)]}) + + inbox_pids = [r['id'] for r in recipients_data['partners'] if r['notif'] == 'inbox'] + if inbox_pids: + notif_create_values = [{ + 'mail_message_id': message.id, + 'res_partner_id': pid, + 'notification_type': 'inbox', + } for pid in inbox_pids] + self.env['mail.notification'].sudo().create(notif_create_values) + + bus_notifications = [] if inbox_pids or channel_ids: - message_values = False + message_format_values = False if inbox_pids: - message_values = message.message_format()[0] + message_format_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)]) + bus_notifications.append([(self._cr.dbname, 'ir.needaction', partner), dict(message_format_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 notifications: - self.env['bus.bus'].sudo().sendmany(notifications) - return True + bus_notifications += self.env['mail.channel'].sudo().browse(channel_ids)._channel_message_notifications(message, message_format_values) + if bus_notifications: + self.env['bus.bus'].sudo().sendmany(bus_notifications) - def _notify_record_by_email(self, message, partners_data, msg_vals=False, model_description=False, mail_auto_delete=True, send_after_commit=True): + def _notify_record_by_email(self, message, recipients_data, msg_vals=False, + model_description=False, mail_auto_delete=True, check_existing=False, + force_send=True, send_after_commit=True, + **kwargs): """ Method to send email linked to notified messages. + :param message: mail.message record to notify; - :param partners_data: partner to notify by email coming from _notify_compute_recipients - :param msg_vals: message creation values if available + :param recipients_data: see ``_notify_thread``; + :param msg_vals: see ``_notify_thread``; + + :param model_description: model description used in email notification process + (computed if not given); + :param mail_auto_delete: delete notification emails once sent; + :param check_existing: check for existing notifications to update based on + mailed recipient, otherwise create new notifications; + + :param force_send: send emails directly instead of using queue; :param send_after_commit: if force_send, tells whether to send emails after the transaction has been committed using a post-commit hook; - :param model_description: optional data used in notification process (see - notification templates); - :param mail_auto_delete: delete notification emails once sent; """ + partners_data = [r for r in recipients_data['partners'] if r['notif'] == 'email'] + if not partners_data: + return True + 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) # one query for display name recipients_groups_data = self._notify_classify_recipients(partners_data, model_name) if not recipients_groups_data: return True - - force_send = self.env.context.get('mail_notify_force_send', True) + force_send = self.env.context.get('mail_notify_force_send', force_send) template_values = self._notify_prepare_template_context(message, msg_vals, model_description=model_description) # 10 queries @@ -2062,7 +2089,6 @@ class MailThread(models.AbstractModel): _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 = { @@ -2080,6 +2106,7 @@ class MailThread(models.AbstractModel): emails = self.env['mail.mail'].sudo() # loop on groups (customer, portal, user, ... + model specific like group_sale_salesman) + notif_create_values = [] recipients_max = 50 for recipients_group_data in recipients_groups_data: # generate notification email content @@ -2093,7 +2120,8 @@ class MailThread(models.AbstractModel): else: mail_body = message.body mail_body = self._replace_local_links(mail_body) - # send email + + # create email 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'] @@ -2110,22 +2138,32 @@ class MailThread(models.AbstractModel): email = Mail.create(create_values) 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)) # 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, + tocreate_recipient_ids = list(recipient_ids) + if check_existing: + existing_notifications = self.env['mail.notification'].sudo().search([ + ('mail_message_id', '=', message.id), + ('notification_type', '=', 'email'), + ('res_partner_id', 'in', tocreate_recipient_ids) + ]) + if existing_notifications: + tocreate_recipient_ids = [rid for rid in recipient_ids if rid not in existing_notifications.mapped('res_partner_id.id')] + existing_notifications.write({ + 'notification_status': 'ready', + 'mail_id': email.id, + }) + notif_create_values += [{ + 'mail_message_id': message.id, + 'res_partner_id': recipient_id, + 'notification_type': 'email', 'mail_id': email.id, - 'is_read': True, # handle by email discards Inbox notification - 'email_status': 'ready', - }) + 'is_read': True, # discard Inbox notification + 'notification_status': 'ready', + } for recipient_id in tocreate_recipient_ids] emails |= email + if notif_create_values: + self.env['mail.notification'].sudo().create(notif_create_values) + # NOTE: # 1. for more than 50 followers, use the queue system # 2. do not send emails immediately if the registry is not loaded, @@ -2158,7 +2196,7 @@ class MailThread(models.AbstractModel): 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 + 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 @@ -2222,13 +2260,14 @@ class MailThread(models.AbstractModel): # get values from msg_vals or from message if msg_vals doen't exists pids = msg_vals.get('partner_ids', []) if msg_vals else msg_sudo.partner_ids.ids cids = msg_vals.get('channel_ids', []) if msg_vals else msg_sudo.channel_ids.ids + message_type = msg_vals.get('message_type') if msg_vals else msg_sudo.message_type subtype_id = msg_vals.get('subtype_id') if msg_vals else msg_sudo.subtype_id.id # is it possible to have record but no subtype_id ? recipient_data = { 'partners': [], 'channels': [], } - res = self.env['mail.followers']._get_recipient_data(self, subtype_id, pids, cids) + res = self.env['mail.followers']._get_recipient_data(self, message_type, subtype_id, pids, cids) if not res: return recipient_data @@ -2243,11 +2282,11 @@ class MailThread(models.AbstractModel): if notif == 'inbox': recipient_data['partners'].append(dict(pdata, notif=notif, type='user')) elif not pshare and notif: # has an user and is not shared, is therefore user - recipient_data['partners'].append(dict(pdata, notif='email', type='user')) + recipient_data['partners'].append(dict(pdata, notif=notif, type='user')) elif pshare and notif: # has an user but is shared, is therefore portal - recipient_data['partners'].append(dict(pdata, notif='email', type='portal')) + recipient_data['partners'].append(dict(pdata, notif=notif, type='portal')) else: # has no user, is therefore customer - recipient_data['partners'].append(dict(pdata, notif='email', type='customer')) + recipient_data['partners'].append(dict(pdata, notif=notif if notif else 'email', type='customer')) elif cid: recipient_data['channels'].append({'id': cid, 'notif': notif, 'type': ctype}) @@ -2514,9 +2553,6 @@ class MailThread(models.AbstractModel): 'email_to': False, 'recipient_ids': recipient_ids, } - @api.multi - def _notify_customize_recipients(self, message, msg_vals): - return {} # ------------------------------------------------------ # Followers API diff --git a/addons/mail/views/mail_message_views.xml b/addons/mail/views/mail_message_views.xml index 70657edd08c..14c48dc4738 100644 --- a/addons/mail/views/mail_message_views.xml +++ b/addons/mail/views/mail_message_views.xml @@ -75,8 +75,8 @@ - - + + diff --git a/addons/mail/wizard/invite.py b/addons/mail/wizard/invite.py index 5a899e1a7c6..5c7889b6a76 100644 --- a/addons/mail/wizard/invite.py +++ b/addons/mail/wizard/invite.py @@ -72,13 +72,13 @@ class Invite(models.TransientModel): 'no_auto_thread': True, 'add_sign': True, }) - partners_data = [{ - 'id': pid, - 'share': True, - 'notif': 'email', - 'type': 'customer', + recipients_data = {'partners': [{ + 'id': pid, + 'share': True, + 'notif': 'email', + 'type': 'customer', 'groups': [] - } for pid in new_partners.ids] - document._notify_record_by_email(message, partners_data, send_after_commit=False) + } for pid in new_partners.ids]} + document._notify_record_by_email(message, recipients_data, send_after_commit=False) message.unlink() return {'type': 'ir.actions.act_window_close'} diff --git a/addons/mail/wizard/mail_resend_cancel.py b/addons/mail/wizard/mail_resend_cancel.py index 277ed8b96db..f3bb8307754 100644 --- a/addons/mail/wizard/mail_resend_cancel.py +++ b/addons/mail/wizard/mail_resend_cancel.py @@ -4,7 +4,7 @@ from odoo import _, api, fields, models -class MailCancelResend(models.TransientModel): +class MailResendCancel(models.TransientModel): _name = 'mail.resend.cancel' _description = 'Dismiss notification for resend by model' @@ -26,7 +26,7 @@ class MailCancelResend(models.TransientModel): FROM mail_message_res_partner_needaction_rel notif JOIN mail_message mes ON notif.mail_message_id = mes.id - WHERE notif.email_status IN ('bounce', 'exception') + WHERE notif.notification_status IN ('bounce', 'exception') AND mes.model = %s AND mes.author_id = %s """, (wizard.model, author_id)) @@ -34,6 +34,6 @@ class MailCancelResend(models.TransientModel): notif_ids = [row[0] for row in res] messages_ids = list(set([row[1] for row in res])) if notif_ids: - self.env["mail.notification"].browse(notif_ids).sudo().write({'email_status': 'canceled'}) - self.env["mail.message"].browse(messages_ids)._notify_failure_update() + self.env["mail.notification"].browse(notif_ids).sudo().write({'notification_status': 'canceled'}) + self.env["mail.message"].browse(messages_ids)._notify_mail_failure_update() return {'type': 'ir.actions.act_window_close'} diff --git a/addons/mail/wizard/mail_resend_message.py b/addons/mail/wizard/mail_resend_message.py index e5e4c069df1..9c72944961b 100644 --- a/addons/mail/wizard/mail_resend_message.py +++ b/addons/mail/wizard/mail_resend_message.py @@ -28,16 +28,14 @@ class MailResendMessage(models.TransientModel): message_id = self._context.get('mail_message_to_resend') if message_id: mail_message_id = self.env['mail.message'].browse(message_id) - notification_ids = mail_message_id.notification_ids.filtered(lambda notif: notif.email_status in ('exception', 'bounce')) - partner_ids = [(0, 0, - { - "partner_id": notif.res_partner_id.id, - "name": notif.res_partner_id.name, - "email": notif.res_partner_id.email, - "resend": True, - "message": notif.format_failure_reason(), - } - ) for notif in notification_ids] + notification_ids = mail_message_id.notification_ids.filtered(lambda notif: notif.notification_type == 'email' and notif.notification_status in ('exception', 'bounce')) + partner_ids = [(0, 0, { + "partner_id": notif.res_partner_id.id, + "name": notif.res_partner_id.name, + "email": notif.res_partner_id.email, + "resend": True, + "message": notif.format_failure_reason(), + }) for notif in notification_ids] has_user = any([notif.res_partner_id.user_ids for notif in notification_ids]) if has_user: partner_readonly = not self.env['res.users'].check_access_rights('write', raise_exception=False) @@ -59,14 +57,14 @@ class MailResendMessage(models.TransientModel): "If a partner disappeared from partner list, we cancel the notification" to_cancel = wizard.partner_ids.filtered(lambda p: not p.resend).mapped("partner_id") to_send = wizard.partner_ids.filtered(lambda p: p.resend).mapped("partner_id") - notif_to_cancel = wizard.notification_ids.filtered(lambda notif: notif.res_partner_id in to_cancel and notif.email_status in ('exception', 'bounce')) - notif_to_cancel.sudo().write({'email_status': 'canceled'}) + notif_to_cancel = wizard.notification_ids.filtered(lambda notif: notif.notification_type == 'email' and notif.res_partner_id in to_cancel and notif.notification_status in ('exception', 'bounce')) + notif_to_cancel.sudo().write({'notification_status': 'canceled'}) if to_send: message = wizard.mail_message_id record = self.env[message.model].browse(message.res_id) if message.is_thread_message() else self.env['mail.thread'] email_partners_data = [] - for pid, cid, active, pshare, ctype, notif, groups in self.env['mail.followers']._get_recipient_data(None, False, pids=to_send.ids): + for pid, cid, active, pshare, ctype, notif, groups in self.env['mail.followers']._get_recipient_data(None, 'comment', False, pids=to_send.ids): if pid and notif == 'email' or not notif: pdata = {'id': pid, 'share': pshare, 'active': active, 'notif': 'email', 'groups': groups or []} if not pshare and notif: # has an user and is not shared, is therefore user @@ -76,17 +74,17 @@ class MailResendMessage(models.TransientModel): else: # has no user, is therefore customer email_partners_data.append(dict(pdata, type='customer')) - record._notify_record_by_email(message, email_partners_data, send_after_commit=False) + record._notify_record_by_email(message, {'partners': email_partners_data}, check_existing=True, send_after_commit=False) - self.mail_message_id._notify_failure_update() + self.mail_message_id._notify_mail_failure_update() return {'type': 'ir.actions.act_window_close'} @api.multi def cancel_mail_action(self): for wizard in self: for notif in wizard.notification_ids: - notif.filtered(lambda notif: notif.email_status in ('exception', 'bounce')).sudo().write({'email_status': 'canceled'}) - wizard.mail_message_id._notify_failure_update() + notif.filtered(lambda notif: notif.notification_type == 'email' and notif.notification_status in ('exception', 'bounce')).sudo().write({'notification_status': 'canceled'}) + wizard.mail_message_id._notify_mail_failure_update() return {'type': 'ir.actions.act_window_close'} diff --git a/addons/test_mail/tests/common.py b/addons/test_mail/tests/common.py index 7df3aa0c96a..9881d631977 100644 --- a/addons/test_mail/tests/common.py +++ b/addons/test_mail/tests/common.py @@ -86,13 +86,13 @@ class BaseFunctionalTest(common.SavepointCase): self.assertEqual(expected, real, 'Invalid number of notification for %s: %s instead of %s' % (partner.name, real, expected)) if partner_notif: - self.assertTrue(all(n.is_email == (notif_type == 'email') for n in partner_notif)) + self.assertTrue(all(n.notification_type == notif_type for n in partner_notif)) self.assertTrue(all(n.is_read == (notif_read == 'read') for n in partner_notif), 'Invalid read status for %s' % partner.name) # for simplification, limitate to single message asserts if hasattr(self, 'assertEmails') and len(new_messages) == 1: - self.assertEmails(new_messages.author_id, new_notifications.filtered(lambda n: n.is_email).mapped('res_partner_id')) + self.assertEmails(new_messages.author_id, new_notifications.filtered(lambda n: n.notification_type == 'email').mapped('res_partner_id')) def assertBusNotification(self, channels, message_items=None, init=True): """ Check for bus notifications. Basic check is about used channels. diff --git a/addons/test_mail/tests/test_mail_race.py b/addons/test_mail/tests/test_mail_race.py index 943a1b30922..310051a7d68 100644 --- a/addons/test_mail/tests/test_mail_race.py +++ b/addons/test_mail/tests/test_mail_race.py @@ -1,13 +1,14 @@ # -*- coding: utf-8 -*- # Part of Odoo. See LICENSE file for full copyright and licensing details. -from contextlib import closing import psycopg2 + from odoo import api from odoo.addons.test_mail.tests import common as mail_common from odoo.tests import common from odoo.tools import mute_logger + class TestMailRace(common.TransactionCase, mail_common.MockEmails): @mute_logger('odoo.addons.mail.models.mail_mail') @@ -26,15 +27,15 @@ class TestMailRace(common.TransactionCase, mail_common.MockEmails): 'subject': 'S', 'body': 'B', 'subtype_id': self.ref('mail.mt_comment'), - 'needaction_partner_ids': [(6, 0, [self.partner.id])], + 'notification_ids': [(0, 0, { + 'res_partner_id': self.partner.id, + 'mail_id': mail.id, + 'notification_type': 'email', + 'is_read': True, + 'notification_status': 'ready', + })], }) notif = self.env['mail.notification'].search([('res_partner_id', '=', self.partner.id)]) - notif.write({ - 'mail_id': mail.id, - 'is_email': True, - 'is_read': True, - 'email_status': 'ready', - }) # we need to commit transaction or cr will keep the lock on notif self.cr.commit() @@ -46,7 +47,7 @@ class TestMailRace(common.TransactionCase, mail_common.MockEmails): with this.registry.cursor() as cr, mute_logger('odoo.sql_db'): try: # try ro aquire lock (no wait) on notification (should fail) - cr.execute("SELECT email_status FROM mail_message_res_partner_needaction_rel WHERE id = %s FOR UPDATE NOWAIT", [notif.id]) + cr.execute("SELECT notification_status FROM mail_message_res_partner_needaction_rel WHERE id = %s FOR UPDATE NOWAIT", [notif.id]) except psycopg2.OperationalError: # record already locked by send, all good bounce_deferred.append(True) @@ -55,14 +56,14 @@ class TestMailRace(common.TransactionCase, mail_common.MockEmails): # Only here to simulate the initial use case # If the record is lock, this line would create a deadlock since we are in the same thread # In practice, the update will wait the end of the send() transaction and set the notif as bounce, as expeced - cr.execute("UPDATE mail_message_res_partner_needaction_rel SET email_status='bounce' WHERE id = %s", [notif.id]) + cr.execute("UPDATE mail_message_res_partner_needaction_rel SET notification_status='bounce' WHERE id = %s", [notif.id]) return message['Message-Id'] self.env['ir.mail_server']._patch_method('send_email', send_email) mail.send() self.assertTrue(bounce_deferred, "The bounce should have been deferred") - self.assertEqual(notif.email_status, 'sent') + self.assertEqual(notif.notification_status, 'sent') # some cleaning since we commited the cr notif.unlink() diff --git a/addons/test_mail/tests/test_mail_resend.py b/addons/test_mail/tests/test_mail_resend.py index e14a5a8784f..a2c94ae65e1 100644 --- a/addons/test_mail/tests/test_mail_resend.py +++ b/addons/test_mail/tests/test_mail_resend.py @@ -50,7 +50,7 @@ class TestMailResend(common.BaseFunctionalTest, common.MockEmails): def assertNotifStates(self, states, message): notif = self.env['mail.notification'].search([('mail_message_id', '=', message.id)], order="res_partner_id asc") - self.assertEquals(tuple(notif.mapped('email_status')), states) + self.assertEquals(tuple(notif.mapped('notification_status')), states) return notif def assertBusMessage(self, partners): diff --git a/addons/test_mail/tests/test_performance.py b/addons/test_mail/tests/test_performance.py index 882f1c48b13..666728d53dd 100644 --- a/addons/test_mail/tests/test_performance.py +++ b/addons/test_mail/tests/test_performance.py @@ -200,7 +200,7 @@ class TestAdvMailPerformance(BaseMailPerformance): 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__=55, emp=58): # com runbot: 55 - 58 // test_mail only: 55 - 58 + with self.assertQueryCount(__system__=52, emp=54): # com runbot: 52 - 54 // test_mail only: 52 - 54 record.write({ 'user_id': self.user_test.id, }) @@ -209,7 +209,7 @@ class TestAdvMailPerformance(BaseMailPerformance): @warmup def test_message_assignation_inbox(self): record = self.env['mail.test.track'].create({'name': 'Test'}) - with self.assertQueryCount(__system__=33, emp=39): # test_mail only: 33 - 39 + with self.assertQueryCount(__system__=31, emp=36): # test_mail only: 31 - 36 record.write({ 'user_id': self.user_test.id, }) @@ -253,7 +253,7 @@ class TestAdvMailPerformance(BaseMailPerformance): def test_message_post_one_email_notification(self): record = self.env['mail.test.simple'].create({'name': 'Test'}) - with self.assertQueryCount(__system__=47, emp=51): # com runbot: 47 - 51 // test_mail only: 47 - 51 + with self.assertQueryCount(__system__=44, emp=47): # com runbot: 44 - 47 // test_mail only: 44 - 47 record.message_post( body='

Test Post Performances with an email ping

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

Test Post Performances with an inbox ping

', partner_ids=self.user_test.partner_id.ids, @@ -373,7 +373,7 @@ class TestHeavyMailPerformance(BaseMailPerformance): self.umbrella.message_subscribe(self.user_portal.partner_id.ids) record = self.umbrella.with_user(self.env.user) - with self.assertQueryCount(__system__=78, emp=82): # com runbot: 78 - 82 // test_mail only: 78 - 82 + with self.assertQueryCount(__system__=82, emp=85): # com runbot: 82 - 85 // test_mail only: 82 - 85 record.message_post( body='

Test Post Performances

', message_type='comment', @@ -390,7 +390,7 @@ class TestHeavyMailPerformance(BaseMailPerformance): record = self.umbrella.with_user(self.env.user) template_id = self.env.ref('test_mail.mail_test_tpl').id - with self.assertQueryCount(__system__=93, emp=99): # com runbot: 93 - 99 // test_mail only: 93 - 99 + with self.assertQueryCount(__system__=98, emp=103): # com runbot: 98 - 103 // test_mail only: 98 - 103 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) @@ -459,7 +459,7 @@ class TestHeavyMailPerformance(BaseMailPerformance): 'user_id': self.env.uid, }) self.assertEqual(rec.message_partner_ids, self.partners | self.env.user.partner_id) - with self.assertQueryCount(__system__=54, emp=57): # com runbot: 54 - 57 // test_mail only: 54 - 57 + with self.assertQueryCount(__system__=51, emp=53): # com runbot: 51 - 53 // test_mail only: 51 - 53 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 @@ -479,7 +479,7 @@ class TestHeavyMailPerformance(BaseMailPerformance): customer_id = self.customer.id user_id = self.user_portal.id - with self.assertQueryCount(__system__=145, emp=149): # com runbot: 145 - 149 // test_mail only: 145 - 149 + with self.assertQueryCount(__system__=146, emp=149): # com runbot: 146 - 149 // test_mail only: 146 - 149 rec = self.env['mail.test.full'].create({ 'name': 'Test', 'umbrella_id': umbrella_id, @@ -506,7 +506,7 @@ class TestHeavyMailPerformance(BaseMailPerformance): }) 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__=95, emp=100): # com runbot: 95 - 100 // test_mail only: 95 - 100 + with self.assertQueryCount(__system__=99, emp=103): # com runbot: 99 -103 // test_mail only: 99 - 103 rec.write({ 'name': 'Test2', 'umbrella_id': self.umbrella.id, @@ -542,7 +542,7 @@ class TestHeavyMailPerformance(BaseMailPerformance): }) self.assertEqual(rec.message_partner_ids, self.user_portal.partner_id | self.env.user.partner_id) - with self.assertQueryCount(__system__=100, emp=105): # test_mail only: 100 - 105 + with self.assertQueryCount(__system__=104, emp=108): # test_mail only: 104 - 108 rec.write({ 'name': 'Test2', 'umbrella_id': umbrella_id, @@ -704,14 +704,14 @@ class TestMailPerformancePost(BaseMailPerformance): partner_ids = [self.user_inbox.partner_id.id, self.user_email.partner_id.id, self.partner.id] channel_ids = [self.channel_inbox.id, self.channel_email.id] record = self.record.with_user(self.env.user) - attachements = [ # not linear on number of attachements + attachements = [ # not linear on number of attachements ('attach tuple 1', "attachement tupple content 1"), ('attach tuple 2', "attachement tupple content 2", {'cid': 'cid1'}), ('attach tuple 3', "attachement tupple content 3", {'cid': 'cid2'}), ] - self.attachements = self.env['ir.attachment'].with_user(self.env.user).create(self.vals) #-> 163-> 165 query + self.attachements = self.env['ir.attachment'].with_user(self.env.user).create(self.vals) attachement_ids = self.attachements.ids - with self.assertQueryCount(emp=175): # com runbot 154 // test_mail only: 133 + with self.assertQueryCount(emp=175): # com runbot 154 // test_mail only: 132 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 6ddb0cb06a6..da71c1b3194 100644 --- a/addons/website_blog/models/website_blog.py +++ b/addons/website_blog/models/website_blog.py @@ -259,14 +259,13 @@ class BlogPost(models.Model): return groups @api.multi - def _notify_customize_recipients(self, message, msg_vals): + def _notify_record_by_inbox(self, message, recipients_data, msg_vals=False, **kwargs): """ 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 super(BlogPost, self)._notify_customize_recipients(message, msg_vals) + if msg_vals.get('message_type', message.message_type) == 'comment': + return + return super(BlogPost, self)._notify_record_by_inbox(message, recipients_data, msg_vals=msg_vals, **kwargs) 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 e668fc173ee..5a98dc12a41 100644 --- a/addons/website_forum/models/forum.py +++ b/addons/website_forum/models/forum.py @@ -830,14 +830,13 @@ 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): + def _notify_record_by_inbox(self, message, recipients_data, msg_vals=False, **kwargs): """ 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 super(Post, self)._notify_customize_recipients(message, msg_vals) + if msg_vals.get('message_type', message.message_type) == 'comment': + return + return super(Post, self)._notify_record_by_inbox(message, recipients_data, msg_vals=msg_vals, **kwargs) class PostReason(models.Model): From bdebcab0cea467e070c1936fed16173247abafdd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Thu, 23 May 2019 10:09:35 +0000 Subject: [PATCH 2/8] [REF] sms: refactor message_post using SMS notifications MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Purpose of this commit is to better include SMS notifications when posting a message. SMS is now just another way of notifying people along with Inbox and email. Following recent mail merge improving notification mechanism [1] we have to define a _notify_record_by_sms method on mail.thread. When a message_post is done using message_type being ``sms`` notification type of customers is set to sms. Customers can be computed on model (generally based on partner_id field) or directly set usign partner°ids. Notification model is updated to store this information directly inside the notification itself. An new ``_message_sms`` helper method is introduced in SMS module allowing to send messages using sms type and notification with a reduced parameters number. It is just a shortcut to message_post, easier to use. Either it computes default recipients on the record set, either it is based on given partners and numbers to notify. The following use cases are notably supported * default computation: find customer, notify by sms; * force recipients to notify by sms (partner_ids); * give a set of numbers to notify by sms (sms_nubmers), not necessarily linked to existing partners; * force number / customer relationship independently of mobile number defined on customer (for example when sending an SMS directly from a mobile field on a lead linked to a customer); Tests are updated accordingly. Performance tests are added in order to have some insights on queries generated when sending SMS, like already done for mail.thread alone. Related to task 1922163 Linked to PR #33510 [1] see be2795513644f09f6b443f462bb67cc5a0762e8d: performance and notification code improvements Co-Authored-By: Thibault Delavallee Co-Authored-By: Pierre Rousseau --- addons/calendar_sms/models/calendar.py | 5 +- addons/mail/models/mail_notification.py | 2 +- .../tools/phone_validation.py | 34 +- addons/sms/__manifest__.py | 3 + addons/sms/data/ir_cron_data.xml | 14 + addons/sms/models/__init__.py | 4 + addons/sms/models/mail_followers.py | 24 ++ addons/sms/models/mail_message.py | 13 + addons/sms/models/mail_notification.py | 18 ++ addons/sms/models/mail_thread.py | 238 ++++++++++++-- addons/sms/models/res_partner.py | 2 +- addons/sms/models/sms_sms.py | 137 ++++++++ addons/sms/security/ir.model.access.csv | 3 + addons/sms/tests/common.py | 93 +++++- addons/sms/views/sms_sms_views.xml | 66 ++++ addons/sms/wizard/sms_composer.py | 49 +-- addons/test_mail/tests/common.py | 4 +- addons/test_mail/tests/test_performance.py | 4 +- addons/test_mail_full/__manifest__.py | 2 +- .../test_mail_full/models/test_mail_models.py | 18 +- .../security/ir.model.access.csv | 2 + addons/test_mail_full/tests/__init__.py | 2 + addons/test_mail_full/tests/common.py | 8 + .../test_mail_full/tests/test_sms_composer.py | 22 +- .../tests/test_sms_performance.py | 88 ++++++ addons/test_mail_full/tests/test_sms_post.py | 299 ++++++++++++++++-- addons/test_mail_full/tests/test_sms_sms.py | 59 ++++ 27 files changed, 1057 insertions(+), 156 deletions(-) create mode 100644 addons/sms/data/ir_cron_data.xml create mode 100644 addons/sms/models/mail_followers.py create mode 100644 addons/sms/models/mail_message.py create mode 100644 addons/sms/models/mail_notification.py create mode 100644 addons/sms/models/sms_sms.py create mode 100644 addons/sms/security/ir.model.access.csv create mode 100644 addons/sms/views/sms_sms_views.xml create mode 100644 addons/test_mail_full/tests/test_sms_performance.py create mode 100644 addons/test_mail_full/tests/test_sms_sms.py diff --git a/addons/calendar_sms/models/calendar.py b/addons/calendar_sms/models/calendar.py index 65cd1a8cad5..0ad5bb3fb31 100644 --- a/addons/calendar_sms/models/calendar.py +++ b/addons/calendar_sms/models/calendar.py @@ -11,7 +11,7 @@ _logger = logging.getLogger(__name__) class CalendarEvent(models.Model): _inherit = 'calendar.event' - def _get_default_sms_recipients(self): + def _sms_get_default_partners(self): """ Method overriden from mail.thread (defined in the sms module). SMS text messages will be sent to attendees that haven't declined the event(s). """ @@ -21,8 +21,7 @@ class CalendarEvent(models.Model): """ Send an SMS text reminder to attendees that haven't declined the event """ for event in self: sms_msg = _("Event reminder: %s on %s.") % (event.name, event.start_datetime or event.start_date) - note_msg = _('SMS text message reminder sent !') - event.message_post_send_sms(sms_msg, note_msg=note_msg) + event._message_sms(sms_msg) class CalendarAlarm(models.Model): diff --git a/addons/mail/models/mail_notification.py b/addons/mail/models/mail_notification.py index 5cf42a2dc72..571023450d9 100644 --- a/addons/mail/models/mail_notification.py +++ b/addons/mail/models/mail_notification.py @@ -42,7 +42,7 @@ class Notification(models.Model): _sql_constraints = [ # email notification;: partner is required ('notification_partner_required', - "CHECK(notification_type in ('email', 'inbox') AND res_partner_id IS NOT NULL)", + "CHECK(notification_type NOT IN ('email', 'inbox') OR res_partner_id IS NOT NULL)", 'Customer is required for inbox / email notification'), ] diff --git a/addons/phone_validation/tools/phone_validation.py b/addons/phone_validation/tools/phone_validation.py index 29828ffd739..e1df861ac8c 100644 --- a/addons/phone_validation/tools/phone_validation.py +++ b/addons/phone_validation/tools/phone_validation.py @@ -77,51 +77,43 @@ except ImportError: def phone_sanitize_numbers(numbers, country_code, country_phone_code, force_format='E164'): - valid, invalid, void_count = [], [], 0 + result = dict.fromkeys(numbers, False) for number in numbers: if not number: - void_count += 1 + result[number] = {'sanitized': False, 'code': 'empty', 'msg': False} continue try: sanitized = phone_format( number, country_code, country_phone_code, force_format=force_format, raise_exception=True) except Exception as e: - invalid.append(number) + result[number] = {'sanitized': False, 'code': 'invalid', 'msg': e} else: - valid.append(sanitized) - return valid, invalid, void_count + result[number] = {'sanitized': sanitized, 'code': False, 'msg': False} + return result -def phone_sanitize_numbers_w_record(numbers, country_code, country_phone_code, record, record_country_fname='country_id', force_format='E164'): - if not country_code or not country_phone_code: - country = False - if record and record_country_fname in record and record[record_country_fname]: +def phone_sanitize_numbers_w_record(numbers, record, country=False, record_country_fname='country_id', force_format='E164'): + if not country: + if record and hasattr(record, record_country_fname) and record[record_country_fname]: country = record[record_country_fname] elif record: country = record.env.company.country_id - if country: - country_code = country_code if country_code else country.code - country_phone_code = country_phone_code if country_phone_code else country.phone_code + country_code = country.code if country else None + country_phone_code = country.phone_code if country else None return phone_sanitize_numbers(numbers, country_code, country_phone_code, force_format=force_format) -def phone_sanitize_numbers_string_w_record(numbers_str, country_code, country_phone_code, record, record_country_fname='country_id', force_format='E164'): +def phone_sanitize_numbers_string_w_record(numbers_str, record, country=False, record_country_fname='country_id', force_format='E164'): found_numbers = [number.strip() for number in numbers_str.split(',')] - return phone_sanitize_numbers_w_record(found_numbers, country_code, country_phone_code, record, record_country_fname, force_format=force_format) + return phone_sanitize_numbers_w_record(found_numbers, record, country=country, record_country_fname=record_country_fname, force_format=force_format) def phone_get_sanitized_records_number(records, number_fname='mobile', country_fname='country_id', force_format='E164'): res = dict.fromkeys(records.ids, False) for record in records: number = record[number_fname] - valid, invalid, void_count = phone_sanitize_numbers_w_record([number], None, None, records, country_fname,force_format=force_format) - if valid: - res[record.id] = valid[0] - elif void_count: - res[record.id] = False - else: - res[record.id] = False + res[record.id] = phone_sanitize_numbers_w_record([number], records, record_country_fname=country_fname,force_format=force_format)[number]['sanitized'] return res diff --git a/addons/sms/__manifest__.py b/addons/sms/__manifest__.py index 4ef5721be8f..dad1ac221da 100644 --- a/addons/sms/__manifest__.py +++ b/addons/sms/__manifest__.py @@ -12,10 +12,13 @@ The service is provided by the In App Purchase Odoo platform. """, 'depends': ['base', 'iap', 'mail', 'phone_validation'], 'data': [ + 'data/ir_cron_data.xml', 'wizard/sms_composer_views.xml', 'views/res_config_settings_views.xml', 'views/res_partner_views.xml', 'views/assets.xml', + 'views/sms_sms_views.xml', + 'security/ir.model.access.csv', ], 'qweb': [ 'static/src/xml/sms_widget.xml', diff --git a/addons/sms/data/ir_cron_data.xml b/addons/sms/data/ir_cron_data.xml new file mode 100644 index 00000000000..e8a5d771dd5 --- /dev/null +++ b/addons/sms/data/ir_cron_data.xml @@ -0,0 +1,14 @@ + + + + SMS: SMS Queue Manager + + code + model._process_queue() + + 1 + hours + -1 + + + \ No newline at end of file diff --git a/addons/sms/models/__init__.py b/addons/sms/models/__init__.py index 7a75a7a5f6d..b855b4b7955 100644 --- a/addons/sms/models/__init__.py +++ b/addons/sms/models/__init__.py @@ -1,6 +1,10 @@ # -*- coding: utf-8 -*- # Part of Odoo. See LICENSE file for full copyright and licensing details. +from . import mail_followers +from . import mail_message +from . import mail_notification from . import mail_thread from . import res_partner from . import sms_api +from . import sms_sms diff --git a/addons/sms/models/mail_followers.py b/addons/sms/models/mail_followers.py new file mode 100644 index 00000000000..76be79b677b --- /dev/null +++ b/addons/sms/models/mail_followers.py @@ -0,0 +1,24 @@ +# -*- coding: utf-8 -*- +# Part of Odoo. See LICENSE file for full copyright and licensing details. + +from odoo import api, fields, models + + +class Followers(models.Model): + _inherit = ['mail.followers'] + + def _get_recipient_data(self, records, message_type, subtype_id, pids=None, cids=None): + if message_type == 'sms': + if pids is None: + sms_pids = records._sms_get_default_partners().ids + else: + sms_pids = pids + res = super(Followers, self)._get_recipient_data(records, message_type, subtype_id, pids=pids, cids=cids) + new_res = [] + for pid, cid, pactive, pshare, ctype, notif, groups in res: + if pid and pid in sms_pids: + notif = 'sms' + new_res.append((pid, cid, pactive, pshare, ctype, notif, groups)) + return new_res + else: + return super(Followers, self)._get_recipient_data(records, message_type, subtype_id, pids=pids, cids=cids) diff --git a/addons/sms/models/mail_message.py b/addons/sms/models/mail_message.py new file mode 100644 index 00000000000..fbd0465f746 --- /dev/null +++ b/addons/sms/models/mail_message.py @@ -0,0 +1,13 @@ +# -*- coding: utf-8 -*- +# Part of Odoo. See LICENSE file for full copyright and licensing details. + +from odoo import api, fields, models + + +class MailMessage(models.Model): + """ Override MailMessage class in order to add a new type: SMS messages. + Those messages comes with their own notification method, using SMS + gateway. """ + _inherit = 'mail.message' + + message_type = fields.Selection(selection_add=[('sms', 'SMS')]) diff --git a/addons/sms/models/mail_notification.py b/addons/sms/models/mail_notification.py new file mode 100644 index 00000000000..c45981ea0b0 --- /dev/null +++ b/addons/sms/models/mail_notification.py @@ -0,0 +1,18 @@ +# -*- coding: utf-8 -*- + +from odoo import api, fields, models +from odoo.tools.translate import _ + + +class Notification(models.Model): + _inherit = 'mail.notification' + + notification_type = fields.Selection(selection_add=[('sms', 'SMS')]) + sms_id = fields.Many2one('sms.sms', string='SMS', index=True, ondelete='set null') + sms_number = fields.Char('SMS Number') + failure_type = fields.Selection(selection_add=[ + ('sms_number_missing', 'Missing Number'), + ('sms_number_format', 'Wrong Number Format'), + ('sms_credit', 'Insufficient Credit'), + ('sms_server', 'Server Error')] + ) diff --git a/addons/sms/models/mail_thread.py b/addons/sms/models/mail_thread.py index a439487e4e9..fd47be27a5d 100644 --- a/addons/sms/models/mail_thread.py +++ b/addons/sms/models/mail_thread.py @@ -3,9 +3,9 @@ import logging -from odoo import models, _ - -from odoo.addons.iap.models.iap import InsufficientCreditError +from odoo import api, models +from odoo.addons.phone_validation.tools import phone_validation +from odoo.tools import html2plaintext _logger = logging.getLogger(__name__) @@ -13,7 +13,7 @@ _logger = logging.getLogger(__name__) class MailThread(models.AbstractModel): _inherit = 'mail.thread' - def _get_default_sms_recipients(self): + def _sms_get_default_partners(self): """ This method will likely need to be overriden by inherited models. :returns partners: recordset of res.partner """ @@ -24,36 +24,210 @@ class MailThread(models.AbstractModel): partners |= self.mapped('partner_ids') return partners - def message_post_send_sms(self, sms_message, numbers=None, partners=None, note_msg=None, log_error=False): - """ Send an SMS text message and post an internal note in the chatter if successfull - :param sms_message: plaintext message to send by sms - :param partners: the numbers to send to, if none are given it will take those - from partners or _get_default_sms_recipients - :param partners: the recipients partners, if none are given it will take those - from _get_default_sms_recipients, this argument - is ignored if numbers is defined - :param note_msg: message to log in the chatter, if none is given a default one - containing the sms_message is logged + def _sms_get_number_fields(self): + """ This method returns the fields to use to find the number to use to + send an SMS on a record. """ + return ['mobile'] + + def _sms_get_recipients_info(self, force_field=False): + """" Get SMS recipient information on current record set. This method + checks for numbers and sanitation in order to centralize computation. + + Example of use cases + + * click on a field -> number is actually forced from field, find customer + linked to record, force its number to field or fallback on customer fields; + * contact -> find numbers from all possible phone fields on record, find + customer, force its number to found field number or fallback on customer fields; + + :return dict: record.id: { + 'partner': a res.partner recordset that is the customer (void or singleton); + 'sanitized': sanitized number to use (coming from record's field or partner's mobile + or phone). Set to False is number impossible to parse and format; + 'number': original number before sanitation; + } for each record in self """ - if not numbers: - if not partners: - partners = self._get_default_sms_recipients() + result = dict.fromkeys(self.ids, False) + number_fields = self._sms_get_number_fields() + for record in self: + tocheck_fields = [force_field] if force_field else number_fields + all_numbers = [record[fname] for fname in tocheck_fields if fname in record] + all_partners = record._sms_get_default_partners() - # Collect numbers, we will consider the message to be sent if at least one number can be found - numbers = list(set([i.mobile for i in partners if i.mobile])) + valid_number = False + for fname in [f for f in tocheck_fields if f in record]: + valid_number = phone_validation.phone_get_sanitized_record_number(record, number_fname=fname) + if valid_number: + break - if numbers: - try: - self.env['sms.api']._send_sms(numbers, sms_message) - mail_message = note_msg or _('SMS message sent: %s') % sms_message + if valid_number: + result[record.id] = { + 'partner': all_partners[0] if all_partners else self.env['res.partner'], + 'sanitized': valid_number, 'number': valid_number, + } + elif all_partners: + partner_number, partner = False, self.env['res.partner'] + for partner in all_partners: + partner_number = partner.mobile or partner.phone + if partner_number: + partner_number = phone_validation.phone_sanitize_numbers_string_w_record(partner_number, record)[partner_number]['sanitized'] + if partner_number: + break - except InsufficientCreditError as e: - if not log_error: - raise e - mail_message = _('Insufficient credit, unable to send SMS message: %s') % sms_message - else: - mail_message = _('No mobile number defined, unable to send SMS message: %s') % sms_message + if partner_number: + result[record.id] = {'partner': partner, 'sanitized': partner_number, 'number': partner_number} + else: + result[record.id] = {'partner': partner, 'sanitized': False, 'number': partner.mobile or partner.phone} + elif all_numbers: + result[record.id] = {'partner': self.env['res.partner'], 'sanitized': False, 'number': all_numbers[0]} + else: + result[record.id] = {'partner': self.env['res.partner'], 'sanitized': False, 'number': False} + return result - for thread in self: - thread.message_post(body=mail_message) - return False + def _message_sms(self, body, subtype_id=False, partner_ids=False, number_field=False, + sms_numbers=None, sms_pid_to_number=None, **kwargs): + """ Main method to post a message on a record using SMS-based notification + method. + + :param body: content of SMS; + :param subtype_id: mail.message.subtype used in mail.message associated + to the sms notification process; + :param partner_ids: if set is a record set of partners to notify; + :param number_field: if set is a name of field to use on current record + to compute a number to notify; + :param sms_numbers: see ``_notify_record_by_sms``; + :param sms_pid_to_number: see ``_notify_record_by_sms``; + """ + self.ensure_one() + sms_pid_to_number = sms_pid_to_number if sms_pid_to_number is not None else {} + + if number_field or (partner_ids is False and sms_numbers is None): + info = self._sms_get_recipients_info(force_field=number_field)[self.id] + info_partner_ids = info['partner'].ids if info['partner'] else False + info_number = info['sanitized'] if info['sanitized'] else info['number'] + if info_partner_ids and info_number: + sms_pid_to_number[info_partner_ids[0]] = info_number + if info_partner_ids: + partner_ids = info_partner_ids + (partner_ids or []) + if info_number and not info_partner_ids: + sms_numbers = [info_number] + (sms_numbers or []) + + if subtype_id is False: + subtype_id = self.env['ir.model.data'].xmlid_to_res_id('mail.mt_comment') + + return self.message_post( + body=body, partner_ids=partner_ids or [], # TDE FIXME: temp fix otherwise crash mail_thread.py + message_type='sms', subtype_id=subtype_id, + sms_numbers=sms_numbers, sms_pid_to_number=sms_pid_to_number, + **kwargs + ) + + @api.multi + def _notify_thread(self, message, msg_vals=False, **kwargs): + recipients_data = super(MailThread, self)._notify_thread(message, msg_vals=msg_vals, **kwargs) + self._notify_record_by_sms(message, recipients_data, msg_vals=msg_vals, **kwargs) + return recipients_data + + @api.multi + def _notify_record_by_sms(self, message, recipients_data, msg_vals=False, + sms_numbers=None, sms_pid_to_number=None, + check_existing=False, put_in_queue=False, **kwargs): + """ Notification method: by SMS. + + :param message: mail.message record to notify; + :param recipients_data: see ``_notify_thread``; + :param msg_vals: see ``_notify_thread``; + + :param sms_numbers: additional numbers to notify in addition to partners + and classic recipients; + :param pid_to_number: force a number to notify for a given partner ID + instead of taking its mobile / phone number; + :param check_existing: check for existing notifications to update based on + mailed recipient, otherwise create new notifications; + :param put_in_queue: use cron to send queued SMS instead of sending them + directly; + """ + sms_pid_to_number = sms_pid_to_number if sms_pid_to_number is not None else {} + sms_numbers = sms_numbers if sms_numbers is not None else [] + sms_create_vals = [] + sms_all = self.env['sms.sms'].sudo() + + # pre-compute SMS data + body = msg_vals['body'] if msg_vals and msg_vals.get('body') else message.body + sms_base_vals = { + 'body': html2plaintext(body).rstrip('\n'), + 'mail_message_id': message.id, + 'state': 'outgoing', + } + + # notify from computed recipients_data (followers, specific recipients) + partners_data = [r for r in recipients_data['partners'] if r['notif'] == 'sms'] + partner_ids = [r['id'] for r in partners_data] + if partner_ids: + for partner in self.env['res.partner'].sudo().browse(partner_ids): + number = sms_pid_to_number.get(partner.id) or partner.mobile or partner.phone + sanitize_res = phone_validation.phone_sanitize_numbers_string_w_record(number, partner)[number] + number = sanitize_res['sanitized'] or number + sms_create_vals.append(dict( + sms_base_vals, + partner_id=partner.id, + number=number + )) + + # notify from additional numbers + if sms_numbers: + sanitized = phone_validation.phone_sanitize_numbers_w_record(sms_numbers, self) + tocreate_numbers = [ + value['sanitized'] or original + for original, value in sanitized.items() + if value['code'] != 'empty' + ] + sms_create_vals += [dict(sms_base_vals, partner_id=False, number=n) for n in tocreate_numbers] + + # create sms and notification + existing_pids, existing_numbers = [], [] + if sms_create_vals: + sms_all |= self.env['sms.sms'].sudo().create(sms_create_vals) + + if check_existing: + existing = self.env['mail.notification'].sudo().search([ + '|', ('res_partner_id', 'in', partner_ids), + '&', ('res_partner_id', '=', False), ('sms_number', 'in', sms_numbers), + ('notification_type', '=', 'sms'), + ('mail_message_id', '=', message.id) + ]) + for n in existing: + if n.res_partner_id.id in partner_ids and n.mail_message_id == message: + existing_pids.append(n.res_partner_id.id) + if not n.res_partner_id and n.sms_number in sms_numbers and n.mail_message_id == message: + existing_numbers.append(n.sms_number) + + notif_create_values = [{ + 'mail_message_id': message.id, + 'res_partner_id': sms.partner_id.id, + 'sms_number': sms.number, + 'notification_type': 'sms', + 'sms_id': sms.id, + 'is_read': True, # discard Inbox notification + 'notification_status': 'ready', + } for sms in sms_all if (sms.partner_id and sms.partner_id.id not in existing_pids) or (not sms.partner_id and sms.number not in existing_numbers)] + if notif_create_values: + self.env['mail.notification'].sudo().create(notif_create_values) + + if existing_pids or existing_numbers: + for sms in sms_all: + notif = next((n for n in existing if + (n.res_partner_id.id in existing_pids and n.res_partner_id.id == sms.partner_id.id) or + (not n.res_partner_id and n.sms_number in existing_numbers and n.sms_number == sms.number)), False) + if notif: + notif.write({ + 'notification_type': 'sms', + 'notification_status': 'ready', + 'sms_id': sms.id, + 'sms_number': sms.number, + }) + + if sms_all and not put_in_queue: + sms_all.send(auto_commit=False, raise_exception=False) + + return True diff --git a/addons/sms/models/res_partner.py b/addons/sms/models/res_partner.py index 6d85ef2fe06..6cc6734cc5c 100644 --- a/addons/sms/models/res_partner.py +++ b/addons/sms/models/res_partner.py @@ -7,7 +7,7 @@ from odoo import models class ResPartner(models.Model): _inherit = 'res.partner' - def _get_default_sms_recipients(self): + def _sms_get_default_partners(self): """ Override of mail.thread method. SMS recipients on partners are the partners themselves. """ diff --git a/addons/sms/models/sms_sms.py b/addons/sms/models/sms_sms.py new file mode 100644 index 00000000000..699f4efa5f5 --- /dev/null +++ b/addons/sms/models/sms_sms.py @@ -0,0 +1,137 @@ +# -*- coding: utf-8 -*- +# Part of Odoo. See LICENSE file for full copyright and licensing details. + +import logging +import threading + +from odoo import api, fields, models, tools + +_logger = logging.getLogger(__name__) + + +class SmsSms(models.Model): + _name = 'sms.sms' + _description = 'Outgoing SMS' + _rec_name = 'number' + + number = fields.Char('Number', required=True) + body = fields.Text() + partner_id = fields.Many2one('res.partner', 'Customer') + mail_message_id = fields.Many2one('mail.message', index=True) + state = fields.Selection([ + ('outgoing', 'In Queue'), + ('sent', 'Sent'), + ('error', 'Error'), + ('canceled', 'Canceled') + ], 'SMS Status', readonly=True, copy=False, default='outgoing', required=True) + error_code = fields.Selection([ + ('sms_number_missing', 'Missing Number'), + ('sms_number_format', 'Wrong Number Format'), + ('sms_credit', 'Insufficient Credit'), + ('sms_server', 'Server Error') + ]) + + @api.multi + def send(self, delete_all=False, auto_commit=False, raise_exception=False): + """ Main API method to send SMS. + + :param delete_all: delete all SMS (sent or not); otherwise delete only + sent SMS; + :param auto_commit: commit after each batch of SMS; + :param raise_exception: raise if there is an issue contacting IAP; + """ + for batch_ids in self._split_batch(): + self.browse(batch_ids)._send(delete_all=delete_all, raise_exception=raise_exception) + if auto_commit is True: + self._cr.commit() + + @api.model + def _process_queue(self, ids=None): + """ Send immediately queued messages, committing after each message is sent. + This is not transactional and should not be called during another transaction! + + :param list ids: optional list of emails ids to send. If passed no search + is performed, and these ids are used instead. + """ + domain = [('state', '=', 'outgoing')] + + filtered_ids = self.search(domain, limit=10000).ids # TDE note: arbitrary limit we might have to update + if ids: + ids = list(set(filtered_ids) & set(ids)) + else: + ids = filtered_ids + ids.sort() + + res = None + try: + # auto-commit except in testing mode + auto_commit = not getattr(threading.currentThread(), 'testing', False) + res = self.browse(ids).send(delete_all=False, auto_commit=auto_commit, raise_exception=False) + except Exception: + _logger.exception("Failed processing SMS queue") + return res + + def _split_batch(self): + batch_size = int(self.env['ir.config_parameter'].sudo().get_param('sms.session.batch.size', 10)) + for sms_batch in tools.split_every(batch_size, self.ids): + yield sms_batch + + @api.multi + def _send(self, delete_all=False, raise_exception=False): + """ This method tries to send SMS after checking the number (presence and + formatting). """ + iap_data = [{ + 'res_id': record.id, + 'number': record.number, + 'content': record.body, + } for record in self] + + try: + iap_results = self.env['sms.api']._send_sms_batch(iap_data) + except Exception as e: + _logger.info('Sent batch %s SMS: %s: failed with exception %s', len(self.ids), self.ids, e) + if raise_exception: + raise + self._postprocess_sent_sms([{'res_id': sms.id, 'state': 'server_error'} for sms in self], delete_all=delete_all) + else: + _logger.info('Send batch %s SMS: %s: gave %s', len(self.ids), self.ids, iap_results) + self._postprocess_sent_sms(iap_results, delete_all=delete_all) + + def _postprocess_sent_sms(self, iap_results, failure_reason=None, delete_all=False): + sms_to_notif_status = { + 'success': False, 'insufficient_credit': 'sms_credit', + 'wrong_format_number': 'sms_number_format', 'server_error': 'sms_server'} + if delete_all: + todelete_sms_ids = [item['res_id'] for item in iap_results] + else: + todelete_sms_ids = [item['res_id'] for item in iap_results if item['state'] == 'success'] + + for state in sms_to_notif_status.keys(): + sms_ids = [item['res_id'] for item in iap_results if item['state'] == state] + if sms_ids: + if not delete_all and state != 'success': + self.env['sms.sms'].sudo().browse(sms_ids).write({ + 'state': 'error', + 'error_code': sms_to_notif_status[state], + }) + notifications = self.env['mail.notification'].sudo().search([ + ('notification_type', '=', 'sms'), + ('sms_id', 'in', sms_ids), + ('notification_status', 'not in', ('sent', 'canceled'))] + ) + if notifications: + notifications.write({ + 'notification_status': 'sent' if state == 'success' else 'exception', + 'failure_type': sms_to_notif_status[state], + 'failure_reason': failure_reason if failure_reason else False, + }) + + if todelete_sms_ids: + self.browse(todelete_sms_ids).sudo().unlink() + + @api.multi + def cancel(self): + self.write({ + 'state': 'canceled', + 'error_code': False + }) diff --git a/addons/sms/security/ir.model.access.csv b/addons/sms/security/ir.model.access.csv new file mode 100644 index 00000000000..42c4d624aaa --- /dev/null +++ b/addons/sms/security/ir.model.access.csv @@ -0,0 +1,3 @@ +id,name,model_id:id,group_id:id,perm_read,perm_write,perm_create,perm_unlink +access_sms_sms_all,access.sms.sms.all,model_sms_sms,,0,0,0,0 +access_sms_sms_system,access.sms.sms.system,model_sms_sms,base.group_system,1,1,1,1 diff --git a/addons/sms/tests/common.py b/addons/sms/tests/common.py index 25ca2dfdf7f..7c0c3b5d22a 100644 --- a/addons/sms/tests/common.py +++ b/addons/sms/tests/common.py @@ -3,7 +3,8 @@ from contextlib import contextmanager from unittest.mock import patch -from odoo import exceptions +from odoo import exceptions, tools +from odoo.addons.phone_validation.tools import phone_validation from odoo.tests import common from odoo.addons.sms.models.sms_api import SmsApi @@ -54,18 +55,100 @@ class MockSMS(common.BaseCase): finally: pass + def _clear_sms_sent(self): + self._sms = [] + def assertSMSSent(self, numbers, content): """ Check sent SMS. Order is not checked. Each number should have received - the same content. Usefull to check batch sending. + the same content. Useful to check batch sending. :param numbers: list of numbers; :param content: content to check for each number; """ - self.assertEqual(len(self._sms), len(numbers)) for number in numbers: sent_sms = next((sms for sms in self._sms if sms['number'] == number), None) self.assertTrue(bool(sent_sms), 'Number %s not found in %s' % (number, repr([s['number'] for s in self._sms]))) self.assertEqual(sent_sms['body'], content) - def _clear_sms_sent(self): - self._sms = [] + def assertSMSCanceled(self, partner, number, error_code, content=None): + """ Check canceled SMS. Search is done for a pair partner / number where + partner can be an empty recordset. """ + sms = self.env['sms.sms'].sudo().search([ + ('partner_id', '=', partner.id), ('number', '=', number), + ('state', '=', 'canceled') + ]) + self.assertTrue(sms, 'SMS: not found canceled SMS for %s (number: %s)' % (partner, number)) + self.assertEqual(sms.error_code, error_code) + if content is not None: + self.assertEqual(sms.body, content) + + def assertSMSFailed(self, partner, number, error_code, content=None): + """ Check failed SMS. Search is done for a pair partner / number where + partner can be an empty recordset. """ + sms = self.env['sms.sms'].sudo().search([ + ('partner_id', '=', partner.id), ('number', '=', number), + ('state', '=', 'error') + ]) + self.assertTrue(sms, 'SMS: not found failed SMS for %s (number: %s)' % (partner, number)) + self.assertEqual(sms.error_code, error_code) + if content is not None: + self.assertEqual(sms.body, content) + + def assertSMSOutgoing(self, partner, number, content=None): + """ Check outgoing SMS. Search is done for a pair partner / number where + partner can be an empty recordset. """ + sms = self.env['sms.sms'].sudo().search([ + ('partner_id', '=', partner.id), ('number', '=', number), + ('state', '=', 'outgoing') + ]) + self.assertTrue(sms, 'SMS: not found failed SMS for %s (number: %s, state)' % (partner, number)) + if content is not None: + self.assertEqual(sms.body, content) + + def assertSMSNotification(self, recipients_info, content, messages=None, check_sms=True): + """ Check content of notifications. + + :param recipients_info: list[{ + 'partner': res.partner record (may be empty), + 'number': number used for notification (may be empty, computed based on partner), + 'state': ready / sent / exception / canceled (sent by default), + 'failure_type': optional: sms_number_missing / sms_number_format / sms_credit / sms_server + }, { ... }] + """ + partners = self.env['res.partner'].concat(*list(p['partner'] for p in recipients_info if p.get('partner'))) + numbers = [p['number'] for p in recipients_info if p.get('number')] + base_domain = [ + '|', ('res_partner_id', 'in', partners.ids), + '&', ('res_partner_id', '=', False), ('sms_number', 'in', numbers), + ('notification_type', '=', 'sms') + ] + if messages is not None: + base_domain += [('mail_message_id', 'in', messages.ids)] + notifications = self.env['mail.notification'].search(base_domain) + + self.assertEqual(notifications.mapped('res_partner_id'), partners) + + for recipient_info in recipients_info: + partner = recipient_info.get('partner', self.env['res.partner']) + number = recipient_info.get('number') + state = recipient_info.get('state', 'sent') + if number is None and partner: + number = phone_validation.phone_get_sanitized_record_number(partner) + + notif = notifications.filtered(lambda n: n.res_partner_id == partner and n.sms_number == number and n.notification_status == state) + self.assertTrue(notif, 'SMS: not found notification for %s (number: %s, state: %s)' % (partner, number, state)) + + if state not in ('sent', 'ready', 'canceled'): + self.assertEqual(notif.failure_type, recipient_info['failure_type']) + if check_sms: + if state == 'sent': + self.assertSMSSent([number], content) + elif state == 'ready': + self.assertSMSOutgoing(partner, number, content) + elif state == 'exception': + self.assertSMSFailed(partner, number, recipient_info['failure_type'], content) + elif state == 'canceled': + self.assertSMSCanceled(partner, number, recipient_info.get('failure_type', False), content) + + for message in messages: + self.assertEqual(content, tools.html2plaintext(message.body).rstrip('\n')) diff --git a/addons/sms/views/sms_sms_views.xml b/addons/sms/views/sms_sms_views.xml new file mode 100644 index 00000000000..a6da804286a --- /dev/null +++ b/addons/sms/views/sms_sms_views.xml @@ -0,0 +1,66 @@ + + + + sms.sms.view.form + sms.sms + +
+
+
+ + + + + + + + + + + + + + + +
+ + + + + sms.sms.view.tree + sms.sms + + + + + + + + + + + + sms.sms.view.search + sms.sms + + + + + + + + + + SMS + sms.sms + tree,form + + + + + + + diff --git a/addons/sms/wizard/sms_composer.py b/addons/sms/wizard/sms_composer.py index 2eefe1c42b4..93b3202a83a 100644 --- a/addons/sms/wizard/sms_composer.py +++ b/addons/sms/wizard/sms_composer.py @@ -1,25 +1,9 @@ # -*- coding: utf-8 -*- # Part of Odoo. See LICENSE file for full copyright and licensing details. -import logging - from odoo import api, fields, models, _ +from odoo.addons.phone_validation.tools import phone_validation from odoo.exceptions import UserError -from odoo.addons.iap.models import iap - -_logger = logging.getLogger(__name__) - -try: - import phonenumbers - _sms_phonenumbers_lib_imported = True - -except ImportError: - _sms_phonenumbers_lib_imported = False - _logger.info( - "The `phonenumbers` Python module is not available. " - "Phone number validation will be skipped. " - "Try `pip3 install phonenumbers` to install it." - ) class SendSMS(models.TransientModel): @@ -29,27 +13,6 @@ class SendSMS(models.TransientModel): recipients = fields.Char('Recipients', required=True) message = fields.Text('Message', required=True) - def _phone_get_country(self, partner): - if 'country_id' in partner: - return partner.country_id - return self.env.company.country_id - - def _sms_sanitization(self, partner, field_name): - number = partner[field_name] - if number and _sms_phonenumbers_lib_imported: - country = self._phone_get_country(partner) - country_code = country.code if country else None - try: - phone_nbr = phonenumbers.parse(number, region=country_code, keep_raw_input=True) - except phonenumbers.phonenumberutil.NumberParseException: - return number - if not phonenumbers.is_possible_number(phone_nbr) or not phonenumbers.is_valid_number(phone_nbr): - return number - phone_fmt = phonenumbers.PhoneNumberFormat.E164 - return phonenumbers.format_number(phone_nbr, phone_fmt) - else: - return number - def _get_records(self, model): if self.env.context.get('active_domain'): records = model.search(self.env.context.get('active_domain')) @@ -64,14 +27,14 @@ class SendSMS(models.TransientModel): result = super(SendSMS, self).default_get(fields) active_model = self.env.context.get('active_model') - if not self.env.context.get('default_recipients') and active_model and hasattr(self.env[active_model], '_get_default_sms_recipients'): + if not self.env.context.get('default_recipients') and active_model and hasattr(self.env[active_model], '_sms_get_default_partners'): model = self.env[active_model] records = self._get_records(model) - partners = records._get_default_sms_recipients() + partners = records._sms_get_default_partners() phone_numbers = [] no_phone_partners = [] for partner in partners: - number = self._sms_sanitization(partner, self.env.context.get('field_name') or 'mobile') + number = phone_validation.phone_get_sanitized_record_number(partner, self.env.context.get('field_name') or 'mobile', 'country_id') if number: phone_numbers.append(number) else: @@ -86,10 +49,10 @@ class SendSMS(models.TransientModel): numbers = [number.strip() for number in self.recipients.split(',') if number.strip()] active_model = self.env.context.get('active_model') - if active_model and hasattr(self.env[active_model], 'message_post_send_sms'): + if active_model and hasattr(self.env[active_model], '_message_sms'): model = self.env[active_model] records = self._get_records(model) - records.message_post_send_sms(self.message, numbers=numbers) + records[0]._message_sms(self.message, sms_numbers=numbers) else: self.env['sms.api']._send_sms(numbers, self.message) return True diff --git a/addons/test_mail/tests/common.py b/addons/test_mail/tests/common.py index 9881d631977..752c489cadb 100644 --- a/addons/test_mail/tests/common.py +++ b/addons/test_mail/tests/common.py @@ -174,13 +174,13 @@ class TestRecipients(common.SavepointCase): 'name': 'Valid Lelitre', 'email': 'valid.lelitre@agrolait.com', 'country_id': cls.env.ref('base.be').id, - 'mobile': '0475001122', + 'mobile': '0456001122', }) cls.partner_2 = Partner.create({ 'name': 'Valid Poilvache', 'email': 'valid.other@gmail.com', 'country_id': cls.env.ref('base.be').id, - 'mobile': '+32 475 22 11 00', + 'mobile': '+32 456 22 11 00', }) diff --git a/addons/test_mail/tests/test_performance.py b/addons/test_mail/tests/test_performance.py index 666728d53dd..2099e0ccc1b 100644 --- a/addons/test_mail/tests/test_performance.py +++ b/addons/test_mail/tests/test_performance.py @@ -506,7 +506,7 @@ class TestHeavyMailPerformance(BaseMailPerformance): }) 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=103): # com runbot: 99 -103 // test_mail only: 99 - 103 + with self.assertQueryCount(__system__=100, emp=106): # com runbot: 100 -106 // test_mail only: 100 - 106 rec.write({ 'name': 'Test2', 'umbrella_id': self.umbrella.id, @@ -542,7 +542,7 @@ class TestHeavyMailPerformance(BaseMailPerformance): }) self.assertEqual(rec.message_partner_ids, self.user_portal.partner_id | self.env.user.partner_id) - with self.assertQueryCount(__system__=104, emp=108): # test_mail only: 104 - 108 + with self.assertQueryCount(__system__=105, emp=111): # test_mail only: 105 - 111 rec.write({ 'name': 'Test2', 'umbrella_id': umbrella_id, diff --git a/addons/test_mail_full/__manifest__.py b/addons/test_mail_full/__manifest__.py index dab4550230a..3f6c16e5639 100644 --- a/addons/test_mail_full/__manifest__.py +++ b/addons/test_mail_full/__manifest__.py @@ -15,7 +15,7 @@ real applications. """, 'mail', 'mail_bot', # 'snailmail', - 'mass_mailing', + # 'mass_mailing', 'phone_validation', 'sms', ], diff --git a/addons/test_mail_full/models/test_mail_models.py b/addons/test_mail_full/models/test_mail_models.py index b351ba8ce45..d0d58e0467a 100644 --- a/addons/test_mail_full/models/test_mail_models.py +++ b/addons/test_mail_full/models/test_mail_models.py @@ -10,6 +10,7 @@ class MailTestSMS(models.Model): _description = 'Chatter Model for SMS Gateway' _name = 'mail.test.sms' _inherit = ['mail.thread'] + _order = 'name asc, id asc' name = fields.Char() subject = fields.Char() @@ -18,6 +19,19 @@ class MailTestSMS(models.Model): mobile_nbr = fields.Char() customer_id = fields.Many2one('res.partner', 'Customer') - @api.multi - def _get_default_sms_recipients(self): + def _sms_get_default_partners(self): return self.mapped('customer_id') + + def _sms_get_number_fields(self): + return ['phone_nbr', 'mobile_nbr'] + + +class MailTestSMSSoLike(models.Model): + """ A model like sale order having only a customer, not specific phone + or mobile fields. """ + _description = 'Chatter Model for SMS Gateway (Partner only)' + _name = 'mail.test.sms.partner' + _inherit = ['mail.thread'] + + name = fields.Char() + partner_id = fields.Many2one('res.partner', 'Customer') diff --git a/addons/test_mail_full/security/ir.model.access.csv b/addons/test_mail_full/security/ir.model.access.csv index 2506e6ac804..186986eef83 100644 --- a/addons/test_mail_full/security/ir.model.access.csv +++ b/addons/test_mail_full/security/ir.model.access.csv @@ -1,3 +1,5 @@ id,name,model_id:id,group_id:id,perm_read,perm_write,perm_create,perm_unlink access_mail_test_sms_all,mail.test.sms.all,model_mail_test_sms,,0,0,0,0 access_mail_test_sms_user,mail.test.sms.user,model_mail_test_sms,base.group_user,1,1,1,1 +access_mail_test_sms_partner_all,mail.test.sms.partner.all,model_mail_test_sms_partner,,0,0,0,0 +access_mail_test_sms_partner_user,mail.test.sms.partner.user,model_mail_test_sms_partner,base.group_user,1,1,1,1 diff --git a/addons/test_mail_full/tests/__init__.py b/addons/test_mail_full/tests/__init__.py index 3eac1b648f9..b5c867844ba 100644 --- a/addons/test_mail_full/tests/__init__.py +++ b/addons/test_mail_full/tests/__init__.py @@ -2,4 +2,6 @@ from . import common from . import test_sms_composer +from . import test_sms_performance from . import test_sms_post +from . import test_sms_sms diff --git a/addons/test_mail_full/tests/common.py b/addons/test_mail_full/tests/common.py index b9abbbec7dc..448ad44ade1 100644 --- a/addons/test_mail_full/tests/common.py +++ b/addons/test_mail_full/tests/common.py @@ -1,5 +1,6 @@ # -*- coding: utf-8 -*- +from odoo.addons.phone_validation.tools import phone_validation from odoo.addons.test_mail.tests import common as test_mail_common @@ -12,3 +13,10 @@ class BaseFunctionalTest(test_mail_common.BaseFunctionalTest): # update country to belgium in order to test sanitization of numbers cls.user_employee.company_id.write({'country_id': cls.env.ref('base.be').id}) + + # some numbers for testing + cls.random_numbers_str = '+32456998877, 0456665544' + cls.random_numbers = cls.random_numbers_str.split(', ') + cls.random_numbers_san = [phone_validation.phone_format(number, 'BE', '32', force_format='E164') for number in cls.random_numbers] + cls.test_numbers = ['+32456010203', '0456 04 05 06'] + cls.test_numbers_san = [phone_validation.phone_format(number, 'BE', '32', force_format='E164') for number in cls.test_numbers] diff --git a/addons/test_mail_full/tests/test_sms_composer.py b/addons/test_mail_full/tests/test_sms_composer.py index bcdcbd32f1e..dd2aaf64ae7 100644 --- a/addons/test_mail_full/tests/test_sms_composer.py +++ b/addons/test_mail_full/tests/test_sms_composer.py @@ -1,10 +1,10 @@ # -*- coding: utf-8 -*- # Part of Odoo. See LICENSE file for full copyright and licensing details. +from odoo.addons.phone_validation.tools import phone_validation from odoo.addons.sms.tests import common as sms_common from odoo.addons.test_mail.tests import common as test_mail_common from odoo.addons.test_mail_full.tests import common as test_mail_full_common -from odoo.addons.phone_validation.tools.phone_validation import phone_format class TestSMSComposer(test_mail_full_common.BaseFunctionalTest, sms_common.MockSMS, test_mail_common.MockEmails, test_mail_common.TestRecipients): @@ -14,11 +14,9 @@ class TestSMSComposer(test_mail_full_common.BaseFunctionalTest, sms_common.MockS super(TestSMSComposer, cls).setUpClass() cls._test_body = 'VOID CONTENT' cls.partner_numbers = [ - phone_format(partner.mobile, partner.country_id.code, partner.country_id.phone_code, force_format='E164') + phone_validation.phone_format(partner.mobile, partner.country_id.code, partner.country_id.phone_code, force_format='E164') for partner in (cls.partner_1 | cls.partner_2) ] - cls.random_numbers_str = '+32475998877, 0475997788' - cls.random_numbers = [phone_format(number, 'BE', '32', force_format='E164') for number in ['+32475998877', '0475997788']] def test_composer_no_model(self): composer = self.env['sms.composer'].with_context().create({ @@ -62,13 +60,13 @@ class TestSMSComposer(test_mail_full_common.BaseFunctionalTest, sms_common.MockS 'mobile': 'coincoin', }) partners = self.partner_1 | self.partner_2 | partner_incorrect - composer = self.env['sms.composer'].with_context( - active_model='res.partner', - active_domain=[('id', 'in', partners.ids)] - ).create({ - 'message': self._test_body, - }) + # composer = self.env['sms.composer'].with_context( + # active_model='res.partner', + # active_domain=[('id', 'in', partners.ids)] + # ).create({ + # 'message': self._test_body, + # }) - with self.mockSMSGateway(): - composer.action_send_sms() + # with self.mockSMSGateway(): + # composer.action_send_sms() # self.assertSMSSent((self.partner_1 | self.partner_2).mapped('mobile'), test_body) # TDE FIXME: actually sanitizer does not work in current master (saas 12.23)) \ No newline at end of file diff --git a/addons/test_mail_full/tests/test_sms_performance.py b/addons/test_mail_full/tests/test_sms_performance.py new file mode 100644 index 00000000000..65c9e2cb4b5 --- /dev/null +++ b/addons/test_mail_full/tests/test_sms_performance.py @@ -0,0 +1,88 @@ +# -*- coding: utf-8 -*- +# Part of Odoo. See LICENSE file for full copyright and licensing details. + +from odoo.addons.sms.tests import common as sms_common +from odoo.addons.test_mail.tests.test_performance import BaseMailPerformance +from odoo.tests.common import users, warmup +from odoo.tests import tagged +from odoo.tools import mute_logger + + +@tagged('mail_performance') +class TestSMSPerformance(BaseMailPerformance, sms_common.MockSMS): + + def setUp(self): + super(TestSMSPerformance, self).setUp() + self.user_employee.write({ + 'login': 'employee', + }) + self.admin = self.env.user + + self.customer = self.env['res.partner'].with_context(self._quick_create_ctx).create({ + 'name': 'Test Customer', + 'email': 'test@example.com', + 'mobile': '0456123456', + 'country_id': self.env.ref('base.be').id, + }) + self.test_record = self.env['mail.test.sms'].with_context(self._quick_create_ctx).create({ + 'name': 'Test', + 'customer_id': self.customer.id, + 'phone_nbr': '0456999999', + }) + + # prepare recipients to test for more realistic workload + Partners = self.env['res.partner'].with_context(self._quick_create_ctx) + self.partners = self.env['res.partner'] + for x in range(0, 10): + self.partners |= Partners.create({ + 'name': 'Test %s' % x, + 'email': 'test%s@example.com' % x, + 'mobile': '0456%s%s0000' % (x, x), + 'country_id': self.env.ref('base.be').id, + }) + + # patch registry to simulate a ready environment + self.patch(self.env.registry, 'ready', True) + + @mute_logger('odoo.addons.sms.models.sms_sms') + @users('employee') + @warmup + def test_message_sms_record_1_partner(self): + record = self.test_record.with_user(self.env.user) + pids = self.customer.ids + with self.mockSMSGateway(), self.assertQueryCount(employee=22): # test_mail_enterprise: 22 + messages = record._message_sms( + body='Performance Test', + partner_ids=pids, + ) + + self.assertEqual(record.message_ids[0].body, '

Performance Test

') + self.assertSMSNotification([{'partner': self.customer}], 'Performance Test', messages) + + @mute_logger('odoo.addons.sms.models.sms_sms') + @users('employee') + @warmup + def test_message_sms_record_10_partners(self): + record = self.test_record.with_user(self.env.user) + pids = self.partners.ids + with self.mockSMSGateway(), self.assertQueryCount(employee=40): # test_mail_enterprise: 40 + messages = record._message_sms( + body='Performance Test', + partner_ids=pids, + ) + + self.assertEqual(record.message_ids[0].body, '

Performance Test

') + self.assertSMSNotification([{'partner': partner} for partner in self.partners], 'Performance Test', messages) + + @mute_logger('odoo.addons.sms.models.sms_sms') + @users('employee') + @warmup + def test_message_sms_record_default(self): + record = self.test_record.with_user(self.env.user) + with self.mockSMSGateway(), self.assertQueryCount(employee=26): # test_mail_enterprise: 26 + messages = record._message_sms( + body='Performance Test', + ) + + self.assertEqual(record.message_ids[0].body, '

Performance Test

') + self.assertSMSNotification([{'partner': self.customer}], 'Performance Test', messages) diff --git a/addons/test_mail_full/tests/test_sms_post.py b/addons/test_mail_full/tests/test_sms_post.py index a12855341a0..f1e7d91b1d0 100644 --- a/addons/test_mail_full/tests/test_sms_post.py +++ b/addons/test_mail_full/tests/test_sms_post.py @@ -1,6 +1,7 @@ # -*- coding: utf-8 -*- # Part of Odoo. See LICENSE file for full copyright and licensing details. +from odoo.addons.phone_validation.tools import phone_validation from odoo.addons.sms.tests import common as sms_common from odoo.addons.test_mail.tests import common as test_mail_common from odoo.addons.test_mail_full.tests import common as test_mail_full_common @@ -11,44 +12,280 @@ class TestSMSPost(test_mail_full_common.BaseFunctionalTest, sms_common.MockSMS, @classmethod def setUpClass(cls): super(TestSMSPost, cls).setUpClass() + cls._test_body = 'VOID CONTENT' + + cls.partner_numbers = [ + phone_validation.phone_format(partner.mobile, partner.country_id.code, partner.country_id.phone_code, force_format='E164') + for partner in (cls.partner_1 | cls.partner_2) + ] + + cls.test_record = cls.env['mail.test.sms'].with_context(**cls._test_context).create({ + 'name': 'Test', + 'customer_id': cls.partner_1.id, + 'mobile_nbr': cls.test_numbers[0], + 'phone_nbr': cls.test_numbers[1], + }) + cls.test_record = cls._reset_mail_context(cls.test_record) + + def test_message_sms_internals_body(self): + with self.sudo('employee'), self.mockSMSGateway(): + test_record = self.env['mail.test.sms'].browse(self.test_record.id) + messages = test_record._message_sms('

Mega SMS
Top moumoutte

', partner_ids=self.partner_1.ids) + + self.assertEqual(messages.body, '

Mega SMS
Top moumoutte

') + self.assertSMSNotification([{'partner': self.partner_1}], 'Mega SMS\nTop moumoutte', messages) + + def test_message_sms_internals_check_existing(self): + with self.sudo('employee'), self.mockSMSGateway(sim_error='wrong_format_number'): + test_record = self.env['mail.test.sms'].browse(self.test_record.id) + messages = test_record._message_sms(self._test_body, partner_ids=self.partner_1.ids) + + self.assertSMSNotification([{'partner': self.partner_1, 'state': 'exception', 'failure_type': 'sms_number_format'}], self._test_body, messages) + + with self.sudo('employee'), self.mockSMSGateway(): + test_record = self.env['mail.test.sms'].browse(self.test_record.id) + test_record._notify_record_by_sms(messages, {'partners': [{'id': self.partner_1.id, 'notif': 'sms'}]}, check_existing=True) + self.assertSMSNotification([{'partner': self.partner_1}], self._test_body, messages) + + def test_message_sms_internals_sms_numbers(self): + with self.sudo('employee'), self.mockSMSGateway(): + test_record = self.env['mail.test.sms'].browse(self.test_record.id) + messages = test_record._message_sms(self._test_body, partner_ids=self.partner_1.ids, sms_numbers=self.random_numbers) + + self.assertSMSNotification([{'partner': self.partner_1}, {'number': self.random_numbers_san[0]}, {'number': self.random_numbers_san[1]}], self._test_body, messages) + + def test_message_sms_internals_pid_to_number(self): + pid_to_number = { + self.partner_1.id: self.random_numbers[0], + self.partner_2.id: self.random_numbers[1], + } + with self.sudo('employee'), self.mockSMSGateway(): + test_record = self.env['mail.test.sms'].browse(self.test_record.id) + messages = test_record._message_sms(self._test_body, partner_ids=(self.partner_1 | self.partner_2).ids, sms_pid_to_number=pid_to_number) + + self.assertSMSNotification([ + {'partner': self.partner_1, 'number': self.random_numbers_san[0]}, + {'partner': self.partner_2, 'number': self.random_numbers_san[1]}], + self._test_body, messages) + + def test_message_sms_model_partner(self): + with self.sudo('employee'), self.mockSMSGateway(): + messages = self.partner_1._message_sms(self._test_body) + messages |= self.partner_2._message_sms(self._test_body) + self.assertSMSNotification([{'partner': self.partner_1}, {'partner': self.partner_2}], self._test_body, messages) + + def test_message_sms_model_partner_fallback(self): + self.partner_1.write({'mobile': False, 'phone': self.random_numbers[0]}) + + with self.mockSMSGateway(): + messages = self.partner_1._message_sms(self._test_body) + messages |= self.partner_2._message_sms(self._test_body) + + self.assertSMSNotification([{'partner': self.partner_1, 'number': self.random_numbers_san[0]}, {'partner': self.partner_2}], self._test_body, messages) + + def test_message_sms_model_w_partner_only(self): + with self.sudo('employee'): + record = self.env['mail.test.sms.partner'].create({'partner_id': self.partner_1.id}) + + with self.mockSMSGateway(): + messages = record._message_sms(self._test_body) + + self.assertSMSNotification([{'partner': self.partner_1}], self._test_body, messages) + + def test_message_sms_model_w_partner_only_void(self): + with self.sudo('employee'): + record = self.env['mail.test.sms.partner'].create({'partner_id': False}) + + with self.mockSMSGateway(): + messages = record._message_sms(self._test_body) + + # should not crash but no sms / no recipients + notifs = self.env['mail.notification'].search([('mail_message_id', 'in', messages.ids)]) + self.assertFalse(notifs) + + def test_message_sms_on_field_w_partner(self): + with self.sudo('employee'), self.mockSMSGateway(): + test_record = self.env['mail.test.sms'].browse(self.test_record.id) + messages = test_record._message_sms(self._test_body, number_field='mobile_nbr') + + self.assertSMSNotification([{'partner': self.partner_1, 'number': self.test_record.mobile_nbr}], self._test_body, messages) + + def test_message_sms_on_field_wo_partner(self): + self.test_record.write({'customer_id': False}) + + with self.sudo('employee'), self.mockSMSGateway(): + test_record = self.env['mail.test.sms'].browse(self.test_record.id) + messages = test_record._message_sms(self._test_body, number_field='mobile_nbr') + + self.assertSMSNotification([{'number': self.test_record.mobile_nbr}], self._test_body, messages) + + def test_message_sms_on_field_wo_partner_default_field(self): + self.test_record.write({'customer_id': False}) + + with self.sudo('employee'), self.mockSMSGateway(): + test_record = self.env['mail.test.sms'].browse(self.test_record.id) + messages = test_record._message_sms(self._test_body) + + self.assertSMSNotification([{'number': self.test_numbers_san[1]}], self._test_body, messages) + + def test_message_sms_on_field_wo_partner_default_field_2(self): + self.test_record.write({'customer_id': False, 'phone_nbr': False}) + + with self.sudo('employee'), self.mockSMSGateway(): + test_record = self.env['mail.test.sms'].browse(self.test_record.id) + messages = test_record._message_sms(self._test_body) + + self.assertSMSNotification([{'number': self.test_numbers_san[0]}], self._test_body, messages) + + def test_message_sms_on_numbers(self): + with self.sudo('employee'), self.mockSMSGateway(): + test_record = self.env['mail.test.sms'].browse(self.test_record.id) + messages = test_record._message_sms(self._test_body, sms_numbers=self.random_numbers_san) + self.assertSMSNotification([{'number': self.random_numbers_san[0]}, {'number': self.random_numbers_san[1]}], self._test_body, messages) + + def test_message_sms_on_numbers_sanitization(self): + with self.sudo('employee'), self.mockSMSGateway(): + test_record = self.env['mail.test.sms'].browse(self.test_record.id) + messages = test_record._message_sms(self._test_body, sms_numbers=self.random_numbers) + self.assertSMSNotification([{'number': self.random_numbers_san[0]}, {'number': self.random_numbers_san[1]}], self._test_body, messages) + + def test_message_sms_on_partner_ids(self): + with self.sudo('employee'), self.mockSMSGateway(): + test_record = self.env['mail.test.sms'].browse(self.test_record.id) + messages = test_record._message_sms(self._test_body, partner_ids=(self.partner_1 | self.partner_2).ids) + + self.assertSMSNotification([{'partner': self.partner_1}, {'partner': self.partner_2}], self._test_body, messages) + + def test_message_sms_on_partner_ids_default(self): + with self.sudo('employee'), self.mockSMSGateway(): + test_record = self.env['mail.test.sms'].browse(self.test_record.id) + messages = test_record._message_sms(self._test_body) + + self.assertSMSNotification([{'partner': self.test_record.customer_id, 'number': self.test_numbers_san[1]}], self._test_body, messages) + + def test_message_sms_on_partner_ids_w_numbers(self): + with self.sudo('employee'), self.mockSMSGateway(): + test_record = self.env['mail.test.sms'].browse(self.test_record.id) + messages = test_record._message_sms(self._test_body, partner_ids=self.partner_1.ids, sms_numbers=self.random_numbers[:1]) + + self.assertSMSNotification([{'partner': self.partner_1}, {'number': self.random_numbers_san[0]}], self._test_body, messages) + + +class TestSMSPostException(test_mail_full_common.BaseFunctionalTest, sms_common.MockSMS, test_mail_common.MockEmails, test_mail_common.TestRecipients): + + @classmethod + def setUpClass(cls): + super(TestSMSPostException, cls).setUpClass() + cls._test_body = 'VOID CONTENT' + cls.test_record = cls.env['mail.test.sms'].with_context(**cls._test_context).create({ 'name': 'Test', 'customer_id': cls.partner_1.id, }) cls.test_record = cls._reset_mail_context(cls.test_record) + cls.partner_3 = cls.env['res.partner'].with_context({ + 'mail_create_nolog': True, + 'mail_create_nosubscribe': True, + 'mail_notrack': True, + 'no_reset_password': True, + }).create({ + 'name': 'Ernestine Loubine', + 'email': 'ernestine.loubine@agrolait.com', + 'country_id': cls.env.ref('base.be').id, + 'mobile': '0475556644', + }) - def test_message_post_with_sms_on_partners(self): - test_body = 'Void body' - with self.mockSMSGateway(): - (self.partner_1 | self.partner_2).message_post_send_sms(test_body) - self.assertSMSSent((self.partner_1 | self.partner_2).mapped('mobile'), test_body) + def test_message_sms_w_numbers_invalid(self): + random_numbers = self.random_numbers + ['6988754'] + with self.sudo('employee'), self.mockSMSGateway(): + test_record = self.env['mail.test.sms'].browse(self.test_record.id) + messages = test_record._message_sms(self._test_body, sms_numbers=random_numbers) - def test_message_post_with_sms_w_default(self): - test_body = 'Void body' - with self.mockSMSGateway(): - self.test_record.message_post_send_sms(test_body) - self.assertSMSSent(self.partner_1.mapped('mobile'), test_body) - self.assertIn(test_body, self.test_record.message_ids.body) + # invalid numbers are still given to IAP currently as they are + self.assertSMSNotification([{'number': self.random_numbers_san[0]}, {'number': self.random_numbers_san[1]}, {'number': random_numbers[2]}], self._test_body, messages) - def test_message_post_with_sms_w_numbers(self): - test_body = 'Void body' - test_numbers = ['0475114477', '0475225588'] - with self.mockSMSGateway(): - self.test_record.message_post_send_sms(test_body, numbers=test_numbers) - self.assertSMSSent(test_numbers, test_body) - self.assertIn(test_body, self.test_record.message_ids.body) + def test_message_sms_w_partners_nocountry(self): + self.test_record.customer_id.write({ + 'mobile': self.random_numbers[0], + 'phone': self.random_numbers[1], + 'country_id': False, + }) + with self.sudo('employee'), self.mockSMSGateway(): + test_record = self.env['mail.test.sms'].browse(self.test_record.id) + messages = test_record._message_sms(self._test_body, partner_ids=self.test_record.customer_id.ids) - def test_message_post_with_sms_w_numbers_duplicate(self): - test_body = 'Void body' - test_numbers = ['0475114477', '0475225588', '0475114477'] - with self.mockSMSGateway(): - self.test_record.message_post_send_sms(test_body, numbers=test_numbers) - self.assertSMSSent(test_numbers, test_body) - self.assertIn(test_body, self.test_record.message_ids.body) + self.assertSMSNotification([{'partner': self.test_record.customer_id}], self._test_body, messages) - def test_message_post_with_sms_w_partners(self): - test_body = 'Void body' - with self.mockSMSGateway(): - self.test_record.message_post_send_sms(test_body, partners=self.partner_1 | self.partner_2) - self.assertSMSSent((self.partner_1 | self.partner_2).mapped('mobile'), test_body) - self.assertIn(test_body, self.test_record.message_ids.body) + def test_message_sms_w_partners_falsy(self): + # TDE FIXME: currently sent to IAP + self.test_record.customer_id.write({ + 'mobile': 'youpie', + 'phone': 'youpla', + }) + with self.sudo('employee'), self.mockSMSGateway(): + test_record = self.env['mail.test.sms'].browse(self.test_record.id) + messages = test_record._message_sms(self._test_body, partner_ids=self.test_record.customer_id.ids) + + # self.assertSMSNotification({self.test_record.customer_id: {}}, {}, self._test_body, messages) + + def test_message_sms_w_numbers_sanitization_duplicate(self): + pass + # TDE FIXME: not sure + # random_numbers = self.random_numbers + [self.random_numbers[1]] + # random_numbers_san = self.random_numbers_san + [self.random_numbers_san[1]] + # with self.sudo('employee'), self.mockSMSGateway(): + # messages = self.test_record._message_sms(self._test_body, sms_numbers=random_numbers) + # self.assertSMSNotification({}, {random_numbers_san[0]: {}, random_numbers_san[1]: {}, random_numbers_san[2]: {}}, self._test_body, messages) + + def test_message_sms_crash_credit(self): + with self.sudo('employee'), self.mockSMSGateway(sim_error='credit'): + test_record = self.env['mail.test.sms'].browse(self.test_record.id) + messages = test_record._message_sms(self._test_body, partner_ids=(self.partner_1 | self.partner_2).ids) + + self.assertSMSNotification([ + {'partner': self.partner_1, 'state': 'exception', 'failure_type': 'sms_credit'}, + {'partner': self.partner_2, 'state': 'exception', 'failure_type': 'sms_credit'}, + ], self._test_body, messages) + + def test_message_sms_crash_credit_single(self): + with self.sudo('employee'), self.mockSMSGateway(nbr_t_error={phone_validation.phone_get_sanitized_record_number(self.partner_2): 'credit'}): + test_record = self.env['mail.test.sms'].browse(self.test_record.id) + messages = test_record._message_sms(self._test_body, partner_ids=(self.partner_1 | self.partner_2 | self.partner_3).ids) + + self.assertSMSNotification([ + {'partner': self.partner_1, 'state': 'sent'}, + {'partner': self.partner_2, 'state': 'exception', 'failure_type': 'sms_credit'}, + {'partner': self.partner_3, 'state': 'sent'}, + ], self._test_body, messages) + + def test_message_sms_crash_server_crash(self): + with self.sudo('employee'), self.mockSMSGateway(sim_error='jsonrpc_exception'): + test_record = self.env['mail.test.sms'].browse(self.test_record.id) + messages = test_record._message_sms(self._test_body, partner_ids=(self.partner_1 | self.partner_2 | self.partner_3).ids) + + self.assertSMSNotification([ + {'partner': self.partner_1, 'state': 'exception', 'failure_type': 'sms_server'}, + {'partner': self.partner_2, 'state': 'exception', 'failure_type': 'sms_server'}, + {'partner': self.partner_3, 'state': 'exception', 'failure_type': 'sms_server'}, + ], self._test_body, messages) + + def test_message_sms_crash_wrong_number(self): + with self.sudo('employee'), self.mockSMSGateway(sim_error='wrong_format_number'): + test_record = self.env['mail.test.sms'].browse(self.test_record.id) + messages = test_record._message_sms(self._test_body, partner_ids=(self.partner_1 | self.partner_2).ids) + + self.assertSMSNotification([ + {'partner': self.partner_1, 'state': 'exception', 'failure_type': 'sms_number_format'}, + {'partner': self.partner_2, 'state': 'exception', 'failure_type': 'sms_number_format'}, + ], self._test_body, messages) + + def test_message_sms_crash_wrong_number_single(self): + with self.sudo('employee'), self.mockSMSGateway(nbr_t_error={phone_validation.phone_get_sanitized_record_number(self.partner_2): 'wrong_format_number'}): + test_record = self.env['mail.test.sms'].browse(self.test_record.id) + messages = test_record._message_sms(self._test_body, partner_ids=(self.partner_1 | self.partner_2 | self.partner_3).ids) + + self.assertSMSNotification([ + {'partner': self.partner_1, 'state': 'sent'}, + {'partner': self.partner_2, 'state': 'exception', 'failure_type': 'sms_number_format'}, + {'partner': self.partner_3, 'state': 'sent'}, + ], self._test_body, messages) diff --git a/addons/test_mail_full/tests/test_sms_sms.py b/addons/test_mail_full/tests/test_sms_sms.py new file mode 100644 index 00000000000..46149f559a8 --- /dev/null +++ b/addons/test_mail_full/tests/test_sms_sms.py @@ -0,0 +1,59 @@ +# -*- coding: utf-8 -*- +# Part of Odoo. See LICENSE file for full copyright and licensing details. + +from unittest.mock import patch +from unittest.mock import DEFAULT + +from odoo import exceptions +from odoo.addons.sms.models.sms_sms import SmsSms as SmsSms +from odoo.addons.sms.tests import common as sms_common +from odoo.addons.test_mail.tests import common as test_mail_common +from odoo.addons.test_mail_full.tests import common as test_mail_full_common + + +class TestSMSPost(test_mail_full_common.BaseFunctionalTest, sms_common.MockSMS, test_mail_common.MockEmails, test_mail_common.TestRecipients): + + @classmethod + def setUpClass(cls): + super(TestSMSPost, cls).setUpClass() + cls._test_body = 'VOID CONTENT' + + cls.sms_all = cls.env['sms.sms'] + for x in range(10): + cls.sms_all |= cls.env['sms.sms'].create({ + 'number': '+324560000%s%s' % (x, x), + 'body': cls._test_body, + }) + + def test_sms_send_batch_size(self): + self.count = 0 + + def _send(sms_self, delete_all=False, raise_exception=False): + self.count += 1 + return DEFAULT + + self.env['ir.config_parameter'].set_param('sms.session.batch.size', '3') + with patch.object(SmsSms, '_send', autospec=True, side_effect=_send) as send_mock: + self.env['sms.sms'].browse(self.sms_all.ids).send() + + self.assertEqual(self.count, 4) + + def test_sms_send_crash_employee(self): + with self.assertRaises(exceptions.AccessError): + self.env['sms.sms'].with_user(self.user_employee).browse(self.sms_all.ids).send() + + def test_sms_send_delete_all(self): + with self.mockSMSGateway(sim_error='jsonrpc_exception'): + self.env['sms.sms'].browse(self.sms_all.ids).send(delete_all=True, raise_exception=False) + self.assertFalse(len(self.sms_all.exists())) + + def test_sms_send_raise(self): + with self.assertRaises(exceptions.AccessError): + with self.mockSMSGateway(sim_error='jsonrpc_exception'): + self.env['sms.sms'].browse(self.sms_all.ids).send(raise_exception=True) + self.assertEqual(set(self.sms_all.mapped('state')), set(['outgoing'])) + + def test_sms_send_raise_catch(self): + with self.mockSMSGateway(sim_error='jsonrpc_exception'): + self.env['sms.sms'].browse(self.sms_all.ids).send(raise_exception=False) + self.assertEqual(set(self.sms_all.mapped('state')), set(['error'])) From 9ad7a88a08ceccc3665acd8972ac47614205bf0e Mon Sep 17 00:00:00 2001 From: Pierre Rousseau Date: Fri, 24 May 2019 10:12:44 +0000 Subject: [PATCH 3/8] [IMP] sms: add SMS template and its support in SMS composer Purpose of this commit is to provide users templates to use when sending SMS. It is inspired from what already exists for mail templates. This commit * adds templates for SMS * users can now create template for SMS Text messages similar to mail templates; * jinja syntax is supported for body like mail templates, which is why templates are linked to a given model; * refactor the SMS composer * templates are supported in composer like the message composer. This is supported only in mass mode to avoid bloating the interface in standard composition mode; * composer code is rewritten to support various use cases (comment, mass sms, numbers, ...) and better fits all use cases; * composer is extended to support notably mass SMS without posting messages; Templates will also be used in a near future in mass sms sending (mass mailing application improvement) and in marketing automation (for enterprise). Tests are updated and added to ensure feature works. Related to task 1922163 Linked to PR #33510 Co-Authored-By: Thibault Delavallee Co-Authored-By: Pierre Rousseau --- addons/sms/__manifest__.py | 1 + addons/sms/models/__init__.py | 1 + addons/sms/models/mail_thread.py | 40 +++ addons/sms/models/sms_template.py | 121 +++++++ addons/sms/security/ir.model.access.csv | 3 + addons/sms/tests/common.py | 8 +- addons/sms/views/res_partner_views.xml | 44 ++- addons/sms/views/sms_template_views.xml | 71 ++++ addons/sms/wizard/sms_composer.py | 279 +++++++++++++-- addons/sms/wizard/sms_composer_views.xml | 77 +++- addons/test_mail_full/tests/__init__.py | 1 + .../test_mail_full/tests/test_sms_composer.py | 337 +++++++++++++++--- .../tests/test_sms_performance.py | 61 ++++ addons/test_mail_full/tests/test_sms_post.py | 42 +++ .../test_mail_full/tests/test_sms_template.py | 76 ++++ 15 files changed, 1044 insertions(+), 118 deletions(-) create mode 100644 addons/sms/models/sms_template.py create mode 100644 addons/sms/views/sms_template_views.xml create mode 100644 addons/test_mail_full/tests/test_sms_template.py diff --git a/addons/sms/__manifest__.py b/addons/sms/__manifest__.py index dad1ac221da..0de865810e7 100644 --- a/addons/sms/__manifest__.py +++ b/addons/sms/__manifest__.py @@ -18,6 +18,7 @@ The service is provided by the In App Purchase Odoo platform. 'views/res_partner_views.xml', 'views/assets.xml', 'views/sms_sms_views.xml', + 'views/sms_template_views.xml', 'security/ir.model.access.csv', ], 'qweb': [ diff --git a/addons/sms/models/__init__.py b/addons/sms/models/__init__.py index b855b4b7955..4adcd67ab64 100644 --- a/addons/sms/models/__init__.py +++ b/addons/sms/models/__init__.py @@ -8,3 +8,4 @@ from . import mail_thread from . import res_partner from . import sms_api from . import sms_sms +from . import sms_template diff --git a/addons/sms/models/mail_thread.py b/addons/sms/models/mail_thread.py index fd47be27a5d..b088309cbfe 100644 --- a/addons/sms/models/mail_thread.py +++ b/addons/sms/models/mail_thread.py @@ -84,6 +84,46 @@ class MailThread(models.AbstractModel): result[record.id] = {'partner': self.env['res.partner'], 'sanitized': False, 'number': False} return result + def _message_sms_schedule_mass(self, body='', template=False, active_domain=None): + """ Shortcut method to schedule a mass sms sending on a recordset. + + :param template: an optional sms.template record; + :param active_domain: bypass self.ids and apply composer on active_domain + instead; + """ + composer_context = { + 'default_res_model': self._name, + 'default_composition_mode': 'mass', + 'default_template_id': template.id if template else False, + 'default_body': body if body and not template else False, + } + if active_domain is not None: + composer_context['default_use_active_domain'] = True + composer_context['default_active_domain'] = repr(active_domain) + else: + composer_context['default_res_ids'] = self.ids + + composer = self.env['sms.composer'].with_context(**composer_context).create({}) + return composer._action_send_sms() + + def _message_sms_with_template(self, template=False, template_xmlid=False, template_fallback='', partner_ids=False, **kwargs): + """ Shortcut method to perform a _message_sms with an sms.template. + + :param template: a valid sms.template record; + :param template_xmlid: XML ID of an sms.template (if no template given); + :param template_fallback: plaintext (jinja-enabled) in case template + and template xml id are falsy (for example due to deleted data); + """ + self.ensure_one() + if not template and template_xmlid: + template = self.env.ref(template_xmlid, raise_if_not_found=False) + if template: + template_w_lang = template._get_context_lang_per_id(self.ids)[self.id] + body = template._render_template(template_w_lang.body, self._name, self.ids)[self.id] + else: + body = self.env['sms.template']._render_template(template_fallback, self._name, self.ids)[self.id] + return self._message_sms(body, partner_ids=partner_ids, **kwargs) + def _message_sms(self, body, subtype_id=False, partner_ids=False, number_field=False, sms_numbers=None, sms_pid_to_number=None, **kwargs): """ Main method to post a message on a record using SMS-based notification diff --git a/addons/sms/models/sms_template.py b/addons/sms/models/sms_template.py new file mode 100644 index 00000000000..6835bac7eb6 --- /dev/null +++ b/addons/sms/models/sms_template.py @@ -0,0 +1,121 @@ +# -*- coding: utf-8 -*- +# Part of Odoo. See LICENSE file for full copyright and licensing details. + +from odoo import api, fields, models, _ + + +class SMSTemplate(models.Model): + "Templates for sending SMS" + _name = "sms.template" + _description = 'SMS Templates' + + @api.model + def default_get(self, fields): + res = super(SMSTemplate, self).default_get(fields) + if not fields or 'model_id' in fields and not res.get('model_id') and res.get('model'): + res['model_id'] = self.env['ir.model']._get(res['model']).id + return res + + name = fields.Char() + model_id = fields.Many2one( + 'ir.model', string='Applies to', required=True, + domain=['&', ('is_mail_thread', '=', True), ('transient', '=', False)], + help="The type of document this template can be used with") + model = fields.Char('Related Document Model', related='model_id.model', index=True, store=True, readonly=True) + body = fields.Char('Body', translate=True, required=True) + lang = fields.Char('Language', placeholder="${object.partner_id.lang}") + # Fake fields used to implement the placeholder assistant + model_object_field = fields.Many2one('ir.model.fields', string="Field", store=False, + help="Select target field from the related document model.\n" + "If it is a relationship field you will be able to select " + "a target field at the destination of the relationship.") + sub_object = fields.Many2one('ir.model', 'Sub-model', readonly=True, store=False, + help="When a relationship field is selected as first field, " + "this field shows the document model the relationship goes to.") + sub_model_object_field = fields.Many2one('ir.model.fields', 'Sub-field', store=False, + help="When a relationship field is selected as first field, " + "this field lets you select the target field within the " + "destination document model (sub-model).") + null_value = fields.Char('Default Value', store=False, help="Optional value to use if the target field is empty") + copyvalue = fields.Char('Placeholder Expression', store=False, + help="Final placeholder expression, to be copy-pasted in the desired template field.") + + @api.onchange('model_object_field', 'sub_model_object_field', 'null_value') + def _onchange_dynamic_placeholder(self): + """ Generate the dynamic placeholder """ + if self.model_object_field: + if self.model_object_field.ttype in ['many2one', 'one2many', 'many2many']: + model = self.env['ir.model']._get(self.model_object_field.relation) + if model: + self.sub_object = model.id + sub_field_name = self.sub_model_object_field.name + self.copyvalue = self._build_expression(self.model_object_field.name, + sub_field_name, self.null_value or False) + else: + self.sub_object = False + self.sub_model_object_field = False + self.copyvalue = self._build_expression(self.model_object_field.name, False, self.null_value or False) + else: + self.sub_object = False + self.copyvalue = False + self.sub_model_object_field = False + self.null_value = False + + @api.model + def _build_expression(self, field_name, sub_field_name, null_value): + """Returns a placeholder expression for use in a template field, + based on the values provided in the placeholder assistant. + + :param field_name: main field name + :param sub_field_name: sub field name (M2O) + :param null_value: default value if the target value is empty + :return: final placeholder expression """ + expression = '' + if field_name: + expression = "${object." + field_name + if sub_field_name: + expression += "." + sub_field_name + if null_value: + expression += " or '''%s'''" % null_value + expression += "}" + return expression + + @api.multi + @api.returns('self', lambda value: value.id) + def copy(self, default=None): + default = dict(default or {}, + name=_("%s (copy)") % self.name) + return super(SMSTemplate, self).copy(default=default) + + @api.multi + def _get_context_lang_per_id(self, res_ids): + self.ensure_one() + if res_ids is None: + return {None: self} + + if self.env.context.get('template_preview_lang'): + lang = self.env.context.get('template_preview_lang') + results = dict((res_id, self.with_context(lang=lang)) for res_id in res_ids) + else: + rendered_langs = self._render_template(self.lang, self.model, res_ids) + results = dict( + (res_id, self.with_context(lang=lang) if lang else self) + for res_id, lang in rendered_langs.items()) + + return results + + @api.multi + def _get_ids_per_lang(self, res_ids): + self.ensure_one() + + rids_to_tpl = self._get_context_lang_per_id(res_ids) + tpl_to_rids = {} + for res_id, template in rids_to_tpl.items(): + tpl_to_rids.setdefault(template._context.get('lang', self.env.user.lang), []).append(res_id) + + return tpl_to_rids + + @api.model + def _render_template(self, template_txt, model, res_ids): + """ Render the jinja template """ + return self.env['mail.template']._render_template(template_txt, model, res_ids) diff --git a/addons/sms/security/ir.model.access.csv b/addons/sms/security/ir.model.access.csv index 42c4d624aaa..c906bc097b7 100644 --- a/addons/sms/security/ir.model.access.csv +++ b/addons/sms/security/ir.model.access.csv @@ -1,3 +1,6 @@ id,name,model_id:id,group_id:id,perm_read,perm_write,perm_create,perm_unlink access_sms_sms_all,access.sms.sms.all,model_sms_sms,,0,0,0,0 access_sms_sms_system,access.sms.sms.system,model_sms_sms,base.group_system,1,1,1,1 +access_sms_template_all,access.sms.template.all,model_sms_template,,0,0,0,0 +access_sms_template_user,access.sms.template.user,model_sms_template,base.group_user,1,0,0,0 +access_sms_template_system,access.sms.template.system,model_sms_template,base.group_system,1,1,1,0 diff --git a/addons/sms/tests/common.py b/addons/sms/tests/common.py index 7c0c3b5d22a..f0f96c0e8d6 100644 --- a/addons/sms/tests/common.py +++ b/addons/sms/tests/common.py @@ -73,6 +73,8 @@ class MockSMS(common.BaseCase): def assertSMSCanceled(self, partner, number, error_code, content=None): """ Check canceled SMS. Search is done for a pair partner / number where partner can be an empty recordset. """ + if number is None and partner: + number = phone_validation.phone_get_sanitized_record_number(partner) sms = self.env['sms.sms'].sudo().search([ ('partner_id', '=', partner.id), ('number', '=', number), ('state', '=', 'canceled') @@ -85,6 +87,8 @@ class MockSMS(common.BaseCase): def assertSMSFailed(self, partner, number, error_code, content=None): """ Check failed SMS. Search is done for a pair partner / number where partner can be an empty recordset. """ + if number is None and partner: + number = phone_validation.phone_get_sanitized_record_number(partner) sms = self.env['sms.sms'].sudo().search([ ('partner_id', '=', partner.id), ('number', '=', number), ('state', '=', 'error') @@ -97,11 +101,13 @@ class MockSMS(common.BaseCase): def assertSMSOutgoing(self, partner, number, content=None): """ Check outgoing SMS. Search is done for a pair partner / number where partner can be an empty recordset. """ + if number is None and partner: + number = phone_validation.phone_get_sanitized_record_number(partner) sms = self.env['sms.sms'].sudo().search([ ('partner_id', '=', partner.id), ('number', '=', number), ('state', '=', 'outgoing') ]) - self.assertTrue(sms, 'SMS: not found failed SMS for %s (number: %s, state)' % (partner, number)) + self.assertTrue(sms, 'SMS: not found failed SMS for %s (number: %s)' % (partner, number)) if content is not None: self.assertEqual(sms.body, content) diff --git a/addons/sms/views/res_partner_views.xml b/addons/sms/views/res_partner_views.xml index 1a483568178..e841fad4005 100644 --- a/addons/sms/views/res_partner_views.xml +++ b/addons/sms/views/res_partner_views.xml @@ -15,12 +15,11 @@