diff --git a/mail_tracking/demo/demo.xml b/mail_tracking/demo/demo.xml index e3bfd311f..aca804e4e 100644 --- a/mail_tracking/demo/demo.xml +++ b/mail_tracking/demo/demo.xml @@ -2,6 +2,31 @@ + + + res.partner + + comment + + acc@testmail.com,asusteK@yourcompany.example.com,thinkbig@yourcompany.example.com + 1 + This is a message with CC

]]> + wood.corner26@example.com + + + Message with CC + + + + Message with CC + + + asusteK@yourcompany.example.com + demo@yourcompany.example.com + sent + + + res.partner @@ -20,8 +45,8 @@ Failed Message - res1@yourcompany.example.com - demo@yourcompany.example.com + demo@yourcompany.example.com + res1@yourcompany.example.com error @@ -44,8 +69,32 @@ Failed Message - res10@yourcompany.example.com - demo@yourcompany.example.com + demo@yourcompany.example.com + res10@yourcompany.example.com + error + + + + + + res.partner + + comment + + 1 + This is another failed message

]]> + admin@example.com + + + Failed Message + + + + Failed Message + + + demo@yourcompany.example.com + admin@example.com error diff --git a/mail_tracking/models/mail_composer.py b/mail_tracking/models/mail_composer.py index 851c150fc..add3f46ea 100644 --- a/mail_tracking/models/mail_composer.py +++ b/mail_tracking/models/mail_composer.py @@ -12,8 +12,8 @@ class MailComposer(models.TransientModel): @api.multi def send_mail(self, auto_commit=False): - """ This method marks as reviewed the message when using the 'Retry' - option in the mail_failed_message widget""" + """This method marks as reviewed the message when using the 'Retry' + option in the mail_failed_message widget""" message = self.env['mail.message'].browse( self._context.get('message_id')) if message.exists(): @@ -29,7 +29,10 @@ class MailComposer(models.TransientModel): @api.model def get_record_data(self, values): + """Overwrite 'partner_ids' if enable 'hide_followers' on mail + composer""" values = super(MailComposer, self).get_record_data(values) + # Only use selected partners without followers if self._context.get('default_hide_followers', False): values['partner_ids'] = [ (6, 0, self._context.get('default_partner_ids', list())) diff --git a/mail_tracking/models/mail_mail.py b/mail_tracking/models/mail_mail.py index 930603919..2156d5448 100644 --- a/mail_tracking/models/mail_mail.py +++ b/mail_tracking/models/mail_mail.py @@ -11,6 +11,7 @@ class MailMail(models.Model): _inherit = 'mail.mail' def _tracking_email_prepare(self, partner, email): + """Prepare email.tracking.email record values""" ts = time.time() dt = datetime.utcfromtimestamp(ts) email_to_list = email.get('email_to', []) @@ -27,6 +28,8 @@ class MailMail(models.Model): } def send_get_email_dict(self, partner=None): + """Creates the mail.tracking.email record and adds the image tracking + to the email""" email = super(MailMail, self).send_get_email_dict(partner=partner) vals = self._tracking_email_prepare(partner, email) tracking_email = self.env['mail.tracking.email'].sudo().create(vals) diff --git a/mail_tracking/models/mail_message.py b/mail_tracking/models/mail_message.py index 8a9cf68fc..2005c5fe9 100644 --- a/mail_tracking/models/mail_message.py +++ b/mail_tracking/models/mail_message.py @@ -22,12 +22,28 @@ class MailMessage(models.Model): " to filter tracking issues", default=False, ) + is_failed_message = fields.Boolean(compute="_compute_is_failed_message") - @api.model def get_failed_states(self): + """The 'failed' states of the message""" return {'error', 'rejected', 'spam', 'bounced', 'soft-bounced'} + @api.depends('mail_tracking_needs_action', 'author_id', 'partner_ids', + 'mail_tracking_ids') + def _compute_is_failed_message(self): + """Compute 'is_failed_message' field for the active user""" + failed_states = self.get_failed_states() + for record in self: + needs_action = record.mail_tracking_needs_action + involves_me = record.env.user.partner_id.id in ( + record.author_id | record.partner_ids).ids + has_failed_trackings = failed_states.intersection( + record.mapped("mail_tracking_ids.state")) + record.is_failed_message = bool( + needs_action and involves_me and has_failed_trackings) + def _tracking_status_map_get(self): + """Map tracking states to be used in chatter""" return { 'False': 'waiting', 'error': 'error', @@ -43,6 +59,7 @@ class MailMessage(models.Model): } def _partner_tracking_status_get(self, tracking_email): + """Determine tracking status""" tracking_status_map = self._tracking_status_map_get() status = 'unknown' if tracking_email: @@ -51,12 +68,23 @@ class MailMessage(models.Model): return status def _partner_tracking_status_human_get(self, status): + """Translations for tracking statuses to be used on qweb""" statuses = {'waiting': _('Waiting'), 'error': _('Error'), 'sent': _('Sent'), 'delivered': _('Delivered'), 'opened': _('Opened'), 'unknown': _('Unknown')} return _("Status: %s") % statuses[status] + @api.model + def _get_error_description(self, tracking): + """Translations for error descriptions to be used on qweb""" + descriptions = { + 'no_recipient': _("The partner doesn't have a defined email"), + } + return descriptions.get(tracking.error_type, + tracking.error_description) + def tracking_status(self): + """Generates a complete status tracking of the messages by partner""" res = {} for message in self: partner_trackings = [] @@ -79,6 +107,9 @@ class MailMessage(models.Model): 'status': status, 'status_human': self._partner_tracking_status_human_get(status), + 'error_type': tracking.error_type, + 'error_description': + self._get_error_description(tracking), 'tracking_id': tracking.id, 'recipient': recipient, 'partner_id': tracking.partner_id.id, @@ -94,81 +125,80 @@ class MailMessage(models.Model): partners |= message.needaction_partner_ids # Remove recipients already included partners -= partners_already + tracking_unkown_values = { + 'status': 'unknown', + 'status_human': self._partner_tracking_status_human_get( + 'unknown'), + 'error_type': False, + 'error_description': False, + 'tracking_id': False, + } for partner in partners: # If there is partners not included, then status is 'unknown' - # Because can be an Cc recipient + # and perhaps a Cc recipient isCc = False if partner.email in email_cc_list: email_cc_list.discard(partner.email) isCc = True - partner_trackings.append({ - 'status': 'unknown', - 'status_human': - self._partner_tracking_status_human_get('unknown'), - 'tracking_id': False, + tracking_unkown_values.update({ 'recipient': partner.name, 'partner_id': partner.id, 'isCc': isCc, }) + partner_trackings.append(tracking_unkown_values.copy()) for email in email_cc_list: # If there is Cc without partner - partner_trackings.append({ - 'status': 'unknown', - 'status_human': - self._partner_tracking_status_human_get('unknown'), - 'tracking_id': False, + tracking_unkown_values.update({ 'recipient': email, 'partner_id': False, 'isCc': True, }) - res[message.id] = partner_trackings - return res - - @api.multi - def _get_failed_message(self): - res = {} - for message in self: - res.update({ - message.id: message.mail_tracking_needs_action - and bool(message.mail_tracking_ids.filtered( - lambda x: x.state in self.get_failed_states())) - }) + partner_trackings.append(tracking_unkown_values.copy()) + res[message.id] = { + 'partner_trackings': partner_trackings, + 'is_failed_message': message.is_failed_message, + } return res @api.model def _message_read_dict_postprocess(self, messages, message_tree): + """Preare values to be used by the chatter widget""" res = super(MailMessage, self)._message_read_dict_postprocess( messages, message_tree) mail_message_ids = {m.get('id') for m in messages if m.get('id')} mail_messages = self.browse(mail_message_ids) - partner_trackings = mail_messages.tracking_status() - failed_message = mail_messages._get_failed_message() + tracking_statuses = mail_messages.tracking_status() for message_dict in messages: mail_message_id = message_dict.get('id', False) if mail_message_id: - message_dict.update({ - 'partner_trackings': partner_trackings[mail_message_id], - 'failed_message': failed_message[mail_message_id], - }) + message_dict.update(tracking_statuses[mail_message_id]) return res @api.model def _prepare_dict_failed_message(self, message): + """Preare values to be used by the chatter widget""" failed_trackings = message.mail_tracking_ids.filtered( lambda x: x.state in self.get_failed_states()) failed_partners = failed_trackings.mapped('partner_id') failed_recipients = failed_partners.name_get() + if message.author_id: + author = message.author_id.name_get()[0] + else: + author = (-1, _('-Unknown Author-')) return { 'id': message.id, 'date': message.date, - 'author_id': message.author_id.name_get()[0], + 'author': author, 'body': message.body, 'failed_recipients': failed_recipients, } @api.multi def get_failed_messages(self): - return [self._prepare_dict_failed_message(msg) for msg in self] + """Returns the list of failed messages to be used by the + failed_messages widget""" + return [self._prepare_dict_failed_message(msg) + for msg in self.sorted('date', reverse=True)] @api.multi def toggle_tracking_status(self): @@ -177,19 +207,25 @@ class MailMessage(models.Model): self.mail_tracking_needs_action = not self.mail_tracking_needs_action return self.mail_tracking_needs_action + @api.model def _get_failed_message_domain(self): - return [ - ('mail_tracking_ids.state', 'in', list(self.get_failed_states())), - ('mail_tracking_needs_action', '=', True) + """Returns the domain used to filter messages on discuss thread""" + domain = self.env['mail.thread']._get_failed_message_domain() + domain += [ + '|', + ('partner_ids', 'in', [self.env.user.partner_id.id]), + ('author_id', '=', self.env.user.partner_id.id), ] + return domain @api.model def get_failed_count(self): - """ Gets the number of failed messages """ + """ Gets the number of failed messages used on discuss mailbox item""" return self.search_count(self._get_failed_message_domain()) @api.model def message_fetch(self, domain, limit=20): + """Force domain to be used on discuss thread messages""" # HACK: Because can't modify the domain in discuss JS to search the # failed messages we force the change here to clean it of # not valid criterias @@ -199,6 +235,7 @@ class MailMessage(models.Model): @api.multi def message_format(self): + """Adds 'failed_recipients' to display on 'failed_messages' widget""" message_values = super().message_format() for message in message_values: message_id = self.browse(message['id']) @@ -213,9 +250,9 @@ class MailMessage(models.Model): @api.multi def _notify(self, force_send=False, send_after_commit=True, user_signature=True): + """Force not adds followers to email (used when retry a message)""" self_sudo = self.sudo() - hide_followers = self_sudo._context.get('default_hide_followers', - False) + hide_followers = self_sudo._context.get('default_hide_followers') if hide_followers: # HACK: Because Odoo uses subtype to found message followers # whe modify it to False to avoid include them. diff --git a/mail_tracking/models/mail_thread.py b/mail_tracking/models/mail_thread.py index 98c3c7eb9..c532f82d4 100644 --- a/mail_tracking/models/mail_thread.py +++ b/mail_tracking/models/mail_thread.py @@ -14,14 +14,28 @@ class MailThread(models.AbstractModel): 'mail.message', 'res_id', string='Failed Messages', domain=lambda self: [('model', '=', self._name)] - + self.env['mail.message']._get_failed_message_domain(), + + self._get_failed_message_domain(), auto_join=True) + def _get_failed_message_domain(self): + """Domain used to display failed messages on the 'failed_messages' + widget""" + failed_states = self.env['mail.message'].get_failed_states() + return [ + ('mail_tracking_needs_action', '=', True), + ('mail_tracking_ids.state', 'in', list(failed_states)), + ] + @api.multi @api.returns('self', lambda value: value.id) def message_post(self, body='', subject=None, message_type='notification', subtype=None, parent_id=False, attachments=None, content_subtype='html', **kwargs): + """Adds CC recipient to the message. + + Because Odoo implementation avoid store cc recipients we ensure that + this information its written into the mail.message record. + """ new_message = super().message_post( body=body, subject=subject, message_type=message_type, subtype=subtype, parent_id=parent_id, attachments=attachments, @@ -69,7 +83,7 @@ class MailThread(models.AbstractModel): res = super().fields_view_get( view_id=view_id, view_type=view_type, toolbar=toolbar, submenu=submenu) - if view_type != 'search' and view_type != 'form': + if view_type not in {'search', 'form'}: return res doc = etree.XML(res['arch']) if view_type == 'search': diff --git a/mail_tracking/models/mail_tracking_email.py b/mail_tracking/models/mail_tracking_email.py index 4c885c766..75c411bb4 100644 --- a/mail_tracking/models/mail_tracking_email.py +++ b/mail_tracking/models/mail_tracking_email.py @@ -161,7 +161,9 @@ class MailTrackingEmail(models.Model): @api.depends('recipient') def _compute_recipient_address(self): for email in self: - if email.recipient: + is_empty_recipient = (not email.recipient + or '' in email.recipient) + if not is_empty_recipient: matches = re.search(r'<(.*@.*)>', email.recipient) if matches: email.recipient_address = matches.group(1).lower() @@ -215,12 +217,24 @@ class MailTrackingEmail(models.Model): @api.multi def smtp_error(self, mail_server, smtp_server, exception): - self.sudo().write({ - 'error_smtp_server': tools.ustr(smtp_server), - 'error_type': exception.__class__.__name__, - 'error_description': tools.ustr(exception), + values = { 'state': 'error', - }) + } + IrMailServer = self.env['ir.mail_server'] + if str(exception) == IrMailServer.NO_VALID_RECIPIENT \ + and not self.recipient_address: + values.update({ + 'error_type': 'no_recipient', + 'error_description': + "The partner doesn't have a defined email", + }) + else: + values.update({ + 'error_smtp_server': tools.ustr(smtp_server), + 'error_type': exception.__class__.__name__, + 'error_description': tools.ustr(exception), + }) + self.sudo().write(values) return True @api.multi diff --git a/mail_tracking/readme/USAGE.rst b/mail_tracking/readme/USAGE.rst index 2a6ed40f1..9afd45709 100644 --- a/mail_tracking/readme/USAGE.rst +++ b/mail_tracking/readme/USAGE.rst @@ -31,6 +31,9 @@ These are all available status icons: .. |cc| image:: ../static/src/img/cc.png :width: 10px +.. |noemail| image:: ../static/src/img/no_email.png + :width: 10px + |unknown| **Unknown**: No email tracking info available. Maybe this notified partner has 'Receive Inbox Notifications by Email' == 'Never' |waiting| **Waiting**: Waiting to be sent @@ -45,6 +48,8 @@ These are all available status icons: |cc| **Cc**: It's a Carbon-Copy recipient. Can't know the status so is 'Unknown' +|noemail| **No Email**: The partner doesn't have a defined email + When the message generates and 'error' status, it will apear on discuss 'Failed' channel. Any view that uses 'mail_thread' widget can show the failed messages @@ -57,3 +62,10 @@ too. * Chatter .. image:: ../static/img/failed_message_widget.png + +You can use "Failed sent messages" filter present in all views to show records +with messages in failed status and that needs an user action. + +* Filter + + .. image:: ../static/img/failed_message_filter.png diff --git a/mail_tracking/static/img/failed_message_filter.png b/mail_tracking/static/img/failed_message_filter.png new file mode 100644 index 000000000..7843d140e Binary files /dev/null and b/mail_tracking/static/img/failed_message_filter.png differ diff --git a/mail_tracking/static/src/img/no_email.png b/mail_tracking/static/src/img/no_email.png new file mode 100644 index 000000000..f35c6c057 Binary files /dev/null and b/mail_tracking/static/src/img/no_email.png differ diff --git a/mail_tracking/static/src/js/failed_message.js b/mail_tracking/static/src/js/failed_message.js index 305353531..576adbcf4 100644 --- a/mail_tracking/static/src/js/failed_message.js +++ b/mail_tracking/static/src/js/failed_message.js @@ -126,7 +126,6 @@ odoo.define('mail_tracking.FailedMessage', function (require) { }, _openComposer: function (context) { - var self = this; var failed_msg = chat_manager.get_message(context.message_id); this.do_action({ type: 'ir.actions.act_window', @@ -138,11 +137,8 @@ odoo.define('mail_tracking.FailedMessage', function (require) { context: context, }, { on_close: function () { - self.trigger('need_refresh'); - chat_manager.get_messages({ - model: failed_msg.model, - res_id: failed_msg.res_id, - }); + chat_manager.bus.trigger('update_message', failed_msg); + core.bus.trigger('force_update_message', failed_msg); }, }).then(this.trigger.bind(this, 'close_composer')); }, @@ -178,7 +174,7 @@ odoo.define('mail_tracking.FailedMessage', function (require) { event.preventDefault(); var message_id = $(event.currentTarget).data('message-id'); var failed_msg = chat_manager.get_message(message_id); - this._rpc({ + return this._rpc({ model: 'mail.message', method: 'toggle_tracking_status', args: [[message_id]], @@ -233,7 +229,8 @@ odoo.define('mail_tracking.FailedMessage', function (require) { Object.defineProperties(msg, { is_failed: property_descr("channel_failed"), }); - msg.is_failed = data.failed_message; + + msg.is_failed = data.is_failed_message; msg.failed_recipients = data.failed_recipients; return msg; }; @@ -291,7 +288,7 @@ odoo.define('mail_tracking.FailedMessage', function (require) { msg.date = moment(time.auto_str_to_date(msg.date)); msg.hour = time_from_now(msg.date); }); - return _.sortBy(messages, 'date'); + return messages; }); } @@ -331,7 +328,7 @@ odoo.define('mail_tracking.FailedMessage', function (require) { init: function () { this._super.apply(this, arguments); - this.failed_messages = this.record.specialData[this.name]; + this.failed_messages = this.record.specialData[this.name] || []; }, _render: function () { if (this.failed_messages.length) { @@ -348,7 +345,7 @@ odoo.define('mail_tracking.FailedMessage', function (require) { }, _reset: function (record) { this._super.apply(this, arguments); - this.failed_messages = this.record.specialData[this.name]; + this.failed_messages = this.record.specialData[this.name] || []; this.res_id = record.res_id; }, @@ -456,6 +453,7 @@ odoo.define('mail_tracking.FailedMessage', function (require) { keepChanges: true, }); } else { + // Workaround to avoid trigger reload event twice. this._super.apply(this, arguments); } }, diff --git a/mail_tracking/static/src/xml/client_action.xml b/mail_tracking/static/src/xml/client_action.xml index c748490c0..582e4df8a 100644 --- a/mail_tracking/static/src/xml/client_action.xml +++ b/mail_tracking/static/src/xml/client_action.xml @@ -1,5 +1,5 @@ - + diff --git a/mail_tracking/static/src/xml/failed_message.xml b/mail_tracking/static/src/xml/failed_message.xml index 367cb4098..f90e59794 100644 --- a/mail_tracking/static/src/xml/failed_message.xml +++ b/mail_tracking/static/src/xml/failed_message.xml @@ -4,20 +4,28 @@
- +
- + - -
+ - + + + Mark Reviewed + + + Retry + + +
Failed Recipients: - - + - -
-
- - Mark Reviewed - - - Retry - -
diff --git a/mail_tracking/static/src/xml/mail_tracking.xml b/mail_tracking/static/src/xml/mail_tracking.xml index 92fe59218..e0df04ce9 100644 --- a/mail_tracking/static/src/xml/mail_tracking.xml +++ b/mail_tracking/static/src/xml/mail_tracking.xml @@ -22,7 +22,8 @@ - + + @@ -64,9 +65,11 @@ + + + t-att-title="title_status"> diff --git a/mail_tracking/tests/test_mail_tracking.py b/mail_tracking/tests/test_mail_tracking.py index 0d124cf1e..a676743c9 100644 --- a/mail_tracking/tests/test_mail_tracking.py +++ b/mail_tracking/tests/test_mail_tracking.py @@ -115,6 +115,29 @@ class TestMailTracking(TransactionCase): tracking_email.event_create('open', metadata) self.assertEqual(tracking_email.state, 'opened') + def test_message_post_partner_no_email(self): + # Create message with recipient without defined email + self.recipient.write({'email': ''}) + message = self.env['mail.message'].create({ + 'subject': 'Message test', + 'author_id': self.sender.id, + 'email_from': self.sender.email, + 'message_type': 'comment', + 'model': 'res.partner', + 'res_id': self.recipient.id, + 'partner_ids': [(4, self.recipient.id)], + 'body': '

This is a test message

', + }) + # Search tracking created + tracking_email = self.env['mail.tracking.email'].search([ + ('mail_message_id', '=', message.id), + ('partner_id', '=', self.recipient.id), + ]) + # No email should generate a error state: no_recipient + self.assertEqual(tracking_email.state, 'error') + self.assertEqual(tracking_email.error_type, 'no_recipient') + self.assertFalse(self.recipient.email_bounced) + def _check_partner_trackings(self, message): message_dict = message.message_format()[0] self.assertEqual(len(message_dict['partner_trackings']), 3) @@ -192,6 +215,10 @@ class TestMailTracking(TransactionCase): self.assertFalse(tracking.mail_message_id.mail_tracking_needs_action) self.assertTrue( self.env['mail.message'].get_failed_count() < failed_count) + # No author_id + tracking.mail_message_id.author_id = False + values = tracking.mail_message_id.get_failed_messages()[0] + self.assertEqual(values['author'][0], -1) def mail_send(self, recipient): mail = self.env['mail.mail'].create({