Merge lp:~cjwatson/launchpad/bpb-basemailer into lp:launchpad

[Colin Watson]

Convert BinaryPackageBuild notifications to BaseMailer.

Convert BinaryPackageBuild notifications to BaseMailer.

This is a better layout for mail notification code in general, and gives us X-Launchpad-Message-Rationale and X-Launchpad-Notification-Type more or less for free. I'm doing this as part of my plan to make notifications more filterable by Gmail users, which relies on making more widespread use of BaseMailer.

[William Grant (wgrant)]

review: Approve (code)

=== modified file 'lib/lp/soyuz/doc/build-failedtoupload-workflow.txt'
--- lib/lp/soyuz/doc/build-failedtoupload-workflow.txt	2015-07-14 11:01:53 +0000
+++ lib/lp/soyuz/doc/build-failedtoupload-workflow.txt	2015-07-14 11:01:53 +0000
@@ -61,9 +61,9 @@
 
   >>> for build_notification in notifications:
   ...      build_notification['To']
-  '[e-mail]'
-  '[e-mail]'
-  '[e-mail]'
+  'Celso Providelo <[e-mail]>'
+  'Foo Bar <[e-mail]>'
+  'Mark Shuttleworth <[e-mail]>'
 
 Note that the generated notification contain the 'extra_info' content:
 
@@ -99,7 +99,8 @@
   If you want further information about this situation, feel free to
   contact a member of the Launchpad Buildd Administrators team.
   <BLANKLINE>
-  --
+  -- =
+  <BLANKLINE>
   i386 build of cdrkit 1.0 in ubuntu breezy-autotest RELEASE
   http://launchpad.dev/ubuntu/+source/cdrkit/1.0/+build/22
   <BLANKLINE>

=== modified file 'lib/lp/soyuz/emailtemplates/build-notification.txt'
--- lib/lp/soyuz/emailtemplates/build-notification.txt	2015-07-14 11:01:53 +0000
+++ lib/lp/soyuz/emailtemplates/build-notification.txt	2015-07-14 11:01:53 +0000
@@ -14,9 +14,3 @@
 
 If you want further information about this situation, feel free to
 contact a member of the Launchpad Buildd Administrators team.
-
---
-%(build_title)s
-%(build_url)s
-
-%(reason)s

=== added file 'lib/lp/soyuz/mail/binarypackagebuild.py'
--- lib/lp/soyuz/mail/binarypackagebuild.py	1970-01-01 00:00:00 +0000
+++ lib/lp/soyuz/mail/binarypackagebuild.py	2015-07-14 11:01:53 +0000
@@ -0,0 +1,241 @@
+# Copyright 2015 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+__metaclass__ = type
+__all__ = [
+    'BinaryPackageBuildMailer',
+    ]
+
+from collections import OrderedDict
+
+from zope.component import getUtility
+
+from lp.app.browser.tales import DurationFormatterAPI
+from lp.app.interfaces.launchpad import ILaunchpadCelebrities
+from lp.archivepublisher.utils import get_ppa_reference
+from lp.buildmaster.enums import BuildStatus
+from lp.services.config import config
+from lp.services.mail.basemailer import (
+    BaseMailer,
+    RecipientReason,
+    )
+from lp.services.mail.helpers import get_contact_email_addresses
+from lp.services.mail.mailwrapper import MailWrapper
+from lp.services.mail.sendmail import format_address
+from lp.services.webapp import canonical_url
+
+
+class BinaryPackageBuildRecipientReason(RecipientReason):
+
+    @classmethod
+    def forCreator(cls, creator, recipient):
+        header = cls.makeRationale("Creator", creator)
+        reason = (
+            "You are receiving this email because you created this version of "
+            "this package.")
+        return cls(creator, recipient, header, reason)
+
+    @classmethod
+    def forSigner(cls, signer, recipient):
+        header = cls.makeRationale("Signer", signer)
+        reason = (
+            "You are receiving this email because you signed this package.")
+        return cls(signer, recipient, header, reason)
+
+    @classmethod
+    def forBuilddAdmins(cls, buildd_admins, recipient):
+        header = cls.makeRationale("Buildd-Admin", buildd_admins)
+        # The team is always the same, so don't bother with %(lc_entity_is)s
+        # here.
+        reason = (
+            "You are receiving this email because you are a buildd "
+            "administrator.")
+        return cls(buildd_admins, recipient, header, reason)
+
+    @classmethod
+    def forArchiveOwner(cls, owner, recipient):
+        header = cls.makeRationale("Owner", owner)
+        reason = (
+            "You are receiving this email because %(lc_entity_is)s the owner "
+            "of this archive.")
+        return cls(owner, recipient, header, reason)
+
+    def getReason(self):
+        """See `RecipientReason`."""
+        return MailWrapper(width=72).format(
+            super(BinaryPackageBuildRecipientReason, self).getReason())
+
+
+class BinaryPackageBuildMailer(BaseMailer):
+
+    app = 'soyuz'
+
+    @classmethod
+    def forStatus(cls, build, extra_info=None):
+        """Create a mailer for notifying about package build status.
+
+        :param build: The relevant build.
+        """
+        # Circular import.
+        from lp.registry.model.person import get_recipients
+
+        recipients = OrderedDict()
+
+        # Currently there are 7038 SPR published in edgy which the creators
+        # have no preferredemail. They are the autosync ones (creator = katie,
+        # 3583 packages) and the untouched sources since we have migrated from
+        # DAK (the rest). We should not spam Debian maintainers.
+
+        # Please note that both the package creator and the package uploader
+        # will be notified of failures if:
+        #     * the 'notify_owner' flag is set
+        #     * the package build (failure) occurred in the original
+        #       archive.
+        creator = build.source_package_release.creator
+        package_was_not_copied = (
+            build.archive == build.source_package_release.upload_archive)
+
+        if package_was_not_copied and config.builddmaster.notify_owner:
+            if (build.archive.is_ppa and creator.inTeam(build.archive.owner)
+                or not build.archive.is_ppa):
+                # If this is a PPA, the package creator should only be
+                # notified if they are the PPA owner or in the PPA team.
+                # (see bug 375757)
+                # Non-PPA notifications inform the creator regardless.
+                for recipient in get_recipients(creator):
+                    if recipient not in recipients:
+                        reason = BinaryPackageBuildRecipientReason.forCreator(
+                            creator, recipient)
+                        recipients[recipient] = reason
+            dsc_key = build.source_package_release.dscsigningkey
+            if dsc_key:
+                for recipient in get_recipients(dsc_key.owner):
+                    if recipient not in recipients:
+                        reason = BinaryPackageBuildRecipientReason.forSigner(
+                            dsc_key.owner, recipient)
+                        recipients[recipient] = reason
+
+        if not build.archive.is_ppa:
+            buildd_admins = getUtility(ILaunchpadCelebrities).buildd_admin
+            for recipient in get_recipients(buildd_admins):
+                if recipient not in recipients:
+                    reason = BinaryPackageBuildRecipientReason.forBuilddAdmins(
+                        buildd_admins, recipient)
+                    recipients[recipient] = reason
+        else:
+            for recipient in get_recipients(build.archive.owner):
+                if recipient not in recipients:
+                    reason = BinaryPackageBuildRecipientReason.forArchiveOwner(
+                        build.archive.owner, recipient)
+                    recipients[recipient] = reason
+
+        # XXX cprov 2006-08-02: pending security recipients for SECURITY
+        # pocket build. We don't build SECURITY yet :(
+
+        fromaddress = format_address(
+            config.builddmaster.default_sender_name,
+            config.builddmaster.default_sender_address)
+        subject = "[Build #%d] %s" % (build.id, build.title)
+        if build.archive.is_ppa:
+            subject += " [%s]" % build.archive.reference
+        return cls(
+            subject, "build-notification.txt", recipients, fromaddress, build,
+            extra_info=extra_info)
+
+    def __init__(self, subject, template_name, recipients, from_address,
+                 build, extra_info=None):
+        super(BinaryPackageBuildMailer, self).__init__(
+            subject, template_name, recipients, from_address,
+            notification_type="package-build-status")
+        self.build = build
+        self.extra_info = extra_info
+
+    def _getHeaders(self, email):
+        """See `BaseMailer`."""
+        headers = super(BinaryPackageBuildMailer, self)._getHeaders(email)
+        build = self.build
+        headers.update({
+            "X-Launchpad-Archive": build.archive.reference,
+            "X-Launchpad-Build-State": build.status.name,
+            "X-Launchpad-Build-Component": build.current_component.name,
+            "X-Launchpad-Build-Arch": build.distro_arch_series.architecturetag,
+            # XXX cprov 2006-10-27: Temporary extra debug info about the
+            # SPR.creator in context, to be used during the service
+            # quarantine, notify_owner will be disabled to avoid *spamming*
+            # Debian people.
+            "X-Creator-Recipient": ",".join(get_contact_email_addresses(
+                build.source_package_release.creator)),
+            })
+        # The deprecated PPA reference header is included for Ubuntu PPAs to
+        # avoid breaking existing consumers.
+        if (build.archive.is_ppa and
+                build.archive.distribution.name == u'ubuntu'):
+            headers["X-Launchpad-PPA"] = get_ppa_reference(build.archive)
+        return headers
+
+    def _getTemplateParams(self, email, recipient):
+        params = super(BinaryPackageBuildMailer, self)._getTemplateParams(
+            email, recipient)
+        build = self.build
+        extra_info = self.extra_info
+
+        if build.archive.is_ppa:
+            source_url = "not available"
+        else:
+            source_url = canonical_url(build.distributionsourcepackagerelease)
+
+        # XXX cprov 2006-08-02: find out a way to glue parameters reported
+        # with the state in the build workflow, maybe by having an
+        # IBuild.statusReport property, which could also be used in the
+        # respective page template.
+        if build.status in (BuildStatus.NEEDSBUILD, BuildStatus.SUPERSEDED):
+            # untouched builds
+            buildduration = "not available"
+            buildlog_url = "not available"
+            builder_url = "not available"
+        elif build.status == BuildStatus.UPLOADING:
+            buildduration = "uploading"
+            buildlog_url = "see builder page"
+            builder_url = "not available"
+        elif build.status == BuildStatus.BUILDING:
+            # build in process
+            buildduration = "not finished"
+            buildlog_url = "see builder page"
+            builder_url = canonical_url(build.buildqueue_record.builder)
+        else:
+            # completed states (success and failure)
+            buildduration = DurationFormatterAPI(
+                build.duration).approximateduration()
+            buildlog_url = build.log_url
+            builder_url = canonical_url(build.builder)
+
+        if build.status == BuildStatus.FAILEDTOUPLOAD:
+            assert extra_info is not None, (
+                "Extra information is required for FAILEDTOUPLOAD "
+                "notifications.")
+            extra_info = "Upload log:\n%s" % extra_info
+        else:
+            extra_info = ""
+
+        params.update({
+            "source_name": build.source_package_release.name,
+            "source_version": build.source_package_release.version,
+            "architecturetag": build.distro_arch_series.architecturetag,
+            "build_state": build.status.title,
+            "build_duration": buildduration,
+            "buildlog_url": buildlog_url,
+            "builder_url": builder_url,
+            "build_title": build.title,
+            "build_url": canonical_url(build),
+            "source_url": source_url,
+            "extra_info": extra_info,
+            "archive_tag": build.archive.reference,
+            "component_tag": build.current_component.name,
+            })
+        return params
+
+    def _getFooter(self, params):
+        """See `BaseMailer`."""
+        return ("%(build_title)s\n"
+                "%(build_url)s\n\n"
+                "%(reason)s\n" % params)

=== modified file 'lib/lp/soyuz/model/binarypackagebuild.py'
--- lib/lp/soyuz/model/binarypackagebuild.py	2015-07-14 11:01:53 +0000
+++ lib/lp/soyuz/model/binarypackagebuild.py	2015-07-14 11:01:53 +0000
@@ -17,7 +17,6 @@
     attrgetter,
     itemgetter,
     )
-import textwrap
 
 import apt_pkg
 import pytz
@@ -47,10 +46,7 @@
 from zope.interface import implementer
 from zope.security.proxy import removeSecurityProxy
 
-from lp.app.browser.tales import DurationFormatterAPI
 from lp.app.errors import NotFoundError
-from lp.app.interfaces.launchpad import ILaunchpadCelebrities
-from lp.archivepublisher.utils import get_ppa_reference
 from lp.buildmaster.enums import (
     BuildFarmJobType,
     BuildStatus,
@@ -83,15 +79,6 @@
     LibraryFileAlias,
     LibraryFileContent,
     )
-from lp.services.mail.helpers import (
-    get_contact_email_addresses,
-    get_email_template,
-    )
-from lp.services.mail.sendmail import (
-    format_address,
-    simple_sendmail,
-    )
-from lp.services.webapp import canonical_url
 from lp.soyuz.enums import (
     ArchivePurpose,
     PackagePublishingStatus,
@@ -106,6 +93,7 @@
     )
 from lp.soyuz.interfaces.distroarchseries import IDistroArchSeries
 from lp.soyuz.interfaces.packageset import IPackagesetSet
+from lp.soyuz.mail.binarypackagebuild import BinaryPackageBuildMailer
 from lp.soyuz.model.binarypackagename import BinaryPackageName
 from lp.soyuz.model.binarypackagerelease import BinaryPackageRelease
 from lp.soyuz.model.files import BinaryPackageFile
@@ -717,152 +705,13 @@
         the record in question (all states are supported), see
         doc/build-notification.txt for further information.
         """
-
         if not config.builddmaster.send_build_notification:
             return
         if self.status == BuildStatus.FULLYBUILT:
             return
-
-        recipients = {}
-
-        fromaddress = format_address(
-            config.builddmaster.default_sender_name,
-            config.builddmaster.default_sender_address)
-
-        extra_headers = {
-            'X-Launchpad-Archive': self.archive.reference,
-            'X-Launchpad-Build-State': self.status.name,
-            'X-Launchpad-Build-Component': self.current_component.name,
-            'X-Launchpad-Build-Arch':
-                self.distro_arch_series.architecturetag,
-            }
-
-        # XXX cprov 2006-10-27: Temporary extra debug info about the
-        # SPR.creator in context, to be used during the service quarantine,
-        # notify_owner will be disabled to avoid *spamming* Debian people.
-        creator = self.source_package_release.creator
-        extra_headers['X-Creator-Recipient'] = ",".join(
-            get_contact_email_addresses(creator))
-
-        # Currently there are 7038 SPR published in edgy which the creators
-        # have no preferredemail. They are the autosync ones (creator = katie,
-        # 3583 packages) and the untouched sources since we have migrated from
-        # DAK (the rest). We should not spam Debian maintainers.
-
-        # Please note that both the package creator and the package uploader
-        # will be notified of failures if:
-        #     * the 'notify_owner' flag is set
-        #     * the package build (failure) occurred in the original
-        #       archive.
-        package_was_not_copied = (
-            self.archive == self.source_package_release.upload_archive)
-
-        if package_was_not_copied and config.builddmaster.notify_owner:
-            if (self.archive.is_ppa and creator.inTeam(self.archive.owner)
-                or
-                not self.archive.is_ppa):
-                # If this is a PPA, the package creator should only be
-                # notified if they are the PPA owner or in the PPA team.
-                # (see bug 375757)
-                # Non-PPA notifications inform the creator regardless.
-                for recipient in get_contact_email_addresses(creator):
-                    recipients.setdefault(
-                        recipient, "you created this version of this package")
-            dsc_key = self.source_package_release.dscsigningkey
-            if dsc_key:
-                for recipient in get_contact_email_addresses(dsc_key.owner):
-                    recipients.setdefault(recipient, "you signed this package")
-
-        # Modify notification contents according to the targeted archive.
-        # 'Archive Tag', 'Subject' and 'Source URL' are customized for PPA.
-        # We only send build-notifications to 'buildd-admin' celebrity for
-        # main archive candidates.
-        # For PPA build notifications we include the archive.owner
-        # contact_address.
-        subject = "[Build #%d] %s" % (self.id, self.title)
-        if not self.archive.is_ppa:
-            buildd_admins = getUtility(ILaunchpadCelebrities).buildd_admin
-            for recipient in get_contact_email_addresses(buildd_admins):
-                recipients.setdefault(
-                    recipient, "you are a buildd administrator")
-            source_url = canonical_url(self.distributionsourcepackagerelease)
-        else:
-            for recipient in get_contact_email_addresses(self.archive.owner):
-                recipients.setdefault(recipient, "you own this archive")
-            # For PPAs we run the risk of having no available contact_address,
-            # for instance, when both, SPR.creator and Archive.owner have
-            # not enabled it.
-            if len(recipients) == 0:
-                return
-            subject += ' [%s]' % self.archive.reference
-            source_url = 'not available'
-            # The deprecated PPA reference header is included for Ubuntu
-            # PPAs to avoid breaking existing consumers.
-            if self.archive.distribution.name == u'ubuntu':
-                extra_headers['X-Launchpad-PPA'] = get_ppa_reference(
-                    self.archive)
-
-        # XXX cprov 2006-08-02: pending security recipients for SECURITY
-        # pocket build. We don't build SECURITY yet :(
-
-        # XXX cprov 2006-08-02: find out a way to glue parameters reported
-        # with the state in the build workflow, maybe by having an
-        # IBuild.statusReport property, which could also be used in the
-        # respective page template.
-        if self.status in [
-            BuildStatus.NEEDSBUILD, BuildStatus.SUPERSEDED]:
-            # untouched builds
-            buildduration = 'not available'
-            buildlog_url = 'not available'
-            builder_url = 'not available'
-        elif self.status == BuildStatus.UPLOADING:
-            buildduration = 'uploading'
-            buildlog_url = 'see builder page'
-            builder_url = 'not available'
-        elif self.status == BuildStatus.BUILDING:
-            # build in process
-            buildduration = 'not finished'
-            buildlog_url = 'see builder page'
-            builder_url = canonical_url(self.buildqueue_record.builder)
-        else:
-            # completed states (success and failure)
-            buildduration = DurationFormatterAPI(
-                self.duration).approximateduration()
-            buildlog_url = self.log_url
-            builder_url = canonical_url(self.builder)
-
-        if self.status == BuildStatus.FAILEDTOUPLOAD:
-            assert extra_info is not None, (
-                'Extra information is required for FAILEDTOUPLOAD '
-                'notifications.')
-            extra_info = 'Upload log:\n%s' % extra_info
-        else:
-            extra_info = ''
-
-        template = get_email_template('build-notification.txt', app='soyuz')
-        replacements = {
-            'source_name': self.source_package_release.name,
-            'source_version': self.source_package_release.version,
-            'architecturetag': self.distro_arch_series.architecturetag,
-            'build_state': self.status.title,
-            'build_duration': buildduration,
-            'buildlog_url': buildlog_url,
-            'builder_url': builder_url,
-            'build_title': self.title,
-            'build_url': canonical_url(self),
-            'source_url': source_url,
-            'extra_info': extra_info,
-            'archive_tag': self.archive.reference,
-            'component_tag': self.current_component.name,
-            }
-
-        for toaddress, reason in recipients.items():
-            replacements['reason'] = textwrap.fill(
-                'You are receiving this email because %s.' % reason, width=72)
-            message = template % replacements
-            simple_sendmail(
-                fromaddress, toaddress, subject, message,
-                headers=extra_headers)
+        mailer = BinaryPackageBuildMailer.forStatus(
+            self, extra_info=extra_info)
+        mailer.sendAll()
 
     def _getDebByFileName(self, filename):
         """Helper function to get a .deb LFA in the context of this build."""

=== modified file 'lib/lp/soyuz/tests/test_build_notify.py'
--- lib/lp/soyuz/tests/test_build_notify.py	2015-07-14 11:01:53 +0000
+++ lib/lp/soyuz/tests/test_build_notify.py	2015-07-14 11:01:53 +0000
@@ -1,10 +1,9 @@
-# Copyright 2011-2014 Canonical Ltd.  This software is licensed under the
+# Copyright 2011-2015 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
 
 from datetime import timedelta
-from operator import itemgetter
 from textwrap import dedent
 
 from zope.component import getUtility
@@ -15,6 +14,7 @@
 from lp.buildmaster.enums import BuildStatus
 from lp.registry.interfaces.person import IPersonSet
 from lp.services.config import config
+from lp.services.mail.sendmail import format_address_for_person
 from lp.services.webapp import canonical_url
 from lp.soyuz.enums import ArchivePurpose
 from lp.soyuz.interfaces.publishing import PackagePublishingPocket
@@ -36,7 +36,9 @@
     "buildd-admin": (
         "You are receiving this email because you are a buildd "
         "administrator."),
-    "owner": "You are receiving this email because you own this archive.",
+    "owner": (
+        "You are receiving this email because you are the owner of this "
+        "archive."),
     }
 
 
@@ -62,17 +64,14 @@
             purpose=ArchivePurpose.PPA)
         buildd_admins = getUtility(IPersonSet).getByName(
             'launchpad-buildd-admins')
-        self.buildd_admins_email = []
         with person_logged_in(self.admin):
-            self.ppa_owner_email = self.ppa.owner.preferredemail.email
             self.publisher = SoyuzTestPublisher()
             self.publisher.prepareBreezyAutotest()
             self.distroseries.nominatedarchindep = self.das
             self.publisher.addFakeChroots(distroseries=self.distroseries)
             self.builder = self.factory.makeBuilder(
                 processors=[self.processor])
-            for member in buildd_admins.activemembers:
-                self.buildd_admins_email.append(member.preferredemail.email)
+            self.buildd_admins_members = list(buildd_admins.activemembers)
         self.builds = []
 
     def create_builds(self, archive):
@@ -101,7 +100,17 @@
                                 ppa=False):
         # Assert that the mail sent (which is in notification), matches
         # the data from the build
-        self.assertEqual(recipient, notification['To'])
+        self.assertEqual(
+            format_address_for_person(recipient), notification['To'])
+        if reason == "buildd-admin":
+            rationale = "Buildd-Admin @launchpad-buildd-admins"
+        else:
+            rationale = reason.title()
+        self.assertEqual(
+            rationale, notification['X-Launchpad-Message-Rationale'])
+        self.assertEqual(
+            'package-build-status',
+            notification['X-Launchpad-Notification-Type'])
         self.assertEqual(
             '[e-mail]', notification['X-Creator-Recipient'])
         self.assertEqual(
@@ -155,20 +164,21 @@
         If you want further information about this situation, feel free to
         contact a member of the Launchpad Buildd Administrators team.
 
-        --
+        %s
         %s
         %s
         """ % (
             build.source_package_release.sourcepackagename.name,
             build.source_package_release.version, self.das.architecturetag,
             build.archive.reference, build.status.title, duration, build_log,
-            builder, source, build.title, canonical_url(build)))
+            builder, source, "-- ", build.title, canonical_url(build)))
         expected_body += "\n" + REASONS[reason] + "\n"
         self.assertEqual(expected_body, body)
 
     def _assert_mails_are_correct(self, build, reasons, ppa=False):
         notifications = pop_notifications()
-        reasons = sorted(reasons, key=itemgetter(0))
+        reasons = sorted(
+            reasons, key=lambda r: format_address_for_person(r[0]))
         for notification, (recipient, reason) in zip(notifications, reasons):
             self._assert_mail_is_correct(
                 build, notification, recipient, reason, ppa=ppa)
@@ -180,8 +190,8 @@
         build = self.builds[BuildStatus.FAILEDTOBUILD.value]
         build.notify()
         expected_reasons = [
-            (email, "buildd-admin") for email in self.buildd_admins_email]
-        expected_reasons.append(("[e-mail]", "creator"))
+            (person, "buildd-admin") for person in self.buildd_admins_members]
+        expected_reasons.append((self.creator, "creator"))
         self._assert_mails_are_correct(build, expected_reasons)
 
     def test_notify_failed_to_build_ppa(self):
@@ -191,8 +201,8 @@
         build = self.builds[BuildStatus.FAILEDTOBUILD.value]
         build.notify()
         expected_reasons = [
-            ("[e-mail]", "signer"),
-            (self.ppa_owner_email, "owner"),
+            (self.creator, "signer"),
+            (self.ppa.owner, "owner"),
             ]
         self._assert_mails_are_correct(build, expected_reasons, ppa=True)
 
@@ -203,8 +213,8 @@
         build = self.builds[BuildStatus.NEEDSBUILD.value]
         build.notify()
         expected_reasons = [
-            (email, "buildd-admin") for email in self.buildd_admins_email]
-        expected_reasons.append(("[e-mail]", "creator"))
+            (person, "buildd-admin") for person in self.buildd_admins_members]
+        expected_reasons.append((self.creator, "creator"))
         self._assert_mails_are_correct(build, expected_reasons)
 
     def test_notify_needs_building_ppa(self):
@@ -214,8 +224,8 @@
         build = self.builds[BuildStatus.NEEDSBUILD.value]
         build.notify()
         expected_reasons = [
-            ("[e-mail]", "signer"),
-            (self.ppa_owner_email, "owner"),
+            (self.creator, "signer"),
+            (self.ppa.owner, "owner"),
             ]
         self._assert_mails_are_correct(build, expected_reasons, ppa=True)
 
@@ -233,8 +243,8 @@
         build = self.builds[BuildStatus.MANUALDEPWAIT.value]
         build.notify()
         expected_reasons = [
-            (email, "buildd-admin") for email in self.buildd_admins_email]
-        expected_reasons.append(("[e-mail]", "creator"))
+            (person, "buildd-admin") for person in self.buildd_admins_members]
+        expected_reasons.append((self.creator, "creator"))
         self._assert_mails_are_correct(build, expected_reasons)
 
     def test_notify_dependency_wait_ppa(self):
@@ -244,8 +254,8 @@
         build = self.builds[BuildStatus.MANUALDEPWAIT.value]
         build.notify()
         expected_reasons = [
-            ("[e-mail]", "signer"),
-            (self.ppa_owner_email, "owner"),
+            (self.creator, "signer"),
+            (self.ppa.owner, "owner"),
             ]
         self._assert_mails_are_correct(build, expected_reasons, ppa=True)
 
@@ -256,8 +266,8 @@
         build = self.builds[BuildStatus.CHROOTWAIT.value]
         build.notify()
         expected_reasons = [
-            (email, "buildd-admin") for email in self.buildd_admins_email]
-        expected_reasons.append(("[e-mail]", "creator"))
+            (person, "buildd-admin") for person in self.buildd_admins_members]
+        expected_reasons.append((self.creator, "creator"))
         self._assert_mails_are_correct(build, expected_reasons)
 
     def test_notify_chroot_problem_ppa(self):
@@ -267,8 +277,8 @@
         build = self.builds[BuildStatus.CHROOTWAIT.value]
         build.notify()
         expected_reasons = [
-            ("[e-mail]", "signer"),
-            (self.ppa_owner_email, "owner"),
+            (self.creator, "signer"),
+            (self.ppa.owner, "owner"),
             ]
         self._assert_mails_are_correct(build, expected_reasons, ppa=True)
 
@@ -280,8 +290,8 @@
         build = self.builds[BuildStatus.SUPERSEDED.value]
         build.notify()
         expected_reasons = [
-            (email, "buildd-admin") for email in self.buildd_admins_email]
-        expected_reasons.append(("[e-mail]", "creator"))
+            (person, "buildd-admin") for person in self.buildd_admins_members]
+        expected_reasons.append((self.creator, "creator"))
         self._assert_mails_are_correct(build, expected_reasons)
 
     def test_notify_build_for_superseded_source_ppa(self):
@@ -292,8 +302,8 @@
         build = self.builds[BuildStatus.SUPERSEDED.value]
         build.notify()
         expected_reasons = [
-            ("[e-mail]", "signer"),
-            (self.ppa_owner_email, "owner"),
+            (self.creator, "signer"),
+            (self.ppa.owner, "owner"),
             ]
         self._assert_mails_are_correct(build, expected_reasons, ppa=True)
 
@@ -304,8 +314,8 @@
         build = self.builds[BuildStatus.BUILDING.value]
         build.notify()
         expected_reasons = [
-            (email, "buildd-admin") for email in self.buildd_admins_email]
-        expected_reasons.append(("[e-mail]", "creator"))
+            (person, "buildd-admin") for person in self.buildd_admins_members]
+        expected_reasons.append((self.creator, "creator"))
         self._assert_mails_are_correct(build, expected_reasons)
 
     def test_notify_currently_building_ppa(self):
@@ -315,8 +325,8 @@
         build = self.builds[BuildStatus.BUILDING.value]
         build.notify()
         expected_reasons = [
-            ("[e-mail]", "signer"),
-            (self.ppa_owner_email, "owner"),
+            (self.creator, "signer"),
+            (self.ppa.owner, "owner"),
             ]
         self._assert_mails_are_correct(build, expected_reasons, ppa=True)
 
@@ -327,8 +337,8 @@
         build = self.builds[BuildStatus.UPLOADING.value]
         build.notify()
         expected_reasons = [
-            (email, "buildd-admin") for email in self.buildd_admins_email]
-        expected_reasons.append(("[e-mail]", "creator"))
+            (person, "buildd-admin") for person in self.buildd_admins_members]
+        expected_reasons.append((self.creator, "creator"))
         self._assert_mails_are_correct(build, expected_reasons)
 
     def test_notify_uploading_build_ppa(self):
@@ -338,8 +348,8 @@
         build = self.builds[BuildStatus.UPLOADING.value]
         build.notify()
         expected_reasons = [
-            ("[e-mail]", "signer"),
-            (self.ppa_owner_email, "owner"),
+            (self.creator, "signer"),
+            (self.ppa.owner, "owner"),
             ]
         self._assert_mails_are_correct(build, expected_reasons, ppa=True)
 
@@ -354,7 +364,7 @@
         [ppa_build] = ppa_spph.createMissingBuilds()
         ppa_build.notify()
         self._assert_mails_are_correct(
-            ppa_build, [(self.ppa_owner_email, "owner")], ppa=True)
+            ppa_build, [(self.ppa.owner, "owner")], ppa=True)
 
     def test_notify_owner_suppresses_mail(self):
         # When the 'notify_owner' config option is False, we don't send mail
@@ -370,7 +380,8 @@
         build.notify()
         self._assert_mails_are_correct(
             build,
-            [(email, "buildd-admin") for email in self.buildd_admins_email])
+            [(person, "buildd-admin")
+             for person in self.buildd_admins_members])
         # And undo what we just did.
         config.pop('notify_owner')
 
@@ -402,7 +413,7 @@
         removeSecurityProxy(spr).dscsigningkey = key
         build.notify()
         expected_reasons = [
-            (email, "buildd-admin") for email in self.buildd_admins_email]
-        expected_reasons.append(("[e-mail]", "creator"))
-        expected_reasons.append(("[e-mail]", "signer"))
+            (person, "buildd-admin") for person in self.buildd_admins_members]
+        expected_reasons.append((self.creator, "creator"))
+        expected_reasons.append((sponsor, "signer"))
         self._assert_mails_are_correct(build, expected_reasons)