Merge lp:~jcsackett/launchpad/620494-downloads-sort into lp:launchpad

[j.c.sackett]

Fixes sorting on plus download by adding milestone name as an additional sort parameter.

= Summary =

Fixes an edge case sorting releases on +download by adding an additional sort field. Per the bug, if too releases have the same date, they can be sorted wrong from the fallback to database id.

== Proposed fix ==

Add an additional sort by milestone.name; this catches the edge case and displays the releases correctly since most milestones use numeric names.

== Pre-implementation notes ==

Talked with Curtis.

== Implementation details ==

As in Proposed fix.

== Tests ==

bin/test -vvc -t getMilestonesAndReleases

== Demo and Q/A ==

On Thunderbird in dev (launchpad.dev/thunderbird) create two releases on trunk with milestones 0.99 and 0.98; make sure they have downloadable files. On http://launchpad.dev/thunderbird/+download you should see 0.99 listed before 0.98.

= Launchpad lint =
Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/registry/model/product.py
  lib/lp/registry/tests/test_product.py

[Aaron Bentley (abentley)]

review: Approve

[Robert Collins (lifeless)]

I think that someone is adding a database sort function for milestone
names anyhow; you should probably use that.

=== modified file 'lib/lp/registry/model/product.py'
--- lib/lp/registry/model/product.py	2010-08-22 19:26:46 +0000
+++ lib/lp/registry/model/product.py	2010-08-24 14:48:47 +0000
@@ -1122,6 +1122,11 @@
             And(Milestone.product == self,
                 Milestone.name == version)).one()
 
+    # XXX: jcsackett 2010-08-23 bug=620494
+    # The second clause in the order_by in this method is a bandaid
+    # on a sorting issue caused by date vs datetime conflicts in the
+    # database. A fix is coming out, but this deals with the edge
+    # case responsible for the referenced bug.
     def getMilestonesAndReleases(self):
         """See `IProduct`."""
         store = Store.of(self)
@@ -1130,7 +1135,9 @@
             And(ProductRelease.milestone == Milestone.id,
                 Milestone.productseries == ProductSeries.id,
                 ProductSeries.product == self))
-        return result.order_by(Desc(ProductRelease.datereleased))
+        return result.order_by(
+            Desc(ProductRelease.datereleased),
+            Desc(Milestone.name))
 
     def packagedInDistros(self):
         return IStore(Distribution).find(

=== modified file 'lib/lp/registry/tests/test_product.py'
--- lib/lp/registry/tests/test_product.py	2010-08-22 19:26:46 +0000
+++ lib/lp/registry/tests/test_product.py	2010-08-24 14:48:47 +0000
@@ -60,6 +60,38 @@
         product.active = False
         self.assertEqual(False, product.active)
 
+    def test_milestone_sorting_getMilestonesAndReleases(self):
+        product = self.factory.makeProduct()
+        series = self.factory.makeProductSeries(product=product)
+        milestone_0_1 = self.factory.makeMilestone(
+            product=product,
+            productseries=series,
+            name='0.1')
+        milestone_0_2 = self.factory.makeMilestone(
+            product=product,
+            productseries=series,
+            name='0.2')
+        release_1 = self.factory.makeProductRelease(
+            product=product,
+            milestone=milestone_0_1)
+        release_2 = self.factory.makeProductRelease(
+            product=product,
+            milestone=milestone_0_2)
+        release_file1 = self.factory.makeProductReleaseFile(
+            product=product,
+            release=release_1,
+            productseries=series,
+            milestone=milestone_0_1)
+        release_file2 = self.factory.makeProductReleaseFile(
+            product=product,
+            release=release_2,
+            productseries=series,
+            milestone=milestone_0_2)
+        expected = [(milestone_0_2, release_2), (milestone_0_1, release_1)]
+        self.assertEqual(
+            expected,
+            list(product.getMilestonesAndReleases()))
+
 
 class TestProductFiles(unittest.TestCase):
     """Tests for downloadable product files."""