launchpad-reviewers team mailing list archive

[Steve Kowalik <stevenk@xxxxxxxxxx>]

Steve Kowalik has proposed merging lp:~stevenk/launchpad/no-active-for-dsd-spr into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #874095 in Launchpad itself: "AttributeError: 'NoneType' object has no attribute 'version' in DistroSeriesDifference page"
  https://bugs.launchpad.net/launchpad/+bug/874095

For more details, see:
https://code.launchpad.net/~stevenk/launchpad/no-active-for-dsd-spr/+merge/93762

Remove the need for publications to be active (that is, PENDING or PUBLISHED) when searching for publications in DSD._package_release(). This is the #2 OOPS in our list, and is happening around 600 times a week. It would have been fine until we started running domination over Debian, and then publications in Debian would have started moving to SUPERSEDED.

I have also removed the separate DistroSeriesDifferenceLibrarianTestCase, since it contained one test, and didn't look to buy us anything at all.
-- 
https://code.launchpad.net/~stevenk/launchpad/no-active-for-dsd-spr/+merge/93762
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/no-active-for-dsd-spr into lp:launchpad.

=== modified file 'lib/lp/registry/model/distroseriesdifference.py'
--- lib/lp/registry/model/distroseriesdifference.py	2012-01-17 21:45:24 +0000
+++ lib/lp/registry/model/distroseriesdifference.py	2012-02-20 04:29:19 +0000
@@ -1,4 +1,4 @@
-# Copyright 2010-2011 Canonical Ltd.  This software is licensed under the
+# Copyright 2010-2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Database classes for a difference between two distribution series."""
@@ -705,8 +705,7 @@
     def _package_release(self, distro_series, version):
         pubs = distro_series.main_archive.getPublishedSources(
             name=self.source_package_name.name, version=version,
-            status=active_publishing_status, distroseries=distro_series,
-            exact_match=True)
+            distroseries=distro_series, exact_match=True)
 
         # Get the most recent publication (pubs are ordered by
         # (name, id)).

=== modified file 'lib/lp/registry/tests/test_distroseriesdifference.py'
--- lib/lp/registry/tests/test_distroseriesdifference.py	2012-01-17 21:45:24 +0000
+++ lib/lp/registry/tests/test_distroseriesdifference.py	2012-02-20 04:29:19 +0000
@@ -1,4 +1,4 @@
-# Copyright 2010-2011 Canonical Ltd.  This software is licensed under the
+# Copyright 2010-2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Model tests for the DistroSeriesDifference class."""
@@ -934,10 +934,20 @@
             pub,
             ds_diff.parent_source_package_release.publishings[0])
 
-
-class DistroSeriesDifferenceLibrarianTestCase(TestCaseWithFactory):
-
-    layer = LaunchpadFunctionalLayer
+    def test_source_package_release_superseded(self):
+        # If the publication is not actively published, it is still returned
+        # by source_package_release()
+        dsp = self.factory.makeDistroSeriesParent()
+        spn = self.factory.makeSourcePackageName()
+        spph = self.factory.makeSourcePackagePublishingHistory(
+            distroseries=dsp.derived_series,
+            archive=dsp.derived_series.main_archive, sourcepackagename=spn,
+            status=PackagePublishingStatus.SUPERSEDED)
+        dsd = getUtility(IDistroSeriesDifferenceSource).new(
+            dsp.derived_series, spn, dsp.parent_series)
+        spr = dsd.source_package_release
+        self.assertEqual(dsp.derived_series, spr.distroseries)
+        self.assertEqual(spph.sourcepackagerelease, spr.sourcepackagerelease)
 
     def test_package_diff_urls(self):
         # Only completed package diffs have urls.