Merge lp:~dimitern/maas/bug-994887-alternate-nodes-list-sorting into lp:~maas-committers/maas/trunk
Proposed by
Dimiter Naydenov
Status: | Merged | ||||
---|---|---|---|---|---|
Approved by: | Dimiter Naydenov | ||||
Approved revision: | no longer in the source branch. | ||||
Merged at revision: | 1294 | ||||
Proposed branch: | lp:~dimitern/maas/bug-994887-alternate-nodes-list-sorting | ||||
Merge into: | lp:~maas-committers/maas/trunk | ||||
Diff against target: |
309 lines (+178/-9) 6 files modified
src/maasserver/static/css/components/table_list.css (+11/-0) src/maasserver/templates/maasserver/node_list.html (+1/-1) src/maasserver/templates/maasserver/nodes_listing.html (+7/-0) src/maasserver/templates/maasserver/tag_view.html (+1/-1) src/maasserver/tests/test_views_nodes.py (+115/-5) src/maasserver/views/nodes.py (+43/-2) |
||||
To merge this branch: | bzr merge lp:~dimitern/maas/bug-994887-alternate-nodes-list-sorting | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Martin Packman (community) | Approve | ||
John A Meinel (community) | Approve | ||
Review via email: mp+131177@code.launchpad.net |
Commit message
Fix bug #994887: Nodes list view now supports sorting by hostname(mac) or status columns.
Description of the change
Implemented sorting for /MAAS/nodes/ list - by hostname or status (asc/desc). This is an alternative, simpler solution, compared to https:/
To post a comment you must log in.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 10/24/2012 02:57 PM, Dimiter Naydenov wrote: reviewers) /bugs.launchpad .net/maas/ +bug/994887 /code.launchpad .net/~dimitern/ maas/bug- 994887- alternate- nodes-list- sorting/ +merge/ 131177 /code.launchpad .net/~dimitern/ maas/bug- 994887- nodes-listing- does-not- support- sorting/ +merge/ 130600
> Dimiter Naydenov has proposed merging
> lp:~dimitern/maas/bug-994887-alternate-nodes-list-sorting into
> lp:maas.
>
> Commit message: Fix bug #994887: Nodes list view now supports
> sorting by hostname(mac) or status columns.
>
> Requested reviews: Launchpad code reviewers (launchpad-
> Related bugs: Bug #994887 in MAAS: "Nodes listing does not support
> sorting" https:/
>
> For more details, see:
> https:/
>
> Implemented sorting for /MAAS/nodes/ list - by hostname or status
> (asc/desc). This is an alternative, simpler solution, compared to
> https:/
> - not using third party modules (django-sortable) anymore. UI
> changes (sorting up/down arrows) were already approved, so just the
> code changes need approval.
>
+ # Just creare some nodes to have something in the list
typo 'create'
However, why do we need the nodes to actually get the links? I would
tend to have no more than 1 'make_node()' call.
... 'node-view' , args=[node. system_ id])
+ node_links = [
+ reverse(
+
+ for node in sorted_nodes]
The spacing in this test looks wrong. You probably want to run 'make
lint' and handle anything that it brings up as a problem.
+ # Now check the reverse order
+ sorted_nodes = sorted(
+ nodes, key=lambda x: x.status, reverse=True)
I don't think you need to re-do the sort, if anything just do sorted_ nodes)" . node_links) and not do anything with re-sorting
"sorted_nodes = reversed(
However, I think all you actually need to do is:
node_links = reversed(
the node objects themselves.
That should simplify the test a bit.
+ # Exclude header sorting links
self. assertEqual( [last_node_ link], xpath(" //div[@ id='nodes' ]/table/ /a/@href" )) xpath(" //div[@ id='nodes' ]/table/ /a/@href" )))
- - document.
+ filter(
+ lambda x: 'sort=' not in x,
+ document.
I'm hesitant about using 'filter' in these cases. I'm wondering if
there is a better way to write the tests. Perhaps giving a specific
class id for the node links, or one for the sorting links?
nodes = Node.objects. get_nodes(
user= self.request. user, prefetch_mac=True, PERMISSION. VIEW,). order_by( '-created' ) PERMISSION. VIEW,). order_by( order_by)
- - perm=NODE_
+ perm=NODE_
I kind of feel like we want to always have a sort field that is
unique. What I mean, is that if you sort by 'status' then all nodes
with the same status can occur in arbitrary order. (It is probably
reasonably static because the db isn't changing a lot in the
background, but it isn't guaranteed). Certainly an unrelated action
that would otherwise not affect the nodes being shown could cause a
node that was on page 1 to suddenly swap places with a node on page 2.
I'm not sure about the specific order of operations, but I think we
want something li...