Merge lp:~edwin-grubbs/launchpad/bug-538469-invalid-upstream-link into lp:launchpad

[Edwin Grubbs]

Summary
-------

When the Packages in Ubuntu portlet on the project index page suggests a
source package, it should only include source packages that have a
publishing history for the current Ubuntu series. This matches the
behavior of $project/+ubuntupkg, which will give you an error if you
try to link to a distroseries without a publishing history for the
given source package.

Implementation details
----------------------

Added assertion to setPackaging() as a failsafe in case another
method of adding a Packaging entry is created.
    lib/lp/registry/model/productseries.py

Added publishing_distroseries parameter to
IDistribution.searchSourcePackage() that filters out source package
names that do not have a publishing history for the given distroseries.
    lib/lp/registry/browser/product.py
    lib/lp/registry/browser/tests/product-portlet-packages-view.txt
    lib/lp/registry/doc/distribution.txt
    lib/lp/registry/interfaces/distribution.txt
    lib/lp/registry/model/distribution.py
    lib/lp/registry/tests/test_productseries.py

Fixed grammar error:
    lib/lp/registry/browser/productseries.py

Fixed docstring:
    lib/lp/registry/tests/test_distroseries.py

Tests
-----

./bin/test -vv -t 'registry.*(product-portlet-packages-view.txt|doc/distribution.txt|test_distroseries|test_productseries)'

Demo and Q/A
------------

* Open https://launchpad.dev/evolution/+packages
  * Remove all the packages.
* Open https://launchpad.dev/evolution
  * The Packages in Ubuntu should still suggest the
    evolution source package, since it has a publishing
    history for the current series, which is hoary.
* Open https://launchpad.dev/ubuntu/grumpy/+admin
  * Change the status to Pre-release Freeze. This status
    has priority over the Development or Current statuses
    when determining the IDistribution.currentseries.
* Open https://launchpad.dev/evolution
  * The evolution source package should no longer be
    suggested.

Lint
----
Linting changed files:
  lib/lp/registry/browser/product.py
  lib/lp/registry/browser/productseries.py
  lib/lp/registry/browser/tests/product-portlet-packages-view.txt
  lib/lp/registry/doc/distribution.txt
  lib/lp/registry/model/distribution.py
  lib/lp/registry/model/productseries.py
  lib/lp/registry/tests/test_distroseries.py
  lib/lp/registry/tests/test_productseries.py

== Pylint notices ==

lib/lp/registry/browser/product.py
    59: [F0401] Unable to import 'lazr.delegates' (No module named delegates)

[Brad Crittenden (bac)]

Edwin this branch looks very good. Thanks.

review: Approve (code)

=== modified file 'lib/lp/registry/browser/product.py'
--- lib/lp/registry/browser/product.py	2010-03-09 00:46:37 +0000
+++ lib/lp/registry/browser/product.py	2010-03-23 15:45:38 +0000
@@ -983,7 +983,8 @@
         super(ProductPackagesPortletView, self).setUpFields()
         ubuntu = getUtility(ILaunchpadCelebrities).ubuntu
         source_packages = ubuntu.searchSourcePackages(
-            self.context.name, has_packaging=False)
+            self.context.name, has_packaging=False,
+            publishing_distroseries=ubuntu.currentseries)
         # Based upon the matches, create a new vocabulary with
         # term descriptions that include a link to the source package.
         self.suggestions = []

=== modified file 'lib/lp/registry/browser/productseries.py'
--- lib/lp/registry/browser/productseries.py	2010-03-09 22:06:12 +0000
+++ lib/lp/registry/browser/productseries.py	2010-03-23 15:45:38 +0000
@@ -442,7 +442,7 @@
         if sourcepackagename is None:
             message = "You must choose the source package name."
             self.setFieldError('sourcepackagename', message)
-        # Do not allow users it create links to unpublished Ubuntu packages.
+        # Do not allow users to create links to unpublished Ubuntu packages.
         elif distroseries.distribution.full_functionality:
             source_package = distroseries.getSourcePackage(sourcepackagename)
             if source_package.currentrelease is None:

=== modified file 'lib/lp/registry/browser/tests/product-portlet-packages-view.txt'
--- lib/lp/registry/browser/tests/product-portlet-packages-view.txt	2010-03-01 21:48:15 +0000
+++ lib/lp/registry/browser/tests/product-portlet-packages-view.txt	2010-03-23 15:45:38 +0000
@@ -80,11 +80,9 @@
     >>> print view.suggestions
     []
 
-An Ubuntu distribution package that is not in the current series
-will be suggested.
+A source package that does NOT have a publishing history
+for the current Ubuntu series will NOT be suggested.
 
-    >>> distro_package = factory.makeDistributionSourcePackage(
-    ...     sourcepackagename=spn, distribution=ubuntu)
     >>> warty = ubuntu.getSeries('warty')
     >>> warty == ubuntu.currentseries
     False
@@ -96,6 +94,18 @@
     ...     principal=product.owner)
     >>> for dsp in view.suggestions:
     ...     print dsp.name
+
+A source package that does have a publishing history for the current
+Ubuntu series will be suggested.
+
+    >>> spph = factory.makeSourcePackagePublishingHistory(
+    ...     sourcepackagename=spn, distroseries=ubuntu.currentseries)
+    >>> (ubuntu, product, spn) = updateCache()
+    >>> view = create_initialized_view(
+    ...     product, name="+portlet-packages",
+    ...     principal=product.owner)
+    >>> for dsp in view.suggestions:
+    ...     print dsp.name
     bingo
 
 And the user is presented with a form to select the distribution

=== modified file 'lib/lp/registry/doc/distribution.txt'
--- lib/lp/registry/doc/distribution.txt	2010-03-09 16:26:04 +0000
+++ lib/lp/registry/doc/distribution.txt	2010-03-23 15:45:38 +0000
@@ -219,6 +219,18 @@
     DistributionSourcePackage: foobar
     DistributionSourcePackage: commercialpackage
 
+searchSourcePackages() also has a publishing_distroseries parameter that
+it just passes on to searchSourcePackageCaches(), and it restricts the
+results based on whether the source package has an entry in the
+SourcePackagePublishingHistory table for the given distroseries.
+
+    >>> packages = ubuntu.searchSourcePackages(
+    ...     'a', publishing_distroseries=ubuntu.currentseries)
+    >>> for dsp in packages:
+    ...     print "%s: %s" % (dsp.__class__.__name__, dsp.name)
+    DistributionSourcePackage: netapplet
+    DistributionSourcePackage: alsa-utils
+
 
 === Searching for binary packages ===
 

=== modified file 'lib/lp/registry/interfaces/distribution.py'
--- lib/lp/registry/interfaces/distribution.py	2010-03-09 16:26:04 +0000
+++ lib/lp/registry/interfaces/distribution.py	2010-03-23 15:45:38 +0000
@@ -406,14 +406,16 @@
     # _schema_circular_imports.py.
     @operation_returns_collection_of(Interface)
     @export_read_operation()
-    def searchSourcePackages(text, has_packaging=None):
+    def searchSourcePackages(
+        text, has_packaging=None, publishing_distroseries=None):
         """Search for source packages that correspond to the given text.
 
         This method just decorates the result of searchSourcePackageCaches()
         to return DistributionSourcePackages.
         """
 
-    def searchSourcePackageCaches(text, has_packaging=None):
+    def searchSourcePackageCaches(
+        text, has_packaging=None, publishing_distroseries=None):
         """Search for source packages that correspond to the given text.
 
         :param text: The text that will be matched.
@@ -421,6 +423,9 @@
             packages with no packaging (i.e. no link to the upstream
             project). False will do the reverse filtering, and None
             will do no filtering on this field.
+        :param publishing_distroseries: If it is not None, then
+            it will filter out source packages that do not have a
+            publishing history for the given distroseries.
         :return: A result set containing
             (DistributionSourcePackageCache, SourcePackageName, rank) tuples
             ordered by rank.

=== modified file 'lib/lp/registry/model/distribution.py'
--- lib/lp/registry/model/distribution.py	2010-03-09 16:26:04 +0000
+++ lib/lp/registry/model/distribution.py	2010-03-23 15:45:38 +0000
@@ -928,7 +928,7 @@
         cache.changelog = ' '.join(sorted(sprchangelog))
 
     def searchSourcePackageCaches(
-        self, text, has_packaging=None):
+        self, text, has_packaging=None, publishing_distroseries=None):
         """See `IDistribution`."""
         # The query below tries exact matching on the source package
         # name as well; this is because source package names are
@@ -949,6 +949,18 @@
                 )
             ]
 
+        publishing_condition = ''
+        if publishing_distroseries is not None:
+            publishing_condition = """
+                AND EXISTS (
+                    SELECT 1
+                    FROM SourcePackageRelease spr
+                        JOIN SourcePackagePublishingHistory spph
+                            ON spph.sourcepackagerelease = spr.id
+                    WHERE spr.sourcepackagename = SourcePackageName.id
+                        AND spph.distroseries = %d
+                    )""" % (
+                publishing_distroseries.id)
 
         packaging_query = """
             SELECT 1
@@ -965,24 +977,28 @@
         # Storm expressions, a 'tuple index out-of-range' error was always
         # raised.
         condition = """
-            distribution = %s AND
-            archive IN %s AND
-            (fti @@ ftq(%s) OR
+            DistributionSourcePackageCache.distribution = %s AND
+            DistributionSourcePackageCache.archive IN %s AND
+            (DistributionSourcePackageCache.fti @@ ftq(%s) OR
              DistributionSourcePackageCache.name ILIKE '%%' || %s || '%%')
             %s
+            %s
             """ % (quote(self), quote(self.all_distro_archive_ids),
-                   quote(text), quote_like(text), has_packaging_condition)
+                   quote(text), quote_like(text), has_packaging_condition,
+                   publishing_condition)
         dsp_caches_with_ranks = store.using(*origin).find(
             find_spec, condition
             ).order_by('rank DESC')
 
         return dsp_caches_with_ranks
 
-    def searchSourcePackages(self, text, has_packaging=None):
+    def searchSourcePackages(
+        self, text, has_packaging=None, publishing_distroseries=None):
         """See `IDistribution`."""
 
         dsp_caches_with_ranks = self.searchSourcePackageCaches(
-            text, has_packaging=has_packaging)
+            text, has_packaging=has_packaging,
+            publishing_distroseries=publishing_distroseries)
 
         # Create a function that will decorate the resulting
         # DistributionSourcePackageCaches, converting

=== modified file 'lib/lp/registry/model/productseries.py'
--- lib/lp/registry/model/productseries.py	2010-03-17 09:13:05 +0000
+++ lib/lp/registry/model/productseries.py	2010-03-23 15:45:38 +0000
@@ -378,6 +378,12 @@
 
     def setPackaging(self, distroseries, sourcepackagename, owner):
         """See IProductSeries."""
+        if distroseries.distribution.full_functionality:
+            source_package = distroseries.getSourcePackage(sourcepackagename)
+            if source_package.currentrelease is None:
+                raise AssertionError(
+                    "The source package is not published in %s." %
+                    distroseries.displayname)
         for pkg in self.packagings:
             if pkg.distroseries == distroseries:
                 # we have found a matching Packaging record

=== modified file 'lib/lp/registry/tests/test_distroseries.py'
--- lib/lp/registry/tests/test_distroseries.py	2010-02-25 16:27:34 +0000
+++ lib/lp/registry/tests/test_distroseries.py	2010-03-23 15:45:38 +0000
@@ -1,7 +1,7 @@
 # Copyright 2009 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
-"""Tests for Distribution."""
+"""Tests for distroseries."""
 
 __metaclass__ = type
 

=== modified file 'lib/lp/registry/tests/test_productseries.py'
--- lib/lp/registry/tests/test_productseries.py	2010-03-18 13:27:47 +0000
+++ lib/lp/registry/tests/test_productseries.py	2010-03-23 15:45:38 +0000
@@ -9,13 +9,68 @@
 
 from zope.component import getUtility
 
+from canonical.launchpad.ftests import login
+from canonical.testing import LaunchpadFunctionalLayer
 from canonical.testing import ZopelessDatabaseLayer
+
 from lp.testing import TestCaseWithFactory
+from lp.registry.interfaces.distribution import IDistributionSet
+from lp.registry.interfaces.distroseries import IDistroSeriesSet
 from lp.registry.interfaces.productseries import IProductSeriesSet
 from lp.translations.interfaces.translations import (
     TranslationsBranchImportMode)
 
 
+class TestProductSeriesSetPackaging(TestCaseWithFactory):
+    """Test for ProductSeries.setPackaging()."""
+
+    layer = LaunchpadFunctionalLayer
+
+    def setUp(self):
+        TestCaseWithFactory.setUp(self)
+        login('[e-mail]')
+
+        self.sourcepackagename = self.factory.makeSourcePackageName()
+
+        # Set up productseries.
+        self.person = self.factory.makePerson()
+        self.product = self.factory.makeProduct(owner=self.person)
+        self.dev_focus = self.product.development_focus
+        self.product_series = self.factory.makeProductSeries(self.product)
+
+        # Set up distroseries.
+        self.distroseries_set = getUtility(IDistroSeriesSet)
+        self.distribution_set = getUtility(IDistributionSet)
+        self.ubuntu = self.distribution_set.getByName("ubuntu")
+        self.debian = self.distribution_set.getByName("debian")
+        self.ubuntu_series = self.factory.makeDistroSeries(self.ubuntu)
+        self.debian_series = self.factory.makeDistroSeries(self.debian)
+
+    def test_setPackaging_without_publishing_history(self):
+        # Fully functional (ubuntu) distributions are prevented from
+        # having a packaging entry for a distroseries that does not
+        # have a source package publishing history.
+        self.assertRaises(
+            AssertionError,
+            self.product_series.setPackaging,
+            self.ubuntu_series, self.sourcepackagename, self.person)
+
+    def test_setPackaging_with_publishing_history(self):
+        # Add the source package publishing history to the distroseries
+        # so that the packaging can be added successfully.
+        self.spph = self.factory.makeSourcePackagePublishingHistory(
+            sourcepackagename=self.sourcepackagename,
+            distroseries=self.ubuntu_series)
+        self.product_series.setPackaging(
+            self.ubuntu_series, self.sourcepackagename, self.person)
+
+    def test_setPackaging_not_ubuntu(self):
+        # A non-fully-functional distribution does not need a source
+        # package publishing history before adding the packaging entry.
+        self.product_series.setPackaging(
+            self.debian_series, self.sourcepackagename, self.person)
+
+
 class TestProductSeriesDrivers(TestCaseWithFactory):
     """Test the 'drivers' attribute of a ProductSeries."""