-
Notifications
You must be signed in to change notification settings - Fork 14k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allowing to set the sort order during query with "group by" #3439
Conversation
Coverage increased (+0.004%) to 69.121% when pulling baaa4a1e330ec628db05786beea50bc838b4edb9 on tc-dc:fmenges/order_direction into 3dfdde1 on apache:master. |
LGTM but please rebase (fixed the build on |
baaa4a1
to
640c953
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing control?
@@ -137,6 +137,9 @@ def query_obj(self): | |||
row_limit = int( | |||
form_data.get("row_limit") or config.get("ROW_LIMIT")) | |||
|
|||
# default order direction | |||
order_desc = form_data.get("order_desc", True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like there's a control (in controls.jsx) missing in this PR (?)
I'm actually working on something related, merged to build around this. |
K, sounds good. |
If you run a "group by" query with a "series limit" you can specify the "sort by" metric but not the sort order. In the following example you can see that you get the top 5 names. There is currently no way to look at the bottom 5 names. (sort order is hard coded to desc).
This change includes the backend changes to add this functionality. I'm happy to help on the front end as well.