launchpad-reviewers team mailing list archive

["j.c.sackett" <jonathan.sackett@xxxxxxxxxxxxx>]

j.c.sackett has proposed merging lp:~jcsackett/launchpad/620494-downloads-sort into lp:launchpad/devel.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  #620494 +downloads does not sort releases made on the same day properly
  https://bugs.launchpad.net/bugs/620494


= 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 above.

== 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


-- 
https://code.launchpad.net/~jcsackett/launchpad/620494-downloads-sort/+merge/33413
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jcsackett/launchpad/620494-downloads-sort into lp:launchpad/devel.

=== 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-23 16:26:01 +0000
@@ -1130,7 +1130,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-23 16:26:01 +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."""