Merge lp:~allenap/maas/zone-sort-by-name--bug-1262160 into lp:~maas-committers/maas/trunk

Proposed by Gavin Panella
Status: Merged
Approved by: Gavin Panella
Approved revision: no longer in the source branch.
Merged at revision: 1801
Proposed branch: lp:~allenap/maas/zone-sort-by-name--bug-1262160
Merge into: lp:~maas-committers/maas/trunk
Diff against target: 30 lines (+2/-7)
2 files modified
src/maasserver/models/zone.py (+1/-0)
src/maasserver/tests/test_views_nodes.py (+1/-7)
To merge this branch: bzr merge lp:~allenap/maas/zone-sort-by-name--bug-1262160
Reviewer Review Type Date Requested Status
Jeroen T. Vermeulen (community) Approve
Review via email: mp+199460@code.launchpad.net

Commit message

Sort nodes by zone name instead of zone ID.

To post a comment you must log in.
Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :

That easy? What a relief! Django has its good sides, eh?

Instead of hard-coding zone names in the test, I'd just create a slightly longer list and let factory.make_zone() come up with names...

    zones = [factory.make_zone() for _ in range(5)]

The longer list will make it most likely that failures to sort will be exposed.

review: Approve
Revision history for this message
Gavin Panella (allenap) wrote :

> That easy? What a relief! Django has its good sides, eh?
>
> Instead of hard-coding zone names in the test, I'd just create a slightly
> longer list and let factory.make_zone() come up with names...
>
> zones = [factory.make_zone() for _ in range(5)]
>
> The longer list will make it most likely that failures to sort will be
> exposed.

Okay, I'll do that. There's the possibility of false success, but it's low.

I'll also remove the TODO you added about this; I missed that.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/maasserver/models/zone.py'
2--- src/maasserver/models/zone.py 2013-12-18 12:27:33 +0000
3+++ src/maasserver/models/zone.py 2013-12-18 13:46:10 +0000
4@@ -41,6 +41,7 @@
5 """Needed for South to recognize this model."""
6 verbose_name = "Availability zone"
7 verbose_name_plural = "Availability zones"
8+ ordering = ["name"]
9
10 name = CharField(
11 max_length=256, unique=True, editable=True,
12
13=== modified file 'src/maasserver/tests/test_views_nodes.py'
14--- src/maasserver/tests/test_views_nodes.py 2013-12-18 11:16:29 +0000
15+++ src/maasserver/tests/test_views_nodes.py 2013-12-18 13:46:10 +0000
16@@ -181,13 +181,7 @@
17 if link.startswith('/nodes/node')])
18
19 def test_node_list_sorts_by_zone(self):
20- # TODO JeroenVermeulen 2013-12-18, bug=1262160: This is cheating.
21- # Sorting by zone should sort by zone name, but it actually sorts
22- # by zone ID. Make this test pass with the default random names!
23- zones = [
24- factory.make_zone(name='z-%d' % counter)
25- for counter in range(3)
26- ]
27+ zones = [factory.make_zone() for _ in range(5)]
28 nodes = [factory.make_node(zone=zone) for zone in zones]
29
30 # First check the ascending sort order