launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/bug-784948 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #784948 in Launchpad itself: "PersonNotificationManager.sendNotifications crashes when logging notification of a team with no preferredemail"
  https://bugs.launchpad.net/launchpad/+bug/784948

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/bug-784948/+merge/61503

12:19:03 < wgrant> Hmm.
12:19:10 < wgrant> can_send will look for team admins if they exist.
12:19:19 < wgrant> But the log message always uses preferredemail.
-- 
https://code.launchpad.net/~wgrant/launchpad/bug-784948/+merge/61503
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/bug-784948 into lp:launchpad.

=== modified file 'lib/lp/registry/scripts/personnotification.py'
--- lib/lp/registry/scripts/personnotification.py	2011-03-22 22:49:13 +0000
+++ lib/lp/registry/scripts/personnotification.py	2011-05-19 04:01:06 +0000
@@ -44,9 +44,7 @@
                     "%s has no email address." % person.name)
                 continue
             self.logger.info(
-                "Sending notification to %s <%s>."
-                % (person.name,
-                   removeSecurityProxy(person).preferredemail.email))
+                "Sending notification to %s." % person.name)
             notification.send()
             notifications_sent = True
             # Commit after each email sent, so that we won't re-mail the

=== modified file 'lib/lp/registry/tests/test_personnotification.py'
--- lib/lp/registry/tests/test_personnotification.py	2011-04-06 23:48:44 +0000
+++ lib/lp/registry/tests/test_personnotification.py	2011-05-19 04:01:06 +0000
@@ -98,6 +98,15 @@
         self.assertEqual([notification], unsent)
         self.assertEqual(None, notification.date_emailed)
 
+    def test_sendNotifications_sent_to_team_admins(self):
+        team = self.factory.makeTeam()
+        self.assertIs(None, team.preferredemail)
+        notification = self.notification_set.addNotification(
+            team, 'subject', 'body')
+        unsent = self.manager.sendNotifications()
+        self.assertEqual(None, unsent)
+        self.failIf(notification.date_emailed is None)
+
     def test_purgeNotifications_old(self):
         user = self.factory.makePerson()
         notification = self.notification_set.addNotification(