Merge lp:~laney/launchpad/add-sponsor-field-to-spph into lp:launchpad

[Iain Lane]

[r=jcsackett,rharding][bug=912247] If copyPackage is sponsored, sases sponsor in SPPH.

If copyPackage is sponsored, save sponsor in SPPH.

== Test ==

bin/test -cvv -t test_copypackage -t publishing.txt -t archive.txt -t builds.txt

Total: 112 tests, 0 failures, 0 errors in 4 minutes 59.660 seconds.

== Demo and QA ==

Find a package which is newer in Debian than Ubuntu and copy it into Ubuntu using copyPackage and setting the sponsored field to some user. Ensure that the sponsor field of the resulting SPPH is set to your user.

== lint ==

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/soyuz/interfaces/publishing.py
  lib/lp/soyuz/model/publishing.py
  lib/lp/soyuz/scripts/packagecopier.py
  lib/lp/soyuz/scripts/tests/test_copypackage.py

./lib/lp/soyuz/scripts/tests/test_copypackage.py
    1456: Line exceeds 78 characters.
    1456: E501 line too long (82 characters)
make: *** [lint] Error 2

Preexisting and AFAICS not fixable as it is matching a string from an email.

[Steve Kowalik (stevenk)]

This can not land until the DB patch in lp:~laney/launchpad/spph-sponsor hits production.

[Richard Harding (rharding)]

Can the test for test_sponsored_copy_sponsor_field just be an additional assert in the test_sponsored_copy_notification test case? Just rename that to test_sponsored_copy perhaps? It seems a lot of duplicate boilerplate.

Other than that, approving.

review: Approve (code*)

[j.c.sackett (jcsackett)]

This looks okay, but I really would like the duplicated boilerplate in the
tests handled.

One approach I would prefer to just mashing the tests together is in the diff
below.

> === modified file 'lib/lp/soyuz/scripts/tests/test_copypackage.py'
> --- lib/lp/soyuz/scripts/tests/test_copypackage.py 2011-12-30 06:14:56 +0000
> +++ lib/lp/soyuz/scripts/tests/test_copypackage.py 2012-01-09 14:10:32 +0000
> @@ -1,4 +1,4 @@
> -# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
> +# Copyright 2009-2012 Canonical Ltd. This software is licensed under the
> # GNU Affero General Public License version 3 (see the file LICENSE).
>
> __metaclass__ = type
> @@ -1526,6 +1526,30 @@
> 'Sponsored <email address hidden>', announcement['From'])
> self.assertEqual(sponsored_person, copied_source.creator)
>
> + def test_sponsored_copy_sponsor_field(self):
> + # If it's a sponsored copy then the SPPH's sponsored field is set to
> + # the user who sponsored the copy.
> + archive = self.test_publisher.ubuntutest.main_archive
> + source = self.test_publisher.getPubSource(
> + archive=archive, version='1.0-2', architecturehintlist='any')
> + changelog = self.factory.makeChangelog(spn="foo", versions=["1.0-2"])
> + source.sourcepackagerelease.changelog = changelog
> + # Copying to a primary archive reads the changes to close bugs.
> + transaction.commit()
> + nobby = self.createNobby(('i386', 'hppa'))
> + getUtility(ISourcePackageFormatSelectionSet).add(
> + nobby, SourcePackageFormat.FORMAT_1_0)
> + nobby.changeslist = '<email address hidden>'
> + sponsored_person = self.factory.makePerson(
> + displayname="Sponsored", <email address hidden>")
> + [copied_source] = do_copy(
> + [source], archive, nobby, source.pocket, False,
> + person=source.sourcepackagerelease.creator,
> + check_permissions=False, send_email=True,
> + sponsored=sponsored_person)
> + self.assertEqual(source.sourcepackagerelease.creator,
> + copied_source.sponsor)
> +
> def test_copy_notification_contains_aggregate_change_log(self):
> # When copying a package that generates a notification,
> # the changelog should contain all of the changelog_entry texts for
> @@ -1663,6 +1687,26 @@
> target_archive.owner,
> copied_source.creator)
>
> + def test_unsponsored_copy_does_not_set_sponsor(self):
> + # If the copy is not sponsored, SPPH.sponsor is none
> + archive = self.test_publisher.ubuntutest.main_archive
> + source = self.test_publisher.getPubSource(
> + archive=archive, version='1.0-2', architecturehintlist='any')
> + source.sourcepackagerelease.changelog_entry = '* Foo!'
> + nobby = self.createNobby(('i386', 'hppa'))
> + getUtility(ISourcePackageFormatSelectionSet).add(
> + nobby, SourcePackageFormat.FORMAT_1_0)
> + target_archive = self.factory.makeArchive(
> + distribution=self.test_publisher.ubun...

review: Needs Fixing

[j.c.sackett (jcsackett)]

I note that this now removes several `changelog_entry = '* Foo!' lines, but it's been verified the tests all still pass.

This looks ok to land.

review: Approve

=== modified file 'lib/lp/soyuz/interfaces/publishing.py'
--- lib/lp/soyuz/interfaces/publishing.py	2011-12-24 16:54:44 +0000
+++ lib/lp/soyuz/interfaces/publishing.py	2012-01-20 13:35:27 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 # pylint: disable-msg=E0211,E0213
@@ -495,6 +495,15 @@
             required=False, readonly=True
         ))
 
+    sponsor = exported(
+        Reference(
+            IPerson,
+            title=_('Publication sponsor'),
+            description=_('The IPerson who sponsored the creation of'
+                'this publication.'),
+            required=False, readonly=True
+        ))
+
     # Really IBinaryPackagePublishingHistory, see below.
     @operation_returns_collection_of(Interface)
     @export_read_operation()
@@ -1009,6 +1018,8 @@
              should be created for the new source publication.
         :param creator: An optional `IPerson`. If this is None, the
             sourcepackagerelease's creator will be used.
+        :param sponsor: An optional `IPerson` indicating the sponsor of this
+            publication.
 
         datecreated will be UTC_NOW.
         status will be PackagePublishingStatus.PENDING

=== modified file 'lib/lp/soyuz/model/publishing.py'
--- lib/lp/soyuz/model/publishing.py	2011-12-30 06:14:56 +0000
+++ lib/lp/soyuz/model/publishing.py	2012-01-20 13:35:27 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 # pylint: disable-msg=E0611,W0212
@@ -456,6 +456,9 @@
     creator = ForeignKey(
         dbName='creator', foreignKey='Person',
         storm_validator=validate_public_person, notNull=False, default=None)
+    sponsor = ForeignKey(
+        dbName='sponsor', foreignKey='Person',
+        storm_validator=validate_public_person, notNull=False, default=None)
 
     @property
     def package_creator(self):
@@ -816,7 +819,7 @@
             archive=current.archive)
 
     def copyTo(self, distroseries, pocket, archive, override=None,
-               create_dsd_job=True, creator=None):
+               create_dsd_job=True, creator=None, sponsor=None):
         """See `ISourcePackagePublishingHistory`."""
         component = self.component
         section = self.section
@@ -834,7 +837,8 @@
             pocket,
             ancestor=self,
             create_dsd_job=create_dsd_job,
-            creator=creator)
+            creator=creator,
+            sponsor=sponsor)
 
     def getStatusSummaryForBuilds(self):
         """See `ISourcePackagePublishingHistory`."""
@@ -1511,7 +1515,7 @@
     def newSourcePublication(self, archive, sourcepackagerelease,
                              distroseries, component, section, pocket,
                              ancestor=None, create_dsd_job=True,
-                             creator=None):
+                             creator=None, sponsor=None):
         """See `IPublishingSet`."""
         # Avoid circular import.
         from lp.registry.model.distributionsourcepackage import (
@@ -1528,7 +1532,8 @@
             status=PackagePublishingStatus.PENDING,
             datecreated=UTC_NOW,
             ancestor=ancestor,
-            creator=creator)
+            creator=creator,
+            sponsor=sponsor)
         DistributionSourcePackage.ensure(pub)
 
         if create_dsd_job:

=== modified file 'lib/lp/soyuz/scripts/packagecopier.py'
--- lib/lp/soyuz/scripts/packagecopier.py	2012-01-06 11:08:30 +0000
+++ lib/lp/soyuz/scripts/packagecopier.py	2012-01-20 13:35:27 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """PackageCopier utilities."""
@@ -652,13 +652,16 @@
             if sponsored is not None:
                 announce_from_person = sponsored
                 creator = sponsored
+                sponsor = person
             else:
                 creator = person
+                sponsor = None
             sub_copies = _do_direct_copy(
                 source, archive, destination_series, pocket,
                 include_binaries, override, close_bugs=close_bugs,
                 create_dsd_job=create_dsd_job,
-                close_bugs_since_version=old_version, creator=creator)
+                close_bugs_since_version=old_version, creator=creator,
+                sponsor=sponsor)
             if send_email:
                 notify(
                     person, source.sourcepackagerelease, [], [], archive,
@@ -674,7 +677,8 @@
 
 def _do_direct_copy(source, archive, series, pocket, include_binaries,
                     override=None, close_bugs=True, create_dsd_job=True,
-                    close_bugs_since_version=None, creator=None):
+                    close_bugs_since_version=None, creator=None,
+                    sponsor=None):
     """Copy publishing records to another location.
 
     Copy each item of the given list of `SourcePackagePublishingHistory`
@@ -701,6 +705,7 @@
         then this parameter says which changelog entries to parse looking
         for bugs to close.  See `close_bugs_for_sourcepackagerelease`.
     :param creator: the requester `IPerson`.
+    :param sponsor: the sponsor `IPerson`, if this copy is being sponsored.
 
     :return: a list of `ISourcePackagePublishingHistory` and
         `BinaryPackagePublishingHistory` corresponding to the copied
@@ -731,7 +736,7 @@
             override = overrides[0]
         source_copy = source.copyTo(
             series, pocket, archive, override, create_dsd_job=create_dsd_job,
-            creator=creator)
+            creator=creator, sponsor=sponsor)
         if close_bugs:
             close_bugs_for_sourcepublication(
                 source_copy, close_bugs_since_version)

=== modified file 'lib/lp/soyuz/scripts/tests/test_copypackage.py'
--- lib/lp/soyuz/scripts/tests/test_copypackage.py	2011-12-30 06:14:56 +0000
+++ lib/lp/soyuz/scripts/tests/test_copypackage.py	2012-01-20 13:35:27 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
@@ -1336,11 +1336,19 @@
         self.assertComponentSectionAndPriority(
             ebin_hppa.component, ebin_hppa, copied_bin_hppa)
 
+    def _setup_archive(self):
+        archive = self.test_publisher.ubuntutest.main_archive
+        source = self.test_publisher.getPubSource(
+            archive=archive, version='1.0-2', architecturehintlist='any')
+        nobby = self.createNobby(('i386', 'hppa'))
+        getUtility(ISourcePackageFormatSelectionSet).add(
+            nobby, SourcePackageFormat.FORMAT_1_0)
+        return nobby, archive, source
+
     def test_existing_publication_overrides_pockets(self):
         # When we copy source/binaries from one pocket to another, the
         # overrides are unchanged from the source publication overrides.
-        nobby = self.createNobby(('i386', 'hppa'))
-        archive = self.test_publisher.ubuntutest.main_archive
+        nobby, archive, _ = self._setup_archive()
         source = self.test_publisher.getPubSource(
             archive=archive, version='1.0-1', architecturehintlist='any',
             distroseries=nobby, pocket=PackagePublishingPocket.PROPOSED)
@@ -1422,15 +1430,10 @@
 
     def test_copy_ppa_generates_notification(self):
         # When a copy into a PPA is performed, a notification is sent.
-        archive = self.test_publisher.ubuntutest.main_archive
-        source = self.test_publisher.getPubSource(
-            archive=archive, version='1.0-2', architecturehintlist='any')
+        nobby, archive, source = self._setup_archive()
         changelog = self.factory.makeChangelog(spn="foo", versions=["1.0-2"])
         source.sourcepackagerelease.changelog = changelog
-        transaction.commit()  # Librarian.
-        nobby = self.createNobby(('i386', 'hppa'))
-        getUtility(ISourcePackageFormatSelectionSet).add(
-            nobby, SourcePackageFormat.FORMAT_1_0)
+        transaction.commit()
         target_archive = self.factory.makeArchive(
             distribution=self.test_publisher.ubuntutest)
         [copied_source] = do_copy(
@@ -1503,16 +1506,11 @@
         # If it's a sponsored copy then the From: address on the
         # notification is changed to the sponsored person and the
         # SPPH.creator is set to the same person.
-        archive = self.test_publisher.ubuntutest.main_archive
-        source = self.test_publisher.getPubSource(
-            archive=archive, version='1.0-2', architecturehintlist='any')
+        nobby, archive, source = self._setup_archive()
         changelog = self.factory.makeChangelog(spn="foo", versions=["1.0-2"])
         source.sourcepackagerelease.changelog = changelog
         # Copying to a primary archive reads the changes to close bugs.
         transaction.commit()
-        nobby = self.createNobby(('i386', 'hppa'))
-        getUtility(ISourcePackageFormatSelectionSet).add(
-            nobby, SourcePackageFormat.FORMAT_1_0)
         nobby.changeslist = '[e-mail]'
         sponsored_person = self.factory.makePerson(
             displayname="Sponsored", email="[e-mail]")
@@ -1526,6 +1524,24 @@
             'Sponsored <[e-mail]>', announcement['From'])
         self.assertEqual(sponsored_person, copied_source.creator)
 
+    def test_sponsored_copy_sponsor_field(self):
+        # If it's a sponsored copy then the SPPH's sponsored field is set to
+        # the user who sponsored the copy.
+        nobby, archive, source = self._setup_archive()
+        changelog = self.factory.makeChangelog(spn="foo", versions=["1.0-2"])
+        source.sourcepackagerelease.changelog = changelog
+        # Copying to a primary archive reads the changes to close bugs.
+        transaction.commit()
+        sponsored_person = self.factory.makePerson(
+            displayname="Sponsored", email="[e-mail]")
+        [copied_source] = do_copy(
+            [source], archive, nobby, source.pocket, False,
+                    person=source.sourcepackagerelease.creator,
+                    check_permissions=False, send_email=True,
+                    sponsored=sponsored_person)
+        self.assertEqual(source.sourcepackagerelease.creator,
+            copied_source.sponsor)
+
     def test_copy_notification_contains_aggregate_change_log(self):
         # When copying a package that generates a notification,
         # the changelog should contain all of the changelog_entry texts for
@@ -1567,14 +1583,7 @@
     def test_copy_generates_rejection_email(self):
         # When a copy into a primary archive fails, we expect a rejection
         # email if the send_email parameter is True.
-        archive = self.test_publisher.ubuntutest.main_archive
-        source = self.test_publisher.getPubSource(
-            archive=archive, version='1.0-2', architecturehintlist='any')
-        source.sourcepackagerelease.changelog_entry = '* Foo!'
-        transaction.commit()  # Librarian.
-        nobby = self.createNobby(('i386', 'hppa'))
-        getUtility(ISourcePackageFormatSelectionSet).add(
-            nobby, SourcePackageFormat.FORMAT_1_0)
+        nobby, archive, source = self._setup_archive()
         # Ensure the same source is already in the destination so that we
         # get a rejection.
         self.test_publisher.getPubSource(
@@ -1602,15 +1611,9 @@
         self.assertIn(expected_text, notification.as_string())
 
     def test_copy_does_not_generate_notification(self):
-        # When notify = False is passed to do_copy, no notification is
+        # When send_email = False is passed to do_copy, no notification is
         # generated.
-        archive = self.test_publisher.ubuntutest.main_archive
-        source = self.test_publisher.getPubSource(
-            archive=archive, version='1.0-2', architecturehintlist='any')
-        source.sourcepackagerelease.changelog_entry = '* Foo!'
-        nobby = self.createNobby(('i386', 'hppa'))
-        getUtility(ISourcePackageFormatSelectionSet).add(
-            nobby, SourcePackageFormat.FORMAT_1_0)
+        nobby, archive, source = self._setup_archive()
         target_archive = self.factory.makeArchive(
             distribution=self.test_publisher.ubuntutest)
         [copied_source] = do_copy(
@@ -1645,13 +1648,7 @@
     def test_copy_sets_creator(self):
         # The creator for the copied SPPH is the person passed
         # to do_copy.
-        archive = self.test_publisher.ubuntutest.main_archive
-        source = self.test_publisher.getPubSource(
-            archive=archive, version='1.0-2', architecturehintlist='any')
-        source.sourcepackagerelease.changelog_entry = '* Foo!'
-        nobby = self.createNobby(('i386', 'hppa'))
-        getUtility(ISourcePackageFormatSelectionSet).add(
-            nobby, SourcePackageFormat.FORMAT_1_0)
+        nobby, archive, source = self._setup_archive()
         target_archive = self.factory.makeArchive(
             distribution=self.test_publisher.ubuntutest)
         [copied_source] = do_copy(
@@ -1663,6 +1660,20 @@
             target_archive.owner,
             copied_source.creator)
 
+    def test_unsponsored_copy_does_not_set_sponsor(self):
+        # If the copy is not sponsored, SPPH.sponsor is none
+        nobby, archive, source = self._setup_archive()
+        target_archive = self.factory.makeArchive(
+            distribution=self.test_publisher.ubuntutest)
+        [copied_source] = do_copy(
+            [source], target_archive, nobby, source.pocket, False,
+            person=target_archive.owner, check_permissions=False,
+            send_email=False)
+
+        self.assertEqual(
+            copied_source.sponsor,
+            None)
+
 
 class TestDoDelayedCopy(TestCaseWithFactory, BaseDoCopyTests):
 

=== modified file 'lib/lp/soyuz/stories/webservice/xx-source-package-publishing.txt'
--- lib/lp/soyuz/stories/webservice/xx-source-package-publishing.txt	2011-12-24 17:49:30 +0000
+++ lib/lp/soyuz/stories/webservice/xx-source-package-publishing.txt	2012-01-20 13:35:27 +0000
@@ -138,6 +138,7 @@
     self_link: u'http://.../~cprov/+archive/ppa/+sourcepub/...'
     source_package_name: u'testwebservice'
     source_package_version: u'666'
+    sponsor_link: None
     status: u'Pending'
 
     >>> webservice.named_get(