Merge lp:~wgrant/launchpad/bug-592914-recipe-distroseries-order into lp:launchpad

[William Grant]

Sort by version the distroseries list when creating recipes and builds.

This branch fixes bug #592914 by sorting the distroseries vocabulary used when creating recipes and requesting builds. It's just reverse sorting by DistroSeries.version, which will fall apart if we ever support multiple distributions, but it will work fine for now.

I've updated the broken test, and there is no lint.

[Māris Fogels (mars)]

Hi William,

The fix looks good, but I had two concerns:

 * Would it be better to use attrgetter instead of the lambda?: sorted(.., key=attrgetter('value.version'))

 * Just a comment, but the call to term.value.version breaks the Law of Demeter (http://en.wikipedia.org/wiki/Law_of_Demeter). That often indicates an object design problem, but since we are working with data objects, I think it can pass here.

Looks good! r=mars

Maris

review: Approve

[William Grant (wgrant)]

I didn't know that attrgetter could traverse multiple levels like that. I've replaced the lambda with it.

[William Grant (wgrant)]

It turns out that attrgetter doesn't actually work like that.

[William Grant (wgrant)]

I've just worked out how it passed locally: python2.6 works as you suggested, but python2.5 only works for a single level.

=== modified file 'lib/lp/code/browser/sourcepackagerecipe.py'
--- lib/lp/code/browser/sourcepackagerecipe.py	2010-06-15 15:08:11 +0000
+++ lib/lp/code/browser/sourcepackagerecipe.py	2010-06-16 11:49:28 +0000
@@ -37,6 +37,7 @@
     LaunchpadView, Link, Navigation, NavigationMenu, stepthrough)
 from canonical.launchpad.webapp.authorization import check_permission
 from canonical.launchpad.webapp.breadcrumb import Breadcrumb
+from canonical.launchpad.webapp.sorting import sorted_dotted_numbers
 from canonical.widgets.itemswidgets import LabeledMultiCheckBoxWidget
 from lp.buildmaster.interfaces.buildbase import BuildStatus
 from lp.code.errors import ForbiddenInstruction
@@ -171,9 +172,12 @@
     ppas = getUtility(IArchiveSet).getPPAsForUser(getUtility(ILaunchBag).user)
     supported_distros = [ppa.distribution for ppa in ppas]
     dsset = getUtility(IDistroSeriesSet).search()
-    terms = [SimpleTerm(distro, distro.id, distro.displayname)
-             for distro in dsset if (
-                 distro.active and distro.distribution in supported_distros)]
+    terms = sorted_dotted_numbers(
+        [SimpleTerm(distro, distro.id, distro.displayname)
+         for distro in dsset if (
+         distro.active and distro.distribution in supported_distros)],
+        key=lambda term: term.value.version)
+    terms.reverse()
     return SimpleVocabulary(terms)
 
 def target_ppas_vocabulary(context):

=== modified file 'lib/lp/code/browser/tests/test_sourcepackagerecipe.py'
--- lib/lp/code/browser/tests/test_sourcepackagerecipe.py	2010-06-15 15:08:11 +0000
+++ lib/lp/code/browser/tests/test_sourcepackagerecipe.py	2010-06-16 11:49:28 +0000
@@ -42,7 +42,7 @@
         self.ppa = self.factory.makeArchive(
             displayname='Secret PPA', owner=self.chef, name='ppa')
         self.squirrel = self.factory.makeDistroSeries(
-            displayname='Secret Squirrel', name='secret',
+            displayname='Secret Squirrel', name='secret', version='100.04',
             distribution=self.ppa.distribution)
         self.squirrel.nominatedarchindep = self.squirrel.newArch(
             'i386', ProcessorFamily.get(1), False, self.chef,
@@ -516,8 +516,8 @@
             Secret PPA (chef/ppa)
             Distribution series:
             Secret Squirrel
+            Hoary
             Warty
-            Hoary
             or
             Cancel""")
         main_text = self.getMainText(recipe, '+request-builds')