launchpad-reviewers team mailing list archive

[Steve Kowalik <stevenk@xxxxxxxxxx>]

Steve Kowalik has proposed merging lp:~stevenk/launchpad/link-recipe-on-ppa-page into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~stevenk/launchpad/link-recipe-on-ppa-page/+merge/50694

This branch adds support to show the recipe that caused the resultant source publication to be created to be shown on the PPA +packages page. This is supported by adding a new property to ISourcePackagePublishingHistory that returns the SourcePackageRecipeBuild or None.

I also fixed bug 722344 in this branch as it made the tests I wrote work. I believe that URL traversal code is the entirely wrong place to assume the distribution for an archive, so I refactored it to not care. Given we force the distribution for PPAs to Ubuntu in many other places, the only place I can see this impacting is our testsuite.

http://people.canonical.com/~stevenk/built-by-recipe.png is screenshot of the UI changes. It adds the line under "Publishing details" of the form "Built by recipe <link to recipe, with anchor text of the recipe name> for <requester of the recipe build>". 

I believe this will also require an UI review. (My first!)
-- 
https://code.launchpad.net/~stevenk/launchpad/link-recipe-on-ppa-page/+merge/50694
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/link-recipe-on-ppa-page into lp:launchpad.

=== modified file 'lib/lp/soyuz/browser/archive.py'
--- lib/lp/soyuz/browser/archive.py	2011-02-09 15:57:56 +0000
+++ lib/lp/soyuz/browser/archive.py	2011-02-22 05:08:17 +0000
@@ -134,6 +134,7 @@
     IArchive,
     IArchiveEditDependenciesForm,
     IArchiveSet,
+    NoSuchPPA,
     )
 from lp.soyuz.interfaces.archivepermission import IArchivePermissionSet
 from lp.soyuz.interfaces.archivesubscriber import IArchiveSubscriberSet
@@ -166,13 +167,10 @@
     :param person_name: The person part of the URL
     :param ppa_name: The PPA name part of the URL
     """
-    # For now, all PPAs are assumed to be Ubuntu-related.  This will
-    # change when we start doing PPAs for other distros.
-    ubuntu = getUtility(ILaunchpadCelebrities).ubuntu
-    archive_set = getUtility(IArchiveSet)
-    archive = archive_set.getPPAByDistributionAndOwnerName(
-            ubuntu, person_name, ppa_name)
-    if archive is None:
+    person = getUtility(IPersonSet).getByName(person_name)
+    try:
+        archive = person.getPPAByName(ppa_name)
+    except NoSuchPPA:
         raise NotFoundError("%s/%s", (person_name, ppa_name))
 
     return archive
@@ -967,7 +965,7 @@
                 builds = status_summary['builds']
 
             latest_updates_list.append({
-                'date_published' : date_published,
+                'date_published': date_published,
                 'title': source_package_name,
                 'status': status_names[current_status.title],
                 'status_class': current_status.title,

=== added file 'lib/lp/soyuz/browser/tests/test_archive_sourcepackagerecipe.py'
--- lib/lp/soyuz/browser/tests/test_archive_sourcepackagerecipe.py	1970-01-01 00:00:00 +0000
+++ lib/lp/soyuz/browser/tests/test_archive_sourcepackagerecipe.py	2011-02-22 05:08:17 +0000
@@ -0,0 +1,72 @@
+# Copyright 2011 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Tests for ArchivePackagesView, with relation to source package
+recipes."""
+
+__metaclass__ = type
+
+from zope.component import getUtility
+
+from canonical.testing.layers import LaunchpadFunctionalLayer
+from canonical.launchpad.webapp.publisher import canonical_url
+
+from lp.registry.interfaces.person import IPersonSet
+from lp.soyuz.enums import PackagePublishingStatus
+from lp.soyuz.tests.test_publishing import SoyuzTestPublisher
+from lp.testing import (
+    BrowserTestCase,
+    person_logged_in,
+    )
+from lp.testing.sampledata import ADMIN_EMAIL
+
+
+class TestArchivePackagesViewSourcePackageRecipe(BrowserTestCase):
+    layer = LaunchpadFunctionalLayer
+
+    def setUp(self):
+        super(TestArchivePackagesViewSourcePackageRecipe, self).setUp()
+        self.admin = getUtility(IPersonSet).getByEmail(ADMIN_EMAIL)
+        # Create everything we need to create builds, such as a
+        # DistroArchSeries and a builder.
+        self.pf = self.factory.makeProcessorFamily()
+        pf_proc = self.pf.addProcessor(self.factory.getUniqueString(), '', '')
+        self.distroseries = self.factory.makeDistroSeries()
+        self.das = self.factory.makeDistroArchSeries(
+            distroseries=self.distroseries, processorfamily=self.pf,
+            supports_virtualized=True)
+        self.archive = self.factory.makeArchive(
+            distribution=self.distroseries.distribution)
+        with person_logged_in(self.admin):
+            self.publisher = SoyuzTestPublisher()
+            self.publisher.prepareBreezyAutotest()
+            self.distroseries.nominatedarchindep = self.das
+            self.publisher.addFakeChroots(distroseries=self.distroseries)
+            self.builder = self.factory.makeBuilder(processor=pf_proc)
+
+    def test_view_with_source_package_recipe(self):
+        # When a SourcePackageRelease is linked to a
+        # SourcePackageRecipeBuild, the view shows which recipe was
+        # responsible for creating the SPR.
+        sprb = self.factory.makeSourcePackageRecipeBuild(
+            archive=self.archive)
+        recipe = sprb.recipe
+        spph = self.publisher.getPubSource(
+            archive=self.archive, status=PackagePublishingStatus.PUBLISHED)
+        spph.sourcepackagerelease.source_package_recipe_build = sprb
+        expected_contents = 'Built by recipe <a href="%s">%s</a> for %s' % (
+            canonical_url(recipe), recipe.name, sprb.requester.displayname)
+        browser = self.getViewBrowser(self.archive, '+packages')
+        browser.getLink('Show details').click()
+        self.assertIn(expected_contents, browser.contents)
+
+    def test_view_without_source_package_recipe(self):
+        # And if a SourcePackageRelease is not linked, there is no sign of it
+        # in the view.
+        with person_logged_in(self.admin):
+            self.publisher.getPubBinaries(
+                archive=self.archive,
+                status=PackagePublishingStatus.PUBLISHED)
+        browser = self.getViewBrowser(self.archive, '+packages')
+        browser.getLink('Show details').click()
+        self.assertNotIn('Built by recipe', browser.contents)

=== modified file 'lib/lp/soyuz/interfaces/publishing.py'
--- lib/lp/soyuz/interfaces/publishing.py	2011-02-09 19:25:43 +0000
+++ lib/lp/soyuz/interfaces/publishing.py	2011-02-22 05:08:17 +0000
@@ -631,6 +631,13 @@
         :return: A URL to the librarian file containing the diff.
         """
 
+    def builtByRecipe():
+        """Return the SourcePackageRecipeBuild that caused this publishing
+        record to be created, or None if one wasn't.
+
+        :return: A `SourcePackageRecipeBuild` or None.
+        """
+
 
 class ISourcePackagePublishingHistory(ISourcePackagePublishingHistoryPublic,
                                       IPublishingEdit):

=== modified file 'lib/lp/soyuz/model/publishing.py'
--- lib/lp/soyuz/model/publishing.py	2011-02-09 19:25:43 +0000
+++ lib/lp/soyuz/model/publishing.py	2011-02-22 05:08:17 +0000
@@ -854,6 +854,11 @@
         getUtility(IPublishingSet).requestDeletion(
             [self], removed_by, removal_comment)
 
+    @property
+    def builtByRecipe(self):
+        """See `ISourcePackagePublishingHistory`."""
+        return self.sourcepackagerelease.source_package_recipe_build
+
 
 class BinaryPackagePublishingHistory(SQLBase, ArchivePublisherBase):
     """A binary package publishing record."""

=== modified file 'lib/lp/soyuz/templates/packagepublishing-details.pt'
--- lib/lp/soyuz/templates/packagepublishing-details.pt	2010-05-17 15:44:05 +0000
+++ lib/lp/soyuz/templates/packagepublishing-details.pt	2011-02-22 05:08:17 +0000
@@ -28,6 +28,9 @@
             tal:content="context/datesuperseded/fmt:displaydate"/>
       by <span tal:replace="context/supersededby/title" />
     </li>
+    <li tal:condition="context/builtByRecipe">
+    Built by recipe <a tal:attributes="href context/builtByRecipe/recipe/fmt:url"><span tal:replace="context/builtByRecipe/recipe/name" /></a> for <span tal:replace="context/builtByRecipe/requester/displayname" />
+    </li>
     <li tal:condition="context/datepublished">
       <strong>Published</strong>
       <span tal:attributes="title context/datepublished/fmt:datetime"