launchpad-reviewers team mailing list archive

[Steve Kowalik <stevenk@xxxxxxxxxx>]

Steve Kowalik has proposed merging lp:~stevenk/launchpad/reject-changes-file-no-email into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #796705 in Launchpad itself: "Rejected build upload OOPS"
  https://bugs.launchpad.net/launchpad/+bug/796705

For more details, see:
https://code.launchpad.net/~stevenk/launchpad/reject-changes-file-no-email/+merge/64523

As the linked bug report shows, reject_changes_file() does not like it at all if the uploader of the changes file has no preferred email. I've added a test, and reject_changes_file() now bails if there are no recipients.
-- 
https://code.launchpad.net/~stevenk/launchpad/reject-changes-file-no-email/+merge/64523
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/reject-changes-file-no-email into lp:launchpad.

=== modified file 'lib/lp/soyuz/adapters/notification.py'
--- lib/lp/soyuz/adapters/notification.py	2011-06-03 07:50:12 +0000
+++ lib/lp/soyuz/adapters/notification.py	2011-06-14 10:12:35 +0000
@@ -65,7 +65,10 @@
     body = template % information
     to_addrs = get_recipients(
         blamer, archive, distroseries, logger, changes=changes)
-    logger.debug("Sending rejection email.")
+    debug(logger, "Sending rejection email.")
+    if not to_addrs:
+        debug(logger, "No recipients have a preferred email.")
+        return
     send_mail(None, archive, to_addrs, subject, body, False, logger=logger)
 
 

=== modified file 'lib/lp/soyuz/adapters/tests/test_notification.py'
--- lib/lp/soyuz/adapters/tests/test_notification.py	2011-06-07 04:16:50 +0000
+++ lib/lp/soyuz/adapters/tests/test_notification.py	2011-06-14 10:12:35 +0000
@@ -6,9 +6,11 @@
 from canonical.testing.layers import LaunchpadZopelessLayer
 from lp.archivepublisher.utils import get_ppa_reference
 from lp.services.mail.sendmail import format_address_for_person
+from lp.services.log.logger import BufferLogger
 from lp.soyuz.adapters.notification import (
     calculate_subject,
     fetch_information,
+    reject_changes_file,
     person_to_email,
     notify,
     )
@@ -128,3 +130,19 @@
         body = notification.as_string()
         self.assertEqual(person_to_email(person), notification['To'])
         self.assertIn('Rejected by archive administrator.\n\n* Foo!\n', body)
+
+    def test_reject_changes_file_no_email(self):
+        # If we are rejecting a mail, and the person to notify has no
+        # preferred email, we should return early.
+        archive = self.factory.makeArchive()
+        distroseries = self.factory.makeDistroSeries()
+        uploader = self.factory.makePerson()
+        removeSecurityProxy(uploader).preferredemail = None
+        email = '%s <foo@xxxxxxxxxxx>' % uploader.displayname
+        changes = {'Changed-By': email, 'Maintainer': email}
+        logger = BufferLogger()
+        reject_changes_file(
+            uploader, '/tmp/changes', changes, archive, distroseries, '',
+            logger=logger)
+        self.assertIn(
+            'No recipients have a preferred email.', logger.getLogBuffer())