1.2.2dev: fix a regression of #12658, ticket notification not sending for authenticated users without email (closes #12759)

git-svn-id: http://trac.edgewall.org/intertrac/log:/branches/1.2-stable@15757 af82e41b-90c4-0310-8c96-b1721e28e2e2

Author: jomae

trac/notification/mail.py

@@ -204,7 +204,7 @@ def smtp_default_domain(self):
         return self.notify_sys.smtp_default_domain
 
     def match_recipient(self, address):
-        if not address:
+        if not address or address == 'anonymous':
             return None
 
         def is_email(address):
@@ -215,19 +215,17 @@ def is_email(address):
                 return False
             return True
 
-        if address == 'anonymous':
-            return None
-        sid = None
-        auth = 0
         if address in self.users:
             sid = address
             auth = 1
-            address = self.users[address][1]
-            if not address:
-                return sid, auth, None
-        elif not is_email(address) and self.nodomaddr_re.match(address):
+            address = (self.users[address][1] or '').strip() or sid
+        else:
+            sid = None
+            auth = 0
+
+        if not is_email(address) and self.nodomaddr_re.match(address):
             if self.use_short_addr:
-                return None, 0, address
+                return sid, auth, address
             if self.smtp_default_domain:
                 address = "%s@%s" % (address, self.smtp_default_domain)
             else:

trac/ticket/tests/notification.py

@@ -395,71 +395,82 @@ def run_bcc_feature(public_cc):
 
     def test_short_login(self):
         """Email addresses without a FQDN"""
-        def _test_short_login(enabled):
+        def _test_short_login(use_short_addr, username, address):
             config_subscriber(self.env, reporter=True)
             ticket = Ticket(self.env)
-            ticket['reporter'] = 'joeuser'
+            ticket['reporter'] = username
             ticket['summary'] = 'This is a summary'
             ticket.insert()
             # Be sure that at least one email address is valid, so that we
             # send a notification even if other addresses are not valid
             self.env.config.set('notification', 'smtp_always_cc',
-                                '[email protected]')
-            self.env.config.set('notification', 'use_short_addr', enabled)
+                                '[email protected], john')
+            self.env.config.set('notification', 'use_short_addr',
+                                'enabled' if use_short_addr else 'disabled')
             notify_ticket_created(self.env, ticket)
             message = notifysuite.smtpd.get_message()
+            recipients = set(notifysuite.smtpd.get_recipients())
             headers, body = parse_smtp_message(message)
             # Msg should always have a 'To' field
             self.assertEqual('undisclosed-recipients: ;', headers['To'])
             # Msg should have a 'Cc' field
             self.assertIn('Cc', headers)
-            cclist = [addr.strip() for addr in headers['Cc'].split(',')]
-            if enabled:
+            cclist = set(addr.strip() for addr in headers['Cc'].split(','))
+            if use_short_addr:
                 # Msg should be delivered to the reporter
-                self.assertIn(ticket['reporter'], cclist)
+                self.assertEqual(set([address, '[email protected]',
+                                      'john']), cclist)
+                self.assertEqual(set([address, '[email protected]',
+                                      'john']), recipients)
             else:
-                # Msg should not be delivered to joeuser
-                self.assertNotIn(ticket['reporter'], cclist)
-            # Msg should still be delivered to the always_cc list
-            self.assertIn(self.env.config.get('notification',
-                                              'smtp_always_cc'), cclist)
+                # Msg should not be delivered to the reporter
+                self.assertEqual(set(['[email protected]']), cclist)
+                self.assertEqual(set(['[email protected]']), recipients)
+
         # Validate with and without the short addr option enabled
-        for enable in False, True:
-            _test_short_login(enable)
+        self.env.insert_users([('bar', 'Bar User', ''),
+                               ('qux', 'Qux User', 'qux-mail')])
+        for use_short_addr in (False, True):
+            _test_short_login(use_short_addr, 'foo', 'foo')
+            _test_short_login(use_short_addr, 'bar', 'bar')
+            _test_short_login(use_short_addr, 'qux', 'qux-mail')
 
     def test_default_domain(self):
         """Default domain name"""
-        def _test_default_domain(enabled):
+        def _test_default_domain(enable):
             config_subscriber(self.env)
             self.env.config.set('notification', 'smtp_always_cc', '')
             ticket = Ticket(self.env)
-            ticket['cc'] = 'joenodom, [email protected]'
+            ticket['cc'] = 'joenodom, foo, bar, qux, [email protected]'
             ticket['summary'] = 'This is a summary'
             ticket.insert()
             # Be sure that at least one email address is valid, so that we
             # send a notification even if other addresses are not valid
             self.env.config.set('notification', 'smtp_always_cc',
                                 '[email protected]')
-            if enabled:
-                self.env.config.set('notification', 'smtp_default_domain',
-                                    'example.org')
+            self.env.config.set('notification', 'smtp_default_domain',
+                                'example.org' if enable else '')
             notify_ticket_created(self.env, ticket)
             message = notifysuite.smtpd.get_message()
             headers, body = parse_smtp_message(message)
             # Msg should always have a 'Cc' field
             self.assertIn('Cc', headers)
-            cclist = [addr.strip() for addr in headers['Cc'].split(',')]
-            self.assertIn('[email protected]', cclist)
-            self.assertIn('[email protected]', cclist)
-            if enabled:
-                self.assertEqual(3, len(cclist))
-                self.assertIn('[email protected]', cclist)
+            cclist = set(addr.strip() for addr in headers['Cc'].split(','))
+            if enable:
+                self.assertEqual(set(['[email protected]',
+                                      '[email protected]',
+                                      '[email protected]',
+                                      '[email protected]',
+                                      '[email protected]',
+                                      '[email protected]']), cclist)
             else:
-                self.assertEqual(2, len(cclist))
-                self.assertNotIn('joenodom', cclist)
+                self.assertEqual(set(['[email protected]',
+                                      '[email protected]']), cclist)
 
         # Validate with and without a default domain
-        for enable in False, True:
+        self.env.insert_users([('bar', 'Bar User', ''),
+                               ('qux', 'Qux User', 'qux-mail')])
+        for enable in (False, True):
             _test_default_domain(enable)
 
     def test_email_map(self):