Merge lp:~adeuring/launchpad/bug-746460-productseries into lp:launchpad

[Abel Deuring]

[bug=760699] [r=jcsackett] ProductSeries.setPackaging() does no longer change existing packaging records, if the distroseries passed as a parameter matches the distroseries attribue of an existing packaging record that points to another sourcepackage.

This branch fixes bug 760699: ProductSeries.setPackaging() should not change existing records

It also ensures that ProductSeries.setPackaging() and SourcePacjage.setPackaging() behave identical.

The fix is quite straightforward: I deleted those lines in ProductSeries.setPackaging() that change an existing packaging record.

test: ./bin/test -vvt lp.registry.tests.test_productseries.TestProductSeriesSetPackaging

no lint

[j.c.sackett (jcsackett)]

This looks fine.

review: Approve

=== modified file 'lib/lp/registry/browser/tests/packaging-views.txt'
--- lib/lp/registry/browser/tests/packaging-views.txt	2011-04-11 16:00:53 +0000
+++ lib/lp/registry/browser/tests/packaging-views.txt	2011-04-14 14:04:14 +0000
@@ -157,10 +157,13 @@
     >>> view.errors
     []
 
+We now have two source packages linked to our productseries.
+
     >>> for packaging in view.ubuntu_history:
     ...     print packaging.distroseries.name
     ...     print packaging.sourcepackagename.name
     hoary thunderbird
+    hoary hot
 
 It is not an error to submit the same sourcepackagename information, the
 action is ignored because there is no change
@@ -178,6 +181,7 @@
     ...     print packaging.distroseries.name
     ...     print packaging.sourcepackagename.name
     hoary thunderbird
+    hoary hot
 
 When the current Ubuntu series changes, the sourcepackagename is not known,
 and a new entry can be added to the packaging history.
@@ -213,6 +217,7 @@
     ...     print packaging.sourcepackagename.name
     grumpy hot
     hoary thunderbird
+    hoary hot
 
 
 Product packages view
@@ -230,12 +235,15 @@
     ...     for series in view.series_batch.batch:
     ...         print series.name
     ...         for package in series.packagings:
-    ...             print '  Package:', package.distroseries.name
+    ...             print '  Package %s: %s' % (
+    ...                 package.sourcepackagename.name,
+    ...                 package.distroseries.name)
     >>> print_packages(view)
     trunk
     hotter
-      Package: grumpy
-      Package: hoary
+      Package hot: grumpy
+      Package thunderbird: hoary
+      Package hot: hoary
     cold
 
 The view provides the distro_packaging property that is a list of
@@ -254,7 +262,8 @@
     >>> [hoary_package] = [
     ...     package for series in view.series_batch.batch
     ...     for package in series.packagings
-    ...     if package.distroseries.name == 'hoary']
+    ...     if package.distroseries.name == 'hoary' and
+    ...         package.sourcepackagename.name == 'thunderbird']
     >>> login_person(hoary_package.sourcepackage.currentrelease.maintainer)
     >>> view = create_initialized_view(
     ...     product, name='+packages',
@@ -262,8 +271,9 @@
     >>> print_packages(view)
     trunk
     hotter
-      Package: grumpy
-      Package: hoary
+      Package hot: grumpy
+      Package thunderbird: hoary
+      Package hot: hoary
     cold
 
     # There are links to the +remove-packaging page.
@@ -285,7 +295,8 @@
     >>> print_packages(view)
     trunk
     hotter
-      Package: grumpy
+      Package hot: grumpy
+      Package hot: hoary
     cold
 
 
@@ -319,6 +330,7 @@
     ...     print packaging.sourcepackagename.name
     netapplet
     evolution
+    hot
 
 
 Distro series +needs-packaging view
@@ -351,5 +363,4 @@
     cnews
     libstdc++
     linux-source-2.6.15
-    hot
     thunderbird

=== modified file 'lib/lp/registry/model/productseries.py'
--- lib/lp/registry/model/productseries.py	2011-04-12 06:21:39 +0000
+++ lib/lp/registry/model/productseries.py	2011-04-14 14:04:14 +0000
@@ -497,16 +497,10 @@
                     "The source package is not published in %s." %
                     distroseries.displayname)
         for pkg in self.packagings:
-            if pkg.distroseries == distroseries:
+            if (pkg.distroseries == distroseries and
+                pkg.sourcepackagename == sourcepackagename):
                 # we have found a matching Packaging record
-                if pkg.sourcepackagename == sourcepackagename:
-                    # and it has the same source package name
-                    return pkg
-                # ok, we need to update this pkging record
-                pkg.sourcepackagename = sourcepackagename
-                pkg.owner = owner
-                pkg.datecreated = UTC_NOW
-                pkg.sync()  # convert UTC_NOW to actual datetime
+                # and it has the same source package name
                 return pkg
 
         # ok, we didn't find a packaging record that matches, let's go ahead
@@ -672,6 +666,7 @@
         """
         seriesID = self.id
         productID = self.productID
+
         def weight_function(bugtask):
             if bugtask.productseriesID == seriesID:
                 return OrderedBugTask(1, bugtask.id, bugtask)

=== modified file 'lib/lp/registry/tests/test_productseries.py'
--- lib/lp/registry/tests/test_productseries.py	2011-03-21 09:56:49 +0000
+++ lib/lp/registry/tests/test_productseries.py	2011-04-14 14:04:14 +0000
@@ -81,6 +81,85 @@
         self.product_series.setPackaging(
             self.debian_series, self.sourcepackagename, self.person)
 
+    def makeSourcePackage(self):
+        # Create a published sourcepackage for self.ubuntu_series.
+        sourcepackage = self.factory.makeSourcePackage(
+            distroseries=self.ubuntu_series)
+        self.factory.makeSourcePackagePublishingHistory(
+            sourcepackagename=sourcepackage.sourcepackagename,
+            distroseries=self.ubuntu_series)
+        return sourcepackage
+
+    def test_setPackaging_two_packagings(self):
+        # More than one sourcepackage from the same distroseries
+        # can be linked to a productseries.
+        sourcepackage_a = self.makeSourcePackage()
+        sourcepackage_b = self.makeSourcePackage()
+        packaging_a = self.product_series.setPackaging(
+            distroseries=self.ubuntu_series,
+            sourcepackagename=sourcepackage_a.sourcepackagename,
+            owner=self.factory.makePerson())
+        packaging_b = self.product_series.setPackaging(
+            distroseries=self.ubuntu_series,
+            sourcepackagename=sourcepackage_b.sourcepackagename,
+            owner=self.factory.makePerson())
+        self.assertEqual(
+            [packaging_b, packaging_a], list(self.product_series.packagings))
+
+    def test_setPackaging_called_for_existing_multiple_packagings(self):
+        # Calling setPackaging for already existing packagings
+        # does not have any effect.
+        sourcepackage_a = self.makeSourcePackage()
+        sourcepackage_b = self.makeSourcePackage()
+        packaging_a = self.product_series.setPackaging(
+            distroseries=self.ubuntu_series,
+            sourcepackagename=sourcepackage_a.sourcepackagename,
+            owner=self.factory.makePerson())
+        packaging_b = self.product_series.setPackaging(
+            distroseries=self.ubuntu_series,
+            sourcepackagename=sourcepackage_b.sourcepackagename,
+            owner=self.factory.makePerson())
+        self.assertEqual(
+            packaging_b,
+            self.product_series.setPackaging(
+                distroseries=self.ubuntu_series,
+                sourcepackagename=sourcepackage_b.sourcepackagename,
+                owner=self.factory.makePerson()))
+        self.assertEqual(
+            packaging_a,
+            self.product_series.setPackaging(
+                distroseries=self.ubuntu_series,
+                sourcepackagename=sourcepackage_a.sourcepackagename,
+                owner=self.factory.makePerson()))
+        self.assertEqual(
+            [packaging_b, packaging_a], list(self.product_series.packagings))
+
+    def test_setPackaging__packagings_created_by_sourcepackage(self):
+        # Calling setPackaging for already existing packagings
+        # created by SourcePackage.setPackaging() does not have any effect.
+        sourcepackage_a = self.makeSourcePackage()
+        sourcepackage_b = self.makeSourcePackage()
+        sourcepackage_a.setPackaging(
+            self.product_series, owner=self.factory.makePerson())
+        sourcepackage_b.setPackaging(
+            self.product_series, owner=self.factory.makePerson())
+        packaging_a = sourcepackage_a.packaging
+        packaging_b = sourcepackage_b.packaging
+        self.assertEqual(
+            packaging_b,
+            self.product_series.setPackaging(
+                distroseries=self.ubuntu_series,
+                sourcepackagename=sourcepackage_b.sourcepackagename,
+                owner=self.factory.makePerson()))
+        self.assertEqual(
+            packaging_a,
+            self.product_series.setPackaging(
+                distroseries=self.ubuntu_series,
+                sourcepackagename=sourcepackage_a.sourcepackagename,
+                owner=self.factory.makePerson()))
+        self.assertEqual(
+            [packaging_b, packaging_a], list(self.product_series.packagings))
+
 
 class TestProductSeriesGetUbuntuTranslationFocusPackage(TestCaseWithFactory):
     """Test for ProductSeries.getUbuntuTranslationFocusPackage."""