Opened 7 years ago

Closed 2 years ago

#6612 closed enhancement (implemented)

sort according to table header in Compass

Reported by: gsathya Owned by: gsathya
Priority: Medium Milestone:
Component: Metrics/Compass Version:
Severity: Normal Keywords:
Cc: karsten, arma, cwacek@… Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

When a table header such as p_exit is clicked, the result should be sorted according to it.

Child Tickets

Change History (17)

comment:1 Changed 7 years ago by karsten

Component: - Select a componentCompass

comment:2 Changed 7 years ago by cypherpunks

It would also be great to have a sort drop down menu on the query page (where you hit the submit button).

comment:3 Changed 7 years ago by arma

Cc: arma added

comment:4 Changed 7 years ago by gsathya

Cc: cwacek@… added

cwacek is currently working on this.

comment:5 Changed 7 years ago by gsathya

Compass sorts according to consensus_weights and then displays
the top results. So the tablesorter sorts on top of the already sorted
data which isn't what we want. You might want to look at:
https://github.com/gsathya/compass/blob/master/compass.py#L303

One approach would be to send all the data unsorted and make the
javascript do all the sorting and display only the top results as
specified. This means that if someone wants to see only the top 10
results, they'd have to download all the data which is not a good
idea.

The other approach would be to modify compass.py to do the various
sorting stuff. This means we'd have to refresh the page every time the
user selects a different header to sort by. The advantage of this
approach would be that the command line part of Compass would get all
these benefits as well. IMHO, this approach sounds better.

Copy pasting conversation between cwacek and me, in case it's useful to someone else.

comment:6 in reply to:  5 Changed 7 years ago by arma

Replying to gsathya:

This means we'd have to refresh the page every time the
user selects a different header to sort by.

That sounds like an acceptable price to pay.

comment:7 Changed 7 years ago by cwacek

I rewrote portions of compass.py so that it now does the sorting stuff. This means that sorting is now possible from the command line.

In addition, I AJAX-ified the front-end using AngularJS, so now all requests are made asynchronously, and the table can be sorted by clicking on the table headers.

My changes are available in the 'angularize' branch of my github repository at https://github.com/cwacek/compass-1/tree/angularize (git clone git://github.com/cwacek/compass-1.git).

comment:8 Changed 7 years ago by karsten

I tried out the new CLI version, and everything looks good there. But how would I deploy the new web version? When I git checkout your branch and restart Apache, I can load the Compass page, but it says "No module: Compass" in http://localhost/static/js/angular/angular.js in the error console. What did I miss?

comment:9 Changed 7 years ago by gsathya

Status: newneeds_revision

Thanks a lot for the changes! This is a pretty big merge which changes a lot of things and I'm still only halfway through reviewing this. I wanted to hold of commenting on this until I finished the review, but I'd like to sort out a few things before we proceed.

1) The URL's no longer change (it's not bookmark-able -- you can't share it). Is this ok? I distinctly remember arma wanting this in the beginning.
2) Why use angular.js if the sorting is going to be done by the backend?
3) When grouping by country, the fingerprint column only says "(XXX rel)" (should be "XXX relays") and remains a hyperlink.

comment:10 in reply to:  8 Changed 7 years ago by cwacek

Replying to karsten:

I tried out the new CLI version, and everything looks good there. But how would I deploy the new web version? When I git checkout your branch and restart Apache, I can load the Compass page, but it says "No module: Compass" in http://localhost/static/js/angular/angular.js in the error console. What did I miss?

I tested using the built in server, but not Apache so I'm not sure. I'll set up an Apache instance and test it there to see what's wrong.

comment:11 in reply to:  9 ; Changed 7 years ago by cwacek

Replying to gsathya:

Thanks a lot for the changes! This is a pretty big merge which changes a lot of things and I'm still only halfway through reviewing this. I wanted to hold of commenting on this until I finished the review, but I'd like to sort out a few things before we proceed.

1) The URL's no longer change (it's not bookmark-able -- you can't share it). Is this ok? I distinctly remember arma wanting this in the beginning.

Ah, I was not there in the beginning so I didn't know this. However, I think that this is a solvable problem (and this ties into your next question), because I can make Angular make sure the urls are bookmarkable. I'll take a look and see if this can be done.

2) Why use angular.js if the sorting is going to be done by the backend?

Angular handles the AJAX callbacks very nicely in terms of updating the data displayed. It also might make addressing your first point very easy.

3) When grouping by country, the fingerprint column only says "(XXX rel)" (should be "XXX relays") and remains a hyperlink.

Ack I'll fix that.

comment:12 in reply to:  11 ; Changed 7 years ago by cwacek

Replying to cwacek:

Replying to gsathya:

Thanks a lot for the changes! This is a pretty big merge which changes a lot of things and I'm still only halfway through reviewing this. I wanted to hold of commenting on this until I finished the review, but I'd like to sort out a few things before we proceed.

1) The URL's no longer change (it's not bookmark-able -- you can't share it). Is this ok? I distinctly remember arma wanting this in the beginning.

Ah, I was not there in the beginning so I didn't know this. However, I think that this is a solvable problem (and this ties into your next question), because I can make Angular make sure the urls are bookmarkable. I'll take a look and see if this can be done.

2) Why use angular.js if the sorting is going to be done by the backend?

Angular handles the AJAX callbacks very nicely in terms of updating the data displayed. It also might make addressing your first point very easy.

3) When grouping by country, the fingerprint column only says "(XXX rel)" (should be "XXX relays") and remains a hyperlink.

Ack I'll fix that.

I've pushed a couple changes that address the points you've raised gsathya. URLs now map to the queries that are made, allowing one to 'save' or bookmark searches. I also fixed the handling of the FP field when grouping by country.

comment:13 in reply to:  12 ; Changed 7 years ago by gsathya

Status: needs_revisionneeds_review

Replying to cwacek:

I've pushed a couple changes that address the points you've raised gsathya. URLs now map to the queries that are made, allowing one to 'save' or bookmark searches. I also fixed the handling of the FP field when grouping by country.

Great, thanks! I just tested it and it's fine :)

I'll wait until Karsten ACK's this and then merge it. (changing it to needs_review)

comment:14 in reply to:  13 ; Changed 7 years ago by karsten

Replying to gsathya:

I'll wait until Karsten ACK's this and then merge it. (changing it to needs_review)

Does it work with Apache yet?

comment:15 in reply to:  14 Changed 7 years ago by cwacek

Replying to karsten:

Replying to gsathya:

I'll wait until Karsten ACK's this and then merge it. (changing it to needs_review)

Does it work with Apache yet?

It should now. I just pushed a new commit to my repo which should fix a relative URL issue. Let me know if it still doesn't work.

comment:16 Changed 7 years ago by karsten

Status: needs_reviewnew

Merged and deployed. Thanks! (gsathya, given that you already reviewed the code, I figured it's easier to just merge it instead of ACK'ing and waiting for you to do it.)

Is there a way to make it more obvious that columns can now be sorted?

comment:17 Changed 2 years ago by karsten

Resolution: implemented
Severity: Normal
Status: newclosed

Given that this feature is merged and deployed as of 5 years ago, let's not keep this ticket open just because it could be better documented.

Note: See TracTickets for help on using tickets.