launchpad-reviewers team mailing list archive

[Julian Edwards <julian.edwards@xxxxxxxxxxxxx>]

Julian Edwards has proposed merging lp:~julian-edwards/launchpad/code-bug-820941 into lp:launchpad/db-devel with lp:~julian-edwards/launchpad/schema-bug-820941 as a prerequisite.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #820941 in Launchpad itself: "Distro uploads always email XXXX_derivatives@xxxxxxxxxxxxxxxxxxxxxx"
  https://bugs.launchpad.net/launchpad/+bug/820941

For more details, see:
https://code.launchpad.net/~julian-edwards/launchpad/code-bug-820941/+merge/70753

The Soyuz notification code currently has a hard-coded BCC address for announcement emails, which is %s_derivatives@xxxxxxxxxxxxxxxxxxxxxx.

This is not that useful for non-Ubuntu distros hosted in LP so this branch makes that address configurable via the Distribution:+edit page.
-- 
https://code.launchpad.net/~julian-edwards/launchpad/code-bug-820941/+merge/70753
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~julian-edwards/launchpad/code-bug-820941 into lp:launchpad/db-devel.

=== modified file 'lib/lp/registry/browser/distribution.py'
--- lib/lp/registry/browser/distribution.py	2011-06-16 13:50:58 +0000
+++ lib/lp/registry/browser/distribution.py	2011-08-08 15:16:24 +0000
@@ -827,6 +827,7 @@
         'description',
         'bug_reporting_guidelines',
         'bug_reported_acknowledgement',
+        'package_derivatives_email',
         'icon',
         'logo',
         'mugshot',

=== modified file 'lib/lp/registry/browser/tests/test_distribution_views.py'
--- lib/lp/registry/browser/tests/test_distribution_views.py	2011-03-16 17:14:26 +0000
+++ lib/lp/registry/browser/tests/test_distribution_views.py	2011-08-08 15:16:24 +0000
@@ -14,6 +14,7 @@
 from lp.registry.interfaces.distribution import IDistributionSet
 from lp.testing import (
     login_celebrity,
+    person_logged_in,
     TestCaseWithFactory,
     )
 from lp.testing.sampledata import LAUNCHPAD_ADMIN
@@ -126,6 +127,26 @@
         self.assertEqual(distribution.registrant, admin)
 
 
+class TestDistroEditView(TestCaseWithFactory):
+    """Test the +edit page for a distro."""
+
+    layer = DatabaseFunctionalLayer
+
+    def test_package_derivatives_email(self):
+        # Test that the edit form allows changing package_derivatives_email
+        distro = self.factory.makeDistribution()
+        email = '{package_name}_thing@xxxxxxx'
+        form = {
+            'field.package_derivatives_email': email,
+            'field.actions.change': 'Change',
+            }
+        with person_logged_in(distro.owner):
+            create_initialized_view(
+                distro, '+edit', principal=distro.owner, method="POST",
+                form=form)
+        self.assertEqual(distro.package_derivatives_email, email)
+
+
 class TestDistroReassignView(TestCaseWithFactory):
     """Test the +reassign page for a new distribution."""
 

=== modified file 'lib/lp/registry/configure.zcml'
--- lib/lp/registry/configure.zcml	2011-08-02 05:35:39 +0000
+++ lib/lp/registry/configure.zcml	2011-08-08 15:16:24 +0000
@@ -1513,6 +1513,7 @@
                 official_blueprints
                 official_malone
                 owner
+                package_derivatives_email
                 security_contact
                 summary
                 title"/>

=== modified file 'lib/lp/registry/interfaces/distribution.py'
--- lib/lp/registry/interfaces/distribution.py	2011-07-22 21:47:34 +0000
+++ lib/lp/registry/interfaces/distribution.py	2011-08-08 15:16:24 +0000
@@ -282,6 +282,13 @@
     uploaders = Attribute(_(
         "ArchivePermission records for uploaders with rights to upload to "
         "this distribution."))
+    package_derivatives_email = TextLine(
+        title=_("Package Derivatives Email Address"),
+        description=_(
+            "The email address to send information about updates to packages "
+            "that are derived from another distribution. The sequence "
+            "{package_name} is replaced with the actual package name."),
+        required=False)
 
     # properties
     currentseries = exported(

=== modified file 'lib/lp/registry/model/distribution.py'
--- lib/lp/registry/model/distribution.py	2011-07-22 21:47:34 +0000
+++ lib/lp/registry/model/distribution.py	2011-08-08 15:16:24 +0000
@@ -276,6 +276,7 @@
         schema=TranslationPermission, default=TranslationPermission.OPEN)
     active = True
     max_bug_heat = Int()
+    package_derivatives_email = StringCol(notNull=False, default=None)
 
     def __repr__(self):
         displayname = self.displayname.encode('ASCII', 'backslashreplace')

=== modified file 'lib/lp/registry/tests/test_distribution.py'
--- lib/lp/registry/tests/test_distribution.py	2011-08-03 11:00:11 +0000
+++ lib/lp/registry/tests/test_distribution.py	2011-08-08 15:16:24 +0000
@@ -7,12 +7,14 @@
 
 from lazr.lifecycle.snapshot import Snapshot
 import soupmatchers
+from storm.store import Store
 from testtools import ExpectedException
 from testtools.matchers import (
     MatchesAny,
     Not,
     )
 from zope.component import getUtility
+from zope.security.interfaces import Unauthorized
 from zope.security.proxy import removeSecurityProxy
 
 from canonical.database.constants import UTC_NOW
@@ -40,6 +42,7 @@
     )
 from lp.testing import (
     login_person,
+    person_logged_in,
     TestCaseWithFactory,
     )
 from lp.testing.matchers import Provides
@@ -186,6 +189,23 @@
             sourcepackage.distribution.guessPublishedSourcePackageName(
                 'my-package').name)
 
+    def test_derivatives_email(self):
+        # Make sure the package_derivatives_email column stores data
+        # correctly.
+        email = "thingy@xxxxxxx"
+        distro = self.factory.makeDistribution()
+        with person_logged_in(distro.owner):
+            distro.package_derivatives_email = email
+        Store.of(distro).flush()
+        self.assertEqual(email, distro.package_derivatives_email)
+
+    def test_derivatives_email_permissions(self):
+        # package_derivatives_email requires lp.edit to set/change.
+        distro = self.factory.makeDistribution()
+        self.assertRaises(
+            Unauthorized,
+            setattr, distro, "package_derivatives_email", "foo")
+
 
 class TestDistributionCurrentSourceReleases(
     TestDistroSeriesCurrentSourceReleases):

=== modified file 'lib/lp/soyuz/adapters/notification.py'
--- lib/lp/soyuz/adapters/notification.py	2011-07-29 15:17:22 +0000
+++ lib/lp/soyuz/adapters/notification.py	2011-08-08 15:16:24 +0000
@@ -251,7 +251,9 @@
         elif bprs:
             name = bprs[0].build.source_package_release.name
         if name:
-            bcc_addr = '%s_derivatives@xxxxxxxxxxxxxxxxxxxxxx' % name
+            email_base = distroseries.distribution.package_derivatives_email
+            if email_base:
+                bcc_addr = email_base.format(package_name=name)
 
         build_and_send_mail(
             'announcement', [str(distroseries.changeslist)], from_addr,

=== modified file 'lib/lp/soyuz/adapters/tests/test_notification.py'
--- lib/lp/soyuz/adapters/tests/test_notification.py	2011-07-29 15:17:22 +0000
+++ lib/lp/soyuz/adapters/tests/test_notification.py	2011-08-08 15:16:24 +0000
@@ -38,7 +38,10 @@
 from lp.soyuz.model.distroseriessourcepackagerelease import (
     DistroSeriesSourcePackageRelease,
     )
-from lp.testing import TestCaseWithFactory
+from lp.testing import (
+    person_logged_in,
+    TestCaseWithFactory,
+    )
 from lp.testing.mail_helpers import pop_notifications
 
 
@@ -83,20 +86,28 @@
             'accepted')
         self.assertEqual(expected_subject, subject)
 
+    def _setup_notification(self, from_person=None, distroseries=None,
+                            spr=None):
+        if spr is None:
+            spr = self.factory.makeSourcePackageRelease()
+        self.factory.makeSourcePackageReleaseFile(sourcepackagerelease=spr)
+        archive = self.factory.makeArchive(purpose=ArchivePurpose.PRIMARY)
+        pocket = PackagePublishingPocket.RELEASE
+        if distroseries is None:
+            distroseries = self.factory.makeDistroSeries()
+        distroseries.changeslist = "blah@xxxxxxxxxxx"
+        blamer = self.factory.makePerson()
+        if from_person is None:
+            from_person = self.factory.makePerson()
+        notify(
+            blamer, spr, [], [], archive, distroseries, pocket,
+            action='accepted', announce_from_person=from_person)
+
     def test_notify_from_person_override(self):
         # notify() takes an optional from_person to override the calculated
         # From: address in announcement emails.
-        spr = self.factory.makeSourcePackageRelease()
-        self.factory.makeSourcePackageReleaseFile(sourcepackagerelease=spr)
-        archive = self.factory.makeArchive(purpose=ArchivePurpose.PRIMARY)
-        pocket = PackagePublishingPocket.RELEASE
-        distroseries = self.factory.makeDistroSeries()
-        distroseries.changeslist = "blah@xxxxxxxxxxx"
-        blamer = self.factory.makePerson()
         from_person = self.factory.makePerson()
-        notify(
-            blamer, spr, [], [], archive, distroseries, pocket,
-            action='accepted', announce_from_person=from_person)
+        self._setup_notification(from_person=from_person)
         notifications = pop_notifications()
         self.assertEqual(2, len(notifications))
         # The first notification is to the blamer,
@@ -105,6 +116,22 @@
         self.assertEqual(
             from_person.preferredemail.email, notifications[1]["From"])
 
+    def test_notify_bcc_to_derivatives_list(self):
+        # notify() will BCC the announcement email to the address defined in
+        # Distribution.package_derivatives_email if it's defined.
+        email = "{package_name}_thing@xxxxxxx"
+        distroseries = self.factory.makeDistroSeries()
+        with person_logged_in(distroseries.distribution.owner):
+            distroseries.distribution.package_derivatives_email = email
+        spr = self.factory.makeSourcePackageRelease()
+        self._setup_notification(distroseries=distroseries, spr=spr)
+
+        notifications = pop_notifications()
+        self.assertEqual(2, len(notifications))
+        bcc_address = notifications[1]["Bcc"]
+        expected_email = email.format(package_name=spr.sourcepackagename.name)
+        self.assertIn(expected_email, bcc_address)
+
 
 class TestNotification(TestCaseWithFactory):