Merge pull request #936 from odoo-dev/master-send-failmail-warning-avo

[IMP] mail: store the reason when failing to send an email; usability improvements in configuration, now showing the number of failed emails + links for easier configuration.

6 files changed, 36 insertions, 6 deletions

diff --git a/addons/mail/mail_mail.py b/addons/mail/mail_mail.py
index aeea2c0b1a5..3409c983bd8 100644
--- a/addons/mail/mail_mail.py
+++ b/addons/mail/mail_mail.py
@@ -30,6 +30,7 @@ from openerp.addons.base.ir.ir_mail_server import MailDeliveryException
 from openerp.osv import fields, osv
 from openerp.tools.safe_eval import safe_eval as eval
 from openerp.tools.translate import _
+import openerp.tools as tools
 
 _logger = logging.getLogger(__name__)
 
@@ -60,6 +61,7 @@ class mail_mail(osv.Model):
         'email_cc': fields.char('Cc', help='Carbon copy message recipients'),
         'body_html': fields.text('Rich-text Contents', help="Rich-text/HTML message"),
         'headers': fields.text('Headers', copy=False),
+        'failure_reason': fields.text('Failure Reason', help="Failure reason. This is usually the exception thrown by the email server, stored to ease the debugging of mailing issues.", readonly=1),
         # Auto-detected based on create() - if 'mail_message_id' was passed then this mail is a notification
         # and during unlink() we will not cascade delete the parent and its attachments
         'notification': fields.boolean('Is Notification',
@@ -296,7 +298,7 @@ class mail_mail(osv.Model):
                     mail.write({'state': 'sent', 'message_id': res})
                     mail_sent = True
                 else:
-                    mail.write({'state': 'exception'})
+                    mail.write({'state': 'exception', 'failure_reason': _('Error without exception. Probably due do sending an email without computed recipients.')})
                     mail_sent = False
 
                 # /!\ can't use mail.state here, as mail.refresh() will cause an error
@@ -312,8 +314,9 @@ class mail_mail(osv.Model):
                                   mail.id, mail.message_id)
                 raise
             except Exception as e:
-                _logger.exception('failed sending mail.mail %s', mail.id)
-                mail.write({'state': 'exception'})
+                failure_reason = tools.ustr(e)
+                _logger.exception('failed sending mail (id: %s) due to %s', mail.id, failure_reason)
+                mail.write({'state': 'exception', 'failure_reason': failure_reason})
                 self._postprocess_sent_message(cr, uid, mail, context=context, mail_sent=False)
                 if raise_exception:
                     if isinstance(e, AssertionError):
diff --git a/addons/mail/mail_mail_view.xml b/addons/mail/mail_mail_view.xml
index 233f407b8e1..bf516840f47 100644
--- a/addons/mail/mail_mail_view.xml
+++ b/addons/mail/mail_mail_view.xml
@@ -52,6 +52,9 @@
                             <page string="Attachments">
                                 <field name="attachment_ids"/>
                             </page>
+                            <page string="Failure Reason" attrs="{'invisible': [('state', '!=', 'exception')]}">
+                                <field name="failure_reason"/>
+                            </page>
                         </notebook>
                     </sheet>
                 </form>
@@ -116,7 +119,7 @@
             <field name="res_model">mail.mail</field>
             <field name="view_type">form</field>
             <field name="view_mode">tree,form</field>
-            <field name="context">{'search_default_outgoing': 1}</field>
+            <field name="context">{}</field>
             <field name="search_view_id" ref="view_mail_search"/>
         </record>
 
diff --git a/addons/mail/res_config.py b/addons/mail/res_config.py
index 2c94b432dc3..e238d9ab105 100644
--- a/addons/mail/res_config.py
+++ b/addons/mail/res_config.py
@@ -20,7 +20,9 @@
 ##############################################################################
 
 import urlparse
+import datetime
 
+from openerp.tools import DEFAULT_SERVER_DATETIME_FORMAT
 from openerp.osv import osv, fields
 
 
@@ -28,11 +30,18 @@ class project_configuration(osv.TransientModel):
     _inherit = 'base.config.settings'
 
     _columns = {
+        'fail_counter': fields.integer('Fail Mail', readonly=True),
         'alias_domain': fields.char('Alias Domain',
                                      help="If you have setup a catch-all email domain redirected to "
                                           "the OpenERP server, enter the domain name here."),
     }
 
+    def get_default_fail_counter(self, cr, uid, ids, context=None):
+        previous_date = datetime.datetime.now() - datetime.timedelta(days=30)
+        return {
+            'fail_counter': self.pool.get('mail.mail').search(cr, uid, [('date', '>=', previous_date.strftime(DEFAULT_SERVER_DATETIME_FORMAT)), ('state', '=', 'exception')], count=True, context=context),
+        }
+
     def get_default_alias_domain(self, cr, uid, ids, context=None):
         alias_domain = self.pool.get("ir.config_parameter").get_param(cr, uid, "mail.catchall.domain", default=None, context=context)
         if alias_domain is None:
diff --git a/addons/mail/res_config_view.xml b/addons/mail/res_config_view.xml
index d3797ba2dcc..aa0b9f176a3 100644
--- a/addons/mail/res_config_view.xml
+++ b/addons/mail/res_config_view.xml
@@ -6,6 +6,15 @@
             <field name="model">base.config.settings</field>
             <field name="inherit_id" ref="base_setup.view_general_configuration"/>
             <field name="arch" type="xml">
+                <xpath expr="//button[@string='Configure outgoing email servers']" position='after'>
+                    <button class="oe_inline oe_link" style="display: inline-block;" name= "%(action_view_mail_mail)d" type="action" 
+                        context="{'search_default_exception': 1, 'search_default_outgoing': 0}"
+                        attrs="{'invisible': [('fail_counter','=',0)]}">
+                        <span> -- </span>
+                        <i class="fa fa-exclamation-triangle"></i> <field class="oe_inline" name="fail_counter"/>
+                        <span>failed emails</span>
+                    </button>
+                </xpath>
                 <xpath expr="//div[@name='email']" position='inside'>
                     <div>
                         <label for="alias_domain" class="oe_inline"/>
diff --git a/addons/mail/static/src/css/mail.css b/addons/mail/static/src/css/mail.css
index 8d461af8586..e7da638b256 100644
--- a/addons/mail/static/src/css/mail.css
+++ b/addons/mail/static/src/css/mail.css
@@ -93,7 +93,6 @@
 .openerp .oe_mail .oe_msg .oe_msg_content .oe_msg_body .oe_mail_cleaned {
     display: none;
 }
-
 /* a) Indented Messages */
 
 .openerp .oe_mail .oe_msg_indented{
diff --git a/addons/mail/tests/test_mail_features.py b/addons/mail/tests/test_mail_features.py
index fdd87bbcfab..d148a8aac4c 100644
--- a/addons/mail/tests/test_mail_features.py
+++ b/addons/mail/tests/test_mail_features.py
@@ -629,7 +629,8 @@ class test_mail(TestMail):
         mail_compose.send_mail(cr, user_raoul.id, [compose_id], {'mail_post_autofollow': True, 'mail_create_nosubscribe': True})
         group_pigs.refresh()
         message = group_pigs.message_ids[0]
-
+        # Test: mail_mail: notifications have been deleted
+        self.assertFalse(self.mail_mail.search(cr, uid, [('mail_message_id', '=', message.id)]),'message_send: mail.mail message should have been auto-deleted!')
         # Test: mail.group: followers (c and d added by auto follow key; raoul not added by nosubscribe key)
         pigs_pids = [p.id for p in group_pigs.message_follower_ids]
         test_pids = [self.partner_admin_id, p_b_id, p_c_id, p_d_id]
@@ -711,6 +712,12 @@ class test_mail(TestMail):
 
         # Test: Pigs and Bird did receive their message
         test_msg_ids = self.mail_message.search(cr, uid, [], limit=2)
+        mail_ids = self.mail_mail.search(cr, uid, [('mail_message_id', '=', message2.id)])
+        mail_record_id = self.mail_mail.browse(cr, uid, mail_ids)[0]
+        self.assertTrue(mail_record_id, "'message_send: mail.mail message should have in processing mail queue'" )
+        #check mass mail state...
+        test_mail_ids = self.mail_mail.search(cr, uid, [('state', '=', 'exception')])
+        self.assertNotIn(mail_ids, test_mail_ids, 'compose wizard: Mail sending Failed!!')
         self.assertIn(message1.id, test_msg_ids, 'compose wizard: Pigs did not receive its mass mailing message')
         self.assertIn(message2.id, test_msg_ids, 'compose wizard: Bird did not receive its mass mailing message')