Code review comment for lp:~dimitern/maas/bug-994887-nodes-listing-does-not-support-sorting

I haven't gone through this in detail, but my first concern is how this
interacts with pagination. Does this just sort the visible nodes, or does
it re do the query with a new sort order? (It is nice to nit have to reload
the page, but with >1k nodes, we won't display them all at once, and then
sorting wouldn't be very useful)

John
=:->
On Oct 19, 2012 8:31 PM, "Dimiter Naydenov" <email address hidden>
wrote:

> The proposal to merge
> lp:~dimitern/maas/bug-994887-nodes-listing-does-not-support-sorting into
> lp:maas has been updated.
>
> Description changed to:
>
> Implemented sorting for /MAAS/nodes/ list - by hostname or status
> (asc/desc). As advised, I pulled only what was needed from the project
> django-sortable and I'm using it in the views/templates. Since
> django-sortable is not in main archive, we cannot use it, but it's only a
> tiny package, so I extracted what was needed and added it to maasserver
> (also did some style formatting so it'll conform to our code).
>
> There are a couple of very minor UI changes, which I was advised also to
> request a separate review from Huw Wilkins. The added elements are 2
> up/down arrows appearing on the right of the two table columns - MAC and
> Status. Here are screenshots how it looks in all states:
>
> http://people.canonical.com/~dimitern/maas-nodes-sort-none.png
> http://people.canonical.com/~dimitern/maas-nodes-sort-mac-asc.png
> http://people.canonical.com/~dimitern/maas-nodes-sort-mac-desc.png
> http://people.canonical.com/~dimitern/maas-nodes-sort-status-asc.png
> http://people.canonical.com/~dimitern/maas-nodes-sort-status-desc.png
>
> For more details, see:
>
> https://code.launchpad.net/~dimitern/maas/bug-994887-nodes-listing-does-not-support-sorting/+merge/130600
> --
>
> https://code.launchpad.net/~dimitern/maas/bug-994887-nodes-listing-does-not-support-sorting/+merge/130600
> You are subscribed to branch lp:maas.
>